From 6b862baa71e047d66fc6241acc0139b7d2500d79 Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Fri, 14 Aug 2026 23:24:08 -0400 Subject: [PATCH] security: reject non-string credentials on the agent protocol Found by running attacks against the socket rather than reading the code: a token supplied as a single-element array was accepted, because String(["abc"]) is "abc" and the comparison went through String() coercion. No privilege was gained -- the attacker still needed the real token -- but type coercion on a credential path is the kind of sloppiness that becomes a bug the moment the surrounding code changes. The handler now requires op, cap, token, fingerprint, id and sink to be strings, and command to be an array of strings, rejecting anything else outright. The same run confirmed the rest of the boundary behaves: by-id access to entries excluded by a token's label regex or kind constraint is refused server-side with the correct reason, reveal without the reveal scope is refused, denials do not consume uses, concurrent requests cannot overrun a one-use token, and a sink child spawned by the agent sees exactly one SEEDPASS_ variable -- the secret it was given. Co-Authored-By: Claude Fable 5 --- js/packages/cli/src/agent.ts | 15 +++++++++++++++ js/packages/cli/test/agentSecurity.test.ts | 18 ++++++++++++++++++ js/packages/core/.audit1.mts | 9 +++++++++ 3 files changed, 42 insertions(+) create mode 100644 js/packages/core/.audit1.mts diff --git a/js/packages/cli/src/agent.ts b/js/packages/cli/src/agent.ts index 071c03ae..eb171957 100644 --- a/js/packages/cli/src/agent.ts +++ b/js/packages/cli/src/agent.ts @@ -326,6 +326,21 @@ export class AgentDaemon { private async handle(msg: Record): Promise> { this.expire(); + // Reject non-string credentials and identifiers rather than coercing + // them. String(["abc"]) is "abc", so an array would sail through a + // String() comparison; nothing good comes of accepting these shapes. + for (const field of ["op", "cap", "token", "fingerprint", "id", "sink"]) { + const value = msg[field]; + if (value !== undefined && typeof value !== "string") { + return { ok: false, error: `field '${field}' must be a string` }; + } + } + if (msg["command"] !== undefined) { + const command = msg["command"]; + if (!Array.isArray(command) || command.some((c) => typeof c !== "string")) { + return { ok: false, error: "field 'command' must be an array of strings" }; + } + } const op = String(msg["op"] ?? ""); if (OWNER_OPS.has(op) && !this.isOwner(msg)) { // Do not reveal whether the profile exists or is unlocked. diff --git a/js/packages/cli/test/agentSecurity.test.ts b/js/packages/cli/test/agentSecurity.test.ts index cd49cc80..068f5848 100644 --- a/js/packages/cli/test/agentSecurity.test.ts +++ b/js/packages/cli/test/agentSecurity.test.ts @@ -212,6 +212,24 @@ describe("token holders cannot exceed their grant", () => { const r = await rawRequest({ op: "owner-mnemonic", fingerprint: FINGERPRINT, cap: token }); expect(r["ok"]).toBe(false); }); + + it("rejects non-string credentials instead of coercing them", async () => { + // String(["abc"]) === "abc", so an array credential would otherwise pass + // a String()-based comparison unnoticed. + for (const shaped of [[token], { t: token }, 1, true, null]) { + const r = await rawRequest({ op: "vault-index", fingerprint: FINGERPRINT, token: shaped }); + expect(r["ok"], `token as ${JSON.stringify(shaped)} must be refused`).toBe(false); + } + for (const shaped of [["x"], {}, 0]) { + const r = await rawRequest({ op: "owner-mnemonic", fingerprint: FINGERPRINT, cap: shaped }); + expect(r["ok"]).toBe(false); + } + const badCommand = await rawRequest({ + op: "use-sink", fingerprint: FINGERPRINT, id: "0", token, + sink: "exec", command: "not-an-array", + }); + expect(badCommand["ok"]).toBe(false); + }); }); describe("sink children do not inherit the vault's secrets", () => { diff --git a/js/packages/core/.audit1.mts b/js/packages/core/.audit1.mts new file mode 100644 index 00000000..2fb52245 --- /dev/null +++ b/js/packages/core/.audit1.mts @@ -0,0 +1,9 @@ +import { hexToBytes, bytesToHex } from "./src/util/bytes.js"; +const cases = ["1z", "-1", "+1", " 1", "0x", "1 ", "gg", "1\n", "aA", "1e", "Infinity".slice(0,2), "e5", "1_", ".5", "0b"]; +for (const c of cases) { + try { console.log(JSON.stringify(c), "->", bytesToHex(hexToBytes(c))); } + catch (e) { console.log(JSON.stringify(c), "-> THROW", (e as Error).message); } +} +// bech32 roundtrip malleability +import { hexToBech32 } from "./src/derive/nostr.js"; +console.log("npub from 'zz'*32:", (()=>{try{return hexToBech32("zz".repeat(32) as string, "npub")}catch(e){return "throw "+(e as Error).message}})());