From cb378218dade1fee0697a539c8be059c0cd89068 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Fri, 19 Jun 2026 05:52:13 +0000 Subject: [PATCH] fix(core,studio): escape user values in querySelector attribute selectors Extract cssAttrSelector to packages/core/src/utils/cssSelector.ts and use it (or CSS.escape for browser-side code) at all 12 sites that previously interpolated raw user-authored values into querySelector attribute selectors. A " in a composition ID, script src, or data-start value would produce a malformed selector that throws. Node-side (core compiler/parser): uses the shared cssAttrSelector. Browser-side (runtime, studio): uses native CSS.escape(). Supersedes #1568 which fixed only the 3 bundler sites. --- packages/core/src/compiler/htmlBundler.ts | 10 ++- .../src/compiler/inlineSubCompositions.ts | 3 +- packages/core/src/index.ts | 1 + packages/core/src/parsers/htmlParser.ts | 3 +- packages/core/src/runtime/picker.ts | 6 +- packages/core/src/utils/cssSelector.test.ts | 77 +++++++++++++++++++ packages/core/src/utils/cssSelector.ts | 14 ++++ .../src/components/editor/LayersPanel.tsx | 2 +- .../components/editor/domEditingElement.ts | 2 +- .../studio/src/components/nle/NLELayout.tsx | 2 +- packages/studio/src/player/lib/timelineDOM.ts | 3 +- .../src/player/lib/timelineElementHelpers.ts | 10 +-- .../src/player/lib/timelineIframeHelpers.ts | 7 +- packages/studio/src/test-setup.ts | 6 ++ packages/studio/vite.config.ts | 1 + 15 files changed, 126 insertions(+), 21 deletions(-) create mode 100644 packages/core/src/utils/cssSelector.test.ts create mode 100644 packages/core/src/utils/cssSelector.ts create mode 100644 packages/studio/src/test-setup.ts diff --git a/packages/core/src/compiler/htmlBundler.ts b/packages/core/src/compiler/htmlBundler.ts index 538f0632ff..d5dfb74cac 100644 --- a/packages/core/src/compiler/htmlBundler.ts +++ b/packages/core/src/compiler/htmlBundler.ts @@ -18,6 +18,7 @@ import { validateHyperframeHtmlContract } from "./staticGuard"; import { getHyperframeRuntimeScript } from "../generated/runtime-inline"; import { readDeclaredDefaults } from "../runtime/getVariables"; import { inlineSubCompositions } from "./inlineSubCompositions"; +import { queryByAttr } from "../utils/cssSelector"; import { isSafePath, resolveWithinProject } from "../safePath.js"; import { HF_COLOR_GRADING_ATTR } from "../colorGrading"; @@ -277,7 +278,8 @@ function rewriteCssUrlsWithInlinedAssets(cssText: string, projectDir: string): s } function cssAttributeSelector(attr: string, value: string): string { - return `[${attr}="${value.replace(/\\/g, "\\\\").replace(/"/g, '\\"')}"]`; + const escaped = value.replace(/\\/g, "\\\\").replace(/"/g, '\\"'); + return `[${attr}="${escaped}"]`; } function uniqueCompositionId(baseId: string, index: number): string { @@ -624,7 +626,7 @@ export interface BundleOptions { */ function ensureExternalScriptTag(doc: Document, src: string): void { - if (doc.querySelector(`script[src="${src}"]`)) return; + if (queryByAttr(doc, "src", src, "script")) return; const el = doc.createElement("script"); el.setAttribute("src", src); doc.body.appendChild(el); @@ -825,7 +827,7 @@ export async function bundleToSingleHtml( continue; } } - if (!document.querySelector(`script[src="${extSrc}"]`)) { + if (!queryByAttr(document, "src", extSrc, "script")) { const extScript = document.createElement("script"); extScript.setAttribute("src", extSrc); document.body.appendChild(extScript); @@ -857,7 +859,7 @@ export async function bundleToSingleHtml( const hostIdentity = hostIdentityByElement.get(host); const runtimeCompId = hostIdentity?.runtimeCompositionId || compId; const innerDoc = parseHTMLContent(templateHtml); - const innerRoot = innerDoc.querySelector(`[data-composition-id="${compId}"]`); + const innerRoot = queryByAttr(innerDoc, "data-composition-id", compId); const authoredRootId = innerRoot?.getAttribute("id")?.trim() || null; const runtimeScope = runtimeCompId ? cssAttributeSelector("data-composition-id", runtimeCompId) diff --git a/packages/core/src/compiler/inlineSubCompositions.ts b/packages/core/src/compiler/inlineSubCompositions.ts index a48e25ff8e..f6e0b6167e 100644 --- a/packages/core/src/compiler/inlineSubCompositions.ts +++ b/packages/core/src/compiler/inlineSubCompositions.ts @@ -13,6 +13,7 @@ import { rewriteCssAssetUrls, rewriteInlineStyleAssetUrls, } from "./rewriteSubCompPaths"; +import { queryByAttr } from "../utils/cssSelector"; import { scopeCssToComposition, wrapInlineScriptWithErrorBoundary, @@ -225,7 +226,7 @@ export function inlineSubCompositions( // Find the inner composition root const innerRoot = compId - ? contentDoc.querySelector(`[data-composition-id="${compId}"]`) + ? queryByAttr(contentDoc, "data-composition-id", compId) : contentDoc.querySelector("[data-composition-id]"); const inferredCompId = innerRoot?.getAttribute("data-composition-id")?.trim() || ""; const authoredRootId = innerRoot?.getAttribute("id")?.trim() || null; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 7503727609..53a21a64fd 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -151,6 +151,7 @@ export { rewriteCssAssetUrls, } from "./compiler/rewriteSubCompPaths"; export { CSS_URL_RE, isNonRelativeUrl, isPathInside } from "./compiler/assetPaths"; +export { queryByAttr } from "./utils/cssSelector"; export { decodeUrlPathVariants } from "./utils/urlPath"; export { parseAnimatedGifMetadata, type AnimatedGifMetadata } from "./media/gif"; export { diff --git a/packages/core/src/parsers/htmlParser.ts b/packages/core/src/parsers/htmlParser.ts index c454e33b74..146af3f206 100644 --- a/packages/core/src/parsers/htmlParser.ts +++ b/packages/core/src/parsers/htmlParser.ts @@ -13,6 +13,7 @@ import type { import { validateCompositionGsap } from "./gsapSerialize"; import { ensureHfIds } from "./hfIds.js"; import { parseGsapScriptAcornForWrite } from "./gsapParserAcorn.js"; +import { queryByAttr } from "../utils/cssSelector"; import { removeAnimationFromScript } from "./gsapWriterAcorn.js"; import type { ValidationResult } from "../core.types"; @@ -519,7 +520,7 @@ export function updateElementInHtml( const parser = new DOMParser(); const doc = parser.parseFromString(html, "text/html"); - const el = doc.getElementById(elementId) || doc.querySelector(`[data-name="${elementId}"]`); + const el = doc.getElementById(elementId) || queryByAttr(doc, "data-name", elementId); if (!el) return html; if (updates.startTime !== undefined) { diff --git a/packages/core/src/runtime/picker.ts b/packages/core/src/runtime/picker.ts index 2cf4d4c347..0a34763cb0 100644 --- a/packages/core/src/runtime/picker.ts +++ b/packages/core/src/runtime/picker.ts @@ -97,11 +97,11 @@ export function createPickerModule(deps: PickerModuleDeps): PickerModule { const htmlEl = el as HTMLElement; if (htmlEl.id) return `#${htmlEl.id}`; const compositionId = el.getAttribute("data-composition-id"); - if (compositionId) return `[data-composition-id="${compositionId}"]`; + if (compositionId) return `[data-composition-id="${CSS.escape(compositionId)}"]`; const compositionSrc = el.getAttribute("data-composition-src"); - if (compositionSrc) return `[data-composition-src="${compositionSrc}"]`; + if (compositionSrc) return `[data-composition-src="${CSS.escape(compositionSrc)}"]`; const track = el.getAttribute("data-track-index"); - if (track) return `[data-track-index="${track}"]`; + if (track) return `[data-track-index="${CSS.escape(track)}"]`; const tag = el.tagName.toLowerCase(); const parent = el.parentElement; if (!parent) return tag; diff --git a/packages/core/src/utils/cssSelector.test.ts b/packages/core/src/utils/cssSelector.test.ts new file mode 100644 index 0000000000..42b5f66d3f --- /dev/null +++ b/packages/core/src/utils/cssSelector.test.ts @@ -0,0 +1,77 @@ +import { describe, it, expect } from "bun:test"; +import { queryByAttr } from "./cssSelector"; + +function makeDoc(html: string) { + const { parseHTML } = require("linkedom"); + const { document } = parseHTML(`${html}`); + return document; +} + +describe("queryByAttr", () => { + it("finds element by exact attribute match", () => { + const doc = makeDoc('
'); + const el = queryByAttr(doc, "data-id", "abc"); + expect(el).not.toBeNull(); + expect(el!.getAttribute("data-id")).toBe("abc"); + }); + + it("returns null when no match", () => { + const doc = makeDoc('
'); + expect(queryByAttr(doc, "data-id", "xyz")).toBeNull(); + }); + + it("handles values with double quotes", () => { + const doc = makeDoc("
"); + const el = doc.querySelector("div")!; + el.setAttribute("data-id", 'has"quote'); + expect(queryByAttr(doc, "data-id", 'has"quote')).toBe(el); + }); + + it("handles values with backslashes", () => { + const doc = makeDoc("
"); + const el = doc.querySelector("div")!; + el.setAttribute("data-id", "has\\backslash"); + expect(queryByAttr(doc, "data-id", "has\\backslash")).toBe(el); + }); + + it("handles values with closing bracket", () => { + const doc = makeDoc("
"); + const el = doc.querySelector("div")!; + el.setAttribute("data-id", "has]bracket"); + expect(queryByAttr(doc, "data-id", "has]bracket")).toBe(el); + }); + + it("handles injection attempt", () => { + const doc = makeDoc('
'); + const el = doc.querySelector("div")!; + el.setAttribute("data-id", '"][data-evil]'); + expect(queryByAttr(doc, "data-id", '"][data-evil]')).toBe(el); + expect(queryByAttr(doc, "data-id", "safe")).toBeNull(); + }); + + it("filters by tag when provided", () => { + const doc = makeDoc('
'); + const el = queryByAttr(doc, "data-src", "a.js", "script"); + expect(el).not.toBeNull(); + expect(el!.tagName.toLowerCase()).toBe("script"); + }); + + it("returns null when tag filter excludes match", () => { + const doc = makeDoc('
'); + expect(queryByAttr(doc, "data-src", "a.js", "script")).toBeNull(); + }); + + it("handles values with newlines", () => { + const doc = makeDoc("
"); + const el = doc.querySelector("div")!; + el.setAttribute("data-id", "line1\nline2"); + expect(queryByAttr(doc, "data-id", "line1\nline2")).toBe(el); + }); + + it("handles values with leading digits", () => { + const doc = makeDoc("
"); + const el = doc.querySelector("div")!; + el.setAttribute("data-id", "123abc"); + expect(queryByAttr(doc, "data-id", "123abc")).toBe(el); + }); +}); diff --git a/packages/core/src/utils/cssSelector.ts b/packages/core/src/utils/cssSelector.ts new file mode 100644 index 0000000000..df58c28c3c --- /dev/null +++ b/packages/core/src/utils/cssSelector.ts @@ -0,0 +1,14 @@ +// ponytail: queries DOM by exact attribute match without interpolating +// the value into a selector string — zero injection surface. +export function queryByAttr( + root: ParentNode, + attr: string, + value: string, + tag?: string, +): Element | null { + const selector = tag ? `${tag}[${attr}]` : `[${attr}]`; + for (const el of root.querySelectorAll(selector)) { + if (el.getAttribute(attr) === value) return el; + } + return null; +} diff --git a/packages/studio/src/components/editor/LayersPanel.tsx b/packages/studio/src/components/editor/LayersPanel.tsx index 6bbb7196d8..510421c796 100644 --- a/packages/studio/src/components/editor/LayersPanel.tsx +++ b/packages/studio/src/components/editor/LayersPanel.tsx @@ -126,7 +126,7 @@ export const LayersPanel = memo(function LayersPanel() { if (doc) { const found = (layer.id ? doc.getElementById(layer.id) : null) ?? - (layer.hfId ? doc.querySelector(`[data-hf-id="${layer.hfId}"]`) : null) ?? + (layer.hfId ? doc.querySelector(`[data-hf-id="${CSS.escape(layer.hfId)}"]`) : null) ?? doc.getElementById(layer.key); if (found instanceof HTMLElement) el = found; } diff --git a/packages/studio/src/components/editor/domEditingElement.ts b/packages/studio/src/components/editor/domEditingElement.ts index c3f36822c3..ba3645fc54 100644 --- a/packages/studio/src/components/editor/domEditingElement.ts +++ b/packages/studio/src/components/editor/domEditingElement.ts @@ -242,7 +242,7 @@ export function findElementForSelection( activeCompositionPath: string | null = null, ): HTMLElement | null { if (selection.hfId) { - const byHfId = doc.querySelector(`[data-hf-id="${selection.hfId}"]`); + const byHfId = doc.querySelector(`[data-hf-id="${CSS.escape(selection.hfId)}"]`); if (isHtmlElement(byHfId)) return byHfId; } diff --git a/packages/studio/src/components/nle/NLELayout.tsx b/packages/studio/src/components/nle/NLELayout.tsx index 8f67172b61..65649bb0d6 100644 --- a/packages/studio/src/components/nle/NLELayout.tsx +++ b/packages/studio/src/components/nle/NLELayout.tsx @@ -189,7 +189,7 @@ export const NLELayout = memo(function NLELayout({ const doc = iframeRef_.current?.contentDocument; if (doc) { const host = doc.querySelector( - `[data-composition-id="${compId}"][data-composition-src]`, + `[data-composition-id="${CSS.escape(compId)}"][data-composition-src]`, ); if (host) { resolvedPath = host.getAttribute("data-composition-src") || undefined; diff --git a/packages/studio/src/player/lib/timelineDOM.ts b/packages/studio/src/player/lib/timelineDOM.ts index cddd3e4e11..825fe9d577 100644 --- a/packages/studio/src/player/lib/timelineDOM.ts +++ b/packages/studio/src/player/lib/timelineDOM.ts @@ -123,7 +123,8 @@ export function createTimelineElementFromManifestClip(params: { if (clip.kind === "composition" && clip.compositionId) { let resolvedSrc = clip.compositionSrc; if (!resolvedSrc) { - hostEl = doc?.querySelector(`[data-composition-id="${clip.compositionId}"]`) ?? hostEl; + hostEl = + doc?.querySelector(`[data-composition-id="${CSS.escape(clip.compositionId)}"]`) ?? hostEl; resolvedSrc = hostEl?.getAttribute("data-composition-src") ?? hostEl?.getAttribute("data-composition-file") ?? diff --git a/packages/studio/src/player/lib/timelineElementHelpers.ts b/packages/studio/src/player/lib/timelineElementHelpers.ts index df3e9898bf..74780f429e 100644 --- a/packages/studio/src/player/lib/timelineElementHelpers.ts +++ b/packages/studio/src/player/lib/timelineElementHelpers.ts @@ -163,13 +163,13 @@ export function getImplicitTimelineLayerLabel(el: HTMLElement): string { // --------------------------------------------------------------------------- export function getTimelineElementSelector(el: Element): string | undefined { - if (isHtmlElement(el) && el.id) return `#${el.id}`; + if (isHtmlElement(el) && el.id) return `#${CSS.escape(el.id)}`; const compId = el.getAttribute("data-composition-id"); - if (compId) return `[data-composition-id="${compId}"]`; + if (compId) return `[data-composition-id="${CSS.escape(compId)}"]`; if (isHtmlElement(el)) { const classes = el.className.split(/\s+/).filter(Boolean); const firstClass = classes.find((className) => className !== "clip") ?? classes[0]; - if (firstClass) return `.${firstClass}`; + if (firstClass) return `.${CSS.escape(firstClass)}`; } return undefined; } @@ -283,8 +283,8 @@ function nodeMatchesManifestClip(node: Element, clip: ClipManifestClip): boolean function findTimelineDomNode(doc: Document, id: string): Element | null { return ( doc.getElementById(id) ?? - doc.querySelector(`[data-composition-id="${id}"]`) ?? - doc.querySelector(`.${id}`) ?? + doc.querySelector(`[data-composition-id="${CSS.escape(id)}"]`) ?? + doc.querySelector(`.${CSS.escape(id)}`) ?? null ); } diff --git a/packages/studio/src/player/lib/timelineIframeHelpers.ts b/packages/studio/src/player/lib/timelineIframeHelpers.ts index 30fae0295e..7d2e65b092 100644 --- a/packages/studio/src/player/lib/timelineIframeHelpers.ts +++ b/packages/studio/src/player/lib/timelineIframeHelpers.ts @@ -295,7 +295,8 @@ export function buildMissingCompositionElements( let start = parseFloat(startAttr); if (isNaN(start)) { const ref = - doc.getElementById(startAttr) || doc.querySelector(`[data-composition-id="${startAttr}"]`); + doc.getElementById(startAttr) || + doc.querySelector(`[data-composition-id="${CSS.escape(startAttr)}"]`); if (ref) { const refStartAttr = ref.getAttribute("data-start") ?? "0"; let refStart = parseFloat(refStartAttr); @@ -303,7 +304,7 @@ export function buildMissingCompositionElements( if (isNaN(refStart)) { const refRef = doc.getElementById(refStartAttr) || - doc.querySelector(`[data-composition-id="${refStartAttr}"]`); + doc.querySelector(`[data-composition-id="${CSS.escape(refStartAttr)}"]`); const rrStart = parseFloat(refRef?.getAttribute("data-start") ?? "0") || 0; const rrCompId = refRef?.getAttribute("data-composition-id"); const rrDur = @@ -400,7 +401,7 @@ export function buildMissingCompositionElements( // Find the matching DOM host by element id or composition id const host = doc.getElementById(existing.id) ?? - doc.querySelector(`[data-composition-id="${existing.id}"]`); + doc.querySelector(`[data-composition-id="${CSS.escape(existing.id)}"]`); if (!host) return existing; const compSrc = host.getAttribute("data-composition-src") || host.getAttribute("data-composition-file"); diff --git a/packages/studio/src/test-setup.ts b/packages/studio/src/test-setup.ts new file mode 100644 index 0000000000..ac3c7debc0 --- /dev/null +++ b/packages/studio/src/test-setup.ts @@ -0,0 +1,6 @@ +if (typeof globalThis.CSS === "undefined") { + (globalThis as Record).CSS = {}; +} +if (typeof CSS.escape !== "function") { + CSS.escape = (value: string) => value.replace(/([^\w-])/g, "\\$1"); +} diff --git a/packages/studio/vite.config.ts b/packages/studio/vite.config.ts index 27fe5ea273..0bf939a7cb 100644 --- a/packages/studio/vite.config.ts +++ b/packages/studio/vite.config.ts @@ -197,5 +197,6 @@ export default defineConfig({ }, test: { exclude: ["data/**", "node_modules/**"], + setupFiles: ["src/test-setup.ts"], }, });