ANW-42 Telemetry: reject an uptrace DSN with a path, query, or fragment tail

The part after the credentials was taken as the host unchecked, so a tail
survived into the endpoint base and the per-signal path appended later
landed inside it. Export was dead while startup logged it as enabled.

Assumed reject over strip: the reporter asked for it and resolve already
documents that an unusable DSN fails at startup; flag if silently
normalizing the DSN is wanted instead.
Assumed --otlp-endpoint stays unvalidated: a path prefix is legitimate
there, and its query-tail case is out of this scope.

README.md

@@ -69,7 +69,7 @@ anwesen version
 | `--bind <addr:port>`        | `ANWESEN_BIND`                 | `127.0.0.1:8080`       | Listen address for `serve`.                                                   |
 | `--log-level <level>`       | `ANWESEN_LOG_LEVEL`            | `info`                 | `error`, `warn`, `info`, `debug`, or `trace`.                                  |
 | `--query <query-string>`    | --                             | _required for `merge`_ | A `/query` query string: frontmatter predicates plus `__anw-` controls.       |
-| `--uptrace-dsn <dsn>`       | `ANWESEN_UPTRACE_DSN`          | unset                  | uptrace DSN (`https://<token>@api.uptrace.dev`). Excludes `--otlp-endpoint`.  |
+| `--uptrace-dsn <dsn>`       | `ANWESEN_UPTRACE_DSN`          | unset                  | uptrace DSN (`https://<token>@host[:port]`). No path or query. Excludes `--otlp-endpoint`. |
 | `--otlp-endpoint <url>`     | `ANWESEN_OTLP_ENDPOINT`        | unset                  | OTLP/HTTP base URL. Excludes `--uptrace-dsn`.                                 |
 | `--otlp-header <key=value>` | `ANWESEN_OTLP_HEADERS`         | none                   | Extra export header, repeatable. The env var takes a comma-separated list.    |
 | `--otlp-slow-request-ms`    | `ANWESEN_OTLP_SLOW_REQUEST_MS` | `500`                  | Requests at or over this duration, or answering 5xx, also export a span.      |

src/telemetry.rs

