[IAmJSD]

Root cause

AzureSPProber.probeCombo (internal/commitmentopts/probe_azure.go) built every Savings Plan ValidatePurchase probe request with a hardcoded CurrencyCode: "USD". Azure checks a commitment against the billing account's currency, so on a non-USD account (EUR, GBP, ...) the probe asked about a purchase that account could never make, and the verdict it got back was about a made-up currency, not the account's.

Fix

• ProbeAzure(ctx, cred, currencyCode string) now takes the billing account's currency from the caller as an explicit parameter next to the credential. Currency belongs to the account, like the credential, so it is a per-call argument rather than a field on the prober. A prober shared between accounts can't leak one account's currency into another, and there's no mutable state to race on.
• New azureProbeCurrency checks the code with ^[A-Za-z]{3}$ and upper-cases it. This matches commitmentCurrency in the cloud-commitments-go Azure Savings Plans provider (go#46), so the probe and the real purchase path accept the same inputs.
• Fail closed: an empty or malformed code returns an error before the client is built or any ValidatePurchase call is sent. There is no USD default.
• probeCombo receives the validated code and puts it in Commitment.CurrencyCode.

No production code calls ProbeAzure today: DefaultAzureProbers is only used in tests, and nothing emits Azure SP recommendations yet, as the issue notes. So the signature change only touches tests. Whoever wires the probe in later will have to supply a resolved currency.

Closes #814

Verification

All evidence below is unit/mock: the Azure ValidatePurchase client is a fake AzureSPValidateAPI. No real Azure API was called. Gap: I have not checked against a live non-USD Azure billing account how ValidatePurchase treats a currency mismatch.

New tests in internal/commitmentopts/probe_azure_test.go:

• TestAzureSPProber_RequestUsesCallerCurrency: EUR, gbp and USD each reach every one of the 6 candidate requests as EUR, GBP and USD.
• TestAzureSPProber_UnknownCurrency_FailsClosed: "", US, EURO, U$D, 12A and " usd" each return an error, build no client and make zero ValidatePurchase calls.
• The existing ProbeAzure tests now pass "USD" explicitly.

Fail before / pass after:

• New tests against the original probe_azure.go from main: the build fails (want ("context".Context, azcore.TokenCredential)), because the old API has no way to pass a currency.
• Mutant with the new signature but the old behavior (currency ignored, "USD" hardcoded, no validation): RequestUsesCallerCurrency/{EUR,gbp} and all 6 UnknownCurrency_FailsClosed subtests FAIL.
• Mutant that drops strings.ToUpper: RequestUsesCallerCurrency/gbp FAILS.
• Mutant that removes the regex anchors ([A-Za-z]{3}): UnknownCurrency_FailsClosed FAILS.
• Mutant that hardcodes "USD" only in the request body: RequestUsesCallerCurrency FAILS.
• With the fix: all 15 AzureSP*/DefaultAzureProbers tests pass.

Commands (run with AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null, because the local ~/.aws/config default profile is invalid on the dev machine; this package doesn't touch AWS config):

go build ./...                                        # ok
go vet ./internal/commitmentopts/                     # ok
go test ./internal/commitmentopts/ -count=1           # ok
go test ./internal/mocks/ -count=1                    # ok (repo-wide test guards)
golangci-lint run --allow-parallel-runners ./internal/commitmentopts/...   # v2.10.1: 0 issues
gocyclo -over 10 -ignore "_test\.go" internal/commitmentopts/            # silent
bash scripts/gofmt-hook.sh <changed files>            # ok
bash scripts/gosec-hook.sh <changed files>            # gosec v2.28.0: ok
bash scripts/test-git-secrets-allowlist.sh            # 42 PASS, 0 FAIL (includes file-argument scan of all tracked files)
git diff --check                                      # clean

No DB, migration or shared-state changes, so there is no concurrency surface to race-test.

Out of scope (noted, not changed)

• The ProbeAzure doc comment says "Service.probeAndPersistAzure wires it up", but that function doesn't exist; nothing in production calls the Azure prober. I left the comment and the wiring alone. Whoever adds probeAndPersistAzure will need to resolve the account's billing currency (for example from the billing account / Cost Management currency) and pass it in.
• The probe still uses a placeholder all-zero subscription as its billing scope (azureProbeSubscriptionID). That's a separate question from currency.

Labels

@cristim, could a maintainer please mirror the issue's triage labels onto this PR: priority/p2, severity/medium, triaged, urgency/this-quarter, impact/few, effort/s, type/bug.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • Azure billing probes now require a valid three-letter currency code.
  • Currency codes are normalized to uppercase before billing validation.
  • Invalid or missing currency values are rejected before any Azure requests are made.
  • Azure billing checks now use the specified currency instead of always defaulting to USD.

[coderabbitai]

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: e15f7094-5288-49ce-9cd9-cf32a61ac0a1

📥 Commits

Reviewing files that changed from the base of the PR and between 8caed39 and f0a1492.

📒 Files selected for processing: 2

• internal/commitmentopts/probe_azure.go
• internal/commitmentopts/probe_azure_test.go

+------------------------+
| GLaDOS? I can fix her! |
+------------------------+
  \
   \     /)/)
        ( '')
        /  |
      o(___)

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

Independent review (claude-cc-platform reviewer, cold brief): APPROVE f0a149237aa6e1f09acdb0a1e6edadaf01816c51

