feat(sdk): add signed manifest helpers - #36
Merged
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0362c52b3c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
marmar9615-cloud
added a commit
that referenced
this pull request
Apr 28, 2026
Two correctness fixes flagged in Codex review of PR #36: 1. (P1) Deep-clone the signed payload before returning. `signManifest` previously returned `{ ...rest, signature }` — a shallow copy that still shared nested references (`actions`, `resources`, `auth`, etc.) with the caller's input manifest. A caller who mutated `manifest.actions[0]` after signing would silently mutate `signed.actions[0]` too, and the signature would no longer cover the published bytes. Fix: round-trip the signature-stripped manifest through JSON.parse(JSON.stringify(...)) before attaching the signature. This produces an independent tree shaped exactly like the bytes a publisher would serve, so subsequent mutations to the input cannot affect the signed output. Test added: build a manifest, sign it, push a tampered entry into `manifest.actions`, assert `signed.actions.length` is unchanged. 2. (P2) Preserve sub-second expiresInSeconds. `resolveExpiresAt` used `Math.floor(expiresInSeconds) * 1000`, which truncated values between 0 and 1 to 0ms — producing a signature that was expired the moment it was minted. A user passing 0.5 (legal positive finite number) silently got expiresAt === signedAt. Fix: compute the offset in milliseconds with `Math.round(expiresInSeconds * 1000)`, and add an explicit "expiresAt must be strictly after signedAt" guard that catches any rounding that collapses to zero (e.g. 0.0001s). Two tests added: 0.5s → 500ms offset accepted; 0.0001s rejected. No package version change. No runtime-API surface change. Unsigned manifests still validate. Verified locally: - npx vitest run packages/sdk/src/tests/signing.test.ts (42/42) - npm test (342/342 across 21 files) Refs: PR #36, issue #31 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Second implementation PR for v0.5.0 signed manifests. Builds on the canonicalization + Zod schemas from PR #35 and adds the publisher-side signer to @marmarlabs/agentbridge-sdk. No verifier, no scanner / MCP / CLI enforcement, no version bump, no behavior change for unsigned manifests. Adds: - packages/sdk/src/signing.ts — signManifest(manifest, options) and createSignedManifest(config, options). Uses Node `crypto` exclusively; no new runtime dependency. signManifest API: - alg: "EdDSA" (default) | "ES256" - kid: string (required) - issuer: string (defaults to new URL(manifest.baseUrl).origin) - privateKey: KeyObject | string (PEM) | Buffer (PEM bytes) - signedAt: Date | string (defaults to new Date()) - expiresAt: Date | string (precedence over expiresInSeconds) - expiresInSeconds: number (sets expiresAt = signedAt + N s) - default expiresAt = signedAt + 24h Internals: - Strips any pre-existing `signature` field via canonicalizeManifestForSigning() (re-signing produces a fresh signature, never one over a payload that already contained a stale signature). - For EdDSA: crypto.sign(null, data, key) — 64-byte raw. - For ES256: crypto.sign("sha256", data, { key, dsaEncoding: "ieee-p1363" }) — 64-byte raw r||s, matching JWS ES256 (NOT DER-encoded). - Validates the produced signature object via the core ManifestSignatureSchema before attaching. - Returns a NEW manifest object; input is never mutated. - Validates the resolved `iss` is a canonical origin (URL(s).origin === s). - Verifies the supplied private key matches the requested algorithm (ed25519 vs P-256/prime256v1). - Errors never echo private key material; createPrivateKey failures surface only the Node error code (e.g. ERR_OSSL_BAD_KEY_TYPE). Raw 32-byte Ed25519 seeds are intentionally NOT supported in this PR. The PKCS#8 conversion is non-trivial; adopters can wrap a seed with their own KeyObject adapter. - packages/sdk/src/tests/signing.test.ts — 39 tests: * Mutation: input manifest is unchanged; pre-existing signature stripped on re-sign without mutating input. * Schema: attached signature passes ManifestSignatureSchema; defaults (alg=EdDSA, iss=baseUrl origin); explicit canonical-origin issuer respected; non-canonical / unparseable issuer rejected. * Time: signedAt as Date / ISO string; expiresAt explicit; expiresInSeconds; default 24h; expiresAt <= signedAt rejected; expiresInSeconds <= 0 rejected; malformed signedAt rejected. * Crypto: Ed25519 round-trip via crypto.verify (KeyObject / PEM string / PEM Buffer inputs); tamper detection (mutated non-signature field fails verification); cross-key verification fails. ES256 round-trip; 64-byte r||s output (not DER); KeyObject + PEM inputs. * Validation: unsupported alg rejected; wrong key type for alg rejected; public KeyObject rejected; unparseable PEM rejected without echoing input; non-buffer/non-string/ non-KeyObject privateKey rejected; empty kid rejected; malformed manifest.baseUrl with no issuer rejected. * Integration: validateManifest accepts the signed result; createSignedManifest signs a manifest produced by createAgentBridgeManifest; signing failures propagate through createSignedManifest. * Determinism: re-signing the same manifest with the same Ed25519 key + same signedAt yields identical bytes; createPublicKey(privateKey) verifies (round-trip via derived public key). * Backward compat: unsigned createAgentBridgeManifest still validates. Modifies: - packages/core/src/signing/canonical.ts — canonicalizeManifestForSigning now JSON-roundtrips the signature-stripped manifest before canonicalizing. SDK-built manifests routinely carry `undefined` in optional slots (outputSchema, humanReadableSummaryTemplate, etc.) when an adopter omits them; without this step every realistic manifest failed canonicalization with "undefined property values are not representable in canonical JSON". The bytes the signer produces now match the bytes the publisher serves (which goes through JSON.stringify and drops the same fields). The strict canonicalizeJson is unchanged — it still throws on undefined / function / symbol / non-finite numbers when called directly. - packages/core/src/tests/canonical.test.ts — adds two tests: one that asserts canonicalizeManifestForSigning drops undefined optional fields (matches publisher JSON.stringify), and one that asserts the strict canonicalizeJson still rejects undefined properties (the relaxation is scoped to the manifest helper). - packages/sdk/src/index.ts — re-exports the signing surface. - packages/sdk/README.md — adds a concise "Signing manifests (v0.5.0)" section with example, algorithm matrix, key input formats, and explicit notes that private keys never belong in manifests, raw seeds aren't supported in this PR, the verifier and runtime enforcement are planned follow-ups, and unsigned manifests still validate. Does NOT (deliberately): - add verifyManifestSignature (verifier ships in a later PR) - fetch remote keys - add scanner signature checks - add MCP server enforcement - add CLI --require-signature - change unsigned-manifest behavior - bump any package version - add any runtime dependency - publish, tag, or release Verified locally: - npm run typecheck:clean (clean) - npm test (339/339 across 21 files; was 298/19 on main = +41) - npm run build (all packages built) - npm run pack:dry-run (all six @marmarlabs/agentbridge-* OK at 0.4.0; SDK packed 9.9 → 16.5KB for the new module) - npx tsx examples/sdk-basic/manifest.ts | node packages/cli/dist/bin.js validate (unsigned SDK example still validates: SDK Basic Support v1.0.0) Tracking: #31 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The "re-signing the same manifest with the same Ed25519 key yields identical bytes" test built the manifest twice via `buildManifest()`, which calls `createAgentBridgeManifest` and stamps a fresh `generatedAt: new Date().toISOString()` on each call. When the two calls landed in different millisecond ticks (visible on CI, hidden on a fast local machine) the manifests differed, so the canonical bytes differed, so the signatures differed. Build the manifest once, sign it twice. Same key + same payload + same fixed signedAt → same Ed25519 signature value. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two correctness fixes flagged in Codex review of PR #36: 1. (P1) Deep-clone the signed payload before returning. `signManifest` previously returned `{ ...rest, signature }` — a shallow copy that still shared nested references (`actions`, `resources`, `auth`, etc.) with the caller's input manifest. A caller who mutated `manifest.actions[0]` after signing would silently mutate `signed.actions[0]` too, and the signature would no longer cover the published bytes. Fix: round-trip the signature-stripped manifest through JSON.parse(JSON.stringify(...)) before attaching the signature. This produces an independent tree shaped exactly like the bytes a publisher would serve, so subsequent mutations to the input cannot affect the signed output. Test added: build a manifest, sign it, push a tampered entry into `manifest.actions`, assert `signed.actions.length` is unchanged. 2. (P2) Preserve sub-second expiresInSeconds. `resolveExpiresAt` used `Math.floor(expiresInSeconds) * 1000`, which truncated values between 0 and 1 to 0ms — producing a signature that was expired the moment it was minted. A user passing 0.5 (legal positive finite number) silently got expiresAt === signedAt. Fix: compute the offset in milliseconds with `Math.round(expiresInSeconds * 1000)`, and add an explicit "expiresAt must be strictly after signedAt" guard that catches any rounding that collapses to zero (e.g. 0.0001s). Two tests added: 0.5s → 500ms offset accepted; 0.0001s rejected. No package version change. No runtime-API surface change. Unsigned manifests still validate. Verified locally: - npx vitest run packages/sdk/src/tests/signing.test.ts (42/42) - npm test (342/342 across 21 files) Refs: PR #36, issue #31 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
marmar9615-cloud
force-pushed
the
feature/v050-sdk-sign-manifest
branch
from
April 28, 2026 22:16
cb626b9 to
c7d83ae
Compare
This was referenced Apr 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Second implementation PR for v0.5.0 signed manifests. Adds the
publisher-side signer to
@marmarlabs/agentbridge-sdk, building onthe canonicalization + Zod schemas from PR #35. No verifier, no
scanner / MCP / CLI enforcement, no version bump, no behavior change
for unsigned manifests.
docs/designs/signed-manifests.mddocs/adr/0002-signed-manifests.md46adc2csignManifest(manifest, options)Returns a NEW manifest with an attached
signatureblock. Input isnever mutated. Any pre-existing
signatureis stripped beforecanonicalization — re-signing produces a fresh signature, never one
over a payload that already contained a stale one.
Algorithms supported
EdDSA(Ed25519) — default.crypto.sign(null, data, key),64-byte raw signature, deterministic.
ES256(ECDSA P-256, SHA-256) —crypto.sign("sha256", data, { key, dsaEncoding: "ieee-p1363" }), 64-byte rawr||soutput(matching JWS ES256, not DER-encoded). For HSM/KMS-bound
publishers.
RSA-family (
RS256,PS256, …) is intentionally excluded per thev0.5.0 design — adding one in a later release is a non-breaking
enum extension.
Private key input formats
KeyObject(recommended in production).
Buffercontaining PEM bytes.Raw 32-byte Ed25519 seeds are not supported in this PR — the
PKCS#8 conversion is non-trivial. Adopters can wrap a seed with their
own KeyObject adapter and pass the result.
Errors never echo private key material; PEM parse failures surface
only the Node error code (
ERR_OSSL_BAD_KEY_TYPEetc.).createSignedManifest(config, options)Sugar for
signManifest(createAgentBridgeManifest(config), options).Useful for adopters whose build pipeline produces and signs a fresh
manifest each release.
Internals worth flagging
canonicalizeManifestForSigningnow JSON-roundtrips thesignature-stripped manifest before canonicalizing. SDK-built
manifests routinely carry
undefinedin optional slots(
outputSchema,humanReadableSummaryTemplate, …) when an adopteromits them; the publisher's
JSON.stringifydrops them, and somust the signer or no realistic manifest could ever be signed. The
strict
canonicalizeJsonis unchanged — it still throws onundefined/ function / symbol / non-finite numbers when calleddirectly. Two new tests in
core/canonical.test.tspin bothbehaviors.
issis always a canonical origin. Default isnew URL(manifest.baseUrl).origin. An explicitoptions.issueris rejected if
URL(s).origin !== s(no trailing slashes, nopaths, etc.) — this keeps verifier comparisons deterministic.
paired with
alg: "ES256"(or vice versa) reject up front, socallers can't accidentally produce a signature with a key the
verifier expects to be a different curve.
Tests added
packages/sdk/src/tests/signing.test.ts— 39 tests covering:stripped on re-sign.
ManifestSignatureSchema; alg/issdefaults; canonical-origin enforcement.
signedAt/expiresAtas Date/string;expiresInSeconds;default 24h; expiry-before-signedAt rejected; non-positive
expiresInSecondsrejected.crypto.verifyfor KeyObject /PEM string / PEM Buffer inputs; tamper detection (mutating any
non-
signaturefield fails verification); cross-key verificationfails. ES256 round-trip; 64-byte
r||soutput (not DER).unparseable PEM (without echoing input), wrong-type
privateKey, emptykid, malformedmanifest.baseUrlwith noissuer.validateManifestaccepts the signed result;createSignedManifestend-to-end; signing failures propagate.same
signedAtyields identical bytes;createPublicKeyround-trip.
createAgentBridgeManifeststillvalidates.
packages/core/src/tests/canonical.test.tsgains 2 new tests forthe
canonicalizeManifestForSigningundefined-drop behavior andthe unchanged strict
canonicalizeJson.Confirmations
tests and by structural-clone semantics in the implementation.
validateManifest. Verified by integration test.remain follow-up PRs in the v0.5.0 line.
manifest.test.ts,spec-examples.test.ts,validateManifest({ ...withoutSignature })returning thesame shape, and by re-running the SDK example through
agentbridge validate.pack:dry-runconfirms allsix
@marmarlabs/agentbridge-*packages still at0.4.0.cryptousage.package-lock.jsonuntouched.edits under
packages/cli/*,packages/scanner/*,examples/*,scripts/*,apps/mcp-server/*,docs/releases/*, the root README, orCHANGELOG.md.pinning, target-origin allowlist, audit redaction, stdio
stdout hygiene, HTTP transport auth/origin checks all
unchanged.
Commands run
Test plan
0.4.0.🤖 Generated with Claude Code