fix(grep): send read diagnostics to stderr and exit 2

crates/bashkit/src/builtins/grep.rs

@@ -586,6 +586,14 @@ impl Builtin for Grep {
         let matcher = opts.build_matcher()?;
 
         let mut output = String::new();
+        // Diagnostics are a separate stream: a `grep: FILE: ...` line written
+        // into `output` lands in the data a pipeline consumes, so
+        // `grep foo *.log | wc -l` counts it as a match.
+        let mut errors = String::new();
+        // GNU grep exits 2 when any operand could not be read, whether or not
+        // another operand matched, and `-s` silences the message but not the
+        // status.
+        let mut read_failed = false;
         let mut any_match = false;
         let mut exit_code = 1; // 1 = no match
         let mut total_matches = 0usize;
@@ -683,9 +691,11 @@ impl Builtin for Grep {
                         inputs.push((file.clone(), text));
                     }
                     Err(e) => {
-                        // Report error but continue with other files
-                        if !opts.quiet && !opts.suppress_errors {
-                            output.push_str(&format!("grep: {}: {}\n", file, e));
+                        // Report the error but continue with the other files.
+                        read_failed = true;
+                        if !opts.suppress_errors {
+                            let reason = crate::error::io_error_reason(&e);
+                            errors.push_str(&format!("grep: {file}: {reason}\n"));
                         }
                     }
                 }
@@ -699,7 +709,10 @@ impl Builtin for Grep {
         } else if opts.show_filename || opts.recursive {
             true
         } else {
-            inputs.len() > 1
+            // Operands requested, not operands successfully read: GNU grep
+            // prints `ok.txt:hello` for `grep hello ok.txt /nope`, because two
+            // files were named even though only one could be opened.
+            opts.files.len().max(inputs.len()) > 1
         };
         let has_context = opts.before_context > 0 || opts.after_context > 0;
 
@@ -966,13 +979,30 @@ impl Builtin for Grep {
         if any_match {
             exit_code = 0;
         }
+        // An unreadable operand outranks "no match", but not a match found
+        // under `-q`: GNU `grep -q hello ok.txt /nope` exits 0, because it
+        // stops at the first match without reaching the bad operand.
+        // `-q` stops at the first match, so GNU grep never reaches a later
+        // unreadable operand: no message and status 0. Bashkit reads every
+        // operand up front, so emulate the short-circuit by dropping both.
+        if opts.quiet && any_match {
+            errors.clear();
+        } else if read_failed {
+            exit_code = 2;
+        }
 
         // In quiet mode, return empty output
         if opts.quiet {
-            return Ok(ExecResult::with_code(String::new(), exit_code));
+            return Ok(ExecResult {
+                stderr: errors.into(),
+                ..ExecResult::with_code(String::new(), exit_code)
+            });
         }
 
-        Ok(ExecResult::with_code(output, exit_code))
+        Ok(ExecResult {
+            stderr: errors.into(),
+            ..ExecResult::with_code(output, exit_code)
+        })
     }
 }
 
@@ -2152,9 +2182,10 @@ mod tests {
         };
 
         let result = grep.execute(ctx).await.unwrap();
-        assert_eq!(result.exit_code, 1);
-        // -s suppresses error messages
+        // GNU grep: -s silences the message but not the status.
+        assert_eq!(result.exit_code, 2);
         assert_eq!(result.stdout, "");
+        assert_eq!(result.stderr, "");
     }
 
     #[tokio::test]
@@ -2186,9 +2217,14 @@ mod tests {
         };
 
         let result = grep.execute(ctx).await.unwrap();
-        assert_eq!(result.exit_code, 1);
-        // Without -s, error message is shown
-        assert!(result.stdout.contains("grep: /nonexistent:"));
+        // GNU grep: the diagnostic goes to stderr, never into the data
+        // stream, and an unreadable operand is status 2, not "no match".
+        assert_eq!(result.exit_code, 2);
+        assert_eq!(result.stdout, "");
+        assert_eq!(
+            result.stderr,
+            "grep: /nonexistent: No such file or directory\n"
+        );
     }
 
     #[tokio::test]
