fix(purchase): gate scheduler/SQS on typed web source constant (follow-up to #1456)

executableByScheduler gated on the bare literal exec.Source == "web", but
web-originated executions are persisted with Source = common.PurchaseSourceWeb
("cudly-web"). The literal never matched, so a pending/notified web row on a
plan with AutoPurchase=true could be auto-executed by the cron sweep and the
SQS execute_purchase path WITHOUT the required token-link human approval -- a
fail-open on a money path (CodeRabbit CRITICAL thread on #1456, left unaddressed).

Fix:
- manager.go: compare against the typed common.PurchaseSourceWeb constant, not
  a stringly-typed literal (repo enum convention).
- manager_test.go / messages_test.go: the prior regression tests set
  Source: "web" (matching the buggy gate) and relied on
  AssertNotCalled("GetPurchasePlan"); the mock's default GetPurchasePlan stub
  does not record the call, so those tests could never detect the bypass. Use
  the real persisted value and assert the invariant via a GetPurchasePlanFn
  sentinel: web rows must short-circuit before the AutoPurchase plan fetch.
  Verified the tests FAIL on the buggy literal and PASS on the fix.

Author: cristim

internal/purchase/manager.go

@@ -11,6 +11,7 @@ import (
 	"github.com/LeanerCloud/CUDly/internal/credentials"
 	"github.com/LeanerCloud/CUDly/internal/email"
 	"github.com/LeanerCloud/CUDly/internal/oidc"
+	"github.com/LeanerCloud/CUDly/pkg/common"
 	"github.com/LeanerCloud/CUDly/pkg/logging"
 	"github.com/LeanerCloud/CUDly/pkg/provider"
 	"github.com/aws/aws-sdk-go-v2/aws"
@@ -494,16 +495,21 @@ func (m *Manager) RecoverStrandedApprovals(ctx context.Context) (int, error) {
 // an explicit human approval action (fail closed on money paths).
 //
 // Rules:
-//   - source="web" rows must wait for the token-link approval path; the
-//     scheduler and SQS paths must never bypass that gate.
+//   - web-submitted rows (Source == common.PurchaseSourceWeb, "cudly-web") must
+//     wait for the token-link approval path; the scheduler and SQS paths must
+//     never bypass that gate.
 //   - All other pending/notified rows require the owning plan to have
 //     AutoPurchase=true. A plan-fetch error is propagated so the caller can
 //     fail closed rather than defaulting to "execute".
 //
 // "approved" rows are handled by the session/token approval paths and
 // RecoverStrandedApprovals; this helper is only called for pending/notified.
 func (m *Manager) executableByScheduler(ctx context.Context, exec *config.PurchaseExecution) (bool, error) {
-	if exec.Source == "web" {
+	// Compare against the typed source constant, not a bare "web" literal: the
+	// persisted value is "cudly-web" (common.PurchaseSourceWeb), so the old
+	// literal never matched and web rows could be auto-executed without the
+	// token-link approval when AutoPurchase=true (fail-open on a money path).
+	if exec.Source == common.PurchaseSourceWeb {
 		return false, nil
 	}
 	plan, err := m.config.GetPurchasePlan(ctx, exec.PlanID)

internal/purchase/manager_test.go

@@ -386,9 +386,12 @@ func TestManager_ProcessScheduledPurchases_PendingAutoFalseSkipped(t *testing.T)
 }
 
 // TestManager_ProcessScheduledPurchases_WebSourceSkipped verifies that a
-// pending execution with Source="web" is never auto-executed by the cron
-// sweep, even when the plan has AutoPurchase=true. Web-submitted rows must
-// wait for the token-link approval path.
+// pending execution with the canonical web source (common.PurchaseSourceWeb,
+// "cudly-web") is never auto-executed by the cron sweep, even when the plan
+// has AutoPurchase=true. Web-submitted rows must wait for the token-link
+// approval path. Uses the real persisted value, not a bare "web" literal, so
+// the test reproduces the production data shape (regression guard for the
+// stringly-typed gate that let "cudly-web" rows through).
 func TestManager_ProcessScheduledPurchases_WebSourceSkipped(t *testing.T) {
 	ctx := context.Background()
 	mockStore := new(MockConfigStore)
@@ -400,15 +403,24 @@ func TestManager_ProcessScheduledPurchases_WebSourceSkipped(t *testing.T) {
 			ExecutionID:   "exec-web",
 			PlanID:        "plan-web",
 			Status:        "pending",
-			Source:        "web",
+			Source:        common.PurchaseSourceWeb,
 			ScheduledDate: pastDate,
 		},
 	}
 
 	mockStore.On("GetStaleApprovedExecutions", ctx, mock.Anything).Return([]config.PurchaseExecution{}, nil)
 	mockStore.On("GetPendingExecutions", ctx).Return(executions, nil)
-	// executableByScheduler short-circuits on Source="web" — GetPurchasePlan must NOT be called.
-	// TransitionExecutionStatus must NOT be called.
+
+	// Sentinel: web rows must short-circuit in executableByScheduler BEFORE the
+	// AutoPurchase plan fetch. GetPurchasePlanFn returns AutoPurchase=true (the
+	// dangerous case: if the gate is bypassed the row would execute), and flips
+	// planFetched. The mock's default GetPurchasePlan stub does not record the
+	// call, so AssertNotCalled alone cannot detect the bypass — hence this flag.
+	planFetched := false
+	mockStore.GetPurchasePlanFn = func(_ context.Context, planID string) (*config.PurchasePlan, error) {
+		planFetched = true
+		return &config.PurchasePlan{ID: planID, AutoPurchase: true}, nil
+	}
 
 	manager := &Manager{
 		config:       mockStore,
@@ -421,9 +433,11 @@ func TestManager_ProcessScheduledPurchases_WebSourceSkipped(t *testing.T) {
 	assert.Equal(t, 0, result.Processed, "web-sourced rows must not be auto-executed")
 	assert.Equal(t, 0, result.Executed)
 	assert.Equal(t, 0, result.Failed)
+	assert.False(t, planFetched,
+		"web rows must short-circuit before the AutoPurchase plan fetch; a bare \"web\" "+
+			"literal gate lets the persisted \"cudly-web\" value through to execution")
 
 	mockStore.AssertExpectations(t)
-	mockStore.AssertNotCalled(t, "GetPurchasePlan", mock.Anything, mock.Anything)
 	mockStore.AssertNotCalled(t, "TransitionExecutionStatus",
 		mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
 }

internal/purchase/messages_test.go

@@ -6,6 +6,7 @@ import (
 	"testing"
 
 	"github.com/LeanerCloud/CUDly/internal/config"
+	"github.com/LeanerCloud/CUDly/pkg/common"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
 )
@@ -118,16 +119,24 @@ func TestManager_ProcessMessage(t *testing.T) {
 			ExecutionID: "exec-web",
 			PlanID:      "plan-1",
 			Status:      "pending",
-			Source:      "web",
+			Source:      common.PurchaseSourceWeb,
 		}
 		mockStore.On("GetExecutionByID", ctx, "exec-web").Return(execution, nil)
-		// GetPurchasePlan must NOT be called (Source=web short-circuits)
-		// TransitionExecutionStatus must NOT be called
+		// Sentinel: web rows must short-circuit BEFORE the AutoPurchase plan
+		// fetch. Return AutoPurchase=true (the dangerous case) and flag the
+		// fetch; the default GetPurchasePlan stub doesn't record the call, so
+		// AssertNotCalled alone can't catch a bypass of the "cudly-web" gate.
+		planFetched := false
+		mockStore.GetPurchasePlanFn = func(_ context.Context, planID string) (*config.PurchasePlan, error) {
+			planFetched = true
+			return &config.PurchasePlan{ID: planID, AutoPurchase: true}, nil
+		}
 
 		err := manager.ProcessMessage(ctx, `{"type": "execute_purchase", "execution_id": "exec-web"}`)
 		assert.Error(t, err)
 		assert.Contains(t, err.Error(), "not eligible for auto-execution")
-		mockStore.AssertNotCalled(t, "GetPurchasePlan", mock.Anything, mock.Anything)
+		assert.False(t, planFetched,
+			"web rows must short-circuit before the AutoPurchase plan fetch on the SQS path")
 		mockStore.AssertNotCalled(t, "TransitionExecutionStatus",
 			mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything)
 	})