Skip to content

feat(signing): default replay store + NewSignedHTTPClient preset#88

Merged
bokelley merged 2 commits into
mainfrom
bokelley/signed-http-client-and-defaults
Apr 25, 2026
Merged

feat(signing): default replay store + NewSignedHTTPClient preset#88
bokelley merged 2 commits into
mainfrom
bokelley/signed-http-client-and-defaults

Conversation

@bokelley

Copy link
Copy Markdown
Contributor

Summary

Two ergonomic upgrades to bring `adcp-go` into parity with the cross-SDK signing ergonomics work:

  • TypeScript (adcp-client#917, merged): defaults on `verifySignatureAsAuthenticator` + `createExpressVerifier`, default fetch on `buildAgentSigningFetch`, new `createAgentSignedFetch` preset.
  • Python (adcp-client-python#272): default `replay_store`, `install_signing_event_hook` preset, migration guide.
  • Go (this PR): default replay store, `NewSignedHTTPClient` preset.

Both backwards compatible — existing callers that pass explicit `Replay` or build the signing client by hand are unaffected.

1. `Middleware` defaults `Replay` to in-memory

`MiddlewareOptions.Replay` was a required field — passing nil silently disabled replay-check, the security regression AdCP verifier checklist step 13 exists to prevent. Now defaults to a fresh `NewMemoryReplayStore(0)` when nil, constructed once at wire-up so every request served by the middleware shares the same dedup state. Multi-replica deployments still pass an explicit shared store (Redis, etc.) to coordinate state across processes.

`Revocation` stays nil-on-omission — the existing wire-up warning ('RequiredFor is non-empty but Revocation is nil — verifier will not enforce key revocation') is the right behavior because revocation is operator-driven; defaulting it would silence a useful diagnostic.

2. `NewSignedHTTPClient(SignedHTTPClientOptions)` preset

Single-call buyer preset that bundles `NewSigner` + `RoundTripper` + a redirect-safe `*http.Client`. Disables `CheckRedirect` for you — `@target-uri` is part of the signature base, so a 3xx redirect would silently re-target the binding (the existing MIGRATION.md calls this out as a common production failure). Adapter authors who want this safety baked in get it without remembering to set `CheckRedirect` themselves:

```go
client, err := signing.NewSignedHTTPClient(signing.SignedHTTPClientOptions{
KeyID: "my-agent-2026",
PrivateKey: priv,
CoverContentDigest: true,
Timeout: 30 * time.Second,
})
```

Lower-level callers still reach for `NewSigner` + `RoundTripper` directly — those primitives are unchanged.

Test plan

  • 6 new tests in `adcp/signing/client_test.go`:
    • `TestNewSignedHTTPClientRoundTripsAgainstVerifier` — preset → middleware round trip with Ed25519 key and seller-side verification.
    • `TestNewSignedHTTPClientRejectsRedirects` — confirms `CheckRedirect` is wired so a 3xx is surfaced to the caller, not followed.
    • `TestNewSignedHTTPClientRequiresKeyMaterial` — both empty options and missing fields error cleanly.
    • `TestNewSignedHTTPClientRespectsCustomInnerTransport` — inner transport composition order proven.
    • `TestMiddlewareDefaultsReplayStoreToInMemory` — middleware accepts signed requests with no `Replay` field set.
    • `TestMiddlewareDefaultReplayStoreRejectsActualReplay` — same (kid, nonce, body) replayed gets a 401 with `request_signature_replayed`.
  • `go test ./...` clean across `adcp`, `adcp/idempotency`, `adcp/signing`, `adcp/webhook`.
  • `go vet ./...` clean.
  • `gofmt` clean.

Migration impact

None. Existing callers:

  • `Middleware(MiddlewareOptions{Replay: myStore, ...})` → unchanged.
  • `Middleware(MiddlewareOptions{...})` without `Replay` → previously had no replay protection, now has in-memory protection. Anyone affected was relying on a footgun.
  • Manual `signer.RoundTripper(...)` integration → unchanged.

cc @benminer — this lands the Go half of the cross-SDK ergonomics work we discussed on adcp#3064. The Python sibling is at adcp-client-python#272.

🤖 Generated with Claude Code

bokelley and others added 2 commits April 24, 2026 19:53
Two ergonomic upgrades to bring adcp-go into parity with the TS
ergonomics shipped in adcp-client #917 and the Python ergonomics in
adcp-client-python #272.

1. Middleware defaults Replay to NewMemoryReplayStore(0) when nil. The
   default is constructed once at wire-up so every request served by
   the middleware shares the same dedup state — lazy per-request
   construction would defeat replay detection. Defaulting to nil
   silently disabled replay-check, the security regression AdCP
   verifier checklist step 13 exists to prevent. Multi-replica
   deployments still pass an explicit shared store (Redis, etc.) to
   coordinate state across processes.

2. NewSignedHTTPClient(SignedHTTPClientOptions) — single-call buyer
   preset that bundles NewSigner + RoundTripper + redirect-safe
   *http.Client. Disables CheckRedirect for you (3xx redirects would
   silently re-target the signature binding because @target-uri is
   covered). The preset is the third option in the README's signing
   examples; lower-level callers still reach for NewSigner +
   RoundTripper directly.

Tests: 6 new (4 for the preset including round-trip-against-verifier,
redirect refusal, key validation, custom inner transport; 2 for the
middleware default proving the in-memory store actually rejects
replays end-to-end). go test ./... clean. go vet clean. gofmt clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds optional CapabilityProvider on SignedHTTPClientOptions for
parity with the Python `capability_provider` and TS `getCapability`
shapes. When set, the preset:

  - reads the seller's RequestSigningCapabilities per outbound request;
  - signs only when the operation appears in required_for / warn_for /
    supported_for (mirrors verifier-side semantics and Python
    operation_needs_signing);
  - honors covers_content_digest per-call ("required" → cover,
    "forbidden" → don't, "either"/absent → fall back to
    CoverContentDigest).

OperationResolver defaults to the `/adcp/<op>` path-suffix convention
to match DefaultOperationResolver on the verifier side. Adapters
using a different routing scheme override OperationResolver.

When CapabilityProvider is nil, behavior is unchanged — every
outbound request is signed unconditionally with CoverContentDigest as
the digest decision.

Tests: 6 new for the capability-aware path (required_for signs,
unlisted skips, nil-capability skips, covers='required' covers,
covers='forbidden' doesn't, supported=false skips). The existing
key-validation test is also tightened to actually exercise the
malformed-key path: previously asserted on `ed25519.PrivateKey([]byte("not-a-key"))`,
which doesn't fail at construction (NewSigner only switches on the
static type, not byte length). Now uses an unsupported key type.

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.

1 participant