@@ -2220,8 +2256,10 @@ mod tests {
         };
 
         let result = grep.execute(ctx).await.unwrap();
-        assert_eq!(result.exit_code, 1);
+        // --no-messages is the long form of -s: same status 2.
+        assert_eq!(result.exit_code, 2);
         assert_eq!(result.stdout, "");
+        assert_eq!(result.stderr, "");
     }
 
     // -Z (--null) tests

crates/bashkit/src/builtins/json.rs

@@ -126,7 +126,10 @@ async fn read_json_input(
         match ctx.fs.read_file(&path).await {
             Ok(bytes) => String::from_utf8(bytes)
                 .map_err(|e| ExecResult::err(format!("json: invalid UTF-8 in {file}: {e}\n"), 1)),
-            Err(e) => Err(ExecResult::err(format!("json: {file}: {e}\n"), 1)),
+            Err(e) => {
+                let reason = crate::error::io_error_reason(&e);
+                Err(ExecResult::err(format!("json: {file}: {reason}\n"), 1))
+            }
         }
     } else if let Some(stdin) = ctx.stdin {
         Ok(stdin.to_string())

crates/bashkit/src/builtins/rg/mod.rs

@@ -2988,10 +2988,10 @@ async fn read_rg_text_file(
     display_path: &str,
     opts: &RgOptions,
 ) -> std::result::Result<String, ExecResult> {
-    let content = fs
-        .read_file(path)
-        .await
-        .map_err(|e| ExecResult::err(format!("rg: {display_path}: {e}\n"), 1))?;
+    let content = fs.read_file(path).await.map_err(|e| {
+        let reason = crate::error::io_error_reason(&e);
+        ExecResult::err(format!("rg: {display_path}: {reason}\n"), 1)
+    })?;
 
     opts.validate_preprocessor_for(path, cwd)
         .map_err(|e| ExecResult::err(format!("rg: {display_path}: {e}\n"), 1))?;

crates/bashkit/src/builtins/sed/mod.rs

@@ -370,7 +370,11 @@ impl Builtin for Sed {
                     },
                     // GNU exits 2 when an input file cannot be read.
                     Err(e) => {
-                        return Ok(ExecResult::err(format!("sed: can't read {name}: {e}\n"), 2));
+                        let reason = crate::error::io_error_reason(&e);
+                        return Ok(ExecResult::err(
+                            format!("sed: can't read {name}: {reason}\n"),
+                            2,
+                        ));
                     }
                 }
             };

crates/bashkit/tests/integration/builtin_fs_error_tests.rs

@@ -14,26 +14,44 @@ use bashkit::Bash;
 use bashkit::testing::assert_no_leak;
 
 /// Every tool here takes a file operand and reads it through the VFS.
