Repository navigation
fix(ops): promotion's frozen copy fills its temp folder entry by entry - #2145
Conversation
Since Node 24.22.0 (nodejs/node#64124) cpSync with errorOnExist throws ERR_FS_CP_EEXIST when the destination folder exists. frozenCopy copied the artefact into the folder mkdtempSync had just made, so every promotion on 24.22 or later was refused before anything was migrated. It now copies each of the artefact's entries to its own new name in that folder, keeping errorOnExist and force: false, so anything already at a name is still refused. The copy is the same private temporary folder as before, closed to others until its entries take their served modes. Listing the artefact by name would follow a link put in its place, which cpSync's own lstat refused, so a source that is no longer a real folder is refused as changed, as it was. Closes #2124 Agent-model: anthropic/claude-opus-5-5 Agent-tool: Claude Code Assisted-by: LLM Signed-off-by: Nathan Mulligan <183608168+Nathan-Mulligan@users.noreply.github.com>
Nathan-Mulligan
added
the
sensitive
|
Director read 8496438: pass Scope: deploy/promotion integrity (the BLOCKED reason), plus sign-in, money and data separation, on d91fc0e..8496438 (
Until this lands, Tuesday's promote steps keep running on Node 24.21.0 by explicit path (register 196). |
|
Finding: S2145-1 Notes: The proof fails on this head on both required Node versions: expected Notes: Independently ran the three required suites, unchanged, on the Mac host using Node 24.22.0 and 24.21.0. At this head each runtime passes all 27 tests. Temporarily restoring only Notes: Two additional proofs pass on both runtimes: private source modes plus umask 077 preserve served directory, ordinary-file and executable-file modes and independence from later store writes; a planted destination entry is refused with Separation: No business, client or person separation is touched by this diff. Coverage: All changed code, its relied-on copy/digest/promotion/trust dependencies and the three required test files were read; all required local runs and reviewer proofs ran. No binary paths or changed test files occur in the range. Merge-group shard 3 remains enforced by the merge gate, not a separate local coverage hold. Notes: Hosted checks on this exact head currently include failed Notes: Proof file is uncommitted at Open findings: 1 (S2145-1) |
|
Dispute: S2145-1 ( The finding is accurate: #2124's done-when includes Evidence: the lane's brief (round 2026-10-18-ops-live-agent-run, briefs/OA-Fix-2124.md, Job step 2) says "Never edit a test to go green and never touch .nvmrc", so the lane may not make this change. The un-pin is its own ticket, #2144, listed under Left undone: in this PR's body. Its done-when is the same line plus the merge group's shard 3 green on the Node setup-node resolves. Keeping it separate also means one PR moves CI's Node for every merge group, and it can be reverted alone: this lane proved Your other proofs (umask 077 with private source modes, a planted destination entry refused with If the orchestrator rules that the brief's line only meant "do not fix this by pinning", the fix is the one line |
Nathan-Mulligan
added
the
ci-next
Why: since Node 24.22.0 (nodejs/node#64124) cpSync with errorOnExist throws ERR_FS_CP_EEXIST on an existing destination folder, and frozenCopy copied into the folder mkdtemp had just made, so
node scripts/ops/promote.mjsrefused every promotion on 24.22 or later.Left undone: #2144
Risk: sensitive (deploy and promotion), low: 8 product lines in one function. The copy is the same 0700 temporary folder and every other step (served modes, digest check, swap) is unchanged. The one behaviour cpSync gave for free, refusing a source that is a link, is now an explicit lstat check.
Closes #2124
Section: ci
What changed
scripts/ops/served-copy.ts:73-82:frozenCopycopies each of the artefact's entries to its own new name in the mkdtemp folder (cpSync(join(src, name), join(copy, name), { recursive: true, errorOnExist: true, force: false })), instead of copying the artefact folder onto the folder that already exists.errorOnExistandforce: falsestay, so anything already at a name is still refused. Product +8/-1, tests 0.lstatSync(...).isDirectory()), else it is refused as changed. cpSync's own lstat of the source refused a link put in the artefact's place;readdirSyncwould follow one (tests/ci/promotion-trust-chain.test.ts"an artefact swapped for a link after selection" holds this).Why not the ticket's
join(copy, 'bytes')shapeBuilt first (3792e6c, never pushed). It passed the three named files but failed three in
tests/ci/promotion-copy-replace.test.tson both Node versions. Those tests inject their faults on the copy atserved/.copy-<x>and its rename from that path, and check that folder stays closed while its entries are set. With the copy one level down, they no longer test what they say. Copying entry by entry keeps the copy at that path, so every test stands unedited.Proofs (Mac arm64, nodejs.org binaries checked against SHASUMS256)
ERR_FS_CP_EEXISTtests/ci+ p12, head 8496438The
tests/cifailures are not this change. Both Node versions:queue-width-and-the-end-of-batch-run.proofhitERR_MODULE_NOT_FOUNDin its.local/self-testcopy during the parallel run, and is 4 / 4 alone on each version. On 24.22.0:production-unit-parks-and-runs-the-worker.proof. On 24.21.0:staging-containmentS0-1. Both of those are Docker:all predefined address pools have been fully subnettedon this Mac's shared daemon. None of the three imports promotion code.tscboth projects, pre-commit): clean on 8496438. Gate and gitleaks: clean.pnpm lint:ratchet: 1141 warnings, baseline 1141, green..nvmrcis untouched (the lane's brief); Put .nvmrc back to 24 once promotion's copy works on Node 24.22 #2144 puts it back to24.Checklist
holds(copy, digest)on the promoter-only copy, which refuses any link inside and any bytes that differ. Before this change the window sat inside cpSync, between its lstat and its read. Now it sits between the lstat andreaddirSync, with the same result.readdirSyncnever returns.or..and a name holds no/, so every target sits directly in the fresh folder.frozenCopydoc comment still holds (same temporary folder, closed until its entries are set); no doc names the copy step.Security review: run against 8496438, no findings
What it checked: link at the artefact's top level and as direct children (copied as links, then refused by the digest); the lstat-to-readdir swap (only promoter-owned real files holding the digest are accepted); names escaping the copy folder; the copy folder staying 0700 until served modes, including 24.22's #65488 mode change; a failed promotion leaving
served/and the link unchanged. Noted, not introduced: a store writer racing link swaps could get files into the copy, whichservedModesopens to others beforeholdsrejects it. Every subfolder was already exposed this way before; this change only adds the top level. The reviewer's suggested hardening is to hash before opening the copy.Review checkpoint
Review record
Reviewer: Sol (Codex)
Model: gpt-6.1-sol
Head SHA: 8496438
Verdict: approve
Code review: no findings
Sol's approval of 8496438 (round 2, rules PASS, open findings 0, open disputes 0; S2145-1 dispute upheld,
.nvmrcun-pin is #2144): #2145 (comment)Director read of 8496438: pass (deploy/promotion), #2145 (comment)