test(e2e): harden suite against config changes that silently break legitimate workflows#87
Merged
Merged
Conversation
… default profile Hardening a sandbox default (a new ProtectedPaths entry, a tightened grant) can silently break a legitimate workflow with nothing to catch it. A snapshot test of the baseline lists wouldn't help — it would only prove the list changed, which git diff already shows in review; it says nothing about the consequences. This instead exercises real developer workflows through a real bwrap/ Seatbelt sandbox under the exact, unmodified sandboxprofile.DefaultProfile() (the template that ships as ~/.config/omac/sandbox-profiles/default.json): git init/add/commit/log/diff, reading ~/.gitconfig, a `go build` that needs GOCACHE/GOPATH under the default's ~/.cache and ~/go grants to actually be writable, common scripting interpreters if present, and the default's filtered-network posture combined with a legitimate open_port grant. A change to baseline.go or DefaultProfile() that breaks any of these now fails fast, locally, with no live agent required. Signed-off-by: Niclas Hülsmann <niclas.huelsmann@tngtech.com>
filesystem.override_deny and filesystem.allow_unix_dir are the documented escape hatches for legitimate access (README's protected-path override and dynamic-socket-dir grant, e.g. Docker/Agent View). Both were previously tested only structurally — grants_test.go checks the Grants struct, bwrap_test.go/sbpl_test.go check generated arg/SBPL strings — with no test proving the actual kernel/policy mechanism grants the access a user following the README recipe expects. Add TestIntegrationOverrideDenyGrantsAccess (Linux + macOS) and TestIntegrationAllowUnixDirGrantsSocketAccess (macOS, real AF_UNIX listener) that run a real bwrap/Seatbelt process and assert the granted access actually works, so a regression in either recipe is caught here instead of only when a user's Docker/daemon-socket grant silently stops working. Signed-off-by: Niclas Hülsmann <niclas.huelsmann@tngtech.com>
…t breaks No test in the repo re-parsed a realistic, on-disk sandbox profile file through the actual Parse -> Validate -> ResolveGrants pipeline. A schema, parser, or resolver change that would silently break an existing user's profile file had no chance to surface until a live e2e run — or a user bug report. Add internal/sandboxrun/testdata/profiles/ with fixtures mirroring real shapes users write today (minimal, workdir+network-allow, the override_deny+allow_unix_dir Docker-socket recipe, a deny-glob), and TestProfileFixturesParseAndResolve, which runs each through the same pipeline TestDenyFullCLIPipeline already exercises for `omac sandbox run`, with spot checks on the resulting grants. Signed-off-by: Niclas Hülsmann <niclas.huelsmann@tngtech.com>
The live security audit (TestE2ESecurityAudit) proves attacker paths are blocked, but had no positive counterpart proving a hardening change (a new ProtectedPaths entry, a tightened deny-glob) doesn't also shadow a path legitimate work depends on. Separately, the AllowanceSpec doc comment claimed a positive FsAllowPaths field existed; it didn't. - audit.sh: add an fs_allow probe (workdir write/read, $HOME/.cache, $TMPDIR) — all guaranteed to exist by the time the script runs, so a denial there means the sandbox blocked it, not that it's absent. - allowance.go: add the real FsAllowLabels field, replacing the aspirational doc comment. - e2e_test.go: add assertFilesystemAllowed/fsAllowDenied, checked per-label (not "any marker anywhere in the section") for the same reason fsReadLeaked/fsWriteLeaked were hardened earlier — one legitimate path silently losing access must not be masked by the others still working. Wired into TestE2ESecurityAudit's positive assertions. - Also extend TestE2EEchoRest's prompt to write/read a workdir file and run a basic git commit, with file-based assertions (git-log.txt, workflow-check.txt) mirroring the existing echo-status.txt pattern — catching a regression in the most basic things a coding agent does every session, riding along on the existing live run. Signed-off-by: Niclas Hülsmann <niclas.huelsmann@tngtech.com>
…ually run internal/sandboxrun's bwrap-based integration tests (TestIntegration*, 15 of them) all call requireBwrap(t), which skips silently when bwrap isn't installed or functional. ci.yml's ubuntu-latest test job never installed it, so every one of those tests — proving the sandbox itself denies/grants access, not just the Grants struct — has been silently skipping in CI. Confirmed empirically: with bwrap removed from PATH, the whole package finishes in 0.008s (all-skip); the real historical ubuntu-latest CI log shows an equally suspicious 1.261s for the same package, versus 21s on macOS where Seatbelt needs no separate setup. Port the bubblewrap install + AppArmor userns allow-rule already proven in e2e.yml (Ubuntu 24.04 restricts unprivileged user namespaces by default) into ci.yml's test job, so these tests genuinely execute on every push/PR instead of only when a developer happens to have a working bwrap locally. Signed-off-by: Niclas Hülsmann <niclas.huelsmann@tngtech.com>
The previous check only asserted git-log.txt was non-empty, which
doesn't actually verify the claim ("a commit was made") — just that
some content landed in the file. Check for the expected commit subject
instead, so a stray unrelated write can't produce a false pass.
Signed-off-by: Niclas Hülsmann <niclas.huelsmann@tngtech.com>
nhuelstng
force-pushed
the
worktree-issue-66-e2e-security-2
branch
from
fa0753b to
1b0ea1b
Compare
…iagnostics Caught by real CI: on macos-latest, TestIntegrationWorkflowGitBattery failed because runSandboxed's CombinedOutput() mixed in a harmless "shell-init"/"chdir: ... getcwd: cannot access parent directories: Operation not permitted" diagnostic that /bin/sh prints when it can't resolve the full parent-directory chain of a narrowly-scoped Seatbelt grant, ahead of the real `git log | wc -l` result. The exact-match `strings.TrimSpace(out) != "2"` check treated that noise as part of the expected value and failed even though the actual git workflow (2 real commits) succeeded. Compare the last whitespace-separated token instead of the whole trimmed output, so the assertion is robust to any such sandbox/shell diagnostic noise. Applied to both platform variants for consistency. Signed-off-by: Niclas Hülsmann <niclas.huelsmann@tngtech.com>
nhuelstng
added a commit
that referenced
this pull request
Jul 15, 2026
#93) requireBwrap, requireUnixSocket, requireLoopback, and requireLoopbackDial all t.Skip() when a runtime capability (bwrap, AF_UNIX, loopback TCP) is unavailable, with no CI-side check that skipped tests didn't just zero out coverage. This already happened for requireBwrap: 15 integration tests silently skipped in ci.yml because bubblewrap wasn't installed there, caught only by chance and fixed in #87. Add skipOrFailCI, mirroring the GITHUB_ACTIONS gating already used in internal/e2e/classify.go: skip locally (a dev sandbox without bwrap or network access shouldn't be blocked), but t.Fatal in CI, where a missing capability is an infrastructure regression, not something to tolerate. Fixes #91 Signed-off-by: Niclas Hülsmann <niclas.huelsmann@tngtech.com>
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
1 participant
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.
What
Follow-up to #66/#84. Adds test coverage for a failure mode the suite had none for: a sandbox hardening change (new
ProtectedPathsentry, tightened default, a regression in a documented escape hatch) silently breaking a legitimate workflow, as opposed to failing to block an attacker.internal/sandboxrun/workflow_integration_{linux,darwin}_test.go): git init/add/commit/log/diff,go build, common interpreters, and a networkopen_portgrant — all run through a real bwrap/Seatbelt sandbox under the exact, unmodifiedsandboxprofile.DefaultProfile().override_deny/allow_unix_dir: these documented escape hatches (Docker socket / custom daemon dir access) were previously tested only structurally (Grants-struct/generated-arg checks). Now proven end-to-end through a real sandboxed process.internal/sandboxrun/testdata/profiles/): checked-in profile JSON mirroring real user shapes, run through the actualParse → Validate → ResolveGrantspipeline.fs_allowprobe in the live security audit (audit.sh/allowance.go/e2e_test.go): proves the workdir, cache dir, and$TMPDIRstay reachable — the positive counterpart to the existingfs_read/fs_writedenial checks, checked per-path for the same reason those were hardened earlier this cycle.TestE2EEchoRestextended with an ordinary git commit + workdir file round-trip, file-asserted like the existingecho-status.txtpattern.ci.yml's Linux test job never installed bubblewrap, so all 15internal/sandboxrunintegration tests (9 pre-existing + 6 new) were silently skipping viarequireBwrap— passing green while testing nothing. Ported the working bwrap+AppArmor install frome2e.yml.assertGitCommitMadenow checks the commit subject, not just non-empty file content.Why
Explicitly requested: "how can we ensure we make no breaking (config) changes that makes the users stopping the tool?" The suite proved attacker paths are blocked but had no signal for the opposite regression. Deliberately did not add a golden/snapshot test pinning the exact baseline lists — that only proves a default changed (which
git diffalready shows in review), not whether it breaks anything real. These tests instead exercise real consequences.The CI fix was found by auditing this work rather than trusting green CI: the bwrap-based tests looked correct in isolation but a check of
ci.ymlshowed bubblewrap was never installed there, confirmed empirically by removing bwrap fromPATHlocally and observing the same near-instant "all-skip" signature as the historical ubuntu-latest CI log.How
See commit messages for per-workstream detail.
internal/sandboxrunchanges are plain Go tests (noe2ebuild tag, no live agent).internal/e2echanges ride along on the existingTestE2ESecurityAudit/TestE2EEchoRestlive runs — no new live-agent cost.Verification
go build ./...,go vet ./...,go vet -tags e2e ./...,gofmt -l .— clean.go test ./...— green (includes the newinternal/sandboxruntests, verified locally with a real bwrap; will now also run for real in CI per theci.ymlfix).GOOS=darwin— clean.workflow_dispatch,skip_claude_code=true) run twice against this branch: https://github.com/TNG/oh-my-agentic-coder/actions/runs/29352091511 — all 7 harness×OS combinations green (onecopilot/macos-latestrun hit an unrelated agent-refusal flake on the first attempt — every probe in that job failed identically, indicating the audit script never ran at all, not a regression; the rerun passed cleanly).