From 981752fea566a9af392bae54f0a411192df00d78 Mon Sep 17 00:00:00 2001 From: Kirill Klimuk Date: Fri, 30 Oct 2026 14:58:46 -0700 Subject: [PATCH 1/2] fix(render): make LibreOffice rendering and file:// images work on Windows Every LibreOffice render failed on Windows: Bun.spawn can't launch soffice.com, the path both `where soffice` and our fallbacks returned. With that fixed, soffice then hung on the file://C:\... profile URL, so the URL now comes from pathToFileURL. The engine also gets a 150s cap, matching Word for Windows, so a stuck soffice fails rather than blocking forever. file:///C:/x.png became /C:/x.png, and %20 stayed literal on every platform. Host-less file URLs now convert properly. One with a host keeps its old meaning, since converting it on Windows would make a UNC path out of an attacker-influenced --image. render.test.ts now asks the engine whether LibreOffice is available, instead of spawning /bin/sh, which crashed the whole file on Windows. A new windows-render CI job keeps all of this covered. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 25 +++++++++++++++ CONTRIBUTING.md | 1 + src/core/image/source.ts | 16 ++++++++-- src/core/render/CLAUDE.md | 4 ++- src/core/render/engines/libreoffice.ts | 42 ++++++++++++++++++++++---- tests/cli/images.test.ts | 21 +++++++++++++ tests/cli/render.test.ts | 14 ++------- 7 files changed, 101 insertions(+), 22 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cdcf4a8..76f1667 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -86,6 +86,31 @@ jobs: - run: bun install --frozen-lockfile - run: DOCX_LO_ALL=1 bun run test:integration # full fixture sweep in CI + windows-render: + name: Render through LibreOffice (Windows) + runs-on: windows-latest + defaults: + run: + shell: bash + steps: + - uses: actions/checkout@v6 + - uses: oven-sh/setup-bun@v2 + - uses: actions/cache@v5 + with: + path: ~/.bun/install/cache + key: bun-${{ runner.os }}-${{ hashFiles('bun.lock') }} + restore-keys: bun-${{ runner.os }}- + - name: Install LibreOffice + shell: pwsh + run: choco install libreoffice-fresh -y --no-progress --limit-output + - run: bun install --frozen-lockfile + - run: bun test tests/cli/render.test.ts tests/cli/images.test.ts + - run: bun run build:binary + - name: Render with the compiled binary + run: | + ./dist/docx render tests/fixtures/markdown-import.docx --out smoke --engine libreoffice --pages 1 + test -s smoke/page-001.png + build-binary: name: Smoke-build compiled binary (${{ matrix.os }}) runs-on: ${{ matrix.os }} diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b5f2e2a..fce0623 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -118,6 +118,7 @@ GitHub Actions (`.github/workflows/ci.yml`) runs these jobs on push to `main` an | `check` | `biome check . && knip-bun && tsc --noEmit` | | `unit-tests` | `bun run test:unit` and `bun run test:benchmark` (no model calls) | | `macos-output` | Output contract and real-pipe regressions on macOS with Bun 1.3.10 and latest | +| `windows-render` | Installs LibreOffice on Windows; runs the render and image tests, then renders with the compiled binary | | `integration-tests` | Installs LibreOffice, runs `bun run test:integration` | | `build-binary` | Builds on Linux and native Intel/ARM macOS; signs/verifies macOS binaries; runs version/help, read, and schema-validation smoke checks | diff --git a/src/core/image/source.ts b/src/core/image/source.ts index 0f8c11f..1c7af6f 100644 --- a/src/core/image/source.ts +++ b/src/core/image/source.ts @@ -4,6 +4,7 @@ // (parsed from the file header) used to size the drawing's . import { lookup } from "node:dns/promises"; +import { fileURLToPath } from "node:url"; import convert from "heic-convert"; import { CANONICAL_IMAGE_EXTENSION, @@ -235,9 +236,7 @@ async function readBodyWithCap( } async function loadFile(path: string): Promise { - const cleaned = path.startsWith("file://") - ? path.slice("file://".length) - : path; + const cleaned = path.startsWith("file://") ? pathFromFileUrl(path) : path; const file = Bun.file(cleaned); if (!(await file.exists())) { throw new ImageSourceError(`Image file not found: ${cleaned}`); @@ -322,6 +321,17 @@ async function transcodeHEICToJPEG(bytes: Uint8Array): Promise { } } +/** `file:///C:/x.png` is `C:\x.png` on Windows, not `/C:/x.png`, and a percent-encoded + * space is a space. A URL with a host keeps meaning the path after the prefix, as it + * always has (`file://relative/x.png`): on Windows, converting it would make a UNC path, + * turning an attacker-influenced `--image` into a request to their server. */ +function pathFromFileUrl(url: string): string { + try { + if (!new URL(url).host) return fileURLToPath(url); + } catch {} + return url.slice("file://".length); +} + function extensionFromPath(path: string): string | undefined { const lastDot = path.lastIndexOf("."); const lastSlash = path.lastIndexOf("/"); diff --git a/src/core/render/CLAUDE.md b/src/core/render/CLAUDE.md index f497c77..95a8643 100644 --- a/src/core/render/CLAUDE.md +++ b/src/core/render/CLAUDE.md @@ -80,7 +80,9 @@ Word for Mac is sandboxed: arbitrary file paths trigger a "Grant File Access" pr ## LibreOffice profile isolation -`soffice --headless` on macOS locks the default user profile (`~/Library/Application Support/LibreOffice/4`). Two concurrent invocations (or one stale soffice process) make subsequent spawns exit non-zero. We mint a temp profile per call via `-env:UserInstallation=file://` and `rm -rf` it after. Mirrors [tests/integration/libreoffice-roundtrip.test.ts](../../../tests/integration/libreoffice-roundtrip.test.ts)'s worker-pool pattern; **any new soffice spawn in this codebase must do the same**. +`soffice --headless` on macOS locks the default user profile (`~/Library/Application Support/LibreOffice/4`). Two concurrent invocations (or one stale soffice process) make subsequent spawns exit non-zero. We mint a temp profile per call via `-env:UserInstallation=` and `rm -rf` it after. Mirrors [tests/integration/libreoffice-roundtrip.test.ts](../../../tests/integration/libreoffice-roundtrip.test.ts)'s worker-pool pattern; **any new soffice spawn in this codebase must do the same**. + +**Windows, both verified on a runner:** spawn `soffice.exe`, never `soffice.com` (Bun.spawn fails with "Executable not found", and `where soffice` lists the `.com` first), and build the profile URL with `pathToFileURL` (soffice hangs on `file://C:\…`). The `windows-render` CI job guards both. ## PDF rasterization details diff --git a/src/core/render/engines/libreoffice.ts b/src/core/render/engines/libreoffice.ts index 85c18a8..deceadf 100644 --- a/src/core/render/engines/libreoffice.ts +++ b/src/core/render/engines/libreoffice.ts @@ -1,8 +1,13 @@ import { mkdtempSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; +import { pathToFileURL } from "node:url"; import { type RenderEngine, RenderEngineError } from "./types"; +/** Long enough for a large document's conversion on a cold start, matching the + * Word for Windows engine's cap. */ +const CONVERT_TIMEOUT_MS = 150_000; + /** Cross-platform LibreOffice engine. Drives `soffice --headless * --convert-to pdf` against the input docx. Used as the fallback when Word * isn't installed; also usable explicitly on a server / CI machine where @@ -35,7 +40,8 @@ export const libreofficeEngine: RenderEngine = { [ soffice, "--headless", - `-env:UserInstallation=file://${profile}`, + // A real file URL: `file://C:\…` isn't one, and soffice hangs on it. + `-env:UserInstallation=${pathToFileURL(profile).href}`, "--convert-to", "pdf", "--outdir", @@ -44,7 +50,23 @@ export const libreofficeEngine: RenderEngine = { ], { stdout: "pipe", stderr: "pipe" }, ); - const exit = await proc.exited; + let timedOut = false; + const killTimer = setTimeout(() => { + timedOut = true; + proc.kill(); + }, CONVERT_TIMEOUT_MS); + let exit: number; + try { + exit = await proc.exited; + } finally { + clearTimeout(killTimer); + } + if (timedOut) { + throw new RenderEngineError( + "RENDER_FAILED", + `soffice did not finish within ${CONVERT_TIMEOUT_MS / 1000}s`, + ); + } if (exit !== 0) { const stderr = await new Response(proc.stderr).text(); throw new RenderEngineError( @@ -64,7 +86,11 @@ export const libreofficeEngine: RenderEngine = { rmSync(sofficeOutput, { force: true }); } } finally { - rmSync(profile, { recursive: true, force: true }); + // A killed soffice can still hold its profile open on Windows; a leftover + // temp folder is harmless and must not replace the real error. + try { + rmSync(profile, { recursive: true, force: true }); + } catch {} } }, }; @@ -73,15 +99,19 @@ async function findSoffice(): Promise { // PATH search first (the brew install on macOS, apt on Linux, the LO // installer's program/ dir on Windows when added to PATH). Then check // the canonical install paths in case PATH wasn't set up. - const onPath = await whichEverywhere("soffice"); + // On Windows, ask for soffice.exe by name: `where soffice` lists soffice.com + // first, and Bun.spawn can't launch a .com ("Executable not found"). + const onPath = await whichEverywhere( + process.platform === "win32" ? "soffice.exe" : "soffice", + ); if (onPath) return onPath; const fallbacks = process.platform === "darwin" ? ["/Applications/LibreOffice.app/Contents/MacOS/soffice"] : process.platform === "win32" ? [ - "C:\\Program Files\\LibreOffice\\program\\soffice.com", - "C:\\Program Files (x86)\\LibreOffice\\program\\soffice.com", + "C:\\Program Files\\LibreOffice\\program\\soffice.exe", + "C:\\Program Files (x86)\\LibreOffice\\program\\soffice.exe", ] : ["/usr/bin/soffice", "/snap/bin/libreoffice"]; for (const candidate of fallbacks) { diff --git a/tests/cli/images.test.ts b/tests/cli/images.test.ts index 4c44190..e84b8cd 100644 --- a/tests/cli/images.test.ts +++ b/tests/cli/images.test.ts @@ -1,6 +1,7 @@ import { beforeAll, describe, expect, test } from "bun:test"; import { mkdirSync, readdirSync } from "node:fs"; import { join } from "node:path"; +import { pathToFileURL } from "node:url"; import { Document, XmlNode } from "@core"; import { Pkg } from "@core/ast/document/package"; import { runCli, tempWorkspace } from "./harness"; @@ -235,6 +236,26 @@ async function dataUri(path: string, mime: string): Promise { } describe("docx images add", () => { + // Slicing off `file://` left `/C:/x.png` on Windows and a literal `%20` everywhere. + test("embeds a PNG from a file:// URL with a space in its path", async () => { + const docPath = await newDoc("img-file-url"); + const spaced = join(tempWorkspace("img file url"), "my pic.png"); + await Bun.write(spaced, Bun.file(PNG_PATH)); + const result = await runCli( + "images", + "add", + docPath, + "--after", + "p0", + "--image", + pathToFileURL(spaced).href, + ); + expect(result.exitCode).toBe(0); + + const pkg = await Pkg.open(docPath); + expect(pkg.listParts()).toContain("word/media/image1.png"); + }); + test("embeds a PNG from a file path: media part, relationship, content-type", async () => { const docPath = await newDoc("img-png"); const result = await runCli( diff --git a/tests/cli/render.test.ts b/tests/cli/render.test.ts index b065d44..613cb62 100644 --- a/tests/cli/render.test.ts +++ b/tests/cli/render.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from "bun:test"; import { existsSync } from "node:fs"; import { join } from "node:path"; +import { libreofficeEngine } from "@core/render"; import { parsePagesSpec } from "../../src/cli/render/parse-pages"; import { runCli, tempWorkspace } from "./harness"; @@ -10,7 +11,7 @@ import { runCli, tempWorkspace } from "./harness"; * machines without LibreOffice (matches the LibreOffice round-trip * integration suite's pattern). Pure-logic tests (--pages parsing, * USAGE errors) always run. */ -const LIBREOFFICE_AVAILABLE = await detectLibreOffice(); +const LIBREOFFICE_AVAILABLE = await libreofficeEngine.available(); /** LibreOffice cold-start (the first soffice launch) routinely exceeds bun's 5s * default per-test timeout under load — e.g. right after the weak-agent-test @@ -19,17 +20,6 @@ const LIBREOFFICE_AVAILABLE = await detectLibreOffice(); * a failure. (They still run in ~2-3s when LibreOffice is warm.) */ const RENDER_TIMEOUT = 60_000; -async function detectLibreOffice(): Promise { - const probe = async (cmd: string[]): Promise => { - const proc = Bun.spawn(cmd, { stdout: "pipe", stderr: "pipe" }); - return (await proc.exited) === 0; - }; - return ( - (await probe(["/bin/sh", "-c", "command -v soffice"])) || - existsSync("/Applications/LibreOffice.app/Contents/MacOS/soffice") - ); -} - describe("docx render — argument parsing (no engine needed)", () => { test("missing FILE → USAGE error", async () => { const result = await runCli("render"); From aaa1d8b3b0bdb165856610c2fa2de0cbbe6cc3e1 Mon Sep 17 00:00:00 2001 From: Kirill Klimuk Date: Fri, 30 Oct 2026 15:04:56 -0700 Subject: [PATCH 2/2] docs(troubleshooting): explain garbled non-ASCII output in PowerShell docx can't fix this itself: on a Windows runner, PowerShell kept decoding as IBM437 even after the child process called SetConsoleOutputCP(65001), and the child still saw 65001, so it can't detect the mismatch either. Setting the encoding in PowerShell does fix it. Co-Authored-By: Claude Opus 5.5 --- skills/docx-cli/references/troubleshooting.md | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/skills/docx-cli/references/troubleshooting.md b/skills/docx-cli/references/troubleshooting.md index c4089bc..35e0ac4 100644 --- a/skills/docx-cli/references/troubleshooting.md +++ b/skills/docx-cli/references/troubleshooting.md @@ -89,6 +89,17 @@ HTML), which can transform literal prose. When you need text in **verbatim**, us the parser-free channel: `insert` and `create` take `--text-file PATH` (`-` = stdin). Every character lands as-is; each newline starts a new paragraph. +## Accents or CJK text look garbled in PowerShell output + +`docx` writes UTF-8, and PowerShell decodes captured output using its own console +encoding, which may be an older code page such as IBM437. The document is fine, +but what you see isn't, so don't write that text back. Set this once per session +before running `docx`, or use Git Bash: + +```powershell +[Console]::OutputEncoding = [Text.Encoding]::UTF8 +``` + ## I edited a file and lost some unusual formatting You shouldn't — docx-cli mutates the underlying XML **in place** and leaves