From 79e894c7b507a88158421a5db9ab01cf75000790 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 02:59:26 +0000 Subject: [PATCH 1/5] Start sweep of core constants and utils Assisted-by: Claude Code:claude-opus-5-5 From 8686fe838b9a3a23ccf584ac0feb452781731a4e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 03:05:19 +0000 Subject: [PATCH 2/5] Sweep core constants and utils helpers constants.rs: drop the tautological USER_AGENT and manifest-path tests, keep one URL test on https, no trailing slash and distinct hosts, and move the test module to the end. cargo_home.rs: cut the module doc's history and bare #1135 refs. composer_version.rs: composer_versions_equivalent compares composer_version_key, the key PurlKey already uses; the sentinel identity key had no other caller. The two pairwise tests become one that checks equivalence against an independently written spec. containment.rs: linked_level_refusal is private; nearest_socket_dir moves to socket_dir.rs, its only user, and socket_project_root is inlined there. The stale is_link doc is trimmed. digest.rs: drop the banner and the stale #706 pending note. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/constants.rs | 69 +++++------------ .../socket-patch-core/src/utils/cargo_home.rs | 21 ++--- .../src/utils/composer_version.rs | 77 +++++-------------- .../src/utils/containment.rs | 35 +-------- crates/socket-patch-core/src/utils/digest.rs | 10 +-- .../socket-patch-core/src/utils/purl_key.rs | 7 +- .../socket-patch-core/src/utils/socket_dir.rs | 23 +++++- 7 files changed, 73 insertions(+), 169 deletions(-) diff --git a/crates/socket-patch-core/src/constants.rs b/crates/socket-patch-core/src/constants.rs index 69a31ecff..5e7ff560d 100644 --- a/crates/socket-patch-core/src/constants.rs +++ b/crates/socket-patch-core/src/constants.rs @@ -12,59 +12,10 @@ pub const DEFAULT_PATCH_API_PROXY_URL: &str = "https://patches-api.socket.dev"; /// Default Socket API URL for authenticated access. pub const DEFAULT_SOCKET_API_URL: &str = "https://api.socket.dev"; -/// User-Agent header value for API requests. -/// -/// The version segment is derived from the crate version at compile time so it -/// tracks the published release instead of drifting from a hardcoded literal. -/// Server-side analytics and any minimum-version gating rely on this reporting -/// the real version. +/// User-Agent header value for API requests, carrying the compiled crate +/// version. pub(crate) const USER_AGENT: &str = concat!("SocketPatchCLI/", env!("CARGO_PKG_VERSION")); -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn user_agent_reports_real_crate_version() { - // USER_AGENT must carry the actual compiled crate version, never a - // hardcoded one. - let expected = format!("SocketPatchCLI/{}", env!("CARGO_PKG_VERSION")); - assert_eq!(USER_AGENT, expected); - assert!(USER_AGENT.starts_with("SocketPatchCLI/")); - assert!( - !USER_AGENT.ends_with("/1.0"), - "USER_AGENT must not be stuck at the stale 1.0 version" - ); - // The version segment must be non-empty. - let version = USER_AGENT.trim_start_matches("SocketPatchCLI/"); - assert!(!version.is_empty(), "version segment must not be empty"); - } - - #[test] - fn api_urls_are_https_without_trailing_slash() { - for url in [DEFAULT_PATCH_API_PROXY_URL, DEFAULT_SOCKET_API_URL] { - assert!(url.starts_with("https://"), "{url} must use https"); - assert!( - !url.ends_with('/'), - "{url} must not end with a trailing slash" - ); - } - // The proxy and authenticated API are distinct hosts; swapping them - // would silently send authed traffic to the public proxy (or vice versa). - assert_ne!(DEFAULT_PATCH_API_PROXY_URL, DEFAULT_SOCKET_API_URL); - assert_eq!( - DEFAULT_PATCH_API_PROXY_URL, - "https://patches-api.socket.dev" - ); - assert_eq!(DEFAULT_SOCKET_API_URL, "https://api.socket.dev"); - } - - #[test] - fn manifest_path_is_under_dot_socket() { - assert_eq!(DEFAULT_PATCH_MANIFEST_PATH, ".socket/manifest.json"); - } -} - /// The npm-family package managers' shared file names. Which subsystem /// accepts which of them (the intentionally divergent subsets: binary locks /// have a native byte reader, `pnpm-lock.yml` is only a pnpm marker) is @@ -106,3 +57,19 @@ pub mod npm_family { /// Workspace globs of vlt <= 0.0.0-12 (`{"packages": ...}`). pub const VLT_LEGACY_WORKSPACES: &str = "vlt-workspaces.json"; } + +#[cfg(test)] +mod tests { + use super::*; + + /// Swapping the two would send authenticated traffic to the public proxy + /// (or the reverse); a trailing slash doubles up when a route is joined. + #[test] + fn api_urls_are_distinct_https_without_trailing_slash() { + for url in [DEFAULT_PATCH_API_PROXY_URL, DEFAULT_SOCKET_API_URL] { + assert!(url.starts_with("https://"), "{url}"); + assert!(!url.ends_with('/'), "{url}"); + } + assert_ne!(DEFAULT_PATCH_API_PROXY_URL, DEFAULT_SOCKET_API_URL); + } +} diff --git a/crates/socket-patch-core/src/utils/cargo_home.rs b/crates/socket-patch-core/src/utils/cargo_home.rs index 371cbc8e0..7f0c2403b 100644 --- a/crates/socket-patch-core/src/utils/cargo_home.rs +++ b/crates/socket-patch-core/src/utils/cargo_home.rs @@ -1,16 +1,9 @@ //! The cargo home cargo itself would use: `$CARGO_HOME`, else //! `/.cargo` with `` resolved the way cargo's `home` crate -//! resolves it. -//! -//! [`super::fs::home_dir`] reads only `HOME` / `USERPROFILE`, but cargo -//! falls back to the account's passwd entry on Unix when `HOME` is unset -//! (`env -i`, systemd units, stripped CI and container entrypoints). A -//! crawler that stopped at `home_dir` probed no registry cache there and -//! reported the crate lockfile-only while cargo built the unpatched copy -//! from the passwd home's `~/.cargo` (#1135). The cargo-home readers (the -//! crawler, the `[patch]` config chain, the build-cache cleaner and the -//! self-update channel check) go through [`cargo_home`] so they agree with -//! cargo and with each other. +//! resolves it. Unlike [`super::fs::home_dir`], that falls back to the +//! account's passwd entry on Unix when `HOME` is unset (`env -i`, systemd +//! units, stripped CI and container entrypoints), so every cargo-home reader +//! goes through [`cargo_home`] to probe the registry cache cargo builds from. use std::ffi::OsString; use std::path::PathBuf; @@ -124,8 +117,8 @@ mod tests { assert_eq!(got, Some(PathBuf::from("/home/u/.cargo"))); } - /// #1135: with `HOME` and `CARGO_HOME` unset cargo builds from the - /// passwd home's `~/.cargo`, so the crawler must probe it too. + /// With `HOME` and `CARGO_HOME` unset cargo builds from the passwd + /// home's `~/.cargo`, so the crawler must probe it too. #[test] fn unset_home_var_falls_back_to_passwd_home() { let got = resolve(None, None, || Some("/root".into())); @@ -169,7 +162,7 @@ mod tests { assert_eq!(got, want); } - /// #1135 through the process environment: `HOME`, `USERPROFILE` and + /// Through the process environment: `HOME`, `USERPROFILE` and /// `CARGO_HOME` all unset, or `HOME` empty, still finds the passwd /// home's `.cargo`. #[cfg(unix)] diff --git a/crates/socket-patch-core/src/utils/composer_version.rs b/crates/socket-patch-core/src/utils/composer_version.rs index b35b5d15a..b44a1296d 100644 --- a/crates/socket-patch-core/src/utils/composer_version.rs +++ b/crates/socket-patch-core/src/utils/composer_version.rs @@ -3,9 +3,7 @@ //! One composer release reaches the CLI under several spellings. Lockfiles //! and `installed.json` carry the pretty tag (`v3.0.2`, `3.0.2`), while a //! patch's base purl may carry the padded `version_normalized` form -//! (`3.0.2.0`) that Socket's SBOM ingestion stores. Comparing them as strings -//! made a patch for `@3.0.2.0` "not found" in a project that locks `3.0.2`, -//! and made `scan --prune` delete it. +//! (`3.0.2.0`) that Socket's SBOM ingestion stores. //! //! [`composer_version_normalize`] is a port of composer/semver 3.4 //! `VersionParser::normalize`, the function Composer and Packagist key a @@ -19,11 +17,9 @@ //! parts too, since that padding erases whether the lock said `X`, `X.0` or //! `X.0.0`. Any other spelling Composer rejects keys as itself minus one //! leading `v`, and is equivalent only to other rejected spellings with the -//! same key. [`composer_version_identity_key`] keeps that space apart with a -//! `\u{1}` sentinel; [`composer_version_key`] (what `PurlKey` uses, so its -//! string stays printable in reports) relies instead on every accepted key -//! being accepted itself, bare and behind a `v`, which -//! `sentinel_free_key_keeps_rejected_spellings_apart` pins over the vectors. +//! same key. That needs no separate key space because every accepted key is +//! accepted itself, bare and behind a `v`, which +//! `key_equality_is_equivalence_over_every_vector_pair` pins over the vectors. //! //! Only comparisons go through this module, and purl comparisons reach it //! only through [`crate::utils::purl_key::PurlKey`]. Stored spellings @@ -251,20 +247,12 @@ pub fn composer_version_key(version: &str) -> String { identity_normalize(version).unwrap_or_else(|| strip_leading_v(version).to_string()) } -/// A key two spellings share exactly when [`composer_versions_equivalent`] -/// holds, for map and set lookups: [`identity_normalize`], else the spelling -/// minus one leading `v` behind a `\u{1}` so that versions Composer rejects -/// get their own key space (depscan's `composerVersionIdentityKey`). -pub fn composer_version_identity_key(version: &str) -> String { - identity_normalize(version).unwrap_or_else(|| format!("\u{1}{}", strip_leading_v(version))) -} - /// Whether two composer version spellings name the same release /// (`3.0.2` ≡ `v3.0.2` ≡ `3.0.2.0`, `8.1.0-RC1` ≡ `v8.1.0-rc.1`). A spelling /// Composer rejects matches only another rejected spelling with the same /// `v`-stripped text. pub fn composer_versions_equivalent(a: &str, b: &str) -> bool { - composer_version_identity_key(a) == composer_version_identity_key(b) + composer_version_key(a) == composer_version_key(b) } #[cfg(test)] @@ -325,44 +313,14 @@ mod tests { assert!(failures.is_empty(), "{}", failures.join("\n")); } + /// [`composer_versions_equivalent`] (one key, no separate space for + /// spellings Composer rejects) against the spec: two accepted spellings + /// are equivalent when they normalize alike, two rejected ones when their + /// `v`-stripped text is equal, and never one of each. Sound only while + /// every accepted key `N` is itself accepted, bare and behind a `v` (the + /// only text a rejected spelling `N` / `vN` would key as). #[test] - fn identity_key_equality_is_equivalence_over_every_vector_pair() { - let v = vectors(); - let mut inputs: Vec<&str> = v["vectors"] - .as_array() - .unwrap() - .iter() - .map(|case| case["input"].as_str().unwrap()) - .collect(); - for case in v["equivalence"].as_array().unwrap() { - inputs.push(case["left"].as_str().unwrap()); - inputs.push(case["right"].as_str().unwrap()); - } - let mut failures = Vec::new(); - for a in &inputs { - for b in &inputs { - let keyed = composer_version_identity_key(a) == composer_version_identity_key(b); - let rejected = |x: &str| identity_normalize(x).is_none(); - let spec = match (rejected(a), rejected(b)) { - (false, false) => composer_version_key(a) == composer_version_key(b), - (true, true) => strip_leading_v(a) == strip_leading_v(b), - _ => false, - }; - if keyed != spec || keyed != composer_versions_equivalent(a, b) { - failures.push(format!("{a:?} vs {b:?}: key {keyed}, spec {spec}")); - } - } - } - assert!(failures.is_empty(), "{}", failures.join("\n")); - } - - /// [`PurlKey`](crate::utils::purl_key::PurlKey) keys on the sentinel-free - /// [`composer_version_key`], which is only sound while a rejected - /// spelling's key can never equal an accepted one's: every accepted key - /// `N` must itself be accepted, bare and behind a `v` (the only text a - /// rejected spelling `N` / `vN` would key as; a `v` only before a digit). - #[test] - fn sentinel_free_key_keeps_rejected_spellings_apart() { + fn key_equality_is_equivalence_over_every_vector_pair() { let v = vectors(); let mut inputs: Vec<&str> = v["vectors"] .as_array() @@ -392,11 +350,16 @@ mod tests { } } } + let rejected = |x: &str| identity_normalize(x).is_none(); for a in &inputs { for b in &inputs { - let plain = composer_version_key(a) == composer_version_key(b); - if plain != composer_versions_equivalent(a, b) { - failures.push(format!("{a:?} vs {b:?}: plain key {plain}")); + let spec = match (rejected(a), rejected(b)) { + (false, false) => identity_normalize(a) == identity_normalize(b), + (true, true) => strip_leading_v(a) == strip_leading_v(b), + _ => false, + }; + if composer_versions_equivalent(a, b) != spec { + failures.push(format!("{a:?} vs {b:?}: spec {spec}")); } } } diff --git a/crates/socket-patch-core/src/utils/containment.rs b/crates/socket-patch-core/src/utils/containment.rs index 3b32365a0..29790b84c 100644 --- a/crates/socket-patch-core/src/utils/containment.rs +++ b/crates/socket-patch-core/src/utils/containment.rs @@ -58,17 +58,13 @@ pub fn try_linked_level(root: &Path, path: &Path) -> std::io::Result bool { std::fs::symlink_metadata(path).is_ok_and(|meta| meta.file_type().is_symlink()) } -/// The refusal text for writing or deleting `what` through the linked -/// `link`. Every guarded writer and deleter shares it, so callers and tests -/// can match on [`LINKED_LEVEL_MARKER`]. -pub fn linked_level_refusal(action: &str, what: &Path, link: &Path) -> std::io::Error { +/// The refusal for writing or deleting `what` through the linked `link`. +fn linked_level_refusal(action: &str, what: &Path, link: &Path) -> std::io::Error { std::io::Error::other(format!( "refusing to {action} {}: {} {LINKED_LEVEL_MARKER}, and its target is not \ socket-patch's to change", @@ -89,20 +85,6 @@ pub fn ensure_unlinked(root: &Path, path: &Path, action: &str) -> std::io::Resul } } -/// The nearest ancestor of `path` (inclusive) literally named `.socket`. -pub fn nearest_socket_dir(path: &Path) -> Option<&Path> { - path.ancestors().find(|a| { - a.file_name() - .is_some_and(|n| n == crate::constants::SOCKET_DIR) - }) -} - -/// The project root a path under `.socket/` belongs to: the parent of -/// [`nearest_socket_dir`]. `None` when `path` is not under a `.socket/`. -pub fn socket_project_root(path: &Path) -> Option<&Path> { - nearest_socket_dir(path).and_then(Path::parent) -} - /// Whether `dir` really (canonically) lives inside `root`. Both must /// exist; an unresolvable path is reported as contained, so the caller's /// own missing-file handling runs instead of a spurious refusal. @@ -183,17 +165,6 @@ mod tests { ); } - #[test] - fn socket_project_root_is_the_parent_of_the_nearest_socket_dir() { - let p = Path::new("/a/b/.socket/vendor/state.json"); - assert_eq!(socket_project_root(p), Some(Path::new("/a/b"))); - assert_eq!( - socket_project_root(Path::new("/a/b/.socket")), - Some(Path::new("/a/b")) - ); - assert_eq!(socket_project_root(Path::new("/a/b/state.json")), None); - } - #[cfg(unix)] #[test] fn resolves_within_follows_links() { diff --git a/crates/socket-patch-core/src/utils/digest.rs b/crates/socket-patch-core/src/utils/digest.rs index 0ef654449..838d1fee7 100644 --- a/crates/socket-patch-core/src/utils/digest.rs +++ b/crates/socket-patch-core/src/utils/digest.rs @@ -50,7 +50,6 @@ pub(crate) fn sha1_hex(s: &str) -> Option { is_hex(s, 40).then(|| s.to_ascii_lowercase()) } -// ── computations ────────────────────────────────────────────────────────── // The `_of` suffix keeps a computation from sharing a name with the // validators above: `sha256_hex(&str)` checks a pin, `sha256_hex_of(&[u8])` // makes one. @@ -98,8 +97,7 @@ mod tests { assert!(is_sri_pin("sha512-abc") && !is_sri_pin("sha512-") && !is_sri_pin("md5-x")); } - /// Known vectors (FIPS 180 / RFC 3174 test strings `""` and `"abc"`) for - /// every computation, so each former inline copy's output is pinned. + /// Known vectors (FIPS 180 / RFC 3174 test strings `""` and `"abc"`). #[test] fn computations_match_known_vectors() { assert_eq!( @@ -130,10 +128,8 @@ mod tests { } } - /// Production files that still compute a digest inline, waiting on - /// #706 slice 2 (they are changed by open PRs). Drop a file from the list - /// when you move it onto the helpers above; the test fails on a stale - /// entry as well as on a new inline copy. + /// Production files that still compute a digest inline. Drop a file + /// when you move it onto the helpers above. const PENDING_INLINE_DIGESTS: &[&str] = &[ "utils/group_commit.rs", "vendor/jvm/mod.rs", diff --git a/crates/socket-patch-core/src/utils/purl_key.rs b/crates/socket-patch-core/src/utils/purl_key.rs index 379f42089..d9ae5740c 100644 --- a/crates/socket-patch-core/src/utils/purl_key.rs +++ b/crates/socket-patch-core/src/utils/purl_key.rs @@ -367,10 +367,6 @@ mod tests { "pkg:composer/psr/log@dev-Feature", "pkg:composer/psr/log@dev-feature" )); - // Rejected versions key without the internal sentinel. - assert!(!PurlKey::new("pkg:composer/psr/log@not-a-version") - .as_str() - .contains('\u{1}')); // Only composer and NuGet get release identity; other types stay // version-exact. assert!(!PurlKey::same("pkg:npm/x@1.0", "pkg:npm/x@1.0.0.0")); @@ -593,8 +589,7 @@ mod tests { /// The composer half of [`PurlKey`] is exactly composer release /// equivalence, over the whole shared vector file (pairs of accepted - /// AND rejected spellings), even though the key drops the internal - /// rejected-version sentinel. + /// AND rejected spellings). #[test] fn composer_key_equality_is_release_equivalence_over_every_vector_pair() { let v: serde_json::Value = serde_json::from_str(include_str!( diff --git a/crates/socket-patch-core/src/utils/socket_dir.rs b/crates/socket-patch-core/src/utils/socket_dir.rs index 26ede4354..0cc79c16c 100644 --- a/crates/socket-patch-core/src/utils/socket_dir.rs +++ b/crates/socket-patch-core/src/utils/socket_dir.rs @@ -105,6 +105,14 @@ fn guard_root(stop_dir: &Path) -> &Path { stop_dir.parent().unwrap_or(stop_dir) } +/// The nearest ancestor of `path` (inclusive) literally named `.socket`. +fn nearest_socket_dir(path: &Path) -> Option<&Path> { + path.ancestors().find(|a| { + a.file_name() + .is_some_and(|n| n == crate::constants::SOCKET_DIR) + }) +} + /// Persist a committed JSON ledger: pretty-printed with a trailing newline /// (deterministic bytes), parent directory created on demand, staged + /// fsync'd + renamed via [`atomic_write_bytes`]. A ledger already holding @@ -128,7 +136,7 @@ fn guard_root(stop_dir: &Path) -> &Path { /// never creates one there, so its target belongs to another project or /// lies outside this one (#887). pub(crate) async fn write_json_ledger(path: &Path, value: &T) -> std::io::Result<()> { - if let Some(root) = containment::socket_project_root(path) { + if let Some(root) = nearest_socket_dir(path).and_then(Path::parent) { containment::ensure_unlinked(root, path, "write")?; } let mut bytes = serde_json::to_vec_pretty(value).map_err(std::io::Error::other)?; @@ -145,7 +153,7 @@ pub(crate) async fn write_json_ledger(path: &Path, value: &T) -> s Ok(()) => Ok(()), Err(e) => { if created_parent { - if let Some(stop) = containment::nearest_socket_dir(parent) { + if let Some(stop) = nearest_socket_dir(parent) { prune_empty_dirs(parent, stop).await; } } @@ -158,6 +166,17 @@ pub(crate) async fn write_json_ledger(path: &Path, value: &T) -> s mod tests { use super::*; + #[test] + fn nearest_socket_dir_is_the_innermost_dot_socket_ancestor() { + let p = Path::new("/a/b/.socket/vendor/state.json"); + assert_eq!(nearest_socket_dir(p), Some(Path::new("/a/b/.socket"))); + assert_eq!( + nearest_socket_dir(Path::new("/a/.socket/b/.socket")), + Some(Path::new("/a/.socket/b/.socket")) + ); + assert_eq!(nearest_socket_dir(Path::new("/a/b/state.json")), None); + } + /// The climb removes every empty level below `stop_dir`, never /// `stop_dir` itself, and continues past a level that is already gone. #[tokio::test] From f9c3988f41e799961b08448e77b9062210ad6507 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 03:05:37 +0000 Subject: [PATCH 3/5] Trim utils/concurrent.rs to one cap constant MAX_API_CONCURRENCY equalled API_CONCURRENCY, so the env clamp uses API_CONCURRENCY. The consts no other file reads are private (PROXY_API_CONCURRENCY is pub(crate) for the API client). The adaptive cap's floor and ceiling parameters only ever took one value, so it is inlined into api_concurrency_for_under, which now reads the env override once. The per-window test asserts literal caps instead of recomputing them with the code under test. Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-core/src/utils/concurrent.rs | 88 +++++++------------ 1 file changed, 33 insertions(+), 55 deletions(-) diff --git a/crates/socket-patch-core/src/utils/concurrent.rs b/crates/socket-patch-core/src/utils/concurrent.rs index 37370149e..a1ea1984c 100644 --- a/crates/socket-patch-core/src/utils/concurrent.rs +++ b/crates/socket-patch-core/src/utils/concurrent.rs @@ -17,38 +17,34 @@ //! [`crate::api::client::hold_back_debug`] and releases it at the fold, so //! each request's lines print where the serial loop's would have and a //! window the caller drops (the batch fallback) announces nothing the -//! serial loop would not have announced. Relying on the order the futures -//! happen to be first-polled in would work today and is exactly what -//! `hold_back_debug` exists to stop depending on. +//! serial loop would not have announced. use std::future::Future; use futures_util::stream::{self, Stream, StreamExt}; -/// In-flight request cap for the authenticated patch API: the ceiling a -/// large window reaches ([`api_concurrency_for`]), and the whole cap for -/// the windows that do not size themselves. Measured with no 429s up to -/// 32 in flight; on a monorepo hosted scan at 100 ms of latency, 32 -/// instead of 8 cut the wall from ~20 s to ~9 s with identical output, -/// the network having run at ~7 of 8 slots. -pub const API_CONCURRENCY: usize = 32; +/// In-flight request cap for the authenticated patch API, and the most +/// [`API_CONCURRENCY_ENV`] can ask for: the ceiling a large window reaches +/// ([`api_concurrency_for`]), and the whole cap for the windows that do not +/// size themselves. The patch API answered 32 in flight with no 429s. +const API_CONCURRENCY: usize = 32; /// Floor of the authenticated API's per-window cap /// ([`api_concurrency_for`]): a window never runs fewer requests in flight /// than this (or than it has requests), however small it is. -pub const API_CONCURRENCY_FLOOR: usize = 8; +const API_CONCURRENCY_FLOOR: usize = 8; /// Requests each in-flight slot should have queued before an authenticated /// window widens past [`API_CONCURRENCY_FLOOR`]: a window of `n` requests /// runs `n / REQUESTS_PER_SLOT` at once, between the floor and /// [`API_CONCURRENCY`]. Each connection then pipelines several requests, /// instead of a short window opening a connection per request. -pub const REQUESTS_PER_SLOT: usize = 4; +const REQUESTS_PER_SLOT: usize = 4; /// In-flight request cap on the public patch proxy, which serializes /// anonymous callers behind one shared server-side semaphore — stay /// polite there. -pub const PROXY_API_CONCURRENCY: usize = 4; +pub(crate) const PROXY_API_CONCURRENCY: usize = 4; /// Operator override for the caps above: the escape hatch for an endpoint /// that caps in-flight requests per client (a self-hosted `--api-url`, a @@ -56,10 +52,6 @@ pub const PROXY_API_CONCURRENCY: usize = 4; /// serial. pub const API_CONCURRENCY_ENV: &str = "SOCKET_API_CONCURRENCY"; -/// Largest value [`API_CONCURRENCY_ENV`] can ask for — the most in-flight -/// requests the patch API was measured to answer without a 429. -const MAX_API_CONCURRENCY: usize = 32; - /// In-flight cap for pristine downloads from the PUBLIC package registries /// (npmjs.org, PyPI, crates.io, RubyGems, the Go proxy, Maven Central). /// Deliberately its own number, not [`API_CONCURRENCY`]: those hosts are @@ -68,7 +60,7 @@ const MAX_API_CONCURRENCY: usize = 32; /// hatch for an operator's *patch API* — turning it up must not turn a /// public registry up with it. 4 keeps the lockfile-only ladder latency- /// flat without bursting at anyone. -pub const REGISTRY_CONCURRENCY: usize = 4; +const REGISTRY_CONCURRENCY: usize = 4; /// The in-flight cap for pristine registry downloads; see /// [`REGISTRY_CONCURRENCY`]. No env override — a tight `RLIMIT_NOFILE` @@ -84,7 +76,7 @@ pub fn registry_concurrency() -> usize { /// authenticated API (`false`). /// /// [`API_CONCURRENCY_ENV`] overrides it, clamped to -/// `1..=MAX_API_CONCURRENCY`; an unset, empty or unparsable value leaves +/// `1..=API_CONCURRENCY`; an unset, empty or unparsable value leaves /// the default in place. On the public proxy the override can only LOWER /// the cap: that server's semaphore is shared across anonymous callers, so /// widening it is not one client's call to make. @@ -140,16 +132,12 @@ fn api_concurrency_for_under( requests: usize, fd_limit_is_tight: bool, ) -> usize { - if fd_limit_is_tight || use_public_proxy || api_concurrency_override().is_some() { + if fd_limit_is_tight || use_public_proxy { return api_concurrency_under(use_public_proxy, fd_limit_is_tight); } - adaptive_concurrency(requests, API_CONCURRENCY_FLOOR, API_CONCURRENCY) -} - -/// `requests / REQUESTS_PER_SLOT`, clamped to `floor..=ceiling` (a floor -/// above the ceiling yields the ceiling). -fn adaptive_concurrency(requests: usize, floor: usize, ceiling: usize) -> usize { - (requests / REQUESTS_PER_SLOT).clamp(floor.min(ceiling), ceiling) + api_concurrency_override().unwrap_or_else(|| { + (requests / REQUESTS_PER_SLOT).clamp(API_CONCURRENCY_FLOOR, API_CONCURRENCY) + }) } /// [`API_CONCURRENCY_ENV`] as a usable limit, or `None` when it is unset, @@ -163,7 +151,7 @@ fn api_concurrency_override() -> Option { return None; } match trimmed.parse::() { - Ok(limit) if limit > 0 => Some(limit.min(MAX_API_CONCURRENCY)), + Ok(limit) if limit > 0 => Some(limit.min(API_CONCURRENCY)), _ => { crate::utils::env_compat::debug_log( "debug", @@ -347,7 +335,7 @@ mod tests { assert_eq!(api_concurrency(false), 16); assert_eq!(api_concurrency(true), PROXY_API_CONCURRENCY); std::env::set_var(API_CONCURRENCY_ENV, "9999"); - assert_eq!(api_concurrency(false), MAX_API_CONCURRENCY); + assert_eq!(api_concurrency(false), API_CONCURRENCY); assert_eq!(api_concurrency(true), PROXY_API_CONCURRENCY); // Unusable values leave the defaults alone rather than failing the @@ -364,19 +352,6 @@ mod tests { } } - #[test] - fn adaptive_cap_scales_with_the_window_between_floor_and_ceiling() { - assert_eq!(adaptive_concurrency(0, 8, 32), 8); - assert_eq!(adaptive_concurrency(31, 8, 32), 8); - assert_eq!(adaptive_concurrency(40, 8, 32), 10); - assert_eq!(adaptive_concurrency(127, 8, 32), 31); - assert_eq!(adaptive_concurrency(128, 8, 32), 32); - assert_eq!(adaptive_concurrency(100_000, 8, 32), 32); - // A floor above the ceiling never lifts the cap past the ceiling. - assert_eq!(adaptive_concurrency(0, 16, 8), 8); - assert_eq!(adaptive_concurrency(1000, 8, 8), 8); - } - /// The per-window cap: adaptive on the authenticated API, and exactly /// [`api_concurrency`] whenever the proxy, the env override or a tight /// descriptor limit decides. Serial: SOCKET_* env is process-global. @@ -385,15 +360,20 @@ mod tests { fn per_window_cap_adapts_only_where_nothing_else_decides() { let orig = std::env::var(API_CONCURRENCY_ENV).ok(); std::env::remove_var(API_CONCURRENCY_ENV); - for n in [0, 1, 7, 8, 31, 32, 33, 64, 127, 128, 129, 560, 100_000] { - assert_eq!( - api_concurrency_for_under(false, n, false), - adaptive_concurrency(n, API_CONCURRENCY_FLOOR, API_CONCURRENCY), - "{n}" - ); - let cap = api_concurrency_for_under(false, n, false); - assert!(cap <= api_concurrency(false), "{n}"); - assert!(cap >= API_CONCURRENCY_FLOOR.min(API_CONCURRENCY), "{n}"); + // `n / 4` requests per slot, between a floor of 8 and a cap of 32. + for (n, cap) in [ + (0, 8), + (1, 8), + (31, 8), + (32, 8), + (40, 10), + (127, 31), + (128, 32), + (129, 32), + (560, 32), + (100_000, 32), + ] { + assert_eq!(api_concurrency_for_under(false, n, false), cap, "{n}"); assert_eq!( api_concurrency_for_under(true, n, false), PROXY_API_CONCURRENCY @@ -417,10 +397,8 @@ mod tests { } } std::env::set_var(API_CONCURRENCY_ENV, "eight"); - assert_eq!( - api_concurrency_for_under(false, 1000, false), - adaptive_concurrency(1000, API_CONCURRENCY_FLOOR, API_CONCURRENCY) - ); + assert_eq!(api_concurrency_for_under(false, 1000, false), 32); + assert_eq!(api_concurrency_for_under(false, 40, false), 10); match orig { Some(v) => std::env::set_var(API_CONCURRENCY_ENV, v), None => std::env::remove_var(API_CONCURRENCY_ENV), From 0a75e5fb7ae86ef20c2d4fb9633f402909754c90 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 03:13:45 +0000 Subject: [PATCH 4/5] Sweep utils durability, env_compat, failpoint durability.rs: barrier checks for pending work, hits its failpoint and runs barrier_blocking on the blocking pool instead of a second copy of the take/sync/restore sequence. sync_all_blocking is private, Pending::moved loses its dead if-let, history phrasing goes, and the test that only asserts Ok is named for what it checks. env_compat.rs: SOCKET_DEBUG and SOCKET_OFFLINE share one env_flag. failpoint.rs: hit matches the point once, then pauses or exits. Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-core/src/utils/durability.rs | 65 ++++++++----------- .../socket-patch-core/src/utils/env_compat.rs | 32 +++++---- .../socket-patch-core/src/utils/failpoint.rs | 11 ++-- 3 files changed, 47 insertions(+), 61 deletions(-) diff --git a/crates/socket-patch-core/src/utils/durability.rs b/crates/socket-patch-core/src/utils/durability.rs index 76b44cd9b..01af02657 100644 --- a/crates/socket-patch-core/src/utils/durability.rs +++ b/crates/socket-patch-core/src/utils/durability.rs @@ -14,7 +14,7 @@ //! `.cargo/config.toml`, the requirement/Pipfile/pyproject surfaces, and //! the ledgers (`.socket/vendor/state.json`, `redirect-state.json`). //! [`super::fs::atomic_write_bytes`] fsyncs the staged file and its -//! directory, as it always did. +//! directory. //! * **Content-verified artifacts** — what vendoring produces under //! `.socket/vendor///`: the patched copy trees, the `.tgz` / //! `.whl` / `.gem` / `.crate` / `.nupkg` / `.jar` + `.pom` artifacts and @@ -38,7 +38,7 @@ //! //! # Why a crash cannot corrupt a project //! -//! A crash (or power loss) can now lose artifact bytes that a durable write +//! A crash (or power loss) can lose artifact bytes that a durable write //! would have kept: a renamed-over artifact may come back empty, truncated, //! or missing after a reboot. That is safe because nothing trusts an //! artifact's bytes without re-verifying them, on every run: @@ -69,9 +69,9 @@ //! field makes the package manager refuse it; the re-run deferral that //! skips the pristine download hashes a file artifact before trusting //! it. (Some backends' in-sync checks look only for the artifact's -//! presence — pypi's among them, unchanged by this module — which is -//! why point 1 never leaves an artifact the committed state names -//! unsynced.) The marker is never a trust input at all. +//! presence, pypi's among them, which is why point 1 never leaves an +//! artifact the committed state names unsynced.) The marker is never a +//! trust input at all. //! //! So a crash can only lose an artifact nothing durable names yet, which //! the next run rebuilds exactly as it would one deleted by hand. @@ -117,17 +117,14 @@ impl Pending { touched = true; } } - let dirs: Vec = self + let renamed: Vec<(PathBuf, PathBuf)> = self .dirs .iter() - .filter(|d| d.starts_with(from)) - .cloned() + .filter_map(|d| Some((d.clone(), to.join(d.strip_prefix(from).ok()?)))) .collect(); - for dir in dirs { - self.dirs.remove(&dir); - if let Ok(rest) = dir.strip_prefix(from) { - self.dirs.insert(to.join(rest)); - } + for (old, new) in renamed { + self.dirs.remove(&old); + self.dirs.insert(new); touched = true; } if touched { @@ -162,28 +159,21 @@ pub(crate) fn moved(from: &Path, to: &Path) { /// pending. An artifact removed since it was written (a failed package's /// unwind) is skipped; any other fsync failure is returned, and the caller /// — a durable commit point — must not proceed, since what it would name -/// may not be on disk. A directory sync stays best-effort, as it always -/// was for the durable writer. A failed barrier keeps everything it took -/// pending, so the next barrier (a caller that carries on, or the one the -/// apply lock's release runs) syncs it again rather than skipping it. +/// may not be on disk. A directory sync stays best-effort. A failed +/// barrier keeps everything it took pending, so the next barrier (a caller +/// that carries on, or the one the apply lock's release runs) syncs it +/// again rather than skipping it. pub(crate) async fn barrier() -> std::io::Result<()> { - let Some((files, dirs)) = take_pending() else { - return Ok(()); - }; - crate::utils::failpoint::hit("durability_barrier"); - let synced = tokio::task::spawn_blocking(move || { - let result = sync_all_blocking(&files, &dirs); - (result, files, dirs) - }) - .await; - match synced { - Ok((Ok(()), _, _)) => Ok(()), - Ok((Err(e), files, dirs)) => { - restore_pending(files, dirs); - Err(e) + { + let pending = pending(); + if pending.files.is_empty() && pending.dirs.is_empty() { + return Ok(()); } - Err(_) => Err(std::io::Error::other("background task failed")), } + crate::utils::failpoint::hit("durability_barrier"); + tokio::task::spawn_blocking(barrier_blocking) + .await + .map_err(|_| std::io::Error::other("background task failed"))? } /// [`barrier`], blocking — for the apply lock's release, which runs it @@ -223,8 +213,8 @@ fn restore_pending(files: Vec, dirs: BTreeSet) { /// read-only descriptor. Windows' `FlushFileBuffers` needs a handle with /// write access — a read-only reopen fails with `ERROR_ACCESS_DENIED`, and /// a write reopen fails on a read-only artifact — so there the stage + rename -/// writer fsyncs every stage through its own handle before the rename, as -/// the durable writer always did, and the barrier has no file left to sync +/// writer fsyncs every stage through its own handle before the rename, and +/// the barrier has no file left to sync /// (directories are not synced off Unix either). Everything is still /// recorded, so the barrier runs at the same points on every platform. pub(crate) const DEFERS_FILE_SYNC: bool = cfg!(unix); @@ -232,10 +222,7 @@ pub(crate) const DEFERS_FILE_SYNC: bool = cfg!(unix); /// Plain-fsync every file and directory, then flush each device's cache /// once where the platform needs a separate call for that. Off Unix both /// were already synced when they were written ([`DEFERS_FILE_SYNC`]). -pub(crate) fn sync_all_blocking( - files: &[PathBuf], - dirs: &BTreeSet, -) -> std::io::Result<()> { +fn sync_all_blocking(files: &[PathBuf], dirs: &BTreeSet) -> std::io::Result<()> { // One open handle per device for the final cache flush. let mut flush: BTreeMap = BTreeMap::new(); let mut seen: BTreeSet<&Path> = BTreeSet::new(); @@ -354,7 +341,7 @@ mod tests { } #[test] - fn sync_skips_vanished_files_and_syncs_the_rest() { + fn sync_skips_vanished_files() { let tmp = tempfile::tempdir().unwrap(); let kept = tmp.path().join("kept"); std::fs::write(&kept, b"x").unwrap(); diff --git a/crates/socket-patch-core/src/utils/env_compat.rs b/crates/socket-patch-core/src/utils/env_compat.rs index 3cd675939..3fb0748ac 100644 --- a/crates/socket-patch-core/src/utils/env_compat.rs +++ b/crates/socket-patch-core/src/utils/env_compat.rs @@ -1,12 +1,14 @@ //! Environment-variable readers shared by the CLI and core, plus the //! accepted `SOCKET_CLI_*` peer aliases. -/// Check if debug mode is enabled via `SOCKET_DEBUG`. +/// Whether `name` is exactly `1` or `true`. +fn env_flag(name: &str) -> bool { + matches!(std::env::var(name).as_deref(), Ok("1" | "true")) +} + +/// Whether `SOCKET_DEBUG` turns debug output on. pub fn is_debug_enabled() -> bool { - matches!( - std::env::var("SOCKET_DEBUG").unwrap_or_default().as_str(), - "1" | "true" - ) + env_flag("SOCKET_DEBUG") } /// `message` made safe for a `SOCKET_DEBUG` line (every URL in it through @@ -26,17 +28,13 @@ pub fn debug_log(channel: &str, message: &str) { } } -/// Strict-airgap gate: `SOCKET_OFFLINE` is `"1"` or `"true"`. It lives -/// here as the single definition of the vocabulary shared by every offline -/// gate (telemetry kill-switch, API-client advisory and org-slug -/// auto-resolution). The CLI mirrors `--offline` (whose clap-side -/// bool parse accepts a wider vocabulary) into `SOCKET_OFFLINE=1` before -/// any of those gates run, so the env read alone is authoritative. +/// Strict-airgap gate shared by every offline check (telemetry kill-switch, +/// API-client advisory, org-slug auto-resolution). The CLI mirrors +/// `--offline` (whose clap parse accepts a wider vocabulary) into +/// `SOCKET_OFFLINE=1` before any of them run, so the env read alone is +/// authoritative. pub(crate) fn is_offline_env() -> bool { - matches!( - std::env::var("SOCKET_OFFLINE").unwrap_or_default().as_str(), - "1" | "true" - ) + env_flag("SOCKET_OFFLINE") } /// The public patch-API proxy base URL: `SOCKET_PROXY_URL`, defaulting to @@ -55,8 +53,8 @@ pub(crate) fn proxy_url_from_env() -> String { /// First entry is the canonical `SOCKET_*` name (what clap and core read); /// second is the accepted `SOCKET_CLI_*` peer name. These are **not** /// deprecated — promotion is silent and the canonical name simply wins -/// when both are set. The list is deliberately tight: `SOCKET_CLI_CONFIG` (ephemeral JSON override), -/// `SOCKET_CLI_API_PROXY` (an HTTP forward proxy — reqwest already honors +/// when both are set. The list is deliberately tight: `SOCKET_CLI_CONFIG` +/// (ephemeral JSON override), `SOCKET_CLI_API_PROXY` (an HTTP forward proxy — reqwest already honors /// `HTTP_PROXY`/`HTTPS_PROXY`), and `SOCKET_CLI_DEBUG` are intentionally /// not mirrored. pub const PEER_ENV_ALIASES: &[(&str, &str)] = &[ diff --git a/crates/socket-patch-core/src/utils/failpoint.rs b/crates/socket-patch-core/src/utils/failpoint.rs index ce29c491f..30d1bb243 100644 --- a/crates/socket-patch-core/src/utils/failpoint.rs +++ b/crates/socket-patch-core/src/utils/failpoint.rs @@ -49,7 +49,10 @@ pub fn hit(name: &str) { Some((p, n)) => (p, n.parse::().unwrap_or(1)), None => (point, 1), }; - if point == name && at == count && pause { + if point != name || at != count { + continue; + } + if pause { drop(hits); eprintln!("socket-patch: failpoint `{name}` #{count} paused"); if let Some(ready) = std::env::var_os("SOCKET_PATCH_FAILPOINT_READY") { @@ -59,10 +62,8 @@ pub fn hit(name: &str) { std::thread::sleep(std::time::Duration::from_secs(3600)); } } - if point == name && at == count { - eprintln!("socket-patch: failpoint `{name}` #{count} hit; exiting"); - std::process::exit(86); - } + eprintln!("socket-patch: failpoint `{name}` #{count} hit; exiting"); + std::process::exit(86); } } #[cfg(not(any(debug_assertions, feature = "failpoints")))] From 14f61b9165732f2e40777a6d1edf16b923e6c0a0 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 1 Nov 2026 03:27:26 +0000 Subject: [PATCH 5/5] Address review: clamp guard, barrier take, doc links The adaptive cap's clamp panics on a floor above the ceiling, so a compile-time assert now pins the constants' order. barrier takes the pending set first again, so a racing barrier can't pass the check and then find nothing, and both barriers share sync_or_restore. Public docs stop linking items this sweep made private. Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-core/src/utils/cargo_home.rs | 2 +- .../socket-patch-core/src/utils/concurrent.rs | 8 ++++--- .../src/utils/containment.rs | 4 ++-- .../socket-patch-core/src/utils/durability.rs | 22 ++++++++++--------- 4 files changed, 20 insertions(+), 16 deletions(-) diff --git a/crates/socket-patch-core/src/utils/cargo_home.rs b/crates/socket-patch-core/src/utils/cargo_home.rs index 7f0c2403b..ea69ec07e 100644 --- a/crates/socket-patch-core/src/utils/cargo_home.rs +++ b/crates/socket-patch-core/src/utils/cargo_home.rs @@ -1,6 +1,6 @@ //! The cargo home cargo itself would use: `$CARGO_HOME`, else //! `/.cargo` with `` resolved the way cargo's `home` crate -//! resolves it. Unlike [`super::fs::home_dir`], that falls back to the +//! resolves it. Unlike `fs::home_dir`, that falls back to the //! account's passwd entry on Unix when `HOME` is unset (`env -i`, systemd //! units, stripped CI and container entrypoints), so every cargo-home reader //! goes through [`cargo_home`] to probe the registry cache cargo builds from. diff --git a/crates/socket-patch-core/src/utils/concurrent.rs b/crates/socket-patch-core/src/utils/concurrent.rs index a1ea1984c..8fe6fbed9 100644 --- a/crates/socket-patch-core/src/utils/concurrent.rs +++ b/crates/socket-patch-core/src/utils/concurrent.rs @@ -33,6 +33,8 @@ const API_CONCURRENCY: usize = 32; /// ([`api_concurrency_for`]): a window never runs fewer requests in flight /// than this (or than it has requests), however small it is. const API_CONCURRENCY_FLOOR: usize = 8; +// `usize::clamp` panics on a floor above the ceiling. +const _: () = assert!(API_CONCURRENCY_FLOOR <= API_CONCURRENCY); /// Requests each in-flight slot should have queued before an authenticated /// window widens past [`API_CONCURRENCY_FLOOR`]: a window of `n` requests @@ -63,7 +65,7 @@ pub const API_CONCURRENCY_ENV: &str = "SOCKET_API_CONCURRENCY"; const REGISTRY_CONCURRENCY: usize = 4; /// The in-flight cap for pristine registry downloads; see -/// [`REGISTRY_CONCURRENCY`]. No env override — a tight `RLIMIT_NOFILE` +/// `REGISTRY_CONCURRENCY`. No env override — a tight `RLIMIT_NOFILE` /// still forces one at a time, for the reason [`api_concurrency`] gives. pub fn registry_concurrency() -> usize { if crate::crawlers::walk_pool::fd_limit_is_tight() { @@ -114,8 +116,8 @@ fn api_concurrency_under(use_public_proxy: bool, fd_limit_is_tight: bool) -> usi /// public proxy (`true`) or the authenticated API (`false`). /// /// On the authenticated API it adapts to the window: -/// `requests / REQUESTS_PER_SLOT`, clamped between [`API_CONCURRENCY_FLOOR`] -/// and [`API_CONCURRENCY`], so a big window fills the cap and a small one +/// `requests / REQUESTS_PER_SLOT`, clamped between `API_CONCURRENCY_FLOOR` +/// and `API_CONCURRENCY`, so a big window fills the cap and a small one /// stays at the floor. Everything else is [`api_concurrency`]'s: the public /// proxy's fixed cap, [`API_CONCURRENCY_ENV`] (which replaces the adaptive /// value outright), and a tight `RLIMIT_NOFILE` forcing 1. diff --git a/crates/socket-patch-core/src/utils/containment.rs b/crates/socket-patch-core/src/utils/containment.rs index 29790b84c..b32cf112b 100644 --- a/crates/socket-patch-core/src/utils/containment.rs +++ b/crates/socket-patch-core/src/utils/containment.rs @@ -73,10 +73,10 @@ fn linked_level_refusal(action: &str, what: &Path, link: &Path) -> std::io::Erro )) } -/// The substring every [`linked_level_refusal`] carries. +/// The substring every linked-level refusal carries. pub const LINKED_LEVEL_MARKER: &str = "is a symlink"; -/// Err with [`linked_level_refusal`] when any level of `path` below `root` +/// Err with a refusal carrying [`LINKED_LEVEL_MARKER`] when any level of `path` below `root` /// is a link, before the caller writes or deletes anything. pub fn ensure_unlinked(root: &Path, path: &Path, action: &str) -> std::io::Result<()> { match linked_level(root, path) { diff --git a/crates/socket-patch-core/src/utils/durability.rs b/crates/socket-patch-core/src/utils/durability.rs index 01af02657..3567afb3f 100644 --- a/crates/socket-patch-core/src/utils/durability.rs +++ b/crates/socket-patch-core/src/utils/durability.rs @@ -164,14 +164,11 @@ pub(crate) fn moved(from: &Path, to: &Path) { /// that carries on, or the one the apply lock's release runs) syncs it /// again rather than skipping it. pub(crate) async fn barrier() -> std::io::Result<()> { - { - let pending = pending(); - if pending.files.is_empty() && pending.dirs.is_empty() { - return Ok(()); - } - } + let Some((files, dirs)) = take_pending() else { + return Ok(()); + }; crate::utils::failpoint::hit("durability_barrier"); - tokio::task::spawn_blocking(barrier_blocking) + tokio::task::spawn_blocking(move || sync_or_restore(files, dirs)) .await .map_err(|_| std::io::Error::other("background task failed"))? } @@ -181,9 +178,14 @@ pub(crate) async fn barrier() -> std::io::Result<()> { /// point (a rebuild that left every lockfile and the ledger unchanged) is /// synced before the command returns. pub(crate) fn barrier_blocking() -> std::io::Result<()> { - let Some((files, dirs)) = take_pending() else { - return Ok(()); - }; + match take_pending() { + Some((files, dirs)) => sync_or_restore(files, dirs), + None => Ok(()), + } +} + +/// Sync what a barrier took, putting it back as pending if that fails. +fn sync_or_restore(files: Vec, dirs: BTreeSet) -> std::io::Result<()> { let result = sync_all_blocking(&files, &dirs); if result.is_err() { restore_pending(files, dirs);