mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-05 16:34:00 +01:00
fix(redact): env.value has no path exemption and matches the value in normalized form
Second adversarial pass: the existence check let the scanned agent exempt any "/"- or "~/"-prefixed secret by creating a path of that name, a silent bypass of the one guard this branch adds, so the exemption is gone; a path-valued SSL_KEY that must appear in a text goes through the allowlist. The engine is back to no I/O. The value is now normalized the way the text is before indexOf, so a credential NFKC rewrites (fullwidth digits) still finds its leaked ASCII form. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
93b793c6ef
commit
7765db84dc
4 files changed
+45
-47
No files matched your search
+3
-3
@@ -5,8 +5,8 @@
|
||||
**The launch environment reaches every gstack command and child through a sourceable file, and nothing in a skill fence may print the environment (GSTA-104).** During GSTA-96's unattended /ship on #3425 (2026-10-26), the Step 14.5 docs-audit child was told to export the `GSTACK_LAUNCH` assignments and re-typed them as `export $(...)`. The substitution came back empty, the shell ran a bare `export`, and the values of ANTHROPIC_API_KEY, OPENAI_API_KEY and UBICLOUD_API_KEY landed in its tool output. The line itself carries no secret (PATH, GSTACK_SESSION_KIND, GSTACK_STATE_ROOT, GSTACK_EPHEMERAL, GSTACK_CAPY, GSTACK_INSTALL_VERDICT); the hazard was re-typing it. Now the installer writes the assignments as a file callers source, every fence that hands the launch environment to a child says so in one line, a free ratchet keeps environment dumps out of every skill fence and shell helper, and the redaction engine refuses a credential value from the writer's own environment, whatever its shape.
|
||||
|
||||
### 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 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`.
|
||||
- **`GSTACK_LAUNCH_FILE: <verdict file without .json>.env`** after the `GSTACK_LAUNCH` and `GSTACK_LAUNCH_PATH` lines of every passing `bin/gstack-capy-install` run, `--check` included (a `gstack: fail` run writes and announces no launch file). 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; names carrying a `PUBLIC` word and gstack's own `GSTACK_BRAIN_LOCK_TOKEN` are exempt, and there is no path exemption) 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. The value is matched in the same normalized form as the text, so a value NFKC rewrites still finds its leaked bytes; 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 existing-file 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 and non-secret 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 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.
|
||||
secret) (`env.value`, HIGH), so a leaked value cannot reach a PR body, receipt,
|
||||
report or Drive file. There is no path exemption: a path-valued `SSL_KEY` that
|
||||
must appear in a text goes through the allowlist.
|
||||
|
||||
`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
|
||||
@@ -362,8 +362,8 @@ directory holding only `bun`, `bunx` and, when needed, `node`, `npm`, `npx` and
|
||||
never puts that directory's other tools (the image's node) in front of PATH.
|
||||
The pregate's lanes launch through the same directory. A child shell's `export` cannot
|
||||
configure the parent's later commands, so the parent supplies these
|
||||
assignments to every gstack command it launches (prefix the command, or
|
||||
export them once in the shell that runs it). A fresh shell without them keeps
|
||||
assignments to every gstack command it launches by sourcing the launch file
|
||||
(`. <GSTACK_LAUNCH_FILE>`) at the start of the shell that runs it. A fresh shell without them keeps
|
||||
reporting the Bun it would really run: `FAIL runtime — bun 1.3.14 at
|
||||
/usr/local/bin/bun ...; verified bun 1.4.2 found at ~/.bun/bin (not on PATH);
|
||||
fix: export PATH="$HOME/.bun/bin:$PATH" or prefix commands with the
|
||||
|
||||
+17
-16
@@ -1,12 +1,10 @@
|
||||
/**
|
||||
* redact-engine — pure scanning + auto-redaction over the shared taxonomy.
|
||||
*
|
||||
* 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.
|
||||
* 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.
|
||||
*
|
||||
* Key behaviors (locked in /plan-eng-review + two Codex passes):
|
||||
* - Normalization BEFORE matching (NFKC + strip zero-width + decode a small
|
||||
@@ -25,8 +23,6 @@
|
||||
* 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,
|
||||
@@ -479,8 +475,11 @@ function scanInternal(input: string, opts: ScanOptions, spans?: Map<Finding, Ori
|
||||
|
||||
// Environment values, by exact value: a credential that is already in the
|
||||
// process environment needs no shape to be recognized.
|
||||
for (const { name, value } of credentialEnvValues(opts.processEnv ?? process.env)) {
|
||||
if (allow.has(value)) continue;
|
||||
for (const { name, value: raw } of credentialEnvValues(opts.processEnv ?? process.env)) {
|
||||
if (allow.has(raw)) continue;
|
||||
// The text was normalized before matching; the value takes the same form.
|
||||
const value = normalizeOriginal(raw).normalized;
|
||||
if (value.length < MIN_ENV_VALUE_LENGTH) continue;
|
||||
let from = 0;
|
||||
let normOffset: number;
|
||||
while ((normOffset = normalized.indexOf(value, from)) !== -1) {
|
||||
@@ -535,12 +534,14 @@ 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 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
|
||||
* be a credential and is not a placeholder. A short or placeholder value would
|
||||
* match ordinary prose. There is no path exemption on purpose: a shape rule
|
||||
* exempted base64 secrets that start with `/`, and an existence check let the
|
||||
* scanned agent exempt a value by creating a path of that name; a path-valued
|
||||
* SSL_KEY that must appear in a text goes through the allowlist. The value is
|
||||
* trimmed, so a key read from a file with a trailing newline still matches its
|
||||
* leaked form.
|
||||
* leaked form; it is matched against the normalized text in its own normalized
|
||||
* form, so a value NFKC rewrites still finds its leaked bytes.
|
||||
*/
|
||||
export function credentialEnvValues(env: Record<string, string | undefined>): Array<{ name: string; value: string }> {
|
||||
const out: Array<{ name: string; value: string }> = [];
|
||||
@@ -548,7 +549,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) || (/^[~/]|^[A-Za-z]:\\/.test(value) && fs.existsSync(value.replace(/^~(?=\/)/, os.homedir())))) continue;
|
||||
if (isPlaceholderSpan(value)) continue;
|
||||
out.push({ name, value });
|
||||
}
|
||||
return out;
|
||||
|
||||
@@ -10,7 +10,6 @@
|
||||
*/
|
||||
import { describe, expect, test } from "bun:test";
|
||||
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";
|
||||
|
||||
@@ -28,6 +27,7 @@ const monkeyValue = "banana-banana-banana";
|
||||
const tokenizerValue = "tokenizer-value-here";
|
||||
const lockOwner = ["12345", "6789", "17"].join("-");
|
||||
const slashSecret = "/Qm9vZ2llV29vZ2ll/abc123XYZ+=";
|
||||
const sslKeyPath = "/etc/ssl/private/x.key";
|
||||
|
||||
const env = {
|
||||
ANTHROPIC_API_KEY: anthropicValue,
|
||||
@@ -146,31 +146,28 @@ 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) 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
|
||||
// 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
|
||||
// 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. 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 });
|
||||
}
|
||||
// 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.
|
||||
// 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.
|
||||
test("more credential suffixes, no path exemption, gstack's own lock token and a normalized value", () => {
|
||||
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,
|
||||
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"]);
|
||||
expect(scan(`b64 ${slashSecret}`, { processEnv: { SLASH_KEY: slashSecret } }).findings.map(f => f.id)).toEqual(["env.value"]);
|
||||
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(scan(`leak: ${anthropicValue} end`, { processEnv: { WIDE_KEY: fullwidth } }).findings.map(f => f.id)).toEqual(["env.value"]);
|
||||
});
|
||||
|
||||
test("the default environment is process.env", () => {
|
||||
|
||||
Reference in new issue
Block a user