Repository navigation
feat(webhooks): sign deliveries under web-bot-auth when a registration asks - #1860
Conversation
…n asks AdCP 3.3 lets a webhook registration omit authentication and name a signing_profile. WebhookSender could not produce web-bot-auth, so a seller built on the SDK, including its task-webhook outbox, could not honor a buyer registered for it. Sender: - WebhookSender.from_wba(private_key, agent_origin=...) signs each attempt afresh with sign_wba_request (new 64-byte nonce and created, thumbprint keyid, Signature-Agent naming the origin). - with_web_bot_auth() adds web-bot-auth to an adcp/webhook-signing/v1 sender; each delivery is signed under its registration's profile, and resend keeps the original's profile. - signing_profiles / signing_algorithms; Signature-Agent is reserved from extra_headers. - sign_web_bot_auth_directory() signs the origin's key directory. - challenge_webhook_destination signs under the config's signing_profile. Outboxes: - PgTaskWebhookOutbox stores push_notification_config.signing_profile with the encrypted registration and delivery and signs every attempt under it. - PgNotificationOutbox persists required_signing_profile (envelope v2 only for non-default profiles, so earlier pods quarantine instead of mis-signing). - The reporting notification worker signs web-bot-auth subscriptions with ReportingWebBotAuthKey; ReportingProductionSigning(web_bot_auth=True) advertises both profiles. Unset profiles keep fingerprints byte-stable. Validation: - Dispatch refuses, with INVALID_REQUEST before the task is issued, a push registration requiring a profile outside the request's effective webhook_signing.profiles (tenant-scoped overrides included), for every publisher. - Boot validation requires a key for every advertised profile and every sender algorithm to be advertised. Also annotates the pre-existing bandit B608/B311 findings in task_webhook_outbox.py (validated table identifier; retry jitter), which the pre-commit hook reports for any change to that file. Closes #1859 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — additive Web Bot Auth webhook signing feature (AdCP 3.3, #1859).
What I checked:
- Semver signal:
feat(webhooks):is correct. New optionalsigning_profile, newweb-bot-authenum value, and new public exports (WEBHOOK_SIGNING_PROFILES,ReportingWebBotAuthKey,from_wba,with_web_bot_auth) are all additive — no removed/narrowed public surface, so no major-bump requirement is triggered. - Wire-shape forward-compat: task-outbox stores the ADCP default as a bare body (byte-identical to pre-profile rows); non-default profiles go in an encrypted/v2 envelope that an older pod quarantines rather than mis-signs. The
_open_delivery_bodyXOR and notification_openversion/profile checks round-trip old envelopes unchanged; reporting subscription fingerprints preserved whensigning_profileis unset. - Rolling-deploy gap (task registered on new pod, completed on old) is documented and mitigated by not advertising the profile until rollout completes.
- Test coverage is thorough, including end-to-end
verify_wba_requestagainst the published directory.
Decision trace: No critical/high findings (row 1 n/a). gated_paths false (row 2 n/a). high_risk is true but all reasons are (modified) with only ONE medium finding — row 5 requires a medium on a modified high-risk file; the single medium is in notifications.py (reporting/production, not a high-risk path), so row 5 does not fire. The medium is not data-loss/schema/infra (row 4 n/a). No deletions (row 3 n/a). No prior decision (row 6 n/a). No team gate (row 7 n/a). Only 1 medium finding, fewer than 3 (row 8 n/a). Falls through to row 9 → approve.
Medium findings (non-blocking):
- src/adcp/reporting/production/notifications.py:133 — web-bot-auth reporting subscription skipped with warn-only, no alerting path.
Medium findings
- src/adcp/reporting/production/notifications.py:133 — web-bot-auth reporting subscription skipped with warn-only, no alerting path
…the outboxes The outboxes read the new WebhookSender.signing_algorithms (and the task outbox signing_profiles) directly. A sender-like object without them - the Postgres conformance tests' MagicMock senders, and adopters' own test doubles - failed the check and its rows were quarantined. Both outboxes and the boot validator now share _sender_signing_algorithms and _sender_signing_profiles, which fall back to _auth.alg and, for an RFC 9421 sender, adcp/webhook-signing/v1, as the boot check already did. The Postgres conformance mock senders also declare both properties. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean, behavior-preserving refactor with no blocking findings.
This delta moves _sender_algorithms verbatim from webhook_emit.py into webhook_sender.py as _sender_signing_algorithms, adds _sender_signing_profiles, and routes call sites in task_webhook_outbox.py, notification_outbox_pg.py, and webhook_emit.py through these duck-typed helpers. Legacy/supervisor senders now fall back fail-closed instead of raising AttributeError. The local import in webhook_emit.py is required by the existing TYPE_CHECKING-only WebhookSender import (circular dep). New test covers legacy-double, unsigned, and dual real-sender paths.
Decision trace:
- Row 1 (critical/high): no findings — skip.
- Row 2 (gated_paths): false — skip.
- Row 3 (deleted high-risk): all high-risk files are (modified), none deleted — skip.
- Row 4 (medium data-loss/schema/infra): no findings — skip.
- Row 5 (modified high-risk + medium finding): no medium findings in this run — skip.
- Row 6 (sticky escalate): prior decision was approve, not escalate — skip.
- Row 7 (no-auto-approve team): no team match — skip.
- Row 8 (3+ medium): zero findings — skip.
- Row 9: approve.
The prior-run medium (notifications.py:133, warn-only web-bot-auth skip) is outside this delta and did not recur. high_risk is true only via (modified) decisioning files, which the reviewer cleared with no medium-or-worse concerns — presumed safe per the high-risk heuristic guidance. review_decision is APPROVED.
A web-bot-auth subscription that production signing cannot sign is skipped so it does not block the account's other subscribers. That subscriber then receives no notifications, so log the skip at error level (the level the outboxes use for a quarantined delivery) with the subscriber and account ids, instead of a warning that can be lost under volume. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean observability fix, no blocking findings.
This is a subsequent review on top of a prior approve. The single new commit (db7756b) upgrades the skipped web-bot-auth subscription log from warn to error and adds subscriber_id and account_id to the message (notifications.py:37-43). Both fields are valid, non-credential references on ReportingNotificationSubscription (routing.py:60-61), the test asserts the error level and both ids (test_reporting_webhook_wba.py:247-251), and the doc matches. A quarantined subscriber is now alertable — a net improvement.
Decision-table walk:
- No critical/high findings → row 1 does not fire.
- gated_paths is false → row 2 N/A (and review_decision is APPROVED anyway).
- high_risk is true but all high-risk reasons are
(modified)with zero medium-or-higher findings on those files — no(deleted)(row 3 N/A), no medium on a modified high-risk file (row 5 N/A). - No medium findings at all → rows 4, 8 N/A.
- Prior decision was
approve, notescalate→ row 6 N/A. - No no-auto-approve team match → row 7 N/A.
- Falls through to row 9 → approve.
No credentials surfaced in logs, no public-surface/semver signal change, no generated-code hand-edits, no CI-gate tampering.
There was a problem hiding this comment.
Ladon verdict: Approve
Approve — clean pass, no blocking or medium findings.
Checked against the decision table top-down:
- Rows 1 (critical/high): no findings.
- Row 2 (gated_paths): gated_paths is false; review_decision is APPROVED regardless.
- Row 3 (deleted high-risk path): all high-risk reasons are (modified), none (deleted).
- Rows 4–5 (medium + schema/infra or modified high-risk): no medium findings.
- Row 6 (sticky escalate): prior decision was approve, not escalate.
- Row 7 (no-auto-approve team): no team match.
- Row 8 (≥3 medium): zero medium findings.
- Row 9: approve.
The PR threads web-bot-auth webhook signing through WebhookSender (additive public surface, correct feat prefix — no breaking semver signal). Reviewer verified the per-attempt resign path has no transport leak, both PG outbox envelopes round-trip byte-identically for pre-feature rows (rolling-deploy quarantine holds), retries re-sign afresh rather than replaying stored signatures, and timeouts + allowed_destination_ports are threaded to the worker. high_risk is true only because decisioning/** and webhook_auth.py were modified, but the reviewer found no medium-or-higher concerns on those files, so the modifications are presumed safe. Consistent with the prior clean approve on an earlier head.
No blocking findings. No escalation triggers.
Closes #1859.
At AdCP 3.3 a webhook registration can omit
authenticationand name asigning_profile.WebhookSendercould not produceweb-bot-auth, so a seller built on the SDK, including its task-webhook outbox, could not honor a buyer registered for it. This PR adds the sender, carries each registration's profile through every outbox to every attempt, and refuses profiles the seller does not advertise.Sender
WebhookSender.from_wba(private_key, *, agent_origin)signs every attempt afresh withsign_wba_request: a new 64-byte nonce andcreated, the key's thumbprint askeyid, andSignature-Agentnaming the origin. Ed25519 only.sender.with_web_bot_auth(key, agent_origin=...)addsweb-bot-authto anadcp/webhook-signing/v1sender. Each delivery is signed under the profile its registration selected, andresendkeeps the original's profile.signing_profiles/signing_algorithmsproperties.sender.sign_web_bot_auth_directory(other_keys=[...])signs the origin's key directory. The directory belongs to the origin, so it also lists request-signing and rotating keys.challenge_webhook_destination(..., signing_profile=)signs proof-of-control under the config's profile.Signature-Agentcan no longer be set throughextra_headers, for either profile.Outboxes
PgTaskWebhookOutboxcapturespush_notification_config.signing_profileat issue, stores it in the encrypted registration and delivery row, and signs every attempt under it. Rows with no stored profile requireadcp/webhook-signing/v1. A registration naming the default is stored exactly like one naming none.PgNotificationOutboxpersistsrequired_signing_profile, which was previously dropped. A non-default profile uses envelope version 2.ReportingNotificationSubscription.signing_profile. When unset it's left out of the fingerprint, so existing subscriptions and queued deliveries are byte-identical.ReportingSigningMaterial(..., web_bot_auth=ReportingWebBotAuthKey(key, origin))supplies the key, and the worker builds afrom_wbasender for each attempt.ReportingProductionSigning(..., web_bot_auth=True)opts production reporting in and advertises both profiles.web-bot-authsubscriber is skipped with a warning rather than blocking the account's other subscribers.Validation
INVALID_REQUEST(fieldpush_notification_config.signing_profile) a push registration requiring a profile outside the request's effectivewebhook_signing.profiles. That set comes from the capabilities resolved for the request, including a tenant'sget_adcp_capabilities_for_requestoverride. The check applies to every publisher: fixed-sender,sender_resolverand externally managed outboxes. A fixed sender is also checked for a key under the profile._handoff_webhook_kwargs→_invoke_platform_method→_project_invocation_result→_project_handoff/_project_workflow_handoff, as an optionalwebhook_signing_profileskeyword.profilesmeansadcp/webhook-signing/v1) and every sender algorithm to be advertised. Both outboxes' resolver checks require every algorithm a resolved sender signs with.Known gap: rolling deploys
Delivery rows stored with
web-bot-authare quarantined by pods running the previous SDK rather than signed under the wrong profile. That holds for the task outbox, the notification outbox (envelope v2) and the reporting worker.One case is not covered: a task registered on an upgraded pod and completed on a pod still on the previous SDK is signed under
adcp/webhook-signing/v1. The old pod ignores the registration's profile. I found no stored format that makes an old pod refuse without also breaking its task completion.docs/webhooks/web-bot-auth.mdsays to finish the rollout before listingweb-bot-authinwebhook_signing.profiles.Tests
tests/test_webhook_sender_wba.pyandtests/test_reporting_webhook_wba.py.verify_wba_requestagainst the published directory, sharing one replay store (an attempt answered 503 followed by its retry).tests/test_decisioning_webhook_emit.py: a tenant-scoped capabilities override admits a profile the static capabilities lack, and other tenants are refused with no task issued.tests/test_webhook_challenge.py: the challenge is signed under the config's profile.tests/test_task_webhook_outbox_pg.pyfor the new registration field.make lintandmake typecheck-allpass. Full suite, last run: 21,774 passed, 1 failed.Timing-sensitive failures in parallel runs. Across three full runs under
pytest -n auto, a different test failed each time:test_branch_parity_absorbs_registry_variance(timing budget)test_real_mcp_dispatch_reports_the_exact_pinned_wire_key(event timing)Both pass when run alone. The second also fails on unmodified
nextwhen its file runs in parallel. Neither is in code this PR touches.Also in this PR
task_webhook_outbox.pygets# nosec B608/# nosec B311annotations on existing lines. The pre-commit bandit hook reports them for any change to that file, and the file fails it identically onnext. The SQL interpolates a table name validated against_SAFE_IDENTIFIER_RE, andrandomis only used for retry jitter.Docs: new
docs/webhooks/web-bot-auth.md; updatedreporting-production.md,reporting-operation-id-3.2.mdand the 3.3 inventory.🤖 Generated with Claude Code