mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 16:34:00 +01:00
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>
This commit is contained in:
1 parent
c56f462b6c
commit
48cff6682f
4 files changed
+54
-36
No files matched your search
+2
-2
@@ -6,7 +6,7 @@
|
||||
|
||||
### Added
|
||||
- **`GSTACK_LAUNCH_FILE: <verdict file without .json>.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 `. <file>` 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 <file>` 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
|
||||
|
||||
|
||||
@@ -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=<dir>:$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
|
||||
|
||||
+17
-12
@@ -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<Finding, Ori
|
||||
const CREDENTIAL_ENV_NAME = /(^|_)(KEY|TOKEN|SECRET|PASSWORD|PASSWD|PASS|PAT|APIKEY)$|_PWD$/i;
|
||||
/** Framework-public keys (NEXT_PUBLIC_*_KEY, VITE_PUBLIC_*, *_PUBLIC_KEY) are committed on purpose. */
|
||||
const PUBLIC_ENV_NAME = /(^|_)PUBLIC(_|$)/i;
|
||||
/** gstack's own credential-named variables whose values are not secrets. */
|
||||
const NON_SECRET_ENV_NAMES = new Set(["GSTACK_BRAIN_LOCK_TOKEN"]);
|
||||
/** A credential-named variable holding a filesystem path names where the credential lives, not the credential. */
|
||||
const PATH_VALUE = /^(~?\/[^\s]*\/[^\s]*|[A-Za-z]:\\[^\s]*)$/;
|
||||
/** gstack's own credential-named variables whose values are not secrets (bin/gstack-brain-lock.sh sets the lock token). */
|
||||
export const NON_SECRET_ENV_NAMES: ReadonlySet<string> = 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<string, string | undefined>): Array<{ name: string; value: string }> {
|
||||
const out: Array<{ name: string; value: string }> = [];
|
||||
@@ -543,7 +548,7 @@ export function credentialEnvValues(env: Record<string, string | undefined>): 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;
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
Reference in new issue
Block a user