Skip to content

Commit 993eb4a

Browse files
committed
fix(studio): secure targeted WebMCP writes
1 parent f780203 commit 993eb4a

24 files changed

Lines changed: 439 additions & 33 deletions

packages/studio-server/src/helpers/projectSignature.test.ts

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,14 @@
1-
import { describe, expect, it } from "vitest";
1+
import { afterEach, describe, expect, it } from "vitest";
2+
import { mkdtempSync, rmSync, statSync, utimesSync, writeFileSync } from "node:fs";
3+
import { tmpdir } from "node:os";
24
import { resolve } from "node:path";
3-
import { affectsProjectSignature } from "./projectSignature.js";
5+
import { affectsProjectSignature, createProjectSignature } from "./projectSignature.js";
6+
7+
const temporaryProjects: string[] = [];
8+
9+
afterEach(() => {
10+
for (const project of temporaryProjects.splice(0)) rmSync(project, { recursive: true });
11+
});
412

513
const PROJECT = resolve("/projects/demo");
614
const affects = (relativePath: string) =>
@@ -42,3 +50,19 @@ describe("affectsProjectSignature", () => {
4250
expect(affectsProjectSignature(PROJECT, PROJECT)).toBe(false);
4351
});
4452
});
53+
54+
describe("createProjectSignature", () => {
55+
it("changes after same-size content is written with the original mtime restored", () => {
56+
const project = mkdtempSync(resolve(tmpdir(), "hf-signature-"));
57+
temporaryProjects.push(project);
58+
const file = resolve(project, "index.html");
59+
writeFileSync(file, "first");
60+
const originalMtime = statSync(file).mtime;
61+
const before = createProjectSignature(project);
62+
63+
writeFileSync(file, "other");
64+
utimesSync(file, originalMtime, originalMtime);
65+
66+
expect(createProjectSignature(project)).not.toBe(before);
67+
});
68+
});

packages/studio-server/src/helpers/projectSignature.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ export function affectsProjectSignature(projectDir: string, changedPath: string)
7070
interface ProjectSignatureFile {
7171
file: string;
7272
mtimeMs: number;
73+
ctimeMs: number;
7374
size: number;
7475
textContentEligible: boolean;
7576
}
@@ -124,6 +125,7 @@ function collectProjectSignatureFiles(
124125
files.push({
125126
file,
126127
mtimeMs: stat.mtimeMs,
128+
ctimeMs: stat.ctimeMs,
127129
size: stat.size,
128130
textContentEligible: isTextContentEligible(file, stat.size),
129131
});
@@ -149,6 +151,7 @@ function collectProjectSignatureManifestFiles(
149151
files.push({
150152
file,
151153
mtimeMs: stat.mtimeMs,
154+
ctimeMs: stat.ctimeMs,
152155
size: stat.size,
153156
textContentEligible: isTextContentEligible(file, stat.size),
154157
});
@@ -165,6 +168,8 @@ function createProjectFingerprint(projectDir: string, files: ProjectSignatureFil
165168
hash.update("\0");
166169
hash.update(String(entry.mtimeMs));
167170
hash.update("\0");
171+
hash.update(String(entry.ctimeMs));
172+
hash.update("\0");
168173
hash.update(entry.textContentEligible ? "text" : "binary");
169174
hash.update("\0");
170175
}

packages/studio-server/src/routes/files.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1333,6 +1333,30 @@ const tl = gsap.timeline({ paused: true });
13331333
expect(res.status).toBe(400);
13341334
});
13351335

