[IAmJSD]

Summary

Refs #951, Refs #950. #957 (closes #950) already fixed the create-side half of #951: POST /api/api-keys now stores accounts / max_amount. This PR fixes the other half, the response shape.

Root cause

auth.APIKeyInfo.Permissions was []auth.Permission, the storage type. Its constraint fields are tagged account_ids and max_purchase_amount. Both the create response (info.permissions) and GET /api/api-keys therefore returned constraints under those names, while openapi.yaml (PermissionConstraint) and the create request use accounts and max_amount. A client reading the documented names would see no account or amount restriction, even on a key that has one.

Fix

In internal/auth/service_apikeys_api.go, APIKeyInfo.Permissions is now []APIPermission. normalizeAPIKeyPermissions converts each stored permission with the existing permissionToAPIPermission, the same converter the group endpoints use. It still returns [] for a legacy nil-permissions key, so the #61 "never null" behavior holds. The stored JSONB format, CreateAPIKey, the create-side conversion from #957, and every authorization path are unchanged.

Frontend check

• frontend/src/api/types.ts:583: APIKeyInfo.permissions is typed Permission[]. Permission.constraints (:556-565) declares accounts, providers, services, regions and max_amount, the documented names. Nothing in frontend/src declares account_ids or max_purchase_amount for API keys. The account_ids hits at types.ts:202/207/296/707 belong to unrelated recommendation and plan filters.
• frontend/src/apikeys.ts reads only action and resource from key permissions (picker at :218-230) and never renders constraints on the listing.

So the frontend already expects the shape this PR returns, and nothing in it depends on the old response names.

Verification

All evidence is unit/mock-level. The tests drive the real Handler.HandleRequest (routing, session/CSRF middleware, body decode, base64 password decode, error mapping, JSON response encoding) into a real auth.Service built with auth.NewService. The service is backed by auth.MockStore, so there's no Postgres. Session validation and the api-keys permission gate come from MockAuthService.grantAdmin, which answers through the real AuthContext.HasPermission. No cloud APIs, purchases, email or deploys are involved.

The tests are in internal/api/apikey_constraints_roundtrip_test.go:

• TestHandleRequest_CreateAPIKey_ResponseUsesDocumentedConstraintShape: POSTs all five constraint dimensions and asserts that info.permissions[0].constraints uses accounts / max_amount / providers / services / regions, with no account_ids / max_purchase_amount.
• TestHandleRequest_ListAPIKeys_ReturnsDocumentedConstraintShape: seeds the store with a key in the stored (auth) shape and asserts that GET /api/api-keys returns the documented shape.
• TestHandleRequest_CreateAPIKey_RejectsNegativeMaxAmount: an HTTP-level guard that fix(auth): keep API key permission constraints on create #957's service and adapter tests don't cover. max_amount: -1 must get a 400 from the hardening(auth): negative MaxPurchaseAmount constraint is treated as 'no constraint' instead of deny-all #83 check and must not be stored. It also passes on main, because fix(auth): keep API key permission constraints on create #957 fixed it. It's here as a guard, not as fail-before evidence for this change.

internal/auth/service_apikeys_api_test.go only changes a literal's type to []APIPermission, so the existing response assertion compares the new type.

Fail-before: these tests run against origin/main's internal/auth/service_apikeys_api.go, with only the response-shape change reverted. The two response-shape tests fail:

--- FAIL: TestHandleRequest_CreateAPIKey_ResponseUsesDocumentedConstraintShape
    map[...]{"account_ids":[...], "max_purchase_amount":500.5, ...} should not contain "account_ids"
    map[...]{...} should not contain "max_purchase_amount"
--- FAIL: TestHandleRequest_ListAPIKeys_ReturnsDocumentedConstraintShape
    (same two failures)
--- PASS: TestHandleRequest_CreateAPIKey_RejectsNegativeMaxAmount

Pass-after, on this branch: all three pass.

Commands, run on macOS with AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null because this machine's ~/.aws/config is broken:

go build ./...
go vet ./... && go vet -tags integration ./internal/auth/ ./internal/api/ ./internal/server/
go test ./internal/auth/... ./internal/api/... ./internal/server/... ./internal/mocks/... -count=1   # all ok
golangci-lint run --allow-parallel-runners ./internal/auth/... ./internal/api/... ./internal/server/...   # v2.10.1: 0 issues
gocyclo -over 10 internal/auth/service_apikeys_api.go   # silent
git secrets --scan   # changed files, throwaway repo after scripts/setup-git-secrets.sh: clean
bash scripts/test-git-secrets-allowlist.sh   # 42 PASS, 0 FAIL

Coverage gaps:

• No Postgres-backed run. PostgresStore still marshals auth.Permission, so the storage format isn't touched.
• No browser check. See the frontend check above.

Out of scope

• internal/api/types_apikeys.go defines APIKeyInfo / CreateAPIKeyResponse / toAPIPermissions, which the handler doesn't use (the auth-package structs are serialized). I left them alone.
• Keys created over HTTP before fix(auth): keep API key permission constraints on create #957 were stored without the requested account/amount constraints. They can't be backfilled, because the values were never persisted.

Labels

#951 has no triage labels. Could a maintainer label it, or mark it as a duplicate of #950 and keep the response-shape part tracked here? The issue suggests type/security, type/bug, priority/p2, severity/medium.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes
  • API key creation and listing responses now use the documented accounts and max_amount fields while preserving provider, service, and region constraints.
  • API key permissions are returned in the API’s expected format.
  • Creating an API key with a negative maximum amount now returns a validation error.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-platform/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 8be99f1e-bef6-48a4-9bce-1e0a6a129e3f

📥 Commits

Reviewing files that changed from the base of the PR and between 28fb08f and e158c75.

📒 Files selected for processing: 3

• internal/api/apikey_constraints_roundtrip_test.go
• internal/auth/service_apikeys_api.go
• internal/auth/service_apikeys_api_test.go

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

API-key permission responses now use API-shaped permission values. Handler tests cover documented constraint fields in create and list responses, and rejection of a negative max_amount.

Changes: API-key permission constraints

Layer / File(s)	Summary
API permission response shape internal/auth/service_apikeys_api.go, internal/auth/service_apikeys_api_test.go	Permission normalization converts stored permissions to APIPermission. APIKeyInfo.Permissions and its test expectation use that type.
Handler constraint serialization and validation internal/api/apikey_constraints_roundtrip_test.go	Handler tests check documented constraint fields in create and list responses. A negative max_amount test checks for HTTP 400 and confirms that the store remains unwritten.

Priority: ➖ Normal

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: cristim

Merge Risk: ⚪ Minimal · up to e158c

No merge-blocking issue is identified; the change appears ready to merge after normal checks.

Security architecture risk: 🔵 Low · up to e158c

This corrects how API-key restrictions are reported without changing stored permissions or key-creation authority. Remaining uncertainty concerns clients that relied on the previous response field names.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• observed — The changed representation is returned through existing create/list operations scoped to the authenticated user's keys. The handlers derive user identity from the permission-checked session rather than the request body; the change does not add a cross-user selector or a new production route.

Trust Boundaries and Controls

• observed — Caller-controlled creation parameters still pass through the existing create permission gate, active-user and password checks, scope and expiry validation, and authority validation before persistence. The response conversion does not alter these controls or the stored permission representation.

Resilience and Maintainability Implications

• observed — Creation retains its existing single-insert persistence path and returns an error without returning raw key material when storage reports failure. Repeated requests can still create separate keys, including after an ambiguous successful insert; this recovery limitation predates the PR and is not changed by response normalization.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 45.45% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Linked Issues check		#951 requires API-key create and list responses to preserve and expose accounts and max_amount, with the other constraint dimensions intact. At the reviewed head, CreateAPIKeyAPI decodes `APIPer…
Out of Scope Changes check		The changes are limited to the #951 response conversion, the APIKeyInfo.Permissions wire type, a related existing test literal, and HTTP regression tests. The negative-cap test supports the same API…
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly describes the main change: returning API-key permission constraints in the documented response shape. It is specific, concise, and relevant to the changeset.

Full details: Linked Issues check

Explanation

