Refuse GET and HEAD on discovery settings and SAML logout

Two routes registered without a mux method pattern changed state on any
method. The demo-mode read-only guard and the CSRF check both admit GET and
HEAD as reads, and the SameSite=Lax session cookie rides cross-site
top-level GET navigations, so each was writable by a safe method:

- /api/discovery/settings had no method check, so a GET or HEAD carrying a
  JSON body changed the discovery staleness threshold, including on a
  DEMO_MODE instance.
- SP-initiated SAML logout at /api/saml/{id}/logout cleared the session and
  redirected to the IdP on GET. Nothing in the UI calls it, the /api/logout
  fallback it delegates to is already POST-only, and the IdP's
  LogoutResponse returns to /slo, not here.

Add requireRequestMethod (405 with Allow) as the shared handler-level
guard. Discovery settings keeps PUT, its documented method, and POST, which
it accepted before; SAML logout takes POST only. An audit of every route in
internal/api found no other caller-requested write reachable by GET or HEAD
beyond the Assistant session routes guarded in #2670. GET flows the
protocol requires (OIDC login and callback, SAML login initiation and SLO,
magic-link and cloud handoff, checkout bridges) and reads that run lazy
initialization or idempotent reconciliation are left as they are. One
known exception is deferred: single-resource report generation still runs
on GET and can call the configured AI narrator, which records provider
usage in the cost ledger; moving it behind POST changes the report API the
UI downloads from, so it is tracked separately.

docs/release-control/v6/internal/subsystems/agent-lifecycle.md

@@ -8927,6 +8927,15 @@ 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.
 
+### Safe methods refused on discovery settings and SAML logout
+
+`internal/api/method_guard.go`, `internal/api/discovery_handlers.go` and
+`internal/api/saml_handlers.go` changed only so `/api/discovery/settings` and
+`/api/saml/{id}/logout` answer `405` to every method outside their allowed set,
+`GET` and `HEAD` included. The settings write still takes `PUT` or `POST` and
+logout still takes `POST`. No agent registration, enrolment, install, update or
+removal path changed.
+
 ### Demo write guard ignores websocket upgrade headers
 
 `internal/api/demo_middleware.go` changed only so a `POST`, `PUT`, `PATCH` or

docs/release-control/v6/internal/subsystems/api-contracts.md

@@ -3896,6 +3896,13 @@ counters exist to measure.
     onboarding telemetry. Customer diagnostics may expose runtime health,
     supportability, and sanitized troubleshooting state; admin analytics must
     stay behind admin-owned metrics routes.
+25. Handlers that perform a caller-requested write (settings, sessions,
+    credentials, workflow or resource state) while answering `GET` or `HEAD`.
+    A route registered without a mux method pattern, or a sub-resource
+    dispatched by path, must refuse safe methods in the mutating handler
+    through `requireRequestMethod` in `internal/api/method_guard.go` (or an
+    equivalent explicit check) instead of relying on its callers, the
+    demo-mode read-only guard, or the CSRF check.
 
 ## Completion Obligations
 
@@ -5136,6 +5143,33 @@ pass the bounded non-symlink regular-file boundary in
 `internal/api/sso_outbound.go`. Cloud handoff redirects consume the shared
 host-local redirect validator rather than carrying route-local prefix checks.
 
