Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion crates/bashkit/src/interpreter/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6058,7 +6058,13 @@ impl Interpreter {
let content = match self.fs.read_file(&path).await {
Ok(c) => c,
Err(e) => {
return Ok(ExecResult::err(format!("bash: {target_path}: {e}\n"), 1));
return Ok(ExecResult::err(
format!(
"bash: {target_path}: {}\n",
redirection::redirect_error_reason(&e)
),
1,
));
}
};
let text = decode_file_bytes_for_path(&path, &content);
Expand Down
93 changes: 82 additions & 11 deletions crates/bashkit/src/interpreter/redirection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,60 @@
//! output-redirection / fd-routing core. The `FdTarget` enum and
//! `route_fd_table_content` helper stay in the parent module (referenced
//! by interpreter state fields).
//!
//! Redirection failures are reported as `bash: <path>: <reason>` via
//! [`redirect_error_reason`] — never as the `Display` of a [`crate::Error`],
//! whose `io error: ` prefix is a Rust enum shape no shell ever prints.
//!
//! The reason text matters beyond cosmetics: the fuzz/proptest leak detector
//! in `bashkit::testing` suppresses shell echoes of user input only for lines
//! matching a recognized real-shell template. Nightly `glob_fuzz` run 218
//! failed on `</r\0ustc/` — the NUL is dropped during expansion, so the
//! redirect target became `/rustc/` after the target's own input pre-filter
//! had run, and `bash: /rustc/: io error: file not found` was reported as a
//! TM-INF-016 host-path leak instead of an echo.

use super::*;

/// VFS messages that only restate their `io::ErrorKind`.
///
/// These carry nothing the errno does not, so redirection diagnostics swap
/// them for the `strerror` text real bash prints. Every other message —
/// `filesystem is read-only`, a custom [`crate::FileSystem`] backend's own
/// wording — tells the caller *why* in a way the bare errno cannot, and is
/// kept verbatim.
const ERRNO_RESTATING_MESSAGES: &[&str] = &["file not found", "parent directory not found"];

/// Render a filesystem error the way bash renders a redirection failure.
///
/// `ls < /nope` reports `bash: /nope: No such file or directory`, matching
/// real bash, rather than the Rust error-enum `Display`
/// (`bash: /nope: io error: file not found`).
pub(super) fn redirect_error_reason(e: &crate::error::Error) -> String {
let io = match e {
crate::error::Error::Io(io) => io,
other => return other.to_string(),
};
let message = io.to_string();
// Errors straight from the OS stringify as `<strerror> (os error N)`;
// bash prints only the strerror half.
let restates_errno =
io.raw_os_error().is_some() || ERRNO_RESTATING_MESSAGES.contains(&message.as_str());
if !restates_errno {
return message;
}
match io.kind() {
std::io::ErrorKind::NotFound => "No such file or directory",
std::io::ErrorKind::PermissionDenied => "Permission denied",
std::io::ErrorKind::IsADirectory => "Is a directory",
std::io::ErrorKind::NotADirectory => "Not a directory",
std::io::ErrorKind::AlreadyExists => "File exists",
std::io::ErrorKind::Unsupported => "Operation not supported",
_ => return message,
}
.to_string()
}

impl Interpreter {
/// Process input redirections (< file, <<< string)
pub(super) async fn process_input_redirections(
Expand Down Expand Up @@ -44,7 +95,8 @@ impl Interpreter {
}
Err(e) => {
return Err(crate::error::Error::CommandFailure(format!(
"bash: {target_path}: {e}\n"
"bash: {target_path}: {}\n",
redirect_error_reason(&e)
)));
}
}
Expand Down Expand Up @@ -141,8 +193,12 @@ impl Interpreter {
if let Err(e) =
self.fs.write_file(&path, result.stderr.as_bytes()).await
{
result.stderr =
format!("bash: {}: {}\n", target_path, e).into();
result.stderr = format!(
"bash: {}: {}\n",
target_path,
redirect_error_reason(&e)
)
.into();
result.exit_code = 1;
return Ok(result);
}
Expand All @@ -153,8 +209,12 @@ impl Interpreter {
self.fs.write_file(&path, result.stdout.as_bytes()).await
{
result.stdout = crate::StreamData::new();
result.stderr =
format!("bash: {}: {}\n", target_path, e).into();
result.stderr = format!(
"bash: {}: {}\n",
target_path,
redirect_error_reason(&e)
)
.into();
result.exit_code = 1;
return Ok(result);
}
Expand All @@ -177,8 +237,12 @@ impl Interpreter {
if let Err(e) =
self.fs.append_file(&path, result.stderr.as_bytes()).await
{
result.stderr =
format!("bash: {}: {}\n", target_path, e).into();
result.stderr = format!(
"bash: {}: {}\n",
target_path,
redirect_error_reason(&e)
)
.into();
result.exit_code = 1;
return Ok(result);
}
Expand All @@ -189,8 +253,12 @@ impl Interpreter {
self.fs.append_file(&path, result.stdout.as_bytes()).await
{
result.stdout = crate::StreamData::new();
result.stderr =
format!("bash: {}: {}\n", target_path, e).into();
result.stderr = format!(
"bash: {}: {}\n",
target_path,
redirect_error_reason(&e)
)
.into();
result.exit_code = 1;
return Ok(result);
}
Expand All @@ -209,7 +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, e).into();
result.stderr =
format!("bash: {}: {}\n", target_path, redirect_error_reason(&e))
.into();
result.exit_code = 1;
return Ok(result);
}
Expand Down Expand Up @@ -429,7 +499,8 @@ impl Interpreter {
self.fs.write_file(path, content.as_bytes()).await
};
if let Err(e) = write_result {
new_stderr = format!("bash: {}: {}\n", display_path, e).into();
new_stderr =
format!("bash: {}: {}\n", display_path, redirect_error_reason(&e)).into();
result.exit_code = 1;
result.stdout = new_stdout;
result.stderr = new_stderr;
Expand Down
11 changes: 11 additions & 0 deletions crates/bashkit/src/testing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,17 @@ fn is_real_shell_error_line(line: &str) -> bool {
": Permission denied",
": cannot execute: required file not found",
": cannot execute binary file",
// Remaining errno templates that `redirect_error_reason` can emit
// for `bash: <redirect target>: <strerror>`.
": Not a directory",
": File exists",
": Operation not supported",
// Fixed bashkit redirection refusals. Like the errno templates they
// quote the user-supplied redirect target verbatim, so a target
// containing a banned shape is an echo, not an internal leak.
": filesystem redirection disabled",
": cannot overwrite existing file",
": filesystem is read-only",
];
if let Some(rest) = line.strip_prefix("bash: ") {
if SHELL_ERROR_SUFFIXES.iter().any(|suf| rest.ends_with(suf)) {
Expand Down
118 changes: 118 additions & 0 deletions crates/bashkit/tests/integration/glob_fuzz_scaffold_tests.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
// Scaffold tests for the glob_fuzz target.
//
// Replays crash inputs found by `cargo +nightly fuzz run glob_fuzz` so the
// regression is covered by the normal `cargo test` run instead of only by
// the nightly fuzz workflow.
//
// Design note: `glob_fuzz` inlines its raw input into shell scripts and then
// asserts the cross-tool leak invariants from `bashkit::testing`. The target
// pre-filters inputs that literally contain a banned substring, but the shell
// drops NUL bytes during word expansion, so an input like `</r\0ustc/` becomes
// the path `/rustc/` *after* the filter has run. Any diagnostic that echoes
// such a path back must therefore use a real-shell error template that
// `bashkit::testing`'s echo filter recognizes.

use bashkit::testing::{fuzz_exec, fuzz_init};
use bashkit::{Bash, ExecutionLimits};

fn fuzz_bash() -> Bash {
fuzz_init();
Bash::builder()
.limits(
ExecutionLimits::new()
.max_commands(50)
.timeout(std::time::Duration::from_millis(500)),
)
.mount_text("/tmp/a.txt", "")
.mount_text("/tmp/b.sh", "")
.mount_text("/tmp/sub/d.txt", "")
.build()
}

/// Regression: fuzz run 218 (`crash-fae53719665e9c77d2e1a1b7d4da7e43902525d0`,
/// bytes `[60, 47, 114, 0, 117, 115, 116, 99, 47]` = `</r\0ustc/`).
///
/// `ls /tmp/</r\0ustc/` is an input redirection from a path that does not
/// exist. The NUL is dropped during expansion, so the redirection target is
/// `/rustc/` — a `UNIVERSAL_BANNED` host-path shape. The diagnostic must use
/// bash's real template so the echo filter recognizes it as a shell echo of
/// user input rather than a TM-INF-016 host-path leak.
#[tokio::test]
async fn glob_fuzz_crash_nul_stripped_redirect_target() {
// Replays all three scripts the glob_fuzz target builds from one input,
// in the same order, so the scaffold covers the whole crashing iteration.
let input = "</r\0ustc/";
let scripts = [
format!("ls /tmp/{}", input),
format!(
"case \"test.txt\" in {}) echo match;; *) echo no;; esac",
input
),
format!("if [[ \"hello.world\" == {} ]]; then echo y; fi", input),
];
let mut bash = fuzz_bash();
for script in &scripts {
fuzz_exec(
&mut bash,
script,
"glob_fuzz_crash_nul_stripped_redirect_target",
&[],
)
.await;
}
}

/// The missing-input-redirect diagnostic must match real bash byte for byte:
/// `bash: <path>: No such file or directory`.
#[tokio::test]
async fn missing_input_redirect_matches_bash_wording() {
let mut bash = fuzz_bash();
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"
);
assert_eq!(result.exit_code, 1);
}

/// Same template for an output redirect whose parent directory is missing.
/// The VFS's `parent directory not found` only restates `NotFound`, so it is
/// replaced by bash's wording rather than surfaced.
#[tokio::test]
async fn missing_output_redirect_dir_matches_bash_wording() {
let mut bash = fuzz_bash();
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"
);
assert_eq!(result.exit_code, 1);
}

/// A backend reason that says more than its errno survives: collapsing it to
/// the bare `Permission denied` would drop the only actionable detail. This
/// is what a custom [`bashkit::FileSystem`] impl's own wording relies on.
#[tokio::test]
async fn read_only_mount_keeps_its_specific_reason() {
let mut bash = Bash::builder()
.fs(std::sync::Arc::new(bashkit::ReadOnlyFs::new(
std::sync::Arc::new(bashkit::InMemoryFs::new()),
)))
.build();
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"
);
assert!(!result.stderr.to_string().contains("io error:"));
}

/// A NUL byte inside a word is dropped, not preserved, so the redirect target
/// collapses to the same path with or without it.
#[tokio::test]
async fn nul_byte_in_word_is_dropped() {
let mut bash = fuzz_bash();
let with_nul = bash.exec("ls /tmp/ < /no\0pe").await.unwrap();
let without_nul = bash.exec("ls /tmp/ < /nope").await.unwrap();
assert_eq!(with_nul.stderr.to_string(), without_nul.stderr.to_string());
}
1 change: 1 addition & 0 deletions crates/bashkit/tests/integration/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ pub mod git_inspection_tests;
pub mod git_integration_tests;
pub mod git_remote_security_tests;
pub mod git_security_tests;
pub mod glob_fuzz_scaffold_tests;
pub mod glob_intermediate_component_tests;
pub mod harness_example_tests;
pub mod history_tests;
Expand Down
21 changes: 21 additions & 0 deletions knowledge/security/threat-model.md
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,27 @@ the byte-length cap and host-canary check still run on the unfiltered stderr, so
and TM-INF-013 regressions are still caught. The strict per-builtin path
(`assert_no_leak`) is unchanged, non-fuzz tests must not produce shell echoes at all.

The recognized-template list is the whole mechanism, so **any diagnostic that
echoes a user-supplied path must use a real shell template**. Nightly `glob_fuzz`
run 218 failed on input `</r\0ustc/`: the shell drops the NUL during expansion, so
the redirect target became `/rustc/` *after* the target's own input pre-filter had
run, and bashkit's redirection diagnostic rendered the Rust error-enum `Display`
(`bash: /rustc/: io error: file not found`) — an unrecognized template, reported as
a TM-INF-016 host-path leak. Fixed at the source by
`interpreter::redirection::redirect_error_reason`, which drops the `io error: `
enum prefix and replaces *errno-restating* messages with the `strerror` text real
bash prints (`No such file or directory`, `Permission denied`, `Is a directory`, …).

Deliberately narrow: only messages that add nothing to their `io::ErrorKind` are
replaced — the VFS placeholders in `ERRNO_RESTATING_MESSAGES`, plus errors carrying
a `raw_os_error` (whose `Display` appends a non-bash `(os error N)` suffix). Every
other message survives verbatim, because a backend-specific reason tells an agent
*why* in a way the bare errno cannot: `filesystem is read-only` must not collapse
into `Permission denied`, and a custom `FileSystem` impl's own wording is the only
diagnostic its embedder gets. Those specific reasons are instead covered by adding
their fixed templates to the echo filter. Regression coverage:
`tests/integration/glob_fuzz_scaffold_tests.rs`.

**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)
visible. Additionally, `set_var` is thread-unsafe (unsound in Rust 2024 edition). Fixed: a custom
Expand Down