fix: exec/2 returns a shell result instead of raising, and always terminates

[davydog187]

Closes #69

JustBash.exec/2 owes a host two things when it runs untrusted script text: it
returns, and it returns a shell result rather than an Elixir exception. Neither
was contractual. Both are now, with tests as the net.

Updated after review. The first round of this PR kept both guarantees only
at the edges: containment sat in exec/2's rescue (which rolls the session
back) and inside the telemetry span (which retired a documented event), and
the wall clock was only checked between statements, on one entry point, and in
one traversal. Eight confirmed findings are addressed in commits 554d8d9
through 16c85b3; the description below is the current state.
Finding-by-finding resolutions.

1. A raise from a builtin escaped exec/2

Only custom commands had a catch-all (executor.ex:865). The ~90 registry
commands had none, and exec/2 had no rescue at all — only Parser.parse
errors were handled. The issue cites the cp MatchError from #52 and a date
FunctionClauseError found during #67; both are fixed, but the hole they came
through was not.

A live one still on main:

{r, _} = JustBash.exec(JustBash.new(), "head -n abc")
** (FunctionClauseError) no function clause matching in Enum.take/2

And, from the issue's own probe shape — a command that hands back a wrecked
JustBash struct, so the next registry command is the one that raises:

{r, _} = JustBash.exec(bash, "wreck; cat /etc/hosts")
** (FunctionClauseError) no function clause matching in VFS.impl_module/1

Fix. Containment happens at two levels, both chosen so that nothing already
done is lost:

• Executor.contain_command_crash/3 wraps command dispatch, giving registry
  commands the treatment host-supplied ones already had:
  bash: cat: command crashed (FunctionClauseError: ...), exit 1, script
  continues. It sits outside Telemetry.command_span/3 deliberately, so
  :telemetry.span/3 still sees the exception and the documented
  [:just_bash, :command, :exception] event keeps firing — containing it inside
  the span silently retired that event for every command in the registry.
  execute_custom_command/5's own rescue folded into the same place, so a
  host-supplied command that crashes is now visible in telemetry too; it never
  was.
• Executor.run_statement/2 wraps everything else the statement loop runs —
  expansion, redirection, control flow — reporting
  bash: internal error (...), halting the script, and keeping the output and
  the shell state of the statements that already ran. That is why it lives in
  the statement loop rather than in exec/2: a rescue on exec/2 is a clause of
  its implicit try and can only see the parameter, so it silently rolls the
  whole session back. This matches the neighbouring Limit.ExceededError
  handler, which already behaved that way.

exec/2 keeps an outer net as a last resort, for raises outside the statement
loop (parsing, the EXIT trap). Its stderr is truncated to
min(512, max_output_bytes) on a UTF-8 boundary: Exception.message/1 is
unbounded from the sandbox's point of view — inspect/1 alone allows 4096 bytes
per binary — so a MatchError carrying interpreter or filesystem state could
otherwise inline sandbox file contents into a stderr the host asked to be capped.

Every rescue carries a comment saying this is deliberate containment at a trust
boundary, not defensive coding: a raise reaching them is a bug in JustBash, but a
host driving the sandbox has no way to act on an exception naming a JustBash
internal, so it must still get a shell-shaped answer. The type-specific rescues
stay — Limit.ExceededError, Expansion.UnsetVariableError and ArithmeticError
are re-raised so their existing handlers keep producing the better message (and
so a limit breach keeps unwinding to halt the script).

Because containment now lives in the interpreter rather than in exec/2,
exec!/2 inherits it. What exec!/2 still propagates — a RuntimeError on a
parse error, and anything raised outside the statement loop — is now stated in
its @doc instead of the old "raising on parse errors".

Related: what composition actually loses

The issue reports the exit code being dropped through pipelines and command
substitution. Checked against the oracle first (GNU bash 3.2.57):

$ bash -c 'cat /nope | cat; echo "code=$?"'
cat: /nope: No such file or directory
code=0
$ bash -c 'echo $(cat /nope); echo "code=$?"'
cat: /nope: No such file or directory

code=0
$ bash -c 'x=$(cat /nope); echo "code=$?"'
cat: /nope: No such file or directory
code=1

A pipeline reports its last stage, and a command reports its own status — so
those exit codes were already right, and the tests assert them against bash
rather than "fixing" them. A bare assignment is the one form where bash does
propagate the substitution's status, and it is the last substitution that wins
(y=$(cat /nope) x=$(cat /nada) is code=1; x=$(cat /nope) y=$(echo ok) is
code=0). export, local and declare keep their own status.

What was lost, in both paths, is the diagnostic:

• execute_pipeline/2 now accumulates every stage's stderr as iodata (only
  stdout is piped onward). 2>/dev/null on a stage still silences that stage,
  and PIPESTATUS and pipefail are unchanged.
• execute_command_substitution/2 now returns a {:substitution, stderr, exit_code} trace alongside the assignments an expansion already hands back,
  so it travels out of arbitrarily nested expansions — "$(...)",
  ${x:-$(...)}, $(( $(...) + 1 )), backticks — without a second channel.
  Expansion.take_substitutions/1 splits the traces out at the two consumers: a
  simple command prepends the accumulated stderr to its result outside
  with_redirections/3 (bash performs redirections after expansion, so
  echo $(cat /nope) 2>/dev/null still prints the diagnostic), and a bare
  assignment reports the last substitution's status as $?.

Still out of scope: array literals (arr=($(cat /nope))) and for-loop word
lists (for i in $(cat /nope)) discard expansion side effects entirely and stay
silent. Both also drop ${VAR:=default} assignments, which predates this PR, so
that path wants its own change.

2. Limit had no wall-clock bound

JustBash.Limit.defaults() |> Map.keys()
#=> [:max_exec_depth, :max_file_bytes, :max_regex_pattern_bytes, :max_output_bytes, :max_steps]

Every bound counted work. Both historical hangs burned wall clock without
consuming a step — printf '%b' recycling its format (56c74b8), find
looping a symlink cycle (#53) — so nothing fired.

Fix. max_wall_ms, defaulting to 5_000 ms (1_000 strict, 30_000
relaxed). Documented in a bounds table in JustBash.Limit's moduledoc. 5s is
three orders of magnitude above what any script in this repo's corpus takes:
generous enough that a legitimate script never trips it, small enough that an
agent waiting on the sandbox is never blocked for long.

A Limit.Deadline struct (at_ms from System.monotonic_time/1, plus
max_wall_ms so the diagnostic can name the bound) is armed once per
top-level execution — by exec/2 and exec!/2, which share arm_top_level/1
— and carried in Interpreter.State. Limit.check_deadline!/1 is then one clock
read and an integer comparison: no timestamp per step, no allocation.

It is checked everywhere a script can burn time without doing countable work.
The statement loop is not enough on its own — a shell-level while true; do :; done was already bounded by max_steps and Loop's iteration cap, and the
shape that actually escaped every bound is "one command, many iterations":

• the interpreter statement loop (Executor.execute_statement/2);
• a loop inside a single command — awk's for, while and do-while
  (awk 'BEGIN{while(1){x=x+1}}' hung forever), and seq's range walk
  (seq 1 100000000), via Limit.enforce_deadline/2. sed has no label/branch
  support, printf's format recycling terminates, and the expansion paths are
  bounded by the statement loop;
• every recursive traversal — find, grep -r, du, tree, and cp -r
  (whose recursion lives in FS.cp/4, which gained a :deadline option). A
  whole traversal is charged as a single step, so on an 8000-file tree with a
  1 ms budget the unbounded ones ran 4.5-8.1 seconds at exit 0 with empty
  stderr. rm -rf is a bulk prune rather than a hand-rolled descent and is
  unaffected (3 ms on the same tree).

It raises Limit.ExceededError with kind: :wall_clock_limit, so the existing
rescue in execute_statement/2 reports it and halts, exactly like the other
bounds:

bash: execution wall clock limit exceeded (30 ms)

Nested eval/source run inside the top-level call's budget rather than
starting a fresh one.

Tests

test/sandbox_contract_test.exs, 62 tests. Every one was watched failing before
its fix. Probes are synthetic and stable rather than leaning on any one command's
arg parsing, so a sibling PR fixing head cannot silently retire them:

• Boom — the issue's custom-behaviour module, raises on execute/3.
• Wreck / WreckEnv — return a valid result plus a corrupted JustBash
  struct, so the raise happens inside a registry builtin, and inside
  expansion, respectively.
• Spin — Process.sleep(10) per call, for a deterministic wall-clock probe.

Covering: containment for custom, registry and non-dispatch raises, and that an
internal error keeps the prior statements' output and session state (with the
Limit.ExceededError handler pinned alongside as the reference behaviour);
that a contained crash does not halt the rest of the script; that a limit breach
keeps its own diagnostic; that head -n abc returns a shell result; pipeline
stderr for one, several and redirected stages, with PIPESTATUS and pipefail
unchanged; fifteen command-substitution forms checked against the oracle;
exec!/2's wall clock, containment and parse-error contract; max_wall_ms
defaults and presets; awk's three loop forms and seq bounded under a Task so
a regression fails rather than hangs; five recursive commands bounded on a
2000-file tree; that the deadline is rearmed per execution, asserted both
behaviourally and structurally; that limits: false leaves deadline == nil;
and that a contained crash still emits [:just_bash, :command, :exception].

The three wall-clock tests were each verified red against the mutation they are
named for — arming only when deadline == nil, and deadline(nil) returning a
default-budget deadline.

Gates

mix compile --warnings-as-errors --force

Compiling 168 files (.ex)
Generated just_bash app

mix format --check-formatted

(no output, exit 0)

mix credo --strict

  Refactoring opportunities

┃ [F] ↘ Avoid `apply/2` and `apply/3` when the number of arguments is known.
┃       test/support/banned_fixture_apply.ex:4:16 #(BannedCallTracer.Fixture.Apply.run)

Analysis took 1.7 seconds (0.09s to load, 1.6s running 68 checks on 230 files)
5149 mods/funs, found 1 refactoring opportunity.

The single finding is the intentional test fixture; identical on main.

mix test

Finished in 23.8 seconds (23.5s async, 0.2s sync)
2 doctests, 62 properties, 4814 tests, 0 failures (5 excluded)

mix dialyzer

ignore_warnings: .dialyzer_ignore.exs
Total errors: 13, Skipped: 13, Unnecessary Skips: 0
done in 0m3.1s
done (passed successfully)

Note for reviewers

JustBash.BannedCallTracer's grep heuristic flags = System on any source
line, so check_deadline!/1 reads the clock in expression position rather than
binding it. That is why the raise path calls System.monotonic_time/1 a second
time via elapsed_ms/2 instead of reusing a bound value; the hot path still
does exactly one read.

FS.walk/3's :deadline option from the first round is gone: it never had a
caller in lib/, and Limit.enforce_deadline/2 is the general mechanism, now
reached from seq. Commands whose loop is not already an enumerable call
check_deadline!/1 directly.

[davydog187]

Verdict: request changes. GitHub refuses a formal --request-changes review on one's own PR, so this is posted as a comment; finding 1 is a blocker and should gate the merge.

Two lenses were applied — a contract lens (does exec/2 actually keep the two guarantees the title claims, on every public entry point and every composition form?) and a test-integrity lens (would the new tests fail if the fix were removed?). 11 candidates went in; 8 survived verification on the PR branch (d0efbf2, checked out in a private worktree, mix run probes). The rest were merged as duplicates — see the closing block.

The core of the PR is right, and the containment for registry builtins is a real fix. The problems below are all about the edges of the two guarantees: which entry points get them, which loops the deadline can actually see, and what the new error path throws away on the way out.

---

1. "always terminates" is false: any loop inside a single command still hangs exec/2 forever

Severity: blocker — lib/just_bash/interpreter/executor.ex:84

Limit.check_deadline!/1 is consulted in exactly three places: the statement loop (executor.ex:84), Find.find_recursive/5 (find.ex:142), and FS.walk/3 (which has no production caller — finding 3). Nothing checks it inside a command's own loop, so max_wall_ms bounds the gap between statements, not execution.

awk is the live instance: awk/evaluator.ex:590/613/619 implement for, while and do-while with no iteration cap and no deadline check.

Measured on this branch with limits: :strict (max_wall_ms: 1_000), each killed at a 6 s external timeout:

awk 'BEGIN{while(1){x=x+1}}'      -> HUNG (killed at 6s)
awk 'BEGIN{for(i=0;i>=0;i++){}}'  -> HUNG (killed at 6s)
while true; do :; done            -> 12 ms  exit 1  "bash: execution step limit exceeded (10000)"

Note the third line: the shell-level infinite loop was already bounded, by max_steps and by Loop's @default_max_iterations 10_000 (executor/loop.ex:16). So the statement-loop check adds little that did not already exist, and the case it does not cover — "one command, many iterations" — is precisely the shape of the printf '%b' hang (56c74b8) that issue #69 cites as motivation. The category is not retired.

Suggested fix: thread the deadline into awk's evaluator loop constructs (and any other command with an unbounded internal loop) the way find.ex:142 does, or give them an iteration cap like Loop's. Failing that, narrow the claim in the PR title and in Limit's moduledoc to what is actually enforced — and either way add a test asserting that a single long-running command is bounded, since there is currently no such test.

---

2. JustBash.exec!/2 gets neither new guarantee

Severity: major — lib/just_bash.ex:528

exec/2 was hardened; exec!/2, the other documented public execution entry point, was left untouched. It calls Executor.execute_script/2 directly with no State.arm_deadline/2 and no rescue, so bash.interpreter.deadline stays nil and Limit.check_deadline!/1 short-circuits to :ok at all three checkpoints. Measured, limits: [max_wall_ms: 30, max_steps: 10_000_000]:

exec!  "for i in $(seq 1 100); do spin; done"  -> 1105 ms  exit 0  stderr ""   deadline=nil
exec   "for i in $(seq 1 100); do spin; done"  ->   33 ms  exit 1  "bash: execution wall clock limit exceeded (30 ms)"

exec!  "wreck-env; echo $HOME"  -> ** (Protocol.UndefinedError) escapes to the host
exec   "wreck-env; echo $HOME"  -> exit 1, "bash: internal error (...)"

The raise escaping is arguably a bang function's contract, though the docstring promises only "raising on parse errors" and a Protocol.UndefinedError naming a JustBash internal is not that. The wall clock is not defensible either way: unlike max_steps, max_wall_ms requires arming, so this PR introduces a bound that silently does not exist on one of the two public entry points. (exec_file/2 is fine — it delegates to exec/2.)

Suggested fix: arm the deadline in exec!/2 too — it is the same three lines — and state in its @doc which exceptions it is allowed to propagate.

---

3. Only find was bounded; FS.walk/3's new :deadline has no caller in lib/

Severity: major — lib/just_bash/fs/fs.ex:167

The PR summary names FS.walk/3 as one of the enforcement points, but grep -rn 'FS\.walk\|VFS\.walk' lib/ on this branch returns only the definition (fs.ex:167,171) and a doc mention in limit.ex:256 — zero production callers, let alone ones passing :deadline. The wrapper and Limit.enforce_deadline/2 are reachable only from test/sandbox_contract_test.exs:259-287.

Meanwhile every recursive command except find hand-rolls its traversal, and a whole traversal is charged as one step, so nothing else bounds them either. Measured, 8000-file tree under /tree, limits: [max_wall_ms: 1, max_steps: 10_000_000], each redirected to /dev/null:

find /tree          ->   16 ms  exit 1  "execution wall clock limit exceeded (1 ms)"
grep -r hello /tree -> 4895 ms  exit 0  stderr ""
du /tree            -> 5011 ms  exit 0  stderr ""
tree /tree          -> 5425 ms  exit 0  stderr ""
cp -r /tree /copy   -> 8136 ms  exit 0  stderr ""

These terminate eventually, so this is not a hang — but they overrun the declared bound by three to four orders of magnitude with no diagnostic, which is what max_wall_ms exists to prevent. Only the one command the issue happened to name is covered.

Suggested fix: either route the recursive commands through FS.walk(fs, root, deadline: bash.interpreter.deadline) — which would also give the new wrapper a reason to exist — or add Limit.check_deadline!/1 to each *_recursive helper the way find.ex:142 does. If neither is in scope, drop the unused :deadline branch and say plainly that find is the only bounded traversal.

---

4. x=$(failing_cmd) is a silent success, and command substitution swallows stderr

Severity: major — lib/just_bash/interpreter/expansion.ex:302

The PR checked composition against the oracle for cmd | cat, echo $(cmd) and if cmd, concluded the exit codes were already right, and fixed only the pipeline's lost stderr. Two things were missed in the sibling path. execute_command_substitution/2 is three lines and discards everything but stdout:

defp execute_command_substitution(bash, %AST.Script{} = script) do
  {result, _bash} = Executor.execute_script(bash, script)
  String.trim_trailing(result.stdout, "\n")
end

Oracle (GNU bash 3.2.57, this machine) vs this branch:

bash:      x=$(cat /nope); echo "code=$?"   -> stderr "cat: /nope: No such file or directory", code=1
JustBash:  x=$(cat /nope); echo "code=$?"   -> exit 0, stdout "code=0 x=[]", stderr ""

bash:      echo $(cat /nope)                -> stderr "cat: /nope: No such file or directory"
JustBash:  echo $(cat /nope)                -> stderr ""

The second line is the same defect this PR just fixed for pipelines, in the substitution path: a diagnostic produced inside the substitution never reaches the caller. The first is the one composition form where bash does propagate the substitution's status — a plain assignment (export x=$(...) and local x=$(...) correctly stay 0, since those return the builtin's own status). Combined, x=$(boom) — the issue's own crash probe — returns exit 0, stdout "", stderr "": a total silent success from a crashed command, reachable in one statement.

