Skip to content

fix(agent): sandbox process execution - #5818

Closed
RaresKeY wants to merge 18 commits into
odysseus-dev:devfrom
RaresKeY:fix/agent-linux-sandbox
Closed

fix(agent): sandbox process execution#5818
RaresKeY wants to merge 18 commits into
odysseus-dev:devfrom
RaresKeY:fix/agent-linux-sandbox

Conversation

@RaresKeY

@RaresKeY RaresKeY commented Jul 28, 2026

Copy link
Copy Markdown
Member

Summary

Routes foreground Bash, Python, tmux sessions, and detached background jobs through one Linux Bubblewrap boundary. Sandbox remains the default process-execution mode: Odysseus probes the real process boundary under the serving user. A capability failure blocks only Bash, Python, tmux, and detached jobs; there is no automatic downgrade. An admin may transiently enable Full Access only after the warning and exact typed confirmation ENABLE FULL ACCESS. Full Access expands filesystem authority to the service-user view but retains the private PID/network boundary. Internet remains networkless by default and brokered public HTTP(S) when enabled; raw container networking is never exposed.

The boundary fails closed when it cannot be established, when the selected workspace cannot be safely constrained, when the trusted launcher or broker configuration is invalid, or when Sandbox mode is unavailable on the current platform. The current per-process limits are: 4 GiB virtual address space, 1 hour CPU time, 4 GiB file-size maximum, and 1,024 file descriptors; there is no hard PID/thread ceiling in this slice. Aggregate agent and disk quotas are deferred. This PR adds only the process-execution Sandbox/Full Access mode. It still does not change the automatic action-approval default or implement the broader Ask/exact-approval authority model owned by #6084 and #5819.

Stack

This cumulative process-boundary PR is superseded by the focused replacement chain below. It remains available as source context for the split; its discussion and commit history remain available, and no branch rewrite or new implementation is planned here.

Superseded map

The cumulative branch is replaced by four independently owned draft PRs in this order:

The process-sandbox tracker is #6091. The authority follow-up remains PR #5819 for Issue #6092; downstream PRs are #6084, #5821, #6100, and #6104. #6119 and #6118 are independently reviewable substrates; #6120 consumes both; #6121 integrates all three for shipped container posture.

PR #6121's standalone image/Trivy check is expected to remain blocked against dev until #6119 lands. Real Ubuntu Docker/Compose/AppArmor host validation remains required and is not claimed by the replacement chain yet.

Target branch

  • This PR targets dev, not main. All PRs land in dev; main is curated by the maintainer at each release. If your PR is on main by accident, click "Edit" on this PR and change the base.

Linked Issue

Part of #6091

Part of #5815

Superseded by PR #6119, PR #6118, PR #6120, and PR #6121.

Part of #4754

Related: #6083, #6084, #3709, and #6021

Follow-up: #6099, #6100, and #6104

Follow-up: Linear SMA-76 for aggregate agent CPU/RAM/PID ceilings and workspace-disk quotas.

Type of Change

  • Bug fix (non-breaking — fixes a confirmed issue)
  • New feature (non-breaking — adds new behaviour)
  • Breaking change (changes or removes existing behaviour)
  • Refactor / cleanup (behaviour unchanged)
  • Documentation only
  • CI / tooling / configuration

Checklist

