Skip to content

fix(sandbox): E2B fails fast on RALPH_DIR at the workspace root or outside it (#383) - #387

Merged
frankbria merged 2 commits into
mainfrom
fix/383-e2b-ralph-dir-shapes
Oct 10, 2026
Merged

frankbria merged 2 commits into
mainfrom
fix/383-e2b-ralph-dir-shapes

Conversation

@frankbria

Copy link
Copy Markdown
Owner

Summary

ralph --sandbox e2b now fails fast when RALPH_DIR is the workspace root or outside the workspace, instead of uploading an empty or state-leaking workspace.

validate_e2b_sandbox_config (first step of init_e2b_sandbox, before any sandbox is created) derives the control dir via _e2b_control_dir and rejects:

  • workspace root — ., ./, a/.., $PWD, $PWD/, the symlink-resolved path → RALPH_DIR '…' is the workspace root
  • outside the workspace — absolute, .., ../x, sub/../../x → RALPH_DIR '…' is outside the workspace

_e2b_control_dir itself is unchanged.

Acceptance Criteria

  • Workspace-root RALPH_DIR fails fast with a clear message
  • Outside-workspace RALPH_DIR fails fast for E2B (the issue allowed fail-fast or a fixed in-sandbox mapping; this takes fail-fast)
  • Tests for both shapes in tests/unit/test_sandbox_e2b.bats

Test Plan

  • 4 new unit tests: root spellings rejected, outside spellings rejected, in-workspace spellings (.ralph, nested, absolute-inside) still accepted, init_e2b_sandbox fails before any helper call
  • npm test: 1368 passed, 0 failed (was 1364)
  • Mutation check: removing the root check, removing the outside check, and narrowing the root check to the empty-string case each fail a new test
  • Real entry point: RALPH_DIR=. | $PWD | <outside dir> ralph --sandbox e2b exits 1 with the new message before the helper runs; RALPH_DIR=.ralph passes the check

Review

Pre-PR cross-family review: codex review --base main — no findings. opencode/GLM (the primary reviewer) stalled twice with no output, so codex was used as the fallback.

Known Limitations / Intentionally Deferred

  • An outside-workspace RALPH_DIR is rejected for E2B rather than mapped to a fixed in-sandbox path. E2B with external control state is unsupported until someone needs it.
  • The check is lexical, like the rest of the control-dir handling: an in-workspace RALPH_DIR that is a symlink to an outside directory is not detected.
  • With / as the working directory, an in-workspace RALPH_DIR such as /.ralph is reported as outside (_e2b_control_dir does not strip a bare / base).

Implementation Notes

The root check lives in the validator, not in _e2b_control_dir: an existing #376 test relies on $PWD/ staying absolute there so the upload pathspec stays valid.

Closes #383

…tside it (#383)

validate_e2b_sandbox_config now rejects a RALPH_DIR that is the workspace
root (., a/.., $PWD, the symlink-resolved path) or outside the workspace
(absolute, ../x) before any sandbox is created. At the root the control-dir
guards cannot tell control state from project files and the upload came
back empty or leaked state; outside it the control files got wrong tar
member paths.
@frankbria

Copy link
Copy Markdown
Owner Author

Cross-family review (codex, pre-PR — diff unchanged since review, HEAD 1ec7d13)

codex review --base main output, verbatim:

The change adds E2B RALPH_DIR validation before sandbox creation and includes targeted unit tests and docs. I did not identify any introduced correctness, security, or maintainability issues in the diff.

opencode/GLM is the primary reviewer; it stalled twice with no output on this diff, so codex was used as the fallback.

@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Claude finished @frankbria's task in 9m 15s —— View job


Precision bug-hunt review — complete

✅ GLM review: no defects found.

Severity File:line Finding
— — No confirmed defects in the diff
  • Fetch full diff (origin/main...HEAD) and PR context
  • Check existing comments for already-posted findings (codex review: none; no prior review comments)
  • Read lib/sandbox_e2b.sh — validate_e2b_sandbox_config, _e2b_control_dir, _e2b_collapse_path, init_e2b_sandbox and callers
  • Trace path-shape edge cases (root spellings, outside spellings, symlink/CWD interactions, quoting)
  • Read new tests in tests/unit/test_sandbox_e2b.bats
  • Post summary (no inline findings — nothing met the defect bar)

What was verified

Guard logic (lib/sandbox_e2b.sh:186-195)

  • Root detection is sound: relative root spellings (., ./, a/..) collapse to empty and hit the -z branch; absolute root spellings ($PWD, $PWD/, $(pwd -P)) are deliberately not relativized by _e2b_control_dir (the "$base"/* prefix glob requires the trailing slash, so an exact-equal path stays absolute) and are then caught by the equality comparisons against collapsed $PWD and pwd -P — both bases mirror exactly the two bases _e2b_control_dir tries, so a symlinked CWD is handled consistently (logical and physical spellings both accepted for in-workspace dirs, both rejected when equal to the root).
  • Outside detection (/*, .., ../*) covers absolutized and lexical-escape shapes; ..hidden correctly does not match (literal dirname, not traversal). _e2b_collapse_path's unset/append array pattern stays dense (unset always removes the tail index), so sub/../../out → ../out collapses correctly.
  • The -z branch cannot misfire in production: RALPH_DIR is defaulted to .ralph at source time in both ralph_loop.sh:58 and lib/sandbox_e2b.sh:40, so an actually-empty value never reaches the validator.
  • Ordering holds: validate_e2b_sandbox_config is the first statement of init_e2b_sandbox (lib/sandbox_e2b.sh:264), before the transport check, credentials, state-file writes, or any sandbox creation — the "fails before any helper call" claim and test are accurate. It is the only caller repo-wide; the ralph_loop.sh:2773 call site runs from the project CWD (the same base every sync guard already assumes) and failure is fatal with cleanup, no host fallback.
  • No injection/quoting hazards: $rb is quoted on the LHS of every comparison, the unquoted /*/../* RHS are the intended patterns, and $RALPH_DIR only reaches stderr messages. Bash 3.2-compatible.

Tests — the bats setup cd "$TEST_DIR" makes $TEST_DIR == $PWD, so the root-spelling and accepted-absolute assertions exercise exactly the production bases; assertions match the echo strings; exports don't leak across bats subshells.

Disclosed limitations confirmed as described (not counted as findings — both are in the PR body):

  • CWD / + absolute in-workspace RALPH_DIR (e.g. /.ralph) is falsely rejected as outside: _e2b_collapse_path("/") = / and the ///* prefix pattern never matches single-slash paths. Fail-closed, requires running the loop from the filesystem root.
  • The check is lexical; an in-workspace RALPH_DIR that is a symlink to an outside directory is not detected.

One degenerate pre-existing (not in this diff) note: if the CWD were deleted mid-run, pwd -P yields empty and _e2b_control_dir's base loop would strip only the leading slash from an absolute RALPH_DIR — unreachable in a functioning environment and unchanged by this PR.

No files were modified and nothing was pushed.

@frankbria

Copy link
Copy Markdown
Owner Author

Demo verification (Showboat, real ralph_loop.sh --sandbox e2b)

The E2B interpreter was replaced by a mock that records every helper call, so "helper calls: none" shows ralph stopped before contacting E2B.

Criterion Action Outcome evidence Status
Workspace-root RALPH_DIR fails fast with a clear message RALPH_DIR=., sub/.., <project abs path> with ralph --sandbox e2b Each: Error: RALPH_DIR '…' is the workspace root — --sandbox e2b needs it in a subdirectory (e.g. .ralph), exit 1, E2B helper calls: none VERIFIED
Outside-workspace RALPH_DIR fails fast for E2B RALPH_DIR=../outside-ralph and its absolute path (valid control files there) Each: Error: RALPH_DIR '…' is outside the workspace — --sandbox e2b needs it inside the project (e.g. .ralph), exit 1, E2B helper calls: none VERIFIED
Tests for both shapes in tests/unit/test_sandbox_e2b.bats bats tests/unit/test_sandbox_e2b.bats -f 383 4 of 4 pass: root rejected, outside rejected, in-workspace accepted, init_e2b_sandbox fails before any helper call VERIFIED
Control: default .ralph unaffected RALPH_DIR=.ralph ralph --sandbox e2b No RALPH_DIR error; ralph proceeds to the helper (E2B helper calls: check), where the mock fails by design VERIFIED

The demo also shows the hazard being closed: with RALPH_DIR=. the upload list for a project containing app.py shrinks from 9 entries to just .ralphrc.

Full Showboat transcript

Issue #383: E2B fails fast on RALPH_DIR at the workspace root or outside it

2026-10-10T06:40:54Z by Showboat 0.6.1

Setup: a throwaway git project with control files in three places (project root, .ralph/, and a sibling outside-ralph/). run.sh <RALPH_DIR> runs the real ralph_loop.sh --sandbox e2b from PR #387 in that project. The E2B python interpreter is a mock that records every helper call and then fails, so "E2B helper calls: none" proves ralph stopped before contacting E2B.

cat <tmp>/run.sh | sed -n '8,19p'
T=<tmp>
cd "$T/demo/proj" || exit 2
rm -f "$T/helper_calls.log"
E2B_API_KEY=demo-key SANDBOX_E2B_PYTHON="$T/mock-python" RALPH_DIR="$1" \
    timeout 30 bash "$R/ralph_loop.sh" --sandbox e2b 2>&1 \
    | sed 's/\x1b\[[0-9;]*m//g' | grep -E "RALPH_DIR|E2B" | sed "s|$T|<tmp>|g"
echo "ralph exit code: ${PIPESTATUS[0]}"
if [[ -f "$T/helper_calls.log" ]]; then
    echo "E2B helper calls: $(tr '\n' ' ' < "$T/helper_calls.log")"
else
    echo "E2B helper calls: none"
fi

The hazard (the upload-list builder is the same on main and on this branch — the PR only adds the validator). With the default RALPH_DIR=.ralph, the project file app.py and the control files are listed for upload. With RALPH_DIR=. the whole project listing disappears: app.py, the prompt and the plan are all gone and only .ralphrc is left. Before this PR nothing stopped ralph from uploading that.

<tmp>/uploadlist.sh .ralph; echo; <tmp>/uploadlist.sh .
.ralph/AGENT.md
.ralph/PROMPT.md
.ralph/fix_plan.md
.ralphrc
.ralphrc
AGENT.md
PROMPT.md
app.py
fix_plan.md
(9 entries for RALPH_DIR=.ralph)

.ralphrc
(1 entries for RALPH_DIR=.)

Criterion 1 — a workspace-root RALPH_DIR fails fast with a clear message. Three spellings of the root: ., sub/.. (the project has a sub/ directory) and the absolute project path. Each exits 1 with the "is the workspace root" error and makes no E2B helper call.

<tmp>/run.sh .; echo; <tmp>/run.sh sub/..; echo; <tmp>/run.sh <tmp>/demo/proj
[2026-10-09 23:41:36] [INFO] ☁️  E2B sandbox mode enabled (template: base, session timeout: 3600s)
Error: RALPH_DIR '.' is the workspace root — --sandbox e2b needs it in a subdirectory (e.g. .ralph)
[2026-10-09 23:41:36] [ERROR] E2B sandbox setup failed — refusing to fall back to host execution
ralph exit code: 1
E2B helper calls: none

[2026-10-09 23:41:36] [INFO] ☁️  E2B sandbox mode enabled (template: base, session timeout: 3600s)
Error: RALPH_DIR 'sub/..' is the workspace root — --sandbox e2b needs it in a subdirectory (e.g. .ralph)
[2026-10-09 23:41:36] [ERROR] E2B sandbox setup failed — refusing to fall back to host execution
ralph exit code: 1
E2B helper calls: none

[2026-10-09 23:41:36] [INFO] ☁️  E2B sandbox mode enabled (template: base, session timeout: 3600s)
Error: RALPH_DIR '<tmp>/demo/proj' is the workspace root — --sandbox e2b needs it in a subdirectory (e.g. .ralph)
[2026-10-09 23:41:36] [ERROR] E2B sandbox setup failed — refusing to fall back to host execution
ralph exit code: 1
E2B helper calls: none

Criterion 2 — an outside-workspace RALPH_DIR fails fast for E2B. A relative ../outside-ralph and the same directory as an absolute path (both hold valid control files, so ralph gets past its own integrity check). Each exits 1 with the "is outside the workspace" error and makes no E2B helper call.

<tmp>/run.sh ../outside-ralph; echo; <tmp>/run.sh <tmp>/demo/outside-ralph
[2026-10-09 23:41:36] [INFO] ☁️  E2B sandbox mode enabled (template: base, session timeout: 3600s)
Error: RALPH_DIR '../outside-ralph' is outside the workspace — --sandbox e2b needs it inside the project (e.g. .ralph)
[2026-10-09 23:41:36] [ERROR] E2B sandbox setup failed — refusing to fall back to host execution
ralph exit code: 1
E2B helper calls: none

[2026-10-09 23:41:36] [INFO] ☁️  E2B sandbox mode enabled (template: base, session timeout: 3600s)
Error: RALPH_DIR '<tmp>/demo/outside-ralph' is outside the workspace — --sandbox e2b needs it inside the project (e.g. .ralph)
[2026-10-09 23:41:36] [ERROR] E2B sandbox setup failed — refusing to fall back to host execution
ralph exit code: 1
E2B helper calls: none

Control — the default RALPH_DIR=.ralph is not affected: validation passes and ralph goes on to call the E2B helper (check), which is where the mock interpreter deliberately fails.

<tmp>/run.sh .ralph
[2026-10-09 23:41:36] [INFO] ☁️  E2B sandbox mode enabled (template: base, session timeout: 3600s)
[2026-10-09 23:41:36] [ERROR] E2B SDK unavailable: helper check failed. Install it with: pip install e2b
[2026-10-09 23:41:36] [ERROR] E2B sandbox setup failed — refusing to fall back to host execution
ralph exit code: 1
E2B helper calls: check 

Criterion 3 — tests for both shapes live in tests/unit/test_sandbox_e2b.bats.

cd /home/frankbria/projects/ralph-claude-code && npx bats tests/unit/test_sandbox_e2b.bats -f 383 2>&1 | grep -E '^(1\.\.|ok|not ok)'
1..4
ok 1 validate_e2b_sandbox_config: rejects RALPH_DIR at the workspace root (#383)
ok 2 validate_e2b_sandbox_config: rejects RALPH_DIR outside the workspace (#383)
ok 3 validate_e2b_sandbox_config: accepts in-workspace RALPH_DIR spellings (#383)
ok 4 init_e2b_sandbox: workspace-root RALPH_DIR fails before any helper call (#383)

@github-actions

Copy link
Copy Markdown

Review — PR 387: E2B fail-fast on bad RALPH_DIR

The approach is correct: adding the guard to validate_e2b_sandbox_config (the first gate of init_e2b_sandbox) means no sandbox is ever created for an invalid config, which is exactly the right place. The three-part condition, docs update, and 4-test coverage are all solid. Below are the observations worth addressing or noting for future readers.


Logic: why the explicit PWD comparisons are load-bearing

The root check is:

rb=$(_e2b_control_dir)
if [[ -z "$rb" || "$rb" == "$(_e2b_collapse_path "$PWD")" || "$rb" == "$(_e2b_collapse_path "$(pwd -P)")" ]]; then

The [[ -z "$rb" ]] branch correctly catches relative paths that collapse to empty (., ./, a/..). The two explicit PWD comparisons are not redundant — they handle the case where RALPH_DIR is an absolute path that equals $PWD. In _e2b_control_dir, the stripping loop uses "$base"/*, which only matches when rb has a subdirectory component after the base. When RALPH_DIR == $PWD (absolute, exact match), the loop condition is false and rb is returned unchanged as the full absolute path. The == $(_e2b_collapse_path "$PWD") comparison then catches it.

The existing comment explains the why (what breaks if RALPH_DIR is at the root), but a brief note that the explicit comparisons handle the absolute-exact-match case that _e2b_control_dir doesn't collapse would help a future reader who sees the empty check and wonders if the rest is redundant. Something like:

# _e2b_control_dir only strips an absolute RALPH_DIR when it has a subdir
# suffix; an exact match ($RALPH_DIR == $PWD) comes back as the full path,
# so we compare explicitly.

Not a blocker, but would prevent confusion during a future refactor.


Test diagnostics: acceptance loop has no per-iteration failure message

The rejection loops correctly identify which value triggered the failure:

[[ "$output" == *"workspace root"* ]] || fail "no root message for RALPH_DIR=$d: $output"

The acceptance loop does not:

for d in .ralph state/ralph "$TEST_DIR/state/ralph/"; do
    export RALPH_DIR="$d"
    run validate_e2b_sandbox_config
    assert_success     # ← if this fails, the output doesn't say which $d
done

Since assert_success just checks $status and prints bats's generic output, a failure here won't show which path caused it. Suggest adding || fail "rejected valid RALPH_DIR=$d: $output" after assert_success (or restructuring to [[ $status -eq 0 ]] || fail ...), consistent with the rejection tests.


Trailing note on init_e2b_sandbox guard test

@test "init_e2b_sandbox: workspace-root RALPH_DIR fails before any helper call (#383)" {
    _mock_e2b ok
    export RALPH_DIR="."
    run init_e2b_sandbox
    assert_failure
    [[ "$output" == *"workspace root"* ]]       # ← bare [[ ]] with no || fail
    [[ \! -f "$TEST_DIR/e2b_args" ]] || fail "helper was called: $(cat "$TEST_DIR/e2b_args")"
}

The bare [[ "$output" == *"workspace root"* ]] silently passes even if the condition is false (bats only fails on non-zero exit or explicit fail). This is a pre-existing pattern in the file, so acceptable to leave as-is, but worth being aware of if this test ever silently regresses. Adding || fail "expected workspace root message: $output" would make it airtight.


No security or correctness issues found

  • $RALPH_DIR appears in error messages quoted but not executed — safe.
  • The outside check (/* || ".." || ../*) correctly catches absolute paths, bare .., and multi-segment escape paths like sub/../../out (which _e2b_collapse_path normalizes to ../out before the comparison).
  • The known limitations (symlinked RALPH_DIR, $PWD=/ edge case) are documented in the PR description.

Overall: the implementation is correct and the test suite proves the three distinct cases (root, outside, in-workspace including absolute-inside spelling). The two items above are minor polish, not blockers. Good to merge once the test diagnostic issue is considered.

…; note why the $PWD comparisons are needed
@frankbria

Copy link
Copy Markdown
Owner Author

Review triage (after the first push)

Source Finding Class Decision
GLM review / review No defects; confirmed the two Known Limitations (cwd /, symlinked RALPH_DIR) — Nothing to do
claude-review Explain why the explicit $PWD / pwd -P comparisons aren't redundant with the empty check Suggestion Applied — comment added in validate_e2b_sandbox_config
claude-review Acceptance loop doesn't say which RALPH_DIR failed Suggestion Applied — || fail "rejected valid RALPH_DIR=$d: …"
claude-review Bare [[ "$output" == *"workspace root"* ]] "silently passes even if false" Inaccurate on bash ≥ 4.1 (a false bare [[ ]] fails a bats test here — checked on bash 5.3); real only on bash 3.2 Applied anyway for the clearer message — || fail added

No Critical or Major findings. The follow-up commit changes one code comment and two test assertions only; e2b unit file still 104/104.

@github-actions

github-actions Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Claude finished @frankbria's task in 4m 20s —— View job


Precision bug-hunt review — complete (follow-up commit becae89)

✅ GLM review: no defects found.

Severity File:line Finding
— — No confirmed defects in the diff
  • Fetch full diff (origin/main...HEAD) and the follow-up commit's changes
  • Check existing comments for already-posted findings (prior GLM pass: clean; claude-review suggestions applied in becae89)
  • Read lib/sandbox_e2b.sh — validator, _e2b_control_dir, _e2b_collapse_path, init_e2b_sandbox and callers
  • Verify the new test assertions and comment are correct (no logic regressions)
  • Post summary

Delta since the last review

becae89 touches exactly two things, neither behavioral:

  1. Comment in validate_e2b_sandbox_config (lib/sandbox_e2b.sh:186-188) — verified accurate against the code: _e2b_control_dir's stripping loop uses "$base"/*, which requires a subdirectory suffix, so an absolute RALPH_DIR exactly equal to the workspace survives as a full absolute path and is caught only by the two explicit $PWD/pwd -P comparisons. The comment states this correctly.
  2. Two test assertions (tests/unit/test_sandbox_e2b.bats:219, 228) — assert_success → [[ "$status" -eq 0 ]] || fail "rejected valid RALPH_DIR=$d: …" and the bare pattern check → … || fail "expected workspace root message: …". Both are the correct bats idiom ($status is always numeric from run; fail interpolates $d/$output for per-case diagnostics). No assertion semantics changed — the same conditions fail on the same inputs.

Re-verification of the guard (unchanged code, re-checked)

  • Root check ordering is load-bearing and correct: the root branch runs before the outside branch, so an absolute RALPH_DIR == $PWD gets the accurate "workspace root" message rather than a misleading "outside"; an absolute path genuinely outside matches /* only after failing both base-equality comparisons. Both comparisons mirror exactly the two bases _e2b_control_dir itself tries ($PWD, pwd -P, both collapsed), so a symlinked CWD is handled consistently.
  • Outside patterns don't over-match: ..hidden (a literal dirname) correctly fails none of /*, .., ../* — ../* requires a / after ... Escape spellings (.., ../out, sub/../../out) all collapse via _e2b_collapse_path (dense-array unset-of-tail verified) into a form one of the three patterns catches.
  • Placement: validate_e2b_sandbox_config remains the first statement of init_e2b_sandbox, ahead of the transport probe, credentials, state writes, and sandbox creation; ralph_loop.sh:2773 is its only production caller, failure is fatal with cleanup and no host fallback.
  • No injection/quoting hazards: $rb is the quoted subject (never the pattern) in every [[ == ]]; the unquoted /*/../* are the intended patterns; $RALPH_DIR reaches only stderr messages.
  • Tests: the bats setup cd "$TEST_DIR" makes $PWD == $TEST_DIR, so the root-spelling assertions exercise the real production bases; exports don't leak across bats test subshells; the accept-list includes the absolute-with-trailing-slash spelling ($TEST_DIR/state/ralph/) that collapses and strips to state/ralph.

Previously disclosed limitations (cwd / false rejection, symlinked RALPH_DIR) are unchanged and remain documented in the PR body — not counted as findings.

No inline findings — nothing met the defect bar.

@github-actions

Copy link
Copy Markdown

Follow-up review (after the polish commit)

All three suggestions from the prior review have been applied:

1. Explanatory comment in validate_e2b_sandbox_config — lib/sandbox_e2b.sh:186-188

The added comment correctly explains both why the guard lives in the validator (not in _e2b_control_dir) and why the two explicit $PWD / pwd -P comparisons are not redundant with the -z check. A future refactor reader will immediately understand the invariant.

2. Acceptance loop diagnostic — test file, acceptance test

[[ "$status" -eq 0 ]] || fail "rejected valid RALPH_DIR=$d: $output"

Now consistent with the rejection loops — a failure will identify which path triggered it.

3. init_e2b_sandbox guard test — test file, No. 4 test

[[ "$output" == *"workspace root"* ]] || fail "expected workspace root message: $output"

Message check is now airtight: a false [[ ]] produces an explicit failure rather than silently passing.


No new issues found. The implementation is correct, the comment explains the non-obvious invariant, and the test suite covers root spellings, outside spellings, in-workspace spellings, and the ordering guarantee (validator fires before any helper call). Good to merge.

@frankbria
frankbria merged commit 4fd2fe9 into main Oct 10, 2026
5 checks passed
@frankbria
frankbria deleted the fix/383-e2b-ralph-dir-shapes branch October 10, 2026 07:13
@frankbria

Copy link
Copy Markdown
Owner Author

Post-merge disposition:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[P2.7] E2B sandbox: handle RALPH_DIR at the workspace root or outside the workspace

1 participant