Fix hosted cargo pin of registry = "crates-io" (#386)

crates/socket-patch-cli/tests/e2e_redirect_cargo_shapes.rs

@@ -123,6 +123,11 @@ struct Shape {
     /// beside the Socket one (#679, #863): VEX must then attest nothing, and
     /// `remove` must merge the Socket block into the crates.io one.
     contest: Option<&'static str>,
+    /// Files `remove` restores to different but equivalent bytes: the
+    /// upstream restore drops the pin, so a declaration that named
+    /// crates.io explicitly (`registry = "crates-io"`, #386) comes back
+    /// without the redundant key, which cargo resolves identically.
+    restored: Vec<(&'static str, String)>,
 }
 
 fn run_socket(cwd: &Path, args: &[&str], cargo_home: &Path) -> (i32, String, String) {
@@ -759,6 +764,11 @@ async fn run_shape(shape: Shape) -> Option<()> {
             // appended registry block with the same bytes whether or not the
             // original file ended in a newline — so an UNTERMINATED file can
             // only come back with its final newline (every other byte exact).
+            let bytes = &shape
+                .restored
+                .iter()
+                .find(|(path, _)| path == rel)
+                .map_or_else(|| bytes.clone(), |(_, text)| text.as_bytes().to_vec());
             let want = match after.get(rel) {
                 Some(got)
                     if !bytes.ends_with(b"\n") && *got == [bytes.as_slice(), b"\n"].concat() =>
@@ -1004,6 +1014,7 @@ async fn cargo_hosted_multi_version_pins_each_declaration_and_removes_cleanly()
         refused: None,
         prefetch: "",
         contest: None,
+        restored: vec![],
     };
     let _ = run_shape(shape).await;
 }
@@ -1029,10 +1040,59 @@ async fn cargo_hosted_legacy_config_is_restored_byte_for_byte() {
         refused: None,
         prefetch: "",
         contest: None,
+        restored: vec![],
     };
     let _ = run_shape(shape).await;
 }
 
+/// #386: `registry = "crates-io"` names Cargo's built-in crates.io
+/// registry, so the declaration is a plain crates.io dependency. It was
+/// refused as "another registry" and nothing was redirected; it must be
+/// pinned in place, in both the inline and the table spelling.
+#[tokio::test(flavor = "multi_thread")]
+async fn cargo_hosted_explicit_crates_io_registry_is_pinned() {
+    for (tag, manifest, restored) in [
+        (
+            "crates-io-inline",
+            consumer_manifest("cfg-if = { version = \"1.0.4\", registry = \"crates-io\" }\n"),
+            consumer_manifest("cfg-if = \"1.0.4\"\n"),
+        ),
+        (
+            "crates-io-table",
+            format!(
+                "{}\n[dependencies.cfg-if]\nversion = \"1.0.4\"\nregistry = \"crates-io\"\n",
+                consumer_manifest("")
+            ),
+            format!(
+                "{}\n[dependencies.cfg-if]\nversion = \"1.0.4\"\n",
+                consumer_manifest("")
+            ),
+        ),
+    ] {
+        let shape = Shape {
+            tag,
+            files: vec![
+                ("Cargo.toml", manifest),
+                ("src/main.rs", "fn main() {}\n".to_string()),
+            ],
+            patches: vec![CFG_IF_1],
+            oracle: vec![(
+                "src/main.rs",
+                "fn main() { println!(\"{}\", cfg_if::socket_patched()); }\n".to_string(),
+            )],
+            crlf: false,
+            lockless: false,
+            refused: None,
+            prefetch: "",
+            contest: None,
+            restored: vec![("Cargo.toml", restored)],
+        };
+        if run_shape(shape).await.is_none() {
+            return;
+        }
+    }
+}
+
 /// Bug H: a config ending in a blank line comes back byte-for-byte (the
 /// appended block's removal once normalized the trailing newline run). A
 /// config without a final newline comes back with every byte but that
@@ -1069,6 +1129,7 @@ async fn cargo_hosted_config_trailing_bytes_are_restored() {
             refused: None,
             prefetch: "",
             contest: None,
+            restored: vec![],
         };
         if run_shape(shape).await.is_none() {
             return;
@@ -1104,6 +1165,7 @@ async fn cargo_hosted_same_line_in_two_sections_removes_cleanly() {
         refused: None,
         prefetch: "",
         contest: None,
+        restored: vec![],
     };
     let _ = run_shape(shape).await;
 }
@@ -1145,6 +1207,7 @@ async fn cargo_hosted_workspace_member_declaration_is_pinned() {
         refused: None,
         prefetch: "",
         contest: None,
+        restored: vec![],
     };
     let _ = run_shape(shape).await;
 }
@@ -1169,6 +1232,7 @@ async fn cargo_hosted_crlf_project_keeps_its_line_endings() {
         refused: None,
         prefetch: "",
         contest: None,
+        restored: vec![],
     };
     let _ = run_shape(shape).await;
 }
