Skip to content

Commit b194fa1

Browse files
mikolalysenkoclaudemikolalysenko
authored
Fix vendor --check passing unwired vendored entries (#725) (#730)
* Start fix for #725 Assisted-by: Claude Code:claude-opus-5-5 * Fail vendor --check when lock drops vendored ref `vendor --check` only verified wiring for Maven/Gradle entries. For every other ecosystem it reported "committed artifact and wiring verified" and exited 0 even after `pipenv lock`, `uv lock`, `npm install` or a hand edit pointed the lockfile back at the registry, so a CI gate stayed green while fresh installs got the unpatched package and `vex` refused the same checkout. Each non-JVM entry is now judged by the same vendor-ledger liveness rule `vex` and `scan` use; an unwired entry fails with `vendor_check_failed` and exit 1. Regression tests cover Pipenv, requirements.txt, Poetry, uv, Hatch and npm. Fixes #725 Assisted-by: Claude Code:claude-opus-5-5 * Run npm wiring check before generic liveness in vendor --check The npm package-lock check (#589) names the exact unwired lock entry; running the generic liveness rule first replaced that reason, failing e2e_vex_vendor after merging main. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01519c1ZV6MhuxyVisVJ5FYz --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: mikolalysenko <mik@socket.dev>
1 parent ddd283e commit b194fa1

4 files changed

Lines changed: 185 additions & 17 deletions

File tree

‎crates/socket-patch-cli/CLI_CONTRACT.md‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1654,7 +1654,11 @@ See [the JVM design](../../docs/design/maven-vendoring.md) for supported shapes.
16541654

16551655
`vendor --check` is an offline, read-only audit. Healthy entries emit `verified`
16561656
with `vendor_check_ok`; drift emits `failed` with `vendor_check_failed`, a
1657-
`partialFailure` envelope and exit 1. For a package-lock entry, drift includes a
1657+
`partialFailure` envelope and exit 1. Drift covers the committed artifact and its
1658+
wiring: an entry whose lockfile or config no longer references its
1659+
`.socket/vendor/` artifact (for example after `pipenv lock`, `uv lock` or
1660+
`npm install` re-resolved it) fails by the same liveness rule as `vex`'s
1661+
`vendor_unwired`. For a package-lock entry, drift also includes a
16581662
`package-lock.json` / `npm-shrinkwrap.json` entry for the vendored `name@version`
16591663
that `vendor` would rewire but that does not resolve to the vendored artifact
16601664
(#588); the reason names that entry. Missing ledger entries fail with

‎crates/socket-patch-cli/src/commands/vendor.rs‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -970,6 +970,17 @@ async fn run_check(args: &VendorArgs) -> i32 {
970970
};
971971
let mut entries: Vec<_> = state.entries.iter().collect();
972972
entries.sort_by_key(|(key, _)| *key);
973+
// The lockfile view `vex` and `scan` judge vendor-ledger liveness from;
974+
// JVM entries are checked against their own layout instead.
975+
let discovery = if state
976+
.entries
977+
.values()
978+
.any(|e| !vendor::jvm::apply::is_jvm_entry(e))
979+
{
980+
Some(crate::commands::discover_wiring(&args.common, root).await)
981+
} else {
982+
None
983+
};
973984
for (key, entry) in entries {
974985
let record = entry.record.as_ref().or_else(|| manifest.patches.get(key));
975986
let mut failure = match record {
@@ -982,11 +993,29 @@ async fn run_check(args: &VendorArgs) -> i32 {
982993
if failure.is_none() && vendor::jvm::apply::is_jvm_entry(entry) {
983994
failure = vendor::jvm::apply::check_entry(root, entry, local_repo.as_deref()).err();
984995
}
996+
// The npm check names the exact unwired lock entry, so it runs
997+
// before the generic liveness rule below.
985998
if failure.is_none() && entry.ecosystem == "npm" {
986999
failure = vendor::npm_flavor::check_npm_wiring(entry, root)
9871000
.await
9881001
.err();
9891002
}
1003+
if let (None, Some(discovery), false) = (
1004+
&failure,
1005+
&discovery,
1006+
vendor::jvm::apply::is_jvm_entry(entry),
1007+
) {
1008+
// A relock (`pipenv lock`, `npm install`, `uv lock`, …) can
1009+
// drop the `.socket/vendor/` reference while the artifact stays
1010+
// intact; a fresh install is then unpatched. Same rule as
1011+
// `vex`'s `vendor_unwired`.
1012+
if !discovery.vendor_entry_live(root, entry).await {
1013+
failure = Some(format!(
1014+
"wiring missing: no lockfile or config references .socket/vendor/{}/{} any more, so a fresh install gets the unpatched package; re-run `socket-patch vendor` to rewire it",
1015+
entry.ecosystem, entry.uuid
1016+
));
1017+
}
1018+
}
9901019
if vendor::jvm::apply::upstream_unverified(entry) {
9911020
env.warnings.push(RunWarning {code: "vendor_jvm_upstream_unverified".into(), detail: format!("{key}: upstream metadata was accepted offline; run vendor online to verify registry checksums")});
9921021
}

‎crates/socket-patch-cli/tests/in_process_vendor.rs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3992,6 +3992,33 @@ async fn revert_completes_when_lock_already_matches_the_original() {
39923992
assert!(state_gone, "ledger entry pruned once the revert converges");
39933993
}
39943994

3995+
/// REGRESSION (#725): `vendor --check` is the CI gate for vendored wiring,
3996+
/// but it only audited the committed tarball, so after `npm install`
3997+
/// re-resolved the lock to the registry it still printed "committed
3998+
/// artifact and wiring verified" and exited 0 while `vex` refused the same
3999+
/// checkout (`vendor_unwired`). It must fail the unwired entry.
4000+
#[tokio::test]
4001+
async fn vendor_check_fails_when_lock_no_longer_wires_artifact() {
4002+
let fx = npm_fixture();
4003+
assert_eq!(vendor_run(vendor_args(fx.root())).await, 0, "vendor");
4004+
let (code, env) = vendor_cli(fx.root(), &["--check"]);
4005+
assert_eq!(code, 0, "{env:#}");
4006+
find_event(&env, "verified", Some("vendor_check_ok"));
4007+
4008+
// The lock re-resolved to the registry; the artifact is untouched.
4009+
std::fs::write(fx.lock_path(), &fx.original_lock).unwrap();
4010+
assert!(fx.tgz_path().is_file());
4011+
let (code, env) = vendor_cli(fx.root(), &["--check"]);
4012+
assert_eq!(code, 1, "{env:#}");
4013+
let event = find_event(&env, "failed", Some("vendor_check_failed"));
4014+
assert!(
4015+
event["reason"]
4016+
.as_str()
4017+
.is_some_and(|r| r.contains("wiring")),
4018+
"{env:#}"
4019+
);
4020+
}
4021+
39954022
/// Manifest-less VEX over the committed state of an in-process npm
39964023
/// `vendor` (the in-process twin of `e2e_vendor_npm_build`'s tail): the
39974024
/// committed tarball is the evidence, so the checkout attests `(vendored)`

‎crates/socket-patch-cli/tests/mode_migration_pypi.rs‎

Lines changed: 124 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -297,6 +297,12 @@ python-versions = ">=3.9"
297297
content-hash = "4b42a89b7ff7b26511b06acdc458dbd85312e5083db8f212b017482bc68cdd01"
298298
"#;
299299

300+
/// A requirements.txt project; returns its wiring files.
301+
fn stage_requirements(root: &Path) -> &'static [&'static str] {
302+
std::fs::write(root.join("requirements.txt"), "idna==3.7\nsix==1.16.0\n").unwrap();
303+
&["requirements.txt"]
304+
}
305+
300306
/// #765: a vendored requirements.txt picks up a superseding patch. The
301307
/// manifest moves `six` from patch A to patch B (different patched bytes);
302308
/// the next `vendor` must re-wire the requirements line to B's wheel in
@@ -380,8 +386,8 @@ async fn requirements_vendored_revendors_superseding_patch() {
380386
#[tokio::test]
381387
async fn requirements_vendored_to_hosted() {
382388
let (_tmp, root) = project();
383-
std::fs::write(root.join("requirements.txt"), "idna==3.7\nsix==1.16.0\n").unwrap();
384-
assert_vendored_to_hosted(&root, &["requirements.txt"]).await;
389+
let files = stage_requirements(&root);
390+
assert_vendored_to_hosted(&root, files).await;
385391
}
386392

387393
#[tokio::test]
@@ -391,9 +397,8 @@ async fn requirements_sole_pin_vendored_to_hosted() {
391397
assert_vendored_to_hosted(&root, &["requirements.txt"]).await;
392398
}
393399

394-
#[tokio::test]
395-
async fn poetry_vendored_to_hosted() {
396-
let (_tmp, root) = project();
400+
/// A Poetry project; returns its wiring files.
401+
fn stage_poetry(root: &Path) -> &'static [&'static str] {
397402
std::fs::write(
398403
root.join("pyproject.toml"),
399404
"[tool.poetry]\nname = \"demo\"\nversion = \"0.1.0\"\ndescription = \"\"\nauthors = [\"x <x@x>\"]\npackage-mode = false\n\n[tool.poetry.dependencies]\npython = \">=3.9\"\nsix = \"1.16.0\"\n",
@@ -406,14 +411,20 @@ async fn poetry_vendored_to_hosted() {
406411
.replace("SDIST_SHA", SDIST_SHA),
407412
)
408413
.unwrap();
409-
assert_vendored_to_hosted(&root, &["poetry.lock", "pyproject.toml"]).await;
414+
&["poetry.lock", "pyproject.toml"]
410415
}
411416

412-
const PIPFILE: &str = "[[source]]\nurl = \"https://pypi.org/simple\"\nverify_ssl = true\nname = \"pypi\"\n\n[packages]\nsix = \"==1.16.0\"\n\n[requires]\npython_version = \"3.11\"\n";
413-
414417
#[tokio::test]
415-
async fn pipenv_vendored_to_hosted() {
418+
async fn poetry_vendored_to_hosted() {
416419
let (_tmp, root) = project();
420+
let files = stage_poetry(&root);
421+
assert_vendored_to_hosted(&root, files).await;
422+
}
423+
424+
const PIPFILE: &str = "[[source]]\nurl = \"https://pypi.org/simple\"\nverify_ssl = true\nname = \"pypi\"\n\n[packages]\nsix = \"==1.16.0\"\n\n[requires]\npython_version = \"3.11\"\n";
425+
426+
/// A Pipenv project; returns its wiring files.
427+
fn stage_pipenv(root: &Path) -> &'static [&'static str] {
417428
std::fs::write(root.join("Pipfile"), PIPFILE).unwrap();
418429
let lock = json!({
419430
"_meta": {
@@ -435,7 +446,14 @@ async fn pipenv_vendored_to_hosted() {
435446
let mut text = serde_json::to_string_pretty(&lock).unwrap();
436447
text.push('\n');
437448
std::fs::write(root.join("Pipfile.lock"), text).unwrap();
438-
assert_vendored_to_hosted(&root, &["Pipfile.lock"]).await;
449+
&["Pipfile.lock"]
450+
}
451+
452+
#[tokio::test]
453+
async fn pipenv_vendored_to_hosted() {
454+
let (_tmp, root) = project();
455+
let files = stage_pipenv(&root);
456+
assert_vendored_to_hosted(&root, files).await;
439457
}
440458

441459
const UV_LOCK: &str = r#"version = 1
@@ -463,9 +481,8 @@ wheels = [
463481
]
464482
"#;
465483

466-
#[tokio::test]
467-
async fn uv_vendored_to_hosted() {
468-
let (_tmp, root) = project();
484+
/// A uv project; returns its wiring files.
485+
fn stage_uv(root: &Path) -> &'static [&'static str] {
469486
std::fs::write(
470487
root.join("pyproject.toml"),
471488
"[project]\nname = \"demo\"\nversion = \"0.1.0\"\nrequires-python = \">=3.9\"\ndependencies = [\"six==1.16.0\"]\n",
@@ -478,18 +495,31 @@ async fn uv_vendored_to_hosted() {
478495
.replace("SDIST_SHA", SDIST_SHA),
479496
)
480497
.unwrap();
481-
assert_vendored_to_hosted(&root, &["uv.lock", "pyproject.toml"]).await;
498+
&["uv.lock", "pyproject.toml"]
482499
}
483500

484501
#[tokio::test]
485-
async fn hatch_vendored_to_hosted() {
502+
async fn uv_vendored_to_hosted() {
486503
let (_tmp, root) = project();
504+
let files = stage_uv(&root);
505+
assert_vendored_to_hosted(&root, files).await;
506+
}
507+
508+
/// A Hatch project; returns its wiring files.
509+
fn stage_hatch(root: &Path) -> &'static [&'static str] {
487510
std::fs::write(
488511
root.join("pyproject.toml"),
489512
"[build-system]\nrequires = [\"hatchling\"]\nbuild-backend = \"hatchling.build\"\n\n[project]\nname = \"demo\"\nversion = \"0.1.0\"\ndependencies = [\"six==1.16.0\"]\n",
490513
)
491514
.unwrap();
492-
assert_vendored_to_hosted(&root, &["pyproject.toml"]).await;
515+
&["pyproject.toml"]
516+
}
517+
518+
#[tokio::test]
519+
async fn hatch_vendored_to_hosted() {
520+
let (_tmp, root) = project();
521+
let files = stage_hatch(&root);
522+
assert_vendored_to_hosted(&root, files).await;
493523
}
494524

495525
/// The uv lock rewrite needs the hosted wheel's METADATA, fetched only
@@ -683,6 +713,84 @@ async fn ledger_update_failure_after_revert_is_stranded() {
683713
assert_eq!(code, 1, "{env:#}");
684714
}
685715

716+
// ── `vendor --check` wiring audit (#725) ─────────────────────────────────
717+
718+
/// Vendor the staged project, confirm `vendor --check` passes, then put
719+
/// the wiring files back to their pre-vendor bytes — what `pipenv lock`,
720+
/// `poetry lock`, `uv lock` or a hand-edited requirements.txt leave behind —
721+
/// and require `vendor --check` to fail: the committed wheel is intact, but
722+
/// nothing installs it any more, so a fresh install is unpatched.
723+
fn assert_check_catches_relock(root: &Path, files: &[&str]) {
724+
let pristine: Vec<Vec<u8>> = files
725+
.iter()
726+
.map(|f| std::fs::read(root.join(f)).unwrap())
727+
.collect();
728+
vendor_project(root, files);
729+
730+
let (code, env) = run_cli(root, &["vendor", "--check"], &[]);
731+
assert_eq!(code, 0, "wired project passes: {env:#}");
732+
assert_eq!(env["events"][0]["errorCode"], "vendor_check_ok", "{env:#}");
733+
734+
for (f, bytes) in files.iter().zip(&pristine) {
735+
std::fs::write(root.join(f), bytes).unwrap();
736+
}
737+
let (code, env) = run_cli(root, &["vendor", "--check"], &[]);
738+
assert_eq!(code, 1, "{files:?} no longer wire the artifact: {env:#}");
739+
let event = &env["events"][0];
740+
assert_eq!(event["action"], "failed", "{env:#}");
741+
assert_eq!(event["errorCode"], "vendor_check_failed", "{env:#}");
742+
assert!(
743+
event["reason"]
744+
.as_str()
745+
.is_some_and(|r| r.contains("wiring")),
746+
"the failure names the missing wiring: {env:#}"
747+
);
748+
assert_eq!(env["summary"]["failed"], 1, "{env:#}");
749+
}
750+
751+
#[tokio::test]
752+
async fn vendor_check_fails_after_pipenv_relock() {
753+
let (_tmp, root) = project();
754+
let files = stage_pipenv(&root);
755+
assert_check_catches_relock(&root, files);
756+
757+
// Human mode must not claim the wiring was verified.
758+
let (code, stdout, stderr) = run_raw(&root, &["vendor", "--check"], &[]);
759+
assert_eq!(code, 1, "stdout:\n{stdout}\nstderr:\n{stderr}");
760+
assert!(
761+
!stdout.contains("wiring verified"),
762+
"stdout:\n{stdout}\nstderr:\n{stderr}"
763+
);
764+
}
765+
766+
#[tokio::test]
767+
async fn vendor_check_fails_after_requirements_rewrite() {
768+
let (_tmp, root) = project();
769+
let files = stage_requirements(&root);
770+
assert_check_catches_relock(&root, files);
771+
}
772+
773+
#[tokio::test]
774+
async fn vendor_check_fails_after_poetry_relock() {
775+
let (_tmp, root) = project();
776+
let files = stage_poetry(&root);
777+
assert_check_catches_relock(&root, files);
778+
}
779+
780+
#[tokio::test]
781+
async fn vendor_check_fails_after_uv_relock() {
782+
let (_tmp, root) = project();
783+
let files = stage_uv(&root);
784+
assert_check_catches_relock(&root, files);
785+
}
786+
787+
#[tokio::test]
788+
async fn vendor_check_fails_after_hatch_dependency_reset() {
789+
let (_tmp, root) = project();
790+
let files = stage_hatch(&root);
791+
assert_check_catches_relock(&root, files);
792+
}
793+
686794
/// #699: hosted mode rewrites only the ROOT `requirements.txt`, while
687795
/// vendored mode also wires a pin in a `-r` include or appends a managed
688796
/// `(transitive)` line. A vendored → hosted takeover of such a pin used to

0 commit comments

Comments
 (0)