diff --git a/CHANGELOG.md b/CHANGELOG.md index a6d356e..3a135cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -137,6 +137,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/). ### Fixed +- The cumulative budget (`CUDLY_MCP_BUDGET_USD`, in memory and in + `CUDLY_MCP_BUDGET_STATE`) adds and compares whole cents instead of float64 + dollars, so a residual such as 0.1 + 0.2 = 0.30000000000000004 no longer + refuses a purchase that exactly fits. Each purchase total rounds up to the cent + and the budget rounds down, so spend is never undercounted. The state file + format is unchanged; an older file's fraction of a cent reads rounded up, and a + `null` amount is now corrupt instead of 0 (#82). - A partial- or all-upfront dry run whose offering cannot be priced now writes an audit record (`status: "error"`, `dry_run: true`) instead of returning without one, as the README promises for previews (Refs #129). diff --git a/README.md b/README.md index 701f72d..9ab8bd5 100644 --- a/README.md +++ b/README.md @@ -125,7 +125,7 @@ Every other provider's purchase tool (`cudly_aws_savingsplans_purchase`, `cudly_ - A real `partial-upfront` or `all-upfront` EC2, RDS, OpenSearch, Redshift or MemoryDB RI purchase requires `approved_upfront` and `approved_currency` copied from its preview. They are passed to the provider as the approved upfront, and the provider refuses to buy an offering whose upfront (`FixedPrice` x `count`) exceeds it by more than 5%, so a price change between preview and purchase cannot buy at the higher price. The figure is caller-supplied: it bounds price drift, not a deliberately inflated figure, which the USD cap and budget below still bound. `approved_upfront` must be a positive number and `approved_currency` must be `USD`; both are rejected for `no-upfront`. - `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_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. `CUDLY_MCP_BUDGET_USD` (positive number, USD) is also required and is a **cumulative** budget: the USD totals of all real purchases in this server process may not exceed it, so a fresh `idempotency_nonce`, region or profile cannot repeat a purchase past it. **Limits you must understand:** the other caps apply **per call**. By default the budget is held in memory, so it resets when the server restarts and is not shared across processes or MCP clients; set `CUDLY_MCP_BUDGET_STATE` to persist it (see [Persisted budget](#persisted-budget)). Either way it cannot see purchases made in the cloud console, CLI or web. A purchase whose outcome is unknown, or that fails after the provider was called, counts as spent. Two cases are released: a clean no-op, and a purchase the provider refused before any purchase call (for example the approved-upfront price guard), which it marks with `common.ErrRefusedBeforePurchase`. An existing commitment the library adopts on an idempotent retry is released, but the retry still needs headroom for its own total at the time of the call. Choose the USD cap as the most one call may ever spend. +- 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. `CUDLY_MCP_BUDGET_USD` (positive number, USD) is also required and is a **cumulative** budget: the USD totals of all real purchases in this server process may not exceed it, so a fresh `idempotency_nonce`, region or profile cannot repeat a purchase past it. **Limits you must understand:** the other caps apply **per call**. By default the budget is held in memory, so it resets when the server restarts and is not shared across processes or MCP clients; set `CUDLY_MCP_BUDGET_STATE` to persist it (see [Persisted budget](#persisted-budget)). Either way it cannot see purchases made in the cloud console, CLI or web. A purchase whose outcome is unknown, or that fails after the provider was called, counts as spent. Two cases are released: a clean no-op, and a purchase the provider refused before any purchase call (for example the approved-upfront price guard), which it marks with `common.ErrRefusedBeforePurchase`. An existing commitment the library adopts on an idempotent retry is released, but the retry still needs headroom for its own total at the time of the call. Choose the USD cap as the most one call may ever spend. The budget counts whole cents: each purchase total rounds up to the cent and the budget rounds down, so rounding never admits more than the budget. - 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. @@ -179,7 +179,7 @@ Set `CUDLY_MCP_BUDGET_STATE` to a file path to keep the cumulative budget across - **Window:** one calendar month in UTC. The first purchase check in a new month starts `spent_usd` again from 0; there is no other automatic reset. Reservations still unsettled carry into the new month and keep counting until settled or reset. A file dated after the current UTC month (clock skew) refuses purchases and is not rewritten. - **File:** JSON, written atomically (temp file, fsync, rename) under an exclusive lock on `.lock`, so concurrent servers cannot overspend together. The parent directory is created `0700` and the files `0600`, as for the audit log. Locking needs Linux or macOS; elsewhere a configured file refuses purchases. -- **Format:** `{"month": "2026-10", "reservations": [{"id": "...", "usd": 40}], "spent_usd": 60, "version": 1}`. A reservation is written before the purchase call and settled after. One still present (a crash or timeout mid-purchase) counts as spent. +- **Format:** `{"month": "2026-10", "reservations": [{"id": "...", "usd": 40}], "spent_usd": 60, "version": 1}`. A reservation is written before the purchase call and settled after. One still present (a crash or timeout mid-purchase) counts as spent. Amounts are written as exact cents; a fraction of a cent (from a file written by an older release) reads rounded up, and `null`, a quoted or a negative amount makes the file corrupt. - **Fail closed:** a missing file is a fresh budget. A file that is corrupt, has an unknown version, or cannot be read or locked within 10 seconds refuses every real purchase, with the refusal recorded in the audit log. It is never overwritten. - **Stale temp files:** a crash between creating the temp file and the rename can leave a `.tmp-*` file next to the state file. It is safe to delete. - **Reset:** stop every server that uses the file, then delete the file (or edit `spent_usd` and `reservations`) and restart. Do this after confirming whether an unsettled reservation actually bought anything. diff --git a/tools/attempt_store_test.go b/tools/attempt_store_test.go index ae78bc9..a519ff9 100644 --- a/tools/attempt_store_test.go +++ b/tools/attempt_store_test.go @@ -97,7 +97,7 @@ func mustBudgetSpent(t *testing.T, path string) float64 { t.Helper() st, err := newFileBudget(path).load() require.NoError(t, err) - return st.totalUSD() + return float64(st.totalUSD()) / 100 } type recordingProviderOpts struct { diff --git a/tools/budget.go b/tools/budget.go index 1aadb00..d2fa33b 100644 --- a/tools/budget.go +++ b/tools/budget.go @@ -16,8 +16,8 @@ import ( // clients, and cannot see purchases made elsewhere. type budgetLedger struct { mu sync.Mutex - committed float64 // USD of purchases that were, or may have been, made - reserved float64 // USD of purchases in flight + committed usdCents // purchases that were, or may have been, made + reserved usdCents // purchases in flight } var processBudget = &budgetLedger{} @@ -49,17 +49,35 @@ type budgetReservation struct { ledger *budgetLedger // in-memory mode file *fileBudget // persisted mode (EnvBudgetState) id string // file mode: reservation ID in the state file - usd float64 - committed float64 // USD settled before this reservation, for logging + usd usdCents + committed usdCents // settled before this reservation, for logging +} + +// budgetCents converts a purchase's USD total and the budget limit for the +// ledgers (spend rounds up, the limit down; see spendCents). +func budgetCents(usd, limit float64) (spend, lim usdCents, err error) { + spend, err = spendCents(usd) + if err != nil { + return 0, 0, fmt.Errorf("refusing real purchase: purchase total: %w", err) + } + lim, err = limitCents(limit) + if err != nil { + return 0, 0, fmt.Errorf("refusing real purchase: %s: %w", EnvBudgetUSD, err) + } + return spend, lim, nil } // reserve refuses when committed+reserved+usd would exceed limit (equal is // allowed) and otherwise holds usd until settle. -func (l *budgetLedger) reserve(usd, limit float64) (*budgetReservation, error) { +func (l *budgetLedger) reserve(usdF, limitF float64) (*budgetReservation, error) { + usd, limit, err := budgetCents(usdF, limitF) + if err != nil { + return nil, err + } l.mu.Lock() defer l.mu.Unlock() if l.committed+l.reserved+usd > limit { - return nil, fmt.Errorf("refusing real purchase: %.2f USD would exceed the cumulative budget %s=%.2f (%.2f USD remaining; the budget is per process and resets on restart)", + return nil, fmt.Errorf("refusing real purchase: %s USD would exceed the cumulative budget %s=%s (%s USD remaining; the budget is per process and resets on restart)", usd, EnvBudgetUSD, limit, max(0, limit-l.committed-l.reserved)) } l.reserved += usd @@ -83,7 +101,7 @@ func (r *budgetReservation) settle(spent bool) { func (l *budgetLedger) committedUSD() float64 { l.mu.Lock() defer l.mu.Unlock() - return l.committed + return float64(l.committed) / 100 } // purchaseSpent reports whether a purchase that reached PurchaseCommitment diff --git a/tools/budget_cents.go b/tools/budget_cents.go new file mode 100644 index 0000000..dc84af9 --- /dev/null +++ b/tools/budget_cents.go @@ -0,0 +1,78 @@ +package tools + +import ( + "fmt" + "math/big" + "strconv" +) + +// usdCents is a budget amount in whole US cents. The cumulative budget adds +// and compares cents so float residuals cannot flip a boundary check (#82). +// In JSON it is a USD number ("spent_usd": 60.25), the format older releases +// wrote, so the state file stays readable across versions. +type usdCents int64 + +// maxBudgetUSD bounds every budget amount. Below it, cents/100 formats to its +// exact decimal, so a state file round-trips without loss. +const maxBudgetUSD = 1e13 + +const maxBudgetCents = usdCents(maxBudgetUSD * 100) + +// spendCents converts a spend amount, rounding up so spend is never +// undercounted. Half-even or half-up rounding could drop up to half a cent. +func spendCents(usd float64) (usdCents, error) { + return toCents(usd, true) +} + +// limitCents converts a budget limit, rounding down, so rounding can only +// refuse earlier than exact arithmetic, never admit more. +func limitCents(usd float64) (usdCents, error) { + return toCents(usd, false) +} + +// toCents converts exactly, from the shortest decimal that prints usd (0.07 is +// 7 cents, although 0.07*100 is 7.000000000000001 in float64). NaN and ±Inf +// print as words that do not parse. +func toCents(usd float64, roundUp bool) (usdCents, error) { + r, ok := new(big.Rat).SetString(strconv.FormatFloat(usd, 'g', -1, 64)) + if !ok { + return 0, fmt.Errorf("budget amount %v is not a finite number", usd) + } + return ratCents(r, roundUp) +} + +func ratCents(r *big.Rat, roundUp bool) (usdCents, error) { + if r.Sign() < 0 || r.Cmp(new(big.Rat).SetFloat64(maxBudgetUSD)) > 0 { + return 0, fmt.Errorf("budget amount %s USD is outside 0 to %.0f", r.FloatString(2), float64(maxBudgetUSD)) + } + c := new(big.Rat).Mul(r, big.NewRat(100, 1)) + q, m := new(big.Int).QuoRem(c.Num(), c.Denom(), new(big.Int)) + if roundUp && m.Sign() > 0 { + q.Add(q, big.NewInt(1)) + } + return usdCents(q.Int64()), nil +} + +// String formats the amount as USD with two decimals. +func (c usdCents) String() string { return fmt.Sprintf("%d.%02d", c/100, c%100) } + +func (c usdCents) MarshalJSON() ([]byte, error) { + return []byte(strconv.FormatFloat(float64(c)/100, 'f', -1, 64)), nil +} + +// UnmarshalJSON reads the number literal exactly and rounds it up to cents, so +// a non-cent amount in a file written before #82 is never undercounted. null, +// a quoted number or a negative or out-of-range amount is an error (encoding/json +// passes the raw token, and none of those parse as a big.Rat in range). +func (c *usdCents) UnmarshalJSON(b []byte) error { + r, ok := new(big.Rat).SetString(string(b)) + if !ok { + return fmt.Errorf("budget amount %s is not a number", b) + } + v, err := ratCents(r, true) + if err != nil { + return err + } + *c = v + return nil +} diff --git a/tools/budget_cents_test.go b/tools/budget_cents_test.go new file mode 100644 index 0000000..daa135f --- /dev/null +++ b/tools/budget_cents_test.go @@ -0,0 +1,98 @@ +package tools + +import ( + "encoding/json" + "math" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// #82: the ledger adds and compares whole cents, so float residuals cannot +// flip an exact-boundary check. +func TestBudgetLedgerExactBoundary(t *testing.T) { + l := &budgetLedger{} + _, err := l.reserve(0.1, 0.3) + require.NoError(t, err) + _, err = l.reserve(0.2, 0.3) + require.NoError(t, err, "0.10 + 0.20 is exactly the 0.30 budget") +} + +func TestBudgetLedgerReleaseLeavesNoResidual(t *testing.T) { + l := &budgetLedger{} + a, err := l.reserve(0.1, 0.3) + require.NoError(t, err) + b, err := l.reserve(0.2, 0.3) + require.NoError(t, err) + a.settle(false) + b.settle(false) + _, err = l.reserve(0.3, 0.3) + require.NoError(t, err, "after releasing everything the whole budget is free") +} + +// Spend rounds up and the limit rounds down, so rounding never admits more +// than exact arithmetic would. +func TestBudgetLedgerRoundsTowardRefusing(t *testing.T) { + l := &budgetLedger{} + _, err := l.reserve(0.011, 0.02) // 2 cents of 2 + require.NoError(t, err) + _, err = l.reserve(0.001, 0.02) + require.Error(t, err, "a sub-cent spend counts as a whole cent") + + l = &budgetLedger{} + _, err = l.reserve(0.01, 0.019) // the limit is 1 cent + require.NoError(t, err) + _, err = l.reserve(0.001, 0.019) + require.Error(t, err, "a sub-cent budget remainder is not spendable") +} + +func TestBudgetLedgerRefusesUnrepresentableAmounts(t *testing.T) { + for _, usd := range []float64{1e14, math.NaN(), math.Inf(1), -1} { + _, err := (&budgetLedger{}).reserve(usd, maxBudgetUSD) + assert.Error(t, err, "spend %v", usd) + } + for _, limit := range []float64{maxBudgetUSD + 1, math.Inf(1), math.NaN()} { + _, err := (&budgetLedger{}).reserve(1, limit) + assert.Error(t, err, "limit %v", limit) + } +} + +func TestSpendAndLimitCents(t *testing.T) { + for _, c := range []struct { + usd float64 + spend, lim usdCents + }{ + {0, 0, 0}, {0.07, 7, 7}, {0.29, 29, 29}, {1.005, 101, 100}, {0.001, 1, 0}, {60, 6000, 6000}, + {1e-300, 1, 0}, {maxBudgetUSD, maxBudgetUSD * 100, maxBudgetUSD * 100}, + } { + s, err := spendCents(c.usd) + require.NoError(t, err, c.usd) + assert.Equal(t, c.spend, s, "spend %v", c.usd) + l, err := limitCents(c.usd) + require.NoError(t, err, c.usd) + assert.Equal(t, c.lim, l, "limit %v", c.usd) + } + _, err := spendCents(maxBudgetUSD + 0.01) + assert.Error(t, err) +} + +func TestUSDCentsJSON(t *testing.T) { + for cents, want := range map[usdCents]string{0: "0", 30: "0.3", 6000: "60", 1234: "12.34", 5: "0.05", maxBudgetUSD * 100: "10000000000000"} { + b, err := json.Marshal(cents) + require.NoError(t, err) + assert.Equal(t, want, string(b)) + var back usdCents + require.NoError(t, json.Unmarshal(b, &back)) + assert.Equal(t, cents, back, "round trip %s", want) + } + for raw, want := range map[string]usdCents{"0.105": 11, "1e2": 10000, "0.30000000000000004": 31} { + var got usdCents + require.NoError(t, json.Unmarshal([]byte(raw), &got), raw) + assert.Equal(t, want, got, "a legacy non-cent amount %s rounds up", raw) + } + for _, raw := range []string{`null`, `"1"`, `-1`, `true`, `1e14`} { + var got usdCents + assert.Error(t, json.Unmarshal([]byte(raw), &got), raw) + } +} diff --git a/tools/budget_persist_test.go b/tools/budget_persist_test.go index aecee5e..758e8b8 100644 --- a/tools/budget_persist_test.go +++ b/tools/budget_persist_test.go @@ -35,7 +35,7 @@ func TestPersistedBudgetSurvivesRestart(t *testing.T) { err := runWithNonce(fake, "b") require.ErrorContains(t, err, EnvBudgetUSD) assert.Equal(t, 1, fake.purchaseCalls) - assert.InDelta(t, 60, readState(t, path).SpentUSD, 1e-9) + assert.Equal(t, usdCents(6000), readState(t, path).SpentUSD) assert.Zero(t, processBudget.committedUSD(), "persisted mode must not use the in-memory ledger") } @@ -44,7 +44,7 @@ func TestPersistedBudgetFailedPurchaseKeepsCounted(t *testing.T) { fake := pricedFake(60, common.PurchaseResult{}, assert.AnError) require.Error(t, runWithNonce(fake, "a")) st := readState(t, path) - assert.InDelta(t, 60, st.SpentUSD, 1e-9) + assert.Equal(t, usdCents(6000), st.SpentUSD) assert.Empty(t, st.Reservations) } @@ -123,7 +123,7 @@ func TestPersistedBudgetKeepsCountedWhenPurchaseAPIFailsAfterGuard(t *testing.T) _, _, err = tool.handle(context.Background(), nil, confirmed(args, preview.ApprovedUpfront, preview.ApprovedCurrency)) require.Error(t, err) assert.Equal(t, 1, api.purchaseCalls) - assert.Greater(t, readState(t, path).SpentUSD, 0.0) + assert.Greater(t, readState(t, path).SpentUSD, usdCents(0)) } type failingPurchaseEC2API struct{ *pricedEC2API } diff --git a/tools/budget_settle_test.go b/tools/budget_settle_test.go index 832e756..30fadea 100644 --- a/tools/budget_settle_test.go +++ b/tools/budget_settle_test.go @@ -43,7 +43,7 @@ func TestBudgetPanicInPurchaseStaysReserved(t *testing.T) { processBudget.mu.Lock() reserved := processBudget.reserved processBudget.mu.Unlock() - assert.InDelta(t, 60, reserved, 1e-9) + assert.Equal(t, usdCents(6000), reserved) other := pricedFake(60, common.PurchaseResult{Success: true, CommitmentID: "id"}, nil) require.ErrorContains(t, runWithNonce(other, "b"), EnvBudgetUSD) diff --git a/tools/budget_state.go b/tools/budget_state.go index 0b9c891..532a594 100644 --- a/tools/budget_state.go +++ b/tools/budget_state.go @@ -7,7 +7,6 @@ import ( "fmt" "io/fs" "log" - "math" "os" "path/filepath" "time" @@ -28,17 +27,17 @@ const budgetLockTimeout = 10 * time.Second type budgetState struct { Month string `json:"month"` Reservations []budgetReservEntry `json:"reservations"` - SpentUSD float64 `json:"spent_usd"` + SpentUSD usdCents `json:"spent_usd"` Version int `json:"version"` } type budgetReservEntry struct { - ID string `json:"id"` - USD float64 `json:"usd"` + ID string `json:"id"` + USD usdCents `json:"usd"` } // totalUSD is spend plus every unsettled reservation. -func (s budgetState) totalUSD() float64 { +func (s budgetState) totalUSD() usdCents { total := s.SpentUSD for _, r := range s.Reservations { total += r.USD @@ -46,8 +45,6 @@ func (s budgetState) totalUSD() float64 { return total } -func validBudgetAmount(v float64) bool { return v >= 0 && !math.IsNaN(v) && !math.IsInf(v, 0) } - func (s budgetState) validate() error { if s.Version != budgetStateVersion { return fmt.Errorf("unsupported version %d", s.Version) @@ -55,18 +52,22 @@ func (s budgetState) validate() error { if _, err := time.Parse("2006-01", s.Month); err != nil { return fmt.Errorf("invalid month %q: %w", s.Month, err) } - if !validBudgetAmount(s.SpentUSD) { - return fmt.Errorf("invalid spent_usd %v", s.SpentUSD) - } seen := make(map[string]bool, len(s.Reservations)) + // Each amount is at most maxBudgetCents, so checking before adding cannot + // overflow; a total past the bound would let a wrapped sum read as headroom. + total := s.SpentUSD for _, r := range s.Reservations { - if r.ID == "" || !validBudgetAmount(r.USD) { + if r.ID == "" { return fmt.Errorf("invalid reservation %+v", r) } if seen[r.ID] { return fmt.Errorf("duplicate reservation id %q", r.ID) } seen[r.ID] = true + if total > maxBudgetCents-r.USD { + return fmt.Errorf("spent_usd plus reservations exceeds %.0f USD", float64(maxBudgetUSD)) + } + total += r.USD } return nil } @@ -186,14 +187,18 @@ func writeStateAtomically(path string, v any, what string) error { // reserve refuses when the month's spend plus reservations plus usd would // exceed limit (equal is allowed) and otherwise records a reservation before // returning, so the hold is on disk before the purchase call. -func (f *fileBudget) reserve(usd, limit float64) (*budgetReservation, error) { +func (f *fileBudget) reserve(usdF, limitF float64) (*budgetReservation, error) { + usd, limit, err := budgetCents(usdF, limitF) + if err != nil { + return nil, err + } id := uuid.NewString() - var committed float64 + var committed usdCents exceeded := false - err := f.update(func(st *budgetState) (bool, error) { + err = f.update(func(st *budgetState) (bool, error) { if st.totalUSD()+usd > limit { exceeded = true - return false, fmt.Errorf("refusing real purchase: %.2f USD would exceed the cumulative budget %s=%.2f (%.2f USD remaining in %s UTC; state file %s, resets next calendar month)", + return false, fmt.Errorf("refusing real purchase: %s USD would exceed the cumulative budget %s=%s (%s USD remaining in %s UTC; state file %s, resets next calendar month)", usd, EnvBudgetUSD, limit, max(0, limit-st.totalUSD()), st.Month, f.path) } st.Reservations = append(st.Reservations, budgetReservEntry{ID: id, USD: usd}) @@ -215,7 +220,7 @@ func (f *fileBudget) reserve(usd, limit float64) (*budgetReservation, error) { // when money was spent. A failed write cannot undo the purchase: the // reservation then stays on disk and keeps counting, which errs toward // refusing. -func (f *fileBudget) settle(id string, usd float64, spent bool) { +func (f *fileBudget) settle(id string, usd usdCents, spent bool) { err := f.update(func(st *budgetState) (bool, error) { for i, r := range st.Reservations { if r.ID == id { @@ -229,6 +234,6 @@ func (f *fileBudget) settle(id string, usd float64, spent bool) { return true, nil }) if err != nil { - log.Printf("mcp budget: settle %.2f USD (spent=%t) in %s failed, reservation stays counted: %v", usd, spent, f.path, err) + log.Printf("mcp budget: settle %s USD (spent=%t) in %s failed, reservation stays counted: %v", usd, spent, f.path, err) } } diff --git a/tools/budget_state_test.go b/tools/budget_state_test.go index 0b2b10b..74f6535 100644 --- a/tools/budget_state_test.go +++ b/tools/budget_state_test.go @@ -103,7 +103,7 @@ func TestFileBudgetTwoProcessesShareLimit(t *testing.T) { } assert.Equal(t, 10, total, "limit 100 / 10 per purchase") st := readState(t, path) - assert.InDelta(t, 100, st.SpentUSD, 1e-9) + assert.Equal(t, usdCents(10000), st.SpentUSD) assert.Empty(t, st.Reservations) } @@ -162,7 +162,7 @@ func TestFileBudgetMonthRollover(t *testing.T) { r.settle(true) st := readState(t, path) assert.Equal(t, "2026-11", st.Month) - assert.InDelta(t, 100, st.SpentUSD, 1e-9) + assert.Equal(t, usdCents(10000), st.SpentUSD) } func TestFileBudgetMonthIsUTC(t *testing.T) { @@ -185,7 +185,7 @@ func TestFileBudgetSettleAfterRolloverCountsInNewMonth(t *testing.T) { r.settle(true) st := readState(t, path) assert.Equal(t, "2026-11", st.Month) - assert.InDelta(t, 30, st.SpentUSD, 1e-9) + assert.Equal(t, usdCents(3000), st.SpentUSD) assert.Empty(t, st.Reservations) } @@ -303,3 +303,63 @@ func TestFileBudgetLockFileSymlinkRefused(t *testing.T) { _, err := newFileBudget(path).reserve(1, 100) require.ErrorContains(t, err, "budget lock file") } + +// #82: the persisted ledger sums whole cents and writes exact decimals. +func TestFileBudgetExactCents(t *testing.T) { + path := stateFile(t) + fb := newFileBudget(path) + a, err := fb.reserve(0.1, 0.3) + require.NoError(t, err) + b, err := fb.reserve(0.2, 0.3) + require.NoError(t, err, "0.10 + 0.20 is exactly the 0.30 budget") + a.settle(true) + b.settle(true) + raw, err := os.ReadFile(path) // #nosec G304 -- test temp file + require.NoError(t, err) + assert.Contains(t, string(raw), `"spent_usd": 0.3,`) +} + +// A file written before #82 may hold a non-cent amount; it reads rounded up. +func TestFileBudgetLegacyAmountRoundsUp(t *testing.T) { + path := stateFile(t) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o700)) + month := time.Now().UTC().Format("2006-01") + legacy := fmt.Sprintf(`{"month": %q, "reservations": [{"id": "r1", "usd": 0.001}], "spent_usd": 0.105, "version": 1}`, month) + require.NoError(t, os.WriteFile(path, []byte(legacy), 0o600)) + _, err := newFileBudget(path).reserve(0.01, 0.129) + require.Error(t, err, "0.105 reads as 0.11 and 0.001 as 0.01, so 0.01 more exceeds 0.12") + _, err = newFileBudget(path).reserve(0.01, 0.131) + require.NoError(t, err, "0.11 + 0.01 + 0.01 fits 0.13") +} + +func TestFileBudgetNullAmountIsCorrupt(t *testing.T) { + path := stateFile(t) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o700)) + month := time.Now().UTC().Format("2006-01") + require.NoError(t, os.WriteFile(path, []byte(fmt.Sprintf(`{"month": %q, "reservations": [], "spent_usd": null, "version": 1}`, month)), 0o600)) + _, err := newFileBudget(path).reserve(1, 100) + require.ErrorContains(t, err, "corrupt") +} + +// #82 review: a total past the amount bound is corrupt. Unchecked, enough +// maximum-size reservations wrap the int64 sum negative, which reads as headroom. +func TestFileBudgetTotalOverflowIsCorrupt(t *testing.T) { + month := time.Now().UTC().Format("2006-01") + for name, n := range map[string]int{"wraps negative": 9300, "just over the bound": 1} { + t.Run(name, func(t *testing.T) { + path := stateFile(t) + require.NoError(t, os.MkdirAll(filepath.Dir(path), 0o700)) + var res strings.Builder + for i := range n { + if i > 0 { + res.WriteString(",") + } + fmt.Fprintf(&res, `{"id": "r%d", "usd": %d}`, i, int64(maxBudgetUSD)) + } + legacy := fmt.Sprintf(`{"month": %q, "reservations": [%s], "spent_usd": 0.01, "version": 1}`, month, res.String()) + require.NoError(t, os.WriteFile(path, []byte(legacy), 0o600)) + _, err := newFileBudget(path).reserve(1, 100) + require.ErrorContains(t, err, "corrupt") + }) + } +} diff --git a/tools/purchase.go b/tools/purchase.go index 402d42c..1326c2c 100644 --- a/tools/purchase.go +++ b/tools/purchase.go @@ -377,7 +377,7 @@ func preflightRealPurchase(ctx context.Context, req PurchaseRequest, rec common. if err != nil { return nil, nil, err } - log.Printf("mcp budget: reserved %.2f of %.2f USD (committed %.2f)", usd, limits.budget, reservation.committed) + log.Printf("mcp budget: reserved %s of %.2f USD (committed %s)", reservation.usd, limits.budget, reservation.committed) return client, reservation, nil }