[IAmJSD]

Summary

Two gaps from the #552 review in internal/credentials/host_identity.go:

1. No cancellation test. VerifyHostAccount wraps the caller's ctx in context.WithTimeout. Every test passed context.Background() and none canceled the parent, so the mutant context.WithTimeout(context.Background(), ...) survived. With that mutant, a request the client cancels would keep the STS call running for up to hostIdentityTimeout (5s). The code on main is correct; only the test was missing.
2. Wrong tenant advice. VerifyHostAccount returned ErrNotHostAccount ("...; set aws_role_arn") for every failure: a missing STS client, an STS error or timeout, and an empty STS answer. That text is stored in tenant-visible rows, so tenants were told to change config when the platform was at fault.

Fix

• New sentinel credentials.ErrHostIdentityUnavailable. It is returned for no STS client, an STS error or timeout (a canceled caller included) and an empty STS answer. Its text is credentials: the CUDly host account could not be verified because of a platform-side error; no change to this account's configuration is needed, retry later. It carries no host account id and no STS text.
• ErrNotHostAccount is now returned only for a genuine account mismatch, and its text is unchanged.
• Fail-closed behavior is unchanged. ErrHostIdentityUnavailable has an Is method that also matches ErrNotHostAccount. Every existing errors.Is(err, credentials.ErrNotHostAccount) check (resolver, scheduler, purchase, server commitment probe, marketplace) still refuses the account exactly as before. Only the text differs. The existing resolver, scheduler, purchase and server tests that assert ErrorIs(..., ErrNotHostAccount) for the STS-failure cases pass unchanged.
• The API layer has its own fixed strings for this check. internal/api/handler_accounts_awsprobe.go gets the same split:
  • requireHostForEmptyRoleARN (account create, update and registration approval): a host AWS config load failure or ErrHostIdentityUnavailable now returns hostIdentityUnavailableMessage. A mismatch keeps roleARNNotHostMessage.
  • classifyAWSResolveError (account test endpoint): it checks ErrHostIdentityUnavailable first, because that error also matches ErrNotHostAccount.
  • The marketplace resolver path returns err.Error() in its 400, so it picks up the new text automatically.
• HTTP status codes are unchanged (400 everywhere).

Closes #559

Verification

Unit tests with fakes and stubs only (in-process CallerIdentityClient fakes and stub aws.HTTPClient STS transports). No real AWS calls. Env for every run: AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null AWS_EC2_METADATA_DISABLED=true.

New and changed tests:

• internal/credentials/host_identity_test.go
  • TestVerifyHostAccount_PropagatesCallerCancellation (new): raises hostIdentityTimeout to 1h, starts the call, cancels the parent ctx, and asserts the fake STS sees context.Canceled within 5s and that VerifyHostAccount returns.
  • TestVerifyHostAccount_OnlyMismatchSuggestsRoleARN (new).
  • TestVerifyHostAccount_ErrorDoesNotLeakHostIdentity and TestVerifyHostAccount_BoundsSTSCall: now expect the right sentinel per case.
• internal/api/handler_accounts_hostcheck_test.go: the create, update and registration cases now expect a message per case, plus TestHostCheckMessages_OnlyMismatchSuggestsSettingRoleARN.
• internal/api/handler_accounts_awsprobe_test.go: new case "role_arn without role ARN when the host identity check fails".
• internal/api/handler_marketplace_registered_account_test.go: new TestLoadMarketplaceAWSConfig_RegisteredRoleWithoutARNHostCheckFailure.

Fail-before / pass-after (unit evidence):

• Mutation from the issue. I replaced context.WithTimeout(ctx, ...) with context.WithTimeout(context.Background(), ...). Result: TestVerifyHostAccount_PropagatesCallerCancellation FAILS ("the STS call did not see the caller's cancellation", 5.00s). It passes on main and on this branch.

• Text, credentials package. I ran main's host_identity.go with a compile-only shim var ErrHostIdentityUnavailable = ErrNotHostAccount. Result: TestVerifyHostAccount_OnlyMismatchSuggestsRoleARN FAILS. The unavailable cases contain "aws_role_arn" and equal the mismatch text.

