From acae5db3a5c13fad759b2cbe23b037ba1da46846 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 25 Sep 2026 03:31:43 -0400 Subject: [PATCH 01/12] fix(get): order the narrowed patch selection by purl, not by HashMap bucket MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `filter_to_installed_releases` buckets every release-variant purl (PyPI `?artifact_id=`, RubyGems `?platform=`, Maven `?classifier=`) into a `HashMap` keyed by base purl and then drains it. That drain is the function's OUTPUT order, which is the order the download loop walks — so `download.patches` (and `apply.patches`, and the per-patch stderr lines) came out in `HashMap` bucket order: two identical runs of the same project emitted the same records in different orders. Sort the multi-variant bases before resolving them (stable warnings) and sort the kept selection by purl before returning it, matching how every sibling collection in the same envelope is ordered (scan's `packages`, the agent flow's `skip_records`). The `--all-releases` pass-through gets the same order so both arms of the function share one contract. Two tests: one on the narrowing itself and one on the emitted `download.patches` array. Co-Authored-By: Claude Opus 5 (1M context) --- crates/socket-patch-cli/src/commands/get.rs | 145 +++++++++++++++++++- 1 file changed, 144 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-cli/src/commands/get.rs b/crates/socket-patch-cli/src/commands/get.rs index 2e32ee27..7c8eb93c 100644 --- a/crates/socket-patch-cli/src/commands/get.rs +++ b/crates/socket-patch-cli/src/commands/get.rs @@ -1309,7 +1309,9 @@ async fn filter_to_installed_releases( ) { let mut views: HashMap = HashMap::new(); if all_releases { - return (selected.to_vec(), Vec::new(), views); + let mut kept = selected.to_vec(); + sort_by_purl(&mut kept); + return (kept, Vec::new(), views); } // Group release-variant ecosystem selections (PyPI / RubyGems / Maven) @@ -1341,8 +1343,18 @@ async fn filter_to_installed_releases( multi.push((base, variants)); } } + // `variant_groups` is a HashMap, so both drains above are in bucket + // order — which is this function's OUTPUT order, and therefore the + // order the download loop emits `download.patches` / `apply.patches` + // in. Two identical runs produced different JSON. Sort the multi- + // variant bases so their warnings and kept variants are stable, and + // sort the whole kept list by purl before returning (below and at the + // early return): every sibling collection in the same envelope — + // scan's `packages`, the agent flow's `skip_records` — is purl-sorted. + multi.sort_by(|a, b| a.0.cmp(&b.0)); if multi.is_empty() { + sort_by_purl(&mut kept); return (kept, warnings, views); } @@ -1425,9 +1437,18 @@ async fn filter_to_installed_releases( let kept_uuids: std::collections::HashSet<&str> = kept.iter().map(|s| s.uuid.as_str()).collect(); views.retain(|uuid, _| kept_uuids.contains(uuid.as_str())); + sort_by_purl(&mut kept); (kept, warnings, views) } +/// Order a patch selection the way every other collection in the JSON +/// envelope is ordered: by purl, uuid breaking a tie (a release-variant +/// base can keep several qualified purls, and `--all-releases` can keep +/// several patches for one purl). +fn sort_by_purl(patches: &mut [PatchSearchResult]) { + patches.sort_by(|a, b| a.purl.cmp(&b.purl).then_with(|| a.uuid.cmp(&b.uuid))); +} + /// Does this purl carry an exact version (`pkg:type/name@version`)? An /// exact-versioned PURL identifier is exempt from the coarse installed- /// version narrowing, like a UUID: the user named the version explicitly. @@ -6844,4 +6865,126 @@ mod tests { ); std::env::remove_var("COVGAP_GET_GUARD_PROBE"); } + + /// Release-variant narrowing must not randomize the selection order. + /// + /// `filter_to_installed_releases` buckets every release-variant purl + /// (PyPI / RubyGems / Maven) into a `HashMap` keyed by base purl and + /// then drains it, so the singleton bases — the common case — came back + /// in `HashMap` iteration order. That order is the download loop's + /// order, which is the order `download.patches` / `apply.patches` are + /// emitted in, so two identical runs produced different JSON. Every + /// sibling collection in the same envelope is purl-sorted + /// (`scan`'s `packages`, the agent flow's `skip_records`), so this one + /// must be too. + #[tokio::test] + async fn release_narrowing_keeps_a_stable_purl_order() { + let names = [ + "urllib3", + "requests", + "idna", + "certifi", + "charset-normalizer", + "jinja2", + "markupsafe", + "werkzeug", + "click", + "itsdangerous", + "blinker", + "flask", + ]; + let selected: Vec = { + let mut v: Vec = names + .iter() + .enumerate() + .map(|(i, n)| { + mk_patch( + &format!("uuid-{i}"), + &format!("pkg:pypi/{n}@1.0.0?artifact_id=wheel"), + "free", + "2026-01-01T00:00:00Z", + ) + }) + .collect(); + v.sort_by(|a, b| a.purl.cmp(&b.purl)); + v + }; + let tmp = tempfile::tempdir().expect("tempdir"); + let options = CrawlerOptions { + cwd: tmp.path().to_path_buf(), + global: false, + global_prefix: None, + }; + // No mock server is needed: every base has exactly one variant, so + // the narrowing returns before it queries the crawler or the API. + let client = test_client("http://127.0.0.1:1").await; + let (kept, _warnings, _views) = + filter_to_installed_releases(&selected, false, &options, true, &client).await; + let got: Vec<&str> = kept.iter().map(|p| p.purl.as_str()).collect(); + let want: Vec<&str> = selected.iter().map(|p| p.purl.as_str()).collect(); + assert_eq!( + got, want, + "the narrowing must preserve the caller's purl order, not the \ + HashMap's bucket order" + ); + } + + /// The vendored/agent envelope's `download.patches` array must come out + /// in the same order on every run. It is built by walking the narrowed + /// selection, so the `HashMap`-ordered narrowing above leaked straight + /// into the JSON: two identical runs of the same project emitted the + /// same records in different orders. No view is mounted — wiremock + /// answers 404, so every purl lands on the fetch-miss arm and records + /// one `patches[]` entry, which is all this pins. + #[tokio::test] + #[serial_test::serial] + async fn download_patches_json_is_purl_ordered() { + use wiremock::MockServer; + + let _env = EnvVarGuard::scrub(&["SOCKET_PROXY_URL", "SOCKET_PATCH_PROXY_URL"]); + let server = MockServer::start().await; + let tmp = tempfile::tempdir().unwrap(); + let names = [ + "urllib3", + "requests", + "idna", + "certifi", + "charset-normalizer", + "jinja2", + "markupsafe", + "werkzeug", + "click", + "itsdangerous", + "blinker", + "flask", + ]; + let mut selected: Vec = names + .iter() + .enumerate() + .map(|(i, n)| { + mk_patch( + &format!("{:08x}-aaaa-4aaa-8aaa-aaaaaaaaaaaa", i), + &format!("pkg:pypi/{n}@1.0.0?artifact_id=wheel"), + "free", + "2026-01-01T00:00:00Z", + ) + }) + .collect(); + selected.sort_by(|a, b| a.purl.cmp(&b.purl)); + + let (_code, json, _records) = + download_patch_records(&selected, &detached_params(tmp.path()), &server.uri()).await; + + let got: Vec<&str> = json["patches"] + .as_array() + .expect("patches[]") + .iter() + .map(|p| p["purl"].as_str().expect("purl")) + .collect(); + let want: Vec<&str> = selected.iter().map(|p| p.purl.as_str()).collect(); + assert_eq!( + got, want, + "download.patches must be emitted in the selection's purl order; json={json}" + ); + } } From a34bb9d01de5d4dad5842d8bed02a68b9c6df39f Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 25 Sep 2026 03:38:37 -0400 Subject: [PATCH 02/12] fix(vendor): report an unstageable patch per package, not as a dead run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The patch view serves `blobContent` only for the files a patch CHANGES. A zero-delta file — `beforeHash == afterHash` — comes back with hashes and no content, which the in-memory vendor stager counts as a failed fetch. One such patch made the WHOLE run bail `no_local_source`: exit 1, `status: error`, zero events, and every other package in the manifest left unvendored without a word. Live example: `pkg:npm/tar-fs@2.1.1`, patch `8ff3e0c7-6855-4224-924b-3e1151744ed4` — seven zero-delta fixture files plus one changed `package/index.js`. A three-package project (tar-fs, braces, minimist) downloaded all three records and then vendored none. A package whose patch content cannot be obtained is an unsatisfiable package like any other (`vendor_fetch_failed`, `redirect_revert_failed`, the Bun refusals …): it gets its own `failed` event and the run carries on. `stage_vendor_sources_in_memory` now hands those purls back in `MemStagedSources::unavailable()`; `vendor`, `scan --vendor` / `get --mode vendored` and `repair` report them one by one and run the engine over the rest. The pre-event `no_local_source` bail stays for the case it was written for — NOTHING in the manifest can be staged, so there are no events to report — which is the shape every existing test pins. The stager's own stderr summary is unchanged, so `--silent` still gets exactly one error line per arm. Repro on the live API (same project, built binaries): before, exit 1 / `status: error` / `no_local_source` / 0 events; after, exit 1 / `partial_failure` with `failed pkg:npm/tar-fs@2.1.1 no_local_source` plus `applied` braces and minimist, both in the ledger. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/commands/fetch_stage.rs | 65 +++- .../src/commands/repair_vendor.rs | 94 ++++-- .../src/commands/scan/vendor_flow.rs | 21 +- .../socket-patch-cli/src/commands/vendor.rs | 12 +- .../tests/vendor_partial_staging_e2e.rs | 318 ++++++++++++++++++ 5 files changed, 464 insertions(+), 46 deletions(-) create mode 100644 crates/socket-patch-cli/tests/vendor_partial_staging_e2e.rs diff --git a/crates/socket-patch-cli/src/commands/fetch_stage.rs b/crates/socket-patch-cli/src/commands/fetch_stage.rs index 3179bab5..e0b4cb8f 100644 --- a/crates/socket-patch-cli/src/commands/fetch_stage.rs +++ b/crates/socket-patch-cli/src/commands/fetch_stage.rs @@ -7,6 +7,7 @@ //! cache is `repair`'s job, keeping these commands read-only against //! `.socket/`). +use std::borrow::Cow; use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; @@ -22,6 +23,7 @@ use tempfile::TempDir; use super::get::base64_decode; use crate::args::GlobalArgs; use crate::commands::bun_preflight::LedgerLoad; +use crate::json_envelope::{Envelope, PatchAction, PatchEvent}; use crate::ui::{plural, StatusLine}; /// Resolved artifact locations for the patch pipeline. Holds the overlay @@ -439,6 +441,12 @@ pub(crate) struct MemStagedSources { diffs: PathBuf, packages: PathBuf, mem: HashMap>, + /// The purls this staging could NOT obtain patch content for, while at + /// least one other patch staged fine. Each is an unsatisfiable package + /// the caller reports per-package (and leaves out of the engine run) — + /// see [`stage_vendor_sources_in_memory`]. Sorted, so the per-package + /// reports come out in the same order every run. + unavailable: Vec, } impl MemStagedSources { @@ -452,6 +460,11 @@ impl MemStagedSources { mem_blobs: Some(&self.mem), } } + + /// See [`MemStagedSources::unavailable`]. + pub(crate) fn unavailable(&self) -> &[String] { + &self.unavailable + } } /// The in-memory staging outcome (mirror of [`StageOutcome`]). @@ -472,6 +485,16 @@ pub(crate) enum MemStageOutcome { /// tempdir), so this returns the outcome directly — every failure is the /// soft `Unavailable`. /// +/// A patch whose content the VIEW cannot supply (a 404, a transport error, +/// or a file the server serves with no `blobContent` — which is how it +/// serves a zero-delta file, `beforeHash == afterHash`) is an unsatisfiable +/// PACKAGE, not a broken run: its purl comes back in +/// [`MemStagedSources::unavailable`] for the caller to report per-package, +/// and the patches that did stage still run. `Unavailable` is reserved for +/// the case it was written for — NOTHING in the manifest can be staged, so +/// there are no per-package events to report and the caller's pre-event +/// `no_local_source` error is the whole story. +/// /// `ledger` is the caller's single `load_state` outcome (the harvest reads /// the committed artifacts it names; an unreadable ledger harvests /// nothing). `seed` pre-populates the in-memory blob set — the vendored @@ -498,6 +521,7 @@ pub(crate) async fn stage_vendor_sources_in_memory( let missing_blobs = get_missing_blobs(manifest, &blobs).await; let missing_package_archives = get_missing_archives(manifest, &packages).await; let mut mem = seed; + let mut unavailable: Vec = Vec::new(); // A diff archive alone is NOT a sufficient source here, unlike the disk // stager: vendoring runs the auto-force policy, where a beforeHash @@ -619,7 +643,9 @@ pub(crate) async fn stage_vendor_sources_in_memory( // the envelope (printed exclusively under --json), so muting // this under --silent meant exit 1 with zero output — the // CLI_CONTRACT violation ("errors only", NEVER nothing) fixed - // for the disk stager's arms above. + // for the disk stager's arms above. It stays the ONE human + // channel for these purls in both arms below: the per-package + // arm only records events. if !common.json { eprintln!( "Error: Could not fetch patch content for {}:", @@ -629,7 +655,15 @@ pub(crate) async fn stage_vendor_sources_in_memory( eprintln!("{line}"); } } - return MemStageOutcome::Unavailable; + // Nothing in the manifest is usable ⇒ the pre-event bail (no + // events to report). Otherwise these purls are unsatisfiable + // packages the caller reports one by one, and the rest of the + // run continues. + if failed.len() == manifest.patches.len() { + return MemStageOutcome::Unavailable; + } + unavailable = failed.into_iter().map(str::to_string).collect(); + unavailable.sort(); } } @@ -638,9 +672,36 @@ pub(crate) async fn stage_vendor_sources_in_memory( diffs, packages, mem, + unavailable, }) } +/// Record the per-package `failed` event for every purl +/// [`stage_vendor_sources_in_memory`] could not obtain patch content for, +/// and hand back the records the run can still vendor. `true` when at least +/// one purl was dropped (the run has errors). Borrows `records` untouched +/// on the overwhelmingly common empty path. +pub(crate) fn drop_unstageable<'a>( + env: &mut Envelope, + records: &'a HashMap, + unavailable: &[String], +) -> (Cow<'a, HashMap>, bool) { + if unavailable.is_empty() { + return (Cow::Borrowed(records), false); + } + for purl in unavailable { + env.record( + PatchEvent::new(PatchAction::Failed, purl.clone()).with_error( + "no_local_source", + "patch artifacts unavailable (offline or download failure)", + ), + ); + } + let mut kept = records.clone(); + kept.retain(|purl, _| !unavailable.contains(purl)); + (Cow::Owned(kept), true) +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/socket-patch-cli/src/commands/repair_vendor.rs b/crates/socket-patch-cli/src/commands/repair_vendor.rs index c880ff65..e06d215c 100644 --- a/crates/socket-patch-cli/src/commands/repair_vendor.rs +++ b/crates/socket-patch-cli/src/commands/repair_vendor.rs @@ -376,6 +376,54 @@ fn fail(env: &mut Envelope, json: bool, purl: &str, code: &str, detail: String) env.mark_partial_failure(); } +/// Report every candidate whose patch content this run could not obtain: +/// a soft one is restored without a fingerprint (counted as rebuilt), any +/// other fails with its own reason code. Shared by the two staging arms — +/// "nothing could be staged" (the whole pass ends here) and "these purls +/// could not, while others staged fine" (the pass continues without them). +/// `unrebuildable` names candidates that already failed earlier and must +/// not be reported twice. +fn report_no_local_source( + env: &mut Envelope, + common: &GlobalArgs, + candidates: &[Candidate], + unrebuildable: &HashSet, + rebuilt: &mut usize, +) { + for c in candidates { + if unrebuildable.contains(&c.purl) { + continue; + } + if c.soft { + soft_restore_without_fingerprint( + env, + common, + &c.purl, + &c.entry.artifact.path, + "its patch content has no local source to rebuild from", + ); + *rebuilt += 1; + continue; + } + fail( + env, + common.json, + &c.purl, + c.reason, + format!( + "the vendored artifact at {} is broken and its patch content has \ + no local source ({})", + c.entry.artifact.path, + if common.offline { + "--offline prevents fetching it" + } else { + "download failed" + } + ), + ); + } +} + /// `Error: Cannot repair vendored artifact for : `. fn format_repair_failure(purl: &str, detail: &str) -> String { format!( @@ -1158,41 +1206,23 @@ pub(crate) async fn repair_vendored_artifacts_with_references( { MemStageOutcome::Ready(s) => s, MemStageOutcome::Unavailable => { - for c in &candidates { - if unrebuildable.contains(&c.purl) { - continue; - } - if c.soft { - soft_restore_without_fingerprint( - env, - common, - &c.purl, - &c.entry.artifact.path, - "its patch content has no local source to rebuild from", - ); - rebuilt += 1; - continue; - } - fail( - env, - common.json, - &c.purl, - c.reason, - format!( - "the vendored artifact at {} is broken and its patch content has \ - no local source ({})", - c.entry.artifact.path, - if common.offline { - "--offline prevents fetching it" - } else { - "download failed" - } - ), - ); - } + report_no_local_source(env, common, &candidates, &unrebuildable, &mut rebuilt); return rebuilt; } }; + // Staging could obtain SOME candidates' content but not others'. The + // ones it could not get the same report the all-unavailable arm above + // gives, and leave the pass; the rest are still rebuilt. + if !staged.unavailable().is_empty() { + let (stuck, rest): (Vec, Vec) = candidates + .into_iter() + .partition(|c| staged.unavailable().contains(&c.purl)); + report_no_local_source(env, common, &stuck, &unrebuildable, &mut rebuilt); + candidates = rest; + if candidates.is_empty() { + return rebuilt; + } + } let sources = staged.as_patch_sources(); // ── Pristine package sources ───────────────────────────────────────── diff --git a/crates/socket-patch-cli/src/commands/scan/vendor_flow.rs b/crates/socket-patch-cli/src/commands/scan/vendor_flow.rs index 662bc2ee..db28b55e 100644 --- a/crates/socket-patch-cli/src/commands/scan/vendor_flow.rs +++ b/crates/socket-patch-cli/src/commands/scan/vendor_flow.rs @@ -31,7 +31,9 @@ use std::time::Duration; use crate::args::GlobalArgs; use crate::commands::bun_preflight::bun_vendor_preflight_with_ledger; -use crate::commands::fetch_stage::{stage_vendor_sources_in_memory, MemStageOutcome}; +use crate::commands::fetch_stage::{ + drop_unstageable, stage_vendor_sources_in_memory, MemStageOutcome, +}; use crate::commands::get::{download_patch_records_with, DetachedDownload, DownloadParams}; use crate::commands::lock_cli::lock_failure; use crate::commands::vendor::{ @@ -275,6 +277,11 @@ async fn stage_and_vendor( } }; let sources = staged.as_patch_sources(); + // A record whose content this run could not obtain is an unsatisfiable + // PACKAGE, reported per-package and left out of the engine run — the + // rest of the selection still vendors (the stager reserves its + // whole-run `no_local_source` bail for "nothing is stageable"). + let (records, staging_errors) = drop_unstageable(env, &manifest.patches, staged.unavailable()); // Honor `--vendor-source` (and `--vendor-url` / `--patch-server-url`) // exactly as the `vendor` command does: the SAME service-config // assembler, over the run's one client, so `scan --mode vendored` and a @@ -282,15 +289,9 @@ async fn stage_and_vendor( // service-download under `auto`) instead of scan silently building // locally. let service = common.vendor_service_config(Some(client), use_public_proxy); - Ok(boxed_vendor_records( - common, - &manifest.patches, - &sources, - Some(&service), - ledger, - env, - ) - .await) + let engine_errors = + boxed_vendor_records(common, &records, &sources, Some(&service), ledger, env).await; + Ok(staging_errors || engine_errors) } /// The ledger key addressable as `purl`: the exact key, else the entry diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index 76f43262..5e4a4773 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -40,7 +40,9 @@ use std::time::Duration; use crate::args::{apply_env_toggles, GlobalArgs}; use crate::commands::apply::{representative_file, result_to_event, variant_matches_installed}; use crate::commands::bun_preflight::bun_vendor_preflight_pairs; -use crate::commands::fetch_stage::{stage_vendor_sources_in_memory, MemStageOutcome}; +use crate::commands::fetch_stage::{ + drop_unstageable, stage_vendor_sources_in_memory, MemStageOutcome, +}; use crate::commands::lock_cli::acquire_or_emit; use crate::commands::rollback::VendorRevertStep; use crate::commands::vex::{ @@ -945,13 +947,19 @@ async fn run_vendor( } }; let sources = staged.as_patch_sources(); + // A patch whose content this run could not obtain is an unsatisfiable + // PACKAGE, reported per-package and left out of the engine run — the + // rest of the manifest still vendors (the stager reserves its + // whole-run `no_local_source` bail for "nothing is stageable"). + let (records, staging_errors) = drop_unstageable(env, &manifest.patches, staged.unavailable()); + has_errors |= staging_errors; if manifest.patches.is_empty() && !common.json && !common.silent { println!("The manifest has no patches; nothing to vendor."); } has_errors |= vendor_records( common, - &manifest.patches, + &records, &sources, false, args.force, diff --git a/crates/socket-patch-cli/tests/vendor_partial_staging_e2e.rs b/crates/socket-patch-cli/tests/vendor_partial_staging_e2e.rs new file mode 100644 index 00000000..806f58f9 --- /dev/null +++ b/crates/socket-patch-cli/tests/vendor_partial_staging_e2e.rs @@ -0,0 +1,318 @@ +//! One unstageable patch must not abort a whole vendored run. +//! +//! The patch view serves `blobContent` only for files the patch actually +//! CHANGES: a file whose `beforeHash` equals its `afterHash` comes back with +//! hashes and no content (live example: `pkg:npm/tar-fs@2.1.1`, patch +//! `8ff3e0c7-6855-4224-924b-3e1151744ed4`, seven zero-delta fixture files +//! plus one changed `package/index.js`). The in-memory vendor stager treats +//! any such view as a failed fetch, and a single failed fetch made the WHOLE +//! run bail `no_local_source` — exit 1, `status: error`, zero events, and +//! every OTHER package in the manifest left unvendored without a word. +//! +//! A package whose patch content cannot be obtained is an unsatisfiable +//! package like any other (`vendor_fetch_failed`, `redirect_revert_failed`, +//! the Bun refusals …): it gets its own `failed` event and the run carries +//! on. The pre-event `no_local_source` bail stays for the case it was +//! written for — NOTHING in the manifest can be staged, so there are no +//! events to report. +//! +//! Hermetic: the API is a `wiremock` mock, `--vendor-source build` keeps the +//! vendoring service out of the run, and every package is installed on disk +//! so no registry fetch happens. + +use std::path::Path; +use std::process::Command; + +use serde_json::{json, Value}; +use socket_patch_core::hash::git_sha256::compute_git_sha256_from_bytes; +use wiremock::matchers::{method, path as wm_path}; +use wiremock::{Mock, MockServer, ResponseTemplate}; + +const ORG: &str = "test-org"; + +/// The satisfiable package: its after-blob is staged under `.socket/blobs`, +/// so staging never fetches its view. +const GOOD_PURL: &str = "pkg:npm/left-pad@1.3.0"; +const GOOD_UUID: &str = "9f6b2c4e-1d3a-4f6b-8c2d-7e5a9b1c3d5f"; +const GOOD_ORIG: &[u8] = b"module.exports = () => 'orig';\n"; +const GOOD_PATCHED: &[u8] = b"module.exports = () => 'patched';\n"; + +/// The JS-7 package: one changed file plus one zero-delta file the view +/// serves with no `blobContent`. +const BAD_PURL: &str = "pkg:npm/tar-fs@2.1.1"; +const BAD_UUID: &str = "8ff3e0c7-6855-4224-924b-3e1151744ed4"; +const BAD_ORIG: &[u8] = b"module.exports = require('./lib');\n"; +const BAD_PATCHED: &[u8] = b"module.exports = require('./lib'); // patched\n"; +/// Zero-delta: identical `beforeHash`/`afterHash`, never served as content. +const BAD_FIXTURE: &[u8] = b""; + +fn git_hash(bytes: &[u8]) -> String { + compute_git_sha256_from_bytes(bytes) +} + +fn patch_record(uuid: &str, files: Value) -> Value { + json!({ + "uuid": uuid, + "exportedAt": "2026-01-01T00:00:00Z", + "files": files, + "vulnerabilities": {}, + "description": "synthetic vendor staging test patch", + "license": "MIT", + "tier": "free" + }) +} + +fn good_files() -> Value { + json!({ + "package/index.js": { + "beforeHash": git_hash(GOOD_ORIG), + "afterHash": git_hash(GOOD_PATCHED), + } + }) +} + +fn bad_files() -> Value { + json!({ + "package/index.js": { + "beforeHash": git_hash(BAD_ORIG), + "afterHash": git_hash(BAD_PATCHED), + }, + "package/test/fixtures/d/file1": { + "beforeHash": git_hash(BAD_FIXTURE), + "afterHash": git_hash(BAD_FIXTURE), + } + }) +} + +/// A two-package npm project: both installed, both in the v3 lock, both in +/// the manifest. Only the good package's after-blob is staged on disk. +fn fixture(root: &Path) { + for (name, version, index, extra) in [ + ("left-pad", "1.3.0", GOOD_ORIG, None), + ( + "tar-fs", + "2.1.1", + BAD_ORIG, + Some(("test/fixtures/d/file1", BAD_FIXTURE)), + ), + ] { + let pkg = root.join("node_modules").join(name); + std::fs::create_dir_all(&pkg).unwrap(); + std::fs::write( + pkg.join("package.json"), + format!(r#"{{"name":"{name}","version":"{version}"}}"#), + ) + .unwrap(); + std::fs::write(pkg.join("index.js"), index).unwrap(); + if let Some((rel, bytes)) = extra { + let p = pkg.join(rel); + std::fs::create_dir_all(p.parent().unwrap()).unwrap(); + std::fs::write(p, bytes).unwrap(); + } + } + + std::fs::write( + root.join("package.json"), + br#"{"name":"fixture","version":"1.0.0","private":true}"#, + ) + .unwrap(); + let lock = json!({ + "name": "fixture", + "version": "1.0.0", + "lockfileVersion": 3, + "requires": true, + "packages": { + "": { + "name": "fixture", + "version": "1.0.0", + "dependencies": { "left-pad": "^1.3.0", "tar-fs": "^2.1.1" } + }, + "node_modules/left-pad": { + "version": "1.3.0", + "resolved": "https://registry.npmjs.org/left-pad/-/left-pad-1.3.0.tgz", + "integrity": "sha512-orig==" + }, + "node_modules/tar-fs": { + "version": "2.1.1", + "resolved": "https://registry.npmjs.org/tar-fs/-/tar-fs-2.1.1.tgz", + "integrity": "sha512-orig2==" + } + } + }); + let mut lock_bytes = serde_json::to_vec_pretty(&lock).unwrap(); + lock_bytes.push(b'\n'); + std::fs::write(root.join("package-lock.json"), &lock_bytes).unwrap(); + + let manifest = json!({ "patches": { + GOOD_PURL: patch_record(GOOD_UUID, good_files()), + BAD_PURL: patch_record(BAD_UUID, bad_files()), + }}); + let socket = root.join(".socket"); + std::fs::create_dir_all(socket.join("blobs")).unwrap(); + let mut manifest_bytes = serde_json::to_vec_pretty(&manifest).unwrap(); + manifest_bytes.push(b'\n'); + std::fs::write(socket.join("manifest.json"), &manifest_bytes).unwrap(); + // Only the good package is locally satisfied. + std::fs::write( + socket.join("blobs").join(git_hash(GOOD_PATCHED)), + GOOD_PATCHED, + ) + .unwrap(); +} + +/// The JS-7 view: the changed file carries `blobContent`, the zero-delta +/// file carries hashes only. +async fn mount_contentless_view(server: &MockServer) { + use base64::Engine; + let b64 = base64::engine::general_purpose::STANDARD.encode(BAD_PATCHED); + Mock::given(method("GET")) + .and(wm_path(format!("/v0/orgs/{ORG}/patches/view/{BAD_UUID}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "uuid": BAD_UUID, + "purl": BAD_PURL, + "publishedAt": "2026-01-01T00:00:00Z", + "files": { + "package/index.js": { + "beforeHash": git_hash(BAD_ORIG), + "afterHash": git_hash(BAD_PATCHED), + "blobContent": b64, + }, + "package/test/fixtures/d/file1": { + "beforeHash": git_hash(BAD_FIXTURE), + "afterHash": git_hash(BAD_FIXTURE), + } + }, + "vulnerabilities": {}, + "description": "d", + "license": "MIT", + "tier": "free", + }))) + .mount(server) + .await; +} + +/// `vendor --json --vendor-source build` against the mock API, with every +/// ambient `SOCKET_*` var scrubbed from the child. +fn vendor_cli(root: &Path, api_url: &str) -> (i32, Value, String) { + let mut cmd = Command::new(env!("CARGO_BIN_EXE_socket-patch")); + cmd.args([ + "vendor", + "--json", + "--vendor-source", + "build", + "--api-url", + api_url, + "--api-token", + "fake-token", + "--org", + ORG, + ]) + .current_dir(root); + for (key, _) in std::env::vars() { + if key.starts_with("SOCKET_") && key != "SOCKET_NO_CONFIG" { + cmd.env_remove(key); + } + } + cmd.env("SOCKET_TELEMETRY_DISABLED", "1"); + let out = cmd.output().expect("spawn socket-patch"); + let stdout = String::from_utf8_lossy(&out.stdout).into_owned(); + let stderr = String::from_utf8_lossy(&out.stderr).into_owned(); + let env: Value = serde_json::from_str(stdout.trim()).unwrap_or_else(|e| { + panic!("vendor --json must emit an envelope: {e}\nstdout:\n{stdout}\nstderr:\n{stderr}") + }); + (out.status.code().unwrap_or(-1), env, stderr) +} + +fn events(env: &Value) -> &Vec { + env["events"].as_array().expect("events array") +} + +fn event_for<'a>(env: &'a Value, purl: &str) -> &'a Value { + events(env) + .iter() + .find(|e| e["purl"] == purl) + .unwrap_or_else(|| panic!("expected an event for {purl} in:\n{env:#}")) +} + +#[tokio::test] +async fn contentless_patch_view_fails_only_its_own_package() { + let server = MockServer::start().await; + mount_contentless_view(&server).await; + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + fixture(root); + + let (code, env, stderr) = vendor_cli(root, &server.uri()); + + assert_eq!( + code, 1, + "an unstageable package still fails the run: {env:#}\nstderr:\n{stderr}" + ); + assert_eq!( + env["status"], "partialFailure", + "one bad package is a partial failure, not a pre-event abort: {env:#}" + ); + assert!( + env["error"].is_null(), + "no run-level error payload: the failure is per-package: {env:#}" + ); + + let bad = event_for(&env, BAD_PURL); + assert_eq!(bad["action"], "failed", "{env:#}"); + assert_eq!( + bad["errorCode"], "no_local_source", + "the per-package failure keeps the staging code: {env:#}" + ); + + let good = event_for(&env, GOOD_PURL); + assert_eq!( + good["action"], "applied", + "the rest of the run must continue: {env:#}" + ); + assert!( + root.join(format!(".socket/vendor/npm/{GOOD_UUID}/left-pad-1.3.0.tgz")) + .is_file(), + "the satisfiable package must still be vendored: {env:#}" + ); + // The unstageable package is left completely alone. + assert!( + !root.join(format!(".socket/vendor/npm/{BAD_UUID}")).exists(), + "nothing is written for the unstageable package: {env:#}" + ); + let lock: Value = + serde_json::from_slice(&std::fs::read(root.join("package-lock.json")).unwrap()).unwrap(); + assert_eq!( + lock["packages"]["node_modules/tar-fs"]["resolved"], + "https://registry.npmjs.org/tar-fs/-/tar-fs-2.1.1.tgz", + "the unstageable package's lock entry stays registry-resolved: {env:#}" + ); +} + +/// The pre-event bail survives for the case it was written for: when NO +/// patch in the manifest can be staged there are no per-package events to +/// report, so the run keeps its top-level `no_local_source` error. +#[tokio::test] +async fn every_patch_unstageable_keeps_the_run_level_error() { + let server = MockServer::start().await; + mount_contentless_view(&server).await; + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + fixture(root); + // Drop the good package's staged blob: now both patches need a view, + // and neither view is complete (left-pad's 404s). + std::fs::remove_file(root.join(".socket/blobs").join(git_hash(GOOD_PATCHED))).unwrap(); + + let (code, env, stderr) = vendor_cli(root, &server.uri()); + + assert_eq!(code, 1, "{env:#}\nstderr:\n{stderr}"); + assert_eq!(env["status"], "error", "{env:#}"); + assert_eq!(env["error"]["code"], "no_local_source", "{env:#}"); + assert!( + events(&env).is_empty(), + "a pre-event abort reports no events: {env:#}" + ); + assert!( + !root.join(".socket/vendor").exists(), + "an aborted run vendors nothing: {env:#}" + ); +} From 11de805cb9c0fc39d8171204b27a5ee4fd221f85 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 25 Sep 2026 03:44:53 -0400 Subject: [PATCH 03/12] fix(vendor): keep an already-redirected requirements.txt line in the inventory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hosted rewriter turns `name==X` into the PEP 508 direct reference `name @ --hash=sha256:…`. The requirements.txt inventory only reads exact `==` pins, so every line a hosted run had already rewritten vanished from it — and the second hosted run over a wet requirements.txt reported `packagesWithPatches: 1` instead of 12, having "lost" the eleven packages it had just wired. uv.lock keeps its `[[package]]` name/version through the same rewrite, and Pipfile.lock's reader already keeps a Socket-written reference as the package it replaces (`socket_reference_coords`); requirements.txt now does too, through that same reader and for both shapes it writes (the hosted url and the vendored `.socket/vendor/pypi/…` path). The recovered entry is discovery-only — `resolved: None`, `integrity: None`, exactly what a `==` pin beside it yields — so the PATCHED artifact the line points at can never be fetched as a pristine source, and VEX ledger liveness keeps reading it as the "proves nothing" entry it reads a hosted uv.lock/Cargo.lock entry as. A user's own file/url reference is still not ours to resolve and stays out; a reference whose coordinates contradict the requirement's own name is skipped fail-closed. Live repro on the phase-3 `req-big` fixture (391 pins, 11 redirected): before, run 1 reported 12 packages with patches and run 2 reported 1 with 0 redirected; after, both runs report 12 / 11 with the same single pre-existing warning and a byte-identical requirements.txt. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/utils/requirements.rs | 15 +++ .../src/vendor/lock_inventory/pypi.rs | 30 +++++- .../src/vendor/lock_inventory/tests.rs | 96 +++++++++++++++++++ 3 files changed, 137 insertions(+), 4 deletions(-) diff --git a/crates/socket-patch-core/src/utils/requirements.rs b/crates/socket-patch-core/src/utils/requirements.rs index 8fc5a77c..302feaa5 100644 --- a/crates/socket-patch-core/src/utils/requirements.rs +++ b/crates/socket-patch-core/src/utils/requirements.rs @@ -113,6 +113,21 @@ pub(crate) fn exact_pin(code: &str) -> Option<(&str, &str)> { Some((name, version)) } +/// The `(name as spelled, reference)` of a PEP 508 direct reference +/// (`name[extras] @ `) — the shape the hosted redirect and the +/// vendored requirements writer rewrite an exact pin INTO. `None` for +/// anything else, [`exact_pin`]s included (a pin has no `@` before its +/// specifier). Like `exact_pin` this reads a logical line's code part and +/// stops at an optional `; marker`; the name cannot contain an `@`, so the +/// first one is always the separator and a url's own `user@host` stays +/// inside the reference. +pub(crate) fn direct_reference(code: &str) -> Option<(&str, &str)> { + let (name, rest) = code.split(';').next()?.split_once('@')?; + let name = name.split('[').next()?.trim(); + let reference = rest.split_whitespace().next()?; + (!name.is_empty() && !reference.is_empty()).then_some((name, reference)) +} + /// The `(name, version)` of a `socket-patch vendor: ==[ (transitive)]` /// comment tag (a logical line's comment part) — the tag the vendored /// requirements writer appends to its wheel-path line. diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/pypi.rs b/crates/socket-patch-core/src/vendor/lock_inventory/pypi.rs index 8bbd452a..4d5ce6a5 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/pypi.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/pypi.rs @@ -542,6 +542,16 @@ async fn inventory_pdm_lock(project_root: &Path) -> Option> { /// logical lines with the shared requirements lexer /// ([`crate::utils::requirements`]: continuations joined, comments cut, one /// leading BOM dropped), the same one the planner and discovery use. +/// +/// A line we ourselves rewrote — the hosted `name @ ` and +/// the vendored `name @ ./.socket/vendor/pypi/…` direct references — is +/// still the package it replaced, at its version: it stays in the inventory +/// so a re-scan of an already-wired project counts (and re-confirms) it +/// instead of reporting the package gone. Same rule, and the same +/// [`socket_reference_coords`] reader, as Pipfile.lock's own entries; a +/// uv.lock keeps its `[[package]]` name/version through the rewrite for +/// free. A user's OWN file/url reference is not ours to resolve and stays +/// out, exactly as before. async fn inventory_requirements_txt(project_root: &Path) -> Option> { let text = read_regular_to_string(&project_root.join("requirements.txt")) .await @@ -554,11 +564,23 @@ async fn inventory_requirements_txt(project_root: &Path) -> Option (canonicalize_pypi_name(raw_name), version.to_string()), + None => { + let Some((raw_name, reference)) = crate::utils::requirements::direct_reference(t) + .and_then(|(n, r)| Some((n, socket_reference_coords(r)?))) + else { + continue; + }; + // The line's own name must agree with the artifact's: + // a reference whose coordinates contradict the requirement + // it stands on is not one of ours, whoever wrote it. + if canonicalize_pypi_name(raw_name) != reference.0 { + continue; + } + reference + } }; - let name = canonicalize_pypi_name(raw_name); - let version = version.to_string(); let Some(purl) = pypi_purl(&name, &version) else { continue; }; diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs b/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs index 90c2047a..21bd44b8 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs @@ -2382,3 +2382,99 @@ fn pnpm_resolution_tokens_cover_maps_the_grammar_refuses() { assert!(ok.resolution.is_some()); assert_eq!(ok.resolution_tokens(), vec!["integrity:", "sha512-ok"]); } + +/// A requirements.txt the HOSTED redirect already rewrote must still +/// inventory as the package it pins. +/// +/// The rewriter turns `name==X` into the PEP 508 direct reference +/// `name @ --hash=sha256:…`, which the exact-pin rule +/// does not match — so every redirected line dropped out of the inventory +/// and a second hosted run over a wet requirements.txt reported ONE package +/// with patches instead of twelve. uv.lock keeps its `[[package]]` +/// name/version through the same rewrite; requirements.txt must too, the +/// way Pipfile.lock's own reader already keeps a Socket-written reference +/// (`socket_reference_coords`). +#[tokio::test] +async fn already_redirected_requirements_lines_stay_in_the_inventory() { + const GRANT: &str = "11111111-1111-1111-1111-111111111111"; + const PATCH: &str = "33333333-3333-3333-3333-333333333333"; + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "requirements.txt", + &format!( + "requests[security] @ https://patch.socket.dev/patch/pypi/requests/2.28.1/{GRANT}/{PATCH}/requests-2.28.1-py3-none-any.whl ; python_version >= \"3.7\" --hash=sha256:{sha}\n\ + urllib3 @ https://patch.socket.dev/patch/pypi/urllib3/1.26.18/{GRANT}/{PATCH}/urllib3-1.26.18-py2.py3-none-any.whl --hash=sha256:{sha}\n\ + flask==3.0.0\n\ + local-thing @ file:///home/me/wheels/local_thing-1.0-py3-none-any.whl\n", + sha = "c".repeat(64), + ), + ) + .await; + let entries = inventory_pypi_locks(tmp.path()).await.unwrap(); + assert_eq!( + sorted_pairs(&entries), + vec![ + ("flask".to_string(), "3.0.0".to_string()), + ("requests".to_string(), "2.28.1".to_string()), + ("urllib3".to_string(), "1.26.18".to_string()), + ], + "a user's own file reference stays out; ours come back as the \ + package they replace: {entries:?}" + ); + // Discovery-only, exactly like the `==` pins beside them: the pinned + // artifact is the PATCHED one, so it must never be fetched as pristine. + for e in &entries { + assert_eq!(e.integrity, LockIntegrity::None, "{e:?}"); + assert_eq!(e.resolved, None, "{e:?}"); + } +} + +/// The two requirements grammars — the one the hosted rewriter WRITES and +/// the one the inventory READS — must agree: rewrite a real pin and feed +/// the output straight back in. +#[tokio::test] +async fn the_hosted_rewriters_own_output_reinventories() { + use crate::patch::redirect::{rewrite_registry_redirect, DepOverride, Integrity}; + + let tmp = tempfile::tempdir().unwrap(); + let source = "requests==2.28.1 --hash=sha256:old\nflask==3.0.0\n"; + let dep = DepOverride { + ecosystem: "pypi".into(), + name: "requests".into(), + namespace: None, + version: "2.28.1".into(), + token: "11111111-1111-1111-1111-111111111111".into(), + patch_uuid: "33333333-3333-3333-3333-333333333333".into(), + artifact_url: "https://patch.socket.dev/patch/pypi/requests/2.28.1/\ + 11111111-1111-1111-1111-111111111111/\ + 33333333-3333-3333-3333-333333333333/\ + requests-2.28.1-py3-none-any.whl" + .into(), + integrity: Integrity { + sha256: Some("c".repeat(64)), + ..Default::default() + }, + berry_zip_url: None, + registry_override: None, + }; + let rewritten = rewrite_registry_redirect( + &std::collections::BTreeMap::from([("requirements.txt".to_string(), source.to_string())]), + std::slice::from_ref(&dep), + ); + let wet = rewritten + .files + .get("requirements.txt") + .expect("the rewriter must have rewritten the pin"); + write(tmp.path(), "requirements.txt", wet).await; + + let entries = inventory_pypi_locks(tmp.path()).await.unwrap(); + assert_eq!( + sorted_pairs(&entries), + vec![ + ("flask".to_string(), "3.0.0".to_string()), + ("requests".to_string(), "2.28.1".to_string()), + ], + "wet requirements.txt:\n{wet}\nentries: {entries:?}" + ); +} From 25756641d0e35700fdc0348482fd25ba39767d8f Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 25 Sep 2026 03:49:47 -0400 Subject: [PATCH 04/12] fix(vendor): refuse a lockfile-only gem in build mode before downloading it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A bundler path source will not load without the eval-able stub gemspec rubygems writes into `/specifications/` when the gem is INSTALLED, and a downloaded `.gem` carries its gemspec only as YAML in `metadata.gz` — which is precisely why the vendoring service converts it and serves a separate `gem-stub-gemspec` artifact. So a local build can never vendor a fetched gem. The auto-fetch rung downloaded the `.gem` anyway and only then hit the backend's `gem_spec_missing`: a wasted registry round trip on every `--vendor-source build` run, ending in a message whose remedy ("use --vendor-source=service") did not name the mode that actually works from here. Refuse before the fetch, for gem purls only, only when the run cannot use the patch service at all (`--vendor-source build`, or no service config), and only for the purls a fetch would actually be attempted for — the ones `fetch_pristine_package` resolves from the lockfile or recovers from the ledger. A gem that resolves from nowhere has nothing to say about gemspecs and keeps its calm `package_not_installed` skip. The refusal is the same `gem_spec_missing` code and the same `failed` event, with a detail that says why a fetched gem is unusable and points at `bundle install` or `--vendor-source=auto`. `auto` and `service` still fetch: the service path needs the staged pristine copy, and that is the mode that CAN vendor this gem. The backend keeps its own refusal as the backstop for every other route into it (including the security case where a staging dir must never yield a stub). Repro: a lockfile-only gem against a mock rubygems host — before, the `.gem` was downloaded and the run then failed `gem_spec_missing`; after, the host sees no request at all and the same failure arrives with the real remedy. Two guard twins pin the scope: `auto` still downloads, and a gem no lockfile resolves still reports `package_not_installed`. Co-Authored-By: Claude Opus 5 (1M context) --- .../socket-patch-cli/src/commands/vendor.rs | 40 +++ .../tests/vendor_gem_lockfile_only_e2e.rs | 275 ++++++++++++++++++ 2 files changed, 315 insertions(+) create mode 100644 crates/socket-patch-cli/tests/vendor_gem_lockfile_only_e2e.rs diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index 5e4a4773..339d2e92 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -1363,6 +1363,46 @@ pub(crate) async fn vendor_records( inventory = Some(lock_inventory::inventory_project(&common.cwd).await); } let inv = inventory.as_deref().expect("filled just above"); + // A NOT-INSTALLED gem can only be vendored through the patch + // service. The bundler path source the gem backend wires + // needs the eval-able stub gemspec rubygems writes into + // `/specifications/` at INSTALL time; a fetched + // `.gem` carries its gemspec only as YAML in `metadata.gz`, + // which is exactly why the service serves a converted + // `gem-stub-gemspec` second artifact. With the service off + // (`--vendor-source build`, or no config at all) the fetched + // copy is unusable, so the backend refused `gem_spec_missing` + // — AFTER paying for the download, on every run. Refuse here + // instead, with the same code and a detail that names the + // real remedy. The backend keeps its own refusal as the + // backstop for every other route into it. + // + // Scoped to the purls a fetch would actually be attempted + // for (what `fetch_pristine_package` resolves from the + // lockfile or the ledger): a gem that resolves from nowhere + // has nothing to say about gemspecs and keeps the calm + // `package_not_installed` skip below. + if purl.starts_with("pkg:gem/") + && !service.is_some_and(VendorServiceConfig::service_enabled) + && (lock_inventory::lookup(inv, purl).is_some() || ledger_entry.is_some()) + { + fetch_failed.insert(purl.clone()); + let detail = format!( + "{} is not installed, and a local build cannot vendor a fetched \ + gem: the bundler path source needs the stub gemspec rubygems \ + writes into specifications/ when the gem is installed, which a \ + downloaded .gem does not carry. Install the gem (e.g. \ + `bundle install`) and re-run, or use --vendor-source=auto to \ + vendor it from the patch service.", + normalize_purl(purl) + ); + env.record( + PatchEvent::new(PatchAction::Failed, purl.clone()) + .with_error("gem_spec_missing", detail.clone()), + ); + report_vendor_failure(common, purl, &detail); + continue; + } match fetch_pristine_package(&common.cwd, inv, &client, purl, ledger_entry).await { PristineFetch::Fetched(fetched) => { record_warning( diff --git a/crates/socket-patch-cli/tests/vendor_gem_lockfile_only_e2e.rs b/crates/socket-patch-cli/tests/vendor_gem_lockfile_only_e2e.rs new file mode 100644 index 00000000..b0c8b2d0 --- /dev/null +++ b/crates/socket-patch-cli/tests/vendor_gem_lockfile_only_e2e.rs @@ -0,0 +1,275 @@ +//! `vendor --vendor-source build` on a gem the project only has in its +//! lockfile. +//! +//! The local gem build needs the eval-able stub gemspec rubygems writes +//! into `/specifications/` when the gem is INSTALLED — a bundler +//! path source will not load without one, and a downloaded `.gem` carries +//! its gemspec only as YAML in `metadata.gz` (the vendoring service's +//! converter is what turns that into the Ruby form, and serves it as the +//! `gem-stub-gemspec` second artifact). So build mode cannot vendor a +//! fetched gem, ever — yet the auto-fetch rung downloaded the `.gem` from +//! the registry first and only then hit the backend's `gem_spec_missing` +//! refusal. The download is pure waste on every run. +//! +//! The refusal now happens BEFORE the fetch, with a message that says why +//! and what to do. `auto` (and `service`) still fetch: the service path +//! needs the staged dir, and that is the mode that CAN vendor this gem. +//! +//! Hermetic: a `wiremock` stand-in for the rubygems download host, named by +//! the lock's `remote:`, and a `.socket/blobs` entry so patch staging never +//! reaches the API. + +use std::path::{Path, PathBuf}; +use std::process::Command; + +use sha2::{Digest, Sha256}; +use wiremock::matchers::{method, path as wm_path}; +use wiremock::{Mock, MockServer, ResponseTemplate}; + +/// A name nothing on the developer's machine can have installed, so the +/// crawler always reports the package missing. +const NAME: &str = "socketfixturegem"; +const VERSION: &str = "1.0.0"; +const PURL: &str = "pkg:gem/socketfixturegem@1.0.0"; +const UUID: &str = "11111111-1111-4111-8111-111111111111"; +const LIB: &str = "lib/socketfixturegem.rb"; +const PRISTINE: &[u8] = b"module SocketFixtureGem; VERSION = '1.0.0'; end\n"; +const PATCHED: &[u8] = b"module SocketFixtureGem; VERSION = '1.0.0'; SAFE = true; end\n"; + +fn binary() -> PathBuf { + env!("CARGO_BIN_EXE_socket-patch").into() +} + +fn git_sha256(content: &[u8]) -> String { + let mut hasher = Sha256::new(); + hasher.update(format!("blob {}\0", content.len()).as_bytes()); + hasher.update(content); + hex::encode(hasher.finalize()) +} + +/// A minimal `.gem`: an uncompressed tar whose only entry the fetcher reads +/// is `data.tar.gz`, itself a gzipped tar of the gem's files at the root. +fn make_gem() -> Vec { + let mut data = tar::Builder::new(Vec::new()); + let mut header = tar::Header::new_gnu(); + header.set_size(PRISTINE.len() as u64); + header.set_mode(0o644); + header.set_cksum(); + data.append_data(&mut header, LIB, PRISTINE).unwrap(); + let data_tar = data.into_inner().unwrap(); + let mut gz = flate2::write::GzEncoder::new(Vec::new(), flate2::Compression::default()); + std::io::Write::write_all(&mut gz, &data_tar).unwrap(); + let data_tar_gz = gz.finish().unwrap(); + + let mut gem = tar::Builder::new(Vec::new()); + let mut header = tar::Header::new_gnu(); + header.set_size(data_tar_gz.len() as u64); + header.set_mode(0o644); + header.set_cksum(); + gem.append_data(&mut header, "data.tar.gz", data_tar_gz.as_slice()) + .unwrap(); + gem.into_inner().unwrap() +} + +/// Gemfile + a bundler >= 2.6 Gemfile.lock resolving the gem against +/// `remote` with a CHECKSUMS pin (the fetch layer refuses an entry with no +/// verifier, so without this nothing would ever be downloaded), plus the +/// manifest and staged after-blob. +fn write_fixture(root: &Path, remote: &str, gem_sha256: &str) { + std::fs::write( + root.join("Gemfile"), + format!("source \"{remote}\"\ngem \"{NAME}\"\n"), + ) + .unwrap(); + std::fs::write( + root.join("Gemfile.lock"), + format!( + "GEM\n remote: {remote}\n specs:\n {NAME} ({VERSION})\n\n\ + PLATFORMS\n ruby\n\n\ + DEPENDENCIES\n {NAME}\n\n\ + CHECKSUMS\n {NAME} ({VERSION}) sha256={gem_sha256}\n\n\ + BUNDLED WITH\n 2.6.2\n" + ), + ) + .unwrap(); + + let socket = root.join(".socket"); + std::fs::create_dir_all(socket.join("blobs")).unwrap(); + let manifest = serde_json::json!({ + "patches": { + PURL: { + "uuid": UUID, + "exportedAt": "2026-01-01T00:00:00Z", + "files": { + LIB: { + "beforeHash": git_sha256(PRISTINE), + "afterHash": git_sha256(PATCHED), + } + }, + "vulnerabilities": {}, + "description": "synthetic gem vendor test patch", + "license": "MIT", + "tier": "free" + } + } + }); + std::fs::write( + socket.join("manifest.json"), + serde_json::to_vec_pretty(&manifest).unwrap(), + ) + .unwrap(); + std::fs::write(socket.join("blobs").join(git_sha256(PATCHED)), PATCHED).unwrap(); +} + +async fn mount_gem_download(mock: &MockServer, gem: Vec) { + Mock::given(method("GET")) + .and(wm_path(format!("/downloads/{NAME}-{VERSION}.gem"))) + .respond_with(ResponseTemplate::new(200).set_body_bytes(gem)) + .mount(mock) + .await; +} + +/// `vendor --json --vendor-source ` through the built binary, with +/// every ambient `SOCKET_*` var scrubbed and the API pointed at a +/// guaranteed-dead endpoint (patch staging is satisfied from +/// `.socket/blobs`, so nothing should reach it). +fn run_vendor(root: &Path, source: &str, api_url: &str) -> (i32, serde_json::Value, String) { + let mut cmd = Command::new(binary()); + cmd.args([ + "vendor", + "--json", + "--vendor-source", + source, + "--api-url", + api_url, + "--proxy-url", + api_url, + "--api-token", + "fake-token", + "--org", + "test-org", + ]) + .current_dir(root); + for (key, _) in std::env::vars() { + if key.starts_with("SOCKET_") && key != "SOCKET_NO_CONFIG" { + cmd.env_remove(key); + } + } + cmd.env("SOCKET_TELEMETRY_DISABLED", "1"); + let out = cmd.output().expect("run socket-patch vendor"); + let stdout = String::from_utf8_lossy(&out.stdout).into_owned(); + let stderr = String::from_utf8_lossy(&out.stderr).into_owned(); + let v: serde_json::Value = serde_json::from_str(stdout.trim()) + .unwrap_or_else(|e| panic!("vendor --json must emit JSON: {e}\n{stdout}\n{stderr}")); + (out.status.code().unwrap_or(-1), v, stderr) +} + +/// A guaranteed-unreachable local endpoint: bind an ephemeral port, then +/// release it, so every request fails fast with connection-refused. +fn dead_endpoint() -> String { + let port = std::net::TcpListener::bind("127.0.0.1:0") + .unwrap() + .local_addr() + .unwrap() + .port(); + format!("http://127.0.0.1:{port}") +} + +fn failed_event(v: &serde_json::Value) -> &serde_json::Value { + v["events"] + .as_array() + .expect("events array") + .iter() + .find(|e| e["action"] == "failed") + .unwrap_or_else(|| panic!("expected a failed event in:\n{v:#}")) +} + +#[tokio::test] +async fn build_mode_refuses_a_lockfile_only_gem_before_downloading_it() { + let mock = MockServer::start().await; + let gem = make_gem(); + let sha = hex::encode(Sha256::digest(&gem)); + mount_gem_download(&mock, gem).await; + + let tmp = tempfile::tempdir().unwrap(); + write_fixture(tmp.path(), &mock.uri(), &sha); + + let (code, v, stderr) = run_vendor(tmp.path(), "build", &dead_endpoint()); + + assert_eq!(code, 1, "the refusal fails the run: {v:#}\n{stderr}"); + assert!( + mock.received_requests() + .await + .unwrap_or_default() + .is_empty(), + "build mode cannot use a fetched gem, so it must not download one" + ); + let failed = failed_event(&v); + assert_eq!(failed["purl"], PURL, "{v:#}"); + assert_eq!( + failed["errorCode"], "gem_spec_missing", + "the backend's own refusal code, raised earlier: {v:#}" + ); + let detail = failed["error"].as_str().unwrap_or_default(); + assert!( + detail.contains("not installed") && detail.contains("--vendor-source"), + "the refusal must say why and name the remedy: {detail}" + ); + assert!( + !tmp.path().join(".socket/vendor").exists(), + "nothing is written: {v:#}" + ); + let lock = std::fs::read_to_string(tmp.path().join("Gemfile.lock")).unwrap(); + assert!(lock.contains("GEM\n"), "the lock is untouched: {lock}"); +} + +/// The gate is scoped to build-only runs: `auto` may still vendor this gem +/// through the patch service, and the service path needs the fetched copy +/// staged, so the download must still happen there. +#[tokio::test] +async fn auto_mode_still_fetches_a_lockfile_only_gem() { + let mock = MockServer::start().await; + let gem = make_gem(); + let sha = hex::encode(Sha256::digest(&gem)); + mount_gem_download(&mock, gem).await; + + let tmp = tempfile::tempdir().unwrap(); + write_fixture(tmp.path(), &mock.uri(), &sha); + + // The patch service is unreachable, so `auto` falls back to the local + // build and lands on the same refusal — AFTER the fetch, which is the + // behavior this mode needs. + let (code, v, stderr) = run_vendor(tmp.path(), "auto", &dead_endpoint()); + + assert_eq!(code, 1, "{v:#}\n{stderr}"); + assert_eq!(failed_event(&v)["purl"], PURL, "{v:#}"); + assert_eq!( + mock.received_requests().await.unwrap_or_default().len(), + 1, + "auto must still stage the pristine gem for the service path" + ); +} + +/// The gate is also scoped to gems a fetch would actually be attempted for. +/// A gem that no lockfile resolves and no ledger entry recovers has nothing +/// to fetch and nothing to say about gemspecs: it keeps the calm +/// `package_not_installed` skip, not a gemspec refusal. +#[tokio::test] +async fn a_gem_that_resolves_from_nowhere_still_reports_not_installed() { + let tmp = tempfile::tempdir().unwrap(); + write_fixture(tmp.path(), "https://rubygems.org", &"0".repeat(64)); + // No lockfile at all: nothing resolves the gem. + std::fs::remove_file(tmp.path().join("Gemfile.lock")).unwrap(); + + let (code, v, stderr) = run_vendor(tmp.path(), "build", &dead_endpoint()); + + assert_eq!(code, 1, "{v:#}\n{stderr}"); + let event = v["events"] + .as_array() + .expect("events array") + .iter() + .find(|e| e["purl"] == PURL) + .unwrap_or_else(|| panic!("expected an event for {PURL} in:\n{v:#}")); + assert_eq!(event["action"], "skipped", "{v:#}"); + assert_eq!(event["errorCode"], "package_not_installed", "{v:#}"); +} From 66a6e4b94c00cfd72b9c14a3b59d9e96aee80719 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 25 Sep 2026 05:00:24 -0400 Subject: [PATCH 05/12] fix(vendor): scope the gem build-mode refusal to gems a fetch would download MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pre-fetch `gem_spec_missing` gate keyed off "the lockfile resolves this purl, or the ledger knows it" — mere RESOLVABILITY. The download it exists to save needs VERIFIABILITY, and the already-vendored case needs no local build at all, so the gate fired in two cases where `main` never fetched anything and never failed: * A bundler < 2.6 `Gemfile.lock` (no `CHECKSUMS` section — the majority of real locks) resolves the gem but records no verifier. `registry_fetch::fetch_and_stage` refuses a `LockIntegrity::None` entry before any network I/O, so CLI_CONTRACT's documented pair fires instead (`vendor_fetch_unverifiable` warning + the calm `package_not_installed` skip). The gate replaced that with `failed`/`gem_spec_missing` — for a download that never existed, and with a remedy that cannot work: the same fixture under `--vendor-source auto` still yields the skip pair, because the purl never reaches the gem backend in any mode. * An already-vendored gem on a fresh clone (committed `.socket/vendor/gem/` copy + wired lock, `bundle install` not yet run) has a ledger entry, which is exactly the case `fetch_pristine_package`'s ledger-recovery rung exists for ("an already-vendored lock-only checkout re-scans green"). The recovered fetch feeds the gem backend's idempotent hot path, which re-confirms the wired lock and returns `already_vendored` without ever needing a stub gemspec. Measured on `main`: exit 0, `status: success`, events `[skipped/vendor_fetched_missing, skipped/already_vendored]`. With the gate: exit 1, `partialFailure`, `failed`/`gem_spec_missing` — a green idempotent re-run turned into a failure, with nothing wrong with the project. `scan --vendor` / `get --mode vendored` under `--vendor-source build` broke the same way. Mirror `fetch_pristine_package`'s own `fetchable` filter instead: an inventory entry whose integrity is not `LockIntegrity::None`, and no ledger entry. GEM-4's real case — a not-installed gem a bundler >= 2.6 lock CAN verify — still refuses before the download, unchanged. Two regression tests, both green on `origin/main` and red on the gate as written: the unverifiable lock keeps its documented skip pair with zero registry requests, and a vendor-then-`rm -rf vendor/` re-run stays exit 0 with `already_vendored`. Co-Authored-By: Claude Opus 5 (1M context) --- .../socket-patch-cli/src/commands/vendor.rs | 28 ++- .../tests/vendor_gem_lockfile_only_e2e.rs | 159 ++++++++++++++++++ 2 files changed, 181 insertions(+), 6 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index 339d2e92..0bd9f10b 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -1377,14 +1377,30 @@ pub(crate) async fn vendor_records( // real remedy. The backend keeps its own refusal as the // backstop for every other route into it. // - // Scoped to the purls a fetch would actually be attempted - // for (what `fetch_pristine_package` resolves from the - // lockfile or the ledger): a gem that resolves from nowhere - // has nothing to say about gemspecs and keeps the calm - // `package_not_installed` skip below. + // Scoped to the purls a DOWNLOAD would actually happen for, + // since a wasted download is the whole point — mirroring + // `fetch_pristine_package`'s own `fetchable` filter: + // + // * a gem the lock cannot VERIFY (no `CHECKSUMS` section — + // every bundler < 2.6 lock) is never fetched at all + // (`registry_fetch::fetch_and_stage` refuses a + // `LockIntegrity::None` entry before any network I/O), so + // it keeps its documented `vendor_fetch_unverifiable` + // warning + calm `package_not_installed` skip — all the + // more so because the remedy below cannot help it: the + // purl never reaches the gem backend in ANY mode. + // * a gem the ledger already holds is the already-vendored + // fresh-clone case the ladder exists for: its committed + // copy is re-confirmed by the backend's idempotent hot + // path, which needs no stub gemspec of its own, and the + // run is green. Never refuse it. + // * a gem that resolves from nowhere has nothing to say + // about gemspecs and keeps the calm skip below. if purl.starts_with("pkg:gem/") && !service.is_some_and(VendorServiceConfig::service_enabled) - && (lock_inventory::lookup(inv, purl).is_some() || ledger_entry.is_some()) + && ledger_entry.is_none() + && lock_inventory::lookup(inv, purl) + .is_some_and(|e| e.integrity != lock_inventory::LockIntegrity::None) { fetch_failed.insert(purl.clone()); let detail = format!( diff --git a/crates/socket-patch-cli/tests/vendor_gem_lockfile_only_e2e.rs b/crates/socket-patch-cli/tests/vendor_gem_lockfile_only_e2e.rs index b0c8b2d0..898e6fac 100644 --- a/crates/socket-patch-cli/tests/vendor_gem_lockfile_only_e2e.rs +++ b/crates/socket-patch-cli/tests/vendor_gem_lockfile_only_e2e.rs @@ -121,6 +121,31 @@ fn write_fixture(root: &Path, remote: &str, gem_sha256: &str) { std::fs::write(socket.join("blobs").join(git_sha256(PATCHED)), PATCHED).unwrap(); } +/// Install the gem the way a `bundle install --path vendor/bundle` +/// deployment does: the unpacked gem under `vendor/bundle/gems//` and +/// the eval-able stub rubygems writes beside it in +/// `vendor/bundle/specifications/.gemspec` (with the `summary` + +/// `authors` rubygems requires, which the local-build write choke point +/// re-validates). +fn install_gem(root: &Path) { + let leaf = format!("{NAME}-{VERSION}"); + let bundle = root.join("vendor").join("bundle"); + let gem_dir = bundle.join("gems").join(&leaf); + std::fs::create_dir_all(gem_dir.join("lib")).unwrap(); + std::fs::write(gem_dir.join(LIB), PRISTINE).unwrap(); + let specs = bundle.join("specifications"); + std::fs::create_dir_all(&specs).unwrap(); + std::fs::write( + specs.join(format!("{leaf}.gemspec")), + format!( + "Gem::Specification.new do |s|\n s.name = \"{NAME}\"\n \ + s.version = \"{VERSION}\"\n s.summary = \"a synthetic fixture gem\"\n \ + s.authors = [\"Socket\"]\n s.require_paths = [\"lib\"]\nend\n" + ), + ) + .unwrap(); +} + async fn mount_gem_download(mock: &MockServer, gem: Vec) { Mock::given(method("GET")) .and(wm_path(format!("/downloads/{NAME}-{VERSION}.gem"))) @@ -273,3 +298,137 @@ async fn a_gem_that_resolves_from_nowhere_still_reports_not_installed() { assert_eq!(event["action"], "skipped", "{v:#}"); assert_eq!(event["errorCode"], "package_not_installed", "{v:#}"); } + +// ── scope guards ──────────────────────────────────────────────────────── +// +// The refusal must fire ONLY where the wasted download it replaces would +// really have happened: a gem the lock resolves WITH a verifier, and that +// the run is not already vendoring from its committed artifact. Two cases +// where a fetch never happens on `main` must keep `main`'s outcome. + +/// A bundler < 2.6 `Gemfile.lock` (no `CHECKSUMS` section — the majority of +/// real locks) resolves the gem but cannot VERIFY it, and +/// `registry_fetch::fetch_and_stage` refuses such an entry before any +/// network I/O. CLI_CONTRACT: "Entries the lock cannot verify are NEVER +/// fetched (`vendor_fetch_unverifiable` warning + the calm +/// `package_not_installed` skip)". There is no download to save here, so +/// the gemspec refusal must not replace that documented pair — all the more +/// so because its remedy (`--vendor-source=auto`) cannot work either: the +/// purl never reaches the gem backend in any mode. +#[tokio::test] +async fn an_unverifiable_lock_entry_keeps_the_documented_skip_pair() { + let mock = MockServer::start().await; + mount_gem_download(&mock, make_gem()).await; + + let tmp = tempfile::tempdir().unwrap(); + write_fixture(tmp.path(), &mock.uri(), &"0".repeat(64)); + // Re-write the lock the way bundler < 2.6 does: no CHECKSUMS section. + std::fs::write( + tmp.path().join("Gemfile.lock"), + format!( + "GEM\n remote: {}\n specs:\n {NAME} ({VERSION})\n\n\ + PLATFORMS\n ruby\n\n\ + DEPENDENCIES\n {NAME}\n\n\ + BUNDLED WITH\n 2.4.10\n", + mock.uri() + ), + ) + .unwrap(); + + let (code, v, stderr) = run_vendor(tmp.path(), "build", &dead_endpoint()); + + assert_eq!(code, 1, "{v:#}\n{stderr}"); + assert!( + mock.received_requests() + .await + .unwrap_or_default() + .is_empty(), + "an unverifiable entry is never fetched: {v:#}" + ); + let codes: Vec<(&str, &str)> = v["events"] + .as_array() + .expect("events array") + .iter() + .filter(|e| e["purl"] == PURL) + .map(|e| { + ( + e["action"].as_str().unwrap_or_default(), + e["errorCode"].as_str().unwrap_or_default(), + ) + }) + .collect(); + assert_eq!( + codes, + vec![ + ("skipped", "vendor_fetch_unverifiable"), + ("skipped", "package_not_installed"), + ], + "an unverifiable lock entry keeps its documented warning + calm \ + skip, not a gemspec refusal: {v:#}" + ); +} + +/// An ALREADY-VENDORED gem on a fresh clone (the committed +/// `.socket/vendor/gem/` copy is the dependency; no installed gem, +/// because `bundle install` has not run yet) must re-scan green in build +/// mode: the gem backend's idempotent hot path re-confirms the wired lock +/// and returns `already_vendored` without ever needing a stub gemspec of +/// its own. `fetch_pristine_package` exists precisely for this case — its +/// ledger-recovery rung is commented "an already-vendored lock-only +/// checkout re-scans green". +#[tokio::test] +async fn an_already_vendored_gem_re_runs_green_on_a_fresh_clone() { + let mock = MockServer::start().await; + let gem = make_gem(); + let sha = hex::encode(Sha256::digest(&gem)); + mount_gem_download(&mock, gem).await; + + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + write_fixture(root, &mock.uri(), &sha); + install_gem(root); + + // Run 1: the gem is installed, so the local build vendors it. + let (code, v, stderr) = run_vendor(root, "build", &dead_endpoint()); + assert_eq!( + code, 0, + "run 1 must vendor the installed gem: {v:#}\n{stderr}" + ); + assert!( + root.join(format!(".socket/vendor/gem/{UUID}")).is_dir(), + "run 1 must commit the vendored copy: {v:#}" + ); + + // Fresh clone: the committed artifact and the wired lock are checked + // in, the installed gem is not. + std::fs::remove_dir_all(root.join("vendor")).unwrap(); + + let (code, v, stderr) = run_vendor(root, "build", &dead_endpoint()); + + assert_eq!( + code, 0, + "an in-sync re-run of an already-vendored gem is green: {v:#}\n{stderr}" + ); + assert_eq!(v["status"], "success", "{v:#}\n{stderr}"); + let codes: Vec<(&str, &str)> = v["events"] + .as_array() + .expect("events array") + .iter() + .filter(|e| e["purl"] == PURL) + .map(|e| { + ( + e["action"].as_str().unwrap_or_default(), + e["errorCode"].as_str().unwrap_or_default(), + ) + }) + .collect(); + assert_eq!( + codes, + vec![ + ("skipped", "vendor_fetched_missing"), + ("skipped", "already_vendored"), + ], + "the ledger-recovered fetch re-confirms the committed copy and the \ + hot path reports it in sync: {v:#}" + ); +} From 45425953bc5f121040a360294097cce952a7ec58 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 25 Sep 2026 05:04:25 -0400 Subject: [PATCH 06/12] fix(vendor): keep an already-vendored requirements.txt line in the inventory MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The hosted half of this fix landed in 11de805c; the vendored half did not, although both docstrings claimed it. The hosted rewriter emits a PEP 508 direct reference (`name @ `), but the VENDORED requirements writer emits something else entirely — a bare path line, `./[ ; marker] --hash=sha256: # socket-patch vendor: ==` (`vendor::pypi_requirements::vendor_line`) — with no `name @` at all. `direct_reference` splits on `@` and so never matched it, and `inventory_requirements_txt` discarded the comment part where the `socket-patch vendor:` tag lives, so a vendored requirements.txt kept the exact symptom the hosted arm fixed: its packages drop out of `lock_inventory`, which is what `scan/discovery.rs::lockfile_supplement` counts, so a re-scan of an already-vendored lockfile-only checkout under-reports them. The `.socket/vendor/pypi/` arm of `socket_reference_coords` was only ever reachable from Pipfile.lock. Read the vendored shape too: keep the logical line's comment, and when the code part is a bare path `socket_reference_coords` recognizes, take the requirement name from the `socket-patch vendor:` tag — the same `utils::requirements::vendor_tag` reader `vex::discover::pypi_other` already uses — and cross-check it against the path's own coordinates, the same fail-closed rule the hosted arm applies to its url. The recovered entry stays discovery-only (`resolved: None`, `integrity: None`), so the PATCHED wheel it points at can never be fetched as a pristine source. A user's own wheel path is still not ours to resolve and stays out. Two tests, both red before: the vendored shape (with and without an env marker, beside a `==` pin and a user's own wheel path), and a round trip through the writer's own `vendor_line` formatter — the twin of the hosted `the_hosted_rewriters_own_output_reinventories` guard. Both docstrings now describe the two shapes they actually read. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/utils/requirements.rs | 15 ++-- .../src/vendor/lock_inventory/pypi.rs | 38 +++++++--- .../src/vendor/lock_inventory/tests.rs | 70 +++++++++++++++++++ crates/socket-patch-core/src/vendor/mod.rs | 2 +- .../src/vendor/pypi_requirements.rs | 6 +- 5 files changed, 113 insertions(+), 18 deletions(-) diff --git a/crates/socket-patch-core/src/utils/requirements.rs b/crates/socket-patch-core/src/utils/requirements.rs index 302feaa5..421cb6ef 100644 --- a/crates/socket-patch-core/src/utils/requirements.rs +++ b/crates/socket-patch-core/src/utils/requirements.rs @@ -114,13 +114,14 @@ pub(crate) fn exact_pin(code: &str) -> Option<(&str, &str)> { } /// The `(name as spelled, reference)` of a PEP 508 direct reference -/// (`name[extras] @ `) — the shape the hosted redirect and the -/// vendored requirements writer rewrite an exact pin INTO. `None` for -/// anything else, [`exact_pin`]s included (a pin has no `@` before its -/// specifier). Like `exact_pin` this reads a logical line's code part and -/// stops at an optional `; marker`; the name cannot contain an `@`, so the -/// first one is always the separator and a url's own `user@host` stays -/// inside the reference. +/// (`name[extras] @ `) — the shape the HOSTED redirect +/// rewrites an exact pin INTO. The VENDORED requirements writer emits a +/// bare path line tagged with [`vendor_tag`] instead, which this does NOT +/// match: it has no `name @`. `None` for anything else, [`exact_pin`]s +/// included (a pin has no `@` before its specifier). Like `exact_pin` this +/// reads a logical line's code part and stops at an optional `; marker`; +/// the name cannot contain an `@`, so the first one is always the separator +/// and a url's own `user@host` stays inside the reference. pub(crate) fn direct_reference(code: &str) -> Option<(&str, &str)> { let (name, rest) = code.split(';').next()?.split_once('@')?; let name = name.split('[').next()?.trim(); diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/pypi.rs b/crates/socket-patch-core/src/vendor/lock_inventory/pypi.rs index 4d5ce6a5..1c3f2bb7 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/pypi.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/pypi.rs @@ -543,14 +543,25 @@ async fn inventory_pdm_lock(project_root: &Path) -> Option> { /// ([`crate::utils::requirements`]: continuations joined, comments cut, one /// leading BOM dropped), the same one the planner and discovery use. /// -/// A line we ourselves rewrote — the hosted `name @ ` and -/// the vendored `name @ ./.socket/vendor/pypi/…` direct references — is -/// still the package it replaced, at its version: it stays in the inventory -/// so a re-scan of an already-wired project counts (and re-confirms) it -/// instead of reporting the package gone. Same rule, and the same -/// [`socket_reference_coords`] reader, as Pipfile.lock's own entries; a -/// uv.lock keeps its `[[package]]` name/version through the rewrite for -/// free. A user's OWN file/url reference is not ours to resolve and stays +/// A line we ourselves rewrote is still the package it replaced, at its +/// version: it stays in the inventory so a re-scan of an already-wired +/// project counts (and re-confirms) it instead of reporting the package +/// gone. Same rule, and the same [`socket_reference_coords`] reader, as +/// Pipfile.lock's own entries; a uv.lock keeps its `[[package]]` +/// name/version through the rewrite for free. The two writers spell their +/// line differently, so both shapes are read: +/// +/// * hosted redirect — the PEP 508 direct reference +/// `name @ ` (`utils::requirements::direct_reference`); +/// * vendored requirements — a BARE path line, +/// `./.socket/vendor/pypi// --hash=sha256:… +/// # socket-patch vendor: ==` +/// (`vendor::pypi_requirements::vendor_line`), whose requirement name +/// lives ONLY in that comment tag +/// (`utils::requirements::vendor_tag`, the reader +/// `vex::discover::pypi_other` already uses). +/// +/// A user's OWN file/url/path reference is not ours to resolve and stays /// out, exactly as before. async fn inventory_requirements_txt(project_root: &Path) -> Option> { let text = read_regular_to_string(&project_root.join("requirements.txt")) @@ -558,7 +569,8 @@ async fn inventory_requirements_txt(project_root: &Path) -> Option Option { let Some((raw_name, reference)) = crate::utils::requirements::direct_reference(t) .and_then(|(n, r)| Some((n, socket_reference_coords(r)?))) + .or_else(|| { + // The VENDORED writer's shape: a bare path line + // whose requirement name lives only in the + // `socket-patch vendor:` comment tag it appends. + let coords = socket_reference_coords(t.split_whitespace().next()?)?; + let (tag_name, _) = crate::utils::requirements::vendor_tag(comment?)?; + Some((tag_name, coords)) + }) else { continue; }; diff --git a/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs b/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs index 21bd44b8..53253b2f 100644 --- a/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs +++ b/crates/socket-patch-core/src/vendor/lock_inventory/tests.rs @@ -2478,3 +2478,73 @@ async fn the_hosted_rewriters_own_output_reinventories() { "wet requirements.txt:\n{wet}\nentries: {entries:?}" ); } + +/// The VENDORED requirements.txt shape must inventory too. +/// +/// `already_redirected_requirements_lines_stay_in_the_inventory` covers the +/// hosted `name @ ` half. The vendored writer emits something else +/// entirely — a BARE path line, +/// `./[ ; marker] --hash=sha256: # socket-patch vendor: +/// ==` (`vendor::pypi_requirements::vendor_line`) — with no +/// `name @` at all, so the direct-reference reader never sees it and the +/// package drops out of the inventory exactly the way the hosted lines did. +/// The `socket-patch vendor:` comment tag is the name/version the writer +/// left for its readers; cross-check it against the path's own coordinates. +#[tokio::test] +async fn already_vendored_requirements_lines_stay_in_the_inventory() { + const UUID: &str = "33333333-3333-3333-3333-333333333333"; + let tmp = tempfile::tempdir().unwrap(); + write( + tmp.path(), + "requirements.txt", + &format!( + "./.socket/vendor/pypi/{UUID}/requests-2.28.1-py3-none-any.whl \ + --hash=sha256:{sha} # socket-patch vendor: requests==2.28.1\n\ + ./.socket/vendor/pypi/{UUID}/urllib3-1.26.18-py2.py3-none-any.whl ; \ + python_version >= \"3.7\" --hash=sha256:{sha} \ + # socket-patch vendor: urllib3==1.26.18 (transitive)\n\ + flask==3.0.0\n\ + ./wheels/local_thing-1.0-py3-none-any.whl\n", + sha = "c".repeat(64), + ), + ) + .await; + let entries = inventory_pypi_locks(tmp.path()).await.unwrap(); + assert_eq!( + sorted_pairs(&entries), + vec![ + ("flask".to_string(), "3.0.0".to_string()), + ("requests".to_string(), "2.28.1".to_string()), + ("urllib3".to_string(), "1.26.18".to_string()), + ], + "a user's own wheel path stays out; ours come back as the package \ + they replace: {entries:?}" + ); + for e in &entries { + assert_eq!(e.integrity, LockIntegrity::None, "{e:?}"); + assert_eq!(e.resolved, None, "{e:?}"); + } +} + +/// The vendored writer's OWN output must re-inventory: build the line with +/// the writer's formatter and feed it straight back in. +#[tokio::test] +async fn the_vendored_requirements_writers_own_output_reinventories() { + const UUID: &str = "44444444-4444-4444-4444-444444444444"; + let tmp = tempfile::tempdir().unwrap(); + let line = crate::vendor::pypi_requirements::vendor_line( + &format!(".socket/vendor/pypi/{UUID}/requests-2.28.1-py3-none-any.whl"), + &"c".repeat(64), + "requests", + "2.28.1", + &None, + false, + ); + write(tmp.path(), "requirements.txt", &format!("{line}\n")).await; + let entries = inventory_pypi_locks(tmp.path()).await.unwrap(); + assert_eq!( + sorted_pairs(&entries), + vec![("requests".to_string(), "2.28.1".to_string())], + "wet requirements.txt:\n{line}\nentries: {entries:?}" + ); +} diff --git a/crates/socket-patch-core/src/vendor/mod.rs b/crates/socket-patch-core/src/vendor/mod.rs index d331a849..023aa335 100644 --- a/crates/socket-patch-core/src/vendor/mod.rs +++ b/crates/socket-patch-core/src/vendor/mod.rs @@ -80,7 +80,7 @@ mod pypi_lock; pub mod pypi_pdm; pub mod pypi_pipenv; pub mod pypi_poetry; -mod pypi_requirements; +pub(crate) mod pypi_requirements; mod pypi_uv; mod pypi_wheel; pub mod registry_fetch; diff --git a/crates/socket-patch-core/src/vendor/pypi_requirements.rs b/crates/socket-patch-core/src/vendor/pypi_requirements.rs index 56fa3a73..bd920202 100644 --- a/crates/socket-patch-core/src/vendor/pypi_requirements.rs +++ b/crates/socket-patch-core/src/vendor/pypi_requirements.rs @@ -574,7 +574,11 @@ async fn plan_requirements( /// The committed vendor line. `transitive` adds the `(transitive)` note so a /// reader knows the line was appended (no pin was replaced). -fn vendor_line( +/// +/// Visible to the rest of `vendor` so the lockfile inventory's round-trip +/// test can read back exactly what this writes (the two grammars — the one +/// that writes a vendored line and the one that reads it — must agree). +pub(in crate::vendor) fn vendor_line( rel_wheel: &str, sha256_hex: &str, canon_name: &str, From 7dbc90a19087156d3ec146d82546c5ea1b81eee0 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 25 Sep 2026 05:07:57 -0400 Subject: [PATCH 07/12] fix(vendor): a zero-delta patch file needs no blob content to vendor MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The JS-7 package is still unvendorable. a34bb9d0 stopped one contentless view from killing a whole run, but the run's own example package — `pkg:npm/tar-fs@2.1.1`, patch `8ff3e0c7-…`, one changed file plus seven zero-delta fixture files — still fails on every run, and nothing in it actually needs the bytes the view withholds. `covered()` demanded the after-blob for EVERY file of a record. A zero-delta file (`beforeHash == afterHash`) is already at its patched content in the pristine copy: `verify_file_patch` answers `AlreadyPatched` as soon as the on-disk hash equals `afterHash` (`patch/apply.rs`), which is exactly why the view serves such a file with hashes and no `blobContent`. Requiring it made every patch that carries one permanently unvendorable — measured on origin/main as the original JS-7 symptom (exit 1, `status: error`, `no_local_source`, zero events) and on a34bb9d0 as a per-package `failed`/`no_local_source` on every run. `needs_blob()` is now the one rule, used by `covered()` (which decides what to fetch) and by the fetch loop (which decides whether a view came back complete), so the two can never disagree about which files a fetch must bring back. The loop also collects every genuinely contentless file instead of breaking at the first one: `patch.files` is a `HashMap`, so "the first file with no content" was bucket order, and a partial view abandoned its remaining files at random. Tests: `a_view_whose_only_contentless_files_are_zero_delta_vendors` is the live JS-7 shape — red on unmodified main (`status: error`) and on a34bb9d0, green now, with the vendored tarball asserted to carry the changed file at its patched bytes AND the zero-delta file at the bytes it always had. `contentless_patch_view_fails_only_its_own_package` and `every_patch_unstageable_keeps_the_run_level_error` (both added by a34bb9d0) encoded the wrong classification: they made their package unstageable with a zero-delta file, i.e. they asserted that the JS-7 package must fail. Every assertion in both is unchanged; only the fixture's view changed, so the package they exercise is now unsatisfiable for a reason that really is unsatisfiable — the file the patch CHANGES is served with no content, so its patched bytes exist nowhere. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/commands/fetch_stage.rs | 76 ++++++++-- .../tests/vendor_partial_staging_e2e.rs | 134 +++++++++++++++--- 2 files changed, 173 insertions(+), 37 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/fetch_stage.rs b/crates/socket-patch-cli/src/commands/fetch_stage.rs index e0b4cb8f..8c091ec0 100644 --- a/crates/socket-patch-cli/src/commands/fetch_stage.rs +++ b/crates/socket-patch-cli/src/commands/fetch_stage.rs @@ -16,7 +16,7 @@ use socket_patch_core::api::blob_fetcher::{ DownloadMode, FetchMissingBlobsResult, }; use socket_patch_core::api::client::{get_api_client_with_overrides, ApiClient}; -use socket_patch_core::manifest::schema::{PatchManifest, PatchRecord}; +use socket_patch_core::manifest::schema::{PatchFileInfo, PatchManifest, PatchRecord}; use socket_patch_core::patch::apply::{is_valid_blob_hash, PatchSources}; use tempfile::TempDir; @@ -473,6 +473,18 @@ pub(crate) enum MemStageOutcome { Unavailable, } +/// Does vendoring this file need the patch's after-BLOB? +/// +/// No, when the patch does not change it (`beforeHash == afterHash`): the +/// pristine copy already carries the patched bytes, and the apply pipeline +/// answers `AlreadyPatched` for it without writing anything. The patch view +/// says the same thing by serving such a file with hashes and no +/// `blobContent`, so treating it as a failed fetch made any patch with a +/// zero-delta file permanently unvendorable. +fn needs_blob(file: &PatchFileInfo) -> bool { + file.before_hash != file.after_hash +} + /// Stage patch sources for a VENDOR run without writing anything: /// a record is locally satisfied when all its after-blobs are on disk or /// a package archive is (a diff archive is NOT sufficient — vendor's @@ -530,12 +542,22 @@ pub(crate) async fn stage_vendor_sources_in_memory( // produce. On-disk diffs still serve Strategy 2 for clean files; the // after-blob content must additionally exist (disk, seed/harvest, or // fetch). + // + // …for the files the patch CHANGES. A ZERO-DELTA file + // (`beforeHash == afterHash`) is already at its patched content in the + // pristine copy — `verify_file_patch` answers `AlreadyPatched` as soon + // as the on-disk hash equals `afterHash` — so it needs no blob, which + // is exactly why the view serves it with hashes and no `blobContent`. + // Demanding it made such a patch permanently unvendorable (JS-7: + // `pkg:npm/tar-fs@2.1.1`, seven zero-delta fixture files). This + // predicate is the AUTHORITY the fetch loop below agrees with, so the + // two can never disagree about which files a fetch must bring back. let covered = |record: &PatchRecord, mem: &HashMap>| { - record - .files - .values() - .all(|f| !missing_blobs.contains(&f.after_hash) || mem.contains_key(&f.after_hash)) - || !missing_package_archives.contains(&record.uuid) + record.files.values().all(|f| { + !needs_blob(f) + || !missing_blobs.contains(&f.after_hash) + || mem.contains_key(&f.after_hash) + }) || !missing_package_archives.contains(&record.uuid) }; let mut to_fetch: Vec<(&str, &str)> = manifest .patches @@ -598,19 +620,35 @@ pub(crate) async fn stage_vendor_sources_in_memory( to_fetch.len() )); } + // The record is what `covered` above judged, so it is also what + // decides which of this view's files actually need bytes. + let record = manifest.patches.get(*purl); match client.fetch_patch(uuid).await { Ok(Some(patch)) => { let mut complete = true; + // Named so the per-file report is the same on every run: + // `patch.files` is a `HashMap`, so "the first file with + // no content" is otherwise bucket order. + let mut contentless: Vec<&str> = Vec::new(); for (file, info) in &patch.files { - let (Some(b64), Some(hash)) = (&info.blob_content, &info.after_hash) else { - // An error, not progress chatter: prints even - // under --silent (same rule as - // report_offline_missing above). - if !common.json { - status.println(format!( - " [error] {purl}: no blob content served for {file}" - )); + let Some(b64) = &info.blob_content else { + // A zero-delta file is served without content + // because it needs none (see `covered` above). + // Anything else the patch changes is genuinely + // unsatisfiable — collect them all rather than + // abandoning the view's remaining files in + // `HashMap` order. + if record + .and_then(|r| r.files.get(file)) + .is_some_and(|f| !needs_blob(f)) + { + continue; } + contentless.push(file); + complete = false; + continue; + }; + let Some(hash) = &info.after_hash else { complete = false; break; }; @@ -630,6 +668,16 @@ pub(crate) async fn stage_vendor_sources_in_memory( } } } + contentless.sort_unstable(); + // An error, not progress chatter: prints even under + // --silent (same rule as report_offline_missing above). + if !common.json { + for file in &contentless { + status.println(format!( + " [error] {purl}: no blob content served for {file}" + )); + } + } if !complete { failed.push(purl); } diff --git a/crates/socket-patch-cli/tests/vendor_partial_staging_e2e.rs b/crates/socket-patch-cli/tests/vendor_partial_staging_e2e.rs index 806f58f9..db5e9ed2 100644 --- a/crates/socket-patch-cli/tests/vendor_partial_staging_e2e.rs +++ b/crates/socket-patch-cli/tests/vendor_partial_staging_e2e.rs @@ -1,20 +1,26 @@ -//! One unstageable patch must not abort a whole vendored run. +//! Two rules about a patch view that does not serve every file's bytes. //! -//! The patch view serves `blobContent` only for files the patch actually -//! CHANGES: a file whose `beforeHash` equals its `afterHash` comes back with -//! hashes and no content (live example: `pkg:npm/tar-fs@2.1.1`, patch +//! The view serves `blobContent` only for files the patch actually CHANGES: +//! a file whose `beforeHash` equals its `afterHash` comes back with hashes +//! and no content (live example: `pkg:npm/tar-fs@2.1.1`, patch //! `8ff3e0c7-6855-4224-924b-3e1151744ed4`, seven zero-delta fixture files -//! plus one changed `package/index.js`). The in-memory vendor stager treats -//! any such view as a failed fetch, and a single failed fetch made the WHOLE -//! run bail `no_local_source` — exit 1, `status: error`, zero events, and -//! every OTHER package in the manifest left unvendored without a word. +//! plus one changed `package/index.js`). +//! +//! 1. A zero-delta file needs NO content — the pristine copy already holds +//! the patched bytes — so such a view stages and the package vendors +//! (`a_view_whose_only_contentless_files_are_zero_delta_vendors`). +//! 2. A file the patch CHANGES that is served without content is genuinely +//! unsatisfiable. That is a broken PACKAGE, not a broken run: it gets +//! its own `failed` event and the rest of the run carries on. A single +//! such patch used to make the WHOLE run bail `no_local_source` — exit +//! 1, `status: error`, zero events, and every OTHER package in the +//! manifest left unvendored without a word. //! //! A package whose patch content cannot be obtained is an unsatisfiable //! package like any other (`vendor_fetch_failed`, `redirect_revert_failed`, -//! the Bun refusals …): it gets its own `failed` event and the run carries -//! on. The pre-event `no_local_source` bail stays for the case it was -//! written for — NOTHING in the manifest can be staged, so there are no -//! events to report. +//! the Bun refusals …). The pre-event `no_local_source` bail stays for the +//! case it was written for — NOTHING in the manifest can be staged, so +//! there are no events to report. //! //! Hermetic: the API is a `wiremock` mock, `--vendor-source build` keeps the //! vendoring service out of the run, and every package is installed on disk @@ -37,8 +43,9 @@ const GOOD_UUID: &str = "9f6b2c4e-1d3a-4f6b-8c2d-7e5a9b1c3d5f"; const GOOD_ORIG: &[u8] = b"module.exports = () => 'orig';\n"; const GOOD_PATCHED: &[u8] = b"module.exports = () => 'patched';\n"; -/// The JS-7 package: one changed file plus one zero-delta file the view -/// serves with no `blobContent`. +/// The JS-7 package shape: one changed file plus one zero-delta file the +/// view always serves with no `blobContent`. Whether the CHANGED file is +/// served with content is what each test varies. const BAD_PURL: &str = "pkg:npm/tar-fs@2.1.1"; const BAD_UUID: &str = "8ff3e0c7-6855-4224-924b-3e1151744ed4"; const BAD_ORIG: &[u8] = b"module.exports = require('./lib');\n"; @@ -160,11 +167,20 @@ fn fixture(root: &Path) { .unwrap(); } -/// The JS-7 view: the changed file carries `blobContent`, the zero-delta -/// file carries hashes only. -async fn mount_contentless_view(server: &MockServer) { +/// Mount the bad package's view. `changed_content` is the `blobContent` +/// the CHANGED file is served with; `None` makes the view genuinely +/// unsatisfiable (the patch needs those bytes and nothing can supply +/// them). The zero-delta file always comes back with hashes and no +/// content — that is how the API serves a file a patch does not change. +async fn mount_view(server: &MockServer, changed_content: Option<&[u8]>) { use base64::Engine; - let b64 = base64::engine::general_purpose::STANDARD.encode(BAD_PATCHED); + let mut changed = json!({ + "beforeHash": git_hash(BAD_ORIG), + "afterHash": git_hash(BAD_PATCHED), + }); + if let Some(bytes) = changed_content { + changed["blobContent"] = json!(base64::engine::general_purpose::STANDARD.encode(bytes)); + } Mock::given(method("GET")) .and(wm_path(format!("/v0/orgs/{ORG}/patches/view/{BAD_UUID}"))) .respond_with(ResponseTemplate::new(200).set_body_json(json!({ @@ -172,11 +188,7 @@ async fn mount_contentless_view(server: &MockServer) { "purl": BAD_PURL, "publishedAt": "2026-01-01T00:00:00Z", "files": { - "package/index.js": { - "beforeHash": git_hash(BAD_ORIG), - "afterHash": git_hash(BAD_PATCHED), - "blobContent": b64, - }, + "package/index.js": changed, "package/test/fixtures/d/file1": { "beforeHash": git_hash(BAD_FIXTURE), "afterHash": git_hash(BAD_FIXTURE), @@ -191,6 +203,37 @@ async fn mount_contentless_view(server: &MockServer) { .await; } +/// A view the run genuinely cannot satisfy: the file the patch CHANGES is +/// served with no `blobContent`, so the patched bytes exist nowhere. +async fn mount_contentless_view(server: &MockServer) { + mount_view(server, None).await; +} + +/// The live JS-7 view: the changed file carries `blobContent`, and only +/// the zero-delta file comes back contentless — which needs no content. +async fn mount_zero_delta_view(server: &MockServer) { + mount_view(server, Some(BAD_PATCHED)).await; +} + +/// The `path -> bytes` map of a gzipped tarball's regular members. +fn tgz_members(tgz: &Path) -> std::collections::BTreeMap> { + let file = std::fs::File::open(tgz).unwrap_or_else(|e| panic!("open {}: {e}", tgz.display())); + let mut archive = tar::Archive::new(flate2::read::GzDecoder::new(file)); + let mut out = std::collections::BTreeMap::new(); + for entry in archive.entries().expect("tar entries") { + let mut entry = entry.expect("tar entry"); + let path = entry + .path() + .expect("tar path") + .to_string_lossy() + .into_owned(); + let mut bytes = Vec::new(); + std::io::Read::read_to_end(&mut entry, &mut bytes).expect("tar member bytes"); + out.insert(path, bytes); + } + out +} + /// `vendor --json --vendor-source build` against the mock API, with every /// ambient `SOCKET_*` var scrubbed from the child. fn vendor_cli(root: &Path, api_url: &str) -> (i32, Value, String) { @@ -316,3 +359,48 @@ async fn every_patch_unstageable_keeps_the_run_level_error() { "an aborted run vendors nothing: {env:#}" ); } + +/// The JS-7 package itself must VENDOR, not merely fail politely. +/// +/// `pkg:npm/tar-fs@2.1.1` patch `8ff3e0c7-…` changes one file and carries +/// seven zero-delta fixture files (`beforeHash == afterHash`). The view +/// serves `blobContent` only for the file it CHANGES, so those seven come +/// back contentless — and a zero-delta file needs no content: the pristine +/// copy already holds the patched bytes, which is exactly what +/// `verify_file_patch` answers `AlreadyPatched` for. Requiring the +/// after-blob for every file made this patch permanently unvendorable. +#[tokio::test] +async fn a_view_whose_only_contentless_files_are_zero_delta_vendors() { + let server = MockServer::start().await; + mount_zero_delta_view(&server).await; + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + fixture(root); + + let (code, env, stderr) = vendor_cli(root, &server.uri()); + + assert_eq!( + code, 0, + "nothing in this patch needs the unserved bytes: {env:#}\nstderr:\n{stderr}" + ); + assert_eq!(env["status"], "success", "{env:#}"); + assert_eq!(event_for(&env, BAD_PURL)["action"], "applied", "{env:#}"); + assert_eq!(event_for(&env, GOOD_PURL)["action"], "applied", "{env:#}"); + + // The vendored tarball carries BOTH files — the changed one at its + // patched bytes, the zero-delta one at the bytes it always had. + let tgz = root.join(format!(".socket/vendor/npm/{BAD_UUID}/tar-fs-2.1.1.tgz")); + let members = tgz_members(&tgz); + assert_eq!( + members.get("package/index.js").map(Vec::as_slice), + Some(BAD_PATCHED), + "the changed file is the patched content: {members:?}" + ); + assert_eq!( + members + .get("package/test/fixtures/d/file1") + .map(Vec::as_slice), + Some(BAD_FIXTURE), + "the zero-delta file is vendored from the pristine copy: {members:?}" + ); +} From ec2c1c7dad744e3bdd563a721f0c2615f82de821 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 25 Sep 2026 05:10:24 -0400 Subject: [PATCH 08/12] fix(vendor): name the real reason in a per-package staging failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `drop_unstageable` recorded every dropped purl with one verbatim run-level string, "patch artifacts unavailable (offline or download failure)". For the case the per-package report was written for — the view is served fine, 200, but a file the patch changes carries no `blobContent` — the run is neither offline nor a download failure, so the one machine-readable explanation named two causes that are both false. The stager knew the real one (it formats `[error] : no blob content served for `), but every human channel in that block is gated on `if !common.json`, so a `--json` consumer — depscan, CI — saw only the misleading detail and never learned which file was contentless. The whole point of the per-package report is per-package diagnostics, and the per-package slot was the one place the specific reason was dropped. Carry the reason out of the fetch loop with its purl and put it in that package's `failed` event: which file was served without content (and how many others), which file carried a malformed or undecodable blob, that no view is served for the uuid at all, or the transport error. `no_local_source` stays the stable `errorCode`; only the free-text `error` changes. The `[error]`/summary stderr lines are untouched. Pinned by an added assertion on the existing e2e: the failed event's `error` must read "the patch view served no blob content for package/index.js". Co-Authored-By: Claude Opus 5 (1M context) --- .../src/commands/fetch_stage.rs | 78 ++++++++++++------- .../src/commands/repair_vendor.rs | 2 +- .../tests/vendor_partial_staging_e2e.rs | 10 +++ 3 files changed, 63 insertions(+), 27 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/fetch_stage.rs b/crates/socket-patch-cli/src/commands/fetch_stage.rs index 8c091ec0..62a6d8e2 100644 --- a/crates/socket-patch-cli/src/commands/fetch_stage.rs +++ b/crates/socket-patch-cli/src/commands/fetch_stage.rs @@ -441,12 +441,13 @@ pub(crate) struct MemStagedSources { diffs: PathBuf, packages: PathBuf, mem: HashMap>, - /// The purls this staging could NOT obtain patch content for, while at - /// least one other patch staged fine. Each is an unsatisfiable package - /// the caller reports per-package (and leaves out of the engine run) — - /// see [`stage_vendor_sources_in_memory`]. Sorted, so the per-package - /// reports come out in the same order every run. - unavailable: Vec, + /// The purls this staging could NOT obtain patch content for, each with + /// the reason, while at least one other patch staged fine. Each is an + /// unsatisfiable package the caller reports per-package (and leaves out + /// of the engine run) — see [`stage_vendor_sources_in_memory`]. Sorted + /// by purl, so the per-package reports come out in the same order every + /// run. + unavailable: Vec<(String, String)>, } impl MemStagedSources { @@ -462,7 +463,7 @@ impl MemStagedSources { } /// See [`MemStagedSources::unavailable`]. - pub(crate) fn unavailable(&self) -> &[String] { + pub(crate) fn unavailable(&self) -> &[(String, String)] { &self.unavailable } } @@ -533,7 +534,7 @@ pub(crate) async fn stage_vendor_sources_in_memory( let missing_blobs = get_missing_blobs(manifest, &blobs).await; let missing_package_archives = get_missing_archives(manifest, &packages).await; let mut mem = seed; - let mut unavailable: Vec = Vec::new(); + let mut unavailable: Vec<(String, String)> = Vec::new(); // A diff archive alone is NOT a sufficient source here, unlike the disk // stager: vendoring runs the auto-force policy, where a beforeHash @@ -610,7 +611,12 @@ pub(crate) async fn stage_vendor_sources_in_memory( &built } }; - let mut failed: Vec<&str> = Vec::new(); + // Each dropped purl with WHY it was dropped. The reason is the only + // machine-readable explanation the caller can put in that package's + // `failed` event, and the human `[error]` lines below are printed + // exclusively under `!--json` — so without it a `--json` consumer + // learned nothing about which file was contentless. + let mut failed: Vec<(&str, String)> = Vec::new(); for (i, (purl, uuid)) in to_fetch.iter().enumerate() { if to_fetch.len() > 1 { status.set(format!( @@ -625,11 +631,11 @@ pub(crate) async fn stage_vendor_sources_in_memory( let record = manifest.patches.get(*purl); match client.fetch_patch(uuid).await { Ok(Some(patch)) => { - let mut complete = true; // Named so the per-file report is the same on every run: // `patch.files` is a `HashMap`, so "the first file with // no content" is otherwise bucket order. let mut contentless: Vec<&str> = Vec::new(); + let mut malformed: Option = None; for (file, info) in &patch.files { let Some(b64) = &info.blob_content else { // A zero-delta file is served without content @@ -645,17 +651,19 @@ pub(crate) async fn stage_vendor_sources_in_memory( continue; } contentless.push(file); - complete = false; continue; }; let Some(hash) = &info.after_hash else { - complete = false; + malformed = + Some(format!("the patch view served no afterHash for {file}")); break; }; // Same key guard as the disk writer: the hash names the // lookup key the apply pipeline gates writes on. if !is_valid_blob_hash(hash) { - complete = false; + malformed = Some(format!( + "the patch view served an invalid afterHash for {file}" + )); break; } match base64_decode(b64) { @@ -663,7 +671,9 @@ pub(crate) async fn stage_vendor_sources_in_memory( mem.insert(hash.clone(), bytes); } Err(_) => { - complete = false; + malformed = Some(format!( + "the patch view served undecodable blob content for {file}" + )); break; } } @@ -678,11 +688,12 @@ pub(crate) async fn stage_vendor_sources_in_memory( )); } } - if !complete { - failed.push(purl); + if let Some(reason) = malformed.or_else(|| contentless_reason(&contentless)) { + failed.push((purl, reason)); } } - _ => failed.push(purl), + Ok(None) => failed.push((purl, format!("no patch view is served for {uuid}"))), + Err(e) => failed.push((purl, format!("the patch view could not be fetched: {e}"))), } } status.finish(); @@ -695,11 +706,12 @@ pub(crate) async fn stage_vendor_sources_in_memory( // channel for these purls in both arms below: the per-package // arm only records events. if !common.json { + let purls: Vec<&str> = failed.iter().map(|(purl, _)| *purl).collect(); eprintln!( "Error: Could not fetch patch content for {}:", plural(failed.len(), "patch", "patches") ); - for line in format_purl_list(&failed, 5) { + for line in format_purl_list(&purls, 5) { eprintln!("{line}"); } } @@ -710,7 +722,10 @@ pub(crate) async fn stage_vendor_sources_in_memory( if failed.len() == manifest.patches.len() { return MemStageOutcome::Unavailable; } - unavailable = failed.into_iter().map(str::to_string).collect(); + unavailable = failed + .into_iter() + .map(|(purl, reason)| (purl.to_string(), reason)) + .collect(); unavailable.sort(); } } @@ -732,24 +747,35 @@ pub(crate) async fn stage_vendor_sources_in_memory( pub(crate) fn drop_unstageable<'a>( env: &mut Envelope, records: &'a HashMap, - unavailable: &[String], + unavailable: &[(String, String)], ) -> (Cow<'a, HashMap>, bool) { if unavailable.is_empty() { return (Cow::Borrowed(records), false); } - for purl in unavailable { + for (purl, reason) in unavailable { env.record( - PatchEvent::new(PatchAction::Failed, purl.clone()).with_error( - "no_local_source", - "patch artifacts unavailable (offline or download failure)", - ), + PatchEvent::new(PatchAction::Failed, purl.clone()) + .with_error("no_local_source", reason.clone()), ); } let mut kept = records.clone(); - kept.retain(|purl, _| !unavailable.contains(purl)); + kept.retain(|purl, _| !unavailable.iter().any(|(dropped, _)| dropped == purl)); (Cow::Owned(kept), true) } +/// The reason string for a view that came back missing the blob content of +/// `contentless` (already sorted). `None` when nothing was missing. +fn contentless_reason(contentless: &[&str]) -> Option { + let (first, rest) = contentless.split_first()?; + Some(match rest.len() { + 0 => format!("the patch view served no blob content for {first}"), + n => format!( + "the patch view served no blob content for {first} (and {n} other {})", + plural(n, "file", "files") + ), + }) +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/socket-patch-cli/src/commands/repair_vendor.rs b/crates/socket-patch-cli/src/commands/repair_vendor.rs index e06d215c..b1cc09c4 100644 --- a/crates/socket-patch-cli/src/commands/repair_vendor.rs +++ b/crates/socket-patch-cli/src/commands/repair_vendor.rs @@ -1216,7 +1216,7 @@ pub(crate) async fn repair_vendored_artifacts_with_references( if !staged.unavailable().is_empty() { let (stuck, rest): (Vec, Vec) = candidates .into_iter() - .partition(|c| staged.unavailable().contains(&c.purl)); + .partition(|c| staged.unavailable().iter().any(|(purl, _)| purl == &c.purl)); report_no_local_source(env, common, &stuck, &unrebuildable, &mut rebuilt); candidates = rest; if candidates.is_empty() { diff --git a/crates/socket-patch-cli/tests/vendor_partial_staging_e2e.rs b/crates/socket-patch-cli/tests/vendor_partial_staging_e2e.rs index db5e9ed2..ae6159d8 100644 --- a/crates/socket-patch-cli/tests/vendor_partial_staging_e2e.rs +++ b/crates/socket-patch-cli/tests/vendor_partial_staging_e2e.rs @@ -306,6 +306,16 @@ async fn contentless_patch_view_fails_only_its_own_package() { bad["errorCode"], "no_local_source", "the per-package failure keeps the staging code: {env:#}" ); + // The per-package slot is the ONE machine-readable explanation a + // `--json` consumer gets (every human channel in the stager is gated + // on `!--json`), so it must carry the REAL reason. This run is neither + // offline nor a download failure: the view was served, 200, with a + // file it had no content for. + assert_eq!( + bad["error"].as_str(), + Some("the patch view served no blob content for package/index.js"), + "the failure names the file that was served without content: {env:#}" + ); let good = event_for(&env, GOOD_PURL); assert_eq!( From b8f08b070856fcef071b88c5de4f2090186571db Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 25 Sep 2026 05:13:34 -0400 Subject: [PATCH 09/12] docs(get): the --all-releases arm is purl-ordered, not a pass-through MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit acae5db3 made `filter_to_installed_releases` sort both arms by (purl, uuid) but left the contract sentence "With `--all-releases` set this is a verbatim pass-through" in place. Harmless today — `select_patches` hands this function one patch per purl, so the uuid tiebreak never decides which record survives the purl-keyed `records` map in `download_patch_records_preflighted` — but a future caller would read a sentence that is no longer true. Say what the arm does now: nothing is narrowed away and no view is fetched, and the output order is the same one the narrowed arm returns. Co-Authored-By: Claude Opus 5 (1M context) --- crates/socket-patch-cli/src/commands/get.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-cli/src/commands/get.rs b/crates/socket-patch-cli/src/commands/get.rs index 7c8eb93c..80a61956 100644 --- a/crates/socket-patch-cli/src/commands/get.rs +++ b/crates/socket-patch-cli/src/commands/get.rs @@ -1295,7 +1295,10 @@ fn crawler_options_for(common: &GlobalArgs) -> CrawlerOptions { /// those from memory instead of fetching every view a second time. Only /// successful fetches are cached: a variant whose view errored or 404'd is /// re-fetched by the loop so the failure surfaces per patch as before. -/// With `--all-releases` set this is a verbatim pass-through. +/// With `--all-releases` set no variant is narrowed away and no view is +/// fetched — the whole selection comes back, in the same purl order +/// ([`sort_by_purl`]) as the narrowed arm, so both arms of this function +/// share one output contract. async fn filter_to_installed_releases( selected: &[PatchSearchResult], all_releases: bool, From 18deedef18baa92c41b0c741b9e14e1475bdd837 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 25 Sep 2026 05:13:34 -0400 Subject: [PATCH 10/12] test(vendor): cover the vendored scan's per-package staging drop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `drop_unstageable` has three callers — `vendor`, `scan`/`get --mode vendored` (`scan::vendor_flow`) and `repair` — and every test that reached it drove `vendor`. The suites that touch the vendored scan (`scan_vendor_step_error_e2e`, `covgap_commands_scan_vendor_flow`, `covgap_commands_get`) all mount single-patch manifests, so they only ever exercised the preserved whole-run bail: a regression in the vendored scan's `Ok(staging_errors || engine_errors)` fold — dropping the staging-error bit, or reporting a stuck package twice — passed the whole suite. One mixed-selection case for `scan --mode vendored`, manifest-free the way vendored mode really runs (discovery + the download phase's blob seed, no `.socket/` on disk): one package whose view serves the file it changes without content and one it serves complete. The stuck package is reported exactly once as `failed`/`no_local_source` with the reason naming the file, the other still vendors, and the envelope is `partialFailure`. Co-Authored-By: Claude Opus 5 (1M context) --- .../tests/vendor_partial_staging_e2e.rs | 169 ++++++++++++++++++ 1 file changed, 169 insertions(+) diff --git a/crates/socket-patch-cli/tests/vendor_partial_staging_e2e.rs b/crates/socket-patch-cli/tests/vendor_partial_staging_e2e.rs index ae6159d8..dc001f98 100644 --- a/crates/socket-patch-cli/tests/vendor_partial_staging_e2e.rs +++ b/crates/socket-patch-cli/tests/vendor_partial_staging_e2e.rs @@ -414,3 +414,172 @@ async fn a_view_whose_only_contentless_files_are_zero_delta_vendors() { "the zero-delta file is vendored from the pristine copy: {members:?}" ); } + +// ── the other caller of the per-package drop ──────────────────────────── +// +// `drop_unstageable` is wired into `vendor` (above), `scan --mode vendored` +// / `get --mode vendored` (`scan::vendor_flow`) and `repair`. The vendored +// SCAN fold — `Ok(staging_errors || engine_errors)` — has its own error +// path, and every existing suite that touches it mounts a single-patch +// manifest, so it only ever exercised the preserved whole-run bail. + +const GOOD_ENCODED: &str = "pkg%3Anpm%2Fleft-pad%401.3.0"; +const BAD_ENCODED: &str = "pkg%3Anpm%2Ftar-fs%402.1.1"; + +/// Discovery for both packages: the batch endpoint plus the per-package +/// search each purl falls back to. +async fn mount_discovery(server: &MockServer) { + Mock::given(method("POST")) + .and(wm_path(format!("/v0/orgs/{ORG}/patches/batch"))) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "packages": [ + { "purl": GOOD_PURL, "patches": [{ + "uuid": GOOD_UUID, "purl": GOOD_PURL, "tier": "free", + "cveIds": ["CVE-2026-0001"], "ghsaIds": [], "severity": "high", + "title": "good" }] }, + { "purl": BAD_PURL, "patches": [{ + "uuid": BAD_UUID, "purl": BAD_PURL, "tier": "free", + "cveIds": ["CVE-2026-0002"], "ghsaIds": [], "severity": "high", + "title": "bad" }] }, + ], + "canAccessPaidPatches": false, + }))) + .mount(server) + .await; + for (encoded, uuid, purl) in [ + (GOOD_ENCODED, GOOD_UUID, GOOD_PURL), + (BAD_ENCODED, BAD_UUID, BAD_PURL), + ] { + Mock::given(method("GET")) + .and(wm_path(format!( + "/v0/orgs/{ORG}/patches/by-package/{encoded}" + ))) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "patches": [{ + "uuid": uuid, + "purl": purl, + "publishedAt": "2026-01-01T00:00:00Z", + "description": "d", + "license": "MIT", + "tier": "free", + "vulnerabilities": {}, + }], + "canAccessPaidPatches": false, + }))) + .mount(server) + .await; + } +} + +/// The good package's view, served complete. +async fn mount_good_view(server: &MockServer) { + use base64::Engine; + Mock::given(method("GET")) + .and(wm_path(format!("/v0/orgs/{ORG}/patches/view/{GOOD_UUID}"))) + .respond_with(ResponseTemplate::new(200).set_body_json(json!({ + "uuid": GOOD_UUID, + "purl": GOOD_PURL, + "publishedAt": "2026-01-01T00:00:00Z", + "files": { "package/index.js": { + "beforeHash": git_hash(GOOD_ORIG), + "afterHash": git_hash(GOOD_PATCHED), + "blobContent": base64::engine::general_purpose::STANDARD.encode(GOOD_PATCHED), + }}, + "vulnerabilities": {}, + "description": "d", + "license": "MIT", + "tier": "free", + }))) + .mount(server) + .await; +} + +/// The project WITHOUT `.socket/`: vendored mode is manifest-free, so the +/// records come from discovery and the blobs from the download phase. +fn scan_fixture(root: &Path) { + fixture(root); + std::fs::remove_dir_all(root.join(".socket")).unwrap(); +} + +fn scan_vendored_cli(root: &Path, api_url: &str) -> (i32, Value, String) { + let mut cmd = Command::new(env!("CARGO_BIN_EXE_socket-patch")); + cmd.args([ + "scan", + "--json", + "--mode", + "vendored", + "--yes", + "--vendor-source", + "build", + "--api-url", + api_url, + "--api-token", + "fake-token", + "--org", + ORG, + ]) + .current_dir(root); + for (key, _) in std::env::vars() { + if key.starts_with("SOCKET_") && key != "SOCKET_NO_CONFIG" { + cmd.env_remove(key); + } + } + cmd.env("SOCKET_TELEMETRY_DISABLED", "1"); + let out = cmd.output().expect("spawn socket-patch"); + let stdout = String::from_utf8_lossy(&out.stdout).into_owned(); + let stderr = String::from_utf8_lossy(&out.stderr).into_owned(); + let env: Value = serde_json::from_str(stdout.trim()).unwrap_or_else(|e| { + panic!("scan --json must emit an envelope: {e}\nstdout:\n{stdout}\nstderr:\n{stderr}") + }); + (out.status.code().unwrap_or(-1), env, stderr) +} + +/// `scan --mode vendored` over a mixed selection: one package the view +/// cannot supply and one it can. The unsatisfiable package is reported +/// once, per package, and the other still vendors — the vendored scan's +/// own fold, not `vendor`'s. +#[tokio::test] +async fn scan_vendored_reports_an_unstageable_package_and_vendors_the_rest() { + let server = MockServer::start().await; + mount_discovery(&server).await; + mount_good_view(&server).await; + mount_contentless_view(&server).await; + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + scan_fixture(root); + + let (code, env, stderr) = scan_vendored_cli(root, &server.uri()); + + assert_eq!(code, 1, "{env:#}\nstderr:\n{stderr}"); + let vendor = &env["vendor"]; + assert_eq!( + vendor["status"], "partialFailure", + "one bad package is a partial failure, not a step abort: {env:#}" + ); + let bad = event_for(vendor, BAD_PURL); + assert_eq!(bad["action"], "failed", "{env:#}"); + assert_eq!(bad["errorCode"], "no_local_source", "{env:#}"); + assert_eq!( + bad["error"].as_str(), + Some("the patch view served no blob content for package/index.js"), + "{env:#}" + ); + assert_eq!( + events(vendor) + .iter() + .filter(|e| e["purl"] == BAD_PURL) + .count(), + 1, + "the stuck package is reported exactly once: {env:#}" + ); + assert_eq!(event_for(vendor, GOOD_PURL)["action"], "applied", "{env:#}"); + assert!( + root.join(format!(".socket/vendor/npm/{GOOD_UUID}/left-pad-1.3.0.tgz")) + .is_file(), + "the satisfiable package must still be vendored: {env:#}" + ); + assert!( + !root.join(format!(".socket/vendor/npm/{BAD_UUID}")).exists(), + "nothing is written for the unstageable package: {env:#}" + ); +} From ee41402d8ac219c617221e70968e3e97d37d1ad4 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 25 Sep 2026 05:13:34 -0400 Subject: [PATCH 11/12] docs: record the vendored-envelope, ordering and gem changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four user- and consumer-visible behavior changes shipped on this branch with no `[Unreleased]` entry, although the file's own header states the Release workflow refuses to publish a version that does not appear in it (`scripts/release-lint.sh`) and every recent merge to main updates it. CHANGELOG `[Unreleased]` → `Fixed` now carries one entry per fix, each calling out what a JSON consumer sees change: the vendor envelope's shift from `status: "error"` + top-level `no_local_source` to `partialFailure` + per-package `failed` events, the purl ordering of `download.patches` / `apply.patches`, the requirements.txt inventory recovery, the zero-delta staging fix, and the gem build-mode refusal — including the `vendor_fetched_missing` warning that disappears from a build-mode run that no longer downloads. CLI_CONTRACT's error-code table gains the two facts a consumer needs and could not previously read anywhere: * `no_local_source` is now reported at TWO levels, and which one arrives depends on whether anything else in the manifest staged (so a one-patch manifest still gets the run-level shape). The table says so explicitly rather than leaving the inconsistency undocumented, and `json_envelope`'s `error_code` doc points at it. * `gem_spec_missing` was never in the table at all. Its row states the pre-fetch refusal, its exact scope (the lock both resolves AND verifies the gem, and no ledger entry already vendors it), what is deliberately unaffected (`vendor_fetch_unverifiable`'s documented pair, an already-vendored re-run, `auto`/`service`), and the dropped warning. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 47 ++++++++++++++++++++ crates/socket-patch-cli/CLI_CONTRACT.md | 5 ++- crates/socket-patch-cli/src/json_envelope.rs | 8 ++++ 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8540e431..fa1ebf3e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -514,6 +514,53 @@ into the new version's section — see docs/releasing.md. ### Fixed +- **A patch file the patch never changes no longer blocks vendoring.** The + patch view serves `blobContent` only for the files a patch CHANGES, so a + zero-delta file (`beforeHash == afterHash`) comes back with hashes and no + content — and needs none: the pristine copy already carries the patched + bytes. The vendor stager counted such a view as a failed fetch, which made + any patch carrying a zero-delta file permanently unvendorable (live + example: `pkg:npm/tar-fs@2.1.1`). +- **One unstageable patch no longer kills a whole vendored run.** A package + whose patch content cannot be obtained now gets its own `failed` event + with `errorCode: "no_local_source"` and the rest of the run still vendors, + in `vendor`, `scan`/`get --mode vendored` and `repair` alike. The event's + `error` names the real reason (which file the view served without content, + a malformed blob, or the fetch error) instead of the generic run-level + "patch artifacts unavailable (offline or download failure)". + **JSON consumers:** for a partial staging failure the vendor envelope is + now `status: "partialFailure"` with `error: null` and per-package events, + where it used to be `status: "error"` with a top-level + `error.code: "no_local_source"` and an empty `events[]`. The run-level + shape is unchanged when NOTHING in the manifest can be staged (including + a one-patch manifest) — `no_local_source` can therefore arrive run-level + or event-level, and both shapes are documented in CLI_CONTRACT.md. +- **`get` emits its patch lists in a stable order.** The release-variant + narrowing drained a `HashMap`, so `download.patches`, `apply.patches` and + the per-patch stderr lines came out in bucket order: two identical runs of + the same project emitted the same records in different orders. All of them + are purl-ordered now, matching every sibling collection in the envelope. +- **A requirements.txt this CLI already rewired stays in the lockfile + inventory.** Both shapes we write — the hosted `name @ ` + direct reference and the vendored bare `./.socket/vendor/pypi/…` wheel + path tagged `# socket-patch vendor: ==` — are read back as the + package they replace (discovery-only, exactly like the `==` pin they + replaced). A second hosted run over a wet requirements.txt reported + `packagesWithPatches: 1` instead of 12; a vendored one under-reported the + same way. +- **`vendor --vendor-source build` no longer downloads a gem it cannot use.** + A gem the lockfile resolves and verifies, with no installed copy, is + refused with `failed`/`gem_spec_missing` BEFORE the registry round trip: + the bundler path source needs the stub gemspec rubygems writes at install + time, which a downloaded `.gem` does not carry, so the local build refused + it after paying for the download on every run. The refusal names the real + remedy (`bundle install`, or `--vendor-source=auto`). + **JSON consumers:** that run no longer carries the + `vendor_fetched_missing` warning event it used to emit before failing. + Unaffected: a gem the lock cannot verify keeps its documented + `vendor_fetch_unverifiable` + `package_not_installed` pair, an + already-vendored gem still re-runs green, and `auto`/`service` still + fetch. - **Hosted Go redirects no longer claim patches that did not land.** `scan`/`get --mode hosted` counted a Go module as redirected (recorded it in the redirect ledger, so `vex` attested it) whenever any project diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index e4b72f53..8a4e4ac5 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -1171,7 +1171,7 @@ Every `--json` invocation emits a single JSON object that follows the **unified | `already_patched` | `skipped` | apply: every file's hash already matches `afterHash`. | | `package_not_installed` | `skipped` | apply: manifest entry has no matching installed package. | | `apply_failed` | `failed` | apply: hash mismatch, write error, archive read error. | -| `no_local_source` | `skipped`/`failed` | `--offline` and the patch is missing from `.socket/`. | +| `no_local_source` | `skipped`/`failed` | `--offline` and the patch is missing from `.socket/`. **Vendored staging (v5.0) reports it at TWO levels:** per package (a `failed` event whose `error` names the reason — which file the patch view served with no `blobContent`, a malformed blob, or the fetch error — envelope `partialFailure`, `error: null`) when at least one other patch staged; and run-level (top-level `error.code`, `status: "error"`, empty `events[]`) when NOTHING in the manifest can be staged, which includes a one-patch manifest. A consumer routing on this code must handle both. A file the patch does not change (`beforeHash == afterHash`) is never a reason: the view serves it without content because it needs none. | | `offline_missing_sources` / `sources_download_failed` | apply run-level `warnings[]` | apply (additive): the patch sources were unavailable — `--offline` with no local source, or the download left a patch with no source — so nothing was attempted. The envelope keeps its pinned shape (`partialFailure`, empty `events[]`, zero summary, no top-level `error`); the warning is its machine-readable reason (the human path prints the staging `Error:` line on stderr instead, even under `--silent`). | | `paid_required` | `failed` / status=`paidRequired` | get/scan: patch needs a paid plan and the caller's token isn't entitled. `get ` on the public proxy reports it (exit 0) both for a `tier: "paid"` view and for the proxy's 403 refusal, whose record then carries only `uuid` + `tier` (the proxy never named the purl). | | `download_failed` | `failed` | repair/get: network or 404 on patch fetch. | @@ -1226,7 +1226,7 @@ Every `--json` invocation emits a single JSON object that follows the **unified | `vendor_content_mismatch_overwritten` | `skipped` (warning) | vendor: a staged file matched NEITHER beforeHash nor afterHash (patch built against different bytes, or local edits); the stage was overwritten with the verified patched content and the vendor succeeded. | | `vendor_fetched_missing` | `skipped` (warning) | vendor: the package was not installed; its pristine artifact was fetched per the lockfile resolution (or staged from the committed vendor artifact), integrity-verified, and vendored — the project tree was not touched. For `poetry.lock` (which records hashes but no URLs) the pure-Python wheel's sha256 selects the file through PyPI's JSON API (`SOCKET_PYPI_JSON_API` overrides the endpoint); Poetry 0.12's bare `[metadata.hashes]` names no wheel, so those locks still need an installed copy (`vendor_fetch_unverifiable`). | | `vendor_fetch_failed` | `failed` | vendor: the lockfile-resolved fetch was attempted and failed (HTTP error, size cap, integrity mismatch, or a PRESENT-but-corrupt committed artifact — pointed at `socket-patch repair`). A MISSING committed artifact no longer lands here: it falls through to the ledger-recovered registry fetch. Suppresses the duplicate `package_not_installed` skip. | -| `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_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. Unchanged for gems by the build-mode `gem_spec_missing` refusal below, which fires only where a fetch WOULD have run. | | `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_reused` | `skipped` (verbose note) | vendor / scan `--vendor` (pypi): the wiring was dropped by a relock but the committed wheel the ledger vouches for verified, so it was re-wired as-is — no service download, no rebuild; the lock pins the first run's sha again. | @@ -1236,6 +1236,7 @@ Every `--json` invocation emits a single JSON object that follows the **unified | `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. | | `content_mismatch_overwritten` | `skipped` (warning) | apply (default policy): a file matched NEITHER beforeHash nor afterHash and was overwritten with the full verified patched content. `--strict` turns this case into a `failed` event instead. | | `vendor_lock_checksums_unsupported` / `vendor_stale_lock_checksum` | `failed` | vendor (gem): an ambiguous/platform CHECKSUMS entry, or a v1-wired lock whose stale token blocks the hot path (run `vendor --revert` + re-vendor). | +| `gem_spec_missing` | `failed` | vendor (gem): the gem is not installed and the run cannot use the patch service (`--vendor-source build`, or no service config), so the local build has no stub gemspec to give bundler's path source — a downloaded `.gem` carries its gemspec only as YAML in `metadata.gz`. Raised BEFORE the registry round trip when the lockfile both resolves AND verifies the gem and no ledger entry already vendors it (that run never downloaded anything usable); the gem backend keeps the same refusal as the backstop for every other route into it. A run that would not have fetched at all is unaffected: an unverifiable lock entry keeps `vendor_fetch_unverifiable` + `package_not_installed`, and an already-vendored gem re-runs green. The detail names the remedy (`bundle install`, or `--vendor-source=auto`). Because the pre-fetch refusal skips the download, that run no longer emits the `vendor_fetched_missing` warning it used to emit before failing. | | `redirect_pypi_stale_install` | `redirect.warnings[]` (warning) | Hosted Python redirect: readable installed files differ from patched hashes. Read-only, repeated on re-scan, and excludes the package from same-run VEX. See the "Python stale-install guard" section. | | `redirect_gem_stale_install` | `redirect.warnings[]` (warning) | scan `--mode hosted` (gem): a stale UNPATCHED materialization (installed gem, or committed `vendor/cache` archive) that `bundle install` will reuse instead of fetching the redirected patch; the detail carries the verified remedy. Full rules and flavors: the "Gem stale-install guard" section. | | `redirect_pipenv_refused` | `redirect.warnings[]` (warning) | scan `--mode hosted` (pipenv): the Pipfile.lock pins another version or a non-registry / foreign source for the package — refused atomically across categories, and the patch is vetoed from the sibling Python rewriters (see the "Pipenv hosted redirect" section). | diff --git a/crates/socket-patch-cli/src/json_envelope.rs b/crates/socket-patch-cli/src/json_envelope.rs index 76b55db7..8b22bc52 100644 --- a/crates/socket-patch-cli/src/json_envelope.rs +++ b/crates/socket-patch-cli/src/json_envelope.rs @@ -201,6 +201,14 @@ pub struct PatchEvent { /// Stable, lowercase, snake_case reason tag for programmatic routing. /// Examples: `already_patched`, `package_not_installed`, /// `hash_mismatch`, `no_local_source`, `paid_required`. + /// + /// A code may be reported at either level: `no_local_source` arrives + /// HERE (per package, envelope `partialFailure`, no top-level `error`) + /// when vendored staging could not obtain one patch's content while + /// another staged, and as the top-level `error.code` (`status: + /// "error"`, empty `events[]`) when NOTHING in the manifest can be + /// staged — including a one-patch manifest. See CLI_CONTRACT.md's + /// error-code table. #[serde(skip_serializing_if = "Option::is_none")] pub error_code: Option, /// Underlying error message for `Failed` events. From 5f5984365d9711a198877b2c4c890216dd747b3f Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Fri, 25 Sep 2026 05:27:18 -0400 Subject: [PATCH 12/12] fix(vendor): count the extra contentless files once, not twice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `plural()` already carries the count ("2 files"), so the multi-file arm of the staging reason read "(and 2 other 2 files)". Say "(and 2 more files)", and pin all three arms — none, one, several — with a unit test, since that string is the only machine-readable explanation a `--json` consumer gets for a per-package `no_local_source`. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/commands/fetch_stage.rs | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/fetch_stage.rs b/crates/socket-patch-cli/src/commands/fetch_stage.rs index 62a6d8e2..211c2f98 100644 --- a/crates/socket-patch-cli/src/commands/fetch_stage.rs +++ b/crates/socket-patch-cli/src/commands/fetch_stage.rs @@ -770,8 +770,8 @@ fn contentless_reason(contentless: &[&str]) -> Option { Some(match rest.len() { 0 => format!("the patch view served no blob content for {first}"), n => format!( - "the patch view served no blob content for {first} (and {n} other {})", - plural(n, "file", "files") + "the patch view served no blob content for {first} (and {n} more file{})", + if n == 1 { "" } else { "s" } ), }) } @@ -780,6 +780,27 @@ fn contentless_reason(contentless: &[&str]) -> Option { mod tests { use super::*; + /// The per-package `no_local_source` detail is the ONE machine-readable + /// explanation a `--json` consumer gets (every human channel in the + /// stager is gated on `!--json`), so its wording is pinned here — + /// including the count, which `plural` already carries. + #[test] + fn contentless_reason_names_the_file_and_counts_the_rest() { + assert_eq!(contentless_reason(&[]), None); + assert_eq!( + contentless_reason(&["package/index.js"]).as_deref(), + Some("the patch view served no blob content for package/index.js") + ); + assert_eq!( + contentless_reason(&["a.js", "b.js"]).as_deref(), + Some("the patch view served no blob content for a.js (and 1 more file)") + ); + assert_eq!( + contentless_reason(&["a.js", "b.js", "c.js"]).as_deref(), + Some("the patch view served no blob content for a.js (and 2 more files)") + ); + } + #[test] fn progress_lines_name_no_internal_tags() { assert_eq!(