fix(security): allow 2>/dev/null and 2>&1 in shell commands, add policy logging
The redirect blocker was rejecting safe stderr patterns like 2>/dev/null and 2>&1. Strip these before operator checks so they don't trigger the generic > or & blockers. Also adds debug/trace logging to all early rejection paths in is_command_allowed for audit visibility. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
5b896f3378
commit
0e5215e1ef
1 changed files with 47 additions and 9 deletions
|
|
@ -184,6 +184,19 @@ fn contains_single_ampersand(s: &str) -> bool {
|
||||||
false
|
false
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Strip safe stderr redirection patterns before policy checks.
|
||||||
|
///
|
||||||
|
/// Removes `2>/dev/null`, `2> /dev/null`, and `2>&1` so they don't
|
||||||
|
/// trigger the generic `>` or `&` blockers.
|
||||||
|
fn strip_safe_stderr(s: &str) -> String {
|
||||||
|
let mut result = s.to_string();
|
||||||
|
// Order matters: longest patterns first
|
||||||
|
for pat in ["2> /dev/null", "2>/dev/null", "2>&1"] {
|
||||||
|
result = result.replace(pat, "");
|
||||||
|
}
|
||||||
|
result
|
||||||
|
}
|
||||||
|
|
||||||
impl SecurityPolicy {
|
impl SecurityPolicy {
|
||||||
/// Classify command risk. Any high-risk segment marks the whole command high.
|
/// Classify command risk. Any high-risk segment marks the whole command high.
|
||||||
pub fn command_risk_level(&self, command: &str) -> CommandRiskLevel {
|
pub fn command_risk_level(&self, command: &str) -> CommandRiskLevel {
|
||||||
|
|
@ -359,37 +372,46 @@ impl SecurityPolicy {
|
||||||
/// - Blocks dangerous arguments (e.g. `find -exec`, `git config`)
|
/// - Blocks dangerous arguments (e.g. `find -exec`, `git config`)
|
||||||
pub fn is_command_allowed(&self, command: &str) -> bool {
|
pub fn is_command_allowed(&self, command: &str) -> bool {
|
||||||
if self.autonomy == AutonomyLevel::ReadOnly {
|
if self.autonomy == AutonomyLevel::ReadOnly {
|
||||||
|
tracing::trace!(command, "Command blocked: read-only mode");
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Strip safe stderr redirections (2>/dev/null, 2>&1) before
|
||||||
|
// operator checks so they don't trigger the generic `>` or `&` blockers.
|
||||||
|
let sanitized = strip_safe_stderr(command);
|
||||||
|
|
||||||
// Block subshell/expansion operators — these allow hiding arbitrary
|
// Block subshell/expansion operators — these allow hiding arbitrary
|
||||||
// commands inside an allowed command (e.g. `echo $(rm -rf /)`)
|
// commands inside an allowed command (e.g. `echo $(rm -rf /)`)
|
||||||
if command.contains('`')
|
if sanitized.contains('`')
|
||||||
|| command.contains("$(")
|
|| sanitized.contains("$(")
|
||||||
|| command.contains("${")
|
|| sanitized.contains("${")
|
||||||
|| command.contains("<(")
|
|| sanitized.contains("<(")
|
||||||
|| command.contains(">(")
|
|| sanitized.contains(">(")
|
||||||
{
|
{
|
||||||
|
tracing::debug!(command, "Command blocked: subshell/expansion operator");
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Block output redirections — they can write to arbitrary paths
|
// Block output redirections that write to arbitrary paths.
|
||||||
if command.contains('>') {
|
if sanitized.contains('>') {
|
||||||
|
tracing::debug!(command, "Command blocked: output redirection");
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Block `tee` — it can write to arbitrary files, bypassing the
|
// Block `tee` — it can write to arbitrary files, bypassing the
|
||||||
// redirect check above (e.g. `echo secret | tee /etc/crontab`)
|
// redirect check above (e.g. `echo secret | tee /etc/crontab`)
|
||||||
if command
|
if sanitized
|
||||||
.split_whitespace()
|
.split_whitespace()
|
||||||
.any(|w| w == "tee" || w.ends_with("/tee"))
|
.any(|w| w == "tee" || w.ends_with("/tee"))
|
||||||
{
|
{
|
||||||
|
tracing::debug!(command, "Command blocked: tee can write arbitrary files");
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Block background command chaining (`&`), which can hide extra
|
// Block background command chaining (`&`), which can hide extra
|
||||||
// sub-commands and outlive timeout expectations. Keep `&&` allowed.
|
// sub-commands and outlive timeout expectations. Keep `&&` allowed.
|
||||||
if contains_single_ampersand(command) {
|
if contains_single_ampersand(&sanitized) {
|
||||||
|
tracing::debug!(command, "Command blocked: background & operator");
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -1107,6 +1129,22 @@ mod tests {
|
||||||
let p = default_policy();
|
let p = default_policy();
|
||||||
assert!(!p.is_command_allowed("echo secret > /etc/crontab"));
|
assert!(!p.is_command_allowed("echo secret > /etc/crontab"));
|
||||||
assert!(!p.is_command_allowed("ls >> /tmp/exfil.txt"));
|
assert!(!p.is_command_allowed("ls >> /tmp/exfil.txt"));
|
||||||
|
assert!(!p.is_command_allowed("cat file > /tmp/out"));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn stderr_to_devnull_allowed() {
|
||||||
|
let p = SecurityPolicy {
|
||||||
|
allowed_commands: vec!["*".into()],
|
||||||
|
..SecurityPolicy::default()
|
||||||
|
};
|
||||||
|
assert!(p.is_command_allowed("ls -la /tmp/*.py 2>/dev/null"));
|
||||||
|
assert!(p.is_command_allowed("ls -la 2> /dev/null"));
|
||||||
|
assert!(p.is_command_allowed("grep pattern file 2>&1"));
|
||||||
|
assert!(p.is_command_allowed("cmd 2>/dev/null | grep foo"));
|
||||||
|
// Stdout redirect still blocked
|
||||||
|
assert!(!p.is_command_allowed("echo hello > /tmp/file"));
|
||||||
|
assert!(!p.is_command_allowed("echo hello 1> /tmp/file"));
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue