diff --git a/.github/workflows/version-gate.yml b/.github/workflows/version-gate.yml index 0dff86cb2..4943a1f6e 100644 --- a/.github/workflows/version-gate.yml +++ b/.github/workflows/version-gate.yml @@ -77,6 +77,6 @@ jobs: PR_VERSION: ${{ steps.versions.outputs.pr_version }} PR_AUTHOR: ${{ github.event.pull_request.user.login }} run: | - PR_CHANGED_FILES=$(gh api --paginate "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files" --jq '.[].filename' || true) + if ! PR_CHANGED_FILES=$(gh api --paginate "repos/${{ github.repository }}/pulls/${{ github.event.pull_request.number }}/files" --jq '.[].filename'); then PR_CHANGED_FILES=""; fi export PR_CHANGED_FILES bun run scripts/compare-pr-version.ts next.json "${{ github.event.pull_request.number }}" diff --git a/TODOS.md b/TODOS.md index 36676711c..aa26453fa 100644 --- a/TODOS.md +++ b/TODOS.md @@ -42,7 +42,7 @@ The 158 older open issues were re-checked against main 5cb5e1c (v1.91.83.0). Of - #2993: tracked under the /cso capacity follow-ups above. **Priority:** P2. - **Dependency security ledgers** (not code defects; keep open until the trigger) - `.osv-scanner.toml` ignores katex 0.16.47 in lib/diagram-render (GHSA-238p-pmpm-9mq7, low) until 2027-01-31. The fix (0.18.2) is outside mermaid 11's range; when mermaid-js/mermaid#8388 ships, override katex, rebuild the bundle and drop the ignore. **Effort:** S. **Priority:** P3. - - html-to-docx 1.8.0 bundles its own image-size 1.x, which the package.json override cannot reach. `make-pdf --to docx` only hands it PNG, JPEG, GIF, WebP or SVG bytes, but with `--allow-network` html-to-docx still downloads and sizes remote images itself. Drop html-to-docx's bundled sizer (a fork or a replacement converter) or validate remote images before conversion. **Effort:** M. **Priority:** P3. + - #2753 residual: html-to-docx 1.8.0 bundles its own image-size 1.x, which the package.json override cannot reach. `make-pdf --to docx` never hands it ICNS bytes (make-pdf/src/docx-images.ts), but with `--allow-network` html-to-docx downloads remote images and sizes them itself. Inline remote images before the guard for docx, or replace html-to-docx. Revisit 2026-12-15. **Effort:** M. **Priority:** P2. - **Tests and CI** - #2756: the periodic lane. Tracked under "Periodic-lane stabilization (#2756)". **Priority:** P2. - #2967: `test/redact-prepush-scan-range.test.ts` (and its rebase twin) run fixture pushes with the inherited global config, so a developer's `core.hooksPath` pre-push runs inside the fixture. Set `GIT_CONFIG_GLOBAL=/dev/null` and `GIT_CONFIG_NOSYSTEM=1` in the helper. **Effort:** S. **Priority:** P3. diff --git a/bun.lock b/bun.lock index f3360c084..f96507bb4 100644 --- a/bun.lock +++ b/bun.lock @@ -10,6 +10,7 @@ "cross-spawn": "^7.0.6", "diff": "^9.0.0", "html-to-docx": "1.8.0", + "html-to-vdom": "0.7.0", "marked": "^18.0.11", "playwright": "^1.62.1", "sharp": "^0.35.4", diff --git a/make-pdf/src/docx-images.ts b/make-pdf/src/docx-images.ts new file mode 100644 index 000000000..1df00b6d6 --- /dev/null +++ b/make-pdf/src/docx-images.ts @@ -0,0 +1,78 @@ +/** + * DOCX image guard. html-to-docx 1.8.0 sizes every data-URI with its + * own bundled copy of image-size 1.x, which the package.json override cannot + * reach. That copy's detector sends a buffer to its ICNS parser exactly when + * the bytes start with "icns", and the parser loops forever on a zero-length + * entry (GHSA-w3rx-r6r6-pgpr). No other parser in the copy has a known flaw. + * + * guardDocxImages drops the tags it can see that would size unsafe + * bytes, then re-parses the result with the same html-to-vdom html-to-docx + * uses and refuses the document if any it would size is still unsafe, + * so a tag the regex misreads fails closed instead of hanging. + */ +import HTMLToVDOM from "html-to-vdom"; + +/** html-to-docx's data-URI pattern, shared by all three of its sizeOf call sites. */ +const DATA_URI = /^data:([A-Za-z-+\/]+);base64,(.+)$/; +const IMG_TAG = /]*>/gi; +const SRC_ATTR = /(?:^|\s)src\s*=\s*(?:"([^"]*)"|'([^']*)'|([^\s>]+))/i; + +interface ImgNode { tagName?: string; properties?: { src?: unknown }; children?: ImgNode[] } + +const toVdom = HTMLToVDOM({ + VNode: function (this: ImgNode, tagName: string, properties: ImgNode["properties"], children: ImgNode[]) { + Object.assign(this, { tagName, properties, children }); + }, + VText: function () {}, +}); + +function tryDecode(text: string): string | null { + try { + return decodeURIComponent(text); + } catch { + return null; + } +} + +function isIcns(base64: string | null | undefined): boolean { + return base64 != null && Buffer.from(base64, "base64").toString("ascii", 0, 4) === "icns"; +} + +/** + * False when html-to-docx would hand ICNS bytes to its bundled sizer. Its two + * paragraph paths match the raw src and URI-decode the payload; buildImage + * URI-decodes the whole src, matches that, and sizes the payload as is. + */ +function sizesSafely(src: unknown): boolean { + if (typeof src !== "string") return true; + const raw = src.match(DATA_URI); + if (raw && isIcns(tryDecode(raw[2]))) return false; + return !isIcns(tryDecode(src)?.match(DATA_URI)?.[2]); +} + +/** html-to-docx's minifyHTMLString, so the re-parse sees the string it parses. */ +function minify(html: string): string { + return html.replace(/\n/g, " ").replace(/\r/g, " ").replace(/[\t]+[\t ]+<").replace(/>[\t ]+$/g, ">"); +} + +function* imgSources(nodes: ImgNode | ImgNode[]): Generator { + for (const node of Array.isArray(nodes) ? nodes : [nodes]) { + if (node.tagName === "img") yield node.properties?.src; + if (node.children) yield* imgSources(node.children); + } +} + +export function guardDocxImages(html: string, warn: (msg: string) => void): string { + const guarded = html.replace(IMG_TAG, (tag) => { + const src = tag.match(SRC_ATTR); + if (sizesSafely(src?.[1] ?? src?.[2] ?? src?.[3])) return tag; + warn("docx: dropped an ICNS image (html-to-docx's bundled image-size hangs on it)"); + return ""; + }); + for (const src of imgSources(toVdom(minify(guarded)))) { + if (!sizesSafely(src)) { + throw new Error("docx: an tag hides an ICNS image, which html-to-docx's bundled image-size hangs on; convert or remove it"); + } + } + return guarded; +} diff --git a/make-pdf/src/html-to-vdom.d.ts b/make-pdf/src/html-to-vdom.d.ts new file mode 100644 index 000000000..4224f4fdf --- /dev/null +++ b/make-pdf/src/html-to-vdom.d.ts @@ -0,0 +1,3 @@ +declare module 'html-to-vdom' { + export default function HTMLToVDOM(dependencies: { VNode: Function; VText: Function }): (html: string) => any; +} diff --git a/make-pdf/src/image-size.ts b/make-pdf/src/image-size.ts index 94d1e02a9..e4690c1b8 100644 --- a/make-pdf/src/image-size.ts +++ b/make-pdf/src/image-size.ts @@ -21,18 +21,6 @@ export function imageDims(buf: Buffer): ImageDims | null { return pngDims(buf) ?? jpegDims(buf) ?? gifDims(buf) ?? webpDims(buf) ?? svgDims(buf); } -/** - * html-to-docx 1.8.0 sizes every embedded image with its own bundled copy of - * image-size 1.x. The package.json override cannot reach that copy, and its - * ICNS parser loops forever on a zero-length entry (GHSA-w3rx-r6r6-pgpr), so - * DOCX conversion only hands it bytes that are recognizably PNG, JPEG, GIF or - * WebP, or text that opens as SVG markup. - */ -export function docxSafeImage(buf: Buffer): boolean { - if (pngDims(buf) ?? jpegDims(buf) ?? gifDims(buf) ?? webpDims(buf)) return true; - return /^\uFEFF?\s* { progress.end("Rasterizing diagrams for DOCX"); } finalHtml = convertDiagnosticsForDocx(finalHtml); - finalHtml = finalHtml.replace(/]*>/gi, (tag) => { - const src = tag.match(/\bsrc\s*=\s*(?:"([^"]*)"|'([^']*)'|([^\s>]+))/i); - const payload = (src?.[1] ?? src?.[2] ?? src?.[3] ?? "").match(/^data:[^;,]+;base64,([\s\S]+)$/)?.[1]; - if (payload === undefined) return tag; - let base64 = ""; - try { - base64 = decodeURIComponent(payload); - } catch { /* malformed escape: drop it below */ } - if (/^[A-Za-z0-9+/=\s]+$/.test(base64) && docxSafeImage(Buffer.from(base64, "base64"))) return tag; - warn("docx: dropped an image that is not PNG, JPEG, GIF, WebP or SVG"); - return ""; - }); + finalHtml = guardDocxImages(finalHtml, warn); } // ─── --to html: write the self-contained document, no print round-trip ── diff --git a/make-pdf/test/docx-image-size.test.ts b/make-pdf/test/docx-image-size.test.ts index ffdfbde5f..8301303c9 100644 --- a/make-pdf/test/docx-image-size.test.ts +++ b/make-pdf/test/docx-image-size.test.ts @@ -1,11 +1,10 @@ /** * DOCX image sizing (#2753). package.json overrides image-size to 2.x, which - * fixes GHSA-5p2g-fcmc-qvqq and GHSA-w3rx-r6r6-pgpr for the lockfile, but - * html-to-docx 1.8.0 runs a bundled image-size 1.x copy the override cannot - * reach. The golden document.xml was captured before the override, so any - * change to the Word output shows up as a diff here. The orchestrator only - * hands html-to-docx images whose bytes are PNG, JPEG, GIF, WebP or SVG, so - * the bundled ICNS parser's infinite loop never runs. + * fixes GHSA-5p2g-fcmc-qvqq and GHSA-w3rx-r6r6-pgpr in the lockfile, but + * html-to-docx 1.8.0 never loads it: it runs its own bundled image-size 1.x. + * The golden document.xml pins the Word output the change must not move. The + * guard tests prove ICNS bytes, which hang that bundled copy, never reach it, + * however the tag is spelled. */ import { describe, expect, test } from "bun:test"; import { execFileSync } from "node:child_process"; @@ -13,7 +12,7 @@ import * as fs from "node:fs"; import * as os from "node:os"; import * as path from "node:path"; -import { docxSafeImage } from "../src/image-size"; +import { guardDocxImages } from "../src/docx-images"; import { generate } from "../src/orchestrator"; const FIXTURES = path.join(import.meta.dir, "fixtures", "docx-images"); @@ -56,18 +55,42 @@ describe("make-pdf --to docx image sizing", () => { } const normalized = xml.replace(/image-[A-Za-z0-9_-]{21}\.(png|jpeg|gif)/g, "image-ID.$1"); - expect(normalized.trimEnd()).toBe(fs.readFileSync(path.join(FIXTURES, "document.golden.xml"), "utf8").trimEnd()); + const golden = fs.readFileSync(path.join(FIXTURES, "document.golden.xml"), "utf8").replace(/\r\n/g, "\n"); + expect(normalized.trimEnd()).toBe(golden.trimEnd()); } finally { fs.rmSync(workDir, { recursive: true, force: true }); } }); - test("only PNG, JPEG, GIF, WebP and SVG markup reach html-to-docx's bundled sizer", () => { - for (const img of IMAGES) expect(docxSafeImage(fs.readFileSync(path.join(FIXTURES, img.file)))).toBe(true); - expect(docxSafeImage(Buffer.from('\n'))).toBe(true); - expect(docxSafeImage(icnsLoop())).toBe(false); - expect(docxSafeImage(Buffer.concat([icnsLoop(), Buffer.from('')]))).toBe(false); - expect(docxSafeImage(Buffer.from("BM not an image at all"))).toBe(false); + test("the guard drops ICNS images however the tag is spelled and keeps every other format", () => { + const icns = "data:image/png;base64," + icnsLoop().toString("base64"); + const png = "data:image/png;base64," + fs.readFileSync(path.join(FIXTURES, "wide.png")).toString("base64"); + const bmp = "data:image/bmp;base64," + Buffer.from("BM" + "\0".repeat(52)).toString("base64"); + const warnings: string[] = []; + const guard = (html: string) => guardDocxImages(html, msg => warnings.push(msg)); + expect(guard(`

