## Security

> All eight previously-listed findings (DNS-rebinding host-allowlist,
> XSS via `body.slice()` → `innerHTML`, IN-list / SQL-LIMIT DoS, LIKE-wildcard
> injection in `file` and `tests-cover`, `cmd_serve` / `xdg-open` URL hazard,
> unauthenticated serve) are already in `docs/audit-triage-v1.30.0.md` as
> SEC-1 … SEC-8. SEC-7 (no-auth) is fixed by #1118. The findings below are
> NEW exposures introduced by the v1.30.0 surface (auth token plumbing,
> tracing, host-header edge cases) that the prior pass did not catch.

#### Auth token leaked into tracing spans by `TraceLayer::new_for_http()`
- **Difficulty:** easy
- **Location:** `src/serve/mod.rs:195` (`TraceLayer::new_for_http()`), interacts with `src/serve/auth.rs:194-232` (`enforce_auth`) and `src/cli/commands/serve.rs:73-76` (token-bearing URL passed to `xdg-open`)
- **Description:** `auth.rs` is meticulous about not leaking the token: constant-time compare, `HttpOnly; SameSite=Strict` cookie, redirect that strips `?token=` from the address bar, and an explicit comment at `auth.rs:226-228` claiming "Tracing happens once per launch (banner) and never per-request — auditors can grep for the count of 401s without seeing tokens." That contract is silently broken by `TraceLayer::new_for_http()`, which is wired as the outermost layer (`mod.rs:195`) and thus runs **before** `enforce_auth`. The default `MakeSpan` of `tower-http` 0.6's TraceLayer records `http.uri` (full path + query string) on every request span. So the very first browser navigation `GET /?token=<43 chars>` lands the token in the span at DEBUG, and any operator running with `--log-level=debug`, `RUST_LOG=tower_http=debug`, or `RUST_LOG=info,tower_http=debug` (commonly recommended for diagnosing serve issues) writes the token to journald / log files / pipes that long-outlive the per-launch token. The same applies to `xdg-open`: the URL passed to `Command::new("xdg-open")` (`serve.rs:97`) typically ends up in shell history, in xdg-open's own logging, and in the browser's session-restore database — but those are user-local and arguably the intended cost of `--open`. The TraceLayer leak is the surprise: it converts a defensible "token visible to your local browser" model into "token visible to anyone who can read your logs." Coupled with the loud-warning banner that prints the token to stdout (`mod.rs:113-116`), captured stdout in CI / systemd `StandardOutput=` ends up in journald too. Severity: medium — token rotates per-launch, so leaked tokens are bounded by uptime, but a 30-day journal retention easily covers a long-running daemon.
- **Suggested fix:** Customize TraceLayer's `MakeSpan` to scrub the query string before recording: `TraceLayer::new_for_http().make_span_with(|req: &Request<_>| { let path = req.uri().path(); tracing::info_span!("http_request", method = %req.method(), path) })`. Alternatively, redirect-then-trace by reordering the layers so the auth redirect runs before TraceLayer sees the URI — but that swaps the ordering invariant the comment relies on, and breaks the "401 responses still get traced" guarantee. The MakeSpan override is the surgical fix.

#### `enforce_host_allowlist` lets a missing-`Host:` request bypass DNS-rebinding protection
- **Difficulty:** easy
- **Location:** `src/serve/mod.rs:234-251` (specifically the `None => Ok(next.run(req).await)` branch at line 240)
- **Description:** The allowlist middleware passes through when `Host:` is absent: `match req.headers().get(header::HOST) { None => Ok(next.run(req).await), … }`. The doc-comment justification at `mod.rs:230-233` is "HTTP/1.1 requires one and hyper always provides one on real traffic, but unit tests built via `Request::builder()` without a `.uri()` that includes a host don't get one synthesized, and we'd rather not break that ergonomic." This privileges test ergonomics over runtime safety. (1) HTTP/1.0 does not require Host — a hand-crafted `GET /api/chunk/<id> HTTP/1.0\r\n\r\n` from `nc 127.0.0.1 8080` reaches the handler with zero Host header and zero allowlist check. With `--no-auth` the entire DNS-rebinding protection is bypassed for that request shape. With auth, the auth layer still gates it (so the immediate damage is bounded), but the v1 spec explicitly relies on the host allowlist as defense-in-depth, and this hole quietly disables it. (2) Browsers in error states (eg. `XMLHttpRequest` against a non-standard scheme handler) have historically dropped the Host header. (3) An axum middleware quirk: the middleware sees the raw header map; some HTTP libraries emit `Host:` with empty value (`Host: \r\n`), which `.to_str().unwrap_or("")` collapses to `""`, but `to_str()` on an empty `HeaderValue` actually returns `Ok("")`, which then fails the allowlist check (good). But a request with a non-ASCII byte in Host (`Host: \xff`) fails `.to_str()` → `unwrap_or("")` → empty → rejected. So the only real escape is the literal-missing case at line 240 — which is the most exploitable variant. The "don't break test ergonomics" excuse is also questionable: tests inside the crate can call `enforce_host_allowlist_with_default_allowed` or stamp a Host header in fixtures (`Request::builder().header(HOST, "127.0.0.1:8080").uri("/foo").body(...)`), which is a one-line change in the existing test file.
- **Suggested fix:** Reject missing-Host as a malformed request: replace `None => Ok(next.run(req).await)` with `None => Err((StatusCode::BAD_REQUEST, "missing Host header"))`. Update tests in `src/serve/tests.rs` to set a Host header explicitly — those tests are already exercising the real auth + host paths; adding a header is consistent with what hyper does on real traffic and removes a real bypass.

