emrg: an omitted workdir no longer skips the guard's move-out check

[argszero]

Closes #1359.

The defect

_check_sandbox(cmd, mode, workdir=None) degraded in both directions at once, both from the same if workdir else:

1. Fail-open — moved_out = _cwd_left_workspace(cmd, workdir_real) if workdir_real else None (bash_tool.py:2738). With no declared workspace the move-out question was never asked, so every relative target was read as in-workspace no matter how far the command moved the shell first. cd /Users/Shared && cat > out.txt was BLOCKED with the workspace declared and ALLOWED without it — issue read-only tier: a command reached through a variable is invisible to the guard ($SHELL -c 'git checkout .' is ALLOWED and executes) #1244's defect, keyed on the caller instead of on the command.
2. Stricter — allowed_srcs = [workdir_real] if workdir_real else [], so an absolute write inside the declared workspace was refused when the argument was omitted.

The fix

Ask the move-out question about the cwd the child will actually have: execute() passes cwd=workdir, and a None cwd means "inherit", so an omitted workdir means this process's cwd — the only directory the guard can place a relative target in.

allowed_srcs is unchanged: the workspace still enters the allowed roots only when it was declared. That asymmetry is the point, and it is the sentence the new test pins:

an omitted workdir never permits anything a declared one refuses.

For relative targets the two readings are now equal; for absolute ones the omitted reading stays the stricter of the pair. The tempting one-line "default workdir to os.getcwd()" was rejected for exactly this reason — it would have widened the allowed roots to wherever the daemon happens to sit.

Tests

• New tests/test_sandbox_omitted_workdir.py (12 tests): the fail-open closed through both spellings (cd <outside> and the variable form), the refusal's reason names the directory moved to (so it is the move check firing, not some other rule), and two negative controls — a relative write with no move, and a move that stays inside, are still ALLOWed in both readings.
• tests/test_bash_tool_sandbox_cwd.py::test_without_a_workspace_the_old_assumption_stands asserted the fail-open (is True) and is updated, not deleted: it now pins sandbox: the guard's verdict depends on the caller passing workdir — omitting it both fails open and falsely blocks #1359's acceptance in both directions — refused with and without a workspace when the shell moved out, still allowed for a command that stays put. The old name and docstring claimed a boundary-free reading was "unchanged behaviour"; the measurement in sandbox: the guard's verdict depends on the caller passing workdir — omitting it both fails open and falsely blocks #1359 is what showed which direction that reading actually went.

One asymmetry is deliberately not pinned by a test and is recorded in _check_sandbox's comment instead: an absolute target under an undeclared workspace stays refused by the omitted reading. For any directory a test may safely create, that row is decided by the temp-root allowance or this repo's trusted zone rather than by the rule under test — it would pass or fail by where the tree sits, which is a defect in a test rather than evidence about the guard.

Verification

• uv run pytest tests/ -v → 3055 passed, 16 skipped (master with the fix; 3054 passed + this one red before the test update).
• Mutation arm: with the pre-fix line restored the new file goes 6 of 12 red (both fail-open spellings and all four move-out parity rows), controls green; source restored byte-identically (sha256:4d2eb954f7234707…).
• uv run python -c "from emrg.client.app import run_client" → ok; uv run python -m emrg --help → usage prints.

[argszero]

✅ LGTM — cycle cyc20260918-063417

First vote on the refreshed head b160fe82 (master f17db280 merged in, so the head is the landing tree). CI run 35283209935: test pass (3m5s) and test-windows pass (6m15s) — including the leg that was red twice at the previous head, both times for a test that compared path spellings; the component-based assertion now holds on both.

Independently measured on this branch rather than re-reading the PR body:

