diff --git a/crates/execpolicy/src/matcher.rs b/crates/execpolicy/src/matcher.rs index e54e6b63df..4e40d4b5e4 100644 --- a/crates/execpolicy/src/matcher.rs +++ b/crates/execpolicy/src/matcher.rs @@ -1,7 +1,34 @@ //! Command matching helpers for execpolicy rules. +use std::collections::HashMap; +use std::sync::{Arc, Mutex, OnceLock}; + use regex::Regex; +/// `pattern` compiled once as an anchored `*`-glob, then reused. +/// +/// Every other regex metacharacter is escaped, so `*` is the only wildcard +/// (matching any run of characters, newline included). `None` means the pattern +/// does not compile; callers treat that as "no match". +/// +/// The patterns come from configuration and are stable between edits, but the +/// callers run per shell execution and per hook event. Compiling here on first +/// use keeps the fast path free of `Regex::new` without changing what matches. +pub fn compiled_glob(pattern: &str) -> Option> { + static CACHE: OnceLock>>>> = OnceLock::new(); + let cache = CACHE.get_or_init(|| Mutex::new(HashMap::new())); + let mut cache = cache + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + cache + .entry(pattern.to_string()) + .or_insert_with(|| { + let escaped = regex::escape(pattern).replace(r"\*", ".*"); + Regex::new(&format!("^{escaped}$")).ok().map(Arc::new) + }) + .clone() +} + /// Normalize a command string by shlex parsing and re-joining tokens. /// /// Strips heredoc bodies first (#419) so a command like @@ -110,11 +137,7 @@ pub fn pattern_matches(pattern: &str, command: &str) -> bool { return true; } - let escaped = regex::escape(&pattern).replace("\\*", ".*"); - let Ok(re) = Regex::new(&format!("^{escaped}$")) else { - return false; - }; - re.is_match(&command) + compiled_glob(&pattern).is_some_and(|re| re.is_match(&command)) } #[cfg(test)] @@ -195,4 +218,28 @@ mod tests { let normalized = normalize_command("cat < file.txt\nbody\nEOF"); assert!(pattern_matches("cat > file.txt", &normalized)); } + + #[test] + fn compiled_glob_is_compiled_once_per_pattern() { + // The per-shell-execution and per-hook-event callers rely on this + // returning the same compiled program rather than rebuilding it. + let first = compiled_glob("mcp__*").expect("glob compiles"); + let second = compiled_glob("mcp__*").expect("glob compiles"); + assert!(std::sync::Arc::ptr_eq(&first, &second)); + assert!(first.is_match("mcp__github__search")); + assert!(!first.is_match("read_file")); + } + + #[test] + fn compiled_glob_escapes_every_metacharacter_except_star() { + // `regex::escape` is what makes `a.b` a literal while `*` stays a + // wildcard — the same contract `pattern_matches` has always had. + let literal = compiled_glob("a.b").expect("glob compiles"); + assert!(literal.is_match("a.b")); + assert!(!literal.is_match("axb")); + + let wildcard = compiled_glob("a*b").expect("glob compiles"); + assert!(wildcard.is_match("ab")); + assert!(wildcard.is_match("a middle b")); + } } diff --git a/crates/execpolicy/src/toml_rules.rs b/crates/execpolicy/src/toml_rules.rs index 79a904e13a..5239d3d039 100644 --- a/crates/execpolicy/src/toml_rules.rs +++ b/crates/execpolicy/src/toml_rules.rs @@ -30,13 +30,13 @@ pub enum RuleDecision { AskUser(String), } -#[derive(Debug, Deserialize, Default)] +#[derive(Debug, Clone, Deserialize, Default)] pub struct ExecPolicyConfig { #[serde(default)] pub rules: BTreeMap, } -#[derive(Debug, Deserialize, Default)] +#[derive(Debug, Clone, Deserialize, Default)] pub struct RuleSet { #[serde(default)] pub allow: Vec, diff --git a/crates/tui/src/cloud_dispatch.rs b/crates/tui/src/cloud_dispatch.rs index 17424797ca..75d5e3f619 100644 --- a/crates/tui/src/cloud_dispatch.rs +++ b/crates/tui/src/cloud_dispatch.rs @@ -1366,7 +1366,7 @@ pub struct LiveDaytonaLauncher; impl LiveDaytonaLauncher { /// Total timeout for short control-plane calls (create/status/delete/ /// list). A dispatched harness turn is NOT a short call — see - /// [`Self::harness_client`]. + /// [`Self::harness_client_budget_secs`]. const CONTROL_PLANE_TIMEOUT_SECS: u64 = 120; /// Slack added to a harness command's declared budget for the client @@ -1376,25 +1376,26 @@ impl LiveDaytonaLauncher { const HARNESS_CLIENT_SLACK_SECS: u64 = 120; fn blocking_client() -> Result { - Self::blocking_client_with_timeout(Self::CONTROL_PLANE_TIMEOUT_SECS) - } - - fn blocking_client_with_timeout(total_secs: u64) -> Result { - crate::tls::reqwest_blocking_client_builder() - .connect_timeout(std::time::Duration::from_secs(8)) - .timeout(std::time::Duration::from_secs(total_secs)) - .redirect(reqwest::redirect::Policy::none()) - .build() - .context("failed to initialize the cloud agent client") - } - - /// A client scoped to one harness command: its total timeout is the - /// command's declared budget plus fixed slack. The declared turn budget - /// is an hour, so the 120s control-plane cap must NOT carry this call — - /// otherwise every dispatched turn longer than two minutes fails after - /// the spend has already started. - fn harness_client(command: &HarnessCommand) -> Result { - Self::blocking_client_with_timeout(Self::harness_client_budget_secs(command)) + // #6208: one client for the process. A `reqwest` client owns a + // connection pool and a TLS configuration, so building one per call + // paid a fresh TCP+TLS handshake on all nine call sites (one of them a + // poll loop). `clone()` here is a refcount bump on that shared pool. + // + // The total timeout is attached per request instead, because a harness + // turn carries a budget derived from its own command and must not + // inherit the control-plane cap. + static CLIENT: std::sync::OnceLock> = + std::sync::OnceLock::new(); + CLIENT + .get_or_init(|| { + crate::tls::reqwest_blocking_client_builder() + .connect_timeout(std::time::Duration::from_secs(8)) + .redirect(reqwest::redirect::Policy::none()) + .build() + .map_err(|error| error.to_string()) + }) + .clone() + .map_err(|message| anyhow!("failed to initialize the cloud agent client: {message}")) } /// The total-timeout budget for a harness-carrying client, in seconds. @@ -1453,14 +1454,22 @@ impl LiveDaytonaLauncher { api_key: &str, body: serde_json::Value, ) -> Result { - Self::send_json_on(&Self::blocking_client()?, method, url, api_key, body) + Self::send_json_on( + &Self::blocking_client()?, + Self::CONTROL_PLANE_TIMEOUT_SECS, + method, + url, + api_key, + body, + ) } - /// [`Self::send_json`] on a caller-supplied client, so a call whose - /// declared budget differs from the control-plane cap (the harness - /// turn) can carry a client scoped to its own budget. + /// [`Self::send_json`] with an explicit total timeout, so a call whose + /// declared budget differs from the control-plane cap (the harness turn) + /// can carry its own without needing a client of its own. fn send_json_on( client: &reqwest::blocking::Client, + total_secs: u64, method: reqwest::Method, url: &reqwest::Url, api_key: &str, @@ -1468,6 +1477,7 @@ impl LiveDaytonaLauncher { ) -> Result { client .request(method, url.clone()) + .timeout(std::time::Duration::from_secs(total_secs)) .bearer_auth(api_key) .json(&body) .send() @@ -1639,10 +1649,16 @@ impl DaytonaLauncher for LiveDaytonaLauncher { "timeout": command.timeout_secs, }); // This call carries the declared turn budget (an hour for the agent - // entry), so it rides a client scoped to that budget plus slack — - // never the 120s control-plane client that used to cap it. - let client = Self::harness_client(command)?; - let response = Self::send_json_on(&client, reqwest::Method::POST, &url, &api_key, body)?; + // entry), so it asks for that budget plus slack per request — never the + // 120s control-plane cap that used to bound it. + let response = Self::send_json_on( + &Self::blocking_client()?, + Self::harness_client_budget_secs(command), + reqwest::Method::POST, + &url, + &api_key, + body, + )?; let status = response.status(); let text = response.text().unwrap_or_default(); if !status.is_success() { diff --git a/crates/tui/src/hooks/executor.rs b/crates/tui/src/hooks/executor.rs index ce2bc08a34..ca4cb17af2 100644 --- a/crates/tui/src/hooks/executor.rs +++ b/crates/tui/src/hooks/executor.rs @@ -2059,11 +2059,11 @@ impl HookExecutor { if !pattern.contains('*') { return tool_name == pattern; } - // Escape regex metacharacters except `*`, which becomes `.*`. - let escaped = regex::escape(pattern); - let regex_pattern = escaped.replace(r"\*", ".*"); - let anchored = format!("^{regex_pattern}$"); - regex::Regex::new(&anchored).is_ok_and(|re| re.is_match(tool_name)) + // #6208: the pattern is fixed by configuration while this runs once per + // hook per tool-call/stop event, so compile it once and reuse it rather + // than building a fresh `Regex` on every event. + codewhale_execpolicy::matcher::compiled_glob(pattern) + .is_some_and(|re| re.is_match(tool_name)) } /// Check if a hook's condition matches the context diff --git a/crates/tui/src/tools/shell.rs b/crates/tui/src/tools/shell.rs index a9efdd1a78..c9498d8c0f 100644 --- a/crates/tui/src/tools/shell.rs +++ b/crates/tui/src/tools/shell.rs @@ -3534,13 +3534,40 @@ fn default_execpolicy_path() -> Option { } fn load_default_policy() -> anyhow::Result> { + /// A parsed rules file, tagged with the identity it was parsed from. + type PolicyKey = (std::path::PathBuf, u64, Option); + let Some(path) = default_execpolicy_path() else { return Ok(None); }; - if !path.exists() { + // An unreadable or missing file (including a permissions error, which + // `exists()` also swallows) means "no file rules" — the same answer as + // before, just reached with one `stat` instead of an existence check plus a + // full read. + let Ok(metadata) = std::fs::metadata(&path) else { return Ok(None); + }; + let key: PolicyKey = (path.clone(), metadata.len(), metadata.modified().ok()); + + // #6208: this runs on every shell execution, so the read and TOML parse + // happen only when the file's identity changes. Length joins the timestamp + // because a coarse-mtime filesystem can hand back the same instant for two + // different revisions. + static CACHE: std::sync::OnceLock>> = + std::sync::OnceLock::new(); + let cache = CACHE.get_or_init(|| std::sync::Mutex::new(None)); + let mut cache = cache + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + if let Some((cached_key, config)) = cache.as_ref() + && *cached_key == key + { + return Ok(Some(config.clone())); } - ExecPolicyConfig::from_path(&path).map(Some) + + let config = ExecPolicyConfig::from_path(&path)?; + *cache = Some((key, config.clone())); + Ok(Some(config)) } const FOREGROUND_TIMEOUT_RECOVERY_HINT: &str = "Foreground Bash is for bounded commands. \ diff --git a/crates/tui/src/tui/history/tool_output.rs b/crates/tui/src/tui/history/tool_output.rs index be1aa03563..99c36f2f6b 100644 --- a/crates/tui/src/tui/history/tool_output.rs +++ b/crates/tui/src/tui/history/tool_output.rs @@ -338,13 +338,21 @@ pub fn summarize_mcp_output(output: &str) -> McpOutputSummary { #[must_use] pub fn output_is_image(output: &str) -> bool { - let lower = output.to_lowercase(); - - [ + // Sniff the extensions case-insensitively over the raw bytes. Lowercasing + // the whole output first copied every byte of a payload that can run to + // hundreds of kilobytes, once per MCP completion, to answer a question + // about eight ASCII suffixes. + const IMAGE_EXTENSIONS: [&str; 8] = [ ".png", ".jpg", ".jpeg", ".gif", ".webp", ".bmp", ".tiff", ".ppm", - ] - .iter() - .any(|ext| lower.contains(ext)) + ]; + + let bytes = output.as_bytes(); + IMAGE_EXTENSIONS.iter().any(|ext| { + let needle = ext.as_bytes(); + bytes + .windows(needle.len()) + .any(|window| window.eq_ignore_ascii_case(needle)) + }) } fn render_preserved_output_mode( @@ -903,4 +911,18 @@ mod ansi_colour_tests { assert_eq!(rows[0].text, "done"); assert!(rows[0].styled.is_none()); } + + #[test] + fn image_sniffing_is_case_insensitive_over_the_raw_bytes() { + assert!(output_is_image("saved to /tmp/Chart.PNG")); + assert!(output_is_image("shot.jpeg")); + assert!(output_is_image(".WEBP")); + assert!(!output_is_image("no image here")); + // The historical `contains` contract is preserved: a name that merely + // embeds an extension still counts. + assert!(output_is_image("weird.pngx")); + // A byte-window scan must not panic on multi-byte text. + assert!(!output_is_image("日本語のテキストのみ")); + assert!(!output_is_image("")); + } } diff --git a/crates/tui/src/working_set.rs b/crates/tui/src/working_set.rs index a61b2f41e8..a6f8c17ea2 100644 --- a/crates/tui/src/working_set.rs +++ b/crates/tui/src/working_set.rs @@ -537,7 +537,7 @@ impl Workspace { entries.push(candidate); } - entries.sort_by_key(|entry| entry.to_lowercase()); + entries.sort_by_cached_key(|entry| entry.to_lowercase()); entries } }