From f0fe9438220de18b47a4f2a44dfbbb9f5174e535 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Wed, 4 Nov 2026 10:35:57 -0600 Subject: [PATCH] fix(bus): keep foreign working repos refused, hold the organization lock through protected commits, fence serving on the activate receipt (#802) --- BUILD-LOG.md | 2 + docs/SESSIONS.md | 1 + packages/bus/src/request-bootstrap.mjs | 61 +++--- packages/bus/src/runtime.mjs | 14 +- packages/bus/tests/request-bootstrap.test.mjs | 176 ++++++++++++++++++ packages/business/src/organization.mjs | 27 ++- .../business/tests/project-bootstrap.test.mjs | 17 ++ packages/project/src/index.mjs | 36 ++-- packages/seat/src/working.mjs | 60 ++++-- packages/seat/tests/working.test.mjs | 18 ++ 10 files changed, 345 insertions(+), 67 deletions(-) diff --git a/BUILD-LOG.md b/BUILD-LOG.md index 37b05551c..808cb60d0 100644 --- a/BUILD-LOG.md +++ b/BUILD-LOG.md @@ -6009,3 +6009,5 @@ Owner directed static-only fallback after actual nativepriority drift: no new pr 2026-11-04T02:51:19Z | Lead (Claude Opus 5.5, T3 c5c1e8ba) | #797 / Gitea #1839 lifecycle Task 2 landing | Task 2 c55849806 (Sonnet 5.5 xhigh): exact-add organization writer, journaled bootstrap steps, 3o->3p append-only migration, receipt-backed runtime admission. Review R1 (GPT-6.1 Sol): NEEDS FIXES, C1 seed write through a committed .mosaic link, C2 recursive code.tmp delete, I1 #800 freshness only at reservation, I2 missing marker admitted, I3 no host preparation/admission wiring, I4 Task 1 existing-project root/repo comparison reversed. Fix round 883003041 (Sonnet 5.5 xhigh): O_EXCL|O_NOFOLLOW publish, claimed code.tmp, per-effect fresh(), layout-based managedState, request-host.mjs wiring, corrected localPlan. Gates on 883003041: focused 79/79, kept-green 49/49, bus 1477/1477 (first run had one model-final-completion startNice flake, reviewer found no causal path), business 305/305, project+seat 267/267, stack-release 225/225, zero skips (/mnt/storage/scratch/tmp/fix797). Review R2: C1 C2 I3 I4 resolved; I1 (org write during intent step Git reads still lets a stale intent write), I2 (failed final marker rename or later marker removal leaves admission ready) and new N1 (preexisting /working ancestor link sends the working repo outside dataRoot) open. Lead decision: land under the MVP one-round rule; all three need write access to dataRoot, a concurrent organization edit or an I/O fault, and none is reachable by a sandboxed worker. They form one hardening follow-up that must land before bootstrap is exercised on .53. Minors are tracker #801. 2026-11-04T03:23:23Z | Claude Sonnet 5.5 implementation sub-agent, t3/harden-project-bootstrap-executor | #802 bootstrap executor hardening after review R2 | One commit on 6dd4f92b0 for the three open R2 findings; nothing else touched (Minors #801, Tasks 3-8, roles/*.json, ROUTED, migrations out of scope; no schema change). N1: the working repository path is confined before and inside the helper. working.mjs ensure() now walks every directory between the data root and the repository with lstat, creates missing ones one at a time and refuses a link or a non-directory (working-repo-invalid); push() goes through the same ensure. The executor refuses a linked or non-directory ancestor as bootstrap-uncertain, keeps a repository that is already there on a first attempt, and on a retry adopts only a bare repository whose refs are root and main at the seed commit. I1: request-bootstrap.mjs gains guarded(effect): the freshness checks and the start of the effect run in one synchronous run with the organization lock held, after every asynchronous read the effect depends on. It wraps the clone, claim, remote removal and rename, the seed publish, add and commit, the working init/rename/fetch/ref writes (initializeWorking and project commitFile take an optional `around` that runs each write), the records init and the intent file publish. The old lock comment, which claimed more than fresh() gave, is replaced with what each form proves. I2: activate publishes the complete marker first and calls services.activate second, so a failed marker rename ends the step before the broker is touched. Readiness needs both a complete marker and a committed activate receipt at boot (pendingProject), at reload (activateManagedProject refuses bootstrap-marker-incomplete) and in admission() (project-pending), where a receipt alone used to be enough. RED/GREEN: run against 6dd4f92b0 source with the new tests copied in, 7 of 39 bus tests and 3 of 31 seat tests fail (n1 working link, n1 business-directory link, n1 foreign ref on retry, i1 intent interleaving, i2 EIO at the marker rename, i2 runtime marker fence, i2 marker removal live and after reopen; seat: both link refusals and the five-write around test); all pass now. Honest notes: five new tests also pass on the old source and are guards, not reproducers: the working-file ancestor (one bus test, one seat test), the preexisting repository on a first attempt, the kill-and-adopt retry, and the i1 seed interleaving (the old code ran fresh() directly before the publish on the default path, where origin is already removed); my first business-directory test asserted the link at the wrong path and was corrected. GREEN: brief trio 90/90 (was 79), bus 1488/1488 (was 1477), business 305/305, project+seat 271/271 (was 267), launcher+stack-release 261/261, zero skips. Limits: a git child's own running time after it starts is not covered by the lock, and the next step's check catches a change made meanwhile; between services.activate and the receipt commit there is one microtask where the broker already serves the project, and admission() fences that but tool serving is not fenced; push now refuses a symlinked working ancestor it used to follow. Logs /mnt/storage/scratch/tmp/V7Ps0O/. Not executed on any running instance; local commit only, no push/merge/PR/deployment. + +2026-11-04T16:35:38Z | GPT-6.1 Sol implementation sub-agent, t3/bootstrap-hardening-fix-802-1 | #802 bootstrap hardening fix round 1 before/after | Before: fast-forwarded the assigned worktree to reviewed 52b283c8a; C1 foreign working-repo retry adoption/internal object links, C2 stale records and intent commits, C3 broker serving before the activate receipt. After: a first-attempt foreign repository gets a durable refused working receipt; recovery requires this plan's ownership claim, created exclusively inside the init directory before rename, as well as the existing ref checks. Every working-repository Git child checks for internal links and alternates; bootstrap also rechecks between its recovery reads. The existing kill-and-adopt recovery remains green. Organization holdAsync keeps the existing on-disk lock through asynchronous effects and releases it on success or failure; records initialization, intent publication/add/commit and intent rollback stay protected, while the bootstrap's exact-add mapping keeps its own transaction. The complete marker is verified and the activate receipt commits before the broker reload; runtime no longer overrides pending admission, and receipt I/O failure stays fenced until a fault-free retry. RED on 52b283c8a with final tests overlaid: 79 existing tests pass and all 16 new regressions fail as expected across request-bootstrap, project-bootstrap and working (95 total, zero skips); all five supplied reproducers were ported with corrected assertions. Final gates, all zero failures/skips/cancellations: focused 103/103 (baseline 90), bus 1499/1499 (1488), business 307/307 (305), project+seat 274/274 (271), launcher+stack-release including external-release cases 286/286 (286). Gate 5 uses node --test packages/cli/tests/launcher.test.mjs packages/cli/tests/stack-release*.test.mjs because these are CLI test files, not separate packages. Corrections: an initial gate-5 command named nonexistent package directories and ran no tests; an intermediate bus run failed the newly added per-child C1 regression, also reproduced on the base and fixed before the final green rerun. An earlier candidate gate-5 run was superseded and only its own descendant processes stopped; the final run completed. No preexisting test failure was found. Evidence: /mnt/storage/scratch/tmp/qRkZh1/ (before-1..4.log, before-5-cli.log, after-1..5.log, red-final-snapshot.log, verified-files.sha256). Provider interruption recovered from complete logs and matching source/test hashes. The queue refuses linked worktrees, so the explicit controller assignment was followed without changing the canonical queue. One local commit only; no push, live-instance action, role/contract/migration edit or residual C1/C2/C3 finding. diff --git a/docs/SESSIONS.md b/docs/SESSIONS.md index a2293db8e..3efef103c 100644 --- a/docs/SESSIONS.md +++ b/docs/SESSIONS.md @@ -1998,3 +1998,4 @@ are never rewritten or removed; corrections are new entries. 2026-11-04 | Claude Sonnet 5.5 implementation sub-agent, t3/lifecycle-bootstrap-fix-round | #797 / Gitea #1839 lifecycle Task 2 fix round | C1 link-confined seed and marker writes, C2 receipt-backed code.tmp kept as bootstrap-uncertain when unproven, I1 freshness recheck before every effect, I2 layout-based managed classification, I3 host preparePlans/admitPlan wired (named setup waits), I4 existing-project comparison. Brief 79/79, kept-green 49/49, bus 1477/1477, business 305/305, project+seat 267/267, stack-release 225/225, zero skips; no schema change; local commit only, no push. 2026-11-04 | Lead (Claude Opus 5.5, T3 c5c1e8ba) | #797 lifecycle Task 2 review, fix round and landing | R1 needs fixes, fix 883003041, R2 left I1/I2/N1 residuals as a pre-.53 hardening follow-up; landed on refactor 2026-11-04 | Claude Sonnet 5.5 implementation sub-agent, t3/harden-project-bootstrap-executor | #802 bootstrap executor hardening | N1 working path confined before and inside the helper, I1 guarded() makes the freshness check and every protected write one synchronous run under the organization lock, I2 complete marker published before activation and readiness needs marker plus committed receipt at boot, reload and admission. Brief trio 90/90, bus 1488/1488, business 305/305, project+seat 271/271, launcher+stack-release 261/261, zero skips; no schema change; local commit only, no push. +2026-11-04 | GPT-6.1 Sol implementation sub-agent, t3/bootstrap-hardening-fix-802-1 | #802 bootstrap hardening fix round 1 | C1 permanent foreign-repo refusal, exclusive ownership claim and per-child internal-link/alternates checks; C2 async-held organization lock through records init, intent commit and rollback; C3 committed activate receipt before broker reload, fenced receipt failure and successful retry. All 16 added regressions fail on 52b283c8a; final focused 103/103, bus 1499/1499, business 307/307, project+seat 274/274, launcher+release including external cases 286/286, zero skips. Logs /mnt/storage/scratch/tmp/qRkZh1/; one local commit, no push, no role/contract/migration/live changes. diff --git a/packages/bus/src/request-bootstrap.mjs b/packages/bus/src/request-bootstrap.mjs index 929261589..2ac2dacfc 100644 --- a/packages/bus/src/request-bootstrap.mjs +++ b/packages/bus/src/request-bootstrap.mjs @@ -18,7 +18,7 @@ // Nothing is ever deleted recursively, and every write goes through an exclusively created file and a rename, // inside directories that are verified not to be links, so a link in a source checkout cannot redirect a write. // Before every effect the organization revision and the chat association are checked again (#800), and the check -// and the start of the effect are one synchronous run under the organization lock (`guarded`). A project is ready +// and the whole effect, including asynchronous git children, run under the organization lock (`guarded`). A project is ready // only with both a complete marker and a committed activate receipt, so the marker goes first and nothing is // admitted until it is published. import { createHash } from 'node:crypto'; @@ -27,7 +27,7 @@ import { closeSync, constants, lstatSync, mkdirSync, openSync, readFileSync, rea import { dirname, join, relative, sep } from 'node:path'; import { digest, refuse, validatePlan } from './request-store.mjs'; import { gitEnv, runGit } from '../../seat/src/checkout.mjs'; -import { initializeWorking, workingRepo } from '../../seat/src/working.mjs'; +import { initializeWorking, inspectWorkingRepository, workingRepo } from '../../seat/src/working.mjs'; import { initProject, writeHumanFile } from '../../project/src/index.mjs'; import { SAFE, identity } from '../../project/src/git.mjs'; import { loadProject } from '../../business/src/project.mjs'; @@ -97,6 +97,7 @@ export async function prepareBootstrap({ business, request, sourceProject, proje } class Uncertain extends Error {} +class ForeignWorking extends Uncertain {} // executeBootstrap({ binding, owner, journal, services }) -> { project, chat_project, spec_commit, organization_revision } // services: dataRoot, organization (the writer), chat { state, associate }, activate (synchronous), barrier (tests). @@ -109,6 +110,7 @@ export async function executeBootstrap({ binding, owner, journal, services }) { const { business, project, request } = plan; const planDigest = binding.digest, base = name => `bootstrap:${planDigest}:${name}`; const here = managedPaths({ dataRoot, business, project }); + const workingPath = workingRepo(dataRoot, business, project); const product = dirname(here.root), marker = markerFile({ dataRoot, business, project }); const chatId = chatProjectId(request, planDigest), human = () => writer.read(business).document.human; const uncertain = () => { throw new Uncertain('bootstrap-uncertain'); }; @@ -145,23 +147,28 @@ export async function executeBootstrap({ binding, owner, journal, services }) { // association is not another's. `fresh` runs under the organization lock, so a bare `fresh()` proves the world // matched at that instant and nothing more: the lock is released when it returns. What keeps a write from landing // under a stale plan is `guarded(effect)`: the caller has finished every asynchronous read the effect depends on, - // and the check and the start of the effect run in one synchronous run with the lock held, so no organization - // write lands between them. An effect that is a git child is started inside the lock; the child's own running - // time is not covered, and the next step's check catches a change made meanwhile. A mismatch keeps what is done - // and performs nothing further. `effect` must be synchronous and must not take the organization lock itself. - const fresh = effect => writer.hold(business, ({ revision, document }) => { + // and the check and the entire effect run with the lock held, through every awaited child. A mismatch keeps + // what is done and performs nothing further. The effect must not take the organization lock itself; mapping + // uses the writer's own exact-add transaction separately. + const checkFresh = ({ revision, document }) => { const stale = () => refuse('request-plan-stale'); if (document.id !== business || document.human !== owner) stale(); const added = writer.findAddition(business, { request, plan_digest: planDigest, project }); if (added) { if (revision !== added.after || canonicalJson(document.projects?.[project]) !== canonicalJson(entry)) stale(); } else if (revision !== plan.organization_revision || Object.hasOwn(document.projects ?? {}, project)) stale(); if (services.chat.state({ business, project, name: plan.name, id: chatId }) === 'conflict') stale(); - effect?.(); - }); - const guarded = effect => { let started; fresh(() => { started = effect(); }); return started; }; + }; + const fresh = () => writer.hold(business, checkFresh); + const guarded = effect => writer.holdAsync(business, now => { checkFresh(now); return effect(); }); - const git = (cwd, args, extra = {}, file = false) => runGit([...SAFE, ...(file ? ['-c', 'protocol.file.allow=always'] : []), ...(cwd ? ['-C', cwd] : []), ...args], - { ...gitEnv(), ...(cwd ? { GIT_CEILING_DIRECTORIES: dirname(cwd) } : {}), ...extra }); + const git = (cwd, args, extra = {}, file = false) => { + if (args.includes(`--git-dir=${workingPath}`)) { + if (!confined(workingPath)) uncertain(); + inspectWorkingRepository(workingPath); + } + return runGit([...SAFE, ...(file ? ['-c', 'protocol.file.allow=always'] : []), ...(cwd ? ['-C', cwd] : []), ...args], + { ...gitEnv(), ...(cwd ? { GIT_CEILING_DIRECTORIES: dirname(cwd) } : {}), ...extra }); + }; const out = async (cwd, args, extra, file) => { const r = await git(cwd, args, extra, file); if (!r.ok) refuse('bootstrap-git-failed'); return r.out.trim(); }; const tryOut = async (cwd, args) => { const r = await git(cwd, args); return r.ok ? r.out.trim() : null; }; @@ -183,7 +190,7 @@ export async function executeBootstrap({ binding, owner, journal, services }) { result = await run({ recovering: last !== undefined }); } catch (e) { if (!(e instanceof Uncertain)) throw e; - journal.finishEffect({ ...header, stage: 'uncertain', result: null, code: 'bootstrap-uncertain' }); + journal.finishEffect({ ...header, stage: e instanceof ForeignWorking ? 'refused' : 'uncertain', result: null, code: 'bootstrap-uncertain' }); refuse('bootstrap-uncertain'); } if (after) await services.barrier?.(name); @@ -302,9 +309,9 @@ export async function executeBootstrap({ binding, owner, journal, services }) { // 3. working. The working repository's main follows the checkout's HEAD, which is the seed commit. Nothing is made // while a directory between the data root and the repository is a link or a file (N1: `working` linked to a // sibling directory would take the repository and every file in it out of the data root). A repository that is - // already there is this plan's only on a retry, and only if it is a bare repository holding no ref but root and - // main, both at the seed commit; on a first attempt it is someone else's. Either way it is kept as it is. - const workingPath = workingRepo(dataRoot, business, project); + // already there on a first attempt stays refused on every retry. Recovery additionally requires the ownership + // claim written inside the exclusively created init directory before its rename, plus no foreign refs. + const workingClaim = JSON.stringify({ version: 1, kind: 'bootstrap-working', request, plan_digest: planDigest, project }) + '\n'; const workingExists = () => { let at = dataRoot; for (const name of relative(dataRoot, workingPath).split(sep)) { @@ -318,14 +325,17 @@ export async function executeBootstrap({ binding, owner, journal, services }) { const working = async ({ recovering }) => { const seedCommit = done('seed').commit; if (workingExists()) { - if (!recovering) uncertain(); + if (!recovering) throw new ForeignWorking('bootstrap-uncertain'); + inspectWorkingRepository(workingPath); + const claim = join(workingPath, 'mosaic-bootstrap.json'), owned = exists(claim); + if (!owned?.isFile() || readFileSync(claim, 'utf8') !== workingClaim) uncertain(); const at = [`--git-dir=${workingPath}`]; const refs = await tryOut(null, [...at, 'for-each-ref', '--format=%(refname) %(objectname)']); const bare = await tryOut(null, [...at, 'rev-parse', '--is-bare-repository']); const expected = new Set(['refs/heads/root', 'refs/heads/main'].map(ref => `${ref} ${seedCommit}`)); if (bare !== 'true' || refs === null || refs.split('\n').filter(Boolean).some(line => !expected.has(line))) uncertain(); } - const w = await initializeWorking({ dataRoot, business, project, root: here.root, around: guarded }); + const w = await initializeWorking({ dataRoot, business, project, root: here.root, around: guarded, claim: workingClaim }); if (w.commit !== seedCommit) uncertain(); return { commit: w.commit }; }; @@ -389,16 +399,15 @@ export async function executeBootstrap({ binding, owner, journal, services }) { return { chat_project: chatId }; }; - // 8. activate. The complete marker is published first and the broker admits the project second, so a project is - // never admitted without it: a failed marker write ends the step before the broker is touched. A complete marker - // with no committed receipt admits nothing, since readiness at runtime and at startup needs both, and a revision - // that moved before the admission leaves the marker as it is and the project pending. + // 8. activate. Publish and verify the complete marker, then commit the receipt before reloading the broker. + // A receipt failure leaves the live broker fenced; retries replay committed receipts and retry the reload. const activate = async () => { await services.barrier?.('activate'); const mapped = done('mapping'); - guarded(() => publish(marker, `${marker}.tmp`, markerBytes({ state: 'complete', request, plan_digest: planDigest, project }))); - try { services.activate({ business, project, receipt, organization_revision: mapped.organization_revision }); } - catch (e) { if (e?.code === 'bootstrap-revision-stale') refuse('request-plan-stale'); throw e; } + guarded(() => { + publish(marker, `${marker}.tmp`, markerBytes({ state: 'complete', request, plan_digest: planDigest, project })); + if (!markerOwns() || readMarker(marker).state !== 'complete') uncertain(); + }); return { project, organization_revision: mapped.organization_revision }; }; @@ -411,5 +420,7 @@ export async function executeBootstrap({ binding, owner, journal, services }) { const saved = await step('intent', intent); const associated = await step('chat', chat); const active = await step('activate', activate, { after: false }); + try { services.activate({ business, project, receipt, organization_revision: active.organization_revision }); } + catch (e) { if (e?.code === 'bootstrap-revision-stale') refuse('request-plan-stale'); throw e; } return { project, chat_project: associated.chat_project, spec_commit: saved.commit, organization_revision: active.organization_revision }; } diff --git a/packages/bus/src/runtime.mjs b/packages/bus/src/runtime.mjs index d523cf91a..0aaa5bfde 100644 --- a/packages/bus/src/runtime.mjs +++ b/packages/bus/src/runtime.mjs @@ -297,8 +297,8 @@ export async function startBroker({ const state = managedState({ dataRoot, business, project, entry }); return state !== 'unmanaged' && !(state === 'complete' && activated.has(project)); }; - const withoutPending = (business, b, admit = []) => { - const activated = new Set([...activatedProjects(business), ...admit]); + const withoutPending = (business, b) => { + const activated = activatedProjects(business); const projects = Object.fromEntries(Object.entries(b.projects ?? {}).filter(([project, entry]) => !pendingProject(business, project, entry, activated))); return Object.keys(projects).length === Object.keys(b.projects ?? {}).length ? b : { ...b, projects }; }; @@ -519,11 +519,10 @@ export async function startBroker({ } } // Lifecycle Task 2 (#797): the only way a project joins a running bus. The bootstrap executor calls it - // synchronously after every earlier step passed and the complete marker is published. It admits exactly the + // synchronously after every earlier step passed, the complete marker is published and the activate receipt commits. It admits exactly the // receipt-backed addition under the organization lock: the file must still be this business plus that entry, // with the credentials, bots, libraries, roles and prior mappings the bus booted with. Anything else leaves - // the product pending. The executor commits the activate receipt right after this returns; admission() and a - // restart treat the product as ready only once that receipt exists. + // the product pending. Both readiness records must exist before even the broker's reload begins. const writer = organization ? organizationWriter({ dir: organization.dir, rolesDir: organization.rolesDir }) : null; const known = new Map(Object.entries(businesses).map(([id, b]) => [id, canonicalJson(b.projects ?? {})])); function activateManagedProject({ business: id, project, receipt, organization_revision }) { @@ -544,7 +543,8 @@ export async function startBroker({ if (!entry || entry.kind === 'library' || entry.root !== here.root || entry.repo !== here.repo || Object.keys(entry).some(k => !['root', 'repo', 'remote'].includes(k))) refuse('bootstrap-mapping-mismatch'); if (managedState({ dataRoot, business: id, project, entry }) !== 'complete') refuse('bootstrap-marker-incomplete'); - const next = withoutPending(id, full, [project]); + if (!activatedProjects(id).has(project)) refuse('bootstrap-receipt-mismatch'); + const next = withoutPending(id, full); const { [project]: _added, ...before } = next.projects; if (canonicalJson(before) !== known.get(id)) refuse('bootstrap-mapping-mismatch'); const expected = { ...booted.get(id), roots: canonicalJson([...JSON.parse(booted.get(id).roots), entry.root].sort()) }; @@ -560,7 +560,7 @@ export async function startBroker({ // broker does not hold is never ready, and a declared one that is not yet held says it is pending. A managed // product the broker holds is ready only while it has a complete marker and a committed activate receipt: the // answer is read from both on every call, so a marker removed after the activation closes it again, and the - // broker's admission, which comes before the receipt commits, is not yet readiness (#802). + // broker's reload now follows both readiness records (#802). function admission({ business: id, project }) { const no = code => ({ ready: false, revision: null, code }); if (typeof id !== 'string' || typeof project !== 'string' || !Object.hasOwn(businesses, id)) return no('project-unknown'); diff --git a/packages/bus/tests/request-bootstrap.test.mjs b/packages/bus/tests/request-bootstrap.test.mjs index a61b30cf6..9117569d7 100644 --- a/packages/bus/tests/request-bootstrap.test.mjs +++ b/packages/bus/tests/request-bootstrap.test.mjs @@ -547,6 +547,182 @@ const moveOrganization = f => { const now = f.writer.read('demo'), next = structuredClone(now.document); next.vars['tracker.pollSeconds'] = 61; f.writer.write('demo', next, { revision: now.revision, actor: 'jason' }); }; + +// #802 C1: a refused foreign repository never becomes this attempt's on retry. +for (const linked of [false, true]) { + test(`c1_foreign_working_repo_stays_refused_on_retry_${linked ? 'linked_objects' : 'empty_refs'}`, async t => { + let outside, repo; + const f = await setup(t, { configureFactory: ({ root, f }) => { + outside = external(root, 'external-objects'); + repo = workingRepo(f.dataRoot, 'demo', 'timer'); mkdirSync(repo, { recursive: true }); + git(repo, 'init', '-q', '--bare', '--template='); + if (linked) { rmSync(join(repo, 'objects'), { recursive: true }); symlinkSync(outside, join(repo, 'objects')); } + } }); + assert.equal((await f.choose()).wait.code, 'bootstrap-uncertain'); + assert.deepEqual(readdirSync(outside), []); + // Even replacing the foreign repository with matching refs does not undo its refusal. + if (!linked) git(repo, 'fetch', '-q', f.paths.code, 'HEAD:refs/heads/main', 'HEAD:refs/heads/root'); + for (let retry = 0; retry < 2; retry++) { + const again = await f.bus.requestLifecycle.resume('demo', f.request); + assert.equal(again.phase, 'bootstrap'); assert.equal(again.wait.code, 'bootstrap-uncertain'); + assert.deepEqual(readdirSync(outside), []); assert.equal(f.additions().length, 0); + assert.equal(f.bus.admission({ business: 'demo', project: 'timer' }).ready, false); + } + }); +} + +for (const refs of ['empty', 'matching']) { + test(`c1_retry_refuses_a_replacement_working_repo_without_ownership_${refs}`, async t => { + let armed = true; + const f = await setup(t, { barrier: step => { if (armed && step === 'working') { armed = false; throw new Error('killed'); } } }); + assert.equal((await f.choose()).wait.code, 'bootstrap-uncertain'); + const repo = workingRepo(f.f.dataRoot, 'demo', 'timer'); + rmSync(repo, { recursive: true }); mkdirSync(repo); + git(repo, 'init', '-q', '--bare', '--template='); + if (refs === 'matching') git(repo, 'fetch', '-q', f.paths.code, 'HEAD:refs/heads/main', 'HEAD:refs/heads/root'); + const before = git(repo, 'for-each-ref', '--format=%(refname) %(objectname)'); + const again = await f.bus.requestLifecycle.resume('demo', f.request); + assert.equal(again.phase, 'bootstrap'); assert.equal(again.wait.code, 'bootstrap-uncertain'); + assert.equal(git(repo, 'for-each-ref', '--format=%(refname) %(objectname)'), before); + assert.equal(f.additions().length, 0); + }); +} + +test('c1_retry_checks_internal_links_before_inspecting_working_refs', async t => { + let armed = true; + const f = await setup(t, { barrier: step => { if (armed && step === 'working') { armed = false; throw new Error('killed'); } } }); + assert.equal((await f.choose()).wait.code, 'bootstrap-uncertain'); + const repo = workingRepo(f.f.dataRoot, 'demo', 'timer'), outside = external(f.root); + symlinkSync(outside, join(repo, 'objects/info/linked')); + let children = 0; + const result = await withSpawnHook((binary, args) => { + if (binary === 'git' && args.includes(`--git-dir=${repo}`)) children++; + }, () => f.bus.requestLifecycle.resume('demo', f.request)); + assert.equal(result.phase, 'bootstrap'); assert.equal(result.wait.code, 'bootstrap-uncertain'); + assert.equal(children, 0); assert.deepEqual(readdirSync(outside), []); +}); + +test('c1_retry_rechecks_internal_links_before_each_working_git_child', async t => { + let armed = true; + const f = await setup(t, { barrier: step => { if (armed && step === 'working') { armed = false; throw new Error('killed'); } } }); + await f.choose(); + const repo = workingRepo(f.f.dataRoot, 'demo', 'timer'), outside = external(f.root); + let children = 0; + const result = await withSpawnHook((binary, args) => { + if (binary === 'git' && args.includes(`--git-dir=${repo}`)) { + children++; + if (args.includes('for-each-ref')) queueMicrotask(() => symlinkSync(outside, join(repo, 'objects/info/linked'))); + } + }, () => f.bus.requestLifecycle.resume('demo', f.request)); + assert.equal(children, 1); assert.equal(result.wait.code, 'bootstrap-uncertain'); + assert.deepEqual(readdirSync(outside), []); +}); + +// #802 C2: these hooks run real organization writes at the review's two scheduling points. +test('c2_intent_commit_holds_the_organization_lock_until_git_finishes', async t => { + const f = await setup(t), revision = () => f.writer.read('demo').revision; + let attempted = false, before, refusal; + const result = await withSpawnHook((binary, args) => { + if (!attempted && binary === 'git' && args.includes(f.paths.records) && args.includes('commit') && args.includes('Edit spec/north-star.md')) { + attempted = true; before = revision(); + assert.equal(readFileSync(join(f.paths.records, 'spec/north-star.md'), 'utf8'), INTENT); + assert.equal(git(f.paths.records, 'rev-list', '--count', 'HEAD'), '1'); + try { moveOrganization(f); } catch (e) { refusal = e.code; } + } + }, () => f.choose()); + assert.equal(attempted, true); assert.equal(refusal, 'organization-busy'); + assert.equal(revision(), before); assert.equal(result.phase, 'discovery'); + assert.equal(git(f.paths.records, 'log', '-1', '--format=%s'), 'Edit spec/north-star.md'); + moveOrganization(f); assert.notEqual(revision(), before, 'the lock is released after the effect'); +}); + +test('c2_records_init_holds_the_organization_lock_across_awaits', async t => { + const f = await setup(t), original = cp.spawn; + let attempted = false, refusal, before; + cp.spawn = function (binary, args, ...rest) { + const child = original.call(this, binary, args, ...rest); + if (!attempted && binary === 'git' && args.includes(f.paths.records) && args.includes('init')) { + attempted = true; before = f.writer.read('demo').revision; + queueMicrotask(() => { try { moveOrganization(f); } catch (e) { refusal = e.code; } }); + } + return child; + }; + syncBuiltinESMExports(); + let result; + try { result = await f.choose(); } finally { cp.spawn = original; syncBuiltinESMExports(); } + assert.equal(attempted, true); assert.equal(refusal, 'organization-busy'); + assert.equal(f.writer.read('demo').revision, before); assert.equal(result.phase, 'discovery'); + assert.equal(git(f.paths.records, 'log', '--format=%s').split('\n')[1], 'Initialize project layout v1'); +}); + +test('c2_intent_rollback_keeps_the_organization_lock_and_retry_completes', async t => { + const f = await setup(t); + let failed = false, refusal, before; + const result = await withSpawnHook((binary, args) => { + if (binary !== 'git' || !args.includes(f.paths.records)) return; + if (!failed && args.includes('commit') && args.includes('Edit spec/north-star.md')) { + failed = true; before = f.writer.read('demo').revision; + throw new Error('injected commit spawn failure'); + } + if (failed && args.includes('reset')) { + try { moveOrganization(f); } catch (e) { refusal = e.code; } + } + }, () => f.choose()); + assert.equal(failed, true); assert.equal(refusal, 'organization-busy'); + assert.equal(f.writer.read('demo').revision, before); assert.equal(result.wait.code, 'bootstrap-uncertain'); + assert.equal(git(f.paths.records, 'rev-list', '--count', 'HEAD'), '1'); + assert.equal(git(f.paths.records, 'status', '--porcelain'), ''); + assert.notEqual(readFileSync(join(f.paths.records, 'spec/north-star.md'), 'utf8'), INTENT); + assert.equal((await f.bus.requestLifecycle.resume('demo', f.request)).phase, 'discovery'); +}); + +test('c3_broker_reload_microtask_only_serves_after_the_activate_receipt', async t => { + const f = await setup(t), broker = f.bus.broker, original = broker.reloadBusiness; + let observed; + broker.reloadBusiness = function (...args) { + const result = original.apply(this, args); + queueMicrotask(() => { + try { observed = { active: activations(f), admission: admission(f), made: f.call('human', 'project.create', { name: 'After receipt', repo: 'timer' }) }; } + catch (error) { observed = { error }; } + }); + return result; + }; + let result; + try { result = await f.choose(); } finally { broker.reloadBusiness = original; } + assert.ok(observed); assert.ifError(observed.error); + assert.equal(observed.active.length, 1); assert.equal((await observed.admission).ready, true); + assert.ok(observed.made.id); assert.equal(result.phase, 'discovery'); +}); + +test('c3_activate_receipt_io_failure_keeps_broker_fenced_until_retry', async t => { + const f = await setup(t), original = DatabaseSync.prototype.prepare; + let faulted = false, servedBeforeReceipt = false; + DatabaseSync.prototype.prepare = function (sql, ...rest) { + const statement = original.call(this, sql, ...rest); + if (sql.startsWith('INSERT INTO request_effects(')) { + const run = statement.run; + statement.run = function (...args) { + if (!faulted && String(args[4]).endsWith(':activate') && args[7] === 'committed') { + faulted = true; + try { f.call('human', 'project.create', { name: 'Before receipt', repo: 'timer' }); servedBeforeReceipt = true; } catch {} + throw Object.assign(new Error('injected activation receipt I/O error'), { code: 'SQLITE_IOERR' }); + } + return run.apply(this, args); + }; + } + return statement; + }; + let result; + try { result = await f.choose(); } finally { DatabaseSync.prototype.prepare = original; } + assert.equal(faulted, true); assert.equal(result.phase, 'bootstrap'); assert.equal(result.wait.code, 'bootstrap-uncertain'); + assert.equal(marked(f), 'complete'); assert.deepEqual(activations(f), []); + assert.equal((await admission(f)).ready, false); + assert.throws(() => f.call('human', 'project.create', { name: 'After failed receipt', repo: 'timer' })); + assert.equal(servedBeforeReceipt, false); + assert.equal((await f.bus.requestLifecycle.resume('demo', f.request)).phase, 'discovery'); + assert.equal(activations(f).length, 1); assert.equal((await admission(f)).ready, true); + assert.ok(f.call('human', 'project.create', { name: 'After retry', repo: 'timer' }).id); +}); // `before(binary, args)` runs just ahead of every child the code spawns while `during` runs. async function withSpawnHook(before, during) { const original = cp.spawn; diff --git a/packages/business/src/organization.mjs b/packages/business/src/organization.mjs index cf11c2857..de7861f8c 100644 --- a/packages/business/src/organization.mjs +++ b/packages/business/src/organization.mjs @@ -187,11 +187,20 @@ export function organizationWriter({ dir, rolesDir, barrier = () => {} }) { } fail("organization-uncertain", "stale writer lock: confirm the old writer is gone before removing it"); } - try { - return fn({ home, parent }); - } finally { + const release = () => { rmSync(lock, { recursive: true }); syncDir(home); + }; + let pending = false; + try { + const out = fn({ home, parent }); + if (out !== null && typeof out === "object" && typeof out.then === "function") { + pending = true; + return Promise.resolve(out).finally(release); + } + return out; + } finally { + if (!pending) release(); } } @@ -353,6 +362,16 @@ export function organizationWriter({ dir, rolesDir, barrier = () => {} }) { }); } + // Like hold, but the lock stays on disk until an asynchronous effect settles, including on failure. + // The effect must not call an organization writer: it would find this same lock and refuse busy. + function holdAsync(id, fn) { + requireId(id, "organization id"); + return withLock(id, () => { + recover(id); + return fn(read(id)); + }); + } + // The committed addition of this request and plan, if there is one. A prepared receipt whose // `after` is the file's current hash counts: the rename landed and the commit line did not. function findAddition(id, { request, plan_digest, project }) { @@ -393,5 +412,5 @@ export function organizationWriter({ dir, rolesDir, barrier = () => {} }) { }); } - return { logFile, read, readRaw, write, addProject, hold, findAddition, record, recover: id => withLock(id, () => recover(id)) }; + return { logFile, read, readRaw, write, addProject, hold, holdAsync, findAddition, record, recover: id => withLock(id, () => recover(id)) }; } diff --git a/packages/business/tests/project-bootstrap.test.mjs b/packages/business/tests/project-bootstrap.test.mjs index dd55fd1d8..dcd2f087d 100644 --- a/packages/business/tests/project-bootstrap.test.mjs +++ b/packages/business/tests/project-bootstrap.test.mjs @@ -119,6 +119,23 @@ test('hold_runs_the_check_under_the_writer_lock_after_recovery', t => { assert.throws(() => f.writer.hold('acme', async () => {}), { code: 'organization-hold-sync' }, 'a hold callback must be synchronous'); }); +for (const rejected of [false, true]) { + test(`hold_async_keeps_writers_busy_until_${rejected ? 'rejection' : 'fulfillment'}`, async t => { + const f = fixture(t), before = f.writer.read('acme').revision, failure = new Error('effect failed'); + const held = f.writer.holdAsync('acme', async ({ revision }) => { + assert.equal(revision, before); + await Promise.resolve(); + assert.throws(() => f.add(), { code: 'organization-busy' }); + if (rejected) throw failure; + return 'done'; + }); + if (rejected) await assert.rejects(held, error => error === failure); + else assert.equal(await held, 'done'); + assert.equal(f.writer.read('acme').revision, before); + assert.ok(f.add(), 'the lock releases on either outcome'); + }); +} + test('a_crash_before_publication_leaves_the_file_and_retry_recovers_the_prepared_receipt', t => { let armed = true; const f = fixture(t, { barrier: step => { if (armed && step === 'prepared') { armed = false; throw new Error('killed'); } } }); diff --git a/packages/project/src/index.mjs b/packages/project/src/index.mjs index 9a4f5cf28..01fc28e82 100644 --- a/packages/project/src/index.mjs +++ b/packages/project/src/index.mjs @@ -192,8 +192,7 @@ function publish(file, bytes) { // One file, one local commit: the lock, the clean-tree and layout checks, publish, commit and the // rollback. The agent writer and the human writer (row 148) differ only in who authors the commit // and in `guard`, which runs under the lock with the file's current bytes (null when absent). -// `around(effect)` (the bootstrap executor's, #802) runs `effect`, the one write that starts the commit, after every -// read above it has finished; it checks its caller's precondition and starts the effect in one synchronous run. +// `around(effect)` (#802) checks freshness after the reads and protects publication through the last git child. async function commitFile({ context, repo, path, bytes, name, subject, guard, around }) { const metadata = lstatSync(join(repo, '.git')); if (!metadata.isDirectory() || metadata.isSymbolicLink()) @@ -216,20 +215,25 @@ async function commitFile({ context, repo, path, bytes, name, subject, guard, ar if (guard) await guard({ before }); if (before?.equals(bytes)) throw new ProjectError('project-record-unchanged', 'record already has these bytes'); head = await git(repo, ['rev-parse', 'HEAD']); - if (around) around(() => publish(file, bytes)); else publish(file, bytes); - published = true; - await git(repo, ['add', '--', path]); - await git(repo, ['commit', '--only', '-m', subject, '--', path], identity(name)); - const commit = await git(repo, ['rev-parse', 'HEAD']); - return { repo, path, commit }; - } catch (error) { - if (published) { - const current = await git(repo, ['rev-parse', 'HEAD']); - if (current !== head) throw new ProjectError('project-commit-uncertain', 'HEAD changed; preserve evidence for reconciliation'); - await git(repo, ['reset', '--quiet', 'HEAD', '--', path]); - if (before === null) unlinkSync(file); else publish(file, before); - } - throw error; + const effect = async () => { + try { + publish(file, bytes); + published = true; + await git(repo, ['add', '--', path]); + await git(repo, ['commit', '--only', '-m', subject, '--', path], identity(name)); + const commit = await git(repo, ['rev-parse', 'HEAD']); + return { repo, path, commit }; + } catch (error) { + if (published) { + const current = await git(repo, ['rev-parse', 'HEAD']); + if (current !== head) throw new ProjectError('project-commit-uncertain', 'HEAD changed; preserve evidence for reconciliation'); + await git(repo, ['reset', '--quiet', 'HEAD', '--', path]); + if (before === null) unlinkSync(file); else publish(file, before); + } + throw error; + } + }; + return await (around ? around(effect) : effect()); } finally { closeSync(fd); if (lstatSync(lock).ino === lockStat.ino) unlinkSync(lock); diff --git a/packages/seat/src/working.mjs b/packages/seat/src/working.mjs index 38961a18e..6e959a82a 100644 --- a/packages/seat/src/working.mjs +++ b/packages/seat/src/working.mjs @@ -81,7 +81,7 @@ // returned: it can carry the URL. import { randomBytes } from "node:crypto"; -import { closeSync, lstatSync, mkdirSync, openSync, readSync, renameSync, rmSync, writeFileSync } from "node:fs"; +import { closeSync, lstatSync, mkdirSync, openSync, readdirSync, readFileSync, readSync, renameSync, rmSync, writeFileSync } from "node:fs"; import { dirname, join, relative, sep } from "node:path"; import { gitEnv, GIT_LIMITS, runGit } from "./checkout.mjs"; import { withoutProcessing } from '../../processing-control/src/effects.mjs'; @@ -120,7 +120,11 @@ export const FSCK = ["-c", "transfer.fsckObjects=true"]; // (checkout.mjs) runs git in its own process group and signals the group. const git = (args, env, { pre = [], limits = GIT_LIMITS, stdin = null, pass = null } = {}) => runGit([...SAFE, ...pre, ...args], gitEnv(env), limits, { stdin, pass }); // git with this call's env and limits, for the helpers below. -const bound = (env, limits) => (args, opts = {}) => git(args, env, { ...opts, limits }); +const bound = (env, limits, dataRoot, repo) => (args, opts = {}) => { + if (dataRoot !== null) ancestors(dataRoot, repo); + inspectWorkingRepository(repo); + return git(args, env, { ...opts, limits }); +}; // The branch name a task's work goes to. A task ref holds a colon, which a // ref name can't. @@ -188,6 +192,21 @@ function ancestors(dataRoot, repo, make = false) { return true; } +// Inspect before git can follow any metadata path, including when bootstrap inspects refs on retry. +// Missing repositories may be initialized; existing ones must contain no filesystem indirection. +export function inspectWorkingRepository(repo) { + if (look(repo) === null) return; + const walk = path => { + const st = look(path); + if (!st || st.isSymbolicLink()) refuse("working-repo-invalid", "working repository contains a link or changed during inspection"); + if (st.isDirectory()) for (const name of readdirSync(path)) walk(join(path, name)); + }; + if (!look(repo).isDirectory()) refuse("working-repo-invalid", "working repository isn't a directory"); + walk(repo); + for (const name of ['objects/info/alternates', 'objects/info/http-alternates', 'commondir']) + if (look(join(repo, name))) refuse("working-repo-invalid", "working repository indirection refused"); +} + // The effect runs as it is unless the caller gives a wrapper. initializeWorking's caller (the bootstrap // executor) passes one that checks, under the organization lock, that its plan is still fresh and starts the // effect in the same synchronous run, so a write here never begins under a plan that went stale while this @@ -197,18 +216,27 @@ const direct = (effect) => effect(); // The bare repository, made on first use in a temp directory beside it and // renamed into place. One that exists must be a real directory, and so must every // directory above it up to the data root. -async function ensure(dataRoot, repo, g, around = direct) { +async function ensure(dataRoot, repo, g, around = direct, claim) { if (ancestors(dataRoot, repo)) { const st = look(repo); if (st !== null) { if (st.isSymbolicLink() || !st.isDirectory()) refuse("working-repo-invalid", `${repo} isn't a directory`); + inspectWorkingRepository(repo); + if (claim !== undefined) { + const file = join(repo, 'mosaic-bootstrap.json'), owned = look(file); + if (!owned?.isFile() || readFileSync(file, 'utf8') !== claim) refuse("working-repo-invalid", "working repository ownership is unproven"); + } return; } } around(() => ancestors(dataRoot, repo, true)); const tmp = join(dirname(repo), `.init-${randomBytes(4).toString("hex")}`); try { - const r = await around(() => g(["init", "-q", "--bare", "--template=", tmp])); + const r = await around(() => { + mkdirSync(tmp, { mode: 0o700 }); + if (claim !== undefined) writeFileSync(join(tmp, 'mosaic-bootstrap.json'), claim, { flag: 'wx', mode: 0o600 }); + return g(["init", "-q", "--bare", "--template=", tmp]); + }); if (!r.ok) refuse("working-repo-invalid", `git init failed: ${r.why}`); around(() => renameSync(tmp, repo)); } finally { @@ -259,7 +287,7 @@ export function push({ dataRoot, business, project, root, taskRef, branch, bundl if (!validBranch(branch)) refuse("invalid-request", "not a branch name"); const ref = `refs/heads/${workRef(taskRef)}`; const repo = workingRepo(dataRoot, business, project); - const g = bound(env, limits); + const g = bound(env, limits, dataRoot, repo); return locked(repo, async () => { if (!header(bundle)) refuse("invalid-bundle", "not a v2 git bundle"); await ensure(dataRoot, repo, g); @@ -309,13 +337,14 @@ export function push({ dataRoot, business, project, root, taskRef, branch, bundl // uses. It makes no work ref, takes no bundle and has no remote to push to. A second call is the // same seed: it moves nothing the first call left. Resolves { repo, commit } with main's commit. // `around(effect)` wraps every write (the directories, the init and its rename, the seed's fetch and -// ref update); it must run `effect` synchronously and return what it returns. -export function initializeWorking({ dataRoot, business, project, root, env = process.env, limits = GIT_LIMITS, around = direct }) { +// ref update); it must run `effect` synchronously and protect it until its return value settles. +// Bootstrap's optional claim is written only in a new, exclusively created init directory, never an existing repo. +export function initializeWorking({ dataRoot, business, project, root, env = process.env, limits = GIT_LIMITS, around = direct, claim }) { if (!PROJECT.test(project ?? "") || !PROJECT.test(business ?? "")) refuse("invalid-request", "not an id"); const repo = workingRepo(dataRoot, business, project); - const g = bound(env, limits); + const g = bound(env, limits, dataRoot, repo); return locked(repo, async () => { - await ensure(dataRoot, repo, g, around); + await ensure(dataRoot, repo, g, around, claim); await seed(repo, root, g, around); const commit = await tip(repo, "refs/heads/main", g); if (!commit) refuse("working-repo-invalid", "main has no commit after the seed"); @@ -331,7 +360,7 @@ export function review({ dataRoot, business, project, root, taskRef, commit, hav if (!PROJECT.test(project ?? "") || !OID.test(commit ?? "")) refuse("invalid-request"); if (!Array.isArray(have) || have.length > HAVE_MAX || have.some((h) => !OID.test(h ?? ""))) refuse("invalid-request", "have must list commit ids"); const repo = workingRepo(dataRoot, business, project); - const g = bound(env, limits); + const g = bound(env, limits, dataRoot, repo); return locked(repo, async () => { try { if (!lstatSync(repo).isDirectory()) refuse("working-repo-invalid"); @@ -384,7 +413,7 @@ export function prepareLand({ dataRoot, business, project, root, taskRef, commit if (!PROJECT.test(project ?? "") || !OID.test(commit ?? "") || !/^[a-z0-9][a-z0-9-]{0,62}$/.test(name ?? "")) refuse("invalid-request"); const work = workRef(taskRef); const repo = workingRepo(dataRoot, business, project); - const g = bound(env, limits); + const g = bound(env, limits, dataRoot, repo); const worktree = join(dir, name); return locked(repo, async () => { try { @@ -447,7 +476,7 @@ export function prepareLand({ dataRoot, business, project, root, taskRef, commit // land.old to land.commit, then record() runs. Resolves { previous, commit }. export function finishLand({ dataRoot, business, project, land, record = async () => {}, env = process.env, limits = GIT_LIMITS }) { const repo = workingRepo(dataRoot, business, project); - const g = bound(env, limits); + const g = bound(env, limits, dataRoot, repo); const main = "refs/heads/main"; return locked(repo, async () => { const at = [`--git-dir=${repo}`]; @@ -469,7 +498,7 @@ export function finishLand({ dataRoot, business, project, land, record = async ( // node_modules link the gate added goes first, so nothing follows it. export function dropLand({ dataRoot, business, project, worktree, env = process.env, limits = GIT_LIMITS }) { const repo = workingRepo(dataRoot, business, project); - return withoutProcessing(() => locked(repo, () => removeWorktree(repo, worktree, bound(env, limits)))); + return withoutProcessing(() => locked(repo, () => removeWorktree(repo, worktree, bound(env, limits, dataRoot, repo)))); } async function removeWorktree(repo, worktree, g) { @@ -488,7 +517,7 @@ export function publish({ dataRoot, business, project, root, taskRef, source, em if (!PROJECT.test(project ?? "") || !OID.test(source ?? "") || typeof email !== "string" || !email) refuse("invalid-request"); const ref = `refs/heads/${workRef(taskRef)}`; const repo = workingRepo(dataRoot, business, project); - const g = bound(env, limits); + const g = bound(env, limits, dataRoot, repo); return locked(repo, async () => { try { if (!lstatSync(repo).isDirectory()) refuse("working-repo-invalid"); @@ -530,12 +559,13 @@ export async function publishRemote({ repo, fork, user, source, dst, token, tls if (url.protocol !== "https:" || !HOST.test(url.host) || !NAME.test(user ?? "") || !OID.test(source ?? "") || !dst?.startsWith(`refs/heads/${user}/`)) refuse("invalid-request"); const secret = String(token).trim(); const net = async (args) => { + inspectWorkingRepository(repo); const pre = [...tls, "-c", "protocol.https.allow=always", "-c", "http.followRedirects=false", "-c", "credential.helper=", "-c", `credential.helper=${helper(url.host, user)}`]; const r = await git([`--git-dir=${repo}`, ...args], env, { pre, limits, pass: `${secret}\n` }); if (r.out.includes(secret) || r.err.includes(secret)) refuse("credential-leak"); return r; }; - const g = bound(env, limits); + const g = bound(env, limits, null, repo); const listed = await net(["ls-remote", "--symref", fork, "HEAD", dst]); if (!listed.ok) refuse("fork-unavailable"); let head = null; diff --git a/packages/seat/tests/working.test.mjs b/packages/seat/tests/working.test.mjs index 3392921c6..8d795276b 100644 --- a/packages/seat/tests/working.test.mjs +++ b/packages/seat/tests/working.test.mjs @@ -168,6 +168,24 @@ test("initializeWorking runs each of its five writes through around, and a refus } }); +// #802 C1: no git child may follow indirection in an existing bare repository. +for (const kind of ['objects_link', 'nested_link', 'alternates']) { + test(`c1_working_refuses_${kind}_before_any_git_child`, async t => { + const w = world(t); + await initializeWorking({ dataRoot: w.dataRoot, business: 'acme', project: 'stack', root: w.root }); + const outside = join(w.dir, 'outside'); mkdirSync(outside); + if (kind === 'objects_link') { + rmSync(join(w.repo, 'objects'), { recursive: true }); symlinkSync(outside, join(w.repo, 'objects')); + } else if (kind === 'nested_link') symlinkSync(outside, join(w.repo, 'objects/info/linked')); + else writeFileSync(join(w.repo, 'objects/info/alternates'), outside + '\n'); + const before = fingerprint(w.repo), touched = join(w.dir, 'git-started'); + const env = wrapGit(w, `touch '${touched}'\nexec "$REAL" "$@"`); + await assert.rejects(() => initializeWorking({ dataRoot: w.dataRoot, business: 'acme', project: 'stack', root: w.root, env }), { code: 'working-repo-invalid' }); + assert.equal(existsSync(touched), false); assert.equal(fingerprint(w.repo), before); + assert.deepEqual(readdirSync(outside), []); + }); +} + test("workRef and validBranch take only what a ref name can hold", () => { assert.equal(workRef("vikunja:12/345"), "work/vikunja-12-345"); for (const bad of ["vikunja:0/1", "vikunja:1", "gitea:1/2", "vikunja:1/2\n", null]) assert.throws(() => workRef(bad), WorkingError);