fix(catalog): compare/cache raw catalog bytes, not a re-serialization

Live verification on .228 showed catalog_changed:true on every
check-updates call: AppCatalog.apps is a HashMap, so the re-serialized
cache bytes flap with key order and the changed-detection never reports
"unchanged" — which would have reloaded the manifest overlay every
hourly poll. Cache the raw fetched body instead and compare against
that; as a bonus the cache now holds the exact signed preimage, so a
present release-root signature stays verifiable from disk.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
archipelago 2026-07-09 19:11:25 -04:00
parent ceb319c292
commit 7e866c7c94

View File

@ -373,9 +373,9 @@ pub async fn refresh_catalog(data_dir: &Path) -> anyhow::Result<CatalogRefresh>
let mut last_err: Option<anyhow::Error> = None; let mut last_err: Option<anyhow::Error> = None;
for url in &urls { for url in &urls {
match fetch_one(&client, url).await { match fetch_one(&client, url).await {
Ok(catalog) => { Ok((catalog, body)) => {
let count = catalog.apps.len(); let count = catalog.apps.len();
let changed = write_cache(data_dir, &catalog)?; let changed = write_cache(data_dir, &body)?;
if changed { if changed {
// Invalidate the in-process cache so the next read re-parses. // Invalidate the in-process cache so the next read re-parses.
*CACHE.lock().unwrap() = None; *CACHE.lock().unwrap() = None;
@ -400,7 +400,10 @@ pub async fn refresh_catalog(data_dir: &Path) -> anyhow::Result<CatalogRefresh>
Err(last_err.unwrap_or_else(|| anyhow::anyhow!("no catalog mirrors reachable"))) Err(last_err.unwrap_or_else(|| anyhow::anyhow!("no catalog mirrors reachable")))
} }
async fn fetch_one(client: &reqwest::Client, url: &str) -> anyhow::Result<AppCatalog> { async fn fetch_one(
client: &reqwest::Client,
url: &str,
) -> anyhow::Result<(AppCatalog, String)> {
let resp = client.get(url).send().await?; let resp = client.get(url).send().await?;
if !resp.status().is_success() { if !resp.status().is_success() {
anyhow::bail!("HTTP {}", resp.status()); anyhow::bail!("HTTP {}", resp.status());
@ -439,23 +442,27 @@ async fn fetch_one(client: &reqwest::Client, url: &str) -> anyhow::Result<AppCat
} }
} }
Ok(catalog) Ok((catalog, body))
} }
/// Atomically write the catalog cache. Returns `false` (skipping the write) /// Atomically write the catalog cache. Caches the RAW fetched bytes — not a
/// when the serialized bytes are identical to what's already cached, so /// re-serialization of the typed struct — for two reasons: the struct's
/// callers can tell a genuine catalog change from an unchanged poll. /// `apps` HashMap serializes in nondeterministic order (a re-serialized
fn write_cache(data_dir: &Path, catalog: &AppCatalog) -> anyhow::Result<bool> { /// comparison would report "changed" on every poll), and the raw bytes are
/// the signed preimage, so the cache stays signature-verifiable. Returns
/// `false` (skipping the write) when the bytes are identical to what's
/// already cached, so callers can tell a genuine catalog change from an
/// unchanged poll.
fn write_cache(data_dir: &Path, body: &str) -> anyhow::Result<bool> {
let dest = data_dir.join(APP_CATALOG_FILE); let dest = data_dir.join(APP_CATALOG_FILE);
let json = serde_json::to_string_pretty(catalog)?;
if std::fs::read_to_string(&dest) if std::fs::read_to_string(&dest)
.map(|current| current == json) .map(|current| current == body)
.unwrap_or(false) .unwrap_or(false)
{ {
return Ok(false); return Ok(false);
} }
let tmp = data_dir.join(format!("{}.tmp", APP_CATALOG_FILE)); let tmp = data_dir.join(format!("{}.tmp", APP_CATALOG_FILE));
std::fs::write(&tmp, json)?; std::fs::write(&tmp, body)?;
std::fs::rename(&tmp, &dest)?; std::fs::rename(&tmp, &dest)?;
Ok(true) Ok(true)
} }
@ -498,25 +505,26 @@ mod tests {
fn write_cache_reports_changed_only_on_new_bytes() { fn write_cache_reports_changed_only_on_new_bytes() {
// The changed flag gates the runtime manifest-overlay reload: an // The changed flag gates the runtime manifest-overlay reload: an
// unchanged hourly poll must NOT report changed (or every tick would // unchanged hourly poll must NOT report changed (or every tick would
// rebuild the manifest map), while a genuinely new catalog must. // rebuild the manifest map), while a genuinely new catalog must. Raw
// fetched bytes are compared — a re-serialized comparison would flap
// on the apps HashMap's nondeterministic key order (seen live on .228).
let dir = tempfile::tempdir().unwrap(); let dir = tempfile::tempdir().unwrap();
let cat: AppCatalog = serde_json::from_str( let body = r#"{"schema":1,"apps":{"demo":{"version":"1.0.0"}}}"#;
r#"{"schema":1,"apps":{"demo":{"version":"1.0.0"}}}"#, assert!(write_cache(dir.path(), body).unwrap(), "first write is a change");
)
.unwrap();
assert!(write_cache(dir.path(), &cat).unwrap(), "first write is a change");
assert!( assert!(
!write_cache(dir.path(), &cat).unwrap(), !write_cache(dir.path(), body).unwrap(),
"identical rewrite is not a change" "identical rewrite is not a change"
); );
let cat2: AppCatalog = serde_json::from_str( let body2 = r#"{"schema":1,"apps":{"demo":{"version":"1.0.1"}}}"#;
r#"{"schema":1,"apps":{"demo":{"version":"1.0.1"}}}"#,
)
.unwrap();
assert!( assert!(
write_cache(dir.path(), &cat2).unwrap(), write_cache(dir.path(), body2).unwrap(),
"new catalog bytes are a change" "new catalog bytes are a change"
); );
assert_eq!(
std::fs::read_to_string(dir.path().join(APP_CATALOG_FILE)).unwrap(),
body2,
"cache holds the raw signed preimage bytes"
);
} }
#[test] #[test]