Repository navigation
Conversation
Legacy API keys grant no access: the data proxy has not accepted them since source-cooperative/data.source.coop#116, and this API has accepted only the proxy's signed tokens and session cookies since #283. Yet both create routes still minted plaintext secrets, and DELETE /api/v1/api-keys/{access_key_id} authorized the caller, deleted nothing, and answered "API key deleted successfully". POST /api/v1/accounts/{account_id}/api-keys and POST /api/v1/products/{account_id}/{repository_id}/api-keys now answer 410 Gone with a JSON error pointing to service-account API keys, without reading the session or the body. GET /api/v1/api-keys/{access_key_id}/auth answers 410 too: its only caller was the pre-Workers proxy, which authenticated with a credential this API no longer accepts, and it was the one route that returned a secret_access_key. The two list routes keep serving redacted keys for one release with Deprecation (RFC 9745) and Sunset (RFC 8594) headers, so owners can find their keys. DELETE now deletes the row, carries the same headers, and answers 500 rather than success when the delete fails. Code with no remaining caller goes: generateAccessKeyID and generateSecretAccessKey, APIKeyRequestSchema, the api_key:create action and its rule, and the table client's create, update and upsert. The served OpenAPI components drop APIKey and APIKeyRequest, and the ApiKeyAuth scheme that described the retired credential. The api-keys table, its CDK definition and its local seed stay until the routes are removed. Part of #549. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R1eiTse4416N6uTgAy4Ddd
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Claude finished @alukach's task in 1m 19s —— View job Review
✅ No blocking issues — safe to merge. Went through the full diff (
Nothing here reads as a correctness, security, or best-practice problem. Simplify (ponytail): nothing to flag — this PR is net −316 lines, the new Docs: The description already covers this per CLAUDE.md's requirement — it states docs.source.coop has no page describing legacy keys (checked 💰 Estimated review cost: $0.57 · 1m19s · 25 turns |
## What I'm changing The legacy API keys in the `api-keys` table grant no access. The data proxy stopped accepting them in source-cooperative/data.source.coop#116, and this API has accepted only the proxy's signed tokens and session cookies since #283. The six routes that still touch the table only do harm: two POSTs mint new plaintext secrets, `GET /api-keys/{access_key_id}/auth` hands a key's plaintext secret to an admin, and `DELETE /api-keys/{access_key_id}` deletes nothing while answering "API key deleted successfully". This PR removes the subsystem outright: - **Routes** (under `/api/v1`): `POST`/`GET /accounts/{account_id}/api-keys`, `POST`/`GET /products/{account_id}/{repository_id}/api-keys`, `GET /api-keys/{access_key_id}/auth` and `DELETE /api-keys/{access_key_id}`. All now 404. - **Code:** the `APIKey`, `APIKeyRequest` and `RedactedAPIKey` schemas, the `apiKeysTable` client, `generateAccessKeyID`/`generateSecretAccessKey` (all of `src/lib/actions/crypto.ts`, which nothing else calls), the `api_key:get|create|revoke` and `account:listAPIKeys`/`repository:listAPIKeys` authz actions with their checks and tests, and the api-key mocks in the product route tests. - **OpenAPI:** the `ApiKeyAuth` security scheme, which described the `<access-key-id> <secret-access-key>` header this API stopped accepting in #283. No replacement scheme is added here. - **Infrastructure:** the `api-keys` table in `DatabaseConstruct` and its Vercel grant in `api-stack.ts`, its creation in `scripts/init-local.ts`, and `fixtures/api-keys.json`. Untouched: `ORY_PROJECT_API_KEY` and the proxy's Ory `adminApiKey`, which are unrelated, and the service-account API keys (`service-account-keys` table) from #567/#580. No UI ever managed legacy keys, so there is no UI or Storybook change. ## Decisions to flag **No deprecation window.** #582 proposed one release of `Deprecation`/`Sunset` headers on the list routes and a working DELETE. This PR goes straight to removal instead: the keys authorize nothing, so the only clients the window protects are scripts listing keys that do nothing. Merging this supersedes #582. **What happens to the table on deploy.** `api-stack.ts` gives production `RemovalPolicy.RETAIN`, so dropping the table from CDK leaves the prod `api-keys` table in place, no longer managed by the stack, plaintext secrets included. Dev and staging use `DESTROY`, so their tables are deleted on the next deploy. Once this ships, the prod table still needs auditing and then deleting out of band (see Follow-ups). **`fix`, not `chore`.** release-please leaves `chore` out of the changelog, and the release notes are where removing public routes gets announced. ## How you can test it What I ran on this branch, rebased onto `main` at 5c35162: - `npm run type-check`: clean. - `npx jest --forceExit`: 83 suites, 873 tests pass. - `npm run lint`: warnings only, none on lines this PR touches. - A grep for `api-keys`, `apiKeysTable`, `api_key:`, `listAPIKeys`, `APIKeySchema`, `ApiKeyAuth` and the crypto helpers across `src`, `scripts`, `deploy` and `fixtures` finds nothing left. The remaining `secret_access_key` hits are data-connection credentials. Not run: `npm run build` (it needs AWS credentials), `cdk diff`, and nothing against a preview or staging. Expected `cdk diff` against prod: the `api-keys` table resource and its grant on the Vercel role disappear, with the table retained. By hand on the preview: `(await fetch("/api/v1/accounts/<you>/api-keys")).status` gives 404. ## Docs and ADRs - **docs.source.coop:** a search of the repo finds no page describing legacy keys; `docs/using-source/data-upload.md` covers STS session credentials only. Nothing is invalidated. - **data.source.coop ADRs:** ADR-013's context note says the `api-keys` endpoints that "exist in `source.coop` today" are legacy, unrelated to its design, and not accepted by the proxy (ADR-001). Once this merges, "today" is out of date, but the note still records the context correctly and no decision moves, so no ADR changes. - **Out of date once the prod table is deleted:** the org profile README (`profile/README.md` in `source-cooperative/.github`) lists `api-keys: Stores API keys for authentication`. ## Follow-ups 1. Audit the retained prod `api-keys` rows, then delete the table out of band (point-in-time recovery keeps it restorable for its retention window). 2. Drop `api-keys` from the org profile README's table list. ## Related Closes the route and table work in #549; part of #491. Supersedes #582. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
|
Overtaken by #374 |
…665) Closes #594. Part of #588. ## What Service accounts and their API keys over `/api/v1`, documented at `/api/docs`: | Endpoint | Operation | |---|---| | `GET /accounts/{owner}/service-accounts` | `listServiceAccounts` → `{items, next_cursor: null}` | | `POST /accounts/{owner}/service-accounts` | `createServiceAccount` — `local_id`, `name`, `github_subjects[]`, `grants[{product_id, role}]`, optional `key{label, expires_at}`; the response carries the issued key when one is asked for | | `GET /accounts/{sa}/trusts` | `listTrusts` → `{items, next_cursor: null}` | | `POST /accounts/{sa}/trusts` | `addGithubTrust` — `{subject}` | | `DELETE /accounts/{sa}/trusts?issuer=&subject=` | `removeTrust` — 404 for a trust the account doesn't have; returns the trust as it was | | `GET /accounts/{sa}/keys` | `listKeys` → `{items, next_cursor: null}`, revoked and expired keys included, never a hash | | `POST /accounts/{sa}/keys` | `issueKey` — `{label, expires_at}` → 201 with `key`, the only time it appears | | `PATCH /accounts/{sa}/keys/{key_id}` | `updateKey` — `{expires_at?, revoked?: true}` | `src/lib/operations/service-accounts.ts` holds the rules the issue names: `GITHUB_ACTIONS_SUBJECT_REGEX` (as a Zod schema shared with the OpenAPI doc), the read/write-only grant roles, `managedServiceAccount` (every "not yours / missing / not a service account" answers 404, keeping the single answer it gave before), and the rule that a service account can't own another. The server actions in `src/lib/actions/service-accounts.ts` now adapt FormData into those operations, and the service-accounts settings page lists through `listServiceAccounts`. Editing and deleting a service account are main's `PATCH` and `DELETE /accounts/{account_id}` (#674, #675): rename and disable call `updateAccount`, and delete calls `deleteAccount`, so this PR adds no operation of its own for them. Trusts are returned through `AccountTrustSchema`, so the table's `identity` sort key stays out of responses. `setProductAccess` now calls `inviteMember` / `updateMembership` / `revokeMembership`, and creation grants each product through `inviteMember` too, so every grant passes the same authorization, duplicate check and audit log; so product access over the API is the product members endpoints from #589 with role `read_data` or `write_data`, and gets no endpoint of its own. API keys were first left to #570 and #580–#582, which built the keys and their settings UI but no API; with those merged, this PR adds the API for them (scope noted on #594). `issueApiKey`, `revokeApiKey`, `setApiKeyExpiry` and the create action now map form fields onto `issueKey`, `updateKey` and `createServiceAccount`, so no action touches the keys table, and the key rules (hash-only storage, the six-character hint, refusing a disabled account, the expiry range) are tested once, on the operations. ## Worth a look - Revoking is `PATCH {"revoked": true}`, not `DELETE`: the record stays, with `revoked_at`, `revoked_via` and `revoked_by`, as disabling an account is a `PATCH`. `revoked: false` is refused with 400, since a revoked key can't be restored; revoking one twice is 409, and a PATCH with neither field is 400. - `expires_at` is an ISO time, at most 3650 days away, or `null` for a key that lasts until revoked. It is required on issue, so a caller chooses no-expiry deliberately. The settings form still takes days and converts them. - A key requested with `POST /service-accounts` is validated with the rest of the body, before anything is written, and stored after the grants and trusts. - `createServiceAccount` checks every grant against `InviteMembership` before it writes the account, so a grant the caller may not make leaves nothing behind and a retry with the same `local_id` succeeds. `inviteMember` can refuse nothing else afterwards: the account is new and enabled, and the grants are deduplicated first. - Deleting a service account is main's `deleteAccount` on both the API and the settings page. It deletes the account's API keys rather than revoking them, and the key exchange also refuses a key older than its account, so a key from a deleted account can't sign in as the next account to take its id. ADR 013 still holds. - `setProductAccess` refuses a new grant to a disabled service account (#666) through `inviteMember` and `updateMembership`, which both check `serviceAccountGrantProblem`. Revoking still works while the account is disabled. - Service-account lists use the `{items, next_cursor}` page shape, as products and data connections do, so pagination can arrive later without a breaking change. ## Docs, ADRs, llms.txt - docs.source.coop `docs/using-source/upload/with-service-account.md` walks through the settings UI, which is unchanged, so it still holds; it could later mention issuing a key over the API, which #595's reference covers meanwhile. The API reference is #595's to publish. - data.source.coop ADRs: I checked 005 (authorization), 010 (account-owned roles) and 013 (API keys). ADR 013 has source.coop generate the key, store only its hash, show it once, refuse a disabled service account and keep expiry editable, with `key_id` as the public handle; the key endpoints do exactly that, so this PR implements existing decisions and moves none. - `public/llms.txt`: no change. These endpoints all need auth, so they can't go on the file's no-auth URL list, and its existing "API reference" line points at `/api/openapi`, which now includes them. ## Storybook No component changed. The settings page now gets its list from the operation, and the key actions keep their form fields and state shapes, so `IssueApiKeyDialog` and the rest are untouched. ## Testing - `npm run type-check`: clean. - `npx jest`: 96 suites, 1068 tests pass. That includes the service-account action tests, now run through the operations; the key action tests, now checking only the form mapping; new operation tests (list authz, JSON create, the 404 on an unknown trust, a refused grant or bad key writing nothing, issuing, listing, expiring and revoking keys); and the OpenAPI validity and route-coverage tests. - Not exercised against a running server. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
What I'm changing
The legacy API keys in the
api-keystable grant no access. The data proxy stopped accepting them in source-cooperative/data.source.coop#116, and this API has accepted only the proxy's signed tokens and session cookies since #283. Six routes still touch the table, though: two POSTs mint new plaintext secrets, andDELETE /api/v1/api-keys/{access_key_id}authorizes the caller, deletes nothing, and answers "API key deleted successfully", so every key a user believes they revoked is still in the table in plaintext.This PR retires those routes (paths under
/api/v1):POST /accounts/{account_id}/api-keysPOST /products/{account_id}/{repository_id}/api-keysGET /accounts/{account_id}/api-keysDeprecationandSunsetheadersGET /products/{account_id}/{repository_id}/api-keysDeprecationandSunsetheadersGET /api-keys/{access_key_id}/authDELETE /api-keys/{access_key_id}DeprecationandSunsetheaders; 500 rather than success if the delete failsEvery 410 answers
{"error": "Legacy API keys are retired and grant no access. For software that needs access, create a service account in your account settings and issue it an API key."}.Code with no remaining caller goes:
generateAccessKeyIDandgenerateSecretAccessKey(all ofsrc/lib/actions/crypto.ts),APIKeyRequestSchema, theapi_key:createaction with its authz rule and test, and the table client'screate,updateandupsert. The table client keepsfetchByIdandlistByAccountfor the routes that still serve, and gainsdelete. No UI ever managed these keys, so there is no UI or Storybook change.The table stays.
deploy/,scripts/init-local.ts,fixtures/api-keys.jsonand the fixture loader that seeds it are untouched; dropping the table is sequenced after the routes are gone (see Follow-ups).Decisions to flag
The list GETs keep serving for one release, with
DeprecationandSunsetheaders. They return only redacted fields, so serving them costs nothing, and they are how an owner finds a key's id in order to delete it. Answering 410 at once would break any script without notice and leave owners no way to find their keys. The headers areDeprecation: @1790294400(2026-09-25; RFC 9745) andSunset: Sun, 01 Nov 2026 00:00:00 GMT(RFC 8594). The sunset date is my choice: about five weeks out, which on the recent release cadence (v1.5.0 on 4 Aug to v1.6.1 on 24 Sep, with gaps of 9 to 25 days) spans the release carrying this PR and the one after. The follow-up that removes the routes should not ship before it; if this PR merges late, move the date so the window still spans a release.GET /api-keys/{access_key_id}/authanswers 410 now, which departs from the issue's "deprecation response for the GET routes". It is not a listing. It was the pre-Workers proxy's credential lookup, and it returns a key'ssecret_access_keyin plaintext to an admin. Its only caller was removed in source-cooperative/data.source.coop#116, and that caller authenticated with a raw API key header thatgetApiSessionstopped accepting in #283, so no client can depend on it. A deprecation window would keep serving plaintext secrets to protect a caller that cannot exist.DELETE deletes, rather than answering 410. It is the smallest honest fix, and with the list routes it lets owners purge their plaintext keys during the window instead of waiting for the table drop. It keeps its authorization (
api_key:revoke) and its 200 body, which is now true. That rule still refuses a non-admin on a key markeddisabled; no route sets the flag, and an admin can delete any such key. Rows deleted this way are gone before the audit below, though point-in-time recovery keeps them restorable for its retention window.fix, notchore. release-please leaveschoreout of the changelog, and the release notes are where the deprecation window gets announced.OpenAPI. Each changed route's
@openapiJSDoc marks itdeprecated: trueand describes its new behaviour; the account route gains the JSDoc it lacked. That JSDoc does not reach the served spec:/api/openapihas servedpaths: {}since swagger-jsdoc was removed in #369. The served spec changes only in its components.APIKey, which publishedsecret_access_key, andAPIKeyRequestgo;RedactedAPIKeystays for the list routes; and theApiKeyAuthsecurity scheme goes, because it described the<access-key-id> <secret-access-key>header this API stopped accepting in #283. No replacement scheme is added here.The 410 points to service-account API keys, which ship with #570. Service accounts are already on
main(#567). If this merges before #570, the pointer is one step ahead until #570 lands.A quirk left alone:
GET /accounts/{account_id}/api-keyslists the signed-in account's keys whateveraccount_idnames, as it always has. Its new JSDoc says "the signed-in account's". Fixing a route that is going away isn't worth it.How you can test it
What I ran on this branch:
npm run type-check: clean.npx jest api-keys src/lib/api/authz.test.ts "src/app/api/v1/products/\[account_id\]/route.test.ts" "src/app/api/v1/products/\[account_id\]/\[repository_id\]/route.test.ts" --forceExit: 8 suites, 80 tests pass.npx jest --forceExit(the whole suite): 79 suites, 813 tests pass.npm run lint: exits 0. It prints warnings only, and none on lines this PR adds.await apiKeysTable.delete(...)line, or only itsawait, fails the DELETE suite.The tests cover: 410 on each POST and on
/auth; the list GETs still return redacted keys and carry both headers; DELETE deletes the row when authorized, deletes nothing when the caller may not (401) or the key is missing (404), and answers 500 when the delete fails;APIKeysTable.deletesends aDeleteCommandkeyed byaccess_key_idto the right table.Not run:
npm run build(it needs AWS credentials), and nothing against the preview, staging or a local DynamoDB.By hand, from a signed-in browser console on the preview:
(await fetch("/api/v1/accounts/<you>/api-keys", { method: "POST" })).statusgives 410;(await fetch("/api/v1/accounts/<you>/api-keys")).headers.get("Sunset")gives the sunset date; and afterfetch("/api/v1/api-keys/<id>", { method: "DELETE" })the key no longer appears in the list.Docs and ADRs
mainfor API-key instructions. No page describes the legacy keys:docs/using-source/data-upload.mdcovers STS session credentials only, and already tells users not to request permanent access keys. Nothing is invalidated.mainand as revised in docs(adr): revise ADR-013 — API keys are opaque secrets resolved by the platform data.source.coop#234. ADR-013's context note says theapi-keysendpoints are legacy, unrelated to its design, and not accepted by the proxy. That still holds. This PR moves no decision, so no ADR changes.profile/README.mdinsource-cooperative/.github) listsapi-keys: Stores API keys for authentication.Follow-ups
These follow #549's sequence, each as its own change after this one ships:
APIKeySchemaandRedactedAPIKeySchema, the remainingapi_key:*and*:listAPIKeysauthz actions,fixtures/api-keys.jsonand its loader.deploy/lib/database-construct.ts, and the Vercel grant indeploy/lib/api-stack.ts) and fromscripts/init-local.ts. Production setsRETAIN, with point-in-time recovery on, so then delete the table out of band.Related
Part of #549: the routes are not gone yet, and the table drop, delete and audit remain. Part of #491. This re-cuts the stale draft #374 from current
maininstead of rebasing it; #374 is left as it is.🤖 Generated with Claude Code
https://claude.ai/code/session_01R1eiTse4416N6uTgAy4Ddd