Skip to content

fix(api): marketplace rows with a NULL cloud_account_id use the host's ambient credentials #560

Closed

Description

@cristim

Found in the review of #554 (merged as e522f3d). Same defect class as #261 (marketplace list/cancel signing with the host's ambient AWS credentials), remaining for legacy rows. Related: #547, #553.

Location (origin/main)

internal/api/handler_marketplace.go:71-73, loadMarketplaceAWSConfig: when row.CloudAccountID is nil or blank it returns the host's own credentials (base) and ignores the row's external AccountID.

Failure scenario

purchase_history.cloud_account_id was added in migration 000011 with no backfill and is ON DELETE SET NULL, so legacy rows and rows whose cloud account was deleted have a NULL account. A sell-any caller with unrestricted scope (scoped or sell-own callers get 403 from authorizeSessionSell, handler_marketplace.go:433) lists or cancels such a row. The EC2 call is signed as the host account. If the RI belongs to another AWS account, AWS returns InvalidReservedInstancesId.NotFound, which mapAWSMarketplaceError turns into a misleading 400. Not a cross-tenant bypass (host credentials only reach the host account), but the feature silently does not work for these rows.

Suggested fix (owner decision)

Resolve the account by (aws, row AccountID), as merged #547 and #553 do. If it resolves to a non-host account, use that account's credentials. If it does not resolve and the id differs from the host identity, refuse with a 400.

Acceptance

A marketplace list/cancel on a row with a NULL cloud_account_id never signs with host credentials unless the row's AccountID is the host account; covered by a test for each branch.

Activity

  1. IAmJSD commented on Oct 16, 2026

    @IAmJSD
    Collaborator

    Released for now: builds on #1031; will pick up after it merges (details below)

  2. self-assigned this
    on Oct 16, 2026
  3. IAmJSD commented on Oct 16, 2026

    @IAmJSD
    Collaborator

    Released for now. #1031 (for #261) already covers this issue's fail-closed half: a NULL-cloud_account_id row is signed with host credentials only when its account_id passes credentials.VerifyHostAccount, and every other such row gets a 400. That's tested for each branch. What's left here is the suggested lookup by (aws, account_id), as #547 and #553 do, so that a registered non-host account uses its own credentials. That change goes on top of #1031's code, so I'll pick it up once #1031 merges rather than stacking PRs.

  4. removed their assignment
    on Oct 16, 2026
  5. IAmJSD commented on Oct 16, 2026

    @IAmJSD
    Collaborator

    Claiming: working on this now that #1031 has merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions