DO NOT MERGE: combined CI for batch 17 (#927)

.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

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".

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,

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();
 

scripts/check-dco.mjs

@@ -2,7 +2,7 @@
 // Fail if any commit in <base>..<head> lacks a Signed-off-by trailer that
 // matches its author (Developer Certificate of Origin, see CONTRIBUTING.md).
 //   node scripts/check-dco.mjs <base> [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' },
 );

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));
 

scripts/ci/affected.mjs

@@ -8,7 +8,7 @@
 //   node scripts/ci/affected.mjs --event pull_request --base <rev>   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);

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) {

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;

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'));

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) });

scripts/ci/wizard-e2e.mjs

@@ -17,10 +17,10 @@
 // browser asked for) and <out dir>/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'). */