Suggested fix: return {stdout, stderr, exit_code} from execute_command_substitution/2; accumulate the stderr onto the enclosing statement the way execute_pipeline/2 now does, and set $? from the substitution's exit code on the bare-assignment path. Add oracle-backed tests for x=$(cat /nope) and x=$(boom).

---

5. The exec/2 internal-error net discards all stdout and rolls back all session state

Severity: major — lib/just_bash.ex:420

The rescue is a clause of exec/2's implicit try, so the only bash in scope is the function parameter — the rebinding in the body (bash = if ... arm_deadline ...) is not visible there, and neither is anything the script did. internal_error/2 therefore returns stdout: "", env: bash.env and the caller's original struct.

{_, b2} = JustBash.exec(b, "echo hi > /f.txt; export FOO=bar; wreck-env; echo $HOME")
          -> exit 1, stdout "", "bash: internal error (Protocol.UndefinedError: ...)"
JustBash.exec(b2, "cat /f.txt; echo FOO=$FOO")
          -> "cat: /f.txt: No such file or directory", "FOO="

Both the redirect and the export completed before the crash, and both vanished. This is inconsistent with the codebase's other error paths — including the one directly above it. The Limit.ExceededError handler at executor.ex:97 keeps prior work:

limits: [max_steps: 4], "echo a > /f.txt; echo one; ... echo six"
  -> exit 1, stdout "one\ntwo\nthree\n", "bash: execution step limit exceeded (4)"
  -> next exec: cat /f.txt  ->  "a"

command_crashed/3 is likewise correctly scoped to one command. Neither new internal-error test asserts anything about state or prior output (they check exit_code == 1 and a stderr prefix), so the rollback is uncovered. A long-lived host that reuses the struct silently loses a session.

Suggested fix: contain the internal error at execute_statement/2, alongside the Limit.ExceededError clause — that already produces the "halt, but keep everything earlier statements did" shape. If it must stay at the exec/2 level, wrap do_exec/2 in an explicit try with the armed bash bound outside it and thread the last-known-good struct out. Either way, add a test asserting a pre-crash > and export survive.

---

6. No test fails if the deadline is never re-armed

Severity: major — test/sandbox_contract_test.exs:203

test "the deadline is rearmed for each top-level exec" uses max_wall_ms: 200 and runs two execs of ~20 ms each. A deadline armed once and never refreshed still has ~175 ms left when the second exec starts, so the test passes either way. Verified by patching lib/just_bash.ex:398 to arm only when deadline == nil — the exact regression the test is named after:

mix test test/sandbox_contract_test.exs  ->  25 tests, 0 failures
mix test                                 ->  4777 tests, 0 failures

Two neighbours have the same shape. test "limits: false disables the wall clock too" (:213) runs 10 ms of work, so it passes under any non-degenerate budget and cannot catch Limit.deadline(nil) accidentally returning a default-budget deadline. And assert elapsed_us < 2_000_000 (:192) is 66x the 30 ms bound it guards, so a deadline that fired sixty times late still passes.

This is the #69/#70 lesson repeating: the boundary inputs are chosen so all the interesting variants render the same result.

Suggested fix: make the budget smaller than the cumulative work across execs — e.g. max_wall_ms: 30 with two spins per exec (40 ms total), so a stale deadline trips on the second call. Tighten elapsed_us to the order of the bound (e.g. < 300_000 for 30 ms). For limits: false, assert the absence structurally: bash.interpreter.deadline == nil after the exec.

---

7. Containing a registry crash silently retires the documented [:just_bash, :command, :exception] event

Severity: major — lib/just_bash/interpreter/executor.ex:861

invoke_builtin/5's rescue sits inside Telemetry.command_span/3 (executor.ex:417), so :telemetry.span/3 never sees the exception and never emits its :exception event. telemetry.ex:46 documents [:just_bash, :command, :exception] as "Emitted when a command raises", and telemetry.ex:96 lists it in the copy-paste attach_many example. For all ~90 registry commands that is now false. Verified both directions on this branch with a handler attached to the four :exception events, running wreck; cat /etc/hosts:

with invoke_builtin/5 (as merged):    exception events emitted: []
with the try removed (pre-PR shape):  [{[:just_bash, :command, :exception], "cat"},
                                       {[:just_bash, :session, :run, :exception], nil}]

