diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 64a343aa03..600e45ca35 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -8910,3 +8910,12 @@ agent from its last report instead of from the PVE poll, and mark it stale at their own heartbeat cutoff (`fleethealth.AgentStaleThreshold`, which is separate from the monitoring reporting lease). Agent registration, enrolment, install, update, removal and report identity are unchanged. + +### Demo write guard ignores websocket upgrade headers + +`internal/api/demo_middleware.go` changed only so a `POST`, `PUT`, `PATCH` or +`DELETE` carrying `Upgrade: websocket` is judged by the demo read-only guard +like any other write instead of always reaching its handler. Agent command +websocket handshakes to `/api/agent/ws` are HTTP/1.1 `GET` requests, the only +transport its gorilla upgrader serves, and still connect on a demo instance. +No agent registration, enrolment, install, update or removal path changed. diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 6b204396df..4227abe9af 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -7651,6 +7651,17 @@ read-only demo account. That same hidden read-side boundary includes `GET` and `HEAD` reads for `/api/admin/users` and manual discovery at `/api/discover`; public demo mode may block writes generically, but it must not reveal that admin-user inventory or manual-discovery read routes exist. +That generic write block keys on the request method alone. The demo +middleware admits `GET`, `HEAD` and `OPTIONS` plus the login, logout, AI +execute and OIDC login/callback allowlist; HTTP/1.1 websocket handshakes for +`/ws` and `/api/agent/ws` are `GET` requests and connect through that read +branch, so an `Upgrade: websocket` or `Connection: Upgrade` header never +carries a `POST`, `PUT`, `PATCH` or `DELETE` past the guard. A demo deployment that signs +visitors in as the configured admin, as the public demo does, has only this +guard between that session and admin writes such as `/api/system/mock-mode`. +`TestContract_DemoModeUpgradeHeadersDoNotExemptWrites` in +`internal/api/contract_test.go` signs in as that admin through the full router +and pins both halves. That shared infrastructure install boundary now also preserves copied shell command payload continuity: any privilege-escalation wrapper applied at `frontend-modern/src/components/Settings/InfrastructureInstallerSection.tsx` diff --git a/docs/release-control/v6/internal/subsystems/storage-recovery.md b/docs/release-control/v6/internal/subsystems/storage-recovery.md index addd5ef2d7..1a55c2d23c 100644 --- a/docs/release-control/v6/internal/subsystems/storage-recovery.md +++ b/docs/release-control/v6/internal/subsystems/storage-recovery.md @@ -6895,3 +6895,14 @@ migration or recovery authority is added or moved. The performance report and reporting runtime snapshot handlers apply the same test before tabulating a disk temperature. That changes only which held value a report shows, not any storage or recovery path. + +### Demo write guard ignores websocket upgrade headers + +`internal/api/demo_middleware.go` no longer exempts every request carrying +`Upgrade: websocket` from the demo read-only guard. On a `DEMO_MODE` instance a +`POST`, `PUT`, `PATCH` or `DELETE` with websocket handshake headers is now +judged like any other write: outside the login, logout, AI execute and OIDC +allowlist it gets the generic demo `403`, or `404` on a hidden route, and that +includes config import at `/api/config/import`. HTTP/1.1 websocket handshakes +are `GET` requests and still connect. No storage, retention, backup, migration +or recovery path is added or moved. diff --git a/internal/api/contract_test.go b/internal/api/contract_test.go index 1adcbc7d63..953ed0fba2 100644 --- a/internal/api/contract_test.go +++ b/internal/api/contract_test.go @@ -60,6 +60,7 @@ import ( "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources" "github.com/rcourtman/pulse-go-rewrite/internal/updates" "github.com/rcourtman/pulse-go-rewrite/internal/vmware" + pulsews "github.com/rcourtman/pulse-go-rewrite/internal/websocket" agentshost "github.com/rcourtman/pulse-go-rewrite/pkg/agents/host" "github.com/rcourtman/pulse-go-rewrite/pkg/aicontracts" "github.com/rcourtman/pulse-go-rewrite/pkg/audit" @@ -9261,6 +9262,119 @@ func TestContract_DemoModeCommercialSurfacePolicy(t *testing.T) { }) } +// The public demo signs every visitor in as the configured admin, so admin +// routes accept a demo session and the demo guard is the only thing between it +// and a write. Websocket handshake headers must not carry a write past that +// guard, while a genuine handshake from the same session must still connect. +func TestContract_DemoModeUpgradeHeadersDoNotExemptWrites(t *testing.T) { + setMockModeForTest(t, true) + + dataDir := t.TempDir() + hashedPass, err := authpkg.HashPassword("demo") + if err != nil { + t.Fatalf("hash password: %v", err) + } + cfg := &config.Config{ + DataPath: dataDir, + ConfigPath: dataDir, + DemoMode: true, + AuthUser: "demo", + AuthPass: hashedPass, + } + + monitor, err := monitoring.New(cfg) + if err != nil { + t.Fatalf("new monitor: %v", err) + } + t.Cleanup(func() { monitor.Stop() }) + if err := monitor.SetMockMode(true); err != nil { + t.Fatalf("set monitor mock mode: %v", err) + } + + hub := pulsews.NewHub(nil) + go hub.Run() + t.Cleanup(hub.Stop) + router := NewRouter(cfg, monitor, nil, hub, nil, "1.0.0") + cleanupTestRouter(t, router) + server := newIPv4HTTPServer(t, router.Handler()) + t.Cleanup(server.Close) + + loginResp, err := http.Post(server.URL+"/api/login", "application/json", strings.NewReader(`{"username":"demo","password":"demo"}`)) + if err != nil { + t.Fatalf("demo login: %v", err) + } + loginResp.Body.Close() + if loginResp.StatusCode != http.StatusOK { + t.Fatalf("demo login status = %d, want %d", loginResp.StatusCode, http.StatusOK) + } + var cookiePairs []string + csrfToken := "" + for _, cookie := range loginResp.Cookies() { + cookiePairs = append(cookiePairs, cookie.Name+"="+cookie.Value) + if cookie.Name == CookieNameCSRF { + csrfToken = cookie.Value + } + } + if csrfToken == "" { + t.Fatal("expected CSRF cookie after demo login") + } + sessionCookies := strings.Join(cookiePairs, "; ") + + send := func(method, path, body string, headers map[string]string) (int, string) { + t.Helper() + req, err := http.NewRequest(method, server.URL+path, strings.NewReader(body)) + if err != nil { + t.Fatalf("build %s %s: %v", method, path, err) + } + req.Header.Set("Cookie", sessionCookies) + for key, value := range headers { + req.Header.Set(key, value) + } + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("%s %s: %v", method, path, err) + } + defer resp.Body.Close() + var payload bytes.Buffer + _, _ = payload.ReadFrom(resp.Body) + return resp.StatusCode, payload.String() + } + + if status, body := send(http.MethodGet, "/api/system/mock-mode", "", nil); status != http.StatusOK { + t.Fatalf("demo session read of admin route status = %d, want %d: %s", status, http.StatusOK, body) + } + + status, body := send(http.MethodPost, "/api/system/mock-mode", `{"enabled":false}`, map[string]string{ + "Content-Type": "application/json", + "X-CSRF-Token": csrfToken, + "Connection": "Upgrade", + "Upgrade": "websocket", + "Sec-WebSocket-Version": "13", + "Sec-WebSocket-Key": "dGhlIHNhbXBsZSBub25jZQ==", + }) + if status != http.StatusForbidden || !strings.Contains(body, "Demo mode enabled") { + t.Fatalf("demo write with websocket upgrade headers status = %d, want %d demo block: %s", status, http.StatusForbidden, body) + } + if !mock.IsMockEnabled() { + t.Fatal("demo write with websocket upgrade headers switched mock mode off") + } + + wsHeaders := wsHeadersForHTTP(t, server.URL) + wsHeaders.Set("Cookie", sessionCookies) + conn, resp, err := websocket.DefaultDialer.Dial(wsURLForHTTP(server.URL)+"/ws", wsHeaders) + if err != nil { + status := 0 + if resp != nil { + status = resp.StatusCode + } + t.Fatalf("demo session websocket handshake failed: %v (status %d)", err, status) + } + defer conn.Close() + if resp.StatusCode != http.StatusSwitchingProtocols { + t.Fatalf("demo session websocket handshake status = %d, want %d", resp.StatusCode, http.StatusSwitchingProtocols) + } +} + func TestContract_ReleaseDemoFixtureRuntimeGuardrailsRemainCanonical(t *testing.T) { requiredSnippets := map[string][]string{ "licensing_handlers.go": { diff --git a/internal/api/demo_middleware.go b/internal/api/demo_middleware.go index 7b55a8bf7c..3584529b27 100644 --- a/internal/api/demo_middleware.go +++ b/internal/api/demo_middleware.go @@ -33,18 +33,15 @@ func DemoModeMiddleware(cfg *config.Config, next http.Handler) http.Handler { } } - // Allow GET and HEAD requests (read-only) + // Allow GET and HEAD requests (read-only). WebSocket handshakes are GET + // requests (RFC 6455 section 4.1), so /ws and /api/agent/ws connect + // through this branch. An Upgrade header never exempts another method: + // a POST carrying "Upgrade: websocket" is still a write. if r.Method == http.MethodGet || r.Method == http.MethodHead || r.Method == http.MethodOptions { next.ServeHTTP(w, r) return } - // Allow WebSocket upgrades - if strings.ToLower(r.Header.Get("Upgrade")) == "websocket" { - next.ServeHTTP(w, r) - return - } - // Allow authentication endpoints (login is read-only - verifies credentials) authPaths := []string{ "/api/login", diff --git a/internal/api/demo_middleware_test.go b/internal/api/demo_middleware_test.go index 3ed38b032f..b4afd8aa97 100644 --- a/internal/api/demo_middleware_test.go +++ b/internal/api/demo_middleware_test.go @@ -4,8 +4,11 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "strings" "testing" + "time" + "github.com/gorilla/websocket" "github.com/rcourtman/pulse-go-rewrite/internal/config" ) @@ -44,8 +47,8 @@ func TestDemoModeMiddleware(t *testing.T) { {"demo on websocket GET", true, http.MethodGet, "/api/ws", "websocket", true, http.StatusOK, true}, {"demo on websocket case insensitive", true, http.MethodGet, "/api/ws", "WebSocket", true, http.StatusOK, true}, {"demo on websocket uppercase", true, http.MethodGet, "/api/ws", "WEBSOCKET", true, http.StatusOK, true}, - // WebSocket upgrade with POST method (tests websocket branch after GET/HEAD/OPTIONS check) - {"demo on websocket POST", true, http.MethodPost, "/api/ws", "websocket", true, http.StatusOK, true}, + // An Upgrade header does not turn a write into a websocket handshake + {"demo on websocket POST", true, http.MethodPost, "/api/ws", "websocket", false, http.StatusForbidden, true}, // Demo mode enabled - auth endpoints allowed (POST) {"demo on login", true, http.MethodPost, "/api/login", "", true, http.StatusOK, true}, @@ -172,3 +175,79 @@ func TestDemoModeMiddleware_BlockedResponse(t *testing.T) { t.Error("message should not be empty") } } + +// A write that carries websocket handshake headers is still a write. The demo +// guard used to wave through any request with "Upgrade: websocket", so a demo +// session could switch mock mode off or change settings by adding two headers. +func TestDemoModeMiddlewareUpgradeHeadersDoNotExemptWrites(t *testing.T) { + handler := DemoModeMiddleware(&config.Config{DemoMode: true}, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + t.Errorf("write with websocket upgrade headers reached the handler: %s %s", r.Method, r.URL.Path) + })) + + methods := []string{http.MethodPost, http.MethodPut, http.MethodPatch, http.MethodDelete} + paths := []string{"/api/system/mock-mode", "/api/system/settings/update", "/api/orgs", "/ws", "/api/agent/ws"} + for _, method := range methods { + for _, path := range paths { + req := httptest.NewRequest(method, path, strings.NewReader(`{"enabled":false}`)) + req.Header.Set("Connection", "Upgrade") + req.Header.Set("Upgrade", "websocket") + req.Header.Set("Sec-WebSocket-Version", "13") + req.Header.Set("Sec-WebSocket-Key", "dGhlIHNhbXBsZSBub25jZQ==") + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + if rec.Code != http.StatusForbidden { + t.Errorf("%s %s with websocket upgrade headers: status = %d, want %d", method, path, rec.Code, http.StatusForbidden) + } + } + } +} + +func TestDemoModeMiddlewareAllowsWebSocketHandshake(t *testing.T) { + upgrader := websocket.Upgrader{CheckOrigin: func(*http.Request) bool { return true }} + echo := func(w http.ResponseWriter, r *http.Request) { + conn, err := upgrader.Upgrade(w, r, nil) + if err != nil { + return + } + defer conn.Close() + messageType, payload, err := conn.ReadMessage() + if err != nil { + return + } + _ = conn.WriteMessage(messageType, payload) + } + mux := http.NewServeMux() + mux.HandleFunc("/ws", echo) + mux.HandleFunc("/api/agent/ws", echo) + + server := newIPv4HTTPServer(t, DemoModeMiddleware(&config.Config{DemoMode: true}, mux)) + defer server.Close() + + for _, path := range []string{"/ws", "/api/agent/ws"} { + conn, resp, err := websocket.DefaultDialer.Dial(wsURLForHTTP(server.URL)+path, nil) + if err != nil { + status := 0 + if resp != nil { + status = resp.StatusCode + } + t.Fatalf("websocket handshake to %s failed in demo mode: %v (status %d)", path, err, status) + } + if resp.StatusCode != http.StatusSwitchingProtocols { + conn.Close() + t.Fatalf("websocket handshake to %s: status = %d, want %d", path, resp.StatusCode, http.StatusSwitchingProtocols) + } + if err := conn.SetReadDeadline(time.Now().Add(5 * time.Second)); err != nil { + conn.Close() + t.Fatalf("set read deadline for %s: %v", path, err) + } + if err := conn.WriteMessage(websocket.TextMessage, []byte("ping")); err != nil { + conn.Close() + t.Fatalf("write to %s: %v", path, err) + } + _, payload, err := conn.ReadMessage() + conn.Close() + if err != nil || string(payload) != "ping" { + t.Fatalf("echo from %s = %q, %v; want %q", path, payload, err, "ping") + } + } +}