From 04592e032749036eb45bb8c7d4d3f9e531dd1e8c Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Wed, 7 Oct 2026 23:13:55 +0000 Subject: [PATCH] fix(interpreter): redirection errors read like bash Three gaps in what a failed redirect prints. The diagnostic had no line number, so a script could not tell which redirect failed. The in-memory backend's lowercase "is a directory" reached stderr instead of the strerror text bash prints. And a target written with a trailing slash created a file under the stripped name instead of failing, so `echo x > nodir/` silently made a file called `nodir`. Claude-Session: https://claude.ai/code/session_019aFikmptPc91Fj4N2iDXQA --- crates/bashkit/src/error.rs | 5 ++ crates/bashkit/src/interpreter/redirection.rs | 84 +++++++++++++------ .../integration/builtin_fs_error_tests.rs | 2 +- .../integration/glob_fuzz_scaffold_tests.rs | 6 +- crates/bashkit/tests/integration/main.rs | 1 + .../tests/integration/redirect_error_tests.rs | 58 +++++++++++++ .../bash/redirect-error-wording.test.sh | 20 +++++ knowledge/log.md | 1 + 8 files changed, 148 insertions(+), 29 deletions(-) create mode 100644 crates/bashkit/tests/integration/redirect_error_tests.rs create mode 100644 crates/bashkit/tests/spec_cases/bash/redirect-error-wording.test.sh diff --git a/crates/bashkit/src/error.rs b/crates/bashkit/src/error.rs index e50a4c31a..7d498ad88 100644 --- a/crates/bashkit/src/error.rs +++ b/crates/bashkit/src/error.rs @@ -183,6 +183,11 @@ pub(crate) fn io_error_reason(e: &Error) -> String { other => return other.to_string(), }; let message = io.to_string(); + // The in-memory backend reports this with no errno, so the kind match + // below cannot reach it; bash prints the capitalized strerror. + if message == "is a directory" { + return "Is a directory".to_string(); + } // Errors straight from the OS stringify as ` (os error N)`; // real tools print only the strerror half. let restates_errno = diff --git a/crates/bashkit/src/interpreter/redirection.rs b/crates/bashkit/src/interpreter/redirection.rs index 8c7bfd909..22fbf1285 100644 --- a/crates/bashkit/src/interpreter/redirection.rs +++ b/crates/bashkit/src/interpreter/redirection.rs @@ -22,6 +22,26 @@ use super::*; use crate::error::io_error_reason; impl Interpreter { + /// A target written with a trailing slash names a directory, so an + /// output redirect to it fails without creating anything. `resolve_path` + /// drops the slash, so the check has to run on the word as written. + fn rejects_directory_target(&self, target: &str, result: &mut ExecResult) -> bool { + if !target.ends_with('/') { + return false; + } + result.stdout = crate::StreamData::new(); + result.stderr = self.redirect_error(target, "Is a directory").into(); + result.exit_code = 1; + true + } + + /// `bash: line N: PATH: reason` — the shape bash uses for a redirection + /// that could not be opened. Bash names the script instead of `bash` + /// when running a file; the interpreter does not track a script path. + fn redirect_error(&self, path: &str, reason: &str) -> String { + format!("bash: line {}: {path}: {reason}\n", self.current_line) + } + /// Process input redirections (< file, <<< string) pub(super) async fn process_input_redirections( &mut self, @@ -59,10 +79,9 @@ impl Interpreter { stdin = Some(content.into()); } Err(e) => { - return Err(crate::error::Error::CommandFailure(format!( - "bash: {target_path}: {}\n", - io_error_reason(&e) - ))); + return Err(crate::error::Error::CommandFailure( + self.redirect_error(&target_path, &io_error_reason(&e)), + )); } } } @@ -140,6 +159,9 @@ impl Interpreter { match redirect.kind { RedirectKind::Output | RedirectKind::Clobber => { let target_path = self.expand_word(&redirect.target).await?; + if self.rejects_directory_target(&target_path, &mut result) { + return Ok(result); + } let path = self.resolve_path(&target_path); if let Some(target_fd) = dev_fd_alias(&path) { self.dup_output_fast(&mut result, redirect.fd.unwrap_or(1), target_fd) @@ -155,9 +177,9 @@ impl Interpreter { && self.fs.stat(&path).await.is_ok() { result.stdout = crate::StreamData::new(); - result.stderr = - format!("bash: {}: cannot overwrite existing file\n", target_path) - .into(); + result.stderr = self + .redirect_error(&target_path, "cannot overwrite existing file") + .into(); result.exit_code = 1; return Ok(result); } @@ -166,9 +188,9 @@ impl Interpreter { if let Err(e) = self.fs.write_file(&path, result.stderr.as_bytes()).await { - result.stderr = - format!("bash: {}: {}\n", target_path, io_error_reason(&e)) - .into(); + result.stderr = self + .redirect_error(&target_path, &io_error_reason(&e)) + .into(); result.exit_code = 1; return Ok(result); } @@ -179,9 +201,9 @@ impl Interpreter { self.fs.write_file(&path, result.stdout.as_bytes()).await { result.stdout = crate::StreamData::new(); - result.stderr = - format!("bash: {}: {}\n", target_path, io_error_reason(&e)) - .into(); + result.stderr = self + .redirect_error(&target_path, &io_error_reason(&e)) + .into(); result.exit_code = 1; return Ok(result); } @@ -192,6 +214,9 @@ impl Interpreter { } RedirectKind::Append => { let target_path = self.expand_word(&redirect.target).await?; + if self.rejects_directory_target(&target_path, &mut result) { + return Ok(result); + } let path = self.resolve_path(&target_path); if let Some(target_fd) = dev_fd_alias(&path) { self.dup_output_fast(&mut result, redirect.fd.unwrap_or(1), target_fd) @@ -207,9 +232,9 @@ impl Interpreter { if let Err(e) = self.fs.append_file(&path, result.stderr.as_bytes()).await { - result.stderr = - format!("bash: {}: {}\n", target_path, io_error_reason(&e)) - .into(); + result.stderr = self + .redirect_error(&target_path, &io_error_reason(&e)) + .into(); result.exit_code = 1; return Ok(result); } @@ -220,9 +245,9 @@ impl Interpreter { self.fs.append_file(&path, result.stdout.as_bytes()).await { result.stdout = crate::StreamData::new(); - result.stderr = - format!("bash: {}: {}\n", target_path, io_error_reason(&e)) - .into(); + result.stderr = self + .redirect_error(&target_path, &io_error_reason(&e)) + .into(); result.exit_code = 1; return Ok(result); } @@ -252,8 +277,9 @@ impl Interpreter { let mut combined = result.stdout.as_bytes().to_vec(); combined.extend_from_slice(result.stderr.as_bytes()); if let Err(e) = self.fs.write_file(&path, &combined).await { - result.stderr = - format!("bash: {}: {}\n", target_path, io_error_reason(&e)).into(); + result.stderr = self + .redirect_error(&target_path, &io_error_reason(&e)) + .into(); result.exit_code = 1; return Ok(result); } @@ -387,6 +413,9 @@ impl Interpreter { match redirect.kind { RedirectKind::Output | RedirectKind::Clobber => { let target_path = self.expand_word(&redirect.target).await?; + if self.rejects_directory_target(&target_path, &mut result) { + return Ok(result); + } let path = self.resolve_path(&target_path); if let Some(target_fd) = dev_fd_alias(&path) { let src_fd = redirect.fd.unwrap_or(1); @@ -400,9 +429,9 @@ impl Interpreter { && self.fs.stat(&path).await.is_ok() { result.stdout = crate::StreamData::new(); - result.stderr = - format!("bash: {}: cannot overwrite existing file\n", target_path) - .into(); + result.stderr = self + .redirect_error(&target_path, "cannot overwrite existing file") + .into(); result.exit_code = 1; self.clear_pending_fd_redirect_state(); return Ok(result); @@ -421,6 +450,9 @@ impl Interpreter { } RedirectKind::Append => { let target_path = self.expand_word(&redirect.target).await?; + if self.rejects_directory_target(&target_path, &mut result) { + return Ok(result); + } let path = self.resolve_path(&target_path); if let Some(target_fd) = dev_fd_alias(&path) { let src_fd = redirect.fd.unwrap_or(1); @@ -489,7 +521,9 @@ impl Interpreter { self.fs.write_file(path, content.as_bytes()).await }; if let Err(e) = write_result { - new_stderr = format!("bash: {}: {}\n", display_path, io_error_reason(&e)).into(); + new_stderr = self + .redirect_error(display_path, &io_error_reason(&e)) + .into(); result.exit_code = 1; result.stdout = new_stdout; result.stderr = new_stderr; diff --git a/crates/bashkit/tests/integration/builtin_fs_error_tests.rs b/crates/bashkit/tests/integration/builtin_fs_error_tests.rs index eb2fb11fd..3e018b70a 100644 --- a/crates/bashkit/tests/integration/builtin_fs_error_tests.rs +++ b/crates/bashkit/tests/integration/builtin_fs_error_tests.rs @@ -106,7 +106,7 @@ async fn read_only_backend_reason_survives() { let result = bash.exec("echo hi > /nope.txt").await.unwrap(); assert_eq!( result.stderr.to_string(), - "bash: /nope.txt: filesystem is read-only\n" + "bash: line 1: /nope.txt: filesystem is read-only\n" ); } diff --git a/crates/bashkit/tests/integration/glob_fuzz_scaffold_tests.rs b/crates/bashkit/tests/integration/glob_fuzz_scaffold_tests.rs index 00a5f9925..8fc498f98 100644 --- a/crates/bashkit/tests/integration/glob_fuzz_scaffold_tests.rs +++ b/crates/bashkit/tests/integration/glob_fuzz_scaffold_tests.rs @@ -105,7 +105,7 @@ async fn missing_input_redirect_matches_bash_wording() { let result = bash.exec("ls /tmp/ < /nope/missing").await.unwrap(); assert_eq!( result.stderr.to_string(), - "bash: /nope/missing: No such file or directory\n" + "bash: line 1: /nope/missing: No such file or directory\n" ); assert_eq!(result.exit_code, 1); } @@ -119,7 +119,7 @@ async fn missing_output_redirect_dir_matches_bash_wording() { let result = bash.exec("echo hi > /nope/missing/out.txt").await.unwrap(); assert_eq!( result.stderr.to_string(), - "bash: /nope/missing/out.txt: No such file or directory\n" + "bash: line 1: /nope/missing/out.txt: No such file or directory\n" ); assert_eq!(result.exit_code, 1); } @@ -137,7 +137,7 @@ async fn read_only_mount_keeps_its_specific_reason() { let result = bash.exec("echo hi > /tmp/nope.txt").await.unwrap(); assert_eq!( result.stderr.to_string(), - "bash: /tmp/nope.txt: filesystem is read-only\n" + "bash: line 1: /tmp/nope.txt: filesystem is read-only\n" ); assert!(!result.stderr.to_string().contains("io error:")); } diff --git a/crates/bashkit/tests/integration/main.rs b/crates/bashkit/tests/integration/main.rs index f45d5ba72..9e12809db 100644 --- a/crates/bashkit/tests/integration/main.rs +++ b/crates/bashkit/tests/integration/main.rs @@ -115,6 +115,7 @@ pub mod pipeline_tests; pub mod proptest_differential; pub mod python_integration_tests; pub mod python_security_tests; +pub mod redirect_error_tests; pub mod regex_limit_tests; pub mod release_profile_tests; pub mod request_lifecycle_contract_tests; diff --git a/crates/bashkit/tests/integration/redirect_error_tests.rs b/crates/bashkit/tests/integration/redirect_error_tests.rs new file mode 100644 index 000000000..3dbe97e7c --- /dev/null +++ b/crates/bashkit/tests/integration/redirect_error_tests.rs @@ -0,0 +1,58 @@ +//! Wording and status of a redirection that cannot be opened. +//! +//! Bash prefixes the diagnostic with the line the redirect is on and uses the +//! capitalized strerror text (`Is a directory`). A target written with a +//! trailing slash names a directory, so no file is created for it. + +use bashkit::Bash; + +async fn run(script: &str) -> bashkit::ExecResult { + Bash::builder().build().exec(script).await.unwrap() +} + +#[tokio::test] +async fn noclobber_names_the_line() { + let r = run("echo a > f.txt\nset -o noclobber\necho b > f.txt").await; + assert_eq!( + r.stderr.to_string(), + "bash: line 3: f.txt: cannot overwrite existing file\n" + ); + assert_eq!(r.exit_code, 1); +} + +#[tokio::test] +async fn redirecting_onto_a_directory_fails() { + let r = run("mkdir d\necho x > d").await; + assert_eq!(r.stderr.to_string(), "bash: line 2: d: Is a directory\n"); + assert_eq!(r.exit_code, 1); +} + +#[tokio::test] +async fn a_trailing_slash_names_a_directory_and_creates_nothing() { + let r = run("echo y > nodir/\necho \"rc=$?\"\nls").await; + assert_eq!( + r.stderr.to_string(), + "bash: line 1: nodir/: Is a directory\n" + ); + assert_eq!(r.stdout.to_string(), "rc=1\n"); +} + +#[tokio::test] +async fn append_rejects_a_trailing_slash_too() { + let r = run("echo z >> nodir/").await; + assert_eq!( + r.stderr.to_string(), + "bash: line 1: nodir/: Is a directory\n" + ); + assert_eq!(r.exit_code, 1); +} + +#[tokio::test] +async fn a_missing_parent_directory_names_the_line() { + let r = run("echo w > /nope/deep/f").await; + assert_eq!( + r.stderr.to_string(), + "bash: line 1: /nope/deep/f: No such file or directory\n" + ); + assert_eq!(r.exit_code, 1); +} diff --git a/crates/bashkit/tests/spec_cases/bash/redirect-error-wording.test.sh b/crates/bashkit/tests/spec_cases/bash/redirect-error-wording.test.sh new file mode 100644 index 000000000..8d35a4d3c --- /dev/null +++ b/crates/bashkit/tests/spec_cases/bash/redirect-error-wording.test.sh @@ -0,0 +1,20 @@ +### redirect_onto_a_directory_fails +### exit_code: 1 +mkdir d +echo x > d 2>/dev/null +### end + +### trailing_slash_target_creates_nothing +echo y > nodir/ 2>/dev/null +echo "rc=$?" +ls +### expect +rc=1 +### end + +### append_to_a_trailing_slash_fails +echo z >> nodir/ 2>/dev/null +echo "rc=$?" +### expect +rc=1 +### end diff --git a/knowledge/log.md b/knowledge/log.md index 86ad11dd4..97c4fe668 100644 --- a/knowledge/log.md +++ b/knowledge/log.md @@ -14,6 +14,7 @@ * **Fix**: `grep` follows GNU grep 3.11. Prefixes print as `FILE:LINE:BYTE:` (context lines with `-`), group separators appear between non-adjacent groups and files whenever a context option is given (`-NUM`, `--group-separator`, `--no-group-separator`), trailing context after `-m` is printed in full, `-o` skips empty matches and is POSIX leftmost-longest (`grep -oE 'ab|abcd'` prints `abcd`), `-b -o` gives each match's offset. New `grep_pattern` translates GNU BRE/ERE (brackets, intervals, `\< \> \b \w`, literal leading `*` in BRE, dropped leading ERE operator with GNU's warning, glibc error messages) onto awk's leftmost-longest `AwkRegex`; back-references work in BRE and ERE via fancy-regex (TM-DOS-025 cap). Options parse like getopt_long (unique prefixes such as `--no-gr`, `-T`, `-d`, `--label`, `-` for stdin, `-R` follows symlinks), an empty `-f` file matches nothing, a directory operand without `-r` reports `Is a directory`, invalid UTF-8 output turns the file binary, `\r` is line data. Debian-oracle grep score 60.5% to 96.8% (185 cases); grep's probes in the regex set all pass except two raw-`\x01` patterns the shell drops. Remaining misses are readdir order (L-GREP-003); L-GREP-002 lists the other gaps. See foundations/builtins "grep". * **Fix**: builtins reject the arguments bash rejects. `return` outside a function or a sourced script reports `can only \`return' from a function or sourced script`, exits 2 and the script carries on (it used to end the script with status 0); `return N` inside a sourced script is now `source`'s status; `unset -v` insists the name is one it could have assigned (`arr[`, `a-b`, `1bad` are errors, `a[0]` is fine) while plain `unset` and `unset -f` still take any word; and `read -t`/`-n` report `invalid timeout specification` / `invalid number` instead of silently ignoring the option. +* **Fix**: a redirection that cannot be opened reads like bash: the diagnostic carries the line (`bash: line 3: f.txt: cannot overwrite existing file`), the in-memory backend's `is a directory` is capitalized to the strerror text bash prints, and a target written with a trailing slash (`> nodir/`) is a directory, so it fails with `Is a directory` instead of creating a file called `nodir`. * **Fix**: jq `implode` maps invalid code points to U+FFFD, `strptime`/`fromdate` fail with jq's `date "X" does not match format "F"`, and `@base64d` accepts unpadded input and non-zero trailing bits with jq's error texts. * **Feature**: jq destructuring alternatives `T as P1 ?// P2 | BODY`: every pattern variable is bound (null when unbound), an error in a pattern or the body moves to the next pattern, the last error propagates. jaq does not parse `?//`; `builtins/jq/altpat.rs` rewrites it to `try`/`catch` before compiling (body copied per pattern, at most 16 rewrites). * **Feature**: jq `$__loc__` (`{"file":"","line":N}`), including `{$__loc__}` shorthand and string interpolation. jaq has no `$__loc__`, so the filter text is rewritten before compiling (`builtins/jq/loc.rs` skips strings and comments).