fix: prevent stale catalog updates and redundant container recreation

core/archipelago/src/container/app_catalog.rs

@@ -400,7 +400,7 @@ pub fn available_update_for_app(app_id: &str, running_image: &str) -> Option<Str
     }
     if let Some(catalog_image) = catalog_primary_image(app_id) {
         // Catalog covers this app with a concrete image -> authoritative.
-        return crate::container::image_versions::available_update_for_images(
+        return crate::container::image_versions::available_catalog_update_for_images(
             &catalog_image,
             running_image,
         );

core/archipelago/src/container/image_versions.rs

@@ -100,6 +100,12 @@ fn parse_image_versions(content: &str) -> HashMap<String, String> {
 
         // Match VAR="value" or VAR=value
         if let Some((key, val)) = parse_assignment(line) {
+            // Read a self-default assignment without evaluating shell code.
+            let default_prefix = format!("${{{key}:-");
+            let val = val
+                .strip_prefix(&default_prefix)
+                .and_then(|v| v.strip_suffix('}'))
+                .unwrap_or(val);
             let expanded = val.replace("$ARCHY_REGISTRY", &registry);
             if key == "ARCHY_REGISTRY" {
                 registry = expanded.clone();
@@ -205,48 +211,71 @@ pub fn available_update_for_app(app_id: &str, running_image: &str) -> Option<Str
 }
 
 pub fn available_update_for_images(pinned: &str, running_image: &str) -> Option<String> {
-    let pinned_version = extract_version_from_image(&pinned);
+    if image_without_registry_or_tag(pinned) != image_without_registry_or_tag(running_image) {
+        return None;
+    }
+    available_catalog_update_for_images(pinned, running_image)
+}
+
+/// A signed catalog binds the image to an app id, so a publisher namespace
+/// migration must not hide a real upgrade. Baseline pins still require the
+/// same repository via `available_update_for_images` above.
+pub fn available_catalog_update_for_images(pinned: &str, running_image: &str) -> Option<String> {
+    let pinned_version = extract_version_from_image(pinned);
     if is_floating_tag(&pinned_version) {
         return None;
     }
-
-    let running_version = extract_version_from_image(running_image);
-    if pinned_version == running_version {
-        return None;
-    }
-
-    let pinned_repo = image_without_registry_or_tag(&pinned);
-    let running_repo = image_without_registry_or_tag(running_image);
-    if pinned_repo != running_repo {
-        return None;
-    }
-
-    // Never advertise a LOWER version as an update.
-    //
-    // Everything upstream of here is a version claim that can go stale: the
-    // signed catalog, a legacy catalog entry with no manifest, the
-    // image-versions.sh baseline pin. When one lags behind what a node is
-    // actually running, a bare `pinned != running` check turns that staleness
-    // into an "Update" button that rolls the node BACKWARDS — and a rollback
-    // to a version withdrawn for a vulnerability is precisely the case where
-    // that must not happen. Observed with BTCPay: 2.4.2 installed, a stale
-    // 2.3.9 pin, and the UI offering "update" to the exploited release.
-    //
-    // Only suppress when both tags parse as comparable version numbers, so
-    // apps with opaque tags (RELEASE.2024-11-07T00-52-20Z, 14-vectorchord0.4.3)
-    // keep the previous behaviour rather than silently losing updates.
-    if let (Some(p), Some(r)) = (
-        parse_version_parts(&pinned_version),
-        parse_version_parts(&running_version),
+    if matches!(
+        compare_image_versions(pinned, running_image),
+        Some(std::cmp::Ordering::Less | std::cmp::Ordering::Equal)
     ) {
-        if p < r {
-            return None;
-        }
+        return None;
     }
-
     Some(pinned_version)
 }
 
+/// Compare explicit image tags, ignoring registry and namespace. `None` means
+/// unknown ordering (including floating tags), never permission to downgrade.
+/// Archipelago's `-archyN` is a downstream patch revision ABOVE the upstream
+/// release, not a SemVer prerelease below it.
+pub fn compare_image_versions(target: &str, running: &str) -> Option<std::cmp::Ordering> {
+    use std::cmp::Ordering;
+    let target = extract_version_from_image(target);
+    let running = extract_version_from_image(running);
+    if is_floating_tag(&target) || is_floating_tag(&running) {
+        return None;
+    }
+    let target = target.strip_prefix('v').unwrap_or(&target);
+    let running = running.strip_prefix('v').unwrap_or(&running);
+    if target == running {
+        return Some(Ordering::Equal);
+    }
+    let mut target_core = parse_version_parts(target)?;
+    let mut running_core = parse_version_parts(running)?;
+    while target_core.last() == Some(&0) {
+        target_core.pop();
+    }
+    while running_core.last() == Some(&0) {
+        running_core.pop();
+    }
+    match target_core.cmp(&running_core) {
+        Ordering::Equal => {
+            fn patch_revision(tag: &str) -> Option<u64> {
+                if let Some((base, revision)) = tag.rsplit_once("-archy") {
+                    if base.chars().all(|c| c.is_ascii_digit() || c == '.') {
+                        return revision.parse().ok();
+                    }
+                }
+                tag.chars()
+                    .all(|c| c.is_ascii_digit() || c == '.')
+                    .then_some(0)
+            }
+            Some(patch_revision(target)?.cmp(&patch_revision(running)?))
+        }
+        order => Some(order),
+    }
+}
+
 /// Numeric components of a version tag, for ordering comparisons only.
 ///
 /// Accepts a leading `v` and a trailing pre-release suffix (`v0.18.4-beta`),
@@ -423,6 +452,57 @@ mod tests {
         );
     }
 
+    #[test]
+    fn downstream_patch_is_newer_than_upstream_and_orders_revisions() {
+        let upstream = "registry.test/team/mempool-frontend:v3.3.1";
+        let patch1 = "registry.test/team/mempool-frontend:v3.3.1-archy1";
+        let patch2 = "registry.test/team/mempool-frontend:v3.3.1-archy2";
+        assert_eq!(available_update_for_images(upstream, patch1), None);
+        assert_eq!(available_update_for_images(patch1, patch2), None);
+        assert_eq!(
+            available_update_for_images(patch1, upstream),
+            Some("v3.3.1-archy1".into())
+        );
+        assert_eq!(
+            available_update_for_images(patch2, patch1),
+            Some("v3.3.1-archy2".into())
+        );
+    }
+
+    #[test]
+    fn catalog_namespace_migration_does_not_hide_patch_or_offer_reinstall() {
+        let old = "registry.test/lfg2025/mempool-frontend:v3.3.1";
+        let patched = "registry.test/chaum/mempool-frontend:v3.3.1-archy1";
+        assert_eq!(
+            available_catalog_update_for_images(patched, old),
+            Some("v3.3.1-archy1".into())
+        );
+        assert_eq!(
+            available_catalog_update_for_images(
+                patched,
+                "registry.test/lfg2025/mempool-frontend:v3.3.1-archy1"
+            ),
+            None
+        );
+        assert_eq!(available_update_for_images(patched, old), None);
+    }
+
+    #[test]
+    fn equivalent_version_spelling_does_not_offer_update() {
+        assert_eq!(
+            available_update_for_images("r.test/team/app:v3.3.1", "r.test/team/app:3.3.1"),
+            None
+        );
+        assert_eq!(
+            available_update_for_images("r.test/team/app:3.3.0", "r.test/team/app:3.3"),
+            None
+        );
+        assert_eq!(
+            compare_image_versions("r.test/team/app:latest", "r.test/team/app:latest"),
+            None
+        );
+    }
+
     #[test]
     fn test_parse_image_versions() {
         let content = r#"
@@ -445,6 +525,22 @@ NOT_AN_IMAGE="something"
         assert!(!parsed.contains_key("ARCHY_REGISTRY"));
     }
 
+    #[test]
+    fn shipped_image_pins_expand_shell_defaults_to_concrete_refs() {
+        let images = parse_image_versions(include_str!("../../../../scripts/image-versions.sh"));
+        assert_eq!(
+            images["MEMPOOL_WEB_IMAGE"],
+            "source.archipelago-foundation.org/chaum/mempool-frontend:v3.3.1-archy1"
+        );
+        assert_eq!(
+            images["MEMPOOL_BACKEND_IMAGE"],
+            "source.archipelago-foundation.org/lfg2025/mempool-backend:v3.3.1"
+        );
+        assert!(images
+            .values()
+            .all(|v| !v.contains('$') && !v.contains('}')));
+    }
+
     #[test]
     fn test_image_var_mapping() {
         assert_eq!(image_var_for_app("lnd"), Some("LND_IMAGE"));

core/archipelago/src/container/prod_orchestrator.rs

@@ -4667,6 +4667,27 @@ impl ContainerOrchestrator for ProdContainerOrchestrator {
         let lock = self.app_lock(app_id).await;
         let _guard = lock.lock().await;
         let name = compute_container_name(&lm.manifest);
+        let mut resolved = lm.manifest.clone();
+        resolve_catalog_image(&mut resolved);
+        if resolved.app.container.build.is_none() {
+            if let Some(target) = &resolved.app.container.image {
+                if let Ok(running) = self.runtime.get_container_status(&name).await {
+                    match crate::container::image_versions::compare_image_versions(
+                        target,
+                        &running.image,
+                    ) {
+                        Some(std::cmp::Ordering::Less) => anyhow::bail!(
+                            "Refusing to downgrade {} from {} to {} during update",
+                            app_id,
+                            running.image,
+                            target
+                        ),
+                        Some(std::cmp::Ordering::Equal) => return Ok(()),
+                        _ => {}
+                    }
+                }
+            }
+        }
         let _ = self.runtime.stop_container(&name).await;
         let _ = self.runtime.remove_container(&name).await;
         self.install_fresh(&lm).await
@@ -5076,6 +5097,7 @@ mod tests {
         calls: StdMutex<Vec<String>>,
         /// container_name -> ContainerState. Absence = "doesn't exist".
         containers: StdMutex<HashMap<String, ContainerState>>,
+        running_images: StdMutex<HashMap<String, String>>,
         /// container_name -> Podman health status.
         health: StdMutex<HashMap<String, String>>,
         /// image_ref -> present. Absence = "not present in local storage".
@@ -5200,7 +5222,13 @@ mod tests {
                 health,
                 exit_code: None,
                 started_at: None,
-                image: "test-image".to_string(),
+                image: self
+                    .running_images
+                    .lock()
+                    .unwrap()
+                    .get(name)
+                    .cloned()
+                    .unwrap_or_else(|| "test-image".to_string()),
                 created: "now".to_string(),
                 ports: vec![],
                 lan_address: None,
@@ -6771,6 +6799,41 @@ app:
         assert_eq!(ids, vec!["bitcoin-knots", "bitcoin-ui"]);
     }
 
+    #[tokio::test]
+    async fn upgrade_preserves_container_when_catalog_is_stale_or_already_installed() {
+        for (target, should_error) in [("v3.3.1", true), ("v3.3.1-archy1", false)] {
+            let rt = Arc::new(MockRuntime::default());
+            rt.set_state("update-regression", ContainerState::Running);
+            rt.running_images.lock().unwrap().insert(
+                "update-regression".into(),
+                "registry.test/old/mempool-frontend:v3.3.1-archy1".into(),
+            );
+            let orch = orch_with(rt.clone()).await;
+            orch.insert_manifest_for_test(
+                pull_manifest(
+                    "update-regression",
+                    &format!("registry.test/new/mempool-frontend:{target}"),
+                ),
+                PathBuf::from("/tmp/update-regression"),
+            )
+            .await;
+            assert_eq!(
+                orch.upgrade("update-regression").await.is_err(),
+                should_error
+            );
+            assert!(
+                !rt.calls()
+                    .iter()
+                    .any(|call| call.starts_with("stop_container:")
+                        || call.starts_with("remove_container:")
+                        || call.starts_with("pull_image:")
+                        || call.starts_with("create_container:")),
+                "{:?}",
+                rt.calls()
+            );
+        }
+    }
+
     #[tokio::test]
     async fn upgrade_removes_and_reinstalls() {
         let rt = Arc::new(MockRuntime::default());