-const READERS: &[(&str, &str)] = &[
-    ("cat", "cat /nope"),
-    ("head", "head /nope"),
-    ("tail", "tail /nope"),
-    ("strings", "strings /nope"),
-    ("wc", "wc /nope"),
-    ("sort", "sort /nope"),
-    ("uniq", "uniq /nope"),
-    ("cut", "cut -c1 /nope"),
-    ("nl", "nl /nope"),
-    ("awk", "awk '{print}' /nope"),
-    ("paste", "paste /nope"),
-    ("rev", "rev /nope"),
-    ("tac", "tac /nope"),
-    ("column", "column /nope"),
+/// The exit code is each tool's own: GNU `grep`, `sed` and `rg` report an
+/// unreadable operand as 2, the coreutils text tools as 1.
+const READERS: &[(&str, &str, i32)] = &[
+    ("cat", "cat /nope", 1),
+    ("head", "head /nope", 1),
+    ("tail", "tail /nope", 1),
+    ("strings", "strings /nope", 1),
+    ("wc", "wc /nope", 1),
+    ("sort", "sort /nope", 1),
+    ("uniq", "uniq /nope", 1),
+    ("cut", "cut -c1 /nope", 1),
+    ("nl", "nl /nope", 1),
+    ("awk", "awk '{print}' /nope", 1),
+    ("paste", "paste /nope", 1),
+    ("rev", "rev /nope", 1),
+    ("tac", "tac /nope", 1),
+    ("column", "column /nope", 1),
+    ("split", "split /nope", 1),
+    ("comm", "comm /nope /nope2", 1),
+    ("shuf", "shuf /nope", 1),
+    ("diff", "diff /nope /nope2", 1),
+    ("dotenv", "dotenv /nope", 1),
+    ("template", "template /nope", 1),
+    ("json", "json get x /nope", 1),
+    ("od", "od /nope", 1),
+    ("xxd", "xxd /nope", 1),
+    ("hexdump", "hexdump /nope", 1),
+    ("base64", "base64 /nope", 1),
+    ("fold", "fold /nope", 1),
+    ("expand", "expand /nope", 1),
+    ("grep", "grep x /nope", 2),
+    ("sed", "sed s/a/b/ /nope", 2),
+    ("rg", "rg foo /nope", 2),
 ];
 
 #[tokio::test]
 async fn missing_operand_reports_the_real_errno_text() {
-    for (name, script) in READERS {
+    for (name, script, code) in READERS {
         let mut bash = Bash::new();
         let result = bash
             .exec(script)
@@ -44,17 +62,25 @@ async fn missing_operand_reports_the_real_errno_text() {
             stderr.contains("No such file or directory"),
             "{name}: expected the real errno text, got {stderr:?}"
         );
-        assert_eq!(result.exit_code, 1, "{name} exit code");
+        assert_eq!(result.exit_code, *code, "{name} exit code");
     }
 }
 
 /// The specific shape that regressed: the `Display` of `bashkit::Error`.
 #[tokio::test]
 async fn missing_operand_does_not_leak_the_error_enum_shape() {
-    for (name, script) in READERS {
+    for (name, script, _) in READERS {
         let mut bash = Bash::new();
         let result = bash.exec(script).await.unwrap();
         let stderr = result.stderr.to_string();
+        // The diagnostic belongs on stderr, never in the data stream.
+        assert!(
+            !result
+                .stdout
+                .to_string()
+                .contains("No such file or directory"),
+            "{name} put its diagnostic in stdout"
+        );
         assert!(
             !stderr.contains("io error:"),
             "{name} leaks the Error enum shape: {stderr:?}"
@@ -95,3 +121,83 @@ async fn readable_operand_still_reads() {
         assert_eq!(result.stderr.to_string(), "", "{script} stderr");
     }
 }
+
+/// `grep` had three defects beyond the wording, all verified against GNU
+/// grep 3.11:
+///  1. the diagnostic went into stdout, so `grep foo *.log | wc -l` counted it
+///     as a match;
+///  2. an unreadable operand did not set status 2;
+///  3. the filename prefix followed the files it managed to read, not the
+///     files it was asked for.
+mod grep_unreadable_operand {
+    use bashkit::Bash;
+
+    async fn run(script: &str) -> bashkit::ExecResult {
+        let mut bash = Bash::builder().mount_text("/w/ok.txt", "hello\n").build();
+        bash.exec(&format!("cd /w; {script}")).await.unwrap()
+    }
+
+    #[tokio::test]
+    async fn diagnostic_goes_to_stderr_not_stdout() {
+        let r = run("grep hello ok.txt /nope").await;
+        assert_eq!(r.stdout.to_string(), "ok.txt:hello\n");
+        assert_eq!(
+            r.stderr.to_string(),
+            "grep: /nope: No such file or directory\n"
+        );
+    }
+
+    /// The prefix is present because two operands were named, even though only
+    /// one could be opened.
+    #[tokio::test]
+    async fn filename_prefix_follows_operands_asked_for() {
+        let r = run("grep hello ok.txt /nope").await;
+        assert!(r.stdout.to_string().starts_with("ok.txt:"));
+        // A single readable operand still gets no prefix.
+        let single = run("grep hello ok.txt").await;
+        assert_eq!(single.stdout.to_string(), "hello\n");
+    }
+
+    #[tokio::test]
+    async fn unreadable_operand_is_status_2() {
+        // Status 2 outranks both "matched" and "no match".
+        assert_eq!(run("grep hello ok.txt /nope").await.exit_code, 2);
+        assert_eq!(run("grep zzz ok.txt /nope").await.exit_code, 2);
+        assert_eq!(run("grep hello /nope").await.exit_code, 2);
+        // No error, no change.
+        assert_eq!(run("grep hello ok.txt").await.exit_code, 0);
+        assert_eq!(run("grep zzz ok.txt").await.exit_code, 1);
+    }
+
+    /// `-s` silences the message but not the status.
+    #[tokio::test]
+    async fn suppress_errors_keeps_the_status() {
+        let r = run("grep -s hello ok.txt /nope").await;
+        assert_eq!(r.exit_code, 2);
+        assert_eq!(r.stderr.to_string(), "");
+        assert_eq!(r.stdout.to_string(), "ok.txt:hello\n");
+    }
+
+    /// `-q` stops at the first match, so GNU grep never reaches the bad
+    /// operand: status 0 and no message. Without a match it does reach it.
+    #[tokio::test]
+    async fn quiet_short_circuits_on_a_match_only() {
+        let matched = run("grep -q hello ok.txt /nope").await;
+        assert_eq!(matched.exit_code, 0);
+        assert_eq!(matched.stderr.to_string(), "");
+
+        let unmatched = run("grep -q zzz ok.txt /nope").await;
+        assert_eq!(unmatched.exit_code, 2);
+        assert_eq!(
+            unmatched.stderr.to_string(),
+            "grep: /nope: No such file or directory\n"
+        );
+    }
+
+    /// The bug that motivated the stream split.
+    #[tokio::test]
+    async fn pipeline_does_not_see_the_diagnostic() {
+        let r = run("grep hello ok.txt /nope 2>/dev/null | wc -l").await;
+        assert_eq!(r.stdout.to_string().trim(), "1");
+    }
+}

crates/bashkit/tests/spec_cases/grep/grep.test.sh

@@ -594,11 +594,12 @@ mkdir -p /tmp/grepexd/src /tmp/grepexd/vendor && printf 'match\n' > /tmp/grepexd
 ### end
 
 ### grep_suppress_errors
-# -s suppresses error messages for nonexistent files
+# -s suppresses the error message but not the status: GNU grep exits 2
+# whenever an operand could not be read, matched or not.
 grep -s foo /tmp/nonexistent_grep_file_xyz
 echo $?
 ### expect
-1
+2
 ### end
 
 ### grep_null_filename_l

knowledge/security/threat-model.md

@@ -470,7 +470,15 @@ Most file-reading builtins go through `builtins::read_text_file` /
 `read_stream_file`, so a single leaking formatter there reached ~25 tools at
 once (`wc`, `sort`, `cut`, `nl`, `awk`, `paste`, `uniq`, `rev`, `tac`,
 `column`, `jq`, `diff`, …); `cat`, `head`, `tail` and `strings` read the VFS
-directly and leaked it separately. Coverage: `builtin_fs_error_tests`.
+directly and leaked it separately, as did `rg`, `sed` and `json`. The sweep
+is complete: `builtin_fs_error_tests` walks all thirty file-reading builtins
+and fails on any banned shape in either stream.
+
+A diagnostic also has to reach the right stream. `grep` accumulated its
+`grep: FILE: ...` line into the stdout buffer, so `grep foo *.log | wc -l`
+counted it as a match — a wrong-stream bug is a data-corruption bug, not a
+cosmetic one. Status matters too: GNU `grep`, `sed` and `rg` report an
+unreadable operand as exit 2, which outranks both "matched" and "no match".
 
 A related shape: a user-reachable failure classified as `Error::Internal`
 aborts the whole script *and* prints `internal error:`. Nightly `glob_fuzz`