`)).toBe(`

`); + expect(warnings).toEqual([]); + for (const tag of [ + ``, + `x`, + ``, + ``, + ``, + ``, + ``, + ``, + ]) expect(guard(`

${tag}

`)).toBe("

"); + expect(warnings).toHaveLength(8); + }); + + test("a tag the regex misreads fails closed through html-to-docx's own parser", () => { + const icns = "data:image/png;base64," + icnsLoop().toString("base64"); + expect(() => guardDocxImages(`

>

`, () => {})).toThrow("hides an ICNS image"); + }); + + test("the re-parse uses the html-to-vdom copy html-to-docx loads", () => { + const fromDocx = require.resolve("html-to-vdom/package.json", { paths: [path.dirname(require.resolve("html-to-docx/package.json"))] }); + expect(fromDocx).toBe(require.resolve("html-to-vdom/package.json")); }); test("an ICNS file named .png is dropped with a warning instead of hanging the conversion", () => { @@ -81,7 +104,7 @@ await generate({ input: ${JSON.stringify(path.join(workDir, "doc.md"))}, output: const run = Bun.spawnSync([process.execPath, "-e", script], { timeout: 30_000, stdout: "pipe", stderr: "pipe" }); const stderr = run.stderr.toString(); expect(run.exitCode).toBe(0); - expect(stderr.match(/docx: dropped an image that is not PNG, JPEG, GIF, WebP or SVG/g)).toHaveLength(2); + expect(stderr.match(/docx: dropped an ICNS image/g)).toHaveLength(2); expect(unzip(["-p", out, "word/document.xml"]).toString("utf8")).not.toContain(""); } finally { fs.rmSync(workDir, { recursive: true, force: true }); diff --git a/package.json b/package.json index 61a646cb7..6768bd83a 100644 --- a/package.json +++ b/package.json @@ -73,6 +73,7 @@ "cross-spawn": "^7.0.6", "diff": "^9.0.0", "html-to-docx": "1.8.0", + "html-to-vdom": "0.7.0", "marked": "^18.0.11", "playwright": "^1.62.1", "sharp": "^0.35.4", diff --git a/test/compare-pr-version.test.ts b/test/compare-pr-version.test.ts index dd5013ee2..e4a4d3890 100644 --- a/test/compare-pr-version.test.ts +++ b/test/compare-pr-version.test.ts @@ -73,6 +73,7 @@ describe('version gate: dependabot dependency-manifest exemption', () => { expect(step.env.GH_TOKEN).toBe('${{ secrets.GITHUB_TOKEN }}'); expect(step.run).toContain('pulls/${{ github.event.pull_request.number }}/files'); expect(step.run).toContain('--paginate'); + expect(step.run).toContain('then PR_CHANGED_FILES=""; fi'); expect(step.run).toContain('export PR_CHANGED_FILES'); expect(workflow.jobs.check.permissions).toEqual({ contents: 'read', 'pull-requests': 'read' }); }); diff --git a/tsconfig.test.json b/tsconfig.test.json index 6276a3d73..da9480913 100644 --- a/tsconfig.test.json +++ b/tsconfig.test.json @@ -8,7 +8,8 @@ "design/test/**/*.ts", "ios-qa/**/*.test.ts", "ios-qa/daemon/test/**/*.ts", - "make-pdf/src/html-to-docx.d.ts" + "make-pdf/src/html-to-docx.d.ts", + "make-pdf/src/html-to-vdom.d.ts" ], "exclude": [ "lib/diagram-render/**",