Skip to content

Commit 150629e

Browse files
committed
Guard every URL decode behind decodedOrNull
Routes every URL-reachable decodeURIComponent call in apps/web and packages/ through one shared, guarded helper instead of leaving each call site to throw on a malformed percent-escape: - New leaf package @corbits/url-path (no deps, safe in the browser bundle and hub code alike) holding decodedOrNull — packages/artifact-ui, packages/chat, and apps/hub can't depend on apps/web, so the helper can't stay local to path-ids.ts. - apps/web/src/path-ids.ts imports it instead of keeping its own copy. - apps/web/src/workbench-path.ts: workbenchIdFromPath, workbenchSettingsSectionFromPath (now also validated against WorkbenchSettingsSectionId via the new isWorkbenchSettingsSectionId, replacing the unchecked `as` cast — packages/chat-ui exports both the guard and its backing WORKBENCH_SETTINGS_SECTION_IDS list), workbenchSettingsEntityIdFromPath. - apps/web/src/insights-path.ts (moved out of the 1700-line insights-page.tsx, which now just imports it): parseInsightsPath's workbenchId/runId. A malformed escape now falls all the way back to {mode: "landing"} — returning {mode: "workbench", workbenchId: null} would render a bench-scoped dashboard mislabeled "All workbenches" with permanently-empty panels instead of the plain landing view every other unresolvable path already gets. - packages/artifact-ui/src/kind-filter.ts: libraryArtifactIdFromPath. - packages/chat/src/routes.ts: DELETE .../participants/:address now 400s on a malformed address instead of throwing mid-request. - apps/hub/src/index.ts's createStaticHandler (exported for the new test): the SPA static-file fallback ahead of every non-/api route, reachable unauthenticated — GET /%zz threw a 500 before this; now falls through to the platform's own 404 like any other unresolvable path. packages/connections/src/oauth-routes.ts's decode was already try/caught and is left as-is. Not in scope: apps/hub's dbConfigFromUrl decodes a database URL's own user/password at boot — operator-supplied config, never a client-reachable URL, so it's noted here rather than guarded (its shape is mirrored by six test doubles across apps/hub, packages/insights, packages/inbox, and packages/approvals, none of them production code either). Claude-Session: https://claude.ai/code/session_01Shhie5zM8L54bLHq5gFQti
1 parent d3fbfaf commit 150629e

22 files changed

Lines changed: 396 additions & 77 deletions

‎apps/hub/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
"@corbits/tasks": "workspace:*",
4545
"@corbits/tool-registry-publish": "workspace:*",
4646
"@corbits/turn-artifacts": "workspace:*",
47+
"@corbits/url-path": "workspace:*",
4748
"@corbits/webhook-triggers": "workspace:*",
4849
"@corbits/workflow-catalog": "workspace:*",
4950
"@corbits/workflow-source": "workspace:*",

