Skip to content

Commit 5dae1dc

Browse files
mikolalysenkoclaude
andcommitted
fix: 10 prod bugs + 7 test-harness bugs surfaced by a no-fail-fast full-suite sweep
CI has been failing one binary per run (cargo fail-fast hides every later binary), so a local `cargo test --workspace --all-features --no-fail-fast` sweep (155 targets) was run to flush the whole backlog at once. Prod fixes: - scan/hosted.rs: --silent swallowed the redirect-VEX failure ("errors only", never "nothing" — exit 1 with no message is undiagnosable), and human mode never printed the rewriter's own warnings (JSON already carried them; e.g. the no-lockfile warning). - lock_cli.rs: lock_held/lock_io errors were muted under --silent; the error and its remediation hint now always print in human mode. - get.rs: the selection_required error advised `--id <UUID>`, a value-taking form the CLI rejects (--id is a boolean type-tag); it now directs re-running with the chosen UUID as the identifier. - repair_vendor.rs: rebuilds from an installed copy for RECONSTRUCTED entries (ledger gone) were never verified against the rewired lockfile's integrity — a drifted/tampered copy was blessed into a fresh ledger. Register the wired integrity for those candidates (same trust anchor as the unverified-registry rung), and snapshot/restore the wiring files so a rejected rebuild can't leave the backend's integrity refresh behind. - cargo_config.rs: wrote into .cargo/config.toml when a legacy .cargo/config coexists — cargo reads the extensionless file and warns, so the [patch] entry was silently inert; and the atomic write reset the destination's permission bits (now mode-preserving, matching the other lock writers). - cargo.rs: re-vendoring a new patch uuid over live wiring failed on the already-detached lock entry and unwound the live config entry (bricking builds); a detach failure mid-re-vendor dropped the prior socket entry instead of restoring it; and an artifact-only rebuild of a wholesale- deleted uuid dir never restored the committed marker file. - pypi.rs/pypi_requirements.rs: the requirements flavor had no in-sync or stale-uuid pre-flight (unlike uv/poetry/pdm/pipenv) — every re-run appended a duplicate wheel line and clobbered the ledger record. Same uuid is now the in-sync/artifact-rebuild path; a different uuid refuses (pypi_requirements_already_vendored) before any writes. - pth_hook/detect.rs + setup.rs: the lockfile refresh ran bare `poetry lock` / `pdm lock`, re-resolving the user's whole pinned set on Poetry 1.x; the refresh now tries the pin-preserving spelling first (`lock --no-update` / `lock --update-reuse`) with bare `lock` as the fallback for versions that dropped the flag. Test-harness fixes (hermeticity/scrub bugs that fail suites on their own): - cli_parse_vendor/e2e_safety_unlock/remove_rollback_api_overrides: env scrub lists were missing SOCKET_VENDOR_SOURCE / SOCKET_VENDOR_URL / SOCKET_PATCH_SERVER_URL / SOCKET_STRICT (drift guards demanded them); remove_rollback also gains the standard drift-guard test. - setup_matrix_deno.rs: the roundtrip plants SOCKET_STRICT decoys its own fixed-list scrub didn't strip; converted to the prefix scrub its pypi sibling uses (with the telemetry force-off). - e2e_golang.rs: hostile env seeds were added without the paired env_remove lines, so SOCKET_ECOSYSTEMS=npm leaked and filtered the go crawler out of every scan. - e2e_vendor_yarn_classic_build.rs: corepack() scrubbed env AFTER applying caller overrides, wiping its own YARN_CACHE_FOLDER (the fixture install silently used the global cache); scrub-then-seed like the berry sibling. - scan_vendor_e2e.rs: run_cli_env never scrubbed or disabled telemetry, so runs phoned the configured endpoint (and the live API in ambient-env shells); prefix scrub + SOCKET_TELEMETRY_DISABLED=1, caller env last. Verified: full workspace sweep + targeted reruns of every affected binary (all green), core lib 1895 tests, cli lib 318 tests, clippy -D warnings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 8f4d150 commit 5dae1dc

17 files changed

Lines changed: 357 additions & 144 deletions

crates/socket-patch-cli/src/commands/get.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -587,7 +587,7 @@ pub(crate) fn select_patches(
587587
"{}",
588588
serde_json::to_string_pretty(&serde_json::json!({
589589
"status": "selection_required",
590-
"error": format!("Multiple patches available for {purl}. Specify --id <UUID> to select one."),
590+
"error": format!("Multiple patches available for {purl}. Re-run with the chosen UUID as the identifier (`socket-patch get <uuid>`) to select one."),
591591
"purl": purl,
592592
"options": options_json,
593593
}))

crates/socket-patch-cli/src/commands/lock_cli.rs

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -115,12 +115,12 @@ pub(crate) fn acquire_or_emit(
115115
} else {
116116
(held_message(timeout), Hint::UnlockOrBreakLock)
117117
};
118-
emit(command, json, silent, dry_run, "lock_held", &msg, hint);
118+
emit(command, json, dry_run, "lock_held", &msg, hint);
119119
Err(1)
120120
}
121121
Err(LockError::Io { path, source }) => {
122122
let msg = format!("failed to open lock file at {}: {}", path.display(), source);
123-
emit(command, json, silent, dry_run, "lock_io", &msg, Hint::None);
123+
emit(command, json, dry_run, "lock_io", &msg, Hint::None);
124124
Err(1)
125125
}
126126
}
@@ -203,21 +203,17 @@ enum Hint {
203203
UnlockOrBreakLock,
204204
}
205205

