Skip to content

πŸ› fix: byte-boundary panic (#148) + rmcp allowed_hosts env vars (#149)#157

Merged
flupkede merged 1 commit into
developfrom
fix/issues-148-149
Jul 23, 2026
Merged

πŸ› fix: byte-boundary panic (#148) + rmcp allowed_hosts env vars (#149)#157
flupkede merged 1 commit into
developfrom
fix/issues-148-149

Conversation

@flupkede

Copy link
Copy Markdown
Owner

Two unrelated community-reported issues bundled in one PR per maintainer direction.

#148 β€” UTF-8 panic in search snippet output

Reported by @tony-nexartis: codesearch search panicked with byte index 100 is not a char boundary when a result snippet contained multi-byte UTF-8 characters (box-drawing separators in comment art, CJK, emoji) at the truncation point.

Root cause: &snippet[..100] byte-sliced a UTF-8 string. Byte offset 100 falling inside a multi-byte character is a panic.

Originally flagged in PR #152 review as "out-of-scope, deferred"; reported as #148. (My sanitize_for_terminal from #152 preserves multi-byte chars, which paradoxically made this panic deterministically reproducible for box-drawing content.)

Fix: str::floor_char_boundary(100) (stabilized in Rust 1.82; we're on 1.95) finds the largest char boundary ≀ 100 bytes, then slice. 1-line change.

Test: test_byte_truncation_preserves_char_boundary constructs a 120-byte string of 40 Γ— U+2500 (─) β€” byte 100 lands inside char #33 (bytes 99–102), so the pre-fix code genuinely panics on this fixture.

#149 β€” rmcp allowed_hosts env var overrides

Reported by @stdweird: containerised deployments of codesearch serve fail because rmcp β‰₯ 1.4.0's DNS-rebinding defence (GHSA-89vp-x53w-74fx, CVE-2026-42559) defaults StreamableHttpServerConfig::allowed_hosts to loopback-only ["localhost", "127.0.0.1", "::1"]. The container's Host header (its hostname) is rejected with WARN ... rejected request with disallowed Host header.

Fix: expose two env vars, both read once at serve startup:

Env var Effect
CODESEARCH_ALLOWED_HOSTS=host[,host:port,...] Comma-separated list replaces the rmcp default allowlist. Whitespace-trimmed, empties dropped.
CODESEARCH_DISABLE_HOST_VALIDATION=1|true Disables Host validation entirely (disable_allowed_hosts() β†’ empty allowlist β†’ rmcp allows all hosts). Dangerous β€” only safe behind a reverse proxy that validates Host itself. Accepts 1 or true (case-insensitive); any other value is ignored. Takes precedence over ALLOWED_HOSTS.

When both unset (or ALLOWED_HOSTS is empty after trim), the rmcp loopback-only default applies unchanged.

Implementation: new module-level helper build_streamable_http_config() in src/serve/mod.rs encapsulates the resolution order (disable > custom > default). Called once from run_serve in place of the previous inline StreamableHttpServerConfig::default(). New constants ALLOWED_HOSTS_ENV and DISABLE_HOST_VALIDATION_ENV in src/constants.rs follow the existing pattern (ALLOWED_ROOTS_ENV, SERVE_API_KEY_ENV).

Tests: 7 unit tests in mod allowed_hosts_tests cover all branches:

  • default loopback-only (both env vars unset)
  • custom ALLOWED_HOSTS replaces default
  • DISABLE=1 clears allowlist
  • DISABLE=TRUE case-insensitive
  • DISABLE=yes (other value) ignored
  • empty ALLOWED_HOSTS falls back to default
  • DISABLE takes precedence over ALLOWED_HOSTS

Validation

  • cargo fmt --check clean
  • cargo clippy --all-targets -- -D warnings clean
  • cargo test --lib --bins: 1188 passed, 36 ignored, 0 failed

Files

File Changes
src/constants.rs +2 env var constants with doc-comments
src/search/mod.rs 1-line floor_char_boundary fix + regression test
src/serve/mod.rs new build_streamable_http_config() helper + call-site swap + 7 unit tests

Closes #148.
Closes #149.

…s env vars (#149)

Two unrelated fixes bundled in one PR per maintainer direction.

#148 β€” UTF-8 panic at src/search/mod.rs:1343
============================================
Pre-existing bug: `&snippet[..100]` byte-sliced a UTF-8 string, panicking
with "byte index 100 is not a char boundary" when byte 100 landed inside a
multi-byte character (box-drawing separators in comment art, CJK, emoji).
Originally flagged in PR #152 review as "out-of-scope, deferred"; reported
as issue #148 by @tony-nexartis.

Fix: use `str::floor_char_boundary(100)` (stabilized in Rust 1.82; we're on
1.95) to find the largest char boundary ≀ 100 bytes, then slice. 1-line
change at the print site. Regression test `test_byte_truncation_preserves_
char_boundary` in src/search/mod.rs constructs a 120-byte string of U+2500
box-drawing chars and asserts no panic + correct char-boundary cut.

#149 β€” Container hostname rejected by rmcp default allowlist
=============================================================
rmcp β‰₯ 1.4.0 added DNS-rebinding defence (GHSA-89vp-x53w-74fx,
CVE-2026-42559): `StreamableHttpServerConfig::allowed_hosts` defaults to
loopback-only `["localhost", "127.0.0.1", "::1"]`. Containerised
deployments (where the Host header is the container hostname, not
localhost) get `WARN ... rejected request with disallowed Host header`.
Reported as issue #149 by @stdweird.

Fix: expose two env vars, both read once at serve startup:

  CODESEARCH_ALLOWED_HOSTS=host[,host:port,...]
    Comma-separated list of hostnames / `host:port` authorities. Replaces
    the rmcp default allowlist. Whitespace-trimmed, empties dropped.

  CODESEARCH_DISABLE_HOST_VALIDATION=1|true
    Disables Host validation entirely (calls rmcp's `disable_allowed_hosts()`).
    DANGEROUS β€” only safe behind a reverse proxy that validates Host itself.
    Accepts `1` or `true` (case-insensitive); any other value is ignored.
    Takes precedence over CODESEARCH_ALLOWED_HOSTS.

New module-level helper `build_streamable_http_config()` in src/serve/mod.rs
encapsulates the resolution order (disable > custom > default). Called once
from `run_serve` in place of the previous inline `StreamableHttpServerConfig
::default()`. 7 unit tests in `mod allowed_hosts_tests` cover all branches.

Both env vars documented in src/constants.rs with the same comment style as
the existing ALLOWED_ROOTS_ENV / SERVE_API_KEY_ENV.

Validation
==========
- `cargo fmt --check` clean
- `cargo clippy --all-targets -- -D warnings` clean
- `cargo test --lib --bins`: 1188 passed, 36 ignored, 0 failed
  (includes 7 new allowed_hosts tests + 1 byte_truncation test)

Closes #148.
Closes #149.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant