Skip to content

feat(signing): SigningProvider abstraction for KMS-backed RFC 9421 signing#1017

Merged
bokelley merged 6 commits into
mainfrom
bokelley/kms-signing
Apr 26, 2026
Merged

feat(signing): SigningProvider abstraction for KMS-backed RFC 9421 signing#1017
bokelley merged 6 commits into
mainfrom
bokelley/kms-signing

Conversation

@bokelley

@bokelley bokelley commented Apr 26, 2026

Copy link
Copy Markdown
Contributor

What this PR does

Adds a pluggable SigningProvider interface so AdCP RFC 9421 private keys can live in a managed key store (GCP KMS, AWS KMS, Azure Key Vault, HashiCorp Vault Transit) instead of process memory. The async sign(payload) boundary matches RFC 9421 §3.1: the SDK produces the canonical signature base, the provider returns wire-format signature bytes.

Closes #1009. Sibling work tracked in #1015 (distributed ReplayStore for multi-instance verifiers).

Wire format unchanged. No AdCP version bump. SDK minor: 5.19.0 → 5.20.0.

What's worth a second pair of eyes

1. Sync and async share the same canonicalization helpers

signRequest and signRequestAsync go through prepareRequestSignature + finalizeRequestSignature in src/lib/signing/signer.ts. Same for webhook. The only path-difference is whether the bytes go to produceSignature (sync, in-process Node crypto) or await provider.sign (async, KMS round-trip). Canonicalization can't drift between the two paths because there's only one canonicalization. The shared helpers are exported from @adcp/client/signing so other entry points can reuse them.

There's also a property test (fast-check, 50 fuzzed requests per run) asserting byte-identical signatureBase, Signature-Input, and Signature between sync and async for Ed25519 — defense in depth in case anyone reintroduces duplicate code paths in the future.

2. Discriminated AgentRequestSigningConfig on kind

type AgentRequestSigningConfig = AgentRequestSigningConfigInline | AgentRequestSigningConfigProvider;

'inline' is the default (kind optional on the existing shape — current literals work without changes). 'provider' is required on the new shape. The narrowing flows through to agent-context.ts and agent-fetch.ts via the isInlineSigningConfig / isProviderSigningConfig predicates so unsafe field access is caught at compile time.

3. Defensive cache-key hashing for provider-supplied fingerprints

buildAgentSigningContext SHA-256s algorithm + '\0' + keyid + '\0' + fingerprint before composing transport- and capability-cache keys. A buggy adapter that returns kid, '', 'default', or a tenant-controlled string for fingerprint still produces 64-bit collision-resistant output. Two distinct keys with identical raw fingerprints don't collide because algorithm and kid are folded in. This preserves the multi-tenant isolation property that the in-memory path has always provided.

4. Identity snapshot at fetch construction

buildAgentSigningFetch wraps the provider in a frozen-identity view (keyid / algorithm / fingerprint snapshotted once) before passing it to createSigningFetchAsync. Wire keyid/alg cannot drift away from the cache key the connection was bound to even if a downstream adapter mutates fields after construction.

5. Non-UTF-8 byte bodies are rejected explicitly

bodyToString in both fetch.ts and fetch-async.ts uses TextDecoder fatal-mode and throws on invalid UTF-8. Previously Buffer.from(body).toString('utf8') silently replaced invalid bytes with U+FFFD; verification still passed because the wire and the digest agreed on the lossy string, but the seller received mangled content. Real footgun for binary payloads — KMS audience hits this faster than the inline-key audience.

6. InMemorySigningProvider NODE_ENV gate, scoped to the reference impl

Constructor refuses to instantiate when NODE_ENV=production (case-insensitive) unless ADCP_ALLOW_IN_MEMORY_SIGNER=1 is set. Lives under @adcp/client/signing/testing so production imports surface the KMS path first. The interface JSDoc explicitly notes that this gate is a self-discipline aid for the bundled implementation only — the SDK can't (and doesn't try to) enforce hygiene on third-party providers.

7. GCP KMS reference adapter as an example, not a bundled dep

