From e6940049147c1dd1da8a046d989f26a2a3269d9a Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Mon, 28 Sep 2026 05:49:55 +0200
Subject: [PATCH 1/4] sec(auth): enforce per-permission Constraints on
 execute-any/own, approve-any/own, runPlanned

SEC-01 (issue #1141, PR cli-monorepo#1210) enforced per-permission
Constraints (MaxPurchaseAmount, Providers, Services, Regions, AccountIDs)
on the base execute:purchases permission. The same fail-open class
remained on three sibling money-mutating call sites: the direct-execute
gate (authorizeSessionExecuteDirect, execute-any/execute-own), the
session-approve gate (authorizeSessionApprove, approve-any/approve-own,
which hands off to ApproveAndExecute), and runPlannedPurchase's
pending/paused -> running transition. Each ran only a bare
HasPermissionAPI verb check, so a session capped tighter on one of
these verbs than on its base execute:purchases permission could still
mutate past that cap.

requirePermissionConstraints previously hardcoded its action parameter
to "execute" since every existing caller checked exactly that action.
Threaded action through explicitly now that execute-any, execute-own,
approve-any and approve-own are real distinct values, and wired all
three new call sites to build the same purchaseConstraintSets already
used by the web execute path and re-check them against the verb that
actually authorized the mutation.

Regression tests mirror TestHandler_executePurchase_PermissionConstraintsDenied
for each new call site (execute-any, execute-own, approve-any,
approve-own), asserting a 403 before any state mutation or SDK call.
Verified with a standalone repro: a session with execute:purchases
(uncapped) and execute-any:purchases capped at $100 could direct-execute
a $50,000 purchase pre-fix; post-fix the same request is denied with a
403 naming the exceeded constraints.

Co-Authored-By: claude-flow <ruv@ruv.net>
---
 internal/api/grantadmin_carveout_test.go |   4 +-
 internal/api/handler.go                  |  39 +++--
 internal/api/handler_purchases.go        |  69 +++++++--
 internal/api/handler_purchases_test.go   | 183 ++++++++++++++++++++++-
 internal/api/handler_ri_exchange.go      |   6 +-
 internal/api/handler_test.go             |   7 +
 6 files changed, 265 insertions(+), 43 deletions(-)

diff --git a/internal/api/grantadmin_carveout_test.go b/internal/api/grantadmin_carveout_test.go
index 5b1ac9cc1..19215ceac 100644
--- a/internal/api/grantadmin_carveout_test.go
+++ b/internal/api/grantadmin_carveout_test.go
@@ -332,7 +332,7 @@ func TestRequirePermissionConstraints_ConstrainedGrantIsBoundedAtHandler(t *test
 	session := &Session{UserID: "u1"}
 
 	require.NoError(t,
-		h.requirePermissionConstraints(ctx, session, auth.ResourceRIExchange, []auth.PermissionConstraints{{
+		h.requirePermissionConstraints(ctx, session, auth.ActionExecute, auth.ResourceRIExchange, []auth.PermissionConstraints{{
 			Providers: []string{"aws"}, MaxPurchaseAmount: 500,
 		}}),
 		"a request inside every dimension must pass the gate")
@@ -345,7 +345,7 @@ func TestRequirePermissionConstraints_ConstrainedGrantIsBoundedAtHandler(t *test
 		{"over the spend cap", auth.PermissionConstraints{Providers: []string{"aws"}, MaxPurchaseAmount: 5000}},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
-			err := h.requirePermissionConstraints(ctx, session, auth.ResourceRIExchange,
+			err := h.requirePermissionConstraints(ctx, session, auth.ActionExecute, auth.ResourceRIExchange,
 				[]auth.PermissionConstraints{tc.request})
 			require.Error(t, err)
 			ce, ok := IsClientError(err)
diff --git a/internal/api/handler.go b/internal/api/handler.go
index a3c0ae379..52bac12cb 100644
--- a/internal/api/handler.go
+++ b/internal/api/handler.go
@@ -465,25 +465,24 @@ func (h *Handler) authorizeAPIKeyAny(ctx context.Context, apiKey string, verbs [
 // AccountIDs constraint still matches via the empty-permission-side rule.
 const unattributedAccountConstraint = "unattributed"
 
-// requirePermissionConstraintsAction is the action every current caller of
-// requirePermissionConstraints checks (execute:purchases, execute:ri-exchange
-// for both AWS and Azure). Every constraint-gated operation today is an
-// irreversible execute; hardcoded rather than threaded as a parameter since
-// a parameter with only one real value across all call sites is dead
-// flexibility (a genuinely new action should add a real parameter back,
-// not resurrect an unused one).
-const requirePermissionConstraintsAction = "execute"
-
 // requirePermissionConstraints re-checks an already-authenticated session
 // against request-derived permission constraint sets, so the Constraints
 // (MaxPurchaseAmount, Providers, Services, Regions, AccountIDs) configured on
 // the granting group permission are enforced at execution time (SEC-01,
-// issue #1141). Callers must have passed requirePermission for the same
-// action/resource first; this adds the constraint dimension once the request
-// body is parsed and validated. The stateless admin API key is a full-access
-// infrastructure credential with no user row, so it bypasses the check just
-// like it bypasses requirePermission's per-user lookup. Fails closed on a
-// missing auth service or a lookup error.
+// issue #1141). Callers must have passed requirePermission (or an equivalent
+// verb gate) for the same action/resource first; this adds the constraint
+// dimension once the request body is parsed and validated. The stateless
+// admin API key is a full-access infrastructure credential with no user row,
+// so it bypasses the check just like it bypasses requirePermission's
+// per-user lookup. Fails closed on a missing auth service or a lookup error.
+//
+// action is threaded explicitly (not hardcoded to "execute") because issue
+// #60 added callers on the approve-any/approve-own and execute-any/
+// execute-own verbs, which are distinct permissions from the base
+// execute:purchases permission the original SEC-01 fix covered: a session
+// can hold execute:purchases with one set of Constraints and execute-any:
+// purchases with a different (or absent) set, so the constraint lookup must
+// match the verb that actually authorized the mutation.
 //
 // For user-API-key sessions (session.UserAPIKeyID != ""), constraints are
 // evaluated against the KEY's effective permissions (the intersection of the
@@ -491,7 +490,7 @@ const requirePermissionConstraintsAction = "execute"
 // prevents a CI key with MaxPurchaseAmount=$100 from spending up to the
 // owning user's full group limit by inheriting the broader group permissions
 // (adversarial-review F2).
-func (h *Handler) requirePermissionConstraints(ctx context.Context, session *Session, resource string, constraintSets []auth.PermissionConstraints) error {
+func (h *Handler) requirePermissionConstraints(ctx context.Context, session *Session, action, resource string, constraintSets []auth.PermissionConstraints) error {
 	if session == nil {
 		return fmt.Errorf("internal error: nil session passed to requirePermissionConstraints")
 	}
@@ -504,21 +503,21 @@ func (h *Handler) requirePermissionConstraints(ctx context.Context, session *Ses
 	// User API key: evaluate constraints against the key's effective permissions,
 	// not the owning user's full group permissions.
 	if session.UserAPIKeyID != "" {
-		has, err := h.auth.HasAPIKeyPermissionForConstraintsAPI(ctx, session.UserAPIKeyID, session.UserID, requirePermissionConstraintsAction, resource, constraintSets)
+		has, err := h.auth.HasAPIKeyPermissionForConstraintsAPI(ctx, session.UserAPIKeyID, session.UserID, action, resource, constraintSets)
 		if err != nil {
 			return fmt.Errorf("permission constraint check failed: %w", err)
 		}
 		if !has {
-			return NewClientError(403, fmt.Sprintf("permission denied: this request exceeds the constraints configured on your %s permission for %s", requirePermissionConstraintsAction, resource))
+			return NewClientError(403, fmt.Sprintf("permission denied: this request exceeds the constraints configured on your %s permission for %s", action, resource))
 		}
 		return nil
 	}
-	has, err := h.auth.HasPermissionForConstraintsAPI(ctx, session.UserID, requirePermissionConstraintsAction, resource, constraintSets)
+	has, err := h.auth.HasPermissionForConstraintsAPI(ctx, session.UserID, action, resource, constraintSets)
 	if err != nil {
 		return fmt.Errorf("permission constraint check failed: %w", err)
 	}
 	if !has {
-		return NewClientError(403, fmt.Sprintf("permission denied: this request exceeds the constraints configured on your %s permission for %s", requirePermissionConstraintsAction, resource))
+		return NewClientError(403, fmt.Sprintf("permission denied: this request exceeds the constraints configured on your %s permission for %s", action, resource))
 	}
 	return nil
 }
diff --git a/internal/api/handler_purchases.go b/internal/api/handler_purchases.go
index df8957e3a..81ff4beaf 100644
--- a/internal/api/handler_purchases.go
+++ b/internal/api/handler_purchases.go
@@ -324,6 +324,30 @@ func (h *Handler) runPlannedPurchase(ctx context.Context, req *events.LambdaFunc
 		return nil, err
 	}
 
+	// Re-hydrate the persisted recommendations and enforce the session's
+	// execute:purchases Constraints against them before RunPlannedPurchaseNow
+	// fires the purchase (issue #60). requirePermission above only checked
+	// the bare execute:purchases verb: a session running a plan via
+	// update-any:purchases (which authorizeExecutionManagement lets bypass
+	// the creator check) would otherwise bypass any MaxPurchaseAmount/
+	// Providers/Services/Regions/AccountIDs cap configured on their own
+	// execute:purchases permission, exactly the fail-open class the web
+	// execute path (validateExecutePurchaseRequest) already closed for
+	// SEC-01 (#1141). Routed through the shared enforcePurchaseConstraints
+	// helper so a planned execution whose recs sum to $0 is refused by
+	// requireNonZeroCommitment rather than reading as an uncapped
+	// MaxPurchaseAmount (issue #60 review follow-up).
+	execution, fetchErr := h.config.GetExecutionByID(ctx, executionID)
+	if errors.Is(fetchErr, config.ErrNotFound) {
+		return nil, errNotFound
+	}
+	if fetchErr != nil {
+		return nil, fmt.Errorf("failed to get execution: %w", fetchErr)
+	}
+	if constraintErr := h.enforcePurchaseConstraints(ctx, session, "execute", execution.Recommendations); constraintErr != nil {
+		return nil, constraintErr
+	}
+
 	if err := h.purchase.RunPlannedPurchaseNow(ctx, executionID, fourEyesActorIdentity(session), resolveCreatorUserID(session)); err != nil {
 		return nil, NewClientError(409, fmt.Sprintf("execution %s cannot be started: %v", executionID, err))
 	}
@@ -745,6 +769,16 @@ func (h *Handler) approvePurchaseViaSession(ctx context.Context, req *events.Lam
 // approve the given execution under the approve-any / approve-own RBAC
 // rules added in issue #286. Returns a 403 ClientError otherwise.
 // Mirror of authorizeSessionCancel.
+//
+// Also enforces the approve-any/approve-own permission's own per-permission
+// Constraints against execution.Recommendations (issue #60): approving here
+// hands off to purchase.Manager.ApproveAndExecute, which fires the AWS
+// purchase synchronously, so this is as much a money-spending path as
+// execute-any/execute-own and must be capped the same way. Without this, a
+// session whose approve-any is constrained to e.g. MaxPurchaseAmount: $1,000
+// could approve (and thereby execute) an arbitrarily large pending execution
+// created by someone else, even though the identically-capped execute-any
+// permission already blocks a matching direct-execute.
 func (h *Handler) authorizeSessionApprove(ctx context.Context, session *Session, execution *config.PurchaseExecution) error {
 	// The stateless admin API key has full access and no user row to resolve
 	// permissions from. Administrators-group users fall through and pass via
@@ -762,7 +796,7 @@ func (h *Handler) authorizeSessionApprove(ctx context.Context, session *Session,
 		return fmt.Errorf("permission check failed: %w", err)
 	}
 	if hasAny {
-		return nil
+		return h.requirePermissionConstraints(ctx, session, auth.ActionApproveAny, auth.ResourcePurchases, purchaseConstraintSets(execution.Recommendations))
 	}
 
 	hasOwn, err := h.auth.HasPermissionAPI(ctx, session.UserID, auth.ActionApproveOwn, auth.ResourcePurchases)
@@ -776,7 +810,7 @@ func (h *Handler) authorizeSessionApprove(ctx context.Context, session *Session,
 	if execution.CreatedByUserID == nil || *execution.CreatedByUserID != session.UserID {
 		return NewClientError(403, "permission denied: cannot approve another user's pending purchase")
 	}
-	return nil
+	return h.requirePermissionConstraints(ctx, session, auth.ActionApproveOwn, auth.ResourcePurchases, purchaseConstraintSets(execution.Recommendations))
 }
 
 // approveWithDelay is the Gmail-style pre-fire delay branch (issue #291 wave-2).
@@ -984,16 +1018,25 @@ func (h *Handler) requireDifferentApprover(ctx context.Context, session *Session
 //
 // creatorID is the creator of the execution being submitted (resolved via
 // resolveCreatorUserID before this call; "" on non-human or legacy rows).
+// recs is the execution's (store-priced) recommendation set, used to build
+// the same per-permission Constraints sets validateExecutePurchaseRequest
+// already enforces against the base execute:purchases permission (issue
+// #60): execute-any/execute-own are DISTINCT permissions from execute, so a
+// session capped tighter on execute-any than on execute could otherwise
+// direct-execute past that tighter cap by holding both verbs.
 //
 // Gate logic (mirrors authorizeSessionApprove / authorizeSessionCancel):
 //   - stateless admin API key: always permitted (apiKeyAdminUserID sentinel).
-//   - execute-any: permitted regardless of creator. Administrators-group users
-//     pass here because {admin, *} matches ActionExecuteAny.
-//   - execute-own: permitted only when creatorID == session.UserID and both
-//     are non-empty (prevents an empty-string collision from granting access).
-//   - no matching grant: 403 fail-closed; nil auth component is a 500 as
-//     per feedback_fail_closed_middleware.md.
-func (h *Handler) authorizeSessionExecuteDirect(ctx context.Context, session *Session, creatorID string) error {
+//   - execute-any: permitted when the execute-any permission's own
+//     Constraints also allow this batch, regardless of creator.
+//     Administrators-group users pass the verb check here because
+//     {admin, *} matches ActionExecuteAny.
+//   - execute-own: permitted only when creatorID == session.UserID (both
+//     non-empty) AND the execute-own permission's own Constraints allow
+//     this batch.
+//   - no matching grant, or a constraint denial: 403 fail-closed; nil auth
+//     component is a 500 as per feedback_fail_closed_middleware.md.
+func (h *Handler) authorizeSessionExecuteDirect(ctx context.Context, session *Session, creatorID string, recs []config.RecommendationRecord) error {
 	// Stateless admin API key: full access, no user row. Administrators-group
 	// users pass via the execute-any HasPermissionAPI check below, since
 	// {admin, *} matches any requested permission.
@@ -1009,7 +1052,7 @@ func (h *Handler) authorizeSessionExecuteDirect(ctx context.Context, session *Se
 		return fmt.Errorf("permission check failed: %w", err)
 	}
 	if hasAny {
-		return nil
+		return h.requirePermissionConstraints(ctx, session, auth.ActionExecuteAny, auth.ResourcePurchases, purchaseConstraintSets(recs))
 	}
 
 	hasOwn, err := h.auth.HasPermissionAPI(ctx, session.UserID, auth.ActionExecuteOwn, auth.ResourcePurchases)
@@ -1025,7 +1068,7 @@ func (h *Handler) authorizeSessionExecuteDirect(ctx context.Context, session *Se
 	if session.UserID == "" || creatorID == "" || creatorID != session.UserID {
 		return NewClientError(403, "permission denied: execute-own requires you to be the creator of this purchase")
 	}
-	return nil
+	return h.requirePermissionConstraints(ctx, session, auth.ActionExecuteOwn, auth.ResourcePurchases, purchaseConstraintSets(recs))
 }
 
 func (h *Handler) cancelPurchase(ctx context.Context, req *events.LambdaFunctionURLRequest, execID, token string) (any, error) {
@@ -2216,7 +2259,7 @@ func (h *Handler) enforcePurchaseConstraints(ctx context.Context, session *Sessi
 	if err := requireNonZeroCommitment(constraintSets); err != nil {
 		return err
 	}
-	return h.requirePermissionConstraints(ctx, session, "purchases", constraintSets)
+	return h.requirePermissionConstraints(ctx, session, "execute", "purchases", constraintSets)
 }
 
 // purchaseConstraintSets builds one auth.PermissionConstraints per
@@ -2680,7 +2723,7 @@ func (h *Handler) executePurchase(ctx context.Context, req *events.LambdaFunctio
 	// prevents a client that sets execute_mode="direct" but only holds the
 	// base execute:purchases verb from silently degrading to the email flow.
 	if execReq.ExecuteMode == "direct" {
-		if err := h.authorizeSessionExecuteDirect(ctx, session, creatorID); err != nil {
+		if err := h.authorizeSessionExecuteDirect(ctx, session, creatorID, execReq.Recommendations); err != nil {
 			return nil, err
 		}
 		return h.directExecutePurchase(ctx, req, execution, session, paymentAdjustments)
diff --git a/internal/api/handler_purchases_test.go b/internal/api/handler_purchases_test.go
index 2009b7a5a..7592104c3 100644
--- a/internal/api/handler_purchases_test.go
+++ b/internal/api/handler_purchases_test.go
@@ -376,6 +376,99 @@ func TestHandler_approvePurchase_SessionExecuteFailureSurfacesAs409(t *testing.T
 	assert.Contains(t, ce.Error(), "could not be approved")
 }
 
+// TestHandler_approvePurchase_SessionApproveAny_PermissionConstraintsDenied
+// is the SEC-01 (#1141) follow-up regression test for issue #60: a session
+// whose approve-any:purchases verb is granted (the bare-verb gate passes,
+// exactly as it does for a permission that carries Constraints) but whose
+// per-permission Constraints reject the request must receive a 403 BEFORE
+// ApproveAndExecute fires the AWS purchase. Pre-fix, authorizeSessionApprove
+// never consulted the approve-any permission's own Constraints, so a
+// $50,000 execution created by someone else was approved (and executed) by
+// a session whose approve-any was configured with a $1,000 cap.
+func TestHandler_approvePurchase_SessionApproveAny_PermissionConstraintsDenied(t *testing.T) {
+	ctx := context.Background()
+	execID := "12345678-1234-1234-1234-123456789abc"
+	otherCreator := "creator-user-uuid"
+
+	mockConfig := new(MockConfigStore)
+	t.Cleanup(func() { mockConfig.AssertExpectations(t) })
+	exec := &config.PurchaseExecution{
+		ExecutionID:     execID,
+		ApprovalToken:   "valid-token",
+		Status:          "pending",
+		CreatedByUserID: &otherCreator,
+		Recommendations: []config.RecommendationRecord{
+			{Provider: "aws", Service: "ec2", Region: "us-east-1", UpfrontCost: 50000},
+		},
+	}
+	mockConfig.On("GetExecutionByID", ctx, execID).Return(exec, nil)
+
+	mockAuth := new(MockAuthService)
+	t.Cleanup(func() { mockAuth.AssertExpectations(t) })
+	userSession := &Session{UserID: "dddddddd-dddd-dddd-dddd-dddddddddddd", Email: "capped@example.com"}
+	mockAuth.On("ValidateSession", ctx, "sess-tok").Return(userSession, nil)
+	// The bare verb/resource gate passes - this is exactly what happens for
+	// a permission that carries Constraints, because HasPermissionAPI checks
+	// with nil request-side constraints.
+	mockAuth.On("HasPermissionAPI", ctx, userSession.UserID, "approve-any", "purchases").Return(true, nil)
+	mockAuth.On("HasPermissionForConstraintsAPI", ctx, userSession.UserID, "approve-any", "purchases",
+		purchaseConstraintSets(exec.Recommendations)).Return(false, nil)
+	// authorizeSessionApprove's constraint denial is a plain 403, which
+	// isPermissionDenied treats the same as a bare-verb 403 and falls
+	// through to approvePurchaseViaSession (token is empty in this test, so
+	// that is the terminal branch either way). That path enforces CSRF
+	// before re-running the same RBAC gate.
+	mockAuth.On("ValidateCSRFToken", ctx, "sess-tok", "").Return(nil)
+
+	mockPurchase := new(MockPurchaseManager)
+	t.Cleanup(func() { mockPurchase.AssertExpectations(t) })
+
+	handler := &Handler{purchase: mockPurchase, config: mockConfig, auth: mockAuth}
+	req := &events.LambdaFunctionURLRequest{Headers: map[string]string{"authorization": "Bearer sess-tok"}}
+	_, err := handler.approvePurchase(ctx, req, execID, "")
+	require.Error(t, err)
+	ce, ok := IsClientError(err)
+	require.True(t, ok, "expected a clientError, got: %v", err)
+	assert.Equal(t, 403, ce.code)
+	assert.Contains(t, ce.Error(), "constraints")
+	// The purchase must never fire.
+	mockPurchase.AssertNotCalled(t, "ApproveAndExecute", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+	mockPurchase.AssertNotCalled(t, "ApproveExecution", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+}
+
+// TestHandler_authorizeSessionApprove_ApproveOwn_ConstraintsDenied mirrors
+// the approve-any case above for approve-own:purchases (issue #60): the
+// creator-ownership check passes, but the approve-own permission's own
+// Constraints reject the request.
+func TestHandler_authorizeSessionApprove_ApproveOwn_ConstraintsDenied(t *testing.T) {
+	ctx := context.Background()
+	mockAuth := new(MockAuthService)
+	t.Cleanup(func() { mockAuth.AssertExpectations(t) })
+
+	userID := "cccccccc-cccc-cccc-cccc-cccccccccccc"
+	session := &Session{UserID: userID}
+	execution := &config.PurchaseExecution{
+		ExecutionID:     "exec-1",
+		CreatedByUserID: &userID,
+		Recommendations: []config.RecommendationRecord{
+			{Provider: "aws", Service: "ec2", Region: "us-east-1", UpfrontCost: 50000},
+		},
+	}
+
+	mockAuth.On("HasPermissionAPI", ctx, userID, "approve-any", "purchases").Return(false, nil)
+	mockAuth.On("HasPermissionAPI", ctx, userID, "approve-own", "purchases").Return(true, nil)
+	mockAuth.On("HasPermissionForConstraintsAPI", ctx, userID, "approve-own", "purchases",
+		purchaseConstraintSets(execution.Recommendations)).Return(false, nil)
+
+	handler := &Handler{auth: mockAuth}
+	err := handler.authorizeSessionApprove(ctx, session, execution)
+	require.Error(t, err)
+	ce, ok := IsClientError(err)
+	require.True(t, ok, "expected a clientError, got: %v", err)
+	assert.Equal(t, 403, ce.code)
+	assert.Contains(t, ce.Error(), "constraints")
+}
+
 // --- F3 regression: global-config read error must fail closed (not execute) ---
 
 // TestHandler_approveViaToken_GlobalConfigError_FailsClosed pins the F3 fix:
@@ -1311,6 +1404,13 @@ func TestHandler_runPlannedPurchase(t *testing.T) {
 
 	mockAuth.On("ValidateSession", ctx, "admin-token").Return(adminSession, nil)
 	mockAuth.grantAdminPurchaser()
+	// runPlannedPurchase re-hydrates the execution to enforce the session's
+	// execute:purchases Constraints against its recommendations (issue #60).
+	mockStore.On("GetExecutionByID", ctx, "11111111-1111-1111-1111-111111111111").Return(&config.PurchaseExecution{
+		ExecutionID:     "11111111-1111-1111-1111-111111111111",
+		Status:          "pending",
+		Recommendations: []config.RecommendationRecord{{Provider: "aws", Service: "ec2", Region: "us-east-1", UpfrontCost: 100}},
+	}, nil)
 	// runPlannedPurchase ("Run now") must delegate to the same 4-eyes-gated,
 	// CAS-guarded funnel ApproveAndExecute uses (issue #218) rather than a
 	// bare TransitionExecutionStatus flip to "running" that no executor
@@ -4325,7 +4425,9 @@ func TestHandler_authorizeSessionExecuteDirect_ExecuteOwn_NonOwner(t *testing.T)
 	mockAuth.On("HasPermissionAPI", ctx, sessionUserID, "execute-own", "purchases").Return(true, nil)
 
 	handler := &Handler{auth: mockAuth}
-	err := handler.authorizeSessionExecuteDirect(ctx, session, differentCreatorID)
+	// The ownership mismatch is rejected before any constraint set is built,
+	// so recs is nil here (no HasPermissionForConstraintsAPI mock needed).
+	err := handler.authorizeSessionExecuteDirect(ctx, session, differentCreatorID, nil)
 	require.Error(t, err)
 	ce, ok := IsClientError(err)
 	require.True(t, ok, "expected a clientError")
@@ -4336,7 +4438,8 @@ func TestHandler_authorizeSessionExecuteDirect_ExecuteOwn_NonOwner(t *testing.T)
 // TestHandler_authorizeSessionExecuteDirect_AdminGroupViaExecuteAny verifies
 // that an Administrators-group user whose {admin,*} wildcard resolves to
 // execute-any is PERMITTED by the HasPermissionAPI path (not a dead
-// session.Role shortcut that was removed in issue #940).
+// session.Role shortcut that was removed in issue #940), and that the
+// execute-any permission's own Constraints are then consulted (issue #60).
 func TestHandler_authorizeSessionExecuteDirect_AdminGroupViaExecuteAny(t *testing.T) {
 	ctx := context.Background()
 	mockAuth := new(MockAuthService)
@@ -4345,15 +4448,78 @@ func TestHandler_authorizeSessionExecuteDirect_AdminGroupViaExecuteAny(t *testin
 	adminUserID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"
 	creatorID := "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb"
 	session := &Session{UserID: adminUserID}
+	recs := []config.RecommendationRecord{{Provider: "aws", Service: "ec2", Region: "us-east-1", UpfrontCost: 500}}
 
 	// Administrators-group wildcard {admin,*} covers execute-any.
 	mockAuth.On("HasPermissionAPI", ctx, adminUserID, "execute-any", "purchases").Return(true, nil)
+	mockAuth.On("HasPermissionForConstraintsAPI", ctx, adminUserID, "execute-any", "purchases",
+		purchaseConstraintSets(recs)).Return(true, nil)
 
 	handler := &Handler{auth: mockAuth}
-	err := handler.authorizeSessionExecuteDirect(ctx, session, creatorID)
+	err := handler.authorizeSessionExecuteDirect(ctx, session, creatorID, recs)
 	require.NoError(t, err)
 }
 
+// TestHandler_authorizeSessionExecuteDirect_ExecuteAny_ConstraintsDenied is
+// the SEC-01 (#1141) follow-up regression test for issue #60: a session
+// whose execute-any:purchases verb is granted (the bare-verb gate passes)
+// but whose per-permission Constraints reject the request must receive a
+// 403. Pre-fix, authorizeSessionExecuteDirect never consulted the
+// execute-any permission's own Constraints, so this request was allowed
+// through to directExecutePurchase.
+func TestHandler_authorizeSessionExecuteDirect_ExecuteAny_ConstraintsDenied(t *testing.T) {
+	ctx := context.Background()
+	mockAuth := new(MockAuthService)
+	t.Cleanup(func() { mockAuth.AssertExpectations(t) })
+
+	userID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa"
+	creatorID := "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb"
+	session := &Session{UserID: userID}
+	recs := []config.RecommendationRecord{{Provider: "aws", Service: "ec2", Region: "us-east-1", UpfrontCost: 50000}}
+
+	mockAuth.On("HasPermissionAPI", ctx, userID, "execute-any", "purchases").Return(true, nil)
+	mockAuth.On("HasPermissionForConstraintsAPI", ctx, userID, "execute-any", "purchases",
+		purchaseConstraintSets(recs)).Return(false, nil)
+
+	handler := &Handler{auth: mockAuth}
+	err := handler.authorizeSessionExecuteDirect(ctx, session, creatorID, recs)
+	require.Error(t, err)
+	ce, ok := IsClientError(err)
+	require.True(t, ok, "expected a clientError, got: %v", err)
+	assert.Equal(t, 403, ce.code)
+	assert.Contains(t, ce.Error(), "constraints")
+}
+
+// TestHandler_authorizeSessionExecuteDirect_ExecuteOwn_ConstraintsDenied
+// mirrors the ExecuteAny case above for execute-own:purchases (issue #60):
+// the creator-ownership check passes, but the execute-own permission's own
+// Constraints reject the request.
+func TestHandler_authorizeSessionExecuteDirect_ExecuteOwn_ConstraintsDenied(t *testing.T) {
+	ctx := context.Background()
+	mockAuth := new(MockAuthService)
+	t.Cleanup(func() { mockAuth.AssertExpectations(t) })
+
+	userID := "cccccccc-cccc-cccc-cccc-cccccccccccc"
+	session := &Session{UserID: userID}
+	recs := []config.RecommendationRecord{{Provider: "aws", Service: "ec2", Region: "us-east-1", UpfrontCost: 50000}}
+
+	mockAuth.On("HasPermissionAPI", ctx, userID, "execute-any", "purchases").Return(false, nil)
+	mockAuth.On("HasPermissionAPI", ctx, userID, "execute-own", "purchases").Return(true, nil)
+	mockAuth.On("HasPermissionForConstraintsAPI", ctx, userID, "execute-own", "purchases",
+		purchaseConstraintSets(recs)).Return(false, nil)
+
+	handler := &Handler{auth: mockAuth}
+	// creatorID == userID: the ownership gate passes, so the constraint
+	// check is the only thing standing between this session and a direct
+	// execute past its cap.
+	err := handler.authorizeSessionExecuteDirect(ctx, session, userID, recs)
+	require.Error(t, err)
+	ce, ok := IsClientError(err)
+	require.True(t, ok, "expected a clientError, got: %v", err)
+	assert.Equal(t, 403, ce.code)
+	assert.Contains(t, ce.Error(), "constraints")
+}
+
 // TestHandler_authorizeSessionExecuteDirect_NoGrant verifies that a session
 // without execute-any or execute-own on purchases is rejected with 403.
 func TestHandler_authorizeSessionExecuteDirect_NoGrant(t *testing.T) {
@@ -4369,7 +4535,7 @@ func TestHandler_authorizeSessionExecuteDirect_NoGrant(t *testing.T) {
 	mockAuth.On("HasPermissionAPI", ctx, userID, "execute-own", "purchases").Return(false, nil)
 
 	handler := &Handler{auth: mockAuth}
-	err := handler.authorizeSessionExecuteDirect(ctx, session, creatorID)
+	err := handler.authorizeSessionExecuteDirect(ctx, session, creatorID, nil)
 	require.Error(t, err)
 	ce, ok := IsClientError(err)
 	require.True(t, ok, "expected a clientError")
@@ -4383,7 +4549,7 @@ func TestHandler_authorizeSessionExecuteDirect_NilAuth(t *testing.T) {
 	session := &Session{UserID: "eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee"}
 
 	handler := &Handler{auth: nil}
-	err := handler.authorizeSessionExecuteDirect(ctx, session, "")
+	err := handler.authorizeSessionExecuteDirect(ctx, session, "", nil)
 	require.Error(t, err)
 	ce, ok := IsClientError(err)
 	require.True(t, ok, "expected a clientError")
@@ -5831,6 +5997,13 @@ func TestHandler_approvePurchaseViaSession_FourEyesOn_DifferentApproverSucceeds(
 		ApprovalToken:   "valid-token",
 		Status:          "pending",
 		CreatedByUserID: &creatorID,
+		// authorizeSessionApprove builds constraint sets from Recommendations
+		// (issue #60); an empty set is a caller bug the constraint check
+		// fails closed on, so this fixture needs at least one recommendation
+		// like every other execution a real submit would have produced.
+		Recommendations: []config.RecommendationRecord{
+			{Provider: "aws", Service: "ec2", Region: "us-east-1", UpfrontCost: 100},
+		},
 	}
 	approved := &config.PurchaseExecution{ExecutionID: execID, PlanID: planID, Status: "approved", StepNumber: 1}
 	mockConfig.On("GetExecutionByID", ctx, execID).Return(exec, nil)
diff --git a/internal/api/handler_ri_exchange.go b/internal/api/handler_ri_exchange.go
index 07a60e34a..ebfe84c81 100644
--- a/internal/api/handler_ri_exchange.go
+++ b/internal/api/handler_ri_exchange.go
@@ -1078,7 +1078,7 @@ func (h *Handler) checkAzureExecuteConstraints(ctx context.Context, session *Ses
 		attempt.MaxPurchaseAmount = math.MaxFloat64
 	}
 
-	err := h.requirePermissionConstraints(ctx, session, "ri-exchange", []auth.PermissionConstraints{attempt})
+	err := h.requirePermissionConstraints(ctx, session, "execute", "ri-exchange", []auth.PermissionConstraints{attempt})
 	if err == nil || isUSD {
 		return err
 	}
@@ -1087,7 +1087,7 @@ func (h *Handler) checkAzureExecuteConstraints(ctx context.Context, session *Ses
 	// specifically the cause.
 	withoutAmount := base
 	withoutAmount.MaxPurchaseAmount = 0
-	if otherErr := h.requirePermissionConstraints(ctx, session, "ri-exchange", []auth.PermissionConstraints{withoutAmount}); otherErr != nil {
+	if otherErr := h.requirePermissionConstraints(ctx, session, "execute", "ri-exchange", []auth.PermissionConstraints{withoutAmount}); otherErr != nil {
 		return otherErr
 	}
 	return NewClientError(403, fmt.Sprintf(
@@ -1846,7 +1846,7 @@ func (h *Handler) executeExchange(ctx context.Context, req *events.LambdaFunctio
 	if err != nil {
 		return nil, fmt.Errorf("failed to resolve cloud account scope: %w", err)
 	}
-	err = h.requirePermissionConstraints(ctx, session, "ri-exchange", []auth.PermissionConstraints{{
+	err = h.requirePermissionConstraints(ctx, session, "execute", "ri-exchange", []auth.PermissionConstraints{{
 		AccountIDs:        []string{cloudAccountID},
 		Providers:         []string{string(common.ProviderAWS)},
 		Services:          []string{string(common.ServiceEC2)},
diff --git a/internal/api/handler_test.go b/internal/api/handler_test.go
index 27e14b1e2..340304de5 100644
--- a/internal/api/handler_test.go
+++ b/internal/api/handler_test.go
@@ -1254,6 +1254,13 @@ func TestHandler_HandleRequest_RunPlannedPurchase(t *testing.T) {
 	mockAuth.On("ValidateCSRFToken", ctx, mock.Anything, mock.Anything).Return(nil)
 
 	mockPurchase := new(MockPurchaseManager)
+	// runPlannedPurchase re-hydrates the execution to enforce the session's
+	// execute:purchases Constraints against its recommendations (issue #60).
+	mockStore.On("GetExecutionByID", mock.Anything, "11111111-1111-1111-1111-111111111111").Return(&config.PurchaseExecution{
+		ExecutionID:     "11111111-1111-1111-1111-111111111111",
+		Status:          "pending",
+		Recommendations: []config.RecommendationRecord{{Provider: "aws", Service: "ec2", Region: "us-east-1", UpfrontCost: 100}},
+	}, nil)
 	// This request authenticates via the admin API key (X-API-Key), which
 	// resolves to the stateless admin-api-key principal rather than the
 	// bearer-token session, so the actor identity fourEyesActorIdentity

From 8efd142d9c2a9f8ce22ed31bdc81ca520292e5a7 Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Mon, 28 Sep 2026 13:19:56 +0200
Subject: [PATCH 2/4] sec(api): make approve constraint denial terminal, close
 token-flow bypass

Independent review of #378 found that approvePurchase's session-vs-token
dispatch treated authorizeSessionApprove's Constraints-exceeded 403 the
same as a bare missing-verb 403 (isPermissionDenied matches any 403
ClientError), so it fell through to approveViaToken -- which never
evaluates Constraints. A session with a capped approve-any/approve-own
permission who was also the account's registered contact_email could
approve past its own cap via the email deep-link (which always carries a
session, since the frontend forces a login first).

Fix: wrap a Constraints-exceeded 403 in a new constraintDeniedError type
so isPermissionDenied's deliberately strict, unwrapped *clientError
assertion no longer recognizes it, forcing the dispatch's default
(propagate, terminal) branch instead of the fall-through one.
IsClientError still resolves it via errors.As + Unwrap, so HTTP response
mapping is unaffected. approveViaToken also gets its own independent
Constraints check (approveConstraintsForSession) as defense in depth, in
case it is ever reached some other way with such a session present.

Also: authorizeSessionApprove and runPlannedPurchase previously skipped
requireNonZeroCommitment, so an execution whose recommendations summed
to /bin/zsh would read as an uncapped MaxPurchaseAmount. All five constrained
call sites (execute-any, execute-own, approve-any, approve-own,
runPlannedPurchase) now route through enforcePurchaseConstraints, which
threads an explicit action parameter and always runs
requireNonZeroCommitment before the Constraints check.

Filed LeanerCloud/cloud-commitments-platform#404 to track the five
remaining call sites named in issue #60's review comments (ladder
config, global config, RI-exchange config, marketplace list, revoke)
that carry the same fail-open shape but are out of scope for this PR.

Regression tests: TestHandler_approvePurchase_SessionApproveAny_TokenPresent_ConstraintsDenied
proves the dispatch-level fix with a non-empty token (the exact shape
the review flagged); TestHandler_approveViaToken_SessionApproveAny_ConstraintsDenied
drives approveViaToken directly to prove its own independent check.
Both fail against the pre-fix code (verified via git stash) and pass
against the fix.
---
 internal/api/handler_purchases.go      | 145 ++++++++++++++++--
 internal/api/handler_purchases_test.go | 203 ++++++++++++++++++++++---
 internal/api/middleware_test.go        |  15 +-
 internal/api/purchase_pricing.go       |   2 +-
 4 files changed, 325 insertions(+), 40 deletions(-)

diff --git a/internal/api/handler_purchases.go b/internal/api/handler_purchases.go
index 81ff4beaf..dcfeae28d 100644
--- a/internal/api/handler_purchases.go
+++ b/internal/api/handler_purchases.go
@@ -631,6 +631,21 @@ func (h *Handler) approveViaToken(ctx context.Context, req *events.LambdaFunctio
 	if err := h.requireDifferentApprover(ctx, tokenSession, execution); err != nil {
 		return nil, err
 	}
+	// SEC-01 follow-up (issue #60 review): the deep-link flow always forces
+	// a login, so tokenSession is populated whenever a browser reaches this
+	// endpoint. approvePurchase's session-vs-token dispatch already denies a
+	// session whose approve-any/approve-own is capped tighter than this
+	// request before it ever reaches approveViaToken (authorizeSessionApprove
+	// returns a terminal constraintDeniedError, not a fall-through 403). This
+	// is a second, independent evaluation of that same session's Constraints
+	// so a capped approver who is ALSO the account's registered
+	// contact_email cannot use the email link to approve past their own cap
+	// -- the contact_email gate above authorizes a completely different
+	// principal (the account's contact), and must not be read as clearing
+	// Constraints on this session's own approve-* permission.
+	if err := h.approveConstraintsForSession(ctx, tokenSession, execution); err != nil {
+		return nil, err
+	}
 	// Check for Gmail-style pre-fire delay (issue #291 wave-2).
 	// Token/email-link path: no authenticated session UUID is available, so the
 	// scheduled transition is recorded as system-initiated (transitioned_by = NULL).
@@ -779,6 +794,23 @@ func (h *Handler) approvePurchaseViaSession(ctx context.Context, req *events.Lam
 // could approve (and thereby execute) an arbitrarily large pending execution
 // created by someone else, even though the identically-capped execute-any
 // permission already blocks a matching direct-execute.
+//
+// A session holding BOTH approve-any and approve-own is evaluated on
+// approve-any ONLY (the first branch below returns unconditionally once
+// hasAny is true): approve-any's Constraints are what gate it, even for an
+// execution this session happens to have created itself. This mirrors
+// authorizeSessionExecuteDirect's execute-any/execute-own ordering and the
+// group model's own semantics -- approve-any is the broader grant, so a
+// group holding both is expected to configure whichever Constraints the
+// broader grant should carry, not rely on approve-own's (narrower-intent)
+// Constraints ever being consulted for such a session.
+//
+// The returned error is wrapped via wrapConstraintDenied whenever it comes
+// from the Constraints check (as opposed to the bare verb/ownership checks
+// above it): approvePurchase's session-vs-token dispatch must treat a
+// Constraints-exceeded denial as terminal, not as a signal to fall through
+// to the token/contact_email flow, which never evaluates Constraints (issue
+// #60 review follow-up; see constraintDeniedError's doc comment).
 func (h *Handler) authorizeSessionApprove(ctx context.Context, session *Session, execution *config.PurchaseExecution) error {
 	// The stateless admin API key has full access and no user row to resolve
 	// permissions from. Administrators-group users fall through and pass via
@@ -796,7 +828,7 @@ func (h *Handler) authorizeSessionApprove(ctx context.Context, session *Session,
 		return fmt.Errorf("permission check failed: %w", err)
 	}
 	if hasAny {
-		return h.requirePermissionConstraints(ctx, session, auth.ActionApproveAny, auth.ResourcePurchases, purchaseConstraintSets(execution.Recommendations))
+		return wrapConstraintDenied(h.enforcePurchaseConstraints(ctx, session, auth.ActionApproveAny, execution.Recommendations))
 	}
 
 	hasOwn, err := h.auth.HasPermissionAPI(ctx, session.UserID, auth.ActionApproveOwn, auth.ResourcePurchases)
@@ -810,7 +842,42 @@ func (h *Handler) authorizeSessionApprove(ctx context.Context, session *Session,
 	if execution.CreatedByUserID == nil || *execution.CreatedByUserID != session.UserID {
 		return NewClientError(403, "permission denied: cannot approve another user's pending purchase")
 	}
-	return h.requirePermissionConstraints(ctx, session, auth.ActionApproveOwn, auth.ResourcePurchases, purchaseConstraintSets(execution.Recommendations))
+	return wrapConstraintDenied(h.enforcePurchaseConstraints(ctx, session, auth.ActionApproveOwn, execution.Recommendations))
+}
+
+// approveConstraintsForSession is approveViaToken's independent Constraints
+// check (issue #60 review follow-up): it mirrors authorizeSessionApprove's
+// approve-any/approve-own grant-then-constrain matrix, but returns nil
+// (rather than a 403) when the session holds neither verb, or holds only
+// approve-own for an execution it didn't create. Those are not denials here
+// -- a session with no applicable approve-* permission simply has no
+// Constraints to be bound by, and is being authorized entirely through the
+// token flow's separate contact_email mechanism. This function only ever
+// narrows what that mechanism would otherwise allow; it never widens it.
+//
+// session may be nil (a pure email-client click with no browser session at
+// all); that also returns nil, since there is no session-held permission to
+// evaluate. session.UserID == apiKeyAdminUserID or a nil h.auth are treated
+// the same way authorizeSessionApprove treats them.
+func (h *Handler) approveConstraintsForSession(ctx context.Context, session *Session, execution *config.PurchaseExecution) error {
+	if session == nil || session.UserID == apiKeyAdminUserID || h.auth == nil {
+		return nil
+	}
+	hasAny, err := h.auth.HasPermissionAPI(ctx, session.UserID, auth.ActionApproveAny, auth.ResourcePurchases)
+	if err != nil {
+		return fmt.Errorf("permission check failed: %w", err)
+	}
+	if hasAny {
+		return h.enforcePurchaseConstraints(ctx, session, auth.ActionApproveAny, execution.Recommendations)
+	}
+	hasOwn, err := h.auth.HasPermissionAPI(ctx, session.UserID, auth.ActionApproveOwn, auth.ResourcePurchases)
+	if err != nil {
+		return fmt.Errorf("permission check failed: %w", err)
+	}
+	if hasOwn && execution.CreatedByUserID != nil && *execution.CreatedByUserID == session.UserID {
+		return h.enforcePurchaseConstraints(ctx, session, auth.ActionApproveOwn, execution.Recommendations)
+	}
+	return nil
 }
 
 // approveWithDelay is the Gmail-style pre-fire delay branch (issue #291 wave-2).
@@ -1052,7 +1119,7 @@ func (h *Handler) authorizeSessionExecuteDirect(ctx context.Context, session *Se
 		return fmt.Errorf("permission check failed: %w", err)
 	}
 	if hasAny {
-		return h.requirePermissionConstraints(ctx, session, auth.ActionExecuteAny, auth.ResourcePurchases, purchaseConstraintSets(recs))
+		return h.enforcePurchaseConstraints(ctx, session, auth.ActionExecuteAny, recs)
 	}
 
 	hasOwn, err := h.auth.HasPermissionAPI(ctx, session.UserID, auth.ActionExecuteOwn, auth.ResourcePurchases)
@@ -1068,7 +1135,7 @@ func (h *Handler) authorizeSessionExecuteDirect(ctx context.Context, session *Se
 	if session.UserID == "" || creatorID == "" || creatorID != session.UserID {
 		return NewClientError(403, "permission denied: execute-own requires you to be the creator of this purchase")
 	}
-	return h.requirePermissionConstraints(ctx, session, auth.ActionExecuteOwn, auth.ResourcePurchases, purchaseConstraintSets(recs))
+	return h.enforcePurchaseConstraints(ctx, session, auth.ActionExecuteOwn, recs)
 }
 
 func (h *Handler) cancelPurchase(ctx context.Context, req *events.LambdaFunctionURLRequest, execID, token string) (any, error) {
@@ -1680,7 +1747,7 @@ func (h *Handler) retryPurchase(ctx context.Context, req *events.LambdaFunctionU
 	// store-derived prices stamped at submit time
 	// (priceRecommendationsFromStore), so this re-check never sees
 	// client-supplied numbers.
-	if constraintErr := h.enforcePurchaseConstraints(ctx, session, failedExec.Recommendations); constraintErr != nil {
+	if constraintErr := h.enforcePurchaseConstraints(ctx, session, "execute", failedExec.Recommendations); constraintErr != nil {
 		return nil, constraintErr
 	}
 
@@ -2048,6 +2115,44 @@ func (h *Handler) tryResolveActorEmail(ctx context.Context, req *events.LambdaFu
 	return ""
 }
 
+// constraintDeniedError marks a 403 produced by requirePermissionConstraints
+// (a granted verb whose own per-permission Constraints reject the request)
+// so isPermissionDenied's strict, unwrapped *clientError assertion does NOT
+// recognize it (issue #60 review follow-up, same mechanism as errCSRFRejected
+// above). This matters because approvePurchase's session-vs-token dispatch
+// treats any isPermissionDenied 403 as "no approve-* grant at all, fall
+// through to the token/contact_email flow" -- correct for a bare missing-verb
+// denial, but wrong for a Constraints-exceeded denial: approveViaToken never
+// evaluates Constraints, so a session holding a capped approve-any/
+// approve-own would otherwise bypass its own cap by using the email
+// deep-link (which forces a login and therefore always carries a session)
+// instead of the dashboard Approve button. Wrapping forces the dispatch's
+// `default:` branch (propagate, terminal) instead of the fall-through
+// branch. IsClientError still resolves this to the right HTTP code and
+// message via errors.As + Unwrap, so response mapping is unaffected.
+type constraintDeniedError struct {
+	*clientError
+}
+
+func (e *constraintDeniedError) Unwrap() error { return e.clientError }
+
+// wrapConstraintDenied marks a non-nil requirePermissionConstraints /
+// enforcePurchaseConstraints error as terminal (see constraintDeniedError).
+// A nil err, or an err that isn't a *clientError (e.g. an auth-service
+// failure wrapped with fmt.Errorf), passes through unchanged -- the latter
+// is already un-recognized by isPermissionDenied's own strict assertion, so
+// it already hits the propagate branch without needing to be wrapped here.
+func wrapConstraintDenied(err error) error {
+	if err == nil {
+		return nil
+	}
+	ce, ok := err.(*clientError) //nolint:errorlint // wrapping the concrete constructor's return; see constraintDeniedError doc
+	if !ok {
+		return err
+	}
+	return &constraintDeniedError{clientError: ce}
+}
+
 // isPermissionDenied reports whether err is *directly* a 403 ClientError
 // (not merely something that wraps one). Used by the cancel-from-email
 // session pre-check to distinguish a legitimate "your session lacks
@@ -2061,7 +2166,8 @@ func (h *Handler) tryResolveActorEmail(ctx context.Context, req *events.LambdaFu
 // wrapper's intent), not "this is still a 403". errors.As-style unwrapping
 // would erase that distinction and silently route wrapped backend
 // failures into the contact_email gate — exactly the misclassification
-// the propagate-vs-fall-through split is meant to prevent.
+// the propagate-vs-fall-through split is meant to prevent. constraintDeniedError
+// above deliberately exploits the same strictness from the other direction.
 func isPermissionDenied(err error) bool {
 	ce, ok := err.(*clientError) //nolint:errorlint // strict (unwrapped) assertion is deliberate; see comment above
 	return ok && ce.code == 403
@@ -2245,21 +2351,28 @@ func (h *Handler) validateExecutePurchaseRequest(ctx context.Context, req *event
 
 // enforcePurchaseConstraints builds the per-recommendation
 // auth.PermissionConstraints sets (purchaseConstraintSets) and enforces them
-// against session's execute:purchases permission, first rejecting a batch
+// against session's action:purchases permission, first rejecting a batch
 // whose total commitment computes to zero (requireNonZeroCommitment).
-// Shared by the direct-execute request validation and the retry path
-// (retryPurchase) so both re-derive and check the exact same Constraints
-// (SEC-01, issue #1141; adversarial review follow-up to #1210 -- the retry
-// path previously skipped this check entirely). The web execute path
-// reaches this through priceAndEnforcePurchaseConstraints, so recs carry
-// store-derived costs; the retry path enforces against the persisted row,
-// which was priced when it was written.
-func (h *Handler) enforcePurchaseConstraints(ctx context.Context, session *Session, recs []config.RecommendationRecord) error {
+// Shared by every money-mutating purchases call site (direct-execute request
+// validation, the retry path, execute-any/own, approve-any/own and
+// runPlannedPurchase) so all of them re-derive and check the exact same
+// Constraints against the exact same zero-commitment guard (SEC-01, issue
+// #1141; adversarial review follow-up to #1210 -- the retry path previously
+// skipped this check entirely; issue #60 review follow-up -- approve-any/own
+// and runPlannedPurchase previously skipped requireNonZeroCommitment, so an
+// execution whose recs summed to $0 read as an uncapped MaxPurchaseAmount).
+// action is threaded through (not hardcoded) because callers check
+// different verbs: "execute" for the web submit and retry paths and
+// runPlannedPurchase, "execute-any"/"execute-own" for direct-execute,
+// "approve-any"/"approve-own" for session approve. recs carry store-derived
+// or persisted-at-submit-time costs depending on the caller; none of these
+// call sites ever price from client-supplied numbers.
+func (h *Handler) enforcePurchaseConstraints(ctx context.Context, session *Session, action string, recs []config.RecommendationRecord) error {
 	constraintSets := purchaseConstraintSets(recs)
 	if err := requireNonZeroCommitment(constraintSets); err != nil {
 		return err
 	}
-	return h.requirePermissionConstraints(ctx, session, "execute", "purchases", constraintSets)
+	return h.requirePermissionConstraints(ctx, session, action, "purchases", constraintSets)
 }
 
 // purchaseConstraintSets builds one auth.PermissionConstraints per
diff --git a/internal/api/handler_purchases_test.go b/internal/api/handler_purchases_test.go
index 7592104c3..8fc79fa33 100644
--- a/internal/api/handler_purchases_test.go
+++ b/internal/api/handler_purchases_test.go
@@ -22,6 +22,13 @@ import (
 // to satisfy the post-hardening approver-set policy (see
 // authorizeApprovalAction): the global notify mailbox is no longer an
 // authorized approver, so tests must wire a per-account contact email.
+//
+// The recommendation carries a non-zero UpfrontCost: authorizeSessionApprove
+// / approveConstraintsForSession route through enforcePurchaseConstraints
+// (issue #60 review follow-up), which refuses a batch whose total commitment
+// computes to $0 (requireNonZeroCommitment) before ever consulting
+// Constraints -- a $0-cost fixture would hit that guard on the session-
+// approve path, not the specific behavior each test exercises.
 func approvalTestExec(execID, contact string, mockConfig *MockConfigStore) *config.PurchaseExecution {
 	accountID := "acct-1"
 	exec := &config.PurchaseExecution{
@@ -29,7 +36,7 @@ func approvalTestExec(execID, contact string, mockConfig *MockConfigStore) *conf
 		ApprovalToken: "valid-token",
 		Status:        "pending",
 		Recommendations: []config.RecommendationRecord{
-			{ID: "r1", CloudAccountID: &accountID},
+			{ID: "r1", Provider: "aws", Service: "ec2", Region: "us-east-1", CloudAccountID: &accountID, UpfrontCost: 100},
 		},
 	}
 	mockConfig.GetCloudAccountFn = func(_ context.Context, id string) (*config.CloudAccount, error) {
@@ -293,8 +300,12 @@ func TestHandler_approvePurchase_SessionApproveAnyChainsToExecute(t *testing.T)
 		ExecutionID:   execID,
 		ApprovalToken: "valid-token",
 		Status:        "pending",
+		// UpfrontCost non-zero: authorizeSessionApprove now routes through
+		// enforcePurchaseConstraints, which refuses a $0-commitment batch
+		// before this test's approve-any flow is even exercised (issue #60
+		// review follow-up).
 		Recommendations: []config.RecommendationRecord{
-			{ID: "r1"},
+			{ID: "r1", Provider: "aws", Service: "ec2", Region: "us-east-1", UpfrontCost: 100},
 		},
 	}
 	mockConfig.On("GetExecutionByID", ctx, execID).Return(exec, nil)
@@ -344,10 +355,13 @@ func TestHandler_approvePurchase_SessionExecuteFailureSurfacesAs409(t *testing.T
 
 	mockConfig := new(MockConfigStore)
 	exec := &config.PurchaseExecution{
-		ExecutionID:     execID,
-		ApprovalToken:   "valid-token",
-		Status:          "pending",
-		Recommendations: []config.RecommendationRecord{{ID: "r1"}},
+		ExecutionID:   execID,
+		ApprovalToken: "valid-token",
+		Status:        "pending",
+		// Non-zero UpfrontCost: see the comment on the sibling test above.
+		Recommendations: []config.RecommendationRecord{
+			{ID: "r1", Provider: "aws", Service: "ec2", Region: "us-east-1", UpfrontCost: 100},
+		},
 	}
 	mockConfig.On("GetExecutionByID", ctx, execID).Return(exec, nil)
 	// approvePurchaseViaSession checks PurchaseDelayHours (issue #291 wave-2)
@@ -385,6 +399,15 @@ func TestHandler_approvePurchase_SessionExecuteFailureSurfacesAs409(t *testing.T
 // never consulted the approve-any permission's own Constraints, so a
 // $50,000 execution created by someone else was approved (and executed) by
 // a session whose approve-any was configured with a $1,000 cap.
+//
+// This is also the terminal-dispatch regression test from the #60 review:
+// this call uses an EMPTY token, which pre-#60-review would reach
+// approvePurchaseViaSession/its CSRF check either way, masking whether the
+// constraint denial itself was terminal. The absence of ANY CSRF (or other)
+// mock beyond the two RBAC ones registered below is what proves it: if
+// isPermissionDenied ever again started recognizing a wrapped
+// constraintDeniedError, this test would panic on an unexpected call
+// instead of silently reading a different error message.
 func TestHandler_approvePurchase_SessionApproveAny_PermissionConstraintsDenied(t *testing.T) {
 	ctx := context.Background()
 	execID := "12345678-1234-1234-1234-123456789abc"
@@ -413,12 +436,14 @@ func TestHandler_approvePurchase_SessionApproveAny_PermissionConstraintsDenied(t
 	mockAuth.On("HasPermissionAPI", ctx, userSession.UserID, "approve-any", "purchases").Return(true, nil)
 	mockAuth.On("HasPermissionForConstraintsAPI", ctx, userSession.UserID, "approve-any", "purchases",
 		purchaseConstraintSets(exec.Recommendations)).Return(false, nil)
-	// authorizeSessionApprove's constraint denial is a plain 403, which
-	// isPermissionDenied treats the same as a bare-verb 403 and falls
-	// through to approvePurchaseViaSession (token is empty in this test, so
-	// that is the terminal branch either way). That path enforces CSRF
-	// before re-running the same RBAC gate.
-	mockAuth.On("ValidateCSRFToken", ctx, "sess-tok", "").Return(nil)
+	// authorizeSessionApprove wraps the constraint denial in
+	// constraintDeniedError (issue #60 review follow-up), so
+	// approvePurchase's session-vs-token dispatch treats it as terminal: the
+	// `default:` branch returns it directly instead of falling through to
+	// approvePurchaseViaSession's CSRF check / a second RBAC pass, or to the
+	// token/contact_email flow. No ValidateCSRFToken (or any other) mock is
+	// registered beyond the two above, and mockAuth.AssertExpectations in
+	// t.Cleanup would fail this test if the dispatch reached either path.
 
 	mockPurchase := new(MockPurchaseManager)
 	t.Cleanup(func() { mockPurchase.AssertExpectations(t) })
@@ -436,6 +461,70 @@ func TestHandler_approvePurchase_SessionApproveAny_PermissionConstraintsDenied(t
 	mockPurchase.AssertNotCalled(t, "ApproveExecution", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
 }
 
+// TestHandler_approvePurchase_SessionApproveAny_TokenPresent_ConstraintsDenied
+// is the MAJOR-finding regression test from the #60 independent review: the
+// email deep-link flow forces a login, so a session is ALWAYS present
+// alongside a non-empty token. Pre-review-fix, approvePurchase's dispatch
+// treated authorizeSessionApprove's constraint denial as an ordinary
+// isPermissionDenied 403 and fell through to approveViaToken, which never
+// evaluates Constraints -- so a capped approve-any session who was ALSO the
+// account's registered contact_email could approve straight past its own
+// cap by using the emailed link instead of the dashboard Approve button.
+// This session models exactly that: contact_email matches the session's
+// own email, so authorizeApprovalAction (approveViaToken's own gate) would
+// have granted it every bit as readily as the pre-fix dispatch's fall-
+// through did. No GetGlobalConfig / GetCloudAccountFn / ApproveExecution
+// mock is registered, so a regression that ever reaches approveViaToken
+// again panics on an unexpected call instead of silently approving.
+func TestHandler_approvePurchase_SessionApproveAny_TokenPresent_ConstraintsDenied(t *testing.T) {
+	ctx := context.Background()
+	execID := "eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee"
+	otherCreator := "creator-user-uuid"
+	accountID := "acct-1"
+	capped := &Session{UserID: "ffffffff-ffff-ffff-ffff-ffffffffffff", Email: "capped-contact@example.com"}
+
+	mockConfig := new(MockConfigStore)
+	t.Cleanup(func() { mockConfig.AssertExpectations(t) })
+	exec := &config.PurchaseExecution{
+		ExecutionID:     execID,
+		ApprovalToken:   "valid-token",
+		Status:          "pending",
+		CreatedByUserID: &otherCreator,
+		Recommendations: []config.RecommendationRecord{
+			{Provider: "aws", Service: "ec2", Region: "us-east-1", UpfrontCost: 50000, CloudAccountID: &accountID},
+		},
+	}
+	mockConfig.On("GetExecutionByID", ctx, execID).Return(exec, nil)
+	// Deliberately NOT wired: GetCloudAccountFn (the account's contact_email
+	// lookup approveViaToken's authorizeApprovalAction gate depends on) is
+	// left unset. The fixed dispatch never reaches approveViaToken for this
+	// session at all, so this test doesn't need it; TestHandler_approveViaToken_
+	// SessionApproveAny_ConstraintsDenied below exercises that gate directly
+	// (wired to succeed) to isolate approveConstraintsForSession's own check.
+
+	mockAuth := new(MockAuthService)
+	t.Cleanup(func() { mockAuth.AssertExpectations(t) })
+	mockAuth.On("ValidateSession", ctx, "capped-token").Return(capped, nil)
+	mockAuth.On("HasPermissionAPI", ctx, capped.UserID, "approve-any", "purchases").Return(true, nil)
+	mockAuth.On("HasPermissionForConstraintsAPI", ctx, capped.UserID, "approve-any", "purchases",
+		purchaseConstraintSets(exec.Recommendations)).Return(false, nil)
+
+	mockPurchase := new(MockPurchaseManager)
+	t.Cleanup(func() { mockPurchase.AssertExpectations(t) })
+
+	handler := &Handler{purchase: mockPurchase, config: mockConfig, auth: mockAuth}
+	req := &events.LambdaFunctionURLRequest{Headers: map[string]string{"authorization": "Bearer capped-token"}}
+	// Non-empty token: the email deep-link shape.
+	_, err := handler.approvePurchase(ctx, req, execID, "valid-token")
+	require.Error(t, err)
+	ce, ok := IsClientError(err)
+	require.True(t, ok, "expected a clientError, got: %v", err)
+	assert.Equal(t, 403, ce.code)
+	assert.Contains(t, ce.Error(), "constraints")
+	mockPurchase.AssertNotCalled(t, "ApproveAndExecute", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+	mockPurchase.AssertNotCalled(t, "ApproveExecution", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+}
+
 // TestHandler_authorizeSessionApprove_ApproveOwn_ConstraintsDenied mirrors
 // the approve-any case above for approve-own:purchases (issue #60): the
 // creator-ownership check passes, but the approve-own permission's own
@@ -520,6 +609,65 @@ func TestHandler_approveViaToken_GlobalConfigError_FailsClosed(t *testing.T) {
 		mock.Anything, mock.Anything, mock.Anything, mock.Anything)
 }
 
+// TestHandler_approveViaToken_SessionApproveAny_ConstraintsDenied drives
+// approveViaToken DIRECTLY (bypassing approvePurchase's dispatch) to prove
+// approveConstraintsForSession's own, independent Constraints evaluation
+// (issue #60 review follow-up finding #1's second half) denies a capped
+// approve-any session even when authorizeApprovalAction's contact_email
+// gate would otherwise let the request through. In production this
+// function is only reached via the dispatch, which already blocks this
+// same session earlier (see the TokenPresent test above); this test isolates
+// approveConstraintsForSession's own wiring so a future refactor that
+// changes how approveViaToken is reached still has direct coverage.
+func TestHandler_approveViaToken_SessionApproveAny_ConstraintsDenied(t *testing.T) {
+	ctx := context.Background()
+	execID := "12121212-1212-1212-1212-121212121212"
+	accountID := "acct-capped"
+	capped := &Session{UserID: "13131313-1313-1313-1313-131313131313", Email: "capped-contact@example.com"}
+
+	mockConfig := new(MockConfigStore)
+	t.Cleanup(func() { mockConfig.AssertExpectations(t) })
+	exec := &config.PurchaseExecution{
+		ExecutionID:   execID,
+		ApprovalToken: "valid-token",
+		Status:        "pending",
+		Recommendations: []config.RecommendationRecord{
+			{ID: "r1", Provider: "aws", Service: "ec2", Region: "us-east-1", UpfrontCost: 50000, CloudAccountID: &accountID},
+		},
+	}
+	// capped's own email is the account's registered contact_email, so
+	// authorizeApprovalAction's gate passes -- exactly the scenario where,
+	// pre-fix, the token flow's silence on Constraints mattered.
+	mockConfig.GetCloudAccountFn = func(_ context.Context, id string) (*config.CloudAccount, error) {
+		return &config.CloudAccount{ID: id, ContactEmail: capped.Email}, nil
+	}
+	// GetGlobalConfig is read twice: once (best-effort) inside
+	// authorizeApprovalAction, once inside requireDifferentApprover
+	// (mode off by default, so it returns nil without further reads).
+	mockConfig.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{}, nil)
+
+	mockAuth := new(MockAuthService)
+	t.Cleanup(func() { mockAuth.AssertExpectations(t) })
+	mockAuth.On("ValidateSession", ctx, "capped-token").Return(capped, nil)
+	mockAuth.On("HasPermissionAPI", ctx, capped.UserID, "approve-any", "purchases").Return(true, nil)
+	mockAuth.On("HasPermissionForConstraintsAPI", ctx, capped.UserID, "approve-any", "purchases",
+		purchaseConstraintSets(exec.Recommendations)).Return(false, nil)
+
+	mockPurchase := new(MockPurchaseManager)
+	t.Cleanup(func() { mockPurchase.AssertExpectations(t) })
+
+	handler := &Handler{purchase: mockPurchase, config: mockConfig, auth: mockAuth}
+	req := &events.LambdaFunctionURLRequest{Headers: map[string]string{"authorization": "Bearer capped-token"}}
+	_, err := handler.approveViaToken(ctx, req, exec, "valid-token")
+	require.Error(t, err)
+	ce, ok := IsClientError(err)
+	require.True(t, ok, "expected a clientError, got: %v", err)
+	assert.Equal(t, 403, ce.code)
+	assert.Contains(t, ce.Error(), "constraints")
+	mockPurchase.AssertNotCalled(t, "ApproveExecution", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+	mockPurchase.AssertNotCalled(t, "ApproveAndExecute", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+}
+
 // TestHandler_approvePurchaseViaSession_GlobalConfigError_FailsClosed pins the
 // F3 fix for the session (dashboard) approve path: same contract as the token
 // path — a transient config error must not discard the free-cancel window.
@@ -530,10 +678,15 @@ func TestHandler_approvePurchaseViaSession_GlobalConfigError_FailsClosed(t *test
 
 	mockConfig := new(MockConfigStore)
 	exec := &config.PurchaseExecution{
-		ExecutionID:     execID,
-		ApprovalToken:   "valid-token",
-		Status:          "pending",
-		Recommendations: []config.RecommendationRecord{{ID: "r1"}},
+		ExecutionID:   execID,
+		ApprovalToken: "valid-token",
+		Status:        "pending",
+		// Non-zero UpfrontCost: authorizeSessionApprove's requireNonZeroCommitment
+		// guard must not shadow the GetGlobalConfig failure this test targets
+		// (issue #60 review follow-up).
+		Recommendations: []config.RecommendationRecord{
+			{ID: "r1", Provider: "aws", Service: "ec2", Region: "us-east-1", UpfrontCost: 100},
+		},
 	}
 	mockConfig.On("GetExecutionByID", ctx, execID).Return(exec, nil)
 	// GetGlobalConfig always fails — approvePurchaseViaSession must return the
@@ -658,7 +811,9 @@ func TestHandler_approvePurchase_AWSOrphanFallsThrough(t *testing.T) {
 		ApprovalToken: "valid-token",
 		Status:        "pending",
 		// CloudAccountID nil — but provider is AWS, so fallback applies.
-		Recommendations: []config.RecommendationRecord{{ID: "r1", Provider: "aws"}},
+		// UpfrontCost non-zero so enforcePurchaseConstraints' zero-commitment
+		// guard doesn't preempt the orphan-fallback behavior this test targets.
+		Recommendations: []config.RecommendationRecord{{ID: "r1", Provider: "aws", UpfrontCost: 100}},
 	}
 	mockConfig.On("GetExecutionByID", ctx, execID).Return(exec, nil)
 	mockConfig.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{}, nil)
@@ -698,7 +853,9 @@ func TestHandler_approvePurchase_NonOrphanUnchanged(t *testing.T) {
 		ApprovalToken: "valid-token",
 		Status:        "pending",
 		// CloudAccountID is set — normal case, guard must not fire.
-		Recommendations: []config.RecommendationRecord{{ID: "r1", Provider: "azure", CloudAccountID: &accountID}},
+		// UpfrontCost non-zero so enforcePurchaseConstraints' zero-commitment
+		// guard doesn't preempt the orphan-fallback behavior this test targets.
+		Recommendations: []config.RecommendationRecord{{ID: "r1", Provider: "azure", CloudAccountID: &accountID, UpfrontCost: 100}},
 		CloudAccountID:  &accountID,
 	}
 	mockConfig.On("GetExecutionByID", ctx, execID).Return(exec, nil)
@@ -5948,7 +6105,15 @@ func TestHandler_approvePurchaseViaSession_FourEyesOn_DeniesSelfApprove(t *testi
 		ApprovalToken:   "valid-token",
 		Status:          "pending",
 		CreatedByUserID: &adminID,
-		Recommendations: []config.RecommendationRecord{{ID: "r1"}},
+		// Non-zero UpfrontCost: this session reaches authorizeSessionApprove's
+		// approve-own branch (the bare admin wildcard covers approve-own,
+		// which is not carved out the way approve-any is), which now routes
+		// through enforcePurchaseConstraints and would otherwise refuse the
+		// $0-commitment batch before ever reaching the 4-eyes check this test
+		// targets (issue #60 review follow-up).
+		Recommendations: []config.RecommendationRecord{
+			{ID: "r1", Provider: "aws", Service: "ec2", Region: "us-east-1", UpfrontCost: 100},
+		},
 	}
 	mockConfig.On("GetExecutionByID", ctx, execID).Return(exec, nil)
 	mockConfig.On("GetGlobalConfig", ctx).Return(fourEyesCfgOn(), nil)
diff --git a/internal/api/middleware_test.go b/internal/api/middleware_test.go
index 7fb4a1742..8a319ccf4 100644
--- a/internal/api/middleware_test.go
+++ b/internal/api/middleware_test.go
@@ -349,10 +349,17 @@ func TestApproveViaSession_PassesCSRF(t *testing.T) {
 
 	mockConfig := new(MockConfigStore)
 	exec := &config.PurchaseExecution{
-		ExecutionID:     execID,
-		ApprovalToken:   "email-tok",
-		Status:          "pending",
-		Recommendations: []config.RecommendationRecord{{ID: "r1"}},
+		ExecutionID:   execID,
+		ApprovalToken: "email-tok",
+		Status:        "pending",
+		// Non-zero UpfrontCost: this test's CSRF token is valid, so the flow
+		// reaches authorizeSessionApprove's approve-any branch, which now
+		// routes through enforcePurchaseConstraints and would otherwise
+		// refuse the $0-commitment batch before the CSRF-pass assertion this
+		// test targets is reached (issue #60 review follow-up).
+		Recommendations: []config.RecommendationRecord{
+			{ID: "r1", Provider: "aws", Service: "ec2", Region: "us-east-1", UpfrontCost: 100},
+		},
 	}
 	mockConfig.On("GetExecutionByID", ctx, execID).Return(exec, nil)
 	mockConfig.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{}, nil)
diff --git a/internal/api/purchase_pricing.go b/internal/api/purchase_pricing.go
index 3308258e3..855392bf1 100644
--- a/internal/api/purchase_pricing.go
+++ b/internal/api/purchase_pricing.go
@@ -22,7 +22,7 @@ func (h *Handler) priceAndEnforcePurchaseConstraints(ctx context.Context, sessio
 	if err := h.priceRecommendationsFromStore(ctx, recs); err != nil {
 		return err
 	}
-	return h.enforcePurchaseConstraints(ctx, session, recs)
+	return h.enforcePurchaseConstraints(ctx, session, "execute", recs)
 }
 
 // priceRecommendationsFromStore rewrites recs in place. Each rec must match

From 0060cfda39c0d1056feb06f7c19d1cb1d2cc4693 Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Mon, 28 Sep 2026 17:26:11 +0200
Subject: [PATCH 3/4] test(api): cover runPlanned constraint and
 zero-commitment guards (#60)

---
 internal/api/handler_purchases_test.go | 73 ++++++++++++++++++++++++++
 1 file changed, 73 insertions(+)

diff --git a/internal/api/handler_purchases_test.go b/internal/api/handler_purchases_test.go
index 8fc79fa33..82030b6df 100644
--- a/internal/api/handler_purchases_test.go
+++ b/internal/api/handler_purchases_test.go
@@ -1590,6 +1590,79 @@ func TestHandler_runPlannedPurchase(t *testing.T) {
 	mockPurchase.AssertExpectations(t)
 }
 
+// runPlannedPurchaseGuardHarness wires a runPlannedPurchase call for a
+// session holding admin:* (so update-any lets authorizeExecutionManagement
+// pass) plus the given execute:purchases permission, against an execution
+// carrying recs. RunPlannedPurchaseNow is deliberately left unstubbed: a
+// regression that reaches the funnel panics on the unexpected call.
+func runPlannedPurchaseGuardHarness(t *testing.T, executePerm auth.Permission, recs []config.RecommendationRecord) (*MockPurchaseManager, error) {
+	t.Helper()
+	ctx := context.Background()
+	execID := "11111111-1111-1111-1111-111111111111"
+
+	mockStore := new(MockConfigStore)
+	t.Cleanup(func() { mockStore.AssertExpectations(t) })
+	mockStore.On("GetExecutionByID", ctx, execID).Return(&config.PurchaseExecution{
+		ExecutionID:     execID,
+		Status:          "pending",
+		Recommendations: recs,
+	}, nil)
+
+	mockAuth := new(MockAuthService)
+	t.Cleanup(func() { mockAuth.AssertExpectations(t) })
+	session := &Session{UserID: "dddddddd-dddd-dddd-dddd-dddddddddddd", Email: "capped@example.com"}
+	mockAuth.On("ValidateSession", ctx, "capped-token").Return(session, nil)
+	mockAuth.grantPermissions([]auth.Permission{
+		{Action: auth.ActionAdmin, Resource: auth.ResourceAll},
+		executePerm,
+	})
+
+	mockPurchase := new(MockPurchaseManager)
+	t.Cleanup(func() { mockPurchase.AssertExpectations(t) })
+
+	handler := &Handler{purchase: mockPurchase, config: mockStore, auth: mockAuth}
+	req := &events.LambdaFunctionURLRequest{Headers: map[string]string{"Authorization": "Bearer capped-token"}}
+	_, err := handler.runPlannedPurchase(ctx, req, execID)
+	return mockPurchase, err
+}
+
+// TestHandler_runPlannedPurchase_ConstraintDenied: a MaxPurchaseAmount cap on
+// the session's execute:purchases permission below the execution's total
+// commitment must refuse "Run now" with 403 before the purchase funnel fires
+// (issue #60).
+func TestHandler_runPlannedPurchase_ConstraintDenied(t *testing.T) {
+	mockPurchase, err := runPlannedPurchaseGuardHarness(t,
+		auth.Permission{
+			Action:      auth.ActionExecute,
+			Resource:    auth.ResourcePurchases,
+			Constraints: &auth.PermissionConstraints{MaxPurchaseAmount: 100},
+		},
+		[]config.RecommendationRecord{{Provider: "aws", Service: "ec2", Region: "us-east-1", UpfrontCost: 50000}},
+	)
+	require.Error(t, err)
+	ce, ok := IsClientError(err)
+	require.True(t, ok, "expected a clientError, got: %v", err)
+	assert.Equal(t, 403, ce.code)
+	assert.Contains(t, ce.Error(), "constraints")
+	mockPurchase.AssertNotCalled(t, "RunPlannedPurchaseNow", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+}
+
+// TestHandler_runPlannedPurchase_ZeroCommitment: an execution whose recs sum
+// to $0 must be refused with 400 by requireNonZeroCommitment rather than
+// reading as an uncapped MaxPurchaseAmount (issue #60 review follow-up).
+func TestHandler_runPlannedPurchase_ZeroCommitment(t *testing.T) {
+	mockPurchase, err := runPlannedPurchaseGuardHarness(t,
+		auth.Permission{Action: auth.ActionExecute, Resource: auth.ResourcePurchases},
+		[]config.RecommendationRecord{{Provider: "aws", Service: "ec2", Region: "us-east-1"}},
+	)
+	require.Error(t, err)
+	ce, ok := IsClientError(err)
+	require.True(t, ok, "expected a clientError, got: %v", err)
+	assert.Equal(t, 400, ce.code)
+	assert.Contains(t, ce.Error(), "zero total commitment")
+	mockPurchase.AssertNotCalled(t, "RunPlannedPurchaseNow", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+}
+
 func TestHandler_deletePlannedPurchase(t *testing.T) {
 	ctx := context.Background()
 	mockStore := new(MockConfigStore)

From 225a82940d9ab263c0efb0871ba09945410b8002 Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Mon, 28 Sep 2026 17:44:59 +0200
Subject: [PATCH 4/4] test(api): assert runPlanned funnel not called on a
 locally typed mock (#60)

---
 internal/api/handler_purchases_test.go | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/internal/api/handler_purchases_test.go b/internal/api/handler_purchases_test.go
index 82030b6df..1a704260e 100644
--- a/internal/api/handler_purchases_test.go
+++ b/internal/api/handler_purchases_test.go
@@ -1594,8 +1594,9 @@ func TestHandler_runPlannedPurchase(t *testing.T) {
 // session holding admin:* (so update-any lets authorizeExecutionManagement
 // pass) plus the given execute:purchases permission, against an execution
 // carrying recs. RunPlannedPurchaseNow is deliberately left unstubbed: a
-// regression that reaches the funnel panics on the unexpected call.
-func runPlannedPurchaseGuardHarness(t *testing.T, executePerm auth.Permission, recs []config.RecommendationRecord) (*MockPurchaseManager, error) {
+// regression that reaches the funnel panics on the unexpected call, and
+// AssertNotCalled states the contract explicitly.
+func runPlannedPurchaseGuardHarness(t *testing.T, executePerm auth.Permission, recs []config.RecommendationRecord) error {
 	t.Helper()
 	ctx := context.Background()
 	execID := "11111111-1111-1111-1111-111111111111"
@@ -1623,7 +1624,8 @@ func runPlannedPurchaseGuardHarness(t *testing.T, executePerm auth.Permission, r
 	handler := &Handler{purchase: mockPurchase, config: mockStore, auth: mockAuth}
 	req := &events.LambdaFunctionURLRequest{Headers: map[string]string{"Authorization": "Bearer capped-token"}}
 	_, err := handler.runPlannedPurchase(ctx, req, execID)
-	return mockPurchase, err
+	mockPurchase.AssertNotCalled(t, "RunPlannedPurchaseNow", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+	return err
 }
 
 // TestHandler_runPlannedPurchase_ConstraintDenied: a MaxPurchaseAmount cap on
@@ -1631,7 +1633,7 @@ func runPlannedPurchaseGuardHarness(t *testing.T, executePerm auth.Permission, r
 // commitment must refuse "Run now" with 403 before the purchase funnel fires
 // (issue #60).
 func TestHandler_runPlannedPurchase_ConstraintDenied(t *testing.T) {
-	mockPurchase, err := runPlannedPurchaseGuardHarness(t,
+	err := runPlannedPurchaseGuardHarness(t,
 		auth.Permission{
 			Action:      auth.ActionExecute,
 			Resource:    auth.ResourcePurchases,
@@ -1644,14 +1646,13 @@ func TestHandler_runPlannedPurchase_ConstraintDenied(t *testing.T) {
 	require.True(t, ok, "expected a clientError, got: %v", err)
 	assert.Equal(t, 403, ce.code)
 	assert.Contains(t, ce.Error(), "constraints")
-	mockPurchase.AssertNotCalled(t, "RunPlannedPurchaseNow", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
 }
 
 // TestHandler_runPlannedPurchase_ZeroCommitment: an execution whose recs sum
 // to $0 must be refused with 400 by requireNonZeroCommitment rather than
 // reading as an uncapped MaxPurchaseAmount (issue #60 review follow-up).
 func TestHandler_runPlannedPurchase_ZeroCommitment(t *testing.T) {
-	mockPurchase, err := runPlannedPurchaseGuardHarness(t,
+	err := runPlannedPurchaseGuardHarness(t,
 		auth.Permission{Action: auth.ActionExecute, Resource: auth.ResourcePurchases},
 		[]config.RecommendationRecord{{Provider: "aws", Service: "ec2", Region: "us-east-1"}},
 	)
@@ -1660,7 +1661,6 @@ func TestHandler_runPlannedPurchase_ZeroCommitment(t *testing.T) {
 	require.True(t, ok, "expected a clientError, got: %v", err)
 	assert.Equal(t, 400, ce.code)
 	assert.Contains(t, ce.Error(), "zero total commitment")
-	mockPurchase.AssertNotCalled(t, "RunPlannedPurchaseNow", mock.Anything, mock.Anything, mock.Anything, mock.Anything)
 }
 
 func TestHandler_deletePlannedPurchase(t *testing.T) {