fix: remove reachable panics from untrusted remote and repo input

crates/code-integrations/src/mcp/stdio.rs

@@ -7,7 +7,7 @@ use std::time::Duration;
 use serde::de::DeserializeOwned;
 use serde::{Deserialize, Serialize};
 use serde_json::Value as JsonValue;
-use tokio::io::{AsyncBufReadExt, AsyncReadExt, AsyncWriteExt, BufReader};
+use tokio::io::{AsyncBufRead, AsyncBufReadExt, AsyncReadExt, AsyncWriteExt, BufReader};
 use tokio::process::{Child, ChildStdin, ChildStdout, Command};
 use tokio::time::timeout;
 
@@ -1206,37 +1206,7 @@ impl McpStdioProcess {
     }
 
     pub async fn read_frame(&mut self) -> io::Result<Vec<u8>> {
-        let mut content_length = None;
-        loop {
-            let mut line = String::new();
-            let bytes_read = self.stdout.read_line(&mut line).await?;
-            if bytes_read == 0 {
-                return Err(io::Error::new(
-                    io::ErrorKind::UnexpectedEof,
-                    "MCP stdio stream closed while reading headers",
-                ));
-            }
-            if line == "\r\n" {
-                break;
-            }
-            let header = line.trim_end_matches(['\r', '\n']);
-            if let Some((name, value)) = header.split_once(':') {
-                if name.trim().eq_ignore_ascii_case("Content-Length") {
-                    let parsed = value
-                        .trim()
-                        .parse::<usize>()
-                        .map_err(|error| io::Error::new(io::ErrorKind::InvalidData, error))?;
-                    content_length = Some(parsed);
-                }
-            }
-        }
-
-        let content_length = content_length.ok_or_else(|| {
-            io::Error::new(io::ErrorKind::InvalidData, "missing Content-Length header")
-        })?;
-        let mut payload = vec![0_u8; content_length];
-        self.stdout.read_exact(&mut payload).await?;
-        Ok(payload)
+        read_frame_from(&mut self.stdout).await
     }
 
     pub async fn write_jsonrpc_message<T: Serialize>(&mut self, message: &T) -> io::Result<()> {
@@ -1380,6 +1350,52 @@ fn apply_env(command: &mut Command, env: &BTreeMap<String, String>) {
     }
 }
 
