From 775689eef32bc28c30be063641d4a798d31f3734 Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Wed, 10 Jun 2026 17:05:10 -0700
Subject: [PATCH 1/4] fix(auth): enforce per-permission constraints on execute
 paths

Per-permission Constraints (MaxPurchaseAmount, Providers, Services,
Regions, AccountIDs) configured on a group's execute:purchases or
execute:ri-exchange permission were never enforced at execution time:
HasPermissionAPI hardcoded nil request-side constraints, so the
constraint matchers in checkPermissionConstraints were unreachable dead
code and a group member could execute purchases of any size in any
provider/service/region up to the global 10M sanity cap, silently
ignoring the operator-configured spend cap (SEC-01).

Fix:
- Add Service.HasPermissionForConstraintsAPI, which evaluates every
  request-derived constraint set against the user's effective
  permissions (single fetch, union semantics per set) and fails closed
  on an empty set list or lookup error. HasPermission's matching loop
  is extracted into permissionsAllow for reuse.
- Add Handler.requirePermissionConstraints and call it from
  validateExecutePurchaseRequest with one constraint set per
  recommendation (provider/service/region/account plus the batch's
  total upfront cost, so the cap cannot be evaded by splitting), and
  from executeExchange with the AWS EC2 scope, request region, and the
  max_payment_due_usd guardrail as the amount.
- The stateless admin API key bypasses the check exactly as it
  bypasses requirePermission's per-user lookup.

Regression tests replicate the real failing scenario (constrained
permission passes the bare verb/resource gate, constraint check must
403 before any execution is persisted) and were confirmed to fail
against the pre-fix code on both paths.

Closes #1141
---
 internal/api/handler.go                       |  28 ++++
 .../api/handler_per_account_perms_test.go     |   4 +
 internal/api/handler_purchases.go             |  43 ++++++
 internal/api/handler_purchases_test.go        |  65 +++++++++
 internal/api/handler_ri_exchange.go           |  19 ++-
 internal/api/handler_ri_exchange_test.go      |  45 +++++++
 internal/api/mocks_test.go                    |  22 ++-
 internal/api/types.go                         |   7 +
 internal/auth/service_api.go                  |  27 ++++
 internal/auth/service_api_test.go             | 125 ++++++++++++++++++
 internal/auth/service_group.go                |  14 +-
 internal/server/app.go                        |   4 +
 12 files changed, 398 insertions(+), 5 deletions(-)

diff --git a/internal/api/handler.go b/internal/api/handler.go
index bfaaefcda..a3e64e229 100644
--- a/internal/api/handler.go
+++ b/internal/api/handler.go
@@ -10,6 +10,7 @@ import (
 	"time"
 
 	"github.com/LeanerCloud/CUDly/internal/accounts"
+	"github.com/LeanerCloud/CUDly/internal/auth"
 	"github.com/LeanerCloud/CUDly/internal/config"
 	"github.com/LeanerCloud/CUDly/internal/credentials"
 	"github.com/LeanerCloud/CUDly/internal/email"
@@ -242,6 +243,33 @@ func (h *Handler) requirePermission(ctx context.Context, req *events.LambdaFunct
 	return session, nil
 }
 
+// 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.
+func (h *Handler) requirePermissionConstraints(ctx context.Context, session *Session, action, resource string, constraintSets []auth.PermissionConstraints) error {
+	if session.UserID == apiKeyAdminUserID {
+		return nil
+	}
+	if h.auth == nil {
+		return fmt.Errorf("authentication service not configured")
+	}
+	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", action, resource))
+	}
+	return nil
+}
+
 // getAllowedAccounts returns the list of account IDs the user is allowed to
 // access. Empty slice means all access (Administrators-group members carry the
 // "*" wildcard, which GetAllowedAccountsAPI surfaces as unrestricted). The
diff --git a/internal/api/handler_per_account_perms_test.go b/internal/api/handler_per_account_perms_test.go
index e70c1947b..05692fa18 100644
--- a/internal/api/handler_per_account_perms_test.go
+++ b/internal/api/handler_per_account_perms_test.go
@@ -74,6 +74,10 @@ func scopedAuthMock(ctx context.Context) *MockAuthService {
 	// Grant every permission so role-gating doesn't interfere with what we
 	// actually want to test (account-level scoping).
 	m.On("HasPermissionAPI", ctx, permsScopedUserID, mock.Anything, mock.Anything).Return(true, nil)
+	// Likewise grant the SEC-01 execution-time constraint check; constraint
+	// behaviour has its own dedicated tests.
+	m.On("HasPermissionForConstraintsAPI", ctx, permsScopedUserID, mock.Anything, mock.Anything, mock.Anything).
+		Return(true, nil).Maybe()
 	m.On("GetAllowedAccountsAPI", ctx, permsScopedUserID).Return([]string{permsAccA}, nil)
 	return m
 }
diff --git a/internal/api/handler_purchases.go b/internal/api/handler_purchases.go
index 3a7cee1f1..3c381f087 100644
--- a/internal/api/handler_purchases.go
+++ b/internal/api/handler_purchases.go
@@ -1548,9 +1548,52 @@ func (h *Handler) validateExecutePurchaseRequest(ctx context.Context, req *event
 	if err := validateCapacityConsistency(execReq.Recommendations, execReq.CapacityPercent); err != nil {
 		return ExecutePurchaseRequest{}, nil, err
 	}
+	// Enforce the per-permission Constraints (MaxPurchaseAmount, Providers,
+	// Services, Regions, AccountIDs) configured on the granting
+	// execute:purchases permission (SEC-01, issue #1141). Runs after the
+	// per-rec validation above so provider tokens are already normalized.
+	// Each recommendation must individually be granted by a permission; the
+	// amount cap is checked against the batch's total upfront cost so it
+	// cannot be evaded by splitting a large purchase across recs.
+	if err := h.requirePermissionConstraints(ctx, session, "execute", "purchases", purchaseConstraintSets(execReq.Recommendations)); err != nil {
+		return ExecutePurchaseRequest{}, nil, err
+	}
 	return execReq, session, nil
 }
 
+// purchaseConstraintSets builds one auth.PermissionConstraints per
+// recommendation in a web execute request, for the SEC-01 constraint
+// enforcement in validateExecutePurchaseRequest. Single-value Provider/
+// Service/Region/AccountIDs lists make the auth service's any-overlap
+// matcher equivalent to strict containment, so a batch cannot pass on the
+// strength of one in-scope rec while another rec is out of scope.
+// MaxPurchaseAmount carries the batch's total upfront cost (the same basis
+// as the global $10M sanity cap in validateAndTotalRecommendations) on
+// every set. A rec without a CloudAccountID omits the AccountIDs dimension;
+// account scoping is independently enforced against the session's
+// allowed_accounts by validatePurchaseRecommendationScope.
+func purchaseConstraintSets(recs []config.RecommendationRecord) []auth.PermissionConstraints {
+	var totalUpfront float64
+	for i := range recs {
+		totalUpfront += recs[i].UpfrontCost
+	}
+	sets := make([]auth.PermissionConstraints, 0, len(recs))
+	for i := range recs {
+		rec := &recs[i]
+		c := auth.PermissionConstraints{
+			Providers:         []string{rec.Provider},
+			Services:          []string{rec.Service},
+			Regions:           []string{rec.Region},
+			MaxPurchaseAmount: totalUpfront,
+		}
+		if rec.CloudAccountID != nil && *rec.CloudAccountID != "" {
+			c.AccountIDs = []string{*rec.CloudAccountID}
+		}
+		sets = append(sets, c)
+	}
+	return sets
+}
+
 // normalizeCapacityPercent defaults an absent/zero capacity_percent to 100
 // and rejects anything outside [1, 100]. capacity_percent is audit-only but
 // still bounded: a value outside the range is a client bug worth surfacing
