fix(purchase): refuse a plan-less execution whose recs span cloud accounts

[cristim]

What

Closes #1902.

A direct-execute purchase whose selected recommendations spanned two cloud accounts never fanned out. singleCloudAccountIDFromRecs could not distinguish "no account on any recommendation" from "more than one distinct account", returning an empty result for both, and the caller then built a provider client from the host's ambient credentials. Every commitment for both target accounts was bought in the CUDly host account, and the saved history stamped the ambient identity. The per-recommendation permission check had already passed against the real target accounts, so authorization succeeded for accounts the money never reached.

The helper is now exported as SingleCloudAccountIDFromRecs and returns errAmbiguousAccountScope for that shape. resolveSingleAccountProvider propagates it, so every executor entry point fails closed, and validateExecutePurchaseRecommendations applies the same rule at the API boundary so a 400 comes back naming the accounts before an execution row is persisted.

Three things the issue did not say

Seven entry points share the defect, not one. Direct execute, session approval, email-token approval, the SQS approve and execute messages, retry, the delayed scheduled fire, the cron path and the reaper re-drive all funnel through executeSingleAccount for a plan-less row. One guard at the resolver covers all of them; the API guard additionally stops the row being created.

A third defective shape. A batch mixing attributed and unattributed recommendations resolved to the attributed account and bought the unattributed one under those credentials. That is now refused too.

The helper scanned the wrong set. It inspected every recommendation on the execution while the purchase only buys the selected ones, so an unselected row from another account could push a legitimate single-account batch onto the ambient path. It now scans selected recommendations only.

Refusal, not fan-out

Plan-less fan-out would need per-account child rows carrying recommendation subsets, because executeForAccount copies every recommendation to every account by design, plus history, retry and re-drive support for those children. That is a feature, and this is an effort/s defect. The frontend already sends one request per bucket, so the refusal is not reachable through normal use. A follow-up should add cloud_account_id to the frontend's bucket key so it stays that way.

The provider factory is deliberately not guarded

The obvious second guard would be to refuse a nil provider config at the factory. That would break real deployments: resolveGCPProvider returns a nil config for application default credentials by design, and the ambient single-account deployment relies on the same contract. Verified that after this change a nil config reaches a purchase only from GCP ADC and from a batch whose selected recommendations carry no account at all.

Two purchase paths outside purchase.Manager are unaffected by design: the MCP tool has its own explicit-account refusal, and the CLI runs in ambient local mode.

Verification

Both regression tests were observed failing on the pre-fix code, and the failures are the real defect rather than a setup error: the two-account batch dispatched under the host's ambient path, and the mixed batch dispatched both recommendations under the first account's credentials. Both purchases actually completed pre-fix. After the change, neither reaches CreateAndValidateProvider, PurchaseCommitment, SavePurchaseHistory, GetCloudAccount or the confirmation email, and no recommendation is marked purchased.

Check	Result
go build ./..., go vet ./...	exit 0
go test ./internal/purchase/ ./internal/api/	ok
go test ./... (full backend suite)	every package ok
golangci-lint at the CI pin	0 issues
gocyclo -over 10 on both touched files	clean (8, 9 and 5)

A second commit fixes three UK spellings that the CI Lint job's US-locale misspell rule would have rejected. The pre-commit hook does not run misspell, so that would have failed only after pushing. It also corrects executePurchase's doc header, which still described the non-fan-out branch as falling back to ambient credentials.

Note for a maintainer

The API guard and the resolver guard call the same exported function, so the two cannot drift. The 400 names the offending accounts, which is deliberate: the operator needs to know which accounts collided, and the caller already had permission on all of them.

Summary by CodeRabbit

• Bug Fixes
  • Plan-less purchase executions now require recommendations to reference exactly one cloud account.
  • Submissions spanning multiple accounts, or mixing account-attributed and unattributed recommendations, are rejected with a clear HTTP 400 error.
  • Invalid purchase requests no longer create executions, send notifications, look up accounts, or purchase recommendations.
  • Valid selections continue using the account identified by the selected recommendations.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Essentials

Run ID: 19838d71-8d73-4ccf-82f1-b1c41f951909

📥 Commits

Reviewing files that changed from the base of the PR and between aa26544 and ff8702f.

📒 Files selected for processing (5)

• internal/api/handler_purchases.go
• internal/api/handler_purchases_test.go
• internal/purchase/execution.go
• internal/purchase/execution_test.go
• internal/purchase/money_path_regression_test.go

Included review availability: 3 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.

---

📝 Walkthrough

Walkthrough

Plan-less purchases now derive account scope from selected recommendations. Multi-account and mixed-attribution selections return errors before provider creation, execution persistence, notifications, or purchase execution. The API rejects invalid batches with HTTP 400.

Changes

Purchase scope validation

Layer / File(s)	Summary
Account scope validator internal/purchase/execution.go, internal/purchase/execution_test.go	SingleCloudAccountIDFromRecs evaluates selected recommendations, returns one account when unambiguous, and reports multi-account or mixed-attribution selections with deterministic errors.
Execution account resolution internal/purchase/execution.go, internal/purchase/money_path_regression_test.go	Plan-less execution uses validated recommendation scope and stops before account lookup, provider creation, persistence, notifications, or purchases when scope is ambiguous.
API request rejection internal/api/handler_purchases.go, internal/api/handler_purchases_test.go	Direct and approval-mode requests spanning multiple accounts return HTTP 400 before execution creation or approval execution.

Priority: ⬆️ High — Impact reflects high issue severity.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Severity of issue fixed: High

Merge Risk: ⚪ Minimal · up to ff870

Plan-less purchases spanning ambiguous account scopes are rejected before execution or persistence, preventing ambient credentials from purchasing in the wrong account. The change is ready to merge.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: rejecting plan-less executions whose recommendations span cloud accounts.
Linked Issues check	✅ Passed	The changes satisfy issue [#1902]. They distinguish ambiguous account scopes, reject multi-account and mixed-attribution plan-less executions, preserve valid ambient-credential cases, and prevent exec…
Out of Scope Changes check	✅ Passed	The production changes, documentation updates, and regression tests directly support the linked issue and PR objectives. No unrelated code changes are identified.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 5 files.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/1902-multi-account-direct-execute

---

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