From 9cc7ffcf3b32ac9f412faca91185a886830f1487 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Thu, 15 Oct 2026 20:08:01 +0200 Subject: [PATCH] A version number is pushed once, and every build addresses its CSS differently MEASURED FIRST (audits/app-page-2026-10-15b/P0/VERDICT.md). The tab bar rendered unstyled through the public address for one reason, and it was not the box: exactly one guest runs 0.315.0 and it runs the registry's current digest (8887343011d6, identical to the registry's). The floor is still 0.314.0, so nothing else could have taken it. What it was, in four measured facts: 1. the stylesheet URL is /static/style.css?v={{.Version}} -> ?v=0.315.0 for all THREE different 0.315.0 builds (e66d466, fe0228c, db52f5a), the first of which had no .app-tabs rules at all; 2. the origin sends NO validator at all (ETag present = 0, Last-Modified present = 0), so a cached copy cannot be revalidated - it can only expire; 3. Cloudflare caches that URL (cf-cache-status HIT, age climbing) and rewrites the browser TTL from the origin's 3600 to 14400 - four hours; 4. the session's first request returned cf-cache-status EXPIRED, which only happens when the edge holds an entry that has aged out. Origin, edge and box now agree byte-for-byte. The stale object itself is gone - that first probe forced its revalidation - and the REPORT says so rather than claiming a HIT nobody can reproduce. THE FIX IS IN TWO PARTS, NEITHER OF WHICH IS PURGING A CACHE: build.sh refuses to push a version that already exists, before building, for --push and --multiarch. A registry that cannot be asked is a refusal too: proving the tag is new is the point, and "I could not ask" is not that proof. Local builds are untouched - reusing a version while iterating is normal. scripts/test_build_push_once.sh drives the real build.sh with a fake docker on PATH over six cases; red-proved twice (check deleted -> a present tag builds and pushes; an unreachable registry treated as absent -> two cases fail). assetTag() now hashes version + GitCommit + BuildTime, and EVERY shell's stylesheet link uses it (seven of the nine asked for ?v={{.Version}}; only the two stranger-facing share pages already had it). It stays opaque and 8 hex, because the share pages are reached by a capability URL and naming the build to whoever holds one is a gift to somebody hunting a known fault. Cache-Control is unchanged: the new address is the fix, not a shorter cache. The parity harness normalises the asset tag on BOTH sides of the comparison, as it already does for the per-session CSRF token, so all 148 fixtures stay byte-identical on disk and stop depending on which build rendered them. The same normalisation is applied at the second comparison site in i18n_wiring_test.go, which had the same assumption. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_0117kRTSgs3rJWUFGxyoWN6Y --- controller/build.sh | 41 +++++++ controller/cmd/controller/main.go | 4 + controller/internal/web/asset_tag_test.go | 104 ++++++++++++++++ controller/internal/web/i18n_parity_test.go | 24 +++- controller/internal/web/i18n_wiring_test.go | 5 +- controller/internal/web/server.go | 43 +++++-- controller/internal/web/templates/claim.html | 2 +- .../internal/web/templates/familygate.html | 2 +- controller/internal/web/templates/layout.html | 2 +- controller/internal/web/templates/login.html | 2 +- .../internal/web/templates/recovery.html | 2 +- .../internal/web/templates/setupgate.html | 2 +- .../internal/web/templates/signupclosed.html | 2 +- controller/scripts/test_build_push_once.sh | 111 ++++++++++++++++++ 14 files changed, 325 insertions(+), 21 deletions(-) create mode 100644 controller/internal/web/asset_tag_test.go create mode 100644 controller/scripts/test_build_push_once.sh diff --git a/controller/build.sh b/controller/build.sh index a6f9f10..7ce67cb 100755 --- a/controller/build.sh +++ b/controller/build.sh @@ -66,6 +66,47 @@ info "Build dir: ${BUILD_DIR}" info "Image: ${IMAGE}:${VERSION}" echo "" +# ========================================================================= +# A VERSION NUMBER IS PUSHED ONCE (v0.316.0) +# ========================================================================= +# 0.315.0 was built and pushed THREE times from three different commits. A box that pulled between +# the first and the last runs different code under the same number — and will never fetch the fix, +# because to the box, and to the floor, that number is already current (`09-update-architecture.md`: +# a box compares VERSIONS, and re-pulls a tag it already has only when the tag changes). Nothing in +# the build stopped it. This does. +# +# The check runs BEFORE the build, so a refusal costs no minutes. A registry that cannot be reached +# is a REFUSAL too: the point is to prove the tag is new, and "I could not ask" is not that proof. +# Local builds (no --push) are unaffected — reusing a version while iterating is the normal case. +# +# `docker manifest inspect` is the probe, so a test can put a fake `docker` on PATH +# (scripts/test_build_push_once.sh). +if [[ "${ACTION}" == "--push" || "${ACTION}" == "--multiarch" ]]; then + step "0/4 — Checking the registry: is ${VERSION} already published?" + # `set -e` is on, and a failing command substitution would abort the script before the rc is + # ever read — which would turn "the registry said no such tag" into a silent exit. The `|| true` + # keeps the failure for us to classify; that classification is the whole check. + probe_rc=0 + probe_out=$(docker manifest inspect "${IMAGE}:${VERSION}" 2>&1) || probe_rc=$? + if [[ ${probe_rc} -eq 0 ]]; then + error "${IMAGE}:${VERSION} already exists in the registry — bump the version." + error "A version number is pushed once. Two images under one number mean a box can run" + error "either of them and never learn the difference. If this is a fix for something already" + error "pushed, release it as the next patch version instead." + exit 1 + fi + # Absent is the ONLY other acceptable answer. Anything that is not a clean "not found" (the + # registry is down, the login expired, DNS failed) leaves the question unanswered, and an + # unanswered question is a refusal. + if ! grep -qiE 'no such manifest|manifest unknown|not found|does not exist|NAME_UNKNOWN|MANIFEST_UNKNOWN' <<<"${probe_out}"; then + error "Could not establish whether ${IMAGE}:${VERSION} exists — refusing to build." + error "The registry answered: ${probe_out}" + error "Proving the tag is new is the point of this check; 'I could not ask' is not that proof." + exit 1 + fi + info "${VERSION} is not present in the registry — proceeding." +fi + # ========================================================================= # Step 1: Prepare clean build workspace # ========================================================================= diff --git a/controller/cmd/controller/main.go b/controller/cmd/controller/main.go index 2547953..dd349b9 100644 --- a/controller/cmd/controller/main.go +++ b/controller/cmd/controller/main.go @@ -1963,6 +1963,10 @@ func main() { // --- Initialize web server --- webServer := web.NewServer(cfg, stackMgr, cpuCollector, backupMgr, sched, sett, alertMgr, notifier, updater, logger, Version) + // v0.316.0: which BUILD this is, not just which version it calls itself. Every shell's stylesheet + // URL is derived from it, so two builds of one version can never share a cache entry. Init-only, + // before the HTTP server starts. + webServer.SetBuildIdentity(GitCommit, BuildTime) // R-871 (decision 110): the missed-backup banner reads the night ledger and the box's own on/off record. webServer.SetMissedBackupBanner(webNightLedger, metricsStore) // Migration done-hook: a decommission-initiated migration finalizes the source decommission on diff --git a/controller/internal/web/asset_tag_test.go b/controller/internal/web/asset_tag_test.go new file mode 100644 index 0000000..197c929 --- /dev/null +++ b/controller/internal/web/asset_tag_test.go @@ -0,0 +1,104 @@ +package web + +import ( + "strings" + "testing" +) + +// THE STYLESHEET'S ADDRESS CHANGES WITH EVERY BUILD (v0.316.0). +// +// v0.315.0 was built and pushed three times from three different commits, and every one of them +// asked the browser for `/static/style.css?v=0.315.0`. A cache holding the first build's stylesheet +// therefore kept serving it under the address the third build still asked for — measured through +// Cloudflare (`audits/app-page-2026-10-15b/P0/VERDICT.md`: cf-cache-status HIT, a browser TTL of +// 14400s that Cloudflare sets itself, and NO ETag and NO Last-Modified from the origin, so a stale +// copy cannot be revalidated — only expire). +// +// build.sh now refuses a second push of a version, so this must never happen again. The tag is the +// belt to that brace: even two builds of one version address their CSS differently. +func TestAssetTag_ChangesPerBuild(t *testing.T) { + same := &Server{version: "0.316.0"} + same.SetBuildIdentity("aaaaaaa", "2026-10-15T20:00:00Z") + + // SAME version, DIFFERENT commit — the exact shape of the 0.315.0 incident. + otherCommit := &Server{version: "0.316.0"} + otherCommit.SetBuildIdentity("bbbbbbb", "2026-10-15T20:00:00Z") + if same.assetTag() == otherCommit.assetTag() { + t.Error("two builds of the SAME version from different commits share an asset tag — " + + "a cache cannot tell them apart, which is the defect this exists to prevent") + } + + // SAME version and commit, rebuilt — a rebuild is a different binary too. + rebuilt := &Server{version: "0.316.0"} + rebuilt.SetBuildIdentity("aaaaaaa", "2026-10-15T21:30:00Z") + if same.assetTag() == rebuilt.assetTag() { + t.Error("a rebuild of the same commit shares an asset tag with the original") + } + + // A different version obviously differs too. + newer := &Server{version: "0.317.0"} + newer.SetBuildIdentity("aaaaaaa", "2026-10-15T20:00:00Z") + if same.assetTag() == newer.assetTag() { + t.Error("two versions share an asset tag") + } + + // DETERMINISTIC for one binary: the same build must produce the same tag on every request, or + // the browser would re-download the stylesheet on every page load. + again := &Server{version: "0.316.0"} + again.SetBuildIdentity("aaaaaaa", "2026-10-15T20:00:00Z") + if same.assetTag() != again.assetTag() || same.assetTag() != same.assetTag() { + t.Error("the tag is not stable within one build — every page load would refetch the CSS") + } + + // OPAQUE: it discloses neither the version nor the commit. The stranger-facing share pages use + // this same value (TestShareGuest_HeadersTilesNoAdminChrome), and telling anyone holding a + // capability URL which build is running is a gift to somebody looking for a known fault. + tag := same.assetTag() + if len(tag) != 8 { + t.Errorf("tag %q is %d chars, want 8 hex", tag, len(tag)) + } + for _, leak := range []string{"0.316.0", "0316", "aaaaaaa", "2026"} { + if strings.Contains(tag, leak) { + t.Errorf("the tag %q discloses %q", tag, leak) + } + } +} + +// TestAssetTag_EveryShellUsesIt — the fix is only a fix if the shells ask for it. Before v0.316.0 +// two stranger-facing pages used the tag and the other seven shells used `?v={{.Version}}`, which +// is exactly the address that did not change between the three 0.315.0 builds. +// +// RED-PROOF (REPORT.md): put `?v={{.Version}}` back in layout.html -> this fails naming it. +func TestAssetTag_EveryShellUsesIt(t *testing.T) { + ents, err := templateFS.ReadDir("templates") + if err != nil { + t.Fatalf("templates: %v", err) + } + checked := 0 + for _, e := range ents { + if e.IsDir() || !strings.HasSuffix(e.Name(), ".html") { + continue + } + name := e.Name() + b, err := templateFS.ReadFile("templates/" + name) + if err != nil { + t.Fatalf("%s: %v", name, err) + } + src := string(b) + if !strings.Contains(src, "/static/style.css") { + continue + } + checked++ + if strings.Contains(src, "/static/style.css?v={{.Version}}") { + t.Errorf("%s asks for the stylesheet by VERSION — two builds of one version then share "+ + "a cache entry, which is the 0.315.0 defect", name) + } + if !strings.Contains(src, "/static/style.css?v={{.AssetTag}}") { + t.Errorf("%s links the stylesheet without the per-build asset tag", name) + } + } + // A loop that matched nothing would pass in silence — the shape this project calls a hollow test. + if checked < 5 { + t.Fatalf("only %d shell(s) link the stylesheet — the scan found nothing to check", checked) + } +} diff --git a/controller/internal/web/i18n_parity_test.go b/controller/internal/web/i18n_parity_test.go index 10c3cfe..125ddd3 100644 --- a/controller/internal/web/i18n_parity_test.go +++ b/controller/internal/web/i18n_parity_test.go @@ -641,16 +641,26 @@ func renderI18nCase(t *testing.T, s *Server, lang string, c i18nCase) string { return normaliseI18nRender(buf.String()) } +// assetTagRe matches the stylesheet's cache-buster. Since v0.316.0 that is a PER-BUILD tag, so it +// differs between two builds of the same source — which is the whole point of it (a cache must not +// be able to serve a previous build's CSS under an address the new build still asks for). A parity +// fixture pins COPY, not which build rendered it, so the tag is blanked on both sides of every +// comparison, exactly as a per-session CSRF token is. Without this, every fixture in the directory +// would churn on every build and the whole set would stop meaning anything. +var assetTagRe = regexp.MustCompile(`(/static/style\.css\?v=)[^"']*`) + // csrfValueRe matches a CSRF field's value. A session CSRF token is random per session, so it can // never be a fixture value; it is blanked on BOTH sides of every parity comparison, exactly as // relative ages are. What the fixture still pins is that the field is THERE and which form it is in — // which is the part that says whether the page posts to the household switch or to /lang. var csrfValueRe = regexp.MustCompile(`(name="_csrf" value=")[^"]*"`) -// normaliseI18nRender removes the two things that legitimately differ between two renders of the same -// page: a relative age off the wall clock, and a per-session CSRF token. +// normaliseI18nRender removes the three things that legitimately differ between two renders of the +// same page: a relative age off the wall clock, a per-session CSRF token, and the per-build asset +// tag. None of them is copy; all three would otherwise make the fixtures unmaintainable. func normaliseI18nRender(s string) string { s = relativeAgeRe.ReplaceAllString(s, "# $1") + s = assetTagRe.ReplaceAllString(s, "${1}ASSET") return csrfValueRe.ReplaceAllString(s, "${1}CSRF\"") } @@ -699,11 +709,17 @@ func TestI18nParity(t *testing.T) { t.Logf("%s: fixture WRITTEN", c.name) continue } - want, err := os.ReadFile(p) + raw, err := os.ReadFile(p) if err != nil { t.Fatalf("%s: no parity fixture (%v) — fixtures are captured from UNCONVERTED templates, never regenerated to make a conversion pass", c.name, err) } - if got != string(want) { + // BOTH sides are normalised, and the function is idempotent (a normalised render passes + // through it unchanged). It has to be both: v0.316.0 moved every shell's stylesheet from + // `?v=` to a per-build tag, and normalising only the fresh render would have + // convicted all 148 fixtures for a difference that is not copy. Normalising both leaves the + // stored bytes untouched and keeps the fixtures meaningful across builds. + want := normaliseI18nRender(string(raw)) + if got != want { gl, wl := strings.Split(got, "\n"), strings.Split(string(want), "\n") for i := 0; i < len(gl) || i < len(wl); i++ { var g, w string diff --git a/controller/internal/web/i18n_wiring_test.go b/controller/internal/web/i18n_wiring_test.go index 7d3de8f..3817818 100644 --- a/controller/internal/web/i18n_wiring_test.go +++ b/controller/internal/web/i18n_wiring_test.go @@ -358,7 +358,10 @@ func TestI18nDirectRenderPagesFollowLanguage(t *testing.T) { if err != nil { t.Fatal(err) } - if got != string(want) { + // Both sides through the normaliser, as TestI18nParity does and for the same reason: + // the per-build asset tag, the per-session CSRF token and a relative age are not + // copy, and the function is idempotent. + if got != normaliseI18nRender(string(want)) { t.Errorf("%s: Hungarian render through executeTemplateLang differs from fixture %s", p.tmpl, p.caseName) } continue diff --git a/controller/internal/web/server.go b/controller/internal/web/server.go index d2b70d1..32ab9cd 100644 --- a/controller/internal/web/server.go +++ b/controller/internal/web/server.go @@ -54,7 +54,9 @@ type Server struct { updater *selfupdate.Updater logger *log.Logger version string - encKey []byte // AES-256 key for decrypting app.yaml values + // buildID distinguishes two builds of the SAME version — see SetBuildIdentity and assetTag. + buildID string + encKey []byte // AES-256 key for decrypting app.yaml values // gate / gateClock (v0.280.0, decision 46): the setup gate's key + used tokens; the clock is a test seam. gate gateState fam familyState // v0.287.0: the family gate (family_gate.go) @@ -473,10 +475,10 @@ func (s *Server) hasSession(r *http.Request) bool { // predecessor got wrong twice. Idempotent: an existing Version is left alone, so a fixture case that // pins its own version keeps it. // -// `AssetTag` is the buster for a page a STRANGER may open — the two guest share pages, reached by a -// capability URL. `TestShareGuest_HeadersTilesNoAdminChrome` refuses the version string on those for a -// reason worth keeping: telling anyone holding the link exactly which build is running is a gift to -// somebody looking for a known fault. An opaque per-build tag busts the cache just as well. +// `AssetTag` is the buster EVERY shell's stylesheet link uses (v0.316.0). It was only on the two +// stranger-facing guest share pages; the rest asked for `?v={{.Version}}`, which does not change +// between two builds of one version — the defect Part 0 measured. `Version` stays in the data for +// the pages that display it. func (s *Server) addShellAssetData(data map[string]interface{}) { if data == nil { return @@ -487,11 +489,34 @@ func (s *Server) addShellAssetData(data map[string]interface{}) { data["AssetTag"] = s.assetTag() } -// assetTag is an opaque per-build cache-buster: it changes when the version changes and discloses -// nothing about it. Used by the pages a stranger can open; everywhere else the version itself is -// already on the page and the tag would only be indirection. +// SetBuildIdentity records which BUILD this binary is, as opposed to which version it calls itself. +// Init-only, called once from main before the HTTP server starts (the `SetStackProvider` contract). +// +// It exists because a version number is not a build. v0.315.0 was built and pushed three times from +// three different commits; every one of them asked the browser for `/static/style.css?v=0.315.0`, +// so a cache that already held the first build's stylesheet kept serving it — measured through +// Cloudflare (`audits/app-page-2026-10-15b/P0/VERDICT.md`). build.sh now refuses a second push of a +// version, and this makes the asset URL change even when the version does not. +func (s *Server) SetBuildIdentity(gitCommit, buildTime string) { + s.buildID = gitCommit + "/" + buildTime +} + +// assetTag is an opaque per-BUILD cache-buster: it changes whenever the binary changes, and +// discloses neither the version nor the commit. +// +// Two jobs in one value, and both need it opaque: +// - every shell's stylesheet link uses it, so no cache at any layer (browser, Cloudflare, or +// anything between) can serve a previous build's CSS under an address the new build still asks +// for. The origin sends no ETag and no Last-Modified, so a stale copy cannot be revalidated — +// it can only expire, or be addressed differently. This is the "addressed differently". +// - the pages a STRANGER can open use it too. `TestShareGuest_HeadersTilesNoAdminChrome` refuses +// the version string there for a reason worth keeping: telling anyone holding a capability URL +// exactly which build is running is a gift to somebody looking for a known fault. +// +// Deterministic for a given binary, so the i18n fixtures stay stable within a build; the parity +// harness normalises it anyway, because a fixture pins COPY, not which build rendered it. func (s *Server) assetTag() string { - sum := sha256.Sum256([]byte("felhom-asset-tag-v1:" + s.version)) + sum := sha256.Sum256([]byte("felhom-asset-tag-v1:" + s.version + ":" + s.buildID)) return hex.EncodeToString(sum[:4]) } diff --git a/controller/internal/web/templates/claim.html b/controller/internal/web/templates/claim.html index 6e6ae9c..64fc4e5 100644 --- a/controller/internal/web/templates/claim.html +++ b/controller/internal/web/templates/claim.html @@ -5,7 +5,7 @@ {{if .IsReset}}{{T "claim.jelszo_visszaallitasa"}}{{else}}{{T "claim.a_szerver_beallitasa"}}{{end}} — Felhom - +