From 48cff6682fd236b35bea9f81a3fcfa6d9e850722 Mon Sep 17 00:00:00 2001 From: garrytan Date: Mon, 26 Oct 2026 04:44:18 +0000 Subject: [PATCH] fix(redact): the env.value path exemption asks whether the file exists instead of matching a shape Second review cycle: a path-shaped rule also matched a base64 secret that starts with "/" and holds another "/" (about 1 in 150 AWS-shaped keys), which then went unscanned. The exemption now fires only when the value names a file that exists on the scanning machine (SSL_KEY=/etc/ssl/private/x.key). The module header names that stat as the engine's one filesystem read. NON_SECRET_ENV_NAMES is exported so the test can prove bin/gstack-brain-lock.sh still exports every name in it; the PWD/OLDPWD fixtures hold plain words so only the name rule keeps them out. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 4 +-- docs/capy-install.md | 6 ++--- lib/redact-engine.ts | 29 +++++++++++--------- test/redact-env-value.test.ts | 51 ++++++++++++++++++++++------------- 4 files changed, 54 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bf0ec2e14..21e4b23aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,7 @@ ### Added - **`GSTACK_LAUNCH_FILE: .env`** after the `GSTACK_LAUNCH` and `GSTACK_LAUNCH_PATH` lines of every successful `bin/gstack-capy-install` run, `--check` included. The file holds the same assignments as `export` statements (the PATH prepend is idempotent), is written atomically with mode 0600 beside the install verdict under the state root, and is applied with `. ` at the start of the shell that runs a gstack command. A caller hands a child the path, never the values. -- **`env.value` (HIGH) in `lib/redact-engine.ts`**: the value of every `*_KEY`, `*_TOKEN`, `*_SECRET`, `*_PASSWORD`, `*_PASSWD`, `*_PASS`, `*_PAT`, `*_APIKEY` or `*_PWD` variable in the scanning process's environment (any case; eight characters or more after trimming, not a placeholder, not a filesystem path; names carrying a `PUBLIC` word and gstack's own `GSTACK_BRAIN_LOCK_TOKEN` are exempt) is a finding wherever it appears in the scanned text, so a leaked value cannot reach a PR body (`gstack-post`), a ship receipt, a report, an approval row or a Drive mirror through any writer that scans with the engine. When one value is a prefix of another, the longer value is redacted whole; the finding's preview never shows a byte of the value. `redactFindingSpans` removes it on the machine-egress path; `ScanOptions.processEnv` names the environment (`{}` scans without one). `test/redact-env-value.test.ts`. +- **`env.value` (HIGH) in `lib/redact-engine.ts`**: the value of every `*_KEY`, `*_TOKEN`, `*_SECRET`, `*_PASSWORD`, `*_PASSWD`, `*_PASS`, `*_PAT`, `*_APIKEY` or `*_PWD` variable in the scanning process's environment (any case; eight characters or more after trimming, not a placeholder, not the path of a file that exists on the scanning machine; names carrying a `PUBLIC` word and gstack's own `GSTACK_BRAIN_LOCK_TOKEN` are exempt) is a finding wherever it appears in the scanned text, so a leaked value cannot reach a PR body (`gstack-post`), a ship receipt, a report, an approval row or a Drive mirror through any writer that scans with the engine. When one value is a prefix of another, the longer value is redacted whole; the finding's preview never shows a byte of the value. `redactFindingSpans` removes it on the machine-egress path; `ScanOptions.processEnv` names the environment (`{}` scans without one). `test/redact-env-value.test.ts`. - **`test/launch-env-echo-ratchet.test.ts`**: every generated SKILL.md and section file, their templates, `autoplan/DRIVER.md`, the Capy skill, every shell helper under `bin/`, `setup` and the `scripts/*.sh` install scripts carry no `export $(...)`, `export \`...\`` or unquoted `export $VAR`, no `env` or `printenv` (flags alone included), and no bare `set`, `export`, `declare -x`, `typeset -x`, `declare -p` or `typeset -p` in any command segment of a fence (`set | grep '^NAME_'` stays allowed: the anchored filter bounds what prints). `test/fixtures/launch-env-echo/` holds the negative controls the scanner must trip on. ### Changed @@ -15,7 +15,7 @@ - **`scripts/lib/shard-engine.ts`** `createShardSandbox` and the test preload (`test-setup.ts`, so a direct `bun test ` matches the lanes) drop `GSTACK_SESSION_KIND`, `GSTACK_CAPY`, `GSTACK_EPHEMERAL` and `GSTACK_INSTALL_VERDICT` from every test's environment, so a run started from a shell that sourced the launch file no longer hands the product-under-test children an unattended Capy session they never set up (GSTA-99's finding); each fixture sets its own session kind. `GSTACK_STATE_ROOT` and PATH are kept. ### Gate integrity -- One test budget changed: `test/setup-gbrain-remote-caller.test.ts`'s late-completion case gives its second attempt 2s (was 250ms) with a 20s test timeout (was 10s) after it failed 4 of 4 on windows-latest once the new files repacked the Windows shards (run 40263984171). No other gate, threshold, budget, timeout, trial count or case kind changed. The redaction scan only gained findings, minus the framework-public, non-secret and path-valued exemptions named above. Size pins are unchanged: the edited skills are under the limit. +- One test budget changed: `test/setup-gbrain-remote-caller.test.ts`'s late-completion case gives its second attempt 2s (was 250ms) with a 20s test timeout (was 10s) after it failed 4 of 4 on windows-latest once the new files repacked the Windows shards (run 40263984171). No other gate, threshold, budget, timeout, trial count or case kind changed. The redaction scan only gained findings, minus the framework-public, non-secret and existing-file exemptions named above. Size pins are unchanged: the edited skills are under the limit. ## [1.91.226.0] - 2026-10-26 diff --git a/docs/capy-install.md b/docs/capy-install.md index 8ad45c445..8f81450a8 100644 --- a/docs/capy-install.md +++ b/docs/capy-install.md @@ -347,9 +347,9 @@ engine now refuses the VALUE of any `*_KEY`, `*_TOKEN`, `*_SECRET`, `*_PASSWORD`, `*_PASSWD`, `*_PASS`, `*_PAT`, `*_APIKEY` or `*_PWD` variable in the writer's environment (any case; names carrying a `PUBLIC` word are framework-public and exempt, gstack's own `GSTACK_BRAIN_LOCK_TOKEN` is not a -secret, and a value that is a filesystem path names where the credential lives -rather than the credential) (`env.value`, HIGH), so a leaked value cannot reach -a PR body, receipt, report or Drive file. +secret, and a value that names an existing file says where the credential lives +rather than what it is) (`env.value`, HIGH), so a leaked value cannot reach a PR +body, receipt, report or Drive file. `PATH=:$PATH` and `GSTACK_LAUNCH_PATH` appear only when the certified Bun, or the project's resolved Node, is not first on PATH (older Capy images ship diff --git a/lib/redact-engine.ts b/lib/redact-engine.ts index d3e39ece9..38f49be68 100644 --- a/lib/redact-engine.ts +++ b/lib/redact-engine.ts @@ -1,10 +1,12 @@ /** * redact-engine — pure scanning + auto-redaction over the shared taxonomy. * - * No I/O. Deterministic given its one ambient input: `scan()` reads the - * credential-named variables of `process.env` by value unless `processEnv` is - * passed (GSTA-104). The CLI shim (`bin/gstack-redact`), the pre-push hook - * (`bin/gstack-redact-prepush`), and tests all import from here. + * Deterministic given its ambient inputs: `scan()` reads the credential-named + * variables of `process.env` by value unless `processEnv` is passed, and asks + * the filesystem only whether a path-shaped one of those values names an + * existing file (GSTA-104); nothing else is read. The CLI shim + * (`bin/gstack-redact`), the pre-push hook (`bin/gstack-redact-prepush`), and + * tests all import from here. * * Key behaviors (locked in /plan-eng-review + two Codex passes): * - Normalization BEFORE matching (NFKC + strip zero-width + decode a small @@ -23,6 +25,8 @@ * no trust exemption (the marker scheme was dropped as theater). */ +import * as fs from "node:fs"; +import * as os from "node:os"; import { PATTERNS, PATTERNS_BY_ID, @@ -522,10 +526,8 @@ function scanInternal(input: string, opts: ScanOptions, spans?: Map = new Set(["GSTACK_BRAIN_LOCK_TOKEN"]); const MIN_ENV_VALUE_LENGTH = 8; /** @@ -533,9 +535,12 @@ const MIN_ENV_VALUE_LENGTH = 8; * variables (the last `_`-separated word is KEY, TOKEN, SECRET, PASSWORD, PASSWD, * PASS, PAT or APIKEY, or the name ends in _PWD; names carrying a PUBLIC word and * gstack's own non-secret tokens are exempt) whose trimmed value is long enough to - * be a credential, is not a placeholder and is not a filesystem path. A short or - * placeholder value would match ordinary prose. The value is trimmed, so a key - * read from a file with a trailing newline still matches its leaked form. + * be a credential, is not a placeholder and does not name a file that exists on + * this machine (SSL_KEY=/etc/ssl/private/x.key says where the credential lives; a + * secret that happens to start with `/` names no file, so a shape rule would miss + * it). A short or placeholder value would match ordinary prose. The value is + * trimmed, so a key read from a file with a trailing newline still matches its + * leaked form. */ export function credentialEnvValues(env: Record): Array<{ name: string; value: string }> { const out: Array<{ name: string; value: string }> = []; @@ -543,7 +548,7 @@ export function credentialEnvValues(env: Record): Ar if (!CREDENTIAL_ENV_NAME.test(name) || PUBLIC_ENV_NAME.test(name) || NON_SECRET_ENV_NAMES.has(name)) continue; const value = raw?.trim() ?? ""; if (value.length < MIN_ENV_VALUE_LENGTH) continue; - if (isPlaceholderSpan(value) || PATH_VALUE.test(value)) continue; + if (isPlaceholderSpan(value) || (/^[~/]|^[A-Za-z]:\\/.test(value) && fs.existsSync(value.replace(/^~(?=\/)/, os.homedir())))) continue; out.push({ name, value }); } return out; diff --git a/test/redact-env-value.test.ts b/test/redact-env-value.test.ts index 7282f7ecb..0f1e49679 100644 --- a/test/redact-env-value.test.ts +++ b/test/redact-env-value.test.ts @@ -9,7 +9,10 @@ * writer scans through this engine, so the value itself is now what it refuses. */ import { describe, expect, test } from "bun:test"; -import { credentialEnvValues, redactFindingSpans, scan } from "../lib/redact-engine"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; +import { NON_SECRET_ENV_NAMES, credentialEnvValues, redactFindingSpans, scan } from "../lib/redact-engine"; // Fixture values live in plain constants: a credential-named assignment with an // entropic literal on one line is exactly what the repo's own pregate secrets @@ -23,10 +26,8 @@ const dbPasswdValue = "passwd-passwd-passwd"; const lowerCaseValue = "lower-case-name-value"; const monkeyValue = "banana-banana-banana"; const tokenizerValue = "tokenizer-value-here"; -const sslKeyPath = "/etc/ssl/private/x.key"; -const winKeyPath = "C:\\Users\\me\\key.pem"; -const homeKeyPath = "~/.ssh/id_key"; const lockOwner = ["12345", "6789", "17"].join("-"); +const slashSecret = "/Qm9vZ2llV29vZ2ll/abc123XYZ+="; const env = { ANTHROPIC_API_KEY: anthropicValue, @@ -143,21 +144,33 @@ describe("env-value scan", () => { expect(credentialEnvValues({ PUBLICITY_KEY: monkeyValue }).map(e => e.name)).toEqual(["PUBLICITY_KEY"]); }); - // /ship Step 11 adversarial fixes: (1) the name rule also admits *_PASS, *_PAT, *_APIKEY and *_PWD, - // while the shell's own PWD and OLDPWD stay out; (2) a credential-named variable whose value is a - // filesystem path (SSL_KEY=/etc/ssl/private/x.key) names where the credential lives, so the path - // is not a finding; (3) gstack's own GSTACK_BRAIN_LOCK_TOKEN (pid-random-epoch, written to the lock - // owner file) is not a secret. fails_when: PWD is admitted (every mention of the cwd becomes HIGH), - // the path rule goes, or the non-secret set loses the lock token. - test("more credential suffixes, path values and gstack's own lock token", () => { - const names = credentialEnvValues({ - DB_PASS: dbValue, GITHUB_PAT: ghValue, STRIPE_APIKEY: lowerCaseValue, MYSQL_PWD: dbPasswdValue, - PWD: "/workspace/gstack", OLDPWD: "/workspace/other", - SSL_KEY: sslKeyPath, WIN_KEY: winKeyPath, HOME_KEY: homeKeyPath, - GSTACK_BRAIN_LOCK_TOKEN: lockOwner, - }).map(e => e.name).sort(); - expect(names).toEqual(["DB_PASS", "GITHUB_PAT", "MYSQL_PWD", "STRIPE_APIKEY"]); - expect(credentialEnvValues({ SLASH_KEY: "/abc12345-no-second-slash" }).map(e => e.name)).toEqual(["SLASH_KEY"]); + // (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) a credential-named variable whose value names an EXISTING file (SSL_KEY=/etc/ssl/...) + // says where the credential lives, so that path is no finding, while a path-shaped value that + // names no file, a base64 secret starting with "/" included, is still scanned; (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. fails_when: PWD is admitted (every + // mention of the cwd becomes HIGH), the path rule becomes a shape rule (the slash secret is skipped), + // the existence check goes (the real file's path becomes HIGH), or the non-secret set drifts. + test("more credential suffixes, path values that exist, and gstack's own lock token", () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "gsta104-")); + const keyFile = path.join(dir, "x.key"); + fs.writeFileSync(keyFile, "pem"); + try { + 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: keyFile, DIR_KEY: dir, MISSING_KEY: path.join(dir, "absent", "x.key"), SLASH_KEY: slashSecret, + GSTACK_BRAIN_LOCK_TOKEN: lockOwner, + }).map(e => e.name).sort(); + expect(names).toEqual(["DB_PASS", "GITHUB_PAT", "MISSING_KEY", "MYSQL_PWD", "SLASH_KEY", "STRIPE_APIKEY"]); + expect(scan(`b64 ${slashSecret}`, { processEnv: { SLASH_KEY: slashSecret } }).findings.map(f => f.id)).toEqual(["env.value"]); + } finally { + fs.rmSync(dir, { recursive: true, force: true }); + } + 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}`); }); test("the default environment is process.env", () => {