From f2fc06b99c416b6906de50a63f6e575a61562489 Mon Sep 17 00:00:00 2001 From: James Kominick Date: Sun, 26 Jul 2026 17:15:04 -0400 Subject: [PATCH 1/8] docs: record the signed-off bundle-install design decisions Replace the open-questions section of the bundle-install spec with the decisions, and flip the status to pending: - rename the archive-side setter to `bundle_path_in_archive`, symmetric with `bin_path_in_archive` - hard `build()` error when bundle mode is combined with an explicit `bin_path_in_archive` / `bin_install_path` - macOS resolves the install path from the nearest `.app` ancestor of `current_exe()` when unset; no ancestor is an error - non-macOS in-bundle swaps allowed, with the file-locking caveat documented - no cross-device fallback: staging lives in the destination parent, so every rename is same-filesystem - `verify_binary` receives the staged bundle root - add `Error::NoAppBundle`, `Error::ConflictingConfig`, and `Error::AppTranslocated`; `Error` is `#[non_exhaustive]`, so these are not breaking - detect an `AppTranslocation` path component and fail early rather than surfacing a read-only-filesystem error mid-swap (BNDL-5-3) --- specs/README.md | 3 +- specs/bundle-install.md | 85 +++++++++++++++++++++++++---------------- 2 files changed, 55 insertions(+), 33 deletions(-) diff --git a/specs/README.md b/specs/README.md index a85d2ce..619f3f9 100644 --- a/specs/README.md +++ b/specs/README.md @@ -51,7 +51,8 @@ design before it can be built). Keep each row's status current with `spec.py set | Restart After Update | done | [ref-restart.md](ref-restart.md) | | Update-check Interval Guard | done | [ref-check-interval.md](ref-check-interval.md) | | Manifest Backend | done | [ref-manifest-backend.md](ref-manifest-backend.md) | -| Bundle Install | research | [bundle-install.md](bundle-install.md) | +| Bundle Install | pending | [bundle-install.md](bundle-install.md) | +| Auth Token from Env | pending | [auth-token-from-env.md](auth-token-from-env.md) | ## Conventions diff --git a/specs/bundle-install.md b/specs/bundle-install.md index e4593db..681bb0f 100644 --- a/specs/bundle-install.md +++ b/specs/bundle-install.md @@ -1,6 +1,7 @@ # Bundle Install (directory bundles, #145 phase A) -Status: research (design for maintainer sign-off; not implemented) +Status: pending (design signed off 2026-07-26, see Design decisions; not +implemented) ## Problem @@ -42,7 +43,7 @@ Non-goals). Phase C (relaunch) shipped as `restart()` / `restart_with()` ## BNDL-1: builder API -BNDL-1-1. `bundle_root_in_archive(path: impl Into) -> &mut Self` is added +BNDL-1-1. `bundle_path_in_archive(path: impl Into) -> &mut Self` is added to the common builder setters (`src/macros.rs`), available on every backend's `UpdateBuilder`. It names the bundle root directory inside the archive, relative to the archive root (e.g. `MyApp.app` or `{{ bin }}-{{ version }}/MyApp.app`). @@ -53,7 +54,7 @@ substitution and `is_safe_asset_name` traversal defense as `bin_path_in_archive` BNDL-1-2. `bundle_install_path(path: impl AsRef) -> &mut Self` names the installed bundle directory to replace (e.g. `/Applications/MyApp.app`). -BNDL-1-3. Setting `bundle_root_in_archive` selects bundle mode. Default +BNDL-1-3. Setting `bundle_path_in_archive` selects bundle mode. Default `bundle_install_path` on macOS: the nearest ancestor of `std::env::current_exe()` whose file name ends in `.app`. Resolution happens in `build()`; no `.app` ancestor and no explicit path => a config error naming the @@ -98,7 +99,7 @@ There is no cross-device case by construction, and phase A has no copy fallback (open question Q5). BNDL-2-3. Extraction: `Extract::from_source(archive).extract_into(staging)`. -The staged bundle root is `staging/`. +The staged bundle root is `staging/`. Missing or not a directory => error, nothing touched. BNDL-2-4. The `verify_binary` hook, when set, runs against the staged bundle @@ -176,13 +177,22 @@ written as regular files (documented; `.app` is not a windows concern). See ## BNDL-5: errors and guarantees -BNDL-5-1. New error variants (naming open, Q7): +BNDL-5-1. New error variants. `Error` is `#[non_exhaustive]` +(`src/errors.rs:21`), so each addition is a minor-version change: - `Error::NoAppBundle { exe: PathBuf }` ("ConfigError: no `.app` ancestor of ; set bundle_install_path explicitly") for failed macOS default - detection. -- A config-conflict error for BNDL-1-4 (either a new - `Error::ConflictingConfig { .. }` or reuse of the `MissingField` display - family; decide with the maintainer). + detection. Matchable so a caller can prompt for a path instead of failing. +- `Error::ConflictingConfig { field, conflict }` for BNDL-1-4. +- `Error::AppTranslocated { exe: PathBuf }` for BNDL-5-3. + +BNDL-5-3. App Translocation: a quarantined `.app` runs from a read-only +randomized mount, so the detected bundle path cannot be swapped. Detection +checks for an `AppTranslocation` path component in `current_exe()` during +default-path resolution and returns `Error::AppTranslocated`, naming the +translocated exe and directing the user to move the app (which clears +quarantine) before updating. Without the check the failure surfaces as a bare +read-only-filesystem IO error from mid-swap, on the most common +first-run-after-download path on macOS. BNDL-5-2. Rollback guarantee: before step 2 of BNDL-2-5 nothing under `bundle_install_path` has changed. A failure at step 2 or 3 restores the old @@ -204,7 +214,7 @@ no per-file partial window) and the exe-aside step for running-image safety. shipped. Docs must note: a bundle modified after signing fails Gatekeeper, and a quarantined app running under App Translocation executes from a read-only randomized mount, so default `.app` detection finds a path that - cannot be swapped (documented limitation; detection option in Q9). + cannot be swapped (detected and rejected, BNDL-5-3). - No privilege escalation (consistent with #112): an unwritable `/Applications` surfaces as an error; sudo/UAC re-exec is the application's choice. @@ -234,28 +244,39 @@ no per-file partial window) and the exe-aside step for running-image safety. post-swap `codesign --verify` and relaunch via `restart()`; windows and linux directory-bundle swap with the exe inside and outside the bundle. -## Open questions (maintainer sign-off needed) - -Q1. Naming: `bundle_root_in_archive` / `bundle_install_path` vs alternatives - (`bundle_path_in_archive`, `bundle_dir`). Spec assumes the former. -Q2. Mutual exclusion (BNDL-1-4): hard `build()` error vs last-setter-wins. - Spec recommends the hard error. -Q3. macOS default detection: on automatically whenever bundle mode is set - without an explicit path (spec's position), or opt-in only / always - explicit everywhere. -Q4. Non-macOS in-bundle swaps: allow with documented caveats (spec's - position) vs hard error on windows when `current_exe()` is inside the - bundle. -Q5. Cross-device / staging fallback: none (spec's position: staging in the - destination parent makes cross-device impossible; a copy fallback would - forfeit atomicity) vs fall back to copy. -Q6. `verify_binary` hook target in bundle mode: staged bundle root (spec's - position), staged exe path, or skip the hook in bundle mode. -Q7. Error variant names for BNDL-5-1. -Q8. RESOLVED: the zip-symlink fix (BNDL-4-2) landed as a standalone bug-fix - PR (#199) ahead of bundle mode. -Q9. App Translocation: detect (`/AppTranslocation/` path component) and fail - with a specific error, or document-only. Spec leans detect-and-error. +## Design decisions (signed off 2026-07-26) + +D1. Naming: `bundle_path_in_archive` / `bundle_install_path`, symmetric with + the existing `bin_path_in_archive` / `bin_install_path` pair. The + directory-ness is carried by the docs and the swap semantics, not by the + setter name (rejected: `bundle_root_in_archive`, `bundle_dir`). +D2. Mutual exclusion (BNDL-1-4): hard `build()` error. Silently dropping one + setter can install to the wrong path; a config conflict fails before any + network work (rejected: last-setter-wins, warn-and-continue). +D3. macOS default detection: on automatically whenever bundle mode is set + without an explicit path, mirroring `bin_install_path`'s `current_exe()` + default. No `.app` ancestor is a hard error naming the exe, and D9 covers + the quarantined case, so every failure mode is explicit. +D4. Non-macOS in-bundle swaps: allowed, with the BNDL-2-7 caveat documented. + One code path on all platforms; a windows file-locking failure rolls back + and surfaces an error naming the path, so the failure is diagnosable + rather than corrupting (rejected: hard error on windows, ack-setter gate). +D5. Cross-device: no fallback. Staging in `bundle_install_path.parent()` makes + a cross-device rename impossible by construction; a copy fallback would + forfeit the all-or-nothing rename guarantee (rejected: copy on EXDEV). +D6. `verify_binary` in bundle mode receives the staged bundle root. Always + resolvable (the exe path is not, when the running exe lives outside the + bundle) and it is the path `codesign --verify --deep` wants. Documented as + a directory in bundle mode; a hook that wants the exe joins the relative + path itself (rejected: staged exe path, skipping the hook). +D7. Errors: three new variants, `Error::NoAppBundle`, + `Error::ConflictingConfig`, and `Error::AppTranslocated` (BNDL-5-1). + `Error` is `#[non_exhaustive]`, so these are matchable without a breaking + change (rejected: string-only distinction via the existing config family). +D8. The zip-symlink fix (BNDL-4-2) landed as a standalone bug-fix PR (#199) + ahead of bundle mode. +D9. App Translocation: detect and fail with `Error::AppTranslocated` + (BNDL-5-3), not document-only. ## Related From cf645c719e216b25db92c0ac5e772f3dd3c72ac5 Mon Sep 17 00:00:00 2001 From: James Kominick Date: Sun, 26 Jul 2026 17:15:12 -0400 Subject: [PATCH 2/8] docs: spec `auth_token_from_env` and a distinct rate-limit error Records the design for the shared-egress-IP case, where the 60/hour unauthenticated GitHub REST budget is pooled across every client behind one public IP and a check returns 403 through no traffic of its own: - `auth_token_from_env()` on the backend builders, reading the conventional per-forge variables (`GITHUB_TOKEN` then `GH_TOKEN`, `GITLAB_TOKEN` then `CI_JOB_TOKEN`, `GITEA_TOKEN`, `GITEE_TOKEN`) and leaving the token unset when none is present. Opt-in, never an automatic env read. - `Error::RateLimited { status, url, reset_at, retry_after }`, classified from a zero remaining-quota header on a 403 or 429 so it is distinguishable from a credential failure. - Documents that the unauthenticated budget is counted per source IP. --- specs/auth-token-from-env.md | 112 +++++++++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 specs/auth-token-from-env.md diff --git a/specs/auth-token-from-env.md b/specs/auth-token-from-env.md new file mode 100644 index 0000000..8b474d0 --- /dev/null +++ b/specs/auth-token-from-env.md @@ -0,0 +1,112 @@ +# Auth token from env, and rate-limit errors + +Status: pending (decided 2026-07-26; not implemented) + +## Problem + +An update check behind a shared egress IP (a NAT'd corporate network) fails with +HTTP 403 once the unauthenticated GitHub REST budget, 60 requests/hour counted +per source IP, is exhausted by everyone sharing that IP. The fix is to send a +token, but the crate makes each consumer plumb one in itself, and the resulting +403 is indistinguishable from a real credential failure. + +Current behavior: + +- `auth_token(impl Into)` on every backend builder is the only way to + supply a token (`src/backends/github.rs:147`). There is no env-var path, so + every consumer writes the same `std::env::var("GITHUB_TOKEN")` plumbing, + including the skip-when-empty case. +- The token is already forwarded safely: `apply_auth` attaches it only to a URL + whose host matches the configured API base or an `allow_auth_host` entry, and + only over https (`src/backends/common.rs:322-356`), with a per-backend scheme + (github/gitea `Token`, gitlab `Bearer`, `common.rs:196`). Nothing about the + env source changes that gate. +- A rate-limited response surfaces as `Error::Unauthorized { status: 403, url }` + (`src/errors.rs:75`), the same variant as a bad token, so a caller cannot tell + "wait for the window to reset, or set a token" from "these credentials are + wrong". README:360 documents the limits and tells the reader to recognize the + rate-limit case by its symptom. + +## AUTH-1: token from the environment + +AUTH-1-1. `auth_token_from_env()` is added to the backend `UpdateBuilder` and +`ReleaseListBuilder` types that take an `auth_token`. It reads the backend's +conventional env vars in order and uses the first that is present and non-empty +after trimming surrounding whitespace: + +- github: `GITHUB_TOKEN`, then `GH_TOKEN` (matching the `gh` CLI). +- gitlab: `GITLAB_TOKEN`, then `CI_JOB_TOKEN`. +- gitea: `GITEA_TOKEN`. +- gitee: `GITEE_TOKEN`. + +AUTH-1-2. No variable set (or all empty) leaves `auth_token` unset: the request +goes out unauthenticated exactly as today, no error. This makes the call safe to +place unconditionally in an application that also runs outside CI or a corporate +network. + +AUTH-1-3. Reading env is opt-in, never automatic. A library that harvests +credentials from the environment without being asked is surprising, and the +configured API base can be a self-hosted host, so an implicit read would decide +on its own to send a user's token somewhere. The explicit call keeps the +decision with the embedding application. `auth_token(..)` and +`auth_token_from_env()` are last-setter-wins. + +AUTH-1-4. The env read happens in the setter (not at request time), so the +resolved value is visible in the builder's `Debug` output (redacted as +``, `common.rs:273`) and the behavior does not depend on env changes made +later in the process. + +AUTH-1-5. Tests: the env-var precedence is exercised through a pure helper +taking the candidate `(name, value)` pairs, so no test mutates process env +(which is racy under the parallel test harness). Cover first-wins, empty-skip, +whitespace-trim, and none-set. + +## AUTH-2: distinguishable rate-limit error + +AUTH-2-1. New variant `Error::RateLimited { status, url, reset_at, retry_after }` +(`Error` is `#[non_exhaustive]`, `src/errors.rs:21`, so this is a minor-version +addition). `reset_at` is the parsed reset instant when the response carries one, +`retry_after` the `Retry-After` delay when present; both `Option`. + +AUTH-2-2. A 403 (or 429) response is classified as `RateLimited` instead of +`Unauthorized` when it carries a zero remaining-quota header: +`x-ratelimit-remaining: 0` with `x-ratelimit-reset` (github, gitea, gitee), or +`RateLimit-Remaining: 0` (gitlab). Absent those headers the classification is +unchanged. + +AUTH-2-3. `Error::http_status()` returns the status for `RateLimited` as it does +for the other HTTP variants, and `Error::url()` returns its URL +(`src/errors.rs:267`, `:279`). + +AUTH-2-4. The `Display` string names rate limiting, the reset time when known, +and the token remedy, rather than reading as an auth failure. + +AUTH-2-5. Tests: classification from synthetic response headers (403 with +remaining 0 -> `RateLimited`; 403 without the headers -> `Unauthorized`; 429 +with headers -> `RateLimited`), plus `http_status()` / `url()` accessor +coverage. + +## AUTH-3: docs + +AUTH-3-1. The README / lib.rs rate-limit section gains the shared-IP mechanism: +the 60/hour budget is per source IP, so on a NAT'd network it is pooled across +everyone behind that IP and can be exhausted by other people entirely, which is +why a lightly-used application still sees 403s there. + +AUTH-3-2. It also documents `auth_token_from_env()` as the one-line remedy, and +`Error::RateLimited` as the variant to match for backoff. + +## Non-goals + +- No automatic env fallback (AUTH-1-3). +- No credential-helper, keychain, netrc, or `gh auth token` shell-out lookups. +- No automatic wait-and-retry on `RateLimited`. Retrying a rate-limited request + only consumes more quota; backing off is the caller's policy decision, and + `UpdateCheckGuard` (`ref-check-interval.md`) is the throttle the crate offers. + +## Related + +- `ref-github-backend.md`, `ref-common-config.md` (auth token threading and the + host gate) +- `ref-errors.md` (variant inventory) +- `ref-check-interval.md` (reducing check frequency) From 53f7c08fbe6dc17e2745137eea940584d22a667c Mon Sep 17 00:00:00 2001 From: James Kominick Date: Sun, 26 Jul 2026 18:19:03 -0400 Subject: [PATCH 3/8] feat: install directory bundles via `bundle_path_in_archive` Setting `bundle_path_in_archive(..)` selects bundle mode, where the named directory inside the archive replaces `bundle_install_path(..)` as one unit instead of a single file replacing `bin_install_path`. Aimed at macOS `.app` bundles, whose resources and code signature have to move with the executable. - Add the `bundle_path_in_archive` / `bundle_install_path` setters to the shared builder surface, threaded through `CommonBuilderConfig` -> `CommonConfig` -> `FinishCtx`, so the sync and async finish tails stay identical. `build()` rejects bundle mode combined with an explicit `bin_install_path` or `bin_path_in_archive` as `Error::ConflictingConfig`. - Resolve the install path on macOS from the nearest `.app` ancestor of `current_exe()` when unset, with `Error::NoAppBundle` when there is none and `Error::AppTranslocated` for a quarantined app running from a read-only translocated mount. Other targets require the setter. - Swap by rename: extract into a temp dir inside the destination's parent (so every rename is same-filesystem, with no copy fallback), stash the displaced tree, then rename the staged tree into place. A failure at any step reverses the applied renames and returns the original error. A running executable inside the bundle is renamed aside first, so its path holds the new executable afterwards and no `self_replace` call is involved. - Run `verify_binary` against the staged bundle root, and point the opt-in `check_install_path_writable` preflight at the bundle's parent directory. - Document the flow in the crate docs and record it in the bundle-install, update-pipeline, errors, and common-config specs. --- CHANGELOG.md | 13 + README.md | 54 +++ specs/README.md | 2 +- specs/bundle-install.md | 13 +- specs/ref-common-config.md | 14 + specs/ref-errors.md | 21 +- specs/ref-update-pipeline.md | 85 +++- src/backends/common.rs | 166 +++++++ src/errors.rs | 53 +++ src/lib.rs | 55 +++ src/macros.rs | 69 +++ src/update.rs | 830 ++++++++++++++++++++++++++++++++++- 12 files changed, 1337 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6909889..2d9eea5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,19 @@ ## [unreleased] ### Added +- Directory-bundle installs (macOS `.app`): `bundle_path_in_archive(..)` names the bundle directory + inside the release archive and selects bundle mode, where the whole tree replaces + `bundle_install_path(..)` instead of one file replacing `bin_install_path`. The new bundle is + staged in the destination's parent and swapped by rename with the displaced tree stashed, so a + failure restores the original bundle; a running executable inside the bundle is renamed aside + first, so its path holds the new executable afterwards and composes with `restart()`. On macOS + `bundle_install_path` defaults to the nearest `.app` ancestor of the running executable. The + `verify_binary` hook receives the staged bundle root, and the opt-in + `check_install_path_writable` preflight probes the bundle's parent directory. Adds + `Error::NoAppBundle` (no `.app` ancestor to derive the path from), `Error::ConflictingConfig` + (bundle mode combined with an explicit `bin_install_path` / `bin_path_in_archive`), and + `Error::AppTranslocated` (a quarantined app running from a read-only translocated mount). + ([#145](https://github.com/jaemk/self_update/issues/145)) - `compression-tar-xz` feature: decode `.tar.xz` / `.txz` archives and plain `.xz` single-file assets (pure-Rust `lzma-rs`, no C `liblzma` dependency, so it cross-compiles like the rest of the default stack). Opt-in, mirroring `compression-tar-gz`. Adds `Compression::Xz`. diff --git a/README.md b/README.md index fe5beea..0ea95c1 100644 --- a/README.md +++ b/README.md @@ -239,6 +239,60 @@ fn update() -> Result<(), Box> { } ``` +### Bundle installs (macOS `.app`) + +A macOS application is a *directory* bundle, so replacing only the executable inside +`MyApp.app/Contents/MacOS/` leaves stale resources behind and breaks the bundle's code signature. +Set `bundle_path_in_archive` to name the bundle directory inside the release archive and the whole +tree is installed as one unit: + +```rust +fn update() -> Result<(), Box> { + self_update::backends::github::Update::configure() + .repo_owner("me") + .repo_name("myapp") + .bin_name("myapp") + .current_version(self_update::cargo_crate_version!()) + // The bundle directory inside the archive; `{{ bin }}` / `{{ target }}` / `{{ version }}` + // substitutions work here exactly as in `bin_path_in_archive`. + .bundle_path_in_archive("MyApp.app") + // Optional on macOS: defaults to the nearest `.app` ancestor of the running executable. + .bundle_install_path("/Applications/MyApp.app") + .build()? + .update()?; + Ok(()) +} +``` + +How the swap works, and what it guarantees: + +- The archive is extracted in full into a temporary directory **inside the install path's parent**, + so every rename is on one filesystem (there is no cross-device fallback, and the parent needs + room for one more copy of the bundle). +- The installed tree is stashed, then the staged tree is renamed into place. A failure at any step + restores the original bundle, and the error names the bundle path. Once the final rename lands the + update is committed. +- When the running executable lives inside the bundle it is renamed aside first, so the old tree + holds no running image. After a successful update the running executable's path holds the new + bundle's executable, and the process can relaunch itself with `restart()` (see + [Restarting after an update](#restarting-after-an-update)). +- Bundle mode replaces a directory, so combining it with an explicit `bin_install_path` or + `bin_path_in_archive` is rejected by `build()` (`Error::ConflictingConfig`). `bin_name` is still + required: it selects the asset and feeds `{{ bin }}`. +- The `verify_binary` hook receives the **staged bundle root**, which is what + `codesign --verify --deep` wants; a rejection aborts before anything is replaced. +- The crate never signs, notarizes, or staples: ship an already-signed (and, for Gatekeeper, + notarized) `.app` and the swap preserves exactly what you shipped. A quarantined app running from + a read-only App Translocation mount cannot update itself in place; that is detected up front as + `Error::AppTranslocated`, and the fix is to move the app (which clears the quarantine) and + relaunch it. + +Directory bundles on linux and windows go through the same code path. On windows the swap fails, +and rolls back, if the process holds files inside the bundle open beyond its own executable (a DLL +loaded from the bundle, for example). `.deb` / `.msi` packages are a different shape entirely -- +hand the downloaded file to `dpkg -i` / `msiexec /i` yourself; the crate's replace-and-verify +semantics do not apply to a system installer. + ### Checksum verification With the `checksums` feature, the crate verifies the downloaded artifact against a digest diff --git a/specs/README.md b/specs/README.md index 619f3f9..20a95db 100644 --- a/specs/README.md +++ b/specs/README.md @@ -51,7 +51,7 @@ design before it can be built). Keep each row's status current with `spec.py set | Restart After Update | done | [ref-restart.md](ref-restart.md) | | Update-check Interval Guard | done | [ref-check-interval.md](ref-check-interval.md) | | Manifest Backend | done | [ref-manifest-backend.md](ref-manifest-backend.md) | -| Bundle Install | pending | [bundle-install.md](bundle-install.md) | +| Bundle Install | done | [bundle-install.md](bundle-install.md) | | Auth Token from Env | pending | [auth-token-from-env.md](auth-token-from-env.md) | ## Conventions diff --git a/specs/bundle-install.md b/specs/bundle-install.md index 681bb0f..b15f547 100644 --- a/specs/bundle-install.md +++ b/specs/bundle-install.md @@ -1,7 +1,16 @@ # Bundle Install (directory bundles, #145 phase A) -Status: pending (design signed off 2026-07-26, see Design decisions; not -implemented) +Status: implemented (design signed off 2026-07-26, see Design decisions; shipped +for directory bundles as specified below, with `.deb`/`.msi` remaining a +docs-only recipe per Non-goals) + +Implementation: `bundle_path_in_archive` / `bundle_install_path` on the common +builder setters (`src/macros.rs`), resolved by +`CommonBuilderConfig::resolve_bundle_mode` (`src/backends/common.rs:638`) and +`default_bundle_install_path` (`src/update.rs:1801`); the finish tail branches to +`install_bundle` / `swap_bundle` (`src/update.rs:1630`, `:1683`). See +`ref-update-pipeline.md` ("Bundle install") for the behavior reference and the +test list. ## Problem diff --git a/specs/ref-common-config.md b/specs/ref-common-config.md index dad4d6f..99a13e7 100644 --- a/specs/ref-common-config.md +++ b/specs/ref-common-config.md @@ -50,6 +50,14 @@ auto-derived from `bin_name`), `show_download_progress`, `show_output`, - Defaulted: `target` falls back to `get_target()` (`common.rs:148-151`); `bin_install_path` falls back to `std::env::current_exe()` (`common.rs:162-165`), which can itself error and propagates via `?`. +- Bundle mode is resolved first, by `resolve_bundle_mode` (`common.rs:638`), which returns the + `(bundle_path_in_archive, bundle_install_path)` pair stored on `CommonConfig` (both `None` when + `bundle_path_in_archive` is unset). With it set: an explicit `bin_install_path`, or a + `bin_path_in_archive` whose `bin_path_in_archive_auto` is `false`, is + `Error::ConflictingConfig { field, conflict }`; an unset `bundle_install_path` resolves through + `update::default_bundle_install_path()` (macOS: the nearest `.app` ancestor of `current_exe()`, + else `Error::NoAppBundle` / `Error::AppTranslocated`; other targets: + `Error::MissingField { field: "bundle_install_path" }`). - All other fields are cloned through unchanged. Note `target` and `current_version` become owned `String`, and `bin_install_path` an owned `PathBuf`, in `CommonConfig`. @@ -89,6 +97,12 @@ The `@shared` vocabulary (`macros.rs:231-462`): - `bin_path_in_archive(impl Into)` (`macros.rs:328`) - supports `{{ bin }}`, `{{ target }}`, `{{ version }}` substitutions; sets `bin_path_in_archive_auto = false` so a later `bin_name` call will not overwrite it. +- `bundle_path_in_archive(impl Into)` - names the bundle directory inside the archive and + selects bundle mode; supports the same `{{ bin }}` / `{{ target }}` / `{{ version }}` + substitutions as `bin_path_in_archive`. +- `bundle_install_path>(A)` - the installed bundle directory bundle mode replaces; + optional on macOS (defaults to the nearest `.app` ancestor of the running exe), required + elsewhere in bundle mode. - `show_download_progress(bool)` (`macros.rs:336`). - `progress_style(ProgressStyle)` (`macros.rs:342`) - sets template and chars via the typed `ProgressStyle { template, chars }` newtype (`ProgressStyle::new(template, chars)`). diff --git a/specs/ref-errors.md b/specs/ref-errors.md index 22cf3f3..1fa9e61 100644 --- a/specs/ref-errors.md +++ b/specs/ref-errors.md @@ -35,7 +35,10 @@ code builds them via the public constructors (`http_status_error(404, ..)`, | `MissingAssetField { field: String }` | A release/asset payload was missing a required field (`url`/`name`/`tag_name`/`created_at`/`assets`/`browser_download_url`/`assets.links`) in each backend's DTO conversion (`github.rs`, `gitlab.rs`, `gitea.rs`). `String` so a custom source can report a dynamic field path (e.g. `assets[2].url`). `#[non_exhaustive]`. | none | no (struct fields) | | `InvalidResponse { source: Box }` | A backend response could not be parsed: a malformed (non-array) JSON release-listing body (`github.rs`, `gitlab.rs`, `gitea.rs`), the S3 listing regex build failure, and the S3 XML parse failure (`s3.rs`). The underlying error is carried as `source`. `#[non_exhaustive]`. | none | yes (boxed source) | | `MissingField { field: &'static str }` | A required builder/configuration field was not set: `current_version`/`bin_name`/`bin_path_in_archive` (`common.rs`), `version` (`update.rs`), `source` (`custom.rs`), `repo_owner`/`repo_name` (`github.rs`, `gitlab.rs`, `gitea.rs`), `host` (`gitea.rs`), `bucket_name`/`region` (`s3.rs`). `#[non_exhaustive]`. | none | no (struct fields) | -| `InstallPathNotWritable { path: PathBuf }` | The opt-in preflight probe (`check_install_path_writable(true)`, `probe_install_path_writable` at `update.rs:1606`) when the path is definitely not writable, or the install step (`map_install_io_error` at `update.rs:1582`) when the replace/move fails with `PermissionDenied`. `path` is the configured `bin_install_path`. `#[non_exhaustive]`. | none | no (struct fields) | +| `InstallPathNotWritable { path: PathBuf }` | The opt-in preflight probe (`check_install_path_writable(true)`, `probe_writable` at `update.rs:1865`) when the path is definitely not writable, or the install step (`map_install_io_error` at `update.rs:1582`) when the replace/move fails with `PermissionDenied`. `path` is the configured `bin_install_path`, or in bundle mode the bundle's parent directory. `#[non_exhaustive]`. | none | no (struct fields) | +| `NoAppBundle { exe: PathBuf }` | Bundle mode with no explicit `bundle_install_path` on macOS, when `current_exe()` has no `.app` ancestor to derive it from (`default_bundle_install_path` at `update.rs:1801`). `exe` is the running executable. macOS only: other targets get `MissingField { field: "bundle_install_path" }`. `#[non_exhaustive]`. | none | no (struct fields) | +| `ConflictingConfig { field: &'static str, conflict: &'static str }` | Two builder settings that cannot both apply were set; raised from `build()` (`resolve_bundle_mode` at `backends/common.rs:638`) for `bundle_path_in_archive` combined with an explicit `bin_install_path` or `bin_path_in_archive`. `field` is the rejected setting, `conflict` the one it clashes with. `#[non_exhaustive]`. | none | no (struct fields) | +| `AppTranslocated { exe: PathBuf }` | Bundle mode on macOS when the running executable is inside an `AppTranslocation` mount, i.e. a quarantined copy on a read-only randomized path whose bundle is not the installed one (`is_translocated` at `update.rs:1838`, via `default_bundle_install_path`). `#[non_exhaustive]`. | none | no (struct fields) | | `InvalidHeader { source: Box }` | A request header (`request_header` on the builders or on `Download`) was not a valid HTTP header. The setters are infallible; the error is deferred and surfaced from `build()` (via `common.rs`) or from `Download::download_to` / `download_to_async` (`lib.rs`). The source is a crate-internal `MessageError` carrying the validation message. `#[non_exhaustive]`. | none | yes (boxed source) | | `InvalidAuthToken { source: Box }` | An auth token could not be encoded as an HTTP `Authorization` header value (`github.rs`, `gitlab.rs`, `gitea.rs`, `update.rs`). The underlying header-value parse error is carried as `source`. `#[non_exhaustive]`. | none | yes (boxed source) | | `InvalidCertificate { source: Box }` | A custom TLS root certificate could not be parsed, or the HTTP client that would trust it could not be built. Produced by `RequestConfig::check()` (`common.rs`, surfaced from `build()`) and by `Download::download_to` / `download_to_async` (`lib.rs`) when `add_root_certificate` certs are supplied. Exception: on a ureq-only build a malformed **DER** certificate is not caught at `build()` (ureq's `from_der` is infallible) and surfaces as `Transport` at connection time; PEM is validated at `build()` on both clients. `#[non_exhaustive]`. | none | yes (boxed source) | @@ -128,6 +131,9 @@ Each variant renders with a specific Display string: - `InvalidResponse { source }` -> `"ReleaseError: invalid response: {source}"` - `MissingField { field }` -> `"ConfigError: \`{field}\` required"` - `InstallPathNotWritable { path }` -> `"InstallPathNotWritableError: cannot write to install path {path}: run with elevated privileges or choose a user-writable bin_install_path"` +- `NoAppBundle { exe }` -> ``"ConfigError: no `.app` ancestor of {exe}; set bundle_install_path explicitly"`` +- `ConflictingConfig { field, conflict }` -> ``"ConfigError: `{field}` conflicts with `{conflict}`; set one or the other"`` +- `AppTranslocated { exe }` -> `"AppTranslocatedError: {exe} is running from a translocated (quarantined) copy on a read-only mount, so its bundle cannot be replaced: move the app (e.g. to /Applications) and relaunch it before updating"` - `InvalidHeader { source }` -> `"ConfigError: invalid HTTP header: {source}"` - `InvalidAuthToken { source }` -> `"ConfigError: failed to parse auth token: {source}"` - `InvalidCertificate { source }` -> `"ConfigError: invalid root certificate: {source}"` @@ -159,9 +165,9 @@ boxed-source variants `InvalidResponse`, `InvalidHeader`, `InvalidAuthToken`, `Internal` when its `source` is `Some` -- each via deref of the box. The `Internal { source: None }` form and all field-only variants (`VerificationRejected`, `ChecksumMismatch`, `Aborted`, `NotFound`, `Unauthorized`, `HttpStatus`, -`NoReleaseFound`, `MissingAssetField`, `MissingField`, `InstallPathNotWritable`, -`ArchiveNotEnabled`, `CompressionNotEnabled`, `InvalidAssetName`, `NoSignatures`, -`SignatureNonUTF8`) return `None`. The concrete inner error of +`NoReleaseFound`, `MissingAssetField`, `MissingField`, `InstallPathNotWritable`, `NoAppBundle`, +`ConflictingConfig`, `AppTranslocated`, `ArchiveNotEnabled`, `CompressionNotEnabled`, +`InvalidAssetName`, `NoSignatures`, `SignatureNonUTF8`) return `None`. The concrete inner error of a boxed variant is reachable at runtime through `source()` and `downcast_ref::()` (e.g. `err.source().and_then(|s| s.downcast_ref::())`). @@ -272,7 +278,12 @@ type directly, since `std::io::Error` is stable std.) `HttpStatus`, `Internal`, `VerificationRejected`, `NoReleaseFound`, `MissingAssetField`, `InvalidResponse`, `MissingField`, `InstallPathNotWritable`, `InvalidHeader`, `InvalidAuthToken`, `InvalidCertificate`, `InvalidProgressStyle`, `InvalidAssetName`, `NotFound`, - `ChecksumMismatch`). + `ChecksumMismatch`, `NoAppBundle`, `ConflictingConfig`, `AppTranslocated`). +- The bundle-mode config variants are raised from `build()`, before any request: `NoAppBundle` + (macOS, no `.app` ancestor to derive `bundle_install_path` from), `ConflictingConfig` (bundle mode + plus an explicit `bin_install_path`/`bin_path_in_archive`), and `AppTranslocated` (a quarantined + app running from a read-only translocated mount). Off macOS, bundle mode without an explicit + `bundle_install_path` is `MissingField { field: "bundle_install_path" }` instead. - `Error::Internal` is reserved for genuine internal/invariant failures: extractor invariants, archive-path failures, and tokio blocking-task join failures (which carry the `JoinError` as `source`). diff --git a/specs/ref-update-pipeline.md b/specs/ref-update-pipeline.md index 05e0e43..8c41fba 100644 --- a/specs/ref-update-pipeline.md +++ b/specs/ref-update-pipeline.md @@ -59,8 +59,10 @@ and `update_extended_async`'s future stays `Send` (the `PageRequest::parse` pars ### Download `resolve_and_confirm` prints the release-status block and (unless `no_confirm`) prompts -(see below). If `check_install_path_writable()` is `true`, `probe_install_path_writable` -(`update.rs:1606`) runs immediately after the confirmation and before any download +(see below). If `check_install_path_writable()` is `true`, `probe_writable` (`update.rs:1865`) runs +immediately after the confirmation and before any download, probing the bundle's parent directory in +bundle mode (`probe_dir_writable`) and otherwise `bin_install_path` +(`probe_install_path_writable`) (`update.rs:1005-1009` sync, `update.rs:1509-1510` async): only a definite `PermissionDenied` errors as `Error::InstallPathNotWritable { path }`; any other result (missing parent directory, unusual filesystem, `Ok`) proceeds. Default is `false` (off). Then a `tempfile::TempDir` is @@ -134,9 +136,9 @@ In `finish_update`, before any extraction or replacement: implemented for `.tar.gz` and `.zip` assets, not gz files". All three run on the *downloaded archive bytes* and before extraction. The last hook, -`verify_binary`, runs later inside `install_binary` on the *extracted binary*, -immediately before the swap. Ordering: verify_checksum -> release digest -> verify_keys -> -extract -> verify_binary -> replace. +`verify_binary`, runs later inside `install_binary` on the *extracted binary* (in bundle mode, on +the *staged bundle root*), immediately before the swap. Ordering: verify_checksum -> release +digest -> verify_keys -> extract -> verify_binary -> replace. ### Replace @@ -158,6 +160,45 @@ rewrapped as `Error::Io` with the message `"installing to {path}: {orig}"`, pres original `ErrorKind` for inspection. This annotation is always on, independent of the opt-in preflight probe (`check_install_path_writable`). +### Bundle install (directory bundles) + +`bundle_path_in_archive()` being `Some` selects bundle mode, resolved at `build()` time by +`CommonBuilderConfig::resolve_bundle_mode` (`backends/common.rs:638`): an explicit +`bin_install_path` or a non-auto `bin_path_in_archive` alongside it is +`Error::ConflictingConfig { field, conflict }`, and an unset `bundle_install_path` resolves via +`default_bundle_install_path` (`update.rs:1801`) -- on macOS the nearest `.app` ancestor of +`current_exe()` (`enclosing_app_bundle`, `update.rs:1825`), with a translocated exe +(`is_translocated`, `update.rs:1838`) rejected as `Error::AppTranslocated` and no `.app` ancestor as +`Error::NoAppBundle`; on every other target `Error::MissingField { field: "bundle_install_path" }`. + +In the finish tail the same `{{ bin }}` / `{{ target }}` / `{{ version }}` substitution runs over +the bundle path, then `install_bundle` (`update.rs:1630`) replaces the single-file +extract-and-install pair: two `tempfile::TempDir`s (staging and stash) are created inside +`install_parent(bundle_install_path)`, so every rename is same-filesystem and there is no +cross-device case; `Extract::extract_into` unpacks the whole archive into staging; the staged root +is `staging/`. Failure to create either temp dir goes through +`map_install_io_error` naming the bundle path. + +`swap_bundle` (`update.rs:1683`) performs the swap, taking the running exe as a parameter (so it is +testable against a temp tree). Pre-swap checks, none of which touch the destination: the staged root +must exist and be a directory (else `Error::Io` NotFound naming it); when `exe_inside_bundle` +(`update.rs:1771`, canonicalizing both sides like `same_file`) reports the running exe inside the +installed bundle, the staged tree must carry a file at the same relative path; then the +`verify_binary` hook runs against the *staged bundle root* via the shared `run_verify_hook` +(`update.rs:1611`). Then, in order: rename the running exe to `stash/exe-aside` (only when it is +inside the bundle), rename `bundle_install_path` to `stash/old` (only when it exists), rename the +staged root onto `bundle_install_path`. A failure at either later step reverses the applied renames +(old tree first, then the exe, via `restore_stashed`, `update.rs:1753`) and returns the original +error mapped by `map_install_io_error`; rollback is best-effort and logged, matching the `MoveAll` +contract. After the final rename the update is committed and the file at the running exe's path is +the new tree's executable, so no `self_replace` call is involved. On unix the stashed old image is +unlinked with the stash `TempDir`; on windows it may stay locked until process exit, which never +affects the installed tree. The swap is one code path on all targets: a windows bundle holding other +open files (a loaded DLL) fails at the directory rename and rolls back. + +Output messages in bundle mode are "Extracting archive... Done" then "Replacing bundle directory... +Done"; `ReleaseStatus` / `VersionStatus` reporting is unchanged. + ### Multi-file install `MoveAll` (`lib.rs:988`) is the transactional multi-file primitive, not used by the @@ -248,9 +289,17 @@ under feature `async`; the free `update::update_extended_async` they route to is `!no_confirm`. Suppressing one does not suppress the other. - The retry budget covers the download's request-establishment phase (before bytes stream); mid-stream failures are not retried. User `request_headers` override the crate's ACCEPT/auth headers on the download. -- When `check_install_path_writable` is `true`, the preflight probe (`probe_install_path_writable`, - `update.rs:1606`) runs after confirmation and before any download; only a definite - `PermissionDenied` errors, indeterminate results proceed. Default is `false`. +- When `check_install_path_writable` is `true`, the preflight probe (`probe_writable`, + `update.rs:1865`) runs after confirmation and before any download, targeting the bundle's parent + directory in bundle mode and `bin_install_path` otherwise; only a definite `PermissionDenied` + errors, indeterminate results proceed. Default is `false`. +- Bundle mode is all-or-nothing at whole-tree granularity: nothing under `bundle_install_path` + changes until the old tree is stashed, a failure at any step restores the old tree (and the + running exe inside it), and the original error is returned with rollback failures logged only. It + never falls back to a copy, so an install is never partially visible; and it never calls + `self_replace` (the exe rides along inside the swapped tree). +- Bundle mode and the single-file `bin_*` paths are mutually exclusive: setting both explicitly is + `Error::ConflictingConfig` from `build()`, not a silently-dropped setter. - The install step always annotates IO failures with the install path: `PermissionDenied` becomes `Error::InstallPathNotWritable { path }` and other kinds become `Error::Io` with the path in the message, `ErrorKind` preserved (`map_install_io_error`, `update.rs:1582`). Independent of the @@ -273,7 +322,25 @@ sorts-out-of-order / ignores-unparseable / falls-back-to-incompatible); `finish_update_rejects_a_mismatched_release_digest_by_default`, `finish_update_passes_a_matching_release_digest_then_proceeds`, `finish_update_release_digest_opt_out_skips_the_gate`, -`finish_update_rejects_an_unsupported_release_digest` (feature-gated). `lib.rs` `mod tests`: +`finish_update_rejects_an_unsupported_release_digest` (feature-gated); the bundle set +`swap_bundle_installs_when_nothing_is_there`, `swap_bundle_replaces_the_whole_tree`, +`swap_bundle_rejects_a_missing_or_non_directory_staged_root`, +`swap_bundle_rolls_back_when_the_install_rename_fails`, +`swap_bundle_moves_the_running_exe_aside_and_restores_its_path`, +`swap_bundle_rollback_restores_the_running_exe_inside_the_old_tree`, +`swap_bundle_requires_the_staged_tree_to_carry_the_running_exe_path`, +`swap_bundle_verifies_the_staged_root_and_a_rejection_replaces_nothing`, +`install_bundle_extracts_and_swaps_a_real_archive` (zip fixture with an exec bit and a symlink), +`exe_inside_bundle_detects_containment_through_symlinks`, +`enclosing_app_bundle_finds_the_nearest_app_ancestor`, +`is_translocated_matches_the_translocation_mount`, +`probe_writable_probes_the_bundle_parent_in_bundle_mode`, and +`probe_writable_falls_back_to_the_bin_path_without_bundle_mode`; `backends/common.rs` `mod tests` +covers the bundle-mode resolution (`build_resolves_bundle_mode_with_an_explicit_install_path`, +`build_leaves_bundle_fields_none_without_the_setter`, +`build_rejects_bundle_mode_with_an_explicit_bin_install_path`, +`build_rejects_bundle_mode_only_with_an_explicit_bin_path_in_archive`, +`build_requires_bundle_install_path_off_macos`). `lib.rs` `mod tests`: `detect_*` (archive detection), `unpack_*` / `test_extract_into` / `test_extract_file` (extraction), `move_all_commits_every_move`, `move_all_rolls_back_on_failure`, `move_all_installs_fresh_destinations`, `move_all_second_commit_is_a_noop`, diff --git a/src/backends/common.rs b/src/backends/common.rs index a259646..587d879 100644 --- a/src/backends/common.rs +++ b/src/backends/common.rs @@ -474,6 +474,14 @@ pub(crate) struct CommonBuilderConfig { /// the user). Used by `bin_name` to re-derive when called again, while leaving an explicitly /// set value untouched. pub(crate) bin_path_in_archive_auto: bool, + /// The bundle directory inside the archive, relative to the archive root (e.g. `MyApp.app`). + /// `Some` selects bundle mode: the whole directory replaces `bundle_install_path` instead of + /// one file replacing `bin_install_path`. Set via `bundle_path_in_archive`. + pub bundle_path_in_archive: Option, + /// The installed bundle directory to replace in bundle mode. Defaults on macOS to the nearest + /// `.app` ancestor of the running executable; required on every other platform. Set via + /// `bundle_install_path`. + pub bundle_install_path: Option, pub show_download_progress: bool, pub show_output: bool, pub no_confirm: bool, @@ -517,6 +525,8 @@ impl Default for CommonBuilderConfig { check_install_path_writable: false, bin_path_in_archive: None, bin_path_in_archive_auto: false, + bundle_path_in_archive: None, + bundle_install_path: None, show_download_progress: false, show_output: true, no_confirm: false, @@ -551,6 +561,9 @@ impl CommonBuilderConfig { /// current executable. `current_version`, `bin_name`, and `bin_path_in_archive` are /// required (the last is set automatically by the `bin_name` setter). pub(crate) fn build(&self) -> Result { + // Bundle mode: reject a conflicting single-file config and resolve the install path (which + // may consult `current_exe()`), before any other work. + let (bundle_path_in_archive, bundle_install_path) = self.resolve_bundle_mode()?; // Resolve the auth scheme/token into the request config so the shared header-derivation // (`apply_auth`) can apply it on both the listing and download paths. let mut request = self.request.clone(); @@ -587,6 +600,8 @@ impl CommonBuilderConfig { .ok_or(Error::MissingField { field: "bin_path_in_archive", })?, + bundle_path_in_archive, + bundle_install_path, show_download_progress: self.show_download_progress, show_output: self.show_output, no_confirm: self.no_confirm, @@ -607,6 +622,41 @@ impl CommonBuilderConfig { verifying_keys: self.verifying_keys.clone(), }) } + + /// Validate and resolve the bundle-mode options, returning the pair stored on the built + /// [`CommonConfig`]: `(bundle_path_in_archive, bundle_install_path)`, both `None` when bundle + /// mode is off. + /// + /// Bundle mode is selected by `bundle_path_in_archive`. It replaces a whole directory instead + /// of one file, so combining it with an explicit `bin_install_path` or `bin_path_in_archive` is + /// a config conflict rather than a silently-dropped setter. The value + /// `bin_path_in_archive` auto-derives from `bin_name` does not count as explicit (it is simply + /// unused in bundle mode). + /// + /// With no explicit `bundle_install_path`, macOS derives it from the running executable (the + /// nearest `.app` ancestor); every other platform requires it. + fn resolve_bundle_mode(&self) -> Result<(Option, Option)> { + let Some(path_in_archive) = self.bundle_path_in_archive.clone() else { + return Ok((None, None)); + }; + if self.bin_install_path.is_some() { + return Err(Error::ConflictingConfig { + field: "bundle_path_in_archive", + conflict: "bin_install_path", + }); + } + if self.bin_path_in_archive.is_some() && !self.bin_path_in_archive_auto { + return Err(Error::ConflictingConfig { + field: "bundle_path_in_archive", + conflict: "bin_path_in_archive", + }); + } + let install_path = match &self.bundle_install_path { + Some(p) => p.clone(), + None => crate::update::default_bundle_install_path()?, + }; + Ok((Some(path_in_archive), Some(install_path))) + } } /// The resolved common options of a built `Update`, embedded by every backend's `Update`. @@ -623,6 +673,12 @@ pub(crate) struct CommonConfig { /// Opt-in preflight writability probe of `bin_install_path` (default `false`). pub check_install_path_writable: bool, pub bin_path_in_archive: String, + /// The bundle directory inside the archive; `Some` means bundle mode, in which case + /// `bundle_install_path` is also `Some` and the single-file `bin_*` paths are unused. + pub bundle_path_in_archive: Option, + /// The resolved installed bundle directory to replace, `Some` exactly when + /// `bundle_path_in_archive` is. + pub bundle_install_path: Option, pub show_download_progress: bool, pub show_output: bool, pub no_confirm: bool, @@ -838,6 +894,116 @@ mod tests { assert_eq!(with_target.build().unwrap().target, "custom-target"); } + // --- bundle mode (BNDL-1) ---------------------------------------------------------------- + + use std::path::PathBuf; + + // A builder config with the required single-file fields set, as a base for the bundle tests. + fn bundle_base() -> CommonBuilderConfig { + CommonBuilderConfig { + current_version: Some("0.1.0".to_string()), + bin_name: Some("app".to_string()), + // As the `bin_name` setter derives it: auto, not explicit. + bin_path_in_archive: Some("app".to_string()), + bin_path_in_archive_auto: true, + ..Default::default() + } + } + + // BNDL-1-2/BNDL-1-3: with an explicit `bundle_install_path`, bundle mode resolves to that path + // on every platform and carries the archive-side path through to the built config. + #[test] + fn build_resolves_bundle_mode_with_an_explicit_install_path() { + let cfg = CommonBuilderConfig { + bundle_path_in_archive: Some("MyApp.app".to_string()), + bundle_install_path: Some(PathBuf::from("/Applications/MyApp.app")), + ..bundle_base() + }; + let built = cfg + .build() + .expect("an explicit bundle install path must build"); + assert_eq!(built.bundle_path_in_archive.as_deref(), Some("MyApp.app")); + assert_eq!( + built.bundle_install_path.as_deref(), + Some(std::path::Path::new("/Applications/MyApp.app")) + ); + } + + // Bundle mode is off by default: both resolved fields stay `None`, so the pipeline takes the + // single-file path. + #[test] + fn build_leaves_bundle_fields_none_without_the_setter() { + let built = bundle_base().build().unwrap(); + assert!(built.bundle_path_in_archive.is_none()); + assert!(built.bundle_install_path.is_none()); + } + + // BNDL-1-4: bundle mode plus an explicit `bin_install_path` is a config conflict, named in the + // error rather than silently dropping one of the two. + #[test] + fn build_rejects_bundle_mode_with_an_explicit_bin_install_path() { + let cfg = CommonBuilderConfig { + bundle_path_in_archive: Some("MyApp.app".to_string()), + bundle_install_path: Some(PathBuf::from("/Applications/MyApp.app")), + bin_install_path: Some(PathBuf::from("/usr/local/bin/app")), + ..bundle_base() + }; + match cfg.build() { + Err(crate::errors::Error::ConflictingConfig { field, conflict }) => { + assert_eq!(field, "bundle_path_in_archive"); + assert_eq!(conflict, "bin_install_path"); + } + other => panic!("expected ConflictingConfig, got {other:?}"), + } + } + + // BNDL-1-4: likewise for an explicit `bin_path_in_archive` -- but NOT for the value the + // `bin_name` setter auto-derives, which is simply unused in bundle mode. + #[test] + fn build_rejects_bundle_mode_only_with_an_explicit_bin_path_in_archive() { + let explicit = CommonBuilderConfig { + bundle_path_in_archive: Some("MyApp.app".to_string()), + bundle_install_path: Some(PathBuf::from("/Applications/MyApp.app")), + bin_path_in_archive: Some("dist/app".to_string()), + bin_path_in_archive_auto: false, + ..bundle_base() + }; + match explicit.build() { + Err(crate::errors::Error::ConflictingConfig { field, conflict }) => { + assert_eq!(field, "bundle_path_in_archive"); + assert_eq!(conflict, "bin_path_in_archive"); + } + other => panic!("expected ConflictingConfig, got {other:?}"), + } + + let auto = CommonBuilderConfig { + bundle_path_in_archive: Some("MyApp.app".to_string()), + bundle_install_path: Some(PathBuf::from("/Applications/MyApp.app")), + ..bundle_base() + }; + assert!( + auto.build().is_ok(), + "the auto-derived bin_path_in_archive must not count as a conflict" + ); + } + + // BNDL-1-3: off macOS there is no default bundle install path, so bundle mode without the + // setter is a missing-field error naming it. + #[cfg(not(target_os = "macos"))] + #[test] + fn build_requires_bundle_install_path_off_macos() { + let cfg = CommonBuilderConfig { + bundle_path_in_archive: Some("MyApp.app".to_string()), + ..bundle_base() + }; + match cfg.build() { + Err(crate::errors::Error::MissingField { field }) => { + assert_eq!(field, "bundle_install_path"); + } + other => panic!("expected MissingField, got {other:?}"), + } + } + // --- Item 5: self-fixing error messages -------------------------------------------------- #[test] diff --git a/src/errors.rs b/src/errors.rs index 3f6fb5c..6e1e35d 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -137,6 +137,42 @@ pub enum Error { /// The install path (`bin_install_path`) that could not be written. path: std::path::PathBuf, }, + /// Bundle mode is on but the bundle install path could not be derived: the running executable + /// (`current_exe()`) has no `.app` ancestor. + /// + /// Only produced on macOS, where `bundle_install_path` defaults to the nearest enclosing + /// `.app` directory. Set `bundle_install_path` explicitly (on every other platform it is + /// required in bundle mode, surfacing as + /// [`MissingField`](Error::MissingField) instead). + #[non_exhaustive] + NoAppBundle { + /// The running executable that has no enclosing `.app` bundle. + exe: std::path::PathBuf, + }, + /// Two builder settings that cannot both apply were set. + /// + /// `field` is the setting that was rejected and `conflict` the one it clashes with, e.g. + /// `bundle_path_in_archive` (which replaces a whole directory bundle) together with an + /// explicit `bin_install_path` (which replaces a single file). Returned from `build()`, before + /// any request is made. + #[non_exhaustive] + ConflictingConfig { + /// The setting that was rejected. + field: &'static str, + /// The already-set setting it conflicts with. + conflict: &'static str, + }, + /// The running app is a translocated copy, so its bundle cannot be replaced. + /// + /// macOS runs a quarantined (freshly downloaded, un-cleared) app from a read-only randomized + /// `AppTranslocation` mount, so the enclosing `.app` path is not the installed one and is not + /// writable. Move the app (for example to `/Applications`), which clears the quarantine flag, + /// and relaunch it before updating. + #[non_exhaustive] + AppTranslocated { + /// The running executable, inside the translocated bundle. + exe: std::path::PathBuf, + }, /// A bare release listing ([`ReleaseList::fetch`](crate::backends)) carries no current version, /// so [`Releases::is_update_available`](crate::update::Releases::is_update_available) has nothing /// to compare its releases against. @@ -425,6 +461,23 @@ impl std::fmt::Display for Error { privileges or choose a user-writable bin_install_path", path.display() ), + NoAppBundle { exe } => write!( + f, + "ConfigError: no `.app` ancestor of {}; set bundle_install_path explicitly", + exe.display() + ), + ConflictingConfig { field, conflict } => write!( + f, + "ConfigError: `{}` conflicts with `{}`; set one or the other", + field, conflict + ), + AppTranslocated { exe } => write!( + f, + "AppTranslocatedError: {} is running from a translocated (quarantined) copy on a \ + read-only mount, so its bundle cannot be replaced: move the app (e.g. to \ + /Applications) and relaunch it before updating", + exe.display() + ), NoCurrentVersion => write!( f, "ReleaseError: this Releases has no current_version to compare against; use \ diff --git a/src/lib.rs b/src/lib.rs index e8a78c4..b7e1576 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -246,6 +246,61 @@ fn update() -> Result<(), Box> { } ``` +### Bundle installs (macOS `.app`) + +A macOS application is a *directory* bundle, so replacing only the executable inside +`MyApp.app/Contents/MacOS/` leaves stale resources behind and breaks the bundle's code signature. +Set `bundle_path_in_archive` to name the bundle directory inside the release archive and the whole +tree is installed as one unit: + +```rust +# #[cfg(feature = "github")] +fn update() -> Result<(), Box> { + self_update::backends::github::Update::configure() + .repo_owner("me") + .repo_name("myapp") + .bin_name("myapp") + .current_version(self_update::cargo_crate_version!()) + // The bundle directory inside the archive; `{{ bin }}` / `{{ target }}` / `{{ version }}` + // substitutions work here exactly as in `bin_path_in_archive`. + .bundle_path_in_archive("MyApp.app") + // Optional on macOS: defaults to the nearest `.app` ancestor of the running executable. + .bundle_install_path("/Applications/MyApp.app") + .build()? + .update()?; + Ok(()) +} +``` + +How the swap works, and what it guarantees: + +- The archive is extracted in full into a temporary directory **inside the install path's parent**, + so every rename is on one filesystem (there is no cross-device fallback, and the parent needs + room for one more copy of the bundle). +- The installed tree is stashed, then the staged tree is renamed into place. A failure at any step + restores the original bundle, and the error names the bundle path. Once the final rename lands the + update is committed. +- When the running executable lives inside the bundle it is renamed aside first, so the old tree + holds no running image. After a successful update the running executable's path holds the new + bundle's executable, and the process can relaunch itself with `restart()` (see + [Restarting after an update](#restarting-after-an-update)). +- Bundle mode replaces a directory, so combining it with an explicit `bin_install_path` or + `bin_path_in_archive` is rejected by `build()` (`Error::ConflictingConfig`). `bin_name` is still + required: it selects the asset and feeds `{{ bin }}`. +- The `verify_binary` hook receives the **staged bundle root**, which is what + `codesign --verify --deep` wants; a rejection aborts before anything is replaced. +- The crate never signs, notarizes, or staples: ship an already-signed (and, for Gatekeeper, + notarized) `.app` and the swap preserves exactly what you shipped. A quarantined app running from + a read-only App Translocation mount cannot update itself in place; that is detected up front as + `Error::AppTranslocated`, and the fix is to move the app (which clears the quarantine) and + relaunch it. + +Directory bundles on linux and windows go through the same code path. On windows the swap fails, +and rolls back, if the process holds files inside the bundle open beyond its own executable (a DLL +loaded from the bundle, for example). `.deb` / `.msi` packages are a different shape entirely -- +hand the downloaded file to `dpkg -i` / `msiexec /i` yourself; the crate's replace-and-verify +semantics do not apply to a system installer. + ### Checksum verification With the `checksums` feature, the crate verifies the downloaded artifact against a digest diff --git a/src/macros.rs b/src/macros.rs index 274e675..2aa00ad 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -294,6 +294,12 @@ macro_rules! impl_update_config_accessors { fn bin_path_in_archive(&self) -> &str { &self.common.bin_path_in_archive } + fn bundle_path_in_archive(&self) -> Option<&str> { + self.common.bundle_path_in_archive.as_deref() + } + fn bundle_install_path(&self) -> Option<&std::path::Path> { + self.common.bundle_install_path.as_deref() + } fn show_download_progress(&self) -> bool { self.common.show_download_progress } @@ -604,6 +610,69 @@ macro_rules! impl_common_builder_setters { self } + /// Install a whole directory bundle (a macOS `.app`) instead of a single executable, + /// naming the bundle directory *inside the archive*, relative to its root (e.g. + /// `"MyApp.app"` or `"{{ bin }}-{{ version }}/MyApp.app"`). + /// + /// Calling this selects bundle mode: the archive is extracted in full and the named + /// directory replaces [`bundle_install_path`](Self::bundle_install_path) as one unit, so a + /// bundle's resources and code signature stay consistent with its executable (replacing + /// only the exe inside an `.app` leaves stale resources and breaks the signature). + /// + /// The same `{{ bin }}` / `{{ target }}` / `{{ version }}` substitutions as + /// [`bin_path_in_archive`](Self::bin_path_in_archive) apply. + /// + /// Bundle mode replaces a directory rather than a file, so combining it with an explicit + /// [`bin_install_path`](Self::bin_install_path) or + /// [`bin_path_in_archive`](Self::bin_path_in_archive) is rejected by `build()` with + /// [`Error::ConflictingConfig`](crate::errors::Error::ConflictingConfig) rather than + /// silently ignoring one of them. [`bin_name`](Self::bin_name) is still required (it names + /// the asset and feeds `{{ bin }}`); the path it auto-derives is simply unused. + /// + /// The replacement is a whole-tree swap: the new bundle is staged next to the destination, + /// the old tree is stashed, and the two are renamed, so a failure at any step restores the + /// original bundle. When the running executable lives inside the bundle it is renamed aside + /// first (the mechanism a self-replace relies on), so no running image remains in the old + /// tree; after a successful update the running exe's path holds the new executable and the + /// process can relaunch itself with [`restart`](crate::restart::restart). + /// + /// Phase A targets macOS `.app` bundles. A directory bundle on linux or windows is swapped + /// by the same code path, but on windows the swap fails (and rolls back) if the process + /// holds files inside the bundle open beyond its own executable, for example a DLL loaded + /// from it. + /// + /// See the crate-level "Bundle installs" section for a full example. + pub fn bundle_path_in_archive(&mut self, bundle_path: impl Into) -> &mut Self { + self.common.bundle_path_in_archive = Some(bundle_path.into()); + self + } + + /// Set the installed bundle directory that bundle mode replaces, e.g. + /// `"/Applications/MyApp.app"`. Only consulted when + /// [`bundle_path_in_archive`](Self::bundle_path_in_archive) is set. + /// + /// On macOS this defaults to the nearest `.app` ancestor of the running executable, so an + /// app launched from `/Applications/MyApp.app/Contents/MacOS/myapp` updates itself in + /// place; a running executable with no `.app` ancestor is an + /// [`Error::NoAppBundle`](crate::errors::Error::NoAppBundle) from `build()`, and a + /// quarantined app running from a read-only translocated mount is an + /// [`Error::AppTranslocated`](crate::errors::Error::AppTranslocated). On every other + /// platform there is no default and bundle mode requires this setter. + /// + /// The swap stages the new tree inside this path's parent directory, so the parent must be + /// writable and hold enough free space for one more copy of the bundle. There is no + /// cross-filesystem fallback (staging in the parent makes one unnecessary), and no + /// privilege escalation: an unwritable `/Applications` surfaces as + /// [`Error::InstallPathNotWritable`](crate::errors::Error::InstallPathNotWritable). + pub fn bundle_install_path>( + &mut self, + bundle_install_path: A, + ) -> &mut Self { + self.common.bundle_install_path = + Some(std::path::PathBuf::from(bundle_install_path.as_ref())); + self + } + /// Toggle download progress bar, defaults to `off`. pub fn show_download_progress(&mut self, show: bool) -> &mut Self { self.common.show_download_progress = show; diff --git a/src/update.rs b/src/update.rs index 4013ad4..a98484d 100644 --- a/src/update.rs +++ b/src/update.rs @@ -807,6 +807,21 @@ pub trait UpdateConfig: sealed::Sealed { /// Path of the binary to be extracted from release package fn bin_path_in_archive(&self) -> &str; + /// The bundle directory inside the archive (set via `bundle_path_in_archive`), relative to the + /// archive root. `Some` selects bundle mode: the whole directory replaces + /// [`bundle_install_path`](Self::bundle_install_path) instead of a single file replacing + /// [`bin_install_path`](Self::bin_install_path). Defaults to `None`. + fn bundle_path_in_archive(&self) -> Option<&str> { + None + } + + /// The installed bundle directory replaced in bundle mode, resolved at `build()` time. `Some` + /// exactly when [`bundle_path_in_archive`](Self::bundle_path_in_archive) is. Defaults to + /// `None`. + fn bundle_install_path(&self) -> Option<&std::path::Path> { + None + } + /// Flag indicating if progress information shall be output when downloading a release fn show_download_progress(&self) -> bool; @@ -1003,7 +1018,7 @@ pub trait ReleaseUpdate: UpdateConfig + UpdateInternals { // Opt-in preflight: bail before downloading if the install path is definitely not writable. if self.check_install_path_writable() { - probe_install_path_writable(self.bin_install_path())?; + probe_writable(self.bin_install_path(), self.bundle_install_path())?; } let tmp_archive_dir = tempfile::TempDir::new()?; @@ -1320,6 +1335,10 @@ struct FinishCtx { target: String, bin_name: String, bin_path_in_archive: String, + /// The bundle directory inside the archive; `Some` puts the finish tail in bundle mode, where + /// `bundle_install_path` is also `Some` and the `bin_*` paths are unused. + bundle_path_in_archive: Option, + bundle_install_path: Option, show_output: bool, verify_callback: Option>, #[cfg(feature = "checksums")] @@ -1351,6 +1370,8 @@ impl FinishCtx { target: u.target().to_string(), bin_name: u.bin_name().to_string(), bin_path_in_archive: u.bin_path_in_archive().to_string(), + bundle_path_in_archive: u.bundle_path_in_archive().map(str::to_string), + bundle_install_path: u.bundle_install_path().map(std::path::Path::to_path_buf), show_output: u.show_output(), verify_callback: u.verify_callback(), #[cfg(feature = "checksums")] @@ -1414,7 +1435,12 @@ fn finish_update_owned( print_flush(show_output, "Extracting archive... ")?; - let bin_path_str = Cow::Borrowed(ctx.bin_path_in_archive.as_str()); + // In bundle mode the archive path names a directory to swap wholesale, otherwise the single + // executable to extract; the `{{ .. }}` substitutions below are identical either way. + let path_in_archive = Cow::Borrowed(match ctx.bundle_path_in_archive.as_deref() { + Some(bundle_path) => bundle_path, + None => ctx.bin_path_in_archive.as_str(), + }); // The `{{ version }}` / `{{ target }}` / `{{ bin }}` template matchers. Hoisted to `static` // `LazyLock` (I6) so each is compiled once, not rebuilt from its constant pattern on @@ -1445,13 +1471,26 @@ fn finish_update_owned( Ok(re.replace_all(str, regex::NoExpand(val))) } - let bin_path_str = substitute(&VERSION_RE, &bin_path_str, ctx.release.version())?; - let bin_path_str = substitute(&TARGET_RE, &bin_path_str, &ctx.target)?; - let bin_path_str = substitute(&BIN_RE, &bin_path_str, &ctx.bin_name)?; - let bin_path_str = bin_path_str.as_ref(); + let path_in_archive = substitute(&VERSION_RE, &path_in_archive, ctx.release.version())?; + let path_in_archive = substitute(&TARGET_RE, &path_in_archive, &ctx.target)?; + let path_in_archive = substitute(&BIN_RE, &path_in_archive, &ctx.bin_name)?; + let path_in_archive = path_in_archive.as_ref(); - Extract::from_source(tmp_archive_path).extract_file(tmp_archive_dir.path(), bin_path_str)?; - let new_exe = tmp_archive_dir.path().join(bin_path_str); + // Bundle mode: extract the whole tree beside the destination and swap the directory in one + // rename, instead of extracting and moving a single file. + if let Some(bundle_install_path) = ctx.bundle_install_path.as_deref() { + install_bundle( + tmp_archive_path, + path_in_archive, + bundle_install_path, + ctx.verify_callback.as_deref(), + show_output, + )?; + return Ok(ReleaseStatus::Updated(ctx.release)); + } + + Extract::from_source(tmp_archive_path).extract_file(tmp_archive_dir.path(), path_in_archive)?; + let new_exe = tmp_archive_dir.path().join(path_in_archive); println(show_output, "Done"); @@ -1507,7 +1546,7 @@ where // Opt-in preflight: bail before downloading if the install path is definitely not writable. // Shares the sync probe for exact parity with `update_extended`. if u.check_install_path_writable() { - probe_install_path_writable(u.bin_install_path())?; + probe_writable(u.bin_install_path(), u.bundle_install_path())?; } let tmp_archive_dir = tempfile::TempDir::new()?; @@ -1542,18 +1581,7 @@ fn install_binary( bin_install_path: &std::path::Path, verify: Option<&crate::DynVerifyFn>, ) -> Result<()> { - if let Some(verify) = verify { - // A hook that returns `Err` (an explicit rejection or a hook IO error) aborts the install; - // its message becomes the rejection reason. An error that already is a - // `VerificationRejected` (e.g. built via `Error::verification_rejected`) passes through - // unwrapped so the reason is not nested inside another rejection message. - verify(new_exe).map_err(|e| match e { - Error::VerificationRejected { .. } => e, - other => Error::VerificationRejected { - reason: Some(other.to_string()), - }, - })?; - } + run_verify_hook(new_exe, verify)?; let current_exe = std::env::current_exe()?; // Only the two install-step writes are wrapped with path context (not `current_exe()` or the // verify hook above): a permission failure here becomes `InstallPathNotWritable` naming the @@ -1573,6 +1601,243 @@ fn install_binary( Ok(()) } +/// Run the post-update verification hook (if any) on `new_path` -- the freshly-extracted binary, or +/// in bundle mode the staged bundle root -- before anything is replaced. +/// +/// A hook that returns `Err` (an explicit rejection or a hook IO error) aborts the install; its +/// message becomes the rejection reason. An error that already is a `VerificationRejected` (e.g. +/// built via `Error::verification_rejected`) passes through unwrapped so the reason is not nested +/// inside another rejection message. +fn run_verify_hook(new_path: &std::path::Path, verify: Option<&crate::DynVerifyFn>) -> Result<()> { + if let Some(verify) = verify { + verify(new_path).map_err(|e| match e { + Error::VerificationRejected { .. } => e, + other => Error::VerificationRejected { + reason: Some(other.to_string()), + }, + })?; + } + Ok(()) +} + +/// Extract the archive and replace `bundle_install_path` with the bundle directory it carries at +/// `bundle_path_in_archive`. +/// +/// Both the extraction target and the rollback stash are temporary directories created inside the +/// destination's *parent*, so every rename in [`swap_bundle`] is same-filesystem and there is no +/// cross-device case to fall back from. A failure to create either is an install-step IO error +/// naming the bundle path. +fn install_bundle( + archive: &std::path::Path, + bundle_path_in_archive: &str, + bundle_install_path: &std::path::Path, + verify: Option<&crate::DynVerifyFn>, + show_output: bool, +) -> Result<()> { + let parent = install_parent(bundle_install_path); + let staging = tempfile::TempDir::new_in(parent) + .map_err(|e| map_install_io_error(e, bundle_install_path))?; + let stash = tempfile::TempDir::new_in(parent) + .map_err(|e| map_install_io_error(e, bundle_install_path))?; + + Extract::from_source(archive).extract_into(staging.path())?; + let staged_root = staging.path().join(bundle_path_in_archive); + println(show_output, "Done"); + + print_flush(show_output, "Replacing bundle directory... ")?; + swap_bundle( + &staged_root, + bundle_install_path, + stash.path(), + &std::env::current_exe()?, + verify, + )?; + println(show_output, "Done"); + Ok(()) +} + +/// Replace the directory at `bundle_install_path` with the staged tree at `staged_root`, stashing +/// the displaced tree under `stash` so a failure can be rolled back. +/// +/// The swap is whole-tree and rename-only: +/// +/// 1. When the running executable lives inside the installed bundle, rename it aside into `stash`, +/// so the old tree holds no running image before the directory itself is renamed (the same +/// rename-the-running-image mechanism a self-replace relies on). +/// 2. Rename the installed bundle to `stash` (skipped when nothing is installed yet). +/// 3. Rename the staged tree onto `bundle_install_path`. +/// +/// A failure at step 2 or 3 reverses the renames already applied, so the original bundle (and the +/// running executable's path inside it) is restored; the error returned is always the original one. +/// Rollback is best-effort and a rollback failure is logged rather than surfaced, matching the +/// [`MoveAll`](crate::MoveAll) contract. Once step 3 succeeds the update is committed and the file +/// at the running executable's path is the new bundle's executable. +/// +/// Nothing under `bundle_install_path` is touched unless the archive really carried the bundle: the +/// staged root must exist and be a directory, and when `running_exe` is inside the bundle the staged +/// tree must carry a file at the same relative path. The `verify_binary` hook runs against the +/// staged bundle root, also before any rename. +/// +/// `running_exe` is the process's own executable (`current_exe()`), passed in rather than read here +/// so the swap is exercisable against a temporary tree. +fn swap_bundle( + staged_root: &std::path::Path, + bundle_install_path: &std::path::Path, + stash: &std::path::Path, + running_exe: &std::path::Path, + verify: Option<&crate::DynVerifyFn>, +) -> Result<()> { + if !staged_root.is_dir() { + return Err(Error::Io(std::io::Error::new( + std::io::ErrorKind::NotFound, + format!( + "the archive has no bundle directory at {}", + staged_root.display() + ), + ))); + } + + let exe_aside = match exe_inside_bundle(running_exe, bundle_install_path) { + Some((exe, rel)) => { + if !staged_root.join(&rel).is_file() { + return Err(Error::Io(std::io::Error::new( + std::io::ErrorKind::NotFound, + format!( + "the staged bundle has no file at {}, the path the running executable \ + occupies inside {}", + rel.display(), + bundle_install_path.display() + ), + ))); + } + Some(exe) + } + None => None, + }; + + run_verify_hook(staged_root, verify)?; + + let stashed_exe = stash.join("exe-aside"); + let stashed_old = stash.join("old"); + + if let Some(exe) = exe_aside.as_deref() { + fs::rename(exe, &stashed_exe).map_err(|e| map_install_io_error(e, bundle_install_path))?; + } + + let old_stashed = bundle_install_path.exists(); + if old_stashed && let Err(e) = fs::rename(bundle_install_path, &stashed_old) { + if let Some(exe) = exe_aside.as_deref() { + restore_stashed(&stashed_exe, exe); + } + return Err(map_install_io_error(e, bundle_install_path)); + } + + if let Err(e) = fs::rename(staged_root, bundle_install_path) { + // Reverse the applied renames: the old tree first, so the executable can go back inside it. + if old_stashed { + restore_stashed(&stashed_old, bundle_install_path); + } + if let Some(exe) = exe_aside.as_deref() { + restore_stashed(&stashed_exe, exe); + } + return Err(map_install_io_error(e, bundle_install_path)); + } + + Ok(()) +} + +/// Best-effort rollback rename of a stashed path back into place. A failure is logged rather than +/// returned, so the caller still surfaces the original error that triggered the rollback. +fn restore_stashed(from: &std::path::Path, to: &std::path::Path) { + if let Err(e) = fs::rename(from, to) { + log::error!( + "failed to restore {:?} from stash {:?} during rollback: {}", + to, + from, + e + ); + } +} + +/// The running executable's real path plus its path relative to `bundle`, when it lives inside the +/// bundle; `None` otherwise. +/// +/// Both sides are canonicalized (resolving symlinks and `..`) before comparing, for the same reason +/// [`same_file`] does it: `current_exe()` is symlink-resolved on some platforms while a configured +/// install path is not, so a raw prefix test can miss that the exe is inside the bundle. The +/// returned exe path is the canonical one, which is the file the swap renames aside. +fn exe_inside_bundle( + exe: &std::path::Path, + bundle: &std::path::Path, +) -> Option<(std::path::PathBuf, std::path::PathBuf)> { + let exe = fs::canonicalize(exe).unwrap_or_else(|_| exe.to_path_buf()); + let bundle = fs::canonicalize(bundle).unwrap_or_else(|_| bundle.to_path_buf()); + let rel = exe.strip_prefix(&bundle).ok()?.to_path_buf(); + if rel.as_os_str().is_empty() { + return None; + } + Some((exe, rel)) +} + +/// The directory an install path lives in: its parent, or the current directory for a bare name. +fn install_parent(path: &std::path::Path) -> &std::path::Path { + match path.parent() { + Some(p) if !p.as_os_str().is_empty() => p, + _ => std::path::Path::new("."), + } +} + +/// Resolve the default `bundle_install_path` for bundle mode when the caller set none. +/// +/// macOS derives it from the running executable ([`enclosing_app_bundle`]), mirroring how +/// `bin_install_path` defaults to `current_exe()`. A running executable with no `.app` ancestor is +/// [`Error::NoAppBundle`], and one running from a read-only translocated mount (a quarantined app, +/// whose bundle path is not the installed one) is [`Error::AppTranslocated`] rather than a +/// read-only-filesystem error from the middle of the swap. +/// +/// Every other platform has no meaningful default, so the setter is required there. +pub(crate) fn default_bundle_install_path() -> Result { + #[cfg(target_os = "macos")] + { + let exe = std::env::current_exe()?; + if is_translocated(&exe) { + return Err(Error::AppTranslocated { exe }); + } + enclosing_app_bundle(&exe).ok_or(Error::NoAppBundle { exe }) + } + #[cfg(not(target_os = "macos"))] + { + Err(Error::MissingField { + field: "bundle_install_path", + }) + } +} + +/// The nearest ancestor of `exe` whose name ends in `.app`, i.e. the macOS application bundle the +/// executable is installed in. `None` when the executable is not inside one. +/// +/// Pure and path-lexical (no filesystem access), so it is exercised on every platform even though +/// only macOS uses it for the default install path. Innermost wins for a nested bundle (an +/// `.app` shipped inside another `.app`). +#[cfg_attr(not(target_os = "macos"), allow(dead_code))] +pub(crate) fn enclosing_app_bundle(exe: &std::path::Path) -> Option { + exe.ancestors() + .skip(1) + .find(|a| a.extension() == Some(std::ffi::OsStr::new("app"))) + .map(std::path::Path::to_path_buf) +} + +/// Whether `exe` is running from a macOS App Translocation mount, i.e. a quarantined copy executing +/// from a read-only randomized path such as +/// `/private/var/folders/../AppTranslocation//d/MyApp.app`. +/// +/// Pure and path-lexical, matching on the `AppTranslocation` path component. +#[cfg_attr(not(target_os = "macos"), allow(dead_code))] +pub(crate) fn is_translocated(exe: &std::path::Path) -> bool { + exe.components() + .any(|c| c.as_os_str() == std::ffi::OsStr::new("AppTranslocation")) +} + /// Map an IO error from an install-step write into the crate error, always naming the install path. /// /// A `PermissionDenied` becomes [`Error::InstallPathNotWritable`] carrying the path; any other kind @@ -1592,6 +1857,39 @@ fn map_install_io_error(e: std::io::Error, bin_install_path: &std::path::Path) - } } +/// Dispatch the opt-in preflight probe to the path the install step will actually write: the +/// bundle's parent directory in bundle mode (the swap needs create+rename permission there, not on +/// the bundle itself), otherwise `bin_install_path`. +pub(crate) fn probe_writable( + bin_install_path: &std::path::Path, + bundle_install_path: Option<&std::path::Path>, +) -> Result<()> { + match bundle_install_path { + Some(bundle) => probe_dir_writable(install_parent(bundle)), + None => probe_install_path_writable(bin_install_path), + } +} + +/// Best-effort preflight probe of whether a directory accepts new entries, by creating and +/// immediately dropping a temporary file in it. Conservative in the same way as +/// [`probe_install_path_writable`]: only a definite +/// [`PermissionDenied`](std::io::ErrorKind::PermissionDenied) fails, naming the directory probed. +pub(crate) fn probe_dir_writable(dir: &std::path::Path) -> Result<()> { + match tempfile::Builder::new() + .prefix(".self_update_writecheck") + .tempfile_in(dir) + { + Ok(_) => Ok(()), + Err(e) if e.kind() == std::io::ErrorKind::PermissionDenied => { + Err(Error::InstallPathNotWritable { + path: dir.to_path_buf(), + }) + } + // Indeterminate (missing dir, weird fs, etc.): proceed and let the real op surface it. + Err(_) => Ok(()), + } +} + /// Best-effort preflight probe of whether `bin_install_path` can be written, run before any /// download when `check_install_path_writable` is set. /// @@ -2931,6 +3229,494 @@ mod tests { .expect("a missing parent dir is indeterminate and must probe Ok"); } + // --- bundle mode (BNDL-*) ------------------------------------------------------------------- + + // Build a staged bundle tree with an executable at `Contents/MacOS/` plus one resource, + // returning the bundle root. Mirrors the shape of a macOS `.app`. + fn staged_bundle( + parent: &std::path::Path, + name: &str, + exe: &str, + marker: &[u8], + ) -> std::path::PathBuf { + let root = parent.join(name); + let macos = root.join("Contents").join("MacOS"); + std::fs::create_dir_all(&macos).unwrap(); + std::fs::write(macos.join(exe), marker).unwrap(); + std::fs::write(root.join("Contents").join("Info.plist"), marker).unwrap(); + root + } + + // BNDL-2-5: a fresh install (nothing at the destination yet) renames the staged tree into + // place, contents intact. + #[test] + fn swap_bundle_installs_when_nothing_is_there() { + let dir = tempfile::tempdir().unwrap(); + let staging = dir.path().join("staging"); + std::fs::create_dir(&staging).unwrap(); + let staged = staged_bundle(&staging, "MyApp.app", "myapp", b"new"); + let stash = dir.path().join("stash"); + std::fs::create_dir(&stash).unwrap(); + let dest = dir.path().join("MyApp.app"); + let outside_exe = dir.path().join("updater"); + + super::swap_bundle(&staged, &dest, &stash, &outside_exe, None) + .expect("a fresh bundle install must work"); + + assert!(dest.is_dir(), "the bundle must be installed at the dest"); + assert_eq!( + std::fs::read(dest.join("Contents").join("MacOS").join("myapp")).unwrap(), + b"new", + "the installed tree must be the staged one" + ); + assert!(!staged.exists(), "the staged tree is moved, not copied"); + } + + // BNDL-2-5/BNDL-5-2: replacing an existing bundle swaps the whole tree, so a file that the new + // bundle does not carry is gone afterwards (no merge with the old contents). + #[test] + fn swap_bundle_replaces_the_whole_tree() { + let dir = tempfile::tempdir().unwrap(); + let staging = dir.path().join("staging"); + std::fs::create_dir(&staging).unwrap(); + let staged = staged_bundle(&staging, "MyApp.app", "myapp", b"new"); + let stash = dir.path().join("stash"); + std::fs::create_dir(&stash).unwrap(); + let dest = staged_bundle(dir.path(), "MyApp.app", "myapp", b"old"); + let stale = dest.join("Contents").join("stale-resource"); + std::fs::write(&stale, b"stale").unwrap(); + let outside_exe = dir.path().join("updater"); + + super::swap_bundle(&staged, &dest, &stash, &outside_exe, None) + .expect("replacing a bundle must work"); + + assert_eq!( + std::fs::read(dest.join("Contents").join("MacOS").join("myapp")).unwrap(), + b"new", + "the exe must come from the new tree" + ); + assert!( + !stale.exists(), + "a whole-tree swap must not leave stale files from the old bundle" + ); + } + + // BNDL-2-3: a staged root that the archive did not carry (missing, or a file rather than a + // directory) is an error, and the installed bundle is left untouched. + #[test] + fn swap_bundle_rejects_a_missing_or_non_directory_staged_root() { + let dir = tempfile::tempdir().unwrap(); + let stash = dir.path().join("stash"); + std::fs::create_dir(&stash).unwrap(); + let dest = staged_bundle(dir.path(), "MyApp.app", "myapp", b"old"); + let outside_exe = dir.path().join("updater"); + + let missing = dir.path().join("staging").join("MyApp.app"); + let err = super::swap_bundle(&missing, &dest, &stash, &outside_exe, None) + .expect_err("a missing staged root must error"); + assert!( + matches!(&err, Error::Io(io) if io.kind() == std::io::ErrorKind::NotFound), + "expected a NotFound Io error, got {err:?}" + ); + + let as_file = dir.path().join("not-a-dir"); + std::fs::write(&as_file, b"file").unwrap(); + super::swap_bundle(&as_file, &dest, &stash, &outside_exe, None) + .expect_err("a staged root that is a file must error"); + + assert_eq!( + std::fs::read(dest.join("Contents").join("MacOS").join("myapp")).unwrap(), + b"old", + "the installed bundle must be untouched when the staged tree is unusable" + ); + } + + // BNDL-5-2: when the swap's final rename fails, the stashed old tree is restored, so the + // destination is left with its original contents and the original error surfaces. + #[test] + fn swap_bundle_rolls_back_when_the_install_rename_fails() { + let dir = tempfile::tempdir().unwrap(); + let staging = dir.path().join("staging"); + std::fs::create_dir(&staging).unwrap(); + let staged = staged_bundle(&staging, "MyApp.app", "myapp", b"new"); + let stash = dir.path().join("stash"); + std::fs::create_dir(&stash).unwrap(); + let dest = staged_bundle(dir.path(), "MyApp.app", "myapp", b"old"); + + // Inject a failure between the stash and the install: a verify hook is the only in-process + // seam that runs before the renames, so instead remove the staged tree through a hook, + // making the final `rename(staged -> dest)` fail with NotFound after the old tree is + // stashed. + let sabotage: Box = { + let staged = staged.clone(); + Box::new(move |_: &std::path::Path| { + std::fs::remove_dir_all(&staged).unwrap(); + Ok(()) + }) + }; + let err = super::swap_bundle( + &staged, + &dest, + &stash, + &dir.path().join("updater"), + Some(&*sabotage), + ) + .expect_err("a failed install rename must error"); + assert!( + matches!(&err, Error::Io(io) if io.kind() == std::io::ErrorKind::NotFound), + "the original rename error must surface, got {err:?}" + ); + + assert!(dest.is_dir(), "the old bundle must be restored"); + assert_eq!( + std::fs::read(dest.join("Contents").join("MacOS").join("myapp")).unwrap(), + b"old", + "rollback must restore the old tree byte-for-byte" + ); + } + + // BNDL-2-5: with the running executable inside the bundle it is renamed aside before the + // directory swap, and after a successful swap its path holds the NEW bundle's executable. + #[test] + fn swap_bundle_moves_the_running_exe_aside_and_restores_its_path() { + let dir = tempfile::tempdir().unwrap(); + let staging = dir.path().join("staging"); + std::fs::create_dir(&staging).unwrap(); + let staged = staged_bundle(&staging, "MyApp.app", "myapp", b"new"); + let stash = dir.path().join("stash"); + std::fs::create_dir(&stash).unwrap(); + let dest = staged_bundle(dir.path(), "MyApp.app", "myapp", b"old"); + let running_exe = dest.join("Contents").join("MacOS").join("myapp"); + + super::swap_bundle(&staged, &dest, &stash, &running_exe, None) + .expect("an in-bundle swap must work"); + + assert_eq!( + std::fs::read(&running_exe).unwrap(), + b"new", + "the running exe's path must hold the new bundle's executable" + ); + assert!( + stash.join("exe-aside").exists(), + "the old running image must be stashed aside, not left in the installed tree" + ); + } + + // BNDL-5-2: a failed swap with the running exe inside the bundle restores BOTH the old tree and + // the executable at its original path, so the process is left able to keep running. + #[test] + fn swap_bundle_rollback_restores_the_running_exe_inside_the_old_tree() { + let dir = tempfile::tempdir().unwrap(); + let staging = dir.path().join("staging"); + std::fs::create_dir(&staging).unwrap(); + let staged = staged_bundle(&staging, "MyApp.app", "myapp", b"new"); + let stash = dir.path().join("stash"); + std::fs::create_dir(&stash).unwrap(); + let dest = staged_bundle(dir.path(), "MyApp.app", "myapp", b"old"); + let running_exe = dest.join("Contents").join("MacOS").join("myapp"); + + let sabotage: Box = { + let staged = staged.clone(); + Box::new(move |_: &std::path::Path| { + std::fs::remove_dir_all(&staged).unwrap(); + Ok(()) + }) + }; + super::swap_bundle(&staged, &dest, &stash, &running_exe, Some(&*sabotage)) + .expect_err("a failed install rename must error"); + + assert_eq!( + std::fs::read(&running_exe).unwrap(), + b"old", + "rollback must put the running executable back inside the restored tree" + ); + assert!( + !stash.join("exe-aside").exists(), + "the stashed executable must have been moved back, not left in the stash" + ); + } + + // BNDL-2-4/BNDL-5-2: the verify hook receives the staged BUNDLE ROOT (a directory), and a + // rejection aborts before anything is renamed. + #[test] + fn swap_bundle_verifies_the_staged_root_and_a_rejection_replaces_nothing() { + let dir = tempfile::tempdir().unwrap(); + let staging = dir.path().join("staging"); + std::fs::create_dir(&staging).unwrap(); + let staged = staged_bundle(&staging, "MyApp.app", "myapp", b"new"); + let stash = dir.path().join("stash"); + std::fs::create_dir(&stash).unwrap(); + let dest = staged_bundle(dir.path(), "MyApp.app", "myapp", b"old"); + + let seen = std::sync::Arc::new(std::sync::Mutex::new(None::)); + let reject: Box = { + let seen = std::sync::Arc::clone(&seen); + Box::new(move |p: &std::path::Path| { + *seen.lock().unwrap() = Some(p.to_path_buf()); + Err(Error::verification_rejected("bundle is not signed")) + }) + }; + let err = super::swap_bundle( + &staged, + &dest, + &stash, + &dir.path().join("updater"), + Some(&*reject), + ) + .expect_err("a rejecting hook must abort the swap"); + match err { + Error::VerificationRejected { reason } => assert_eq!( + reason.as_deref(), + Some("bundle is not signed"), + "the hook's reason must pass through unwrapped" + ), + other => panic!("expected VerificationRejected, got {other:?}"), + } + assert_eq!( + seen.lock().unwrap().as_deref(), + Some(staged.as_path()), + "the hook must receive the staged bundle root, not a file inside it" + ); + assert_eq!( + std::fs::read(dest.join("Contents").join("MacOS").join("myapp")).unwrap(), + b"old", + "a rejected verification must leave the installed bundle in place" + ); + assert!(staged.is_dir(), "the staged tree is left for the caller"); + } + + // BNDL-1-6: when the running executable is inside the installed bundle, the staged tree must + // carry a file at the same relative path; otherwise the swap errors before touching anything + // (rather than leaving the process's own path missing). + #[test] + fn swap_bundle_requires_the_staged_tree_to_carry_the_running_exe_path() { + let dir = tempfile::tempdir().unwrap(); + let stash = dir.path().join("stash"); + std::fs::create_dir(&stash).unwrap(); + let dest = staged_bundle(dir.path(), "MyApp.app", "myapp", b"old"); + let running_exe = dest.join("Contents").join("MacOS").join("myapp"); + + // A staged tree whose executable sits at a different relative path than the running one. + let staging = dir.path().join("staging"); + std::fs::create_dir(&staging).unwrap(); + let staged = staged_bundle(&staging, "MyApp.app", "renamed", b"new"); + + let err = super::swap_bundle(&staged, &dest, &stash, &running_exe, None) + .expect_err("a staged tree missing the running exe path must error"); + let msg = err.to_string(); + assert!( + msg.contains("Contents/MacOS/myapp") || msg.contains("Contents\\MacOS\\myapp"), + "the error must name the missing relative path, got: {msg}" + ); + assert_eq!( + std::fs::read(&running_exe).unwrap(), + b"old", + "the installed bundle must be untouched by the rejected swap" + ); + } + + // `exe_inside_bundle` reports the exe's path relative to the bundle, resolving symlinks on both + // sides (mirroring `same_file`), and `None` when the exe is outside. + #[test] + fn exe_inside_bundle_detects_containment_through_symlinks() { + let dir = tempfile::tempdir().unwrap(); + let bundle = staged_bundle(dir.path(), "MyApp.app", "myapp", b"exe"); + let exe = bundle.join("Contents").join("MacOS").join("myapp"); + + let (found, rel) = super::exe_inside_bundle(&exe, &bundle) + .expect("an exe inside the bundle must be detected"); + assert_eq!(rel, std::path::Path::new("Contents/MacOS/myapp")); + assert_eq!(found, std::fs::canonicalize(&exe).unwrap()); + + let outside = dir.path().join("elsewhere"); + std::fs::write(&outside, b"exe").unwrap(); + assert!( + super::exe_inside_bundle(&outside, &bundle).is_none(), + "an exe outside the bundle must not be detected as inside" + ); + + assert!( + super::exe_inside_bundle(&bundle, &bundle).is_none(), + "the bundle root itself is not an exe inside the bundle" + ); + + #[cfg(unix)] + { + // A symlinked view of the bundle still matches: both sides canonicalize. + let link = dir.path().join("Linked.app"); + std::os::unix::fs::symlink(&bundle, &link).unwrap(); + let (_, rel) = super::exe_inside_bundle(&exe, &link) + .expect("a symlinked bundle path must still match"); + assert_eq!(rel, std::path::Path::new("Contents/MacOS/myapp")); + } + } + + // BNDL-1-3: the macOS default install path is the NEAREST `.app` ancestor of the exe, and + // `None` when there is none. Pure and path-lexical, so it runs on every platform. + #[test] + fn enclosing_app_bundle_finds_the_nearest_app_ancestor() { + let nested = std::path::Path::new("/Applications/Outer.app/Contents/MacOS/Inner.app/x/exe"); + assert_eq!( + super::enclosing_app_bundle(nested).unwrap(), + std::path::Path::new("/Applications/Outer.app/Contents/MacOS/Inner.app"), + "the innermost .app ancestor wins" + ); + assert_eq!( + super::enclosing_app_bundle(std::path::Path::new( + "/Applications/MyApp.app/Contents/MacOS/myapp" + )) + .unwrap(), + std::path::Path::new("/Applications/MyApp.app") + ); + assert_eq!( + super::enclosing_app_bundle(std::path::Path::new("/usr/local/bin/myapp")), + None, + "an exe with no .app ancestor has no default bundle path" + ); + assert_eq!( + super::enclosing_app_bundle(std::path::Path::new("/Applications/MyApp.app")), + None, + "the .app itself is not its own ancestor" + ); + } + + // BNDL-5-3: a translocated (quarantined) app is detected from its path component. + #[test] + fn is_translocated_matches_the_translocation_mount() { + assert!(super::is_translocated(std::path::Path::new( + "/private/var/folders/x1/T/AppTranslocation/1E4-UUID/d/MyApp.app/Contents/MacOS/myapp" + ))); + assert!( + !super::is_translocated(std::path::Path::new( + "/Applications/MyApp.app/Contents/MacOS/myapp" + )), + "an installed app is not translocated" + ); + assert!( + !super::is_translocated(std::path::Path::new( + "/Applications/AppTranslocationHelper.app/Contents/MacOS/x" + )), + "the match is on a whole path component, not a substring" + ); + } + + // BNDL-2-2/BNDL-2-3: the whole install step over a real archive -- extract beside the + // destination, then swap -- preserving the executable bit and the framework-style symlinks an + // `.app` needs. Staging happens in the destination's parent, so nothing else in the temp dir + // survives the call. + #[cfg(all(feature = "archive-zip", unix))] + #[test] + fn install_bundle_extracts_and_swaps_a_real_archive() { + use std::io::Write as _; + use std::os::unix::fs::PermissionsExt as _; + + let dir = tempfile::tempdir().unwrap(); + let archive_path = dir.path().join("MyApp.app.zip"); + { + let f = std::fs::File::create(&archive_path).unwrap(); + let mut zip = zip::ZipWriter::new(f); + let exe_opts = zip::write::SimpleFileOptions::default() + .compression_method(zip::CompressionMethod::Stored) + .unix_permissions(0o755); + zip.start_file("MyApp.app/Contents/MacOS/myapp", exe_opts) + .unwrap(); + zip.write_all(b"#!/bin/sh\necho new\n").unwrap(); + let opts = zip::write::SimpleFileOptions::default() + .compression_method(zip::CompressionMethod::Stored); + zip.start_file("MyApp.app/Contents/Resources/data.txt", opts) + .unwrap(); + zip.write_all(b"payload").unwrap(); + // A relative symlink, as a bundled framework's `Versions/Current` would be. + zip.add_symlink("MyApp.app/Contents/Current", "Resources", opts) + .unwrap(); + zip.finish().unwrap(); + } + + // An older bundle already installed, with a file the new one does not carry. + let install_root = dir.path().join("Applications"); + std::fs::create_dir(&install_root).unwrap(); + let dest = staged_bundle(&install_root, "MyApp.app", "myapp", b"old"); + std::fs::write(dest.join("Contents").join("gone.txt"), b"stale").unwrap(); + + super::install_bundle(&archive_path, "MyApp.app", &dest, None, false) + .expect("installing a bundle from an archive must work"); + + let installed_exe = dest.join("Contents").join("MacOS").join("myapp"); + assert_eq!( + std::fs::read(&installed_exe).unwrap(), + b"#!/bin/sh\necho new\n", + "the installed exe must come from the archive" + ); + assert!( + std::fs::metadata(&installed_exe) + .unwrap() + .permissions() + .mode() + & 0o111 + != 0, + "the archived executable bit must survive the install" + ); + assert!( + std::fs::symlink_metadata(dest.join("Contents").join("Current")) + .unwrap() + .file_type() + .is_symlink(), + "a bundled symlink must be installed as a symlink" + ); + assert!( + !dest.join("Contents").join("gone.txt").exists(), + "the swap replaces the whole tree" + ); + // Staging and stash were temp dirs inside the install parent; both are cleaned up. + let leftovers: Vec<_> = std::fs::read_dir(&install_root) + .unwrap() + .map(|e| e.unwrap().file_name()) + .filter(|n| n != "MyApp.app") + .collect(); + assert!( + leftovers.is_empty(), + "staging/stash dirs must not be left behind, found: {leftovers:?}" + ); + } + + // BNDL-3-1: in bundle mode the preflight probes the bundle's PARENT (the directory the swap + // needs create+rename permission in), naming that parent when it is read-only. + #[cfg(unix)] + #[test] + fn probe_writable_probes_the_bundle_parent_in_bundle_mode() { + use std::os::unix::fs::PermissionsExt; + let dir = tempfile::tempdir().unwrap(); + let ro_dir = dir.path().join("Applications"); + std::fs::create_dir(&ro_dir).unwrap(); + let bundle = ro_dir.join("MyApp.app"); + std::fs::create_dir(&bundle).unwrap(); + std::fs::set_permissions(&ro_dir, std::fs::Permissions::from_mode(0o555)).unwrap(); + + let res = + super::probe_writable(std::path::Path::new("/definitely/not/used"), Some(&bundle)); + std::fs::set_permissions(&ro_dir, std::fs::Permissions::from_mode(0o755)).unwrap(); + + match res { + Err(Error::InstallPathNotWritable { path }) => assert_eq!( + path, ro_dir, + "bundle mode must name the bundle's parent directory" + ), + other => panic!("expected InstallPathNotWritable, got {other:?}"), + } + } + + // The same dispatch returns `Ok(())` for a writable bundle parent, and falls back to the + // single-file probe when bundle mode is off. + #[test] + fn probe_writable_falls_back_to_the_bin_path_without_bundle_mode() { + let dir = tempfile::tempdir().unwrap(); + let bundle = dir.path().join("MyApp.app"); + super::probe_writable(std::path::Path::new("/unused"), Some(&bundle)) + .expect("a writable bundle parent must probe Ok"); + super::probe_writable(&dir.path().join("app"), None) + .expect("without bundle mode the bin install path is probed"); + } + // Build a custom-backend `Update` carrying `checksum`, to drive `finish_update` directly. #[cfg(feature = "checksums")] fn update_with_checksum(checksum: crate::Checksum) -> crate::backends::custom::Update { @@ -3970,6 +4756,8 @@ mod tests { target: "x86_64-unknown-linux-gnu".to_string(), bin_name: "app".to_string(), bin_path_in_archive: bin_path_in_archive.to_string(), + bundle_path_in_archive: None, + bundle_install_path: None, show_output: false, verify_callback: None, #[cfg(feature = "checksums")] From 38096493530e9e84b829cb547f4dabb79ed9f0a6 Mon Sep 17 00:00:00 2001 From: James Kominick Date: Sun, 26 Jul 2026 19:52:55 -0400 Subject: [PATCH 4/8] test: cover the bundle-mode finish tail, staging failures, and tar.gz bundles Adds the paths the first cut left untested: - the finish tail in bundle mode end to end, including a nested path with all three `{{ .. }}` templates, and the `is_safe_asset_name` traversal guard against a malicious release version substituted into the bundle path - `swap_bundle` failures at the exe-aside and stash-old steps, and a hook error that is not already a `VerificationRejected` - `install_bundle` over a `.tar.gz` bundle (exec bit and symlink target), an unwritable install parent, and an archive carrying no bundle directory, asserting no staging or stash residue on the error paths - `install_parent` for bare, nested, and absolute names; `probe_dir_writable` for a missing directory; `exe_inside_bundle`'s lexical fallback and its component-wise containment (`MyApp.app.bak` is outside `MyApp.app`) - `FinishCtx` carrying the bundle fields on both the sync and async paths - the setters and accessors on a real backend builder, `bin_name` still being required in bundle mode, and the conflict being reported before the install path is resolved --- src/backends/common.rs | 76 +++++ src/backends/github.rs | 74 +++++ src/update.rs | 617 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 767 insertions(+) diff --git a/src/backends/common.rs b/src/backends/common.rs index 587d879..7aed849 100644 --- a/src/backends/common.rs +++ b/src/backends/common.rs @@ -1004,6 +1004,82 @@ mod tests { } } + // BNDL-1-3: `bundle_path_in_archive` alone selects bundle mode -- `bundle_install_path` does + // not. Set on its own it is inert: the built config stays single-file (both bundle fields + // `None`, so the pipeline never takes the swap branch) and an explicit `bin_install_path` + // alongside it is therefore NOT a conflict. + #[test] + fn build_ignores_bundle_install_path_without_the_archive_path() { + let cfg = CommonBuilderConfig { + bundle_install_path: Some(PathBuf::from("/Applications/MyApp.app")), + bin_install_path: Some(PathBuf::from("/usr/local/bin/app")), + ..bundle_base() + }; + let built = cfg + .build() + .expect("bundle_install_path alone must not select bundle mode"); + assert!(built.bundle_path_in_archive.is_none()); + assert!( + built.bundle_install_path.is_none(), + "an install path with no bundle mode must not reach the built config" + ); + assert_eq!(built.bin_install_path, PathBuf::from("/usr/local/bin/app")); + } + + // BNDL-1-5: bundle mode does not relax the shared required fields -- `current_version` and + // `bin_name` (which names the asset and feeds `{{ bin }}`) are still required. + #[test] + fn build_still_requires_current_version_and_bin_name_in_bundle_mode() { + let no_version = CommonBuilderConfig { + bundle_path_in_archive: Some("MyApp.app".to_string()), + bundle_install_path: Some(PathBuf::from("/Applications/MyApp.app")), + current_version: None, + ..bundle_base() + }; + match no_version.build() { + Err(crate::errors::Error::MissingField { field }) => { + assert_eq!(field, "current_version"); + } + other => panic!("expected MissingField(current_version), got {other:?}"), + } + + let no_bin_name = CommonBuilderConfig { + bundle_path_in_archive: Some("MyApp.app".to_string()), + bundle_install_path: Some(PathBuf::from("/Applications/MyApp.app")), + bin_name: None, + // No `bin_name` setter call means no auto-derived archive path either. + bin_path_in_archive: None, + bin_path_in_archive_auto: false, + ..bundle_base() + }; + match no_bin_name.build() { + Err(crate::errors::Error::MissingField { field }) => { + assert_eq!(field, "bin_name"); + } + other => panic!("expected MissingField(bin_name), got {other:?}"), + } + } + + // BNDL-1-4: the conflict is reported before the install path is resolved, so a caller who set + // both gets the actionable "these two setters conflict" error rather than a + // missing/undetectable-`bundle_install_path` error from the default resolution (which on macOS + // would even consult `current_exe()` first). + #[test] + fn build_reports_the_conflict_before_resolving_the_install_path() { + let cfg = CommonBuilderConfig { + bundle_path_in_archive: Some("MyApp.app".to_string()), + bin_install_path: Some(PathBuf::from("/usr/local/bin/app")), + ..bundle_base() + }; + match cfg.build() { + Err(crate::errors::Error::ConflictingConfig { field, conflict }) => { + assert_eq!(field, "bundle_path_in_archive"); + assert_eq!(conflict, "bin_install_path"); + } + other => panic!("expected ConflictingConfig, got {other:?}"), + } + } + // --- Item 5: self-fixing error messages -------------------------------------------------- #[test] diff --git a/src/backends/github.rs b/src/backends/github.rs index 3315799..8a40fee 100644 --- a/src/backends/github.rs +++ b/src/backends/github.rs @@ -1937,6 +1937,80 @@ mod tests { assert!(upd.verify_checksum().is_some()); } + // BNDL-1-1/BNDL-1-2: the bundle setters are part of the shared builder surface, so they exist + // on a real backend's builder and their values reach the built `Update`'s accessors. Bundle + // mode is off by default, in which case both accessors are `None` (the trait defaults). + #[test] + fn builder_stores_bundle_paths() { + let plain = super::Update::configure() + .repo_owner("o") + .repo_name("r") + .bin_name("app") + .current_version("0.1.0") + .build() + .unwrap(); + assert!( + plain.bundle_path_in_archive().is_none() && plain.bundle_install_path().is_none(), + "bundle mode must be off unless bundle_path_in_archive is set" + ); + + let bundled = super::Update::configure() + .repo_owner("o") + .repo_name("r") + .bin_name("app") + .current_version("0.1.0") + .bundle_path_in_archive("{{ bin }}-{{ version }}/MyApp.app") + .bundle_install_path("/Applications/MyApp.app") + .build() + .unwrap(); + assert_eq!( + bundled.bundle_path_in_archive(), + Some("{{ bin }}-{{ version }}/MyApp.app") + ); + assert_eq!( + bundled.bundle_install_path(), + Some(std::path::Path::new("/Applications/MyApp.app")) + ); + } + + // BNDL-1-4: bundle mode combined with an explicit single-file setter is rejected by `build()` + // (naming both sides) rather than silently dropping one and installing to the wrong path. The + // value `bin_name` auto-derives is not an explicit call, so it never conflicts -- but an + // explicit `bin_path_in_archive` does, even when it repeats the auto-derived value. + #[test] + fn builder_rejects_bundle_mode_combined_with_the_single_file_setters() { + let conflict = |res: crate::errors::Result| match res { + Err(crate::errors::Error::ConflictingConfig { field, conflict }) => { + assert_eq!(field, "bundle_path_in_archive"); + conflict + } + other => panic!("expected ConflictingConfig, got {:?}", other.map(|_| ())), + }; + + let res = super::Update::configure() + .repo_owner("o") + .repo_name("r") + .bin_name("app") + .current_version("0.1.0") + .bundle_path_in_archive("MyApp.app") + .bundle_install_path("/Applications/MyApp.app") + .bin_install_path("/usr/local/bin/app") + .build(); + assert_eq!(conflict(res), "bin_install_path"); + + let res = super::Update::configure() + .repo_owner("o") + .repo_name("r") + .bin_name("app") + .current_version("0.1.0") + .bundle_path_in_archive("MyApp.app") + .bundle_install_path("/Applications/MyApp.app") + // Even repeating what `bin_name` already derived is an explicit call, and conflicts. + .bin_path_in_archive("app") + .build(); + assert_eq!(conflict(res), "bin_path_in_archive"); + } + #[test] fn builder_stores_asset_matcher() { let upd = super::Update::configure() diff --git a/src/update.rs b/src/update.rs index a98484d..a87d011 100644 --- a/src/update.rs +++ b/src/update.rs @@ -3717,6 +3717,623 @@ mod tests { .expect("without bundle mode the bin install path is probed"); } + // --- bundle mode: gaps closed by the coverage pass (BNDL-*) --------------------------------- + + // BNDL-2-2: `install_parent` picks the directory the staging and stash temp dirs are created + // in. A bare relative bundle name has an *empty* parent component, which must resolve to the + // current directory -- `TempDir::new_in("")` fails, so an empty parent would break the swap for + // a relative `bundle_install_path`. + #[test] + fn install_parent_resolves_a_bare_name_to_the_current_dir() { + assert_eq!( + super::install_parent(std::path::Path::new("MyApp.app")), + std::path::Path::new("."), + "a bare bundle name must stage in the current directory" + ); + assert_eq!( + super::install_parent(std::path::Path::new("/Applications/MyApp.app")), + std::path::Path::new("/Applications") + ); + assert_eq!( + super::install_parent(std::path::Path::new("sub/MyApp.app")), + std::path::Path::new("sub"), + "a relative nested path keeps its real parent" + ); + } + + // BNDL-3-1: the bundle-parent probe is best-effort in exactly the way the single-file probe is: + // only a definite permission refusal fails. A directory that does not exist is indeterminate, + // so the update proceeds and the real install step surfaces the outcome. + #[test] + fn probe_dir_writable_treats_a_missing_dir_as_indeterminate() { + let dir = tempfile::tempdir().unwrap(); + super::probe_dir_writable(&dir.path().join("nope").join("deeper")) + .expect("a missing directory is indeterminate and must probe Ok"); + super::probe_dir_writable(dir.path()).expect("a writable directory must probe Ok"); + } + + // BNDL-5-2/BNDL-3-2: a failure at the stash step (step 2 of the swap) happens before anything + // under the install path has changed, so the installed bundle is left exactly as it was, the + // ORIGINAL io error surfaces, and it names the install path. Injected by pointing the swap at a + // stash directory that does not exist. + #[test] + fn swap_bundle_leaves_the_bundle_intact_when_the_stash_rename_fails() { + let dir = tempfile::tempdir().unwrap(); + let staging = dir.path().join("staging"); + std::fs::create_dir(&staging).unwrap(); + let staged = staged_bundle(&staging, "MyApp.app", "myapp", b"new"); + let missing_stash = dir.path().join("no-such-stash"); + let dest = staged_bundle(dir.path(), "MyApp.app", "myapp", b"old"); + + let err = super::swap_bundle( + &staged, + &dest, + &missing_stash, + &dir.path().join("updater"), + None, + ) + .expect_err("an unusable stash must fail the swap"); + assert!( + matches!(&err, Error::Io(io) if io.kind() == std::io::ErrorKind::NotFound), + "the original rename error (and its kind) must surface, got {err:?}" + ); + assert!( + err.to_string().contains(&dest.display().to_string()), + "the install error must name the bundle install path, got: {err}" + ); + assert_eq!( + std::fs::read(dest.join("Contents").join("MacOS").join("myapp")).unwrap(), + b"old", + "nothing may change before the stash rename succeeds" + ); + assert!( + staged.is_dir(), + "the staged tree must be left for the caller when the swap never started" + ); + } + + // BNDL-5-2: the very first rename -- the running executable moved aside (step 1) -- can fail + // too. Nothing has moved at that point, so both the installed bundle and the running + // executable's path are untouched, and the error names the install path. + #[test] + fn swap_bundle_reports_a_failed_exe_aside_with_nothing_moved() { + let dir = tempfile::tempdir().unwrap(); + let staging = dir.path().join("staging"); + std::fs::create_dir(&staging).unwrap(); + let staged = staged_bundle(&staging, "MyApp.app", "myapp", b"new"); + let missing_stash = dir.path().join("no-such-stash"); + let dest = staged_bundle(dir.path(), "MyApp.app", "myapp", b"old"); + let running_exe = dest.join("Contents").join("MacOS").join("myapp"); + + let err = super::swap_bundle(&staged, &dest, &missing_stash, &running_exe, None) + .expect_err("an unusable stash must fail the exe-aside rename"); + assert!( + err.to_string().contains(&dest.display().to_string()), + "the install error must name the bundle install path, got: {err}" + ); + assert_eq!( + std::fs::read(&running_exe).unwrap(), + b"old", + "the running executable must stay in place when the swap never started" + ); + assert!(dest.is_dir(), "the installed bundle must be untouched"); + } + + // BNDL-2-4: a hook failing with an ordinary error (not an explicit rejection) still aborts the + // swap; its message becomes the `VerificationRejected` reason and nothing is renamed. The + // rejection arm is covered above -- this is the wrap-any-other-error arm, in bundle mode. + #[test] + fn swap_bundle_wraps_a_hook_error_as_a_rejection_and_replaces_nothing() { + let dir = tempfile::tempdir().unwrap(); + let staging = dir.path().join("staging"); + std::fs::create_dir(&staging).unwrap(); + let staged = staged_bundle(&staging, "MyApp.app", "myapp", b"new"); + let stash = dir.path().join("stash"); + std::fs::create_dir(&stash).unwrap(); + let dest = staged_bundle(dir.path(), "MyApp.app", "myapp", b"old"); + + let hook: Box = Box::new(|_: &std::path::Path| { + Err(Error::Io(std::io::Error::other("codesign hook blew up"))) + }); + let err = super::swap_bundle( + &staged, + &dest, + &stash, + &dir.path().join("updater"), + Some(&*hook), + ) + .expect_err("a failing hook must abort the swap"); + match err { + Error::VerificationRejected { reason } => assert!( + reason + .as_deref() + .is_some_and(|r| r.contains("codesign hook blew up")), + "the hook's error message must become the rejection reason, got {reason:?}" + ), + other => panic!("expected VerificationRejected, got {other:?}"), + } + assert_eq!( + std::fs::read(dest.join("Contents").join("MacOS").join("myapp")).unwrap(), + b"old", + "a failed verification must leave the installed bundle in place" + ); + } + + // BNDL-2-5: containment is decided even when neither path exists yet -- a fresh install has no + // destination to canonicalize, so the comparison falls back to the lexical paths -- and it is + // component-wise: a sibling whose name merely starts with the bundle's name is outside it. + #[test] + fn exe_inside_bundle_falls_back_to_a_lexical_comparison() { + let base = std::path::Path::new("/no/such/root"); + let bundle = base.join("MyApp.app"); + let exe = bundle.join("Contents").join("MacOS").join("myapp"); + + let (found, rel) = super::exe_inside_bundle(&exe, &bundle) + .expect("paths that cannot be canonicalized must still compare lexically"); + assert_eq!(rel, std::path::Path::new("Contents/MacOS/myapp")); + assert_eq!(found, exe, "the un-canonicalizable exe path is used as-is"); + assert!( + super::exe_inside_bundle(&base.join("MyApp.app.bak").join("myapp"), &bundle).is_none(), + "a sibling sharing the bundle's name prefix is not inside the bundle" + ); + } + + // BNDL-2-5: a `bundle_install_path` that is a symlink to the real bundle still ends up + // resolving to the NEW tree after the swap -- the guarantee a caller who configured that path + // depends on. (Whether the link itself survives the swap is deliberately not asserted here: + // that detail is not fixed by the committed spec.) + #[cfg(unix)] + #[test] + fn swap_bundle_through_a_symlinked_install_path_installs_the_new_tree() { + let dir = tempfile::tempdir().unwrap(); + let staging = dir.path().join("staging"); + std::fs::create_dir(&staging).unwrap(); + let staged = staged_bundle(&staging, "MyApp.app", "myapp", b"new"); + let stash = dir.path().join("stash"); + std::fs::create_dir(&stash).unwrap(); + + // The real bundle lives elsewhere; the configured install path is a symlink to it. + let real_root = dir.path().join("real"); + std::fs::create_dir(&real_root).unwrap(); + let real = staged_bundle(&real_root, "MyApp.app", "myapp", b"old"); + let link = dir.path().join("MyApp.app"); + std::os::unix::fs::symlink(&real, &link).unwrap(); + + super::swap_bundle(&staged, &link, &stash, &dir.path().join("updater"), None) + .expect("a symlinked install path must swap"); + + assert_eq!( + std::fs::read(link.join("Contents").join("MacOS").join("myapp")).unwrap(), + b"new", + "the configured install path must resolve to the new bundle after the swap" + ); + } + + // BNDL-2-2: the staging/stash dirs are created inside the destination's parent, so an + // unwritable parent fails there -- before the archive is even opened -- as + // `InstallPathNotWritable` naming the bundle install path. + #[cfg(unix)] + #[test] + fn install_bundle_reports_an_unwritable_install_parent() { + use std::os::unix::fs::PermissionsExt as _; + + let dir = tempfile::tempdir().unwrap(); + let ro_dir = dir.path().join("Applications"); + std::fs::create_dir(&ro_dir).unwrap(); + let dest = ro_dir.join("MyApp.app"); + std::fs::set_permissions(&ro_dir, std::fs::Permissions::from_mode(0o555)).unwrap(); + + // The archive is never read: creating the staging dir fails first. + let res = super::install_bundle( + &dir.path().join("never-read.zip"), + "MyApp.app", + &dest, + None, + false, + ); + std::fs::set_permissions(&ro_dir, std::fs::Permissions::from_mode(0o755)).unwrap(); + + match res { + Err(Error::InstallPathNotWritable { path }) => assert_eq!( + path, dest, + "the staging failure must name the bundle install path" + ), + other => panic!("expected InstallPathNotWritable, got {other:?}"), + } + } + + // BNDL-2-3: an archive that does not carry the configured bundle directory is an error naming + // the missing staged root, with the installed bundle untouched and no staging/stash residue + // left beside it. + #[cfg(feature = "archive-zip")] + #[test] + fn install_bundle_errors_when_the_archive_has_no_bundle_directory() { + use std::io::Write as _; + + let dir = tempfile::tempdir().unwrap(); + let archive_path = dir.path().join("release.zip"); + { + let f = std::fs::File::create(&archive_path).unwrap(); + let mut zip = zip::ZipWriter::new(f); + let opts = zip::write::SimpleFileOptions::default() + .compression_method(zip::CompressionMethod::Stored); + zip.start_file("some-other-dir/readme.txt", opts).unwrap(); + zip.write_all(b"not a bundle").unwrap(); + zip.finish().unwrap(); + } + + let install_root = dir.path().join("Applications"); + std::fs::create_dir(&install_root).unwrap(); + let dest = staged_bundle(&install_root, "MyApp.app", "myapp", b"old"); + + let err = super::install_bundle(&archive_path, "MyApp.app", &dest, None, false) + .expect_err("an archive without the bundle directory must error"); + assert!( + matches!(&err, Error::Io(io) if io.kind() == std::io::ErrorKind::NotFound), + "expected a NotFound Io error, got {err:?}" + ); + assert!( + err.to_string().contains("MyApp.app"), + "the error must name the bundle directory it looked for, got: {err}" + ); + assert_eq!( + std::fs::read(dest.join("Contents").join("MacOS").join("myapp")).unwrap(), + b"old", + "the installed bundle must be untouched" + ); + let leftovers: Vec<_> = std::fs::read_dir(&install_root) + .unwrap() + .map(|e| e.unwrap().file_name()) + .filter(|n| n != "MyApp.app") + .collect(); + assert!( + leftovers.is_empty(), + "the staging/stash dirs must be cleaned up on the error path too, found: {leftovers:?}" + ); + } + + // BNDL-4-1/BNDL-4-2: the same install over a **tar.gz** bundle archive (the zip case is + // covered above). `tar`'s unpack carries the executable bit and restores symlinks, both of + // which a macOS `.app` (framework `Versions/Current` links, signed executables) depends on. + #[cfg(all(feature = "compression-tar-gz", unix))] + #[test] + fn install_bundle_extracts_and_swaps_a_tar_gz_archive() { + use std::os::unix::fs::PermissionsExt as _; + + let dir = tempfile::tempdir().unwrap(); + let archive_path = dir.path().join("MyApp.app.tar.gz"); + { + let mut ar = tar::Builder::new(Vec::new()); + + let exe = b"#!/bin/sh\necho new\n"; + let mut header = tar::Header::new_gnu(); + header.set_entry_type(tar::EntryType::Regular); + header.set_mode(0o755); + header.set_size(exe.len() as u64); + ar.append_data(&mut header, "MyApp.app/Contents/MacOS/myapp", &exe[..]) + .unwrap(); + + let data = b"payload"; + let mut header = tar::Header::new_gnu(); + header.set_entry_type(tar::EntryType::Regular); + header.set_mode(0o644); + header.set_size(data.len() as u64); + ar.append_data( + &mut header, + "MyApp.app/Contents/Resources/data.txt", + &data[..], + ) + .unwrap(); + + // A relative symlink, as a bundled framework's `Versions/Current` would be. + let mut header = tar::Header::new_gnu(); + header.set_entry_type(tar::EntryType::Symlink); + header.set_mode(0o777); + header.set_size(0); + ar.append_link(&mut header, "MyApp.app/Contents/Current", "Resources") + .unwrap(); + + let tarred = ar.into_inner().unwrap(); + let out = std::fs::File::create(&archive_path).unwrap(); + let mut gz = flate2::write::GzEncoder::new(out, flate2::Compression::default()); + std::io::copy(&mut tarred.as_slice(), &mut gz).unwrap(); + gz.finish().unwrap(); + } + + let install_root = dir.path().join("Applications"); + std::fs::create_dir(&install_root).unwrap(); + let dest = staged_bundle(&install_root, "MyApp.app", "myapp", b"old"); + std::fs::write(dest.join("Contents").join("gone.txt"), b"stale").unwrap(); + + super::install_bundle(&archive_path, "MyApp.app", &dest, None, false) + .expect("installing a bundle from a tar.gz must work"); + + let installed_exe = dest.join("Contents").join("MacOS").join("myapp"); + assert_eq!( + std::fs::read(&installed_exe).unwrap(), + b"#!/bin/sh\necho new\n", + "the installed exe must come from the tar.gz" + ); + assert!( + std::fs::metadata(&installed_exe) + .unwrap() + .permissions() + .mode() + & 0o111 + != 0, + "the archived executable bit must survive a tar.gz install" + ); + let link = dest.join("Contents").join("Current"); + assert!( + std::fs::symlink_metadata(&link) + .unwrap() + .file_type() + .is_symlink(), + "a tar symlink entry must be installed as a symlink" + ); + assert_eq!( + std::fs::read_link(&link).unwrap(), + std::path::Path::new("Resources"), + "the symlink must still point at its original relative target" + ); + assert!( + !dest.join("Contents").join("gone.txt").exists(), + "the swap replaces the whole tree" + ); + let leftovers: Vec<_> = std::fs::read_dir(&install_root) + .unwrap() + .map(|e| e.unwrap().file_name()) + .filter(|n| n != "MyApp.app") + .collect(); + assert!( + leftovers.is_empty(), + "staging/stash dirs must not be left behind, found: {leftovers:?}" + ); + } + + // A bundle-mode [`FinishCtx`], built on the same shape as `traversal_ctx` (which the + // substitution-guard tests use) with the bundle fields filled in, so the finish tail takes the + // `install_bundle` branch. + fn bundle_ctx( + bundle_path_in_archive: &str, + version: &str, + bundle_install_path: &std::path::Path, + ) -> super::FinishCtx { + let mut ctx = traversal_ctx("unused-bin-path", version); + ctx.bundle_path_in_archive = Some(bundle_path_in_archive.to_string()); + ctx.bundle_install_path = Some(bundle_install_path.to_path_buf()); + ctx + } + + // BNDL-1-1/BNDL-2-3: the finish tail end to end in bundle mode, with all three `{{ .. }}` + // templates inside a *nested* bundle path. The substitution is shared with the single-file + // path, so this pins that bundle mode reads it (and reads `bundle_path_in_archive`, not + // `bin_path_in_archive`), installs the nested directory, and reports the release. + #[cfg(feature = "archive-zip")] + #[test] + fn finish_update_owned_installs_a_templated_nested_bundle_path() { + use std::io::Write as _; + + let install_dir = tempfile::tempdir().unwrap(); + let dest = install_dir.path().join("MyApp.app"); + + let archive_dir = tempfile::tempdir().unwrap(); + let archive_path = archive_dir.path().join("release.zip"); + { + let f = std::fs::File::create(&archive_path).unwrap(); + let mut zip = zip::ZipWriter::new(f); + let opts = zip::write::SimpleFileOptions::default() + .compression_method(zip::CompressionMethod::Stored); + zip.start_file( + "myapp-1.2.3-x86_64-unknown-linux-gnu/MyApp.app/Contents/MacOS/myapp", + opts, + ) + .unwrap(); + zip.write_all(b"new-exe").unwrap(); + zip.start_file( + "myapp-1.2.3-x86_64-unknown-linux-gnu/MyApp.app/Contents/Info.plist", + opts, + ) + .unwrap(); + zip.write_all(b"plist").unwrap(); + zip.finish().unwrap(); + } + + let mut ctx = bundle_ctx( + "{{ bin }}-{{ version }}-{{ target }}/MyApp.app", + "1.2.3", + &dest, + ); + ctx.bin_name = "myapp".to_string(); + // A path that only the single-file branch would ever write to. + let never_written = install_dir.path().join("single-file-install"); + ctx.bin_install_path = never_written.clone(); + + let status = super::finish_update_owned(ctx, archive_dir, &archive_path) + .expect("a bundle-mode finish must install the bundle"); + assert!(status.is_updated(), "bundle mode must report an update"); + assert_eq!( + status.version(), + Some("1.2.3"), + "the installed release must be reported" + ); + assert_eq!( + std::fs::read(dest.join("Contents").join("MacOS").join("myapp")).unwrap(), + b"new-exe", + "the templated nested bundle directory must be installed at the bundle install path" + ); + assert!( + dest.join("Contents").join("Info.plist").exists(), + "the whole bundle tree must be installed, not just the executable" + ); + assert!( + !never_written.exists(), + "bundle mode must not run the single-file install" + ); + let leftovers: Vec<_> = std::fs::read_dir(install_dir.path()) + .unwrap() + .map(|e| e.unwrap().file_name()) + .filter(|n| n != "MyApp.app") + .collect(); + assert!( + leftovers.is_empty(), + "staging/stash dirs must not be left beside the installed bundle, found: {leftovers:?}" + ); + } + + // BNDL-1-1 (traversal defense): the `is_safe_asset_name` guard covers a value substituted into + // the BUNDLE path too, so a malicious release version cannot redirect the swap's source outside + // the staging dir. The guard fires before the archive is read (it does not even exist here), + // and the installed bundle is untouched. + #[test] + fn finish_update_rejects_traversal_in_a_substituted_bundle_path() { + let install_dir = tempfile::tempdir().unwrap(); + let dest = staged_bundle(install_dir.path(), "MyApp.app", "myapp", b"old"); + let archive_dir = tempfile::tempdir().unwrap(); + let archive = archive_dir.path().join("release.zip"); + + let ctx = bundle_ctx("{{ version }}/MyApp.app", "../evil", &dest); + match super::finish_update_owned(ctx, archive_dir, &archive) { + Err(Error::InvalidAssetName { name }) => { + assert_eq!(name, "../evil", "the offending component must be named"); + } + other => panic!("expected InvalidAssetName for a traversal version, got {other:?}"), + } + assert_eq!( + std::fs::read(dest.join("Contents").join("MacOS").join("myapp")).unwrap(), + b"old", + "a rejected substitution must not touch the installed bundle" + ); + } + + // A clonable release source offering one newer release with an asset for the target the bundle + // tests configure, so the same source can drive the sync updater, the async updater (the async + // adapter requires `Clone`), and a full `update_extended` run up to the preflight. + #[derive(Clone)] + struct BundleSource; + impl BundleSource { + fn release(version: &str) -> Result { + Release::builder() + .version(version) + .asset(crate::update::ReleaseAsset::new( + "myapp-x86_64-unknown-linux-gnu.zip", + // Unroutable on purpose: reaching the download at all is a test failure. + "http://127.0.0.1:1/myapp-x86_64-unknown-linux-gnu.zip", + )) + .build() + } + } + impl crate::update::ReleaseSource for BundleSource { + fn get_latest_release(&self) -> Result { + Self::release("1.2.3") + } + fn get_releases(&self) -> Result> { + Ok(vec![Self::release("1.2.3")?]) + } + fn get_release_version(&self, v: &str) -> Result { + Self::release(v) + } + } + + // BNDL-3-1: the opt-in preflight through a real updater in bundle mode. It probes the BUNDLE'S + // PARENT (not `bin_install_path`, which defaults to the running test binary and is writable) + // and bails before anything is downloaded -- the asset URL is unroutable, so a preflight that + // failed to fire would surface a transport error instead. + #[cfg(unix)] + #[test] + fn update_extended_preflight_rejects_an_unwritable_bundle_parent() { + use std::os::unix::fs::PermissionsExt as _; + + let dir = tempfile::tempdir().unwrap(); + let ro_dir = dir.path().join("Applications"); + std::fs::create_dir(&ro_dir).unwrap(); + let bundle = ro_dir.join("MyApp.app"); + std::fs::set_permissions(&ro_dir, std::fs::Permissions::from_mode(0o555)).unwrap(); + + let upd = crate::backends::custom::Update::configure() + .source(BundleSource) + .bin_name("myapp") + .target("x86_64-unknown-linux-gnu") + .current_version("1.0.0") + .bundle_path_in_archive("MyApp.app") + .bundle_install_path(&bundle) + .check_install_path_writable(true) + .no_confirm(true) + .show_output(false) + .build() + .unwrap(); + let res = upd.update_extended(); + std::fs::set_permissions(&ro_dir, std::fs::Permissions::from_mode(0o755)).unwrap(); + + match res { + Err(Error::InstallPathNotWritable { path }) => assert_eq!( + path, ro_dir, + "the preflight must name the unwritable bundle parent" + ), + other => panic!("expected InstallPathNotWritable from the preflight, got {other:?}"), + } + } + + // BNDL-1-7: the bundle fields ride through `FinishCtx`, which is what the sync tail and the + // async tail (inside `spawn_blocking`) both capture -- so bundle mode behaves identically on + // both. Without the setters the captured context stays in single-file mode. + #[test] + fn finish_ctx_captures_the_bundle_fields() { + let asset = crate::update::ReleaseAsset::new("release.zip", "https://host/release.zip"); + + let bundled = crate::backends::custom::Update::configure() + .source(BundleSource) + .bin_name("myapp") + .target("x86_64-unknown-linux-gnu") + .current_version("1.0.0") + .bundle_path_in_archive("MyApp.app") + .bundle_install_path("/Applications/MyApp.app") + .build() + .unwrap(); + let ctx = super::FinishCtx::capture(&bundled, rel("1.2.3"), &asset); + assert_eq!(ctx.bundle_path_in_archive.as_deref(), Some("MyApp.app")); + assert_eq!( + ctx.bundle_install_path.as_deref(), + Some(std::path::Path::new("/Applications/MyApp.app")) + ); + + let plain = crate::backends::custom::Update::configure() + .source(BundleSource) + .bin_name("myapp") + .target("x86_64-unknown-linux-gnu") + .current_version("1.0.0") + .build() + .unwrap(); + let ctx = super::FinishCtx::capture(&plain, rel("1.2.3"), &asset); + assert!( + ctx.bundle_path_in_archive.is_none() && ctx.bundle_install_path.is_none(), + "without the setters the finish tail must stay in single-file mode" + ); + } + + // BNDL-1-7 (async lane): an updater built with `build_async` carries the same bundle fields + // into the shared `FinishCtx`, so `update_extended_async` installs bundles like the sync verb. + #[cfg(feature = "async")] + #[test] + fn async_update_captures_the_bundle_fields_too() { + let upd = crate::backends::custom::AsyncUpdate::configure() + .source(crate::backends::custom::Blocking::new(BundleSource)) + .bin_name("myapp") + .target("x86_64-unknown-linux-gnu") + .current_version("1.0.0") + .bundle_path_in_archive("MyApp.app") + .bundle_install_path("/Applications/MyApp.app") + .build_async() + .unwrap(); + let asset = crate::update::ReleaseAsset::new("release.zip", "https://host/release.zip"); + let ctx = super::FinishCtx::capture(&upd, rel("1.2.3"), &asset); + assert_eq!(ctx.bundle_path_in_archive.as_deref(), Some("MyApp.app")); + assert_eq!( + ctx.bundle_install_path.as_deref(), + Some(std::path::Path::new("/Applications/MyApp.app")) + ); + } + // Build a custom-backend `Update` carrying `checksum`, to drive `finish_update` directly. #[cfg(feature = "checksums")] fn update_with_checksum(checksum: crate::Checksum) -> crate::backends::custom::Update { From 6090cf8ed87585404e0fdb5155ad9216925f723f Mon Sep 17 00:00:00 2001 From: James Kominick Date: Sun, 26 Jul 2026 19:59:34 -0400 Subject: [PATCH 5/8] fix: replace the tree behind a symlinked `bundle_install_path`, not the link `rename` does not follow a path's final component, so a symlinked install path was stashed as a link and replaced by a real directory, leaving the installed bundle orphaned on disk with no error. `resolve_bundle_target` now maps a live symlink to the tree it designates before the swap, so that tree is what gets replaced, the link survives, and staging is still created beside the real tree (keeping every rename same-filesystem). Also in bundle mode: - Test for an existing destination with `symlink_metadata` instead of `exists()`, so a dangling symlink is stashed and replaced rather than being renamed onto (which fails with ENOTDIR). - Match the `.app` extension case-insensitively, since macOS's default filesystem preserves case without distinguishing it. - Reject `bundle_install_path` set without `bundle_path_in_archive` as `Error::MissingField` instead of silently discarding the install path. - Name the bundle path in the confirmation block ("Current bundle:") and say the bundle directory will be replaced, since `bin_install_path` is never written. - Drop `bin_install_path` from the `InstallPathNotWritable` docs and message: the same variant now also carries a bundle path, or its parent from the preflight. --- CHANGELOG.md | 5 +- README.md | 8 +- specs/bundle-install.md | 34 ++++++-- specs/ref-update-pipeline.md | 24 ++++-- src/backends/common.rs | 37 +++++---- src/errors.rs | 20 +++-- src/lib.rs | 8 +- src/macros.rs | 10 ++- src/update.rs | 156 +++++++++++++++++++++++++++++++++-- 9 files changed, 248 insertions(+), 54 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d9eea5..2d6cef0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,10 @@ `check_install_path_writable` preflight probes the bundle's parent directory. Adds `Error::NoAppBundle` (no `.app` ancestor to derive the path from), `Error::ConflictingConfig` (bundle mode combined with an explicit `bin_install_path` / `bin_path_in_archive`), and - `Error::AppTranslocated` (a quarantined app running from a read-only translocated mount). + `Error::AppTranslocated` (a quarantined app running from a read-only translocated mount). A + symlinked `bundle_install_path` is resolved first, so the tree behind the link is replaced and the + link survives; `bundle_install_path` without `bundle_path_in_archive` is a `MissingField` error + rather than a silently discarded path. ([#145](https://github.com/jaemk/self_update/issues/145)) - `compression-tar-xz` feature: decode `.tar.xz` / `.txz` archives and plain `.xz` single-file assets (pure-Rust `lzma-rs`, no C `liblzma` dependency, so it cross-compiles like the rest of the diff --git a/README.md b/README.md index 0ea95c1..9532d12 100644 --- a/README.md +++ b/README.md @@ -268,7 +268,8 @@ How the swap works, and what it guarantees: - The archive is extracted in full into a temporary directory **inside the install path's parent**, so every rename is on one filesystem (there is no cross-device fallback, and the parent needs - room for one more copy of the bundle). + room for one more copy of the bundle). A symlinked `bundle_install_path` is resolved first, so the + tree behind the link is replaced, the link survives, and staging still lands beside the real tree. - The installed tree is stashed, then the staged tree is renamed into place. A failure at any step restores the original bundle, and the error names the bundle path. Once the final rename lands the update is committed. @@ -277,8 +278,9 @@ How the swap works, and what it guarantees: bundle's executable, and the process can relaunch itself with `restart()` (see [Restarting after an update](#restarting-after-an-update)). - Bundle mode replaces a directory, so combining it with an explicit `bin_install_path` or - `bin_path_in_archive` is rejected by `build()` (`Error::ConflictingConfig`). `bin_name` is still - required: it selects the asset and feeds `{{ bin }}`. + `bin_path_in_archive` is rejected by `build()` (`Error::ConflictingConfig`), and setting + `bundle_install_path` without `bundle_path_in_archive` is an `Error::MissingField` rather than a + silently discarded path. `bin_name` is still required: it selects the asset and feeds `{{ bin }}`. - The `verify_binary` hook receives the **staged bundle root**, which is what `codesign --verify --deep` wants; a rejection aborts before anything is replaced. - The crate never signs, notarizes, or staples: ship an already-signed (and, for Gatekeeper, diff --git a/specs/bundle-install.md b/specs/bundle-install.md index b15f547..6bae09c 100644 --- a/specs/bundle-install.md +++ b/specs/bundle-install.md @@ -63,7 +63,11 @@ substitution and `is_safe_asset_name` traversal defense as `bin_path_in_archive` BNDL-1-2. `bundle_install_path(path: impl AsRef) -> &mut Self` names the installed bundle directory to replace (e.g. `/Applications/MyApp.app`). -BNDL-1-3. Setting `bundle_path_in_archive` selects bundle mode. Default +BNDL-1-3. Setting `bundle_path_in_archive` selects bundle mode; `bundle_install_path` +on its own does not, and is `Error::MissingField { field: "bundle_path_in_archive" }` +rather than a silently discarded install path. The `.app` suffix is matched +case-insensitively, since macOS's default filesystem preserves case without +distinguishing it. Default `bundle_install_path` on macOS: the nearest ancestor of `std::env::current_exe()` whose file name ends in `.app`. Resolution happens in `build()`; no `.app` ancestor and no explicit path => a config error naming the @@ -203,13 +207,27 @@ quarantine) before updating. Without the check the failure surfaces as a bare read-only-filesystem IO error from mid-swap, on the most common first-run-after-download path on macOS. -BNDL-5-2. Rollback guarantee: before step 2 of BNDL-2-5 nothing under -`bundle_install_path` has changed. A failure at step 2 or 3 restores the old -tree (and the exe-aside) via reverse renames. After step 3 succeeds the update -is committed. The guarantees match `MoveAll`: all-or-nothing at rename -granularity, original error surfaced, best-effort logged rollback. The bundle -swap adds on top of `MoveAll`: whole-tree granularity (one rename each way, so -no per-file partial window) and the exe-aside step for running-image safety. +BNDL-5-2. Rollback guarantee: every pre-swap check (staged root present and a +directory, staged tree carries the running exe's relative path, `verify_binary`) +runs before any rename, so a rejection there leaves `bundle_install_path` +byte-for-byte untouched. Step 1 does move one file out of the bundle (the +running exe, when it is inside), and a failure at step 2 or 3 restores the old +tree and that exe via reverse renames. After step 3 succeeds the update is +committed. The guarantees match `MoveAll`: all-or-nothing at rename granularity, +original error surfaced, best-effort logged rollback. The bundle swap adds on top +of `MoveAll`: whole-tree granularity (one rename each way, so no per-file partial +window) and the exe-aside step for running-image safety. + +BNDL-5-4. A symlinked `bundle_install_path` is resolved to its real path before +the swap, so the installed tree behind the link is what gets replaced and the +link itself survives; staging follows the resolved path's parent, keeping every +rename same-filesystem. A dangling symlink at the path counts as an existing +entry and is stashed and replaced rather than being renamed onto. + +BNDL-5-5. Concurrency is not coordinated: the existence check and the renames +are not atomic as a unit, so two updaters racing on one bundle can interleave and +each report success while only one tree survives. Single-writer is assumed, as it +is for the single-file `Move` / `self_replace` path. ## Non-goals diff --git a/specs/ref-update-pipeline.md b/specs/ref-update-pipeline.md index 8c41fba..fe11c63 100644 --- a/specs/ref-update-pipeline.md +++ b/specs/ref-update-pipeline.md @@ -165,7 +165,9 @@ preflight probe (`check_install_path_writable`). `bundle_path_in_archive()` being `Some` selects bundle mode, resolved at `build()` time by `CommonBuilderConfig::resolve_bundle_mode` (`backends/common.rs:638`): an explicit `bin_install_path` or a non-auto `bin_path_in_archive` alongside it is -`Error::ConflictingConfig { field, conflict }`, and an unset `bundle_install_path` resolves via +`Error::ConflictingConfig { field, conflict }`; a `bundle_install_path` set *without* +`bundle_path_in_archive` is `Error::MissingField { field: "bundle_path_in_archive" }` rather than a +silently discarded path; and an unset `bundle_install_path` resolves via `default_bundle_install_path` (`update.rs:1801`) -- on macOS the nearest `.app` ancestor of `current_exe()` (`enclosing_app_bundle`, `update.rs:1825`), with a translocated exe (`is_translocated`, `update.rs:1838`) rejected as `Error::AppTranslocated` and no `.app` ancestor as @@ -173,10 +175,13 @@ preflight probe (`check_install_path_writable`). In the finish tail the same `{{ bin }}` / `{{ target }}` / `{{ version }}` substitution runs over the bundle path, then `install_bundle` (`update.rs:1630`) replaces the single-file -extract-and-install pair: two `tempfile::TempDir`s (staging and stash) are created inside -`install_parent(bundle_install_path)`, so every rename is same-filesystem and there is no -cross-device case; `Extract::extract_into` unpacks the whole archive into staging; the staged root -is `staging/`. Failure to create either temp dir goes through +extract-and-install pair: the configured path is first run through `resolve_bundle_target`, which +maps a live symlink to the tree it designates (`rename` does not follow a path's final component, so +swapping onto the link itself would stash the link and orphan the installed tree; a dangling link and +a plain path pass through unchanged); two `tempfile::TempDir`s (staging and stash) are created inside +`install_parent()`, so every rename is same-filesystem and there is no cross-device +case; `Extract::extract_into` unpacks the whole archive into staging; the staged root is +`staging/`. Failure to create either temp dir goes through `map_install_io_error` naming the bundle path. `swap_bundle` (`update.rs:1683`) performs the swap, taking the running exe as a parameter (so it is @@ -197,7 +202,14 @@ affects the installed tree. The swap is one code path on all targets: a windows open files (a loaded DLL) fails at the directory rename and rolls back. Output messages in bundle mode are "Extracting archive... Done" then "Replacing bundle directory... -Done"; `ReleaseStatus` / `VersionStatus` reporting is unchanged. +Done"; the confirmation block names the bundle path ("Current bundle:") and says the existing bundle +directory will be replaced, since `bin_install_path` is never written in bundle mode. +`ReleaseStatus` / `VersionStatus` reporting is unchanged. + +Existence at the destination is tested with `fs::symlink_metadata`, not `exists()`: a dangling +symlink is an entry that must be stashed out of the way (renaming a directory onto one fails with +`ENOTDIR`), where `exists()` would report it absent. Concurrency is not coordinated: the existence +test and the renames are not atomic as a unit, so racing updaters can interleave. ### Multi-file install diff --git a/src/backends/common.rs b/src/backends/common.rs index 7aed849..46f3a4a 100644 --- a/src/backends/common.rs +++ b/src/backends/common.rs @@ -637,6 +637,14 @@ impl CommonBuilderConfig { /// nearest `.app` ancestor); every other platform requires it. fn resolve_bundle_mode(&self) -> Result<(Option, Option)> { let Some(path_in_archive) = self.bundle_path_in_archive.clone() else { + // `bundle_install_path` alone does not select bundle mode, and silently installing a + // single file to the default path instead is the same footgun the conflict check below + // exists to prevent, so say which setter is missing. + if self.bundle_install_path.is_some() { + return Err(Error::MissingField { + field: "bundle_path_in_archive", + }); + } return Ok((None, None)); }; if self.bin_install_path.is_some() { @@ -1004,26 +1012,27 @@ mod tests { } } - // BNDL-1-3: `bundle_path_in_archive` alone selects bundle mode -- `bundle_install_path` does - // not. Set on its own it is inert: the built config stays single-file (both bundle fields - // `None`, so the pipeline never takes the swap branch) and an explicit `bin_install_path` - // alongside it is therefore NOT a conflict. + // BNDL-1-3: `bundle_path_in_archive` is what selects bundle mode, so `bundle_install_path` set + // on its own is a missing-field error naming the setter that is absent. Installing a single file + // to the default path instead would silently discard the caller's install path, the same footgun + // the bin/bundle conflict check prevents from the other direction. #[test] - fn build_ignores_bundle_install_path_without_the_archive_path() { + fn build_rejects_a_bundle_install_path_without_the_archive_path() { let cfg = CommonBuilderConfig { bundle_install_path: Some(PathBuf::from("/Applications/MyApp.app")), - bin_install_path: Some(PathBuf::from("/usr/local/bin/app")), ..bundle_base() }; - let built = cfg - .build() - .expect("bundle_install_path alone must not select bundle mode"); + match cfg.build() { + Err(crate::errors::Error::MissingField { field }) => { + assert_eq!(field, "bundle_path_in_archive"); + } + other => panic!("expected MissingField, got {other:?}"), + } + + // Neither bundle setter: plain single-file mode, both bundle fields unset. + let built = bundle_base().build().expect("single-file mode must build"); assert!(built.bundle_path_in_archive.is_none()); - assert!( - built.bundle_install_path.is_none(), - "an install path with no bundle mode must not reach the built config" - ); - assert_eq!(built.bin_install_path, PathBuf::from("/usr/local/bin/app")); + assert!(built.bundle_install_path.is_none()); } // BNDL-1-5: bundle mode does not relax the shared required fields -- `current_version` and diff --git a/src/errors.rs b/src/errors.rs index 6e1e35d..06c598f 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -125,16 +125,18 @@ pub enum Error { /// The name of the missing required field. field: &'static str, }, - /// The binary's install path (or its parent directory) is not writable by this process. + /// The install path (or the directory it lives in) is not writable by this process. /// /// Returned either by the opt-in preflight writability check /// (`check_install_path_writable(true)`), which probes before any download, or by the install - /// step itself when the replace/move fails with a permission error. `path` is the configured - /// `bin_install_path`. Re-run with elevated privileges, or choose a user-writable - /// `bin_install_path`; the crate never escalates privileges itself. + /// step itself when the replace/move fails with a permission error. `path` is the path that + /// could not be written: the configured `bin_install_path`, or in bundle mode the + /// `bundle_install_path` (the preflight names its parent directory, which is where the swap + /// needs permission). Re-run with elevated privileges, or configure a user-writable install + /// path; the crate never escalates privileges itself. #[non_exhaustive] InstallPathNotWritable { - /// The install path (`bin_install_path`) that could not be written. + /// The path that could not be written. path: std::path::PathBuf, }, /// Bundle mode is on but the bundle install path could not be derived: the running executable @@ -458,7 +460,7 @@ impl std::fmt::Display for Error { InstallPathNotWritable { path } => write!( f, "InstallPathNotWritableError: cannot write to install path {}: run with elevated \ - privileges or choose a user-writable bin_install_path", + privileges or configure a user-writable install path", path.display() ), NoAppBundle { exe } => write!( @@ -1322,10 +1324,12 @@ mod tests { shown.contains("/usr/local/bin/app"), "InstallPathNotWritable Display must name the path, got: {shown}" ); + // The remedy is named without naming a specific setter: the same variant covers + // `bin_install_path` and, in bundle mode, `bundle_install_path` (or its parent). assert!( - shown.contains("elevated privileges") && shown.contains("bin_install_path"), + shown.contains("elevated privileges") && shown.contains("user-writable install path"), "InstallPathNotWritable Display must suggest elevated privileges or a user-writable \ - bin_install_path, got: {shown}" + install path, got: {shown}" ); assert!( err.source().is_none(), diff --git a/src/lib.rs b/src/lib.rs index b7e1576..c3d8b64 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -276,7 +276,8 @@ How the swap works, and what it guarantees: - The archive is extracted in full into a temporary directory **inside the install path's parent**, so every rename is on one filesystem (there is no cross-device fallback, and the parent needs - room for one more copy of the bundle). + room for one more copy of the bundle). A symlinked `bundle_install_path` is resolved first, so the + tree behind the link is replaced, the link survives, and staging still lands beside the real tree. - The installed tree is stashed, then the staged tree is renamed into place. A failure at any step restores the original bundle, and the error names the bundle path. Once the final rename lands the update is committed. @@ -285,8 +286,9 @@ How the swap works, and what it guarantees: bundle's executable, and the process can relaunch itself with `restart()` (see [Restarting after an update](#restarting-after-an-update)). - Bundle mode replaces a directory, so combining it with an explicit `bin_install_path` or - `bin_path_in_archive` is rejected by `build()` (`Error::ConflictingConfig`). `bin_name` is still - required: it selects the asset and feeds `{{ bin }}`. + `bin_path_in_archive` is rejected by `build()` (`Error::ConflictingConfig`), and setting + `bundle_install_path` without `bundle_path_in_archive` is an `Error::MissingField` rather than a + silently discarded path. `bin_name` is still required: it selects the asset and feeds `{{ bin }}`. - The `verify_binary` hook receives the **staged bundle root**, which is what `codesign --verify --deep` wants; a rejection aborts before anything is replaced. - The crate never signs, notarizes, or staples: ship an already-signed (and, for Gatekeeper, diff --git a/src/macros.rs b/src/macros.rs index 2aa00ad..53e9c13 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -648,8 +648,14 @@ macro_rules! impl_common_builder_setters { } /// Set the installed bundle directory that bundle mode replaces, e.g. - /// `"/Applications/MyApp.app"`. Only consulted when - /// [`bundle_path_in_archive`](Self::bundle_path_in_archive) is set. + /// `"/Applications/MyApp.app"`. Requires + /// [`bundle_path_in_archive`](Self::bundle_path_in_archive), which is what selects bundle + /// mode; setting this alone is an + /// [`Error::MissingField`](crate::errors::Error::MissingField) from `build()` rather than a + /// silently discarded install path. + /// + /// A symlinked path is resolved to the tree it points at, so the installed bundle behind the + /// link is what gets replaced and the link itself survives the update. /// /// On macOS this defaults to the nearest `.app` ancestor of the running executable, so an /// app launched from `/Applications/MyApp.app/Contents/MacOS/myapp` updates itself in diff --git a/src/update.rs b/src/update.rs index a87d011..33a6c63 100644 --- a/src/update.rs +++ b/src/update.rs @@ -1225,7 +1225,12 @@ fn resolve_and_confirm( let prompt_confirmation = !u.no_confirm(); if u.show_output() || prompt_confirmation { println!("\n{} release status:", u.bin_name()); - println!(" * Current exe: {:?}", u.bin_install_path()); + // In bundle mode the install target is the bundle directory, not `bin_install_path` (which + // the swap never writes), so confirm against the path that will actually be replaced. + match u.bundle_install_path() { + Some(bundle) => println!(" * Current bundle: {:?}", bundle), + None => println!(" * Current exe: {:?}", u.bin_install_path()), + } println!(" * New exe release: {:?}", target_asset.name()); println!( " * New exe download url: {:?}", @@ -1238,8 +1243,13 @@ fn resolve_and_confirm( println!(" * Release notes:\n{}", body); } } + let replaced = match u.bundle_install_path() { + Some(_) => "the existing bundle directory will be replaced", + None => "the existing binary will be replaced", + }; println!( - "\nThe new release will be downloaded/extracted and the existing binary will be replaced." + "\nThe new release will be downloaded/extracted and {}.", + replaced ); } if prompt_confirmation { @@ -1634,7 +1644,11 @@ fn install_bundle( verify: Option<&crate::DynVerifyFn>, show_output: bool, ) -> Result<()> { - let parent = install_parent(bundle_install_path); + // Resolve a symlinked install path to the tree it points at, so the swap replaces the installed + // bundle rather than the link (`rename` does not follow its final component) and staging lands + // beside the real tree, keeping every rename same-filesystem. + let target = resolve_bundle_target(bundle_install_path); + let parent = install_parent(&target); let staging = tempfile::TempDir::new_in(parent) .map_err(|e| map_install_io_error(e, bundle_install_path))?; let stash = tempfile::TempDir::new_in(parent) @@ -1647,7 +1661,7 @@ fn install_bundle( print_flush(show_output, "Replacing bundle directory... ")?; swap_bundle( &staged_root, - bundle_install_path, + &target, stash.path(), &std::env::current_exe()?, verify, @@ -1724,7 +1738,9 @@ fn swap_bundle( fs::rename(exe, &stashed_exe).map_err(|e| map_install_io_error(e, bundle_install_path))?; } - let old_stashed = bundle_install_path.exists(); + // `symlink_metadata` rather than `exists()`: a dangling symlink at the path is still an entry + // that must be stashed out of the way, and renaming a directory onto one fails with ENOTDIR. + let old_stashed = fs::symlink_metadata(bundle_install_path).is_ok(); if old_stashed && let Err(e) = fs::rename(bundle_install_path, &stashed_old) { if let Some(exe) = exe_aside.as_deref() { restore_stashed(&stashed_exe, exe); @@ -1779,6 +1795,29 @@ fn exe_inside_bundle( Some((exe, rel)) } +/// The bundle directory a configured install path actually designates: the symlink target when the +/// path is a symlink, else the path itself. +/// +/// `rename` does not follow a path's final component, so swapping straight onto a symlinked +/// `bundle_install_path` would stash the *link*, plant a real directory in its place, and leave the +/// installed tree orphaned on disk. Resolving first means the tree behind the link is what gets +/// replaced, the link survives the update, and staging is created beside the real tree so the swap's +/// renames stay on one filesystem. +/// +/// Only an existing symlink is resolved: a plain path (including one that does not exist yet, the +/// fresh-install case) and a dangling link are returned unchanged, the latter so the swap stashes and +/// replaces the stale entry. +fn resolve_bundle_target(bundle_install_path: &std::path::Path) -> std::path::PathBuf { + let is_symlink = fs::symlink_metadata(bundle_install_path) + .map(|m| m.file_type().is_symlink()) + .unwrap_or(false); + if !is_symlink { + return bundle_install_path.to_path_buf(); + } + // A dangling link has no target to replace; keep the configured path. + fs::canonicalize(bundle_install_path).unwrap_or_else(|_| bundle_install_path.to_path_buf()) +} + /// The directory an install path lives in: its parent, or the current directory for a bare name. fn install_parent(path: &std::path::Path) -> &std::path::Path { match path.parent() { @@ -1818,12 +1857,18 @@ pub(crate) fn default_bundle_install_path() -> Result { /// /// Pure and path-lexical (no filesystem access), so it is exercised on every platform even though /// only macOS uses it for the default install path. Innermost wins for a nested bundle (an -/// `.app` shipped inside another `.app`). +/// `.app` shipped inside another `.app`). The extension is matched case-insensitively: macOS's +/// default filesystem preserves case but does not distinguish it, so a `MyApp.App` on disk is the +/// same bundle as `MyApp.app`. #[cfg_attr(not(target_os = "macos"), allow(dead_code))] pub(crate) fn enclosing_app_bundle(exe: &std::path::Path) -> Option { exe.ancestors() .skip(1) - .find(|a| a.extension() == Some(std::ffi::OsStr::new("app"))) + .find(|a| { + a.extension() + .and_then(|e| e.to_str()) + .is_some_and(|e| e.eq_ignore_ascii_case("app")) + }) .map(std::path::Path::to_path_buf) } @@ -3578,6 +3623,16 @@ mod tests { None, "the .app itself is not its own ancestor" ); + // macOS's default filesystem preserves case without distinguishing it, so an odd-cased + // bundle directory on disk is still the same bundle. + assert_eq!( + super::enclosing_app_bundle(std::path::Path::new( + "/Applications/MyApp.App/Contents/MacOS/myapp" + )) + .unwrap(), + std::path::Path::new("/Applications/MyApp.App"), + "the .app extension must match case-insensitively" + ); } // BNDL-5-3: a translocated (quarantined) app is detected from its path component. @@ -3892,14 +3947,16 @@ mod tests { let stash = dir.path().join("stash"); std::fs::create_dir(&stash).unwrap(); - // The real bundle lives elsewhere; the configured install path is a symlink to it. + // The real bundle lives elsewhere; the configured install path is a symlink to it. The + // resolved target is what the swap is given, so the tree behind the link is replaced. let real_root = dir.path().join("real"); std::fs::create_dir(&real_root).unwrap(); let real = staged_bundle(&real_root, "MyApp.app", "myapp", b"old"); let link = dir.path().join("MyApp.app"); std::os::unix::fs::symlink(&real, &link).unwrap(); - super::swap_bundle(&staged, &link, &stash, &dir.path().join("updater"), None) + let target = super::resolve_bundle_target(&link); + super::swap_bundle(&staged, &target, &stash, &dir.path().join("updater"), None) .expect("a symlinked install path must swap"); assert_eq!( @@ -3907,6 +3964,87 @@ mod tests { b"new", "the configured install path must resolve to the new bundle after the swap" ); + assert!( + std::fs::symlink_metadata(&link) + .unwrap() + .file_type() + .is_symlink(), + "the caller's symlink must survive the update, not be replaced by a real directory" + ); + assert_eq!( + std::fs::read(real.join("Contents").join("MacOS").join("myapp")).unwrap(), + b"new", + "the tree behind the link is the one that gets replaced" + ); + } + + // BNDL-5-4: `resolve_bundle_target` maps a symlinked install path to the tree it designates, so + // the swap replaces that tree (and stages beside it) instead of renaming the link itself, which + // would orphan the installed bundle on disk. Everything else passes through unchanged. + #[test] + fn resolve_bundle_target_follows_only_a_live_symlink() { + let dir = tempfile::tempdir().unwrap(); + + // A plain directory, and a path that does not exist yet (fresh install): unchanged. + let plain = staged_bundle(dir.path(), "Plain.app", "myapp", b"x"); + assert_eq!(super::resolve_bundle_target(&plain), plain); + let missing = dir.path().join("Missing.app"); + assert_eq!(super::resolve_bundle_target(&missing), missing); + + #[cfg(unix)] + { + let real = staged_bundle(dir.path(), "Real.app", "myapp", b"x"); + let link = dir.path().join("Link.app"); + std::os::unix::fs::symlink(&real, &link).unwrap(); + assert_eq!( + super::resolve_bundle_target(&link), + std::fs::canonicalize(&real).unwrap(), + "a live symlink resolves to its target" + ); + + // A dangling link has no target to replace, so the configured path is kept and the swap + // stashes the stale entry. + let dangling = dir.path().join("Dangling.app"); + std::os::unix::fs::symlink(dir.path().join("nowhere"), &dangling).unwrap(); + assert_eq!(super::resolve_bundle_target(&dangling), dangling); + } + } + + // BNDL-5-4: a dangling symlink at the install path counts as an existing entry -- it is stashed + // and replaced. Renaming the staged directory straight onto it would fail with ENOTDIR. + #[cfg(unix)] + #[test] + fn swap_bundle_replaces_a_dangling_symlink_at_the_install_path() { + let dir = tempfile::tempdir().unwrap(); + let staging = dir.path().join("staging"); + std::fs::create_dir(&staging).unwrap(); + let staged = staged_bundle(&staging, "MyApp.app", "myapp", b"new"); + let stash = dir.path().join("stash"); + std::fs::create_dir(&stash).unwrap(); + + let dest = dir.path().join("MyApp.app"); + std::os::unix::fs::symlink(dir.path().join("gone"), &dest).unwrap(); + + let target = super::resolve_bundle_target(&dest); + super::swap_bundle(&staged, &target, &stash, &dir.path().join("updater"), None) + .expect("a dangling symlink must be replaced, not renamed onto"); + + assert!( + dest.is_dir() + && !std::fs::symlink_metadata(&dest) + .unwrap() + .file_type() + .is_symlink(), + "the stale link must be gone, replaced by the installed bundle" + ); + assert_eq!( + std::fs::read(dest.join("Contents").join("MacOS").join("myapp")).unwrap(), + b"new" + ); + assert!( + stash.join("old").exists() || std::fs::symlink_metadata(stash.join("old")).is_ok(), + "the stale link must have been stashed rather than clobbered" + ); } // BNDL-2-2: the staging/stash dirs are created inside the destination's parent, so an From 9587ddde1ee733f3f71967b9e4afe7fea328b00a Mon Sep 17 00:00:00 2001 From: James Kominick Date: Mon, 27 Jul 2026 08:58:30 -0400 Subject: [PATCH 6/8] ci: run the suite on macOS, make the bundle default-path resolution host-agnostic macOS is the target platform for directory-bundle installs but had no CI job, so the `.app` default-path resolution was never compiled, let alone run. Two changes: - Add a `macos` job (`macos-latest`, arm64) running the reqwest and ureq test lanes. Beyond the bundle swap this covers APFS's case-insensitive filenames, macOS symlink and rename semantics, and `self_replace`. None of that is architecture-dependent, so arm64 alone is enough. - Split `resolve_default_bundle_path(exe, has_default)` out of `default_bundle_install_path`, with the macOS policy carried by a `cfg!` value instead of a `#[cfg]` branch. Every arm now compiles and is tested on every host: an exe inside a bundle resolves to it, one outside is `NoAppBundle`, a translocated exe is rejected ahead of the `.app` lookup, and a target with no `.app` convention requires the setter. This also drops the `allow(dead_code)` the two path helpers needed off macOS. The manual matrix in the spec now covers only what needs a signed build or a real download: `codesign --verify`, Gatekeeper, a quarantined copy, Finder launch, and relaunch via `restart()`. --- .github/workflows/build.yml | 14 ++++++ specs/bundle-install.md | 19 +++++--- src/update.rs | 87 +++++++++++++++++++++++++++++++------ 3 files changed, 100 insertions(+), 20 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e81b679..13345c8 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -50,6 +50,20 @@ jobs: # Pin the declared MSRV: the full reqwest feature set must build on 1.88 (zip 8 requires 1.88). - run: cargo build --features "github gitlab gitea s3 archive-tar archive-zip compression-tar-gz compression-zip-deflate compression-zip-bzip2 signatures s3-auth checksums" + macos: + # macOS is the target platform for directory-bundle (`.app`) installs, so run the suite there. + # `macos-latest` is arm64 (Apple Silicon). Beyond the bundle swap this covers APFS's + # case-insensitive filenames, macOS symlink and rename semantics, and `self_replace`. None of + # that is architecture-dependent, so arm64 alone is enough. Runners are free for public repos. + runs-on: macos-latest + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + # `make` is available here, but the fmt/README lanes are already covered on linux, so run the + # test lanes directly on both clients. + - run: cargo test --features "github gitlab gitea gitee manifest s3 archive-tar archive-zip compression-tar-gz compression-tar-xz compression-zip-deflate compression-zip-bzip2 signatures s3-auth checksums" + - run: cargo test --no-default-features --features "ureq native-tls github gitlab gitea gitee manifest s3 archive-tar archive-zip compression-tar-gz compression-tar-xz compression-zip-deflate compression-zip-bzip2 signatures s3-auth checksums" + windows: runs-on: windows-latest steps: diff --git a/specs/bundle-install.md b/specs/bundle-install.md index 6bae09c..61781e7 100644 --- a/specs/bundle-install.md +++ b/specs/bundle-install.md @@ -264,12 +264,19 @@ is for the single-file `Move` / `self_replace` path. - macOS default detection: pure function over a supplied exe path (no real `.app` needed) covering `.app` ancestor found / not found / nested `.app`. - Preflight: parent-dir probe under a 0555 parent (unix), nothing downloaded. -- CI cannot exercise a real `.app` relaunch. Manual test matrix to document in - the PR: macOS (x86_64 + aarch64) with zip and tar.gz `.app` archives, - launched from Finder and from a terminal, quarantined (translocated) vs - cleared, install under `/Applications` and under `~/Applications`; - post-swap `codesign --verify` and relaunch via `restart()`; windows and - linux directory-bundle swap with the exe inside and outside the bundle. +- Default-path resolution is a pure function of `(exe, has_default)` + (`resolve_default_bundle_path`), with the macOS policy carried by a `cfg!` + value rather than a `#[cfg]` branch, so every arm compiles and is tested on + every host instead of the macOS arm being invisible to a linux run. +- The suite runs on macOS in CI (`macos-latest`, arm64), which covers the swap on + APFS: case-insensitive filenames, macOS symlink and rename semantics, and + `self_replace`. None of that is architecture-dependent, so arm64 alone is + enough and no x86_64 runner is used. That leaves as genuinely + manual only what needs a signed build or a real download: `codesign --verify` + on a signed/notarized `.app`, Gatekeeper, a quarantined (translocated) copy, + launching from Finder, and relaunch via `restart()`. Document that run in the + PR, for `.app` archives in both zip and tar.gz form, installed under + `/Applications` and under `~/Applications`. ## Design decisions (signed off 2026-07-26) diff --git a/src/update.rs b/src/update.rs index 33a6c63..3d6023a 100644 --- a/src/update.rs +++ b/src/update.rs @@ -1836,20 +1836,37 @@ fn install_parent(path: &std::path::Path) -> &std::path::Path { /// /// Every other platform has no meaningful default, so the setter is required there. pub(crate) fn default_bundle_install_path() -> Result { - #[cfg(target_os = "macos")] - { - let exe = std::env::current_exe()?; - if is_translocated(&exe) { - return Err(Error::AppTranslocated { exe }); - } - enclosing_app_bundle(&exe).ok_or(Error::NoAppBundle { exe }) - } - #[cfg(not(target_os = "macos"))] - { - Err(Error::MissingField { + resolve_default_bundle_path(&std::env::current_exe()?, HAS_APP_BUNDLE_DEFAULT) +} + +/// Whether this target derives a default `bundle_install_path` from the running executable. Only +/// macOS has the `.app` convention to derive one from. +/// +/// A `cfg!` value rather than a `#[cfg]` branch so both policies below compile, and are tested, on +/// every target: the macOS resolution is the part most worth pinning and the least reachable from CI. +const HAS_APP_BUNDLE_DEFAULT: bool = cfg!(target_os = "macos"); + +/// Resolve the default bundle install path from `exe`, for a target that `has_default`. +/// +/// Split out from [`default_bundle_install_path`] over an explicit exe path and policy flag so every +/// arm is exercisable on any host, rather than the macOS arm being invisible to a linux test run. +fn resolve_default_bundle_path( + exe: &std::path::Path, + has_default: bool, +) -> Result { + if !has_default { + return Err(Error::MissingField { field: "bundle_install_path", - }) + }); + } + if is_translocated(exe) { + return Err(Error::AppTranslocated { + exe: exe.to_path_buf(), + }); } + enclosing_app_bundle(exe).ok_or_else(|| Error::NoAppBundle { + exe: exe.to_path_buf(), + }) } /// The nearest ancestor of `exe` whose name ends in `.app`, i.e. the macOS application bundle the @@ -1860,7 +1877,6 @@ pub(crate) fn default_bundle_install_path() -> Result { /// `.app` shipped inside another `.app`). The extension is matched case-insensitively: macOS's /// default filesystem preserves case but does not distinguish it, so a `MyApp.App` on disk is the /// same bundle as `MyApp.app`. -#[cfg_attr(not(target_os = "macos"), allow(dead_code))] pub(crate) fn enclosing_app_bundle(exe: &std::path::Path) -> Option { exe.ancestors() .skip(1) @@ -1877,7 +1893,6 @@ pub(crate) fn enclosing_app_bundle(exe: &std::path::Path) -> Option/d/MyApp.app`. /// /// Pure and path-lexical, matching on the `AppTranslocation` path component. -#[cfg_attr(not(target_os = "macos"), allow(dead_code))] pub(crate) fn is_translocated(exe: &std::path::Path) -> bool { exe.components() .any(|c| c.as_os_str() == std::ffi::OsStr::new("AppTranslocation")) @@ -3635,6 +3650,50 @@ mod tests { ); } + // BNDL-1-3/BNDL-5-3: the default-path resolution as a whole, on every host. With a target that + // has the `.app` convention, a translocated exe is rejected before the `.app` lookup, an exe + // inside a bundle yields it, and one outside any bundle is `NoAppBundle`. A target without the + // convention has no default at all, so the setter is required. + #[test] + fn resolve_default_bundle_path_covers_every_arm() { + let inside = std::path::Path::new("/Applications/MyApp.app/Contents/MacOS/myapp"); + assert_eq!( + super::resolve_default_bundle_path(inside, true).unwrap(), + std::path::Path::new("/Applications/MyApp.app") + ); + + let outside = std::path::Path::new("/usr/local/bin/myapp"); + match super::resolve_default_bundle_path(outside, true) { + Err(Error::NoAppBundle { exe }) => assert_eq!(exe, outside), + other => panic!("expected NoAppBundle, got {other:?}"), + } + + // Translocation wins over the `.app` lookup: the enclosing `.app` exists here, but it is the + // read-only translocated copy, so returning it would hand back an unswappable path. + let translocated = std::path::Path::new( + "/private/var/folders/x1/T/AppTranslocation/UUID/d/MyApp.app/Contents/MacOS/myapp", + ); + match super::resolve_default_bundle_path(translocated, true) { + Err(Error::AppTranslocated { exe }) => assert_eq!(exe, translocated), + other => panic!("expected AppTranslocated, got {other:?}"), + } + + match super::resolve_default_bundle_path(inside, false) { + Err(Error::MissingField { field }) => assert_eq!(field, "bundle_install_path"), + other => panic!("expected MissingField, got {other:?}"), + } + } + + // The policy flag matches the target: only macOS derives a default install path. + #[test] + fn has_app_bundle_default_is_macos_only() { + assert_eq!( + super::HAS_APP_BUNDLE_DEFAULT, + cfg!(target_os = "macos"), + "only macOS has the `.app` convention to derive a default install path from" + ); + } + // BNDL-5-3: a translocated (quarantined) app is detected from its path component. #[test] fn is_translocated_matches_the_translocation_mount() { From b8ec34e1268b22f398c86e38a397645f0634bf69 Mon Sep 17 00:00:00 2001 From: James Kominick Date: Mon, 27 Jul 2026 06:55:38 -0400 Subject: [PATCH 7/8] ci: test `gitee`, `manifest`, and `compression-tar-xz` on windows, guard the feature lists The windows job's feature list had drifted from the Makefile's `REQWEST_FEATURES` / `UREQ_FEATURES`, so the `gitee` and `manifest` backends and `.tar.xz` decoding had never been tested on windows, and the msrv job was building a narrower set than the "full reqwest feature set" its comment claimed. - Hoist both lists into workflow-level `env`, consumed by the windows, macos, and msrv jobs (the first two cannot run `make`). - Add `make check/workflow-features`, wired into `check`, comparing those env values against the Makefile and failing with both sides printed on a mismatch. Verified by injecting a drift. - Point msrv at the same list. `cargo +1.88.0 build` with the full set passes, so the added backends do not move the declared MSRV. --- .github/workflows/build.yml | 19 ++++++++++++------- Makefile | 36 ++++++++++++++++++++++++++++++++++-- 2 files changed, 46 insertions(+), 9 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 13345c8..c1eab43 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -8,6 +8,10 @@ on: env: CARGO_TERM_COLOR: always + # The full per-client feature sets, for the jobs that cannot run `make` (the Makefile is the source + # of truth for the lanes; `make check/workflow-features` fails if these drift from it). + REQWEST_FEATURES: github gitlab gitea gitee manifest s3 archive-tar archive-zip compression-tar-gz compression-tar-xz compression-zip-deflate compression-zip-bzip2 signatures checksums s3-auth + UREQ_FEATURES: ureq native-tls github gitlab gitea gitee manifest s3 archive-tar archive-zip compression-tar-gz compression-tar-xz compression-zip-deflate compression-zip-bzip2 signatures checksums s3-auth jobs: ci: @@ -48,7 +52,7 @@ jobs: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@1.88.0 # Pin the declared MSRV: the full reqwest feature set must build on 1.88 (zip 8 requires 1.88). - - run: cargo build --features "github gitlab gitea s3 archive-tar archive-zip compression-tar-gz compression-zip-deflate compression-zip-bzip2 signatures s3-auth checksums" + - run: cargo build --features "${{ env.REQWEST_FEATURES }}" macos: # macOS is the target platform for directory-bundle (`.app`) installs, so run the suite there. @@ -61,15 +65,16 @@ jobs: - uses: dtolnay/rust-toolchain@stable # `make` is available here, but the fmt/README lanes are already covered on linux, so run the # test lanes directly on both clients. - - run: cargo test --features "github gitlab gitea gitee manifest s3 archive-tar archive-zip compression-tar-gz compression-tar-xz compression-zip-deflate compression-zip-bzip2 signatures s3-auth checksums" - - run: cargo test --no-default-features --features "ureq native-tls github gitlab gitea gitee manifest s3 archive-tar archive-zip compression-tar-gz compression-tar-xz compression-zip-deflate compression-zip-bzip2 signatures s3-auth checksums" + - run: cargo test --features "${{ env.REQWEST_FEATURES }}" + - run: cargo test --no-default-features --features "${{ env.UREQ_FEATURES }}" windows: runs-on: windows-latest steps: - uses: actions/checkout@v4 - uses: dtolnay/rust-toolchain@stable - # Exercise the Windows-sensitive paths (asset-name guard, zip extraction, self-replace) on both - # clients. `make` is not available on the Windows runner, so run the lanes directly. - - run: cargo test --features "github gitlab gitea s3 archive-tar archive-zip compression-tar-gz compression-zip-deflate compression-zip-bzip2 signatures s3-auth checksums" - - run: cargo test --no-default-features --features "ureq native-tls github gitlab gitea s3 archive-tar archive-zip compression-tar-gz compression-zip-deflate compression-zip-bzip2 signatures s3-auth checksums" + # Exercise the Windows-sensitive paths (asset-name guard, zip extraction, self-replace, the + # bundle swap's file-locking caveat) on both clients. `make` is not available on the Windows + # runner, so run the lanes directly. + - run: cargo test --features "${{ env.REQWEST_FEATURES }}" + - run: cargo test --no-default-features --features "${{ env.UREQ_FEATURES }}" diff --git a/Makefile b/Makefile index 19665ae..7af3718 100644 --- a/Makefile +++ b/Makefile @@ -40,10 +40,15 @@ EXAMPLE_TARGETS = examples $(SELF_UPDATE_EXAMPLE_TARGETS) TEST_TARGETS = tests tests/default tests/reqwest tests/ureq tests/async BUILD_TARGETS = build/all-features DOC_TARGETS = docs docs/readme -CHECK_TARGETS = check check/fmt check/readme check/clippy check/clippy/reqwest check/clippy/ureq check/clippy/async check/help +CHECK_TARGETS = check check/fmt check/readme check/clippy check/clippy/reqwest check/clippy/ureq check/clippy/async check/help check/workflow-features CLEAN_TARGETS = clean clean/cargo HELP_TARGETS = help ci $(EXAMPLE_TARGETS) $(TEST_TARGETS) $(BUILD_TARGETS) $(DOC_TARGETS) fmt $(CHECK_TARGETS) $(CLEAN_TARGETS) +# The CI workflow. The windows and macos jobs cannot run `make`, so they carry +# the per-client feature sets above as workflow env vars; +# `check/workflow-features` fails if the two copies drift apart. +WORKFLOW = .github/workflows/build.yml + # Cargo command used to run `build`, `test`, `clippy`... Useful if you keep # multiple cargo versions installed on your machine. CARGO_COMMAND = cargo @@ -89,6 +94,7 @@ help: ## List all supported Make targets check/clippy/ureq) desc="Run clippy with the full ureq feature set" ;; \ check/clippy/async) desc="Run clippy with the async API feature set" ;; \ check/help) desc="Verify the help output covers every supported target" ;; \ + check/workflow-features) desc="Verify the workflow feature lists match this Makefile" ;; \ clean) desc="Remove all generated artifacts" ;; \ clean/cargo) desc="Run cargo clean" ;; \ *) desc="" ;; \ @@ -168,7 +174,7 @@ fmt: ################################################################################ # Runs all checks. -check: check/fmt check/readme check/clippy check/help +check: check/fmt check/readme check/clippy check/help check/workflow-features # Checks that the crate is well formatted. check/fmt: FMT_CCFLAGS += --check @@ -212,6 +218,32 @@ check/help: exit 1; \ fi +# Verifies the workflow's per-client feature lists match this Makefile, which is +# the source of truth for the lanes. A stale copy silently stops testing whole +# backends on the runners that cannot use `make`. +check/workflow-features: + @echo [$@]: Checking workflow feature lists... + @fail=0; \ + for var in REQWEST_FEATURES UREQ_FEATURES; do \ + case "$$var" in \ + REQWEST_FEATURES) expected="$(REQWEST_FEATURES)" ;; \ + UREQ_FEATURES) expected="$(UREQ_FEATURES)" ;; \ + esac; \ + expected="$$(printf '%s' "$$expected" | tr -s ' ')"; \ + actual="$$(grep -E "^ $$var:" $(WORKFLOW) | head -1 | cut -d: -f2- \ + | tr -s ' ' | sed -e 's/^ *//' -e 's/ *$$//')"; \ + if [ -z "$$actual" ]; then \ + echo "$$var is missing from $(WORKFLOW)" >&2; \ + fail=1; \ + elif [ "$$expected" != "$$actual" ]; then \ + echo "$$var differs between the Makefile and $(WORKFLOW):" >&2; \ + echo " Makefile: $$expected" >&2; \ + echo " workflow: $$actual" >&2; \ + fail=1; \ + fi; \ + done; \ + exit $$fail + ################################################################################ # Cleans all generated artifacts. clean: clean/cargo From a3b14f289ace4b3fd0c4d7cb4e06e7cd047c5944 Mon Sep 17 00:00:00 2001 From: James Kominick Date: Mon, 27 Jul 2026 07:22:13 -0400 Subject: [PATCH 8/8] test: name the manifest fixture's archive entry with the platform exe suffix `bin_name("app")` appends `EXE_SUFFIX`, so the derived `bin_path_in_archive` is `app.exe` on windows, but `app_tar_gz` hardcoded an entry named `app`. Both manifest end-to-end tests failed there with "Could not find the required path in the archive". Build the entry name the same way the custom, gitea, and gitlab fixtures already do. Latent since the backend landed: the windows job's feature list did not include `manifest`, so these tests had never run on windows. --- src/backends/manifest.rs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/backends/manifest.rs b/src/backends/manifest.rs index 7fffbf8..da057c3 100644 --- a/src/backends/manifest.rs +++ b/src/backends/manifest.rs @@ -986,13 +986,16 @@ mod tests { ); } - /// Build a tiny tar.gz in memory containing a single file named `app` (the default - /// `bin_path_in_archive` on a unix target, where EXE_SUFFIX is empty). + /// Build a tiny tar.gz in memory containing the single file the updater will look for: the + /// default `bin_path_in_archive` derived from `bin_name("app")`, which carries the platform exe + /// suffix (`app.exe` on windows, `app` where EXE_SUFFIX is empty). #[cfg(all(feature = "archive-tar", feature = "compression-tar-gz"))] fn app_tar_gz(payload: &[u8]) -> Vec { let mut tar = tar::Builder::new(Vec::new()); let mut header = tar::Header::new_gnu(); - header.set_path("app").unwrap(); + header + .set_path(format!("app{}", std::env::consts::EXE_SUFFIX)) + .unwrap(); header.set_size(payload.len() as u64); header.set_mode(0o755); header.set_cksum();