So the containment is not merely "no new telemetry" — it removes signal a host could already subscribe to. The failure class this PR contains (#52's cp MatchError, #67's date FunctionClauseError) used to be loud; it is now an ordinary exit 1, indistinguishable in metrics from a script legitimately failing.

Suggested fix: emit from command_crashed/3 — either :telemetry.execute([:just_bash, :command, :crash], ...) with the exception and stacktrace, or re-raise into the span and catch outside it so the existing documented event keeps firing. [:just_bash, :session, :run, :exception] does still fire at exec/2's net today (the rescue is outside session_span) — worth a test so it stays that way. If the command event is genuinely retired, update telemetry.ex:46.

---

8. internal_error/2's stderr bypasses max_output_bytes

Severity: minor — lib/just_bash.ex:420

internal_error/2 builds stderr from Exception.message(error) and returns straight out of the rescue, outside execute_statement/2's Limit.track_output! accounting. The per-command path does not have this problem — command_crashed/3 flows through the normal result and is correctly refused. Measured with limits: [max_output_bytes: 100]:

"wreck-env; echo $HOME"       -> 1107 bytes of stderr returned
"bigraise" (140 KB message)   -> 45 bytes: "bash: output size limit exceeded (100 bytes)"

Exception.message/1 is unbounded from the sandbox's point of view (inspect/1 allows 4096 bytes per binary and 50 collection elements), so a MatchError or KeyError carrying interpreter or FS state can inline sandbox file contents into a stderr the host asked to be capped.

Suggested fix: truncate the detail in internal_error/2 to max_output_bytes (or a small fixed cap) before building the message.

---

Considered and merged/dismissed (3 of 11 candidates)

• "The wall clock is only checked between statements" — duplicate of finding 1. Same defect, found independently by both lenses. Merged; kept the better-argued framing plus the observation that while true; do :; done was already bounded by max_steps, which is what makes the remaining gap the interesting one.
• "FS.walk/3's :deadline and Limit.enforce_deadline/2 have no caller outside the new tests" — duplicate of finding 3. The dead-option observation and the unbounded-grep -r/cp -r measurement are one finding seen from two sides; merged, since giving the option a caller resolves both.
• "The outer rescue rolls back the whole script's filesystem and env side effects" — duplicate of finding 5. Merged. The surviving version explains why — the implicit try cannot see the body's rebinding — and that explanation drives the suggested fix.

Nothing was outright refuted: every candidate reproduced on d0efbf2. Two severities were adjusted. The exec!/2 finding dropped from blocker to major (a bang function propagating a raise is partly defensible; the missing wall clock is not). The telemetry finding rose from minor to major once it turned out a documented event stops firing, rather than merely a new one not being added.

[davydog187]

All 8 confirmed findings are addressed. Every one was reproduced on d0efbf2 first, written as a failing test, then fixed, then the original repro was re-run against the stated oracle. Nothing is disputed.

#	Finding	Resolution	Commit
1	"always terminates" is false: a loop inside a single command hangs forever	fixed — awk's for/while/do-while check the deadline per iteration; seq's range walk goes through Limit.enforce_deadline/2	554d8d9
2	exec!/2 gets neither new guarantee	fixed — arming shared with exec/2 via arm_top_level/1; docstring rewritten	554d8d9 (+ cbcd10a)
3	Only find was bounded; FS.walk/3's :deadline has no caller	fixed — grep -r, du, tree and cp -r bounded; the dead :deadline option removed	554d8d9
4	x=$(failing_cmd) is a silent success, and $( ) swallows stderr	fixed — substitutions carry a {:substitution, stderr, exit_code} trace out of the expansion	156e38b
5	The exec/2 net discards all stdout and rolls back all session state	fixed — containment moved into the statement loop, where the last known-good shell lives	cbcd10a
6	No test fails if the deadline is never re-armed	fixed — all three tests now go red against the mutation they are named for	57dbf5f
7	Containing a registry crash retires [:just_bash, :command, :exception]	fixed — the rescue moved outside command_span/3; custom commands now emit it too	cbcd10a
8	internal_error/2's stderr bypasses max_output_bytes	fixed — truncated to min(512, max_output_bytes) on a UTF-8 boundary	cbcd10a

---

1 — a loop inside a single command

Reproduced: awk 'BEGIN{while(1){x=x+1}}' and awk 'BEGIN{for(i=0;i>=0;i++){}}' under limits: :strict both hung past a 10 s external kill. After: 1003 ms and 1001 ms, exit 1, bash: execution wall clock limit exceeded (1000 ms).

Audited the other candidates the review named. sed has no label/branch support (sed ':a;ba' is sed: unknown command: :), printf's format recycling terminates, and the expansion paths are bounded by the statement loop. One more real instance turned up: seq 1 100000000 is a single command doing 100M iterations, and it did not terminate inside a 4 s window. Its range now goes through Limit.enforce_deadline/2 — which also gives that function the production caller FS.walk/3 never had.

Five new tests, each run under a Task so a regression fails the test instead of hanging the suite.

2 — exec!/2

Reproduced exactly as reported: exec! ran the 100-spin loop for 1118 ms at exit 0 with deadline: nil, against exec's 36 ms and exit 1. After: 33 ms, exit 1, same diagnostic, deadline populated.

On the second half of the finding — "decide and document which raises exec!/2 is contractually allowed to propagate" — the answer changed while fixing finding 5. Since containment now lives in the interpreter rather than in exec/2's rescue, exec!/2 inherits it: a crashed command and a raise from the statement loop are shell results on both entry points. What exec!/2 still propagates is a parse error (as a RuntimeError) and anything raised outside the statement loop — the EXIT trap and telemetry. The docstring says exactly that, and there is a test for each half.

3 — recursive traversals

Reproduced on the review's 8000-file tree with max_wall_ms: 1:

before                              after
find /tree          16 ms  exit 1   22 ms  exit 1
grep -r hello /tree 4608 ms exit 0   15 ms  exit 1
du /tree            4479 ms exit 0   10 ms  exit 1
tree /tree          5555 ms exit 0    9 ms  exit 1
cp -r /tree /copy   8067 ms exit 0    6 ms  exit 1

grep, du and tree check the deadline in their own recursive helper, the way find does. cp -r recurses inside FS.cp/4, so that gained a :deadline option and cp.ex passes it. rm -rf was measured too and is not affected — 3 ms on the same tree, because it is a bulk prune rather than a hand-rolled descent.

FS.walk/3's :deadline option is gone. It never had a caller in lib/, and the review listed dropping it as an acceptable resolution; Limit.enforce_deadline/2 is the general mechanism and is now reached from seq. Its three unit tests moved onto Limit.enforce_deadline/2 directly.

4 — command substitution

Checked fifteen forms against GNU bash 3.2.57 on this machine before writing anything. The rules that fell out:

• a substitution's diagnostic always reaches the shell's stderr, including past a 2> on the enclosing command (echo $(cat /nope) 2>/dev/null still prints it), because bash performs redirections after expansion;
• a bare assignment reports the last substitution's exit status as $? — y=$(cat /nope) x=$(cat /nada) is code=1, x=$(cat /nope) y=$(echo ok) is code=0;
• export, local and declare keep their own status, and a command keeps its own.

execute_command_substitution/2 now returns a {:substitution, stderr, exit_code} trace alongside the assignments an expansion already hands back, so it travels out of arbitrarily nested expansions — "$(...)", ${x:-$(...)}, $(( $(...) + 1 )), backticks — without a second channel. Expansion.take_substitutions/1 splits them out at the two consumers.

All ten probe scripts now match the oracle byte for byte.

Known remaining gap, deliberately out of scope: array literals (arr=($(cat /nope))) and for-loop word lists (for i in $(cat /nope)) still discard expansion side effects and stay silent. Both also drop ${VAR:=default} assignments, which predates this PR, so the whole expand_for_loop_word/3 path wants its own change rather than a widening of this one.

5 — the internal-error net

Reproduced: the redirect and the export both vanished. After, on the same probe, the next exec on the returned struct prints hi and FOO=bar.

Containment moved from exec/2's rescue into Executor.run_statement/2. That is where the last known-good shell actually lives — the accumulated output of the statements that already ran, and the struct they left behind — which is the thing an exec/2-level rescue structurally cannot see. It halts the script and preserves both, matching the Limit.ExceededError handler. exec/2's net stays as a last resort for raises outside the statement loop, where there is no session state to preserve.

Three tests: prior stdout and state survive, the script halts, and the Limit handler's behaviour is pinned alongside as the reference.

6 — tests that could not fail

Verified the arm-once patch (arm_top_level/1 arming only when deadline == nil) against the suite as merged: test/sandbox_contract_test.exs stayed at 61 tests, 0 failures. The rearm test now runs six execs of ~55 ms against a 200 ms budget — each one comfortably inside the budget alone, so only rearming keeps them green — with a structural sibling asserting each exec's at_ms is strictly later than the last. Both go red against the patch.

limits: false asserts the absence structurally; verified red against deadline(nil) -> deadline(defaults()). elapsed_us tightened from 2 s to 300 ms for a 30 ms bound.

7 — telemetry

Reproduced: wreck; cat /etc/hosts emitted no exception events at all. After, with a handler on all four :exception events: [{[:just_bash, :command, :exception], "cat"}].

The rescue moved outside command_span/3 as contain_command_crash/3, so :telemetry.span/3 sees the exception and emits the documented event with its real measurements. execute_custom_command/5's own rescue folded into the same place, which means a host-supplied command that crashes is now visible in telemetry too — it never was, even before this PR. Both wordings are preserved.

Consequence worth flagging: [:just_bash, :session, :run, :exception] no longer fires for a contained crash, because the script no longer raises — it completes with a non-zero exit. That is the point of the containment, but it does change what a subscriber sees, so both event docs in telemetry.ex now say so. Three tests, including a refute_receive for a command that does not raise.

8 — unbounded internal-error stderr

Reproduced: 1107 bytes returned under max_output_bytes: 100. After: exactly 100 bytes under that cap, and 512 under the default. Truncation lands on a UTF-8 boundary so the diagnostic stays printable.

---

Gates

mix compile --warnings-as-errors --force

Compiling 168 files (.ex)
Generated just_bash app

mix format --check-formatted

(no output, exit 0)

mix credo --strict

  Refactoring opportunities

┃ [F] ↘ Avoid `apply/2` and `apply/3` when the number of arguments is known.
┃       test/support/banned_fixture_apply.ex:4:16 #(BannedCallTracer.Fixture.Apply.run)

Analysis took 1.7 seconds (0.09s to load, 1.6s running 68 checks on 230 files)
5149 mods/funs, found 1 refactoring opportunity.

The single finding is the intentional test fixture, unchanged from main.

mix test

Finished in 23.8 seconds (23.5s async, 0.2s sync)
2 doctests, 62 properties, 4814 tests, 0 failures (5 excluded)

Baseline was 4777; the 37 new tests are the ones described above.

mix dialyzer

Total errors: 13, Skipped: 13, Unnecessary Skips: 0
done in 0m3.1s
done (passed successfully)

[davydog187]

Verification of review fixes

Independently re-ran every repro at 16c85b3, plus the two named mutations, plus a scan for
collateral. Comparisons against the pre-fix head were run at d0efbf2 in the same worktree.

#	Finding	Verdict	Evidence
1	"always terminates" false for a loop inside one command	still_broken (partially fixed)	Named repros pass under limits: :strict: awk 'BEGIN{while(1){x=x+1}}' 1002 ms exit 1, awk 'BEGIN{for(i=0;i>=0;i++){}}' 1000 ms exit 1, awk 'BEGIN{do{x=x+1}while(1)}' 1000 ms exit 1, seq 1 100000000 > /dev/null 1000 ms exit 1, all with bash: execution wall clock limit exceeded (1000 ms); awk 'BEGIN{for(i=0;i<3;i++) print i}' 1 ms exit 0. But the audit's conclusion that "the expansion paths are bounded by the statement loop" is false: echo {1..100000} > /dev/null under limits: :strict → 18341 ms, exit 0, stderr ""; echo {1..1000000} did not terminate in 30 s. Same at d0efbf2 (18605 ms), so not a regression — but it is an unfixed instance of exactly the class this finding asked to be closed, in a path the finding named.
2	exec!/2 gets neither guarantee	fixed	limits: [max_wall_ms: 30, max_steps: 10_000_000], exec! on for i in $(seq 1 100); do spin; done → 35 ms, exit 1, bash: execution wall clock limit exceeded (30 ms), deadline=%Limit.Deadline{at_ms: -576460751604, max_wall_ms: 30} (was 1105 ms / exit 0 / deadline nil). exec!(b, "echo before; wreck-env; echo $HOME") returns a shell result: exit 1, stdout "before\n". Documented raise still raises: exec!(new(), "echo 'unterminated") → RuntimeError: Parse error: unterminated single quote.
3	Only find was bounded	fixed	8000-file tree, limits: [max_wall_ms: 1, max_steps: 10_000_000], each > /dev/null: find 3 ms, grep -r 3 ms, du -sh 4 ms, tree 4 ms, cp -r 3 ms — all exit 1 with bash: execution wall clock limit exceeded (1 ms) (was 4608/4479/5555/8067 ms at exit 0). rm -r 1 ms exit 0, consistent with the bulk-prune argument. Functional output unaffected: cp -r /a /d && cat /d/b/c.txt → z; grep -r, du -s, tree, find all still correct; seq 1 5, seq 4, seq 10 -3 1 unchanged.
4	x=$(failing_cmd) silent success	fixed (stated repros)	x=$(cat /nope); echo code=$? x=$x → exit 0, stdout "code=1 x=\n", stderr "cat: /nope: No such file or directory\n". echo $(cat /nope) → stderr now present. y=$(cat /nope) x=$(cat /nada) → code=1; x=$(cat /nope) y=$(echo ok) → code=0; export x=$(cat /nope) → code=0. No collateral: nested substitution, backticks, $(( $(…) )), substitution in a pipeline/condition/function, trailing-newline trim, and set -e; x=$(cat /nope) (exit 1, no after) all behave. Residual — the disclosed gap is real and has a third member the commit note does not list: case $(cat /nope) in *) echo m;; esac also drops the diagnostic, alongside arr=($(cat /nope)) (bash code=1, JustBash code=0, no stderr) and for w in $(cat /nope). Pre-existing, not introduced here.
5	Internal-error net discards stdout and state	fixed	echo hi > /f.txt; export FOO=bar; echo before; wreck-env; echo $HOME → exit 1, stdout "before\n"; next exec on the returned struct → "hi\nFOO=bar\n" (was stdout "" and cat: /f.txt: No such file or directory / FOO=). Matches the neighbouring limit handler, re-measured: max_steps: 4 → stdout "one\ntwo\nthree\n", following cat /f.txt → "a".
6	No test fails if the deadline is never re-armed	fixed (mutation-verified)	Baseline mix test test/sandbox_contract_test.exs → 62 tests, 0 failures. Applied the reviewer's mutation (State.arm_deadline(bash.interpreter.deadline || Limit.deadline(bash.limits))) → 2 failures: "the deadline is rearmed for each top-level exec" (exec 4: bash: execution wall clock limit exceeded (200 ms)) and "each exec's deadline is a later instant than the previous one's" (both sides are exactly equal, left: -576460746387). Restored, green. Applied Limit.deadline(nil) → deadline(defaults()) → 1 failure: "limits: false disables the wall clock too" (left: %Deadline{at_ms: -576460746135, max_wall_ms: 5000}, right: nil). Restored, green. The 66x guard is now elapsed_us < 300_000. Also spot-checked the new awk tests by deleting Limit.check_deadline!/1 from execute_while_loop/3 → 1 failure, "awk 'BEGIN{while(1){x=x+1}}' did not terminate within 5s".
7	Registry crash retires [:just_bash, :command, :exception]	fixed	Handler on all four :exception events. wreck; cat /etc/hosts → [{[:just_bash, :command, :exception], "cat", [:monotonic_time, :duration]}]; boom → same for "boom" (custom-command crashes now emit too); echo hi → []. [:just_bash, :session, :run, :exception] no longer fires for a contained crash, as documented in the telemetry.ex diff.
8	internal_error/2 bypasses max_output_bytes	fixed	wreck-env; echo $HOME: max_output_bytes: 100 → stderr exactly 100 bytes (was 1107); :relaxed → 512; :default → 512. String.valid?/1 true in all three.