206-
fn emit(
207-
command: Command,
208-
json: bool,
209-
silent: bool,
210-
dry_run: bool,
211-
code: &str,
212-
message: &str,
213-
hint: Hint,
214-
) {
206+
fn emit(command: Command, json: bool, dry_run: bool, code: &str, message: &str, hint: Hint) {
215207
if json {
216208
println!(
217209
"{}",
218210
error_envelope(command, dry_run, code, message).to_pretty_json()
219211
);
220-
} else if !silent {
212+
} else {
213+
// Errors print even under --silent ("errors only", never "nothing"
214+
// — CLI_CONTRACT.md): exit 1 with no message would be
215+
// undiagnosable. The remediation hint is part of the error report,
216+
// not informational chatter, so it prints with the error.
221217
eprintln!("Error: {message}.");
222218
match hint {
223219
Hint::None => {}

crates/socket-patch-cli/src/commands/repair_vendor.rs

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -504,7 +504,23 @@ pub(crate) async fn repair_vendored_artifacts(
504504
let mut must_verify: HashMap<String, lock_inventory::LockIntegrity> = HashMap::new();
505505
for c in &candidates {
506506
if all_packages.contains_key(&c.purl) {
507-
continue; // installed copy: works offline too
507+
// Installed copy: works offline too. But for a RECONSTRUCTED
508+
// entry the copy is an unverified source — the ledger that
509+
// recorded the artifact sha is gone, so the rewired lockfile's
510+
// integrity is the ONLY trust anchor. A copy that drifted since
511+
// vendoring (build-tool artifacts, edited unpatched files) packs
512+
// into a tarball the package manager would reject on its next
513+
// install; register the wired integrity so the rebuilt artifact
514+
// is verified below, exactly like the unverified-registry rung.
515+
if c.reconstructed {
516+
if let Some(wired) =
517+
lock_inventory::wired_vendor_integrity(&common.cwd, &c.entry.artifact.path)
518+
.await
519+
{
520+
must_verify.insert(c.purl.clone(), wired);
521+
}
522+
}
523+
continue;
508524
}
509525
if common.offline {
510526
fail(
@@ -590,6 +606,31 @@ pub(crate) async fn repair_vendored_artifacts(
590606
let Some(pkg_path) = all_packages.get(&c.purl).cloned() else {
591607
continue; // failed above
592608
};
609+
// For an unverified-source rebuild the rewired lockfile is the trust
610+
// anchor: snapshot the wiring files so a failed post-verify can put
611+
// them back byte-for-byte. The backend's re-wire may refresh the
612+
// recorded integrity/checksum to the rebuilt tarball's — blessing
613+
// exactly the drifted bytes the verify below is about to reject.
614+
let wiring_snapshot: Option<Vec<(std::path::PathBuf, Vec<u8>)>> =
615+
if must_verify.contains_key(&c.purl) {
616+
let mut snap = Vec::new();
617+
for name in [
618+
"package-lock.json",
619+
"npm-shrinkwrap.json",
620+
"pnpm-lock.yaml",
621+
"yarn.lock",
622+
"bun.lock",
623+
"package.json",
624+
] {
625+
let p = common.cwd.join(name);
626+
if let Ok(bytes) = tokio::fs::read(&p).await {
627+
snap.push((p, bytes));
628+
}
629+
}
630+
Some(snap)
631+
} else {
632+
None
633+
};
593634
let outcome = dispatch_vendor_one(
594635
&c.purl,
595636
&pkg_path,
@@ -652,6 +693,14 @@ pub(crate) async fn repair_vendored_artifacts(
652693
};
653694
if let Err(detail) = verdict {
654695
remove_vendor_dir(&common.cwd, &c.entry.ecosystem, &c.entry.uuid).await;
696+
// Put the trust anchor back exactly as it was: the
697+
// backend's re-wire may have refreshed the recorded
698+
// integrity to the rejected rebuild's.
699+
if let Some(snap) = &wiring_snapshot {
700+
for (path, bytes) in snap {
701+
let _ = tokio::fs::write(path, bytes).await;
702+
}
703+
}
655704
fail(
656705
env,
657706
quiet,

crates/socket-patch-cli/src/commands/scan/hosted.rs

Lines changed: 45 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -486,42 +486,53 @@ pub(super) async fn run_redirect(
486486
result["error"] = serde_json::json!({ "code": code, "message": message });
487487
}
488488
println!("{}", serde_json::to_string_pretty(&result).unwrap());
489-
} else if !args.common.silent {
490-
let verb = if args.common.dry_run {
491-
"would rewrite"
492-
} else {
493-
"rewrote"
494-
};
495-
println!(
496-
"Redirected {} package(s); {verb} {} file(s).",
497-
confirmed.len(),
498-
rewritten.len()
499-
);
500-
for s in &skipped {
501-
eprintln!(" skipped {} ({})", s["purl"], s["reason"]);
502-
}
503-
for w in &record_warnings {
504-
eprintln!(" warning: {}", w["detail"]);
505-
}
506-
for w in &migration_warnings {
507-
eprintln!(" warning: {}", w["detail"]);
508-
}
509-
for w in &rush_warnings {
510-
eprintln!(" warning: {}", w["detail"]);
511-
}
512-
if let Some(statements) = vex_statements {
513-
eprintln!(
514-
"Wrote OpenVEX document with {} statement(s) to {} (redirected patches are \
515-
attested from the ledger, not hash-verified — their bytes are fetched at \
516-
install time; run `socket-patch vex` after installing to verify against \
517-
the installed tree).",
518-
statements,
519-
args.vex.vex.as_ref().unwrap().display(),
489+
} else {
490+
if !args.common.silent {
491+
let verb = if args.common.dry_run {
492+
"would rewrite"
493+
} else {
494+
"rewrote"
495+
};
496+
println!(
497+
"Redirected {} package(s); {verb} {} file(s).",
498+
confirmed.len(),
499+
rewritten.len()
520500
);
521-
} else if let Some((_, message)) = &vex_error {
501+
for s in &skipped {
502+
eprintln!(" skipped {} ({})", s["purl"], s["reason"]);
503+
}
504+
// Same warning set as the JSON envelope, same order: the
505+
// rewriter's own warnings first (e.g. `no package-lock.json`),
506+
// then the record/migration/rush extras.
507+
for w in &rewrite.warnings {
508+
eprintln!(" warning: {}", w.detail);
509+
}
510+
for w in &record_warnings {
511+
eprintln!(" warning: {}", w["detail"]);
512+
}
513+
for w in &migration_warnings {
514+
eprintln!(" warning: {}", w["detail"]);
515+
}
516+
for w in &rush_warnings {
517+
eprintln!(" warning: {}", w["detail"]);
518+
}
519+
if let Some(statements) = vex_statements {
520+
eprintln!(
521+
"Wrote OpenVEX document with {} statement(s) to {} (redirected patches are \
522+
attested from the ledger, not hash-verified — their bytes are fetched at \
523+
install time; run `socket-patch vex` after installing to verify against \
524+
the installed tree).",
525+
statements,
526+
args.vex.vex.as_ref().unwrap().display(),
527+
);
528+
} else if args.vex.vex.is_some() && args.common.dry_run {
529+
eprintln!("Skipping VEX generation (--dry-run).");
530+
}
531+
}
532+
// Errors print even under --silent ("errors only", never
533+
// "nothing"): exit 1 with no message would be undiagnosable.
534+
if let Some((_, message)) = &vex_error {
522535
eprintln!("Error: VEX generation failed: {message}");
523-
} else if args.vex.vex.is_some() && args.common.dry_run {
524-
eprintln!("Skipping VEX generation (--dry-run).");
525536
}
526537
}
527538
vex_code

crates/socket-patch-cli/src/commands/setup.rs

Lines changed: 40 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -472,8 +472,13 @@ async fn finalize_python(plan: &PythonPlan, edits: &[PthEditResult], cwd: &Path)
472472
return warnings;
473473
}
474474
// Lockfile refresh (broad auto-edit): only when the manager uses a lockfile
475-
// that exists. Best-effort — never fatal.
476-
if let Some((program, args)) = plan.pm.lock_command() {
475+
// that exists. Best-effort — never fatal. The spellings are tried in
476+
// order: pin-preserving first (`poetry lock --no-update`,
477+
// `pdm lock --update-reuse`), bare `lock` as the fallback for versions
478+
// that dropped the flag (Poetry 2.x, where bare `lock` is already
479+
// pin-preserving). A successful fallback is not a failure — only warn
480+
// when every spelling failed.
481+
if let Some((program, spellings)) = plan.pm.lock_commands() {
477482
let lockfile = match plan.pm {
478483
PythonPackageManager::Uv => Some("uv.lock"),
479484
PythonPackageManager::Poetry => Some("poetry.lock"),
@@ -485,22 +490,39 @@ async fn finalize_python(plan: &PythonPlan, edits: &[PthEditResult], cwd: &Path)
485490
None => false,
486491
};
487492
if lock_present {
488-
match tokio::process::Command::new(program)
489-
.args(args)
490-
.current_dir(cwd)
491-
.output()
492-
.await
493-
{
494-
Ok(o) if o.status.success() => {}
495-
Ok(o) => warnings.push(format!(
496-
"`{program} {}` failed ({}); update the lockfile manually",
497-
args.join(" "),
498-
o.status
499-
)),
500-
Err(e) => warnings.push(format!(
501-
"could not run `{program} {}`: {e}; update the lockfile manually",
502-
args.join(" ")
503-
)),
493+
let mut failure: Option<String> = None;
494+
for args in spellings {
495+
match tokio::process::Command::new(program)
496+
.args(*args)
497+
.current_dir(cwd)
498+
.output()
499+
.await
500+
{
501+
Ok(o) if o.status.success() => {
502+
failure = None;
503+
break;
504+
}
505+
Ok(o) => {
506+
failure = Some(format!(
507+
"`{program} {}` failed ({}); update the lockfile manually",
508+
args.join(" "),
509+
o.status
510+
));
511+
}
512+
Err(e) => {
513+
// The program itself didn't spawn (not installed /
514+
// not on PATH): retrying another spelling of the
515+
// same program is pointless.
516+
failure = Some(format!(
517+
"could not run `{program} {}`: {e}; update the lockfile manually",
518+
args.join(" ")
519+
));
520+
break;
521+
}
522+
}
523+
}
524+
if let Some(w) = failure {
525+
warnings.push(w);
504526
}
505527
}
506528
}

crates/socket-patch-cli/tests/cli_parse_vendor.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,11 @@ const SOCKET_ENV_VARS: &[&str] = &[
4343
"SOCKET_PROXY_URL",
4444
"SOCKET_ECOSYSTEMS",
4545
"SOCKET_DOWNLOAD_MODE",
46+
"SOCKET_VENDOR_SOURCE",
47+
"SOCKET_VENDOR_URL",
48+
"SOCKET_PATCH_SERVER_URL",
4649
"SOCKET_OFFLINE",
50+
"SOCKET_STRICT",
4751
"SOCKET_GLOBAL",
4852
"SOCKET_GLOBAL_PREFIX",
4953
"SOCKET_JSON",

crates/socket-patch-cli/tests/e2e_golang.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,11 @@ async fn run(args: &[&str], cwd: &Path, gomodcache: &Path, api_url: &str) -> Out
8888
.env("SOCKET_GLOBAL_PREFIX", "/nonexistent")
8989
.env("SOCKET_JSON", "true")
9090
.env("SOCKET_SILENT", "true")
91+
.env_remove("SOCKET_ECOSYSTEMS")
92+
.env_remove("SOCKET_GLOBAL")
93+
.env_remove("SOCKET_GLOBAL_PREFIX")
94+
.env_remove("SOCKET_JSON")
95+
.env_remove("SOCKET_SILENT")
9196
.env_remove("GOPATH")
9297
.env_remove("SOCKET_OFFLINE")
9398
.env_remove("SOCKET_PROXY_URL")

crates/socket-patch-cli/tests/e2e_safety_unlock.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,11 @@ const SOCKET_ENV_VARS: &[&str] = &[
4040
"SOCKET_PROXY_URL",
4141
"SOCKET_ECOSYSTEMS",
4242
"SOCKET_DOWNLOAD_MODE",
43+
"SOCKET_VENDOR_SOURCE",
44+
"SOCKET_VENDOR_URL",
45+
"SOCKET_PATCH_SERVER_URL",
4346
"SOCKET_OFFLINE",
47+
"SOCKET_STRICT",
4448
"SOCKET_GLOBAL",
4549
"SOCKET_GLOBAL_PREFIX",
4650
"SOCKET_JSON",

crates/socket-patch-cli/tests/e2e_vendor_yarn_classic_build.rs

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,16 @@ fn has_corepack_pm(pm: &str) -> bool {
6363
/// prompt disabled, and every `SOCKET_*` var scrubbed.
6464
fn corepack(cwd: &Path, pm: &str, args: &[&str], extra_env: &[(&str, &str)]) -> Output {
6565
let mut cmd = Command::new("corepack");
66-
cmd.arg(pm)
67-
.args(args)
68-
.current_dir(cwd)
69-
.env("COREPACK_ENABLE_DOWNLOAD_PROMPT", "0");
66+
cmd.arg(pm).args(args).current_dir(cwd);
67+
// Scrub FIRST (it removes YARN_CACHE_FOLDER / SOCKET_* from the inherited
68+
// env), then seed the hermetic flags so they survive (Command: last env
69+
// call wins). Scrubbing last wiped the caller's private cache override,
70+
// so the fixture install silently used the developer's global cache.
71+
scrub_socket_env(&mut cmd);
72+
cmd.env("COREPACK_ENABLE_DOWNLOAD_PROMPT", "0");
7073
for (k, v) in extra_env {
7174
cmd.env(k, v);
7275
}
73-
scrub_socket_env(&mut cmd);
7476
cmd.output().expect("failed to run corepack")
7577
}
7678

crates/socket-patch-cli/tests/remove_rollback_api_overrides.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,11 @@ const SOCKET_ENV_VARS: &[&str] = &[
4040
"SOCKET_PROXY_URL",
4141
"SOCKET_ECOSYSTEMS",
4242
"SOCKET_DOWNLOAD_MODE",
43+
"SOCKET_VENDOR_SOURCE",
44+
"SOCKET_VENDOR_URL",
45+
"SOCKET_PATCH_SERVER_URL",
4346
"SOCKET_OFFLINE",
47+
"SOCKET_STRICT",
4448
"SOCKET_GLOBAL",
4549
"SOCKET_GLOBAL_PREFIX",
4650
"SOCKET_JSON",
@@ -56,6 +60,21 @@ const SOCKET_ENV_VARS: &[&str] = &[
5660
"SOCKET_SKIP_ROLLBACK",
5761
];
5862

63+
/// Drift guard: the scrub must cover every env var `GlobalArgs` binds — the
64+
/// production `GLOBAL_ARG_ENV_VARS` list is the source of truth. A var
65+
/// missing here escapes the scrub, so an ambient value in the developer's
66+
/// shell or CI (e.g. `SOCKET_STRICT=garbage`) aborts every invocation in
67+
/// this file. Mirrors `cli_parse_vendor.rs`.
68+
#[test]
69+
fn env_scrub_covers_every_global_arg_env_var() {
70+
for var in socket_patch_cli::args::GLOBAL_ARG_ENV_VARS {
71+
assert!(
72+
SOCKET_ENV_VARS.contains(var),
73+
"{var} is bound by GlobalArgs but missing from SOCKET_ENV_VARS — the scrub won't strip it",
74+
);
75+
}
76+
}
77+
5978
/// Git-SHA256: SHA256("blob <len>\0" ++ content).
6079
fn git_sha256(content: &[u8]) -> String {
6180
let header = format!("blob {}\0", content.len());

0 commit comments

Comments
 (0)