mirror of
https://github.com/storytold/craft-libs.git
synced 2026-11-14 06:22:24 +00:00
0da3928eee4880139488e28ec6f444ee2dd722fc
9
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 smittie2000
|
797d21a3f3 |
craft-update: accept exactly one download link per asset (craft-launcher#764); keep the Linux self-update off other copies
Fixes storytold/craft-launcher#764 in craft-update, and with it storytold/craft-launcher#765 (each link is tied to its own release's tag). Also brings back a Linux self-update rule that the extraction lost.
## Why
craft-update was copied out of ArtCraft Launcher in #311, so its `release.rs`, `version.rs`, `platform.rs`, `distro.rs` and `proxy.rs` are the launcher's files. craft-launcher#764 was fixed only in the launcher's copy. This PR brings the fix here, where every Craft app's self-update runs. Once this PR and #389 merge, the launcher drops its copy (storytold/craft-launcher#930).
## 1. Each asset has exactly one acceptable download link (#764)
**The problem:** an asset's link could point at another release, or climb out of the app's releases with `..` or `%2e%2e`, which github.com decodes before routing.
**The fix:**
- `release::expected_download_url(repo, tag, name)` builds the only link an asset may have: `https://github.com/<repo>/releases/download/<tag>/<name>`. The tag and name may only use ASCII letters, digits and `._+-`. A `+` is written `%2B`, as GitHub writes it.
- Reading the GitHub API, an asset is kept only when its link equals that URL. This also ties each link to its own release.
- `release_from_sums` builds its links with the same function.
- `bind_artifact_version`, which compares the tag and file name inside the link with the release's, reads `%2B` there as `+`. It decodes nothing else.
- An encoded dot (`%2e`) is rejected like an encoded slash (`%2f`, `%5c`), both in links and in file names from `SHA256SUMS.txt`.
- `tag_from_redirect` refuses `.` and `..`.
- `install::check_download_link` checks the asset's link, and the link of the release's `SHA256SUMS.txt`, just before downloading. It runs in `install_at` **and** in `self_update::update`, because a self-update doesn't go through `install_at`.
## 2. Linux self-update: only `/usr/bin/<id>` counts as the package
Before #311, the launcher checked the package databases (dpkg, rpm, pacman) only for `/usr/bin/<id>`. The extraction dropped that check and added a `Standalone` fallback, which broke two cases:
- **A copy in `~/bin`** was treated as the deb/rpm/Arch package whenever that package was also installed. The update installed the package with pkexec, then relaunched the `~/bin` copy, which it never touched.
- **An AUR build in `/usr/bin`**, which the code comment says is pacman's to update, became `Standalone`. The update then tried to overwrite a file pacman owns.
`linux_location` brings the old rule back:
| Where the executable is | What happens |
|---|---|
| `/usr/bin/<id>` | checked against the package databases, as before |
| anywhere else under `/usr`, `/opt`, `/bin`, `/sbin`, `/nix`, `/snap` or `/gnu` (Guix), or in a Homebrew `.linuxbrew` prefix | no self-update: its package manager updates it |
| `/usr/local`, the home folder, anywhere else | updates itself (`Standalone`) |
**Symlinked installs are left alone on purpose.** `current_exe()` follows symlinks, so a package that puts the real file in `/opt` and a symlink in `/usr/bin` lands in the second row and doesn't self-update. That's intended. All 14 Craft apps' packages (`packaging/linux/nfpm.yaml`) put the real file at `/usr/bin/<id>`, so this only happens with someone else's repackaging, such as an AUR `-bin` package, and that copy belongs to its package manager.
## 3. Fixes from reviewing this PR with craft-launcher (3 November)
Rebased onto main (`55332dd`) instead of merging it; #460, #392, #481 and #478 changed the same files. Then two reviews against craft-launcher main, which feeds this crate its releases:
- **A release knows its repository.** `Release::repo` is the repository the release was read from, and `check_download_link` checks links against it (empty means the app's own). The launcher's feed can rename a repository (`Feed::repo_for`); checking against `app.repo` refused the renamed repository's downloads.
- **A `+` matches either way.** `release::same_download_url` reads `%2B` as `+` on both sides. The launcher's own `release_from_sums` and feed write a plain `+`, and so do releases saved by an older version, so only accepting `%2B` refused those self-updates. Nothing else is decoded (`%2b` still fails).
- **The launcher's tag and file-name rule (craft-launcher#1024).** `valid_tag`: up to 64 of `[A-Za-z0-9._+-]`, no leading dot, used for the API's `tag_name`, the redirect and download links. `valid_file_name`: the same characters, no leading or trailing dot, no Windows device name (`CON`, `nul.txt`, `COM1.msi`). The launcher's feed calls `release::valid_tag`, so craft-launcher#930 needs it here.
- **`ask_pages` fetches `SHA256SUMS.txt` from `expected_download_url`**, the link it stores.
- **Guix and Homebrew on Linux** are left to their package manager (table above), like Homebrew and Nix on macOS (#460).
## What changes for apps
- **Loopback mirrors:** a build that downloads from a developer's loopback mirror must turn on `craft-update/local-mirror`, or the new check refuses the mirror's links. The launcher's `local-mirror` feature turns it on (craft-launcher#930). Apps rolling out under #319 without a mirror need no change.
- **Linux:** copies under the package-manager folders above no longer update themselves. That's how the launcher behaved before #311.
- **`+` in a tag:** handled (section 1). No Craft release uses `+` today. GitHub's link form was checked on bell-sw/Liberica's releases: the tag and name keep `+`, the link has `%2B`, and the `releases/latest` redirect has a plain `+`.
## Compared with the launcher
I compared launcher 2d12245 (the snapshot #311 copied) and launcher `main` with this crate.
- **All of the launcher's security fixes from 15–25 October are here:** #747/#864, #865/#748, #867, the proxy part of #868, #689, #815, #821 and the archive limits. The launcher's two commits after the snapshot (#871, #903) change no behaviour here.
- **Two things are here but not yet in the launcher:**
- the minute-long `hdiutil attach` retry (#349): craft-launcher#930 takes it;
- the zip 9 drive-prefix test (#335): it waits for the launcher's zip 9 bump (storytold/craft-launcher#899).
- **Follow-ups that don't belong in this PR** are filed separately (linked below once filed).
## Tests
- **New:** `a_download_link_must_be_its_releases_own`, `a_plus_in_the_tag_is_percent_encoded_in_the_link`, `install_accepts_a_plus_release_up_to_the_download`, `update_accepts_a_plus_release_up_to_the_download`, `checksum_names_with_encoded_separators_make_no_download`, `update_refuses_a_link_that_is_not_its_releases_own`, `only_the_packages_own_path_is_a_package_and_system_folders_are_left_alone`.
- **Extended:** `install_rejects_an_artifact_whose_version_is_not_the_advertised_one` (another repo's link, a foreign checksums link), `drops_downloads_that_point_elsewhere`, `file_names_cannot_escape`, `tag_from_redirect`.
- **Mutation check:** `update_refuses_a_link…` fails when the call in `self_update::update` is removed.
- **Mutation check:** both `…_accepts_a_plus_release_up_to_the_download` tests fail with the bind error when the `%2B` decoding is removed.
- **On Windows, after merging main (31 October):** `cargo test -p craft-update`: 232 passed, 3 ignored, the same without default features. Clippy `-D warnings` is clean with default features and with `egui,local-mirror`, and `cargo fmt --check` is clean.
- **Merged with #389 (before the 31 October merge of main):** 173 passed, 28 without default features, and clippy for wasm32 is clean.
- **Launcher (before the 31 October merge of main):** with this branch merged with #389 (craft-launcher#930 pins that merge), the full ArtCraft Launcher suite passes: 401 tests, plus clippy, fmt and the wasm32 build of its core.
- **Added for review (
|
||
 .CooCoo
|
5a93050c9f |
update: integration of the open fixes to craft-update (#763)
* update: an unsigned Windows copy gets Download instead of Update and Restart The update helper installs nothing for an unsigned copy, so the prompt's Update and Restart button on a local or unsigned build downloaded the release and then stopped with "this copy isn't signed". is_self_updating now answers false for an unsigned copy on Windows, and the prompt offers Download. CRAFT_UPDATE_DEV still offers the update. * update: check that a Windows copy can install before downloading windows_precheck ran after the download, so a copy that could not install downloaded the whole release first. The checks that do not depend on the download now run before it; a new MSI's signature is checked after it. The cache folder is created first, so a first update passes the check. * update: a failure recorded before the running program is not reported take_outcome reported any recorded failure at the next start, so a copy put in place after an update had failed still opened with "The update could not be installed". A failure record older than the running program is now removed without being reported. A failure recorded after the program was put in place, such as a failed helper's, is still reported. * update: a Cargo build with a separate build directory is a development build With build.build-dir set, Cargo keeps .rustc_info.json and the profile's .fingerprint, deps and build folders in the build directory, and the program's own folder has only its lock file. Such a build counted as an installed copy and was offered Update and Restart. A profile folder with Cargo's .cargo-lock under a target root with CACHEDIR.TAG is now a development build, and gets the download link. * update: a cache folder that can't be created is reported as that same_cache_as_helper ignored the error from creating the folder, so a failure read as a cache in the wrong place. It now names the folder and the error. * craft-update: a slow macOS signature check is not reported as a bad signature `signed_by` gave `codesign` 120 seconds and treated every failure as a bad signature, so a Mac that could not finish in time was told the app "isn't signed by its developer". The check now has 10 minutes and checks one bundle at a time. A job waiting for its turn reports `JobPhase::Waiting` and can be canceled. A timeout, a `codesign` that cannot start or is stopped by a signal, and any exit other than 1 or 3 report that the signature couldn't be checked. Exit 3 refuses the app as not signed by its developer. Exit 1 refuses it with `codesign`'s own reason, because `codesign` also exits 1 for a bundle it cannot read. The app is not installed in any of these cases. On macOS, an install or uninstall now stops when `pgrep` gives no answer, where a timeout used to read as "not running". `pgrep` has 30 seconds. An install looks again after the signature check, which can wait behind other apps' checks, and does not replace an app opened during the wait. A self-update skips that second look. * craft-update: a copy that cannot be replaced gets Download A macOS bundle, an AppImage or a Linux standalone program was offered Update and Restart even when the update could not replace it, so the update failed only after the whole download. `current_at` now leaves out such a copy when `access(2)` refuses what the update needs. On macOS that is adding a subdirectory to the parent, and adding a subdirectory to and deleting the bundle. On Linux it is writing and searching the folder. A macOS copy under App Translocation is also left out. Any other error from the check leaves the update on offer, and `CRAFT_UPDATE_DEV` still offers it. * craft-update: a macOS standalone copy that cannot be replaced gets Download macOS `detect` returns `Install::Standalone` for a loose program, and the update replaces it by writing beside it, as on Linux. `can_replace` now checks that folder on macOS too, so a copy in a folder the user cannot write gets Download instead of failing after the download. `CRAFT_UPDATE_DEV` still offers the update. * craft-update: a standalone copy whose share files cannot be replaced gets Download The update replaces a standalone copy's files under `share` as well as the program when the app's `share/<id>` folder sits next to `bin`. The offer now also checks those folders, the `applications` and `icons/hicolor` folders a release writes to, and the nearest existing parent of one that doesn't exist yet. * craft-update: a standalone macOS program without a release tarball gets Download On macOS a program outside an app bundle counts as a standalone copy, and the self-update replaces it from a macOS .tar.gz. Releases carry a disk image and a command-line zip for macOS and no .tar.gz, so the prompt showed Update and Restart and the update stopped with "has no Tarball (.tar.gz) for macOS". The prompt now also asks release_installs, which on macOS offers Update and Restart to a standalone program only when the release lists that asset. Without it the prompt shows Download. A release whose file list was not loaded keeps the offer, because the install fetches the full release first. CRAFT_UPDATE_DEV still offers the update. Other platforms and other kinds of install are not checked against the release. * craft-update: CRAFT_UPDATE_DEV no longer skips the release check A standalone macOS program run with CRAFT_UPDATE_DEV was still offered Update and Restart for a release without a macOS .tar.gz, and that update failed. release_installs now checks the release for that copy too. release_installs_at takes the executable, the opt-in setting and the platform, and a new test checks a loose program in a temporary folder against releases with and without the .tar.gz. The tests use SoundCraft's release files, since craft-launcher ships no command-line zip. * craft-update: a failed self-update keeps its download for the next try On macOS, Linux and FreeBSD, prepare_update removed the downloaded release file whether or not the install succeeded, so every retry after a failed copy, signature check, package install or swap downloaded the whole release again. A failed install now keeps the file, as install_at already does, and the next try reuses it through Http::download, which checks it against the published SHA-256 first and fetches it again when it does not match. - The cache folder's self-update-download.txt records the name and size of the release file before the download starts. A self-update for a different release file removes the finished and partial download that the record names, so each app keeps the files of one release at most. - An install that succeeds removes the file and the record. So does a failed install whose record could not be written, since nothing would remove that file later. - On macOS, when hdiutil cannot attach the disk image, the file is still removed: an image left attached by an install that ended part way makes hdiutil refuse the same file with "Resource busy", while a new download of it attaches. - Windows is unchanged: its update helper removes the download after the app exits. * craft-update: a macOS install that stopped partway no longer blocks the next install When a macOS install ends before it finishes (the app quits or crashes), the copy it was making stays beside the app and the update's disk image stays attached. The next attach of the same file fails with "Resource busy". - Each staging folder's name now carries the installing process's id, and the folder holds an empty marker file. Before it stages a copy, an install removes the marked staging folders of processes that are no longer running, and their empty unmarked ones. It also removes a staging folder from earlier launcher versions when it holds only the app's bundle and has not changed for a day. Backup folders and other apps' folders are left alone. - Before it attaches an image, an install detaches the images that earlier installs of the same app left at their mount points in the cache, when their process has ended or, in this process, when no install uses them any more. - When the attach fails, the install detaches its image if it is attached with no volume mounted and no other attach of the same file is running. - A staging folder that can't be removed, or an image that stays attached, is logged, and the error of a failed install says so after its reason. * craft-update: the kept-download test accepts either signature refusal An unsigned bundle is refused as "isn't signed" today and as a failed signature check once codesign's exit status is read, so the test checks that the install stopped on its signature. * craft-update: the share check covers nested and pixmaps folders A standalone copy in a prefix gets Update and Restart only when the update can write every `share` folder it replaces files in. The check covered `share/<id>`, `share/applications` and `share/icons/hicolor` only, but the update writes a copy of each file next to its destination first. A folder inside `share/<id>` (such as `models`), an icon folder such as `icons/hicolor/256x256/apps`, or `share/pixmaps` (where an older flat release puts its icon) that the user can't write stopped the update after the whole download. The check now also takes in every folder inside `share/<id>`, each `icons/hicolor/<size>/apps` folder that holds the app's icon, and `share/pixmaps`. Icon folders of other packages and other themes are not checked. A linked folder is checked but not walked. The walk reads at most 4,096 folder entries, so a large tree does not slow every start; a folder past that is reported by the install as before. The same check runs for a standalone copy on Linux, so a Linux copy whose nested share folder or `share/pixmaps` it can't write now gets the download link too. * craft-update: a release folder that can't be read stops a standalone update A standalone update in a prefix copies every file under the release's `share` folder. The walk skipped any folder it could not list, and the archive keeps each folder's mode, so a folder that nobody may list left its files out while the update reported success. An error reading an entry of the release's `bin` folder was dropped the same way. These are now errors that name the folder, and the update stops before any installed file changes. A release without a `share` folder still updates. A release whose `share` or `bin` is a link was followed, so the update copied files from outside the release into the prefix. A `share` or `bin` that is a link, or a `share` that is a file, now stops the update. Links further down under `share` are still skipped. The unpacked release is removed after a failed update even when it holds a folder that its owner can't list. Before, such a folder stayed in the cache. * craft-update: the share check covers doc, metainfo and mime, and follows links The share check for a standalone copy in a prefix now also takes in `share/doc/<id>` and every folder inside it, `share/metainfo` and `share/mime/packages`. Release archives such as VectorCraft's Linux and FreeBSD ones put files in all three. The walk now follows links, as the update's writes do. A copy whose `share/<id>` is itself a link had none of its folders checked, and a folder behind a link inside `share/<id>` was not checked either. A folder the walk reaches twice is read once. When `doc/<id>`, `metainfo`, `mime/packages` or `pixmaps` is missing, the check no longer falls back to `share` itself. A copy whose `share` the user can't write, but whose app, `applications` and `icons/hicolor` folders exist, gets Update and Restart again, as it did before the previous commit. The entry limit of 4,096 now applies to each of `share/<id>`, `doc/<id>` and `icons/hicolor` separately, and it counts files and folders. A large app folder no longer turns off the icon folder check. In a folder with the sticky bit, such as `/tmp`, the user can rename over a file only when the user owns the file or the folder. A copy now gets the download link when such a folder holds one of the app's files that another user owns: the program itself, its other programs (`<id>-<name>`, such as `<id>-cli`), anything in `share/<id>` or `share/doc/<id>`, or a `share` file named `<id>.<ext>` or `<bundle id>.<ext>`. An icon size that the installed copy doesn't have yet is still not checked, and a read-only folder that the user added under `share/<id>` does count. Linux runs the same checks for a standalone copy, and the sticky check on the program file for an AppImage. * craft-update: the app-program check has its own name The check for the app's programs in a bin folder was named is_program, the name of the Windows check for an MZ header, so the crate did not build on Windows. It is now is_app_program. * craft-update: a missing doc, metainfo or mime folder is checked through share A Linux release writes files into `share/doc/<id>`, `share/metainfo` and `share/mime/packages`, and the update creates each of these folders when it is missing. When one was missing, the share check took its nearest existing parent inside `share` and never `share` itself. A copy whose `share` the user can't write and that lacked one of these folders was offered Update and Restart, and the update failed after the whole download. For a missing `doc/<id>`, `metainfo` or `mime/packages`, the check now takes the nearest existing parent with `share` included, as it already did for `applications`. Only the old flat release layout writes to `share/pixmaps`, so a missing `pixmaps` is still checked inside `share` only, and a copy whose `share` the user can't write keeps Update and Restart when every other release folder exists. A folder in the app's folder or in `doc/<id>` that the user can't read is still checked against its own permissions, as before. The copy gets Download when the user can't write and search that folder. The check can't list the folders inside it and does not check them. A new test keeps this rule, and the check does not refuse a copy only because a folder in it can't be read. * craft-update: the staging cleanup's doc says which folders it removes A marked staging folder is removed once its process has ended, whichever app that install was for: the folder's name does not record the app. Only staging folders of earlier launcher versions are checked for the app's bundle. * craft-update: a failed attach leaves its image attached when pgrep gives no answer detach_unmounted, from #766, checked for another attach of the same image file with is_running, which reads a pgrep that gives no answer as "not running", and then force-detached the image's unmounted attachments. It now uses process::running, from #753, and detaches only when the check answers that nothing is running. |
||
|
dotCooCoo
|
f89c6686df |
craft-update: keep link addresses out of the opener's warnings
The warnings that open_url_with_fallback and the system opener log no longer include the address being opened, so a link carrying a token or personal details does not reach the application's log. The reasons from the system stay in the messages. |
||
|
dotCooCoo
|
5eb0bca0b1 |
craft-update: start a detached process only from the thread that waits for it
On Unix, spawn_detached starts the waiting thread first, and that thread starts the process and waits for it. When no thread can be started, the call returns an error and the process is not started, so no exited process is left unreaped. On Windows, where an exited process needs no wait, the process starts on the calling thread and no thread waits for it. The list of processes kept for a later call, and the reaping it needed, are removed. When the opener of open_url_with_fallback is still running after its timeout, the open-url thread waits for it. An opener whose state can't be read is stopped and waited for. The tests run spawn_detached's waiting-thread path and check that the waiting thread gets the process's exit status, that the process is not started when no thread can start, and that a program that cannot start reports it. They no longer need ps. |
||
|
dotCooCoo
|
8f17421a0a |
craft-update: say when a kept process is reaped and that start_pending_update is for exit
A process that no thread could wait for is reaped when a later call starts another process after it has exited. start_pending_update is called only as the app exits: afterwards a finishing installation starts its helper itself instead of reporting through tick, and one still running when the process ends installs nothing. The reaping test waits for the process to exit through its output and fails instead of looping when reaping does not happen. |
||
|
dotCooCoo
|
324f06f639 |
craft-update: wait later for a started process when no thread can wait for it
When spawn_detached or the system opener cannot start a thread to wait for the process it started, the process is kept and waited for by a later call once it has exited, so on Unix it does not stay in the process table. |
||
|
dotCooCoo
|
df66c35b07 |
craft-update: keep the app running when a helper thread cannot start
spawn_detached, run_command and open_url_with_fallback started their reaper, pipe-reader and opener threads with std::thread::spawn, which panics when the thread cannot be created. The Windows update helper now starts from the UI thread when the app closes, so such a panic would end the app. Each thread now starts through std::thread::Builder: a process that has already started keeps running and the failed wait is logged, run_command stops the helper and returns an error, and a failed opener thread is logged. |
||
 VoiceLessQ
|
aaa6742479 |
fix(update): handle a bundle Flatpak before 1.16.2 won't reinstall
Flatpak before 1.16.2 ignores --reinstall for a bundle and fails with "<id> already installed" when that commit is deployed. An app install now uninstalls the user's copy and installs again, without --delete-data, as the launcher does (storytold/craft-launcher#1014). A failure after that says what happened and what to do. A self-update never uninstalls the app it runs from: its download goes right after, so a failed install would leave nothing. That error there means an earlier update landed and never restarted, so it goes on to the restart. Root gets a flatpak command, so tests can record the real calls. |
||
 Dylan M. Taylor
|
fe61243ebf |
feat(update): add craft-update crate for release checking, install, and self-updating (#311)
* feat(update): add craft-update crate for release checking, install, and self-updating * fix(update): provide non-windows stub for finish * fix(update): address CodeRabbit review feedback on craft-update * feat(dialog,self_update): enhance prompt styling, support standalone self-updates * fix(update): make startup prompt modal with Escape, skip non-file tar entries |