fix(builtins): surface readonly errors from unset/declare/export

crates/bashkit/src/builtins/export.rs

@@ -27,6 +27,9 @@ impl Builtin for Export {
             return Ok(ExecResult::ok(output));
         }
 
+        let mut stderr = String::new();
+        let mut exit_code = 0;
+
         for arg in ctx.args {
             // Handle NAME=VALUE format
             if let Some(eq_pos) = arg.find('=') {
@@ -43,8 +46,11 @@ impl Builtin for Export {
                 if is_internal_variable(name) {
                     continue;
                 }
-                // THREAT[TM-INJ-021]: Refuse to overwrite readonly variables
+                // THREAT[TM-INJ-021]: Refuse to overwrite readonly variables and
+                // surface the error so callers cannot mistake a silent skip for success.
                 if ctx.variables.contains_key(&format!("_READONLY_{}", name)) {
+                    stderr.push_str(&format!("bash: export: {name}: readonly variable\n"));
+                    exit_code = 1;
                     continue;
                 }
                 ctx.variables.insert(name.to_string(), value.to_string());
@@ -54,6 +60,10 @@ impl Builtin for Export {
             }
         }
 
-        Ok(ExecResult::ok(String::new()))
+        Ok(ExecResult {
+            stderr,
+            exit_code,
+            ..Default::default()
+        })
     }
 }

crates/bashkit/src/interpreter/mod.rs

@@ -5911,6 +5911,9 @@ impl Interpreter {
             }
         }
 
+        let mut stderr = String::new();
+        let mut exit_code: i32 = 0;
+
         for arg in &var_args {
             if unset_function {
                 self.functions.remove(arg.as_str());
@@ -5938,13 +5941,22 @@ impl Interpreter {
                 let resolved = self.resolve_nameref(arg).to_string();
                 // THREAT[TM-INJ-009]: Block unset of internal marker variables
                 if is_internal_variable(&resolved) {
+                    stderr.push_str(&format!(
+                        "bash: unset: {resolved}: cannot unset: readonly variable\n"
+                    ));
+                    exit_code = 1;
                     continue;
                 }
-                // THREAT[TM-INJ-019]: Refuse to unset readonly variables
+                // THREAT[TM-INJ-019]: Refuse to unset readonly variables and surface
+                // the error so callers cannot mistake a silent skip for success.
                 if self
                     .variables
                     .contains_key(&format!("_READONLY_{}", resolved))
                 {
+                    stderr.push_str(&format!(
+                        "bash: unset: {resolved}: cannot unset: readonly variable\n"
+                    ));
+                    exit_code = 1;
                     continue;
                 }
                 self.variables.remove(&resolved);
@@ -5956,7 +5968,11 @@ impl Interpreter {
                 }
             }
         }
-        let result = ExecResult::ok(String::new());
+        let result = ExecResult {
+            stderr,
+            exit_code,
+            ..Default::default()
+        };
         self.apply_redirections(result, redirects).await
     }
 
@@ -6429,6 +6445,9 @@ impl Interpreter {
         // Reconstruct compound assignments: declare -A m=([a]="1" [b]="2")
         let merged_names = merge_compound_assignments(&names);
 
+        let mut declare_stderr = String::new();
+        let mut declare_exit_code: i32 = 0;
+
         // Set variables
         for name in &merged_names {
             if let Some(eq_pos) = name.find('=') {
@@ -6440,11 +6459,15 @@ impl Interpreter {
                     continue;
                 }
 
-                // THREAT[TM-INJ-020]: Refuse to overwrite readonly variables
+                // THREAT[TM-INJ-020]: Refuse to overwrite readonly variables and
+                // surface the error so callers cannot mistake a silent skip for success.
                 if self
                     .variables
                     .contains_key(&format!("_READONLY_{}", var_name))
                 {
+                    declare_stderr
+                        .push_str(&format!("bash: declare: {var_name}: readonly variable\n"));
+                    declare_exit_code = 1;
                     continue;
                 }
 
@@ -6594,7 +6617,11 @@ impl Interpreter {
             }
         }
 
-        let mut result = ExecResult::ok(String::new());
+        let mut result = ExecResult {
+            stderr: declare_stderr,
+            exit_code: declare_exit_code,
+            ..Default::default()
+        };
         result = self.apply_redirections(result, redirects).await?;
         Ok(result)
     }

crates/bashkit/tests/blackbox_security_tests.rs

@@ -339,6 +339,122 @@ mod finding_readonly_bypass {
         );
     }
 
