diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a52eada..de07604 100644 --- a/.github/workflows/release.yml +++ b/.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,16 +253,12 @@ 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: | @@ -263,6 +266,7 @@ jobs: - 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)') @@ -270,26 +274,22 @@ jobs: 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 diff --git a/scripts/revision-manifest.py b/scripts/revision-manifest.py index 63b30a2..c258aa8 100644 --- a/scripts/revision-manifest.py +++ b/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=(",", ":")) diff --git a/scripts/test_revision_manifest.py b/scripts/test_revision_manifest.py index 1ca3cfd..9a052a4 100644 --- a/scripts/test_revision_manifest.py +++ b/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 \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__": diff --git a/src/network.rs b/src/network.rs index afcd60b..bbcb168 100644 --- a/src/network.rs +++ b/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 { + 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; diff --git a/src/self_update.rs b/src/self_update.rs index 7f0abe8..8ca6c4a 100644 --- a/src/self_update.rs +++ b/src/self_update.rs @@ -171,7 +171,7 @@ pub fn check(paths: &Paths) -> Result> { "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> { }; #[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)?; } diff --git a/src/self_update_linux.rs b/src/self_update_linux.rs index 88fc1c3..b3d4eb0 100644 --- a/src/self_update_linux.rs +++ b/src/self_update_linux.rs @@ -94,6 +94,16 @@ fn create_stage_root(paths: &Paths) -> Result { Ok(paths.at("runtime/self-update")) } +fn install_command(kind: PackageKind) -> &'static [&'static str] { + match kind { + // Compatible Debian downloads retain the release version even when + // their executable advances to a newer signed build revision. + PackageKind::Debian => &["/usr/bin/apt-get", "install", "--reinstall", "-y"], + PackageKind::Rpm => &["/usr/bin/dnf", "install", "-y"], + PackageKind::Arch => &["/usr/bin/pacman", "-U", "--noconfirm"], + } +} + pub(super) fn prepare(paths: &Paths, available: &Available, job: &Job) -> Result { use std::os::unix::fs::DirBuilderExt; let kind = package_kind()?; @@ -125,11 +135,7 @@ pub(super) fn prepare(paths: &Paths, available: &Available, job: &Job) -> Result job.check()?; job.stage("Installing package", None, "Approve the administrator prompt. The package manager must finish before the manager restarts."); let log = fs::File::create(stage.join("install.log"))?; - let args = match kind { - PackageKind::Debian => ["/usr/bin/apt-get", "install", "-y"], - PackageKind::Rpm => ["/usr/bin/dnf", "install", "-y"], - PackageKind::Arch => ["/usr/bin/pacman", "-U", "--noconfirm"], - }; + let args = install_command(kind); let status = Command::new("pkexec") .args(args) .arg(&package) @@ -201,6 +207,21 @@ pub(super) fn prepare(paths: &Paths, available: &Available, job: &Job) -> Result mod tests { use super::*; #[test] + fn debian_revisions_reinstall_while_other_package_managers_keep_upgrade_commands() { + assert_eq!( + install_command(PackageKind::Debian), + ["/usr/bin/apt-get", "install", "--reinstall", "-y"] + ); + assert_eq!( + install_command(PackageKind::Rpm), + ["/usr/bin/dnf", "install", "-y"] + ); + assert_eq!( + install_command(PackageKind::Arch), + ["/usr/bin/pacman", "-U", "--noconfirm"] + ); + } + #[test] fn staging_works_in_a_fresh_library_and_rejects_links() { let root = std::env::temp_dir().join(format!("craft-native-stage-{}", uuid::Uuid::new_v4())); diff --git a/src/update_revisions.rs b/src/update_revisions.rs index f1423b9..5d45fce 100644 --- a/src/update_revisions.rs +++ b/src/update_revisions.rs @@ -183,9 +183,9 @@ mod tests { assert_eq!(display_version("0.8.3", 2), "0.8.3 ยท Rev. 2"); } #[test] - fn friendly_revision_filenames_are_selected_from_signed_metadata() { + fn legacy_compatible_filenames_are_selected_from_signed_metadata() { let (mut manifest, mut release) = fixture(); - let name = "Craft Apps Manager-0.8.3-Rev-2-linux-x64.zip"; + let name = "Craft-Apps-Manager-0.8.3-linux-x64.zip"; manifest.packages[0].asset = name.into(); release.assets[0].name = name.into(); release.assets[0].browser_download_url = format!( @@ -197,6 +197,14 @@ mod tests { .unwrap(); assert_eq!(offered.asset.name, name); assert_eq!(offered.revision, 2); + assert_eq!( + select(&manifest, &release, ("linux", "x64", "zip"), ("0.8.2", 1)) + .unwrap() + .unwrap() + .asset + .name, + name + ); } fn hex_text(bytes: &[u8]) -> String { bytes.iter().map(|byte| format!("{byte:02x}")).collect()