• Text, API. I ran main's host_identity.go and handler_accounts_awsprobe.go with compile-only shims that define only the new identifiers. These FAIL:

  • Create, Update and ApproveRegistration _EmptyRoleARNRequiresHostAccount, subtests "STS unreachable", "STS rejects the host credentials" and "host AWS config unavailable".
  • TestTestAccountCredentials_AWSFailuresAreClassified/role_arn_without_role_ARN_when_the_host_identity_check_fails.
  • TestLoadMarketplaceAWSConfig_RegisteredRoleWithoutARNHostCheckFailure.

  The "non-host account" subtests still pass.

• Fail-closed probe. I changed the Is method to return false. Result: the credentials tests FAIL, including TestResolveAWSCredentialProvider_SelfAccount_NonHostFailsClosed on main's unchanged resolver_test.go. So the errors.Is compatibility is pinned.

• All source changes were restored after each probe.

Commands run on this branch:

go build ./...
go vet ./...
go vet -tags integration ./...
go test ./internal/credentials/ -count=1                  # ok
go test ./internal/api/ -count=1                          # ok
go test ./internal/scheduler/ -count=1                    # ok
go test ./internal/purchase/ -count=1                     # ok
go test ./internal/server/ -count=1                       # ok
go test ./internal/mocks/ -count=1                        # ok
go test -race ./internal/credentials/ -count=50 -run TestVerifyHostAccount   # ok (flake check)
golangci-lint run --allow-parallel-runners ./internal/credentials/ ./internal/api/   # 0 issues
gocyclo -over 10 -ignore "_test\.go" internal/credentials/host_identity.go internal/api/handler_accounts_awsprobe.go   # clean

No integration tests were added or needed: the change has no DB or concurrency surface.

Combined-tree verification. This branch was tested together with the other queued branches on a local, never-pushed tree: queue/platform @ 936996ad = origin/main a14dc8f2 plus these heads, merged in this order:

Branch	Tested head
fix/issue-337	ff3420c
fix/issue-169	7d75eb5
fix/issue-342	66b221f
fix/issue-310	5e11c21
fix/issue-326	5f0ea59
fix/issue-243	365e959
fix/issue-343	6c94036
fix/issue-312	d2848a8
fix/issue-316	acd60dd
fix/issue-339	75ea9b9
fix/issue-339-commitment-type	73bdb19
fix/issue-353	5d25fcd
fix/issue-313	3156bc4
fix/issue-357	53d9e58
fix/issue-368	0ba727d
fix/issue-419	61648c9
fix/issue-111	b04c372
fix/issue-686	0ad90b7
fix/issue-674	793095d
fix/issue-1055	4ec96f1
fix/issue-559	dfa9525
fix/issue-558	034eed4
fix/issue-874	1720539
fix/issue-666	1e120da
fix/issue-668	fb26345
fix/issue-568	ce04173
fix/issue-311	e54f250
fix/issue-177	0cbb88a
fix/issue-151	c21377c
fix/issue-166	41fb77b
fix/issue-27	2d67622
fix/issue-323	69288ae
fix/issue-59	36463e8
fix/issue-43	35e4069
fix/issue-158	6f76f8a
fix/issue-104	bb00491
fix/issue-55	971ef1b
fix/issue-361	89ac220
fix/issue-120	b458719
fix/issue-36	42bac6e
fix/issue-525	57c97dd
fix/issue-712	e5bcda0
fix/issue-72	6cc63ed

fix/issue-72 and fix/issue-312 conflict on one line in internal/api/handler_dashboard.go (both change the calculateCommitmentMetrics call). The tree used #72's side with #312's validated provider variable; whichever PR merges second takes that same resolution.

Linux, Go 1.26.9, AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null AWS_EC2_METADATA_DISABLED=true; Docker Postgres via testcontainers:

