craft-update: accept exactly one download link per asset (craft-launcher#764)

craft-update was extracted from ArtCraft Launcher (#311) with the launcher's release.rs as it
was, so it has the hole craft-launcher#764 reports: github.com resolves `..` and `%2e%2e`
segments before routing, and `download_url_under` only rejected literal `..` and `%2f`/`%5c`.
This ports storytold/craft-launcher#921:

- release::expected_download_url(repo, tag, name): `<prefix><tag>/<name>`, tag and name
  limited to ASCII letters, digits and `._+-`. release_from_value keeps an asset only when its
  link equals it, which also ties the link to its own release.
- contains_encoded_separator rejects `%2e` too, and valid_file_name rejects percent-encoded
  separators and dots, so release_from_sums can't build a link from `%2e%2e%2fother`.
- install::check_download_link, called just before the download in install_at and in
  self_update::update. The self-update path doesn't go through install_at, so without the
  second call every app's self-update would skip the check.
- truncate uses floor_char_boundary; bind_artifact_version uses split_once.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

crates/update/src/install.rs

@@ -90,6 +90,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,7 +171,7 @@ 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) {
+    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("");
@@ -187,6 +188,19 @@ 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. 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 mirrored = (cfg!(feature = "local-mirror") || cfg!(test)) && crate::http::is_loopback(&asset.url);
+    if mirrored || release::expected_download_url(app.repo, &release.tag, &asset.name).as_deref() == Some(asset.url.as_str()) {
+        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(
@@ -2640,6 +2654,19 @@ 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());
     }
 
     #[cfg(target_os = "linux")]

crates/update/src/release.rs

@@ -115,6 +115,17 @@ 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.
+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))
+    };
+    (plain(tag, 64) && plain(name, 200) && valid_file_name(name)).then(|| format!("{}{tag}/{name}", download_prefix(repo)))
+}
+
 /// 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 +162,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 +199,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 +209,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 })
                 })
@@ -265,6 +280,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 +290,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 +359,29 @@ 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:?}");
+        }
+    }
+
+    /// 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)"] {
@@ -392,6 +427,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}");
@@ -428,11 +466,22 @@ mod tests {
         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"]);
+    }
 }

crates/update/src/self_update.rs

@@ -313,6 +313,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 +1739,35 @@ mod tests {
         assert!(result.unwrap_err().contains("is not newer than currently running version"));
     }
 
+    /// 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"));
+    }
+
     #[cfg(target_os = "linux")]
     #[test]
     fn parses_flatpak_metadata_for_restart() {