Skip to content

refactor(api-keys): single dashboard-only create path with explicit roles - #2182

Open
multipletwigs wants to merge 3 commits into
bashtwigs/api-key-role-drop-defaultfrom
bashtwigs/api-key-create-consistency
Open

multipletwigs wants to merge 3 commits into
bashtwigs/api-key-role-drop-defaultfrom
bashtwigs/api-key-create-consistency

Conversation

@multipletwigs

@multipletwigs multipletwigs commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

Stacked on #2197 (drops the api_keys.role column default); #2198 on top revokes every existing key. Consolidates API-key creation onto one dashboard-only path. POST /v1/api-keys requires a signed-in user plus x-project-id; /v1/projects/:projectId/api-keys is deleted. created_by is always the acting user.

  • API keys can no longer mint API keys: requireUserActor runs before requirePermissions on create, so any key gets 403.
  • role is required: zod default removed; the column default drop ships in fix(api): drop the api_keys.role column default #2197.
  • A permissions override may only narrow the role preset, checked before the org:admin exemption and shared by create, update, and rotate. fix(api): revoke every existing API key #2198 revokes every key minted before this lands.
  • provisionWallet / walletLabel / walletPurpose removed from create; create the wallet via custody, then bind it.
  • Create is internal-spec only: it is Clerk-authenticated, so it is out of the public OpenAPI document and Postman collection.

API before/after (local, key actor; before values reconstructed):

POST /v1/projects/{projectId}/api-keys        201 → 404
POST /v1/api-keys  (Bearer sk_test_…)         201 → 403 "API key creation requires a signed-in user"
PATCH /v1/api-keys/{id} permissions:["payments:write"] on api_readonly   200 → 400 "permissions cannot exceed the api_readonly role preset"

Verification: @sdp/api + sdp-web typecheck pass; local dashboard create flow walked end to end (wizard → generated key → api_keys and audit_logs rows); adversarial pass over 8 vectors with live probes, 0 exploitable. Not run: vitest. CI is red on six stale suites that still exercise the deleted route and key-mints-key paths; a follow-up test PR rewrites them.

@vercel

vercel Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
sdp-docs Ready Ready Preview Oct 4, 2026 3:17am UTC
sdp-web Ready Ready Preview Oct 4, 2026 3:17am UTC

Request Review

@greptile-apps

greptile-apps Bot commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 4/5

[Critical risk] Restricts API key creation to signed-in users only.

The PR does not yet appear safe to merge because legacy keys can retain permissions beyond their role during unrelated updates.

Summary

The PR consolidates API-key creation into a signed-in-user-only route, requires explicit roles, restricts permission overrides, and removes the former project creation route and wallet-provisioning option. The latest changes update tests and integration helpers for that contract.

Reviews (7) · Last reviewed commit: "test(api-keys): mint keys as a signed-in..."

Comment thread apps/sdp-api/src/openapi/paths/api-keys.ts Outdated
Comment thread apps/sdp-api/src/openapi/schemas/api-keys.ts Outdated
Comment thread apps/sdp-api/src/services/api-key-scope.service.ts
…oles

Delete POST/GET /v1/projects/:projectId/api-keys and route all key
creation through POST /v1/api-keys, which now requires a signed-in user
plus x-project-id; API keys can no longer mint API keys. Role is required
(zod default and column default removed), a permissions override may only
narrow the role preset, and the wallet-provisioning fields are gone from
the create body. created_by is always the acting user. Docs and generated
reference updated.
Register POST /v1/api-keys in the internal document only, since the
public document defines no clerkBearerAuth scheme, and regenerate the
Postman collection and llms-full.txt. Expiration examples move to a
future date. The role DROP DEFAULT migration moves to its own
migrations-only PR so this change stays rollback-safe.
@multipletwigs
multipletwigs force-pushed the bashtwigs/api-key-create-consistency branch from de2e1ac to 566ceb7 Compare October 4, 2026 03:00
@multipletwigs
multipletwigs changed the base branch from bashtwigs/hoo-1890-revoke-all-api-keys to bashtwigs/api-key-role-drop-default October 4, 2026 03:01
Key creation is Clerk-only now, so tests mint keys through a signed-in
admin with x-project-id and an explicit role. Integration tests get a
signInTestUser helper. Cases for the deleted project route, create-time
wallet provisioning, and key-mints-key guards are removed.
@multipletwigs

Copy link
Copy Markdown
Collaborator Author

On the remaining risk ("existing keys remain active" and the legacy-update gap): #2198, stacked on top of this PR, revokes every existing API key (0124_revoke_all_api_keys.sql). It can only merge with or after this PR, so no pre-cap key survives the rollout. The public auth scheme, Postman, and expiration-example findings are fixed in 566ceb7.

This branch was successfully deployed

2 active deployments
Preview – sdp-web — df32adc1 Deployed Oct 4, 2026 by vercel[bot]
Preview – sdp-docs — df32adc1 Deployed Oct 4, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant