Files
Abderrahmen Gharsallah 59aa7613c7 fix: use world-writable /tmp safely in modal sandbox and test guardrails (#647)
* fix: use a private mode-700 dir for the modal foreground pidfile

exec_foreground recorded the remote pid at a fixed, predictable path in the
world-writable /tmp (/tmp/oa-foreground.pid). A co-tenant process in the
sandbox could pre-seed that path as a symlink (so `echo $$ > ...` writes
through it) or overwrite its contents (so `kill $(cat ...)` signals an
arbitrary pid).

Record the pid in a private, unpredictably-named dir created with
`mkdir -m 700` (no -p, so it fails closed if the path already exists), and
only signal a numeric pid read back from that file before removing the dir.
Update the tests to assert the new structure instead of the fixed path.

Signed-off-by: agharsallah <17379925+agharsallah@users.noreply.github.com>

* fix: resolve symlinks before trusting a SQLite path as a test DB

looks_like_test_db accepted a file-backed path on its 'test' name token or its
temp-dir location without resolving symlinks first. A symlink planted in a
world-writable dir like /tmp (e.g. sqlite:////tmp/test.db) could therefore
point a 'throwaway' test DB at a real database and pass the guardrail.

Resolve the path before the token and temp-dir checks so the resolved target
is what gets classified, and add a regression test covering a test-named
symlink that resolves outside any temp root.

Signed-off-by: agharsallah <17379925+agharsallah@users.noreply.github.com>

* fix: share safe foreground-pidfile helper across sandbox launchers

Extract a single fail-closed foreground-pidfile implementation into
base.py (foreground_pidfile / foreground_record_prefix /
foreground_kill_command) and route Modal, CoreWeave (cwsandbox), and
OpenShell through it, closing the same /tmp symlink-redirect + pid-spoof
vector the Modal-only fix addressed in two other shipped providers.

- cwsandbox: drops the vulnerable fixed /tmp/oa-foreground.pid and
  unvalidated 'kill $(cat ...)' — now uses the private mode-700 dir
  with a numeric-gated kill. Adds exec_foreground regression tests
  (none existed before) and extends the cwsandbox fake to record exec
  commands and raise on wait.
- openshell: drops the predictable {sandbox_id} pidfile template and
  unvalidated kill for the shared, numeric-gated path.
- modal: drops its inline copy and imports the helper; behavior
  unchanged for the security properties.
- All three: clean up the run dir on normal exit too (previously only
  on Ctrl-C), so a successful run no longer orphans a mode-700 dir.
- Helper hardening: shlex.quote the derived run_dir/pidfile inside
  foreground_record_prefix and foreground_kill_command so the public
  API stays injection-safe even if a future caller passes a non-hex
  path. Hex paths quote harmlessly.

All 268 tests/onboarding/sandboxes tests pass; ruff check + format clean.

Signed-off-by: Zeyi (Rice) Fan <zeyi.f@databricks.com>

---------

Signed-off-by: agharsallah <17379925+agharsallah@users.noreply.github.com>
Signed-off-by: Zeyi (Rice) Fan <zeyi.f@databricks.com>
Co-authored-by: Zeyi (Rice) Fan <zeyi.f@databricks.com>
2026-07-16 19:14:12 -07:00
..