Repository navigation
fix: sandbox Hermes gateway validation to protect real user units - #284
Merged
Merged
Conversation
…ser unit A no-mistakes validation run once started `hermes gateway run` with a throwaway HERMES_HOME under `~/.no-mistakes/worktrees/` but the real HOME. Hermes resolves its user unit through `Path.home()`, and its own temp-home belt only rejects /tmp and pytest paths, so the start rewrote the captain's `~/.config/systemd/user/hermes-gateway.service` to point at that worktree. - `bin/fm-hermes-sandbox.sh` runs any Hermes command with HOME, XDG_CONFIG_HOME, and HERMES_HOME confined to a throwaway root, refuses roots that overlap the real home, proves `Path.home()` stayed inside, and fails the run when a watched `<home>/.config/systemd/user` changed. - `tests/fm-hermes-sandbox.test.sh` reproduces the unguarded rewrite, proves the guard confines it, exercises every refusal, and runs the installed Hermes path when present. It records the real unit checksums before and after, which stay identical. - `.no-mistakes.yaml` test instructions and the ship brief rules point test steps at the guard; the plugin README owns the gateway testing rule and the script header owns the mechanics.
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.
Summary
One executable guard, one regression, three pointers.
bin/fm-hermes-sandbox.sh(new): resolves every sandbox destination physically and refuses symlinks that leave the root or touch a real home or unit directory; walks the sandbox unit tree so a linked unit cannot be followed into the real one; exportsHOME,XDG_CONFIG_HOME,XDG_DATA_HOME,XDG_CACHE_HOME,XDG_STATE_HOME, andHERMES_HOME; unsetsXDG_RUNTIME_DIRand pointsDBUS_SESSION_BUS_ADDRESSat/dev/nullso a check cannot reach the real user manager; checksums the watched real unit directories before and after the command and exits 70 on any change.tests/fm-hermes-sandbox.test.sh(new): the regression, 15 assertions..no-mistakes.yaml: the gate's test instruction points at the guard and forbids service-control subcommands.bin/fm-brief.sh: ship rule 9 says the same for every future brief.contrib/hermes-gateway-firstmate-comms/README.md: "Testing a gateway" owns the rule; the header owns the mechanics.docs/scripts.mdandbin/fm-test-run.sh: script inventory and family/changed-file mapping for the new test.Evidence
Before - on the base commit, a gateway started with a throwaway
HERMES_HOMEbut the realHOMErewrites the real user unit. The regression's first case reproduces it with a stub and a fake real home:After - the same command through the guard, and the same test file at head:
Recorded real
~/.config/systemd/userchecksums are identical before and after, includinghermes-gateway.service ac35d99f5a87f77320bb79dac1ee5bd53d8a25eb0baea6aaa674a26c5976332d..no-mistakes.yamltest instruction,bin/fm-brief.shrule 9, plugin README; this PR's checksInventory of every tracked
hermes gatewaystart andHERMES_HOMEset (grep -rnE "hermes gateway|HERMES_HOME" tests bin docs skills .agents/skills contrib .no-mistakes.yaml):hermes gatewaybin/fm-spawn.sh,bin/fm-hermes-turnend-hook.shHERMES_HOMEfor TUI/hookssystemd_install,systemd_start,systemd_restart, andrun_gatewaybin/fm-ext-lib.sh, plugin, bridge testsHERMES_HOMEreadstests/fm-hermes-live-e2e.test.sh,docs/verification/runtime-backends.mdHERMES_HOME, realHOMELocal gates at head:
FM_LINT_JOBS=1 bin/fm-lint.shclean;bin/fm-test-run.sh --allran once (209 scripts, 55 failures, none in this change: 29 pass with the task-worker environment scrubbed, 23 reproduce at base, 3 are Herdr lanes contending with live fleet sessions).Merge Danger
Door: two-way. Revert the commit and the guard, test, and instruction pointers disappear; nothing in the live gateway,
~/.hermes, or thefmcommsprofile is touched by this change or by its revert.Blast Radius: test and validation only. The YAML instruction is read by the gate from the default branch, so merging is what starts steering validation runs to the guard.
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 4 issues found → auto-fixed (2) ✅
bin/fm-hermes-sandbox.sh:202- The confinement check compares path strings without resolving symlinks. With a reused --root whose home/.config points to the real ~/.config, gateway refresh writes the real unit: mkdir accepts the link, the prefix check passes, and Path.home() returns the lexical sandbox HOME. Resolve and validate filesystem destinations before execution, including HOME, HERMES_HOME, all four XDG directories (lines 185–205), and HOME/.config/systemd/user. The post-write tripwire cannot prevent the damage.bin/fm-hermes-sandbox.sh:250- HOME isolation does not isolate systemctl --user. The command inherits the real user bus and XDG_RUNTIME_DIR, so gateway start/restart can operate the captain's existing service on the supported Hermes version where arbitrary HERMES_HOME paths share hermes-gateway.service (contrib/hermes-gateway-firstmate-comms/README.md:13). Its service process then uses the live unit's environment despite this wrapper's exports. gateway install also reaches the real manager. Refuse these service-control operations or isolate their manager access before exec; apply the same boundary to the install recipe at line 54 and the gateway instructions in .no-mistakes.yaml:49, bin/fm-brief.sh:755, and contrib/hermes-gateway-firstmate-comms/README.md:22.tests/fm-hermes-sandbox.test.sh:155- Criterion 2 requires using the real Hermes install when available and asserting that 'the sandboxed unit is the one written.' This added probe calls refresh_systemd_unit_if_needed() with no existing sandbox unit (line 160), then explicitly requires False (lines 173–174). Installed Hermes immediately returns False when that file is absent, so the real-install case never exercises the failing rewrite. Replace this no-write probe with a safely isolated stale-unit rewrite and assert the resulting sandbox unit and unchanged fake-real unit.bin/fm-hermes-sandbox.sh:86- Simplification: the added --keep mode (lines 91–94, 112, 262–264; tests/fm-hermes-sandbox.test.sh:138–143) and --root= alias (lines 86–90) exceed the requested simplest guard. Explicit --root already supports retained fixtures and live-check setup. Remove --keep and the alternate argument spelling; neither is required by the acceptance criteria.🔧 Fix applied.
4 issues (3 errors, 1 warning) still open:
bin/fm-hermes-sandbox.sh:202- The confinement check compares path strings without resolving symlinks. With a reused --root whose home/.config points to the real ~/.config, gateway refresh writes the real unit: mkdir accepts the link, the prefix check passes, and Path.home() returns the lexical sandbox HOME. Resolve and validate filesystem destinations before execution, including HOME, HERMES_HOME, all four XDG directories (lines 185–205), and HOME/.config/systemd/user. The post-write tripwire cannot prevent the damage.bin/fm-hermes-sandbox.sh:250- HOME isolation does not isolate systemctl --user. The command inherits the real user bus and XDG_RUNTIME_DIR, so gateway start/restart can operate the captain's existing service on the supported Hermes version where arbitrary HERMES_HOME paths share hermes-gateway.service (contrib/hermes-gateway-firstmate-comms/README.md:13). Its service process then uses the live unit's environment despite this wrapper's exports. gateway install also reaches the real manager. Refuse these service-control operations or isolate their manager access before exec; apply the same boundary to the install recipe at line 54 and the gateway instructions in .no-mistakes.yaml:49, bin/fm-brief.sh:755, and contrib/hermes-gateway-firstmate-comms/README.md:22.bin/fm-hermes-sandbox.sh:86- Simplification: the added --keep mode (lines 91–94, 112, 262–264; tests/fm-hermes-sandbox.test.sh:138–143) and --root= alias (lines 86–90) exceed the requested simplest guard. Explicit --root already supports retained fixtures and live-check setup. Remove --keep and the alternate argument spelling; neither is required by the acceptance criteria.bin/fm-hermes-sandbox.sh:208- Round 1 fixed directory symlinks but left unit-file symlinks unchecked. Reusing --root with home/.config/systemd/user/hermes-gateway.service linked to the real unit passes every directory check; the rewrite modeled at tests/fm-hermes-sandbox.test.sh:54–56 then follows that link and overwrites the real unit. The tripwire detects damage only afterward. Validate existing hermes-gateway*.service destinations before execution, including default and profile units. tests/fm-hermes-sandbox.test.sh:157 currently covers directory symlinks only; include a linked unit file in that behavioral coverage.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
TMPDIR="$PWD/.sandbox-validation/tmp" PYTHONDONTWRITEBYTECODE=1 bash tests/fm-hermes-sandbox.test.shpython3 ~/.no-mistakes/evidence/01M6256Q8VCHS2612XHHP1G8ZJ/drive-boundaries.pypython3 ~/.no-mistakes/evidence/01M6256Q8VCHS2612XHHP1G8ZJ/drive-sandbox.py— installed Hermes foreground startup with a stale sandbox unit, followed by SIGINT shutdownrg -n 'hermes gateway|HERMES_HOME' tests bin docs skills .agents/skills contrib .no-mistakes.yamland inspection of relevant callersCompared before/after SHA-256 maps for all real user unit files, removed disposable fixtures and confirmed a clean working tree✅ **Document** - passed
✅ No issues found.
🔧 **Lint** - 1 issue found → auto-fixed ✅
🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Push** - passed
✅ No issues found.