No blocking findings. The fix matches #814: the hardcoded "USD" is gone, and the currency is an explicit caller argument with no default. Empty or malformed input errors in azureProbeCurrency before a client is built or ValidatePurchase is called. The format check is the same ^[A-Za-z]{3}$ the provider uses (cloud-commitments-go providers/azure/services/savingsplans/client.go:287-299).

Callers: nothing outside probe_azure*.go calls ProbeAzure, AzureSPProber or DefaultAzureProbers, so no production path relies on USD today. Whoever wires the production caller must resolve the real billing currency from Azure. A wrong but well-formed code will surface as an Azure mismatch error or verdict.

Money model: the probe sends a 0.001 hourly placeholder and keeps only the term/plan validity, so a non-USD probe does not conflict with the USD-only money model.

Mutation probes (all caught):

• request body hardcoded to "USD": the EUR and gbp subtests fail;
• strings.ToUpper dropped: the gbp subtest fails;
• regex anchors removed: the EURO and _usd subtests fail;
• empty string defaulting to USD: the empty-string case fails;
• validation error swallowed: four subtests fail.

Base drift: 19 commits behind main, but none touch these files, and the merge is clean (tree 5325ab8a). go test ./internal/commitmentopts/ passes at the head and on the merged tree.

Nits:

• The doc comment at probe_azure.go:109-121 names a non-existent probeAndPersistAzure.
• The PR does not yet carry fix(azure): commitmentopts validate probe hardcodes CurrencyCode USD #814's labels.

Not verified: live Azure currency-mismatch behaviour (needs an account).

[cristim]

Independent review (claude-fleet): APPROVE f0a1492

Findings (none blocking):

• internal/commitmentopts/probe_azure.go:122 [non-blocking] - Nothing resolves the currency end to end.
  • ProbeAzure takes the currency from its caller, but no non-test caller exists. A grep for ProbeAzure, AzureSPProber and DefaultAzureProbers outside probe_azure*.go finds only docs and audits.
  • No billing account, subscription or config source is wired in.
  • Failure scenario: none today, because the path never runs. Whoever wires it later must resolve the real billing currency, and the only guard against a wrong but well-formed code is Azure's own verdict.
  • This matches the PR's stated scope and the issue's "fail closed when unknown".
• internal/commitmentopts/probe_azure.go:119-121 [non-blocking, pre-existing] - The doc comment says Service.probeAndPersistAzure wires this up. That function does not exist; this is already logged in docs/audits/codebase-audit-2026-09-02.md:7758. Failure scenario: a reader concludes the Azure probe runs in production.
• Caching and tenant bleed: not applicable.
  • Service.probeAndPersist (internal/commitmentopts/service.go:76) ranges only over the AWS s.probers, so no persisted or cached Azure result exists to key by currency.
  • The currency is a per-call argument, not a field on the shared prober, so concurrent multi-tenant callers cannot leak one tenant's currency into another's call.

Attacked:

• Vendor contract: armbillingbenefits v1.0.0 (pinned in go.mod) has Commitment.CurrencyCode *string, JSON currencyCode (models.go:50, models_serde.go:105/122). The PR sets exactly that field (probe_azure.go:189).
• Hard-coded USD: the removed currencyCode := "USD" was the only default in the file, and none remains.
• Duplication: the validation regex and ToUpper mirror commitmentCurrency in cloud-commitments-go (providers/azure/services/savingsplans/client.go:315-327). That function is unexported and lives in another module, so the mirror is unavoidable. The comment at probe_azure.go:32-35 references the original.
• Input checks: ASCII [A-Za-z] runs before upper-casing, so Unicode case-fold inputs are rejected. The FailsClosed test covers empty, 2 letters, 4 letters, symbols, digits and leading whitespace. It also asserts that no client is built and no ValidatePurchase call is made.
• Mutation probes on a scratch copy of HEAD, each restored and confirmed identical with cmp:
  1. CurrencyCode: to.Ptr("USD") in the request body: RequestUsesCallerCurrency/EUR and /gbp FAIL.
  2. An empty currency defaults to "USD" before validation: UnknownCurrency_FailsClosed/#00 (empty) FAILS.
  3. The validation error is ignored and the raw input passed through: all 6 UnknownCurrency_FailsClosed subtests FAIL.
  4. strings.ToUpper dropped: RequestUsesCallerCurrency/gbp FAILS.

Verified locally, on a git archive export of HEAD with no cloud credentials:

• go build ./...: OK.
• go vet ./internal/commitmentopts/: OK.
• go test -count=1 -race ./internal/commitmentopts/: ok.
• gofmt -l on both changed files: clean.
• The new probe_azure_test.go against the base export fails to build (signature change), so the tests fail before the fix.
• Not verified: how live Azure ValidatePurchase behaves on a currency mismatch. That needs a real non-USD billing account, and no cloud calls were made.

Reviewed by an independent agent with no author context. Review only: nothing was pushed, and the GitHub review state is unchanged.

[cristim]

Local verification (macOS, exact head f0a149237aa6e1f09acdb0a1e6edadaf01816c51, detached worktree): go build ./..., go test -race -short -count=1 ./... and make lint pass.

Gate status: two independent reviews approve this SHA (posted above), local verification passes, and CI is green. Merge waits on @IAmJSD marking the PR ready (see #974).

[cristim]

Marked ready and merged by the owner's agent after 2 independent reviews and local verification (see the review and verification comments above, all at f0a149237aa6e1f09acdb0a1e6edadaf01816c51).