feat/one-api

Deleted minting key bypasses liveness guard — orphaned child escapes cascade

domains/core/src/api-key/api-key.shells.ts:133

In createApiKeyShell, if command.createdByApiKeyId is set but findById returns null (key hard-deleted between resolveCaller and the transaction), granterKey is null and decideCreate's isLiveKey guard is skipped. The inserted key carries a createdByApiKeyId pointing to a non-existent row, making it invisible to revokeDescendants' recursive CTE — a permanently orphaned child. Attack window is narrow (requires double race), but the fix is one line: if command.createdByApiKeyId != null && granterKey == null → return err(ApiKeyErrors.granterNotLive()).

Rotate shell does not re-read granter key liveness under the org advisory lock

domains/core/src/api-key/api-key.shells.ts:202

createApiKeyShell re-reads the granting key under acquireOrgCreateLock. rotateApiKeyShell takes no org advisory lock and performs no in-transaction re-read. OCC on the rotated key's row provides some protection, but the defense-in-depth gap vs createApiKeyShell is a consistency risk worth a follow-on issue.

scopesSatisfied([], []) is vacuously true — undocumented edge case

packages/shared-kernel/src/scopes.ts:52

scopesSatisfied uses required.every(), returning true when required is empty. Not a live vulnerability (decideCreate guards against empty-scope keys), but the vacuous-true behavior should be documented or asserted in case the empty-scopes guard is relaxed.

mapRotateApiKeyError missing ApiKeyScopeEscalation — silent undefined return

apps/platform/src/api/mappers/api-key.mapper.ts:82

RotateApiKeyError now includes ApiKeyScopeEscalationError, but mapRotateApiKeyError has no case for it. The switch has no exhaustiveness guard, so the tag falls through and returns undefined — a scope-escalation error on rotate will produce an undefined response body rather than a 403. The public mapper handles it correctly; only the dashboard mapper is missing this case.

decisions.ts imports type from queries.ts — dependency-direction violation

domains/core/src/api-key/api-key.decisions.ts:30

import type { InsertApiKeyValues } from ./api-key.queries pulls a type from an infrastructure file (queries.ts imports drizzle-orm). canonical-form.md states decisions import nothing from infrastructure. InsertApiKeyValues should be co-located in decisions.ts or type.ts to keep the dependency direction clean.

Liveness predicate duplicated between handler and isLiveKey — drift risk

apps/platform/src/api/handlers/public-v1/api-keys.handler.ts:116

resolveCaller() re-implements the isLiveKey predicate inline (status==='active' || rotating+rotationExpiresAt>now). isLiveKey is exported precisely to be the single authoritative predicate so 'issuance and exchange cannot drift apart' (its JSDoc). The handler should call isLiveKey(caller, new Date()).

revokeApiKeyShell .catch() swallows all errors without rethrowing unknown

domains/core/src/api-key/api-key.shells.ts:402

The canonical shell pattern scopes .catch() to specific errors and rethrows unknowns. The revoke .catch() maps every rejection to err(databaseError), potentially silencing non-transient failures. The comment explains the intent, but it deviates from the documented pattern.

deleteColumnConfigShell and setOrgDefaultShell lack transaction boundary .catch()

domains/core/src/column-configuration/column-configuration.shells.ts:315

Pre-existing: after the sweep fixing create/update/duplicate, these two shells still return raw transaction promises without a .catch(). If an outbox insert fails they throw rather than return Result<T,E>. Follow-on cleanup.

No test for active machine key listing — coverage gap in list authority suite

apps/platform/src/api/handlers/public-v1/__tests__/api-keys.authority.test.ts:112

The list describe block tests member→403, admin→200, revoked-machine→403. Missing: active machine key with api-keys:read scope → 200. Without a positive machine-key-list test, a future refactor that accidentally applies the admin guard to machine callers would pass the suite but break a legitimate caller.

No test for rotating+null-expiry minting key in decideCreate

domains/core/src/api-key/__tests__/api-key.decisions.test.ts:161

The it.each covers ['revoked', null] and ['rotating', expired-date] but not { status: 'rotating', rotationExpiresAt: null }. isLiveKey checks `rotationExpiresAt !== null` before comparing, so null-expiry rotating key is dead — but untested. A future refactor could interpret null as 'no expiry = forever live'.

Revoke 'forwards caller grant' test verifies forwarding only, not the enforcement 403

apps/platform/src/api/handlers/public-v1/__tests__/api-keys.authority.test.ts:251

The test checks command.granterScopes is forwarded but does NOT verify the handler returns 403 when caller scopes are insufficient against the target. Need a test: caller=['api-keys:write','bills:read'], target=['*'] → handler returns 403. What pins the wiring, not just the forwarding.

No test for granterKey=null when createdByApiKeyId is non-null (hard-deleted parent)

domains/core/src/api-key/__tests__/api-key.decisions.test.ts:162

When createdByApiKeyId is supplied but the parent was hard-deleted, granterKey is null and decideCreate proceeds unchecked. Should be an explicit test clarifying the design decision — or the shell should return err(granterNotLive()) in this case.

updateColumnConfigShell non-unique-violation never-throw not covered

domains/core/src/column-configuration/__tests__/column-configuration.shells.integration.test.ts:627

The non-unique-violation databaseError test only exercises createColumnConfigShell. updateColumnConfigShell and duplicateColumnConfigShell have the same fix but no parallel test for non-23505 infrastructure errors.

Duplicate comment block in revokeApiKeyShell .catch()

domains/core/src/api-key/api-key.shells.ts

The catch handler has two overlapping comment blocks — the old 4-line block was not removed when the new fuller explanation was added. Delete the old block.

CI NEXT_PUBLIC_* extraction spawns two extra supabase status invocations

.github/workflows/pr-checks.yml

SUPABASE_URL and SUPABASE_ANON_KEY are already in $GITHUB_ENV after the first invocation. NEXT_PUBLIC_* can simply be `echo "NEXT_PUBLIC_SUPABASE_URL=$SUPABASE_URL" >> "$GITHUB_ENV"` — no second subprocess needed. The current pattern uses two additional supabase status calls with throwaway single-char overrides.

isLiveKey not reused in findActiveByPrefix — drift surface for grace-window rule

domains/core/src/api-key/api-key.queries.ts

isLiveKey's JSDoc says it exists so issuance and exchange 'cannot drift apart', but findActiveByPrefix implements the same check as a SQL condition independently. Consider using isLiveKey as a post-query filter or adding a cross-reference comment in queries.ts.

ApiKeyGranterNotLive mapped to generic 'forbidden' in public mapper — not actionable for clients

apps/platform/src/api/mappers/public-v1/api-keys.mapper.ts

ApiKeyGranterNotLive maps to error:'forbidden' — a generic code that gives the client no way to distinguish 'your minting key was revoked between check and commit' from a generic permission denial. A dedicated code like 'granter_revoked' would be more actionable for API consumers debugging rotation races.