From d38fd8bb0d7009e5017a912b6f845eef2c770ba9 Mon Sep 17 00:00:00 2001 From: DanConwayDev Date: Mon, 17 Aug 2026 16:36:27 +0000 Subject: [PATCH] fix(storage): preserve refs with missing targets Production preflight found legacy refs whose target objects are already absent. Family retention correctly rejects those tips, but the launch migration must not lose the original ref name or loop forever trying to recreate it through update-ref. Advertise only tips whose objects are readable in the verified family. For a pre-existing missing target, write the validated loose ref atomically into the thin view, verify its exact snapshot, and emit bounded operational warnings. Readable refs continue through Git's update-ref and family base-retention paths. This does not invent missing objects, advertise broken tips to related repositories, repair legacy data, or relax validation of readable object and ref data. Validated with cargo fmt, workspace/all-targets Clippy with warnings denied, all seven migration unit tests, and a read-only production scan identifying four affected refs. --- src/git/migration.rs | 118 ++++++++++++++++++++++++++++++++++++------- 1 file changed, 99 insertions(+), 19 deletions(-) diff --git a/src/git/migration.rs b/src/git/migration.rs index 285463c..684f3c5 100644 --- a/src/git/migration.rs +++ b/src/git/migration.rs @@ -188,8 +188,7 @@ pub(crate) fn import_archived_repository( copy_object_database(source, &family, key.object_format)?; ensure_oids(&family, &expected_oids)?; for (name, reference) in snapshot_refs(source)? { - storage.retain_tip(key, &name, &reference.oid)?; - storage.advertise_base_tip(key, &name, &reference.oid)?; + retain_family_tip(storage, key, &family, &name, &reference.oid)?; } Ok(()) } @@ -453,8 +452,7 @@ fn build_family_union( ensure_oids(&family, &expected_oids)?; for candidate in candidates { for (name, reference) in snapshot_refs(&candidate.source_path)? { - storage.retain_tip(key, &name, &reference.oid)?; - storage.advertise_base_tip(key, &name, &reference.oid)?; + retain_family_tip(storage, key, &family, &name, &reference.oid)?; } } audit_family_connectivity(&family)?; @@ -623,12 +621,32 @@ fn install_refs(repo: &Path, refs: &BTreeMap) -> Result<()> &["symbolic-ref", name, target], "install migrated symbolic ref", )?; - } else { + } else if oid_is_available(repo, &reference.oid)? { run_git( Some(repo), &["update-ref", name, &reference.oid], "install migrated ref", )?; + } else { + let relative = Path::new(name); + if !name.starts_with("refs/") + || relative.is_absolute() + || relative + .components() + .any(|component| !matches!(component, std::path::Component::Normal(_))) + { + return Err(anyhow!("unsafe migrated ref name {name:?}")); + } + write_atomic( + &repo.join(relative), + format!("{}\n", reference.oid).as_bytes(), + )?; + warn!( + repository = %repo.display(), + reference = %name, + oid = %reference.oid, + "Preserving a pre-existing ref whose target object is missing" + ); } } Ok(()) @@ -645,8 +663,15 @@ fn verify_view( if std::fs::read(view.join("HEAD"))? != expected_head { return Err(anyhow!("migrated HEAD differs in {}", view.display())); } - for reference in expected_refs.values() { - ensure_oid(view, &reference.oid)?; + for (name, reference) in expected_refs { + if !oid_is_available(view, &reference.oid)? { + warn!( + repository = %view.display(), + reference = %name, + oid = %reference.oid, + "Verified a preserved ref whose target object is missing" + ); + } } Ok(()) } @@ -765,15 +790,12 @@ fn verify_pack(index: &Path) -> Result<()> { Ok(()) } -fn ensure_oid(repo: &Path, oid: &str) -> Result<()> { - let status = Command::new("git") +fn oid_is_available(repo: &Path, oid: &str) -> Result { + let output = Command::new("git") .args(["cat-file", "-e", oid]) .current_dir(repo) - .status()?; - if !status.success() { - return Err(anyhow!("object {oid} is missing from {}", repo.display())); - } - Ok(()) + .output()?; + Ok(output.status.success()) } fn ensure_oids(repo: &Path, expected: &[String]) -> Result<()> { @@ -814,6 +836,27 @@ fn audit_family_connectivity(repo: &Path) -> Result<()> { Ok(()) } +fn retain_family_tip( + storage: &LocalGitStorage, + key: &FamilyKey, + family: &Path, + name: &str, + oid: &str, +) -> Result<()> { + if oid_is_available(family, oid)? { + storage.retain_tip(key, name, oid)?; + storage.advertise_base_tip(key, name, oid)?; + } else { + warn!( + repository = %family.display(), + reference = %name, + oid = %oid, + "Not advertising a pre-existing ref whose target object is missing" + ); + } + Ok(()) +} + fn install_if_absent(source: &Path, destination: &Path) -> Result<()> { if destination.exists() { return Ok(()); @@ -1091,7 +1134,7 @@ mod tests { assert_eq!(report.migrated_views, 1); let key = FamilyKey::new(ObjectFormat::Sha1, "incomplete").unwrap(); - ensure_oid(&storage.family_repo_path(&key), &commit_oid).unwrap(); + assert!(oid_is_available(&storage.family_repo_path(&key), &commit_oid).unwrap()); } #[tokio::test] @@ -1120,7 +1163,44 @@ mod tests { let key = FamilyKey::new(ObjectFormat::Sha1, "orphan-pack").unwrap(); let family = storage.family_repo_path(&key); assert!(!family.join("objects/pack").join(orphan_name).exists()); - ensure_oid(&family, &oid).unwrap(); + assert!(oid_is_available(&family, &oid).unwrap()); + } + + #[tokio::test] + async fn migration_preserves_a_ref_with_a_missing_target() { + let temp = tempfile::tempdir().unwrap(); + let storage = LocalGitStorage::new(temp.path().join("git")); + let owner = Keys::generate().public_key().to_bech32().unwrap(); + let (repo, _) = legacy_repo(storage.git_data_path(), &owner, "missing-ref", b"present\n"); + let missing = "2".repeat(40); + let broken_ref = repo.join("refs/heads/broken"); + std::fs::create_dir_all(broken_ref.parent().unwrap()).unwrap(); + std::fs::write(&broken_ref, format!("{missing}\n")).unwrap(); + assert_eq!( + snapshot_refs(&repo).unwrap()["refs/heads/broken"].oid, + missing + ); + + let report = migrate_on_startup(&storage).await.unwrap(); + + assert_eq!(report.migrated_views, 1); + assert_eq!( + std::fs::read_to_string(&broken_ref).unwrap(), + format!("{missing}\n") + ); + assert_eq!( + snapshot_refs(&repo).unwrap()["refs/heads/broken"].oid, + missing + ); + let key = FamilyKey::new(ObjectFormat::Sha1, "missing-ref").unwrap(); + let family = storage.family_repo_path(&key); + assert!(!oid_is_available(&family, &missing).unwrap()); + assert!(!git( + &family, + &["for-each-ref", "--format=%(objectname)", "refs/grasp/"] + ) + .lines() + .any(|oid| oid == missing)); } #[tokio::test] @@ -1171,7 +1251,7 @@ mod tests { super::super::get_ref_commit(&second, "refs/test/data"), Some(second_oid) ); - ensure_oid(&storage.family_repo_path(&key), &unreachable).unwrap(); + assert!(oid_is_available(&storage.family_repo_path(&key), &unreachable).unwrap()); assert!(backup_root(&storage) .join(&owner_one) .join("example.git") @@ -1249,8 +1329,8 @@ mod tests { import_archived_repository(&storage, &key, &archive).unwrap(); let family = storage.family_repo_path(&key); - ensure_oid(&family, &referenced).unwrap(); - ensure_oid(&family, &unreachable).unwrap(); + assert!(oid_is_available(&family, &referenced).unwrap()); + assert!(oid_is_available(&family, &unreachable).unwrap()); assert!(!git(&family, &["for-each-ref", "refs/grasp/retained/"]).is_empty()); } }