Skip to content

fix(api): revoke every existing API key - #2198

Open
multipletwigs wants to merge 1 commit into
bashtwigs/api-key-create-consistencyfrom
bashtwigs/hoo-1890-revoke-all-api-keys
Open

multipletwigs wants to merge 1 commit into
bashtwigs/api-key-create-consistencyfrom
bashtwigs/hoo-1890-revoke-all-api-keys

Conversation

@multipletwigs

@multipletwigs multipletwigs commented Oct 4, 2026 •

Copy link
Copy Markdown
Collaborator

Top of the stack, on #2182 (and #2197). Revokes every active API key so no key created before explicit roles can carry permissions beyond its role preset.

Verification: check:migration-compat passes against #2197's branch; no service in this repo or sdp-infra authenticates with a stored SDP API key. Migration not applied to a live DB; no migration test.

@vercel

vercel Bot commented Oct 4, 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

@linear

linear Bot commented Oct 4, 2026

Copy link
Copy Markdown

HOO-1890

@greptile-apps

greptile-apps Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 3/5

[Critical risk] Database migration that revokes all active API keys.

The PR does not appear safe to merge while the migration can revoke production keys and leave some cached keys usable beyond the next reconciliation tick.

Findings

  1. P1 Production keys will be revoked ▶
  2. P1 Security Bulk revocation misses cached keys ▶

Summary

The PR adds a PostgreSQL migration that marks every active API key as revoked and records a revocation timestamp. The migration has not changed since the previous review.

Reviews (3) · Last reviewed commit: "fix(db): revoke every existing API key"

@@ -0,0 +1,2 @@
-- sdp:migration-compat: breaking
UPDATE api_keys SET status = 'revoked', revoked_at = sdp_datetime_now() WHERE status = 'active';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Production keys will be revoked

This update has no devnet-only condition. The migration runner applies it to every database receiving this release, including production. If production has active API keys, deploying this migration will revoke them and their clients will lose access, contrary to the stated devnet-only rollout.

@@ -0,0 +1,2 @@
-- sdp:migration-compat: breaking
UPDATE api_keys SET status = 'revoked', revoked_at = sdp_datetime_now() WHERE status = 'active';

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 security Bulk revocation misses cached keys

If more than 10,000 keys are active when this runs, the update gives them the same revoked_at value. The cache reconciler selects only 10,000 matching rows per tick, with no cursor; repaired rows still match its query, so later ticks can select them again instead of reaching the remaining keys. A remaining key cached as active can keep authenticating until its one-hour cache TTL expires, rather than being evicted on the next tick.

How this was verified: Authentication accepts cached active keys, and cache repair does not remove their database rows from the reconciler’s limited query.

Knowledge Base Used: Platform API service

@multipletwigs
multipletwigs force-pushed the bashtwigs/hoo-1890-revoke-all-api-keys branch from 6ec6ccb to cd57e57 Compare October 4, 2026 03:01
@multipletwigs
multipletwigs changed the base branch from bashtwigs/api-key-role-drop-default to bashtwigs/api-key-create-consistency October 4, 2026 03:01
Keys created before explicit roles can hold permissions beyond their
role preset, and no update or rotate path corrects them. Devnet only,
so revoke them all; integrators mint fresh keys from the dashboard.

This branch was successfully deployed

2 active deployments
Preview – sdp-web — 3fd37224 Deployed Oct 4, 2026 by vercel[bot]
Preview – sdp-docs — 3fd37224 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