From d188005b102d6c5896317ebb61a63e026d3d69de Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN Date: Wed, 19 Aug 2026 23:34:21 -0400 Subject: [PATCH] Refuse unrecognised token scopes and malformed sink commands over the wire MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both checks fail closed if removed, and both fail closed SILENTLY, which is the reason they exist. An unknown scope grants nothing downstream — tokenAllows asks for read, use or reveal by name — so a token issued with `["admin"]` simply never authorises anything. The operator gets a token back, believes they issued it, and finds out when the job using it is denied. The likely cases are near-misses: "Read", "reveal " with a trailing space, "write" by analogy with other systems. Naming the bad scope in the refusal is the whole value. The sink `command` shape check is the same shape of argument. Every malformed command already fails closed, because the allowlist is checked with parseCommandSpec against the SAME array that is later spawned — so the check and the spawn cannot disagree. But failing closed there means a TypeError or a nonsense command word, and this daemon's stated posture is that every caller is untrusted. Refuse the shape while it is still describable. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Vtx94nXDmNgXEeThUZP4QN --- js/packages/cli/test/agentSecurity.test.ts | 50 ++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/js/packages/cli/test/agentSecurity.test.ts b/js/packages/cli/test/agentSecurity.test.ts index 861311fc..176ad094 100644 --- a/js/packages/cli/test/agentSecurity.test.ts +++ b/js/packages/cli/test/agentSecurity.test.ts @@ -622,6 +622,34 @@ describe("token-issue validates before it mints", () => { expect(r["token"]).toBeUndefined(); }); + it("refuses a scope it does not recognise, naming it", async () => { + // A bogus scope grants nothing downstream — tokenAllows asks for read, + // use or reveal by name — so dropping this check fails closed. But it + // fails closed SILENTLY: the operator gets a token back, believes they + // issued it, and only finds out when the job using it is denied. And a + // near-miss is the likely case: "Read", "reveal " with a space, "write" + // by analogy with other systems. + const base = { + op: "token-issue", + cap: ownerCap, + fingerprint: FINGERPRINT, + name: "bad-scope", + ttl: 600, + uses: 1, + }; + for (const scopes of [["admin"], ["read", "write"], ["Read"], ["reveal "]]) { + const r = await rawRequest({ ...base, scopes }); + expect(r["ok"]).toBe(false); + expect(String(r["error"])).toContain("unknown scopes"); + expect(r["token"]).toBeUndefined(); + } + + // All three real scopes together are fine, so this rejects the unknown + // ones rather than anything about issuing a multi-scope token. + const good = await rawRequest({ ...base, scopes: ["read", "use", "reveal"] }); + expect(good["ok"]).toBe(true); + }); + it("refuses a bare string where an array is required", async () => { // `"read"` and `["read"]` are easy to confuse when hand-writing the // protocol, and the difference is not cosmetic: a string `kinds` turns @@ -662,6 +690,28 @@ describe("token-issue validates before it mints", () => { }); }); +describe("the wire protocol refuses shapes, not just values", () => { + it("insists a sink command is an array of strings", async () => { + // The exec allowlist is checked with parseCommandSpec against the SAME + // array that is later spawned, so a wrong shape cannot cause the check + // and the spawn to disagree — every malformed shape fails closed. But + // "fails closed" here means a TypeError or a nonsense command word, not + // a refusal, and this daemon's stated posture is that every caller is + // untrusted. Refuse the shape while it is still describable. + for (const command of ["wc -c", 42, { cmd: "wc" }, ["wc", 5], [null]]) { + const r = await rawRequest({ + op: "use-sink", + fingerprint: FINGERPRINT, + id: "1", + sink: "exec", + command, + }); + expect(r["ok"]).toBe(false); + expect(String(r["error"])).toContain("array of strings"); + } + }); +}); + describe("starting a daemon over an existing socket", () => { it("refuses to replace a LIVE agent", async () => { // Without this check the second daemon deletes the socket and listens in