fix: Flatpak review fixes, and add the signature lookaside for signed bundles

Review of #1147:
- A failed OCI migration precheck (no version in the remote's AppStream data, polkit refusing
  remote-add, offline) now falls back to the bundle update instead of failing it; only errors
  once the reinstall starts are reported. The ref must list exactly the selected version.
- `flatpak list` names a named installation `system (<name>)`; the parser keeps `<name>`, and
  one packages::flatpak_scope builds the --user/--system/--installation= argument everywhere.
- A repository-managed Flatpak on a pinned or nightly track gets the "update with flatpak"
  hint instead of a plain `flatpak update`, and status no longer offers a GitHub version for it.
- The migration refuses while the app runs; the origin check after the reinstall can't be
  cancelled; is_flatpark_origin matches only `flatpark`; settings_install keeps its doc; the
  no-op oci_migration Cargo features are gone.

Signed bundles (storytold/craft-common-actions#129): a signed bundle's origin has the release
key but no signature lookaside, which `flatpak build-bundle` can't record, so its first update
from the ArtCraft OCI repository fails with "no valid signatures". The updater then sets
--signature-lookaside=https://getartcraft.com/signatures on that origin once and retries;
any other origin or error is left alone.
This commit is contained in:
Dylan M. Taylor committed 2026-11-02 21:29:10 -05:00
1 parent b1c3e8025d
commit e5d3039747
8 files changed
+147 -107

No files matched your search

-2
View File
@@ -13,8 +13,6 @@ publish = false
workspace = true
[features]
# Compatibility feature; migration now requires the saved Advanced setting.
oci_migration = ["artcraft-launcher-native/oci_migration"]
# Development only: install from `cargo xtask local serve` (see the README).
local-mirror = ["artcraft-launcher-native/local-mirror"]
+5
View File
@@ -128,6 +128,8 @@ pub fn status(installed: Option<&Installed>, latest: Latest<'_>) -> AppStatus {
}
(Some(_), Latest::Pending | Latest::Failed | Latest::NoRelease) => AppStatus::Installed,
(Some(inst), Latest::Found { version: latest, .. }) => match (latest, inst.version.as_ref()) {
// A repository-managed copy updates to what its repository has, which GitHub doesn't say.
(Some(l), Some(i)) if version::is_update(l, i) && inst.externally_managed => AppStatus::Installed,
(Some(l), Some(i)) if version::is_update(l, i) => AppStatus::UpdateAvailable,
(Some(_), Some(_)) => AppStatus::UpToDate,
// Version unknown on one side: we can't promise "up to date".
@@ -166,6 +168,9 @@ mod tests {
assert_eq!(status(Some(&inst("0.6.0")), found(true)), AppStatus::UpToDate);
assert_eq!(status(Some(&inst("0.4.0")), Latest::Failed), AppStatus::Installed);
assert_eq!(status(Some(&inst("garbage")), found(true)), AppStatus::Installed);
let repo = Installed { externally_managed: true, ..inst("0.4.0") };
assert_eq!(status(Some(&repo), found(true)), AppStatus::Installed);
assert_eq!(status(Some(&Installed { externally_managed: true, ..inst("0.5.0") }), found(true)), AppStatus::UpToDate);
assert!(AppStatus::UpdateAvailable.is_installed() && !AppStatus::Available.is_installed());
}
}
-2
View File
@@ -11,8 +11,6 @@ publish = false
workspace = true
[features]
# Compatibility feature; migration now requires the saved Advanced setting.
oci_migration = []
# Lets ARTCRAFT_LAUNCHER_DOWNLOAD_BASE point the launcher at a developer's own loopback mirror
# (`cargo xtask local serve`). Development only: release builds never enable it, so a shipped
# launcher can't be steered away from GitHub's releases by an environment variable.
+6 -6
View File
@@ -516,23 +516,23 @@ fn run_install(
&& inst.source == artcraft_launcher_core::InstallSource::Flatpak
&& inst.externally_managed
{
// `flatpak update` only follows the repository's latest build, never a pinned or nightly track.
if !matches!(options.versions.get(app.id), None | Some(VersionTrack::Stable)) {
return Err(install::managed_elsewhere(app, inst, false).unwrap_or_else(|| "Update this app through its Flatpak repository".into()));
}
#[cfg(target_os = "linux")]
{
shared.emit(Event::Progress { app: app.id.to_string(), phase: JobPhase::Installing });
let mut phase = |p| shared.emit(Event::Progress { app: app.id.to_string(), phase: p });
let default_id = app.bundle_id();
let target_id = inst.package.as_deref().unwrap_or(&default_id);
let scope = match inst.installation.as_deref() {
Some("user") => "--user".to_string(),
Some("system") | None => "--system".to_string(),
Some(name) => format!("--installation={name}"),
};
let scope = crate::packages::flatpak_scope(inst.installation.as_deref());
let deployed_commit = || {
crate::process::capture_flatpak(&["info", &scope, "--show-commit", target_id], std::time::Duration::from_secs(15))
.filter(|value| !value.trim().is_empty())
};
let before = deployed_commit();
install::linux::update_flatpak_app(app, target_id, inst.installation.as_deref(), stop, &mut phase)?;
install::linux::update_flatpak_app(app, target_id, inst.installation.as_deref(), inst.origin.as_deref(), stop, &mut phase)?;
let after = deployed_commit();
return Ok(match (before, after) {
(Some(before), Some(after)) if before.trim() == after.trim() => format!("{} is already up to date", app.name),
+3 -18
View File
@@ -507,19 +507,8 @@ pub mod linux {
.iter_mut()
.filter(|(id, v, _, _)| v.is_empty() && artcraft_launcher_core::SUITE.iter().any(|a| a.bundle_ids().any(|b| b == *id)))
{
let custom_scope;
let scope_arg = if !scope.is_empty() {
vec!["--user"]
} else {
match installation.as_deref() {
Some("system") | None => vec!["--system"],
Some(named) => {
custom_scope = format!("--installation={named}");
vec![custom_scope.as_str()]
}
}
};
let info_args = [&["info"][..], &scope_arg, &[&arch, id]].concat();
let scope_arg = packages::flatpak_scope(if scope.is_empty() { installation.as_deref() } else { Some("user") });
let info_args = ["info", &scope_arg, &arch, id];
if let Some(v) = crate::process::capture_flatpak(&info_args, t).as_deref().and_then(packages::parse_flatpak_info) {
*version = v;
}
@@ -624,11 +613,7 @@ pub mod linux {
if packages::is_flatpark_origin(origin) {
return true;
}
let scope = match p.installation.as_deref() {
Some("user") => "--user".to_string(),
Some("system") | None => "--system".to_string(),
Some(name) => format!("--installation={name}"),
};
let scope = packages::flatpak_scope(p.installation.as_deref());
crate::process::capture_flatpak(&["remotes", &scope, "--show-disabled", "--columns=name,url"], Duration::from_secs(10))
.is_none_or(|remotes| packages::remote_manages_flatpak(origin, &remotes))
});
+75 -57
View File
@@ -636,11 +636,7 @@ fn managed_elsewhere_with(app: &CraftApp, installed: &Installed, uninstall: bool
InstallSource::Flatpak if !uninstall && installed.externally_managed => {
let default_id = app.bundle_id();
let pkg = installed.package.as_deref().unwrap_or(&default_id);
let scope = match installed.installation.as_deref() {
Some("user") => "--user".to_string(),
Some("system") | None => "--system".to_string(),
Some(name) => format!("--installation={name}"),
};
let scope = crate::packages::flatpak_scope(installed.installation.as_deref());
Some(format!("{} was installed from a Flatpak repository. Update it with flatpak update {scope} {pkg}", app.name))
}
InstallSource::Binary => Some(format!("{} was installed outside the launcher. Update or remove it the way it was installed.", app.name)),
@@ -1231,7 +1227,8 @@ pub(crate) mod linux {
}
}
/// Opt-in OCI migration; only an absent ref permits bundle fallback.
/// Opt-in OCI migration. Ok(false) keeps the bundle: anything that fails before the reinstall
/// starts (no matching ref or version, polkit refusal, offline) falls back to the bundle update.
pub fn migrate_bundle_to_oci(app: &CraftApp, installed: &Installed, expected: &super::version::Version, stop: &Stop) -> Result<bool, String> {
if installed.source != InstallSource::Flatpak || installed.externally_managed || !app.repo.starts_with("storytold/") {
return Ok(false);
@@ -1240,13 +1237,10 @@ pub(crate) mod linux {
if !installed.origin.as_deref().is_some_and(|origin| origin.ends_with("-origin")) {
return Ok(false);
}
let id = installed.package.as_deref().ok_or("Flatpak installation has no package ID")?;
let scope = match installed.installation.as_deref() {
Some("user") => "--user".to_string(),
Some("system") => "--system".to_string(),
Some(name) => format!("--installation={name}"),
None => return Err("Flatpak installation scope is unknown".into()),
let (Some(id), Some(installation)) = (installed.package.as_deref(), installed.installation.as_deref()) else {
return Ok(false);
};
let scope = crate::packages::flatpak_scope(Some(installation));
// `cancellable` is false for steps that must not stop part-way: killing flatpak-spawn
// doesn't wait for the host transaction, and `--reinstall` removes the old ref first.
let flatpak = |args: &[&str], cancellable: bool| -> Result<String, String> {
@@ -1265,47 +1259,58 @@ pub(crate) mod linux {
Ok(String::from_utf8_lossy(&output.stdout).into_owned())
};
let run = |args: &[&str]| flatpak(args, true);
let reference = run(&["info", "--show-ref", id])?;
let reference = reference.trim();
if !reference.starts_with(&format!("app/{id}/")) || !reference.ends_with("/stable") {
return Ok(false);
}
let remotes = run(&["remotes", "--show-disabled", "--columns=name,url"])?;
let remote = remotes.lines().find_map(|line| {
let (name, _) = line.split_once('\t')?;
crate::packages::is_artcraft_remote(name, line).then(|| name.to_string())
});
let remote = match remote {
Some(remote) => remote,
None => {
if remotes.lines().any(|line| line.split('\t').next() == Some("artcraft-oci")) {
return Err("The artcraft-oci remote points to a different repository".into());
}
// Another app's migration may add the remote first; the URL is checked below.
run(&["remote-add", "--if-not-exists", "artcraft-oci", "https://getartcraft.com/artcraft-oci.flatpakrepo"])?;
let remotes = run(&["remotes", "--show-disabled", "--columns=name,url"])?;
if !crate::packages::is_artcraft_remote("artcraft-oci", &remotes) {
return Err("The ArtCraft OCI descriptor configured an unexpected repository".into());
}
"artcraft-oci".to_string()
let precheck = || -> Result<Option<(String, String)>, String> {
let reference = run(&["info", "--show-ref", id])?.trim().to_string();
if !reference.starts_with(&format!("app/{id}/")) || !reference.ends_with("/stable") {
return Ok(None);
}
let remotes = run(&["remotes", "--show-disabled", "--columns=name,url"])?;
let remote = remotes.lines().find_map(|line| {
let (name, _) = line.split_once('\t')?;
crate::packages::is_artcraft_remote(name, line).then(|| name.to_string())
});
let remote = match remote {
Some(remote) => remote,
None => {
if remotes.lines().any(|line| line.split('\t').next() == Some("artcraft-oci")) {
return Ok(None);
}
// Another app's migration may add the remote first; the URL is checked below.
run(&["remote-add", "--if-not-exists", "artcraft-oci", "https://getartcraft.com/artcraft-oci.flatpakrepo"])?;
let remotes = run(&["remotes", "--show-disabled", "--columns=name,url"])?;
if !crate::packages::is_artcraft_remote("artcraft-oci", &remotes) {
return Ok(None);
}
"artcraft-oci".to_string()
}
};
// The Version column comes from the remote's AppStream catalog, which Flatpak only fetches
// on demand; without a refresh a new remote lists refs with empty versions.
run(&["update", "--appstream", &remote])?;
let refs = run(&["remote-ls", "--app", "--columns=ref,version", &remote])?;
Ok(crate::packages::oci_ref_available(&reference, expected, &refs).then_some((remote, reference)))
};
// The Version column comes from the remote's AppStream catalog, which Flatpak only fetches
// on demand; without a refresh a new remote lists refs with empty versions.
run(&["update", "--appstream", &remote])?;
let refs = run(&["remote-ls", "--app", "--columns=ref,version", &remote])?;
if !crate::packages::oci_ref_available(reference, expected, &refs)? {
return Ok(false);
let (remote, reference) = match precheck() {
Ok(Some(found)) => found,
Err(e) if e == CANCELLED => return Err(e),
Ok(None) | Err(_) => return Ok(false),
};
// `install_at` won't replace a running app either; the reinstall can't be cancelled.
if process::capture_flatpak(&["ps", "--columns=application"], std::time::Duration::from_secs(10))
.is_some_and(|ps| ps.lines().any(|line| line.trim() == id))
{
return Err(format!("{} is running. Quit it, then try again.", app.name));
}
stop.check()?;
if let Err(e) = flatpak(&["install", "--reinstall", "-y", "--noninteractive", &remote, reference], false) {
if let Err(e) = flatpak(&["install", "--reinstall", "-y", "--noninteractive", &remote, &reference], false) {
// The bundle origin has no repository to restore from, so point at the bundle itself.
if flatpak(&["info", id], false).is_err() {
return Err(format!("{e}. {} is no longer installed; reinstall it from the .flatpak bundle at {}", app.name, app.releases_url()));
}
return Err(e);
}
let origin = run(&["info", "--show-origin", id])?;
// Past the reinstall, a cancel would misreport a finished migration.
let origin = flatpak(&["info", "--show-origin", id], false)?;
if origin.trim() != remote {
return Err("Flatpak did not switch the installed app to the ArtCraft OCI remote".into());
}
@@ -1317,27 +1322,40 @@ pub(crate) mod linux {
app: &CraftApp,
id: &str,
installation: Option<&str>,
origin: Option<&str>,
stop: &Stop,
phase: &mut dyn FnMut(JobPhase),
) -> Result<(), String> {
phase(JobPhase::Installing);
stop.check()?;
let scope_arg = match installation {
Some("user") => "--user".to_string(),
Some("system") | None => "--system".to_string(),
Some(named) => format!("--installation={named}"),
};
let scope_arg = crate::packages::flatpak_scope(installation);
// Let Flatpak's system helper authorize system updates through polkit.
// watch-bus also terminates the host command if the sandbox proxy exits.
let mut cmd = if process::in_sandbox() {
let mut command = process::command("flatpak-spawn");
command.args(["--host", "--watch-bus", "flatpak"]);
command
} else {
process::command("flatpak")
let flatpak = |args: &[&str]| {
let mut cmd = if process::in_sandbox() {
let mut command = process::command("flatpak-spawn");
command.args(["--host", "--watch-bus", "flatpak"]);
command
} else {
process::command("flatpak")
};
cmd.args(args);
process::cancellable_output(cmd, || stop.check())
};
cmd.args(["update", &scope_arg, "-y", "--noninteractive", id]);
let output = process::cancellable_output(cmd, || stop.check())?;
let update = ["update", &scope_arg, "-y", "--noninteractive", id];
let mut output = flatpak(&update)?;
// A signed bundle's origin can't carry the ArtCraft OCI signature lookaside, so its first
// update from the repository finds no signature: add the lookaside once and retry.
if !output.status.success()
&& let Some(origin) = origin
&& let Some(remotes) = process::capture_flatpak(&["remotes", &scope_arg, "--columns=name,url"], std::time::Duration::from_secs(10))
&& crate::packages::needs_signature_lookaside(&String::from_utf8_lossy(&output.stderr), origin, &remotes)
{
let lookaside = format!("--signature-lookaside={}", crate::packages::ARTCRAFT_SIGNATURE_LOOKASIDE);
if flatpak(&["remote-modify", &scope_arg, &lookaside, origin])?.status.success() {
output = flatpak(&update)?;
}
}
if output.status.success() {
return Ok(());
}
@@ -3507,7 +3525,7 @@ mod tests {
let stop = Stop::default();
stop.cancel();
let mut phase = |_| ();
assert_eq!(linux::update_flatpak_app(app, "ai.storyteller.photocraft", Some("user"), &stop, &mut phase), Err(CANCELLED.into()));
assert_eq!(linux::update_flatpak_app(app, "ai.storyteller.photocraft", Some("user"), None, &stop, &mut phase), Err(CANCELLED.into()));
}
#[test]
+56 -21
View File
@@ -63,7 +63,12 @@ pub fn parse_flatpak_list_with_origin(text: &str) -> Vec<(String, String, Option
}
let version = f.next().unwrap_or("").trim().to_string();
let origin = f.next().map(str::trim).filter(|o| !o.is_empty()).map(ToString::to_string);
let installation = f.next().map(str::trim).filter(|i| !i.is_empty()).map(ToString::to_string);
// A named installation lists as `system (<name>)`, but `--installation=` takes the bare name.
let installation = f
.next()
.map(str::trim)
.filter(|i| !i.is_empty())
.map(|i| i.strip_prefix("system (").and_then(|n| n.strip_suffix(')')).unwrap_or(i).to_string());
Some((id.to_string(), version, origin, installation))
})
.collect()
@@ -77,8 +82,16 @@ pub fn parse_flatpak_list(text: &str) -> Vec<(String, String)> {
/// Whether an origin string indicates an install from FlatPark.
pub fn is_flatpark_origin(origin: &str) -> bool {
let o = origin.trim();
o.eq_ignore_ascii_case("flatpark") || o.to_ascii_lowercase().contains("flatpark")
origin.trim().eq_ignore_ascii_case("flatpark")
}
/// The `flatpak` argument that selects an installation (`user`, `system`, or a named one).
pub fn flatpak_scope(installation: Option<&str>) -> String {
match installation {
Some("user") => "--user".into(),
Some("system") | None => "--system".into(),
Some(name) => format!("--installation={name}"),
}
}
/// The `Version:` line of `flatpak info <id>`.
@@ -224,7 +237,7 @@ mod tests {
#[test]
fn parses_flatpak_list_with_origin_and_spots_flatpark() {
let text = "ai.storyteller.photocraft\t0.19.0\tflatpark\tuser\nai.storyteller.cadcraft\t0.16.0\tcadcraft-origin\tsystem\nai.storyteller.wordcraft\t0.14.0\t\t\n";
let text = "ai.storyteller.photocraft\t0.19.0\tflatpark\tuser\nai.storyteller.cadcraft\t0.16.0\tcadcraft-origin\tsystem\nai.storyteller.wordcraft\t0.14.0\t\t\nai.storyteller.filmcraft\t0.3.0\tflatpark\tsystem (extra)\n";
let list = parse_flatpak_list_with_origin(text);
assert_eq!(
list,
@@ -232,11 +245,15 @@ mod tests {
("ai.storyteller.photocraft".into(), "0.19.0".into(), Some("flatpark".into()), Some("user".into())),
("ai.storyteller.cadcraft".into(), "0.16.0".into(), Some("cadcraft-origin".into()), Some("system".into())),
("ai.storyteller.wordcraft".into(), "0.14.0".into(), None, None),
("ai.storyteller.filmcraft".into(), "0.3.0".into(), Some("flatpark".into()), Some("extra".into())),
]
);
assert_eq!(flatpak_scope(Some("extra")), "--installation=extra");
assert_eq!(flatpak_scope(Some("user")), "--user");
assert_eq!(flatpak_scope(None), "--system");
assert!(is_flatpark_origin("flatpark"));
assert!(is_flatpark_origin("FlatPark"));
assert!(is_flatpark_origin("flatpark-origin"));
assert!(!is_flatpark_origin("flatpark-origin"));
assert!(!is_flatpark_origin("cadcraft-origin"));
assert!(!is_flatpark_origin("flathub"));
assert!(!is_flatpark_origin(""));
@@ -253,6 +270,16 @@ pub fn is_artcraft_remote(origin: &str, remotes: &str) -> bool {
})
}
/// Where Flatpak finds the ArtCraft OCI images' signatures (artcraft-oci.flatpakrepo's
/// `SignatureLookaside`), redirected to each release's `.oci.sig` asset.
pub const ARTCRAFT_SIGNATURE_LOOKASIDE: &str = "https://getartcraft.com/signatures";
/// Did an update fail only because `origin`, an ArtCraft OCI remote, has no signature lookaside
/// (a signed bundle's origin, which `flatpak build-bundle` can't give one)?
pub fn needs_signature_lookaside(stderr: &str, origin: &str, remotes: &str) -> bool {
stderr.contains(&format!("{origin}: no valid signatures for ")) && is_artcraft_remote(origin, remotes)
}
#[cfg(test)]
mod remote_url_tests {
use super::*;
@@ -272,23 +299,29 @@ mod remote_url_tests {
}
assert!(!is_artcraft_remote("other", "custom\toci+https://getartcraft.com"));
}
#[test]
fn only_an_artcraft_origin_missing_its_signatures_gets_the_lookaside() {
// Flatpak 1.18's error for a verified OCI remote with no lookaside.
let stderr = "error: Failed to update ai.storyteller.photocraft: photocraft-origin: no valid signatures for \
ghcr.io/storytold/photocraft@sha256:968d75c9d16d5f7d089775f14a3b418d72a4c70d86986811ee2dd818da956b92";
let remotes = "photocraft-origin\toci+https://getartcraft.com\nflathub\thttps://dl.flathub.org/repo/\n";
assert!(needs_signature_lookaside(stderr, "photocraft-origin", remotes));
// Another remote, another origin's failure, or another error: leave the remote alone.
assert!(!needs_signature_lookaside(stderr, "photocraft-origin", "photocraft-origin\thttps://example.org\n"));
assert!(!needs_signature_lookaside(stderr, "flathub", remotes));
assert!(!needs_signature_lookaside("error: Server returned status 404", "photocraft-origin", remotes));
}
}
/// A missing ref permits bundle fallback; an unknown or different version must not replace it.
/// Whether the remote lists `reference` at exactly `expected`; a missing ref, or one with no or
/// another version (the AppStream catalog may list none), keeps the bundle.
#[cfg(target_os = "linux")]
pub(crate) fn oci_ref_available(reference: &str, expected: &version::Version, listing: &str) -> Result<bool, String> {
for line in listing.lines() {
pub(crate) fn oci_ref_available(reference: &str, expected: &version::Version, listing: &str) -> bool {
listing.lines().any(|line| {
let mut fields = line.split('\t');
if fields.next().map(str::trim) != Some(reference) {
continue;
}
let candidate = fields.next().and_then(|value| version::parse(value.trim()));
if candidate.as_ref() != Some(expected) {
return Err("The ArtCraft OCI repository does not yet provide the selected version".into());
}
return Ok(true);
}
Ok(false)
fields.next().map(str::trim) == Some(reference) && fields.next().and_then(|v| version::parse(v.trim())).as_ref() == Some(expected)
})
}
#[cfg(all(test, target_os = "linux"))]
@@ -298,10 +331,12 @@ mod migration_tests {
fn migration_requires_the_exact_ref_and_selected_version() {
let reference = "app/ai.storyteller.photocraft/x86_64/stable";
let expected = version::parse("1.2.3").unwrap();
assert_eq!(oci_ref_available(reference, &expected, &format!("{reference}\t1.2.3")), Ok(true));
assert_eq!(oci_ref_available(reference, &expected, "app/ai.storyteller.photocraft/aarch64/stable\t1.2.3"), Ok(false));
assert!(oci_ref_available(reference, &expected, &format!("{reference}\t1.2.3")));
assert!(!oci_ref_available(reference, &expected, "app/ai.storyteller.photocraft/aarch64/stable\t1.2.3"));
// The live remote prints no version column at all.
assert!(!oci_ref_available(reference, &expected, reference));
for value in ["", "1.2.2", "1.2.4", "unknown"] {
assert!(oci_ref_available(reference, &expected, &format!("{reference}\t{value}")).is_err());
assert!(!oci_ref_available(reference, &expected, &format!("{reference}\t{value}")));
}
}
}
+2 -1
View File
@@ -202,7 +202,7 @@ impl LauncherUi {
});
}
/// Where apps go and in what format, for the desktop builds that manage installs.
/// Opt-in Linux behaviour that is still experimental.
fn settings_advanced(&mut self, ui: &mut Ui) {
if !self.caps.manage_apps || !self.platform.is_some_and(|p| p.os == Os::Linux) {
return;
@@ -214,6 +214,7 @@ impl LauncherUi {
});
}
/// Where apps go and in what format, for the desktop builds that manage installs.
fn settings_install(&mut self, ui: &mut Ui, host: &mut dyn Host) {
if !self.caps.manage_apps {
return;