fix: version the dashboard asset URLs, and stop preflight crying wolf

[OsherElhadad]

Two deploy-hygiene defects found during this morning's redeploy. The first shipped a
broken UI to production; the second makes the diagnostic that should have caught it
untrustworthy.

1. Dashboard assets had no cache-busting

dash/ui.go serves index.html as no-cache and its assets as
public, max-age=3600, while the markup named them unversioned — src="app.js".

So a deploy that changed index.html, app.js, tools.js and style.css together
served new HTML against an hour-old app.js to every returning visitor. Observed
symptom, reported by the service owner: the new buttons rendered and did nothing, the
previous 8-second refresh interval kept running, and it healed itself an hour later —
which is indistinguishable from a deploy that silently failed.

Confirmed against the live service before the fix:

asset	Cache-Control	URL in markup
index.html	no-cache	—
app.js	public, max-age=3600	src="app.js"
tools.js	public, max-age=3600	src="tools.js"
style.css	public, max-age=3600	href="style.css"

After — the served page (the HTML is already no-cache, so it is the right place
to inject):

href="style.css?v=fd6f54029fdc"
src="app.js?v=fd6f54029fdc"
src="tools.js?v=fd6f54029fdc"

max-age=3600 is now correct rather than dangerous, because the stale URL is never
requested again.

Three deliberate choices:

• Content hash, not the build commit — a dirty local rebuild also gets fresh URLs.
• One token across all assets, not one per asset. tools.css is loaded by
  tools.js, not by the HTML, so a per-asset hash of tools.js would not move when
  only tools.css changed — leaving the identical stale-pair bug one level down.
• The rewrite is driven by the embedded directory listing, not a string match, so
  it is blind to attribute order, quoting and element; it covers a new asset the day it
  lands; and it is idempotent.

2. install.sh preflight failed on a healthy host, twice

Both were false negatives, and both had to be disproved by hand before this morning's
deploy could proceed.

• rclone missing — resolved with a bare command -v, which under sudo uses
  secure_path (/sbin:/bin:/usr/sbin:/usr/bin) and therefore cannot see
  /usr/local/bin, where rclone is installed. The backup unit inherits systemd's PATH
  and had been uploading to Box successfully all along. Tools are now resolved against
  the PATH the unit will run with, python3 included.
• credential for SOME_VAR — not configured anywhere. It is the placeholder inside
  the comment that documents key_env in upstreams.yaml, and it ships with the
  file, so this fired on every host. Comments are now stripped before extraction,
  trailing ones included, and a configured key_env is still checked.

Acceptance test is the real host. Before: Preflight FAILED with those two lines.
After, from this branch, against the live /etc/context-guru/upstreams.yaml:

  ✓ python3 (nightly control-db backup)
  ✓ rclone (nightly control-db backup)
  ✓ allow-list /etc/context-guru/upstreams.yaml
  ✓ credential drop-in present
  ✓ MANAGER_EMAIL is set
  ✓ rclone config for cold storage

Preflight PASSED

A preflight that cries wolf is the one people learn to ignore.

Tests

Four, one per property:

• TestServedUIVersionsEveryAssetItReferences — every asset the served page references
  carries a token
• TestAssetVersionFollowsAssetBytes — the token changes when the bytes change
• TestPreflightReadsKeyEnvFromConfigNotComments
• TestPreflightResolvesToolsOnTheServicePATH

CGO_ENABLED=1 go test -race -count=1 ./... → 25 packages ok, exit 0.
go vet ./... clean. gofmt -l . empty. No schema change; schemaVersion untouched.

[OsherElhadad]

Post-merge review of a33c9ac. Both fixes do what they claim and the implementations are sound — the rewrite is computed once at init, not per request, and the CSP is byte-identical to HEAD~1. Two follow-up items, then nits. Nothing here justifies a revert.

Every claim below was mutation-tested: each fix was broken deliberately and the tests re-run, since that is the only thing that establishes a guard test guards anything.

---

1. configured_key_envs silently skips a quoted key_env — a false PASS on a missing credential

deploy/service/install.sh:70-72. [[:space:]]* requires the name to start immediately after the whitespace, so neither of these is extracted:

key_env: "UPSTREAM_QUOTED_KEY"
key_env: 'UPSTREAM_SINGLE_KEY'

Both are legal here — config/upstreams.go:30 is a plain KeyEnv string, and the loader parses them fine (parsed KeyEnv="UPSTREAM_QUOTED_KEY"). config/upstreams.go:93 then refuses to boot when the named variable is unset. So preflight prints PASSED, the operator proceeds, and the service fails to start on the credential preflight was meant to check — the inversion where a false PASS is worse than the false FAIL it replaced.

Pre-existing (the old grep -oE had the same requirement), so not a regression — but this PR rewrites this function and adds the test that claims to guard it. One character class fixes it, with no quote characters needed inside the single-quoted sed expression:

sed -nE 's/#.*//; s/.*key_env:[^A-Z0-9_]*([A-Z0-9_]+).*/\1/p' "$1" 2>/dev/null | sort -u

Verified: picks up all three shapes, still silent with rc=0 on upstreams.example.yaml and on the live allow-list.

2. TestPreflightResolvesToolsOnTheServicePATH only tests the direction that cannot hurt you

cmd/context-guru-proxy/preflight_test.go:91-127 proves a tool on the service PATH is found and a tool that exists nowhere is missing. It never proves the false-PASS direction: a tool on the caller's PATH but absent from the service's must be reported missing.

