fix(api): answer a hidden execution like a missing one on approve, cancel and revoke

internal/api/coverage_extras_test.go

@@ -87,6 +87,8 @@ func TestHandler_approvePurchase_PurchaseError(t *testing.T) {
 
 	mockAuth.On("GetAllowedAccountsAPI", mock.Anything, mock.Anything).Return(nil, nil).Maybe()
 	mockAuth.On("ValidateSession", ctx, "sess-tok").Return(&Session{Email: approver}, nil)
+	// CSRF is validated before the execution lookup (issue #597).
+	mockAuth.On("ValidateCSRFToken", ctx, "sess-tok", "").Return(nil)
 	// After issue #286, approvePurchase is session-first: with a Bearer
 	// header present the dispatch consults the approve-{any,own} RBAC
 	// matrix BEFORE falling through to the token branch. The session
@@ -160,6 +162,8 @@ func TestHandler_cancelPurchase_PurchaseError(t *testing.T) {
 
 	mockAuth.On("GetAllowedAccountsAPI", mock.Anything, mock.Anything).Return(nil, nil).Maybe()
 	mockAuth.On("ValidateSession", ctx, "sess-tok").Return(&Session{Email: approver}, nil)
+	// CSRF is validated before the execution lookup (issue #597).
+	mockAuth.On("ValidateCSRFToken", ctx, "sess-tok", "").Return(nil)
 	// Session has no admin role / cancel permissions → cancelPurchase's
 	// session-authed pre-check falls through to authorizeApprovalAction →
 	// purchase manager invocation, which is what this test's

internal/api/executed_notification_flow_test.go

@@ -94,7 +94,7 @@ func TestExecutedNotification_TokenApprovePath(t *testing.T) {
 		},
 	}
 
-	// First call: loadApproveExecution fetches the pending execution.
+	// First call: getExecutionOr404 fetches the pending execution.
 	// Second call: approveViaToken re-fetches after ApproveExecution returns,
 	// to pick up the FINAL execution state (status, completed_at) for the
 	// email body -- not the token, which comes from the return value below.
@@ -108,6 +108,8 @@ func TestExecutedNotification_TokenApprovePath(t *testing.T) {
 
 	mockAuth.On("GetAllowedAccountsAPI", mock.Anything, mock.Anything).Return(nil, nil).Maybe()
 	mockAuth.On("ValidateSession", ctx, "sess-tok").Return(&Session{Email: contact}, nil)
+	// CSRF is validated before the execution lookup (issue #597).
+	mockAuth.On("ValidateCSRFToken", ctx, "sess-tok", "").Return(nil)
 	mockAuth.On("HasPermissionAPI", ctx, "", "approve-any", "purchases").Return(false, nil).Maybe()
 	mockAuth.On("HasPermissionAPI", ctx, "", "approve-own", "purchases").Return(false, nil).Maybe()
 
@@ -164,7 +166,7 @@ func TestExecutedNotification_TokenApprovePath_RefetchFailureStillEmailsToken(t
 	mockConfig := new(MockConfigStore)
 	exec := approvalTestExec(execID, contact, mockConfig)
 
-	// First call: loadApproveExecution fetches the pending execution.
+	// First call: getExecutionOr404 fetches the pending execution.
 	// Second call: approveViaToken re-fetches for the final email state,
 	// but this time the store errors -- the handler must fall back to the
 	// pre-approve snapshot for those fields, and still emails the token.
@@ -179,6 +181,8 @@ func TestExecutedNotification_TokenApprovePath_RefetchFailureStillEmailsToken(t
 
 	mockAuth.On("GetAllowedAccountsAPI", mock.Anything, mock.Anything).Return(nil, nil).Maybe()
 	mockAuth.On("ValidateSession", ctx, "sess-tok").Return(&Session{Email: contact}, nil)
+	// CSRF is validated before the execution lookup (issue #597).
+	mockAuth.On("ValidateCSRFToken", ctx, "sess-tok", "").Return(nil)
 	mockAuth.On("HasPermissionAPI", ctx, "", "approve-any", "purchases").Return(false, nil).Maybe()
 	mockAuth.On("HasPermissionAPI", ctx, "", "approve-own", "purchases").Return(false, nil).Maybe()
 

internal/api/execution_scope_test.go

@@ -246,7 +246,7 @@ func TestExecutionEndpoints_ScopeMatrix(t *testing.T) {
 
 					if !sc.allow {
 						require.Error(t, err)
-						assert.True(t, IsNotFoundError(err), "expected 404 not-found, got %v", err)
+						assert.True(t, isNotFound404(err), "expected 404 not-found, got %v", err)
 						// A denied call reaches no mutation of any endpoint.
 						store.AssertNotCalled(t, "SavePurchaseExecution")
 						store.AssertNotCalled(t, "TransitionExecutionStatus")
@@ -301,11 +301,23 @@ func TestExecutionEndpoints_OutOfScopeNoTokenIs404(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			h := newScopeTestHandler(t, scopeTestExecution(adhoc, tc.status), []string{"Other"}, new(MockConfigStore), new(MockPurchaseManager))
 			_, err := tc.call(h, scopeTestRequest())
-			assert.True(t, IsNotFoundError(err), "expected 404 not-found, got %v", err)
+			assert.True(t, isNotFound404(err), "expected 404 not-found, got %v", err)
 		})
 	}
 }
 
+// isNotFound404 accepts either form of the enumeration-safe 404: errNotFound
+// from the scope gate, or the "execution not found" ClientError the approve,
+// cancel and revoke routes answer for both a missing and a hidden execution
+// (issue #597).
+func isNotFound404(err error) bool {
+	if IsNotFoundError(err) {
+		return true
+	}
+	ce, ok := IsClientError(err)
+	return ok && ce.code == 404 && ce.message == "execution not found"
+}
+
 // TestRevoke_AuthorizedNonRevocableStatusIs409: moving the status check
 // behind authorization must not drop it. An authorized caller, via the
 // session or the email token, still gets the 409 for a status with nothing

internal/api/handler_purchases.go

@@ -616,25 +616,20 @@ func (h *Handler) disablePlan(ctx context.Context, planID string) error {
 	return nil
 }
 
-// loadApproveExecution fetches an execution by ID, returns a 404 ClientError
-// when not found, and then runs the issue-#609 orphan preflight check.
-// Extracted from approvePurchase to keep that function below the gocyclo
-// threshold — same pattern as loadCancelableExecution in the purchase package.
-func (h *Handler) loadApproveExecution(ctx context.Context, execID string) (*config.PurchaseExecution, error) {
-	execution, err := h.config.GetExecutionByID(ctx, execID)
-	if errors.Is(err, config.ErrNotFound) {
-		return nil, NewClientError(404, "execution not found")
-	}
-	if err != nil {
-		return nil, fmt.Errorf("failed to get execution: %w", err)
-	}
-	// Preflight (issue #609): reject non-AWS orphan executions before the
-	// cloud SDK is reached. Delegates to the centralized predicate in the
-	// purchase package so the logic is maintained in one place.
+// rejectOrphanExecution runs the issue-#609 orphan preflight: non-AWS
+// executions whose account no longer exists are rejected with a 409 before
+// the cloud SDK is reached. Delegates to the centralized predicate in the
+// purchase package so the logic is maintained in one place. It runs only
+// once the session is authorized for the execution, so the 409 cannot tell
+// an unauthorized caller that the execution exists (issue #597). The token
+// branch needs no copy: an orphan has no per-account contact_email, so
+// authorizeApprovalAction never authorizes it, and ApproveExecution runs
+// the same predicate itself.
+func rejectOrphanExecution(execution *config.PurchaseExecution) error {
 	if err := purchase.OrphanExecutionError(execution); err != nil {
-		return nil, NewClientError(409, err.Error())
+		return NewClientError(409, err.Error())
 	}
-	return execution, nil
+	return nil
 }
 
 // Purchase action handlers.
@@ -647,7 +642,7 @@ func (h *Handler) approvePurchase(ctx context.Context, req *events.LambdaFunctio
 		return nil, err
 	}
 
-	execution, err := h.loadApproveExecution(ctx, execID)
+	execution, err := h.getExecutionOr404(ctx, execID)
 	if err != nil {
 		return nil, err
 	}
@@ -665,6 +660,7 @@ func (h *Handler) approvePurchase(ctx context.Context, req *events.LambdaFunctio
 	//      validates the token itself before mutating state.
 	//   3. token == "" → a session denial above is final (issue #173);
 	//      with no session, approvePurchaseViaSession returns 401.
+	// A final denial answers exactly like a missing execution (issue #597).
 	if session := h.tryGetSession(ctx, req); session != nil {
 		switch err := h.authorizeScopedSessionApprove(ctx, session, execution); {
 		case err == nil:
@@ -675,8 +671,9 @@ func (h *Handler) approvePurchase(ctx context.Context, req *events.LambdaFunctio
 			// chance (a logged-in user without approve-* or outside the
 			// account scope may still be the per-account contact recipient).
 		default:
-			// Transient failure — propagate instead of silently widening.
-			return nil, err
+			// A final denial (no token) or a transient failure — propagate
+			// instead of silently widening; the denial is concealed.
+			return nil, concealExecutionDenial(err)
 		}
 	}
 
@@ -692,13 +689,59 @@ func (h *Handler) approvePurchase(ctx context.Context, req *events.LambdaFunctio
 // approve/cancel/revoke routes needs a session (the token alone never
 // authorizes), so an unauthenticated caller gets the same response for a
 // missing and an existing execution ID (issue #435).
+//
+// CSRF is validated here too, before the lookup (issue #597). Every
+// remaining caller is session-authenticated, and the session bearer token
+// is cookie-equivalent, so the token branch needs CSRF as much as the
+// session branch does. Checking it only on the session branch, after the
+// lookup, answered "CSRF validation failed" for an existing execution and
+// 404 for a missing one.
 func (h *Handler) requireSessionBeforeLookup(ctx context.Context, req *events.LambdaFunctionURLRequest) error {
 	if h.tryGetSession(ctx, req) == nil {
 		return NewClientError(401, "sign in with the account's contact email to approve or cancel this purchase")
 	}
+	if err := h.validateCSRF(ctx, req); err != nil {
+		return errCSRFRejected
+	}
 	return nil
 }
 
+// executionNotFound is the answer for an execution ID that does not exist
+// on the approve, cancel and revoke routes. concealExecutionDenial returns
+// the same error for an execution the caller may not act on, so the two
+// cases have the same status, headers and body (issue #597).
+func executionNotFound() *clientError {
+	return &clientError{code: 404, message: "execution not found"}
+}
+
+// concealedDenialError is the "execution not found" 404 returned in place of
+// a final authorization denial. It responds exactly like executionNotFound
+// (IsClientError resolves it through Unwrap) and keeps the denial it hides
+// in cause, for logs and tests only.
+type concealedDenialError struct {
+	*clientError
+	cause error
+}
+
+func (e *concealedDenialError) Unwrap() error { return e.clientError }
+
+// concealExecutionDenial maps a final authorization denial on an existing
+// execution (an account-scope miss, an RBAC or contact_email 403, or a 401
+// from the contact_email gate) to the 404 a missing execution gets, so a
+// signed-in caller cannot use the approve, cancel and revoke routes to
+// learn which execution IDs exist (issue #597). Only direct *clientError
+// values are mapped: a Constraints denial (constraintDeniedError) goes to a
+// caller who already holds approve-* for the execution, and wrapped
+// backend failures must keep surfacing as 500s.
+func concealExecutionDenial(err error) error {
+	ce, ok := err.(*clientError) //nolint:errorlint // strict (unwrapped) assertion is deliberate; see isPermissionDenied
+	if !errors.Is(err, errNotFound) && (!ok || (ce.code != 401 && ce.code != 403)) {
+		return err
+	}
+	logging.Infof("purchase action denied, answered as execution not found: %v", err)
+	return &concealedDenialError{clientError: executionNotFound(), cause: err}
+}
+
 // tokenActionError maps the email-link approve/cancel failures that are the
 // caller's fault to a 4xx: a wrong token is 403 (as on the RI exchange
 // approve path), an expired token is 410, and an execution
@@ -725,7 +768,7 @@ func tokenActionError(err error) error {
 func (h *Handler) approveViaToken(ctx context.Context, req *events.LambdaFunctionURLRequest, execution *config.PurchaseExecution, token string) (any, error) {
 	actor, err := h.authorizeApprovalAction(ctx, req, execution)
 	if err != nil {
-		return nil, err
+		return nil, concealExecutionDenial(err)
 	}
 	// 4-eyes mode (issue #1005): resolve the session again for identity
 	// comparison. On the email-token deep-link flow the user is logged in
@@ -832,6 +875,12 @@ func (h *Handler) approvePurchaseViaSession(ctx context.Context, req *events.Lam
 		return nil, err
 	}
 
+	// approvePurchase only calls this with a session it has authorized, so
+	// the orphan 409 reveals nothing to an unauthorized caller (issue #597).
+	if err := rejectOrphanExecution(execution); err != nil {
+		return nil, err
+	}
+
 	if execution.Status != "pending" && execution.Status != "notified" {
 		return nil, NewClientError(409, fmt.Sprintf("execution %s cannot be approved (status=%s)", execution.ExecutionID, execution.Status))
 	}
@@ -1351,15 +1400,16 @@ func (h *Handler) cancelPurchase(ctx context.Context, req *events.LambdaFunction
 			// returning a wrapped error, h.auth==nil 500). Propagate
 			// instead of silently widening to the contact_email gate —
 			// a stale auth backend should not mask itself as a 403 about
-			// missing contact emails. CR feedback on PR #216.
-			return nil, err
+			// missing contact emails. CR feedback on PR #216. A final
+			// denial answers like a missing execution (issue #597).
+			return nil, concealExecutionDenial(err)
 		}
 	}
 
 	if token != "" {
 		actor, err := h.authorizeApprovalAction(ctx, req, execution)
 		if err != nil {
-			return nil, err
+			return nil, concealExecutionDenial(err)
 		}
 		if err := h.purchase.CancelExecution(ctx, execID, token, actor); err != nil {
 			return nil, tokenActionError(err)
@@ -1554,7 +1604,7 @@ func (h *Handler) revokeViaEmailToken(ctx context.Context, req *events.LambdaFun
 	// this iteration) and confirm the caller is the authorized contact email.
 	actor, err := h.authorizeApprovalAction(ctx, req, execution)
 	if err != nil {
-		return nil, err
+		return nil, concealExecutionDenial(err)
 	}
 	if err := validateRevokeToken(execution, token); err != nil {
 		return nil, err
@@ -1570,7 +1620,7 @@ func (h *Handler) revokeViaEmailToken(ctx context.Context, req *events.LambdaFun
 func (h *Handler) getExecutionOr404(ctx context.Context, execID string) (*config.PurchaseExecution, error) {
 	execution, err := h.config.GetExecutionByID(ctx, execID)
 	if errors.Is(err, config.ErrNotFound) || (err == nil && execution == nil) {
-		return nil, NewClientError(404, "execution not found")
+		return nil, executionNotFound()
 	}
 	if err != nil {
 		return nil, fmt.Errorf("failed to get execution: %w", err)
@@ -1633,7 +1683,8 @@ func (h *Handler) tryRevokeViaSession(ctx context.Context, req *events.LambdaFun
 		// Fall through to the token branch.
 		return nil, false, nil
 	default:
-		return nil, true, sessErr
+		// A final denial answers like a missing execution (issue #597).
+		return nil, true, concealExecutionDenial(sessErr)
 	}
 }