Skip to content

Commit a156d87

Browse files
committed
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
1 parent 7ba6b81 commit a156d87

1 file changed

Lines changed: 198 additions & 42 deletions

File tree

  • crates/socket-patch-core/src/patch/redirect

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

Lines changed: 198 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -894,6 +894,18 @@ fn rewrite_cargo(
894894
&root_workspace,
895895
) {
896896
Ok(plan) => {
897+
if let Err(reason) = validate_cargo_toml_pins(
898+
&plan.content,
899+
&dep.name,
900+
&dep.version,
901+
&other_versions,
902+
&reg,
903+
&plan.workspace,
904+
&root_workspace,
905+
) {
906+
refused = Some((path.clone(), reason));
907+
break;
908+
}
897909
if path == "Cargo.toml" {
898910
root_workspace = plan.workspace.clone();
899911
}
@@ -1740,6 +1752,96 @@ fn cargo_toml_inline_string(inner: &str, key: &str) -> Option<String> {
17401752
.map(str::to_string)
17411753
}
17421754

1755+
fn validate_cargo_toml_pins(
1756+
content: &str,
1757+
crate_name: &str,
1758+
version: &str,
1759+
other_versions: &[String],
1760+
registry: &str,
1761+
workspace: &BTreeMap<String, CargoWorkspaceEntry>,
1762+
inherited: &BTreeMap<String, CargoWorkspaceEntry>,
1763+
) -> Result<(), String> {
1764+
let document = content
1765+
.parse::<toml_edit::DocumentMut>()
1766+
.map_err(|_| "the planned manifest does not parse as TOML".to_string())?;
1767+
let unpinned = |dependencies: &dyn toml_edit::TableLike| {
1768+
dependencies.iter().find_map(|(key, entry)| {
1769+
let table = entry.as_table_like();
1770+
let field = |name: &str| table.and_then(|table| table.get(name));
1771+
if field("workspace").and_then(toml_edit::Item::as_bool) == Some(true) {
1772+
return match workspace.get(key).or(inherited.get(key)) {
1773+
Some(
1774+
CargoWorkspaceEntry::Pinned
1775+
| CargoWorkspaceEntry::OtherVersion
1776+
| CargoWorkspaceEntry::OtherPackage,
1777+
) => None,
1778+
None if key != crate_name => None,
1779+
None => Some(key.to_string()),
1780+
};
1781+
}
1782+
let name = field("package")
1783+
.and_then(toml_edit::Item::as_str)
1784+
.unwrap_or(key);
1785+
if name != crate_name {
1786+
return None;
1787+
}
1788+
let requirement = entry
1789+
.as_str()
1790+
.or_else(|| field("version").and_then(toml_edit::Item::as_str));
1791+
match cargo_req_selects(requirement, version, other_versions) {
1792+
CargoReqMatch::NotOurs => return None,
1793+
CargoReqMatch::Ambiguous => return Some(key.to_string()),
1794+
CargoReqMatch::Ours => {}
1795+
}
1796+
let is_pinned = field("registry").and_then(toml_edit::Item::as_str) == Some(registry)
1797+
&& field("path").is_none()
1798+
&& field("git").is_none()
1799+
&& field("registry-index").is_none();
1800+
(!is_pinned).then(|| key.to_string())
1801+
})
1802+
};
1803+
let mut scopes: Vec<&dyn toml_edit::TableLike> = vec![document.as_table()];
1804+
if let Some(targets) = document
1805+
.get("target")
1806+
.and_then(toml_edit::Item::as_table_like)
1807+
{
1808+
scopes.extend(
1809+
targets
1810+
.iter()
1811+
.filter_map(|(_, target)| target.as_table_like()),
1812+
);
1813+
}
1814+
for scope in scopes {
1815+
for kind in [
1816+
"dependencies",
1817+
"dev-dependencies",
1818+
"build-dependencies",
1819+
"dev_dependencies",
1820+
"build_dependencies",
1821+
] {
1822+
if let Some(key) = scope
1823+
.get(kind)
1824+
.and_then(toml_edit::Item::as_table_like)
1825+
.and_then(&unpinned)
1826+
{
1827+
return Err(format!("dependency declaration {key} was not pinned"));
1828+
}
1829+
}
1830+
}
1831+
if let Some(key) = document
1832+
.get("workspace")
1833+
.and_then(toml_edit::Item::as_table_like)
1834+
.and_then(|workspace| workspace.get("dependencies"))
1835+
.and_then(toml_edit::Item::as_table_like)
1836+
.and_then(unpinned)
1837+
{
1838+
return Err(format!(
1839+
"workspace dependency declaration {key} was not pinned"
1840+
));
1841+
}
1842+
Ok(())
1843+
}
1844+
17431845
struct CargoTomlPlan {
17441846
content: String,
17451847
edits: Vec<FileEdit>,
@@ -9070,6 +9172,86 @@ mod tests {
90709172
assert!(r.confirmed_cargo_uuids.contains(CARGO_UUID));
90719173
}
90729174

9175+
#[test]
9176+
fn cargo_root_dependency_forms_cannot_leave_partial_redirect() {
9177+
for dependency in [
9178+
"dependencies.serde = \"1.0.190\"",
9179+
"dependencies = { serde = \"1.0.190\" }",
9180+
"dependencies = { serde = { version = \"1.0.190\" } }",
9181+
"target.'cfg(unix)'.dependencies.serde = \"1.0.190\"",
9182+
"workspace.dependencies.serde = \"1.0.190\"",
9183+
"workspace = { dependencies = { serde = \"1.0.190\" } }",
9184+
] {
9185+
let manifest = format!(
9186+
"{dependency}\n\n[package]\nname = \"app\"\nversion = \"0.1.0\"\n\n\
9187+
[dev-dependencies]\nserde = \"1.0.190\"\n"
9188+
);
9189+
assert!(manifest.parse::<toml_edit::DocumentMut>().is_ok());
9190+
let result =
9191+
rewrite_registry_redirect(&cargo_files(&manifest), &[cargo_sparse_override()]);
9192+
assert!(result.files.is_empty(), "{manifest}: {:?}", result.files);
9193+
assert!(result.edits.is_empty(), "{manifest}");
9194+
assert!(result.confirmed_cargo_uuids.is_empty(), "{manifest}");
9195+
assert!(result
9196+
.warnings
9197+
.iter()
9198+
.any(|warning| warning.code == "redirect_cargo_toml_dep_unrewritable"));
9199+
}
9200+
}
9201+
9202+
#[test]
9203+
fn cargo_semantic_pin_guard_preserves_other_version_declarations() {
9204+
let manifest = "dependencies.serde = \"0.9\"\n\n\
9205+
[package]\nname = \"app\"\nversion = \"0.1.0\"\n\n\
9206+
[dev-dependencies]\nserde = \"1.0.190\"\n";
9207+
let mut files = cargo_files(manifest);
9208+
files.get_mut("Cargo.lock").unwrap().push_str(&format!(
9209+
"\n[[package]]\nname = \"serde\"\nversion = \"0.9.15\"\n\
9210+
source = \"registry+https://github.com/rust-lang/crates.io-index\"\n\
9211+
checksum = \"{}\"\n",
9212+
"a".repeat(64)
9213+
));
9214+
let result = rewrite_registry_redirect(&files, &[cargo_sparse_override()]);
9215+
assert!(result.warnings.is_empty(), "{:?}", result.warnings);
9216+
assert!(result.confirmed_cargo_uuids.contains(CARGO_UUID));
9217+
let document = result.files["Cargo.toml"]
9218+
.parse::<toml_edit::DocumentMut>()
9219+
.unwrap();
9220+
assert_eq!(document["dependencies"]["serde"].as_str(), Some("0.9"));
9221+
assert_eq!(
9222+
document["dev-dependencies"]["serde"]["registry"].as_str(),
9223+
Some(cargo_reg().as_str())
9224+
);
9225+
}
9226+
9227+
#[test]
9228+
fn cargo_semantic_pin_guard_checks_unchanged_members() {
9229+
let mut files = cargo_files(
9230+
"[package]\nname = \"app\"\nversion = \"0.1.0\"\n\n\
9231+
[workspace]\nmembers = [\"member\"]\n\n\
9232+
[dependencies]\nserde = \"1.0.190\"\n",
9233+
);
9234+
files.insert(
9235+
"member/Cargo.toml".to_string(),
9236+
"dependencies = { serde = \"1.0.190\" }\n\n\
9237+
[package]\nname = \"member\"\nversion = \"0.1.0\"\n"
9238+
.to_string(),
9239+
);
9240+
files.get_mut("Cargo.lock").unwrap().push_str(
9241+
"\n[[package]]\nname = \"member\"\nversion = \"0.1.0\"\n\
9242+
dependencies = [\"serde\"]\n",
9243+
);
9244+
let result = rewrite_registry_redirect(&files, &[cargo_sparse_override()]);
9245+
assert!(result.files.is_empty());
9246+
assert!(result.edits.is_empty());
9247+
assert!(result.confirmed_cargo_uuids.is_empty());
9248+
assert!(result.warnings.iter().any(|warning| {
9249+
warning.code == "redirect_cargo_toml_dep_unrewritable"
9250+
&& warning.detail.contains("member/Cargo.toml")
9251+
&& warning.detail.contains("was not pinned")
9252+
}));
9253+
}
9254+
90739255
#[test]
90749256
fn cargo_dotted_literal_renames_refuse_every_declaration() {
90759257
for alias in ["alias", "\"alias\"", "'alias'"] {
@@ -16535,64 +16717,38 @@ packages:
1653516717
// tolerance legs, workspace-inheritance satisfaction, and the remaining
1653616718
// diagnosis spellings.
1653716719

16538-
/// Malformed Cargo.toml section headers (unbalanced quote in a segment,
16539-
/// an unclosed `[dependencies`) must classify as non-dependency sections
16540-
/// — their entries stay byte-identical — and garbage lines inside the
16541-
/// real [dependencies] table are skipped while the real entry still
16542-
/// gains the pin.
1654316720
#[test]
16544-
fn cargo_malformed_headers_and_table_lines_are_skipped_not_fatal() {
16721+
fn cargo_malformed_manifest_headers_and_lines_refuse_redirect() {
1654516722
let files = cargo_files(
1654616723
"[package]\nname = \"app\"\nversion = \"0.1.0\"\n\n\
1654716724
[target.'cfg(unix).dependencies]\nserde = \"9.9.9\"\n\n\
1654816725
[dependencies\nserde = \"8.8.8\"\n\n\
1654916726
[dependencies]\n= \"junk\"\njunk\nserde = \"1.0.190\"\n",
1655016727
);
1655116728
let r = rewrite_registry_redirect(&files, &[cargo_sparse_override()]);
16552-
assert!(
16553-
r.warnings.is_empty(),
16554-
"garbage headers/lines are skipped, not refused: {:?}",
16555-
r.warnings
16556-
);
16557-
let toml = r.files.get("Cargo.toml").expect("Cargo.toml rewritten");
16558-
let pinned = format!(
16559-
"serde = {{ version = \"1.0.190\", registry = \"{}\" }}",
16560-
cargo_reg()
16561-
);
16562-
assert_eq!(
16563-
toml.matches(&pinned).count(),
16564-
1,
16565-
"only the real [dependencies] entry is pinned: {toml}"
16566-
);
16567-
assert!(
16568-
toml.contains("serde = \"9.9.9\"") && toml.contains("serde = \"8.8.8\""),
16569-
"entries under malformed headers stay byte-identical: {toml}"
16570-
);
16571-
assert!(
16572-
toml.contains("= \"junk\"\njunk\n"),
16573-
"garbage table lines survive untouched: {toml}"
16574-
);
16729+
assert!(r.files.is_empty());
16730+
assert!(r.edits.is_empty());
16731+
assert!(r.confirmed_cargo_uuids.is_empty());
16732+
assert!(r.warnings.iter().any(|warning| {
16733+
warning.code == "redirect_cargo_toml_dep_unrewritable"
16734+
&& warning.detail.contains("does not parse as TOML")
16735+
}));
1657516736
}
1657616737

16577-
/// Unparseable lines INSIDE a `[dependencies.<key>]` table block (a bare
16578-
/// `= …`, a key token with no `=`) are skipped by the block scanner while
16579-
/// the block still gains its `registry` pin right after the header.
1658016738
#[test]
16581-
fn cargo_dep_entry_block_garbage_lines_are_skipped() {
16739+
fn cargo_malformed_dep_entry_block_refuses_redirect() {
1658216740
let files = cargo_files(
1658316741
"[package]\nname = \"app\"\nversion = \"0.1.0\"\n\n\
1658416742
[dependencies.serde]\n= \"zap\"\npackage \"serde\"\nversion = \"1.0.190\"\n",
1658516743
);
1658616744
let r = rewrite_registry_redirect(&files, &[cargo_sparse_override()]);
16587-
assert!(r.warnings.is_empty(), "{:?}", r.warnings);
16588-
let toml = r.files.get("Cargo.toml").expect("Cargo.toml rewritten");
16589-
assert!(
16590-
toml.contains(&format!(
16591-
"[dependencies.serde]\nregistry = \"{}\"\n= \"zap\"\npackage \"serde\"\nversion = \"1.0.190\"",
16592-
cargo_reg()
16593-
)),
16594-
"registry pin inserted after the header, garbage lines untouched: {toml}"
16595-
);
16745+
assert!(r.files.is_empty());
16746+
assert!(r.edits.is_empty());
16747+
assert!(r.confirmed_cargo_uuids.is_empty());
16748+
assert!(r.warnings.iter().any(|warning| {
16749+
warning.code == "redirect_cargo_toml_dep_unrewritable"
16750+
&& warning.detail.contains("does not parse as TOML")
16751+
}));
1659616752
}
1659716753

1659816754
/// A `[workspace.dependencies]` entry ALREADY pinned to the managed

0 commit comments

Comments
 (0)