From da1dc90ac59ec5a16c111060c4b1e7e7503e042f Mon Sep 17 00:00:00 2001 From: mrhid6 Date: Tue, 25 Aug 2026 09:04:47 +0000 Subject: [PATCH] fix: resolve the public status page's tenant from a trusted X-Forwarded-Host MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SSR fetch set `Host` to the visitor's hostname. `Host` is a forbidden header name and undici discards it silently, so the Go server saw `server:8080`, `hostSlug` returned "", `InstanceFromHost` returned false and every public status page 404'd on every deployment. The feature did not work. - `web/` now forwards the visitor's host as `X-Forwarded-Host`, and their address on `X-Forwarded-For` — without the latter gin sees a request from the Next pod with no XFF and every visitor of every page shares one 120/min bucket, tripped by exactly the traffic an outage produces. - `publicStatusInstance` honours `X-Forwarded-Host` only when `c.RemoteIP()` is in `TRUSTED_PROXIES`. It is a tenant selector, so an untrusted peer must not be able to name one; `RemoteIP()` rather than `ClientIP()` because the latter is reconstructed from the very headers being judged. `TrustedProxies()` moves from main.go into the api package so the variable keeps one parser. - A host naming no slug on a non-cloud deployment resolves the sole instance, the way bootstrap does. A self-hosted install at vantage.acme.com or an IP has no slug and could never serve a status page; more than one instance is a 404 rather than a guess, and an unknown-but-well-formed slug stays a 404. - `InstanceFromHost` gains an explicit-host variant rather than a second copy of the slug rules, and now caches negative lookups: an unknown host cost a Mongo query per anonymous request, which is also a timing oracle separating "no such instance" from "instance exists, page does not". - The handler's `@Router` annotation is dropped. openapi.json declares one server of `/api`, so it published `/api/public/status/{pageId}` — a path that does not exist. The real address is described in prose instead. --- server/cmd/main.go | 23 +------ server/internal/api/docs/openapi.json | 51 --------------- server/internal/api/publicstatus.go | 48 +++++++++++++- server/internal/api/trustedproxies.go | 84 +++++++++++++++++++++++++ server/internal/auth/instancehost.go | 77 ++++++++++++++++++++--- web/app/status/[pageId]/page.tsx | 90 +++++++++++++++++++++++---- 6 files changed, 276 insertions(+), 97 deletions(-) create mode 100644 server/internal/api/trustedproxies.go diff --git a/server/cmd/main.go b/server/cmd/main.go index 6103ff7..21b64b2 100644 --- a/server/cmd/main.go +++ b/server/cmd/main.go @@ -175,7 +175,7 @@ func runSchemaSetup() { } if err := services.EnsureStatusPageIndexes(); err != nil { - log.Printf("status page indexes: %v", err) + log.Printf("warning: failed to ensure status page indexes: %v", err) } if err := services.EnsureAuditIndexes(); err != nil { @@ -268,7 +268,7 @@ func serve() { // only produced audit strings; the public status limiter makes it load // bearing. Empty means trust nobody, which is correct for a direct // exposure and wrong behind a proxy — hence the explicit setting. - if err := r.SetTrustedProxies(trustedProxies()); err != nil { + if err := r.SetTrustedProxies(api.TrustedProxies()); err != nil { log.Fatalf("trusted proxies: %v", err) } r.Use(gin.Recovery()) @@ -326,25 +326,6 @@ func corsMiddleware() gin.HandlerFunc { } } -// trustedProxies reads TRUSTED_PROXIES, a comma-separated list of CIDRs or -// addresses. Unset means trust none: ClientIP() is then the peer address, -// which is right for a direct exposure and means every request behind an -// un-configured proxy shares one address for rate limiting. That is a visible -// failure (one client limited) rather than an invisible one (no limit at all). -func trustedProxies() []string { - v := strings.TrimSpace(os.Getenv("TRUSTED_PROXIES")) - if v == "" { - return nil - } - out := []string{} - for _, p := range strings.Split(v, ",") { - if p = strings.TrimSpace(p); p != "" { - out = append(out, p) - } - } - return out -} - func getEnv(key, fallback string) string { if v := os.Getenv(key); v != "" { return v diff --git a/server/internal/api/docs/openapi.json b/server/internal/api/docs/openapi.json index 796174e..a33f251 100644 --- a/server/internal/api/docs/openapi.json +++ b/server/internal/api/docs/openapi.json @@ -4948,57 +4948,6 @@ ] } }, - "/public/status/{pageId}": { - "get": { - "parameters": [ - { - "description": "Status page id", - "in": "path", - "name": "pageId", - "required": true, - "schema": { - "type": "string" - } - } - ], - "responses": { - "200": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/services.StatusSnapshot" - } - } - }, - "description": "OK" - }, - "404": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/api.ErrorResponse" - } - } - }, - "description": "Not Found" - }, - "429": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/api.ErrorResponse" - } - } - }, - "description": "Too Many Requests" - } - }, - "summary": "Public status page", - "tags": [ - "status" - ] - } - }, "/runs/{runId}": { "get": { "parameters": [ diff --git a/server/internal/api/publicstatus.go b/server/internal/api/publicstatus.go index 75ecc1b..c1f39ce 100644 --- a/server/internal/api/publicstatus.go +++ b/server/internal/api/publicstatus.go @@ -7,7 +7,9 @@ import ( "time" "gitea.hostxtra.co.uk/mrhid6/vantage/server/internal/auth" + "gitea.hostxtra.co.uk/mrhid6/vantage/server/internal/models" "gitea.hostxtra.co.uk/mrhid6/vantage/server/internal/services" + "gitea.hostxtra.co.uk/mrhid6/vantage/shared/license" "github.com/gin-gonic/gin" ) @@ -60,6 +62,15 @@ func RateLimitPublicStatus() gin.HandlerFunc { // // Unknown host, unknown page and unpublished page all answer the same 404. // +// It carries no @Router annotation deliberately. openapi.json declares a +// single server of "/api", so a @Router of /public/status/{pageId} would be +// published as /api/public/status/{pageId} — a path that does not exist, and +// which would sit behind auth.Middleware if it did. The real address is: +// +// GET {scheme}://{instance-host}/public/status/{pageId} +// +// on the gin root, unauthenticated, rate limited per client address. +// // @Summary Public status page // @Tags status // @Produce json @@ -67,9 +78,8 @@ func RateLimitPublicStatus() gin.HandlerFunc { // @Success 200 {object} services.StatusSnapshot // @Failure 404 {object} ErrorResponse // @Failure 429 {object} ErrorResponse -// @Router /public/status/{pageId} [get] func getPublicStatusPage(c *gin.Context) { - inst, ok := auth.InstanceFromHost(c) + inst, ok := publicStatusInstance(c) if !ok { c.JSON(http.StatusNotFound, gin.H{"error": "not found"}) return @@ -88,3 +98,37 @@ func getPublicStatusPage(c *gin.Context) { c.Header("Cache-Control", "public, max-age=30") c.JSON(http.StatusOK, snap) } + +// publicStatusInstance resolves which instance a public request is for. +// +// The browser never reaches this handler directly: the request arrives from +// the Next server, which forwards the visitor's host in X-Forwarded-Host +// because the Host header cannot be set on a fetch (undici drops it silently, +// as a forbidden header name). That makes X-Forwarded-Host a tenant selector, +// so it is honoured only when the machine that opened the connection is one of +// the configured trusted proxies. +// +// When the resulting host names no slug at all — vantage.acme.com, +// status.acme.com, a bare IP — and the deployment is not cloud, the single +// instance of that install is used. A self-hosted install has exactly one, and +// without this every self-hosted status page 404s forever. More than one is a +// refusal rather than a guess. +func publicStatusInstance(c *gin.Context) (*models.Instance, bool) { + host := c.Request.Host + if trustedPeer(c) { + if h := firstForwarded(c.GetHeader("X-Forwarded-Host")); h != "" { + host = h + } + } + if inst, ok := auth.InstanceForHost(host); ok { + return inst, true + } + if auth.HostSlug(host) != "" { + // The host named an instance and that instance does not exist. + return nil, false + } + if services.DeploymentMode() == license.DeploymentCloud { + return nil, false + } + return auth.SoleInstance() +} diff --git a/server/internal/api/trustedproxies.go b/server/internal/api/trustedproxies.go new file mode 100644 index 0000000..8693682 --- /dev/null +++ b/server/internal/api/trustedproxies.go @@ -0,0 +1,84 @@ +package api + +import ( + "net" + "os" + "strings" + "sync" + + "github.com/gin-gonic/gin" +) + +// TrustedProxies reads TRUSTED_PROXIES, a comma-separated list of CIDRs or +// addresses. Unset means trust none: ClientIP() is then the peer address, +// which is right for a direct exposure and means every request behind an +// un-configured proxy shares one address for rate limiting. That is a visible +// failure (one client limited) rather than an invisible one (no limit at all). +// +// This lives here rather than in main.go because the string has two consumers: +// gin's own SetTrustedProxies, which main.go calls with it, and trustedPeer +// below, which the public status page uses to decide whether to believe an +// X-Forwarded-Host. One variable, one parser. +func TrustedProxies() []string { + v := strings.TrimSpace(os.Getenv("TRUSTED_PROXIES")) + if v == "" { + return nil + } + out := []string{} + for _, p := range strings.Split(v, ",") { + if p = strings.TrimSpace(p); p != "" { + out = append(out, p) + } + } + return out +} + +var ( + trustedNetsOnce sync.Once + trustedNets []*net.IPNet +) + +func parsedTrustedNets() []*net.IPNet { + trustedNetsOnce.Do(func() { + for _, entry := range TrustedProxies() { + if _, n, err := net.ParseCIDR(entry); err == nil { + trustedNets = append(trustedNets, n) + continue + } + // A bare address is a /32 or /128. + if ip := net.ParseIP(entry); ip != nil { + bits := 32 + if ip.To4() == nil { + bits = 128 + } + trustedNets = append(trustedNets, &net.IPNet{IP: ip, Mask: net.CIDRMask(bits, bits)}) + } + } + }) + return trustedNets +} + +// trustedPeer reports whether the immediate peer is one of the configured +// proxies. +// +// It deliberately uses RemoteIP() rather than ClientIP(): ClientIP() is the +// reconstructed *client* address, which is derived from the very headers this +// function exists to decide whether to believe. X-Forwarded-Host selects a +// tenant on the public status route, so it is only honoured when the machine +// that actually opened the connection is trusted to have set it. +func trustedPeer(c *gin.Context) bool { + nets := parsedTrustedNets() + if len(nets) == 0 { + return false + } + ip := net.ParseIP(c.RemoteIP()) + if ip == nil { + return false + } + for _, n := range nets { + if n.Contains(ip) { + return true + } + } + return false +} diff --git a/server/internal/auth/instancehost.go b/server/internal/auth/instancehost.go index f057d57..b1ae8b6 100644 --- a/server/internal/auth/instancehost.go +++ b/server/internal/auth/instancehost.go @@ -23,6 +23,10 @@ var ( const instanceCacheTTL = 60 * time.Second +// soleInstanceCacheKey cannot collide with a slug: a slug is [a-z0-9-] and can +// never contain a NUL. +const soleInstanceCacheKey = "\x00sole" + func appRootLabel() string { if v := os.Getenv("APP_ROOT_LABEL"); v != "" { return strings.ToLower(v) @@ -50,25 +54,78 @@ func hostSlug(host string) string { return parts[0] } +// HostSlug exposes the slug rules to callers outside this package that need to +// distinguish "this host names no instance at all" from "this host names an +// instance that does not exist". It is a thin wrapper rather than a second +// implementation on purpose. +func HostSlug(host string) string { return hostSlug(host) } + +// InstanceFromHost resolves the instance named by the request's own Host +// header. Callers that must resolve a host from somewhere else — the public +// status page reads a trusted X-Forwarded-Host — use InstanceForHost so the +// slug rules and the 60s cache stay single-implementation. func InstanceFromHost(c *gin.Context) (*models.Instance, bool) { - slug := hostSlug(c.Request.Host) + return InstanceForHost(c.Request.Host) +} + +// InstanceForHost is InstanceFromHost with the host supplied explicitly. +func InstanceForHost(host string) (*models.Instance, bool) { + slug := hostSlug(host) if slug == "" { return nil, false } - instanceCacheMu.Lock() - if e, ok := instanceCache[slug]; ok && time.Since(e.at) < instanceCacheTTL { - instanceCacheMu.Unlock() - return e.instance, e.instance != nil + if inst, hit := cachedInstanceFor(slug); hit { + return inst, inst != nil } - instanceCacheMu.Unlock() inst, err := services.GetInstanceBySlug(slug) if err != nil || inst == nil { - + // Negative entries are cached too. Without them an unknown but + // well-formed host costs a Mongo query per anonymous request, which + // the public status page exposes to the open internet — and the + // round trip is itself a timing oracle separating "no such instance" + // from "instance exists, page does not". + storeInstance(slug, nil) return nil, false } - instanceCacheMu.Lock() - instanceCache[slug] = cachedInstance{instance: inst, at: time.Now()} - instanceCacheMu.Unlock() + storeInstance(slug, inst) return inst, true } + +// SoleInstance resolves the one instance of a deployment that has exactly one. +// It is how a self-hosted install serves a host that names no slug at all — +// vantage.acme.com, status.acme.com, or a bare address. It reuses the same +// count-then-read that bootstrap uses, and refuses rather than guessing when +// more than one instance exists. +func SoleInstance() (*models.Instance, bool) { + if inst, hit := cachedInstanceFor(soleInstanceCacheKey); hit { + return inst, inst != nil + } + n, err := services.CountInstances() + if err != nil || n != 1 { + storeInstance(soleInstanceCacheKey, nil) + return nil, false + } + inst, err := services.FirstInstance() + if err != nil || inst == nil { + storeInstance(soleInstanceCacheKey, nil) + return nil, false + } + storeInstance(soleInstanceCacheKey, inst) + return inst, true +} + +func cachedInstanceFor(key string) (*models.Instance, bool) { + instanceCacheMu.Lock() + defer instanceCacheMu.Unlock() + if e, ok := instanceCache[key]; ok && time.Since(e.at) < instanceCacheTTL { + return e.instance, true + } + return nil, false +} + +func storeInstance(key string, inst *models.Instance) { + instanceCacheMu.Lock() + instanceCache[key] = cachedInstance{instance: inst, at: time.Now()} + instanceCacheMu.Unlock() +} diff --git a/web/app/status/[pageId]/page.tsx b/web/app/status/[pageId]/page.tsx index 9149875..e96a018 100644 --- a/web/app/status/[pageId]/page.tsx +++ b/web/app/status/[pageId]/page.tsx @@ -1,4 +1,5 @@ import { notFound } from "next/navigation"; +import { headers } from "next/headers"; import StatusPageView from "./StatusPageView"; import type { StatusSnapshot } from "@/lib/api"; @@ -6,16 +7,69 @@ import type { StatusSnapshot } from "@/lib/api"; // auth redirect. This page is served to the public. export const dynamic = "force-dynamic"; -async function fetchSnapshot(host: string, pageId: string): Promise { +type FetchResult = + | { kind: "ok"; snapshot: StatusSnapshot } + | { kind: "not-found" } + | { kind: "unavailable" }; + +async function fetchSnapshot(host: string, forwardedFor: string, pageId: string): Promise { const base = process.env.API_URL ?? process.env.NEXT_PUBLIC_API_URL ?? "http://localhost:8080"; - // The instance is resolved server-side from the Host header, so it has to - // be forwarded explicitly — the server-to-server fetch does not carry it. - const res = await fetch(`${base}/public/status/${encodeURIComponent(pageId)}`, { - headers: { Host: host }, - cache: "no-store", - }); - if (!res.ok) return null; - return res.json(); + + // The instance is resolved server-side from the visitor's host, so it has + // to be forwarded explicitly — this is a server-to-server call and its own + // Host names the Go service. + // + // It goes in X-Forwarded-Host and NOT in Host: `Host` is a forbidden header + // name, and undici (the fetch behind Node) discards it silently. Setting it + // looked like it worked and delivered `host: ` upstream, so every + // status page resolved no instance and 404'd. + const outbound: Record = { "X-Forwarded-Host": host }; + + // Likewise the visitor's own address. Without it the Go server sees a + // request from this pod with no XFF and rate limits every visitor of every + // page into one 120/min bucket — tripped by exactly the traffic an outage + // produces. Appending rather than replacing keeps the chain in front of us + // intact. + if (forwardedFor) outbound["X-Forwarded-For"] = forwardedFor; + + let res: Response; + try { + res = await fetch(`${base}/public/status/${encodeURIComponent(pageId)}`, { + headers: outbound, + cache: "no-store", + }); + } catch { + // The control plane is unreachable. That is not "no such page". + return { kind: "unavailable" }; + } + + if (res.status === 404) return { kind: "not-found" }; + // A 429, a 500 or anything else is a page that exists and cannot be read + // right now. Telling a customer mid-outage that their status page does not + // exist is the worst available answer. + if (!res.ok) return { kind: "unavailable" }; + + try { + return { kind: "ok", snapshot: (await res.json()) as StatusSnapshot }; + } catch { + return { kind: "unavailable" }; + } +} + +// The shell StatusPageView already renders for available:false, reused rather +// than written a second time so there is one unavailable page, not two. +function unavailableSnapshot(): StatusSnapshot { + return { + available: false, + reason: "licence_inactive", + title: "Status", + overall: "no_data", + sections: [], + active_incidents: [], + upcoming_maintenance: [], + history: [], + generated_at: new Date().toISOString(), + }; } export default async function PublicStatusPage({ @@ -24,14 +78,24 @@ export default async function PublicStatusPage({ params: Promise<{ pageId: string }>; }) { const { pageId } = await params; - const { headers } = await import("next/headers"); const h = await headers(); const host = h.get("x-forwarded-host") ?? h.get("host") ?? ""; - const snapshot = await fetchSnapshot(host, pageId); - if (!snapshot) notFound(); + const inboundFor = h.get("x-forwarded-for"); + const peer = h.get("x-real-ip"); + const forwardedFor = [inboundFor, inboundFor ? null : peer] + .filter((v): v is string => !!v) + .join(", "); - return ; + const result = await fetchSnapshot(host, forwardedFor, pageId); + if (result.kind === "not-found") notFound(); + + return ( + + ); } export async function generateMetadata({ params }: { params: Promise<{ pageId: string }> }) {