+A handler that performs a caller-requested write behind a route whose mux
+pattern carries no method must refuse `GET` and `HEAD` itself.
+`requireRequestMethod` in `internal/api/method_guard.go` is the shared
+handler-level guard: it admits only the listed methods and answers any other
+method that reaches the handler with `405` and an `Allow` header naming them.
+The demo-mode read-only guard and the CSRF check both admit `GET` and `HEAD` as
+reads, and the `SameSite=Lax` session cookie rides cross-site top-level `GET`
+navigations, so the method contract belongs to the mutating handler rather than
+to its callers. `/api/discovery/settings` accepts `PUT`, its documented method,
+and `POST`, which it took before the guard existed. SP-initiated SAML logout at
+`/api/saml/{id}/logout` accepts only `POST`, matching the `/api/logout`
+fallback it delegates to. The IdP's LogoutResponse keeps returning to
+`/api/saml/{id}/slo`, which must accept the HTTP-Redirect binding's `GET` and
+validates the signed response before it touches the session.
+`TestDemoModeSafeMethodsCannotReachRouteMutations` in
+`internal/api/ai_handlers_more_test.go` drives both routes through the full
+`DEMO_MODE` router and proves the threshold and the session survive.
+The rule does not cover `GET` flows the protocol or the browser handoff
+requires (OIDC login and callback, SAML login initiation and SLO, magic-link
+verify, cloud handoff, the checkout start and activation bridges, websocket
+handshakes), or reads that run one-time lazy initialization or idempotent
+reconciliation, such as the license and onboarding-overflow bootstrap behind
+the entitlement reads and the action-expiry sweep behind the action reads.
+Single-resource report generation at `GET /api/admin/reports/generate` still
+runs the configured AI narrator, a paid provider call recorded in the cost
+ledger; that is a known open exception, not part of this contract.
+
 Alert delivery diagnosis is a read-only monitoring API contract.
 `GET /api/alerts/delivery-diagnosis?alertIdentifier=<id>` returns the alert
 manager's current delivery-policy projection for one active alert, including

docs/release-control/v6/internal/subsystems/storage-recovery.md

@@ -6867,6 +6867,15 @@ 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.
 
+### Safe methods refused on discovery settings and SAML logout
+
+`internal/api/method_guard.go` adds `requireRequestMethod`, and
+`internal/api/discovery_handlers.go` and `internal/api/saml_handlers.go` use it
+so `/api/discovery/settings` (`PUT` or `POST`) and `/api/saml/{id}/logout`
+(`POST`) answer `405` to any other method, `GET` and `HEAD` included, instead
+of changing the discovery staleness threshold or clearing a session. No
+storage, retention, backup, migration or recovery path is added or moved.
+
 ### Demo write guard ignores websocket upgrade headers
 
 `internal/api/demo_middleware.go` no longer exempts every request carrying

internal/api/ai_handlers_more_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/rcourtman/pulse-go-rewrite/internal/monitoring"
 	"github.com/rcourtman/pulse-go-rewrite/internal/servicediscovery"
 	unifiedresources "github.com/rcourtman/pulse-go-rewrite/internal/unifiedresources"
+	internalauth "github.com/rcourtman/pulse-go-rewrite/pkg/auth"
 )
 
 type stubMetadataProvider struct{}
@@ -521,6 +522,78 @@ func TestAISettingsHandler_DiscoveryStoreAccessors(t *testing.T) {
 	}
 }
 
