From 977fbbeec9bd96b8826a69f382e3b56f8ff958b1 Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN Date: Mon, 17 Aug 2026 22:37:13 -0400 Subject: [PATCH] refactor(agent): remove dead code that read as security controls Two blocks looked load-bearing under review and never executed. authorize() took an optional entry and checked kind/label constraints against it -- but no call site ever passed one; the real constraint check is tokenMaySee, after the vault opens. The parameter and branch are gone, and the split is now documented where it matters: a use is consumed at pre-auth even when the entry turns out missing or uncovered, deliberately, so probing entry ids costs the prober their limited uses instead of handing back a free enumeration budget. sinkEnv() deleted FORBIDDEN_ENV keys from an env built off an allowlist that never contains them -- a loop that could suggest the delete, rather than the allowlist, is what keeps the seed out of sink children. Replaced with a prefix guard inside the allowlist loop (nothing SEEDPASS_-prefixed rides along even if a future edit adds one) and a comment stating plainly that the allowlist is the control and this is the backstop. `extra` stays exempt: the exec sink's whole job is injecting SEEDPASS_SECRET. No behaviour change; 149 CLI + 220 core tests pass. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Tkj4rzKBA3uhfgk7kqjsb8 --- js/packages/cli/src/agent.ts | 20 +++++++++----------- js/packages/cli/src/sinks.ts | 26 +++++++++----------------- 2 files changed, 18 insertions(+), 28 deletions(-) diff --git a/js/packages/cli/src/agent.ts b/js/packages/cli/src/agent.ts index f3d35416..2ccec150 100644 --- a/js/packages/cli/src/agent.ts +++ b/js/packages/cli/src/agent.ts @@ -215,14 +215,20 @@ export class AgentDaemon { } /** - * Validate a bearer token for an action against an entry, consuming one - * use on success. Returns the token record or a denial reason. + * Validate a bearer token for an action, consuming one use for + * secret-bearing actions. Returns the token record or a denial reason. + * + * Entry-level constraints (kinds, label regex) are NOT checked here — that + * is tokenMaySee's job, after the vault is opened. A use is therefore + * consumed even when the entry turns out to be missing or outside the + * token's constraints. Deliberate: charging for the attempt means probing + * entry ids costs the prober their limited uses, and refunding denied + * attempts would hand back a free enumeration budget. */ private authorize( tokenSecret: string, fingerprint: string, action: TokenScope, - entry?: { kind: string; label: string }, ): { token?: TokenRecord; deny?: string } { const hash = sha256Hex(utf8(tokenSecret)); const token = [...this.tokens.values()].find( @@ -233,14 +239,6 @@ export class AgentDaemon { if (token.expires_at <= Date.now() / 1000) return { deny: "token expired" }; if (token.uses_remaining <= 0) return { deny: "token exhausted" }; if (!token.scopes.includes(action)) return { deny: `scope '${action}' not granted` }; - if (entry) { - if (token.kinds && !token.kinds.includes(entry.kind)) { - return { deny: `kind '${entry.kind}' not granted` }; - } - if (!new RegExp(token.label_regex).test(entry.label)) { - return { deny: "label not matched by token constraint" }; - } - } // "read" is not consumption; secret-bearing actions decrement uses if (action !== "read") token.uses_remaining -= 1; return { token }; diff --git a/js/packages/cli/src/sinks.ts b/js/packages/cli/src/sinks.ts index a4f4ac50..e2ce475b 100644 --- a/js/packages/cli/src/sinks.ts +++ b/js/packages/cli/src/sinks.ts @@ -17,35 +17,27 @@ export interface SinkResult { export const EXEC_ENV_VAR = "SEEDPASS_SECRET"; /** - * Variables that must never reach a sink child. - * - * A helper invoked to receive one password would otherwise inherit the - * parent seed, the master password, a bearer token and the agent socket - * path — everything needed to take the whole vault. + * Variables a child plausibly needs to run at all. This allowlist is the + * control that keeps SEEDPASS_MNEMONIC, SEEDPASS_PASSWORD, tokens and the + * agent socket path out of sink children — a helper invoked to receive one + * password must not inherit everything needed to take the whole vault. */ -const FORBIDDEN_ENV = [ - "SEEDPASS_MNEMONIC", - "SEEDPASS_PASSWORD", - "SEEDPASS_TOKEN", - "SEEDPASS_AGENT_SOCK", - "SEEDPASS_AGENT_CAP", - "SEEDPASS_APP_DIR", -]; - -/** Variables a child plausibly needs to run at all. */ const PASSTHROUGH_ENV = ["PATH", "HOME", "LANG", "LC_ALL", "TERM", "TZ", "TMPDIR", "DISPLAY", "WAYLAND_DISPLAY", "XAUTHORITY"]; /** - * Build the environment for a sink child: a minimal allowlist, never the + * Build the environment for a sink child: the allowlist above, never the * caller's full environment. */ export function sinkEnv(extra: Record = {}): Record { const env: Record = {}; for (const key of PASSTHROUGH_ENV) { + // Backstop, not the control: nothing SEEDPASS-prefixed rides along even + // if a future edit adds one to the allowlist. `extra` is exempt — the + // exec sink's whole job is injecting SEEDPASS_SECRET. + if (key.startsWith("SEEDPASS_")) continue; const value = process.env[key]; if (value !== undefined) env[key] = value; } - for (const key of FORBIDDEN_ENV) delete env[key]; return { ...env, ...extra }; }