From 75593a0eacc795cbe27bdf98faac7d93259a63ef Mon Sep 17 00:00:00 2001
From: huqiantao <huqiantao@users.noreply.github.com>
Date: Sun, 7 Jun 2026 19:35:20 +0800
Subject: [PATCH] fix: address security review comments

1. Fix whitespace bypass in normalize_command (execpolicy/lib.rs:446)
   - Collapse internal whitespace to prevent 'git  status' bypassing 'git status'
   - split_whitespace().join(' ') normalizes all whitespace

2. Fix 'never'/'deny' approval mapping (app-server/lib.rs:287)
   - Map to AskForApproval::Never instead of OnRequest
   - 'never'/'deny' should forbid commands, not prompt for approval

3. Optimize prefix matching (execpolicy/lib.rs:355, bash_arity.rs:375)
   - Avoid format! allocation on every check
   - Use byte comparison for space boundary check
---
 crates/app-server/src/lib.rs        |  2 +-
 crates/execpolicy/src/bash_arity.rs |  3 ++-
 crates/execpolicy/src/lib.rs        | 11 +++++++++--
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/crates/app-server/src/lib.rs b/crates/app-server/src/lib.rs
index bb5d7dcf..40fcca49 100644
--- a/crates/app-server/src/lib.rs
+++ b/crates/app-server/src/lib.rs
@@ -284,7 +284,7 @@ async fn tool_handler(
             .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::OnRequest),
+                "never" | "deny" => Some(codewhale_execpolicy::AskForApproval::Never),
                 _ => None,
             })
             .unwrap_or(codewhale_execpolicy::AskForApproval::OnRequest)
diff --git a/crates/execpolicy/src/bash_arity.rs b/crates/execpolicy/src/bash_arity.rs
index 467c1640..e87afa30 100644
--- a/crates/execpolicy/src/bash_arity.rs
+++ b/crates/execpolicy/src/bash_arity.rs
@@ -373,7 +373,8 @@ impl BashArityDict {
             .collect::<Vec<_>>()
             .join(" ");
         command_norm == pattern_norm
-            || command_norm.starts_with(&format!("{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.
diff --git a/crates/execpolicy/src/lib.rs b/crates/execpolicy/src/lib.rs
index 05ed7a4e..2d7b0b02 100644
--- a/crates/execpolicy/src/lib.rs
+++ b/crates/execpolicy/src/lib.rs
@@ -352,7 +352,9 @@ impl ExecPolicyEngine {
         // 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(&format!("{norm_rule} "))
+            normalized == norm_rule
+                || (normalized.starts_with(&norm_rule)
+                    && normalized.as_bytes().get(norm_rule.len()) == Some(&b' '))
         }) {
             return Ok(ExecPolicyDecision {
                 allow: false,
@@ -444,7 +446,12 @@ 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 {