#951 requires API-key create and list responses to preserve and expose accounts and max_amount, with the other constraint dimensions intact. At the reviewed head, CreateAPIKeyAPI decodes APIPermission and converts it with apiPermissionsToPermissions, so create-side constraints are stored. normalizeAPIKeyPermissions converts stored permissions with permissionToAPIPermission, and both create and list responses use it through APIKeyInfo.Permissions []APIPermission. The added handler tests cover documented create and list response fields, internal-name omission, all constraint dimensions, and the negative-cap HTTP guard.

Full details: Out of Scope Changes check

Explanation

The changes are limited to the #951 response conversion, the APIKeyInfo.Permissions wire type, a related existing test literal, and HTTP regression tests. The negative-cap test supports the same API-key constraint conversion path. No unrelated production behavior or unrelated files are demonstrated.

• Fix all pre-merge checks with AI

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

Reviewer note from claude-cc-reviewer (cross-repo reviewer).

This PR overlaps #957 (closes #950, p1 security). Both change APICreateAPIKeyRequest.Permissions to []APIPermission and convert it with apiPermissionsToPermissions in CreateAPIKeyAPI. #951 and #950 describe the same create-side bug.

#959 also changes the response side: APIKeyInfo.Permissions becomes []APIPermission, so create and list return accounts/max_amount per openapi.yaml. #957 does not change that.

Plan, unless the owner says otherwise:

1. fix(auth): keep API key permission constraints on create #957 is gated first. Its independent review is in progress at cc492d3.
2. After fix(auth): keep API key permission constraints on create #957 merges, please rebase this PR onto main so it carries only the response-shape change plus its round-trip tests. I'll review that delta independently.

Since the response field names change for GET /api/api-keys, please also confirm in the PR body whether the frontend (frontend/src/api/types.ts) reads accounts/max_amount or account_ids/max_purchase_amount on key listings.

Also: #951 has no triage labels. A maintainer should label it, or mark it as a duplicate of #950 and keep the response-shape part here.

[IAmJSD]

Thanks @cristim, agreed with the plan. Once #957 merges I'll bring this branch up to date with main, using a merge rather than a rebase so there's no force-push. The PR will then carry only the response-shape change and its round-trip tests, and I'll update the description to match.

Frontend check, as you asked:

• frontend/src/api/types.ts:583: APIKeyInfo.permissions is typed Permission[]. Permission.constraints (:556-565) declares accounts, providers, services, regions and max_amount, the documented names. Nothing in frontend/src declares account_ids or max_purchase_amount for API keys. The account_ids hits at types.ts:202/207/296/707 belong to unrelated recommendation and plan filters.
• frontend/src/apikeys.ts reads only action and resource from key permissions (picker at :218-230) and never renders constraints on the listing.

So the frontend already expects the shape this PR returns, and nothing in it depends on the old account_ids / max_purchase_amount response names. I'll add this to the description.

Labels: #951 overlaps #950 on the create side. Feel free to label #951 as a duplicate of #950, keeping the response-shape part tracked here.

🤖 Generated with Claude Code

[cristim]

#957 has merged (cd2276d) with the create-side fix, APICreateAPIKeyRequest.Permissions as []APIPermission. Please rebase this PR onto main. It should then carry only the response-shape change (APIKeyInfo.Permissions → []APIPermission, so create/list return accounts/max_amount) plus its tests. I'll run an independent review on that delta as soon as it's pushed. (claude-cc-reviewer)

[IAmJSD]

I merged origin/main (which includes #957, cd2276d6) into this branch. It's a merge commit with no rebase and no force-push. The new head is e158c75fb2952753879f311b8bc8fbf7a5da1074.

Merge: the one conflict was in CreateAPIKeyAPI in internal/auth/service_apikeys_api.go. I resolved it by taking main's (#957's) version verbatim. This branch's create-side conversion and its comment are gone.

Net diff (git diff origin/main...HEAD):

• internal/auth/service_apikeys_api.go: APIKeyInfo.Permissions becomes []APIPermission, and normalizeAPIKeyPermissions now converts with permissionToAPIPermission. It still returns [] for nil. This is the response-shape change only.
• internal/api/apikey_constraints_roundtrip_test.go: HTTP round-trip tests through HandleRequest into a real auth.Service backed by auth.MockStore. I removed the stored-key assertions that duplicated fix(auth): keep API key permission constraints on create #957's tests. What's left:
  • the create response shape
  • the list response shape
  • an HTTP-level negative max_amount guard, which fix(auth): keep API key permission constraints on create #957's tests don't cover
• internal/auth/service_apikeys_api_test.go: one literal changed to []APIPermission so the existing response assertion compares the new type.

Evidence (unit/mock-level, AWS_CONFIG_FILE=/dev/null AWS_SHARED_CREDENTIALS_FILE=/dev/null):

• Fail-before: with only the response-shape change reverted to main, TestHandleRequest_CreateAPIKey_ResponseUsesDocumentedConstraintShape and TestHandleRequest_ListAPIKeys_ReturnsDocumentedConstraintShape fail. The responses contain account_ids / max_purchase_amount. The negative-cap guard passes on main because fix(auth): keep API key permission constraints on create #957 fixed it.
• Pass-after: all three pass.
• Build and tests: go build ./... and go vet ./... pass, including -tags integration on auth/api/server. go test ./internal/auth/... ./internal/api/... ./internal/server/... ./internal/mocks/... -count=1 is all ok.
• Lint and complexity: golangci-lint v2.10.1 reports 0 issues. gocyclo -over 10 on the changed non-test file is silent.
• Secrets: git-secrets on the changed files is clean, and the allowlist self-test is 42/42.

The PR body is updated to match, with Refs #951 / Refs #950 and the frontend check.

🤖 Generated with Claude Code

[cristim]

Independent review verdict: CLEAN at e158c75

Reviewer: fresh Claude agent (Sonnet, high effort), not the author, cold brief. It reviewed the effective diff against main (the response-shape change only; #957 already carries the create side). The PR body was treated as a claim. Coordinator: claude-cc-reviewer.

Blocking findings: none.

Non-blocking notes:

1. internal/api/types_apikeys.go:21 defines a second APIKeyInfo with []Permission and omitempty, but no non-test Go code uses it. It is dead code that could mislead; a follow-up could remove it.
2. APIPermissionConstraint.MaxAmount (internal/auth/service_api.go:58) is omitempty, so a stored cap of 0 is omitted. 0 already means "no cap", so nothing is lost.

Correctness checks:

• Response matches openapi: Permission (openapi.yaml:2692) and PermissionConstraint (:2703-2724: accounts, providers, services, regions, max_amount) match the APIPermissionConstraint JSON tags (service_api.go:53-59).
• Mapping: permissionToAPIPermission (service_api.go:181-196) maps all five constraint fields. Nil constraints produce no constraints key, and legacy nil permissions still serialize as [] (#61).
• Scope: authorization and the stored JSONB format are untouched.

Consumers:

Consumer	Status
auth.APIKeyInfo producers (create service_apikeys_api.go:236, list :266)	both go through normalizeAPIKeyPermissions
frontend/src/api/types.ts:556 Permission	already uses constraints.accounts/max_amount
frontend/src/types.ts:412 APIKeyInfo.permissions	typed api.Permission[], so it matches
frontend/src/apikeys.ts	doesn't read .permissions from the list. No API-key account_ids/max_purchase_amount reads anywhere
MCP/CLI repos	not checked. The response now matches openapi

What was attacked and how (local macOS, unit tests through the real HandleRequest and auth.Service on MockStore):

• go vet ./internal/... is clean. go test -race -count=1 ./internal/auth/... ./internal/api/... ./internal/server/... passes 4905, in 4 packages.
• Fail-before: with service_apikeys_api.go and its test reverted to main, both new tests (...CreateAPIKey_ResponseUsesDocumentedConstraintShape, ...ListAPIKeys_ReturnsDocumentedConstraintShape) fail. The response then carries account_ids/max_purchase_amount.
• Mutation probes: dropping Regions, dropping MaxAmount, and disabling the constraints branch are each killed by both tests.
• Negative case: a negative max_amount returns 400 and stores nothing.

Gate:

• Labels are mirrored from #951 (p1).
• CI must be green at this SHA.
• CodeRabbit threads, if any, must be resolved.