diff --git a/internal/api/auto_purchase_grant.go b/internal/api/auto_purchase_grant.go new file mode 100644 index 00000000..856c75f6 --- /dev/null +++ b/internal/api/auto_purchase_grant.go @@ -0,0 +1,138 @@ +package api + +import ( + "context" + "fmt" + "math" + "strconv" + "time" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/logging" + "github.com/LeanerCloud/cloud-commitments-platform/internal/auth" + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" + "github.com/aws/aws-lambda-go/events" +) + +// Auto-purchase arming is a money-spending authorization: once the global kill +// switch is on, an armed plan buys with no approver. Arming therefore needs the +// same execute:purchases verb the direct execute handlers require, which is +// carved out of admin:* (auth.adminCarvedOuts), instead of the update:config / +// update:plans verbs any admin holds. The stateless admin API key skips every +// permission check, so it is refused outright for escalations. +// De-escalations (switch off, disarm, lower cap) stay on the ordinary verbs. + +const autoPurchaseGrantDenied = "arming auto-purchase requires a user holding execute:purchases; the admin API key cannot do it" + +// planArmed reports whether the plan, with the kill switch on, would buy without +// an approver: enabled, auto-purchase, and a non-NULL step cap (a NULL cap is +// refused at run time). It deliberately ignores the global switch so plan +// handlers never read global_config. +func planArmed(p *config.PurchasePlan) bool { + return p != nil && p.Enabled && p.AutoPurchase && p.MaxStepCommitment != nil +} + +// capCents returns the cap in exact cents. Plans reaching the gate have passed +// Validate, which rejects caps that are not whole cents, so the error branch is +// unreachable there; it still fails closed (treated as the maximum, so any write +// is a raise and needs the grant). +func capCents(v *float64) int64 { + if v == nil { + return 0 + } + c, err := config.StepCapCents(*v) + if err != nil { + return math.MaxInt64 + } + return c +} + +// planArmEscalates reports whether the write takes the plan from not-armed to +// armed, or raises the cap of an already armed plan. before is nil on create. +func planArmEscalates(before, after *config.PurchasePlan) bool { + if !planArmed(after) { + return false + } + if !planArmed(before) { + return true + } + return capCents(after.MaxStepCommitment) > capCents(before.MaxStepCommitment) +} + +// credentialKind labels the credential behind a session for logs and stamps. +func credentialKind(ctx context.Context, session *Session) string { + if principal, ok := principalFromContext(ctx); ok { + return string(principal.Kind) + } + if session != nil && session.UserID == apiKeyAdminUserID { + return string(PrincipalAdminAPIKey) + } + return string(PrincipalSession) +} + +// requireAutoPurchaseGrant refuses the admin API key and otherwise requires +// execute:purchases, evaluated against the credential presented. +func (h *Handler) requireAutoPurchaseGrant(ctx context.Context, req *events.LambdaFunctionURLRequest, session *Session) error { + if session == nil || session.UserID == apiKeyAdminUserID { + return NewClientError(403, autoPurchaseGrantDenied) + } + _, err := h.requirePermission(ctx, req, auth.ActionExecute, auth.ResourcePurchases) + return err +} + +// requirePlanArmGrant gates a plan write. before is nil on create. +func (h *Handler) requirePlanArmGrant(ctx context.Context, req *events.LambdaFunctionURLRequest, session *Session, before, after *config.PurchasePlan) error { + if !planArmEscalates(before, after) { + return nil + } + if err := h.requireAutoPurchaseGrant(ctx, req, session); err != nil { + return err + } + var oldCap *float64 + if before != nil { + oldCap = before.MaxStepCommitment + } + logging.Infof("auto-purchase plan arm or cap raise authorized (write follows): plan=%s actor=%s via=%s armed %t -> true cap %s -> %s", + after.ID, session.UserID, credentialKind(ctx, session), planArmed(before), formatCap(oldCap), formatCap(after.MaxStepCommitment)) + return nil +} + +// validateAndGatePlan validates the plan (400) and then applies the arming +// gate (403), in that order so a malformed write never reveals grant state. +func (h *Handler) validateAndGatePlan(ctx context.Context, req *events.LambdaFunctionURLRequest, session *Session, before, after *config.PurchasePlan) error { + if err := after.Validate(); err != nil { + return NewClientError(400, fmt.Sprintf("validation error: %s", err)) + } + return h.requirePlanArmGrant(ctx, req, session, before, after) +} + +func formatCap(v *float64) string { + if v == nil { + return "NULL" + } + return strconv.FormatFloat(*v, 'f', 2, 64) +} + +// requireAutoPurchaseSwitchGrant gates and stamps a change of the global kill +// switch inside the UpdateGlobalConfigAtomic closure. Only off->on needs the +// grant; any change is stamped so the last actor is always recorded. +func (h *Handler) requireAutoPurchaseSwitchGrant(ctx context.Context, req *events.LambdaFunctionURLRequest, session *Session, before bool, cfg *config.GlobalConfig) error { + if before == cfg.AutoPurchaseEnabled { + return nil + } + if cfg.AutoPurchaseEnabled { + if err := h.requireAutoPurchaseGrant(ctx, req, session); err != nil { + return err + } + } + by, via, at := session.UserID, credentialKind(ctx, session), time.Now().UTC() + cfg.AutoPurchaseChangedBy, cfg.AutoPurchaseChangedVia, cfg.AutoPurchaseChangedAt = &by, &via, &at + return nil +} + +// logAutoPurchaseSwitchChange records a committed change of the kill switch. +func logAutoPurchaseSwitchChange(ctx context.Context, session *Session, before bool, after *config.GlobalConfig) { + if after == nil || after.AutoPurchaseEnabled == before { + return + } + logging.Infof("auto-purchase kill switch changed: actor=%s via=%s %t -> %t", session.UserID, credentialKind(ctx, session), before, after.AutoPurchaseEnabled) +} diff --git a/internal/api/auto_purchase_grant_test.go b/internal/api/auto_purchase_grant_test.go new file mode 100644 index 00000000..fb200f23 --- /dev/null +++ b/internal/api/auto_purchase_grant_test.go @@ -0,0 +1,287 @@ +package api + +import ( + "context" + "testing" + "time" + + "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/auth" + "github.com/LeanerCloud/cloud-commitments-platform/internal/config" +) + +// Coverage for the auto-purchase arming gate (platform#631 R1b). Arming a plan +// or the global kill switch lets plan steps buy with no approver, so it needs +// execute:purchases (carved out of admin:*), and the stateless admin API key, +// which skips every permission check, is refused outright. De-escalations stay +// on the ordinary verbs. + +const ( + apgPlanID = "11111111-1111-1111-1111-111111111111" + apgAdminKy = "apg-admin-key" + apgUserID = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" +) + +var ( + apgAdminOnly = []auth.Permission{{Action: auth.ActionAdmin, Resource: auth.ResourceAll}} + apgAdminPlus = []auth.Permission{ + {Action: auth.ActionAdmin, Resource: auth.ResourceAll}, + {Action: auth.ActionExecute, Resource: auth.ResourcePurchases}, + } +) + +func apgCap(v float64) *float64 { return &v } + +func apgStoredPlan(armed bool, capUSD *float64) *config.PurchasePlan { + return &config.PurchasePlan{ + ID: apgPlanID, Name: "p", Enabled: true, AutoPurchase: armed, MaxStepCommitment: capUSD, + NotificationDaysBefore: 3, CreatedAt: time.Now().AddDate(0, -1, 0), + Services: map[string]config.ServiceConfig{"aws/rds": {Provider: "aws", Service: "rds"}}, + } +} + +// apgCaller: "" = admin API key, otherwise a bearer session holding perms. +type apgCaller struct { + perms []auth.Permission + key bool +} + +func (c apgCaller) handlerAndReq(store *MockConfigStore, body string) (*Handler, *events.LambdaFunctionURLRequest) { + mockAuth := new(MockAuthService) + mockAuth.On("ValidateSession", mock.Anything, "tok").Return(&Session{UserID: apgUserID}, nil).Maybe() + mockAuth.grantPermissions(c.perms) + mockAuth.On("GetAllowedAccountsAPI", mock.Anything, mock.Anything).Return([]string{"*"}, nil).Maybe() + h := &Handler{config: store, auth: mockAuth} + req := &events.LambdaFunctionURLRequest{Headers: map[string]string{"Authorization": "Bearer tok"}, Body: body} + if c.key { + h.apiKey = apgAdminKy + req.Headers = map[string]string{"x-api-key": apgAdminKy} + } + return h, req +} + +func apgCalled(m *MockConfigStore, method string) bool { + for _, c := range m.Calls { + if c.Method == method { + return true + } + } + return false +} + +func apgRefused(t *testing.T, err error) { + t.Helper() + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok, "must be a client error, not a 500: %v", err) + assert.Equal(t, 403, ce.code) +} + +func TestAutoPurchaseGate_PlanWrites(t *testing.T) { + admin := apgCaller{perms: apgAdminOnly} + granted := apgCaller{perms: apgAdminPlus} + key := apgCaller{key: true} + + tests := []struct { + name string + caller apgCaller + method string // patch | put + stored *config.PurchasePlan + body string + wantRefused bool + }{ + {"patch arms a capped plan: admin without grant refused", admin, "patch", apgStoredPlan(false, apgCap(1000)), `{"auto_purchase":true}`, true}, + {"patch arms a capped plan: execute:purchases allowed", granted, "patch", apgStoredPlan(false, apgCap(1000)), `{"auto_purchase":true}`, false}, + {"patch arms a capped plan: admin API key refused", key, "patch", apgStoredPlan(false, apgCap(1000)), `{"auto_purchase":true}`, true}, + {"patch enables a disabled armed-shaped plan: refused", admin, "patch", func() *config.PurchasePlan { + p := apgStoredPlan(true, apgCap(1000)) + p.Enabled = false + return p + }(), `{"enabled":true}`, true}, + {"patch disarm with admin API key allowed", key, "patch", apgStoredPlan(true, apgCap(1000)), `{"auto_purchase":false}`, false}, + {"patch rename of armed plan by admin allowed", admin, "patch", apgStoredPlan(true, apgCap(1000)), `{"name":"x"}`, false}, + {"patch auto_purchase on a plan with NULL cap is not armed: allowed", admin, "patch", apgStoredPlan(false, nil), `{"auto_purchase":true}`, false}, + {"put raises cap: admin refused", admin, "put", apgStoredPlan(true, apgCap(1000)), `{"name":"p","enabled":true,"auto_purchase":true,"max_step_commitment":2000}`, true}, + {"put raises cap: admin API key refused", key, "put", apgStoredPlan(true, apgCap(1000)), `{"name":"p","enabled":true,"auto_purchase":true,"max_step_commitment":2000}`, true}, + {"put raises cap: execute:purchases allowed", granted, "put", apgStoredPlan(true, apgCap(1000)), `{"name":"p","enabled":true,"auto_purchase":true,"max_step_commitment":2000}`, false}, + {"put lowers cap: admin allowed", admin, "put", apgStoredPlan(true, apgCap(1000)), `{"name":"p","enabled":true,"auto_purchase":true,"max_step_commitment":500}`, false}, + {"put same cap: admin allowed", admin, "put", apgStoredPlan(true, apgCap(1000)), `{"name":"p","enabled":true,"auto_purchase":true,"max_step_commitment":1000}`, false}, + {"put 1000.0 is the same cap: admin allowed", admin, "put", apgStoredPlan(true, apgCap(1000)), `{"name":"p","enabled":true,"auto_purchase":true,"max_step_commitment":1000.0}`, false}, + {"put 100.10 (binary-float trap) over 100.10 is the same cap: admin allowed", admin, "put", apgStoredPlan(true, apgCap(100.10)), `{"name":"p","enabled":true,"auto_purchase":true,"max_step_commitment":100.10}`, false}, + {"put one cent more is a raise: admin refused", admin, "put", apgStoredPlan(true, apgCap(1000)), `{"name":"p","enabled":true,"auto_purchase":true,"max_step_commitment":1000.01}`, true}, + {"put omitting the cap keeps it: admin allowed", admin, "put", apgStoredPlan(true, apgCap(1000)), `{"name":"p","enabled":true,"auto_purchase":true}`, false}, + {"put disarms with admin API key allowed", key, "put", apgStoredPlan(true, apgCap(1000)), `{"name":"p","enabled":true,"auto_purchase":false}`, false}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + store := new(MockConfigStore) + store.On("GetPurchasePlan", mock.Anything, apgPlanID).Return(tc.stored, nil) + store.On("UpdatePurchasePlan", mock.Anything, mock.Anything).Return(nil).Maybe() + h, req := tc.caller.handlerAndReq(store, tc.body) + var err error + if tc.method == "patch" { + _, err = h.patchPlan(context.Background(), req, apgPlanID) + } else { + _, err = h.updatePlan(context.Background(), req, apgPlanID) + } + if tc.wantRefused { + apgRefused(t, err) + assert.False(t, apgCalled(store, "UpdatePurchasePlan"), "a refused write must not reach the store") + return + } + require.NoError(t, err) + assert.True(t, apgCalled(store, "UpdatePurchasePlan")) + }) + } +} + +func TestAutoPurchaseGate_CreatePlan(t *testing.T) { + acct := "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb" + armedBody := `{"name":"n","enabled":true,"auto_purchase":true,"max_step_commitment":1000,"provider":"aws","service":"rds","target_accounts":["` + acct + `"]}` + for _, tc := range []struct { + name string + caller apgCaller + wantRefused bool + }{ + {"admin without grant refused", apgCaller{perms: apgAdminOnly}, true}, + {"admin API key refused", apgCaller{key: true}, true}, + {"execute:purchases allowed", apgCaller{perms: apgAdminPlus}, false}, + } { + t.Run(tc.name, func(t *testing.T) { + store := new(MockConfigStore) + store.On("CreatePurchasePlan", mock.Anything, mock.Anything).Return(nil).Maybe() + store.On("SetPlanAccounts", mock.Anything, mock.Anything, mock.Anything).Return(nil).Maybe() + store.GetCloudAccountFn = func(_ context.Context, id string) (*config.CloudAccount, error) { + return &config.CloudAccount{ID: id, Provider: "aws"}, nil + } + h, req := tc.caller.handlerAndReq(store, armedBody) + _, err := h.createPlan(context.Background(), req) + if tc.wantRefused { + apgRefused(t, err) + assert.False(t, apgCalled(store, "CreatePurchasePlan")) + return + } + require.NoError(t, err) + assert.True(t, apgCalled(store, "CreatePurchasePlan")) + }) + } +} + +func TestAutoPurchaseGate_CapValidation(t *testing.T) { + for _, tc := range []struct{ name, body string }{ + {"zero", `{"name":"p","max_step_commitment":0}`}, + {"negative", `{"name":"p","max_step_commitment":-5}`}, + {"below one cent", `{"name":"p","max_step_commitment":0.004}`}, + {"1e308 overflows the column", `{"name":"p","max_step_commitment":1e308}`}, + {"quoted number", `{"name":"p","max_step_commitment":"100"}`}, + {"half cent 1.005", `{"name":"p","max_step_commitment":1.005}`}, + {"0.015", `{"name":"p","max_step_commitment":0.015}`}, + {"1.0049", `{"name":"p","max_step_commitment":1.0049}`}, + {"1e-3", `{"name":"p","max_step_commitment":1e-3}`}, + } { + t.Run(tc.name, func(t *testing.T) { + store := new(MockConfigStore) + store.On("GetPurchasePlan", mock.Anything, apgPlanID).Return(apgStoredPlan(false, nil), nil) + store.On("UpdatePurchasePlan", mock.Anything, mock.Anything).Return(nil).Maybe() + h, req := apgCaller{perms: apgAdminPlus}.handlerAndReq(store, tc.body) + _, err := h.updatePlan(context.Background(), req, apgPlanID) + require.Error(t, err) + ce, ok := IsClientError(err) + require.True(t, ok, "%v", err) + assert.Equal(t, 400, ce.code) + assert.False(t, apgCalled(store, "UpdatePurchasePlan")) + }) + } +} + +// apgConfigCall runs PUT /api/config logic and returns the config handed to +// SaveGlobalConfig (nil when nothing was saved). +func apgConfigCall(t *testing.T, caller apgCaller, stored config.GlobalConfig, body string) (*config.GlobalConfig, error) { + t.Helper() + store := autoGateStore(t, stored) + var saved *config.GlobalConfig + store.ExpectedCalls = nil + seeded := stored + store.On("GetGlobalConfig", mock.Anything).Return(&seeded, nil).Maybe() + store.On("ListServiceConfigs", mock.Anything).Return([]config.ServiceConfig{}, nil).Maybe() + store.On("SaveGlobalConfig", mock.Anything, mock.Anything).Run(func(a mock.Arguments) { + c := *(a.Get(1).(*config.GlobalConfig)) + saved = &c + }).Return(nil).Maybe() + h, req := caller.handlerAndReq(store, body) + _, err := h.updateConfig(context.Background(), req) + return saved, err +} + +func TestAutoPurchaseGate_KillSwitch(t *testing.T) { + off, on := baseValidConfig(), baseValidConfig() + on.AutoPurchaseEnabled = true + + t.Run("off to on: admin without grant refused", func(t *testing.T) { + saved, err := apgConfigCall(t, apgCaller{perms: apgAdminOnly}, off, `{"auto_purchase_enabled":true}`) + apgRefused(t, err) + assert.Nil(t, saved) + }) + t.Run("off to on: admin API key refused", func(t *testing.T) { + saved, err := apgConfigCall(t, apgCaller{key: true}, off, `{"auto_purchase_enabled":true}`) + apgRefused(t, err) + assert.Nil(t, saved) + }) + t.Run("off to on: execute:purchases allowed and stamped", func(t *testing.T) { + before := time.Now().UTC().Add(-time.Second) + saved, err := apgConfigCall(t, apgCaller{perms: apgAdminPlus}, off, `{"auto_purchase_enabled":true}`) + require.NoError(t, err) + require.NotNil(t, saved) + assert.True(t, saved.AutoPurchaseEnabled) + require.NotNil(t, saved.AutoPurchaseChangedBy) + assert.Equal(t, apgUserID, *saved.AutoPurchaseChangedBy) + require.NotNil(t, saved.AutoPurchaseChangedVia) + assert.Equal(t, string(PrincipalSession), *saved.AutoPurchaseChangedVia) + require.NotNil(t, saved.AutoPurchaseChangedAt) + assert.True(t, saved.AutoPurchaseChangedAt.After(before)) + }) + t.Run("on to off: admin API key allowed and stamped", func(t *testing.T) { + saved, err := apgConfigCall(t, apgCaller{key: true}, on, `{"auto_purchase_enabled":false}`) + require.NoError(t, err) + require.NotNil(t, saved) + assert.False(t, saved.AutoPurchaseEnabled) + require.NotNil(t, saved.AutoPurchaseChangedVia) + assert.Equal(t, string(PrincipalAdminAPIKey), *saved.AutoPurchaseChangedVia) + }) + t.Run("unchanged switch: admin may save other settings, no stamp", func(t *testing.T) { + saved, err := apgConfigCall(t, apgCaller{perms: apgAdminOnly}, on, `{"auto_purchase_enabled":true,"default_coverage":70}`) + require.NoError(t, err) + require.NotNil(t, saved) + assert.Nil(t, saved.AutoPurchaseChangedBy) + }) +} + +// The stamps are json:"-": PUT /api/config unmarshals the body onto GlobalConfig, +// so a tagged field would let any admin forge who last flipped the switch. Go's +// decoder matches keys case-insensitively, so cover the case variants and the +// Go field names too. +func TestAutoPurchaseGate_StampsCannotBeForgedViaConfigBody(t *testing.T) { + stored := baseValidConfig() + by, via := "real-actor", "user" + at := time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) + stored.AutoPurchaseChangedBy, stored.AutoPurchaseChangedVia, stored.AutoPurchaseChangedAt = &by, &via, &at + + for _, body := range []string{ + `{"auto_purchase_changed_by":"mallory","auto_purchase_changed_via":"x","auto_purchase_changed_at":"2030-01-01T00:00:00Z"}`, + `{"AUTO_PURCHASE_CHANGED_BY":"mallory","Auto_Purchase_Changed_Via":"x","Auto_Purchase_Changed_At":"2030-01-01T00:00:00Z"}`, + `{"AutoPurchaseChangedBy":"mallory","AutoPurchaseChangedVia":"x","AutoPurchaseChangedAt":"2030-01-01T00:00:00Z"}`, + `{"autopurchasechangedby":"mallory"}`, + } { + saved, err := apgConfigCall(t, apgCaller{perms: apgAdminOnly}, stored, body) + require.NoError(t, err, body) + require.NotNil(t, saved, body) + require.NotNil(t, saved.AutoPurchaseChangedBy, body) + assert.Equal(t, "real-actor", *saved.AutoPurchaseChangedBy, body) + assert.Equal(t, "user", *saved.AutoPurchaseChangedVia, body) + assert.True(t, saved.AutoPurchaseChangedAt.Equal(at), body) + } +} diff --git a/internal/api/handler_config.go b/internal/api/handler_config.go index c521a7ca..fc3aa5c3 100644 --- a/internal/api/handler_config.go +++ b/internal/api/handler_config.go @@ -94,7 +94,8 @@ func (h *Handler) updateConfig(ctx context.Context, req *events.LambdaFunctionUR // ClientError(400) on a bad body or validation failure, which the store // propagates unchanged; DB/transport errors surface as 500. A changed // default reaches the service configs inside the same transaction. - _, err := h.config.UpdateGlobalConfigAtomic(ctx, func(existing *config.GlobalConfig) error { + var switchBefore bool + merged, err := h.config.UpdateGlobalConfigAtomic(ctx, func(existing *config.GlobalConfig) error { // Snapshot before the wholesale unmarshal: this body writes onto the // entire GlobalConfig, so ri_exchange_mode / ri_exchange_enabled reach // the scheduler through this endpoint exactly as they do through the @@ -102,6 +103,7 @@ func (h *Handler) updateConfig(ctx context.Context, req *events.LambdaFunctionUR // closure so the comparison reads the same advisory-locked row the // write lands on. before := riExchangeArmStateOf(existing) + switchBefore = existing.AutoPurchaseEnabled // grace_period_days is a map: json.Unmarshal into a non-nil map MERGES // keys (an omitted key can never be deleted). When the caller sends the @@ -116,11 +118,15 @@ func (h *Handler) updateConfig(ctx context.Context, req *events.LambdaFunctionUR if vErr := existing.Validate(); vErr != nil { return NewClientError(400, fmt.Sprintf("validation error: %s", vErr)) } - return h.requireRIExchangeAutoModeGrant(ctx, req, before, riExchangeArmStateOf(existing)) + if gErr := h.requireRIExchangeAutoModeGrant(ctx, req, before, riExchangeArmStateOf(existing)); gErr != nil { + return gErr + } + return h.requireAutoPurchaseSwitchGrant(ctx, req, session, switchBefore, existing) }) if err != nil { return nil, err } + logAutoPurchaseSwitchChange(ctx, session, switchBefore, merged) return &StatusResponse{Status: "updated"}, nil } diff --git a/internal/api/handler_plans.go b/internal/api/handler_plans.go index 376f2701..f44a1e85 100644 --- a/internal/api/handler_plans.go +++ b/internal/api/handler_plans.go @@ -178,8 +178,8 @@ func (h *Handler) createPlan(ctx context.Context, httpReq *events.LambdaFunction plan := req.toPurchasePlan() // Validate the plan - if err := plan.Validate(); err != nil { - return nil, NewClientError(400, fmt.Sprintf("validation error: %s", err)) + if err := h.validateAndGatePlan(ctx, httpReq, session, nil, plan); err != nil { + return nil, err } if err := h.config.CreatePurchasePlan(ctx, plan); err != nil { @@ -311,6 +311,10 @@ func (h *Handler) updatePlan(ctx context.Context, httpReq *events.LambdaFunction return nil, err } + if err := h.requirePlanArmGrant(ctx, httpReq, session, existingPlan, plan); err != nil { + return nil, err + } + if err := h.config.UpdatePurchasePlan(ctx, plan); err != nil { return nil, mapCreatePlanStorageError(err, "plan not found", "failed to update plan", "updatePlan: UpdatePurchasePlan failed") } @@ -708,12 +712,13 @@ func (h *Handler) patchPlan(ctx context.Context, httpReq *events.LambdaFunctionU return nil, NewClientError(404, fmt.Sprintf("plan not found: %s", planID)) } + before := *plan if err := applyPatchFields(plan, req); err != nil { return nil, err } - if err := plan.Validate(); err != nil { - return nil, NewClientError(400, fmt.Sprintf("validation error: %s", err)) + if err := h.validateAndGatePlan(ctx, httpReq, session, &before, plan); err != nil { + return nil, err } if err := h.config.UpdatePurchasePlan(ctx, plan); err != nil { diff --git a/internal/config/store_postgres_auto_purchase_stamps_integration_test.go b/internal/config/store_postgres_auto_purchase_stamps_integration_test.go index dd15468a..f471e99f 100644 --- a/internal/config/store_postgres_auto_purchase_stamps_integration_test.go +++ b/internal/config/store_postgres_auto_purchase_stamps_integration_test.go @@ -73,3 +73,53 @@ func TestPostgresStoreDB_AutoPurchaseStamps_FreshMigrateAndRIWritePreserve(t *te require.NotNil(t, cfg.AutoPurchaseChangedVia) assert.Equal(t, "user-session", *cfg.AutoPurchaseChangedVia) } + +// TestPostgresStoreDB_StepCapStoredEqualsCompared proves, on a real PostgreSQL, +// that for every cap StepCapCents accepts, the NUMERIC(18,2) column stores the +// same cents the gate compared (B1: 1.005 used to store as 1.01 while the gate +// saw 1.00), and that caps it rejects include the half-cent inputs. +func TestPostgresStoreDB_StepCapStoredEqualsCompared(t *testing.T) { + dsn := os.Getenv("AUTO_PURCHASE_STAMPS_TEST_DATABASE_URL") + if dsn == "" { + t.Skip("set AUTO_PURCHASE_STAMPS_TEST_DATABASE_URL to a dedicated r1a_* test database") + } + parsed, err := url.Parse(dsn) + require.NoError(t, err) + require.True(t, strings.HasPrefix(strings.TrimPrefix(parsed.Path, "/"), "r1a_"), "database must be dedicated to R1a verification") + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cancel() + pool, err := pgxpool.New(ctx, dsn) + require.NoError(t, err) + defer pool.Close() + require.NoError(t, migrations.RunMigrations(ctx, pool, getTestMigrationsPath(), "", "")) + store := &PostgresStore{db: pool} + + plan := &PurchasePlan{ + Name: "cap-roundtrip", Enabled: true, Services: map[string]ServiceConfig{}, + RampSchedule: RampSchedule{Type: "immediate", PercentPerStep: 100, TotalSteps: 1}, + } + require.NoError(t, store.CreatePurchasePlan(ctx, plan)) + + caps := []float64{100.10, 12345678.9, 0.07, 1e15, 999999999999999.99} + for c := 1; c <= 3000; c++ { + caps = append(caps, float64(c)/100) + } + for _, v := range caps { + sent, err := StepCapCents(v) + require.NoError(t, err, "cap %v", v) + got, err := store.GetPurchasePlan(ctx, plan.ID) + require.NoError(t, err) + v := v + got.MaxStepCommitment = &v + require.NoError(t, store.UpdatePurchasePlan(ctx, got), "cap %v", v) + var cents int64 + require.NoError(t, pool.QueryRow(ctx, `SELECT (max_step_commitment * 100)::bigint FROM purchase_plans WHERE id = $1`, plan.ID).Scan(¢s)) + require.Equal(t, sent, cents, "stored cents must equal compared cents for cap %v", v) + } + + for _, v := range []float64{1.005, 0.015, 1.0049, 1e-3, 1000.005} { + _, err := StepCapCents(v) + require.Error(t, err, "cap %v must be rejected, not rounded", v) + } +} diff --git a/internal/config/validation.go b/internal/config/validation.go index 1fe212ff..8f65fa1d 100644 --- a/internal/config/validation.go +++ b/internal/config/validation.go @@ -7,6 +7,7 @@ import ( "math" "net/mail" "sort" + "strconv" "strings" "github.com/LeanerCloud/cloud-commitments-go/pkg/ladder" @@ -521,15 +522,47 @@ func (c *ServiceConfig) validateConfigCoverage() error { return nil } +// MinStepCommitment and MaxStepCommitmentLimit bound max_step_commitment to what +// the NUMERIC(18,2) column stores: below one cent it rounds to 0 and violates +// CHECK > 0 (a 500), and 1e308 overflows it. NaN fails both comparisons. +const ( + MinStepCommitment = 0.01 + MaxStepCommitmentLimit = 1e15 +) + +// StepCapCents returns the cap in whole cents, exactly. The column is +// NUMERIC(18,2) and Postgres rounds the decimal text pgx sends (the shortest +// representation of the float), so a cap with more than two decimals would be +// stored as a different value than the one compared (1.005 stores as 1.01). +// Such caps are rejected instead of rounded, which makes the stored and the +// compared value identical. The decimal text is parsed, not multiplied, because +// v*100 loses exactness near the 1e15 limit. +func StepCapCents(v float64) (int64, error) { + if !(v >= MinStepCommitment) || !(v <= MaxStepCommitmentLimit) { + return 0, fmt.Errorf("max_step_commitment must be between %v and %v, got: %v", MinStepCommitment, MaxStepCommitmentLimit, v) + } + whole, frac, _ := strings.Cut(strconv.FormatFloat(v, 'f', -1, 64), ".") + if len(frac) > 2 { + return 0, fmt.Errorf("max_step_commitment must have at most 2 decimal places, got: %v", v) + } + n, err := strconv.ParseInt(whole+(frac + "00")[:2], 10, 64) + if err != nil { + return 0, fmt.Errorf("max_step_commitment is not representable: %w", err) + } + return n, nil +} + // validateNotificationAndCap checks the notification window and requires a set -// cap to be a positive finite amount. A nil cap means no cap, which refuses -// auto-purchase of steps with recommendations. +// cap to be a positive finite amount in whole cents. A nil cap means no cap, +// which refuses auto-purchase of steps with recommendations. func (p *PurchasePlan) validateNotificationAndCap() error { if p.NotificationDaysBefore < 0 || p.NotificationDaysBefore > MaxNotificationDaysBefore { return fmt.Errorf("notification days must be between 0 and %d, got: %d", MaxNotificationDaysBefore, p.NotificationDaysBefore) } - if p.MaxStepCommitment != nil && (!(*p.MaxStepCommitment > 0) || math.IsInf(*p.MaxStepCommitment, 0)) { - return fmt.Errorf("max_step_commitment must be a positive finite number, got: %v", *p.MaxStepCommitment) + if p.MaxStepCommitment != nil { + if _, err := StepCapCents(*p.MaxStepCommitment); err != nil { + return err + } } return nil } diff --git a/internal/config/validation_step_cap_test.go b/internal/config/validation_step_cap_test.go new file mode 100644 index 00000000..d1ce4496 --- /dev/null +++ b/internal/config/validation_step_cap_test.go @@ -0,0 +1,28 @@ +package config + +import ( + "math" + "testing" + + "github.com/stretchr/testify/assert" +) + +// max_step_commitment is stored in NUMERIC(18,2) CHECK > 0: below a cent it +// rounds to 0 and overflowing values fail the insert, either of which was a 500. +func TestValidateNotificationAndCap_StepCapBounds(t *testing.T) { + for _, tc := range []struct { + v float64 + want bool + }{ + {0, false}, {-1, false}, {0.004, false}, {math.NaN(), false}, {math.Inf(1), false}, + {1e308, false}, {MaxStepCommitmentLimit * 2, false}, + {1.005, false}, {0.015, false}, {1.0049, false}, {1e-3, false}, + {0.01, true}, {100.10, true}, {2500.5, true}, {MaxStepCommitmentLimit, true}, + } { + v := tc.v + p := &PurchasePlan{MaxStepCommitment: &v} + err := p.validateNotificationAndCap() + assert.Equal(t, tc.want, err == nil, "cap %v: %v", tc.v, err) + } + assert.NoError(t, (&PurchasePlan{}).validateNotificationAndCap(), "a NULL cap stays valid (refused at run time)") +}