fix(purchase): refuse creator-less purchase creation while four-eyes is on

internal/api/four_eyes_unattributed_creator_test.go

@@ -0,0 +1,212 @@
+package api
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/aws/aws-lambda-go/events"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+
+	"github.com/LeanerCloud/cloud-commitments-platform/internal/config"
+)
+
+// Issue #677: the stateless admin API key has no user UUID, so every pending
+// purchase execution it minted was stored with created_by_user_id NULL and
+// created_by_system false. Under 4-eyes, requireDifferentApprover and the
+// manager's checkDifferentApprover deny that row for every approver, so it
+// could only expire. The fix refuses the creating request while 4-eyes is on.
+// These tests drive the three API writers end to end with the admin key.
+
+const fourEyesAdminKey = "test-key"
+
+func adminKeyRequest(body string) *events.LambdaFunctionURLRequest {
+	return &events.LambdaFunctionURLRequest{
+		Headers: map[string]string{"x-api-key": fourEyesAdminKey},
+		Body:    body,
+	}
+}
+
+// errPersistReached is returned by a registered WithTx expectation in the
+// mode-off controls: getting it back proves the request passed the new guard
+// and reached the write.
+var errPersistReached = NewClientError(418, "test sentinel: persistence reached")
+
+func requireFourEyesUnattributedRefusal(t *testing.T, err error) {
+	t.Helper()
+	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(), "no attributable user")
+}
+
+func TestExecutePurchase_AdminAPIKey_FourEyesOn_RefusedBeforeAnyWrite(t *testing.T) {
+	for _, body := range map[string]string{
+		"approval email": `{"recommendations":[{"id":"rec-1","provider":"aws","service":"ec2","count":1,"term":1,"payment":"all-upfront","upfront_cost":500.0,"savings":100.0}]}`,
+		"direct execute": directExecRecBody,
+	} {
+		ctx := context.Background()
+		store := new(MockConfigStore)
+		store.On("GetGlobalConfig", ctx).Return(fourEyesCfgOn(), nil)
+		expectStoredRecs(store, config.RecommendationRecord{
+			ID: "rec-1", Provider: "aws", Service: "ec2", Count: 1, Term: 1, Payment: "all-upfront", UpfrontCost: 500, Savings: 100,
+		})
+		handler := &Handler{config: store, apiKey: fourEyesAdminKey}
+
+		_, err := handler.executePurchase(ctx, adminKeyRequest(body))
+
+		requireFourEyesUnattributedRefusal(t, err)
+		store.AssertNotCalled(t, "WithTx")
+		store.AssertNotCalled(t, "SavePurchaseExecutionTx")
+		store.AssertNotCalled(t, "SavePurchaseExecution")
+	}
+}
+
+func TestExecutePurchase_AdminAPIKey_FourEyesOff_ReachesPersistence(t *testing.T) {
+	ctx := context.Background()
+	store := new(MockConfigStore)
+	store.On("GetGlobalConfig", ctx).Return(fourEyesCfgOff(), nil)
+	expectStoredRecs(store, config.RecommendationRecord{
+		ID: "rec-1", Provider: "aws", Service: "ec2", Count: 1, Term: 1, Payment: "all-upfront", UpfrontCost: 500, Savings: 100,
+	})
+	store.On("WithTx", ctx, mock.Anything).Return(errPersistReached)
+	handler := &Handler{config: store, apiKey: fourEyesAdminKey}
+
+	_, err := handler.executePurchase(ctx, adminKeyRequest(`{"recommendations":[{"id":"rec-1","provider":"aws","service":"ec2","count":1,"term":1,"payment":"all-upfront","upfront_cost":500.0,"savings":100.0}]}`))
+
+	require.ErrorIs(t, err, errPersistReached)
+}
+
+func TestRetryPurchase_UnattributedSession_FourEyesOn_RefusedBeforeAnyWrite(t *testing.T) {
+	creator := retryOtherID
+	failed := &config.PurchaseExecution{
+		ExecutionID:     retryExecID,
+		Status:          "failed",
+		Error:           "send failed: transient SES throttle",
+		CreatedByUserID: &creator,
+		Recommendations: []config.RecommendationRecord{{Provider: "aws", Service: "rds", Term: 1, UpfrontCost: 100}},
+	}
+	ctx := context.Background()
+	store := new(MockConfigStore)
+	store.On("GetExecutionByID", ctx, retryExecID).Return(failed, nil)
+	store.On("GetGlobalConfig", ctx).Return(fourEyesCfgOn(), nil)
+	handler := &Handler{config: store, auth: unattributedRetryAuth(t)}
+
+	_, err := handler.retryPurchase(ctx, sessionRetryReq(), retryExecID)
+
+	requireFourEyesUnattributedRefusal(t, err)
+	store.AssertNotCalled(t, "WithTx")
+	store.AssertNotCalled(t, "SavePurchaseExecutionTx")
+	store.AssertNotCalled(t, "LinkRetryExecutionAtomic")
+}
+
+func TestRetryPurchase_UnattributedSession_FourEyesOff_ReachesPersistence(t *testing.T) {
+	creator := retryOtherID
+	failed := &config.PurchaseExecution{
+		ExecutionID:     retryExecID,
+		Status:          "failed",
+		Error:           "send failed: transient SES throttle",
+		CreatedByUserID: &creator,
+		Recommendations: []config.RecommendationRecord{{Provider: "aws", Service: "rds", Term: 1, UpfrontCost: 100}},
+	}
+	ctx := context.Background()
+	store := new(MockConfigStore)
+	store.On("GetExecutionByID", ctx, retryExecID).Return(failed, nil)
+	store.On("GetGlobalConfig", ctx).Return(fourEyesCfgOff(), nil)
+	store.On("WithTx", ctx, mock.Anything).Return(errPersistReached)
+	handler := &Handler{config: store, auth: unattributedRetryAuth(t)}
+
+	_, err := handler.retryPurchase(ctx, sessionRetryReq(), retryExecID)
+
+	require.ErrorIs(t, err, errPersistReached)
+}
+
+// unattributedRetryAuth resolves the retry request's bearer token to a session
+// whose UserID is not a user UUID. The retry route needs a bearer session, so
+// the admin API key cannot reach it today (it gets a 401); this covers any
+// session type that would still stamp a NULL creator onto the successor.
+func unattributedRetryAuth(t *testing.T) *MockAuthService {
+	t.Helper()
+	session := &Session{UserID: "service-principal"}
+	auth := new(MockAuthService)
+	auth.On("ValidateSession", mock.Anything, "sess-tok").Return(session, nil)
+	auth.On("HasPermissionAPI", mock.Anything, session.UserID, "retry-any", "purchases").Return(true, nil).Maybe()
+	auth.On("GetAllowedAccountsAPI", mock.Anything, session.UserID).Return([]string{}, nil).Maybe()
+	auth.allowConstraintChecks()
+	return auth
+}
+
+const fourEyesPlanID = "11111111-1111-1111-1111-111111111111"
+
+func TestCreatePlannedPurchases_AdminAPIKey_FourEyesOn_RefusedBeforeAnyWrite(t *testing.T) {
+	ctx := context.Background()
+	store := new(MockConfigStore)
+	store.On("GetGlobalConfig", ctx).Return(fourEyesCfgOn(), nil)
+	handler := &Handler{config: store, apiKey: fourEyesAdminKey}
+
+	_, err := handler.createPlannedPurchases(ctx, adminKeyRequest(`{"count": 1, "start_date": "2030-01-01"}`), fourEyesPlanID)
+
+	requireFourEyesUnattributedRefusal(t, err)
+	store.AssertNotCalled(t, "WithTx")
+	store.AssertNotCalled(t, "SavePurchaseExecutionTx")
+}
+
+func TestCreatePlannedPurchases_AdminAPIKey_FourEyesOff_ReachesPersistence(t *testing.T) {
+	ctx := context.Background()
+	store := new(MockConfigStore)
+	store.On("GetGlobalConfig", ctx).Return(fourEyesCfgOff(), nil)
+	store.On("GetPurchasePlan", ctx, fourEyesPlanID).Return(&config.PurchasePlan{ID: fourEyesPlanID, Enabled: true}, nil)
+	store.On("WithTx", ctx, mock.Anything).Return(errPersistReached)
+	handler := &Handler{config: store, apiKey: fourEyesAdminKey}
+
+	_, err := handler.createPlannedPurchases(ctx, adminKeyRequest(`{"count": 1, "start_date": "2030-01-01"}`), fourEyesPlanID)
+
+	require.ErrorIs(t, err, errPersistReached)
+}
+
+// The guard itself, for the cases the endpoint tests above do not reach.
+func TestRequireAttributablePurchaseCreator(t *testing.T) {
+	ctx := context.Background()
+	adminKey := &Session{UserID: apiKeyAdminUserID}
+
+	t.Run("user session passes without reading config", func(t *testing.T) {
+		store := new(MockConfigStore)
+		h := &Handler{config: store}
+		require.NoError(t, h.requireAttributablePurchaseCreator(ctx, &Session{UserID: retryCallerID, Email: "u@example.com"}))
+		require.NoError(t, h.requireAttributablePurchaseCreator(ctx, &Session{UserID: retryCallerID, UserAPIKeyID: "key-1"}))
+		store.AssertNotCalled(t, "GetGlobalConfig")
+	})
+
+	t.Run("config load failure fails closed", func(t *testing.T) {
+		store := new(MockConfigStore)
+		store.On("GetGlobalConfig", ctx).Return(nil, errors.New("db down"))
+		h := &Handler{config: store}
+		err := h.requireAttributablePurchaseCreator(ctx, adminKey)
+		require.Error(t, err)
+		assert.Contains(t, err.Error(), "4-eyes policy check")
+	})
+
+	t.Run("nil config means 4-eyes off", func(t *testing.T) {
+		store := new(MockConfigStore)
+		store.On("GetGlobalConfig", ctx).Return(nil, nil)
+		h := &Handler{config: store}
+		require.NoError(t, h.requireAttributablePurchaseCreator(ctx, adminKey))
+	})
+
+	t.Run("nil session under 4-eyes is refused", func(t *testing.T) {
+		store := new(MockConfigStore)
+		store.On("GetGlobalConfig", ctx).Return(fourEyesCfgOn(), nil)
+		h := &Handler{config: store}
+		requireFourEyesUnattributedRefusal(t, h.requireAttributablePurchaseCreator(ctx, nil))
+	})
+
+	t.Run("non-UUID user id under 4-eyes is refused", func(t *testing.T) {
+		store := new(MockConfigStore)
+		store.On("GetGlobalConfig", ctx).Return(fourEyesCfgOn(), nil)
+		h := &Handler{config: store}
+		requireFourEyesUnattributedRefusal(t, h.requireAttributablePurchaseCreator(ctx, &Session{UserID: "not-a-uuid"}))
+	})
+}

