From d2efe29456c7bb230c5a93b7e75b3bae3adea073 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 16:46:57 +0000 Subject: [PATCH 01/12] Add /deeplink/* redirect route Resolves the signed-in user's current organization, project and environment and redirects to the canonical page, so /deeplink/apikeys lands on /orgs/{org}/projects/{project}/env/{env}/apikeys. Only the environment page segments navigation already knows about are followed (derived from ENV_PAGE_META), so an unrecognised path redirects to the environment root rather than becoming the redirect target. Deeper segments and the query string are preserved. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01MvNi2jRtYatPQdXFJRK9SW --- .server-changes/deeplink-routes.md | 6 +++ .../components/navigation/favoritePages.tsx | 8 +++ apps/webapp/app/routes/deeplink.$.ts | 54 +++++++++++++++++++ 3 files changed, 68 insertions(+) create mode 100644 .server-changes/deeplink-routes.md create mode 100644 apps/webapp/app/routes/deeplink.$.ts diff --git a/.server-changes/deeplink-routes.md b/.server-changes/deeplink-routes.md new file mode 100644 index 00000000000..56ab7c9bbc6 --- /dev/null +++ b/.server-changes/deeplink-routes.md @@ -0,0 +1,6 @@ +--- +area: webapp +type: feature +--- + +Short links like /deeplink/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. diff --git a/apps/webapp/app/components/navigation/favoritePages.tsx b/apps/webapp/app/components/navigation/favoritePages.tsx index 4439aadedfd..8173b0ceccd 100644 --- a/apps/webapp/app/components/navigation/favoritePages.tsx +++ b/apps/webapp/app/components/navigation/favoritePages.tsx @@ -237,6 +237,14 @@ const ENV_PAGE_META: Record = { playground: { icon: "test", name: "Test", singular: "Test" }, }; +/** + * The page segments that live under an environment ("apikeys", "runs", …), derived from the pages + * navigation knows about. Used by the /deeplink/* route to validate its redirect target. + */ +export const ENV_PAGE_SEGMENTS: ReadonlySet = new Set( + Object.keys(ENV_PAGE_META).filter((segment) => segment.length > 0) +); + const ORG_SETTINGS_PAGE_META: Record = { "": { icon: "org-settings", name: "Organization settings" }, team: { icon: "team", name: "Team" }, diff --git a/apps/webapp/app/routes/deeplink.$.ts b/apps/webapp/app/routes/deeplink.$.ts new file mode 100644 index 00000000000..2c4654bef26 --- /dev/null +++ b/apps/webapp/app/routes/deeplink.$.ts @@ -0,0 +1,54 @@ +import { redirect, type LoaderFunctionArgs } from "@remix-run/server-runtime"; +import { ENV_PAGE_SEGMENTS } from "~/components/navigation/favoritePages"; +import { prisma } from "~/db.server"; +import { SelectBestEnvironmentPresenter } from "~/presenters/SelectBestEnvironmentPresenter.server"; +import { requireUser } from "~/services/session.server"; +import { newOrganizationPath, newProjectPath, v3EnvironmentPath } from "~/utils/pathBuilder"; + +/** + * Stable links that don't name an org, project or environment: /deeplink/apikeys redirects to + * /orgs/{org}/projects/{project}/env/{env}/apikeys for whoever is signed in. Only pages the + * dashboard knows about are followed, so an unrecognised path can never become the redirect + * target — it lands on the resolved environment instead. + */ +export const loader = async ({ request, params }: LoaderFunctionArgs) => { + const user = await requireUser(request); + + const segments = (params["*"] ?? "").split("/").filter(Boolean); + //deeper segments are kept, so /deeplink/runs/run_123 reaches the run + const page = ENV_PAGE_SEGMENTS.has(segments[0] ?? "") ? segments.join("/") : undefined; + + const presenter = new SelectBestEnvironmentPresenter(); + try { + const { project, organization, environment } = await presenter.call({ user }); + const environmentPath = v3EnvironmentPath(organization, project, environment); + + if (!page) { + return redirect(environmentPath); + } + + const { search } = new URL(request.url); + return redirect(`${environmentPath}/${page}${search}`); + } catch (_e) { + //the presenter throws when the user has no projects, same as the dashboard index + 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()); + } +}; From e326919772f8fb1fa615ad76e5c2fcb76f1c333f Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 17:00:13 +0000 Subject: [PATCH 02/12] Harden the deeplink suffix against traversal and delimiter injection The splat arrives percent-decoded from the router, so the preserved remainder could contain "." / ".." segments and literal "?" or "#" characters. Traversal segments let a crafted suffix climb back out of the resolved environment path (/deeplink/apikeys/../../../../../../x resolved to /orgs/x), and a decoded "?" or "#" became part of the target's query or hash rather than its path. Drop traversal segments and re-encode each remaining segment when rebuilding the path. The redirect can no longer leave the resolved environment path. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01MvNi2jRtYatPQdXFJRK9SW --- apps/webapp/app/routes/deeplink.$.ts | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/apps/webapp/app/routes/deeplink.$.ts b/apps/webapp/app/routes/deeplink.$.ts index 2c4654bef26..79e850baf8e 100644 --- a/apps/webapp/app/routes/deeplink.$.ts +++ b/apps/webapp/app/routes/deeplink.$.ts @@ -14,21 +14,24 @@ import { newOrganizationPath, newProjectPath, v3EnvironmentPath } from "~/utils/ export const loader = async ({ request, params }: LoaderFunctionArgs) => { const user = await requireUser(request); - const segments = (params["*"] ?? "").split("/").filter(Boolean); - //deeper segments are kept, so /deeplink/runs/run_123 reaches the run - const page = ENV_PAGE_SEGMENTS.has(segments[0] ?? "") ? segments.join("/") : undefined; + //traversal segments are dropped so a crafted suffix can't climb out of the environment path + const segments = (params["*"] ?? "") + .split("/") + .filter((segment) => segment.length > 0 && segment !== "." && segment !== ".."); + //deeper segments are kept, so /deeplink/runs/run_123 reaches the run. They arrive decoded, so + //they're re-encoded: a "?" or "#" in a segment must not become the target's query or hash. + const page = ENV_PAGE_SEGMENTS.has(segments[0] ?? "") + ? segments.map(encodeURIComponent).join("/") + : undefined; + + const { search } = new URL(request.url); const presenter = new SelectBestEnvironmentPresenter(); try { const { project, organization, environment } = await presenter.call({ user }); const environmentPath = v3EnvironmentPath(organization, project, environment); - if (!page) { - return redirect(environmentPath); - } - - const { search } = new URL(request.url); - return redirect(`${environmentPath}/${page}${search}`); + return redirect(page ? `${environmentPath}/${page}${search}` : environmentPath); } catch (_e) { //the presenter throws when the user has no projects, same as the dashboard index const organization = await prisma.organization.findFirst({ From a1fa3858a76442da96948e92cc5320bc9b4d0ea7 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 17:49:08 +0000 Subject: [PATCH 03/12] Derive deeplink allowlist from the environment route segments The allowlist came from ENV_PAGE_META, which omits tasks, agents and settings because those URL shapes are special-cased when resolving page metadata. Deeplinks to them fell through to the environment root. Move the list to its own module and populate it from the environment layout route segments instead, so task, agent and project settings deeplinks resolve. --- .../components/navigation/favoritePages.tsx | 8 ----- apps/webapp/app/routes/deeplink.$.ts | 6 ++-- apps/webapp/app/utils/deeplinkPages.ts | 35 +++++++++++++++++++ 3 files changed, 38 insertions(+), 11 deletions(-) create mode 100644 apps/webapp/app/utils/deeplinkPages.ts diff --git a/apps/webapp/app/components/navigation/favoritePages.tsx b/apps/webapp/app/components/navigation/favoritePages.tsx index 8173b0ceccd..4439aadedfd 100644 --- a/apps/webapp/app/components/navigation/favoritePages.tsx +++ b/apps/webapp/app/components/navigation/favoritePages.tsx @@ -237,14 +237,6 @@ const ENV_PAGE_META: Record = { playground: { icon: "test", name: "Test", singular: "Test" }, }; -/** - * The page segments that live under an environment ("apikeys", "runs", …), derived from the pages - * navigation knows about. Used by the /deeplink/* route to validate its redirect target. - */ -export const ENV_PAGE_SEGMENTS: ReadonlySet = new Set( - Object.keys(ENV_PAGE_META).filter((segment) => segment.length > 0) -); - const ORG_SETTINGS_PAGE_META: Record = { "": { icon: "org-settings", name: "Organization settings" }, team: { icon: "team", name: "Team" }, diff --git a/apps/webapp/app/routes/deeplink.$.ts b/apps/webapp/app/routes/deeplink.$.ts index 79e850baf8e..47de2d9b1b2 100644 --- a/apps/webapp/app/routes/deeplink.$.ts +++ b/apps/webapp/app/routes/deeplink.$.ts @@ -1,14 +1,14 @@ import { redirect, type LoaderFunctionArgs } from "@remix-run/server-runtime"; -import { ENV_PAGE_SEGMENTS } from "~/components/navigation/favoritePages"; import { prisma } from "~/db.server"; import { SelectBestEnvironmentPresenter } from "~/presenters/SelectBestEnvironmentPresenter.server"; import { requireUser } from "~/services/session.server"; +import { ENV_PAGE_SEGMENTS } from "~/utils/deeplinkPages"; import { newOrganizationPath, newProjectPath, v3EnvironmentPath } from "~/utils/pathBuilder"; /** * Stable links that don't name an org, project or environment: /deeplink/apikeys redirects to - * /orgs/{org}/projects/{project}/env/{env}/apikeys for whoever is signed in. Only pages the - * dashboard knows about are followed, so an unrecognised path can never become the redirect + * /orgs/{org}/projects/{project}/env/{env}/apikeys for whoever is signed in. Only the environment + * pages in ENV_PAGE_SEGMENTS are followed, so an unrecognised path can never become the redirect * target — it lands on the resolved environment instead. */ export const loader = async ({ request, params }: LoaderFunctionArgs) => { diff --git a/apps/webapp/app/utils/deeplinkPages.ts b/apps/webapp/app/utils/deeplinkPages.ts new file mode 100644 index 00000000000..dd4e9d3454a --- /dev/null +++ b/apps/webapp/app/utils/deeplinkPages.ts @@ -0,0 +1,35 @@ +/** + * Pages that /deeplink/* is allowed to redirect to. This mirrors the first path segment of the + * environment-layout routes (`_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.*`), + * so add an entry here when a new page is added under that layout. + */ +export const ENV_PAGE_SEGMENTS: ReadonlySet = new Set([ + "agents", + "alerts", + "apikeys", + "batches", + "branches", + "bulk-actions", + "concurrency", + "dashboards", + "deployments", + "dev-branches", + "environment-variables", + "errors", + "limits", + "logs", + "metrics", + "models", + "playground", + "prompts", + "query", + "queues", + "regions", + "runs", + "schedules", + "sessions", + "settings", + "tasks", + "test", + "waitpoints", +]); From cf24f034b06a5535001d339349e64eec360e3d16 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 17:58:09 +0000 Subject: [PATCH 04/12] Add a test that the deeplink allowlist matches the environment routes The allowlist is maintained by hand, so it can fall behind when a page is added. The test derives the expected set from the environment layout's route filenames and names any segment that drifts. --- apps/webapp/app/utils/deeplinkPages.test.ts | 61 +++++++++++++++++++++ 1 file changed, 61 insertions(+) create mode 100644 apps/webapp/app/utils/deeplinkPages.test.ts diff --git a/apps/webapp/app/utils/deeplinkPages.test.ts b/apps/webapp/app/utils/deeplinkPages.test.ts new file mode 100644 index 00000000000..77bb8e03686 --- /dev/null +++ b/apps/webapp/app/utils/deeplinkPages.test.ts @@ -0,0 +1,61 @@ +import { readdirSync } from "node:fs"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; +import { ENV_PAGE_SEGMENTS } from "./deeplinkPages"; + +// Flat-route prefix for every page that renders inside an environment. The trailing dot matters: +// it excludes the layout route itself (`…env.$envParam`), which has no segment of its own. +const ENV_ROUTE_PREFIX = "_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam."; + +/** + * Segments that are route files but not deeplink targets: + * - `_index` is the environment root, which is already where an unrecognised deeplink lands. + * - `queues_` is Remix's "opt out of the parent layout" spelling of `queues`, not a distinct URL. + */ +const NOT_DEEPLINKABLE = new Set(["_index", "queues_"]); + +/** The first path segment of every environment page, read off the route filenames. */ +function envRouteSegments(): Set { + const entries = readdirSync(join(__dirname, "../routes")); + const segments = new Set(); + + for (const entry of entries) { + if (!entry.startsWith(ENV_ROUTE_PREFIX)) continue; + // `metrics.$dashboardKey.ts` -> `metrics`, `agents` -> `agents`, `errors._index` -> `errors` + const segment = entry.slice(ENV_ROUTE_PREFIX.length).split(/[./]/)[0]; + // Guards against a future `…env.$envParam.tsx` contributing its extension as a segment. + if (!segment || segment === "ts" || segment === "tsx") continue; + if (NOT_DEEPLINKABLE.has(segment)) continue; + segments.add(segment); + } + + return segments; +} + +describe("deeplink allowlist", () => { + it("matches the environment layout's route segments", () => { + // Sorted arrays rather than sets so a mismatch names the segment that drifted. + expect([...ENV_PAGE_SEGMENTS].sort()).toEqual([...envRouteSegments()].sort()); + }); + + it("found the routes directory", () => { + // Guards the test itself: an empty derived set would make the assertion above vacuous + // if the allowlist were ever emptied too. + expect(envRouteSegments().size).toBeGreaterThan(20); + }); + + it("excludes the environment root and the layout-opt-out spelling", () => { + expect(ENV_PAGE_SEGMENTS.has("_index")).toBe(false); + expect(ENV_PAGE_SEGMENTS.has("queues_")).toBe(false); + // `queues` itself is still reachable — it is the real URL segment. + expect(ENV_PAGE_SEGMENTS.has("queues")).toBe(true); + }); + + it("includes the pages that ENV_PAGE_META omits", () => { + // These have no entry in ENV_PAGE_META (their icon/label is special-cased when resolving + // page metadata), which is why the allowlist is derived from routes and not from that map. + for (const segment of ["tasks", "agents", "settings"]) { + expect(ENV_PAGE_SEGMENTS.has(segment)).toBe(true); + } + }); +}); From c72f50109716d19b684289b060f7145cafc2b880 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 18:12:18 +0000 Subject: [PATCH 05/12] Send deeplinks whose own segment has no page to a page that exists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tasks, waitpoints and metrics only exist as the parent of param routes, so a bare /deeplink/tasks redirected to a URL matching no route and rendered a 404 — worse than the environment root it used to fall back to. Replace the segment allowlist with a map from deeplink name to target path. tasks points at the environment root, which is the task list; waitpoints points at waitpoints/tokens; metrics is dropped, being only a legacy redirect shim with no page of its own. Deeper segments are still kept as given, so task and run detail links keep working. The test now checks that every target resolves to a real route and that no environment page is missing, instead of comparing bare segment names. --- apps/webapp/app/routes/deeplink.$.ts | 27 ++--- apps/webapp/app/utils/deeplinkPages.test.ts | 122 +++++++++++++++----- apps/webapp/app/utils/deeplinkPages.ts | 92 ++++++++++----- 3 files changed, 165 insertions(+), 76 deletions(-) diff --git a/apps/webapp/app/routes/deeplink.$.ts b/apps/webapp/app/routes/deeplink.$.ts index 47de2d9b1b2..7b823ae1d11 100644 --- a/apps/webapp/app/routes/deeplink.$.ts +++ b/apps/webapp/app/routes/deeplink.$.ts @@ -2,28 +2,19 @@ import { redirect, type LoaderFunctionArgs } from "@remix-run/server-runtime"; import { prisma } from "~/db.server"; import { SelectBestEnvironmentPresenter } from "~/presenters/SelectBestEnvironmentPresenter.server"; import { requireUser } from "~/services/session.server"; -import { ENV_PAGE_SEGMENTS } from "~/utils/deeplinkPages"; +import { resolveDeeplinkPage } from "~/utils/deeplinkPages"; import { newOrganizationPath, newProjectPath, v3EnvironmentPath } from "~/utils/pathBuilder"; /** * Stable links that don't name an org, project or environment: /deeplink/apikeys redirects to - * /orgs/{org}/projects/{project}/env/{env}/apikeys for whoever is signed in. Only the environment - * pages in ENV_PAGE_SEGMENTS are followed, so an unrecognised path can never become the redirect - * target — it lands on the resolved environment instead. + * /orgs/{org}/projects/{project}/env/{env}/apikeys for whoever is signed in. Only the pages in + * ENV_PAGE_TARGETS are followed, so an unrecognised path can never become the redirect target — + * it lands on the resolved environment instead. */ export const loader = async ({ request, params }: LoaderFunctionArgs) => { const user = await requireUser(request); - //traversal segments are dropped so a crafted suffix can't climb out of the environment path - const segments = (params["*"] ?? "") - .split("/") - .filter((segment) => segment.length > 0 && segment !== "." && segment !== ".."); - //deeper segments are kept, so /deeplink/runs/run_123 reaches the run. They arrive decoded, so - //they're re-encoded: a "?" or "#" in a segment must not become the target's query or hash. - const page = ENV_PAGE_SEGMENTS.has(segments[0] ?? "") - ? segments.map(encodeURIComponent).join("/") - : undefined; - + const page = resolveDeeplinkPage(params["*"] ?? ""); const { search } = new URL(request.url); const presenter = new SelectBestEnvironmentPresenter(); @@ -31,7 +22,13 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => { const { project, organization, environment } = await presenter.call({ user }); const environmentPath = v3EnvironmentPath(organization, project, environment); - return redirect(page ? `${environmentPath}/${page}${search}` : environmentPath); + //an unrecognised path keeps nothing: it lands on the environment as if no suffix was given + if (page === undefined) { + return redirect(environmentPath); + } + + //`tasks` targets the environment root, so there is no suffix to append + return redirect(page ? `${environmentPath}/${page}${search}` : `${environmentPath}${search}`); } catch (_e) { //the presenter throws when the user has no projects, same as the dashboard index const organization = await prisma.organization.findFirst({ diff --git a/apps/webapp/app/utils/deeplinkPages.test.ts b/apps/webapp/app/utils/deeplinkPages.test.ts index 77bb8e03686..0f9457192a4 100644 --- a/apps/webapp/app/utils/deeplinkPages.test.ts +++ b/apps/webapp/app/utils/deeplinkPages.test.ts @@ -1,61 +1,125 @@ -import { readdirSync } from "node:fs"; +import { existsSync, readdirSync, statSync } from "node:fs"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; -import { ENV_PAGE_SEGMENTS } from "./deeplinkPages"; +import { ENV_PAGE_TARGETS, resolveDeeplinkPage } from "./deeplinkPages"; + +const ROUTES_DIR = join(__dirname, "../routes"); // Flat-route prefix for every page that renders inside an environment. The trailing dot matters: // it excludes the layout route itself (`…env.$envParam`), which has no segment of its own. const ENV_ROUTE_PREFIX = "_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam."; /** - * Segments that are route files but not deeplink targets: - * - `_index` is the environment root, which is already where an unrecognised deeplink lands. + * Segments that are route files but are not deeplink names: + * - `_index` is the environment root. It is where an unrecognised deeplink already lands, and + * `tasks` is the name that points at it. * - `queues_` is Remix's "opt out of the parent layout" spelling of `queues`, not a distinct URL. */ -const NOT_DEEPLINKABLE = new Set(["_index", "queues_"]); +const NOT_DEEPLINK_NAMES = new Set(["_index", "queues_"]); + +const routeEntries = readdirSync(ROUTES_DIR); + +/** A route directory only contributes a route if it actually holds a `route` module. */ +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")); +} + +/** + * The route file that a bare `/env/{env}/{target}` URL matches, or undefined when nothing does. + * `target` may span segments ("waitpoints/tokens"); "" is the environment root. + * + * Only literal route names are considered — a param route (`metrics.$dashboardKey`) is not a page + * you can land on without supplying the param, which is exactly what this needs to reject. + */ +function routeForTarget(target: string): string | undefined { + if (target === "") { + return isRouteModule(`${ENV_ROUTE_PREFIX}_index`) ? `${ENV_ROUTE_PREFIX}_index` : undefined; + } + + const base = ENV_ROUTE_PREFIX + target.split("/").join("."); + // A leaf route, or a layout whose index child supplies the bare URL. + return [base, `${base}.tsx`, `${base}.ts`, `${base}._index`].find( + (candidate) => routeEntries.includes(candidate) && isRouteModule(candidate) + ); +} -/** The first path segment of every environment page, read off the route filenames. */ +/** Every first segment appearing under the environment layout. */ function envRouteSegments(): Set { - const entries = readdirSync(join(__dirname, "../routes")); const segments = new Set(); - - for (const entry of entries) { + for (const entry of routeEntries) { if (!entry.startsWith(ENV_ROUTE_PREFIX)) continue; // `metrics.$dashboardKey.ts` -> `metrics`, `agents` -> `agents`, `errors._index` -> `errors` const segment = entry.slice(ENV_ROUTE_PREFIX.length).split(/[./]/)[0]; // Guards against a future `…env.$envParam.tsx` contributing its extension as a segment. if (!segment || segment === "ts" || segment === "tsx") continue; - if (NOT_DEEPLINKABLE.has(segment)) continue; segments.add(segment); } - return segments; } -describe("deeplink allowlist", () => { - it("matches the environment layout's route segments", () => { - // Sorted arrays rather than sets so a mismatch names the segment that drifted. - expect([...ENV_PAGE_SEGMENTS].sort()).toEqual([...envRouteSegments()].sort()); - }); - +describe("deeplink targets", () => { it("found the routes directory", () => { - // Guards the test itself: an empty derived set would make the assertion above vacuous - // if the allowlist were ever emptied too. + // Without this, every assertion below would pass vacuously if the glob ever broke. expect(envRouteSegments().size).toBeGreaterThan(20); }); - it("excludes the environment root and the layout-opt-out spelling", () => { - expect(ENV_PAGE_SEGMENTS.has("_index")).toBe(false); - expect(ENV_PAGE_SEGMENTS.has("queues_")).toBe(false); - // `queues` itself is still reachable — it is the real URL segment. - expect(ENV_PAGE_SEGMENTS.has("queues")).toBe(true); + it("every target resolves to a real environment route", () => { + const unresolved = [...ENV_PAGE_TARGETS.entries()] + .filter(([, target]) => !routeForTarget(target)) + .map(([name, target]) => `${name} -> ${target || "(environment root)"}`); + + expect(unresolved).toEqual([]); + }); + + it("every environment page has a deeplink name", () => { + // A segment that resolves bare is a page someone could reasonably want to link to. + const missing = [...envRouteSegments()] + .filter((segment) => !NOT_DEEPLINK_NAMES.has(segment)) + .filter((segment) => routeForTarget(segment) && !ENV_PAGE_TARGETS.has(segment)) + .sort(); + + expect(missing).toEqual([]); }); - it("includes the pages that ENV_PAGE_META omits", () => { - // These have no entry in ENV_PAGE_META (their icon/label is special-cased when resolving - // page metadata), which is why the allowlist is derived from routes and not from that map. - for (const segment of ["tasks", "agents", "settings"]) { - expect(ENV_PAGE_SEGMENTS.has(segment)).toBe(true); + it("names whose own segment 404s are redirected, not mapped to themselves", () => { + // These exist only as the parent of param/child routes, so a bare URL matches no route. + for (const segment of ["tasks", "waitpoints", "metrics"]) { + expect(routeForTarget(segment)).toBeUndefined(); } + + // `tasks` and `waitpoints` therefore point somewhere else; `metrics` is only a legacy redirect + // shim with no page of its own, so it is deliberately not a deeplink name at all. + expect(ENV_PAGE_TARGETS.get("tasks")).toBe(""); + expect(ENV_PAGE_TARGETS.get("waitpoints")).toBe("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("keeps deeper segments, which address a real sub-route", () => { + expect(resolveDeeplinkPage("runs/run_123")).toBe("runs/run_123"); + expect(resolveDeeplinkPage("tasks/standard/my-task")).toBe("tasks/standard/my-task"); + expect(resolveDeeplinkPage("waitpoints/tokens")).toBe("waitpoints/tokens"); + }); + + it("rejects a name that is not a page", () => { + expect(resolveDeeplinkPage("")).toBeUndefined(); + expect(resolveDeeplinkPage("nonsense")).toBeUndefined(); + expect(resolveDeeplinkPage("metrics")).toBeUndefined(); + }); + + it("drops traversal segments and encodes the rest", () => { + expect(resolveDeeplinkPage("runs/../../../etc/passwd")).toBe("runs/etc/passwd"); + expect(resolveDeeplinkPage("../runs")).toBe("runs"); + expect(resolveDeeplinkPage("runs/a?b#c")).toBe("runs/a%3Fb%23c"); + expect(resolveDeeplinkPage("runs//run_1")).toBe("runs/run_1"); }); }); diff --git a/apps/webapp/app/utils/deeplinkPages.ts b/apps/webapp/app/utils/deeplinkPages.ts index dd4e9d3454a..daf66c015ce 100644 --- a/apps/webapp/app/utils/deeplinkPages.ts +++ b/apps/webapp/app/utils/deeplinkPages.ts @@ -1,35 +1,63 @@ /** - * Pages that /deeplink/* is allowed to redirect to. This mirrors the first path segment of the - * environment-layout routes (`_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.*`), - * so add an entry here when a new page is added under that layout. + * Where each /deeplink/ lands, relative to the resolved environment. Most names are a page + * in their own right and map to themselves. A few exist only as the parent of param routes + * (`tasks.standard.$taskParam`, `waitpoints.tokens`) — a bare `/tasks` matches no route and would + * 404 — so those map to the page a user actually wants instead. + * + * This mirrors the environment-layout routes + * (`_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.*`): add a page here when one + * is added there. `deeplinkPages.test.ts` checks every target against the route files and fails if + * a page is missing or a target stops resolving. */ -export const ENV_PAGE_SEGMENTS: ReadonlySet = new Set([ - "agents", - "alerts", - "apikeys", - "batches", - "branches", - "bulk-actions", - "concurrency", - "dashboards", - "deployments", - "dev-branches", - "environment-variables", - "errors", - "limits", - "logs", - "metrics", - "models", - "playground", - "prompts", - "query", - "queues", - "regions", - "runs", - "schedules", - "sessions", - "settings", - "tasks", - "test", - "waitpoints", +export const ENV_PAGE_TARGETS: ReadonlyMap = new Map([ + ["agents", "agents"], + ["alerts", "alerts"], + ["apikeys", "apikeys"], + ["batches", "batches"], + ["branches", "branches"], + ["bulk-actions", "bulk-actions"], + ["concurrency", "concurrency"], + ["dashboards", "dashboards"], + ["deployments", "deployments"], + ["dev-branches", "dev-branches"], + ["environment-variables", "environment-variables"], + ["errors", "errors"], + ["limits", "limits"], + ["logs", "logs"], + ["models", "models"], + ["playground", "playground"], + ["prompts", "prompts"], + ["query", "query"], + ["queues", "queues"], + ["regions", "regions"], + ["runs", "runs"], + ["schedules", "schedules"], + ["sessions", "sessions"], + ["settings", "settings"], + // The environment root is the task list (its route is the env `_index`, titled "Tasks"), so a + // bare /deeplink/tasks belongs there rather than at the secondary /tasks/dashboard view. + ["tasks", ""], + ["test", "test"], + ["waitpoints", "waitpoints/tokens"], ]); + +/** + * The path a deeplink suffix should redirect to, relative to the environment, or undefined when the + * first segment names no page. Returns "" for a target that is the environment root itself. + * + * Segments beyond the first are kept as given, because they address a real sub-route + * (`/deeplink/runs/run_123`, `/deeplink/tasks/standard/my-task`); only a bare name uses the mapped + * landing page. They arrive decoded, so they are re-encoded: a "?" or "#" in a segment must not + * become the target's query or hash. + */ +export function resolveDeeplinkPage(splat: string): string | undefined { + //traversal segments are dropped so a crafted suffix can't climb out of the environment path + const segments = splat + .split("/") + .filter((segment) => segment.length > 0 && segment !== "." && segment !== ".."); + + const target = ENV_PAGE_TARGETS.get(segments[0] ?? ""); + if (target === undefined) return undefined; + + return segments.length > 1 ? segments.map(encodeURIComponent).join("/") : target; +} From 90577d28590566284a3ed2627c784d1f8d060eb4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 18:21:59 +0000 Subject: [PATCH 06/12] Give deeplink targets a separate landing path and deep prefix A mapped target only applied to a bare name, so /deeplink/waitpoints landed on waitpoints/tokens but /deeplink/waitpoints/waitpoint_123 became waitpoints/waitpoint_123, which matches no route. Prefixing the target unconditionally would break tasks, whose landing is the environment root but whose detail pages are under /tasks, so each entry now carries both a landing path and a prefix for deeper segments. An unrecognised name also kept no query string while a mapped one did, which became visible once tasks started resolving to the environment root: three links to the same page behaved two ways. Both now keep it. The test walks the real child routes under each prefix rather than probing one synthetic segment, so a target whose deep form stops resolving fails. --- apps/webapp/app/routes/deeplink.$.ts | 10 +- apps/webapp/app/utils/deeplinkPages.test.ts | 117 +++++++++++++++----- apps/webapp/app/utils/deeplinkPages.ts | 109 +++++++++++------- 3 files changed, 163 insertions(+), 73 deletions(-) diff --git a/apps/webapp/app/routes/deeplink.$.ts b/apps/webapp/app/routes/deeplink.$.ts index 7b823ae1d11..c3b82a137bc 100644 --- a/apps/webapp/app/routes/deeplink.$.ts +++ b/apps/webapp/app/routes/deeplink.$.ts @@ -22,13 +22,11 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => { const { project, organization, environment } = await presenter.call({ user }); const environmentPath = v3EnvironmentPath(organization, project, environment); - //an unrecognised path keeps nothing: it lands on the environment as if no suffix was given - if (page === undefined) { - return redirect(environmentPath); - } + //Both an unrecognised name and `tasks` (which targets the environment root) leave no suffix, + //and the query survives either way, so all three spellings of "the environment" agree. + const suffix = page ? `/${page}` : ""; - //`tasks` targets the environment root, so there is no suffix to append - return redirect(page ? `${environmentPath}/${page}${search}` : `${environmentPath}${search}`); + return redirect(`${environmentPath}${suffix}${search}`); } catch (_e) { //the presenter throws when the user has no projects, same as the dashboard index const organization = await prisma.organization.findFirst({ diff --git a/apps/webapp/app/utils/deeplinkPages.test.ts b/apps/webapp/app/utils/deeplinkPages.test.ts index 0f9457192a4..6f4fba75a1d 100644 --- a/apps/webapp/app/utils/deeplinkPages.test.ts +++ b/apps/webapp/app/utils/deeplinkPages.test.ts @@ -17,6 +17,9 @@ const ENV_ROUTE_PREFIX = "_app.orgs.$organizationSlug.projects.$projectParam.env */ const NOT_DEEPLINK_NAMES = new Set(["_index", "queues_"]); +/** Stands in for a param segment, so a deep path under test looks like a real URL. */ +const PROBE = "probe_01ABC"; + const routeEntries = readdirSync(ROUTES_DIR); /** A route directory only contributes a route if it actually holds a `route` module. */ @@ -27,21 +30,28 @@ function isRouteModule(entry: string): boolean { } /** - * The route file that a bare `/env/{env}/{target}` URL matches, or undefined when nothing does. - * `target` may span segments ("waitpoints/tokens"); "" is the environment root. - * - * Only literal route names are considered — a param route (`metrics.$dashboardKey`) is not a page - * you can land on without supplying the param, which is exactly what this needs to reject. + * Every environment route as its URL segments. A trailing `_index` is dropped (it supplies the + * parent's bare URL) and a trailing `_` is trimmed from each segment, because `queues_.$queueParam` + * serves `/queues/{id}` — the underscore only opts out of the parent layout. */ -function routeForTarget(target: string): string | undefined { - if (target === "") { - return isRouteModule(`${ENV_ROUTE_PREFIX}_index`) ? `${ENV_ROUTE_PREFIX}_index` : undefined; - } - - const base = ENV_ROUTE_PREFIX + target.split("/").join("."); - // A leaf route, or a layout whose index child supplies the bare URL. - return [base, `${base}.tsx`, `${base}.ts`, `${base}._index`].find( - (candidate) => routeEntries.includes(candidate) && isRouteModule(candidate) +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(/_+$/, ""))); + +/** Does any route match this environment-relative URL? Param segments match the deep case only. */ +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])) ); } @@ -59,25 +69,68 @@ function envRouteSegments(): Set { return segments; } +/** + * Every route below this prefix, as the segments that follow it, with param segments replaced by a + * value a real URL would carry. These are the deep links that can actually be made under a name. + */ +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("found the routes directory", () => { // Without this, every assertion below would pass vacuously if the glob ever broke. expect(envRouteSegments().size).toBeGreaterThan(20); + expect(envRoutes.length).toBeGreaterThan(40); }); - it("every target resolves to a real environment route", () => { - const unresolved = [...ENV_PAGE_TARGETS.entries()] - .filter(([, target]) => !routeForTarget(target)) - .map(([name, target]) => `${name} -> ${target || "(environment root)"}`); + it("every bare name lands on a real page", () => { + // A landing page is a page you arrive at with no id, so a param route does not count. + const broken = [...ENV_PAGE_TARGETS.entries()] + .filter(([name]) => !routeMatches(resolveDeeplinkPage(name) ?? " ", { allowParams: false })) + .map(([name, { landing }]) => `${name} -> ${landing || "(environment root)"}`); - expect(unresolved).toEqual([]); + expect(broken).toEqual([]); + }); + + it("every deep path lands on a real route", () => { + // The invariant a bare segment list could not express: /deeplink/waitpoints/{id} has to reach + // waitpoints/tokens/{id}, not waitpoints/{id}. Driven off the real child routes rather than one + // synthetic segment, so names whose children are all literal (settings/general) count too. + 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", () => { + // Keeps the assertion above from passing because it iterated nothing. + 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", () => { // A segment that resolves bare is a page someone could reasonably want to link to. const missing = [...envRouteSegments()] .filter((segment) => !NOT_DEEPLINK_NAMES.has(segment)) - .filter((segment) => routeForTarget(segment) && !ENV_PAGE_TARGETS.has(segment)) + .filter( + (segment) => routeMatches(segment, { allowParams: false }) && !ENV_PAGE_TARGETS.has(segment) + ) .sort(); expect(missing).toEqual([]); @@ -86,13 +139,16 @@ describe("deeplink targets", () => { it("names whose own segment 404s are redirected, not mapped to themselves", () => { // These exist only as the parent of param/child routes, so a bare URL matches no route. for (const segment of ["tasks", "waitpoints", "metrics"]) { - expect(routeForTarget(segment)).toBeUndefined(); + expect(routeMatches(segment, { allowParams: false })).toBe(false); } - // `tasks` and `waitpoints` therefore point somewhere else; `metrics` is only a legacy redirect - // shim with no page of its own, so it is deliberately not a deeplink name at all. - expect(ENV_PAGE_TARGETS.get("tasks")).toBe(""); - expect(ENV_PAGE_TARGETS.get("waitpoints")).toBe("waitpoints/tokens"); + // `tasks` and `waitpoints` therefore point elsewhere; `metrics` is only a legacy redirect shim + // with no page of its own, so it is deliberately not a deeplink name at all. + 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); }); }); @@ -104,10 +160,19 @@ describe("resolveDeeplinkPage", () => { expect(resolveDeeplinkPage("tasks")).toBe(""); }); - it("keeps deeper segments, which address a real sub-route", () => { + it("grafts deeper segments onto the prefix", () => { expect(resolveDeeplinkPage("runs/run_123")).toBe("runs/run_123"); + // The landing is the environment root, but task detail still lives under /tasks. expect(resolveDeeplinkPage("tasks/standard/my-task")).toBe("tasks/standard/my-task"); + // The prefix supplies the `tokens` segment the caller did not have to know about. + 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", () => { diff --git a/apps/webapp/app/utils/deeplinkPages.ts b/apps/webapp/app/utils/deeplinkPages.ts index daf66c015ce..d452fc65ff2 100644 --- a/apps/webapp/app/utils/deeplinkPages.ts +++ b/apps/webapp/app/utils/deeplinkPages.ts @@ -1,54 +1,74 @@ /** - * Where each /deeplink/ lands, relative to the resolved environment. Most names are a page - * in their own right and map to themselves. A few exist only as the parent of param routes - * (`tasks.standard.$taskParam`, `waitpoints.tokens`) — a bare `/tasks` matches no route and would - * 404 — so those map to the page a user actually wants instead. + * Where each /deeplink/ goes, relative to the resolved environment. + * + * Two pieces, because a name's own page and the things underneath it are not always in the same + * place. `landing` is used for a bare `/deeplink/`; `prefix` is what deeper segments hang off. + * They differ only where a segment is not a page in its own right: + * + * - `tasks` has no bare route, and the task list is the environment root — but task detail pages do + * live under `/tasks`, so the landing is the root while the prefix stays `tasks`. + * - `waitpoints` has no bare route either, and its only page and its detail pages are both under + * `/waitpoints/tokens`, so both are that. * * This mirrors the environment-layout routes * (`_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.*`): add a page here when one - * is added there. `deeplinkPages.test.ts` checks every target against the route files and fails if - * a page is missing or a target stops resolving. + * is added there. `deeplinkPages.test.ts` checks every landing and every deep path against the + * route files and fails if a page is missing or a target stops resolving. */ -export const ENV_PAGE_TARGETS: ReadonlyMap = new Map([ - ["agents", "agents"], - ["alerts", "alerts"], - ["apikeys", "apikeys"], - ["batches", "batches"], - ["branches", "branches"], - ["bulk-actions", "bulk-actions"], - ["concurrency", "concurrency"], - ["dashboards", "dashboards"], - ["deployments", "deployments"], - ["dev-branches", "dev-branches"], - ["environment-variables", "environment-variables"], - ["errors", "errors"], - ["limits", "limits"], - ["logs", "logs"], - ["models", "models"], - ["playground", "playground"], - ["prompts", "prompts"], - ["query", "query"], - ["queues", "queues"], - ["regions", "regions"], - ["runs", "runs"], - ["schedules", "schedules"], - ["sessions", "sessions"], - ["settings", "settings"], - // The environment root is the task list (its route is the env `_index`, titled "Tasks"), so a - // bare /deeplink/tasks belongs there rather than at the secondary /tasks/dashboard view. - ["tasks", ""], - ["test", "test"], - ["waitpoints", "waitpoints/tokens"], +export type DeeplinkTarget = { + /** Path for a bare `/deeplink/`. "" is the environment root. */ + landing: string; + /** Deeper segments are appended to this: `/deeplink//a/b` -> `/a/b`. */ + prefix: string; +}; + +/** An ordinary page: its own segment is the page, and its children hang off it. */ +function page(name: string): DeeplinkTarget { + return { landing: name, prefix: name }; +} + +export const ENV_PAGE_TARGETS: ReadonlyMap = new Map([ + ["agents", page("agents")], + ["alerts", page("alerts")], + ["apikeys", page("apikeys")], + ["batches", page("batches")], + ["branches", page("branches")], + ["bulk-actions", page("bulk-actions")], + ["concurrency", page("concurrency")], + ["dashboards", page("dashboards")], + ["deployments", page("deployments")], + ["dev-branches", page("dev-branches")], + ["environment-variables", page("environment-variables")], + ["errors", page("errors")], + ["limits", page("limits")], + ["logs", page("logs")], + ["models", page("models")], + ["playground", page("playground")], + ["prompts", page("prompts")], + ["query", page("query")], + ["queues", page("queues")], + ["regions", page("regions")], + ["runs", page("runs")], + ["schedules", page("schedules")], + ["sessions", page("sessions")], + ["settings", page("settings")], + ["tasks", { landing: "", prefix: "tasks" }], + ["test", page("test")], + ["waitpoints", { landing: "waitpoints/tokens", prefix: "waitpoints/tokens" }], ]); /** * The path a deeplink suffix should redirect to, relative to the environment, or undefined when the * first segment names no page. Returns "" for a target that is the environment root itself. * - * Segments beyond the first are kept as given, because they address a real sub-route - * (`/deeplink/runs/run_123`, `/deeplink/tasks/standard/my-task`); only a bare name uses the mapped - * landing page. They arrive decoded, so they are re-encoded: a "?" or "#" in a segment must not - * become the target's query or hash. + * A bare name uses its landing path. Deeper segments are grafted onto the prefix, so + * `/deeplink/waitpoints/waitpoint_123` reaches the token that actually lives at + * `/waitpoints/tokens/waitpoint_123`. A suffix that already spells out a path under the prefix is + * kept as it was written, so both `/deeplink/waitpoints/waitpoint_123` and the longhand + * `/deeplink/waitpoints/tokens/waitpoint_123` arrive at the same place. + * + * Only the caller's own segments are encoded — the prefix is our own literal. They arrive decoded, + * so a "?" or "#" in a segment must not become the target's query or hash. */ export function resolveDeeplinkPage(splat: string): string | undefined { //traversal segments are dropped so a crafted suffix can't climb out of the environment path @@ -59,5 +79,12 @@ export function resolveDeeplinkPage(splat: string): string | undefined { const target = ENV_PAGE_TARGETS.get(segments[0] ?? ""); if (target === undefined) return undefined; - return segments.length > 1 ? segments.map(encodeURIComponent).join("/") : target; + if (segments.length === 1) return target.landing; + + const encoded = segments.map(encodeURIComponent); + const written = encoded.join("/"); + //already written out under the prefix, so grafting would duplicate it + if (written === target.prefix || written.startsWith(`${target.prefix}/`)) return written; + + return [target.prefix, ...encoded.slice(1)].join("/"); } From 16604509b4c4d5746692dd0150a28dff6d203538 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 18:36:19 +0000 Subject: [PATCH 07/12] Read the deeplink suffix from the pathname, and honour pending invites React Router decodes the splat param, so an id containing an escaped slash arrived split in two: /deeplink/tasks/standard/group%2Fmy-task became tasks/standard/group/my-task and matched no route, breaking a link copied from the dashboard. The suffix now comes from the request pathname, which keeps %2F intact, and its segments are passed through as they arrived rather than being encoded a second time. Traversal rejection now also covers the escaped spellings: a segment is dropped when it decodes to . or .. , or when it is not decodable at all. new URL already normalises %2e%2e and resolves it, which can move the pathname out of /deeplink entirely, so a suffix outside the prefix is treated as absent. A user with pending invites is also sent to the invites page first, as the dashboard index does, so an invitee following a deeplink before joining an organization is not offered organization creation instead. --- apps/webapp/app/routes/deeplink.$.ts | 25 +++++++-- apps/webapp/app/utils/deeplinkPages.test.ts | 56 +++++++++++++++++++-- apps/webapp/app/utils/deeplinkPages.ts | 52 +++++++++++++++---- 3 files changed, 115 insertions(+), 18 deletions(-) diff --git a/apps/webapp/app/routes/deeplink.$.ts b/apps/webapp/app/routes/deeplink.$.ts index c3b82a137bc..dd46fc73760 100644 --- a/apps/webapp/app/routes/deeplink.$.ts +++ b/apps/webapp/app/routes/deeplink.$.ts @@ -1,9 +1,15 @@ 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 { resolveDeeplinkPage } from "~/utils/deeplinkPages"; -import { newOrganizationPath, newProjectPath, v3EnvironmentPath } from "~/utils/pathBuilder"; +import { deeplinkSuffix, resolveDeeplinkPage } from "~/utils/deeplinkPages"; +import { + invitesPath, + newOrganizationPath, + newProjectPath, + v3EnvironmentPath, +} from "~/utils/pathBuilder"; /** * Stable links that don't name an org, project or environment: /deeplink/apikeys redirects to @@ -11,11 +17,20 @@ import { newOrganizationPath, newProjectPath, v3EnvironmentPath } from "~/utils/ * ENV_PAGE_TARGETS are followed, so an unrecognised path can never become the redirect target — * it lands on the resolved environment instead. */ -export const loader = async ({ request, params }: LoaderFunctionArgs) => { +export const loader = async ({ request }: LoaderFunctionArgs) => { const user = await requireUser(request); - const page = resolveDeeplinkPage(params["*"] ?? ""); - const { search } = new URL(request.url); + //the suffix comes from the pathname, not the splat param, so an id containing an escaped slash + //survives — see deeplinkSuffix + const { pathname, search } = new URL(request.url); + const page = resolveDeeplinkPage(deeplinkSuffix(pathname)); + + //a deeplink is the kind of URL a new invitee is sent, so take them to the invite first, exactly + //as the dashboard index does + const invites = await getUsersInvites({ email: user.email }); + if (invites.length > 0) { + return redirect(invitesPath()); + } const presenter = new SelectBestEnvironmentPresenter(); try { diff --git a/apps/webapp/app/utils/deeplinkPages.test.ts b/apps/webapp/app/utils/deeplinkPages.test.ts index 6f4fba75a1d..ca7261665ec 100644 --- a/apps/webapp/app/utils/deeplinkPages.test.ts +++ b/apps/webapp/app/utils/deeplinkPages.test.ts @@ -1,7 +1,7 @@ import { existsSync, readdirSync, statSync } from "node:fs"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; -import { ENV_PAGE_TARGETS, resolveDeeplinkPage } from "./deeplinkPages"; +import { deeplinkSuffix, ENV_PAGE_TARGETS, resolveDeeplinkPage } from "./deeplinkPages"; const ROUTES_DIR = join(__dirname, "../routes"); @@ -181,10 +181,60 @@ describe("resolveDeeplinkPage", () => { expect(resolveDeeplinkPage("metrics")).toBeUndefined(); }); - it("drops traversal segments and encodes the rest", () => { + 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/a?b#c")).toBe("runs/a%3Fb%23c"); expect(resolveDeeplinkPage("runs//run_1")).toBe("runs/run_1"); + // `%2e%2e` decodes to `..`, so it has to be rejected in the encoded form too. + expect(resolveDeeplinkPage("runs/%2e%2e/%2E%2E/run_1")).toBe("runs/run_1"); + expect(resolveDeeplinkPage("runs/%2e/run_1")).toBe("runs/run_1"); + // A malformed escape can't be part of a URL we build. + expect(resolveDeeplinkPage("runs/%ZZ/run_1")).toBe("runs/run_1"); + }); + + it("passes encoded segments through without re-encoding them", () => { + // The dashboard writes a task id containing a slash this way, so it must survive as one + // segment rather than being split or double-encoded into %252F. + expect(resolveDeeplinkPage("tasks/standard/group%2Fmy-task")).toBe( + "tasks/standard/group%2Fmy-task" + ); + expect(resolveDeeplinkPage("runs/a%3Fb%23c")).toBe("runs/a%3Fb%23c"); + // An escaped slash stays escaped, so this addresses one odd id rather than climbing out. + expect(resolveDeeplinkPage("runs/..%2f..%2fetc")).toBe("runs/..%2f..%2fetc"); + }); +}); + +describe("deeplinkSuffix", () => { + it("strips the route's own prefix", () => { + expect(deeplinkSuffix("/deeplink/tasks")).toBe("tasks"); + expect(deeplinkSuffix("/deeplink/runs/run_123")).toBe("runs/run_123"); + }); + + it("keeps an escaped slash intact, unlike the decoded splat param", () => { + expect(deeplinkSuffix("/deeplink/tasks/standard/group%2Fmy-task")).toBe( + "tasks/standard/group%2Fmy-task" + ); + }); + + it("treats a bare prefix, a trailing slash and anything outside it as no suffix", () => { + expect(deeplinkSuffix("/deeplink")).toBe(""); + expect(deeplinkSuffix("/deeplink/")).toBe(""); + // What `new URL` leaves behind once it has normalised and resolved `%2e%2e` itself. + expect(deeplinkSuffix("/etc")).toBe(""); + }); + + it("matches what the URL parser actually produces", () => { + // The behaviour above is only correct if `new URL` really does keep %2F and really does + // resolve %2e%2e, so assert that rather than assuming it. + const encodedSlash = new URL("http://x/deeplink/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" + ); + + // `%2e%2e` is normalised to `..` and resolved by the parser, leaving the prefix behind. + const traversal = new URL("http://x/deeplink/runs/%2e%2e/%2e%2e/etc"); + expect(traversal.pathname).toBe("/etc"); + expect(resolveDeeplinkPage(deeplinkSuffix(traversal.pathname))).toBeUndefined(); }); }); diff --git a/apps/webapp/app/utils/deeplinkPages.ts b/apps/webapp/app/utils/deeplinkPages.ts index d452fc65ff2..9514610f1e5 100644 --- a/apps/webapp/app/utils/deeplinkPages.ts +++ b/apps/webapp/app/utils/deeplinkPages.ts @@ -57,6 +57,42 @@ export const ENV_PAGE_TARGETS: ReadonlyMap = new Map([ ["waitpoints", { landing: "waitpoints/tokens", prefix: "waitpoints/tokens" }], ]); +/** Where this route is mounted. Matches the `deeplink.$` route filename. */ +export const DEEPLINK_PATH_PREFIX = "/deeplink"; + +/** + * The still-encoded suffix after /deeplink, taken from the request's pathname rather than the + * splat param. React Router decodes the splat, which turns an id containing an escaped slash + * (`group%2Fmy-task`, as the dashboard's own link builder writes it) into two segments that match + * no route. The pathname keeps `%2F` intact. + * + * Returns "" for anything that is not under the prefix. That includes a pathname the URL parser has + * already rewritten: it normalises `%2e%2e` to `..` and resolves it, so a traversal attempt can + * leave the prefix entirely before this ever sees it. + */ +export function deeplinkSuffix(pathname: string): string { + const withSlash = `${DEEPLINK_PATH_PREFIX}/`; + if (!pathname.startsWith(withSlash)) return ""; + + return pathname.slice(withSlash.length); +} + +/** Segments that must not reach the target path, tested in the encoded form we receive. */ +function isUsableSegment(segment: string): boolean { + if (segment.length === 0 || segment === "." || segment === "..") return false; + + let decoded: string; + try { + decoded = decodeURIComponent(segment); + } catch { + //malformed escape, so it can't be part of a URL we build + return false; + } + + //`%2e%2e` and friends, which would otherwise climb out of the environment path + return decoded !== "." && decoded !== ".."; +} + /** * The path a deeplink suffix should redirect to, relative to the environment, or undefined when the * first segment names no page. Returns "" for a target that is the environment root itself. @@ -67,24 +103,20 @@ export const ENV_PAGE_TARGETS: ReadonlyMap = new Map([ * kept as it was written, so both `/deeplink/waitpoints/waitpoint_123` and the longhand * `/deeplink/waitpoints/tokens/waitpoint_123` arrive at the same place. * - * Only the caller's own segments are encoded — the prefix is our own literal. They arrive decoded, - * so a "?" or "#" in a segment must not become the target's query or hash. + * `suffix` is expected already encoded (see `deeplinkSuffix`) and is passed through untouched — an + * `encodeURIComponent` pass here would double-encode every id that contains an escape. */ -export function resolveDeeplinkPage(splat: string): string | undefined { - //traversal segments are dropped so a crafted suffix can't climb out of the environment path - const segments = splat - .split("/") - .filter((segment) => segment.length > 0 && segment !== "." && segment !== ".."); +export function resolveDeeplinkPage(suffix: string): string | undefined { + const segments = suffix.split("/").filter(isUsableSegment); const target = ENV_PAGE_TARGETS.get(segments[0] ?? ""); if (target === undefined) return undefined; if (segments.length === 1) return target.landing; - const encoded = segments.map(encodeURIComponent); - const written = encoded.join("/"); + const written = segments.join("/"); //already written out under the prefix, so grafting would duplicate it if (written === target.prefix || written.startsWith(`${target.prefix}/`)) return written; - return [target.prefix, ...encoded.slice(1)].join("/"); + return [target.prefix, ...segments.slice(1)].join("/"); } From 71c6d3c9e044f2e86cad456899ac62ec37b510d9 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 18:52:59 +0000 Subject: [PATCH 08/12] Match a deeplink's prefix and page name the way the router does React Router compiles route paths with the `i` flag unless a route opts into `caseSensitive`, so /Deeplink/apikeys reaches this loader. The prefix strip required the literal lowercase /deeplink/, so the suffix came back empty and the link landed on the environment root instead of the page. The page-name lookup had the same problem one level down: /deeplink/APIKeys fell through even though /env/{env}/APIKeys would have matched. Fold the case of the prefix and of the first segment only, and resolve the name to the map's own spelling. Everything after the first segment is left exactly as written, since task and run ids are case-sensitive. --- apps/webapp/app/utils/deeplinkPages.test.ts | 57 ++++++++++++++++++++- apps/webapp/app/utils/deeplinkPages.ts | 24 ++++++--- 2 files changed, 74 insertions(+), 7 deletions(-) diff --git a/apps/webapp/app/utils/deeplinkPages.test.ts b/apps/webapp/app/utils/deeplinkPages.test.ts index ca7261665ec..41eb5a3a1d4 100644 --- a/apps/webapp/app/utils/deeplinkPages.test.ts +++ b/apps/webapp/app/utils/deeplinkPages.test.ts @@ -1,7 +1,13 @@ +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 { deeplinkSuffix, ENV_PAGE_TARGETS, resolveDeeplinkPage } from "./deeplinkPages"; +import { + DEEPLINK_PATH_PREFIX, + deeplinkSuffix, + ENV_PAGE_TARGETS, + resolveDeeplinkPage, +} from "./deeplinkPages"; const ROUTES_DIR = join(__dirname, "../routes"); @@ -181,6 +187,35 @@ describe("resolveDeeplinkPage", () => { expect(resolveDeeplinkPage("metrics")).toBeUndefined(); }); + it("matches the page name whatever its case, and resolves it to the map's spelling", () => { + // `/env/{env}/APIKeys` matches its route, so the short link has to agree rather than falling + // through to the environment root. + expect(resolveDeeplinkPage("APIKeys")).toBe("apikeys"); + expect(resolveDeeplinkPage("Waitpoints")).toBe("waitpoints/tokens"); + expect(resolveDeeplinkPage("TASKS")).toBe(""); + expect(resolveDeeplinkPage("Bulk-Actions")).toBe("bulk-actions"); + // Case doesn't turn a non-page into a page. + expect(resolveDeeplinkPage("Nonsense")).toBeUndefined(); + expect(resolveDeeplinkPage("Metrics")).toBeUndefined(); + }); + + it("leaves the case of everything after the name alone", () => { + // Only the name is folded. Ids are case-sensitive, so lowercasing one would break the link far + // more thoroughly than the miss the folding fixes. + 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"); + // Grafted onto the prefix and already written out under it, both with the id untouched. + expect(resolveDeeplinkPage("Waitpoints/waitpoint_ABC")).toBe("waitpoints/tokens/waitpoint_ABC"); + expect(resolveDeeplinkPage("Waitpoints/tokens/waitpoint_ABC")).toBe( + "waitpoints/tokens/waitpoint_ABC" + ); + // An escaped slash inside a capitalised id survives as one segment, as it does in lower case. + expect(resolveDeeplinkPage("Tasks/standard/Group%2FMy-Task")).toBe( + "tasks/standard/Group%2FMy-Task" + ); + }); + it("drops traversal segments, in plain and escaped spellings", () => { expect(resolveDeeplinkPage("runs/../../../etc/passwd")).toBe("runs/etc/passwd"); expect(resolveDeeplinkPage("../runs")).toBe("runs"); @@ -216,6 +251,26 @@ describe("deeplinkSuffix", () => { ); }); + it("strips the prefix whatever its case, and only the prefix", () => { + expect(deeplinkSuffix("/Deeplink/apikeys")).toBe("apikeys"); + expect(deeplinkSuffix("/DEEPLINK/runs/run_ABC123")).toBe("runs/run_ABC123"); + // The remainder comes back as it was written, capitals and all. + expect(deeplinkSuffix("/DeepLink/tasks/standard/My-Task")).toBe("tasks/standard/My-Task"); + expect(deeplinkSuffix("/Deeplink")).toBe(""); + expect(deeplinkSuffix("/Deeplink/")).toBe(""); + }); + + it("folds case because the route it is mounted on does", () => { + // The assertion the test above rests on: React Router compiles a route path with the `i` flag + // unless it opts into `caseSensitive`, so a capitalised prefix really does reach this loader + // instead of 404ing before it. If that ever changed, the folding would be dead weight. + const route = `${DEEPLINK_PATH_PREFIX}/*`; + expect(matchPath(route, "/deeplink/apikeys")?.params["*"]).toBe("apikeys"); + expect(matchPath(route, "/Deeplink/apikeys")?.params["*"]).toBe("apikeys"); + // And the splat keeps the case it was given, which is why only the first segment is folded. + expect(matchPath(route, "/DEEPLINK/APIKeys")?.params["*"]).toBe("APIKeys"); + }); + it("treats a bare prefix, a trailing slash and anything outside it as no suffix", () => { expect(deeplinkSuffix("/deeplink")).toBe(""); expect(deeplinkSuffix("/deeplink/")).toBe(""); diff --git a/apps/webapp/app/utils/deeplinkPages.ts b/apps/webapp/app/utils/deeplinkPages.ts index 9514610f1e5..7cfb4199d74 100644 --- a/apps/webapp/app/utils/deeplinkPages.ts +++ b/apps/webapp/app/utils/deeplinkPages.ts @@ -69,10 +69,15 @@ export const DEEPLINK_PATH_PREFIX = "/deeplink"; * Returns "" for anything that is not under the prefix. That includes a pathname the URL parser has * already rewritten: it normalises `%2e%2e` to `..` and resolves it, so a traversal attempt can * leave the prefix entirely before this ever sees it. + * + * The prefix is matched case-insensitively because React Router's route matching is: it compiles + * every path with the `i` flag unless the route opts into `caseSensitive`, so `/Deeplink/apikeys` + * reaches this loader too. Only the prefix is folded — the remainder is returned as it was written, + * since the ids after the first segment are case-sensitive. */ export function deeplinkSuffix(pathname: string): string { const withSlash = `${DEEPLINK_PATH_PREFIX}/`; - if (!pathname.startsWith(withSlash)) return ""; + if (!pathname.toLowerCase().startsWith(withSlash)) return ""; return pathname.slice(withSlash.length); } @@ -105,18 +110,25 @@ function isUsableSegment(segment: string): boolean { * * `suffix` is expected already encoded (see `deeplinkSuffix`) and is passed through untouched — an * `encodeURIComponent` pass here would double-encode every id that contains an escape. + * + * Only the first segment is matched case-insensitively, to the same end as the prefix in + * `deeplinkSuffix`: `/env/{env}/APIKeys` would have matched its route, so `/deeplink/APIKeys` should + * reach it rather than falling through to the environment root. The name resolves to the map's own + * spelling, and every segment after it is left exactly as written — folding the case of a task or + * run id would break the link far more thoroughly than the miss this fixes. */ export function resolveDeeplinkPage(suffix: string): string | undefined { - const segments = suffix.split("/").filter(isUsableSegment); + const [first = "", ...rest] = suffix.split("/").filter(isUsableSegment); - const target = ENV_PAGE_TARGETS.get(segments[0] ?? ""); + const name = first.toLowerCase(); + const target = ENV_PAGE_TARGETS.get(name); if (target === undefined) return undefined; - if (segments.length === 1) return target.landing; + if (rest.length === 0) return target.landing; - const written = segments.join("/"); + const written = [name, ...rest].join("/"); //already written out under the prefix, so grafting would duplicate it if (written === target.prefix || written.startsWith(`${target.prefix}/`)) return written; - return [target.prefix, ...segments.slice(1)].join("/"); + return [target.prefix, ...rest].join("/"); } From 6b2e0f97e52b1a085c29987f3f89f84a02d98dfe Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 19:02:57 +0000 Subject: [PATCH 09/12] Compare a written-out deeplink prefix with its case folded too Folding only the first segment left a prefix that spans more than one segment half-matched: `Waitpoints/Tokens/wp_123` did not equal `waitpoints/tokens`, so the graft branch fired on top of it and produced `waitpoints/tokens/Tokens/wp_123`, which matches no route. The all-lowercase spelling worked, so this was a regression the previous commit introduced. Compare as many leading segments as the prefix spans, lowercased, and return the prefix in the map's own spelling with everything past it exactly as written. Driven off `prefix` rather than special-cased for waitpoints, so a second multi-segment entry is covered when it is added. --- apps/webapp/app/utils/deeplinkPages.test.ts | 31 +++++++++++++++++++++ apps/webapp/app/utils/deeplinkPages.ts | 30 ++++++++++++-------- 2 files changed, 50 insertions(+), 11 deletions(-) diff --git a/apps/webapp/app/utils/deeplinkPages.test.ts b/apps/webapp/app/utils/deeplinkPages.test.ts index 41eb5a3a1d4..fc8b5f8126a 100644 --- a/apps/webapp/app/utils/deeplinkPages.test.ts +++ b/apps/webapp/app/utils/deeplinkPages.test.ts @@ -216,6 +216,37 @@ describe("resolveDeeplinkPage", () => { ); }); + it("recognises a written-out prefix whatever its case, however many segments it spans", () => { + // `waitpoints`' prefix is two segments, so folding only the first left `Tokens` looking like a + // segment of its own: the graft fired on top of it and produced waitpoints/tokens/Tokens/{id}, + // which matches no route. The lowercase spelling worked, so this was case-folding's own bug. + 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"); + // The bare longhand, with nothing beyond the prefix to carry. + expect(resolveDeeplinkPage("Waitpoints/Tokens")).toBe("waitpoints/tokens"); + }); + + it("holds for every multi-segment prefix in the map, not just waitpoints", () => { + // Driven off the map so a second such entry is covered the day it is added rather than the day + // someone notices. Every prefix segment is upper-cased and the id is left mixed. + const multiSegment = [...ENV_PAGE_TARGETS.values()].filter(({ prefix }) => + prefix.includes("/") + ); + + // Guards against this passing because it iterated nothing. + 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"); diff --git a/apps/webapp/app/utils/deeplinkPages.ts b/apps/webapp/app/utils/deeplinkPages.ts index 7cfb4199d74..500047616ab 100644 --- a/apps/webapp/app/utils/deeplinkPages.ts +++ b/apps/webapp/app/utils/deeplinkPages.ts @@ -111,24 +111,32 @@ function isUsableSegment(segment: string): boolean { * `suffix` is expected already encoded (see `deeplinkSuffix`) and is passed through untouched — an * `encodeURIComponent` pass here would double-encode every id that contains an escape. * - * Only the first segment is matched case-insensitively, to the same end as the prefix in - * `deeplinkSuffix`: `/env/{env}/APIKeys` would have matched its route, so `/deeplink/APIKeys` should - * reach it rather than falling through to the environment root. The name resolves to the map's own - * spelling, and every segment after it is left exactly as written — folding the case of a task or - * run id would break the link far more thoroughly than the miss this fixes. + * The name is matched case-insensitively, to the same end as the prefix in `deeplinkSuffix`: + * `/env/{env}/APIKeys` would have matched its route, so `/deeplink/APIKeys` should reach it rather + * than falling through to the environment root. So is the written-out prefix, which is why the + * comparison is against the lowercased path rather than the path itself — a prefix can be more than + * one segment (`waitpoints/tokens`), and reading only `Tokens` as a segment of its own would graft + * the prefix on top of it and produce `waitpoints/tokens/Tokens/{id}`. + * + * The prefix comes back in the map's own spelling and everything past it exactly as written, since + * folding the case of a task or run id would break the link far more thoroughly than the miss this + * fixes. */ export function resolveDeeplinkPage(suffix: string): string | undefined { - const [first = "", ...rest] = suffix.split("/").filter(isUsableSegment); + const segments = suffix.split("/").filter(isUsableSegment); + const [first = "", ...rest] = segments; - const name = first.toLowerCase(); - const target = ENV_PAGE_TARGETS.get(name); + const target = ENV_PAGE_TARGETS.get(first.toLowerCase()); if (target === undefined) return undefined; if (rest.length === 0) return target.landing; - const written = [name, ...rest].join("/"); + //however many segments the prefix spans, so the whole of it is compared and none of it re-grafted + const prefixDepth = target.prefix.split("/").length; + const writesPrefix = segments.slice(0, prefixDepth).join("/").toLowerCase() === target.prefix; + //already written out under the prefix, so grafting would duplicate it - if (written === target.prefix || written.startsWith(`${target.prefix}/`)) return written; + const beyondPrefix = writesPrefix ? segments.slice(prefixDepth) : rest; - return [target.prefix, ...rest].join("/"); + return [target.prefix, ...beyondPrefix].join("/"); } From f93a89278e4e12cd5fcee13df8b4b5147acf271d Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 10:31:42 +0000 Subject: [PATCH 10/12] Serve deeplinks from /_ instead of the deeplink literal Escape the underscore in the route filename so it stays a literal URL segment: a flat-route segment beginning with `_` is a pathless layout, so `_.$` would have mounted the route at `/*`. `createRoutePath` skips a segment only when its cooked and raw spellings both start with `_`, and the raw spelling of `[_]` does not, so `[_].$` serves `/_/*`. The prefix comparison no longer folds case. It existed because React Router matches routes case-insensitively and `/Deeplink/apikeys` really did reach the loader, but `_` has no case, so the fold is dead. Page-name folding stays, since `/_/APIKeys` still has a route to agree with. Drops `/deeplink/*` rather than keeping it as an alias; the route has not shipped, so nothing links to it yet. --- .server-changes/deeplink-routes.md | 2 +- .../app/routes/{deeplink.$.ts => [_].$.ts} | 4 +- apps/webapp/app/utils/deeplinkPages.test.ts | 52 ++++++++++-------- apps/webapp/app/utils/deeplinkPages.ts | 53 +++++++++++-------- 4 files changed, 64 insertions(+), 47 deletions(-) rename apps/webapp/app/routes/{deeplink.$.ts => [_].$.ts} (95%) diff --git a/.server-changes/deeplink-routes.md b/.server-changes/deeplink-routes.md index 56ab7c9bbc6..81367d655fb 100644 --- a/.server-changes/deeplink-routes.md +++ b/.server-changes/deeplink-routes.md @@ -3,4 +3,4 @@ area: webapp type: feature --- -Short links like /deeplink/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. +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. diff --git a/apps/webapp/app/routes/deeplink.$.ts b/apps/webapp/app/routes/[_].$.ts similarity index 95% rename from apps/webapp/app/routes/deeplink.$.ts rename to apps/webapp/app/routes/[_].$.ts index dd46fc73760..51e9b22a83a 100644 --- a/apps/webapp/app/routes/deeplink.$.ts +++ b/apps/webapp/app/routes/[_].$.ts @@ -12,10 +12,12 @@ import { } from "~/utils/pathBuilder"; /** - * Stable links that don't name an org, project or environment: /deeplink/apikeys redirects to + * Stable links that don't name an org, project or environment: /_/apikeys redirects to * /orgs/{org}/projects/{project}/env/{env}/apikeys for whoever is signed in. Only the pages in * ENV_PAGE_TARGETS are followed, so an unrecognised path can never become the redirect target — * it lands on the resolved environment instead. + * + * The filename escapes the underscore for a reason — see DEEPLINK_PATH_PREFIX. */ export const loader = async ({ request }: LoaderFunctionArgs) => { const user = await requireUser(request); diff --git a/apps/webapp/app/utils/deeplinkPages.test.ts b/apps/webapp/app/utils/deeplinkPages.test.ts index fc8b5f8126a..688cb7f927e 100644 --- a/apps/webapp/app/utils/deeplinkPages.test.ts +++ b/apps/webapp/app/utils/deeplinkPages.test.ts @@ -105,7 +105,7 @@ describe("deeplink targets", () => { }); it("every deep path lands on a real route", () => { - // The invariant a bare segment list could not express: /deeplink/waitpoints/{id} has to reach + // The invariant a bare segment list could not express: /_/waitpoints/{id} has to reach // waitpoints/tokens/{id}, not waitpoints/{id}. Driven off the real child routes rather than one // synthetic segment, so names whose children are all literal (settings/general) count too. const broken: string[] = []; @@ -272,54 +272,60 @@ describe("resolveDeeplinkPage", () => { describe("deeplinkSuffix", () => { it("strips the route's own prefix", () => { - expect(deeplinkSuffix("/deeplink/tasks")).toBe("tasks"); - expect(deeplinkSuffix("/deeplink/runs/run_123")).toBe("runs/run_123"); + 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("/deeplink/tasks/standard/group%2Fmy-task")).toBe( + expect(deeplinkSuffix("/_/tasks/standard/group%2Fmy-task")).toBe( "tasks/standard/group%2Fmy-task" ); }); - it("strips the prefix whatever its case, and only the prefix", () => { - expect(deeplinkSuffix("/Deeplink/apikeys")).toBe("apikeys"); - expect(deeplinkSuffix("/DEEPLINK/runs/run_ABC123")).toBe("runs/run_ABC123"); - // The remainder comes back as it was written, capitals and all. - expect(deeplinkSuffix("/DeepLink/tasks/standard/My-Task")).toBe("tasks/standard/My-Task"); - expect(deeplinkSuffix("/Deeplink")).toBe(""); - expect(deeplinkSuffix("/Deeplink/")).toBe(""); + it("strips only the prefix, leaving the remainder's case alone", () => { + // The prefix has no case to fold — `_` is the same character either way — so unlike the page + // name there is no case-insensitive comparison here. What still matters is that the remainder + // comes back exactly as written, capitals and all, because ids are case-sensitive. + expect(deeplinkSuffix("/_/runs/run_ABC123")).toBe("runs/run_ABC123"); + expect(deeplinkSuffix("/_/tasks/standard/My-Task")).toBe("tasks/standard/My-Task"); }); - it("folds case because the route it is mounted on does", () => { - // The assertion the test above rests on: React Router compiles a route path with the `i` flag - // unless it opts into `caseSensitive`, so a capitalised prefix really does reach this loader - // instead of 404ing before it. If that ever changed, the folding would be dead weight. + it("is mounted where the route filename says it is", () => { + // `[_].$` is an escaped literal, not a pathless layout: Remix's `createRoutePath` drops a + // segment only when the cooked and the raw spelling both start with `_`, and the raw spelling + // is `[_]`. A plain `_.$` would compile to `/*` and swallow the site, so this pins the prefix + // the loader strips to the URL the router actually serves. const route = `${DEEPLINK_PATH_PREFIX}/*`; - expect(matchPath(route, "/deeplink/apikeys")?.params["*"]).toBe("apikeys"); - expect(matchPath(route, "/Deeplink/apikeys")?.params["*"]).toBe("apikeys"); - // And the splat keeps the case it was given, which is why only the first segment is folded. - expect(matchPath(route, "/DEEPLINK/APIKeys")?.params["*"]).toBe("APIKeys"); + expect(route).toBe("/_/*"); + expect(matchPath(route, "/_/apikeys")?.params["*"]).toBe("apikeys"); + expect(matchPath(route, "/_/runs/run_123")?.params["*"]).toBe("runs/run_123"); + // The splat keeps the case it was given, which is why the loader folds only the page name. + expect(matchPath(route, "/_/APIKeys")?.params["*"]).toBe("APIKeys"); + // And it is a literal segment, so it matches nothing else. + 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("/deeplink")).toBe(""); - expect(deeplinkSuffix("/deeplink/")).toBe(""); + expect(deeplinkSuffix("/_")).toBe(""); + expect(deeplinkSuffix("/_/")).toBe(""); // What `new URL` leaves behind once it has normalised and resolved `%2e%2e` itself. expect(deeplinkSuffix("/etc")).toBe(""); + // A prefix that merely starts with the same character is not this route. + expect(deeplinkSuffix("/_app/orgs")).toBe(""); }); it("matches what the URL parser actually produces", () => { // The behaviour above is only correct if `new URL` really does keep %2F and really does // resolve %2e%2e, so assert that rather than assuming it. - const encodedSlash = new URL("http://x/deeplink/tasks/standard/group%2Fmy-task"); + 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" ); // `%2e%2e` is normalised to `..` and resolved by the parser, leaving the prefix behind. - const traversal = new URL("http://x/deeplink/runs/%2e%2e/%2e%2e/etc"); + const traversal = new URL("http://x/_/runs/%2e%2e/%2e%2e/etc"); expect(traversal.pathname).toBe("/etc"); expect(resolveDeeplinkPage(deeplinkSuffix(traversal.pathname))).toBeUndefined(); }); diff --git a/apps/webapp/app/utils/deeplinkPages.ts b/apps/webapp/app/utils/deeplinkPages.ts index 500047616ab..c200c61aeed 100644 --- a/apps/webapp/app/utils/deeplinkPages.ts +++ b/apps/webapp/app/utils/deeplinkPages.ts @@ -1,8 +1,8 @@ /** - * Where each /deeplink/ goes, relative to the resolved environment. + * Where each /_/ goes, relative to the resolved environment. * * Two pieces, because a name's own page and the things underneath it are not always in the same - * place. `landing` is used for a bare `/deeplink/`; `prefix` is what deeper segments hang off. + * place. `landing` is used for a bare `/_/`; `prefix` is what deeper segments hang off. * They differ only where a segment is not a page in its own right: * * - `tasks` has no bare route, and the task list is the environment root — but task detail pages do @@ -16,9 +16,9 @@ * route files and fails if a page is missing or a target stops resolving. */ export type DeeplinkTarget = { - /** Path for a bare `/deeplink/`. "" is the environment root. */ + /** Path for a bare `/_/`. "" is the environment root. */ landing: string; - /** Deeper segments are appended to this: `/deeplink//a/b` -> `/a/b`. */ + /** Deeper segments are appended to this: `/_//a/b` -> `/a/b`. */ prefix: string; }; @@ -57,12 +57,20 @@ export const ENV_PAGE_TARGETS: ReadonlyMap = new Map([ ["waitpoints", { landing: "waitpoints/tokens", prefix: "waitpoints/tokens" }], ]); -/** Where this route is mounted. Matches the `deeplink.$` route filename. */ -export const DEEPLINK_PATH_PREFIX = "/deeplink"; +/** + * Where this route is mounted. Matches the `[_].$` route filename. + * + * The brackets are Remix's escape, and they are load-bearing rather than decorative: a flat-route + * segment that starts with `_` is a pathless layout and contributes nothing to the URL, so a plain + * `_.$` would mount this at `/*` and swallow the whole site. Escaping the underscore makes it a + * literal segment — `createRoutePath` skips a segment only when the cooked *and* the raw spelling + * both start with `_`, and the raw spelling here is `[_]`, so `[_].$` really does serve `/_/*`. + */ +export const DEEPLINK_PATH_PREFIX = "/_"; /** - * The still-encoded suffix after /deeplink, taken from the request's pathname rather than the - * splat param. React Router decodes the splat, which turns an id containing an escaped slash + * The still-encoded suffix after /_, taken from the request's pathname rather than the splat param. + * React Router decodes the splat, which turns an id containing an escaped slash * (`group%2Fmy-task`, as the dashboard's own link builder writes it) into two segments that match * no route. The pathname keeps `%2F` intact. * @@ -70,14 +78,14 @@ export const DEEPLINK_PATH_PREFIX = "/deeplink"; * already rewritten: it normalises `%2e%2e` to `..` and resolves it, so a traversal attempt can * leave the prefix entirely before this ever sees it. * - * The prefix is matched case-insensitively because React Router's route matching is: it compiles - * every path with the `i` flag unless the route opts into `caseSensitive`, so `/Deeplink/apikeys` - * reaches this loader too. Only the prefix is folded — the remainder is returned as it was written, - * since the ids after the first segment are case-sensitive. + * The comparison is exact, unlike the page name's. React Router still matches the route + * case-insensitively, but `_` has no case for it to differ in, so there is nothing to fold. + * The remainder is returned as it was written, since the ids after the first segment are + * case-sensitive. */ export function deeplinkSuffix(pathname: string): string { const withSlash = `${DEEPLINK_PATH_PREFIX}/`; - if (!pathname.toLowerCase().startsWith(withSlash)) return ""; + if (!pathname.startsWith(withSlash)) return ""; return pathname.slice(withSlash.length); } @@ -103,20 +111,21 @@ function isUsableSegment(segment: string): boolean { * first segment names no page. Returns "" for a target that is the environment root itself. * * A bare name uses its landing path. Deeper segments are grafted onto the prefix, so - * `/deeplink/waitpoints/waitpoint_123` reaches the token that actually lives at + * `/_/waitpoints/waitpoint_123` reaches the token that actually lives at * `/waitpoints/tokens/waitpoint_123`. A suffix that already spells out a path under the prefix is - * kept as it was written, so both `/deeplink/waitpoints/waitpoint_123` and the longhand - * `/deeplink/waitpoints/tokens/waitpoint_123` arrive at the same place. + * kept as it was written, so both `/_/waitpoints/waitpoint_123` and the longhand + * `/_/waitpoints/tokens/waitpoint_123` arrive at the same place. * * `suffix` is expected already encoded (see `deeplinkSuffix`) and is passed through untouched — an * `encodeURIComponent` pass here would double-encode every id that contains an escape. * - * The name is matched case-insensitively, to the same end as the prefix in `deeplinkSuffix`: - * `/env/{env}/APIKeys` would have matched its route, so `/deeplink/APIKeys` should reach it rather - * than falling through to the environment root. So is the written-out prefix, which is why the - * comparison is against the lowercased path rather than the path itself — a prefix can be more than - * one segment (`waitpoints/tokens`), and reading only `Tokens` as a segment of its own would graft - * the prefix on top of it and produce `waitpoints/tokens/Tokens/{id}`. + * The name is matched case-insensitively because React Router's route matching is: it compiles + * every path with the `i` flag unless the route opts into `caseSensitive`, so `/env/{env}/APIKeys` + * would have matched its route, and `/_/APIKeys` should reach it rather than falling through to the + * environment root. So is the written-out prefix, which is why the comparison is against the + * lowercased path rather than the path itself — a prefix can be more than one segment + * (`waitpoints/tokens`), and reading only `Tokens` as a segment of its own would graft the prefix + * on top of it and produce `waitpoints/tokens/Tokens/{id}`. * * The prefix comes back in the map's own spelling and everything past it exactly as written, since * folding the case of a task or run id would break the link far more thoroughly than the miss this From 3de75c10090729ecf469280e7ceb4348c8a8779c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 11:05:48 +0000 Subject: [PATCH 11/12] Assert the deeplink route's path from Remix's own route manifest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous test built the route pattern out of DEEPLINK_PATH_PREFIX and then asserted that pattern equalled the same constant, so it could not fail if Remix compiled `[_].$.ts` to something else. That is the one thing worth guarding here: an unescaped `_` reads as a pathless layout, which would mount this loader at `/*` — a splat over the whole site whose loader redirects unconditionally. Run the real routes directory through the same `flatRoutes` the vite plugin uses, derive the mounted path from the manifest entry for the route file, and check the constant against that. Also assert no route anywhere compiles to a bare site-wide splat, which is the assertion that catches the failure independently of this route. Verified by mutation: unescaping the filename fails the suite, and a decoy pathless splat route fails the splat assertion. --- apps/webapp/app/utils/deeplinkPages.test.ts | 90 +++++++++++++++++++-- 1 file changed, 82 insertions(+), 8 deletions(-) diff --git a/apps/webapp/app/utils/deeplinkPages.test.ts b/apps/webapp/app/utils/deeplinkPages.test.ts index 688cb7f927e..cfe2e7649cc 100644 --- a/apps/webapp/app/utils/deeplinkPages.test.ts +++ b/apps/webapp/app/utils/deeplinkPages.test.ts @@ -1,3 +1,5 @@ +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"; @@ -9,7 +11,8 @@ import { resolveDeeplinkPage, } from "./deeplinkPages"; -const ROUTES_DIR = join(__dirname, "../routes"); +const APP_DIR = join(__dirname, ".."); +const ROUTES_DIR = join(APP_DIR, "routes"); // Flat-route prefix for every page that renders inside an environment. The trailing dot matters: // it excludes the layout route itself (`…env.$envParam`), which has no segment of its own. @@ -26,6 +29,40 @@ const NOT_DEEPLINK_NAMES = new Set(["_index", "queues_"]); /** Stands in for a param segment, so a deep path under test looks like a real URL. */ const PROBE = "probe_01ABC"; +/** The route module whose filename is what produces the deeplink URL. */ +const DEEPLINK_ROUTE_FILE = "routes/[_].$.ts"; + +/** + * The app's routes as Remix itself compiles them, so the URL under test is the one the router will + * really serve rather than one this file asserts into existence. `flatRoutes` is the same function + * the vite plugin calls, and the ignore list mirrors `ignoredRouteFiles` in `vite.config.ts`. + */ +const compiledRoutes: RouteManifest = flatRoutes(APP_DIR, ["**/.*"]); + +/** + * A route's whole URL, walking up the manifest — a `path` is relative to its parent's, and a + * pathless layout contributes nothing. Top-level routes name `root`, which the manifest omits. + */ +function compiledUrl(id: string): string { + // An unknown id would otherwise walk zero routes and quietly read as the site root. + 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("/")}`; +} + +/** What Remix actually mounts `[_].$.ts` at, e.g. `/_`. Derived, never assumed. */ +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); /** A route directory only contributes a route if it actually holds a `route` module. */ @@ -270,6 +307,45 @@ describe("resolveDeeplinkPage", () => { }); }); +describe("the route Remix compiles from the filename", () => { + // The escape in `[_].$.ts` is the whole reason this route works, and getting it wrong fails + // catastrophically rather than visibly: a flat-route segment starting with `_` is a *pathless + // layout* contributing nothing to the URL, so an unescaped `_.$.ts` would mount this loader at + // `/*` — a splat over the entire site whose loader redirects unconditionally. Nothing else in + // the app uses `[…]` escaping, so there is no precedent to lean on and the compiled manifest is + // the only honest source. Everything here is read out of `flatRoutes`, never asserted into it. + + it("mounts the deeplink route at /_ and nowhere else", () => { + expect(COMPILED_DEEPLINK_PATH).toBe("/_"); + // The constant the loader strips has to agree with what Remix mounted, so renaming either the + // file or the constant without the other fails here. + expect(COMPILED_DEEPLINK_PATH).toBe(DEEPLINK_PATH_PREFIX); + }); + + it("does not mount anything as a site-wide splat", () => { + // The disaster case, asserted for every route rather than just this one: had the underscore + // been read as pathless, this is the assertion that would have caught it. + const siteWide = Object.values(compiledRoutes) + .filter((route) => compiledUrl(route.id) === "/*") + .map((route) => route.file); + + expect(siteWide).toEqual([]); + }); + + it("compiled the manifest it is reading", () => { + // Keeps the two assertions above from passing because `flatRoutes` returned nothing useful. + expect(Object.keys(compiledRoutes).length).toBeGreaterThan(400); + // A sample of ordinary routes, so a manifest full of undefined paths would not read as a pass. + expect(compiledUrl("routes/login.magic")).toBe("/login/magic"); + // `queues_.$queueParam` opts out of the parent layout; the trailing `_` is not a URL character. + 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"); @@ -290,13 +366,11 @@ describe("deeplinkSuffix", () => { expect(deeplinkSuffix("/_/tasks/standard/My-Task")).toBe("tasks/standard/My-Task"); }); - it("is mounted where the route filename says it is", () => { - // `[_].$` is an escaped literal, not a pathless layout: Remix's `createRoutePath` drops a - // segment only when the cooked and the raw spelling both start with `_`, and the raw spelling - // is `[_]`. A plain `_.$` would compile to `/*` and swallow the site, so this pins the prefix - // the loader strips to the URL the router actually serves. - const route = `${DEEPLINK_PATH_PREFIX}/*`; - expect(route).toBe("/_/*"); + it("matches the URL the router serves for it", () => { + // Behaviour of the pattern itself. What the pattern *is* is settled against the compiled + // manifest above — building it from DEEPLINK_PATH_PREFIX alone would only compare the constant + // with itself. + const route = `${COMPILED_DEEPLINK_PATH}/*`; expect(matchPath(route, "/_/apikeys")?.params["*"]).toBe("apikeys"); expect(matchPath(route, "/_/runs/run_123")?.params["*"]).toBe("runs/run_123"); // The splat keeps the case it was given, which is why the loader folds only the page name. From f5050541797194b22dc1333f1461778b9638820c Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 7 Aug 2026 12:06:11 +0000 Subject: [PATCH 12/12] Cut the deeplink route's comments back to what the code can't say itself Delete the explanatory blocks and every comment that restated the line below it. What survives: why the route file's underscore is escaped, and why `.`/`..` segments are dropped. Rename `isUsableSegment` to `isSafeSegment` and put the intent that was commented into the test names instead. --- apps/webapp/app/routes/[_].$.ts | 16 +-- apps/webapp/app/utils/deeplinkPages.test.ts | 111 ++------------------ apps/webapp/app/utils/deeplinkPages.ts | 80 +------------- 3 files changed, 15 insertions(+), 192 deletions(-) diff --git a/apps/webapp/app/routes/[_].$.ts b/apps/webapp/app/routes/[_].$.ts index 51e9b22a83a..d5b0cedc89d 100644 --- a/apps/webapp/app/routes/[_].$.ts +++ b/apps/webapp/app/routes/[_].$.ts @@ -11,24 +11,13 @@ import { v3EnvironmentPath, } from "~/utils/pathBuilder"; -/** - * Stable links that don't name an org, project or environment: /_/apikeys redirects to - * /orgs/{org}/projects/{project}/env/{env}/apikeys for whoever is signed in. Only the pages in - * ENV_PAGE_TARGETS are followed, so an unrecognised path can never become the redirect target — - * it lands on the resolved environment instead. - * - * The filename escapes the underscore for a reason — see DEEPLINK_PATH_PREFIX. - */ +//`[_]` escapes the underscore: an unescaped `_.$` is a pathless layout, mounted at `/*`. export const loader = async ({ request }: LoaderFunctionArgs) => { const user = await requireUser(request); - //the suffix comes from the pathname, not the splat param, so an id containing an escaped slash - //survives — see deeplinkSuffix const { pathname, search } = new URL(request.url); const page = resolveDeeplinkPage(deeplinkSuffix(pathname)); - //a deeplink is the kind of URL a new invitee is sent, so take them to the invite first, exactly - //as the dashboard index does const invites = await getUsersInvites({ email: user.email }); if (invites.length > 0) { return redirect(invitesPath()); @@ -39,13 +28,10 @@ export const loader = async ({ request }: LoaderFunctionArgs) => { const { project, organization, environment } = await presenter.call({ user }); const environmentPath = v3EnvironmentPath(organization, project, environment); - //Both an unrecognised name and `tasks` (which targets the environment root) leave no suffix, - //and the query survives either way, so all three spellings of "the environment" agree. const suffix = page ? `/${page}` : ""; return redirect(`${environmentPath}${suffix}${search}`); } catch (_e) { - //the presenter throws when the user has no projects, same as the dashboard index const organization = await prisma.organization.findFirst({ where: { members: { diff --git a/apps/webapp/app/utils/deeplinkPages.test.ts b/apps/webapp/app/utils/deeplinkPages.test.ts index cfe2e7649cc..0d7e4737531 100644 --- a/apps/webapp/app/utils/deeplinkPages.test.ts +++ b/apps/webapp/app/utils/deeplinkPages.test.ts @@ -14,37 +14,19 @@ import { const APP_DIR = join(__dirname, ".."); const ROUTES_DIR = join(APP_DIR, "routes"); -// Flat-route prefix for every page that renders inside an environment. The trailing dot matters: -// it excludes the layout route itself (`…env.$envParam`), which has no segment of its own. +// 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."; -/** - * Segments that are route files but are not deeplink names: - * - `_index` is the environment root. It is where an unrecognised deeplink already lands, and - * `tasks` is the name that points at it. - * - `queues_` is Remix's "opt out of the parent layout" spelling of `queues`, not a distinct URL. - */ +// Route files that name no deeplink: the environment root, and Remix's layout-opt-out spelling. const NOT_DEEPLINK_NAMES = new Set(["_index", "queues_"]); -/** Stands in for a param segment, so a deep path under test looks like a real URL. */ const PROBE = "probe_01ABC"; -/** The route module whose filename is what produces the deeplink URL. */ const DEEPLINK_ROUTE_FILE = "routes/[_].$.ts"; -/** - * The app's routes as Remix itself compiles them, so the URL under test is the one the router will - * really serve rather than one this file asserts into existence. `flatRoutes` is the same function - * the vite plugin calls, and the ignore list mirrors `ignoredRouteFiles` in `vite.config.ts`. - */ const compiledRoutes: RouteManifest = flatRoutes(APP_DIR, ["**/.*"]); -/** - * A route's whole URL, walking up the manifest — a `path` is relative to its parent's, and a - * pathless layout contributes nothing. Top-level routes name `root`, which the manifest omits. - */ function compiledUrl(id: string): string { - // An unknown id would otherwise walk zero routes and quietly read as the site root. if (!compiledRoutes[id]) throw new Error(`no compiled route with id ${id}`); const segments: string[] = []; @@ -56,7 +38,6 @@ function compiledUrl(id: string): string { return `/${segments.join("/")}`; } -/** What Remix actually mounts `[_].$.ts` at, e.g. `/_`. Derived, never assumed. */ 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`); @@ -65,18 +46,13 @@ const COMPILED_DEEPLINK_PATH = (() => { const routeEntries = readdirSync(ROUTES_DIR); -/** A route directory only contributes a route if it actually holds a `route` module. */ 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")); } -/** - * Every environment route as its URL segments. A trailing `_index` is dropped (it supplies the - * parent's bare URL) and a trailing `_` is trimmed from each segment, because `queues_.$queueParam` - * serves `/queues/{id}` — the underscore only opts out of the parent layout. - */ +// 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) => @@ -88,7 +64,6 @@ const envRoutes: string[][] = routeEntries .map((segments) => (segments.at(-1) === "_index" ? segments.slice(0, -1) : segments)) .map((segments) => segments.map((segment) => segment.replace(/_+$/, ""))); -/** Does any route match this environment-relative URL? Param segments match the deep case only. */ function routeMatches(path: string, { allowParams }: { allowParams: boolean }): boolean { const wanted = path === "" ? [] : path.split("/"); return envRoutes.some( @@ -98,24 +73,17 @@ function routeMatches(path: string, { allowParams }: { allowParams: boolean }): ); } -/** Every first segment appearing under the environment layout. */ function envRouteSegments(): Set { const segments = new Set(); for (const entry of routeEntries) { if (!entry.startsWith(ENV_ROUTE_PREFIX)) continue; - // `metrics.$dashboardKey.ts` -> `metrics`, `agents` -> `agents`, `errors._index` -> `errors` const segment = entry.slice(ENV_ROUTE_PREFIX.length).split(/[./]/)[0]; - // Guards against a future `…env.$envParam.tsx` contributing its extension as a segment. if (!segment || segment === "ts" || segment === "tsx") continue; segments.add(segment); } return segments; } -/** - * Every route below this prefix, as the segments that follow it, with param segments replaced by a - * value a real URL would carry. These are the deep links that can actually be made under a name. - */ function descendantsOf(prefix: string): string[][] { const depth = prefix === "" ? 0 : prefix.split("/").length; return envRoutes @@ -126,14 +94,12 @@ function descendantsOf(prefix: string): string[][] { } describe("deeplink targets", () => { - it("found the routes directory", () => { - // Without this, every assertion below would pass vacuously if the glob ever broke. + 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", () => { - // A landing page is a page you arrive at with no id, so a param route does not count. + 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)"}`); @@ -141,10 +107,7 @@ describe("deeplink targets", () => { expect(broken).toEqual([]); }); - it("every deep path lands on a real route", () => { - // The invariant a bare segment list could not express: /_/waitpoints/{id} has to reach - // waitpoints/tokens/{id}, not waitpoints/{id}. Driven off the real child routes rather than one - // synthetic segment, so names whose children are all literal (settings/general) count too. + it("every deep path lands on a real route, prefix graft included", () => { const broken: string[] = []; for (const [name, { prefix }] of ENV_PAGE_TARGETS) { @@ -161,14 +124,12 @@ describe("deeplink targets", () => { }); it("has deep paths worth checking", () => { - // Keeps the assertion above from passing because it iterated nothing. 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", () => { - // A segment that resolves bare is a page someone could reasonably want to link to. const missing = [...envRouteSegments()] .filter((segment) => !NOT_DEEPLINK_NAMES.has(segment)) .filter( @@ -179,14 +140,11 @@ describe("deeplink targets", () => { expect(missing).toEqual([]); }); - it("names whose own segment 404s are redirected, not mapped to themselves", () => { - // These exist only as the parent of param/child routes, so a bare URL matches no route. + 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); } - // `tasks` and `waitpoints` therefore point elsewhere; `metrics` is only a legacy redirect shim - // with no page of its own, so it is deliberately not a deeplink name at all. expect(ENV_PAGE_TARGETS.get("tasks")).toEqual({ landing: "", prefix: "tasks" }); expect(ENV_PAGE_TARGETS.get("waitpoints")).toEqual({ landing: "waitpoints/tokens", @@ -205,9 +163,7 @@ describe("resolveDeeplinkPage", () => { it("grafts deeper segments onto the prefix", () => { expect(resolveDeeplinkPage("runs/run_123")).toBe("runs/run_123"); - // The landing is the environment root, but task detail still lives under /tasks. expect(resolveDeeplinkPage("tasks/standard/my-task")).toBe("tasks/standard/my-task"); - // The prefix supplies the `tokens` segment the caller did not have to know about. expect(resolveDeeplinkPage("waitpoints/waitpoint_123")).toBe("waitpoints/tokens/waitpoint_123"); }); @@ -225,53 +181,39 @@ describe("resolveDeeplinkPage", () => { }); it("matches the page name whatever its case, and resolves it to the map's spelling", () => { - // `/env/{env}/APIKeys` matches its route, so the short link has to agree rather than falling - // through to the environment root. expect(resolveDeeplinkPage("APIKeys")).toBe("apikeys"); expect(resolveDeeplinkPage("Waitpoints")).toBe("waitpoints/tokens"); expect(resolveDeeplinkPage("TASKS")).toBe(""); expect(resolveDeeplinkPage("Bulk-Actions")).toBe("bulk-actions"); - // Case doesn't turn a non-page into a page. expect(resolveDeeplinkPage("Nonsense")).toBeUndefined(); expect(resolveDeeplinkPage("Metrics")).toBeUndefined(); }); it("leaves the case of everything after the name alone", () => { - // Only the name is folded. Ids are case-sensitive, so lowercasing one would break the link far - // more thoroughly than the miss the folding fixes. 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"); - // Grafted onto the prefix and already written out under it, both with the id untouched. expect(resolveDeeplinkPage("Waitpoints/waitpoint_ABC")).toBe("waitpoints/tokens/waitpoint_ABC"); expect(resolveDeeplinkPage("Waitpoints/tokens/waitpoint_ABC")).toBe( "waitpoints/tokens/waitpoint_ABC" ); - // An escaped slash inside a capitalised id survives as one segment, as it does in lower case. 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", () => { - // `waitpoints`' prefix is two segments, so folding only the first left `Tokens` looking like a - // segment of its own: the graft fired on top of it and produced waitpoints/tokens/Tokens/{id}, - // which matches no route. The lowercase spelling worked, so this was case-folding's own bug. 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"); - // The bare longhand, with nothing beyond the prefix to carry. expect(resolveDeeplinkPage("Waitpoints/Tokens")).toBe("waitpoints/tokens"); }); it("holds for every multi-segment prefix in the map, not just waitpoints", () => { - // Driven off the map so a second such entry is covered the day it is added rather than the day - // someone notices. Every prefix segment is upper-cased and the id is left mixed. const multiSegment = [...ENV_PAGE_TARGETS.values()].filter(({ prefix }) => prefix.includes("/") ); - // Guards against this passing because it iterated nothing. expect(multiSegment.length).toBeGreaterThan(0); for (const { prefix } of multiSegment) { @@ -288,43 +230,28 @@ describe("resolveDeeplinkPage", () => { expect(resolveDeeplinkPage("runs/../../../etc/passwd")).toBe("runs/etc/passwd"); expect(resolveDeeplinkPage("../runs")).toBe("runs"); expect(resolveDeeplinkPage("runs//run_1")).toBe("runs/run_1"); - // `%2e%2e` decodes to `..`, so it has to be rejected in the encoded form too. expect(resolveDeeplinkPage("runs/%2e%2e/%2E%2E/run_1")).toBe("runs/run_1"); expect(resolveDeeplinkPage("runs/%2e/run_1")).toBe("runs/run_1"); - // A malformed escape can't be part of a URL we build. expect(resolveDeeplinkPage("runs/%ZZ/run_1")).toBe("runs/run_1"); }); it("passes encoded segments through without re-encoding them", () => { - // The dashboard writes a task id containing a slash this way, so it must survive as one - // segment rather than being split or double-encoded into %252F. expect(resolveDeeplinkPage("tasks/standard/group%2Fmy-task")).toBe( "tasks/standard/group%2Fmy-task" ); expect(resolveDeeplinkPage("runs/a%3Fb%23c")).toBe("runs/a%3Fb%23c"); - // An escaped slash stays escaped, so this addresses one odd id rather than climbing out. + // 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", () => { - // The escape in `[_].$.ts` is the whole reason this route works, and getting it wrong fails - // catastrophically rather than visibly: a flat-route segment starting with `_` is a *pathless - // layout* contributing nothing to the URL, so an unescaped `_.$.ts` would mount this loader at - // `/*` — a splat over the entire site whose loader redirects unconditionally. Nothing else in - // the app uses `[…]` escaping, so there is no precedent to lean on and the compiled manifest is - // the only honest source. Everything here is read out of `flatRoutes`, never asserted into it. - it("mounts the deeplink route at /_ and nowhere else", () => { expect(COMPILED_DEEPLINK_PATH).toBe("/_"); - // The constant the loader strips has to agree with what Remix mounted, so renaming either the - // file or the constant without the other fails here. expect(COMPILED_DEEPLINK_PATH).toBe(DEEPLINK_PATH_PREFIX); }); it("does not mount anything as a site-wide splat", () => { - // The disaster case, asserted for every route rather than just this one: had the underscore - // been read as pathless, this is the assertion that would have caught it. const siteWide = Object.values(compiledRoutes) .filter((route) => compiledUrl(route.id) === "/*") .map((route) => route.file); @@ -332,12 +259,9 @@ describe("the route Remix compiles from the filename", () => { expect(siteWide).toEqual([]); }); - it("compiled the manifest it is reading", () => { - // Keeps the two assertions above from passing because `flatRoutes` returned nothing useful. + it("compiled the manifest it is reading, paths and all", () => { expect(Object.keys(compiledRoutes).length).toBeGreaterThan(400); - // A sample of ordinary routes, so a manifest full of undefined paths would not read as a pass. expect(compiledUrl("routes/login.magic")).toBe("/login/magic"); - // `queues_.$queueParam` opts out of the parent layout; the trailing `_` is not a URL character. expect( compiledUrl( "routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.queues_.$queueParam" @@ -359,23 +283,15 @@ describe("deeplinkSuffix", () => { }); it("strips only the prefix, leaving the remainder's case alone", () => { - // The prefix has no case to fold — `_` is the same character either way — so unlike the page - // name there is no case-insensitive comparison here. What still matters is that the remainder - // comes back exactly as written, capitals and all, because ids are case-sensitive. 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", () => { - // Behaviour of the pattern itself. What the pattern *is* is settled against the compiled - // manifest above — building it from DEEPLINK_PATH_PREFIX alone would only compare the constant - // with itself. + 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"); - // The splat keeps the case it was given, which is why the loader folds only the page name. expect(matchPath(route, "/_/APIKeys")?.params["*"]).toBe("APIKeys"); - // And it is a literal segment, so it matches nothing else. expect(matchPath(route, "/deeplink/apikeys")).toBeNull(); expect(matchPath(route, "/apikeys")).toBeNull(); }); @@ -383,22 +299,17 @@ describe("deeplinkSuffix", () => { it("treats a bare prefix, a trailing slash and anything outside it as no suffix", () => { expect(deeplinkSuffix("/_")).toBe(""); expect(deeplinkSuffix("/_/")).toBe(""); - // What `new URL` leaves behind once it has normalised and resolved `%2e%2e` itself. expect(deeplinkSuffix("/etc")).toBe(""); - // A prefix that merely starts with the same character is not this route. expect(deeplinkSuffix("/_app/orgs")).toBe(""); }); - it("matches what the URL parser actually produces", () => { - // The behaviour above is only correct if `new URL` really does keep %2F and really does - // resolve %2e%2e, so assert that rather than assuming it. + 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" ); - // `%2e%2e` is normalised to `..` and resolved by the parser, leaving the prefix behind. const traversal = new URL("http://x/_/runs/%2e%2e/%2e%2e/etc"); expect(traversal.pathname).toBe("/etc"); expect(resolveDeeplinkPage(deeplinkSuffix(traversal.pathname))).toBeUndefined(); diff --git a/apps/webapp/app/utils/deeplinkPages.ts b/apps/webapp/app/utils/deeplinkPages.ts index c200c61aeed..bd626690f76 100644 --- a/apps/webapp/app/utils/deeplinkPages.ts +++ b/apps/webapp/app/utils/deeplinkPages.ts @@ -1,28 +1,8 @@ -/** - * Where each /_/ goes, relative to the resolved environment. - * - * Two pieces, because a name's own page and the things underneath it are not always in the same - * place. `landing` is used for a bare `/_/`; `prefix` is what deeper segments hang off. - * They differ only where a segment is not a page in its own right: - * - * - `tasks` has no bare route, and the task list is the environment root — but task detail pages do - * live under `/tasks`, so the landing is the root while the prefix stays `tasks`. - * - `waitpoints` has no bare route either, and its only page and its detail pages are both under - * `/waitpoints/tokens`, so both are that. - * - * This mirrors the environment-layout routes - * (`_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.*`): add a page here when one - * is added there. `deeplinkPages.test.ts` checks every landing and every deep path against the - * route files and fails if a page is missing or a target stops resolving. - */ export type DeeplinkTarget = { - /** Path for a bare `/_/`. "" is the environment root. */ landing: string; - /** Deeper segments are appended to this: `/_//a/b` -> `/a/b`. */ prefix: string; }; -/** An ordinary page: its own segment is the page, and its children hang off it. */ function page(name: string): DeeplinkTarget { return { landing: name, prefix: name }; } @@ -57,32 +37,8 @@ export const ENV_PAGE_TARGETS: ReadonlyMap = new Map([ ["waitpoints", { landing: "waitpoints/tokens", prefix: "waitpoints/tokens" }], ]); -/** - * Where this route is mounted. Matches the `[_].$` route filename. - * - * The brackets are Remix's escape, and they are load-bearing rather than decorative: a flat-route - * segment that starts with `_` is a pathless layout and contributes nothing to the URL, so a plain - * `_.$` would mount this at `/*` and swallow the whole site. Escaping the underscore makes it a - * literal segment — `createRoutePath` skips a segment only when the cooked *and* the raw spelling - * both start with `_`, and the raw spelling here is `[_]`, so `[_].$` really does serve `/_/*`. - */ export const DEEPLINK_PATH_PREFIX = "/_"; -/** - * The still-encoded suffix after /_, taken from the request's pathname rather than the splat param. - * React Router decodes the splat, which turns an id containing an escaped slash - * (`group%2Fmy-task`, as the dashboard's own link builder writes it) into two segments that match - * no route. The pathname keeps `%2F` intact. - * - * Returns "" for anything that is not under the prefix. That includes a pathname the URL parser has - * already rewritten: it normalises `%2e%2e` to `..` and resolves it, so a traversal attempt can - * leave the prefix entirely before this ever sees it. - * - * The comparison is exact, unlike the page name's. React Router still matches the route - * case-insensitively, but `_` has no case for it to differ in, so there is nothing to fold. - * The remainder is returned as it was written, since the ids after the first segment are - * case-sensitive. - */ export function deeplinkSuffix(pathname: string): string { const withSlash = `${DEEPLINK_PATH_PREFIX}/`; if (!pathname.startsWith(withSlash)) return ""; @@ -90,49 +46,22 @@ export function deeplinkSuffix(pathname: string): string { return pathname.slice(withSlash.length); } -/** Segments that must not reach the target path, tested in the encoded form we receive. */ -function isUsableSegment(segment: string): boolean { +//`.` and `..`, plain or escaped as `%2e%2e`, would climb out of the environment path. +function isSafeSegment(segment: string): boolean { if (segment.length === 0 || segment === "." || segment === "..") return false; let decoded: string; try { decoded = decodeURIComponent(segment); } catch { - //malformed escape, so it can't be part of a URL we build return false; } - //`%2e%2e` and friends, which would otherwise climb out of the environment path return decoded !== "." && decoded !== ".."; } -/** - * The path a deeplink suffix should redirect to, relative to the environment, or undefined when the - * first segment names no page. Returns "" for a target that is the environment root itself. - * - * A bare name uses its landing path. Deeper segments are grafted onto the prefix, so - * `/_/waitpoints/waitpoint_123` reaches the token that actually lives at - * `/waitpoints/tokens/waitpoint_123`. A suffix that already spells out a path under the prefix is - * kept as it was written, so both `/_/waitpoints/waitpoint_123` and the longhand - * `/_/waitpoints/tokens/waitpoint_123` arrive at the same place. - * - * `suffix` is expected already encoded (see `deeplinkSuffix`) and is passed through untouched — an - * `encodeURIComponent` pass here would double-encode every id that contains an escape. - * - * The name is matched case-insensitively because React Router's route matching is: it compiles - * every path with the `i` flag unless the route opts into `caseSensitive`, so `/env/{env}/APIKeys` - * would have matched its route, and `/_/APIKeys` should reach it rather than falling through to the - * environment root. So is the written-out prefix, which is why the comparison is against the - * lowercased path rather than the path itself — a prefix can be more than one segment - * (`waitpoints/tokens`), and reading only `Tokens` as a segment of its own would graft the prefix - * on top of it and produce `waitpoints/tokens/Tokens/{id}`. - * - * The prefix comes back in the map's own spelling and everything past it exactly as written, since - * folding the case of a task or run id would break the link far more thoroughly than the miss this - * fixes. - */ export function resolveDeeplinkPage(suffix: string): string | undefined { - const segments = suffix.split("/").filter(isUsableSegment); + const segments = suffix.split("/").filter(isSafeSegment); const [first = "", ...rest] = segments; const target = ENV_PAGE_TARGETS.get(first.toLowerCase()); @@ -140,11 +69,8 @@ export function resolveDeeplinkPage(suffix: string): string | undefined { if (rest.length === 0) return target.landing; - //however many segments the prefix spans, so the whole of it is compared and none of it re-grafted const prefixDepth = target.prefix.split("/").length; const writesPrefix = segments.slice(0, prefixDepth).join("/").toLowerCase() === target.prefix; - - //already written out under the prefix, so grafting would duplicate it const beyondPrefix = writesPrefix ? segments.slice(prefixDepth) : rest; return [target.prefix, ...beyondPrefix].join("/");