GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — Fix UTF-16 requirements.txt silently skipped (#721) by mikolalysenko · Pull Request #724 · SocketDev/socket-patch Conversation
|
| Issue symptom | Test | Before fix | After fix |
|---|---|---|---|
| #721 hosted exits 0 unpatched | in_process_get_hosted_ecosystems::pypi_requirements_hosted_refuses_a_utf16_file (UTF-16 LE and BE) |
FAILED: exit 0 (left: 0, right: 1) |
ok |
| #721 lock-only "No packages found" | scan_requirements_lock_only::lock_only_scan_discovers_utf16_pins (LE and BE, hosted and --vendor) |
FAILED: lockfileOnlyPackages: 0 |
ok |
| engine guard, disk + memory parity | hosted::engine::tests::an_undecodable_candidate_file_refuses_its_ecosystem |
(new field) | ok |
discovery decode incl. -r include, disk + memory |
lock_inventory::tests::requirements_utf16_files_are_inventoried |
– | ok |
| takeover reverted before the refusal (Bugbot) | mode_migration_pypi::undecodable_candidate_refuses_before_the_takeover_reverts (wet + --dry-run) |
FAILED: dry run exit 0 (left: 0, right: 1) |
ok |
| vendored UTF-16 root / include (Bugbot) | pypi_requirements::tests::a_utf16_requirements_file_is_refused_by_name |
– | ok |
| pip BOM table | utils::requirements::tests::decode_follows_pips_byte_order_marks |
– | ok |
Local checks:
cargo clippy --workspace --all-features -- -D warnings: clean.cargo fmt: my hunks are rustfmt-clean. main itself isn't fmt-clean under the pinned 1.93.1 toolchain, and CI doesn't gate on fmt, so I didn't reformat unrelated files.cargo test -p socket-patch-core --all-features: 4845 passed. 4 failed, all read-only-permission tests that can't fail when run as root (uid 0, this sandbox): copy_tree::relax_loop_must_not_traverse_symlinked_root, vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry, pypi_poetry::wire_write_failure_maps_error_and_leaves_lock_untouched, pypi_requirements::wire_failure_rolls_back_already_written_files. None touch the changed code.--lib (834), hosted_memory_engine, hosted_memory_parity, hosted_memory_rollout, covgap_commands_scan_hosted, e2e_vex_redirect, in_process_redirect_pipenv, in_process_rollback_hosted, mode_migration_pypi, scan_requirements_lock_only and in_process_get_hosted_ecosystems all pass. in_process_redirect: 104 passed. 3 failed, again chmod 0o555 write-failure tests that root bypasses.b3daafc: all green (479 success, 6 skipped). One native (macos-latest, 1.3.10) Bun job against the production patch hosts failed on the first attempt. It passed on 5444dd6, and this PR touches no Bun code. It passed on its single re-run.main (05fd82b) into this branch was clean. Clippy, in_process_get_hosted_ecosystems, scan_requirements_lock_only and mode_migration_pypi pass on it. It was not pushed, because the PR is in the merge queue.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Assisted-by: Claude Code:claude-opus-5-5
Windows PowerShell 5.1 writes `pip freeze > requirements.txt` as UTF-16 with a byte-order mark, and pip installs from it. Hosted scan read every candidate file as UTF-8 and treated a file it could not decode as missing, so the run exited 0 as a success with nothing pinned, and pip kept installing the unpatched release. A fresh checkout's lock-only scan said "No packages found" for the same file. Hosted runs (disk and in-memory alike) now refuse with candidate_file_unreadable, naming the file and asking for it to be re-saved as UTF-8, whenever a non-UTF-8 candidate file belongs to an ecosystem being redirected. Nothing is written. Lock-only discovery decodes requirements.txt and its -r includes by BOM the way pip does, so the pins are found. Fixes #721 Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Stale Bugbot comment from a previous run.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
A vendored project switching to hosted mode had its vendored wiring reverted before the new non-UTF-8 check ran. A UTF-16 requirements.txt then refused the run with the vendored package already unwired, so it installed unpatched in both modes, and --dry-run predicted success. The check now runs before any revert, wet or dry. Vendored mode also named only "cannot read" for a UTF-16 root requirements.txt and silently skipped a UTF-16 -r include that pip installs from. Both now refuse by name with a re-save-as-UTF-8 hint. Refs #721 Assisted-by: Claude Code:claude-opus-5-5
|
BugBot review Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Stale Bugbot comment from a previous run.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Bugbot Autofix prepared a fix for the issue found in the latest run.
Or push these changes by commenting: Preview (a9d1c657c4)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/[email protected]", &record.uuid), - pin("pkg:npm/[email protected]", "33333333-3333-4333-8333-333333333333"), + pin( + "pkg:npm/[email protected]", + "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; @@ -141,9 +141,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 @@ -153,10 +155,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(), @@ -164,7 +164,8 @@ description: String::new(), license: String::new(), tier: String::new(), - }); + } + }); } state } @@ -191,4 +192,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/discovery.rs b/crates/socket-patch-cli/src/commands/scan/discovery.rs --- a/crates/socket-patch-cli/src/commands/scan/discovery.rs +++ b/crates/socket-patch-cli/src/commands/scan/discovery.rs @@ -168,29 +168,32 @@ } // `(ledger key, base purl, entry)`; the artifact fallback has no // entries to probe, so it never reports unwired keys. - let candidates: Vec<(String, String, Option<&socket_patch_core::vendor::VendorEntry>)> = - match state { - Ok(state) => state - .entries - .iter() - .map(|(key, entry)| { - ( - key.clone(), - strip_purl_qualifiers(&entry.base_purl).to_string(), - Some(entry), - ) - }) - .collect(), - // Corrupt/unreadable ledger (a MISSING file is Ok(empty) above): - // recover the vendored set from the committed artifacts, or - // `scan --prune` (whose ledger exemption also degrades to empty) - // would delete still-vendored packages' manifest entries and blobs. - Err(_) => vendored_purls_from_artifacts(common) - .await - .into_iter() - .map(|base| (base.clone(), base, None)) - .collect(), - }; + let candidates: Vec<( + String, + String, + Option<&socket_patch_core::vendor::VendorEntry>, + )> = match state { + Ok(state) => state + .entries + .iter() + .map(|(key, entry)| { + ( + key.clone(), + strip_purl_qualifiers(&entry.base_purl).to_string(), + Some(entry), + ) + }) + .collect(), + // Corrupt/unreadable ledger (a MISSING file is Ok(empty) above): + // recover the vendored set from the committed artifacts, or + // `scan --prune` (whose ledger exemption also degrades to empty) + // would delete still-vendored packages' manifest entries and blobs. + Err(_) => vendored_purls_from_artifacts(common) + .await + .into_iter() + .map(|base| (base.clone(), base, None)) + .collect(), + }; // Composer by release identity: a ledger `@3.0.2.0` is the crawled // `@3.0.2`, not a second package to supplement. let key = |p: &str| composer_purl_identity(p).unwrap_or_else(|| normalize_purl(p).into_owned()); @@ -1038,7 +1041,9 @@ ..GlobalArgs::default() }; let state = socket_patch_core::vendor::load_state(root).await; - vendored_ledger_supplement(&args, crawled, &state).await.packages + vendored_ledger_supplement(&args, crawled, &state) + .await + .packages } /// A ledger entry vendored as `@3.0.2.0` is the crawled composer @@ -1073,7 +1078,9 @@ out.iter().map(|p| &p.purl).collect::<Vec<_>>() ); - let out = vendored_ledger_supplement(&args, &[], &Ok(state)).await.packages; + let out = vendored_ledger_supplement(&args, &[], &Ok(state)) + .await + .packages; assert_eq!( out.iter().map(|p| p.purl.as_str()).collect::<Vec<_>>(), vec!["pkg:composer/psr/[email protected]"] @@ -1176,7 +1183,10 @@ let state = npm_ledger_with_lock(tmp.path(), lock.as_deref()).await; let out = vendored_ledger_supplement(&args, &[], &state).await; assert_eq!( - out.packages.iter().map(|p| p.purl.as_str()).collect::<Vec<_>>(), + out.packages + .iter() + .map(|p| p.purl.as_str()) + .collect::<Vec<_>>(), vec!["pkg:npm/[email protected]"], "lock={lock:?}" ); 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 @@ -789,6 +789,43 @@ // ownership known" for both consumers. let mut vendor_state = socket_patch_core::vendor::load_state(&common.cwd).await; + // Read candidate files BEFORE the takeover to detect encoding issues + // early. The encoding check must happen before any writes (see the + // pre-takeover encoding guard below). + let read = if !candidates.is_empty() { + engine::read_candidate_files(&view, &std::collections::BTreeSet::new(), &candidates).await + } else { + CandidateFiles::default() + }; + + // PRE-TAKEOVER ENCODING GUARD: refuse if any undecodable candidate file + // matches a takeover-capable candidate's ecosystem. This prevents the + // takeover from writing before the encoding check in `engine::guard` + // would refuse the run, ensuring "nothing was written" stays true. + if !read.undecodable_reads.is_empty() { + use std::collections::BTreeSet; + let takeover_capable = |p: &str| { + p.starts_with("pkg:cargo/") + || p.starts_with("pkg:npm/") + || p.starts_with("pkg:golang/") + || p.starts_with("pkg:pypi/") + }; + let candidate_ecosystems: BTreeSet<&str> = candidates + .iter() + .filter(|c| takeover_capable(&c.purl)) + .map(|c| c.dep.ecosystem.as_str()) + .collect(); + if let Some(rel) = read.undecodable_reads.iter().find(|rel| { + engine::file_ecosystem(rel).is_some_and(|eco| candidate_ecosystems.contains(eco)) + }) { + return refuse( + common, + scan_result.take(), + &engine::undecodable_refusal(rel), + ); + } + } + // Cross-mode takeover of still-vendored purls (see `vendored_takeover`). let Takeover { pre_warnings: takeover_pre_warnings, @@ -802,15 +839,13 @@ Err(refusal) => return refuse(common, scan_result.take(), &refusal), }; - // Read the project's candidate files. Skipped when no candidate - // survived and no dry-run takeover preview is pending (the rewriters do - // nothing without a dep); everything after the rewrite still runs. A - // dry-run takeover preview still needs the root locks for the - // install-policy previews below. - let read = if !candidates.is_empty() || !dry_run_takeover_urls.is_empty() { + // Re-read candidate files if the takeover modified any, or if a dry-run + // takeover preview is pending (the rewriters need the root locks for + // install-policy previews). Otherwise reuse the pre-takeover read. + let read = if !takeover_files.is_empty() || !dry_run_takeover_urls.is_empty() { engine::read_candidate_files(&view, &std::collections::BTreeSet::new(), &candidates).await } else { - CandidateFiles::default() + read }; let mut python_metadata = std::collections::BTreeMap::new(); @@ -932,7 +967,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, @@ -944,6 +980,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. @@ -2304,13 +2341,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 @@ -4391,19 +4434,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/[email protected]`). 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 @@ -1704,8 +1709,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 { @@ -1860,13 +1868,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; } @@ -2222,7 +2229,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, @@ -2515,12 +2523,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. @@ -2982,14 +2985,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 @@ -746,7 +746,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, +}; ... diff truncated: showing 800 of 6990 lines You can send follow-ups to the cloud agent here. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Ready for review. Head
Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
CLI_CONTRACT.md: kept this PR's #721 non-UTF-8 candidate paragraph and main's expanded gem stale-install (global bundler config tier) paragraph. Co-Authored-By: Claude <[email protected]>
|
bugbot run Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Stale Bugbot comment from a previous run.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
[agent] Three checks are red on Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Brings in the vex_consumed alias test fix (#849) that main's red test/test-release/coverage jobs were waiting on. Co-Authored-By: Claude <[email protected]>
|
bugbot run Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Stale Bugbot comment from a previous run.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
[agent]
The case logs are in artifact Update: the single re-run of Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Burn-down agent: labeled Ready for review at
Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Resolved crates/socket-patch-core/src/hosted/engine.rs: this branch adds CandidateFiles::undecodable_reads (#721) and main added CandidateFiles::gradle_unreadable (#646) in the same spot of the struct and of the destructuring in rewrite(). Kept both fields; the two are independent (a non-UTF-8 Gradle file in a memory view is recorded in both, and each guard refuses on its own). Co-Authored-By: Claude <[email protected]>
|
bugbot run Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Stale Bugbot comment from a previous run.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Resolves the CLI_CONTRACT.md conflict by keeping both the non-UTF-8 candidate files paragraph and main's updated gem stale-install guard paragraph. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01YED4tY7Yytk79MTPzLnSfA
Resolve the CLI_CONTRACT.md conflict: keep this PR's non-UTF-8 candidate-file paragraph (#721) and take main's updated gem stale-install guard paragraph. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01YED4tY7Yytk79MTPzLnSfA
|
[agent] The merge queue removed this PR for a merge conflict with
Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
[agent] Update on I'm not porting #1102 into this branch. The leg isn't part of the required Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
[agent] The merge queue removed this PR at 05:06Z for The failure isn't this PR's. The test failed in its fixture warm-up, before any socket-patch code ran: Maven Central couldn't serve Also: the two earlier removals (23:38Z and 04:10Z) were Generated by Claude Code |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
#1083 added RewriteOptions::yarn_classic_outer; this PR's new test initializer predates it, so the merge-group build failed with E0063. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
|
[agent] The merge queue dropped this at 05:55Z because the merge-group build failed: |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Resolve scan/hosted.rs against #724: its non-UTF-8 pre-check in vendored_takeover moves to the staged takeover's plan step (hosted/takeover.rs), before any revert is staged, dry runs included. Drop the removed rewrite() argument from #724's two new engine tests. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
#724 landed against the old requirements-walk callback, which named the file `path`; on this branch the walk hands over the root-relative `rel`. Name the file from `root.join(rel)`. Its new engine test also needs this branch's RewriteOptions fields. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Brings in #1044 (governing lock table), #1035, #1038, #1083, #724. Drop the BUN_LOCK/BUN_LOCKB/NPM_LOCKS imports #1044 added to npm_flavor.rs: their only user, vendored_entry_in_use, is removed by this PR (liveness now comes from Discovery::vendor_entry_in_use). This unused import failed clippy in the merge group. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Resolve hosted/engine.rs: keep main's undecodable_reads retain (#724) alongside this PR's twin-manifest-ambiguous gem refusal. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
Hosted and lock-only scans treat a UTF-16 requirements.txt (what Windows PowerShell'spip freeze > writes) as absent: exit 0, no warning, and pip keeps installing the unpatched pin
There was an error while loading. Please reload this page.