#### `--bind 0.0.0.0` allows LAN attackers to hit the server but the host-allowlist breaks the legitimate browser path — operator footgun pushes them to `--no-auth`
- **Difficulty:** medium
- **Location:** `src/serve/mod.rs:207-218` (`allowed_host_set`), `src/cli/commands/serve.rs:27-37` (warning gating)
- **Description:** With `cqs serve --bind 0.0.0.0`, the allowlist ends up containing `{localhost, localhost:8080, 127.0.0.1, 127.0.0.1:8080, [::1], [::1]:8080, 0.0.0.0:8080, 0.0.0.0}`. A LAN client navigating to `http://192.168.1.5:8080/` sends `Host: 192.168.1.5:8080`, which is NOT in the allowlist → 400 "disallowed Host header". So `--bind 0.0.0.0` is effectively unusable from any non-loopback peer despite the operator having explicitly opted into LAN binding. The expected operator response is to add `--no-auth` (which doesn't help — the host check is in front of auth) or to dig into the source and discover that `--bind <actual-IP>` is the real path. Worse: the only "bind-without-auth-is-loud" warning at `serve.rs:27-37` triggers only when `bind != "127.0.0.1" && bind != "localhost" && bind != "::1"`, but treats `0.0.0.0` as non-localhost-y and fires the warning — pushing operators toward `--no-auth` to "make it work." The result is a UX that punishes the secure path (auth on + bind 0.0.0.0 → 400) and rewards the insecure path (`--no-auth` → works for LAN). Threat model: an operator legitimately needs `cqs serve` reachable from a teammate's laptop on the same VLAN, sees auth-on returns 400 on the laptop, falls back to `--no-auth`, exposes the entire indexed corpus.
- **Suggested fix:** When `bind == 0.0.0.0` (or `::`), the host allowlist should accept any RFC-1918 / loopback / link-local Host plus the bind port. Cleaner: when binding to a wildcard, populate the allowlist from `nix::ifaddrs` / `if_addrs` so the actual interface IPs are accepted. Even simpler: when `bind_addr.ip().is_unspecified()`, skip the host-header check entirely (auth is the only gate) and emit a one-line stderr warning at startup explaining that the host-header DNS-rebinding protection is disabled in wildcard mode.

#### Token printed to stdout is captured by systemd `journald` — long-lived servers leak their token to a 30-day-retention log
- **Difficulty:** easy
- **Location:** `src/serve/mod.rs:111-117` (`println!("cqs serve listening on http://{actual}/?token={}")`)
- **Description:** The auth comment at `auth.rs:226-228` claims tokens never reach the structured log stream. The startup banner does write the token to stdout, intentionally, so a copy-paste from the operator's terminal lands in an authenticated session. But a long-running `cqs serve` deployed under systemd (`StandardOutput=journal`, the default) or via container (`docker run` capturing stdout into the container log driver) immediately persists the token banner into a log retention store the user never thinks about. The user's mental model is "the token only shows up in my terminal scrollback"; the reality is "anyone with `journalctl -u cqs-serve` (or the container log endpoint) can read the token until log rotation." Combined with that the token doesn't rotate during a long-lived launch, a 30-day-old banner is a 30-day-valid credential. Severity: low without a public binding (loopback only); medium with `--bind` to a LAN IP and any user on the box that has `journalctl -u`. Note: systemd `cqs-watch.service` already runs `cqs watch --serve`, not the HTTP server, but a future `cqs-serve.service` (or anyone wrapping `cqs serve` in systemd) inherits this trap silently.
- **Suggested fix:** Two complementary mitigations: (1) print the token banner only to a controlling-tty-detected stderr (`atty::is(Stream::Stderr)`), or to a write-only file (`stdout` redirected to `/dev/null` when not interactive), so it doesn't leak to journald by default; (2) document in the listening banner that the token is per-launch and the user should rotate by restarting after disclosure. Optional: write the token to a `0o600`-permissioned file in `$XDG_RUNTIME_DIR` and print only the file path, with a `cqs serve token` subcommand that re-emits it on demand — same pattern Jupyter Lab uses since 2018.

#### Auth state ignored by `quiet=true` callers — internal API lets tests build a router that omits auth without auditing
- **Difficulty:** easy
- **Location:** `src/serve/mod.rs:78-83` (`run_server` signature accepts `auth: Option<AuthToken>`), `src/serve/mod.rs:154-178` (`build_router`)
- **Description:** `run_server` and `build_router` both take `auth: Option<AuthToken>`. Passing `None` silently disables auth — there is no `AuthRequired` enum, no `Either<NoAuth, Auth>` type. The callsite in `cmd_serve` (`serve.rs:61-65`) correctly defaults to `Some(random())` and only opts into `None` on `--no-auth`, but the type signature does not constrain future internal callers (e.g. an embedded `cqs serve` for a doctor-style smoke test, an alternate CLI surface, or a feature-gated dev mode) from passing `None` and quietly disabling auth. There is no compile-time gate or runtime warning when the auth-disabled path is taken; the only signal is the `eprintln!` at `mod.rs:120-123`, which is only emitted when `quiet == false`. So a future `run_server(store, addr, true, None)` ships a fully open server with zero output — a regression that nothing in the type system catches. The pattern matters because the equivalent invariant in `cqs ref add` is enforced via type and validate-by-construction, but here the "default secure" property lives only in convention.
- **Suggested fix:** Replace `Option<AuthToken>` with a `enum AuthMode { Required(AuthToken), Disabled { ack: NoAuthAcknowledgement } }` where `NoAuthAcknowledgement` is a `pub` zero-sized type only constructable inside `cmd_serve` after the `--no-auth` flag is parsed. Future internal callers physically cannot instantiate the disabled variant without intentionally importing the proof type. Alternative: keep `Option<AuthToken>` but add a `tracing::error!` (not warn) in the `None` branch of `build_router` so any test or future caller that disables auth shows up loudly in logs.

#### `AuthToken::from_string` is `#[cfg(test)]` but exposed via `pub(crate)` — a future code path can construct a token with CR/LF and crash the server
- **Difficulty:** hard
- **Location:** `src/serve/auth.rs:75-78` (`from_string`), `src/serve/auth.rs:218-220` (`HeaderValue::from_str(&cookie).expect(…)`)
- **Description:** `AuthToken::from_string` is gated on `#[cfg(test)]` so it's not callable from production code today. But the contract of `AuthToken` ("alphabet is URL-safe base64; HeaderValue construction infallible" — `auth.rs:215-218`) is enforced only by the docstring on `random()`, not by the type. If a future refactor lifts the cfg-gate (e.g. for a "fixed token from env var" feature, which is a reasonable ask for scripted automation that wants stable tokens across launches), an attacker who controls that env var can write `CR/LF` into the token and crash the worker on every redirect — `HeaderValue::from_str` rejects bytes that aren't valid HTTP header bytes, and the `.expect(…)` at line 218 panics. The crash is per-request (axum catches handler panics into a 500), so it's not a server-killer, but it's a guaranteed 500 for any request that hits the query-param redirect path. More subtly: a token containing `;` or `,` would split the cookie syntax and let an attacker who knows the format inject a second cookie pair, e.g. token = `validbase64; admin=true; ` — then on the redirect `Set-Cookie: cqs_token=validbase64; admin=true; ; Path=/; HttpOnly; SameSite=Strict`. The cookie would be malformed and most browsers would reject the whole header, but the principle of "only ever construct a HeaderValue from validated bytes" is broken.
- **Suggested fix:** Make the alphabet a structural property of `AuthToken`: have `random()` return `AuthToken(String)` where the wrapped string is verified at construction time to be `[A-Za-z0-9_-]+` and panic on construction (not on use) if it's not. For `from_string`, do the same validation; tests that want to build a deterministic token still get one, but they cannot smuggle invalid bytes in. With the alphabet enforced at the type level, the `.expect(…)` at `auth.rs:218` becomes a real safety proof rather than a fragile docstring.

#### `cqs serve` request body is unbounded — POST or chunked uploads can exhaust memory; no `RequestBodyLimitLayer`
- **Difficulty:** easy
- **Location:** `src/serve/mod.rs:154-196` (`build_router` — no body-limit layer in the chain)
- **Description:** The router declares only `GET` routes (`routes.rs:160-168`), and axum returns 405 for non-GET on those routes. But axum still BUFFERS the request body before dispatching — and for a route declared as `get(...)`, axum 0.7 reads the body into memory before deciding to 405. There's no `tower_http::limit::RequestBodyLimitLayer` in the chain. So an attacker (after passing host + auth) can `POST /api/stats` with `Content-Length: 99999999999` and `Transfer-Encoding: chunked`, and axum will buffer up to the OS-level read limit before responding 405. With the auth layer running *outside* the body read, the 405 path can be reached by any authenticated client. Memory pressure proportional to body size, repeated forever. Since auth is in front, an external attacker without a token can't do this — but a multi-user box where one user has the token (per the `journald` finding above) can. Also: for the GET-only routes that DO expect query strings (e.g. `/api/search?q=…`), there is no `Content-Length` cap on the body that some clients might send anyway. axum forgivingly ignores the body for GET, but not before reading it into memory.
- **Suggested fix:** Add `RequestBodyLimitLayer::new(64 * 1024)` to the layer chain (sits inside CompressionLayer, outside auth so the limit applies to even rejected requests). 64 KiB is plenty for query strings and cookies; legitimate clients never approach it. Tower-http already has the layer; no new dep.

#### `Path=/` cookie scope plus 127.0.0.1 sharing — multiple `cqs serve` instances on the same host share cookies and can hijack each other
- **Difficulty:** hard
- **Location:** `src/serve/auth.rs:211-214` (`Set-Cookie: cqs_token={token}; Path=/; HttpOnly; SameSite=Strict`)
- **Description:** Two `cqs serve` instances on the same machine, on different ports (e.g. project A on 8080, project B on 8081), both share the same cookie origin from the browser's perspective: cookies on `localhost`/`127.0.0.1` are scoped by host but NOT by port. Both servers set `cqs_token=...` with `Path=/`. A user authenticates to project A, gets cookie `cqs_token=A_TOKEN`. They later visit `http://127.0.0.1:8081/?token=B_TOKEN` for project B. The redirect sets `cqs_token=B_TOKEN`, OVERWRITING the project A cookie in the browser jar (same name + host + path). Now the user's tab on project A is silently logged out and (worse) any link they click through to project A sends `cqs_token=B_TOKEN`, which fails ct_eq and 401s. Browsers do scope cookies by `Path=/` but not by port, so this is a fundamental browser-cookie limitation, not a server bug. It's still a real footgun for any user running two `cqs serve` instances, plus a downgrade vector: an attacker who can make the user visit their own attacker-controlled `cqs serve` on a port they control — say, by phishing the user into running `cqs serve --bind 127.0.0.1 --port 8081` against a malicious project — replaces the legitimate project's cookie. Combined with SameSite=Strict-bypass via top-level navigation (the user *is* navigating top-level), the attacker can drop any cookie they like into the victim's localhost cookie jar. Mitigation note: the comment at `auth.rs:42-47` ("Pinned to `cqs_token` so a future second-server instance running in another tab on the same host uses a different cookie path") explicitly acknowledges the issue but does not actually solve it.
- **Suggested fix:** Use `__Host-` cookie prefix (`__Host-cqs_token`) per RFC 6265bis — requires `Path=/`, `Secure`, and forbids `Domain`. The `Secure` requirement means the cookie won't stick on plain HTTP, which is a problem for localhost. Alternative: include the bind port in the cookie name (`cqs_token_8080`) so two instances don't collide. Best alternative: set `Path=/api/__cqs_<port>/` and have the auth layer rewrite the request path — heavy. Pragmatic: set the cookie name from a hash of `(bind_addr, launch_time)` so two instances don't collide and a new launch invalidates the old cookie automatically. The knob count rises but the multi-instance footgun goes away.

## One-line summary

Eight new findings on top of SEC-1 … SEC-8 (already triaged). All are in the v1.30.0 auth surface (#1118): TraceLayer leaks the token via URI logging despite the careful HttpOnly/redirect handoff; missing-Host requests bypass DNS-rebinding protection; `--bind 0.0.0.0` is broken in a way that pushes operators to `--no-auth`; the launch banner persists tokens to journald for log-retention lifetime; `Option<AuthToken>` permits silently building a no-auth router; `from_string` is cfg-gated but the alphabet invariant relies on a docstring rather than a type; no request-body-limit layer; cookies aren't port-scoped on localhost so two instances stomp each other.
