From 7ba6b81eee5f0c5381129e2d46dd41026c83f02f Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 25 Sep 2026 14:37:42 -0400 Subject: [PATCH 1/3] Fix Cargo hosted workspace redirects Resolve inherited dependency aliases through their workspace definitions and match local lockfile owners by both package name and version. This allows valid workspace patches while refusing consumers outside the editable project. Read quoted dependency values and annotated registry tables consistently so repeated application cannot leave an alias unpatched or create an invalid duplicate TOML table. Validated with 70 Cargo redirect unit tests and real service-backed hosted, vendored service, and vendored build installs using fresh Cargo caches, locked offline builds, integrity rejection, and revert. Assisted-by: Codex:gpt-6-astra --- .../src/patch/redirect/mod.rs | 563 +++++++++++++++--- 1 file changed, 471 insertions(+), 92 deletions(-) diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index ebcd2fab..b22fe03e 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -765,11 +765,13 @@ fn rewrite_cargo( for (path, text) in manifests.iter_mut() { *text = to_lf(path, std::mem::take(text)); } - // Each manifest's own `[package] name` — how Cargo.lock names the - // source-less (workspace / path) package it declares. - let manifest_packages: Vec> = manifests + let workspace_version = files + .get("Cargo.toml") + .and_then(|text| text.parse::().ok()) + .and_then(|doc| cargo_workspace_package_version(&doc).map(str::to_string)); + let manifest_packages: Vec> = manifests .iter() - .map(|(_, text)| cargo_manifest_package_name(text)) + .map(|(path, text)| cargo_manifest_package_id(text, path, workspace_version.as_deref())) .collect(); let edits_before = result.edits.len(); let mut changed_manifests: std::collections::BTreeSet = @@ -950,9 +952,10 @@ fn rewrite_cargo( // a symlink) — keeps resolving it from crates.io, so the repointed // lock is unsatisfiable and that consumer compiles the unpatched copy. if let Some(lock_text) = cargo_lock.as_deref() { - let pinned_packages: std::collections::BTreeSet<&str> = toml_plans + let pinned_packages: std::collections::BTreeSet<(&str, &str)> = toml_plans .iter() - .filter_map(|(i, _)| manifest_packages[*i].as_deref()) + .filter_map(|(i, _)| manifest_packages[*i].as_ref()) + .map(|(name, version)| (name.as_str(), version.as_str())) .collect(); let blocking = cargo_unpinnable_dependents(lock_text, &dep.name, &dep.version, &pinned_packages); @@ -1187,14 +1190,44 @@ fn cargo_requirement_excludes_detail( ) } -/// The `[package] name` a manifest declares (`None` for a virtual workspace -/// root or an unparseable file). -fn cargo_manifest_package_name(text: &str) -> Option { - let doc = text.parse::().ok()?; - doc.get("package")? - .get("name")? +fn cargo_workspace_package_version(doc: &toml_edit::DocumentMut) -> Option<&str> { + doc.get("workspace")? + .get("package")? + .get("version")? .as_str() - .map(str::to_string) +} + +fn cargo_manifest_package_id( + text: &str, + path: &str, + workspace_version: Option<&str>, +) -> Option<(String, String)> { + let doc = text.parse::().ok()?; + let package = doc.get("package")?; + let name = package.get("name")?.as_str()?; + let version = match package.get("version") { + None => "0.0.0", + Some(version) => match version.as_str() { + Some(version) => version, + None if version.get("workspace").and_then(toml_edit::Item::as_bool) == Some(true) => { + if doc.get("workspace").is_some() { + cargo_workspace_package_version(&doc)? + } else { + if let Some(workspace) = package.get("workspace") { + let dir = path.strip_suffix("/Cargo.toml").unwrap_or(""); + if crate::utils::cargo_workspace::normalize_rel(dir, workspace.as_str()?) + .is_none_or(|workspace| !workspace.is_empty()) + { + return None; + } + } + workspace_version? + } + } + None => return None, + }, + }; + Some((name.to_string(), version.to_string())) } /// The Cargo.lock packages that depend on `crate_name@version` and that a @@ -1208,7 +1241,7 @@ fn cargo_unpinnable_dependents( lock: &str, crate_name: &str, version: &str, - pinned_packages: &std::collections::BTreeSet<&str>, + pinned_packages: &std::collections::BTreeSet<(&str, &str)>, ) -> Vec { let Ok(doc) = lock.parse::() else { return vec!["Cargo.lock (it does not parse as TOML)".to_string()]; @@ -1246,7 +1279,7 @@ fn cargo_unpinnable_dependents( }; out.push(format!("{name} {pkg_version} ({kind})")); } - None if !pinned_packages.contains(name) => { + None if !pinned_packages.contains(&(name, pkg_version)) => { out.push(format!( "{name} {pkg_version} (a path package whose Cargo.toml is outside the \ project or not rewritable)" @@ -1477,12 +1510,6 @@ fn is_socket_patch_registry_name(value: &str) -> bool { /// the inline spelling and read the other three as "not redirected". pub(crate) fn cargo_socket_registry_pin(content: &str, crate_name: &str) -> Option { let lines: Vec<&str> = content.split('\n').collect(); - let socket_value = |text: &str| -> Option { - CARGO_TOML_REGISTRY_VAL_RE - .captures(text) - .map(|c| c[1].to_string()) - .filter(|v| is_socket_patch_registry_name(v)) - }; let mut section = CargoTomlSection::Other; for (idx, raw) in lines.iter().enumerate() { let trimmed = raw.trim_start(); @@ -1517,13 +1544,7 @@ pub(crate) fn cargo_socket_registry_pin(content: &str, crate_name: &str) -> Opti if k != name { return None; } - let v = rest.trim_start().strip_prefix('=')?.trim(); - Some( - v.strip_prefix('"') - .and_then(|s| s.split('"').next()) - .unwrap_or(v) - .to_string(), - ) + cargo_toml_string(rest.trim_start().strip_prefix('=')?) }) }; let is_ours = match value_of("package") { @@ -1548,11 +1569,14 @@ pub(crate) fn cargo_socket_registry_pin(content: &str, crate_name: &str) -> Opti if let Some(dotted) = rest_trim.strip_prefix('.') { // `.registry = "socket-patch-…"`: a spelling this rewriter // refuses to write, but a hand edit can leave one behind. - if key == crate_name - && parse_cargo_entry_key(dotted).is_some_and(|(k, _)| k == "registry") - { - if let Some(reg) = socket_value(trimmed) { - return Some(reg); + if key == crate_name { + let registry = parse_cargo_entry_key(dotted) + .filter(|(key, _)| key == "registry") + .and_then(|(_, rest)| rest.trim_start().strip_prefix('=')) + .and_then(cargo_toml_string) + .filter(|registry| is_socket_patch_registry_name(registry)); + if registry.is_some() { + return registry; } } continue; @@ -1567,12 +1591,14 @@ pub(crate) fn cargo_socket_registry_pin(content: &str, crate_name: &str) -> Opti continue; }; let inner = &value[1..close]; - let is_ours = match CARGO_TOML_PACKAGE_RE.captures(inner) { - Some(c) => c[1] == *crate_name, + let is_ours = match cargo_toml_inline_string(inner, "package") { + Some(package) => package == crate_name, None => key == crate_name, }; if is_ours { - if let Some(reg) = socket_value(inner) { + if let Some(reg) = cargo_toml_inline_string(inner, "registry") + .filter(|registry| is_socket_patch_registry_name(registry)) + { return Some(reg); } } @@ -1695,6 +1721,25 @@ fn parse_cargo_entry_key(line: &str) -> Option<(String, &str)> { } } +fn cargo_toml_string(value: &str) -> Option { + let document = format!("value = {value}") + .parse::() + .ok()?; + document.get("value")?.as_str().map(str::to_string) +} + +fn cargo_toml_inline_string(inner: &str, key: &str) -> Option { + let document = format!("dependency = {{{inner}}}") + .parse::() + .ok()?; + document + .get("dependency")? + .as_inline_table()? + .get(key)? + .as_str() + .map(str::to_string) +} + struct CargoTomlPlan { content: String, edits: Vec, @@ -1740,11 +1785,9 @@ enum CargoTomlAction { static CARGO_TOML_HEADER_RE: LazyLock = LazyLock::new(|| { Regex::new(r"^\[([^\]]+)\]\s*(?:#.*)?$").expect("static section-header regex is valid") }); -static CARGO_TOML_PACKAGE_RE: LazyLock = LazyLock::new(|| { - Regex::new(r#"\bpackage\s*=\s*"([^"]*)""#).expect("static package-key regex is valid") -}); static CARGO_TOML_REGISTRY_VAL_RE: LazyLock = LazyLock::new(|| { - Regex::new(r#"\bregistry\s*=\s*"([^"]*)""#).expect("static registry-value regex is valid") + Regex::new(r#"(?:\bregistry|"registry"|'registry')\s*=\s*(?:"[^"]*"|'[^']*')"#) + .expect("static registry-value regex is valid") }); static CARGO_TOML_REGISTRY_KEY_RE: LazyLock = LazyLock::new(|| { Regex::new(r"\bregistry\s*=").expect("static registry-key probe regex is valid") @@ -1759,10 +1802,6 @@ static CARGO_TOML_PATH_GIT_RE: LazyLock = LazyLock::new(|| { Regex::new(r"\b(?:path|git)\s*=").expect("static path/git probe regex is valid") }); -static CARGO_TOML_VERSION_VAL_RE: LazyLock = LazyLock::new(|| { - Regex::new(r#"\bversion\s*=\s*"([^"]*)""#).expect("static version-value regex is valid") -}); - /// Whether one declaration's version requirement selects the patched /// version. Cargo resolves a declaration to ONE version, so a project that /// locks several versions of a crate (`cfg-if = "1"` beside a renamed @@ -1817,6 +1856,7 @@ enum CargoWorkspaceEntry { Pinned, /// The entry names the crate at another version. OtherVersion, + OtherPackage, } /// Every version of `crate_name` a Cargo.lock holds other than `version`. @@ -1853,13 +1893,11 @@ fn plan_cargo_toml( ) -> Result { let lines: Vec<&str> = content.split('\n').collect(); let header_re: &Regex = &CARGO_TOML_HEADER_RE; - let package_re: &Regex = &CARGO_TOML_PACKAGE_RE; let registry_val_re: &Regex = &CARGO_TOML_REGISTRY_VAL_RE; let registry_key_re: &Regex = &CARGO_TOML_REGISTRY_KEY_RE; let registry_index_re: &Regex = &CARGO_TOML_REGISTRY_INDEX_RE; let workspace_key_re: &Regex = &CARGO_TOML_WORKSPACE_KEY_RE; let path_git_re: &Regex = &CARGO_TOML_PATH_GIT_RE; - let version_val_re: &Regex = &CARGO_TOML_VERSION_VAL_RE; let ambiguous = || format!("its version requirement also matches another locked version of {crate_name}"); @@ -1913,39 +1951,37 @@ fn plan_cargo_toml( if k == key_name { let rest = rest.trim_start(); if let Some(v) = rest.strip_prefix('=') { - let v = v.trim(); - let v = v - .strip_prefix('"') - .and_then(|s| s.split('"').next()) - .unwrap_or(v); - return Some((*j, v.to_string())); + return cargo_toml_string(v).map(|value| (*j, value)); } } } } None }; + let has = |name: &str| { + block.iter().any(|(_, t)| { + parse_cargo_entry_key(t).is_some_and(|(k, rest)| { + k == name && rest.trim_start().starts_with('=') + }) + }) + }; + if has("workspace") { + pending.push(Pending::NeedsWorkspacePin(key.clone())); + continue; + } let package_val = find_value("package").map(|(_, v)| v); let is_ours = match &package_val { Some(p) => p == crate_name, None => key == crate_name, }; if !is_ours { + if ws { + ws_entries.insert(key.clone(), CargoWorkspaceEntry::OtherPackage); + } continue; } - let has = |name: &str| { - block.iter().any(|(_, t)| { - parse_cargo_entry_key(t).is_some_and(|(k, rest)| { - k == name && rest.trim_start().starts_with('=') - }) - }) - }; let req = find_value("version").map(|(_, v)| v); - let selects = if has("workspace") { - CargoReqMatch::Ours - } else { - cargo_req_selects(req.as_deref(), version, other_versions) - }; + let selects = cargo_req_selects(req.as_deref(), version, other_versions); if selects == CargoReqMatch::NotOurs { excluded.extend(req); if ws { @@ -1953,9 +1989,7 @@ fn plan_cargo_toml( } continue; } - if has("workspace") { - pending.push(Pending::NeedsWorkspacePin(key.clone())); - } else if selects == CargoReqMatch::Ambiguous { + if selects == CargoReqMatch::Ambiguous { pending.push(Pending::Refuse(ambiguous())); } else if has("path") || has("git") { pending.push(Pending::Refuse( @@ -2012,19 +2046,19 @@ fn plan_cargo_toml( if let Some(dotted) = rest_trim.strip_prefix('.') { // Dotted entry (`serde.workspace = true`, `serde.version = "1"`, // `alias.package = "serde"`, …). - let sub = parse_cargo_entry_key(dotted).map(|(k, _)| k); - if key == crate_name { - if sub.as_deref() == Some("workspace") { - pending.push(Pending::NeedsWorkspacePin(key.clone())); - } else { - pending.push(Pending::Refuse( - "declared with dotted keys this rewriter does not edit".to_string(), - )); - } - } else if sub.as_deref() == Some("package") - && package_re - .captures(trimmed) - .is_some_and(|c| &c[1] == crate_name) + let sub = parse_cargo_entry_key(dotted); + if sub.as_ref().is_some_and(|(key, _)| key == "workspace") { + pending.push(Pending::NeedsWorkspacePin(key.clone())); + } else if key == crate_name { + pending.push(Pending::Refuse( + "declared with dotted keys this rewriter does not edit".to_string(), + )); + } else if sub + .filter(|(key, _)| key == "package") + .and_then(|(_, rest)| rest.trim_start().strip_prefix('=')) + .and_then(cargo_toml_string) + .as_deref() + == Some(crate_name) { pending.push(Pending::Refuse( "declared with dotted keys this rewriter does not edit".to_string(), @@ -2049,19 +2083,22 @@ fn plan_cargo_toml( continue; }; let inner = &value[1..close]; - let package_val = package_re.captures(inner).map(|c| c[1].to_string()); + if workspace_key_re.is_match(inner) { + pending.push(Pending::NeedsWorkspacePin(key.clone())); + continue; + } + let package_val = cargo_toml_inline_string(inner, "package"); let is_ours = match &package_val { Some(p) => p == crate_name, None => key == crate_name, }; if !is_ours { + if workspace { + ws_entries.insert(key.clone(), CargoWorkspaceEntry::OtherPackage); + } continue; } - if workspace_key_re.is_match(inner) { - pending.push(Pending::NeedsWorkspacePin(key.clone())); - continue; - } - let req = version_val_re.captures(inner).map(|c| c[1].to_string()); + let req = cargo_toml_inline_string(inner, "version"); match cargo_req_selects(req.as_deref(), version, other_versions) { CargoReqMatch::NotOurs => { excluded.extend(req); @@ -2080,8 +2117,7 @@ fn plan_cargo_toml( pending.push(Pending::Refuse( "declared as a path/git dependency".to_string(), )); - } else if let Some(c) = registry_val_re.captures(inner) { - let value = c[1].to_string(); + } else if let Some(value) = cargo_toml_inline_string(inner, "registry") { if value == reg { pending.push(Pending::Action(CargoTomlAction::Already)); if workspace { @@ -2216,12 +2252,13 @@ fn plan_cargo_toml( actions.push(CargoTomlAction::InheritsWorkspace); } // Inherits another version of the crate: not this dep. - Some(CargoWorkspaceEntry::OtherVersion) => {} - None => { + Some(CargoWorkspaceEntry::OtherVersion | CargoWorkspaceEntry::OtherPackage) => {} + None if key == crate_name => { return Err("inherits from [workspace.dependencies] with no rewritable \ entry for it" .to_string()); } + None => {} }, Pending::Refuse(reason) => return Err(reason), } @@ -2570,7 +2607,19 @@ fn plan_cargo_config( let header = format!("[registries.{reg}]"); let index_line = format!("index = \"{index_url}\""); let lines: Vec<&str> = config.split('\n').collect(); - let header_idx = lines.iter().position(|l| l.trim() == header); + let header_idx = lines.iter().position(|line| { + if !line.trim_start().starts_with('[') { + return false; + } + let Ok(document) = line.parse::() else { + return false; + }; + document + .get("registries") + .and_then(|registries| registries.get(reg)) + .and_then(toml_edit::Item::as_table) + .is_some_and(|table| !table.is_implicit()) + }); if let Some(i) = header_idx { let mut end = lines.len(); for (j, l) in lines.iter().enumerate().skip(i + 1) { @@ -2583,7 +2632,17 @@ fn plan_cargo_config( while end > i + 1 && lines[end - 1].trim().is_empty() { end -= 1; } - let healthy = lines[i + 1..end].iter().any(|l| l.trim() == index_line); + let healthy = lines[i + 1..end] + .join("\n") + .parse::() + .ok() + .and_then(|document| { + document + .get("index") + .and_then(toml_edit::Item::as_str) + .map(|index| index == index_url) + }) + .unwrap_or(false); if healthy { return None; } @@ -9011,6 +9070,83 @@ mod tests { assert!(r.confirmed_cargo_uuids.contains(CARGO_UUID)); } + #[test] + fn cargo_dotted_literal_renames_refuse_every_declaration() { + for alias in ["alias", "\"alias\"", "'alias'"] { + for package_key in ["package", "\"package\"", "'package'"] { + let manifest = format!( + "[package]\nname = \"app\"\nversion = \"0.1.0\"\n\n\ + [dependencies]\nserde = \"1.0.190\"\n\ + {alias}.{package_key} = 'serde'\n{alias}.version = '1.0.190'\n" + ); + assert!(manifest.parse::().is_ok()); + let result = + rewrite_registry_redirect(&cargo_files(&manifest), &[cargo_sparse_override()]); + assert!(result.files.is_empty(), "{manifest}"); + assert!(result.edits.is_empty(), "{manifest}"); + assert!(result.confirmed_cargo_uuids.is_empty(), "{manifest}"); + assert!(result + .warnings + .iter() + .any(|warning| warning.code == "redirect_cargo_toml_dep_unrewritable")); + } + } + } + + #[test] + fn cargo_literal_renames_pin_inline_and_table_forms() { + for declaration in [ + "[dependencies]\nalias = { package = 'serde', version = '1.0.190' }\n", + "[dependencies.alias]\npackage = 'serde'\nversion = '1.0.190'\n", + "[dependencies]\nalias = { 'package' = 'serde', 'version' = '1.0.190' }\n", + "[dependencies.'alias']\n'package' = 'serde'\n'version' = '1.0.190'\n", + ] { + let manifest = + format!("[package]\nname = \"app\"\nversion = \"0.1.0\"\n\n{declaration}"); + let files = cargo_files(&manifest); + let result = rewrite_registry_redirect(&files, &[cargo_sparse_override()]); + assert!(result.warnings.is_empty(), "{:?}", result.warnings); + assert!(result.confirmed_cargo_uuids.contains(CARGO_UUID)); + let updated = &result.files["Cargo.toml"]; + let document = updated.parse::().unwrap(); + assert_eq!( + document["dependencies"]["alias"]["registry"].as_str(), + Some(cargo_reg().as_str()) + ); + assert_eq!( + cargo_socket_registry_pin(updated, "serde"), + Some(cargo_reg()) + ); + let mut rerun_files = files; + rerun_files.extend(result.files); + let rerun = rewrite_registry_redirect(&rerun_files, &[cargo_sparse_override()]); + assert!(rerun.files.is_empty()); + assert!(rerun.warnings.is_empty(), "{:?}", rerun.warnings); + assert!(rerun.confirmed_cargo_uuids.contains(CARGO_UUID)); + } + } + + #[test] + fn cargo_literal_registry_pins_are_superseded() { + let previous = "socket-patch-11111111-1111-1111-1111-111111111111"; + for declaration in [ + format!("[dependencies]\nalias = {{ package = 'serde', version = '1.0.190', registry = '{previous}' }}\n"), + format!("[dependencies.alias]\npackage = 'serde'\nversion = '1.0.190'\nregistry = '{previous}' # previous pin\n"), + format!("[dependencies.alias]\npackage = 'serde'\nversion = '1.0.190'\n'registry' = '{previous}'\n"), + ] { + let manifest = format!( + "[package]\nname = \"app\"\nversion = \"0.1.0\"\n\n{declaration}" + ); + let result = rewrite_registry_redirect(&cargo_files(&manifest), &[cargo_sparse_override()]); + assert!(result.warnings.is_empty(), "{:?}", result.warnings); + let updated = &result.files["Cargo.toml"]; + let document = updated.parse::().unwrap(); + assert_eq!(document["dependencies"]["alias"]["registry"].as_str(), Some(cargo_reg().as_str())); + assert_eq!(cargo_socket_registry_pin(updated, "serde"), Some(cargo_reg())); + assert!(result.confirmed_cargo_uuids.contains(CARGO_UUID)); + } + } + /// AUDIT A5(a) alone: when the ONLY key match renames a different crate, /// the dep is genuinely not declared → not-found, and NOTHING is written /// (no config block, no lock repoint). @@ -9267,6 +9403,54 @@ mod tests { assert!(second.confirmed_cargo_uuids.contains(CARGO_UUID)); } + #[test] + fn cargo_config_quoted_commented_headers_are_reused() { + for header in [ + format!("[registries.{}] # managed registry", cargo_reg()), + format!("[registries.\"{}\"]", cargo_reg()), + format!("['registries'.'{}'] # managed registry", cargo_reg()), + format!("[ \"registries\" . '{}' ]", cargo_reg()), + ] { + for index in [ + format!("index = \"{}\" # current", cargo_index_url()), + format!("index = '{}'", cargo_index_url()), + format!("'index' = '{}' # current", cargo_index_url()), + ] { + let config = format!("{header}\n{index}\n\n[build]\njobs = 4\n"); + assert!(config.parse::().is_ok()); + assert!( + plan_cargo_config( + &config, + ".cargo/config.toml", + &cargo_reg(), + &cargo_index_url() + ) + .is_none(), + "{config}" + ); + } + let config = + format!("{header}\nindex = 'sparse+https://old.example/'\n\n[build]\njobs = 4\n"); + let plan = plan_cargo_config( + &config, + ".cargo/config.toml", + &cargo_reg(), + &cargo_index_url(), + ) + .expect("stale registry repaired"); + let document = plan + .content + .parse::() + .expect("no duplicate tables"); + assert_eq!( + document["registries"][&cargo_reg()]["index"].as_str(), + Some(cargo_index_url().as_str()) + ); + assert_eq!(document["build"]["jobs"].as_integer(), Some(4)); + assert_eq!(plan.edit.action, "rewritten"); + } + } + /// A degraded managed block (header intact, index line commented or /// stale) is regenerated in place rather than trusted. #[test] @@ -9710,6 +9894,96 @@ mod tests { assert!(r.confirmed_cargo_uuids.contains(CARGO_UUID)); } + #[test] + fn cargo_workspace_member_inherits_renamed_dependency() { + for workspace_entry in [ + "[workspace.dependencies]\nserial = { package = \"serde\", version = \"=1.0.190\", features = [\"std\"] }\n", + "[workspace.dependencies.serial]\npackage = \"serde\"\nversion = \"=1.0.190\"\nfeatures = [\"std\"]\n", + ] { + for declaration in [ + "[dependencies]\nserial.workspace = true\n", + "[dependencies]\nserial = { workspace = true, features = [\"derive\"] }\n", + "[dependencies.serial]\nworkspace = true\n", + "[dev-dependencies]\nserial.workspace = true\n", + "[build-dependencies]\nserial = { workspace = true }\n", + "[target.'cfg(unix)'.dependencies.serial]\nworkspace = true\n", + ] { + let mut files = cargo_files(&format!( + "[workspace]\nmembers = [\"consumer\"]\n\n{workspace_entry}" + )); + files.insert( + "consumer/Cargo.toml".into(), + format!( + "[package]\nname = \"consumer\"\nversion = \"0.1.0\"\n\n{declaration}" + ), + ); + files.get_mut("Cargo.lock").unwrap().push_str( + "\n[[package]]\nname = \"consumer\"\nversion = \"0.1.0\"\ndependencies = [\"serde\"]\n", + ); + + let result = rewrite_registry_redirect(&files, &[cargo_sparse_override()]); + assert!(result.warnings.is_empty(), "{:?}", result.warnings); + assert!(result.confirmed_cargo_uuids.contains(CARGO_UUID)); + assert!(result.files["Cargo.toml"] + .contains(&format!("registry = \"{}\"", cargo_reg()))); + assert!(!result.files.contains_key("consumer/Cargo.toml")); + assert!(result.files["Cargo.lock"].contains(&cargo_index_url())); + + files.extend(result.files); + let repeated = rewrite_registry_redirect(&files, &[cargo_sparse_override()]); + assert!(repeated.warnings.is_empty(), "{:?}", repeated.warnings); + assert!(repeated.edits.is_empty() && repeated.files.is_empty()); + assert!(repeated.confirmed_cargo_uuids.contains(CARGO_UUID)); + } + } + } + + #[test] + fn cargo_root_inherits_renamed_dependency_before_workspace_declaration() { + let files = cargo_files( + "[package]\nname = \"app\"\nversion = \"0.1.0\"\n\n\ + [dependencies]\nserial.workspace = true\n\n\ + [workspace.dependencies]\nserial = { package = \"serde\", version = \"=1.0.190\" }\n", + ); + let result = rewrite_registry_redirect(&files, &[cargo_sparse_override()]); + assert!(result.warnings.is_empty(), "{:?}", result.warnings); + assert!(result.confirmed_cargo_uuids.contains(CARGO_UUID)); + assert!(result.files["Cargo.toml"].contains("serial.workspace = true")); + assert!(result.files["Cargo.toml"].contains(&format!( + "serial = {{ package = \"serde\", version = \"=1.0.190\", registry = \"{}\" }}", + cargo_reg() + ))); + } + + #[test] + fn cargo_workspace_inherited_key_renaming_another_package_is_ignored() { + for other_entry in [ + "[workspace.dependencies]\nserde = { package = \"unrelated\", version = \"1\" }\n", + "[workspace.dependencies.serde]\npackage = \"unrelated\"\nversion = \"1\"\n", + ] { + let root = format!( + "[workspace]\nmembers = [\"consumer\"]\n\n\ + {other_entry}\n\ + [workspace.dependencies.serial]\npackage = \"serde\"\nversion = \"=1.0.190\"\n" + ); + let mut files = cargo_files(&root); + files.insert( + "consumer/Cargo.toml".into(), + "[package]\nname = \"consumer\"\nversion = \"0.1.0\"\n\n\ + [dependencies]\nserde.workspace = true\nserial.workspace = true\n" + .into(), + ); + files.get_mut("Cargo.lock").unwrap().push_str( + "\n[[package]]\nname = \"consumer\"\nversion = \"0.1.0\"\ndependencies = [\"serde\"]\n", + ); + let result = rewrite_registry_redirect(&files, &[cargo_sparse_override()]); + assert!(result.warnings.is_empty(), "{:?}", result.warnings); + assert!(result.confirmed_cargo_uuids.contains(CARGO_UUID)); + assert!(result.files["Cargo.toml"].contains(other_entry)); + assert!(!result.files.contains_key("consumer/Cargo.toml")); + } + } + /// A member that cannot be pinned (a path dependency here) refuses the /// WHOLE dep: the root stays untouched too. #[test] @@ -15072,6 +15346,111 @@ packages: ); } + #[test] + fn cargo_source_less_dependents_match_both_name_and_version() { + let root = "[package]\nname = \"app\"\nversion = \"0.1.0\"\n\n\ + [dependencies]\ninside = { package = \"foo\", path = \"inside\" }\n\ + outside = { package = \"foo\", path = \"../outside\" }\n"; + let mut files = cargo_files(root); + files.insert( + "inside/Cargo.toml".into(), + "[package]\nname = \"foo\"\nversion = \"0.1.0\"\n\n\ + [dependencies]\nserde = \"1.0.190\"\n" + .into(), + ); + files.get_mut("Cargo.lock").unwrap().push_str( + "\n[[package]]\nname = \"app\"\nversion = \"0.1.0\"\n\ + dependencies = [\"foo 0.1.0\", \"foo 0.2.0\"]\n\n\ + [[package]]\nname = \"foo\"\nversion = \"0.1.0\"\ndependencies = [\"serde\"]\n\n\ + [[package]]\nname = \"foo\"\nversion = \"0.2.0\"\ndependencies = [\"serde\"]\n", + ); + let refused = rewrite_registry_redirect(&files, &[cargo_sparse_override()]); + assert!(refused.files.is_empty() && refused.edits.is_empty()); + assert!(refused.confirmed_cargo_uuids.is_empty()); + assert_eq!( + warning_codes(&refused), + vec!["redirect_cargo_transitive_dependents"] + ); + assert!(refused.warnings[0] + .detail + .contains("foo 0.2.0 (a path package")); + + files.insert("Cargo.toml".into(), root.replace("../outside", "outside")); + files.insert( + "outside/Cargo.toml".into(), + "[package]\nname = \"foo\"\nversion = \"0.2.0\"\n\n\ + [dependencies]\nserde = \"1.0.190\"\n" + .into(), + ); + let accepted = rewrite_registry_redirect(&files, &[cargo_sparse_override()]); + assert!(accepted.warnings.is_empty(), "{:?}", accepted.warnings); + assert!(accepted.confirmed_cargo_uuids.contains(CARGO_UUID)); + assert!(accepted.files.contains_key("inside/Cargo.toml")); + assert!(accepted.files.contains_key("outside/Cargo.toml")); + } + + #[test] + fn cargo_source_less_dependents_resolve_workspace_and_default_versions() { + for (version_field, locked_version) in [ + ("version.workspace = true\n", "0.2.0"), + ("version = { workspace = true }\n", "0.2.0"), + ("version.workspace = true\nworkspace = \"..\"\n", "0.2.0"), + ("", "0.0.0"), + ] { + let mut files = cargo_files( + "[workspace]\nmembers = [\"consumer\"]\n\n\ + [workspace.package]\nversion = \"0.2.0\"\n", + ); + files.insert( + "consumer/Cargo.toml".into(), + format!( + "[package]\nname = \"consumer\"\n{version_field}\n\ + [dependencies]\nserde = \"1.0.190\"\n" + ), + ); + files.get_mut("Cargo.lock").unwrap().push_str(&format!( + "\n[[package]]\nname = \"consumer\"\nversion = \"{locked_version}\"\n\ + dependencies = [\"serde\"]\n" + )); + let result = rewrite_registry_redirect(&files, &[cargo_sparse_override()]); + assert!( + result.warnings.is_empty(), + "{version_field}: {:?}", + result.warnings + ); + assert!(result.confirmed_cargo_uuids.contains(CARGO_UUID)); + } + } + + #[test] + fn cargo_package_identity_keeps_workspace_version_ownership() { + let manifest = "[package]\nname = \"consumer\"\nversion.workspace = true\n"; + assert_eq!( + cargo_manifest_package_id( + &format!("{manifest}\n[workspace.package]\nversion = \"0.3.0\"\n"), + "consumer/Cargo.toml", + Some("0.2.0"), + ), + Some(("consumer".into(), "0.3.0".into())), + ); + assert_eq!( + cargo_manifest_package_id( + &format!("{manifest}\n[workspace]\n"), + "consumer/Cargo.toml", + Some("0.2.0"), + ), + None, + ); + assert_eq!( + cargo_manifest_package_id( + &format!("{manifest}workspace = \"../../other\"\n"), + "consumer/Cargo.toml", + Some("0.2.0"), + ), + None, + ); + } + /// A checksum-less entry whose `source` line ends the block (the /// trailing newline sits outside the block region) still gets its pin. #[test] From a156d87f4e3246df9422bc930f622906d122203a Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 25 Sep 2026 14:46:41 -0400 Subject: [PATCH 2/3] Refuse incomplete Cargo manifest redirects Validate planned dependency pins against parsed TOML before writing any files. Legal root and target dotted or inline declarations that the source-preserving editor cannot rewrite now refuse the entire patch instead of leaving a mixture of original and patched sources. Validate unchanged member manifests too, while preserving workspace inheritance and declarations for other package versions. Malformed TOML is refused without changing the project. Validated with 73 Cargo unit tests and the real converter/API/CLI matrix, including an actual Cargo build proving the dotted manifest is valid and remains byte-identical after refusal. Assisted-by: Codex:gpt-6-astra --- .../src/patch/redirect/mod.rs | 240 +++++++++++++++--- 1 file changed, 198 insertions(+), 42 deletions(-) diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index b22fe03e..f15515b7 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -894,6 +894,18 @@ fn rewrite_cargo( &root_workspace, ) { Ok(plan) => { + if let Err(reason) = validate_cargo_toml_pins( + &plan.content, + &dep.name, + &dep.version, + &other_versions, + ®, + &plan.workspace, + &root_workspace, + ) { + refused = Some((path.clone(), reason)); + break; + } if path == "Cargo.toml" { root_workspace = plan.workspace.clone(); } @@ -1740,6 +1752,96 @@ fn cargo_toml_inline_string(inner: &str, key: &str) -> Option { .map(str::to_string) } +fn validate_cargo_toml_pins( + content: &str, + crate_name: &str, + version: &str, + other_versions: &[String], + registry: &str, + workspace: &BTreeMap, + inherited: &BTreeMap, +) -> Result<(), String> { + let document = content + .parse::() + .map_err(|_| "the planned manifest does not parse as TOML".to_string())?; + let unpinned = |dependencies: &dyn toml_edit::TableLike| { + dependencies.iter().find_map(|(key, entry)| { + let table = entry.as_table_like(); + let field = |name: &str| table.and_then(|table| table.get(name)); + if field("workspace").and_then(toml_edit::Item::as_bool) == Some(true) { + return match workspace.get(key).or(inherited.get(key)) { + Some( + CargoWorkspaceEntry::Pinned + | CargoWorkspaceEntry::OtherVersion + | CargoWorkspaceEntry::OtherPackage, + ) => None, + None if key != crate_name => None, + None => Some(key.to_string()), + }; + } + let name = field("package") + .and_then(toml_edit::Item::as_str) + .unwrap_or(key); + if name != crate_name { + return None; + } + let requirement = entry + .as_str() + .or_else(|| field("version").and_then(toml_edit::Item::as_str)); + match cargo_req_selects(requirement, version, other_versions) { + CargoReqMatch::NotOurs => return None, + CargoReqMatch::Ambiguous => return Some(key.to_string()), + CargoReqMatch::Ours => {} + } + let is_pinned = field("registry").and_then(toml_edit::Item::as_str) == Some(registry) + && field("path").is_none() + && field("git").is_none() + && field("registry-index").is_none(); + (!is_pinned).then(|| key.to_string()) + }) + }; + let mut scopes: Vec<&dyn toml_edit::TableLike> = vec![document.as_table()]; + if let Some(targets) = document + .get("target") + .and_then(toml_edit::Item::as_table_like) + { + scopes.extend( + targets + .iter() + .filter_map(|(_, target)| target.as_table_like()), + ); + } + for scope in scopes { + for kind in [ + "dependencies", + "dev-dependencies", + "build-dependencies", + "dev_dependencies", + "build_dependencies", + ] { + if let Some(key) = scope + .get(kind) + .and_then(toml_edit::Item::as_table_like) + .and_then(&unpinned) + { + return Err(format!("dependency declaration {key} was not pinned")); + } + } + } + if let Some(key) = document + .get("workspace") + .and_then(toml_edit::Item::as_table_like) + .and_then(|workspace| workspace.get("dependencies")) + .and_then(toml_edit::Item::as_table_like) + .and_then(unpinned) + { + return Err(format!( + "workspace dependency declaration {key} was not pinned" + )); + } + Ok(()) +} + struct CargoTomlPlan { content: String, edits: Vec, @@ -9070,6 +9172,86 @@ mod tests { assert!(r.confirmed_cargo_uuids.contains(CARGO_UUID)); } + #[test] + fn cargo_root_dependency_forms_cannot_leave_partial_redirect() { + for dependency in [ + "dependencies.serde = \"1.0.190\"", + "dependencies = { serde = \"1.0.190\" }", + "dependencies = { serde = { version = \"1.0.190\" } }", + "target.'cfg(unix)'.dependencies.serde = \"1.0.190\"", + "workspace.dependencies.serde = \"1.0.190\"", + "workspace = { dependencies = { serde = \"1.0.190\" } }", + ] { + let manifest = format!( + "{dependency}\n\n[package]\nname = \"app\"\nversion = \"0.1.0\"\n\n\ + [dev-dependencies]\nserde = \"1.0.190\"\n" + ); + assert!(manifest.parse::().is_ok()); + let result = + rewrite_registry_redirect(&cargo_files(&manifest), &[cargo_sparse_override()]); + assert!(result.files.is_empty(), "{manifest}: {:?}", result.files); + assert!(result.edits.is_empty(), "{manifest}"); + assert!(result.confirmed_cargo_uuids.is_empty(), "{manifest}"); + assert!(result + .warnings + .iter() + .any(|warning| warning.code == "redirect_cargo_toml_dep_unrewritable")); + } + } + + #[test] + fn cargo_semantic_pin_guard_preserves_other_version_declarations() { + let manifest = "dependencies.serde = \"0.9\"\n\n\ + [package]\nname = \"app\"\nversion = \"0.1.0\"\n\n\ + [dev-dependencies]\nserde = \"1.0.190\"\n"; + let mut files = cargo_files(manifest); + files.get_mut("Cargo.lock").unwrap().push_str(&format!( + "\n[[package]]\nname = \"serde\"\nversion = \"0.9.15\"\n\ + source = \"registry+https://github.com/rust-lang/crates.io-index\"\n\ + checksum = \"{}\"\n", + "a".repeat(64) + )); + let result = rewrite_registry_redirect(&files, &[cargo_sparse_override()]); + assert!(result.warnings.is_empty(), "{:?}", result.warnings); + assert!(result.confirmed_cargo_uuids.contains(CARGO_UUID)); + let document = result.files["Cargo.toml"] + .parse::() + .unwrap(); + assert_eq!(document["dependencies"]["serde"].as_str(), Some("0.9")); + assert_eq!( + document["dev-dependencies"]["serde"]["registry"].as_str(), + Some(cargo_reg().as_str()) + ); + } + + #[test] + fn cargo_semantic_pin_guard_checks_unchanged_members() { + let mut files = cargo_files( + "[package]\nname = \"app\"\nversion = \"0.1.0\"\n\n\ + [workspace]\nmembers = [\"member\"]\n\n\ + [dependencies]\nserde = \"1.0.190\"\n", + ); + files.insert( + "member/Cargo.toml".to_string(), + "dependencies = { serde = \"1.0.190\" }\n\n\ + [package]\nname = \"member\"\nversion = \"0.1.0\"\n" + .to_string(), + ); + files.get_mut("Cargo.lock").unwrap().push_str( + "\n[[package]]\nname = \"member\"\nversion = \"0.1.0\"\n\ + dependencies = [\"serde\"]\n", + ); + let result = rewrite_registry_redirect(&files, &[cargo_sparse_override()]); + assert!(result.files.is_empty()); + assert!(result.edits.is_empty()); + assert!(result.confirmed_cargo_uuids.is_empty()); + assert!(result.warnings.iter().any(|warning| { + warning.code == "redirect_cargo_toml_dep_unrewritable" + && warning.detail.contains("member/Cargo.toml") + && warning.detail.contains("was not pinned") + })); + } + #[test] fn cargo_dotted_literal_renames_refuse_every_declaration() { for alias in ["alias", "\"alias\"", "'alias'"] { @@ -16535,13 +16717,8 @@ packages: // tolerance legs, workspace-inheritance satisfaction, and the remaining // diagnosis spellings. - /// Malformed Cargo.toml section headers (unbalanced quote in a segment, - /// an unclosed `[dependencies`) must classify as non-dependency sections - /// — their entries stay byte-identical — and garbage lines inside the - /// real [dependencies] table are skipped while the real entry still - /// gains the pin. #[test] - fn cargo_malformed_headers_and_table_lines_are_skipped_not_fatal() { + fn cargo_malformed_manifest_headers_and_lines_refuse_redirect() { let files = cargo_files( "[package]\nname = \"app\"\nversion = \"0.1.0\"\n\n\ [target.'cfg(unix).dependencies]\nserde = \"9.9.9\"\n\n\ @@ -16549,50 +16726,29 @@ packages: [dependencies]\n= \"junk\"\njunk\nserde = \"1.0.190\"\n", ); let r = rewrite_registry_redirect(&files, &[cargo_sparse_override()]); - assert!( - r.warnings.is_empty(), - "garbage headers/lines are skipped, not refused: {:?}", - r.warnings - ); - let toml = r.files.get("Cargo.toml").expect("Cargo.toml rewritten"); - let pinned = format!( - "serde = {{ version = \"1.0.190\", registry = \"{}\" }}", - cargo_reg() - ); - assert_eq!( - toml.matches(&pinned).count(), - 1, - "only the real [dependencies] entry is pinned: {toml}" - ); - assert!( - toml.contains("serde = \"9.9.9\"") && toml.contains("serde = \"8.8.8\""), - "entries under malformed headers stay byte-identical: {toml}" - ); - assert!( - toml.contains("= \"junk\"\njunk\n"), - "garbage table lines survive untouched: {toml}" - ); + assert!(r.files.is_empty()); + assert!(r.edits.is_empty()); + assert!(r.confirmed_cargo_uuids.is_empty()); + assert!(r.warnings.iter().any(|warning| { + warning.code == "redirect_cargo_toml_dep_unrewritable" + && warning.detail.contains("does not parse as TOML") + })); } - /// Unparseable lines INSIDE a `[dependencies.]` table block (a bare - /// `= …`, a key token with no `=`) are skipped by the block scanner while - /// the block still gains its `registry` pin right after the header. #[test] - fn cargo_dep_entry_block_garbage_lines_are_skipped() { + fn cargo_malformed_dep_entry_block_refuses_redirect() { let files = cargo_files( "[package]\nname = \"app\"\nversion = \"0.1.0\"\n\n\ [dependencies.serde]\n= \"zap\"\npackage \"serde\"\nversion = \"1.0.190\"\n", ); let r = rewrite_registry_redirect(&files, &[cargo_sparse_override()]); - assert!(r.warnings.is_empty(), "{:?}", r.warnings); - let toml = r.files.get("Cargo.toml").expect("Cargo.toml rewritten"); - assert!( - toml.contains(&format!( - "[dependencies.serde]\nregistry = \"{}\"\n= \"zap\"\npackage \"serde\"\nversion = \"1.0.190\"", - cargo_reg() - )), - "registry pin inserted after the header, garbage lines untouched: {toml}" - ); + assert!(r.files.is_empty()); + assert!(r.edits.is_empty()); + assert!(r.confirmed_cargo_uuids.is_empty()); + assert!(r.warnings.iter().any(|warning| { + warning.code == "redirect_cargo_toml_dep_unrewritable" + && warning.detail.contains("does not parse as TOML") + })); } /// A `[workspace.dependencies]` entry ALREADY pinned to the managed From 5b42bb173d41b9cc657bfb7523e5abc26735d5fa Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 25 Sep 2026 15:19:01 -0400 Subject: [PATCH 3/3] Keep Cargo refusal checks lint-clean Combine identical refusal branches for dotted dependencies without changing which declarations are refused. Full workspace Clippy and all 73 Cargo redirect tests pass. Assisted-by: Codex:gpt-6-astra --- .../socket-patch-core/src/patch/redirect/mod.rs | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index f15515b7..ccfcadac 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -2151,16 +2151,13 @@ fn plan_cargo_toml( let sub = parse_cargo_entry_key(dotted); if sub.as_ref().is_some_and(|(key, _)| key == "workspace") { pending.push(Pending::NeedsWorkspacePin(key.clone())); - } else if key == crate_name { - pending.push(Pending::Refuse( - "declared with dotted keys this rewriter does not edit".to_string(), - )); - } else if sub - .filter(|(key, _)| key == "package") - .and_then(|(_, rest)| rest.trim_start().strip_prefix('=')) - .and_then(cargo_toml_string) - .as_deref() - == Some(crate_name) + } else if key == crate_name + || sub + .filter(|(key, _)| key == "package") + .and_then(|(_, rest)| rest.trim_start().strip_prefix('=')) + .and_then(cargo_toml_string) + .as_deref() + == Some(crate_name) { pending.push(Pending::Refuse( "declared with dotted keys this rewriter does not edit".to_string(),