How to Test

  1. Run python -m pytest -q tests/test_process_execution_mode.py tests/test_process_execution_routes.py tests/test_execution_sandbox.py tests/test_process_sandbox_dispatch.py tests/test_bg_job_launch_cleanup.py tests/test_sandbox_network_policy.py tests/test_egress_broker.py tests/test_agent_bash_windows.py tests/test_foreground_model_routing.py tests/test_workspace_confine.py tests/test_seccomp_generator.py tests/test_seccomp_launcher.py tests/test_seccomp_policy.py; the focused process-mode and sandbox tests cover the process authority, dispatch, network, broker, and workspace-boundary paths.
  2. Run the adjacent execution/background/platform suite and the repository suite, then run python -m compileall -q core routes src, the seccomp generator/build checks, and git diff --check origin/dev...HEAD. Current CI evidence for head b3994bd396b8eb35fa66df252006ad0a6f66ba19: full pytest 5903 passed, 49 skipped, 9 warnings; Python compileall: passed; JavaScript syntax: passed; CI, CodeQL, dependency review, workflow security, secret scan, and both container scans: passed.
  3. Build and run the Docker image with the shipped outer seccomp profile. Select a narrow workspace and verify Bash, Python, tmux, and detached jobs can modify ordinary workspace files but cannot read inherited secrets, application data stores, credential directories, protected home paths, configured database files, writable .git metadata, or paths outside the workspace.
  4. With Internet disabled, verify the sandbox has no external route. With the existing web toggle enabled, verify public HTTP on port 80 and HTTPS CONNECT on port 443 work through the broker while loopback, LAN, private addresses, arbitrary ports, UDP, SSH, proxy bypass variables, and raw container networking remain unavailable.
  5. Verify missing Bubblewrap, an unsupported Sandbox platform, a broad or sensitive workspace, nested mounts or special files, malformed paths, missing CA state, launcher/package drift, or an unavailable broker fails closed without starting the model-requested command.
  6. Manually verify the authority-mode flow: restart defaults to Sandbox; non-admins cannot change mode; Full Access rejects the wrong confirmation; exact ENABLE FULL ACCESS enables it; the red Full Access indicator remains visible; restart resets to Sandbox; capability failure never enables Full Access automatically.

Not run: final stock Ubuntu 24.04 plain-Compose runtime, native ARM64 execution, native macOS/Windows Sandbox mode, or end-to-end browser/UI validation. Independent latest-head security review remains appropriate for this security-sensitive boundary.

Visual / UI changes — REQUIRED if you touched anything that renders

This PR now changes Settings → Tools by adding the process-capability/status card, Sandbox/Full Access control, warning and typed-confirmation flow, and persistent red Full Access indicator.

  • Screenshot or short clip of the desktop change in the running app, attached below.
  • Mobile screenshot of the change in the running app, attached below.
  • Style match: follows the existing Settings → Tools visual language.
  • No new component patterns. Reuses existing component patterns.
  • I am not an LLM agent submitting a bulk PR. This is a focused, one-off, user-directed security slice rather than an automated or mass submission.

Screenshots / clips

Pending running-app desktop and mobile evidence.

@StressTestor

Copy link
Copy Markdown
Collaborator

re-reviewed at 28342d3 after the rebase. most of what i had is closed.

arm_tool_gate defaulting to True in untrusted_context_message is the right shape and it closes the coverage problem outright: all 24 call sites now arm the gate, with an explicit opt-out and a test pinning both directions. observe_messages at src/agent_loop.py:3822 covers late prompt construction, and switching the result check to is not ResultIntegrity.SYSTEM means workspace reads finally arm it, which was the gap that let a poisoned repo file stay untainted.

that last one also closes something bigger by accident. teacher takeover used to start a fresh ToolRunSecurityContext with no taint, and run_teacher_inline still doesn't forward external_untrusted_context_seen. it no longer has to: the folded tool result carries tool_gate_untrusted, survives the role != "system" filter into teacher_messages, and the constructor's fallback detector re-derives the taint. i confirmed the whole chain end to end.

worth knowing it holds by inference rather than propagation, though. anything that later rebuilds history without metadata, or hands the teacher reconstructed text, silently reopens it. the kwarg already exists and forwarding it is one line, so i'd do that as a belt.

the overlay contract

src/execution_sandbox.py is byte-identical to the head i reviewed before, so this one stands.

_workspace_overlays (:139-177) builds masks from os.walk at launch, so a mask only exists for a path that already exists. an initially-empty fake home had .ssh and .profile created after overlay generation with no corresponding mount argument.

the list is also incomplete, and interestingly this repo disagrees with itself about it. src/tool_execution.py protects .profile, .bash_logout, .cshrc and .tcshrc for file tools; src/execution_sandbox.py (:42-71) doesn't. both cover .bashrc, .bash_profile, .zshrc, .zprofile and .zshenv. .profile is the one that matters: the workspace is bound writable at :290-294, and a host login shell sources it outside the sandbox, so that's persistence. .config/gh, .pgpass and .cargo are missing too.

