fix(api): sign no-cloud-account marketplace rows as the host only for the host account

[IAmJSD]

Root cause

Issue #261 reported that marketplace list and cancel signed EC2 calls with the host's ambient credentials (loadAWSConfigWithRegion) for any account. The main part was fixed by #554 (merged as e522f3d): loadMarketplaceAWSConfig resolves the row's cloud account through credentials.ResolveAWSCredentialProviderWithOpts, with the same ambient options the purchase path uses. #554 said "Refs #261", so the issue stayed open.

One branch still fell back to ambient credentials. When cloud_account_id was NULL or blank, loadMarketplaceAWSConfig returned the host config without checking which AWS account the RI belongs to. The issue's suggested fix covers this case ("return 400 when the row has no CloudAccountID ..."). Such rows are not only legacy rows:

• the ambient single-account purchase path still writes them (purchase.SingleCloudAccountIDFromRecs returns nil, and executeSingleAccount stamps the ambient STS identity into account_id);
• migration 000011 added the column with no backfill;
• the FK is ON DELETE SET NULL.

Fix

loadMarketplaceHostAWSConfig handles a row with no cloud account. It uses the host credentials only when the row's account_id passes credentials.VerifyHostAccount. That is the existing check the resolver applies to a role_arn account with no role ARN, so no new resolver is added. Every other case gets a 400 before any EC2 client is built:

• empty or blank account_id;
• a different account;
• a failed STS GetCallerIdentity.

The message has a fixed text and does not contain the host account id. This keeps the ambient single-account deployment working (its rows carry the host id) and fails closed for everything else.

Handler / path	Row with cloud account	Row with NULL/blank cloud account (before)	Row with NULL/blank cloud account (after)
marketplaceList (create listing, incl. offering-class lookup and attempt reconciliation finder)	resolver, per #554 (unchanged)	host credentials, any account	host credentials only if account_id is the host; else 400, no EC2 call
marketplaceCancel	resolver, per #554 (unchanged)	host credentials, any account	same as above
Compensating cancel after a failed DB write in list	reuses the list's client (unchanged)	host	inherits the list's decision
authorizeSessionSell	unchanged	unchanged (NULL rows reachable only by unrestricted sell-any)	unchanged

Create-listing is marketplaceList (POST .../marketplace-list). All of its AWS calls go through the same cfg: FetchOfferingClass, CreateMarketplaceListing, the buildMarketplaceFinder reconcile and the rollback cancel. The fix covers all of them. No other marketplace handler builds an AWS config.

Relationship to #560

This PR covers part of #560 but does not close it. #560's acceptance is "never signs with host credentials unless the row's AccountID is the host account". This PR enforces that, with a test for each branch.

What #560 still has to decide is the non-host branch. Today a NULL-account row whose account_id is a different, registered AWS account is refused with 400. #560 suggests resolving it by (aws, account_id), as #547/#553 do, and using that account's credentials. To do that, #560 replaces the refusal in loadMarketplaceHostAWSConfig with a lookup: if it resolves, call the existing resolver path; if not, keep the 400. The host branch and the fail-closed default stay as they are.

Behaviour change

A NULL-account row whose account_id is empty or is not the host account now gets 400 instead of a call signed as the host. That call was always wrong, because host credentials cannot act on another account's RI (AWS returned InvalidReservedInstancesId.NotFound, mapped to a misleading 400). An STS failure during the host check is also a 400. This matches how ErrNotHostAccount is handled today for role_arn self accounts. A host-account NULL row now makes one extra STS GetCallerIdentity call per list/cancel.

Verification

Fakes only. No real AWS or STS calls were made, and no listings were created or canceled. STS is a fake aws.HTTPClient transport; EC2 is the existing stubMarketplaceEC2 through marketplaceEC2Factory. Tests ran with AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null.

New tests in internal/api/handler_marketplace_credentials_test.go (replacing the test that documented the host fallback):

• TestLoadMarketplaceAWSConfig_NoCloudAccountHostRowUsesHostCredentials: nil, empty and blank cloud_account_id with the host account_id get the host key, after exactly one host identity check.
• TestLoadMarketplaceAWSConfig_NoCloudAccountRefusedUnlessHost: member account, empty and blank account_id (no STS call), and an STS failure each get 400 with the fixed message and no credentials.
• TestMarketplaceHandlers_NoCloudAccountRow: list and cancel end to end. A member-account row is refused with no EC2 client built; a host-account row is signed with the host key; GetCloudAccount is never called.
• The existing fix(api): sign marketplace list and cancel with the purchase account's credentials #554 tests still pass unchanged: federated access_keys and role_arn rows resolve that account's credentials, non-host role_arn is refused, a nil or erroring account lookup and resolver errors never fall back to the host, and authz is unchanged.
• The integration fixture legacy-account in permission_constraints_marketplace_integration_test.go now gives the NULL-account row the host account_id. Without that fixture change both legacy-account subtests fail with the new code, which is the intended behaviour change.

Fail-before/pass-after: putting back main's return base, nil fails 7 of the new subtests. Mutation probes, each caught by at least one test:

• skip VerifyHostAccount;
• fall back to host credentials on a verify error;
• drop the empty-id guard;
• call GetCloudAccount for a NULL-account row.

Commands run on this revision:

• go build ./..., go vet ./..., go vet -tags integration ./internal/api/
• go test ./internal/api/ -count=1, go test ./internal/credentials/ -count=1, go test ./internal/mocks/ -count=1
• go test -tags integration ./internal/api/ -run 'TestMarketplacePermissionConstraintsHTTP|Compensation' (testcontainers Postgres)
• golangci-lint run --allow-parallel-runners ./internal/api/... (v2.10.1): 0 issues
• gocyclo -over 10 internal/api/handler_marketplace.go: clean
• git-secrets scan of the changed files in a fresh repo with isolated HOME: clean; bash scripts/test-git-secrets-allowlist.sh: 42 PASS, 0 FAIL

Not verified against real AWS/STS.

Out of scope

• The (aws, account_id) lookup for non-host NULL-account rows (fix(api): marketplace rows with a NULL cloud_account_id use the host's ambient credentials #560).
• An STS throttle or network failure during the host check returns 400 instead of a retryable status. This mirrors the existing ErrNotHostAccount handling.
• The RI-exchange handlers' use of loadAWSConfigWithRegion (separate handler family).

Closes #261

🤖 Generated with Claude Code

[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: 484fcc75-ba53-43da-9e06-05c1816dee7b

📥 Commits

Reviewing files that changed from the base of the PR and between 6e80319 and 1e7c394.

📒 Files selected for processing: 3

• internal/api/handler_marketplace.go
• internal/api/handler_marketplace_credentials_test.go
• internal/api/permission_constraints_marketplace_integration_test.go

+-----------------------------------+
| Colonel Panic is my arch-nemesis. |
+-----------------------------------+
  \
   \   (\__/)
       (•ㅅ•)
       / 　 づ

✨ 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 of 1e7c394: draft: blocks ready

No blocking findings.

Findings

• NON-BLOCKING, internal/api/handler_marketplace.go (loadMarketplaceHostAWSConfig, the VerifyHostAccount call): any host STS failure (throttle, 5xx, network) returns the fixed non-retryable 400 "not confirmed as the host account". VerifyHostAccount collapses every failure into ErrNotHostAccount (internal/credentials/host_identity.go:38-41), so the #937 retryable 503 classification cannot apply here. This matches the existing role_arn-without-role path (handler_marketplace.go:90-93), so it is consistent with current behavior; a follow-up could separate transient from mismatch.

Answers

• Host account is whatever STS GetCallerIdentity returns for the ambient credentials (host_identity.go:38,47), compared with row.AccountID. That field is server-written at purchase time (internal/purchase/execution.go:993, from the execution's resolved account), not taken from any request body. Both callers (list at handler_marketplace.go:232, cancel at :349) go through the same loadMarketplaceAWSConfig, and no other caller exists.
• A no-cloud-account row for another account, an empty or blank id, or a failed STS call returns a 400 client error with a fixed message (no host id), before any EC2 client is built. There is no ambient fallback.
• Consistent with #910 (same VerifyHostAccount probe) and with the existing ErrNotHostAccount 400 mapping; see the NON-BLOCKING note for #937.

What I attacked (local, no real AWS)

• Mutation: VerifyHostAccount failure ignored. Fails: NoCloudAccountRefusedUnlessHost (member account, host identity fail) and NoCloudAccountRow list/cancel member_account_refused. Reverted.
• Fail-before: old behavior (return base for nil account) fails the HostRow tests (nil, empty, blank) and the empty, blank and host-identity-fail refusal cases. Pass-after: go test ./internal/api -run 'Marketplace|PermissionConstraints' ok at this SHA.

Gate status

• Independent review: this comment, at the SHA above.
• CI: all checks still pending at review time (draft, fork run); not green yet.
• CodeRabbit: no review comments; not pinged.
• Labels: match issue #261.
• Local verification: unit and handler tests with fake STS transports on macOS; no real-account check.

Blocks ready

• CI green on this SHA.
• Author marks the PR ready for review.

[cristim]

Gate record for 1e7c394 (origin/main CLAUDE.md "Review gate"):

• Independent review of 1e7c394: no blocking findings.
  • "Host account" comes from STS GetCallerIdentity on the ambient credentials (internal/credentials/host_identity.go:38,47). It is compared with row.AccountID, which the server writes at purchase time (internal/purchase/execution.go:993), so it can't be spoofed.
  • List (handler_marketplace.go:232) and cancel (:349) both go through loadMarketplaceAWSConfig.
  • These rows are refused with a fixed 400, before any EC2 client is built and with no ambient fallback: a non-host row with no cloud account, an empty or blank account, or a failed STS call.
  • Non-blocking: a transient host STS failure returns the non-retryable 400, matching the existing role_arn-without-role path. fix(api): return a retryable status for transient marketplace AssumeRole failures #937's 503 does not apply here.
• CI on 1e7c394: CI Success and Run pre-commit hooks pass, on a fresh pull_request run against current main (after test(api): pin the submit dedupe as a predicate, not a 1000-row page (closes #266) #1030).
• Local verification (macOS, fake STS, no real AWS):
  • Ignoring the VerifyHostAccount result fails the member-account, host-identity-fail and list/cancel member_account_refused tests.
  • Restoring the old "return base" behaviour fails the host-row and refusal tests.
• CR skipped: draft and quota-limited; covered by the independent review plus local verification.
• Labels mirrored from fix(api): marketplace list/cancel use the host's ambient AWS credentials for any account #261.

Marking ready (owner-authorized) and merging at 1e7c394 with --match-head-commit.