fix(api): refuse direct RI exchange execution while 4-eyes mode is on

internal/api/handler_ri_exchange.go

@@ -1202,6 +1202,11 @@ func (h *Handler) executeAzureExchange(ctx context.Context, req *events.LambdaFu
 		return nil, err
 	}
 
+	err = h.requireNoDirectExchangeUnderFourEyes(ctx)
+	if err != nil {
+		return nil, err
+	}
+
 	targets, err := toAzureExchangeTargets(body.Targets, body.SubscriptionID)
 	if err != nil {
 		return nil, err
@@ -1898,28 +1903,7 @@ 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, 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.resolveReshapeCloudAccountIDOrUnattributed(ctx)
-	if err != nil {
-		return nil, fmt.Errorf("failed to resolve cloud account scope: %w", err)
-	}
-	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},
-		MaxPurchaseAmount: maxPayment,
-	}})
+	cloudAccountID, err := h.authorizeAWSExchangeExecution(ctx, session, region, maxRat)
 	if err != nil {
 		return nil, err
 	}
@@ -2486,6 +2470,60 @@ func (h *Handler) authorizeSessionApproveRIExchange(ctx context.Context, session
 	return nil
 }
 
+// authorizeAWSExchangeExecution applies the gates on the direct AWS execute
+// endpoint that follow session permission and scope: the permission
+// Constraints, then the 4-eyes policy. It returns the deployment's cloud
+// account ID for the idempotency key.
+func (h *Handler) authorizeAWSExchangeExecution(ctx context.Context, session *Session, region string, maxRat *big.Rat) (string, error) {
+	// 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, 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.resolveReshapeCloudAccountIDOrUnattributed(ctx)
+	if err != nil {
+		return "", fmt.Errorf("failed to resolve cloud account scope: %w", err)
+	}
+	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},
+		MaxPurchaseAmount: maxPayment,
+	}})
+	if err != nil {
+		return "", err
+	}
+
+	err = h.requireNoDirectExchangeUnderFourEyes(ctx)
+	if err != nil {
+		return "", err
+	}
+	return cloudAccountID, nil
+}
+
+// requireNoDirectExchangeUnderFourEyes refuses the direct execute endpoints
+// while 4-eyes mode is on. A direct execute quotes and runs an irreversible
+// exchange in one request, so there is no pending record and no second person
+// to approve it; the approval flow (pending record + different approver) is the
+// only path that satisfies the policy.
+func (h *Handler) requireNoDirectExchangeUnderFourEyes(ctx context.Context) error {
+	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 NewClientError(403, "direct RI exchange execution is disabled while 4-eyes approval mode is on; use the approval flow")
+	}
+	return nil
+}
+
 // requireDifferentRIExchangeApprover applies the purchase 4-eyes check when a creator
 // is recorded; automated exchanges have none and stay approvable (see issue #233).
 func (h *Handler) requireDifferentRIExchangeApprover(ctx context.Context, session *Session, record *config.RIExchangeRecord) error {

internal/api/handler_ri_exchange_azure_test.go

@@ -2223,3 +2223,32 @@ func TestExchangeRegions(t *testing.T) {
 		})
 	}
 }
+
+// TestExecuteAzureExchange_FourEyesRefusesDirectExecute pins issue #517 for
+// Azure: with RequireDifferentApprover on, the direct execute endpoint refuses
+// after authorization and before any quote or exchange call. The ops client
+// registers only the ownership listing, so a CalculateExchange or
+// ExecuteExchange call would fail the strict mock.
+func TestExecuteAzureExchange_FourEyesRefusesDirectExecute(t *testing.T) {
+	ctx := context.Background()
+	opsClient := new(mockAzureExchangeOpsClient)
+	ownsAzureSource(opsClient)
+	t.Cleanup(func() { opsClient.AssertExpectations(t) })
+
+	h := newAzureExecuteSourceGateHandler(t, opsClient)
+	store, ok := h.config.(*MockConfigStore)
+	require.True(t, ok)
+	store.On("GetGlobalConfig", mock.Anything).Return(&config.GlobalConfig{RequireDifferentApprover: true}, nil)
+
+	_, err := h.executeAzureExchange(ctx, &events.LambdaFunctionURLRequest{
+		Headers: map[string]string{"authorization": "Bearer tok"},
+		Body:    validAzureExecuteBody,
+	})
+	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.Equal(t, "direct RI exchange execution is disabled while 4-eyes approval mode is on; use the approval flow", ce.Error())
+	opsClient.AssertNotCalled(t, "ExecuteExchange", mock.Anything, mock.Anything)
+	opsClient.AssertNotCalled(t, "CalculateExchange", mock.Anything, mock.Anything, mock.Anything)
+}

