Fix signed revision updates without duplicate release downloads

.github/workflows/release.yml

@@ -9,14 +9,18 @@ on:
         required: true
         default: v0.8.3
       revision:
-        description: New build revision (2 or higher)
+        description: Build revision (1 rebuilds a draft baseline)
         required: true
         default: '2'
       platform:
         description: Platform receiving the hotfix
         type: choice
-        options: [windows, linux, macos]
+        options: [all, windows, linux, macos]
         required: true
+      publish_baseline:
+        description: Publish the baseline after rebuilding (leave off to keep it draft)
+        type: boolean
+        default: false
       source_ref:
         description: Commit SHA containing the hotfix
         required: true
@@ -46,9 +50,12 @@ jobs:
           REVISION_SOURCE_REF: ${{ inputs.source_ref }}
         run: |
           test -n "$CRAFT_UPDATE_SIGNING_KEY" || { echo "Configure CRAFT_UPDATE_SIGNING_KEY before publishing" >&2; exit 1; }
-          if [ "${{ github.event_name }}" = workflow_dispatch ] && [ "$CRAFT_BUILD_REVISION" = 1 ]; then
-            echo "Manual hotfix publication requires revision 2 or higher" >&2
-            exit 1
+          if [ "${{ github.event_name }}" = workflow_dispatch ]; then
+            case "$REVISION_SOURCE_REF" in *[!0-9a-f]*|'') echo "Use an immutable commit SHA" >&2; exit 1;; esac
+            test "${#REVISION_SOURCE_REF}" -eq 40
+            if [ "$CRAFT_BUILD_REVISION" = 1 ]; then
+              test "${{ inputs.platform }}" = all || { echo "Baseline rebuilds must include all platforms" >&2; exit 1; }
+            fi
           fi
           if [ "$CRAFT_BUILD_REVISION" != 1 ]; then
             case "$CRAFT_BUILD_REVISION" in *[!0-9]*|'') exit 1;; esac
@@ -246,50 +253,43 @@ jobs:
         run: |
           cd packages
           checksum=SHA256SUMS.txt
-          if [ "$CRAFT_BUILD_REVISION" != 1 ]; then
-            checksum="SHA256SUMS-${REVISION_PLATFORM}-Rev-${CRAFT_BUILD_REVISION}.txt"
-          fi
-          # Compatibility downloads change during a hotfix. Only immutable
-          # revision packages belong in permanent checksum lists.
-          pattern="Craft Apps Manager-${CRAFT_RELEASE_TAG#v}-*"
-          if [ "$CRAFT_BUILD_REVISION" != 1 ]; then
-            pattern="Craft Apps Manager-${CRAFT_RELEASE_TAG#v}-Rev-${CRAFT_BUILD_REVISION}-*"
-          fi
-          find . -maxdepth 1 -type f -name "$pattern" -print0 | sort -z | xargs -0 -r sha256sum > "$checksum"
+          python3 - <<'PY' > "$checksum"
+          import json
+          manifest = json.loads(json.loads(open("updates-v1.json").read())["payload"])
+          for package in sorted(manifest["packages"], key=lambda p: p["asset"]):
+              print(f"{package['sha256']}  {package['asset']}")
+          PY
           cat "$checksum"
       - name: Release notes from the changelog
         run: |
           python3 scripts/validate-release.py --tag "$CRAFT_RELEASE_TAG" --notes-file notes.md
       - name: Publish packages before revision metadata
         env:
           GH_TOKEN: ${{ github.token }}
+          PUBLISH_BASELINE: ${{ github.event_name == 'workflow_dispatch' && inputs.publish_baseline }}
         run: |
           releases=$(gh api "repos/$GITHUB_REPOSITORY/releases?per_page=100")
           existing=$(printf '%s' "$releases" | jq -c --arg tag "$CRAFT_RELEASE_TAG" '.[] | select(.tag_name == $tag)')
           manifest="$RUNNER_TEMP/updates-v1.json"
           mv packages/updates-v1.json "$manifest"
           if [ "$CRAFT_BUILD_REVISION" != 1 ]; then
             test -n "$existing" && test "$(printf '%s' "$existing" | jq -r .draft)" = false
