-
-
Notifications
You must be signed in to change notification settings - Fork 0
fix: enforce route-scoped auth for realtime RPC #123
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -548,9 +548,11 @@ const createRouteServerAccessEntries = async ( | |
| routes: Awaited<ReturnType<typeof createRoutes>>, | ||
| actions: ReadonlyArray<{ filePath: string; id: string }>, | ||
| loaders: ReadonlyArray<{ filePath: string; id: string }>, | ||
| realtimes: ReadonlyArray<{ filePath: string; id: string }>, | ||
| ) => { | ||
| const actionIdsByFilePath = toIdsByFilePath(actions) | ||
| const loaderIdsByFilePath = toIdsByFilePath(loaders) | ||
| const realtimeIdsByFilePath = toIdsByFilePath(realtimes) | ||
|
|
||
| return await Promise.all( | ||
| routes.map(async (route) => { | ||
|
|
@@ -560,6 +562,7 @@ const createRouteServerAccessEntries = async ( | |
| return { | ||
| actionIds: reachableFiles.flatMap((filePath) => actionIdsByFilePath.get(filePath) ?? []), | ||
| loaderIds: reachableFiles.flatMap((filePath) => loaderIdsByFilePath.get(filePath) ?? []), | ||
| realtimeIds: reachableFiles.flatMap((filePath) => realtimeIdsByFilePath.get(filePath) ?? []), | ||
| } | ||
| }), | ||
| ) | ||
|
|
@@ -606,7 +609,7 @@ const renderAppModule = ( | |
| loaders: Array<{ filePath: string; id: string }>, | ||
| realtimes: Array<{ filePath: string; id: string }>, | ||
| routes: Awaited<ReturnType<typeof createRoutes>>, | ||
| routeServerAccessEntries: Array<{ actionIds: string[]; loaderIds: string[] }>, | ||
| routeServerAccessEntries: Array<{ actionIds: string[]; loaderIds: string[]; realtimeIds: string[] }>, | ||
| routeManifest: RouteManifest, | ||
| routeDataEndpoint: boolean, | ||
| serverHooksUrl: string | null, | ||
|
|
@@ -666,6 +669,7 @@ const ROUTE_PARAMS_PROP = "__eclipsa_route_params"; | |
| const ROUTE_ERROR_PROP = "__eclipsa_route_error"; | ||
| const ROUTE_DATA_REQUEST_HEADER = ${JSON.stringify(ROUTE_DATA_REQUEST_HEADER)}; | ||
| const ROUTE_PREFLIGHT_REQUEST_HEADER = "x-eclipsa-route-preflight"; | ||
| const realtimeRouteMatches = new WeakMap(); | ||
| const CHUNK_CACHE_MESSAGE_TYPE = "eclipsa:chunk-cache-precache"; | ||
| const hooksPromise = (async () => { | ||
| const appHooks = appHooksServerUrl ? await import(appHooksServerUrl) : {}; | ||
|
|
@@ -932,7 +936,7 @@ const reroutePathname = (hooks, request, pathname, baseUrl) => | |
| const getRouteServerAccess = (route) => { | ||
| const routeIndex = routes.indexOf(route); | ||
| const entry = routeIndex >= 0 ? routeServerAccessEntries[routeIndex] : null; | ||
| return entry ?? { actionIds: [], loaderIds: [] }; | ||
| return entry ?? { actionIds: [], loaderIds: [], realtimeIds: [] }; | ||
| }; | ||
|
|
||
| const resolveRouteForCurrentUrl = (hooks, request, currentUrl) => { | ||
|
|
@@ -1577,20 +1581,41 @@ if (realtimeWebSocket?.upgradeWebSocket) { | |
| if (!id) { | ||
| return c.text("Not Found", 404); | ||
| } | ||
| const { appHooks } = await hooksPromise; | ||
| const routeMatch = getRpcCurrentRoute(appHooks, c); | ||
| if (!routeMatch) { | ||
| return c.text("Bad Request", 400); | ||
| } | ||
| const routeAccess = getRouteServerAccess(routeMatch.route); | ||
| if (!routeAccess.realtimeIds.includes(id)) { | ||
| return c.text("Not Found", 404); | ||
| } | ||
| const moduleUrl = realtimes[id]; | ||
| if (!moduleUrl) { | ||
| return c.text("Not Found", 404); | ||
| } | ||
| if (!hasRealtime(id)) { | ||
| await import(moduleUrl); | ||
| } | ||
| realtimeRouteMatches.set(c.req.raw, routeMatch); | ||
| await next(); | ||
| }, | ||
| createRealtimeHonoUpgradeHandler(realtimeWebSocket.upgradeWebSocket, async (c, socket) => { | ||
| await resolveRequest(c, async (requestContext) => { | ||
| await resolveRequest(c, async (requestContext, appHooks) => { | ||
| const id = requestContext.req.param("id"); | ||
| await executeRealtime(id, requestContext, socket); | ||
| return requestContext.body(null, 204); | ||
| const routeMatch = realtimeRouteMatches.get(requestContext.req.raw) ?? getRpcCurrentRoute(appHooks, requestContext); | ||
| if (!routeMatch) { | ||
| return requestContext.text("Bad Request", 400); | ||
| } | ||
| return composeRouteMiddlewares( | ||
| routeMatch.route, | ||
| requestContext, | ||
| routeMatch.params, | ||
| async () => { | ||
| await executeRealtime(id, requestContext, socket); | ||
| return requestContext.body(null, 204); | ||
| }, | ||
| ); | ||
|
||
| }); | ||
| }), | ||
| ); | ||
|
|
@@ -1858,7 +1883,7 @@ export const build = async ( | |
| const loaders = await collectAppLoaders(root) | ||
| const realtimes = await collectAppRealtimes(root) | ||
| const routes = await createRoutes(root) | ||
| const routeServerAccessEntries = await createRouteServerAccessEntries(routes, actions, loaders) | ||
| const routeServerAccessEntries = await createRouteServerAccessEntries(routes, actions, loaders, realtimes) | ||
| const staticPageRoutes = routes.filter( | ||
| (route) => route.page && resolveRouteRenderMode(route, options.output) === 'static', | ||
| ) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -160,6 +160,7 @@ interface RouteDataResponse { | |
| interface RouteServerAccessEntry { | ||
| actionIds: Set<string> | ||
| loaderIds: Set<string> | ||
| realtimeIds: Set<string> | ||
| route: RouteEntry | ||
| } | ||
|
|
||
|
|
@@ -246,9 +247,11 @@ const createRouteServerAccessEntries = async ( | |
| routes: readonly RouteEntry[], | ||
| actions: ReadonlyArray<{ filePath: string; id: string }>, | ||
| loaders: ReadonlyArray<{ filePath: string; id: string }>, | ||
| realtimes: ReadonlyArray<{ filePath: string; id: string }>, | ||
| ) => { | ||
| const actionIdsByFilePath = toIdsByFilePath(actions) | ||
| const loaderIdsByFilePath = toIdsByFilePath(loaders) | ||
| const realtimeIdsByFilePath = toIdsByFilePath(realtimes) | ||
|
|
||
| return await Promise.all( | ||
| routes.map(async (route) => { | ||
|
|
@@ -262,6 +265,9 @@ const createRouteServerAccessEntries = async ( | |
| loaderIds: new Set( | ||
| reachableFiles.flatMap((filePath) => loaderIdsByFilePath.get(filePath) ?? []), | ||
| ), | ||
| realtimeIds: new Set( | ||
| reachableFiles.flatMap((filePath) => realtimeIdsByFilePath.get(filePath) ?? []), | ||
| ), | ||
| route, | ||
| } satisfies RouteServerAccessEntry | ||
| }), | ||
|
|
@@ -524,7 +530,12 @@ const createDevApp = async (init: DevAppInit) => { | |
| const actionModules = new Map(actions.map((action) => [action.id, action.filePath])) | ||
| const loaderModules = new Map(loaders.map((loader) => [loader.id, loader.filePath])) | ||
| const realtimeModules = new Map(realtimes.map((realtime) => [realtime.id, realtime.filePath])) | ||
| const routeServerAccessEntries = await createRouteServerAccessEntries(routes, actions, loaders) | ||
| const routeServerAccessEntries = await createRouteServerAccessEntries( | ||
| routes, | ||
| actions, | ||
| loaders, | ||
| realtimes, | ||
| ) | ||
| const routeServerAccessByRoute = new Map( | ||
| routeServerAccessEntries.map((entry) => [entry.route, entry] as const), | ||
| ) | ||
|
|
@@ -569,6 +580,7 @@ const createDevApp = async (init: DevAppInit) => { | |
| routeServerAccessByRoute.get(route) ?? { | ||
| actionIds: new Set<string>(), | ||
| loaderIds: new Set<string>(), | ||
| realtimeIds: new Set<string>(), | ||
| route, | ||
| } | ||
|
|
||
|
|
@@ -606,6 +618,10 @@ const createDevApp = async (init: DevAppInit) => { | |
| } | ||
| return resolveRouteForCurrentUrl(requestContext.req.raw, currentUrl) | ||
| } | ||
| const realtimeRouteMatches = new WeakMap< | ||
| Request, | ||
| ReturnType<typeof resolveRouteForCurrentUrl> | ||
| >() | ||
|
|
||
| const resolveRequest = async <E extends Context>( | ||
| c: E, | ||
|
|
@@ -1203,6 +1219,14 @@ const createDevApp = async (init: DevAppInit) => { | |
| if (!id) { | ||
| return c.text('Not Found', 404) | ||
| } | ||
| const routeMatch = getRpcCurrentRoute(c as unknown as AppContext) | ||
| if (!routeMatch) { | ||
| return c.text('Bad Request', 400) | ||
| } | ||
| const routeAccess = getRouteServerAccess(routeMatch.route) | ||
| if (!routeAccess.realtimeIds.has(id)) { | ||
| return c.text('Not Found', 404) | ||
| } | ||
| const modulePath = realtimeModules.get(id) | ||
| if (!modulePath) { | ||
| return c.text('Not Found', 404) | ||
|
|
@@ -1211,14 +1235,27 @@ const createDevApp = async (init: DevAppInit) => { | |
| if (!hasRealtime(id)) { | ||
| await init.runner.import(modulePath) | ||
| } | ||
| realtimeRouteMatches.set(c.req.raw, routeMatch) | ||
| await next() | ||
| }, | ||
| createRealtimeHonoUpgradeHandler(realtimeWebSocket.upgradeWebSocket, async (c, socket) => { | ||
| await resolveRequest(c, async (requestContext) => { | ||
| const { executeRealtime } = await init.runner.import('eclipsa') | ||
| const id = requestContext.req.param('id') | ||
| await executeRealtime(id, requestContext, socket) | ||
| return requestContext.body(null, 204) | ||
| const routeMatch = | ||
| realtimeRouteMatches.get(requestContext.req.raw) ?? getRpcCurrentRoute(requestContext) | ||
| if (!routeMatch) { | ||
| return requestContext.text('Bad Request', 400) | ||
| } | ||
| return composeRouteMiddlewares( | ||
| routeMatch.route, | ||
| requestContext, | ||
| routeMatch.params, | ||
| async () => { | ||
| await executeRealtime(id, requestContext, socket) | ||
| return requestContext.body(null, 204) | ||
| }, | ||
| ) as Promise<Response> | ||
|
||
| }) | ||
| }), | ||
| ) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This new pre-upgrade check makes realtime upgrades fail whenever
getRpcCurrentRoute(...)is null, but browser clients cannot providex-eclipsa-route-urlduringnew WebSocket(...)connects (the runtime client inpackages/eclipsa/core/realtime.tsopens a socket URL only). In practice, normalrealtime().connect()calls will now hit400 Bad Requestfor production (and the same logic exists in dev), so this change breaks legitimate browser realtime usage instead of only blocking cross-route ID probing.Useful? React with 👍 / 👎.