mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-10 02:30:39 +01:00
* Stop the secrets check flagging code that reads an environment variable (GST-1894) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Keep the new secrets test rows out of the line-start credential shape Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Give the fixture-label exemption only to files named like tests A production file such as ab-test.ts or load-test.ts matched the test-file pattern, so a weak real password in it stopped being flagged. The pattern now matches only the documented forms: *.test.*, *.spec.*, *_test.* and test_*.py. Review-Finding: p1/red-team#1 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Keep flagging a hardcoded fallback in backticks or on the next line The env-read exemption only looked for a quoted fallback on the same line, so token = opts?.env?.API_TOKEN ?? followed by a backtick literal, or by a literal wrapped onto the next line, passed the secrets check. Backtick literals now count, and the next line is read when the expression continues there. The changelog now says plainly that an ALL-CAPS literal used as the variable name still passes. Review-Finding: voices/outside-voice#1 reader=outside-voice Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Exempt an env read only when its line has no fallback, and stop reading the next line Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> * Say in the changelog that quoted lowercase env names now fail the secrets check Review-Finding: p1/red-team#1 reader=red-team Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --------- Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
199 lines
13 KiB
TypeScript
199 lines
13 KiB
TypeScript
/**
|
|
* pregate `secrets` scans each file's added lines with that file's type (#3211)
|
|
* and never reads an empty or rewritten diff as clean: a `+++` content line
|
|
* inside a hunk is scanned, and diff.external / textconv drivers are ignored.
|
|
*/
|
|
import { afterEach, describe, expect, test } from 'bun:test';
|
|
import { spawnSync } from 'child_process';
|
|
import * as fs from 'fs';
|
|
import * as os from 'os';
|
|
import * as path from 'path';
|
|
import { parseAddedLineNumbers, parseAddedLines } from '../lib/pregate/checks/secrets';
|
|
import { cleanup, commit, makeFixtureRepo, runBin, BASE_PREGATE, type FixtureRepo } from './helpers/pregate-fixture';
|
|
|
|
const PREGATE = path.resolve(import.meta.dir, '..', 'bin', 'gstack-pregate');
|
|
const AWS = 'AKIA' + 'ZQ3XN7P2LM4RT8WV';
|
|
const roots: string[] = [];
|
|
afterEach(() => { for (const r of roots.splice(0)) fs.rmSync(r, { recursive: true, force: true }); });
|
|
|
|
function repo(files: Record<string, string>) {
|
|
const root = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'pregate-secrets-')));
|
|
roots.push(root);
|
|
const dir = path.join(root, 'work');
|
|
const env = { ...process.env, HOME: root, GIT_CONFIG_GLOBAL: '/dev/null', GIT_CONFIG_NOSYSTEM: '1' };
|
|
const git = (...args: string[]) => {
|
|
const r = spawnSync('git', args, { cwd: dir, encoding: 'utf8', env, timeout: 30_000 });
|
|
if (r.status !== 0) throw new Error(`git ${args.join(' ')}: ${r.stderr}`);
|
|
};
|
|
fs.mkdirSync(dir);
|
|
git('init', '-q', '-b', 'main');
|
|
git('config', 'user.email', 'fixture@example.test');
|
|
git('config', 'user.name', 'Fixture');
|
|
fs.writeFileSync(path.join(dir, 'README'), 'base\n');
|
|
git('add', '.');
|
|
git('commit', '-qm', 'base');
|
|
git('init', '-q', '--bare', path.join(root, 'remote.git'));
|
|
git('remote', 'add', 'origin', path.join(root, 'remote.git'));
|
|
git('push', '-q', 'origin', 'main');
|
|
git('fetch', '-q', 'origin');
|
|
git('checkout', '-qb', 'feat');
|
|
for (const [rel, text] of Object.entries(files)) fs.writeFileSync(path.join(dir, rel), text);
|
|
git('add', '.');
|
|
git('commit', '-qm', 'feat');
|
|
const secrets = () => {
|
|
const r = spawnSync(PREGATE, ['--base', 'main', '--only', 'secrets', '--json'], { cwd: dir, encoding: 'utf8', env, timeout: 120_000 });
|
|
const check = JSON.parse(r.stdout).checks.find((c: { id: string }) => c.id === 'secrets');
|
|
return { status: check.status as string, detail: check.detail as string, lines: (check.lines ?? []) as string[] };
|
|
};
|
|
return { dir, git, secrets };
|
|
}
|
|
|
|
describe('pregate secrets: source context and diff integrity', () => {
|
|
test('parseAddedLines groups by file and keeps a "+++" content line inside a hunk', () => {
|
|
const diff = ['diff --git a/a.ts b/a.ts', '--- a/a.ts', '+++ b/a.ts', '@@ -0,0 +1,2 @@', '+one', '+++two', 'diff --git a/b.env b/b.env', '--- /dev/null', '+++ b/b.env', '@@ -0,0 +1 @@', '+three'].join('\n');
|
|
expect([...parseAddedLines(diff)]).toEqual([['a.ts', ['one', '++two']], ['b.env', ['three']]]);
|
|
});
|
|
|
|
test('a Python call that reads key.json is code; the same key name holding a literal key still fails (GST-1064)', () => {
|
|
expect(repo({ 'attach.py': 'import json\nkey = json.loads(files.get("key.json", ""))\n' }).secrets().status).toBe('pass');
|
|
const fake = 'Zx9Q' + 'w8Er7Ty6Ui5Op4As3Df2';
|
|
const r = repo({ 'attach.py': `key = json.loads(files.get("key.json")) or "${fake}"\n`, '.env': `KEY=${fake}\n` }).secrets();
|
|
expect(r.status).toBe('fail');
|
|
expect(r.lines.join('\n')).toContain('attach.py');
|
|
expect(r.lines.join('\n')).toContain('.env');
|
|
});
|
|
|
|
test('a TS object property that reads a credential at runtime is code, not a secret (#3211)', () => {
|
|
const f = repo({ 'app.ts': 'const cfg = {\n token: credentials.token,\n apiKey: settings.apiKey,\n};\n' });
|
|
expect(f.secrets().status).toBe('pass');
|
|
});
|
|
|
|
test('a Python parameter annotation is code, not a secret; a quoted value on the same name still fails (#3630)', () => {
|
|
expect(repo({ 'app.py': 'def fetch_days(\n *,\n session: HttpSession,\n start: int,\n) -> None:\n pass\n' }).secrets().status).toBe('pass');
|
|
const r = repo({ 'app.py': 'SETTINGS = dict(\n session="8Fk2pQ9vXz4wL7mN3rT6yB1cD5eG0hJ",\n)\nCONFIG = {\n "session": "8Fk2pQ9vXz4wL7mN3rT6yB1cD5eG0hJ",\n}\n' }).secrets();
|
|
expect(r.status).toBe('fail');
|
|
expect(r.lines.join('\n')).toContain('app.py, line 5)');
|
|
});
|
|
|
|
test('parseAddedLineNumbers numbers each added line from its hunk header', () => {
|
|
const diff = ['diff --git a/a.py b/a.py', '--- a/a.py', '+++ b/a.py', '@@ -3,0 +4,2 @@', '+one', '+two', '@@ -9 +11 @@', '-old', '+new'].join('\n');
|
|
expect([...parseAddedLineNumbers(diff)]).toEqual([['a.py', [4, 5, 11]]]);
|
|
});
|
|
|
|
test('a line added inside an existing triple-quoted block in a Python or Dart file still fails; a separate hunk keeps its own context (#3630)', () => {
|
|
const token = 'Fk2pQ9vXz4wL7mN3rT6yB1cD5eG0hJx';
|
|
for (const [file, open, close] of [['app.py', 'CONFIG = """', '"""'], ['lib/config.dart', "const config = '''", "''';"]] as const) {
|
|
const base = `def fetch(\n *,\n start: int,\n) -> None:\n pass\n\n${open}\nhost: db\n${close}\n`;
|
|
const fx: FixtureRepo = makeFixtureRepo({ pregate: BASE_PREGATE, baseFiles: { [file]: base } });
|
|
try {
|
|
commit(fx, { [file]: base.replace(' start: int,\n', ' session: HttpSession,\n start: int,\n').replace('host: db\n', `host: db\npassword: ${token}\n`) });
|
|
const r = runBin(fx, 'gstack-pregate', ['--base', 'main', '--only', 'secrets', '--json']);
|
|
const check = JSON.parse(r.out).checks.find((c: { id: string }) => c.id === 'secrets');
|
|
expect({ file, status: check.status }).toEqual({ file, status: 'fail' });
|
|
expect((check.lines as string[]).filter((l) => l.includes('env.kv'))).toEqual([expect.stringContaining(`${file}, line 10)`)]);
|
|
} finally { cleanup(fx); }
|
|
}
|
|
});
|
|
|
|
test('an untracked Python file reads annotations as code and still fails a literal, naming its line (#3630)', () => {
|
|
const token = 'Fk2pQ9vXz4wL7mN3rT6yB1cD5eG0hJx';
|
|
const fx: FixtureRepo = makeFixtureRepo({ pregate: BASE_PREGATE, baseFiles: { 'README.md': 'x\n' } });
|
|
try {
|
|
const secrets = () => JSON.parse(runBin(fx, 'gstack-pregate', ['--base', 'main', '--only', 'secrets', '--json']).out).checks.find((c: { id: string }) => c.id === 'secrets');
|
|
fs.writeFileSync(path.join(fx.work, 'app.py'), 'def f(\n session: HttpSession,\n) -> None:\n pass\n');
|
|
expect(secrets().status).toBe('pass');
|
|
fs.writeFileSync(path.join(fx.work, 'app.py'), `def f(\n session: HttpSession,\n) -> None:\n pass\n\nCONFIG = """\npassword: ${token}\n"""\n`);
|
|
const check = secrets();
|
|
expect({ status: check.status, rows: (check.lines as string[]).filter((l) => l.includes('env.kv')) }).toEqual({ status: 'fail', rows: [expect.stringContaining('app.py, line 7)')] });
|
|
} finally { cleanup(fx); }
|
|
});
|
|
|
|
test('a Python file whose working copy no longer holds the diff\'s added lines is scanned as git diffed it (#3630, GST-738)', () => {
|
|
const token = 'Fk2pQ9vXz4wL7mN3rT6yB1cD5eG0hJx';
|
|
const fx: FixtureRepo = makeFixtureRepo({ pregate: BASE_PREGATE, baseFiles: { 'app.py': 'x = 1\n' } });
|
|
try {
|
|
commit(fx, { 'app.py': `x = 1\nAPI_TOKEN = "${token}"\n` });
|
|
fx.git('update-index', '--assume-unchanged', 'app.py');
|
|
fs.writeFileSync(path.join(fx.work, 'app.py'), 'x = 1\ny = 2\n');
|
|
const check = JSON.parse(runBin(fx, 'gstack-pregate', ['--base', 'main', '--only', 'secrets', '--json']).out).checks.find((c: { id: string }) => c.id === 'secrets');
|
|
expect({ status: check.status, kv: (check.lines as string[]).some((l) => l.includes('env.kv') && l.includes('app.py, line 2')) }).toEqual({ status: 'fail', kv: true });
|
|
} finally { cleanup(fx); }
|
|
});
|
|
|
|
// Value: protects=the Python/Dart exemption reads the text git diffed, never a working copy that differs from it; fails_when=a committed credential inside a """ block passes because the working copy's other lines were edited under assume-unchanged;
|
|
// why_new=GST-738: the added lines matched, so the edited delimiters around them were trusted; seam=git diff; no_claim=a file edited after the scan
|
|
test('a committed credential in a Python string still fails when the working copy rewrites the lines around it (GST-738)', () => {
|
|
const token = 'Fk2pQ9vXz4wL7mN3rT6yB1cD5eG0hJx';
|
|
const fx: FixtureRepo = makeFixtureRepo({ pregate: BASE_PREGATE, baseFiles: { 'app.py': 'x = 1\nCONFIG = """\n"""\n' } });
|
|
try {
|
|
commit(fx, { 'app.py': `x = 1\nCONFIG = """\npassword: ${token}\n"""\n` });
|
|
fx.git('update-index', '--assume-unchanged', 'app.py');
|
|
fs.writeFileSync(path.join(fx.work, 'app.py'), `x = 1\nCONFIG = 1 #\npassword: ${token}\n#\n`);
|
|
const check = JSON.parse(runBin(fx, 'gstack-pregate', ['--base', 'main', '--only', 'secrets', '--json']).out).checks.find((c: { id: string }) => c.id === 'secrets');
|
|
expect({ status: check.status, kv: ((check.lines ?? []) as string[]).some((l) => l.includes('env.kv') && l.includes('app.py')) }).toEqual({ status: 'fail', kv: true });
|
|
} finally { cleanup(fx); }
|
|
});
|
|
|
|
test('diff.suppressBlankEmpty does not shift the diffed Python text (GST-738)', () => {
|
|
const fx: FixtureRepo = makeFixtureRepo({ pregate: BASE_PREGATE, baseFiles: { 'app.py': 'x = 1\n\ny = 2\n' } });
|
|
try {
|
|
commit(fx, { 'app.py': 'x = 1\n\ny = 2\nrows = {\n "password": session.password,\n}\n' });
|
|
fx.git('config', 'diff.suppressBlankEmpty', 'true');
|
|
const check = JSON.parse(runBin(fx, 'gstack-pregate', ['--base', 'main', '--only', 'secrets', '--json']).out).checks.find((c: { id: string }) => c.id === 'secrets');
|
|
expect(check.status).toBe('pass');
|
|
} finally { cleanup(fx); }
|
|
});
|
|
|
|
test('a one-line change to a Python file counts one added line (#3630)', () => {
|
|
const fx: FixtureRepo = makeFixtureRepo({ pregate: BASE_PREGATE, baseFiles: { 'app.py': 'x = 1\n'.repeat(50) } });
|
|
try {
|
|
commit(fx, { 'app.py': 'x = 1\n'.repeat(50) + 'y = 2\n' });
|
|
const check = JSON.parse(runBin(fx, 'gstack-pregate', ['--base', 'main', '--only', 'secrets', '--json']).out).checks.find((c: { id: string }) => c.id === 'secrets');
|
|
expect({ status: check.status, detail: check.detail }).toEqual({ status: 'pass', detail: expect.stringMatching(/^1 added lines clean/) });
|
|
} finally { cleanup(fx); }
|
|
});
|
|
|
|
test('a Python file too large to read whole has its added lines scanned without the source exemption (#3630)', () => {
|
|
const token = 'Fk2pQ9vXz4wL7mN3rT6yB1cD5eG0hJx';
|
|
const base = `CONFIG = """\nhost: db\n"""\n${'# pad\n'.repeat(800_000)}`;
|
|
const fx: FixtureRepo = makeFixtureRepo({ pregate: BASE_PREGATE, baseFiles: { 'app.py': base } });
|
|
try {
|
|
commit(fx, { 'app.py': base.replace('host: db\n', `host: db\npassword: ${token}\n`) });
|
|
const r = runBin(fx, 'gstack-pregate', ['--base', 'main', '--only', 'secrets', '--json']);
|
|
const check = JSON.parse(r.out).checks.find((c: { id: string }) => c.id === 'secrets');
|
|
expect({ status: check.status, kv: (check.lines as string[]).some((l) => l.includes('env.kv') && l.includes('app.py')) }).toEqual({ status: 'fail', kv: true });
|
|
} finally { cleanup(fx); }
|
|
});
|
|
|
|
test('a test file reading an env var in code and holding a self-named fixture label passes; a literal beside the read or in a .env file still fails (GST-1894)', () => {
|
|
const token = '8Fk2pQ9vXz4wL7mN3rT6yB1cD5eG0hJ';
|
|
const qm = 'let token: string | undefined;\nfunction start(opts?: { env?: Record<string, string> }) {\n token = opts?.env?.AGENT_API_TOKEN;\n}\nconst brain = {\n clientSecret: "personal-secret",\n};\n';
|
|
expect(repo({ 'memory-registry-pg.test.ts': qm }).secrets().status).toBe('pass');
|
|
const r = repo({ 'memory-registry-pg.test.ts': qm.replace('AGENT_API_TOKEN;', `AGENT_API_TOKEN ?? "${token}";`), 'agent.env': `AGENT_API_TOKEN=${token}\n` }).secrets();
|
|
expect(r.status).toBe('fail');
|
|
const kv = r.lines.filter((l) => l.includes('env.kv'));
|
|
expect(kv).toHaveLength(2);
|
|
expect(kv).toEqual(expect.arrayContaining([expect.stringContaining('agent.env, added line 1'), expect.stringContaining('memory-registry-pg.test.ts, added line 3')]));
|
|
});
|
|
|
|
test('the same assignment in a .env file still fails, naming the file', () => {
|
|
const f = repo({ 'deploy.env': 'API_TOKEN=8Fk2pQ9vXz4wL7mN3rT6yB1cD5eG0hJ\n' });
|
|
const r = f.secrets();
|
|
expect(r.status).toBe('fail');
|
|
expect(r.lines.join('\n')).toContain('deploy.env, added line 1');
|
|
});
|
|
|
|
test('an added line whose text starts with "++" is scanned', () => {
|
|
const f = repo({ 'notes.txt': `++aws_access_key_id = ${AWS}\n` });
|
|
expect(f.secrets().status).toBe('fail');
|
|
});
|
|
|
|
test('a diff.external driver that prints nothing does not make the branch clean', () => {
|
|
const f = repo({ 'notes.txt': `aws_access_key_id = ${AWS}\n` });
|
|
const driver = path.join(path.dirname(f.dir), 'empty-diff.sh');
|
|
fs.writeFileSync(driver, '#!/bin/sh\nexit 0\n', { mode: 0o755 });
|
|
f.git('config', 'diff.external', driver);
|
|
expect(f.secrets().status).toBe('fail');
|
|
});
|
|
});
|