fix(security): sandbox read-only runs on every harness; move notify queues out of the repo - #787
Merged
Merged
Conversation
…ueues out of the repo Three linked defects, fixed together because the first two were what made the third unfixable. 1. .pending-<skill>.md was being COMMITTED. .gitignore ignores `.pending-*/` (directories), not the file, so the json-render staging file was tracked. A sandboxed harness that could not overwrite it left the PREVIOUS run's copy in place, and "Capture skill output" published that as the new run's output - a green run reporting another run's work, and feeding it to the health scorer. Measured live: four consecutive runs captured a byte-identical artifact from a fifth. 2. notify's queues (re-delivery, dedup hashes, json-render staging) lived in the workspace, so under the OS sandbox they could not be written at all. They now live in $AEON_PENDING_DIR, exported by the workflow and defaulted in notify.sh so local runs and other entry points keep working. 3. That was the ONLY reason claude and grok ran with --no-sandbox, so read-only skills on those two had no runtime enforcement. On grok it was total: its adapter runs --permission-mode bypassPermissions and never applied an allowlist, so a read-only skill could write anywhere in the repo - verified by asking one to create a file, which it did - with only the post-run guard reverting it afterwards. Both now run under the wrapper sandbox. Bubblewrap installation had to be hoisted out of "Install harness CLI" (which skips claude entirely and skipped grok by name) into its own step, or run-harness would have found no bwrap and silently degraded to "read-only is advisory" with nothing but a stderr notice. Also: the read-only guard now cleans what it reverts. `git checkout` only undoes edits to TRACKED files; the `git clean` list covered 6 of the ~15 CODE_PATHS, so a file a read-only skill CREATED outside those 6 survived and was committed by `git add -A`. Both lists are now the same set. Verified live on a real instance (grok, read-only, MCP enabled): `bwrap: userns OK` -> `read-only: workspace write-locked via bwrap` -> glim MCP still reachable -> chain artifact captured at this run's own 1169 bytes instead of the stale 3436-byte digest that four earlier runs had republished.
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.
Three linked defects. Fixed together because the first two were what made the third unfixable.
1.
.pending-<skill>.mdwas being committed.gitignoreignores.pending-*/— directories, with the trailing slash. The json-render staging file wasn't covered, so it was tracked.A sandboxed harness can't overwrite it. "Capture skill output" prefers it. So the step published the previous run's digest as the new run's output — and handed it to the health scorer. Measured on a live instance: four consecutive runs captured a byte-identical 3436-byte artifact belonging to a fifth.
2. notify's queues lived in the workspace
The re-delivery queue, the dedup hash file and the json-render staging file were all written into the repo. Under the OS sandbox they simply can't be — the run loses its dedup state, its retry queue, and its feed entry.
They now live in
$AEON_PENDING_DIR(exported by the workflow, defaulted innotify.shso local runs and other entry points keep working).3. …which was the only reason
claudeandgrokskipped the sandboxWith the queue out of the workspace,
--no-sandboxhas no justification left. It mattered most on grok, whose adapter runs--permission-mode bypassPermissionsand never applied an allowlist — so a read-only grok skill had no runtime enforcement at all.Not theoretical. I asked one to create a file:
run-harness grok --mode read-only--mode read-only --no-sandbox(whataeon.ymlpassed)Only the post-run guard reverted it, after the fact. Both claude and grok now run under the wrapper sandbox.
Bubblewrap had to move. It was installed inside "Install harness CLI", which skips claude entirely and skipped grok by name. Enabling the sandbox without fixing that would have found no
bwrapand silently degraded to "read-only is advisory" with nothing but a stderr notice — a worse outcome than before, because it would look enforced. It's now its own step covering every harness.Plus: the guard now cleans what it reverts
git checkoutonly undoes edits to tracked files — it cannot remove a file the run created. Thegit cleanlist covered 6 of the ~15CODE_PATHS, so a new untracked file outside those 6 (e.g. underapps/dashboard/lib/, or a brand-new top-level directory) survived the guard and was committed bygit add -A. I reproduced it. Both lists are now the same set. No-x, so gitignored build artifacts are untouched;memory/andoutput/are in neither list and still commit by design.Verified live, not by inspection
A real instance, grok, read-only skill, MCP enabled:
The skill still reached glim over MCP and returned live data (HN item
49072361, score 111), and "Skill output captured to output/.chains/glim-mcp.md (1169 bytes)" — this run's own output, where four previous runs had all republished the same stale 3436-byte file.Test suite:
test_notify.shrepointed at$AEON_PENDING_DIR, its read-only case rewritten to assert the new, stronger property (a read-only cwd still queues, because the queue is outside the workspace) and a new case added for the original fail-soft path (unwritable queue → inline delivery, noset -eabort). All suites pass;okf-validateOK;actionlintclean.Note for operators
Instances that already committed a
.pending-<skill>.mdshould delete it once (git rm --cached) — the new.gitignorerules stop it recurring. I did this on the instance I tested against.