collective
Browse / github.com/storytold/craft-libs / patch
craft-update: accept exactly one download link per asset (craft-launcher#764); keep the Linux self-update off other copies
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 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'
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- smittie2000 (imported from pull request #388, open)
- Base
bd97cb760c36c4398def923817702a7e7f3096af- Size
- +292 −26 in 3 files
- Review
- pending
- Shared
- 2026-10-28
- Id
21899ccd74827c7165cc95f3
How it went
No other operator has reported using it yet.
Diff
raw · crates/update/src/install.rs crates/update/src/release.rs crates/update/src/self_update.rs
@@ -58,6 +58,11 @@ pub fn install( type RecordCommit<'a> = Option<&'a mut dyn FnMut(&Record) -> 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,@@ -90,6 +95,7 @@ pub fn install_at( return Err(format!("Cannot update {} to {}: installed version {} is newer", app.name, release.version_label(), inst_v)); } log::info!("{}: starting {} to {} ({})", app.name, if existing.is_some() { "update" } else { "install" }, release.version_label(), kind.label());+ check_download_link(app, release, asset)?; let file = download(release, asset, job)?; log::info!("{}: download ready ({}), running installer...", app.name, file.display()); // Past this point the install can't be interrupted halfway.@@ -170,13 +176,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()); }@@ -187,6 +195,23 @@ 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 app's own GitHub download for this release's tag and this asset's name, 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 `local-mirror` build also+/// accepts the developer's loopback mirror, whose links the feed parser has already confined; so+/// do this crate's tests, whose downloads come from a server on 127.0.0.1.+pub(crate) fn check_download_link(app: &App, release: &Release, asset: &Asset) -> Result<(), String> {+ let own = |a: &Asset| {+ ((cfg!(feature = "local-mirror") || cfg!(test)) && crate::http::is_loopback(&a.url))+ || release::expected_download_url(app.repo, &release.tag, &a.name).as_deref() == Some(a.url.as_str())+ };+ 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(@@ -2595,6 +2620,8 @@ mod tests { 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@@ -2640,6 +2667,55 @@ mod tests { ..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 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()); } #[cfg(target_os = "linux")]@@ -115,6 +115,20 @@ 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` when the tag or name holds anything but ASCII letters,+/// digits and `._+-`, which no Craft App release uses and which github.com could 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 plain = |s: &str, max: usize| {+ !s.is_empty() && s.len() <= max && s != "." && s != ".." && s.bytes().all(|b| b.is_ascii_alphanumeric() || b"._+-".contains(&b))+ };+ let encoded = |s: &str| s.replace('+', "%2B");+ (plain(tag, 64) && plain(name, 200) && valid_file_name(name)).then(|| format!("{}{}/{}", download_prefix(repo), encoded(tag), encoded(name)))+}+ /// Whether `url` is a download that truly belongs to `repo`. /// /// A raw `starts_with(prefix)` check is not enough: a value such as@@ -151,11 +165,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`).@@ -187,6 +202,9 @@ 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)) });@@ -194,14 +212,14 @@ fn release_from_value(v: &serde_json::Value, repo: &str) -> Result<Release, Stri let tag = v["tag_name"].as_str().filter(|t| !t.is_empty() && t.len() <= 64).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 }) })@@ -221,14 +239,13 @@ fn release_from_value(v: &serde_json::Value, repo: &str) -> Result<Release, Stri } /// 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),@@ -244,7 +261,8 @@ pub fn release_from_sums(repo: &str, tag: &str, sums: &[(String, String)]) -> Re /// 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())+ (!rest.is_empty() && rest != "." && rest != ".." && rest.len() <= 64 && rest.bytes().all(|b| b.is_ascii_alphanumeric() || b"._-+".contains(&b)))+ .then(|| rest.to_string()) } /// Reads `sha256sum` output: `<hex> <name>` (or `<hex> *<name>`) per line.@@ -265,6 +283,7 @@ pub fn valid_file_name(name: &str) -> bool { && name != "." && name != ".." && !name.starts_with('.')+ && !contains_encoded_separator(name) && !name.chars().any(|c| c.is_control() || matches!(c, '/' | '\\' | ':' | '<' | '>' | '"' | '|' | '?' | '*')) } @@ -274,11 +293,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)]@@ -347,6 +362,48 @@ 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",+ ] {+ 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:?}");+ }+ }++ /// 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)"] {@@ -382,6 +439,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).@@ -392,6 +451,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}");@@ -416,6 +478,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"));@@ -423,16 +487,30 @@ 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))); } + /// 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"] { assert!(!valid_file_name(bad), "{bad:?}"); } assert!(valid_file_name("photocraft-0.5.0-linux-x86_64.AppImage")); }++ /// 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"]);+ } }@@ -214,6 +214,9 @@ fn msi_install_dir(app: &App) -> Option<PathBuf> { (!dir.is_empty()).then(|| PathBuf::from(dir)) } +/// How the running copy at `exe` was installed on Linux, or `None` when it isn't this app's to+/// update: a Flatpak bundle installed per user, an AppImage, the deb/rpm/Arch package at+/// `/usr/bin/<id>`, or a standalone copy outside the package managers' folders ([`linux_location`]). #[cfg(target_os = "linux")] fn detect(app: &App, exe: &Path) -> Option<Install> { if process::in_sandbox() {@@ -226,6 +229,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()));@@ -238,12 +246,38 @@ 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()));- } 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`): a distribution, AUR, Nix or Snap 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).+ 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"].iter().any(|dir| exe.starts_with(dir)) {+ LinuxLocation::System+ } else {+ LinuxLocation::Elsewhere+ }+}+ /// Whether this sandbox runs the per-user installation's copy, from `app-path` in `/.flatpak-info`. #[cfg(target_os = "linux")] fn is_user_install() -> bool {@@ -313,6 +347,7 @@ pub fn 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)?; let mut job = Job { http, paths, stop, phase, install_locations: &Default::default(), replace: false }; let file = install::download(release, &asset, &mut job)?; #[cfg(windows)]@@ -1738,6 +1773,83 @@ 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",+ ] {+ 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",+ ] {+ 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() {