Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions internal/purchase/manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
28 changes: 21 additions & 7 deletions internal/purchase/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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,
Expand All @@ -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)
}
Expand Down
17 changes: 13 additions & 4 deletions internal/purchase/messages_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand Down Expand Up @@ -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)
})
Expand Down
Loading