diff --git a/packages/cli/src/commands/review/lib/worktree.test.ts b/packages/cli/src/commands/review/lib/worktree.test.ts index 946020fc8f9..9d6fe904ba3 100644 --- a/packages/cli/src/commands/review/lib/worktree.test.ts +++ b/packages/cli/src/commands/review/lib/worktree.test.ts @@ -32,9 +32,14 @@ import { tmpdir } from 'node:os'; import { basename, dirname, join } from 'node:path'; import { isolateHostGitConfig } from './test-utils.js'; import { + MAX_SCREEN_CANDIDATES, + MAX_SCREEN_KEYS, + carriesReplacementChar, discardWorktree, exposeDependencies, + localFilterCommands, sanitizedGitEnv, + screenStopDetail, worktreeCreateFailureDetail, worktreeResidue, } from './worktree.js'; @@ -1885,3 +1890,489 @@ describe('worktreeCreateFailureDetail', () => { ); }); }); + +describe('localFilterCommands', () => { + let dir: string; + let isolation: { dispose: () => void }; + + const initRepo = (d: string) => { + execFileSync('git', ['init', '-q', '-b', 'main', '--template=', '.'], { + cwd: d, + }); + }; + + beforeEach(() => { + isolation = isolateHostGitConfig(); + dir = mkdtempSync(join(tmpdir(), 'qwen-screen-')); + initRepo(dir); + }); + + afterEach(() => { + try { + chmodSync(join(dir, '.git', 'worktrees'), 0o755); + } catch { + // Not every case creates it; rmSync below is the cleanup either way. + } + isolation.dispose(); + rmSync(dir, { recursive: true, force: true }); + }); + + it('finds a repo-local filter and reports the total', () => { + execFileSync('git', ['config', 'filter.evil.smudge', 'cat'], { cwd: dir }); + + const screen = localFilterCommands(dir); + + expect(screen.keys).toEqual(['filter.evil.smudge']); + expect(screen.total).toBe(1); + expect(screen.unreadable).toBeNull(); + }); + + it("never reports the user's global keys through an include", () => { + // The screen does not follow includes at all — a declared limit, see the + // spawn's comment. This pins the half of it that MUST hold: an + // `include.path` naming the user's own global config never drags their + // `filter.lfs.*` into a repo-local refusal. `git lfs install` writes that + // key, so following the edge would put every contributor with git-lfs into + // permanent refusal — the failure this screen is scoped to avoid. + const outside = mkdtempSync(join(tmpdir(), 'qwen-userconf-')); + try { + const userConfig = join(outside, 'gitconfig'); + writeFileSync( + userConfig, + '[filter "lfs"]\n\tclean = git-lfs clean -- %f\n', + ); + appendFileSync( + join(dir, '.git', 'config'), + `[include]\n\tpath = ${userConfig}\n`, + ); + + const screen = localFilterCommands(dir); + + expect(screen.keys).toEqual([]); + expect(screen.stopped).toBeNull(); + } finally { + rmSync(outside, { recursive: true, force: true }); + } + }); + + it('never reports global keys through a `~` include either', () => { + // Same rule through the tilde form: `[include] path = ~/.gitconfig` is the + // one-line write a probe can make into the never-wiped common config, and + // `sanitizedGitEnv()` keeps HOME so it would expand to the reviewing + // user's real config. + const home = process.env['HOME']; + expect(home).toBeTruthy(); + writeFileSync( + join(home as string, '.gitconfig'), + '[filter "lfs"]\n\tclean = git-lfs clean -- %f\n', + ); + appendFileSync( + join(dir, '.git', 'config'), + '[include]\n\tpath = ~/.gitconfig\n', + ); + + const screen = localFilterCommands(dir); + + expect(screen.keys).toEqual([]); + expect(screen.stopped).toBeNull(); + }); + + it('matches a filter key whose subsection carries an invalid UTF-8 byte', () => { + // `--get-regexp` is POSIX matching and locale-sensitive: under a UTF-8 + // locale a key holding an invalid byte never matches, so git exits 1 and + // that reads as the ordinary "no key matched". The checkout resolves + // filter keys by exact name, which no locale affects — so the screen would + // certify clean over a filter the checkout runs. + appendFileSync( + join(dir, '.git', 'config'), + Buffer.concat([ + Buffer.from('[filter "'), + Buffer.from([0xff]), + Buffer.from('"]\n\tsmudge = cat\n'), + ]), + ); + + const screen = localFilterCommands(dir); + + expect(screen.total).toBe(1); + expect(screen.keys[0]).toMatch(/^filter\..*\.smudge$/); + }); + + it('refuses when an admin entry name did not survive the decode', () => { + // Entry names are the third attacker-influenced source of candidate bytes, + // after the two rev-parse answers. An invalid byte in the name comes back + // from `readdirSync` as U+FFFD, `join` re-encodes it, `existsSync` is then + // false, and the loop would skip a `config.worktree` that really exists + // and really defines a filter — the clean verdict authorising the + // checkout that runs it. + // + // The fixture uses a real U+FFFD in the name rather than an invalid byte: + // it is what the guard actually tests for, and unlike an invalid byte it + // can be created on every platform (macOS rejects those with EILSEQ). + mkdirSync(join(dir, '.git', 'worktrees', 'w\uFFFDx'), { recursive: true }); + + const screen = localFilterCommands(dir); + + expect(screen.stopped).toBe('unreadable'); + expect(screen.unreadable).toBe(join(dir, '.git', 'worktrees')); + }); + + it('treats a transcoded rev-parse answer as unmeasured', () => { + // `encoding: 'utf8'` turns an invalid byte in the repository path into + // U+FFFD, so every candidate built from the answer names a file that does + // not exist, the admin readdir hits ENOENT — the branch treated as the + // ordinary "no linked worktrees" case — and the screen would answer clean + // over whatever the real path holds. Unmeasured is not clean; the residue + // walker in this file fails the same shape closed. + // + // Pinned at this level deliberately. The end-to-end fixture cannot be + // built from Node: a path carrying an invalid byte does not survive + // `Buffer`→`string`, and `cwd` accepts only a string, so the repository + // cannot be created and then addressed from a test. (Attempting it is how + // this test got written the first time — the round-trip re-encoded 0xFF as + // 0xC3 0xBF and git failed with ENOENT on a path that did not exist.) + expect(carriesReplacementChar('/tmp/plain/path')).toBe(false); + expect(carriesReplacementChar('')).toBe(false); + expect(carriesReplacementChar('/tmp/n\uFFFDd/repo')).toBe(true); + expect(carriesReplacementChar('\uFFFD')).toBe(true); + }); + + // POSIX-only: `mkfifo` does not exist on Windows. The bound it pins is + // platform-independent; only this way of provoking a block is not. + it.skipIf(process.platform === 'win32')( + 'refuses rather than hanging when an include names a FIFO', + () => { + // git follows includes at startup, so the very first `rev-parse` blocks on + // a FIFO nobody writes to — and `spawnSync` blocks the event loop, so no + // JS timer can interrupt it. The bound has to be on the spawn, and a + // killed discovery must not read as "not a usable repository". + const fifo = join(dir, '.git', 'evil.fifo'); + execFileSync('mkfifo', [fifo]); + appendFileSync( + join(dir, '.git', 'config'), + `[include]\n\tpath = ${fifo}\n`, + ); + + const started = Date.now(); + const screen = localFilterCommands(dir); + const elapsed = Date.now() - started; + + expect(screen.stopped).toBe('unreadable'); + // One spawn timeout, not two. A null `commonDir` already decides the + // refusal above, so re-running discovery for `gitDir` only doubles the + // synchronous block: 40s measured, most of vitest's fixed 60s worker RPC + // budget, which on the Linux lane exits an all-green suite red. + expect(elapsed).toBeLessThan(30_000); + }, + 60_000, + ); + + it('caps the reported keys and keeps the pre-cap total', () => { + // The keys come from an attacker-writable file that cleanup never wipes; + // an unbounded list is its own denial-of-service in the refusal message. + let cfg = ''; + for (let i = 0; i < MAX_SCREEN_KEYS + 5; i++) { + cfg += `[filter "evil${i}"]\n\tsmudge = cat\n`; + } + appendFileSync(join(dir, '.git', 'config'), cfg); + + const screen = localFilterCommands(dir); + + expect(screen.keys).toHaveLength(MAX_SCREEN_KEYS); + expect(screen.total).toBe(MAX_SCREEN_KEYS + 5); + // Still names a real planted key — a cap that reported none would be a + // refusal nobody can act on. + expect(screen.keys[0]).toMatch(/^filter\.evil\d+\.smudge$/); + }); + + // POSIX-only: the shim is a `#!/bin/sh` script found through `which`, and + // neither works on Windows. The ORDER it pins is platform-independent, so + // skipping there loses the witness, not the behaviour. + it.skipIf(process.platform === 'win32')( + 'reads `/config` LAST, after the amplifiable admin set', + () => { + // Order is load-bearing, not cosmetic. The checkout this screen authorises + // reads merged config at its start, so the gap between the screen's read + // of a file and that checkout is a window a plant can land in. The + // admin-dir set is attacker-amplifiable — filler entries cost spawns and + // stretch the walk — so reading the common config first would put the + // likeliest plant target at the START of a window the plant itself can + // widen. Pinned through a `git` shim on PATH, which is deterministic; + // racing it would not be. + const shimDir = mkdtempSync(join(tmpdir(), 'qwen-shim-')); + const log = join(shimDir, 'calls.log'); + const realGit = execFileSync('which', ['git'], { + encoding: 'utf8', + }).trim(); + writeFileSync( + join(shimDir, 'git'), + `#!/bin/sh\nprintf '%s\\n' "$*" >> ${log}\nexec ${realGit} "$@"\n`, + ); + chmodSync(join(shimDir, 'git'), 0o755); + for (const e of ['w1', 'w2']) { + mkdirSync(join(dir, '.git', 'worktrees', e), { recursive: true }); + writeFileSync(join(dir, '.git', 'worktrees', e, 'config.worktree'), ''); + } + writeFileSync(join(dir, '.git', 'config.worktree'), ''); + const savedPath = process.env['PATH']; + try { + process.env['PATH'] = `${shimDir}:${savedPath ?? ''}`; + + localFilterCommands(dir); + + const reads = readFileSync(log, 'utf8') + .split('\n') + .filter((l) => l.includes('--file')) + .map((l) => l.split(' --file ')[1]?.split(' ')[0] ?? ''); + const own = join(dir, '.git', 'config.worktree'); + const commonCfg = join(dir, '.git', 'config'); + // The file the checkout honours is dead last... + expect(reads[reads.length - 1]).toBe(own); + // ...with the common config immediately before it, and the paddable + // admin entries before both. + expect(reads[reads.length - 2]).toBe(commonCfg); + expect( + reads.indexOf(join(dir, '.git', 'worktrees', 'w1', 'config.worktree')), + ).toBeLessThan(reads.indexOf(commonCfg)); + } finally { + process.env['PATH'] = savedPath; + rmSync(shimDir, { recursive: true, force: true }); + } + }, + ); + + // POSIX-only, same shim mechanism as the sibling above. + it.skipIf(process.platform === 'win32')( + 'reads a CROSS-tree checkout target last, not the screened tree\'s own', + () => { + // `scratch-tree` screens the review worktree while authorising a + // checkout in the scratch tree. The producer cannot tell which admin + // entry that is, so the caller names it — and it, not the screened + // tree's own config, is what has to be read last. + const shimDir = mkdtempSync(join(tmpdir(), 'qwen-xshim-')); + const log = join(shimDir, 'calls.log'); + const realGit = execFileSync('which', ['git'], { + encoding: 'utf8', + }).trim(); + writeFileSync( + join(shimDir, 'git'), + `#!/bin/sh\nprintf '%s\\n' "$*" >> ${log}\nexec ${realGit} "$@"\n`, + ); + chmodSync(join(shimDir, 'git'), 0o755); + for (const e of ['other', 'scratchy']) { + mkdirSync(join(dir, '.git', 'worktrees', e), { recursive: true }); + writeFileSync(join(dir, '.git', 'worktrees', e, 'config.worktree'), ''); + } + writeFileSync(join(dir, '.git', 'config.worktree'), ''); + const savedPath = process.env['PATH']; + try { + process.env['PATH'] = `${shimDir}:${savedPath ?? ''}`; + + localFilterCommands(dir, join(dirname(dir), 'scratchy')); + + const reads = readFileSync(log, 'utf8') + .split('\n') + .filter((l) => l.includes('--file')) + .map((l) => l.split(' --file ')[1]?.split(' ')[0] ?? ''); + expect(reads[reads.length - 1]).toBe( + join(dir, '.git', 'worktrees', 'scratchy', 'config.worktree'), + ); + // The screened tree's own config is no longer the last read. + expect(reads).not.toContain(join(dir, '.git', 'config.worktree')); + } finally { + process.env['PATH'] = savedPath; + rmSync(shimDir, { recursive: true, force: true }); + } + }, + ); + + it("refuses past the candidate cap instead of walking a plant's filler", () => { + // Each entry costs one synchronous `git config` spawn, on every screen + // call — once per mutant. Skipping past the bound would let filler hide a + // real filter behind it, so the screen refuses. + for (let i = 0; i <= MAX_SCREEN_CANDIDATES; i++) { + mkdirSync(join(dir, '.git', 'worktrees', `filler-${i}`), { + recursive: true, + }); + } + + const screen = localFilterCommands(dir); + + expect(screen.unreadable).toBe(join(dir, '.git', 'worktrees')); + expect(screen.keys).toEqual([]); + // Read fine, just too many — a different remedy from an unreadable file. + expect(screen.stopped).toBe('over-cap'); + }); + + // POSIX DAC bits only: Windows does not restrict listing through chmod, and + // root bypasses the mode entirely — the fixture would be red on both rather + // than pinning anything. Same guard as the other permission-bit fixture in + // this file. + it.skipIf(process.platform === 'win32' || process.getuid?.() === 0)( + 'refuses when the worktrees admin dir cannot be read', + () => { + // git opens each `worktrees//config.worktree` by direct path — the + // search bit alone suffices — so a readdir that fails with EACCES means + // the screen never saw candidates git will still honour. Swallowing that + // as "no linked worktrees" answers clean on a config it did not read. + const admin = join(dir, '.git', 'worktrees'); + mkdirSync(admin, { recursive: true }); + chmodSync(admin, 0o111); + + const screen = localFilterCommands(dir); + + expect(screen.unreadable).toBe(admin); + // The two refusal causes must stay distinguishable: this one is a file + // to fix, the over-cap one is a directory to prune. + expect(screen.stopped).toBe('unreadable'); + }, + ); + + it('names a key whose subsection carries whitespace, intact', () => { + // A config subsection name may legally hold spaces, and `--get-regexp` + // prints `key value` on one line — so splitting on whitespace named + // `filter.evil` for a planted `filter.evil name.smudge`. The refusal then + // prescribed an unset that exits 5 while the plant stood, and two distinct + // keys sharing a truncated prefix collapsed into one, understating `total`. + execFileSync('git', ['config', 'filter.evil name.smudge', 'CMD'], { + cwd: dir, + }); + execFileSync('git', ['config', 'filter.evil other.smudge', 'CMD2'], { + cwd: dir, + }); + + const screen = localFilterCommands(dir); + + expect(screen.keys.sort()).toEqual([ + 'filter.evil name.smudge', + 'filter.evil other.smudge', + ]); + expect(screen.total).toBe(2); + }); + + it('sees a filter through a repo path that carries a newline', () => { + // The screened trees are linked worktrees inheriting the user's repo path. + // A combined `rev-parse --git-common-dir --git-dir` answer splits into four + // records under such a path instead of two, every candidate resolves to + // nothing, the admin readdir hits ENOENT — the branch treated as ordinary — + // and the screen answers clean over a live plant. + const base = mkdtempSync(join(tmpdir(), 'qwen-nl-')); + try { + const holder = join(base, 'nl\ndir'); + mkdirSync(holder, { recursive: true }); + const main = join(holder, 'main'); + mkdirSync(main); + initRepo(main); + writeFileSync(join(main, 'a.ts'), 'x\n'); + execFileSync('git', ['add', '-A'], { cwd: main }); + execFileSync( + 'git', + ['-c', 'user.email=t@t.t', '-c', 'user.name=t', 'commit', '-qm', 'i'], + { cwd: main }, + ); + execFileSync('git', ['config', 'filter.evil.smudge', 'cat'], { + cwd: main, + }); + const linked = join(holder, 'wt'); + execFileSync( + 'git', + ['worktree', 'add', '-q', '--detach', linked, 'HEAD'], + { cwd: main }, + ); + + const screen = localFilterCommands(linked); + + expect(screen.keys).toEqual(['filter.evil.smudge']); + } finally { + rmSync(base, { recursive: true, force: true }); + } + }); + + it('stays clean on a repo with no linked worktrees at all', () => { + // ENOENT on the admin dir is the ordinary case and must NOT refuse. + expect(existsSync(join(dir, '.git', 'worktrees'))).toBe(false); + + const screen = localFilterCommands(dir); + + expect(screen).toEqual({ + keys: [], + total: 0, + unreadable: null, + stopped: null, + }); + }); + + it('refuses a candidate git cannot parse', () => { + // Opens fine, and git then dies on it with exit 128 — not the exit 1 that + // means "no key matched". + writeFileSync( + join(dir, '.git', 'config.worktree'), + '[filter "evil"\n\tsmudge = cat\n', + ); + + const screen = localFilterCommands(dir); + + expect(screen.unreadable).toBe(join(dir, '.git', 'config.worktree')); + }); + + it('does NOT see a filter one include-hop away — a declared limit', () => { + // `git config --file` does not expand includes and this screen does not + // walk them either, so a filter behind an include is invisible here while + // the checkout, reading merged config, would run it. That gap is real and + // deliberate: both ways of closing it were tried and both were worse (the + // spawn's comment records why), and #10441 carries the design that closes + // it properly. This test exists so the limit is visible rather than + // assumed — if a later change starts following includes, it goes red and + // whoever wrote it has to decide deliberately. + writeFileSync( + join(dir, '.git', 'evil.inc'), + '[filter "evil"]\n\tsmudge = cat\n', + ); + appendFileSync( + join(dir, '.git', 'config'), + '[include]\n\tpath = evil.inc\n', + ); + + const screen = localFilterCommands(dir); + + expect(screen.keys).toEqual([]); + expect(screen.stopped).toBeNull(); + }); +}); + +describe('screenStopDetail', () => { + // The two refusal causes call for OPPOSITE remedies, so they must not share a + // sentence — an unreadable candidate is a file to fix or remove, while an + // over-cap admin directory was read perfectly well and "remove that file" + // there would deregister every legitimate linked worktree. Each arm is pinned + // directly: the producer's classification is tested elsewhere, but the + // rendered remedy a reader acts on is only here. + it('names the prune remedy for an over-cap directory, not a file to remove', () => { + const detail = screenStopDetail({ + keys: [], + total: 0, + unreadable: '/repo/.git/worktrees', + stopped: 'over-cap', + }); + + expect(detail).toContain('git worktree prune'); + expect(detail).toContain('registered under'); + expect(detail).toContain(String(MAX_SCREEN_CANDIDATES)); + expect(detail).toContain('/repo/.git/worktrees'); + // Must NOT tell the reader the directory could not be read — it was. + expect(detail).not.toContain('could not be read to the end'); + }); + + it('names the unreadable file plainly, without the prune remedy', () => { + const detail = screenStopDetail({ + keys: [], + total: 0, + unreadable: '/repo/.git/config.worktree', + stopped: 'unreadable', + }); + + expect(detail).toContain('/repo/.git/config.worktree'); + expect(detail).toContain('could not be read to the end'); + expect(detail).not.toContain('git worktree prune'); + }); +}); diff --git a/packages/cli/src/commands/review/lib/worktree.ts b/packages/cli/src/commands/review/lib/worktree.ts index 13a122da70f..245b7a19b11 100644 --- a/packages/cli/src/commands/review/lib/worktree.ts +++ b/packages/cli/src/commands/review/lib/worktree.ts @@ -18,6 +18,8 @@ import { spawnSync } from 'node:child_process'; import { + accessSync, + constants as fsConstants, existsSync, lstatSync, mkdtempSync, @@ -34,6 +36,7 @@ import { } from 'node:fs'; import { tmpdir } from 'node:os'; import { basename, dirname, isAbsolute, join, resolve, sep } from 'node:path'; +import { inertPath } from './paths.js'; import { readWorkspacePackages } from './workspaces.js'; export type SweepResult = ReturnType; @@ -578,6 +581,467 @@ function trackedIgnoreSources( */ export const RESIDUE_PATH_CAP = 12; +/** + * Caps for the config screen, which reads attacker-writable files. + * + * `MAX_SCREEN_KEYS` bounds what a refusal REPORTS — the message names the + * first keys and the caller says how many more; an unbounded join is its own + * denial-of-service, the same rule the `unreadable` field follows by naming + * one file rather than a list. `MAX_SCREEN_CANDIDATES` bounds what the screen + * READS: one synchronous spawn per registered worktree, on a directory a + * probe can fill. Both fail closed — past the candidate bound the screen + * refuses, because skipping would let filler entries hide a real filter. + */ +export const MAX_SCREEN_KEYS = RESIDUE_PATH_CAP; +export const MAX_SCREEN_CANDIDATES = 256; + +/** + * The repo-local `filter.` COMMANDS, when any are defined. + * + * A checkout EXECUTES these whenever it rewrites a file, and disabling hooks + * does not cover filters — they are separate config. The callers that screen + * through this function are the scratch tree's reset and rebuild, the probe + * tree's per-run restore, and the probe tree's revert. That list is the set of + * SCREENED sites, not a census of the pipeline's filter-executing checkouts: + * the `worktree add` spawns that CREATE the probe and base trees run + * unscreened today, and hooks are neutralised only at the spawns that pass + * `core.hooksPath` explicitly, not pipeline-wide. Do not read this comment as + * a completeness map — widening the screen means adding a call site, and the + * count above is what has one. + * + * The planting surface this screen covers is two plain writes a probe can + * make into the COMMON dir this command's report calls shared: + * `git config filter.evil.smudge CMD` and one line appended to + * `$(git rev-parse --git-path info/attributes)`. discard and cleanup never + * wipe the common dir, so a filter planted while reviewing one PR fires on + * every later matching checkout of the user's OWN repository — persistence + * planted by reviewing a malicious PR, measured live. The candidate config + * files are read with `--file`, one file at a time, rather than merged config: + * filters in the user's global config (git-lfs is the common one) are the + * user's own contract, exactly like any git command they run, so global config + * stays out of scope. + * + * TWO limits, both declared rather than discovered. Screening repo-local-only + * is a deliberate TRADE, not a claim that repo-local files are all a probe can + * reach: probe code runs as the user, so `git config --global` is open to it, + * and a filter planted there is read by every checkout here and never seen by + * this screen. And a filter reached only through an `include.path` / + * `includeIf` directive is not seen either — see the spawn below for why + * neither `--includes` nor a hand-walk is the answer, and #10441 for the + * design that is. + * Refusing on merged config is not the answer — `git lfs install` writes + * `filter.lfs.clean` globally, and refusing on that is permanent refusal for + * every contributor who has git-lfs. The state cannot be told apart from a + * filter the user set deliberately, and cannot be safely wiped, so a hit is a + * refusal upstream, not a cleanup here. + */ +export interface LocalFilterScreen { + /** + * The repo-local `filter.` command keys found, capped at + * `MAX_SCREEN_KEYS`. `total` says how many there were before the cap, so a + * refusal can name what it is not showing instead of silently shortening. + */ + keys: string[]; + /** + * How many matching entries the screen found, against `keys.length` shown. + * + * Exact while under `MAX_SCREEN_KEYS`; beyond it an upper bound, because + * retention stops at the cap and there is nothing left to deduplicate a + * later repeat against. Either way it answers the only question a refusal + * needs it for: is there more than what is named. + */ + total: number; + /** + * Why the screen stopped, when it did. + * + * `unreadable` and `over-cap` both refuse, but they call for opposite + * remedies: an unreadable candidate is a file to fix or remove, while an + * over-cap admin directory was read perfectly well and holds more + * registered worktrees than the bound — "remove that file" there would + * deregister every legitimate linked worktree, the pipeline's own included. + */ + stopped: 'unreadable' | 'over-cap' | null; + /** + * The first candidate file the screen could not read to completion, when one + * stopped it — otherwise null. + * + * Exit 1 is git's ordinary "no key matched" and is not a failure. Anything + * else is: a spawn error, a spawn killed at the timeout, a config git could + * not parse to the end (a malformed section header exits 128), or an ENOBUFS + * from more filter keys than the buffer holds. Skipping such a file reports the + * repository clean on the one file that might define the filter, so the caller + * refuses instead. One file, not a list: the refusal only has to name where + * the screen stopped, and an unbounded join is its own denial-of-service. + */ + unreadable: string | null; +} + +/** + * The screened keys, flattened and counted, for a refusal message. + * + * `keys` arrives already capped by the producer, so this never re-slices; it + * reports the shortfall instead. Both halves matter: `inertPath` because a + * config subsection name legally carries control and format characters that + * `--get-regexp` prints verbatim, and these strings land in the agent-facing + * report; the count because a message naming 12 of 13 keys sends a user to + * remove the 12 and be refused again on the one it never mentioned. + */ +export function screenKeyList(screen: LocalFilterScreen): string { + const shown = screen.keys.map(inertPath).join(', '); + return screen.total > screen.keys.length + ? `${shown} (capped enumeration: ${screen.keys.length} shown of ${screen.total})` + : shown; +} + +/** + * Why a screen that stopped could not finish, phrased for the reader who has + * to act on it. + * + * The two causes need opposite remedies, so they must not share a sentence: + * an unreadable candidate is a file to fix or remove, while an over-cap admin + * directory was read perfectly well — telling that reader to "remove that + * file" would deregister every legitimate linked worktree they have. + */ +export function screenStopDetail(screen: LocalFilterScreen): string { + const where = inertPath(screen.unreadable ?? ''); + return screen.stopped === 'over-cap' + ? `more than ${MAX_SCREEN_CANDIDATES} worktrees are registered under ${where} — more than this screen will walk, so it cannot tell whether one of them defines a content filter. Prune stale ones (\`git worktree prune\`) and re-run` + : `${where} could not be read to the end`; +} + +/** + * A single `git rev-parse ` answer, as a path, or null on any failure. + * + * One invocation per value, never a combined newline-delimited request: the + * answers are arbitrary filesystem paths and a POSIX path may itself carry a + * newline, so a combined answer cannot be split unambiguously — a healthy + * worktree below a directory whose name holds a newline parses to extra + * records, misassigns the paths, and the caller reports a live plant as clean + * or a real worktree as not-a-worktree. Measured with exactly such a directory. + * The only byte removed is git's terminal record delimiter; every other byte + * belongs to the path, so neither a split nor a trim is a parse here. Callers + * that need absolute paths pass `--path-format=absolute` as a leading flag. + */ +/** + * The screen's spawn budget. + * + * Every read below opens attacker-writable config, and git follows an + * `include.path` at startup — so a plant naming a FIFO (or a symlink to + * `/dev/zero`) blocks the very first `rev-parse`. `spawnSync` blocks the event + * loop, so no JS timer can interrupt it; the bound has to be on the spawn. + */ +const SCREEN_SPAWN_TIMEOUT_MS = 20_000; + +/** + * A rev-parse answer, or null when the screen could not get one. + * + * `encoding: 'utf8'` transcodes an invalid byte to U+FFFD, so a repository + * under a path carrying one yields an answer that names no real file. Callers + * must treat that as unmeasured rather than resolve candidates against it — + * the residue walker in this file fails the same shape closed for the same + * reason. + */ +function revParsePath(cwd: string, ...flags: string[]): string | null { + const r = spawnSync('git', ['rev-parse', ...flags], { + cwd, + encoding: 'utf8', + timeout: SCREEN_SPAWN_TIMEOUT_MS, + killSignal: 'SIGKILL', + env: sanitizedGitEnv(), + }); + if (r.error || r.status !== 0 || typeof r.stdout !== 'string') return null; + return r.stdout.endsWith('\n') ? r.stdout.slice(0, -1) : r.stdout; +} + +/** + * True when a transcode replaced bytes this screen would otherwise resolve. + * + * Exported for its own test: the end-to-end fixture cannot be built from Node. + * A path holding an invalid UTF-8 byte survives neither `Buffer`→`string` nor + * the `cwd` option, which takes a string only — so the repository that would + * produce such a rev-parse answer cannot be created and then addressed from a + * test. The rule this function states is pinned here instead, and its two call + * sites are one `if` each. + */ +export function carriesReplacementChar(value: string): boolean { + return value.includes('\uFFFD'); +} + +export function localFilterCommands( + worktree: string, + /** + * The tree the authorised checkout will run in, when that is not `worktree`. + * + * The screen reads candidates in sequence and the checkout reads merged + * config at its start, so each file's window runs from ITS read to the + * spawn. The file that decides what the checkout executes therefore has to + * be read last. For the restore and the revert that tree is the screened + * one, which is the default; `scratch-tree` screens the review worktree + * while authorising a checkout in the scratch tree, and only the caller + * knows which tree that is. + * + * Its per-worktree config is looked for at `/worktrees//config.worktree`, which is where `git worktree add` registers + * it. If the tree does not exist yet — the rebuild path creates it after + * this screen — the file does not exist either and the read is a no-op, + * which is the correct answer: there is nothing there to plant in. + */ + checkoutTree?: string, +): LocalFilterScreen { + const commonDir = revParsePath(worktree, '--git-common-dir'); + // The second discovery spawn is skipped once the first fails. Both read + // config at git startup, so a plant that blocks one blocks the other for the + // whole `SCREEN_SPAWN_TIMEOUT_MS`, and a null `commonDir` already decides the + // refusal below whatever `gitDir` would have answered. Running both anyway + // doubled that stall to a measured 40s of blocked event loop — most of + // vitest's fixed 60s worker RPC budget, which on the Linux lane (no + // unhandled-error exemption) exits an all-green suite red. + const gitDir = + commonDir === null ? null : revParsePath(worktree, '--git-dir'); + if (commonDir === null || gitDir === null) { + // Two very different reasons land here, and only one is benign. A plain + // "not a repository" answer is the caller's own problem — its checkout has + // nothing to run in either. But a discovery spawn KILLED at the timeout + // above reaches this line too, and that one happens precisely when a plant + // has made git block, so it must not read as clean. There is no way to + // tell them apart from the outside, so this fails closed: a screen that + // could not discover the repository did not screen it. + return { + keys: [], + total: 0, + unreadable: worktree, + stopped: 'unreadable', + }; + } + if (carriesReplacementChar(commonDir) || carriesReplacementChar(gitDir)) { + // An invalid byte in the repository path came back as U+FFFD, so every + // candidate built from it names a file that does not exist, the admin + // readdir hits ENOENT — "the ordinary case" — and the screen would answer + // clean over whatever the real path holds. Unmeasured, not clean. + return { + keys: [], + total: 0, + unreadable: worktree, + stopped: 'unreadable', + }; + } + const common = resolve(worktree, commonDir); + // `/config` is read LAST, and the order is load-bearing. The + // checkout this screen authorises reads merged config at its own start, so + // everything between the screen's read of a file and that checkout is a + // window in which a plant can land in it. The admin-dir set below is + // attacker-AMPLIFIABLE — up to MAX_SCREEN_CANDIDATES one-byte filler + // entries, each costing spawns — so reading the common config first put the + // most likely plant target at the START of a walk the plant itself can + // stretch to seconds. Measured: 256 fillers stretched the walk to ~9 s, and + // a blind planter won. Reading it last leaves only the walk's own tail + // between it and the checkout. This does not close the window — nothing + // here can, since a checkout either reads merged config or does not run — + // it removes the amplification. + // The checkout tree's own per-worktree config, held back to the very end. + const ownAdminConfig = + checkoutTree === undefined || resolve(checkoutTree) === resolve(worktree) + ? join(resolve(worktree, gitDir), 'config.worktree') + : join( + common, + 'worktrees', + basename(resolve(checkoutTree)), + 'config.worktree', + ); + const candidates: string[] = []; + // Every OTHER worktree's per-worktree config too. This screen runs against + // the review worktree, but the checkout it authorises runs in the SCRATCH + // tree, whose own `/worktrees/