From badd0b1b479030eb8d397550567ebcc9edd56121 Mon Sep 17 00:00:00 2001 From: Josh Creek <8179928+jcreek@users.noreply.github.com> Date: Tue, 1 Sep 2026 22:58:28 +0100 Subject: [PATCH] fix(multiplayer): validate contract route ids --- multiplayer-next.md | 2 ++ server/api/service.go | 10 ++++++---- server/api/service_test.go | 36 +++++++++++++++++++++++++++++++----- 3 files changed, 39 insertions(+), 9 deletions(-) diff --git a/multiplayer-next.md b/multiplayer-next.md index bfffcab2..335ac401 100644 --- a/multiplayer-next.md +++ b/multiplayer-next.md @@ -1613,6 +1613,8 @@ Proposal responses now require and normalize the contract's RFC3339 `expires_at` Queue responses now validate the complete published shape before projection: opaque ticket/player IDs, playlist and lifecycle enums, integral revision, and RFC3339 enqueue/expiry timestamps. +The public `/api/v1` route adapters now reject non-opaque queue, proposal, assignment, and server path identifiers before delegating to the legacy handlers; adversarial route tests cover short and separator-bearing IDs. + Signed MatchNet claims now also require exact JSON string/integer types for every identity, protocol, expiry, slot, team, and generation field; string-number coercion is rejected before canonical signature verification. Presentation progress: a shared `Game/themes/cosmic_clash_theme.tres` now gives the menu, lobby, matchmaking, and settings surfaces consistent button, input, option, and label styling. The custom-font portion of `TODO.md` remains open until a distributable font asset is selected. diff --git a/server/api/service.go b/server/api/service.go index 90c0eaca..af6fb32f 100644 --- a/server/api/service.go +++ b/server/api/service.go @@ -470,7 +470,7 @@ func (s *Service) contractQueueCreate(w http.ResponseWriter, r *http.Request) { func (s *Service) contractQueueMutation(w http.ResponseWriter, r *http.Request) { path := strings.TrimPrefix(r.URL.Path, "/api/v1/queue/tickets/") parts := strings.Split(path, "/") - if path == "" || len(parts) > 2 || parts[0] == "" || (len(parts) == 2 && parts[1] != "heartbeat") { + if path == "" || len(parts) > 2 || !controlPlaneResourceIDRE.MatchString(parts[0]) || (len(parts) == 2 && parts[1] != "heartbeat") { writeError(w, http.StatusNotFound, "not_found") return } @@ -493,7 +493,8 @@ func (s *Service) contractQueueMutation(w http.ResponseWriter, r *http.Request) func (s *Service) contractProposalMutation(w http.ResponseWriter, r *http.Request) { path := strings.TrimPrefix(r.URL.Path, "/api/v1/proposals/") - if path == "" { + parts := strings.Split(path, "/") + if path == "" || len(parts) > 2 || !controlPlaneResourceIDRE.MatchString(parts[0]) { writeError(w, http.StatusNotFound, "not_found") return } @@ -504,7 +505,7 @@ func (s *Service) contractProposalMutation(w http.ResponseWriter, r *http.Reques func (s *Service) contractAssignment(w http.ResponseWriter, r *http.Request) { path := strings.TrimPrefix(r.URL.Path, "/api/v1/assignments/") - if path == "" || strings.Contains(path, "/") { + if path == "" || strings.Contains(path, "/") || !controlPlaneResourceIDRE.MatchString(path) { writeError(w, http.StatusNotFound, "not_found") return } @@ -520,7 +521,8 @@ func (s *Service) contractServerMutation(w http.ResponseWriter, r *http.Request) // any "/" would 404 every real call. Delegate shape validation to // serverMutation, which already enforces exactly {id}/{result|register}. path := strings.TrimPrefix(r.URL.Path, "/api/v1/servers/") - if path == "" { + parts := strings.Split(path, "/") + if path == "" || len(parts) < 2 || !controlPlaneResourceIDRE.MatchString(parts[0]) { writeError(w, http.StatusNotFound, "not_found") return } diff --git a/server/api/service_test.go b/server/api/service_test.go index c791142b..cb6e6ebd 100644 --- a/server/api/service_test.go +++ b/server/api/service_test.go @@ -277,6 +277,32 @@ func TestDocumentedContractRoutesAdaptToServiceAPI(t *testing.T) { } } +func TestDocumentedContractRoutesRejectNonOpaqueResourceIDs(t *testing.T) { + service := &Service{} + server := httptest.NewServer(service.Handler()) + defer server.Close() + paths := []string{ + "/api/v1/queue/tickets/short/heartbeat", + "/api/v1/proposals/proposal/unsafe/accept", + "/api/v1/assignments/match/unsafe", + "/api/v1/servers/server/unsafe/result", + } + for _, path := range paths { + request, err := http.NewRequest(http.MethodGet, server.URL+path, nil) + if err != nil { + t.Fatal(err) + } + response, err := http.DefaultClient.Do(request) + if err != nil { + t.Fatal(err) + } + if response.StatusCode != http.StatusNotFound { + t.Fatalf("%s status = %d, want 404", path, response.StatusCode) + } + response.Body.Close() + } +} + func TestAuthenticatedWebSocketDeliversOnlyTargetedRevisionedEvents(t *testing.T) { service := &Service{SessionBackend: &sessionBackendSpy{}} server := httptest.NewServer(service.Handler()) @@ -1144,7 +1170,7 @@ func TestServerResultAPIRequiresBoundWorkloadAndDelegatesDurableSubmission(t *te func TestContractServerRoutesAdaptTwoSegmentPaths(t *testing.T) { now := time.Unix(1000, 0).UTC() - binding := domain.WorkloadBinding{AllocationID: "allocation-1", MatchID: "match-1", ServerID: "server-1"} + binding := domain.WorkloadBinding{AllocationID: "allocation-1", MatchID: "match_1234567890", ServerID: "server_123456789"} submitter := &resultSubmitterSpy{} registrar := &serverRegistrarSpy{} service := &Service{Now: func() time.Time { return now }, WorkloadVerify: func(token string, _ time.Time) (domain.WorkloadBinding, error) { @@ -1156,8 +1182,8 @@ func TestContractServerRoutesAdaptTwoSegmentPaths(t *testing.T) { server := httptest.NewServer(service.Handler()) defer server.Close() - registerBody := `{"match_id":"match-1","protocol_version":1,"image_digest":"sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","assignment_ready":false}` - req, _ := http.NewRequest(http.MethodPost, server.URL+"/api/v1/servers/server-1/register", strings.NewReader(registerBody)) + registerBody := `{"match_id":"match_1234567890","protocol_version":1,"image_digest":"sha256:aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa","assignment_ready":false}` + req, _ := http.NewRequest(http.MethodPost, server.URL+"/api/v1/servers/server_123456789/register", strings.NewReader(registerBody)) req.Header.Set("Authorization", "Bearer workload-token") req.Header.Set("Idempotency-Key", "contract-register-key-1") response, err := http.DefaultClient.Do(req) @@ -1166,8 +1192,8 @@ func TestContractServerRoutesAdaptTwoSegmentPaths(t *testing.T) { } response.Body.Close() - resultBody := `{"match_id":"match-1","result_nonce":"nonce-1234567890","score":{"team_0":3,"team_1":2},"integrity_state":"CERTIFIED"}` - req, _ = http.NewRequest(http.MethodPost, server.URL+"/api/v1/servers/server-1/result", strings.NewReader(resultBody)) + resultBody := `{"match_id":"match_1234567890","result_nonce":"nonce-1234567890","score":{"team_0":3,"team_1":2},"integrity_state":"CERTIFIED"}` + req, _ = http.NewRequest(http.MethodPost, server.URL+"/api/v1/servers/server_123456789/result", strings.NewReader(resultBody)) req.Header.Set("Authorization", "Bearer workload-token") req.Header.Set("Idempotency-Key", "contract-result-key-123") response, err = http.DefaultClient.Do(req)