test(session): pin no-authority nested managed reads on every platform - #4585
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Dependency hold — audited against submitted head
No approval is being asserted while the dependency is open. — |
|
Stacked-PR CI status note. This PR targets Local evidence at exact head
Plan: once #4583 merges, retarget this PR to |
e4824ff to
68ec59a
Compare
|
Dependency hold and lane-ownership receipt:
The hold clears only after #4583 terminally merges or closes; this lane then performs the required exact-dev reconstruction/disposition immediately. — |
68ec59a to
f87f4b8
Compare
f87f4b8 to
bb0fa99
Compare
|
Base fix #4583 has merged to dev as squash — |
ff99612 to
c0985d0
Compare
|
Post-merge reconstruction is complete on the existing PR and branch.
Fresh exact-head independent review is requested; merge remains blocked pending non-author approval and replacement CI. — |
|
Authoritative replacement CI is terminal product-green for exact head Run 31892452530: canonical plan, four GJC state shards and aggregate, native build, Windows session regression, package check/type build, the new pin test, resident-cache regression, CLI smoke, evidence producer/aggregate, and virtual integration all passed. The sole red job is the intentional @probepark @HaD0Yun @IYENTeam — fresh non-author exact-head approval is requested. The diff is one equivalent follow-up commit over merged #4583: explanatory guard comment plus the three-case no-authority nested-read pin. Local negative control restores the rejected guard and fails 2/3 with — |
probepark
left a comment
There was a problem hiding this comment.
Approve at c0985d0a1 - this pins by test what I could only assert by reading
When I approved #4583 I accepted the removal of #assertPathBackedReadRelative on the grounds that
#assertPathBackedDirectoryChain plus captureManagedFileNoFollow subsume it. That was an argument
from reading the source, and I said so - the suite there showed no differential. This closes that
gap properly.
The assertion that matters
it("still rejects a symlinked intermediate directory via the per-component chain walk", async () => {
const { store, baseDir } = createStoreWithoutAuthority();
publishNested(store, "artifacts/kept.txt", "present");
const outside = path.join(baseDir, "outside");
fs.mkdirSync(outside, { mode: 0o700 });
fs.writeFileSync(path.join(outside, "escape.txt"), "attacker");
fs.rmSync(path.join(store.dir, "artifacts"), { recursive: true });
fs.symlinkSync(outside, path.join(store.dir, "artifacts"));
expect(() => store.readExpected("artifacts/escape.txt")).toThrow();
});That is a real escape attempt, not a shape assertion: a genuine directory is replaced with a symlink
pointing outside the store, and the read of a file behind it must throw. Exactly the case the
removed blanket guard used to catch incidentally.
I verified it discriminates rather than trusting it
Deleting the chain walk from the production path:
- this.#assertPathBackedDirectoryChain(resolved);
+ // MUTATED: chain walk removed(fail) ManagedSessionDescendantStore.readExpected nested reads without retained authority > still rejects a symlinked intermediate directory via the per-component chain walk
2 pass 1 fail
Restored, 3 pass / 0 fail. So the test is a live guard: if anyone later decides the chain walk is
redundant the way the blanket check was, CI stops them. That is the property #4583 was missing.
returns null for a missing nested leaf without throwing is the right companion - it pins that
tightening the path checks did not turn a benign ENOENT into an exception, which is the usual
collateral damage when hardening a read path.
merge-approved.
Reviewed by @probepark - method: fresh-worktree run at the exact head, then mutated the production chain-walk call away to confirm the symlink-escape assertion fails without it, and restored to confirm the clean result.
Linux CI retains native root authority, leaving the Darwin no-authority read path unexercised. Pin the nested read contract under spoofed construction so regressions fail on every platform. Lore-id: pr-4585-test-only-reconcile Constraint: test must exercise the authority-absent nested read contract Confidence: high Scope-risk: narrow Reversibility: trivial Tested: pending targeted validation
c0985d0 to
3d76e44
Compare
|
gajae.reconcile-evidence.v1 base:6696988b6dd757f898ab7b33984ba830b6344109 head:3d76e44be757a534e9c62d804b5fe637c1bf9661 sha256:9662d2625c12219813865118af5aeb1fd09373a6c9f2c243048a5aa5cec20a4d scope:test-only local:bun-test-managed-nested-read-pin-3-pass-and-coding-agent-check-pass approval:probepark-exact-head-requested |
|
dev advanced pre-merge — rebase/reconstruct executed, stale review/CI invalidated. dev moved Reconstruction:
The prior exact-head approval (probepark review 4956988640 @ — |
What
This refresh contains one test file only:
packages/coding-agent/test/session/managed-nested-read-pin.test.ts.It pins the authority-absent nested
readExpectedcontract on every CI platform by spoofing Darwin only while constructing the store. The production source comment fromc0985d0a1was intentionally excluded; this PR does not modify production code.Exact-head evidence
6696988b6dd757f898ab7b33984ba830b6344109(origin/dev)3d76e44be757a534e9c62d804b5fe637c1bf96619662d2625c12219813865118af5aeb1fd09373a6c9f2c243048a5aa5cec20a4dbun test packages/coding-agent/test/session/managed-nested-read-pin.test.ts— 3 pass, 0 fail.bun --cwd=packages/coding-agent run check— pass (Biome and TypeScript).Approval gate
The prior
probeparkapproval was forc0985d0a1and is not valid for this refreshed head. An independently authenticated exact-headprobeparkreview has been requested. No self-approval is used.gajae.pr-review-verdict.v1 merge-approved sha256:9662d2625c12219813865118af5aeb1fd09373a6c9f2c243048a5aa5cec20a4d reviewer:human reviewer-id:probepark evidence:independent-architect-review-cross-platform-nested-managed-read-pin-non-tautological-public-readExpected-contract-symlink-boundary-retained-no-global-mutation