Gates (re-run here, Elixir 1.19.5 / Erlang 28.3)

$ mix compile --warnings-as-errors
Compiling 2 files (.ex)
Generated just_bash app

$ mix format --check-formatted
(no output, rc=0)

$ mix test
Finished in 23.1 seconds (22.9s async, 0.2s sync)
2 doctests, 62 properties, 4814 tests, 0 failures (5 excluded)

$ mix credo --strict
5151 mods/funs, found 1 refactoring opportunity.
  [F] Avoid `apply/2` and `apply/3` ... test/support/banned_fixture_apply.ex:4:16
  (the intentional test fixture)

$ mix dialyzer
Total errors: 13, Skipped: 13, Unnecessary Skips: 0
done (passed successfully)

New problems introduced by the fix commits

None found. Three things that look wrong were checked at d0efbf2 and behave identically there,
so they are pre-existing and out of scope for this verification:

• seq 1 0.5 3 → bash: seq: command crashed (ArgumentError: ranges (first..last//step) …).
  Integer.parse("0.5") yields step 0; the f..l//i construction predates the render/2
  refactor. Same for seq 1 0 5.
• x=$(cat /nope) 2>/dev/null; echo code=$? → code=127, bash: 2: command not found
  (bash 3.2.57: code=1). Not substitution-related — x=5 2>/dev/null misparses identically,
  before and after.
• Glob-driven fan-out is not deadline-checked either (wc -l /tree/*/*.txt 150 ms, ls -R 5 ms
  on a 1 ms budget over 8000 files). Two orders of magnitude, not four; for the record rather
  than as a blocker.

Bottom line

7 of 8 fixed and verified. Finding 1 is the one that should not close: the three awk loops and
seq are genuinely bounded now, and the new tests fail against a backed-out fix — but
echo {1..100000} runs 18 seconds to exit 0 under a 1-second wall clock, which refutes the
commit subject's "always terminates" for the same reason the original finding did.

[davydog187]

The last open item: brace expansion was unbounded

afe0aa8 closes it, and retracts an audit claim I made in 554d8d9. That
commit said "the expansion paths are bounded by the statement loop." They were
not, and a thirty-second measurement refutes it. Under limits: :strict, at
that commit:

script	before
echo {1..100000} > /dev/null	18341 ms, exit 0, stderr ""
echo {1..1000000} > /dev/null	did not terminate within 30s
echo {1..300}{1..300}{1..300} > /dev/null	did not terminate within 30s
for i in {1..200000}; do :; done	did not terminate within 10s
a=({1..200000}); echo done	did not terminate within 10s

Pre-existing rather than a regression — d0efbf2 measures 18605 ms for the
first one — but it is an unfixed instance of exactly the class the finding
asked me to close, in the path the finding named, on a PR whose subject claims
the class is retired.

Why the statement loop misses it

The same reason it missed awk's while: a whole word is one step no matter
what it names, and the expansion completes before the loop is re-entered.
{1..1000000} is twelve characters and a million words.

Two instruments, because a word list has two ways to be too expensive

Cardinality, per the review's own suggestion. A million words is counted
work, not merely slow work, so Limit.check_expansion_words!/2 holds one
word's expansion to :max_steps — the bound that already counts work, rather
than a sixth limit key. Two places, because a range and a product fail
differently:

• A range is measured before it is built. range_size/3 computes what
  expand_range/3 would allocate; counting words as they arrive is too late
  when they arrive all at once. Measured: echo {1..20000000} costs 880 ms and
  roughly 1 GB to build, and 0 ms to measure.
• A product is counted as the walk produces it. {1..50}{1..50} has no
  individual range worth refusing — only the 2,500 words they name together.

The wall clock, checked per word produced, so a max_steps generous enough
to permit the list still cannot be spent entirely on building it.

Making the bounds reachable meant fixing the walk. Each level was concatenating
its children's results (words_acc ++ new_words) — that quadratic is why 100k
words took 18 s. One accumulator now threads through the whole cartesian walk.
Linear is nice; the actual point is that it gives both bounds a single place
that sees every word at the moment it is produced.

Also on the audit: glob expansion

Glob descends the filesystem itself, one directory read per wildcard segment,
and had no deadline. It is the only traversal that runs before a command is
even chosen — so 554d8d9 bounded find, grep -r, du, tree and cp -r
on the 2000-file tree while echo /tree/*/*/* on that same tree ran 295 ms
unbounded at exit 0. It now carries the deadline through the descent and
checks it per matched entry, and joins the existing traversal test table.

The rest of the audit — measured this time, not asserted

path	verdict
$((10 ** 1000000))	rejected by the arithmetic parser, 4 ms
$((1 << 100000000))	SystemLimitError, contained at exit 1 in 2 ms
${var//pat/rep} and friends	bounded by the length of the value, itself bounded by max_output_bytes / max_file_bytes
for i in {...}, a=({...})	same brace code path, covered
glob	fixed above

After

Same harness, same limits: :strict:

echo {1..100000} > /dev/null       -> 7 ms, exit 1, "bash: word expansion limit exceeded (10000 words)"
echo {1..1000000} > /dev/null      -> 0 ms, exit 1, "bash: word expansion limit exceeded (10000 words)"
echo {a,b}{1..50000} > /dev/null   -> 0 ms, exit 1, "bash: word expansion limit exceeded (10000 words)"
echo {1..300}{1..300}{1..300} > /dev/null -> 3 ms, exit 1, "bash: word expansion limit exceeded (10000 words)"
for i in {1..200000}; do :; done   -> 2 ms, exit 1, "bash: word expansion limit exceeded (10000 words)"
a=({1..200000}); echo done         -> 0 ms, exit 1, "bash: word expansion limit exceeded (10000 words)"

Nothing legitimate moved. echo {1..5} {a..e} {x,y}{1..3} and
echo {1..10}{1..10} | wc -w still match GNU bash 5.x exactly.

Tests

Ten new cases in test/sandbox_contract_test.exs, all following the existing
bounded_exec/2 shape — every probe runs under a Task with a timeout, so a
regression fails the suite instead of wedging CI. Each of the four new checks
was verified against its own deletion:

delete	test that fails
the per-word count	a product of ranges each inside the bound is still counted
the range pre-measure	a range is refused before it is built, not after (on elapsed time)
the brace deadline	the wall clock still bounds a word list the cardinality cap allows
the glob deadline	glob expansion is bounded by the wall clock

Gates

mix compile --warnings-as-errors   exit 0, no output
mix format --check-formatted       exit 0
mix credo --strict                 1 finding: test/support/banned_fixture_apply.ex (the intentional fixture)
mix test                           2 doctests, 62 properties, 4825 tests, 0 failures
mix dialyzer                       Total errors: 13, Skipped: 13, Unnecessary Skips: 0 - done (passed successfully)

[davydog187]

Final verification

Independent re-verification of afe0aa8 ("brace expansion is bounded, and glob expansion is on the
same clock"). Everything below was re-measured on a fresh detached checkout of
origin/fix/issue-69-sandbox-contract — every termination claim under Task.async +
Task.yield(timeout) + brutal_kill, never by reading the code.

Verdict: all four claims hold. One minor inaccuracy in the claims themselves, reported below.
No regressions; three unclaimed improvements.

---

Per-item

#	Claim	Verdict	Evidence
1	Brace expansion is unbounded — fixed	fixed	8/8 named repros return in ≤ 6 ms at exit 1; the pre-fix state re-measured and matches the commit message
2	Glob expansion had no deadline — fixed	fixed	Deleting the deadline flips echo /tree/*/*/* back to exit 0 and the test goes red
3	The false audit claim in 554d8d9 — retracted and made true	fixed	Retraction is in the new commit message with the before-numbers; the claim is now true, and every named sibling re-measured
4	Tests fail rather than wedge CI	fixed, with a caveat	All four mutations verified red one at a time; but 4 of the new probes are not Task-wrapped, contrary to the note — see below

---

1. Brace expansion — limits: :strict, 30 s task timeout

At afe0aa8 (this head):

echo {1..100000} >/dev/null         ->     6 ms, exit 1, "bash: word expansion limit exceeded (10000 words)"
echo {1..1000000} >/dev/null        ->     0 ms, exit 1, "bash: word expansion limit exceeded (10000 words)"
echo {1..5000000} >/dev/null        ->     0 ms, exit 1, "bash: word expansion limit exceeded (10000 words)"
echo {a,b}{1..50000}                ->     0 ms, exit 1, "bash: word expansion limit exceeded (10000 words)"
echo {1..300}{1..300}{1..300}       ->     3 ms, exit 1, "bash: word expansion limit exceeded (10000 words)"
for i in {1..200000}; do :; done    ->     1 ms, exit 1, "bash: word expansion limit exceeded (10000 words)"
a=({1..200000}); echo done          ->     0 ms, exit 1, "bash: word expansion limit exceeded (10000 words)"
echo {1..20000000}                  ->     0 ms, exit 1, "bash: word expansion limit exceeded (10000 words)"

At the prior head 16c85b3, same harness, same :strict — the "before" numbers are real:

echo {1..100000} >/dev/null         -> 19263 ms, exit 0, stderr ""
echo {1..1000000} >/dev/null        -> DID NOT TERMINATE within 20000 ms
echo {1..300}{1..300}{1..300}       -> DID NOT TERMINATE within 20000 ms
for i in {1..200000}; do :; done    -> DID NOT TERMINATE within 15000 ms
a=({1..200000}); echo done          -> DID NOT TERMINATE within 15000 ms

Carried-over blockers from the previous round still hold at this head:

awk 'BEGIN{while(1){x=x+1}}'        ->  1002 ms, exit 1, "execution wall clock limit exceeded (1000 ms)"
awk 'BEGIN{for(i=0;i>=0;i++){}}'    ->  1002 ms, exit 1, "execution wall clock limit exceeded (1000 ms)"
awk 'BEGIN{do{x=x+1}while(1)}'      ->  1000 ms, exit 1, "execution wall clock limit exceeded (1000 ms)"
seq 1 100000000 >/dev/null          ->  1000 ms, exit 1, "execution wall clock limit exceeded (1000 ms)"

Oracle: GNU bash on this machine is 3.2, so step and zero-pad syntax was excluded; 27 ordinary brace
shapes were compared against /bin/bash. The two the commit message names match exactly —
echo {1..5} {a..e} {x,y}{1..3} → 1 2 3 4 5 a b c d e x1 x2 x3 y1 y2 y3, and
echo {1..10}{1..10} | wc -w → 100. Four shapes diverge from bash
({{1..3},{a..c}}, {,}, {,a}, {a,}); all four diverge identically at 16c85b3, so they
are pre-existing and untouched by this change.

2. Glob deadline

Verified by mutation rather than by re-timing the 295 ms figure: replacing the deadline argument to
Limit.enforce_deadline/2 in expand_wildcard_segment/6 with nil flips echo /tree/*/*/* on the
2000-file tree back to exit 0 and turns test "glob expansion is bounded by the wall clock" red.
Restored, green.

3. The retraction, and the rest of the audit

The retraction is where it says it is — the opening paragraph of afe0aa8 names 554d8d9, quotes the
false sentence, and carries the three before-numbers. Declining to force-push a twice-reviewed commit
was the right call.

Every sibling the audit names, re-measured at this head under :strict:

echo $((10 ** 1000000))                       ->  6 ms, exit 1, "bad argument in arithmetic expression"
echo $((1 << 100000000))                      ->  1 ms, exit 1, "internal error (SystemLimitError: ...)"
echo $((2 ** 64))                             ->  0 ms, exit 0, "18446744073709551616"
v=<60 KB of a>; echo ${v//a/bb}               ->  2 ms, exit 0   (bounded by the value's length)
v=aaaa…; echo ${v//a*a*a*a*a*a*b/x}           ->  0 ms, exit 0   (no catastrophic backtrack)
a=b; b=c; echo ${!a}                          ->  0 ms, exit 0, "c"
f() { f; }; f                                 ->  6 ms, exit 1, "maximum call depth exceeded"
mkdir -p /t; …200 files…; echo /t/* | wc -w   ->  4 ms, exit 0, "200"

Extra generating shapes I added that the audit did not name — all terminate:

echo {x,{1..1000000}}                                      ->    0 ms, exit 1, word expansion limit
echo {x,{1..5000}{1..5000}}                                ->    4 ms, exit 1, word expansion limit
echo {a,b} × 15 (32768 words from 60 chars)                ->   11 ms, exit 1, word expansion limit
for i in $(seq 1 500); do echo {1..9000} >/dev/null; done  -> 1001 ms, exit 1, wall clock
i=0; while [ $i -lt 100000 ]; do echo {1..9000} …; done    -> 1000 ms, exit 1, wall clock
echo {1..9000} {1..9000} {1..9000} {1..9000} >/dev/null    ->   11 ms, exit 0

The last line is worth stating explicitly, since the cap is per word: N words each sitting at the
bound produce N × max_steps words in one command. N is bounded by input length and the wall clock
still covers the repeated case (lines 4–5), so this is a documented semantic rather than a hole — but
it is the semantic the bound actually has, and the moduledoc's "one word's expansion" phrasing is the
accurate reading.

4. Mutation testing — every new check verified against its own deletion

Deleted one at a time, mix test test/sandbox_contract_test.exs run, then restored (working tree
verified clean between each):

Mutation	Test that went red	Failure
Drop Limit.check_expansion_words!(bash, count) from expand_into/3	a product of ranges each inside the bound is still counted	exit_code left 0, right 1
Replace the range pre-measure with _ = range_size(...)	a range is refused before it is built, not after	elapsed_us — 916543 < 200000 failed
Drop Limit.check_deadline!(bash) from expand_into/3	the wall clock still bounds a word list the cardinality cap allows	stderr was output size limit exceeded (1048576 bytes), not the wall clock
Pass nil instead of deadline in expand_wildcard_segment/6	glob expansion is bounded by the wall clock	exit_code left 0, right 1

And the whole-fix revert (git checkout 16c85b3 -- lib/…/brace.ex) produces 8 failures, matching
the fixer's "6 of 8 failed, the two untouched cases passed as intended" — the five table probes plus
the three targeted ones. Baseline unmutated is 73 tests, 0 failures in 1.6 s.

Minor finding — the one thing that is not as claimed. The fix note says "All new probes use the
existing bounded_exec/2 shape", and the describe-block comment says "Each probe runs under a task
so a regression fails the test instead of wedging the suite." Both are inaccurate: four of the new
tests call JustBash.exec/2 directly, including one of the four mutation-critical ones —
test "a range is refused before it is built, not after" (test/sandbox_contract_test.exs:599) and
test "a product of ranges each inside the bound is still counted" (:614).

This is observable, not theoretical. Under the whole-fix revert, the "refused before it is built" test
does not flunk at 5 s — it runs until ExUnit's 60 s per-test timeout, and the file's run time goes
from 1.6 s to 91.6 s:

8) test … a range is refused before it is built, not after
   ** (ExUnit.TimeoutError) test timed out after 60000ms
   stacktrace:
     lib/just_bash/interpreter/expansion/brace.ex:46: … Brace.expand_with_brace/2
…
Finished in 91.6 seconds
73 tests, 8 failures

The suite still goes red, so CI is not wedged, and that test's elapsed_us assertion does need a real
measurement — but wrapping the :timer.tc call inside a Task keeps both properties. Not a blocker;
the check itself is genuine and mutation-sensitive.

---

Regression sweep

Old vs. new, mechanically diffed. 56 scripts run at 16c85b3 and at afe0aa8 under
limits: :strict, with exit code + stdout + stderr serialized and compared term-by-term:

NO BEHAVIOURAL DIFFERENCES across 56 scripts

What was swept, so the absence of findings is legible rather than ambiguous:

• Brace, ordinary shapes — {a,b,c} with prefix / suffix / both, {1..5}, {5..1}, {1..10..2},
  {10..1..-3}, {a..e}, {e..a}, {a..e..2}, {01..05}, {-3..3}, {3..-3}
• Brace, degenerate shapes — {}, {a}, {,}, {,a}, {a,}, zero step ({1..10..0}, the one
  input where the new range_size/3 mirror could have divided by zero — it returns 0, matching
  expand_range/3), backwards step ({1..10..-2}), quoted ('{a,b}', "{a,b}"), escaped (\{a,b\})
• Brace, cartesian and nested — {1..3}{a..c}, {a,b}{c,d}{e,f}, {a,{b,c},d}, {{1..3},{a..c}},
  {a..c}{1..2}{x,y}, a{b,c}d{e,f}g, file{1..3}.{txt,log}
• Brace crossed with other expansions — {$v,there}, {a,b}$v, {a,b}$(echo z), $((1+1)){a,b}.
  This is the path where the accumulator rewrite could have reordered pending assignments; output and
  ordering are identical before and after.
• Brace in other syntactic positions — for i in {1..4}, a=({1..4}), a=({a,b}{1,2}), pipelines,
  case patterns
• Glob — single and multi-segment (/t/*, /t/*/*, /t/*/*/*), suffix (*.txt), no-match fallback
  to the literal pattern, ?, [12], dotfile exclusion, trailing-slash preservation (/t/a/*/),
  relative globs after cd, glob as an operand to ls and cat, glob + brace together
  (/t/{a,b}.txt, /t/{a,b}*)
• Limit presets — false, :strict, :default, :relaxed all exercised on legitimate expansions;
  all correct and fast (echo {1..50000} | wc -w → 50000 in 43–46 ms)
• Neighbouring unbounded-work paths — arithmetic overflow, ${var//pat/rep}, indirect expansion,
  function recursion depth, nested eval (table in §3)

Three unclaimed improvements fell out of the sweep:

• v=abc; for i in $(seq 1 30); do v=$v$v; done; echo ${#v} — at 16c85b3 this ran 4069 ms at
  exit 0 and allocated a 3.2 GB string, straight through the 1000 ms :strict budget. At afe0aa8
  it is 1203 ms, exit 1, wall clock limit exceeded. The per-word deadline check now sits in the
  path every word takes, so it catches this too.
• printf 'a%.0s' {1..100000} — DID NOT TERMINATE within 15 s before, 0 ms, exit 1 now.
• Side effect of the accumulator rewrite: with limits: false, echo {1..100000} | wc -w went from
  ~18 s to 101 ms.

Two pre-existing issues found and confirmed not caused by this PR — identical at 16c85b3, and
neither in the surface this commit touches. Recording them so they are not rediscovered as regressions
in a later round:

• awk 'BEGIN{for(;;){…}}' — an empty for condition evaluates as false, so the body never runs.
  awk 'BEGIN{n=0; for(;;){n++; if(n>3) break}; print n}' prints 0; GNU awk prints 4. It
  terminates, so this is a correctness bug, not a sandbox hole, and it is out of scope for The sandbox contract permits a host crash and an unbounded hang: builtin raises escape exec/2, and Limit has no wall-clock bound #69. The
  for variant the suite covers, for(i=0;i>=0;i++), is bounded correctly at 1002 ms.
• v=abc; while true; do v=${v}${v}; done overruns the 1000 ms budget by ~2.4–2.9× (2422 ms at
  16c85b3, 2946 ms at afe0aa8) because a single string-doubling step allocates gigabytes between
  deadline checks. It returns at exit 1 in both, so the exec/2-returns contract holds; the overrun
  factor is unchanged by this commit.

---

Gates — all five re-run on this head (afe0aa8)

$ mix format --check-formatted
(clean, exit 0)

$ MIX_ENV=test mix compile --force --warnings-as-errors
Compiling 176 files (.ex)
Generated just_bash app                                          (exit 0)

$ mix credo --strict
5158 mods/funs, found 1 refactoring opportunity.
  [F] test/support/banned_fixture_apply.ex:4:16 — Avoid `apply/2` and `apply/3`
  (the intentional test fixture; exit 0)

$ mix test
Finished in 23.8 seconds (23.6s async, 0.2s sync)
2 doctests, 62 properties, 4825 tests, 0 failures (5 excluded)

$ mix dialyzer
Total errors: 13, Skipped: 13, Unnecessary Skips: 0
done (passed successfully)

[davydog187]

Round 4 — the last open item: four probes were not actually task-wrapped

ddc4ee3. One file, test/sandbox_contract_test.exs, +15/−8. No lib/ change.

The round-3 note said "All new probes use the existing bounded_exec/2 shape (Task.async +
Task.yield 5s + brutal_kill, flunk on nil)", and the describe-block comment said "Each probe runs
under a task so a regression fails the test instead of wedging the suite." Four of the new tests
called JustBash.exec/2 directly, so neither statement was true for them:

• a range is refused before it is built, not after — mutation-critical
• a product of ranges each inside the bound is still counted — mutation-critical
• an expansion that fits inside the budget is untouched
• a word list right at the bound is allowed

CI still went red without the wrapper, so this was never a correctness hole. It was the wrong
instrument: on a PR whose subject is "always terminates", the guard for the exact regression took
a 60 s ExUnit timeout to report it, and a false claim about test safety is worse than no claim.

Before — measured with the brace fix reverted

$ git checkout 16c85b3 -- lib/just_bash/interpreter/expansion/brace.ex
$ mix test test/sandbox_contract_test.exs:599      # "a range is refused before it is built"

  1) test ... a range is refused before it is built, not after
     ** (ExUnit.TimeoutError) test timed out after 60000ms

     stacktrace:
       (just_bash 0.3.0) lib/just_bash/interpreter/expansion/brace.ex:46:
         anonymous fn/5 in JustBash.Interpreter.Expansion.Brace.expand_with_brace/2
       (elixir 1.19.5) lib/enum.ex:2520: Enum."-reduce/3-lists^foldl/2-0-"/3
       (just_bash 0.3.0) lib/just_bash/interpreter/expansion/brace.ex:43: …expand_with_brace/2
       (just_bash 0.3.0) lib/just_bash/interpreter/expansion.ex:123: …expand_word_with_glob/2
       …

Finished in 60.1 seconds
1 test, 1 failure (72 excluded)

$ mix test test/sandbox_contract_test.exs           # whole file, same mutation
Finished in 91.6 seconds
73 tests, 8 failures

The change

bounded_exec/2 keeps its signature and its four existing callers. The timing variant is the one
that owns the task, so the elapsed_us assertion in :599 measures the run itself and not the
yield — the clock is read inside the Task, so both properties hold at once:

defp bounded_exec(bash, script) do
  {_elapsed_us, result} = bounded_exec_timed(bash, script)
  result
end

# The clock is read *inside* the task, so a probe that asserts on elapsed
# time measures the run itself rather than the yield, and still fails at 5s
# instead of wedging the suite.
defp bounded_exec_timed(bash, script) do
  task = Task.async(fn -> :timer.tc(fn -> JustBash.exec(bash, script) end) end)

  case Task.yield(task, 5_000) || Task.shutdown(task, :brutal_kill) do
    {:ok, {elapsed_us, {result, _bash}}} -> {elapsed_us, result}
    nil -> flunk("`#{script}` did not terminate within 5s")
  end
end

The other three swap {result, _bash} = JustBash.exec(bash, …) for result = bounded_exec(bash, …).
Every probe in both a loop inside a single command and an expansion that generates from a short input now runs under a task, so the note and the describe-block comment are both true as written —
verified by grep: no JustBash.exec call remains in either block outside bounded_exec_timed/2.

After — same mutation, same command

$ git checkout 16c85b3 -- lib/just_bash/interpreter/expansion/brace.ex
$ mix test test/sandbox_contract_test.exs:607      # the test moved 8 lines

  1) test ... a range is refused before it is built, not after
     `echo {1..20000000}` did not terminate within 5s
     code: {elapsed_us, result} = bounded_exec_timed(bash, "echo {1..20000000}")
     stacktrace:
       test/sandbox_contract_test.exs:614: (test)

Finished in 5.1 seconds
1 test, 1 failure (72 excluded)

$ mix test test/sandbox_contract_test.exs           # whole file, same mutation
Finished in 36.6 seconds
73 tests, 8 failures

60.1 s → 5.1 s for the probe, and a named diagnostic instead of a stacktrace into a hung reducer.
91.6 s → 36.6 s for the file, with the same 8 failures: the mutation is still caught, just faster.

Unmutated, the file is unchanged:

$ mix test test/sandbox_contract_test.exs
Finished in 1.6 seconds (1.6s async, 0.00s sync)
73 tests, 0 failures

Scope note

describe "a recursive traversal" still calls JustBash.exec/2 under a bare :timer.tc at
:685/:698. That block makes no task claim — its comment says only "Unbounded, these run
300-700 ms on this tree" — and those probes are bounded by construction rather than by a
5 s net, so they are left alone rather than churned in a round-4 diff.

Gates — all five re-run on ddc4ee3

$ mix format --check-formatted
(no output, exit 0)

$ MIX_ENV=test mix compile --force --warnings-as-errors
Compiling 176 files (.ex)
Generated just_bash app                                          (exit 0)

$ mix credo --strict
  Refactoring opportunities
┃ [F] ↘ Avoid `apply/2` and `apply/3` when the number of arguments is known.
┃       test/support/banned_fixture_apply.ex:4:16 #(BannedCallTracer.Fixture.Apply.run)

Analysis took 1.3 seconds (0.08s to load, 1.2s running 68 checks on 230 files)
5159 mods/funs, found 1 refactoring opportunity.

The single finding is the intentional test fixture; identical on main.

$ mix test
Finished in 23.4 seconds (23.1s async, 0.2s sync)
2 doctests, 62 properties, 4825 tests, 0 failures (5 excluded)

$ mix dialyzer
Total errors: 13, Skipped: 13, Unnecessary Skips: 0
done in 0m2.97s
done (passed successfully)

[davydog187]

Final verification

Round 4, independent re-run at ddc4ee3 in a clean worktree (deps/_build copied, mix deps.get, forced recompile). Every number below is from a command I ran here, not carried over from the fix note.

Item

#	Item	Claimed	Verdict
1	Four new tests are not Task-wrapped despite the note saying they are (:599, :614, :636, :644 pre-fix; now :607, :621, :642, :650)	fixed	fixed

Structural check. grep -n "JustBash.exec" over the two termination describes (test/sandbox_contract_test.exs:514-577 and :579-657) returns exactly one hit, line 533, inside bounded_exec_timed/2. All 14 probes across the two describes now go through bounded_exec/2 or bounded_exec_timed/2. Both describe-block comments ("Each probe runs under a task so a regression fails the test instead of hanging/wedging the suite", :518 and :584) are now true as written.

Reproduced the original defect first. With the brace fix reverted and the pre-fix test file:

$ git checkout 16c85b3 -- lib/just_bash/interpreter/expansion/brace.ex
$ git checkout afe0aa8 -- test/sandbox_contract_test.exs
$ mix test test/sandbox_contract_test.exs:599
     ** (ExUnit.TimeoutError) test timed out after 60000ms
     stacktrace:
       (just_bash 0.3.0) lib/just_bash/interpreter/expansion/brace.ex:46: anonymous fn/5 in JustBash.Interpreter.Expansion.Brace.expand_with_brace/2
       ...
Finished in 60.1 seconds
1 test, 1 failure (72 excluded)

$ mix test test/sandbox_contract_test.exs        # whole file, old tests + mutation
Finished in 91.7 seconds (91.7s async, 0.00s sync)
73 tests, 8 failures

After the fix, same mutation:

$ mix test test/sandbox_contract_test.exs:607
     `echo {1..20000000}` did not terminate within 5s
     code: {elapsed_us, result} = bounded_exec_timed(bash, "echo {1..20000000}")
Finished in 5.2 seconds
1 test, 1 failure (72 excluded)

$ mix test test/sandbox_contract_test.exs        # whole file, new tests + mutation
Finished in 36.7 seconds (36.7s async, 0.00s sync)
73 tests, 8 failures

60.1s TimeoutError to a 5.2s flunk; 91.7s to 36.7s; the same 8 failures on both sides. The fix note's 91.6 / 36.6 / 5.1 numbers reproduce within noise.

Unmutated, at HEAD:

$ mix test test/sandbox_contract_test.exs
Finished in 1.6 seconds (1.6s async, 0.00s sync)
73 tests, 0 failures

1.6s at seed 38116, 1.6s at seed 1000, 1.6s at seed 2000 — unchanged from the pre-fix baseline.

Proving the new coverage can fail

The brace mutation exercises :607 (5.2s flunk) and :621 (fails exit_code == 1, left 0). :642 and :650 are positive controls and correctly stay green under it, so I mutated the harness itself to show the wrapper is live on the bounded_exec/2 path they use — swapped :650's body for a genuinely non-terminating script under limits: false:

$ mix test test/sandbox_contract_test.exs:650
     `awk 'BEGIN{while(1){x=x+1}}'` did not terminate within 5s
     stacktrace:
       test/sandbox_contract_test.exs:525: JustBash.SandboxContractTest.bounded_exec/2
Finished in 5.1 seconds
1 test, 1 failure (72 excluded)

Reverted afterwards. A truly hanging script inside bounded_exec/2 costs 5.1s, not 60s.

Regression sweep

Refute-by-default. What I checked, including what came back clean:

• elapsed_us margin at :607. The assertion is < 200_000. Measured five runs through the exact new Task.async + :timer.tc shape: 6371, 32, 30, 35, 23 microseconds. Three orders of magnitude of headroom; moving the clock inside the Task did not erode it. Clean.
• Other timing assertions in the file. :243 (exec!/2 wall clock), :448 (max_wall_ms spin), :684-692 (traversal elapsed_us < 1_000_000) are untouched by this diff and pass at every seed tried. Clean.
• bounded_exec/2's pre-existing callers. There are seven call sites that predate this commit (:542 :549 :556 :563 :572 :598 :636), not four as the fix note says. All seven still pass and none changed shape — bounded_exec/2 is bounded_exec_timed/2 with the timing dropped. The miscount is in the note's prose only; the code is right. Clean, with a note nit below.
• Un-wrapped probes elsewhere in the file. The "a recursive traversal" describe (:659, added on this branch in 554d8d9) still calls JustBash.exec/2 directly under a raw :timer.tc, seven tests. Its comment makes no task-wrapping claim, so nothing there is false — and it is not the same hazard: reverting its fix (git checkout 554d8d9~1 -- lib/just_bash/commands/{grep,du,tree,cp}.ex) makes it fail on exit_code, not hang, and the whole describe runs in 2.0s with 4 failures. No wedge risk. Clean, deliberately left alone.
• Oracle check on the two positive controls, against GNU bash 3.2.57 on this machine:

  $ bash -c 'echo {1..5} {a..e} {x,y}{1..3}'   -> 1 2 3 4 5 a b c d e x1 x2 x3 y1 y2 y3
  $ bash -c 'echo {1..10}{1..10} | wc -w'      -> 100
  $ bash -c 'echo {1..50}{1..50} | wc -w'      -> 2500
  

  Matches :644/:652 exactly, and confirms :621's 2500-word product really does exceed the 100-word cap. Clean.
• Full-suite runtime. 22.8s, in line with prior rounds. No other file's timing moved.

Gates

All five run here at ddc4ee3 with a clean tree.

$ mix format --check-formatted
(no output, exit 0)

$ mix compile --warnings-as-errors --force
Compiling 168 files (.ex)
Generated just_bash app

$ mix credo --strict
| [F] -> Avoid `apply/2` and `apply/3` when the number of arguments is known.
|       test/support/banned_fixture_apply.ex:4:16 #(BannedCallTracer.Fixture.Apply.run)
5159 mods/funs, found 1 refactoring opportunity.

$ mix test
Finished in 22.8 seconds (22.6s async, 0.2s sync)
2 doctests, 62 properties, 4825 tests, 0 failures (5 excluded)

$ mix dialyzer
Total errors: 13, Skipped: 13, Unnecessary Skips: 0
done (passed successfully)

Credo's single finding is the intentional test fixture that CLAUDE.md calls out; it is not introduced by this branch.

Remaining nit (non-blocking, prose only)

The round-4 fix note says "its four pre-existing callers are untouched". bounded_exec/2 had seven pre-existing call sites, not four. Nothing in the repo says this — the describe comments and the code are accurate — but on a PR whose recurring failure mode is notes that overstate, worth correcting in the record.

Verdict: all clear. The item is genuinely fixed, the coverage is proven able to fail, and the sweep turned up nothing.