-            # Revision packages are immutable. Only compatibility filenames are
-            # replaced; their original revision-1 snapshots remain available.
+            # Replace affected platform downloads, then publish signed metadata.
             for package in packages/*; do
-              name=$(basename "$package")
-              case "$name" in "Craft Apps Manager-"*.zip|"Craft Apps Manager-"*.msi|"Craft Apps Manager-"*.deb|"Craft Apps Manager-"*.rpm|"Craft Apps Manager-"*.pkg.tar.zst|*-r*.zip|*-r*.msi|*-r*.deb|*-r*.rpm|*-r*.pkg.tar.zst)
-                  old_digest=$(printf '%s' "$existing" | jq -r --arg name "$name" '.assets[] | select(.name == $name) | .digest')
-                  if [ -n "$old_digest" ]; then
-                    local_digest="sha256:$(sha256sum "$package" | cut -d ' ' -f1)"
-                    test "$old_digest" = "$local_digest" || { echo "Existing immutable revision asset differs: $name" >&2; exit 1; }
-                  else
-                    gh release upload "$CRAFT_RELEASE_TAG" "$package"
-                  fi;;
-                *) gh release upload "$CRAFT_RELEASE_TAG" "$package" --clobber;; esac
+              gh release upload "$CRAFT_RELEASE_TAG" "$package" --clobber
             done
             gh release upload "$CRAFT_RELEASE_TAG" "$manifest" --clobber
           elif [ -n "$existing" ]; then
             test "$(printf '%s' "$existing" | jq -r .draft)" = true || { echo "Refusing to replace a published baseline release" >&2; exit 1; }
             gh release upload "$CRAFT_RELEASE_TAG" packages/* --clobber
             gh release upload "$CRAFT_RELEASE_TAG" "$manifest" --clobber
-            gh release edit "$CRAFT_RELEASE_TAG" --title "Craft Apps Manager ${CRAFT_RELEASE_TAG#v}" --notes-file notes.md --draft=false
+            gh release edit "$CRAFT_RELEASE_TAG" --title "Craft Apps Manager ${CRAFT_RELEASE_TAG#v}" --notes-file notes.md
+            if [ "$PUBLISH_BASELINE" = true ]; then
+              gh release edit "$CRAFT_RELEASE_TAG" --draft=false
+            fi
           else
-            gh release create "$CRAFT_RELEASE_TAG" packages/* "$manifest" --verify-tag --title "Craft Apps Manager ${CRAFT_RELEASE_TAG#v}" --notes-file notes.md
+            gh release create "$CRAFT_RELEASE_TAG" packages/* "$manifest" --verify-tag --title "Craft Apps Manager ${CRAFT_RELEASE_TAG#v}" --notes-file notes.md --draft
+            if [ "${{ github.event_name }}" = push ] || [ "$PUBLISH_BASELINE" = true ]; then
+              gh release edit "$CRAFT_RELEASE_TAG" --draft=false
+            fi
           fi

scripts/revision-manifest.py

@@ -42,14 +42,6 @@ def compatibility_name(version, platform, architecture, kind):
     return f"craft-apps-manager-{version}-1-x86_64.pkg.tar.zst"
 
 
-def revision_name(version, revision, platform, architecture, kind):
-    suffix = ".pkg.tar.zst" if kind == "arch" else "." + kind
-    label = f"-Rev-{revision}" if revision >= 2 else ""
-    # Friendly immutable names differ from the legacy compatibility aliases,
-    # which can be refreshed without replacing the original baseline package.
-    return f"Craft Apps Manager-{version}{label}-{platform}-{architecture}{suffix}"
-
-
 def native_version(file, kind):
     if kind == "deb":
         return subprocess.check_output(["dpkg-deb", "-f", str(file), "Version"], text=True).strip()
@@ -100,13 +92,10 @@ def main():
         old = packages.get(identity)
         if old and args.revision <= old["revision"]:
             raise ValueError("Revision must advance for every affected target")
-        canonical = args.packages / revision_name(args.version, args.revision, *identity)
-        shutil.copyfile(file, canonical)
-        packages[identity] = dict(platform=identity[0], architecture=identity[1], format=identity[2], revision=args.revision, asset=canonical.name, size=canonical.stat().st_size, sha256=hashlib.sha256(canonical.read_bytes()).hexdigest(), package_version=native_version(canonical, identity[2]))
         legacy = args.packages / compatibility_name(args.version, *identity)
         if identity[2] == "deb" and args.revision > 1:
-            # Pre-0.8.3 updaters require exact Version metadata. This compatibility
-            # package carries the same revision executable with the old metadata.
+            # Older managers require exact Version metadata; sign this same
+            # compatible package for newer managers as well.
             work = args.packages / ("compat-deb-" + identity[1])
             subprocess.run(["dpkg-deb", "-R", str(file), str(work)], check=True)
             control = work / "DEBIAN/control"
@@ -115,6 +104,9 @@ def main():
             shutil.rmtree(work)
         elif file != legacy:
             shutil.copyfile(file, legacy)
+        if file != legacy:
+            file.unlink()
+        packages[identity] = dict(platform=identity[0], architecture=identity[1], format=identity[2], revision=args.revision, asset=legacy.name, size=legacy.stat().st_size, sha256=hashlib.sha256(legacy.read_bytes()).hexdigest(), package_version=native_version(legacy, identity[2]))
     if not seen:
         raise ValueError("No packages to publish")
     payload = json.dumps(dict(schema=1, repository=args.repository, version=args.version, expires=int(time.time()) + 365 * 86400, packages=list(packages.values())), separators=(",", ":"))

scripts/test_revision_manifest.py

@@ -1,7 +1,6 @@
 import json
 import hashlib
 import os
-import runpy
 from pathlib import Path
 import shutil
 import subprocess
@@ -45,13 +44,13 @@ def baseline(self):
         self.assertEqual(result.returncode, 0, result.stderr)
         return folder
 
-    def test_baseline_keeps_legacy_downloads_and_creates_signed_immutable_copies(self):
+    def test_baseline_signs_only_legacy_downloads(self):
         folder = self.baseline()
         manifest = self.read_manifest(folder)
         self.assertEqual(len(manifest["packages"]), 3)
         for package in manifest["packages"]:
             self.assertEqual(package["revision"], 1)
-            self.assertEqual(package["asset"], f"Craft Apps Manager-0.8.3-{package['platform']}-{package['architecture']}.zip")
+            self.assertEqual(package["asset"], f"Craft-Apps-Manager-0.8.3-{package['platform']}-{package['architecture']}.zip")
             self.assertNotIn("Rev", package["asset"])
             self.assertTrue((folder / package["asset"]).exists())
         self.assertTrue((folder / "Craft-Apps-Manager-0.8.3-linux-x64.zip").exists())
@@ -68,22 +67,15 @@ def test_linux_hotfix_preserves_other_targets_and_refreshes_legacy_download(self
         revisions = {p["platform"]: p["revision"] for p in manifest["packages"]}
         self.assertEqual(revisions, {"windows": 1, "linux": 2, "macos": 1})
         self.assertEqual(legacy.read_bytes(), b"revision 2 fixture")
-        self.assertEqual((baseline / "Craft Apps Manager-0.8.3-linux-x64.zip").read_bytes(), b"revision 1 fixture")
+        self.assertEqual((baseline / "Craft-Apps-Manager-0.8.3-linux-x64.zip").read_bytes(), b"revision 1 fixture")
         linux = next(p for p in manifest["packages"] if p["platform"] == "linux")
-        self.assertEqual(linux["asset"], "Craft Apps Manager-0.8.3-Rev-2-linux-x64.zip")
+        self.assertEqual(linux["asset"], "Craft-Apps-Manager-0.8.3-linux-x64.zip")
+        self.assertEqual(len(list(folder.glob("*.zip"))), 1)
+        self.assertEqual(linux["sha256"], hashlib.sha256(legacy.read_bytes()).hexdigest())
 
-    def test_revision_names_cover_all_package_formats(self):
-        name = runpy.run_path(str(self.script))["revision_name"]
-        for platform, architecture, kind, extension in [
-            ("windows", "x64", "zip", ".zip"),
-            ("windows", "x86", "msi", ".msi"),
-            ("macos", "universal", "zip", ".zip"),
-            ("linux", "x64", "deb", ".deb"),
-            ("linux", "x86", "rpm", ".rpm"),
-            ("linux", "x64", "arch", ".pkg.tar.zst"),
-        ]:
-            self.assertEqual(name("0.8.3", 1, platform, architecture, kind), f"Craft Apps Manager-0.8.3-{platform}-{architecture}{extension}")
-            self.assertEqual(name("0.8.3", 2, platform, architecture, kind), f"Craft Apps Manager-0.8.3-Rev-2-{platform}-{architecture}{extension}")
+    def test_baseline_has_no_duplicate_packages(self):
+        folder = self.baseline()
+        self.assertEqual(len(list(folder.glob("*.zip"))), 3)
 
     def test_nonadvancing_revision_and_tampered_previous_manifest_are_rejected(self):
         baseline = self.baseline()
@@ -124,15 +116,34 @@ def test_debian_legacy_download_contains_latest_payload_with_compatible_metadata
         self.assertEqual(result.returncode, 0, result.stderr)
         entry = next(p for p in self.read_manifest(folder)["packages"] if p["format"] == "deb")
         canonical = folder / entry["asset"]
-        self.assertEqual(entry["package_version"], "0.8.3-2")
+        self.assertEqual(entry["package_version"], "0.8.3")
         self.assertEqual(entry["sha256"], hashlib.sha256(canonical.read_bytes()).hexdigest())
         compatibility = folder / "craft-apps-manager_0.8.3_amd64.deb"
-        for label, file, expected in [("canonical", canonical, "0.8.3-2"), ("legacy", compatibility, "0.8.3")]:
+        for label, file, expected in [("canonical", canonical, "0.8.3"), ("legacy", compatibility, "0.8.3")]:
             version = subprocess.check_output(["dpkg-deb", "-f", str(file), "Version"], text=True).strip()
             self.assertEqual(version, expected)
             destination = self.root / label
             subprocess.run(["dpkg-deb", "-x", str(file), str(destination)], check=True)
             self.assertEqual((destination / "usr/bin/craft-apps-manager").read_bytes(), payload)
+        self.assertEqual(len(list(folder.glob("*.deb"))), 1)
+        if shutil.which("apt-get"):
+            # Simulate both upgrade routes against an isolated package database.
+            # No administrator access or real package installation is involved.
+            status = self.root / "status"
+            for installed in ["0.8.2", "0.8.3"]:
+                status.write_text(
+                    "Package: craft-apps-manager\nStatus: install ok installed\n"
+                    f"Version: {installed}\nArchitecture: amd64\n"
+                    "Maintainer: Test <test@example.com>\nDescription: Fixture\n\n"
+                )
+                result = subprocess.run([
+                    "apt-get", "--simulate", "-o", f"Dir::State::status={status}",
+                    "-o", "Debug::NoLocking=true", "install", "--reinstall",
+                    str(compatibility.resolve())
+                ], capture_output=True, text=True)
+                self.assertEqual(result.returncode, 0, result.stderr)
+                self.assertIn("Inst craft-apps-manager", result.stdout)
+
 
 
 if __name__ == "__main__":

src/network.rs

@@ -305,7 +305,7 @@ impl Network {
         Ok(())
     }
     pub fn asset(&self, a: &Asset, dest: &Path, job: &Job) -> Result<()> {
-        self.download(&a.browser_download_url, dest, job)?;
+        self.download(&asset_download_url(a)?, dest, job)?;
         job.check()?;
         job.stage(
             "Verifying checksum",
@@ -315,6 +315,18 @@ impl Network {
         verify_asset(dest, a)
     }
 }
+/// A replaced GitHub asset must not reuse a cached download redirect from the
+/// previous bytes. The published digest versions the URL without extra files.
+pub(crate) fn asset_download_url(asset: &Asset) -> Result<String> {
+    let mut url = reqwest::Url::parse(&asset.browser_download_url)?;
+    if url.host_str() == Some("github.com") && url.path().contains("/releases/download/") {
+        if let Some(digest) = &asset.digest {
+            url.query_pairs_mut().append_pair("asset_digest", digest);
+        }
+    }
+    Ok(url.into())
+}
+
 pub fn verify_asset(p: &Path, a: &Asset) -> Result<()> {
     if fs::metadata(p)?.len() != a.size {
         bail!("Download size mismatch");
@@ -337,6 +349,42 @@ pub fn verify_asset(p: &Path, a: &Asset) -> Result<()> {
 mod token_tests {
     use super::*;
 
+    #[test]
+    fn replaced_release_assets_use_distinct_urls_without_changing_unverified_urls() {
+        let mut asset = Asset {
+            name: "manager.zip".into(),
+            size: 1,
+            digest: Some(format!("sha256:{}", "ab".repeat(32))),
+            browser_download_url:
+                "https://github.com/owner/manager/releases/download/v0.8.3/manager.zip?download=1"
+                    .into(),
+        };
+        let first = asset_download_url(&asset).unwrap();
+        asset.digest = Some(format!("sha256:{}", "cd".repeat(32)));
+        let second = asset_download_url(&asset).unwrap();
+        assert_ne!(first, second);
+        let url = reqwest::Url::parse(&second).unwrap();
+        assert_eq!(url.host_str(), Some("github.com"));
+        assert!(url
+            .query_pairs()
+            .any(|(key, value)| key == "download" && value == "1"));
+        assert!(url
+            .query_pairs()
+            .any(|(key, value)| key == "asset_digest"
+                && Some(value.as_ref()) == asset.digest.as_deref()));
+        asset.digest = None;
+        assert_eq!(
+            asset_download_url(&asset).unwrap(),
+            asset.browser_download_url
+        );
+        asset.browser_download_url = "https://example.com/file?download=1".into();
+        asset.digest = Some("sha256:fixture".into());
+        assert_eq!(
+            asset_download_url(&asset).unwrap(),
+            asset.browser_download_url
+        );
+    }
+
     #[test]
     fn failed_download_publication_removes_partial_file() {
         use std::net::TcpListener;

src/self_update.rs

@@ -171,7 +171,7 @@ pub fn check(paths: &Paths) -> Result<Option<Available>> {
             "Invalid revision manifest location or size"
         );
         let envelope: crate::update_revisions::Envelope =
-            network.json_bounded(&asset.browser_download_url, 256 * 1024)?;
+            network.json_bounded(&crate::network::asset_download_url(asset)?, 256 * 1024)?;
         let version = release.tag_name.trim_start_matches('v');
         let manifest = crate::update_revisions::verify(
             &envelope,
@@ -197,16 +197,20 @@ pub fn check(paths: &Paths) -> Result<Option<Available>> {
         };
         #[cfg(not(target_os = "linux"))]
         let format = if installed_with_msi() { "msi" } else { "zip" };
-        // A manifest can advance while the five-minute release cache still
-        // lists older assets. Refresh that snapshot once when this is observed.
+        // Hotfixes replace downloads under the same compatible filenames.
+        // Refresh a cached release when its asset metadata no longer matches
+        // the signed manifest; never weaken verification to accept stale data.
         if manifest.packages.iter().any(|package| {
             package.platform == crate::model::release_os()
                 && package.architecture == architecture
                 && package.format == format
-                && !release
-                    .assets
-                    .iter()
-                    .any(|asset| asset.name == package.asset)
+                && !release.assets.iter().any(|asset| {
+                    asset.name == package.asset
+                        && asset.size == package.size
+                        && asset.digest.as_ref().is_none_or(|digest| {
+                            digest.eq_ignore_ascii_case(&format!("sha256:{}", package.sha256))
+                        })
+                })
         }) {
             release = network.json_fresh(&url)?;
         }