@@ -1196,6 +1260,7 @@ async fn cargo_hosted_refuses_a_crate_another_crate_depends_on() {
         refused: Some("redirect_cargo_transitive_dependents"),
         prefetch: "",
         contest: None,
+        restored: vec![],
     };
     let _ = run_shape(shape).await;
 }
@@ -1224,6 +1289,7 @@ async fn cargo_hosted_refuses_a_lockless_project_with_other_dependencies() {
         refused: Some("redirect_cargo_lockless_dependents"),
         prefetch: "",
         contest: None,
+        restored: vec![],
     };
     let _ = run_shape(shape).await;
 }
@@ -1268,6 +1334,7 @@ async fn cargo_hosted_refuses_a_crate_the_root_manifest_patch_overrides() {
         refused: Some("redirect_cargo_dep_overridden"),
         prefetch: "cfg-if = \"=1.0.4\"\n",
         contest: None,
+        restored: vec![],
     };
     let _ = run_shape(shape).await;
 }
@@ -1301,6 +1368,7 @@ async fn cargo_hosted_refuses_a_crate_a_config_patch_overrides() {
         refused: Some("redirect_cargo_dep_overridden"),
         prefetch: "cfg-if = \"=1.0.4\"\n",
         contest: None,
+        restored: vec![],
     };
     let _ = run_shape(shape).await;
 }
@@ -1324,6 +1392,7 @@ async fn cargo_hosted_contested_by_a_later_crates_io_copy() {
         lockless: false,
         refused: None,
         contest: Some("crc32fast = \"=1.5.0\""),
+        restored: vec![],
         prefetch: "",
     };
     let _ = run_shape(shape).await;

crates/socket-patch-cli/tests/mode_migration_cargo.rs

