sec(money): currency is dropped at every layer boundary, so USD-denominated caps are compared against non-USD amounts [go part] #124

Split from LeanerCloud/cloud-commitments-cli#1637 (cross-component issue, monorepo split)

This is the pkg/ and providers/-scoped half of a cross-cutting class finding. Nothing in the original issue touches cmd/*.go or docs/cli/, so cloud-commitments-cli is not an affected repo and the original issue is closed with backlinks rather than kept open. The internal/-scoped half of the same class went to cloud-commitments-platform (see the shared root-cause section below, repeated in both issues since the fix direction only makes sense read as a whole).

Reviewed commit: be11bdcb5. Re-verified against this repo's current tree (2026-09-28); all three new instances below still reproduce, with updated line numbers where they had drifted.

What this issue is (and is not)

This is the class, not another copy of the instances. Four instances already have their own issues in cloud-commitments-cli (not yet split as of this writing, since they predate this monorepo split and are independently fixable regardless of which repo they end up scoped to):

• #1558 fix(exchange): RI-exchange spend caps are compared against a quote whose CurrencyCode is never checked (this repo: pkg/exchange; ledger half in the platform repo: internal/config/store_postgres.go)
• #1555 fix(api): Azure revoke refund-divergence guard is opt-in and currency-blind (platform repo: internal/api/handler_purchases_revoke.go)
• #1562 fix(azure): Savings Plan purchase hardcodes CurrencyCode USD (this repo: providers/azure/services/savingsplans/client.go)
• #1087 AWS Savings Plans purchase path is USD-only (this repo: providers/aws/services/savingsplans/client.go)

Those stay open and stay independently fixable; do not double-fix them here or as part of this split. This issue (and its cloud-commitments-platform sibling) exist because fixing those one at a time will not close the class: the same shape reappears in more places, 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:357-360 states the assumption in a comment: the amountValue helper unwraps Modern's *Amount{Currency, Value} to a bare float and discards the currency on the stated assumption that 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. This repo's pkg/exchange's MaxPaymentDueUSD *big.Rat (pkg/exchange/exchange.go:104) and MaxPaymentPerExchangeUSD / MaxPaymentDailyUSD (pkg/exchange/auto.go:74-75); same shape for internal/auth/types.go's MaxPurchaseAmount in the platform repo. The USD is in the identifier, not in the type.

The result is that a guardrail comparison is 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 in this repo (new in this sweep, not filed elsewhere)

• GCP Money.CurrencyCode is never read

  • Where: providers/gcp/services/computeengine/client.go:1241-1260 (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 (platform repo). The Duration half compounds it: a projection over a 1-year duration is stored as a monthly figure, 12x overstated.
  • Fix direction: 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.

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

  • Where: providers/azure/recommendations.go:441 (rec.EstimatedSavings = extFloat(ext, "annualSavingsAmount") / 12), extFloat at :453-460.
  • 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.
  • Fix direction: 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:357-364 (amountValue), called from the converter's amount-extraction path.
  • 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 because downstream Recommendation consumers assume a single-currency view per subscription, same as the Legacy path. The sibling amountValuePtr immediately below (:366-376) 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.
  • Fix direction: this is the load-bearing fix in the "carry the currency to the guardrail" step below; amountValue's caller chain needs to carry the source Amount.Currency into common.Recommendation rather than discarding it 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 above 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. This repo's pkg/exchange caps (checkInitialQuote / checkReQuote / the auto-exchange caps), and the platform repo's Azure revoke divergence guard / MaxPurchaseAmount evaluation. PR #1515 already settled the rule for one call site; this applies the same rule at the rest.
3. Delete the "USD" defaults (the platform repo's firstNonEmptyCurrency, and the two Azure SP literals tracked on #1562) and replace them with an explicit unknown that step 2 then refuses.

Deliberately not in scope, 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 (#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 (platform repo). With step 2 in place no non-USD amount can be written, so the daily-spend ledger query cannot sum across denominations. The column is needed on the day the product supports a second currency, and should be added with that work.

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

• #1558, #1555, #1562, #1087 (the individually-filed instances, in cloud-commitments-cli as of this writing).
• #1081 (make PurchaseHistoryRecord.MonthlyCost a *float64 to distinguish absent from zero): same root shape (money metadata dropped at a layer boundary), different metadata.
• Prior art: PR #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.

Acceptance criteria

• All three new instances above are fixed to fail closed on a non-USD or unresolvable currency, with a regression test that exercises a genuinely non-USD input.
• common.Recommendation (or an equivalent carrier) gains a place to hold the currency, coordinated with the cloud-commitments-platform sibling issue so the value actually reaches the guardrails there.