+    /// TM-INJ-019: unset on a readonly variable reports the error and exits 1.
+    /// Value preservation is already covered above; this guards against silent skips.
+    #[tokio::test]
+    async fn unset_readonly_reports_error_and_exit_1() {
+        let mut bash = tight_bash();
+        let result = bash
+            .exec(
+                r#"
+                readonly LOCKED=v
+                unset LOCKED
+                echo "exit=$?"
+                "#,
+            )
+            .await
+            .unwrap();
+        assert!(
+            result.stderr.contains("LOCKED") && result.stderr.contains("readonly"),
+            "expected stderr to mention readonly, got: {:?}",
+            result.stderr
+        );
+        assert!(
+            result.stdout.contains("exit=1"),
+            "expected unset to exit 1, got: {:?}",
+            result.stdout
+        );
+    }
+
+    /// TM-INJ-020: declare on a readonly variable reports the error and exits 1.
+    #[tokio::test]
+    async fn declare_readonly_reports_error_and_exit_1() {
+        let mut bash = tight_bash();
+        let result = bash
+            .exec(
+                r#"
+                readonly LOCKED=original
+                declare LOCKED=overwritten
+                echo "exit=$?"
+                echo "value=$LOCKED"
+                "#,
+            )
+            .await
+            .unwrap();
+        assert!(
+            result.stderr.contains("declare")
+                && result.stderr.contains("LOCKED")
+                && result.stderr.contains("readonly"),
+            "expected stderr to mention declare/readonly, got: {:?}",
+            result.stderr
+        );
+        assert!(
+            result.stdout.contains("exit=1"),
+            "expected declare to exit 1, got: {:?}",
+            result.stdout
+        );
+        assert!(
+            result.stdout.contains("value=original"),
+            "value must be preserved, got: {:?}",
+            result.stdout
+        );
+    }
+
+    /// TM-INJ-021: export on a readonly variable reports the error and exits 1.
+    #[tokio::test]
+    async fn export_readonly_reports_error_and_exit_1() {
+        let mut bash = tight_bash();
+        let result = bash
+            .exec(
+                r#"
+                readonly LOCKED=original
+                export LOCKED=overwritten
+                echo "exit=$?"
+                echo "value=$LOCKED"
+                "#,
+            )
+            .await
+            .unwrap();
+        assert!(
+            result.stderr.contains("export")
+                && result.stderr.contains("LOCKED")
+                && result.stderr.contains("readonly"),
+            "expected stderr to mention export/readonly, got: {:?}",
+            result.stderr
+        );
+        assert!(
+            result.stdout.contains("exit=1"),
+            "expected export to exit 1, got: {:?}",
+            result.stdout
+        );
+        assert!(
+            result.stdout.contains("value=original"),
+            "value must be preserved, got: {:?}",
+            result.stdout
+        );
+    }
+
+    /// declare/export must keep processing remaining args even after a readonly hit.
+    #[tokio::test]
+    async fn declare_continues_after_readonly_error() {
+        let mut bash = tight_bash();
+        let result = bash
+            .exec(
+                r#"
+                readonly LOCKED=original
+                declare LOCKED=skip OTHER=ok
+                echo "exit=$?"
+                echo "locked=$LOCKED"
+                echo "other=$OTHER"
+                "#,
+            )
+            .await
+            .unwrap();
+        assert!(result.stdout.contains("locked=original"));
+        assert!(result.stdout.contains("other=ok"));
+        assert!(result.stdout.contains("exit=1"));
+    }
+
     /// Non-finding: readonly via local in function is bash-compatible shadowing.
     #[tokio::test]
     async fn local_shadows_readonly_in_function() {

specs/threat-model.md

@@ -536,9 +536,9 @@ Bash::builder()
 
 | TM-INJ-011 | Cyclic nameref silent resolution | Cyclic namerefs (a→b→a) silently resolve after 10 iterations instead of erroring | — | **OPEN** |
 | TM-INJ-018 | `dotenv` internal variable prefix injection | `.env` file with `_NAMEREF_x=target` sets internal interpreter variables via `ctx.variables.insert()` | — | **OPEN** |
-| TM-INJ-019 | `unset` removes readonly variables | `readonly X=v; unset X` removes the variable despite readonly attribute | — | **OPEN** |
-| TM-INJ-020 | `declare` overwrites readonly variables | `readonly X=v; declare X=new` overwrites without error | — | **OPEN** |
-| TM-INJ-021 | `export` overwrites readonly variables | `readonly X=v; export X=new` overwrites without error | — | **OPEN** |
+| TM-INJ-019 | `unset` removes readonly variables | `readonly X=v; unset X` removes the variable despite readonly attribute | `execute_unset_builtin` and `Unset` builtin both check `_READONLY_<name>` markers, emit `bash: unset: <name>: cannot unset: readonly variable`, and return exit 1 | **MITIGATED** |
+| TM-INJ-020 | `declare` overwrites readonly variables | `readonly X=v; declare X=new` overwrites without error | `declare` assignment path checks `_READONLY_<name>`, emits `bash: declare: <name>: readonly variable`, returns exit 1 | **MITIGATED** |
+| TM-INJ-021 | `export` overwrites readonly variables | `readonly X=v; export X=new` overwrites without error | `export NAME=VALUE` checks `_READONLY_<name>`, emits `bash: export: <name>: readonly variable`, returns exit 1 | **MITIGATED** |
 
 **TM-INJ-011**: `interpreter/mod.rs:7547-7560` — cyclic namerefs silently resolve to whatever
 variable is current after 10 iterations. Real bash errors with `circular name reference`. Can
@@ -1299,9 +1299,9 @@ This section maps former vulnerability IDs to the new threat ID scheme and track
 | TM-DOS-056 | `source` self-recursion stack overflow | Process crash (SIGABRT) via script that sources itself | Track source depth like function depth; apply `max_function_depth` limit |
 | TM-DOS-057 | `sleep` bypasses execution timeout | CPU/time exhaustion; `sleep`, subshell sleep, pipeline sleep, background sleep, `timeout` builtin all ignore `ExecutionLimits::timeout` | Implement timeout as tokio::time::timeout wrapper around exec(), not cooperative check |
 | TM-DOS-058 | Single-builtin unbounded output | OOM via `seq 1 1000000` producing 1M lines despite command limit of 50 | Add `max_stdout_bytes` / `max_stderr_bytes` to ExecutionLimits (see #648) |
-| TM-INJ-019 | `unset` removes readonly variables | Integrity bypass — readonly protection defeated | Check readonly attribute in unset before removal |
-| TM-INJ-020 | `declare` overwrites readonly variables | Integrity bypass — `declare X=new` overwrites `readonly X=old` | Check readonly attribute in declare assignment path |
-| TM-INJ-021 | `export` overwrites readonly variables | Integrity bypass — `export X=new` overwrites `readonly X=old` | Check readonly attribute in export assignment path |
+| ~~TM-INJ-019~~ | ~~`unset` removes readonly variables~~ | ~~Integrity bypass — readonly protection defeated~~ | ~~Check readonly attribute in unset before removal~~ (**FIXED**) |
+| ~~TM-INJ-020~~ | ~~`declare` overwrites readonly variables~~ | ~~Integrity bypass — `declare X=new` overwrites `readonly X=old`~~ | ~~Check readonly attribute in declare assignment path~~ (**FIXED**) |
+| ~~TM-INJ-021~~ | ~~`export` overwrites readonly variables~~ | ~~Integrity bypass — `export X=new` overwrites `readonly X=old`~~ | ~~Check readonly attribute in export assignment path~~ (**FIXED**) |
 | 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()` |
 | 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()` |
 | 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**) |