extending the list doesn't fix it, because the gap isn't membership. /home/alice passes vet_workspace (src/tool_execution.py:256) since _BROAD_WORKSPACE_ROOTS has /home and not /home/<user>, and a root that broad can't be defended by an existence-time denylist. the fix i'd want is the contract: reject home-like roots after canonicalization, mount the project read-only, give a dedicated writable output subtree.

two test oracles

test_sandbox_rejects_broad_workspace (:79) is genuinely fixed now. the _stable_bubblewrap_lookup fixture fakes the binary lookup, so it reaches _normalized_workspace instead of passing off the platform check. that was accidental, but it's the right outcome.

two are still green regardless of the property:

  • test_sandbox_network_namespace_has_no_external_route (:190) connects to 127.0.0.1:9 and treats any OSError as isolation. a closed port refuses with or without a namespace, so the assertion holds even if the sandbox wrapper is bypassed entirely. a host listener the sandbox can't reach would prove the real property.
  • test_sandbox_hides_odysseus_data_inside_broader_workspace (:85) is negative-only, so "the payload never ran" also passes. its sibling at :151 has an allowed.txt positive marker, which is the shape this one wants.

@requires_bubblewrap is a real improvement, since these now skip instead of passing vacuously off-Linux. the pytest job is still continue-on-error: true with no bubblewrap installed, so none of it gates merge as configured.

smaller

a file-backed DATABASE_URL relocated outside DATA_DIR but inside the selected workspace isn't in _odysseus_data_overlays. the default sits under DATA_DIR and is masked, so this is configuration-dependent.

detached jobs get per-process prlimits and no aggregate cap. three distinct #!bg calls in one session produced three launches, with agent_max_tool_calls defaulting to 0.

Dockerfile installs bubblewrap and util-linux but grants no capability, and the odysseus service in docker-compose.yml has no cap_add or security_opt while the entrypoint gosu-drops to non-root. on a default docker/colima profile here, a minimal unshare -Ur probe in python:3.12-slim was refused as both root and uid 1000, and succeeded only with --cap-add SYS_ADMIN or seccomp=unconfined. i never built the real image though, so whether the sandbox starts under the shipped runtime is untested. neither relaxation looks like the right fix.

reviewed on macOS, so the argv is read and the Linux runtime behaviour is unverified here. #6021 covers the scheduled-action path separately, since none of those files are in this diff.

@RaresKeY
RaresKeY force-pushed the fix/agent-linux-sandbox branch from 28342d3 to ee2e71b Compare August 16, 2026 23:55
@github-actions github-actions Bot added needs runtime validation Runtime validation not attested — tick the app-run box after running it, or state the gap and removed ready for review Description complete — ready for maintainer review labels Aug 16, 2026
@RaresKeY
RaresKeY force-pushed the fix/agent-linux-sandbox branch from ee2e71b to 527d01d Compare August 16, 2026 23:58
@RaresKeY
RaresKeY marked this pull request as ready for review August 17, 2026 11:17
@github-actions github-actions Bot added ready for review Description complete — ready for maintainer review and removed needs runtime validation Runtime validation not attested — tick the app-run box after running it, or state the gap labels Aug 17, 2026
@RaresKeY
RaresKeY force-pushed the fix/agent-linux-sandbox branch from 5494bee to f655450 Compare August 17, 2026 13:30
@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown

⚠️ PR description is complete; validation evidence is still outstanding

Changed-file classification: UI-sensitive.

Author-reported runtime / visual state

  • The author explicitly reports that app/runtime validation was not performed.
  • The screenshot/clip checkbox is not checked for this UI-sensitive change.
  • The Screenshots / clips section does not contain an actual attachment or link.

Checkboxes are author attestations. GitHub Actions results remain the execution evidence for CI; this check does not prove that a local command ran.


This comment updates automatically when the description or changed files change.

@github-actions github-actions Bot added needs runtime validation Runtime validation not attested — tick the app-run box after running it, or state the gap and removed ready for review Description complete — ready for maintainer review labels Aug 17, 2026
Route foreground Bash, Python, tmux sessions, and detached jobs through one positive-mount bubblewrap profile. Clear inherited environment and network access, protect credentials and repository metadata, hide Odysseus data roots, and apply bounded resources while preserving one writable workspace.

