From d3e7ef07fce7f8354964dca8e35e74a328c2c874 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 23 Sep 2026 17:35:02 -0400 Subject: [PATCH 1/7] fix(vendor): refresh the ledger fingerprint when a wired gem/maven/nuget artifact is rebuilt When the hot path found the committed artifact missing or stale, it rebuilt it but returned no entry, so the ledger kept the previous fingerprint: the gem file inventory, the maven/nuget sha256, and the nuget lock pin. A rebuild from the other source (service vs local) then produced bytes the ledger did not describe. VEX and verify reported tamper, repair of a service-vendored maven/nuget entry could fail, --revert left packages.lock.json pinned to the patched contentHash, and a service-sourced rebuild was labelled already_vendored. The rebuild branches now return a refreshed entry built from the new bytes or tree, with no wiring of their own. For nuget the entry carries only the re-pinned lock record, with original: None. carry_forward_wiring (same uuid) re-attaches the first run's records and fills in the true pre-vendor hash. The CLI does not record such an entry when the ledger has no previous one, because it would carry no wiring and --revert would delete the artifact while the project still points at it. repair carries the repaired entry's wiring forward before persisting, and emits vendor_inventory_refreshed when the backend's refreshed inventory differs from the recorded one. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 15 ++ crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../src/commands/repair_vendor.rs | 41 +++- .../socket-patch-cli/src/commands/vendor.rs | 10 + .../tests/in_process_vendor.rs | 80 +++++++ crates/socket-patch-core/src/vendor/gem.rs | 168 +++++++++++-- .../src/vendor/maven_repo.rs | 191 +++++++++++++-- .../src/vendor/nuget_feed.rs | 221 +++++++++++++++++- 8 files changed, 666 insertions(+), 62 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5277041c..91d4025a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -639,6 +639,21 @@ into the new version's section — see docs/releasing.md. contract; previously the entry was deleted, stranding a live ledger entry with no backing record. An all-kept run exits 1 `partialFailure` with `summary.removed: 0` (never `not_found` — the identifier matched). +- **Rebuilding a missing gem, maven or nuget vendored artifact now updates + the ledger.** When `vendor` / `scan --vendor` found a wired project whose + committed artifact was missing or broken, it rebuilt the artifact but kept + the old fingerprint in `.socket/vendor/state.json` (the gem file + inventory, the maven/nuget `sha256`, and the nuget `packages.lock.json` + pin). If the rebuild came from the other source (the patch service instead + of a local build, or the reverse), the new bytes no longer matched the + ledger. VEX and verification then reported the artifact as tampered, + `repair` could fail, and `vendor --revert` left `packages.lock.json` + pinned to the patched `contentHash`. A rebuild from the patch service was + also reported as `already_vendored` instead of `applied`. The rebuild now + records the new fingerprint and keeps the entry's original wiring records, + so revert still restores the pre-vendor files. If `state.json` has no + entry for the package, the rebuild still runs but no entry is added, + because the run has no pre-vendor originals to record. ### Changed diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index caaf2ae5..c8779a1e 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1123,7 +1123,7 @@ Every `--json` invocation emits a single JSON object that follows the **unified | `vendor_fetch_unverifiable` | `skipped` (warning) | vendor: the lockfile records no usable integrity for the missing package; nothing was fetched (fail-closed) and the `package_not_installed` skip follows. | | `vendor_artifact_missing` | `skipped` (warning) / `failed` | vendor: the committed artifact is gone — the registry resolution is recovered from the ledger and the artifact rebuilt (warning); repair `--offline` with no local source surfaces it as the per-entry failure instead. | | `vendor_artifact_corrupt` | `failed` | repair `--offline`: the committed artifact fails verification (member afterHashes or the ledger's whole-file sha256) and no local source can rebuild it. Online repairs rebuild instead. | -| `vendor_artifact_rebuilt` | `skipped` (warning) | vendor / scan `--vendor`: a wired-but-missing/stale artifact was rebuilt in place; lockfiles and the ledger entry untouched. (Under `repair` the `rebuilt` event carries this signal.) | +| `vendor_artifact_rebuilt` | `skipped` (warning) | vendor / scan `--vendor`: a wired-but-missing/stale artifact was rebuilt in place. The lockfiles are untouched, except that nuget re-pins `packages.lock.json` to the rebuilt bytes. gem/maven/nuget: the package's event is `applied` (also for a rebuild from the patch service), and the ledger entry's artifact fingerprint (gem `fileInventory`, maven/nuget `sha256` + `size`, and the nuget lock pin) is refreshed to the rebuilt bytes, and its wiring records are kept unchanged, so `--revert` still restores the pre-vendor files. A rebuild with no existing ledger entry records none. Other ecosystems leave the ledger entry untouched. (Under `repair` the `rebuilt` event carries this signal.) | | `vendor_artifact_rebuild_failed` | `failed` | repair: the rebuild ran but the result failed verification against the recorded fingerprint (e.g. an edited state.json sha); the unverifiable artifact was removed. | | `vendor_artifact_unrepairable` | `failed` | repair: no verifiable pristine source exists (not installed + lockfile rewired + no recoverable ledger fragment), the wheel is platform-locked with no installed copy, or the ledger entry itself cannot be trusted. | | `vendor_uuid_mismatch` | `skipped` | repair: the manifest's patch uuid moved past the vendored artifact — a re-vendor (`vendor` / `scan --vendor`) is pending; repair does not cross patch generations. | diff --git a/crates/socket-patch-cli/src/commands/repair_vendor.rs b/crates/socket-patch-cli/src/commands/repair_vendor.rs index 5817afec..f0f13e12 100644 --- a/crates/socket-patch-cli/src/commands/repair_vendor.rs +++ b/crates/socket-patch-cli/src/commands/repair_vendor.rs @@ -1541,6 +1541,30 @@ pub(crate) async fn repair_vendored_artifacts_with_references( // fingerprint computed from the rebuilt bytes. let from_backend = entry.is_some(); let mut check_entry = entry.unwrap_or_else(|| c.entry.clone()); + // An artifact-only rebuild hands back a refreshed entry with + // no wiring of its own: re-attach the repaired entry's + // records (a reconstructed entry is not in the ledger yet, + // so the persist below has nothing to carry them from). + if from_backend { + vendor::carry_forward_wiring(&c.entry, &mut check_entry); + } + // The backend's refreshed entry already re-inventoried the + // member-verified rebuild; a changed inventory is the same + // provenance flip the post-verify refresh below reports. + if from_backend + && c.entry.artifact.file_inventory.is_some() + && check_entry.artifact.file_inventory != c.entry.artifact.file_inventory + { + record_warning( + env, + &c.purl, + &VendorWarning::new( + "vendor_inventory_refreshed", + INVENTORY_REFRESHED_DETAIL, + ), + common, + ); + } if !from_backend && c.reconstructed { fill_artifact_fingerprint(&common.cwd, &mut check_entry).await; } @@ -1588,13 +1612,7 @@ pub(crate) async fn repair_vendored_artifacts_with_references( &c.purl, &VendorWarning::new( "vendor_inventory_refreshed", - "the rebuilt artifact's patched files verify but its \ - tree differs from the recorded file inventory (the \ - entry was likely vendored from the patch service's \ - prebuilt artifact; repair rebuilds locally); the \ - inventory was refreshed from the verified rebuild — \ - run `socket-patch vendor` to restore the \ - service-built tree", + INVENTORY_REFRESHED_DETAIL, ), common, ); @@ -1660,6 +1678,15 @@ pub(crate) async fn repair_vendored_artifacts_with_references( rebuilt } +/// Detail of the `vendor_inventory_refreshed` advisory. +const INVENTORY_REFRESHED_DETAIL: &str = "the rebuilt artifact's patched files verify but its \ + tree differs from the recorded file inventory (the \ + entry was likely vendored from the patch service's \ + prebuilt artifact; repair rebuilds locally); the \ + inventory was refreshed from the verified rebuild — \ + run `socket-patch vendor` to restore the \ + service-built tree"; + /// Compute and record the artifact fingerprint on a re-synthesized ledger /// entry: sha256 + size for file-shaped artifacts, the whole-tree file /// inventory for dir-shaped ones. An uninventoriable dir stays `None` — diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index 3628764a..57ee83dc 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -1781,6 +1781,16 @@ pub(crate) async fn vendor_records( record_warning(env, candidate, w, common); } } + // An artifact-only rebuild hands back a refreshed + // fingerprint with no wiring of its own: it relies on + // the ledger entry it replaces for the pre-vendor + // originals. With no such entry, recording it would give + // `--revert` an entry that deletes the artifact yet + // cannot unwire the project — leave the ledger as is. + let entry = entry.filter(|_| { + state.entries.contains_key(candidate.as_str()) + || !warnings.iter().any(|w| w.code == "vendor_artifact_rebuilt") + }); if let Some(entry) = entry { if let Some(flavor) = entry.flavor.as_deref() { wired_flavors.insert(flavor.to_string()); diff --git a/crates/socket-patch-cli/tests/in_process_vendor.rs b/crates/socket-patch-cli/tests/in_process_vendor.rs index 7a92bb60..c94c7995 100644 --- a/crates/socket-patch-cli/tests/in_process_vendor.rs +++ b/crates/socket-patch-cli/tests/in_process_vendor.rs @@ -2234,6 +2234,86 @@ async fn scan_vendor_gem_detached_writes_no_manifest_and_reverts() { assert!(!fx.root().join(".socket/vendor").exists()); } +/// A wired gem whose committed copy went missing is rebuilt artifact-only, +/// and the ledger entry the run persists must still describe it: the +/// refreshed fingerprint (inventory) verifies against the rebuilt tree and +/// the first run's pair-edit records ride along, so `vendor --revert` still +/// byte-restores both files. +#[tokio::test] +async fn scan_vendor_gem_artifact_rebuild_keeps_ledger_verifiable_and_revertable() { + let mock = wiremock::MockServer::start().await; + mount_gem_patch_api(&mock, GEM_PURL).await; + let fx = gem_fixture(); + let (code, env) = run_scan_vendor(fx.root(), &mock.uri(), &[]); + assert_eq!(code, 0, "first vendor: {env:#}"); + let state1: Value = serde_json::from_slice(&std::fs::read(fx.state_path()).unwrap()).unwrap(); + + std::fs::remove_file(fx.vendored_lib()).unwrap(); + let (code, env2) = run_scan_vendor(fx.root(), &mock.uri(), &[]); + assert_eq!(code, 0, "rebuild run: {env2:#}"); + assert_eq!(std::fs::read(fx.vendored_lib()).unwrap(), GEM_PATCHED); + let state2: Value = serde_json::from_slice(&std::fs::read(fx.state_path()).unwrap()).unwrap(); + let entry2 = &state2["entries"][GEM_PURL]; + assert_eq!( + entry2["wiring"], state1["entries"][GEM_PURL]["wiring"], + "the pair-edit revert records survive the artifact-only rebuild" + ); + assert!( + entry2["artifact"]["fileInventory"].is_object(), + "the rebuilt tree is inventoried: {entry2:#}" + ); + + // `repair` re-checks every ledger fingerprint: nothing to rebuild. + let root = fx.root().to_str().unwrap(); + let (code, stdout, stderr) = run_cli( + fx.root(), + &["repair", "--json", "--dry-run", "--offline", "--cwd", root], + &[], + ); + assert_eq!(code, 0, "repair --dry-run: {stdout}\n{stderr}"); + assert!( + !stdout.contains("wouldRebuild"), + "the persisted fingerprint must verify: {stdout}" + ); + + let (code, renv) = vendor_cli(fx.root(), &["--revert"]); + assert_eq!(code, 0, "revert: {renv:#}"); + assert_eq!( + std::fs::read(fx.gemfile_path()).unwrap(), + GEM_GEMFILE.as_bytes() + ); + assert_eq!(std::fs::read(fx.lock_path()).unwrap(), GEM_LOCK.as_bytes()); +} + +/// The same artifact-only rebuild with NO ledger entry to refresh (the +/// state file was lost) must not invent one: the refreshed entry carries +/// no wiring of its own, so recording it would give `vendor --revert` an +/// entry that deletes the copy while the Gemfile still points at it. +#[tokio::test] +async fn scan_vendor_gem_artifact_rebuild_without_ledger_entry_records_none() { + let mock = wiremock::MockServer::start().await; + mount_gem_patch_api(&mock, GEM_PURL).await; + let fx = gem_fixture(); + let (code, env) = run_scan_vendor(fx.root(), &mock.uri(), &[]); + assert_eq!(code, 0, "first vendor: {env:#}"); + + std::fs::remove_file(fx.state_path()).unwrap(); + std::fs::remove_file(fx.vendored_lib()).unwrap(); + let (code, env2) = run_scan_vendor(fx.root(), &mock.uri(), &[]); + assert_eq!(code, 0, "rebuild run: {env2:#}"); + assert_eq!( + std::fs::read(fx.vendored_lib()).unwrap(), + GEM_PATCHED, + "the copy is still rebuilt" + ); + let recorded = std::fs::read(fx.state_path()) + .ok() + .and_then(|b| serde_json::from_slice::(&b).ok()) + .map(|s| !s["entries"][GEM_PURL].is_null()) + .unwrap_or(false); + assert!(!recorded, "no wiring-less ledger entry invented: {env2:#}"); +} + // ───────────────────────────────────────────────────────────────────── // hosted → vendored mode conversion (takeover reconciliation, pnpm v9) // ───────────────────────────────────────────────────────────────────── diff --git a/crates/socket-patch-core/src/vendor/gem.rs b/crates/socket-patch-core/src/vendor/gem.rs index 7b4ec7ae..3e53f98a 100644 --- a/crates/socket-patch-core/src/vendor/gem.rs +++ b/crates/socket-patch-core/src/vendor/gem.rs @@ -374,16 +374,31 @@ pub async fn vendor_gem( Ok(result) => result, Err(outcome) => return *outcome, }; - if result.success { - warnings.push(VendorWarning::new( - "vendor_artifact_rebuilt", - format!( - "the committed vendored copy for {name}@{version} was missing or \ - stale; rebuilt at {copy_rel} (Gemfile and Gemfile.lock untouched)" - ), - )); + if !result.success { + return done(result, None, warnings); } - return done(result, None, warnings); + warnings.push(VendorWarning::new( + "vendor_artifact_rebuilt", + format!( + "the committed vendored copy for {name}@{version} was missing or \ + stale; rebuilt at {copy_rel} (Gemfile and Gemfile.lock untouched)" + ), + )); + // The rebuilt tree may differ from the one the ledger + // inventoried (a service ↔ local flip swaps the stub gemspec): + // hand back a refreshed entry. Its wiring is empty ON PURPOSE — + // the caller's `carry_forward_wiring` (same uuid) re-attaches the + // first run's records, the only copy of the pre-vendor originals. + let file_inventory = + gem_inventory_or_warn(©_dir, name, version, &mut warnings).await; + let entry = gem_entry( + build_gem_purl(name, version), + record, + copy_rel, + file_inventory, + Vec::new(), + ); + return done(result, Some(entry), warnings); } // Dry runs fall through to the verify-only preview below. } else { @@ -626,13 +641,25 @@ pub async fn vendor_gem( } } - // Whole-tree inventory of the committed copy (stub gemspec included): - // no lockfile integrity covers a path source's bytes, so this is the - // only whole-artifact drift/tamper anchor verify/VEX/repair have for a - // dir-shaped artifact. Fail-soft: an uninventoriable copy (symlink, - // non-UTF-8 name) vendors like a pre-inventory entry, with the gap - // surfaced here and again at repair time. - let file_inventory = match super::verify::compute_dir_inventory(©_dir).await { + let file_inventory = gem_inventory_or_warn(©_dir, name, version, &mut warnings).await; + let entry = gem_entry(base_purl, record, copy_rel, file_inventory, wiring); + + done(result, Some(entry), warnings) +} + +/// Whole-tree inventory of the committed copy (stub gemspec included): no +/// lockfile integrity covers a path source's bytes, so this is the only +/// whole-artifact drift/tamper anchor verify/VEX/repair have for a +/// dir-shaped artifact. Fail-soft: an uninventoriable copy (symlink, +/// non-UTF-8 name) vendors like a pre-inventory entry, with the gap +/// surfaced here and again at repair time. +async fn gem_inventory_or_warn( + copy_dir: &Path, + name: &str, + version: &str, + warnings: &mut Vec, +) -> Option> { + match super::verify::compute_dir_inventory(copy_dir).await { Ok(inv) => Some(inv), Err(detail) => { warnings.push(VendorWarning::new( @@ -644,9 +671,20 @@ pub async fn vendor_gem( )); None } - }; + } +} - let entry = VendorEntry { +/// The ledger entry for a vendored gem copy: `wiring` is the Gemfile + lock +/// records on a full vendor, empty on an artifact-only rebuild (see the hot +/// path). +fn gem_entry( + base_purl: String, + record: &PatchRecord, + copy_rel: String, + file_inventory: Option>, + wiring: Vec, +) -> VendorEntry { + VendorEntry { ecosystem: "gem".to_string(), base_purl, uuid: record.uuid.clone(), @@ -668,9 +706,7 @@ pub async fn vendor_gem( poetry: None, pdm: None, pipenv: None, - }; - - done(result, Some(entry), warnings) + } } // ── materialisation (service download / local build) ────────────────────────── @@ -3368,9 +3404,18 @@ mod tests { let (r2, e2, w2) = unwrap_done(run_vendor(&root, &blobs, &installed, &record, false).await); assert!(r2.success, "{:?}", r2.error); + // Artifact-only rebuild: a refreshed fingerprint with NO wiring of + // its own (re-recording the live pair edit as `original` would + // break --revert; the caller carries the first run's records). + let e2 = e2.expect("the rebuild refreshes the ledger fingerprint"); assert!( - e2.is_none(), - "artifact-only rebuild must not re-record the ledger entry" + e2.wiring.is_empty(), + "no re-recorded wiring: {:?}", + e2.wiring + ); + assert!( + e2.artifact.file_inventory.is_some(), + "rebuilt tree inventoried" ); assert!( w2.iter().any(|w| w.code == "vendor_artifact_rebuilt"), @@ -5206,9 +5251,11 @@ mod tests { let (result2, entry2, warnings2) = unwrap_done(run_vendor(&root, &blobs, &installed, &record, false).await); assert!(result2.success, "{:?}", result2.error); + let entry2 = entry2.expect("the rebuild refreshes the ledger fingerprint"); assert!( - entry2.is_none(), - "artifact-only rebuild must not re-record a ledger entry" + entry2.wiring.is_empty(), + "artifact-only rebuild must not re-record wiring: {:?}", + entry2.wiring ); assert!( warnings2 @@ -8068,4 +8115,75 @@ mod tests { "the freshly-built uuid dir is unwound" ); } + + /// Vendored from the service (served stub gemspec), the + /// committed copy is lost and rebuilt LOCALLY (local gemspec → different + /// tree). The ledger the CLI persists must carry the rebuilt tree's + /// inventory, and its carried-forward wiring must still revert cleanly. + #[tokio::test] + async fn wired_rebuild_refreshes_ledger_inventory() { + let (_tmp, root, installed, blobs, record) = fixture(GEMFILE_DIRECT, LOCK_DIRECT).await; + let gem = make_gem(&[("lib/rack.rb", PATCHED)]); + let server = wiremock::MockServer::start().await; + mount_gem_granted( + &server, + &gem, + &sri_sha512(&gem), + Some((SERVICE_STUB, &sri_sha512(SERVICE_STUB))), + ) + .await; + let cfg = gem_service_cfg(&server.uri(), VendorSource::Service, false); + let (r1, e1, _) = + unwrap_done(run_vendor_service(&root, &blobs, &installed, &record, &cfg).await); + assert!(r1.success, "{:?}", r1.error); + let e1 = e1.expect("first vendor records an entry"); + assert_eq!( + crate::vendor::check_vendored_artifact(&root, &e1, &record).await, + crate::vendor::ArtifactHealth::Healthy + ); + + tokio::fs::remove_file(copy_lib(&root)).await.unwrap(); + let (r2, e2, w2) = unwrap_done(run_vendor(&root, &blobs, &installed, &record, false).await); + assert!(r2.success, "{:?}", r2.error); + assert!( + w2.iter().any(|w| w.code == "vendor_artifact_rebuilt"), + "{w2:?}" + ); + assert_eq!( + tokio::fs::read_to_string(copy_gemspec(&root)) + .await + .unwrap(), + GEMSPEC, + "precondition: the local rebuild used the local stub" + ); + + let ledger = match e2 { + Some(mut fresh) => { + crate::vendor::carry_forward_wiring(&e1, &mut fresh); + fresh + } + None => e1.clone(), + }; + assert_eq!( + crate::vendor::check_vendored_artifact(&root, &ledger, &record).await, + crate::vendor::ArtifactHealth::Healthy, + "the ledger inventory must describe the rebuilt copy" + ); + assert_eq!( + ledger.wiring, e1.wiring, + "Gemfile/lock revert records preserved" + ); + let rv = revert_gem(&ledger, &root, false).await; + assert!(rv.success, "{:?}", rv.error); + assert_eq!( + tokio::fs::read_to_string(root.join(GEMFILE)).await.unwrap(), + GEMFILE_DIRECT + ); + assert_eq!( + tokio::fs::read_to_string(root.join(GEMFILE_LOCK)) + .await + .unwrap(), + LOCK_DIRECT + ); + } } diff --git a/crates/socket-patch-core/src/vendor/maven_repo.rs b/crates/socket-patch-core/src/vendor/maven_repo.rs index d27cb8e3..141cf90e 100644 --- a/crates/socket-patch-core/src/vendor/maven_repo.rs +++ b/crates/socket-patch-core/src/vendor/maven_repo.rs @@ -271,7 +271,7 @@ pub async fn vendor_maven( // re-record the live vendored pom.xml as `original`, breaking revert. if !dry_run { let mut warnings: Vec = vec![shadow_warning]; - let (_bytes, mut result) = match materialise_and_write( + let (jar_bytes, mut result) = match materialise_and_write( purl, installed_dir, &uuid_dir, @@ -305,7 +305,19 @@ pub async fn vendor_maven( stale; rebuilt at {leaf_rel} (pom.xml untouched)" ), )); - return done(result, None, warnings); + // The rebuilt jar may differ byte-wise from the one the ledger + // fingerprinted (a service ↔ local flip): hand back a refreshed + // entry. Its wiring is empty ON PURPOSE — the caller's + // `carry_forward_wiring` (same uuid) re-attaches the first run's + // records, the only copy of the verbatim pre-vendor pom.xml. + let entry = maven_entry( + build_maven_purl(group_id, artifact_id, version), + record, + jar_copy_rel, + &jar_bytes, + Vec::new(), + ); + return done(result, Some(entry), warnings); } // Dry runs fall through to the verify-only preview below. } @@ -390,7 +402,34 @@ pub async fn vendor_maven( // (the pom.xml itself always pre-existed — a gradle-only / // pom-less project is refused above); revert restores the `original` bytes // when the live pom.xml still carries our repo id. - let entry = VendorEntry { + let entry = maven_entry( + base_purl, + record, + jar_copy_rel, + &jar_bytes, + vec![WiringRecord { + file: PROJECT_POM.to_string(), + kind: REPO_WIRING_KIND.to_string(), + action: WiringAction::Added, + key: Some(repo_id), + original: Some(Value::String(pom_xml_text)), + new: Some(Value::String(new_pom_xml)), + }], + ); + + done(result, Some(entry), warnings) +} + +/// The ledger entry for a vendored jar: `wiring` is the pom.xml record on a +/// full vendor, empty on an artifact-only rebuild (see the hot path). +fn maven_entry( + base_purl: String, + record: &PatchRecord, + jar_copy_rel: String, + jar_bytes: &[u8], + wiring: Vec, +) -> VendorEntry { + VendorEntry { ecosystem: "maven".to_string(), base_purl, uuid: record.uuid.clone(), @@ -399,19 +438,12 @@ pub async fn vendor_maven( // tooling (harvest re-derives per-entry git hashes from the zip, so // the vendored copy is self-describing without a network). path: jar_copy_rel, - sha256: hex::encode(Sha256::digest(&jar_bytes)), + sha256: hex::encode(Sha256::digest(jar_bytes)), size: Some(jar_bytes.len() as u64), platform_locked: None, file_inventory: None, }, - wiring: vec![WiringRecord { - file: PROJECT_POM.to_string(), - kind: REPO_WIRING_KIND.to_string(), - action: WiringAction::Added, - key: Some(repo_id), - original: Some(Value::String(pom_xml_text)), - new: Some(Value::String(new_pom_xml)), - }], + wiring, lock: None, took_over_go_patches: false, detached: false, @@ -422,9 +454,7 @@ pub async fn vendor_maven( poetry: None, pdm: None, pipenv: None, - }; - - done(result, Some(entry), warnings) + } } /// Revert a Maven vendor entry: surgically remove our `` from @@ -1556,9 +1586,13 @@ mod tests { let (r2, e2, w2) = unwrap_done(run_vendor(root, &blobs, &installed, &record, false).await); assert!(r2.success, "{:?}", r2.error); + // A refreshed fingerprint with NO wiring of its own: re-recording + // would clobber the pre-vendor pom.xml (the caller carries it). + let e2 = e2.expect("the rebuild refreshes the ledger fingerprint"); assert!( - e2.is_none(), - "artifact-only rebuild must not re-record (would clobber the pre-vendor pom.xml)" + e2.wiring.is_empty(), + "no re-recorded wiring: {:?}", + e2.wiring ); assert!( w2.iter().any(|w| w.code == "vendor_artifact_rebuilt"), @@ -2256,7 +2290,10 @@ mod tests { .await; let (r2, e2, w2) = unwrap_done(outcome); assert!(r2.success, "{:?}", r2.error); - assert!(e2.is_none(), "artifact-only rebuild must not re-record"); + assert!( + e2.is_some_and(|e| e.wiring.is_empty()), + "artifact-only rebuild refreshes the fingerprint, never the wiring" + ); assert!( w2.iter().any(|w| w.code == "vendor_artifact_rebuilt"), "FIFO pom must read as stale and trigger the rebuild: {w2:?}" @@ -3431,7 +3468,10 @@ mod tests { let (r2, e2, w2) = unwrap_done(run_vendor(root, &blobs, &installed, &record, false).await); assert!(r2.success, "{:?}", r2.error); - assert!(e2.is_none(), "artifact-only rebuild must not re-record"); + assert!( + e2.is_some_and(|e| e.wiring.is_empty()), + "artifact-only rebuild refreshes the fingerprint, never the wiring" + ); assert!( w2.iter().any(|w| w.code == "vendor_artifact_rebuilt"), "a missing sidecar must read as stale and rebuild: {w2:?}" @@ -3592,4 +3632,117 @@ mod tests { let no_close = "\n \n "; assert_eq!(strip_empty_repositories(no_close), no_close); } + + /// Mount a granted service response serving `body` as the prebuilt jar. + async fn mount_granted_jar(body: &[u8]) -> wiremock::MockServer { + use wiremock::matchers::{method, path}; + use wiremock::{Mock, MockServer, ResponseTemplate}; + let sri = crate::vendor::npm_pack::PackedTarball::from_bytes(body).integrity; + let serve_path = "/patch/maven/commons-text/1.10.0/tok/uuid/commons-text-1.10.0.jar"; + let server = MockServer::start().await; + let serve_url = format!("{}{serve_path}", server.uri()); + Mock::given(method("POST")) + .and(path("/v0/orgs/acme/patches/package")) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "results": { UUID: { + "status": "granted", + "url": serve_url, + "artifacts": [{ "kind": "tarball", "url": serve_url, + "integrity": { "sha512": sri } }] + }} + }))) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path(serve_path)) + .respond_with(ResponseTemplate::new(200).set_body_bytes(body.to_vec())) + .mount(&server) + .await; + server + } + + /// The wired-but-missing rebuild must hand back a refreshed + /// ledger entry. Vendored from the service, the committed jar is lost and + /// rebuilt LOCALLY (different bytes); the ledger the CLI persists (the + /// refreshed entry carried forward over the first, or the first when the + /// backend returns None) must describe the rebuilt jar. + #[tokio::test] + async fn wired_rebuild_refreshes_ledger_sha256() { + // A service build: same members, STORED (uncompressed), so its bytes + // differ from the local deflate re-zip as a real service jar's would. + let served = { + let mut zw = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); + let opts = zip::write::SimpleFileOptions::default() + .compression_method(zip::CompressionMethod::Stored); + for (name, bytes) in [ + ("META-INF/MANIFEST.MF", &b"Manifest-Version: 1.0\n"[..]), + (JAR_FILE, PATCHED), + ( + "org/apache/commons/text/StringSubstitutor.class", + &b"\xca\xfe\xba\xbe-fake-class"[..], + ), + ] { + zw.start_file(name, opts).unwrap(); + zw.write_all(bytes).unwrap(); + } + zw.finish().unwrap().into_inner() + }; + let server = mount_granted_jar(&served).await; + let (dir, blobs, installed, record) = fixture(Some(project_pom()), true, true).await; + let root = dir.path(); + let cfg = service_cfg( + Some(&server.uri()), + crate::vendor::VendorSource::Service, + false, + ); + let (r1, e1, _w1) = + unwrap_done(run_vendor_with_service(root, &blobs, &installed, &record, &cfg).await); + assert!(r1.success, "{:?}", r1.error); + let e1 = e1.expect("first vendor records an entry"); + assert_eq!( + crate::vendor::check_vendored_artifact(root, &e1, &record).await, + crate::vendor::ArtifactHealth::Healthy + ); + + tokio::fs::remove_file(root.join(jar_rel())).await.unwrap(); + // Local rebuild (no service). + let (r2, e2, w2) = unwrap_done(run_vendor(root, &blobs, &installed, &record, false).await); + assert!(r2.success, "{:?}", r2.error); + assert!( + w2.iter().any(|w| w.code == "vendor_artifact_rebuilt"), + "{w2:?}" + ); + let rebuilt = tokio::fs::read(root.join(jar_rel())).await.unwrap(); + assert!( + rebuilt != served, + "precondition: the two sources differ byte-wise" + ); + + let ledger = match e2 { + Some(mut fresh) => { + crate::vendor::carry_forward_wiring(&e1, &mut fresh); + fresh + } + None => e1.clone(), + }; + assert_eq!( + ledger.artifact.sha256, + hex::encode(Sha256::digest(&rebuilt)), + "the ledger fingerprint must describe the rebuilt jar" + ); + assert_eq!( + crate::vendor::check_vendored_artifact(root, &ledger, &record).await, + crate::vendor::ArtifactHealth::Healthy + ); + // The carried-forward pom.xml wiring still reverts cleanly. + assert_eq!(ledger.wiring, e1.wiring, "pom.xml revert record preserved"); + let rv = revert_maven(&ledger, root, false).await; + assert!(rv.success, "{:?}", rv.error); + assert_eq!( + tokio::fs::read_to_string(root.join(PROJECT_POM)) + .await + .unwrap(), + project_pom() + ); + } } diff --git a/crates/socket-patch-core/src/vendor/nuget_feed.rs b/crates/socket-patch-core/src/vendor/nuget_feed.rs index b1a2a5c3..82418839 100644 --- a/crates/socket-patch-core/src/vendor/nuget_feed.rs +++ b/crates/socket-patch-core/src/vendor/nuget_feed.rs @@ -315,6 +315,12 @@ pub async fn vendor_nuget( // first run) is still wired at this feed, so deleting the uuid dir // would leave a wired config pointing at nothing and brick every // restore. + // The refreshed entry's only wiring: the re-pinned lock record, + // `original: None` like any re-vendor of our own pin — the + // caller's `carry_forward_wiring` (same uuid) fills the true + // pre-vendor contentHash from the entry being replaced and + // re-attaches the untouched config records. + let mut wiring: Vec = Vec::new(); if let Some(text) = &lock_text { let new_hash = content_hash(&bytes); match edit_lock(text, name, &version_norm, &new_hash) { @@ -327,6 +333,14 @@ pub async fn vendor_nuget( result.error = Some(format!("failed to rewrite {PACKAGES_LOCK}: {e}")); return done(result, None, warnings); } + wiring.push(WiringRecord { + file: PACKAGES_LOCK.to_string(), + kind: LOCK_WIRING_KIND.to_string(), + action: WiringAction::Rewritten, + key: Some(name.to_string()), + original: None, + new: Some(Value::String(new_hash)), + }); } Ok(None) => {} Err(detail) => { @@ -343,7 +357,17 @@ pub async fn vendor_nuget( rebuilt at {copy_rel} (nuget.config untouched)" ), )); - return done(result, None, warnings); + // The rebuilt nupkg may differ byte-wise from the one the ledger + // fingerprinted (a service ↔ local flip): hand back a refreshed + // entry so the ledger sha256 and lock pin describe these bytes. + let entry = nuget_entry( + build_nuget_purl(name, version), + record, + copy_rel, + &bytes, + wiring, + ); + return done(result, Some(entry), warnings); } // Dry runs fall through to the verify-only preview below. } @@ -515,7 +539,22 @@ pub async fn vendor_nuget( wiring.push(rec); } - let entry = VendorEntry { + let entry = nuget_entry(base_purl, record, copy_rel, &nupkg_bytes, wiring); + + done(result, Some(entry), warnings) +} + +/// The ledger entry for a vendored nupkg: `wiring` is the config + lock +/// records on a full vendor, only the re-pinned lock record on an +/// artifact-only rebuild (see the hot path). +fn nuget_entry( + base_purl: String, + record: &PatchRecord, + copy_rel: String, + nupkg_bytes: &[u8], + wiring: Vec, +) -> VendorEntry { + VendorEntry { ecosystem: "nuget".to_string(), base_purl, uuid: record.uuid.clone(), @@ -524,7 +563,7 @@ pub async fn vendor_nuget( // for tooling (harvest re-derives per-entry git hashes from the // zip, so the vendored copy is self-describing without a network). path: copy_rel, - sha256: hex::encode(sha2::Sha256::digest(&nupkg_bytes)), + sha256: hex::encode(sha2::Sha256::digest(nupkg_bytes)), size: Some(nupkg_bytes.len() as u64), platform_locked: None, file_inventory: None, @@ -540,9 +579,7 @@ pub async fn vendor_nuget( poetry: None, pdm: None, pipenv: None, - }; - - done(result, Some(entry), warnings) + } } /// Revert a NuGet vendor entry: undo the lock pin, restore/delete the @@ -2533,7 +2570,14 @@ mod tests { let (r2, e2, w2) = unwrap_done(run_vendor(root, &blobs, &installed, &record, false).await); assert!(r2.success, "{:?}", r2.error); - assert!(e2.is_none()); + // A refreshed fingerprint whose only wiring is the re-pinned lock: + // the config records stay with the first run's entry. + let e2 = e2.expect("the rebuild refreshes the ledger fingerprint"); + assert!( + e2.wiring.iter().all(|w| w.kind == LOCK_WIRING_KIND), + "{:?}", + e2.wiring + ); assert!(w2.iter().any(|w| w.code == "vendor_artifact_rebuilt")); assert_eq!( r2.package_path, @@ -3064,7 +3108,10 @@ mod tests { .await; let (r2, e2, w2) = unwrap_done(outcome); assert!(r2.success, "{:?}", r2.error); - assert!(e2.is_none(), "artifact-only rebuild must not re-record"); + assert!( + e2.is_some_and(|e| e.wiring.iter().all(|w| w.kind == LOCK_WIRING_KIND)), + "artifact-only rebuild never re-records the config wiring" + ); assert!( w2.iter().any(|w| w.code == "vendor_artifact_rebuilt"), "FIFO nupkg must read as stale and trigger the rebuild: {w2:?}" @@ -3282,7 +3329,10 @@ mod tests { let (r2, e2, w2) = unwrap_done(run_vendor(root, &blobs, &installed, &record, false).await); assert!(r2.success, "{:?}", r2.error); - assert!(e2.is_none(), "artifact-only rebuild must not re-record"); + assert!( + e2.is_some_and(|e| e.wiring.is_empty()), + "no lock to re-pin: a refreshed fingerprint with no wiring" + ); assert!( w2.iter().any(|w| w.code == "vendor_artifact_rebuilt"), "{w2:?}" @@ -3383,7 +3433,10 @@ mod tests { let (r2, e2, w2) = unwrap_done(run_vendor(root, &blobs, &installed, &record, false).await); assert!(r2.success, "{:?}", r2.error); - assert!(e2.is_none()); + assert!( + e2.is_some_and(|e| e.wiring.is_empty()), + "nothing re-pinned: a refreshed fingerprint with no wiring" + ); assert!( w2.iter().any(|w| w.code == "vendor_artifact_rebuilt"), "{w2:?}" @@ -4664,4 +4717,152 @@ mod tests { "the drifted live config is untouched after the failed write" ); } + + fn integrity_cfg( + server: Option<&wiremock::MockServer>, + source: crate::vendor::VendorSource, + ) -> VendorServiceConfig { + use crate::api::client::{ApiClient, ApiClientOptions}; + VendorServiceConfig { + source, + client: server.map(|s| { + ApiClient::new(ApiClientOptions { + api_url: s.uri(), + api_token: Some("sktsec_placeholder_value_for_tests_api".into()), + use_public_proxy: false, + org_slug: Some("acme".into()), + }) + }), + use_public_proxy: false, + vendor_url: None, + patch_server_url: None, + offline: false, + } + } + + /// Mount a granted service response serving `served` as the prebuilt nupkg. + async fn mount_granted_nupkg(served: &[u8]) -> wiremock::MockServer { + use wiremock::matchers::{method, path}; + use wiremock::{Mock, MockServer, ResponseTemplate}; + let server = MockServer::start().await; + let sri = crate::vendor::npm_pack::PackedTarball::from_bytes(served).integrity; + let serve_path = + "/patch/nuget/newtonsoft.json/13.0.3/tok/uuid/newtonsoft.json.13.0.3.nupkg"; + let serve_url = format!("{}{serve_path}", server.uri()); + Mock::given(method("POST")) + .and(path("/v0/orgs/acme/patches/package")) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "results": { UUID: { + "status": "granted", "url": serve_url, + "artifacts": [{ "kind": "tarball", "url": serve_url, + "integrity": { "sha512": sri } }] + }} + }))) + .mount(&server) + .await; + Mock::given(method("GET")) + .and(path(serve_path)) + .respond_with(ResponseTemplate::new(200).set_body_bytes(served.to_vec())) + .mount(&server) + .await; + server + } + + async fn run_vendor_cfg( + root: &Path, + blobs: &Path, + installed: &Path, + record: &PatchRecord, + cfg: Option<&VendorServiceConfig>, + ) -> VendorOutcome { + let sources = PatchSources::blobs_only(blobs); + vendor_nuget( + PURL, + installed, + root, + record, + &sources, + "2026-06-09T00:00:00Z", + false, + false, + cfg, + ) + .await + } + + /// Vendored from the service, the committed nupkg is lost and + /// rebuilt LOCALLY (signature dropped → different bytes, lock re-pinned). + /// The ledger the CLI persists must describe the rebuilt nupkg AND the + /// re-pinned lock, so `--revert` restores the pristine contentHash. + #[tokio::test] + async fn wired_rebuild_refreshes_ledger_and_revert_restores_lock() { + let (dir, blobs, installed, record) = fixture(true, None).await; + let root = dir.path(); + let served = make_nupkg(PATCHED); + let server = mount_granted_nupkg(&served).await; + let cfg = integrity_cfg(Some(&server), crate::vendor::VendorSource::Service); + let (r1, e1, _) = + unwrap_done(run_vendor_cfg(root, &blobs, &installed, &record, Some(&cfg)).await); + assert!(r1.success, "{:?}", r1.error); + let e1 = e1.expect("first vendor records an entry"); + + tokio::fs::remove_file(root.join(copy_rel())).await.unwrap(); + let (r2, e2, w2) = + unwrap_done(run_vendor_cfg(root, &blobs, &installed, &record, None).await); + assert!(r2.success, "{:?}", r2.error); + assert!( + w2.iter().any(|w| w.code == "vendor_artifact_rebuilt"), + "{w2:?}" + ); + let rebuilt = tokio::fs::read(root.join(copy_rel())).await.unwrap(); + assert!( + rebuilt != served, + "precondition: the two sources differ byte-wise" + ); + let lock = tokio::fs::read_to_string(root.join(PACKAGES_LOCK)) + .await + .unwrap(); + assert!( + lock.contains(&content_hash(&rebuilt)), + "lock re-pinned at the rebuild" + ); + + let ledger = match e2 { + Some(mut fresh) => { + crate::vendor::carry_forward_wiring(&e1, &mut fresh); + fresh + } + None => e1.clone(), + }; + assert_eq!( + ledger.artifact.sha256, + hex::encode(sha2::Sha256::digest(&rebuilt)), + "the ledger fingerprint must describe the rebuilt nupkg" + ); + assert_eq!( + crate::vendor::check_vendored_artifact(root, &ledger, &record).await, + crate::vendor::ArtifactHealth::Healthy + ); + let rv = revert_nuget(&ledger, root, false).await; + assert!(rv.success, "{:?}", rv.error); + assert!( + !rv.warnings + .iter() + .any(|w| w.code == "vendor_lock_entry_drifted"), + "{:?}", + rv.warnings + ); + let lock = tokio::fs::read_to_string(root.join(PACKAGES_LOCK)) + .await + .unwrap(); + assert_eq!( + lock, + lock_json("ORIGINALcachedhash=="), + "revert restores the pristine contentHash" + ); + assert!( + !root.join("nuget.config").exists(), + "created config removed" + ); + } } From 32b73d5fb4c0cf01cca11d5813082344f8b1b9c0 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 23 Sep 2026 17:37:52 -0400 Subject: [PATCH 2/7] fix(vendor): verify a served maven/nuget archive carries the patch before accepting it service_archive_copy (the Tier-A maven .jar / nuget .nupkg path) accepted any archive that matched its SRI. The bytes were written verbatim and every patched file was reported AlreadyPatched, so a served archive without the patch was committed as patched. The hot path then saw a stale artifact on every later run and rebuilt it. The helper now takes the PatchRecord and requires every patched member to hash to its afterHash (zip_bytes_match_after_hashes, the check the hot path already uses) before returning Used. A mismatch is a service miss: auto falls back to the local build with vendor_prebuilt_layout_mismatch, and service refuses with vendor_prebuilt_required. The vendor_prebuilt_downloaded advisory is only pushed for accepted bytes. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 9 + crates/socket-patch-cli/CLI_CONTRACT.md | 1 + .../src/vendor/maven_repo.rs | 66 +++++++- .../src/vendor/nuget_feed.rs | 19 ++- .../src/vendor/service_fetch.rs | 158 ++++++++++++++++-- 5 files changed, 237 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 91d4025a..4e89f4be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -654,6 +654,15 @@ into the new version's section — see docs/releasing.md. so revert still restores the pre-vendor files. If `state.json` has no entry for the package, the rebuild still runs but no entry is added, because the run has no pre-vendor originals to record. +- **A prebuilt maven `.jar` or nuget `.nupkg` from the patch service must now + contain the patched files.** Checking its integrity hash only showed that + the download was intact, not that the archive carried the patch. The + archive was still written as-is and every file was reported as already + patched, so an unpatched archive could be committed and then rebuilt on + every run. Each patched file inside the archive is now checked against the + patch's expected hash before the archive is used. On a mismatch, `auto` + builds the archive locally and warns `vendor_prebuilt_layout_mismatch`, + and `--vendor-source=service` refuses with `vendor_prebuilt_required`. ### Changed diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index c8779a1e..dc64775a 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -473,6 +473,7 @@ per service outcome: |---|---|---| | granted/reused, integrity ok | **use service** | **use service** | | integrity mismatch | cargo/maven/nuget: **refuse** (`vendor_prebuilt_integrity_mismatch`) — tampered bytes never fall back; other ecosystems (to be aligned): local build + `vendor_prebuilt_integrity_mismatch` | refuse (cargo/maven/nuget: `vendor_prebuilt_integrity_mismatch`; others: `vendor_prebuilt_required`) | +| integrity ok, but the archive does not carry the patched files (a member at a recorded path fails its `afterHash`; checked for cargo/golang/composer/gem after extraction, and for maven/nuget before the archive is written) | local build + `vendor_prebuilt_layout_mismatch` | refuse (`vendor_prebuilt_required`) | | still building (`pending_build` / serve 408) | local build + `vendor_prebuilt_pending` | refuse | | not built / withdrawn / not found / no usable artifact | local build (quiet) | refuse | | gem stub gemspec missing / invalid | local build + `vendor_prebuilt_stub_missing` / `vendor_prebuilt_stub_invalid` (invalid + gem not installed: refuse `vendor_prebuilt_stub_invalid` — no stub source exists) | refuse (`vendor_prebuilt_required` / `vendor_prebuilt_stub_invalid`) | diff --git a/crates/socket-patch-core/src/vendor/maven_repo.rs b/crates/socket-patch-core/src/vendor/maven_repo.rs index 141cf90e..9b3a2d34 100644 --- a/crates/socket-patch-core/src/vendor/maven_repo.rs +++ b/crates/socket-patch-core/src/vendor/maven_repo.rs @@ -600,7 +600,7 @@ async fn materialise_and_write( // The patched jar first (service Tier A, else local rebuild). A non-fatal // failure returns an un-successful ApplyResult with nothing written. let (jar_bytes, result) = - match service_archive_copy(service, &record.uuid, artifact_id, ".jar", warnings).await { + match service_archive_copy(service, record, artifact_id, ".jar", warnings).await { ServiceCopy::Used(bytes) => { (bytes, already_patched_result(purl, jar_path, &record.files)) } @@ -3745,4 +3745,68 @@ mod tests { project_pom() ); } + + /// A served jar that passes the SRI floor but whose patched + /// member does NOT carry the record's afterHash must never be accepted. + /// Under `service` it is a refusal with nothing written. + #[tokio::test] + async fn service_jar_failing_after_hashes_refused_under_service() { + let server = mount_granted_jar(&make_jar(PRISTINE)).await; + let (dir, blobs, installed, record) = fixture(Some(project_pom()), true, true).await; + let root = dir.path(); + let cfg = service_cfg( + Some(&server.uri()), + crate::vendor::VendorSource::Service, + false, + ); + let outcome = run_vendor_with_service(root, &blobs, &installed, &record, &cfg).await; + let VendorOutcome::Refused { code, .. } = outcome else { + panic!("an unpatched service jar was accepted: {outcome:?}"); + }; + assert_eq!(code, "vendor_prebuilt_required"); + assert!(!root.join(".socket").exists(), "nothing written"); + assert_eq!( + tokio::fs::read_to_string(root.join(PROJECT_POM)) + .await + .unwrap(), + project_pom() + ); + } + + /// Under `auto`: the bad served jar falls back (loudly) to the + /// local rebuild, so the committed jar really carries the patch and a + /// re-run is in sync (no perpetual rebuild). + #[tokio::test] + async fn service_jar_failing_after_hashes_falls_back_under_auto() { + let server = mount_granted_jar(&make_jar(PRISTINE)).await; + let (dir, blobs, installed, record) = fixture(Some(project_pom()), true, true).await; + let root = dir.path(); + let cfg = service_cfg( + Some(&server.uri()), + crate::vendor::VendorSource::Auto, + false, + ); + let (r1, e1, w1) = + unwrap_done(run_vendor_with_service(root, &blobs, &installed, &record, &cfg).await); + assert!(r1.success, "{:?}", r1.error); + assert!( + w1.iter() + .any(|w| w.code == "vendor_prebuilt_layout_mismatch"), + "the rejected service jar is surfaced: {w1:?}" + ); + let e1 = e1.expect("entry"); + assert_eq!( + crate::vendor::check_vendored_artifact(root, &e1, &record).await, + crate::vendor::ArtifactHealth::Healthy, + "the committed jar must carry the afterHash" + ); + let (r2, e2, w2) = + unwrap_done(run_vendor_with_service(root, &blobs, &installed, &record, &cfg).await); + assert!(r2.success); + assert!(e2.is_none(), "re-run is in sync"); + assert!( + !w2.iter().any(|w| w.code == "vendor_artifact_rebuilt"), + "{w2:?}" + ); + } } diff --git a/crates/socket-patch-core/src/vendor/nuget_feed.rs b/crates/socket-patch-core/src/vendor/nuget_feed.rs index 82418839..a8e6acaf 100644 --- a/crates/socket-patch-core/src/vendor/nuget_feed.rs +++ b/crates/socket-patch-core/src/vendor/nuget_feed.rs @@ -740,7 +740,7 @@ async fn materialise_patched_nupkg( config_wired: bool, warnings: &mut Vec, ) -> Result<(Vec, ApplyResult), Box> { - match service_archive_copy(service, &record.uuid, name, ".nupkg", warnings).await { + match service_archive_copy(service, record, name, ".nupkg", warnings).await { ServiceCopy::Used(bytes) => { if let Err(e) = write_nupkg(uuid_dir, nupkg_path, &bytes).await { if !config_wired { @@ -4865,4 +4865,21 @@ mod tests { "created config removed" ); } + + /// A served nupkg that passes the SRI floor but whose patched + /// member does NOT carry the afterHash is refused under `service`. + #[tokio::test] + async fn service_nupkg_failing_after_hashes_refused_under_service() { + let (dir, blobs, installed, record) = fixture(true, None).await; + let root = dir.path(); + let server = mount_granted_nupkg(&make_nupkg(PRISTINE)).await; + let cfg = integrity_cfg(Some(&server), crate::vendor::VendorSource::Service); + let outcome = run_vendor_cfg(root, &blobs, &installed, &record, Some(&cfg)).await; + let VendorOutcome::Refused { code, .. } = outcome else { + panic!("an unpatched service nupkg was accepted: {outcome:?}"); + }; + assert_eq!(code, "vendor_prebuilt_required"); + assert!(!root.join(".socket").exists(), "nothing written"); + assert!(!root.join("nuget.config").exists()); + } } diff --git a/crates/socket-patch-core/src/vendor/service_fetch.rs b/crates/socket-patch-core/src/vendor/service_fetch.rs index 9d80c160..f19cd799 100644 --- a/crates/socket-patch-core/src/vendor/service_fetch.rs +++ b/crates/socket-patch-core/src/vendor/service_fetch.rs @@ -9,11 +9,12 @@ //! extract it into the vendor directory) and the build-vs-service policy. use crate::api::client::{SecondaryArtifact, VendorServiceOutcome}; +use crate::manifest::schema::PatchRecord; use crate::vendor::lock_inventory::LockIntegrity; use crate::vendor::registry_fetch::{artifact_matches_integrity, verify_go_h1}; use crate::vendor::VendorServiceConfig; use crate::vendor::{ - common::{refused, service_offline_conflict}, + common::{refused, service_offline_conflict, zip_bytes_match_after_hashes}, VendorOutcome, VendorWarning, }; @@ -129,7 +130,7 @@ pub(crate) enum ServiceCopy { /// policy. `noun` is the artifact kind used in messages (".jar" / ".nupkg"). pub(crate) async fn service_archive_copy( service: Option<&VendorServiceConfig>, - uuid: &str, + record: &PatchRecord, name: &str, noun: &str, warnings: &mut Vec, @@ -160,7 +161,24 @@ pub(crate) async fn service_archive_copy( ServiceCopy::FallBack } }; - match fetch_verified_archive(cfg, uuid).await { + match fetch_verified_archive(cfg, &record.uuid).await { + // The SRI proves the download is intact, not that it carries the + // patch: the bytes are written verbatim and reported AlreadyPatched, + // so every patched member must hash to its afterHash first (the + // Tier-B backends' extracted-tree check). Fail closed → `auto` + // falls back to the local rebuild. + ServiceArtifact::Ready(archive) + if !zip_bytes_match_after_hashes(&archive.bytes, &record.files) => + { + miss( + warnings, + "vendor_prebuilt_layout_mismatch", + format!( + "prebuilt {noun} for {name} does not carry the patched files at their \ + recorded paths" + ), + ) + } ServiceArtifact::Ready(archive) => { warnings.push(VendorWarning::new( "vendor_prebuilt_downloaded", @@ -266,6 +284,29 @@ mod tests { const UUID: &str = "22222222-2222-2222-2222-222222222222"; const SERVE_PATH: &str = "/patch/npm/x/1.0.0/tok/uuid/x-1.0.0.tgz"; + /// A files-less record for [`UUID`]: the Tier-A afterHash gate then only + /// requires the served bytes to be a readable zip. + fn record() -> PatchRecord { + PatchRecord { + uuid: UUID.to_string(), + exported_at: String::new(), + files: std::collections::HashMap::new(), + vulnerabilities: std::collections::HashMap::new(), + description: String::new(), + license: String::new(), + tier: String::new(), + } + } + + /// An empty (member-less) zip — passes the afterHash gate of a + /// files-less [`record`]. + fn empty_zip() -> Vec { + zip::ZipWriter::new(std::io::Cursor::new(Vec::new())) + .finish() + .unwrap() + .into_inner() + } + fn cfg_for(server: &MockServer) -> VendorServiceConfig { VendorServiceConfig { source: VendorSource::Service, @@ -373,7 +414,7 @@ mod tests { let mut cfg = cfg_for(&server); cfg.source = VendorSource::Auto; let mut warnings = Vec::new(); - match service_archive_copy(Some(&cfg), UUID, "x", ".jar", &mut warnings).await { + match service_archive_copy(Some(&cfg), &record(), "x", ".jar", &mut warnings).await { ServiceCopy::HardFail(outcome) => match *outcome { VendorOutcome::Refused { code, .. } => { assert_eq!(code, "vendor_prebuilt_integrity_mismatch"); @@ -397,7 +438,7 @@ mod tests { let mut cfg = cfg_for(&server); cfg.offline = true; let mut warnings = Vec::new(); - match service_archive_copy(Some(&cfg), UUID, "x", ".jar", &mut warnings).await { + match service_archive_copy(Some(&cfg), &record(), "x", ".jar", &mut warnings).await { ServiceCopy::HardFail(outcome) => match *outcome { VendorOutcome::Refused { code, .. } => { assert_eq!(code, "vendor_service_offline_conflict"); @@ -420,7 +461,7 @@ mod tests { cfg.offline = true; let mut warnings = Vec::new(); assert!(matches!( - service_archive_copy(Some(&cfg), UUID, "x", ".jar", &mut warnings).await, + service_archive_copy(Some(&cfg), &record(), "x", ".jar", &mut warnings).await, ServiceCopy::FallBack )); assert!(warnings.is_empty(), "quiet fallback, no warning"); @@ -486,11 +527,18 @@ mod tests { #[tokio::test] async fn service_copy_ready_returns_used_bytes_with_downloaded_note() { let server = MockServer::start().await; - let body = b"prebuilt jar bytes"; + let body = &empty_zip()[..]; let sri = PackedTarball::from_bytes(body).integrity; mount_granted(&server, &sri, body).await; let mut warnings = Vec::new(); - match service_archive_copy(Some(&cfg_for(&server)), UUID, "x", ".jar", &mut warnings).await + match service_archive_copy( + Some(&cfg_for(&server)), + &record(), + "x", + ".jar", + &mut warnings, + ) + .await { ServiceCopy::Used(bytes) => assert_eq!(bytes, body), ServiceCopy::HardFail(outcome) => panic!("expected Used, got HardFail({outcome:?})"), @@ -522,7 +570,7 @@ mod tests { cfg.source = VendorSource::Auto; let mut warnings = Vec::new(); assert!(matches!( - service_archive_copy(Some(&cfg), UUID, "x", ".jar", &mut warnings).await, + service_archive_copy(Some(&cfg), &record(), "x", ".jar", &mut warnings).await, ServiceCopy::FallBack )); assert_eq!(warnings.len(), 1); @@ -540,7 +588,14 @@ mod tests { let server = MockServer::start().await; mount_status(&server, "pending_build").await; let mut warnings = Vec::new(); - match service_archive_copy(Some(&cfg_for(&server)), UUID, "x", ".jar", &mut warnings).await + match service_archive_copy( + Some(&cfg_for(&server)), + &record(), + "x", + ".jar", + &mut warnings, + ) + .await { ServiceCopy::HardFail(outcome) => match *outcome { VendorOutcome::Refused { code, detail } => { @@ -564,7 +619,14 @@ mod tests { let server = MockServer::start().await; mount_status(&server, "not_found").await; let mut warnings = Vec::new(); - match service_archive_copy(Some(&cfg_for(&server)), UUID, "x", ".jar", &mut warnings).await + match service_archive_copy( + Some(&cfg_for(&server)), + &record(), + "x", + ".jar", + &mut warnings, + ) + .await { ServiceCopy::HardFail(outcome) => match *outcome { VendorOutcome::Refused { code, detail } => { @@ -592,7 +654,7 @@ mod tests { cfg.source = VendorSource::Auto; let mut warnings = Vec::new(); assert!(matches!( - service_archive_copy(Some(&cfg), UUID, "x", ".jar", &mut warnings).await, + service_archive_copy(Some(&cfg), &record(), "x", ".jar", &mut warnings).await, ServiceCopy::FallBack )); assert!( @@ -616,7 +678,7 @@ mod tests { cfg.source = VendorSource::Auto; let mut warnings = Vec::new(); assert!(matches!( - service_archive_copy(Some(&cfg), UUID, "x", ".jar", &mut warnings).await, + service_archive_copy(Some(&cfg), &record(), "x", ".jar", &mut warnings).await, ServiceCopy::FallBack )); assert_eq!(warnings.len(), 1); @@ -646,7 +708,14 @@ mod tests { .mount(&server) .await; let mut warnings = Vec::new(); - match service_archive_copy(Some(&cfg_for(&server)), UUID, "x", ".jar", &mut warnings).await + match service_archive_copy( + Some(&cfg_for(&server)), + &record(), + "x", + ".jar", + &mut warnings, + ) + .await { ServiceCopy::HardFail(outcome) => match *outcome { VendorOutcome::Refused { code, detail } => { @@ -699,4 +768,65 @@ mod tests { } } } + + /// A served archive that passes its SRI but does not carry the + /// record's patched bytes is never `Used`: `service` refuses, `auto` + /// falls back loudly — and neither pushes the `vendor_prebuilt_downloaded` + /// advisory for bytes it rejected. + #[tokio::test] + async fn service_copy_ready_failing_after_hashes_is_rejected() { + use crate::hash::git_sha256::compute_git_sha256_from_bytes; + use crate::manifest::schema::PatchFileInfo; + let body = { + use std::io::Write as _; + let mut zw = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); + zw.start_file("lib/x.txt", zip::write::SimpleFileOptions::default()) + .unwrap(); + zw.write_all(b"unpatched").unwrap(); + zw.finish().unwrap().into_inner() + }; + let mut rec = record(); + rec.files.insert( + "lib/x.txt".to_string(), + PatchFileInfo { + before_hash: compute_git_sha256_from_bytes(b"unpatched"), + after_hash: compute_git_sha256_from_bytes(b"patched"), + }, + ); + for source in [VendorSource::Service, VendorSource::Auto] { + let server = MockServer::start().await; + let sri = PackedTarball::from_bytes(&body).integrity; + mount_granted(&server, &sri, &body).await; + let mut cfg = cfg_for(&server); + cfg.source = source; + let mut warnings = Vec::new(); + let copy = service_archive_copy(Some(&cfg), &rec, "x", ".jar", &mut warnings).await; + match (source, copy) { + (VendorSource::Service, ServiceCopy::HardFail(outcome)) => match *outcome { + VendorOutcome::Refused { code, detail } => { + assert_eq!(code, "vendor_prebuilt_required"); + assert!( + detail.contains("does not carry the patched files"), + "{detail}" + ); + } + other => panic!("expected Refused, got {other:?}"), + }, + (VendorSource::Auto, ServiceCopy::FallBack) => { + assert_eq!(warnings.len(), 1, "{warnings:?}"); + assert_eq!(warnings[0].code, "vendor_prebuilt_layout_mismatch"); + } + (source, ServiceCopy::Used(_)) => { + panic!("{source:?}: unpatched service bytes were accepted") + } + (source, _) => panic!("{source:?}: unexpected outcome"), + } + assert!( + !warnings + .iter() + .any(|w| w.code == "vendor_prebuilt_downloaded"), + "{warnings:?}" + ); + } + } } From 8c37f5ab841cd5a8461e902176b9e257d6d3d92a Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 23 Sep 2026 17:42:30 -0400 Subject: [PATCH 3/7] fix(vendor): refuse a prebuilt artifact that fails integrity under auto too ServiceArtifact documents IntegrityMismatch as always a hard error, but golang, composer, pypi, npm and gem (the .gem and the stub gemspec) mapped it through their miss policy. Under auto they warned and built the package locally, so a sign of tampering became a quiet fallback. Under service they refused with the generic vendor_prebuilt_required code. These arms now refuse in every mode, as cargo already did: golang, composer, pypi and gem return vendor_prebuilt_integrity_mismatch, and npm fails the package with the integrity detail. The npm test that pinned the old fallback is replaced by one that expects the refusal. The service-mode tests for these ecosystems now expect the specific code. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 11 +++ crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../src/vendor/composer_lock.rs | 41 +++++++++-- crates/socket-patch-core/src/vendor/gem.rs | 60 ++++++++++++---- crates/socket-patch-core/src/vendor/golang.rs | 48 +++++++++++-- .../src/vendor/npm_common.rs | 24 ++----- .../socket-patch-core/src/vendor/npm_lock.rs | 68 +++++++++---------- crates/socket-patch-core/src/vendor/pypi.rs | 40 ++++++++++- 8 files changed, 216 insertions(+), 78 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e89f4be..19ae5b15 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -663,6 +663,17 @@ into the new version's section — see docs/releasing.md. patch's expected hash before the archive is used. On a mismatch, `auto` builds the archive locally and warns `vendor_prebuilt_layout_mismatch`, and `--vendor-source=service` refuses with `vendor_prebuilt_required`. +- **A prebuilt artifact that fails its integrity check is always refused.** + Under the default `--vendor-source=auto`, npm, pypi, golang, composer and + gem (both the `.gem` and its stub gemspec) printed a warning and built the + package locally when the downloaded bytes did not match the integrity the + patch service reported. Bytes that fail verification may have been + tampered with, so these ecosystems now refuse the package in every mode, + as cargo, maven and nuget already did. The refusal code is + `vendor_prebuilt_integrity_mismatch` (npm fails the package with the + integrity detail). Under `--vendor-source=service`, golang, composer, gem + and pypi now report `vendor_prebuilt_integrity_mismatch` instead of + `vendor_prebuilt_required`. ### Changed diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index dc64775a..1429cb8e 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -472,7 +472,7 @@ per service outcome: | Service outcome | `auto` | `service` | |---|---|---| | granted/reused, integrity ok | **use service** | **use service** | -| integrity mismatch | cargo/maven/nuget: **refuse** (`vendor_prebuilt_integrity_mismatch`) — tampered bytes never fall back; other ecosystems (to be aligned): local build + `vendor_prebuilt_integrity_mismatch` | refuse (cargo/maven/nuget: `vendor_prebuilt_integrity_mismatch`; others: `vendor_prebuilt_required`) | +| integrity mismatch (including the gem stub gemspec) | **refuse** (`vendor_prebuilt_integrity_mismatch`; npm: the package fails with the integrity detail). Tampered bytes never fall back to a local build | refuse (same) | | integrity ok, but the archive does not carry the patched files (a member at a recorded path fails its `afterHash`; checked for cargo/golang/composer/gem after extraction, and for maven/nuget before the archive is written) | local build + `vendor_prebuilt_layout_mismatch` | refuse (`vendor_prebuilt_required`) | | still building (`pending_build` / serve 408) | local build + `vendor_prebuilt_pending` | refuse | | not built / withdrawn / not found / no usable artifact | local build (quiet) | refuse | diff --git a/crates/socket-patch-core/src/vendor/composer_lock.rs b/crates/socket-patch-core/src/vendor/composer_lock.rs index e86fdcc4..8d9c7034 100644 --- a/crates/socket-patch-core/src/vendor/composer_lock.rs +++ b/crates/socket-patch-core/src/vendor/composer_lock.rs @@ -702,10 +702,15 @@ async fn composer_service_copy( )); ComposerServiceCopy::Used } - ServiceArtifact::IntegrityMismatch(reason) => miss( - warnings, + // Bytes that fail integrity verification are an active tamper signal: + // ALWAYS a hard error, in `auto` exactly as in `service` — never a + // quiet local-build fallback (`ServiceArtifact`'s documented contract). + ServiceArtifact::IntegrityMismatch(reason) => hard( "vendor_prebuilt_integrity_mismatch", - format!("prebuilt dist zip failed integrity ({reason})"), + format!( + "prebuilt dist zip for {pkg} failed integrity verification ({reason}); \ + refusing to fall back to a local build on tampered bytes" + ), ), ServiceArtifact::Pending => miss( warnings, @@ -1999,7 +2004,7 @@ mod tests { ) .await, ); - assert_eq!(code, "vendor_prebuilt_required"); + assert_eq!(code, "vendor_prebuilt_integrity_mismatch"); assert!(!root .join(format!(".socket/vendor/composer/{UUID}")) .exists()); @@ -3480,4 +3485,32 @@ mod tests { "lock untouched" ); } + + /// An integrity mismatch is a hard failure under `auto` too — + /// never a quiet local-build fallback (service_fetch's contract). + #[tokio::test] + async fn service_integrity_mismatch_auto_hard_fails() { + let lock = lock_value("psr/log", "3.0.2", false); + let (dir, blobs, installed, record) = fixture(&lock).await; + let root = dir.path(); + let zip = make_dist_zip("x", &[("src/LoggerInterface.php", PATCHED)]); + let wrong = sri_sha512(b"different bytes"); + let server = wiremock::MockServer::start().await; + mount_composer_granted(&server, &wrong, &zip).await; + let outcome = vendor_with_service( + root, + &blobs, + &installed, + &record, + &composer_service_cfg(&server.uri(), VendorSource::Auto, false), + ) + .await; + let VendorOutcome::Refused { code, .. } = outcome else { + panic!("tampered bytes fell back to a local build: {outcome:?}"); + }; + assert_eq!(code, "vendor_prebuilt_integrity_mismatch"); + assert!(!root + .join(format!(".socket/vendor/composer/{UUID}")) + .exists()); + } } diff --git a/crates/socket-patch-core/src/vendor/gem.rs b/crates/socket-patch-core/src/vendor/gem.rs index 3e53f98a..d0795817 100644 --- a/crates/socket-patch-core/src/vendor/gem.rs +++ b/crates/socket-patch-core/src/vendor/gem.rs @@ -813,13 +813,16 @@ async fn gem_service_copy( // Step 1: the prebuilt `.gem` (sha512-verified against the reference). let archive = match fetch_verified_archive(cfg, &record.uuid).await { ServiceArtifact::Ready(archive) => archive, + // Bytes that fail integrity verification are an active tamper signal: + // ALWAYS a hard error, in `auto` exactly as in `service` — never a + // quiet local-build fallback (`ServiceArtifact`'s documented contract). ServiceArtifact::IntegrityMismatch(reason) => { - return miss( - warnings, + return hard( "vendor_prebuilt_integrity_mismatch", - ("vendor_prebuilt_required", ""), - format!("prebuilt .gem failed integrity ({reason})"), - false, + format!( + "prebuilt .gem for {name} failed integrity verification ({reason}); \ + refusing to fall back to a local build on tampered bytes" + ), ); } ServiceArtifact::Pending => { @@ -866,12 +869,12 @@ async fn gem_service_copy( ); } SecondaryArtifactResult::IntegrityMismatch(reason) => { - return miss( - warnings, + return hard( "vendor_prebuilt_integrity_mismatch", - ("vendor_prebuilt_required", ""), - format!("prebuilt stub gemspec failed integrity ({reason})"), - false, + format!( + "prebuilt stub gemspec for {name} failed integrity verification ({reason}); \ + refusing to fall back to a local build on tampered bytes" + ), ); } SecondaryArtifactResult::Failed(reason) => { @@ -4855,7 +4858,7 @@ mod tests { ) .await; let (code, _) = unwrap_refused(outcome); - assert_eq!(code, "vendor_prebuilt_required"); + assert_eq!(code, "vendor_prebuilt_integrity_mismatch"); assert!(!root.join(format!(".socket/vendor/gem/{UUID}")).exists()); // The lock is untouched. assert_eq!( @@ -4894,7 +4897,7 @@ mod tests { ) .await; let (code, _) = unwrap_refused(outcome); - assert_eq!(code, "vendor_prebuilt_required"); + assert_eq!(code, "vendor_prebuilt_integrity_mismatch"); assert!(!root.join(format!(".socket/vendor/gem/{UUID}")).exists()); } @@ -8186,4 +8189,37 @@ mod tests { LOCK_DIRECT ); } + + /// A `.gem` or stub that fails integrity verification is a hard + /// failure under `auto` too — never a quiet local-build fallback. + #[tokio::test] + async fn integrity_mismatch_hard_fails_under_auto() { + let gem = make_gem(&[("lib/rack.rb", PATCHED)]); + for bad_stub in [false, true] { + let (_tmp, root, installed, blobs, record) = fixture(GEMFILE_DIRECT, LOCK_DIRECT).await; + let server = wiremock::MockServer::start().await; + let (gem_sri, stub_sri) = if bad_stub { + (sri_sha512(&gem), sri_sha512(b"not the stub")) + } else { + (sri_sha512(b"different bytes"), sri_sha512(SERVICE_STUB)) + }; + mount_gem_granted(&server, &gem, &gem_sri, Some((SERVICE_STUB, &stub_sri))).await; + let cfg = gem_service_cfg(&server.uri(), VendorSource::Auto, false); + let outcome = run_vendor_service(&root, &blobs, &installed, &record, &cfg).await; + let VendorOutcome::Refused { code, .. } = outcome else { + panic!("bad_stub={bad_stub}: tampered bytes fell back: {outcome:?}"); + }; + assert_eq!( + code, "vendor_prebuilt_integrity_mismatch", + "bad_stub={bad_stub}" + ); + assert!(!root.join(format!(".socket/vendor/gem/{UUID}")).exists()); + assert_eq!( + tokio::fs::read_to_string(root.join(GEMFILE_LOCK)) + .await + .unwrap(), + LOCK_DIRECT + ); + } + } } diff --git a/crates/socket-patch-core/src/vendor/golang.rs b/crates/socket-patch-core/src/vendor/golang.rs index 4edcaffd..6fad05f2 100644 --- a/crates/socket-patch-core/src/vendor/golang.rs +++ b/crates/socket-patch-core/src/vendor/golang.rs @@ -586,10 +586,15 @@ async fn go_service_redirect( )); GoServiceRedirect::Used } - ServiceArtifact::IntegrityMismatch(reason) => miss( - warnings, + // Bytes that fail integrity verification are an active tamper signal: + // ALWAYS a hard error, in `auto` exactly as in `service` — never a + // quiet local-build fallback (`ServiceArtifact`'s documented contract). + ServiceArtifact::IntegrityMismatch(reason) => hard( "vendor_prebuilt_integrity_mismatch", - format!("prebuilt module zip failed integrity ({reason})"), + format!( + "prebuilt module zip for {module} failed integrity verification ({reason}); \ + refusing to fall back to a local build on tampered bytes" + ), ), ServiceArtifact::Pending => miss( warnings, @@ -1702,7 +1707,7 @@ mod tests { Some(&go_service_cfg(&server.uri(), VendorSource::Service, false)), ) .await; - expect_refused(outcome, "vendor_prebuilt_required"); + expect_refused(outcome, "vendor_prebuilt_integrity_mismatch"); assert!(!root.join(format!(".socket/vendor/golang/{UUID}")).exists()); } @@ -2595,4 +2600,39 @@ mod tests { "the artifact dir survives a failed wiring restore (retryable)" ); } + + /// An integrity mismatch is a hard failure under `auto` too — + /// never a quiet local-build fallback (service_fetch's contract). + #[tokio::test] + async fn service_integrity_mismatch_auto_hard_fails() { + let (dir, blobs, pristine, record) = fixture().await; + let root = dir.path(); + let gomod_before = tokio::fs::read(root.join("go.mod")).await.unwrap(); + let zip = make_module_zip(&[ + ("go.mod", b"module github.com/foo/bar\n\ngo 1.21\n"), + ("bar.go", PATCHED), + ]); + let wrong = sri_sha512(b"different bytes"); + let server = wiremock::MockServer::start().await; + mount_go_granted(&server, &wrong, None, &zip).await; + let sources = PatchSources::blobs_only(&blobs); + let outcome = vendor_go_module( + PURL, + &pristine, + root, + &record, + &sources, + "2026-06-09T00:00:00Z", + false, + false, + Some(&go_service_cfg(&server.uri(), VendorSource::Auto, false)), + ) + .await; + expect_refused(outcome, "vendor_prebuilt_integrity_mismatch"); + assert!(!root.join(format!(".socket/vendor/golang/{UUID}")).exists()); + assert_eq!( + tokio::fs::read(root.join("go.mod")).await.unwrap(), + gomod_before + ); + } } diff --git a/crates/socket-patch-core/src/vendor/npm_common.rs b/crates/socket-patch-core/src/vendor/npm_common.rs index 195dadad..8abcef6a 100644 --- a/crates/socket-patch-core/src/vendor/npm_common.rs +++ b/crates/socket-patch-core/src/vendor/npm_common.rs @@ -379,23 +379,13 @@ async fn try_service_pack( Err(outcome) => ServicePackDecision::HardFail(outcome), } } - // An artifact that downloaded but failed integrity is NEVER silently - // used; under `auto` we fall back to a fresh local build (loudly). - ServiceArtifact::IntegrityMismatch(reason) => { - if cfg.source.requires_service() { - hard_fail(format!( - "prebuilt artifact failed integrity verification: {reason}" - )) - } else { - warnings.push(VendorWarning::new( - "vendor_prebuilt_integrity_mismatch", - format!( - "prebuilt artifact failed integrity ({reason}); building locally instead" - ), - )); - ServicePackDecision::FallBack - } - } + // Bytes that fail integrity verification are an active tamper signal: + // ALWAYS a hard error, in `auto` exactly as in `service` — never a + // quiet local-build fallback (`ServiceArtifact`'s documented contract). + ServiceArtifact::IntegrityMismatch(reason) => hard_fail(format!( + "prebuilt artifact failed integrity verification ({reason}); \ + refusing to fall back to a local build on tampered bytes" + )), ServiceArtifact::Pending => { if cfg.source.requires_service() { hard_fail("prebuilt artifact is still building".to_string()) diff --git a/crates/socket-patch-core/src/vendor/npm_lock.rs b/crates/socket-patch-core/src/vendor/npm_lock.rs index c169c185..ceab9443 100644 --- a/crates/socket-patch-core/src/vendor/npm_lock.rs +++ b/crates/socket-patch-core/src/vendor/npm_lock.rs @@ -3386,43 +3386,6 @@ mod tests { ); } - /// `auto` + integrity mismatch falls back to a local build (loudly): the - /// lock ends up rewired to the LOCALLY-recomputed integrity, not the bad - /// service value. - #[tokio::test] - async fn service_integrity_mismatch_auto_falls_back_to_build() { - let (served, _) = locally_built_artifact().await; - let wrong = sri_sha512(b"not the real tarball"); - let server = wiremock::MockServer::start().await; - mount_granted(&server, &wrong, &served).await; - - let fx = fixture().await; - let (result, entry, warnings) = expect_done( - vendor_service(&fx, &service_cfg(&server.uri(), VendorSource::Auto, false)).await, - ); - assert!( - result.success, - "auto must fall back to a successful build: {:?}", - result.error - ); - assert!(entry.is_some()); - let on_disk = tokio::fs::read(fx.root().join(fx.expected_rel_tgz())) - .await - .unwrap(); - let local_sri = sri_sha512(&on_disk); - assert_eq!( - lock_integrity(&fx.read_lock().await, "node_modules/left-pad"), - local_sri, - "fallback build's integrity, not the bad service value" - ); - assert!( - warnings - .iter() - .any(|w| w.code == "vendor_prebuilt_integrity_mismatch"), - "expected a vendor_prebuilt_integrity_mismatch advisory, got {warnings:?}" - ); - } - /// `auto` + pending_build falls back to a local build (with an advisory). #[tokio::test] async fn service_pending_build_auto_falls_back() { @@ -3569,4 +3532,35 @@ mod tests { ); } } + + /// An integrity mismatch is a hard failure under `auto` too — + /// never a quiet local-build fallback (service_fetch's contract); + /// the project is byte-untouched. + #[tokio::test] + async fn service_integrity_mismatch_auto_hard_fails() { + let (served, _) = locally_built_artifact().await; + let wrong = sri_sha512(b"not the real tarball"); + let server = wiremock::MockServer::start().await; + mount_granted(&server, &wrong, &served).await; + let fx = fixture().await; + let before = tokio::fs::read(fx.lock_path()).await.unwrap(); + let (result, entry, _) = expect_done( + vendor_service(&fx, &service_cfg(&server.uri(), VendorSource::Auto, false)).await, + ); + assert!( + !result.success, + "tampered bytes must not fall back to a build" + ); + assert!(entry.is_none()); + assert!( + result + .error + .as_deref() + .is_some_and(|e| e.contains("integrity")), + "{:?}", + result.error + ); + assert_eq!(tokio::fs::read(fx.lock_path()).await.unwrap(), before); + assert!(!fx.root().join(fx.expected_rel_tgz()).exists()); + } } diff --git a/crates/socket-patch-core/src/vendor/pypi.rs b/crates/socket-patch-core/src/vendor/pypi.rs index e3f21910..9b737b08 100644 --- a/crates/socket-patch-core/src/vendor/pypi.rs +++ b/crates/socket-patch-core/src/vendor/pypi.rs @@ -1540,10 +1540,15 @@ async fn try_pypi_service_wheel( platform_tags_display, })) } - ServiceArtifact::IntegrityMismatch(reason) => miss( - warnings, + // Bytes that fail integrity verification are an active tamper signal: + // ALWAYS a hard error, in `auto` exactly as in `service` — never a + // quiet local-build fallback (`ServiceArtifact`'s documented contract). + ServiceArtifact::IntegrityMismatch(reason) => hard_fail( "vendor_prebuilt_integrity_mismatch", - format!("prebuilt wheel failed integrity ({reason})"), + format!( + "prebuilt wheel failed integrity verification ({reason}); \ + refusing to fall back to a local build on tampered bytes" + ), ), ServiceArtifact::Pending => miss( warnings, @@ -5972,6 +5977,35 @@ wheels = [{url = "https://files.pythonhosted.org/six.whl", hash = "sha256:upstre .iter() .any(|warning| warning.code == "pypi_unmatched_lockfiles")); } + + /// An integrity mismatch is a hard failure under `auto` too — + /// never a quiet local-build fallback (service_fetch's contract). + #[tokio::test] + async fn service_integrity_mismatch_auto_hard_fails() { + let fx = e2e_fixture().await; + let sources = PatchSources::blobs_only(&fx.blobs); + let bytes = b"the real wheel bytes"; + let wrong = sri_sha512(b"different bytes entirely"); + let server = wiremock::MockServer::start().await; + mount_pypi_granted(&server, WHEEL_NAME, &wrong, bytes).await; + let outcome = vendor_pypi( + "pkg:pypi/six@1.16.0", + &fx.site_packages, + &fx.root, + &fx.record, + &sources, + "2026-06-09T00:00:00Z", + false, + false, + Some(&pypi_service_cfg(&server.uri(), VendorSource::Auto, false)), + ) + .await; + let VendorOutcome::Refused { code, .. } = outcome else { + panic!("tampered bytes fell back to a local build: {outcome:?}"); + }; + assert_eq!(code, "vendor_prebuilt_integrity_mismatch"); + assert!(!fx.root.join(format!(".socket/vendor/pypi/{UUID}")).exists()); + } } #[cfg(test)] From 611c8bb8a12aeac8f68576d08bbc9feb3a606d2f Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 23 Sep 2026 17:44:54 -0400 Subject: [PATCH 4/7] fix(vendor): refuse --vendor-source=service when no API client is configured Every backend's service helper treats !service_enabled() as "build locally", and service_enabled() is false without a client. The service-only policy was enforced by common::service_offline_conflict, which checked only --offline. So a VendorServiceConfig with source: Service and client: None built the artifact locally in every backend, contradicting service's fail-closed promise. The CLI always passes a client, so only library callers could reach this. The gate every backend already calls at its entry point now also refuses that combination (vendor_prebuilt_required), before any service consultation or write. Regression tests cover all eight backends. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 7 +++++ crates/socket-patch-cli/CLI_CONTRACT.md | 1 + crates/socket-patch-core/src/vendor/cargo.rs | 25 ++++++++++++++++ crates/socket-patch-core/src/vendor/common.rs | 20 ++++++++++--- .../src/vendor/composer_lock.rs | 22 ++++++++++++++ crates/socket-patch-core/src/vendor/gem.rs | 15 ++++++++++ crates/socket-patch-core/src/vendor/golang.rs | 30 +++++++++++++++++++ .../src/vendor/maven_repo.rs | 15 ++++++++++ .../socket-patch-core/src/vendor/npm_lock.rs | 16 ++++++++++ .../src/vendor/nuget_feed.rs | 15 ++++++++++ crates/socket-patch-core/src/vendor/pypi.rs | 27 +++++++++++++++++ .../src/vendor/service_fetch.rs | 4 +-- 12 files changed, 191 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 19ae5b15..ae4c42e8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -674,6 +674,13 @@ into the new version's section — see docs/releasing.md. integrity detail). Under `--vendor-source=service`, golang, composer, gem and pypi now report `vendor_prebuilt_integrity_mismatch` instead of `vendor_prebuilt_required`. +- **`service` vendor source without an API client is refused.** This affects + `socket-patch-core` callers that pass a `VendorServiceConfig` with + `source: Service` and no `client` (the CLI always configures a client). + Every backend used to build the artifact locally in that case, even though + `service` promises that only the patch service's artifact is used. They now + refuse with `vendor_prebuilt_required` before doing any work, the same way + `--offline` is already refused. ### Changed diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 1429cb8e..51423d1f 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -479,6 +479,7 @@ per service outcome: | gem stub gemspec missing / invalid | local build + `vendor_prebuilt_stub_missing` / `vendor_prebuilt_stub_invalid` (invalid + gem not installed: refuse `vendor_prebuilt_stub_invalid` — no stub source exists) | refuse (`vendor_prebuilt_required` / `vendor_prebuilt_stub_invalid`) | | 401 / 403 grant / 5xx / network error | local build + `vendor_prebuilt_unavailable` | refuse | | `--offline` | local build | refuse (`vendor_service_offline_conflict`) | +| no API client configured (library callers of the vendor engine; the CLI always configures one) | local build | refuse (`vendor_prebuilt_required`) | **golang service leg staging (v5.0)**: the module zip is downloaded, extracted and `h1:`-verified in a `.socket-stage` sibling and swapped into place only afterwards; a failed re-download of a WIRED, present copy keeps the copy and its `replace` directive (previously both were torn down), while a missing copy still drops the dangling directive. diff --git a/crates/socket-patch-core/src/vendor/cargo.rs b/crates/socket-patch-core/src/vendor/cargo.rs index db4027a8..52b905a5 100644 --- a/crates/socket-patch-core/src/vendor/cargo.rs +++ b/crates/socket-patch-core/src/vendor/cargo.rs @@ -3148,4 +3148,29 @@ mod tests { assert!(!backup_dir_for(©).exists(), "no parked backup"); assert!(stage.exists(), "the stage is left for the caller's cleanup"); } + + /// `--vendor-source=service` with no configured client must fail + /// closed, never quietly build locally. + #[tokio::test] + async fn service_mode_without_client_refuses() { + let (dir, blobs, pristine, record) = fixture().await; + let root = dir.path(); + let mut cfg = cargo_service_cfg("http://127.0.0.1:1", VendorSource::Service, false); + cfg.client = None; + let sources = PatchSources::blobs_only(&blobs); + let outcome = vendor_cargo_crate( + PURL, + &pristine, + root, + &record, + &sources, + "2026-06-09T00:00:00Z", + false, + false, + Some(&cfg), + ) + .await; + expect_refused(outcome, "vendor_prebuilt_required"); + assert!(!root.join(format!(".socket/vendor/cargo/{UUID}")).exists()); + } } diff --git a/crates/socket-patch-core/src/vendor/common.rs b/crates/socket-patch-core/src/vendor/common.rs index 7f2dc5a2..54c2f680 100644 --- a/crates/socket-patch-core/src/vendor/common.rs +++ b/crates/socket-patch-core/src/vendor/common.rs @@ -92,19 +92,31 @@ pub(crate) fn done( } } -/// Shared helper the vendor backends delegate to: the fail-closed refusal -/// for `--vendor-source=service` combined with `--offline`, checked before -/// any service consultation. +/// Shared helper the vendor backends delegate to: the fail-closed refusals +/// for a `--vendor-source=service` run that cannot reach the service — +/// combined with `--offline`, or with no API client configured — checked +/// before any service consultation. Every backend's service helper treats +/// `!service_enabled()` as "build locally", so this is the one gate that +/// keeps `service` mode from silently building. pub(crate) fn service_offline_conflict( service: Option<&VendorServiceConfig>, ) -> Option { let cfg = service?; - if cfg.source.requires_service() && cfg.offline { + if !cfg.source.requires_service() { + return None; + } + if cfg.offline { return Some(refused( "vendor_service_offline_conflict", "--vendor-source=service needs the network but --offline is set", )); } + if cfg.client.is_none() { + return Some(refused( + "vendor_prebuilt_required", + "--vendor-source=service needs the patch service but no API client is configured", + )); + } None } diff --git a/crates/socket-patch-core/src/vendor/composer_lock.rs b/crates/socket-patch-core/src/vendor/composer_lock.rs index 8d9c7034..e69740f8 100644 --- a/crates/socket-patch-core/src/vendor/composer_lock.rs +++ b/crates/socket-patch-core/src/vendor/composer_lock.rs @@ -3513,4 +3513,26 @@ mod tests { .join(format!(".socket/vendor/composer/{UUID}")) .exists()); } + + /// `--vendor-source=service` with no configured client must fail + /// closed, never quietly build locally. + #[tokio::test] + async fn service_mode_without_client_refuses() { + let lock = lock_value("psr/log", "3.0.2", false); + let (dir, blobs, installed, record) = fixture(&lock).await; + let root = dir.path(); + let lock_before = tokio::fs::read(root.join(COMPOSER_LOCK)).await.unwrap(); + let mut cfg = composer_service_cfg("http://127.0.0.1:1", VendorSource::Service, false); + cfg.client = None; + let (code, _) = + unwrap_refused(vendor_with_service(root, &blobs, &installed, &record, &cfg).await); + assert_eq!(code, "vendor_prebuilt_required"); + assert!(!root + .join(format!(".socket/vendor/composer/{UUID}")) + .exists()); + assert_eq!( + tokio::fs::read(root.join(COMPOSER_LOCK)).await.unwrap(), + lock_before + ); + } } diff --git a/crates/socket-patch-core/src/vendor/gem.rs b/crates/socket-patch-core/src/vendor/gem.rs index d0795817..cb8e6985 100644 --- a/crates/socket-patch-core/src/vendor/gem.rs +++ b/crates/socket-patch-core/src/vendor/gem.rs @@ -8222,4 +8222,19 @@ mod tests { ); } } + + /// `--vendor-source=service` with no configured client must + /// fail closed, never quietly build locally. + #[tokio::test] + async fn service_mode_without_client_refuses() { + let (_tmp, root, installed, blobs, record) = fixture(GEMFILE_DIRECT, LOCK_DIRECT).await; + let mut cfg = gem_service_cfg("http://127.0.0.1:1", VendorSource::Service, false); + cfg.client = None; + let outcome = run_vendor_service(&root, &blobs, &installed, &record, &cfg).await; + let VendorOutcome::Refused { code, .. } = outcome else { + panic!("service mode without a client built locally: {outcome:?}"); + }; + assert_eq!(code, "vendor_prebuilt_required"); + assert!(!root.join(".socket").exists(), "nothing written"); + } } diff --git a/crates/socket-patch-core/src/vendor/golang.rs b/crates/socket-patch-core/src/vendor/golang.rs index 6fad05f2..e3fffd53 100644 --- a/crates/socket-patch-core/src/vendor/golang.rs +++ b/crates/socket-patch-core/src/vendor/golang.rs @@ -2635,4 +2635,34 @@ mod tests { gomod_before ); } + + /// `--vendor-source=service` with no configured client must fail + /// closed, never quietly build locally. + #[tokio::test] + async fn service_mode_without_client_refuses() { + let (dir, blobs, pristine, record) = fixture().await; + let root = dir.path(); + let gomod_before = tokio::fs::read(root.join("go.mod")).await.unwrap(); + let mut cfg = go_service_cfg("http://127.0.0.1:1", VendorSource::Service, false); + cfg.client = None; + let sources = PatchSources::blobs_only(&blobs); + let outcome = vendor_go_module( + PURL, + &pristine, + root, + &record, + &sources, + "2026-06-09T00:00:00Z", + false, + false, + Some(&cfg), + ) + .await; + expect_refused(outcome, "vendor_prebuilt_required"); + assert!(!root.join(format!(".socket/vendor/golang/{UUID}")).exists()); + assert_eq!( + tokio::fs::read(root.join("go.mod")).await.unwrap(), + gomod_before + ); + } } diff --git a/crates/socket-patch-core/src/vendor/maven_repo.rs b/crates/socket-patch-core/src/vendor/maven_repo.rs index 9b3a2d34..5587caef 100644 --- a/crates/socket-patch-core/src/vendor/maven_repo.rs +++ b/crates/socket-patch-core/src/vendor/maven_repo.rs @@ -3809,4 +3809,19 @@ mod tests { "{w2:?}" ); } + + /// `--vendor-source=service` with no configured client must + /// fail closed, never quietly build locally. + #[tokio::test] + async fn service_mode_without_client_refuses() { + let (dir, blobs, installed, record) = fixture(Some(project_pom()), true, true).await; + let root = dir.path(); + let cfg = service_cfg(None, crate::vendor::VendorSource::Service, false); + let outcome = run_vendor_with_service(root, &blobs, &installed, &record, &cfg).await; + let VendorOutcome::Refused { code, .. } = outcome else { + panic!("service mode without a client built locally: {outcome:?}"); + }; + assert_eq!(code, "vendor_prebuilt_required"); + assert!(!root.join(".socket").exists(), "nothing written"); + } } diff --git a/crates/socket-patch-core/src/vendor/npm_lock.rs b/crates/socket-patch-core/src/vendor/npm_lock.rs index ceab9443..424a2f72 100644 --- a/crates/socket-patch-core/src/vendor/npm_lock.rs +++ b/crates/socket-patch-core/src/vendor/npm_lock.rs @@ -3563,4 +3563,20 @@ mod tests { assert_eq!(tokio::fs::read(fx.lock_path()).await.unwrap(), before); assert!(!fx.root().join(fx.expected_rel_tgz()).exists()); } + + /// `--vendor-source=service` with no configured client must fail + /// closed, never quietly build locally. + #[tokio::test] + async fn service_mode_without_client_refuses() { + let fx = fixture().await; + let before = tokio::fs::read(fx.lock_path()).await.unwrap(); + let mut cfg = service_cfg("http://127.0.0.1:1", VendorSource::Service, false); + cfg.client = None; + match vendor_service(&fx, &cfg).await { + VendorOutcome::Refused { code, .. } => assert_eq!(code, "vendor_prebuilt_required"), + other => panic!("service mode without a client built locally: {other:?}"), + } + assert_eq!(tokio::fs::read(fx.lock_path()).await.unwrap(), before); + assert!(!fx.root().join(fx.expected_rel_tgz()).exists()); + } } diff --git a/crates/socket-patch-core/src/vendor/nuget_feed.rs b/crates/socket-patch-core/src/vendor/nuget_feed.rs index a8e6acaf..4b18f8a0 100644 --- a/crates/socket-patch-core/src/vendor/nuget_feed.rs +++ b/crates/socket-patch-core/src/vendor/nuget_feed.rs @@ -4882,4 +4882,19 @@ mod tests { assert!(!root.join(".socket").exists(), "nothing written"); assert!(!root.join("nuget.config").exists()); } + + /// `--vendor-source=service` with no configured client must + /// fail closed, never quietly build locally. + #[tokio::test] + async fn service_mode_without_client_refuses() { + let (dir, blobs, installed, record) = fixture(true, None).await; + let root = dir.path(); + let cfg = integrity_cfg(None, crate::vendor::VendorSource::Service); + let outcome = run_vendor_cfg(root, &blobs, &installed, &record, Some(&cfg)).await; + let VendorOutcome::Refused { code, .. } = outcome else { + panic!("service mode without a client built locally: {outcome:?}"); + }; + assert_eq!(code, "vendor_prebuilt_required"); + assert!(!root.join(".socket").exists(), "nothing written"); + } } diff --git a/crates/socket-patch-core/src/vendor/pypi.rs b/crates/socket-patch-core/src/vendor/pypi.rs index 9b737b08..88a2c53f 100644 --- a/crates/socket-patch-core/src/vendor/pypi.rs +++ b/crates/socket-patch-core/src/vendor/pypi.rs @@ -6006,6 +6006,33 @@ wheels = [{url = "https://files.pythonhosted.org/six.whl", hash = "sha256:upstre assert_eq!(code, "vendor_prebuilt_integrity_mismatch"); assert!(!fx.root.join(format!(".socket/vendor/pypi/{UUID}")).exists()); } + + /// `--vendor-source=service` with no configured client must fail + /// closed, never quietly build locally. + #[tokio::test] + async fn service_mode_without_client_refuses() { + let fx = e2e_fixture().await; + let sources = PatchSources::blobs_only(&fx.blobs); + let mut cfg = pypi_service_cfg("http://127.0.0.1:1", VendorSource::Service, false); + cfg.client = None; + let outcome = vendor_pypi( + "pkg:pypi/six@1.16.0", + &fx.site_packages, + &fx.root, + &fx.record, + &sources, + "2026-06-09T00:00:00Z", + false, + false, + Some(&cfg), + ) + .await; + let VendorOutcome::Refused { code, .. } = outcome else { + panic!("service mode without a client built locally: {outcome:?}"); + }; + assert_eq!(code, "vendor_prebuilt_required"); + assert!(!fx.root.join(format!(".socket/vendor/pypi/{UUID}")).exists()); + } } #[cfg(test)] diff --git a/crates/socket-patch-core/src/vendor/service_fetch.rs b/crates/socket-patch-core/src/vendor/service_fetch.rs index f19cd799..81e53d6e 100644 --- a/crates/socket-patch-core/src/vendor/service_fetch.rs +++ b/crates/socket-patch-core/src/vendor/service_fetch.rs @@ -136,8 +136,8 @@ pub(crate) async fn service_archive_copy( warnings: &mut Vec, ) -> ServiceCopy { // The maven/nuget flows have no earlier guard, so the fail-closed - // `--vendor-source=service` + `--offline` refusal lives here (the other - // backends check the same helper at their entry points). + // `--vendor-source=service` refusals (`--offline`, no API client) live + // here (the other backends check the same helper at their entry points). if let Some(refusal) = service_offline_conflict(service) { return ServiceCopy::HardFail(Box::new(refusal)); } From c0581ac95db7fffd2d7a0a794b93617c27504a3d Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 23 Sep 2026 18:19:19 -0400 Subject: [PATCH 5/7] fix(vendor): route the cargo wired-copy rebuild through --vendor-source The in-sync hot path rebuilt a missing/stale crate copy with copy_and_patch directly, so `--vendor-source=service` with --offline or no API client rebuilt locally and reported success, and online service mode never consulted the patch service. Refuse via service_offline_conflict first and prefer cargo_service_copy, falling back to the local build only where the policy allows, as composer and gem already do. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 9 ++ crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- crates/socket-patch-core/src/vendor/cargo.rs | 149 ++++++++++++++++--- 3 files changed, 140 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ae4c42e8..a6556554 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -681,6 +681,15 @@ into the new version's section — see docs/releasing.md. `service` promises that only the patch service's artifact is used. They now refuse with `vendor_prebuilt_required` before doing any work, the same way `--offline` is already refused. +- **Rebuilding a missing cargo vendored copy now honours + `--vendor-source`.** When a wired project's committed crate copy was + missing or stale, `vendor` always rebuilt it locally from the installed + source. Under `--vendor-source=service` it did so even with `--offline` + or without an API client, and reported success. The rebuild now uses the + patch service's prebuilt crate like a fresh vendor does, so `service` + mode refuses (`vendor_service_offline_conflict` / `vendor_prebuilt_required`) + when the service cannot be used, and `auto` still builds locally when it + has no prebuilt crate. ### Changed diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 51423d1f..f0574b1b 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1125,7 +1125,7 @@ Every `--json` invocation emits a single JSON object that follows the **unified | `vendor_fetch_unverifiable` | `skipped` (warning) | vendor: the lockfile records no usable integrity for the missing package; nothing was fetched (fail-closed) and the `package_not_installed` skip follows. | | `vendor_artifact_missing` | `skipped` (warning) / `failed` | vendor: the committed artifact is gone — the registry resolution is recovered from the ledger and the artifact rebuilt (warning); repair `--offline` with no local source surfaces it as the per-entry failure instead. | | `vendor_artifact_corrupt` | `failed` | repair `--offline`: the committed artifact fails verification (member afterHashes or the ledger's whole-file sha256) and no local source can rebuild it. Online repairs rebuild instead. | -| `vendor_artifact_rebuilt` | `skipped` (warning) | vendor / scan `--vendor`: a wired-but-missing/stale artifact was rebuilt in place. The lockfiles are untouched, except that nuget re-pins `packages.lock.json` to the rebuilt bytes. gem/maven/nuget: the package's event is `applied` (also for a rebuild from the patch service), and the ledger entry's artifact fingerprint (gem `fileInventory`, maven/nuget `sha256` + `size`, and the nuget lock pin) is refreshed to the rebuilt bytes, and its wiring records are kept unchanged, so `--revert` still restores the pre-vendor files. A rebuild with no existing ledger entry records none. Other ecosystems leave the ledger entry untouched. (Under `repair` the `rebuilt` event carries this signal.) | +| `vendor_artifact_rebuilt` | `skipped` (warning) | vendor / scan `--vendor`: a wired-but-missing/stale artifact was rebuilt in place. The lockfiles are untouched, except that nuget re-pins `packages.lock.json` to the rebuilt bytes. gem/maven/nuget: the package's event is `applied` (also for a rebuild from the patch service), and the ledger entry's artifact fingerprint (gem `fileInventory`, maven/nuget `sha256` + `size`, and the nuget lock pin) is refreshed to the rebuilt bytes, and its wiring records are kept unchanged, so `--revert` still restores the pre-vendor files. A rebuild with no existing ledger entry records none. cargo/composer/gem rebuilds honour `--vendor-source` like a fresh vendor (`service` downloads the prebuilt artifact and refuses when it cannot). Other ecosystems leave the ledger entry untouched. (Under `repair` the `rebuilt` event carries this signal.) | | `vendor_artifact_rebuild_failed` | `failed` | repair: the rebuild ran but the result failed verification against the recorded fingerprint (e.g. an edited state.json sha); the unverifiable artifact was removed. | | `vendor_artifact_unrepairable` | `failed` | repair: no verifiable pristine source exists (not installed + lockfile rewired + no recoverable ledger fragment), the wheel is platform-locked with no installed copy, or the ledger entry itself cannot be trusted. | | `vendor_uuid_mismatch` | `skipped` | repair: the manifest's patch uuid moved past the vendored artifact — a re-vendor (`vendor` / `scan --vendor`) is pending; repair does not cross patch generations. | diff --git a/crates/socket-patch-core/src/vendor/cargo.rs b/crates/socket-patch-core/src/vendor/cargo.rs index 52b905a5..288eab0f 100644 --- a/crates/socket-patch-core/src/vendor/cargo.rs +++ b/crates/socket-patch-core/src/vendor/cargo.rs @@ -553,26 +553,39 @@ pub async fn vendor_cargo_crate( // first run's unrecoverable lock originals. The rebuild is staged: a // failure must leave the previous (drifted-but-buildable) copy and // the live wiring exactly as they were, never a deleted copy under a - // still-pointing `[patch]` entry. + // still-pointing `[patch]` entry. Service-preferred like the full + // path, so `--vendor-source=service` never quietly builds locally. + if let Some(refusal) = service_offline_conflict(service) { + return refusal; + } let mut warnings: Vec = Vec::new(); - let result = match copy_and_patch( - purl, - pristine_src, - ©_dir, - &uuid_dir, - record, - sources, - force, - false, // live-wired: never unwind the uuid dir on failure - name, - version, - &mut warnings, - ) - .await - { - Ok(result) => result, - Err(result) => return done(result, None, warnings), - }; + let result = + match cargo_service_copy(service, record, name, ©_dir, &uuid_dir, &mut warnings) + .await + { + CargoServiceCopy::Used => already_patched_result(purl, ©_dir, &record.files), + CargoServiceCopy::HardFail(outcome) => return *outcome, + CargoServiceCopy::FallBack => { + match copy_and_patch( + purl, + pristine_src, + ©_dir, + &uuid_dir, + record, + sources, + force, + false, // live-wired: never unwind the uuid dir on failure + name, + version, + &mut warnings, + ) + .await + { + Ok(result) => result, + Err(result) => return done(result, None, warnings), + } + } + }; warnings.push(VendorWarning::new( "vendor_artifact_rebuilt", format!( @@ -3173,4 +3186,102 @@ mod tests { expect_refused(outcome, "vendor_prebuilt_required"); assert!(!root.join(format!(".socket/vendor/cargo/{UUID}")).exists()); } + + /// Vendor locally, then delete the committed copy so the next run takes + /// the wired-copy rebuild path. + async fn wired_with_missing_copy() -> (tempfile::TempDir, PathBuf, PathBuf, PatchRecord) { + let (dir, blobs, pristine, record) = fixture().await; + expect_done(run_vendor(PURL, dir.path(), &blobs, &pristine, &record, false).await); + crate::patch::copy_tree::remove_tree(&dir.path().join(copy_rel())) + .await + .unwrap(); + (dir, blobs, pristine, record) + } + + /// The wired-copy rebuild honours `--vendor-source=service` like the + /// fresh path: no API client or `--offline` refuses instead of quietly + /// rebuilding locally, and the copy is not recreated. + #[tokio::test] + async fn wired_rebuild_service_mode_unreachable_refuses() { + for (offline, want) in [ + (false, "vendor_prebuilt_required"), + (true, "vendor_service_offline_conflict"), + ] { + let (dir, blobs, pristine, record) = wired_with_missing_copy().await; + let root = dir.path(); + let mut cfg = cargo_service_cfg("http://127.0.0.1:1", VendorSource::Service, offline); + if !offline { + cfg.client = None; + } + let sources = PatchSources::blobs_only(&blobs); + let outcome = vendor_cargo_crate( + PURL, + &pristine, + root, + &record, + &sources, + "2026-06-09T00:00:00Z", + false, + false, + Some(&cfg), + ) + .await; + expect_refused(outcome, want); + assert!( + !root.join(copy_rel()).exists(), + "service mode must not rebuild the copy locally (offline={offline})" + ); + } + } + + /// Online `service` mode rebuilds the wired copy from the prebuilt crate, + /// not from the pristine source (a deliberately-missing path here). + #[tokio::test] + async fn wired_rebuild_service_mode_uses_prebuilt_crate() { + let (dir, blobs, _pristine, record) = wired_with_missing_copy().await; + let root = dir.path(); + let crate_tgz = make_crate_tgz( + "cfg-if-1.0.4", + &[ + ("src/lib.rs", PATCHED), + ( + "Cargo.toml", + b"[package]\nname = \"cfg-if\"\nversion = \"1.0.4\"\n", + ), + ], + ); + let server = wiremock::MockServer::start().await; + mount_cargo_granted(&server, &sri_sha512(&crate_tgz), &crate_tgz).await; + let sources = PatchSources::blobs_only(&blobs); + let outcome = vendor_cargo_crate( + PURL, + &root.join("no-such-pristine"), + root, + &record, + &sources, + "2026-06-09T00:00:00Z", + false, + false, + Some(&cargo_service_cfg( + &server.uri(), + VendorSource::Service, + false, + )), + ) + .await; + let (result, entry, warnings) = expect_done(outcome); + assert!(result.success, "{:?}", result.error); + assert!(entry.is_none(), "artifact-only rebuild records no entry"); + assert!( + warnings.iter().any(|w| w.code == "vendor_artifact_rebuilt"), + "{warnings:?}" + ); + assert!( + warnings + .iter() + .any(|w| w.code == "vendor_prebuilt_downloaded"), + "{warnings:?}" + ); + assert_eq!(tokio::fs::read(copy_lib(root)).await.unwrap(), PATCHED); + } } From 276c8a953acefa46fb35d62ffd8c652e3fec05a8 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 23 Sep 2026 18:19:29 -0400 Subject: [PATCH 6/7] fix(vendor): verify a served pypi wheel / npm tarball carries the patch The service SRI proves only that the download is intact. A wheel or tarball whose patched members still held the original bytes was written as-is, reported as already patched, and pinned in the lockfile. Check each patched member against its afterHash before using the artifact: auto builds locally with vendor_prebuilt_layout_mismatch, service refuses (pypi: vendor_prebuilt_required; npm fails the package). Adds read_archive_bytes_to_map for the in-memory tarball check. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 7 +- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- crates/socket-patch-core/src/patch/package.rs | 13 +- .../src/vendor/npm_common.rs | 113 ++++++++++++++++- crates/socket-patch-core/src/vendor/pypi.rs | 116 ++++++++++++++++-- 5 files changed, 234 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a6556554..40820fe0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -654,15 +654,16 @@ into the new version's section — see docs/releasing.md. so revert still restores the pre-vendor files. If `state.json` has no entry for the package, the rebuild still runs but no entry is added, because the run has no pre-vendor originals to record. -- **A prebuilt maven `.jar` or nuget `.nupkg` from the patch service must now - contain the patched files.** Checking its integrity hash only showed that +- **A prebuilt maven `.jar`, nuget `.nupkg`, pypi wheel or npm tarball from + the patch service must now contain the patched files.** Checking its integrity hash only showed that the download was intact, not that the archive carried the patch. The archive was still written as-is and every file was reported as already patched, so an unpatched archive could be committed and then rebuilt on every run. Each patched file inside the archive is now checked against the patch's expected hash before the archive is used. On a mismatch, `auto` builds the archive locally and warns `vendor_prebuilt_layout_mismatch`, - and `--vendor-source=service` refuses with `vendor_prebuilt_required`. + and `--vendor-source=service` refuses with `vendor_prebuilt_required` (npm + fails the package with the detail). - **A prebuilt artifact that fails its integrity check is always refused.** Under the default `--vendor-source=auto`, npm, pypi, golang, composer and gem (both the `.gem` and its stub gemspec) printed a warning and built the diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index f0574b1b..d38199c8 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -473,7 +473,7 @@ per service outcome: |---|---|---| | granted/reused, integrity ok | **use service** | **use service** | | integrity mismatch (including the gem stub gemspec) | **refuse** (`vendor_prebuilt_integrity_mismatch`; npm: the package fails with the integrity detail). Tampered bytes never fall back to a local build | refuse (same) | -| integrity ok, but the archive does not carry the patched files (a member at a recorded path fails its `afterHash`; checked for cargo/golang/composer/gem after extraction, and for maven/nuget before the archive is written) | local build + `vendor_prebuilt_layout_mismatch` | refuse (`vendor_prebuilt_required`) | +| integrity ok, but the archive does not carry the patched files (a member at a recorded path fails its `afterHash`; checked for cargo/golang/composer/gem after extraction, and for maven/nuget/pypi/npm before the archive is written; npm under `service` fails the package with the detail) | local build + `vendor_prebuilt_layout_mismatch` | refuse (`vendor_prebuilt_required`) | | still building (`pending_build` / serve 408) | local build + `vendor_prebuilt_pending` | refuse | | not built / withdrawn / not found / no usable artifact | local build (quiet) | refuse | | gem stub gemspec missing / invalid | local build + `vendor_prebuilt_stub_missing` / `vendor_prebuilt_stub_invalid` (invalid + gem not installed: refuse `vendor_prebuilt_stub_invalid` — no stub source exists) | refuse (`vendor_prebuilt_required` / `vendor_prebuilt_stub_invalid`) | diff --git a/crates/socket-patch-core/src/patch/package.rs b/crates/socket-patch-core/src/patch/package.rs index 9e9ae261..ca305937 100644 --- a/crates/socket-patch-core/src/patch/package.rs +++ b/crates/socket-patch-core/src/patch/package.rs @@ -86,10 +86,21 @@ pub fn read_archive_to_map(archive_path: &Path) -> Result Result>, ArchiveError> { + read_tgz_to_map(bytes) +} + +fn read_tgz_to_map(reader: R) -> Result>, ArchiveError> { // Hard-cap decompressed bytes to defuse gzip / tar bombs. Reads // beyond the limit yield EOF, which the tar parser surfaces as a // truncated-archive error. - let bounded = GzDecoder::new(file).take(MAX_TOTAL_DECOMPRESSED_BYTES); + let bounded = GzDecoder::new(reader).take(MAX_TOTAL_DECOMPRESSED_BYTES); let mut tar = Archive::new(bounded); let mut out: HashMap> = HashMap::new(); diff --git a/crates/socket-patch-core/src/vendor/npm_common.rs b/crates/socket-patch-core/src/vendor/npm_common.rs index 8abcef6a..04eb4fb4 100644 --- a/crates/socket-patch-core/src/vendor/npm_common.rs +++ b/crates/socket-patch-core/src/vendor/npm_common.rs @@ -347,6 +347,27 @@ async fn try_service_pack( let hard_fail = |detail: String| ServicePackDecision::HardFail(Box::new(done_failure(purl, detail))); match fetch_verified_archive(cfg, &record.uuid).await { + // The SRI proves only that the transfer is intact: require the + // tarball to carry every patched file at its afterHash before + // reporting the package patched and wiring the lock to it. + ServiceArtifact::Ready(archive) + if !tgz_bytes_match_after_hashes(&archive.bytes, record) => + { + let reason = format!( + "prebuilt tarball for {}@{} does not carry the patched files at their \ + recorded paths", + coords.name, coords.version + ); + if cfg.source.requires_service() { + hard_fail(reason) + } else { + warnings.push(VendorWarning::new( + "vendor_prebuilt_layout_mismatch", + format!("{reason}; building locally instead"), + )); + ServicePackDecision::FallBack + } + } ServiceArtifact::Ready(archive) => { match staged_pack_from_service_bytes( purl, @@ -367,8 +388,8 @@ async fn try_service_pack( ), )); // No local apply to verify — every patched file reads as - // `AlreadyPatched` (trust is the service-verified - // integrity). + // `AlreadyPatched` (the tarball's members were checked + // against their afterHashes above). let result = already_patched_result( purl, &project_root.join(&staged.rel_tgz), @@ -501,6 +522,19 @@ async fn staged_pack_from_service_bytes( }) } +/// True when the downloaded npm tarball (`package/`-rooted, like the +/// `record.files` keys) has every patched file hashing to its `afterHash`. +fn tgz_bytes_match_after_hashes(bytes: &[u8], record: &PatchRecord) -> bool { + use crate::hash::git_sha256::compute_git_sha256_from_bytes; + let Ok(map) = crate::patch::package::read_archive_bytes_to_map(bytes) else { + return false; + }; + record.files.iter().all(|(file_name, info)| { + map.get(normalize_file_path(file_name)) + .is_some_and(|content| compute_git_sha256_from_bytes(content) == info.after_hash) + }) +} + /// Read the patched `package.json` out of a written vendored tarball (used /// only when the patch rewrote it — the lock's dependency mirror is then /// stale and recomputed from this). @@ -1216,7 +1250,7 @@ mod tests { let server = wiremock::MockServer::start().await; mount_granted(&server, &served_sri, &tgz).await; - let record = record_with_uuid(UUID); + let record = patched_index_record(); for source in [VendorSource::Service, VendorSource::Auto] { let tmp = tempfile::tempdir().unwrap(); let cfg = service_cfg(&server.uri(), source); @@ -1233,6 +1267,79 @@ mod tests { } } + /// A record patching `package/index.js` to `PATCHED_INDEX` (real hashes, + /// so a served tarball carrying the patch passes the afterHash check). + fn patched_index_record() -> PatchRecord { + use crate::hash::git_sha256::compute_git_sha256_from_bytes; + let mut record = record_with_uuid(UUID); + record.files.clear(); + record.files.insert( + "package/index.js".to_string(), + PatchFileInfo { + before_hash: compute_git_sha256_from_bytes(ORIG_INDEX), + after_hash: compute_git_sha256_from_bytes(PATCHED_INDEX), + }, + ); + record + } + + /// A served tarball with an intact SRI whose `index.js` is still the + /// ORIGINAL bytes is not the patched package: `service` fails the package + /// and writes nothing; `auto` warns and builds locally instead. + #[tokio::test] + async fn service_tarball_failing_after_hashes_is_rejected() { + let tgz = build_tgz(&[("index.js", ORIG_INDEX)]).await; + let sri = PackedTarball::from_bytes(&tgz).integrity; + let server = wiremock::MockServer::start().await; + mount_granted(&server, &sri, &tgz).await; + + let tmp = tempfile::tempdir().unwrap(); + let cfg = service_cfg(&server.uri(), VendorSource::Service); + let err = expect_err(run_pipeline(tmp.path(), &patched_index_record(), Some(&cfg)).await); + expect_done_failure(err, "does not carry the patched files"); + assert!( + !tmp.path().join(".socket/vendor").exists(), + "an unpatched service tarball is never written" + ); + + let (tmp, record) = local_fixture(b"{\"name\":\"left-pad\",\"version\":\"1.3.0\"}").await; + let root = tmp.path(); + let blobs = root.join(".socket/blobs"); + let sources = PatchSources::blobs_only(&blobs); + let mut warnings = Vec::new(); + let cfg = service_cfg(&server.uri(), VendorSource::Auto); + let Ok((Some(staged), result)) = stage_patch_pack( + LP_PURL, + &root.join("node_modules/left-pad"), + root, + &record, + &sources, + false, + false, + &mut warnings, + Some(&cfg), + ) + .await + else { + panic!("auto must fall back to the local build"); + }; + assert!(result.success, "{:?}", result.error); + assert!( + warnings + .iter() + .any(|w| w.code == "vendor_prebuilt_layout_mismatch"), + "{warnings:?}" + ); + assert!( + !warnings + .iter() + .any(|w| w.code == "vendor_prebuilt_downloaded"), + "{warnings:?}" + ); + let written = tokio::fs::read(root.join(&staged.rel_tgz)).await.unwrap(); + assert_ne!(written, tgz, "the served tarball was not used"); + } + // ──────────── staged_pack_from_service_bytes unit matrix ──────────── fn record_also_patching_package_json() -> PatchRecord { diff --git a/crates/socket-patch-core/src/vendor/pypi.rs b/crates/socket-patch-core/src/vendor/pypi.rs index 88a2c53f..19bfe0b3 100644 --- a/crates/socket-patch-core/src/vendor/pypi.rs +++ b/crates/socket-patch-core/src/vendor/pypi.rs @@ -22,6 +22,7 @@ use crate::utils::toml_edit_ext::has_table; use super::common::{ already_patched_result, done, prune_empty_vendor_levels, refused, service_offline_conflict, + zip_bytes_match_after_hashes, }; use super::path::vendor_uuid_dir_rel; use super::pypi_pdm::{PdmProject, PdmTarget}; @@ -1482,6 +1483,20 @@ async fn try_pypi_service_wheel( .to_string(), ); }; + // The SRI proves only that the transfer is intact. A wheel's + // members are site-packages-relative (the `record.files` keys), + // so require each patched file to carry its afterHash before + // reporting the package patched and pinning the lockfile to it. + if !zip_bytes_match_after_hashes(&archive.bytes, &record.files) { + return miss( + warnings, + "vendor_prebuilt_layout_mismatch", + format!( + "prebuilt wheel for {base} does not carry the patched files at \ + their recorded paths" + ), + ); + } let rel_wheel = format!("{uuid_dir_rel}/{wheel_name}"); let sha256_hex = hex::encode(Sha256::digest(&archive.bytes)); // In-sync rebuild: the lockfile still pins the first vendor's @@ -2469,6 +2484,24 @@ wheels = [ const WHEEL_NAME: &str = "six-1.16.0-py2.py3-none-any.whl"; + /// A wheel zip whose `six.py` member is `six_py`, plus a `tag` member so + /// callers can make the bytes differ from any other wheel. + fn wheel_with(six_py: &[u8], tag: &[u8]) -> Vec { + use std::io::Write as _; + let mut zip = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); + let opts = zip::write::SimpleFileOptions::default(); + zip.start_file("six.py", opts).unwrap(); + zip.write_all(six_py).unwrap(); + zip.start_file("socket-test-tag.txt", opts).unwrap(); + zip.write_all(tag).unwrap(); + zip.finish().unwrap().into_inner() + } + + /// A served wheel that carries the patched `six.py`. + fn served_wheel(tag: &[u8]) -> Vec { + wheel_with(PATCHED, tag) + } + fn sri_sha512(bytes: &[u8]) -> String { use base64::Engine as _; format!( @@ -2536,7 +2569,7 @@ wheels = [ async fn service_success_requirements_writes_wheel_and_wires_sha256() { let fx = e2e_fixture().await; let sources = PatchSources::blobs_only(&fx.blobs); - let bytes = b"prebuilt wheel bytes from the service"; + let bytes: &[u8] = &served_wheel(b"prebuilt wheel bytes from the service"); let sri = sri_sha512(bytes); let server = wiremock::MockServer::start().await; mount_pypi_granted(&server, WHEEL_NAME, &sri, bytes).await; @@ -2593,6 +2626,67 @@ wheels = [ ); } + /// A served wheel with an intact SRI whose `six.py` is still the ORIGINAL + /// bytes is not the patched package: `service` refuses and writes no + /// wheel; `auto` warns and builds locally (which carries the patch). + #[tokio::test] + async fn service_wheel_failing_after_hashes_is_rejected() { + for source in [VendorSource::Service, VendorSource::Auto] { + let fx = e2e_fixture().await; + let sources = PatchSources::blobs_only(&fx.blobs); + let bytes = wheel_with(ORIG, b"unpatched"); + let server = wiremock::MockServer::start().await; + mount_pypi_granted(&server, WHEEL_NAME, &sri_sha512(&bytes), &bytes).await; + let outcome = vendor_pypi( + "pkg:pypi/six@1.16.0", + &fx.site_packages, + &fx.root, + &fx.record, + &sources, + "2026-06-09T00:00:00Z", + false, + false, + Some(&pypi_service_cfg(&server.uri(), source, false)), + ) + .await; + let wheel = fx + .root + .join(format!(".socket/vendor/pypi/{UUID}/{WHEEL_NAME}")); + match source { + VendorSource::Service => { + let VendorOutcome::Refused { code, .. } = &outcome else { + panic!("service must refuse an unpatched wheel, got {outcome:?}"); + }; + assert_eq!(*code, "vendor_prebuilt_required"); + assert!(!wheel.exists(), "no unpatched wheel written"); + } + _ => { + let VendorOutcome::Done { + result, warnings, .. + } = &outcome + else { + panic!("auto must fall back, got {outcome:?}"); + }; + assert!(result.success, "{:?}", result.error); + assert!( + warnings + .iter() + .any(|w| w.code == "vendor_prebuilt_layout_mismatch"), + "{warnings:?}" + ); + assert!( + !warnings + .iter() + .any(|w| w.code == "vendor_prebuilt_downloaded"), + "{warnings:?}" + ); + let on_disk = tokio::fs::read(&wheel).await.unwrap(); + assert_ne!(on_disk, bytes, "the served wheel was not used"); + } + } + } + } + /// An sdist service artifact (not a `.whl`) falls back to the local wheel /// build under `auto` — pypi vendoring is wheel-based. #[tokio::test] @@ -2750,7 +2844,8 @@ wheels = [ .unwrap(); // The service offers a wheel whose bytes do NOT match the wired pin. - let bytes = b"service-built wheel bytes that differ from the local build"; + let bytes: &[u8] = + &served_wheel(b"service-built wheel bytes that differ from the local build"); let sri = sri_sha512(bytes); let server = wiremock::MockServer::start().await; mount_pypi_granted(&server, WHEEL_NAME, &sri, bytes).await; @@ -2834,7 +2929,8 @@ wheels = [ .await .unwrap(); - let bytes = b"service-built wheel bytes that differ from the local build"; + let bytes: &[u8] = + &served_wheel(b"service-built wheel bytes that differ from the local build"); let sri = sri_sha512(bytes); let server = wiremock::MockServer::start().await; mount_pypi_granted(&server, WHEEL_NAME, &sri, bytes).await; @@ -2874,7 +2970,7 @@ wheels = [ async fn in_sync_local_rebuild_pin_mismatch_fails_loudly() { let fx = e2e_fixture().await; let sources = PatchSources::blobs_only(&fx.blobs); - let bytes = b"prebuilt wheel bytes from the service"; + let bytes: &[u8] = &served_wheel(b"prebuilt wheel bytes from the service"); let sri = sri_sha512(bytes); let server = wiremock::MockServer::start().await; mount_pypi_granted(&server, WHEEL_NAME, &sri, bytes).await; @@ -2974,7 +3070,8 @@ wheels = [ .await .unwrap(); - let bytes = b"service-built wheel bytes that differ from the local build"; + let bytes: &[u8] = + &served_wheel(b"service-built wheel bytes that differ from the local build"); let sri = sri_sha512(bytes); let server = wiremock::MockServer::start().await; mount_pypi_granted(&server, WHEEL_NAME, &sri, bytes).await; @@ -3032,7 +3129,7 @@ wheels = [ async fn in_sync_ledgerless_local_rebuild_pin_mismatch_fails_loudly() { let fx = e2e_fixture().await; let sources = PatchSources::blobs_only(&fx.blobs); - let bytes = b"prebuilt wheel bytes from the service"; + let bytes: &[u8] = &served_wheel(b"prebuilt wheel bytes from the service"); let sri = sri_sha512(bytes); let server = wiremock::MockServer::start().await; mount_pypi_granted(&server, WHEEL_NAME, &sri, bytes).await; @@ -5352,7 +5449,7 @@ wheels = [ async fn service_uuid_dir_create_failure_hard_fails() { let fx = e2e_fixture().await; let sources = PatchSources::blobs_only(&fx.blobs); - let bytes = b"prebuilt wheel bytes from the service"; + let bytes: &[u8] = &served_wheel(b"prebuilt wheel bytes from the service"); let sri = sri_sha512(bytes); let server = wiremock::MockServer::start().await; mount_pypi_granted(&server, WHEEL_NAME, &sri, bytes).await; @@ -5391,7 +5488,7 @@ wheels = [ async fn service_wheel_write_failure_hard_fails_even_under_auto() { let fx = e2e_fixture().await; let sources = PatchSources::blobs_only(&fx.blobs); - let bytes = b"prebuilt wheel bytes from the service"; + let bytes: &[u8] = &served_wheel(b"prebuilt wheel bytes from the service"); let sri = sri_sha512(bytes); let server = wiremock::MockServer::start().await; mount_pypi_granted(&server, WHEEL_NAME, &sri, bytes).await; @@ -5723,7 +5820,8 @@ wheels = [ tokio::fs::remove_dir_all(uuid_dir_of(&fx)).await.unwrap(); // The service offers a wheel whose bytes do NOT match the wired pin. - let bytes = b"service-built wheel bytes that differ from the local build"; + let bytes: &[u8] = + &served_wheel(b"service-built wheel bytes that differ from the local build"); let sri = sri_sha512(bytes); let server = wiremock::MockServer::start().await; mount_pypi_granted(&server, WHEEL_NAME, &sri, bytes).await; From 032ab2b2d73d92d91683a16ab888261fd8a1b9a1 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Wed, 23 Sep 2026 18:19:31 -0400 Subject: [PATCH 7/7] fix(vendor): only record a rebuilt entry over a same-uuid predecessor A wiring-less refreshed entry relies on carry_forward_wiring, which re-attaches wiring only from a same-uuid ledger entry. Over an entry from another patch uuid (the run that wired this uuid never saved), the guard let it through, saving an entry --revert could not unwire and sweeping the other uuid's dir. Leave the ledger as it is instead. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 5 ++- crates/socket-patch-cli/CLI_CONTRACT.md | 2 +- .../socket-patch-cli/src/commands/vendor.rs | 19 +++++--- .../tests/in_process_vendor.rs | 43 +++++++++++++++++++ 4 files changed, 60 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 40820fe0..9163f2bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -652,8 +652,9 @@ into the new version's section — see docs/releasing.md. also reported as `already_vendored` instead of `applied`. The rebuild now records the new fingerprint and keeps the entry's original wiring records, so revert still restores the pre-vendor files. If `state.json` has no - entry for the package, the rebuild still runs but no entry is added, - because the run has no pre-vendor originals to record. + entry for the package, or only an entry from another patch uuid, the + rebuild still runs but the ledger is left as it is, because the run has no + pre-vendor originals to record. - **A prebuilt maven `.jar`, nuget `.nupkg`, pypi wheel or npm tarball from the patch service must now contain the patched files.** Checking its integrity hash only showed that the download was intact, not that the archive carried the patch. The diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index d38199c8..39e39453 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1125,7 +1125,7 @@ Every `--json` invocation emits a single JSON object that follows the **unified | `vendor_fetch_unverifiable` | `skipped` (warning) | vendor: the lockfile records no usable integrity for the missing package; nothing was fetched (fail-closed) and the `package_not_installed` skip follows. | | `vendor_artifact_missing` | `skipped` (warning) / `failed` | vendor: the committed artifact is gone — the registry resolution is recovered from the ledger and the artifact rebuilt (warning); repair `--offline` with no local source surfaces it as the per-entry failure instead. | | `vendor_artifact_corrupt` | `failed` | repair `--offline`: the committed artifact fails verification (member afterHashes or the ledger's whole-file sha256) and no local source can rebuild it. Online repairs rebuild instead. | -| `vendor_artifact_rebuilt` | `skipped` (warning) | vendor / scan `--vendor`: a wired-but-missing/stale artifact was rebuilt in place. The lockfiles are untouched, except that nuget re-pins `packages.lock.json` to the rebuilt bytes. gem/maven/nuget: the package's event is `applied` (also for a rebuild from the patch service), and the ledger entry's artifact fingerprint (gem `fileInventory`, maven/nuget `sha256` + `size`, and the nuget lock pin) is refreshed to the rebuilt bytes, and its wiring records are kept unchanged, so `--revert` still restores the pre-vendor files. A rebuild with no existing ledger entry records none. cargo/composer/gem rebuilds honour `--vendor-source` like a fresh vendor (`service` downloads the prebuilt artifact and refuses when it cannot). Other ecosystems leave the ledger entry untouched. (Under `repair` the `rebuilt` event carries this signal.) | +| `vendor_artifact_rebuilt` | `skipped` (warning) | vendor / scan `--vendor`: a wired-but-missing/stale artifact was rebuilt in place. The lockfiles are untouched, except that nuget re-pins `packages.lock.json` to the rebuilt bytes. gem/maven/nuget: the package's event is `applied` (also for a rebuild from the patch service), and the ledger entry's artifact fingerprint (gem `fileInventory`, maven/nuget `sha256` + `size`, and the nuget lock pin) is refreshed to the rebuilt bytes, and its wiring records are kept unchanged, so `--revert` still restores the pre-vendor files. A rebuild whose ledger has no entry for the package, or only one from another patch uuid, records none. cargo/composer/gem rebuilds honour `--vendor-source` like a fresh vendor (`service` downloads the prebuilt artifact and refuses when it cannot). Other ecosystems leave the ledger entry untouched. (Under `repair` the `rebuilt` event carries this signal.) | | `vendor_artifact_rebuild_failed` | `failed` | repair: the rebuild ran but the result failed verification against the recorded fingerprint (e.g. an edited state.json sha); the unverifiable artifact was removed. | | `vendor_artifact_unrepairable` | `failed` | repair: no verifiable pristine source exists (not installed + lockfile rewired + no recoverable ledger fragment), the wheel is platform-locked with no installed copy, or the ledger entry itself cannot be trusted. | | `vendor_uuid_mismatch` | `skipped` | repair: the manifest's patch uuid moved past the vendored artifact — a re-vendor (`vendor` / `scan --vendor`) is pending; repair does not cross patch generations. | diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index 57ee83dc..c02e1e4f 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -1784,12 +1784,19 @@ pub(crate) async fn vendor_records( // An artifact-only rebuild hands back a refreshed // fingerprint with no wiring of its own: it relies on // the ledger entry it replaces for the pre-vendor - // originals. With no such entry, recording it would give - // `--revert` an entry that deletes the artifact yet - // cannot unwire the project — leave the ledger as is. - let entry = entry.filter(|_| { - state.entries.contains_key(candidate.as_str()) - || !warnings.iter().any(|w| w.code == "vendor_artifact_rebuilt") + // originals, and `carry_forward_wiring` re-attaches them + // only from a SAME-uuid predecessor. With no such entry + // (none at all, or one from another patch generation), + // recording it would give `--revert` an entry that + // deletes the artifact yet cannot unwire the project — + // leave the ledger as is. + let rebuilt = warnings.iter().any(|w| w.code == "vendor_artifact_rebuilt"); + let entry = entry.filter(|e| { + !rebuilt + || state + .entries + .get(candidate.as_str()) + .is_some_and(|prev| prev.uuid == e.uuid) }); if let Some(entry) = entry { if let Some(flavor) = entry.flavor.as_deref() { diff --git a/crates/socket-patch-cli/tests/in_process_vendor.rs b/crates/socket-patch-cli/tests/in_process_vendor.rs index c94c7995..69e0b85d 100644 --- a/crates/socket-patch-cli/tests/in_process_vendor.rs +++ b/crates/socket-patch-cli/tests/in_process_vendor.rs @@ -2314,6 +2314,49 @@ async fn scan_vendor_gem_artifact_rebuild_without_ledger_entry_records_none() { assert!(!recorded, "no wiring-less ledger entry invented: {env2:#}"); } +/// The same rebuild when the ledger entry belongs to ANOTHER patch +/// generation (the run that wired this uuid never saved its entry): the +/// refreshed entry cannot inherit that entry's wiring, so recording it would +/// leave `vendor --revert` unable to unwire the Gemfile. The ledger keeps +/// the other-uuid entry, wiring intact. +#[tokio::test] +async fn scan_vendor_gem_artifact_rebuild_over_other_uuid_entry_keeps_ledger() { + let mock = wiremock::MockServer::start().await; + mount_gem_patch_api(&mock, GEM_PURL).await; + let fx = gem_fixture(); + let (code, env) = run_scan_vendor(fx.root(), &mock.uri(), &[]); + assert_eq!(code, 0, "first vendor: {env:#}"); + + let mut state: Value = + serde_json::from_slice(&std::fs::read(fx.state_path()).unwrap()).unwrap(); + let other = "99999999-9999-4999-8999-999999999999"; + state["entries"][GEM_PURL]["uuid"] = Value::String(other.to_string()); + let wiring = state["entries"][GEM_PURL]["wiring"].clone(); + assert!( + wiring.as_array().is_some_and(|w| !w.is_empty()), + "fixture entry is wired: {state:#}" + ); + std::fs::write(fx.state_path(), serde_json::to_vec_pretty(&state).unwrap()).unwrap(); + std::fs::remove_file(fx.vendored_lib()).unwrap(); + + let (code, env2) = run_scan_vendor(fx.root(), &mock.uri(), &[]); + assert_eq!(code, 0, "rebuild run: {env2:#}"); + assert_eq!( + std::fs::read(fx.vendored_lib()).unwrap(), + GEM_PATCHED, + "the copy is still rebuilt" + ); + let after: Value = serde_json::from_slice(&std::fs::read(fx.state_path()).unwrap()).unwrap(); + assert_eq!( + after["entries"][GEM_PURL]["uuid"], other, + "the other-uuid entry is not replaced: {env2:#}" + ); + assert_eq!( + after["entries"][GEM_PURL]["wiring"], wiring, + "its wiring survives: {env2:#}" + ); +} + // ───────────────────────────────────────────────────────────────────── // hosted → vendored mode conversion (takeover reconciliation, pnpm v9) // ─────────────────────────────────────────────────────────────────────