fix(api): enforce four-eyes on RI exchange approval

internal/api/handler_purchases.go

@@ -651,7 +651,7 @@ func (h *Handler) approveViaToken(ctx context.Context, req *events.LambdaFunctio
 	// session; requireDifferentApprover returns 500 when mode is on and no
 	// session is available (fail-closed).
 	tokenSession := h.tryGetSession(ctx, req)
-	if err := h.requireDifferentApprover(ctx, tokenSession, execution); err != nil {
+	if err := h.requireDifferentApprover(ctx, tokenSession, execution.ExecutionID, execution.CreatedByUserID); err != nil {
 		return nil, err
 	}
 	// SEC-01 follow-up (issue #60 review): the deep-link flow always forces
@@ -809,7 +809,7 @@ func (h *Handler) authorizeApproveSession(ctx context.Context, session *Session,
 	// pass the approve-any / approve-own gate reach this check. The admin
 	// wildcard inside authorizeSessionApprove short-circuits RBAC but NOT this
 	// check; admins who created the row must disable the mode first.
-	return h.requireDifferentApprover(ctx, session, execution)
+	return h.requireDifferentApprover(ctx, session, execution.ExecutionID, execution.CreatedByUserID)
 }
 
 // authorizeScopedSessionApprove runs the account-scope gate (issue #92)
@@ -1082,15 +1082,15 @@ func (h *Handler) sendPurchaseScheduledEmail(ctx context.Context, execution *con
 //   - session == nil AND mode on: returns 500 (fail-closed; we cannot determine
 //     identity without a session; the email-token path should always carry a
 //     session when mode is on because the deep-link flow forces a login).
-//   - session present AND execution.CreatedByUserID == nil: returns 403 with a
+//   - session present AND creatorUserID == nil: returns 403 with a
 //     targeted message explaining that the legacy row predates dual-control and
 //     an admin must disable 4-eyes mode to proceed.
-//   - session present AND session.UserID == *execution.CreatedByUserID: returns 403.
-//   - session present AND session.UserID != *execution.CreatedByUserID: returns nil.
+//   - session present AND session.UserID == *creatorUserID: returns 403.
+//   - session present AND session.UserID != *creatorUserID: returns nil.
 //
 // Admin wildcard is intentionally NOT exempt: an admin who created an execution
 // must disable the mode in Settings before approving their own row.
-func (h *Handler) requireDifferentApprover(ctx context.Context, session *Session, execution *config.PurchaseExecution) error {
+func (h *Handler) requireDifferentApprover(ctx context.Context, session *Session, subjectID string, creatorUserID *string) error {
 	cfg, err := h.config.GetGlobalConfig(ctx)
 	if err != nil {
 		return fmt.Errorf("4-eyes policy check: failed to load global config: %w", err)
@@ -1101,19 +1101,19 @@ func (h *Handler) requireDifferentApprover(ctx context.Context, session *Session
 
 	// Mode is on.
 	if session == nil {
-		logging.Warnf("purchase[%s]: 4-eyes mode on but no session available; denying (fail-closed)", execution.ExecutionID)
+		logging.Warnf("4-eyes[%s]: mode on but no session available; denying (fail-closed)", subjectID)
 		return NewClientError(500, "4-eyes approval mode is enabled but no session could be resolved; sign in before approving")
 	}
 
-	if execution.CreatedByUserID == nil {
-		logging.Warnf("purchase[%s]: 4-eyes mode on; NULL creator (legacy row) attempted by user %s, denied",
-			execution.ExecutionID, session.UserID)
+	if creatorUserID == nil {
+		logging.Warnf("4-eyes[%s]: mode on; NULL creator (legacy row) attempted by user %s, denied",
+			subjectID, session.UserID)
 		return NewClientError(403, "approval declined: this execution predates the dual-control feature and has no recorded creator; an admin must disable 4-eyes mode to approve")
 	}
 
-	if session.UserID == *execution.CreatedByUserID {
-		logging.Warnf("purchase[%s]: 4-eyes mode on; creator %s attempted self-approval, denied",
-			execution.ExecutionID, session.UserID)
+	if session.UserID == *creatorUserID {
+		logging.Warnf("4-eyes[%s]: mode on; creator %s attempted self-approval, denied",
+			subjectID, session.UserID)
 		return NewClientError(403, "approval declined: 4-eyes mode requires a different approver than the requester")
 	}
 

internal/api/handler_purchases_test.go

@@ -6308,7 +6308,7 @@ func TestRequireDifferentApprover_ModeOff_AllowsSameUser(t *testing.T) {
 	mockConfig.On("GetGlobalConfig", ctx).Return(fourEyesCfgOff(), nil)
 
 	handler := &Handler{config: mockConfig}
-	err := handler.requireDifferentApprover(ctx, session, exec)
+	err := handler.requireDifferentApprover(ctx, session, exec.ExecutionID, exec.CreatedByUserID)
 	require.NoError(t, err, "mode off: creator self-approve must be allowed")
 }
 
@@ -6326,7 +6326,7 @@ func TestRequireDifferentApprover_ModeOn_DeniesSameUser(t *testing.T) {
 	mockConfig.On("GetGlobalConfig", ctx).Return(fourEyesCfgOn(), nil)
 
 	handler := &Handler{config: mockConfig}
-	err := handler.requireDifferentApprover(ctx, session, exec)
+	err := handler.requireDifferentApprover(ctx, session, exec.ExecutionID, exec.CreatedByUserID)
 	require.Error(t, err)
 	ce, ok := IsClientError(err)
 	require.True(t, ok)
@@ -6348,7 +6348,7 @@ func TestRequireDifferentApprover_ModeOn_AllowsDifferentUser(t *testing.T) {
 	mockConfig.On("GetGlobalConfig", ctx).Return(fourEyesCfgOn(), nil)
 
 	handler := &Handler{config: mockConfig}
-	err := handler.requireDifferentApprover(ctx, session, exec)
+	err := handler.requireDifferentApprover(ctx, session, exec.ExecutionID, exec.CreatedByUserID)
 	require.NoError(t, err, "mode on: different user must be allowed")
 }
 
@@ -6366,7 +6366,7 @@ func TestRequireDifferentApprover_ModeOn_DeniesAdminSelfApprove(t *testing.T) {
 	mockConfig.On("GetGlobalConfig", ctx).Return(fourEyesCfgOn(), nil)
 
 	handler := &Handler{config: mockConfig}
-	err := handler.requireDifferentApprover(ctx, session, exec)
+	err := handler.requireDifferentApprover(ctx, session, exec.ExecutionID, exec.CreatedByUserID)
 	require.Error(t, err, "mode on: admin self-approve must be denied")
 	ce, ok := IsClientError(err)
 	require.True(t, ok)
@@ -6387,7 +6387,7 @@ func TestRequireDifferentApprover_ModeOn_NullCreatorDenied(t *testing.T) {
 	mockConfig.On("GetGlobalConfig", ctx).Return(fourEyesCfgOn(), nil)
 
 	handler := &Handler{config: mockConfig}
-	err := handler.requireDifferentApprover(ctx, session, exec)
+	err := handler.requireDifferentApprover(ctx, session, exec.ExecutionID, exec.CreatedByUserID)
 	require.Error(t, err)
 	ce, ok := IsClientError(err)
 	require.True(t, ok)
@@ -6407,7 +6407,7 @@ func TestRequireDifferentApprover_NilAuth_500(t *testing.T) {
 	mockConfig.On("GetGlobalConfig", ctx).Return(fourEyesCfgOn(), nil)
 
 	handler := &Handler{config: mockConfig}
-	err := handler.requireDifferentApprover(ctx, nil, exec)
+	err := handler.requireDifferentApprover(ctx, nil, exec.ExecutionID, exec.CreatedByUserID)
 	require.Error(t, err)
 	ce, ok := IsClientError(err)
 	require.True(t, ok)

internal/api/handler_ri_exchange.go

@@ -2201,7 +2201,7 @@ func (h *Handler) approveRIExchange(ctx context.Context, req *events.LambdaFunct
 	}
 
 	if token != "" {
-		return h.approveRIExchangeViaToken(ctx, id, token)
+		return h.approveRIExchangeViaToken(ctx, req, id, token)
 	}
 
 	return h.approveRIExchangeViaSession(ctx, req, id, nil)
@@ -2210,11 +2210,16 @@ func (h *Handler) approveRIExchange(ctx context.Context, req *events.LambdaFunct
 // approveRIExchangeViaToken is the legacy email-link branch of approveRIExchange.
 // It validates the approval token, transitions the exchange to processing, and
 // executes it. Extracted to keep approveRIExchange within cyclomatic-complexity limits.
-func (h *Handler) approveRIExchangeViaToken(ctx context.Context, id, token string) (any, error) {
+func (h *Handler) approveRIExchangeViaToken(ctx context.Context, req *events.LambdaFunctionURLRequest, id, token string) (any, error) {
 	record, err := h.validateExchangeApproval(ctx, id, token)
 	if err != nil {
 		return nil, err
 	}
+	// The deep link forces a login, so the approver's session is resolvable here.
+	err = h.requireDifferentRIExchangeApprover(ctx, h.tryGetSession(ctx, req), record)
+	if err != nil {
+		return nil, err
+	}
 
 	// Token-based approval: no session user, so transitioned_by = NULL.
 	transitioned, err := h.config.TransitionRIExchangeStatus(ctx, id, "pending", "processing", nil)
@@ -2258,6 +2263,10 @@ func (h *Handler) approveRIExchangeViaSession(ctx context.Context, req *events.L
 	if err != nil {
 		return nil, err
 	}
+	err = h.requireDifferentRIExchangeApprover(ctx, session, record)
+	if err != nil {
+		return nil, err
+	}
 
 	// Session-authed approval: stamp the session user as the actor.
 	transitioned, err := h.config.TransitionRIExchangeStatus(ctx, id, "pending", "processing", resolveCreatorUserID(session))
@@ -2380,6 +2389,15 @@ func (h *Handler) authorizeSessionApproveRIExchange(ctx context.Context, session
 	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 {
+	if record.CreatedByUserID == nil {
+		return nil
+	}
+	return h.requireDifferentApprover(ctx, session, record.ID, record.CreatedByUserID)
+}
+
 // validateExchangeApproval validates ID, token, and record state for an exchange approval.
 func (h *Handler) validateExchangeApproval(ctx context.Context, id, token string) (*config.RIExchangeRecord, error) {
 	if err := validateUUID(id); err != nil {

internal/api/handler_ri_exchange_test.go

@@ -505,6 +505,138 @@ func TestApproveRIExchange_SessionApproveOwn(t *testing.T) {
 	})
 }
 
+type riApproverGrant func(m *MockAuthService, userID string)
+
+func grantRIAdmin(m *MockAuthService, _ string) { m.grantAdminPurchaser() }
+
+func grantRIApproveOwn(m *MockAuthService, userID string) {
+	m.On("HasPermissionAPI", context.Background(), userID, auth.ActionApproveAny, auth.ResourcePurchases).Return(false, nil)
+	m.On("HasPermissionAPI", context.Background(), userID, auth.ActionApproveOwn, auth.ResourcePurchases).Return(true, nil)
+}
+
+func grantRINone(m *MockAuthService, userID string) {
+	m.On("HasPermissionAPI", context.Background(), userID, mock.Anything, auth.ResourcePurchases).Return(false, nil)
+}
+
+// fourEyesRIExchangeFixture wires a pending exchange created by creatorID and a
+// session for approverID holding the given approve grant.
+func fourEyesRIExchangeFixture(t *testing.T, creatorID *string, approverID string, grant riApproverGrant, fourEyes bool) (*Handler, *MockConfigStore, string, string) {
+	t.Helper()
+	ctx := context.Background()
+	id := "550e8400-e29b-41d4-a716-446655440021"
+	token := "four-eyes-token"
+	mockStore := new(MockConfigStore)
+	mockAuth := new(MockAuthService)
+
+	mockAuth.On("ValidateSession", ctx, "sess").Return(&Session{UserID: approverID, Email: approverID + "@example.com"}, nil)
+	mockAuth.On("ValidateCSRFToken", ctx, "sess", "csrf").Return(nil)
+	grant(mockAuth, approverID)
+
+	mockStore.On("GetRIExchangeRecord", ctx, id).Return(&config.RIExchangeRecord{
+		ID:              id,
+		Status:          "pending",
+		ApprovalToken:   config.HashApprovalToken(token),
+		SourceRIIDs:     []string{"ri-1"},
+		PaymentDue:      "50.00",
+		CreatedByUserID: creatorID,
+	}, nil)
+	mockStore.On("GetGlobalConfig", ctx).Return(&config.GlobalConfig{
+		RequireDifferentApprover:    fourEyes,
+		RIExchangeMaxDailyUSD:       1000,
+		RIExchangeMaxPerExchangeUSD: 500,
+	}, nil)
+	mockStore.On("TransitionRIExchangeStatus", ctx, id, "pending", "processing", mock.Anything).
+		Return(&config.RIExchangeRecord{ID: id, Status: "processing", SourceRIIDs: []string{"ri-1"}, PaymentDue: "50.00"}, nil)
+	mockStore.On("GetRIExchangeDailySpend", mock.Anything, mock.Anything).Return("0", nil)
+	mockStore.On("FailRIExchange", ctx, id, mock.AnythingOfType("string")).Return(nil)
+	mockStore.On("StampRIExchangeApprovedBy", ctx, id, mock.Anything).Return(nil)
+
+	return &Handler{config: mockStore, auth: mockAuth}, mockStore, id, token
+}
+
+func csrfSessionReq() *events.LambdaFunctionURLRequest {
+	req := sessionReq("sess")
+	req.Headers["x-csrf-token"] = "csrf"
+	return req
+}
+
+// TestApproveRIExchange_FourEyes (issue #221): with 4-eyes on, the creator cannot
+// approve their own exchange through the session or the email-token branch.
+func TestApproveRIExchange_FourEyes(t *testing.T) {
+	ctx := context.Background()
+	creator := "creator-uuid"
+
+	denied := func(t *testing.T, mockStore *MockConfigStore, err error, code int) {
+		t.Helper()
+		require.Error(t, err)
+		ce, ok := IsClientError(err)
+		require.True(t, ok, "want ClientError, got %v", err)
+		assert.Equal(t, code, ce.code)
+		mockStore.AssertNotCalled(t, "TransitionRIExchangeStatus", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
+	}
+	approved := func(t *testing.T, mockStore *MockConfigStore, err error) {
+		t.Helper()
+		require.NoError(t, err)
+		mockStore.AssertCalled(t, "TransitionRIExchangeStatus", ctx, mock.Anything, "pending", "processing", mock.Anything)
+	}
+
+	t.Run("session: creator with approve-own denied", func(t *testing.T) {
+		h, store, id, _ := fourEyesRIExchangeFixture(t, &creator, creator, grantRIApproveOwn, true)
+		_, err := h.approveRIExchange(ctx, csrfSessionReq(), id, "")
+		denied(t, store, err, 403)
+		assert.Contains(t, err.Error(), "4-eyes mode")
+	})
+
+	t.Run("session: admin creator denied", func(t *testing.T) {
+		h, store, id, _ := fourEyesRIExchangeFixture(t, &creator, creator, grantRIAdmin, true)
+		_, err := h.approveRIExchange(ctx, csrfSessionReq(), id, "")
+		denied(t, store, err, 403)
+	})
+
+	t.Run("session: creator holding the email token still denied", func(t *testing.T) {
+		h, store, id, token := fourEyesRIExchangeFixture(t, &creator, creator, grantRIApproveOwn, true)
+		_, err := h.approveRIExchange(ctx, csrfSessionReq(), id, token)
+		denied(t, store, err, 403)
+	})
+
+	t.Run("token: creator session without approve rights denied", func(t *testing.T) {
+		h, store, id, token := fourEyesRIExchangeFixture(t, &creator, creator, grantRINone, true)
+		_, err := h.approveRIExchange(ctx, sessionReq("sess"), id, token)
+		denied(t, store, err, 403)
+		assert.Contains(t, err.Error(), "4-eyes mode")
+	})
+
+	t.Run("token: no session fails closed", func(t *testing.T) {
+		h, store, id, token := fourEyesRIExchangeFixture(t, &creator, creator, grantRIAdmin, true)
+		_, err := h.approveRIExchange(ctx, &events.LambdaFunctionURLRequest{}, id, token)
+		denied(t, store, err, 500)
+	})
+
+	t.Run("session: different approver allowed", func(t *testing.T) {
+		h, store, id, _ := fourEyesRIExchangeFixture(t, &creator, "approver-uuid", grantRIAdmin, true)
+		_, err := h.approveRIExchange(ctx, csrfSessionReq(), id, "")
+		approved(t, store, err)
+	})
+
+	t.Run("token: different approver allowed", func(t *testing.T) {
+		h, store, id, token := fourEyesRIExchangeFixture(t, &creator, "approver-uuid", grantRINone, true)
+		_, err := h.approveRIExchange(ctx, sessionReq("sess"), id, token)
+		approved(t, store, err)
+	})
+
+	t.Run("mode off: creator allowed", func(t *testing.T) {
+		h, store, id, _ := fourEyesRIExchangeFixture(t, &creator, creator, grantRIApproveOwn, false)
+		_, err := h.approveRIExchange(ctx, csrfSessionReq(), id, "")
+		approved(t, store, err)
+	})
+
+	t.Run("automated exchange without creator stays approvable", func(t *testing.T) {
+		h, store, id, _ := fourEyesRIExchangeFixture(t, nil, "approver-uuid", grantRIAdmin, true)
+		_, err := h.approveRIExchange(ctx, csrfSessionReq(), id, "")
+		approved(t, store, err)
+	})
+}
+
 // TestApproveRIExchange_LegacyTokenStillWorks verifies that the token-only path
 // continues to work for non-session callers after the dual-auth refactor (backwards-compat).
 func TestApproveRIExchange_LegacyTokenStillWorks(t *testing.T) {