@@ -131,6 +131,12 @@ fn parse_headers(specs: &[String]) -> anyhow::Result<Vec<(String, String)>> {
 /// Parse an uptrace DSN (`https://<token>@host[:port]`) into the OTLP
 /// endpoint base URL and the `uptrace-dsn` header uptrace expects. The full
 /// DSN is echoed as the header value per uptrace's ingest contract.
+///
+/// The part after the credentials must be exactly `host[:port]`. A path,
+/// query, or fragment tail is rejected: it would survive into the endpoint
+/// base, and the per-signal path appended in [`TelemetryInner::new`] would
+/// land inside it, giving a URL that no collector answers
+/// ([ANW-42](https://crvrs.youtrack.cloud/issue/ANW-42)).
 fn parse_uptrace_dsn(dsn: &str) -> anyhow::Result<(String, (String, String))> {
     let dsn = dsn.trim();
     let (scheme, rest) = dsn
@@ -144,10 +150,57 @@ fn parse_uptrace_dsn(dsn: &str) -> anyhow::Result<(String, (String, String))> {
     if host.is_empty() {
         bail!("uptrace DSN has no host");
     }
+    validate_dsn_authority(host)?;
     let endpoint = format!("{scheme}://{host}");
     Ok((endpoint, ("uptrace-dsn".to_string(), dsn.to_string())))
 }
 
+/// Check that a DSN's post-credentials part is `host[:port]` and nothing
+/// else. `host` is already non-empty and free of a trailing slash.
+fn validate_dsn_authority(host: &str) -> anyhow::Result<()> {
+    if let Some(bad) = host.find(['/', '?', '#']) {
+        // Name the offending character: the operator pasted a URL where a
+        // DSN was wanted, and the tail is what has to go.
+        let tail = &host[bad..];
+        bail!(
+            "uptrace DSN host {host:?} has a trailing {tail:?} \
+             (expected https://<token>@host[:port], with no path or query)"
+        );
+    }
+
+    // An IPv6 literal is bracketed, so only a colon past the closing bracket
+    // separates the port.
+    let (name, port) = if host.starts_with('[') {
+        let close = host
+            .find(']')
+            .with_context(|| format!("uptrace DSN host {host:?} has an unclosed IPv6 bracket"))?;
+        let (name, after) = host.split_at(close + 1);
+        if after.is_empty() {
+            (name, None)
+        } else {
+            let port = after
+                .strip_prefix(':')
+                .with_context(|| format!("uptrace DSN host {host:?} is not host[:port]"))?;
+            (name, Some(port))
+        }
+    } else {
+        match host.rsplit_once(':') {
+            Some((name, port)) => (name, Some(port)),
+            None => (host, None),
+        }
+    };
+
+    if name.is_empty() || name == "[]" {
+        bail!("uptrace DSN has no host");
+    }
+    if let Some(port) = port
+        && (port.is_empty() || !port.bytes().all(|b| b.is_ascii_digit()))
+    {
+        bail!("uptrace DSN host {host:?} has a non-numeric port (expected host[:port])");
+    }
+    Ok(())
+}
+
 /// Semantic route bucket for the `http.route` label. Coarser than the axum
 /// template on purpose: `/notes/{*path}` serves both a note fetch and a
 /// folder listing, and "304 share of note fetches" needs the two apart. The
@@ -578,6 +631,76 @@ mod tests {
         assert!(err.to_string().contains("scheme"));
     }
 
+    /// Every shape that would otherwise leave a tail in the endpoint base,
+    /// so the `/v1/metrics` appended later lands inside it (ANW-42).
+    #[test]
+    fn resolve_rejects_dsn_with_a_tail() {
+        for dsn in [
+            "https://tok@api.uptrace.dev?grpc=4317",
+            "https://tok@api.uptrace.dev/v1",
+            "https://tok@api.uptrace.dev#frag",
+            "https://tok@api.uptrace.dev:4318?grpc=4317",
+            "https://tok@api.uptrace.dev/?grpc=4317",
+        ] {
+            let err = TelemetryConfig::resolve(RawTelemetryArgs {
+                uptrace_dsn: Some(dsn.into()),
+                ..raw()
+            })
+            .unwrap_err();
+            assert!(err.to_string().contains("trailing"), "{dsn}: {err}");
+        }
+    }
+
+    #[test]
+    fn resolve_rejects_dsn_with_a_bad_port() {
+        for dsn in [
+            "https://tok@api.uptrace.dev:grpc",
+            "https://tok@api.uptrace.dev:",
+            "https://tok@[::1]:grpc",
+        ] {
+            let err = TelemetryConfig::resolve(RawTelemetryArgs {
+                uptrace_dsn: Some(dsn.into()),
+                ..raw()
+            })
+            .unwrap_err();
+            assert!(err.to_string().contains("port"), "{dsn}: {err}");
+        }
+    }
+
+    #[test]
+    fn resolve_rejects_dsn_with_no_host_before_the_port() {
+        let err = TelemetryConfig::resolve(RawTelemetryArgs {
+            uptrace_dsn: Some("https://tok@:4318".into()),
+            ..raw()
+        })
+        .unwrap_err();
+        assert!(err.to_string().contains("no host"), "{err}");
+    }
+
+    /// The shapes a real DSN takes still resolve, port and IPv6 included.
+    #[test]
+    fn resolve_accepts_plain_host_forms() {
+        for (dsn, endpoint) in [
+            ("https://tok@api.uptrace.dev", "https://api.uptrace.dev"),
+            ("https://tok@api.uptrace.dev/", "https://api.uptrace.dev"),
+            (
+                "https://tok@collector.internal:4318",
+                "https://collector.internal:4318",
+            ),
+            ("http://tok@[::1]:4318", "http://[::1]:4318"),
+            ("http://tok@[::1]", "http://[::1]"),
+            ("https://api.uptrace.dev", "https://api.uptrace.dev"),
+        ] {
+            let cfg = TelemetryConfig::resolve(RawTelemetryArgs {
+                uptrace_dsn: Some(dsn.into()),
+                ..raw()
+            })
+            .unwrap_or_else(|e| panic!("{dsn}: {e}"))
+            .expect("telemetry on");
+            assert_eq!(cfg.endpoint, endpoint, "{dsn}");
+        }
+    }
+
     #[test]
     fn classify_route_buckets() {
         assert_eq!(classify_route("/health"), "health");