diff --git a/changelog.d/gsta258-s1-envrc-redact.md b/changelog.d/gsta258-s1-envrc-redact.md index 46a7d71ee..4f374daae 100644 --- a/changelog.d/gsta258-s1-envrc-redact.md +++ b/changelog.d/gsta258-s1-envrc-redact.md @@ -1,9 +1,9 @@ **Security: the lean /autoplan reviewer no longer gets `.envrc`, `.env-production` or a key pasted into an ordinary file (GST-1164, #3914).** Since v1.93.36.0 (#3897), the lean run puts the branch's draft code into a prompt that goes to an outside model provider. It left out only some secret-named untracked files: `.env` and `.env.*`, but not `.envrc` or `.env-production`. It applied no name rule to tracked files and no content check at all. So a direnv `.envrc`, a `.env-production`, a committed or edited secret file, or a key sitting in any source file could reach the provider. ### Fixed -- **`lib/autoplan-lean.ts` `captureDraft`:** the secret-name rule now covers the whole `.env*` family, `.envrc` included, every `*credentials*` name, `_netrc`, `.pgpass`, `*.jks` and `id_*_sk` keys. Matching files are never diffed, tracked or untracked, and the draft names each one it left out. Every other file's patch goes through gstack's redact engine (`redactFindingSpans`), so a key in an ordinary file reaches the reviewer as ``. A patch the engine can't redact safely, including one over 1 MiB, is left out with a note. +- **`lib/autoplan-lean.ts` `captureDraft`:** the secret-name rule now covers the whole `.env*` family, `.envrc` included, every `*credentials*` name, `_netrc`, `.pgpass`, `*.jks` and `id_*_sk` keys. Matching files are never diffed, tracked or untracked, and the draft names each one it left out. Renames are listed as a delete and an add, so a secret file renamed on the branch stays out under both names. Every other file's patch goes through gstack's redact engine (`redactFindingSpans`), so a key in an ordinary file reaches the reviewer as ``. A patch the engine can't redact safely, including one over 1 MiB, is left out with a note. ### Tests -- `test/autoplan-lean.test.ts`: an untracked `.envrc`, `.env-production` and six other secret files stay out of `draft.diff` and the prompt; a committed and edited `.envrc` stays out of the tracked diff; a fake key in an untracked file and in a working-tree edit to a tracked file comes out redacted. All three fail on main at b42ad1e. +- `test/autoplan-lean.test.ts`: an untracked `.envrc`, `.env-production` and six other secret files stay out of `draft.diff` and the prompt; a committed and edited `.envrc` stays out of the tracked diff, and so does one renamed to `.envrc.local`; a fake key in an untracked file and in a working-tree edit to a tracked file comes out redacted. All four fail on main at b42ad1e. Thanks to the /ship pre-landing review of #3901, which filed the finding. diff --git a/lib/autoplan-lean.ts b/lib/autoplan-lean.ts index d690132f8..bc84ceea6 100644 --- a/lib/autoplan-lean.ts +++ b/lib/autoplan-lean.ts @@ -75,8 +75,8 @@ export function captureDraft(cwd: string, dir: string, out: string, base?: strin const inRun = (f: string) => !runDir.startsWith('..') && !path.isAbsolute(runDir) && (runDir === '' || f === runDir || f.startsWith(`${runDir}/`)); const failed = (r: ReturnType, what: string, ok = [0]) => r.error || r.status === null || !ok.includes(r.status) ? `draft capture failed: ${what} ${r.error ? `errored (${r.error.message})` : `exited ${r.status ?? r.signal}`}${r.stderr?.trim() ? `: ${r.stderr.trim().split('\n')[0]}` : ''}` : undefined; - const raw = ['diff', '--no-color', '--no-ext-diff', '--no-textconv']; - const names = git(['diff', '--name-only', '-z', mergeBase, '--'], root); + const raw = ['diff', '--no-color', '--no-ext-diff', '--no-textconv', '--no-renames']; + const names = git(['diff', '--name-only', '-z', '--no-renames', mergeBase, '--'], root); const others = git(['ls-files', '--others', '--exclude-standard', '-z'], root); const listed = failed(names, 'git diff') ?? failed(others, 'git ls-files'); if (listed) return { base: ref, none: listed }; diff --git a/test/autoplan-lean.test.ts b/test/autoplan-lean.test.ts index 9fa011d6f..79abe33ce 100644 --- a/test/autoplan-lean.test.ts +++ b/test/autoplan-lean.test.ts @@ -423,6 +423,27 @@ describe('draft capture edges', () => { expect(diff).toContain('+export const JITTER = 0.1;'); }); + test('a secret file renamed on the branch stays out under both names', () => { + const cwd = repo('rename-envrc', false); + git(cwd, 'checkout', '-q', 'main'); + fs.writeFileSync(path.join(cwd, '.envrc'), 'dotenv_if_exists marker-zz9-old\n'); + git(cwd, 'add', '.envrc'); + git(cwd, 'commit', '-q', '-m', 'direnv'); + git(cwd, 'checkout', '-q', 'feature'); + git(cwd, 'merge', '-q', 'main'); + git(cwd, 'mv', '.envrc', '.envrc.local'); + fs.appendFileSync(path.join(cwd, '.envrc.local'), 'dotenv_if_exists marker-zz9-new\n'); + fs.appendFileSync(path.join(cwd, 'lib', 'worker.ts'), 'export const JITTER = 0.1;\n'); + const { ps, prompt } = lean('rename-envrc', cwd); + const diff = fs.readFileSync(path.join(ps.snapshot_dir, 'draft.diff'), 'utf8'); + for (const text of [diff, prompt]) { + expect(text).not.toContain('marker-zz9'); + expect(text).toContain('# draft: .envrc left out (its name looks like a secret)'); + expect(text).toContain('# draft: .envrc.local left out (its name looks like a secret)'); + } + expect(diff).toContain('+export const JITTER = 0.1;'); + }); + test('a key inside an ordinary file is redacted, untracked or edited in the working tree', () => { const cwd = repo('redact', true); fs.writeFileSync(path.join(cwd, 'lib', 'client.ts'), `export const KEY = '${FAKE_KEY}';\n`);