feat(orgtoken): enforce per-org mint ceiling + optional expires_at #3017
Reference in New Issue
Block a user
Delete Branch "fix/orgtoken-mint-ceiling"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes KI-004 item 7.
Adds
ORG_TOKEN_MINT_CEILING(default 100). For anchored org API tokens,orgtoken.Issuenow counts live tokens for that org before inserting and returnsErrMintCeilingExceededwhen the ceiling is reached. The handler maps that to HTTP 429.Also adds optional
expires_atsupport:expires_at TIMESTAMPTZtoorg_api_tokens.orgtoken.IssueWithExpiryallows setting an expiry;Issuedelegates to it with a nil expiry (legacy unbounded behaviour).Validaterejects tokens whoseexpires_athas passed.Listreturnsexpires_atin token metadata.POST /org/tokensaccepts and echoes an optionalexpires_atfield.Changes
workspace-server/internal/orgtoken/tokens.go: addErrMintCeilingExceeded, env-drivenmintCeiling, count-before-insert guard,IssueWithExpiry, expiry validation, and list scanning.workspace-server/internal/handlers/org_tokens.go: map ceiling error to 429; accept/returnexpires_at.workspace-server/internal/orgtoken/tokens_test.go+org_tokens_test.go: update for the new column and add expiry tests.workspace-server/migrations/20260617130000_org_api_tokens_expires_at.*.sql: new column.Test plan
go test ./internal/orgtoken -count=1passes.go test ./internal/handlers -run TestOrgTokenHandler -count=1passes.go build ./...passes.SOP checklist
comprehensive-testing): unit tests added/updated for ceiling and expiry paths; handler tests passlocal-postgres-e2e): N/A — nullable expires_at migration; existing org_api_tokens path covered by unit testsstaging-smoke): N/A — no runtime deploy path touchedroot-cause): Closes KI-004 item 7 (unbounded mint + missing expiry surface)five-axis-review): reviewed (correctness/readability/architecture/security/performance)no-backwards-compat): additive; ceiling disableable via env, expiry optionalmemory-consulted): N/A — follows existing orgtoken patterns🤖 Generated with Claude Code
fix(orgtoken): enforce per-org org API token mint ceilingto feat(orgtoken): enforce per-org mint ceiling + optional expires_at5-axis review on head
899072f2: REQUEST_CHANGES. Correctness: the mint ceiling claims to cap live anchored org tokens, andValidatenow treats expired tokens as invalid, but the ceiling query counts everyrevoked_at IS NULLrow regardless ofexpires_at. Expired-but-unrevoked tokens therefore still consume the per-org ceiling forever and can block new mints even though they are no longer valid/live. Please change the count to exclude expired tokens, e.g.AND (expires_at IS NULL OR expires_at > now()), and add a test where an org at the raw row count but with expired tokens can still mint. Also consider rejecting pastexpires_aton create, because minting an immediately invalid token is poor UX. Security goal is good; expiry validation/listing otherwise looks sound.REQUEST_CHANGES after independent 5-axis review.
Correctness blocker: the new per-org mint ceiling counts
WHERE org_id = $1 AND revoked_at IS NULL, but this PR also introducesexpires_atandValidaterejects expired tokens. That means expired, unusable tokens still consume the “live-token” ceiling forever until manually revoked. An org with 100 expired non-revoked tokens would be unable to mint a replacement even though it has zero valid tokens.Please make the ceiling count only currently valid tokens, e.g.
revoked_at IS NULL AND (expires_at IS NULL OR expires_at > now()), and add a regression test where expired non-revoked rows do not block minting. Security/readability are otherwise reasonable; the expiry feature and listing shape are coherent once the ceiling matches the validity semantics.APPROVED after re-review at head
39987086a9.My prior blocker is resolved: the mint-ceiling query now counts only non-revoked tokens whose
expires_atis null or in the future, so expired tokens no longer consume the live-token ceiling. The added regression test covers the replacement-token case with expired rows ignored. Correctness/robustness/security/performance/readability look good for this fix.APPROVED. Fresh re-review on head
39987086.The prior mint-ceiling blocker is resolved: the ceiling query now counts only currently-valid anchored org tokens (
revoked_at IS NULLandexpires_at IS NULL OR expires_at > now()), so expired tokens that Validate would reject no longer consume capacity. The regression coverage includes ceiling-hit, under-ceiling with expired rows excluded, disabled-ceiling, expiry storage, expired-token validation rejection, and non-expired expiry acceptance.5-axis: correctness matches the requested behavior; robustness/error paths are materially improved by rejecting past expiries and mapping ceiling overflow; security is unchanged except for tighter token lifecycle control; performance is a single indexed count before mint; readability is clear. CI / all-required is green on this head.
New commits pushed, approval review dismissed automatically according to repository settings
New commits pushed, approval review dismissed automatically according to repository settings
afc96ca7e1to8afad90612APPROVED. Fresh re-review on rebased head
8afad906.Verified the rebase preserved the orgtoken mint-ceiling fix.
IssueWithExpiryrejects a non-zero pastexpires_atbefore insert, and the per-org ceiling now counts only currently valid anchored tokens:revoked_at IS NULLand(expires_at IS NULL OR expires_at > now()). Expired tokens no longer consume the mint ceiling, and the regression coverage includes ceiling exceeded, expired-token ignored, disabled ceiling, past-expiry rejection, and expiry validation/list paths.5-axis: correctness matches the requested fix; robustness is acceptable for optional expiry and disabled-ceiling behavior; security improves by bounding live org-token minting and rejecting expired tokens; performance is a single indexed-count style query per anchored mint; readability is clear in the orgtoken package. Note: current Gitea statuses still show CI/all-required and SOP/gate failures on this head, so this is a code-review approval, not a merge-gate-green statement.
APPROVED after re-review at head
8afad90612.The rebase preserved the prior mint-ceiling fix: IssueWithExpiry rejects past expires_at values before insert, the per-org ceiling COUNT includes only non-revoked tokens where expires_at is NULL or in the future, and Validate rejects expired tokens. Regression coverage is present for ceiling hit, disabled ceiling, expired-token exclusion from the ceiling, past-expiry rejection, expired-token validation rejection, and non-expired validation success.
5-axis review remains clean for the rebased code: correctness/security are improved by bounding live anchored org tokens and rejecting expired credentials; robustness is covered by targeted unit tests; performance is a single COUNT on mint; readability is acceptable and follows existing orgtoken patterns. Note: CI/SOP were not green when checked (all-required/gate/SOP pending or failing), so merge should still wait for branch protection and SOP acks.
/sop-ack comprehensive-testing
/sop-ack local-postgres-e2e
/sop-ack staging-smoke
/sop-ack root-cause
/sop-ack five-axis-review
/sop-ack no-backwards-compat
/sop-ack memory-consulted
merge-queue: could not update this branch with
main— the update returned a merge conflict (HTTP 409) that the queue cannot auto-resolve (POST /repos/molecule-ai/molecule-core/pulls/3017/update -> HTTP 409: {"message":"merge failed because of conflict","url":"https://git.moleculesai.app/api/swagger"}). Appliedmerge-queue-holdto unblock the queue (HOL guard). Fix: rebase/mergemaininto this branch and resolve the conflicts, then removemerge-queue-holdto requeue.- Adds AuditLogRequestContext{} to Issue/IssueWithExpiry/Validate calls that were missing it after the rebase onto main. - Adds audit-log expectations for anchored-token mint tests so sqlmock stays happy with the new LogMint path. Co-Authored-By: Claude <noreply@anthropic.com>8afad90612to00a8868ed6New commits pushed, approval review dismissed automatically according to repository settings
New commits pushed, approval review dismissed automatically according to repository settings
APPROVED. Re-review on rebased head
00a8868e.Verified the rebase conflict resolution in org_tokens_test.go: it preserves main's verified-session created_by format (
session:<user_id>) while carrying the branch's new nullableexpires_atinsert arg. The orgtoken functionality from the prior approved head is still intact: IssueWithExpiry rejects past expiries, Validate rejects expired tokens, List exposes expires_at, and the mint ceiling counts only currently valid anchored tokens (revoked_at IS NULLand(expires_at IS NULL OR expires_at > now())) so expired rows do not consume the ceiling.5-axis: correctness and regression coverage look sound; robustness/error paths cover ceiling, disabled ceiling, past expiry, expired validation, and optional expiry; security improves lifecycle control for privileged org tokens; performance is a single count before anchored mint; readability is acceptable. CI is still settling/failing in Gitea at review time, so this is a code approval, not a merge-green statement.
APPROVED after re-reviewing rebased head
00a8868e.Correctness: the org_tokens_test.go conflict resolution is correct. The verified-session mint expectation preserves main's provenance behavior (
created_by=session:<cp_session_user_id>) and also includes the branch's new nullableexpires_atargument (nil) in the insert expectation. The handler/test updates consistently carry optionalexpires_atthrough create/list/validate paths.Tests/robustness: focused orgtoken and handler tests cover expiry, expired-token rejection, list metadata, and mint ceiling behavior.
Security: the mint ceiling and expiry support reduce token risk; verified-session provenance is preserved by the rebase.
Scope/backcompat: additive nullable column and optional request/response field; legacy
Issuedelegates to nil expiry.Ops/readability: migration is reversible and the env-driven ceiling remains explicit.
SOP note: I did not repost /sop-ack comments because the current status already reports
sop-checklistacked 7/7.Verdict: APPROVED. CI/mergeable: mergeable=true; SOP 7/7; several contexts still re-running/pending at review time (Platform-Go/API Smoke/Local Provision), with qa/security/reserved gates red pending their reviewers.