Merge PR #2882 from HUQIANTAO: security fixes in execution policy and approval mapping

This commit is contained in:
Hunter B
2026-06-07 10:21:00 -07:00
5 changed files with 54 additions and 20 deletions
+13 -8
View File
@@ -277,14 +277,19 @@ async fn tool_handler(
let cwd = req
.cwd
.unwrap_or_else(|| std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")));
match runtime
.invoke_tool(
req.call,
codewhale_execpolicy::AskForApproval::OnRequest,
&cwd,
)
.await
{
// Resolve approval policy from config instead of hardcoding.
let approval_mode = {
let cfg = state.config.read().await;
cfg.approval_policy
.as_deref()
.and_then(|p| match p.trim().to_ascii_lowercase().as_str() {
"auto" | "yolo" => Some(codewhale_execpolicy::AskForApproval::UnlessTrusted),
"never" | "deny" => Some(codewhale_execpolicy::AskForApproval::Never),
_ => None,
})
.unwrap_or(codewhale_execpolicy::AskForApproval::OnRequest)
};
match runtime.invoke_tool(req.call, approval_mode, &cwd).await {
Ok(value) => Json(value),
Err(err) => Json(json!({ "ok": false, "error": err.to_string() })),
}
+6 -3
View File
@@ -359,8 +359,9 @@ impl BashArityDict {
return true;
}
// Fallback: plain normalised prefix match for patterns not in the table
// (preserves backward compatibility with exact-match allow rules).
// Fallback: word-boundary prefix match for patterns not in the arity table.
// Matches the exact pattern or the pattern followed by a space (i.e., at
// word boundary), so "ls" matches "ls" and "ls -la" but NOT "lsof".
let command_lower = command.trim().to_ascii_lowercase();
// Normalise whitespace in both sides before comparing.
let pattern_norm: String = pattern_lower
@@ -371,7 +372,9 @@ impl BashArityDict {
.split_whitespace()
.collect::<Vec<_>>()
.join(" ");
command_norm == pattern_norm || command_norm.starts_with(&format!("{pattern_norm} "))
command_norm == pattern_norm
|| (command_norm.starts_with(&pattern_norm)
&& command_norm.as_bytes().get(pattern_norm.len()) == Some(&b' '))
}
/// Iterate over all entries in the dictionary.
+16 -6
View File
@@ -347,11 +347,15 @@ impl ExecPolicyEngine {
pub fn check(&self, ctx: ExecPolicyContext<'_>) -> Result<ExecPolicyDecision> {
let normalized = normalize_command(ctx.command);
let (trusted_prefixes, denied_prefixes) = self.resolve_prefixes();
// Deny rules use simple prefix matching (no arity semantics needed).
if let Some(rule) = denied_prefixes
.iter()
.find(|rule| normalized.starts_with(&normalize_command(rule)))
{
// Deny rules use word-boundary prefix matching: the command must either
// equal the rule or start with the rule followed by a space, so "rm"
// blocks "rm -rf /" but NOT "rmdir" or "rmview".
if let Some(rule) = denied_prefixes.iter().find(|rule| {
let norm_rule = normalize_command(rule);
normalized == norm_rule
|| (normalized.starts_with(&norm_rule)
&& normalized.as_bytes().get(norm_rule.len()) == Some(&b' '))
}) {
return Ok(ExecPolicyDecision {
allow: false,
requires_approval: false,
@@ -442,7 +446,13 @@ impl ExecPolicyEngine {
}
fn normalize_command(value: &str) -> String {
value.trim().to_ascii_lowercase()
// Normalize: lowercase, collapse internal whitespace to single spaces.
// This prevents bypass via "git status" (double space) vs "git status".
value
.split_whitespace()
.collect::<Vec<_>>()
.join(" ")
.to_ascii_lowercase()
}
fn first_token(command: &str) -> String {
+7 -2
View File
@@ -1106,13 +1106,18 @@ fn apply_hunk(
.collect();
// Try to find the location with fuzzy matching
// Apply cumulative offset from previous hunks
// Apply cumulative offset from previous hunks, clamping to valid range.
let base_idx = if hunk.old_start > 0 {
hunk.old_start - 1
} else {
0
};
let start_idx = ((base_idx as isize) + *cumulative_offset).max(0) as usize;
// Use checked_add_signed to safely handle negative offsets without
// risking isize overflow on adversarial input.
let start_idx = base_idx
.checked_add_signed(*cumulative_offset)
.unwrap_or(0)
.min(lines.len());
for fuzz in 0..=max_fuzz {
// Try at exact position first, then nearby
+12 -1
View File
@@ -726,7 +726,18 @@ fn leading_whitespace_fuzzy_matches(contents: &str, search: &str) -> Vec<(usize,
let Some(&mapped_start) = byte_map.get(norm_start) else {
break;
};
let original_start = line_start_before(contents, mapped_start);
// Use the actual match start position, expanding to line start only
// when the match begins at a line boundary in the normalized text.
// This prevents destroying preceding text on the same line when
// the match starts mid-line after whitespace stripping.
let original_start =
if norm_start == 0 || normalized_contents.as_bytes()[norm_start - 1] == b'\n' {
// Match starts at a line boundary — use line start for full-line replacement.
line_start_before(contents, mapped_start)
} else {
// Match starts mid-line — use the exact mapped position.
mapped_start
};
let original_end = byte_map.get(norm_end).copied().unwrap_or(contents.len());
matches.push((original_start, original_end));
cursor = norm_start.saturating_add(1);