examples/gcp-kms-signing-provider.ts — type-checked under npm run typecheck:examples, demonstrates the DER → IEEE P1363 conversion (via the shared derEcdsaToP1363 helper), pre-flight algorithm validation via one getPublicKey call, and the consumer-side KeyManagementServiceClient wiring. The published package stays free of @google-cloud/kms; users npm i the cloud SDK they need.

Migration

Existing callers passing { kid, alg, private_key, agent_url } continue to work unchanged (no kind field needed). New callers pass { kind: 'provider', provider, agent_url }.

In-process cache-key shape changed for inline configs (sha256(alg + kid + sha256(kid + d).slice(16)) vs old sha256(kid + d).slice(16)). Cache is in-process with a 5-minute TTL, no persistence boundary — entries become unreachable on upgrade and re-prime within seconds. Verified call sites (protocols/mcp.ts, protocols/a2a.ts, capability-priming.ts) consume cacheKey opaquely.

Test plan

  • npm run typecheck — clean
  • npm run typecheck:examples — clean
  • npm run format:check — clean
  • npm run build:lib — clean
  • 25 new tests in test/request-signing-provider.test.js
  • Existing 188 signing tests pass — no regressions
  • Property test (fast-check, 50 runs) asserts sync/async equivalence

Deferred follow-ups

  • Distributed ReplayStore adapter (Redis) for multi-instance verifiers — filed as Distributed ReplayStore adapter (Redis) for multi-instance verifier deployments #1015, no dependency on this PR.
  • AWS KMS / Azure Key Vault example adapters — pattern is identical to the GCP example; ship if/when there's demand.
  • @google-cloud/kms's crc32c integrity field on asymmetricSign — defensive, not required by the spec.
  • InMemorySigningProvider.sign re-imports createPrivateKey per call — testing-only path, perf optimization not correctness.

🤖 Generated with Claude Code

…1 signing

Adds a pluggable `SigningProvider` interface so private keys can live in a
managed key store (GCP KMS, AWS KMS, Azure Key Vault, HashiCorp Vault Transit)
instead of process memory. The async `sign(payload)` boundary matches RFC
9421 §3.1 — the SDK produces the canonical signature base, the provider
returns wire-format signature bytes.

`AgentRequestSigningConfig` is now a discriminated union on `kind`:
- `'inline'` (default — `kind` is optional on this shape so existing literals
  continue to work) holds a private JWK in process.
- `'provider'` delegates `sign()` to a `SigningProvider`.

`buildAgentSigningContext` defensively hashes the provider-supplied
`fingerprint` together with `algorithm` and `kid` before composing transport-
and capability-cache keys, preserving the multi-tenant isolation property
the in-memory path has always provided. The signing identity is snapshotted
at context-build time, and `buildAgentSigningFetch` wraps the provider in a
frozen-identity view before passing it to `createSigningFetchAsync` — so the
on-wire `keyid`/`alg` and the cache key cannot drift if a provider mutates
fields after construction.

`InMemorySigningProvider` ships under `@adcp/client/signing/testing` so
production imports surface the KMS path first; the constructor refuses to
instantiate when `NODE_ENV=production` (case-insensitive) unless
`ADCP_ALLOW_IN_MEMORY_SIGNER=1` is set.

Wire format unchanged. No AdCP version bump.

A reference GCP KMS adapter ships at `examples/gcp-kms-signing-provider.ts`,
type-checked under `npm run typecheck:examples`. The shared `derEcdsaToP1363`
helper handles the DER → IEEE P1363 conversion any KMS adapter needs.

23 new tests cover: provider sign→verify round-trip (Ed25519 + ECDSA-P256 +
full `createSigningFetchAsync` HTTP path), wrong-key rejection, signWebhookAsync
parity, cache isolation under same-`kid` distinct fingerprints, defensive
hashing of low-entropy fingerprints, NODE_ENV production gate, algorithm
mismatch error, and DER bounds-check / left-pad edge cases.

Closes #1009. Sibling work tracked in #1015 (distributed `ReplayStore` for
multi-instance verifiers).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Comment thread test/request-signing-provider.test.js Fixed
Comment thread test/request-signing-provider.test.js Fixed
bokelley and others added 3 commits April 26, 2026 08:10
CI's format:check failed on the four files added in the prior commit.
No logic change — pure prettier --write.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Removes `canonicalTargetUri` from the destructured import block and a stray
`const kid` declaration in the defensive-fingerprint test that was shadowed
by the inline `keyid` literals on `provA`/`provB`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses PR #1017 review feedback. Substantive changes:

1. Sync/async share canonicalization. Extracts `prepareRequestSignature`,
   `prepareWebhookSignature`, and `finalizeRequestSignature` into
   `signer.ts`. Both `signRequest`/`signWebhook` (sync) and
   `signRequestAsync`/`signWebhookAsync` (async) now go through the same
   helpers, so canonicalization can no longer drift between the two
   paths. Signer/provider call is the only difference.

2. Reject non-UTF-8 byte bodies. `bodyToString` in both `fetch.ts` and
   `fetch-async.ts` now uses `TextDecoder` fatal-mode and throws a clear
   `TypeError` when a `Uint8Array`/`ArrayBuffer` body isn't valid UTF-8.
   Previously `Buffer.from(body).toString('utf8')` silently replaced
   invalid bytes with U+FFFD — verification still passed because the
   wire and the digest agreed on the lossy string, but the seller
   received mangled content. Real footgun for KMS-audience binary
   payloads.

Smaller items:

3. `AgentRequestSigningConfigProvider.provider` now uses a top-level
   `import type` from `../signing/provider` instead of an inline
   `import()`, so consumer-facing `.d.ts` resolves cleanly.

4. `derEcdsaToP1363` now parses and validates the SEQUENCE total
   content length against buffer end, in addition to the per-INTEGER
   bounds checks.

5. `SigningProvider` JSDoc explicitly notes the NODE_ENV gate scope:
   `InMemorySigningProvider`'s production guard is a self-discipline
   aid for the bundled reference implementation; the SDK can't enforce
   hygiene on third-party providers.

6. GCP example usage block now shows the consumer-side
   `KeyManagementServiceClient` construction and passes it via
   the `client` parameter.

Tests:

- New property test (fast-check, 50 runs/property): asserts
  `signRequest` and `signRequestAsync` produce identical
  `signatureBase`, `Signature-Input`, and `Signature` headers across
  fuzzed requests for Ed25519. Locks the parallel structure beyond
  what the fixed-input round-trip can catch.
- New test asserts `createSigningFetchAsync` rejects non-UTF-8
  `Uint8Array` bodies with a clear `TypeError`.
- Existing 188 signing tests still pass; new total 25 provider tests.

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

Copy link
Copy Markdown
Contributor Author

Pushed c61b1a80 addressing the substantive items.

Two big ones, fixed properly:

  1. Sync/async share canonicalization. Took the refactor route over the test-only route. prepareRequestSignature / prepareWebhookSignature / finalizeRequestSignature live in signer.ts; both sync and async paths consume them. The only difference between paths is now produceSignature vs await provider.sign. Added a fast-check property test (50 fuzzed requests per run) that asserts byte-identical Ed25519 sync/async output as belt-and-suspenders — if anyone reintroduces duplicate canonicalization in the future, the property fails.

  2. Non-UTF-8 byte bodies — fixed in both wrappers, not deferred. bodyToString in fetch.ts and fetch-async.ts now uses TextDecoder fatal-mode and throws a clear TypeError on invalid UTF-8. Previous lossy Buffer.toString('utf8') behavior is gone from both paths. Added a regression test for the rejection path.

Smaller items:

  • AgentRequestSigningConfigProvider.provider is now a top-level import type rather than inline import(). Verified there's no cycle — signing/provider.ts imports AdcpSignAlg from signing/types.ts, and signing/types.ts doesn't import from types/adcp.ts.
  • derEcdsaToP1363 now parses the SEQUENCE total content length and validates against buffer end (in addition to per-INTEGER bounds checks already there).
  • SigningProvider JSDoc explicitly notes the NODE_ENV gate scope: it's a self-discipline aid for the bundled InMemorySigningProvider only; the SDK can't enforce hygiene on third-party providers.
  • GCP example usage block now shows the consumer-side KeyManagementServiceClient construction and explicit client parameter.

