diff --git a/cmd/pimonitor/main.go b/cmd/pimonitor/main.go index 799f6bf..934d02f 100644 --- a/cmd/pimonitor/main.go +++ b/cmd/pimonitor/main.go @@ -68,7 +68,7 @@ func run(args []string) error { } coll := collector.New(collCfg, log) - staticHandler, err := web.Handler() + staticHandler, err := web.Handler(version) if err != nil { return fmt.Errorf("load embedded web assets: %w", err) } diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 957299c..a8c86c7 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -249,13 +249,21 @@ server-side; it also echoes back the build-time `version` injected via ## Web dashboard (`internal/web`) -`web.Handler()` embeds `internal/web/assets/*` (`index.html`, `app.js`, `chart.js`, +`web.Handler(version)` embeds `internal/web/assets/*` (`index.html`, `app.js`, `chart.js`, `gauge.js`, `theme-init.js`, `style.css`) into the binary via `//go:embed` and serves them with `http.FileServerFS`. There is **no frontend build step** — no bundler, no npm toolchain, no framework — the assets are plain HTML/CSS/JS shipped as-is, consistent with the project's "prefer the standard library, minimize dependency surface" philosophy (see [`CONTRIBUTING.md`](CONTRIBUTING.md)). +Because embedded files carry a zero `ModTime`, `http.FileServerFS` alone would emit no +`Last-Modified`/`ETag` and no `Cache-Control`, forcing a full refetch of every asset on +every page load. `Handler` wraps the file server to set `Cache-Control: no-cache` and an +`Etag` derived from the build-time `version` (the same string injected via +`-ldflags -X main.version=...` and echoed by `GET /api/v1/config`), so browsers +revalidate with a cheap conditional request (a 304 when the version is unchanged) and +still refetch immediately once an upgrade changes `version`. + **Stored-XSS prevention is enforced by a repository rule, not just convention.** `internal/web/xss_test.go` (`TestAppJS_NoInnerHTMLInterpolation`) scans `app.js` at test time and fails the build if any line assigns non-empty content to `.innerHTML` — the only diff --git a/internal/web/embed.go b/internal/web/embed.go index f4b2201..052872e 100644 --- a/internal/web/embed.go +++ b/internal/web/embed.go @@ -4,6 +4,7 @@ package web import ( "embed" + "fmt" "io/fs" "net/http" ) @@ -12,10 +13,24 @@ import ( var assetsFS embed.FS // Handler returns an http.Handler serving the embedded dashboard at "/". -func Handler() (http.Handler, error) { +// +// Embedded files carry a zero ModTime, so http.FileServerFS would otherwise +// emit no Last-Modified/ETag and no Cache-Control, forcing a full refetch of +// every asset on every page load. Handler sets a version-derived ETag and a +// Cache-Control: no-cache directive instead, so browsers revalidate cheaply +// (a 304 on an unchanged version) and still pick up new assets immediately +// after an upgrade changes version. +func Handler(version string) (http.Handler, error) { sub, err := fs.Sub(assetsFS, "assets") if err != nil { return nil, err } - return http.FileServerFS(sub), nil + fileServer := http.FileServerFS(sub) + etag := fmt.Sprintf(`"%s"`, version) + + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Cache-Control", "no-cache") + w.Header().Set("Etag", etag) + fileServer.ServeHTTP(w, r) + }), nil } diff --git a/internal/web/embed_test.go b/internal/web/embed_test.go index 69bf72d..f325a5f 100644 --- a/internal/web/embed_test.go +++ b/internal/web/embed_test.go @@ -8,7 +8,7 @@ import ( ) func TestHandler_ServesIndex(t *testing.T) { - h, err := Handler() + h, err := Handler("1.2.3") if err != nil { t.Fatalf("Handler: %v", err) } @@ -26,7 +26,7 @@ func TestHandler_ServesIndex(t *testing.T) { } func TestHandler_ServesStaticAssets(t *testing.T) { - h, err := Handler() + h, err := Handler("1.2.3") if err != nil { t.Fatalf("Handler: %v", err) } @@ -42,7 +42,7 @@ func TestHandler_ServesStaticAssets(t *testing.T) { } func TestHandler_ServesThemeToggle(t *testing.T) { - h, err := Handler() + h, err := Handler("1.2.3") if err != nil { t.Fatalf("Handler: %v", err) } @@ -72,7 +72,7 @@ func TestHandler_ServesThemeToggle(t *testing.T) { } func TestHandler_UnknownPath404s(t *testing.T) { - h, err := Handler() + h, err := Handler("1.2.3") if err != nil { t.Fatalf("Handler: %v", err) } @@ -83,3 +83,53 @@ func TestHandler_UnknownPath404s(t *testing.T) { t.Fatalf("status = %d, want 404", rec.Code) } } + +func TestHandler_SetsCacheHeaders(t *testing.T) { + h, err := Handler("1.2.3") + if err != nil { + t.Fatalf("Handler: %v", err) + } + + req := httptest.NewRequest(http.MethodGet, "/", nil) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + if got, want := rec.Header().Get("Cache-Control"), "no-cache"; got != want { + t.Errorf("Cache-Control = %q, want %q", got, want) + } + if got, want := rec.Header().Get("Etag"), `"1.2.3"`; got != want { + t.Errorf("Etag = %q, want %q", got, want) + } +} + +func TestHandler_RevalidatesOnMatchingETag(t *testing.T) { + h, err := Handler("1.2.3") + if err != nil { + t.Fatalf("Handler: %v", err) + } + + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.Header.Set("If-None-Match", `"1.2.3"`) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + if rec.Code != http.StatusNotModified { + t.Fatalf("status = %d, want %d for a matching If-None-Match", rec.Code, http.StatusNotModified) + } +} + +func TestHandler_RefetchesOnVersionChange(t *testing.T) { + h, err := Handler("2.0.0") + if err != nil { + t.Fatalf("Handler: %v", err) + } + + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.Header.Set("If-None-Match", `"1.2.3"`) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want %d when the client's cached ETag is stale", rec.Code, http.StatusOK) + } +}