From a681c431fbb53a8ebe3e1fae4c793acee56d710a Mon Sep 17 00:00:00 2001 From: mptyl Date: Sun, 9 Aug 2026 20:38:54 +0200 Subject: [PATCH] fix: sanitize workspace API envelopes --- frontend/src/api/workspaces.test.ts | 50 ++++++++++++++++++++++- frontend/src/api/workspaces.ts | 62 +++++++++++++++++++++++++---- 2 files changed, 103 insertions(+), 9 deletions(-) diff --git a/frontend/src/api/workspaces.test.ts b/frontend/src/api/workspaces.test.ts index cfe7c285..8d199778 100644 --- a/frontend/src/api/workspaces.test.ts +++ b/frontend/src/api/workspaces.test.ts @@ -41,7 +41,7 @@ test("uploads a workspace bundle without JSON content type", async () => { let contentType: string | null = null; server.use(http.post("/api/workspaces/import", ({ request }) => { contentType = request.headers.get("content-type"); - return HttpResponse.json({ draft: { workspace: {} } }); + return HttpResponse.json({ draft: { workspace } }); })); await importWorkspace(new File(["zip"], "clinical.thoth-workspace.zip", { type: "application/zip" })); @@ -52,6 +52,54 @@ test("uploads a workspace bundle without JSON content type", async () => { expect(contentType ?? "").not.toMatch(/application\/json/i); }); +test("sanitizes imported Evidence before returning a browser draft", async () => { + server.use(http.post("/api/workspaces/import", () => HttpResponse.json({ + draft: { workspace: evidenceWorkspace, contract: { variables: [] } }, + }))); + + const result = await importWorkspace(new File(["zip"], "clinical.thoth-workspace.zip")); + + expect(result.draft.workspace.evidence).toEqual(evidenceWorkspace.evidence); + expect(result.draft.workspace).not.toBe(evidenceWorkspace); +}); + +test("rejects imported Evidence with a secret-shaped field", async () => { + const malformed = { + ...evidenceWorkspace, + evidence: { ...evidenceWorkspace.evidence, signed_urls_file: "/run/secrets/urls" }, + }; + server.use(http.post("/api/workspaces/import", () => HttpResponse.json({ + draft: { workspace: malformed, contract: {} }, + }))); + + await expect(importWorkspace(new File(["zip"], "clinical.thoth-workspace.zip"))) + .rejects.toThrow("invalid imported workspace draft"); +}); + +test("rejects read responses with a missing or inconsistent revision", async () => { + server.use(http.get("/api/workspaces/psd-clinical", () => HttpResponse.json({ + workspace: evidenceWorkspace, + revision: { ...revision, id: "other-workspace" }, + }))); + await expect(getWorkspace("psd-clinical")).rejects.toThrow("invalid workspace revision"); + + server.use(http.get("/api/workspaces/psd-clinical", () => HttpResponse.json({ + workspace: evidenceWorkspace, revision: null, + }))); + await expect(getWorkspace("psd-clinical")).rejects.toThrow("invalid workspace revision"); +}); + +test("rejects a publish response with a malformed revision", async () => { + server.use(http.post("/api/workspaces/publish", () => HttpResponse.json({ + revision: { ...revision, commit: "not-a-commit" }, + }))); + + await expect(publishWorkspace({ + action: "update", workspace: evidenceWorkspace, + baseCommit: revision.commit, baseBlob: revision.blob, + })).rejects.toThrow("invalid workspace revision"); +}); + test("rejects a conflict payload that attempts to surface a secret field", async () => { server.use(http.post("/api/workspaces/publish", () => HttpResponse.json({ code: "workspace_conflict", message: "Workspace changed in the registry.", fields: ["dwh.password"], diff --git a/frontend/src/api/workspaces.ts b/frontend/src/api/workspaces.ts index b9551c5c..7861217c 100644 --- a/frontend/src/api/workspaces.ts +++ b/frontend/src/api/workspaces.ts @@ -200,6 +200,33 @@ function object(value: unknown): Record | undefined { : undefined; } +function exactObject(value: unknown, keys: readonly string[]): Record | undefined { + const source = object(value); + return source && Object.keys(source).every((key) => keys.includes(key)) ? source : undefined; +} + +function workspaceRevision(value: unknown, expectedId: string): WorkspaceRevision | undefined { + const source = exactObject(value, ["id", "commit", "blob", "snapshotPath", "state"]); + if (!source) return undefined; + const { id, commit, blob, snapshotPath, state } = source; + if ( + id !== expectedId + || typeof id !== "string" || !/^[a-z][a-z0-9-]{2,62}$/.test(id) + || typeof commit !== "string" || !/^[0-9a-f]{40}$/.test(commit) + || typeof blob !== "string" || !/^[0-9a-f]{40}$/.test(blob) + || typeof snapshotPath !== "string" || snapshotPath.length === 0 + || snapshotPath.trim() !== snapshotPath || /[\u0000-\u001f\u007f]/u.test(snapshotPath) + || (state !== "operational" && state !== "migration_required") + ) return undefined; + return { id, commit, blob, snapshotPath, state }; +} + +function requireWorkspaceRevision(value: unknown, expectedId: string): WorkspaceRevision { + const revision = workspaceRevision(value, expectedId); + if (!revision) throw new Error("Workspace API returned an invalid workspace revision"); + return revision; +} + function conflictRevision(value: unknown): WorkspaceConflictRevision | undefined { const source = object(value); const commit = source?.commit; @@ -258,9 +285,10 @@ export const getWorkspace = async (id: string): Promise => { const response = await apiFetch(`/workspaces/${encodeURIComponent(id)}`); const source = object(response); if (!source) throw new Error("Workspace API returned an invalid workspace record"); + const workspace = requireCanonicalWorkspace(source.workspace); return { - workspace: requireCanonicalWorkspace(source.workspace), - revision: source.revision as WorkspaceRevision, + workspace, + revision: requireWorkspaceRevision(source.revision, workspace.workspace.id), }; }; export const getWorkspaceRegistryStatus = () => apiFetch("/workspace-registry/status"); @@ -276,20 +304,38 @@ export const validateWorkspace = async (workspace: CanonicalWorkspace) => { }; export const testWorkspace = (id: string) => apiFetch(`/workspaces/${encodeURIComponent(id)}/test`, { method: "POST" }); -export const publishWorkspace = (request: PublishWorkspaceRequest) => { +export const publishWorkspace = async (request: PublishWorkspaceRequest) => { const safeRequest: PublishWorkspaceRequest = request.action === "delete" ? request : { ...request, workspace: requireCanonicalWorkspace(request.workspace) }; - return apiFetch<{ revision: WorkspaceRevision } | undefined>("/workspaces/publish", { + const response = await apiFetch("/workspaces/publish", { method: "POST", body: JSON.stringify(safeRequest), }); + if (response === undefined) return undefined; + const source = exactObject(response, ["revision"]); + const expectedId = safeRequest.action === "delete" ? safeRequest.id : safeRequest.workspace.workspace.id; + if (!source) throw new Error("Workspace API returned an invalid publish result"); + return { revision: requireWorkspaceRevision(source.revision, expectedId) }; }; export const exportWorkspace = (id: string) => apiFetchBlob(`/workspaces/${encodeURIComponent(id)}/export`); -export const importWorkspace = (bundle: File) => { +export const importWorkspace = async (bundle: File) => { const body = new FormData(); body.set("bundle", bundle); - return apiFetch<{ draft: { workspace: CanonicalWorkspace; contract?: unknown } }>("/workspaces/import", { - method: "POST", body, - }); + const response = await apiFetch("/workspaces/import", { method: "POST", body }); + const source = exactObject(response, ["draft"]); + const draft = exactObject(source?.draft, ["workspace", "contract"]); + if (!source || !draft) throw new Error("Workspace API returned an invalid imported workspace draft"); + let workspace: CanonicalWorkspace; + try { + workspace = requireCanonicalWorkspace(draft.workspace); + } catch { + throw new Error("Workspace API returned an invalid imported workspace draft"); + } + return { + draft: { + workspace, + ...(draft.contract === undefined ? {} : { contract: draft.contract }), + }, + }; };