Repository navigation
feat(cli): apply stays inside the worker's write dirs (#1082) - #1087
Merged
Merged
Conversation
Six rows measured against main 0c9e936 with only this file present: an outside destination, an outside tree, the .bak-N slot, an empty allowlist, the nested-worker default, and show --diff reporting the refused target.
The CLI apply had no write boundary; the MCP surfaces did. Build the same check from surface_common.ensure_write_path and pass it into the one apply_with_target call, so a destination, a materialised tree and the .bak-N slot are all refused before anything is written. An unset AGENTDESCENT_SURFACE_WRITE_DIRS keeps the terminal behaviour (write anywhere). Inside a host worker (AGENTDESCENT_NESTED=1) it now defaults to the MCP boundary, cwd plus $AGENTDESCENT_HOME, because a nested process writing outside its run is the escape this guards. An explicitly empty value refuses every write. show --diff writes nothing, so it still renders; its apply_plan reports the allowlist error instead of promising a refused write.
HugoXK
added a commit
that referenced
this pull request
Nov 14, 2026
) Row 3 of #1082: the run `launch` detaches is our own process, so it inherits all of os.environ -- including the worker session's DSH_*/CLAUDE*/CODEX*/MCP_* variables and its MCP server config. Drop those prefixes with surface_common.scrub_host_env; the API keys and PATH the run needs still pass through. Row 1 (the CLI write allowlist) already has its own carrier in #1087, so this change does not touch it. Refs #1082: the per-Ticket budget-cap row is still open.
HugoXK
marked this pull request as ready for review
November 15, 2026 04:04
Collaborator
|
Verdict: PASS Tip SHA: Ran:
Not run: a real dsh-superagent worker end-to-end. |
Birfy
pushed a commit
that referenced
this pull request
Nov 15, 2026
…) (#1088) Partial: `#1082` — row 3. Row 1 (the CLI write allowlist) is `#1087`, merged as `f5e93335`; row 2 (per-Ticket budget caps) stays open on the issue. ## What changed `runstore.launch` built the detached run's environment from all of `os.environ`, so the worker's own session leaked into the run: its `DSH_*` markers, its `CLAUDE*`/`CODEX*` host markers, and any `MCP_*` server configuration that would hand the run a second set of tools. The run is our own process, so the candidate allowlist (`runners._child_env`) cannot be reused — it would drop the API keys and `PATH` the run needs. - `agentdescent/surface_common.py`: the package's one host-session denylist `SESSION_MARKERS`, `is_session_marker`, the `DETACHED_KEEP_VARS` keep set, and `scrub_host_env(environ=None)`. - `agentdescent/agents.py`: imports the list from `surface_common` instead of carrying a second copy. - `agentdescent/runstore.py`: `launch` builds the child env with `scrub_host_env({**os.environ, ...})`, so `env=` cannot put the session back. - `tests/test_host_env_scrub.py` (new): drop, keep, one-list, four-host-root, isolation and launch rows. - `docs/dsh-super-agent.md` §5 and `changelog.d/1082-host-env-scrub.md`. No model or network call. `cli.py` is untouched: the `apply`/`show --diff` write allowlist is `#1087`'s row, and the budget caps are row 2. ## Review findings 1. **The deny list deleted host config dirs the run reads.** `integrations._host_root` reads four variables — `DSH_HOME`, `CODEX_HOME`, `XDG_CONFIG_HOME` (for `_opencode_root`) and `CLAUDE_CONFIG_DIR` — so `DETACHED_KEEP_VARS` keeps all four by exact name. Everything else in `SESSION_MARKERS` still goes, `OPENCODE_CONFIG_DIR` and `OPENCODE_CONFIG_CONTENT` included. The measurement is under "Verification at the tip": all four survive with their values, the four roots resolve against them, and the session markers are gone. 2. **Second, divergent list.** `SESSION_MARKERS` lives in `surface_common` and `agents` imports it; a row asserts the two names are the same object, and the parametrised row walks the tuple, so a marker added for a new host is covered the day it is added. ## Red at base `c94e9d67` Fresh tree at the base this PR targets, with only `tests/test_host_env_scrub.py` copied in: ``` tests/test_host_env_scrub.py:16: in <module> from agentdescent.surface_common import scrub_host_env E ImportError: cannot import name 'scrub_host_env' from 'agentdescent.surface_common' ERROR tests/test_host_env_scrub.py !!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!! 1 error in 2.10s ``` ## Red before the keep sets At `c5f021e9`, with no keep set: `CLAUDE_CONFIG_DIR survives: False`, `CODEX_HOME survives: False`, `DSH_HOME survives: False`, `XDG_CONFIG_HOME survives: False`, and `_opencode_root` returned `<home>/.config/opencode` while `XDG_CONFIG_HOME` pointed at `<home>/.config-work`. At `50bde3c2`, with the keep set holding three and the four-variable rows in the tree: `pytest -q tests/test_host_env_scrub.py` -> `4 failed, 18 passed`, every failure `KeyError: 'XDG_CONFIG_HOME'`. Tip SHA: 85b2974 Commits: `b5b66bf8` (the original scrub), `9766bad1` (forward-merge onto `c94e9d67`), `c5f021e9` (one list, and `launch` re-scrubs the merged env), `50bde3c2` (keep the three config dirs), `85b2974` (keep `XDG_CONFIG_HOME`, isolation rows asserted by value). Fast-forward only: no force, no rebase, no merge-up. ## Verification at the tip Ran: - probe over the review variables -> `CLAUDE_CONFIG_DIR`, `CODEX_HOME`, `DSH_HOME`, `XDG_CONFIG_HOME` survive with their values; `CLAUDECODE`, `CODEX_SESSION_ID`, `DSH_SESSION_ID`, `MCP_SERVERS`, `OPENAI_AGENT_ID` dropped - `integrations._host_root` re-resolved with the scrubbed env in place -> `claude <home>/.claude-work`, `codex <home>/.codex-work`, `dsh <home>/.dsh-work`, `opencode <home>/.config-work/opencode` - `pytest -q tests/test_host_env_scrub.py` -> pass (22 passed) - focused set (`test_host_env_scrub`, `test_runstore_cli`, `test_runstore_tilde`, `test_cli_surface_p1`, `test_mcp_write_confinement`) -> pass (102 passed) - `pytest -q tests/test_plugin_runner.py` -> pass (15 passed) - `python -m tools.gen_api_docs --check` -> pass (`docs/api.md is up to date`) - `python -m tools.changelog_fragments check` -> pass (40 fragments OK) - `mkdocs build --strict` -> exit 0 (13.47 s) - full `pytest -q` at this tip: the 492 files from `ls tests/test_*.py | sort`, split into 8 even chunks and run in parallel as `pytest -q <chunk files>` with an `AGENTDESCENT_HOME` per chunk -> 7418 passed, 12 failed, 48 skipped, 10 errors. The red set is the environment set: `test_era_srbench` 6, `test_genesis_example` 2, `test_sandbox_container` 2 failed + 10 errors (the docker image cannot be pulled on this host), `test_wheel_smoke` 1, and `test_integrations::test_the_launcher_install_writes_ignores_a_shadowing_cwd` 1, which fails identically at base `c94e9d67` (a stale console script in this box's venv). None of them is in `runstore.py`, `surface_common.py`, `agents.py` or `tests/test_host_env_scrub.py`. Not run: - the docker sandbox, `test_era_srbench`, `test_genesis_example`, wheel-smoke and read-only-`$HOME` legs -> environmental on this host Size: 6 files, `+397/-27`. T3: over the 250-line cap, and `risk:T3` from the controller matches.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Row 1 of
#1082(#963remainder):applyobeys the operator's write allowlist.What changed (
20a75044, on0c9e936d)agentdescent/cli.py:_operator_write_guard()builds acheck_writefromsurface_common.ensure_write_path(SurfaceError→RunStoreError, exit 2) andcmd_applypasses it into the one existingapply_with_targetcall._check_apply_pathsalready checks the destination, every written leaf and the.bak-Nslot before anything is written, so the refusal lands before disk is touched — file and tree/skill-library,--from,--install-toincluded.showwrites nothing, so--diffstill renders;_refuse_plan_outside_write_dirs()makes itsapply_planreport the same refusal instead of promising a write thatapplywould reject.Semantics — three states, not two
AGENTDESCENT_SURFACE_WRITE_DIRSAGENTDESCENT_NESTED=1cwd+$AGENTDESCENT_HOMEallow=None)The nested default is the amendment on the plan: a host worker that is handed no variable must not be free to write anywhere, which is the escape
#963is about. Interactive CLI behaviour is unchanged.Red at base
0c9e936dClean tree at the base this PR was cut from, with only
tests/test_mcp_write_confinement.pypresent:6 failed, 17 passed-- every new refusal row red, the two positive rows green as they should be. Live main has moved on (c94e9d67); this PR's base is0c9e936dand the branch is not refreshed.Verification at the tip
Tip SHA: 20a7504
Ran:
pytest -q tests/test_mcp_write_confinement.pyat the tip -> pass (23 passed)test_apply_backup_away_from_skills,test_apply_from,test_apply_noop,test_apply_symlink_dest,test_cli_surface_error_envelope,test_cli_surface_p1,test_public_surface,test_rsi_surface_integration,test_skill_library_apply_install,test_surface_governed_integrity,test_surface_out_link_guard) -> 102 passed, 7 failed; the same 11 files on a detached tree at base0c9e936d-> 102 passed, 7 failed, the same ids, every oneOSError: [Errno 30] Read-only file system: '/home/xuke/.agentdescent'(these tests write~/.agentdescentbehind a read-onlyHOME), so none of the 7 is this diffpytest -q tests -k "apply or surface"at the tip -> 168 passed, 7 failed, 7237 deselected (the same 7 ids)0c9e936dwith only the new test file copied inpython -m tools.gen_api_docs --check-> pass (docs/api.md is up to date)python -m tools.changelog_fragments check-> pass (34 fragment(s) OK)Not run:
pytest -qat this tip -> not run in this pass; CItest (3.9)--(3.13)is green at20a750447ccf, and the base red above is the fail-on-main evidencemkdocs build --strict-> not run in this pass; CIbuildis green at the tipDocs/notes
docs/plugins.mdgains the worker-default rule;docs/dsh-super-agent.mdnotes it next to the long-run page;changelog.d/1082-cli-write-dir-enforcement.md.apply --install-to <host dir>for host directories outsidecwd+$AGENTDESCENT_HOME.Partial: #1082— rows 2 (per-Ticket budget caps) and 3 (child-env scrub test) are still open.