From 3d59120a9e2bfb323a12ff562d1e1594b0a25c7e Mon Sep 17 00:00:00 2001 From: HackingGate Date: Thu, 29 Oct 2026 01:45:16 +0900 Subject: [PATCH] supply-chain reads a submodule deleted in the pushed range as files, not as a moved pointer collect_range sent every gitlink line of `git diff --raw` to expand_gitlink, including one whose destination is no longer a gitlink: a submodule removed, or replaced by files tracked at the same path, as when a repository is merged into its superproject with its history. expand_gitlink then demanded a checkout of a submodule that no longer exists and the push was refused, exit 2, naming it -- with no way to satisfy the remedy it printed. Only a pointer that is still a gitlink at the end of the range has a tree to read. One deleted in the range is skipped; what stands at the path now is in the name-only diff as files. That diff also takes `T` now, so a gitlink or symlink that became a regular file is scanned as one. Two cases, both failing before this change: a submodule folded into the superproject has its replacing lockfile scanned, and a submodule removed outright needs no checkout. --- src/supply.rs | 14 ++++++++--- tests/supply_chain_cli.rs | 52 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 3 deletions(-) diff --git a/src/supply.rs b/src/supply.rs index 847416e..ee020b4 100644 --- a/src/supply.rs +++ b/src/supply.rs @@ -725,7 +725,9 @@ fn collect_range( crate::git::run_elsewhere(&directory, args) } }; - for line in ask(&["diff", "--name-only", "--diff-filter=ACMR", &range])?.lines() { + // `T` as well: a path whose type changed -- a gitlink, a symlink -- into a + // regular file is new content at that path, and a scanner reads files. + for line in ask(&["diff", "--name-only", "--diff-filter=ACMRT", &range])?.lines() { let path = prefix.join(line); if interesting(&path) { out.insert(path); @@ -742,10 +744,16 @@ fn collect_range( continue; }; let fields: Vec<&str> = meta.split_whitespace().collect(); - let [source_mode, destination_mode, old, new, ..] = fields[..] else { + let [_, destination_mode, old, new, ..] = fields[..] else { continue; }; - if source_mode != ":160000" && destination_mode != "160000" { + // Only a pointer that is still a gitlink at the end of the range has a + // tree to read. One deleted in the range -- the submodule removed, or + // replaced by files tracked in its place -- points at nothing, and + // whatever stands at the path now is in the diff above as files. + // Expanding it demanded a checkout of a submodule that no longer + // exists and refused the push for it. + if destination_mode != "160000" { continue; } let submodule = prefix.join(path); diff --git a/tests/supply_chain_cli.rs b/tests/supply_chain_cli.rs index e7b8c68..6d67509 100644 --- a/tests/supply_chain_cli.rs +++ b/tests/supply_chain_cli.rs @@ -1387,6 +1387,58 @@ fn a_submodule_that_is_not_checked_out_is_refused_rather_than_widened() { assert!(said.contains("sub"), "{said}"); } +/// A submodule folded into the superproject is files, not a moved pointer. +/// +/// The gitlink is deleted and the member's tree is tracked at the same path, +/// which is how a repository is merged into its superproject with its history. +/// The pointer then points at nothing, so there is no member checkout to read +/// and none was asked for -- the push was refused for one, naming a +/// submodule that no longer exists. What stands at the path now is the +/// superproject's own files, and the lockfile among them is what gets scanned. +#[test] +fn a_submodule_replaced_by_its_own_files_is_scanned_as_files() { + let root = tracked(); + with_a_submodule(&root); + let before = head(&root); + support::git(&root, &["rm", "-q", "--cached", "sub"]); + support::git(&root, &["rm", "-q", "-f", ".gitmodules"]); + std::fs::remove_dir_all(root.join("sub")).unwrap(); + write( + &root, + "sub/uv.lock", + "version = 1\n# now the superproject's own\n", + ); + let after = commit(&root, "fold the member in"); + let tools = stubs(&[ + ("osv-scanner", &recording("exit 0")), + ("uv", &recording("echo 'requests==2.0.0'")), + ]); + let output = pushed(&root, &tools, &before, &after); + assert_eq!(code(&output), 0, "{}", text(&output)); + let said = text(&output); + assert!(!said.contains("not checked out"), "{said}"); + assert!(said.contains("1 lockfile(s) in this range"), "{said}"); +} + +/// A submodule removed outright leaves nothing in the range to read. +/// +/// The pointer is gone and nothing replaces it, so the range adds no manifest. +/// Demanding a checkout of the removed member would refuse every push that +/// drops a dependency, which is the push least likely to add a risk. +#[test] +fn a_removed_submodule_needs_no_checkout() { + let root = tracked(); + with_a_submodule(&root); + let before = head(&root); + support::git(&root, &["rm", "-q", "-f", "sub"]); + let after = commit(&root, "drop the member"); + let tools = stubs(&[("osv-scanner", &recording("exit 0"))]); + let output = pushed(&root, &tools, &before, &after); + assert_eq!(code(&output), 0, "{}", text(&output)); + let said = text(&output); + assert!(!said.contains("not checked out"), "{said}"); +} + /// cargo-vet's finding and cargo-vet's refusal to start share exit 255. /// /// A dependency carrying no audit and a store that does not parse are the same