sec(money): currency is dropped at every layer boundary, so USD-denominated caps are compared against non-USD amounts

[cristim]

Reviewed commit: be11bdcb5. Note: origin/main moved to 3e9660d06 during the review; re-verify against current main before changing code, since a finding may have been fixed or moved.

Cross-cutting sweep from the 2026-07-28 full-repo review. Every instance below was re-verified against origin/main @ 3e9660d06 while writing this issue; where a stated line number had moved, the corrected one is given.

What this issue is (and is not)

This is the class, not another copy of the instances. Four instances already have their own issues:

• #1558 fix(exchange): RI-exchange spend caps are compared against a quote whose CurrencyCode is never checked
• #1555 fix(api): Azure revoke refund-divergence guard is opt-in and currency-blind
• #1562 fix(azure): Savings Plan purchase hardcodes CurrencyCode USD
• #1087 AWS Savings Plans purchase path is USD-only; aws-cn accounts can onboard but can't buy SPs

Those stay open and stay independently fixable. This issue exists because fixing them one at a time will not close the class: the same shape reappears in at least five more places that nobody has filed, and a per-site fix leaves the underlying type design intact so the next money field added reintroduces it.

The shared root cause

Two facts, together:

1. Money crosses every layer boundary as a bare float64 (or a bare decimal string), with the currency dropped at the boundary. The currency is frequently read from the provider response and then either stored in a display-only field or discarded outright. providers/azure/internal/recommendations/converter.go:297-306 states the assumption in a comment: "Currency is discarded -- downstream Recommendation consumers assume a single-currency view per subscription". common.Recommendation carries no currency field at all, so there is nowhere for it to go even if a caller wanted to keep it.

2. Every spend cap in the system is USD-denominated by naming convention only, with no currency field to compare against. internal/auth/types.go:65-66:

   // MaxPurchaseAmount limits the maximum purchase amount
   MaxPurchaseAmount float64 `json:"max_purchase_amount,omitempty" dynamodbav:"MaxPurchaseAmount"`

   Same for pkg/exchange's MaxPaymentDueUSD *big.Rat (pkg/exchange/exchange.go:104) and MaxPaymentPerExchangeUSD / MaxPaymentDailyUSD (pkg/exchange/auto.go:74-75). The USD is in the identifier, not in the type.

The result is that a guardrail comparison like paymentDue.Cmp(maxPayment) == 1 is a comparison between two numbers whose units are asserted by a variable name. On a non-USD deployment (aws-cn, an MCA subscription billed in EUR, a GBP-billed GCP billing account) that comparison is wrong in whichever direction the exchange rate happens to point: it silently blocks a legal purchase, or it silently authorises one an order of magnitude over the operator's stated ceiling.

The correct behaviour is to fail closed on a non-USD money path, not to compare across currencies. A guardrail that cannot establish the denomination of the number it is checking has not checked anything, and "assume USD" is exactly the fabricated-value pattern the project's no-silent-fallbacks rule exists to prevent. Where the currency genuinely cannot be resolved, the money path must error out and say so, rather than proceed on an assumption.

---

Instances

Already filed individually (listed for completeness of the class; do not double-fix)

