Skip to content

feat(signing): signerProvider option on createWebhookEmitter for KMS-backed webhook signing#1021

Merged
bokelley merged 4 commits into
mainfrom
claude/issue-1020-webhook-emitter-signing-provider
Apr 26, 2026
Merged

feat(signing): signerProvider option on createWebhookEmitter for KMS-backed webhook signing#1021
bokelley merged 4 commits into
mainfrom
claude/issue-1020-webhook-emitter-signing-provider

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Closes #1020

Summary

Adopters who moved request signing to a managed key store (GCP KMS, AWS KMS, Azure Key Vault) via the 5.20.0 SigningProvider path still held an in-process JWK for webhook delivery, defeating the KMS threat model. This PR closes the gap by adding signerProvider?: SigningProvider to WebhookEmitterOptions (and createAdcpServer's webhooks config). The emitter routes through signWebhookAsync when a provider is set and signWebhook when signerKey is set — exactly one must be provided, enforced at construction time. All delivery semantics (retries, idempotency-key stability per adcp#2417, content-digest, compact-separator serialization per adcp#2478, redirect policy) are identical between the two paths.

Migration note: signerKey changes from required to optional at the TypeScript type level. Existing callers that pass signerKey are unaffected. Callers who forward WebhookEmitterOptions and rely on signerKey being required in their own type signatures should update those types.

JWKS note: The JWK published at jwks_uri for a key wrapped by a signerProvider MUST carry adcp_use: "webhook-signing" — receivers enforce key purpose at this field (adcp#2423).

adcp_use enforcement: The SDK cannot validate adcp_use on a SigningProvider (the interface exposes keyid/algorithm/fingerprint, not JWKS metadata). This is a publisher JWKS-setup concern; the JSDoc on signerProvider in WebhookEmitterOptions makes it explicit.

Changes

  • src/lib/server/webhook-emitter.tssignerKey optional; signerProvider?: SigningProvider added; construction-time mutual-exclusion validation; buildHeaders becomes async; RFC 9421 path dispatches to signWebhookAsync for the provider path.
  • src/lib/server/create-adcp-server.tssignerProvider added to the webhooks Pick; JSDoc updated to document the either/or contract.
  • src/lib/server/index.tsSigningProvider type re-exported so consumers can type provider variables without reaching into the signing sub-barrel.
  • test/lib/webhook-emitter.test.js — 5 new tests: verifier round-trip, delivery/compact-body, retry key-stability (provider path); reject-both, reject-neither construction guards.
  • .changeset/webhook-emitter-signer-provider.md — minor bump (5.20.0 → 5.21.0).

What tested

  • npm run format:check — clean
  • npx tsc --project tsconfig.lib.json — clean (0 errors)
  • node --test test/lib/webhook-emitter.test.js — 19 tests, 0 failures (14 pre-existing + 5 new)
  • Pre-existing unrelated suite failures on main: 315 (unchanged on this branch)

Pre-PR review

  • code-reviewer: approved after two blockers fixed — createAdcpServer webhooks Pick now includes signerProvider; SigningProvider re-exported from server barrel; retry test idempotency assertion added.
  • ad-tech-protocol-expert: approved — async path is protocol-correct; created/expires stamped before provider.sign() so KMS latency cannot widen the signature window; no RFC 9421 or AdCP webhook-signing profile conformance concern.

Triage-managed PR. This bot does not currently iterate on
review comments or PR conversation threads (only on the source
issue). To unblock:

  • Push fixup commits directly: gh pr checkout <num>
    fix → push.
  • Or re-trigger: comment /triage execute on the source
    issue.

See adcp#3121
for context.

Session: https://claude.ai/code/session_01EsiKFg3cUHDxgwDpfXJLRJ


Generated by Claude Code

claude and others added 3 commits April 26, 2026 16:22
…KMS-backed webhook signing

Closes #1020. Adopters who moved to the 5.20.0 SigningProvider path for
request signing still held an in-process JWK for webhook delivery; this
closes that gap by letting createWebhookEmitter accept a signerProvider
that routes through signWebhookAsync instead.

All emitter semantics (retries, idempotency-key stability, content-digest,
redirect policy) are identical between the two paths.

https://claude.ai/code/session_01EsiKFg3cUHDxgwDpfXJLRJ
…export SigningProvider

Pre-PR review caught two blockers:
- createAdcpServer webhooks Pick omitted signerProvider, blocking KMS path via the server factory
- SigningProvider type not re-exported from server barrel, requiring internal import path

Also adds final_status assertion to provider retry test per review nit.

https://claude.ai/code/session_01EsiKFg3cUHDxgwDpfXJLRJ
CI's format:check failed on the test file added in #1021. No logic
change — pure prettier --write.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley force-pushed the claude/issue-1020-webhook-emitter-signing-provider branch from 31fa2a8 to c61d055 Compare April 26, 2026 20:40
…test

Three reviewers (security, code, protocol). Security: clean. Code:
no blockers. Protocol: sound-with-caveats. Addressing the convergent
items:

- **Single-purpose key warning on `signerProvider`** (protocol caveat +
  code-review nit). JSDoc now explicitly tells operators NOT to wire
  the same `SigningProvider` instance to both `request_signing.provider`
  and `webhooks.signerProvider` — per `SIGNING-GUIDE.md:38-43`, AdCP
  requires distinct key material per purpose. Mint a second
  `cryptoKeyVersion` for webhook signing.

- **`WebhookEmitResult.errors` leak warning** (security LOW). When a
  KMS adapter rejection bubbles through the retry loop, the message
  text comes from the adapter and may include infra detail (KMS
  resource names, IAM principals, project IDs). New JSDoc mirrors the
  caution flagged on `SigningProvider.fingerprint`.

- **Wire-byte equality test** (code-review issue). The PR claims "only
  the dispatch differs between sync and provider paths" — now
  asserted: same body + same nonce + same `created` produces identical
  `signatureBase`, identical `Signature-Input`, identical Ed25519
  Signature bytes (deterministic), identical `Content-Digest`. Locks
  the contract beyond prose.

Test totals: 20/20 (was 19).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley

Copy link
Copy Markdown
Contributor Author

Three-reviewer pass (security, code, protocol). No blockers. Convergent doc + test items addressed in 90c6222a.

Reviewer Verdict Items addressed
Security clean — ship-ready Two LOW items both fixed (provider error leak warning, JSDoc rot)
Code no blockers One issue (wire-byte equality untested) — fixed; one nit (HMAC override × signerProvider untested) — skipped, behavior is correct, low value
Protocol sound-with-caveats Two doc-level gaps (single-purpose key reuse warning, adcp_use runtime enforcement clarity) — both fixed

Specific fixes:

  • Single-purpose key warning on signerProvider JSDoc explicitly tells operators NOT to wire the same SigningProvider instance to both request_signing.provider and webhooks.signerProvider. Per SIGNING-GUIDE.md:38-43, AdCP requires distinct key material per purpose; mint a second cryptoKeyVersion. Also clarifies that the SDK can't enforce adcp_use at runtime (the interface exposes only keyid/algorithm/fingerprint) — purpose enforcement happens at JWKS publication time, receiver-side.

  • WebhookEmitResult.errors leak warning. When a KMS rejection bubbles through retry, the adapter's err.message lands in errors[] and may include infra detail (resource names, IAM principals, project IDs). JSDoc mirrors the same caution we already had on SigningProvider.fingerprint so operators redact at their logging boundary.

  • Wire-byte equality test added. Locks the "only dispatch differs" claim: same body + same nonce + same created → identical signatureBase, identical Signature-Input, identical Ed25519 Signature bytes (deterministic), identical Content-Digest.

Skipped per reviewer's own framing:

  • HMAC override × signerProvider test (behavior is correct — HMAC path returns early; cheap to test but low signal)
  • attempts: errors.length pre-existing bug in failure-return shape (out of scope; not introduced by this PR)

Test totals: 20/20 (was 19). Rebased onto current main (post-#1023). All 11 CI checks expected green.

@bokelley bokelley marked this pull request as ready for review April 26, 2026 21:13
@bokelley bokelley merged commit ecf015e into main Apr 26, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(signing): signerProvider option on createWebhookEmitter for KMS-backed webhook signing

2 participants