fix(sql): enforce the identity the playground claims to record (audit H9)

[passcod]

Fixes H9 (high) from the audit in #370.

The bug

execute_query and get_last_user_query are annotated security(("tailscale-user" = [])), and execute_query documents recording each query "under the caller's identity". Both took the extractor as Result<TailscaleUser, AppError> and called unwrap_or_default() on it.

In debug builds the extractor short-circuits to a fake admin, so nothing shows. In release, where that bypass is compiled out, a failed extraction was swallowed and the request proceeded anyway, attributed to the empty-string login.

A request reaching private-server without identity headers from a non-tailnet client IP — in-cluster traffic bypassing the Tailscale ingress, say; tailnet_guard only rejects header-less callers whose IP is in the tailnet ranges — could therefore run arbitrary read-only SQL against RO_DATABASE_URL, with the audit row's tailscale_user blank.

Every sibling read endpoint enforces with a bare _user: TailscaleUser. The one legitimate use of the Result form is commons::is_current_user_admin, which needs to answer "no" rather than reject and carries a comment saying so; these had none.

The fix

Both handlers take the bare extractor, so a failed extraction is a 401 before any work happens. The user.unwrap_or_default() lines go away, and the recorded identity is now necessarily the caller's.

No OpenAPI change: security was already declared, and the tailscale-user endpoints in this codebase don't enumerate 401 in responses (see issues.rs), so the committed spec is byte-identical — committed_spec_matches_generated passes.

On testing

There's no new test, and I don't think an honest one exists. The failure path only occurs in release builds — the extractor returns a fake admin under debug_assertions, which is exactly why every endpoint test can omit the Tailscale headers — so no test binary can reach it. The audit rates this one "hard" for the same reason.

What I did verify: the existing sql and openapi_spec suites pass unchanged.

Worth noting the same security-without-enforcement pattern remains in bestool::save_snippet (M15) and both silenced_refs list handlers (M16), and the audit suggests a sweep across every annotated handler. Not in this PR.

---

Generated by Claude Code

[passcod]

Not in this PR

Counterpoint: yes in this PR, please

[passcod]

Done — swept in this PR. 24 more handlers across 7 modules, so every security-annotated handler in fns/ now actually enforces. Verified by re-running the sweep: zero remaining.

module	handlers
backups.rs	get, list, group_schedules, type_defaults, stats, run_progress, capabilities
bestool.rs	save_snippet (M15)
certificates.rs	for_server, for_group, authority
domains.rs	zones, for_group, grant_availability
restore_replicas.rs	for_group, consumers, checks
server_groups.rs	list, server_counts, get, list_archived, search
silenced_refs.rs	list_for_server, list_for_group (M16)

save_snippet was the other Result + unwrap_or_default() case — same shape as the two sql ones, recording snippets under an empty-string author when identity failed.

Two things worth your eye before merging, since this is a wider behaviour change than the original PR:

1. The remaining 22 are all read handlers, and the split was systematic — every mutation already enforced, every read didn't. So it plausibly read as a convention rather than an oversight; I'd flagged that on fix(bestool): don't attribute a snippet to nobody when identity fails (audit M15) #445 and defaulted to leaving them. Enforcing is the audit's direction and the conservative one, and it's safe for the SPA (it already calls the enforcing mutation endpoints through the same ingress with the same headers) — what it stops is a caller with no identity at all. But if any of those reads is deliberately open to something that isn't the SPA, this will 401 it in release. Flag any and I'll drop the annotation instead of adding the extractor.

2. No spec change. security was already declared on all of them and the tailscale-user endpoints here don't enumerate 401 in responses, so the committed spec is byte-identical and committed_spec_matches_generated passes. Documenting the 401 would be more accurate, but that's a codebase-wide convention change rather than this PR's business.

That makes #445 redundant — it only carried the M15 fix and the M16 write-up. Happy to close it, or leave it for you to.

private-server suite passes apart from the domains::* tests, which fail on a different subset each run and fail identically with my change to domains.rs reverted — pre-existing flakiness in my sandbox, not from this.

---

Generated by Claude Code