feat(api-token): personal access tokens with an opt-in public api surface - #59
Merged
Conversation
PAT org-scopés, HMAC-SHA256 + pepper serveur, format clean_<base58url-32> + checksum CRC32, surface publique opt-in via une sous-app /api/v1 dédiée. Trois corrections : - le per-row salt détruisait le lookup O(1) sans rien apporter contre 256 bits d'entropie ; remplacé par HMAC + pepper (frontière DB/secret, argument SOC2 pour D.4) - read:uploads abandonné : le module uploads n'a ni route de listing ni table de métadonnées, et le storage est un profil docker opt-in - @better-auth/api-key évalué puis écarté : aucun hook de cycle de vie, donc pas d'event dans la même TX (règle §6) Ajoute à C.5 la curation du catalogue public : les 57 events subscribables sont aujourd'hui exposés en permanence, alors qu'une publication est un contrat qu'on ne peut plus renommer. Claude-Session: https://claude.ai/code/session_01VTzaSyvpBCoWAocwbiywpk
- IApiTokenRepository port with ApiTokenRecord, ApiTokenError, TokenOwner - DrizzleApiTokenRepository: §8-instrumented, all 8 methods - touchLastUsed bucket-check lives in WHERE clause (no prior read) - findByIdForOwner returns Option.none for wrong owner (no 403 leak) - Integration tests against real DB: 4/4 pass Claude-Session: https://claude.ai/code/session_01VTzaSyvpBCoWAocwbiywpk
…d no-write check Claude-Session: https://claude.ai/code/session_01VTzaSyvpBCoWAocwbiywpk
- requireApiToken: bearer-only, parseToken gate before any DB call, transparent pepper rotation with rehash, revoke/expiry/scope checks, touchLastUsed + API_TOKEN_USED event gated on actual bucket write - API_TOKEN_POLICY (per-token, 600 req/min) and API_TOKEN_IP_POLICY (per-IP, 1200 req/min) as two sequential policies — never composite to prevent IP-limit bypass by token rotation - touchLastUsed: Result<void> → Result<boolean> (returning) so the middleware can gate event emission on actual DB writes - findUserById added to auth-queries.ts following established pattern Claude-Session: https://claude.ai/code/session_01VTzaSyvpBCoWAocwbiywpk
- revoked/expired guards now run before rehash: a dead token found via the previous pepper is rejected without any DB write (finding #1) - pepper version is no longer relative (record version + 1) but sourced from API_TOKEN_PEPPER_VERSION env (default 1), threaded through the service config, module wiring, and middleware deps so both creation and rotation always write the same canonical version (finding #2) - two new tests: revoked-via-prev-pepper does not call rehash; prev- pepper rehash uses deps.pepperVersion (3) not record version + 1 (2) Claude-Session: https://claude.ai/code/session_01VTzaSyvpBCoWAocwbiywpk
POST / returns raw secret once; GET and DELETE strip tokenHmac and pepperVersion from responses. Org-scoped creation applies requireOrg + requireOrgPermission inline — body scope drives auth, not path. Wrong-owner revoke returns 404 via AppErrorException(API_TOKEN_NOT_FOUND). Also renames API_TOKEN_EXPIRY_TOO_LONG → API_TOKEN_EXPIRY_INVALID to satisfy ErrorCode suffix constraint required by AppErrorException. Claude-Session: https://claude.ai/code/session_01VTzaSyvpBCoWAocwbiywpk
an admin impersonating a user could call POST /settings/tokens, obtain a persistent API token scoped to the impersonated account, then end the session — the token would survive indefinitely with the audit log attributing it to the victim, not the admin. adding denyImpersonated after requireAuth on both mutating routes closes this escalation path. GET is left open (read-only support access is legitimate). test covers the 403 on impersonated POST. Claude-Session: https://claude.ai/code/session_01VTzaSyvpBCoWAocwbiywpk
Implements POST /api/token-scanning/github which receives GitHub Secret Scanning notifications, verifies the ECDSA P-256 signature (DER→raw conversion required — WebCrypto expects IEEE P1363, not DER), revokes each matching token with reason "leaked", emits api_token.revoked, and notifies the owner via email. Claude-Session: https://claude.ai/code/session_01VTzaSyvpBCoWAocwbiywpk
…an op the resolveKey call was outside the try/catch in verify() — a network error to api.github.com at cold start propagated as 500 instead of a clean 403 refusal, leaving a leaked token active during the outage window. moved it inside the try so any key-resolution failure captures and returns false. the crypto.subtle.verify span carried op: "http.client" — pure cpu compute, no outbound request. removed op so otel/sentry do not classify it as an http call in latency metrics. test added: github endpoint unreachable → verify() returns false, no throw. Claude-Session: https://claude.ai/code/session_01VTzaSyvpBCoWAocwbiywpk
- promote SecretRevealDialog to shared/components with optional title/description props - add features/api-tokens: queries, mutations, TokenRow, TokenForm, page, route, tests - register /settings/api-tokens tab (no requiresOrg) and route under settingsLayout - org/personal scope selector renders only when hasMembership + apiToken:create Claude-Session: https://claude.ai/code/session_01VTzaSyvpBCoWAocwbiywpk
Missing test fixture added after template type was declared in templates.ts but stub not added to the exhaustive STUB_VARS map. Claude-Session: https://claude.ai/code/session_01VTzaSyvpBCoWAocwbiywpk
mock.module is process-global in Bun, so the module mocks boundary.test.ts needed to import publicApiV1 leaked into every file that ran after it — a mocked env stripped the internal routes of their signing key and turned 39 unrelated assertions red. createPublicApiV1(deps) takes the repository, outbox, pepper config, limiter and ip resolver, so the test injects stubs instead of rewriting the module registry. The middleware order still lives in the factory, so the boundary tests keep exercising the real mount. Claude-Session: https://claude.ai/code/session_01VTzaSyvpBCoWAocwbiywpk
The stub was changed from success:true to success:false to prevent
destructuring crashes on leaked mocks, but assertPayloadValid throws
on !success, breaking 3 enqueue tests.
The correct stub is { success: true, data: {} } — assertPayloadValid
passes, leaked code doing const { x } = parsed.data gets {} not undefined.
Includes scanning.routes factory refactor and associated type fixes.
Claude-Session: https://claude.ai/code/session_01VTzaSyvpBCoWAocwbiywpk
Aligns four test files with the project's established pattern: mock.module() before await import(), full export superset. - drizzle-api-token.repository.test.ts: full rewrite as mock-based unit test (was integration test using real Postgres). mock.module(@packages/drizzle) with controlled dbBehavior + spyOn pattern, same as webhook repo tests. - revoke-on-membership-lost.test.ts: mock.module(@packages/events) with real OrgMemberRemovedPayload Zod schema + mock.module(event-emitter) restoring outbox.enqueue call; dynamic import of handler after mocks. - notify-impersonated-user.test.ts: mock.module(@packages/events) with real AdminImpersonationStartedPayload before existing dynamic import. - github-key-verifier.test.ts: mock.module(event-emitter) restoring real outbox.enqueue behaviour before dynamic import of scanning.routes. All mocks use the full EventTypes superset to avoid partial-mock leaks to subsequent files running in the same Bun worker. Claude-Session: https://claude.ai/code/session_01VTzaSyvpBCoWAocwbiywpk
EVENTS.md documents the visibility allowlist and its three consumers: publishing an event makes it a contract we can no longer rename, so the decision lives in code and gets reviewed in a PR. Catalogue: 65 events, 28 public / 37 internal. apps/api/CLAUDE.md gains the public-api/ layout entry and, more importantly, writes down the AppType exception — /api/v1 stays outside the chained routes because it serves external consumers, not the typed internal client. Without that note the next reader applies the general rule and collapses the boundary. Also translates the seven admin.* event descriptions to English (they feed the integrator-facing catalogue) and updates the webhooks group test, which now asserts the absence of a "webhook" group since every webhook.* event became internal. Claude-Session: https://claude.ai/code/session_01VTzaSyvpBCoWAocwbiywpk
- banned user bypass: check banned+banExpires in api-token middleware at request time (not on ban event — ban may be temporary) - secret scanning pepper gap: fall back to pepperPrevious in scanning routes so tokens rehashed mid-rotation are not false-positived - bulk revoke disable: restrict isRevoking to the mutating row via revoke.variables === token.id Claude-Session: https://claude.ai/code/session_01VTzaSyvpBCoWAocwbiywpk
|
🎉 This PR is included in version 1.24.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase C.4 — Personal Access Tokens, plus the two public-surface decisions that came with them.
What ships
Tokens.
clean_+ 44 base58 chars + a 6-char CRC32 checksum. The prefix makes the token findable by GitHub's scanner, the checksum makes it verifiable offline (a malformed token is rejected before touching the database), and 256 bits of entropy make it unguessable. Stored asHMAC-SHA256(pepper, token)in a unique-indexed column — not sha256 + per-row salt, which would have destroyed the O(1) lookup while buying nothing against a 256-bit secret. The pepper lives in the environment, so a database dump alone yields no usable token.Pepper rotation runs without downtime:
API_TOKEN_PEPPER_PREVIOUSis tried on miss and the row is re-hashed in place, withpepper_versiontelling you exactly what is left to migrate before you drop the old value.The public surface is opt-in by construction.
/api/v1is a separate sub-app that never mountssessionMiddleware; every other route stays session-only. A cookie opens nothing there, a token opens nothing elsewhere, and the boundary is physical rather than a convention re-checked at every PR. A useful side effect: token management lives outside/api/v1, so a token can never mint a token — the scope-escalation vector simply doesn't exist in this topology.The event catalogue is now curated. Publishing an event makes it a contract we can no longer rename, so
visibility-map.tsdeclares each onepublicorinternal, exhaustively enough that a new event won't compile without a decision. 65 events: 28 public, 37 internal.Also: cascade revocation when a token's creator loses their membership, and
POST /api/token-scanning/github— ECDSA P-256, not the HMAC used by ordinary webhooks — which revokes leaked tokens within seconds of a bad push and emails the owner.Notable catches during review
denyImpersonatedwas missing on token creation: an admin impersonating a user could mint a token that outlived the session, attributed to the victim in the audit log.pepper_versionwas incremented relatively, which broke on the second rotation.Verification
pnpm testgreen (16/16 Turbo tasks) ·pnpm ci:checkexit 0, zero warnings · 641 API tests, 112 app tests.New required env var in production:
API_TOKEN_PEPPER(min 32 chars, fail-hard at boot). Rotation procedure documented indocs/DEPLOY-RAILWAY.md.https://claude.ai/code/session_01VTzaSyvpBCoWAocwbiywpk