Skip to content

API key management: MCP key minting, archived-team keys, and listing bugs #59

Closed

Description

@clbotdev

Found while porting SendLit's API key design to MediaLit. Each item below was checked against main (00bf083).

Security

1. A team API key can create and delete team keys over MCP

  • authorizeMcpTool returns early for team keys (apps/api/src/mcp/policy.ts:140, if (authKind === "team_key") return null;).
  • create_api_key and delete_api_key (apps/api/src/mcp/tools/teams.ts:211, :255) add no check of their own.
  • REST already refuses this: team/routes.ts returns 403 user_auth_required to key callers.

Impact: a leaked key can mint replacement keys for itself, so revoking the leaked key doesn't cut off the attacker. It can also revoke the team's other keys.

Fix: deny list_api_keys, create_api_key and delete_api_key to team_key callers, as FrontLit does with apiKeyDeniedTools.

2. Keys of archived teams keep authenticating

  • archiveTeam (apps/api/src/team/queries.ts:~426) sets status = 'archived', but doesn't revoke the team's keys.
  • Closing an organization (organization/queries.ts:~131) revokes organization keys and archives teams, but leaves the team keys active.
  • getApiKeyBySecret (apikey/queries.ts:47) checks only revoked_at and expires_at. It doesn't join teams to check status.

Fix: revoke a team's keys in the same transaction that archives it, including from the organization-close path. Also join teams in getApiKeyBySecret and reject archived teams, so the check holds even if a revoke is missed.

3. Any team member can manage team keys over REST

listKeys, createKey and removeKey use resolveTeamParam (team/routes.ts:83). It only requires membership, with no role check, so any member can mint a full-access key for the team. FrontLit makes these operations owner-only. If member access is intended, please close this item; otherwise, restrict them to owners or admins.

Correctness

  1. last_used_at is never written. No code path updates team_api_keys.last_used_at or organization_api_keys.last_used_at, so the dashboard always shows "Never". Update it in the resolver on successful authentication, throttled (for example, WHERE last_used_at IS NULL OR last_used_at < now() - interval '5 minutes').
  2. Revoked keys are listed as active. getApiKeysByTeamId (apikey/queries.ts:67) doesn't filter revoked_at. The settings page renders every row, including revoked ones, each with a Revoke button (apps/web/app/(dashboard)/settings/page.tsx:560). The MCP list_api_keys tool returns them too.
  3. DELETE /teams/:teamId/keys/:keyId returns 204 for unknown or already-revoked keys (team/routes.ts:226-227). It should return 404 when nothing was revoked, as FrontLit does.
  4. All organization keys share one display prefix. DISPLAY_PREFIX_LENGTH = 12 equals the length of sl_org_live_, so displayPrefix() returns exactly sl_org_live_ for every organization key (apikey/secret.ts:9), and keys can't be told apart in the UI. Make the display prefix a fixed number of characters past the type prefix.

Docs

  1. apps/api/docs/apikey-management.md still describes the api_keys table, kind: "apikey" and immediate deletion on revoke. That model was replaced when team_api_keys, soft revocation and organization keys were introduced.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingseverity: highMajor gap or broken flow for many users

    Type

    No type

    Projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions