mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-25 16:42:16 +03:00
fix(authz): match exact public routes exactly, not as prefixes (#11417)
`isPublicApiRoute()` matched every entry of PUBLIC_API_ROUTE_PREFIXES with `startsWith()`, but 11 of the 15 entries name ONE route, not a subtree. As a prefix each also marked every adjacent path sharing its leading characters as PUBLIC, which skips the MANAGEMENT auth gate. That is reachable today: Next resolves `/api/usage/om-usage<anything>` to the dynamic route `/api/usage/[connectionId]`, and that handler carries no auth of its own — it relies entirely on being classified MANAGEMENT. An unauthenticated caller therefore reaches `fetchAndPersistProviderLimits()`, which is an existence oracle over connection ids (409/404/400/200) and, for a connection id actually starting with `om-usage`, discloses live quota JSON and can drive an OAuth token refresh (a write side effect) with no credentials. Split the allowlist by shape: - PUBLIC_API_ROUTE_PREFIXES keeps only genuine subtrees, every entry ending in "/" (asserted by a unit test, so the class cannot come back silently). - PUBLIC_API_ROUTES_EXACT holds the single routes, matched exactly in both spellings. - The three read-only "prefixes" were single routes too and move to PUBLIC_READONLY_CORS_API_ROUTES, matched exactly. classify.ts now asks `isPublicReadonlyCorsRoute()` instead of scanning the raw list, so the CORS origin relaxation pipeline.ts keys on cannot be inherited by a sibling either (`/api/monitoring/health-detail` was taking it). - `/api/health` deliberately stays in its own set so it keeps classifying as `public_prefix`; folding it into the read-only set would widen CORS on it. dashboardCsrf.ts had a second copy of the prefix scan; it now shares `isPublicApiRoute()` so the client CSRF exemption and the server classification cannot disagree. Side effect in the safe direction: the three LOCAL_ONLY oauth auto-import routes were CSRF-exempt on the client while the server already required the token — the client now attaches it. Reported by @ntdat812 (GHSA-74g9-q8f6-793h), with the shape of the fix and the two gotchas above called out in the report. Closes GHSA-74g9-q8f6-793h Co-authored-by: Xiangzhe <bakryun0718@proton.me> Co-authored-by: Nguyen Thanh Dat <ntdat812.dev@gmail.com>
This commit is contained in:
committed by
GitHub
parent
56d64e29a4
commit
bbc7bf4351
@@ -108,24 +108,48 @@ A successful policy returns `AuthSubject` with `kind ∈ { client_api_key, dashb
|
||||
|
||||
`src/shared/constants/publicApiRoutes.ts` is the explicit allowlist:
|
||||
|
||||
The list is split by **shape**, and the split is load-bearing (GHSA-74g9-q8f6-793h): a prefix is
|
||||
matched with `startsWith()`, so it also matches every adjacent path sharing its leading characters.
|
||||
`/api/usage/om-usage` as a prefix marked `/api/usage/om-usage<anything>` PUBLIC, and Next resolves
|
||||
that to `/api/usage/[connectionId]` — a handler with no auth of its own.
|
||||
|
||||
```ts
|
||||
// Genuine subtrees. Every entry MUST end in "/" (asserted by a unit test).
|
||||
PUBLIC_API_ROUTE_PREFIXES = [
|
||||
"/api/auth/oidc/",
|
||||
"/api/v1/", // treated as CLIENT_API in classify, not as "no-auth public"
|
||||
"/api/oauth/",
|
||||
"/api/codex/connect/",
|
||||
"/api/telegram/",
|
||||
"/api/cursor-cli/",
|
||||
];
|
||||
|
||||
// Single routes, matched EXACTLY (with or without a trailing slash).
|
||||
PUBLIC_API_ROUTES_EXACT = new Set([
|
||||
"/api/auth/login",
|
||||
"/api/auth/logout",
|
||||
"/api/auth/status",
|
||||
"/api/init",
|
||||
"/api/v1/", // treated as CLIENT_API in classify, not as "no-auth public"
|
||||
"/api/cloud/",
|
||||
"/api/sync/bundle",
|
||||
"/api/oauth/",
|
||||
"/api/cli/connect",
|
||||
"/api/usage/om-usage",
|
||||
"/api/skills/collect/chaos",
|
||||
]);
|
||||
|
||||
// Read-only single routes that also take the CORS origin relaxation.
|
||||
PUBLIC_READONLY_CORS_API_ROUTES = [
|
||||
"/api/health/ping",
|
||||
"/api/monitoring/health",
|
||||
"/api/settings/require-login",
|
||||
];
|
||||
|
||||
PUBLIC_READONLY_API_ROUTE_PREFIXES = ["/api/monitoring/health", "/api/settings/require-login"];
|
||||
// Read-only single route WITHOUT the CORS relaxation.
|
||||
PUBLIC_READONLY_API_ROUTES_EXACT = new Set(["/api/health"]);
|
||||
|
||||
PUBLIC_READONLY_METHODS = new Set(["GET", "HEAD", "OPTIONS"]);
|
||||
```
|
||||
|
||||
Read-only prefixes are public **only** for safe methods. Note: `classifyRoute()` excludes `/api/v1/*` and `/api/v1beta/*` from the PUBLIC fall-through — those are always `CLIENT_API` so the Bearer-key policy still applies.
|
||||
Read-only routes are public **only** for safe methods. Note: `classifyRoute()` excludes `/api/v1/*` and `/api/v1beta/*` from the PUBLIC fall-through — those are always `CLIENT_API` so the Bearer-key policy still applies.
|
||||
|
||||
## Adding a New Route
|
||||
|
||||
@@ -168,7 +192,7 @@ export async function POST(request: Request) {
|
||||
|
||||
### Pattern 3 — Adding to the public allowlist
|
||||
|
||||
Add the prefix to `PUBLIC_API_ROUTE_PREFIXES` (or `PUBLIC_READONLY_API_ROUTE_PREFIXES` for GET-only). Update unit tests at `tests/unit/public-api-routes.test.ts` and `tests/unit/authz/classify.test.ts`.
|
||||
Pick the set by shape, not by convenience. One route goes in `PUBLIC_API_ROUTES_EXACT` (or `PUBLIC_READONLY_CORS_API_ROUTES` for GET-only); only a genuine subtree goes in `PUBLIC_API_ROUTE_PREFIXES`, and it **must end in `/`**. Putting a single route in the prefix list also publishes every adjacent path that shares its leading characters — including dynamic-segment siblings added later (GHSA-74g9-q8f6-793h). Update unit tests at `tests/unit/public-api-routes.test.ts`, `tests/unit/authz/public-route-exact-match.test.ts` and `tests/unit/authz/classify.test.ts`.
|
||||
|
||||
## Scopes
|
||||
|
||||
|
||||
Reference in New Issue
Block a user