Skip to content

feat(server): AuditSink Protocol + LoggingAuditSink/SlackAlertSink#362

Merged
bokelley merged 1 commit intomainfrom
bokelley/issue-351
May 2, 2026
Merged

feat(server): AuditSink Protocol + LoggingAuditSink/SlackAlertSink#362
bokelley merged 1 commit intomainfrom
bokelley/issue-351

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

@bokelley bokelley commented May 2, 2026

Closes #351.

Summary

Round-2 P1 observability seam paralleling DeliveryLogSink. New module src/adcp/audit_sink.py:

  • AuditEvent — frozen Pydantic model. Field naming aligns with ToolContext (caller_identity, tenant_id, request_id) so middleware copies fields without translation. extra="forbid" catches typo'd field names at construction.
  • AuditSink — runtime-checkable Protocol. Adopter extension point.
  • LoggingAuditSink — stdlib logging at INFO, structured JSON via model_dump_json().
  • SlackAlertSink — SecOps alerting on a sensitive-ops subset (NOT audit-of-record). Routes through build_async_ip_pinned_transport with trust_env=False; details egress gated by an explicit allowed_fields allowlist (default frozenset() — no details reach Slack); webhook URL redacted in __repr__.
  • make_audit_middleware() — composes one or more sinks into a SkillMiddleware. Per-sink asyncio.wait_for + log-and-swallow so a wedged sink cannot stall dispatch. Records on success AND on exception (re-raises).

Design decisions

The triage on #351 raised four open questions. Resolutions:

  1. Namingcaller_identity/tenant_id on AuditEvent, not principal_id. Matches ToolContext.
  2. SSRFSlackAlertSink uses build_async_ip_pinned_transport + trust_env=False, like WebhookSender.
  3. details filtering — explicit allowed_fields: frozenset[str] allowlist, default frozenset() (emit nothing from details to Slack unless opted in).
  4. Integrity contract — best-effort observability paralleling DeliveryLogSink. Module docstring explicitly names SOX, GDPR Art. 30, and IAB TCF as regimes this design does not satisfy, with the "implement AuditSink against a transactional store directly" escape hatch made impossible to miss.

Expert review applied

After the initial draft, ran two parallel expert reviews (Python ecosystem patterns + ad-tech observability standards). Resulting changes:

  • AuditEvent: dataclass → frozen Pydantic for SDK consistency, free model_dump_json(), and datetime validation at construction.
  • SlackAuditSinkSlackAlertSink: Slack is universally an alerting channel in ad-tech ops, never audit-of-record. Rename prevents the misuse pattern of wiring it as the only sink and assuming compliance is satisfied.
  • Strengthened compliance disclaimer with explicit SOX/GDPR/TCF callouts.

Deferred as follow-ups: OTelAuditSink reference impl (own PR — adds dep tree), AuditEvent.to_cloudevent() projector (YAGNI until a real adopter asks).

Test plan

  • 24 tests, all passing locally (ruff, mypy, pytest)
  • Sibling test_webhook_supervisor.py still passes (no regression)
  • Coverage: HTTPS rejection, repr redaction, sensitive-ops gate, IP-pinned transport wiring (via httpx.MockTransport), default allowlist drops all details, allowlist filtering, non-2xx surfaces to operator log, exception-path audit, sink timeout isolation, sink raise isolation, multi-sink continuation after one bad sink, empty-sinks no-op, unauthenticated-request handling, frozen-Pydantic assignment raises, extra="forbid" rejects unknown fields, native JSON serialization

🤖 Generated with Claude Code

…ference impls

Round-2 P1 observability seam paralleling DeliveryLogSink. AuditEvent
(frozen Pydantic) records skill dispatches; AuditSink Protocol is the
adopter extension point. make_audit_middleware() composes one or more
sinks into a SkillMiddleware with per-sink timeout + log-and-swallow so
a wedged sink can't stall dispatch.

Reference impls:
- LoggingAuditSink: stdlib logging at INFO, structured JSON via
  Pydantic model_dump_json
- SlackAlertSink: SecOps alerting on a sensitive-ops subset, routed
  through build_async_ip_pinned_transport with trust_env=False;
  details egress gated by an explicit allowed_fields allowlist
  (default frozenset() — no details reach Slack); webhook URL redacted
  in __repr__

Field naming aligns with ToolContext (caller_identity, tenant_id,
request_id) so middleware copies fields without translation.

Module docstring explicitly names SOX, GDPR Art. 30, and IAB TCF as
regimes this best-effort design does NOT satisfy — adopters needing
those implement AuditSink against a transactional store directly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit 66c3c57 into main May 2, 2026
12 checks passed
@bokelley bokelley deleted the bokelley/issue-351 branch May 2, 2026 19:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(decisioning): AuditSink Protocol + reference implementations

1 participant