fix(interpreter): terminate command-resolution diagnostics with a newline

crates/bashkit/src/interpreter/mod.rs

@@ -592,12 +592,17 @@ fn unavailable_command_hint(name: &str) -> Option<&'static str> {
 }
 
 /// Build a "command not found" error with optional suggestions.
+///
+/// The message is newline-terminated: it goes straight into stderr, and real
+/// bash ends every diagnostic with a newline. Without it consecutive failures
+/// run together on one line (`...command not foundbash: next: ...`).
 fn command_not_found_message(name: &str, known_commands: &[&str]) -> String {
     let mut msg = format!("bash: {}: command not found", name);
 
     // Check for unavailable command hints first
     if let Some(hint) = unavailable_command_hint(name) {
         msg.push_str(&format!(". {}", hint));
+        msg.push('\n');
         return msg;
     }
 
@@ -626,6 +631,7 @@ fn command_not_found_message(name: &str, known_commands: &[&str]) -> String {
         msg.push_str(&format!(". Did you mean: {}?", names.join(", ")));
     }
 
+    msg.push('\n');
     msg
 }
 
@@ -6531,7 +6537,7 @@ impl Interpreter {
             && matches!(name, "exec" | "bash" | "sh" | "source" | ".")
         {
             return Some(Ok(ExecResult::err(
-                format!("bash: {}: command not found", name),
+                format!("bash: {}: command not found\n", name),
                 127,
             )));
         }
@@ -6616,7 +6622,7 @@ impl Interpreter {
             if name.contains('/') {
                 if self.shell_profile.is_logic_only() {
                     return Ok(ExecResult::err(
-                        format!("bash: {}: command not found", name),
+                        format!("bash: {}: command not found\n", name),
                         127,
                     ));
                 }
@@ -6716,7 +6722,7 @@ impl Interpreter {
             Ok(m) => m,
             Err(_) => {
                 return Ok(ExecResult::err(
-                    format!("bash: {}: No such file or directory", name),
+                    format!("bash: {}: No such file or directory\n", name),
                     127,
                 ));
             }
@@ -6725,15 +6731,15 @@ impl Interpreter {
         // Directory check
         if meta.file_type.is_dir() {
             return Ok(ExecResult::err(
-                format!("bash: {}: Is a directory", name),
+                format!("bash: {}: Is a directory\n", name),
                 126,
             ));
         }
 
         // Execute permission check
         if meta.mode & 0o111 == 0 {
             return Ok(ExecResult::err(
-                format!("bash: {}: Permission denied", name),
+                format!("bash: {}: Permission denied\n", name),
                 126,
             ));
         }
@@ -6743,7 +6749,7 @@ impl Interpreter {
             Ok(c) => decode_file_bytes_for_path(&path, &c),
             Err(_) => {
                 return Ok(ExecResult::err(
-                    format!("bash: {}: No such file or directory", name),
+                    format!("bash: {}: No such file or directory\n", name),
                     127,
                 ));
             }
@@ -12176,7 +12182,7 @@ echo "count=$COUNT"
         let msg = command_not_found_message("grpe", &["type", "true", "tree", "grep"]);
         assert_eq!(
             msg,
-            "bash: grpe: command not found. Did you mean: grep, tree, true?"
+            "bash: grpe: command not found. Did you mean: grep, tree, true?\n"
         );
     }
 

crates/bashkit/src/interpreter/redirection.rs

@@ -302,7 +302,9 @@ impl Interpreter {
 
         for redirect in redirects {
             if word_has_process_substitution(&redirect.target) {
-                return Some("bash: process substitution disabled in logic-only shell".to_string());
+                return Some(
+                    "bash: process substitution disabled in logic-only shell\n".to_string(),
+                );
             }
 
             if matches!(

crates/bashkit/tests/integration/glob_fuzz_scaffold_tests.rs

@@ -200,3 +200,107 @@ async fn hex_dumpers_report_missing_operand_like_real_od() {
         );
     }
 }
+
+/// Regression: fuzz run 229 (`crash-bd17afb20a1f27f7c30dc099a77b76e7b26f13bb`,
+/// bytes `[10, 36, 114, 10, 126, 10, 45, 84, 111, 107, 58, 10, 126, 58]`
+/// = `\n$r\n~\n-Tok:\n~:`).
+///
+/// Three command-resolution diagnostics in a row. None of them terminated its
+/// line, so all three arrived as a single run-on line
+/// `bash: /home/sandbox: Is a directorybash: -Tok:: command not found...`.
+/// The echo filter matches a diagnostic per line, and that line opened with a
+/// recognized template but ended with a suggestion suffix, so nothing was
+/// stripped and the `-Tok:` echo (rendered `-Tok::` by the `: ` separator)
+/// tripped the banned parser-token shape `Tok::`.
+#[tokio::test]
+async fn glob_fuzz_crash_unterminated_command_diagnostics() {
+    let input = "\n$r\n~\n-Tok:\n~:";
+    // The pre-filter cannot catch this one: the input holds `-Tok:` with a
+    // single colon, and the second colon comes from the template's own
+    // `: ` separator. The echo filter is the defense that has to hold.
+    assert!(!bashkit::testing::input_echo_would_trip(input));
+
+    let mut bash = fuzz_bash();
+    for script in [
+        format!("ls /tmp/{}", input),
+        format!(
+            "case \"test.txt\" in {}) echo match;; *) echo no;; esac",
+            input
+        ),
+        format!("if [[ \"hello.world\" == {} ]]; then echo y; fi", input),
+    ] {
+        fuzz_exec(
+            &mut bash,
+            &script,
+            "glob_fuzz_crash_unterminated_command_diagnostics",
+            &[],
+        )
+        .await;
+    }
+}
+
+/// Each command-resolution diagnostic terminates its own line, the way real
+/// bash writes them. Verified against GNU bash 5.2.21:
+///
+///     $ bash -c $'~\n-Tok:\nnotacmd' 2>&1 | cat -A
+///     bash: line 1: /root: Is a directory$
+///     bash: line 2: -Tok:: command not found$
+///     bash: line 3: notacmd: command not found$
+#[tokio::test]
+async fn consecutive_command_failures_are_separate_lines() {
+    let mut bash = fuzz_bash();
+    let result = bash.exec("nosuchcmd\nalsomissing").await.unwrap();
+    assert_eq!(
+        result.stderr.to_string(),
+        "bash: nosuchcmd: command not found\nbash: alsomissing: command not found\n"
+    );
+}
+
+/// Every `bash:` diagnostic the command resolver emits ends in a newline.
+/// A run-on stderr is not just cosmetic: a host that splits stderr into
+/// lines sees two failures as one, and the fuzz harness's echo filter — which
+/// matches one real-shell template per line — stops recognizing them.
+#[tokio::test]
+async fn command_resolution_diagnostics_end_with_newline() {
+    let cases: &[(&str, &str)] = &[
+        ("nosuchcmd", "bash: nosuchcmd: command not found\n"),
+        ("/tmp", "bash: /tmp: Is a directory\n"),
+        (
+            "/nope/missing",
+            "bash: /nope/missing: No such file or directory\n",
+        ),
+    ];
+    for (script, expected) in cases {
+        let mut bash = fuzz_bash();
+        let result = bash.exec(script).await.unwrap();
+        assert_eq!(result.stderr.to_string(), *expected, "script: {script}");
+    }
+}
+
+/// The suggestion variant (`. Did you mean: …`) is still one terminated line.
+#[tokio::test]
+async fn command_not_found_suggestion_ends_with_newline() {
+    let mut bash = fuzz_bash();
+    let result = bash.exec("grpe").await.unwrap();
+    let stderr = result.stderr.to_string();
+    assert!(
+        stderr.starts_with("bash: grpe: command not found. Did you mean: "),
+        "unexpected wording: {stderr:?}"
+    );
+    assert!(stderr.ends_with("\n"), "not terminated: {stderr:?}");
+    assert_eq!(stderr.lines().count(), 1, "not one line: {stderr:?}");
+}
+
+/// A non-executable file reports `Permission denied` on its own line.
+#[tokio::test]
+async fn non_executable_script_denied_ends_with_newline() {
+    let mut bash = Bash::builder()
+        .mount_text("/tmp/noexec.sh", "echo hi")
+        .build();
+    let result = bash.exec("/tmp/noexec.sh").await.unwrap();
+    assert_eq!(
+        result.stderr.to_string(),
+        "bash: /tmp/noexec.sh: Permission denied\n"
+    );
+    assert_eq!(result.exit_code, 126);
+}

knowledge/security/threat-model.md

@@ -552,10 +552,35 @@ second, independent gap:
   cannot be swallowed *inside* a span because each shell diagnostic is formatted
   and written as one string with nothing interleaved.
 
-Regression coverage: `tests/integration/glob_fuzz_scaffold_tests.rs` replays both
-crash inputs through all three scripts the target builds, and the
-`strip_*`/`input_echo_would_trip` unit tests in `src/testing.rs` pin both the
-strip and keep directions.
+Run 229 (`\n$r\n~\n-Tok:\n~:`) showed the assumption underneath the span rule was
+only half true. The filter matches one diagnostic per line, which requires each
+diagnostic to *end* its line — but the command-resolution diagnostics were built
+with `format!("bash: {}: command not found", name)` and friends and never
+terminated, so three consecutive failures arrived as one run-on line:
+
+    bash: /home/sandbox: Is a directorybash: -Tok:: command not foundbash: ~:: command not found. Did you mean: :?
+
+That line opened with a recognized template but ended with a suggestion suffix, so
+nothing was stripped and the echoed `-Tok:` — rendered `-Tok::` by the template's
+own `: ` separator — tripped the parser-token shape `Tok::`. The pre-filter could
+not have caught it either: the input holds a single colon, and the second one comes
+from the template.
+
+Fixed at the source rather than in the filter. **Every `bash:` diagnostic written to
+stderr terminates its own line**, which is what real bash does (`bash -c $'~\n-Tok:'`
+prints two `$`-terminated lines under `cat -A`). The run-on was a user-visible defect
+in its own right: a host that splits stderr into lines saw two failures as one. The
+newline is added at the emit sites in `interpreter/mod.rs` (`command_not_found_message`
+and the `try_execute_script_by_path` stat/dir/permission diagnostics) and at the
+process-substitution refusal in `interpreter/redirection.rs`, whose sibling return in
+the same function already terminated. `Error::Execution` payloads are deliberately
+left unterminated: they surface through a `Display` impl, not through stderr.
+
+Regression coverage: `tests/integration/glob_fuzz_scaffold_tests.rs` replays all three
+crash inputs through all three scripts the target builds, pins the terminator on each
+command-resolution diagnostic, and asserts two consecutive failures land on separate
+lines; the `strip_*`/`input_echo_would_trip` unit tests in `src/testing.rs` pin both
+the strip and keep directions.
 
 **TM-INF-013**: The jq builtin previously called `std::env::set_var()` to expose
 shell variables to jaq's `env` function. This also made host process env vars (API keys, tokens)