diff --git a/internal/api/handler_purchases_test.go b/internal/api/handler_purchases_test.go
index 946a456ff..f25b4c501 100644
--- a/internal/api/handler_purchases_test.go
+++ b/internal/api/handler_purchases_test.go
@@ -8,6 +8,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/LeanerCloud/CUDly/internal/auth"
 	"github.com/LeanerCloud/CUDly/internal/config"
 	"github.com/aws/aws-lambda-go/events"
 	"github.com/stretchr/testify/assert"
@@ -3327,6 +3328,7 @@ func TestHandler_executePurchase_DirectExec_NoPermission(t *testing.T) {
 	// Base execute:purchases grant — passes the validateExecutePurchaseRequest
 	// gate but does not carry execute-any or execute-own for the direct path.
 	mockAuth.On("HasPermissionAPI", ctx, userSession.UserID, "execute", "purchases").Return(true, nil)
+	mockAuth.allowConstraintChecks()
 	mockAuth.On("HasPermissionAPI", ctx, userSession.UserID, "execute-any", "purchases").Return(false, nil)
 	mockAuth.On("HasPermissionAPI", ctx, userSession.UserID, "execute-own", "purchases").Return(false, nil)
 	// Scope check: no allowed_accounts restriction for this test.
@@ -3346,6 +3348,67 @@ func TestHandler_executePurchase_DirectExec_NoPermission(t *testing.T) {
 	assert.Contains(t, ce.Error(), "execute-any or execute-own")
 }
 
+// TestHandler_executePurchase_PermissionConstraintsDenied is the SEC-01
+// (#1141) regression test: a session whose execute:purchases permission is
+// granted (the bare verb/resource gate passes, exactly as it does for a
+// constrained permission) but whose per-permission Constraints reject the
+// request must receive a 403 BEFORE any execution is persisted. Pre-fix the
+// handler never consulted the constraints, so this request was saved and
+// proceeded to the approval flow.
+func TestHandler_executePurchase_PermissionConstraintsDenied(t *testing.T) {
+	ctx := context.Background()
+	mockStore := new(MockConfigStore)
+	mockAuth := new(MockAuthService)
+	t.Cleanup(func() { mockAuth.AssertExpectations(t) })
+	// No store expectations registered: the request must be rejected before
+	// SavePurchaseExecution / GetPendingExecutions are reached.
+	t.Cleanup(func() { mockStore.AssertExpectations(t) })
+
+	userSession := &Session{
+		UserID: "dddddddd-dddd-dddd-dddd-dddddddddddd",
+		Email:  "capped@example.com",
+	}
+	mockAuth.On("ValidateSession", ctx, "capped-token").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, "execute", "purchases").Return(true, nil)
+	mockAuth.On("GetAllowedAccountsAPI", ctx, userSession.UserID).Return([]string{}, nil)
+	// The constraint check must receive one set per recommendation, each
+	// carrying the batch's TOTAL upfront cost ($3000 + $2500 = $5500) and
+	// that rec's provider/service/region as single-value lists.
+	mockAuth.On("HasPermissionForConstraintsAPI", ctx, userSession.UserID, "execute", "purchases",
+		mock.MatchedBy(func(sets []auth.PermissionConstraints) bool {
+			if len(sets) != 2 {
+				return false
+			}
+			for _, c := range sets {
+				if c.MaxPurchaseAmount != 5500.0 {
+					return false
+				}
+			}
+			return assert.ObjectsAreEqual([]string{"aws"}, sets[0].Providers) &&
+				assert.ObjectsAreEqual([]string{"ec2"}, sets[0].Services) &&
+				assert.ObjectsAreEqual([]string{"us-east-1"}, sets[0].Regions) &&
+				assert.ObjectsAreEqual([]string{"eu-west-1"}, sets[1].Regions)
+		})).Return(false, nil)
+
+	handler := &Handler{config: mockStore, auth: mockAuth}
+	req := &events.LambdaFunctionURLRequest{
+		Headers: map[string]string{"Authorization": "Bearer capped-token"},
+		Body: `{"recommendations": [
+			{"id": "rec-1", "provider": "aws", "service": "ec2", "region": "us-east-1", "count": 1, "term": 1, "payment": "all-upfront", "upfront_cost": 3000.0, "savings": 50.0},
+			{"id": "rec-2", "provider": "aws", "service": "ec2", "region": "eu-west-1", "count": 2, "term": 1, "payment": "all-upfront", "upfront_cost": 2500.0, "savings": 100.0}
+		]}`,
+	}
+	_, err := handler.executePurchase(ctx, req)
+	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_executePurchase_DirectExec_ExecuteAny verifies that a session
 // with execute-any:purchases can direct-execute a purchase (no ownership
 // check). The handler must call ApproveAndExecute and return status=completed.
@@ -3368,6 +3431,7 @@ func TestHandler_executePurchase_DirectExec_ExecuteAny(t *testing.T) {
 	// removed in issue #940 — HasPermissionAPI is now always consulted.
 	// First, the outer gate: requirePermission("execute","purchases").
 	mockAuth.On("HasPermissionAPI", ctx, adminSession.UserID, "execute", "purchases").Return(true, nil)
+	mockAuth.allowConstraintChecks()
 	// Then the direct-execute gate: authorizeSessionExecuteDirect("execute-any").
 	mockAuth.On("HasPermissionAPI", ctx, adminSession.UserID, "execute-any", "purchases").Return(true, nil)
 	// Scope check: no allowed_accounts restriction for this test.
@@ -3410,6 +3474,7 @@ func TestHandler_executePurchase_DirectExec_ExecuteOwn_Owner(t *testing.T) {
 	}
 	mockAuth.On("ValidateSession", ctx, "owner-token").Return(ownerSession, nil)
 	mockAuth.On("HasPermissionAPI", ctx, ownerID, "execute", "purchases").Return(true, nil)
+	mockAuth.allowConstraintChecks()
 	mockAuth.On("HasPermissionAPI", ctx, ownerID, "execute-any", "purchases").Return(false, nil)
 	mockAuth.On("HasPermissionAPI", ctx, ownerID, "execute-own", "purchases").Return(true, nil)
 	mockAuth.On("GetAllowedAccountsAPI", ctx, ownerID).Return([]string{}, nil)
diff --git a/internal/api/handler_ri_exchange.go b/internal/api/handler_ri_exchange.go
index ec3e117bd..4ea21aa79 100644
--- a/internal/api/handler_ri_exchange.go
+++ b/internal/api/handler_ri_exchange.go
@@ -20,6 +20,7 @@ import (
 	"github.com/LeanerCloud/CUDly/internal/auth"
 	"github.com/LeanerCloud/CUDly/internal/config"
 	"github.com/LeanerCloud/CUDly/internal/credentials"
+	"github.com/LeanerCloud/CUDly/pkg/common"
 	"github.com/LeanerCloud/CUDly/pkg/exchange"
 	"github.com/LeanerCloud/CUDly/pkg/logging"
 	awsprovider "github.com/LeanerCloud/CUDly/providers/aws"
@@ -716,7 +717,8 @@ func validateExecuteExchangeBody(body ExchangeExecuteRequestBody) error {
 // Requires execute:ri-exchange (deliberately separate from execute:purchases)
 // because RI exchanges are financially irreversible once submitted to AWS.
 func (h *Handler) executeExchange(ctx context.Context, req *events.LambdaFunctionURLRequest) (any, error) {
-	if _, err := h.requirePermission(ctx, req, "execute", "ri-exchange"); err != nil {
+	session, err := h.requirePermission(ctx, req, "execute", "ri-exchange")
+	if err != nil {
 		return nil, err
 	}
 
@@ -735,6 +737,21 @@ func (h *Handler) executeExchange(ctx context.Context, req *events.LambdaFunctio
 
 	region := body.Region
 
+	// Enforce the per-permission Constraints configured on the granting
+	// execute:ri-exchange permission (SEC-01, issue #1141). RI exchanges
+	// are AWS EC2 only and region-scoped; the amount cap is checked against
+	// the caller's max_payment_due_usd guardrail, which ExecuteExchange
+	// independently enforces against the actual quoted payment due.
+	maxPayment, _ := maxRat.Float64()
+	if err := h.requirePermissionConstraints(ctx, session, "execute", "ri-exchange", []auth.PermissionConstraints{{
+		Providers:         []string{string(common.ProviderAWS)},
+		Services:          []string{string(common.ServiceEC2)},
+		Regions:           []string{region},
+		MaxPurchaseAmount: maxPayment,
+	}}); err != nil {
+		return nil, err
+	}
+
 	exchangeID, quote, err := exchange.ExecuteExchange(ctx, exchange.ExchangeExecuteRequest{
 		Region:           region,
 		ReservedIDs:      body.RIIDs,
diff --git a/internal/api/handler_ri_exchange_test.go b/internal/api/handler_ri_exchange_test.go
index 250180c49..3ac12aebf 100644
--- a/internal/api/handler_ri_exchange_test.go
+++ b/internal/api/handler_ri_exchange_test.go
@@ -1018,6 +1018,9 @@ func (m *mockAuthForExchange) ListGroupsAPI(_ context.Context) (any, error)
 func (m *mockAuthForExchange) HasPermissionAPI(_ context.Context, _, _, _ string) (bool, error) {
 	return true, nil
 }
+func (m *mockAuthForExchange) HasPermissionForConstraintsAPI(_ context.Context, _, _, _ string, _ []auth.PermissionConstraints) (bool, error) {
+	return true, nil
+}
 func (m *mockAuthForExchange) GetUserPermissionsAPI(_ context.Context, _ string) (any, error) {
 	return nil, nil
 }
@@ -1288,6 +1291,48 @@ func TestExecuteExchange_EmptyRegionReturns400(t *testing.T) {
 	assert.Contains(t, err.Error(), "region is required")
 }
 
+// TestExecuteExchange_PermissionConstraintsDenied is the SEC-01 (#1141)
+// regression test for the exchange path: a session that passes the bare
+// execute:ri-exchange gate (as a constrained permission does) but whose
+// per-permission Constraints reject the request must receive a 403 before
+// the exchange is submitted to AWS. The constraint set must carry the AWS
+// EC2 scope, the request's region, and the max_payment_due_usd guardrail
+// as the amount.
+func TestExecuteExchange_PermissionConstraintsDenied(t *testing.T) {
+	ctx := context.Background()
+	mockAuth := new(MockAuthService)
+	t.Cleanup(func() { mockAuth.AssertExpectations(t) })
+
+	userSession := &Session{
+		UserID: "eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee",
+		Email:  "exchanger@example.com",
+	}
+	mockAuth.On("ValidateSession", ctx, "exchange-token").Return(userSession, nil)
+	mockAuth.On("HasPermissionAPI", ctx, userSession.UserID, "execute", "ri-exchange").Return(true, nil)
+	mockAuth.On("HasPermissionForConstraintsAPI", ctx, userSession.UserID, "execute", "ri-exchange",
+		mock.MatchedBy(func(sets []auth.PermissionConstraints) bool {
+			if len(sets) != 1 {
+				return false
+			}
+			c := sets[0]
+			return assert.ObjectsAreEqual([]string{"aws"}, c.Providers) &&
+				assert.ObjectsAreEqual([]string{"ec2"}, c.Services) &&
+				assert.ObjectsAreEqual([]string{"eu-central-1"}, c.Regions) &&
+				c.MaxPurchaseAmount == 250.50
+		})).Return(false, nil)
+
+	h := &Handler{auth: mockAuth}
+	_, err := h.executeExchange(ctx, &events.LambdaFunctionURLRequest{
+		Headers: map[string]string{"authorization": "Bearer exchange-token"},
+		Body:    `{"ri_ids":["ri-123"],"target_offering_id":"off-1","max_payment_due_usd":"250.50","region":"eu-central-1"}`,
+	})
+	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")
+}
+
 // TestGetExchangeQuote_EmptyRegionResolvesFromSDK pins finding 01-L4:
 // getExchangeQuote must resolve the region from the AWS SDK chain when
 // the caller omits it, matching getReshapeRecommendations, instead of
diff --git a/internal/api/mocks_test.go b/internal/api/mocks_test.go
index d3bbad7ed..da56b4569 100644
--- a/internal/api/mocks_test.go
+++ b/internal/api/mocks_test.go
@@ -3,6 +3,7 @@ package api
 import (
 	"context"
 
+	"github.com/LeanerCloud/CUDly/internal/auth"
 	"github.com/LeanerCloud/CUDly/internal/config"
 	"github.com/LeanerCloud/CUDly/internal/credentials"
 	"github.com/LeanerCloud/CUDly/internal/mocks"
@@ -251,12 +252,27 @@ func (m *MockAuthService) HasPermissionAPI(ctx context.Context, userID, action,
 	return args.Bool(0), args.Error(1)
 }
 
+func (m *MockAuthService) HasPermissionForConstraintsAPI(ctx context.Context, userID, action, resource string, constraintSets []auth.PermissionConstraints) (bool, error) {
+	args := m.Called(ctx, userID, action, resource, constraintSets)
+	return args.Bool(0), args.Error(1)
+}
+
 func (m *MockAuthService) GetUserPermissionsAPI(ctx context.Context, userID string) (any, error) {
 	args := m.Called(ctx, userID)
 	return args.Get(0), args.Error(1)
 }
 
-// grantAdmin makes every HasPermissionAPI check succeed, modeling an
+// allowConstraintChecks stubs the SEC-01 execution-time permission
+// constraint check (HasPermissionForConstraintsAPI) to succeed for any
+// request, modelling a granting permission with no Constraints configured.
+// Tests that target constraint behaviour register an explicit expectation
+// instead.
+func (m *MockAuthService) allowConstraintChecks() {
+	m.On("HasPermissionForConstraintsAPI", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+		Return(true, nil).Maybe()
+}
+
+// grantAdmin makes every HasPermissionAPI check succeed, modelling an
 // Administrators-group member. Authorization is group-membership-only after
 // issue #907, so admin-gated handlers resolve "is admin" / specific permissions
 // through HasPermissionAPI rather than a Session.Role short-circuit; tests that
@@ -267,6 +283,10 @@ func (m *MockAuthService) GetUserPermissionsAPI(ctx context.Context, userID stri
 func (m *MockAuthService) grantAdmin() {
 	m.On("HasPermissionAPI", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
 		Return(true, nil).Maybe()
+	// Admins hold {admin, *}, which grants every constraint set, so the
+	// SEC-01 execution-time constraint check succeeds too.
+	m.On("HasPermissionForConstraintsAPI", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+		Return(true, nil).Maybe()
 	// Administrators-group members carry the "*" wildcard, surfaced as
 	// unrestricted access (nil/empty). Handlers that scope by account call
 	// GetAllowedAccountsAPI after the permission check, so stub it too.
diff --git a/internal/api/types.go b/internal/api/types.go
index 9a53fa546..74dd07d10 100644
--- a/internal/api/types.go
+++ b/internal/api/types.go
@@ -6,6 +6,7 @@ import (
 	"time"
 
 	"github.com/LeanerCloud/CUDly/internal/analytics"
+	"github.com/LeanerCloud/CUDly/internal/auth"
 	"github.com/LeanerCloud/CUDly/internal/commitmentopts"
 	"github.com/LeanerCloud/CUDly/internal/config"
 	"github.com/LeanerCloud/CUDly/internal/credentials"
@@ -210,6 +211,12 @@ type AuthServiceInterface interface {
 	ListGroupsAPI(ctx context.Context) (any, error)
 	// Permission checking
 	HasPermissionAPI(ctx context.Context, userID, action, resource string) (bool, error)
+	// HasPermissionForConstraintsAPI checks action on resource against
+	// request-derived constraint sets so per-permission Constraints
+	// (MaxPurchaseAmount, Providers, Services, Regions, AccountIDs) are
+	// enforced at execution time. Every constraint set must be granted by
+	// at least one of the user's permissions (SEC-01, issue #1141).
+	HasPermissionForConstraintsAPI(ctx context.Context, userID, action, resource string, constraintSets []auth.PermissionConstraints) (bool, error)
 	// GetUserPermissionsAPI returns the effective permission set for a user
 	// (union of all group permissions). Used by GET /api/auth/me/permissions.
 	// Returns []auth.APIPermission converted to []PermissionEntry by the handler.
diff --git a/internal/auth/service_api.go b/internal/auth/service_api.go
index df1eca79a..c961e27fd 100644
--- a/internal/auth/service_api.go
+++ b/internal/auth/service_api.go
@@ -359,6 +359,33 @@ func (s *Service) HasPermissionAPI(ctx context.Context, userID, action, resource
 	return s.HasPermission(ctx, userID, action, resource, nil)
 }
 
+// HasPermissionForConstraintsAPI checks that the user holds action on
+// resource for EVERY request-derived constraint set, so per-permission
+// Constraints (MaxPurchaseAmount, Providers, Services, Regions, AccountIDs)
+// configured on a group permission are enforced at execution time instead of
+// being silently ignored (SEC-01, issue #1141). A single effective-permission
+// fetch covers all sets; each set must be granted by at least one permission
+// (different sets may be satisfied by different permissions, matching the
+// union semantics of group grants).
+//
+// Fail closed: an empty constraintSets slice is a caller bug, not a grant -
+// it returns an explicit error rather than allowing.
+func (s *Service) HasPermissionForConstraintsAPI(ctx context.Context, userID, action, resource string, constraintSets []PermissionConstraints) (bool, error) {
+	if len(constraintSets) == 0 {
+		return false, fmt.Errorf("no permission constraint sets provided for %s on %s", action, resource)
+	}
+	permissions, err := s.GetUserPermissions(ctx, userID)
+	if err != nil {
+		return false, err
+	}
+	for i := range constraintSets {
+		if !s.permissionsAllow(permissions, action, resource, &constraintSets[i]) {
+			return false, nil
+		}
+	}
+	return true, nil
+}
+
 // GetUserPermissionsAPI returns the effective permission set for a user via
 // the API. Calls GetUserPermissions (the same union path the server enforces
 // with) and converts each Permission to an APIPermission for the wire format.
diff --git a/internal/auth/service_api_test.go b/internal/auth/service_api_test.go
index 7956accc2..9c9827f04 100644
--- a/internal/auth/service_api_test.go
+++ b/internal/auth/service_api_test.go
@@ -628,6 +628,131 @@ func TestService_HasPermissionAPI(t *testing.T) {
 	})
 }
 
+// TestService_HasPermissionForConstraintsAPI is the SEC-01 (#1141) regression
+// suite: per-permission Constraints (MaxPurchaseAmount, Providers, Services,
+// Regions) configured on a group permission must be enforced when the request
+// supplies constraint sets, instead of being silently ignored the way the
+// nil-constraints HasPermissionAPI path ignores them.
+func TestService_HasPermissionForConstraintsAPI(t *testing.T) {
+	ctx := context.Background()
+
+	// stubConstrainedPurchaser models the real failing scenario from the
+	// finding: an operator scoped a group's execute:purchases permission
+	// with MaxPurchaseAmount=$1000, providers=[aws], regions=[us-east-1].
+	stubConstrainedPurchaser := func(mockStore *MockStore) {
+		user := &User{ID: "purchaser-1", GroupIDs: []string{"purchasers"}}
+		mockStore.On("GetUserByID", ctx, "purchaser-1").Return(user, nil).Once()
+		mockStore.On("GetGroup", ctx, "purchasers").Return(&Group{
+			ID:   "purchasers",
+			Name: "Purchasers",
+			Permissions: []Permission{{
+				Action:   ActionExecute,
+				Resource: ResourcePurchases,
+				Constraints: &PermissionConstraints{
+					Providers:         []string{"aws"},
+					Regions:           []string{"us-east-1"},
+					MaxPurchaseAmount: 1000,
+				},
+			}},
+		}, nil).Once()
+	}
+
+	t.Run("purchase over MaxPurchaseAmount is denied", func(t *testing.T) {
+		mockStore := new(MockStore)
+		service := createTestService(mockStore, new(MockEmailSender))
+		stubConstrainedPurchaser(mockStore)
+
+		has, err := service.HasPermissionForConstraintsAPI(ctx, "purchaser-1", ActionExecute, ResourcePurchases, []PermissionConstraints{
+			{Providers: []string{"aws"}, Regions: []string{"us-east-1"}, MaxPurchaseAmount: 5000},
+		})
+		require.NoError(t, err)
+		assert.False(t, has, "a $5000 purchase must not pass a $1000 MaxPurchaseAmount cap")
+		mockStore.AssertExpectations(t)
+	})
+
+	t.Run("purchase within all constraints is allowed", func(t *testing.T) {
+		mockStore := new(MockStore)
+		service := createTestService(mockStore, new(MockEmailSender))
+		stubConstrainedPurchaser(mockStore)
+
+		has, err := service.HasPermissionForConstraintsAPI(ctx, "purchaser-1", ActionExecute, ResourcePurchases, []PermissionConstraints{
+			{Providers: []string{"aws"}, Regions: []string{"us-east-1"}, MaxPurchaseAmount: 500},
+		})
+		require.NoError(t, err)
+		assert.True(t, has)
+		mockStore.AssertExpectations(t)
+	})
+
+	t.Run("out-of-scope provider is denied", func(t *testing.T) {
+		mockStore := new(MockStore)
+		service := createTestService(mockStore, new(MockEmailSender))
+		stubConstrainedPurchaser(mockStore)
+
+		has, err := service.HasPermissionForConstraintsAPI(ctx, "purchaser-1", ActionExecute, ResourcePurchases, []PermissionConstraints{
+			{Providers: []string{"azure"}, Regions: []string{"us-east-1"}, MaxPurchaseAmount: 500},
+		})
+		require.NoError(t, err)
+		assert.False(t, has, "an azure purchase must not pass a providers=[aws] constraint")
+		mockStore.AssertExpectations(t)
+	})
+
+	t.Run("batch with one out-of-scope set is denied", func(t *testing.T) {
+		mockStore := new(MockStore)
+		service := createTestService(mockStore, new(MockEmailSender))
+		stubConstrainedPurchaser(mockStore)
+
+		has, err := service.HasPermissionForConstraintsAPI(ctx, "purchaser-1", ActionExecute, ResourcePurchases, []PermissionConstraints{
+			{Providers: []string{"aws"}, Regions: []string{"us-east-1"}, MaxPurchaseAmount: 500},
+			{Providers: []string{"aws"}, Regions: []string{"eu-west-1"}, MaxPurchaseAmount: 500},
+		})
+		require.NoError(t, err)
+		assert.False(t, has, "every constraint set must be granted; an out-of-region rec fails the batch")
+		mockStore.AssertExpectations(t)
+	})
+
+	t.Run("admin passes any constraint set", func(t *testing.T) {
+		mockStore := new(MockStore)
+		service := createTestService(mockStore, new(MockEmailSender))
+
+		adminUser := &User{ID: "admin-123", GroupIDs: []string{DefaultAdminGroupID}}
+		mockStore.On("GetUserByID", ctx, "admin-123").Return(adminUser, nil).Once()
+		mockStore.On("GetGroup", ctx, DefaultAdminGroupID).Return(&Group{
+			ID:          DefaultAdminGroupID,
+			Permissions: []Permission{{Action: ActionAdmin, Resource: ResourceAll}},
+		}, nil).Once()
+
+		has, err := service.HasPermissionForConstraintsAPI(ctx, "admin-123", ActionExecute, ResourcePurchases, []PermissionConstraints{
+			{Providers: []string{"gcp"}, Regions: []string{"europe-west1"}, MaxPurchaseAmount: 9_999_999},
+		})
+		require.NoError(t, err)
+		assert.True(t, has)
+		mockStore.AssertExpectations(t)
+	})
+
+	t.Run("empty constraint sets fail loud", func(t *testing.T) {
+		mockStore := new(MockStore)
+		service := createTestService(mockStore, new(MockEmailSender))
+
+		has, err := service.HasPermissionForConstraintsAPI(ctx, "purchaser-1", ActionExecute, ResourcePurchases, nil)
+		require.Error(t, err, "an empty constraint-set slice is a caller bug, not a grant")
+		assert.False(t, has)
+		mockStore.AssertExpectations(t)
+	})
+
+	t.Run("store error propagates and denies", func(t *testing.T) {
+		mockStore := new(MockStore)
+		service := createTestService(mockStore, new(MockEmailSender))
+		mockStore.On("GetUserByID", ctx, "err-1").Return(nil, fmt.Errorf("database error")).Once()
+
+		has, err := service.HasPermissionForConstraintsAPI(ctx, "err-1", ActionExecute, ResourcePurchases, []PermissionConstraints{
+			{Providers: []string{"aws"}},
+		})
+		require.Error(t, err)
+		assert.False(t, has)
+		mockStore.AssertExpectations(t)
+	})
+}
+
 // Test error paths and edge cases
 
 // TestUserToAPIUser_EmptyGroups verifies the nil→[]string{} substitution
diff --git a/internal/auth/service_group.go b/internal/auth/service_group.go
index 9a983ae53..d38c915de 100644
--- a/internal/auth/service_group.go
+++ b/internal/auth/service_group.go
@@ -161,9 +161,17 @@ func (s *Service) HasPermission(ctx context.Context, userID, action, resource st
 		return false, err
 	}
 
+	return s.permissionsAllow(permissions, action, resource, constraints), nil
+}
+
+// permissionsAllow reports whether any permission in the effective set grants
+// action on resource under the given request-side constraints. Extracted from
+// HasPermission so batch callers (HasPermissionForConstraintsAPI) can evaluate
+// several constraint sets against a single permission fetch.
+func (s *Service) permissionsAllow(permissions []Permission, action, resource string, constraints *PermissionConstraints) bool {
 	for _, perm := range permissions {
 		if checkAdminPermission(perm) {
-			return true, nil
+			return true
 		}
 
 		if !checkPermissionMatch(perm, action, resource) {
@@ -174,10 +182,10 @@ func (s *Service) HasPermission(ctx context.Context, userID, action, resource st
 			continue
 		}
 
-		return true, nil
+		return true
 	}
 
-	return false, nil
+	return false
 }
 
 func checkAdminPermission(perm Permission) bool {
diff --git a/internal/server/app.go b/internal/server/app.go
index bcc390c4b..17fa87fbe 100644
--- a/internal/server/app.go
+++ b/internal/server/app.go
@@ -1138,6 +1138,10 @@ func (a *authServiceAdapter) HasPermissionAPI(ctx context.Context, userID, actio
 	return a.service.HasPermissionAPI(ctx, userID, action, resource)
 }
 
+func (a *authServiceAdapter) HasPermissionForConstraintsAPI(ctx context.Context, userID, action, resource string, constraintSets []auth.PermissionConstraints) (bool, error) {
+	return a.service.HasPermissionForConstraintsAPI(ctx, userID, action, resource, constraintSets)
+}
+
 func (a *authServiceAdapter) GetUserPermissionsAPI(ctx context.Context, userID string) (any, error) {
 	return a.service.GetUserPermissionsAPI(ctx, userID)
 }

From d867e5d42bf763c4d32a88fef66e101517031ae9 Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Wed, 10 Jun 2026 22:22:49 -0700
Subject: [PATCH 2/4] fix(auth): close AccountIDs fail-open on execute
 constraint sets

The auth matcher treats an empty request-side list as "dimension not
specified = satisfied", so constraint sets that omitted AccountIDs let
a permission constrained to specific accounts authorize the request
anyway (CR findings on PR #1210, SEC-01 / issue #1141):

- purchases: a recommendation without a cloud_account_id omitted the
  AccountIDs dimension entirely. It now always carries a single-value
  AccountIDs list, using the new unattributedAccountConstraint sentinel
  for unattributed recs so an AccountIDs-constrained permission denies
  them (fail closed) while unconstrained permissions are unaffected.
- ri-exchange: the request-derived set never populated AccountIDs. It
  now carries the registered cloud account the running deployment
  resolves to (resolveReshapeCloudAccountID), failing closed on a
  resolution error and falling back to the sentinel when the
  deployment maps to no registered account.

The sentinel is preferred over rejecting unattributed requests with a
400 because executing unattributed recommendations is a documented,
legitimate flow for sessions without account restrictions
(validatePurchaseRecommendationScope only rejects them for scoped
sessions); only permissions that actually constrain AccountIDs should
deny.

Regression tests pin the new constraint-set shape on both paths plus
the resolver fail-closed and sentinel branches; all confirmed to FAIL
against the pre-fix code and pass after.
---
 internal/api/handler.go                  | 13 ++++
 internal/api/handler_purchases.go        | 22 +++---
 internal/api/handler_purchases_test.go   | 32 ++++++++-
 internal/api/handler_ri_exchange.go      | 19 +++++-
 internal/api/handler_ri_exchange_test.go | 86 ++++++++++++++++++++++--
 5 files changed, 155 insertions(+), 17 deletions(-)

diff --git a/internal/api/handler.go b/internal/api/handler.go
index a3e64e229..f023ba8c7 100644
--- a/internal/api/handler.go
+++ b/internal/api/handler.go
@@ -243,6 +243,19 @@ func (h *Handler) requirePermission(ctx context.Context, req *events.LambdaFunct
 	return session, nil
 }
 
+// unattributedAccountConstraint is the request-side AccountIDs value passed
+// to requirePermissionConstraints when a request cannot be attributed to a
+// registered cloud account (a recommendation without a cloud_account_id, or
+// an RI exchange on a deployment whose running account is not registered).
+// The auth matcher treats an EMPTY request-side list as "dimension not
+// specified = satisfied", so omitting AccountIDs would let a permission
+// constrained to specific accounts authorize an unattributed request
+// (fail-open). Sending this sentinel keeps the list non-empty: it can never
+// equal a real cloud account UUID, so a permission constrained to real
+// accounts denies the request (fail closed), while a permission without an
+// AccountIDs constraint still matches via the empty-permission-side rule.
+const unattributedAccountConstraint = "unattributed"
+
 // requirePermissionConstraints re-checks an already-authenticated session
 // against request-derived permission constraint sets, so the Constraints
 // (MaxPurchaseAmount, Providers, Services, Regions, AccountIDs) configured on
diff --git a/internal/api/handler_purchases.go b/internal/api/handler_purchases.go
index 3c381f087..5de9d0cd8 100644
--- a/internal/api/handler_purchases.go
+++ b/internal/api/handler_purchases.go
@@ -1569,9 +1569,12 @@ func (h *Handler) validateExecutePurchaseRequest(ctx context.Context, req *event
 // strength of one in-scope rec while another rec is out of scope.
 // MaxPurchaseAmount carries the batch's total upfront cost (the same basis
 // as the global $10M sanity cap in validateAndTotalRecommendations) on
-// every set. A rec without a CloudAccountID omits the AccountIDs dimension;
-// account scoping is independently enforced against the session's
-// allowed_accounts by validatePurchaseRecommendationScope.
+// every set. AccountIDs is ALWAYS populated: a rec without a CloudAccountID
+// carries unattributedAccountConstraint so an AccountIDs-constrained
+// permission denies it (the auth matcher treats an empty request-side list
+// as satisfied, so omitting the dimension would fail open). Session-level
+// allowed_accounts scoping is independently enforced by
+// validatePurchaseRecommendationScope.
 func purchaseConstraintSets(recs []config.RecommendationRecord) []auth.PermissionConstraints {
 	var totalUpfront float64
 	for i := range recs {
@@ -1580,16 +1583,17 @@ func purchaseConstraintSets(recs []config.RecommendationRecord) []auth.Permissio
 	sets := make([]auth.PermissionConstraints, 0, len(recs))
 	for i := range recs {
 		rec := &recs[i]
-		c := auth.PermissionConstraints{
+		accountID := unattributedAccountConstraint
+		if rec.CloudAccountID != nil && *rec.CloudAccountID != "" {
+			accountID = *rec.CloudAccountID
+		}
+		sets = append(sets, auth.PermissionConstraints{
 			Providers:         []string{rec.Provider},
 			Services:          []string{rec.Service},
 			Regions:           []string{rec.Region},
+			AccountIDs:        []string{accountID},
 			MaxPurchaseAmount: totalUpfront,
-		}
-		if rec.CloudAccountID != nil && *rec.CloudAccountID != "" {
-			c.AccountIDs = []string{*rec.CloudAccountID}
-		}
-		sets = append(sets, c)
+		})
 	}
 	return sets
 }
diff --git a/internal/api/handler_purchases_test.go b/internal/api/handler_purchases_test.go
index f25b4c501..921c9738b 100644
--- a/internal/api/handler_purchases_test.go
+++ b/internal/api/handler_purchases_test.go
@@ -3376,7 +3376,9 @@ func TestHandler_executePurchase_PermissionConstraintsDenied(t *testing.T) {
 	mockAuth.On("GetAllowedAccountsAPI", ctx, userSession.UserID).Return([]string{}, nil)
 	// The constraint check must receive one set per recommendation, each
 	// carrying the batch's TOTAL upfront cost ($3000 + $2500 = $5500) and
-	// that rec's provider/service/region as single-value lists.
+	// that rec's provider/service/region as single-value lists. These recs
+	// carry no cloud_account_id, so AccountIDs must be the unattributed
+	// sentinel (never empty, which the auth matcher treats as satisfied).
 	mockAuth.On("HasPermissionForConstraintsAPI", ctx, userSession.UserID, "execute", "purchases",
 		mock.MatchedBy(func(sets []auth.PermissionConstraints) bool {
 			if len(sets) != 2 {
@@ -3386,6 +3388,9 @@ func TestHandler_executePurchase_PermissionConstraintsDenied(t *testing.T) {
 				if c.MaxPurchaseAmount != 5500.0 {
 					return false
 				}
+				if !assert.ObjectsAreEqual([]string{unattributedAccountConstraint}, c.AccountIDs) {
+					return false
+				}
 			}
 			return assert.ObjectsAreEqual([]string{"aws"}, sets[0].Providers) &&
 				assert.ObjectsAreEqual([]string{"ec2"}, sets[0].Services) &&
@@ -3409,6 +3414,31 @@ func TestHandler_executePurchase_PermissionConstraintsDenied(t *testing.T) {
 	assert.Contains(t, ce.Error(), "constraints")
 }
 
+// TestPurchaseConstraintSets_AccountDimensionAlwaysPopulated pins the SEC-01
+// fail-closed shape of the AccountIDs dimension: every constraint set must
+// carry a non-empty AccountIDs list. The auth matcher treats an empty
+// request-side list as "dimension not specified = satisfied", so a rec
+// without a cloud_account_id must carry the unattributed sentinel; omitting
+// the dimension would let an AccountIDs-constrained execute:purchases
+// permission authorize an unattributed purchase (fail-open).
+func TestPurchaseConstraintSets_AccountDimensionAlwaysPopulated(t *testing.T) {
+	attributed := "11111111-2222-3333-4444-555555555555"
+	empty := ""
+	sets := purchaseConstraintSets([]config.RecommendationRecord{
+		{Provider: "aws", Service: "ec2", Region: "us-east-1", UpfrontCost: 100, CloudAccountID: &attributed},
+		{Provider: "aws", Service: "rds", Region: "eu-west-1", UpfrontCost: 200, CloudAccountID: nil},
+		{Provider: "aws", Service: "ec2", Region: "us-east-1", UpfrontCost: 300, CloudAccountID: &empty},
+	})
+	require.Len(t, sets, 3)
+	assert.Equal(t, []string{attributed}, sets[0].AccountIDs)
+	assert.Equal(t, []string{unattributedAccountConstraint}, sets[1].AccountIDs)
+	assert.Equal(t, []string{unattributedAccountConstraint}, sets[2].AccountIDs)
+	for i, c := range sets {
+		assert.NotEmpty(t, c.AccountIDs, "set %d must always carry the AccountIDs dimension", i)
+		assert.Equal(t, 600.0, c.MaxPurchaseAmount, "set %d must carry the batch total upfront", i)
+	}
+}
+
 // TestHandler_executePurchase_DirectExec_ExecuteAny verifies that a session
 // with execute-any:purchases can direct-execute a purchase (no ownership
 // check). The handler must call ApproveAndExecute and return status=completed.
diff --git a/internal/api/handler_ri_exchange.go b/internal/api/handler_ri_exchange.go
index 4ea21aa79..1f83fe542 100644
--- a/internal/api/handler_ri_exchange.go
+++ b/internal/api/handler_ri_exchange.go
@@ -739,11 +739,24 @@ func (h *Handler) executeExchange(ctx context.Context, req *events.LambdaFunctio
 
 	// Enforce the per-permission Constraints configured on the granting
 	// execute:ri-exchange permission (SEC-01, issue #1141). RI exchanges
-	// are AWS EC2 only and region-scoped; the amount cap is checked against
-	// the caller's max_payment_due_usd guardrail, which ExecuteExchange
-	// independently enforces against the actual quoted payment due.
+	// are AWS EC2 only and region-scoped, and operate on the RIs of the
+	// deployment's own AWS account, so AccountIDs carries the registered
+	// cloud account the running deployment resolves to (fail closed on a
+	// resolution error; unattributedAccountConstraint when the deployment
+	// maps to no registered account, so an AccountIDs-constrained
+	// permission denies). The amount cap is checked against the caller's
+	// max_payment_due_usd guardrail, which ExecuteExchange independently
+	// enforces against the actual quoted payment due.
 	maxPayment, _ := maxRat.Float64()
+	cloudAccountID, err := h.resolveReshapeCloudAccountID(ctx)
+	if err != nil {
+		return nil, fmt.Errorf("failed to resolve cloud account scope: %w", err)
+	}
+	if cloudAccountID == "" {
+		cloudAccountID = unattributedAccountConstraint
+	}
 	if err := h.requirePermissionConstraints(ctx, session, "execute", "ri-exchange", []auth.PermissionConstraints{{
+		AccountIDs:        []string{cloudAccountID},
 		Providers:         []string{string(common.ProviderAWS)},
 		Services:          []string{string(common.ServiceEC2)},
 		Regions:           []string{region},
diff --git a/internal/api/handler_ri_exchange_test.go b/internal/api/handler_ri_exchange_test.go
index 3ac12aebf..de7479c65 100644
--- a/internal/api/handler_ri_exchange_test.go
+++ b/internal/api/handler_ri_exchange_test.go
@@ -1296,13 +1296,14 @@ func TestExecuteExchange_EmptyRegionReturns400(t *testing.T) {
 // execute:ri-exchange gate (as a constrained permission does) but whose
 // per-permission Constraints reject the request must receive a 403 before
 // the exchange is submitted to AWS. The constraint set must carry the AWS
-// EC2 scope, the request's region, and the max_payment_due_usd guardrail
-// as the amount.
+// EC2 scope, the request's region, the deployment's registered cloud
+// account, and the max_payment_due_usd guardrail as the amount.
 func TestExecuteExchange_PermissionConstraintsDenied(t *testing.T) {
 	ctx := context.Background()
 	mockAuth := new(MockAuthService)
 	t.Cleanup(func() { mockAuth.AssertExpectations(t) })
 
+	const deploymentAccountID = "11111111-2222-3333-4444-555555555555"
 	userSession := &Session{
 		UserID: "eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee",
 		Email:  "exchanger@example.com",
@@ -1315,13 +1316,19 @@ func TestExecuteExchange_PermissionConstraintsDenied(t *testing.T) {
 				return false
 			}
 			c := sets[0]
-			return assert.ObjectsAreEqual([]string{"aws"}, c.Providers) &&
+			return assert.ObjectsAreEqual([]string{deploymentAccountID}, c.AccountIDs) &&
+				assert.ObjectsAreEqual([]string{"aws"}, c.Providers) &&
 				assert.ObjectsAreEqual([]string{"ec2"}, c.Services) &&
 				assert.ObjectsAreEqual([]string{"eu-central-1"}, c.Regions) &&
 				c.MaxPurchaseAmount == 250.50
 		})).Return(false, nil)
 
-	h := &Handler{auth: mockAuth}
+	h := &Handler{
+		auth: mockAuth,
+		reshapeAccountResolver: func(_ context.Context) (string, error) {
+			return deploymentAccountID, nil
+		},
+	}
 	_, err := h.executeExchange(ctx, &events.LambdaFunctionURLRequest{
 		Headers: map[string]string{"authorization": "Bearer exchange-token"},
 		Body:    `{"ri_ids":["ri-123"],"target_offering_id":"off-1","max_payment_due_usd":"250.50","region":"eu-central-1"}`,
@@ -1333,6 +1340,77 @@ func TestExecuteExchange_PermissionConstraintsDenied(t *testing.T) {
 	assert.Contains(t, ce.Error(), "constraints")
 }
 
+// TestExecuteExchange_AccountResolutionErrorFailsClosed pins the fail-closed
+// behavior of the SEC-01 AccountIDs dimension: when the deployment's cloud
+// account cannot be resolved (STS error, account lookup failure), the
+// handler must abort BEFORE the constraint check and the AWS call rather
+// than evaluating constraints without the account dimension.
+func TestExecuteExchange_AccountResolutionErrorFailsClosed(t *testing.T) {
+	ctx := context.Background()
+	mockAuth := new(MockAuthService)
+	// No HasPermissionForConstraintsAPI expectation: it must NOT be called.
+	t.Cleanup(func() { mockAuth.AssertExpectations(t) })
+
+	userSession := &Session{
+		UserID: "eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee",
+		Email:  "exchanger@example.com",
+	}
+	mockAuth.On("ValidateSession", ctx, "exchange-token").Return(userSession, nil)
+	mockAuth.On("HasPermissionAPI", ctx, userSession.UserID, "execute", "ri-exchange").Return(true, nil)
+
+	h := &Handler{
+		auth: mockAuth,
+		reshapeAccountResolver: func(_ context.Context) (string, error) {
+			return "", fmt.Errorf("sts get-caller-identity denied")
+		},
+	}
+	_, err := h.executeExchange(ctx, &events.LambdaFunctionURLRequest{
+		Headers: map[string]string{"authorization": "Bearer exchange-token"},
+		Body:    `{"ri_ids":["ri-123"],"target_offering_id":"off-1","max_payment_due_usd":"250.50","region":"eu-central-1"}`,
+	})
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "resolve cloud account scope")
+}
+
+// TestExecuteExchange_UnattributedAccountStillConstrained pins the sentinel
+// behavior: when the deployment resolves to no registered cloud account
+// (non-AWS host, bootstrap), the constraint set must still carry a non-empty
+// AccountIDs list (the unattributed sentinel) so a permission constrained to
+// specific accounts denies the exchange instead of matching the auth
+// layer's "empty request list = satisfied" rule (fail closed).
+func TestExecuteExchange_UnattributedAccountStillConstrained(t *testing.T) {
+	ctx := context.Background()
+	mockAuth := new(MockAuthService)
+	t.Cleanup(func() { mockAuth.AssertExpectations(t) })
+
+	userSession := &Session{
+		UserID: "eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee",
+		Email:  "exchanger@example.com",
+	}
+	mockAuth.On("ValidateSession", ctx, "exchange-token").Return(userSession, nil)
+	mockAuth.On("HasPermissionAPI", ctx, userSession.UserID, "execute", "ri-exchange").Return(true, nil)
+	mockAuth.On("HasPermissionForConstraintsAPI", ctx, userSession.UserID, "execute", "ri-exchange",
+		mock.MatchedBy(func(sets []auth.PermissionConstraints) bool {
+			return len(sets) == 1 &&
+				assert.ObjectsAreEqual([]string{unattributedAccountConstraint}, sets[0].AccountIDs)
+		})).Return(false, nil)
+
+	h := &Handler{
+		auth: mockAuth,
+		reshapeAccountResolver: func(_ context.Context) (string, error) {
+			return "", nil
+		},
+	}
+	_, err := h.executeExchange(ctx, &events.LambdaFunctionURLRequest{
+		Headers: map[string]string{"authorization": "Bearer exchange-token"},
+		Body:    `{"ri_ids":["ri-123"],"target_offering_id":"off-1","max_payment_due_usd":"250.50","region":"eu-central-1"}`,
+	})
+	require.Error(t, err)
+	ce, ok := IsClientError(err)
+	require.True(t, ok, "expected a ClientError, got: %v", err)
+	assert.Equal(t, 403, ce.code)
+}
+
 // TestGetExchangeQuote_EmptyRegionResolvesFromSDK pins finding 01-L4:
 // getExchangeQuote must resolve the region from the AWS SDK chain when
 // the caller omits it, matching getReshapeRecommendations, instead of

From 925833061523bfc266e12720584e7c341d5e1d8e Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Fri, 19 Jun 2026 16:14:57 +0200
Subject: [PATCH 3/4] fix(auth): address CodeRabbit findings on constraint
 enforcement

- Add defensive nil-session guard in requirePermissionConstraints before
  dereferencing session.UserID (CR nitpick, handler.go:269)
- Assert Providers and Services on sets[1] in the constraint denial test
  to prevent regressions dropping those dimensions (CR outside-diff,
  handler_purchases_test.go:3362)
---
 internal/api/handler.go                           | 3 +++
 internal/api/handler_purchases_test.go            | 2 ++
 internal/config/store_postgres_recommendations.go | 3 ---
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/internal/api/handler.go b/internal/api/handler.go
index f023ba8c7..e564ce75e 100644
--- a/internal/api/handler.go
+++ b/internal/api/handler.go
@@ -267,6 +267,9 @@ const unattributedAccountConstraint = "unattributed"
 // like it bypasses requirePermission's per-user lookup. Fails closed on a
 // missing auth service or a lookup 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")
+	}
 	if session.UserID == apiKeyAdminUserID {
 		return nil
 	}
diff --git a/internal/api/handler_purchases_test.go b/internal/api/handler_purchases_test.go
index 921c9738b..fc837166f 100644
--- a/internal/api/handler_purchases_test.go
+++ b/internal/api/handler_purchases_test.go
@@ -3395,6 +3395,8 @@ func TestHandler_executePurchase_PermissionConstraintsDenied(t *testing.T) {
 			return assert.ObjectsAreEqual([]string{"aws"}, sets[0].Providers) &&
 				assert.ObjectsAreEqual([]string{"ec2"}, sets[0].Services) &&
 				assert.ObjectsAreEqual([]string{"us-east-1"}, sets[0].Regions) &&
+				assert.ObjectsAreEqual([]string{"aws"}, sets[1].Providers) &&
+				assert.ObjectsAreEqual([]string{"ec2"}, sets[1].Services) &&
 				assert.ObjectsAreEqual([]string{"eu-west-1"}, sets[1].Regions)
 		})).Return(false, nil)
 
diff --git a/internal/config/store_postgres_recommendations.go b/internal/config/store_postgres_recommendations.go
index 452c6bda4..abeb06eda 100644
--- a/internal/config/store_postgres_recommendations.go
+++ b/internal/config/store_postgres_recommendations.go
@@ -322,9 +322,6 @@ func recEffectiveSavingsPct(rec *RecommendationRecord) (float64, bool) {
 		!math.IsNaN(*rec.SavingsPercentage) && !math.IsInf(*rec.SavingsPercentage, 0) {
 		return *rec.SavingsPercentage, true
 	}
-	if rec.Term == 0 {
-		return 0, false
-	}
 	onDemand, ok := recOnDemandBaseline(rec)
 	if !ok || onDemand == 0 {
 		return 0, false

From 91ba9ff23a1c6df083752e4dbeb99f741efa3de0 Mon Sep 17 00:00:00 2001
From: Cristian Magherusan-Stanciu <cristi.magherusan@gmail.com>
Date: Fri, 19 Jun 2026 17:25:54 +0200
Subject: [PATCH 4/4] chore(rebase): restore Term guard and drop stale
 migration files

During rebase onto main, two items needed manual cleanup:

- store_postgres_recommendations.go: the `if rec.Term == 0` guard in
  recEffectiveSavingsPct was dropped when resolving the conflict with
  main's #1254 refactor (which already extracted recOnDemandBaseline).
  The guard lives in the caller per the new docstring; restore it so
  term=0 rows still return (0, false) as the test asserts.

- 000074_audit_actor_stamps.{up,down}.sql: these files were already on
  the branch at its branching point and collide with
  000074_repair_partial_migration_058_067 (already on main). They were
  renumbered to 000077 on main via fix/migration-74-collision; remove
  the stale 000074 files here so there is no duplicate migration number.
---
 internal/config/store_postgres_recommendations.go | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/internal/config/store_postgres_recommendations.go b/internal/config/store_postgres_recommendations.go
index abeb06eda..452c6bda4 100644
--- a/internal/config/store_postgres_recommendations.go
+++ b/internal/config/store_postgres_recommendations.go
@@ -322,6 +322,9 @@ func recEffectiveSavingsPct(rec *RecommendationRecord) (float64, bool) {
 		!math.IsNaN(*rec.SavingsPercentage) && !math.IsInf(*rec.SavingsPercentage, 0) {
 		return *rec.SavingsPercentage, true
 	}
+	if rec.Term == 0 {
+		return 0, false
+	}
 	onDemand, ok := recOnDemandBaseline(rec)
 	if !ok || onDemand == 0 {
 		return 0, false