From 41cea95711c3db60654b8b41447c4a74a4518114 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 13:30:24 +0000 Subject: [PATCH] Imports: refuse a NuGet feed's .nupkg that is not the listed id and version A feed import passed no expected identity to the hosted write path, so a feed whose flat container served another package's file for a listed version stored that other package and, with on_conflict overwrite, replaced a version Forge already held. Every package import now passes the listed id and version, like the npm and apt imports, and the item fails with "the package is ..., not ...". Fixes #813 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012e6DJb9nERqzebpXXBCBRg --- crates/server/src/import/mod.rs | 8 +-- crates/server/tests/import.rs | 86 ++++++++++++++++++++++++++++++ docs/changes/2.2.0/813.security.md | 5 ++ 3 files changed, 95 insertions(+), 4 deletions(-) create mode 100644 docs/changes/2.2.0/813.security.md diff --git a/crates/server/src/import/mod.rs b/crates/server/src/import/mod.rs index de8fd7c1..67ed2f0f 100644 --- a/crates/server/src/import/mod.rs +++ b/crates/server/src/import/mod.rs @@ -1548,10 +1548,10 @@ impl Run<'_> { downloaded_at: Some(item.downloaded_at.unwrap_or_else(Utc::now)), uploader: item.uploader.clone(), source: ctx.source.clone(), - // A registry names the tarball in the package's packument: it - // must be that package. - expected: matches!(from, PackageSource::Npm(_)) - .then(|| (item.name.clone(), item.version.clone())), + // The source listed this id and version and named the file: it + // must be that package, or a feed could plant (and, with + // overwrite, replace) another one. + expected: Some((item.name.clone(), item.version.clone())), summary_fields: None, }; let outcome = writer diff --git a/crates/server/tests/import.rs b/crates/server/tests/import.rs index 8af57f4c..77777e2e 100644 --- a/crates/server/tests/import.rs +++ b/crates/server/tests/import.rs @@ -1358,6 +1358,92 @@ async fn a_failed_item_is_retried_on_its_own() { ); } +/// A feed whose flat container serves another package's .nupkg for a +/// version it lists plants nothing, whatever the registration's SHA-512 +/// says: not a version Forge lacks, and not over one it has, even with +/// `overwrite`. +#[tokio::test] +async fn a_feed_cannot_plant_another_package_under_a_listed_version() { + let srv = forge().await; + let source = Source::start().await; + let repo = repo_name("nuget-planted"); + create_repo(&srv, json!({"name": repo, "kind": "hosted"})).await; + // Forge holds Acme.Victim 2.0.0. + let ours = make_nupkg("Acme.Victim", "2.0.0"); + let token = srv.admin_token().await; + let resp = srv + .client() + .put(format!("{}/repository/{repo}/package", srv.base_url)) + .header("X-NuGet-ApiKey", &token) + .multipart(reqwest::multipart::Form::new().part( + "package", + reqwest::multipart::Part::bytes(ours.clone()).file_name("p.nupkg"), + )) + .send() + .await + .unwrap(); + assert_eq!(resp.status(), 201); + + // The feed lists Acme.Foo 1.0.0 and Acme.Bar 1.0.0, and its + // registration lists the SHA-512 of the files it serves for them: other + // bytes of Acme.Victim 2.0.0, and Acme.Fresh 3.0.0 (which Forge lacks). + source.nuget( + "nuget-hosted", + "Acme.Foo", + "1.0.0", + make_nupkg_with_payload("Acme.Victim", "2.0.0", 64), + "2021-01-01T00:00:00.000+00:00", + ); + source.nuget( + "nuget-hosted", + "Acme.Bar", + "1.0.0", + make_nupkg("Acme.Fresh", "3.0.0"), + "2021-01-01T00:00:00.000+00:00", + ); + + for on_conflict in ["skip", "overwrite"] { + let mut body = from(source.feed_url("nuget-hosted")); + body["on_conflict"] = json!(on_conflict); + let id = start(&srv, &repo, body).await; + let done = run(&srv, &repo, id).await; + assert_eq!(done["state"], "completed", "{on_conflict}: {done}"); + let p = &done["progress"]; + assert_eq!( + ( + p["failed"].as_i64(), + p["imported"].as_i64(), + p["replaced"].as_i64() + ), + (Some(2), Some(0), Some(0)), + "{on_conflict}: {p}" + ); + for (name, planted) in [ + ("Acme.Foo", "Acme.Victim 2.0.0"), + ("Acme.Bar", "Acme.Fresh 3.0.0"), + ] { + let failed = item_of(&srv, &repo, id, name, "1.0.0").await; + assert_eq!(failed["state"], "failed", "{on_conflict}: {failed}"); + assert!( + failed["detail"] + .as_str() + .unwrap() + .contains(&format!("the package is {planted}, not {name} 1.0.0")), + "{on_conflict}: {failed}" + ); + } + // Nothing stored, neither under the listed ids nor the planted ones. + assert!(nupkg(&srv, &repo, "Acme.Foo", "1.0.0").await.is_none()); + assert!(nupkg(&srv, &repo, "Acme.Bar", "1.0.0").await.is_none()); + assert!(nupkg(&srv, &repo, "Acme.Fresh", "3.0.0").await.is_none()); + assert_eq!( + nupkg(&srv, &repo, "Acme.Victim", "2.0.0").await.unwrap(), + ours, + "{on_conflict}" + ); + } +} + #[tokio::test] async fn cancel_resume_delete_and_one_import_at_a_time() { let srv = forge().await; diff --git a/docs/changes/2.2.0/813.security.md b/docs/changes/2.2.0/813.security.md new file mode 100644 index 00000000..24cbb5df --- /dev/null +++ b/docs/changes/2.2.0/813.security.md @@ -0,0 +1,5 @@ +A NuGet feed import now refuses a `.nupkg` that is not the id and version the +feed listed, and reports the item as failed. Before, a compromised or +misbehaving feed could serve another package's file for a version it listed: +the import stored that other package, and with `on_conflict: overwrite` +replaced a version Forge already held with the feed's bytes.