From bdbc6db1a3c2d19e8edd9c563cab4b6710e6b674 Mon Sep 17 00:00:00 2001 From: xsvm <2479667226@qq.com> Date: Wed, 28 Oct 2026 19:52:03 +0800 Subject: [PATCH 1/6] feat(upgrade): self-upgrade on Windows by staging, then swapping the binary in A running .exe can't be overwritten on Windows, but it can be renamed. Install into a staging folder beside the binary, and only after install.sh exits 0 rename the running binary to docx.exe.old and the staged one into place. A failed or interrupted upgrade leaves docx.exe untouched. --- README.md | 2 +- SECURITY.md | 2 +- skills/docx-cli/references/troubleshooting.md | 8 +++ src/cli/CLAUDE.md | 18 ++++-- src/cli/upgrade/index.ts | 55 +++++++++++++------ src/cli/upgrade/swap-in.ts | 15 +++++ tests/cli/upgrade.test.ts | 34 ++++++++++-- 7 files changed, 106 insertions(+), 28 deletions(-) create mode 100644 src/cli/upgrade/swap-in.ts diff --git a/README.md b/README.md index 6edda11..dc02424 100644 --- a/README.md +++ b/README.md @@ -127,7 +127,7 @@ bunx bun-docx read doc.docx # run without installing Every command works directly on the `.docx` file. Only `docx render` (page images, for checking layout) needs Word (macOS/Windows) or LibreOffice installed. -**Upgrading:** `docx upgrade` replaces a standalone binary in place, using the installer embedded in the binary at build time (never fetched), pinned to a release tag and SHA-256-verified. `docx upgrade --to v0.27.0` pins a version; `--dry-run` reports what would change. It won't touch an npm install (use your package manager), a renamed binary, or Windows, where a running `.exe` can't be replaced (download the new one by hand). +**Upgrading:** `docx upgrade` replaces a standalone binary in place, using the installer embedded in the binary at build time (never fetched), pinned to a release tag and SHA-256-verified. `docx upgrade --to v0.27.0` pins a version; `--dry-run` reports what would change. It won't touch an npm install (use your package manager) or a renamed binary. On Windows it needs Git Bash (for `sh`); a running `.exe` can be renamed but not overwritten, so the verified binary is swapped in and the old one is left as `docx.exe.old` until the next upgrade. ### Use it from your own code diff --git a/SECURITY.md b/SECURITY.md index 3ef841b..c245222 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -107,7 +107,7 @@ Nothing in docx-cli phones home after that. - `docx-windows-x64.exe` is not Authenticode-signed, so SmartScreen, WDAC, or AppLocker may block it. Allowlist it by hash. - Verify it with `Get-FileHash docx-windows-x64.exe -Algorithm SHA256` against `SHA256SUMS`. - `install.sh` also works under Git Bash/MSYS. -- `docx upgrade` can't replace a running `.exe`, so it refuses on Windows. Install the new version the same way as the first. +- `docx upgrade` works from Git Bash. A running `.exe` can't be overwritten, so the installer writes to a staging folder beside it, and only the verified binary is swapped in; the old one is left as `docx.exe.old` until the next upgrade. **macOS.** Release binaries are ad-hoc signed and signature-verified before checksumming, but not Developer ID-signed or notarized. Downloading with `curl` (as the installer does) doesn't set the quarantine flag. A browser download does, so Gatekeeper will ask. diff --git a/skills/docx-cli/references/troubleshooting.md b/skills/docx-cli/references/troubleshooting.md index c4089bc..2089d49 100644 --- a/skills/docx-cli/references/troubleshooting.md +++ b/skills/docx-cli/references/troubleshooting.md @@ -50,6 +50,14 @@ The updated release workflow ad-hoc signs and verifies macOS binaries before generating `SHA256SUMS`; existing release assets are unchanged. Ad-hoc signing does not provide Developer ID identity or Apple notarization. +## `docx upgrade` on Windows says `sh` is not on PATH + +Upgrade runs its embedded installer with `sh`, so on Windows run it from Git +Bash. Windows won't overwrite a running `.exe`, so it installs into a staging +folder and swaps the verified binary in, leaving the old one as `docx.exe.old` +until the next upgrade. A failed or interrupted upgrade leaves `docx.exe` as it +was. + ## `docx render` fails or hangs `render` is the **only** command that needs an external app. Everything else diff --git a/src/cli/CLAUDE.md b/src/cli/CLAUDE.md index fa3ad50..f643d7e 100644 --- a/src/cli/CLAUDE.md +++ b/src/cli/CLAUDE.md @@ -74,11 +74,19 @@ embedded text equals the file on disk. Install-method detection is `import.meta.path.startsWith("/$bunfs/")` (Bun's virtual FS for a compiled single-file executable; `~BUN` on Windows). Standalone → self-replace via install.sh's rename, which works over a running binary on POSIX. Package-manager install -→ print `bun add -g bun-docx` and change nothing. Windows → print the release URL, since -a running `.exe` cannot be replaced. A standalone binary the user RENAMED is refused -too: install.sh owns the platform→name rule and always writes `${PREFIX}/docx`, so -"upgrading" a `mydocx` would drop a SECOND file beside it (clobbering any unrelated -`docx` there) and leave the command the user actually runs stale — while we reported +→ print `bun add -g bun-docx` and change nothing. Windows locks a running `.exe` +against overwrite but not rename, so there install.sh gets `PREFIX=/.docx-upgrade` +and, only once it exits 0 (downloaded AND checksum-verified), `swapIn` +([upgrade/swap-in.ts](upgrade/swap-in.ts)) renames the running binary to `docx.exe.old` +(sweeping the previous one, which couldn't delete itself while running) and the staged +one into its place; the staging folder is removed either way. Nothing moves before the +install succeeds, so there is nothing to restore — don't "simplify" this into stepping +aside first: a Ctrl+C mid-download would strand the user without `docx`, and Ctrl+C +can't be trapped reliably on Windows (MSYS `sh` on piped stdio ignores it). `sh` must +be on PATH (Git Bash), checked up front. A standalone binary the user RENAMED is +refused too: install.sh owns the platform→name rule and always writes `${PREFIX}/docx` +(`docx.exe` on Windows), so "upgrading" a `mydocx` would drop a SECOND file beside it +(clobbering any unrelated `docx` there) and leave the command the user actually runs stale — while we reported success. The suite always runs from source, so only the package-manager branch is reachable in-process; the standalone paths are covered by the compiled-binary test. diff --git a/src/cli/upgrade/index.ts b/src/cli/upgrade/index.ts index 20f3af9..76ef67a 100644 --- a/src/cli/upgrade/index.ts +++ b/src/cli/upgrade/index.ts @@ -1,4 +1,5 @@ -import { basename, dirname } from "node:path"; +import { rm } from "node:fs/promises"; +import { basename, dirname, join } from "node:path"; // Embedded at BUILD time, never fetched — see the installer invariant in the root // CLAUDE.md for why that distinction is load-bearing. import installScript from "../../../skills/docx-cli/scripts/install.sh" with { @@ -15,6 +16,7 @@ import { writeStderr, writeStdout, } from "../respond"; +import { swapIn } from "./swap-in"; const HELP = `docx upgrade — replace the installed docx binary with a newer release @@ -29,7 +31,10 @@ Options: Only the STANDALONE binary upgrades itself, replacing it where it already lives (PREFIX is ignored). An npm/bun install is owned by the package manager, so -upgrade reports that and exits nonzero rather than fighting it. +upgrade reports that and exits nonzero rather than fighting it. On Windows, +where a running .exe can be renamed but not overwritten, the verified new +binary is swapped in and the old one is left as docx.exe.old; the next upgrade +deletes it. It runs the same installer published as a release asset — embedded in this binary, not downloaded — so the new binary is pinned to a release tag and its @@ -39,8 +44,8 @@ anything. Output: Success one-line ack (--verbose → {ok, operation, from, to, path}) --dry-run {operation, dryRun, from, to, path} - Not upgraded {code, error, hint} + nonzero exit (npm install, Windows, or a - failed install) — the binary is left untouched + Not upgraded {code, error, hint} + nonzero exit (npm install, a renamed + binary, or a failed install) — the binary is left untouched `; /** A release tag, and nothing that can steer the URL somewhere else. `--to` is @@ -109,29 +114,32 @@ export async function run(args: string[]): Promise { const binaryPath = process.execPath; const prefix = dirname(binaryPath); - if (process.platform === "win32") { - // POSIX can rename over a running binary's inode; Windows cannot. - return fail( - "UPGRADE_FAILED", - `Windows cannot replace ${binaryPath} while it is running`, - "Download the new binary and swap it while docx is not running: https://github.com/kklimuk/docx-cli/releases/latest", - ); - } + const windows = process.platform === "win32"; - // install.sh always writes `${PREFIX}/docx` — it owns the platform→name rule. So a + // install.sh writes `${PREFIX}/docx` (`docx.exe` on Windows) — it owns the name. So a // binary the user RENAMED cannot be self-replaced: the install would drop a second // file beside it (clobbering any unrelated `docx` already there) and leave the // command the user actually runs on the old version, while we reported success. // Refuse instead of reporting a replacement that did not happen. const binaryName = basename(binaryPath); - if (binaryName !== "docx") { + const installedName = windows ? "docx.exe" : "docx"; + if (binaryName !== installedName) { return fail( "UPGRADE_FAILED", - `the installer places the binary as "docx", but this one is named "${binaryName}" — upgrading would leave ${binaryPath} on ${VERSION} and write a separate ${prefix}/docx`, + `the installer places the binary as "${installedName}", but this one is named "${binaryName}" — upgrading would leave ${binaryPath} on ${VERSION} and write a separate ${prefix}/${installedName}`, `Replace it by hand: download docx- from https://github.com/kklimuk/docx-cli/releases/latest, verify it against SHA256SUMS, and move it over ${binaryPath}.`, ); } + // Always there on POSIX; on Windows only under Git Bash/MSYS (or with it on PATH). + if (!Bun.which("sh")) { + return fail( + "UPGRADE_FAILED", + "upgrade runs its installer with sh, which is not on PATH", + "Run it from Git Bash, or replace the binary by hand: https://github.com/kklimuk/docx-cli/releases/latest", + ); + } + if (parsed.values["dry-run"]) { await respond({ operation: "upgrade", @@ -143,6 +151,12 @@ export async function run(args: string[]): Promise { return EXIT.OK; } + // install.sh's final rename lands over a running binary on POSIX. Windows refuses + // that, so there it installs into a staging folder and the verified binary is + // swapped in after: nothing moves until the download has passed its checksum, + // so a failure or a Ctrl+C mid-download leaves the old binary where it was. + const installPrefix = windows ? join(prefix, ".docx-upgrade") : prefix; + // Pipe the embedded script to `sh -s` rather than staging a temp file: install.sh // reads no stdin and uses no $0, so this leaves nothing behind in a PATH directory, // and it hands the PREFIX-writability decision back to install.sh, whose error for @@ -153,7 +167,7 @@ export async function run(args: string[]): Promise { stderr: "pipe", env: { ...process.env, - PREFIX: prefix, + PREFIX: installPrefix, VERSION: target, REQUIRE_CHECKSUM: "1", }, @@ -167,7 +181,14 @@ export async function run(args: string[]): Promise { forward(proc.stderr, writeStderr), ]); - if ((await proc.exited) !== 0) { + const exitCode = await proc.exited; + if (windows) { + if (exitCode === 0) { + await swapIn(join(installPrefix, installedName), binaryPath); + } + await rm(installPrefix, { recursive: true, force: true }); + } + if (exitCode !== 0) { // install.sh has already narrated the specific cause on stderr; don't paper over // it with a blanket "use npm instead" that misdirects on a bad tag or a blip. // The one thing worth correcting: its unwritable-PREFIX hint says to re-run with diff --git a/src/cli/upgrade/swap-in.ts b/src/cli/upgrade/swap-in.ts new file mode 100644 index 0000000..fa6f3a7 --- /dev/null +++ b/src/cli/upgrade/swap-in.ts @@ -0,0 +1,15 @@ +import { rename, rm } from "node:fs/promises"; + +/** Windows locks a running `.exe` against overwrite and delete, but not against + * rename. So a verified binary staged beside it is swapped in with two renames: + * the running one steps aside to `.old` (it can't delete itself, so the + * next upgrade sweeps it) and the staged one takes its name. */ +export async function swapIn( + staged: string, + binaryPath: string, +): Promise { + const aside = `${binaryPath}.old`; + await rm(aside, { force: true }); + await rename(binaryPath, aside); + await rename(staged, binaryPath); +} diff --git a/tests/cli/upgrade.test.ts b/tests/cli/upgrade.test.ts index 61fd542..a065c97 100644 --- a/tests/cli/upgrade.test.ts +++ b/tests/cli/upgrade.test.ts @@ -1,8 +1,10 @@ import { describe, expect, test } from "bun:test"; +import { join } from "node:path"; import installScript from "../../skills/docx-cli/scripts/install.sh" with { type: "text", }; -import { runCli } from "./harness"; +import { swapIn } from "../../src/cli/upgrade/swap-in"; +import { runCli, tempWorkspace } from "./harness"; describe("upgrade", () => { // The whole point of the design: `upgrade` reuses the installer rather than @@ -82,6 +84,26 @@ describe("upgrade", () => { }); }); +// Windows locks a running .exe against overwrite but not rename, so upgrade stages +// the verified binary and swaps it in. Plain file moves, so this runs anywhere. +describe("upgrade (Windows swap-in)", () => { + // The previous upgrade's `.old` could not delete itself while it ran; by now it + // isn't running, so the next upgrade clears it instead of failing on it. + test("puts the staged binary in place and keeps the old one beside it", async () => { + const workspace = tempWorkspace("swap-in"); + const binary = `${workspace}/docx.exe`; + const staged = `${workspace}/.docx-upgrade/docx.exe`; + await Bun.write(binary, "current"); + await Bun.write(`${binary}.old`, "previous"); + await Bun.write(staged, "new"); + + await swapIn(staged, binary); + expect(await Bun.file(binary).text()).toBe("new"); + expect(await Bun.file(`${binary}.old`).text()).toBe("current"); + expect(await Bun.file(staged).exists()).toBe(false); + }); +}); + // isStandaloneBinary() keys on `/$bunfs/`, an UNDOCUMENTED Bun internal path that a // Bun upgrade could rename silently — and the failure is invisible from the source // tree, since every standalone `docx upgrade` would just degrade to the npm advisory @@ -91,6 +113,8 @@ describe("upgrade (compiled binary)", () => { test("a compiled binary detects itself as standalone", async () => { const dir = `${import.meta.dir}/../../node_modules/.cache/upgrade-smoke`; const binary = `${dir}/docx`; + // `bun build --compile` appends it on Windows, and install.sh names it the same. + const exe = process.platform === "win32" ? ".exe" : ""; const build = Bun.spawnSync([ "bun", "build", @@ -107,15 +131,17 @@ describe("upgrade (compiled binary)", () => { const stdout = result.stdout.toString(); expect(stdout).toContain('"dryRun":true'); // It reports the binary it would replace — process.execPath, normalized. - expect(stdout).toContain("upgrade-smoke/docx"); + expect(JSON.parse(stdout).path).toEndWith( + join("upgrade-smoke", `docx${exe}`), + ); // The npm advisory would mean detection broke. expect(stdout).not.toContain("bun add -g bun-docx"); // install.sh always writes `${PREFIX}/docx`, so a RENAMED binary can't be // self-replaced: the install would drop a second file beside it and leave the // command the user runs stale, while upgrade reported success on the old path. - const renamed = `${dir}/mydocx`; - expect(Bun.spawnSync(["cp", binary, renamed]).exitCode).toBe(0); + const renamed = `${dir}/mydocx${exe}`; + expect(Bun.spawnSync(["cp", `${binary}${exe}`, renamed]).exitCode).toBe(0); const refusal = Bun.spawnSync([renamed, "upgrade", "--dry-run"]); expect(refusal.exitCode).not.toBe(0); expect(refusal.stdout.toString()).toContain("mydocx"); From e98d08b137960b3bc1d1cfe47d6f76f279f1d10d Mon Sep 17 00:00:00 2001 From: xsvm <2479667226@qq.com> Date: Fri, 30 Oct 2026 02:23:39 +0800 Subject: [PATCH 2/6] fix(upgrade): restore the binary if the Windows swap fails, drop the staging PATH note - swapIn renames the running binary back when moving the staged one in fails (antivirus often holds a fresh .exe open), so the user is never left with only docx.exe.old; upgrade reports UPGRADE_FAILED as {code,error,hint} and always removes the staging folder. - On Windows install.sh's closing PATH note named the staging folder, which upgrade deletes right after; withoutStagingPathHint drops those lines from the forwarded stdout. install.sh is untouched. - The renamed-binary guard compares case-insensitively on Windows. - SECURITY.md no longer says Windows must upgrade by hand, and the docx upgrade entry describes the staging swap. Follows the triage review on #26. --- SECURITY.md | 4 +-- src/cli/CLAUDE.md | 9 ++++-- src/cli/upgrade/index.ts | 30 +++++++++++++++--- src/cli/upgrade/staging-path-hint.ts | 30 ++++++++++++++++++ src/cli/upgrade/swap-in.ts | 12 ++++++-- tests/cli/upgrade.test.ts | 46 ++++++++++++++++++++++++++++ 6 files changed, 120 insertions(+), 11 deletions(-) create mode 100644 src/cli/upgrade/staging-path-hint.ts diff --git a/SECURITY.md b/SECURITY.md index c245222..24c43f0 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -18,7 +18,7 @@ bun add -g bun-docx # or: npm install -g bun-docx ``` -On a macOS/Linux standalone-binary install, run **`docx upgrade`** — it replaces the binary in place, pinned and checksum-verified. (Or re-run the installer by hand from the [latest release](https://github.com/kklimuk/docx-cli/releases/latest).) On Windows, download and verify the replacement binary from Releases. The skill's `scripts/bootstrap.sh` also updates a stale binary, but only when an agent runs it: the skill tells the agent to run it when `docx` isn't installed, and nothing runs it automatically. +On a standalone-binary install, run **`docx upgrade`** (on Windows, from Git Bash) — it replaces the binary in place, pinned and checksum-verified. (Or re-run the installer by hand from the [latest release](https://github.com/kklimuk/docx-cli/releases/latest).) The skill's `scripts/bootstrap.sh` also updates a stale binary, but only when an agent runs it: the skill tells the agent to run it when `docx` isn't installed, and nothing runs it automatically. ## Install integrity @@ -56,7 +56,7 @@ docx-cli runs entirely **locally** against `.docx` files on disk. **No command s Network access happens in three places in docx-cli itself (plus your package manager, if you install through npm), and none of them uploads anything: - **`skills/docx-cli/scripts/install.sh`** (also published as the release asset) — fetches the prebuilt `docx` binary and its `SHA256SUMS` manifest from this repo's GitHub Releases over HTTPS. **`scripts/bootstrap.sh`** additionally reads the releases page to learn the latest tag. -- **`docx upgrade`** — downloads a release binary, and only when you run it. It carries `install.sh` **embedded at build time** (never fetched) and executes that copy with `REQUIRE_CHECKSUM=1`, so the new binary is pinned to a tag and SHA-256-verified before it replaces anything. It refuses to touch a package-manager-owned install. Note the installer it runs is the one embedded in the release you are _currently on_, not the target's — so an installer bug shipped in vN can't be fixed by upgrading from vN; fetch that release's `install.sh` by hand instead (see above). +- **`docx upgrade`** — downloads a release binary, and only when you run it. It carries `install.sh` **embedded at build time** (never fetched) and executes that copy with `REQUIRE_CHECKSUM=1`, so the new binary is pinned to a tag and SHA-256-verified before it replaces anything. On Windows the installer writes into a staging folder beside the binary and only the verified binary is swapped in; a failed or interrupted upgrade leaves `docx.exe` as it was. It refuses to touch a package-manager-owned install. Note the installer it runs is the one embedded in the release you are _currently on_, not the target's — so an installer bug shipped in vN can't be fixed by upgrading from vN; fetch that release's `install.sh` by hand instead (see above). - **`docx images add --image https://…`** — the one verb that fetches _content_ you asked for, when and only when you pass an `http(s)` source (a local path or `--markdown` with a local ref never does). It is guarded against SSRF: private/link-local addresses are blocked, redirects are walked manually and re-checked, capped at 5 hops. Your document content is never sent anywhere by any of them. diff --git a/src/cli/CLAUDE.md b/src/cli/CLAUDE.md index f643d7e..c30eda0 100644 --- a/src/cli/CLAUDE.md +++ b/src/cli/CLAUDE.md @@ -79,8 +79,13 @@ against overwrite but not rename, so there install.sh gets `PREFIX=/.docx-u and, only once it exits 0 (downloaded AND checksum-verified), `swapIn` ([upgrade/swap-in.ts](upgrade/swap-in.ts)) renames the running binary to `docx.exe.old` (sweeping the previous one, which couldn't delete itself while running) and the staged -one into its place; the staging folder is removed either way. Nothing moves before the -install succeeds, so there is nothing to restore — don't "simplify" this into stepping +one into its place; the staging folder is removed either way. If that second rename +fails (antivirus often holds a fresh `.exe` open), `swapIn` renames the running binary +back before rethrowing, and `upgrade` reports `UPGRADE_FAILED` as `{code,error,hint}`. +install.sh's closing PATH note names its PREFIX — the staging folder — so on Windows +`withoutStagingPathHint` ([upgrade/staging-path-hint.ts](upgrade/staging-path-hint.ts)) +drops those lines from the forwarded stdout; install.sh itself stays untouched. Nothing +moves before the install succeeds — don't "simplify" this into stepping aside first: a Ctrl+C mid-download would strand the user without `docx`, and Ctrl+C can't be trapped reliably on Windows (MSYS `sh` on piped stdio ignores it). `sh` must be on PATH (Git Bash), checked up front. A standalone binary the user RENAMED is diff --git a/src/cli/upgrade/index.ts b/src/cli/upgrade/index.ts index 76ef67a..ec18817 100644 --- a/src/cli/upgrade/index.ts +++ b/src/cli/upgrade/index.ts @@ -16,6 +16,7 @@ import { writeStderr, writeStdout, } from "../respond"; +import { withoutStagingPathHint } from "./staging-path-hint"; import { swapIn } from "./swap-in"; const HELP = `docx upgrade — replace the installed docx binary with a newer release @@ -123,7 +124,11 @@ export async function run(args: string[]): Promise { // Refuse instead of reporting a replacement that did not happen. const binaryName = basename(binaryPath); const installedName = windows ? "docx.exe" : "docx"; - if (binaryName !== installedName) { + // Windows filenames are case-insensitive: `Docx.exe` IS the installed binary. + const sameName = windows + ? binaryName.toLowerCase() === installedName + : binaryName === installedName; + if (!sameName) { return fail( "UPGRADE_FAILED", `the installer places the binary as "${installedName}", but this one is named "${binaryName}" — upgrading would leave ${binaryPath} on ${VERSION} and write a separate ${prefix}/${installedName}`, @@ -176,17 +181,32 @@ export async function run(args: string[]): Promise { // Forward as it arrives, not after it finishes: install.sh prints "→ Downloading …" // BEFORE the ~75 MB transfer precisely so the terminal isn't dead for a minute on a // slow link. Routing through respond.ts's sinks keeps the one stdout chokepoint. + const stdout = windows + ? withoutStagingPathHint(writeStdout, installPrefix) + : { write: writeStdout, flush: async () => {} }; await Promise.all([ - forward(proc.stdout, writeStdout), + forward(proc.stdout, stdout.write), forward(proc.stderr, writeStderr), ]); + await stdout.flush(); const exitCode = await proc.exited; if (windows) { - if (exitCode === 0) { - await swapIn(join(installPrefix, installedName), binaryPath); + try { + if (exitCode === 0) { + await swapIn(join(installPrefix, installedName), binaryPath); + } + } catch { + // swapIn put the running binary back; say so as {code,error,hint} + // rather than letting a stack trace escape to the catch-all. + return fail( + "UPGRADE_FAILED", + `could not swap the new binary into ${binaryPath} — it is unchanged`, + "Something (often antivirus scanning the new file) held it open. Wait a moment and run docx upgrade again.", + ); + } finally { + await rm(installPrefix, { recursive: true, force: true }); } - await rm(installPrefix, { recursive: true, force: true }); } if (exitCode !== 0) { // install.sh has already narrated the specific cause on stderr; don't paper over diff --git a/src/cli/upgrade/staging-path-hint.ts b/src/cli/upgrade/staging-path-hint.ts new file mode 100644 index 0000000..e13f3c6 --- /dev/null +++ b/src/cli/upgrade/staging-path-hint.ts @@ -0,0 +1,30 @@ +/** On Windows, install.sh runs with PREFIX set to the staging folder, so its + * closing "Note: is not on your PATH … export PATH=…" fires on every + * upgrade and tells the user to put a folder on PATH that is deleted a moment + * later. install.sh is the one shared installer, so the hint is dropped here, + * from the forwarded stdout, rather than patched there. Output is held only + * until its line ends; every other line passes through unchanged. */ +export function withoutStagingPathHint( + write: (text: string) => Promise, + stagingPrefix: string, +): { write: (text: string) => Promise; flush: () => Promise } { + let pending = ""; + const isHint = (line: string): boolean => + line.includes(`${stagingPrefix} is not on your PATH`) || + line.includes(`export PATH="${stagingPrefix}:`) || + line.includes("Add this to your shell profile"); + return { + async write(text) { + pending += text; + const lines = pending.split("\n"); + pending = lines.pop() ?? ""; + const kept = lines.filter((line) => !isHint(line)); + if (kept.length > 0) await write(`${kept.join("\n")}\n`); + }, + async flush() { + const rest = pending; + pending = ""; + if (rest && !isHint(rest)) await write(rest); + }, + }; +} diff --git a/src/cli/upgrade/swap-in.ts b/src/cli/upgrade/swap-in.ts index fa6f3a7..cfbe232 100644 --- a/src/cli/upgrade/swap-in.ts +++ b/src/cli/upgrade/swap-in.ts @@ -3,7 +3,10 @@ import { rename, rm } from "node:fs/promises"; /** Windows locks a running `.exe` against overwrite and delete, but not against * rename. So a verified binary staged beside it is swapped in with two renames: * the running one steps aside to `.old` (it can't delete itself, so the - * next upgrade sweeps it) and the staged one takes its name. */ + * next upgrade sweeps it) and the staged one takes its name. If that second + * rename fails (antivirus often holds a freshly written `.exe` open for a + * moment), the running binary is put back before the error propagates, so the + * user is never left with only `.old`. */ export async function swapIn( staged: string, binaryPath: string, @@ -11,5 +14,10 @@ export async function swapIn( const aside = `${binaryPath}.old`; await rm(aside, { force: true }); await rename(binaryPath, aside); - await rename(staged, binaryPath); + try { + await rename(staged, binaryPath); + } catch (error) { + await rename(aside, binaryPath); + throw error; + } } diff --git a/tests/cli/upgrade.test.ts b/tests/cli/upgrade.test.ts index a065c97..f6b5ee9 100644 --- a/tests/cli/upgrade.test.ts +++ b/tests/cli/upgrade.test.ts @@ -3,6 +3,7 @@ import { join } from "node:path"; import installScript from "../../skills/docx-cli/scripts/install.sh" with { type: "text", }; +import { withoutStagingPathHint } from "../../src/cli/upgrade/staging-path-hint"; import { swapIn } from "../../src/cli/upgrade/swap-in"; import { runCli, tempWorkspace } from "./harness"; @@ -102,6 +103,51 @@ describe("upgrade (Windows swap-in)", () => { expect(await Bun.file(`${binary}.old`).text()).toBe("current"); expect(await Bun.file(staged).exists()).toBe(false); }); + + // Antivirus can hold a freshly written .exe open for a moment, so the second + // rename can fail after the first moved the running binary aside. The user + // must not be left with only docx.exe.old. + test("puts the running binary back when the staged one can't be moved in", async () => { + const workspace = tempWorkspace("swap-in-fail"); + const binary = `${workspace}/docx.exe`; + await Bun.write(binary, "current"); + + await expect( + swapIn(`${workspace}/.docx-upgrade/docx.exe`, binary), + ).rejects.toThrow(); + expect(await Bun.file(binary).text()).toBe("current"); + }); + + // install.sh's PATH note names its PREFIX, which on Windows is the staging + // folder upgrade deletes right after — so the note is dropped, nothing else. + test("drops install.sh's PATH note about the staging folder, keeps the rest", async () => { + const staging = "C:\\bin\\.docx-upgrade"; + const lines = [ + "→ Downloading docx-windows-x64.exe from https://example/v1", + "✓ Verified SHA-256 (docx-windows-x64.exe)", + `✓ Installed: ${staging}/docx.exe`, + "", + ` Note: ${staging} is not on your PATH.`, + " Add this to your shell profile (~/.zshrc, ~/.bashrc, etc):", + ` export PATH="${staging}:$PATH"`, + "", + "docx 0.29.0", + ]; + let out = ""; + const filter = withoutStagingPathHint(async (text) => { + out += text; + }, staging); + // Chunk boundaries fall mid-line, as they do on a real pipe. + const text = `${lines.join("\n")}\n`; + for (let start = 0; start < text.length; start += 7) { + await filter.write(text.slice(start, start + 7)); + } + await filter.flush(); + expect(out).not.toContain("not on your PATH"); + expect(out).not.toContain("export PATH"); + expect(out).toContain("✓ Verified SHA-256"); + expect(out).toContain("docx 0.29.0\n"); + }); }); // isStandaloneBinary() keys on `/$bunfs/`, an UNDOCUMENTED Bun internal path that a From 541ca5c15a3d0ac4b6fec3d5ea2c92f9d779d731 Mon Sep 17 00:00:00 2001 From: xsvm <2479667226@qq.com> Date: Fri, 30 Oct 2026 02:37:58 +0800 Subject: [PATCH 3/6] fix(upgrade): pass install.sh a forward-slash PREFIX on Windows A native C:\...\.docx-upgrade PREFIX put backslashes in the downloaded file's path. GNU sha256sum (Git Bash) escapes such a name and prefixes its output line with a backslash, which install.sh's awk '{print $1}' keeps, so every Windows upgrade failed with "SHA-256 mismatch" (actual hash shown with a leading backslash). Found by running docx upgrade on Windows 11 / Git Bash; with a C:/... PREFIX it verifies, swaps in the release binary, and removes the staging folder. install.sh is unchanged. --- src/cli/upgrade/index.ts | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/cli/upgrade/index.ts b/src/cli/upgrade/index.ts index ec18817..31ef42c 100644 --- a/src/cli/upgrade/index.ts +++ b/src/cli/upgrade/index.ts @@ -1,5 +1,5 @@ import { rm } from "node:fs/promises"; -import { basename, dirname, join } from "node:path"; +import { basename, dirname, join, sep } from "node:path"; // Embedded at BUILD time, never fetched — see the installer invariant in the root // CLAUDE.md for why that distinction is load-bearing. import installScript from "../../../skills/docx-cli/scripts/install.sh" with { @@ -161,6 +161,13 @@ export async function run(args: string[]): Promise { // swapped in after: nothing moves until the download has passed its checksum, // so a failure or a Ctrl+C mid-download leaves the old binary where it was. const installPrefix = windows ? join(prefix, ".docx-upgrade") : prefix; + // What install.sh sees as PREFIX. Git Bash accepts `C:/...`, and a backslash in + // the path must not reach it: GNU sha256sum escapes such a file name and prefixes + // its output line with a backslash, which install.sh's `awk '{print $1}'` keeps, + // so every Windows upgrade failed its checksum ("actual: \48a6…"). + const shellPrefix = windows + ? installPrefix.split(sep).join("/") + : installPrefix; // Pipe the embedded script to `sh -s` rather than staging a temp file: install.sh // reads no stdin and uses no $0, so this leaves nothing behind in a PATH directory, @@ -172,7 +179,7 @@ export async function run(args: string[]): Promise { stderr: "pipe", env: { ...process.env, - PREFIX: installPrefix, + PREFIX: shellPrefix, VERSION: target, REQUIRE_CHECKSUM: "1", }, @@ -182,7 +189,7 @@ export async function run(args: string[]): Promise { // BEFORE the ~75 MB transfer precisely so the terminal isn't dead for a minute on a // slow link. Routing through respond.ts's sinks keeps the one stdout chokepoint. const stdout = windows - ? withoutStagingPathHint(writeStdout, installPrefix) + ? withoutStagingPathHint(writeStdout, shellPrefix) : { write: writeStdout, flush: async () => {} }; await Promise.all([ forward(proc.stdout, stdout.write), From cc0f4ed0b13d897e235ebb9ceb5951afaa4f2440 Mon Sep 17 00:00:00 2001 From: Kirill Klimuk Date: Fri, 30 Oct 2026 13:12:00 -0700 Subject: [PATCH 4/6] fix(install): hash from stdin, pin scripts to LF, and test on Windows GNU sha256sum escapes a file name holding a backslash by prefixing its output line with `\`, which landed in the digest and failed the comparison. Every checksum tool now reads stdin, so no file name reaches its output. v0.28.0's Windows binary embeds install.sh with CRLF line endings, because the Windows runner checks text out that way and the checked-out bytes are what the binary ships. .gitattributes pins *.sh to LF. build-binary now runs on windows-latest, and every platform does a real `docx upgrade` to a pinned release. The Windows unit-test pass is informational until its known failures are fixed. Co-Authored-By: Claude Opus 5.5 --- .gitattributes | 3 +++ .github/workflows/ci.yml | 30 ++++++++++++++++++++- CLAUDE.md | 2 +- CONTRIBUTING.md | 2 +- install.sh | 18 +++++++------ skills/docx-cli/scripts/install.sh | 18 +++++++------ tests/cli/installer.test.ts | 42 ++++++++++++++++++++++++++++++ 7 files changed, 96 insertions(+), 19 deletions(-) create mode 100644 .gitattributes diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 0000000..8377a21 --- /dev/null +++ b/.gitattributes @@ -0,0 +1,3 @@ +# Git on Windows runners checks text out as CRLF, and `docx upgrade` embeds install.sh +# at build time and pipes it to `sh -s`, so the checked-out bytes are what ships. +*.sh text eol=lf diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cdcf4a8..92408fc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -92,7 +92,7 @@ jobs: strategy: fail-fast: false matrix: - os: [ubuntu-latest, macos-15-intel, macos-latest] + os: [ubuntu-latest, macos-15-intel, macos-latest, windows-latest] steps: - uses: actions/checkout@v6 - uses: oven-sh/setup-bun@v2 @@ -107,8 +107,36 @@ jobs: if: runner.os == 'macOS' run: sh scripts/sign-macos-binary.sh ./dist/docx - name: Verify binary runs + # Git Bash on Windows, the shell `docx upgrade` needs there; bash also stops at + # the first failing line, where the default PowerShell would carry on. + shell: bash run: | ./dist/docx --version ./dist/docx --help > /dev/null ./dist/docx read tests/fixtures/markdown-import.docx > /dev/null ./dist/docx validate tests/fixtures/markdown-import.docx + - name: Verify the installer on a Windows checkout + if: runner.os == 'Windows' + shell: bash + run: bun test tests/cli/installer.test.ts + - name: Unit tests on Windows (informational) + # Known Windows-only failures remain, so this reports without failing the job. + if: runner.os == 'Windows' + continue-on-error: true + timeout-minutes: 15 + shell: bash + run: bun run test:unit + - name: Upgrade the compiled binary against a real release + # Runs last: it replaces dist/docx. The version is pinned so that a new release + # can't change what this checks, and only the release's assets are downloaded. + shell: bash + run: | + ./dist/docx upgrade --dry-run + ./dist/docx upgrade --to v0.28.0 + ./dist/docx --version | grep -F 0.28.0 + - name: Verify the Windows swap + if: runner.os == 'Windows' + shell: bash + run: | + test -f dist/docx.exe.old + test ! -e dist/.docx-upgrade diff --git a/CLAUDE.md b/CLAUDE.md index 43981f4..3695361 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -47,7 +47,7 @@ These invariants are NOT SUGGESTIONS. These MUST be followed. - **Track-changes is doc-level** — see [src/cli/track-changes](src/cli/track-changes/CLAUDE.md). - **`docx render` is the only command that needs an external APPLICATION.** Every other verb works purely against the .docx zip + XML; `render` shells out to Word (macOS/Windows) or LibreOffice (cross-platform) to produce a PDF, then rasterizes in-process via the bundled `@hyzyla/pdfium` WASM package (no system tools needed for the rasterizer). The lens lives at [`@core/render`](src/core/render/CLAUDE.md) (`renderDocxPages` is the entry point); [`src/cli/render`](src/cli/render/CLAUDE.md) is a thin arg-parse shell. Agents that consume PNGs (and you, when verifying a fixture) use this command; the rest of the CLI never invokes it. - **Raw OOXML enters only through the gate pipeline.** `docx raw insert`/`replace` is the one surface where the user hands us arbitrary XML, and every fragment runs `Raw.prepareFragment`'s gates before it may touch the tree (see [src/core/raw](src/core/raw/CLAUDE.md)): well-formedness via `XMLValidator` (our own parser NEVER throws — it silently mangles bad XML, so it can't be the gate), addressable roots only (`w:p`/`w:tbl`, or one `w:sectPr` replacing an `sN` — anything else has no locator and would vanish from `read`, breaking the write-read loop), inter-element whitespace strip (pretty-printed fragments corrupt Word), ECMA child-order checks against the same order tables the emitters use, namespace auto-declare/reject, reference integrity (dangling rIds/note ids reject; drawing/bookmark id collisions re-mint), and a final full-document baseline-diff schema validation (bundled ECMA-376 transitional XSDs via libxml2-wasm; a mutation may not ADD schema errors; `--no-validate` skips only this gate; `docx validate` exposes the same engine standalone). **Nothing is written when any gate fails** — all mutation is in-memory until the post-gate save. Inserted roots are stamped `dcx:raw="1"` (the `dcx` prefix rides `mc:Ignorable`, so Word and the validator skip it) and surface as a `raw` token on the `docx:p`/`docx:table` note + `rawXml` in the AST. Raw changes are never tracked (`--track` rejects); under doc-level tracking they apply untracked + audit comment, per the hyperlinks/images precedent. **Relationships are raw-addressable too** (`raw get --at rels|rIdN`, insert a `` fragment routed by shape, Id-preserving replace) with their own proportional gate set (Id/Type/Target/TargetMode is the whole rels schema; internal Targets must resolve to an existing part; colliding/renamed Ids reject). **Headers/footers are raw-GET-readable** by their `hdrN`/`ftrN` id — a read-only serialize of the ``/`` part (a read needs no gate); the id is a first-class locator (`headers`/`footers list` reports it, `read` leads its `docx:header`/`docx:footer` hint with it, `headers`/`footers set/clear --at ftrN` author by it), and authoring stays on the modeled `docx headers`/`footers` verbs. **OPC parts are addressable too, but NOT as locators** — locators address document content; parts are the container, so they get their own noun: `raw part list|get|add|replace|edit FILE --name NAME` (`add`: XML gated, binary verbatim, content type required or extension-Default; whole-part replace/edit with root tag preserved — unmodeled parts via `Pkg`, styles/numbering/settings/marginals via a VIEW reparse so the swap survives the save, and notes/comments/document.xml/rels rejecting toward their own surfaces because their ids pair with body markers). **`raw edit` / `raw part edit` is the patch loop as one gated call**: literal `--find`/`--with` over the target's exact serialization, all occurrences, zero matches errors, result runs the full replace gate set. Part names and native ids are the ONLY sub-document handles — no positional addressing inside a part, ever. The full embedded-object loop is `raw part add` → insert `` → insert the body fragment referencing the rId. -- **Installers fetch release ASSETS, never remote code.** Every install path resolves a release **tag** or the `latest` release pointer — never a branch — then downloads that release's prebuilt binary plus its published `SHA256SUMS` and verifies the digest before the binary lands. (`install.sh` is itself covered by `SHA256SUMS`, but nothing self-verifies it: checking the installer is the operator's step, which is why the README fetches it to a file instead of piping it.) None of them downloads a script and runs it — that was a Snyk CRITICAL on the skills.sh listing, because the skill's bootstrap used to `sh` a fetched `install.sh`, and neither a scanner nor a reviewer can audit a file the script goes off and gets. There is exactly ONE implementation of the install logic, `skills/docx-cli/scripts/install.sh`, delivered THREE ways, chosen by who is asking: as a release asset (the operator, by hand), inside the skill folder where `bootstrap.sh` runs it as a local sibling (an agent at session start), and EMBEDDED into the binary at build time via `import … with { type: "text" }` for `docx upgrade` (an installed human). Because of the third route install.sh must stay POSIX `sh` with no sibling or `$0` dependencies — `upgrade` pipes it to `sh -s`. **Don't collapse these into one route**: `bootstrap.sh` can't call `docx upgrade` (it must work when `docx` is absent, must not downgrade a local build, and must not trust the possibly-stale installer inside whatever binary is present), and `upgrade` can't fetch the script (that's the forbidden pattern). Note `upgrade` runs the installer from the release you're ON, not the one you're going TO, so an installer bug in vN can't be fixed by upgrading from vN. The repo-root `install.sh` is a byte-identical DEPRECATION SHIM, not a second implementation — bootstraps already published to npm/skills.sh (≤ v0.22.0) fetch `raw.githubusercontent.com/…//install.sh`, so a tag whose tree lacks that path breaks every deployed skill's session-start check. `tests/cli/installer.test.ts` enforces the copy, the platform-table↔release-matrix agreement, and the absence of fetch-then-execute; delete the shim and its test once those bootstraps age out. It lives under `skills/` precisely so its sibling `bootstrap.sh` can delegate to it as a LOCAL file — executing a shipped sibling is not the flagged pattern; fetching a script and running it is. Keep the split by question: **install.sh answers "put `docx` here"** (detect platform, download, verify, place) and **bootstrap.sh answers "keep `docx` current"** (compare versions, update only if behind, never downgrade a local build, degrade to exit 0 offline, confirm the PATH `docx` is the one just installed). Don't reintroduce install logic into bootstrap. Posture differences are PARAMETERS, not forks: `bootstrap.sh` passes `REQUIRE_CHECKSUM=1` so an agent gets a refusal where a human running install.sh by hand gets a warn-and-install. The admin controls are parameters too: `DOCX_CLI_VERSION` (validated against the same release-tag shape as `upgrade --to`, since it lands in a URL) pins what bootstrap installs and, like `DOCX_CLI_NO_UPDATE=1`, stops it from moving a binary that's already present — SECURITY.md's "Deploying in an organization" documents them. Order the checks cheapest-first: platform, checksum policy, `PREFIX` writability, the few-hundred-byte manifest, and only then the ~100 MB binary. +- **Installers fetch release ASSETS, never remote code.** Every install path resolves a release **tag** or the `latest` release pointer — never a branch — then downloads that release's prebuilt binary plus its published `SHA256SUMS` and verifies the digest before the binary lands. (`install.sh` is itself covered by `SHA256SUMS`, but nothing self-verifies it: checking the installer is the operator's step, which is why the README fetches it to a file instead of piping it.) None of them downloads a script and runs it — that was a Snyk CRITICAL on the skills.sh listing, because the skill's bootstrap used to `sh` a fetched `install.sh`, and neither a scanner nor a reviewer can audit a file the script goes off and gets. There is exactly ONE implementation of the install logic, `skills/docx-cli/scripts/install.sh`, delivered THREE ways, chosen by who is asking: as a release asset (the operator, by hand), inside the skill folder where `bootstrap.sh` runs it as a local sibling (an agent at session start), and EMBEDDED into the binary at build time via `import … with { type: "text" }` for `docx upgrade` (an installed human). Because of the third route install.sh must stay POSIX `sh` with no sibling or `$0` dependencies — `upgrade` pipes it to `sh -s` — and must stay LF even on a Windows checkout (`.gitattributes` pins `*.sh`; the Windows job in `ci.yml` runs the guard), since the checked-out bytes are what the binary embeds. Its checksum stays in shell for the same reason: install.sh runs where no `docx` or Bun exists yet. **Don't collapse these into one route**: `bootstrap.sh` can't call `docx upgrade` (it must work when `docx` is absent, must not downgrade a local build, and must not trust the possibly-stale installer inside whatever binary is present), and `upgrade` can't fetch the script (that's the forbidden pattern). Note `upgrade` runs the installer from the release you're ON, not the one you're going TO, so an installer bug in vN can't be fixed by upgrading from vN. The repo-root `install.sh` is a byte-identical DEPRECATION SHIM, not a second implementation — bootstraps already published to npm/skills.sh (≤ v0.22.0) fetch `raw.githubusercontent.com/…//install.sh`, so a tag whose tree lacks that path breaks every deployed skill's session-start check. `tests/cli/installer.test.ts` enforces the copy, the platform-table↔release-matrix agreement, and the absence of fetch-then-execute; delete the shim and its test once those bootstraps age out. It lives under `skills/` precisely so its sibling `bootstrap.sh` can delegate to it as a LOCAL file — executing a shipped sibling is not the flagged pattern; fetching a script and running it is. Keep the split by question: **install.sh answers "put `docx` here"** (detect platform, download, verify, place) and **bootstrap.sh answers "keep `docx` current"** (compare versions, update only if behind, never downgrade a local build, degrade to exit 0 offline, confirm the PATH `docx` is the one just installed). Don't reintroduce install logic into bootstrap. Posture differences are PARAMETERS, not forks: `bootstrap.sh` passes `REQUIRE_CHECKSUM=1` so an agent gets a refusal where a human running install.sh by hand gets a warn-and-install. The admin controls are parameters too: `DOCX_CLI_VERSION` (validated against the same release-tag shape as `upgrade --to`, since it lands in a URL) pins what bootstrap installs and, like `DOCX_CLI_NO_UPDATE=1`, stops it from moving a binary that's already present — SECURITY.md's "Deploying in an organization" documents them. Order the checks cheapest-first: platform, checksum policy, `PREFIX` writability, the few-hundred-byte manifest, and only then the ~100 MB binary. - **No undo, no journal.** Mutating commands overwrite `FILE` in place; git is the history. `-o/--output PATH` writes a parallel file; `--dry-run` previews (wins over `--output`). Track changes is the closest thing to a journal, but not fully reliable because some operations are irreversible in OOXML. Our system denotes these with comments. ## Commands diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b5f2e2a..cc1c2ad 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -119,7 +119,7 @@ GitHub Actions (`.github/workflows/ci.yml`) runs these jobs on push to `main` an | `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 | | `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 | +| `build-binary` | Builds on Linux, native Intel/ARM macOS, and Windows; signs/verifies macOS binaries; runs version/help, read, and schema-validation smoke checks, then a real `docx upgrade` to a pinned release; on Windows, under Git Bash, also runs the installer tests and an informational unit-test pass | `.github/workflows/release.yml` triggers on `v*` tags, matrix-builds the five binaries, and uploads them to a GitHub Release via [`softprops/action-gh-release`](https://github.com/softprops/action-gh-release). diff --git a/install.sh b/install.sh index 349dadb..8e86855 100755 --- a/install.sh +++ b/install.sh @@ -82,18 +82,20 @@ fi # ─── Pick a checksum tool ─── # sha256_of() prints ONLY the lowercase hex digest, so callers need not know each tool's -# output shape: the *sum tools put the digest FIRST (" "), while `openssl -# dgst` puts it LAST ("SHA256(file)= " on LibreSSL, "SHA2-256(file)= " on -# OpenSSL 3.x). openssl is the third fallback because it is a native binary: on macOS, -# /usr/bin/shasum is a PERL script, and Apple has deprecated the bundled scripting -# runtimes, so shasum disappears whenever Perl does. +# output shape: the *sum tools put the digest FIRST (" -"), while `openssl dgst` +# puts it LAST ("SHA2-256(stdin)= " on OpenSSL 3.x, a bare "" on LibreSSL). +# Each tool reads STDIN so no file name reaches its output: GNU sha256sum escapes a name +# holding a backslash (a native Windows PREFIX) by prefixing the line with `\`, which +# would land in the digest and fail every comparison. openssl is the third fallback +# because it is a native binary: on macOS, /usr/bin/shasum is a PERL script, and Apple +# has deprecated the bundled scripting runtimes, so shasum disappears whenever Perl does. verify=1 if command -v sha256sum >/dev/null 2>&1; then - sha256_of() { sha256sum "$1" | awk '{print $1}'; } + sha256_of() { sha256sum < "$1" | awk '{print $1}'; } elif command -v shasum >/dev/null 2>&1; then - sha256_of() { shasum -a 256 "$1" | awk '{print $1}'; } + sha256_of() { shasum -a 256 < "$1" | awk '{print $1}'; } elif command -v openssl >/dev/null 2>&1; then - sha256_of() { openssl dgst -sha256 "$1" | awk '{print $NF}'; } + sha256_of() { openssl dgst -sha256 < "$1" | awk '{print $NF}'; } else verify=0 fi diff --git a/skills/docx-cli/scripts/install.sh b/skills/docx-cli/scripts/install.sh index 349dadb..8e86855 100755 --- a/skills/docx-cli/scripts/install.sh +++ b/skills/docx-cli/scripts/install.sh @@ -82,18 +82,20 @@ fi # ─── Pick a checksum tool ─── # sha256_of() prints ONLY the lowercase hex digest, so callers need not know each tool's -# output shape: the *sum tools put the digest FIRST (" "), while `openssl -# dgst` puts it LAST ("SHA256(file)= " on LibreSSL, "SHA2-256(file)= " on -# OpenSSL 3.x). openssl is the third fallback because it is a native binary: on macOS, -# /usr/bin/shasum is a PERL script, and Apple has deprecated the bundled scripting -# runtimes, so shasum disappears whenever Perl does. +# output shape: the *sum tools put the digest FIRST (" -"), while `openssl dgst` +# puts it LAST ("SHA2-256(stdin)= " on OpenSSL 3.x, a bare "" on LibreSSL). +# Each tool reads STDIN so no file name reaches its output: GNU sha256sum escapes a name +# holding a backslash (a native Windows PREFIX) by prefixing the line with `\`, which +# would land in the digest and fail every comparison. openssl is the third fallback +# because it is a native binary: on macOS, /usr/bin/shasum is a PERL script, and Apple +# has deprecated the bundled scripting runtimes, so shasum disappears whenever Perl does. verify=1 if command -v sha256sum >/dev/null 2>&1; then - sha256_of() { sha256sum "$1" | awk '{print $1}'; } + sha256_of() { sha256sum < "$1" | awk '{print $1}'; } elif command -v shasum >/dev/null 2>&1; then - sha256_of() { shasum -a 256 "$1" | awk '{print $1}'; } + sha256_of() { shasum -a 256 < "$1" | awk '{print $1}'; } elif command -v openssl >/dev/null 2>&1; then - sha256_of() { openssl dgst -sha256 "$1" | awk '{print $NF}'; } + sha256_of() { openssl dgst -sha256 < "$1" | awk '{print $NF}'; } else verify=0 fi diff --git a/tests/cli/installer.test.ts b/tests/cli/installer.test.ts index 04f2bc4..892163a 100644 --- a/tests/cli/installer.test.ts +++ b/tests/cli/installer.test.ts @@ -1,4 +1,5 @@ import { describe, expect, test } from "bun:test"; +import { tempWorkspace } from "./harness"; const SKILL_INSTALLER = "skills/docx-cli/scripts/install.sh"; const ROOT_INSTALLER = "install.sh"; @@ -63,4 +64,45 @@ describe("installer drift guards", () => { expect(source).not.toMatch(/(sh|bash)\s+-c\s+"\$\(/); } }); + + // `docx upgrade` embeds the checked-out bytes and pipes them to `sh -s`. Only a + // Windows checkout can produce CRLF here, so the Windows CI job is what guards this + // (v0.28.0's Windows binary shipped a CRLF copy). + test("shell scripts check out with LF line endings", async () => { + for (const path of [SKILL_INSTALLER, ROOT_INSTALLER, BOOTSTRAP]) { + expect(await Bun.file(path).text()).not.toContain("\r"); + } + }); +}); + +describe("installer sha256_of", () => { + // GNU sha256sum escapes a file name holding a backslash by prefixing its output line + // with `\`, which used to land in the digest and fail every Windows checksum. Each + // fallback is run on its own, since a machine normally only reaches the first. + test("every checksum tool prints the bare digest for a backslash path", async () => { + const installer = await Bun.file(SKILL_INSTALLER).text(); + const definitions = [ + ...installer.matchAll(/sha256_of\(\) \{ (\S+) .*\}/g), + ].map((match) => ({ definition: match[0], tool: match[1] as string })); + expect(definitions.map(({ tool }) => tool)).toEqual([ + "sha256sum", + "shasum", + "openssl", + ]); + + const path = `${tempWorkspace("sha256-of")}/back\\slash`; + await Bun.write(path, "abc"); + const expected = new Bun.CryptoHasher("sha256").update("abc").digest("hex"); + + for (const { definition, tool } of definitions) { + if (!Bun.which(tool)) continue; + const proc = Bun.spawn( + ["sh", "-c", `${definition}\nsha256_of "$1"`, "sh", path], + { stdout: "pipe" }, + ); + const stdout = await new Response(proc.stdout).text(); + expect(await proc.exited).toBe(0); + expect(stdout.trim()).toBe(expected); + } + }); }); From 875523fa340fde696345a5239e2680843bf6547d Mon Sep 17 00:00:00 2001 From: Kirill Klimuk Date: Fri, 30 Oct 2026 13:12:34 -0700 Subject: [PATCH 5/6] fix(upgrade): report the real outcome when the Windows swap or cleanup fails A throw from the staging cleanup in `finally` replaced the result, so even a successful swap could exit nonzero. And when the rollback rename failed too, the error still said the binary was unchanged; it now names docx.exe.old. Also notes that Windows binaries up to v0.28.0 need one last manual swap. Co-Authored-By: Claude Opus 5.5 --- skills/docx-cli/references/troubleshooting.md | 3 +++ src/cli/upgrade/index.ts | 22 +++++++++++++------ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/skills/docx-cli/references/troubleshooting.md b/skills/docx-cli/references/troubleshooting.md index 2089d49..d228f56 100644 --- a/skills/docx-cli/references/troubleshooting.md +++ b/skills/docx-cli/references/troubleshooting.md @@ -58,6 +58,9 @@ folder and swaps the verified binary in, leaving the old one as `docx.exe.old` until the next upgrade. A failed or interrupted upgrade leaves `docx.exe` as it was. +Windows binaries up to v0.28.0 refuse to upgrade themselves, so replace those by +hand once, or reinstall with `install.sh` from Git Bash. + ## `docx render` fails or hangs `render` is the **only** command that needs an external app. Everything else diff --git a/src/cli/upgrade/index.ts b/src/cli/upgrade/index.ts index 31ef42c..7cf5411 100644 --- a/src/cli/upgrade/index.ts +++ b/src/cli/upgrade/index.ts @@ -161,10 +161,8 @@ export async function run(args: string[]): Promise { // swapped in after: nothing moves until the download has passed its checksum, // so a failure or a Ctrl+C mid-download leaves the old binary where it was. const installPrefix = windows ? join(prefix, ".docx-upgrade") : prefix; - // What install.sh sees as PREFIX. Git Bash accepts `C:/...`, and a backslash in - // the path must not reach it: GNU sha256sum escapes such a file name and prefixes - // its output line with a backslash, which install.sh's `awk '{print $1}'` keeps, - // so every Windows upgrade failed its checksum ("actual: \48a6…"). + // What install.sh sees as PREFIX: Git Bash accepts `C:/...`, and its output (which + // withoutStagingPathHint matches on) then names the path the same way. const shellPrefix = windows ? installPrefix.split(sep).join("/") : installPrefix; @@ -204,15 +202,25 @@ export async function run(args: string[]): Promise { await swapIn(join(installPrefix, installedName), binaryPath); } } catch { - // swapIn put the running binary back; say so as {code,error,hint} - // rather than letting a stack trace escape to the catch-all. + // swapIn puts the running binary back; say so as {code,error,hint} rather + // than letting a stack trace escape to the catch-all. If that rename failed + // too, there is no docx.exe left, and the only useful answer is where it went. + if (!(await Bun.file(binaryPath).exists())) { + return fail( + "UPGRADE_FAILED", + `could not swap the new binary into ${binaryPath}, or put the old one back`, + `Your previous binary is at ${binaryPath}.old. Rename it to ${installedName} to restore docx.`, + ); + } return fail( "UPGRADE_FAILED", `could not swap the new binary into ${binaryPath} — it is unchanged`, "Something (often antivirus scanning the new file) held it open. Wait a moment and run docx upgrade again.", ); } finally { - await rm(installPrefix, { recursive: true, force: true }); + // A leftover folder is harmless and the next upgrade clears it, so a staged + // file still held open must not replace the result with a thrown error. + await rm(installPrefix, { recursive: true, force: true }).catch(() => {}); } } if (exitCode !== 0) { From 3f90a6f10fd4fc4137fd8e5bd02c3ba81b738a12 Mon Sep 17 00:00:00 2001 From: Kirill Klimuk Date: Fri, 30 Oct 2026 13:46:38 -0700 Subject: [PATCH 6/6] test(install): ask sh which checksum tools it can run On the Windows runner Bun.which found a shasum that sh can't execute, so the test failed on a tool install.sh would never pick. Ask sh with command -v, the same check install.sh uses. Co-Authored-By: Claude Opus 5.5 --- tests/cli/installer.test.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/tests/cli/installer.test.ts b/tests/cli/installer.test.ts index 892163a..1b0d94b 100644 --- a/tests/cli/installer.test.ts +++ b/tests/cli/installer.test.ts @@ -95,14 +95,15 @@ describe("installer sha256_of", () => { const expected = new Bun.CryptoHasher("sha256").update("abc").digest("hex"); for (const { definition, tool } of definitions) { - if (!Bun.which(tool)) continue; - const proc = Bun.spawn( - ["sh", "-c", `${definition}\nsha256_of "$1"`, "sh", path], - { stdout: "pipe" }, - ); - const stdout = await new Response(proc.stdout).text(); + // Asks sh, as install.sh does: on Windows Bun.which also finds a shasum.bat + // that sh can't run. An empty stdout therefore means the tool is absent. + const script = `command -v ${tool} >/dev/null 2>&1 || exit 0\n${definition}\nsha256_of "$1"`; + const proc = Bun.spawn(["sh", "-c", script, "sh", path], { + stdout: "pipe", + }); + const stdout = (await new Response(proc.stdout).text()).trim(); expect(await proc.exited).toBe(0); - expect(stdout.trim()).toBe(expected); + if (stdout) expect(stdout).toBe(expected); } }); });