From e84210cfb647b716fec574b9003559e928cb8e6d Mon Sep 17 00:00:00 2001 From: globalmysterysnailrevolution Date: Thu, 1 Oct 2026 02:44:47 -0700 Subject: [PATCH 1/5] test(verifier,gateway): reproduce non-canonical session-key scope (#361 E7 F4) #361 commits sessionKeyAuthDigest = sha256(canonicalize(SessionKeyAuthorization)). The cross-family review found one authorization with several digests: issueSessionKey (and deriveSessionKey) put the caller's scope.allowedActions and scope.contractIds into the emitted key verbatim, and canonicalSessionKeyBytes sorts a copy for signing but keeps duplicates. Reproduced first on 75fd440b, tests only, no source change: - verifier ephemeral-identity-canonical-scope.test.ts: 11 failed, 9 passed (20). ["workflow_step_complete","evidence_submit","evidence_submit"] with ["c2","c1","c2"] is emitted unsorted and duplicated; two spellings of one scope emit different arrays; a duplicate changes the signed canonical bytes (randomness, time and the session key are pinned, so the only difference in the signed JSON is the duplicate inside scope); code-unit order ("B" before "a") is not applied; the emitted arrays alias the caller's. - gateway identity-session.test.ts: 3 failed, 1 passed (4). POST /api/identity/session returns the same unsorted, duplicated scope. The passing tests are deliberate characterization and must stay green after the fix: reordering alone is already absorbed by the canonicalizer's sort, an issued key verifies end to end, and keys signed over sorted-but-duplicated arrays (what the old issuer produced) still verify. Agent: implementer-sessionkey Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01GihjSLwaMhkBW2DAERqsi1 --- .../src/__tests__/identity-session.test.ts | 132 +++++++ ...ephemeral-identity-canonical-scope.test.ts | 366 ++++++++++++++++++ 2 files changed, 498 insertions(+) create mode 100644 packages/gateway/src/__tests__/identity-session.test.ts create mode 100644 packages/verifier/src/workflow/__tests__/ephemeral-identity-canonical-scope.test.ts diff --git a/packages/gateway/src/__tests__/identity-session.test.ts b/packages/gateway/src/__tests__/identity-session.test.ts new file mode 100644 index 000000000..8a6b150e1 --- /dev/null +++ b/packages/gateway/src/__tests__/identity-session.test.ts @@ -0,0 +1,132 @@ +/** + * POST /api/identity/session — the scope it returns is canonical. + * + * #361 commits sessionKeyAuthDigest = sha256(canonicalize(SessionKeyAuthorization)) + * over the scope this route returns. If the route hands back the caller's arrays + * verbatim, one authorization has several digests ([a,b], [b,a], [a,a,b]). + * The scope must come back sorted (UTF-16 code-unit order) and de-duplicated, + * and it must be the SAME scope the parent signed. + */ + +import { describe, it, expect, beforeAll, afterAll } from "vitest"; +import Fastify, { type FastifyInstance } from "fastify"; +import nacl from "tweetnacl"; +import { identitySessionRoutes } from "../routes/identity-session.js"; + +let app: FastifyInstance; + +beforeAll(async () => { + app = Fastify({ logger: false }); + await app.register(identitySessionRoutes); + await app.ready(); +}); + +afterAll(async () => { + await app.close(); +}); + +const hex = (bytes: Uint8Array): string => Buffer.from(bytes).toString("hex"); + +/** + * The route derives a mock principal from the id (no wallet yet): seed = the + * first 32 characters of principalAgentId, NUL padded. Mirror it to get the + * parent public key that /verify needs. + */ +function parentPublicKeyFor(principalAgentId: string): Uint8Array { + const seed = new TextEncoder().encode(principalAgentId.padEnd(32, "\0").slice(0, 32)); + return nacl.sign.keyPair.fromSeed(seed).publicKey; +} + +async function issue(payload: Record) { + return app.inject({ method: "POST", url: "/api/identity/session", payload }); +} + +describe("POST /api/identity/session", () => { + it("returns a sorted, de-duplicated scope for an unsorted, duplicated request", async () => { + const res = await issue({ + principalAgentId: "agent-skey-route-1", + scope: { + allowedActions: ["workflow_step_complete", "evidence_submit", "evidence_submit"], + contractIds: ["c2", "c1", "c2"], + }, + ttlSeconds: 300, + }); + + expect(res.statusCode).toBe(200); + expect(res.json().sessionKey.scope).toEqual({ + allowedActions: ["evidence_submit", "workflow_step_complete"], + contractIds: ["c1", "c2"], + maxSignatures: 1000, + }); + }); + + it("returns the same scope for requests that differ only in order or duplicates", async () => { + const spellings = [ + { allowedActions: ["evidence_submit", "workflow_step_complete"], contractIds: ["c1", "c2"] }, + { allowedActions: ["workflow_step_complete", "evidence_submit"], contractIds: ["c2", "c1"] }, + { + allowedActions: ["evidence_submit", "evidence_submit", "workflow_step_complete"], + contractIds: ["c1", "c1", "c2", "c1"], + }, + ]; + + const scopes = []; + for (const scope of spellings) { + const res = await issue({ principalAgentId: "agent-skey-route-2", scope }); + expect(res.statusCode).toBe(200); + scopes.push(res.json().sessionKey.scope); + } + + expect(scopes[1]).toEqual(scopes[0]); + expect(scopes[2]).toEqual(scopes[0]); + }); + + it("orders contract ids by UTF-16 code unit, not by locale", async () => { + const res = await issue({ + principalAgentId: "agent-skey-route-3", + scope: { contractIds: ["a", "B"] }, + }); + + expect(res.statusCode).toBe(200); + expect(res.json().sessionKey.scope.contractIds).toEqual(["B", "a"]); + }); + + it("the returned key verifies through /verify with the scope exactly as returned", async () => { + const principalAgentId = "agent-skey-route-4"; + const res = await issue({ + principalAgentId, + scope: { + allowedActions: ["workflow_step_complete", "evidence_submit", "evidence_submit"], + contractIds: ["c2", "c1", "c2"], + }, + }); + expect(res.statusCode).toBe(200); + const issued = res.json(); + + const eventData = new TextEncoder().encode("route-round-trip"); + const sessionSignature = nacl.sign.detached( + eventData, + Uint8Array.from(Buffer.from(issued.sessionPrivateKey, "hex")), + ); + + const verified = await app.inject({ + method: "POST", + url: "/api/identity/session/verify", + payload: { + event: { + eventData: hex(eventData), + sessionSignature: hex(sessionSignature), + proof: { + sessionKey: issued.sessionKey, + parentPublicKey: hex(parentPublicKeyFor(principalAgentId)), + }, + }, + action: "evidence_submit", + }, + }); + + expect(verified.statusCode).toBe(200); + expect(verified.json().failures).toEqual([]); + expect(verified.json().valid).toBe(true); + }); +}); diff --git a/packages/verifier/src/workflow/__tests__/ephemeral-identity-canonical-scope.test.ts b/packages/verifier/src/workflow/__tests__/ephemeral-identity-canonical-scope.test.ts new file mode 100644 index 000000000..0f3056c89 --- /dev/null +++ b/packages/verifier/src/workflow/__tests__/ephemeral-identity-canonical-scope.test.ts @@ -0,0 +1,366 @@ +/** + * Canonical scope for issued session keys (answers #361 E7 F4). + * + * scope.allowedActions and scope.contractIds are SETS in meaning but ARRAYS on + * the wire. #361 commits sessionKeyAuthDigest = sha256(canonicalize( + * SessionKeyAuthorization)), so unless the issuer emits ONE spelling of a + * scope, [a,b], [b,a] and [a,a,b] are three digests for one authorization. + * + * The canonical spelling is: duplicates dropped, then sorted in UTF-16 + * code-unit order (Array.prototype.sort() with no comparator, never + * localeCompare), built BEFORE the struct is signed so the emitted object and + * the signature agree. + * + * Randomness and time are stubbed so two issuances can be compared byte for + * byte: same sessionId, same session key, same issuedAt and expiresAt. + * Ed25519 is deterministic, so equal canonical bytes <=> equal parentSignature. + */ + +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import nacl from "tweetnacl"; +import type { + AgentRegistryId, + PrincipalKey, + SessionAction, + SessionKey, + SessionKeyConfig, + SessionScope, +} from "@pcc/spec"; +import { DEFAULT_SESSION_KEY_CONFIG } from "@pcc/spec"; +import { SessionKeyService } from "../ephemeral-identity.js"; + +// --------------------------------------------------------------------------- +// Controllable randomUUID. Transparent (real UUIDs) unless a test pins it. +// --------------------------------------------------------------------------- + +const rng = vi.hoisted(() => ({ uuid: undefined as string | undefined })); + +vi.mock("node:crypto", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, randomUUID: () => rng.uuid ?? actual.randomUUID() }; +}); + +// --------------------------------------------------------------------------- +// Fixtures +// --------------------------------------------------------------------------- + +const FIXED_NOW_MS = 1_700_000_000_000; +const FIXED_ISSUED_AT = FIXED_NOW_MS / 1000; +const FIXED_UUID = "00000000-0000-4000-8000-000000000001"; + +// Keys come from fixed seeds (tweetnacl), never from real key material. +const principalKeypair = nacl.sign.keyPair.fromSeed(new Uint8Array(32).fill(7)); +const sessionKeypair = nacl.sign.keyPair.fromSeed(new Uint8Array(32).fill(9)); + +const principal: PrincipalKey = { + agentId: "eip155:84532:0xDeadBeef00000000000000000000000000000001" as AgentRegistryId, + walletAddress: "0xDeadBeef00000000000000000000000000000001", + publicKey: principalKeypair.publicKey, +}; + +const service = new SessionKeyService(); + +const hex = (bytes: Uint8Array): string => Buffer.from(bytes).toString("hex"); + +/** The task's example: unsorted, with a duplicate in each list. */ +const MESSY: Pick = { + allowedActions: ["workflow_step_complete", "evidence_submit", "evidence_submit"], + contractIds: ["c2", "c1", "c2"], +}; + +/** The one canonical spelling of MESSY. */ +const CANONICAL: Pick = { + allowedActions: ["evidence_submit", "workflow_step_complete"], + contractIds: ["c1", "c2"], +}; + +/** Inputs that all mean the same authorization, spelled four ways. */ +const VARIANTS: Array<{ label: string; scope: Partial }> = [ + { label: "already canonical", scope: { ...CANONICAL } }, + { + label: "reordered", + scope: { + allowedActions: ["workflow_step_complete", "evidence_submit"], + contractIds: ["c2", "c1"], + }, + }, + { + label: "duplicated", + scope: { + allowedActions: ["evidence_submit", "evidence_submit", "workflow_step_complete"], + contractIds: ["c1", "c1", "c2"], + }, + }, + { label: "reordered and duplicated", scope: { ...MESSY } }, +]; + +/** The documented canonical-bytes layout, written out independently of the + * production helper: what the parent signed before AND after this change. */ +function referenceCanonicalJson(sk: Omit): string { + return JSON.stringify({ + sessionId: sk.sessionId, + parentAgentId: sk.parentAgentId, + publicKey: hex(sk.publicKey), + issuedAt: sk.issuedAt, + expiresAt: sk.expiresAt, + scope: { + allowedActions: [...sk.scope.allowedActions].sort(), + contractIds: [...sk.scope.contractIds].sort(), + maxSignatures: sk.scope.maxSignatures, + }, + ...(sk.derivationPath !== undefined ? { derivationPath: sk.derivationPath } : {}), + }); +} + +function issue(scope?: Partial) { + return service.issueSessionKey({ + principal, + principalPrivateKey: principalKeypair.secretKey, + scope, + }); +} + +/** Issue a key and capture the exact bytes the parent signed. */ +function issueCapturing(scope?: Partial) { + const detached = vi.spyOn(nacl.sign, "detached"); + try { + const issued = issue(scope); + expect(detached).toHaveBeenCalledTimes(1); + const signed = detached.mock.calls[0]![0]; + return { ...issued, signedJson: new TextDecoder().decode(signed) }; + } finally { + // Restore at once: verification needs the real nacl.sign.detached.verify. + detached.mockRestore(); + } +} + +function verifyIssued(issued: { sessionKey: SessionKey; sessionPrivateKey: Uint8Array }, action = "evidence_submit") { + const event = service.signEvent({ + eventData: new TextEncoder().encode("canonical-scope-evidence"), + sessionKey: issued.sessionKey, + sessionPrivateKey: issued.sessionPrivateKey, + parentPublicKey: principalKeypair.publicKey, + }); + return service.verifySessionSignedEvent({ event, action }); +} + +beforeEach(() => { + rng.uuid = FIXED_UUID; + vi.spyOn(Date, "now").mockReturnValue(FIXED_NOW_MS); +}); + +afterEach(() => { + rng.uuid = undefined; + vi.restoreAllMocks(); +}); + +// --------------------------------------------------------------------------- +// issueSessionKey +// --------------------------------------------------------------------------- + +describe("issueSessionKey emits a canonical scope", () => { + beforeEach(() => { + vi.spyOn(nacl.sign, "keyPair").mockImplementation(() => sessionKeypair); + }); + + it("(a) sorts and de-duplicates allowedActions and contractIds in the emitted scope", () => { + const { sessionKey } = issue({ ...MESSY, maxSignatures: 7 }); + + expect(sessionKey.scope).toEqual({ ...CANONICAL, maxSignatures: 7 }); + }); + + it("(a) orders by UTF-16 code unit, not by locale", () => { + // Locale order puts "a" before "B" and e-acute before "z"; code-unit order + // is the reverse of both. + const { sessionKey } = issue({ contractIds: ["a", "B", "z", "é", "b", "A"] }); + + expect(sessionKey.scope.contractIds).toEqual(["A", "B", "a", "b", "z", "é"]); + }); + + it("(a) neither aliases nor reorders the caller's arrays", () => { + const allowedActions = [...MESSY.allowedActions]; + const contractIds = [...MESSY.contractIds]; + + const { sessionKey } = issue({ allowedActions, contractIds }); + + expect(allowedActions).toEqual(MESSY.allowedActions); + expect(contractIds).toEqual(MESSY.contractIds); + expect(sessionKey.scope.allowedActions).not.toBe(allowedActions); + expect(sessionKey.scope.contractIds).not.toBe(contractIds); + + // A later edit to the caller's array cannot change what was signed. + contractIds.push("c9"); + expect(sessionKey.scope.contractIds).toEqual(CANONICAL.contractIds); + }); + + it("(a) canonicalizes defaults that come from the config", () => { + const config: SessionKeyConfig = { + defaultTTLSeconds: 60, + maxTTLSeconds: 120, + defaultMaxSignatures: 5, + defaultAllowedActions: ["heartbeat", "evidence_submit", "heartbeat"], + }; + + const { sessionKey } = service.issueSessionKey({ + principal, + principalPrivateKey: principalKeypair.secretKey, + config, + }); + + expect(sessionKey.scope.allowedActions).toEqual(["evidence_submit", "heartbeat"]); + }); + + it.each(VARIANTS)("(b) $label input emits the canonical scope arrays", ({ scope }) => { + const { sessionKey } = issue(scope); + + expect(sessionKey.scope.allowedActions).toEqual(CANONICAL.allowedActions); + expect(sessionKey.scope.contractIds).toEqual(CANONICAL.contractIds); + }); + + it.each(VARIANTS)( + "(b) $label input signs the same canonical bytes as the canonical input", + ({ scope }) => { + const reference = issueCapturing({ ...CANONICAL }); + const run = issueCapturing(scope); + + expect(run.sessionKey.sessionId).toBe(reference.sessionKey.sessionId); + expect(run.sessionKey.issuedAt).toBe(FIXED_ISSUED_AT); + expect(run.signedJson).toBe(reference.signedJson); + expect(hex(run.sessionKey.parentSignature)).toBe(hex(reference.sessionKey.parentSignature)); + }, + ); + + it("(c) a duplicate changes the signed canonical bytes, because sort keeps duplicates", () => { + const duplicated = issueCapturing({ + allowedActions: ["evidence_submit", "evidence_submit"], + contractIds: ["c1", "c1"], + }); + const once = issueCapturing({ + allowedActions: ["evidence_submit"], + contractIds: ["c1"], + }); + + // One authorization, so one set of canonical bytes. + expect(duplicated.signedJson).toBe(once.signedJson); + }); + + it("(c) reordering alone is already absorbed by the canonicalizer's sort", () => { + const ordered = issueCapturing({ ...CANONICAL }); + const reversed = issueCapturing({ + allowedActions: ["workflow_step_complete", "evidence_submit"], + contractIds: ["c2", "c1"], + }); + + expect(reversed.signedJson).toBe(ordered.signedJson); + }); + + it("(d) the parent signed exactly the canonical form of what was emitted, and it verifies", () => { + const issued = issueCapturing({ ...MESSY }); + + // The signature and the emitted object must agree. + expect(issued.signedJson).toBe(referenceCanonicalJson(issued.sessionKey)); + + const result = verifyIssued(issued); + expect(result.failures).toEqual([]); + expect(result.valid).toBe(true); + expect(result.principalAgentId).toBe(principal.agentId); + }); +}); + +// --------------------------------------------------------------------------- +// Keys issued before the issuer canonicalized must keep verifying +// --------------------------------------------------------------------------- + +describe("keys issued before this change still verify", () => { + /** + * Hand-sign a key the way the old issuer did: scope exactly as the caller + * gave it, parent signature over the canonical form that SORTS but does not + * de-duplicate. + */ + function legacyKey(scope: SessionScope) { + const body = { + sessionId: "legacy-session-0001", + parentAgentId: principal.agentId, + publicKey: sessionKeypair.publicKey, + issuedAt: FIXED_ISSUED_AT, + expiresAt: FIXED_ISSUED_AT + 3600, + scope, + }; + const parentSignature = nacl.sign.detached( + new TextEncoder().encode(referenceCanonicalJson(body)), + principalKeypair.secretKey, + ); + return { sessionKey: { ...body, parentSignature }, sessionPrivateKey: sessionKeypair.secretKey }; + } + + it.each([ + { + label: "sorted and de-duplicated", + allowedActions: ["evidence_submit", "workflow_step_complete"] as SessionAction[], + contractIds: ["c1", "c2"], + }, + { + label: "sorted but still carrying duplicates", + allowedActions: ["evidence_submit", "evidence_submit", "workflow_step_complete"] as SessionAction[], + contractIds: ["c1", "c2", "c2"], + }, + { + label: "unsorted with duplicates, as the old issuer emitted it", + allowedActions: [...MESSY.allowedActions], + contractIds: [...MESSY.contractIds], + }, + ])("verifies a key whose scope is $label", ({ allowedActions, contractIds }) => { + const issued = legacyKey({ allowedActions, contractIds, maxSignatures: 1000 }); + + const event = service.signEvent({ + eventData: new TextEncoder().encode("legacy-evidence"), + sessionKey: issued.sessionKey, + sessionPrivateKey: issued.sessionPrivateKey, + parentPublicKey: principalKeypair.publicKey, + }); + const result = service.verifySessionSignedEvent({ + event, + action: "evidence_submit", + currentTimestamp: FIXED_ISSUED_AT + 10, + }); + + expect(result.failures).toEqual([]); + expect(result.valid).toBe(true); + }); +}); + +// --------------------------------------------------------------------------- +// deriveSessionKey builds its scope the same way +// --------------------------------------------------------------------------- + +describe("deriveSessionKey emits the same canonical scope", () => { + const parentSeed = Uint8Array.from({ length: 32 }, (_, i) => i + 1); + const path = "m/8004'/84532'/1'/0'"; + + function derive(scope?: Partial) { + return service.deriveSessionKey({ + parentSeed, + path, + principal, + principalPrivateKey: principalKeypair.secretKey, + scope, + }); + } + + it("sorts and de-duplicates the emitted scope", () => { + const { sessionKey } = derive({ ...MESSY }); + + expect(sessionKey.scope).toEqual({ + ...CANONICAL, + maxSignatures: DEFAULT_SESSION_KEY_CONFIG.defaultMaxSignatures, + }); + }); + + it("verifies end to end with a messy input scope", () => { + const derived = derive({ ...MESSY }); + + const result = verifyIssued(derived); + expect(result.failures).toEqual([]); + expect(result.valid).toBe(true); + }); +}); From a81e3d35d1616c0aa8d22856fac65c68687e1c87 Mon Sep 17 00:00:00 2001 From: globalmysterysnailrevolution Date: Thu, 1 Oct 2026 02:47:24 -0700 Subject: [PATCH 2/5] fix(verifier): issueSessionKey and deriveSessionKey emit a canonical scope (#361 E7 F4) Both issuers now build scope.allowedActions and scope.contractIds as [...new Set(xs)].sort() (UTF-16 code-unit order, never localeCompare) BEFORE the SessionKey struct is signed. The emitted object, the bytes the parent signs and any digest taken over the emitted authorization now describe one scope, so one authorization has one digest however the caller spelled the lists. The arrays are fresh copies: a later edit to the caller's array can no longer change an issued key. A scope list that is not an array of strings is refused with a clear Error before anything is signed ("scope.contractIds[1] must be a string, got null"). A bare string is refused too, since spreading it would turn "abc" into ["a","b","c"]. canonicalSessionKeyBytes is deliberately unchanged. It sorts a copy but does not de-duplicate, and it must stay that way: a key signed over sorted-with-duplicates arrays (what the old issuer produced) is verified against exactly those bytes, so de-duplicating there would break keys that are already in the field. A test hand-signs such keys, including the unsorted-with-duplicates shape the old issuer emitted, and pins that they still verify. verifySessionSignedEvent is untouched. Existing test changed, one assertion: test 2 in ephemeral-identity.test.ts passed ["touchstone_response","attestation_sign"] and asserted the same order back, i.e. it pinned the old verbatim emission this fix removes. It is still an exact toEqual with the same unsorted input; the expected array is now the canonical order, with a comment saying why. Tests added to ephemeral-identity-canonical-scope.test.ts: malformed scope rejected by both issuers (8 shapes each) and before the signer is reached; empty lists unchanged; a surrogate pair sorts before U+FF5E (code unit, not code point). The reproduction tests from e84210cf now pass: 39/39 in that file, and 39/39 in the existing file. Agent: implementer-sessionkey Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01GihjSLwaMhkBW2DAERqsi1 --- ...ephemeral-identity-canonical-scope.test.ts | 94 ++++++++++++++++++ .../__tests__/ephemeral-identity.test.ts | 5 +- .../src/workflow/ephemeral-identity.ts | 98 ++++++++++++++++--- 3 files changed, 180 insertions(+), 17 deletions(-) diff --git a/packages/verifier/src/workflow/__tests__/ephemeral-identity-canonical-scope.test.ts b/packages/verifier/src/workflow/__tests__/ephemeral-identity-canonical-scope.test.ts index 0f3056c89..7acc85d4d 100644 --- a/packages/verifier/src/workflow/__tests__/ephemeral-identity-canonical-scope.test.ts +++ b/packages/verifier/src/workflow/__tests__/ephemeral-identity-canonical-scope.test.ts @@ -177,6 +177,15 @@ describe("issueSessionKey emits a canonical scope", () => { expect(sessionKey.scope.contractIds).toEqual(["A", "B", "a", "b", "z", "é"]); }); + it("(a) orders by code UNIT, so a surrogate pair sorts before U+FF5E (code-point order would not)", () => { + // U+1F600 is the pair D83D DE00, and 0xD83D < 0xFF5E, so code-unit order puts + // it first. Its code point (0x1F600) is larger, so code-point and UTF-8 byte + // order put it last. Any verifier recomputing the digest must match this. + const { sessionKey } = issue({ contractIds: ["～", "😀"] }); + + expect(sessionKey.scope.contractIds).toEqual(["😀", "～"]); + }); + it("(a) neither aliases nor reorders the caller's arrays", () => { const allowedActions = [...MESSY.allowedActions]; const contractIds = [...MESSY.contractIds]; @@ -267,6 +276,91 @@ describe("issueSessionKey emits a canonical scope", () => { }); }); +// --------------------------------------------------------------------------- +// A malformed scope is refused, and before anything is signed +// --------------------------------------------------------------------------- + +describe("a malformed scope is refused before anything is signed", () => { + const parentSeed = Uint8Array.from({ length: 32 }, (_, i) => i + 1); + + // Every row is something an untyped JSON request body can carry past the types. + const malformed: Array<[label: string, scope: unknown, message: RegExp]> = [ + [ + "a number in allowedActions", + { allowedActions: ["evidence_submit", 5] }, + /^scope\.allowedActions\[1\] must be a string, got number$/, + ], + [ + "null in contractIds", + { contractIds: ["c1", null] }, + /^scope\.contractIds\[1\] must be a string, got null$/, + ], + [ + "undefined in contractIds", + { contractIds: ["c1", undefined] }, + /^scope\.contractIds\[1\] must be a string, got undefined$/, + ], + [ + "a hole in a sparse contractIds", + { contractIds: ["c1", , "c3"] }, + /^scope\.contractIds\[1\] must be a string, got undefined$/, + ], + [ + "an object in contractIds", + { contractIds: [{ id: "c1" }] }, + /^scope\.contractIds\[0\] must be a string, got object$/, + ], + [ + "a nested array in allowedActions", + { allowedActions: [["evidence_submit"]] }, + /^scope\.allowedActions\[0\] must be a string, got array$/, + ], + [ + "a bare string where allowedActions belongs", + { allowedActions: "evidence_submit" }, + /^scope\.allowedActions must be an array of strings, got string$/, + ], + [ + "a plain object where contractIds belongs", + { contractIds: { 0: "c1" } }, + /^scope\.contractIds must be an array of strings, got object$/, + ], + ]; + + it.each(malformed)("issueSessionKey rejects %s", (_label, scope, message) => { + expect(() => issue(scope as Partial)).toThrow(message); + }); + + it.each(malformed)("deriveSessionKey rejects %s", (_label, scope, message) => { + expect(() => + service.deriveSessionKey({ + parentSeed, + path: "m/8004'/84532'/1'/0'", + principal, + principalPrivateKey: principalKeypair.secretKey, + scope: scope as Partial, + }), + ).toThrow(message); + }); + + it("never reaches the signer", () => { + const detached = vi.spyOn(nacl.sign, "detached"); + try { + expect(() => issue({ contractIds: [42] } as unknown as Partial)).toThrow(); + expect(detached).not.toHaveBeenCalled(); + } finally { + detached.mockRestore(); + } + }); + + it("still accepts empty lists and returns them empty", () => { + const { sessionKey } = issue({ allowedActions: [], contractIds: [] }); + + expect(sessionKey.scope.allowedActions).toEqual([]); + expect(sessionKey.scope.contractIds).toEqual([]); + }); +}); + // --------------------------------------------------------------------------- // Keys issued before the issuer canonicalized must keep verifying // --------------------------------------------------------------------------- diff --git a/packages/verifier/src/workflow/__tests__/ephemeral-identity.test.ts b/packages/verifier/src/workflow/__tests__/ephemeral-identity.test.ts index 0ab6bd8ba..ae4921f68 100644 --- a/packages/verifier/src/workflow/__tests__/ephemeral-identity.test.ts +++ b/packages/verifier/src/workflow/__tests__/ephemeral-identity.test.ts @@ -125,9 +125,12 @@ describe("SessionKeyService", () => { scope: customScope, }); + // The emitted scope is canonical (de-duplicated, sorted by UTF-16 code + // unit), so the deliberately unsorted input above comes back sorted. + // See ephemeral-identity-canonical-scope.test.ts. expect(sessionKey.scope.allowedActions).toEqual([ - "touchstone_response", "attestation_sign", + "touchstone_response", ]); expect(sessionKey.scope.contractIds).toEqual([ "contract-alpha", diff --git a/packages/verifier/src/workflow/ephemeral-identity.ts b/packages/verifier/src/workflow/ephemeral-identity.ts index 62666ee6b..5ff59d122 100644 --- a/packages/verifier/src/workflow/ephemeral-identity.ts +++ b/packages/verifier/src/workflow/ephemeral-identity.ts @@ -45,6 +45,69 @@ function toHex(bytes: Uint8Array): string { .join(""); } +/** + * Name a value's type for an error message, telling "null" and "array" apart + * from the generic "object". + */ +function describeType(value: unknown): string { + if (value === null) return "null"; + if (Array.isArray(value)) return "array"; + return typeof value; +} + +/** + * Canonical form of one scope list (allowedActions or contractIds): every + * element must be a string, duplicates are dropped, and the rest is sorted in + * UTF-16 code-unit order. That is Array.prototype.sort() with NO comparator; + * never localeCompare, whose order depends on the host's locale and ICU data. + * + * Returns a fresh array, so the emitted scope never aliases the caller's. + * + * @throws If `value` is not an array or any element is not a string. A bare + * string is refused too: spreading it would quietly turn "abc" into + * ["a", "b", "c"]. + */ +function canonicalScopeList(field: string, value: unknown): string[] { + if (!Array.isArray(value)) { + throw new Error(`${field} must be an array of strings, got ${describeType(value)}`); + } + // Index loop, not forEach: a sparse array's holes must be seen as undefined. + for (let i = 0; i < value.length; i++) { + if (typeof value[i] !== "string") { + throw new Error(`${field}[${i}] must be a string, got ${describeType(value[i])}`); + } + } + return [...new Set(value as string[])].sort(); +} + +/** + * Resolve the caller's scope overrides against the config defaults and return + * the CANONICAL scope: allowedActions and contractIds de-duplicated and sorted. + * + * issueSessionKey and deriveSessionKey call this BEFORE the SessionKey struct + * is signed, so one scope has one spelling everywhere: the emitted object, the + * bytes the parent signs, and any digest taken over the emitted authorization. + * Left verbatim, [a,b], [b,a] and [a,a,b] are three spellings (three digests) + * of a single authorization. + * + * canonicalSessionKeyBytes below still only sorts; it does not de-duplicate. + * Keys issued before this existed were signed over sorted-but-duplicated + * arrays, and de-duplicating there would change the bytes they verify against. + */ +function buildCanonicalScope( + overrides: Partial | undefined, + config: SessionKeyConfig, +): SessionScope { + return { + allowedActions: canonicalScopeList( + "scope.allowedActions", + overrides?.allowedActions ?? config.defaultAllowedActions, + ) as SessionAction[], + contractIds: canonicalScopeList("scope.contractIds", overrides?.contractIds ?? []), + maxSignatures: overrides?.maxSignatures ?? config.defaultMaxSignatures, + }; +} + /** * Produce the canonical byte representation of a SessionKey struct * for signing/verification. @@ -55,6 +118,10 @@ function toHex(bytes: Uint8Array): string { * Deterministic JSON: sorted keys, no whitespace, binary fields as hex. * This format is stable across JS engines because we control key order explicitly. * + * The scope lists are sorted here as defense in depth, but NOT de-duplicated: + * a key signed over sorted-with-duplicates arrays (issued before the issuers + * canonicalized their scope) must keep verifying. See buildCanonicalScope. + * * `derivationPath` is INCLUDED in the canonical form ONLY when present. * This keeps backward compatibility: a legacy sessionKey (no derivationPath) * produces the same canonical bytes it did before the field was added, so @@ -113,7 +180,9 @@ export class SessionKeyService { * * @param params.principal - The principalKey (registered agent identity) * @param params.principalPrivateKey - Ed25519 private key (64 bytes in tweetnacl format) - * @param params.scope - Optional scope overrides (merged with defaults) + * @param params.scope - Optional scope overrides (merged with defaults). The + * emitted scope is canonical: allowedActions and contractIds come back + * de-duplicated and sorted (UTF-16 code-unit order), in new arrays. * @param params.ttlSeconds - Optional TTL override * @param params.config - Optional config override * @@ -121,6 +190,7 @@ export class SessionKeyService { * * @throws If TTL exceeds maxTTLSeconds * @throws If principalPrivateKey length is invalid + * @throws If scope.allowedActions or scope.contractIds is not an array of strings */ issueSessionKey(params: { principal: PrincipalKey; @@ -152,12 +222,10 @@ export class SessionKeyService { // Generate a fresh Ed25519 keypair for the session const sessionKeypair = nacl.sign.keyPair(); - // Build scope with defaults - const scope: SessionScope = { - allowedActions: (params.scope?.allowedActions ?? config.defaultAllowedActions) as SessionAction[], - contractIds: params.scope?.contractIds ?? [], - maxSignatures: params.scope?.maxSignatures ?? config.defaultMaxSignatures, - }; + // Build the CANONICAL scope (defaults applied, lists de-duplicated and + // sorted) BEFORE anything is signed, so the emitted scope and the + // signature describe the same bytes. + const scope = buildCanonicalScope(params.scope, config); const now = Math.floor(Date.now() / 1000); @@ -216,7 +284,9 @@ export class SessionKeyService { * Used to populate parentAgentId on the SessionKey struct. * @param params.principalPrivateKey - The principalKey's 64-byte tweetnacl * secretKey. Used to sign the SessionKey struct (authorization). - * @param params.scope - Optional scope overrides (merged with defaults) + * @param params.scope - Optional scope overrides (merged with defaults). The + * emitted scope is canonical, exactly as for issueSessionKey: allowedActions + * and contractIds de-duplicated and sorted (UTF-16 code-unit order). * @param params.ttlSeconds - Optional TTL override * @param params.config - Optional config override * @@ -226,6 +296,7 @@ export class SessionKeyService { * * @throws If the path is non-hardened or malformed (via derivePath) * @throws If TTL exceeds maxTTLSeconds, TTL <= 0, or principalPrivateKey length is wrong + * @throws If scope.allowedActions or scope.contractIds is not an array of strings */ deriveSessionKey(params: { parentSeed: Uint8Array; @@ -270,14 +341,9 @@ export class SessionKeyService { // returns (keeps the signEvent/verify API identical for both modes). const sessionKeypair = nacl.sign.keyPair.fromSeed(derived.privateKey); - // Build scope with defaults (identical logic to issueSessionKey) - const scope: SessionScope = { - allowedActions: (params.scope?.allowedActions ?? - config.defaultAllowedActions) as SessionAction[], - contractIds: params.scope?.contractIds ?? [], - maxSignatures: - params.scope?.maxSignatures ?? config.defaultMaxSignatures, - }; + // Build the CANONICAL scope (same helper as issueSessionKey) BEFORE the + // struct is signed, so derived keys get the same single spelling. + const scope = buildCanonicalScope(params.scope, config); const now = Math.floor(Date.now() / 1000); From 7528bb13c36218430641b427221fafe3876a9593 Mon Sep 17 00:00:00 2001 From: globalmysterysnailrevolution Date: Thu, 1 Oct 2026 02:48:43 -0700 Subject: [PATCH 3/5] fix(gateway): POST /api/identity/session answers a malformed scope with 400 invalid_scope (#361 E7 F4) The route validated only principalAgentId and cast the rest of the body, so a scope list that was not an array of strings went straight to issueSessionKey. Now that issueSessionKey refuses such a list (a81e3d35), that would surface as a 500 session_key_issuance_failed. Reproduced first, route unchanged: 5 tests failed with "expected 500 to be 400". A present allowedActions or contractIds that is not an array of strings is now a 400 { error: "invalid_scope", message: "scope.contractIds[1] must be a string" } and issues nothing (no key material in the body). Absent or null lists still mean the defaults and explicit empty lists still mean empty; tests that pass both before and after this change pin both. The scope the route returns is canonical (sorted, de-duplicated) because issueSessionKey now makes it so; the route needed no change for that, and the route tests from e84210cf pass (13/13 in the file). The route's verify and revoke handlers are untouched. Agent: implementer-sessionkey Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01GihjSLwaMhkBW2DAERqsi1 --- .../src/__tests__/identity-session.test.ts | 77 +++++++++++++++++++ .../gateway/src/routes/identity-session.ts | 37 +++++++++ 2 files changed, 114 insertions(+) diff --git a/packages/gateway/src/__tests__/identity-session.test.ts b/packages/gateway/src/__tests__/identity-session.test.ts index 8a6b150e1..ff50355aa 100644 --- a/packages/gateway/src/__tests__/identity-session.test.ts +++ b/packages/gateway/src/__tests__/identity-session.test.ts @@ -129,4 +129,81 @@ describe("POST /api/identity/session", () => { expect(verified.json().failures).toEqual([]); expect(verified.json().valid).toBe(true); }); + + describe("a malformed scope is a 400 and issues nothing", () => { + // Each row is a request body the route's own types claim cannot happen. + const malformed: Array<[label: string, scope: unknown, message: string]> = [ + [ + "a number in allowedActions", + { allowedActions: ["evidence_submit", 5] }, + "scope.allowedActions[1] must be a string", + ], + [ + "null in contractIds", + { contractIds: ["c1", null] }, + "scope.contractIds[1] must be a string", + ], + [ + "an object in contractIds", + { contractIds: [{ id: "c1" }] }, + "scope.contractIds[0] must be a string", + ], + [ + "a bare string where allowedActions belongs", + { allowedActions: "evidence_submit" }, + "scope.allowedActions must be an array of strings", + ], + [ + "a plain object where contractIds belongs", + { contractIds: { 0: "c1" } }, + "scope.contractIds must be an array of strings", + ], + ]; + + it.each(malformed)("rejects %s", async (_label, scope, message) => { + const res = await issue({ principalAgentId: "agent-skey-route-5", scope }); + + expect(res.statusCode).toBe(400); + expect(res.json()).toEqual({ error: "invalid_scope", message }); + // Nothing was issued: no key material in the body. + expect(res.body).not.toContain("sessionPrivateKey"); + }); + }); + + describe("absent and empty lists keep their meaning", () => { + const DEFAULT_ACTIONS = ["evidence_submit", "workflow_step_complete"]; + + it("issues the defaults when there is no scope at all", async () => { + const res = await issue({ principalAgentId: "agent-skey-route-6" }); + + expect(res.statusCode).toBe(200); + expect(res.json().sessionKey.scope.allowedActions).toEqual(DEFAULT_ACTIONS); + expect(res.json().sessionKey.scope.contractIds).toEqual([]); + }); + + it.each([ + ["an empty scope object", {}], + ["null lists", { allowedActions: null, contractIds: null }], + ])("falls back to the defaults for %s", async (_label, scope) => { + const res = await issue({ principalAgentId: "agent-skey-route-7", scope }); + + expect(res.statusCode).toBe(200); + expect(res.json().sessionKey.scope).toEqual({ + allowedActions: DEFAULT_ACTIONS, + contractIds: [], + maxSignatures: 1000, + }); + }); + + it("returns explicitly empty lists as empty, not as defaults", async () => { + const res = await issue({ + principalAgentId: "agent-skey-route-8", + scope: { allowedActions: [], contractIds: [] }, + }); + + expect(res.statusCode).toBe(200); + expect(res.json().sessionKey.scope.allowedActions).toEqual([]); + expect(res.json().sessionKey.scope.contractIds).toEqual([]); + }); + }); }); diff --git a/packages/gateway/src/routes/identity-session.ts b/packages/gateway/src/routes/identity-session.ts index d4a87af97..49279328d 100644 --- a/packages/gateway/src/routes/identity-session.ts +++ b/packages/gateway/src/routes/identity-session.ts @@ -31,6 +31,29 @@ function fromHex(hex: string): Uint8Array { return bytes; } +// ── Request validation ────────────────────────────────────────────────────── + +/** + * The first problem with the scope lists in an UNTYPED request body, or null. + * + * A list that is absent (undefined or null) means "use the default", as it + * always has. A list that is present must be an array of strings; anything else + * is a 400 here rather than a 500 from issueSessionKey, which enforces the same + * rule and then sorts and de-duplicates the lists. + */ +function scopeProblem(scope: unknown): string | null { + if (scope === null || typeof scope !== "object") return null; + const lists = scope as Record; + for (const field of ["allowedActions", "contractIds"] as const) { + const value = lists[field]; + if (value === undefined || value === null) continue; + if (!Array.isArray(value)) return `scope.${field} must be an array of strings`; + const bad = value.findIndex((item) => typeof item !== "string"); + if (bad !== -1) return `scope.${field}[${bad}] must be a string`; + } + return null; +} + // ── Service singleton ─────────────────────────────────────────────────────── const sessionKeyService = new SessionKeyService(); @@ -58,6 +81,12 @@ export async function identitySessionRoutes(app: FastifyInstance) { * sessionPrivateKey: hex (ONLY returned once!) * } * + * The returned scope is canonical: allowedActions and contractIds are + * de-duplicated and sorted (UTF-16 code-unit order) whatever order or + * duplicates the request carried, and it is the scope the parent signed. + * A present scope list that is not an array of strings is a 400 + * `invalid_scope`. + * * The private key is returned exactly ONCE and never stored by the gateway. */ app.post("/api/identity/session", async (req, reply) => { @@ -76,6 +105,14 @@ export async function identitySessionRoutes(app: FastifyInstance) { }); } + const invalidScope = scopeProblem(body.scope); + if (invalidScope) { + return reply.status(400).send({ + error: "invalid_scope", + message: invalidScope, + }); + } + try { // Generate a mock principal keypair for the authenticated user. // In production, the principal's private key would come from the From 682d5bd82adbb903562222fa86f8e1f4e5194cc9 Mon Sep 17 00:00:00 2001 From: globalmysterysnailrevolution Date: Thu, 1 Oct 2026 11:57:55 -0700 Subject: [PATCH 4/5] test(verifier): reproduce astra pack 145 F1, where scope validation and canonicalization read different values canonicalScopeList validated value[i] by index, then built the scope from `new Set(value)`, which reads the caller's array a second time through its iterator. A direct service caller (JSON cannot carry these) can supply a custom Symbol.iterator, or an index accessor that changes between reads, and get an unvalidated element signed. Three tests (iterator, accessor, allowedActions) fail at 7528bb13: 3 failed | 39 passed. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01GihjSLwaMhkBW2DAERqsi1 --- ...ephemeral-identity-canonical-scope.test.ts | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/packages/verifier/src/workflow/__tests__/ephemeral-identity-canonical-scope.test.ts b/packages/verifier/src/workflow/__tests__/ephemeral-identity-canonical-scope.test.ts index 7acc85d4d..2b883776d 100644 --- a/packages/verifier/src/workflow/__tests__/ephemeral-identity-canonical-scope.test.ts +++ b/packages/verifier/src/workflow/__tests__/ephemeral-identity-canonical-scope.test.ts @@ -458,3 +458,50 @@ describe("deriveSessionKey emits the same canonical scope", () => { expect(result.valid).toBe(true); }); }); + +// astra pack 145, F1 (MEDIUM): validation and canonicalization must consume the SAME +// values. A direct service caller (JSON cannot carry these) can hand an array whose +// iterator, or an index accessor, yields something other than what the index check saw. +describe("validation and canonicalization read each element exactly once (astra pack 145 F1)", () => { + it("[neg] an array whose iterator yields an unvalidated value is refused, never signed", () => { + const contractIds = ["c1"]; + (contractIds as unknown as { [Symbol.iterator]: () => Iterator })[Symbol.iterator] = function* () { + yield 42 as unknown as string; + }; + expect(() => issue({ contractIds })).toThrow(); + }); + + it("[neg] an accessor that returns a string on its first read and a number afterwards is refused or canonicalized from the first read", () => { + const contractIds: string[] = []; + let reads = 0; + Object.defineProperty(contractIds, 0, { + enumerable: true, + configurable: true, + get() { + reads += 1; + return reads === 1 ? "c1" : (7 as unknown as string); + }, + }); + contractIds.length = 1; + let emitted: string[] | undefined; + try { + emitted = issue({ contractIds }).sessionKey.scope.contractIds; + } catch { + emitted = undefined; // refusing is also acceptable + } + if (emitted !== undefined) { + expect(emitted).toEqual(["c1"]); // only the validated first read may be signed + expect(emitted.every((x) => typeof x === "string")).toBe(true); + } + expect(reads).toBe(1); // each element is read exactly once + }); + + it("[neg] the same holds for allowedActions (the shared helper also backs deriveSessionKey)", () => { + const allowedActions = ["evidence_submit"]; + (allowedActions as unknown as { [Symbol.iterator]: () => Iterator })[Symbol.iterator] = function* () { + yield { not: "a string" } as unknown as string; + }; + expect(() => issue({ allowedActions: allowedActions as never })).toThrow(); + }); +}); + From a762445101224bb8e332c880140e2361bb78d05e Mon Sep 17 00:00:00 2001 From: globalmysterysnailrevolution Date: Thu, 1 Oct 2026 11:58:43 -0700 Subject: [PATCH 5/5] fix(verifier): canonical scope lists validate and sign the same snapshot (astra pack 145 F1) canonicalScopeList now reads each indexed element exactly once into a local, validates it, and copies it into a fresh snapshot; only the snapshot is de-duplicated and sorted, so what is signed is exactly what was validated. An array carrying its own Symbol.iterator is refused outright (defense in depth). Accessors that change between reads can no longer smuggle an unvalidated element into the signed scope. The canonicalizer and verification are unchanged. Scope tests 42/42 (3 failed at 7528bb13). Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01GihjSLwaMhkBW2DAERqsi1 --- .../src/workflow/ephemeral-identity.ts | 24 +++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/packages/verifier/src/workflow/ephemeral-identity.ts b/packages/verifier/src/workflow/ephemeral-identity.ts index 5ff59d122..9ed57056e 100644 --- a/packages/verifier/src/workflow/ephemeral-identity.ts +++ b/packages/verifier/src/workflow/ephemeral-identity.ts @@ -72,12 +72,28 @@ function canonicalScopeList(field: string, value: unknown): string[] { throw new Error(`${field} must be an array of strings, got ${describeType(value)}`); } // Index loop, not forEach: a sparse array's holes must be seen as undefined. - for (let i = 0; i < value.length; i++) { - if (typeof value[i] !== "string") { - throw new Error(`${field}[${i}] must be a string, got ${describeType(value[i])}`); + // Each element is read EXACTLY ONCE into a local, validated, and copied into a + // fresh snapshot; only the snapshot is de-duplicated and sorted. Validating + // value[i] but then consuming `new Set(value)` would read the caller's array a + // second time through its iterator, so a custom Symbol.iterator or an accessor + // that changes between reads could slip an unvalidated element past the check + // and into the signed scope (astra pack 145, F1). + // An array with its own iterator is not a plain JSON-shaped list. Refuse it rather + // than guess which of its two "contents" the caller meant (defense in depth; the + // snapshot below already ensures only validated reads are signed). + if (Object.prototype.hasOwnProperty.call(value, Symbol.iterator)) { + throw new Error(`${field} must be a plain array of strings (a custom iterator is not accepted)`); + } + const length = value.length; + const snapshot: string[] = []; + for (let i = 0; i < length; i++) { + const element: unknown = value[i]; + if (typeof element !== "string") { + throw new Error(`${field}[${i}] must be a string, got ${describeType(element)}`); } + snapshot.push(element); } - return [...new Set(value as string[])].sort(); + return [...new Set(snapshot)].sort(); } /**