fix: harden workflow console rpc boundary
This commit is contained in:
@@ -10,11 +10,12 @@
|
||||
"preview": "vite preview --host 127.0.0.1"
|
||||
},
|
||||
"dependencies": {
|
||||
"@fontsource/barlow-condensed": "5.2.8",
|
||||
"@fontsource-variable/source-sans-3": "5.2.9",
|
||||
"@fontsource/barlow-condensed": "5.2.8",
|
||||
"@fontsource/ibm-plex-mono": "5.2.7",
|
||||
"react": "19.2.7",
|
||||
"react-dom": "19.2.7"
|
||||
"react-dom": "19.2.7",
|
||||
"valibot": "1.4.2"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@testing-library/jest-dom": "6.9.1",
|
||||
|
||||
@@ -58,7 +58,7 @@ export const App = () => {
|
||||
id: `sources-${Date.now()}`,
|
||||
operation: "workflow.sources.list",
|
||||
label: "Source inventory",
|
||||
equivalentCli: "uv run wf sources list --limit 50",
|
||||
equivalentCli: result.equivalentCli,
|
||||
request: result.exchange.request,
|
||||
response: result.exchange.response,
|
||||
durationMs: result.durationMs,
|
||||
@@ -72,7 +72,7 @@ export const App = () => {
|
||||
id: `sources-${Date.now()}`,
|
||||
operation: "workflow.sources.list",
|
||||
label: "Source inventory",
|
||||
equivalentCli: "uv run wf sources list --limit 50",
|
||||
equivalentCli: "uv run wf source list --limit 50",
|
||||
request: result.exchange.request,
|
||||
response: result.exchange.response,
|
||||
durationMs: 0,
|
||||
|
||||
@@ -45,25 +45,35 @@ describe("initialState", () => {
|
||||
expect(state.draftTarget).toBe("http://127.0.0.1:8765/rpc");
|
||||
});
|
||||
|
||||
it("restores target from localStorage when available", () => {
|
||||
it("restores target from sessionStorage when available", () => {
|
||||
try {
|
||||
localStorage.setItem(STORAGE_KEY, "http://custom:9999/rpc");
|
||||
sessionStorage.setItem(STORAGE_KEY, "http://custom:9999/rpc");
|
||||
} catch {
|
||||
return; // skip if localStorage unavailable
|
||||
return; // skip if sessionStorage unavailable
|
||||
}
|
||||
const state = initialState();
|
||||
expect(state.draftTarget).toBe("http://custom:9999/rpc");
|
||||
});
|
||||
|
||||
it("restored target does not trigger automatic connection", () => {
|
||||
try {
|
||||
sessionStorage.setItem(STORAGE_KEY, "http://custom:9999/rpc");
|
||||
} catch {
|
||||
return; // skip if sessionStorage unavailable
|
||||
}
|
||||
const state = initialState();
|
||||
expect(state.phase).toBe("not_configured");
|
||||
expect(state.connectedTarget).toBeNull();
|
||||
});
|
||||
|
||||
it("ignores localStorage so persistence is session-scoped", () => {
|
||||
try {
|
||||
localStorage.setItem(STORAGE_KEY, "http://custom:9999/rpc");
|
||||
} catch {
|
||||
return; // skip if localStorage unavailable
|
||||
}
|
||||
const state = initialState();
|
||||
expect(state.phase).toBe("not_configured");
|
||||
expect(state.connectedTarget).toBeNull();
|
||||
expect(state.draftTarget).toBe("http://127.0.0.1:8765/rpc");
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -47,14 +47,6 @@ export type ConnectionState = {
|
||||
|
||||
export const STORAGE_KEY = "lda.workflowConsole.target";
|
||||
|
||||
const safeLocalStorage = (): Storage | null => {
|
||||
try {
|
||||
return typeof localStorage !== "undefined" ? localStorage : null;
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
};
|
||||
|
||||
const safeSessionStorage = (): Storage | null => {
|
||||
try {
|
||||
return typeof sessionStorage !== "undefined" ? sessionStorage : null;
|
||||
@@ -64,8 +56,8 @@ const safeSessionStorage = (): Storage | null => {
|
||||
};
|
||||
|
||||
const getDefaultTarget = (): string => {
|
||||
const ls = safeLocalStorage();
|
||||
return ls?.getItem(STORAGE_KEY) ?? "http://127.0.0.1:8765/rpc";
|
||||
const ss = safeSessionStorage();
|
||||
return ss?.getItem(STORAGE_KEY) ?? "http://127.0.0.1:8765/rpc";
|
||||
};
|
||||
|
||||
export const initialState = (): ConnectionState => ({
|
||||
|
||||
@@ -138,6 +138,14 @@ describe("error handling", () => {
|
||||
).rejects.toThrow("empty response");
|
||||
});
|
||||
|
||||
it("throws for structurally malformed JSON response", async () => {
|
||||
mockFetch.mockReturnValue(jsonResponse({ ok: true, connection: {} }));
|
||||
|
||||
await expect(
|
||||
connectToServer("http://127.0.0.1:8000/rpc"),
|
||||
).rejects.toThrow("malformed response");
|
||||
});
|
||||
|
||||
it("throws on network failure", async () => {
|
||||
mockFetch.mockReturnValue(Promise.reject(new Error("network error")));
|
||||
|
||||
|
||||
@@ -3,8 +3,13 @@ import type {
|
||||
RpcResponse,
|
||||
OperationName,
|
||||
} from "./contracts.js";
|
||||
import { parseConnectResponse, parseRpcResponse } from "./contracts.js";
|
||||
|
||||
const fetchJson = async <T>(url: string, init?: RequestInit): Promise<T> => {
|
||||
const fetchJson = async <T>(
|
||||
url: string,
|
||||
init: RequestInit,
|
||||
parse: (data: unknown) => T,
|
||||
): Promise<T> => {
|
||||
const res = await fetch(url, init);
|
||||
const text = await res.text();
|
||||
if (!text) {
|
||||
@@ -16,25 +21,33 @@ const fetchJson = async <T>(url: string, init?: RequestInit): Promise<T> => {
|
||||
} catch {
|
||||
throw new Error("malformed JSON response from server");
|
||||
}
|
||||
return data as T;
|
||||
return parse(data);
|
||||
};
|
||||
|
||||
export const connectToServer = async (
|
||||
target: string,
|
||||
): Promise<ConnectResponse> =>
|
||||
fetchJson<ConnectResponse>("/api/connect", {
|
||||
method: "POST",
|
||||
headers: { "content-type": "application/json" },
|
||||
body: JSON.stringify({ target }),
|
||||
});
|
||||
fetchJson(
|
||||
"/api/connect",
|
||||
{
|
||||
method: "POST",
|
||||
headers: { "content-type": "application/json" },
|
||||
body: JSON.stringify({ target }),
|
||||
},
|
||||
parseConnectResponse,
|
||||
);
|
||||
|
||||
export const callOperation = async (
|
||||
operation: OperationName,
|
||||
target: string,
|
||||
params: unknown = {},
|
||||
): Promise<RpcResponse> =>
|
||||
fetchJson<RpcResponse>("/api/rpc", {
|
||||
method: "POST",
|
||||
headers: { "content-type": "application/json" },
|
||||
body: JSON.stringify({ operation, target, params }),
|
||||
});
|
||||
fetchJson(
|
||||
"/api/rpc",
|
||||
{
|
||||
method: "POST",
|
||||
headers: { "content-type": "application/json" },
|
||||
body: JSON.stringify({ operation, target, params }),
|
||||
},
|
||||
parseRpcResponse,
|
||||
);
|
||||
|
||||
@@ -1,46 +1,79 @@
|
||||
export type ConnectionSuccess = {
|
||||
readonly ok: true;
|
||||
readonly connection: {
|
||||
readonly status: "connected";
|
||||
readonly target: string;
|
||||
readonly serverStatus: "ok";
|
||||
readonly storeRoot: string;
|
||||
readonly durationMs: number;
|
||||
};
|
||||
readonly exchange: { readonly request: unknown; readonly response: unknown };
|
||||
readonly equivalentCli: string;
|
||||
import * as v from "valibot";
|
||||
|
||||
const BrowserErrorCodeSchema = v.union([
|
||||
v.literal("invalid_target"),
|
||||
v.literal("unknown_operation"),
|
||||
v.literal("upstream_unreachable"),
|
||||
v.literal("upstream_timeout"),
|
||||
v.literal("rpc_remote_error"),
|
||||
v.literal("rpc_protocol_error"),
|
||||
v.literal("rpc_decode_error"),
|
||||
v.literal("response_too_large"),
|
||||
]);
|
||||
|
||||
const ExchangeSchema = v.object({
|
||||
request: v.nullish(v.unknown(), null),
|
||||
response: v.nullish(v.unknown(), null),
|
||||
});
|
||||
|
||||
const ApiFailureSchema = v.object({
|
||||
ok: v.literal(false),
|
||||
error: v.object({
|
||||
code: BrowserErrorCodeSchema,
|
||||
message: v.string(),
|
||||
}),
|
||||
exchange: ExchangeSchema,
|
||||
});
|
||||
|
||||
const ConnectionSuccessSchema = v.object({
|
||||
ok: v.literal(true),
|
||||
connection: v.object({
|
||||
status: v.literal("connected"),
|
||||
target: v.string(),
|
||||
serverStatus: v.literal("ok"),
|
||||
storeRoot: v.string(),
|
||||
durationMs: v.number(),
|
||||
}),
|
||||
exchange: ExchangeSchema,
|
||||
equivalentCli: v.string(),
|
||||
});
|
||||
|
||||
const OperationNameSchema = v.union([
|
||||
v.literal("workflow.health"),
|
||||
v.literal("workflow.sources.list"),
|
||||
]);
|
||||
|
||||
const OperationSuccessSchema = v.object({
|
||||
ok: v.literal(true),
|
||||
operation: OperationNameSchema,
|
||||
label: v.string(),
|
||||
interpreted: v.unknown(),
|
||||
exchange: ExchangeSchema,
|
||||
equivalentCli: v.string(),
|
||||
durationMs: v.number(),
|
||||
});
|
||||
|
||||
const ConnectResponseSchema = v.union([ConnectionSuccessSchema, ApiFailureSchema]);
|
||||
const RpcResponseSchema = v.union([OperationSuccessSchema, ApiFailureSchema]);
|
||||
|
||||
export type ConnectionSuccess = v.InferOutput<typeof ConnectionSuccessSchema>;
|
||||
export type OperationSuccess = v.InferOutput<typeof OperationSuccessSchema>;
|
||||
export type BrowserErrorCode = v.InferOutput<typeof BrowserErrorCodeSchema>;
|
||||
export type ApiFailure = v.InferOutput<typeof ApiFailureSchema>;
|
||||
export type ConnectResponse = v.InferOutput<typeof ConnectResponseSchema>;
|
||||
export type RpcResponse = v.InferOutput<typeof RpcResponseSchema>;
|
||||
export type OperationName = v.InferOutput<typeof OperationNameSchema>;
|
||||
|
||||
const parseDto = <T>(schema: v.GenericSchema<unknown, T>, data: unknown): T => {
|
||||
try {
|
||||
return v.parse(schema, data);
|
||||
} catch {
|
||||
throw new Error("malformed response from server");
|
||||
}
|
||||
};
|
||||
|
||||
export type OperationSuccess = {
|
||||
readonly ok: true;
|
||||
readonly operation: string;
|
||||
readonly label: string;
|
||||
readonly interpreted: unknown;
|
||||
readonly exchange: { readonly request: unknown; readonly response: unknown };
|
||||
readonly equivalentCli: string;
|
||||
readonly durationMs: number;
|
||||
};
|
||||
export const parseConnectResponse = (data: unknown): ConnectResponse =>
|
||||
parseDto(ConnectResponseSchema, data);
|
||||
|
||||
export type BrowserErrorCode =
|
||||
| "invalid_target"
|
||||
| "unknown_operation"
|
||||
| "upstream_unreachable"
|
||||
| "upstream_timeout"
|
||||
| "rpc_remote_error"
|
||||
| "rpc_protocol_error"
|
||||
| "rpc_decode_error"
|
||||
| "response_too_large";
|
||||
|
||||
export type ApiFailure = {
|
||||
readonly ok: false;
|
||||
readonly error: { readonly code: BrowserErrorCode; readonly message: string };
|
||||
readonly exchange: {
|
||||
readonly request: unknown | null;
|
||||
readonly response: unknown | null;
|
||||
};
|
||||
};
|
||||
|
||||
export type ConnectResponse = ConnectionSuccess | ApiFailure;
|
||||
export type RpcResponse = OperationSuccess | ApiFailure;
|
||||
|
||||
export type OperationName = "workflow.health" | "workflow.sources.list";
|
||||
export const parseRpcResponse = (data: unknown): RpcResponse =>
|
||||
parseDto(RpcResponseSchema, data);
|
||||
|
||||
@@ -1,11 +1,15 @@
|
||||
import { describe, it, expect, vi } from "vitest";
|
||||
import { createApp, type RunOperation } from "./app.js";
|
||||
import type { OperationExchange } from "@lda/workflow-rpc";
|
||||
import * as fs from "node:fs";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
|
||||
const makeExchange = (
|
||||
overrides: Partial<OperationExchange> = {},
|
||||
): OperationExchange => ({
|
||||
operation: "workflow.health",
|
||||
target: "http://127.0.0.1:8765/rpc",
|
||||
label: "Health check",
|
||||
interpreted: { status: "ok", store_root: "/tmp/store" },
|
||||
exchange: { request: {}, response: { status: "ok" } },
|
||||
@@ -15,13 +19,16 @@ const makeExchange = (
|
||||
});
|
||||
|
||||
const okRunner: RunOperation = vi.fn(async (operation) =>
|
||||
makeExchange({ operation }),
|
||||
makeExchange({ operation, target: "http://127.0.0.1:8765/rpc" }),
|
||||
);
|
||||
|
||||
const failRunner =
|
||||
(code: string, message: string): RunOperation =>
|
||||
async () => {
|
||||
throw Object.assign(new Error(message), { _tag: code });
|
||||
throw Object.assign(new Error(message), {
|
||||
_tag: code,
|
||||
exchange: { request: { method: "x" }, response: { error: message } },
|
||||
});
|
||||
};
|
||||
|
||||
const app = createApp({ runOperation: okRunner });
|
||||
@@ -46,7 +53,7 @@ describe("POST /api/connect", () => {
|
||||
const body = await res.json();
|
||||
expect(body.ok).toBe(true);
|
||||
expect(body.connection.status).toBe("connected");
|
||||
expect(body.connection.target).toBe("http://127.0.0.1:8000/rpc");
|
||||
expect(body.connection.target).toBe("http://127.0.0.1:8765/rpc");
|
||||
expect(body.connection.serverStatus).toBe("ok");
|
||||
expect(okRunner).toHaveBeenCalledWith(
|
||||
"workflow.health",
|
||||
@@ -139,6 +146,10 @@ describe("error mapping", () => {
|
||||
const body = await res.json();
|
||||
expect(body.ok).toBe(false);
|
||||
expect(body.error.code).toBe("upstream_timeout");
|
||||
expect(body.exchange).toEqual({
|
||||
request: { method: "x" },
|
||||
response: { error: "timed out" },
|
||||
});
|
||||
expect(body.error.stack).toBeUndefined();
|
||||
});
|
||||
|
||||
@@ -238,3 +249,29 @@ describe("error mapping", () => {
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("static console routes", () => {
|
||||
it("serves the SPA and keeps unknown API paths as JSON 404", async () => {
|
||||
const consoleRoot = fs.mkdtempSync(path.join(os.tmpdir(), "wf-console-"));
|
||||
fs.mkdirSync(path.join(consoleRoot, "assets"));
|
||||
fs.writeFileSync(path.join(consoleRoot, "index.html"), "<main>console</main>");
|
||||
fs.writeFileSync(path.join(consoleRoot, "assets", "app.js"), "console.log('ok')");
|
||||
try {
|
||||
const staticApp = createApp({ runOperation: okRunner, consoleRoot });
|
||||
|
||||
const index = await staticApp.request("/workflows");
|
||||
expect(index.status).toBe(200);
|
||||
expect(await index.text()).toContain("console");
|
||||
|
||||
const asset = await staticApp.request("/assets/app.js");
|
||||
expect(asset.status).toBe(200);
|
||||
expect(await asset.text()).toContain("ok");
|
||||
|
||||
const unknownApi = await staticApp.request("/api/nope");
|
||||
expect(unknownApi.status).toBe(404);
|
||||
expect(await unknownApi.json()).toEqual({ error: "not found" });
|
||||
} finally {
|
||||
fs.rmSync(consoleRoot, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2,6 +2,7 @@ import { Hono } from "hono";
|
||||
import { bodyLimit } from "hono/body-limit";
|
||||
import type { ContentfulStatusCode } from "hono/utils/http-status";
|
||||
import type { OperationExchange, OperationName } from "@lda/workflow-rpc";
|
||||
import { addStaticRoutes, validateConsoleRoot } from "./static.js";
|
||||
|
||||
export type RunOperation = (
|
||||
operation: OperationName,
|
||||
@@ -51,8 +52,9 @@ const mapErrorToStatus = (
|
||||
|
||||
export function createApp(dependencies: {
|
||||
readonly runOperation: RunOperation;
|
||||
readonly consoleRoot?: string;
|
||||
}): Hono {
|
||||
const { runOperation } = dependencies;
|
||||
const { runOperation, consoleRoot } = dependencies;
|
||||
const app = new Hono();
|
||||
|
||||
app.get("/api/health", (c) =>
|
||||
@@ -94,10 +96,12 @@ export function createApp(dependencies: {
|
||||
ok: true,
|
||||
connection: {
|
||||
status: "connected",
|
||||
target: body.target,
|
||||
target: exchange.target,
|
||||
serverStatus: "ok",
|
||||
storeRoot: (
|
||||
exchange.interpreted as { store_root?: string }
|
||||
exchange.interpreted as { storeRoot?: string; store_root?: string }
|
||||
).storeRoot ?? (
|
||||
exchange.interpreted as { storeRoot?: string; store_root?: string }
|
||||
).store_root ?? "",
|
||||
durationMs: exchange.durationMs,
|
||||
},
|
||||
@@ -115,7 +119,7 @@ export function createApp(dependencies: {
|
||||
{
|
||||
ok: false,
|
||||
error: { code, message: msg },
|
||||
exchange: { request: null, response: null },
|
||||
exchange: exchangeFromError(e),
|
||||
},
|
||||
status,
|
||||
);
|
||||
@@ -193,12 +197,38 @@ export function createApp(dependencies: {
|
||||
{
|
||||
ok: false,
|
||||
error: { code, message: msg },
|
||||
exchange: { request: null, response: null },
|
||||
exchange: exchangeFromError(e),
|
||||
},
|
||||
status,
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
if (consoleRoot) {
|
||||
validateConsoleRoot(consoleRoot);
|
||||
addStaticRoutes(app, { consoleRoot });
|
||||
}
|
||||
|
||||
return app;
|
||||
}
|
||||
|
||||
const exchangeFromError = (
|
||||
error: unknown,
|
||||
): { readonly request: unknown | null; readonly response: unknown | null } => {
|
||||
if (error && typeof error === "object" && "exchange" in error) {
|
||||
const exchange = (error as { readonly exchange?: unknown }).exchange;
|
||||
if (exchange && typeof exchange === "object") {
|
||||
return {
|
||||
request:
|
||||
"request" in exchange
|
||||
? (exchange as { readonly request?: unknown }).request ?? null
|
||||
: null,
|
||||
response:
|
||||
"response" in exchange
|
||||
? (exchange as { readonly response?: unknown }).response ?? null
|
||||
: null,
|
||||
};
|
||||
}
|
||||
}
|
||||
return { request: null, response: null };
|
||||
};
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { Effect, Layer } from "effect";
|
||||
import { serve } from "@hono/node-server";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import {
|
||||
WorkflowRpc,
|
||||
makeWorkflowRpcLayer,
|
||||
@@ -16,7 +17,8 @@ if (Number.isNaN(port) || port < 1 || port > 65535) {
|
||||
|
||||
const hostname = process.env.WEB_HOST ?? "127.0.0.1";
|
||||
|
||||
const liveLayer = makeWorkflowRpcLayer;
|
||||
const liveLayer = makeWorkflowRpcLayer();
|
||||
const consoleRoot = fileURLToPath(new URL("../../console/dist", import.meta.url));
|
||||
|
||||
const runOperation: RunOperation = async (
|
||||
operation: OperationName,
|
||||
@@ -28,7 +30,7 @@ const runOperation: RunOperation = async (
|
||||
return yield* execute(operation, target, params);
|
||||
}).pipe(Effect.provide(liveLayer), Effect.runPromise);
|
||||
|
||||
const app = createApp({ runOperation });
|
||||
const app = createApp({ runOperation, consoleRoot });
|
||||
|
||||
serve({
|
||||
fetch: app.fetch,
|
||||
|
||||
@@ -23,19 +23,8 @@ export function addStaticRoutes(
|
||||
): void {
|
||||
const { consoleRoot } = options;
|
||||
|
||||
app.all("/api/*", (c) => c.json({ error: "not found" }, 404));
|
||||
app.use("/assets/*", serveStatic({ root: consoleRoot }));
|
||||
|
||||
app.get("*", (c) => {
|
||||
if (c.req.path.startsWith("/api/")) {
|
||||
return c.json({ error: "not found" }, 404);
|
||||
}
|
||||
return serveStatic({ root: consoleRoot, path: "index.html" })(c);
|
||||
});
|
||||
|
||||
app.all("*", (c) => {
|
||||
if (c.req.path.startsWith("/api/")) {
|
||||
return c.json({ error: "not found" }, 404);
|
||||
}
|
||||
return c.json({ error: "not found" }, 404);
|
||||
});
|
||||
app.get("*", serveStatic({ root: consoleRoot, path: "index.html" }));
|
||||
app.all("*", (c) => c.json({ error: "not found" }, 404));
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user