daniel543x/craft-libs
1
0 Fork 0
mirror of https://github.com/storytold/craft-libs.git synced 2026-11-11 21:12:18 +00:00
Code Issues Packages Projects Releases Wiki Activity

craft-update: accept exactly one download link per asset (craft-launcher#764); keep the Linux self-update off other copies

Browse Source 
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 (3ad9074):** `two_links_for_one_asset_keep_only_the_releases_own`, `install_refuses_links_off_the_releases_own_before_downloading` (other hosts, and a `SHA256SUMS.txt` linked elsewhere), `install_refuses_while_the_installed_copy_is_running`, `a_download_that_fails_its_checksum_replaces_nothing`; `a_download_link_must_be_its_releases_own` also refuses other hosts. On Windows: 236 passed, 3 ignored; clippy and fmt clean. The bad-checksum test fails when the server sends the real file.
- **Added in section 3:** `tags_are_plain`, `a_plus_link_matches_either_way_and_nothing_else_does`, `a_release_knows_its_repository`; `file_names_cannot_escape` takes #1024's cases; the install test checks a renamed repository and a plain `+`; the Linux location test adds Guix and Linuxbrew. On Windows after the rebase: 251 passed, 3 ignored; the workspace builds; clippy `-D warnings` is clean for this crate (with `local-mirror`) apart from `translate::button` being unused, which comes from main's #508.
- **Not built yet:** the Linux-only `detect` code, and the Unix branch of the running-app test. CI builds it first, once a maintainer approves the workflow runs.


<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
<!-- coderabbit-ai-generated:v1 -->

## Summary by CodeRabbit

* **Bug Fixes**
  * Update downloads are now restricted to assets belonging to the selected release, including checksum files. Unrelated or invalid links are rejected before a download begins.
  * Release links with unsafe path components or invalid redirect tags are rejected. Encoded plus signs in release tags and filenames are handled correctly.
  * Linux installation detection more accurately distinguishes package-managed installations, other system locations, and standalone installations, improving whether self-updates are available.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
This commit is contained in:
smittie2000 authored and GitHub committed 2026-11-11 07:47:52 +01:00
1 parent 49d865dbdd
commit 797d21a3f3
5 files changed
+579 -40
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

crates/update/src/install.rs
+206 -4
View File
@@ -67,6 +67,11 @@ fn not_running(app: &App, answer: Result<bool, String>) -> Result<(), String> {
}
}
/// [`install`], with `commit` called once a portable ZIP's new files are in place and its version
/// checked, so the caller can save its record; an error from it rolls the previous copy back
/// (Windows only; ignored elsewhere). Refuses before downloading when the app is running, the
/// asset isn't the release's version (`bind_artifact_version`), a newer copy is installed, or the
/// link isn't the release's own (`check_download_link`).
#[allow(clippy::too_many_arguments)]
pub fn install_at(
app: &App,
@@ -97,6 +102,7 @@ pub fn install_at(
{
return Err(format!("Cannot update {} to {}: installed version {} is newer", app.name, release.version_label(), inst_v));
}
check_download_link(app, release, asset)?;
log::info!("{}: starting {} to {} ({})", app.name, if existing.is_some() { "update" } else { "install" }, release.version_label(), kind.label());
let file = download(release, asset, job)?;
log::info!("{}: download ready ({}), running installer...", app.name, file.display());
@@ -180,13 +186,15 @@ pub(crate) fn bind_artifact_version(app: &App, release: &Release, asset: &Asset)
}
// The release tag and filename embedded in a release download URL (`.../releases/download/<tag>/<name>`).
if let Some(rest) = asset.url.split("/releases/download/").nth(1) {
// A `+` in either is `%2B` there, as GitHub and `expected_download_url` write it; no other
// escape is decoded, so anything else still has to match as written.
if let Some((_, rest)) = asset.url.split_once("/releases/download/") {
let mut parts = rest.split('/');
let tag = parts.next().unwrap_or("");
let url_filename = parts.next().unwrap_or("");
let tag = parts.next().unwrap_or("").replace("%2B", "+");
let url_filename = parts.next().unwrap_or("").replace("%2B", "+");
if tag.is_empty()
|| release_version
.map_or(tag != release.tag, |release_version| tag != release.tag && version::parse(tag).as_ref() != Some(release_version))
.map_or(tag != release.tag, |release_version| tag != release.tag && version::parse(&tag).as_ref() != Some(release_version))
{
return Err(mismatch());
}
@@ -197,6 +205,29 @@ pub(crate) fn bind_artifact_version(app: &App, release: &Release, asset: &Asset)
Ok(())
}
/// The last check before any bytes are fetched (storytold/craft-launcher#764): the link must be
/// exactly the GitHub download for this release's tag and this asset's name in the release's
/// repository ([`Release::repo`], else the app's own), whichever converter (API, feed or
/// `SHA256SUMS.txt`) built the asset, and so must the link of the release's `SHA256SUMS.txt` that
/// the download is checked against. A `+` may be written plainly or as `%2B`
/// ([`release::same_download_url`]).
///
/// With this crate's `local-mirror` feature, the developer's loopback mirror is accepted too. The
/// consumer confines those links (the launcher's feed parser does) and must turn the feature on
/// itself (`craft-update/local-mirror`): its own feature of that name doesn't. This crate's tests
/// accept loopback as well, as their downloads come from a server on 127.0.0.1.
pub fn check_download_link(app: &App, release: &Release, asset: &Asset) -> Result<(), String> {
let repo = if release.repo.is_empty() { app.repo } else { release.repo.as_str() };
let own = |a: &Asset| {
((cfg!(feature = "local-mirror") || cfg!(test)) && crate::http::is_loopback(&a.url))
|| release::expected_download_url(repo, &release.tag, &a.name).is_some_and(|expected| release::same_download_url(&a.url, &expected))
};
if own(asset) && release.sums_asset().is_none_or(own) {
return Ok(());
}
Err(format!("{}'s download link isn't the one its release publishes ({}), so it wasn't downloaded.", app.name, asset.name))
}
/// What a new install would write over: something already at its destination that the launcher
/// didn't put there.
pub fn occupied(
@@ -3292,6 +3323,8 @@ esac
assert!(result.unwrap_err().contains("installed version 0.5.0 is newer"));
}
/// An asset whose file name, link tag or link file name isn't the release's is refused before
/// the download, and so is one in another app's releases or with a foreign `SHA256SUMS.txt`.
#[test]
fn install_rejects_an_artifact_whose_version_is_not_the_advertised_one() {
// The release advertises 99.0.0, but the selected artifact is an older official build
@@ -3337,6 +3370,175 @@ esac
..Asset::default()
};
assert!(bind_artifact_version(app, &release, &bad_tag).is_err());
// storytold/craft-launcher#764: another app's release with the right tag and name passes
// the bind (it reads only the path after `/releases/download/`), but the check before the
// download refuses it.
let other_repo = Asset {
name: "photocraft-99.0.0-linux-x86_64.AppImage".into(),
url: "https://github.com/storytold/vectorcraft/releases/download/v99.0.0/photocraft-99.0.0-linux-x86_64.AppImage".into(),
..Asset::default()
};
assert!(bind_artifact_version(app, &release, &other_repo).is_ok());
let err = install_at(app, &release, &other_repo, PackageKind::AppImage, None, None, &mut job, None).unwrap_err();
assert!(err.contains("isn't the one its release publishes"), "{err}");
assert!(check_download_link(app, &release, &good).is_ok());
// The checksums the download is verified against must come from the release too.
let mut foreign_sums = release.clone();
foreign_sums.assets.push(Asset {
name: "SHA256SUMS.txt".into(),
url: "https://github.com/storytold/vectorcraft/releases/download/v99.0.0/SHA256SUMS.txt".into(),
..Asset::default()
});
assert!(check_download_link(app, &foreign_sums, &good).is_err());
// A release the feed reads from a renamed repository records it, and its links are
// checked against that one; the same links are refused for a release that doesn't.
let renamed =
release::release_from_sums("storytold/photocraft-next", "v0.5.0", &[("a".repeat(64), "photocraft-0.5.0-linux-x86_64.AppImage".into())]);
assert!(check_download_link(app, &renamed, &renamed.assets[0]).is_ok());
let unrecorded = Release { repo: String::new(), ..renamed.clone() };
assert!(check_download_link(app, &unrecorded, &unrecorded.assets[0]).is_err());
// A `+` written plainly, as an older launcher's feed and saved releases have it, passes.
let plus = release::release_from_sums(app.repo, "v1.0.0+b.1", &[("a".repeat(64), "photocraft-1.0.0+b.1-linux-x86_64.AppImage".into())]);
let mut plain = plus.clone();
for a in &mut plain.assets {
a.url = a.url.replace("%2B", "+");
}
assert!(check_download_link(app, &plain, &plain.assets[0]).is_ok());
}
/// A release whose tag has a `+` (semver build metadata) has `%2B` in its links. Both checks
/// before the download accept them, so `install_at` gets as far as the download, which the
/// cancelled job ends before anything is fetched. A link with another escape is still refused.
#[test]
fn install_accepts_a_plus_release_up_to_the_download() {
let dir = tempfile::tempdir().unwrap();
let paths = Paths::under(dir.path());
let http = Http::new(crate::TEST_AGENT).unwrap();
let phase = &mut |_| ();
let locations = InstallLocations::default();
let app = App::by_id("photocraft").unwrap();
let name = "photocraft-1.0.0+build.1-linux-x86_64.AppImage";
let release = release::release_from_sums(app.repo, "v1.0.0+build.1", &[("a".repeat(64), name.to_string())]);
let asset = release.assets[0].clone();
assert!(asset.url.ends_with("/download/v1.0.0%2Bbuild.1/photocraft-1.0.0%2Bbuild.1-linux-x86_64.AppImage"), "{}", asset.url);
assert_eq!(bind_artifact_version(app, &release, &asset), Ok(()));
let stop = Stop::default();
stop.cancel();
let mut job = Job { http: &http, paths: &paths, stop: &stop, phase, install_locations: &locations, replace: false };
let result = install_at(app, &release, &asset, PackageKind::AppImage, None, None, &mut job, None);
assert_eq!(result.unwrap_err(), crate::protocol::CANCELLED);
let other_escape = Asset { url: asset.url.replace("%2B", "%2b"), ..asset };
assert!(bind_artifact_version(app, &release, &other_escape).is_err());
}
/// craft-launcher#764: `install_at` refuses, before downloading, an asset whose link is the
/// right path on another host, and a release whose `SHA256SUMS.txt` (what the download is
/// checked against) is linked elsewhere. The stop is cancelled, so if the check stopped working
/// the install would end at the download with `CANCELLED` instead of fetching anything.
#[test]
fn install_refuses_links_off_the_releases_own_before_downloading() {
let dir = tempfile::tempdir().unwrap();
let paths = Paths::under(dir.path());
let http = Http::new(crate::TEST_AGENT).unwrap();
let locations = InstallLocations::default();
let app = App::by_id("photocraft").unwrap();
let name = "photocraft-0.5.0-linux-x86_64.AppImage";
let own = release::release_from_sums(app.repo, "v0.5.0", &[("a".repeat(64), name.to_string())]);
let refuse = |release: &Release, asset: &Asset| {
let stop = Stop::default();
stop.cancel();
let phase = &mut |_| ();
let mut job = Job { http: &http, paths: &paths, stop: &stop, phase, install_locations: &locations, replace: false };
let result = install_at(app, release, asset, PackageKind::AppImage, None, None, &mut job, None);
assert!(result.as_ref().is_err_and(|e| e.contains("isn't the one its release publishes")), "{}: {result:?}", asset.url);
};
for url in [
format!("https://evil.example/storytold/photocraft/releases/download/v0.5.0/{name}"),
format!("http://github.com/storytold/photocraft/releases/download/v0.5.0/{name}"),
format!("https://github.com.evil.example/storytold/photocraft/releases/download/v0.5.0/{name}"),
] {
refuse(&own, &Asset { url, ..own.assets[0].clone() });
}
let mut sums_elsewhere = own.clone();
let sums = sums_elsewhere.assets.iter_mut().find(|a| a.name == "SHA256SUMS.txt").unwrap();
sums.url = "https://evil.example/storytold/photocraft/releases/download/v0.5.0/SHA256SUMS.txt".into();
refuse(&sums_elsewhere, &sums_elsewhere.assets[0]);
}
/// A download that fails its checksum (twice, as a resumed file gets one fresh try) replaces
/// nothing: the installed copy keeps its bytes, the install never starts, the record isn't
/// committed, and the bad file isn't kept for the next try.
#[test]
fn a_download_that_fails_its_checksum_replaces_nothing() {
use crate::http::tests::{Reply, body, serve};
let app = App::by_id("photocraft").unwrap();
let (body, sha) = body();
let (url, requests) = serve(body.clone(), vec![Reply::Corrupt, Reply::Corrupt]);
let name = "photocraft-0.5.0-linux-x86_64.AppImage";
let asset = Asset { name: name.into(), size: body.len() as u64, url, sha256: Some(sha) };
let release = Release { version: version::parse("0.5.0"), tag: "v0.5.0".into(), assets: vec![asset.clone()], ..Release::default() };
let dir = tempfile::tempdir().unwrap();
let paths = Paths::under(dir.path());
let http = Http::new(crate::TEST_AGENT).unwrap();
let old = dir.path().join("Apps").join("PhotoCraft.AppImage");
std::fs::create_dir_all(old.parent().unwrap()).unwrap();
std::fs::write(&old, b"the installed 0.4.0").unwrap();
let installed =
Installed { version: version::parse("0.4.0"), source: InstallSource::Launcher, location: old.display().to_string(), package: None };
let previous = record("0.4.0", PackageKind::AppImage, old.clone(), vec![], false);
let mut phases = Vec::new();
let mut phase = |p: JobPhase| phases.push(p);
let locations = InstallLocations::default();
let stop = Stop::default();
let mut job = Job { http: &http, paths: &paths, stop: &stop, phase: &mut phase, install_locations: &locations, replace: false };
let mut committed = false;
let mut commit = |_: &Record| {
committed = true;
Ok(())
};
let result = install_at(app, &release, &asset, PackageKind::AppImage, Some(&installed), Some(&previous), &mut job, Some(&mut commit));
assert!(result.as_ref().is_err_and(|e| e.contains("checksum")), "{result:?}");
assert_eq!(std::fs::read(&old).unwrap(), b"the installed 0.4.0");
assert!(!phases.contains(&JobPhase::Installing), "{phases:?}");
assert!(!committed);
assert!(!download_path(&paths, &asset).exists());
assert_eq!(requests.lock().unwrap().len(), 2);
}
/// A running installed copy isn't replaced; the stop is cancelled in case the check stops
/// working. Windows finds a process by its file name, so the running test binary stands in for
/// the copy. Elsewhere `pgrep -f` matches the command line, so a probe runs with the copy's path.
#[test]
fn install_refuses_while_the_installed_copy_is_running() {
let dir = tempfile::tempdir().unwrap();
let paths = Paths::under(dir.path());
let http = Http::new(crate::TEST_AGENT).unwrap();
let phase = &mut |_| ();
let locations = InstallLocations::default();
let app = App::by_id("photocraft").unwrap();
let name = "photocraft-0.5.0-linux-x86_64.AppImage";
let release = release::release_from_sums(app.repo, "v0.5.0", &[("a".repeat(64), name.to_string())]);
#[cfg(windows)]
let location = std::env::current_exe().unwrap().display().to_string();
#[cfg(unix)]
let location = dir.path().join("PhotoCraft.AppImage").display().to_string();
#[cfg(unix)]
let _probe = crate::process::tests::RunningProbe::start(&location);
assert!(process::is_running(Path::new(&location)), "{location} should be seen as running");
let installed = Installed { version: version::parse("0.4.0"), source: InstallSource::Launcher, location, package: None };
let stop = Stop::default();
stop.cancel();
let mut job = Job { http: &http, paths: &paths, stop: &stop, phase, install_locations: &locations, replace: false };
let result = install_at(app, &release, &release.assets[0], PackageKind::AppImage, Some(&installed), None, &mut job, None);
assert!(result.as_ref().is_err_and(|e| e.contains("is running")), "{result:?}");
}
#[cfg(target_os = "linux")]
crates/update/src/process.rs
+4 -3
View File
@@ -488,16 +488,17 @@ fn pgrep_pattern(path: &str) -> String {
#[cfg(test)]
#[cfg(unix)]
mod tests {
pub(crate) mod tests {
use std::io::{BufRead, BufReader};
use std::process::Child;
use super::*;
struct RunningProbe(Child);
/// A process whose command line is `path`, as `pgrep -f` sees it, until dropped.
pub(crate) struct RunningProbe(Child);
impl RunningProbe {
fn start(path: &str) -> Self {
pub(crate) fn start(path: &str) -> Self {
let child = command("/bin/sh")
.args(["-c", "printf 'ready\\n'; read line", path])
.stdin(Stdio::piped())
crates/update/src/release.rs
+235 -29
View File
@@ -35,6 +35,12 @@ pub struct Release {
pub notes: String,
pub prerelease: bool,
pub assets: Vec<Asset>,
/// The `owner/name` repository the release was read from, whose download links its assets
/// must be (`install::check_download_link`). Usually the app's own; a feed that announces a
/// renamed repository reads the release from the new one. Empty means the app's own (a
/// release saved before this field, or one built by hand).
#[serde(default)]
pub repo: String,
}
impl Release {
@@ -117,6 +123,25 @@ pub fn download_prefix(repo: &str) -> String {
format!("https://github.com/{repo}/releases/download/")
}
/// The one link a GitHub release asset may have: `https://github.com/<repo>/releases/download/<tag>/<name>`,
/// built from the release's own tag and the asset's own name, so a link can't point at another
/// release or anywhere else (#764). `None` unless the tag is a [`valid_tag`] and the name a
/// [`valid_file_name`]: only ASCII letters, digits and `._+-`, which github.com can't re-interpret.
/// A `+` (semver build metadata) is written `%2B`, as GitHub writes it in `browser_download_url`
/// (`.../download/17.0.9%2B9/jdk-17.0.9%2B9.deb` for the tag `17.0.9+9`).
pub fn expected_download_url(repo: &str, tag: &str, name: &str) -> Option<String> {
let encoded = |s: &str| s.replace('+', "%2B");
(valid_tag(tag) && valid_file_name(name)).then(|| format!("{}{}/{}", download_prefix(repo), encoded(tag), encoded(name)))
}
/// Whether `url` is `expected`, an [`expected_download_url`], with each `%2B` in either read as
/// `+`. GitHub serves a `+` in a download path the same either way, and links built before
/// `%2B` (an older launcher's feed, or a release saved by an older version) write it plainly.
/// No other escape is read, so `%2b` or `%2e` still has to match as written.
pub fn same_download_url(url: &str, expected: &str) -> bool {
url == expected || url.replace("%2B", "+") == expected.replace("%2B", "+")
}
/// Whether `url` is a download that truly belongs to `repo`.
///
/// A raw `starts_with(prefix)` check is not enough: a value such as
@@ -145,7 +170,7 @@ fn download_suffix_ok(rest: &str) -> bool {
}
let mut parts = rest.split('/');
let (Some(tag), Some(name), None) = (parts.next(), parts.next(), parts.next()) else { return false };
!tag.is_empty() && tag != "." && tag != ".." && tag.len() <= 64 && !tag.contains('\\') && valid_file_name(name)
valid_tag(tag) && valid_file_name(name)
}
/// Any `.` or `..` path segment that could climb out of the repository.
@@ -153,11 +178,12 @@ fn contains_traversal(rest: &str) -> bool {
rest.split('/').any(|seg| seg == "." || seg == "..")
}
/// Percent-encoded slash or backslash (`%2f`, `%5c`): a client could decode these back into
/// separators after the check, so a confined URL must not contain them.
/// Percent-encoded slash, backslash or dot (`%2f`, `%5c`, `%2e`): a client or github.com could
/// decode these back into separators or a `..` segment after the check (#764), so a confined URL
/// must not contain them.
fn contains_encoded_separator(rest: &str) -> bool {
let bytes = rest.as_bytes();
bytes.windows(3).any(|w| w[0] == b'%' && matches!(&w[1..].to_ascii_lowercase()[..], b"2f" | b"5c"))
bytes.windows(3).any(|w| w[0] == b'%' && matches!(&w[1..].to_ascii_lowercase()[..], b"2f" | b"5c" | b"2e"))
}
/// Reads `GET /repos/<repo>/releases/latest` (or one element of `/releases`).
@@ -189,21 +215,24 @@ pub fn parse_github_release_list(json: &str, repo: &str, allow_prerelease: bool)
Ok(best)
}
/// One release object of the GitHub API, with every text capped and only the assets whose name is
/// a plain file name and whose link is exactly [`expected_download_url`]. An API error object
/// (`message`) becomes its text.
fn release_from_value(v: &serde_json::Value, repo: &str) -> Result<Release, String> {
if let Some(msg) = v["message"].as_str() {
return Err(if msg == "Not Found" { "no releases yet".into() } else { format!("GitHub says: {}", truncate(msg, 200)) });
}
let tag = v["tag_name"].as_str().filter(|t| !t.is_empty() && t.len() <= 64).ok_or("no release found")?.to_string();
let tag = v["tag_name"].as_str().filter(|t| valid_tag(t)).ok_or("no release found")?.to_string();
let releases = format!("https://github.com/{repo}/releases");
let html_url = v["html_url"].as_str().and_then(|u| safe_release_html_url(repo, u)).unwrap_or_else(|| releases.clone());
let prefix = download_prefix(repo);
let assets = v["assets"]
.as_array()
.map(|list| {
list.iter()
.filter_map(|a| {
let name = a["name"].as_str().filter(|n| valid_file_name(n))?.to_string();
let url = a["browser_download_url"].as_str().filter(|u| download_url_under(u, &prefix))?.to_string();
let url =
a["browser_download_url"].as_str().filter(|u| expected_download_url(repo, &tag, &name).as_deref() == Some(*u))?.to_string();
let sha256 = a["digest"].as_str().and_then(|d| d.strip_prefix("sha256:")).filter(|h| is_hex64(h)).map(str::to_ascii_lowercase);
Some(Asset { name, size: a["size"].as_u64().unwrap_or(0), url, sha256 })
})
@@ -219,18 +248,18 @@ fn release_from_value(v: &serde_json::Value, repo: &str) -> Result<Release, Stri
notes: v["body"].as_str().map(|b| truncate(b, MAX_NOTES)).unwrap_or_default(),
prerelease: v["prerelease"].as_bool().unwrap_or(false),
assets,
repo: repo.to_string(),
})
}
/// A release known only from its tag and `SHA256SUMS.txt` (the quota-free route: the
/// `releases/latest` redirect names the tag, and the sums list every file).
/// `releases/latest` redirect names the tag, and the sums list every file). Each link is
/// [`expected_download_url`], so it's the one the check before the download accepts; a file
/// without one is left out.
pub fn release_from_sums(repo: &str, tag: &str, sums: &[(String, String)]) -> Release {
let prefix = download_prefix(repo);
let mut assets: Vec<Asset> = sums
.iter()
.map(|(hash, name)| Asset { name: name.clone(), size: 0, url: format!("{prefix}{tag}/{name}"), sha256: Some(hash.clone()) })
.collect();
assets.push(Asset { name: "SHA256SUMS.txt".into(), size: 0, url: format!("{prefix}{tag}/SHA256SUMS.txt"), sha256: None });
let asset =
|name: &str, sha256: Option<String>| expected_download_url(repo, tag, name).map(|url| Asset { name: name.to_string(), size: 0, url, sha256 });
let assets: Vec<Asset> = sums.iter().filter_map(|(hash, name)| asset(name, Some(hash.clone()))).chain(asset("SHA256SUMS.txt", None)).collect();
Release {
tag: tag.to_string(),
version: version::parse(tag),
@@ -240,13 +269,25 @@ pub fn release_from_sums(repo: &str, tag: &str, sums: &[(String, String)]) -> Re
notes: String::new(),
prerelease: false,
assets,
repo: repo.to_string(),
}
}
/// The tag in a `https://github.com/<repo>/releases/tag/<tag>` redirect target.
pub fn tag_from_redirect(location: &str, repo: &str) -> Option<String> {
let rest = location.strip_prefix(&format!("https://github.com/{repo}/releases/tag/"))?;
(!rest.is_empty() && rest.len() <= 64 && rest.bytes().all(|b| b.is_ascii_alphanumeric() || b"._-+".contains(&b))).then(|| rest.to_string())
valid_tag(rest).then(|| rest.to_string())
}
/// A release tag we'd put in a URL, a file or the UI: up to 64 of `[A-Za-z0-9._+-]`, not
/// starting with a dot, so no separators, control or bidi characters and no `.`/`..`.
pub fn valid_tag(tag: &str) -> bool {
tag.len() <= 64 && !tag.starts_with('.') && plain_name(tag)
}
/// Non-empty and only `[A-Za-z0-9._+-]`, the characters every Craft release tag and asset uses.
fn plain_name(s: &str) -> bool {
!s.is_empty() && s.bytes().all(|b| b.is_ascii_alphanumeric() || b"._-+".contains(&b))
}
/// Reads `sha256sum` output: `<hex> <name>` (or `<hex> *<name>`) per line, one entry per name.
@@ -281,14 +322,19 @@ pub fn parse_sha256sums(text: &str) -> Vec<(String, String)> {
sums
}
/// A bare file name we'd be willing to save: no paths, no control characters.
/// A bare file name we'd be willing to save: only `[A-Za-z0-9._+-]` (so no path, escape, control
/// or bidi character), no leading or trailing dot, and not a name Windows reserves for a device
/// (`CON`, `nul.txt`, `COM1.msi`).
pub fn valid_file_name(name: &str) -> bool {
!name.is_empty()
&& name.len() <= 200
&& name != "."
&& name != ".."
&& !name.starts_with('.')
&& !name.chars().any(|c| c.is_control() || matches!(c, '/' | '\\' | ':' | '<' | '>' | '"' | '|' | '?' | '*'))
name.len() <= 200 && !name.starts_with('.') && !name.ends_with('.') && plain_name(name) && !windows_device(name)
}
/// Whether `name`'s part before the first dot is a Windows device name, which opens the device
/// instead of a file in any folder.
fn windows_device(name: &str) -> bool {
let stem = name.split('.').next().unwrap_or_default().to_ascii_uppercase();
matches!(stem.as_str(), "CON" | "PRN" | "AUX" | "NUL")
|| matches!(stem.as_bytes(), [b'C', b'O', b'M', b'1'..=b'9'] | [b'L', b'P', b'T', b'1'..=b'9'])
}
/// `s` cut to at most `max` bytes (on a char boundary) with an ellipsis, for text from a
@@ -297,11 +343,7 @@ pub(crate) fn truncate(s: &str, max: usize) -> String {
if s.len() <= max {
return s.to_string();
}
let mut end = max;
while !s.is_char_boundary(end) {
end -= 1;
}
format!("{}…", &s[..end])
format!("{}…", &s[..s.floor_char_boundary(max)])
}
#[cfg(test)]
@@ -370,6 +412,79 @@ mod tests {
assert_eq!(r.assets[0].size, 123);
}
/// #764: an API asset's link must be exactly this release's `<tag>/<name>`.
#[test]
fn a_download_link_must_be_its_releases_own() {
for url in [
// Another release of the same app (a rollback), or another file name.
"https://github.com/storytold/photocraft/releases/download/v0.4.0/photocraft-0.5.0-macos-universal.dmg",
"https://github.com/storytold/photocraft/releases/download/v0.5.0/photocraft-0.4.0-macos-universal.dmg",
"https://github.com/storytold/photocraft/releases/download/%2e%2e/%2E%2E/%2e%2e/%2e%2e/other/repo/releases/download/v1/photocraft-0.5.0-macos-universal.dmg",
// The right path on another host, over plain http, or behind a look-alike host.
"https://evil.example/storytold/photocraft/releases/download/v0.5.0/photocraft-0.5.0-macos-universal.dmg",
"http://github.com/storytold/photocraft/releases/download/v0.5.0/photocraft-0.5.0-macos-universal.dmg",
"https://github.com.evil.example/storytold/photocraft/releases/download/v0.5.0/photocraft-0.5.0-macos-universal.dmg",
"https://github.com@evil.example/storytold/photocraft/releases/download/v0.5.0/photocraft-0.5.0-macos-universal.dmg",
] {
assert!(parse_github_release(&api_json(url), REPO).unwrap().assets.is_empty(), "{url}");
}
assert_eq!(
expected_download_url(REPO, "v0.6.0-rc.1", "photocraft-0.6.0-rc.1-macos-universal.dmg").as_deref(),
Some("https://github.com/storytold/photocraft/releases/download/v0.6.0-rc.1/photocraft-0.6.0-rc.1-macos-universal.dmg")
);
for (tag, name) in [("", "x.dmg"), ("..", "x.dmg"), ("v1", ".."), ("v1", "a/b.dmg"), ("v1", "a%2fb.dmg"), ("v 1", "x.dmg"), ("v1", ".x.dmg")]
{
assert_eq!(expected_download_url(REPO, tag, name), None, "{tag:?} {name:?}");
}
}
/// Two entries for one asset name, one with the release's own link and one elsewhere: only the
/// release's own survives, in either order, so a later lookup by name can't pick the other.
/// Built from `SHA256SUMS.txt`, every entry for a repeated name gets that same single link.
#[test]
fn two_links_for_one_asset_keep_only_the_releases_own() {
let name = "photocraft-0.5.0-macos-universal.dmg";
let own = format!("https://github.com/storytold/photocraft/releases/download/v0.5.0/{name}");
let asset = |url: &str| format!(r#"{{"name":"{name}","size":123,"browser_download_url":"{url}","digest":"sha256:{HASH}"}}"#);
let elsewhere = "https://evil.example/storytold/photocraft/releases/download/v0.5.0/photocraft-0.5.0-macos-universal.dmg";
for (first, second) in [(elsewhere, own.as_str()), (own.as_str(), elsewhere)] {
let json = format!(
r#"{{"tag_name":"v0.5.0","name":"PhotoCraft v0.5.0","html_url":"https://github.com/storytold/photocraft/releases/tag/v0.5.0",
"published_at":"2026-10-08T14:52:50Z","prerelease":false,"body":"Notes","assets":[{},{}]}}"#,
asset(first),
asset(second)
);
let r = parse_github_release(&json, REPO).unwrap();
assert_eq!(r.assets.len(), 1, "{first} {second}");
assert_eq!(r.assets[0].url, own);
}
let r = release_from_sums(REPO, "v0.5.0", &[("a".repeat(64), name.to_string()), ("b".repeat(64), name.to_string())]);
let links: Vec<&str> = r.assets.iter().filter(|a| a.name == name).map(|a| a.url.as_str()).collect();
assert!(!links.is_empty() && links.iter().all(|url| *url == own), "{links:?}");
}
/// A `+` in the tag or name (semver build metadata) is `%2B` in GitHub's download links, but a
/// plain `+` in the asset's name and in the `releases/latest` redirect, as on bell-sw/Liberica's
/// releases. The API's assets, and those built from `SHA256SUMS.txt`, get the same link.
#[test]
fn a_plus_in_the_tag_is_percent_encoded_in_the_link() {
let name = "photocraft-1.0.0+build.1-linux-x86_64.AppImage";
let github = "https://github.com/storytold/photocraft/releases/download/v1.0.0%2Bbuild.1/photocraft-1.0.0%2Bbuild.1-linux-x86_64.AppImage";
let api = |url: &str| format!(r#"{{"tag_name":"v1.0.0+build.1","assets":[{{"name":"{name}","size":1,"browser_download_url":"{url}"}}]}}"#);
let r = parse_github_release(&api(github), REPO).unwrap();
assert_eq!((r.assets[0].name.as_str(), r.assets[0].url.as_str()), (name, github));
// GitHub never writes the `+` itself into the link.
assert!(parse_github_release(&api(&github.replace("%2B", "+")), REPO).unwrap().assets.is_empty());
let tag = tag_from_redirect("https://github.com/storytold/photocraft/releases/tag/v1.0.0+build.1", REPO).unwrap();
let r = release_from_sums(REPO, &tag, &[(HASH.to_string(), name.to_string())]);
assert_eq!(r.assets[0].url, github);
assert_eq!(r.sums_asset().unwrap().url, "https://github.com/storytold/photocraft/releases/download/v1.0.0%2Bbuild.1/SHA256SUMS.txt");
}
/// Links to other hosts or repositories, or that climb out with `..` or encoded separators,
/// are dropped.
#[test]
fn drops_downloads_that_point_elsewhere() {
for url in ["https://evil.example/photocraft.dmg", "https://github.com/storytold/other/releases/download/v1/x.dmg", "javascript:alert(1)"] {
@@ -405,6 +520,8 @@ mod tests {
}
}
/// A link that shares the download prefix as text but climbs into another repository with `..`
/// or an encoded separator or dot isn't under it.
#[test]
fn traversal_cannot_escape_the_repository() {
// Raw-prefix matches but the interpreted path climbs into another repository (CWE-180).
@@ -415,6 +532,9 @@ mod tests {
"https://github.com/storytold/photocraft/releases/download/v1/sub/x.dmg",
"https://github.com/storytold/photocraft/releases/download/v1%2f..%2f..%2fx.dmg",
"https://github.com/storytold/photocraft/releases/download/v1/x%2fy.dmg",
"https://github.com/storytold/photocraft/releases/download/%2e%2e/x.dmg",
"https://github.com/storytold/photocraft/releases/download/%2E%2E/x.dmg",
"https://github.com/storytold/photocraft/releases/download/v1/%2e%2e",
] {
let prefix = download_prefix(REPO);
assert!(!download_url_under(url, &prefix), "{url}");
@@ -439,6 +559,8 @@ mod tests {
assert_eq!(parse_github_release_list("[]", REPO, false).unwrap(), None);
}
/// `SHA256SUMS.txt` lines, the tag in a `releases/latest` redirect, and the release built from
/// both; malformed lines, traversing names and `.`/`..` tags are dropped.
#[test]
fn reads_checksums_and_redirects() {
let sums = parse_sha256sums(&format!("{HASH} photocraft-0.5.0-macos-universal.dmg\n{HASH} *b.zip\nnot a line\n{HASH} ../escape\n"));
@@ -446,6 +568,9 @@ mod tests {
assert_eq!(tag_from_redirect("https://github.com/storytold/photocraft/releases/tag/v0.5.0", REPO).as_deref(), Some("v0.5.0"));
assert_eq!(tag_from_redirect("https://github.com/storytold/photocraft/releases", REPO), None);
assert_eq!(tag_from_redirect("https://evil/storytold/photocraft/releases/tag/v1", REPO), None);
for dots in [".", ".."] {
assert_eq!(tag_from_redirect(&format!("https://github.com/storytold/photocraft/releases/tag/{dots}"), REPO), None);
}
let r = release_from_sums(REPO, "v0.5.0", &sums);
assert_eq!(r.assets[0].url, "https://github.com/storytold/photocraft/releases/download/v0.5.0/photocraft-0.5.0-macos-universal.dmg");
assert_eq!(r.version, Some(Version::new(0, 5, 0)));
@@ -484,11 +609,92 @@ mod tests {
assert!(sums.iter().enumerate().all(|(i, (h, n))| *h == HASH && *n == format!("f{}", i + 1)));
}
/// A name with a path, a hidden or reserved form, a control character or a percent-encoded
/// separator isn't a file name we'd save.
#[test]
fn file_names_cannot_escape() {
for bad in ["", "..", "a/b", "a\\b", ".hidden", "c:x", "x\u{0}"] {
for bad in [
"",
"..",
"a/b",
"a\\b",
".hidden",
"c:x",
"x\u{0}",
"%2e%2e%2fother",
"a%2Fb",
"a%5cb",
"x%2E%2E",
"a b.dmg",
"a#b",
"evil\u{202e}gpj.exe",
"x.msi.",
"CON",
"nul.txt",
"COM1.msi",
"lpt9",
"prn",
"aux.log",
"a".repeat(201).as_str(),
] {
assert!(!valid_file_name(bad), "{bad:?}");
}
assert!(valid_file_name("photocraft-0.5.0-linux-x86_64.AppImage"));
for good in [
"photocraft-0.5.0-linux-x86_64.AppImage",
"lightcraft_0.9.0_amd64.deb",
"x-1.0+git.tar.gz",
"console.zip",
"COM10.txt",
"COM0.msi",
"lpt0",
] {
assert!(valid_file_name(good), "{good:?}");
}
}
/// The tag rule the launcher's feed shares (craft-launcher#1024): plain characters, no
/// leading dot, at most 64 bytes, for the API's `tag_name`, a redirect and a download link.
#[test]
fn tags_are_plain() {
for good in ["v0.6.0", "v1.0.0-beta.2", "nightly-2026-10-29", "v1+build.7"] {
assert!(valid_tag(good), "{good:?}");
}
for bad in ["", ".", "..", ".v1", "v1/x", "v1\\x", "v1\nx", "v1\u{0}x", "v1\u{202e}", "v 1", "v1?x", "v".repeat(65).as_str()] {
assert!(!valid_tag(bad), "{bad:?}");
}
let api = r#"{"tag_name": "v1\u202egpj", "html_url": "", "assets": []}"#;
assert!(parse_github_release(api, REPO).is_err());
let prefix = download_prefix(REPO);
assert!(download_url_under(&format!("{prefix}v1/a.dmg"), &prefix));
assert!(!download_url_under(&format!("{prefix}v1\u{202e}/a.dmg"), &prefix), "an odd tag in a download link");
assert!(!download_url_under(&format!("{prefix}v1/a\u{202e}.dmg"), &prefix), "an odd name in a download link");
assert_eq!(tag_from_redirect(&format!("https://github.com/{REPO}/releases/tag/v1\u{202e}"), REPO), None);
}
/// A `+` matches written plainly or as `%2B`, either side; no other escape is read.
#[test]
fn a_plus_link_matches_either_way_and_nothing_else_does() {
let expected = expected_download_url(REPO, "v1.0.0+b.1", "photocraft-1.0.0+b.1-linux-x86_64.AppImage").unwrap();
assert!(same_download_url(&expected, &expected));
assert!(same_download_url(&expected.replace("%2B", "+"), &expected));
assert!(!same_download_url(&expected.replace("%2B", "%2b"), &expected));
assert!(!same_download_url(&expected.replace("v1.0.0", "v1.0.1"), &expected));
}
/// Both converters record the repository they read the release from.
#[test]
fn a_release_knows_its_repository() {
assert_eq!(release_from_sums(REPO, "v0.5.0", &[]).repo, REPO);
let api = r#"{"tag_name": "v0.5.0", "assets": []}"#;
assert_eq!(parse_github_release(api, REPO).unwrap().repo, REPO);
}
/// A `SHA256SUMS.txt` line whose name hides a traversal is dropped, so `release_from_sums`
/// never builds a link from it; the ordinary line beside it is kept.
#[test]
fn checksum_names_with_encoded_separators_make_no_download() {
let hash = "a".repeat(64);
let sums = parse_sha256sums(&format!("{hash} %2e%2e%2f%2e%2e%2fother%2frepo%2fx.dmg\n{hash} photocraft-0.5.0-macos-universal.dmg\n"));
assert_eq!(sums.iter().map(|(_, n)| n.as_str()).collect::<Vec<_>>(), ["photocraft-0.5.0-macos-universal.dmg"]);
}
}
crates/update/src/self_update.rs
+132 -3
View File
@@ -575,6 +575,11 @@ fn exe_after_msi(app: &App, registered: bool, exe: &Path) -> PathBuf {
.unwrap_or_else(|| exe.to_path_buf())
}
/// How the running copy at `exe` was installed on Linux, or `None` when it isn't this app's to
/// update: a Flatpak release bundle (per user or system-wide), an AppImage, the deb/rpm/Arch
/// package at `/usr/bin/<id>`, or a standalone copy outside the package managers' folders
/// ([`linux_location`]). A `/usr/bin/<id>` that none of those package databases claims is also
/// `None`: it sits in a system folder, so it isn't treated as a standalone copy.
#[cfg(target_os = "linux")]
fn detect(app: &App, exe: &Path) -> Option<Install> {
if process::in_sandbox() {
@@ -589,6 +594,11 @@ fn detect(app: &App, exe: &Path) -> Option<Install> {
if let Some(appimage) = std::env::var_os("APPIMAGE") {
return Some(Install::AppImage(PathBuf::from(appimage)));
}
match linux_location(app, exe) {
LinuxLocation::PackagePath => {}
LinuxLocation::System => return None,
LinuxLocation::Elsewhere => return exe.is_file().then(|| Install::Standalone(exe.to_path_buf())),
}
let t = std::time::Duration::from_secs(10);
if process::capture("dpkg-query", &["-W", "-f", "${db:Status-Status}", app.id], t).is_some_and(|s| s.trim() == "installed") {
return Some(Install::Package(PackageKind::Deb, exe.to_path_buf()));
@@ -601,12 +611,47 @@ fn detect(app: &App, exe: &Path) -> Option<Install> {
if crate::detect::linux::has_pacman_db() && process::capture("pacman", &["-Q", &app.bundle_id()], t).is_some() {
return Some(Install::Package(PackageKind::ArchPkg, exe.to_path_buf()));
}
if exe.is_file() {
return Some(Install::Standalone(exe.to_path_buf()));
}
// `/usr/bin/<id>` that no package database claims, e.g. copied there by hand or put there by a
// package whose name isn't the app's id. Replacing it would write over a system folder without
// a package to do it, so it doesn't update itself.
log::info!("{}: {} belongs to no deb, rpm or Arch package, so it doesn't update itself", app.name, exe.display());
None
}
/// Where a Linux executable outside a Flatpak or AppImage runs from, for [`detect`].
#[cfg_attr(not(target_os = "linux"), allow(dead_code))]
#[derive(Debug, PartialEq, Eq)]
enum LinuxLocation {
/// `/usr/bin/<id>`, where the release's deb, rpm and Arch packages put it. Only this path is
/// checked against the package databases: a copy elsewhere isn't the package's even when the
/// package is installed too.
PackagePath,
/// Anywhere else under a folder a package manager owns (`/usr` but not `/usr/local`, `/opt`,
/// `/bin`, `/sbin`, `/nix`, `/snap` or `/var/lib/snapd/snap` (Snap on Fedora and Arch), Guix's
/// `/gnu`, or a Homebrew `.linuxbrew` prefix): a
/// distribution, AUR, Nix, Snap, Guix or Homebrew copy. It's theirs to update, and replacing
/// it in place would write over a file the package manager owns (or fail on a read-only
/// store), as on macOS ([`macos_package_manager_owns`]).
System,
/// Anywhere else, such as a folder in the user's home: a standalone copy that replaces itself.
Elsewhere,
}
/// Which [`LinuxLocation`] `exe` is in, from its path alone.
#[cfg_attr(not(target_os = "linux"), allow(dead_code))]
fn linux_location(app: &App, exe: &Path) -> LinuxLocation {
if exe == Path::new("/usr/bin").join(app.id) {
LinuxLocation::PackagePath
} else if (!exe.starts_with("/usr/local") && ["/usr", "/opt", "/bin", "/sbin", "/nix", "/snap", "/var/lib/snapd/snap", "/gnu"].iter().any(|dir| exe.starts_with(dir)))
// Homebrew on Linux: /home/linuxbrew/.linuxbrew, or ~/.linuxbrew for one user
|| exe.components().any(|c| c.as_os_str() == ".linuxbrew")
{
LinuxLocation::System
} else {
LinuxLocation::Elsewhere
}
}
/// Where this sandbox's Flatpak copy is installed, from `app-path` in `/.flatpak-info`.
#[cfg(target_os = "linux")]
#[derive(Clone, Copy, Debug, PartialEq, Eq)]
@@ -750,6 +795,7 @@ pub fn prepare_update(
.ok_or_else(|| format!("{} {} has no {} for {}", app.name, release.version_label(), install.kind().label(), platform.label()))?;
let asset = asset.clone();
crate::install::bind_artifact_version(app, release, &asset)?;
crate::install::check_download_link(app, release, &asset)?;
// Nothing is downloaded for a copy that can't install it.
#[cfg(windows)]
windows_precheck(app, install, paths)?;
@@ -4233,6 +4279,89 @@ mod tests {
assert!(result.unwrap_err().contains("is not newer than currently running version"));
}
/// Only `/usr/bin/<id>` is checked against the package databases, as before the launcher's
/// self-update moved here; another copy under a system folder (an AUR build, say) is left to
/// its package manager, and only a copy outside them (`/usr/local` included, which no package
/// manager owns) replaces itself.
#[test]
fn only_the_packages_own_path_is_a_package_and_system_folders_are_left_alone() {
let app = &crate::suite::LAUNCHER;
assert_eq!(linux_location(app, Path::new("/usr/bin/artcraft-launcher")), LinuxLocation::PackagePath);
for system in [
"/usr/bin/artcraft-launcher-bin",
"/nix/store/abc-artcraft-launcher/bin/artcraft-launcher",
"/opt/artcraft-launcher/artcraft-launcher",
"/bin/artcraft-launcher",
"/gnu/store/abc-artcraft-launcher/bin/artcraft-launcher",
"/snap/artcraft-launcher/12/bin/artcraft-launcher",
"/var/lib/snapd/snap/artcraft-launcher/12/bin/artcraft-launcher",
"/home/linuxbrew/.linuxbrew/Cellar/artcraft-launcher/1.0/bin/artcraft-launcher",
"/home/me/.linuxbrew/bin/artcraft-launcher",
] {
assert_eq!(linux_location(app, Path::new(system)), LinuxLocation::System, "{system}");
}
for own in [
"/usr/local/bin/artcraft-launcher",
"/home/me/bin/artcraft-launcher",
"/home/me/.local/bin/artcraft-launcher",
"/usrlocal/artcraft-launcher",
"/var/lib/snapdx/artcraft-launcher",
] {
assert_eq!(linux_location(app, Path::new(own)), LinuxLocation::Elsewhere, "{own}");
}
}
/// storytold/craft-launcher#764: a self-update downloads only its own release's link. One into
/// another app's releases, with the right tag and name, is refused before anything is fetched.
#[test]
fn update_refuses_a_link_that_is_not_its_releases_own() {
let dir = tempfile::tempdir().unwrap();
let paths = Paths::under(dir.path());
let http = Http::new(crate::TEST_AGENT).unwrap();
let mut phase = |_| ();
let last = || Ok(());
let stop = Stop::default();
let name = "artcraft-launcher-0.3.0-linux-x86_64.AppImage";
let release = Release {
version: version::parse("0.3.0"),
tag: "v0.3.0".into(),
assets: vec![crate::release::Asset {
name: name.into(),
size: 1,
url: format!("https://github.com/storytold/photocraft/releases/download/v0.3.0/{name}"),
sha256: None,
}],
..Release::default()
};
let install = Install::AppImage(dir.path().join("launcher.AppImage"));
use crate::{Arch, Os};
let platform = Platform { os: Os::Linux, arch: Arch::X64 };
let result = update(&crate::suite::LAUNCHER, &install, &release, platform, &http, &paths, &stop, &mut phase, &last, Some("0.2.0"));
assert!(result.unwrap_err().contains("isn't the one its release publishes"));
}
/// A launcher release whose tag has a `+` (semver build metadata) has `%2B` in its links.
/// The self-update accepts them and gets as far as the download, which the cancelled stop ends
/// before anything is fetched.
#[test]
fn update_accepts_a_plus_release_up_to_the_download() {
let dir = tempfile::tempdir().unwrap();
let paths = Paths::under(dir.path());
let http = Http::new(crate::TEST_AGENT).unwrap();
let mut phase = |_| ();
let last = || Ok(());
let stop = Stop::default();
stop.cancel();
let app = &crate::suite::LAUNCHER;
let name = "artcraft-launcher-0.3.0+build.1-linux-x86_64.AppImage";
let release = crate::release::release_from_sums(app.repo, "v0.3.0+build.1", &[("a".repeat(64), name.to_string())]);
let install = Install::AppImage(dir.path().join("launcher.AppImage"));
use crate::{Arch, Os};
let platform = Platform { os: Os::Linux, arch: Arch::X64 };
let result = update(app, &install, &release, platform, &http, &paths, &stop, &mut phase, &last, Some("0.2.0"));
assert_eq!(result.unwrap_err(), crate::protocol::CANCELLED);
}
#[cfg(target_os = "linux")]
#[test]
fn parses_flatpak_metadata_for_restart() {
crates/update/src/source.rs
+2 -1
View File
@@ -130,7 +130,8 @@ impl GitHubSource {
{
return Ok(Answer { latest: LatestRelease::Found(r.clone()), source: Source::GitHubPages });
}
let sums_url = format!("{}{tag}/SHA256SUMS.txt", release::download_prefix(repo));
// The same link `release_from_sums` stores and the check before the download accepts.
let sums_url = release::expected_download_url(repo, &tag, "SHA256SUMS.txt").ok_or("the latest release's tag isn't one we'd fetch")?;
let sums = self.http.get_text(&sums_url)?.map(|t| release::parse_sha256sums(&t)).unwrap_or_default();
let r = release::release_from_sums(repo, &tag, &sums);
self.store(repo, Entry { etag: None, release: Some(r.clone()), prereleases: false, fetched_at: crate::now() });