fix(template): reject URI query at build and render

UriChecker had no query handling. Two attack surfaces:

1. Build: a template like ?tenant={{ tenant }} lets an attacker
   supply a value containing & to append arbitrary key=value pairs.
   Every attempt to reason about & boundaries and multi-field
   patterns was either bypassable or false-positive on operator-
   authored static suffixes. Reject any URI template whose literal
   prefix crosses into the query at build time; operators can put
   dynamic routing in the path instead.

2. Render: with the build check in place, a URI checker only exists
   for pathful templates. So any rendered URI with a query means a
   field value smuggled ?... into the path. Example: template
   .../ingest/{{ tenant }} with tenant = 'ok?tenant=evil' renders
   to .../ingest/ok?tenant=evil — same authority, same path prefix,
   attacker-controlled query. Reject any rendered URI with a
   non-empty query.

Author: pront

src/sinks/elasticsearch/config.rs

@@ -614,7 +614,13 @@ where
 /// value violating those constraints is either malicious or would be
 /// rejected on ingest.
 fn is_valid_data_stream_component(s: &str) -> bool {
-    if s.is_empty() || s.len() > 255 {
+    // Empty is legitimate: `DataStreamConfig::index` filters empty parts
+    // out of the joined `type-dataset-namespace` name, so an event field
+    // that explicitly overrides one of the parts to `""` just skips it.
+    if s.is_empty() {
+        return true;
+    }
+    if s.len() > 255 {
         return false;
     }
     if s.contains('\0') || s.contains('/') || s.contains('\\') {
@@ -623,7 +629,9 @@ fn is_valid_data_stream_component(s: &str) -> bool {
     // Reject any path segment that is exactly `.` or `..` to block
     // filesystem-style traversal even if the surrounding value looks
     // otherwise identifier-like.
-    if s.split(&['/', '\\'][..]).any(|seg| seg == ".." || seg == ".") {
+    if s.split(&['/', '\\'][..])
+        .any(|seg| seg == ".." || seg == ".")
+    {
         return false;
     }
     // Control characters have no place in a routing identifier.
@@ -743,7 +751,8 @@ mod tests {
         assert!(!is_valid_data_stream_component("logs/tenant"));
         assert!(!is_valid_data_stream_component("logs\\tenant"));
         // Empty + length cap.
-        assert!(!is_valid_data_stream_component(""));
+        // Empty is accepted — DataStreamConfig::index filters empty parts.
+        assert!(is_valid_data_stream_component(""));
         assert!(!is_valid_data_stream_component(&"x".repeat(256)));
         // NUL + control chars.
         assert!(!is_valid_data_stream_component("logs\0"));

src/template.rs

@@ -1022,6 +1022,17 @@ impl UriChecker {
             .scheme_str()
             .expect("scheme present because prefix starts with http(s)://")
             .to_ascii_lowercase();
+        // Reject URI templates whose literal prefix has already crossed
+        // into the query. Composing a robust prefix pattern from operator-
+        // authored `?tenant={{ tenant }}` shapes is fragile — every
+        // starts_with/`&`-boundary combination has either a bypass or a
+        // false-positive on operator-authored static suffixes. Force
+        // dynamic routing into the path instead.
+        if uri.query().is_some() {
+            return Err(BuildError::NoStaticUriAuthority {
+                prefix: prefix.to_string(),
+            });
+        }
         match uri.authority() {
             Some(auth) if !auth.as_str().is_empty() => Ok(Self {
                 scheme,
@@ -1096,6 +1107,20 @@ impl UriChecker {
             });
         }
 
+        // 5. Reject any rendered query. `from_prefix` rejects URI templates
+        //    whose literal prefix crosses into the query, so a URI checker is
+        //    only ever built for pathful templates. A query at render time
+        //    means a field value smuggled `?...` into the path — for
+        //    example `.../ingest/{{ tenant }}` with a tenant value of
+        //    `ok?tenant=evil` renders to `.../ingest/ok` with query
+        //    `tenant=evil`; same authority + same path prefix, but the
+        //    destination now has an attacker-controlled routing parameter.
+        if uri.query().is_some() {
+            return Err(ConfineError::OutsideBase {
+                rendered: rendered.to_string(),
+                base: format!("{}://{}{}", self.scheme, self.authority, self.path_prefix),
+            });
+        }
         Ok(())
     }
 }
@@ -1657,18 +1682,6 @@ mod tests {
         ));
     }
 
-    #[test]
-    fn uri_query_only_suffix_accepted() {
-        // `https://host{{ query }}` with `query = "?x=1"` should pass.
-        let tpl = Template::try_from("https://api.internal{{ query }}").unwrap();
-        let c = ConfinementChecker::for_template(&tpl).unwrap().unwrap();
-        assert!(c.confine("https://api.internal?x=1").is_ok());
-        // http::Uri silently strips fragments per RFC 7230 (fragments are not sent
-        // to servers), so the fragment is invisible to the authority + path checks
-        // and the rendered value is accepted.
-        assert!(c.confine("https://api.internal#fragment").is_ok());
-    }
-
     #[test]
     fn uri_percent_encoded_dotdot_rejected() {
         // Servers that decode percent-encoding before path resolution can be
@@ -1771,6 +1784,51 @@ mod tests {
         ));
     }
 
+    #[test]
+    fn uri_path_field_cannot_smuggle_query() {
+        // The build-time check rejects URI templates that put a field in
+        // the query, so UriChecker only exists for pathful templates. If a
+        // rendered URI has any query at all, a field value must have
+        // smuggled it in through the path. Example: template
+        // `https://api.internal/ingest/{{ tenant }}` + tenant value
+        // `ok?tenant=evil` renders to `.../ingest/ok?tenant=evil` — same
+        // authority and path prefix, attacker-controlled query.
+        let tpl = Template::try_from("https://api.internal/ingest/{{ tenant }}").unwrap();
+        let c = ConfinementChecker::for_template(&tpl).unwrap().unwrap();
+        assert!(matches!(
+            c.confine("https://api.internal/ingest/ok?tenant=evil")
+                .unwrap_err(),
+            ConfineError::OutsideBase { .. }
+        ));
+        // Fragments are stripped by `http::Uri` before reaching the server,
+        // so a rendered `#frag` doesn't count as a query and is accepted.
+        assert!(c.confine("https://api.internal/ingest/ok#frag").is_ok());
+    }
+
+    #[test]
+    fn uri_field_in_query_rejected_at_build() {
+        // URIs whose template puts a field in the query are fragile to
+        // confine: every attempt at reasoning about `&` boundaries, static
+        // suffixes, and multi-field patterns has been either bypassable
+        // (a value like `a&tenant=other` sneaking through a bare prefix
+        // check) or false-positive (a legitimate operator-authored
+        // `&source=vector` static suffix). Reject at build.
+        for template_str in &[
+            "https://api.internal/ingest?tenant={{ tenant }}",
+            "https://api.internal/ingest?tenant=team-{{ tenant }}",
+            "https://api.internal/ingest?apiVersion=1.0&tenant={{ tenant }}",
+        ] {
+            let tpl = Template::try_from(*template_str).unwrap();
+            assert!(
+                matches!(
+                    ConfinementChecker::for_template(&tpl).unwrap_err(),
+                    BuildError::NoStaticUriAuthority { .. }
+                ),
+                "expected NoStaticUriAuthority for {template_str}"
+            );
+        }
+    }
+
     #[test]
     fn no_static_uri_authority_rejected() {
         // `https://{{ host }}/ingest` has prefix `https://` — any host can be