internal/api/handler_plans.go

@@ -457,7 +457,10 @@ func (h *Handler) createPlannedPurchases(ctx context.Context, httpReq *events.La
 	// hold this verb; Plan Authors (create/update/delete:plans only)
 	// do not, closing issue #1406 / #1418. The prior gate ("create","plans")
 	// incorrectly admitted Plan Authors because they hold create:plans.
-	session, err := h.requirePermission(ctx, httpReq, "update", "purchases")
+	// Scheduled rows are stamped with resolveCreatorUserID(session); a
+	// creator-less row could never be approved under 4-eyes, so that case
+	// is refused here (issue #677).
+	session, err := h.requirePurchaseCreatingPermission(ctx, httpReq, "update", "purchases")
 	if err != nil {
 		return nil, err
 	}

internal/api/handler_purchases.go

@@ -2372,6 +2372,14 @@ func (h *Handler) persistRetryExecution(ctx context.Context, failedExec *config.
 	// "successor" is a footgun we want to remove at the source.
 	copiedRecs := append([]config.RecommendationRecord(nil), failedExec.Recommendations...)
 
+	// The successor is stamped with the retrying session's creator below. A
+	// creator-less successor under 4-eyes could never be approved, and the
+	// linkage written in the same tx would also block retrying the original
+	// again, so refuse before anything is written (issue #677).
+	if err := h.requireAttributablePurchaseCreator(ctx, session); err != nil {
+		return nil, "", err
+	}
+
 	// Generate a crypto/rand-backed approval token for the retry execution
 	// (issue #408). uuid.New().String() was used here previously, which only
 	// provides 122 bits of entropy in a known format; common.GenerateApprovalToken
@@ -2791,7 +2799,9 @@ type ExecutePurchaseRequest struct {
 // the per-rec validation, for the response to surface to the caller (#1503
 // follow-up); it is empty/nil when every rec was already canonical.
 func (h *Handler) validateExecutePurchaseRequest(ctx context.Context, req *events.LambdaFunctionURLRequest) (ExecutePurchaseRequest, *Session, []PaymentAdjustment, error) {
-	session, err := h.requirePermission(ctx, req, "execute", "purchases")
+	// Refuses a creator-less submit under 4-eyes before anything is
+	// persisted, on both the approval-email and direct-execute paths (#677).
+	session, err := h.requirePurchaseCreatingPermission(ctx, req, "execute", "purchases")
 	if err != nil {
 		return ExecutePurchaseRequest{}, nil, nil, err
 	}
@@ -3073,6 +3083,52 @@ func resolveCreatorUserID(session *Session) *string {
 	return &uid
 }
 
+// requireAttributablePurchaseCreator refuses, while 4-eyes mode is on, any
+// request that would mint a pending purchase execution with no recorded
+// creator (issue #677). resolveCreatorUserID stores NULL for a session whose
+// UserID is not a user UUID, which in practice is the stateless admin API
+// key. Under 4-eyes, requireDifferentApprover and the manager's
+// checkDifferentApprover deny a NULL-creator row that is not
+// created_by_system for every approver, so such a row could only sit pending
+// until it expired (and, on the direct-execute path, be refused after it was
+// already written). Refusing up front gives the caller a clear error instead.
+// The key is never stamped with a human creator on its behalf: that would let
+// that human's colleague "approve" a purchase no human requested, or let the
+// key holder approve their own request.
+//
+// Sessions with a user UUID (signed-in users and per-user API keys) pass
+// without a config read. A config-load failure denies (fail closed), matching
+// requireDifferentApprover. A nil config means 4-eyes is off, also matching it.
+func (h *Handler) requireAttributablePurchaseCreator(ctx context.Context, session *Session) error {
+	if resolveCreatorUserID(session) != nil {
+		return nil
+	}
+	cfg, err := h.config.GetGlobalConfig(ctx)
+	if err != nil {
+		return fmt.Errorf("4-eyes policy check: failed to load global config: %w", err)
+	}
+	if cfg == nil || !cfg.RequireDifferentApprover {
+		return nil
+	}
+	logging.Warnf("4-eyes: mode on; refusing to create a purchase execution with no attributable creator")
+	return NewClientError(403, "4-eyes approval mode is enabled and this request has no attributable user (for example the admin API key), so the purchase it creates could never be approved; sign in or use a per-user API key instead")
+}
+
+// requirePurchaseCreatingPermission is requirePermission for endpoints that
+// mint pending purchase executions: after the RBAC check it applies
+// requireAttributablePurchaseCreator so no NULL-creator row is created while
+// 4-eyes mode is on (issue #677).
+func (h *Handler) requirePurchaseCreatingPermission(ctx context.Context, req *events.LambdaFunctionURLRequest, action, resource string) (*Session, error) {
+	session, err := h.requirePermission(ctx, req, action, resource)
+	if err != nil {
+		return nil, err
+	}
+	if err := h.requireAttributablePurchaseCreator(ctx, session); err != nil {
+		return nil, err
+	}
+	return session, nil
+}
+
 // fourEyesActorIdentity returns the identity string passed as the actor to
 // purchase.Manager.ApproveAndExecute for the 4-eyes dual-control comparison
 // (issue #1005 / PR #1500 adversarial review). Real human sessions always

internal/api/openapi.yaml

@@ -481,7 +481,16 @@ paths:
         '401':
           $ref: '#/components/responses/Unauthorized'
         '403':
-          $ref: '#/components/responses/Forbidden'
+          description: >
+            CSRF validation failed, the caller lacks permission or access to
+            the plan, or 4-eyes mode is on and the caller has no attributable
+            user (the admin API key). Rows scheduled that way could never be
+            approved under 4-eyes, so the request is refused; sign in or use a
+            per-user API key instead.
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
         '404':
           $ref: '#/components/responses/NotFound'
         '409':
@@ -522,7 +531,17 @@ paths:
         '401':
           $ref: '#/components/responses/Unauthorized'
         '403':
-          $ref: '#/components/responses/Forbidden'
+          description: >
+            CSRF validation failed, the caller lacks permission or exceeds its
+            Constraints, 4-eyes mode rejects a direct execute, or 4-eyes mode is
+            on and the caller has no attributable user (the admin API key). An
+            execution created that way could never be approved under 4-eyes,
+            so it is refused before anything is written; sign in or use a
+            per-user API key instead.
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
         '409':
           description: >
             One of two conflict families. Idempotency: an identical submit