## Security

#### Store::open creates index.db with permissive umask, post-open chmod has TOCTOU window
- **Difficulty:** easy
- **Location:** src/store/mod.rs:973-1014
- **Description:** `connect_with_config` lets sqlx create `index.db`, `db-wal`, and `db-shm` honoring the user's umask (typically 0o022 → 0o644 / world-readable), then post-fix permissions to 0o600 only after `connect_with(...).await`. On a multi-user box, any local user can read the freshly-born WAL/SHM during the window between SQLite's first commit and the `set_permissions` call. The cache path (`src/cache.rs:295-420`) closed exactly this gap with SEC-V1.33-2 by wrapping pool creation in `libc::umask(0o077)` + restore. The main store, which holds the most sensitive data (chunks, embeddings, summaries, notes derivatives), still has the race. Read-only opens skip the chmod entirely, so a concurrent process opening read-only never tightens perms it inherited from the original creator.
- **Suggested fix:** Mirror the cache pattern: tighten umask to 0o077 around `pool.connect_with(connect_opts)` for write opens, then restore. Keep the post-open `set_permissions` as belt-and-suspenders (matches the cache comment).

#### Daemon `tracing::info!` snapshot logs every CQS_* env var to journald
- **Difficulty:** easy
- **Location:** src/cli/watch/mod.rs:633-656
- **Description:** Daemon startup logs every `CQS_*` env var at info level into journald with a hardcoded suffix-only redactor (`_API_KEY`, `_TOKEN`, `_PASSWORD`, `_SECRET`). Misses common shapes used in this codebase: `ANTHROPIC_API_KEY` (no `CQS_` prefix is fine — it's filtered by prefix anyway, so safe), but `CQS_LLM_API_BASE` carrying `https://user:pass@host/...`, `CQS_PROXY_URL`, `CQS_AUTH_TOKEN` style names not ending in one of the four exact suffixes, or any user-extension env (`CQS_FOO_BEARER`, `CQS_VLLM_KEY`) — none redacted. journald keeps this for 30 days per the rotation policy mentioned in `auth.rs:151`. With OB-V1.30-1 surfacing info-level to journald, operators harvesting logs find these.
- **Suggested fix:** Switch the predicate from "name ends with one of 4 suffixes" to "name contains any of {KEY, TOKEN, SECRET, PASSWORD, BEARER, AUTH, CRED, PASS}" (case-insensitive substring), or take a deny-by-default approach for any name not on a small allowlist (`CQS_LLM_MODEL`, `CQS_LLM_API_BASE` minus userinfo, etc.).

#### `open_browser` on Windows passes URL with auth token to `cmd /C start "" <url>` — cmd.exe metachar parsing
- **Difficulty:** medium
- **Location:** src/cli/commands/serve.rs:127-156
- **Description:** `cmd /C start "" "<url>"` and the WSL `cmd.exe /C start "" "<url>"` branches hand the listening URL (which contains `?token=<43char>`) to `cmd.exe`. `Command::new("cmd").arg(url)` quotes the arg for CreateProcess but `cmd.exe` then re-parses, treating `&`, `^`, `|`, `>`, `<`, `(`, `)`, `%` as shell metacharacters even inside double quotes for pipe and redirect operators when they appear unquoted post-expansion. The current bind-addr + token form (`http://127.0.0.1:8080/?token=ABC`) is alphanumeric so today is safe, but: (a) any future addition to the URL (e.g. `?token=X&foo=bar`) would be split by cmd.exe into two commands; (b) `--bind` accepts arbitrary user input, so a hostile config file or shell script that sets `bind = "127.0.0.1:8080/&calc&"` — the token alphabet is enforced, but the URL host:port comes from user input via `bind_addr`. This is a latent foot-gun more than an active exploit.
- **Suggested fix:** Use `cmd.exe /D /S /C` and pass URL via `start` with a verified-safe URL; or skip `cmd.exe` and call `ShellExecuteW` via the `windows` crate / `open` crate directly so cmd.exe's parser is bypassed.

#### LLM error path logs full HTTP body at `tracing::debug!` — prompts + reflected secrets land in journald
- **Difficulty:** easy
- **Location:** src/llm/batch.rs:87-94 (and 131-137, 234-240)
- **Description:** On any non-success response from the Anthropic batch endpoint, `tracing::debug!(body = %body, ...)` logs the entire HTTP error body at debug level. Anthropic's 4xx responses regularly echo input fields (the prompt, the offending content). Operators who run `RUST_LOG=cqs=debug` (recommended in CONTRIBUTING.md) get the full prompt — which on cqs's pipeline is *the indexed source code, signatures, and surrounding context* — into journald. If the input body contained anything sensitive (private repo content, secrets accidentally indexed via `extra_paths`), they're now in the 30-day journal. The user-visible `LlmError::Api { message }` correctly uses `redacted_api_message`; the debug log undoes that.
- **Suggested fix:** Drop `body = %body` from the debug log entirely. Keep `body_len` and `parsed_anthropic_message` (if you trust Anthropic's `error.message` not to echo prompts — it does sometimes, so consider dropping that too). For local-debugging, gate the full body behind `tracing::trace!`.

#### `slot_dir` / `slot_config_path` build paths from unvalidated slot name (read path)
- **Difficulty:** easy
- **Location:** src/slot/mod.rs:194-244 (consumers in src/lib.rs:377, src/cli/store.rs:49, src/cli/watch/mod.rs:520, etc.)
- **Description:** `slot_dir(project_cqs_dir, slot_name)` and `slot_config_path` do a raw `project_cqs_dir.join(SLOTS_DIR).join(slot_name)` with no `validate_slot_name` call. The write paths (`write_slot_model`, `slot_create`, etc.) all validate up front. The read paths — `read_slot_model:257`, the public `cqs::resolve_slot_dir:377` (lib export), and every `slot_dir(...)` call site that takes a slot name from `CQS_SLOT` env, `--slot` CLI flag, or `.cqs/active_slot` pointer — do not. `resolve_slot_name` validates names that come through *its* code path, but `slot_dir` is a public helper and external code (or future call sites) can hand it `"../../etc/passwd"`, getting a `<project>/.cqs/slots/../../etc/passwd` path. `Path::join` does not normalize `..`. Today the consumers pass the path to SQLite, which would just fail noisily — not a live exploit — but a future caller that uses the path for `read_dir` / file enumeration will leak.
- **Suggested fix:** Make `slot_dir` and `slot_config_path` either return `Result<PathBuf, SlotError>` and call `validate_slot_name` internally, or accept a typed `&ValidatedSlotName` newtype that can only be constructed via `validate_slot_name`. The current shape invites the bug.

#### `cqs serve` — full request URI not redacted in axum extractor errors
- **Difficulty:** medium
- **Location:** src/serve/handlers.rs:147-159, 244-270
- **Description:** P1.11 (already fixed) made the TraceLayer span record `path` only, not the full URI, so `?token=…` doesn't bleed into structured logs. But when axum's built-in `Query<T>` extractor fails (malformed query string, type-mismatched arg), axum's default rejection emits the *full URI fragment* in its 400 response body and emits a debug log via `tower_http`. The `cqs serve` router includes a `TraceLayer::new_for_http()` that captures `make_span_with` for the *successful* path, but extractor failures fall through to the layer's default error response handler which logs the unparsed query. A client sending `GET /api/graph?token=ABC&max_nodes=notanumber` produces a 400 whose body and the trace event both include `token=ABC`. The auth middleware sits *inside* the extractor for query handlers (it's added via `from_fn_with_state` in `build_router:320` after the routes), so an attacker can't reach the handler — but the 401 path in `enforce_auth:657-662` already strips query strings. The extractor-failure path doesn't.
- **Suggested fix:** Add a custom `axum::error_handling` layer that intercepts 400-class extractor rejections and rewrites the response body / log fields to drop any `token` parameter. Or: parse query strings manually in handlers (verbose) so the extractor's error path can't fire on auth-bearing requests.

#### `validate_repo_id` allows `..` in HF model name → optimum subprocess fetch
- **Difficulty:** easy
- **Location:** src/cli/commands/train/export_model.rs:17-30
- **Description:** The repo-id allowlist permits `[A-Za-z0-9._/-]` which lets `..` through (`.` in the set + `.` in the set = `..`). A repo id like `org/../../etc` or `../foo` is accepted and passed verbatim to `optimum.exporters.onnx --model <repo>`. Optimum first tries to interpret `--model` as a local path before falling through to HF Hub. A repo id `../../home/user/secrets` would cause optimum to attempt loading from `<cwd>/../../home/user/secrets` as a local model directory, surfacing the existence (and possibly partial content via error messages) of that path in the user-visible failure. Low impact today — failure surface only — but the in-source comment on line 14 says "HuggingFace repo IDs are documented as `[A-Za-z0-9._/-]` only" and HF Hub explicitly rejects `..` for this reason; cqs is laxer than the upstream contract.
- **Suggested fix:** Add `if repo.contains("..") { bail!(..) }` to `validate_repo_id`, mirroring the `embedder/models.rs:698` SEC-28 check on the user-config repo path which already excludes `..`.

#### Audit-mode TOCTOU on Windows — no ACL or atomic create
- **Difficulty:** medium
- **Location:** src/audit.rs:140-160
- **Description:** Acknowledged at audit-triage P4-19 as a known issue. Recording sub-aspect: even on Unix, `OpenOptions::new().create(true).truncate(true).open(&tmp_path)` creates the temp file with the user's umask if the `.mode(0o600)` builder method is somehow bypassed by a future refactor — `.mode()` is on `OpenOptionsExt`, not on the cross-platform `OpenOptions`, so a clippy lint or a Windows-clean refactor could silently drop it. Hold-out from #1107 / #1108 era: the same pattern lives in `notes.rs`, `project.rs`, `slot/mod.rs`, `audit.rs` — five separate atomic-write paths with the same Unix-only chmod, three of which (`audit`, `project`, `slot`) overlap. None has a Windows equivalent.
- **Suggested fix:** Centralize the "private temp file" pattern in `crate::fs` (next to `atomic_replace`) so a single `private_tempfile_for(path)` helper sets `.mode(0o600)` on Unix and applies a sensible Windows ACL (deny `Authenticated Users : Read`) via `windows::Win32::Security`. All five callers reduce to one line and Windows finally gets parity.

#### `serve` request-body limit (64 KiB) sits outside the auth layer — pre-auth memory pressure
- **Difficulty:** easy
- **Location:** src/serve/mod.rs:341-344
- **Description:** Comment says the body limit "applies even to rejected requests (preventing OOM-then-401 attacks)" — it does, but only because axum applies layers in the documented order. A 64 KiB cap means an attacker on the host-allowlist (or via a WebSocket-upgrade attempt) can fan out N concurrent connections, each holding a 64 KiB body buffer pre-auth. With the default tokio worker pool, the pre-auth body buffers can cumulatively allocate ~64 KiB × N where N is bounded only by the OS file descriptor limit, since the `blocking_permits` semaphore (P2.76) gates only the blocking pool, not the async accept loop. For a localhost-bound box this is academic; for the `--bind 0.0.0.0` path with `--no-auth` documented carve-out, less so.
- **Suggested fix:** Cap concurrent inbound connections with `tokio::sync::Semaphore` outside the body-limit layer (per-IP would be ideal but requires extracting the peer addr). Or document explicitly that operators must use a reverse proxy in front for the LAN-bind path.

#### Daemon socket cleanup races a hostile local user — bind succeeds with TOCTOU
- **Difficulty:** medium
- **Location:** src/cli/watch/mod.rs:540-611
- **Description:** Cleanup logic uses `symlink_metadata` + `remove_file` + `bind`. Between the `remove_file` and `UnixListener::bind`, a local user can `socket(AF_UNIX) + bind(sock_path)`, planting their own socket — the next `bind()` call from the daemon then either fails (and the daemon exits) or, on platforms where `bind()` follows symlinks (Linux does not, BSD differs), connects through. The umask 0o077 wrap closes the *permission* race but not the *identity* race. If the daemon socket lives under `/tmp/cqs-<hex>.sock` (the `XDG_RUNTIME_DIR` unset fallback at `daemon_translate.rs:228`), `/tmp` is world-writable so the race window matters. The hex is `blake3(canonical_project_path)`; an attacker who knows the project root (often visible in the systemd unit file or `ps`) can pre-compute it and squat on the socket name during early daemon boot.
- **Suggested fix:** Bind under `XDG_RUNTIME_DIR` whenever it's set (already done) and refuse to start when it isn't — or `mkdir -m 0700 /tmp/cqs-<uid>/` first and bind inside that, so the parent dir's perms gate squatters even when `/tmp` itself is sticky-but-shared. Document the `XDG_RUNTIME_DIR`-required posture in SECURITY.md.

DONE
