-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add /_/* redirect route #4523
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
Merged
+455
−0
Merged
Add /_/* redirect route #4523
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
d2efe29
Add /deeplink/* redirect route
claude e326919
Harden the deeplink suffix against traversal and delimiter injection
claude a1fa385
Derive deeplink allowlist from the environment route segments
claude cf24f03
Add a test that the deeplink allowlist matches the environment routes
claude c72f501
Send deeplinks whose own segment has no page to a page that exists
claude 90577d2
Give deeplink targets a separate landing path and deep prefix
claude 1660450
Read the deeplink suffix from the pathname, and honour pending invites
claude 71c6d3c
Match a deeplink's prefix and page name the way the router does
claude 6b2e0f9
Compare a written-out deeplink prefix with its case folded too
claude f93a892
Serve deeplinks from /_ instead of the deeplink literal
claude 3de75c1
Assert the deeplink route's path from Remix's own route manifest
claude f505054
Cut the deeplink route's comments back to what the code can't say itself
claude File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| area: webapp | ||
| type: feature | ||
| --- | ||
|
|
||
| Short links like /_/apikeys now take you straight to that page in your current project and environment, so you no longer need the full URL with your org, project and environment in it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,55 @@ | ||
| import { redirect, type LoaderFunctionArgs } from "@remix-run/server-runtime"; | ||
| import { prisma } from "~/db.server"; | ||
| import { getUsersInvites } from "~/models/member.server"; | ||
| import { SelectBestEnvironmentPresenter } from "~/presenters/SelectBestEnvironmentPresenter.server"; | ||
| import { requireUser } from "~/services/session.server"; | ||
| import { deeplinkSuffix, resolveDeeplinkPage } from "~/utils/deeplinkPages"; | ||
| import { | ||
| invitesPath, | ||
| newOrganizationPath, | ||
| newProjectPath, | ||
| v3EnvironmentPath, | ||
| } from "~/utils/pathBuilder"; | ||
|
|
||
| //`[_]` escapes the underscore: an unescaped `_.$` is a pathless layout, mounted at `/*`. | ||
| export const loader = async ({ request }: LoaderFunctionArgs) => { | ||
| const user = await requireUser(request); | ||
|
|
||
| const { pathname, search } = new URL(request.url); | ||
| const page = resolveDeeplinkPage(deeplinkSuffix(pathname)); | ||
|
|
||
| const invites = await getUsersInvites({ email: user.email }); | ||
| if (invites.length > 0) { | ||
| return redirect(invitesPath()); | ||
| } | ||
|
|
||
| const presenter = new SelectBestEnvironmentPresenter(); | ||
| try { | ||
| const { project, organization, environment } = await presenter.call({ user }); | ||
| const environmentPath = v3EnvironmentPath(organization, project, environment); | ||
|
|
||
| const suffix = page ? `/${page}` : ""; | ||
|
|
||
| return redirect(`${environmentPath}${suffix}${search}`); | ||
| } catch (_e) { | ||
| const organization = await prisma.organization.findFirst({ | ||
| where: { | ||
| members: { | ||
| some: { | ||
| userId: user.id, | ||
| }, | ||
| }, | ||
| deletedAt: null, | ||
| }, | ||
| orderBy: { | ||
| createdAt: "desc", | ||
| }, | ||
| }); | ||
|
|
||
| if (organization) { | ||
| return redirect(newProjectPath(organization)); | ||
| } | ||
|
|
||
| return redirect(newOrganizationPath()); | ||
| } | ||
| }; | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,317 @@ | ||
| import { flatRoutes } from "@remix-run/dev/dist/config/flat-routes.js"; | ||
| import type { RouteManifest } from "@remix-run/dev/dist/config/routes.js"; | ||
| import { matchPath } from "@remix-run/router"; | ||
| import { existsSync, readdirSync, statSync } from "node:fs"; | ||
| import { join } from "node:path"; | ||
| import { describe, expect, it } from "vitest"; | ||
| import { | ||
| DEEPLINK_PATH_PREFIX, | ||
| deeplinkSuffix, | ||
| ENV_PAGE_TARGETS, | ||
| resolveDeeplinkPage, | ||
| } from "./deeplinkPages"; | ||
|
|
||
| const APP_DIR = join(__dirname, ".."); | ||
| const ROUTES_DIR = join(APP_DIR, "routes"); | ||
|
|
||
| // The trailing dot excludes the environment layout route itself, which has no segment of its own. | ||
| const ENV_ROUTE_PREFIX = "_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam."; | ||
|
|
||
| // Route files that name no deeplink: the environment root, and Remix's layout-opt-out spelling. | ||
| const NOT_DEEPLINK_NAMES = new Set(["_index", "queues_"]); | ||
|
|
||
| const PROBE = "probe_01ABC"; | ||
|
|
||
| const DEEPLINK_ROUTE_FILE = "routes/[_].$.ts"; | ||
|
|
||
| const compiledRoutes: RouteManifest = flatRoutes(APP_DIR, ["**/.*"]); | ||
|
|
||
| function compiledUrl(id: string): string { | ||
| if (!compiledRoutes[id]) throw new Error(`no compiled route with id ${id}`); | ||
|
|
||
| const segments: string[] = []; | ||
| let route = compiledRoutes[id]; | ||
| while (route) { | ||
| if (route.path) segments.unshift(route.path); | ||
| route = route.parentId ? compiledRoutes[route.parentId] : undefined; | ||
| } | ||
| return `/${segments.join("/")}`; | ||
| } | ||
|
|
||
| const COMPILED_DEEPLINK_PATH = (() => { | ||
| const entry = Object.values(compiledRoutes).find((route) => route.file === DEEPLINK_ROUTE_FILE); | ||
| if (!entry) throw new Error(`${DEEPLINK_ROUTE_FILE} is not in the compiled route manifest`); | ||
| return compiledUrl(entry.id).replace(/\/\*$/, ""); | ||
| })(); | ||
|
|
||
| const routeEntries = readdirSync(ROUTES_DIR); | ||
|
|
||
| function isRouteModule(entry: string): boolean { | ||
| const path = join(ROUTES_DIR, entry); | ||
| if (!statSync(path).isDirectory()) return true; | ||
| return existsSync(join(path, "route.tsx")) || existsSync(join(path, "route.ts")); | ||
| } | ||
|
|
||
| // A trailing `_` only opts out of the parent layout: `queues_.$queueParam` serves `/queues/{id}`. | ||
| const envRoutes: string[][] = routeEntries | ||
| .filter((entry) => entry.startsWith(ENV_ROUTE_PREFIX) && isRouteModule(entry)) | ||
| .map((entry) => | ||
| entry | ||
| .slice(ENV_ROUTE_PREFIX.length) | ||
| .replace(/\.(tsx|ts)$/, "") | ||
| .split(".") | ||
| ) | ||
| .map((segments) => (segments.at(-1) === "_index" ? segments.slice(0, -1) : segments)) | ||
| .map((segments) => segments.map((segment) => segment.replace(/_+$/, ""))); | ||
|
|
||
| function routeMatches(path: string, { allowParams }: { allowParams: boolean }): boolean { | ||
| const wanted = path === "" ? [] : path.split("/"); | ||
| return envRoutes.some( | ||
| (route) => | ||
| route.length === wanted.length && | ||
| route.every((segment, i) => (segment.startsWith("$") ? allowParams : segment === wanted[i])) | ||
| ); | ||
| } | ||
|
|
||
| function envRouteSegments(): Set<string> { | ||
| const segments = new Set<string>(); | ||
| for (const entry of routeEntries) { | ||
| if (!entry.startsWith(ENV_ROUTE_PREFIX)) continue; | ||
| const segment = entry.slice(ENV_ROUTE_PREFIX.length).split(/[./]/)[0]; | ||
| if (!segment || segment === "ts" || segment === "tsx") continue; | ||
| segments.add(segment); | ||
| } | ||
| return segments; | ||
| } | ||
|
|
||
| function descendantsOf(prefix: string): string[][] { | ||
| const depth = prefix === "" ? 0 : prefix.split("/").length; | ||
| return envRoutes | ||
| .filter((route) => route.length > depth && route.slice(0, depth).join("/") === prefix) | ||
| .map((route) => | ||
| route.slice(depth).map((segment) => (segment.startsWith("$") ? PROBE : segment)) | ||
| ); | ||
| } | ||
|
|
||
| describe("deeplink targets", () => { | ||
| it("read enough routes for the assertions below to mean anything", () => { | ||
| expect(envRouteSegments().size).toBeGreaterThan(20); | ||
| expect(envRoutes.length).toBeGreaterThan(40); | ||
| }); | ||
|
|
||
| it("every bare name lands on a real page that needs no id", () => { | ||
| const broken = [...ENV_PAGE_TARGETS.entries()] | ||
| .filter(([name]) => !routeMatches(resolveDeeplinkPage(name) ?? " ", { allowParams: false })) | ||
| .map(([name, { landing }]) => `${name} -> ${landing || "(environment root)"}`); | ||
|
|
||
| expect(broken).toEqual([]); | ||
| }); | ||
|
|
||
| it("every deep path lands on a real route, prefix graft included", () => { | ||
| const broken: string[] = []; | ||
|
|
||
| for (const [name, { prefix }] of ENV_PAGE_TARGETS) { | ||
| for (const rest of descendantsOf(prefix)) { | ||
| const suffix = [name, ...rest].join("/"); | ||
| const resolved = resolveDeeplinkPage(suffix); | ||
| if (!routeMatches(resolved ?? " ", { allowParams: true })) { | ||
| broken.push(`${suffix} -> ${resolved}`); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| expect(broken).toEqual([]); | ||
| }); | ||
|
|
||
| it("has deep paths worth checking", () => { | ||
| expect(descendantsOf("waitpoints/tokens").length).toBeGreaterThan(0); | ||
| expect(descendantsOf("tasks").length).toBeGreaterThan(2); | ||
| expect(descendantsOf("runs").length).toBeGreaterThan(0); | ||
| }); | ||
|
|
||
| it("every environment page has a deeplink name", () => { | ||
| const missing = [...envRouteSegments()] | ||
| .filter((segment) => !NOT_DEEPLINK_NAMES.has(segment)) | ||
| .filter( | ||
| (segment) => routeMatches(segment, { allowParams: false }) && !ENV_PAGE_TARGETS.has(segment) | ||
| ) | ||
| .sort(); | ||
|
|
||
| expect(missing).toEqual([]); | ||
| }); | ||
|
|
||
| it("points a 404ing name elsewhere, and gives a redirect shim no name at all", () => { | ||
| for (const segment of ["tasks", "waitpoints", "metrics"]) { | ||
| expect(routeMatches(segment, { allowParams: false })).toBe(false); | ||
| } | ||
|
|
||
| expect(ENV_PAGE_TARGETS.get("tasks")).toEqual({ landing: "", prefix: "tasks" }); | ||
| expect(ENV_PAGE_TARGETS.get("waitpoints")).toEqual({ | ||
| landing: "waitpoints/tokens", | ||
| prefix: "waitpoints/tokens", | ||
| }); | ||
| expect(ENV_PAGE_TARGETS.has("metrics")).toBe(false); | ||
| }); | ||
| }); | ||
|
|
||
| describe("resolveDeeplinkPage", () => { | ||
| it("maps a bare name to its landing page", () => { | ||
| expect(resolveDeeplinkPage("apikeys")).toBe("apikeys"); | ||
| expect(resolveDeeplinkPage("waitpoints")).toBe("waitpoints/tokens"); | ||
| expect(resolveDeeplinkPage("tasks")).toBe(""); | ||
| }); | ||
|
|
||
| it("grafts deeper segments onto the prefix", () => { | ||
| expect(resolveDeeplinkPage("runs/run_123")).toBe("runs/run_123"); | ||
| expect(resolveDeeplinkPage("tasks/standard/my-task")).toBe("tasks/standard/my-task"); | ||
| expect(resolveDeeplinkPage("waitpoints/waitpoint_123")).toBe("waitpoints/tokens/waitpoint_123"); | ||
| }); | ||
|
|
||
| it("does not duplicate a prefix the caller already wrote out", () => { | ||
| expect(resolveDeeplinkPage("waitpoints/tokens")).toBe("waitpoints/tokens"); | ||
| expect(resolveDeeplinkPage("waitpoints/tokens/waitpoint_123")).toBe( | ||
| "waitpoints/tokens/waitpoint_123" | ||
| ); | ||
| }); | ||
|
|
||
| it("rejects a name that is not a page", () => { | ||
| expect(resolveDeeplinkPage("")).toBeUndefined(); | ||
| expect(resolveDeeplinkPage("nonsense")).toBeUndefined(); | ||
| expect(resolveDeeplinkPage("metrics")).toBeUndefined(); | ||
| }); | ||
|
|
||
| it("matches the page name whatever its case, and resolves it to the map's spelling", () => { | ||
| expect(resolveDeeplinkPage("APIKeys")).toBe("apikeys"); | ||
| expect(resolveDeeplinkPage("Waitpoints")).toBe("waitpoints/tokens"); | ||
| expect(resolveDeeplinkPage("TASKS")).toBe(""); | ||
| expect(resolveDeeplinkPage("Bulk-Actions")).toBe("bulk-actions"); | ||
| expect(resolveDeeplinkPage("Nonsense")).toBeUndefined(); | ||
| expect(resolveDeeplinkPage("Metrics")).toBeUndefined(); | ||
| }); | ||
|
|
||
| it("leaves the case of everything after the name alone", () => { | ||
| expect(resolveDeeplinkPage("runs/run_ABC123")).toBe("runs/run_ABC123"); | ||
| expect(resolveDeeplinkPage("Runs/run_ABC123")).toBe("runs/run_ABC123"); | ||
| expect(resolveDeeplinkPage("TASKS/standard/My-Task")).toBe("tasks/standard/My-Task"); | ||
| expect(resolveDeeplinkPage("Waitpoints/waitpoint_ABC")).toBe("waitpoints/tokens/waitpoint_ABC"); | ||
| expect(resolveDeeplinkPage("Waitpoints/tokens/waitpoint_ABC")).toBe( | ||
| "waitpoints/tokens/waitpoint_ABC" | ||
| ); | ||
| expect(resolveDeeplinkPage("Tasks/standard/Group%2FMy-Task")).toBe( | ||
| "tasks/standard/Group%2FMy-Task" | ||
| ); | ||
| }); | ||
|
|
||
| it("recognises a written-out prefix whatever its case, however many segments it spans", () => { | ||
| expect(resolveDeeplinkPage("Waitpoints/Tokens/wp_123")).toBe("waitpoints/tokens/wp_123"); | ||
| expect(resolveDeeplinkPage("waitpoints/Tokens/wp_123")).toBe("waitpoints/tokens/wp_123"); | ||
| expect(resolveDeeplinkPage("WAITPOINTS/TOKENS/wp_123")).toBe("waitpoints/tokens/wp_123"); | ||
| expect(resolveDeeplinkPage("Waitpoints/Tokens")).toBe("waitpoints/tokens"); | ||
| }); | ||
|
|
||
| it("holds for every multi-segment prefix in the map, not just waitpoints", () => { | ||
| const multiSegment = [...ENV_PAGE_TARGETS.values()].filter(({ prefix }) => | ||
| prefix.includes("/") | ||
| ); | ||
|
|
||
| expect(multiSegment.length).toBeGreaterThan(0); | ||
|
|
||
| for (const { prefix } of multiSegment) { | ||
| const shouted = prefix | ||
| .split("/") | ||
| .map((segment) => segment.toUpperCase()) | ||
| .join("/"); | ||
| expect(resolveDeeplinkPage(`${shouted}/${PROBE}`)).toBe(`${prefix}/${PROBE}`); | ||
| expect(resolveDeeplinkPage(shouted)).toBe(prefix); | ||
| } | ||
| }); | ||
|
|
||
| it("drops traversal segments, in plain and escaped spellings", () => { | ||
| expect(resolveDeeplinkPage("runs/../../../etc/passwd")).toBe("runs/etc/passwd"); | ||
| expect(resolveDeeplinkPage("../runs")).toBe("runs"); | ||
| expect(resolveDeeplinkPage("runs//run_1")).toBe("runs/run_1"); | ||
| expect(resolveDeeplinkPage("runs/%2e%2e/%2E%2E/run_1")).toBe("runs/run_1"); | ||
| expect(resolveDeeplinkPage("runs/%2e/run_1")).toBe("runs/run_1"); | ||
| expect(resolveDeeplinkPage("runs/%ZZ/run_1")).toBe("runs/run_1"); | ||
| }); | ||
|
|
||
| it("passes encoded segments through without re-encoding them", () => { | ||
| expect(resolveDeeplinkPage("tasks/standard/group%2Fmy-task")).toBe( | ||
| "tasks/standard/group%2Fmy-task" | ||
| ); | ||
| expect(resolveDeeplinkPage("runs/a%3Fb%23c")).toBe("runs/a%3Fb%23c"); | ||
| // The slash stays escaped, so this addresses one odd id rather than climbing out. | ||
| expect(resolveDeeplinkPage("runs/..%2f..%2fetc")).toBe("runs/..%2f..%2fetc"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("the route Remix compiles from the filename", () => { | ||
| it("mounts the deeplink route at /_ and nowhere else", () => { | ||
| expect(COMPILED_DEEPLINK_PATH).toBe("/_"); | ||
| expect(COMPILED_DEEPLINK_PATH).toBe(DEEPLINK_PATH_PREFIX); | ||
| }); | ||
|
|
||
| it("does not mount anything as a site-wide splat", () => { | ||
| const siteWide = Object.values(compiledRoutes) | ||
| .filter((route) => compiledUrl(route.id) === "/*") | ||
| .map((route) => route.file); | ||
|
|
||
| expect(siteWide).toEqual([]); | ||
| }); | ||
|
|
||
| it("compiled the manifest it is reading, paths and all", () => { | ||
| expect(Object.keys(compiledRoutes).length).toBeGreaterThan(400); | ||
| expect(compiledUrl("routes/login.magic")).toBe("/login/magic"); | ||
| expect( | ||
| compiledUrl( | ||
| "routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues_.$queueParam" | ||
| ) | ||
| ).toBe("/orgs/:organizationSlug/projects/:projectParam/env/:envParam/queues/:queueParam"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("deeplinkSuffix", () => { | ||
| it("strips the route's own prefix", () => { | ||
| expect(deeplinkSuffix("/_/tasks")).toBe("tasks"); | ||
| expect(deeplinkSuffix("/_/runs/run_123")).toBe("runs/run_123"); | ||
| }); | ||
|
|
||
| it("keeps an escaped slash intact, unlike the decoded splat param", () => { | ||
| expect(deeplinkSuffix("/_/tasks/standard/group%2Fmy-task")).toBe( | ||
| "tasks/standard/group%2Fmy-task" | ||
| ); | ||
| }); | ||
|
|
||
| it("strips only the prefix, leaving the remainder's case alone", () => { | ||
| expect(deeplinkSuffix("/_/runs/run_ABC123")).toBe("runs/run_ABC123"); | ||
| expect(deeplinkSuffix("/_/tasks/standard/My-Task")).toBe("tasks/standard/My-Task"); | ||
| }); | ||
|
|
||
| it("matches the URL the router serves for it, splat case and all", () => { | ||
| const route = `${COMPILED_DEEPLINK_PATH}/*`; | ||
| expect(matchPath(route, "/_/apikeys")?.params["*"]).toBe("apikeys"); | ||
| expect(matchPath(route, "/_/runs/run_123")?.params["*"]).toBe("runs/run_123"); | ||
| expect(matchPath(route, "/_/APIKeys")?.params["*"]).toBe("APIKeys"); | ||
| expect(matchPath(route, "/deeplink/apikeys")).toBeNull(); | ||
| expect(matchPath(route, "/apikeys")).toBeNull(); | ||
| }); | ||
|
|
||
| it("treats a bare prefix, a trailing slash and anything outside it as no suffix", () => { | ||
| expect(deeplinkSuffix("/_")).toBe(""); | ||
| expect(deeplinkSuffix("/_/")).toBe(""); | ||
| expect(deeplinkSuffix("/etc")).toBe(""); | ||
| expect(deeplinkSuffix("/_app/orgs")).toBe(""); | ||
| }); | ||
|
|
||
| it("matches what the URL parser actually produces, keeping %2F and resolving %2e%2e", () => { | ||
| const encodedSlash = new URL("http://x/_/tasks/standard/group%2Fmy-task"); | ||
| expect(deeplinkSuffix(encodedSlash.pathname)).toBe("tasks/standard/group%2Fmy-task"); | ||
| expect(resolveDeeplinkPage(deeplinkSuffix(encodedSlash.pathname))).toBe( | ||
| "tasks/standard/group%2Fmy-task" | ||
| ); | ||
|
|
||
| const traversal = new URL("http://x/_/runs/%2e%2e/%2e%2e/etc"); | ||
| expect(traversal.pathname).toBe("/etc"); | ||
| expect(resolveDeeplinkPage(deeplinkSuffix(traversal.pathname))).toBeUndefined(); | ||
| }); | ||
| }); |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.