Fix Python env discovery ignoring uv/PDM env (#502, #525, #528) - #540
Open
Mikola Lysenko (mikolalysenko) wants to merge 6 commits into
Open
Mikola Lysenko (mikolalysenko) wants to merge 6 commits into
Mikola Lysenko (mikolalysenko) wants to merge 6 commits into
Conversation
This was referenced Oct 2, 2026
socket-patch picked a Python project's environment from VIRTUAL_ENV, Pipenv, Poetry, ./.venv and ./venv. It never asked uv or PDM where they installed the project. With UV_PROJECT_ENVIRONMENT set, a PDM .pdm-python pointing at an out-of-tree venv, or a PEP 582 __pypackages__ layout, agent mode patched a stray .venv or the PATH Python. It also skipped the package as not installed, the hosted stale-install warning stayed silent, and vex attested installs that were still unpatched. Discovery now checks the env the project's manager records before the generic probes: PDM's saved interpreter (its venv, or __pypackages__/<X.Y>/lib for a base interpreter or PDM 1.x), then UV_PROJECT_ENVIRONMENT for a project uv drives. Fixes #502, #525, #528. Assisted-by: Claude Code:claude-opus-5-5
Assisted-by: Claude Code:claude-opus-5-5
PDM disregards the saved .pdm-python interpreter when PDM_IGNORE_SAVED_PYTHON is set, so discovery does too and falls back to the generic probes. Assisted-by: Claude Code:claude-opus-5-5
The new PDM env probe opened pyproject.toml, .pdm-python and .pdm.toml with a plain read, so a FIFO in their place wedged scan forever (caught by hosted_scan_returns_with_fifo_candidate). Read them with read_regular_to_string, as the Poetry probe already does. Assisted-by: Claude Code:claude-opus-5-5
Mikola Lysenko (mikolalysenko)
marked this pull request as ready for review
October 2, 2026 08:18
Collaborator
Author
|
BugBot review Generated by Claude Code |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: PDM env shadows uv projects
- Added priority check in pdm_project_site_packages to return None when uv.lock or poetry.lock exists, ensuring uv and Poetry environments take precedence over PDM's saved interpreter.
Or push these changes by commenting:
@cursor push 810f39b188
Preview (810f39b188)
diff --git a/crates/socket-patch-cli/src/commands/apply.rs b/crates/socket-patch-cli/src/commands/apply.rs
--- a/crates/socket-patch-cli/src/commands/apply.rs
+++ b/crates/socket-patch-cli/src/commands/apply.rs
@@ -2,9 +2,7 @@
use socket_patch_core::api::blob_fetcher::get_missing_blobs;
use socket_patch_core::api::client::{get_api_client_with_overrides, ApiClient};
use socket_patch_core::crawlers::ruby_crawler::config_path_ignored_warning;
-use socket_patch_core::crawlers::{
- detect_npm_pkg_manager, Ecosystem, NpmPkgManager, RubyCrawler,
-};
+use socket_patch_core::crawlers::{detect_npm_pkg_manager, Ecosystem, NpmPkgManager, RubyCrawler};
use socket_patch_core::manifest::operations::read_manifest;
use socket_patch_core::manifest::schema::{PatchFileInfo, PatchManifest, PatchRecord};
use socket_patch_core::patch::apply::{
diff --git a/crates/socket-patch-cli/src/commands/list.rs b/crates/socket-patch-cli/src/commands/list.rs
--- a/crates/socket-patch-cli/src/commands/list.rs
+++ b/crates/socket-patch-cli/src/commands/list.rs
@@ -431,7 +431,10 @@
detail: detail.clone(),
});
} else if !args.common.silent {
- eprintln!("Warning: {}", crate::commands::rollback::capitalize_first(detail));
+ eprintln!(
+ "Warning: {}",
+ crate::commands::rollback::capitalize_first(detail)
+ );
}
}
let vendor_state = crate::commands::vendor_state_lenient(&loaded.vendor, args.common.silent);
@@ -773,12 +776,18 @@
let listings = HostedListing::from_pins(
&[
pin("pkg:npm/minimist@1.2.2", &record.uuid),
- pin("pkg:npm/other@1.0.0", "33333333-3333-4333-8333-333333333333"),
+ pin(
+ "pkg:npm/other@1.0.0",
+ "33333333-3333-4333-8333-333333333333",
+ ),
],
Some(&legacy),
);
assert_eq!(listings[0].record, record);
- assert_eq!(listings[1].record.uuid, "33333333-3333-4333-8333-333333333333");
+ assert_eq!(
+ listings[1].record.uuid,
+ "33333333-3333-4333-8333-333333333333"
+ );
assert!(listings[1].record.vulnerabilities.is_empty());
assert_eq!(listings[1].lockfiles, vec!["yarn.lock".to_string()]);
}
diff --git a/crates/socket-patch-cli/src/commands/mod.rs b/crates/socket-patch-cli/src/commands/mod.rs
--- a/crates/socket-patch-cli/src/commands/mod.rs
+++ b/crates/socket-patch-cli/src/commands/mod.rs
@@ -1,7 +1,7 @@
pub mod apply;
pub(crate) mod bun_preflight;
+pub(crate) mod composer_hints;
pub(crate) mod context;
-pub(crate) mod composer_hints;
pub(crate) mod fetch_stage;
pub mod get;
pub mod hosted_bundle;
@@ -9,11 +9,11 @@
pub(crate) mod lock_cli;
pub mod remove;
pub mod repair;
-pub(crate) mod vendored_backend;
pub mod rollback;
pub mod scan;
pub mod update;
pub mod vendor;
+pub(crate) mod vendored_backend;
pub mod vex;
pub(crate) mod vex_consumed;
pub(crate) mod vex_sources;
@@ -135,9 +135,11 @@
common: &crate::args::GlobalArgs,
root: &Path,
) -> socket_patch_core::patch::redirect::RedirectState {
- hosted_state_from_pins(&socket_patch_core::patch::redirect::upstream::HostedPin::all(
- &discover_wiring(common, root).await,
- ))
+ hosted_state_from_pins(
+ &socket_patch_core::patch::redirect::upstream::HostedPin::all(
+ &discover_wiring(common, root).await,
+ ),
+ )
}
/// [`hosted_state_from_lockfiles`] over already-discovered pins. A purl
@@ -147,10 +149,8 @@
) -> socket_patch_core::patch::redirect::RedirectState {
let mut state = socket_patch_core::patch::redirect::RedirectState::new();
for pin in pins {
- state
- .records
- .entry(pin.purl.clone())
- .or_insert_with(|| socket_patch_core::manifest::schema::PatchRecord {
+ state.records.entry(pin.purl.clone()).or_insert_with(|| {
+ socket_patch_core::manifest::schema::PatchRecord {
uuid: pin.uuid.clone(),
exported_at: String::new(),
files: Default::default(),
@@ -158,7 +158,8 @@
description: String::new(),
license: String::new(),
tier: String::new(),
- });
+ }
+ });
}
state
}
@@ -185,4 +186,3 @@
}
}
}
-
diff --git a/crates/socket-patch-cli/src/commands/remove.rs b/crates/socket-patch-cli/src/commands/remove.rs
--- a/crates/socket-patch-cli/src/commands/remove.rs
+++ b/crates/socket-patch-cli/src/commands/remove.rs
@@ -17,9 +17,9 @@
pin_before_hash_blobs, rollback_patches_inner, run_hosted_leg, sweep_failure,
sweep_unused_artifacts, HostedLegOutcome, InnerSelection,
};
-use crate::commands::vendored_backend::{RevertedEntry, VendorRevertStep, VendoredBackend};
use crate::args::{apply_env_toggles, GlobalArgs};
use crate::commands::lock_cli::acquire_or_emit;
+use crate::commands::vendored_backend::{RevertedEntry, VendorRevertStep, VendoredBackend};
use crate::json_envelope::{Command, Envelope, EnvelopeError, PatchAction, PatchEvent, Status};
use crate::ui::plural;
diff --git a/crates/socket-patch-cli/src/commands/rollback.rs b/crates/socket-patch-cli/src/commands/rollback.rs
--- a/crates/socket-patch-cli/src/commands/rollback.rs
+++ b/crates/socket-patch-cli/src/commands/rollback.rs
@@ -10,13 +10,13 @@
};
use socket_patch_core::manifest::schema::{PatchFileInfo, PatchManifest, PatchRecord};
use socket_patch_core::patch::apply::select_installed_variants;
+use socket_patch_core::patch::redirect::upstream::HostedPin;
use socket_patch_core::patch::rollback::{
cannot_rollback_error, rollback_package_patch, verify_file_rollback, RollbackResult,
VerifyRollbackResult, VerifyRollbackStatus,
};
use socket_patch_core::telemetry::{track_patch_rollback_failed, track_patch_rolled_back};
use socket_patch_core::utils::purl::{patch_matches, strip_purl_qualifiers};
-use socket_patch_core::patch::redirect::upstream::HostedPin;
use socket_patch_core::vendor::{purl_keys_cover, RevertOpts, VendorState};
use std::collections::{HashMap, HashSet};
use std::path::{Path, PathBuf};
@@ -1026,7 +1026,8 @@
.iter()
.map(|(code, detail)| (code.to_string(), detail.clone())),
);
- out.edited_files.extend(outcome.reverted_files.iter().cloned());
+ out.edited_files
+ .extend(outcome.reverted_files.iter().cloned());
let unwound: Vec<_> = vlt_targets
.into_iter()
.filter(|t| out.reverted.iter().any(|p| p == &t.purl))
@@ -1170,7 +1171,11 @@
} else if !args.common.silent {
println!(
"{} the pre-v5 hosted ledger {}: no lockfile pins a hosted patch.",
- if args.common.dry_run { "Would remove" } else { "Removed" },
+ if args.common.dry_run {
+ "Would remove"
+ } else {
+ "Removed"
+ },
socket_patch_core::patch::redirect::REDIRECT_STATE_REL
);
}
diff --git a/crates/socket-patch-cli/src/commands/scan/hosted.rs b/crates/socket-patch-cli/src/commands/scan/hosted.rs
--- a/crates/socket-patch-cli/src/commands/scan/hosted.rs
+++ b/crates/socket-patch-cli/src/commands/scan/hosted.rs
@@ -934,7 +934,8 @@
socket_patch_core::utils::fs::read_regular_to_string_sync(path).ok()
})
};
- let rewrite_options = || RewriteOptions {
+ let rewrite_options = || {
+ RewriteOptions {
dry_run: common.dry_run,
targets_pipenv_lock,
pipenv_major,
@@ -946,6 +947,7 @@
npm_allow_remote_config: !common.no_npm_allow_remote_config,
npm_outer: &npm_outer,
blocking: true,
+ }
};
// The rollout gate plans again without its deferred rows: keep what
// the second pass needs.
@@ -2171,13 +2173,19 @@
/// artifacts, then verify with `vex`. After a vendored→hosted takeover
/// (`vendored_removed`) the commit also has to carry the deleted vendored
/// ledger entries and artifacts.
-fn format_next_steps(files: &[String], edits: &[socket_patch_core::patch::redirect::FileEdit], vendored_removed: bool) -> Vec<String> {
+fn format_next_steps(
+ files: &[String],
+ edits: &[socket_patch_core::patch::redirect::FileEdit],
+ vendored_removed: bool,
+) -> Vec<String> {
if files.is_empty() && !vendored_removed {
return Vec::new();
}
let mut commit: Vec<String> = Vec::new();
if vendored_removed {
- commit.push(".socket/vendor/ (the removed vendored ledger entries and artifacts)".to_string());
+ commit.push(
+ ".socket/vendor/ (the removed vendored ledger entries and artifacts)".to_string(),
+ );
}
commit.extend(files.iter().cloned());
let npm = files
@@ -4162,19 +4170,43 @@
use super::npm_allow_remote_one_line;
let hosts = ["patch.socket.dev"];
let cases = [
- (npm_allow_remote_configured_detail(&hosts, true, false), "Note: set"),
- (npm_allow_remote_configured_detail(&hosts, false, false), "Note: set"),
- (npm_allow_remote_configured_detail(&hosts, true, true), "Note: would set"),
- (npm_allow_remote_already_detail(&hosts), "Note: .npmrc already"),
- (npm_allow_remote_user_set_detail(&hosts, "none"), "Warning: npm >=12"),
- (npm_allow_remote_env_set_detail(&hosts, "npm_config_allow_remote", "none"), "Warning: npm >=12"),
+ (
+ npm_allow_remote_configured_detail(&hosts, true, false),
+ "Note: set",
+ ),
+ (
+ npm_allow_remote_configured_detail(&hosts, false, false),
+ "Note: set",
+ ),
+ (
+ npm_allow_remote_configured_detail(&hosts, true, true),
+ "Note: would set",
+ ),
+ (
+ npm_allow_remote_already_detail(&hosts),
+ "Note: .npmrc already",
+ ),
+ (
+ npm_allow_remote_user_set_detail(&hosts, "none"),
+ "Warning: npm >=12",
+ ),
+ (
+ npm_allow_remote_env_set_detail(&hosts, "npm_config_allow_remote", "none"),
+ "Warning: npm >=12",
+ ),
(npm_allow_remote_manual_detail(&hosts), "Warning: npm >=12"),
- (npm_allow_remote_unreadable_detail(&hosts, "is a symlink"), "Warning: npm >=12"),
+ (
+ npm_allow_remote_unreadable_detail(&hosts, "is a symlink"),
+ "Warning: npm >=12",
+ ),
];
for (detail, start) in cases {
let line = npm_allow_remote_one_line(&detail);
assert!(line.starts_with(start), "{line}");
- assert!(!line.contains('\n') && line.ends_with("(details: --verbose)."), "{line}");
+ assert!(
+ !line.contains('\n') && line.ends_with("(details: --verbose)."),
+ "{line}"
+ );
}
}
}
diff --git a/crates/socket-patch-cli/src/commands/scan/mod.rs b/crates/socket-patch-cli/src/commands/scan/mod.rs
--- a/crates/socket-patch-cli/src/commands/scan/mod.rs
+++ b/crates/socket-patch-cli/src/commands/scan/mod.rs
@@ -35,17 +35,17 @@
use super::get::{download_and_apply_patches_with, DownloadParams, DownloadRun};
+use self::policy::{load_invocation_policy, InvocationPolicy, PolicyLoadError, ScanPolicy};
pub use self::socket_yml_args::{SocketYmlArgs, MIN_SEVERITY_ENV};
-use self::policy::{load_invocation_policy, InvocationPolicy, PolicyLoadError, ScanPolicy};
mod discovery;
mod gc;
pub(crate) mod hosted;
pub(crate) mod policy;
-mod socket_yml_args;
pub(crate) mod render;
pub(crate) mod rollout;
pub mod rollout_args;
+mod socket_yml_args;
pub(crate) mod vendor_flow;
use self::discovery::{
@@ -65,13 +65,13 @@
pub(crate) use self::hosted::boxed_run_redirect_selected;
use self::hosted::run_redirect;
pub(crate) use self::hosted::{vlt_rollback_heal, vlt_takeover_heal};
-pub(crate) use self::vendor_flow::{
- boxed_vendor_step, preview_vendor_json, print_dry_run_refusals, VendorStep,
-};
use self::vendor_flow::{
boxed_vendor_interactive_path, boxed_vendor_json_path, fold_vendored_skips_into_apply,
partition_skipped_selected,
};
+pub(crate) use self::vendor_flow::{
+ boxed_vendor_step, preview_vendor_json, print_dry_run_refusals, VendorStep,
+};
/// Packages per batch request on the authenticated API when `--batch-size`
/// is not given: the server's own per-request maximum
@@ -318,11 +318,7 @@
/// `requests`), or a purl with or without its version
/// (`pkg:npm/lodash`, `pkg:pypi/requests@2.31.0`). Repeat the flag or
/// separate with commas
- #[arg(
- long = "package",
- env = "SOCKET_SCAN_PACKAGES",
- value_delimiter = ','
- )]
+ #[arg(long = "package", env = "SOCKET_SCAN_PACKAGES", value_delimiter = ',')]
pub packages: Vec<String>,
/// On a successful scan, also generate an OpenVEX 0.2.0 document.
@@ -500,9 +496,10 @@
telemetry.flush().await;
let error_count = failures.len();
if error_count > 0 && error_count == packages.len() {
- let err = failures
- .last()
- .map_or_else(|| "all patch-detail queries failed".to_string(), |(_, e)| e.clone());
+ let err = failures.last().map_or_else(
+ || "all patch-detail queries failed".to_string(),
+ |(_, e)| e.clone(),
+ );
let message = format!("all {error_count} patch-detail queries failed: {err}");
if detail_error_line {
eprintln!("{}", render::fetch_details_failed(&failures));
@@ -568,7 +565,11 @@
packages: &[BatchPackagePatches],
result: Option<&mut serde_json::Value>,
) -> Vec<rollout::Row> {
- let failed: Vec<String> = discovered.failed.iter().map(|(purl, _)| purl.clone()).collect();
+ let failed: Vec<String> = discovered
+ .failed
+ .iter()
+ .map(|(purl, _)| purl.clone())
+ .collect();
stage.incomplete = rollout::lookup_incomplete(&recorded.index, &failed, batch_failed);
let rows = rollout::classify(&discovered.offers, &recorded.index, &stage.project);
if let Some(result) = result {
@@ -1317,7 +1318,8 @@
let joined = cwd.join(raw);
if raw.contains(['*', '?', '[']) {
let pattern = joined.to_string_lossy().into_owned();
- let matches = glob::glob(&pattern).map_err(|e| format!("invalid path pattern `{raw}`: {e}"))?;
+ let matches =
+ glob::glob(&pattern).map_err(|e| format!("invalid path pattern `{raw}`: {e}"))?;
let before = dirs.len();
dirs.extend(
matches
@@ -1390,7 +1392,10 @@
}
// One budget per invocation (§5.2): the directories spend it in sorted
// order, and a package admitted in one is admitted free in the next.
- let configured = match args.rollout.resolve_from_env(invocation.policy.max_new_patches()) {
+ let configured = match args
+ .rollout
+ .resolve_from_env(invocation.policy.max_new_patches())
+ {
Ok(max) => max,
Err(message) => {
eprintln!("Error: {message}");
@@ -1491,7 +1496,10 @@
// error.
let configured_cap = match args.rollout.carry.as_ref() {
Some(carry) => carry.lock().configured,
- None => match args.rollout.resolve_from_env(invocation.policy.max_new_patches()) {
+ None => match args
+ .rollout
+ .resolve_from_env(invocation.policy.max_new_patches())
+ {
Ok(max) => max,
Err(message) => {
eprintln!("Error: {message}");
@@ -1499,11 +1507,8 @@
}
},
};
- let mut stage = rollout::Stage::new(
- configured_cap,
- args.rollout.carry.clone(),
- &args.common.cwd,
- );
+ let mut stage =
+ rollout::Stage::new(configured_cap, args.rollout.carry.clone(), &args.common.cwd);
// Strict airgap (CLI_CONTRACT.md `--offline`): scan's patch discovery
// is remote data, so refuse before the crawl and before the API client
@@ -1689,8 +1694,11 @@
.filter(|pkg| args.common.purl_ecosystem_selected(&pkg.purl))
.collect();
- let package_specs: Vec<&String> =
- args.packages.iter().filter(|s| !s.trim().is_empty()).collect();
+ let package_specs: Vec<&String> = args
+ .packages
+ .iter()
+ .filter(|s| !s.trim().is_empty())
+ .collect();
let filtered_crawled: Vec<_> = if package_specs.is_empty() {
filtered_crawled
} else {
@@ -1828,13 +1836,12 @@
// `redirectState` rides the empty-discovery envelope too
// (same rule as the ≥1-package path). `wiringLive` is empty
// by construction: this run covered zero packages.
- let redirect_state = (!args.common.is_global()).then_some(
- crate::commands::hosted_state_from_pins(
+ let redirect_state =
+ (!args.common.is_global()).then_some(crate::commands::hosted_state_from_pins(
&socket_patch_core::patch::redirect::upstream::HostedPin::all(
ctx.discovery().await,
),
- ),
- );
+ ));
if let Some(state) = redirect_state_json(redirect_state.as_ref(), &[]) {
result["redirectState"] = state;
}
@@ -2190,7 +2197,8 @@
// A report-only run selects nothing, but a severity floor or
// `enabled: false` still hides candidates; report them like the
// human arm does (the detail fetch runs only then).
- if !apply && !vendor && policy.reports_selection() && !all_packages_with_patches.is_empty() {
+ if !apply && !vendor && policy.reports_selection() && !all_packages_with_patches.is_empty()
+ {
if let Err((code, message)) = discover_selected(
&api_client,
&all_packages_with_patches,
@@ -2483,12 +2491,7 @@
&all_packages_with_patches,
None,
);
- updates = offer_updates(
- &rows,
- &discovered,
- &recorded,
- &all_packages_with_patches,
- );
+ updates = offer_updates(&rows, &discovered, &recorded, &all_packages_with_patches);
rows
}
// `discover_selected` already printed the failure to stderr.
@@ -2950,14 +2953,20 @@
dirs.iter()
.map(|(d, explicit)| {
(
- d.strip_prefix(tmp.path()).unwrap().to_string_lossy().replace('\\', "/"),
+ d.strip_prefix(tmp.path())
+ .unwrap()
+ .to_string_lossy()
+ .replace('\\', "/"),
*explicit,
)
})
.collect()
};
- let got = project_dirs(tmp.path(), &["apps/*".into(), "libs/core".into(), "apps/web".into()])
- .unwrap();
+ let got = project_dirs(
+ tmp.path(),
+ &["apps/*".into(), "libs/core".into(), "apps/web".into()],
+ )
+ .unwrap();
// Named literally = explicit (also when a glob matches it too).
assert_eq!(
rel(got),
diff --git a/crates/socket-patch-cli/src/commands/scan/policy.rs b/crates/socket-patch-cli/src/commands/scan/policy.rs
--- a/crates/socket-patch-cli/src/commands/scan/policy.rs
+++ b/crates/socket-patch-cli/src/commands/scan/policy.rs
@@ -11,9 +11,9 @@
use socket_patch_core::api::types::PatchSearchResult;
use socket_patch_core::manifest::schema::PatchManifest;
use socket_patch_core::policy::{
- canon, find_repo_root_with_warnings, policy_block, FilteredEntry, RetainedEntry, patch_severity_order, repo_relative_checked, sanitize, severity_name,
- DiskPolicyFs, FilterReason, Offers, PolicyError, PolicySource, PolicyWarning, Root, SelectionPolicy,
- PATCHES_DISABLED,
+ canon, find_repo_root_with_warnings, patch_severity_order, policy_block, repo_relative_checked,
+ sanitize, severity_name, DiskPolicyFs, FilterReason, FilteredEntry, Offers, PolicyError,
+ PolicySource, PolicyWarning, RetainedEntry, Root, SelectionPolicy, PATCHES_DISABLED,
};
use socket_patch_core::utils::purl::normalize_purl;
@@ -42,12 +42,18 @@
/// Load the policy for `args` (4.5): `--global` scans have no repo and read
/// no file; everything else reads the repo root's socket.yml.
pub(crate) fn load_invocation_policy(args: &ScanArgs) -> Result<InvocationPolicy, PolicyLoadError> {
- let overrides = args.socket_yml.overrides().map_err(PolicyLoadError::Usage)?;
+ let overrides = args
+ .socket_yml
+ .overrides()
+ .map_err(PolicyLoadError::Usage)?;
let cwd = std::fs::canonicalize(&args.common.cwd).unwrap_or_else(|_| args.common.cwd.clone());
if args.common.is_global() {
- let policy = SelectionPolicy::load(&socket_patch_core::policy::MemoryPolicyFs::default(), &overrides)
- .map_err(PolicyLoadError::Policy)?
- .0;
+ let policy = SelectionPolicy::load(
+ &socket_patch_core::policy::MemoryPolicyFs::default(),
+ &overrides,
+ )
+ .map_err(PolicyLoadError::Policy)?
+ .0;
return Ok(InvocationPolicy {
policy,
repo_root: cwd,
@@ -56,8 +62,8 @@
});
}
let (repo_root, mut warnings) = find_repo_root_with_warnings(&cwd);
- let (policy, load_warnings) =
- SelectionPolicy::load(&DiskPolicyFs::new(&repo_root), &overrides).map_err(PolicyLoadError::Policy)?;
+ let (policy, load_warnings) = SelectionPolicy::load(&DiskPolicyFs::new(&repo_root), &overrides)
+ .map_err(PolicyLoadError::Policy)?;
warnings.extend(load_warnings);
Ok(InvocationPolicy {
policy,
@@ -138,7 +144,12 @@
impl ScanPolicy {
/// The policy for the project rooted at `root_dir`.
- pub(crate) fn for_root(invocation: &InvocationPolicy, root_dir: &Path, explicit: bool, global: bool) -> Self {
+ pub(crate) fn for_root(
+ invocation: &InvocationPolicy,
+ root_dir: &Path,
+ explicit: bool,
+ global: bool,
+ ) -> Self {
let root_dir = std::fs::canonicalize(root_dir).unwrap_or_else(|_| root_dir.to_path_buf());
let project = repo_relative_checked(&invocation.repo_root, &root_dir).unwrap_or_default();
let root_verdict = if global {
@@ -171,7 +182,9 @@
severity: None,
});
}
- let announce_warnings = !invocation.warned.swap(true, std::sync::atomic::Ordering::Relaxed);
+ let announce_warnings = !invocation
+ .warned
+ .swap(true, std::sync::atomic::Ordering::Relaxed);
Self {
policy: invocation.policy.clone(),
warnings,
@@ -224,7 +237,10 @@
/// exclude stays in the query (so `upgradeAvailable` can be reported)
/// but joins the retained set, which never reaches a writer.
pub(crate) fn admit_crawled(&self, purl: &str) -> bool {
- let verdict = self.root_verdict.clone().and_then(|()| self.policy.admits_purl(purl));
+ let verdict = self
+ .root_verdict
+ .clone()
+ .and_then(|()| self.policy.admits_purl(purl));
let reason = match verdict {
Ok(()) => return true,
Err(reason) => reason,
@@ -334,7 +350,8 @@
// (not when a lower-ranked admitted patch simply wins).
let top_withheld = self.policy.admits_severity(patch_severity_order(&group[0]));
if let Err(reason) = top_withheld {
- let upgrade_withheld = chosen.is_some() && chosen == recorded_at && recorded_at != Some(0);
+ let upgrade_withheld =
+ chosen.is_some() && chosen == recorded_at && recorded_at != Some(0);
if chosen.is_none() || upgrade_withheld {
report.filtered.push(FilteredEntry {
purl: Some(canon(&purl)),
@@ -522,17 +539,20 @@
let verdict = if !policy.enabled() {
Err(FilterReason::Disabled)
} else {
- root_verdict.clone().and_then(|()| policy.admits_purl(purl)).and_then(|()| {
- // The floor only hides a package when none of its patches pass.
- match group
- .iter()
- .map(|p| policy.admits_severity(patch_severity_order(p)))
- .find(Result::is_ok)
- {
- Some(ok) => ok,
- None => policy.admits_severity(patch_severity_order(group[0])),
- }
- })
+ root_verdict
+ .clone()
+ .and_then(|()| policy.admits_purl(purl))
+ .and_then(|()| {
+ // The floor only hides a package when none of its patches pass.
+ match group
+ .iter()
+ .map(|p| policy.admits_severity(patch_severity_order(p)))
+ .find(Result::is_ok)
+ {
+ Some(ok) => ok,
+ None => policy.admits_severity(patch_severity_order(group[0])),
+ }
+ })
};
if let Err(reason) = verdict {
out.push((
diff --git a/crates/socket-patch-cli/src/commands/scan/render.rs b/crates/socket-patch-cli/src/commands/scan/render.rs
--- a/crates/socket-patch-cli/src/commands/scan/render.rs
+++ b/crates/socket-patch-cli/src/commands/scan/render.rs
@@ -726,7 +726,10 @@
#[test]
fn report_only_hint_names_agent_mode() {
- assert_eq!(report_only_hint()[0], "To apply these patches in place, run:");
+ assert_eq!(
+ report_only_hint()[0],
+ "To apply these patches in place, run:"
+ );
assert!(report_only_hint()[1].contains("--mode agent"));
}
diff --git a/crates/socket-patch-cli/src/commands/scan/rollout.rs b/crates/socket-patch-cli/src/commands/scan/rollout.rs
--- a/crates/socket-patch-cli/src/commands/scan/rollout.rs
+++ b/crates/socket-patch-cli/src/commands/scan/rollout.rs
@@ -4,8 +4,10 @@
use std::collections::{BTreeMap, BTreeSet, HashSet};
-use socket_patch_core::rollout::{canonical_base_purl, severity_label, MaxNew, MaxNewSource, Recorded, RolloutPlan};
pub(crate) use socket_patch_core::rollout::stage::*;
+use socket_patch_core::rollout::{
+ canonical_base_purl, severity_label, MaxNew, MaxNewSource, Recorded, RolloutPlan,
+};
use super::discovery::UpdateInfo;
@@ -208,11 +210,11 @@
mod tests {
use super::*;
use socket_patch_core::api::types::PatchSearchResult;
+ use socket_patch_core::api::types::VulnerabilityResponse;
use socket_patch_core::manifest::schema::PatchManifest;
- use std::path::Path;
- use socket_patch_core::api::types::VulnerabilityResponse;
use socket_patch_core::manifest::schema::PatchRecord;
use std::collections::HashMap;
+ use std::path::Path;
fn offer(purl: &str, uuid: &str, published: &str, severities: &[&str]) -> PatchSearchResult {
PatchSearchResult {
@@ -357,13 +359,21 @@
let stored = manifest(&[("pkg:composer/psr/log@3.0.2.0", "old")]);
let recorded = RecordedIndex::new(Some(&stored), &[]);
let offers = offers_from_results(
- &[offer("pkg:composer/psr/log@v3.0.2", "new", "2026-02-01T00:00:00Z", &["high"])],
+ &[offer(
+ "pkg:composer/psr/log@v3.0.2",
+ "new",
+ "2026-02-01T00:00:00Z",
+ &["high"],
+ )],
false,
);
let rows = classify(&offers, &recorded, "");
let plan = socket_patch_core::rollout::plan_rollout(
rows.into_iter().map(|row| row.candidate).collect(),
- &MaxNew { value: Some(0), source: MaxNewSource::Flag },
+ &MaxNew {
+ value: Some(0),
+ source: MaxNewSource::Flag,
+ },
false,
&BTreeSet::new(),
);
diff --git a/crates/socket-patch-cli/src/commands/scan/rollout_args.rs b/crates/socket-patch-cli/src/commands/scan/rollout_args.rs
--- a/crates/socket-patch-cli/src/commands/scan/rollout_args.rs
+++ b/crates/socket-patch-cli/src/commands/scan/rollout_args.rs
@@ -1,7 +1,6 @@
//! `scan --max-new-patches` (see the rollout guide,
//! `docs/configuration.md#gradual-rollout`).
-
use clap::Args;
pub(crate) use socket_patch_core::rollout::stage::RolloutCarry;
use socket_patch_core::rollout::{resolve_max_new, MaxNew};
@@ -77,7 +76,6 @@
}
}
-
#[cfg(test)]
mod tests {
use super::*;
diff --git a/crates/socket-patch-cli/tests/apply/apply_network.rs b/crates/socket-patch-cli/tests/apply/apply_network.rs
--- a/crates/socket-patch-cli/tests/apply/apply_network.rs
+++ b/crates/socket-patch-cli/tests/apply/apply_network.rs
@@ -940,7 +940,10 @@
"a legacy package archive must not cover the patch; stdout={stdout}\nstderr={stderr}"
);
let content = std::fs::read(tmp.path().join("node_modules/pkgcache/index.js")).unwrap();
- assert_eq!(content, before, "the file must not be patched from the legacy archive");
+ assert_eq!(
+ content, before,
+ "the file must not be patched from the legacy archive"
+ );
let requests = mock.received_requests().await.unwrap_or_default();
let blob_path = format!("/v0/orgs/{ORG_SLUG}/patches/blob/{after_hash}");
@@ -1043,10 +1046,7 @@
v["summary"]["applied"], 1,
"the drifted nested copy must be warn-overwritten.\nstdout={v:#}"
);
- assert_eq!(
- v["summary"]["failed"], 0,
- "no copy may fail.\nstdout={v:#}"
- );
+ assert_eq!(v["summary"]["failed"], 0, "no copy may fail.\nstdout={v:#}");
// The nested copy's blob was fetched on demand…
let requests = mock.received_requests().await.unwrap();
diff --git a/crates/socket-patch-cli/tests/apply/in_process_gem_config_warning.rs b/crates/socket-patch-cli/tests/apply/in_process_gem_config_warning.rs
--- a/crates/socket-patch-cli/tests/apply/in_process_gem_config_warning.rs
+++ b/crates/socket-patch-cli/tests/apply/in_process_gem_config_warning.rs
@@ -201,7 +201,9 @@
"non-silent stderr must carry the {CODE} warning; got:\n{stderr}"
);
assert_eq!(
- stderr.matches("Warning: bundler app config BUNDLE_PATH").count(),
+ stderr
+ .matches("Warning: bundler app config BUNDLE_PATH")
+ .count(),
1,
"exactly ONE warning line (not one per discovery call); got:\n{stderr}"
);
diff --git a/crates/socket-patch-cli/tests/cli/covgap_output.rs b/crates/socket-patch-cli/tests/cli/covgap_output.rs
--- a/crates/socket-patch-cli/tests/cli/covgap_output.rs
+++ b/crates/socket-patch-cli/tests/cli/covgap_output.rs
@@ -168,9 +168,8 @@
.expect("spawn socket-patch in PTY");
drop(pair.slave);
- let reader_handle = crate::pty_io::PtyOutput::spawn(
- pair.master.try_clone_reader().expect("clone reader"),
- );
+ let reader_handle =
+ crate::pty_io::PtyOutput::spawn(pair.master.try_clone_reader().expect("clone reader"));
// Watchdog: detached kill after `timeout`; a no-op if the child exits
// naturally first.
@@ -261,7 +260,10 @@
"\n",
Duration::from_secs(15),
);
- assert_eq!(code, 0, "remove with bare Enter must succeed; got: {output}");
+ assert_eq!(
+ code, 0,
+ "remove with bare Enter must succeed; got: {output}"
+ );
// The interactive confirm MUST have run — otherwise this test passes
// vacuously against a regression that drops the TTY gate and
// auto-proceeds. Match the distinctive prompt verbatim (the loose
diff --git a/crates/socket-patch-cli/tests/cli/interactive_prompts_e2e.rs b/crates/socket-patch-cli/tests/cli/interactive_prompts_e2e.rs
--- a/crates/socket-patch-cli/tests/cli/interactive_prompts_e2e.rs
+++ b/crates/socket-patch-cli/tests/cli/interactive_prompts_e2e.rs
@@ -112,9 +112,8 @@
// closed. The previous design used a chunked read+mpsc loop
// because it interleaved with a try_wait poll; the simplified
// design serializes wait → drop master → read_to_end joins.
- let reader_handle = crate::pty_io::PtyOutput::spawn(
- pair.master.try_clone_reader().expect("clone reader"),
- );
+ let reader_handle =
+ crate::pty_io::PtyOutput::spawn(pair.master.try_clone_reader().expect("clone reader"));
// Watchdog: detach a thread that kills the child after `timeout`.
// The cloned ChildKiller is independent of the main `child`
diff --git a/crates/socket-patch-cli/tests/cli_config_fallback.rs b/crates/socket-patch-cli/tests/cli_config_fallback.rs
--- a/crates/socket-patch-cli/tests/cli_config_fallback.rs
+++ b/crates/socket-patch-cli/tests/cli_config_fallback.rs
@@ -59,8 +59,7 @@
let mut cmd = Command::new(BINARY);
// Human mode: core's proxy advisory (the oracle below) is muted under
// `--json`/`--silent`.
- cmd.args(["scan", "-e", "npm", "--cwd"])
- .arg(project);
+ cmd.args(["scan", "-e", "npm", "--cwd"]).arg(project);
for (key, _) in std::env::vars_os() {
... diff truncated: showing 800 of 6421 linesYou can send follow-ups to the cloud agent here.
PDM_PYTHON outranks .pdm-python, so discovery now follows it too. A project with uv.lock or poetry.lock is installed by uv or Poetry (they drive hosted installs ahead of pdm.lock), so a leftover PDM record or __pypackages__ there no longer decides the env. Found by Bugbot review. Assisted-by: Claude Code:claude-opus-5-5
Collaborator
Author
|
BugBot review Generated by Claude Code |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 60dfb81. Configure here.
Mikola Lysenko (mikolalysenko)
added
the
Ready for review
Collaborator
Author
|
[agent] Ready for review at head
Slack announcement: not sent this run (Slack send tool unavailable to the agent); will retry next run. Generated by Claude Code |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
LLM Description written by Claude Code:claude-opus-5-5
Fixes #502
Fixes #525
Fixes #528
Summary
For the Python project env, socket-patch now uses the env that uv or PDM actually installs into: a PDM venv, a PEP 582
__pypackages__dir, or a uvUV_PROJECT_ENVIRONMENT. It no longer guesses./.venvor falls back to the PATH interpreter. Agent mode patches that env. The hosted stale-install warning andvexcheck it, sovexno longer reportsnot_affectedfor an install that is still unpatched.Root cause
find_local_venv_site_packages_with(crates/socket-patch-core/src/crawlers/python_crawler.rs) picks the project env fromVIRTUAL_ENV, Pipenv, Poetry,./.venvand./venv. It never asks the project's own package manager where it installed:UV_PROJECT_ENVIRONMENTmoves the project env, anduv sync/uv runuse it in place of./.venvor an activatedVIRTUAL_ENV..pdm-python(or the older.pdm.toml[python] path) records the interpreter PDM installs into. That can be an out-of-tree venv (venv.in_project = false) or one picked withpdm use. PDM prefers it over an activated venv.__pypackages__(PEP 582) project, hosted mode gives no stale-install warning andvexattests not_affected while the copypdm runimports is still unpatched #528): when the recorded interpreter isn't a venv, or a PDM 0.x/1.x project saved none, PDM installs into__pypackages__/<X.Y>/lib, which nothing probed.So discovery fell through to a stray
./.venv, or to the PATH interpreter through theis_python_projectfallback. All three consumers read this one function: agent apply, the hostedredirect_pypi_stale_installprobe (scan/hosted/python.rs), andvex. All three issues are fixed in this one place.Fix
A new step 0,
package_manager_recorded_site_packages, runs ahead of the generic probes:.pdm-python,pdm.lock,.pdm.tomlor[tool.pdm]) that has nouv.lockorpoetry.lock, since those drive installs ahead ofpdm.lock, as the hosted rewriters already assume. The interpreter isPDM_PYTHON, else the saved one (.pdm-python, else.pdm.toml; ignored underPDM_IGNORE_SAVED_PYTHON). If that interpreter belongs to a venv (<root>/{bin,Scripts}/python…withpyvenv.cfg), that venv's site-packages is the env. Otherwise the env is__pypackages__/*/lib.UV_PROJECT_ENVIRONMENT(absolute, or relative to the project) only for a project uv drives, so the ambient variable can't hijack a Poetry, PDM or Pipenv project. That means a project withuv.lock, or apyproject.tomlthat nopoetry.lock,pdm.lock,.pdm-pythonorPipfileclaims../.venvis ignored. When it doesn't exist yet (not synced), the existing probes and fallback decide, so behavior there is unchanged.read_regular_to_string, so a FIFO in place ofpyproject.tomlcan't wedge the scan.Docs: CLI_CONTRACT probe list, the PDM page (replaces the "
__pypackages__not covered" limit), and a uvUV_PROJECT_ENVIRONMENTnote.Out of scope, unchanged: when no project env exists at all, the
is_python_project→ global-interpreter fallback still applies. That's the open maintainer question in #504.Per-issue test checklist (red on
main→ green here)python_crawler::tests::pdm_saved_interpreter_venv_is_the_project_env(also coversPDM_PYTHON,PDM_IGNORE_SAVED_PYTHON, legacy.pdm.toml, anduv.lock/poetry.lockprecedence),in_process_python_envs::pdm_saved_interpreter_venv_is_scanned_not_a_stray_dot_venv,in_process_redirect_pdm::pdm_recorded_env_is_probed_for_stale_hosted_installs(out-of-tree venv case)python_crawler::tests::uv_project_environment_is_the_project_env(absolute, relative, activatedVIRTUAL_ENV, lock-less, other-manager and non-project controls, unsynced env),in_process_python_envs::uv_project_environment_is_scanned_not_a_stray_dot_venv__pypackages__(PEP 582) project, hosted mode gives no stale-install warning andvexattests not_affected while the copypdm runimports is still unpatched #528:python_crawler::tests::pdm_pep582_pypackages_is_the_project_env,in_process_python_envs::pdm_pep582_pypackages_is_scanned_not_a_stray_dot_venv,in_process_redirect_pdm::pdm_recorded_env_is_probed_for_stale_hosted_installs(PEP 582 case: stale warning plus no VEX file, then attests once patched)To show red, I disabled step 0. All 3 core tests and all 4 CLI tests then fail. On
main, the hosted PDM test reproduces the false attestation (exit 0,"warnings":[],vex.statements: 1over upstream bytes).Review rounds
ab7aeb6found two issues, both real and both fixed in60dfb81with tests:PDM_PYTHONwas ignored, and a leftover PDM record could shadow auv.lockproject. Bugbot's re-review of60dfb81found no new issues.Verification
60dfb81: all 340 check runs green (334 success, 6 skipped matrix/conditional jobs).cargo clippy --workspace --all-features -- -D warnings: clean (re-run on60dfb81).cargo test --workspace --all-features --no-fail-fast: 9474 passed. The 13 failures, besides one FIFO case, are all write-failure or unremovable-file tests that can't fail when running as root (uid 0) in this container. I confirmed the core ones fail identically on unmodifiedmainin this container. The FIFO case (scan::hosted_symlinked_files::hosted_scan_returns_with_fifo_candidate) was a real regression in an earlier commit, also caught by CIcoverage.ab7aeb6fixes it, and thescansuite now passes 104/104. On60dfb81I re-ran the corepython_crawlertests and thein_process_python_envs/in_process_redirect_pdmsuites: all green.cargo fmt --all -- --checkisn't clean onmainwith this toolchain either, and CI doesn't run it, so I kept fmt changes to my own hunks.🤖 Generated with Claude Code
Generated by Claude Code