diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index 3ecf74ad..40203486 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -289,17 +289,6 @@ struct CallFrame { positional: Vec, } -/// Shell options that can be set via `set -o` or `set -x` -#[derive(Debug, Clone, Default)] -pub struct ShellOptions { - /// Exit immediately if a command exits with non-zero status (set -e) - pub errexit: bool, - /// Print commands before execution (set -x) - pub xtrace: bool, - /// Return rightmost non-zero exit code from pipeline (set -o pipefail) - pub pipefail: bool, -} - /// A snapshot of shell state (variables, env, cwd, options). /// /// Captures the serializable portions of the interpreter state. @@ -323,12 +312,6 @@ pub struct ShellState { pub aliases: HashMap, /// Trap handlers pub traps: HashMap, - /// Shell options - pub errexit: bool, - /// Shell options - pub xtrace: bool, - /// Shell options - pub pipefail: bool, } /// Interpreter state. @@ -362,8 +345,6 @@ pub struct Interpreter { counters: ExecutionCounters, /// Job table for background execution (shared for wait builtin access) jobs: SharedJobTable, - /// Shell options (set -e, set -x, etc.) - options: ShellOptions, /// Current line number for $LINENO current_line: usize, /// HTTP client for network builtins (curl, wget) @@ -686,7 +667,6 @@ impl Interpreter { trace: crate::trace::TraceCollector::default(), counters: ExecutionCounters::new(), jobs: jobs::new_shared_job_table(), - options: ShellOptions::default(), current_line: 1, #[cfg(feature = "http_client")] http_client: None, @@ -726,38 +706,20 @@ impl Interpreter { } } - /// Get mutable access to shell options (for builtins like `set`) - #[allow(dead_code)] - pub fn options_mut(&mut self) -> &mut ShellOptions { - &mut self.options - } - - /// Get shell options - #[allow(dead_code)] - pub fn options(&self) -> &ShellOptions { - &self.options - } - - /// Check if errexit (set -e) is enabled - /// This checks both the options struct and the SHOPT_e variable - /// (the `set` builtin stores options in SHOPT_e) + /// Check if errexit (set -e) is enabled. fn is_errexit_enabled(&self) -> bool { - self.options.errexit - || self - .variables - .get("SHOPT_e") - .map(|v| v == "1") - .unwrap_or(false) + self.variables + .get("SHOPT_e") + .map(|v| v == "1") + .unwrap_or(false) } - /// Check if xtrace (set -x) is enabled + /// Check if xtrace (set -x) is enabled. fn is_xtrace_enabled(&self) -> bool { - self.options.xtrace - || self - .variables - .get("SHOPT_x") - .map(|v| v == "1") - .unwrap_or(false) + self.variables + .get("SHOPT_x") + .map(|v| v == "1") + .unwrap_or(false) } /// Set execution limits. @@ -791,7 +753,6 @@ impl Interpreter { pub fn reset_transient_state(&mut self) { self.traps.clear(); self.last_exit_code = 0; - self.options = ShellOptions::default(); self.variables.retain(|k, _| !k.starts_with("SHOPT_")); } @@ -919,9 +880,6 @@ impl Interpreter { last_exit_code: self.last_exit_code, aliases: self.aliases.clone(), traps: self.traps.clone(), - errexit: self.options.errexit, - xtrace: self.options.xtrace, - pipefail: self.options.pipefail, } } @@ -935,9 +893,6 @@ impl Interpreter { self.last_exit_code = state.last_exit_code; self.aliases = state.aliases.clone(); self.traps = state.traps.clone(); - self.options.errexit = state.errexit; - self.options.xtrace = state.xtrace; - self.options.pipefail = state.pipefail; } /// Set an output callback for streaming output during execution. @@ -1297,7 +1252,6 @@ impl Interpreter { let saved_traps = self.traps.clone(); let saved_call_stack = self.call_stack.clone(); let saved_exit = self.last_exit_code; - let saved_options = self.options.clone(); let saved_aliases = self.aliases.clone(); let saved_coproc = self.coproc_buffers.clone(); @@ -1341,7 +1295,6 @@ impl Interpreter { self.traps = saved_traps; self.call_stack = saved_call_stack; self.last_exit_code = saved_exit; - self.options = saved_options; self.aliases = saved_aliases; self.coproc_buffers = saved_coproc; result @@ -8400,13 +8353,6 @@ impl Interpreter { flags.push(opt); } } - // Also check options struct - if self.options.errexit && !flags.contains('e') { - flags.push('e'); - } - if self.options.xtrace && !flags.contains('x') { - flags.push('x'); - } return flags; } "RANDOM" => { @@ -8544,12 +8490,10 @@ impl Interpreter { /// Check if pipefail (`set -o pipefail`) is active. fn is_pipefail(&self) -> bool { - self.options.pipefail - || self - .variables - .get("SHOPT_pipefail") - .map(|v| v == "1") - .unwrap_or(false) + self.variables + .get("SHOPT_pipefail") + .map(|v| v == "1") + .unwrap_or(false) } /// Run ERR trap if registered. Appends trap output to stdout/stderr. diff --git a/crates/bashkit/tests/snapshot_tests.rs b/crates/bashkit/tests/snapshot_tests.rs index 342e6ef4..47442031 100644 --- a/crates/bashkit/tests/snapshot_tests.rs +++ b/crates/bashkit/tests/snapshot_tests.rs @@ -200,3 +200,58 @@ async fn combined_snapshot_restore_multi_turn() { .unwrap(); assert_eq!(result.stdout, "gone\n"); } + +// ==================== Shell options snapshot/restore ==================== + +#[tokio::test] +async fn shell_options_survive_snapshot_roundtrip() { + let mut bash = Bash::new(); + + // Set options via `set` builtin. Options live in SHOPT_* variables which + // are included in the variables snapshot (no more split brain with a + // separate ShellOptions struct). + bash.exec("set -e; set -o pipefail").await.unwrap(); + + let state = bash.shell_state(); + + // Options should be present in snapshotted variables + assert_eq!( + state.variables.get("SHOPT_e").map(|s| s.as_str()), + Some("1") + ); + assert_eq!( + state.variables.get("SHOPT_pipefail").map(|s| s.as_str()), + Some("1") + ); + + // Serialize → deserialize to prove options survive JSON roundtrip + let json = serde_json::to_string(&state).unwrap(); + let restored: bashkit::ShellState = serde_json::from_str(&json).unwrap(); + assert_eq!( + restored.variables.get("SHOPT_e").map(|s| s.as_str()), + Some("1") + ); + assert_eq!( + restored.variables.get("SHOPT_pipefail").map(|s| s.as_str()), + Some("1") + ); + + // Restore into a fresh interpreter and verify options are active + let mut bash2 = Bash::new(); + bash2.restore_shell_state(&restored); + + // exec() calls reset_transient_state which clears SHOPT_* vars, + // so we verify the state was restored correctly by inspecting it + // before the next exec() call. + let state2 = bash2.shell_state(); + assert_eq!( + state2.variables.get("SHOPT_e").map(|s| s.as_str()), + Some("1"), + "errexit should survive snapshot/restore roundtrip" + ); + assert_eq!( + state2.variables.get("SHOPT_pipefail").map(|s| s.as_str()), + Some("1"), + "pipefail should survive snapshot/restore roundtrip" + ); +} diff --git a/specs/006-threat-model.md b/specs/006-threat-model.md index 324bf29c..8c2a3b53 100644 --- a/specs/006-threat-model.md +++ b/specs/006-threat-model.md @@ -768,7 +768,7 @@ Only exact domain matches are allowed (TM-NET-017). | TM-ISO-004 | Cross-session env pollution via jq | `std::env::set_var()` in jq | Custom jaq global variable (`$__bashkit_env__`) | **MITIGATED** | | TM-ISO-007 | Alias leakage | Aliases defined in session A visible in session B | Per-instance alias HashMap | **MITIGATED** | | TM-ISO-008 | Trap handler leakage | Trap from session A fires in session B | Per-instance trap HashMap | **MITIGATED** | -| TM-ISO-009 | Shell option leakage | `set -e` in session A affects session B | Per-instance ShellOptions | **MITIGATED** | +| TM-ISO-009 | Shell option leakage | `set -e` in session A affects session B | Per-instance SHOPT_* variables | **MITIGATED** | | TM-ISO-010 | Exported env var leakage | `export` in session A visible in session B | Per-instance env HashMap | **MITIGATED** | | TM-ISO-011 | Array leakage | Indexed/associative arrays cross sessions | Per-instance array HashMaps | **MITIGATED** | | TM-ISO-012 | Working directory leakage | `cd` in session A changes session B's cwd | Per-instance `cwd: PathBuf` | **MITIGATED** |