[IAmJSD]

Closes #1430

Root cause

requireAccountAccess (internal/api/scoping.go) passed the account id straight to GetCloudAccount. cloud_accounts.id is a UUID column, so a non-UUID value made Postgres fail with invalid input syntax for type uuid. The store returned that as a plain error, and handleRequestError turned it into a 500 logged at error level. The account handlers run validateUUID on their path ids first. The ladder routes don't, because they take the id from the request body (PUT /api/ladder/configs, cloud_account_id) or the query string (GET /api/ladder/runs and /api/ladder/tranches, account_id).

Fix

requireAccountAccess now refuses a non-UUID id with a 400 ClientError ("invalid account id: must be a valid UUID") before it touches the store. The same uuidRegex backs validateUUID. This covers the ladder config write, both timeline reads, and any future caller. Valid UUIDs keep their current answers (found, missing or out of scope). The other callers already pass validated path ids or stored tranche account ids, so they don't change.

The ladder handler tests that used "acct-1" / "ghost" as account ids now use UUID constants. Each test still asserts what it asserted before.

Verification

All local runs, Linux, Go 1.26.9 toolchain (go1.26.10 auto-selected):

• New tests: TestUpsertLadderConfig_NonUUIDAccountIDIs400 (PUT /api/ladder/configs) and TestLadderTimelineNonUUIDAccountIDIs400 (subtests for /api/ladder/tranches and /api/ladder/runs, driven through NewRouter(h).Route). The mock store's GetCloudAccount answers a non-UUID id the way Postgres does, with a plain pq: invalid input syntax for type uuid error. Each test asserts a 400 ClientError, that GetCloudAccount was never called with the bad id, and that nothing reached UpsertLadderConfig or the timeline store.
• Fail-before: with internal/api/scoping.go restored from origin/main, go test -short -run NonUUID ./internal/api/ fails all three cases with got *fmt.wrapError: accounts: pq: invalid input syntax for type uuid: "not-a-uuid", which is the 500 path.
• Pass-after: the same command passes.
• go build ./..., go vet ./..., go vet -tags integration ./..., (cd tests/e2e && go vet -tags e2e ./...): clean.
• golangci-lint v2.10.1 on ./internal/api/, with and without --build-tags integration: 0 issues. gocyclo -over 10 internal/api/scoping.go: nothing reported.
• go test -race -short -count=1 ./internal/api/: ok.
• go test -race -tags integration -count=1 ./internal/api/: ok (656s; the full package, not only the CI-selected integration tests)
• secrets-scan.sh on the changed files: "clean; planted key flagged". scripts/test-git-secrets-allowlist.sh: 183 PASS, 0 FAIL.

Not verified locally: a real Postgres answering a non-UUID id. The mock reproduces its error shape, and the new guard returns before any store call, so the database is never reached.

🤖 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]

Opened at head b96468e: requireAccountAccess refuses a non-UUID account id with a 400 before the store lookup; fail-before/pass-after and local check results are in the PR body.

[IAmJSD]

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

Blocking:

• none

Non-blocking:

• internal/api/scoping.go:161: requireAccountsAccess (the plan-accounts batch check) now also gives a scoped session a 400 for a non-UUID id. An unrestricted session returns before the loop, so the same bad id still goes on to the store, and possibly a 500, further down. That's harmless and outside fix(api): non-UUID account IDs answer 500 instead of 400 (ladder config write, ladder runs/tranches) #1430. It's worth knowing if someone later files "plan accounts give a 500 on a bad id".
• uuidRegex accepts only the canonical hyphenated form, in either case. Postgres also accepts {...}-braced and unhyphenated UUIDs, so those used to resolve and now get a 400. No client sends them as far as I can see (the frontend and stored ids are canonical), so this doesn't block.

Attacked:

• Callers: I checked all 13 requireAccountAccess callers. The account handlers pass ids that validateUUID already checked. handler_ladder_amend.go:68 and handler_ladder_resolve.go:107 pass LadderTrancheScope's c.cloud_account_id, which is joined to cloud_accounts.id (uuid), so it is always canonical. The regex check is syntax-only and runs before the scope check, so it reveals nothing about which accounts exist. Found, missing and out-of-scope UUIDs keep their behaviour.
• Fail-before: with internal/api/scoping.go reverted to origin/main, TestUpsertLadderConfig_NonUUIDAccountIDIs400 and both TestLadderTimelineNonUUIDAccountIDIs400 subtests fail with the wrapped pq error (the 500 path). Restored, they pass. The mock's GetCloudAccount records calls before the Fn override (internal/mocks/stores.go:1427), so the AssertNotCalled checks mean something.
• go build ./...; go vet on ./internal/api/ (plain and -tags integration); go test -race -short -count=1 ./internal/api/ ./internal/server/ ./cmd/lambda/ (the packages that import internal/api): ok. go test -race -tags integration -count=1 ./internal/api/ -run 'Ladder|Account|Scope|Plan': ok. The first run failed only because a shared testcontainer was reaped ("No such container"), and the rerun passed. golangci-lint, with and without integration tags: 0 issues. gocyclo -over 10 on scoping.go: nothing.

Body-stale:

• none

_{Reviewer: rC3. 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 418f221526174a0c0e02e68388f30c46fdc06bab merged onto main (d87b96ff), together with #1437, #1438, #1439, #1440, #1441 in that order (combined tree 37f4d0bb, 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 1 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: 1 approving independent review(s) on 418f2215, green CI, and the combined-tree verification above. Squash-merged at exactly 418f221526174a0c0e02e68388f30c46fdc06bab.