From d2e389b86a365b5bafcee4b5f9e15f3bfc869543 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Fri, 16 Oct 2026 03:33:52 +0200 Subject: [PATCH] feat(mcp): require a USD cap on every real purchase Per-call caps bounded request size, not money: CUDLY_MCP_MAX_COUNT=50 still admitted 50 x u-24tb1, roughly $286M over three years. Real purchases now also need CUDLY_MCP_MAX_COMMITMENT_USD. An unset or invalid value refuses before credentials are resolved; after ResolveClient the purchase is priced with the helpers from the previous commit and refused when its total exceeds the cap, before an idempotency token is derived or anything is bought. Real GCP CUD purchases are now refused until the library pin includes the CUD pricing fix (go#321, follow-up #67). GCP tests that asserted a successful real purchase now assert the refusal; #67 restores them. ExecutePurchase's response tail moves into purchaseResponse to stay under the gocyclo limit. Manifest, server.json, README and CHANGELOG document the new required setting. Refs #51 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 21 +++++- README.md | 2 +- cmd/cudly-mcp/gcp_purchase_protocol_test.go | 27 ++----- cmd/cudly-mcp/main_test.go | 1 + mcpb/manifest.json | 12 ++- server.json | 9 ++- tools/adopted_go_safeguards_test.go | 2 + tools/commitment_usd.go | 27 +++---- tools/gcp_computeengine_cud_test.go | 10 +-- tools/purchase.go | 34 +++++++-- tools/purchase_test.go | 13 +++- tools/search_dedupe_test.go | 6 +- tools/spend_caps.go | 11 ++- tools/spend_caps_test.go | 9 ++- tools/usd_cap_wiring_test.go | 83 +++++++++++++++++++++ 15 files changed, 205 insertions(+), 62 deletions(-) create mode 100644 tools/usd_cap_wiring_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index d1ce545..d8ae629 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,8 +14,16 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). and `ARCHERA_PLAN_ID`; unconfigured calls error and send no request. It reads `https://api.archera.ai` only, never retries, and makes no purchase. +- `CUDLY_MCP_MAX_COMMITMENT_USD`: a per-call ceiling on the total USD one + real purchase commits over its full term, priced from the provider's own + offering lookup (Savings Plans: hourly commitment x 8760 x years). USD only; + unpriceable purchases are refused. + ### Security +- A small `CUDLY_MCP_MAX_COUNT` no longer admits an enormous purchase (50 x + `u-24tb1` is about $286M): the USD cap bounds the real total. + - `cudly_archera_comparison`: a transport error that echoed the API key with zero-width, bidi or control characters spliced into it could reassemble the plain key in the tool error, because the key was masked before those @@ -48,12 +56,21 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### Breaking +- **`CUDLY_MCP_MAX_COMMITMENT_USD` is now required for every real purchase.** + Unset, empty or invalid refuses the purchase before credentials are + resolved. The MCPB bundle has a matching setting that must be filled in before any real purchase. Executing a + purchase now also needs credentials and network access for the provider's + offering lookup. +- **Real GCP CUD purchases are refused** until the `cloud-commitments-go` pin + includes the CUD pricing fix (go#321), because their USD total cannot be + determined yet. Previews are unaffected. + - **Real purchases now require operator spend caps.** With `CUDLY_MCP_ENABLE_REAL_PURCHASES=1`, every real purchase is refused unless the relevant cap is set to a valid value: `CUDLY_MCP_MAX_COUNT` (RIs, and vCPUs for GCP CUDs), `CUDLY_MCP_MAX_HOURLY_COMMITMENT` (Savings Plans) and - `CUDLY_MCP_MAX_MEMORY_GB` (GCP CUDs). Caps are per call and are not USD - caps. The MCPB bundle exposes them as optional settings. Previews are + `CUDLY_MCP_MAX_MEMORY_GB` (GCP CUDs). Caps are per call; the count and + hourly caps are not USD caps (see `CUDLY_MCP_MAX_COMMITMENT_USD`). The MCPB bundle exposes them as optional settings. Previews are unaffected. Savings Plan previews now report `cost` (total commitment over the term), and Savings Plan audit lines now carry that figure as the estimated cost. diff --git a/README.md b/README.md index bdcb375..81da13d 100644 --- a/README.md +++ b/README.md @@ -124,7 +124,7 @@ Every other provider's purchase tool (`cudly_aws_savingsplans_purchase`, `cudly_ - `dry_run` defaults to `true` on every purchase tool. A dry-run call never contacts the cloud provider and never spends money -- it only validates your parameters. It reports pricing (`cost`/`on_demand_cost`/`estimated_savings`/`savings_percentage`) only when a real figure is genuinely known; those fields are omitted, not zeroed, when it isn't. Only Savings Plan previews are priced (total commitment over the term, computed locally); RI and CUD previews show no price. - `dry_run=true` always previews, including when `confirm=true`. A real purchase requires **both** `dry_run=false` **and** `confirm=true`; `dry_run=false` with `confirm=false` is refused with a structured error, not silently downgraded to a preview. - A real purchase **also** requires the operator to have set `CUDLY_MCP_ENABLE_REAL_PURCHASES=1` (or `true`, case-insensitive) in the environment `cudly-mcp` was launched with. This is layered underneath `confirm`: `confirm` only proves the model asked to spend money, it does not prove the operator running this server wants it able to. The gate is unset (disabled) by default -- unset, empty, `0`, `false`, or any other value all disable real purchases -- so a fresh install cannot spend money until the operator explicitly opts in. When disabled, a `dry_run=false, confirm=true` call is refused before any provider or credential is touched, naming the flag to set. Dry runs are unaffected by this flag; they never spend regardless of its value. -- A real purchase also requires **operator-set spend caps**, read from the server's environment (never from tool arguments, which the model controls). Unset, empty or invalid caps refuse the purchase, naming the variable; there is no default and no "unlimited". `CUDLY_MCP_MAX_COUNT` (positive integer) bounds `count` for every RI purchase and `vcpu_count` for GCP CUDs. `CUDLY_MCP_MAX_HOURLY_COMMITMENT` (positive number, USD/hour) bounds Savings Plan `hourly_commitment`. `CUDLY_MCP_MAX_MEMORY_GB` (positive number) bounds `memory_gb` for GCP CUDs. Previews need no caps. **Limits you must understand:** the caps apply **per call**, not cumulatively, so a fresh `idempotency_nonce`, region or profile repeats the call and spends again; and the count cap is **not a USD cap**: unit prices differ by orders of magnitude (50 x `u-24tb1` at about $218/hour each for 3 years is roughly $286M under a `CUDLY_MCP_MAX_COUNT=50` that looks small). Choose caps with the most expensive resource type in mind. A USD cap and a cumulative budget are tracked in [#51](https://github.com/LeanerCloud/cloud-commitments-mcp/issues/51). +- A real purchase also requires **operator-set spend caps**, read from the server's environment (never from tool arguments, which the model controls). Unset, empty or invalid caps refuse the purchase, naming the variable; there is no default and no "unlimited". `CUDLY_MCP_MAX_COMMITMENT_USD` (positive number, USD) is required for every real purchase and bounds the **total USD one call commits over the full term**: it is read from the provider's own offering lookup just before buying (Savings Plans: hourly commitment x 8760 x years, computed locally), so executing needs credentials and network access for that lookup. Only USD is supported; a purchase whose price is missing, zero, non-USD or errors is refused rather than assumed cheap. Real GCP CUD purchases are refused until the `cloud-commitments-go` pin includes the CUD pricing fix (go#321). The Azure figure is the Retail Prices list price, not a negotiated rate. The lookup and the purchase are separate calls, so the price can change between them. `CUDLY_MCP_MAX_COUNT` (positive integer) bounds `count` for every RI purchase and `vcpu_count` for GCP CUDs. `CUDLY_MCP_MAX_HOURLY_COMMITMENT` (positive number, USD/hour) bounds Savings Plan `hourly_commitment`. `CUDLY_MCP_MAX_MEMORY_GB` (positive number) bounds `memory_gb` for GCP CUDs. Previews need no caps. **Limits you must understand:** the caps apply **per call**, not cumulatively, so a fresh `idempotency_nonce`, region or profile repeats the call and spends again; a cumulative budget is tracked in [#51](https://github.com/LeanerCloud/cloud-commitments-mcp/issues/51). Choose the USD cap as the most one call may ever spend. - A real purchase requires the **target account to be named**: `aws_profile` (AWS) or `azure_subscription_id` (Azure), either as the tool argument or via the matching environment variable the provider itself reads (`AWS_PROFILE`, `AZURE_SUBSCRIPTION_ID`). **GCP has no environment fallback, so `gcp_project_id` is required for a real CUD purchase.** That is deliberate: nothing in `providers/gcp` reads `GOOGLE_CLOUD_PROJECT` or `CLOUDSDK_CORE_PROJECT`, and with no project configured the provider falls back to the *first active project* in your `ListProjects` response -- an artifact of IAM visibility and API ordering rather than a project anyone chose, which is not a defensible default for spending money. Pointing the scope at an environment variable the provider ignores would be worse still: the idempotency token would name one project while the commitment landed in another. If neither is set, the real purchase is refused before any credential is touched, naming the argument to pass. **Dry runs do not require it** -- you can price a purchase without naming an account. This is not bookkeeping: the account is folded into the idempotency token, so leaving it to ambient credentials on one call and naming it explicitly on the next derives two *different* tokens for the *same* account. Every provider's dedupe is token-keyed, so the second call's lookup would miss and buy a second commitment. Refusing an undeterminable account makes that aliasing unreachable. Naming an account explicitly and inheriting the same value from the environment always agree, so they still dedupe normally. - Every money-affecting parameter (region, resource type, count, term, payment option, and any provider-specific dimension such as RDS's `az_config`) is validated against an explicit enum or non-empty check before anything is built or sent. There is no silent default for a value that materially changes what gets purchased. - Every real purchase is tagged with a source identifying it came from this MCP server (never a user-suppliable string) and a deterministic idempotency token derived from the request's own parameters. By default, retrying an identical tool call -- however long after the original, and regardless of any clock boundary -- always derives the same token, so the provider dedupes the retry instead of buying twice; this is a fail-safe default, since the worst case of a false dedupe is a skipped intentional repeat, never a double purchase. To deliberately make a second, otherwise-identical purchase (e.g. "buy 3 RIs now" and "buy 3 more next week"), pass a fresh `idempotency_nonce` value on the second call; passing the same nonce on a retry of that same call still dedupes correctly. diff --git a/cmd/cudly-mcp/gcp_purchase_protocol_test.go b/cmd/cudly-mcp/gcp_purchase_protocol_test.go index 1a300a2..099a21f 100644 --- a/cmd/cudly-mcp/gcp_purchase_protocol_test.go +++ b/cmd/cudly-mcp/gcp_purchase_protocol_test.go @@ -2,7 +2,6 @@ package main import ( "context" - "encoding/json" "errors" "sync/atomic" "testing" @@ -125,11 +124,12 @@ func (p *fakeGCPProvider) GetRecommendationsClient(_ context.Context) (provider. // gcpComputeEngineRecommendationFromArgs never sets CommitmentCost, so old // and new library pins both yield "cost":0 here. The pin-sensitive cost proof // lives at the library boundary (tools/gcp_computeengine_cud_safeguards_test.go). -func TestGCPComputeEngineCUDPurchaseStructuredCost(t *testing.T) { +func TestGCPComputeEngineCUDPurchaseRefusedByUSDCap(t *testing.T) { t.Setenv(tools.EnvEnableRealPurchases, "1") t.Setenv(tools.EnvMaxCount, "1000") t.Setenv(tools.EnvMaxHourlyCommitment, "1000") t.Setenv(tools.EnvMaxMemoryGB, "1000") + t.Setenv(tools.EnvMaxCommitmentUSD, "1000000000") service := &fakeComputeEngineCommitmentsService{} provider.GetRegistry().Unregister("gcp") @@ -190,21 +190,10 @@ func TestGCPComputeEngineCUDPurchaseStructuredCost(t *testing.T) { }, }) require.NoError(t, err, "CallTool itself must not return a transport-level error") - require.False(t, result.IsError, "a successful fake-backed purchase must not surface as a tool error: %+v", result.Content) - require.Equal(t, int32(1), service.insertCalls.Load()) - - require.NotNil(t, result.StructuredContent, "structured output must be present") - encoded, err := json.Marshal(result.StructuredContent) - require.NoError(t, err) - var structured map[string]any - require.NoError(t, json.Unmarshal(encoded, &structured)) - - assert.Equal(t, true, structured["success"], "purchase must report success, got %s", encoded) - assert.Equal(t, false, structured["dry_run"], "this was a real purchase, got %s", encoded) - // The cost contract is an EXPLICIT zero: PurchaseResponse.Cost is a - // *float64 with omitempty, so a nil cost would drop the key entirely; a - // known zero upfront charge must survive as "cost":0. - cost, present := structured["cost"] - require.True(t, present, "structured output must carry an explicit cost field, got %s", encoded) - assert.Equal(t, 0.0, cost, "a Compute Engine CUD has no upfront charge, got %s", encoded) + // Real GCP CUD purchases are refused until USD totals exist (go#321). + require.True(t, result.IsError, "a real GCP CUD purchase must be refused: %+v", result.Content) + text, ok := result.Content[0].(*gosdk.TextContent) + require.True(t, ok) + assert.Contains(t, text.Text, "go#321") + require.Equal(t, int32(0), service.insertCalls.Load()) } diff --git a/cmd/cudly-mcp/main_test.go b/cmd/cudly-mcp/main_test.go index d8b3856..d547fd2 100644 --- a/cmd/cudly-mcp/main_test.go +++ b/cmd/cudly-mcp/main_test.go @@ -303,6 +303,7 @@ func TestRealPurchasePastProviderRegistration(t *testing.T) { t.Setenv(tools.EnvMaxCount, "1000") t.Setenv(tools.EnvMaxHourlyCommitment, "1000") t.Setenv(tools.EnvMaxMemoryGB, "1000") + t.Setenv(tools.EnvMaxCommitmentUSD, "1000000000") server, err := cudlymcp.NewServer("test-regression") require.NoError(t, err) diff --git a/mcpb/manifest.json b/mcpb/manifest.json index d2059b8..66d24a7 100644 --- a/mcpb/manifest.json +++ b/mcpb/manifest.json @@ -29,6 +29,7 @@ "CUDLY_MCP_MAX_COUNT": "${user_config.max_count}", "CUDLY_MCP_MAX_HOURLY_COMMITMENT": "${user_config.max_hourly_commitment}", "CUDLY_MCP_MAX_MEMORY_GB": "${user_config.max_memory_gb}", + "CUDLY_MCP_MAX_COMMITMENT_USD": "${user_config.max_commitment_usd}", "ARCHERA_API_KEY": "${user_config.archera_api_key}", "ARCHERA_ORG_ID": "${user_config.archera_org_id}", "ARCHERA_PLAN_ID": "${user_config.archera_plan_id}" @@ -42,6 +43,7 @@ "CUDLY_MCP_MAX_COUNT": "${user_config.max_count}", "CUDLY_MCP_MAX_HOURLY_COMMITMENT": "${user_config.max_hourly_commitment}", "CUDLY_MCP_MAX_MEMORY_GB": "${user_config.max_memory_gb}", + "CUDLY_MCP_MAX_COMMITMENT_USD": "${user_config.max_commitment_usd}", "ARCHERA_API_KEY": "${user_config.archera_api_key}", "ARCHERA_ORG_ID": "${user_config.archera_org_id}", "ARCHERA_PLAN_ID": "${user_config.archera_plan_id}" @@ -55,6 +57,7 @@ "CUDLY_MCP_MAX_COUNT": "${user_config.max_count}", "CUDLY_MCP_MAX_HOURLY_COMMITMENT": "${user_config.max_hourly_commitment}", "CUDLY_MCP_MAX_MEMORY_GB": "${user_config.max_memory_gb}", + "CUDLY_MCP_MAX_COMMITMENT_USD": "${user_config.max_commitment_usd}", "ARCHERA_API_KEY": "${user_config.archera_api_key}", "ARCHERA_ORG_ID": "${user_config.archera_org_id}", "ARCHERA_PLAN_ID": "${user_config.archera_plan_id}" @@ -74,7 +77,7 @@ "max_count": { "type": "number", "title": "Maximum count per purchase call", - "description": "Per-call ceiling on instances/nodes (or vCPUs for GCP CUDs) for a real purchase. Required before any real purchase: leave it empty and purchases are refused. Applies per call, not cumulatively, and is not a USD cap.", + "description": "Per-call ceiling on instances/nodes (or vCPUs for GCP CUDs) for a real purchase. Required before any real purchase: leave it empty and purchases are refused. Applies per call, not cumulatively; it bounds size, not dollars (see the USD setting).", "min": 1, "required": false }, @@ -92,6 +95,13 @@ "min": 1, "required": false }, + "max_commitment_usd": { + "type": "number", + "title": "Maximum commitment per purchase call (USD)", + "description": "Per-call ceiling on the total USD one purchase commits over its full term, priced by the provider just before buying. Required before any real purchase: leave it empty and purchases are refused. USD only; purchases that cannot be priced (including real GCP CUD purchases for now) are refused. Applies per call, not cumulatively.", + "min": 1, + "required": false + }, "archera_api_key": { "type": "string", "title": "Archera API key", diff --git a/server.json b/server.json index 141c0fd..ec30ec8 100644 --- a/server.json +++ b/server.json @@ -26,7 +26,7 @@ }, { "name": "CUDLY_MCP_MAX_COUNT", - "description": "Positive integer. Per-call ceiling on count (RIs) and vcpu_count (GCP CUDs) for real purchases. Required for any real purchase: unset, empty or invalid refuses it. Not a USD cap and not cumulative; see README 'Safety model'.", + "description": "Positive integer. Per-call ceiling on count (RIs) and vcpu_count (GCP CUDs) for real purchases. Required for any real purchase: unset, empty or invalid refuses it. Bounds size, not dollars (see CUDLY_MCP_MAX_COMMITMENT_USD); not cumulative. See README 'Safety model'.", "isRequired": false, "isSecret": false, "format": "string" @@ -45,6 +45,13 @@ "isSecret": false, "format": "string" }, + { + "name": "CUDLY_MCP_MAX_COMMITMENT_USD", + "description": "Positive number (USD). Per-call ceiling on the total USD one real purchase commits over its full term, priced by the provider just before buying. Required for any real purchase: unset, empty or invalid refuses it. USD only; purchases that cannot be priced (including real GCP CUD purchases for now) are refused. Not cumulative.", + "isRequired": false, + "isSecret": false, + "format": "string" + }, { "name": "CUDLY_MCP_AUDIT_LOG", "description": "Overrides the local JSONL purchase audit path after trimming. When unset, writes to $XDG_STATE_HOME/cudly/mcp-audit.jsonl; if XDG_STATE_HOME is unset, empty, or relative, it is ignored and the fallback is ~/.local/state/cudly/mcp-audit.jsonl. An empty or whitespace-only CUDLY_MCP_AUDIT_LOG value disables auditing.", diff --git a/tools/adopted_go_safeguards_test.go b/tools/adopted_go_safeguards_test.go index 97a3bec..7b93bee 100644 --- a/tools/adopted_go_safeguards_test.go +++ b/tools/adopted_go_safeguards_test.go @@ -89,6 +89,8 @@ func rdsOffering(id, engine string) rdstypes.ReservedDBInstancesOffering { MultiAZ: aws.Bool(true), Duration: aws.Int32(3 * 365 * 24 * 3600), OfferingType: aws.String("No Upfront"), + FixedPrice: aws.Float64(100), + CurrencyCode: aws.String("USD"), } } diff --git a/tools/commitment_usd.go b/tools/commitment_usd.go index 5b940fb..a34dbc1 100644 --- a/tools/commitment_usd.go +++ b/tools/commitment_usd.go @@ -11,9 +11,6 @@ import ( "github.com/LeanerCloud/cloud-commitments-go/pkg/provider" ) -// usdCapEnv names the cap in refusal messages. -const usdCapEnv = "CUDLY_MCP_MAX_COMMITMENT_USD" - // committedUSD returns the total USD a purchase commits over the full term. // Anything it cannot price with confidence is an error, never zero. func committedUSD(ctx context.Context, client provider.ServiceClient, rec common.Recommendation) (float64, error) { @@ -21,11 +18,11 @@ func committedUSD(ctx context.Context, client provider.ServiceClient, rec common case rec.CommitmentType == common.CommitmentSavingsPlan: return savingsPlanUSD(rec) case rec.Provider == common.ProviderGCP || rec.CommitmentType == common.CommitmentCUD: - return 0, errors.New("USD totals for GCP committed-use discounts are unavailable at the pinned cloud-commitments-go (go#321), so real GCP CUD purchases are refused") + return 0, errors.New("refusing real purchase: USD totals for GCP committed-use discounts are unavailable at the pinned cloud-commitments-go (go#321), so real GCP CUD purchases are refused") } od, err := client.GetOfferingDetails(ctx, rec) if err != nil { - return 0, fmt.Errorf("cannot determine USD total: %w", err) + return 0, fmt.Errorf("refusing real purchase: cannot determine USD total: %w", err) } return unitOfferingUSD(od, rec.Count) } @@ -35,15 +32,15 @@ func committedUSD(ctx context.Context, client provider.ServiceClient, rec common func savingsPlanUSD(rec common.Recommendation) (float64, error) { details, ok := rec.Details.(*common.SavingsPlanDetails) if !ok || details == nil { - return 0, errors.New("cannot determine USD total: savings plan details are missing") + return 0, errors.New("refusing real purchase: cannot determine USD total: savings plan details are missing") } hourly := details.HourlyCommitment if math.IsNaN(hourly) || math.IsInf(hourly, 0) || hourly <= 0 { - return 0, fmt.Errorf("cannot determine USD total: invalid hourly commitment %v", hourly) + return 0, fmt.Errorf("refusing real purchase: cannot determine USD total: invalid hourly commitment %v", hourly) } years := termYearsFromRecommendationTerm(rec.Term) if years != 1 && years != 3 { - return 0, fmt.Errorf("cannot determine USD total: unsupported savings plan term %q", rec.Term) + return 0, fmt.Errorf("refusing real purchase: cannot determine USD total: unsupported savings plan term %q", rec.Term) } return finiteUSD(hourly * savingsPlanHoursPerYear * float64(years)) } @@ -51,23 +48,23 @@ func savingsPlanUSD(rec common.Recommendation) (float64, error) { // unitOfferingUSD scales the provider's per-reservation total by count. func unitOfferingUSD(od *common.OfferingDetails, count int) (float64, error) { if od == nil { - return 0, errors.New("cannot determine USD total: provider returned no offering details") + return 0, errors.New("refusing real purchase: cannot determine USD total: provider returned no offering details") } if math.IsNaN(od.TotalCost) || math.IsInf(od.TotalCost, 0) || od.TotalCost <= 0 { - return 0, fmt.Errorf("cannot determine USD total: provider total cost %v is not a positive price", od.TotalCost) + return 0, fmt.Errorf("refusing real purchase: cannot determine USD total: provider total cost %v is not a positive price", od.TotalCost) } if !strings.EqualFold(strings.TrimSpace(od.Currency), "USD") { - return 0, fmt.Errorf("cannot determine USD total: offering currency %q is not USD", od.Currency) + return 0, fmt.Errorf("refusing real purchase: cannot determine USD total: offering currency %q is not USD", od.Currency) } if count < 1 { - return 0, fmt.Errorf("cannot determine USD total: count %d is below 1", count) + return 0, fmt.Errorf("refusing real purchase: cannot determine USD total: count %d is below 1", count) } return finiteUSD(od.TotalCost * float64(count)) } func finiteUSD(total float64) (float64, error) { if math.IsNaN(total) || math.IsInf(total, 0) { - return 0, errors.New("cannot determine USD total: computed total overflows") + return 0, errors.New("refusing real purchase: cannot determine USD total: computed total overflows") } return total, nil } @@ -76,14 +73,14 @@ func finiteUSD(total float64) (float64, error) { // allowed) and returns the total otherwise. func enforceUSDCap(ctx context.Context, client provider.ServiceClient, rec common.Recommendation, limit float64) (float64, error) { if math.IsNaN(limit) || math.IsInf(limit, 0) || limit <= 0 { - return 0, fmt.Errorf("invalid %s limit %v: must be a finite number above 0", usdCapEnv, limit) + return 0, fmt.Errorf("refusing real purchase: %s limit %v must be a finite number above 0", EnvMaxCommitmentUSD, limit) } total, err := committedUSD(ctx, client, rec) if err != nil { return 0, err } if total > limit { - return 0, fmt.Errorf("commitment total %.2f USD exceeds the %s limit of %.2f USD", total, usdCapEnv, limit) + return 0, fmt.Errorf("refusing real purchase: commitment total %.2f USD exceeds the operator cap %s=%.2f", total, EnvMaxCommitmentUSD, limit) } return total, nil } diff --git a/tools/gcp_computeengine_cud_test.go b/tools/gcp_computeengine_cud_test.go index a92966c..4ddacea 100644 --- a/tools/gcp_computeengine_cud_test.go +++ b/tools/gcp_computeengine_cud_test.go @@ -131,7 +131,7 @@ func TestGCPComputeEngineCUDPurchaseHandleDryRunNeverCallsProvider(t *testing.T) assert.True(t, resp.DryRun) } -func TestGCPComputeEngineCUDPurchaseHandleRealPurchase(t *testing.T) { +func TestGCPComputeEngineCUDPurchaseHandleRealPurchaseRefused(t *testing.T) { t.Parallel() fake := &fakeServiceClient{purchaseResult: common.PurchaseResult{Success: true, CommitmentID: "cud-1"}} var gotService common.ServiceType @@ -149,11 +149,11 @@ func TestGCPComputeEngineCUDPurchaseHandleRealPurchase(t *testing.T) { args.DryRun = boolPtr(false) args.Confirm = boolPtr(true) - _, resp, err := tool.handle(context.Background(), nil, args) - require.NoError(t, err) - assert.True(t, resp.Success) + // Real GCP CUD purchases are refused until USD totals exist (go#321). + _, _, err := tool.handle(context.Background(), nil, args) + require.ErrorContains(t, err, "go#321") assert.Equal(t, common.ServiceCompute, gotService) - assert.Equal(t, common.PurchaseSourceMCP, fake.lastOpts.Source) + assert.Zero(t, fake.purchaseCalls) } // TestGCPComputeEngineRecommendationSetsMonthlyPaymentOption pins that the diff --git a/tools/purchase.go b/tools/purchase.go index 5df6fee..a678f8b 100644 --- a/tools/purchase.go +++ b/tools/purchase.go @@ -576,26 +576,32 @@ func logPurchaseOutcome(rec common.Recommendation, token, commitmentID string, s // read. Order matters: both the operator opt-in and the credential-scope // check run before ResolveClient, so a refusal never resolves credentials or // contacts a provider. -func authorizeRealPurchase(req PurchaseRequest, rec common.Recommendation) error { +func authorizeRealPurchase(req PurchaseRequest, rec common.Recommendation) (float64, error) { // Operator authorization. See EnvEnableRealPurchases for why the // model-supplied confirm flag alone is not enough. if !realPurchasesEnabled() { - return fmt.Errorf("real purchases are disabled: set %s=1 to allow the MCP server to execute real purchases", + return 0, fmt.Errorf("real purchases are disabled: set %s=1 to allow the MCP server to execute real purchases", EnvEnableRealPurchases) } // Operator spend ceiling, per call. See enforceSpendCaps. if err := enforceSpendCaps(rec); err != nil { - return err + return 0, err + } + // The USD cap is read here, before any credential or network use; the + // total itself needs the provider client, so it is checked in ExecutePurchase. + usdLimit, err := readUSDCap() + if err != nil { + return 0, err } // Target account must be determinable. See requireCredentialScope for why // an undeterminable one is a double-purchase hazard, not a cosmetic gap. if err := requireCredentialScope(rec.Provider, req.CredentialScope); err != nil { - return err + return 0, err } if req.ResolveClient == nil { - return fmt.Errorf("internal error: no ResolveClient configured for real purchase") + return 0, fmt.Errorf("internal error: no ResolveClient configured for real purchase") } - return nil + return usdLimit, nil } // ExecutePurchase runs the shared dry_run/confirm safety gate, then every @@ -631,7 +637,8 @@ func ExecutePurchase(ctx context.Context, req PurchaseRequest) (*PurchaseRespons // against the err declared above, while `err =` trips gocritic's // sloppyReassign. A distinct name satisfies both (same reason as // validateSavingsPlanArgs' commitErr in aws_savingsplans.go). - if authErr := authorizeRealPurchase(req, rec); authErr != nil { + usdLimit, authErr := authorizeRealPurchase(req, rec) + if authErr != nil { return nil, authErr } @@ -640,6 +647,12 @@ func ExecutePurchase(ctx context.Context, req PurchaseRequest) (*PurchaseRespons return nil, fmt.Errorf("resolve %s service client: %w", rec.Provider, err) } + // Like a ResolveClient failure, a cap refusal happens before any purchase + // attempt and writes no audit record. + if _, capErr := enforceUSDCap(ctx, client, rec, usdLimit); capErr != nil { + return nil, capErr + } + token := common.DeriveIdempotencyToken(idempotencyKeyFor(req.Region, rec, req.CredentialScope, req.Nonce), 0) opts := common.PurchaseOptions{ Source: common.PurchaseSourceMCP, @@ -663,6 +676,11 @@ func ExecutePurchase(ctx context.Context, req PurchaseRequest) (*PurchaseRespons logPurchaseOutcome(rec, token, auditResult.CommitmentID, purchaseSucceeded(auditResult), auditResult.Error) recordPurchaseAudit(rec, req.CredentialScope, auditResult, auditStatusFor(auditResult), false) + return purchaseResponse(rec, result), nil +} + +// purchaseResponse maps a provider result to the tool response. +func purchaseResponse(rec common.Recommendation, result common.PurchaseResult) *PurchaseResponse { resp := &PurchaseResponse{ Success: purchaseSucceeded(result), DryRun: result.DryRun, @@ -685,5 +703,5 @@ func ExecutePurchase(ctx context.Context, req PurchaseRequest) (*PurchaseRespons if purchaseSucceeded(result) { resp.Archera = archeraOffer() } - return resp, nil + return resp } diff --git a/tools/purchase_test.go b/tools/purchase_test.go index 70dc97e..ce873c1 100644 --- a/tools/purchase_test.go +++ b/tools/purchase_test.go @@ -45,7 +45,7 @@ func TestMain(m *testing.M) { // Generous default caps so suites that predate the spend ceiling keep // exercising what happens once a purchase is authorized. The cap tests // override these (non-parallel) per case. - for env, v := range map[string]string{EnvMaxCount: "1000000", EnvMaxHourlyCommitment: "1000000", EnvMaxMemoryGB: "1000000"} { + for env, v := range map[string]string{EnvMaxCount: "1000000", EnvMaxHourlyCommitment: "1000000", EnvMaxMemoryGB: "1000000", EnvMaxCommitmentUSD: "1000000000"} { if err := os.Setenv(env, v); err != nil { log.Printf("set %s for MCP tests: %v", env, err) os.Exit(1) @@ -76,10 +76,13 @@ func TestMain(m *testing.M) { // PurchaseCommitment is exercised by these tests; the rest of the interface // is implemented trivially to satisfy the type. type fakeServiceClient struct { - purchaseResult common.PurchaseResult purchaseErr error + offeringErr error + offering *common.OfferingDetails + purchaseResult common.PurchaseResult lastOpts common.PurchaseOptions purchaseCalls int + offeringCalls int } func (f *fakeServiceClient) GetServiceType() common.ServiceType { return common.ServiceEC2 } @@ -100,7 +103,11 @@ func (f *fakeServiceClient) ValidateOffering(_ context.Context, _ common.Recomme return nil } func (f *fakeServiceClient) GetOfferingDetails(_ context.Context, _ common.Recommendation) (*common.OfferingDetails, error) { - return nil, nil + f.offeringCalls++ + if f.offering == nil && f.offeringErr == nil { + return &common.OfferingDetails{TotalCost: 1, Currency: "USD"}, nil + } + return f.offering, f.offeringErr } func (f *fakeServiceClient) GetValidResourceTypes(_ context.Context) ([]string, error) { return nil, nil diff --git a/tools/search_dedupe_test.go b/tools/search_dedupe_test.go index 50dffcf..ab83249 100644 --- a/tools/search_dedupe_test.go +++ b/tools/search_dedupe_test.go @@ -40,10 +40,10 @@ var dedupeRecent = time.Now().Add(-time.Hour) // listingClient is a ServiceClient whose existing-commitment listing is // scripted and counted. type listingClient struct { - err error - fakeServiceClient + err error commitments []common.Commitment - listCalls int + fakeServiceClient + listCalls int } func (l *listingClient) GetExistingCommitments(_ context.Context) ([]common.Commitment, error) { diff --git a/tools/spend_caps.go b/tools/spend_caps.go index 2c7c55d..6c8482c 100644 --- a/tools/spend_caps.go +++ b/tools/spend_caps.go @@ -12,8 +12,9 @@ import ( // Operator-set spend caps. They are environment variables, not tool // arguments: a cap the model could raise would not bound a hallucinating or -// prompt-injected model. Each applies PER CALL, not cumulatively, and -// none is a USD cap: they bound the size of one purchase request. +// prompt-injected model. Each applies PER CALL, not cumulatively. +// EnvMaxCommitmentUSD bounds the total USD of one purchase; the others bound +// its size. const ( // EnvMaxCount caps rec.Count (instances or nodes; vCPUs for GCP CUDs) // for every non-Savings-Plan real purchase. @@ -23,6 +24,9 @@ const ( // EnvMaxMemoryGB caps the memory committed by a GCP CUD. MaxCount bounds // only vcpu_count, so memory_gb needs its own ceiling. EnvMaxMemoryGB = "CUDLY_MCP_MAX_MEMORY_GB" + // EnvMaxCommitmentUSD caps the total USD one purchase commits over its + // full term, priced by the provider just before the purchase. + EnvMaxCommitmentUSD = "CUDLY_MCP_MAX_COMMITMENT_USD" ) // capSetting names one cap for error messages: the environment variable and, @@ -36,8 +40,11 @@ var ( capCount = capSetting{EnvMaxCount, "Maximum count per purchase call"} capHourly = capSetting{EnvMaxHourlyCommitment, "Maximum hourly commitment (USD/hour)"} capMemory = capSetting{EnvMaxMemoryGB, "Maximum committed memory (GB)"} + capUSD = capSetting{EnvMaxCommitmentUSD, "Maximum commitment per purchase call (USD)"} ) +func readUSDCap() (float64, error) { return capUSD.readFloat() } + func (c capSetting) unset() error { return fmt.Errorf("refusing real purchase: spend cap %s is not set; set it (Claude Desktop: %q) to the largest value one purchase call may use", c.env, c.desktop) diff --git a/tools/spend_caps_test.go b/tools/spend_caps_test.go index a0f4640..6347253 100644 --- a/tools/spend_caps_test.go +++ b/tools/spend_caps_test.go @@ -174,6 +174,11 @@ func TestSpendCapCountAcrossEveryCountBearingBuilder(t *testing.T) { atCap := rec atCap.Count = 1000 _, _, calls, err = runReal(t, atCap) + if name == "gcp" { // real GCP CUD purchases are refused by the USD cap (go#321) + require.ErrorContains(t, err, "go#321", name) + assert.Zero(t, calls, name) + continue + } require.NoError(t, err, name) assert.Equal(t, 1, calls, name) } @@ -194,8 +199,8 @@ func TestSpendCapGCPMemory(t *testing.T) { rec, _, _, _, err = gcpComputeEngineRecommendationFromArgs(args) require.NoError(t, err) _, _, calls, err = runReal(t, rec) - require.NoError(t, err) - assert.Equal(t, 1, calls) + require.ErrorContains(t, err, "go#321", "within the memory cap, the USD cap still refuses GCP CUDs") + assert.Zero(t, calls) rec.Details = &common.ComputeDetails{MemoryGB: 1} _, _, _, err = runReal(t, rec) diff --git a/tools/usd_cap_wiring_test.go b/tools/usd_cap_wiring_test.go new file mode 100644 index 0000000..4dcfa2c --- /dev/null +++ b/tools/usd_cap_wiring_test.go @@ -0,0 +1,83 @@ +package tools + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/LeanerCloud/cloud-commitments-go/pkg/common" + "github.com/LeanerCloud/cloud-commitments-go/pkg/provider" +) + +// runRealWith drives a confirmed real purchase against fake and reports +// whether the client was resolved. +func runRealWith(fake *fakeServiceClient, rec common.Recommendation) (resolved bool, err error) { + _, err = ExecutePurchase(context.Background(), PurchaseRequest{ + Region: rec.Region, Recommendation: rec, Confirm: true, CredentialScope: "scope", + ResolveClient: func(_ context.Context) (provider.ServiceClient, error) { + resolved = true + return fake, nil + }, + }) + return resolved, err +} + +// The issue's shape: a small count cap admits 50 x u-24tb1 (~$286M). +func TestUSDCapRegressionExpensiveRI(t *testing.T) { + t.Setenv(EnvMaxCommitmentUSD, "1000") + fake := &fakeServiceClient{ + purchaseResult: common.PurchaseResult{Success: true, CommitmentID: "id"}, + offering: &common.OfferingDetails{TotalCost: 286e6 / 50, Currency: "USD"}, + } + rec := testRecommendation() + rec.ResourceType = "u-24tb1.112xlarge" + rec.Count = 50 + + resolved, err := runRealWith(fake, rec) + require.ErrorContains(t, err, EnvMaxCommitmentUSD) + assert.Zero(t, fake.purchaseCalls) + assert.True(t, resolved, "pricing needs the client") + assert.Equal(t, 1, fake.offeringCalls) +} + +func TestUSDCapUnpriceableRefusedBeforePurchase(t *testing.T) { + t.Setenv(EnvMaxCommitmentUSD, "1000") + fake := &fakeServiceClient{offeringErr: assert.AnError} + _, err := runRealWith(fake, testRecommendation()) + require.ErrorContains(t, err, "cannot determine USD total") + assert.Zero(t, fake.purchaseCalls) +} + +func TestUSDCapUnsetRefusesBeforeResolve(t *testing.T) { + t.Setenv(EnvMaxCommitmentUSD, "") + fake := &fakeServiceClient{} + resolved, err := runRealWith(fake, testRecommendation()) + require.ErrorContains(t, err, EnvMaxCommitmentUSD) + assert.False(t, resolved) + assert.Zero(t, fake.offeringCalls) + assert.Zero(t, fake.purchaseCalls) +} + +func TestUSDCapEnvParsing(t *testing.T) { + for _, v := range []string{"", " ", "abc", "NaN", "Inf", "-1", "0"} { + t.Setenv(EnvMaxCommitmentUSD, v) + _, err := readUSDCap() + require.ErrorContains(t, err, EnvMaxCommitmentUSD, "value %q", v) + } + t.Setenv(EnvMaxCommitmentUSD, "1e3") + got, err := readUSDCap() + require.NoError(t, err) + assert.InDelta(t, 1000, got, 1e-9) +} + +func TestUSDCapRefusesRealGCPCUDPurchase(t *testing.T) { + rec, _, _, _, err := gcpComputeEngineRecommendationFromArgs(validGCPCUDArgs()) + require.NoError(t, err) + fake := &fakeServiceClient{} + _, err = runRealWith(fake, rec) + require.ErrorContains(t, err, "go#321") + assert.Zero(t, fake.purchaseCalls) + assert.Zero(t, fake.offeringCalls) +}