Publish state confirmed: npm view @adcp/client@5.19.0 shows the currently-published version (without SigningProvider). The changeset is minor, so this PR ships 5.20.0 on merge. The package.json 5.19.0 you're seeing locally is just the most recent release — no skew with what's on npm.

PR description rewritten to lead with the seven substantive items rather than the review-cleared framing. Reviewers can now see what they're actually being asked to look at.

Test totals: 25 provider tests (was 23) + 188 existing signing tests, all green.

- Changeset now calls out the behavior change in `createSigningFetch` /
  `createSigningFetchAsync` for non-UTF-8 byte bodies (was silent U+FFFD
  substitution; now throws TypeError). Added per PR review — anyone whose
  CI starts failing has a finder.
- Property test now runs against ECDSA-P256 in addition to Ed25519. ECDSA
  is non-deterministic so signature bytes legitimately differ, but the
  canonicalization invariant (signatureBase + Signature-Input) must hold
  across sync/async paths for both algorithms.

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

Copy link
Copy Markdown
Contributor Author

Pushed 38a35271:

  • Changeset note added. New paragraph calls out the behavior change for non-UTF-8 byte bodies (was silent U+FFFD; now TypeError), names the escape hatch, gives anyone whose CI starts failing a finder.
  • ECDSA property test added. Same fast-check arbitrary, asserts identical signatureBase + Signature-Input across sync/async (50 runs). Skips comparing Signature bytes since ECDSA is non-deterministic. The canonicalization invariant now holds for both algs.

Skipping the DER trailing-bytes nit and the SigningProviderErrorCode single-member-union per your "won't push / no objection." Will revisit the union if/when a second code joins it.

Thanks for the careful read — the prepare/finalize helpers being reusable for future signers (sigstore, custom HSM) is a good framing; I hadn't named that benefit explicitly but it's exactly why I picked the refactor over the test-only path.

Ready when you are.

- SIGNING-GUIDE.md: new "Step 3.5: Production Key Storage — KMS / HSM /
  Vault" section. Covers the SigningProvider interface, kind: 'provider'
  config, the GCP KMS reference adapter, GCP setup walkthrough (key,
  IAM, JWKS publication, rotation), authentication when the runtime
  isn't on GCP (SA JSON path, WIF, Cloud Run sidecar), and the
  threat-model upgrade table. Documents InMemorySigningProvider's
  NODE_ENV gate.
- migration-4.x-to-5.x.md: new Part 6 covering the 5.20.0 changes —
  SigningProvider abstraction (additive) and the strict UTF-8 body
  decode (small breaking).
- BUILD-AN-AGENT.md: cross-link to the new Production Key Storage
  section, plus a note that server-side webhook signing currently
  accepts only an in-process SignerKey (KMS-backed webhook signing is
  a follow-up).
- llms.txt / TYPE-SUMMARY.md: regenerated via npm run generate-agent-docs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@bokelley bokelley merged commit c4afc75 into main Apr 26, 2026
10 checks passed
bokelley added a commit that referenced this pull request Apr 26, 2026
#1019)

* feat(testing): adcp grade signer — validate a signer end-to-end before going live

Pairs with #1017's SigningProvider abstraction. After standing up a KMS
adapter, operators need a way to validate "is this thing producing valid
AdCP signatures?" before pushing live signed traffic — otherwise the only
feedback channel is the seller's `request_signature_invalid` rejection,
which is generic and arrives only after the misconfiguration is in
production.

The grader produces a sample signed AdCP request through the operator's
signer, then verifies it against the operator's published JWKS via the
SDK's RFC 9421 verifier. Pass means a counterparty will accept your
signatures; fail surfaces a specific verifier error_code + step, so
DER-vs-P1363 / kid-mismatch / wrong-key / algorithm-mismatch each show
up as distinct diagnostics.

Two signer-source modes:

- `--key-file <path>` for local dev (JWK with `d`)
- `--signer-url <url>` for KMS-backed signers — minimal HTTP oracle
  protocol (POST {payload_b64, kid, alg} → {signature_b64}) so any KMS
  adapter can put a small handler in front of provider.sign() without
  exposing the underlying KMS to the grader

