From 6253b620a94bff43ef2f177fc1e82097ab834bb2 Mon Sep 17 00:00:00 2001 From: Josh Creek <8179928+jcreek@users.noreply.github.com> Date: Mon, 31 Aug 2026 21:49:22 +0100 Subject: [PATCH] fix: restrict supervisor drain to loopback --- multiplayer-next.md | 4 +++- multiplayer-todo.md | 2 +- server/supervisor/supervisor.go | 25 +++++++++++++++++++++++++ server/supervisor/supervisor_test.go | 13 +++++++++++++ 4 files changed, 42 insertions(+), 2 deletions(-) diff --git a/multiplayer-next.md b/multiplayer-next.md index 25b4cd64..d3390fe4 100644 --- a/multiplayer-next.md +++ b/multiplayer-next.md @@ -130,7 +130,9 @@ product policy are in [`docs/MATCHMAKING.md`](docs/MATCHMAKING.md). - [ ] Benchmark native x86_64 boot, p99 CPU/RSS/network and tick health; set requests/limits and node density from measurements plus 30% headroom. - [ ] Add 30 s no-show handling, Go PID-1 TERM/drain supervision, PDB/Fleet - drain, signed result annotation/retry, RPO <=5 m and RTO <=30 m. + drain, signed result annotation/retry, RPO <=5 m and RTO <=30 m. The Go + drain boundary is now authenticated and loopback-only; lifecycle/PDB/Fleet + integration remains. - [ ] Rehearse migration only after the second provider's EU/NA locations have Valve approval, POP/certs, public UDP/firewall and coordinator trust. diff --git a/multiplayer-todo.md b/multiplayer-todo.md index 23ce4418..bdcc3deb 100644 --- a/multiplayer-todo.md +++ b/multiplayer-todo.md @@ -1218,7 +1218,7 @@ the local/CI/community transport, not a silent production fallback. | 8.33 `[D:8.26,8.32]` | On-demand-only live capacity and measured N+1: loss of largest node leaves two Ready slots plus headroom for surviving Allocated matches | Interruptible nodes cannot receive live matches; forced node loss neither overloads survivors nor prevents the next allocation | | 8.34 `[D:8.28,8.29]` | Native x86_64 benchmark of boot-to-process-ready and assignment-ready, p99 CPU/RSS/network and 60 Hz ticks; limits/node cap with 30% headroom | Measurements replace old estimates and certify density with no tick backlog | | 8.35 `[D:8.17,8.19,8.20,8.30,8.31]` | **IN PROGRESS.** Pure Go initial-connect policy decides ranked 30 s no-show cancellation with abandon ladder and casual 60 s bot start only when each team has a human; empty-team casual allocations cancel | `server/domain/noshow.go` covers wait/deadline boundaries, deterministic no-show/innocent ordering, ranked cooldown history and no pre-live rating action; persistent ticket restoration, allocation shutdown, bot spawn and live integration remain | -| 8.36 `[D:8.10,8.25,8.28,8.30]` | **IN PROGRESS.** Supervisor exposes an authenticated localhost drain request boundary that never places the token in command arguments/logs | `server/supervisor/` covers bearer-token enforcement and rejection of missing drain credentials; TERM signal handling, 300 s/285 s lifecycle, PDB/Fleet drain and infrastructure-abort classification remain | +| 8.36 `[D:8.10,8.25,8.28,8.30]` | **IN PROGRESS.** Supervisor exposes an authenticated loopback-only drain request boundary that never places the token in command arguments/logs and rejects remote/partial/query-bearing configurations | `server/supervisor/` covers bearer-token enforcement, loopback URL validation, secret-safe configuration and rejection of missing drain credentials; TERM signal handling, 300 s/285 s lifecycle, PDB/Fleet drain and infrastructure-abort classification remain | | 8.37 `[D:8.5,8.10,8.25,8.26,8.31]` | Horizontally scaled primary control plane + warm standby, EU/NA fleets, RPO <=5 m/RTO <=30 m; signed result annotation/reconciliation preserves delivery outages | Restore/failover meet targets; live simulation continues; valid delayed result commits exactly once after recovery | | 8.38 `[D:7.7,8.26,8.36,8.37]` | Provider migration after Valve approves both providers' EU/NA POPs/certs and public UDP: restore, validate coordinator trust, switch allocations, drain old | Both geographies complete Hosted-SDR matches on new provider and no old-provider live match is terminated | diff --git a/server/supervisor/supervisor.go b/server/supervisor/supervisor.go index d20bab22..bddce6bb 100644 --- a/server/supervisor/supervisor.go +++ b/server/supervisor/supervisor.go @@ -7,7 +7,9 @@ import ( "context" "encoding/json" "fmt" + "net" "net/http" + "net/url" "os" "os/exec" "strconv" @@ -63,9 +65,32 @@ func New(config Config) (*Supervisor, error) { if config.HTTPClient == nil { config.HTTPClient = http.DefaultClient } + if (config.DrainURL == "") != (config.DrainToken == "") { + return nil, fmt.Errorf("drain URL and token must be configured together") + } + if config.DrainURL != "" { + if err := validateLocalDrainURL(config.DrainURL); err != nil { + return nil, err + } + } return &Supervisor{config: config, client: config.HTTPClient}, nil } +func validateLocalDrainURL(raw string) error { + parsed, err := url.Parse(raw) + if err != nil || (parsed.Scheme != "http" && parsed.Scheme != "https") || parsed.Host == "" || parsed.Path == "" || parsed.User != nil || parsed.RawQuery != "" || parsed.Fragment != "" { + return fmt.Errorf("drain URL must be a loopback HTTP endpoint") + } + host := parsed.Hostname() + if host != "localhost" { + ip := net.ParseIP(host) + if ip == nil || !ip.IsLoopback() { + return fmt.Errorf("drain URL must be a loopback HTTP endpoint") + } + } + return nil +} + // Start launches the process and marks Agones Ready only after the explicit // readiness probe succeeds. No stdout/log scraping is used. With no SDK URL, // this is direct/Compose mode and the command is simply started. diff --git a/server/supervisor/supervisor_test.go b/server/supervisor/supervisor_test.go index fe81cf34..dfe76872 100644 --- a/server/supervisor/supervisor_test.go +++ b/server/supervisor/supervisor_test.go @@ -167,3 +167,16 @@ func TestAssignedEndpointRejectsMalformedAddressAndPort(t *testing.T) { server.Close() } } + +func TestSupervisorRejectsRemoteOrPartialDrainConfiguration(t *testing.T) { + for _, config := range []Config{ + {Command: []string{"/bin/sh", "-c", "exit 0"}, DrainURL: "https://example.com/drain", DrainToken: "token-1234567890123456"}, + {Command: []string{"/bin/sh", "-c", "exit 0"}, DrainURL: "http://127.0.0.1/drain"}, + {Command: []string{"/bin/sh", "-c", "exit 0"}, DrainToken: "token-1234567890123456"}, + {Command: []string{"/bin/sh", "-c", "exit 0"}, DrainURL: "http://127.0.0.1/drain?token=leaked", DrainToken: "token-1234567890123456"}, + } { + if _, err := New(config); err == nil { + t.Fatalf("unsafe drain configuration accepted: %+v", config) + } + } +}