[gabrielvfonseca]

Summary

A panic audit of in-scope parsing paths (server/webhook handlers, provider and MCP transports, WebFetch/search HTML, runtime bash validation, CLI and config parsing, Rust SDK). Four sites could be crashed by input an attacker or a malicious remote controls. Each is fixed with a regression test. Valid-input behaviour is unchanged.

Fixed sites

1. crates/code-tools/src/lib.rs extract_title (~L3732)

   • Input: HTML body from a fetched URL (WebFetch with a prompt containing "title").
   • Cause: body was lowercased with Unicode to_lowercase, then offsets from the lowercased copy were used on the raw body. Characters whose lowercase form has a different byte length (e.g. İ) shifted the offsets and panicked on a char boundary or out of range.
   • Fix: to_ascii_lowercase(), which keeps byte offsets identical. Tests: title_extraction_survives_unicode_that_changes_length_when_lowercased, title_extraction_handles_unclosed_title_with_unicode_prefix.

2. crates/code-tools/src/lib.rs extract_search_hits (~L3827)

   • Input: search-result HTML (WebSearch) with a result__a anchor whose > is followed by end-of-input or a multi-byte character and no </a>.
   • Cause: after_tag[1..] panicked on an empty string or a non-boundary index.
   • Fix: new helper skip_one_char that advances by one char. Tests: search_hit_parsing_survives_anchor_ending_at_end_of_input, search_hit_parsing_survives_multibyte_char_after_anchor_tag.

3. crates/code-runtime/src/bash_validation.rs extract_first_command (~L639)

   • Input: a shell command with an env prefix, whitespace after =, and a multi-byte quoted value, e.g. A= 'é' ls. Commands come from model output, so this is reachable from a remote API user.
   • Cause: find_end_of_value returns an offset into the trimmed value, but it was applied to the untrimmed text, so the slice could land inside a character.
   • Fix: slice the trimmed value. Also fixes the wrong command name this produced for such input. Test: env_prefix_with_whitespace_and_multibyte_quoted_value_does_not_panic.

4. crates/code-integrations/src/mcp/utils.rs percent_decode (~L185)

   • Input: mcp_url query value in a CCR proxy URL. A repo's .mcp.json can set this URL, and signature computation runs on it.
   • Cause: &value[index+1..index+3] sliced the str at byte offsets, which panics when the window ends inside a multi-byte character (e.g. %aé).
   • Fix: decode on bytes with a hex_nibble helper. Test: unwrapping_ccr_proxy_url_with_multibyte_percent_sequence_does_not_panic.

5. crates/code-integrations/src/mcp/stdio.rs read_frame (~L1237)

   • Input: a Content-Length header from an MCP stdio server.
   • Cause: vec![0_u8; content_length] preallocated the peer-declared size. Very large values panic with "capacity overflow" or abort on allocation.
   • Fix: the body moved into read_frame_from<R: AsyncBufRead>, which reads through take() and returns UnexpectedEof on a short stream. Tests: huge_declared_content_length_returns_error_instead_of_panicking, reads_well_formed_content_length_frame.

Verification

• Verified the regression tests fail on the original logic: title_extraction_survives_unicode..., search_hit_parsing_survives_anchor_ending_at_end_of_input, and search_hit_parsing_survives_multibyte_char_after_anchor_tag all panic on the old code (checked by temporarily restoring it; the fixed code was restored afterwards).
• cargo fmt --all -- --check: pass
• cargo clippy -p code-tools -p code-runtime -p code-integrations --all-targets -- -D warnings: pass
• cargo test -p code-tools -p code-runtime -p code-integrations: all pass, including the new tests (rebased onto current origin/main and re-run)

Reviewed and left as-is

