diff --git a/BUILD-LOG.md b/BUILD-LOG.md index bec7c4a45..8f2a898dd 100644 --- a/BUILD-LOG.md +++ b/BUILD-LOG.md @@ -5879,3 +5879,58 @@ Q284 / #470 source-only security fix 2026-10-29T03:33:00.640624+00:00: pin worke 2026-11-02 | Lead sub-agent (Claude Opus 5.5, under T3 c5c1e8ba) | Row 227 R2 (#1728): test-case2.sh now runs standalone. Removed every tie to the R1 slot controller (ROW227_CONTROL_ROOT, nonce and permits, priority.mjs, owned bwrap wrapper, controller cleanup marker); leak check reads fixture-credentials.mjs; driver under env -i with TMPDIR in the sandbox; cleanup bounded to 3 s then SIGKILL. R1 red 0.294 s confirmed; all 17 steps passed with no step fix; two green runs 4.96 s and 5.74 s; mutants m1-m5 red at S2b, S3c, S4, S5b, S6, m6 (requireCitation) survived and is a follow-up; tip check on fddae79ef green 5.05 s, no conflicts. Code 04e1ce24c; record in agents/steelbeak/work/queue-227/BUILD.md "R2". 2026-11-02 | Lead sub-agent (Claude Opus 5.5, under T3 c5c1e8ba) | Row 227 R3 (#1728): test-case2.sh sandbox guard. A failed mktemp, mkdir or output redirect now exits 2 before any path, trap or process; the root must be a new empty directory of this user, kept as its physical path with its device:inode, and sweep_sandbox and remove_sandbox in scripts/case2/sandbox.sh refuse on their own unless it still matches. The kill sweep compares cwd targets as strings (root or root plus "/"), so *, ? and [ are literal, with one find over 1031 links in 56 ms. A TMPDIR whose tools.sock path would pass 107 bytes is refused at entry (TMPDIR plus 64, so at most 43 bytes). R2 red shown on dummy links only; failure controls green inside a fresh PID namespace; bracket control killed 3 sandbox sentinels and spared 4 lookalikes; plain runs 5.64 s and 5.20 s; TERM and INT closed in 130 to 1247 ms, a held driver in 3.07 to 3.10 s; tip check on 5df0e9d6f green 5.18 s, no conflicts. Code abc66a598; record in agents/steelbeak/work/queue-227/BUILD.md "R3: the sandbox guard". + +## 2026-11-02, Vikunja #780 and #781: two gate flakes made deterministic + +Before: project read.test.mjs "path history is bounded at 100" failed with a raw ENOENT on .git/objects/pack/tmp_idx_*, +and webui preview.test.mjs:209 got 200 where it expects 502. Code 7e0d3c93f (#780), 1009bbfaf (#781), not pushed. + +Causes, each confirmed +- #780: the fixture git helpers ran a bare git commit. Git 2.55 then starts a detached "git maintenance run --auto" + that repacks the fixture repository and creates and removes tmp_idx_* and pack temp files while inspectGit walks + .git (trace2 capture shows the spawn). inspectGit lists a name, the file is gone at lstat, raw ENOENT escapes. + SAFE already carried maintenance.auto=false and gc.auto=0 for product git calls, so only the test helpers and + inspectGit's own walk were exposed. +- #781: leasePort ignored {closed:true}. Without a lane it probed an ephemeral port and closed it, so nothing held it + and a parallel test file could bind it and answer. With a lane the per-port lock was held but no socket was. + +Design +- #780: the project, bus spec-fixtures and webui tester-fixture git helpers pass -c maintenance.auto=false + -c gc.auto=0. inspectGit maps ENOENT from a listed entry's lstat or a directory readdir to ProjectError + project-repository "git metadata changed during inspection": it refuses, never skips or retries, because the + metadata did not hold still. A missing .git keeps its own error. The pin test replaces fs.readdirSync + (syncBuiltinESMExports) and removes the named entry inside the call, file and directory. No production hook. +- #781: leaseClosedPort holds a bound, non-listening socket in a perl child until the test's after hook. Connects get + ECONNREFUSED, other binds get EADDRINUSE, the kernel's bind(0) skips it (checked over 200 allocations). Lane mode keeps + its band and per-port flock and releases the socket before the lock. The child exits on stdin EOF, so an abrupt death + releases it. leasePort now refuses a closed option, so a stale call cannot get the unheld port again. + +Choices and tradeoffs +- Rejected a listener that resets on accept: the client sees ECONNRESET or UND_ERR_SOCKET depending on timing, and + service.test.mjs:473 asserts "unreachable (ECONNREFUSED)". The bound socket keeps every assertion unchanged. +- preview, webui serve and service (4 sites) take leaseClosedPort(t), which needs perl and fails closed naming it without. + stack-release freePort, control-board getFreePort and tester-cli freePort listen on the port themselves, keep leasePort + and drop the ignored option. Those keep a probe-then-bind window; closing it needs the caller to bind, out of scope. +- The first mutant run hung: my new tests did not release holders on a failed assertion. They now drain in t.after. + +After (pass/fail/skip): project 42/0/0 1.2 s; webui 496/0/0 69.5 s; control-board 130/0/0 0.8 s; stack-release.test +225/0/0 628 s; gate-lane-fixtures 13/0/0; gate-lane 13/0/0; bus and harness spec-fixtures importers 100/0/0 15 s; library +sync, mirror, case2-install 69/0/0; read.test.mjs 10 of 10 green. No hash or size pins on any changed file. Mutants, each +restored by git checkout with a clean status: closed lease probes and closes again, and holder released immediately, each +killed by three gate-lane-fixtures tests; leasePort accepts closed again, killed by its refusal test; inspectGit mapping +removed, killed by the read.test.mjs vanished-entry test. + +## 2026-11-03, Vikunja #780 R2: fresh recursive lstat restored; corrections to the R1 entry above + +- The R1 walk change was broader than the ENOENT mapping. Passing the parent's stat into the recursive walk dropped the + fresh lstat at recursive entry: a directory replaced by a symlink after its listed stat passed the stale check and + readdirSync read through the link; one replaced by a regular file leaked a raw ENOTDIR (GPT review P2). +- The tester-cli freePort helper (packages/webui/tests/tester-cli.test.mjs:21) has no callers and binds nothing; the + active tester CLI test uses a private Unix socket. R1 wrongly counted it among the callers that listen themselves. +- stack-release.test.mjs:2051 and control-board serve.test.mjs:78 still probe, close, then bind: a known risk, and + the lead files a follow-up. +R2, code 1977f4ffc, not pushed: walk(directory, root) stats with root ? lstatSync : statEntry and recurses walk(path), +the reviewer's control shape; .git keeps its raw lstat, the mapper stays ENOENT only. Two read.test.mjs tests swap a +.git/objects directory, inside its listed lstat after the real stat, for a symlink to a fixture-only outside directory +or a file; both refuse "expected local .git directory", and were red on 9944100ad (symlink resolved, raw ENOTDIR). +Mutant walk(path, entry) using the passed stat: killed by both. project 44/0/0; read.test.mjs alone 11/0/0 5 of 5. diff --git a/docs/SESSIONS.md b/docs/SESSIONS.md index d4dfbc6e8..ffe138351 100644 --- a/docs/SESSIONS.md +++ b/docs/SESSIONS.md @@ -1957,3 +1957,7 @@ are never rewritten or removed; corrections are new entries. 2026-11-02 | Lead sub-agent (Claude Opus 5.5, under T3 c5c1e8ba) | Row 227 R3 (#1728): sandbox guard, literal cwd sweep and TMPDIR limit for test-case2.sh | Code abc66a598 on t3/row227-r3-fix: P1 to P3 fixed, 40 no-signal checks pass, namespace failure controls exit 2 with nothing started, suite green twice (5.64 s, 5.20 s), tip check green. Not pushed; record in queue-227 BUILD.md "R3: the sandbox guard". 2026-11-02 | Lead (Claude Opus 5.5, T3 c5c1e8ba) | PR 1838 (#725) landing: disk-space governance PRD as version 1.19 | Merge 31222d81d of the Haiku rebase 3d00efe39 onto ea02c061e; GPT review APPROVE, P3s fixed here: approved wording in the tasks requirements test header, location correction in REBASE-1.19.md. Full gate before push. + +2026-11-02 | Lead sub-agent (Claude Sonnet 5.5, under T3 c5c1e8ba) | Vikunja #780 and #781: project inspectGit race with git maintenance, closed fixture ports | Code 7e0d3c93f (#780) and 1009bbfaf (#781) on t3/fix-flakes-780-781: both flakes made deterministic, pin tests added, three mutants plus one extra each killed and restored, project, webui, control-board, stack-release and lane fixture suites green, read.test.mjs 10 of 10. Not pushed; record in BUILD-LOG.md. + +2026-11-03 | Lead sub-agent (Claude Opus 5.5, under T3 c5c1e8ba) | Vikunja #780 R2: restore the fresh recursive lstat in project inspectGit | Code 1977f4ffc on t3/flakes-780-781-r2-fix: symlink and file replacement tests red on 9944100ad and green now, stale-stat mutant killed by both, project 44/0/0, read.test.mjs 5 of 5. BUILD-LOG entry corrects the R1 walk and tester-cli descriptions. Not pushed. diff --git a/packages/bus/tests/spec-fixtures.mjs b/packages/bus/tests/spec-fixtures.mjs index 60dec8e69..7114df7d9 100644 --- a/packages/bus/tests/spec-fixtures.mjs +++ b/packages/bus/tests/spec-fixtures.mjs @@ -5,7 +5,7 @@ import { execFileSync } from 'node:child_process'; import { Store } from '../src/store.mjs'; import { Broker } from '../src/broker.mjs'; import { initProject, readSpec, writeRecord, listProjectFiles, readProjectFile, writeHumanFile, ProjectError } from '../../project/src/index.mjs'; -export const git = (repo, ...args) => execFileSync('git', ['-c', 'core.hooksPath=/dev/null', '-C', repo, ...args], { encoding: 'utf8' }).trim(); +export const git = (repo, ...args) => execFileSync('git', ['-c', 'core.hooksPath=/dev/null', '-c', 'maintenance.auto=false', '-c', 'gc.auto=0', '-C', repo, ...args], { encoding: 'utf8' }).trim(); const options = [{ key: 'yes', text: 'Yes' }, { key: 'no', text: 'No' }]; export async function fixture(t, { adapter = true, roles = {}, arbiters = { technical: 'reviewer', delivery: 'reviewer' }, access = null } = {}) { const root = mkdtempSync(join(tmpdir(), 'bus-spec-')); diff --git a/packages/cli/tests/gate-lane-fixtures.test.mjs b/packages/cli/tests/gate-lane-fixtures.test.mjs index 83e61c1de..e256f20b8 100644 --- a/packages/cli/tests/gate-lane-fixtures.test.mjs +++ b/packages/cli/tests/gate-lane-fixtures.test.mjs @@ -1,5 +1,5 @@ import test from 'node:test';import assert from 'node:assert/strict'; -import { mkdtempSync,rmSync,mkdirSync,writeFileSync } from 'node:fs';import { join } from 'node:path';import { createServer } from 'node:net';import { once } from 'node:events'; +import { mkdtempSync,rmSync,mkdirSync,writeFileSync } from 'node:fs';import { join } from 'node:path';import { createServer,connect } from 'node:net';import { once } from 'node:events'; const api=await import('../../../scripts/testing/lane-fixtures.mjs').catch(e=>{if(e.code!=='ERR_MODULE_NOT_FOUND')throw e;return {};}); const evidence=await import('../../../scripts/testing/lane-evidence.mjs').catch(e=>{if(e.code!=='ERR_MODULE_NOT_FOUND')throw e;return {};}); test('parallel lease holders use only the active lane and its shared state',async t=>{ @@ -92,3 +92,52 @@ test('an unrelated session can append while a supervised step runs without becom const changed=evidence.homeMetadata(home).filter(e=>prior.get(e.path)!==JSON.stringify(e));assert.ok(changed.some(e=>e.path===file)); const attributed=evidence.attributeEntries(changed,{nonce:'abcdef123456',tempRoot:join(root,'temp'),fixturePaths:[]});assert.equal(attributed.owned.length,0);assert.ok(attributed.unrelated.length>0); }); +// Vikunja #781: a port that nothing answers on is held for the test's life, so a parallel file +// cannot bind it and answer a request the test expects to fail. +const connectCode=port=>new Promise(resolve=>{const c=connect(port,'127.0.0.1');c.once('error',e=>resolve(e.code));c.once('connect',()=>{c.destroy();resolve('connected');});}); +const bindCode=(port,host='127.0.0.1')=>new Promise(resolve=>{const s=createServer();s.once('error',e=>resolve(e.code));s.listen(port,host,()=>s.close(()=>resolve('bound')));}); +for(const mode of ['lane','no lane'])test(`a closed port lease refuses connects and stays held against binds, other leases and the kernel until release (${mode})`,async t=>{ + const ctx=leaseContext(t),context=mode==='lane'?ctx:{lane:''},callbacks=[],fixture={after:f=>callbacks.push(f)}; + const release=async()=>{for(const f of callbacks.splice(0).reverse())await f();};t.after(release); + const port=await api.leaseClosedPort(fixture,context),other=await api.leaseClosedPort(fixture,context),listener=await api.leasePort(fixture,context); + const lock=n=>join(ctx.stateRoot,'ports',ctx.lane,`${n}.lock`); + assert.equal(new Set([port,other,listener]).size,3); + if(mode==='lane'){const [lo,hi]=BANDS[ctx.lane];for(const n of [port,other,listener]){assert.ok(n>=lo&&n<=hi);assert.equal(spawnSync('flock',['-n',lock(n),'true']).status,1);}} + assert.equal(await connectCode(port),'ECONNREFUSED'); + assert.equal(await bindCode(port),'EADDRINUSE');assert.equal(await bindCode(port,'0.0.0.0'),'EADDRINUSE'); + if(mode==='no lane'){ + const seen=new Set(); + for(let i=0;i<200;i++){const s=createServer();await new Promise(r=>s.listen(0,'127.0.0.1',r));seen.add(s.address().port);await new Promise(r=>s.close(r));} + assert.equal(seen.has(port),false);assert.equal(seen.has(other),false); + } + await release(); + assert.equal(await bindCode(port),'bound');assert.equal(await connectCode(port),'ECONNREFUSED'); + if(mode==='lane')for(const n of [port,other,listener])assert.equal(spawnSync('flock',['-n',lock(n),'true']).status,0); +}); +test('a closed port lease skips a locked port and a foreign listener, and refuses an exhausted band',async t=>{ + const ctx=leaseContext(t),callbacks=[],original=BANDS[ctx.lane]; + const port=await api.leasePort({after:f=>callbacks.push(f)},ctx),server=createServer(); + try{ + BANDS[ctx.lane]=[port,port]; + await assert.rejects(api.leaseClosedPort(t,ctx),/exhausted/); + await new Promise((resolve,reject)=>{server.once('error',reject);server.listen(port,'127.0.0.1',resolve);}); + await callbacks.pop()(); + await assert.rejects(api.leaseClosedPort(t,ctx),/exhausted/);assert.equal(server.listening,true); + await new Promise(r=>server.close(r)); + const got=await api.leaseClosedPort({after:f=>callbacks.push(f)},ctx);assert.equal(got,port); + assert.equal(await connectCode(port),'ECONNREFUSED'); + }finally{BANDS[ctx.lane]=original;if(server.listening)await new Promise(r=>server.close(r));for(const f of callbacks.splice(0).reverse())await f();} +}); +test('leasePort refuses the closed option it used to ignore',async()=>{ + await assert.rejects(api.leasePort({after(){}},{closed:true}),/leaseClosedPort/); +}); +test('abrupt owner death releases a closed port holder',async t=>{ + const ctx=leaseContext(t),modulePath=new URL('../../../scripts/testing/lane-fixtures.mjs',import.meta.url).href; + const code=`const {leaseClosedPort}=await import(${JSON.stringify(modulePath)});const port=await leaseClosedPort({after:()=>{}},JSON.parse(process.argv[1]));console.log(port);setInterval(()=>{},1000);`; + const child=spawn(process.execPath,['--input-type=module','-e',code,JSON.stringify(ctx)],{stdio:['ignore','pipe','pipe']});const done=new Promise(r=>child.once('exit',r));ctx.cleanup(async()=>{child.kill('SIGKILL');await done;}); + const port=await new Promise(r=>child.stdout.once('data',b=>r(Number(String(b).trim())))); + assert.equal(await bindCode(port),'EADDRINUSE'); + child.kill('SIGKILL');await done; + let code2;for(let i=0;i<100&&code2!=='bound';i++){code2=await bindCode(port);if(code2!=='bound')await delay(20);} + assert.equal(code2,'bound'); +}); diff --git a/packages/cli/tests/stack-release.test.mjs b/packages/cli/tests/stack-release.test.mjs index 370fbe6da..38a93b125 100644 --- a/packages/cli/tests/stack-release.test.mjs +++ b/packages/cli/tests/stack-release.test.mjs @@ -2048,7 +2048,7 @@ for(const [id,v,froms] of [[B,'3g',['3f']],[C,'3h',['3f','3g']],[A,'3g',['3f']], } const TESTER='7499'; -const freePort=async()=>String(await leasePort({after:laneAfter},{closed:true})); +const freePort=async()=>String(await leasePort({after:laneAfter})); const rh=(e,...p)=>join(e.env.HOME,...p); const rfault=(e,name)=>{mkdirSync(rh(e,'faults'),{recursive:true});writeFileSync(rh(e,'faults',name),'')}; diff --git a/packages/control-board/tests/serve.test.mjs b/packages/control-board/tests/serve.test.mjs index b69911eed..42737197f 100644 --- a/packages/control-board/tests/serve.test.mjs +++ b/packages/control-board/tests/serve.test.mjs @@ -75,7 +75,7 @@ function closeServer(server) { // Grab an ephemeral free port, then hand it back immediately so a caller can // try to bind it themselves (used to prove startServer never opened a socket). -function getFreePort() { return leasePort({after:laneAfter}, {closed:true}); } +function getFreePort() { return leasePort({after:laneAfter}); } function runCli(args) { return spawnSync(process.execPath, [cli, ...args], { encoding: "utf8", timeout: 15000 }); diff --git a/packages/project/src/git.mjs b/packages/project/src/git.mjs index 0414230f8..a1dca1faf 100644 --- a/packages/project/src/git.mjs +++ b/packages/project/src/git.mjs @@ -17,17 +17,27 @@ export async function git(repo, args, identity = {}) { } export async function inspectGit(repo) { // Refuse gitdir indirection, alternates, links or hardlinks before git reads metadata. - const walk = directory => { - const st = lstatSync(directory); + // Everything below .git was named by a readdir, so a name that is gone when it is read was + // removed by a concurrent git process (for example a pack temp file). That refuses; it is + // never skipped or retried, because the metadata did not hold still while it was checked. + const changed = e => e.code === 'ENOENT' + ? new ProjectError('project-repository', 'git metadata changed during inspection') : e; + const statEntry = path => { try { return lstatSync(path); } catch (e) { throw changed(e); } }; + const namesIn = directory => { try { return readdirSync(directory); } catch (e) { throw changed(e); } }; + // Every directory takes a fresh lstat when the walk enters it, so one replaced by a link or a + // file after its listed stat still refuses. The root is not a listed name: a missing .git + // keeps its own error, as before. + const walk = (directory, root = false) => { + const st = root ? lstatSync(directory) : statEntry(directory); if (!st.isDirectory() || st.isSymbolicLink()) throw new ProjectError('project-repository', 'expected local .git directory'); - for (const name of readdirSync(directory)) { - const path = join(directory, name), entry = lstatSync(path); + for (const name of namesIn(directory)) { + const path = join(directory, name), entry = statEntry(path); if (entry.isDirectory()) walk(path); else if (!entry.isFile() || entry.nlink !== 1) throw new ProjectError('project-repository', 'git metadata must contain regular private files'); } }; - walk(join(repo, '.git')); + walk(join(repo, '.git'), true); for (const name of ['commondir', 'objects/info/alternates', 'objects/info/http-alternates']) { try { lstatSync(join(repo, '.git', name)); } catch (e) { if (e.code === 'ENOENT') continue; throw e; } diff --git a/packages/project/tests/human-file.test.mjs b/packages/project/tests/human-file.test.mjs index 7c862c31d..87fc6eaea 100644 --- a/packages/project/tests/human-file.test.mjs +++ b/packages/project/tests/human-file.test.mjs @@ -6,7 +6,7 @@ import { join } from 'node:path'; import { execFileSync } from 'node:child_process'; import * as api from '../src/index.mjs'; // Row 148 (#1642): the human's file list and save. The agent writer's own tests stay in project.test.mjs. -const git = (repo, ...args) => execFileSync('git', ['-c', 'core.hooksPath=/dev/null', '-C', repo, ...args], { encoding: 'utf8' }).trim(); +const git = (repo, ...args) => execFileSync('git', ['-c', 'core.hooksPath=/dev/null', '-c', 'maintenance.auto=false', '-c', 'gc.auto=0', '-C', repo, ...args], { encoding: 'utf8' }).trim(); async function fixture(t) { const root = mkdtempSync(join(tmpdir(), 'project-human-file-')); t.after(() => rmSync(root, { recursive: true, force: true })); diff --git a/packages/project/tests/project.test.mjs b/packages/project/tests/project.test.mjs index 52279b4bb..789934f85 100644 --- a/packages/project/tests/project.test.mjs +++ b/packages/project/tests/project.test.mjs @@ -8,7 +8,7 @@ const api = await import('../src/index.mjs').catch(e => { if (e.code === 'ERR_MODULE_NOT_FOUND') return {}; throw e; }); -const git = (repo, ...args) => execFileSync('git', ['-c', 'core.hooksPath=/dev/null', '-C', repo, ...args], { encoding: 'utf8' }).trim(); +const git = (repo, ...args) => execFileSync('git', ['-c', 'core.hooksPath=/dev/null', '-c', 'maintenance.auto=false', '-c', 'gc.auto=0', '-C', repo, ...args], { encoding: 'utf8' }).trim(); const actor = { definition: 'coder', instance: 'builder' }; const requirement = { recordVersion: 1, id: 'REQ-PROJECT-1', title: 'A requirement', text: 'Keep local evidence', status: 'draft' }; const decision = { recordVersion: 1, id: 'DEC-1', question: 'Which path?', options: [{ key: 'yes', text: 'Yes' }, { key: 'no', text: 'No' }], recommendation: 'yes', status: 'open', choice: null }; diff --git a/packages/project/tests/read.test.mjs b/packages/project/tests/read.test.mjs index 274f1a090..5066aba6a 100644 --- a/packages/project/tests/read.test.mjs +++ b/packages/project/tests/read.test.mjs @@ -1,11 +1,12 @@ import test from 'node:test'; import assert from 'node:assert/strict'; -import { mkdtempSync, rmSync, writeFileSync, readFileSync, symlinkSync } from 'node:fs'; +import fs, { mkdirSync, mkdtempSync, rmSync, writeFileSync, readFileSync, symlinkSync } from 'node:fs'; +import { syncBuiltinESMExports } from 'node:module'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { execFileSync } from 'node:child_process'; import * as api from '../src/index.mjs'; -const git = (repo, ...args) => execFileSync('git', ['-c', 'core.hooksPath=/dev/null', '-C', repo, ...args], { encoding: 'utf8' }).trim(); +const git = (repo, ...args) => execFileSync('git', ['-c', 'core.hooksPath=/dev/null', '-c', 'maintenance.auto=false', '-c', 'gc.auto=0', '-C', repo, ...args], { encoding: 'utf8' }).trim(); async function fixture(t) { const root = mkdtempSync(join(tmpdir(), 'project-spec-read-')); t.after(() => rmSync(root, { recursive: true, force: true })); @@ -100,6 +101,78 @@ test('path history is bounded at 100 and explicitly marks truncation', async t = assert.equal(result.log[0].commit, result.commit); }); +// Row 780: git can write and rename pack temp files while the metadata walk is running. The walk +// calls readdirSync and lstatSync through the builtin's live bindings, so the test replaces +// fs.readdirSync for one path and removes the named entry itself. The product then sees exactly +// what a concurrent git process produces, with no hook in production code. +test('a git metadata entry that vanishes during inspection refuses as project-repository, never raw ENOENT', async t => { + const { context, repo, commit } = await fixture(t); + const pack = join(repo, '.git/objects/pack'), temp = join(pack, 'tmp_idx_vanishes'); + const directory = join(repo, '.git/objects/tmp_dir_vanishes'); + writeFileSync(temp, 'x'); mkdirSync(directory); + assert.equal((await api.readSpec(context)).commit, commit, 'control: the extra entries alone do not refuse'); + const real = fs.readdirSync, unlink = fs.unlinkSync, rmdir = fs.rmdirSync; + let armed = null; + fs.readdirSync = function (path, ...rest) { + if (armed === 'file' && String(path) === pack) { const names = real.call(this, path, ...rest); unlink(temp); return names; } + if (armed === 'directory' && String(path) === directory) rmdir(directory); + return real.call(this, path, ...rest); + }; + syncBuiltinESMExports(); + t.after(() => { fs.readdirSync = real; syncBuiltinESMExports(); }); + for (const kind of ['file', 'directory']) { + armed = kind; + await assert.rejects(api.readSpec(context), error => { + assert.equal(error.code, 'project-repository', `${kind}: ${error.message}`); + assert.match(error.message, /changed during inspection/); + return true; + }); + assert.equal(armed, kind); + armed = null; + writeFileSync(temp, 'x'); if (!fs.existsSync(directory)) mkdirSync(directory); + } + assert.equal(git(repo, 'rev-parse', 'HEAD'), commit); +}); + +// Row 780 R2: the walk stats a listed directory, then must stat it again when it enters it. The +// test replaces fs.lstatSync for one path and swaps the directory for something else inside the +// call that stats it as a listed entry, after the real stat is taken. A walk that trusts that +// stale stat would read through a link or leak ENOTDIR; the fresh stat refuses both. +async function replaceListedDirectory(t, replace) { + const { context, repo, commit, root } = await fixture(t); + const directory = join(repo, '.git/objects/tmp_dir_replaced'); + const outside = join(root, 'outside'); + mkdirSync(directory); mkdirSync(outside); writeFileSync(join(outside, 'private'), 'x'); + assert.equal((await api.readSpec(context)).commit, commit, 'control: the extra directory alone does not refuse'); + const real = fs.lstatSync; + let armed = true; + fs.lstatSync = function (path, ...rest) { + if (!armed || String(path) !== directory) return real.call(this, path, ...rest); + armed = false; + const stat = real.call(this, path, ...rest); + fs.rmdirSync(directory); replace(directory, outside); + return stat; + }; + syncBuiltinESMExports(); + try { + await assert.rejects(api.readSpec(context), error => { + assert.equal(error.code, 'project-repository', error.message); + assert.match(error.message, /expected local \.git directory/); + return true; + }); + } finally { fs.lstatSync = real; syncBuiltinESMExports(); } + assert.equal(armed, false, 'the listed directory was replaced during inspection'); + assert.equal(git(repo, 'rev-parse', 'HEAD'), commit); +} + +test('a git metadata directory replaced by a symlink after its listed stat refuses, never reads through it', async t => { + await replaceListedDirectory(t, (directory, outside) => symlinkSync(outside, directory)); +}); + +test('a git metadata directory replaced by a file after its listed stat refuses, never raw ENOTDIR', async t => { + await replaceListedDirectory(t, directory => writeFileSync(directory, 'x')); +}); + test('committed UTF-8 BOM, Unicode and trailing newlines are returned without normalization', async t => { const { context, save } = await fixture(t); const value = '\uFEFF# North star 🧭\r\n\r\n'; @@ -117,7 +190,7 @@ test('D4: spec log refuses a commit subject larger than its 256 KiB read bound', writeFileSync(message, 'X'.repeat(256 * 1024 + 1) + '\n'); writeFileSync(join(repo, 'spec/north-star.md'), '# Small spec\n'); execFileSync('git', ['-C', repo, 'add', '--', 'spec/north-star.md']); - execFileSync('git', ['-c', 'core.hooksPath=/dev/null', '-c', 'commit.gpgsign=false', + execFileSync('git', ['-c', 'core.hooksPath=/dev/null', '-c', 'maintenance.auto=false', '-c', 'gc.auto=0', '-c', 'commit.gpgsign=false', '-c', 'user.name=jason', '-c', 'user.email=jason@mosaic.invalid', '-C', repo, 'commit', '-F', message], { stdio: ['ignore', 'ignore', 'pipe'] }); await assert.rejects(api.readSpec({ business, project: 'stack', dataRoot: root }), { code: 'project-git' }); diff --git a/packages/project/tests/tester-init.test.mjs b/packages/project/tests/tester-init.test.mjs index 214469813..fcdf9c4ad 100644 --- a/packages/project/tests/tester-init.test.mjs +++ b/packages/project/tests/tester-init.test.mjs @@ -8,7 +8,7 @@ import { join } from 'node:path'; import { execFileSync } from 'node:child_process'; import { initProject, testerProjectArea } from '../src/index.mjs'; -const git = (repo, ...args) => execFileSync('git', ['-c', 'core.hooksPath=/dev/null', '-C', repo, ...args], { encoding: 'utf8' }).trim(); +const git = (repo, ...args) => execFileSync('git', ['-c', 'core.hooksPath=/dev/null', '-c', 'maintenance.auto=false', '-c', 'gc.auto=0', '-C', repo, ...args], { encoding: 'utf8' }).trim(); function context(t, repo) { const root = mkdtempSync(join(tmpdir(), 'project-tester-')); t.after(() => rmSync(root, { recursive: true, force: true })); diff --git a/packages/webui/tests/preview.test.mjs b/packages/webui/tests/preview.test.mjs index 87ea4e553..53bd92b08 100644 --- a/packages/webui/tests/preview.test.mjs +++ b/packages/webui/tests/preview.test.mjs @@ -1,5 +1,4 @@ -import { leasePort } from '../../../scripts/testing/lane-fixtures.mjs'; -import { after as laneAfter } from 'node:test'; +import { leaseClosedPort } from '../../../scripts/testing/lane-fixtures.mjs'; // Row 74 (#1568, lead decision 93): the preview mode. A request for the public name needs the // proxy's header equal to the one user id, else 401 with nothing read; in preview mode nothing // but GET and HEAD gets through, whatever the Host; loopback requests work as they did. Round 2: @@ -207,7 +206,7 @@ test('in preview mode every write answers 403 read-only, from the public name an }); test('loopback requests are unchanged in preview mode, and the bus views work with the board down', async t => { - const dead = `http://127.0.0.1:${await leasePort({after:laneAfter}, {closed:true})}`; + const dead = `http://127.0.0.1:${await leaseClosedPort(t)}`; const { port, busCalls } = await setup(t, { board: dead, timeout: 200 }); const loopback = { host: `127.0.0.1:${port}` }; const inbox = await send(port, { headers: loopback }); diff --git a/packages/webui/tests/serve.test.mjs b/packages/webui/tests/serve.test.mjs index 901a6dd52..32f6b18ba 100644 --- a/packages/webui/tests/serve.test.mjs +++ b/packages/webui/tests/serve.test.mjs @@ -1,5 +1,4 @@ -import { leasePort } from '../../../scripts/testing/lane-fixtures.mjs'; -import { after as laneAfter } from 'node:test'; +import { leaseClosedPort } from '../../../scripts/testing/lane-fixtures.mjs'; import { test } from 'node:test'; import assert from 'node:assert/strict'; import { createServer, get } from 'node:http'; @@ -98,8 +97,8 @@ test('proxy preserves exact request bytes, status and receipt, rejects forms and } finally { await close(web); await close(upstream); } }); -test('unreachable board reports URL; CLI rejects unsupported options', async () => { - const url = `http://127.0.0.1:${await leasePort({after:laneAfter}, {closed:true})}`; +test('unreachable board reports URL; CLI rejects unsupported options', async t => { + const url = `http://127.0.0.1:${await leaseClosedPort(t)}`; const web = await startServer({ port: 0, board: url, timeout: 100 }); try { const res = await fetch(`http://127.0.0.1:${web.address().port}/api/board`); diff --git a/packages/webui/tests/service.test.mjs b/packages/webui/tests/service.test.mjs index 299675809..c5b017101 100644 --- a/packages/webui/tests/service.test.mjs +++ b/packages/webui/tests/service.test.mjs @@ -1,5 +1,4 @@ -import { leasePort } from '../../../scripts/testing/lane-fixtures.mjs'; -import { after as laneAfter } from 'node:test'; +import { leaseClosedPort } from '../../../scripts/testing/lane-fixtures.mjs'; import { test } from 'node:test'; import assert from 'node:assert/strict'; import { spawn } from 'node:child_process'; @@ -48,7 +47,7 @@ function filled(flags, root = REPO) { // A loopback port with nothing listening, so status's /healthz request // reaches no real Console. -async function closedPort() { return leasePort({after:laneAfter}, {closed:true}); } +async function closedPort(t) { return leaseClosedPort(t); } function tmp(t) { const dir = mkdtempSync(join(tmpdir(), 'webui-service-')); @@ -183,7 +182,7 @@ test('tester service install and status show the tester port without changing se const dir = tmp(t); const unitDir = join(dir, 'units'); const bin = standIn(dir); - const port = await closedPort(); + const port = await closedPort(t); const installed = await run(dir, ['install', ...INSTALL, '--dir', unitDir, '--no-reload', '--port', String(port), '--tester-port', '1024', ...SSO_SEND], bin); assert.equal(installed.status, 0, installed.stderr); assert.match(installed.stdout, /^serve: .* --tester-port 1024( |$)/m); @@ -454,7 +453,7 @@ test('install --send into --dir with --no-reload writes the sending unit, and st const dir = tmp(t); const unitDir = join(dir, 'units'); const target = join(unitDir, 'mosaic-webui.service'); - const port = await closedPort(); + const port = await closedPort(t); const flags = `--port ${port} --sso`; const r = await run(dir, ['install', ...INSTALL, '--dir', unitDir, '--no-reload', '--port', String(port), ...SSO_SEND]); assert.equal(r.status, 0, r.stderr); @@ -728,7 +727,7 @@ test('status names the copy and whether refactor has moved past it, and says whe const unitDir = join(dir, 'units'); const target = join(unitDir, 'mosaic-webui.service'); const bin = standIn(mkdtempSync(join(dir, 'si-'))); - const port = await closedPort(); + const port = await closedPort(t); const head = await new Promise((resolve, reject) => { const git = spawn('git', ['-C', REPO, 'rev-parse', '--verify', 'refs/heads/refactor']); let out = ''; @@ -804,7 +803,7 @@ test('proxy proof render refuses incomplete non-preview missing unsafe-mode and test('proxy proof status retains read-only and tester reporting with both proof flags', async t => { const dir = tmp(t), unitDir = join(dir, 'units'), file = join(dir, 'proof.fixture'); writeFileSync(file, 'fixture', { mode: 0o600 }); - const port = await closedPort(), bin = standIn(dir); + const port = await closedPort(t), bin = standIn(dir); const r = await run(dir, ['install', ...INSTALL, '--dir', unitDir, '--no-reload', '--port', String(port), ...PREVIEW, '--tester-port', '1024', '--proof-header', 'x-console-proof', '--proof-file', file], bin); assert.equal(r.status, 0, r.stderr); const status = await run(dir, ['status', '--dir', unitDir], bin); diff --git a/packages/webui/tests/tester-cli.test.mjs b/packages/webui/tests/tester-cli.test.mjs index de41d8306..88b387ccc 100644 --- a/packages/webui/tests/tester-cli.test.mjs +++ b/packages/webui/tests/tester-cli.test.mjs @@ -18,7 +18,7 @@ const get = (port, path) => new Promise((resolve, reject) => { let text = ''; r.on('data', c => text += c); r.on('end', () => resolve({ status: r.statusCode, body: text })); }); q.on('error', reject); q.end(); }); -async function freePort() { return leasePort({after:laneAfter}, {closed:true}); } +async function freePort() { return leasePort({after:laneAfter}); } test('tester-port validates range tunnel main port and requires configured bus before listening', t => { const root = tmp(t, 'tester-cli-invalid-'), f = fixture(root); diff --git a/packages/webui/tests/tester-fixture.mjs b/packages/webui/tests/tester-fixture.mjs index 26bd3ff68..4d06a4620 100644 --- a/packages/webui/tests/tester-fixture.mjs +++ b/packages/webui/tests/tester-fixture.mjs @@ -39,7 +39,7 @@ export async function testerFixture(t, { enabled = true } = {}) { writeFileSync(join(root, 'outside.md'), OUTSIDE); symlinkSync(join(root, 'outside.md'), join(root, 'records/docs/out.md')); execFileSync('git', ['-c', 'core.hooksPath=/dev/null', 'add', 'docs/out.md'], { cwd: join(root, 'records') }); - execFileSync('git', ['-c', 'core.hooksPath=/dev/null', 'commit', '-m', 'Fixture symlink'], { + execFileSync('git', ['-c', 'core.hooksPath=/dev/null', '-c', 'maintenance.auto=false', '-c', 'gc.auto=0', 'commit', '-m', 'Fixture symlink'], { cwd: join(root, 'records'), env: { ...process.env, GIT_AUTHOR_NAME: 'jason', GIT_AUTHOR_EMAIL: 'fixture@invalid', GIT_COMMITTER_NAME: 'jason', GIT_COMMITTER_EMAIL: 'fixture@invalid' }, }); mkdirSync(f.dataRoot, { recursive: true, mode: 0o700 }); diff --git a/scripts/testing/lane-fixtures.mjs b/scripts/testing/lane-fixtures.mjs index 8f9e65570..3b076c4e7 100644 --- a/scripts/testing/lane-fixtures.mjs +++ b/scripts/testing/lane-fixtures.mjs @@ -1,4 +1,4 @@ -// Imported only by tests. The port holder lives until the fixture closes. +// Imported only by tests. Every port lease is held until the fixture closes. import { spawn } from 'node:child_process'; import { mkdirSync, realpathSync } from 'node:fs'; import { join } from 'node:path'; @@ -12,21 +12,54 @@ async function hold(file){ if(!ok)return null; return async()=>{child.stdin.end('\n');await exited;}; } +// A free port the caller will listen on itself. Nothing holds the port itself, so this is for +// callers that bind it next; the port is probed and closed. async function probe(port){const s=createServer();try{await new Promise((resolve,reject)=>{s.once('error',reject);s.listen(port,'127.0.0.1',resolve);});const found=s.address().port;await new Promise(r=>s.close(r));return found;}catch(e){if(s.listening)await new Promise(r=>s.close(r));if(e.code==='EADDRINUSE')return null;throw e;}} -export async function leasePort(t,{lane=process.env.MOSAIC_GATE_LANE,stateRoot=process.env.MOSAIC_GATE_STATE}={}){ - if(!lane){const port=await probe(0);return port;} +const probed=async port=>{const found=await probe(port);return found?{port:found}:null;}; +// A port nothing answers on. A bound socket that never listens refuses every connect, like any +// closed port, while bind() from another process and the kernel's ephemeral allocation both +// skip it. Node cannot make one (its listen() always follows bind()), so a perl child holds the +// socket and exits when its stdin closes, which also covers the fixture dying abruptly. +const BOUND=String.raw`use strict;use Socket;$|=1; +socket(my $s,PF_INET,SOCK_STREAM,0)or die"socket: $!\n"; +bind($s,sockaddr_in($ARGV[0],inet_aton("127.0.0.1")))or do{my $busy=$!{EADDRINUSE};print STDERR"bind: $!\n";exit($busy?75:76)}; +my($port)=sockaddr_in(getsockname($s));print"bound $port\n";;`; +async function bound(port){ + const child=spawn('perl',['-e',BOUND,String(port)],{stdio:['pipe','pipe','pipe']}); + let err='',out='';child.stderr.on('data',c=>err+=c);child.stdin.on('error',()=>{}); + const exited=once(child,'exit');exited.catch(()=>{}); + const found=await new Promise((resolve,reject)=>{ + child.stdout.on('data',c=>{out+=c;const m=out.match(/^bound (\d+)\n/);if(m)resolve(Number(m[1]));}); + child.once('exit',code=>code===75?resolve(null):reject(Error(`port holder exited ${code}: ${err}`))); + child.once('error',e=>reject(Error(`port holder did not start (perl is required): ${e.message}`))); + }); + if(found===null)return null; + return {port:found,release:async()=>{child.stdin.end('\n');await exited;}}; +} +// A lane takes ports from its own band under a per-port lock; no lane takes the kernel's. +async function lease(t,{lane=process.env.MOSAIC_GATE_LANE,stateRoot=process.env.MOSAIC_GATE_STATE}={},attempt){ + if(!lane){ + const got=await attempt(0);if(!got)refuse('no port available'); + if(got.release)t.after(got.release); + return got.port; + } if(!BANDS[lane]||!stateRoot)refuse('invalid port lane state'); const state=realpathSync(stateRoot);if(!within(SCRATCH,state))refuse('port state outside scratch'); const dir=join(state,'ports',lane);mkdirSync(dir,{recursive:true,mode:0o700}); const [lo,hi]=BANDS[lane]; for(let port=lo;port<=hi;port++){ const release=await hold(join(dir,`${port}.lock`));if(!release)continue; - let actual;try{actual=await probe(port);}catch(e){await release();throw e;} - if(!actual){await release();continue;} - t.after(release);return port; + let got;try{got=await attempt(port);}catch(e){await release();throw e;} + if(!got){await release();continue;} + t.after(async()=>{try{await got.release?.();}finally{await release();}});return got.port; } refuse('lane port band exhausted'); } +export async function leasePort(t,{closed,...context}={}){ + if(closed!==undefined)refuse('leasePort takes no closed option; leaseClosedPort holds a port nothing answers on'); + return lease(t,context,probed); +} +export async function leaseClosedPort(t,context={}){return lease(t,context,bound);} export function fixtureCacheEnv(root){ mkdirSync(root,{recursive:true,mode:0o700});const dir=realpathSync(root);for(const n of ['tmp','jiti','cache'])mkdirSync(join(dir,n),{recursive:true,mode:0o700}); return {TMPDIR:join(dir,'tmp'),JITI_FS_CACHE:join(dir,'jiti'),XDG_CACHE_HOME:join(dir,'cache')};