Skip to content

feat: allow externally managed webhook signing capabilities#876

Merged
bokelley merged 1 commit into
mainfrom
python-sdk-version-negotiation
May 26, 2026
Merged

feat: allow externally managed webhook signing capabilities#876
bokelley merged 1 commit into
mainfrom
python-sdk-version-negotiation

Conversation

@bokelley
Copy link
Copy Markdown
Contributor

Summary

  • add DecisioningCapabilities.webhook_signing_managed_externally for adopters that sign webhooks outside the SDK sender stack
  • keep SDK sender/supervisor validation active whenever one is wired
  • fail closed on non-bool flag values and document the request-scoped capabilities usage

Fixes #875

Validation

  • Expert review: protocol reviewer found no issues
  • Expert review: code reviewer found a truthiness bypass; fixed and re-reviewed clean
  • PYTHONPATH=src python3 -m pytest tests/test_tools_list_output_schema.py tests/test_schema_validation_server.py tests/test_dispatcher_version_routing.py tests/test_decisioning_capabilities_submodule.py tests/test_decisioning_capabilities_projection.py tests/test_webhook_signing_capabilities.py -q
  • PYTHONPATH=src python3 -m pytest tests/test_decisioning_capabilities_projection.py tests/test_webhook_signing_capabilities.py -q
  • PYTHONPATH=src mypy src/adcp/decisioning/platform.py src/adcp/decisioning/webhook_emit.py
  • ruff check src/adcp/decisioning/platform.py src/adcp/decisioning/webhook_emit.py tests/test_webhook_signing_capabilities.py tests/test_decisioning_capabilities_projection.py
  • pre-commit hooks on commit: black, ruff, mypy, bandit, whitespace/end-of-file, yaml/json checks, large files, merge/case conflicts, private key detection

@bokelley bokelley changed the title Allow externally managed webhook signing capabilities feat: allow externally managed webhook signing capabilities May 26, 2026
Copy link
Copy Markdown

@aao-ipr-bot aao-ipr-bot Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Right shape: the flag is SDK-internal, never projects to the wire, and the validator's short-circuit is correctly narrow (sender is None AND supervisor is None) so wiring an SDK sender still triggers full RFC 9421 introspection.

Things I checked

  • webhook_signing_managed_externally lives at the top level of DecisioningCapabilities (platform.py:181), not inside the WebhookSigning Pydantic model. The projection at handler.py:1697-1700 only dumps caps.webhook_signing.model_dump(...) — flag has zero wire reachability. Pinned by the new projection test at test_decisioning_capabilities_projection.py:360-365.
  • Bypass safety: isinstance(adopter_managed, bool) at webhook_emit.py:447 fail-closes on truthy strings, ints, numpy.bool_, etc. The subsequent adopter_managed is True at L462 is defense-in-depth — identity, not equality, so 1 == True doesn't sneak through.
  • Wired-sender precedence: short-circuit at L462 requires sender is None and supervisor is None. A bearer-token sender wired alongside webhook_signing_managed_externally=True still trips the non-JWK gate at L511 — pinned by test_adopter_managed_flag_does_not_bypass_wired_sender_validation.
  • Request-scoped path: handler.py:1624 re-runs the validator per-request when get_adcp_capabilities_for_request returns scoped caps, so per-tenant flag flipping works as documented. _validate_scoped_capabilities_static_contract constrains only specialisms and supported_protocols, leaving the new flag free to vary per-tenant (intended).
  • Conventional commit feat: is correct. New field is keyword-defaulted (bool = False); no public-API break, no ! required.
  • RequestContext.metadata / _build_request_context / credential-shaped key suffix list untouched. No credential-leak surface change.
  • Spec compliance: AdCP webhook_signing.supported=true binds the seller to RFC 9421, not the SDK specifically. Moving the contract from SDK-validated to operator-asserted is spec-legal.

Follow-ups (non-blocking — file as issues)

  • INFO vs WARNING on the bypass log. webhook_emit.py:463 emits at INFO, but every sibling skip-path in this file uses WARNING (L170, L253, L279, L295, L321, L480) — including the structurally identical "we skipped validation, you own the contract" branch at L480. A fat-fingered webhook_signing_managed_externally=True on a broken external signer should be the loudest line in boot logs, not the quietest. (security-reviewer-deep: Low.)
  • Validator docstring drift. webhook_emit.py:399-437 still describes unconditional fail-fast; the new bypass lives only in the body. Add a sentence naming the webhook_signing_managed_externally carve-out so a reader auditing the contract via the docstring sees the operator-owned escape hatch. (ad-tech-protocol-expert-deep.)
  • Test gap on the supervisor path. New tests cover sender=None+supervisor=None (passes) and sender=bearer+supervisor=None (still fails). Missing: supervisor=InMemoryWebhookDeliverySupervisor(bearer_sender) + sender=None + adopter_managed=True — should still fail, since the SDK is doing the signing. The behavior is correct (the supervisor path at L471-L489 doesn't re-consult the flag); the test is the gap.
  • Optional: scoped-caps gate symmetry. _validate_scoped_capabilities_static_contract (handler.py:976) blocks scoped caps from changing specialisms / supported_protocols. Consider extending to reject scoped caps enabling webhook_signing.supported=True when static caps had it disabled — would close the per-tenant trust-surface expansion. Not a security hole today (failure is loud on the buyer side, contained to the asserting tenant), but symmetric with the existing guard.

Approving on the strength of the fail-closed non-bool guard plus the narrow short-circuit condition.

@bokelley bokelley merged commit 3c312a0 into main May 26, 2026
39 of 40 checks passed
@bokelley bokelley deleted the python-sdk-version-negotiation branch May 26, 2026 15:21
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: allow adopter-owned webhook_signing capability without SDK WebhookSender

1 participant