This comment was marked as resolved.

@StressTestor StressTestor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

re-reviewed at 87b93e6. the overlay list, process-home rejection, sqlite DATABASE_URL mask and the network-namespace oracle close my 8/13 items.

residual: masks are built at launch, so a .bash_login that doesn't exist yet has none. reproduced: .bashrc present gets EACCES and stays intact on the host; .profile and .bash_login written in the same run land on it. needs the workspace to be another login home, so follow-up.

shipped runtime (your item 3)

slim image off the same python:3.14-slim base, PR apt set, make -C security/{seccomp,egress} install, run on Ubuntu 24.04 (6.8) / docker 28 with compose's security_opt: seccomp=..., as the dropped uid. bwrap never starts. three layers:

  • Failed to make / slave: Permission denied. docker-default AppArmor denies mount. the seccomp profile works; docker's stock one fails earlier at namespace creation.
  • apparmor=unconfined: loopback: Failed RTM_NEWADDR: Operation not permitted. natively on the same VM, unprivileged bwrap works at kernel.apparmor_restrict_unprivileged_userns=0 and fails exactly so at Ubuntu's default 1.
  • root + CAP_SYS_ADMIN + apparmor=unconfined: Can't mount proc on /newroot/proc: Operation not permitted. docker masks /proc paths; procfs won't mount in a userns unless the parent's is fully visible. dropping --proc /proc (execution_sandbox.py:533) fixes that run, systempaths=unconfined is the other exit. THREAT_MODEL.md says the sandbox omits /proc; the code mounts it. the docker constraint favors the doc.

so every bash/python/tmux/bg call is BLOCKED on this host class. fail-closed holds, nothing gets weaker by merging, but compose ships a sandbox that can't start on an Ubuntu 24.04 docker host, and the one-line workaround is privileged: true.

focused suite: 100 passed / 4 failed / 29 errors under the shipped posture; 130 / 3 under root + SYS_ADMIN + apparmor=unconfined + systempaths=unconfined, the only posture short of --privileged that started (the 3: no tmux in my slim image, plus the two test items below). no unprivileged uid started inside docker here at all, so that half wants a bare Ubuntu docker host. CI installs no bubblewrap, hence your 29 skips.

before landing i'd want an in-repo AppArmor profile (userns, mount, pivot_root, per Ubuntu's bwrap-userns-restrict) wired into compose, the /proc call made with doc and code agreeing, the sysctl documented, and a boot-time bwrap ... /bin/true self-test that logs loudly.

small

  • odysseus_egress_bridge.py:190-191 appends the worker before start(); test_loopback_bridge_releases_a_slot_when_the_broker_closes_first joins it unstarted (cannot join thread before it is started). 4/4 full-file runs on 2 cpus, passes alone.
  • test_sandbox_network_namespace_has_no_external_route hardcodes /usr/bin/python3; the base image only has /usr/local/bin/python3, so it fails rather than skips.
  • detached jobs: per-process prlimits, no aggregate cap.

code's fine by me. i'd hold the merge until the compose posture starts the sandbox somewhere that isn't --privileged.

@RaresKeY

Copy link
Copy Markdown
Member Author

Caveat: I still need to test via docker, previously tested with podman, which has a default AppArmor profile
This is still WIP, some things remain unadressed as of a67cc16

@github-actions github-actions Bot added the needs visual evidence UI-sensitive change without an attested screenshot or clip from the running app label Aug 19, 2026
@RaresKeY

Copy link
Copy Markdown
Member Author

The 11k-line cumulative branch has now been split into four independently owned draft PRs: #6119 (seccomp substrate), #6118 (egress), #6120 (process core), and #6121 (container/runtime integration). The updated body records the dependency order and the remaining real-host validation gap; discussion and commit history remain available here for context. This PR is being closed as superseded by those public replacements.

@RaresKeY RaresKeY closed this Aug 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs runtime validation Runtime validation not attested — tick the app-run box after running it, or state the gap needs visual evidence UI-sensitive change without an attested screenshot or clip from the running app

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants