From 3e58c9689880a3ce517a7ee3e866598cbaf895ef Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 26 Oct 2026 05:07:28 +0000 Subject: [PATCH] fix(redact): the env.value allowlist and placeholder rules see the normalized value too An allowlisted value in NFC did not suppress a match on its NFKC-composed form, and a value that is a placeholder only after normalization was still searched for. Fixture nits from the third review cycle: fixed directory string instead of process.cwd(), the fullwidth assertion guards its own vacuity, a zero-width-padded value under the floor is pinned, and the ratchet comment names the one kept export and the PATH case line. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- lib/redact-engine.ts | 10 ++++++---- test/launch-env-echo-ratchet.test.ts | 5 +++-- test/redact-env-value.test.ts | 10 +++++++--- 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/lib/redact-engine.ts b/lib/redact-engine.ts index e08aa9103..db8eda2e8 100644 --- a/lib/redact-engine.ts +++ b/lib/redact-engine.ts @@ -476,10 +476,10 @@ function scanInternal(input: string, opts: ScanOptions, spans?: Map { expect(installer).toContain('LAUNCH_FILE="${VERDICT%.json}.env"'); expect(installer).toContain('chmod 0600 "$tmp"'); for (const name of CAPY_LAUNCH_MARKERS) expect(installer, name).toMatch(new RegExp(`^\\s*echo "export ${name}=`, 'm')); - // Both directions: every variable the launch file exports is either a marker the shard sandbox - // and the test preload strip, or one of the two they keep on purpose (GSTA-99). + // Both directions: every `export NAME=` line the launch file writes is either a marker the shard + // sandbox and the test preload strip, or GSTACK_STATE_ROOT, which they keep on purpose (GSTA-99). + // The PATH prepend is a separate `case` line this regex ignores; PATH is kept too. const exported = [...installer.matchAll(/^\s*echo "export ([A-Z_]+)=/gm)].map(m => m[1]).sort(); expect(exported).toEqual([...CAPY_LAUNCH_MARKERS, 'GSTACK_STATE_ROOT'].sort()); expect(installer).toContain('echo "GSTACK_LAUNCH_FILE: $LAUNCH_FILE"'); diff --git a/test/redact-env-value.test.ts b/test/redact-env-value.test.ts index 62179f38a..c16b31935 100644 --- a/test/redact-env-value.test.ts +++ b/test/redact-env-value.test.ts @@ -147,11 +147,12 @@ describe("env-value scan", () => { // (1) the name rule also admits *_PASS, *_PAT, *_APIKEY and *_PWD, while the shell's own PWD and // OLDPWD stay out by NAME (their fixture values are plain words, so only the name rule keeps them // out); (2) there is no path exemption: a path-valued SSL_KEY and a base64 secret starting with - // "/" are both scanned, and an existing directory as a value changes nothing (an existence check + // "/" are both scanned, and a directory path as a value changes nothing (an existence check // let the scanned agent exempt a value by creating a path of that name); (3) gstack's own // GSTACK_BRAIN_LOCK_TOKEN (pid-random-epoch, written to the lock owner file) is not a secret, and // bin/gstack-brain-lock.sh still exports every name in that set; (4) a value NFKC rewrites - // (fullwidth digits) is matched in its normalized form, so its leaked ASCII form is found. + // (fullwidth digits) is matched in its normalized form, so its leaked ASCII form is found, and a + // value whose normalized form is under eight characters is not searched for. // fails_when: PWD is admitted (every mention of the cwd becomes HIGH), a shape or existence rule // returns (the slash secret or the directory value is skipped), the non-secret set drifts, or the // value is matched raw against the normalized text. @@ -159,7 +160,7 @@ describe("env-value scan", () => { const names = credentialEnvValues({ DB_PASS: dbValue, GITHUB_PAT: ghValue, STRIPE_APIKEY: lowerCaseValue, MYSQL_PWD: dbPasswdValue, PWD: "workspace-dir-name", OLDPWD: "previous-dir-name", - SSL_KEY: sslKeyPath, DIR_KEY: process.cwd(), SLASH_KEY: slashSecret, + SSL_KEY: sslKeyPath, DIR_KEY: "/usr/local/share/ca-certificates", SLASH_KEY: slashSecret, GSTACK_BRAIN_LOCK_TOKEN: lockOwner, }).map(e => e.name).sort(); expect(names).toEqual(["DB_PASS", "DIR_KEY", "GITHUB_PAT", "MYSQL_PWD", "SLASH_KEY", "SSL_KEY", "STRIPE_APIKEY"]); @@ -167,7 +168,10 @@ describe("env-value scan", () => { const lock = fs.readFileSync(path.join(import.meta.dir, "..", "bin", "gstack-brain-lock.sh"), "utf8"); for (const name of NON_SECRET_ENV_NAMES) expect(lock, name).toContain(`export ${name}`); const fullwidth = anthropicValue.replace(/[0-9]/g, c => String.fromCharCode(c.charCodeAt(0) + 0xfee0)); + expect(fullwidth).not.toBe(anthropicValue); expect(scan(`leak: ${anthropicValue} end`, { processEnv: { WIDE_KEY: fullwidth } }).findings.map(f => f.id)).toEqual(["env.value"]); + const zeroWidthPadded = "ab\u200bcd\u200bef\u200bg"; + expect(scan("plain abcdefg here", { processEnv: { ZW_KEY: zeroWidthPadded } }).findings).toEqual([]); }); test("the default environment is process.env", () => {