feat(decisioning): AuditSink Protocol + reference implementations

[bokelley]

Summary

Round-2 P1 — observability seam paralleling the existing DeliveryLogSink. Provides an AuditSink Protocol plus LoggingAuditSink and SlackAuditSink reference implementations.

Design

Surface

• AuditSink Protocol with record(event) async method
• AuditEvent frozen dataclass: operation, principal_id, tenant_id, success, details, timestamp
• Reference implementations:
  • LoggingAuditSink — writes structured JSON via stdlib logging
  • SlackAuditSink — POSTs to a webhook URL on sensitive ops, configurable allowlist
• Adopters compose these via the existing call_next() middleware seam in adcp.server

Adoption story

Salesagent's audit_logger.py (~360 LOC) maps to a thin AuditSink adapter once this lands.

Cross-references

• RFC: docs/proposals/v3-identity-bundle-design.md
• Webhook supervisor (merged): PR #348
• JS parity tracker: adcp-client#1249

🤖 Generated with Claude Code

[bokelley]

Triage

Classification: Feature request
Bucket(s): middleware (no middleware label exists in repo — gap noted)
Status: ready-for-human

What the experts said:

• ad-tech-protocol-expert: Two blockers — (1) SlackAuditSink has no timeout/isolation contract; on a Slack outage it will block every skill dispatch since the issue places it directly in SkillMiddleware without the asyncio.wait_for + swallow wrapper DeliveryLogSink uses; (2) principal_id conflicts with ToolContext.caller_identity — the same field under two names forces an unexplained rename at every middleware call site.
• security-reviewer: Four blockers — (1) SSRF: webhook_url is not routed through the existing IpPinnedTransport that every other outbound call in this SDK uses (WebhookSender, JWKS fetcher, revocation fetcher); (2) details: dict[str, Any] sent to Slack verbatim can carry financial data and PII from params (credit limits, contact fields, budget values); (3) the Slack webhook URL is a bearer token that appears verbatim in httpx connection-error messages — needs redaction analogous to WebhookSender.__repr__; (4) audit integrity contract is undefined — the DeliveryLogSink "drop on failure" trade-off is correct for delivery logs but wrong for a sink named "audit" that adopters will wire for compliance.

My take: The AuditSink Protocol and LoggingAuditSink are sound and match the DeliveryLogSink pattern exactly. The SlackAuditSink is where the work is: it needs four design decisions stated in the issue before implementation can start, and they're non-trivial enough that I shouldn't resolve them unilaterally.

Questions for @bokelley:

1. SSRF: Should SlackAuditSink use IpPinnedTransport (consistent with every other outbound call in the SDK)? Or is hooks.slack.com a trusted static domain and we just enforce HTTPS + port 443?
2. details filtering: Should SlackAuditSink take an explicit allowed_fields: frozenset[str] param (default frozenset() — emit nothing from details unless opted in) to prevent financial/PII egress? Or is the intent that adopters are responsible for what they put in details?
3. Integrity contract: Best-effort observability (drop on failure, like DeliveryLogSink) or strict (fail-open to a local fallback log)? The feature name implies compliance; the proposed design implies observability. Needs to be stated so the docstring can be accurate.
4. principal_id naming: Align with ToolContext.caller_identity, or keep principal_id with a comment explaining the difference?

---

Triaged by Claude Code. Session: https://claude.ai/code/${CLAUDE_CODE_REMOTE_SESSION_ID}

---

Generated by Claude Code