• Passed at 936996ad: go build ./..., go vet ./..., go vet -tags integration ./... and the full go test -race -short -count=1 ./....
• Static checks at 936996ad: golangci-lint v2.10.1 found 0 issues on every changed package, with and without --build-tags integration. tests/e2e is a separate module, so it was vetted under its own e2e tag. gocyclo -over 10 is silent on the changed non-test files.
• Integration tests, every changed package, one package at a time (-tags integration -count=1, 60-minute local timeout because the VM was shared):
  • At 936996ad, these pass: internal/server, internal/credentials, internal/scheduler, internal/database/postgres/migrations, internal/purchase and internal/api.
  • internal/config passes except TestPostgresStoreDB_ResolveStaleRIExchange_ValidatesCloudCheck, which fails the same way on unmodified main (fixed by fix(config): compare resolve checked_at at the database's microsecond precision #1252).
  • These were run on the first build of this tree (26795376, same branches at earlier heads): internal/analytics, internal/auth, internal/commitmentopts, internal/database and cmd/lambda/clear-rate-limit all pass. The later fixes don't touch those packages.
• What the combined run caught, each fixed on its own branch before this PR opened:
  • a compile break between fix/issue-104's test and fix/issue-326's httpToLambdaRequest signature change;
  • fix/issue-316 rejecting legacy-keyed plans;
  • fix/issue-120 versus main's global-filter test.
• Secrets: git-secrets, using the repo's scripts/setup-git-secrets.sh patterns in a fresh repo with an isolated HOME, found nothing in the changed files, and a planted fake key was flagged. scripts/test-git-secrets-allowlist.sh: 137 PASS, 0 FAIL.
• Nothing was applied to any cloud.

Out of scope (deliberately left)

• Status codes. An STS outage during the empty-role-ARN check still returns 400 on account save and marketplace list/cancel. 503 would describe a platform-side failure more accurately, but changing it alters API behavior beyond this issue.
• Marketplace no-cloud-account path. loadMarketplaceHostAWSConfig uses one fixed marketplaceNoCloudAccountMessage for both a non-host id and an STS failure. That message does not tell the tenant to set aws_role_arn, so I left it alone.
• Scheduler. It returns the error to its caller unchanged. Whatever records it now gets the non-actionable text for STS failures.

Frontend follow-up for the UI owner

None required. The UI shows the server's message text, and git grep "set aws_role_arn" -- frontend/ finds no UI code that matches on the old string.

Maintainer: please mirror the issue's triage labels onto this PR: impact/internal, type/chore, triaged, priority/p3, severity/low, urgency/eventually, effort/xs.

🤖 Generated with Claude Code

[coderabbitai]

Caution

Review failed

The pull request is closed.

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.}

[IAmJSD]

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE f6caeaa2a9d189749497bb98b71363b629969319

Blocking:

• none

Non-blocking:

• internal/api/handler_accounts_awsprobe.go:65 and the ErrHostIdentityUnavailable text in internal/credentials/host_identity.go:36: "no change to aws_role_arn is needed" / "no change to this account's configuration is needed" is a claim the platform can't make when the check didn't run. If the account really isn't the host, the tenant is told nothing is wrong, and then gets the mismatch message once STS recovers. "could not be verified because of a platform-side error; retry later" would be accurate either way. This is a wording choice and the issue's acceptance criteria are met as written.

Attacked:

• Fail-closed on an auth path: listed every VerifyHostAccount and ErrNotHostAccount consumer (credentials/resolver.go:203, scheduler/scheduler.go:833, api/handler_marketplace.go:96,149, api/handler_accounts_awsprobe.go:91,103). Each one refuses on any non-nil error, so the new sentinel can't open a path. Is only widens matching toward ErrNotHostAccount, and classifyAWSResolveError checks the new sentinel first. Neither message carries the host account ID or STS detail.
• Fail-before, three mutations, each restored: (1) the issue's mutant context.WithTimeout(context.Background(), ...) makes TestVerifyHostAccount_PropagatesCallerCancellation fail ("did not see the caller's cancellation"). (2) Returning ErrNotHostAccount for the unavailable cases fails 4 credentials tests plus the Create, Update and ApproveRegistration host-check tests, the awsprobe classification test and the marketplace test. (3) Is returning false fails TestResolveAWSCredentialProvider_SelfAccount_NonHostFailsClosed and TestScheduler_CollectAWSForAccount_RoleARNWithoutARNOnNonHostFailsClosed, so the errors.Is compatibility is pinned.
• Ran go build ./..., go vet (with and without -tags integration) on internal/credentials + internal/api, go test -race -short -count=1 on credentials, api, scheduler, purchase and server (all ok), and golangci-lint v2.10.1 with and without --build-tags integration (0 issues). The package-level hostIdentityTimeout override is safe because no test in the package uses t.Parallel.

