mirror of
https://github.com/Kpa-clawbot/meshcore-analyzer.git
synced 2026-10-23 08:52:21 +00:00
Closes #1551. ## Problem `/api/*` Go responses emit no `Cache-Control` header. CDNs (Cloudflare, nginx, Varnish) default to caching `application/json` for **15 min – 4 h** when no directive is set. Observed against a public Cloudflare-fronted CoreScope instance (`meshcore.meshat.se`): - 17 consecutive polls of `/api/observers` over ~10 min returned byte-identical responses - Response headers showed `cf-cache-status: HIT`, `age: 878` (~15 min) - Cache-busting query param → `cf-cache-status: MISS` with fresh `last_seen` values This causes WebSocket pushes to diverge from REST GETs (WS fresh, REST stale) and produces false-positive stale/online flips for observers near the 10-min threshold. ## Fix New `noStoreAPIMiddleware` in `cmd/server/routes.go` wired into the gorilla/mux chain alongside the existing `backfillStatusMiddleware`. Sets `Cache-Control: no-store` on every response whose request path starts with `/api/`. ## Design choice: `no-store` vs `private, max-age=0` Chose `no-store`. CoreScope's REST endpoints are fresh-on-every-request by contract (WS pushes diff against REST GETs), so any intermediary cache is wrong. `no-store` forbids **any** cache (CDN, browser, intermediary). `private, max-age=0` still permits short browser caches and some intermediaries — no benefit here. ## Scope discipline - `/api/` prefix only. - Static assets (`/`, `/app.js`, `/style.css`, …) keep their existing `no-cache, no-store, must-revalidate` headers from `spaHandler` in `main.go`. Hashed assets stay CDN-cacheable by design. - The middleware runs for **all** registered routes including the websocket upgrade HTTP request, since `/ws` is served through the same mux. ## TDD - **Red** `1beb5432`: `cmd/server/cache_control_api_test.go` asserts `Cache-Control: no-store` on `/api/stats`, `/api/observers`, `/api/packets`, `/api/nodes`, and asserts the middleware does NOT leak onto `/` or `/app.js`. Fails on assertion (no Cache-Control header emitted) — not a compile error. - **Green** `13be675f`: middleware + wiring. All assertions pass; full `cmd/server` suite stays green. ## Files - `cmd/server/routes.go` — middleware definition + `r.Use(noStoreAPIMiddleware)` - `cmd/server/cache_control_api_test.go` — 6 sub-tests across 2 top-level tests ## Preflight `bash ~/.openclaw/skills/pr-preflight/scripts/run-all.sh origin/master` → clean (exit 0). --------- Co-authored-by: corescope-bot <bot@corescope>
This commit is contained in:
1 parent
8d2b42574b
commit
0c908d2bca
2 files changed
+131
No files matched your search
@@ -0,0 +1,98 @@
|
|||||||
|
package main
|
||||||
|
|
||||||
|
// Issue #1551: /api/* responses must emit Cache-Control: no-store so
|
||||||
|
// CDNs (Cloudflare, nginx, Varnish) do not cache JSON. Static assets
|
||||||
|
// (app.js, /, etc.) intentionally remain CDN-cacheable.
|
||||||
|
|
||||||
|
import (
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"os"
|
||||||
|
"path/filepath"
|
||||||
|
"testing"
|
||||||
|
|
||||||
|
"github.com/gorilla/mux"
|
||||||
|
)
|
||||||
|
|
||||||
|
// TestAPIRoutesEmitNoStoreCacheControl asserts every covered /api/*
|
||||||
|
// endpoint sets Cache-Control: no-store. This is a black-box test
|
||||||
|
// against the real router, exercising whatever middleware chain is
|
||||||
|
// wired by RegisterRoutes.
|
||||||
|
func TestAPIRoutesEmitNoStoreCacheControl(t *testing.T) {
|
||||||
|
_, router := setupTestServer(t)
|
||||||
|
|
||||||
|
apiPaths := []string{
|
||||||
|
"/api/stats",
|
||||||
|
"/api/observers",
|
||||||
|
"/api/packets?limit=10",
|
||||||
|
"/api/nodes?limit=10",
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, p := range apiPaths {
|
||||||
|
t.Run(p, func(t *testing.T) {
|
||||||
|
req := httptest.NewRequest("GET", p, nil)
|
||||||
|
w := httptest.NewRecorder()
|
||||||
|
router.ServeHTTP(w, req)
|
||||||
|
|
||||||
|
if w.Code != http.StatusOK {
|
||||||
|
t.Fatalf("%s: expected 200, got %d (body: %s)", p, w.Code, w.Body.String())
|
||||||
|
}
|
||||||
|
cc := w.Header().Get("Cache-Control")
|
||||||
|
if cc != "no-store" {
|
||||||
|
t.Errorf("%s: expected Cache-Control: no-store, got %q", p, cc)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// TestStaticAssetsDoNotEmitNoStore guards against scope creep: the
|
||||||
|
// no-store middleware must be scoped to /api/* only. Static assets
|
||||||
|
// (HTML, JS, CSS) keep their existing browser-cache headers
|
||||||
|
// ("no-cache, no-store, must-revalidate" today via spaHandler) and
|
||||||
|
// must NOT be downgraded to bare "no-store" by the API middleware —
|
||||||
|
// i.e. the API middleware must not run on these paths. If a future
|
||||||
|
// change moves static assets behind no-store middleware, CDN caching
|
||||||
|
// of immutable hashed assets breaks; assert the contract explicitly.
|
||||||
|
func TestStaticAssetsDoNotEmitBareNoStore(t *testing.T) {
|
||||||
|
// Build a temp public dir so spaHandler has real files to serve.
|
||||||
|
dir := t.TempDir()
|
||||||
|
if err := os.WriteFile(filepath.Join(dir, "index.html"), []byte("<html>SPA</html>"), 0644); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if err := os.WriteFile(filepath.Join(dir, "app.js"), []byte("console.log('app')"), 0644); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
|
||||||
|
_, router := setupTestServer(t)
|
||||||
|
// Wire the SPA handler exactly the way main.go does for non-/api paths.
|
||||||
|
fs := http.FileServer(http.Dir(dir))
|
||||||
|
router.PathPrefix("/").Handler(spaHandler(dir, fs))
|
||||||
|
|
||||||
|
cases := []struct {
|
||||||
|
path string
|
||||||
|
wantCacheCC string
|
||||||
|
}{
|
||||||
|
// spaHandler sets this exact value for HTML/JS/CSS.
|
||||||
|
{"/app.js", "no-cache, no-store, must-revalidate"},
|
||||||
|
{"/", "no-cache, no-store, must-revalidate"},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, c := range cases {
|
||||||
|
t.Run(c.path, func(t *testing.T) {
|
||||||
|
req := httptest.NewRequest("GET", c.path, nil)
|
||||||
|
w := httptest.NewRecorder()
|
||||||
|
router.ServeHTTP(w, req)
|
||||||
|
cc := w.Header().Get("Cache-Control")
|
||||||
|
if cc == "no-store" {
|
||||||
|
t.Errorf("%s: API no-store middleware leaked onto static asset (got bare %q, expected %q)", c.path, cc, c.wantCacheCC)
|
||||||
|
}
|
||||||
|
if cc != c.wantCacheCC {
|
||||||
|
t.Errorf("%s: expected Cache-Control %q, got %q", c.path, c.wantCacheCC, cc)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Ensure mux import used (test compiles even if setupTestServer signature
|
||||||
|
// changes).
|
||||||
|
var _ = mux.NewRouter
|
||||||
@@ -164,6 +164,14 @@ func (s *Server) RegisterRoutes(r *mux.Router) {
|
|||||||
// Backfill status header middleware
|
// Backfill status header middleware
|
||||||
r.Use(s.backfillStatusMiddleware)
|
r.Use(s.backfillStatusMiddleware)
|
||||||
|
|
||||||
|
// /api/* responses must not be cached by upstream CDNs (#1551).
|
||||||
|
// Cloudflare/nginx/Varnish default zone policies cache
|
||||||
|
// application/json for 15min–4h when no Cache-Control is set,
|
||||||
|
// causing operators behind a CDN to serve stale observers/packets/
|
||||||
|
// stats data. Scope: /api/ prefix only — static assets stay
|
||||||
|
// CDN-cacheable (their headers are set by spaHandler).
|
||||||
|
r.Use(noStoreAPIMiddleware)
|
||||||
|
|
||||||
// Config endpoints
|
// Config endpoints
|
||||||
r.HandleFunc("/api/config/cache", s.handleConfigCache).Methods("GET")
|
r.HandleFunc("/api/config/cache", s.handleConfigCache).Methods("GET")
|
||||||
r.HandleFunc("/api/config/client", s.handleConfigClient).Methods("GET")
|
r.HandleFunc("/api/config/client", s.handleConfigClient).Methods("GET")
|
||||||
@@ -257,6 +265,31 @@ func (s *Server) RegisterRoutes(r *mux.Router) {
|
|||||||
r.HandleFunc("/api/docs", s.handleSwaggerUI).Methods("GET")
|
r.HandleFunc("/api/docs", s.handleSwaggerUI).Methods("GET")
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// noStoreAPIMiddleware sets Cache-Control: no-store on every response
|
||||||
|
// whose request path starts with /api/. See #1551 — CDNs cache JSON
|
||||||
|
// for minutes when no Cache-Control is present, which causes observers/
|
||||||
|
// packets/stats responses to go stale through Cloudflare/nginx/Varnish.
|
||||||
|
//
|
||||||
|
// Why no-store (not private,max-age=0):
|
||||||
|
// - no-store is the most conservative directive; forbids ANY cache
|
||||||
|
// (CDN, browser, intermediary) from storing the response.
|
||||||
|
// - private,max-age=0 still permits short browser caches and some
|
||||||
|
// intermediaries; we don't gain anything from it because the data
|
||||||
|
// is fresh-on-every-request semantics by contract (WS pushes diff
|
||||||
|
// against REST GETs).
|
||||||
|
//
|
||||||
|
// Scope: /api/ prefix only. Static assets (HTML/JS/CSS) keep their
|
||||||
|
// existing headers from spaHandler and remain CDN-cacheable on
|
||||||
|
// hashed URLs.
|
||||||
|
func noStoreAPIMiddleware(next http.Handler) http.Handler {
|
||||||
|
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
if strings.HasPrefix(r.URL.Path, "/api/") {
|
||||||
|
w.Header().Set("Cache-Control", "no-store")
|
||||||
|
}
|
||||||
|
next.ServeHTTP(w, r)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
func (s *Server) backfillStatusMiddleware(next http.Handler) http.Handler {
|
func (s *Server) backfillStatusMiddleware(next http.Handler) http.Handler {
|
||||||
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
if s.store != nil && s.store.backfillComplete.Load() {
|
if s.store != nil && s.store.backfillComplete.Load() {
|
||||||
|
|||||||
Reference in new issue
Block a user