Skip to content

Commit 838b87b

Browse files
authored
fix(interpreter): preserve shopt options across exec() calls (#1131)
## Summary - `reset_transient_state()` was wiping ALL `SHOPT_*` variables between `exec()` calls, including `shopt` options like `expand_aliases`. This made alias expansion impossible when `shopt` and `alias` commands were in separate `exec()` calls (the JS/Python API pattern). - Now only `set` options (`SHOPT_e`, `SHOPT_x`, etc.) are reset between `exec()` calls for safety (TM-ISO-023), while `shopt` options (`expand_aliases`, `extglob`, `dotglob`, etc.) persist as session configuration. ## Test plan - [x] New test `alias_expansion_persists_across_exec_calls` verifies aliases work across separate `exec()` calls - [x] Existing `set_e_does_not_leak_between_exec` confirms `set -e` is still properly reset - [x] Existing `threat_isolation_alias_isolation` confirms cross-session alias isolation - [x] Snapshot roundtrip tests updated and passing - [x] All 167 threat model + security + snapshot tests pass Closes #1130
1 parent e03199f commit 838b87b

4 files changed

Lines changed: 45 additions & 7 deletions

File tree

crates/bashkit/src/interpreter/mod.rs

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -877,13 +877,33 @@ impl Interpreter {
877877
&self.limits
878878
}
879879

880+
/// `set -o` option variable names (SHOPT_e, SHOPT_x, etc.) that are
881+
/// transient and must be reset between exec() calls (TM-ISO-023).
882+
/// `shopt` options (SHOPT_expand_aliases, SHOPT_extglob, etc.) are
883+
/// persistent session configuration and are NOT reset.
884+
const SET_OPTION_VARS: &'static [&'static str] = &[
885+
"SHOPT_a",
886+
"SHOPT_e",
887+
"SHOPT_f",
888+
"SHOPT_n",
889+
"SHOPT_u",
890+
"SHOPT_v",
891+
"SHOPT_x",
892+
"SHOPT_C",
893+
"SHOPT_pipefail",
894+
];
895+
880896
/// THREAT[TM-ISO-005/006/007]: Reset per-exec transient state.
881897
/// Called by Bash::exec() before each top-level execution to prevent
882-
/// traps, exit code, and shell options from leaking across calls.
898+
/// traps, exit code, and `set` options from leaking across calls.
899+
/// `shopt` options (expand_aliases, extglob, etc.) are intentionally
900+
/// preserved — they are persistent session configuration.
883901
pub fn reset_transient_state(&mut self) {
884902
self.traps.clear();
885903
self.last_exit_code = 0;
886-
self.variables.retain(|k, _| !k.starts_with("SHOPT_"));
904+
for var in Self::SET_OPTION_VARS {
905+
self.variables.remove(*var);
906+
}
887907
}
888908

889909
/// Set an environment variable.

