mirror of
https://github.com/garrytan/gstack.git
synced 2026-11-13 11:57:19 +01:00
fix(make-pdf): guard every html-to-docx sizing path, keep non-ICNS formats
Review found three spellings that got past the regex guard and still hung --to docx: a data-src decoy, a percent-encoded separator, and an unquoted encoded colon. html-to-docx sizes images on three paths, and buildImage URI-decodes the whole src first. The guard now mirrors all three, drops ICNS (the only input its bundled image-size routes to the looping parser) instead of allowlisting formats, so BMP and TIFF embed as before, and re-parses the result with html-to-docx's own html-to-vdom to fail closed on anything the regex misreads. The golden test tolerates CRLF checkouts on Windows. Version Gate treats a failed file lookup as an empty list, so a partial paginated result cannot decide the exemption. Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
1 parent
8e878d9ee9
commit
65ca746bda
11 files changed
+128
-43
No files matched your search
@@ -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 }}"
|
||||
@@ -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.
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -0,0 +1,78 @@
|
||||
/**
|
||||
* DOCX image guard. html-to-docx 1.8.0 sizes every data-URI <img> 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 <img> 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 <img> 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 = /<img\b[^>]*>/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]+</g, "<").replace(/>[\t ]+</g, "><").replace(/>[\t ]+$/g, ">");
|
||||
}
|
||||
|
||||
function* imgSources(nodes: ImgNode | ImgNode[]): Generator<unknown> {
|
||||
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 <img> tag hides an ICNS image, which html-to-docx's bundled image-size hangs on; convert or remove it");
|
||||
}
|
||||
}
|
||||
return guarded;
|
||||
}
|
||||
Vendored
+3
@@ -0,0 +1,3 @@
|
||||
declare module 'html-to-vdom' {
|
||||
export default function HTMLToVDOM(dependencies: { VNode: Function; VText: Function }): (html: string) => any;
|
||||
}
|
||||
@@ -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*</.test(buf.toString("utf8", 0, 64)) && /<svg\b/i.test(buf.toString("utf8", 0, 4096));
|
||||
}
|
||||
|
||||
function pngDims(b: Buffer): ImageDims | null {
|
||||
// 8-byte signature, then IHDR chunk: length(4) "IHDR"(4) width(4) height(4)
|
||||
if (b.length < 24) return null;
|
||||
|
||||
@@ -41,7 +41,7 @@ import {
|
||||
substituteSlots,
|
||||
} from "./diagram-prepass";
|
||||
import { applyImagePolicy } from "./image-policy";
|
||||
import { docxSafeImage } from "./image-size";
|
||||
import { guardDocxImages } from "./docx-images";
|
||||
|
||||
/** Default output location (`$P generate letter.md` → /tmp/letter.pdf). */
|
||||
export const OUTPUT_TMP_DIR = process.platform === "win32" ? os.tmpdir() : "/tmp";
|
||||
@@ -175,18 +175,7 @@ export async function generate(opts: GenerateOptions): Promise<string> {
|
||||
progress.end("Rasterizing diagrams for DOCX");
|
||||
}
|
||||
finalHtml = convertDiagnosticsForDocx(finalHtml);
|
||||
finalHtml = finalHtml.replace(/<img\b[^>]*>/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 ──
|
||||
|
||||
@@ -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 <img> 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('<?xml version="1.0"?>\n<svg xmlns="http://www.w3.org/2000/svg"></svg>'))).toBe(true);
|
||||
expect(docxSafeImage(icnsLoop())).toBe(false);
|
||||
expect(docxSafeImage(Buffer.concat([icnsLoop(), Buffer.from('<svg width="1" height="1">')]))).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(`<p><img src="${png}"><img src="${bmp}"></p>`)).toBe(`<p><img src="${png}"><img src="${bmp}"></p>`);
|
||||
expect(warnings).toEqual([]);
|
||||
for (const tag of [
|
||||
`<img src="${icns}">`,
|
||||
`<img src='${icns}' alt="x">`,
|
||||
`<img src=${icns}>`,
|
||||
`<IMG SRC="${icns}">`,
|
||||
`<img data-src="${png}" src="${icns}">`,
|
||||
`<img src="${icns.replace("base64,a", "base64,%61")}">`,
|
||||
`<img src="${icns.replace(";", "%3B")}">`,
|
||||
`<img src=${icns.replace(":", "%3A")}>`,
|
||||
]) expect(guard(`<p>${tag}</p>`)).toBe("<p></p>");
|
||||
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(`<p><img alt=">" src="${icns}"></p>`, () => {})).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("<w:drawing>");
|
||||
} finally {
|
||||
fs.rmSync(workDir, { recursive: true, force: true });
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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' });
|
||||
});
|
||||
|
||||
+2
-1
@@ -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/**",
|
||||
|
||||
Reference in new issue
Block a user