internal/api/handler_ri_exchange_test.go

@@ -2552,3 +2552,69 @@ func TestExecuteApprovedExchange_LedgerWriteFailure_ReturnsError(t *testing.T) {
 	assert.Contains(t, err.Error(), "exch-h4-test",
 		"error must include the exchange ID for operator correlation with AWS")
 }
+
+// fourEyesExchangeBody is a valid AWS execute body used by the 4-eyes gate tests.
+const fourEyesExchangeBody = `{"ri_ids":["ri-123"],"target_offering_id":"off-1","target_count":1,"max_payment_due_usd":"250.50","region":"eu-central-1"}`
+
+// newFourEyesExecuteExchangeHandler builds an AWS handler whose session clears
+// permission, account scope and constraints, so the only remaining gate before
+// any exchange side effect is the 4-eyes policy under test.
+func newFourEyesExecuteExchangeHandler(t *testing.T, store *MockConfigStore) *Handler {
+	t.Helper()
+	ctx := context.Background()
+	const deploymentAccountID = "11111111-2222-3333-4444-555555555555"
+
+	mockAuth := new(MockAuthService)
+	t.Cleanup(func() { mockAuth.AssertExpectations(t) })
+	mockAuth.On("ValidateSession", ctx, "exchange-token").Return(&Session{UserID: "eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee"}, nil)
+	mockAuth.On("HasPermissionAPI", ctx, "eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee", "execute", "ri-exchange").Return(true, nil)
+	allowAnyAccountScope(mockAuth)
+	mockAuth.On("HasPermissionForConstraintsAPI", ctx, "eeeeeeee-eeee-eeee-eeee-eeeeeeeeeeee", "execute", "ri-exchange", mock.Anything).Return(true, nil)
+
+	return &Handler{
+		auth:   mockAuth,
+		config: store,
+		reshapeAccountResolver: func(_ context.Context) (string, error) {
+			return deploymentAccountID, nil
+		},
+	}
+}
+
+// TestExecuteExchange_FourEyesRefusesDirectExecute pins issue #517: with
+// RequireDifferentApprover on, the direct AWS execute endpoint must refuse
+// before any exchange is attempted. The handler has no exchange client or
+// idempotency store wired, so any code path past the gate would panic.
+func TestExecuteExchange_FourEyesRefusesDirectExecute(t *testing.T) {
+	ctx := context.Background()
+	store := &MockConfigStore{}
+	store.On("GetGlobalConfig", mock.Anything).Return(&config.GlobalConfig{RequireDifferentApprover: true}, nil)
+	h := newFourEyesExecuteExchangeHandler(t, store)
+
+	_, err := h.executeExchange(ctx, &events.LambdaFunctionURLRequest{
+		Headers: map[string]string{"authorization": "Bearer exchange-token"},
+		Body:    fourEyesExchangeBody,
+	})
+	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.Equal(t, "direct RI exchange execution is disabled while 4-eyes approval mode is on; use the approval flow", ce.Error())
+}
+
+// TestExecuteExchange_FourEyesPolicyLookupErrorFailsClosed: an unreadable
+// global config must not be treated as "4-eyes off".
+func TestExecuteExchange_FourEyesPolicyLookupErrorFailsClosed(t *testing.T) {
+	ctx := context.Background()
+	store := &MockConfigStore{}
+	store.On("GetGlobalConfig", mock.Anything).Return(nil, errors.New("db down"))
+	h := newFourEyesExecuteExchangeHandler(t, store)
+
+	_, err := h.executeExchange(ctx, &events.LambdaFunctionURLRequest{
+		Headers: map[string]string{"authorization": "Bearer exchange-token"},
+		Body:    fourEyesExchangeBody,
+	})
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "4-eyes policy check")
+	_, isClient := IsClientError(err)
+	assert.False(t, isClient, "a lookup failure is a server error, not a client refusal")
+}