crates/bashkit/tests/snapshot_tests.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -240,9 +240,9 @@ async fn shell_options_survive_snapshot_roundtrip() {
240240
let mut bash2 = Bash::new();
241241
bash2.restore_shell_state(&restored);
242242

243-
// exec() calls reset_transient_state which clears SHOPT_* vars,
244-
// so we verify the state was restored correctly by inspecting it
245-
// before the next exec() call.
243+
// `set` options (SHOPT_e, SHOPT_pipefail) are transient — they are
244+
// cleared by reset_transient_state() between exec() calls (TM-ISO-023).
245+
// Verify the snapshot restored them correctly before the next exec().
246246
let state2 = bash2.shell_state();
247247
assert_eq!(
248248
state2.variables.get("SHOPT_e").map(|s| s.as_str()),

crates/bashkit/tests/threat_model_tests.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,24 @@ mod session_isolation {
590590
assert!(!result.stdout.contains("LEAKED"));
591591
}
592592

593+
/// Alias expansion must work across separate exec() calls (issue #1130).
594+
/// shopt -s expand_aliases set in one exec() must persist to the next.
595+
#[tokio::test]
596+
async fn alias_expansion_persists_across_exec_calls() {
597+
let mut bash = Bash::new();
598+
599+
bash.exec("shopt -s expand_aliases").await.unwrap();
600+
bash.exec("alias ll='echo alias_worked'").await.unwrap();
601+
602+
let result = bash.exec("ll").await.unwrap();
603+
assert_eq!(
604+
result.exit_code, 0,
605+
"alias should resolve: {}",
606+
result.stderr
607+
);
608+
assert_eq!(result.stdout.trim(), "alias_worked");
609+
}
610+
593611
/// TM-ISO-020: Trap handlers in one session must not fire in another
594612
#[tokio::test]
595613
async fn threat_isolation_trap_isolation() {

specs/006-threat-model.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -804,7 +804,7 @@ Only exact domain matches are allowed (TM-NET-017).
804804
| TM-ISO-006 | No per-instance variable/memory budget | Unbounded `HashMap` growth in variables, arrays, functions exhausts process memory || **OPEN** |
805805
| TM-ISO-021 | EXIT trap leaks across `exec()` calls | EXIT trap set in one `exec()` fires in subsequent `exec()` calls on same Bash instance || **OPEN** |
806806
| TM-ISO-022 | `$?` leaks across `exec()` calls | Exit code from one `exec()` visible as `$?` in next `exec()` instead of resetting to 0 || **OPEN** |
807-
| TM-ISO-023 | `set -e` leaks across `exec()` calls | Shell options (`set -e`, etc.) persist across `exec()` calls, causing unexpected abort behavior | | **OPEN** |
807+
| TM-ISO-023 | `set -e` leaks across `exec()` calls | `set` options (`-e`, `-x`, etc.) persist across `exec()` calls, causing unexpected abort behavior | `reset_transient_state()` clears `SET_OPTION_VARS` | **FIXED** |
808808

809809
**TM-ISO-004**: Fixed. The jq builtin now injects shell variables via a custom jaq context variable
810810
(`$__bashkit_env__`) and overrides the `env` filter to read from it instead of `std::env`.
@@ -1224,7 +1224,7 @@ This section maps former vulnerability IDs to the new threat ID scheme and track
12241224
| TM-INJ-021 | `export` overwrites readonly variables | Integrity bypass — `export X=new` overwrites `readonly X=old` | Check readonly attribute in export assignment path |
12251225
| TM-ISO-021 | EXIT trap leaks across `exec()` calls | Cross-invocation interference — trap from exec N fires in exec N+1 | Reset traps in `reset_for_execution()` |
12261226
| TM-ISO-022 | `$?` leaks across `exec()` calls | State pollution — exit code from previous exec visible to next exec | Reset `last_exit_code` in `reset_for_execution()` |
1227-
| TM-ISO-023 | `set -e` leaks across `exec()` calls | Unexpected abort — shell options from previous exec affect next exec | Reset shell options in `reset_for_execution()` |
1227+
| TM-ISO-023 | `set -e` leaks across `exec()` calls | Unexpected abort — `set` options from previous exec affect next exec | `SET_OPTION_VARS` cleared in `reset_transient_state()` (**FIXED**) |
12281228
| TM-ISO-024 | `$?` leaks into VFS subprocess | Parent `last_exit_code` visible inside VFS script subprocess, causing false `set -e` failures | Reset `last_exit_code = 0` and `nounset_error = None` in `execute_script_content` subprocess isolation |
12291229
| TM-INT-007 | `/dev/urandom` empty with `head -c` | Weak randomness — `head -c 16 /dev/urandom` returns empty string | Fix virtual device pipe handling in head builtin |
12301230
| TM-DOS-044 | Nested `$()` stack overflow (regression) | Process crash (SIGABRT) at depth ~50 despite #492 fix | Interpreter execution path may need separate depth tracking from lexer fix |

0 commit comments

Comments
 (0)