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/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..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. @@ -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/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/references/troubleshooting.md b/skills/docx-cli/references/troubleshooting.md index c4089bc..d228f56 100644 --- a/skills/docx-cli/references/troubleshooting.md +++ b/skills/docx-cli/references/troubleshooting.md @@ -50,6 +50,17 @@ 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. + +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/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/src/cli/CLAUDE.md b/src/cli/CLAUDE.md index fa3ad50..c30eda0 100644 --- a/src/cli/CLAUDE.md +++ b/src/cli/CLAUDE.md @@ -74,11 +74,24 @@ 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. 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 +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..7cf5411 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, 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 { @@ -15,6 +16,8 @@ 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 @@ -29,7 +32,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 +45,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 +115,36 @@ 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"; + // 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 "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 +156,17 @@ 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; + // 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; + // 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 +177,7 @@ export async function run(args: string[]): Promise { stderr: "pipe", env: { ...process.env, - PREFIX: prefix, + PREFIX: shellPrefix, VERSION: target, REQUIRE_CHECKSUM: "1", }, @@ -162,12 +186,44 @@ 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, shellPrefix) + : { write: writeStdout, flush: async () => {} }; await Promise.all([ - forward(proc.stdout, writeStdout), + forward(proc.stdout, stdout.write), forward(proc.stderr, writeStderr), ]); + await stdout.flush(); - if ((await proc.exited) !== 0) { + const exitCode = await proc.exited; + if (windows) { + try { + if (exitCode === 0) { + await swapIn(join(installPrefix, installedName), binaryPath); + } + } catch { + // 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 { + // 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) { // 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/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 new file mode 100644 index 0000000..cfbe232 --- /dev/null +++ b/src/cli/upgrade/swap-in.ts @@ -0,0 +1,23 @@ +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. 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, +): Promise { + const aside = `${binaryPath}.old`; + await rm(aside, { force: true }); + await rename(binaryPath, aside); + try { + await rename(staged, binaryPath); + } catch (error) { + await rename(aside, binaryPath); + throw error; + } +} diff --git a/tests/cli/installer.test.ts b/tests/cli/installer.test.ts index 04f2bc4..1b0d94b 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,46 @@ 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) { + // 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); + if (stdout) expect(stdout).toBe(expected); + } + }); }); diff --git a/tests/cli/upgrade.test.ts b/tests/cli/upgrade.test.ts index 61fd542..f6b5ee9 100644 --- a/tests/cli/upgrade.test.ts +++ b/tests/cli/upgrade.test.ts @@ -1,8 +1,11 @@ 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 { withoutStagingPathHint } from "../../src/cli/upgrade/staging-path-hint"; +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 +85,71 @@ 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); + }); + + // 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 // 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 +159,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 +177,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");