mirror of
https://github.com/rcourtman/Pulse.git
synced 2026-10-24 12:42:49 +00:00
Require POST for Assistant session mutation routes (#2670)
The session router dispatches `/api/ai/sessions/{id}/abort`,
`summarize`, `fork`, `undo` and `redo` by path alone, and none of those
handlers checked the HTTP method, so a GET or HEAD cancelled a running
turn, compacted or rewound a transcript, or persisted a fork.
- **Demo mode:** `DemoModeMiddleware` admits every GET/HEAD as a read,
so a public-demo session could mutate Assistant sessions despite the
read-only guard.
- **CSRF:** `CheckCSRF` skips GET/HEAD, and session cookies default to
SameSite=Lax, which browsers send on cross-site top-level GET
navigations. A link carrying a session ID could therefore trigger these
mutations for a signed-in user.
## Change
- `requireAssistantSessionMutationMethod` in
`internal/api/ai_handler.go`: abort, summarize, fork, undo, redo and
steer admit only POST and answer anything else with `405` + `Allow:
POST` before resolving the Assistant service. Steer already refused
non-POST but sent no `Allow` header; it now uses the same guard.
- The guard sits in the handlers, matching how chat, steer and the
approval approve/deny handlers already own their method contracts.
- `messages` (GET) and the unsupported `diff`/`revert`/`unrevert` routes
are unchanged. Session rename/delete and the collection route already
dispatch on the method.
- Callers: `frontend-modern/src/api/aiChat.ts` uses POST for all six;
pulse-mobile's abort call and its OTA compatibility probe use POST.
## Proof
- `TestAssistantSessionMutationsRequirePost`: GET/HEAD/PUT/DELETE get
405 + `Allow: POST` for every mutation, with no mutation call and no
`IsRunning` check reaching the service. POST still dispatches.
- `TestAssistantSessionReadSubresourcesKeepTheirMethods`: GET `messages`
returns 200 and GET `diff` returns 501, with no mutation either way.
- `TestDemoModeRouterRejectsAssistantSessionMutationsOverSafeMethods`:
drives `NewRouter(...).Handler()` with `DemoMode` on. GET/HEAD to every
mutation pass the demo guard and get 405, POST stays 403, nothing
mutates, and GET `messages` still reads.
- Negative controls via `go test -overlay`: on main's `ai_handler.go`,
every non-POST case fails and the demo test reports `GET abort status =
200`. With abort's guard moved after the `IsRunning` check, the ordering
assertion fails.
This commit is contained in:
@@ -4712,6 +4712,12 @@ agent-lifecycle operations. If those routes are called directly, the API must
|
||||
fail them as unsupported rather than presenting file diffs or reverts as
|
||||
agent command rollback, enrollment repair, update rollback, or fleet-control
|
||||
authority.
|
||||
Assistant session mutations (`/abort`, `/summarize`, `/fork`, `/undo`,
|
||||
`/redo`, `/steer` under `/api/ai/sessions/{id}`) accept only `POST`; any other
|
||||
method that reaches those handlers gets `405` before touching session state.
|
||||
That method guard
|
||||
is AI-runtime transport hardening in `internal/api/ai_handler.go`; it adds no
|
||||
agent command, enrollment, update, or fleet-control path.
|
||||
That same shared dependency now also assumes hosted cloud handoff authorizes
|
||||
tenant org access before browser lifecycle continues. Lifecycle-adjacent opens
|
||||
into hosted workspaces may depend on `internal/api/cloud_handoff_handlers.go`,
|
||||
|
||||
@@ -3556,6 +3556,12 @@ query...`, and `Reading storage...` before streamed tool arguments are
|
||||
answer (nothing streaming, no error block, no pending approval or
|
||||
question, and a user prompt available to re-send) because the undo
|
||||
boundary only operates on the last durable turn.
|
||||
Session mutations stay POST-only at the handler: abort, summarize, fork,
|
||||
undo, redo, and steer refuse every other method with `405` and
|
||||
`Allow: POST` before resolving the Assistant service. A `GET` or `HEAD`
|
||||
that the demo-mode read-only guard or the CSRF check admits as a read
|
||||
therefore cannot cancel a running turn, compact or rewind a transcript,
|
||||
or persist a fork.
|
||||
The empty Assistant drawer may surface recent non-empty sessions as direct
|
||||
resume actions using the backend session list already owned by the drawer;
|
||||
it must not create a parallel recent-chat store or product-authored prompt
|
||||
|
||||
@@ -2090,6 +2090,20 @@ payload shape change when the portal presents compact client rows.
|
||||
index; clients must treat the endpoint's `accepted:true` as inbox
|
||||
receipt, not delivery. Browser clients must use the shared
|
||||
`AIChatAPI.steerSession(sessionId, { prompt, clientMessageId })` helper.
|
||||
Every Assistant session mutation sub-resource under
|
||||
`/api/ai/sessions/{id}` (`/abort`, `/summarize`, `/fork`, `/undo`,
|
||||
`/redo`, `/steer`) accepts only `POST`. Any other method that reaches the
|
||||
handler returns `405` with `Allow: POST` through
|
||||
`requireAssistantSessionMutationMethod` in `internal/api/ai_handler.go`,
|
||||
before the session service is resolved (earlier layers still answer
|
||||
first: `Router.ServeHTTP` returns `200` for `OPTIONS` preflight, and
|
||||
auth, scope, CSRF, and demo-mode guards keep their own refusals). The
|
||||
session router dispatches these sub-resources by path alone, and the
|
||||
demo-mode read-only guard and the CSRF check both treat `GET` and `HEAD`
|
||||
as reads, so the method contract belongs to the mutating handler rather
|
||||
than to its callers. `GET /api/ai/sessions/{id}/messages` stays the read
|
||||
path, and the unsupported `/diff`, `/revert`, and `/unrevert` handlers
|
||||
keep answering `501` for every method that reaches them.
|
||||
OpenCode-style file diff/revert session routes are deliberately not part
|
||||
of Pulse's supported Assistant session contract: Pulse sessions do not own
|
||||
local code-file edits, and infrastructure mutations must be reviewed
|
||||
|
||||
@@ -1413,6 +1413,12 @@ state only. A restored prompt or restored message count may help an operator
|
||||
continue a protected-item investigation, but it must not become backup coverage
|
||||
evidence, recovery freshness, restore entitlement, storage-owner identity,
|
||||
approval policy, or a provider-local recovery command handoff.
|
||||
The POST-only guard on Assistant session mutations (`/abort`,
|
||||
`/summarize`, `/fork`, `/undo`, `/redo`, `/steer`; `405` with `Allow: POST`
|
||||
for any other method that reaches the handler) is transport hardening in
|
||||
`internal/api/ai_handler.go`.
|
||||
It opens no storage, backup, or recovery read or write path, and a rejected
|
||||
request never reaches session state.
|
||||
Approved Assistant tool execution through `internal/api/router_routes_ai_relay.go`
|
||||
is also adjacent API/AI action plumbing for storage/recovery consumers.
|
||||
`AssistantToolExecutor` / `ApprovedAssistantToolExecutor` may execute an already
|
||||
|
||||
@@ -2971,8 +2971,25 @@ func (h *AIHandler) HandleMessages(w http.ResponseWriter, r *http.Request, sessi
|
||||
json.NewEncoder(w).Encode(messages)
|
||||
}
|
||||
|
||||
// requireAssistantSessionMutationMethod admits only POST to an Assistant
|
||||
// session mutation (abort, summarize, fork, undo, redo, steer). The session
|
||||
// router dispatches these sub-resources by path alone, and GET/HEAD pass both
|
||||
// the demo-mode read-only guard and the CSRF check while SameSite=Lax cookies
|
||||
// ride cross-site top-level GET navigations, so each mutation owns its method.
|
||||
func requireAssistantSessionMutationMethod(w http.ResponseWriter, r *http.Request) bool {
|
||||
if r.Method == http.MethodPost {
|
||||
return true
|
||||
}
|
||||
w.Header().Set("Allow", http.MethodPost)
|
||||
http.Error(w, "Method not allowed", http.StatusMethodNotAllowed)
|
||||
return false
|
||||
}
|
||||
|
||||
// HandleAbort handles POST /api/ai/sessions/{id}/abort
|
||||
func (h *AIHandler) HandleAbort(w http.ResponseWriter, r *http.Request, sessionID string) {
|
||||
if !requireAssistantSessionMutationMethod(w, r) {
|
||||
return
|
||||
}
|
||||
ctx := r.Context()
|
||||
if !h.IsRunning(ctx) {
|
||||
http.Error(w, "Pulse Assistant is not running", http.StatusServiceUnavailable)
|
||||
@@ -3187,6 +3204,9 @@ func (h *AIHandler) recordFirstPartyWorkflowPromptActivity(ctx context.Context,
|
||||
// HandleSummarize handles POST /api/ai/sessions/{id}/summarize
|
||||
// Compresses context when nearing model limits
|
||||
func (h *AIHandler) HandleSummarize(w http.ResponseWriter, r *http.Request, sessionID string) {
|
||||
if !requireAssistantSessionMutationMethod(w, r) {
|
||||
return
|
||||
}
|
||||
ctx := r.Context()
|
||||
if !h.IsRunning(ctx) {
|
||||
http.Error(w, "Pulse Assistant is not running", http.StatusServiceUnavailable)
|
||||
@@ -3218,6 +3238,9 @@ func (h *AIHandler) HandleDiff(w http.ResponseWriter, r *http.Request, sessionID
|
||||
// HandleFork handles POST /api/ai/sessions/{id}/fork
|
||||
// Creates a branch point in the conversation
|
||||
func (h *AIHandler) HandleFork(w http.ResponseWriter, r *http.Request, sessionID string) {
|
||||
if !requireAssistantSessionMutationMethod(w, r) {
|
||||
return
|
||||
}
|
||||
ctx := r.Context()
|
||||
if !h.IsRunning(ctx) {
|
||||
http.Error(w, "Pulse Assistant is not running", http.StatusServiceUnavailable)
|
||||
@@ -3244,6 +3267,9 @@ func (h *AIHandler) HandleFork(w http.ResponseWriter, r *http.Request, sessionID
|
||||
// The body is optional; retry/regenerate sends SessionTurnUndoOptions with the
|
||||
// prompt being re-run so a stale retry can never remove a different turn.
|
||||
func (h *AIHandler) HandleUndoLastTurn(w http.ResponseWriter, r *http.Request, sessionID string) {
|
||||
if !requireAssistantSessionMutationMethod(w, r) {
|
||||
return
|
||||
}
|
||||
ctx := r.Context()
|
||||
if !h.IsRunning(ctx) {
|
||||
http.Error(w, "Pulse Assistant is not running", http.StatusServiceUnavailable)
|
||||
@@ -3276,6 +3302,9 @@ func (h *AIHandler) HandleUndoLastTurn(w http.ResponseWriter, r *http.Request, s
|
||||
|
||||
// HandleRedoLastTurn handles POST /api/ai/sessions/{id}/redo.
|
||||
func (h *AIHandler) HandleRedoLastTurn(w http.ResponseWriter, r *http.Request, sessionID string) {
|
||||
if !requireAssistantSessionMutationMethod(w, r) {
|
||||
return
|
||||
}
|
||||
ctx := r.Context()
|
||||
if !h.IsRunning(ctx) {
|
||||
http.Error(w, "Pulse Assistant is not running", http.StatusServiceUnavailable)
|
||||
@@ -3303,8 +3332,7 @@ func (h *AIHandler) HandleRedoLastTurn(w http.ResponseWriter, r *http.Request, s
|
||||
// loop; accepted=false with a reason is a normal outcome and the client
|
||||
// falls back to the ordinary follow-up queue.
|
||||
func (h *AIHandler) HandleSteerSession(w http.ResponseWriter, r *http.Request, sessionID string) {
|
||||
if r.Method != http.MethodPost {
|
||||
http.Error(w, "Method not allowed", http.StatusMethodNotAllowed)
|
||||
if !requireAssistantSessionMutationMethod(w, r) {
|
||||
return
|
||||
}
|
||||
ctx := r.Context()
|
||||
|
||||
@@ -7,6 +7,7 @@ import (
|
||||
"net/http/httptest"
|
||||
"reflect"
|
||||
"strings"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
"unsafe"
|
||||
@@ -14,6 +15,7 @@ import (
|
||||
"github.com/gorilla/websocket"
|
||||
"github.com/rcourtman/pulse-go-rewrite/internal/agentexec"
|
||||
"github.com/rcourtman/pulse-go-rewrite/internal/ai"
|
||||
"github.com/rcourtman/pulse-go-rewrite/internal/ai/chat"
|
||||
"github.com/rcourtman/pulse-go-rewrite/internal/ai/cost"
|
||||
"github.com/rcourtman/pulse-go-rewrite/internal/ai/memory"
|
||||
"github.com/rcourtman/pulse-go-rewrite/internal/alerts"
|
||||
@@ -22,6 +24,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{}
|
||||
@@ -795,3 +798,204 @@ func TestHandleExportAICostHistoryCarriesPromptCacheBuckets(t *testing.T) {
|
||||
t.Fatalf("csv row missing cache buckets or cache-rate pricing: %s", lines[1])
|
||||
}
|
||||
}
|
||||
|
||||
// sessionMutationRecordingAIService records every Assistant session mutation
|
||||
// and every running check that reaches the service, so a rejected request can
|
||||
// be proven to stop before the handler touches the service at all.
|
||||
type sessionMutationRecordingAIService struct {
|
||||
capturingAIService
|
||||
mu sync.Mutex
|
||||
mutations []string
|
||||
runningCalls int
|
||||
}
|
||||
|
||||
func newSessionMutationRecordingAIService() *sessionMutationRecordingAIService {
|
||||
return &sessionMutationRecordingAIService{capturingAIService: capturingAIService{running: true}}
|
||||
}
|
||||
|
||||
func (s *sessionMutationRecordingAIService) record(op, sessionID string) {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
s.mutations = append(s.mutations, op+":"+sessionID)
|
||||
}
|
||||
|
||||
func (s *sessionMutationRecordingAIService) recorded() []string {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
return append([]string(nil), s.mutations...)
|
||||
}
|
||||
|
||||
func (s *sessionMutationRecordingAIService) IsRunning() bool {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
s.runningCalls++
|
||||
return s.capturingAIService.running
|
||||
}
|
||||
|
||||
func (s *sessionMutationRecordingAIService) runningChecks() int {
|
||||
s.mu.Lock()
|
||||
defer s.mu.Unlock()
|
||||
return s.runningCalls
|
||||
}
|
||||
|
||||
func (s *sessionMutationRecordingAIService) AbortSession(ctx context.Context, sessionID string) error {
|
||||
s.record("abort", sessionID)
|
||||
return nil
|
||||
}
|
||||
|
||||
func (s *sessionMutationRecordingAIService) SummarizeSession(ctx context.Context, sessionID string) (map[string]interface{}, error) {
|
||||
s.record("summarize", sessionID)
|
||||
return map[string]interface{}{"success": true}, nil
|
||||
}
|
||||
|
||||
func (s *sessionMutationRecordingAIService) ForkSession(ctx context.Context, sessionID string) (*chat.Session, error) {
|
||||
s.record("fork", sessionID)
|
||||
return &chat.Session{ID: sessionID + "-fork"}, nil
|
||||
}
|
||||
|
||||
func (s *sessionMutationRecordingAIService) UndoLastTurn(ctx context.Context, sessionID string, opts chat.SessionTurnUndoOptions) (*chat.SessionTurnUndoResult, error) {
|
||||
s.record("undo", sessionID)
|
||||
return &chat.SessionTurnUndoResult{Success: true, SessionID: sessionID}, nil
|
||||
}
|
||||
|
||||
func (s *sessionMutationRecordingAIService) RedoLastTurn(ctx context.Context, sessionID string) (*chat.SessionTurnRedoResult, error) {
|
||||
s.record("redo", sessionID)
|
||||
return &chat.SessionTurnRedoResult{Success: true, SessionID: sessionID}, nil
|
||||
}
|
||||
|
||||
func (s *sessionMutationRecordingAIService) SteerSession(ctx context.Context, sessionID string, req chat.SessionSteerRequest) (*chat.SessionSteerResult, error) {
|
||||
s.record("steer", sessionID)
|
||||
return &chat.SessionSteerResult{Accepted: true, SessionID: sessionID}, nil
|
||||
}
|
||||
|
||||
var assistantSessionMutationSubresources = []string{"abort", "summarize", "fork", "undo", "redo", "steer"}
|
||||
|
||||
func TestAssistantSessionMutationsRequirePost(t *testing.T) {
|
||||
for _, sub := range assistantSessionMutationSubresources {
|
||||
for _, method := range []string{http.MethodGet, http.MethodHead, http.MethodPut, http.MethodDelete} {
|
||||
t.Run(sub+"/"+method, func(t *testing.T) {
|
||||
svc := newSessionMutationRecordingAIService()
|
||||
handler := &AIHandler{}
|
||||
setUnexportedField(t, handler, "defaultService", svc)
|
||||
router := &Router{aiHandler: handler}
|
||||
|
||||
req := httptest.NewRequest(method, "/api/ai/sessions/session-1/"+sub, nil)
|
||||
rec := httptest.NewRecorder()
|
||||
router.routeAISessions(rec, req)
|
||||
|
||||
if rec.Code != http.StatusMethodNotAllowed {
|
||||
t.Fatalf("%s %s status = %d, want %d", method, sub, rec.Code, http.StatusMethodNotAllowed)
|
||||
}
|
||||
if allow := rec.Header().Get("Allow"); allow != http.MethodPost {
|
||||
t.Fatalf("%s %s Allow = %q, want %q", method, sub, allow, http.MethodPost)
|
||||
}
|
||||
if got := svc.recorded(); len(got) != 0 {
|
||||
t.Fatalf("%s %s reached the service: %v", method, sub, got)
|
||||
}
|
||||
if checks := svc.runningChecks(); checks != 0 {
|
||||
t.Fatalf("%s %s resolved the service (%d running checks) before rejecting the method", method, sub, checks)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
t.Run(sub+"/POST", func(t *testing.T) {
|
||||
svc := newSessionMutationRecordingAIService()
|
||||
handler := &AIHandler{}
|
||||
setUnexportedField(t, handler, "defaultService", svc)
|
||||
router := &Router{aiHandler: handler}
|
||||
|
||||
body := ""
|
||||
if sub == "steer" {
|
||||
body = `{"prompt":"check the backup job too"}`
|
||||
}
|
||||
req := httptest.NewRequest(http.MethodPost, "/api/ai/sessions/session-1/"+sub, strings.NewReader(body))
|
||||
rec := httptest.NewRecorder()
|
||||
router.routeAISessions(rec, req)
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("POST %s status = %d, want %d: %s", sub, rec.Code, http.StatusOK, rec.Body.String())
|
||||
}
|
||||
if got := svc.recorded(); !reflect.DeepEqual(got, []string{sub + ":session-1"}) {
|
||||
t.Fatalf("POST %s mutations = %v, want [%s:session-1]", sub, got, sub)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestAssistantSessionReadSubresourcesKeepTheirMethods(t *testing.T) {
|
||||
svc := newSessionMutationRecordingAIService()
|
||||
handler := &AIHandler{}
|
||||
setUnexportedField(t, handler, "defaultService", svc)
|
||||
router := &Router{aiHandler: handler}
|
||||
|
||||
messagesReq := httptest.NewRequest(http.MethodGet, "/api/ai/sessions/session-1/messages", nil)
|
||||
messagesRec := httptest.NewRecorder()
|
||||
router.routeAISessions(messagesRec, messagesReq)
|
||||
if messagesRec.Code != http.StatusOK {
|
||||
t.Fatalf("GET messages status = %d, want %d: %s", messagesRec.Code, http.StatusOK, messagesRec.Body.String())
|
||||
}
|
||||
|
||||
diffReq := httptest.NewRequest(http.MethodGet, "/api/ai/sessions/session-1/diff", nil)
|
||||
diffRec := httptest.NewRecorder()
|
||||
router.routeAISessions(diffRec, diffReq)
|
||||
if diffRec.Code != http.StatusNotImplemented {
|
||||
t.Fatalf("GET diff status = %d, want %d: %s", diffRec.Code, http.StatusNotImplemented, diffRec.Body.String())
|
||||
}
|
||||
|
||||
if got := svc.recorded(); len(got) != 0 {
|
||||
t.Fatalf("read sub-resources mutated the session: %v", got)
|
||||
}
|
||||
}
|
||||
|
||||
// TestDemoModeRouterRejectsAssistantSessionMutationsOverSafeMethods drives the
|
||||
// production handler chain: the demo-mode guard admits GET and HEAD as reads,
|
||||
// so the session mutation itself must refuse them.
|
||||
func TestDemoModeRouterRejectsAssistantSessionMutationsOverSafeMethods(t *testing.T) {
|
||||
tempDir := t.TempDir()
|
||||
hashed, err := internalauth.HashPassword("Password!1")
|
||||
if err != nil {
|
||||
t.Fatalf("hash password: %v", err)
|
||||
}
|
||||
cfg := &config.Config{
|
||||
DataPath: tempDir,
|
||||
ConfigPath: tempDir,
|
||||
AuthUser: "admin",
|
||||
AuthPass: hashed,
|
||||
DemoMode: true,
|
||||
}
|
||||
router := NewRouter(cfg, nil, nil, nil, nil, "1.0.0")
|
||||
t.Cleanup(router.shutdownBackgroundWorkers)
|
||||
svc := newSessionMutationRecordingAIService()
|
||||
setUnexportedField(t, router.aiHandler, "defaultService", svc)
|
||||
|
||||
serve := func(method, path string) *httptest.ResponseRecorder {
|
||||
t.Helper()
|
||||
req := httptest.NewRequest(method, path, nil)
|
||||
req.SetBasicAuth("admin", "Password!1")
|
||||
rec := httptest.NewRecorder()
|
||||
router.Handler().ServeHTTP(rec, req)
|
||||
return rec
|
||||
}
|
||||
|
||||
for _, sub := range assistantSessionMutationSubresources {
|
||||
for _, method := range []string{http.MethodGet, http.MethodHead} {
|
||||
rec := serve(method, "/api/ai/sessions/session-1/"+sub)
|
||||
if rec.Header().Get("X-Demo-Mode") != "true" {
|
||||
t.Fatalf("%s %s did not pass through the demo-mode guard", method, sub)
|
||||
}
|
||||
if rec.Code != http.StatusMethodNotAllowed {
|
||||
t.Fatalf("demo %s %s status = %d, want %d: %s", method, sub, rec.Code, http.StatusMethodNotAllowed, rec.Body.String())
|
||||
}
|
||||
}
|
||||
if rec := serve(http.MethodPost, "/api/ai/sessions/session-1/"+sub); rec.Code != http.StatusForbidden {
|
||||
t.Fatalf("demo POST %s status = %d, want %d: %s", sub, rec.Code, http.StatusForbidden, rec.Body.String())
|
||||
}
|
||||
}
|
||||
if got := svc.recorded(); len(got) != 0 {
|
||||
t.Fatalf("demo-mode requests mutated Assistant sessions: %v", got)
|
||||
}
|
||||
|
||||
if rec := serve(http.MethodGet, "/api/ai/sessions/session-1/messages"); rec.Code != http.StatusOK {
|
||||
t.Fatalf("demo GET messages status = %d, want %d: %s", rec.Code, http.StatusOK, rec.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user