diff --git a/.worklane/review.yaml b/.worklane/review.yaml index 8bf057d46..355e66a2e 100644 --- a/.worklane/review.yaml +++ b/.worklane/review.yaml @@ -32,6 +32,7 @@ categories: - test/package-files.test.ts - test/managed-apps.test.ts - test/merge-idempotent.test.ts + - test/git-runner.test.ts - test/vacuity.test.ts - test/merge-policy-vacuity.test.ts - test/merge-vacuity-coordinator.test.ts diff --git a/changelog.d/927-git-runner.md b/changelog.d/927-git-runner.md new file mode 100644 index 000000000..a7723986c --- /dev/null +++ b/changelog.d/927-git-runner.md @@ -0,0 +1,31 @@ +### Upgrading (from v0.5.2) + +- **Every git the engine's JS and TS code runs ignores replace refs and grafts, on git 2.42 and later (#927).** An + agent could write `refs/replace/*` or `.git/info/grafts` in the shared repo and make the push limits, the secret + scan and the review read other commits. Measured on git 2.43. Before 2.42, a config `core.useReplaceRefs=true` may + turn replace refs back on (inferred, not run). The `.sh` helpers under `scripts/` don't go through the runner. A change's recorded + `patch_hash` now covers every file's lines, so it differs from an older engine's for the same change. Run + `npm run setup:hooks` once after upgrading: until then, an install's old copy of the push-gate hook runs its own + git calls (the one that loads the gate among them) without these settings. It keeps working. +- **A `GIT_CONFIG_COUNT` that isn't a plain decimal from 0 to 9999 now makes git refuse (#927)**, for every git the + runner starts and in the push gate: the runner can't add its setting after a count it can't read. Nothing in the + engine writes such a count; an environment that sets one must write it plainly. + +### Guardrail changes: what each one does + +- **The git runner (#927):** `src/os/git.ts` (and `scripts/lib/git.cjs` for scripts) starts every git with replace + refs off and no graft file, over whatever environment the caller passes; gitleaks gets the same settings. Replace + refs stay off whatever the config says on git 2.42 and later (measured on 2.43; before 2.42, see above). A test + fails on a git call in the JS and TS files under `src/` and `scripts/` that doesn't come through it (it doesn't + read `.sh` files, git named in a shell string, or a name built at run time), and the runner and that test wait for + the owner. +- **A shallow cut in a pushed range is refused (#927):** a `.git/shallow` (a shallow clone's, or one written by hand) + that cuts the range being pushed made the push limits read only part of it. The push is now refused, and the + message names `git fetch --unshallow`. A shallow clone still pushes changes that lie wholly above its cut. The push + limits and the change's secret scan read one checked copy of the shallow file, so changing it while they run + changes nothing, and a shallow file with a line git might read differently is refused. +- **The commit-graph is off for every git the runner starts (#927):** a forged commit-graph could make a walk skip a + commit; git now reads each commit's own parents (measured: about 30 ms more for three walks over 2,900 commits). +- **The patch hash (#927)** covers a file's lines even when the change's `.gitattributes` marks it `-diff` or + `binary`. The owner's decision diff and the fix-forward culprit diff are as before: a binary file shows as + "Binary files ... differ". diff --git a/scripts/bench/grade.ts b/scripts/bench/grade.ts index 1d17b8383..742f6f5b0 100644 --- a/scripts/bench/grade.ts +++ b/scripts/bench/grade.ts @@ -4,10 +4,10 @@ // graded is exactly what was exported. It then writes SWE-bench predictions (one set per label: each needs its own // harness run id) into the held-out store, the only place a task key meets its instance id. The official harness // grades them in fresh containers from the pristine images, after the whole run. -import { execFileSync } from 'node:child_process'; import { lstatSync, mkdtempSync, readdirSync, readFileSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; +import { gitExecFileSync } from '../../src/os/git.js'; import { ownerOnlyDir, writeOwnerOnly } from '../../src/os/index.js'; import type { BenchLayout } from './paths.js'; import { readManifest, TASK_KEY, writeStore } from './store.js'; @@ -45,8 +45,8 @@ export function exportPatch(o: { layout: BenchLayout; runId: string; key: string } const bare = mkdtempSync(join(tmpdir(), 'bench-export-')); try { - execFileSync('git', ['init', '--bare', '-q', '--template=', bare], { env: GIT_ENV(), timeout: 60_000 }); - const diff = execFileSync('git', [...GIT_HARDENED, '--git-dir', bare, 'diff', '--binary', '--no-color', '--no-ext-diff', '--no-textconv', '--src-prefix=a/', '--dst-prefix=b/', o.base, o.head, '--'], { + gitExecFileSync(['init', '--bare', '-q', '--template=', bare], { env: GIT_ENV(), timeout: 60_000 }); + const diff = gitExecFileSync([...GIT_HARDENED, '--git-dir', bare, 'diff', '--binary', '--no-color', '--no-ext-diff', '--no-textconv', '--src-prefix=a/', '--dst-prefix=b/', o.base, o.head, '--'], { env: GIT_ENV({ GIT_ALTERNATE_OBJECT_DIRECTORIES: objects }), encoding: 'utf8', maxBuffer: 64 * 1024 * 1024, diff --git a/scripts/bench/pull.ts b/scripts/bench/pull.ts index d82e5b351..3bde6df07 100644 --- a/scripts/bench/pull.ts +++ b/scripts/bench/pull.ts @@ -40,11 +40,11 @@ // What is printed: keys and counts only, never an instance id, an image name, a digest beside its key or a tree hash. // docker's own words (they name the image) are never passed on: a failure says its stage and exit status. Every line // still goes through cli.ts scrub()'s withholding backstop. -import { execFileSync } from 'node:child_process'; import { randomBytes, createHash } from 'node:crypto'; import { closeSync, constants, fstatSync, lstatSync, mkdirSync, openSync, readSync, renameSync, rmdirSync, rmSync, type Stats } from 'node:fs'; import { join } from 'node:path'; import { createInflate, inflateSync } from 'node:zlib'; +import { gitExecFileSync } from '../../src/os/git.js'; import type { Mode } from './caps.js'; import { LimitsNotApplied, OutputTooLarge, TaskRunner, WORK, type Docker, type DockerOptions, type TaskContainer } from './container.js'; import { pullImage, PullRefused, writeVerified } from './download.js'; @@ -287,8 +287,10 @@ const GIT_ENV: NodeJS.ProcessEnv = { PATH: process.env.PATH, GIT_CONFIG_NOSYSTEM // objects, so their memory grew with the host's CPUs: an admitted 12.5 MB pack peaked index-pack at 0.86 to 1.96 GB // with git's default threads on 16 CPUs, 3.27 GB with 16, and 283 MiB with one (#947 round 4, measured in review). const GIT_HARDENED = ['-c', 'core.fsmonitor=false', '-c', 'core.hooksPath=/dev/null', '-c', 'protocol.allow=never', '-c', 'protocol.file.allow=always', '-c', 'pack.threads=1']; +// Through the engine's git runner (#807): it sets replace refs, grafts and the commit-graph off last over GIT_ENV +// (gitExecFileSync applies gitEnv to the env it is handed). const git = (args: readonly string[], input?: Buffer): string => - execFileSync('git', [...GIT_HARDENED, ...args], { env: GIT_ENV, stdio: ['pipe', 'pipe', 'pipe'], timeout: 30 * 60_000, maxBuffer: 256 * 1024 * 1024, ...(input ? { input } : {}) }) + gitExecFileSync([...GIT_HARDENED, ...args], { env: GIT_ENV, stdio: ['pipe', 'pipe', 'pipe'], timeout: 30 * 60_000, maxBuffer: 256 * 1024 * 1024, ...(input ? { input } : {}) }) .toString('utf8') .trim(); diff --git a/scripts/check-dco.mjs b/scripts/check-dco.mjs index d9a092842..d58e661e6 100644 --- a/scripts/check-dco.mjs +++ b/scripts/check-dco.mjs @@ -2,7 +2,7 @@ // Fail if any commit in .. lacks a Signed-off-by trailer that // matches its author (Developer Certificate of Origin, see CONTRIBUTING.md). // node scripts/check-dco.mjs [head] -import { execFileSync } from 'node:child_process'; +import { gitExecFileSync } from './lib/git.cjs'; const [base, head = 'HEAD'] = process.argv.slice(2); if (!base) { @@ -11,8 +11,7 @@ if (!base) { } const SEP = '\x1e'; -const log = execFileSync( - 'git', +const log = gitExecFileSync( ['log', '--no-merges', `--format=%H%x1f%an <%ae>%x1f%B${SEP}`, `${base}..${head}`], { encoding: 'utf8' }, ); diff --git a/scripts/check-denylist.mjs b/scripts/check-denylist.mjs index 3f248651d..74adcc65a 100644 --- a/scripts/check-denylist.mjs +++ b/scripts/check-denylist.mjs @@ -3,7 +3,7 @@ // // node scripts/check-denylist.mjs scan tracked + new files // node scripts/check-denylist.mjs --hash t print the hash entry for term t -import { execFileSync } from 'node:child_process'; +import { gitExecFileSync } from './lib/git.cjs'; import { readFileSync } from 'node:fs'; import { join } from 'node:path'; import { loadRules, scanText, sha, SKIP } from './lib/denylist.mjs'; @@ -15,7 +15,7 @@ if (args[0] === '--hash') { } const root = args[0] ?? '.'; const rules = loadRules(root); -const files = execFileSync('git', ['ls-files', '-co', '--exclude-standard'], { cwd: root, encoding: 'utf8' }) +const files = gitExecFileSync(['ls-files', '-co', '--exclude-standard'], { cwd: root, encoding: 'utf8' }) .split('\n') .filter((f) => f && !SKIP.has(f)); diff --git a/scripts/ci/affected.mjs b/scripts/ci/affected.mjs index 63c377b06..9198ec5c7 100644 --- a/scripts/ci/affected.mjs +++ b/scripts/ci/affected.mjs @@ -8,7 +8,7 @@ // node scripts/ci/affected.mjs --event pull_request --base the selected test files (test/x.test.ts), one per line // node scripts/ci/affected.mjs --event push every test file (any event but pull_request) // add --explain for why each one was selected (on stderr) -import { execFileSync } from 'node:child_process'; +import { gitExecFileSync } from '../lib/git.cjs'; import { readFileSync } from 'node:fs'; import { join } from 'node:path'; import { fileURLToPath, pathToFileURL } from 'node:url'; @@ -98,7 +98,7 @@ export async function plan(dir, changed, added = new Set()) { return a.selectTests(g, changed, { isTest, forceAll, added }); } -const git = (dir, ...args) => execFileSync('git', args, { cwd: dir, encoding: 'utf8', maxBuffer: 64 << 20 }).split('\0').filter(Boolean); +const git = (dir, ...args) => gitExecFileSync(args, { cwd: dir, encoding: 'utf8', maxBuffer: 64 << 20 }).split('\0').filter(Boolean); function arg(args, name) { const i = args.indexOf(name); diff --git a/scripts/ci/installed-engine-config.mjs b/scripts/ci/installed-engine-config.mjs index 6f7223fa5..28cedf579 100644 --- a/scripts/ci/installed-engine-config.mjs +++ b/scripts/ci/installed-engine-config.mjs @@ -40,7 +40,8 @@ // A change that touches neither .worklane/ nor the pin has nothing to load: it prints so and exits 0, before fetching // any engine (the workflow's paths filter already skips most such PRs; this keeps the script right without it). // Exit 1: refused, or it couldn't check. -import { execFileSync, spawnSync } from 'node:child_process'; +import { spawnSync } from 'node:child_process'; +import { gitExecFileSync, gitSpawnSync } from '../lib/git.cjs'; import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { dirname, join } from 'node:path'; @@ -77,7 +78,7 @@ class Refused extends Error {} const fail = (msg) => { throw new Refused(msg); }; -const git = (...a) => spawnSync('git', a, { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], maxBuffer: 64 * 1024 * 1024 }); +const git = (...a) => gitSpawnSync(a, { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], maxBuffer: 64 * 1024 * 1024 }); const gitText = (...a) => { const r = git(...a); return r.status === 0 ? r.stdout.trim() : null; @@ -442,7 +443,7 @@ async function loadWith({ commit, label: engine }, root, head) { const dir = mkdtempSync(join(tmpdir(), 'installed-engine-')); try { try { - const tar = execFileSync('git', ['archive', '--format=tar', commit, 'dist/src'], { maxBuffer: 512 * 1024 * 1024, stdio: ['ignore', 'pipe', 'pipe'] }); + const tar = gitExecFileSync(['archive', '--format=tar', commit, 'dist/src'], { maxBuffer: 512 * 1024 * 1024, stdio: ['ignore', 'pipe', 'pipe'] }); const x = spawnSync('tar', ['-x', '-C', dir], { input: tar, stdio: ['pipe', 'ignore', 'pipe'] }); if (x.status !== 0) throw new Error(lastLine(x.stderr) || `tar exited ${x.status}`); } catch (e) { diff --git a/scripts/ci/new-deps.mjs b/scripts/ci/new-deps.mjs index 33a20022a..5a137e448 100644 --- a/scripts/ci/new-deps.mjs +++ b/scripts/ci/new-deps.mjs @@ -16,6 +16,8 @@ // else from the event payload at $GITHUB_EVENT_PATH (pull_request.body). Exit 1 with each problem when any. import { execFileSync } from 'node:child_process'; import { readFileSync } from 'node:fs'; +import { devNull } from 'node:os'; +import { join } from 'node:path'; import { pathToFileURL } from 'node:url'; export const SPEC_FIELDS = ['dependencies', 'optionalDependencies', 'peerDependencies']; @@ -142,12 +144,18 @@ export function check({ basePkg, headPkg, baseLock, headLock, body }) { return { changed: [...changed].map(([name, c]) => ({ name, versions: [...c.versions], licences: [...c.licences], why: c.why })), problems }; } -const git = (...a) => execFileSync('git', a, { encoding: 'utf8', maxBuffer: 256 * 1024 * 1024 }); +// The git runner's settings (#807; src/os/git.ts, scripts/lib/git.cjs): replace refs off and no graft file. Written +// out here because CI runs the base's copy of this file alone, from a temp dir (new-deps.yml), so it imports nothing +// of the repo's; test/git-runner.test.ts checks they are the runner's. +const GIT_FIXED_ARGS = ['--no-replace-objects', '-c', 'core.commitGraph=false']; +const GIT_ENV = { ...process.env, GIT_NO_REPLACE_OBJECTS: '1', GIT_GRAFT_FILE: join(devNull, 'no-grafts') }; +const runGit = (args, opts) => execFileSync('git', [...GIT_FIXED_ARGS, ...args], { ...opts, env: GIT_ENV }); +const git = (...a) => runGit(a, { encoding: 'utf8', maxBuffer: 256 * 1024 * 1024 }); /** A JSON file at a revision; null when the revision doesn't have it. */ function jsonAt(rev, path) { let text; try { - text = execFileSync('git', ['show', `${rev}:${path}`], { encoding: 'utf8', maxBuffer: 256 * 1024 * 1024, stdio: ['ignore', 'pipe', 'ignore'] }); + text = runGit(['show', `${rev}:${path}`], { encoding: 'utf8', maxBuffer: 256 * 1024 * 1024, stdio: ['ignore', 'pipe', 'ignore'] }); } catch { git('rev-parse', '--verify', '--quiet', `${rev}^{commit}`); // a bad revision fails loudly; a missing file is null return null; diff --git a/scripts/ci/testrun-real-systemd.mjs b/scripts/ci/testrun-real-systemd.mjs index 62e61ff36..d049db582 100644 --- a/scripts/ci/testrun-real-systemd.mjs +++ b/scripts/ci/testrun-real-systemd.mjs @@ -14,7 +14,7 @@ import assert from 'node:assert/strict'; import { execFileSync, spawnSync } from 'node:child_process'; import { appendFileSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, writeFileSync } from 'node:fs'; import { connect } from 'node:net'; -import { homedir } from 'node:os'; +import { devNull, homedir } from 'node:os'; import { join } from 'node:path'; import { pathToFileURL } from 'node:url'; @@ -56,7 +56,11 @@ const socketFromHere = (path) => new Promise((resolve) => { // A repo of this user's own: main, and a head commit on top of it. const repo = mkdtempSync(join(homedir(), 'repo-')); -const git = (...a) => execFileSync('git', ['-C', repo, '-c', 'user.name=ci', '-c', 'user.email=ci@example.com', ...a], { encoding: 'utf8', env: { PATH: '/usr/bin:/bin', HOME: homedir(), GIT_CONFIG_GLOBAL: '/dev/null', GIT_CONFIG_NOSYSTEM: '1' } }).trim(); +// The git runner's settings (#807; src/os/git.ts, scripts/lib/git.cjs): replace refs off and no graft file. Written +// out here because the job copies this file alone into the VM (ci-slow.yml); test/git-runner.test.ts checks them. +const GIT_FIXED_ARGS = ['--no-replace-objects', '-c', 'core.commitGraph=false']; +const GIT_ENV = { PATH: '/usr/bin:/bin', HOME: homedir(), GIT_CONFIG_GLOBAL: '/dev/null', GIT_CONFIG_NOSYSTEM: '1', GIT_NO_REPLACE_OBJECTS: '1', GIT_GRAFT_FILE: join(devNull, 'no-grafts') }; +const git = (...a) => execFileSync('git', [...GIT_FIXED_ARGS, '-C', repo, '-c', 'user.name=ci', '-c', 'user.email=ci@example.com', ...a], { encoding: 'utf8', env: GIT_ENV }).trim(); git('init', '-q', '-b', 'main'); writeFileSync(join(repo, 'a.txt'), 'main\n'); mkdirSync(join(repo, 'art')); diff --git a/scripts/ci/updater-setup-root.mjs b/scripts/ci/updater-setup-root.mjs index 8fea55144..b92cb1cf0 100644 --- a/scripts/ci/updater-setup-root.mjs +++ b/scripts/ci/updater-setup-root.mjs @@ -26,6 +26,7 @@ import { existsSync, mkdirSync, mkdtempSync, readFileSync, readlinkSync, rmSync, import { createRequire } from 'node:module'; import { join } from 'node:path'; import { pathToFileURL } from 'node:url'; +import { commandExecFileSync, gitExecFileSync } from '../lib/git.cjs'; assert.equal(process.getuid(), 0, 'run this as root'); const coordinator = process.argv[2]; @@ -58,7 +59,8 @@ const owner = (f) => { const sha = (f) => (existsSync(f) ? createHash('sha256').update(readFileSync(f)).digest('hex') : 'absent'); const changes = () => readFileSync(CHANGES, 'utf8').trim().split('\n').map((l) => JSON.parse(l)); const pending = () => JSON.parse(readFileSync(PENDING, 'utf8')).pending; -const sh = (file, args, cwd = '/') => execFileSync(file, args, { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); +// A caller's git (the command named git) through the git runner (#807). +const sh = (file, args, cwd = '/') => commandExecFileSync(file, args, { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); /** One run of the updater's unit, as its timer starts it; when it fails, its own journal says why. */ const updaterRun = () => { // This proof starts the unit many times in a few seconds, past systemd's default start limit (5 in 10 s); its timer @@ -164,7 +166,7 @@ release('v1.0.2', other.file); // The installed updater's own check, as root, against the machine's signers file. const updater = createRequire(import.meta.url)(UPDATER); let n = 0; -const verify = (tag) => updater.verifyRelease(origin, tag, SIGNERS, (file, args) => execFileSync(file, args, { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }), join(work, `verify-${++n}.git`)); +const verify = (tag) => updater.verifyRelease(origin, tag, SIGNERS, (file, args) => commandExecFileSync(file, args, { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }), join(work, `verify-${++n}.git`)); // ---- every other argument list: refused, and nothing changes const k = other.pub; @@ -405,9 +407,9 @@ const deps = { installed: () => installedSha, installedVersion: () => '1.0.2', // The release tags and their check: real, against the local repo, as root. - verifyRelease: (_url, tag, signers) => updater.verifyRelease(origin, tag, signers, (file, args) => execFileSync(file, args, { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }), join(work, `run-verify-${++n}.git`)), + verifyRelease: (_url, tag, signers) => updater.verifyRelease(origin, tag, signers, (file, args) => commandExecFileSync(file, args, { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }), join(work, `run-verify-${++n}.git`)), exec: (file, args) => { - if (file === 'git' && args[0] === 'ls-remote') return execFileSync('git', ['ls-remote', '--tags', origin, 'refs/tags/v*'], { encoding: 'utf8' }); + if (file === 'git' && args[0] === 'ls-remote') return gitExecFileSync(['ls-remote', '--tags', origin, 'refs/tags/v*'], { encoding: 'utf8' }); if (file === 'systemctl' && args[0] === 'list-units') return 'worklane-ci-standin.service loaded active running x\n'; if (file === 'systemctl' && args[0] === 'show') return 'ActiveState=active\nSubState=running\nNRestarts=0\nResult=success\n'; runLogs.push({ exec: [file, ...args].join(' ').slice(0, 80) }); diff --git a/scripts/ci/wizard-e2e.mjs b/scripts/ci/wizard-e2e.mjs index 44ca0383b..9b5e8f704 100644 --- a/scripts/ci/wizard-e2e.mjs +++ b/scripts/ci/wizard-e2e.mjs @@ -17,10 +17,10 @@ // browser asked for) and /automation.json (each step's "Without this page" line as the page showed it), for // the job's leak check. Nothing secret is ever printed: the key goes into the page from its file, never typed here. import assert from 'node:assert/strict'; -import { spawnSync } from 'node:child_process'; import { existsSync, mkdirSync, readFileSync, statSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import { pathToFileURL } from 'node:url'; +import { commandSpawnSync } from '../lib/git.cjs'; const [mode, dist, wizardUrl, keyFile, out, extra] = process.argv.slice(2); if (!['local', 'server', 'setup-loopback'].includes(mode) || !dist || !wizardUrl || !keyFile || !out || !extra) { @@ -116,8 +116,9 @@ async function firstRun(fresh = false) { return r.ok ? r.json() : { error: r.status }; })()`); } +// A caller's git (the command named git) through the git runner (#807). const sh = (cmd, args, o = {}) => { - const r = spawnSync(cmd, args, { encoding: 'utf8', ...o }); + const r = commandSpawnSync(cmd, args, { encoding: 'utf8', ...o }); return { status: r.status, out: (r.stdout ?? '').trim(), err: (r.stderr ?? '').trim() }; }; /** A file's owner and octal mode (through sudo on a server: the files are other users'). */ diff --git a/scripts/dev/compare-screenshots.mjs b/scripts/dev/compare-screenshots.mjs index 58aae4032..4a3df8edc 100644 --- a/scripts/dev/compare-screenshots.mjs +++ b/scripts/dev/compare-screenshots.mjs @@ -7,11 +7,11 @@ // Writes /before/*.png and /after/*.png (light and dark per page). // Stopped (Ctrl-C, a signal) or failing partway, it stops the step it is running, whose own clean-up // stops what that step started (the screenshot script's dashboards and Chrome), and removes its clones. -import { spawn, spawnSync } from 'node:child_process'; import { mkdtempSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; +import { commandSpawn, gitSpawnSync } from '../lib/git.cjs'; import { exitCleanup } from './children.mjs'; const arg = (name, fallback) => { @@ -24,11 +24,12 @@ const cleanup = exitCleanup(); const run = (cmd, args, cwd) => new Promise((ok, fail) => { // npm is npm.cmd on Windows, which only a shell resolves. - const p = cleanup.track(spawn(cmd, args, { cwd, stdio: 'inherit', shell: process.platform === 'win32' && cmd === 'npm' })); + // A caller's git (the command named git) through the git runner (#807). + const p = cleanup.track(commandSpawn(cmd, args, { cwd, stdio: 'inherit', shell: process.platform === 'win32' && cmd === 'npm' })); p.on('error', fail); p.on('exit', (code, signal) => (code === 0 ? ok() : fail(new Error(`${cmd} ${args.join(' ')} failed in ${cwd}${signal ? ` (${signal})` : ''}`)))); }); -const git = (...args) => spawnSync('git', args, { cwd: repo, encoding: 'utf8' }).stdout.trim(); +const git = (...args) => gitSpawnSync(args, { cwd: repo, encoding: 'utf8' }).stdout.trim(); await run('git', ['fetch', '-q', 'origin'], repo); const base = arg('--base', 'origin/main'); diff --git a/scripts/eval-roles.mjs b/scripts/eval-roles.mjs index ed8437452..df4a7b521 100644 --- a/scripts/eval-roles.mjs +++ b/scripts/eval-roles.mjs @@ -32,7 +32,7 @@ // [--fix-cap 10] // --write-held-out: write a held-out set for each role that has none (one case-writer run per role, from its // contract only); --score: score the current prompts once, no base (no --runs, no --fix). -import { execFileSync } from 'node:child_process'; +import { gitExecFileSync } from './lib/git.cjs'; import { existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { dirname, join } from 'node:path'; @@ -92,7 +92,7 @@ let noBase = false; if (!o.scoreOnly) { let baseSource; try { - baseSource = execFileSync('git', ['show', `${o.base}:src/roles.ts`], { cwd: root, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }); + baseSource = gitExecFileSync(['show', `${o.base}:src/roles.ts`], { cwd: root, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }); } catch { console.error(`no src/roles.ts at ${o.base}`); // Without --spend the plan still prints (and the run stops there anyway). diff --git a/scripts/lib/git.cjs b/scripts/lib/git.cjs new file mode 100644 index 000000000..82d802c51 --- /dev/null +++ b/scripts/lib/git.cjs @@ -0,0 +1,75 @@ +'use strict'; +// The one way scripts/ runs git (#807): the engine runner's settings (src/os/git.ts) for scripts, which run without a +// build. Every git process runs with replace refs off (`--no-replace-objects`, GIT_NO_REPLACE_OBJECTS=1) and reads no +// graft file (GIT_GRAFT_FILE names a path that can't exist), set last over the caller's environment. +// test/git-runner.test.ts checks these are the engine's settings, and fails on a git call in scripts/ that doesn't +// come through here. CommonJS, so a .cjs script can require it and a .mjs script can import it. +const { execFileSync, spawn, spawnSync } = require('node:child_process'); +const { devNull } = require('node:os'); +const { join } = require('node:path'); + +const GIT_FIXED_ARGS = Object.freeze(['--no-replace-objects', '-c', 'core.commitGraph=false']); +const GIT_FIXED_ENV = Object.freeze({ GIT_NO_REPLACE_OBJECTS: '1', GIT_GRAFT_FILE: join(devNull, 'no-grafts') }); +/** git config every git also gets through its environment (GIT_CONFIG_COUNT pairs), after the caller's own pairs. */ +const GIT_FIXED_CONFIG = Object.freeze([Object.freeze(['core.commitGraph', 'false'])]); + +/** The only GIT_CONFIG_COUNT read: a plain decimal 0 to 9999. Any other becomes a value git refuses. */ +const CANONICAL_CONFIG_COUNT = /^(?:0|[1-9][0-9]{0,3})$/; +const REFUSED_CONFIG_COUNT = 'refused'; + +/** + * `env` (or this process's environment), with the fixed settings on top, and the fixed config after the caller's own + * GIT_CONFIG_COUNT pairs and at the end of GIT_CONFIG_PARAMETERS (which git reads last). A count that isn't canonical + * is replaced with one git refuses, so the git fails closed (as src/os/git.ts gitEnv). + */ +const gitEnv = (env) => { + const out = { ...(env || process.env), ...GIT_FIXED_ENV }; + // Own keys only: an unset value must never be read through Object.prototype (#846). + const own = (k) => (Object.hasOwn(out, k) ? out[k] : undefined); + const raw = own('GIT_CONFIG_COUNT'); + if (raw === undefined || CANONICAL_CONFIG_COUNT.test(raw)) { + let n = raw === undefined ? 0 : Number(raw); + for (const [k, v] of GIT_FIXED_CONFIG) { + out[`GIT_CONFIG_KEY_${n}`] = k; + out[`GIT_CONFIG_VALUE_${n}`] = v; + n++; + } + out.GIT_CONFIG_COUNT = String(n); + } else out.GIT_CONFIG_COUNT = REFUSED_CONFIG_COUNT; + const params = own('GIT_CONFIG_PARAMETERS'); + const fixed = GIT_FIXED_CONFIG.map(([k, v]) => `'${k}=${v}'`).join(' '); + out.GIT_CONFIG_PARAMETERS = params ? `${params} ${fixed}` : fixed; + return out; +}; +/** The argument list a git process gets: the fixed arguments, then `args`. */ +const gitArgs = (args) => [...GIT_FIXED_ARGS, ...args]; +/** True for a command that runs git: `git`, or a path to it, in any case. */ +const isGitCommand = (cmd) => ['git', 'git.exe'].includes((String(cmd).split(/[\\/]/).pop() || '').toLowerCase()); + +/** spawnSync of git. */ +const gitSpawnSync = (args, options = {}) => spawnSync('git', gitArgs(args), { ...options, env: gitEnv(options.env) }); +/** execFileSync of git. */ +const gitExecFileSync = (args, options = {}) => execFileSync('git', gitArgs(args), { ...options, env: gitEnv(options.env) }); +/** spawn of git. */ +const gitSpawn = (args, options = {}) => spawn('git', gitArgs(args), { ...options, env: gitEnv(options.env) }); +/** spawnSync of a command named at run time: git (by name or path) with the settings, anything else as it is. */ +const commandSpawnSync = (cmd, args, options = {}) => (isGitCommand(cmd) ? spawnSync(cmd, gitArgs(args), { ...options, env: gitEnv(options.env) }) : spawnSync(cmd, args, options)); +/** execFileSync of a command named at run time: git (by name or path) with the settings, anything else as it is. */ +const commandExecFileSync = (cmd, args, options = {}) => (isGitCommand(cmd) ? execFileSync(cmd, gitArgs(args), { ...options, env: gitEnv(options.env) }) : execFileSync(cmd, args, options)); +/** spawn of a command named at run time: git (by name or path) with the settings, anything else as it is. */ +const commandSpawn = (cmd, args, options = {}) => (isGitCommand(cmd) ? spawn(cmd, gitArgs(args), { ...options, env: gitEnv(options.env) }) : spawn(cmd, args, options)); + +exports.GIT_FIXED_ARGS = GIT_FIXED_ARGS; +exports.GIT_FIXED_ENV = GIT_FIXED_ENV; +exports.GIT_FIXED_CONFIG = GIT_FIXED_CONFIG; +exports.CANONICAL_CONFIG_COUNT = CANONICAL_CONFIG_COUNT; +exports.REFUSED_CONFIG_COUNT = REFUSED_CONFIG_COUNT; +exports.gitEnv = gitEnv; +exports.gitArgs = gitArgs; +exports.isGitCommand = isGitCommand; +exports.gitSpawnSync = gitSpawnSync; +exports.gitExecFileSync = gitExecFileSync; +exports.gitSpawn = gitSpawn; +exports.commandSpawnSync = commandSpawnSync; +exports.commandExecFileSync = commandExecFileSync; +exports.commandSpawn = commandSpawn; diff --git a/scripts/lib/release-tree.mjs b/scripts/lib/release-tree.mjs index aafa674ab..8c8c78979 100644 --- a/scripts/lib/release-tree.mjs +++ b/scripts/lib/release-tree.mjs @@ -2,7 +2,7 @@ // dist/web/licenses.txt that wasn't the signed one, and it was published.) Compared by git blob id: a file's raw bytes // hashed as `git hash-object --no-filters` does, against the id `git ls-tree` lists for that path in the commit. No // clean filter, no line-ending conversion: equal ids mean equal bytes. Modes are not compared (npm normalises them). -import { execFileSync } from 'node:child_process'; +import { gitExecFileSync } from './git.cjs'; import { createHash } from 'node:crypto'; import { readdirSync, readFileSync } from 'node:fs'; import { join, relative, sep } from 'node:path'; @@ -31,7 +31,7 @@ export function filesUnder(base, dir = base) { /** The commit's files under `prefixes`: path -> blob id. */ export function committedFiles(commit, prefixes, cwd = process.cwd()) { - const raw = execFileSync('git', ['ls-tree', '-r', '-z', '--full-tree', commit, '--', ...prefixes], { cwd, encoding: 'utf8', maxBuffer: 256 * 1024 * 1024 }); + const raw = gitExecFileSync(['ls-tree', '-r', '-z', '--full-tree', commit, '--', ...prefixes], { cwd, encoding: 'utf8', maxBuffer: 256 * 1024 * 1024 }); const files = new Map(); for (const rec of raw.split('\0').filter(Boolean)) { const [meta, path] = [rec.slice(0, rec.indexOf('\t')), rec.slice(rec.indexOf('\t') + 1)]; diff --git a/scripts/pin-engine.mjs b/scripts/pin-engine.mjs index b14e63414..b7f0597fb 100644 --- a/scripts/pin-engine.mjs +++ b/scripts/pin-engine.mjs @@ -10,7 +10,7 @@ // - its commit is a release commit: one commit on top of a commit on origin/main, changing only dist/, carrying // dist/src/cli.js, with package.json at X.Y.Z (the shape scripts/release.mjs makes); // then writes ENGINE_TAG=vX.Y.Z and ENGINE_COMMIT= into scripts/bootstrap.sh, and nothing else. -import { execFileSync } from 'node:child_process'; +import { gitExecFileSync } from './lib/git.cjs'; import { readFileSync, writeFileSync } from 'node:fs'; import { readPin, releaseProblem, writePin } from './lib/engine-pin.mjs'; import { canVerify } from './lib/signing.mjs'; @@ -28,7 +28,7 @@ const fail = (msg) => { if (!/^v\d+\.\d+\.\d+$/.test(tag ?? '')) fail('usage: pin-engine.mjs vX.Y.Z [--allowed-signers ] [--bootstrap ]'); const file = opt('bootstrap') ?? 'scripts/bootstrap.sh'; const signers = opt('allowed-signers'); -const git = (...a) => execFileSync('git', a, { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); +const git = (...a) => gitExecFileSync(a, { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); const ok = (...a) => { try { git(...a); @@ -69,7 +69,7 @@ const facts = { parents: parents.length, // --no-renames: a file moved into dist/ still names the path it left. -z, untrimmed: a valid UTF-8 name as itself; // any other has its bad bytes read as U+FFFD, so it starts with `dist/` only if its bytes do (#683). - changed: parents.length === 1 ? execFileSync('git', ['diff', '--name-only', '--no-renames', '-z', parents[0], commit], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).split('\0').filter(Boolean) : [], + changed: parents.length === 1 ? gitExecFileSync(['diff', '--name-only', '--no-renames', '-z', parents[0], commit], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).split('\0').filter(Boolean) : [], hasCli: ok('cat-file', '-e', `${commit}:dist/src/cli.js`), version: (() => { try { diff --git a/scripts/push-gate-hook.mjs b/scripts/push-gate-hook.mjs index 00fa87e11..e9575ef2d 100644 --- a/scripts/push-gate-hook.mjs +++ b/scripts/push-gate-hook.mjs @@ -18,7 +18,7 @@ // it is refused inside an agent run, and each use prints a loud line and is appended to HATCH_LOG in the git dir. import { execFileSync, spawnSync } from 'node:child_process'; import { appendFileSync, chmodSync, copyFileSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; -import { tmpdir, userInfo } from 'node:os'; +import { devNull, tmpdir, userInfo } from 'node:os'; import { dirname, join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -40,8 +40,19 @@ class Refusal extends Error {} /** A refusal that first-time setup may answer with PUSH_GATE_BOOTSTRAP=1 (origin doesn't hold the gate yet). */ class NotOnOrigin extends Refusal {} +/** + * The git runner's settings (#807; src/os/git.ts, scripts/lib/git.cjs): replace refs off and no graft file, over the + * caller's environment. Written out here because this file runs on its own (setup-hooks copies it alone into the + * hooks dir), so it can't import them; test/git-runner.test.ts checks they are the runner's. The gate gets them too, + * in its environment, so every git it and gitleaks run has them, whichever gate origin holds. + */ +export const GIT_FIXED_ARGS = Object.freeze(['--no-replace-objects', '-c', 'core.commitGraph=false']); +export const GIT_FIXED_ENV = Object.freeze({ GIT_NO_REPLACE_OBJECTS: '1', GIT_GRAFT_FILE: join(devNull, 'no-grafts') }); +/** execFileSync of git, with the settings. Every git this file runs comes through here. */ +const runGit = (args, opts = {}) => execFileSync('git', [...GIT_FIXED_ARGS, ...args], { ...opts, env: { ...(opts.env || process.env), ...GIT_FIXED_ENV } }); + function git(args, opts = {}) { - return execFileSync('git', args, { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], maxBuffer: 64 * 1024 * 1024, ...opts }); + return runGit(args, { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], maxBuffer: 64 * 1024 * 1024, ...opts }); } const tryGit = (args, opts) => { try { @@ -93,7 +104,7 @@ function extractFromRev(rev, label, dir) { throw new NotOnOrigin(`${label} has no ${path}.`); } mkdirSync(dirname(join(dir, path)), { recursive: true }); - writeFileSync(join(dir, path), execFileSync('git', ['show', `${rev}:${path}`], { maxBuffer: 64 * 1024 * 1024 })); + writeFileSync(join(dir, path), runGit(['show', `${rev}:${path}`], { maxBuffer: 64 * 1024 * 1024 })); } } @@ -229,7 +240,7 @@ function main(argv) { if (hookMode) noteIfHookIsOld(); // WHY: the gate reads its rules (and gitleaks its config) from the directory it runs in, so it runs in the // private copy; GIT_DIR points its git commands at this repo, and nothing else of the checkout is in reach. - const env = { ...process.env, GIT_DIR: gitDir }; + const env = { ...process.env, GIT_DIR: gitDir, ...GIT_FIXED_ENV }; for (const k of ['GIT_WORK_TREE', 'GIT_INDEX_FILE', 'GIT_PREFIX']) delete env[k]; const r = spawnSync(process.execPath, [join(dir, GATE_CODE[0]), ...argv], { cwd: dir, stdio: 'inherit', env }); if (r.error) throw r.error; diff --git a/scripts/push-gate.mjs b/scripts/push-gate.mjs index 4e72d9842..0d6ad3a57 100644 --- a/scripts/push-gate.mjs +++ b/scripts/push-gate.mjs @@ -19,10 +19,31 @@ // email (a squash merge's DCO trailers) is dropped whole, name included. import { execFileSync, spawnSync } from 'node:child_process'; import { readFileSync } from 'node:fs'; +import { devNull } from 'node:os'; +import { join } from 'node:path'; import { loadRules, scanText, SKIP } from './lib/denylist.mjs'; const ZERO = /^0+$/; -const git = (...a) => execFileSync('git', a, { encoding: 'utf8', maxBuffer: 256 * 1024 * 1024 }); +// The git runner's settings (#807; src/os/git.ts, scripts/lib/git.cjs): replace refs off and no graft file, so a +// `git replace` or a graft in the repo can't swap the commits, trees or blobs scanned here. Written out here because +// the hook runs this file alone, fetched from origin (scripts/push-gate-hook.mjs GATE_CODE), so it imports nothing +// new; test/git-runner.test.ts checks they are the runner's. +// The commit-graph is off too (a forged one can skip a commit), in the arguments and, for gitleaks' own gits, in the +// environment: a config pair after the caller's GIT_CONFIG_COUNT pairs, and at the end of GIT_CONFIG_PARAMETERS +// (what `git -c` hands the gits it runs), which git reads last. A count that isn't a plain decimal 0 to 9999 is +// replaced with one git refuses, so the git fails closed (as src/os/git.ts gitEnv). +const GIT_FIXED_ARGS = ['--no-replace-objects', '-c', 'core.commitGraph=false']; +const graphOff = (e) => { + const raw = Object.hasOwn(e, 'GIT_CONFIG_COUNT') ? e.GIT_CONFIG_COUNT : undefined; + const params = Object.hasOwn(e, 'GIT_CONFIG_PARAMETERS') ? e.GIT_CONFIG_PARAMETERS : undefined; + const last = { GIT_CONFIG_PARAMETERS: params ? `${params} 'core.commitGraph=false'` : "'core.commitGraph=false'" }; + if (raw !== undefined && !/^(?:0|[1-9][0-9]{0,3})$/.test(raw)) return { ...e, GIT_CONFIG_COUNT: 'refused', ...last }; + const n = raw === undefined ? 0 : Number(raw); + return { ...e, [`GIT_CONFIG_KEY_${n}`]: 'core.commitGraph', [`GIT_CONFIG_VALUE_${n}`]: 'false', GIT_CONFIG_COUNT: String(n + 1), ...last }; +}; +const GIT_ENV = graphOff({ ...process.env, GIT_NO_REPLACE_OBJECTS: '1', GIT_GRAFT_FILE: join(devNull, 'no-grafts') }); +const runGit = (args, opts) => execFileSync('git', [...GIT_FIXED_ARGS, ...args], { ...opts, env: GIT_ENV }); +const git = (...a) => runGit(a, { encoding: 'utf8', maxBuffer: 256 * 1024 * 1024 }); function fail(why) { console.error(`push gate: REFUSED. ${why}`); @@ -69,7 +90,7 @@ function repoOwner() { if (fromEnv) return fromEnv[1].toLowerCase(); let url; try { - url = execFileSync('git', ['remote', 'get-url', 'origin'], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] }).trim(); + url = runGit(['remote', 'get-url', 'origin'], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] }).trim(); } catch { return null; // no origin: no allowance } @@ -181,7 +202,8 @@ function scanTree(rules, tip) { function gitleaks(commits) { if (!commits.length) return; - const r = spawnSync('gitleaks', ['git', '--no-banner', '--redact', '--exit-code', '1', '--log-opts', `--no-walk ${commits.join(' ')}`, '.'], { encoding: 'utf8' }); + // gitleaks runs git itself: with the same settings, in its environment. + const r = spawnSync('gitleaks', ['git', '--no-banner', '--redact', '--exit-code', '1', '--log-opts', `--no-walk ${commits.join(' ')}`, '.'], { encoding: 'utf8', env: GIT_ENV }); if (r.error) fail(`gitleaks could not run (${r.error.code ?? r.error.message}); install it, the gate never skips the secret scan`); if (r.status === 1) fail(`gitleaks found a secret:\n${(r.stdout + r.stderr).trim().slice(-3000)}`); if (r.status !== 0) fail(`gitleaks exited ${r.status}:\n${r.stderr.trim().slice(-1000)}`); diff --git a/scripts/release.mjs b/scripts/release.mjs index 40d8a736e..f70589c90 100644 --- a/scripts/release.mjs +++ b/scripts/release.mjs @@ -22,6 +22,7 @@ // Pushing the tag runs .github/workflows/release.yml, which only STAGES the version on npm (after checking the tag's // signature against main's .github/release-signers); it goes live when the owner approves it on npmjs.com with 2FA. import { execFileSync } from 'node:child_process'; +import { gitExecFileSync } from './lib/git.cjs'; import { existsSync, mkdtempSync, readFileSync, unlinkSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -42,7 +43,7 @@ const fail = (msg) => { process.exit(1); }; if (!/^\d+\.\d+\.\d+$/.test(version ?? '')) fail('usage: release.mjs X.Y.Z | release.mjs changelog X.Y.Z [--repo owner/repo]'); -const git = (...a) => execFileSync('git', a, { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); +const git = (...a) => gitExecFileSync(a, { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); const gitConfig = (key) => { try { return git('config', '--get', key); diff --git a/scripts/rename.mjs b/scripts/rename.mjs index 9a2fe41c0..152782981 100644 --- a/scripts/rename.mjs +++ b/scripts/rename.mjs @@ -3,7 +3,7 @@ // npm run rename -- [--dry-run] // Reads the current name from src/brand.ts, then rewrites the three case // variants (Name, name, NAME) in every tracked or new non-ignored text file. -import { execFileSync } from 'node:child_process'; +import { gitExecFileSync } from './lib/git.cjs'; import { readFileSync, writeFileSync } from 'node:fs'; // Files that mention the name as data, not as our identity. @@ -31,7 +31,7 @@ const pairs = [ [current, next], ]; -const files = execFileSync('git', ['ls-files', '-co', '--exclude-standard'], { encoding: 'utf8' }) +const files = gitExecFileSync(['ls-files', '-co', '--exclude-standard'], { encoding: 'utf8' }) .split('\n') .filter((f) => f && !SKIP.has(f)); diff --git a/scripts/setup-hooks.mjs b/scripts/setup-hooks.mjs index bc4859585..c61e9a20f 100644 --- a/scripts/setup-hooks.mjs +++ b/scripts/setup-hooks.mjs @@ -7,14 +7,14 @@ // branch being pushed can't change it; and the copy holds no rules of its own: it loads the gate and its rules // from origin's default branch on every push. The older `core.hooksPath=.githooks` setup ran the hook straight // from the checked-out branch, so this unsets it. -import { execFileSync } from 'node:child_process'; +import { gitExecFileSync } from './lib/git.cjs'; import { chmodSync, copyFileSync, existsSync, mkdirSync, readFileSync, renameSync, writeFileSync } from 'node:fs'; import { join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; const MARKER = '# managed by setup-hooks: push gate'; const LAUNCHER = 'push-gate-hook.mjs'; -const git = (...a) => execFileSync('git', a, { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); +const git = (...a) => gitExecFileSync(a, { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); const tryGit = (...a) => { try { return git(...a); diff --git a/scripts/setup/engine-release.cjs b/scripts/setup/engine-release.cjs index 1a8bbf9bb..db4b39d64 100644 --- a/scripts/setup/engine-release.cjs +++ b/scripts/setup/engine-release.cjs @@ -12,8 +12,8 @@ // (never origin again), with origin's URL as its remote, checked out at by id. // It prints `verified : tag object , commit ` and exits 0, or names why and exits 1 with no checkout. 'use strict'; -const { execFileSync } = require('node:child_process'); const path = require('node:path'); +const { commandExecFileSync, gitExecFileSync } = require('../lib/git.cjs'); const { verifyRelease } = require('../machine/worklane-update.cjs'); const fail = (msg) => { @@ -26,7 +26,8 @@ if (!/^v[0-9]+\.[0-9]+\.[0-9]+$/.test(tag || '')) fail(`"${tag}" isn't a release if (!/^[0-9a-f]{40}$/.test(commit || '')) fail(`"${commit}" isn't a full commit id: a release is installed only at the commit scripts/bootstrap.sh pins`); if (!path.isAbsolute(signers)) fail(`the allowed signers file must be an absolute path, not ${signers}`); -const exec = (file, args) => execFileSync(file, args, { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], timeout: 300_000 }); +// git (the updater's verifyRelease runs it through exec) through the git runner (#807). +const exec = (file, args) => commandExecFileSync(file, args, { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], timeout: 300_000 }); const gitDir = path.join(work, 'verify.git'); const v = verifyRelease(url, tag, signers, exec, gitDir); if (!v.ok) fail(v.reason); @@ -34,7 +35,7 @@ if (v.commit !== commit) fail(`${tag} is signed and names itself, but points at // The source to build: the verified objects only, at the pinned commit by id. const src = path.join(work, 'src'); -const git = (...a) => exec('git', ['--no-replace-objects', ...a]).trim(); +const git = (...a) => gitExecFileSync(a, { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], timeout: 300_000 }).trim(); try { git('clone', '-q', '--no-checkout', gitDir, src); git('-C', src, 'remote', 'set-url', 'origin', url); diff --git a/scripts/verify-release-tag.mjs b/scripts/verify-release-tag.mjs index 164a6c2d2..7a0b72f95 100644 --- a/scripts/verify-release-tag.mjs +++ b/scripts/verify-release-tag.mjs @@ -14,7 +14,8 @@ // (default release-signer): `ssh-keygen -Y verify` on the tag's payload, as git itself checks it. Not // `git verify-tag`, which matches the signer to the tagger's email. // On success it prints `verified ` and exits 0. -import { execFileSync, spawnSync } from 'node:child_process'; +import { spawnSync } from 'node:child_process'; +import { gitExecFileSync } from './lib/git.cjs'; import { mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -39,7 +40,7 @@ if (!commit) fail(usage); if (!/^[0-9a-f]{40}$/.test(commit)) fail(`--commit must be a full commit id, not "${commit}"`); if (!/^[\w.@-]+$/.test(principal)) fail(`a principal of letters, digits, ".", "@", "_" or "-", not "${principal}"`); -const git = (...a) => execFileSync('git', a, { stdio: ['ignore', 'pipe', 'pipe'] }); +const git = (...a) => gitExecFileSync(a, { stdio: ['ignore', 'pipe', 'pipe'] }); const gitText = (...a) => { try { return git(...a).toString('utf8').trim(); diff --git a/scripts/verify-release-tree.mjs b/scripts/verify-release-tree.mjs index 6db18b282..3881a3352 100644 --- a/scripts/verify-release-tree.mjs +++ b/scripts/verify-release-tree.mjs @@ -10,6 +10,7 @@ // (scripts/lib/release-tree.mjs): raw bytes, no line-ending conversion. // On success it prints `same as the signed tree: files at ` and exits 0. import { execFileSync } from 'node:child_process'; +import { gitExecFileSync } from './lib/git.cjs'; import { existsSync, mkdtempSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; @@ -26,7 +27,7 @@ const fail = (msg) => { if (!tgz || !commit || !existsSync(tgz)) fail('usage: verify-release-tree.mjs --commit '); let sha; try { - sha = execFileSync('git', ['rev-parse', '--verify', `${commit}^{commit}`], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); + sha = gitExecFileSync(['rev-parse', '--verify', `${commit}^{commit}`], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); } catch { fail(`${commit} is not a commit here`); } diff --git a/src/auto-merge-resume.ts b/src/auto-merge-resume.ts index 92691731b..fa3303c1e 100644 --- a/src/auto-merge-resume.ts +++ b/src/auto-merge-resume.ts @@ -12,7 +12,8 @@ // Main's checks are JudgedChecks, so a pinned required check is green only from its pinned source. // Fail closed: a stop with no `by`, an unknown one, or an unreadable stop file waits for green main; main with no // required checks configured, a tip that can't be read, or checks that can't be read is not green. -import { spawn } from './os/child-process.js'; +import { gitSpawn } from './os/git.js'; +import type { ChildProcess } from './os/child-process.js'; import type { JudgedChecks } from './check-source.js'; import { killTree, spawnDetached } from './os/index.js'; import { requiredOutcomes } from './pr-watch.js'; @@ -85,9 +86,9 @@ export function remoteTip(repo: string, remote: string, branch: string, o: { tim }; // In its own process group, so a timeout kills git and everything it started (the transport: git-remote-https, // ssh, upload-pack), never leaving one holding a connection. - let child: ReturnType; + let child: ChildProcess; try { - child = spawn('git', ['ls-remote', remote, ref], { cwd: repo, detached: spawnDetached, stdio: ['ignore', 'pipe', 'ignore'] }); + child = gitSpawn(['ls-remote', remote, ref], { cwd: repo, detached: spawnDetached, stdio: ['ignore', 'pipe', 'ignore'] }); } catch { return finish(null); } diff --git a/src/check-pin-block.ts b/src/check-pin-block.ts index 5dc8e13b6..90bc93879 100644 --- a/src/check-pin-block.ts +++ b/src/check-pin-block.ts @@ -11,6 +11,7 @@ // - The block is valid under the engine's own schema, or it isn't offered: no duplicate key, no name the schema or // YAML can't take (those are comments). import { execFileSync } from './os/child-process.js'; +import { gitExecFileSync } from './os/git.js'; import { readFileSync, writeFileSync } from 'node:fs'; import { dirname, join } from 'node:path'; import { parse, parseDocument } from 'yaml'; @@ -58,7 +59,7 @@ export interface PinPlan { type Git = (repo: string, args: string[]) => string | null; const gitIn: Git = (repo, args) => { try { - return execFileSync('git', ['-C', repo, ...args], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'], timeout: 20_000, maxBuffer: 64 * 1024 * 1024 }); + return gitExecFileSync(['-C', repo, ...args], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'], timeout: 20_000, maxBuffer: 64 * 1024 * 1024 }); } catch { return null; } diff --git a/src/checkout-sync.ts b/src/checkout-sync.ts index 49878aa92..28a13cf1b 100644 --- a/src/checkout-sync.ts +++ b/src/checkout-sync.ts @@ -2,7 +2,7 @@ // `git pull` in a terminal. This brings the checkout's default branch up to origin's, as a fast-forward only: never a // reset, a merge commit or a rebase, and never over the person's own work. Anything else is refused, saying why and // what to run instead. -import { spawnSync } from './os/child-process.js'; +import { gitSpawnSync } from './os/git.js'; import { withToolPath } from './tool-path.js'; export interface GitRun { @@ -10,7 +10,7 @@ export interface GitRun { } export const systemGit = (root: string): GitRun => (args) => { - const r = spawnSync('git', args, { cwd: root, encoding: 'utf8', env: { ...withToolPath(), GIT_TERMINAL_PROMPT: '0' }, timeout: 120_000 }); + const r = gitSpawnSync(args, { cwd: root, encoding: 'utf8', env: { ...withToolPath(), GIT_TERMINAL_PROMPT: '0' }, timeout: 120_000 }); return { status: r.error ? null : r.status, stdout: r.stdout ?? '', stderr: r.error ? r.error.message : (r.stderr ?? '') }; }; diff --git a/src/checkout.ts b/src/checkout.ts index f55210a1a..410137ef4 100644 --- a/src/checkout.ts +++ b/src/checkout.ts @@ -5,7 +5,7 @@ // don't read. A test-run tier that needs them says `full_checkout: true` (test_run.tiers). // // Non-cone sparse checkout ("/*" then "!//" per directory): everything, minus those directories. -import { execFileSync } from './os/child-process.js'; +import { gitExecFileSync } from './os/git.js'; import { mkdirSync, writeFileSync } from 'node:fs'; import { dirname, resolve } from 'node:path'; import { z } from 'zod'; @@ -58,7 +58,7 @@ export function sparsePatterns(dirs: readonly string[]): string { return ['/*', ...dirs.map((d) => `!/${normalizeSparseDir(d)}/`)].join('\n') + '\n'; } -const git = (cwd: string, ...args: string[]) => execFileSync('git', args, { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); +const git = (cwd: string, ...args: string[]) => gitExecFileSync(args, { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); /** * Make the linked worktree at `path` (added with --no-checkout) sparse, leaving `dirs` out, and check its HEAD out. diff --git a/src/claims.ts b/src/claims.ts index 6d676bf87..847372fae 100644 --- a/src/claims.ts +++ b/src/claims.ts @@ -7,7 +7,7 @@ // release = delete it, expecting our lease // Two coordinators (one per human) racing for the same issue: exactly one // push succeeds. Labels and assignees are cosmetic and follow the ref. -import { execFileSync, spawnSync } from './os/child-process.js'; +import { gitExecFileSync, gitSpawnSync } from './os/git.js'; import { BRAND } from './brand.js'; export interface Lease { @@ -27,7 +27,7 @@ export interface ClaimOptions { export const claimRef = (issue: number) => `refs/${BRAND.cli}/claims/issue-${issue}`; function git(repo: string, args: string[], input?: string): string { - return execFileSync('git', args, { cwd: repo, encoding: 'utf8', input, stdio: ['pipe', 'pipe', 'pipe'] }).trim(); + return gitExecFileSync(args, { cwd: repo, encoding: 'utf8', input, stdio: ['pipe', 'pipe', 'pipe'] }).trim(); } /** A parentless commit holding the lease as its message. CAS needs no ancestry. */ @@ -39,7 +39,7 @@ function leaseCommit(repo: string, lease: Lease): string { /** Push `newSha` (or delete, when null) to the claim ref, only if it currently equals `expect` ('' = absent). */ function casPush(repo: string, remote: string, issue: number, expect: string, newSha: string | null): boolean { const ref = claimRef(issue); - const r = spawnSync('git', ['push', '--porcelain', `--force-with-lease=${ref}:${expect}`, remote, `${newSha ?? ''}:${ref}`], { cwd: repo, encoding: 'utf8' }); + const r = gitSpawnSync(['push', '--porcelain', `--force-with-lease=${ref}:${expect}`, remote, `${newSha ?? ''}:${ref}`], { cwd: repo, encoding: 'utf8' }); if (r.status === 0) return true; if (/\[rejected\]|stale info|already exists|fetch first/.test(r.stdout + r.stderr)) return false; throw new Error(`claim push failed: ${(r.stderr || r.stdout).trim().split('\n').pop()}`); diff --git a/src/cli.ts b/src/cli.ts index 48c512413..b67210bf8 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -71,6 +71,7 @@ import { sourceViewCommand } from './intake/source-view.js'; import { intakeCheckCommand } from './intake/check.js'; import { installService, stableNodePath, uninstallService } from './os/index.js'; import { ownEnv } from './os/own-env.js'; +import { gitExecFileSync } from './os/git.js'; import { projectStateDir } from './guardrails/context.js'; import { fileURLToPath } from 'node:url'; @@ -310,7 +311,7 @@ function option(args: string[], name: string): string | undefined { } function changedTestFiles(root: string, globs: string[], branch: string): string[] { - const base = execFileSync('git', ['merge-base', `origin/${branch}`, 'HEAD'], { cwd: root, encoding: 'utf8' }).trim(); + const base = gitExecFileSync(['merge-base', `origin/${branch}`, 'HEAD'], { cwd: root, encoding: 'utf8' }).trim(); // A renamed test file is an added one, and is checked (with renames on it was R, and skipped); a valid UTF-8 path as itself (#683). const files = changedNames(root, ['--diff-filter=AM', base]); const res = globs.map((g) => globToRegExp(g)); @@ -1031,8 +1032,8 @@ async function main(argv: string[]): Promise { const gh = i.credentials.github; const identity = i.commitIdentity ?? (gh.kind === 'app' ? await appBotIdentity({ appId: gh.app_id, installationId: gh.installation_id, keyPath: gh.key_path }) : { name: `${BRAND.cli}-${i.name}`, email: `${BRAND.cli}-${i.name}@users.noreply.invalid` }); const main = cfg.project.project.default_branch; - execFileSync('git', ['fetch', '-q', 'origin', main], { cwd: i.repo.path, stdio: 'inherit' }); - const base = execFileSync('git', ['rev-parse', `origin/${main}`], { cwd: i.repo.path, encoding: 'utf8' }).trim(); + gitExecFileSync(['fetch', '-q', 'origin', main], { cwd: i.repo.path, stdio: 'inherit' }); + const base = gitExecFileSync(['rev-parse', `origin/${main}`], { cwd: i.repo.path, encoding: 'utf8' }).trim(); const dryRun = flag(rest, '--dry-run'); const r = await tune({ // Through worktreeOptions (#890 round 20): the root resolved once, here, and the instance's task directory kept. diff --git a/src/comment-policy.ts b/src/comment-policy.ts index 7457d111e..684088792 100644 --- a/src/comment-policy.ts +++ b/src/comment-policy.ts @@ -3,13 +3,13 @@ // function, checkComments, for the PR diff check (checkRange, scripts/check-comments.mjs) and the agents' post-edit // hook (checkWorkingFile, wired in #330's second part), so the two can't drift. Comments are found by a line // scanner per language (no parser dependency). -import { spawnSync } from './os/child-process.js'; import { existsSync, readFileSync } from 'node:fs'; import { join } from 'node:path'; import { parseDocument } from 'yaml'; import type { z } from 'zod'; import { BRAND } from './brand.js'; import { CommentPolicyConfig } from './config/schema.js'; +import { gitSpawnSync } from './os/git.js'; export type CommentPolicy = z.infer; @@ -492,7 +492,7 @@ export function addedLines(diff: string): Map> { // INVARIANT: no command a checkout's or user's git config names runs here (#330): fsmonitor off, no textconv or external diff. const git = (cwd: string, args: string[], timeoutMs?: number) => - spawnSync('git', ['-c', 'core.quotePath=false', '-c', 'core.fsmonitor=false', ...args], { cwd, encoding: 'utf8', maxBuffer: 256 * 1024 * 1024, ...(timeoutMs ? { timeout: timeoutMs } : {}) }); + gitSpawnSync(['-c', 'core.quotePath=false', '-c', 'core.fsmonitor=false', ...args], { cwd, encoding: 'utf8', maxBuffer: 256 * 1024 * 1024, ...(timeoutMs ? { timeout: timeoutMs } : {}) }); // INVARIANT: the hook and CI read the same new lines whatever the user's git config (prefixes, algorithm, context). const DIFF = [ ...['-c', 'diff.noprefix=false', '-c', 'diff.mnemonicPrefix=false', '-c', 'diff.interHunkContext=0', '-c', 'diff.suppressBlankEmpty=false'], diff --git a/src/config-change.ts b/src/config-change.ts index 139543742..c0447d113 100644 --- a/src/config-change.ts +++ b/src/config-change.ts @@ -3,6 +3,7 @@ // the same loader the coordinator uses, and proposed as a PR from a branch of their own; nothing is pushed to the // default branch, and nothing in the checkout is touched. import { spawnSync } from './os/child-process.js'; +import { gitSpawnSync } from './os/git.js'; import { writeFileSync } from 'node:fs'; import { join } from 'node:path'; import { parseDocument } from 'yaml'; @@ -75,7 +76,7 @@ export interface ConfigChange { type Git = (args: string[], cwd: string) => { status: number | null; stdout: string; stderr: string }; const realGit: Git = (args, cwd) => { - const r = spawnSync('git', args, { cwd, encoding: 'utf8' }); + const r = gitSpawnSync(args, { cwd, encoding: 'utf8' }); return { status: r.status, stdout: r.stdout ?? '', stderr: r.stderr ?? '' }; }; diff --git a/src/config/live.ts b/src/config/live.ts index 9acce3e01..28bd1dbea 100644 --- a/src/config/live.ts +++ b/src/config/live.ts @@ -4,7 +4,7 @@ // schema and checks as at start, and swapped in whole. A config that fails validation is never applied: the last // good one stays, and why is recorded. Settings read once at start (below) keep their running value until a restart. // Merges are still judged by the config at their own parent (main-watch.ts requiredAt): this is for everything else. -import { spawnSync } from '../os/child-process.js'; +import { gitSpawnSync } from '../os/git.js'; import { BRAND } from '../brand.js'; import { ConfigInvalid, loadConfigAt, type Config, type ConfigError } from './load.js'; @@ -13,7 +13,7 @@ export const CONFIG_INCIDENT = 'config_invalid'; /** The git tree of the config directory at `rev`, or null (no such rev, or no config there). */ export function configTree(repo: string, rev: string): string | null { - const r = spawnSync('git', ['rev-parse', '-q', '--verify', `${rev}:${BRAND.configDir}`], { cwd: repo, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }); + const r = gitSpawnSync(['rev-parse', '-q', '--verify', `${rev}:${BRAND.configDir}`], { cwd: repo, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }); return r.status === 0 ? r.stdout.trim() : null; } @@ -148,7 +148,7 @@ export class LiveConfig { } private sha(): string | null { - const r = spawnSync('git', ['rev-parse', '-q', '--verify', `${this.ref}^{commit}`], { cwd: this.o.repo, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }); + const r = gitSpawnSync(['rev-parse', '-q', '--verify', `${this.ref}^{commit}`], { cwd: this.o.repo, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }); return r.status === 0 ? r.stdout.trim() : null; } } @@ -159,7 +159,7 @@ export class LiveConfig { */ export function startConfig(o: { repo: string; remote: string; fallback: Config; check?: (cfg: Config) => ConfigError[] }): { cfg: Config; tree: string | null; note: string | null } { const branch = o.fallback.project.project.default_branch; - spawnSync('git', ['fetch', '-q', o.remote, `+refs/heads/${branch}:refs/remotes/${o.remote}/${branch}`], { cwd: o.repo, stdio: 'ignore', timeout: 60_000 }); + gitSpawnSync(['fetch', '-q', o.remote, `+refs/heads/${branch}:refs/remotes/${o.remote}/${branch}`], { cwd: o.repo, stdio: 'ignore', timeout: 60_000 }); const ref = `refs/remotes/${o.remote}/${branch}`; const tree = configTree(o.repo, ref); // WHY (#715): whenever main's config can't run (none there, or invalid), the checkout's config as committed (the diff --git a/src/config/load.ts b/src/config/load.ts index 7edbaeeca..032479629 100644 --- a/src/config/load.ts +++ b/src/config/load.ts @@ -1,4 +1,4 @@ -import { spawnSync } from '../os/child-process.js'; +import { gitSpawnSync } from '../os/git.js'; import { closeSync, constants, existsSync, fstatSync, openSync, readSync, statSync } from 'node:fs'; import { join } from 'node:path'; import { parseDocument } from 'yaml'; @@ -120,7 +120,7 @@ export function loadConfig(root: string): Config { * schema and checks as loadConfig. Throws ConfigInvalid listing all errors. `dir` stays the checkout's. */ export function loadConfigAt(root: string, rev: string): Config { - const git = (...args: string[]) => spawnSync('git', args, { cwd: root, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], maxBuffer: 16 * 1024 * 1024 }); + const git = (...args: string[]) => gitSpawnSync(args, { cwd: root, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], maxBuffer: 16 * 1024 * 1024 }); if (git('rev-parse', '-q', '--verify', `${rev}:${BRAND.configDir}`).status !== 0) throw new ConfigInvalid([{ file: BRAND.configDir, path: '', message: `missing at ${rev}` }]); const show = (file: string) => { const r = git('show', `${rev}:${BRAND.configDir}/${file}`); diff --git a/src/coordinator.ts b/src/coordinator.ts index 1d51b42ce..2ddcab11a 100644 --- a/src/coordinator.ts +++ b/src/coordinator.ts @@ -3,7 +3,8 @@ // step is decided from events plus GitHub, so a restart picks up where the // log says it was. Agents only ever propose: they commit on their own // branch in their own worktree; everything outward-facing happens here. -import { execFileSync, spawn, spawnSync } from './os/child-process.js'; +import { spawn } from './os/child-process.js'; +import { gitExecFileSync, gitSpawnSync } from './os/git.js'; import { createHash, randomBytes } from 'node:crypto'; import { cpSync, mkdirSync, readFileSync, existsSync, rmSync } from 'node:fs'; import { dirname, join } from 'node:path'; @@ -38,7 +39,7 @@ import { DEFAULT_HOTSPOTS, estimateFiles, fileIndex, holdReason, hotspotsIn, pic import { TaskGraph } from './coordinator/task-graph.js'; import { bodyHash, childBody, GOAL_LABEL, GOAL_SPLIT_LABEL, GOAL_SPLIT_SCHEMA, readGoalSplit, splitComment } from './goal-split.js'; import { changedFiles } from './merge-base.js'; -import { changedNames, diffStatText, rawChanges, untrackedNames } from './coordinator/change-files.js'; +import { changedNames, diffStatText, patchText, rawChanges, untrackedNames } from './coordinator/change-files.js'; import { importsOf, lightCheckPlan, lightCheckWait } from './light-check.js'; import { readRun, runsForIssue } from './run-record.js'; import { claimsOf, unbackedClaims, type CheckRecord } from './receipts.js'; @@ -511,7 +512,7 @@ export class Coordinator extends AutomationRuns { */ refreshConfig(): void { this.liveCfg ??= new LiveConfig({ repo: this.d.repo, remote: this.remote, branch: this.branch, ...(this.d.liveConfig?.tree !== undefined ? { tree: this.d.liveConfig.tree } : {}), ...(this.d.liveConfig?.check ? { check: this.d.liveConfig.check } : {}) }); - spawnSync('git', ['fetch', '-q', this.remote, `+refs/heads/${this.branch}:refs/remotes/${this.remote}/${this.branch}`], { cwd: this.d.repo, stdio: 'ignore', timeout: 60_000 }); + gitSpawnSync(['fetch', '-q', this.remote, `+refs/heads/${this.branch}:refs/remotes/${this.remote}/${this.branch}`], { cwd: this.d.repo, stdio: 'ignore', timeout: 60_000 }); const r = this.liveCfg.poll(this.repoCfg); if (r.kind === 'same') return; if (r.kind === 'invalid') { @@ -831,7 +832,7 @@ export class Coordinator extends AutomationRuns { this.poolBusy = true; this.poolRefill = (async () => { try { - spawnSync('git', ['fetch', '-q', this.remote, this.branch], { cwd: this.d.repo }); + gitSpawnSync(['fetch', '-q', this.remote, this.branch], { cwd: this.d.repo }); const base = this.git(this.d.repo, 'rev-parse', `${this.remote}/${this.branch}`); const wt = this.wt; const keyFiles = this.d.cfg.tests.worktree.setup_key; @@ -1521,7 +1522,8 @@ export class Coordinator extends AutomationRuns { } const added = files.reduce((s, f) => s + f.added, 0); const removed = files.reduce((s, f) => s + f.removed, 0); - const patchHash = createHash('sha256').update(this.git(path, 'diff', `${base}..${head}`)).digest('hex'); + // Over every file's lines, whatever the change's attributes say: a `-diff` file's new content is a new hash (#807). + const patchHash = createHash('sha256').update(patchText(path, [`${base}..${head}`])).digest('hex'); this.emit('change.proposed', { issue: n, branch: this.paths(n).branch, base, head, files: names, lines: added + removed, added, removed, patch_hash: patchHash }); return { files, tampered, patchHash }; } @@ -1791,10 +1793,10 @@ export class Coordinator extends AutomationRuns { const imports: Record = {}; if (mainChanged.length) { // -z, as prFiles: a valid UTF-8 path as itself, so an import of a file with an odd byte in its name still resolves. - const tree = new Set(execFileSync('git', ['ls-tree', '-r', '-z', '--name-only', pr.headSha], { cwd: this.d.repo, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], maxBuffer: 256 * 1024 * 1024 }).split('\0').filter(Boolean)); + const tree = new Set(gitExecFileSync(['ls-tree', '-r', '-z', '--name-only', pr.headSha], { cwd: this.d.repo, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], maxBuffer: 256 * 1024 * 1024 }).split('\0').filter(Boolean)); for (const f of prFiles) { if (!/\.((m|c)?(j|t)sx?|py)$/.test(f)) continue; - const r = spawnSync('git', ['show', `${pr.headSha}:${f}`], { cwd: this.d.repo, encoding: 'utf8' }); + const r = gitSpawnSync(['show', `${pr.headSha}:${f}`], { cwd: this.d.repo, encoding: 'utf8' }); if (r.status === 0) imports[f] = importsOf(f, r.stdout, tree); } } @@ -1842,7 +1844,7 @@ export class Coordinator extends AutomationRuns { // The monitor reads on the cheaper model routing gives a reflection (the short read-only run before a retry). const monitorModel = routeRun(this.d.cfg, 'reflection'); const show = (commit: string, path: string) => { - const r = spawnSync('git', ['show', `${commit}:${path}`], { cwd: this.d.repo, maxBuffer: 64 * 1024 * 1024 }); + const r = gitSpawnSync(['show', `${commit}:${path}`], { cwd: this.d.repo, maxBuffer: 64 * 1024 * 1024 }); return r.status === 0 ? r.stdout : null; }; return { @@ -1903,7 +1905,7 @@ export class Coordinator extends AutomationRuns { }); this.emit('run.cost', { issue: null, role: 'experimenter', model: r.model, usd: r.costUsd, turns: r.turns }); const files = o.hide.filter((h) => !h.includes('*')); - if (files.length) execFileSync('git', ['checkout', o.commit, '--', ...files], { cwd: path, stdio: 'ignore' }); + if (files.length) gitExecFileSync(['checkout', o.commit, '--', ...files], { cwd: path, stdio: 'ignore' }); return await then(r, path, lines.join('\n')); } finally { try { @@ -1933,7 +1935,7 @@ export class Coordinator extends AutomationRuns { }, bundle: (ref, commit) => { this.git(this.d.repo, 'update-ref', ref, commit); - const r = spawnSync('git', ['bundle', 'create', '-q', '-', ref], { cwd: this.d.repo, maxBuffer: 1024 * 1024 * 1024 }); + const r = gitSpawnSync(['bundle', 'create', '-q', '-', ref], { cwd: this.d.repo, maxBuffer: 1024 * 1024 * 1024 }); if (r.status !== 0) throw new Error(`git bundle ${ref}: ${r.stderr.toString().slice(0, 300)}`); return r.stdout; }, diff --git a/src/coordinator/base.ts b/src/coordinator/base.ts index 744602efa..0d3b41a8d 100644 --- a/src/coordinator/base.ts +++ b/src/coordinator/base.ts @@ -1,7 +1,7 @@ // The coordinator's state and the helpers every part uses: its fields and constructor (the runner wrapper), // events, git, project commands, paths and task files, the governor, cost, asking, blocking and releasing. import type { Coordinator } from '../coordinator.js'; -import { execFileSync } from '../os/child-process.js'; +import { gitExecFileSync } from '../os/git.js'; import { randomBytes } from 'node:crypto'; import { dirname, join, resolve } from 'node:path'; import { BRAND } from '../brand.js'; @@ -306,7 +306,7 @@ export abstract class CoordinatorBase { } protected git(cwd: string, ...args: string[]) { - return execFileSync('git', args, { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); + return gitExecFileSync(args, { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); } protected emit[0]>(type: T, payload: EventPayload) { diff --git a/src/coordinator/change-files.ts b/src/coordinator/change-files.ts index 8bdb946d6..30d2d1c2e 100644 --- a/src/coordinator/change-files.ts +++ b/src/coordinator/change-files.ts @@ -26,7 +26,7 @@ // for a name that can't be handed back to git, from its blobs). review.ts flags a change that adds such a mark, since // GitHub and a checkout still honour it. import { Buffer, isUtf8 } from 'node:buffer'; -import { spawnSync } from '../os/child-process.js'; +import { gitSpawnSync } from '../os/git.js'; import { readFileSync } from 'node:fs'; import { resolve } from 'node:path'; import { attributeTransitionHides, isGitattributesPath, isGitmodulesPath, macrosDefined, OUTCOME_ATTRIBUTES, type ChangeFile } from '../review.js'; @@ -56,16 +56,16 @@ function withGit(args: string[]): string[] { } /** Run git; its stdout, untrimmed (a path may start or end with a space), or an Error saying why it failed. */ -function run(cwd: string, args: string[], timeout?: number): string | Error { - const r = spawnSync('git', withGit(args), { cwd, encoding: 'utf8', maxBuffer: MAX_BUFFER, ...(timeout ? { timeout } : {}) }); +function run(cwd: string, args: string[], timeout?: number, env?: NodeJS.ProcessEnv): string | Error { + const r = gitSpawnSync(withGit(args), { cwd, encoding: 'utf8', maxBuffer: MAX_BUFFER, ...(timeout ? { timeout } : {}), ...(env ? { env } : {}) }); if (r.error) return r.error; if (r.status !== 0) return new Error(`git ${args.join(' ').slice(0, 200)} failed: ${(r.stderr || `exit ${r.status}`).trim().slice(0, 500)}`); return r.stdout; } /** run, with stdout as bytes: for output that holds paths. */ -function runBytes(cwd: string, args: string[], timeout?: number): Buffer | Error { - const r = spawnSync('git', withGit(args), { cwd, encoding: 'buffer', maxBuffer: MAX_BUFFER, ...(timeout ? { timeout } : {}) }); +function runBytes(cwd: string, args: string[], timeout?: number, env?: NodeJS.ProcessEnv): Buffer | Error { + const r = gitSpawnSync(withGit(args), { cwd, encoding: 'buffer', maxBuffer: MAX_BUFFER, ...(timeout ? { timeout } : {}), ...(env ? { env } : {}) }); if (r.error) return r.error; if (r.status !== 0) return new Error(`git ${args.join(' ').slice(0, 200)} failed: ${(r.stderr.toString('utf8') || `exit ${r.status}`).trim().slice(0, 500)}`); return r.stdout; @@ -134,22 +134,23 @@ export function changedNames(cwd: string, args: string[], o: NameListOptions = { /** * The paths any commit in `range` (base..head) touches, not just its net diff: a file added and removed again still * counts. A merge is read against its first parent; every submodule link is seen. Each path read by nameOf, once, so a - * name git would C-quote (a non-ASCII byte, a newline, a quote) is itself (#724). Throws when git fails. + * name git would C-quote (a non-ASCII byte, a newline, a quote) is itself (#724). Throws when git fails. `env`: a + * gitSnapshot's, so the walk reads the push limits' one checked view of the history (#807). */ -export function rangeNames(cwd: string, range: string): string[] { +export function rangeNames(cwd: string, range: string, env?: NodeJS.ProcessEnv): string[] { if (range.startsWith('-')) throw new Error(`rangeNames: ${range} is not a range`); // Plumbing only (#724, round 4): `git log` is porcelain and reads log.* config. log.showSignature glued a signature // check's text to the first name ("No signature\nsecrets/k.key", so 1 problem became 0, measured) and // log.showRoot=false hid a root commit's files. rev-list and diff-tree read none of it: each commit of the range // against its first parent, a root against the empty tree. - const commits = must(run(cwd, ['rev-list', range])); + const commits = must(run(cwd, ['rev-list', range], undefined, env)); if (!commits.trim()) return []; - return [...new Set(names(must(plumbingBytes(cwd, ['diff-tree', '--stdin', '-r', '--root', '-m', '--first-parent', '--no-renames', '--name-only', '--no-commit-id', '-z'], commits))))]; + return [...new Set(names(must(plumbingBytes(cwd, ['diff-tree', '--stdin', '-r', '--root', '-m', '--first-parent', '--no-renames', '--name-only', '--no-commit-id', '-z'], commits, env))))]; } /** runBytes with `input` on git's stdin. */ -function plumbingBytes(cwd: string, args: string[], input: string): Buffer | Error { - const r = spawnSync('git', withGit(args), { cwd, input: Buffer.from(input, 'utf8'), encoding: 'buffer', maxBuffer: MAX_BUFFER }); +function plumbingBytes(cwd: string, args: string[], input: string, env?: NodeJS.ProcessEnv): Buffer | Error { + const r = gitSpawnSync(withGit(args), { cwd, input: Buffer.from(input, 'utf8'), encoding: 'buffer', maxBuffer: MAX_BUFFER, ...(env ? { env } : {}) }); if (r.error) return r.error; if (r.status !== 0) return new Error(`git ${args.join(' ').slice(0, 200)} failed: ${(r.stderr.toString('utf8') || `exit ${r.status}`).trim().slice(0, 500)}`); return r.stdout; @@ -167,15 +168,15 @@ const REGULAR_FILE = new Set(['100644', '100755']); * still read (#724). Only a regular file of at most `maxBytes` is read; anything else says why it wasn't: a symlink * (its blob is its target's path, not the file a checkout opens), a submodule link, a blob over the size, or one git * can't give. A name with no entry at `rev` isn't in the map: that is the only "not there". Throws when the tree - * can't be listed. + * can't be listed. `env`: a gitSnapshot's, as for rangeNames (`rev` must then be a commit id: a snapshot has no refs). */ -export function blobsByName(cwd: string, rev: string, names: string[], maxBytes = Infinity): Map { +export function blobsByName(cwd: string, rev: string, names: string[], maxBytes = Infinity, env?: NodeJS.ProcessEnv): Map { const out = new Map(); const wanted = new Set(names); if (!wanted.size) return out; if (rev.startsWith('-')) throw new Error(`blobsByName: ${rev} is not a revision`); // -r -z -l: "mode SP type SP object SP+ size TAB path NUL" for every file of the tree. - for (const rec of fields(must(runBytes(cwd, ['ls-tree', '-r', '-z', '-l', '--full-tree', rev])))) { + for (const rec of fields(must(runBytes(cwd, ['ls-tree', '-r', '-z', '-l', '--full-tree', rev], undefined, env)))) { const tab = rec.indexOf(9); if (tab === -1) continue; const name = nameOf(rec.subarray(tab + 1)); @@ -185,7 +186,7 @@ export function blobsByName(cwd: string, rev: string, names: string[], maxBytes if (mode === '120000') return { unread: 'a symbolic link, not a file' }; if (type !== 'blob' || !sha || !REGULAR_FILE.has(mode ?? '')) return { unread: `not a regular file (mode ${mode})` }; if (!(Number(size) <= maxBytes)) return { unread: `${size} bytes, over the ${maxBytes}-byte limit for it` }; - const text = run(cwd, ['cat-file', 'blob', sha]); + const text = run(cwd, ['cat-file', 'blob', sha], undefined, env); return text instanceof Error ? { unread: 'git can\'t give its blob' } : { text }; })(); out.set(name, [...(out.get(name) ?? []), file]); @@ -230,6 +231,16 @@ export function diffStatText(cwd: string, rev: string): string { return must(run(cwd, ['diff', '--stat', '--no-renames', '--no-color', ...nameArgs([rev])])).trim(); } +/** + * `git diff ` as a patch, for a hash only, never shown (#807): every file's lines whatever the attributes say + * (a `-diff` or `binary` mark, a textconv or an external diff), and every submodule link, a binary file's bytes + * included. `args` as changedNames takes them. Never parsed, and never put on a page, in a prompt or in an issue: the + * owner cut `--text` from every diff a person reads (#927). + */ +export function patchText(cwd: string, args: string[]): string { + return must(run(cwd, ['diff', '--no-color', ...AS_TEXT, ...nameArgs(args)])); +} + /** `git show --stat --patch` of one commit, as text for a person. Never parsed. */ export function commitText(cwd: string, commit: string): string { return must(run(cwd, ['show', '--stat', '--patch', '--no-color', '--no-ext-diff', '--format=commit %H', ...nameArgs([commit])])); @@ -350,7 +361,7 @@ function treePaths(cwd: string, rev: string): Map { */ function attributeStates(cwd: string, rev: string, paths: Buffer[]): Map> | null { const input = Buffer.concat(paths.flatMap((p) => [p, Buffer.from([0])])); - const r = spawnSync('git', withGit(['check-attr', `--source=${rev}`, '-z', '--stdin', ...OUTCOME_ATTRIBUTES]), { cwd, input, encoding: 'buffer', maxBuffer: MAX_BUFFER, env: { ...process.env, GIT_ATTR_NOSYSTEM: '1' } }); + const r = gitSpawnSync(withGit(['check-attr', `--source=${rev}`, '-z', '--stdin', ...OUTCOME_ATTRIBUTES]), { cwd, input, encoding: 'buffer', maxBuffer: MAX_BUFFER, env: { ...process.env, GIT_ATTR_NOSYSTEM: '1' } }); if (r.error || r.status !== 0) return null; const out = new Map>(); // -z: "path NUL attribute NUL state NUL" for each path and attribute. @@ -406,7 +417,7 @@ const OUTCOME_SHOWN = 10; /** git's exit status and stdout, for the calls whose status is an answer (`cat-file -e`, `config --get-regexp`). */ function runStatus(cwd: string, args: string[]): { status: number | null; stdout: string } { - const r = spawnSync('git', withGit(args), { cwd, encoding: 'utf8', maxBuffer: MAX_BUFFER }); + const r = gitSpawnSync(withGit(args), { cwd, encoding: 'utf8', maxBuffer: MAX_BUFFER }); return { status: r.error ? null : r.status, stdout: r.stdout ?? '' }; } @@ -609,7 +620,7 @@ export function exactMoves(cwd: string, base: string, head: string): Map ['-e', n]), rev, '--']), { + const r = gitSpawnSync(withGit(['grep', '-l', '-z', '-a', '-F', '--no-color', '--no-textconv', ...needles.flatMap((n) => ['-e', n]), rev, '--']), { cwd: top, encoding: 'buffer', maxBuffer: MAX_BUFFER, diff --git a/src/coordinator/checks-from-feedback.ts b/src/coordinator/checks-from-feedback.ts index 787637e7d..804a172bd 100644 --- a/src/coordinator/checks-from-feedback.ts +++ b/src/coordinator/checks-from-feedback.ts @@ -3,7 +3,7 @@ // repo has sgconfig.yml) or a test, written by one run in a scratch worktree and checked before it is pushed; or an // issue, when the constraint (its evidence's files, or the draft's) is in a protected area. The PR is never recorded // as the harness's (no pr.opened) and its branch never auto-merges. See src/checks-from-feedback.ts. -import { spawnSync } from '../os/child-process.js'; +import { gitSpawnSync } from '../os/git.js'; import { existsSync, readFileSync } from 'node:fs'; import { join } from 'node:path'; import { BRAND } from '../brand.js'; @@ -65,7 +65,7 @@ export abstract class ChecksFromFeedback extends Optimizer { const tip = this.git(this.d.repo, 'rev-parse', `${this.remote}/${this.branch}`); const dir = join(this.d.stateDir, 'feedback-checks', c.id); unlockWorktree(this.d.repo, dir); - spawnSync('git', ['worktree', 'remove', '--force', dir], { cwd: this.d.repo }); + gitSpawnSync(['worktree', 'remove', '--force', dir], { cwd: this.d.repo }); try { this.git(this.d.repo, 'worktree', 'add', '-q', '--detach', dir, tip); // Locked against a prune run inside another agent's sandbox, where it is hidden (src/worktrees.ts WORKTREE_LOCK_REASON). @@ -112,8 +112,8 @@ export abstract class ChecksFromFeedback extends Optimizer { this.emit('feedback_check.proposed', { constraint: c.id, keys, kind: 'pr', number: pr.number, url: pr.url, mode: mode.kind, why: a.summary.slice(0, 1000) }); } finally { unlockWorktree(this.d.repo, dir); - spawnSync('git', ['worktree', 'remove', '--force', dir], { cwd: this.d.repo }); - spawnSync('git', ['worktree', 'prune'], { cwd: this.d.repo }); + gitSpawnSync(['worktree', 'remove', '--force', dir], { cwd: this.d.repo }); + gitSpawnSync(['worktree', 'prune'], { cwd: this.d.repo }); } } diff --git a/src/coordinator/conversation.ts b/src/coordinator/conversation.ts index 2201c0ae5..a1ea8065a 100644 --- a/src/coordinator/conversation.ts +++ b/src/coordinator/conversation.ts @@ -4,7 +4,7 @@ // intake role; shows the draft; and files it `ready` only on an explicit yes from the person who posted it or an // admin (mayFileReady). Until then the draft is only in the coordinator's private store. Once filed, the thread // gets the issue's progress from the events: planned, PR opened, merged, live. -import { execFileSync } from '../os/child-process.js'; +import { gitExecFileSync } from '../os/git.js'; import { existsSync, readFileSync, rmSync, statSync } from 'node:fs'; import { dirname, join } from 'node:path'; import type { EventPayload } from '../events/types.js'; @@ -280,7 +280,7 @@ export abstract class Conversation extends Channel { private includes(deploy: string, sha: string): boolean { if (deploy === sha) return true; try { - execFileSync('git', ['merge-base', '--is-ancestor', sha, deploy], { cwd: this.d.repo, stdio: 'ignore' }); + gitExecFileSync(['merge-base', '--is-ancestor', sha, deploy], { cwd: this.d.repo, stdio: 'ignore' }); return true; } catch { return false; diff --git a/src/coordinator/decisions.ts b/src/coordinator/decisions.ts index 5573eee41..deccc827e 100644 --- a/src/coordinator/decisions.ts +++ b/src/coordinator/decisions.ts @@ -1,5 +1,5 @@ // Decisions: held plans, refused hosts, and the owner's answers on issues. -import { spawnSync } from '../os/child-process.js'; +import { gitSpawnSync } from '../os/git.js'; import { randomBytes } from 'node:crypto'; import { rmSync } from 'node:fs'; import { join } from 'node:path'; @@ -69,7 +69,7 @@ export abstract class Decisions extends Recovery { } const review: ReviewConfig = this.d.cfg.review ?? DEFAULT_REVIEW; // The plan gate: always (the default) holds every plan for the owner; risky only one the merge policy's checks hold. - const reasons = gateReasons(this.d.cfg.project.plan_gate, planHoldReasons(plan, { review, moneyPaths: this.moneyPaths(review.money_path_source), scope: lockScopeOf(this.d), existsAtBase: (d) => spawnSync('git', ['cat-file', '-e', `${base}:${d}`], { cwd: this.d.repo }).status === 0 })); + const reasons = gateReasons(this.d.cfg.project.plan_gate, planHoldReasons(plan, { review, moneyPaths: this.moneyPaths(review.money_path_source), scope: lockScopeOf(this.d), existsAtBase: (d) => gitSpawnSync(['cat-file', '-e', `${base}:${d}`], { cwd: this.d.repo }).status === 0 })); await this.d.backlog.comment(n, planComment(plan, owner, reasons)); const commentsAt = (await this.d.backlog.comments(n)).length; if (reasons.length) { @@ -207,15 +207,15 @@ export abstract class Decisions extends Recovery { const file = `${BRAND.configDir}/guardrails.yaml`; this.git(this.d.repo, 'fetch', '-q', this.remote, this.branch); const base = this.git(this.d.repo, 'rev-parse', `${this.remote}/${this.branch}`); - const shown = spawnSync('git', ['show', `${base}:${file}`], { cwd: this.d.repo, encoding: 'utf8' }); + const shown = gitSpawnSync(['show', `${base}:${file}`], { cwd: this.d.repo, encoding: 'utf8' }); if (shown.status !== 0) return { pr: null, detail: `${file} isn't on ${this.branch}, so no change was proposed` }; const next = withAllowedHost(shown.stdout, host); if (next === null) return { pr: null, detail: `${host} is already allowed on ${this.branch}` }; - const blob = spawnSync('git', ['hash-object', '-w', '--stdin'], { cwd: this.d.repo, encoding: 'utf8', input: next }).stdout.trim(); + const blob = gitSpawnSync(['hash-object', '-w', '--stdin'], { cwd: this.d.repo, encoding: 'utf8', input: next }).stdout.trim(); const index = join(this.d.stateDir, `allow-${randomBytes(3).toString('hex')}.index`); const env = { ...process.env, GIT_INDEX_FILE: index, GIT_AUTHOR_NAME: this.identity.name, GIT_AUTHOR_EMAIL: this.identity.email, GIT_COMMITTER_NAME: this.identity.name, GIT_COMMITTER_EMAIL: this.identity.email }; const g = (args: string[], input?: string) => { - const r = spawnSync('git', args, { cwd: this.d.repo, encoding: 'utf8', env, ...(input !== undefined ? { input } : {}) }); + const r = gitSpawnSync(args, { cwd: this.d.repo, encoding: 'utf8', env, ...(input !== undefined ? { input } : {}) }); if (r.status !== 0) throw new Error(`git ${args[0]} failed: ${(r.stderr || '').trim().slice(-300)}`); return r.stdout.trim(); }; diff --git a/src/coordinator/fix-runs.ts b/src/coordinator/fix-runs.ts index c5693444e..536dff809 100644 --- a/src/coordinator/fix-runs.ts +++ b/src/coordinator/fix-runs.ts @@ -1,7 +1,7 @@ // CI fix runs and conflict fix runs on the harness's own PRs. import { localDoneWhen } from '../testrun/local.js'; import type { Coordinator } from '../coordinator.js'; -import { spawnSync } from '../os/child-process.js'; +import { gitSpawnSync } from '../os/git.js'; import { randomBytes } from 'node:crypto'; import { BRAND } from '../brand.js'; import type { ReviewConfig } from '../config/schema.js'; @@ -314,7 +314,7 @@ export abstract class FixRuns extends Landing { const why = s.no_change_needed ?? `it made no commit (${s.summary})`; return await give(trigger.kind === 'ci' ? `the fix run found the failure isn't caused by this change: ${why}` : `the fix run changed nothing for the review: ${why}`); } - if (spawnSync('git', ['merge-base', '--is-ancestor', pr.headSha, head], { cwd: path }).status !== 0) return await give('the fix run rewrote the branch history instead of adding commits'); + if (gitSpawnSync(['merge-base', '--is-ancestor', pr.headSha, head], { cwd: path }).status !== 0) return await give('the fix run rewrote the branch history instead of adding commits'); const change = await this.inspect(n, path, claimed.base, head, repro); if ('rejected' in change) return await give(`the fix was rejected: ${change.rejected}`); const checks = await this.verify(n, path, head, doneWhen, repro); @@ -422,7 +422,7 @@ export abstract class FixRuns extends Landing { const baseSha = this.git(this.d.repo, 'rev-parse', `${this.remote}/${baseRef}`); // The branch already contains its base: GitHub's merge check is behind. Look again soon instead. this.git(this.d.repo, 'fetch', '-q', this.remote, `+refs/heads/${pr.head}:refs/remotes/${this.remote}/${pr.head}`); - if (spawnSync('git', ['merge-base', '--is-ancestor', baseSha, pr.headSha], { cwd: this.d.repo }).status === 0) { + if (gitSpawnSync(['merge-base', '--is-ancestor', baseSha, pr.headSha], { cwd: this.d.repo }).status === 0) { this.prPolled.set(number, Date.now() - (this.d.prPollMs ?? 60_000) + MERGEABLE_RECHECK_MS); return; } @@ -476,7 +476,7 @@ export abstract class FixRuns extends Landing { /** git, returning null instead of throwing (a path missing at a commit, an unmerged index entry). */ protected gitOrNull(cwd: string, ...args: string[]): string | null { - const r = spawnSync('git', args, { cwd, encoding: 'utf8' }); + const r = gitSpawnSync(args, { cwd, encoding: 'utf8' }); return r.status === 0 ? r.stdout.replace(/\n$/, '') : null; } @@ -562,7 +562,7 @@ export abstract class FixRuns extends Landing { if (this.gitOrNull(path, 'rev-parse', 'HEAD^1') !== pr.headSha || this.gitOrNull(path, 'rev-parse', 'HEAD^2') !== baseSha) return await give('the fix run did not finish the merge commit of the branch and its base (it rewrote history, added commits, or left the merge uncommitted)'); // Exactly as committed (no trimming): the comparison is line for line, the last line included. const committed = (f: string) => { - const r = spawnSync('git', ['show', `HEAD:${f}`], { cwd: path, encoding: 'utf8' }); + const r = gitSpawnSync(['show', `HEAD:${f}`], { cwd: path, encoding: 'utf8' }); return r.status === 0 ? r.stdout : null; }; const resolved = Object.fromEntries(files.map((f) => [f, committed(f)])); diff --git a/src/coordinator/intake.ts b/src/coordinator/intake.ts index 3aa5f3ed2..8b7b923cd 100644 --- a/src/coordinator/intake.ts +++ b/src/coordinator/intake.ts @@ -1,5 +1,5 @@ // Work intake (docs/intake.md): pulling the sources, triage, filing or asking, replies, and recovery after a restart. -import { execFileSync } from '../os/child-process.js'; +import { gitExecFileSync } from '../os/git.js'; import { createHash } from 'node:crypto'; import { readFileSync, existsSync, rmSync, statSync } from 'node:fs'; import { dirname, join } from 'node:path'; @@ -225,7 +225,7 @@ export abstract class Intake extends QuickReplies { if (rep?.result !== 'fixed_on_main' || !rep.commit) return t; const { commit, ...rest } = rep; try { - execFileSync('git', ['merge-base', '--is-ancestor', commit, `refs/remotes/${this.remote}/${this.branch}`], { cwd: this.d.repo, stdio: 'ignore', timeout: 10_000 }); + gitExecFileSync(['merge-base', '--is-ancestor', commit, `refs/remotes/${this.remote}/${this.branch}`], { cwd: this.d.repo, stdio: 'ignore', timeout: 10_000 }); return t; } catch { return { ...t, reproduction: rest }; @@ -462,7 +462,7 @@ export abstract class Intake extends QuickReplies { const includes = (deploy: string, sha: string) => { if (deploy === sha) return true; try { - execFileSync('git', ['merge-base', '--is-ancestor', sha, deploy], { cwd: this.d.repo, stdio: 'ignore' }); + gitExecFileSync(['merge-base', '--is-ancestor', sha, deploy], { cwd: this.d.repo, stdio: 'ignore' }); return true; } catch { return false; diff --git a/src/coordinator/landing.ts b/src/coordinator/landing.ts index 5d100bd14..8fa98d49a 100644 --- a/src/coordinator/landing.ts +++ b/src/coordinator/landing.ts @@ -1,5 +1,5 @@ // Landing: the land queue and batches, the gates before landing, opening PRs, and what follows a landing (deploy). -import { spawnSync } from '../os/child-process.js'; +import { gitSpawnSync } from '../os/git.js'; import { randomBytes } from 'node:crypto'; import { BRAND } from '../brand.js'; import { type DoneWhenList } from '../backlog/types.js'; @@ -168,9 +168,9 @@ export abstract class Landing extends Decisions { } for (const q of batch) { const claimed = this.events(q.issue).filter((e) => e.type === 'issue.claimed').at(-1)!.payload as EventPayload<'issue.claimed'>; - const pick = spawnSync('git', ['cherry-pick', `${claimed.base}..${q.head}`], { cwd: path, encoding: 'utf8' }); + const pick = gitSpawnSync(['cherry-pick', `${claimed.base}..${q.head}`], { cwd: path, encoding: 'utf8' }); if (pick.status !== 0) { - spawnSync('git', ['cherry-pick', '--abort'], { cwd: path }); + gitSpawnSync(['cherry-pick', '--abort'], { cwd: path }); result(q.issue, 'conflict', null, `does not apply cleanly on ${tip.slice(0, 8)}${applied.length ? ` after ${applied.map((a) => `#${a.issue}`).join(', ')}` : ''}; never guessing at conflicts`); await this.block(q.issue, ownerOf(q.issue), `conflicts with ${this.branch}; needs a rebase`); continue; diff --git a/src/coordinator/learnings.ts b/src/coordinator/learnings.ts index 26119700e..dccae20ec 100644 --- a/src/coordinator/learnings.ts +++ b/src/coordinator/learnings.ts @@ -1,6 +1,6 @@ // WHY: item 33 wiring (src/learnings.ts): notes when a task ends, and an owner's promote filed as an AGENTS.md PR // built from the default branch with git plumbing (no worktree, no agent), labelled review:L3, never merged here. -import { spawnSync } from '../os/child-process.js'; +import { gitSpawnSync } from '../os/git.js'; import { randomBytes } from 'node:crypto'; import { mkdirSync, rmSync } from 'node:fs'; import { join } from 'node:path'; @@ -44,15 +44,15 @@ export abstract class Learnings extends ChecksFromFeedback { private async fileAgentsMdPr(fp: string, line: string, by: string): Promise<{ pr: { url: string; number: number } | null; detail: string }> { this.git(this.d.repo, 'fetch', '-q', this.remote, this.branch); const base = this.git(this.d.repo, 'rev-parse', `${this.remote}/${this.branch}`); - const shown = spawnSync('git', ['show', `${base}:${AGENTS_MD}`], { cwd: this.d.repo, encoding: 'utf8' }); + const shown = gitSpawnSync(['show', `${base}:${AGENTS_MD}`], { cwd: this.d.repo, encoding: 'utf8' }); const next = withLearning(shown.status === 0 ? shown.stdout : null, line); if (next === null) return { pr: null, detail: `the line is in ${AGENTS_MD} on ${this.branch} already` }; - const blob = spawnSync('git', ['hash-object', '-w', '--stdin'], { cwd: this.d.repo, encoding: 'utf8', input: next }).stdout.trim(); + const blob = gitSpawnSync(['hash-object', '-w', '--stdin'], { cwd: this.d.repo, encoding: 'utf8', input: next }).stdout.trim(); mkdirSync(this.d.stateDir, { recursive: true }); const index = join(this.d.stateDir, `learning-${randomBytes(3).toString('hex')}.index`); const env = { ...process.env, GIT_INDEX_FILE: index, GIT_AUTHOR_NAME: this.identity.name, GIT_AUTHOR_EMAIL: this.identity.email, GIT_COMMITTER_NAME: this.identity.name, GIT_COMMITTER_EMAIL: this.identity.email }; const g = (args: string[], input?: string) => { - const r = spawnSync('git', args, { cwd: this.d.repo, encoding: 'utf8', env, ...(input !== undefined ? { input } : {}) }); + const r = gitSpawnSync(args, { cwd: this.d.repo, encoding: 'utf8', env, ...(input !== undefined ? { input } : {}) }); if (r.status !== 0) throw new Error(`git ${args[0]} failed: ${(r.stderr || '').trim().slice(-300)}`); return r.stdout.trim(); }; diff --git a/src/coordinator/merge-flow.ts b/src/coordinator/merge-flow.ts index 0a738c699..6e2d794fa 100644 --- a/src/coordinator/merge-flow.ts +++ b/src/coordinator/merge-flow.ts @@ -4,7 +4,7 @@ import { startRefusal, TestRunHeld } from '../testrun/gate.js'; import { CI_NONE_MAIN_WATCH, ciIsNone, requiredFor } from '../ci-none.js'; import { PinnedChecks, type JudgedChecks } from '../check-source.js'; import type { Coordinator } from '../coordinator.js'; -import { execFileSync, spawn, spawnSync } from '../os/child-process.js'; +import { gitExecFileSync, gitSpawn, gitSpawnSync } from '../os/git.js'; import { randomBytes } from 'node:crypto'; import { readFileSync, existsSync, rmSync } from 'node:fs'; import { join } from 'node:path'; @@ -132,7 +132,7 @@ export abstract class MergeFlow extends FixRuns { return git('rev-parse', '--verify', '-q', `${this.remote}/${this.branch}`); }, parent: (sha) => git('rev-parse', '--verify', '-q', `${sha}^1`), - isAncestor: (a, b) => spawnSync('git', ['merge-base', '--is-ancestor', a, b], { cwd: this.d.repo }).status === 0, + isAncestor: (a, b) => gitSpawnSync(['merge-base', '--is-ancestor', a, b], { cwd: this.d.repo }).status === 0, firstParents: (from, to) => (git('rev-list', '--first-parent', '--reverse', `${from}..${to}`) ?? '').split('\n').filter(Boolean), subject: (sha) => git('log', '-1', '--format=%s', sha) ?? '', // A rename as both its paths and a valid UTF-8 path as itself, so a locked category on either one holds (#683). @@ -320,7 +320,7 @@ export abstract class MergeFlow extends FixRuns { // A merge into the default branch (by anyone) is when PRs start conflicting: look at all of them now. if (this.watchedPrs().length && now - this.tipCheckedAt >= TIP_CHECK_MS) { this.tipCheckedAt = now; - const r = spawnSync('git', ['ls-remote', this.remote, `refs/heads/${this.branch}`], { cwd: this.d.repo, encoding: 'utf8' }); + const r = gitSpawnSync(['ls-remote', this.remote, `refs/heads/${this.branch}`], { cwd: this.d.repo, encoding: 'utf8' }); const tip = r.status === 0 ? (r.stdout.split(/\s/)[0] ?? '') : ''; if (tip && tip !== this.defaultTip) { if (this.defaultTip) this.prPolled.clear(); @@ -380,7 +380,7 @@ export abstract class MergeFlow extends FixRuns { const signal = this.shutdown.signal; const ok = await new Promise((done) => { if (signal.aborted) return done(false); - const child = spawn('git', ['fetch', '-q', this.remote, `+refs/heads/${this.branch}:refs/remotes/${this.remote}/${this.branch}`], { cwd: this.d.repo, stdio: 'ignore' }); + const child = gitSpawn(['fetch', '-q', this.remote, `+refs/heads/${this.branch}:refs/remotes/${this.remote}/${this.branch}`], { cwd: this.d.repo, stdio: 'ignore' }); const stop = () => child.kill('SIGKILL'); const timer = setTimeout(stop, this.tipFetchMs); signal.addEventListener('abort', stop, { once: true }); @@ -576,7 +576,7 @@ export abstract class MergeFlow extends FixRuns { if (!attempt) return; if (pr.state === 'merged' && pr.mergeCommit) this.git(this.d.repo, 'fetch', '-q', this.remote, this.branch); const commit = (sha: string) => { - if (spawnSync('git', ['merge-base', '--is-ancestor', sha, `${this.remote}/${this.branch}`], { cwd: this.d.repo }).status !== 0) return null; + if (gitSpawnSync(['merge-base', '--is-ancestor', sha, `${this.remote}/${this.branch}`], { cwd: this.d.repo }).status !== 0) return null; const out = this.gitOrNull(this.d.repo, 'log', '-1', '--format=%P%n%B', sha, '--'); if (out === null) return null; const [parents = '', ...message] = out.split('\n'); @@ -676,7 +676,7 @@ export abstract class MergeFlow extends FixRuns { private defaultTipNow(): string | null { if (this.tipThisPass !== undefined) return this.tipThisPass; try { - execFileSync('git', ['fetch', '-q', this.remote, `+refs/heads/${this.branch}:refs/remotes/${this.remote}/${this.branch}`], { cwd: this.d.repo, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], timeout: DEFAULT_TIP_FETCH_MS }); + gitExecFileSync(['fetch', '-q', this.remote, `+refs/heads/${this.branch}:refs/remotes/${this.remote}/${this.branch}`], { cwd: this.d.repo, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], timeout: DEFAULT_TIP_FETCH_MS }); return this.git(this.d.repo, 'rev-parse', '--verify', '-q', `refs/remotes/${this.remote}/${this.branch}^{commit}`) || null; } catch { return null; @@ -712,7 +712,7 @@ export abstract class MergeFlow extends FixRuns { const files = this.changeFiles(this.d.repo, base, pr.headSha); const labels = (this.events(n).filter((e) => e.type === 'issue.seen').at(-1)?.payload as { labels?: string[] } | undefined)?.labels ?? []; const level = computeLevel({ files, labels, moneyPaths: policy.moneyPaths, scope: lockScopeOf(this.d) }, review); - const exists = (p: string) => spawnSync('git', ['cat-file', '-e', `${base}:${p}`], { cwd: this.d.repo }).status === 0; + const exists = (p: string) => gitSpawnSync(['cat-file', '-e', `${base}:${p}`], { cwd: this.d.repo }).status === 0; const newTopLevel = [...new Set(files.map((f) => f.path.split('/')).filter((s) => s.length > 1).map((s) => s[0]!))].filter((d) => !exists(d)).map((d) => `${d}/`); // The verdict on this head, or on the head the harness's merge-ins of the default branch were made on. const judged = carriedHead(mergeIns, number, pr.headSha); @@ -1444,7 +1444,7 @@ export abstract class MergeFlow extends FixRuns { return { why: why.slice(0, 500) }; }; unlockWorktree(this.d.repo, dir); - spawnSync('git', ['worktree', 'remove', '--force', dir], { cwd: this.d.repo }); + gitSpawnSync(['worktree', 'remove', '--force', dir], { cwd: this.d.repo }); try { this.git(this.d.repo, 'worktree', 'add', '-q', '--detach', dir, tip); // Locked against a prune run inside an agent's sandbox, where it is hidden (src/worktrees.ts WORKTREE_LOCK_REASON). @@ -1452,9 +1452,9 @@ export abstract class MergeFlow extends FixRuns { const parents = this.git(this.d.repo, 'rev-list', '--parents', '-n', '1', sha).split(' ').length - 1; // A range: newest first (git revert's order), into one commit (no pre-commit or commit-msg hook, as git revert itself). const rv = range - ? spawnSync('git', [...as, 'revert', '--no-commit', `${range[0]}^..${range.at(-1)}`], { cwd: dir, encoding: 'utf8' }) - : spawnSync('git', [...as, 'revert', ...(parents > 1 ? ['-m', '1'] : []), '--no-edit', sha], { cwd: dir, encoding: 'utf8' }); - const done = rv.status === 0 && range ? spawnSync('git', [...as, 'commit', '-q', '--no-verify', '-m', `${title}\n\nThis reverts commits ${range[0]}^..${range.at(-1)}.`], { cwd: dir, encoding: 'utf8' }) : rv; + ? gitSpawnSync([...as, 'revert', '--no-commit', `${range[0]}^..${range.at(-1)}`], { cwd: dir, encoding: 'utf8' }) + : gitSpawnSync([...as, 'revert', ...(parents > 1 ? ['-m', '1'] : []), '--no-edit', sha], { cwd: dir, encoding: 'utf8' }); + const done = rv.status === 0 && range ? gitSpawnSync([...as, 'commit', '-q', '--no-verify', '-m', `${title}\n\nThis reverts commits ${range[0]}^..${range.at(-1)}.`], { cwd: dir, encoding: 'utf8' }) : rv; if (done.status !== 0) { const lines = `${done.stdout ?? ''}\n${done.stderr ?? ''}`.split('\n').map((l) => l.trim()).filter(Boolean); const conflicts = lines.filter((l) => l.startsWith('CONFLICT')); @@ -1468,8 +1468,8 @@ export abstract class MergeFlow extends FixRuns { return { url: pr.url, number: pr.number }; } finally { unlockWorktree(this.d.repo, dir); - spawnSync('git', ['worktree', 'remove', '--force', dir], { cwd: this.d.repo }); - spawnSync('git', ['worktree', 'prune'], { cwd: this.d.repo }); + gitSpawnSync(['worktree', 'remove', '--force', dir], { cwd: this.d.repo }); + gitSpawnSync(['worktree', 'prune'], { cwd: this.d.repo }); } } @@ -1575,9 +1575,9 @@ export abstract class MergeFlow extends FixRuns { */ private revertRange(sha: string, through: string, commits: number): string[] | null { const tip = `${this.remote}/${this.branch}`; - if (spawnSync('git', ['merge-base', '--is-ancestor', through, tip], { cwd: this.d.repo }).status !== 0) return null; + if (gitSpawnSync(['merge-base', '--is-ancestor', through, tip], { cwd: this.d.repo }).status !== 0) return null; // Parents before children whatever the commit dates say, so only the checks below decide. - const r = spawnSync('git', ['rev-list', '--topo-order', '--parents', '--reverse', `${sha}^..${through}`], { cwd: this.d.repo, encoding: 'utf8' }); + const r = gitSpawnSync(['rev-list', '--topo-order', '--parents', '--reverse', `${sha}^..${through}`], { cwd: this.d.repo, encoding: 'utf8' }); if (r.status !== 0) return null; const rows = r.stdout.split('\n').filter(Boolean).map((l) => l.split(' ')); if (rows.length < 2 || rows.length > Math.min(commits, 250) || rows[0]![0] !== sha || rows.at(-1)![0] !== through) return null; @@ -1587,7 +1587,7 @@ export abstract class MergeFlow extends FixRuns { /** Whether `a` is `b` or an ancestor of it, in the clone. */ private isAncestor(a: string, b: string) { - return spawnSync('git', ['--no-replace-objects', 'merge-base', '--is-ancestor', a, b], { cwd: this.d.repo }).status === 0; + return gitSpawnSync(['--no-replace-objects', 'merge-base', '--is-ancestor', a, b], { cwd: this.d.repo }).status === 0; } /** @@ -1648,7 +1648,7 @@ export abstract class MergeFlow extends FixRuns { } catch { who = null; } - const s = spawnSync('git', ['log', '-1', '--format=%s', sha], { cwd: this.d.repo, encoding: 'utf8' }); + const s = gitSpawnSync(['log', '-1', '--format=%s', sha], { cwd: this.d.repo, encoding: 'utf8' }); const subject = redactString(s.status === 0 ? s.stdout.trim() : '').slice(0, 300); this.emit('main_watch.direct_push', { sha, url, kind: push.kind, pusher: who?.login ?? null, pusher_from: who?.from ?? null, subject, alert: push.alert }); await this.directPushOutbound(); @@ -1713,7 +1713,7 @@ export abstract class MergeFlow extends FixRuns { } catch { // already gone } - spawnSync('git', ['branch', '-D', `${BRAND.cli}/combined-${n}`], { cwd: this.d.repo }); + gitSpawnSync(['branch', '-D', `${BRAND.cli}/combined-${n}`], { cwd: this.d.repo }); this.emit('light_check.finished', { issue: n, number, head: pr.headSha, main_sha: mainSha, overlap, outcome, wait_ms: Math.max(0, Date.now() - t0), detail }); this.prPolled.delete(number); // decide on the next pass, not after the poll interval } @@ -1737,11 +1737,11 @@ export abstract class MergeFlow extends FixRuns { private vacuityBase(base: string, head: string, tipNow: string | null): { base: string; foreign: boolean; tip?: 'unknown' } { const tip = tipNow ?? ''; if (!/^[0-9a-f]{40}(?:[0-9a-f]{24})?$/.test(tip)) return { base, foreign: false, tip: 'unknown' }; - const r = spawnSync('git', ['--no-replace-objects', 'merge-base', tip, head], { cwd: this.d.repo, encoding: 'utf8' }); + const r = gitSpawnSync(['--no-replace-objects', 'merge-base', tip, head], { cwd: this.d.repo, encoding: 'utf8' }); const mb = r.status === 0 ? r.stdout.trim() : ''; const from = /^[0-9a-f]{40}(?:[0-9a-f]{24})?$/.test(mb) && mb !== base && mb !== head && this.isAncestor(base, mb) ? mb : base; // Each merge commit in from..head, with its parents: ` ...`. - const merges = spawnSync('git', ['--no-replace-objects', 'rev-list', '--merges', '--parents', `${from}..${head}`], { cwd: this.d.repo, encoding: 'utf8', maxBuffer: 16 * 1024 * 1024 }); + const merges = gitSpawnSync(['--no-replace-objects', 'rev-list', '--merges', '--parents', `${from}..${head}`], { cwd: this.d.repo, encoding: 'utf8', maxBuffer: 16 * 1024 * 1024 }); if (merges.status !== 0) return { base: from, foreign: true }; const foreign = merges.stdout.split('\n').filter(Boolean).some((line) => line.split(' ').slice(2).some((p) => !this.isAncestor(p, tip))); return { base: from, foreign }; @@ -1753,7 +1753,7 @@ export abstract class MergeFlow extends FixRuns { */ private vacuityFor(n: number, number: number, head: string, changed: string[], foreignMerge: boolean, tip: 'unknown' | undefined, from: string): VacuityCheck { // change-files.ts's guards (#683): no replace ref swaps a blob, no attributes file changes what is read. - const git = (args: string[]) => execFileSync('git', ['--no-replace-objects', '-c', 'core.attributesFile=/dev/null', ...args], { cwd: this.d.repo, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], maxBuffer: 64 * 1024 * 1024 }); + const git = (args: string[]) => gitExecFileSync(['--no-replace-objects', '-c', 'core.attributesFile=/dev/null', ...args], { cwd: this.d.repo, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], maxBuffer: 64 * 1024 * 1024 }); // The test files the task's agreed done_when names: what the coordinator's own verification ran. const contract = this.events(n).filter((e) => e.type === 'contract.agreed').at(-1)?.payload as EventPayload<'contract.agreed'> | undefined; const contractTests = (contract?.done_when ?? []).map((d) => d.test).filter((t): t is string => typeof t === 'string' && t.length > 0); @@ -1873,7 +1873,7 @@ export abstract class MergeFlow extends FixRuns { } catch { // already gone } - spawnSync('git', ['branch', '-D', branch], { cwd: this.d.repo }); + gitSpawnSync(['branch', '-D', branch], { cwd: this.d.repo }); // A shutdown cut it short: nothing was measured, so nothing is recorded; a later start runs it again (capped). if (!this.shutdown.signal.aborted) this.emit('vacuity.measured', { issue: n, number, head: pr.headSha, tests: run.tests, touched, error: error === null ? null : error.slice(0, 500), base: run.base }); } @@ -1891,7 +1891,7 @@ export abstract class MergeFlow extends FixRuns { const text = await new Promise((done, fail) => { let s = ''; // change-files.ts's command: renames paired, whitespace and blank lines left out, hunks never joined, as text. - const child = spawn('git', changedLinesCommand(base, head), { cwd: this.d.repo, stdio: ['ignore', 'pipe', 'ignore'] }); + const child = gitSpawn(changedLinesCommand(base, head), { cwd: this.d.repo, stdio: ['ignore', 'pipe', 'ignore'] }); const stop = () => child.kill('SIGKILL'); signal.addEventListener('abort', stop, { once: true }); child.stdout!.setEncoding('utf8'); diff --git a/src/coordinator/pr-batch.ts b/src/coordinator/pr-batch.ts index 5196a42aa..74c9cec2e 100644 --- a/src/coordinator/pr-batch.ts +++ b/src/coordinator/pr-batch.ts @@ -11,7 +11,7 @@ // back in the queue), and nothing is merged twice. import { TestRunHeld } from '../testrun/gate.js'; import { GPU_NO_REPEAT, gpuRunsNotToRepeat } from '../testrun/gpu.js'; -import { spawnSync } from '../os/child-process.js'; +import { gitSpawnSync } from '../os/git.js'; import { BRAND } from '../brand.js'; import type { CommitCheck, PullRequest } from '../backlog/types.js'; import type { JudgedChecks } from '../check-source.js'; @@ -623,7 +623,7 @@ export class PrBatcher { } private git(...args: string[]) { - return spawnSync('git', args, { cwd: this.d.repo, encoding: 'utf8' }); + return gitSpawnSync(args, { cwd: this.d.repo, encoding: 'utf8' }); } private gitOk(...args: string[]): string { diff --git a/src/coordinator/pr-hygiene.ts b/src/coordinator/pr-hygiene.ts index ecfea162b..44f7687bc 100644 --- a/src/coordinator/pr-hygiene.ts +++ b/src/coordinator/pr-hygiene.ts @@ -2,7 +2,7 @@ // over the open PRs this instance opened (watchedPrs: a person's PR is never among them). The git work happens in a // scratch worktree of the coordinator's clone; the only push is checkedPush to the PR's own branch, pinned to the // head it read (never over someone else's push, never the default branch). Nothing runs under an emergency stop. -import { spawnSync } from '../os/child-process.js'; +import { gitSpawnSync } from '../os/git.js'; import { join } from 'node:path'; import { BRAND } from '../brand.js'; import type { PullRequest } from '../backlog/types.js'; @@ -113,8 +113,8 @@ export abstract class PrHygiene extends Gardener { const dir = join(this.d.stateDir, 'pr-hygiene', `pr-${number}`); const clean = () => { unlockWorktree(this.d.repo, dir); - spawnSync('git', ['worktree', 'remove', '--force', dir], { cwd: this.d.repo }); - spawnSync('git', ['worktree', 'prune'], { cwd: this.d.repo }); + gitSpawnSync(['worktree', 'remove', '--force', dir], { cwd: this.d.repo }); + gitSpawnSync(['worktree', 'prune'], { cwd: this.d.repo }); }; clean(); try { diff --git a/src/coordinator/test-runs.ts b/src/coordinator/test-runs.ts index 8f4ae6b0c..0372eaa35 100644 --- a/src/coordinator/test-runs.ts +++ b/src/coordinator/test-runs.ts @@ -3,7 +3,7 @@ // new run, and the superseded one is cancelled. Full tiers run per batch of approved PRs (PR mode, Track PB): the // harness's, and people's PRs from this listing (merge-flow.ts batchPeoplesPrs): a mapped admin's or writer's once its // fast checks pass; a bot's or an unmapped author's only once an admin approved it. -import { execFileSync } from '../os/child-process.js'; +import { gitExecFileSync } from '../os/git.js'; import { BRAND } from '../brand.js'; import { changedNames } from './change-files.js'; import type { EventLog } from '../events/log.js'; @@ -154,7 +154,7 @@ export class PrTestRuns { private fetchPr(pr: OpenPr): string { if (this.d.fetchPr) return this.d.fetchPr(pr); const ref = `refs/${BRAND.cli}/test-run/${pr.number}`; - const git = (...a: string[]) => execFileSync('git', ['-C', this.d.repo, ...a], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], timeout: 180_000 }).trim(); + const git = (...a: string[]) => gitExecFileSync(['-C', this.d.repo, ...a], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], timeout: 180_000 }).trim(); git('fetch', '-q', this.d.remote, `+refs/pull/${pr.number}/head:${ref}`, `+refs/heads/${pr.base}:refs/remotes/${this.d.remote}/${pr.base}`); return git('rev-parse', `${ref}^{commit}`); } diff --git a/src/dashboard.ts b/src/dashboard.ts index af7be4607..6b6af1cdd 100644 --- a/src/dashboard.ts +++ b/src/dashboard.ts @@ -14,6 +14,7 @@ import { PINS_FILE } from './experiments/store.js'; import { handleHook, hooksGatewayToken } from './automations/webhook.js'; import { AUTOMATIONS_FILE, AutomationsPolicy, limitsInForce } from './automations/schema.js'; import { spawnSync } from './os/child-process.js'; +import { gitSpawnSync } from './os/git.js'; import { createServer, type IncomingMessage, type Server, type ServerResponse } from 'node:http'; import { createHash, randomBytes, timingSafeEqual } from 'node:crypto'; import { existsSync, mkdirSync, readdirSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs'; @@ -1298,7 +1299,7 @@ export function startDashboard(opts: DashboardOptions): Promise<{ server: Server }; /** The test files the repo's runner can name (the first-task form's picker): tracked files that look like tests. */ const testFiles = (): string[] => { - const r = spawnSync('git', ['ls-files'], { cwd: opts.root, encoding: 'utf8', maxBuffer: 32 * 1024 * 1024 }); + const r = gitSpawnSync(['ls-files'], { cwd: opts.root, encoding: 'utf8', maxBuffer: 32 * 1024 * 1024 }); if (r.status !== 0) return []; return r.stdout.split('\n').filter((f) => /(^|\/)(tests?|__tests__|spec)\/.+\.[a-z]+$|\.(test|spec)\.[cm]?[jt]sx?$|_test\.(go|py|rb)$|(^|\/)test_[^/]+\.py$/.test(f)).slice(0, 300); }; @@ -2285,7 +2286,7 @@ export function startDashboard(opts: DashboardOptions): Promise<{ server: Server if (url.pathname === '/api/first-run/checks-seen' && req.method === 'GET') { if (!isWriter(who.role)) return json(res, 403, { error: writersOnly('the first-run checklist is', who) }); const branch = opts.cfg.project.project.default_branch; - const sha = spawnSync('git', ['rev-parse', `origin/${branch}`], { cwd: opts.root, encoding: 'utf8' }).stdout.trim(); + const sha = gitSpawnSync(['rev-parse', `origin/${branch}`], { cwd: opts.root, encoding: 'utf8' }).stdout.trim(); if (!/^[0-9a-f]{40}$/.test(sha)) return json(res, 200, { checks: [], error: `origin/${branch} isn't fetched here` }); const gh = actionDeps().gh; const runs = gh(['api', `repos/${repo}/commits/${sha}/check-runs`, '--jq', '.check_runs[].name']); diff --git a/src/decision-detail.ts b/src/decision-detail.ts index e2e009a3d..402b732e5 100644 --- a/src/decision-detail.ts +++ b/src/decision-detail.ts @@ -9,7 +9,7 @@ // - a viewer gets only the fields VIEWER_DECISION_FIELDS allows (src/viewer-events.ts), never the context; // - a writer gets the context too, as the agent runs it comes from (mayReadRuns), minus the owner-only parts; // - the owners also get the owner-only parts: raw check output, the tamper guard's flags, and the diff. -import { execFile } from './os/child-process.js'; +import { gitExecFile } from './os/git.js'; import { BRAND } from './brand.js'; import { DEFAULT_REVIEW } from './coordinator/shared.js'; import { isOwner, isWriter, mayAnswerKind, type Role } from './dashboard-auth.js'; @@ -346,12 +346,16 @@ export function decisionDetail(id: string, d: DetailDeps): DecisionDetail | null }; } -/** The change's diff, base..head, from the checkout's git (owners only: the route checks); redacted, capped. */ +/** + * The change's diff, base..head, from the checkout's git (owners only: the route checks); redacted, capped. main's + * arguments, through the git runner (#807): a binary file shows as "Binary files ... differ" (the owner, 2026-10-21: + * "Cut --text, land the runner."). + */ export async function decisionDiff(root: string, base: string, head: string): Promise<{ diff: string; truncated: boolean } | { diff: null; why: string }> { if (!SHA.test(base) || !SHA.test(head)) return { diff: null, why: 'no commits recorded for this change' }; const MAX = 512 * 1024; return new Promise((done) => { - execFile('git', ['diff', '--no-color', '--no-ext-diff', `${base}..${head}`], { cwd: root, env: { ...withToolPath(), GIT_TERMINAL_PROMPT: '0' }, timeout: 15_000, maxBuffer: 8 * 1024 * 1024, encoding: 'utf8' }, (err, stdout, stderr) => { + gitExecFile(['diff', '--no-color', '--no-ext-diff', `${base}..${head}`], { cwd: root, env: { ...withToolPath(), GIT_TERMINAL_PROMPT: '0' }, timeout: 15_000, maxBuffer: 8 * 1024 * 1024, encoding: 'utf8' }, (err, stdout, stderr) => { if (err) return done({ diff: null, why: `not in this checkout's git: ${(stderr || err.message).trim().split('\n')[0]}` }); const truncated = stdout.length > MAX; // Redacted before it is cut, over the kept part and a margin only (#889): up to 8 MB of diff, and the redaction is diff --git a/src/demo.ts b/src/demo.ts index 64197560d..75f027071 100644 --- a/src/demo.ts +++ b/src/demo.ts @@ -1,7 +1,8 @@ // A self-contained demo: a copy of the example project with a local "GitHub" // remote and a file backlog, worked by the real coordinator with scripted // agents, so the event log (and the dashboard) shows every state for real. -import { execFileSync, spawnSync } from './os/child-process.js'; +import { spawnSync } from './os/child-process.js'; +import { gitExecFileSync } from './os/git.js'; import { cpSync, existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -17,7 +18,7 @@ import { childEnv } from './os/index.js'; import { DEFAULT_COMMIT_IDENTITY, FakeRunner, type RunRequest } from './runner.js'; import { writeSlotFile } from './slot-file.js'; -const git = (cwd: string, ...a: string[]) => execFileSync('git', a, { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); +const git = (cwd: string, ...a: string[]) => gitExecFileSync(a, { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); function commitAll(cwd: string, msg: string) { git(cwd, 'add', '-A'); diff --git a/src/deploy-gate/wire.ts b/src/deploy-gate/wire.ts index ec30b9c71..153de383c 100644 --- a/src/deploy-gate/wire.ts +++ b/src/deploy-gate/wire.ts @@ -1,6 +1,6 @@ // The deploy gate's real git and GitHub calls, and setting it up at coordinator start (only with tests.yaml // deploy_gate, and only once the instance's test runs are set up: the gate runs its tiers through them). -import { spawnSync } from '../os/child-process.js'; +import { gitSpawnSync } from '../os/git.js'; import { existsSync, readFileSync } from 'node:fs'; import { homedir } from 'node:os'; import { join } from 'node:path'; @@ -21,7 +21,7 @@ type Fetch = typeof fetch; /** The gate's git over the coordinator's clone: both branches fetched (the deploy branch only when it exists). */ export function gateGit(repo: string, remote: string, main: string, deploy: string): GateGit { - const git = (...a: string[]) => spawnSync('git', a, { cwd: repo, encoding: 'utf8', timeout: 180_000, maxBuffer: 64 * 1024 * 1024 }); + const git = (...a: string[]) => gitSpawnSync(a, { cwd: repo, encoding: 'utf8', timeout: 180_000, maxBuffer: 64 * 1024 * 1024 }); return { fetch: () => { const ls = git('ls-remote', remote, `refs/heads/${deploy}`); diff --git a/src/direct-land.ts b/src/direct-land.ts index 41769ce86..3f2225082 100644 --- a/src/direct-land.ts +++ b/src/direct-land.ts @@ -5,7 +5,7 @@ // and refuses to start unless GitHub enforces the branch's protection; a check that errors refuses too; // - every direct land asks again just before its push, and pushes nothing unless the answer is "enforced". // A remote that is a local path (a bare repo on disk: the demo, the tests) has no protection to ask about: allowed. -import { spawnSync } from './os/child-process.js'; +import { gitSpawnSync } from './os/git.js'; import { existsSync } from 'node:fs'; import type { EventLog } from './events/log.js'; import type { EventPayload, StoredEvent } from './events/types.js'; @@ -15,7 +15,7 @@ export type Protection = Pick; /** Whether `remote` in `repo` is a path on this machine (or file://), not a host that could protect a branch. */ export function localRemote(repo: string, remote: string): boolean { - const r = spawnSync('git', ['remote', 'get-url', remote], { cwd: repo, encoding: 'utf8' }); + const r = gitSpawnSync(['remote', 'get-url', remote], { cwd: repo, encoding: 'utf8' }); const url = r.status === 0 ? r.stdout.trim() : ''; if (!url) return false; return url.startsWith('file://') || existsSync(url); diff --git a/src/doctor.ts b/src/doctor.ts index 1506f364f..c45d3459d 100644 --- a/src/doctor.ts +++ b/src/doctor.ts @@ -3,6 +3,7 @@ // that can't run reports why, and a feature the setup's mode can't have is // listed as off, with why. import { execFileSync, spawnSync } from './os/child-process.js'; +import { gitExecFileSync } from './os/git.js'; import { existsSync, lstatSync, readFileSync } from 'node:fs'; import { join, resolve } from 'node:path'; import { agentRecordWarning } from './setup-steps.js'; @@ -223,7 +224,7 @@ export function doctor(rootArg: string, opts: { agentshield?: boolean; /** A pol if (!gl) add('gitleaks', 'fail', `not on PATH or in ${toolBin()}: commit and transcript scans cannot run`); else add('gitleaks', 'ok', execFileSync(gl, ['version'], { encoding: 'utf8' }).trim()); try { - const common = execFileSync('git', ['rev-parse', '--git-common-dir'], { cwd: root, encoding: 'utf8' }).trim(); + const common = gitExecFileSync(['rev-parse', '--git-common-dir'], { cwd: root, encoding: 'utf8' }).trim(); const hook = join(resolve(root, common), 'hooks', 'pre-commit'); if (existsSync(hook) && readFileSync(hook, 'utf8').includes(GIT_HOOK_MARKER)) add('pre-commit', 'ok', 'gitleaks pre-commit installed'); else add('pre-commit', 'warn', `not installed (\`${BRAND.cli} install --git-hooks\`); the land queue still re-scans`); diff --git a/src/experiments/view.ts b/src/experiments/view.ts index ce1fd81a1..9c45440b5 100644 --- a/src/experiments/view.ts +++ b/src/experiments/view.ts @@ -3,7 +3,7 @@ // Who may do what is here too, so the page and the tests share one rule: owners start, resume, approve, decline and // archive; writers (and owners) pause and stop a running job, the safe direction; viewers nothing (403, no nav item). import { createHash } from 'node:crypto'; -import { execFileSync } from '../os/child-process.js'; +import { gitExecFileSync } from '../os/git.js'; import type { Role } from '../dashboard-auth.js'; import type { EventLog } from '../events/log.js'; import type { EventPayload, StoredEvent } from '../events/types.js'; @@ -51,7 +51,7 @@ export const noExperimentsHere = (instance: string | undefined) => const show = (repo: string) => (commit: string, path: string) => { try { - return execFileSync('git', ['show', `${commit}:${path}`], { cwd: repo, stdio: ['ignore', 'pipe', 'ignore'], maxBuffer: 64 * 1024 * 1024 }); + return gitExecFileSync(['show', `${commit}:${path}`], { cwd: repo, stdio: ['ignore', 'pipe', 'ignore'], maxBuffer: 64 * 1024 * 1024 }); } catch { return null; } @@ -67,7 +67,7 @@ const diffOf = (repo: string, commit: string | null): string[] => { }; const tipOf = (site: ExpSite) => { try { - return execFileSync('git', ['rev-parse', `origin/${site.branch}`], { cwd: site.repo, encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] }).trim(); + return gitExecFileSync(['rev-parse', `origin/${site.branch}`], { cwd: site.repo, encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] }).trim(); } catch { return null; } diff --git a/src/harness-evals/replay.ts b/src/harness-evals/replay.ts index 5ed3f71de..c004a7c0d 100644 --- a/src/harness-evals/replay.ts +++ b/src/harness-evals/replay.ts @@ -4,7 +4,7 @@ // scratch clone is deleted when its replay ends. The engine is the one running the replay (the harness under test); // the task's project config is its own at the base, except the harness config taken from the harness under test // (HARNESS_CONFIG_PATHS), so a change to that config is what the replay measures. -import { execFileSync } from '../os/child-process.js'; +import { gitExecFileSync } from '../os/git.js'; import { randomBytes } from 'node:crypto'; import { existsSync, lstatSync, mkdirSync, readdirSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { dirname, isAbsolute, join, relative, resolve, sep } from 'node:path'; @@ -56,7 +56,7 @@ export class BudgetRunner implements AgentRunner { } } -const git = (cwd: string, ...a: string[]) => execFileSync('git', a, { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], timeout: 600_000 }).trim(); +const git = (cwd: string, ...a: string[]) => gitExecFileSync(a, { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], timeout: 600_000 }).trim(); /** The engine's own identity on the scratch config commit, as an agent session commits. */ function commit(cwd: string, msg: string) { diff --git a/src/harness-evals/schedule.ts b/src/harness-evals/schedule.ts index 1f91410a7..8a3d9e44c 100644 --- a/src/harness-evals/schedule.ts +++ b/src/harness-evals/schedule.ts @@ -1,7 +1,7 @@ // When the harness eval suite runs: on this machine, from the nightly window (tests.yaml nightly_at), at most once a // day; weekly, and when the engine running here changed src/ since the last run. Which engine is running comes from // the engine itself: its git commit, or the commit its install directory is named after (/opt//). -import { spawnSync } from '../os/child-process.js'; +import { gitSpawnSync } from '../os/git.js'; import { existsSync, realpathSync } from 'node:fs'; import { basename, join } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -15,7 +15,7 @@ const SHA = /^[0-9a-f]{40}$/; /** The commit of the engine running this code, or null when it can't tell. */ export function engineSha(root: string = ENGINE_ROOT): string | null { if (existsSync(join(root, '.git'))) { - const r = spawnSync('git', ['rev-parse', 'HEAD'], { cwd: root, encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] }); + const r = gitSpawnSync(['rev-parse', 'HEAD'], { cwd: root, encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] }); const sha = r.status === 0 ? r.stdout.trim() : ''; if (SHA.test(sha)) return sha; } @@ -31,7 +31,7 @@ export function engineSha(root: string = ENGINE_ROOT): string | null { export function srcChanged(from: string, to: string, clones: string[]): boolean | null { for (const cwd of clones) { if (!existsSync(join(cwd, '.git'))) continue; - const r = spawnSync('git', ['diff', '--quiet', from, to, '--', 'src/'], { cwd, stdio: 'ignore' }); + const r = gitSpawnSync(['diff', '--quiet', from, to, '--', 'src/'], { cwd, stdio: 'ignore' }); if (r.status === 0) return false; if (r.status === 1) return true; } diff --git a/src/hook.ts b/src/hook.ts index 80b31c425..5fe57383b 100644 --- a/src/hook.ts +++ b/src/hook.ts @@ -4,6 +4,7 @@ // closed (deny or block with a reason); the generated settings also append // `|| exit 2`, so a missing or crashing engine blocks instead of allowing. import { spawnSync } from './os/child-process.js'; +import { gitSpawnSync } from './os/git.js'; import { appendFileSync, existsSync, mkdirSync, readFileSync } from 'node:fs'; import { dirname, isAbsolute, join, relative, resolve, sep } from 'node:path'; import { childEnv, shellCommand } from './os/index.js'; @@ -190,7 +191,7 @@ function preToolUse(input: HookInput, env: NodeJS.ProcessEnv): HookOutput { /** A read-only git query for the commit model: its exit status and output, never an exception. */ function gitQuery(dir: string, args: string[]): { status: number | null; stdout: string } { - const r = spawnSync('git', args, { cwd: dir, encoding: 'utf8', timeout: 5_000 }); + const r = gitSpawnSync(args, { cwd: dir, encoding: 'utf8', timeout: 5_000 }); return { status: r.error ? null : r.status, stdout: r.stdout ?? '' }; } diff --git a/src/init.ts b/src/init.ts index 6dcdb2a98..347529a6f 100644 --- a/src/init.ts +++ b/src/init.ts @@ -12,7 +12,8 @@ // agent's Claude token and the checkout). Init never installs tools there: that stays the owner's step. // // What local mode can't do is listed plainly (unavailableIn), in doctor and on the dashboard, never left to fail. -import { spawn, spawnSync } from './os/child-process.js'; +import { spawn } from './os/child-process.js'; +import { commandSpawnSync } from './os/git.js'; import { existsSync, mkdirSync, readFileSync, statSync, writeFileSync } from 'node:fs'; import { delimiter, dirname, join, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -653,7 +654,8 @@ export function systemIo(o: { interactive: boolean }): InitIo { return { which: (cmd) => findTool(cmd), run: (cmd, args, opt = {}) => { - const r = spawnSync(cmd, args, { encoding: 'utf8', timeout: 60_000, env: probeEnv(opt), ...(opt.cwd ? { cwd: opt.cwd } : {}) }); + // A caller's git (the command named git) through the git runner (#807). + const r = commandSpawnSync(cmd, args, { encoding: 'utf8', timeout: 60_000, env: probeEnv(opt), ...(opt.cwd ? { cwd: opt.cwd } : {}) }); return { status: r.error ? null : r.status, stdout: r.stdout ?? '', stderr: r.error ? r.error.message : (r.stderr ?? '') }; }, serviceManagerEnv: () => serviceManagerEnv(MANAGER_LOGIN_VARS), diff --git a/src/install.ts b/src/install.ts index 6325c5488..96ab2196f 100644 --- a/src/install.ts +++ b/src/install.ts @@ -6,7 +6,7 @@ // checkout's .claude/settings.json (keeping the project's own entries), for // local runs without an instance. Re-running is safe: our entries are // recognized by a marker and replaced, never duplicated. -import { execFileSync } from './os/child-process.js'; +import { gitExecFileSync } from './os/git.js'; import { chmodSync, cpSync, existsSync, mkdirSync, readFileSync, readlinkSync, renameSync, writeFileSync } from 'node:fs'; import { basename, dirname, isAbsolute, join, relative, resolve } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -202,7 +202,7 @@ function readJson(path: string): Settings { export function gitRemoteRepo(root: string): string | null { try { // WHY trim: git ends its answer with a newline; the shared parser (src/managed-apps.ts) takes a URL as given. - return githubRepoOf(execFileSync('git', ['remote', 'get-url', 'origin'], { cwd: root, encoding: 'utf8' }).trim()); + return githubRepoOf(gitExecFileSync(['remote', 'get-url', 'origin'], { cwd: root, encoding: 'utf8' }).trim()); } catch { return null; } @@ -346,14 +346,14 @@ export function installGitHooks(root: string, cfg: Config, engineCli = fileURLTo * `branches` (the config's, when there is one) and whatever origin/HEAD names at push time. */ export function installCheckoutPrePush(root: string, branches: string[], engineCli?: string, node: string = stableNodePath().path): string { - const common = execFileSync('git', ['rev-parse', '--git-common-dir'], { cwd: root, encoding: 'utf8' }).trim(); + const common = gitExecFileSync(['rev-parse', '--git-common-dir'], { cwd: root, encoding: 'utf8' }).trim(); // WHY (#468): the checkout's hook runs node by Homebrew's stable bin/node (or this node's own path), as the service does. return installPrePushHook(join(resolve(root, common), 'hooks'), { branches, node, ...(engineCli ? { engine: engineCli } : {}) }); } /** gitleaks pre-commit hook in the repo's common hooks dir, chaining any existing hook. */ export function installGitHook(root: string): string { - const common = execFileSync('git', ['rev-parse', '--git-common-dir'], { cwd: root, encoding: 'utf8' }).trim(); + const common = gitExecFileSync(['rev-parse', '--git-common-dir'], { cwd: root, encoding: 'utf8' }).trim(); const hooksDir = join(resolve(root, common), 'hooks'); mkdirSync(hooksDir, { recursive: true }); const hook = join(hooksDir, 'pre-commit'); diff --git a/src/main-watch.ts b/src/main-watch.ts index b04d3e490..1d15b945e 100644 --- a/src/main-watch.ts +++ b/src/main-watch.ts @@ -31,7 +31,7 @@ // The required_checks a commit is judged by are the ones in force when it landed: config.yaml as committed at its // first parent, read with git. Not the config loaded at startup: the clone's working tree isn't updated as the default // branch moves, so a check dropped since would still be required of every later merge, and never report. -import { spawnSync } from './os/child-process.js'; +import { gitSpawnSync } from './os/git.js'; import { createHash, randomBytes } from 'node:crypto'; import { existsSync, mkdirSync, readFileSync, renameSync } from 'node:fs'; import { dirname, join } from 'node:path'; @@ -615,7 +615,7 @@ export function subjectPrNumber(subject: string): number | null { } export function repoGit(repo: string, remote: string, branch: string): WatchGit { - const git = (...a: string[]) => spawnSync('git', a, { cwd: repo, encoding: 'utf8' }); + const git = (...a: string[]) => gitSpawnSync(a,{ cwd: repo, encoding: 'utf8' }); return { tip: () => { git('fetch', '-q', remote, `+refs/heads/${branch}:refs/remotes/${remote}/${branch}`); @@ -936,7 +936,7 @@ export interface AcceptGit extends WatchGit { /** AcceptGit over a clone (the instance's repo): the tip is fetched as the watch fetches it. */ export function acceptGit(repo: string, remote: string, branch: string): AcceptGit { - const git = (...a: string[]) => spawnSync('git', a, { cwd: repo, encoding: 'utf8' }); + const git = (...a: string[]) => gitSpawnSync(a,{ cwd: repo, encoding: 'utf8' }); return { ...repoGit(repo, remote, branch), resolve: (ref) => { diff --git a/src/merge-base.ts b/src/merge-base.ts index 9f58ecfab..77c009a20 100644 --- a/src/merge-base.ts +++ b/src/merge-base.ts @@ -2,13 +2,13 @@ // coordinator owns: merge a base into the checked-out branch and report what // conflicted. Merge commits only here (the default strategy); conflict // markers use diff3 style so a resolver also sees the common ancestor. -import { spawnSync } from './os/child-process.js'; +import { gitSpawnSync } from './os/git.js'; import { readFileSync } from 'node:fs'; import { join } from 'node:path'; import { changedNames, changedNamesOrNull, presentNames } from './coordinator/change-files.js'; import { isNonUtf8Path } from './review.js'; -const git = (cwd: string, ...a: string[]) => spawnSync('git', a, { cwd, encoding: 'utf8' }); +const git = (cwd: string, ...a: string[]) => gitSpawnSync(a, { cwd, encoding: 'utf8' }); /** A blob's id at `rev:path` (or the index's, `:path`), or null when there is none. */ const blob = (cwd: string, spec: string) => { const r = git(cwd, 'rev-parse', '--verify', '-q', spec); diff --git a/src/merge-policy-weakening.ts b/src/merge-policy-weakening.ts index 956217353..b5ac40a71 100644 --- a/src/merge-policy-weakening.ts +++ b/src/merge-policy-weakening.ts @@ -216,6 +216,8 @@ export const GUARD_FILES: readonly string[] = [ 'src/os/system-paths.ts', 'src/os/own-env.ts', 'src/os/child-process.ts', + // The git runner (#927): src/coordinator/change-files.ts reads through it, so it runs in a backstop's process too. + 'src/os/git.ts', ]; /** A package manager's settings: every one changes what install and each script run under (`script-shell=`, `ignore-scripts=`). */ diff --git a/src/nightly.ts b/src/nightly.ts index 702ff0fa7..786d2488e 100644 --- a/src/nightly.ts +++ b/src/nightly.ts @@ -2,7 +2,7 @@ // baseline) plus any extra nightly tiers (e.g. mutation testing), queued // behind the machine-wide full-run lock, the project's idle probe and the // load gate, so they never overlap another full run. -import { execFileSync } from './os/child-process.js'; +import { gitExecFileSync } from './os/git.js'; import { fileURLToPath } from 'node:url'; import { BRAND } from './brand.js'; import type { Config } from './config/load.js'; @@ -31,8 +31,8 @@ export function queueBaselineRun(root: string, cfg: Config, eventsDb: string, ac const fmt = cfg.tests.failures; if (!fmt) throw new Error('tests.yaml needs a failures: { section, item } format to record a baseline'); const branch = cfg.project.project.default_branch; - execFileSync('git', ['fetch', '-q', 'origin', branch], { cwd: root }); - const sha = execFileSync('git', ['rev-parse', `origin/${branch}`], { cwd: root, encoding: 'utf8' }).trim(); + gitExecFileSync(['fetch', '-q', 'origin', branch], { cwd: root }); + const sha = gitExecFileSync(['rev-parse', `origin/${branch}`], { cwd: root, encoding: 'utf8' }).trim(); const state = projectStateDir(root); const { o, name, path } = baselineWorktree(root, cfg, sha, runAs, wt); return queueJob({ diff --git a/src/os/git.ts b/src/os/git.ts new file mode 100644 index 000000000..2f2298976 --- /dev/null +++ b/src/os/git.ts @@ -0,0 +1,235 @@ +// The one way the engine runs git (#807). +// +// WHY: refs/replace/* and $GIT_DIR/info/grafts live in the repository, and every worktree of a repo shares them. An +// agent in its worktree can write either, and then every git the engine runs on that repo reads the agent's stand-in +// instead of the object asked for: a `git replace` of a pushed commit hid a refused file from the push limits (1 +// problem became 0, measured on #802's head), and a graft can give a commit other parents, so a range reads as other +// commits. Both are on by default in git. +// +// INVARIANT: every git process the engine starts goes through a function here, and each one runs with replace refs +// off (`--no-replace-objects` before the subcommand, and GIT_NO_REPLACE_OBJECTS=1, which git passes to every git it +// runs in turn) and with no graft file (GIT_GRAFT_FILE names a path that can't exist, so $GIT_DIR/info/grafts is +// never read; `--no-replace-objects` alone still honours grafts, measured on git 2.43). The settings go on last, over +// whatever environment the caller passes, so no caller can turn them off. test/git-runner.test.ts fails on a git call +// in src/ or scripts/ that doesn't come through here (or scripts/lib/git.cjs, the same settings for scripts). +// +// The commit-graph file (objects/info/commit-graph, in the shared objects dir) is the same kind of state: git trusts +// its parent links, and a forged one skipped a commit in a range (measured, #927's round-2 check). It is off for every +// git here too (`-c core.commitGraph=false`, and the same pair in the environment for gits a program runs itself). +// +// A shallow clone's `shallow` file is read (it is how a shallow clone works). A reader that must see a range whole +// takes a gitSnapshot: one copy of that file, checked, that every walk it runs reads, however the shared file changes. +// +// LIMIT: a program the engine runs that runs git itself (gitleaks) gets these settings only when it is handed gitEnv(). +import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { execFile, execFileSync, spawn, spawnSync } from './child-process.js'; +import type { + ChildProcess, + ChildProcessByStdio, + ExecFileException, + ExecFileOptions, + ExecFileOptionsWithStringEncoding, + ExecFileSyncOptions, + ExecFileSyncOptionsWithBufferEncoding, + ExecFileSyncOptionsWithStringEncoding, + SpawnOptions, + SpawnOptionsWithStdioTuple, + SpawnSyncOptions, + SpawnSyncOptionsWithBufferEncoding, + SpawnSyncOptionsWithStringEncoding, + SpawnSyncReturns, + StdioNull, + StdioPipe, +} from 'node:child_process'; +import { devNull, tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import type { Readable } from 'node:stream'; + +/** + * Before every git subcommand: replace refs off for this git (git also sets GIT_NO_REPLACE_OBJECTS for the gits it + * runs), and the commit-graph off. + */ +export const GIT_FIXED_ARGS: readonly string[] = Object.freeze(['--no-replace-objects', '-c', 'core.commitGraph=false']); + +/** git config every git also gets through its environment (GIT_CONFIG_COUNT pairs), after the caller's own pairs. */ +export const GIT_FIXED_CONFIG: readonly (readonly [string, string])[] = Object.freeze([Object.freeze(['core.commitGraph', 'false'] as const)]); + +/** + * On every git process, over the caller's environment. GIT_GRAFT_FILE is a path under the null device, which can't + * exist (ENOTDIR on Linux and macOS), so git reads no graft file and says nothing about it. + */ +export const GIT_FIXED_ENV: Readonly> = Object.freeze({ + GIT_NO_REPLACE_OBJECTS: '1', + GIT_GRAFT_FILE: join(devNull, 'no-grafts'), +}); + +/** The only GIT_CONFIG_COUNT the runner reads: a plain decimal 0 to 9999, as the engine itself writes one. */ +export const CANONICAL_CONFIG_COUNT = /^(?:0|[1-9][0-9]{0,3})$/; +/** What any other count becomes: a value git refuses ("bogus count") on every platform, so the git fails. */ +export const REFUSED_CONFIG_COUNT = 'refused'; + +/** + * `env` (or this process's environment), with the fixed settings on top, and the fixed config in it twice: + * - as GIT_CONFIG_COUNT pairs after the caller's own (a later pair wins). Only a canonical count is read. Any other + * (`-0`, `+1`, ` 1`, `01`, `0x1`, `abc`, an overlong one) is replaced with REFUSED_CONFIG_COUNT, so git fails + * instead of running: git's strtoul reads some of them (`-0`, `+1`) and refuses others, and a parse that tried to + * match it got the sign wrong (#927 rounds 8 and 9). Fails closed; + * - appended to GIT_CONFIG_PARAMETERS (what `git -c` hands the gits it runs), which git reads after the COUNT pairs: + * a caller's `git -c core.commitGraph=true` there would otherwise win (#927 round 9, measured). + * The fixed arguments turn the commit-graph off for the runner's own gits too; the environment covers the gits a + * program like gitleaks runs. + */ +export function gitEnv(env?: NodeJS.ProcessEnv): NodeJS.ProcessEnv { + const out: NodeJS.ProcessEnv = { ...(env ?? process.env), ...GIT_FIXED_ENV }; + // Own keys only: an unset value must never be read through Object.prototype (#846). + const own = (k: string) => (Object.hasOwn(out, k) ? out[k] : undefined); + const raw = own('GIT_CONFIG_COUNT'); + if (raw === undefined || CANONICAL_CONFIG_COUNT.test(raw)) { + let n = raw === undefined ? 0 : Number(raw); + for (const [k, v] of GIT_FIXED_CONFIG) { + out[`GIT_CONFIG_KEY_${n}`] = k; + out[`GIT_CONFIG_VALUE_${n}`] = v; + n++; + } + out.GIT_CONFIG_COUNT = String(n); + } else out.GIT_CONFIG_COUNT = REFUSED_CONFIG_COUNT; + const params = own('GIT_CONFIG_PARAMETERS'); + const fixed = GIT_FIXED_CONFIG.map(([k, v]) => `'${k}=${v}'`).join(' '); + out.GIT_CONFIG_PARAMETERS = params ? `${params} ${fixed}` : fixed; + return out; +} + +/** The argument list a git process gets: the fixed arguments, then `args`. */ +export function gitArgs(args: readonly string[]): string[] { + return [...GIT_FIXED_ARGS, ...args]; +} + +/** True for a command that runs git: `git`, or a path to it (`/usr/bin/git`, `C:\...\git.exe`), in any case. */ +export function isGitCommand(cmd: string): boolean { + const name = (cmd.split(/[\\/]/).pop() ?? '').toLowerCase(); + return name === 'git' || name === 'git.exe'; +} + +/** spawnSync of git. */ +export function gitSpawnSync(args: readonly string[], options: SpawnSyncOptionsWithStringEncoding): SpawnSyncReturns; +export function gitSpawnSync(args: readonly string[], options?: SpawnSyncOptionsWithBufferEncoding): SpawnSyncReturns; +export function gitSpawnSync(args: readonly string[], options?: SpawnSyncOptions): SpawnSyncReturns; +export function gitSpawnSync(args: readonly string[], options: SpawnSyncOptions = {}): SpawnSyncReturns { + return spawnSync('git', gitArgs(args), { ...options, env: gitEnv(options.env) }); +} + +/** execFileSync of git. */ +export function gitExecFileSync(args: readonly string[], options: ExecFileSyncOptionsWithStringEncoding): string; +export function gitExecFileSync(args: readonly string[], options?: ExecFileSyncOptionsWithBufferEncoding): Buffer; +export function gitExecFileSync(args: readonly string[], options?: ExecFileSyncOptions): string | Buffer; +export function gitExecFileSync(args: readonly string[], options: ExecFileSyncOptions = {}): string | Buffer { + return execFileSync('git', gitArgs(args), { ...options, env: gitEnv(options.env) }); +} + +/** spawn of git. */ +export function gitSpawn(args: readonly string[], options: SpawnOptionsWithStdioTuple): ChildProcessByStdio; +export function gitSpawn(args: readonly string[], options: SpawnOptionsWithStdioTuple): ChildProcessByStdio; +export function gitSpawn(args: readonly string[], options: SpawnOptions): ChildProcess; +export function gitSpawn(args: readonly string[], options: SpawnOptions): ChildProcess { + return spawn('git', gitArgs(args), { ...options, env: gitEnv(options.env) }); +} + +/** execFile of git, its output as text. */ +export function gitExecFile( + args: readonly string[], + options: ExecFileOptionsWithStringEncoding, + callback: (error: ExecFileException | null, stdout: string, stderr: string) => void, +): ChildProcess { + const o: ExecFileOptions = { ...options, env: gitEnv(options.env) }; + return execFile('git', gitArgs(args), o, (error, stdout, stderr) => callback(error, String(stdout), String(stderr))); +} + +/** + * spawnSync of a command a caller names at run time (a probe's or a stand-in-able runner's `run(cmd, args)`): git, + * by name or path, through gitSpawnSync, so a runner that takes any command can't run git without the settings; + * anything else as it is. + */ +export function commandSpawnSync(cmd: string, args: readonly string[], options: SpawnSyncOptionsWithStringEncoding): SpawnSyncReturns { + return isGitCommand(cmd) + ? spawnSync(cmd, gitArgs(args), { ...options, env: gitEnv(options.env) }) + : spawnSync(cmd, args, options); +} + +/** A full lowercase object id of each object format git names (`rev-parse --show-object-format`): nothing else. */ +const OBJECT_ID = new Map([['sha1', /^[0-9a-f]{40}$/], ['sha256', /^[0-9a-f]{64}$/]]); + +/** + * The commits a `shallow` file lists, or null when any line isn't exactly one full lowercase object id of the repo's + * own `format`, or the format is one this doesn't know. git reads the file more loosely: an uppercase id, text after + * the id, a line over 1 KB, and in a sha1 repo a 64-hex line, read as its first 40 (each measured to cut a range on + * git 2.43; the last got a refused file onto the remote in 34 of 80 pushes, #927 round 3). So a line this can't read + * the way git might is never taken as "no cut". + */ +export function shallowIds(text: string, format: string): string[] | null { + const id = OBJECT_ID.get(format); + if (!id) return null; + const lines = text.split('\n'); + if (lines.at(-1) === '') lines.pop(); + return lines.every((l) => id.test(l)) ? lines : null; +} + +/** Variables that would point a git at another repo, worktree, index or shallow file than the snapshot's own. */ +const REPO_VARS = ['GIT_DIR', 'GIT_WORK_TREE', 'GIT_INDEX_FILE', 'GIT_COMMON_DIR', 'GIT_OBJECT_DIRECTORY', 'GIT_SHALLOW_FILE', 'GIT_NAMESPACE']; + +/** One fixed view of a repo's history: what every walk of one check reads (#927). */ +export interface GitSnapshot { + /** The environment a git (or a program that runs git, through gitEnv) reads the snapshot with. */ + readonly env: NodeJS.ProcessEnv; + /** The commits the snapshot's shallow file cuts at (empty: not shallow). */ + readonly cut: ReadonlySet; + /** The cut commits inside base..head (object ids): each hides what is below it. */ + cutInRange(base: string, head: string): string[]; + /** Removes the snapshot's directory. */ + dispose(): void; +} + +/** + * A private git directory over `cwd`'s repo: its objects (GIT_OBJECT_DIRECTORY, with their alternates) and one copy + * of its `shallow` file, read once and checked (shallowIds). No refs, no config but its own, no replace refs and no + * grafts. A walk through `env` reads that copy, so writing or removing the repo's own shallow file mid-check changes + * nothing (an agent toggling it every ~3 ms got a refused file past the old before-and-after checks: 23 of 150 runs, + * measured). Documented environment only, not git's internal --shallow-file. Commit ids only: the snapshot has no + * refs, so resolve names in the repo first. Throws, saying why, when the repo or its shallow file can't be read. + */ +export function gitSnapshot(cwd: string): GitSnapshot { + const clean = (): NodeJS.ProcessEnv => { + const e: NodeJS.ProcessEnv = { ...process.env }; + for (const k of REPO_VARS) delete e[k]; + return e; + }; + const [format, objectsPath, shallowPath] = gitExecFileSync(['rev-parse', '--show-object-format', '--git-path', 'objects', '--git-path', 'shallow'], { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).split('\n'); + if (!format || !objectsPath || !shallowPath) throw new Error('git rev-parse gave no object format or paths'); + let text = ''; + try { + text = readFileSync(resolve(cwd, shallowPath), 'utf8'); + } catch (e) { + if ((e as NodeJS.ErrnoException).code !== 'ENOENT') throw new Error(`can't read its shallow file: ${(e as Error).message}`); + } + const ids = shallowIds(text, format); + if (ids === null) throw new Error(`its shallow file has a line that isn't exactly a full lowercase object id of this repo's format (${format}), which git may still read as a cut`); + const dir = mkdtempSync(join(tmpdir(), 'git-snapshot-')); + try { + gitExecFileSync(['init', '-q', '--bare', `--object-format=${format}`, dir], { env: clean(), stdio: ['ignore', 'pipe', 'pipe'] }); + if (ids.length) writeFileSync(join(dir, 'shallow'), `${ids.join('\n')}\n`); + } catch (e) { + rmSync(dir, { recursive: true, force: true }); + throw e; + } + const env: NodeJS.ProcessEnv = { ...clean(), GIT_DIR: dir, GIT_OBJECT_DIRECTORY: resolve(cwd, objectsPath) }; + const cut = new Set(ids); + return { + env, + cut, + cutInRange: (base, head) => { + if (!cut.size) return []; + const listed = gitExecFileSync(['rev-list', `${base}..${head}`], { env, encoding: 'utf8', maxBuffer: 256 * 1024 * 1024, stdio: ['ignore', 'pipe', 'pipe'] }); + return listed.split('\n').filter((c) => cut.has(c)); + }, + dispose: () => rmSync(dir, { recursive: true, force: true }), + }; +} diff --git a/src/plan-decisions.ts b/src/plan-decisions.ts index 221217661..47fb221f5 100644 --- a/src/plan-decisions.ts +++ b/src/plan-decisions.ts @@ -9,7 +9,7 @@ import { existsSync, readdirSync, readFileSync, realpathSync, statSync } from 'n import { basename, dirname, isAbsolute, join, posix, relative, sep } from 'node:path'; import { redactString } from './events/redact.js'; import type { EventPayload, StoredEvent } from './events/types.js'; -import { spawnSync } from './os/child-process.js'; +import { gitSpawnSync } from './os/git.js'; import { DECISION_DIRS, DECISION_FILES } from './review.js'; /** The whole section when nothing recorded bears on the plan. Never beside citations. */ @@ -128,8 +128,9 @@ function verifiedObjects(cwd: string, wanted: { oid: string; type: 'commit' | 't const out = new Map(); if (!wanted.length) return out; if (!wanted.every((w) => OBJECT_ID.test(w.oid))) return null; - // Replace refs (refs/replace) could stand another object in for the one asked for: never read through them. - const r = spawnSync('git', ['--no-replace-objects', 'cat-file', '--batch'], { cwd, input: `${wanted.map((w) => w.oid).join('\n')}\n`, ...GIT_LIMITS }); + // Replace refs (refs/replace) could stand another object in for the one asked for: never read through them. Through + // the git runner (#807), which also sets grafts and the commit-graph off. + const r = gitSpawnSync(['--no-replace-objects', 'cat-file', '--batch'], { cwd, input: `${wanted.map((w) => w.oid).join('\n')}\n`, ...GIT_LIMITS }); if (r.error || r.status !== 0) return null; const buf = r.stdout as Buffer; let at = 0; @@ -183,7 +184,7 @@ const REGULAR = new Set(['100644', '100755']); function committedRecords(checkout: string, rev: string, objects: string): Map | null { let base = rev; if (rev === 'HEAD') { - const git = (args: string[]) => spawnSync('git', ['--no-replace-objects', ...args], { cwd: checkout, encoding: 'utf8', ...GIT_LIMITS }); + const git = (args: string[]) => gitSpawnSync(['--no-replace-objects', ...args], { cwd: checkout, encoding: 'utf8', ...GIT_LIMITS }); const top = git(['rev-parse', '--show-cdup']); if (top.error || top.status !== 0 || top.stdout.trim() !== '') return null; const head = git(['rev-parse', '--verify', '--quiet', 'HEAD^{commit}']); diff --git a/src/plugins/fetch.ts b/src/plugins/fetch.ts index 5b99d1d7f..dbf6875a5 100644 --- a/src/plugins/fetch.ts +++ b/src/plugins/fetch.ts @@ -4,6 +4,7 @@ // DynamicUser) is the root side's job (scripts/setup/plugins.sh); this module is the part that is the same wherever it // runs, and it never executes a file it fetched. import { execFileSync } from '../os/child-process.js'; +import { gitExecFileSync } from '../os/git.js'; import { chmodSync, copyFileSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { dirname, join } from 'node:path'; @@ -43,7 +44,7 @@ export function gitSafety(allowFile: boolean, pin?: string): string[] { return ['-c', 'http.followRedirects=false', ...(pin ? ['-c', `http.curloptResolve=${pin}`] : []), '-c', 'core.hooksPath=/dev/null', '-c', 'protocol.allow=never', '-c', 'protocol.https.allow=always', ...(allowFile ? ['-c', 'protocol.file.allow=always'] : []), '-c', 'submodule.recurse=false', '-c', 'core.symlinks=true', '-c', 'advice.detachedHead=false']; } function git(cwd: string, home: string, args: string[], allowFile: boolean, pin?: string): string { - return execFileSync('git', [...gitSafety(allowFile, pin), ...args], { + return gitExecFileSync([...gitSafety(allowFile, pin), ...args], { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], diff --git a/src/push-check.ts b/src/push-check.ts index 9f9ef5372..3ceb14407 100644 --- a/src/push-check.ts +++ b/src/push-check.ts @@ -5,9 +5,9 @@ // leaves a SKILL.md that fails the skill lint. // The coordinator also runs pushProblems on each change before verifying it, // so a doomed change is caught before a long test run. -import { execFileSync, spawnSync } from './os/child-process.js'; import { blobsByName, rangeNames, sizeFiles } from './coordinator/change-files.js'; import { globToRegExp } from './guardrails/glob.js'; +import { gitExecFileSync, gitSnapshot, gitSpawnSync, type GitSnapshot } from './os/git.js'; import { shownPath } from './path-text.js'; import { lintSkill, SKILL_SECTIONS } from './skill-lint.js'; import { defaultBranchRefusal, type PushGuard } from './push-guard.js'; @@ -23,21 +23,82 @@ export const DEFAULT_PUSH_LIMITS: PushLimits = { max_file_mb: 10, max_changed_li const WORKFLOWS = '.github/workflows/'; const MB = 1024 * 1024; -// Replace refs off (#928, round 4): a `git replace` of a 3 MB blob with a small one got past the size limit (measured). -// #927 moves this onto its git runner, which sets the same and more. -const git = (cwd: string, args: string[], input?: string) => execFileSync('git', ['--no-replace-objects', ...args], { cwd, encoding: 'utf8', maxBuffer: 256 * MB, stdio: ['pipe', 'pipe', 'pipe'], ...(input !== undefined ? { input } : {}) }); +// Through the git runner (#807): replace refs and grafts off, the commit-graph off. #928's round 4 measured a +// `git replace` of a 3 MB blob with a small one getting past the size limit; the runner turns replace refs off. +const git = (cwd: string, args: string[], input?: string) => gitExecFileSync(args, { cwd, encoding: 'utf8', maxBuffer: 256 * MB, stdio: ['pipe', 'pipe', 'pipe'], ...(input !== undefined ? { input } : {}) }); const list = (paths: string[], n = 10) => `${paths.slice(0, n).join(', ')}${paths.length > n ? ` and ${paths.length - n} more` : ''}`; /** Changed paths in a problem's text, each as shownPath shows it (#683): `rev-list --objects` prints a name as it is. */ const listPaths = (paths: string[], n = 10) => list(paths.map(shownPath), n); -/** Every reason the range base..head may not be pushed; empty means it may. */ +/** The refusal for a range the push limits can't read whole (#807). */ +const cutRefusal = (why: string) => + `this repo's history is cut inside the range (${why}): the push limits can't read it whole, so nothing is pushed. A shallow clone pushes only changes above its cut; fetch the rest (git fetch --unshallow), or remove a .git/shallow file nobody meant to write`; + +/** base and head as commit ids, read in the repo (the snapshot has no refs). Throws as git does on a bad name. */ +const commitIds = (cwd: string, base: string, head: string): [string, string] => + [base, head].map((r) => git(cwd, ['rev-parse', '--verify', '--end-of-options', `${r}^{commit}`]).trim()) as [string, string]; + +/** + * Why base..head can't be read whole in `snap`, or null (#807). A shallow repo's `shallow` file lists commits whose + * parents git treats as missing, and any agent can write it into the shared repo: a commit listed inside the range cut + * the walks short (a refused file in an earlier commit read as 0 problems, measured). A cut outside the range (a + * depth-limited clone's base, the harness-eval replay's clone) leaves the range whole and is fine. + */ +function cutIn(snap: GitSnapshot, base: string, head: string): string | null { + const inRange = snap.cutInRange(base, head); + return inRange.length ? cutRefusal(`a shallow boundary at ${inRange.slice(0, 3).map((c) => c.slice(0, 12)).join(', ')}`) : null; +} + +/** A gitSnapshot of `cwd`, or the refusal when one can't be taken (a shallow file it can't read the way git might). */ +function snapshotOf(cwd: string): GitSnapshot | string { + try { + return gitSnapshot(cwd); + } catch (e) { + return cutRefusal(String((e as Error).message).split('\n')[0]!); + } +} + +/** + * Why `cwd`'s history is cut inside base..head right now, or null: the check checkedPush makes once more right before + * the push. Fails closed. + */ +export function shallowCut(cwd: string, base: string, head: string): string | null { + const snap = snapshotOf(cwd); + if (typeof snap === 'string') return snap; + try { + const [b, h] = commitIds(cwd, base, head); + return cutIn(snap, b, h); + } finally { + snap.dispose(); + } +} + +/** + * Every reason the range base..head may not be pushed; empty means it may. Every walk of the range reads one + * gitSnapshot, the one whose shallow file was checked, so a cut written or removed while they run changes nothing + * (#927: toggling it every ~3 ms beat the old before-and-after checks in 23 of 150 runs, measured). + */ export function pushProblems(cwd: string, base: string, head: string, limits: PushLimits): string[] { + const snap = snapshotOf(cwd); + if (typeof snap === 'string') return [snap]; + try { + return judge(cwd, snap, ...commitIds(cwd, base, head), limits); + } finally { + snap.dispose(); + } +} + +function judge(cwd: string, snap: GitSnapshot, base: string, head: string, limits: PushLimits): string[] { + const cut = cutIn(snap, base, head); + if (cut) return [cut]; const range = `${base}..${head}`; + // The range's walks, through the snapshot. + const walk = (args: string[], input?: string) => gitExecFileSync(args, { env: snap.env, encoding: 'utf8', maxBuffer: 256 * MB, stdio: ['pipe', 'pipe', 'pipe'], ...(input !== undefined ? { input } : {}) }); const problems: string[] = []; // Paths touched by any commit in the range, not just the net diff, each as itself (#724): a name git would quote - // (a non-ASCII byte, a newline, a quote) is matched by its own bytes, not its quoted text. - const touched = rangeNames(cwd, range); + // (a non-ASCII byte, a newline, a quote) is matched by its own bytes, not its quoted text. Through the snapshot. + const touched = rangeNames(cwd, range, snap.env); const workflows = touched.filter((p) => p.startsWith(WORKFLOWS)); if (workflows.length) problems.push(`changes CI workflows (${listPaths(workflows)}): the harness's GitHub credential has no Workflows permission, so GitHub refuses this push; a human makes workflow changes`); @@ -53,7 +114,7 @@ export function pushProblems(cwd: string, base: string, head: string, limits: Pu // Every blob new in the range, with its path: a file added and then deleted is still pushed. const paths = new Map(); - for (const l of git(cwd, ['rev-list', '--objects', range]).split('\n')) { + for (const l of walk(['rev-list', '--objects', range]).split('\n')) { const [sha, ...p] = l.split(' '); if (sha && p.length) paths.set(sha, p.join(' ')); } @@ -61,7 +122,7 @@ export function pushProblems(cwd: string, base: string, head: string, limits: Pu if (objects.length) { const limit = limits.max_file_mb * MB; const big: string[] = []; - for (const l of git(cwd, ['cat-file', '--batch-check=%(objecttype) %(objectname) %(objectsize)'], `${objects.join('\n')}\n`).split('\n')) { + for (const l of walk(['cat-file', '--batch-check=%(objecttype) %(objectname) %(objectsize)'], `${objects.join('\n')}\n`).split('\n')) { const [type, sha, size] = l.split(' '); if (type === 'blob' && Number(size) > limit) big.push(`${shownPath(paths.get(sha!) ?? sha!)} (${(Number(size) / MB).toFixed(1)} MB)`); } @@ -81,7 +142,7 @@ export function pushProblems(cwd: string, base: string, head: string, limits: Pu // A skill is prose: a bigger SKILL.md is refused, never read whole. const SKILL_MAX_BYTES = MB; const skills = touched.filter((t) => t === 'SKILL.md' || t.endsWith('/SKILL.md')); - const atHead = blobsByName(cwd, head, skills, SKILL_MAX_BYTES); + const atHead = blobsByName(cwd, head, skills, SKILL_MAX_BYTES, snap.env); for (const p of skills) { for (const file of atHead.get(p) ?? []) { const lint = 'text' in file ? lintSkill(file.text) : [`can't be read at the head: ${file.unread}`]; @@ -107,8 +168,11 @@ export function checkedPush(o: { cwd: string; remote: string; base: string; head if (main) return { ok: false, refused: [main] }; const refused = pushProblems(o.cwd, o.base, o.head, o.limits); if (refused.length) return { ok: false, refused }; + // Once more right before the push, in this process: a cut written since the limits ran (#807). + const late = shallowCut(o.cwd, o.base, o.head); + if (late) return { ok: false, refused: [late] }; const lease = o.expect ? [`--force-with-lease=${o.ref}:${o.expect}`] : []; - const push = spawnSync('git', ['push', ...lease, o.remote, `${o.force ? '+' : ''}${o.head}:${o.ref}`], { cwd: o.cwd, encoding: 'utf8' }); + const push = gitSpawnSync(['push',...lease, o.remote, `${o.force ? '+' : ''}${o.head}:${o.ref}`], { cwd: o.cwd, encoding: 'utf8' }); if (push.status !== 0) return { ok: false, error: (push.stderr || '').trim() }; return { ok: true }; } diff --git a/src/read-denials.ts b/src/read-denials.ts index b3570b23c..707299445 100644 --- a/src/read-denials.ts +++ b/src/read-denials.ts @@ -14,7 +14,7 @@ import { BRAND } from './brand.js'; import { ConfigInvalid, loadConfig, loadConfigAt, readRegular } from './config/load.js'; import { TaskFile } from './contract.js'; import { projectStateDir } from './guardrails/context.js'; -import { spawnSync } from './os/child-process.js'; +import { gitSpawnSync } from './os/git.js'; /** Directory entries listed, at most, to expand one pattern that has an exception; past it the pattern is denied whole. */ export const EXPAND_LIMIT = 5000; @@ -375,7 +375,7 @@ const realOrNull = (p: string) => { function git(common: string, args: string[], ok: number[] = [0], benign?: RegExp): { out: string } | { error: string } { const env: NodeJS.ProcessEnv = {}; for (const [k, v] of Object.entries(process.env)) if (!k.startsWith('GIT_') && k !== 'LANGUAGE') env[k] = v; - const r = spawnSync('git', ['-c', 'core.quotePath=true', '--git-dir', common, ...args], { + const r = gitSpawnSync(['-c', 'core.quotePath=true', '--git-dir', common, ...args], { env: { ...env, LC_ALL: 'C', GIT_TERMINAL_PROMPT: '0' }, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], diff --git a/src/review.ts b/src/review.ts index e09017060..5a4c4cae0 100644 --- a/src/review.ts +++ b/src/review.ts @@ -1,7 +1,7 @@ // Risk-based review levels, computed by code from what a change touches. // The highest matching level wins; a failed or low-confidence evaluator // verdict bumps it up one; an agent can raise its own level, never lower it. -import { spawnSync } from './os/child-process.js'; +import { gitSpawnSync } from './os/git.js'; import { posix } from 'node:path'; import type { ReviewConfig } from './config/schema.js'; import { globToRegExp } from './guardrails/glob.js'; @@ -386,6 +386,9 @@ export const SELF_MODIFICATION_PARTS: { what: string; globs: string[] }[] = oneP { what: 'the merge policy', globs: ['src/merge-policy*', 'src/coordinator/merge-flow*', 'src/pr-rules*', 'src/vacuity.ts', 'src/vacuity-read.ts'] }, { what: 'the main watch', globs: ['src/main-watch*', 'src/github/main-watch-api*'] }, { what: 'the push guard', globs: ['src/push-guard*', 'src/push-check*'] }, + // The one way the engine and its scripts run git, with replace refs and grafts off (#807): every reader above reads + // through it. + { what: 'the git runner', globs: ['src/os/git.ts', 'scripts/lib/git.cjs'] }, { what: 'the updater', globs: ['src/updates*', `scripts/machine/${BRAND.cli}-update*`] }, // src/path-text.ts holds the odd-name tests computeLevel raises on (moved out of src/review.ts, #683), so it stays locked with it. // src/confusables*: the look-alike check and its Unicode table (#763). @@ -477,6 +480,8 @@ export const SELF_MODIFICATION_PARTS: { what: string; globs: string[] }[] = oneP 'test/engine-guard-locks.test.ts', // The test that keeps each lock glob that matches no file yet paired with its lock (#885). 'test/not-yet-files-locks.test.ts', + // The replace-ref and graft routes, and the scan that every git call goes through the git runner (#807). + 'test/git-runner.test.ts', // The test that keeps the engine repo's merge process (docs/process.md) out of the planners' decisions index. 'test/plan-decisions.test.ts', ], @@ -639,7 +644,7 @@ export interface LevelResult { export function loadMoneyPaths(repo: string, src: ReviewConfig['money_path_source'], rev: string): RegExp[] { if (!src) return []; const file = posix.normalize(src.file).replace(/^\.\//, ''); - const shown = spawnSync('git', ['show', `${rev}:${file}`], { cwd: repo, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], maxBuffer: 16 * 1024 * 1024 }); + const shown = gitSpawnSync(['show',`${rev}:${file}`], { cwd: repo, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], maxBuffer: 16 * 1024 * 1024 }); if (shown.status !== 0) throw new Error(`money_path_source: ${file} isn't readable at ${rev}: ${(shown.stderr || shown.error?.message || '').trim().split('\n')[0]}`); let text = shown.stdout; if (src.pattern) { diff --git a/src/scan/secrets.ts b/src/scan/secrets.ts index d28b41d63..f418759f2 100644 --- a/src/scan/secrets.ts +++ b/src/scan/secrets.ts @@ -5,6 +5,7 @@ import { createHash } from 'node:crypto'; import { existsSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, readlinkSync, rmSync, statSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { delimiter, dirname, join, resolve as resolvePath } from 'node:path'; +import { gitEnv, gitExecFileSync, gitSnapshot, gitSpawnSync, type GitSnapshot } from '../os/git.js'; import { presentNames, rawChanges, untrackedNames } from '../coordinator/change-files.js'; import { findTool } from '../tool-path.js'; @@ -179,7 +180,7 @@ const text = (b: Buffer) => Buffer.from(b.toString('latin1').replace(/\0/g, ''), /** git in the checkout, for an object or a revision (never a list of changes: those come from the change reader). */ function gitOut(top: string, args: string[], input?: string): Buffer { - const r = spawnSync('git', ['--no-replace-objects', ...args], { cwd: top, encoding: 'buffer', maxBuffer: 1024 * 1024 * 1024, timeout: 60_000, ...(input !== undefined ? { input: Buffer.from(input, 'utf8') } : {}) }); + const r = gitSpawnSync(['--no-replace-objects', ...args], { cwd: top, encoding: 'buffer', maxBuffer: 1024 * 1024 * 1024, timeout: 60_000, ...(input !== undefined ? { input: Buffer.from(input, 'utf8') } : {}) }); if (r.error) throw r.error; if (r.status !== 0) throw new Error(`git ${args[0]} failed: ${r.stderr.toString('utf8').trim().split('\n').pop() ?? `exit ${r.status}`}`); return r.stdout; @@ -262,7 +263,7 @@ const emptyTree = (top: string) => gitOut(top, ['hash-object', '-t', 'tree', '-- /** HEAD's commit id, or null on a branch with no commit yet. */ function headId(top: string): string | null { - const r = spawnSync('git', ['--no-replace-objects', 'rev-parse', '--verify', '-q', 'HEAD^{commit}'], { cwd: top, encoding: 'utf8', timeout: 60_000 }); + const r = gitSpawnSync(['--no-replace-objects', 'rev-parse', '--verify', '-q', 'HEAD^{commit}'], { cwd: top, encoding: 'utf8', timeout: 60_000 }); if (r.error) throw r.error; return r.status === 0 ? r.stdout.trim() : null; } @@ -361,7 +362,7 @@ export function checkReadPrograms(top: string, scope: CommitScanScope, depth = 0 } // WHY: git's own reading of the monitor setting: a boolean is git's own daemon; anything else is a program's path. if (config.has('core.fsmonitor')) { - const r = spawnSync('git', ['config', '--type=bool', '--get', 'core.fsmonitor'], { cwd: top, encoding: 'utf8', timeout: 60_000 }); + const r = gitSpawnSync(['config', '--type=bool', '--get', 'core.fsmonitor'], { cwd: top, encoding: 'utf8', timeout: 60_000 }); if (r.error || r.status !== 0) throw new Error('the checkout\'s config names a file-system monitor program, which git runs while it reads changes'); } if (scope.logReads && signer) throw new Error(`the checkout's config names a program git runs to check signatures (${signer}), and the line shows commits before it commits`); @@ -517,7 +518,33 @@ function headContent(top: string): (() => Compared[])[] { */ export function scanRange(cwd: string, range: string, binary = findTool('gitleaks')): Promise { if (!binary) return Promise.resolve({ status: 'unavailable', error: 'gitleaks not found on PATH' }); - return scanAsync(binary, (report) => ['git', '--redact', '--no-banner', '--log-opts', range, '--report-format', 'json', '--report-path', report, '--exit-code', String(LEAKS_EXIT), '.'], cwd); + // gitleaks walks the range with git: through one gitSnapshot whose shallow file was checked, so a cut written or + // removed while it runs changes nothing, and a cut inside the range is never "clean" (#927). Commit ids only there. + let snap: GitSnapshot; + let ids: string; + try { + const m = /^([^.].*?)\.\.([^.].*)$/.exec(range); + if (!m) throw new Error(`${range} is not a base..head range`); + const id = (r: string) => gitExecFileSync(['rev-parse', '--verify', '--end-of-options', `${r}^{commit}`], { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); + const [base, head] = [id(m[1]!), id(m[2]!)]; + snap = gitSnapshot(cwd); + const cut = (() => { + try { + return snap.cutInRange(base, head); + } catch (e) { + snap.dispose(); + throw e; + } + })(); + if (cut.length) { + snap.dispose(); + return Promise.resolve({ status: 'unavailable', error: `the repo's history is cut inside ${range} (a shallow boundary at ${cut[0]!.slice(0, 12)}): it can't be scanned whole` }); + } + ids = `${base}..${head}`; + } catch (e) { + return Promise.resolve({ status: 'unavailable', error: `can't read ${range} whole: ${String((e as Error).message).split('\n')[0]}` }); + } + return scanAsync(binary, (report) => ['git', '--redact', '--no-banner', '--log-opts', ids, '--report-format', 'json', '--report-path', report, '--exit-code', String(LEAKS_EXIT), '.'], cwd, snap.env).finally(() => snap.dispose()); } /** Scan one file without blocking (a dashboard upload). Findings name the rule, never the secret (--redact). */ @@ -526,11 +553,13 @@ export function scanFile(path: string, binary = findTool('gitleaks')): Promise ['dir', path, '--redact', '--no-banner', '--max-decode-depth', '2', '--report-format', 'json', '--report-path', report, '--exit-code', String(LEAKS_EXIT)], undefined); } -function scanAsync(binary: string, args: (report: string) => string[], cwd: string | undefined): Promise { +function scanAsync(binary: string, args: (report: string) => string[], cwd: string | undefined, env?: NodeJS.ProcessEnv): Promise { const dir = mkdtempSync(join(tmpdir(), 'scan-')); const report = join(dir, 'report.json'); return new Promise((resolve) => { - const child = spawn(binary, args(report), { ...(cwd ? { cwd } : {}), stdio: ['ignore', 'ignore', 'pipe'] }); + // gitleaks runs git itself (`git log -p`): with the git runner's settings, so a replace ref or a graft can't swap + // the commits it reads (#807). + const child = spawn(binary, args(report), { ...(cwd ? { cwd } : {}), stdio: ['ignore', 'ignore', 'pipe'], env: gitEnv(env) }); let err = ''; child.stderr.on('data', (d: Buffer) => (err = (err + d.toString()).slice(-2000))); const done = (r: ScanResult) => { diff --git a/src/server-auto-config.ts b/src/server-auto-config.ts index 77924c51b..bf2114802 100644 --- a/src/server-auto-config.ts +++ b/src/server-auto-config.ts @@ -8,7 +8,7 @@ // doesn't stop the coordinator; // - there is no terminal: open questions are recorded, and answered with `instance auto-config --answer`. // Its state is the coordinator's own (auto-config.json in the instance's state dir); each run prints one JSON line. -import { spawnSync } from './os/child-process.js'; +import { gitSpawnSync } from './os/git.js'; import { existsSync, mkdirSync, readFileSync } from 'node:fs'; import { dirname, join } from 'node:path'; import { parseDocument } from 'yaml'; @@ -143,7 +143,7 @@ export async function serverAutoConfig(name: string, o: { answers?: Record { - const p = spawnSync('git', args, { cwd, encoding: 'utf8', env: { ...process.env, ...identity, GIT_TERMINAL_PROMPT: '0', [`${BRAND.envPrefix}_INSTANCE`]: name }, timeout: 300_000 }); + const p = gitSpawnSync(args, { cwd, encoding: 'utf8', env: { ...process.env, ...identity, GIT_TERMINAL_PROMPT: '0', [`${BRAND.envPrefix}_INSTANCE`]: name }, timeout: 300_000 }); return { status: p.error ? null : p.status, stdout: p.stdout ?? '', stderr: p.error ? p.error.message : (p.stderr ?? '') }; }, gh: (args) => gh(args), diff --git a/src/setup-actions.ts b/src/setup-actions.ts index 278fd16fc..9b779e262 100644 --- a/src/setup-actions.ts +++ b/src/setup-actions.ts @@ -1,7 +1,8 @@ // WHY: the first-run page fixes some steps itself (Fix kind 'action'). Each action shows its plan before anything // happens and runs the CLI's own code for the step; the dashboard runs it for owners only, confirmed and audited. import { join } from 'node:path'; -import { execFileSync, spawnSync } from './os/child-process.js'; +import { spawnSync } from './os/child-process.js'; +import { commandSpawnSync, gitExecFileSync } from './os/git.js'; import { BRAND } from './brand.js'; import type { Config } from './config/load.js'; import { refreshFingerprints, type RefreshResult } from './guardrails/context.js'; @@ -54,7 +55,7 @@ export function systemActionDeps(): ActionDeps { } const gitCommonHooks = (root: string) => { - const common = execFileSync('git', ['rev-parse', '--git-common-dir'], { cwd: root, encoding: 'utf8' }).trim(); + const common = gitExecFileSync(['rev-parse', '--git-common-dir'], { cwd: root, encoding: 'utf8' }).trim(); return join(common.startsWith('/') ? common : join(root, common), 'hooks'); }; @@ -147,7 +148,8 @@ export const ACTIONS: Record = { }), run: async (c) => { for (const cmd of defaultBranchCommands(c)) { - const r = spawnSync(cmd[0]!, cmd.slice(1), { cwd: c.root, encoding: 'utf8', timeout: 120_000, env: withToolPath() }); + // The git commands below (defaultBranchCommands) through the git runner (#807). + const r = commandSpawnSync(cmd[0]!, cmd.slice(1), { cwd: c.root, encoding: 'utf8', timeout: 120_000, env: withToolPath() }); if (r.status !== 0) return { ok: false, detail: `${cmd.join(' ')} failed: ${(r.stderr || r.error?.message || '').trim().split('\n')[0]}` }; } return { ok: true, detail: `fetched origin/${c.cfg.project.project.default_branch}; origin/HEAD set` }; diff --git a/src/setup-steps.ts b/src/setup-steps.ts index 6dbe8db7e..5330767e8 100644 --- a/src/setup-steps.ts +++ b/src/setup-steps.ts @@ -4,7 +4,8 @@ // and it checks them with these detectors), `doctor` (one line per step) and the dashboard's first-run page // (GET /api/first-run). A detector only looks: it runs read-only commands (`claude auth status`, `gh auth status`, // `git rev-parse`, the service manager's status) and reads files; it never changes anything. -import { spawnSync, startSetupWorker } from './os/child-process.js'; +import { startSetupWorker } from './os/child-process.js'; +import { commandSpawnSync } from './os/git.js'; import { existsSync, readFileSync } from 'node:fs'; import { join } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -95,7 +96,8 @@ export function systemProbe(o: { managerEnv?: ManagerEnv } = {}): SetupProbe { return { which: (cmd) => findTool(cmd), run: (cmd, args, o = {}) => { - const r = spawnSync(cmd, args, { encoding: 'utf8', timeout: 30_000, env: probeEnv(o), ...(o.cwd ? { cwd: o.cwd } : {}) }); + // A caller's git (the command named git) through the git runner (#807). + const r = commandSpawnSync(cmd, args, { encoding: 'utf8', timeout: 30_000, env: probeEnv(o), ...(o.cwd ? { cwd: o.cwd } : {}) }); return { status: r.error ? null : r.status, stdout: r.stdout ?? '', stderr: r.error ? r.error.message : (r.stderr ?? '') }; }, serviceRunning, diff --git a/src/skill-lint.ts b/src/skill-lint.ts index 9e2b4a328..1ee3c9f53 100644 --- a/src/skill-lint.ts +++ b/src/skill-lint.ts @@ -1,6 +1,6 @@ // Skill lint: every SKILL.md has the six sections a skill needs to be used and checked, each with something in it. // Validate (how to check the result) matters most: a skill whose result can't be checked is a guess. -import { execFileSync } from './os/child-process.js'; +import { gitExecFileSync } from './os/git.js'; import { readFileSync } from 'node:fs'; import { join } from 'node:path'; @@ -38,7 +38,7 @@ export function lintSkill(text: string): string[] { /** Every tracked SKILL.md in a git checkout (forward-slash paths, relative to `root`). */ export function trackedSkills(root: string): string[] { - return execFileSync('git', ['ls-files', '-z', '--', '*SKILL.md'], { cwd: root, encoding: 'utf8' }) + return gitExecFileSync(['ls-files', '-z', '--', '*SKILL.md'], { cwd: root, encoding: 'utf8' }) .split('\0') .filter((p) => p === 'SKILL.md' || p.endsWith('/SKILL.md')); } diff --git a/src/starter-ci.ts b/src/starter-ci.ts index 0475a97d4..2932b9b82 100644 --- a/src/starter-ci.ts +++ b/src/starter-ci.ts @@ -7,7 +7,7 @@ // design (src/setup/app-manifest.ts, scripts/setup/app-permissions.mjs); this only drafts text and builds a link. // Once the workflow ran on the default branch, its check is offered for required_checks as an ordinary config // change the person reviews and proposes (src/config-change.ts), never written here. -import { spawnSync } from './os/child-process.js'; +import { commandSpawnSync } from './os/git.js'; import { BRAND } from './brand.js'; import { detectProject, NO_TESTS, type Detection } from './auto-config.js'; import { GITHUB_WEB } from './github-hosts.js'; @@ -166,7 +166,8 @@ export interface StarterCiView extends StarterCi { type Run = (cmd: string, args: string[], cwd: string) => { status: number | null; stdout: string; stderr: string }; const realRun: Run = (cmd, args, cwd) => { - const r = spawnSync(cmd, args, { cwd, encoding: 'utf8', timeout: 60_000 }); + // A caller's git (the command named git) through the git runner (#807). + const r = commandSpawnSync(cmd, args, { cwd, encoding: 'utf8', timeout: 60_000 }); return { status: r.error ? null : r.status, stdout: r.stdout ?? '', stderr: r.stderr ?? '' }; }; diff --git a/src/testrun/cache.ts b/src/testrun/cache.ts index f695213c0..1ee17062d 100644 --- a/src/testrun/cache.ts +++ b/src/testrun/cache.ts @@ -1,7 +1,7 @@ // Cache keys: the sha256 of the files that pin what a cache holds (the lockfile), read at the run's commit from the // coordinator's clone. A changed lockfile is a new key and so a new, empty cache, never a stale one; a key file the // commit doesn't have is part of the key too ("missing"), so adding one later changes it. -import { execFileSync } from '../os/child-process.js'; +import { gitExecFileSync } from '../os/git.js'; import { createHash } from 'node:crypto'; import type { TestRunCache } from './config.js'; @@ -10,7 +10,7 @@ export function cacheKey(repo: string, sha: string, files: readonly string[]): s for (const f of files) { h.update(`${f}\0`); try { - h.update(execFileSync('git', ['-C', repo, 'show', `${sha}:${f}`], { maxBuffer: 256 * 1024 * 1024, stdio: ['ignore', 'pipe', 'ignore'] })); + h.update(gitExecFileSync(['-C', repo, 'show', `${sha}:${f}`], { maxBuffer: 256 * 1024 * 1024, stdio: ['ignore', 'pipe', 'ignore'] })); } catch { h.update('\0missing\0'); } diff --git a/src/testrun/host.ts b/src/testrun/host.ts index 92b05362c..62ee3bf34 100644 --- a/src/testrun/host.ts +++ b/src/testrun/host.ts @@ -13,6 +13,7 @@ import type { Readable } from 'node:stream'; import { BRAND } from '../brand.js'; import { killTree, shellCommand, spawnDetached } from '../os/index.js'; import { ownEnv } from '../os/own-env.js'; +import { gitArgs, gitEnv } from '../os/git.js'; import { SPARSE_MAX, sparseDirProblem } from '../checkout.js'; import { MAX_CONNECTIONS, PINNED_IMAGE } from './config.js'; import { HELPER_PROTOCOL } from './helper.js'; @@ -543,7 +544,8 @@ export function localHost(base: string, o: { docker?: string; path?: string } = const w = await run('sh', ['-c', 'cat > "$1"', '_', file], { stdin: bundle, timeoutMs: 600_000 }); if (w.code !== 0) throw fail('create', `receiving run ${id}'s bundle`, w); const [bash, args] = shellCommand(cloneScript()); - const env = { PATH: o.path ?? ownEnv().get('PATH') ?? '/usr/bin:/bin', HOME: join(dir, 'home'), LANG: 'C.UTF-8' }; + // The clone script runs git: with the git runner's settings, as every engine git does (#807). + const env = gitEnv({ PATH: o.path ?? ownEnv().get('PATH') ?? '/usr/bin:/bin', HOME: join(dir, 'home'), LANG: 'C.UTF-8' }); mkdirSync(join(base, 'store'), { recursive: true, mode: 0o700 }); const r = await run(bash, [...args, '_', file, join(dir, 'work'), refs.branch, refs.head, refs.base, refs.headSha, refs.baseSha, join(base, 'store'), ...refs.exclude], { env, cwd: dir, timeoutMs: 600_000 }); if (r.code !== 0) throw fail('create', `creating run ${id}'s workspace`, r); @@ -551,7 +553,7 @@ export function localHost(base: string, o: { docker?: string; path?: string } = }, async storeTips() { if (!existsSync(join(base, 'store', 'HEAD'))) return []; - const r = await run('git', [`--git-dir=${join(base, 'store')}`, 'for-each-ref', '--format=%(objectname)', 'refs/keep/'], { env: { PATH: o.path ?? ownEnv().get('PATH') ?? '/usr/bin:/bin', GIT_CONFIG_NOSYSTEM: '1', GIT_CONFIG_GLOBAL: '/dev/null' }, timeoutMs: 60_000 }); + const r = await run('git', gitArgs([`--git-dir=${join(base, 'store')}`, 'for-each-ref', '--format=%(objectname)', 'refs/keep/']), { env: gitEnv({ PATH: o.path ?? ownEnv().get('PATH') ?? '/usr/bin:/bin', GIT_CONFIG_NOSYSTEM: '1', GIT_CONFIG_GLOBAL: '/dev/null' }), timeoutMs: 60_000 }); if (r.code !== 0) throw fail('store-tips', 'reading the object store', r); return parseTips(r.stdout); }, diff --git a/src/testrun/measure-cli.ts b/src/testrun/measure-cli.ts index b71a63503..254de94ca 100644 --- a/src/testrun/measure-cli.ts +++ b/src/testrun/measure-cli.ts @@ -3,7 +3,7 @@ // INVARIANT: the host is always the sandbox host (no flag picks another; only a test passes MeasureIo.host), and no // checks dependency, token or App key is loaded here: a measure can't post a check. -import { execFileSync } from '../os/child-process.js'; +import { gitExecFileSync } from '../os/git.js'; import { join } from 'node:path'; import { BRAND } from '../brand.js'; import { EventLog } from '../events/log.js'; @@ -68,7 +68,7 @@ export async function measureCommand(instance: string | undefined, sub: string | const given = args.sha ?? defaultBranch(i.repo.path)?.sha ?? ''; plan = planMeasure(config, { sha: given, tiers: args.tiers, sets: args.sets }); try { - plan.sha = execFileSync('git', ['-C', i.repo.path, 'rev-parse', '--verify', '-q', `${plan.sha}^{commit}`], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] }).trim(); + plan.sha = gitExecFileSync(['-C', i.repo.path, 'rev-parse', '--verify', '-q', `${plan.sha}^{commit}`], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] }).trim(); } catch { throw new MeasureRefused(`${plan.sha} is not a commit in ${i.repo.path}; nothing was started`); } diff --git a/src/testrun/predates.ts b/src/testrun/predates.ts index 980b56190..7d014551e 100644 --- a/src/testrun/predates.ts +++ b/src/testrun/predates.ts @@ -7,7 +7,7 @@ // Only then is anything here called a predates; an up-to-date head that lacks a script deleted it itself. // Every git question is asked of the coordinator's clone (the head and the default branch are fetched there), and // any that can't be answered counts as "no": this never turns a run into a predates on a guess. -import { spawnSync } from '../os/child-process.js'; +import { gitSpawnSync } from '../os/git.js'; import { parse } from 'yaml'; import { BRAND } from '../brand.js'; import { defaultBranch } from './workspace.js'; @@ -25,7 +25,7 @@ export interface Predates { } function git(repo: string, ...args: string[]): { ok: boolean; out: string } { - const r = spawnSync('git', ['-C', repo, ...args], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], timeout: 60_000, maxBuffer: 16 * 1024 * 1024 }); + const r = gitSpawnSync(['-C', repo, ...args], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], timeout: 60_000, maxBuffer: 16 * 1024 * 1024 }); return { ok: r.status === 0, out: r.stdout ?? '' }; } @@ -71,7 +71,7 @@ export function configBehind(repo: string, sha: string): string | null { if (!base) return null; const c = configCommit(repo, base.sha); if (!c) return null; - const r = spawnSync('git', ['-C', repo, 'merge-base', '--is-ancestor', c, sha], { stdio: 'ignore', timeout: 60_000 }); + const r = gitSpawnSync(['-C', repo, 'merge-base', '--is-ancestor', c, sha], { stdio: 'ignore', timeout: 60_000 }); // 1: not an ancestor. Anything else (0, or an error) is no evidence of an old head. return r.status === 1 ? c : null; } diff --git a/src/testrun/select.ts b/src/testrun/select.ts index dee34a78d..af449666b 100644 --- a/src/testrun/select.ts +++ b/src/testrun/select.ts @@ -7,9 +7,9 @@ // // The graph is read from the coordinator's clone at the run's commit (git ls-tree, git cat-file), never from the // workspace: the sandboxed test user owns that. -import { spawnSync } from '../os/child-process.js'; import { builtinModules } from 'node:module'; import { BRAND } from '../brand.js'; +import { gitSpawnSync } from '../os/git.js'; import { buildGraph, CODE_EXT, selectTests, type Plan } from './affected.js'; import type { TestRunConfig } from './config.js'; @@ -52,7 +52,7 @@ function packageCheck(pkgJson: string | null): (spec: string) => boolean { }; } -const git = (repo: string, args: string[], input?: string) => spawnSync('git', ['-C', repo, ...args], { input, maxBuffer: 512 * 1024 * 1024, timeout: 120_000 }); +const git = (repo: string, args: string[], input?: string) => gitSpawnSync(['-C', repo, ...args], { input, maxBuffer: 512 * 1024 * 1024, timeout: 120_000 }); /** Each file's contents at `sha`, through one git cat-file --batch. A file over 1 MB reads as empty (built output). */ export function readAt(repo: string, sha: string, files: readonly string[]): Map { diff --git a/src/testrun/workspace.ts b/src/testrun/workspace.ts index 009bac4e6..1919f58e1 100644 --- a/src/testrun/workspace.ts +++ b/src/testrun/workspace.ts @@ -13,13 +13,13 @@ // // Directories the project leaves out (tests.yaml checkout.sparse_exclude) are left out of the workspace too (sparse // checkout), unless a tier of the run sets full_checkout. -import { execFileSync, spawn } from '../os/child-process.js'; import { Readable } from 'node:stream'; import { BRAND } from '../brand.js'; +import { gitExecFileSync, gitSpawn } from '../os/git.js'; import { safeProjectEnv } from '../project-env.js'; import type { RunDatabase, RunHost } from './host.js'; -const git = (repo: string, ...a: string[]) => execFileSync('git', ['-C', repo, ...a], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], timeout: 120_000 }).trim(); +const git = (repo: string, ...a: string[]) => gitExecFileSync(['-C', repo, ...a], { encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], timeout: 120_000 }).trim(); /** The default branch's name and commit in the coordinator's clone: origin/HEAD, else origin/main, else main. */ export function defaultBranch(repo: string): { name: string; sha: string } | null { @@ -44,7 +44,7 @@ export function defaultBranch(repo: string): { name: string; sha: string } | nul export function sharedTips(repo: string, tips: readonly string[]): string[] { const wanted = [...new Set(tips.filter((t) => /^[0-9a-f]{40}(?:[0-9a-f]{24})?$/.test(t)))]; if (!wanted.length) return []; - const out = execFileSync('git', ['-C', repo, 'cat-file', '--batch-check=%(objectname) %(objecttype)'], { input: wanted.map((t) => `${t}\n`).join(''), encoding: 'utf8', stdio: ['pipe', 'pipe', 'pipe'], timeout: 120_000 }); + const out = gitExecFileSync(['-C', repo, 'cat-file', '--batch-check=%(objectname) %(objecttype)'], { input: wanted.map((t) => `${t}\n`).join(''), encoding: 'utf8', stdio: ['pipe', 'pipe', 'pipe'], timeout: 120_000 }); return out .split('\n') .map((l) => l.split(' ')) @@ -83,7 +83,7 @@ export function treeBundle(repo: string, sha: string, id: string, have: readonly const refs = [...(lacks(repo, full, have) ? [[head, full] as const] : []), ...(lacks(repo, base.sha, have) ? [[baseRef, base.sha] as const] : [])]; if (!refs.length) return { stream: Readable.from([]), branch: base.name, headSha: full, baseSha: base.sha, refs: 0, done: Promise.resolve() }; for (const [r, c] of refs) git(repo, 'update-ref', r, c); - const child = spawn('git', ['-C', repo, 'bundle', 'create', '--quiet', '-', ...refs.map(([r]) => r), ...(have.length ? ['--not', ...have] : [])], { stdio: ['ignore', 'pipe', 'pipe'] }); + const child = gitSpawn(['-C', repo, 'bundle', 'create', '--quiet', '-', ...refs.map(([r]) => r), ...(have.length ? ['--not', ...have] : [])], { stdio: ['ignore', 'pipe', 'pipe'] }); let err = ''; child.stderr.on('data', (d: Buffer) => (err += d.toString())); const cleanup = () => { diff --git a/src/tune.ts b/src/tune.ts index c61854d54..ed690d1b6 100644 --- a/src/tune.ts +++ b/src/tune.ts @@ -1,7 +1,7 @@ // `tune`: profile a project's test suite on this machine (a fresh worktree on main, set up as the agent user; the // suite at 1, 2, 4 and all cores' workers; time and peak worker memory) and propose tests.yaml's `cores.max` and // `{cores}` as a PR, under the usual push limits. Nothing changes until it is reviewed and merged. -import { execFileSync, spawnSync } from './os/child-process.js'; +import { spawnSync } from './os/child-process.js'; import { existsSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { freemem } from 'node:os'; import { join } from 'node:path'; @@ -12,6 +12,7 @@ import type { TestsConfig } from './config/schema.js'; import { CORES_PLACEHOLDER, envWithCores, withCores } from './cores.js'; import { cpuCount, peakMemoryMb, projectCommand, withPeakMemory, writeGroupOnly, type ProjectRunAs } from './os/index.js'; import { machineStats } from './os/stats.js'; +import { gitExecFileSync } from './os/git.js'; import { checkedPush, type PushLimits } from './push-check.js'; import { recommendWorkers, WORKER_VAR, type ProfileRun, type TuneProfile } from './tuning.js'; import { createWorktree, removeWorktree, type WorktreeOptions } from './worktrees.js'; @@ -126,7 +127,7 @@ export async function tune(o: TuneOptions): Promise { const after = applyTuning(before, rec.workers, setting); if (after === before) return { profile, recommendation: rec, changed: false }; writeFileSync(file, after); - const git = (...a: string[]) => execFileSync('git', a, { cwd: path, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); + const git = (...a: string[]) => gitExecFileSync(a, { cwd: path, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); git('add', '--', `${BRAND.configDir}/tests.yaml`); git('-c', `user.name=${o.identity.name}`, '-c', `user.email=${o.identity.email}`, 'commit', '-q', '-m', `Test workers: at most ${rec.workers}, from a profile on this machine`, '-m', rec.why); const head = git('rev-parse', 'HEAD'); diff --git a/src/vacuity.ts b/src/vacuity.ts index 8cc13a4f1..049ed1e0a 100644 --- a/src/vacuity.ts +++ b/src/vacuity.ts @@ -21,6 +21,7 @@ // optional call's arguments, a default parameter or destructuring default, a logical assignment's right // side, code in a string) or a build's map places code only by line. import { spawn, spawnSync } from './os/child-process.js'; +import { gitSpawnSync } from './os/git.js'; import { lstatSync, mkdtempSync, readdirSync, readFileSync, realpathSync, rmSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { dirname, isAbsolute, join, relative, resolve } from 'node:path'; @@ -1126,7 +1127,7 @@ export function baseTextReader(repo: string, base: string): (path: string) => st const seen = new Map(); return (path) => { if (!seen.has(path)) { - const r = spawnSync('git', ['--no-replace-objects', '-c', 'core.attributesFile=/dev/null', 'show', `${base}:${path}`], { cwd: repo, encoding: 'utf8', maxBuffer: SCRIPT_BYTES, stdio: ['ignore', 'pipe', 'ignore'] }); + const r = gitSpawnSync(['--no-replace-objects', '-c', 'core.attributesFile=/dev/null', 'show', `${base}:${path}`], { cwd: repo, encoding: 'utf8', maxBuffer: SCRIPT_BYTES, stdio: ['ignore', 'pipe', 'ignore'] }); seen.set(path, r.status === 0 && typeof r.stdout === 'string' ? r.stdout : null); } return seen.get(path)!; diff --git a/src/worktree-pool.ts b/src/worktree-pool.ts index d87ba70fa..25328e8f5 100644 --- a/src/worktree-pool.ts +++ b/src/worktree-pool.ts @@ -7,7 +7,7 @@ // When main moves and the files that pin the install (worktree.setup_key) didn't change, a pooled worktree // follows main by a checkout in place, keeping what its setup installed, so a merge doesn't empty the pool. It is // still set up on main only and handed to one task, never back: no task's change reaches another's dependencies. -import { execFileSync } from './os/child-process.js'; +import { gitExecFileSync } from './os/git.js'; import { createHash } from 'node:crypto'; import { existsSync, readFileSync } from 'node:fs'; import { join } from 'node:path'; @@ -29,7 +29,7 @@ export interface PoolEntry { export const POOL_MIN_FREE_GB = 10; const poolFile = (o: WorktreeOptions) => join(o.stateDir, 'worktree-pool.json'); -const git = (cwd: string, ...args: string[]) => execFileSync('git', args, { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); +const git = (cwd: string, ...args: string[]) => gitExecFileSync(args, { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); export function readPool(o: WorktreeOptions): PoolEntry[] { try { diff --git a/src/worktrees.ts b/src/worktrees.ts index bf0dbae28..6842dbe36 100644 --- a/src/worktrees.ts +++ b/src/worktrees.ts @@ -1,7 +1,8 @@ // Worktrees the coordinator creates and owns. It never lists, touches or // removes a worktree it didn't create: names carry our prefix AND must be in // our ownership record, so other sessions' worktrees are always safe. -import { execFileSync, spawn, spawnSync } from './os/child-process.js'; +import { spawn, spawnSync } from './os/child-process.js'; +import { gitExecFileSync, gitSpawn, gitSpawnSync } from './os/git.js'; import { existsSync, lstatSync, mkdirSync, readdirSync, readFileSync, realpathSync, rmdirSync, rmSync, unlinkSync, writeFileSync } from 'node:fs'; import { dirname, join, relative, resolve } from 'node:path'; import { BRAND } from './brand.js'; @@ -77,7 +78,7 @@ function setOwned(o: WorktreeOptions, list: string[]) { writeGroupOnly(ownedFile(o), JSON.stringify([...new Set(list)], null, 2)); } -const git = (cwd: string, ...args: string[]) => execFileSync('git', args, { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); +const git = (cwd: string, ...args: string[]) => gitExecFileSync(args, { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); /** * What Claude Code's sandbox leaves in a worktree: on Linux it protects paths by mounting over them, so a missing one @@ -316,7 +317,7 @@ export function lockWorktree(repo: string, path: string): void { /** Unlock the worktree at `path` for its removal; one that isn't locked, or isn't there, is left as it is. */ export function unlockWorktree(repo: string, path: string): void { - spawnSync('git', ['worktree', 'unlock', path], { cwd: repo, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }); + gitSpawnSync(['worktree', 'unlock', path], { cwd: repo, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }); } /** @@ -326,7 +327,7 @@ export function unlockWorktree(repo: string, path: string): void { export function lockOwnedWorktrees(o: WorktreeOptions): void { // Best effort: git refuses one already locked, and a record it can't lock (broken by hand) never stops a new // worktree being made. - for (const p of owned(o)) if (existsSync(p)) spawnSync('git', ['worktree', 'lock', '--reason', WORKTREE_LOCK_REASON, p], { cwd: o.repo, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }); + for (const p of owned(o)) if (existsSync(p)) gitSpawnSync(['worktree', 'lock', '--reason', WORKTREE_LOCK_REASON, p], { cwd: o.repo, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }); } /** The worktree itself, before any setup: on a new branch at `base`, recorded as ours first, and locked. */ @@ -423,7 +424,7 @@ function clearStale(o: WorktreeOptions, path: string): void { const rec = ownedAs(o, path); if (!rec) return; unlockWorktree(o.repo, rec); - spawnSync('git', ['worktree', 'prune'], { cwd: o.repo, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }); + gitSpawnSync(['worktree', 'prune'], { cwd: o.repo, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }); } function removeAt(o: WorktreeOptions, path: string): boolean { @@ -431,8 +432,8 @@ function removeAt(o: WorktreeOptions, path: string): boolean { if (!rec) throw new Error(`refusing to remove ${path}: not created by ${BRAND.cli}`); unlockWorktree(o.repo, rec); if (isOldPlace(o, path)) return emptyInPlace(o, rec, path); - spawnSync('git', ['worktree', 'remove', '--force', rec], { cwd: o.repo, encoding: 'utf8' }); - spawnSync('git', ['worktree', 'prune'], { cwd: o.repo }); + gitSpawnSync(['worktree', 'remove', '--force', rec], { cwd: o.repo, encoding: 'utf8' }); + gitSpawnSync(['worktree', 'prune'], { cwd: o.repo }); return dropWhenGone(o, rec, !existsSync(path)); } @@ -469,7 +470,7 @@ export async function removeWorktreeAsync(o: WorktreeOptions, name: string): Pro unlockWorktree(o.repo, rec); if (isOldPlace(o, path)) return emptyInPlace(o, rec, path); const run = (args: string[]) => new Promise((done) => { - const child = spawn('git', args, { cwd: o.repo, stdio: 'ignore' }); + const child = gitSpawn(args, { cwd: o.repo, stdio: 'ignore' }); child.on('error', () => done()); child.on('close', () => done()); }); @@ -502,7 +503,7 @@ function emptyInPlace(o: WorktreeOptions, rec: string, path: string): boolean { } for (const name of readdirSync(path)) rmSync(join(path, name), { recursive: true, force: true }); realDir(); - spawnSync('git', ['worktree', 'prune'], { cwd: o.repo, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }); + gitSpawnSync(['worktree', 'prune'], { cwd: o.repo, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }); realDir(); return dropWhenGone(o, rec, readdirSync(path).length === 0); } diff --git a/test/binary-diffs-as-main.test.ts b/test/binary-diffs-as-main.test.ts new file mode 100644 index 000000000..b11dd4615 --- /dev/null +++ b/test/binary-diffs-as-main.test.ts @@ -0,0 +1,46 @@ +// The owner's decision diff and the fix-forward culprit diff show a binary file as main does (#927; the owner's +// decision, 2026-10-21: "Cut --text, land the runner."). Both went through git with `--text` in this PR, which showed +// a binary file's bytes (on the owner's dashboard, and in a fix task's tracker issue). They run through the git runner +// as before, with main's arguments: a binary file is "Binary files ... differ", a text file shows its lines. +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { decisionDiff } from '../src/decision-detail.js'; +import { agents, commitAll, coordinator, fixture, git } from './repro-state-graph/harness.js'; + +/** A binary file (a NUL in it) holding a printable run that must not show, and a control byte. */ +const SECRET_RUN = 'NotShownRun7Qx2'; +const BLOB = Buffer.concat([Buffer.from(`header line\n${SECRET_RUN}`), Buffer.from([0, 1, 2, 0x0a]), Buffer.from('tail\n')]); + +/** In the fixture's clone: a commit adding a binary file and changing a text file. Its id and its parent's. */ +function culprit(repo: string): { base: string; head: string } { + const base = git(repo, 'rev-parse', 'HEAD'); + writeFileSync(join(repo, 'blob.bin'), BLOB); + writeFileSync(join(repo, 'notes.md'), 'a text line the diff shows\n'); + commitAll(repo, 'add a blob and notes'); + return { base, head: git(repo, 'rev-parse', 'HEAD') }; +} + +function asMain(diff: string) { + assert.match(diff, /Binary files \/dev\/null and b\/blob\.bin differ/, 'the binary file shows as main shows it'); + assert.match(diff, /^\+a text line the diff shows$/m, "the text file's lines show"); + assert.equal(diff.includes(SECRET_RUN), false, "none of the binary file's bytes show"); + assert.equal(diff.includes('\0'), false); +} + +test("the owner's decision diff shows a binary file as \"Binary files differ\", and a text file's lines", async () => { + const f = fixture(); + const { base, head } = culprit(f.repo); + const d = await decisionDiff(f.repo, base, head); + assert.ok(d.diff !== null, JSON.stringify(d)); + asMain(d.diff); +}); + +test('the fix-forward culprit diff (what a fix task\'s issue body holds) shows a binary file as "Binary files differ"', () => { + const f = fixture(); + const { head } = culprit(f.repo); + const c = coordinator(f, agents()); + const ff = (c as unknown as { fixForward: { d: { git: { diff: (sha: string) => string } } } }).fixForward; + asMain(ff.d.git.diff(head)); +}); diff --git a/test/compare-screenshots.test.ts b/test/compare-screenshots.test.ts index af2a0c926..d69755592 100644 --- a/test/compare-screenshots.test.ts +++ b/test/compare-screenshots.test.ts @@ -33,6 +33,9 @@ function fakeRepo(): string { git(origin, 'init', '-q', '--bare', '-b', 'main'); mkdirSync(join(repo, 'scripts', 'dev'), { recursive: true }); for (const f of ['compare-screenshots.mjs', 'children.mjs']) copyFileSync(join(repoRoot, 'scripts', 'dev', f), join(repo, 'scripts', 'dev', f)); + // The git runner compare-screenshots.mjs imports (#807). + mkdirSync(join(repo, 'scripts', 'lib'), { recursive: true }); + copyFileSync(join(repoRoot, 'scripts', 'lib', 'git.cjs'), join(repo, 'scripts', 'lib', 'git.cjs')); writeFileSync( join(repo, 'scripts', 'dev', 'screenshots.mjs'), [ diff --git a/test/engine-release.test.ts b/test/engine-release.test.ts index dbcaec0f6..5cf1fc3d9 100644 --- a/test/engine-release.test.ts +++ b/test/engine-release.test.ts @@ -97,7 +97,7 @@ function fixture() { // updater beside them, and its own .github/release-signers: the trusted list. const checkout = join(dir, 'checkout'); sh(dir, 'git', ['clone', '-q', origin, checkout]); - for (const f of ['scripts/setup/engine.sh', 'scripts/setup/lib.sh', 'scripts/setup/engine-release.cjs', 'scripts/machine/worklane-update.cjs']) { + for (const f of ['scripts/setup/engine.sh', 'scripts/setup/lib.sh', 'scripts/setup/engine-release.cjs', 'scripts/machine/worklane-update.cjs', 'scripts/lib/git.cjs']) { if (!existsSync(join(repoRoot, f))) continue; mkdirSync(join(checkout, f, '..'), { recursive: true }); copyFileSync(join(repoRoot, f), join(checkout, f)); diff --git a/test/eval-spend-gate.test.ts b/test/eval-spend-gate.test.ts index 30a7e0b8f..0a4003fa4 100644 --- a/test/eval-spend-gate.test.ts +++ b/test/eval-spend-gate.test.ts @@ -160,8 +160,10 @@ test('eval-roles.mjs: a dist/ whose eval-roles.js has no spend option still stop join(root, 'dist', 'src', 'eval-roles.js'), `export * from ${JSON.stringify(real('eval-roles.js'))};\nimport { evalRolesOptions as current } from ${JSON.stringify(real('eval-roles.js'))};\nexport const evalRolesOptions = (argv) => { const o = current(argv); delete o.spend; return o; };\n`, ); - mkdirSync(join(root, 'scripts')); + mkdirSync(join(root, 'scripts', 'lib'), { recursive: true }); writeFileSync(join(root, 'scripts', 'eval-roles.mjs'), readFileSync(evalRoles, 'utf8')); + // The git runner the script imports (#807). + writeFileSync(join(root, 'scripts', 'lib', 'git.cjs'), readFileSync(join(repoRoot, 'scripts', 'lib', 'git.cjs'), 'utf8')); for (const d of ['node_modules', 'src', 'templates']) symlinkSync(join(repoRoot, d), join(root, d)); const r = run(join(root, 'scripts', 'eval-roles.mjs'), ['--score', '--roles', 'feedback-checks'], m.env, root); assert.equal(r.status, 2, r.out); diff --git a/test/git-config-count.test.ts b/test/git-config-count.test.ts new file mode 100644 index 000000000..c1a7ef3fa --- /dev/null +++ b/test/git-config-count.test.ts @@ -0,0 +1,117 @@ +// The git runner's commit-graph setting in the environment, which the gits a program like gitleaks runs get (#927). +// Three copies set it: src/os/git.ts gitEnv, scripts/lib/git.cjs gitEnv, and the push gate's graphOff (it runs alone, +// so it writes them out). Each is tested here, the gate's taken from its source. +// - GIT_CONFIG_COUNT: only a canonical count (a plain decimal 0 to 9999) is read, and the runner's pair goes after the +// caller's. Any other count becomes one git refuses, so the git fails closed. Round 8 tried to read a count as git's +// strtoul does and got the sign wrong: `-0` is 0 to git, and lost the pair (#927 round 8's reviews, measured). +// Before that, Number() turned counts git refuses (`0x1`, `1e0`, `1 `) into a run that applied the caller's pair. +// - GIT_CONFIG_PARAMETERS (what `git -c` hands the gits it runs) is read after the COUNT pairs: a caller's +// `git -c core.commitGraph=true` there won (measured). The setting is appended to it too. +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { spawnSync } from 'node:child_process'; +import { createRequire } from 'node:module'; +import { mkdtempSync, readFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { gitEnv, gitSpawnSync } from '../src/os/git.js'; +import { repoRoot } from './helpers.js'; + +type EnvFn = (e: NodeJS.ProcessEnv) => NodeJS.ProcessEnv; +const scripts = createRequire(import.meta.url)(join(repoRoot, 'scripts', 'lib', 'git.cjs')) as { gitEnv: EnvFn }; +const gateSource = readFileSync(join(repoRoot, 'scripts', 'push-gate.mjs'), 'utf8'); +const gateFn = /const graphOff = (\(e\) => \{[\s\S]*?\n\});/.exec(gateSource); +assert.ok(gateFn, 'the push gate declares graphOff'); +const graphOff = new Function(`return ${gateFn[1]}`)() as EnvFn; +const COPIES: [string, EnvFn][] = [['src/os/git.ts gitEnv', gitEnv], ['scripts/lib/git.cjs gitEnv', scripts.gitEnv], ['scripts/push-gate.mjs graphOff', graphOff]]; + +const dir = mkdtempSync(join(tmpdir(), 'config-count-')); +const base = (): NodeJS.ProcessEnv => ({ PATH: process.env.PATH, HOME: dir, GIT_CONFIG_NOSYSTEM: '1', GIT_CONFIG_GLOBAL: '/dev/null' }); +/** `git config --get key` as a git that reads only its environment (gitleaks' gits): status and value. */ +const envOnly = (env: NodeJS.ProcessEnv, key: string) => { + const r = spawnSync('git', ['config', '--get', key], { cwd: dir, env, encoding: 'utf8' }); + return { status: r.status, value: r.stdout.trim() }; +}; +const pair = { GIT_CONFIG_KEY_0: 'x.y', GIT_CONFIG_VALUE_0: 'z' }; +const NBSP = String.fromCharCode(0xa0); + +// Counts git itself refuses, counts git reads that aren't canonical (`-0`, `+1`, ` 1`, `01`), and overlong ones. +const NOT_CANONICAL = [ + '', '+0', '+1', ' 1', '\v1', '\n1', '01', '0000001', '-0', ' -0', '-00', '\t-0', '-', '+', '+-1', '-+0', '1\n', '0x1', + '1e0', '1 ', 'abc', '-1', '-01', '1.5', ' ', `${NBSP}1`, '10000', '2147483646', '2147483647', '2147483648', '4294967296', + '9007199254740991', '9007199254740993', '18446744073709551615', '18446744073709551616', '-18446744073709551615', +]; + +for (const count of NOT_CANONICAL) { + test(`GIT_CONFIG_COUNT ${JSON.stringify(count)} isn't canonical: every copy makes it one git refuses, and git runs nothing`, () => { + for (const [which, fn] of COPIES) { + const env = fn({ ...base(), GIT_CONFIG_COUNT: count, ...pair }); + assert.equal(env.GIT_CONFIG_COUNT, 'refused', which); + assert.equal(env.GIT_CONFIG_PARAMETERS, "'core.commitGraph=false'", which); + const r = envOnly(env, 'x.y'); + assert.equal(r.status, 128, `${which}: git refuses the count`); + assert.equal(r.value, '', `${which}: the caller's pair is not applied`); + } + // And through the runner itself. + const r = gitSpawnSync(['config', '--get', 'x.y'], { cwd: dir, encoding: 'utf8', env: { ...base(), GIT_CONFIG_COUNT: count, ...pair } }); + assert.equal(r.status, 128, r.stderr); + assert.equal(r.stdout, ''); + }); +} + +for (const count of [undefined, '0', '1', '12', '9999']) { + test(`GIT_CONFIG_COUNT ${count === undefined ? 'unset' : JSON.stringify(count)} is canonical: every copy keeps the caller's pairs and puts the runner's after them`, () => { + const n = count === undefined ? 0 : Number(count); + for (const [which, fn] of COPIES) { + const env = fn({ ...base(), ...(count === undefined ? {} : { GIT_CONFIG_COUNT: count }), ...pair }); + assert.equal(env.GIT_CONFIG_COUNT, String(n + 1), which); + assert.equal(env[`GIT_CONFIG_KEY_${n}`], 'core.commitGraph', which); + assert.equal(env[`GIT_CONFIG_VALUE_${n}`], 'false', which); + assert.equal(env.GIT_CONFIG_PARAMETERS, "'core.commitGraph=false'", which); + if (n > 1) continue; // git needs every pair below the count: the shape is what is checked here + assert.deepEqual(envOnly(env, 'core.commitGraph'), { status: 0, value: 'false' }, which); + assert.deepEqual(envOnly(env, 'x.y'), n === 1 ? { status: 0, value: 'z' } : { status: 1, value: '' }, which); + } + }); +} + +for (const [what, params] of [ + ['old form', "'core.commitGraph=true'"], + ['new form', "'core.commitGraph'='true'"], + ['with another key first', "'a.b'='c' 'core.commitGraph'='true'"], +] as const) { + test(`a caller's GIT_CONFIG_PARAMETERS (${what}) setting core.commitGraph=true: a git that reads only the environment still reads false`, () => { + // Plain git believes the caller's. + assert.deepEqual(envOnly({ ...base(), GIT_CONFIG_PARAMETERS: params }, 'core.commitGraph'), { status: 0, value: 'true' }); + for (const [which, fn] of COPIES) { + const env = fn({ ...base(), GIT_CONFIG_PARAMETERS: params }); + assert.equal(env.GIT_CONFIG_PARAMETERS, `${params} 'core.commitGraph=false'`, which); + assert.deepEqual(envOnly(env, 'core.commitGraph'), { status: 0, value: 'false' }, which); + // Also with the caller's count pair saying true. + const both = fn({ ...base(), GIT_CONFIG_PARAMETERS: params, GIT_CONFIG_COUNT: '1', GIT_CONFIG_KEY_0: 'core.commitGraph', GIT_CONFIG_VALUE_0: 'true' }); + assert.deepEqual(envOnly(both, 'core.commitGraph'), { status: 0, value: 'false' }, which); + } + const r = gitSpawnSync(['config', '--get', 'core.commitGraph'], { cwd: dir, encoding: 'utf8', env: { ...base(), GIT_CONFIG_PARAMETERS: params } }); + assert.equal(r.stdout.trim(), 'false'); + }); +} + +test('a malformed caller GIT_CONFIG_PARAMETERS stays one git refuses, or reads false: never true', () => { + for (const params of ["'a.b'='c", "'core.commitGraph=true", "core.commitGraph=true", "'core.commitGraph'='true"]) { + for (const [which, fn] of COPIES) { + const r = envOnly(fn({ ...base(), GIT_CONFIG_PARAMETERS: params }), 'core.commitGraph'); + assert.ok(r.status === 128 || r.value === 'false', `${which} ${JSON.stringify(params)}: ${JSON.stringify(r)}`); + } + } +}); + +test('the copies agree on every count and parameters value tried here', () => { + for (const count of [...NOT_CANONICAL, '0', '1', '12', '9999']) { + const input = { ...base(), GIT_CONFIG_COUNT: count, ...pair, GIT_CONFIG_PARAMETERS: "'a.b=c'" }; + const want = gitEnv({ ...input }); + for (const [which, fn] of COPIES.slice(1)) { + const got = fn({ ...input }); + for (const k of Object.keys(want).filter((key) => key.startsWith('GIT_CONFIG'))) assert.equal(got[k], want[k], `${which} ${JSON.stringify(count)} ${k}`); + } + } +}); diff --git a/test/git-runner.test.ts b/test/git-runner.test.ts new file mode 100644 index 000000000..f3b56aa76 --- /dev/null +++ b/test/git-runner.test.ts @@ -0,0 +1,306 @@ +// lint-test +// One git runner for every git call the engine makes (#807). refs/replace/* and $GIT_DIR/info/grafts are in the +// repository, which every worktree of it shares, so an agent can write either; every git the engine runs there then +// reads the agent's stand-in. A `git replace` of the pushed commit hid a refused file and a big one from the push +// limits (2 problems became 0), and a graft that makes the base reach the head emptied the range (2 became 0). +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { execFileSync, spawnSync } from 'node:child_process'; +import { createHash } from 'node:crypto'; +import { mkdirSync, mkdtempSync, readdirSync, readFileSync, writeFileSync } from 'node:fs'; +import { createRequire } from 'node:module'; +import { devNull, tmpdir } from 'node:os'; +import { join, relative } from 'node:path'; +import { ReviewConfig } from '../src/config/schema.js'; +import { DEFAULT_PUSH_LIMITS, pushProblems, type PushLimits } from '../src/push-check.js'; +import { computeLevel, SELF_MODIFICATION, selfModificationPart } from '../src/review.js'; +import { findTool } from '../src/tool-path.js'; +import { repoRoot } from './helpers.js'; + +const git = (cwd: string, ...a: string[]) => execFileSync('git', a, { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); +const limits = (over: Partial = {}): PushLimits => ({ ...DEFAULT_PUSH_LIMITS, ...over }); + +/** A repo with one commit (base) and a change on it (head) that adds a refused file and a big one. */ +function repo() { + const work = mkdtempSync(join(tmpdir(), 'git-runner-')); + git(work, 'init', '-q', '-b', 'main'); + for (const [k, v] of [['user.email', 'bot@example.com'], ['user.name', 'bot'], ['commit.gpgsign', 'false']]) git(work, 'config', k!, v!); + writeFileSync(join(work, 'README.md'), 'hello\n'); + git(work, 'add', 'README.md'); + git(work, 'commit', '-q', '-m', 'init'); + const base = git(work, 'rev-parse', 'HEAD'); + mkdirSync(join(work, 'keys')); + writeFileSync(join(work, 'keys', 'secret.pem'), 'not a real key\n'); + writeFileSync(join(work, 'weights.bin'), 'x'.repeat(4096)); + git(work, 'add', '.'); + git(work, 'commit', '-q', '-m', 'add a key and a big file'); + return { work, base, head: git(work, 'rev-parse', 'HEAD') }; +} + +const RULES = limits({ refuse_paths: ['**/*.pem'], max_file_mb: 0.002 }); + +test('a replace ref on the pushed commit doesn\'t hide a refused file or a big one from the push limits', () => { + const r = repo(); + const before = pushProblems(r.work, r.base, r.head, RULES); + assert.equal(before.length, 2, before.join('\n')); + // The decoy: the same parent, base's tree (no key, no big file). + const decoy = git(r.work, 'commit-tree', `${r.base}^{tree}`, '-p', r.base, '-m', 'docs'); + git(r.work, 'replace', r.head, decoy); + // git itself now reads the decoy. + assert.equal(git(r.work, 'log', '--format=', '--name-only', `${r.base}..${r.head}`), ''); + const after = pushProblems(r.work, r.base, r.head, RULES); + assert.equal(after.length, 2, after.join('\n')); + assert.match(after.join('\n'), /keys\/secret\.pem/); + assert.match(after.join('\n'), /weights\.bin/); +}); + +test('a graft that makes the base reach the head doesn\'t empty the range the push limits read', () => { + const r = repo(); + // $GIT_DIR/info/grafts: the base commit now has the head as a parent, so base..head is empty to git. + const grafts = join(r.work, '.git', 'info', 'grafts'); + writeFileSync(grafts, `${r.base} ${r.head}\n`); + assert.equal(execFileSync('git', ['rev-list', `${r.base}..${r.head}`], { cwd: r.work, encoding: 'utf8', stdio: ['ignore', 'pipe', 'ignore'] }), ''); + const after = pushProblems(r.work, r.base, r.head, RULES); + assert.equal(after.length, 2, after.join('\n')); + assert.match(after.join('\n'), /keys\/secret\.pem/); + assert.match(after.join('\n'), /weights\.bin/); +}); + +test('gitleaks, which runs git itself, gets the settings: a replace ref on the leaky commit doesn\'t hide its secret from the change scan', { skip: !findTool('gitleaks') && 'gitleaks not installed' }, async () => { + const { scanRange } = await import('../src/scan/secrets.js'); + const r = repo(); + writeFileSync(join(r.work, 'c.js'), `export const token = '${'ghp_' + 'a1B2c3D4e5F6g7H8i9J0k1L2m3N4o5P6q7R8'}';\n`); + git(r.work, 'add', 'c.js'); + git(r.work, 'commit', '-q', '-m', 'leaky change'); + const leaky = git(r.work, 'rev-parse', 'HEAD'); + assert.equal((await scanRange(r.work, `${r.head}..${leaky}`)).status, 'leaks'); + const decoy = git(r.work, 'commit-tree', `${r.head}^{tree}`, '-p', r.head, '-m', 'docs'); + git(r.work, 'replace', leaky, decoy); + assert.equal((await scanRange(r.work, `${r.head}..${leaky}`)).status, 'leaks'); +}); + +test('the push gate script (scripts/push-gate.mjs), which runs alone, scans the real commits under a replace ref', { skip: !findTool('gitleaks') && 'gitleaks not installed' }, () => { + const r = repo(); + const hash = (t: string) => createHash('sha256').update(t).digest('hex'); + const real = JSON.parse(readFileSync(join(repoRoot, '.denylist.json'), 'utf8')) as { patterns: string[] }; + writeFileSync(join(r.work, '.denylist.json'), JSON.stringify({ hashed: [hash('secretproject')], patterns: real.patterns })); + git(r.work, 'add', '.denylist.json'); + git(r.work, 'commit', '-q', '-m', 'rules'); + const base = git(r.work, 'rev-parse', 'HEAD'); + writeFileSync(join(r.work, 'notes.md'), 'deployed by secretproject\n'); + git(r.work, 'add', 'notes.md'); + git(r.work, 'commit', '-q', '-m', 'add notes'); + const head = git(r.work, 'rev-parse', 'HEAD'); + const gate = () => spawnSync(process.execPath, [join(repoRoot, 'scripts', 'push-gate.mjs'), '--range', `${base}..${head}`], { cwd: r.work, encoding: 'utf8' }); + const before = gate(); + assert.notEqual(before.status, 0, 'refused before the replace'); + assert.match(before.stderr, /notes\.md/); + const decoy = git(r.work, 'commit-tree', `${base}^{tree}`, '-p', base, '-m', 'add notes'); + git(r.work, 'replace', head, decoy); + const after = gate(); + assert.notEqual(after.status, 0, after.stderr); + assert.match(after.stderr, /REFUSED[\s\S]*notes\.md/); +}); + +// ---- The runner itself. + +test('the runner\'s settings go on last: a caller\'s environment can\'t turn replace refs or grafts back on', async () => { + const { GIT_FIXED_ENV, gitEnv, gitExecFileSync, gitSpawnSync } = await import('../src/os/git.js'); + const r = repo(); + // A graft file of the caller's choosing (the base reaches the head): the runner's GIT_GRAFT_FILE wins. + const grafts = join(r.work, 'grafts'); + writeFileSync(grafts, `${r.base} ${r.head}\n`); + const env = { ...process.env, GIT_GRAFT_FILE: grafts }; + assert.equal(execFileSync('git', ['rev-list', `${r.base}..${r.head}`], { cwd: r.work, encoding: 'utf8', env, stdio: ['ignore', 'pipe', 'ignore'] }), ''); + const names = (out: string) => out.split('\n').filter(Boolean).sort(); + const want = ['keys/secret.pem', 'weights.bin']; + assert.deepEqual(names(gitExecFileSync(['log', '--format=', '--name-only', `${r.base}..${r.head}`], { cwd: r.work, encoding: 'utf8', env })), want); + assert.deepEqual(names(gitSpawnSync(['log', '--format=', '--name-only', `${r.base}..${r.head}`], { cwd: r.work, encoding: 'utf8', env }).stdout), want); + const graphOff = { GIT_CONFIG_COUNT: '1', GIT_CONFIG_KEY_0: 'core.commitGraph', GIT_CONFIG_VALUE_0: 'false', GIT_CONFIG_PARAMETERS: "'core.commitGraph=false'" }; + assert.deepEqual({ ...gitEnv({ GIT_GRAFT_FILE: grafts, GIT_NO_REPLACE_OBJECTS: '0' }) }, { ...GIT_FIXED_ENV, ...graphOff }); + // A caller's own config pairs stay; the fixed pair goes after them (a later pair wins). + assert.deepEqual({ ...gitEnv({ GIT_CONFIG_COUNT: '1', GIT_CONFIG_KEY_0: 'core.commitGraph', GIT_CONFIG_VALUE_0: 'true' }) }, { ...GIT_FIXED_ENV, GIT_CONFIG_COUNT: '2', GIT_CONFIG_KEY_0: 'core.commitGraph', GIT_CONFIG_VALUE_0: 'true', GIT_CONFIG_KEY_1: 'core.commitGraph', GIT_CONFIG_VALUE_1: 'false', GIT_CONFIG_PARAMETERS: "'core.commitGraph=false'" }); + // A count on Object.prototype is never read (#846): the fixed pair is pair 0, and nothing else is counted. + const proto = Object.prototype as Record; + proto.GIT_CONFIG_COUNT = '1'; + try { + assert.deepEqual({ ...gitEnv({}) }, { ...GIT_FIXED_ENV, ...graphOff }); + } finally { + delete proto.GIT_CONFIG_COUNT; + } + // Replace refs: `--no-replace-objects` is in the argument list, which no environment takes back. + const decoy = git(r.work, 'commit-tree', `${r.base}^{tree}`, '-p', r.base, '-m', 'docs'); + git(r.work, 'replace', r.head, decoy); + assert.deepEqual(names(gitExecFileSync(['log', '--format=', '--name-only', `${r.base}..${r.head}`], { cwd: r.work, encoding: 'utf8', env: {} })), want); +}); + +test('a runner that takes any command (a probe\'s run) gives git the settings, by name or path, and leaves others as they are', { skip: process.platform === 'win32' && 'sh' }, async () => { + const { commandSpawnSync, isGitCommand } = await import('../src/os/git.js'); + const r = repo(); + const decoy = git(r.work, 'commit-tree', `${r.base}^{tree}`, '-p', r.base, '-m', 'docs'); + git(r.work, 'replace', r.head, decoy); + const gitPath = execFileSync('sh', ['-c', 'command -v git'], { encoding: 'utf8' }).trim(); + for (const cmd of ['git', gitPath]) { + const out = commandSpawnSync(cmd, ['log', '--format=', '--name-only', `${r.base}..${r.head}`], { cwd: r.work, encoding: 'utf8' }).stdout; + assert.match(out, /keys\/secret\.pem/, cmd); + } + assert.ok(isGitCommand('git') && isGitCommand('/usr/bin/git') && isGitCommand('C:\\Git\\cmd\\git.exe') && isGitCommand('GIT')); + assert.ok(!isGitCommand('gitleaks') && !isGitCommand('/usr/bin/legit') && !isGitCommand('gh')); + const argv = commandSpawnSync(process.execPath, ['-e', 'console.log(JSON.stringify(process.argv.slice(1)))', 'a'], { encoding: 'utf8' }); + assert.deepEqual(JSON.parse(argv.stdout), ['a']); +}); + +test('scripts/lib/git.cjs (scripts run without a build) has the engine runner\'s settings, and applies them last', async () => { + const engine = await import('../src/os/git.js'); + const scripts = createRequire(import.meta.url)(join(repoRoot, 'scripts', 'lib', 'git.cjs')) as typeof engine; + assert.deepEqual([...scripts.GIT_FIXED_ARGS], [...engine.GIT_FIXED_ARGS]); + assert.deepEqual({ ...scripts.GIT_FIXED_ENV }, { ...engine.GIT_FIXED_ENV }); + // What the files that run on their own write out (ITS_OWN, below) is these values. + assert.deepEqual(scripts.GIT_FIXED_CONFIG.map((p) => [...p]), engine.GIT_FIXED_CONFIG.map((p) => [...p])); + assert.deepEqual({ ...scripts.gitEnv({ GIT_CONFIG_COUNT: '1', GIT_CONFIG_KEY_0: 'a.b', GIT_CONFIG_VALUE_0: 'c' }) }, { ...engine.gitEnv({ GIT_CONFIG_COUNT: '1', GIT_CONFIG_KEY_0: 'a.b', GIT_CONFIG_VALUE_0: 'c' }) }); + assert.deepEqual([...engine.GIT_FIXED_ARGS], ['--no-replace-objects', '-c', 'core.commitGraph=false']); + assert.deepEqual({ ...engine.GIT_FIXED_ENV }, { GIT_NO_REPLACE_OBJECTS: '1', GIT_GRAFT_FILE: join(devNull, 'no-grafts') }); + const r = repo(); + const decoy = git(r.work, 'commit-tree', `${r.base}^{tree}`, '-p', r.base, '-m', 'docs'); + git(r.work, 'replace', r.head, decoy); + writeFileSync(join(r.work, '.git', 'info', 'grafts'), `${r.base} ${r.head}\n`); + const env = { ...process.env, GIT_GRAFT_FILE: join(r.work, '.git', 'info', 'grafts') }; + assert.match(scripts.gitExecFileSync(['log', '--format=', '--name-only', `${r.base}..${r.head}`], { cwd: r.work, encoding: 'utf8', env }), /keys\/secret\.pem/); + assert.match(scripts.gitSpawnSync(['log', '--format=', '--name-only', `${r.base}..${r.head}`], { cwd: r.work, encoding: 'utf8', env }).stdout, /keys\/secret\.pem/); + assert.match(scripts.commandSpawnSync('git', ['log', '--format=', '--name-only', `${r.base}..${r.head}`], { cwd: r.work, encoding: 'utf8' }).stdout, /keys\/secret\.pem/); +}); + +// ---- No git call in src/ or scripts/ bypasses the runner. + +/** + * Where a source text starts git itself: `git` (or a path to it, or git.exe) as a quoted string in an argument or array + * position or assigned to a name (`spawnSync('git', …)`, `run('git', …)`, `['git', 'fetch']`, `const bin = 'git'`), or + * a shell line that starts with git (`exec('git log')`, `['-c', 'git log']`). A comparison (`=== 'git'`) or an object + * key's value (`type: 'git'`) isn't one. A backstop next to the runner, not a proof. Measured misses (#927's final + * check): a name built at run time (`'gi' + 't'`, `${'git'}`, String.raw`git`), a ternary or a default (`x ? 'git' :`, + * `?? 'git'`, `|| 'git'`), a command from config, env or an object property, git in the middle of a shell string + * (`bash -lc 'cd x && git log'`, `execSync('cd x && git log')`, `spawnSync('git log', { shell: true })`), and the + * `.sh` files under scripts/ (16 run git; not scanned). + */ +function gitStarts(text: string): number { + const patterns = [ + /(?:[([,]|(?])=)\s*(['"`])(?:[^'"`\s]*[\\/])?git(?:\.exe)?\1/gi, + /\b(?:exec|execSync)\s*\(\s*(['"`])\s*git\s/g, + /(['"`])-c\1\s*,\s*(['"`])\s*git\s/g, + ]; + return patterns.reduce((n, re) => n + (text.match(re)?.length ?? 0), 0); +} + +// check-table: never-grow +const RUNNERS = new Set(['src/os/git.ts', 'scripts/lib/git.cjs']); + +/** + * A file that runs on its own (copied alone, or fetched alone from origin) can't import a runner: it holds the + * settings itself, written out exactly so, and the test below ties them to the runner's values. + */ +const ITS_OWN = [/const GIT_FIXED_ARGS = (?:Object\.freeze\()?\['--no-replace-objects', '-c', 'core\.commitGraph=false'\]/, /GIT_NO_REPLACE_OBJECTS: '1', GIT_GRAFT_FILE: join\(devNull, 'no-grafts'\)/]; +/** A runner that takes any command (`run(cmd, args)`), and so gives git the settings by its name. */ +const BY_NAME = (fn: string, call: string) => [new RegExp(`${fn}\\(${call}`)]; + +/** + * Files that start git outside the runners, each with how many times (exactly: one more is a new call) and why. Each + * is a call through a runner that takes any command and gives git the settings by its name (commandSpawnSync), a + * file that runs on its own and holds the settings itself, or text that names git without starting it. + */ +// check-table: never-grow +const OUTSIDE_THE_RUNNER: Record = { + // Through a runner that takes any command: its real one spawns through commandSpawnSync (src/os/git.ts). + 'src/init.ts': { n: 6, why: "`c.io.run('git', ...)` and `io.run('git', ...)`: systemIo().run, here, spawns through commandSpawnSync; and `manual.includes('git')`, a hint's text", must: BY_NAME('commandSpawnSync', 'cmd, args, ') }, + 'src/setup-steps.ts': { n: 7, why: "`k.p.run('git', ...)`: systemProbe().run, here, spawns through commandSpawnSync; and REQUIRED_TOOLS, two lists of tool names", must: BY_NAME('commandSpawnSync', 'cmd, args, ') }, + 'src/setup-mode.ts': { n: 3, why: "originPlan's two commands and `base.run('git', ...)`: each run by an InitIo's run, systemIo() in src/init.ts (commandSpawnSync)" }, + 'src/setup-actions.ts': { n: 3, why: "defaultBranchCommands' two commands, run here through commandSpawnSync; and `manual.includes('git')`, a hint's text", must: BY_NAME('commandSpawnSync', 'cmd\\[0\\]!, cmd\\.slice\\(1\\), ') }, + 'src/starter-ci.ts': { n: 3, why: "`run('git', ...)`: realRun, here, spawns through commandSpawnSync", must: BY_NAME('commandSpawnSync', 'cmd, args, ') }, + 'src/testrun/host.ts': { n: 1, why: "`run('git', gitArgs(...), { env: gitEnv(...) })`: the async runner, given the settings by hand", must: [/run\('git', gitArgs\(\[/, /env: gitEnv\(\{ PATH/] }, + // Text that names git without starting it. + 'src/scan/secrets.ts': { n: 1, why: "gitleaks' `git` subcommand (scanRange); gitleaks runs git itself, so it gets gitEnv() over the snapshot's env. The commit scan's own gitleaks call went with #979 (it scans files, not git)", must: [/\['git', '--redact', '--no-banner', '--log-opts', ids,/, /stdio: \['ignore', 'ignore', 'pipe'\], env: gitEnv\(env\) \}/] }, + 'src/review.ts': { n: 2, why: "`'git'`: the name of a way to read a .gitattributes line" }, + 'src/plugins/approval.ts': { n: 1, why: "`z.literal('git')`: a plugin source's type" }, + 'src/updater-setup.ts': { n: 1, why: 'a comment quoting an allowed-signers line (namespaces="git")' }, + 'src/os/index.ts': { n: 2, why: "`which('git')` and the `Git` folder: where Git for Windows' bash is, never run as git" }, + 'scripts/verify-release-tag.mjs': { n: 2, why: "ssh-keygen's `-n git`: the signature namespace" }, + // Runs on its own, with the settings written out. + 'scripts/push-gate-hook.mjs': { n: 1, why: 'runGit: setup-hooks copies this file alone into the hooks dir; it hands the gate the settings in its environment too', must: [...ITS_OWN, /\{ \.\.\.process\.env, GIT_DIR: gitDir, \.\.\.GIT_FIXED_ENV \}/] }, + 'scripts/push-gate.mjs': { n: 2, why: "runGit, and gitleaks' `git` subcommand (with GIT_ENV): the hook runs this file alone, fetched from origin (GATE_CODE)", must: [...ITS_OWN, /\{ encoding: 'utf8', env: GIT_ENV \}/, /const GIT_ENV = graphOff\(\{ \.\.\.process\.env, /, /\[`GIT_CONFIG_KEY_\$\{n\}`\]: 'core\.commitGraph', \[`GIT_CONFIG_VALUE_\$\{n\}`\]: 'false'/] }, + 'scripts/ci/new-deps.mjs': { n: 1, why: "runGit: CI runs the base's copy of this file alone, from a temp dir (new-deps.yml)", must: ITS_OWN }, + 'scripts/ci/testrun-real-systemd.mjs': { n: 1, why: 'its git: the job copies this file alone into the VM (ci-slow.yml)', must: [/const GIT_FIXED_ARGS = \['--no-replace-objects', '-c', 'core\.commitGraph=false'\]/, /GIT_NO_REPLACE_OBJECTS: '1', GIT_GRAFT_FILE: join\(devNull, 'no-grafts'\)/, /execFileSync\('git', \[\.\.\.GIT_FIXED_ARGS, /] }, + 'scripts/machine/worklane-update.cjs': { + n: 7, + why: "the root updater, installed alone in /usr/local/libexec: its reads of the verification clone carry VERIFY_GIT_ARGS (replace refs off); that clone is root's own, which no agent can write a graft into; the three ls-remote calls read no object", + must: [/^const VERIFY_GIT_ARGS = \['--no-replace-objects'\];$/m], + }, + // CI and dev scripts, through a runner that takes any command: commandSpawnSync / commandExecFileSync / commandSpawn (scripts/lib/git.cjs). + 'scripts/ci/updater-setup-root.mjs': { n: 1, why: "`sh('git', ...)`: sh, here, runs through commandExecFileSync", must: BY_NAME('commandExecFileSync', 'file, args, ') }, + 'scripts/ci/wizard-e2e.mjs': { n: 5, why: "`sh('git', ...)`: sh, here, runs through commandSpawnSync", must: BY_NAME('commandSpawnSync', 'cmd, args, ') }, + 'scripts/dev/compare-screenshots.mjs': { n: 4, why: "`run('git', ...)`: run, here, spawns through commandSpawn", must: BY_NAME('commandSpawn', 'cmd, args, ') }, +}; + +function sources(): string[] { + const files: string[] = []; + const walk = (d: string) => { + for (const e of readdirSync(d, { withFileTypes: true })) { + if (e.isDirectory()) walk(join(d, e.name)); + else if (/\.(ts|mts|cts|mjs|cjs|js)$/.test(e.name)) files.push(join(d, e.name)); + } + }; + walk(join(repoRoot, 'src')); + walk(join(repoRoot, 'scripts')); + return files; +} + +test('every git call in src/ and scripts/ goes through the runner (src/os/git.ts, scripts/lib/git.cjs)', () => { + const bad: string[] = []; + const seen = new Set(); + for (const f of sources()) { + const rel = relative(repoRoot, f).split('\\').join('/'); + if (RUNNERS.has(rel)) continue; + const text = readFileSync(f, 'utf8'); + const n = gitStarts(text); + if (!n) continue; + seen.add(rel); + const allowed = OUTSIDE_THE_RUNNER[rel]; + if (!allowed) bad.push(`${rel}: starts git ${n} time(s) (not on the list)`); + else if (allowed.n !== n) bad.push(`${rel}: starts git ${n} time(s), listed ${allowed.n}`); + else for (const re of allowed.must ?? []) if (!re.test(text)) bad.push(`${rel}: no ${re}`); + } + for (const rel of Object.keys(OUTSIDE_THE_RUNNER)) if (!seen.has(rel)) bad.push(`${rel}: listed, but starts git no more (take it off the list)`); + assert.deepEqual(bad, [], 'run git through src/os/git.ts (gitSpawnSync, gitExecFileSync, gitSpawn, gitExecFile, commandSpawnSync) or scripts/lib/git.cjs'); +}); + +test('the scan catches each way of starting git, and not text that only names it', () => { + const caught = [ + "spawnSync('git', ['log'])", + 'execFileSync("git", ["show", x])', + 'spawn(`git`, args, {})', + "execFile('git', a, o, cb)", + "run('/usr/bin/git', ['fetch'])", + "spawnSync('C:\\\\Git\\\\cmd\\\\git.exe', [])", + "spawnSync('GIT', [])", + "const commands = [['git', 'fetch', '-q']];", + "const bin = 'git';", + "exec('git log -1', cb)", + 'execSync(`git status`)', + "spawnSync('sh', ['-c', 'git log'])", + "spawnSync(\n 'git',\n ['log'],\n)", + ]; + for (const c of caught) assert.equal(gitStarts(c), 1, c); + const named = ["if (bin === 'git') return;", "basename(a[0]) !== 'git'", "{ type: 'git', url }", "// run git log", "'gitleaks'", "spawnSync('legit', [])", "'git log' + x", "the git"]; + for (const c of named) assert.equal(gitStarts(c), 0, c); +}); + +test('the runners and these tests are locked: a change to any of them alone is self-modification, L3, never L0', () => { + const cfg = ReviewConfig.parse({ version: 1, levels: { L0_auto: { when: ['docs-only', 'tests-only'], max_lines: 200 }, L1_evaluator: { when: [] }, L2_notify: { when: [] }, L3_human: { when: [] } } }); + for (const [path, part] of [['test/git-runner.test.ts', 'the tests of the harness controls'], ['src/os/git.ts', 'the git runner'], ['scripts/lib/git.cjs', 'the git runner']] as const) { + const r = computeLevel({ files: [{ path, added: 1, removed: 1 }], labels: [], moneyPaths: [], scope: undefined }, cfg); + assert.deepEqual([r.level, r.categories[SELF_MODIFICATION], selfModificationPart(path)], ['L3', [path], part], path); + } +}); + +test('the runners the scan skips are the two runner files, and each starts git itself', () => { + assert.deepEqual([...RUNNERS].sort(), ['scripts/lib/git.cjs', 'src/os/git.ts']); + for (const rel of RUNNERS) assert.ok(gitStarts(readFileSync(join(repoRoot, rel), 'utf8')) > 0, rel); +}); diff --git a/test/installed-engine-config.test.ts b/test/installed-engine-config.test.ts index 7b31b6151..56300eb1a 100644 --- a/test/installed-engine-config.test.ts +++ b/test/installed-engine-config.test.ts @@ -184,7 +184,7 @@ const labelEvent = (event: 'labeled' | 'unlabeled', login: string, at: string, n const workflow = () => readFileSync(join(repoRoot, '.github', 'workflows', 'installed-engine-config.yml'), 'utf8'); /** The check's scripts as this checkout has them, to commit into a fixture's main. */ const checkScripts = () => - Object.fromEntries(['scripts/ci/installed-engine-config.mjs', 'scripts/lib/engine-pin.mjs', 'scripts/verify-release-tag.mjs'].map((f) => [f, readFileSync(join(repoRoot, f), 'utf8')])); + Object.fromEntries(['scripts/ci/installed-engine-config.mjs', 'scripts/lib/engine-pin.mjs', 'scripts/verify-release-tag.mjs', 'scripts/lib/git.cjs'].map((f) => [f, readFileSync(join(repoRoot, f), 'utf8')])); const refused = (r: { code: number | null; out: string }, why: RegExp) => { assert.equal(r.code, 1, r.out); diff --git a/test/intake-coordinator.test.ts b/test/intake-coordinator.test.ts index c94877f4e..ab4fe0e63 100644 --- a/test/intake-coordinator.test.ts +++ b/test/intake-coordinator.test.ts @@ -177,7 +177,8 @@ test('intake (#333): a bug already fixed on main is answered naming the fixing c onMain = git(t.repo, 'rev-parse', 'HEAD'); git(t.repo, 'update-ref', 'refs/remotes/origin/main', onMain); const spawned = await spawnsDuring(() => t.c.checkIntake()); - const check = (id: string) => `execFileSync git merge-base --is-ancestor ${id} refs/remotes/origin/main`; + // Through the git runner (#807): replace refs off. + const check = (id: string) => `execFileSync git --no-replace-objects -c core.commitGraph=false merge-base --is-ancestor ${id} refs/remotes/origin/main`; assert.deepEqual(spawned, [check(onMain), check('0a1b2c3d4e5f')], 'the only process: one read-only git check per fixing commit, on the trusted checkout'); assert.equal(t.discord.replies.length, 3); assert.match(t.discord.replies[0]!.text, new RegExp(`already fixed.*\\b${onMain}\\b`), 'confirmed on main: named'); diff --git a/test/push-shallow-format.test.ts b/test/push-shallow-format.test.ts new file mode 100644 index 000000000..767786f5c --- /dev/null +++ b/test/push-shallow-format.test.ts @@ -0,0 +1,139 @@ +// Round 4 of #927: a shallow file line is read as one object id of the repo's own format, and nothing else. +// - In a sha1 repo git reads a 64-hex line as a cut at its first 40 hex digits. Round 3 took any 40- or 64-hex line as +// an id, so the line hid the commit that added a refused file, and the snapshot copied the same cut. A separate +// process toggling the line got the file onto the remote in 34 of 80 checkedPush calls (measured by the round-3 check). +// - In a sha256 repo git refuses a 40-hex line (`fatal: bad shallow line`), and round 3's pushProblems threw instead of +// refusing. +// - Round 2's 1 KB form: git reads the file 1 KB at a time, so an id 1,023 characters into one line is a line of its own +// to git. An exact-match check took the whole line as no id at all. +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { execFileSync } from 'node:child_process'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { createRequire, syncBuiltinESMExports } from 'node:module'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { checkedPush, DEFAULT_PUSH_LIMITS, pushProblems } from '../src/push-check.js'; + +const git = (cwd: string, ...a: string[]) => execFileSync('git', a, { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); +const LIMITS = { ...DEFAULT_PUSH_LIMITS, refuse_paths: ['**/*.pem'] }; +const guard = { defaultBranch: 'main', blocked: () => undefined }; +const CUT = /history is cut inside the range \(its shallow file has a line that isn't exactly a full lowercase object id/; + +/** base, K (adds keys/secret.pem), D (drops it), H (a note), in a repo of `format`: the net diff is clean, the history isn't. */ +function repo(format: 'sha1' | 'sha256' = 'sha1') { + const root = mkdtempSync(join(tmpdir(), 'push-shallow-format-')); + const work = join(root, 'work'); + const remote = join(root, 'remote.git'); + git(root, 'init', '-q', '--bare', `--object-format=${format}`, '-b', 'main', remote); + git(root, 'init', '-q', `--object-format=${format}`, '-b', 'main', work); + for (const [k, v] of [['user.email', 'bot@example.com'], ['user.name', 'bot'], ['commit.gpgsign', 'false']]) git(work, 'config', k!, v!); + writeFileSync(join(work, 'README.md'), 'hello\n'); + git(work, 'add', '-A'); + git(work, 'commit', '-q', '-m', 'base'); + const base = git(work, 'rev-parse', 'HEAD'); + git(work, 'push', '-q', remote, 'main'); + mkdirSync(join(work, 'keys')); + writeFileSync(join(work, 'keys', 'secret.pem'), 'k\n'); + git(work, 'add', '-A'); + git(work, 'commit', '-q', '-m', 'add a key'); + git(work, 'rm', '-q', 'keys/secret.pem'); + git(work, 'commit', '-q', '-m', 'drop it'); + const drop = git(work, 'rev-parse', 'HEAD'); + writeFileSync(join(work, 'notes.txt'), 'n\n'); + git(work, 'add', '-A'); + git(work, 'commit', '-q', '-m', 'notes'); + const head = git(work, 'rev-parse', 'HEAD'); + return { work, remote, base, drop, head, shallow: join(work, '.git', 'shallow') }; +} + +const plainCount = (r: ReturnType) => git(r.work, 'rev-list', '--count', `${r.base}..${r.head}`); +const pushedKey = (r: ReturnType) => { + try { + git(r.remote, 'cat-file', '-e', `${r.head}~2:keys/secret.pem`); + return true; + } catch { + return false; + } +}; +const push = (r: ReturnType) => + checkedPush({ cwd: r.work, remote: r.remote, base: r.base, head: r.head, ref: 'refs/heads/task', limits: LIMITS, guard, force: true }); + +/** Runs `fn` with node:child_process's spawnSync wrapped: `before(args)` runs before each call it is handed. */ +function withSpawnHook(before: (args: string[]) => void, fn: () => T): T { + const cp = createRequire(import.meta.url)('node:child_process') as Record unknown>; + const orig = cp.spawnSync!; + cp.spawnSync = (...a: unknown[]) => { + before(Array.isArray(a[1]) ? a[1].map(String) : []); + return orig(...a); + }; + syncBuiltinESMExports(); + try { + return fn(); + } finally { + cp.spawnSync = orig; + syncBuiltinESMExports(); + } +} + +// ---- A sha1 repo: a 64-hex line is a cut at its first 40 to git. + +for (const [what, at] of [['the drop', 'drop'], ['the head', 'head']] as const) { + test(`a sha1 repo whose shallow line is ${what}'s id plus 24 hex digits is refused, never 0 problems`, () => { + const r = repo(); + writeFileSync(r.shallow, `${r[at]}${'0'.repeat(24)}\n`); + assert.notEqual(plainCount(r), '3', 'git reads the line as a cut'); + const p = pushProblems(r.work, r.base, r.head, LIMITS); + assert.equal(p.length, 1, p.join('\n')); + assert.match(p[0]!, CUT); + assert.match(p[0]!, /\(sha1\)/); + }); +} + +test('the same 64-hex line, removed right before the push\'s spawn, lands nothing on the remote', () => { + const r = repo(); + writeFileSync(r.shallow, `${r.drop}${'0'.repeat(24)}\n`); + const res = withSpawnHook((args) => { + if (args.includes('push')) rmSync(r.shallow, { force: true }); + }, () => push(r)); + assert.ok(!res.ok && 'refused' in res && CUT.test(res.refused.join('\n')), JSON.stringify(res)); + assert.equal(pushedKey(r), false); +}); + +// ---- A sha256 repo: a 40-hex line is a refusal, not an exception. + +test('a sha256 repo whose shallow line is 40 hex digits gets a refusal from pushProblems and checkedPush, not a throw', () => { + const r = repo('sha256'); + assert.equal(r.drop.length, 64); + writeFileSync(r.shallow, `${r.drop.slice(0, 40)}\n`); + let p: string[] = []; + assert.doesNotThrow(() => { p = pushProblems(r.work, r.base, r.head, LIMITS); }); + assert.equal(p.length, 1, p.join('\n')); + assert.match(p[0]!, CUT); + assert.match(p[0]!, /\(sha256\)/); + let res: ReturnType | undefined; + assert.doesNotThrow(() => { res = push(r); }); + assert.ok(res && !res.ok && 'refused' in res && CUT.test(res.refused.join('\n')), JSON.stringify(res)); + assert.equal(pushedKey(r), false); +}); + +test('a sha256 repo still reads its own 64-hex line: a cut outside the range pushes, one inside is refused', () => { + const r = repo('sha256'); + writeFileSync(r.shallow, `${r.drop}\n`); + const inside = pushProblems(r.work, r.base, r.head, LIMITS); + assert.equal(inside.length, 1, inside.join('\n')); + assert.match(inside[0]!, /a shallow boundary at /); + writeFileSync(r.shallow, `${r.base}\n`); + assert.ok(pushProblems(r.work, r.base, r.head, LIMITS).some((x) => /keys\/secret\.pem/.test(x))); +}); + +// ---- Round 2's 1 KB form: an id outside the range, 983 characters, then an id inside it, on one line. + +test('a shallow line that hides an in-range id 1,023 characters in is refused, never 0 problems', () => { + const r = repo(); + writeFileSync(r.shallow, `${r.base}${'x'.repeat(983)}${r.drop}\n`); + assert.notEqual(plainCount(r), '3', 'git reads the drop as a cut of its own'); + const p = pushProblems(r.work, r.base, r.head, LIMITS); + assert.equal(p.length, 1, p.join('\n')); + assert.match(p[0]!, /history is cut inside the range/); +}); diff --git a/test/push-shallow.test.ts b/test/push-shallow.test.ts new file mode 100644 index 000000000..83caf8e31 --- /dev/null +++ b/test/push-shallow.test.ts @@ -0,0 +1,94 @@ +// A shallow cut inside the pushed range is refused (#807, round 2). The `shallow` file lists commits whose parents git +// treats as missing, and an agent can write it into the shared repo: with the head's parent listed, the walks of +// base..head stopped short and a refused file in an earlier commit read as 0 problems; deleted again before the push, +// the push carried that file to the remote (measured by #927's final check). The limits now refuse a range cut by +// such a file, before and after their walks and once more right before the push. A cut outside the range (a +// depth-limited clone's base) leaves the range whole and pushes as before. +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { execFileSync } from 'node:child_process'; +import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { pathToFileURL } from 'node:url'; +import { checkedPush, DEFAULT_PUSH_LIMITS, pushProblems } from '../src/push-check.js'; + +const git = (cwd: string, ...a: string[]) => execFileSync('git', a, { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); +const LIMITS = { ...DEFAULT_PUSH_LIMITS, refuse_paths: ['**/*.pem'] }; +const guard = { defaultBranch: 'main', blocked: () => undefined }; + +/** base, then a commit adding keys/secret.pem, then one removing it: the net diff is clean, the history isn't. */ +function repo() { + const root = mkdtempSync(join(tmpdir(), 'push-shallow-')); + const work = join(root, 'work'); + const remote = join(root, 'remote.git'); + git(root, 'init', '-q', '--bare', '-b', 'main', remote); + git(root, 'init', '-q', '-b', 'main', work); + for (const [k, v] of [['user.email', 'bot@example.com'], ['user.name', 'bot'], ['commit.gpgsign', 'false']]) git(work, 'config', k!, v!); + writeFileSync(join(work, 'README.md'), 'hello\n'); + git(work, 'add', '-A'); + git(work, 'commit', '-q', '-m', 'base'); + const base = git(work, 'rev-parse', 'HEAD'); + git(work, 'push', '-q', remote, 'main'); + mkdirSync(join(work, 'keys')); + writeFileSync(join(work, 'keys', 'secret.pem'), 'k\n'); + git(work, 'add', '-A'); + git(work, 'commit', '-q', '-m', 'add a key'); + git(work, 'rm', '-q', 'keys/secret.pem'); + git(work, 'commit', '-q', '-m', 'drop it'); + const head = git(work, 'rev-parse', 'HEAD'); + const shallow = join(work, '.git', 'shallow'); + return { work, remote, base, head, shallow }; +} + +test('a .git/shallow written by hand that cuts the range is refused, never 0 problems', () => { + const r = repo(); + assert.equal(pushProblems(r.work, r.base, r.head, LIMITS).length, 1, 'the key is seen in a whole history'); + writeFileSync(r.shallow, `${r.head}\n`); + const p = pushProblems(r.work, r.base, r.head, LIMITS); + assert.equal(p.length, 1, p.join('\n')); + assert.match(p[0]!, /history is cut inside the range \(a shallow boundary at [0-9a-f]{12}\)/); +}); + +test('the delete-between race: the check sees the cut and refuses, so nothing reaches the remote, and deleting it afterwards changes nothing', () => { + const r = repo(); + writeFileSync(r.shallow, `${r.head}\n`); + const res = checkedPush({ cwd: r.work, remote: r.remote, base: r.base, head: r.head, ref: 'refs/heads/task', limits: LIMITS, guard, force: true }); + assert.ok(!res.ok && 'refused' in res, JSON.stringify(res)); + assert.match((res as { refused: string[] }).refused.join('\n'), /history is cut inside the range/); + rmSync(r.shallow); + assert.equal(git(r.remote, 'for-each-ref', 'refs/heads/task'), '', 'nothing pushed'); + // Pushed again with the file gone, the whole history is judged: the key is refused. + const again = checkedPush({ cwd: r.work, remote: r.remote, base: r.base, head: r.head, ref: 'refs/heads/task', limits: LIMITS, guard, force: true }); + assert.ok(!again.ok && 'refused' in again && again.refused.some((x) => /keys\/secret\.pem/.test(x)), JSON.stringify(again)); + assert.equal(git(r.remote, 'for-each-ref', 'refs/heads/task'), ''); +}); + +test('a shallow file git can\'t read as a list (a directory in its place) fails closed', () => { + const r = repo(); + mkdirSync(r.shallow); + writeFileSync(join(r.shallow, 'x'), ''); + const p = pushProblems(r.work, r.base, r.head, LIMITS); + assert.equal(p.length, 1, p.join('\n')); + assert.match(p[0]!, /history is cut inside the range \(can't read its shallow file/); + assert.ok(existsSync(r.shallow)); +}); + +test('a depth-limited clone pushes a change above its cut as before: the cut is its base, outside the range', () => { + const r = repo(); + const clone = join(mkdtempSync(join(tmpdir(), 'push-shallow-clone-')), 'c'); + git(r.work, 'clone', '-q', '--depth', '1', pathToFileURL(r.remote).href, clone); + assert.equal(git(clone, 'rev-parse', '--is-shallow-repository'), 'true'); + for (const [k, v] of [['user.email', 'bot@example.com'], ['user.name', 'bot'], ['commit.gpgsign', 'false']]) git(clone, 'config', k!, v!); + const base = git(clone, 'rev-parse', 'HEAD'); + writeFileSync(join(clone, 'a.txt'), 'a\n'); + git(clone, 'add', '-A'); + git(clone, 'commit', '-q', '-m', 'a change'); + const head = git(clone, 'rev-parse', 'HEAD'); + assert.deepEqual(pushProblems(clone, base, head, LIMITS), []); + // A refused file above the cut is still refused. + writeFileSync(join(clone, 'b.pem'), 'k\n'); + git(clone, 'add', '-A'); + git(clone, 'commit', '-q', '-m', 'a key'); + assert.match(pushProblems(clone, base, git(clone, 'rev-parse', 'HEAD'), LIMITS).join('\n'), /b\.pem/); +}); diff --git a/test/push-snapshot.test.ts b/test/push-snapshot.test.ts new file mode 100644 index 000000000..8b09124c3 --- /dev/null +++ b/test/push-snapshot.test.ts @@ -0,0 +1,190 @@ +// Round 3 of #927: the push limits read one checked view of the history, whatever an agent does to the shared repo +// while they run, and the views git trusts that an agent can write (a shallow file, a commit-graph) can't hide a commit. +// - git reads a shallow file more loosely than an exact match: an uppercase id, text after the id, and a line over +// 1 KB each cut a range (measured on git 2.43 by the round-2 check), and the old check took them as "no cut"; +// - a shallow file toggled every few ms beat the old before-and-after checks (23 of 150 runs; 5 of 80 pushes landed +// the refused file, measured): every walk now reads one gitSnapshot, taken and checked once; +// - a forged commit-graph parent link made the walk skip the commit that added the file, and the push then landed it +// (measured): the commit-graph is off for every git the engine runs, gitleaks' included. +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { execFileSync } from 'node:child_process'; +import { createHash } from 'node:crypto'; +import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { createRequire, syncBuiltinESMExports } from 'node:module'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { checkedPush, DEFAULT_PUSH_LIMITS, pushProblems } from '../src/push-check.js'; +import { findTool } from '../src/tool-path.js'; + +const git = (cwd: string, ...a: string[]) => execFileSync('git', a, { cwd, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'] }).trim(); +const LIMITS = { ...DEFAULT_PUSH_LIMITS, refuse_paths: ['**/*.pem'] }; +const guard = { defaultBranch: 'main', blocked: () => undefined }; +const TOKEN = 'ghp_' + 'a1B2c3D4e5F6g7H8i9J0k1L2m3N4o5P6q7R8'; + +/** base, K (adds keys/secret.pem and a token), D (drops both), H (a note): the net diff is clean, the history isn't. */ +function repo() { + const root = mkdtempSync(join(tmpdir(), 'push-snapshot-')); + const work = join(root, 'work'); + const remote = join(root, 'remote.git'); + git(root, 'init', '-q', '--bare', '-b', 'main', remote); + git(root, 'init', '-q', '-b', 'main', work); + for (const [k, v] of [['user.email', 'bot@example.com'], ['user.name', 'bot'], ['commit.gpgsign', 'false']]) git(work, 'config', k!, v!); + writeFileSync(join(work, 'README.md'), 'hello\n'); + git(work, 'add', '-A'); + git(work, 'commit', '-q', '-m', 'base'); + const base = git(work, 'rev-parse', 'HEAD'); + git(work, 'push', '-q', remote, 'main'); + mkdirSync(join(work, 'keys')); + writeFileSync(join(work, 'keys', 'secret.pem'), 'k\n'); + writeFileSync(join(work, 'c.js'), `export const token = '${TOKEN}';\n`); + git(work, 'add', '-A'); + git(work, 'commit', '-q', '-m', 'add a key'); + git(work, 'rm', '-q', 'keys/secret.pem', 'c.js'); + git(work, 'commit', '-q', '-m', 'drop it'); + const drop = git(work, 'rev-parse', 'HEAD'); + writeFileSync(join(work, 'notes.txt'), 'n\n'); + git(work, 'add', '-A'); + git(work, 'commit', '-q', '-m', 'notes'); + const head = git(work, 'rev-parse', 'HEAD'); + return { work, remote, base, drop, head, shallow: join(work, '.git', 'shallow') }; +} + +const keyFound = (p: string[]) => p.some((x) => /keys\/secret\.pem/.test(x)); +const pushedKey = (remote: string, head: string) => { + try { + git(remote, 'cat-file', '-e', `${head}~2:keys/secret.pem`); + return true; + } catch { + return false; + } +}; + +// ---- The shallow file, read at least as loosely as git reads it. + +for (const [what, line] of [ + ['an uppercase id', (id: string) => id.toUpperCase()], + ['text right after the id', (id: string) => `${id}zz`], + ['a word after the id', (id: string) => `${id} comment`], + // A line git itself rejects, so round 2 refused it too (only the message differed). Round 2's 1 KB form, which git + // reads as a cut and round 2 took as no cut, is in test/push-shallow-format.test.ts. + ['a line over 1 KB', (id: string) => `${id}${' '.repeat(1100)}x`], +] as const) { + test(`a shallow file line with ${what} is refused, never read as "no cut"`, () => { + const r = repo(); + writeFileSync(r.shallow, `${line(r.drop)}\n`); + const p = pushProblems(r.work, r.base, r.head, LIMITS); + assert.equal(p.length, 1, p.join('\n')); + assert.match(p[0]!, /history is cut inside the range \(its shallow file has a line that isn't exactly a full lowercase object id/); + }); +} + +// ---- One snapshot for every walk: a shallow file written mid-check changes nothing. + +/** + * Runs `fn` with node:child_process's execFileSync and spawnSync wrapped: `during(args)` runs before each call either + * is handed. + */ +async function withExecHook(during: (args: string[]) => (() => void) | void, fn: () => T): Promise { + const cp = createRequire(import.meta.url)('node:child_process') as Record unknown>; + const orig = { execFileSync: cp.execFileSync!, spawnSync: cp.spawnSync! }; + for (const name of ['execFileSync', 'spawnSync'] as const) { + cp[name] = (...a: unknown[]) => { + const after = during(Array.isArray(a[1]) ? a[1].map(String) : []); + try { + return orig[name](...a); + } finally { + after?.(); + } + }; + } + syncBuiltinESMExports(); + try { + return fn(); + } finally { + Object.assign(cp, orig); + syncBuiltinESMExports(); + } +} + +/** + * A cut at D written into the repo while each walk of the range runs (`git log --name-only` before #928; its + * `rev-list` and `diff-tree --stdin` after it; `rev-list --objects`), and removed again as soon as it ends. `fired` + * counts the walks it hit, so the test can't pass with the hook never reaching a walk. + */ +const toggleDuringNameWalk = (r: ReturnType) => { + const hook = (args: string[]) => { + if (!(args.includes('rev-list') || args.includes('diff-tree') || (args.includes('log') && args.includes('--name-only')))) return; + hook.fired++; + writeFileSync(r.shallow, `${r.drop}\n`); + return () => rmSync(r.shallow, { force: true }); + }; + hook.fired = 0; + return hook; +}; + +test('a shallow file written while the walks run, and removed before they end, can\'t hide the key from pushProblems', async () => { + const r = repo(); + const hook = toggleDuringNameWalk(r); + const p = await withExecHook(hook, () => pushProblems(r.work, r.base, r.head, LIMITS)); + assert.ok(hook.fired > 0, 'the hook reached the name walk'); + assert.ok(keyFound(p), p.join('\n')); + assert.ok(!existsSync(r.shallow)); +}); + +test('the same toggle can\'t get checkedPush to land the key on the remote', async () => { + const r = repo(); + const hook = toggleDuringNameWalk(r); + const res = await withExecHook(hook, () => checkedPush({ cwd: r.work, remote: r.remote, base: r.base, head: r.head, ref: 'refs/heads/task', limits: LIMITS, guard, force: true })); + assert.ok(hook.fired > 0, 'the hook reached the name walk'); + assert.ok(!res.ok && 'refused' in res && keyFound(res.refused), JSON.stringify(res)); + assert.equal(pushedKey(r.remote, r.head), false); +}); + +// ---- A forged commit-graph. + +/** Writes the repo's commit-graph, then makes it say D's first parent is the base: K, which added the key, is skipped. */ +function forgeGraph(r: ReturnType) { + git(r.work, 'commit-graph', 'write', '--reachable', '--no-progress'); + const file = join(r.work, '.git', 'objects', 'info', 'commit-graph'); + const buf = Buffer.from(readFileSync(file)); + const chunks: Record = {}; + for (let i = 0; i < buf[6]!; i++) { + const o = 8 + i * 12; + chunks[buf.toString('latin1', o, o + 4)] = Number(buf.readBigUInt64BE(o + 4)); + } + const n = buf.readUInt32BE(chunks.OIDF! + 255 * 4); + const idx = (hex: string) => [...Array(n).keys()].find((i) => buf.toString('hex', chunks.OIDL! + i * 20, chunks.OIDL! + i * 20 + 20) === hex)!; + buf.writeUInt32BE(idx(r.base), chunks.CDAT! + idx(r.drop) * 36 + 20); + createHash('sha1').update(buf.subarray(0, buf.length - 20)).digest().copy(buf, buf.length - 20); + chmodSync(file, 0o644); + writeFileSync(file, buf); + // Plain git now believes it. + assert.equal(git(r.work, 'rev-list', `${r.base}..${r.head}`).split('\n').length, 2, 'the forged graph hides K from plain git'); +} + +test('a forged commit-graph parent link can\'t hide the key from pushProblems or let checkedPush land it', () => { + const r = repo(); + forgeGraph(r); + assert.ok(keyFound(pushProblems(r.work, r.base, r.head, LIMITS))); + const res = checkedPush({ cwd: r.work, remote: r.remote, base: r.base, head: r.head, ref: 'refs/heads/task', limits: LIMITS, guard, force: true }); + assert.ok(!res.ok && 'refused' in res && keyFound(res.refused), JSON.stringify(res)); + assert.equal(pushedKey(r.remote, r.head), false); +}); + +test('gitleaks, which runs git itself, reads past a forged commit-graph too', { skip: !findTool('gitleaks') && 'gitleaks not installed' }, async () => { + const { scanRange } = await import('../src/scan/secrets.js'); + const r = repo(); + assert.equal((await scanRange(r.work, `${r.base}..${r.head}`)).status, 'leaks'); + forgeGraph(r); + assert.equal((await scanRange(r.work, `${r.base}..${r.head}`)).status, 'leaks'); +}); + +test('gitleaks under a shallow cut inside the range is never "clean"; a ref name in the range still works', { skip: !findTool('gitleaks') && 'gitleaks not installed' }, async () => { + const { scanRange } = await import('../src/scan/secrets.js'); + const r = repo(); + assert.equal((await scanRange(r.work, `${r.base}..HEAD`)).status, 'leaks'); + writeFileSync(r.shallow, `${r.drop}\n`); + const cut = await scanRange(r.work, `${r.base}..${r.head}`); + assert.equal(cut.status, 'unavailable', JSON.stringify(cut)); +}); diff --git a/test/review.test.ts b/test/review.test.ts index a4121ecfe..c9c9203fb 100644 --- a/test/review.test.ts +++ b/test/review.test.ts @@ -174,7 +174,7 @@ const GUARD_TREE = [ 'src/events/types.ts', 'src/experiments/paths.ts', 'src/governor-holds.ts', 'src/guard/chat.ts', 'src/guard/commit.ts', 'src/guard/intake.ts', 'src/guard/research.ts', 'src/guardrails/access.ts', 'src/guardrails/context.ts', 'src/guardrails/engine.ts', 'src/guardrails/fingerprint.ts', 'src/guardrails/glob.ts', 'src/guardrails/push-hook.ts', 'src/guardrails/reach.ts', 'src/guardrails/shell.ts', 'src/held-out-paths.ts', 'src/hook-imports.ts', - 'src/hook-main.ts', 'src/hook.ts', 'src/model-routing.ts', 'src/network.ts', 'src/optimize.ts', 'src/os/child-process.ts', 'src/os/index.ts', + 'src/hook-main.ts', 'src/hook.ts', 'src/model-routing.ts', 'src/network.ts', 'src/optimize.ts', 'src/os/child-process.ts', 'src/os/git.ts', 'src/os/index.ts', 'src/os/own-env.ts', 'src/os/process-links.ts', 'src/os/system-paths.ts', 'src/os/system-root-guard.ts', 'src/path-text.ts', 'src/preview/config.ts', 'src/project-env.ts', 'src/provider-hosts.ts', 'src/providers.ts', 'src/quiet-sqlite.ts', 'src/review.ts', 'src/roles.ts', 'src/routing.ts', 'src/scan/secrets.ts', 'src/slot-file.ts', 'src/stage-names.ts', 'src/testrun/affected.ts', 'src/testrun/config.ts', diff --git a/test/testrun-measure.test.ts b/test/testrun-measure.test.ts index 3f1f7bf78..c9fb64d5d 100644 --- a/test/testrun-measure.test.ts +++ b/test/testrun-measure.test.ts @@ -269,7 +269,7 @@ test('no check, no PR: a measure records only testrun.measure, so the PR runs, t const names = [...reach].map((f) => f.slice(join(repoRoot, 'src').length + 1)); for (const bad of ['testrun/checks.ts', 'testrun/index.ts', 'testrun/setup.ts', 'github-app.ts', 'coordinator/test-runs.ts', 'coordinator/pr-batch.ts']) assert.ok(!names.includes(bad), `measure.ts reaches ${bad}`); const cli = readFileSync(join(repoRoot, 'src', 'testrun', 'measure-cli.ts'), 'utf8'); - assert.deepEqual([...cli.matchAll(/^import [^'"]*['"]([^'"]+)['"]/gm)].map((m) => m[1]).sort(), ['../brand.js', '../events/log.js', '../instance.js', '../os/child-process.js', './host.js', './measure.js', './workspace.js', 'node:path']); + assert.deepEqual([...cli.matchAll(/^import [^'"]*['"]([^'"]+)['"]/gm)].map((m) => m[1]).sort(), ['../brand.js', '../events/log.js', '../instance.js', '../os/git.js', './host.js', './measure.js', './workspace.js', 'node:path']); }); test('the helper sees exactly what a PR run of the tier sends it: the same calls, command and env, the measured value aside', { skip: !hasPerl && 'perl for the flock holder' }, async () => {