From fb5ceee35f18925a118615b3eb69dc0093f27730 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Mon, 17 Aug 2026 20:56:26 +0900 Subject: [PATCH 1/3] test(codex): bind the writer effects to one temp path, in order The catalog writer tests asserted that a temp file was written, that something was hardened, and that something was published - three unbound some() checks that all hold even when the three touch different files, which is the failure they exist to catch. On Windows that is the only proof available: chmodSync moves the read-only flag alone and statSync keeps reporting 0o666, so real restriction comes from the per-user NTFS ACL rather than a mode. Order matters as much as membership. Hardening lands on the temp file and publishing moves that already-restricted file into place; a writer that published first and hardened after would leave the destination world-readable for the width of the gap, and a set-membership assertion passes for that writer too. Comparing the recorded indices is what turns this into a claim about the race instead of a claim about the call list. Driven red before landing: forcing the harden index above the publish index fails 4 of the 9 tests, and restoring returns all 9 to green. #1899 reached the same binding for this file; its other two files are already covered by #1881, which is why that branch now conflicts. This is the surviving residue, rewritten with the ordering guarantee that neither #1881 nor #1899 actually asserted. --- tests/codex-catalog-writer.test.ts | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/tests/codex-catalog-writer.test.ts b/tests/codex-catalog-writer.test.ts index a350058976..17114eeb4c 100644 --- a/tests/codex-catalog-writer.test.ts +++ b/tests/codex-catalog-writer.test.ts @@ -237,13 +237,30 @@ 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. Windows makes that the only available proof: + // `chmodSync` there moves the read-only flag alone and `statSync` keeps reporting + // 0o666, so real restriction comes from the per-user NTFS ACL, not from a mode. + // + // 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. + 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"); }); } From 50a057e20cd23707bb118443023f23272b67c7cb Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Mon, 17 Aug 2026 21:02:34 +0900 Subject: [PATCH 2/3] test(codex): say what the writer ordering assertion does not prove The review of the previous commit made two points worth writing into the file. The first is why the index comparison earns its place. For the backup mutators it is the only detector there is: 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 in the test agrees that nothing is wrong. Only the order disagrees. The second is the limit. io is an injected seam, so supplying it bypasses the real implementations: this proves production requests hardening on the temp before publishing, not that hardening restricts anything. The previous comment talked about NTFS ACLs in a test that never reaches them, which invites exactly the over-reading this change is supposed to prevent. The ACL is covered in tests/windows-secret-acl.test.ts and the comment now says so. --- tests/codex-catalog-writer.test.ts | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/tests/codex-catalog-writer.test.ts b/tests/codex-catalog-writer.test.ts index 17114eeb4c..62d2f24bb7 100644 --- a/tests/codex-catalog-writer.test.ts +++ b/tests/codex-catalog-writer.test.ts @@ -244,15 +244,25 @@ for (const mutator of mutators) { // // 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. Windows makes that the only available proof: - // `chmodSync` there moves the read-only flag alone and `statSync` keeps reporting - // 0o666, so real restriction comes from the per-user NTFS ACL, not from a mode. + // 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. + // 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); From 02e14a915d212eca40afe9af42589d62a051c4bb Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Mon, 17 Aug 2026 21:03:35 +0900 Subject: [PATCH 3/3] docs(devlog): record the WP2 outcome and what the ablation found The reviewer's ablation turned up something the plan did not predict. For the two backup mutators the index comparison is the only detector there is: publishNoReplace is linkSync, so a temp hardened after publication still shares the destination's inode - chmod succeeds, statSync reads 0o600, the leftover check passes, and every other assertion agrees nothing is wrong. Only the order disagrees. That is a stronger argument for the assertion than the one the plan made for it. Also records the scope limit now written into the test, and re-verifies #1899 as CONFLICTING after it briefly read UNKNOWN while GitHub recomputed. --- .../020_1899_harden_ordering.md | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) 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`.