Repository navigation
feat(api): add endpoint restrictions for client API keys - #2777
diegosouzapw merged 3 commits into
Conversation
Add `allowedEndpoints` field to API keys that restricts which endpoint categories a key can access. Supports 14 categories (chat, search, embeddings, images, audio, video, music, rerank, models, moderations, batches, files, web-fetch, agents). Empty array = all endpoints allowed for backward compatibility. Includes dashboard UI for configuring restrictions in the permissions modal and 24 unit tests.
There was a problem hiding this comment.
Code Review
This pull request introduces API key endpoint restrictions, allowing keys to be restricted to specific categories of API endpoints (e.g., chat, search, embeddings). The changes span database schema updates, policy enforcement checks, UI components for managing permissions, and comprehensive unit tests. The review feedback highlights critical security and correctness issues: an authorization bypass in getApiKeyMetadata due to a missing camelCase conversion for allowed_endpoints, a potential bypass in relative URL parsing within the policy check, a case-sensitivity bypass in endpoint category resolution, and a recommendation to use a Zod enum to validate allowed endpoint categories.
| scopes: parseStringList((record as JsonRecord).scopes), | ||
| isBanned: parseIsBanned(record.is_banned ?? (record as JsonRecord).isBanned), | ||
| keyHash: (record.key_hash ?? (record as JsonRecord).keyHash) as string | null, | ||
| allowedEndpoints: parseStringList((record as JsonRecord).allowedEndpoints), |
There was a problem hiding this comment.
In getApiKeyMetadata, rowToCamel is not called on the database row. Therefore, the database column allowed_endpoints is not converted to camelCase. Accessing (record as JsonRecord).allowedEndpoints directly will always return undefined, resulting in an empty array and bypassing the endpoint restrictions for all API requests. You should check both record.allowed_endpoints and record.allowedEndpoints to ensure backward compatibility and correct behavior.
| allowedEndpoints: parseStringList((record as JsonRecord).allowedEndpoints), | |
| allowedEndpoints: parseStringList(record.allowed_endpoints ?? (record as JsonRecord).allowedEndpoints), |
| // ── Check 2.5: Endpoint restriction ── | ||
| if (apiKeyInfo.allowedEndpoints && apiKeyInfo.allowedEndpoints.length > 0) { | ||
| try { | ||
| const url = new URL(request.url); |
There was a problem hiding this comment.
If request.url is a relative path (e.g., in certain environments or testing setups), new URL(request.url) will throw a TypeError because it lacks a base URL. This will trigger the catch block and silently fail open, bypassing the endpoint restrictions. Providing a dummy base URL like 'http://localhost' ensures relative URLs are parsed safely without affecting absolute URLs.
| const url = new URL(request.url); | |
| const url = new URL(request.url, "http://localhost"); |
| export function resolveEndpointCategory(pathname: string): string | null { | ||
| for (const { prefix, categoryId } of SORTED_PREFIXES) { | ||
| if (pathname === prefix || pathname.startsWith(prefix + "/")) { | ||
| return categoryId; | ||
| } | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
The path matching is case-sensitive. If a client sends a request with mixed casing (e.g., /v1/Chat/Completions), and the underlying router is case-insensitive, the request will bypass the endpoint restriction because resolveEndpointCategory will return null. Normalizing the pathname to lowercase before matching prevents case-sensitivity bypasses.
| export function resolveEndpointCategory(pathname: string): string | null { | |
| for (const { prefix, categoryId } of SORTED_PREFIXES) { | |
| if (pathname === prefix || pathname.startsWith(prefix + "/")) { | |
| return categoryId; | |
| } | |
| } | |
| return null; | |
| } | |
| export function resolveEndpointCategory(pathname: string): string | null { | |
| const normalizedPath = pathname.toLowerCase(); | |
| for (const { prefix, categoryId } of SORTED_PREFIXES) { | |
| if (normalizedPath === prefix || normalizedPath.startsWith(prefix + "/")) { | |
| return categoryId; | |
| } | |
| } | |
| return null; | |
| } |
| ]) | ||
| .optional(), | ||
| scopes: z.array(z.string().trim().min(1).max(64)).max(16).optional(), | ||
| allowedEndpoints: z.array(z.string().trim().min(1).max(64)).max(20).optional(), |
There was a problem hiding this comment.
The allowedEndpoints field accepts any arbitrary string up to 64 characters. To prevent invalid configurations or typos (e.g., saving 'shat' instead of 'chat'), it is highly recommended to restrict the allowed values to the valid endpoint category IDs using z.enum.
allowedEndpoints: z.array(z.enum([
"chat",
"search",
"embeddings",
"images",
"audio",
"video",
"music",
"rerank",
"models",
"moderations",
"batches",
"files",
"web-fetch",
"agents"
])).max(20).optional(),|
Kilo Code Review could not run — your account is out of credits. Add credits or switch to a free model to enable reviews on this change. |
af5635e
into
diegosouzapw:release/v3.8.5
|
Thank you @hijak for this great contribution! We resolved a small snake_case mapping issue in getApiKeyMetadata and successfully merged it into release/v3.8.5. It will be included in the upcoming release. We appreciate your support! |
* chore(release): bump version to v3.8.5 * fix(docker): rebuild better-sqlite3 after hardened install (#2772) Integrated into release/v3.8.5 * ci: build Docker platforms on native runners (#2774) Integrated into release/v3.8.5 * docs(release): sync v3.8.5 documentation and metadata Update changelog entries, API reference version, package metadata, and localized LLM documentation for the 3.8.5 release. Refresh generated docs source mappings and architecture counts for current executors and OAuth providers. * chore(release): bump to v3.8.5 — changelog, docs, version sync * chore(release): translate Hall of Contributors to English in workflows and changelog * fix(combos): make target timeout configurable (#2775) Merge PR #2775 — fix(combos): make target timeout configurable * feat: fix so restart of server restarts batch jobs instead of failing them (#2755) Merge PR #2755 — feat: fix so restart of server restarts batch jobs instead of failing them * chore(release): update changelog with merged PRs notes and credits * feat(api): add endpoint restrictions for client API keys (#2777) Merge PR #2777 — feat(api): add endpoint restrictions for client API keys * chore(release): update changelog with PR #2777 entry and contributor credit --------- Co-authored-by: Thanet S. <cho.112543@gmail.com> Co-authored-by: Randi <55005611+rdself@users.noreply.github.com> Co-authored-by: Markus Hartung <mail@hartmark.se> Co-authored-by: Jack <5443152+hijak@users.noreply.github.com>
* chore(release): bump version to v3.8.5 * fix(docker): rebuild better-sqlite3 after hardened install (diegosouzapw#2772) Integrated into release/v3.8.5 * ci: build Docker platforms on native runners (diegosouzapw#2774) Integrated into release/v3.8.5 * docs(release): sync v3.8.5 documentation and metadata Update changelog entries, API reference version, package metadata, and localized LLM documentation for the 3.8.5 release. Refresh generated docs source mappings and architecture counts for current executors and OAuth providers. * chore(release): bump to v3.8.5 — changelog, docs, version sync * chore(release): translate Hall of Contributors to English in workflows and changelog * fix(combos): make target timeout configurable (diegosouzapw#2775) Merge PR diegosouzapw#2775 — fix(combos): make target timeout configurable * feat: fix so restart of server restarts batch jobs instead of failing them (diegosouzapw#2755) Merge PR diegosouzapw#2755 — feat: fix so restart of server restarts batch jobs instead of failing them * chore(release): update changelog with merged PRs notes and credits * feat(api): add endpoint restrictions for client API keys (diegosouzapw#2777) Merge PR diegosouzapw#2777 — feat(api): add endpoint restrictions for client API keys * chore(release): update changelog with PR diegosouzapw#2777 entry and contributor credit --------- Co-authored-by: Thanet S. <cho.112543@gmail.com> Co-authored-by: Randi <55005611+rdself@users.noreply.github.com> Co-authored-by: Markus Hartung <mail@hartmark.se> Co-authored-by: Jack <5443152+hijak@users.noreply.github.com>
…w#2777) Merge PR diegosouzapw#2777 — feat(api): add endpoint restrictions for client API keys
…contributor credit
* chore(release): bump version to v3.8.5 * fix(docker): rebuild better-sqlite3 after hardened install (diegosouzapw#2772) Integrated into release/v3.8.5 * ci: build Docker platforms on native runners (diegosouzapw#2774) Integrated into release/v3.8.5 * docs(release): sync v3.8.5 documentation and metadata Update changelog entries, API reference version, package metadata, and localized LLM documentation for the 3.8.5 release. Refresh generated docs source mappings and architecture counts for current executors and OAuth providers. * chore(release): bump to v3.8.5 — changelog, docs, version sync * chore(release): translate Hall of Contributors to English in workflows and changelog * fix(combos): make target timeout configurable (diegosouzapw#2775) Merge PR diegosouzapw#2775 — fix(combos): make target timeout configurable * feat: fix so restart of server restarts batch jobs instead of failing them (diegosouzapw#2755) Merge PR diegosouzapw#2755 — feat: fix so restart of server restarts batch jobs instead of failing them * chore(release): update changelog with merged PRs notes and credits * feat(api): add endpoint restrictions for client API keys (diegosouzapw#2777) Merge PR diegosouzapw#2777 — feat(api): add endpoint restrictions for client API keys * chore(release): update changelog with PR diegosouzapw#2777 entry and contributor credit --------- Co-authored-by: Thanet S. <cho.112543@gmail.com> Co-authored-by: Randi <55005611+rdself@users.noreply.github.com> Co-authored-by: Markus Hartung <mail@hartmark.se> Co-authored-by: Jack <5443152+hijak@users.noreply.github.com>
…w#2777) Merge PR diegosouzapw#2777 — feat(api): add endpoint restrictions for client API keys
…contributor credit
* chore(release): bump version to v3.8.5 * fix(docker): rebuild better-sqlite3 after hardened install (diegosouzapw#2772) Integrated into release/v3.8.5 * ci: build Docker platforms on native runners (diegosouzapw#2774) Integrated into release/v3.8.5 * docs(release): sync v3.8.5 documentation and metadata Update changelog entries, API reference version, package metadata, and localized LLM documentation for the 3.8.5 release. Refresh generated docs source mappings and architecture counts for current executors and OAuth providers. * chore(release): bump to v3.8.5 — changelog, docs, version sync * chore(release): translate Hall of Contributors to English in workflows and changelog * fix(combos): make target timeout configurable (diegosouzapw#2775) Merge PR diegosouzapw#2775 — fix(combos): make target timeout configurable * feat: fix so restart of server restarts batch jobs instead of failing them (diegosouzapw#2755) Merge PR diegosouzapw#2755 — feat: fix so restart of server restarts batch jobs instead of failing them * chore(release): update changelog with merged PRs notes and credits * feat(api): add endpoint restrictions for client API keys (diegosouzapw#2777) Merge PR diegosouzapw#2777 — feat(api): add endpoint restrictions for client API keys * chore(release): update changelog with PR diegosouzapw#2777 entry and contributor credit --------- Co-authored-by: Thanet S. <cho.112543@gmail.com> Co-authored-by: Randi <55005611+rdself@users.noreply.github.com> Co-authored-by: Markus Hartung <mail@hartmark.se> Co-authored-by: Jack <5443152+hijak@users.noreply.github.com>
…w#2777) Merge PR diegosouzapw#2777 — feat(api): add endpoint restrictions for client API keys
…contributor credit
Summary
allowedEndpointsfield to API keys that restricts which endpoint categories a key can accessenforceApiKeyPolicy()— no individual route handler changes neededChanges
src/shared/constants/endpointCategories.ts(new)resolveEndpointCategory()resolversrc/lib/db/apiKeys.tsallowed_endpointsTEXT column, parse/persist in metadatasrc/shared/utils/apiKeyPolicy.tssrc/shared/validation/schemas.tsallowedEndpointsApiManagerPageClient.tsxsrc/i18n/messages/en.jsontests/unit/endpoint-categories.test.ts(new)tests/unit/endpoint-restrictions-policy.test.ts(new)Test plan
resolveEndpointCategory()passnpm run typecheck:core— no new errorsnpx eslinton changed files — 0 errorsallowedEndpoints: ["search"], verify/v1/searchworks but/v1/chat/completionsreturns 403allowedEndpointsstill work on all endpoints