Skip to content

Commit 7f642b0

Browse files
authored
fix(redirect): suppress stderr from builtins with 2>/dev/null (#1138)
## Summary - Fix compound commands with output redirects (e.g. `{ cmd; } 2>/dev/null`) leaking stderr via output_callback before `apply_redirections` ran - Fix glob error sentinel path in `execute_simple_command` returning early without calling `apply_redirections` ## What Two bugs caused `2>/dev/null` to not suppress stderr from builtins: 1. **Streaming callback leak**: When `output_callback` is set (streaming mode), compound commands like `{ ls /nonexistent; } 2>/dev/null` emitted stderr via the callback during body execution, before the outer redirect could suppress it. Fix: suspend `output_callback` during compound body execution when output redirects are present. 2. **Glob sentinel bypass**: When `shopt -s failglob` is enabled and a glob has no matches, the error sentinel path returned `ExecResult::err()` directly without routing through `apply_redirections`. Fix: pass the error result through `apply_redirections`. ## Tests - Unit test: `test_stderr_redirect_devnull_simple_and_compound` (simple, compound, &>, failglob) - Streaming test: `test_stderr_redirect_devnull_streaming` (compound + callback) - Spec tests: `redirect_stderr_suppress_ls`, `redirect_stderr_suppress_compound`, `redirect_combined_suppress_ls` Closes #1116
1 parent eb874b7 commit 7f642b0

File tree

3 files changed

+99
-1
lines changed

3 files changed

+99
-1
lines changed

crates/bashkit/src/interpreter/mod.rs

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1367,7 +1367,29 @@ impl Interpreter {
13671367
} else {
13681368
None
13691369
};
1370+
1371+
// Suspend output callback while output redirects are active
1372+
// so that maybe_emit_output inside the compound body does not
1373+
// leak output that will be redirected (e.g. `{ cmd; } 2>/dev/null`).
1374+
let has_output_redirect = redirects.iter().any(|r| {
1375+
!matches!(
1376+
r.kind,
1377+
RedirectKind::Input | RedirectKind::HereDoc | RedirectKind::HereString
1378+
)
1379+
});
1380+
let saved_callback = if has_output_redirect {
1381+
self.output_callback.take()
1382+
} else {
1383+
None
1384+
};
1385+
13701386
let result = self.execute_compound(compound).await?;
1387+
1388+
// Restore callback before applying redirections
1389+
if let Some(cb) = saved_callback {
1390+
self.output_callback = Some(cb);
1391+
}
1392+
13711393
if let Some(prev) = prev_pipeline_stdin {
13721394
self.pipeline_stdin = prev;
13731395
}
@@ -3593,7 +3615,8 @@ impl Interpreter {
35933615
let err_msg = first.trim_start_matches("\x00ERR\x00").to_string();
35943616
self.last_exit_code = 1;
35953617
self.restore_variables(var_saves);
3596-
return Ok(ExecResult::err(err_msg, 1));
3618+
let result = ExecResult::err(err_msg, 1);
3619+
return self.apply_redirections(result, &command.redirects).await;
35973620
}
35983621

35993622
let xtrace_line = self.build_xtrace_line(&name, &args);
@@ -10853,4 +10876,26 @@ echo "count=$COUNT"
1085310876
let result = run_script(r#"cat <( x=5; (( x > 3 )) && echo YES )"#).await;
1085410877
assert_eq!(result.stdout.trim(), "YES");
1085510878
}
10879+
10880+
#[tokio::test]
10881+
async fn test_stderr_redirect_devnull_simple_and_compound() {
10882+
// Issue #1116: 2>/dev/null must suppress stderr from builtins
10883+
let result = run_script("ls /nonexistent 2>/dev/null; echo exit:$?").await;
10884+
assert_eq!(result.stderr, "", "simple: stderr should be suppressed");
10885+
assert_eq!(result.stdout.trim(), "exit:2");
10886+
10887+
// Compound command
10888+
let result = run_script("{ ls /nonexistent; } 2>/dev/null; echo exit:$?").await;
10889+
assert_eq!(result.stderr, "", "compound: stderr should be suppressed");
10890+
assert_eq!(result.stdout.trim(), "exit:2");
10891+
10892+
// &>/dev/null
10893+
let result = run_script("ls /nonexistent &>/dev/null; echo exit:$?").await;
10894+
assert_eq!(result.stderr, "", "&>: stderr should be suppressed");
10895+
assert_eq!(result.stdout.trim(), "exit:2");
10896+
10897+
// failglob + redirect
10898+
let result = run_script("shopt -s failglob; ls ./*.html 2>/dev/null; echo exit:$?").await;
10899+
assert_eq!(result.stderr, "", "failglob: stderr should be suppressed");
10900+
}
1085610901
}

crates/bashkit/src/lib.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5410,4 +5410,33 @@ echo missing fi"#,
54105410
// 25 doublings of 10 bytes = 335 544 320 without limits; must be capped ≤ 1024
54115411
assert!(len <= 1024, "string length {len} must be ≤ 1024");
54125412
}
5413+
5414+
/// Issue #1116: 2>/dev/null must suppress stderr in streaming mode
5415+
#[tokio::test]
5416+
async fn test_stderr_redirect_devnull_streaming() {
5417+
let stderr_chunks = Arc::new(Mutex::new(Vec::new()));
5418+
let stderr_cb = stderr_chunks.clone();
5419+
let mut bash = Bash::new();
5420+
5421+
// Compound command — the main bug: callback fired before redirect applied
5422+
let result = bash
5423+
.exec_streaming(
5424+
"{ ls /nonexistent; } 2>/dev/null; echo exit:$?",
5425+
Box::new(move |_stdout, stderr| {
5426+
if !stderr.is_empty() {
5427+
stderr_cb.lock().unwrap().push(stderr.to_string());
5428+
}
5429+
}),
5430+
)
5431+
.await
5432+
.unwrap();
5433+
5434+
assert_eq!(result.stderr, "", "final stderr should be empty");
5435+
let stderr_chunks = stderr_chunks.lock().unwrap();
5436+
assert!(
5437+
stderr_chunks.is_empty(),
5438+
"no stderr should be streamed when 2>/dev/null is used, got: {:?}",
5439+
*stderr_chunks
5440+
);
5441+
}
54135442
}

crates/bashkit/tests/spec_cases/bash/pipes-redirects.test.sh

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,30 @@ echo exit: $?
123123
exit: 1
124124
### end
125125

126+
### redirect_stderr_suppress_ls
127+
# Suppress stderr from ls with 2>/dev/null (issue #1116)
128+
ls /nonexistent 2>/dev/null
129+
echo exit: $?
130+
### expect
131+
exit: 2
132+
### end
133+
134+
### redirect_stderr_suppress_compound
135+
# Suppress stderr from compound command with 2>/dev/null (issue #1116)
136+
{ ls /nonexistent; } 2>/dev/null
137+
echo exit: $?
138+
### expect
139+
exit: 2
140+
### end
141+
142+
### redirect_combined_suppress_ls
143+
# Suppress both stdout and stderr with &>/dev/null (issue #1116)
144+
ls /nonexistent &>/dev/null
145+
echo exit: $?
146+
### expect
147+
exit: 2
148+
### end
149+
126150
### redirect_stderr_to_file_content
127151
# Redirect stderr content to file and verify it
128152
sleep abc 2>/tmp/sleep_err.txt

0 commit comments

Comments
 (0)