Skip to content

fix(sandbox): extract common process execution boundary - #6120

Open
RaresKeY wants to merge 1 commit into
odysseus-dev:devfrom
RaresKeY:fix/agent-process-sandbox-core
Open

fix(sandbox): extract common process execution boundary#6120
RaresKeY wants to merge 1 commit into
odysseus-dev:devfrom
RaresKeY:fix/agent-process-sandbox-core

Conversation

@RaresKeY

@RaresKeY RaresKeY commented Aug 19, 2026

Copy link
Copy Markdown
Member

Summary

This draft extracts the common process-execution boundary from #5818 into a reviewable core slice. It routes foreground Bash/Python, interactive tmux, and detached jobs through one server-owned sandbox boundary, protects SQLite/data/credential paths, propagates explicit network profiles, and keeps Sandbox as the default fail-closed mode. The branch deliberately leaves the low-level seccomp and egress implementations, container-host integration, and user-facing authority selection/API/UI to their linked slices.

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

Fixes #6116
Part of #6091
Part of #5815
Depends on PR #6119 and PR #6118 for the low-level substrates.
Requires PR #6121 for shipped Docker/runtime integration.
Supersedes the corresponding core/wiring slice of #5818.

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

  • I searched open issues and open PRs — this is not a duplicate.
  • This PR targets dev
  • My changes are limited to the scope described above — no unrelated refactors or whitespace changes mixed in.
  • I actually ran the app (docker compose up or uvicorn app:app) and verified the change works end-to-end. Type-checks and unit tests are not enough.
  • I did not run the app/runtime validation and stated that gap in How to Test. Leave this unchecked when the app-run box above is checked.

How to Test

All commands below ran in a network-isolated, credential-free validation runner.

  1. Compile the changed Python and focused test modules with python3 -m py_compile src/execution_sandbox.py src/process_execution.py src/sqlite_paths.py src/agent_tools/subprocess_tools.py src/bg_jobs.py src/bg_monitor.py src/tool_execution.py src/agent_loop.py src/teacher_escalation.py routes/chat_routes.py core/database.py tests/test_agent_bash_windows.py tests/test_app_db_permissions.py tests/test_bg_job_launch_cleanup.py tests/test_execution_sandbox.py tests/test_foreground_model_routing.py tests/test_process_execution_mode.py tests/test_process_sandbox_dispatch.py tests/test_sandbox_network_policy.py tests/test_workspace_confine.py.
  2. Run python3 -m pytest -q tests/test_execution_sandbox.py tests/test_process_execution_mode.py tests/test_process_sandbox_dispatch.py tests/test_sandbox_network_policy.py tests/test_bg_job_launch_cleanup.py tests/test_app_db_permissions.py tests/test_agent_bash_windows.py tests/test_foreground_model_routing.py tests/test_workspace_confine.py.
  3. Review the fail-closed cases for unavailable Sandbox capability, explicit network-profile propagation, protected workspace/database paths, tmux policy rotation, and failed detached-launch cleanup.

Result: 219 tests passed and 31 were skipped, with one existing SQLAlchemy deprecation warning. The application, Docker/Compose, AppArmor, host-level Bubblewrap boot, seccomp-launcher, and egress-broker runtime checks were not run; those checks are intentionally deferred to #6117, #6119, and #6118 and are not claimed here.

This slice retains the seven post-87b93e65 hardening fixes from #5818: native process-tool routing, rejection of workspaces containing live SQLite databases, tmux rotation when policy changes, failed sandbox-launch artifact cleanup, the corrected procfs boundary documentation, capability-gated process modes, and aligned process failure contracts.

The reduced boundary is intentional: this PR keeps the internal capability/profile contract required by process callers and later authority work, while Sandbox remains the default and any unavailable process boundary blocks execution. User-facing authority selection, exact approvals, and authority API/UI remain owned by #6092 and #5819.

At the time of drafting, #6119 and #6118 are open draft dependency PRs and PR #6121 is the open container-host integration draft; this PR does not assume any of them is merged. Shipped Docker/runtime readiness is owned by PR #6121 and is not claimed by this core slice.

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

Anything that changes what the UI looks like — buttons, icons, padding, colors, fonts, spacing, layout, CSS, HTML, SVG, or any static/js/ module that draws to the DOM — needs all of the following. PRs that change rendering without these WILL be closed.

  • Screenshot or short clip of the change in the running app, attached below. Mobile screenshot too if the change affects mobile.
  • Style match: the change uses Odysseus's existing visual language. Specifically:
    • Reuse existing CSS variables (--red, --fg, --bg, --card, --border, etc.) — do not introduce new color values, font sizes, or spacing units.
    • Reuse existing button/input/card/border classes. Don't invent parallel styling.
    • No Unicode emoji in UI or code. Use inline SVG (matching the monochrome icon style already in static/index.html) or plain text.
    • Monospaced font (Fira Code) for primary UI text. Don't override.
    • Dark theme is the default; any light-mode work must be wired through the existing theme system, not hard-coded.
  • No new component patterns. If a similar widget already exists in the app, extend it instead of writing a parallel one.
  • I am not an LLM agent submitting a bulk PR. If you are, please open an issue describing the problem first — bulk auto-generated PRs that don't match the project's visual style are closed on sight, even when the underlying code is correct.

Screenshots / clips

Not applicable: this PR changes no UI-sensitive files.

@github-actions

Copy link
Copy Markdown

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

Changed-file classification: backend/runtime.

Author-reported runtime / visual state

  • The author explicitly reports that app/runtime validation was not performed.

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.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Route agent Bash, Python, tmux, and background jobs through the common sandbox core

1 participant