mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-06 00:44:07 +01:00
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>
This commit is contained in:
1 parent
d4f942492d
commit
3e58c96898
3 files changed
+16
-9
No files matched your search
@@ -476,10 +476,10 @@ 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: raw } of credentialEnvValues(opts.processEnv ?? process.env)) {
|
||||
if (allow.has(raw)) continue;
|
||||
// The text was normalized before matching; the value takes the same form.
|
||||
// The text was normalized before matching; the value takes the same form,
|
||||
// and the allowlist and placeholder rules see both forms.
|
||||
const value = normalizeOriginal(raw).normalized;
|
||||
if (value.length < MIN_ENV_VALUE_LENGTH) continue;
|
||||
if (allow.has(raw) || allow.has(value) || value.length < MIN_ENV_VALUE_LENGTH || isPlaceholderSpan(value)) continue;
|
||||
let from = 0;
|
||||
let normOffset: number;
|
||||
while ((normOffset = normalized.indexOf(value, from)) !== -1) {
|
||||
@@ -538,7 +538,9 @@ const MIN_ENV_VALUE_LENGTH = 8;
|
||||
* 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
|
||||
* SSL_KEY that must appear in a text goes through `allowlist` where a caller
|
||||
* passes one (`gstack-redact`, the pre-push hook), else the variable is set
|
||||
* empty for that one command. The value is
|
||||
* trimmed, so a key read from a file with a trailing newline still matches its
|
||||
* leaked form; it is matched against the normalized text in its own normalized
|
||||
* form, so a value NFKC rewrites still finds its leaked bytes.
|
||||
|
||||
@@ -182,8 +182,9 @@ describe('launch-env echo ratchet', () => {
|
||||
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"');
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
Reference in new issue
Block a user