• pkg/exchange spend caps are compared against a quote whose CurrencyCode is never read (filed as #1558)

  • Where: pkg/exchange/exchange.go:252 (CurrencyCode: sdkaws.ToString(out.CurrencyCode) -- captured into ExchangeQuoteSummary.CurrencyCode, exchange.go:23), then checkInitialQuote at pkg/exchange/exchange.go:311-321 and checkReQuote at :326-338; the auto path at pkg/exchange/auto.go:318-324 (per-exchange cap) and :505-535 (daily cap).
  • What: getQuoteWithAPI stores the AWS-reported currency and no guardrail ever reads it. checkInitialQuote is a bare paymentDue.Cmp(maxPayment) == 1. checkReQuote's error message even hardcodes the unit -- "re-quoted payment %s USD exceeds cap %s USD" -- on a number whose currency was never checked.
  • Ledger half: internal/config/store_postgres.go:2572-2577 (CompleteRIExchangeWithPayment writes payment_due) and GetRIExchangeDailySpend at :2633+ SUMs that column with no currency column on the table, so the daily-cap ledger silently mixes denominations across rows.

• Azure revoke: calcRefundCurrency is computed but used only in the error message (filed as #1555)

  • Where: internal/api/handler_purchases_revoke.go:716-731 (azureCalculateRefund reads BillingRefundAmount.CurrencyCode into calcRefundCurrency at :726-728), consumed at :647, interpolated into the divergence error at :661, and passed to MarkPurchaseRevoked at :765 / :773.
  • What: the TOCTOU divergence guard is math.Abs on two bare floats. The currency is carried alongside for the whole call chain and never compared to anything. The client-supplied ExpectedRefundAmount *float64 (:73) carries no currency at all, so there is nothing to compare it to.

• Azure Savings Plan purchase hardcodes CurrencyCode: "USD" (filed as #1562)

  • Where: providers/azure/services/savingsplans/client.go:261 and :355 -- both armbillingbenefits.Commitment literals carry CurrencyCode: toPtr("USD") with Amount taken verbatim from the recommendation.
  • What: Azure requires the commitment currency to match the billing account's currency. Nothing resolves or validates it. Note this is the write side of the same gap that providers/azure/internal/recommendations/converter.go:297-306 creates on the read side by discarding the currency the Consumption API returned.

• AWS Savings Plans purchase path is USD-only (filed as #1087)

  • Where: providers/aws/services/savingsplans/client.go:303 (Currencies: []types.CurrencyCode{types.CurrencyCodeUsd} on the offering query, with the comment "Pin to USD so non-USD currency offerings are excluded server-side") and :635 (Currency: "USD").
  • Note: this one is arguably the correct shape already -- it pins the currency explicitly rather than assuming it -- but it fails silently (an aws-cn account gets an empty offering list) instead of erroring with "this deployment cannot buy SPs in CNY". Keep it under AWS Savings Plans purchase path is USD-only; aws-cn accounts can onboard but can't buy SPs cloud-commitments-go#7; listed here so the class census is complete.

Not filed anywhere -- new in this sweep

• max_payment_due_usd is never asserted to actually be USD

  • Where: internal/api/handler_ri_exchange.go:717-718 (required-field check), :749 (exchange.ParseDecimalRat(body.MaxPaymentDueUSD)), :766 (maxPayment, _ := maxRat.Float64()), :791 (MaxPaymentDueUSD: maxRat); request field declared at :913. Cap type at internal/auth/types.go:65-66.
  • What: the request body field is named max_payment_due_usd and the handler's own error text calls it a "safety guardrail", but no code path establishes that the exchange being guarded is USD-denominated. The value becomes MaxPurchaseAmount, which has no currency field, and is also forwarded to pkg/exchange as MaxPaymentDueUSD. The name is the only thing carrying the unit, across three layers.
  • Failure scenario: an aws-cn deployment. The operator sets max_payment_due_usd: "1000" meaning a thousand dollars. AWS quotes PaymentDue = "5000" in CNY (roughly USD 690). checkInitialQuote compares 5000 against 1000, refuses, and the operator sees a cap breach that is not one. The reverse case -- a currency weaker per unit than the cap assumes -- authorises an exchange over the real ceiling with no signal.
  • Secondary defect at the same site: maxPayment, _ := maxRat.Float64() at :766 discards the exactness flag. Bounded (only inexact at absurd magnitudes) but it is the same "drop the metadata at the boundary" move. Keep the cap comparison in *big.Rat.

• GCP Money.CurrencyCode is never read

  • Where: providers/gcp/services/computeengine/client.go:1045-1067 (extractCostImpactFromRecommendation).
  • What: the function reads costProj.Cost (a google.type.Money), negates Units + Nanos/1e9 into rec.EstimatedSavings, and never touches Cost.CurrencyCode. recommenderpb.CostProjection also carries Duration, likewise unread -- the value is treated as monthly by assumption. So this single function drops both the unit and the period.
  • Failure scenario: a GBP-billed billing account. The CUD recommendation is stored with EstimatedSavings in pounds, ranked against USD-denominated recommendations from other providers on the same dashboard, and checked against a USD-denominated MaxPurchaseAmount. The Duration half compounds it: a projection over a 1-year duration is stored as a monthly figure, 12x overstated.
  • Fix: normalise by Duration to a monthly run-rate, and drop the recommendation with a log when CurrencyCode is absent or is not the deployment's expected billing currency.

• firstNonEmptyCurrency labels reshape output "USD" by fiat when no RI reports a currency

  • Where: internal/api/handler_ri_exchange.go:623-637, consumed at :594 (currencyCode := firstNonEmptyCurrency(instances)).
  • What: the helper walks the ConvertibleRI list and returns the first non-empty CurrencyCode; when none has one it returns the literal "USD". The unknown case is rendered as a definite answer.
  • Failure scenario: an aws-cn deployment whose RIs come back without CurrencyCode sees CNY-denominated savings figures rendered and compared as USD on the Reshape page. Bounded and display-side today because the field is populated in practice, which is exactly why it will survive until the day it is not.
  • Fix: return ("", false) and omit the currency from the response rather than defaulting.

• Azure Advisor: savingsCurrency is never read, and an unparsable savings amount becomes a real 0

  • Where: providers/azure/recommendations.go:442 (rec.EstimatedSavings = extFloat(ext, "annualSavingsAmount") / 12), extFloat at :455-462.
  • What: Advisor's extendedProperties carries a savingsCurrency key that nothing reads. extFloat returns 0 for a missing key and for a strconv.ParseFloat failure, so a payload like "1,234.00" (thousands separator) yields a recommendation valued at exactly zero rather than an error. This instance sits at the intersection of this issue and the zero-vs-absent sweep filed alongside it.
  • Fix: read and validate savingsCurrency; make extFloat return (float64, error) or *float64 so an unparsable amount drops the recommendation with a log instead of shipping a fabricated 0.

• providers/azure/internal/recommendations/converter.go discards the currency at the read boundary, by design

  • Where: providers/azure/internal/recommendations/converter.go:297-306 (amountValue), called at :207-209.
  • What: this is the documented root of the Azure half. amountValue unwraps Modern's *armconsumption.Amount{Currency, Value} to a bare float and the doc comment states the assumption outright: "Currency is discarded -- downstream Recommendation consumers assume a single-currency view per subscription, same as the Legacy path." The sibling amountValuePtr immediately below (:308-320) shows the codebase already knows how to preserve a signal it cares about -- it keeps the absent-vs-zero distinction -- so the currency drop is a deliberate scope decision, not an oversight. It is the decision this issue is asking to revisit.
  • Note on provenance: the review's findings file cited this as internal/recommendations/converter.go:294-303. That path does not exist; the correct path is providers/azure/internal/recommendations/converter.go and the function is at :297-306 on current main. Corrected here.

---

Fix direction for the class

Fixing the individual sites without this will not hold. Suggested order:

1. Carry the currency to the guardrail. amountValue, extractCostImpactFromRecommendation and the Advisor path must stop dropping it, and common.Recommendation needs somewhere to put it. This is the load-bearing step: a comparison cannot fail closed on something it never received.
2. Make every guardrail fail closed when the amount is not USD, or its currency is unknown. checkInitialQuote / checkReQuote / the auto-exchange caps / the Azure revoke divergence guard / MaxPurchaseAmount evaluation. PR feat(azure/ri-exchange): find compatible offerings and execute exchange (closes #596) #1515 already settled the rule for one call site; this applies the same rule at the rest.
3. Delete the "USD" defaults (firstNonEmptyCurrency, the two Azure SP literals) and replace them with an explicit unknown that step 2 then refuses.

Deliberately not in this issue, unless and until CUDly actually transacts in a second currency:

• A Money{Amount, Currency} type in pkg/common, or a currency field on MaxPurchaseAmount and the pkg/exchange cap fields. The caps are USD by construction (feat(azure/ri-exchange): find compatible offerings and execute exchange (closes #596) #1515), so giving them a field to hold "USD" adds a value that is never anything else, threaded through three layers. What the comparison is missing is the denomination of the incoming amount, which is step 1.
• A currency column on ri_exchange_history.payment_due. With step 2 in place no non-USD amount can be written, so GetRIExchangeDailySpend cannot sum across denominations. The column is needed on the day the product supports a second currency, and should be added with that work, where its correctness can be tested against a real non-USD path.

Remedy simplified (2026-08-03): the Money type / cap currency field and the ledger column were removed from the plan, leaving carry-the-currency plus fail-closed-on-non-USD, which is what closes every failure scenario listed above. Both dropped items build multi-currency capability, and this issue's own conclusion (and #1515's) is that a non-USD money path must refuse rather than transact. Multi-currency support is a feature; it deserves its own issue rather than arriving as the fix for a fail-open.

Regression tests must exercise a genuinely non-USD path (a quote with CurrencyCode: "JPY", an EUR-billed MCA subscription fixture), not a USD path with an extra assertion. A test that only ever feeds USD will pass with every one of these defects present.

Related

• fix(exchange): RI-exchange spend caps are compared against a quote whose CurrencyCode is never checked cloud-commitments-go#42, fix(api): Azure revoke refund-divergence guard is opt-in and currency-blind cloud-commitments-platform#96, fix(azure): Savings Plan purchase hardcodes CurrencyCode USD cloud-commitments-go#46, AWS Savings Plans purchase path is USD-only; aws-cn accounts can onboard but can't buy SPs cloud-commitments-go#7 (the individual instances, above).
• fix(exchange): make PurchaseHistoryRecord.MonthlyCost a *float64 to distinguish absent from zero #1081 (make PurchaseHistoryRecord.MonthlyCost a *float64 to distinguish absent from zero) and the zero-vs-absent sweep filed alongside this one: same root shape (money metadata dropped at a layer boundary), different metadata.
• Prior art in this repo: PR feat(azure/ri-exchange): find compatible offerings and execute exchange (closes #596) #1515 established that MaxPurchaseAmount has no currency field and that a non-USD money path must fail closed. This issue is that conclusion applied across the whole surface rather than at one call site.

[cristim]

Split as part of the monorepo split (cloud-commitments-cli / -go / -mcp / -platform). Nothing in this issue touches cmd/*.go or docs/cli/, so this repo is not an affected repo; closing rather than scoping down.

• Platform-repo instances (max_payment_due_usd never asserted as USD, firstNonEmptyCurrency defaulting to "USD"), plus the shared root-cause and fix-direction narrative: sec(money): currency is dropped at every layer boundary, so USD-denominated caps are compared against non-USD amounts [platform part] cloud-commitments-platform#361
• Go-repo instances (GCP Money.CurrencyCode never read, Azure Advisor savingsCurrency never read, providers/azure/internal/recommendations/converter.go discarding currency by design), plus the same shared narrative: sec(money): currency is dropped at every layer boundary, so USD-denominated caps are compared against non-USD amounts [go part] cloud-commitments-go#124

The four already-individually-filed instances (#1558, #1555, #1562, #1087) are untouched by this split and stay open in this repo as-is; they are referenced for completeness in both new issues but not duplicated.