+/// Reads one `Content-Length` framed message. The length comes from the peer, so the payload
+/// is read through `take` rather than preallocated: a huge declared length must not panic
+/// (`vec!` capacity overflow) or abort on allocation. A short stream is an error.
+async fn read_frame_from<R: AsyncBufRead + Unpin>(reader: &mut R) -> io::Result<Vec<u8>> {
+    let mut content_length = None;
+    loop {
+        let mut line = String::new();
+        let bytes_read = reader.read_line(&mut line).await?;
+        if bytes_read == 0 {
+            return Err(io::Error::new(
+                io::ErrorKind::UnexpectedEof,
+                "MCP stdio stream closed while reading headers",
+            ));
+        }
+        if line == "\r\n" {
+            break;
+        }
+        let header = line.trim_end_matches(['\r', '\n']);
+        if let Some((name, value)) = header.split_once(':') {
+            if name.trim().eq_ignore_ascii_case("Content-Length") {
+                let parsed = value
+                    .trim()
+                    .parse::<usize>()
+                    .map_err(|error| io::Error::new(io::ErrorKind::InvalidData, error))?;
+                content_length = Some(parsed);
+            }
+        }
+    }
+
+    let content_length = content_length.ok_or_else(|| {
+        io::Error::new(io::ErrorKind::InvalidData, "missing Content-Length header")
+    })?;
+    let mut payload = Vec::new();
+    let read = (&mut *reader)
+        .take(u64::try_from(content_length).unwrap_or(u64::MAX))
+        .read_to_end(&mut payload)
+        .await?;
+    if read != content_length {
+        return Err(io::Error::new(
+            io::ErrorKind::UnexpectedEof,
+            "MCP stdio stream closed while reading payload",
+        ));
+    }
+    Ok(payload)
+}
+
 fn encode_frame(payload: &[u8]) -> Vec<u8> {
     let header = format!("Content-Length: {}\r\n\r\n", payload.len());
     let mut framed = header.into_bytes();
@@ -1404,6 +1420,39 @@ mod tests {
     use std::fs;
     use std::io::ErrorKind;
     use std::os::unix::fs::PermissionsExt;
+
+    #[test]
+    fn huge_declared_content_length_returns_error_instead_of_panicking() {
+        // A hostile peer declares a payload far larger than it sends. Preallocating the
+        // declared size used to panic ("capacity overflow") or abort on allocation.
+        let runtime = tokio::runtime::Builder::new_current_thread()
+            .enable_all()
+            .build()
+            .expect("runtime");
+        let wire: &[u8] = b"Content-Length: 18446744073709551615\r\n\r\n{\"partial\":true}";
+        let result = runtime.block_on(async {
+            let mut reader = tokio::io::BufReader::new(wire);
+            super::read_frame_from(&mut reader).await
+        });
+        let error = result.expect_err("oversized frame must fail");
+        assert_eq!(error.kind(), ErrorKind::UnexpectedEof);
+    }
+
+    #[test]
+    fn reads_well_formed_content_length_frame() {
+        let runtime = tokio::runtime::Builder::new_current_thread()
+            .enable_all()
+            .build()
+            .expect("runtime");
+        let wire: &[u8] = b"Content-Length: 2\r\n\r\n{}";
+        let payload = runtime
+            .block_on(async {
+                let mut reader = tokio::io::BufReader::new(wire);
+                super::read_frame_from(&mut reader).await
+            })
+            .expect("frame should parse");
+        assert_eq!(payload, b"{}");
+    }
     use std::path::{Path, PathBuf};
     use std::sync::atomic::{AtomicU64, Ordering};
     use std::time::{SystemTime, UNIX_EPOCH};

crates/code-integrations/src/mcp/utils.rs

@@ -175,16 +175,28 @@ fn collapse_underscores(value: &str) -> String {
     collapsed
 }
 
+fn hex_nibble(byte: u8) -> Option<u8> {
+    match byte {
+        b'0'..=b'9' => Some(byte - b'0'),
+        b'a'..=b'f' => Some(byte - b'a' + 10),
+        b'A'..=b'F' => Some(byte - b'A' + 10),
+        _ => None,
+    }
+}
+
 fn percent_decode(value: &str) -> String {
     let bytes = value.as_bytes();
     let mut decoded = Vec::with_capacity(bytes.len());
     let mut index = 0;
     while index < bytes.len() {
         match bytes[index] {
             b'%' if index + 2 < bytes.len() => {
-                let hex = &value[index + 1..index + 3];
-                if let Ok(byte) = u8::from_str_radix(hex, 16) {
-                    decoded.push(byte);
+                // Work on bytes, not `&value[..]`: slicing the str panics when the two bytes
+                // after '%' are part of a multi-byte character.
+                if let (Some(high), Some(low)) =
+                    (hex_nibble(bytes[index + 1]), hex_nibble(bytes[index + 2]))
+                {
+                    decoded.push((high << 4) | low);
                     index += 3;
                     continue;
                 }
@@ -242,6 +254,20 @@ mod tests {
         );
     }
 
+    #[test]
+    fn unwrapping_ccr_proxy_url_with_multibyte_percent_sequence_does_not_panic() {
+        // In "%aé" the escape window after '%' ends in the middle of the 2-byte 'é'.
+        // Slicing the str there used to panic. The malformed escape must be kept literally.
+        let malformed = "https://api.anthropic.com/v2/ccr-sessions/1?mcp_url=%aé";
+        assert_eq!(unwrap_ccr_proxy_url(malformed), "%aé");
+
+        let wrapped = "https://api.anthropic.com/v2/ccr-sessions/1?mcp_url=https%3A%2F%2Fvendor.example%2F%zz%é";
+        assert_eq!(
+            unwrap_ccr_proxy_url(wrapped),
+            "https://vendor.example/%zz%é"
+        );
+    }
+
     #[test]
     fn computes_signatures_for_stdio_and_remote_servers() {
         let stdio = McpServerConfig::Stdio(McpStdioServerConfig {

crates/code-runtime/src/bash_validation.rs

@@ -105,6 +105,15 @@ pub fn validate_read_only(command: &str, mode: PermissionMode) -> ValidationResu
         return ValidationResult::Allow;
     }
 
+    // bash splits words only on space, tab and newline. This parser's trimming and
+    // splitting use Unicode whitespace, so any other whitespace character can make it
+    // report a different command from the one bash runs. Refuse those commands instead.
+    if has_non_bash_whitespace(command) {
+        return ValidationResult::Block {
+            reason: "Command contains whitespace other than space, tab, or newline and cannot be checked in read-only mode".to_string(),
+        };
+    }
+
     let first_command = extract_first_command(command);
 
     // Check for write commands.
@@ -618,6 +627,14 @@ pub fn validate_command(command: &str, mode: PermissionMode, workspace: &Path) -
 // Helpers
 // ---------------------------------------------------------------------------
 
+/// True when the command contains a whitespace character that bash does not treat as a
+/// word separator (for example U+00A0, U+3000, `\r`, or `\x0b`).
+fn has_non_bash_whitespace(command: &str) -> bool {
+    command
+        .chars()
+        .any(|c| c.is_whitespace() && !matches!(c, ' ' | '\t' | '\n'))
+}
+
 /// Extract the first bare command from a pipeline/chain, stripping env vars and sudo.
 fn extract_first_command(command: &str) -> String {
     let trimmed = command.trim();
@@ -634,10 +651,18 @@ fn extract_first_command(command: &str) -> String {
                     .chars()
                     .all(|c| c.is_ascii_alphanumeric() || c == '_')
             {
-                // Skip past the value (might be quoted).
-                let after_eq = &next[eq_pos + 1..];
-                if let Some(space) = find_end_of_value(after_eq) {
-                    remaining = &after_eq[space..];
+                // The value starts immediately after `=`. If that is whitespace (or nothing),
+                // the assignment has an empty value and the next word is the command, so it
+                // must not be consumed as the value. Trimming before measuring would do that
+                // and hide the real command from the permission check.
+                let value_start = &next[eq_pos + 1..];
+                if value_start.is_empty() || value_start.starts_with(char::is_whitespace) {
+                    remaining = value_start;
+                    continue;
+                }
+                // Skip past the value (might be quoted). Measure and slice the same string.
+                if let Some(space) = find_end_of_value(value_start) {
+                    remaining = &value_start[space..];
                     continue;
                 }
                 // No space found means value goes to end of string — no actual command.
@@ -1001,4 +1026,49 @@ mod tests {
     fn extracts_plain_command() {
         assert_eq!(extract_first_command("grep -r pattern ."), "grep");
     }
+
+    #[test]
+    fn env_prefix_with_whitespace_and_multibyte_quoted_value_does_not_panic() {
+        // Whitespace right after '=' makes the assignment empty, so the next word is the
+        // command (bash runs `'é'` here, not `ls`).
+        assert_eq!(extract_first_command("A=  'é' ls"), "'é'");
+        assert_eq!(extract_first_command("A=  \"é\" ls -la"), "\"é\"");
+    }
+
+    #[test]
+    fn empty_assignment_does_not_hide_the_real_command() {
+        // With `A=` followed by whitespace, bash runs the following word. Treating that word
+        // as the assignment's value would report the wrong command to the permission check.
+        assert_eq!(extract_first_command("A=  rm x"), "rm");
+        assert_eq!(extract_first_command("A= rm x"), "rm");
+        assert_eq!(extract_first_command("A=1 B=  rm x"), "rm");
+        assert_eq!(extract_first_command("A="), "");
+        assert_eq!(extract_first_command("A=1 B=2 echo hello"), "echo");
+    }
+
+    #[test]
+    fn read_only_refuses_whitespace_bash_does_not_split_on() {
+        // U+00A0 is whitespace to Rust but part of the word to bash. Here bash runs `rm`
+        // after an empty assignment, but a parser that skips the NBSP would report `ls`.
+        let mode = PermissionMode::ReadOnly;
+        for command in [
+            "A=\u{a0}ls rm -rf /tmp/x",
+            "ls\u{3000}-la",
+            "A=\rls rm x",
+            "A=\u{b}ls rm x",
+        ] {
+            assert!(
+                matches!(
+                    validate_read_only(command, mode),
+                    ValidationResult::Block { .. }
+                ),
+                "expected a block for {command:?}"
+            );
+        }
+        assert_eq!(
+            validate_read_only("ls -la", mode),
+            ValidationResult::Allow,
+            "plain ASCII commands are still allowed"
+        );
+    }
 }

crates/code-tools/src/lib.rs

@@ -3726,7 +3726,10 @@ fn summarize_web_fetch(
 
 fn extract_title(content: &str, raw_body: &str, content_type: &str) -> Option<String> {
     if content_type.contains("html") {
-        let lowered = raw_body.to_lowercase();
+        // ASCII lowercasing keeps byte offsets identical to `raw_body`. Full Unicode
+        // lowercasing can change byte lengths (e.g. 'İ'), which would make the offsets
+        // below land inside a multi-byte character of `raw_body` and panic.
+        let lowered = raw_body.to_ascii_lowercase();
         if let Some(start) = lowered.find("<title>") {
             let after = start + "<title>".len();
             if let Some(end_rel) = lowered[after..].find("</title>") {
@@ -3821,7 +3824,7 @@ fn extract_search_hits(html: &str) -> Vec<SearchHit> {
         };
         let after_tag = &rest[close_tag_idx + 1..];
         let Some(end_anchor_idx) = after_tag.find("</a>") else {
-            remaining = &after_tag[1..];
+            remaining = skip_one_char(after_tag);
             continue;
         };
         let title = html_to_text(&after_tag[..end_anchor_idx]);
@@ -3879,6 +3882,15 @@ fn extract_search_hits_from_generic_links(html: &str) -> Vec<SearchHit> {
     hits
 }
 
+/// Drops the first character of `input`. Unlike `&input[1..]`, this never panics on an
+/// empty string or a multi-byte leading character in untrusted HTML.
+fn skip_one_char(input: &str) -> &str {
+    input
+        .chars()
+        .next()
+        .map_or("", |ch| &input[ch.len_utf8()..])
+}
+
 fn extract_quoted_value(input: &str) -> Option<(String, &str)> {
     let quote = input.chars().next()?;
     if quote != '"' && quote != '\'' {
@@ -6738,6 +6750,43 @@ fn parse_skill_description(contents: &str) -> Option<String> {
 
 pub mod lane_completion;
 
+#[cfg(test)]
+mod untrusted_html_panic_regression_tests {
+    use super::{extract_search_hits, extract_title};
+
+    #[test]
+    fn title_extraction_survives_unicode_that_changes_length_when_lowercased() {
+        // 'İ' (2 bytes) lowercases to 3 bytes, so full Unicode lowercasing shifts the
+        // offset of "<title>" relative to the original body. Must not panic.
+        let body = "<html><body>İİİİ</body><TITLE>Hello &amp; welcome</TITLE></html>";
+        let title = extract_title("", body, "text/html");
+        assert_eq!(title.as_deref(), Some("Hello & welcome"));
+    }
+
+    #[test]
+    fn title_extraction_handles_unclosed_title_with_unicode_prefix() {
+        let body = "İ<title>no end tag";
+        assert_eq!(
+            extract_title("fallback line", body, "text/html").as_deref(),
+            Some("fallback line")
+        );
+    }
+
+    #[test]
+    fn search_hit_parsing_survives_anchor_ending_at_end_of_input() {
+        // The anchor tag closes with '>' and then the input ends, with no "</a>".
+        let html = "result__a href=\"https://example.com/\">";
+        assert!(extract_search_hits(html).is_empty());
+    }
+
+    #[test]
+    fn search_hit_parsing_survives_multibyte_char_after_anchor_tag() {
+        // No "</a>" follows, and the first byte after '>' starts a multi-byte char.
+        let html = "result__a href=\"https://example.com/\">é trailing";
+        assert!(extract_search_hits(html).is_empty());
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use std::collections::BTreeMap;