From aaad7db09d17b62e849bf3706b0d41ade7150523 Mon Sep 17 00:00:00 2001 From: mrhid6 Date: Thu, 17 Sep 2026 07:53:37 +0000 Subject: [PATCH] docs(plan): accept heartbeat token in X-Vantage-Token header and mask URL tokens in logs --- .../2026-09-17-metric-alerts-heartbeats.md | 296 +++++++++++++----- 1 file changed, 226 insertions(+), 70 deletions(-) diff --git a/docs/superpowers/plans/2026-09-17-metric-alerts-heartbeats.md b/docs/superpowers/plans/2026-09-17-metric-alerts-heartbeats.md index 875c469..ce2a16b 100644 --- a/docs/superpowers/plans/2026-09-17-metric-alerts-heartbeats.md +++ b/docs/superpowers/plans/2026-09-17-metric-alerts-heartbeats.md @@ -13,6 +13,7 @@ ## Global Constraints - Every new route under `/api` needs an entry in `routeScopes` (server/internal/api/scopes.go) AND in `serverScopedRoutes` (server/internal/api/serverscope.go), or the server refuses to boot. Run `go test ./internal/api/` after touching routes. +- Heartbeat tokens may be sent in the URL or in the `X-Vantage-Token` header; a URL token wins. URL tokens are masked in the server request log and the bundled nginx access log. - **Deviation from spec:** the public ping endpoints are mounted at `/public/hb/:token` (plus `/start` and `/fail`), not `/hb/:token`. `/public/` is already routed to the Go server by nginx (deploy/docker/nginx/vantage.conf), the Helm ingress and web/next.config.ts. A new top-level prefix would need all three changed. Update the spec's Heartbeats section in Task 3. - Monitors run regardless of licence state. `metricsched` runs inside `bus.RunAsLeader(ctx, "housekeeping", ...)` next to `monitorsched.Start`. - Heartbeat tokens are stored only as SHA-256 hex hashes (`HeartbeatTokenHash`, `json:"-"`). Plaintext is returned only by create and rotate. @@ -811,26 +812,44 @@ git commit -m "feat(monitors): heartbeat token, ping recording and overdue verdi --- -### Task 3: Public ping endpoints, rotate route, sweeper loop +### Task 3: Public ping endpoints, header token, log masking, rotate route, sweeper loop **Files:** - Create: `server/internal/api/heartbeats.go` - Create: `server/internal/api/heartbeats_test.go` - Modify: `server/internal/api/handlers.go:~66` (next to `/public/status/:pageId`) -- Modify: `server/internal/api/monitors.go` (`registerMonitorRoutes`, `createMonitor`) +- Modify: `server/internal/api/monitors.go` (`registerMonitorRoutes`, `createMonitor`, `updateMonitor`) - Modify: `server/internal/api/scopes.go:~109` - Modify: `server/internal/api/serverscope.go:~293` - Create: `server/internal/metricsched/scheduler.go` -- Modify: `server/cmd/main.go:~261` -- Modify: `docs/superpowers/specs/2026-09-17-metric-alerts-heartbeats-design.md` (path change) +- Modify: `server/cmd/main.go:~261` (sweeper) and `:308` (gin logger) +- Modify: `deploy/docker/nginx/vantage.conf` +- Modify: `docs/superpowers/specs/2026-09-17-metric-alerts-heartbeats-design.md` (path and header) **Interfaces:** -- Consumes: `services.RecordHeartbeat`, `services.RotateHeartbeatToken`, `services.SweepHeartbeats`, `services.ErrHeartbeatNotFound`, `services.MaxHeartbeatBody`. -- Produces: routes `GET|POST /public/hb/:token`, `GET|POST /public/hb/:token/:kind`, `POST /api/monitors/:id/rotate-token` returning `{"heartbeat_token": string}`; `metricsched.Start(ctx)`. +- Consumes: `services.RecordHeartbeat`, `services.RotateHeartbeatToken`, `services.SweepHeartbeats`, `services.ErrHeartbeatNotFound`, `services.ErrInvalidMonitor`, `services.MaxHeartbeatBody`, `services.HashHeartbeatToken`. +- Produces: + - Routes (each GET and POST): `/public/hb`, `/public/hb/:a`, `/public/hb/:a/:b` + - `const HeartbeatTokenHeader = "X-Vantage-Token"` + - `func resolveHeartbeat(header, a, b string) (token, kind string, ok bool)` + - `func MaskLogPath(path string) string` + - `POST /api/monitors/:id/rotate-token` returning `{"heartbeat_token": string}` + - `metricsched.Start(ctx)` -- [ ] **Step 1: Write the failing handler test** +The token can arrive two ways, and both are accepted: -`server/internal/api/heartbeats_test.go` tests the parts that need no database: path parsing and the unknown-kind 404. +| Request | Token | Kind | +|---|---|---| +| `/public/hb/` | URL | ping | +| `/public/hb//start` or `/fail` | URL | start / fail | +| `/public/hb` + `X-Vantage-Token` | header | ping | +| `/public/hb/start` or `/fail` + `X-Vantage-Token` | header | start / fail | + +If a URL token is present it wins, and the header is ignored. Anything that doesn't resolve answers 404, the same as an unknown token. + +- [ ] **Step 1: Write the failing tests** + +`server/internal/api/heartbeats_test.go`: ```go package api @@ -843,19 +862,60 @@ import ( "github.com/gin-gonic/gin" ) -func TestHeartbeatKindFromPath(t *testing.T) { - cases := map[string]string{"": "ping", "start": "start", "fail": "fail", "bogus": ""} +func TestResolveHeartbeat(t *testing.T) { + cases := []struct { + name, header, a, b string + token, kind string + ok bool + }{ + {"url ping", "", "tok", "", "tok", "ping", true}, + {"url start", "", "tok", "start", "tok", "start", true}, + {"url fail", "", "tok", "fail", "tok", "fail", true}, + {"url bad kind", "", "tok", "explode", "", "", false}, + {"header ping", "htok", "", "", "htok", "ping", true}, + {"header start", "htok", "start", "", "htok", "start", true}, + {"header fail", "htok", "fail", "", "htok", "fail", true}, + {"url token wins over header", "htok", "tok", "", "tok", "ping", true}, + {"url token and kind win over header", "htok", "tok", "fail", "tok", "fail", true}, + {"nothing", "", "", "", "", "", false}, + // Without a header, /public/hb/start is a token called "start": it + // resolves, and the lookup answers 404 like any unknown token. + {"bare start without header", "", "start", "", "start", "ping", true}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + tok, kind, ok := resolveHeartbeat(c.header, c.a, c.b) + if tok != c.token || kind != c.kind || ok != c.ok { + t.Fatalf("got (%q,%q,%v), want (%q,%q,%v)", tok, kind, ok, c.token, c.kind, c.ok) + } + }) + } +} + +// A ping URL is a credential. Anything that logs request paths must see a +// masked one. +func TestMaskLogPath(t *testing.T) { + cases := map[string]string{ + "/public/hb/abc123": "/public/hb/***", + "/public/hb/abc123/fail": "/public/hb/***/fail", + "/public/hb/abc123?x=1": "/public/hb/***?x=1", + "/public/hb/start": "/public/hb/start", + "/public/hb/fail": "/public/hb/fail", + "/public/hb": "/public/hb", + "/public/status/page": "/public/status/page", + "/api/monitors/abc/uptime": "/api/monitors/abc/uptime", + } for in, want := range cases { - if got := heartbeatKind(in); got != want { - t.Errorf("heartbeatKind(%q) = %q, want %q", in, got, want) + if got := MaskLogPath(in); got != want { + t.Errorf("MaskLogPath(%q) = %q, want %q", in, got, want) } } } -func TestHeartbeatUnknownKindIs404(t *testing.T) { +func TestHeartbeatUnresolvableIs404(t *testing.T) { gin.SetMode(gin.TestMode) r := gin.New() - r.POST("/public/hb/:token/:kind", handleHeartbeat) + r.POST("/public/hb/:a/:b", handleHeartbeat) w := httptest.NewRecorder() r.ServeHTTP(w, httptest.NewRequest(http.MethodPost, "/public/hb/abc/explode", nil)) if w.Code != http.StatusNotFound { @@ -866,8 +926,8 @@ func TestHeartbeatUnknownKindIs404(t *testing.T) { - [ ] **Step 2: Run to verify failure** -Run: `cd server && go test ./internal/api/ -run Heartbeat` -Expected: compile error, `undefined: heartbeatKind`. +Run: `cd server && go test ./internal/api/ -run 'Heartbeat|MaskLogPath'` +Expected: compile error, `undefined: resolveHeartbeat`. - [ ] **Step 3: Implement `api/heartbeats.go`** @@ -880,6 +940,7 @@ import ( "log" "net/http" "strconv" + "strings" "time" "gitea.hostxtra.co.uk/mrhid6/vantage/server/internal/auth" @@ -887,6 +948,12 @@ import ( "github.com/gin-gonic/gin" ) +// HeartbeatTokenHeader carries the ping token for callers who want it out of +// URLs, and so out of their own proxies' access logs. +const HeartbeatTokenHeader = "X-Vantage-Token" + +const heartbeatPathPrefix = "/public/hb/" + func heartbeatKind(seg string) string { switch seg { case "": @@ -899,33 +966,67 @@ func heartbeatKind(seg string) string { return "" } -// rateLimitHeartbeat admits one request per token per second. A cron job -// pinging in a loop should not become a write per request, and a leaked URL -// should not be a way to hammer Mongo. Like the status page limiter it allows -// when Redis is down: a missed ping pages someone. -func rateLimitHeartbeat() gin.HandlerFunc { - return func(c *gin.Context) { - rdb := auth.Redis() - if rdb == nil { - c.Next() - return - } - key := "vantage:hbrl:" + services.HashHeartbeatToken(c.Param("token")) + ":" + strconv.FormatInt(time.Now().Unix(), 10) - count, err := rdb.Incr(c.Request.Context(), key).Result() - if err != nil { - c.Next() - return - } - if count == 1 { - rdb.Expire(c.Request.Context(), key, 2*time.Second) - } - if count > 1 { - c.Header("Retry-After", "1") - c.AbortWithStatusJSON(http.StatusTooManyRequests, gin.H{"error": "too many requests", "code": "rate_limited"}) - return - } - c.Next() +// resolveHeartbeat maps the three route shapes onto a token and a kind. a and b +// are the first and second path segments after /public/hb. A token in the URL +// wins over the header, so a URL copied from the UI behaves the same no matter +// what headers a client adds. +func resolveHeartbeat(header, a, b string) (string, string, bool) { + var token, kindSeg string + switch { + // a is a URL token unless a header is present, no second segment follows + // and a is itself a kind word (the header routes /public/hb/start|fail). + case a != "" && (header == "" || b != "" || heartbeatKind(a) == ""): + token, kindSeg = a, b + case header != "": + token, kindSeg = header, a + default: + return "", "", false } + kind := heartbeatKind(kindSeg) + if token == "" || kind == "" { + return "", "", false + } + return token, kind, true +} + +// MaskLogPath hides a heartbeat token in a request path before it is logged. +// "start" and "fail" directly under the prefix are the header-token routes and +// carry no secret, so they are left readable. +func MaskLogPath(path string) string { + rest, found := strings.CutPrefix(path, heartbeatPathPrefix) + if !found { + return path + } + end := strings.IndexAny(rest, "/?") + if end < 0 { + end = len(rest) + } + seg := rest[:end] + if seg == "" || seg == "start" || seg == "fail" { + return path + } + return heartbeatPathPrefix + "***" + rest[end:] +} + +// heartbeatAllowed admits one request per token per second. A cron job pinging +// in a loop should not become a write per request, and a leaked URL should not +// be a way to hammer Mongo. Like the status page limiter it allows when Redis +// is down: a missed ping pages someone. It runs inside the handler rather than +// as middleware because the token may come from a header. +func heartbeatAllowed(c *gin.Context, token string) bool { + rdb := auth.Redis() + if rdb == nil { + return true + } + key := "vantage:hbrl:" + services.HashHeartbeatToken(token) + ":" + strconv.FormatInt(time.Now().Unix(), 10) + count, err := rdb.Incr(c.Request.Context(), key).Result() + if err != nil { + return true + } + if count == 1 { + rdb.Expire(c.Request.Context(), key, 2*time.Second) + } + return count <= 1 } // handleHeartbeat records a push from a job. It is mounted on the gin root @@ -933,19 +1034,25 @@ func rateLimitHeartbeat() gin.HandlerFunc { // getPublicStatusPage): no session, no token, no licence gate, and /public is // already routed to this server by every deployment. // -// Unknown token, disabled monitor and unknown kind all answer the same 404. +// Unknown token, disabled monitor and an unresolvable path all answer the same +// 404. func handleHeartbeat(c *gin.Context) { - kind := heartbeatKind(c.Param("kind")) - if kind == "" { + token, kind, ok := resolveHeartbeat(c.GetHeader(HeartbeatTokenHeader), c.Param("a"), c.Param("b")) + if !ok { c.String(http.StatusNotFound, "not found") return } + if !heartbeatAllowed(c, token) { + c.Header("Retry-After", "1") + c.String(http.StatusTooManyRequests, "too many requests") + return + } var body string if kind == services.HeartbeatFail && c.Request.Body != nil { b, _ := io.ReadAll(io.LimitReader(c.Request.Body, services.MaxHeartbeatBody)) body = string(b) } - err := services.RecordHeartbeat(c.Param("token"), kind, body, time.Now()) + err := services.RecordHeartbeat(token, kind, body, time.Now()) if errors.Is(err, services.ErrHeartbeatNotFound) { c.String(http.StatusNotFound, "not found") return @@ -987,32 +1094,32 @@ func rotateHeartbeatToken(c *gin.Context) { - [ ] **Step 4: Register routes and declarations** -`handlers.go`, directly under the `/public/status/:pageId` line: +In `handlers.go`, directly under the `/public/status/:pageId` line: ```go - hb := r.Group("/public/hb/:token", rateLimitHeartbeat()) - { - hb.GET("", handleHeartbeat) - hb.POST("", handleHeartbeat) - hb.GET("/:kind", handleHeartbeat) - hb.POST("/:kind", handleHeartbeat) + // Ping endpoints for heartbeat monitors. The token is in the URL or the + // X-Vantage-Token header; see resolveHeartbeat for the shapes. Rate + // limiting is per token inside the handler. + for _, p := range []string{"/public/hb", "/public/hb/:a", "/public/hb/:a/:b"} { + r.GET(p, handleHeartbeat) + r.POST(p, handleHeartbeat) } ``` -`monitors.go` `registerMonitorRoutes`: `g.POST("/monitors/:id/rotate-token", rotateHeartbeatToken)`. +In `monitors.go` `registerMonitorRoutes`: `g.POST("/monitors/:id/rotate-token", rotateHeartbeatToken)`. -`scopes.go` after `"GET /api/monitors/:id/samples"`: `"POST /api/monitors/:id/rotate-token": "monitors:write",` +In `scopes.go`, after `"GET /api/monitors/:id/samples"`: `"POST /api/monitors/:id/rotate-token": "monitors:write",` -`serverscope.go` next to `DELETE /api/monitors/:id`: +In `serverscope.go`, next to `DELETE /api/monitors/:id`: ```go // Rotating a ping token touches no server and returns only the token. "POST /api/monitors/:id/rotate-token": fleetWide, ``` -Also check that `RequireActiveLicense` in `licence.go` gates the rotate POST (deny by default). Leave it gated, matching create/update. +`RequireActiveLicense` in `licence.go` gates the rotate POST (deny by default). Leave it gated, matching create/update. -In `createMonitor` in `api/monitors.go`, map validation errors to 400 instead of 500. `validateHeartbeat` wraps `services.ErrInvalidMonitor` (Task 2), and so do the metric validators later. In the handler: +In `createMonitor` and `updateMonitor` in `api/monitors.go`, answer validation failures with 400 instead of 500. `validateHeartbeat` wraps `services.ErrInvalidMonitor` (Task 2), and so will the metric validators: ```go if errors.Is(err, services.ErrInvalidMonitor) { @@ -1021,9 +1128,49 @@ In `createMonitor` in `api/monitors.go`, map validation errors to 400 instead of } ``` -Apply the same mapping in `updateMonitor`. +- [ ] **Step 5: Mask tokens in the server's request log** -- [ ] **Step 5: Create `metricsched`** +In `server/cmd/main.go:308`, keep `SkipPaths` and add a formatter that is gin's default format with the path masked: + +```go + r.Use(gin.LoggerWithConfig(gin.LoggerConfig{ + SkipPaths: []string{"/api/console/tunnel"}, + // Heartbeat URLs carry a credential; the request log must not. + Formatter: func(p gin.LogFormatterParams) string { + return fmt.Sprintf("[GIN] %v | %3d | %13v | %15s | %-7s %#v\n%s", + p.TimeStamp.Format("2006/01/02 - 15:04:05"), + p.StatusCode, p.Latency, p.ClientIP, p.Method, + api.MaskLogPath(p.Path), p.ErrorMessage) + }, + })) +``` + +Then grep for anything else that logs a request path or URL: `grep -rn "Request.URL\|FullPath()\|c.Request.RequestURI" server/internal --include=*.go | grep -v _test`. Any that can see `/public/hb` must pass the value through `api.MaskLogPath` (or be moved behind a check that skips `/public/hb`). + +- [ ] **Step 6: Mask tokens in the bundled nginx access log** + +In `deploy/docker/nginx/vantage.conf`, next to the existing `map $http_upgrade` block (http context): + +```nginx +# Heartbeat ping URLs carry a credential. Log them with the token replaced; +# the header form (X-Vantage-Token) is never logged by this format. +map $request_uri $vantage_log_uri { + "~^/public/hb/(?!start(?:[/?]|$)|fail(?:[/?]|$))[^/?]+(?.*)$" "/public/hb/***$hb_rest"; + default $request_uri; +} + +log_format vantage '$remote_addr - $remote_user [$time_local] ' + '"$request_method $vantage_log_uri $server_protocol" ' + '$status $body_bytes_sent "$http_referer" "$http_user_agent"'; +``` + +In the `server` block, add `access_log /var/log/nginx/access.log vantage;` under `server_name _;`. + +Verify syntax: `docker run --rm -v "$PWD/deploy/docker/nginx/vantage.conf:/etc/nginx/conf.d/default.conf:ro" nginx:stable nginx -t` (if Docker is unavailable, say so in the report rather than skipping silently). Check `docker-compose.yml` to confirm which nginx image the stack uses, and use that image tag here. + +The Helm chart routes `/public/` through the customer's ingress controller, whose logs we don't control. Task 11 documents this. + +- [ ] **Step 7: Create `metricsched`** `server/internal/metricsched/scheduler.go`: @@ -1072,20 +1219,20 @@ func sweep(now time.Time) { `cmd/main.go`: add `metricsched.Start(jobCtx)` after `monitorsched.Start(jobCtx)` and import the package. -- [ ] **Step 6: Update the spec's path** +- [ ] **Step 8: Update the spec** -In the spec's "Public endpoints" section, replace `/hb/:token` with `/public/hb/:token` in all three bullets and add: "Mounted under /public because every deployment already routes that prefix to the server." +In the spec's "Public endpoints" section, replace `/hb/:token` with `/public/hb/:token` in all three bullets and add: "Mounted under /public because every deployment already routes that prefix to the server. The token may instead be sent in the `X-Vantage-Token` header to `/public/hb`, `/public/hb/start` or `/public/hb/fail`; a URL token wins when both are present. The server's request log and the bundled nginx access log mask the URL token." -- [ ] **Step 7: Run tests** +- [ ] **Step 9: Run tests** Run: `cd server && go build ./... && go vet ./... && go test ./internal/api/ ./internal/services/` Expected: PASS, including `TestRegisteredRoutesPassBootAssertions`. -- [ ] **Step 8: Commit** +- [ ] **Step 10: Commit** ```bash -git add server docs/superpowers/specs/2026-09-17-metric-alerts-heartbeats-design.md -git commit -m "feat(monitors): public heartbeat ping endpoints, token rotation and sweeper" +git add server deploy/docker/nginx/vantage.conf docs/superpowers/specs/2026-09-17-metric-alerts-heartbeats-design.md +git commit -m "feat(monitors): public heartbeat ping endpoints, header token, log masking and sweeper" ``` --- @@ -1170,7 +1317,9 @@ curl -fsS -m 10 --retry 3 ${url} # mark start, to measure duration curl -fsS -m 10 ${url}/start # report failure with output -your-job 2>&1 | tail -c 1024 | curl -fsS -m 10 --data-binary @- ${url}/fail`} +your-job 2>&1 | tail -c 1024 | curl -fsS -m 10 --data-binary @- ${url}/fail +# or keep the token out of URLs (and your proxy logs) +curl -fsS -m 10 -X POST -H "X-Vantage-Token: ${token}" ${window.location.origin}/public/hb`} ); @@ -2320,7 +2469,7 @@ HTTP, TCP, ICMP and TLS checks are pull-based. Each has a `runner`: `"server"` ( Two passive types are never run by `monitorsched` or agents; `metricsched` sweeps them every 30s: -- `heartbeat`: jobs call `/public/hb/` (plus `/start`, `/fail`). Down when a ping is overdue past period + grace, a start never finishes, or `/fail` is called. The token is stored hashed; plaintext is shown on create and `POST /api/monitors/:id/rotate-token` only. +- `heartbeat`: jobs call `/public/hb/` (plus `/start`, `/fail`), or send the token in `X-Vantage-Token` to `/public/hb[/start|/fail]`; a URL token wins. Request logs (gin formatter via `api.MaskLogPath`, bundled nginx `log_format vantage`) mask URL tokens. Down when a ping is overdue past period + grace, a start never finishes, or `/fail` is called. The token is stored hashed; plaintext is shown on create and `POST /api/monitors/:id/rotate-token` only. - `metric`: a tag `selector` plus a rule (`disk_pct`, `disk_free_gb`, `mem_pct`, `load_per_core`, `unit_failed`, `container_unhealthy`, `reboot_pending_days`, `agent_offline_min`) evaluated against stored inventory and workloads. State is per server in `monitor_server_states`, with one incident per breaching server (`Incident.ServerID`). Inventory older than 5 minutes is skipped. Restricted tokens may only use selectors inside their tag scope. All three paths share `applyTransition` (services/monitortransition.go) for incidents and notifications. @@ -2336,7 +2485,7 @@ import { ensureOwner, signIn } from "./helpers"; const BASE_URL = process.env.E2E_BASE_URL ?? "http://localhost:3000"; -test("heartbeat monitor goes up after a ping and down after /fail", async ({ page, request }) => { +test("heartbeat monitor: URL ping, /fail, header ping", async ({ page, request }) => { const owner = await ensureOwner(request); await signIn(page, owner.email, owner.password); @@ -2370,7 +2519,14 @@ test("heartbeat monitor goes up after a ping and down after /fail", async ({ pag const incidents = await (await page.request.get(`${BASE_URL}/api/monitors/${monitor.monitor_id}/incidents`)).json(); expect(incidents.length).toBeGreaterThan(0); + await new Promise((r) => setTimeout(r, 1100)); + const headerPing = await request.post(`${BASE_URL}/public/hb`, { headers: { "X-Vantage-Token": monitor.heartbeat_token } }); + expect(headerPing.status()).toBe(200); + got = await (await page.request.get(`${BASE_URL}/api/monitors/${monitor.monitor_id}`)).json(); + expect(got.state.status).toBe("up"); + expect((await request.get(`${BASE_URL}/public/hb/not-a-real-token`)).status()).toBe(404); + expect((await request.post(`${BASE_URL}/public/hb`)).status()).toBe(404); await page.goto(`${BASE_URL}/monitors/${monitor.monitor_id}`); await expect(page.getByText(/last ping/i)).toBeVisible(); @@ -2395,6 +2551,6 @@ git commit -m "docs(monitors): document heartbeat and metric monitors; add heart ## Out of scope for this plan -- The vantage-docs user guide page. The spec lists it; write it after the feature is verified in a real deployment, and commit it in the vantage-docs repo. +- The vantage-docs user guide page. It must say that behind the Helm chart the ingress controller's access log records URL tokens, and recommend the `X-Vantage-Token` header there. The spec lists it; write it after the feature is verified in a real deployment, and commit it in the vantage-docs repo. - Scheduled workflows pinging a heartbeat automatically. - Marking the gap review artifact as shipped. Do that only after deploy and a real ping and alert have been observed.