Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/hub/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
"@corbits/tasks": "workspace:*",
"@corbits/tool-registry-publish": "workspace:*",
"@corbits/turn-artifacts": "workspace:*",
"@corbits/url-path": "workspace:*",
"@corbits/webhook-triggers": "workspace:*",
"@corbits/workflow-catalog": "workspace:*",
"@corbits/workflow-source": "workspace:*",
Expand Down
9 changes: 5 additions & 4 deletions apps/hub/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ import {
} from "@corbits/chat";
import type { RelaunchNoticePort } from "@corbits/chat";
import type { FinalizedTurnToolCall } from "@corbits/turn-artifacts";
import { decodedOrNull } from "@corbits/url-path";
import {
createCryptoProviderCache,
createTopLevelRunRoutes,
Expand Down Expand Up @@ -413,12 +414,12 @@ function dbConfigFromUrl(databaseUrl: string) {
// Serves the single-page application from the hub origin: a real file
// when one exists, index.html otherwise so client-side routes deep-link,
// and never anything under /api, which stays with the platform routes.
function createStaticHandler(staticDir: string) {
export function createStaticHandler(staticDir: string) {
return async (c: Context<AppEnv>, next: Next) => {
if (c.req.path === "/api" || c.req.path.startsWith("/api/")) return next();
const rel = path
.normalize(decodeURIComponent(c.req.path))
.replace(/^[/\\]+/, "");
const decodedPath = decodedOrNull(c.req.path);
if (decodedPath === null) return next();
const rel = path.normalize(decodedPath).replace(/^[/\\]+/, "");
if (rel === ".." || rel.startsWith(`..${path.sep}`)) return next();
const asset = Bun.file(path.join(staticDir, rel));
if (await asset.exists()) return new Response(asset);
Expand Down
55 changes: 55 additions & 0 deletions apps/hub/test/static-handler.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// `createStaticHandler` serves the SPA from the hub origin and sits ahead
// of every non-/api route, so it sees any attacker-controlled path a
// client cares to send — including one with a malformed percent-escape
// (`GET /%zz`). `decodeURIComponent` throws on that, and until this was
// guarded the throw escaped the handler entirely: a 500 on any
// non-/api request, not the 404/index.html fallback every other
// unresolvable path gets.

import { afterAll, beforeAll, describe, expect, test } from "bun:test";
import { mkdtempSync, rmSync, writeFileSync } from "node:fs";
import { tmpdir } from "node:os";
import path from "node:path";
import { Hono } from "hono";
import type { Context } from "hono";

import { createStaticHandler } from "../src/index.ts";

let staticDir: string;

beforeAll(() => {
staticDir = mkdtempSync(path.join(tmpdir(), "hub-static-"));
writeFileSync(path.join(staticDir, "index.html"), "<html>spa</html>");
});

afterAll(() => {
rmSync(staticDir, { recursive: true, force: true });
});

function buildApp() {
const app = new Hono();
app.get("/*", createStaticHandler(staticDir) as never);
app.notFound((c: Context) => c.text("not found", 404));
return app;
}

describe("createStaticHandler", () => {
test("serves index.html for a normal SPA route", async () => {
const app = buildApp();
const response = await app.request("/workbenches/ch_1");
expect(response.status).toBe(200);
expect(await response.text()).toBe("<html>spa</html>");
});

test("a malformed percent-escape 404s instead of throwing", async () => {
const app = buildApp();
const response = await app.request("/%zz");
expect(response.status).toBe(404);
});

test("/api paths are left for the platform routes regardless", async () => {
const app = buildApp();
const response = await app.request("/api/%zz");
expect(response.status).toBe(404);
});
});
1 change: 1 addition & 0 deletions apps/web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
"@corbits/shell-layout": "workspace:*",
"@corbits/slug": "workspace:*",
"@corbits/tasks-ui": "workspace:*",
"@corbits/url-path": "workspace:*",
"@corbits/workflow-catalog": "workspace:*",
"@corbits/icons": "workspace:*",
"@intx/types": "0.3.0",
Expand Down
5 changes: 4 additions & 1 deletion apps/web/src/bench-context.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,10 @@ export type BenchState = {
readonly onBenchCreated: (tenantId: string) => void;
};

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

/** The membership this context currently treats as selected: the stored
* choice if it still names a bench the account belongs to *and* still
Expand Down
54 changes: 54 additions & 0 deletions apps/web/src/insights-path.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { describe, expect, test } from "bun:test";

import { parseInsightsPath } from "./insights-path";

describe("parseInsightsPath", () => {
test("resolves a workbench deep link", () => {
expect(parseInsightsPath("/insights/workbench/tnt_1")).toEqual({
mode: "workbench",
runId: null,
workbenchId: "tnt_1",
});
});

test("resolves a run deep link", () => {
expect(parseInsightsPath("/insights/runs/run_1")).toEqual({
mode: "run",
runId: "run_1",
workbenchId: null,
});
});

test("resolves the landing and runs-history paths", () => {
expect(parseInsightsPath("/insights")).toEqual({
mode: "landing",
runId: null,
workbenchId: null,
});
expect(parseInsightsPath("/insights/runs")).toEqual({
mode: "runs",
runId: null,
workbenchId: null,
});
});

test("a malformed escape on a workbench deep link falls back to landing, not a throw", () => {
expect(() =>
parseInsightsPath("/insights/workbench/%E0%A4%A"),
).not.toThrow();
expect(parseInsightsPath("/insights/workbench/%E0%A4%A")).toEqual({
mode: "landing",
runId: null,
workbenchId: null,
});
});

test("a malformed escape on a run deep link falls back to landing, not a throw", () => {
expect(() => parseInsightsPath("/insights/runs/%")).not.toThrow();
expect(parseInsightsPath("/insights/runs/%")).toEqual({
mode: "landing",
runId: null,
workbenchId: null,
});
});
});
51 changes: 51 additions & 0 deletions apps/web/src/insights-path.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Insights' own route parser — its own module (not inlined in
// insights-page.tsx) so it can be exercised directly without dragging in
// that page's bench/query-client wiring.

import { decodedOrNull } from "@corbits/url-path";

import { INSIGHTS_PATH_PREFIX, INSIGHTS_RUNS_PATH } from "./path-ids";

/**
* `/insights/workbench/:workbenchId` (CL-5879) is its own dedicated route — a
* conversation's own scoped view, resolved by `InsightsWorkbenchPage`, never
* a sub-mode of the landing. Every other path stays the cross-workbench
* default landing: no per-mode branch needed there, since scoping happens
* in InsightsRoute (which tenantId every query below targets), not here.
* A stale `/insights/workbench/:tenantId` link (that route is retired,
* hard cut) falls through to the plain landing default below rather than
* matching anything.
*
* A malformed percent-escape in the id segment reads as the plain landing
* default too, never as a detail mode with no entity to show — `mode:
* "workbench"` (or `"run"`) with a `null` id would otherwise render that
* mode's own scoped, permanently-empty dashboard instead of falling back
* to the landing view any other unresolvable path already gets.
*/
export function parseInsightsPath(path: string): {
mode: "landing" | "runs" | "run" | "workbench";
runId: string | null;
workbenchId: string | null;
} {
const workbenchMatch = /^\/insights\/workbench\/([^/]+)\/?$/.exec(path);
if (workbenchMatch !== null && workbenchMatch[1] !== undefined) {
const workbenchId = decodedOrNull(workbenchMatch[1]);
if (workbenchId !== null) {
return { mode: "workbench", runId: null, workbenchId };
}
}
if (path === INSIGHTS_PATH_PREFIX || path === `${INSIGHTS_PATH_PREFIX}/`) {
return { mode: "landing", runId: null, workbenchId: null };
}
if (path === INSIGHTS_RUNS_PATH || path === `${INSIGHTS_RUNS_PATH}/`) {
return { mode: "runs", runId: null, workbenchId: null };
}
const match = /^\/insights\/runs\/([^/]+)\/?$/.exec(path);
if (match !== null && match[1] !== undefined) {
const runId = decodedOrNull(match[1]);
if (runId !== null) {
return { mode: "run", runId, workbenchId: null };
}
}
return { mode: "landing", runId: null, workbenchId: null };
}
93 changes: 93 additions & 0 deletions apps/web/src/pages/insights-page-render.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
// A malformed percent-escape on an Insights deep link
// (`/insights/workbench/%E0%A4%A`, `/insights/runs/%`) must render the
// same landing dashboard any other unrecognized Insights path gets — never
// a blank page, and never `InsightsWorkbenchPage`/the run-detail route with
// no entity to show (see `insights-path.ts`'s `parseInsightsPath`, which
// InsightsPage calls with the exact same `path` prop this test passes).

import { afterEach, describe, expect, test } from "bun:test";
import { act } from "react";
import { createRoot } from "react-dom/client";
import type { Root } from "react-dom/client";

import type { APIQuery } from "@corbits/api-query";
import { EMPTY_OVERALL_USAGE } from "@corbits/insights/client";

import { InsightsPage, useInsightsWindow } from "./insights-page";
import { BenchContext } from "../bench-context";
import type { BenchState } from "../bench-context";
import { NavigationProvider } from "../navigation";

let container: HTMLDivElement | null = null;
let root: Root | null = null;

afterEach(() => {
if (root !== null) {
act(() => root?.unmount());
root = null;
}
if (container !== null) {
container.remove();
container = null;
}
});

const readyEmpty = <T,>(data: T): APIQuery<T> => ({ kind: "ready", data });

const benchState: BenchState = {
memberships: { kind: "ready", data: { data: [], nextCursor: null } },
selectedTenantId: "tnt_bench_a",
selectedPrincipalId: "prn_bench_a",
selectTenant: () => {},
onBenchCreated: () => {},
};

function InsightsPageAtPath({ path }: { readonly path: string }) {
const range = useInsightsWindow();
return (
<NavigationProvider navigate={() => {}}>
<BenchContext.Provider value={benchState}>
<InsightsPage
path={path}
summary={readyEmpty(EMPTY_OVERALL_USAGE)}
activity={readyEmpty([])}
byTool={readyEmpty([])}
runs={readyEmpty({ data: [], nextCursor: null })}
routines={readyEmpty([])}
workbenches={readyEmpty({ items: [] })}
latency={{ kind: "loading" }}
range={range}
scope={null}
resolveWorkbenchIdForTenant={() => null}
scopeLabel="All workbenches"
/>
</BenchContext.Provider>
</NavigationProvider>
);
}

function render(path: string) {
container = document.createElement("div");
document.body.appendChild(container);
root = createRoot(container);
act(() => {
root?.render(<InsightsPageAtPath path={path} />);
});
return container;
}

describe("InsightsPage with a malformed URL escape", () => {
test("a malformed workbench deep link still renders the landing dashboard", () => {
const el = render("/insights/workbench/%E0%A4%A");
expect(el.textContent).not.toBe("");
expect(el.textContent).toContain("Insights");
expect(el.textContent).toContain("All workbenches");
});

test("a malformed run deep link still renders the landing dashboard, not run detail", () => {
const el = render("/insights/runs/%");
expect(el.textContent).not.toBe("");
expect(el.textContent).toContain("Insights");
expect(el.textContent).toContain("All workbenches");
});
});
41 changes: 1 addition & 40 deletions apps/web/src/pages/insights-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ import {
type WorkbenchInsightsResolution,
} from "../insights-workbench-scope";
import { workbenchInsightsPath } from "../insights-deeplinks";
import { parseInsightsPath } from "../insights-path";
import {
ActivityResponseSchema,
InsightsScopeSchema,
Expand Down Expand Up @@ -1077,46 +1078,6 @@ export function InsightsRunDetail({
);
}

/**
* `/insights/workbench/:workbenchId` (CL-5879) is its own dedicated route — a
* conversation's own scoped view, resolved by `InsightsWorkbenchPage`, never
* a sub-mode of the landing. Every other path stays the cross-workbench
* default landing: no per-mode branch needed there, since scoping happens
* in InsightsRoute (which tenantId every query below targets), not here.
* A stale `/insights/workbench/:tenantId` link (that route is retired,
* hard cut) falls through to the plain landing default below rather than
* matching anything.
*/
function parseInsightsPath(path: string): {
mode: "landing" | "runs" | "run" | "workbench";
runId: string | null;
workbenchId: string | null;
} {
const workbenchMatch = /^\/insights\/workbench\/([^/]+)\/?$/.exec(path);
if (workbenchMatch !== null && workbenchMatch[1] !== undefined) {
return {
mode: "workbench",
runId: null,
workbenchId: decodeURIComponent(workbenchMatch[1]),
};
}
if (path === INSIGHTS_PATH_PREFIX || path === `${INSIGHTS_PATH_PREFIX}/`) {
return { mode: "landing", runId: null, workbenchId: null };
}
if (path === INSIGHTS_RUNS_PATH || path === `${INSIGHTS_RUNS_PATH}/`) {
return { mode: "runs", runId: null, workbenchId: null };
}
const match = /^\/insights\/runs\/([^/]+)\/?$/.exec(path);
if (match !== null && match[1] !== undefined) {
return {
mode: "run",
runId: decodeURIComponent(match[1]),
workbenchId: null,
};
}
return { mode: "landing", runId: null, workbenchId: null };
}

/**
* The landing view's default scope, and every non-landing mode's scope,
* as one pure decision so it can be unit-tested without mounting the
Expand Down
14 changes: 1 addition & 13 deletions apps/web/src/path-ids.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
// below.

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

export const SETTINGS_PATH_PREFIX = "/settings";
export const AGENTS_PATH_PREFIX = "/agents";
Expand Down Expand Up @@ -43,19 +44,6 @@ function rawSegmentFromTopLevelPath(
return rest === "" ? null : rest;
}

/** A URL segment carries percent-escapes an id needs decoded, and a
* hand-typed or truncated URL can carry a malformed one — which
* `decodeURIComponent` answers with a throw. A path that cannot be decoded
* names no entity, so it reads as no selection at all rather than taking
* the render down with it. */
function decodedOrNull(segment: string): string | null {
try {
return decodeURIComponent(segment);
} catch {
return null;
}
}

/** Extract a sub-selection from a flat top-level route (`/agents/:id`,
* `/skills/:id`) — `null` for the bare prefix or a path outside it. */
function entityIdFromTopLevelPath(path: string, prefix: string): string | null {
Expand Down
Loading
Loading