craft-update: tests for the review's cases (other host, two links, running app, bad checksum)

FredApps asked on #388 for tests of two links for one asset, a link to
another host, a failing commit, a newer installed copy and a running app.
The failing commit and the newer copy were already tested on main
(portable.rs, install_refuses_to_downgrade_when_target_release_is_older).

- release.rs: a_download_link_must_be_its_releases_own also refuses the
  right path on another host, over http, and behind look-alike hosts.
  New two_links_for_one_asset_keep_only_the_releases_own.
- install.rs: install_refuses_links_off_the_releases_own_before_downloading
  covers other hosts and a SHA256SUMS.txt linked elsewhere (the
  sums_asset branch of check_download_link).
  install_refuses_while_the_installed_copy_is_running uses the test binary
  on Windows and process.rs's RunningProbe on Unix, where pgrep -f matches
  the command line.
  a_download_that_fails_its_checksum_replaces_nothing: the installed copy
  keeps its bytes, the install never starts, nothing is committed or kept.
- process.rs: RunningProbe is pub(crate) so install.rs's test can use it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
smittie2000andClaude Opus 5.5 committed 2026-11-01 14:45:08 +02:00
1 parent c4530bb26e
commit 3ad90741b7
3 files changed
+139 -3

No files matched your search

+104
View File
@@ -2844,6 +2844,110 @@ mod tests {
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")]
#[test]
fn package_kinds_order_matches_launcher_format() {
+4 -3
View File
@@ -351,16 +351,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())
+31
View File
@@ -370,6 +370,11 @@ mod tests {
"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}");
}
@@ -383,6 +388,32 @@ mod tests {
}
}
/// 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.