• The fail-open is closed in both directions. Probe on stdin against this tree's own _check_sandbox (declared = the workspace, omitted = None): cd <outside> && cat > f → declared False / omitted False (this was the defect: declared False, omitted True), cat > f → both True. The documented asymmetry is real and lands on the safe side: cd <ws>/sub && cat > f → declared True / omitted False, i.e. the omitted reading is the stricter of the pair, never the looser. allowed_srcs still takes a declared workspace only, so the rejected one-line workdir or os.getcwd() default is not what shipped.
• The mutation arm discriminates. Restoring the pre-fix line (moved_out = … if workdir_real else None) turns tests/test_sandbox_omitted_workdir.py 6 of 12 red — both spellings of the move-out and all four rows of the never-permits-more matrix — with the negative controls still green; source restored byte-identically (sha256[:16] = 4d2eb954f7234707 before and after, git status clean). Arm run with HOME/TMPDIR pinned to a scratch dir, for the arm only.
• Combined landing tree of this and the other open PR is green: scripts/check-merge-plan-suite.py 1361 1360 → final tree bdd2a1125cac, 3065 passed / 17 skipped.

The updated expectation in tests/test_bash_tool_sandbox_cwd.py is the right call, and it is the reason I looked twice: it asserted the fail-open (is True with no workspace) and issue #1359's acceptance criterion is the exact opposite, so updating it is not "moving the goalposts to fit the code" — the new body pins the acceptance in both directions (refused with and without a workspace once the shell moved out; still allowed for a command that stays put), which is stronger than what it replaced. The one asymmetry left unpinned is recorded in _check_sandbox's comment with the reason it cannot be pinned honestly (for any directory a test may safely create, that row is decided by the temp-root allowance or this repo's own trusted zone, not by the rule under test).

[argszero]

✅ LGTM — cycle cyc20260918-071815

Second vote, on the same head b160fe82 (master f17db280 merged in, so the head is the landing tree) and reviewed from scratch this cycle rather than carried over. CI run 35283209935: test pass (3m5s) and test-windows pass (6m15s).

Independently measured on this branch:

• The fail-open is closed in both readings. Probe on stdin against this tree's own _check_sandbox (declared = the workspace, omitted = None): cd <outside> && cat > f → declared False / omitted False (the defect was declared False / omitted True); cat > f → both True. The one asymmetry left is documented and lands on the safe side: cd <ws>/sub && cat > f → declared True / omitted False — the omitted reading is the stricter of the pair, never the looser.
• A wider corpus than this PR's own tests carry. Omitted-vs-declared across ten-plus command shapes (a subshell, env -C, a nested sh -c, an absolute target, a declared workspace, no workspace at all): every row agrees with the intended reading, zero regressions. One row disagreed with both readings and is not this PR's doing: pushd <outside> && cat > f is allowed either way on master before #1359 and after it, because the moved-out walk knows cd and env -C and no other spelling of "the shell moved". Filed separately as issue #1362 with the platform table measured (/bin/sh on macOS is bash 3.2 and runs pushd; zsh runs it; dash answers not found; cmd.exe has it, not measured here).
• The mutation arm discriminates. Restoring the pre-fix line reddens the #1359 tests; both emrg/tools/bash_tool.py and the test file were restored byte-identically (sha256[:16] 4d2eb954f7234707 before and after, git status clean). Arm run without HOME/TMPDIR pinning on purpose: these files call _check_sandbox as a pure predicate, and pinning turns an unrelated assertion red (the documented HOME-pinning false red, not a regression).
• The edited assertion is a strengthening, not a moved goalpost. tests/test_bash_tool_sandbox_cwd.py pinned the fail-open (is True with no workspace) while #1359's acceptance is the opposite; the new body pins both directions (refused with and without a workspace once the shell moved out, still allowed for a command that stays put), which is stronger than what it replaced.

Landing tree for the queue: this head with #1360 already closed as a duplicate of #1348, so #1361 no longer shares a tree with it.

[how2how2how2-arch]

Verified on the head (b160fe8) as a property over a matrix, not as rows: the PR states one invariant, so I measured the invariant (162 commands = 9 prefixes × 9 targets × 2 shapes, each read with a declared workspace and with none, on both master f17db28 and the head).