Programmatic API: `gradeSigner(options)` exported from
`@adcp/client/testing/storyboard/signer-grader`. Returns
`SignerGradeReport` with passed/step/diagnostic + the sample request the
signer produced headers for.

CLI:
  adcp grade signer <agent-url>
    --signer-url ... | --key-file ...
    --kid <kid> --alg <ed25519|ecdsa-p256-sha256>
    --jwks-url <url>
    [--operation <name>] [--signer-auth <header>] [--timeout <ms>]
    [--allow-http] [--json]

9 unit/integration tests cover both signer-source modes, the verifier
failure paths a misconfigured adapter would hit (algorithm mismatch,
wrong-key signing, signer-url 401, JWKS unreachable), and the input
validation (both modes / missing modes / missing key file).

Local CLI smoke test:
  $ adcp grade signer http://127.0.0.1:9999/agent \\
      --key-file ./key.jwk --kid test-ed25519-2026 \\
      --alg ed25519 --jwks-url http://127.0.0.1:N/.well-known/jwks.json \\
      --allow-http
  Result: PASS
    diagnostic: Signature verified end-to-end against JWKS.

Closes #610. Accumulates into Release PR #1014 (still 5.20.0 minor).

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

* fix(testing): tighten signer grader per expert review (SSRF, digest, errors)

Three reviewers (security, code, protocol). Security flagged two HIGH
blockers — both collapse into routing the signing-oracle POST through
ssrfSafeFetch. Protocol flagged two BLOCKERS around content-digest
policy (the most common KMS-adapter misconfiguration was being masked).
Plus the should-fix items from code review.

Security blockers (HIGH):

- httpOracleProvider used plain fetch(), bypassing the SDK's SSRF guard.
  Concrete attack: an internal attacker sets --signer-url to IMDS or
  redirects the POST to capture the --signer-auth Bearer token. Now
  routes through ssrfSafeFetch — gets DNS pinning, IMDS-block,
  manual-redirect (no Location-following), and a 64 KiB body cap.
- --signer-auth header could leak via 302 redirect. Same fix —
  ssrfSafeFetch refuses to follow redirects.
- No response-body size cap. Same fix — ssrfSafeFetch caps at 64 KiB.

Protocol blockers:

- Hardcoded covers_content_digest: 'either' masked the digest-policy
  misconfiguration KMS adapters most commonly hit (forgetting to emit
  Content-Digest against a 'required' verifier). Now defaults to
  'required' (recommended posture for spend-committing operations) and
  exposes --covers-content-digest [required|forbidden|either] so
  operators match what their verifier actually advertises.
- Sample body now reliably triggers step 11 (digest recompute) when
  policy is 'required' — the signer emits Content-Digest, the verifier
  exercises the digest path. Test asserts both branches: the header
  is present under 'required', omitted under 'forbidden'.

Code-review fixes:

- Dead branch for verifier_returned_unsigned (unreachable —
  signRequestAsync always emits headers) replaced with a hard throw so
  an SDK bug surfaces loudly rather than masquerading as a custom code.
- error_code typed as a discriminated union (SignerGradeErrorCode =
  RequestSignatureErrorCode | SignerGraderErrorCode) so consumers
  writing exhaustive switch over the report see grader-side and
  verifier-side failures as distinct categories.
- Case-insensitive header lookup in the CLI report printer (a future
  signer that emits lowercase header names won't silently elide them).
- Lib + CLI error messages now agree on flag names (--key-file /
  --signer-url) so --json reports use names operators recognize.

Protocol note:

- Layered DER-vs-P1363 hint on top of step-10 errors when algorithm is
  ECDSA-P256. The most likely cause of a generic
  request_signature_invalid in that case is a KMS adapter that didn't
  run output through derEcdsaToP1363; surfacing this inline saves
  operators a debugging cycle.

Security MED:

- Stderr breadcrumb when --key-file is loaded so CI logs make the
  dev-tool nature visible — operators reviewing the log can confirm
  the file isn't checked in.

Test totals: 11/11 (was 9). New tests: required-policy emits
Content-Digest; forbidden-policy omits it.

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

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

Add SigningProvider abstraction for external key management (KMS/HSM/Vault)

1 participant