fix(zetesis): close indexer-fetch SSRF, injection, XML-DoS, and key-leak defects

crates/archon/src/serve.rs

@@ -1087,6 +1087,7 @@ fn build_cf_proxy(
     Ok(Arc::new(ByparrProxy::new(
         url.to_string(),
         std::time::Duration::from_secs(config.cf_proxy_timeout_seconds),
+        config.max_response_body_bytes,
     )))
 }
 

crates/zetesis/src/cf_bypass/byparr.rs

@@ -14,6 +14,7 @@ pub struct ByparrProxy {
     client: reqwest::Client,
     endpoint: String,
     timeout: Duration,
+    max_body_bytes: u64,
 }
 
 #[derive(Debug, Serialize)]
@@ -55,7 +56,7 @@ struct ByparrCookie {
 }
 
 impl ByparrProxy {
-    pub fn new(endpoint: String, timeout: Duration) -> Self {
+    pub fn new(endpoint: String, timeout: Duration, max_body_bytes: u64) -> Self {
         let client = reqwest::Client::builder()
             .timeout(timeout + Duration::from_secs(5))
             .build()
@@ -65,6 +66,7 @@ impl ByparrProxy {
             client,
             endpoint,
             timeout,
+            max_body_bytes,
         }
     }
 
@@ -95,7 +97,10 @@ impl CloudflareProxy for ByparrProxy {
 }
 
 impl ByparrProxy {
-    #[instrument(skip(self, ct), fields(endpoint = %self.endpoint))]
+    // WHY: skip `url` — the indexer URL embeds `apikey=` and #[instrument]
+    // would capture the raw value as a span field visible to any event
+    // emitted in the span.
+    #[instrument(skip(self, url, ct), fields(endpoint = %self.endpoint))]
     async fn get_inner(
         &self,
         url: &str,
@@ -124,9 +129,20 @@ impl ByparrProxy {
             }
         };
 
+        // WHY: enforce the body-size cap HERE — the cf_bypass fetch paths
+        // return this ProxyResponse.body directly, skipping read_body_bounded,
+        // so without a cap on the byparr envelope `max_response_body_bytes` is
+        // a no-op for cf_bypass indexers. The cap bounds the byparr JSON
+        // (Content-Length precheck + streamed running counter) before it is
+        // buffered; the wrapped indexer body is a subset, so the effective
+        // ceiling is conservative by the envelope overhead.
+        let raw = crate::client::read_body_bytes_bounded(response, &redacted, self.max_body_bytes)
+            .await?;
         let byparr_resp: ByparrResponse =
-            response.json().await.context(error::HttpRequestSnafu {
+            serde_json::from_slice(&raw).map_err(|e| SearchIndexerError::ParseResponse {
                 url: redacted.clone(),
+                error: e.to_string(),
+                location: snafu::Location::new(file!(), line!(), column!()),
             })?;
 
         if byparr_resp.status != "ok" {
@@ -173,6 +189,32 @@ impl ByparrProxy {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::test_support::spawn_one_shot_http;
+
+    #[tokio::test]
+    async fn get_enforces_body_size_cap() {
+        // WHY: the cf_bypass fetch paths return ByparrProxy's body directly,
+        // so the cap must live here or `max_response_body_bytes` is a no-op
+        // for cf_bypass indexers. A byparr envelope declaring a size above the
+        // cap must be rejected before it is buffered.
+        let big = "x".repeat(10_000);
+        let json = format!(
+            r#"{{"status":"ok","message":"","solution":{{"url":"https://e.example","status":200,"response":"{big}","cookies":[],"userAgent":"UA"}}}}"#
+        );
+        let (endpoint, _server) = spawn_one_shot_http(200, "OK", &[], &json).await;
+        let proxy = ByparrProxy::new(endpoint, Duration::from_secs(5), 64);
+        let err = proxy
+            .get(
+                "https://indexer.example/api?apikey=secret",
+                CancellationToken::new(),
+            )
+            .await
+            .unwrap_err();
+        assert!(
+            matches!(err, SearchIndexerError::ResponseTooLarge { limit: 64, .. }),
+            "expected ResponseTooLarge, got {err:?}"
+        );
+    }
 
     #[test]
     fn byparr_request_serialization() {

crates/zetesis/src/client/cardigann/mod.rs

@@ -296,12 +296,23 @@ impl CardigannClient {
         let mut raw_parts: Vec<String> = Vec::new();
         let mut pairs: Vec<(&str, String)> = Vec::new();
         for (key, value) in inputs {
-            let rendered = ctx
-                .render(value)
-                .map_err(|e| self.invalid(format!("search input {key}: {e}")))?;
             if key == "$raw" {
+                // WHY: $raw is spliced verbatim into the query via set_query
+                // (which does NOT encode), so variable expansions must be
+                // percent-encoded here exactly as the path template already is
+                // via render_url — a keyword like "a&b=c" must not inject an
+                // extra parameter. Only the .Keywords/.Query.* expansions
+                // encode; the definition-author's literal $raw "&"/"=" survive.
+                let rendered = ctx
+                    .render_url(value)
+                    .map_err(|e| self.invalid(format!("search input {key}: {e}")))?;
                 raw_parts.push(rendered);
             } else {
+                // WHY: non-$raw values pass through append_pair below, which
+                // percent-encodes the whole value — render unencoded here.
+                let rendered = ctx
+                    .render(value)
+                    .map_err(|e| self.invalid(format!("search input {key}: {e}")))?;
                 pairs.push((key, rendered));
             }
         }
@@ -599,7 +610,10 @@ impl IndexerClient for CardigannClient {
         }
     }
 
-    #[instrument(skip(self, ct), fields(indexer_id = self.indexer.id))]
+    // WHY: skip `url` — download URLs carry secrets (apikey/passkey in the
+    // query) and #[instrument] would capture the raw value as a span field
+    // visible to any event emitted in the span.
+    #[instrument(skip(self, url, ct), fields(indexer_id = self.indexer.id))]
     async fn download(
         &self,
         url: &str,

crates/zetesis/src/client/cardigann/tests.rs

@@ -234,6 +234,61 @@ async fn keywords_inline_in_path_template_are_percent_encoded() {
     );
 }
 
+const RAW_INPUT_DEF: &str = r#"---
+id: raw-input
+name: Raw Input
+links:
+  - https://raw-input.example/
+caps:
+  categorymappings:
+    - {id: 6, cat: Movies/HD}
+  modes:
+    search: [q]
+search:
+  paths:
+    - path: /browse
+  inputs:
+    $raw: "q={{ .Keywords }}&mode=list"
+  rows:
+    selector: table#torrents > tbody > tr
+  fields:
+    title:
+      selector: a.title
+    download:
+      selector: a.dl
+      attribute: href
+"#;
+
+#[tokio::test]
+async fn raw_input_expansions_are_percent_encoded() {
+    // WHY: $raw is spliced verbatim into the query via set_query (no encoding),
+    // so a keyword like "a&b=c" must be encoded to a single value — never split
+    // into an injected `b=c` parameter. The literal $raw "q="/"&mode=" survive.
+    let (url, server) = spawn_one_shot_http(200, "OK", &[], "<html/>").await;
+    let query = SearchQuery {
+        query_text: Some("a&b=c".to_string()),
+        limit: 100,
+        ..Default::default()
+    };
+    client(RAW_INPUT_DEF, url, None)
+        .unwrap()
+        .search(&query, CancellationToken::new())
+        .await
+        .unwrap();
+
+    let request_head = server.await.unwrap();
+    let request_line = request_head.lines().next().unwrap_or_default().to_string();
+    assert_eq!(
+        request_line, "GET /browse?q=a%26b%3Dc&mode=list HTTP/1.1",
+        "raw expansion must encode; no injected param"
+    );
+    // WHY: guard against a decoded "&b=c" smuggling a second parameter.
+    assert!(
+        !request_line.contains("&b=c"),
+        "injected param leaked: {request_line}"
+    );
+}
+
 #[tokio::test]
 async fn search_without_inputs_appends_no_bare_question_mark() {
     let yaml = PATH_TEMPLATE_DEF.replace("/browse.php?search={{ .Keywords }}&cat=0", "/browse");

crates/zetesis/src/client/mod.rs

@@ -3,9 +3,10 @@ pub mod newznab;
 pub mod torznab;
 pub mod xml;
 
-use std::net::IpAddr;
+use std::net::{IpAddr, Ipv4Addr, SocketAddr};
 
 use futures::StreamExt;
+use reqwest::dns::{Addrs, Name, Resolve, Resolving};
 use snafu::ResultExt;
 use tokio_util::sync::CancellationToken;
 use url::{Host, Url};
@@ -306,14 +307,63 @@ fn ip_is_disallowed(ip: IpAddr) -> bool {
             if let Some(mapped) = v6.to_ipv4_mapped() {
                 return ip_is_disallowed(IpAddr::V4(mapped));
             }
-            v6.is_loopback()
+            // WHY: checked before the ::/96 embed below so :: (unspecified) and
+            // ::1 (loopback) are judged as v6, not as the compatible 0.0.0.0 /
+            // 0.0.0.1.
+            if v6.is_loopback()
                 || v6.is_unspecified()
                 || v6.is_unique_local()
                 || v6.is_unicast_link_local()
+            {
+                return true;
+            }
+            // WHY: deprecated IPv4-compatible IPv6 (::a.b.c.d, ::/96) embeds a
+            // v4 address in the low 32 bits — ::127.0.0.1 must be judged by
+            // that v4 or it reaches loopback unchecked. to_ipv4_mapped only
+            // matches ::ffff:*.
+            let [s0, s1, s2, s3, s4, s5, s6, s7] = v6.segments();
+            if [s0, s1, s2, s3, s4, s5] == [0, 0, 0, 0, 0, 0] {
+                let embedded = Ipv4Addr::from((u32::from(s6) << 16) | u32::from(s7));
+                return ip_is_disallowed(IpAddr::V4(embedded));
+            }
+            false
         }
     }
 }
 
+/// A reqwest DNS resolver that rejects the connection when ANY resolved
+/// address is disallowed, then hands reqwest only the vetted addresses.
+///
+/// WHY: [`validate_fetch_url`] resolves and checks at validation time but then
+/// discards the addresses; the shared client re-resolves at connect time, so a
+/// host answering public at validation and private at connect (DNS rebinding)
+/// would slip past the pre-check. Re-checking every address here — the moment
+/// reqwest is about to connect — closes that TOCTOU window. Fail-closed:
+/// resolution failure or any disallowed address rejects the whole connection.
+#[derive(Debug, Default, Clone, Copy)]
+pub(crate) struct SsrfGuardResolver;
+
+impl Resolve for SsrfGuardResolver {
+    fn resolve(&self, name: Name) -> Resolving {
+        let host = name.as_str().to_string();
+        Box::pin(async move {
+            // WHY: port 0 — the address checks ignore the port and reqwest
+            // overrides it with the target's real port after resolution.
+            let resolved: Vec<SocketAddr> =
+                tokio::net::lookup_host((host.as_str(), 0)).await?.collect();
+            for addr in &resolved {
+                if ip_is_disallowed(addr.ip()) {
+                    return Err(format!(
+                        "refusing to connect to {host}: resolves to a private or local address"
+                    )
+                    .into());
+                }
+            }
+            Ok(Box::new(resolved.into_iter()) as Addrs)
+        })
+    }
+}
+
 fn urlencoding(s: &str) -> String {
     let mut result = String::with_capacity(s.len());
     for b in s.bytes() {
@@ -518,6 +568,10 @@ mod tests {
             "http://[fe80::1]/x",
             "http://[::ffff:127.0.0.1]/x",
             "http://[::ffff:192.168.1.1]/x",
+            // WHY: deprecated IPv4-compatible IPv6 (::/96) embed — must be
+            // judged by the embedded v4 or [::127.0.0.1] reaches loopback.
+            "http://[::127.0.0.1]/x",
+            "http://[::169.254.169.254]/x",
         ] {
             let err = validate_fetch_url(url).await.expect_err(url);
             assert!(matches!(err, SearchIndexerError::UnsafeUrl { .. }));
@@ -559,9 +613,28 @@ mod tests {
         assert!(ip_is_disallowed(IpAddr::V4(Ipv4Addr::new(169, 254, 1, 1))));
         assert!(ip_is_disallowed(IpAddr::V4(Ipv4Addr::new(100, 127, 0, 1))));
         assert!(ip_is_disallowed(IpAddr::V6(Ipv6Addr::LOCALHOST)));
+        // NOTE: deprecated IPv4-compatible IPv6 (::/96) — judged by embedded v4.
+        assert!(ip_is_disallowed("::127.0.0.1".parse().unwrap()));
+        assert!(ip_is_disallowed("::10.0.0.1".parse().unwrap()));
+        assert!(ip_is_disallowed("::169.254.169.254".parse().unwrap()));
         assert!(!ip_is_disallowed(IpAddr::V4(Ipv4Addr::new(203, 0, 113, 1))));
+        assert!(!ip_is_disallowed("::203.0.113.1".parse().unwrap()));
         assert!(!ip_is_disallowed(IpAddr::V4(Ipv4Addr::new(8, 8, 8, 8))));
         assert!(!ip_is_disallowed(IpAddr::V4(Ipv4Addr::new(100, 63, 0, 1))));
         assert!(!ip_is_disallowed(IpAddr::V4(Ipv4Addr::new(172, 32, 0, 1))));
     }
+
+    #[tokio::test]
+    async fn ssrf_resolver_rejects_loopback_resolution() {
+        // WHY: "localhost" resolves to a loopback address via the system
+        // resolver — the connect-time guard must reject it, proving the
+        // rebinding TOCTOU is closed even after validate_fetch_url passed.
+        let resolver = SsrfGuardResolver;
+        let name: Name = "localhost".parse().expect("localhost is a valid DNS name");
+        let result = resolver.resolve(name).await;
+        assert!(
+            result.is_err(),
+            "resolver must reject a host that resolves to a loopback address"
+        );
+    }
 }

crates/zetesis/src/client/newznab.rs

@@ -184,7 +184,10 @@ impl IndexerClient for NewznabClient {
         }
     }
 
-    #[instrument(skip(self, ct), fields(indexer_id = self.config.id))]
+    // WHY: skip `url` — download URLs carry secrets (apikey in the query) and
+    // #[instrument] would capture the raw value as a span field visible to any
+    // event emitted in the span.
+    #[instrument(skip(self, url, ct), fields(indexer_id = self.config.id))]
     async fn download(
         &self,
         url: &str,

crates/zetesis/src/client/torznab.rs

@@ -193,7 +193,10 @@ impl IndexerClient for TorznabClient {
         }
     }
 
-    #[instrument(skip(self, ct), fields(indexer_id = self.config.id))]
+    // WHY: skip `url` — download URLs carry secrets (apikey/passkey in the
+    // query) and #[instrument] would capture the raw value as a span field
+    // visible to any event emitted in the span.
+    #[instrument(skip(self, url, ct), fields(indexer_id = self.config.id))]
     async fn download(
         &self,
         url: &str,