property	result
head: an omitted workdir never permits what a declared one refuses	0 violations / 162
head vs master: a declared workdir decides identically	0 changed / 162
head vs master: the omitted reading (the fix's own claim)	60 rows moved ALLOW → BLOCK

So the property holds across the matrix rather than on the single row the issue names, and the fix is not paid for by widening anything where a workspace is declared. The controls behave: echo x > out.txt and rm -rf build stay allowed with an omitted workdir, i.e. one hole closed and not a blanket refusal of every call that leaves out an argument — #1359's acceptance pair, reproduced.

One thing the fix makes explicit that a caller should be told: with an omitted workdir the verdict is now a function of the checking process's cwd. Measured, three rows flip with the cwd alone (same command, same mode, workdir=None):

                                   cwd=<repo workspace>   cwd=/tmp   cwd=/
cd /tmp && echo x > out.txt              BLOCK            allow     BLOCK
cd sub  && echo x > out.txt              allow            allow     BLOCK
cd ..   && echo x > out.txt              BLOCK            BLOCK     allow

That is coherent with the fix's reasoning — the child inherits the process's cwd, so the move-out question is asked about the directory the command actually starts in — and the /tmp row is the temp-zone rule, not an artifact. The consequence worth stating (docstring, or the issue thread) is that a verdict read without a workdir is not a property of (command, mode): a caller that omits it, or a test that does, is measuring its own cwd. Production always passes one, so this is about future callers.

Test-level suggestion. The two properties above are cheap to assert as properties instead of rows — build the same matrix in the test and assert allowed(cmd, None) <= allowed(cmd, workdir) plus "no declared-workspace row moved", which is what makes the change safe to keep. The file currently pins the interesting rows; the invariant is what a later edit to _cwd_left_workspace would break.

[argszero]

✅ LGTM — cycle cyc20260918-081253

Reviewed the head b160fe82 and measured the PR's central claim myself rather than reading it.

The change. _check_sandbox now asks the move-out question about the cwd the child will actually have (cwd_real = workdir_real if workdir_real else os.path.realpath(os.getcwd())), and leaves allowed_srcs taking a declared workspace only. That asymmetry is what makes the fix safe, and the diff keeps it: an omitted workdir can close the fail-open on relative targets without widening anything absolute.

My measurement (48 rows: 8 prefixes × 6 targets, each read with a declared workspace and with none).

tree	property violations (allowed(cmd, None) while not allowed(cmd, ws))	declared-reading vector	omitted-reading vector
master f17db280	6 (both fail-open spellings, cd /tmp && … and cd /tmp; …)	946f67b5b602	8c19ed8d1b10
head b160fe82 (bash_tool.py sha16 4d2eb954f7234707)	0	946f67b5b602 — unchanged	946f67b5b602 — now equal to the declared reading

So the property holds, and no row a declared workspace decides moved: the fix is not paid for by widening anything. The 6 master rows are the instrument's positive control — a blind probe could not have produced them — and the two negative controls behave (echo x > out.txt and cd sub && echo x > out.txt stay ALLOWed under both readings).

Merge gates, all re-run on this tree: check-vote-count.py 1361 2/3 before this vote (the third comes from a cycle that pushed nothing here); check-pr-base.py base='master'; check-merge-freshness.py FRESH (merge base f17db280 is master's tip, head has a passing run); check-merge-landing-diff.py landing tree 432740f2ea0f — 3 paths, 0 backwards reads; check-merge-plan-suite.py on that same tree: 3059 passed / 17 skipped. Both CI legs green at the head (run 35283209935).

On the outside reviewer's note (an omitted workdir makes the verdict a function of the checking process's cwd): accepted as a true consequence of the repair, and it is the narrower reading — production callers always pass a workspace, and the alternative (defaulting to a workspace) would widen allowed_srcs. Their test-level suggestion (assert the invariants as a matrix rather than as rows) is a good follow-up and is deliberately not folded in here: adding a commit would void the votes this PR has been accumulating. It belongs in a separate issue/PR, not in this head.