+// In DEMO_MODE the demo middleware admits every GET and HEAD as a read, and
+// the CSRF check skips them, so a mutating handler reachable by a safe method
+// is a write any demo visitor can make. These routes dispatch without a mux
+// method pattern, so each handler must refuse GET and HEAD itself.
+func TestDemoModeSafeMethodsCannotReachRouteMutations(t *testing.T) {
+	dataPath := t.TempDir()
+	hashed, err := internalauth.HashPassword("demo-router-test-password")
+	if err != nil {
+		t.Fatalf("HashPassword: %v", err)
+	}
+	cfg := &config.Config{
+		DataPath:   dataPath,
+		ConfigPath: dataPath,
+		DemoMode:   true,
+		AuthUser:   "demo",
+		AuthPass:   hashed,
+	}
+	router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0")
+	cleanupTestRouter(t, router)
+	handler := router.Handler()
+
+	discovery := servicediscovery.NewService(nil, nil, servicediscovery.DefaultConfig())
+	router.SetDiscoveryService(discovery)
+	ageBefore := discovery.GetMaxDiscoveryAge()
+
+	router.samlManager.services["okta"] = newTestSAMLService(t, "okta", `<?xml version="1.0"?>
+<EntityDescriptor xmlns="urn:oasis:names:tc:SAML:2.0:metadata" entityID="idp">
+  <IDPSSODescriptor protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">
+    <SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="https://idp.example.com/sso"/>
+    <SingleLogoutService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="https://idp.example.com/slo"/>
+  </IDPSSODescriptor>
+</EntityDescriptor>`)
+	samlSession := generateSessionToken()
+	GetSessionStore().CreateSAMLSession(samlSession, time.Hour, "agent", "127.0.0.1", "demo", &SAMLTokenInfo{
+		ProviderID:   "okta",
+		NameID:       "name-id",
+		SessionIndex: "sess-1",
+	})
+
+	cases := []struct {
+		path  string
+		body  string
+		allow string
+	}{
+		{path: "/api/discovery/settings", body: `{"max_discovery_age_days":10}`, allow: "PUT, POST"},
+		{path: "/api/saml/okta/logout", allow: http.MethodPost},
+	}
+	for _, tc := range cases {
+		for _, method := range []string{http.MethodGet, http.MethodHead} {
+			req := httptest.NewRequest(method, tc.path, strings.NewReader(tc.body))
+			req.SetBasicAuth("demo", "demo-router-test-password")
+			req.AddCookie(&http.Cookie{Name: "pulse_session", Value: samlSession})
+			rec := httptest.NewRecorder()
+			handler.ServeHTTP(rec, req)
+
+			if rec.Code != http.StatusMethodNotAllowed {
+				t.Fatalf("%s %s status = %d, want %d: %s", method, tc.path, rec.Code, http.StatusMethodNotAllowed, rec.Body.String())
+			}
+			if allow := rec.Header().Get("Allow"); allow != tc.allow {
+				t.Fatalf("%s %s Allow = %q, want %q", method, tc.path, allow, tc.allow)
+			}
+		}
+	}
+
+	if got := discovery.GetMaxDiscoveryAge(); got != ageBefore {
+		t.Fatalf("safe methods changed the max discovery age: %s -> %s", ageBefore, got)
+	}
+	if !ValidateSession(samlSession) {
+		t.Fatal("safe methods cleared the SAML session")
+	}
+}
+
 func TestAISettingsHandler_GetConfig_NonDefaultFallsBackWhenMultiTenantUnavailable(t *testing.T) {
 	handler := newTestAISettingsHandler(&config.Config{APIToken: "token"}, config.NewConfigPersistence(t.TempDir()), nil)
 	ctx := context.WithValue(context.Background(), OrgIDContextKey, "acme")

internal/api/discovery_handlers.go

@@ -881,7 +881,12 @@ func (h *DiscoveryHandlers) HandleRunDiscovery(w http.ResponseWriter, r *http.Re
 
 // HandleUpdateSettings handles PUT /api/discovery/settings
 // Allows updating discovery settings like the staleness threshold.
+// PUT is the documented method. POST stays accepted because the route took any
+// method before; GET and HEAD are refused so a read can never write.
 func (h *DiscoveryHandlers) HandleUpdateSettings(w http.ResponseWriter, r *http.Request) {
+	if !requireRequestMethod(w, r, http.MethodPut, http.MethodPost) {
+		return
+	}
 	if h.service == nil {
 		writeDiscoveryError(w, http.StatusServiceUnavailable, "discovery service not configured")
 		return

internal/api/discovery_handlers_test.go

@@ -476,6 +476,41 @@ func TestHandleUpdateSettings(t *testing.T) {
 	// We can't check service private field easily, but we check 200 OK.
 }
 
+// GET and HEAD pass the demo-mode read-only guard and the CSRF check, so the
+// settings write must refuse them even when they carry a JSON body.
+func TestHandleUpdateSettingsRefusesSafeMethods(t *testing.T) {
+	h, service, _ := setupDiscoveryHandlers(t)
+	before := service.GetMaxDiscoveryAge()
+	reqBody := `{"max_discovery_age_days": 10}`
+	if before == 10*24*time.Hour {
+		t.Fatal("test body must change the default max discovery age")
+	}
+
+	for _, method := range []string{http.MethodGet, http.MethodHead} {
+		req := httptest.NewRequest(method, "/api/discovery/settings", bytes.NewBufferString(reqBody))
+		req.SetBasicAuth("admin", "admin")
+		w := httptest.NewRecorder()
+
+		h.HandleUpdateSettings(w, req)
+
+		assert.Equal(t, http.StatusMethodNotAllowed, w.Code, method)
+		assert.Equal(t, "PUT, POST", w.Header().Get("Allow"), method)
+		assert.Equal(t, before, service.GetMaxDiscoveryAge(), method)
+	}
+
+	for _, method := range []string{http.MethodPut, http.MethodPost} {
+		service.SetMaxDiscoveryAge(before)
+		req := httptest.NewRequest(method, "/api/discovery/settings", bytes.NewBufferString(reqBody))
+		req.SetBasicAuth("admin", "admin")
+		w := httptest.NewRecorder()
+
+		h.HandleUpdateSettings(w, req)
+
+		assert.Equal(t, http.StatusOK, w.Code, method)
+		assert.Equal(t, 10*24*time.Hour, service.GetMaxDiscoveryAge(), method)
+	}
+}
+
 func TestHandleListByType(t *testing.T) {
 	h, _, store := setupDiscoveryHandlers(t)
 

internal/api/method_guard.go

@@ -0,0 +1,23 @@
+package api
+
+import (
+	"net/http"
+	"strings"
+)
+
+// requireRequestMethod admits only the listed methods to a handler that
+// changes state, answering anything else with 405 and an Allow header. Bare
+// mux patterns and path-dispatched routes do not filter methods, and GET/HEAD
+// pass both the demo-mode read-only guard and the CSRF check while
+// SameSite=Lax session cookies ride cross-site top-level GET navigations, so a
+// mutating handler must refuse safe methods itself.
+func requireRequestMethod(w http.ResponseWriter, r *http.Request, allowed ...string) bool {
+	for _, method := range allowed {
+		if r.Method == method {
+			return true
+		}
+	}
+	w.Header().Set("Allow", strings.Join(allowed, ", "))
+	http.Error(w, "Method not allowed", http.StatusMethodNotAllowed)
+	return false
+}

internal/api/saml_handlers.go

@@ -377,8 +377,13 @@ func (r *Router) handleSAMLMetadata(w http.ResponseWriter, req *http.Request) {
 	w.Write(metadata)
 }
 
-// handleSAMLLogout handles SAML Single Logout
+// handleSAMLLogout handles SP-initiated SAML Single Logout. It is POST-only,
+// like the /api/logout fallback: the IdP's LogoutResponse returns to /slo, not
+// here, so a GET to this path could only be a forced logout.
 func (r *Router) handleSAMLLogout(w http.ResponseWriter, req *http.Request) {
+	if !requireRequestMethod(w, req, http.MethodPost) {
+		return
+	}
 	providerID := extractSAMLProviderID(req.URL.Path, "logout")
 	if providerID == "" {
 		// Fall back to regular logout

internal/api/saml_handlers_more_test.go

@@ -247,7 +247,9 @@ func TestHandleSAMLSLO_RejectsIDPInitiatedLogoutRequest(t *testing.T) {
 }
 
 func TestHandleSAMLLogout_SLOUnavailable(t *testing.T) {
-	InitSessionStore(t.TempDir())
+	dataPath := t.TempDir()
+	InitSessionStore(dataPath)
+	InitCSRFStore(dataPath)
 
 	router := &Router{samlManager: NewSAMLServiceManager("https://pulse.example.com")}
 	metadataXML := `<?xml version="1.0"?>
@@ -265,7 +267,7 @@ func TestHandleSAMLLogout_SLOUnavailable(t *testing.T) {
 		SessionIndex: "sess-1",
 	})
 
-	req := httptest.NewRequest(http.MethodGet, "/api/saml/okta/logout", nil)
+	req := httptest.NewRequest(http.MethodPost, "/api/saml/okta/logout", nil)
 	req.AddCookie(&http.Cookie{Name: "pulse_session", Value: token})
 	rec := httptest.NewRecorder()
 
@@ -280,7 +282,9 @@ func TestHandleSAMLLogout_SLOUnavailable(t *testing.T) {
 }
 
 func TestHandleSAMLLogout_SLOSuccess(t *testing.T) {
-	InitSessionStore(t.TempDir())
+	dataPath := t.TempDir()
+	InitSessionStore(dataPath)
+	InitCSRFStore(dataPath)
 
 	router := &Router{samlManager: NewSAMLServiceManager("https://pulse.example.com")}
 	metadataXML := `<?xml version="1.0"?>
@@ -299,7 +303,7 @@ func TestHandleSAMLLogout_SLOSuccess(t *testing.T) {
 		SessionIndex: "sess-1",
 	})
 
-	req := httptest.NewRequest(http.MethodGet, "/api/saml/okta/logout", nil)
+	req := httptest.NewRequest(http.MethodPost, "/api/saml/okta/logout", nil)
 	req.AddCookie(&http.Cookie{Name: "pulse_session", Value: token})
 	rec := httptest.NewRecorder()
 
@@ -314,6 +318,64 @@ func TestHandleSAMLLogout_SLOSuccess(t *testing.T) {
 	}
 }
 
+// A cross-site top-level GET carries the SameSite=Lax session cookie and skips
+// the CSRF check, so SP-initiated logout must not clear the session on GET or
+// HEAD.
+func TestHandleSAMLLogout_RefusesSafeMethods(t *testing.T) {
+	dataPath := t.TempDir()
+	InitSessionStore(dataPath)
+	InitCSRFStore(dataPath)
+
+	router := &Router{samlManager: NewSAMLServiceManager("https://pulse.example.com")}
+	metadataXML := `<?xml version="1.0"?>
+<EntityDescriptor xmlns="urn:oasis:names:tc:SAML:2.0:metadata" entityID="idp">
+  <IDPSSODescriptor protocolSupportEnumeration="urn:oasis:names:tc:SAML:2.0:protocol">
+    <SingleSignOnService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="https://idp.example.com/sso"/>
+    <SingleLogoutService Binding="urn:oasis:names:tc:SAML:2.0:bindings:HTTP-Redirect" Location="https://idp.example.com/slo"/>
+  </IDPSSODescriptor>
+</EntityDescriptor>`
+	router.samlManager.services["okta"] = newTestSAMLService(t, "okta", metadataXML)
+
+	token := generateSessionToken()
+	GetSessionStore().CreateSAMLSession(token, time.Hour, "agent", "127.0.0.1", "user", &SAMLTokenInfo{
+		ProviderID:   "okta",
+		NameID:       "name-id",
+		SessionIndex: "sess-1",
+	})
+
+	for _, method := range []string{http.MethodGet, http.MethodHead} {
+		req := httptest.NewRequest(method, "/api/saml/okta/logout", nil)
+		req.AddCookie(&http.Cookie{Name: "pulse_session", Value: token})
+		rec := httptest.NewRecorder()
+
+		router.handleSAMLLogout(rec, req)
+
+		if rec.Code != http.StatusMethodNotAllowed {
+			t.Fatalf("%s: expected status %d, got %d", method, http.StatusMethodNotAllowed, rec.Code)
+		}
+		if allow := rec.Header().Get("Allow"); allow != http.MethodPost {
+			t.Fatalf("%s: expected Allow %q, got %q", method, http.MethodPost, allow)
+		}
+		if loc := rec.Header().Get("Location"); loc != "" {
+			t.Fatalf("%s: expected no SLO redirect, got %q", method, loc)
+		}
+		if !ValidateSession(token) {
+			t.Fatalf("%s: SAML logout cleared the session", method)
+		}
+	}
+
+	req := httptest.NewRequest(http.MethodPost, "/api/saml/okta/logout", nil)
+	req.AddCookie(&http.Cookie{Name: "pulse_session", Value: token})
+	rec := httptest.NewRecorder()
+	router.handleSAMLLogout(rec, req)
+	if rec.Code != http.StatusFound {
+		t.Fatalf("POST: expected status %d, got %d", http.StatusFound, rec.Code)
+	}
+	if ValidateSession(token) {
+		t.Fatal("POST: expected SAML logout to clear the session")
+	}
+}
+
 func TestExtractSAMLProviderID(t *testing.T) {
 	if got := extractSAMLProviderID("/api/saml/okta/login", "login"); got != "okta" {
 		t.Fatalf("expected okta, got %q", got)

internal/api/saml_runtime_license_gating_test.go

@@ -26,7 +26,7 @@ func TestSAMLRuntimeEndpointsUseCommunitySSOFeature(t *testing.T) {
 		{http.MethodGet, "/api/saml/test-provider/login"},
 		{http.MethodPost, "/api/saml/test-provider/acs"},
 		{http.MethodGet, "/api/saml/test-provider/metadata"},
-		{http.MethodGet, "/api/saml/test-provider/logout"},
+		{http.MethodPost, "/api/saml/test-provider/logout"},
 		{http.MethodPost, "/api/saml/test-provider/slo"},
 	}