sec(api): enforce CSRF on the RI-exchange session-authed approve path

internal/api/handler_ri_exchange.go

@@ -2057,7 +2057,15 @@ func (h *Handler) approveRIExchange(ctx context.Context, req *events.LambdaFunct
 			}
 			// Record-level RBAC denied (e.g. approve-own user is not the creator).
 			// If a token is present, preserve legacy token flow; otherwise surface the error.
-			if token == "" || !isPermissionDenied(sessErr) {
+			//
+			// A CSRF rejection must NOT fall through, even with a token present.
+			// isPermissionDenied is a bare 403 test, so without this it cannot tell a
+			// forged cross-site request from a legitimate approve-own denial, and the
+			// request would proceed on the token alone: the exchange executes, but as
+			// an unattributed system approval (transitioned_by NULL, no approved_by
+			// stamp) on a money path. Checked first so the sentinel is never reached
+			// by the 403 test below.
+			if errors.Is(sessErr, errCSRFRejected) || token == "" || !isPermissionDenied(sessErr) {
 				return nil, sessErr
 			}
 		case isPermissionDenied(err):
@@ -2103,6 +2111,16 @@ func (h *Handler) approveRIExchangeViaToken(ctx context.Context, id, token strin
 // The session parameter may be non-nil (already validated by the caller) or nil
 // (requireSession will validate it and return 401 if absent).
 func (h *Handler) approveRIExchangeViaSession(ctx context.Context, req *events.LambdaFunctionURLRequest, id string, session *Session) (any, error) {
+	// This endpoint is AuthPublic so the outer middleware skips CSRF.
+	// Enforce it here for the session-authed sub-path, mirroring
+	// approvePurchaseViaSession / cancelPurchaseViaSession: the session
+	// bearer token is cookie-equivalent and must be CSRF-protected (issue
+	// #1757 -- this call was previously reachable with no CSRF token at
+	// all, despite a comment claiming parity with the purchases path).
+	if err := h.validateCSRF(ctx, req); err != nil {
+		return nil, errCSRFRejected
+	}
+
 	var err error
 	if session == nil {
 		session, err = h.requireSession(ctx, req)

internal/api/handler_ri_exchange_test.go

@@ -370,6 +370,10 @@ func TestApproveRIExchange_SessionAdmin(t *testing.T) {
 	adminSession := &Session{UserID: "admin-uuid", Email: "admin@example.com"}
 	mockAuth.On("ValidateSession", ctx, "admin-bearer").Return(adminSession, nil)
 	mockAuth.grantAdminPurchaser()
+	// approveRIExchangeViaSession validates CSRF before anything else (issue
+	// #1757); a valid token must be supplied for this session-authed path
+	// to reach the record fetch at all.
+	mockAuth.On("ValidateCSRFToken", ctx, "admin-bearer", "csrf-abc").Return(nil)
 
 	// authorizeSessionApproveRIExchange: admin role short-circuits (no HasPermissionAPI call)
 
@@ -398,7 +402,10 @@ func TestApproveRIExchange_SessionAdmin(t *testing.T) {
 	mockStore.On("StampRIExchangeApprovedBy", ctx, id, adminSession.Email).Return(nil)
 
 	req := &events.LambdaFunctionURLRequest{
-		Headers: map[string]string{"authorization": "Bearer admin-bearer"},
+		Headers: map[string]string{
+			"authorization": "Bearer admin-bearer",
+			"x-csrf-token":  "csrf-abc",
+		},
 	}
 	_, err := h.approveRIExchange(ctx, req, id, "")
 	require.NoError(t, err)
@@ -427,6 +434,8 @@ func TestApproveRIExchange_SessionApproveOwn(t *testing.T) {
 		mockAuth.On("ValidateSession", ctx, "owner-bearer").Return(ownerSession, nil)
 		mockAuth.On("HasPermissionAPI", ctx, ownerID, auth.ActionApproveAny, auth.ResourcePurchases).Return(false, nil)
 		mockAuth.On("HasPermissionAPI", ctx, ownerID, auth.ActionApproveOwn, auth.ResourcePurchases).Return(true, nil)
+		// Issue #1757: CSRF must be validated before the record fetch.
+		mockAuth.On("ValidateCSRFToken", ctx, "owner-bearer", "csrf-abc").Return(nil)
 
 		// authorizeSessionApproveRIExchange fetches record for ownership check
 		mockStore.On("GetRIExchangeRecord", ctx, id).Return(&config.RIExchangeRecord{
@@ -449,7 +458,10 @@ func TestApproveRIExchange_SessionApproveOwn(t *testing.T) {
 		mockStore.On("StampRIExchangeApprovedBy", ctx, id, ownerSession.Email).Return(nil)
 
 		req := &events.LambdaFunctionURLRequest{
-			Headers: map[string]string{"authorization": "Bearer owner-bearer"},
+			Headers: map[string]string{
+				"authorization": "Bearer owner-bearer",
+				"x-csrf-token":  "csrf-abc",
+			},
 		}
 		_, err := h.approveRIExchange(ctx, req, id, "")
 		require.NoError(t, err)
@@ -465,6 +477,9 @@ func TestApproveRIExchange_SessionApproveOwn(t *testing.T) {
 		mockAuth.On("ValidateSession", ctx, "owner-bearer").Return(ownerSession, nil)
 		mockAuth.On("HasPermissionAPI", ctx, ownerID, auth.ActionApproveAny, auth.ResourcePurchases).Return(false, nil)
 		mockAuth.On("HasPermissionAPI", ctx, ownerID, auth.ActionApproveOwn, auth.ResourcePurchases).Return(true, nil)
+		// Issue #1757: CSRF must be validated before the record fetch, so this
+		// test needs a valid token to reach the ownership check it targets.
+		mockAuth.On("ValidateCSRFToken", ctx, "owner-bearer", "csrf-abc").Return(nil)
 
 		// authorizeSessionApproveRIExchange fetches record — creator does not match
 		mockStore.On("GetRIExchangeRecord", ctx, id).Return(&config.RIExchangeRecord{
@@ -475,7 +490,10 @@ func TestApproveRIExchange_SessionApproveOwn(t *testing.T) {
 		}, nil)
 
 		req := &events.LambdaFunctionURLRequest{
-			Headers: map[string]string{"authorization": "Bearer owner-bearer"},
+			Headers: map[string]string{
+				"authorization": "Bearer owner-bearer",
+				"x-csrf-token":  "csrf-abc",
+			},
 		}
 		_, err := h.approveRIExchange(ctx, req, id, "")
 		require.Error(t, err)
@@ -1758,6 +1776,8 @@ func TestApproveRIExchange_SessionActorStamped(t *testing.T) {
 	adminSession := &Session{UserID: actorID, Email: "admin@example.com"}
 	mockAuth.On("ValidateSession", ctx, "admin-bearer").Return(adminSession, nil)
 	mockAuth.grantAdminPurchaser()
+	// Issue #1757: CSRF must be validated before the record fetch.
+	mockAuth.On("ValidateCSRFToken", ctx, "admin-bearer", "csrf-abc").Return(nil)
 
 	mockStore.On("GetRIExchangeRecord", ctx, id).Return(&config.RIExchangeRecord{
 		ID: id, Status: "pending", ApprovalToken: "tok", SourceRIIDs: []string{"ri-1"}, PaymentDue: "10.00",
@@ -1774,7 +1794,10 @@ func TestApproveRIExchange_SessionActorStamped(t *testing.T) {
 	mockStore.On("StampRIExchangeApprovedBy", ctx, id, adminSession.Email).Return(nil)
 
 	req := &events.LambdaFunctionURLRequest{
-		Headers: map[string]string{"authorization": "Bearer admin-bearer"},
+		Headers: map[string]string{
+			"authorization": "Bearer admin-bearer",
+			"x-csrf-token":  "csrf-abc",
+		},
 	}
 	_, err := (&Handler{config: mockStore, auth: mockAuth}).approveRIExchange(ctx, req, id, "")
 	require.NoError(t, err)

internal/api/handler_router.go

@@ -18,6 +18,14 @@ func (h *Handler) routeRequest(ctx context.Context, method, path string, req *ev
 // errNotFound is a sentinel error for 404 responses.
 var errNotFound = &notFoundError{}
 
+// errCSRFRejected marks a CSRF validation failure so it stays distinguishable
+// from an authorization denial. Both surface as 403, and isPermissionDenied
+// tests the code alone, so a dispatch that falls back to a token flow on 403
+// cannot otherwise tell a forged cross-site request from a legitimate
+// approve-own denial. Compare with errors.Is before any 403 test that decides
+// whether to continue rather than return (issue #1757).
+var errCSRFRejected = NewClientError(403, "CSRF validation failed")
+
 type notFoundError struct{}
 
 func (e *notFoundError) Error() string {

internal/api/middleware.go

@@ -287,6 +287,14 @@ func (h *Handler) requiresCSRFValidation(method, path string, req *events.Lambda
 	//
 	// Similarly, /api/ri-exchange/approve/ and /api/ri-exchange/reject/ are
 	// AuthPublic and therefore exempted by isPublicEndpoint(), not here.
+	// approve/ has a session-authed sub-path (approveRIExchangeViaSession)
+	// and, like the purchases functions above, enforces CSRF directly
+	// inside that function -- NOT here, and not automatically just because
+	// it is "similar" to purchases (issue #1757: this exemption used to
+	// exist in prose only, with no actual validateCSRF call backing it).
+	// reject/ has no session-authed path at all -- it is unconditionally
+	// gated on a constant-time comparison against the record's stored
+	// approval token, so CSRF does not apply to it.
 	//
 	// All exemptions use exact-match to prevent a path like
 	// /api/register-malicious from bypassing CSRF via accidental prefix overlap.