From b161bf9cbd3dca9426b0a45b69d523b69860f208 Mon Sep 17 00:00:00 2001
From: Dennis Nemec <dennis.nemec@icloud.com>
Date: Thu, 3 Sep 2026 20:24:50 +0200
Subject: [PATCH] Fix Docker Hub authentication and reject build-id tags as
 candidates

Docker Hub answers with both token and access_token, which made the
serde alias fail as a duplicate field, so every Hub image reported that
no token was returned. Candidates now also have to match the shape of
the running tag; cert-manager v1.14.5 was otherwise offered an upgrade
to the build id 608111629.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 backend/crates/domain/src/image.rs            | 41 +++++++++++++++++--
 backend/crates/infrastructure/src/registry.rs | 21 ++++++++++
 2 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/backend/crates/domain/src/image.rs b/backend/crates/domain/src/image.rs
index 9b34afe..64af999 100644
--- a/backend/crates/domain/src/image.rs
+++ b/backend/crates/domain/src/image.rs
@@ -89,18 +89,24 @@ fn is_prerelease(suffix: &str) -> bool {
     .any(|m| s.starts_with(m) || s.contains(&format!("-{m}")))
 }
 
-/// Tags from the registry that are newer than `current`, oldest first. Only tags with the
-/// same suffix (`-rootless`, `-debian-12` …) are considered, so the variant stays the same.
+/// Tags from the registry that are newer than `current`, oldest first.
+///
+/// A candidate has to be shaped like the running tag: the same variant suffix
+/// (`-rootless`, `-debian-12` …), the same `v` prefix and the same number of version
+/// parts. That keeps build ids and dates (`608111629`, `20260417`) out of the suggestion.
 pub fn newer_tags(current: &str, available: &[String]) -> Vec<String> {
     let Some((now, suffix)) = version_parts(current) else {
         return Vec::new();
     };
+    let prefixed = current.starts_with('v');
     let mut newer: Vec<(Vec<u64>, String)> = available
         .iter()
         .filter_map(|t| {
             let (v, s) = version_parts(t)?;
             (s == suffix
                 && !is_prerelease(&s)
+                && t.starts_with('v') == prefixed
+                && v.len() == now.len()
                 && cmp_version(&v, &now) == std::cmp::Ordering::Greater)
                 .then_some((v, t.clone()))
         })
@@ -218,6 +224,31 @@ mod tests {
         assert_eq!(newest_tag("latest", &tags), None, "no version to compare");
     }
 
+    #[test]
+    fn ignores_tags_that_are_not_shaped_like_the_running_one() {
+        // build ids and dates are numerically larger but are not a newer release
+        let tags: Vec<String> = [
+            "v1.14.5",
+            "608111629",
+            "20260417",
+            "v1.15.0",
+            "v1.14.6",
+            "1.16.0",
+        ]
+        .iter()
+        .map(|s| s.to_string())
+        .collect();
+        assert_eq!(newer_tags("v1.14.5", &tags), vec!["v1.14.6", "v1.15.0"]);
+        assert_eq!(newest_tag("v1.14.5", &tags).as_deref(), Some("v1.15.0"));
+
+        // the number of version parts has to match, so 1.11 does not jump to 1.11.0.1
+        let tags: Vec<String> = ["1.11", "1.12", "1.12.0", "1.12.0.1"]
+            .iter()
+            .map(|s| s.to_string())
+            .collect();
+        assert_eq!(newer_tags("1.11", &tags), vec!["1.12"]);
+    }
+
     #[test]
     fn compares_versions_by_number_not_by_text() {
         let tags: Vec<String> = ["1.9.0", "1.10.0", "1.10", "2.0.0"]
@@ -225,7 +256,11 @@ mod tests {
             .map(|s| s.to_string())
             .collect();
         assert_eq!(newest_tag("1.9.0", &tags).as_deref(), Some("2.0.0"));
-        assert_eq!(newer_tags("1.9.0", &tags), vec!["1.10", "1.10.0", "2.0.0"]);
+        assert_eq!(
+            newer_tags("1.9.0", &tags),
+            vec!["1.10.0", "2.0.0"],
+            "1.10 has fewer parts"
+        );
         assert_eq!(
             newest_tag("v0.6.3", &["v0.7.0".to_string(), "v0.6.4".to_string()]).as_deref(),
             Some("v0.7.0")
diff --git a/backend/crates/infrastructure/src/registry.rs b/backend/crates/infrastructure/src/registry.rs
index 3eaf832..9627707 100644
--- a/backend/crates/infrastructure/src/registry.rs
+++ b/backend/crates/infrastructure/src/registry.rs
@@ -56,6 +56,15 @@ pub fn parse_auth_challenge(headers: &str) -> Option<String> {
     Some(url.trim_end_matches(['&', '?']).to_string())
 }
 
+/// Registries answer with `token`, `access_token`, or both (Docker Hub sends both).
+pub fn parse_token(body: &str) -> Option<String> {
+    let value: serde_json::Value = serde_json::from_str(body).ok()?;
+    ["token", "access_token"]
+        .iter()
+        .find_map(|k| value.get(k).and_then(|v| v.as_str()))
+        .map(|t| t.to_string())
+}
+
 /// `{"tags": ["1.0", "1.1"]}`; a missing or null list means no tags.
 pub fn parse_tags(body: &str) -> Result<Vec<String>, DomainError> {
     #[derive(serde::Deserialize)]
@@ -217,6 +226,18 @@ mod tests {
         assert_eq!(parse_next_link("HTTP/1.1 200 OK\r\n"), None);
     }
 
+    #[test]
+    fn reads_the_token_whichever_field_carries_it() {
+        // Docker Hub sends both fields, which must not be treated as a duplicate
+        assert_eq!(
+            parse_token(r#"{"token":"a","access_token":"a","expires_in":300}"#).as_deref(),
+            Some("a")
+        );
+        assert_eq!(parse_token(r#"{"access_token":"b"}"#).as_deref(), Some("b"));
+        assert_eq!(parse_token(r#"{"errors":[]}"#), None);
+        assert_eq!(parse_token("not json"), None);
+    }
+
     #[test]
     fn reads_the_tag_list() {
         assert_eq!(