Body-stale:

• none

_{Reviewer: rB16. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths.}

[IAmJSD]

Independent review (IAmJSD's agent, fresh context, given only the diff, the issue and the PR text): VERDICT: APPROVE f6caeaa2a9d189749497bb98b71363b629969319

Blocking:

• none

Non-blocking:

• internal/credentials/host_identity.go:41: the one-way Is (Unavailable matches NotHost, not the other way) is what keeps every caller failing closed, and the existing TestResolveAWSCredentialProvider_SelfAccount_NonHostFailsClosed pins it (see below). One thing to keep in mind: any future code that compares with err == credentials.ErrNotHostAccount instead of errors.Is would treat an STS outage as "not this error" and could fail open depending on the branch. Nothing does that today (grepped), but a one-line comment on ErrNotHostAccount saying "compare with errors.Is only" would make it harder to trip over.
• internal/api/handler_marketplace.go:96 now returns 400 "retry later" for an STS outage. As the body says, 503 would fit better. That's fine to leave out of scope.
• From an earlier review (rB16), which I agree with: the outage texts (handler_accounts_awsprobe.go:65, host_identity.go:36) say "no change ... is needed", but when the check couldn't run the platform doesn't know that. If the account really isn't the host, the tenant gets told no change is needed and only learns otherwise on retry. Something like "could not be checked right now; retry later" without the no-change claim would be accurate. Wording only, so not blocking.

Attacked:

• Checked against test(auth): host identity credential tests miss caller-cancellation propagation; tenant error text says set aws_role_arn for STS outages #559's acceptance: a test cancels the parent ctx and asserts the STS fake sees context.Canceled with the timeout raised to 1h, so only the caller's cancel can end it. Only a genuine mismatch mentions aws_role_arn, and outages get a separate, non-actionable text that contains neither the host id nor the STS text. Grepped every non-test use of ErrNotHostAccount (resolver, marketplace, awsprobe): all use errors.Is, so outages still refuse. classifyAWSResolveError checks Unavailable first, which is required because Unavailable also matches NotHost. No integration test or frontend code matches the old text.
• Mutations (each restored): the issue's mutant context.WithTimeout(context.Background(), ...) fails TestVerifyHostAccount_PropagatesCallerCancellation (5.00s). Making Is return false fails TestResolveAWSCredentialProvider_SelfAccount_NonHostFailsClosed (unchanged test from main) plus the new credentials and marketplace tests, so the fail-closed compatibility is pinned.
• Ran: go build ./..., go vet with and without -tags integration on internal/api and internal/credentials, go test -race -short -count=1 on api, credentials, scheduler, purchase and server (all ok), golangci-lint v2.10.1 with and without integration tags (0 issues). There's no integration surface: no DB change, and no integration test references the changed text.

Body-stale:

• none

_{Reviewer: rB17. Bar while the maintainers are away (set by IAmJSD as acting owner): one approving independent review on the head, two for money, auth and security paths.}

[IAmJSD]

Local verification at f6caeaa2a9d189749497bb98b71363b629969319 merged onto main (de56f95b), together with #1114, #1142, #1254, #1274, #1278, #1279, #1280, #1282, #1414 in that order (combined tree e1bf26a0, Linux, Go 1.26.10): go build ./..., go vet with and without -tags integration, the full go test -race -short -count=1 ./..., and go test -race -tags integration -count=1 on every changed package, against Docker Postgres. All pass. CI Success and pre-commit are green on this head, with 2 approving independent review(s) on it. Nothing was run against any cloud.

[IAmJSD]

Merged by IAmJSD's agent at IAmJSD's request (IAmJSD is acting owner while the maintainers are away), under the bar IAmJSD set: 2 approving independent review(s) on f6caeaa2 (two required: money, auth or security path), green CI, and the combined-tree verification above. Squash-merged at exactly f6caeaa2a9d189749497bb98b71363b629969319.