Skip to content
Merged
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
78 changes: 70 additions & 8 deletions src/hooks/rewrite_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,22 @@ enum RewriteOutcome {
}

fn evaluate(cmd: &str, excluded: &[String], transparent_prefixes: &[String]) -> RewriteOutcome {
let verdict = check_command(cmd);
evaluate_with_verdict(cmd, check_command(cmd), excluded, transparent_prefixes)
}

/// Decision logic for [`evaluate`] with the permission verdict supplied by the
/// caller, mirroring [`check_command_with_rules`](super::permissions::check_command_with_rules).
///
/// `check_command` reads the machine's Claude Code settings files, so tests that
/// call [`evaluate`] directly would change verdict with the developer's local
/// `settings.local.json`. Taking the verdict as a parameter keeps the rewrite
/// logic under test independent of the host configuration (#3146).
fn evaluate_with_verdict(
cmd: &str,
verdict: PermissionVerdict,
excluded: &[String],
transparent_prefixes: &[String],
) -> RewriteOutcome {
if verdict == PermissionVerdict::Deny {
return RewriteOutcome::Deny;
}
Expand Down Expand Up @@ -90,53 +104,101 @@ mod tests {
);
}

/// The verdict still drives the outcome: an allow rule yields `Allow`.
/// Pinning both directions keeps the mapping covered without depending
/// on which rules the developer happens to have configured.
#[test]
fn test_allow_verdict_yields_allow() {
assert!(matches!(
evaluate_with_verdict("git status", PermissionVerdict::Allow, &[], &[]),
RewriteOutcome::Allow(_)
));
}

#[test]
fn test_deny_verdict_yields_deny() {
assert_eq!(
evaluate_with_verdict("git status", PermissionVerdict::Deny, &[], &[]),
RewriteOutcome::Deny
);
}

/// Commands with an unattestable construct are always a passthrough,
/// regardless of permission verdict.
///
/// The verdict is pinned to `Default` rather than going through `evaluate`,
/// which reads the developer's own `.claude/settings.local.json`: a
/// `Bash(git *)` allow rule turns the expected `Ask` into `Allow` and these
/// tests fail on that machine only (#3146). Pinning the verdict keeps the
/// assertions about the rewrite logic and nothing about the host.
mod unattestable_passthrough {
use super::super::{evaluate, RewriteOutcome};
use super::super::{evaluate_with_verdict, RewriteOutcome};
use crate::hooks::permissions::PermissionVerdict;

#[test]
fn test_backtick_substitution_passthrough() {
assert_eq!(
evaluate("git status `rm -rf /tmp/x`", &[], &[]),
evaluate_with_verdict(
"git status `rm -rf /tmp/x`",
PermissionVerdict::Default,
&[],
&[]
),
RewriteOutcome::Passthrough
);
}

#[test]
fn test_dollar_substitution_passthrough() {
assert_eq!(
evaluate("git status $(rm -rf /tmp/x)", &[], &[]),
evaluate_with_verdict(
"git status $(rm -rf /tmp/x)",
PermissionVerdict::Default,
&[],
&[]
),
RewriteOutcome::Passthrough
);
}

#[test]
fn test_double_quoted_substitution_passthrough() {
assert_eq!(
evaluate("git log --pretty=\"$(rm -rf /tmp/x)\"", &[], &[]),
evaluate_with_verdict(
"git log --pretty=\"$(rm -rf /tmp/x)\"",
PermissionVerdict::Default,
&[],
&[]
),
RewriteOutcome::Passthrough
);
}

#[test]
fn test_file_redirect_passthrough() {
assert_eq!(
evaluate("git log > /tmp/out.txt", &[], &[]),
evaluate_with_verdict(
"git log > /tmp/out.txt",
PermissionVerdict::Default,
&[],
&[]
),
RewriteOutcome::Passthrough
);
}

#[test]
fn test_fd_dup_redirect_still_rewrites() {
assert!(matches!(
evaluate("git status 2>&1", &[], &[]),
evaluate_with_verdict("git status 2>&1", PermissionVerdict::Default, &[], &[]),
RewriteOutcome::Ask(_)
));
}

#[test]
fn test_plain_command_still_rewrites() {
assert!(matches!(
evaluate("git status", &[], &[]),
evaluate_with_verdict("git status", PermissionVerdict::Default, &[], &[]),
RewriteOutcome::Ask(_)
));
}
Expand Down
Loading