diff --git a/devlog/_plan/260817_wave5_execution/020_1899_harden_ordering.md b/devlog/_plan/260817_wave5_execution/020_1899_harden_ordering.md index da0984d352..e88391d6bb 100644 --- a/devlog/_plan/260817_wave5_execution/020_1899_harden_ordering.md +++ b/devlog/_plan/260817_wave5_execution/020_1899_harden_ordering.md @@ -41,6 +41,31 @@ their Windows/POSIX split is already correct on `dev`. ## Closure -#1899 cannot merge as-is (DIRTY). Land the one-file residue as a direct commit on -`dev`, then close #1899 with a comment naming the commit, what was taken, and -what #1881 already covered. +#1899 cannot merge as-is (CONFLICTING/DIRTY, head `8ab0aa8d0` — re-verified after a +transient `UNKNOWN` reading). Land the one-file residue as a direct commit on `dev`, +then close #1899 with a comment naming the commit, what was taken, and what #1881 +already covered. + +## Outcome (executed) + +DONE. Two commits on `tests/codex-catalog-writer.test.ts`: + +| Commit | Change | +|--------|--------| +| `fb5ceee35` | bind `temp:`/`harden:`/`publish:`\|`rename:` to one temp path; assert `hardenIndex < publishIndex` | +| `50a057e20` | state the scope limit the review asked for | + +**Red proof.** Forcing the harden index above the publish index fails 4 of 9 tests; +restoring returns all 9 to green. An independent reviewer reproduced this with two +ablations on a scratch copy and found something the plan had not predicted: for the +two backup mutators the index comparison is the **only** detector. `publishNoReplace` +is `linkSync`, so a temp hardened after publication still shares the destination's +inode — `chmod` succeeds, `statSync` reads `0o600`, the leftover-`.tmp` check passes, +and every other assertion agrees nothing is wrong. Only the order disagrees. + +**Scope limit, now written into the test.** `io` is an injected seam, so what is +asserted is production's call order (`src/config.ts:236`, +`src/codex/internal/catalog-writer.ts:147` both run write → harden → publish). +Supplying `io` bypasses `hardenSecretPath`, so this proves hardening is *requested* +on the temp before publication, not that it restricts. The Windows NTFS ACL is +covered in `tests/windows-secret-acl.test.ts`. diff --git a/tests/codex-catalog-writer.test.ts b/tests/codex-catalog-writer.test.ts index a350058976..62d2f24bb7 100644 --- a/tests/codex-catalog-writer.test.ts +++ b/tests/codex-catalog-writer.test.ts @@ -237,13 +237,40 @@ for (const mutator of mutators) { ); expect(readFileSync(path, "utf8")).toBe("new bytes\n"); - // Windows exposes synthesized POSIX mode bits, so stat cannot prove that chmod took effect. - // The recorded harden call still proves every mutator requested the permission transition. - expect(effects.some(effect => effect.startsWith("harden:"))).toBe(true); if (process.platform !== "win32") expect(statSync(path).mode & 0o777).toBe(0o600); expect(readdirSync(targetDir).filter(name => name.endsWith(".tmp"))).toEqual([]); - expect(effects.some(effect => effect.startsWith("temp:"))).toBe(true); - expect(effects.some(effect => effect.startsWith(isBackup ? "publish:" : "rename:"))).toBe(true); + + // Bind the three effects to ONE temp path, and to each other in order. + // + // Unbound `some()` checks — "a temp was written, something was hardened, something + // was published" — hold even when the three touch different files, which is the + // failure they exist to catch. + // + // Order matters as much as membership. Hardening lands on the temp file and + // publishing moves that already-restricted file into place; if publish ran first, + // the destination would sit world-readable for the width of the gap. A set-membership + // assertion passes for that writer too, so the index comparison is what makes this a + // claim about the race rather than about the call list. For the backup mutators it is + // the ONLY detector: `publishNoReplace` is `linkSync`, so a temp hardened after + // publication still shares the destination's inode — the mode check reads 0o600 and + // the leftover-`.tmp` check passes, while the write was briefly exposed. + // + // Scope, stated plainly: `io` is an injected seam, so what is asserted here is + // production's call ORDER (src/config.ts and src/codex/internal/catalog-writer.ts + // both run write → harden → publish). Supplying `io` bypasses the real + // implementations, so this proves hardening is REQUESTED on the temp before + // publication — not that it restricts. The Windows NTFS ACL that does the actual + // restricting is exercised in tests/windows-secret-acl.test.ts, and the POSIX mode + // below is the only half `statSync` can observe (Windows reports 0o666 whatever + // `chmodSync` did). + const tempEffect = effects.find(effect => effect.startsWith("temp:")); + expect(tempEffect).toBeDefined(); + const tempPath = tempEffect!.slice("temp:".length); + const hardenIndex = effects.indexOf(`harden:${tempPath}`); + const publishIndex = effects.indexOf(`${isBackup ? "publish" : "rename"}:${tempPath}->${path}`); + expect(hardenIndex).toBeGreaterThanOrEqual(0); + expect(publishIndex).toBeGreaterThanOrEqual(0); + expect(hardenIndex).toBeLessThan(publishIndex); if (isBackup) expect(result).toBe("written"); }); }