• Mutex/RwLock .lock().unwrap() / .expect("... poisoned") in server, runtime, memory, and CLI code: these only panic after another thread panicked while holding the lock, so they are secondary to the panics fixed here.
• code-providers/src/providers/anthropic.rs last_error.expect(...): the first failed attempt always sets last_error before any break, so it is unreachable.
• JSON parser expect/next().expect calls in code-runtime/src/json.rs: guarded by peek().
• code-webhooks HMAC new_from_slice(...).expect: HMAC accepts any key length.
• Slicing in compact.rs, file_ops.rs, policy.rs, permissions.rs, summary_compression.rs, sse.rs, the openai args delta, prompt_cache.rs (ASCII-only input), oauth.rs percent decode and base64, worker_boot.rs, lane_events.rs, the Neo4j row parser, markdown section extraction, and the webhook receiver: each index is bounded or on a char-safe offset.
• Duration and Instant arithmetic on operator policy values in code-server: these are saturating or comparison-only.
• Release builds do not enable overflow-checks, so arithmetic overflow wraps rather than panicking.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Improved handling of incomplete or unusually large message frames, with clearer errors when payloads are cut short.
  • Fixed percent-encoded text handling so malformed escapes involving multibyte characters no longer cause a crash.
  • Improved command parsing for empty environment-variable assignments and multibyte values; read-only validation now rejects unsupported whitespace.
  • Fixed HTML title matching and search-result parsing to handle Unicode text and missing closing tags safely.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: defaults
• Review profile: CHILL
• Plan: Advanced
• Run ID: e0c4312b-ea28-4b8a-8a67-673f9a44581e

Commits

Reviewing files that changed from the base of the PR and between 9225b93 and a7ac712.

Files selected for processing: 4

• crates/code-integrations/src/mcp/stdio.rs
• crates/code-integrations/src/mcp/utils.rs
• crates/code-runtime/src/bash_validation.rs
• crates/code-tools/src/lib.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

The changes update MCP frame reading and percent decoding, Bash read-only command validation and extraction, and HTML title and search-hit parsing. Regression tests cover incomplete frames and malformed or multibyte input.

Changes: MCP frame reading

Layer / File(s)	Summary
Bounded frame reading crates/code-integrations/src/mcp/stdio.rs	McpStdioProcess::read_frame delegates to incremental payload reading. Incomplete payloads return UnexpectedEof. Tests cover incomplete and valid frames.

MCP percent decoding

Layer / File(s)	Summary
Byte-safe percent decoding crates/code-integrations/src/mcp/utils.rs	percent_decode checks escape bytes without slicing across UTF-8 character boundaries. A regression test covers malformed escapes involving a multibyte character.

Bash command validation

Layer / File(s)	Summary
Command extraction and read-only validation crates/code-runtime/src/bash_validation.rs	Command extraction handles empty assignments and multibyte quoted values. Read-only validation blocks commands containing whitespace other than space, tab, or newline. Tests cover these cases.

HTML search parsing

Layer / File(s)	Summary
UTF-8-safe HTML parsing crates/code-tools/src/lib.rs	Title matching uses ASCII-only lowercasing. Search-hit parsing skips a complete Unicode character when an anchor lacks a closing tag. Regression tests cover both cases.

Priority: Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Merge Risk: Minimal · up to a7ac7

This change removes several reachable panics and reduces memory exposure from MCP Content-Length values. No concrete merge-blocking risk remains; a maximum frame size could be added later as optional hardening.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surv…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly summarizes the primary change: removing reachable panics caused by untrusted remote and repository input.
Description check		The description provides detailed motivation, affected sites, root causes, fixes, regression tests, verification results, and follow-up context. It does not use the template's Type of Change or Checkl…
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description provides detailed motivation, affected sites, root causes, fixes, regression tests, verification results, and follow-up context. It does not use the template's Type of Change or Checklist sections, but the required change and testing information is otherwise complete.

Full details: Docstring Coverage

Explanation

Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 20 functions across 4 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1

📝 Generate docstrings: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[gabrielvfonseca]

Follow-up commit 2b2bbf3 addresses an automated security review finding. With whitespace after A=, the earlier trim made the next word the assignment value, so A= rm x was reported as x. The value now starts at = and an empty value consumes no word. Regression tests cover A= rm x, A= rm x, and A=. The old expectation for A= 'é' ls (ls) was wrong; bash runs 'é' there.

[gabrielvfonseca]

Commit security review follow-up: a3 fix for Unicode whitespace. char::is_whitespace also matches U+00A0, U+3000, \r, and \x0b, and bash does not split words on them. A=<NBSP>ls rm -rf / was reported as ls, while bash runs rm. In read-only mode the parser now refuses any command with whitespace other than space, tab, or newline. Regression test: read_only_refuses_whitespace_bash_does_not_split_on.