@@ -1348,9 +1348,11 @@ async fn takeover_over_unrewritable_spelling_keeps_vendored() {
     for (tag, spelling) in [
         ("dotted", format!("{DEP}.version = \"1.0\"\n")),
         ("literal", format!("{DEP} = '1.0'\n")),
+        // An inline table spread over lines (#757). `registry = "crates-io"`
+        // was this case until #386 made hosted mode pin it.
         (
-            "crates-io",
-            format!("{DEP} = {{ version = \"1.0\", registry = \"crates-io\" }}\n"),
+            "multi-line-inline",
+            format!("{DEP} = {{ version = \"1.0\", features = [\n] }}\n"),
         ),
     ] {
         let tmp = tempfile::tempdir().unwrap();

crates/socket-patch-core/src/patch/redirect/mod.rs

@@ -2612,11 +2612,21 @@ fn is_valid_gem_index_url(url: &str) -> bool {
 
 /// A registry name THIS rewriter owns: `socket-patch-<canonical-uuid>`. An
 /// existing pin matching this grammar was written by a previous run and may be
-/// superseded in place; any other registry pin is the user's and is refused.
+/// superseded in place. Other registry names are the user's: only `crates-io`
+/// is overwritten (see [`is_repinnable_registry_name`]), the rest refuse.
 fn is_socket_patch_registry_name(value: &str) -> bool {
     generation::pin_name_uuid(value, false).is_some()
 }
 
+/// A `registry = …` value the Cargo.toml pin may overwrite in place: an
+/// earlier `socket-patch-<uuid>` pin (superseded), or `crates-io`, Cargo's
+/// built-in name for crates.io, which resolves exactly like a declaration
+/// with no `registry` key (#386). Any other name is a registry the user
+/// chose, and the dep is refused.
+fn is_repinnable_registry_name(value: &str) -> bool {
+    value == "crates-io" || is_socket_patch_registry_name(value)
+}
+
 /// The `socket-patch-<uuid>` registry name pinning `crate_name` in this
 /// manifest, in EVERY declaration shape [`plan_cargo_toml`] writes: a
 /// `[…dependencies.<key>]` header table (whose pin is a standalone
@@ -3090,7 +3100,8 @@ enum CargoTomlAction {
 
 /// Plan the full-manifest pin: EVERY occurrence of `crate_name` across every
 /// dependency table gains `registry = "<reg>"`, an existing
-/// `socket-patch-<uuid>` pin is superseded in place, and any occurrence that
+/// `socket-patch-<uuid>` pin (or an explicit `registry = "crates-io"`, #386)
+/// is overwritten in place, and any occurrence that
 /// cannot be handled refuses the whole dep. Nothing is applied unless every
 /// occurrence resolves.
 // The Cargo.toml planner's fixed probes, compiled once: `plan_cargo_toml`
@@ -3320,7 +3331,7 @@ fn plan_cargo_toml(
                         if ws {
                             ws_entries.insert(key.clone(), CargoWorkspaceEntry::Pinned);
                         }
-                    } else if is_socket_patch_registry_name(&value) {
+                    } else if is_repinnable_registry_name(&value) {
                         let old_line = lines[line_idx];
                         let new_text = registry_val_re
                             .replace(old_line, format!("registry = \"{reg}\"").as_str())
@@ -3434,7 +3445,7 @@ fn plan_cargo_toml(
                     if workspace {
                         ws_entries.insert(key.clone(), CargoWorkspaceEntry::Pinned);
                     }
-                } else if is_socket_patch_registry_name(&value) {
+                } else if is_repinnable_registry_name(&value) {
                     let new_text = registry_val_re
                         .replace(raw, format!("registry = \"{reg}\"").as_str())
                         .into_owned();
@@ -22267,6 +22278,67 @@ packages:
         assert!(r.confirmed_cargo_uuids.is_empty());
     }
 
+    /// #386: `crates-io` is Cargo's built-in name for crates.io, so a
+    /// declaration that names it explicitly is a plain crates.io dependency.
+    /// The pin replaces the value in place (inline, table and quoted-key
+    /// spellings) instead of refusing it as another registry, and a re-run
+    /// over the rewritten manifest is a silent no-op.
+    #[test]
+    fn cargo_explicit_crates_io_registry_is_pinned_in_place() {
+        let reg = cargo_reg();
+        for (declaration, expected) in [
+            (
+                "[dependencies]\nserde = { version = \"1.0.190\", registry = \"crates-io\" }\n"
+                    .to_string(),
+                format!("[dependencies]\nserde = {{ version = \"1.0.190\", registry = \"{reg}\" }}\n"),
+            ),
+            (
+                "[dependencies]\nserde = { registry = 'crates-io', version = \"1.0.190\" } # explicit\n"
+                    .to_string(),
+                format!("[dependencies]\nserde = {{ registry = \"{reg}\", version = \"1.0.190\" }} # explicit\n"),
+            ),
+            (
+                "[dependencies.serde]\nversion = \"1.0.190\"\nregistry = \"crates-io\"\n".to_string(),
+                format!("[dependencies.serde]\nversion = \"1.0.190\"\nregistry = \"{reg}\"\n"),
+            ),
+            (
+                "[dependencies.alias]\npackage = \"serde\"\nversion = \"1.0.190\"\n\"registry\" = \"crates-io\"\n"
+                    .to_string(),
+                format!("[dependencies.alias]\npackage = \"serde\"\nversion = \"1.0.190\"\nregistry = \"{reg}\"\n"),
+            ),
+        ] {
+            let manifest = format!("[package]\nname = \"app\"\nversion = \"0.1.0\"\n\n{declaration}");
+            let r = rewrite_registry_redirect(&cargo_files(&manifest), &[cargo_sparse_override()]);
+            assert!(r.warnings.is_empty(), "{declaration}: {:?}", r.warnings);
+            let toml = r.files.get("Cargo.toml").expect("manifest pinned");
+            assert!(
+                toml.ends_with(&expected),
+                "{declaration}: crates-io must be replaced in place:\n{toml}"
+            );
+            assert!(!toml.contains("crates-io"), "{toml}");
+            assert_eq!(cargo_socket_registry_pin(toml, "serde"), Some(reg.clone()));
+            assert!(r.confirmed_cargo_uuids.contains(CARGO_UUID));
+            let toml_edits: Vec<_> = r.edits.iter().filter(|e| e.path == "Cargo.toml").collect();
+            assert_eq!(toml_edits.len(), 1, "{:?}", r.edits);
+            assert!(
+                toml_edits[0]
+                    .original
+                    .as_ref()
+                    .and_then(Value::as_str)
+                    .is_some_and(|s| s.contains("crates-io")),
+                "the edit's original records the crates-io spelling: {:?}",
+                toml_edits[0]
+            );
+
+            let mut rerun_files = cargo_files(&manifest);
+            rerun_files.extend(r.files);
+            let rerun = rewrite_registry_redirect(&rerun_files, &[cargo_sparse_override()]);
+            assert!(rerun.files.is_empty(), "{:?}", rerun.files.keys());
+            assert!(rerun.warnings.is_empty(), "{:?}", rerun.warnings);
+            assert!(rerun.confirmed_cargo_uuids.contains(CARGO_UUID));
+        }
+    }
+
     /// The TABLE-FORM twins of the inline-table registry matrix: an existing
     /// socket pin is superseded in place, a current pin is recognized as
     /// Already (silent, still confirmed), and a foreign pin refuses the whole

crates/socket-patch-core/src/patch/redirect/upstream/cargo.rs

@@ -9,7 +9,9 @@
 //! always crates.io's. A declaration's original spelling is not recorded:
 //! the shorthand `name = { version = "…", registry = "…" }` the rewriter
 //! produces from `name = "…"` collapses back to that shorthand, and every
-//! other form just loses the pin.
+//! other form just loses the pin. That includes a pin that overwrote an
+//! explicit `registry = "crates-io"` (#386): the declaration comes back
+//! without the redundant key, which cargo resolves the same way.
 
 use std::collections::{BTreeMap, BTreeSet};
 
@@ -330,14 +332,22 @@ static SHORTHAND_RE: std::sync::LazyLock<Regex> = std::sync::LazyLock::new(|| {
 });
 
 /// The line with its `registry = "<reg>"` pin removed: `Some(None)` drops
-/// the whole line (the table form's inserted `registry = …` line),
+/// the whole line (the table form's `registry = …` line; a trailing
+/// comment on it survives as a comment-only line),
 /// `Some(Some(line))` is the unpinned declaration, `None` a spelling this
 /// restore does not recognize.
 fn unpin_line(line: &str, reg: &str) -> Option<Option<String>> {
     let trimmed = line.trim();
     let body = trimmed.split('#').next().unwrap_or(trimmed).trim();
     if body == format!("registry = \"{reg}\"") {
-        return Some(None);
+        // The pin may have overwritten a user's commented line in place
+        // (a superseded pin, or `registry = "crates-io"`): the comment
+        // stays, on its own line.
+        // Sliced from the raw line, so a CRLF file's `\r` stays on it.
+        return Some(line.find('#').map(|hash| {
+            let indent = &line[..line.len() - line.trim_start().len()];
+            format!("{indent}{}", &line[hash..])
+        }));
     }
     if let Some(c) = SHORTHAND_RE.captures(line) {
         if &c[3] == reg {
@@ -506,6 +516,21 @@ mod tests {
         );
     }
 
+    /// A table-form pin that overwrote a commented `registry = "crates-io"`
+    /// line (#386) or an earlier pin keeps the user's comment on restore.
+    #[test]
+    fn table_form_line_keeps_its_comment() {
+        assert_eq!(
+            unpin_line(&format!("  registry = \"{REG}\" # stay on crates.io"), REG),
+            Some(Some("  # stay on crates.io".to_string()))
+        );
+        // A CRLF manifest is split on `\n`: the kept line keeps its `\r`.
+        assert_eq!(
+            unpin_line(&format!("registry = \"{REG}\" # note\r"), REG),
+            Some(Some("# note\r".to_string()))
+        );
+    }
+
     #[test]
     fn appended_block_leaves_the_config_byte_identical() {
         let original = "[net]\ngit-fetch-with-cli = true\n";