1336+
it("rejects raw JavaScript expressions at the GSAP mutation boundary", async () => {
1337+
const projectDir = createProjectDir();
1338+
writeHtml(projectDir, "comp.html", FROMTO_COMP);
1339+
const app = new Hono();
1340+
registerFileRoutes(app, createAdapter(projectDir));
1341+
const animation = await getFirstAnimation(app, "comp.html");
1342+
1343+
const response = await app.request("http://localhost/projects/demo/gsap-mutations/comp.html", {
1344+
method: "POST",
1345+
headers: { "Content-Type": "application/json" },
1346+
body: JSON.stringify({
1347+
type: "update-meta",
1348+
animationId: animation.id,
1349+
updates: { ease: "__raw:(()=>alert(1))()" },
1350+
}),
1351+
});
1352+
1353+
expect(response.status).toBe(400);
1354+
expect(await response.json()).toEqual({
1355+
error: "raw JavaScript expressions are not accepted",
1356+
});
1357+
expect(readFileSync(join(projectDir, "comp.html"), "utf8")).toBe(FROMTO_COMP);
1358+
});
1359+
13361360
it("update-from-property updates a fromTo start value in place", async () => {
13371361
const projectDir = createProjectDir();
13381362
writeHtml(projectDir, "comp.html", FROMTO_COMP);

packages/studio-server/src/routes/files.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1186,6 +1186,9 @@ function validateGsapMutationRequest(
11861186
if (!body || typeof body !== "object" || !("type" in body) || !body.type) {
11871187
return c.json({ error: "mutation type required" }, 400);
11881188
}
1189+
if (containsRawGsapExpression(body)) {
1190+
return c.json({ error: "raw JavaScript expressions are not accepted" }, 400);
1191+
}
11891192
const unsafeFields = findUnsafeMutationValues(body);
11901193
if (unsafeFields.length > 0) return rejectUnsafeMutationValues(c, unsafeFields);
11911194
if (
@@ -1197,6 +1200,13 @@ function validateGsapMutationRequest(
11971200
return null;
11981201
}
11991202

1203+
function containsRawGsapExpression(value: unknown): boolean {
1204+
if (typeof value === "string") return value.startsWith("__raw:");
1205+
if (Array.isArray(value)) return value.some(containsRawGsapExpression);
1206+
if (!value || typeof value !== "object") return false;
1207+
return Object.values(value).some(containsRawGsapExpression);
1208+
}
1209+
12001210
async function prepareGsapMutationScript(
12011211
c: RouteContext,
12021212
res: ResolvedGsapFile,

packages/studio-server/src/routes/thumbnail.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { tmpdir } from "node:os";
1414
import { join } from "node:path";
1515
import { pruneThumbnailCache, registerThumbnailRoutes } from "./thumbnail";
1616
import type { StudioApiAdapter } from "../types";
17+
import { createProjectSignature } from "../helpers/projectSignature.js";
1718

1819
const tempProjectDirs: string[] = [];
1920

@@ -370,7 +371,7 @@ describe("registerThumbnailRoutes", () => {
370371
const adapter = createAdapter();
371372
const project = await adapter.resolveProject("demo");
372373
if (!project) throw new Error("missing project");
373-
const getProjectSignature = vi.fn(() => "cached-project-signature");
374+
const getProjectSignature = vi.fn(() => createProjectSignature(project.dir));
374375
adapter.getProjectSignature = getProjectSignature;
375376
let resolve!: (buffer: Buffer) => void;
376377
const generated = new Promise<Buffer>((done) => (resolve = done));

packages/studio-server/src/routes/thumbnail.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { join } from "node:path";
1414
import { createHash, randomUUID } from "node:crypto";
1515
import type { StudioApiAdapter } from "../types.js";
1616
import { STUDIO_MANUAL_EDITS_PATH } from "../helpers/manualEditsRenderScript.js";
17-
import { resolveProjectAndSignature } from "../helpers/projectSignature.js";
17+
import { createProjectSignature, resolveProjectAndSignature } from "../helpers/projectSignature.js";
1818
import { STUDIO_MOTION_PATH } from "../helpers/studioMotionRenderScript.js";
1919
import { thumbnailGenerationCoordinator } from "./thumbnailGenerationCoordinator.js";
2020

@@ -213,10 +213,12 @@ export function registerThumbnailRoutes(api: Hono, adapter: StudioApiAdapter): v
213213
});
214214
if (!generated) return null;
215215
const afterGeneration = await resolveProjectAndSignature(adapter, project.id);
216+
const freshSignature = createProjectSignature(project.dir);
216217
if (
217218
!afterGeneration ||
218219
afterGeneration.project.dir !== project.dir ||
219-
afterGeneration.signature !== projectSignature
220+
afterGeneration.signature !== projectSignature ||
221+
freshSignature !== projectSignature
220222
) {
221223
// The browser may have rendered content written after this request
222224
// captured its cache identity. Return the pixels to this caller,

packages/studio/src/hooks/useDomEditCommits.test.tsx

Lines changed: 56 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ interface RenderDomEditCommitsOptions {
4141
writeProjectFile?: (path: string, content: string, expectedContent?: string) => Promise<void>;
4242
onTrySdkPersist?: () => Promise<CutoverResult>;
4343
queueDomEditSave?: <T>(save: () => Promise<T>) => Promise<T>;
44+
projectIdRef?: MutableRefObject<string | null>;
4445
}
4546

4647
type FetchHandler = (
@@ -207,7 +208,7 @@ function renderDomEditCommits(
207208
const showToast = makeShowToast();
208209
const recordEdit = vi.fn(async () => {});
209210
const previewIframeRef: MutableRefObject<HTMLIFrameElement | null> = { current: iframe };
210-
const projectIdRef: MutableRefObject<string | null> = { current: "p1" };
211+
const projectIdRef: MutableRefObject<string | null> = options.projectIdRef ?? { current: "p1" };
211212
const domEditSaveTimestampRef: MutableRefObject<number> = { current: 0 };
212213
const reloadPreview = vi.fn();
213214

@@ -1111,6 +1112,60 @@ describe("useDomEditCommits style persist handling", () => {
11111112
}
11121113
});
11131114

1115+
it("refuses queued persistence after the active project changes", async () => {
1116+
const firstPatch = createDeferred<Response>();
1117+
const projectIdRef: MutableRefObject<string | null> = { current: "p1" };
1118+
const fetchMock = vi.fn(async (input: Parameters<typeof fetch>[0]) => {
1119+
const url = requestUrl(input);
1120+
if (url.includes("/api/projects/p1/files/")) {
1121+
return jsonResponse({
1122+
content: '<div data-hf-id="hf-card" style="color: red">Card</div>',
1123+
});
1124+
}
1125+
if (url.includes("/api/projects/p1/file-mutations/patch-element/")) {
1126+
return firstPatch.promise;
1127+
}
1128+
throw new Error(`Unexpected fetch: ${url}`);
1129+
});
1130+
vi.stubGlobal("fetch", fetchMock);
1131+
const queue = createDomEditSaveQueue();
1132+
const { iframe, element } = createPreviewElement();
1133+
const rendered = renderDomEditCommits(createSelection(element), iframe, {
1134+
queueDomEditSave: queue.enqueue,
1135+
projectIdRef,
1136+
});
1137+
1138+
try {
1139+
const first = rendered.hook.handleDomStyleCommit("color", "blue");
1140+
await flushAsyncWork();
1141+
const second = rendered.hook.handleDomStyleCommit("color", "green");
1142+
projectIdRef.current = "p2";
1143+
firstPatch.resolve(
1144+
jsonResponse({
1145+
ok: true,
1146+
changed: true,
1147+
matched: true,
1148+
content: '<div data-hf-id="hf-card" style="color: blue">Card</div>',
1149+
path: "index.html",
1150+
version: '"sha256:blue"',
1151+
}),
1152+
);
1153+
1154+
await first;
1155+
await expect(second).resolves.toMatchObject({
1156+
ok: false,
1157+
reason: "persist-failed",
1158+
});
1159+
expect(fetchMock.mock.calls.some(([input]) => requestUrl(input).includes("/p2/"))).toBe(
1160+
false,
1161+
);
1162+
expect(fetchMock).toHaveBeenCalledTimes(2);
1163+
} finally {
1164+
queue.destroy();
1165+
rendered.cleanup();
1166+
}
1167+
});
1168+
11141169
it("preserves the SDK cutover version as the style commit's durable evidence", async () => {
11151170
const fetchMock = stubPatchFetch({ ok: true, changed: true, matched: true });
11161171
const { iframe, element } = createPreviewElement();

packages/studio/src/hooks/useDomEditCommits.ts

Lines changed: 33 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -139,11 +139,18 @@ export function useDomEditCommits({
139139
const reportedUnresolvableRef = useRef(new Set<string>());
140140

141141
// fallow-ignore-next-line complexity
142-
const performPersistDomEditOperations: PersistDomEditOperations = useCallback(
142+
const performPersistDomEditOperations = useCallback(
143143
// fallow-ignore-next-line complexity
144-
async (selection, operations, options) => {
145-
const pid = projectIdRef.current;
146-
if (!pid) throw new Error("No active project");
144+
async (
145+
selection: DomEditSelection,
146+
operations: PatchOperation[],
147+
options: Parameters<PersistDomEditOperations>[2],
148+
expectedProjectId: string,
149+
) => {
150+
if (projectIdRef.current !== expectedProjectId) {
151+
throw new Error("Active project changed before the edit could be saved");
152+
}
153+
const pid = expectedProjectId;
147154
if (options?.shouldSave && !options.shouldSave()) return;
148155

149156
const targetPath = selection.sourceFile || activeCompPath || "index.html";
@@ -160,6 +167,10 @@ export function useDomEditCommits({
160167
throw new Error(`Missing file contents for ${targetPath}`);
161168
}
162169

170+
if (projectIdRef.current !== expectedProjectId) {
171+
throw new Error("Active project changed before the edit could be saved");
172+
}
173+
163174
if (options?.shouldSave && !options.shouldSave()) return;
164175

165176
// Validate layout values BEFORE any persist path runs. The SDK cutover
@@ -298,20 +309,29 @@ export function useDomEditCommits({
298309
);
299310

300311
const persistDomEditOperations: PersistDomEditOperations = useCallback(
301-
(selection, operations, options) =>
302-
queueDomEditSave(() => performPersistDomEditOperations(selection, operations, options)),
303-
[performPersistDomEditOperations, queueDomEditSave],
312+
(selection, operations, options) => {
313+
const expectedProjectId = projectIdRef.current;
314+
if (!expectedProjectId) return Promise.reject(new Error("No active project"));
315+
return queueDomEditSave(() =>
316+
performPersistDomEditOperations(selection, operations, options, expectedProjectId),
317+
);
318+
},
319+
[performPersistDomEditOperations, projectIdRef, queueDomEditSave],
304320
);
305321

306322
const commitDomEditPatchBatches: CommitDomEditPatchBatches = useCallback(
307-
(batches, options) =>
308-
queueDomEditSave(
323+
(batches, options) => {
324+
const expectedProjectId = projectIdRef.current;
325+
if (!expectedProjectId) return Promise.reject(new Error("No active project"));
326+
return queueDomEditSave(
309327
// One queued transaction owns validation, persistence, history, reload,
310328
// and its durable result; splitting those phases risks partial commits.
311329
// fallow-ignore-next-line complexity
312330
async () => {
313-
const pid = projectIdRef.current;
314-
if (!pid) throw new Error("No active project");
331+
if (projectIdRef.current !== expectedProjectId) {
332+
throw new Error("Active project changed before the edit could be saved");
333+
}
334+
const pid = expectedProjectId;
315335
const unsafeFields = batches.flatMap((batch) =>
316336
batch.patches.flatMap((patch) => findUnsafeDomPatchValues(patch)),
317337
);
@@ -373,7 +393,8 @@ export function useDomEditCommits({
373393
label: options.label,
374394
});
375395
throw error;
376-
}),
396+
});
397+
},
377398
[
378399
domEditSaveTimestampRef,
379400
editHistory,

packages/studio/src/hooks/useDomEditWiring.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ export function useDomEditWiring({
223223
// ── Telemetry & fallback ──
224224

225225
const trackGsapInteractionFailure = useGsapInteractionFailureTelemetry(activeCompPath, showToast);
226-
const makeFetchFallback = useGsapAnimationFetchFallback(projectId, gsapSourceFile);
226+
const makeFetchFallback = useGsapAnimationFetchFallback(projectId);
227227

228228
// ── GSAP selection handlers ──
229229

packages/studio/src/hooks/useGsapAnimationFetchFallback.test.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,18 @@
11
import { describe, expect, it } from "vitest";
22
import type { GsapAnimation, ParsedGsap } from "@hyperframes/core/gsap-parser";
3-
import { selectElementAnimationsOrRetry } from "./useGsapAnimationFetchFallback";
3+
import {
4+
gsapSourceFileForSelection,
5+
selectElementAnimationsOrRetry,
6+
} from "./useGsapAnimationFetchFallback";
7+
import type { DomEditSelection } from "../components/editor/domEditingTypes";
8+
9+
describe("gsapSourceFileForSelection", () => {
10+
it("uses the explicit target source instead of ambient selection state", () => {
11+
expect(
12+
gsapSourceFileForSelection({ sourceFile: "compositions/card.html" } as DomEditSelection),
13+
).toBe("compositions/card.html");
14+
});
15+
});
416

517
const anim = (targetSelector: string): GsapAnimation =>
618
({ id: targetSelector, targetSelector, properties: {} }) as unknown as GsapAnimation;

0 commit comments

Comments
 (0)