Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 6 additions & 7 deletions config.docker.toml
Original file line number Diff line number Diff line change
Expand Up @@ -73,14 +73,13 @@ ws_url = "ws://chrome:9222/"
[renderer.chrome_pool]
size = 4

# /v1/search points at the bundled SearXNG sidecar. Use the `searxng-internal`
# alias, which exists ONLY on the internal `search_rpc` network — the bare
# `searxng` name also resolves on `public_egress`, where the egress firewall
# drops service-to-service traffic, so docker DNS returning that IP breaks the
# answer path. `searxng-internal` pins the search_rpc IP (matches the SaaS,
# which already uses it). See crw-saas docker-compose.prod.yml.
# /v1/search points at the bundled SearXNG sidecar. On the reference
# docker-compose stack the sidecar is reachable as `searxng:8080` over the
# default bridge network (the service is named `searxng`). To point CRW at an
# external SearXNG instead, override this without editing the file by setting
# `CRW_SEARCH__SEARXNG_URL=http://your-host:8080` (env wins over this default).
[search]
searxng_url = "http://searxng-internal:8080"
searxng_url = "http://searxng:8080"
# Multi-query expansion (cycle-2 of the answer-quality loop): the proven win.
# On a 50-q SimpleQA+FRAMES sample it lifted overall 64%->76% and FRAMES
# 56%->80% by unioning an entity-rewrite's results with the original. Baked on
Expand Down
2 changes: 1 addition & 1 deletion crates/crw-mcp-proto/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ pub fn tool_definitions(proxy_mode: bool) -> Value {
let _ = proxy_mode;
tools.push(json!({
"name": "crw_search",
"description": "Search the web and return relevant results with titles, URLs, and descriptions/snippets. Backed by a SearXNG sidecar in embedded mode (no API key needed), or by the configured remote API in proxy mode (uses CRW_API_KEY).\n\nReturn shape: `{ \"success\": true, \"data\": { \"results\": [{ \"url\", \"title\", \"description\", \"snippet\", \"position\", \"score\" }, ...] } }`. When `sources` is set, `data.results` is instead an object grouped by source (`{ \"web\": [...], \"news\": [...], \"images\": [...] }`). The `snippet` field is an alias of `description` — both carry the same body text so downstream LLM pipelines that ask for either get a match.\n\nExample: `crw_search(query=\"renewable energy trends 2024\", limit=3)` returns the top 3 web results with title/url/snippet.",
"description": "Search the web and return relevant results with titles, URLs, and descriptions/snippets. Backed by a SearXNG sidecar in embedded mode (no API key needed), or by the configured remote API in proxy mode (uses CRW_API_KEY).\n\nReturn shape: `{ \"success\": true, \"data\": { \"results\": [{ \"url\", \"title\", \"description\", \"snippet\", \"position\", \"score\" }, ...] } }`. When `sources` is set, `data.results` is instead an object grouped by source (`{ \"web\": [...], \"news\": [...], \"images\": [...] }`). The `snippet` field is an alias of `description` — both carry the same body text so downstream LLM pipelines that ask for either get a match.\n\nExample: `crw_search(query=\"renewable energy trends 2024\", limit=3)` returns the top 3 web results with title/url/snippet.\n\nErrors: returns `search_disabled` when no SearXNG backend is configured, or `target_unreachable` / `timeout` (naming the configured host) when the backend can't be reached.",
"inputSchema": {
"type": "object",
"properties": {
Expand Down
12 changes: 11 additions & 1 deletion crates/crw-search/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,13 @@ impl SearxngClient {
}
}

/// Configured base URL (trailing slash trimmed). Exposed so the route layer
/// can name the host in `target_unreachable` errors without leaking it raw
/// (callers sanitize to the origin first — see crw-server `diagnostics`).
pub fn base_url(&self) -> &str {
&self.base_url
}

/// Issue a JSON search request. Errors surface as a typed [`SearchError`]
/// — the route layer maps them onto `CrwError` for HTTP responses.
pub async fn fetch(&self, params: &SearxngParams) -> Result<SearxngResponse, SearchError> {
Expand Down Expand Up @@ -191,7 +198,10 @@ impl SearxngClient {
if e.is_timeout() {
SearchError::Timeout
} else {
SearchError::Transport(e.to_string())
// `without_url()` strips reqwest's embedded request URL from
// the Display string — that URL can carry credentials/tokens
// (issue #90). The route layer re-attaches a sanitized origin.
SearchError::Transport(e.without_url().to_string())
}
})?;

Expand Down
119 changes: 119 additions & 0 deletions crates/crw-server/src/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
//! Operator-facing diagnostics helpers (issue #90).
//!
//! `searxng_url` is operator-set and can carry secrets (`https://user:pass@host`,
//! a `?token=…`, or a token embedded in the path of a reverse-proxy URL). Anything
//! we log or return in an error must be sanitized to the bare origin first.

use crw_core::config::SearchConfig;
use tracing::Level;

/// Reduce a URL to its **origin** — `scheme://host[:port]` — dropping userinfo,
/// path, query, and fragment. This is the only form safe to log or echo in an
/// error, because every other component can carry a secret. Falls back to a
/// fixed redaction string if the URL doesn't parse.
pub fn sanitize_url_origin(raw: &str) -> String {
match url::Url::parse(raw) {
Ok(u) => match (u.host_str(), u.port()) {
(Some(host), Some(port)) => format!("{}://{host}:{port}", u.scheme()),
(Some(host), None) => format!("{}://{host}", u.scheme()),
(None, _) => "<redacted-url>".to_string(),
},
Err(_) => "<redacted-url>".to_string(),
}
}

/// One-line summary of the search subsystem's configured state, for the startup
/// log. Distinguishes the three states that otherwise collapse to a single
/// "search disabled" at request time:
/// - `enabled = false` → intentionally off
/// - enabled, `searxng_url` unset → misconfigured (every call will 503)
/// - enabled, `searxng_url` set → active (host shown, origin-sanitized)
pub fn search_startup_status(cfg: &SearchConfig) -> (Level, String) {
if !cfg.enabled {
(
Level::INFO,
"search: disabled ([search].enabled = false)".to_string(),
)
} else if let Some(url) = &cfg.searxng_url {
(
Level::INFO,
format!("search: enabled (searxng={})", sanitize_url_origin(url)),
)
} else {
(
Level::WARN,
"search: enabled but no [search].searxng_url — /v1/search will return 503".to_string(),
)
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn sanitize_strips_userinfo_path_query_fragment() {
assert_eq!(
sanitize_url_origin("https://user:pass@host:9000/searxng/tok123?q=x#frag"),
"https://host:9000"
);
assert_eq!(
sanitize_url_origin("http://searxng:8080"),
"http://searxng:8080"
);
assert_eq!(
sanitize_url_origin("http://searxng:8080/"),
"http://searxng:8080"
);
// Default port is preserved as written only if explicit; no port → no port.
assert_eq!(
sanitize_url_origin("https://example.com"),
"https://example.com"
);
}

#[test]
fn sanitize_redacts_unparseable() {
assert_eq!(sanitize_url_origin("not a url"), "<redacted-url>");
}

fn cfg(enabled: bool, url: Option<&str>) -> SearchConfig {
let toml = match url {
Some(u) => format!("enabled = {enabled}\nsearxng_url = \"{u}\"\n"),
None => format!("enabled = {enabled}\n"),
};
toml::from_str(&toml).expect("valid SearchConfig")
}

#[test]
fn startup_status_enabled_with_url() {
let (level, msg) = search_startup_status(&cfg(true, Some("http://searxng:8080")));
assert_eq!(level, Level::INFO);
assert!(
msg.contains("enabled (searxng=http://searxng:8080)"),
"{msg}"
);
}

#[test]
fn startup_status_enabled_no_url_warns() {
let (level, msg) = search_startup_status(&cfg(true, None));
assert_eq!(level, Level::WARN);
assert!(msg.contains("no [search].searxng_url"), "{msg}");
}

#[test]
fn startup_status_disabled() {
let (level, msg) = search_startup_status(&cfg(false, Some("http://searxng:8080")));
assert_eq!(level, Level::INFO);
assert!(msg.contains("disabled"), "{msg}");
}

#[test]
fn startup_status_never_leaks_credentials() {
let (_, msg) = search_startup_status(&cfg(true, Some("https://u:secret@host:8080/tok")));
assert!(!msg.contains("secret"), "{msg}");
assert!(!msg.contains("tok"), "{msg}");
assert!(msg.contains("https://host:8080"), "{msg}");
}
}
1 change: 1 addition & 0 deletions crates/crw-server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
//! ```

pub mod app;
pub mod diagnostics;
pub mod error;
pub mod middleware;
pub mod routes;
Expand Down
54 changes: 54 additions & 0 deletions crates/crw-server/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,16 @@ async fn run_server() {
tracing::info!("LLM structured extraction: enabled");
}

// Issue #90: make the search subsystem's configured state visible at boot.
// Three states (disabled / enabled-but-unconfigured / enabled) otherwise
// collapse to a single request-time error. The host is origin-sanitized so
// a credentialed `searxng_url` never reaches the logs.
let (search_level, search_msg) = crw_server::diagnostics::search_startup_status(&config.search);
match search_level {
tracing::Level::WARN => tracing::warn!("{search_msg}"),
_ => tracing::info!("{search_msg}"),
}

// Boot guard: when SaaS fronts opencore it sets `CRW_DISABLE_SERVER_LLM_KEY=1`
// to prevent the most common ops mistake — leaving a server-wide key
// configured behind the SaaS, which would leak the org's key to every
Expand Down Expand Up @@ -111,6 +121,50 @@ async fn run_server() {
tracing::warn!("No CDP renderer active — JS rendering disabled");
}

// Issue #90: one-shot, non-fatal reachability probe so a misconfigured or
// down SearXNG is *spoken* at boot instead of failing silently on the first
// search. Bundled-compose users are already gated by `depends_on:
// searxng condition: service_healthy`, so this mainly helps operators who
// point `CRW_SEARCH__SEARXNG_URL` at an external host. The origin is
// sanitized; the probe hits the origin's `/healthz` (the same path the
// compose healthcheck uses) and is bounded by its own short timeout —
// never `connect_timeout`, which wouldn't bound a stalled response.
if state.config.search.enabled
&& let Some(raw_url) = state.config.search.searxng_url.clone()
{
let origin = crw_server::diagnostics::sanitize_url_origin(&raw_url);
tokio::spawn(async move {
let probe = reqwest::Client::builder()
.timeout(std::time::Duration::from_secs(3))
.build();
match probe {
Ok(client) => {
let healthz = format!("{origin}/healthz");
match client.get(&healthz).send().await {
Ok(resp) if resp.status().is_success() => {
tracing::info!("search: SearXNG reachable at {origin}");
}
Ok(resp) => {
tracing::warn!(
"search: SearXNG at {origin} answered /healthz with {} — \
search calls may fail until it is healthy",
resp.status()
);
}
Err(e) => {
tracing::warn!(
"search: configured host {origin} UNREACHABLE at startup \
({}) — search calls will fail until it resolves",
e.without_url()
);
}
}
}
Err(e) => tracing::warn!("search: could not build startup probe client: {e}"),
}
});
}

// Issue #35 transparency: when auto-extension widens the implicit deadline
// beyond the operator's `deadline_ms_default`, log the effective values so
// operators can correlate "request took longer than my SLO" against the
Expand Down
37 changes: 31 additions & 6 deletions crates/crw-server/src/routes/search.rs
Original file line number Diff line number Diff line change
Expand Up @@ -187,12 +187,12 @@ pub async fn search_inner(
.clamp(1, MAX_QUERY_EXPAND_VARIANTS);
fetch_expanded(&client, &req.query, &params, llm, variants_n)
.await
.map_err(|e| map_search_error(e, state.config.search.timeout_ms))?
.map_err(|e| map_search_error(e, state.config.search.timeout_ms, client.base_url()))?
} else {
client
.fetch(&params)
.await
.map_err(|e| map_search_error(e, state.config.search.timeout_ms))?
.map_err(|e| map_search_error(e, state.config.search.timeout_ms, client.base_url()))?
};

let has_sources = req.sources.as_ref().is_some_and(|s| !s.is_empty());
Expand Down Expand Up @@ -859,7 +859,12 @@ fn validate_request(req: &SearchRequest, max_limit: u32) -> Result<(), CrwError>
Ok(())
}

fn map_search_error(err: SearchError, timeout_ms: u64) -> CrwError {
/// Map a transport/timeout/upstream `SearchError` onto the HTTP `CrwError`.
/// `base_url` is the configured SearXNG URL; the transport (`target_unreachable`)
/// arm names its **origin** (issue #90) so the operator sees *which* host failed
/// — sanitized, so a credentialed URL never reaches the response. Timeouts keep
/// `error_code: "timeout"`; the host is correlated via the startup log instead.
fn map_search_error(err: SearchError, timeout_ms: u64, base_url: &str) -> CrwError {
match err {
SearchError::Timeout => CrwError::Timeout(timeout_ms),
SearchError::Upstream { status, body } => CrwError::HttpError(format!(
Expand All @@ -869,7 +874,10 @@ fn map_search_error(err: SearchError, timeout_ms: u64) -> CrwError {
SearchError::InvalidResponse(msg) => {
CrwError::HttpError(format!("SearXNG returned invalid JSON: {msg}"))
}
SearchError::Transport(msg) => CrwError::TargetUnreachable(format!("SearXNG: {msg}")),
SearchError::Transport(msg) => CrwError::TargetUnreachable(format!(
"SearXNG ({}): {msg}",
crate::diagnostics::sanitize_url_origin(base_url)
)),
}
}

Expand Down Expand Up @@ -1152,7 +1160,7 @@ mod tests {
#[test]
fn map_search_error_timeout_to_timeout() {
assert!(matches!(
map_search_error(SearchError::Timeout, 7500),
map_search_error(SearchError::Timeout, 7500, "http://searxng:8080"),
CrwError::Timeout(7500)
));
}
Expand All @@ -1164,11 +1172,28 @@ mod tests {
body: "down".into(),
};
assert!(matches!(
map_search_error(err, 5000),
map_search_error(err, 5000, "http://searxng:8080"),
CrwError::HttpError(_)
));
}

#[test]
fn map_search_error_transport_names_sanitized_host() {
// issue #90: the unreachable error must name the configured host so the
// operator knows *what* failed — but origin-only, never the raw URL.
let err = SearchError::Transport("dns error: failed to lookup address".into());
let mapped = map_search_error(err, 5000, "https://user:pass@searxng:8080/tok?k=v");
match mapped {
CrwError::TargetUnreachable(msg) => {
assert!(msg.contains("https://searxng:8080"), "{msg}");
assert!(!msg.contains("user"), "must not leak userinfo: {msg}");
assert!(!msg.contains("pass"), "must not leak credentials: {msg}");
assert!(!msg.contains("tok"), "must not leak path token: {msg}");
}
other => panic!("expected TargetUnreachable, got {other:?}"),
}
}

#[test]
fn _suppress_unused_search_source_warning() {
let _ = SearchSource::Web;
Expand Down
Loading
Loading