Skip to content

Tests: stateDir() refuses the real state dir in a test run; a lint that tests guard machine paths with onRealMachine (#815) [PROTECTED: owner merges] - #818

Merged
jayden1711 merged 16 commits into
mainfrom
issue-815-state-dir-refusal
Oct 20, 2026
Merged

jayden1711 merged 16 commits into
mainfrom
issue-815-state-dir-refusal

Conversation

@jayden1711

@jayden1711 jayden1711 commented Oct 19, 2026 •

Copy link
Copy Markdown
Owner

Intent: in a test run, stateDir() now refuses a state dir outside the test machine, the way slotsDir() already does (#669). testStateRefusal throws TestStateRefused before anything is opened, so a test that lost its isolation fails loudly instead of reading or writing the person's own state. Outside a test run nothing changes. A new lint over test/ makes sure every test that creates, writes or removes an engine-resolved machine location calls onRealMachine on it first. The lint works through the compiler's symbols, and it follows loops, locals, helpers and destructuring. It found 44 unguarded writes in 5 test files, owner-terminal-removed.test.ts among them, and each one now has a guard. (The first version of this body said 45, with plugin-approval at 5. False through 55343d3, corrected in round 2: 45 was measured with an earlier lint that counted an O_RDONLY open in readAllowlist as a write; head's lint counts 44, measured below.)

By-design protection (and what is only a backstop)

Under the owner's durability rule (2026-10-19), every new protection must be by design, and a source lint may only be a named backstop.

  • The protection, by design (a run-time check). In a test run, stateDir() refuses at run time anything outside the test machine: testStateRefusal, including its refusals of a .. part and of a symlink inside the machine. slotsDir() does the same through testSlotsRefusal (main's, now with the same .. and symlink refusals). For machine locations (MACHINE_PATHS.*, SYSTEM_SLOTS.*, etcPath() …), the protection is main's existing preload system root, which moves each of them into the test machine.
  • The backstop. test/machine-path-writes.test.ts and its // GAP fixtures. It catches a test that writes a machine location without checking it with onRealMachine first. It is not the protection, and it has named gaps (below).
  • Named gaps, with no run-time refusal behind them:
    • Writes to the paths the preload never moves: asRoot.*, SYSTEM_ROOTS.* and what onRealMachine returns. Only the lint covers these.
    • a12/a13: join('/', onRealMachine(X)), also through a parameter defaulting to '/', slips past the lint's "under a test's own root" exception. GAP fixture g13, missed (measured).
    • s1/s2: a write that climbs out of stateDir() or slotsDir() through .. parts (join(stateDir(), '..', '..', 'x')). The run-time refusal checks the dir itself, not paths built on it, and the lint leaves the state and slot families to that refusal. GAP fixture g14, missed (measured).
    • a7/a8: literal real paths ('/var/tmp/agent-slots/x', join(homedir(), …)). These are outside the lint's model. On Linux the preload's HOME move covers a8 (inferred); on macOS HOME isn't moved.

Final check (head a0eb941)

Answers issuecomment-6276968449; the code itself is unchanged.

Round 4 (head d916730): claims, not a chase

Answers the round-3 review (issuecomment-6275775507). One commit, 89218df, which changes only test/machine-path-writes.test.ts.

Adversarial pass, round 3 (the attacks on the round-3 rules)

The .. check. Measured by the round-3 review (r5-attacks.mjs): 38 dirs × 3 calls (state with root, state rootless, slots).

  • Every .. form is refused:
    • link/.., link/../, link/../slots, link/../../u/slots, link/x/../..;
    • a leading ../ with a relative dir, and a bare ..;
    • machine/../machine/u/state, real/../real, a trailing /.. with no link.
  • A link with no .. is refused as a symlink: link, link/, link/., link/./slots, link//slots.
  • No false refusals: u/.state, u/..state, u/state.., u/... and u/a..b are allowed for all 3 calls.
  • Outside a test run, nothing is checked, as before.

uid 0. Measured by the round-3 review with fault injection: with the line disabled, the as root assertion fails, so the test reaches it.

The "surely" rule:

Attack Result Source
m1 to m5: a branch, a reassigned local, a reassigned for-of let, a yield element, a write through a reassigned for-of let Reported Measured, fixtures
Const aliases of the same location, 9 shapes (an alias guarded then the original written and the reverse, an alias of an alias, …) No false positive Measured by the review
{ p: x } = c ? {…} : {…} and { q: x } = { q: Y, p: … } Reported Measured by the review
Array destructuring of a branch, or of another index Missed: GAP Measured, fixtures g1, g2
A for-of guard and a write to another element in the same body Missed: GAP Measured, g3. A write after the loop is still covered once per element.
A property reassigned before the guard; a src object's property mutated Missed: GAP Measured, g4, g5
resolve(X, <absolute>), join(X, '..', …) as the guard Missed: GAP Measured, g6, g7
A write through an object or class getter Missed: GAP Measured, g8, g9
A guard through a getter (onRealMachine(o.p)) Not taken as a guard Measured by the review. This fails closed.
onRealMachine(String(X)) Not taken as a guard Measured by the review. This fails closed (the callee is likely StringConstructor's nameless signature, inferred).

Round 3 (head 5a20d35)

Answers the round-2 review (issuecomment-6274220846). Commits:

Fail-without is on 692368f.

Round 2 (head 692368f)

Answers the round-1 review (issuecomment-6272854895). Commits: ebf6dab (src), 7e89673 and 692368f (lint). Fail-without is on 55343d3.

  • R1, guard holes (fixed). A guard counts only when it always runs before the write. It must come first in a list of statements that holds the write (a block, the file, a case clause), directly or inside plain { … } blocks of that list. It never counts out of:

    • a loop body, which may run zero times or break first;
    • an if or else branch;
    • a try, catch or finally block;
    • another case, a labelled block or a function;
    • a hoisted function declaration that holds the write, since it can be called before the guard (found by my adversarial pass).

    ||=, &&= and ??= are short-circuits. One loop is let through: a for-of over a non-empty array literal (no spread) whose body never breaks, continues or returns. It runs its body once per element, every time, as in Slot dir: a slot path is an opaque SlotPath that only writeSlotFile and the slot readers take; tsc rejects it at any other writer [PROTECTED: owner merges] #806's for (const p of [A, B, C]) onRealMachine(p); in slot-path.test.ts.

  • R2, the never-moved real paths (fixed). asRoot.* and SYSTEM_ROOTS.* are reported like onRealMachine's result.

  • R3, tracing (fixed).

    • A callback's parameters are read as anything its call is given: the receiver and the other arguments. That covers .then, Array.from, new Promise and array methods.
    • A generator's yields are traced, and so are destructuring assignments ([d] = …, ({ e } = …)).
    • An open counts as a read only when its flags are node's read-only constants, checked by symbol (O_RDONLY with O_NOFOLLOW, O_NONBLOCK and the like, |, ??, 0). Anything else, a variable included, is a write: fail closed.
    • Still not traced, and listed in the lint's header and under Adversarial pass: a Map or Set, iterators other than a generator's yields, .call/.apply/Reflect.apply, and fs reached through createRequire or import().
  • R4, the principle-9 section is below. The rootless path now also refuses as root and under systemd (INVOCATION_ID, JOURNAL_STREAM), as the root's mark does.

  • R5, symlinks (fixed, both dirs). outsideTestMachine lstats each part of the dir that exists inside the test machine, from the machine down. It never follows one and never looks outside the machine, and it stops at the first part that doesn't exist. A symlink among them is refused, whether it points outside or inside. This is a tightening for the state and the slot dir alike.

  • R6 (corrected). The count is 44, as above.

Fail-without, round 2 (measured). I ran the round-2 tests in a scratch worktree at 55343d3 (never committed).

  • The round-2 lint fixtures, fed to 55343d3's lintFixtures, miss 16 cases:

    • 8 control-flow shapes: an empty loop, a break first, the other branch, ||=, &&=, ??=, a continue first, a spread;
    • 8 traces: asRoot, SYSTEM_ROOTS, .then, Array.from, a generator, two destructuring assignments, flags with a variable.
  • The new state-dir-refusal.test.ts fails on 55343d3's src in two places:

    • a link out, then below it, the symlink test;
    • INVOCATION_ID, the rootless refusal.

    Its other 3 tests pass there, as they should.

  • At head, the lint over test/ with the 5 guarded files put back to 178c44e fails with 44 findings: managed-apps 25, owner-terminal-removed 7, plugin-run 7, plugin-approval 4, init 1.

  • The s25 hoisting fixture failing on 7e89673 is inferred. What I measured is the change in behaviour: fixture-b line 13, a write in a function declaration guarded at module level, became reported.

After (measured, head 692368f, scratch HOME, state, held-out dir and TMPDIR):

  • machine-path-writes 8/8 and state-dir-refusal 5/5.
  • isolate, slots, slot-path, slot-dir-writes, update-drain, hub-setup, dashboard-hub, owner-terminal-removed, managed-apps, init, plugin-approval and plugin-run: 217 tests, 215 pass. The 2 failures are the same two isolate tmpdir-length tests as in round 1, failing only because my scratch TMPDIR is long.
  • Both typechecks are clean, and check-denylist reports 1307 files clean.

Principle 9: the service-mode machine mark (an exception, owner-accepted 2026-10-19)

  • The check it exempts. outsideTestMachine requires the system root inside the test machine, and the root's mark for this run's token (systemRootRefusal). That requirement includes systemRootRefusal's refusals as root (uid 0) and in a process systemd started (INVOCATION_ID, JOURNAL_STREAM). The exception applies to the state dir only: testSlotsRefusal still requires the root, and its message is byte-identical (measured, by test).
  • Why that check's finding is wrong for a service-mode child. Service modes (dashboard --hub, --hub-setup) never run with the system root, by design (serviceModeRefusal). So a test's service-mode child has no root to show, yet its state dir is still inside the test machine. Without the exception, hub-setup.test.ts's two CLI tests fail with TestStateRefused (measured in round 1).
  • What it accepts instead. The machine directory's own mark: <machine>/.test-isolation, a file owned by this user alone, holding the token in WORKLANE_TEST_ISOLATION. test/isolate.ts writes it next to the root's mark.
  • Since round 2, the rootless path also refuses as root and under systemd markers, as the root's mark does. The systemd case was measured by test in round 2. The uid-0 case was not: through 692368f the machine-owner check refused first, so no test reached that line (the round-2 review). Since 29e8fc5 both checks run before the machine is looked at, and the uid-0 case is measured by test.
  • Native Windows has no process.getuid, so there, as on the root's route, there is no root check. The owner and mode checks are also skipped on win32, as on the root's route (inferred, read). Native Windows runs no agents (canRunAgents) and isn't in CI.
  • Forgery. One planted file forges it (measured by the round-1 review): a directory owned by this user alone, plus a mark holding the env's token. Only a test doing that on purpose gets through, which is within the existing accident threat model. Nothing in src/ or test/ writes TEST_MARK_FILE outside its own machine (inferred, grep).
  • The token. serviceModeEnv keeps TEST_TOKEN_VAR only for this exception. Without a root, the token moves nothing else (inferred, grep of src/: read only by testRunSign and the mark check).
  • What it reopens. Nothing main had closed: main has no state refusal at all, and the slot refusal is unchanged (inferred).
  • Acceptance. The owner accepted this exception with the round-2 tightenings on 2026-10-19, as relayed by the coordinator.

Risk class: protected. The diff touches src/os/index.ts and src/os/system-paths.ts (engine), .worklane/review.yaml (engine), and test/isolate.ts and test/managed-apps.test.ts (engine-guards). The owner merges. Since round 4's merge of main d5674d0, test/package-files.test.ts is no longer in the diff: its PACKAGE_BUDGET line is main's, unchanged.

Closes #815

What changed

  1. stateDir() (src/os/index.ts, only the stateDir hunk): it resolves the same way as before (now in unguardedStateDir), then calls testStateRefusal(dir) and throws TestStateRefused if that refuses.

  2. testStateRefusal (src/os/system-paths.ts): it applies the same rule as testSlotsRefusal, and both now share outsideTestMachine. The slot refusal's message is byte-identical: a test asserts the full string.

    • One addition, for the state dir only. A service mode that a test starts (dashboard --hub, --hub-setup) runs without the system root (serviceModeEnv), so it can't prove its test machine through the root's mark. In that case only, the machine can prove it with a mark of its own: the same token in <machine>/.test-isolation, a file owned by this user alone. test/isolate.ts now writes that mark, and serviceModeEnv keeps the token (without a root, the token moves nothing). The slot dir still requires the root.
  3. The lint (test/machine-path-writes.test.ts). It builds a ts.Program over test/ and uses the type checker.

    • A write is a call that resolves to an fs create, write, move or remove (node:fs or node:fs/promises, under any name, destructured or aliased). It is also a call to a src/ or test/ function that writes one of its parameters, or an option of one. Those functions are found from the code, including writers that call other writers.
    • The target is traced back through:
      • locals and later assignments;
      • for-of variables and array callbacks (the loop forms);
      • destructuring, object properties and array elements;
      • parameter defaults;
      • path builders, other calls' arguments, and a test helper's return value.
    • A machine location is any src/ declaration that is built from systemPath on every branch. The key names the declaration and the property, so aliases and namespace imports count as the same name. Examples: MACHINE_PATHS.hubUnitFile, SYSTEM_SLOTS.dir, CONSOLE_SHELL, etcPath(), storeDir(), allowlistPath(). env ?? varLibPath(…) and dir === system.dir ? FILE : join(dir, …) don't count, because in a test they are the test's own paths, which onRealMachine would refuse.
    • A guard is an onRealMachine(x) call on the same name that always runs before the write. Since round 2 that means: first in a list of statements holding the write, directly or through plain blocks. It never counts out of a loop body (except a for-of over a non-empty literal with no break, continue or return), an if or else branch, a try, catch or finally block, another case, a labelled block or a function, or into a hoisted function declaration. A guard inside a condition, behind a short-circuit or logical assignment, or in an optional call doesn't count either. (Round 1 also let a guard out of a for-of body or a try block. That was false: the round-1 review measured holes, now fixed.)
    • The state and slot dirs can't take onRealMachine, because neither is inside the system root. Their resolvers refuse in a test run instead. The lint checks that stateDir() and slotsDir() still call their refusal and throw (directly or through a function they call, as slotsDir → resolvedSlotsDir does since Slot dir: a slot path is an opaque SlotPath that only writeSlotFile and the slot readers take; tsc rejects it at any other writer [PROTECTED: owner merges] #806). If either stops, every write built from it is reported. This is a new lint, so it exempts nothing that existed before.
    • Writing what onRealMachine returns is reported, because that value is the real path. The exception is where it is only a later part of join(root, …), i.e. under a test's own root.
  4. The guards it found (each onRealMachine(...) placed before the write):

    File Writes Notes
    owner-terminal-removed 7 oldInstall lines 39-44 (loop form) and 136
    managed-apps 25 asRootsFile, withUpdates and two tests
    init 1 srvCheckoutPath
    plugin-approval 4 (said 5: false through 692368f, see R6)
    plugin-run 7
  5. .worklane/review.yaml: the two new tests are added to engine-guards, next to slot-dir-writes. This is a tightening.

How it was verified

  • Fail-without, refusal (measured). I put the base commit 178c44e's src/os/index.ts, src/os/system-paths.ts and test/isolate.ts back, built, and ran test/state-dir-refusal.test.ts. 3 of its 4 tests fail:

    • the test-run fall-throughs: WORKLANE_STATE_DIR outside the test machine: no state dir, with stateDir() returning the stand-in …/tmp/not-the-test-machine-84527c33/state;
    • the in-process test: Missing expected exception;
    • the pure test: TypeError: testStateRefusal is not a function.

    The production-path test passes there and here, which is the point: outside a test run nothing changed. In the same run, hub-setup.test.ts passes on the old code (12 of 12).

  • Fail-without, lint (measured). I put the 5 guarded test files back to their pre-guard versions, built, and ran the lint test. With round 1's first lint it failed with 45 findings, plugin-approval at 5. With head's lint it fails with 44 (owner-terminal-removed 7, managed-apps 25, init 1, plugin-approval 4, plugin-run 7; measured in round 2). The first one: test/init.test.ts:328: mkdirSync(dirname(checkout)): srvCheckoutPath is a machine location (src/os/system-paths.ts): call onRealMachine on it first. After merging main (Slot dir: a slot path is an opaque SlotPath that only writeSlotFile and the slot readers take; tsc rejects it at any other writer [PROTECTED: owner merges] #806), the lint also reported test/slots.test.ts:91. That write was guarded by assert.equal(onRealMachine(devSlotsPath()), …), which the lint now accepts as a guard (55343d3).

  • After (measured), at head 55343d3 with a scratch HOME, state, held-out dir and TMPDIR:

    • machine-path-writes.test.ts: 6/6 pass. The lint over all of test/ takes about 12 s.
    • state-dir-refusal.test.ts: 4/4 pass.
    • isolate, slots, slot-path, slot-dir-writes, update-drain, hub-setup, dashboard-hub, owner-terminal-removed, managed-apps, init, plugin-approval and plugin-run all pass except two isolate tests (the temporary directory is inside the test machine… and a test run a test starts…). Those two fail only because my scratch TMPDIR is long: Chrome's socket path came to 163/175 bytes, over the 107-byte limit (measured). They are environmental, not caused by this change. CI uses /tmp.
    • tsc -p tsconfig.json --noEmit and tsc -p web/tsconfig.json: clean.
    • check-denylist: denylist: 1307 files clean.
  • Full suite: not run locally. I started fullsuite.sh and then stopped it within about a minute. The log shows only the build step and no test result, and no process was left over. I stopped it because npm test includes test/mcp-broker-unit.test.ts, which spawns test/fixtures/plugins/escape/server.js, and the builder rules forbid running that on this machine. CI runs the full suite.

  • Rule A. No test or probe here reads, stats or writes a real state dir, /var/lib or /etc.

    • Every directory outside the test machine is a stand-in: a path beside the test machine that is never made.
    • Each fall-through runs in a scratch child behind wrappers on node:fs, node:fs/promises and node:child_process. Before the real call, the wrappers refuse any path outside the test machine and the engine's own dist/src modules, and they record every path they see.
    • Positive controls in every child: the probe's own existsSync and the engine's fileAccess (statSync through its named import) are both recorded, and a path outside is refused.
    • Outside a test run, resolving the state dir opened nothing: only the 3 control entries were recorded (measured).

Engine spawns that build their env from scratch (item 3)

I found these with a scratch AST scan of src/, test/ and scripts/ for spawn/exec/fork calls whose env is an object literal (or a same-file const) with no spread of an existing env. It found 75 (measured). Spawns of the engine itself:

Where Env Notes
test/isolate.test.ts:249, :254 (probe and cli slots) {PATH, HOME: mkdtemp, AGENT_SLOTS_DIR} Deliberately not a test run (it checks the engine outside one). The state dir would resolve under the scratch HOME (inferred).
test/init.test.ts:465 (cli init) HOME and WORKLANE_STATE_DIR both mkdtemp Skipped on POSIX, so never run in CI (measured: { skip: posix }).
test/managed-apps.test.ts:286 (node -e importing the engine) scratch HOME plus the test-run marks A test run, so the refusal applies (inferred).
test/state-dir-refusal.test.ts (new) stand-in HOME Nothing opened (measured).
scripts/machine/worklane-machine.cjs:612 (the root helper's cli plugins) {PATH, HOME, LANG, SUDO_USER} Production. Unchanged, since outside a test run nothing is refused (inferred).
  • Not engine spawns (inferred, from reading each command): every src/ hit is git, npm, tar, systemd-run or sudo <helper>. The test/ hits run bash scripts, stand-ins, node scripts/ci/batch.mjs or new-deps.mjs (neither imports dist/src), and the escape fixture's server.js (test/mcp-broker-unit.test.ts:208, not run here).
  • HOME. Every bash script that could run the CLI gets a scratch HOME.
  • Missing HOME (inferred). No engine spawn found here lacks a HOME. Without one, os.homedir() falls back to the user database, i.e. the real home.
  • Limit (inferred). The scan sees literal envs and same-file consts only. An env built by a helper (unisolatedEnv, serviceModeEnv) keeps the parent's variables and isn't in the list.

What this can never do

  • Change anything outside a test run (measured for the three Linux fall-throughs; inferred for macOS and Windows, whose branches are unchanged). testRunSign is null for the engine, so the refusal returns before it looks at anything.
  • Make a test machine out of a stray WORKLANE_TEST_MACHINE (measured). A home directory without the preload's root, or without a mark of its own holding this run's token, is refused.
  • Let the slot dir take the machine's own mark (measured). Without the root, the slot dir is still refused.
  • Pass the lint with a guard that may not run before the write, in the 25 control-flow shapes in the round-2 fixtures (measured). False through 55343d3: the round-1 review measured an empty loop, a break, and ||=/??= passing.
  • Lead the state or slot dir out of the test machine through a symlink inside it: a symlink at any depth is refused since ebf6dab, and a .. part since 29e8fc5 (measured, by test). False through 692368f: the round-2 review measured <link>/../slots passing, because the check walked the path after resolve() had folded link/.. away.

Compromised component

  • An agent or contributor editing tests could:
    • remove a guard and add a write, which fails this lint;
    • weaken the lint itself, which is now engine-guards, so the owner reviews it;
    • write through a form the lint doesn't trace (see Adversarial pass). The other layers still hold: the preload's system root, helpers.ts stopping an unisolated file, and the state and slot refusals.
  • The coordinator, the dashboard, other instances, the hub viewer: nothing new. The change runs only in a test run.
  • Someone who can set the engine's environment could set WORKLANE_TEST_RUN=1 on a real engine, which makes stateDir() and slotsDir() throw for the real paths. That fails closed: the engine refuses to start; it doesn't widen anything. It is the same as slotsDir() today (inferred).
  • Someone with push access: the same as today.

Adversarial pass

Every attack below was run as a fixture or probe (measured) unless marked otherwise.

Attack Result Fixed because of it
Loop forms: for (const f of [A, B]) rmSync(f), [A].forEach((p) => writeOwnerOnly(p)) Reported
Aliases: destructured fs (const { appendFileSync: add } = fs), fs/promises, a namespace import (removed.CONSOLE_SHELL) Reported
Indirect values: a later assignment (let x; x = unitPath(...)), a test helper that removes its parameter, an object's property, an array element, a destructured name Reported
A parameter's default ((p = SYSTEM_SLOTS.config) => rmSync(p)) Was missed Now traced
Writing onRealMachine(...)'s result Reported
join(root, onRealMachine(...)) Not reported (it is the test's own root)
A guard under if, behind &&, in an optional call ?.(…), in another function, after the write, or on another name Each still reported
assert.equal(onRealMachine(x), …) Was not taken as a guard (slots.test.ts:91 was reported) Now taken as one
An O_RDONLY | O_NOFOLLOW open (readRegularFile) Was read as a write Now a read
stateDir()/slotsDir() without their refusals (fixture copies of src/os/index.ts, nothing in src/ edited) Every write built from them reported
State-dir refusal: .. in the dir Normalized and refused (join(testMachine, '..', 'x') measured)
The machine directory itself as the state dir Refused
A stray machine with no mark, a wrong token, a missing token Refused
A service-mode child Inside: allowed. Outside: refused.

Round 2: guards that look valid but don't protect a later write. Every shape below is in the fixtures, fed to lintFixtures (measured).

Shape Reported now On 55343d3
Empty for-of yes missed
Break before the guard yes missed
Continue before the guard yes missed
A spread literal yes missed
The other if branch yes missed
||=, &&=, ??= yes missed
while yes yes
do … while (runs once; not taken, conservative) yes yes
If, else yes yes
Try with a swallowing catch yes yes
Catch yes yes
Finally (not taken, conservative) yes yes
A case falling through yes yes
A labelled block with a break yes yes
A callback with a return yes yes
A ternary yes yes
An IIFE yes yes
A .then callback yes yes
After the write yes yes
A for-of over a non-literal yes yes
A condition inside a loop body yes yes
A hoisted function declaration called before the guard yes not run

Valid shapes stay unreported (measured): the same case, a plain block, the same branch, the same loop body, a plain assignment, a for-of over a non-empty literal (with or without braces), and an arrow defined after the guard.

Gaps left, each inferred:

  • A state dir that is a symlink inside the test machine pointing outside it. Closed in ebf6dab for the state and slot dirs (R5).
  • A Map or Set, an iterator other than a generator's yields, and Reflect.apply.
  • An engine child started with an env from scratch carries no test-run sign (markTestRun's LIMIT). Every such spawn found sets a scratch HOME (table above).
  • An engine function that writes a machine location of its own when a test calls it: the test names no path. The preload's system root still moves the location.
  • Writes through createRequire(...)('node:fs'), .call/.apply, a class field (this.p), a Map, or a child process or shell script.
  • A guard on a different value that mentions the same name. A guard on unitPath('a') also covers unitPath('b'), since keys are per function.
  • A guard whose argument only may be the location. The round-2 shapes are traced since 32f74e8: a branch, a reassigned local or for-of let, a yield element, and an assignment to a for-of let as a write target. "Both traced since 32f74e8" was false through 5a20d35 for the class. Six shapes are still missed, each a GAP fixture since 89218df:
    • array destructuring of a branch, and of another index;
    • a for-of guard covering a write to another element in the same body;
    • a property reassigned before the guard (also through an alias, Object.assign or Object.defineProperty);
    • a src object's property mutated (MACHINE_PATHS isn't frozen);
    • resolve(X, <absolute>) or join(X, '..', …) as the guard.
  • A write through a getter, on an object or a class. A GAP fixture since 89218df.
  • A check-then-swap race: a test that replaces a directory with a link between stateDir()/slotsDir() and the open. It needs a deliberate shape, so it sits inside the accident threat model; the round-2 review accepted it.

Overlaps

New dependencies

None.

Proposed follow-up (not in this PR)

A lint that every spawn of the engine in test/ either passes the parent's env (childEnv()/process.env) or sets HOME and WORKLANE_STATE_DIR to scratch. That would close the from-scratch-env route by check instead of by survey.

…chine, as slotsDir() does

#815. stateDir() fell through WORKLANE_STATE_DIR, then XDG_STATE_HOME, then the home, and nothing refused in a
test run: a test file run without the preload, or an engine child given a real HOME, resolved the person's own
state dir. It now throws TestStateRefused first (testStateRefusal), by the same rule as the slot dir (#669):
only a directory inside the test machine, which must hold the preload's system root and its mark for this run.
testSlotsRefusal and testStateRefusal share that code; the slot refusal's text is unchanged.

A service mode a test starts runs without the system root (serviceModeEnv), so the machine shows it is this
run's with a mark of its own (the same token, test/isolate.ts writes both). Only the state dir takes it; the
slot dir still needs the root. serviceModeEnv keeps the token for that: without a root it moves nothing.

Outside a test run nothing changes: the refusal returns before looking at anything.

Test (fails without): test/state-dir-refusal.test.ts. Each fall-through (WORKLANE_STATE_DIR outside,
XDG_STATE_HOME outside, HOME a stand-in real home) runs in a scratch child behind fs and child_process wrappers
that refuse any path outside the test machine, with positive controls.

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
… onRealMachine first, and the guards it found

#815. owner-terminal-removed.test.ts wrote and removed MACHINE_PATHS.hubUnitFile and CONSOLE_SHELL with nothing
checking they were the test machine's. The class: a lint over test/, through the compiler's symbols, that finds
every call that creates, writes, moves or removes a path (node:fs, node:fs/promises, or a src/ or test/ function
that writes one of its parameters) whose target traces to a machine location in src/ (anything built from
systemPath on every branch: MACHINE_PATHS.*, SYSTEM_SLOTS.*, CONSOLE_SHELL, etcPath(), storeDir(), ...), through
locals, later assignments, for-of variables, array callbacks, destructuring and test helpers. Each needs an
`onRealMachine(x);` statement on the same name before it, in its block or one around it. The state and slot dirs
can't take onRealMachine (neither is in the system root): their resolvers refuse in a test run, and the lint
checks they still do. A write of what onRealMachine returns (the real path) is reported too.

It found 45 unguarded writes in five files: owner-terminal-removed (7), managed-apps (25), init (1),
plugin-approval (5) and plugin-run (7). Each now calls onRealMachine first. The two new tests are listed in engine-guards (review.yaml).

Test (fails without): test/machine-path-writes.test.ts. On the tests before this commit it reports those writes;
its fixture cases cover each form it traces and each guard it refuses.

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
…ads O_RDONLY opens as reads, and follows a parameter's default

#815, after merging main (#806). test/slots.test.ts guards devSlotsPath() with assert.equal(onRealMachine(...), ...):
that call runs whenever its statement does, so it counts as a guard. Behind a condition, a short-circuit, an
optional call or in another function it still doesn't. An open whose flags name O_RDONLY and no write flag
(readRegularFile, through src/slot-file.ts since #806) is a read, not a write. A parameter's default
(`(p = CONSOLE_SHELL) => rmSync(p)`) is traced like an argument.

Test (fails without): test/machine-path-writes.test.ts. On the merge before this commit, the lint over test/
reports test/slots.test.ts:91 (a guarded write). New fixture cases: a guard in a const's initialiser, a
short-circuit and an optional call that are no guard, a parameter's default, a destructured name.

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711

Copy link
Copy Markdown
Owner Author

Evaluator verdict: CHANGES NEEDED (round 1)

Reviewed head 55343d3 against #815. I built it in my own worktrees (head, origin/main 128453a with the new tests, and head with the five guarded files put back to main), with a scratch HOME, state dir and held-out dir and a stand-in claude first on PATH. Every attack below used stand-ins under the preload's test machine or names that were never created. Nothing read, statted or wrote a real state dir, /var/lib or /etc.

Acceptance criteria (#815)

# Criterion Status Evidence
1 stateDir() has a testStateRefusal that mirrors testSlotsRefusal and throws before anything is opened Partly met The refusal works on every fall-through (measured: state-dir-refusal.test.ts passes 4/4). It is not a mirror, though: the state dir also accepts a rootless machine mark (see the loosening below). A symlink inside the machine that points outside still passes (measured, R5).
2 One test per fall-through, each in a scratch child with stand-ins, never touching real state Met Test 1 covers WORKLANE_STATE_DIR, XDG_STATE_HOME and HOME behind fs/child_process wrappers, with positive controls (measured: passes).
3 List the engine spawns that build their env from scratch Met PR body table (inferred; I spot-read the test/ rows).
widened A lint, including loop forms, so every test that writes an engine-resolved machine path calls onRealMachine first Partly met The lint works and found real writes (measured). It has guard holes that contradict the PR's own "can never do" claim (R1), and it misses writes to the never-moved real paths (R2).
constraint No change outside a test run Met Measured (below).

Fail-without (measured)

  • The refusal. I put the PR's two new tests onto origin/main, built (one TS2339 error, for the new export; tsc still emits) and ran node --import ./dist/test/isolate.js --test dist/test/state-dir-refusal.test.js. 3 of 4 fail:

    • WORKLANE_STATE_DIR outside the test machine: no state dir
    • Missing expected exception.
    • TypeError: testStateRefusal is not a function

    The production-path test passes on both, as expected. At head all 4 pass.

  • The lint. At head, with the five guarded files put back to main, machine-path-writes.test.js fails with 44 findings: managed-apps 25, owner-terminal-removed 7, plugin-run 7, plugin-approval 4, init 1. The first is test/init.test.ts:328: mkdirSync(dirname(checkout)): srvCheckoutPath is a machine location. The PR body says 45, with plugin-approval at 5 (see R6). At head the lint passes 6/6.

Production unchanged (measured)

In a child with an env built from scratch (no test-run sign, stand-in HOME), I compared stateDir() from main's build and this PR's build. All five cases matched exactly: WORKLANE_STATE_DIR set, XDG_STATE_HOME set, HOME only, WORKLANE_STATE_DIR="" and a relative WORKLANE_STATE_DIR. The PR's own test also shows that only the 3 control paths were touched. unguardedStateDir is main's body unchanged (diff).

Attacks on the refusal (measured, with stand-ins, by calling testStateRefusal with explicit env)

Attack Result
Each fall-through outside the machine Refused
<machine>/u/state/../../.. Refused
<machine>/ (the machine itself) Refused
<machine>/../<machine name>/u/state Allowed (correct: it is inside)
Relative dir, cwd outside / inside Refused / allowed. The engine resolves it against the same cwd.
Root set but its mark has the wrong token, machine mark right Refused. There is no fallback to the rootless mark.
Root outside the machine Refused
Rootless: mark 0644, mark a symlink, machine a symlink, machine group-writable, another uid Each refused
Rootless: another run's machine with this run's token Refused
A symlink inside the machine pointing outside (dangling stand-in, never followed) Allowed, for both state and slots (the gap the builder named)
Forging, rootless: a stray 0755 dir (a stand-in home) plus .test-isolation holding the env's token Allowed. One file is enough.
Forging through the root route Needs a root dir inside the stray dir as well, then allowed (the existing, documented threat model)
Rootless with INVOCATION_ID set Allowed. The root route refuses it, through systemRootRefusal.
Not a test run, any dir Allowed, and nothing looked at

On the service-mode exception (principle 9). The rootless machine mark is an exception added to the shared guard outsideTestMachine, whose rule needs the system root and the root's mark. So it is a loosening by principle 9, even though main has no state refusal at all.

  • It reopens nothing main had closed. The slot refusal is byte-identical and still needs the root (measured, by the PR's test and my attack run). Passing the refusal only returns the dir the env already names (inferred).
  • Keeping TEST_TOKEN_VAR in serviceModeEnv widens nothing else. Without a root, the token is read only by testRunSign, which TEST_RUN_VAR already satisfies, and by the new mark check (grep of src/, inferred).
  • The token comes from randomBytes(32) per preloaded process (test/isolate.ts:89). Every child that inherits the env can read it. A child could plant a mark anywhere it can write, but only on purpose: no code in src/ or test/ writes TEST_MARK_FILE outside a machine of its own (grep, inferred).
  • The PR body explains why service modes need it, but it doesn't present this as a loosening under principle 9. See R4.

Attacks on the lint (measured: fixture text fed to the PR's own lintFixtures, nothing in the repo edited)

Reported (correctly): the unguarded control, a guard in a try block whose throw is swallowed with the write after the try, spread arguments, and a callee chosen by a conditional.

Missed, and not in the builder's gap list:

  1. for (const _ of [] as string[]) onRealMachine(CONSOLE_SHELL); rmSync(CONSOLE_SHELL);: a guard in a for-of body counts for a write after the loop, even when the loop runs zero times.
  2. for (const f of [CONSOLE_SHELL]) { if (f) break; onRealMachine(f); } rmSync(CONSOLE_SHELL);: same cause.
  3. ok ||= onRealMachine(CONSOLE_SHELL); and ok2 ??= onRealMachine(...): logical assignments are short-circuits, but guardOf counts them as plain operands.
  4. writeFileSync(asRoot.etc('x.json'), '') and mkdirSync(join(SYSTEM_ROOTS.varTmp, 'agent-slots')): these are the real, never-moved paths, and nothing is reported. /var/tmp/agent-slots is writable by any user.
  5. Tracing gaps:
    • Promise.resolve(X).then((p) => rmSync(p))
    • Array.from([X], (p) => rmSync(p))
    • a generator helper (function* g() { yield X }, then for (const f of g()) rmSync(f))
    • a destructuring assignment ([d] = [X]; rmSync(d))
    • openSync(X, O_RDONLY | W) with W = O_WRONLY held in a variable

Missed but already listed by the builder: a shell or child process, and a guard on another instance of the same function (etcPath('a') covering etcPath('b')).

The 45 guards: each is a bare onRealMachine(x) statement, which is pure unless it throws. Every affected file passes at head (measured below), so none changes test behaviour.

Rule 10, scope, protection

  • Rule 10: no test or assertion is removed or weakened (diff read). .worklane/review.yaml only adds the two new tests to engine-guards, which is a tightening. Removing delete out[TEST_TOKEN_VAR] belongs to the loosening above (R4). It is not a separate weakening.
  • Scope: the lint is the widened scope the owner asked for in the issue comment. The rootless mark is a design addition the issue didn't ask for (R4).
  • Protection: the PR touches src/os/**, .worklane/review.yaml, test/isolate.ts and test/managed-apps.test.ts, which fall under engine and engine-guards in main's review.yaml. It is correctly marked: the [PROTECTED: owner merges] title, **Risk class:** protected, the needs-owner label, and "What this can never do" and "Compromised component" sections. One "can never do" claim is false: "Pass the lint with a guard that doesn't run" (lint misses 1 to 3 above, R1).
  • DCO and attribution: the three non-merge commits are signed off, and there are no attribution trailers or lines in the commits or the body (read). The dco check passes (measured).
  • Adversarial pass section: present.

Tests, typecheck, denylist, CI (measured)

Required changes

  1. R1: close the lint's guard holes, or the "can never do" claim is false.

    • A guard inside a for-of body may count only for writes inside that same for-of. Climbing out of a for-of that doesn't hold the write must return false.
    • Treat ||=, &&= and ??= as short-circuits in guardOf.
    • Add fixtures for the empty loop, the break before the guard, and both logical assignments.
  2. R2: report writes of the never-moved real paths. Treat a value built from asRoot.* or SYSTEM_ROOTS.* (src/os/system-paths.ts) like onRealMachine's result, { real: true }, with fixtures. No test imports either today (grep), so nothing existing should turn red (inferred).

  3. R3: name the tracing gaps. Add .then and other non-array callbacks, the Array.from mapper, generators, destructuring assignments and an O_RDONLY | <variable> open to the lint's "Not caught" header and the PR body's gap list, or trace them.

  4. R4: a principle-9 section in the PR body for the rootless machine mark. It must say:

    • The check it exempts: outsideTestMachine's requirement of the system root and its mark. That includes systemRootRefusal's refusals as root and under systemd markers, which the rootless path doesn't apply (measured: allowed with INVOCATION_ID set).
    • Why that check's finding would be wrong for a service-mode child: the root is withheld by design (serviceModeRefusal), while the child's state dir is still inside the test machine.
    • That the machine mark can be forged with one file, on purpose only, under the existing accident threat model (measured above).
    • That keeping the token in serviceModeEnv exists only for this exception.

    If the builder can make the rootless path also refuse systemd markers and uid 0, as the root path does, that narrows it further and should be stated.

  5. R5: fail closed on a symlink inside the machine (principle 1). In outsideTestMachine, lstat each existing component from the machine down to dir, only ever inside the machine and stopping at the first missing one. Refuse if any is a symlink. That keeps "nothing outside the machine is opened". Applying it to the slot dir too is a tightening. The case is real on macOS, where HOME isn't moved, so the real home is reachable from a test. Add a stand-in test with a dangling symlink.

  6. R6: correct the lint fail-without count. Measured at head's lint: 44 findings, with plugin-approval at 4, not 45 and 5. Correct the body, or show where the 45th comes from.

Owner trade-off

The rootless machine mark (R4) is a new loosening by principle 9. My assessment (inferred, backed by the measurements above): it reopens nothing main closed, the slot refusal is unchanged, and an accident can't pass it. Only a test that deliberately plants a mark gets through. Whether to accept the exception is the owner's call.

… the rootless mark never as root or under systemd

#818 round 2 (R4, R5). In a test run, outsideTestMachine now lstats each part of the dir that exists inside the
test machine, from the machine down, never following one and never looking outside the machine, and refuses a
symlink among them: a link in the machine can't lead a test to the person's own state or slots (principle 1). A
tightening for both dirs.

The service-mode exception (the machine's own mark, state dir only; owner-accepted 2026-10-19) now refuses as
the root's mark does: as root, and in a process systemd started (INVOCATION_ID, JOURNAL_STREAM).

Test (fails without): test/state-dir-refusal.test.ts. On 55343d3 the symlink test fails ('a link out, then
below it') and the rootless case with INVOCATION_ID set is allowed.

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
…first, and reports the never-moved real paths

#818 round 2 (R1 to R3).
- A guard counts only from a list of statements that holds the write, directly or through plain blocks: never
  out of a loop body, an if or else branch, a try, catch or finally block, another case, a labelled block or a
  function. The one loop let through is a for-of over a non-empty array literal whose body never breaks,
  continues or returns. Logical assignments (||=, &&=, ??=) are short-circuits.
- asRoot.* and SYSTEM_ROOTS.* are the real machine's paths, reported like onRealMachine's result.
- Traced: a callback's parameters as anything its call is given (.then, Array.from, new Promise), a generator's
  yields, destructuring assignments. An open is a read only when its flags are node's read-only constants (by
  symbol); anything else is a write.

Test (fails without): test/machine-path-writes.test.ts. Fed to 55343d3's lint, the round-2 fixtures miss 8
control-flow shapes (empty loop, break, the other branch, ||=, &&=, ??=, continue, spread) and 8 traces (asRoot,
SYSTEM_ROOTS, .then, Array.from, a generator, two destructuring assignments, flags with a variable).

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
…function declaration

#818 round 2, adversarial pass. A function declaration is hoisted: a guard before it in its scope may run after the function is called (early(); onRealMachine(X); function early() { rmSync(X) }). A write inside a function declaration below the guard's statement list no longer counts that guard. No test in test/ relied on it (the lint over test/ passes).

Test (fails without): test/machine-path-writes.test.ts, the s25 fixture (and fixture-b line 13 is now reported).

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711

Copy link
Copy Markdown
Owner Author

Round 2: answers to the round-1 review (head 692368f)

Commits: ebf6dab (src: symlinks, rootless refusals), 7e89673 and 692368f (lint). The PR body has a new "Round 2" section and a principle-9 section, and its false claims are marked "false through 55343d3".

  • R1 (fixed). A guard counts only when it always runs first. It must come first in a list of statements that holds the write, directly or through plain blocks. It never counts out of:

    • a loop body (the one exception: a for-of over a non-empty array literal with no break, continue or return);
    • if/else branches;
    • try, catch or finally blocks;
    • another case, a labelled block or a function;
    • a hoisted function declaration that holds the write (found in my adversarial pass).

    ||=, &&= and ??= are short-circuits.

  • R2 (fixed). asRoot.* and SYSTEM_ROOTS.* are reported as the real machine's path.

  • R3 (fixed).

    • Traced: callback parameters (as anything the call is given: .then, Array.from), a generator's yields, and destructuring assignments.
    • An open is a read only with node's read-only constants, checked by symbol. Anything else is a write.
    • Still not traced, and named in the lint header and the body: a Map or Set, other iterators, .call/.apply/Reflect.apply, and fs reached through createRequire or import().
  • R4 (done). The rootless mark now refuses as root and under INVOCATION_ID/JOURNAL_STREAM. It still applies to the state dir only. The principle-9 section cites the owner's acceptance (2026-10-19).

  • R5 (fixed, both dirs). Each existing part of the dir inside the test machine is lstat'ed, never followed, and never outside the machine. A symlink among them is refused.

  • R6 (corrected). The count is 44, with plugin-approval at 4 (measured with head's lint). The 45 came from round 1's earlier lint, which read readAllowlist's O_RDONLY open as a write.

Fail-without (measured; the round-2 tests run in a scratch worktree at 55343d3, never committed):

  • The lint fixtures fed to 55343d3's lint miss 8 control-flow shapes and 8 traces.
  • state-dir-refusal fails on the symlink test and on the INVOCATION_ID case.

Adversarial pass, round 2 (measured, fixtures fed to lintFixtures):

  • 25 guard shapes that look valid but don't protect a later write are now all reported: an empty loop, break, continue, a spread literal, while, do-while, if, else, the other branch, a try with a swallowing catch, catch, finally, case fall-through, a labelled break, a callback with a return, ||=, &&=, ??=, a ternary, an IIFE, .then, after the write, a non-literal for-of, a condition inside a loop body, and a hoisted function.
  • 8 valid shapes stay unreported.

Local (measured, scratch HOME, state, held-out dir and TMPDIR):

  • machine-path-writes 8/8 and state-dir-refusal 5/5.
  • The 12 affected files: 215 of 217 pass. The 2 failures are the isolate tmpdir-length tests, which fail only because my scratch TMPDIR path is long.
  • fullsuite.sh (418 files): 3712 pass, 7 fail, 57 skipped.
    • 2 of the failures are those same tmpdir tests.
    • The other 5 are fix-forward-coordinator ×2, testrun-env-argv ×1 and testrun-sandbox ×2 (Docker isn't running). They fail the same way on origin/main 128453a in the same environment (measured), so they are environmental.
  • Both typechecks are clean, and check-denylist reports 1307 files clean.

CI on 692368f: 25 pass, 2 skipped, none failing (measured, gh pr checks 818).

merge-tree at 692368f: clean with #787, #805, #769, #819, #820 and #816. #746 conflicts in CHANGELOG.md, scripts/setup/lib.sh, test/engine-update.test.ts and test/update-checks.test.ts; origin/main has the same conflicts with #746.

@jayden1711

Copy link
Copy Markdown
Owner Author

Evaluator verdict: CHANGES NEEDED (round 2)

I reviewed head 692368f against #815 and the round-1 review (issuecomment-6272854895). I used my own worktrees: the head, origin/main 128453a, and 55343d3 with head's new tests on top. Every run had a scratch HOME, state dir and held-out dir, with a stand-in claude first on PATH.

Every refusal attack used fixtures made under the preload's test machine. Each "outside" path was a name that was never created, and only the refusal itself ran on it. The refusal lstats only inside the machine. Nothing read, statted or wrote a real state dir, /var/lib or /etc.

R1, R2, R3, R4 and R6 are closed (measured). R5 is closed for a symlink at any depth, but not for a symlink followed by ... That form still leads the slot dir out of the machine (measured, details below). Three body claims also need correcting.

Round-1 required changes

# Status Evidence
R1 guard holes Closed All 17 round-1 lint fixtures re-run against head's lintFixtures (measured). The empty loop, the break first and ||=/??= are now reported. The swallowed try is still reported.
R2 asRoot / SYSTEM_ROOTS Closed asRoot.etc(...) and join(SYSTEM_ROOTS.varTmp, ...) are reported (measured). So are src/ constants built from them: rmSync(HUB_UNIT_AS_ROOT) and writeFileSync(ADD_PROJECT_SCRIPT, '') (measured, my fixtures).
R3 tracing Closed .then, the Array.from mapper, a generator, both destructuring assignments and O_RDONLY | W are reported (measured). .call, Reflect.apply and a shell are still missed, and they are named in the header and the body.
R4 principle-9 section Closed The section is present and covers each point; see "Principle 9" below. The rootless path refuses INVOCATION_ID/JOURNAL_STREAM (measured). It refuses uid 0 by code, but no test covers that (see required change 2).
R5 symlinks Partly closed A link at depth 1, 2 and 3, the dir itself a link, a link pointing inside the machine, <link>/., and the machine dir itself a link: each is refused for the state dir (root and rootless) and the slot dir (measured). A link followed by .. is allowed (required change 1).
R6 count Closed At head, with the 5 guarded files put back to origin/main, the lint fails with 44 (measured): managed-apps 25, owner-terminal-removed 7, plugin-run 7, plugin-approval 4, init 1 (test/init.test.ts:328: mkdirSync(dirname(checkout)): srvCheckoutPath is a machine location). One stale 5 remains in the body (required change 3).

Fail-without (measured)

  • The refusal. I ran head's test/state-dir-refusal.test.ts on 55343d3: 3 pass, 2 fail.

    • The symlink test fails with AssertionError: a link out, then below it.
    • The testStateRefusal test fails with AssertionError: INVOCATION_ID (expected: /systemd started this process \(INVOCATION_ID is set\)/).

    At head, all 5 pass.

  • The lint. I fed head's fixture-f and fixture-g marked fixtures to 55343d3's lintFixtures. It misses 9 of 25 control-flow shapes and 8 of 9 traces:

    • Control flow: s1 empty loop, s2 break, s7 the other branch, s14 to s16 ||=/&&=/??=, s21 continue, s23 spread, and s25 hoisting.
    • Traces: asRoot, SYSTEM_ROOTS, .then, Array.from, the generator, both destructuring assignments, and flags held in a variable.

    None of the OK lines is reported. This matches the builder's 8+8. s25, which the builder labelled inferred, is measured missed on 55343d3.

    Note: copying head's machine-path-writes.test.ts onto 55343d3 passes, because the file carries the lint itself. Only feeding the fixtures to the old function shows the fail-without.

  • At head: machine-path-writes 8/8 and state-dir-refusal 5/5 pass (measured).

Attacks on the new rules (measured: fixture text fed to head's lintFixtures, nothing in the repo edited)

The for-of literal exception holds. Each of these is reported:

  • a leading hole [, X] and a trailing hole [X, ,];
  • a spread in the middle [...none, X];
  • a labelled continue outer in the body;
  • a break inside a nested switch in the body;
  • a return inside a try/finally in the body;
  • a throw caught inside the body.

for await (const p of [X]) and a yield in the body before the guard are correctly not reported.

finally, labels, return: each of these is reported:

  • a guard in finally with the write after;
  • a guard in try with the write in finally, with and without a return first;
  • a guard in a labelled block that breaks, directly, inside a plain block, or with a plain block inside it;
  • a guard in catch;
  • a guard in try with the write in catch.

A return in a try/finally before the guard is not reported, which is correct: it leaves the function. Comma-in-short-circuit, a tagged template, an if condition, an optional chain higher up, a function declaration in a plain block, and an arrow defined before the guard are all reported. s += onRealMachine(x) is accepted.

Missed, and not in the gap list (required change 4):

  • A guard whose argument only may be the location counts for it. Guard keys are the union of the argument's possible origins. Each of these is missed:

    • const x = n ? CONSOLE_SHELL : etcPath('y'); onRealMachine(x); rmSync(CONSOLE_SHELL);
    • let x = CONSOLE_SHELL; x = etcPath('y'); onRealMachine(x); rmSync(CONSOLE_SHELL);
    • for (let p of [CONSOLE_SHELL]) { p = etcPath('y'); onRealMachine(p); } rmSync(CONSOLE_SHELL);
    • in a generator, for (const p of [(yield CONSOLE_SHELL) as string]) onRealMachine(p); rmSync(CONSOLE_SHELL); (the guard runs on whatever next() sends).

    In each, the guard runs on another path inside the root, and the write goes to the location.

  • An assignment to a for-of let isn't traced. for (let p of ['/tmp/x']) { p = CONSOLE_SHELL; rmSync(p); } is missed: declOrigins's for-of branch reads only the loop expression, not assignmentsTo(d).

These need a deliberate shape, so they sit inside the accident threat model (inferred). But the header's "Not caught" list and the body's gap list should say so.

Attacks on R5 and R4 (measured, by calling testStateRefusal/testSlotsRefusal with explicit env and stand-ins)

  • A link followed by .. is allowed (required change 1). Take <machine>/u/Lout, a dangling link to a stand-in outside, never followed:

    • testSlotsRefusal('<machine>/u/Lout/../slots') returns null;
    • so does testStateRefusal on the same path, with and without the root.

    linkInside walks relative(machine, resolve(dir)), so resolve collapses Lout/.. lexically and Lout is never lstat'ed. The kernel resolves it the other way. I showed the mechanism with a link that stays inside the scratch machine: <machine>/u/Lin -> <machine>/w/deep. The refusal allows <machine>/u/Lin/../slots2. A raw mkdirSync of that string made <machine>/w/slots2 (the link target's parent), not <machine>/u/slots2 (measured).

    • Slot dir: src/slot-file.ts builds slot paths by concatenation (${raw(dir)}/${name}, line 107), and makeSlotDir mkdirs the raw dir (line 180). So AGENT_SLOTS_DIR=<machine>/u/<link>/../x leads every slot read and write out of the machine (inferred from the code and the measured mkdir).
    • State dir: all 5 stateDir() call sites in src/ go through join(), which collapses .. lexically (grep, inferred), so the engine doesn't reach it today. A raw use would.
    • This makes false the body's "can never do": "Lead the state or slot dir out of the test machine through a symlink inside it (measured, since ebf6dab)".
  • Race (check, then swap a dir for a link).

    • Measured: allowed before the swap, refused when checked again after it.
    • Inferred:
      • stateDir() checks on every call.
      • slotsDir() checks on every call too, through resolvedSlotsDir.
      • Neither check covers the window between the check and the open.

    A test would have to swap a directory for a link between resolution and use on purpose. The documented threat model is accidents, so I accept it. No change needed.

  • INVOCATION_ID="" / JOURNAL_STREAM="". Refused, root and rootless (measured). The check is !== undefined, so it fails closed.

  • uid 0. With uid = 0 the call is refused, but by the machine-owner check (isn't a directory of this user's alone), before machineMarkRefusal runs (measured). The new uid === 0 line is reached only when the machine itself is owned by root. No test reaches it (state-dir-refusal.test.ts has no uid-0 case, grep). The code is right by reading (inferred), but the body's "(measured, by test)" is not (required change 2).

  • Windows (no process.getuid). With uid = null the rootless path allows (measured on Linux), and on win32 the mode and owner checks are skipped as well. That is the same as the root route's systemRootRefusal (inferred, read). Native Windows runs no agents (canRunAgents) and isn't in CI. This is not a regression and not blocking, but the principle-9 section could say that the root and systemd refusals don't apply there.

  • Round-1 cases, re-run, all as before:

    • Refused: a link out (state and slots), .. out of the machine, the machine dir itself, a relative dir with cwd outside, a wrong root mark with no fallback, a root outside, a mark at 0644 / another uid / a symlink, the machine a symlink (root, rootless and slots), a group-writable machine, and another run's machine.
    • Allowed: the rootless slot dir is still refused, and a forged stray dir plus mark is still allowed (the accepted exception).
    • Not a test run: allowed, nothing looked at.

Production unchanged (measured)

I started a child with an env built from scratch (no test-run sign, stand-in HOME) and compared stateDir() from origin/main 128453a's build with this PR's build. All 5 cases are the SAME: WORKLANE_STATE_DIR set, XDG_STATE_HOME set, HOME only, WORKLANE_STATE_DIR="", and a relative WORKLANE_STATE_DIR.

Principle 9: the service-mode machine mark

The body's section covers each point (read):

  • The exempted check: outsideTestMachine's requirement of the system root plus its mark, including systemRootRefusal's root and systemd refusals. It applies to the state dir only, and the slot message is byte-identical.
  • Why its finding is wrong for a service-mode child: serviceModeRefusal withholds the root by design, and hub-setup's two CLI tests fail without the exception.
  • The forgery: one planted file forges it.
  • The token: serviceModeEnv keeps it only for this exception.
  • The owner's acceptance: 2026-10-19, "as relayed by the coordinator". src/os/system-paths.ts carries the same note at testStateRefusal.

The only inaccuracy is the uid-0 "measured, by test" (required change 2).

Rule 10, scope, protection

  • Rule 10: no test or assertion was removed or weakened in round 2 (diff read). fixture-b line 13 moved from OK to reported, which is a tightening.
  • Scope: unchanged from round 1.
  • Protection: the same paths (src/os/**, .worklane/review.yaml, test/isolate.ts, test/managed-apps.test.ts). It is correctly marked: [PROTECTED: owner merges], **Risk class:** protected, the needs-owner label, and real "What this can never do" and "Compromised component" sections. One "can never do" line is false (required change 1).
  • No regression for existing installs: outside a test run nothing changes (measured above). In a test run the slot dir refusal is only tightened.
  • DCO and attribution:
    • ebf6dab, 7e89673 and 692368f are signed off;
    • no Co-Authored-By or "Generated with" in any of the PR's commits (git log --grep, measured) or the body (read);
    • the dco check passes.
  • Rule slip, disclosed by the builder: a commit message passed as a brace+quote shell line (rule 4). It is noted here. It doesn't affect the code.
  • Adversarial pass section: present, with a round-2 table of 25 shapes and the valid ones.

Tests, typecheck, denylist, CI (measured)

Required changes

  1. R5, a symlink followed by .. (principle 1; the "can never do" claim is false).

    • In outsideTestMachine, refuse a dir with any .. segment, or lstat its raw prefixes in the order the kernel walks them.
    • Add a stand-in test: a dangling link inside the machine, then <link>/../x, refused for both the state and slot dir.

    Refusing every .. also refuses <machine>/../<name>/u/state. No test relies on that (state-dir-refusal uses .. only in refused cases), which I inferred.

  2. The uid-0 claim. The body says the rootless path's refusal as root is "measured, by test", but no test reaches machineMarkRefusal's uid === 0 line. The machine-owner check refuses first. Either label it inferred, or move the uid-0 and systemd checks ahead of the machine lstat on the rootless path so a test can reach them (and fail earlier), and add the uid-0 case.

  3. The stale count. In the "What changed" table (item 4), plugin-approval still says 5. Make it 4, or mark it false through 55343d3.

  4. The lint's gap list (principle 5).

    • Add to the header's "Not caught" and the body's gaps: a guard whose argument only may be the location (a conditional, a reassigned local or for-of let, a yield element), and an assignment to a for-of let as a write target.
    • Or trace them. For the second, the for-of branch of declOrigins could add assignmentsTo(d).
    • Add the fixtures above either way.

Non-blocking: the principle-9 section could note that on native Windows (no process.getuid) the rootless path, like the root route, has no root check.

… run; the rootless root and systemd checks come first

#818 round 3. linkInside walked the path after resolve(), which folds `link/..` away by text, while the kernel
goes up from the link's target: `<machine>/u/<link>/../slots` passed the check and a raw mkdir of it (as
src/slot-file.ts makes slot dirs) landed beside the link's target. A dir with any '..' part is now refused, for
the state and the slot dir alike (fail closed; join() already folds the engine's own paths).

The rootless path's refusals as root and under systemd now run before the machine is looked at, so the uid-0
case is reachable and tested (the machine-owner check said no first before).

Test (fails without): test/state-dir-refusal.test.ts. On 692368f a `<link>/../state` dir is allowed, and the
uid-0 rootless case is refused by the machine-owner check, not as root.

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
…surely is; a for-of let assigned in its body is traced

#818 round 3. A guard's names were every location its argument may be, so a guard on `n ? X : Y`, on a local
reassigned from X to Y, on a reassigned for-of `let`, or on a `yield` element counted for X while it ran on
another path. A guard now counts only the locations its argument surely is: every branch and every value a local
is given must agree, and a yield, an index or a parameter is surely nothing. A for-of variable never assigned in
its body is each element in turn. Writes through a for-of `let` that its body assigns are traced too.

Test (fails without): test/machine-path-writes.test.ts, the m1 to m5 fixtures. Fed to 692368f's lint they are
missed (measured in round 3).

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
29e8fc5 split the dir with String.prototype.split, which handed the slot dir to a String method:
test/slot-path.test.ts's A5 check (#806: no patched String or Array method ever sees the slot dir) failed
(`[ 'split' ]`). The check now tests a RegExp against it instead, with the same result.

Test (fails without): test/slot-path.test.ts, "a patched String.prototype or Array.prototype method is never
handed the slot dir" (fails on 29e8fc5 with [ 'split' ]).

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
#818 merged with main d4b4503, with the dashboard UI built, packs to size 1,903,458 and unpacked 5,940,612
(npm pack --dry-run --json; main alone: 1,902,158 and 5,934,411, already over 1,900,000). Only two files
grew: dist/src/os/system-paths.js +5,609 (the state-dir refusal, the machine's own mark, the symlink and '..'
checks) and dist/src/os/index.js +592 (stateDir's refusal). Measured + 2%, rounded up to the next 10,000:
1,950,000 and 6,060,000.

Test (fails without): test/package-files.test.ts, "the package stays within its size budget", on this tree
with the old budget (1,903,458 > 1,900,000).

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711

Copy link
Copy Markdown
Owner Author

Round 3: answers to the round-2 review (head 5a20d35)

Commits:

The body has a "Round 3" section, and each false claim is marked "false through 692368f".

The four required changes:

  1. .. after a link (fixed). In a test run, a state or slot dir written with any .. part is refused (fail closed, checked with a RegExp). My first version used dir.split(), which Slot dir: a slot path is an opaque SlotPath that only writeSlotFile and the slot readers take; tsc rejects it at any other writer [PROTECTED: owner merges] #806's A5 test caught: [ 'split' ], measured on e66f37d and fixed in 9168629. The "can never do" line is corrected.
  2. uid 0 (fixed). The rootless root and systemd refusals now run before the machine is looked at, and a test covers uid 0. The Windows note is added to the principle-9 section.
  3. The stale count (fixed): plugin-approval is 4.
  4. Lint (traced).
    • A guard counts only what its argument surely is: a branch, a reassigned local or for-of let, or a yield element counts for nothing unless every way agrees.
    • A for-of let assigned in its body is traced as a write target.

Fail-without on 692368f (measured, scratch worktree):

  • state-dir-refusal fails on the <link>/../state case and the as root case.
  • The 5 new lint fixtures (m1 to m5) are all missed by 692368f's lintFixtures. No OK line is reported.

Package budget (measured, npm pack --dry-run --json, dashboard UI built):

Local (measured):

  • machine-path-writes 8/8; state-dir-refusal 5/5.
  • slot-path, slots, slot-dir-writes and update-drain: 106/106.
  • typecheck clean; check-denylist 1310 files clean.
  • fullsuite.sh (420 files): 3764 pass, 7 fail.
    • 2 of the failures are the isolate tmpdir-length tests, which fail only because my scratch TMPDIR path is long.
    • The other 5 (fix-forward-coordinator ×2, testrun-env-argv, testrun-sandbox ×2, "Docker isn't running") fail the same way on origin/main in this environment (measured in round 2).

CI on 5a20d35: 28 pass, 3 skipped, none failing.

merge-tree at 5a20d35:

@jayden1711

Copy link
Copy Markdown
Owner Author

Evaluator verdict: CHANGES NEEDED (round 3)

I reviewed head 5a20d35 against #815, the round-2 review (issuecomment-6274220846) and the builder's reply (issuecomment-6275489434). I used my own worktrees:

  • the head;
  • origin/main d4b4503;
  • 692368f with head's tests on top;
  • a scratch copy of head's build, used for fault injection.

Every run had a scratch HOME, state dir and held-out dir, with a stand-in claude first on PATH. Refusal attacks call testStateRefusal/testSlotsRefusal on fixtures under the preload's test machine. Each "outside" target is a name that was never created and never followed. Nothing read, statted or wrote a real state dir, /var/lib or /etc.

Summary.

  • R5 (..), uid 0, the stale count, the budget, CI and fail-without all check out (measured).
  • One problem remains. The lint's new rule, "a guard counts only what its argument surely is", is still false for 6 shapes of the class it was meant to close (measured).
  • The header, the body's "Round 3" section and the struck-through gap line all say that class is traced now. Either trace those shapes, or narrow the claim and list them. That is required change 1.

Round-2 required changes

# Status Evidence
1 R5, link/.. Closed 38 dirs × 3 calls (state with the root, state rootless, slots), measured with r5-attacks.mjs (below).
2 uid 0 Closed The test reaches the line. With the line disabled by fault injection, the as root assertion fails (measured, below).
3 stale count Closed Item 4's table says 4, marked "false through 692368f" (read).
4 lint gaps Partly closed All 5 round-2 shapes are now reported (measured). Six more shapes of the same class are missed and aren't listed (required change 1).

R5: .. in every position (measured)

  • Refused for all 3 calls, with the has a '..' part message:
    • link/.., link/../, link/../slots, link/../../u/slots, link/x/../..;
    • a link pointing inside the machine then /../slots2;
    • link//..//slots and link/./../slots;
    • a leading ../ with a relative dir, and a bare ..;
    • machine/../machine/u/state, real/../real, a trailing /.. with no link;
    • link\..\slots (backslashes);
    • real/../state, relative, with cwd inside the machine.
  • A link with no ..: link, link/, link/., link/./slots and link//slots are each refused as a symlink.
  • Encoded and look-alike forms:
    • %2e%2e, fullwidth .. and U+2025 after a link are refused (as a link).
    • With no link before them they are allowed. That is correct: the kernel takes them as literal names. A raw mkdir of <m>/u/real/%2e%2e/x made exactly that directory and not <m>/u/x (measured).
    • A NUL in the path is refused (can't be looked at).
  • No false refusal of a normal path. Each of these is allowed for all 3 calls:
    • u/state, u/real/deep, u/real/deep/;
    • u/.state, u/..state, u/state.., u/..., u/a..b;
    • u/real/./deep, //u//real//deep;
    • relative state and ./state with cwd inside.
  • My round-1 and round-2 refusal cases, re-run: every result is as in round 2, except that the two .. cases now say has a '..' part. not a test run, any dir is still ALLOWED: the .. check comes after the test-run sign (read, src/os/system-paths.ts:176 and :206).
  • slotsDir() hands the raw AGENT_SLOTS_DIR to the refusal (src/os/index.ts:988), so the RegExp sees what slot-file.ts concatenates onto (read).

uid 0 (measured, fault injected)

I copied head's built dist/ into my scratch dir and ran state-dir-refusal there:

  • Unchanged: 5/5 pass.
  • With only dist/src/os/system-paths.js:194 changed from uid === 0 to uid === -12345: 4 pass, 1 fails, with AssertionError: as root. The actual value was the machine-owner refusal: … isn't a directory of this user's alone.

So the assertion is reached only through the new line, and it pins that line. The systemd checks also run before the lstat now. rootless, INVOCATION_ID/JOURNAL_STREAM with x and "" are each refused with the systemd message (measured).

The lint (measured: fixture text fed to head's lintFixtures, nothing in the repo edited)

Round-2 fixtures, re-run (lint-attacks.mjs, 52 cases):

  • Every R is reported and every OK stays unreported.
  • All 5 round-2 "new" shapes are now reported: the ternary, the reassigned local, the reassigned for-of let, the yield element, and a write through a for-of let.
  • .call, Reflect.apply, a shell and etcPath('a')/etcPath('b') are still missed. They are named gaps.
  • A Map is now reported.

Head's own marked fixtures: fixture-f has 30 R, 0 missed, 11 OK, 0 reported; fixture-g has 9 R, 0 missed, 2 OK, 0 reported.

Const aliases of the same location (lint-surely.mjs): no false positive in 9 shapes. Each is correctly not reported:

  • a const alias guarded, then the original written, and the reverse;
  • an alias of an alias;
  • an import rename (CONSOLE_SHELL as CS) and a namespace import;
  • a let that is never reassigned;
  • a test-local { p: CONSOLE_SHELL };
  • const d = SYSTEM_SLOTS.dir;
  • dirname(a).

Getters:

  • Guarded through a getter: onRealMachine(o.p) with a local or class getter isn't taken as a guard. That is conservative, so it's safe.
  • A write through a getter isn't traced at all. const o = { get p() { return CONSOLE_SHELL; } }; rmSync(o.p); is missed, and so is class K { get p() { return CONSOLE_SHELL; } } rmSync(new K().p);. The header's "a property of an object the lint doesn't trace (this.f, a parameter's field)" may be meant to cover this, but it doesn't say "getter".

"Surely" is still a "may" in these shapes. Each is missed: the guard runs on unitPath('y') (or another value), and the write goes to CONSOLE_SHELL.

Shape Fixture body
destructuring a branch const [x] = c ? [CONSOLE_SHELL] : [unitPath('y')]; onRealMachine(x); rmSync(CONSOLE_SHELL);
destructuring another index const [x] = [unitPath('y'), CONSOLE_SHELL]; onRealMachine(x); rmSync(CONSOLE_SHELL);
a for-of guard, a write to another element in the same body for (const p of [unitPath('y'), CONSOLE_SHELL]) { onRealMachine(p); rmSync(CONSOLE_SHELL); } (also over a const array)
a property reassigned before the guard const o = { p: CONSOLE_SHELL }; o.p = unitPath('y'); onRealMachine(o.p); rmSync(CONSOLE_SHELL); (also through an alias o2.p = …, Object.assign, Object.defineProperty)
a src object's property mutated (MACHINE_PATHS isn't frozen) const s = MACHINE_PATHS.hubUnitFile; MACHINE_PATHS.hubUnitFile = unitPath('y'); onRealMachine(MACHINE_PATHS.hubUnitFile); rmSync(s);
a path builder that drops or climbs out of its first argument onRealMachine(resolve(CONSOLE_SHELL, unitPath('y'))), onRealMachine(join(CONSOLE_SHELL, '..', … , unitPath('y')))
  • Destructuring: { p: x } = c ? {…} : {…} and { q: x } = { q: unitPath('y'), p: … } are reported. The array forms aren't, because surely reads a binding element through declOrigins (every origin it may have).
  • The for-of literal: "each element in turn, each surely checked, once" is true for a write after the loop. It is not true for a write inside the body: on the first pass the guard has checked only the first element.
  • Path builders:
    • The comment at the PATH_TYPES branch says "A path built on it (dirname(X), join(X, …)) is inside the root only if X is". That is false for resolve(X, <absolute>) and for join(X, '..', …).
    • The header's "a guard on a different value that mentions the same name" covers these in spirit. The comment and the fixture v10 make the opposite claim.
  • Reached in the repo? No test mutates MACHINE_PATHS (grep of test/, inferred). Every shape above needs a deliberate form, so all of them sit inside the accident threat model (inferred). The finding is the claim: the header, the body's Round 3 section and the struck-through gap ("Both traced since 32f74e8") say this class is closed. It isn't (principle 5).

Non-blocking: onRealMachine(String(CONSOLE_SHELL)) isn't taken as a guard, though surely has a String branch (measured). That fails closed. The callee probably resolves to StringConstructor's call signature, which has no name (inferred).

Fail-without (measured, scratch worktree at 692368f with head's tests)

  • state-dir-refusal.test.ts: 3 pass, 2 fail.

    • AssertionError: …/link-…/out/../state (actual: '', expected: /has a '\.\.' part/).
    • AssertionError: as root (actual: the machine-owner refusal).

    This matches the builder's 3/2.

  • Head's marked fixtures fed to 692368f's lintFixtures:

    • fixture-f: 30 R, 5 missed: m1 to m5 at lines 45 to 49. 11 OK, 0 reported, so v9, v10 and v11 stay unreported.
    • fixture-g: 0 missed.

    This matches the builder's 5.

  • At head: machine-path-writes, state-dir-refusal, slot-path, slots, slot-dir-writes, update-drain and package-files: 121/121 pass.

Budget (measured)

  • npm pack --dry-run --json --ignore-scripts, run through the npm wrapper, with build and build:web in each worktree:
    • main d4b4503: 1,902,158 / 5,934,411, 512 files.
    • Head: 1,903,458 / 5,940,612, 512 files. Head is the merge with d4b4503: merge-base --is-ancestor, and origin/main is still d4b4503.
  • File-list diff:
    • Only dist/src/os/index.js 71,033 → 71,625 (+592) and dist/src/os/system-paths.js 15,222 → 20,831 (+5,609) changed. No file was added or removed.
    • The deltas sum to +6,201, which equals the unpacked delta. Both files are this PR's intended code.
  • Arithmetic: 1,903,458 × 1.02 = 1,941,527 → 1,950,000. 5,940,612 × 1.02 = 6,059,424 → 6,060,000. Both are correct.
  • The test: against main, test/package-files.test.ts differs only on the PACKAGE_BUDGET line. 5a20d35 touches only that line and the .. RegExp commit is separate (diff). test: raise the package budget to main's size + 2% after #805 and #787 [PROTECTED: owner merges] #823 sets the same two numbers, so it merges cleanly with this PR.

Usual checks (measured unless marked)

Required changes

  1. The "surely" rule (principles 1 and 5). For each shape in the table above, either trace it, or take it out of the claim and list it in the header's "Not caught" and the body's gaps. Add a fixture for each either way. Cheap fixes, inferred:

    • A binding element: surely of the matching element or property when the initializer is a literal, else nothing. Not declOrigins.
    • A guard on a for-of variable: for a write inside the same body, take only what every element agrees on (both over the elements).
    • A property: o.p counts as surely nothing once anything assigns .p on it or an alias. List Object.assign/defineProperty and src object mutation as gaps.
    • Path builders: take only dirname/normalize of X, or a join(X, …) whose other arguments are string literals with no .. part and no leading /. Correct the PATH_TYPES comment.
    • Getters: name writes through a getter in "Not caught", or trace them.

    Then correct the body: the Round 3 section's "A guard counts only the locations its argument surely is", and the struck-through gap "Both traced since 32f74e8".

  2. Adversarial pass: add the round-3 attacks you ran against the surely rule and the .. check, with results.

Non-blocking: refresh the body's merge-tree list. The merge commit has no sign-off (the dco check passes).

…sed today

#818 round 4. The round-3 review measured six shapes where "a guard counts only what its argument surely is"
is still a may, plus writes through a getter. Each needs deliberate code, so they stay outside the accident
threat model and aren't traced. They are now named in the header's "Not caught" list, and each is a GAP
fixture (with .call, Reflect.apply and another call of the same function) that the test expects to be missed,
so a later fix shows up there:
- array destructuring of a branch, and of another index;
- a for-of guard covering a write to another element in the same body;
- a property reassigned before the guard;
- a src object's property mutated;
- resolve(X, <absolute>) and join(X, '..', ...) as the guard;
- a write through an object or class getter.

The PATH_TYPES comment said a path built on X is inside the root only if X is: false through 5a20d35 for
resolve and join with '..'; corrected.

Test: test/machine-path-writes.test.ts, "the known gaps stay named" (12 GAP lines, none reported: measured).
Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711

Copy link
Copy Markdown
Owner Author

Evaluator verdict: CHANGES NEEDED (final check, body only)

I reviewed head d916730 against #815, the round-3 review (issuecomment-6275775507), the owner's acceptance of the service-mode exception, and one owner rule the coordinator relayed during this check (2026-10-19). I used my own worktrees at d916730 and at main d5674d0, plus scratch copies of head's build for fault injection and lint probes. Every run had a scratch HOME, state dir, held-out dir and TMPDIR, with a stand-in claude first on PATH. Nothing read, statted or wrote a real state dir, /var/lib or /etc.

Summary.

  • Code, tests, size, CI and the exception's tightenings all check out (measured).
  • Every round-3 required change is fixed (measured).
  • Two body-only changes remain. Both come from the owner rule relayed today: a new protection must be by design, and a lint is allowed only as a named backstop.
    • The body never names the runtime refusal as the protection, and never calls the lint a backstop.
    • It doesn't say which write routes have only the lint behind them.
  • No code change is required.

The owner rule relayed today: protection by design, the lint as a backstop

  • What is by design here (measured, read).
    • stateDir() refuses at runtime in a test run (src/os/index.ts stateDir → testStateRefusal). The .. part check and the symlink walk are part of that refusal.
    • For machine locations, main's existing preload system root moves every systemPath location into the test machine.
    • The preload also strips every inherited WORKLANE_* variable (test/isolate.ts:22-24). Measured: with a held-out dir inherited from outside, heldOutDir(stateDir()) under the preload resolves inside the test machine.
  • What the body says (read). The Intent paragraph presents the refusal and the lint side by side. Neither "backstop" nor any statement of which one is the protection appears anywhere in the body (grep of the body).
  • Routes with only the lint behind them (measured with fixtures, inferred for permissions). Writes to the never-moved real paths: asRoot.*, SYSTEM_ROOTS.* and what onRealMachine returns.
    • No runtime refusal stands behind these. The system root doesn't move them, by definition.
    • The lint reports every shape I tried, 9 of 9: a destructured or renamed SYSTEM_ROOTS, Object.values(SYSTEM_ROOTS), an alias of asRoot, the result held in a local, a template, a concatenation, dirname() of it, and join(it, '..', …).
    • For the root-owned roots (/etc, /var/lib, /opt, /usr/local/libexec, the unit dir), file permissions stop an unprivileged test (inferred).
    • /var/tmp (SYSTEM_ROOTS.varTmp, whose agent-slots is the machine's real slot dir) and /run/lock are world-writable on a default Ubuntu (inferred, not statted). There, the lint is the only thing between a test and real state.
    • This route is not new: main has no lint at all, so this PR strictly adds a backstop (inferred). It is a named gap, not a blocker, because each shape needs a deliberate reach for the never-moved paths. It has to be named, though.

Round-3 required changes

# Status Evidence
1 "surely" shapes Closed (named) Each shape is in the header's "Not caught" list and is a // GAP fixture in "the known gaps stay named" (read): g1 to g9, plus .call, Reflect.apply and another call of the same function. That makes 12. marked() asserts that the reported lines are exactly the // R lines, and the test passes, so all 12 are misses today (measured). The PATH_TYPES comment is corrected (diff read). The Round 3 claim, the struck-through gap line and the comment are each marked "false through 5a20d35" (read).
2 Adversarial pass Closed The round-3 attacks are in "Adversarial pass, round 3": the .. check, uid 0, and the "surely" table, with results (read).

Not separately fixtured: the alias, Object.assign and Object.defineProperty variants of g4. They are named in the header (read). Non-blocking.

The exception's tightenings (measured, fault injected)

Each fault was injected into a scratch copy of head's built dist/src/os/system-paths.js. state-dir-refusal.test.ts was then run there:

Copy Fault Result
fi0 none (control) 5/5 pass
fi1 rootless uid === 0 → uid === -12345 4/5, AssertionError: as root
fi2 rootless markers → ['JOURNAL_STREAM'] only 4/5, AssertionError: INVOCATION_ID
fi3 rootless markers → ['INVOCATION_ID'] only 4/5, AssertionError: JOURNAL_STREAM
fi4 testSlotsRefusal given rootless: true 4/5, …did not match /doesn't hold the test preload's system root/

So every tightening is real and pinned by a test, including "the state dir only". The rootless uid-0 and systemd checks run before the machine is looked at (src/os/system-paths.ts outsideTestMachine, read). The principle-9 section is present, names the check it exempts and says why that check's finding is wrong for a service-mode child (read).

Gaps: new shapes tried (measured, fixture text fed to head's lintFixtures in a scratch copy; nothing in the repo edited)

  • Reported, 30 of 36.
    • Spread arguments; fs.promises.rm; promises as fsp; a default node:fs import; an fs/promises namespace.
    • The destination of cpSync, renameSync, symlinkSync, copyFileSync and linkSync.
    • createWriteStream, truncateSync, chmodSync, openSync(…, 'w'), callback rm.
    • A template, a concatenation, process.env.X ?? CONSOLE_SHELL.
    • const { hubUnitFile } = MACHINE_PATHS; Object.values(MACHINE_PATHS) through both forEach and for-of.
    • .map, a new Promise executor.
    • The 9 never-moved shapes above.
  • Missed, 6. Each needs deliberate code or is plain words, so each is a named gap, not a blocker (inferred). None of them is named today:
    • a7, a8: literal real paths: '/var/tmp/agent-slots/x', and join(homedir(), '.local', 'state', '<cli>'). These are outside the lint's model (engine-resolved locations). On Linux the preload's HOME move covers a8 (inferred). On macOS HOME isn't moved.
    • a12, a13: the "under a test's own root" exception taken with / as the root: join('/', onRealMachine(X)), and the same through a parameter defaulting to '/'. Either is the real path.
    • s1, s2: a write that climbs out of the resolved dir: rmSync(join(stateDir(), '..', '..', '..', 'x')), and the same on slotsDir(). The runtime refusal checks the dir itself, not paths built on it. The lint exempts the state and slot families.
  • No route by which an ordinary test reaches real state (inferred). Every miss needs a deliberate reach for a never-moved or literal path, or for several ... The preload strips inherited WORKLANE_* (measured above).

Fail-without (measured, my main worktree d5674d0 with head's state-dir-refusal.test.ts and test/isolate.ts)

  • It fails 4 of 5:

    • AssertionError: WORKLANE_STATE_DIR outside the test machine: no state dir;
    • Missing expected exception;
    • TypeError: testStateRefusal is not a function, twice.

    The production-path test passes there, as intended.

  • The build reports TS2339 for the missing export but emits anyway. The first two failures show the bug itself, not a compile artifact.

  • At head: 5/5 pass.

Package size (measured)

npm pack --dry-run --json --ignore-scripts through the wrapper, with build and build:web run in both worktrees:

Tarball Unpacked Files
main d5674d0 1,902,158 5,934,411 512
head d916730 1,903,458 5,940,612 512
Δ +1,300 +6,201 0
  • Per-file diff: only dist/src/os/index.js 71,033 → 71,625 (+592) and dist/src/os/system-paths.js 15,222 → 20,831 (+5,609) changed. No file was added or removed. The deltas sum to +6,201. This matches the body exactly.
  • Budget: git diff d5674d04 d9167303 -- test/package-files.test.ts is empty, so the budget is main's. The budget test passes.

Tests and checks (measured, head, scratch env)

  • machine-path-writes 9/9, state-dir-refusal 5/5, package-files 7/7: 21/21.
    • The "16/16" in the hand-off is machine-path-writes plus package-files.
  • npm run typecheck: clean.
  • npm run check:denylist: denylist: 1310 files clean.
  • fullsuite.sh (420 files; mcp-* and plugin-fetch excluded): 3829 tests, 3761 pass, 11 fail, 57 skipped (measured). None of the 11 is this PR's.
    • 2, measured not this PR's:

      • add-project issue #711 (did not match /sudo -u … claude auth login/);
      • dashboard-speed event cache (read ECONNRESET).

      Each fails the same way, re-run alone, on main d5674d0 and on head. A cause on this machine: the stand-in claude on PATH, or load (guess).

    • 2 isolate tmpdir tests: the Chromium socket path is 179 and 191 bytes under my long scratch TMPDIR. This is the known environmental failure from earlier rounds (measured message; environmental, inferred).

    • 7 listen EADDRINUSE …/docker.sock: bench 1, testrun-env-argv 4 and testrun-sandbox 2. Each socket path is under the same long scratch TMPDIR, over the 107-byte limit (inferred).

    • CI passes every shard on this SHA (measured).

CI and hygiene

Required changes (body only; no code)

  1. Name the protection by design (owner rule, 2026-10-19, relayed). Add a short section saying:

    • the protection is stateDir()'s runtime refusal in a test run, with its .. and symlink refusals;
    • for machine locations, the protection is main's preload system root;
    • the machine-path-writes lint and its GAP fixtures are a backstop.
  2. Name the routes with only the lint behind them, and the new misses, as gaps (header "Not caught", the body's gaps, and the Adversarial pass):

    • writes to the never-moved real paths (asRoot.*, SYSTEM_ROOTS.*, onRealMachine's result), where /var/tmp and /run/lock have nothing else behind them;
    • a12/a13, join('/', onRealMachine(X));
    • s1/s2, a write that climbs out of stateDir()/slotsDir() with ..;
    • a7/a8, literal real paths.

    A // GAP fixture each for a12 and s1 would keep the "known gaps stay named" test honest (cheap, inferred). Literal paths need only a line.

Non-blocking:

  • Refresh the merge-tree list.
  • A by-design guard for the never-moved paths is new scope, and so an owner decision. Two options (inferred):
    • fs write wrappers in the preload that refuse a path outside the test machine, like the builder's probe wrappers;
    • keep asRoot/SYSTEM_ROOTS out of test code's reach.

…l-path and '..' gaps as GAP fixtures

#818 final check. The owner's durability rule (2026-10-19): a protection is by design, and a source lint
only a named backstop. The lint's header now says so: stateDir() and slotsDir() refuse at run time in a test
run, and the preload's system root moves every engine-resolved machine location; the paths it never moves
(asRoot.*, SYSTEM_ROOTS.*, onRealMachine's result) have only this lint.

Named in "Not caught", and two as GAP fixtures (missed today):
- the real path joined under '/' (`join('/', onRealMachine(X))`), past the "under a test's own root" exception;
- a write that climbs out of stateDir() or slotsDir() with '..' parts;
- literal real paths (not modelled).

Test: test/machine-path-writes.test.ts, "the known gaps stay named": 14 GAP lines, none reported (measured).
Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
@jayden1711
jayden1711 merged commit ce744c5 into main Oct 20, 2026
27 of 28 checks passed
@jayden1711
jayden1711 deleted the issue-815-state-dir-refusal branch October 20, 2026 01:02
jayden1711 added a commit that referenced this pull request Oct 20, 2026
…OwnEnv snapshot (#786)

stateDir() read <PREFIX>_STATE_DIR, LOCALAPPDATA and XDG_STATE_HOME by plain property access, and called #818's
testStateRefusal(dir) with the default process.env, whose marks (the test machine, the system root, the token) were
read raw too. A value on Object.prototype moved the state dir, and a test run's child that lost its test machine was
no longer refused when the prototype named one: the raw machine and root paired with the own-only systemRootRefusal,
which reads "unset, nothing to refuse". stateDir() now takes one ownEnv() snapshot, reads the dir through get(), and
hands the refusal env.asProcessEnv(), the pairing slotsDir already uses. The sandboxed project command's LANG is read
the same way, so src/os/index.ts joins the backstop lint's list.

Test (fails without): test/state-dir-refusal.test.ts "#786: a state-dir variable or a test-run mark on
Object.prototype neither moves the state dir nor un-refuses it"; on 2797cfc, 1 of 6 fails, with all four of its
checks wrong (the dir moved twice, two refusals lifted).

Signed-off-by: Jayden Vasquez <jaydenvasquez1711@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tests: stateDir() refuses the real state dir in a test run, like slotsDir() (#806 follow-up)

1 participant