Mutating in_service_path from override to append — the most plausible future "be lenient" edit — leaves the test green:

PATH="$PATH:${p:-}" command -v "$1" >/dev/null 2>&1   # -> ok, 0.773s

With a tool planted only on the caller's PATH:

SHIPPED (override):  caller-only tool -> reported missing (correct)
MUTATED (append):    caller-only tool -> reported PRESENT (false PASS)

Live for any directory sudo's secure_path carries that systemd's PATH does not — the mirror image of the case this PR fixes. The shipped code is right; the guard is one-directional. ~4 lines: plant a third fake tool in a caller-only directory, assert non-zero.

3. The live-host evidence cannot demonstrate half of the claim it is offered for

The body says "a configured key_env is still checked", then immediately "Acceptance test is the real host" with the Preflight PASSED transcript. The live allow-list has no key_env field — the only occurrence is inside the documenting comment — so that run exercised only the zero-configured path. That is genuinely the path that produced the SOME_VAR bug, so the reported defect is demonstrated on the host; the "still checked" half rests on the unit test alone, which is where finding 1 bites.

---

Nits

• install.sh:63-66 — the "not only whole-comment lines" claim is unguarded: stripping ^[[:space:]]*#.* only leaves the test green, because SOME_VAR sits on a whole-comment line and the fixture's trailing comment contains no key_env. One line in the fixture (dialect: anthropic # key_env: OLD_ONE) closes it. The trailing-comment case does earn its place in the other direction — mutating to /#/d fails it.
• dash/ui.go:83-85 — "renaming a file or moving bytes between two of them also changes the token" is documented but untested: deleting the fmt.Fprintf(sum, ...) leaves both dash tests green.
• dash/ui.go:152-154 + the mime import — 4 dead lines. http.ServeContent already calls mime.TypeByExtension when Content-Type is unset; removing them gives byte-identical text/html|text/javascript|text/css; charset=utf-8.
• dash/ui.go:96-103 — worth naming the ceiling. The blind match is the right trade (it is what survives a reshaped index.html), but it also rewrites a quoted asset name that is not a URL: a JS string literal, a nonce="…", the inside of a percent-encoded data URI. Today there is no collision — exactly four quoted asset-name occurrences across all five files, all real references (index.html:7,742,745, tools.js:35) — and log("loaded app.js ok") is correctly untouched, because the quotes must bracket the name exactly.
• dash/ui.go:64-66 — a ui/ subdirectory makes fs.ReadFile error, versionFS return "", nil, and the handler fall back to unversioned URLs at max-age=3600, i.e. this exact bug. CI does catch it (no asset version was computed from the embedded UI), so it cannot reach production; the message just does not say why.

---

Verified sound

• Hot path: var assetVersion, versionedUI = versionAssets() — once at init. Per request it is a map lookup plus ServeContent: 65315 ns/op, 132787 B/op, 22 allocs/op for the whole 44,920-byte /dashboard/ path through the mux, dominated by the recorder's body copy. Hash deterministic across processes (fd6f54029fdc every run). Idempotence and markup-shape blindness are both genuinely guarded. app.js alongside app.js.map resolves correctly — no app.js?v=…map.
• Security headers unchanged: the pre-diff handler set only Cache-Control and CSP, and the diff is a pure addition after the CSP line. 158 bytes of CSP on every path including 404s. dash/csp_test.go already covers /dashboard/, /dashboard/style.css, /dashboard/app.js — all three now route through the new branch and pass.
• Degradation: /dashboard → 301; both entry points 200 + no-cache + versioned; ?v=stale → 200 with current bytes (no 404 for old cached HTML); If-None-Match → 304 with headers intact; Range: bytes=0-9 → 206 + Content-Range; unknown asset and INDEX.HTML → 404; empty embed → fallback and test 1 fatals; an asset named in the HTML but absent from the FS is left unversioned and flagged. No Last-Modified, matching the old FileServer behaviour on embed.FS.
• One-token choice is right: tools.css is loaded only from tools.js:35, never from the HTML, so a per-tools.js hash would not move when only tools.css changed. Cost is 512,489 bytes re-fetched per deploy per visitor. Nothing external defeats it — nginx proxies /dashboard/ with proxy_cache off (nginx.conf:201), and the UI registers no service worker or appcache.
• PATH source is right, not an approximation: the units do not override PATH, so systemctl show-environment is what they actually inherit rather than a guess at it, and the tool the check was failing on is reachable both on that PATH and by the unit's own user. It needs no privileges (cmd_preflight does not call need_root), and degrades to $PATH when systemctl is absent — the old behaviour, which is the case that matters on a host before the unit exists.
• schemaVersion still 6 and untouched. gofmt -l . empty, go vet ./... clean, CGO_ENABLED=1 go test -race -count=1 ./... → 25 packages ok, exit 0 (both flaky: two load-related test flakes in the suite that gates every deploy #91 flakes passed in that run). Scope clean: 4 files, the two fixes and their tests. No secret or PII shapes in the added lines.
• This PR claims no dollar, token or latency figure — the right choice for a deploy-hygiene change, and worth saying out loud.

Test guard summary: 1 guards (killed 6 mutations). 2 guards, missing the name+length claim. 3 guards its headline, missing the inline-comment and quoted-value cases. 4 partially guards — it misses the false-PASS direction, which is the one that matters.