‎apps/hub/src/index.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,7 @@ import {
9595
} from "@corbits/chat";
9696
import type { RelaunchNoticePort } from "@corbits/chat";
9797
import type { FinalizedTurnToolCall } from "@corbits/turn-artifacts";
98+
import { decodedOrNull } from "@corbits/url-path";
9899
import {
99100
createCryptoProviderCache,
100101
createTopLevelRunRoutes,
@@ -413,12 +414,12 @@ function dbConfigFromUrl(databaseUrl: string) {
413414
// Serves the single-page application from the hub origin: a real file
414415
// when one exists, index.html otherwise so client-side routes deep-link,
415416
// and never anything under /api, which stays with the platform routes.
416-
function createStaticHandler(staticDir: string) {
417+
export function createStaticHandler(staticDir: string) {
417418
return async (c: Context<AppEnv>, next: Next) => {
418419
if (c.req.path === "/api" || c.req.path.startsWith("/api/")) return next();
419-
const rel = path
420-
.normalize(decodeURIComponent(c.req.path))
421-
.replace(/^[/\\]+/, "");
420+
const decodedPath = decodedOrNull(c.req.path);
421+
if (decodedPath === null) return next();
422+
const rel = path.normalize(decodedPath).replace(/^[/\\]+/, "");
422423
if (rel === ".." || rel.startsWith(`..${path.sep}`)) return next();
423424
const asset = Bun.file(path.join(staticDir, rel));
424425
if (await asset.exists()) return new Response(asset);

‎apps/web/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
"@corbits/shell-layout": "workspace:*",
3838
"@corbits/slug": "workspace:*",
3939
"@corbits/tasks-ui": "workspace:*",
40+
"@corbits/url-path": "workspace:*",
4041
"@corbits/workflow-catalog": "workspace:*",
4142
"@corbits/icons": "workspace:*",
4243
"@intx/types": "0.3.0",

‎apps/web/src/bench-context.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,10 @@ export type BenchState = {
4545
readonly onBenchCreated: (tenantId: string) => void;
4646
};
4747

48-
const BenchContext = createContext<BenchState | null>(null);
48+
/** Exported only so a render test can inject a fixed `BenchState` without
49+
* standing up `BenchProvider`'s own `/api/me/principals` fetch — every
50+
* real caller still goes through `useBench`/`BenchProvider`. */
51+
export const BenchContext = createContext<BenchState | null>(null);
4952

5053
/** The membership this context currently treats as selected: the stored
5154
* choice if it still names a bench the account belongs to *and* still

‎apps/web/src/insights-path.ts‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// Insights' own route parser — its own module (not inlined in
2+
// insights-page.tsx) so it can be exercised directly without dragging in
3+
// that page's bench/query-client wiring.
4+
5+
import { decodedOrNull } from "@corbits/url-path";
6+
7+
import { INSIGHTS_PATH_PREFIX, INSIGHTS_RUNS_PATH } from "./path-ids";
8+
9+
/**
10+
* `/insights/workbench/:workbenchId` (CL-5879) is its own dedicated route — a
11+
* conversation's own scoped view, resolved by `InsightsWorkbenchPage`, never
12+
* a sub-mode of the landing. Every other path stays the cross-workbench
13+
* default landing: no per-mode branch needed there, since scoping happens
14+
* in InsightsRoute (which tenantId every query below targets), not here.
15+
* A stale `/insights/workbench/:tenantId` link (that route is retired,
16+
* hard cut) falls through to the plain landing default below rather than
17+
* matching anything.
18+
*
19+
* A malformed percent-escape in the id segment reads as the plain landing
20+
* default too, never as a detail mode with no entity to show — `mode:
21+
* "workbench"` (or `"run"`) with a `null` id would otherwise render that
22+
* mode's own scoped, permanently-empty dashboard instead of falling back
23+
* to the landing view any other unresolvable path already gets.
24+
*/
25+
export function parseInsightsPath(path: string): {
26+
mode: "landing" | "runs" | "run" | "workbench";
27+
runId: string | null;
28+
workbenchId: string | null;
29+
} {
30+
const workbenchMatch = /^\/insights\/workbench\/([^/]+)\/?$/.exec(path);
31+
if (workbenchMatch !== null && workbenchMatch[1] !== undefined) {
32+
const workbenchId = decodedOrNull(workbenchMatch[1]);
33+
if (workbenchId !== null) {
34+
return { mode: "workbench", runId: null, workbenchId };
35+
}
36+
}
37+
if (path === INSIGHTS_PATH_PREFIX || path === `${INSIGHTS_PATH_PREFIX}/`) {
38+
return { mode: "landing", runId: null, workbenchId: null };
39+
}
40+
if (path === INSIGHTS_RUNS_PATH || path === `${INSIGHTS_RUNS_PATH}/`) {
41+
return { mode: "runs", runId: null, workbenchId: null };
42+
}
43+
const match = /^\/insights\/runs\/([^/]+)\/?$/.exec(path);
44+
if (match !== null && match[1] !== undefined) {
45+
const runId = decodedOrNull(match[1]);
46+
if (runId !== null) {
47+
return { mode: "run", runId, workbenchId: null };
48+
}
49+
}
50+
return { mode: "landing", runId: null, workbenchId: null };
51+
}

‎apps/web/src/pages/insights-page.tsx‎

Lines changed: 1 addition & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ import {
6161
type WorkbenchInsightsResolution,
6262
} from "../insights-workbench-scope";
6363
import { workbenchInsightsPath } from "../insights-deeplinks";
64+
import { parseInsightsPath } from "../insights-path";
6465
import {
6566
ActivityResponseSchema,
6667
InsightsScopeSchema,
@@ -1077,46 +1078,6 @@ export function InsightsRunDetail({
10771078
);
10781079
}
10791080

1080-
/**
1081-
* `/insights/workbench/:workbenchId` (CL-5879) is its own dedicated route — a
1082-
* conversation's own scoped view, resolved by `InsightsWorkbenchPage`, never
1083-
* a sub-mode of the landing. Every other path stays the cross-workbench
1084-
* default landing: no per-mode branch needed there, since scoping happens
1085-
* in InsightsRoute (which tenantId every query below targets), not here.
1086-
* A stale `/insights/workbench/:tenantId` link (that route is retired,
1087-
* hard cut) falls through to the plain landing default below rather than
1088-
* matching anything.
1089-
*/
1090-
function parseInsightsPath(path: string): {
1091-
mode: "landing" | "runs" | "run" | "workbench";
1092-
runId: string | null;
1093-
workbenchId: string | null;
1094-
} {
1095-
const workbenchMatch = /^\/insights\/workbench\/([^/]+)\/?$/.exec(path);
1096-
if (workbenchMatch !== null && workbenchMatch[1] !== undefined) {
1097-
return {
1098-
mode: "workbench",
1099-
runId: null,
1100-
workbenchId: decodeURIComponent(workbenchMatch[1]),
1101-
};
1102-
}
1103-
if (path === INSIGHTS_PATH_PREFIX || path === `${INSIGHTS_PATH_PREFIX}/`) {
1104-
return { mode: "landing", runId: null, workbenchId: null };
1105-
}
1106-
if (path === INSIGHTS_RUNS_PATH || path === `${INSIGHTS_RUNS_PATH}/`) {
1107-
return { mode: "runs", runId: null, workbenchId: null };
1108-
}
1109-
const match = /^\/insights\/runs\/([^/]+)\/?$/.exec(path);
1110-
if (match !== null && match[1] !== undefined) {
1111-
return {
1112-
mode: "run",
1113-
runId: decodeURIComponent(match[1]),
1114-
workbenchId: null,
1115-
};
1116-
}
1117-
return { mode: "landing", runId: null, workbenchId: null };
1118-
}
1119-
11201081
/**
11211082
* The landing view's default scope, and every non-landing mode's scope,
11221083
* as one pure decision so it can be unit-tested without mounting the

‎apps/web/src/path-ids.ts‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
// below.
99

1010
import { isValidSlug, type Slug } from "@corbits/slug";
11+
import { decodedOrNull } from "@corbits/url-path";
1112

1213
export const SETTINGS_PATH_PREFIX = "/settings";
1314
export const AGENTS_PATH_PREFIX = "/agents";
@@ -43,19 +44,6 @@ function rawSegmentFromTopLevelPath(
4344
return rest === "" ? null : rest;
4445
}
4546

46-
/** A URL segment carries percent-escapes an id needs decoded, and a
47-
* hand-typed or truncated URL can carry a malformed one — which
48-
* `decodeURIComponent` answers with a throw. A path that cannot be decoded
49-
* names no entity, so it reads as no selection at all rather than taking
50-
* the render down with it. */
51-
function decodedOrNull(segment: string): string | null {
52-
try {
53-
return decodeURIComponent(segment);
54-
} catch {
55-
return null;
56-
}
57-
}
58-
5947
/** Extract a sub-selection from a flat top-level route (`/agents/:id`,
6048
* `/skills/:id`) — `null` for the bare prefix or a path outside it. */
6149
function entityIdFromTopLevelPath(path: string, prefix: string): string | null {

‎apps/web/src/workbench-path.ts‎

Lines changed: 19 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,11 @@
88
// `/settings/:section` and `settingsEntityIdFromPath` for the app-level
99
// Settings page.
1010

11-
import type { WorkbenchSettingsSectionId } from "@corbits/chat-ui";
11+
import {
12+
isWorkbenchSettingsSectionId,
13+
type WorkbenchSettingsSectionId,
14+
} from "@corbits/chat-ui";
15+
import { decodedOrNull } from "@corbits/url-path";
1216

1317
export const WORKBENCH_PATH_PREFIX = "/w";
1418
const LEGACY_CHAT_PATH_PREFIX = "/chat";
@@ -33,7 +37,7 @@ export function workbenchIdFromPath(path: string): string | null {
3337
if (!base.startsWith(`${prefix}/`)) continue;
3438
const rest = base.slice(prefix.length + 1);
3539
if (rest === "") return null;
36-
return decodeURIComponent(rest);
40+
return decodedOrNull(rest);
3741
}
3842
return null;
3943
}
@@ -83,12 +87,13 @@ export function workbenchSettingsPath(
8387

8488
/** Extract the section id from `/w/:id/settings/:section` (or
8589
* `/settings/:section/:entityId`, or its legacy `/chat` equivalent) —
86-
* `undefined` for bare `/settings` or a non-settings path. Only the first
87-
* segment after `/settings/` is the section, so a trailing entity id does
88-
* not change the section. Not validated against the known section ids: the
89-
* settings surface already falls back to its first section for an id it
90-
* doesn't recognize, the same contract `settingsSectionIdFromPath` in
91-
* `path-ids.ts` relies on its caller for. */
90+
* `undefined` for bare `/settings`, a non-settings path, a malformed
91+
* escape, or a segment that isn't one of `WorkbenchSettingsSectionId`'s own
92+
* values. Only the first segment after `/settings/` is the section, so a
93+
* trailing entity id does not change the section. An unrecognized id reads
94+
* the same as no section at all: the settings surface already falls back
95+
* to its first section then, the same contract `settingsSectionIdFromPath`
96+
* in `path-ids.ts` relies on its caller for. */
9297
export function workbenchSettingsSectionFromPath(
9398
path: string,
9499
): WorkbenchSettingsSectionId | undefined {
@@ -99,7 +104,11 @@ export function workbenchSettingsSectionFromPath(
99104
if (rest === "") return undefined;
100105
const section = rest.split("/")[0];
101106
if (section === undefined || section === "") return undefined;
102-
return decodeURIComponent(section) as WorkbenchSettingsSectionId;
107+
const decoded = decodedOrNull(section);
108+
if (decoded === null || !isWorkbenchSettingsSectionId(decoded)) {
109+
return undefined;
110+
}
111+
return decoded;
103112
}
104113

105114
/** Extract a section's own sub-selection from
@@ -116,5 +125,5 @@ export function workbenchSettingsEntityIdFromPath(
116125
const index = path.indexOf(sectionPrefix);
117126
if (index === -1) return null;
118127
const rest = path.slice(index + sectionPrefix.length);
119-
return rest === "" ? null : decodeURIComponent(rest);
128+
return rest === "" ? null : decodedOrNull(rest);
120129
}

0 commit comments

Comments
 (0)