From 4816a8c1bc8959a20c3d2aa06b4e02083b4d39e8 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Thu, 13 Aug 2026 15:33:53 -0700 Subject: [PATCH 1/5] =?UTF-8?q?fix(gem):=20bundler=20vendored+hosted=20swe?= =?UTF-8?q?ep=20=E2=80=94=2013=20confirmed=20bugs=20fixed,=20tests=20harde?= =?UTF-8?q?ned?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ULTRACODE review + full test matrix over the gem/bundler ecosystem (vendored and hosted modes, every configuration). 24 findings survived adversarial verification; the 13 well-scoped ones are fixed here, each pinned by a test that fails without its fix. Hosted redirect (patch/redirect/mod.rs, gem section): - splice the Gemfile edit by regex byte range: a commented-out duplicate of the gem line no longer gets rewritten instead of the live line - grant-agnostic idempotency guard: re-running scan --mode hosted with a rotated grant token refreshes the source URL in place instead of nesting a second source block (new edit kind redirect_gemfile_source_url) - fail closed on gem-level git:/github:/path:/source: options (they override the enclosing source block, making the redirect an attested no-op); warn redirect_gem_source_option and skip the dep - fail closed on platform-suffixed CHECKSUMS siblings (redirect_gem_platform_unsupported) instead of inserting a duplicate bare-coordinate pin - recognize paren/tab/multi-space gem declarations; gate the append-branch on the gem being genuinely undeclared (no more duplicate declarations bundler rejects) - never pin the lock CHECKSUMS when the Gemfile source redirect did not land (mixed state guaranteed a checksum failure) - warn that a redirected pair breaks frozen/deployment installs - record the upstream sha256 line as original on the CHECKSUMS edit so a future revert can restore it (golden fixture updated) Vendored backend (vendor/gem.rs): - insert new PATH sections at bundler's sorted position (identifier order, verified against real bundler 4.0.15 bundle lock) — two or more vendored gems no longer churn the committed lock - fail closed on platform-suffixed GEM-specs siblings on no-CHECKSUMS locks (mirrors the existing CHECKSUMS guard) - re-vendor on a patch UPDATE (new uuid, same purl): recognize our own path: wiring and rewire in place instead of refusing with gemfile_declaration_not_editable — the documented automatic re-vendor contract now actually works for gem Auto-fetch (vendor/registry_fetch.rs): - stage fetched gems into the canonical - leaf instead of a dir literally named "gem", which vendor_gem refused as platform_gem_unsupported — lockfile auto-fetch for gems was dead Crawler (crawlers/ruby_crawler.rs): - parse_dir_name_version prefers the last dotted-version boundary, so http-2-1.0.1 parses as (http-2, 1.0.1) instead of the ghost (http, 2) - vendor/bundle discovery enumerates engine dirs (jruby, truffleruby) instead of hardcoding ruby/ Scan/get plumbing: - run_nested_apply now threads --ecosystems: scan --ecosystems gem --sync no longer applies (or mutates) other ecosystems' patches - scan --vendor --dry-run --vex no longer writes the VEX file nor exits 1 on not-yet-vendored state Test hardening: - e2e_gem lifecycle harness: BUNDLE_PATH replaces bundle install --path (removed in bundler 3+; all 3 lifecycle tests green under 4.0.15) - e2e_hosted_production gem leg now asserts the CHECKSUMS digest CHANGED after redirect (client-verifiable today) and, in the success arm, verifies installed content against the published afterHashes — an inert gem patch can no longer stay green (the npm minimist blindspot) - docker_e2e_gem serves the true git-blob beforeHash so the chain exercises the default non-forced apply path, not just --force - setup_matrix_gem module doc: the with-setup Docker cases ARE still a baseline gap (bootstrap deadlock: plugin registration evaluates plugins.rb before any gems land; exit-semantics twin), doc corrected Verified: core 2460/0, cli lib 350/0, clippy+fmt clean; e2e_gem 11/11 (incl. live lifecycle under bundler 4.0.15), e2e_vendor_gem_build 6/6 (incl. real-bundler capstone), docker_e2e_gem 2/2, docker_e2e_vendor_gem 1/1, hosted production gem leg green (redirect verified; install still blocked by the known depscan#23630 compact-index 404 — server-side). Co-Authored-By: Claude Fable 5 --- crates/socket-patch-cli/src/commands/get.rs | 24 +- .../socket-patch-cli/src/commands/scan/mod.rs | 1 + .../src/commands/scan/vendor_flow.rs | 17 +- .../socket-patch-cli/tests/docker_e2e_gem.rs | 112 ++- crates/socket-patch-cli/tests/e2e_gem.rs | 11 +- .../tests/e2e_hosted_production.rs | 169 ++++ .../tests/in_process_get_manifest_path.rs | 181 +++++ .../tests/in_process_get_update_count.rs | 1 + .../socket-patch-cli/tests/in_process_scan.rs | 107 +++ .../tests/setup_matrix_gem.rs | 16 +- .../src/crawlers/ruby_crawler.rs | 162 +++- .../src/patch/redirect/mod.rs | 563 +++++++++++++- crates/socket-patch-core/src/vendor/gem.rs | 735 +++++++++++++++++- .../src/vendor/registry_fetch.rs | 45 +- .../tests/crawler_ruby_e2e.rs | 41 + .../gem/bundler/basic/expected-edits.json | 1 + 16 files changed, 2095 insertions(+), 91 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/get.rs b/crates/socket-patch-cli/src/commands/get.rs index e9d1bf94..60ffec69 100644 --- a/crates/socket-patch-cli/src/commands/get.rs +++ b/crates/socket-patch-cli/src/commands/get.rs @@ -683,6 +683,11 @@ pub struct DownloadParams { /// `--strict` forwarded to the nested apply (a beforeHash mismatch /// fails instead of warn-and-overwrite). pub strict: bool, + /// `--ecosystems` forwarded to the nested apply. Without this the + /// nested apply ran UNSCOPED over the whole manifest, so + /// `scan --ecosystems gem --sync` could mutate other ecosystems' + /// packages the user had explicitly filtered out. + pub ecosystems: Option>, /// Persist downloaded blob content into `.socket/blobs` (the apply /// flows need it for later hook/rollback runs). Vendor flows pass /// `false`: their patch content is staged in memory and the committed @@ -1098,6 +1103,7 @@ async fn run_nested_apply( download_mode: String, strict: bool, api: socket_patch_core::api::client::ApiClientEnvOverrides, + ecosystems: Option>, ) -> bool { // Apply re-resolves a relative manifest path against ITS `--cwd` // (`resolved_manifest_path`), but ours is already cwd-resolved — @@ -1119,6 +1125,11 @@ async fn run_nested_apply( api_token: api.api_token, org: api.org_slug, proxy_url: api.proxy_url, + // Scope the nested apply like the caller was scoped: leaving + // this at the default `None` made `scan --ecosystems gem --sync` + // apply the WHOLE manifest, mutating other ecosystems' packages + // the user filtered out. + ecosystems, ..crate::args::GlobalArgs::default() }, force: false, @@ -1232,10 +1243,7 @@ pub async fn download_and_apply_patches( // status/exit code degrade and it is never auto-applied. if files.is_empty() { if !params.json && !params.silent { - eprintln!( - " [fail] {} (patch has no applicable files)", - patch.purl - ); + eprintln!(" [fail] {} (patch has no applicable files)", patch.purl); } downloaded_patches.push(serde_json::json!({ "purl": patch.purl, @@ -1393,6 +1401,7 @@ pub async fn download_and_apply_patches( params.download_mode.clone(), params.strict, resolved_api_overrides(params), + params.ecosystems.clone(), ) .await; } @@ -1824,6 +1833,7 @@ pub async fn run(args: GetArgs) -> i32 { api_overrides: args.common.api_client_overrides(), all_releases: args.all_releases, strict: args.common.strict, + ecosystems: args.common.ecosystems.clone(), persist_blobs: true, }; @@ -2034,6 +2044,7 @@ async fn save_and_apply_patch(args: &GetArgs, patch: &PatchResponse) -> i32 { args.common.download_mode.clone(), args.common.strict, args.common.api_client_overrides(), + args.common.ecosystems.clone(), ) .await; } @@ -3106,7 +3117,10 @@ mod tests { // record — the guardrail-triggering condition the download/apply // flows now count as failed rather than applied. let mut broken = HashMap::new(); - broken.insert("src/lib.rs".to_string(), file_resp(Some(&"e".repeat(64)), None)); + broken.insert( + "src/lib.rs".to_string(), + file_resp(Some(&"e".repeat(64)), None), + ); let broken_patch = patch_with_files(broken); assert!( files_for_manifest(&broken_patch).is_empty(), diff --git a/crates/socket-patch-cli/src/commands/scan/mod.rs b/crates/socket-patch-cli/src/commands/scan/mod.rs index b76700ab..eefed677 100644 --- a/crates/socket-patch-cli/src/commands/scan/mod.rs +++ b/crates/socket-patch-cli/src/commands/scan/mod.rs @@ -411,6 +411,7 @@ fn download_params(args: &ScanArgs, save_only: bool, json: bool, silent: bool) - api_overrides: args.common.api_client_overrides(), all_releases: args.all_releases, strict: args.common.strict, + ecosystems: args.common.ecosystems.clone(), persist_blobs: args.mode != Some(ScanMode::Vendored), } } diff --git a/crates/socket-patch-cli/src/commands/scan/vendor_flow.rs b/crates/socket-patch-cli/src/commands/scan/vendor_flow.rs index 4851dad4..5927ed20 100644 --- a/crates/socket-patch-cli/src/commands/scan/vendor_flow.rs +++ b/crates/socket-patch-cli/src/commands/scan/vendor_flow.rs @@ -229,7 +229,9 @@ async fn run_scan_vendor_step( } /// The `scan --vendor` JSON path: discovery → (dry-run preview | download -/// → GC → vendor engine) → embedded VEX → print `result` → exit code. +/// → GC → vendor engine → embedded VEX) → print `result` → exit code. +/// The dry-run arm skips the VEX embed (emitting a `vex.skipped` marker +/// instead): a dry run vendors nothing, so there is no state to attest. /// /// Extracted from `run` (and called through `Box::pin`) so its sizeable /// temporaries get their own poll frame, entered only when `--vendor` is @@ -285,10 +287,17 @@ async fn run_vendor_json_path( ) .await; } - let final_code = - embed_vex_into_json(&args.common, &args.vex, manifest_path, 0, result).await; + // Embedded VEX is skipped on a dry run (apply.rs's precedent): + // nothing was vendored, so there is no just-vendored state to + // attest — generating here would verify the deliberately untouched + // tree (failing outright on a not-yet-vendored project) and write + // an attestation file during --dry-run. The marker keeps the + // request visible to JSON consumers instead of silently dropping it. + if args.vex.vex.is_some() { + result["vex"] = serde_json::json!({ "skipped": true, "reason": "dry_run" }); + } println!("{}", serde_json::to_string_pretty(&result).unwrap()); - return final_code; + return 0; } // 1) Download phase. Manifest mode reuses the `--apply` diff --git a/crates/socket-patch-cli/tests/docker_e2e_gem.rs b/crates/socket-patch-cli/tests/docker_e2e_gem.rs index c85450f9..e906f10d 100644 --- a/crates/socket-patch-cli/tests/docker_e2e_gem.rs +++ b/crates/socket-patch-cli/tests/docker_e2e_gem.rs @@ -9,6 +9,15 @@ //! - `gem_global_install_full_apply_chain` — `gem install` without //! --install-dir, installs to the system gem directory; socket-patch //! scans + applies with `--global`. +//! +//! The fixture serves the TRUE git-blob sha256 of the installed +//! `lib/colorize.rb` as `beforeHash` (computed once by a probe container +//! from the real upstream artifact — see [`upstream_before_hash`]), so +//! both apply paths run gated, without `--force`: `scan --sync`'s own +//! nested apply must patch the file in the same run, and the explicit +//! `apply` (against restored pristine bytes) must pass the variant gate. +//! With the old all-zeros placeholder the nested apply failed invisibly +//! and only the `--force` escape hatch was ever exercised. #![cfg(feature = "docker-e2e")] @@ -69,6 +78,52 @@ fn plain_sha256(content: &[u8]) -> String { hex::encode(hasher.finalize()) } +/// Probe: install colorize 1.1.0 from the real registry and emit the +/// git-blob sha256 of the exact `lib/colorize.rb` bytes `gem install` +/// lays down — the value the fixture must serve as `beforeHash` for the +/// default (no `--force`) apply path to pass the variant gate. +const BEFORE_HASH_PROBE_SCRIPT: &str = r#"#!/usr/bin/env bash +set -uo pipefail +gem install --no-document --install-dir /tmp/probe colorize -v 1.1.0 > /tmp/install.log 2>&1 || { + cat /tmp/install.log >&2; exit 1 +} +F=/tmp/probe/gems/colorize-1.1.0/lib/colorize.rb +[ -f "$F" ] || { echo "FAIL: $F missing" >&2; exit 1; } +{ printf 'blob %d\0' "$(wc -c < "$F")"; cat "$F"; } | sha256sum | cut -d' ' -f1 +"#; + +/// True git-blob sha256 of the colorize-1.1.0 `lib/colorize.rb` that +/// `gem install` produces, computed once per process by +/// [`BEFORE_HASH_PROBE_SCRIPT`] in a probe container. Serving this as the +/// fixture's `beforeHash` — instead of an all-zeros placeholder — is what +/// lets the gated apply paths run: with the placeholder, `scan --sync`'s +/// nested apply hit the variant gate and failed invisibly, so the chain +/// only ever proved the `--force` path. +fn upstream_before_hash() -> String { + static HASH: std::sync::OnceLock = std::sync::OnceLock::new(); + HASH.get_or_init(|| { + let out = run_container(BEFORE_HASH_PROBE_SCRIPT); + let stdout = String::from_utf8_lossy(&out.stdout); + let stderr = String::from_utf8_lossy(&out.stderr); + assert!( + out.status.success(), + "beforeHash probe container failed:\nstdout=\n{stdout}\nstderr=\n{stderr}" + ); + stdout + .lines() + .rev() + .map(str::trim) + .find(|l| l.len() == 64 && l.bytes().all(|b| b.is_ascii_hexdigit())) + .unwrap_or_else(|| { + panic!( + "beforeHash probe emitted no 64-hex git-blob sha256:\nstdout=\n{stdout}\nstderr=\n{stderr}" + ) + }) + .to_string() + }) + .clone() +} + /// Shared verification block for both scripts. Expects `GEM_FILE`, /// `EXPECTED_SHA`, and `APPLY_EXIT` to be set, plus the JSON captured in /// `/tmp/scan.json` and `/tmp/apply.json`. @@ -123,7 +178,7 @@ exit 0 "# } -async fn make_mock_server(after_hash: &str) -> MockServer { +async fn make_mock_server(before_hash: &str, after_hash: &str) -> MockServer { let listener = std::net::TcpListener::bind("0.0.0.0:0").expect("bind wiremock"); let server = MockServer::builder().listener(listener).start().await; @@ -169,9 +224,12 @@ async fn make_mock_server(after_hash: &str) -> MockServer { "publishedAt": "2024-01-01T00:00:00Z", "files": { // gem uses `package/` (npm-style) — apply strips - // the prefix and joins with the gem dir. + // the prefix and joins with the gem dir. beforeHash is + // the TRUE git-blob sha256 of the installed upstream + // file so the default (gated, no --force) apply path is + // what the chain exercises. "package/lib/colorize.rb": { - "beforeHash": "0000000000000000000000000000000000000000000000000000000000000000", + "beforeHash": before_hash, "afterHash": after_hash, "blobContent": blob_b64, } @@ -213,6 +271,9 @@ gem install --no-document --install-dir "$INSTALL_DIR" colorize -v 1.1.0 > /tmp/ GEM_FILE="$INSTALL_DIR/gems/colorize-1.1.0/lib/colorize.rb" [ -f "$GEM_FILE" ] || {{ echo "FAIL: $GEM_FILE missing" >&2; exit 1; }} echo "Installed to: $GEM_FILE" >&2 +# Keep a pristine copy: the explicit apply below is exercised against it +# after scan --sync's own nested apply has already patched the live file. +cp "$GEM_FILE" /tmp/pristine.rb # Pre-seed setup.manual so the agent-mode VEX leg keeps the gem patch through # property 7 (this project isn't `socket-patch setup`-configured; agent patches @@ -229,7 +290,19 @@ socket-patch scan --json --sync --yes \ --ecosystems gem > /tmp/scan.json 2>/tmp/sync.err cat /tmp/sync.err >&2 -socket-patch apply --json --force --offline --ecosystems gem > /tmp/apply.json 2>/tmp/apply.err +# The fixture serves the TRUE beforeHash, so scan --sync's own nested apply +# (which never uses --force) must pass the variant gate and patch the file +# in the same run — with an all-zeros placeholder this failed invisibly. +grep -q 'SOCKET-PATCH-E2E-MARKER' "$GEM_FILE" || {{ + echo "FAIL: scan --sync's nested apply left $GEM_FILE unpatched" >&2 + cat /tmp/scan.json >&2; head -3 "$GEM_FILE" >&2; exit 1; }} + +# Restore the pristine file so the explicit apply below exercises the +# default (gated, no --force) path end to end instead of short-circuiting +# on an already-patched file. +cp /tmp/pristine.rb "$GEM_FILE" + +socket-patch apply --json --offline --ecosystems gem > /tmp/apply.json 2>/tmp/apply.err APPLY_EXIT=$? cat /tmp/apply.err >&2 @@ -276,6 +349,9 @@ GEM_DIR=$(gem env gemdir) GEM_FILE="$GEM_DIR/gems/colorize-1.1.0/lib/colorize.rb" [ -f "$GEM_FILE" ] || {{ echo "FAIL: $GEM_FILE missing" >&2; exit 1; }} echo "Global-installed at: $GEM_FILE" >&2 +# Keep a pristine copy: the explicit apply below is exercised against it +# after scan --sync's own nested apply has already patched the live file. +cp "$GEM_FILE" /tmp/pristine.rb mkdir -p /workspace/proj && cd /workspace/proj @@ -285,7 +361,19 @@ socket-patch scan --json --sync --yes --global \ --ecosystems gem > /tmp/scan.json 2>/tmp/sync.err cat /tmp/sync.err >&2 -socket-patch apply --json --force --offline --global --ecosystems gem > /tmp/apply.json 2>/tmp/apply.err +# The fixture serves the TRUE beforeHash, so scan --sync's own nested apply +# (which never uses --force) must pass the variant gate and patch the file +# in the same run — with an all-zeros placeholder this failed invisibly. +grep -q 'SOCKET-PATCH-E2E-MARKER' "$GEM_FILE" || {{ + echo "FAIL: scan --sync's nested apply left $GEM_FILE unpatched" >&2 + cat /tmp/scan.json >&2; head -3 "$GEM_FILE" >&2; exit 1; }} + +# Restore the pristine file so the explicit apply below exercises the +# default (gated, no --force) path end to end instead of short-circuiting +# on an already-patched file. +cp /tmp/pristine.rb "$GEM_FILE" + +socket-patch apply --json --offline --global --ecosystems gem > /tmp/apply.json 2>/tmp/apply.err APPLY_EXIT=$? cat /tmp/apply.err >&2 {verify}"# @@ -390,12 +478,13 @@ async fn assert_api_path_exercised(server: &MockServer) { #[tokio::test] async fn gem_local_install_full_apply_chain() { - let after_hash = git_sha256(PATCHED_RB); - let server = make_mock_server(&after_hash).await; - let api_url = format!("http://host.docker.internal:{}", server.address().port()); if skip_if_no_image() { return; } + let before_hash = upstream_before_hash(); + let after_hash = git_sha256(PATCHED_RB); + let server = make_mock_server(&before_hash, &after_hash).await; + let api_url = format!("http://host.docker.internal:{}", server.address().port()); let expected_sha = plain_sha256(PATCHED_RB); let out = run_container(&local_script(&api_url, &expected_sha)); let stdout = String::from_utf8_lossy(&out.stdout); @@ -418,12 +507,13 @@ async fn gem_local_install_full_apply_chain() { #[tokio::test] async fn gem_global_install_full_apply_chain() { - let after_hash = git_sha256(PATCHED_RB); - let server = make_mock_server(&after_hash).await; - let api_url = format!("http://host.docker.internal:{}", server.address().port()); if skip_if_no_image() { return; } + let before_hash = upstream_before_hash(); + let after_hash = git_sha256(PATCHED_RB); + let server = make_mock_server(&before_hash, &after_hash).await; + let api_url = format!("http://host.docker.internal:{}", server.address().port()); let expected_sha = plain_sha256(PATCHED_RB); let out = run_container(&global_script(&api_url, &expected_sha)); let stdout = String::from_utf8_lossy(&out.stdout); diff --git a/crates/socket-patch-cli/tests/e2e_gem.rs b/crates/socket-patch-cli/tests/e2e_gem.rs index f9dc8120..9fb92a72 100644 --- a/crates/socket-patch-cli/tests/e2e_gem.rs +++ b/crates/socket-patch-cli/tests/e2e_gem.rs @@ -94,6 +94,11 @@ fn assert_run_ok(cwd: &Path, args: &[&str], context: &str) -> (String, String) { fn bundle_run(cwd: &Path, args: &[&str]) { let mut cmd = Command::new("bundle"); cmd.args(args).current_dir(cwd); + // Bundler 4 removed `bundle install --path`; BUNDLE_PATH is honored by + // bundler 2 through 4 and keeps the vendor/bundle/ruby/*/gems layout + // `find_gem_dir` expects. It also upholds cache_env's hermeticity + // invariant that every `bundle install` pins its gem tree to the fixture. + cmd.env("BUNDLE_PATH", "vendor/bundle"); cache_env::isolate(&mut cmd); let out = cmd.output().expect("failed to run bundle"); assert!( @@ -449,7 +454,7 @@ fn test_gem_full_lifecycle() { // -- Setup: create project and install activestorage@5.2.0 ---------------- write_gemfile(cwd); - bundle_run(cwd, &["install", "--path", "vendor/bundle"]); + bundle_run(cwd, &["install"]); let gem_dir = find_gem_dir(cwd); @@ -540,7 +545,7 @@ fn test_gem_dry_run() { let cwd = dir.path(); write_gemfile(cwd); - bundle_run(cwd, &["install", "--path", "vendor/bundle"]); + bundle_run(cwd, &["install"]); let gem_dir = find_gem_dir(cwd); @@ -579,7 +584,7 @@ fn test_gem_save_only() { let cwd = dir.path(); write_gemfile(cwd); - bundle_run(cwd, &["install", "--path", "vendor/bundle"]); + bundle_run(cwd, &["install"]); let gem_dir = find_gem_dir(cwd); diff --git a/crates/socket-patch-cli/tests/e2e_hosted_production.rs b/crates/socket-patch-cli/tests/e2e_hosted_production.rs index 1356509e..437b3092 100644 --- a/crates/socket-patch-cli/tests/e2e_hosted_production.rs +++ b/crates/socket-patch-cli/tests/e2e_hosted_production.rs @@ -558,6 +558,82 @@ async fn published_patch_dates(purl: &str) -> Result, Stri .unwrap_or_default()) } +/// `GET /patch/view/` against the real proxy — the same route the +/// CLI's free-proxy client fetches patch content from. Returns, per file the +/// patch touches, the `(path, beforeHash, afterHash)` triple (hashes are +/// git-blob sha256, `None` for pure additions/deletions respectively). +async fn published_patch_files( + uuid: &str, +) -> Result, Option)>, String> { + let url = format!("{PROXY}/patch/view/{uuid}"); + let resp = reqwest::Client::new() + .get(&url) + .header("Accept", "application/json") + .send() + .await + .map_err(|e| format!("GET {url}: {e}"))?; + let status = resp.status(); + let body = resp + .text() + .await + .map_err(|e| format!("GET {url}: reading body: {e}"))?; + if !status.is_success() { + return Err(format!("GET {url}: HTTP {status}\n{body}")); + } + let v: serde_json::Value = + serde_json::from_str(&body).map_err(|e| format!("GET {url}: bad JSON ({e}):\n{body}"))?; + Ok(v["files"] + .as_object() + .map(|m| { + m.iter() + .map(|(path, f)| { + ( + path.clone(), + f["beforeHash"].as_str().map(str::to_string), + f["afterHash"].as_str().map(str::to_string), + ) + }) + .collect() + }) + .unwrap_or_default()) +} + +/// The `sha256=` hex value the lock's CHECKSUMS section pins for +/// ` ()`, or `None` when the entry is absent. Bundler >= 2.6 +/// writes one two-space-indented ` name (version) sha256=` line per +/// resolved gem. +fn gem_lock_checksum(lock: &str, name: &str, version: &str) -> Option { + let prefix = format!(" {name} ({version}) sha256="); + lock.lines() + .find_map(|l| l.strip_prefix(&prefix).map(|h| h.trim().to_string())) +} + +/// Locate the bundler-installed `gems/-` directory under a +/// `BUNDLE_PATH` root. The `ruby/` segment in between varies by host +/// interpreter, so walk for it (depth-bounded — the layout is only a few +/// levels deep) instead of hardcoding the version. +fn installed_gem_dir(root: &Path, dir_name: &str, depth: usize) -> Option { + for entry in std::fs::read_dir(root).ok()?.flatten() { + let p = entry.path(); + if !p.is_dir() { + continue; + } + if p.file_name().is_some_and(|n| n == dir_name) + && p.parent() + .and_then(|d| d.file_name()) + .is_some_and(|n| n == "gems") + { + return Some(p); + } + if depth > 0 { + if let Some(found) = installed_gem_dir(&p, dir_name, depth - 1) { + return Some(found); + } + } + } + None +} + /// The Socket patch-registry base URL the gem rewriter pinned into `Gemfile` /// as `source "" do`, or `None` when no Socket source block is present. /// @@ -1621,6 +1697,19 @@ async fn gem_bundler_hosted_redirect_and_known_install_defect() { CHECKSUMS section)" ); } + // Capture the UPSTREAM checksum pin before any redirect. Bundler installs + // whatever matches this pin, so the redirect must replace it with the + // patched artifact's digest — the assertion after the redirect below is + // what makes an inert rewrite (URL repointed, upstream digest kept, so + // bundler verify-and-installs the UNPATCHED gem) go red instead of green. + let pristine_lock = read(&proj.join("Gemfile.lock")); + let upstream_sha = + gem_lock_checksum(&pristine_lock, GEM_NAME, GEM_VERSION).unwrap_or_else(|| { + panic!( + "{LEG}: `bundle lock --add-checksums` wrote no sha256 CHECKSUMS \ + entry for {GEM_NAME} ({GEM_VERSION}):\n{pristine_lock}" + ) + }); let install = tool(&proj, "bundle", &["install", "--quiet"], &env); if !ok(&install) { soft_skip!(LEG, "upstream `bundle install` failed:\n{}", dump(&install)); @@ -1642,6 +1731,19 @@ async fn gem_bundler_hosted_redirect_and_known_install_defect() { lock.contains("CHECKSUMS"), "{LEG}: Gemfile.lock lost its CHECKSUMS section:\n{lock}" ); + let redirected_sha = gem_lock_checksum(&lock, GEM_NAME, GEM_VERSION).unwrap_or_else(|| { + panic!( + "{LEG}: redirected Gemfile.lock carries no sha256 CHECKSUMS entry \ + for {GEM_NAME} ({GEM_VERSION}):\n{lock}" + ) + }); + assert_ne!( + redirected_sha, upstream_sha, + "{LEG}: the redirect left {GEM_NAME}'s CHECKSUMS pin at the UPSTREAM \ + sha256 — bundler would verify and install the unpatched artifact, so \ + the hosted patch is inert (the same blindspot that let an inert npm \ + patch stay green).\nGemfile.lock:\n{lock}" + ); // Known-broken leg: reinstall from the redirected Gemfile. std::fs::remove_dir_all(&bundle_path).ok(); @@ -1658,6 +1760,73 @@ async fn gem_bundler_hosted_redirect_and_known_install_defect() { appears to be FIXED — delete the tolerance branch in this test and \ assert unconditionally." ); + // Exit 0 proves only that bundler fetched an artifact matching the + // CHECKSUMS pin. Close the loop on CONTENT: fetch the patch's file + // manifest from the proxy and assert every file it rewrites landed + // on disk byte-exact (afterHash is the git-blob sha256 the patch + // service publishes — the same digest the CLI's apply verifies). + let patch_files = published_patch_files(GEM_UUID).await.unwrap_or_else(|e| { + panic!( + "{LEG}: `bundle install` from the redirected Gemfile succeeded \ + but the patch file manifest could not be fetched to verify \ + the installed content: {e}" + ) + }); + let gem_dir = installed_gem_dir( + Path::new(&bundle_path), + &format!("{GEM_NAME}-{GEM_VERSION}"), + 4, + ) + .unwrap_or_else(|| { + panic!( + "{LEG}: `bundle install` succeeded but no \ + gems/{GEM_NAME}-{GEM_VERSION} directory exists under \ + {bundle_path}" + ) + }); + use socket_patch_core::hash::git_sha256::compute_git_sha256_from_bytes; + let mut verified = 0usize; + let mut rewritten = 0usize; + for (path, before, after) in &patch_files { + // No afterHash = the patch deletes the file; nothing to hash. + let Some(after) = after else { continue }; + let rel = path.strip_prefix("package/").unwrap_or(path.as_str()); + let installed = gem_dir.join(rel); + let bytes = std::fs::read(&installed).unwrap_or_else(|e| { + panic!( + "{LEG}: patch {GEM_UUID} rewrites `{path}` but the \ + installed gem has no readable {}: {e}", + installed.display() + ) + }); + assert_eq!( + compute_git_sha256_from_bytes(&bytes), + *after, + "{LEG}: installed {} does not hash to the patch's afterHash — \ + bundler fetched an artifact whose content is NOT the \ + published patch", + installed.display() + ); + verified += 1; + if before.as_deref() != Some(after.as_str()) { + rewritten += 1; + } + } + assert!( + verified >= 1, + "{LEG}: patch {GEM_UUID} names no files with an afterHash, so \ + nothing was content-verified — the install success is vacuous" + ); + assert!( + rewritten >= 1, + "{LEG}: every file in patch {GEM_UUID} has afterHash == \ + beforeHash — the published patch is inert and this install \ + proved nothing" + ); + println!( + "{LEG}: verified {verified} patched file(s) on disk against the \ + published afterHash ({rewritten} differ from upstream)" + ); return; } let detail = dump(&reinstall); diff --git a/crates/socket-patch-cli/tests/in_process_get_manifest_path.rs b/crates/socket-patch-cli/tests/in_process_get_manifest_path.rs index e8c888cf..00fac6f9 100644 --- a/crates/socket-patch-cli/tests/in_process_get_manifest_path.rs +++ b/crates/socket-patch-cli/tests/in_process_get_manifest_path.rs @@ -18,6 +18,14 @@ //! `get --cwd proj ` made the nested apply look for //! `proj/proj/.socket/manifest.json`, hit the no-manifest clean no-op, //! and report success (`applied: 1`, exit 0) without patching anything. +//! +//! 3. `run_nested_apply` threaded cwd/global/silent/download-mode/strict +//! and the four API flags into the nested `ApplyArgs` but left +//! `ecosystems` at `GlobalArgs::default()` (`None`), so a download +//! scoped with `--ecosystems ` (`scan --ecosystems gem --sync`, or +//! `get --ecosystems npm `) ran its apply step UNSCOPED over the +//! whole manifest — mutating other ecosystems' packages the user had +//! explicitly filtered out. use std::path::Path; @@ -251,3 +259,176 @@ async fn get_with_relative_cwd_actually_applies() { against --cwd twice and apply no-op'd on a missing manifest" ); } + +// --------------------------------------------------------------------------- +// 4. --ecosystems must scope the nested apply, not just the download +// --------------------------------------------------------------------------- + +const COMPOSER_PURL: &str = "pkg:composer/acme/lib@1.0.0"; +const COMPOSER_UUID: &str = "44444444-4444-4444-8444-444444444444"; +const COMPOSER_ORIGINAL: &[u8] = b" (std::path::PathBuf, std::path::PathBuf) { + // The npm target package, installed with pre-patch content. + std::fs::write( + root.join("package.json"), + r#"{"name":"r","version":"0.0.0"}"#, + ) + .unwrap(); + let npm_pkg = root.join("node_modules/manifest-path-test"); + std::fs::create_dir_all(&npm_pkg).unwrap(); + std::fs::write( + npm_pkg.join("package.json"), + r#"{"name":"manifest-path-test","version":"1.0.0"}"#, + ) + .unwrap(); + std::fs::write(npm_pkg.join("index.js"), ORIGINAL).unwrap(); + + // A composer package installed side by side (the composer crawler + // probes `/vendor/composer/installed.json` plus a composer.json + // marker — fully deterministic, no tooling or env vars involved). + std::fs::write(root.join("composer.json"), "{}").unwrap(); + let composer_dir = root.join("vendor/composer"); + std::fs::create_dir_all(&composer_dir).unwrap(); + std::fs::write( + composer_dir.join("installed.json"), + r#"{"packages": [{"name": "acme/lib", "version": "1.0.0"}]}"#, + ) + .unwrap(); + let composer_pkg = root.join("vendor/acme/lib"); + std::fs::create_dir_all(&composer_pkg).unwrap(); + std::fs::write(composer_pkg.join("index.php"), COMPOSER_ORIGINAL).unwrap(); + + // Manifest pre-seeded with the PENDING composer patch record and its + // cached after-blob (no network fetch needed to apply it). + let composer_before_hash = git_sha256(COMPOSER_ORIGINAL); + let composer_after_hash = git_sha256(COMPOSER_PATCHED); + let socket = root.join(".socket"); + std::fs::create_dir_all(socket.join("blobs")).unwrap(); + std::fs::write( + socket.join("blobs").join(&composer_after_hash), + COMPOSER_PATCHED, + ) + .unwrap(); + std::fs::write( + socket.join("manifest.json"), + format!( + r#"{{ "patches": {{ + "{COMPOSER_PURL}": {{ + "uuid": "{COMPOSER_UUID}", + "exportedAt": "2024-01-01T00:00:00Z", + "files": {{ + "package/index.php": {{ + "beforeHash": "{composer_before_hash}", + "afterHash": "{composer_after_hash}" + }} + }}, + "vulnerabilities": {{}}, + "description": "pending composer patch", "license": "MIT", "tier": "free" + }} + }}}}"# + ), + ) + .unwrap(); + + (npm_pkg, composer_pkg) +} + +/// A download restricted with `--ecosystems npm` must run its nested +/// apply under the SAME scope: the pending composer patch staged by +/// [`stage_npm_and_composer_project`] must stay unapplied while the npm +/// package is patched. (The control test below proves the fixture bites — +/// an UNSCOPED run does patch the composer package.) +#[tokio::test] +#[serial] +async fn get_ecosystems_flag_scopes_nested_apply() { + let server = MockServer::start().await; + mount_view_mock(&server).await; + mount_search_mock(&server).await; + + let tmp = tempfile::tempdir().unwrap(); + let (npm_pkg, composer_pkg) = stage_npm_and_composer_project(tmp.path()); + + let mut args = get_args(PURL, tmp.path(), server.uri()); + args.save_only = false; // exercise the nested apply step + args.common.ecosystems = Some(vec!["npm".to_string()]); + + let code = run(args).await; + assert_eq!(code, 0, "scoped get + apply must succeed"); + + // The in-scope npm package was patched... + assert_eq!( + std::fs::read(npm_pkg.join("index.js")).unwrap(), + PATCHED, + "the npm package selected by --ecosystems npm must be patched" + ); + // ...and the out-of-scope composer package was NOT: patching it means + // `ecosystems` was dropped when building the nested ApplyArgs and the + // apply step ran unscoped over the whole manifest. + assert_eq!( + std::fs::read(composer_pkg.join("index.php")).unwrap(), + COMPOSER_ORIGINAL, + "--ecosystems npm must scope the nested apply: the pending \ + composer patch must not be applied" + ); +} + +/// Negative control for the scoped test above: WITHOUT `--ecosystems`, +/// the very same fixture's nested apply covers the whole manifest and +/// patches the pending composer package too. This pins that the fixture +/// genuinely can reach the composer package — if this stopped holding, +/// the scoped test would pass vacuously. +#[tokio::test] +#[serial] +async fn get_without_ecosystems_applies_whole_manifest() { + let server = MockServer::start().await; + mount_view_mock(&server).await; + mount_search_mock(&server).await; + + let tmp = tempfile::tempdir().unwrap(); + let (npm_pkg, composer_pkg) = stage_npm_and_composer_project(tmp.path()); + + let mut args = get_args(PURL, tmp.path(), server.uri()); + args.save_only = false; // exercise the nested apply step + + let code = run(args).await; + assert_eq!(code, 0, "unscoped get + apply must succeed"); + + assert_eq!( + std::fs::read(npm_pkg.join("index.js")).unwrap(), + PATCHED, + "the npm package must be patched" + ); + assert_eq!( + std::fs::read(composer_pkg.join("index.php")).unwrap(), + COMPOSER_PATCHED, + "without --ecosystems the nested apply covers the whole manifest, \ + including the pending composer patch" + ); +} diff --git a/crates/socket-patch-cli/tests/in_process_get_update_count.rs b/crates/socket-patch-cli/tests/in_process_get_update_count.rs index 04bb8ccf..6756f29a 100644 --- a/crates/socket-patch-cli/tests/in_process_get_update_count.rs +++ b/crates/socket-patch-cli/tests/in_process_get_update_count.rs @@ -73,6 +73,7 @@ fn params(root: &Path, server: &MockServer) -> DownloadParams { proxy_url: None, }, strict: false, + ecosystems: None, persist_blobs: true, // Skip release-narrowing; npm has no variants anyway. all_releases: true, diff --git a/crates/socket-patch-cli/tests/in_process_scan.rs b/crates/socket-patch-cli/tests/in_process_scan.rs index 622d2cb4..96cb0111 100644 --- a/crates/socket-patch-cli/tests/in_process_scan.rs +++ b/crates/socket-patch-cli/tests/in_process_scan.rs @@ -1433,3 +1433,110 @@ async fn scan_discovers_maven_and_nuget_in_every_mode() { } std::env::remove_var("MAVEN_REPO_LOCAL"); } + +// --------------------------------------------------------------------------- +// Regression: `scan --vendor --dry-run --vex` must skip the embedded VEX. +// +// The vendor JSON dry-run arm handed base_code 0 straight to +// `embed_vex_into_json`, which generated the document for real: on a +// not-yet-vendored project (no manifest) generation failed and flipped the +// whole dry run to exit 1; with attestable state it wrote the VEX file to +// disk — a mutation during --dry-run. Both must be skipped instead +// (apply.rs's precedent: a dry run has no just-vendored state to attest). +// --------------------------------------------------------------------------- + +#[tokio::test] +#[serial] +async fn scan_vendor_dry_run_with_vex_does_not_fail_on_not_yet_vendored() { + let server = MockServer::start().await; + mock_batch_one(&server).await; + mock_by_package(&server).await; + + let tmp = tempfile::tempdir().unwrap(); + write_root_package_json(tmp.path()); + write_npm_package(tmp.path(), "in-proc-scan", "1.0.0"); + + let vex_path = tmp.path().join("vendor-dry.vex.json"); + let mut args = default_args(tmp.path()); + args.common.api_url = Some(server.uri()); + args.vendor = true; + args.common.dry_run = true; + args.vex.vex = Some(vex_path.clone()); + + let code = run_scrubbed(args).await; + // Nothing is vendored yet (no manifest, no ledger) — that is the normal + // state a vendor dry run previews from, not an error. The old behavior + // generated the VEX for real, hit `manifest_not_found`, and exited 1. + assert_eq!( + code, 0, + "a vendor dry run must not fail for not-yet-vendored state" + ); + assert!(!vex_path.exists(), "dry run must not write the VEX file"); + // Prove the vendor dry-run path was actually entered (not + // short-circuited before discovery): batch + patch details were queried. + let reqs = recorded(&server).await; + assert!( + batch_posts(&reqs).len() == 1 && by_package_gets(&reqs) >= 1, + "vendor dry run must query batch + patch details; batch={}, by_package={}", + batch_posts(&reqs).len(), + by_package_gets(&reqs), + ); +} + +#[tokio::test] +#[serial] +async fn scan_vendor_dry_run_with_vex_does_not_write_attestation_file() { + let server = MockServer::start().await; + mock_batch_one(&server).await; + mock_by_package(&server).await; + + let tmp = tempfile::tempdir().unwrap(); + write_root_package_json(tmp.path()); + write_npm_package(tmp.path(), "in-proc-scan", "1.0.0"); + + // A manifest whose sole record WOULD attest successfully: vulnerability + // metadata for the statement, `setup.manual: ["npm"]` to pass the + // property-7 ecosystem filter, and `--vex-no-verify` below to skip the + // on-disk hash check. Under the old behavior the dry run generated the + // document for real and wrote it to disk. + let socket = tmp.path().join(".socket"); + std::fs::create_dir_all(&socket).unwrap(); + std::fs::write( + socket.join("manifest.json"), + r#"{ "patches": { + "pkg:npm/in-proc-scan@1.0.0": { + "uuid": "11111111-1111-4111-8111-111111111111", + "exportedAt": "2024-01-01T00:00:00Z", + "files": {}, + "vulnerabilities": { "GHSA-aaaa-bbbb-cccc": { + "cves": ["CVE-2024-0001"], "summary": "s", + "severity": "HIGH", "description": "d" + }}, + "description": "x", "license": "MIT", "tier": "free" + } + }, "setup": { "manual": ["npm"] } }"#, + ) + .unwrap(); + let before = std::fs::read_to_string(socket.join("manifest.json")).unwrap(); + + let vex_path = tmp.path().join("vendor-dry.vex.json"); + let mut args = default_args(tmp.path()); + args.common.api_url = Some(server.uri()); + args.vendor = true; + args.common.dry_run = true; + args.vex.vex = Some(vex_path.clone()); + args.vex.vex_no_verify = true; + + let code = run_scrubbed(args).await; + assert_eq!(code, 0, "vendor dry run with attestable state must exit 0"); + assert!( + !vex_path.exists(), + "dry run must not write the VEX file even when generation would succeed" + ); + // The dry run's non-mutation contract extends to the manifest. + let after = std::fs::read_to_string(socket.join("manifest.json")).unwrap(); + assert_eq!( + after, before, + "vendor dry run must leave the manifest byte-for-byte unchanged" + ); +} diff --git a/crates/socket-patch-cli/tests/setup_matrix_gem.rs b/crates/socket-patch-cli/tests/setup_matrix_gem.rs index 06711f6a..c8d68164 100644 --- a/crates/socket-patch-cli/tests/setup_matrix_gem.rs +++ b/crates/socket-patch-cli/tests/setup_matrix_gem.rs @@ -2,8 +2,20 @@ //! — it appends a managed `plugin "socket-patch"` block to the Gemfile and //! generates a committed in-tree Bundler plugin under `.socket/bundler-plugin/` //! whose `plugins.rb` re-runs `socket-patch apply --ecosystems gem` on every -//! `bundle install` (load-time digest gate + `after-install-all` hook). So the -//! with-setup cases are no longer a baseline gap. +//! `bundle install` (load-time digest gate + `after-install-all` hook). +//! +//! The with-setup Docker cases (`baseline_with_setup`, `alt_content_patchset`) +//! are still a [BASELINE GAP], for two structural reasons (verified +//! 2026-08-13): (a) installing the plugin evaluates `plugins.rb` BEFORE any +//! project gems land, and the load-time `SocketPatch.apply!` treats apply's +//! exit 1 ("No packages found that match available patches") as a genuine +//! failure and raises `Bundler::BundlerError`, so the FIRST `bundle install` +//! after `setup` on a never-installed project always dies — this is the gem +//! twin of the documented apply exit-semantics issue; (b) the matrix fixture's +//! synthetic beforeHashes only pass hash-gated ecosystems via npm's +//! mismatch-warn-and-apply path, which gem apply does not have. The +//! `after-install-all` re-apply flow itself works (asserted in-container with +//! realistic hashes). //! //! IMPORTANT — why this file carries a real assertion of its own: //! `smc::run_pm("gem", "bundler")` routes gem through the shared Docker diff --git a/crates/socket-patch-core/src/crawlers/ruby_crawler.rs b/crates/socket-patch-core/src/crawlers/ruby_crawler.rs index 8a6fed15..3d5e4892 100644 --- a/crates/socket-patch-core/src/crawlers/ruby_crawler.rs +++ b/crates/socket-patch-core/src/crawlers/ruby_crawler.rs @@ -22,7 +22,7 @@ impl RubyCrawler { /// Get gem installation paths based on options. /// - /// In local mode, checks `vendor/bundle/ruby/*/gems/` first (Bundler + /// In local mode, checks `vendor/bundle//*/gems/` first (Bundler /// deployment layout), then — only if the cwd holds a Bundler manifest /// or lockfile — falls back to the gem homes `gem env` reports. /// @@ -178,18 +178,31 @@ impl RubyCrawler { paths } - /// Find `vendor/bundle/ruby/*/gems/` directories. + /// Find `vendor/bundle//*/gems/` directories. + /// + /// Bundler's deployment scope is `#{Gem.ruby_engine}/#{ruby_version}` + /// (`Bundler.ruby_scope`) — `ruby` under MRI, but `jruby`/`truffleruby` + /// under the alternative engines. Hardcoding `ruby` made JRuby and + /// TruffleRuby deployments discover zero gems, so enumerate every + /// engine dir that holds `/gems/` children; non-engine + /// clutter is filtered by that shape. async fn get_vendor_bundle_paths(cwd: &Path) -> Vec { - let vendor_ruby = cwd.join("vendor").join("bundle").join("ruby"); + let vendor_bundle = cwd.join("vendor").join("bundle"); let mut paths = Vec::new(); - for entry in list_dir_entries(&vendor_ruby).await { - if !entry_is_dir(&entry).await { + for engine_entry in list_dir_entries(&vendor_bundle).await { + if !entry_is_dir(&engine_entry).await { continue; } - let gems_dir = vendor_ruby.join(entry.file_name()).join("gems"); - if is_dir(&gems_dir).await { - paths.push(gems_dir); + let engine_dir = vendor_bundle.join(engine_entry.file_name()); + for entry in list_dir_entries(&engine_dir).await { + if !entry_is_dir(&entry).await { + continue; + } + let gems_dir = engine_dir.join(entry.file_name()).join("gems"); + if is_dir(&gems_dir).await { + paths.push(gems_dir); + } } } paths @@ -338,22 +351,41 @@ impl RubyCrawler { /// /// Gem directories follow `-` (ruby-platform gems) or /// `--` (platform gems, e.g. - /// `nokogiri-1.16.5-x86_64-linux`). The name/version boundary is the - /// **first** `-` followed by a digit. A RubyGems version is dash-free - /// (prerelease dashes render as `.pre.`), so the version is the run up - /// to the next `-`; anything after that is the platform suffix, which - /// we drop — the installed platform is resolved later by hashing the - /// gem's files (the same model as PyPI's `artifact_id`). The qualified - /// `?platform=` PURL is only ever carried in the manifest/API. + /// `nokogiri-1.16.5-x86_64-linux`). A RubyGems version is dash-free + /// (prerelease dashes render as `.pre.`), so every `-` followed by a + /// digit is a candidate name/version boundary and the version is the + /// dash-free token after it; anything past that is the platform + /// suffix, which we drop — the installed platform is resolved later by + /// hashing the gem's files (the same model as PyPI's `artifact_id`). + /// The qualified `?platform=` PURL is only ever carried in the + /// manifest/API. + /// + /// Names may themselves contain `-` runs (`http-2`, + /// `http-2-next`), so the first candidate boundary is not always + /// right: `http-2-1.0.1` must parse as `("http-2", "1.0.1")`, not the + /// ghost `("http", "2")`. Real versions are almost always dotted while + /// digit runs embedded in names (`-2-`) and trailing platform OS + /// revisions (`-darwin-21`) are not, so prefer the LAST boundary whose + /// version token contains a `.`; fall back to the first dash-digit + /// boundary only when no dotted candidate exists (a bare + /// single-segment version like `g-1` is legal but vanishingly rare). fn parse_dir_name_version(dir_name: &str) -> Option<(String, String)> { - let idx = dir_name + let candidates: Vec = dir_name .match_indices('-') - .find(|(i, _)| dir_name[i + 1..].starts_with(|c: char| c.is_ascii_digit())) - .map(|(i, _)| i)?; - let name = &dir_name[..idx]; - let rest = &dir_name[idx + 1..]; + .filter(|(i, _)| dir_name[i + 1..].starts_with(|c: char| c.is_ascii_digit())) + .map(|(i, _)| i) + .collect(); // Version is the leading dash-free token; drop any `-`. - let version = rest.split('-').next().unwrap_or(rest); + let version_token = |i: usize| { + let rest = &dir_name[i + 1..]; + rest.split('-').next().unwrap_or(rest) + }; + let idx = *candidates + .iter() + .rfind(|&&i| version_token(i).contains('.')) + .or_else(|| candidates.first())?; + let name = &dir_name[..idx]; + let version = version_token(idx); if name.is_empty() || version.is_empty() { return None; } @@ -556,6 +588,34 @@ mod tests { assert_eq!(paths[0], vendor_gems); } + /// Bundler's deployment scope is `/` — `jruby` and + /// `truffleruby` deployments live beside `ruby` under `vendor/bundle` + /// and must be discovered too (hardcoding `ruby` found zero gems + /// there). Non-engine clutter — files, and dirs whose children hold no + /// `gems/` — must not produce paths. + #[tokio::test] + async fn test_get_vendor_bundle_paths_alternative_engines() { + let dir = tempfile::tempdir().unwrap(); + let bundle = dir.path().join("vendor").join("bundle"); + let ruby_gems = bundle.join("ruby").join("3.2.0").join("gems"); + let jruby_gems = bundle.join("jruby").join("3.1.4.0").join("gems"); + let truffle_gems = bundle.join("truffleruby").join("3.2.2").join("gems"); + for gems in [&ruby_gems, &jruby_gems, &truffle_gems] { + tokio::fs::create_dir_all(gems).await.unwrap(); + } + tokio::fs::write(bundle.join("install.log"), b"x") + .await + .unwrap(); + tokio::fs::create_dir_all(bundle.join("cache").join("3.2.0")) + .await + .unwrap(); + + let paths = RubyCrawler::get_vendor_bundle_paths(dir.path()).await; + assert_eq!(paths.len(), 3, "one gems dir per engine; got {paths:?}"); + let found: HashSet = paths.into_iter().collect(); + assert_eq!(found, HashSet::from([ruby_gems, jruby_gems, truffle_gems])); + } + #[tokio::test] async fn test_deduplication() { let dir = tempfile::tempdir().unwrap(); @@ -980,9 +1040,65 @@ mod tests { assert!(!is_safe_gem_coordinate("rails", "C:1.0.0")); } + /// Names with embedded `-` runs (`http-2`, `http-2-next`) must + /// keep the digits in the name: the boundary is the LAST dash-digit + /// whose version token is dotted, not the first dash-digit. Without + /// that preference `http-2-1.0.1` parsed as `("http", "2")` — a ghost + /// PURL — and the real gem was never discovered. + #[test] + fn parse_dir_name_version_prefers_last_dotted_boundary() { + assert_eq!( + RubyCrawler::parse_dir_name_version("http-2-1.0.1"), + Some(("http-2".to_string(), "1.0.1".to_string())) + ); + assert_eq!( + RubyCrawler::parse_dir_name_version("http-2-next-1.0.3"), + Some(("http-2-next".to_string(), "1.0.3".to_string())) + ); + // A platform suffix after the real version still drops. + assert_eq!( + RubyCrawler::parse_dir_name_version("http-2-1.0.1-java"), + Some(("http-2".to_string(), "1.0.1".to_string())) + ); + } + + /// The dotted-boundary preference must not regress the plain shapes: + /// dotted versions, prereleases, platform dirs, and — via the + /// first-boundary fallback — bare single-segment versions (legal per + /// RubyGems, just vanishingly rare). + #[test] + fn parse_dir_name_version_boundary_shapes() { + assert_eq!( + RubyCrawler::parse_dir_name_version("rack-3.1.0"), + Some(("rack".to_string(), "3.1.0".to_string())) + ); + assert_eq!( + RubyCrawler::parse_dir_name_version("aws-sdk-s3-1.140.0"), + Some(("aws-sdk-s3".to_string(), "1.140.0".to_string())) + ); + assert_eq!( + RubyCrawler::parse_dir_name_version("gem2-1.0"), + Some(("gem2".to_string(), "1.0".to_string())) + ); + // No dotted candidate → first dash-digit boundary fallback. + assert_eq!( + RubyCrawler::parse_dir_name_version("g-1"), + Some(("g".to_string(), "1".to_string())) + ); + // Prerelease dashes render as dots, so the token stays dotted. + assert_eq!( + RubyCrawler::parse_dir_name_version("rails-7.1.0.beta1"), + Some(("rails".to_string(), "7.1.0.beta1".to_string())) + ); + assert_eq!( + RubyCrawler::parse_dir_name_version("nokogiri-1.16.0-arm64-darwin"), + Some(("nokogiri".to_string(), "1.16.0".to_string())) + ); + } + /// Gem names with embedded underscores/digits and multi-dash names - /// must keep their full name; the version starts at the first - /// dash-then-digit boundary. + /// must keep their full name; the version starts at the dash-then-digit + /// boundary that opens the dotted version token. #[test] fn parse_dir_name_version_name_shapes() { assert_eq!( diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 0ff40eb5..87740fcf 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -1673,6 +1673,31 @@ pub(crate) fn gem_line_trailing_options(tail: &str) -> String { } } +/// The source-selecting option a `gem` line's argument tail carries, if any +/// (only the code before any `#` comment counts). Bundler allows ONE source +/// per gem, so an option like `git:` preserved into the Socket source block +/// OVERRIDES the block and the redirect becomes a silent no-op. Mirrors the +/// token list `vendor::gem::rest_blocks_edit` refuses for the same reason. +fn gem_tail_source_option(tail: &str) -> Option<&'static str> { + let code = tail.split('#').next().unwrap_or(""); + [ + "path:", + ":path", + "git:", + ":git", + "github:", + ":github", + "source:", + ":source", + "gist:", + ":gist", + "bitbucket:", + ":bitbucket", + ] + .into_iter() + .find(|tok| code.contains(tok)) +} + fn rewrite_gem( files: &BTreeMap, overrides: &[DepOverride], @@ -1713,35 +1738,159 @@ fn rewrite_gem( continue; }; + // Platform-suffixed CHECKSUMS siblings (`name (version-arm64-darwin) + // sha256=`) mean bundler resolves platform-specific gems the patch + // registry does not serve — redirecting would pin the bare-platform + // sha while installs keep fetching the upstream platform gem + // (guaranteed mismatch or a silently unpatched install). Fail closed: + // skip the dep entirely. + if let Some(lk) = lock.as_deref() { + let platform_re = Regex::new( + &(String::from(r"(?m)^ ") + + ®ex::escape(&dep.name) + + r" \(" + + ®ex::escape(&dep.version) + + r"-[^)]+\) sha256="), + ) + .unwrap(); + if platform_re.is_match(lk) { + result.warnings.push(RewriteWarning { + code: "redirect_gem_platform_unsupported".into(), + detail: format!( + "Gemfile.lock CHECKSUMS carries platform-specific entries for {} {} — \ + the patch registry serves only the ruby platform gem; redirect skipped", + dep.name, dep.version + ), + }); + continue; + } + } + + // Whether THIS dep's Gemfile source redirect is in place (just + // written or already present) — the lock pin below is gated on it. + let mut source_placed = false; if let Some(gf) = gemfile.as_mut() { - if !gf.contains(&format!("source \"{}\"", ov.index_url)) { + // Grant-agnostic idempotency guard: the grant-token (and patch + // uuid) segments of the index URL rotate per request, so an + // exact-URL check misses the block a previous run wrote and this + // run would wrap the gem line inside it — nesting source blocks. + // Wildcard the rotating segments instead (mirrors the CHECKSUMS + // at-target guard below). + let mut url_pat = regex::escape(&ov.index_url); + for rotating in [&dep.token, &dep.patch_uuid] { + if !rotating.is_empty() { + url_pat = + url_pat.replace(®ex::escape(&format!("/{rotating}/")), "/[^/\"]+/"); + } + } + let block_re = Regex::new( + &(String::from(r#"(?m)^source "("#) + + &url_pat + + r#")" do\n gem ["']"# + + ®ex::escape(&dep.name) + + r#"["']"#), + ) + .unwrap(); + if let Some(m) = block_re.captures(gf) { + let url = m.get(1).unwrap(); + if url.as_str() == ov.index_url { + source_placed = true; + } else { + // Rotated grant: refresh the URL in place — never nest. + let (range, old_url) = (url.range(), url.as_str().to_string()); + gf.replace_range(range, &ov.index_url); + gemfile_changed = true; + result.edits.push(FileEdit { + path: "Gemfile".into(), + kind: "redirect_gemfile_source_url".into(), + action: "rewritten".into(), + key: Some(dep.name.clone()), + original: Some(Value::String(old_url)), + new: Some(Value::String(ov.index_url.clone())), + }); + source_placed = true; + } + } else { + // Tolerate the legal spellings of a declaration: tab / extra + // spaces after `gem`, and the parenthesized call form. let gem_line_re = Regex::new( - &(String::from(r#"(?m)^\s*gem ["']"#) + &(String::from(r#"(?m)^\s*gem(?:[ \t]*(\()[ \t]*|[ \t]+)["']"#) + ®ex::escape(&dep.name) + r#"["']([^\n]*)$"#), ) .unwrap(); - let block = format!( - "source \"{}\" do\n gem \"{}\", \"{}\"\nend", - ov.index_url, dep.name, dep.version - ); + // Looser "declared at all?" probe: gates the append branch — + // appending next to a declaration the recognizer above cannot + // parse would leave the gem declared twice (bundler + // hard-fails on the duplicate). + let declared_re = Regex::new( + &(String::from(r#"(?m)^[ \t]*gem\b[^\n]*["']"#) + + ®ex::escape(&dep.name) + + r#"["']"#), + ) + .unwrap(); if let Some(m) = gem_line_re.captures(gf) { + let range = m.get(0).unwrap().range(); let original = m.get(0).unwrap().as_str().to_string(); + let paren = m.get(1).is_some(); + let raw_tail = m.get(2).unwrap().as_str().to_string(); + // A parenthesized call keeps its closing `)` in the tail: + // strip it (dropping any comment with it), or fail closed + // when it is absent (the call continues past this line). + let tail = if paren { + let code = raw_tail.split('#').next().unwrap_or("").trim_end(); + match code.strip_suffix(')') { + Some(t) => t.to_string(), + None => { + result.warnings.push(RewriteWarning { + code: "redirect_gem_unrecognized_declaration".into(), + detail: format!( + "the `gem \"{}\"` declaration is in a form the \ + rewriter cannot safely edit; redirect skipped", + dep.name + ), + }); + continue; + } + } + } else { + raw_tail + }; + // A source-selecting option would move into the block and + // OVERRIDE it in bundler's DSL, leaving the redirect a + // silent no-op that still gets attested. Fail closed. + if let Some(tok) = gem_tail_source_option(&tail) { + result.warnings.push(RewriteWarning { + code: "redirect_gem_source_option".into(), + detail: format!( + "the `gem \"{}\"` declaration carries `{tok}`, which would \ + override the Socket source block; redirect skipped", + dep.name + ), + }); + continue; + } // Trailing options (`require: false`, `group: …`) must // survive the move into the source block — dropping // `require: false` auto-requires the gem at boot. - let opts = gem_line_trailing_options(m.get(1).unwrap().as_str()); + let opts = gem_line_trailing_options(&tail); let block = if opts.is_empty() { - block + format!( + "source \"{}\" do\n gem \"{}\", \"{}\"\nend", + ov.index_url, dep.name, dep.version + ) } else { format!( "source \"{}\" do\n gem \"{}\", \"{}\", {opts}\nend", ov.index_url, dep.name, dep.version ) }; - // Plain replacen: the block may carry user text (`opts`), - // which a regex replacement would `$`-expand. - *gf = gf.replacen(&original, &block, 1); + // Splice by the match's byte range: a substring replace of + // the line's TEXT would hit an identical commented-out + // duplicate earlier in the file and corrupt it (and the + // block may carry user text a regex replacement would + // `$`-expand). + gf.replace_range(range, &block); gemfile_changed = true; result.edits.push(FileEdit { path: "Gemfile".into(), @@ -1751,7 +1900,23 @@ fn rewrite_gem( original: Some(Value::String(original)), new: Some(Value::String(block)), }); + source_placed = true; + } else if declared_re.is_match(gf) { + result.warnings.push(RewriteWarning { + code: "redirect_gem_unrecognized_declaration".into(), + detail: format!( + "the `gem \"{}\"` declaration is in a form the rewriter \ + cannot safely edit; redirect skipped", + dep.name + ), + }); + continue; } else { + // Genuinely undeclared (a transitive dep): append a block. + let block = format!( + "source \"{}\" do\n gem \"{}\", \"{}\"\nend", + ov.index_url, dep.name, dep.version + ); let sep = if gf.ends_with('\n') { "" } else { "\n" }; *gf = format!("{gf}{sep}{block}\n"); gemfile_changed = true; @@ -1763,17 +1928,32 @@ fn rewrite_gem( original: None, new: Some(Value::String(block)), }); + source_placed = true; } } } if let Some(lk) = lock.as_mut() { + // The pin only makes sense once the source redirect is in place + // (just written or already present): pinning the patched sha + // while the gem still resolves upstream guarantees a checksum + // failure on the next install. + if !source_placed { + result.warnings.push(RewriteWarning { + code: "redirect_gem_lock_without_source".into(), + detail: format!( + "no Gemfile source redirect is in place for {} — CHECKSUMS pin skipped", + dep.name + ), + }); + continue; + } let sum_line_re = Regex::new( &(String::from(r"(?m)^( ") + ®ex::escape(&dep.name) + r" \(" + ®ex::escape(&dep.version) - + r"\)) sha256=[0-9a-f]+$"), + + r"\)) sha256=([0-9a-f]+)$"), ) .unwrap(); let new_val = format!("{} ({}) sha256={sha256}", dep.name, dep.version); @@ -1781,7 +1961,15 @@ fn rewrite_gem( // target value; recording an edit would grow the ledger forever. if lk.contains(&format!("\n {new_val}\n")) || lk.ends_with(&format!("\n {new_val}")) { // no-op - } else if sum_line_re.is_match(lk) { + } else if let Some(m) = sum_line_re.captures(lk) { + // The pre-edit line goes into the ledger as `original` so a + // future `--revert` can restore the upstream sha. + let old_val = format!( + "{} ({}) sha256={}", + dep.name, + dep.version, + m.get(2).unwrap().as_str() + ); *lk = sum_line_re .replace(lk, format!("${{1}} sha256={sha256}").as_str()) .to_string(); @@ -1791,7 +1979,7 @@ fn rewrite_gem( kind: "redirect_gemfile_lock_checksum".into(), action: "rewritten".into(), key: Some(dep.name.clone()), - original: None, + original: Some(Value::String(old_val)), new: Some(Value::String(new_val)), }); } else if checksums_re.is_match(lk) { @@ -1826,6 +2014,21 @@ fn rewrite_gem( } } + // The rewritten pair breaks bundler's frozen/deployment mode: the lock's + // GEM section still records the upstream source, so `bundle install` with + // `frozen`/`--deployment` set rejects the Gemfile's new source block. + // Mirror of the CLI's pnpm trust-lockfile warning. + if gemfile_changed || lock_changed { + result.warnings.push(RewriteWarning { + code: "redirect_gem_frozen_install".into(), + detail: "Gemfile was repointed at the Socket patch registry but Gemfile.lock's \ + GEM section still records the upstream source; bundler rejects the pair \ + under frozen/deployment mode — run `bundle install` (unfrozen) once to \ + record the new source in Gemfile.lock" + .into(), + }); + } + if gemfile_changed { if let Some(gf) = gemfile { result.files.insert("Gemfile".into(), gf); @@ -3586,6 +3789,338 @@ mod tests { ); } + /// A minimal Gemfile.lock with the given CHECKSUMS lines (rails 7.0.0). + fn gem_lock(checksums: &str) -> String { + format!( + "GEM\n remote: https://rubygems.org/\n specs:\n rails (7.0.0)\n\n\ + PLATFORMS\n ruby\n\nDEPENDENCIES\n rails (= 7.0.0)\n\n\ + CHECKSUMS\n{checksums}\n\nBUNDLED WITH\n 2.6.2\n" + ) + } + + /// The edit must splice by the regex match's byte range: a substring + /// replace of the matched line's TEXT finds an identical commented-out + /// duplicate earlier in the file first and corrupts the comment while the + /// live line keeps resolving upstream. + #[test] + fn gemfile_rewrite_ignores_commented_duplicate() { + let mut files = BTreeMap::new(); + files.insert( + "Gemfile".to_string(), + "source \"https://rubygems.org\"\n\ + # gem \"rails\", \"7.0.0\" pinned during the 6.x upgrade\n\ + gem \"rails\", \"7.0.0\"\n" + .to_string(), + ); + let r = rewrite_registry_redirect(&files, &[gem_override("rails", "7.0.0")]); + let out = r.files.get("Gemfile").expect("Gemfile rewritten"); + assert!( + out.contains("\n# gem \"rails\", \"7.0.0\" pinned during the 6.x upgrade\n"), + "commented-out duplicate left untouched: {out}" + ); + assert!( + out.contains( + "\nsource \"https://patch.test/gem/tok/uuid/\" do\n gem \"rails\", \"7.0.0\"\nend\n" + ), + "live line replaced by the source block: {out}" + ); + } + + /// The grant token in the index URL rotates per request, so a re-run must + /// recognize the source block a previous run wrote (token-wildcard match, + /// not exact URL) and refresh its URL in place — never wrap the block's + /// gem line inside a new nested block. + #[test] + fn gemfile_rerun_with_rotated_grant_updates_url_never_nests() { + fn ov(token: &str) -> DepOverride { + let mut o = gem_override("rails", "7.0.0"); + o.token = token.into(); + if let Some(r) = o.registry_override.as_mut() { + r.index_url = format!("https://patch.test/gem/{token}/uuid/"); + } + o + } + let mut files = BTreeMap::new(); + files.insert( + "Gemfile".to_string(), + "source \"https://rubygems.org\"\n\ngem \"rails\", \"7.0.0\"\n".to_string(), + ); + let first = rewrite_registry_redirect(&files, &[ov("tok-one")]); + let redirected = first.files.get("Gemfile").expect("first run rewrites"); + files.insert("Gemfile".to_string(), redirected.clone()); + + let second = rewrite_registry_redirect(&files, &[ov("tok-two")]); + let out = second + .files + .get("Gemfile") + .expect("rotated grant refreshes the URL"); + assert_eq!( + out.matches("source \"https://patch.test/gem/").count(), + 1, + "exactly one Socket source block, never nested: {out}" + ); + assert!( + out.contains( + "source \"https://patch.test/gem/tok-two/uuid/\" do\n gem \"rails\", \"7.0.0\"\nend" + ), + "URL refreshed in place: {out}" + ); + assert!(!out.contains("tok-one"), "old grant token gone: {out}"); + assert!( + second + .edits + .iter() + .any(|e| e.kind == "redirect_gemfile_source_url" + && e.original + == Some(Value::String("https://patch.test/gem/tok-one/uuid/".into()))), + "URL refresh recorded with the old URL as original: {:?}", + second.edits + ); + + // Same grant again: a true no-op. + files.insert("Gemfile".to_string(), out.clone()); + let third = rewrite_registry_redirect(&files, &[ov("tok-two")]); + assert!( + third.files.is_empty() && third.edits.is_empty(), + "same-grant re-run must be a no-op: files={:?} edits={:?}", + third.files.keys(), + third.edits + ); + } + + /// A gem-level source option (`git:` / `path:` / `github:` / `source:`) + /// preserved into the Socket source block OVERRIDES it in bundler's DSL, + /// leaving the redirect a silent no-op that still gets attested. Fail + /// closed: warn and leave both files untouched. + #[test] + fn gemfile_gem_with_source_option_fails_closed() { + let mut files = BTreeMap::new(); + files.insert( + "Gemfile".to_string(), + "source \"https://rubygems.org\"\n\n\ + gem \"rails\", \"7.0.0\", git: \"https://github.com/rails/rails\"\n" + .to_string(), + ); + files.insert( + "Gemfile.lock".to_string(), + gem_lock(&format!(" rails (7.0.0) sha256={}", "2".repeat(64))), + ); + let r = rewrite_registry_redirect(&files, &[gem_override("rails", "7.0.0")]); + assert!( + r.files.is_empty() && r.edits.is_empty(), + "source-selecting option must skip the redirect: files={:?} edits={:?}", + r.files.keys(), + r.edits + ); + assert!( + r.warnings + .iter() + .any(|w| w.code == "redirect_gem_source_option"), + "skip must warn: {:?}", + r.warnings + ); + } + + /// Platform-specific CHECKSUMS siblings (`rails (7.0.0-arm64-darwin)`) + /// mean bundler resolves a platform gem the patch registry does not + /// serve — the bare-platform pin would leave the platform line at the + /// upstream sha (or duplicate the bare line). Fail closed: skip the dep. + #[test] + fn gem_platform_checksums_fail_closed() { + let mut files = BTreeMap::new(); + files.insert( + "Gemfile".to_string(), + "source \"https://rubygems.org\"\n\ngem \"rails\", \"7.0.0\"\n".to_string(), + ); + files.insert( + "Gemfile.lock".to_string(), + gem_lock(&format!( + " rails (7.0.0) sha256={}\n rails (7.0.0-arm64-darwin) sha256={}", + "2".repeat(64), + "3".repeat(64) + )), + ); + let r = rewrite_registry_redirect(&files, &[gem_override("rails", "7.0.0")]); + assert!( + r.files.is_empty() && r.edits.is_empty(), + "platform gems must skip the whole dep: files={:?} edits={:?}", + r.files.keys(), + r.edits + ); + assert!( + r.warnings + .iter() + .any(|w| w.code == "redirect_gem_platform_unsupported"), + "skip must warn: {:?}", + r.warnings + ); + } + + /// Legal-but-non-canonical declarations (parenthesized call, tab / double + /// space after `gem`) must be recognized and rewritten in place — falling + /// through to the append branch declares the gem twice, which bundler + /// rejects. + #[test] + fn gemfile_paren_and_whitespace_declarations_are_rewritten_not_duplicated() { + let mut files = BTreeMap::new(); + files.insert( + "Gemfile".to_string(), + "source \"https://rubygems.org\"\n\n\ + gem(\"rails\", \"7.0.0\", require: false)\n\ + gem\t\"puma\", \"6.0.0\"\n" + .to_string(), + ); + let r = rewrite_registry_redirect( + &files, + &[ + gem_override("rails", "7.0.0"), + gem_override("puma", "6.0.0"), + ], + ); + let out = r.files.get("Gemfile").expect("Gemfile rewritten"); + assert!( + out.contains( + "source \"https://patch.test/gem/tok/uuid/\" do\n \ + gem \"rails\", \"7.0.0\", require: false\nend" + ), + "paren declaration rewritten with options kept, `)` stripped: {out}" + ); + assert!( + !out.contains("gem(\"rails\"") && !out.contains("gem\t\"puma\""), + "original declarations replaced, not duplicated: {out}" + ); + assert!( + out.contains( + "source \"https://patch.test/gem/tok/uuid/\" do\n gem \"puma\", \"6.0.0\"\nend" + ), + "tab-separated declaration rewritten: {out}" + ); + } + + /// A declaration the recognizer cannot parse (`gem\"rails\"` — legal ruby, + /// no separator) must NOT fall through to the append branch: warn and skip + /// instead of declaring the gem twice. + #[test] + fn gemfile_unrecognizable_declaration_fails_closed_no_append() { + let mut files = BTreeMap::new(); + files.insert( + "Gemfile".to_string(), + "source \"https://rubygems.org\"\n\ngem\"rails\", \"7.0.0\"\n".to_string(), + ); + let r = rewrite_registry_redirect(&files, &[gem_override("rails", "7.0.0")]); + assert!( + r.files.is_empty() && r.edits.is_empty(), + "unrecognizable declaration must not append a duplicate: files={:?} edits={:?}", + r.files.keys(), + r.edits + ); + assert!( + r.warnings + .iter() + .any(|w| w.code == "redirect_gem_unrecognized_declaration"), + "skip must warn: {:?}", + r.warnings + ); + } + + /// The CHECKSUMS pin is gated on the Gemfile source redirect being in + /// place: with no Gemfile in the candidate map, pinning the patched sha + /// while the gem still resolves upstream guarantees a checksum failure. + #[test] + fn gem_lock_pin_gated_on_source_redirect() { + let mut files = BTreeMap::new(); + files.insert( + "Gemfile.lock".to_string(), + gem_lock(&format!(" rails (7.0.0) sha256={}", "2".repeat(64))), + ); + let r = rewrite_registry_redirect(&files, &[gem_override("rails", "7.0.0")]); + assert!( + r.files.is_empty() && r.edits.is_empty(), + "lock pin without a source redirect must be skipped: files={:?} edits={:?}", + r.files.keys(), + r.edits + ); + assert!( + r.warnings + .iter() + .any(|w| w.code == "redirect_gem_lock_without_source"), + "skip must warn: {:?}", + r.warnings + ); + } + + /// A landed gem redirect breaks bundler frozen/deployment installs (the + /// lock's GEM section still records the upstream source), so the rewrite + /// must say so — and only when it actually changed something. + #[test] + fn gem_redirect_warns_about_frozen_installs() { + let mut files = BTreeMap::new(); + files.insert( + "Gemfile".to_string(), + "source \"https://rubygems.org\"\n\ngem \"rails\", \"7.0.0\"\n".to_string(), + ); + files.insert( + "Gemfile.lock".to_string(), + gem_lock(&format!(" rails (7.0.0) sha256={}", "2".repeat(64))), + ); + let ovr = gem_override("rails", "7.0.0"); + let first = rewrite_registry_redirect(&files, std::slice::from_ref(&ovr)); + assert!( + first + .warnings + .iter() + .any(|w| w.code == "redirect_gem_frozen_install"), + "landed redirect must warn about frozen installs: {:?}", + first.warnings + ); + + // No-op re-run: nothing landed, so no frozen-install warning. + for (name, content) in first.files { + files.insert(name, content); + } + let second = rewrite_registry_redirect(&files, std::slice::from_ref(&ovr)); + assert!( + second.files.is_empty() + && !second + .warnings + .iter() + .any(|w| w.code == "redirect_gem_frozen_install"), + "a no-op re-run must not warn: files={:?} warnings={:?}", + second.files.keys(), + second.warnings + ); + } + + /// The rewritten CHECKSUMS edit must carry the pre-edit line as + /// `original` — with `None` the ledger cannot restore the upstream sha on + /// a future revert. + #[test] + fn gem_lock_rewrite_records_original_checksum_line() { + let mut files = BTreeMap::new(); + files.insert( + "Gemfile".to_string(), + "source \"https://rubygems.org\"\n\ngem \"rails\", \"7.0.0\"\n".to_string(), + ); + files.insert( + "Gemfile.lock".to_string(), + gem_lock(&format!(" rails (7.0.0) sha256={}", "2".repeat(64))), + ); + let r = rewrite_registry_redirect(&files, &[gem_override("rails", "7.0.0")]); + let edit = r + .edits + .iter() + .find(|e| e.kind == "redirect_gemfile_lock_checksum" && e.action == "rewritten") + .expect("lock checksum edit recorded"); + assert_eq!( + edit.original, + Some(Value::String(format!( + "rails (7.0.0) sha256={}", + "2".repeat(64) + ))), + "pre-edit CHECKSUMS line captured for revert" + ); + } + /// An unparseable package-lock.json must surface a warning, not silently /// skip the npm redirect entirely (missing-lockfile already warns; a /// corrupt lockfile is strictly worse and was silent). diff --git a/crates/socket-patch-core/src/vendor/gem.rs b/crates/socket-patch-core/src/vendor/gem.rs index 3f3ee42b..2e5c47a2 100644 --- a/crates/socket-patch-core/src/vendor/gem.rs +++ b/crates/socket-patch-core/src/vendor/gem.rs @@ -38,7 +38,11 @@ //! trailing options like `require: false` preserved) or, for a transitive //! dependency, a managed block appended at EOF. Anything //! the conservative line grammar cannot prove safe to rewrite is REFUSED — -//! never guessed at. +//! never guessed at. The one exception is OUR OWN previous wiring: a patch +//! update moves the manifest to a new uuid (same purl), and a `path:` that +//! parses as the socket vendor dir for exactly this gem is repointed in +//! place (the older patch uuid is re-vendored automatically, like the +//! npm/cargo/golang backends — no revert-first). //! //! The stub gemspec from `/specifications/` is copied into the //! vendored dir as `.gemspec` (a path source needs one; the spike showed @@ -63,7 +67,7 @@ use super::common::{ already_patched_result, copy_matches_after_hashes, done, refused, service_offline_conflict, synthesized_result, }; -use super::path::vendor_uuid_dir_rel; +use super::path::{parse_vendor_path, vendor_uuid_dir_rel}; use super::registry_fetch::extract_gem_data; use super::service_fetch::{ fetch_verified_archive, fetch_verified_secondary, SecondaryArtifactResult, ServiceArtifact, @@ -459,15 +463,50 @@ pub async fn vendor_gem( original: None, new: Some(Value::String(block.clone())), }, + // Re-vendor over our own wiring (see `GemfilePlan::RewireOurs`): + // `original: None`, carried forward by the caller. The managed-fence + // form stays `Added` with the whole updated block so revert deletes + // the fence too. + GemfilePlan::RewireOurs { + new_line, + managed_block, + .. + } => match managed_block { + Some(block) => WiringRecord { + file: GEMFILE.to_string(), + kind: GEMFILE_WIRING_KIND.to_string(), + action: WiringAction::Added, + key: Some(name.to_string()), + original: None, + new: Some(Value::String(block.clone())), + }, + None => WiringRecord { + file: GEMFILE.to_string(), + kind: GEMFILE_WIRING_KIND.to_string(), + action: WiringAction::Rewritten, + key: Some(name.to_string()), + original: None, + new: Some(Value::String(new_line.clone())), + }, + }, + }; + // A rewire lifted OUR OWN previous PATH section, not pre-vendor + // fragments: record `original: None` — the true originals live in the + // ledger entry being replaced, which the caller carries forward by + // wiring identity (`persist_vendor_entry`). + let lock_original = if lock_edit.rewired_ours { + None + } else { + let mut original_lines: Vec = lock_edit + .removed_spec_block + .iter() + .map(|l| Value::String(l.clone())) + .collect(); + if let Some(dep) = &lock_edit.old_dep_line { + original_lines.push(Value::String(dep.clone())); + } + Some(Value::Array(original_lines)) }; - let mut original_lines: Vec = lock_edit - .removed_spec_block - .iter() - .map(|l| Value::String(l.clone())) - .collect(); - if let Some(dep) = &lock_edit.old_dep_line { - original_lines.push(Value::String(dep.clone())); - } let mut new_lines: Vec = lock_edit .path_section .iter() @@ -479,7 +518,7 @@ pub async fn vendor_gem( kind: LOCK_WIRING_KIND.to_string(), action: WiringAction::Rewritten, key: Some(name.to_string()), - original: Some(Value::Array(original_lines)), + original: lock_original, new: Some(Value::Array(new_lines)), }; let mut wiring = vec![gemfile_record, lock_record]; @@ -496,6 +535,23 @@ pub async fn vendor_gem( original: Some(Value::String(orig_line.clone())), new: Some(Value::String(new_line.clone())), }); + } else if lock_edit.rewired_ours { + // Re-vendor with the bare path-form line already in place (our + // previous run stripped the registry token): the record must ride + // again with `original: None` — dropped, the first run's registry + // `sha256=` line would vanish from the ledger with the entry being + // replaced, and a later --revert could no longer restore it (a bare + // leftover on a registry gem hard-fails frozen installs, exit 16). + if let Some(bare) = &lock_edit.checksum_bare { + wiring.push(WiringRecord { + file: GEMFILE_LOCK.to_string(), + kind: LOCK_CHECKSUM_WIRING_KIND.to_string(), + action: WiringAction::Rewritten, + key: Some(name.to_string()), + original: None, + new: Some(Value::String(bare.clone())), + }); + } } let entry = VendorEntry { @@ -882,6 +938,22 @@ enum GemfilePlan { original_line: String, new_line: String, }, + /// The declaration already carries OUR OWN `path:` wiring from an older + /// patch uuid (a patch update changes the uuid, never the purl): + /// repoint it at the new copy in place, everything else on the line + /// byte-preserved. The wiring record carries `original: None` — the true + /// pre-vendor line lives in the ledger entry being replaced and the + /// caller carries it forward by wiring identity (`persist_vendor_entry`); + /// recording the old-uuid line would make a later revert "restore" a + /// dangling vendor pointer. `managed_block` is `Some(updated block)` when + /// the line sits inside our managed fence (the transitive-gem form): the + /// record then stays `Added` with the whole block, so revert still + /// deletes the fence. + RewireOurs { + original_line: String, + new_line: String, + managed_block: Option, + }, /// The gem is transitive (not declared): append a fenced managed block. Append { block: String }, } @@ -893,7 +965,9 @@ enum GemfilePlan { /// (inside a `group`/`platforms`/conditional block), parenthesized, /// continued onto the next line, conditional, or already carrying a /// `path:`/`git:`/`github:` source — is refused rather than guessed at: a -/// wrong Gemfile rewrite executes on every `bundle` invocation. +/// wrong Gemfile rewrite executes on every `bundle` invocation. The one +/// `path:` exception is our own vendored dir for this gem (an older patch +/// uuid), which is repointed in place — see [`GemfilePlan::RewireOurs`]. fn plan_gemfile_edit( text: &str, name: &str, @@ -935,6 +1009,30 @@ fn plan_gemfile_edit( "the `gem \"{name}\"` declaration uses a parenthesized call" )); } + // Our own wiring from an older patch uuid: the `path:` value parses as + // the socket vendor dir for exactly this gem. Repoint it in place — + // refusing here (the source-option blocklist below) would make every + // patch update demand a manual `vendor --revert` first. A path that + // parses as anything else (a user fork, another gem's dir) still refuses. + if let Some(prev_rel) = gem_line_path_value(&rest) { + if is_our_vendor_rel(prev_rel, name, version) { + let original_line = lines[idx].to_string(); + // The rel appears exactly once (its charset excludes quotes and + // `#`, and the code before `path:` cannot contain a `/`-bearing + // token); swapping just the value preserves quote style and + // trailing options verbatim. + let new_line = original_line.replacen(prev_rel, rel, 1); + let managed_block = (idx > 0 + && lines[idx - 1] == MANAGED_OPEN + && lines.get(idx + 1).is_some_and(|l| *l == MANAGED_CLOSE)) + .then(|| format!("{MANAGED_OPEN}\n{new_line}\n{MANAGED_CLOSE}\n")); + return Ok(GemfilePlan::RewireOurs { + original_line, + new_line, + managed_block, + }); + } + } if let Some(reason) = rest_blocks_edit(&rest) { return Err(format!( "the `gem \"{name}\"` declaration is not editable: {reason}" @@ -1025,11 +1123,45 @@ fn rest_blocks_edit(rest: &str) -> Option { None } +/// The quoted `path:` option value on a gem line's argument tail (only the +/// code before any `#` comment counts) — the form our own rewrite emits. +/// `None` for anything else (`:path =>`, interpolation, no `path:` at all): +/// those fall through to [`rest_blocks_edit`]'s refusal, fail-closed. +fn gem_line_path_value(rest: &str) -> Option<&str> { + let code = rest.split('#').next().unwrap_or(""); + let idx = code.find("path:")?; + if idx > 0 && !matches!(code.as_bytes()[idx - 1], b' ' | b'\t' | b',') { + return None; + } + let after = code[idx + "path:".len()..].trim_start(); + let q = after.chars().next()?; + if q != '"' && q != '\'' { + return None; + } + let value = &after[1..]; + let end = value.find(q)?; + Some(&value[..end]) +} + +/// True when a `path:`/`remote:` value is OUR vendored dir for exactly this +/// gem (`.socket/vendor/gem//-`) — the shape +/// [`vendor_gem`] wires, and the only wiring a patch UPDATE (new uuid, same +/// purl) may rewire. +fn is_our_vendor_rel(value: &str, name: &str, version: &str) -> bool { + parse_vendor_path(value) + .is_some_and(|p| p.eco == "gem" && p.leaf == format!("{name}-{version}")) +} + fn apply_gemfile_plan(text: &str, plan: &GemfilePlan) -> String { match plan { GemfilePlan::Rewrite { original_line, new_line, + } + | GemfilePlan::RewireOurs { + original_line, + new_line, + .. } => { let mut lines: Vec<&str> = text.split('\n').collect(); if let Some(i) = lines.iter().position(|l| *l == original_line) { @@ -1068,6 +1200,17 @@ struct LockEdit { /// "original" — reverting it onto a registry-sourced lock would break /// frozen installs). checksum_rewrite: Option<(String, String)>, + /// The spec block was lifted from OUR OWN previous PATH section (a + /// re-vendor to a newer patch uuid), not from GEM/specs: the lifted + /// fragments are this backend's own prior wiring, so the caller records + /// `original: None` and the true pre-vendor originals ride forward from + /// the ledger entry being replaced (`persist_vendor_entry`). + rewired_ours: bool, + /// The already-bare CHECKSUMS line for the gem, when one is present. + /// Only consulted on a re-vendor (`rewired_ours`): the checksum record + /// must ride again or the first run's registry `sha256=` restore line + /// drops out of the ledger with the entry being replaced. + checksum_bare: Option, } /// Produce the pair-edited lock text (see the module doc for the canonical @@ -1077,21 +1220,86 @@ struct LockEdit { fn edit_lock(text: &str, name: &str, version: &str, rel: &str) -> Result { let mut lines: Vec = text.split('\n').map(str::to_string).collect(); - // 1. Lift the gem's spec block out of GEM/specs. + // 1. Lift the gem's spec block out of GEM/specs — or, on a re-vendor to + // a newer patch uuid (same purl), out of the PATH section our previous + // run emitted. let (gem_start, gem_end) = section_span(&lines, "GEM").ok_or_else(|| "Gemfile.lock has no GEM section".to_string())?; if !(gem_start..gem_end).any(|i| lines[i] == " specs:") { return Err("Gemfile.lock GEM section has no specs: stanza".to_string()); } - let target = format!(" {name} ({version})"); - let block_start = (gem_start..gem_end) - .find(|&i| lines[i] == target) - .ok_or_else(|| format!("Gemfile.lock GEM specs has no entry `{name} ({version})`"))?; - let mut block_end = block_start + 1; - while block_end < gem_end && lines[block_end].starts_with(" ") { - block_end += 1; + // SECURITY/fail-closed: platform-suffixed installs were refused + // (`platform_gem_unsupported`) before this point, so a platform-suffixed + // GEM spec sibling means the lock disagrees with the installed tree — + // and lifting only the plain entry would leave the sibling behind as a + // stale registry spec. The CHECKSUMS branch below refuses the same + // shape, but only bundler ≥ 2.6 locks have a CHECKSUMS section to catch + // it in. + let platform_prefix = format!("{version}-"); + for line in lines.iter().take(gem_end).skip(gem_start + 1) { + if let Some((n, v)) = spec_entry(line) { + if n == name && v.starts_with(&platform_prefix) { + return Err(format!( + "Gemfile.lock GEM specs has a platform-suffixed entry `{n} ({v})` but the installed gem is not platform-specific; the lock disagrees with the install (re-resolve it before vendoring)" + )); + } + } } - let removed_spec_block: Vec = lines.drain(block_start..block_end).collect(); + let target = format!(" {name} ({version})"); + let mut rewired_ours = false; + let removed_spec_block: Vec = match (gem_start..gem_end).find(|&i| lines[i] == target) { + Some(block_start) => { + let mut block_end = block_start + 1; + while block_end < gem_end && lines[block_end].starts_with(" ") { + block_end += 1; + } + lines.drain(block_start..block_end).collect() + } + None => { + // Re-vendor: the entry lives in the PATH section our previous + // run emitted (remote parses as our vendored dir for exactly + // this gem). Lift the block and drop the old section — step 3 + // re-emits it at the NEW uuid's sorted position. The lifted + // lines are our own wiring, not pre-vendor originals: flagged + // via `rewired_ours` (see the `LockEdit` field docs). + let Some((ps, pe)) = find_our_path_section(&lines, name, version) else { + return Err(format!( + "Gemfile.lock GEM specs has no entry `{name} ({version})`" + )); + }; + let block_start = (ps..pe).find(|&i| lines[i] == target).ok_or_else(|| { + format!( + "Gemfile.lock PATH section for `{name}` lost its `{name} ({version})` spec entry" + ) + })?; + let mut block_end = block_start + 1; + while block_end < pe && lines[block_end].starts_with(" ") { + block_end += 1; + } + // Grammar-strict: besides the block, the section must be exactly + // what vendor wrote (header, one remote, specs:, blank + // separators). Anything extra — a hand edit, a merged-in second + // spec — would be destroyed by the drain below; never guess. + let non_block: Vec<&str> = (ps..pe) + .filter(|i| !(block_start..block_end).contains(i)) + .map(|i| lines[i].as_str()) + .filter(|l| !l.is_empty()) + .collect(); + if non_block.len() != 3 + || non_block[0] != "PATH" + || !non_block[1].starts_with(" remote: ") + || non_block[2] != " specs:" + { + return Err(format!( + "Gemfile.lock PATH section for `{name} ({version})` is not the shape vendor wrote; refusing to rewire it" + )); + } + let block: Vec = lines[block_start..block_end].to_vec(); + lines.drain(ps..pe); + rewired_ours = true; + block + } + }; // 2. DEPENDENCIES: exact pin + `!` path-source marker. A transitive gem // (absent pre-vendor) is inserted at bundler's sorted position — it is a @@ -1122,8 +1330,16 @@ fn edit_lock(text: &str, name: &str, version: &str, rel: &str) -> Result lines.insert(insert_at, new_dep_line.clone()), } - // 3. PATH section directly above the GEM section (bundler's canonical - // placement; spike claim 2). `remote:` is the bare relative path. + // 3. PATH section above the GEM section, at bundler's SORTED position + // among any existing PATH sections: bundler emits path/git/plugin + // sources sorted by identifier (source_list.rb `lock_other_sources`, + // verified against bundler 4.0.15) — `source at ``` for a path + // source, so PATH sections order by their remote path and all sit in one + // contiguous run (no other source's identifier can start with that + // prefix). Splicing at invocation order instead churns the committed + // lock on the next `bundle lock`. Non-PATH leading sections keep the + // legacy insert-before-GEM fallback. `remote:` is the bare relative + // path (spike claim 2). let mut path_section = vec![ "PATH".to_string(), format!(" remote: {rel}"), @@ -1134,9 +1350,29 @@ fn edit_lock(text: &str, name: &str, version: &str, rel: &str) -> Result our_ident => { + at = i; + break; + } + // Ours sorts after this section (a remote-less section is + // grammar-degenerate; keep the legacy after-everything spot). + _ => at = end.min(gem_hdr), + } + i = end; + } else { + i += 1; + } + } let mut insert = path_section.clone(); - insert.push(String::new()); // blank separator before GEM - lines.splice(gem_hdr..gem_hdr, insert); + insert.push(String::new()); // blank separator before the next section + lines.splice(at..at, insert); // 4. CHECKSUMS (bundler ≥ 2.6 `lockfile_checksums`): a path-sourced gem // keeps a BARE ` ()` entry — bundler's own re-lock emits @@ -1147,9 +1383,9 @@ fn edit_lock(text: &str, name: &str, version: &str, rel: &str) -> Result = None; + let mut checksum_bare: Option = None; if let Some((ck_start, ck_end)) = section_span(&lines, "CHECKSUMS") { let bare = format!(" {name} ({version})"); - let platform_prefix = format!("{version}-"); let mut plain_at: Option = None; for (i, line) in lines.iter().enumerate().take(ck_end).skip(ck_start + 1) { match checksum_entry(line) { @@ -1190,6 +1426,8 @@ fn edit_lock(text: &str, name: &str, version: &str, rel: &str) -> Result Result Result Option<(usize, usize)> { let start = lines.iter().position(|l| l.as_str() == header)?; + Some((start, section_end(lines, start))) +} + +/// End (exclusive) of the section whose column-0 header sits at `start` — +/// the [`section_span`] rule for a known header position. +fn section_end(lines: &[String], start: usize) -> usize { let mut end = start + 1; while end < lines.len() { let l = &lines[end]; @@ -1217,7 +1463,39 @@ fn section_span(lines: &[String], header: &str) -> Option<(usize, usize)> { } end += 1; } - Some((start, end)) + end +} + +/// Bundler's lock-sort identifier for a path source — `source at ``` +/// (`Source::Path#to_s`, aliased as `identifier`); sections order by a +/// byte-wise comparison of these, which Rust's `str` ordering matches. +fn path_source_identifier(path: &str) -> String { + format!("source at `{path}`") +} + +/// The ` remote: ` value of the section slice starting at its header line. +fn path_section_remote(section: &[String]) -> Option<&str> { + section.iter().find_map(|l| l.strip_prefix(" remote: ")) +} + +/// Find the PATH section whose `remote:` is OUR vendored dir for this gem — +/// any patch uuid (the previous run's wiring, sought during a re-vendor). +fn find_our_path_section(lines: &[String], name: &str, version: &str) -> Option<(usize, usize)> { + let mut i = 0; + while i < lines.len() { + if lines[i].as_str() == "PATH" { + let end = section_end(lines, i); + if path_section_remote(&lines[i..end]) + .is_some_and(|p| is_our_vendor_rel(p, name, version)) + { + return Some((i, end)); + } + i = end; + } else { + i += 1; + } + } + None } /// Name of a 2-space DEPENDENCIES entry (` rack (~> 3.1)` / ` rack!`). @@ -1239,6 +1517,25 @@ fn spec_entry_name(line: &str) -> Option<&str> { Some(rest.split(' ').next().unwrap_or(rest)) } +/// Parse a 4-space specs entry line: ` ()`, nothing after +/// the closing paren. Returns `(name, parenthesized token)` — the platform +/// suffix stays inside the token, mirroring [`checksum_entry`]'s grammar at +/// specs indentation (` ffi (1.17.2-aarch64-linux-gnu)`). +fn spec_entry(line: &str) -> Option<(&str, &str)> { + let rest = line.strip_prefix(" ")?; + if rest.is_empty() || rest.starts_with(' ') { + return None; + } + let open = rest.find(" (")?; + let after = &rest[open + 2..]; + let close = after.find(')')?; + let (name, ver, tail) = (&rest[..open], &after[..close], &after[close + 1..]); + if name.is_empty() || ver.is_empty() || !tail.is_empty() { + return None; + } + Some((name, ver)) +} + /// Parse a CHECKSUMS entry line: two-space indent, ` ()` or /// ` (-)`, then optional space-separated tokens /// (`sha256=` on registry entries, nothing on path entries). Returns @@ -1665,10 +1962,21 @@ mod tests { installed: &Path, record: &PatchRecord, dry_run: bool, + ) -> VendorOutcome { + run_vendor_purl(PURL, root, blobs, installed, record, dry_run).await + } + + async fn run_vendor_purl( + purl: &str, + root: &Path, + blobs: &Path, + installed: &Path, + record: &PatchRecord, + dry_run: bool, ) -> VendorOutcome { let sources = PatchSources::blobs_only(blobs); vendor_gem( - PURL, + purl, installed, root, record, @@ -1681,6 +1989,23 @@ mod tests { .await } + /// Simulate the CLI caller's `persist_vendor_entry` carry-forward: fill + /// the replacement entry's `original: None` holes from the entry being + /// replaced, by wiring identity (file, kind, key). + fn carry_forward_originals(prev: &VendorEntry, next: &mut VendorEntry) { + for rec in &mut next.wiring { + if rec.action == WiringAction::Rewritten && rec.original.is_none() { + if let Some(p) = prev + .wiring + .iter() + .find(|p| p.file == rec.file && p.kind == rec.kind && p.key == rec.key) + { + rec.original = p.original.clone(); + } + } + } + } + fn expected_lock_direct() -> String { format!( "PATH\n remote: {rel}\n specs:\n rack (3.2.6)\n base64 (>= 0.1.0)\n\nGEM\n remote: https://rubygems.org/\n specs:\n puma (6.4.2)\n nio4r (~> 2.0)\n\nPLATFORMS\n arm64-darwin-23\n ruby\n\nDEPENDENCIES\n puma\n rack (= 3.2.6)!\n\nBUNDLED WITH\n 2.5.22\n", @@ -2777,6 +3102,360 @@ mod tests { ); } + // ── multiple vendored gems: PATH sections sort like bundler's ──────────── + + /// Second gem for multi-PATH tests. Its uuid sorts BEFORE rack's + /// (`1a…` < `9f…`), so vendoring rack first is the order a naive + /// insert-before-GEM splice would leave unsorted. + const UUID_PUMA: &str = "1a2b3c4d-5e6f-4a1b-8c2d-3e4f5a6b7c8d"; + const PURL_PUMA: &str = "pkg:gem/puma@6.4.2"; + const PRISTINE_PUMA: &[u8] = b"module Puma\n VERSION = \"6.4.2\"\nend\n"; + const PATCHED_PUMA: &[u8] = + b"module Puma\n SOCKET_PATCHED = true\n VERSION = \"6.4.2\"\nend\n"; + const GEMSPEC_PUMA: &str = "Gem::Specification.new do |s|\n s.name = \"puma\"\n s.version = \"6.4.2\"\n s.require_paths = [\"lib\"]\nend\n"; + + fn puma_rel() -> String { + format!(".socket/vendor/gem/{UUID_PUMA}/puma-6.4.2") + } + + /// Add a puma install + blob + record alongside [`fixture`]'s rack, so a + /// test can vendor TWO gems into one project. + async fn add_puma_fixture(installed_rack: &Path, blobs: &Path) -> (PathBuf, PatchRecord) { + let gems = installed_rack.parent().unwrap(); + let installed = gems.join("puma-6.4.2"); + tokio::fs::create_dir_all(installed.join("lib")) + .await + .unwrap(); + tokio::fs::write(installed.join("lib/puma.rb"), PRISTINE_PUMA) + .await + .unwrap(); + let specs = gems.parent().unwrap().join("specifications"); + tokio::fs::write(specs.join("puma-6.4.2.gemspec"), GEMSPEC_PUMA) + .await + .unwrap(); + let before = compute_git_sha256_from_bytes(PRISTINE_PUMA); + let after = compute_git_sha256_from_bytes(PATCHED_PUMA); + tokio::fs::write(blobs.join(&after), PATCHED_PUMA) + .await + .unwrap(); + let mut files = HashMap::new(); + files.insert( + "lib/puma.rb".to_string(), + PatchFileInfo { + before_hash: before, + after_hash: after, + }, + ); + let record = PatchRecord { + uuid: UUID_PUMA.to_string(), + exported_at: "2026-06-09T00:00:00Z".to_string(), + files, + vulnerabilities: HashMap::new(), + description: String::new(), + license: String::new(), + tier: String::new(), + }; + (installed, record) + } + + fn expected_lock_two_path() -> String { + format!( + "PATH\n remote: {puma}\n specs:\n puma (6.4.2)\n nio4r (~> 2.0)\n\nPATH\n remote: {rack}\n specs:\n rack (3.2.6)\n base64 (>= 0.1.0)\n\nGEM\n remote: https://rubygems.org/\n specs:\n\nPLATFORMS\n arm64-darwin-23\n ruby\n\nDEPENDENCIES\n puma (= 6.4.2)!\n rack (= 3.2.6)!\n\nBUNDLED WITH\n 2.5.22\n", + puma = puma_rel(), + rack = copy_rel() + ) + } + + /// Bundler regenerates PATH sections sorted by source identifier — by + /// remote path, the uuid level deciding here (verified against a real + /// bundler 4.0.15 `bundle lock` over this exact two-PATH shape). The + /// splice must land each new section at that sorted position no matter + /// the vendor invocation order, or the committed lock churns on the + /// next `bundle lock`/`bundle install`. + #[tokio::test] + async fn test_two_path_sections_sorted_regardless_of_vendor_order() { + for rack_first in [true, false] { + let (_tmp, root, installed_rack, blobs, record_rack) = + fixture(GEMFILE_DIRECT, LOCK_DIRECT).await; + let (installed_puma, record_puma) = add_puma_fixture(&installed_rack, &blobs).await; + let runs: [(&str, &Path, &PatchRecord); 2] = if rack_first { + [ + (PURL, &installed_rack, &record_rack), + (PURL_PUMA, &installed_puma, &record_puma), + ] + } else { + [ + (PURL_PUMA, &installed_puma, &record_puma), + (PURL, &installed_rack, &record_rack), + ] + }; + for (purl, installed, record) in runs { + let (result, _e, _w) = unwrap_done( + run_vendor_purl(purl, &root, &blobs, installed, record, false).await, + ); + assert!(result.success, "vendor {purl} failed: {:?}", result.error); + } + let lock = tokio::fs::read_to_string(root.join(GEMFILE_LOCK)) + .await + .unwrap(); + assert_eq!(lock, expected_lock_two_path(), "rack_first={rack_first}"); + } + } + + // ── re-vendor: a patch update (new uuid, same purl) ────────────────────── + + /// Re-vendor uuid; sorts BEFORE `UUID_PUMA`'s (`0e…` < `1a…`). + const UUID2: &str = "0e1f2a3b-4c5d-4e6f-8a7b-9c0d1e2f3a4b"; + + /// A patch update moves the manifest to a NEW uuid for the same gem. The + /// CLI re-vendors straight over the first run's live wiring (originals + /// carried forward and the old uuid dir swept by the caller — no + /// revert-first; the cargo backend pins the same design). Both pair + /// files must be repointed in place, with `original: None` on the + /// rewired records. + #[tokio::test] + async fn test_revendor_new_uuid_direct_rewires_in_place() { + let (_tmp, root, installed, blobs, record) = fixture(GEMFILE_DIRECT, LOCK_DIRECT).await; + let (r1, e1, _) = unwrap_done(run_vendor(&root, &blobs, &installed, &record, false).await); + assert!(r1.success); + let entry1 = e1.unwrap(); + + let mut record2 = record.clone(); + record2.uuid = UUID2.to_string(); + let (r2, e2, _) = unwrap_done(run_vendor(&root, &blobs, &installed, &record2, false).await); + assert!(r2.success, "re-vendor must succeed: {:?}", r2.error); + + let new_rel = format!(".socket/vendor/gem/{UUID2}/rack-3.2.6"); + assert_eq!( + tokio::fs::read_to_string(root.join(GEMFILE)).await.unwrap(), + format!( + "source \"https://rubygems.org\"\n\ngem \"puma\"\ngem \"rack\", \"3.2.6\", path: \"{new_rel}\"\n" + ), + "Gemfile repointed in place" + ); + assert_eq!( + tokio::fs::read_to_string(root.join(GEMFILE_LOCK)) + .await + .unwrap(), + expected_lock_direct().replace(UUID, UUID2), + "lock repointed in place" + ); + // New copy built; the old uuid dir is left for the caller's + // stale-artifact sweep (the caller owns the ledger). + assert_eq!( + tokio::fs::read(root.join(&new_rel).join("lib/rack.rb")) + .await + .unwrap(), + PATCHED + ); + assert!(root.join(format!(".socket/vendor/gem/{UUID}")).exists()); + + // The rewired records carry `original: None` — never the old-uuid + // lines (reverting those would "restore" a dangling vendor pointer). + let mut entry2 = e2.expect("re-vendor emits the new ledger entry"); + assert_eq!(entry2.uuid, UUID2); + assert_eq!(entry2.wiring.len(), 2); + for rec in &entry2.wiring { + assert_eq!(rec.action, WiringAction::Rewritten); + assert!(rec.original.is_none(), "{rec:?}"); + } + + // With the caller's carry-forward applied, revert restores the + // PRE-VENDOR files byte-exactly. + carry_forward_originals(&entry1, &mut entry2); + let outcome = revert_gem(&entry2, &root, false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!( + !outcome + .warnings + .iter() + .any(|w| w.code == "vendor_lock_entry_drifted"), + "clean revert must not report drift: {:?}", + outcome.warnings + ); + assert_eq!( + tokio::fs::read_to_string(root.join(GEMFILE)).await.unwrap(), + GEMFILE_DIRECT + ); + assert_eq!( + tokio::fs::read_to_string(root.join(GEMFILE_LOCK)) + .await + .unwrap(), + LOCK_DIRECT + ); + } + + /// Transitive form: the managed block is repointed in place (never + /// duplicated) and the record stays `Added` with the WHOLE updated block, + /// so a later revert deletes the fence. + #[tokio::test] + async fn test_revendor_new_uuid_transitive_updates_managed_block() { + let (_tmp, root, installed, blobs, record) = + fixture(GEMFILE_TRANSITIVE, LOCK_TRANSITIVE).await; + let (r1, e1, _) = unwrap_done(run_vendor(&root, &blobs, &installed, &record, false).await); + assert!(r1.success); + let entry1 = e1.unwrap(); + + let mut record2 = record.clone(); + record2.uuid = UUID2.to_string(); + let (r2, e2, _) = unwrap_done(run_vendor(&root, &blobs, &installed, &record2, false).await); + assert!(r2.success, "re-vendor must succeed: {:?}", r2.error); + + let new_rel = format!(".socket/vendor/gem/{UUID2}/rack-3.2.6"); + let new_block = format!( + "{MANAGED_OPEN}\ngem \"rack\", \"3.2.6\", path: \"{new_rel}\"\n{MANAGED_CLOSE}\n" + ); + assert_eq!( + tokio::fs::read_to_string(root.join(GEMFILE)).await.unwrap(), + format!("source \"https://rubygems.org\"\n\ngem \"puma\"\n{new_block}"), + "ONE managed block, repointed — never a duplicate declaration" + ); + + let mut entry2 = e2.unwrap(); + assert_eq!(entry2.wiring[0].action, WiringAction::Added); + assert!(entry2.wiring[0].original.is_none()); + assert_eq!( + entry2.wiring[0].new.as_ref().unwrap(), + &Value::String(new_block) + ); + + carry_forward_originals(&entry1, &mut entry2); + let outcome = revert_gem(&entry2, &root, false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!( + !outcome + .warnings + .iter() + .any(|w| w.code == "vendor_lock_entry_drifted"), + "clean revert must not report drift: {:?}", + outcome.warnings + ); + assert_eq!( + tokio::fs::read_to_string(root.join(GEMFILE)).await.unwrap(), + GEMFILE_TRANSITIVE + ); + assert_eq!( + tokio::fs::read_to_string(root.join(GEMFILE_LOCK)) + .await + .unwrap(), + LOCK_TRANSITIVE + ); + } + + /// A re-vendor must RE-SORT: the replacement PATH section lands wherever + /// the NEW uuid sorts among the other vendored gems' sections, not where + /// the old one sat. + #[tokio::test] + async fn test_revendor_new_uuid_resorts_path_sections() { + let (_tmp, root, installed_rack, blobs, record_rack) = + fixture(GEMFILE_DIRECT, LOCK_DIRECT).await; + let (installed_puma, record_puma) = add_puma_fixture(&installed_rack, &blobs).await; + for (purl, installed, record) in [ + (PURL, &installed_rack, &record_rack), + (PURL_PUMA, &installed_puma, &record_puma), + ] { + let (result, _e, _w) = + unwrap_done(run_vendor_purl(purl, &root, &blobs, installed, record, false).await); + assert!(result.success, "vendor {purl} failed: {:?}", result.error); + } + + // The patch update moves rack to a uuid sorting BEFORE puma's. + let mut rack2 = record_rack.clone(); + rack2.uuid = UUID2.to_string(); + let (result, _e, _w) = + unwrap_done(run_vendor(&root, &blobs, &installed_rack, &rack2, false).await); + assert!(result.success, "re-vendor must succeed: {:?}", result.error); + + let lock = tokio::fs::read_to_string(root.join(GEMFILE_LOCK)) + .await + .unwrap(); + assert_eq!( + lock, + expected_lock_two_path() + .replace( + &format!("PATH\n remote: {puma}\n specs:\n puma (6.4.2)\n nio4r (~> 2.0)\n\nPATH\n remote: {rack}\n specs:\n rack (3.2.6)\n base64 (>= 0.1.0)\n\n", puma = puma_rel(), rack = copy_rel()), + &format!("PATH\n remote: {rack}\n specs:\n rack (3.2.6)\n base64 (>= 0.1.0)\n\nPATH\n remote: {puma}\n specs:\n puma (6.4.2)\n nio4r (~> 2.0)\n\n", puma = puma_rel(), rack = copy_rel().replace(UUID, UUID2)), + ), + "rack's section moved to the new uuid's sorted position" + ); + } + + /// On a re-vendor over a CHECKSUMS lock the checksum record must ride + /// AGAIN with `original: None`: dropped, the first run's registry + /// `sha256=` line would vanish from the ledger with the replaced entry, + /// and a post-update revert would leave a bare CHECKSUMS entry on a + /// registry gem (frozen installs exit 16). + #[tokio::test] + async fn test_revendor_new_uuid_checksums_keeps_restore_data() { + let (_tmp, root, installed, blobs, record) = + fixture_318(SPIKE_GEMFILE_CHECKSUMS, SPIKE_LOCK_CHECKSUMS_BEFORE).await; + let (r1, e1, _) = + unwrap_done(run_vendor_318(&root, &blobs, &installed, &record, false).await); + assert!(r1.success); + let entry1 = e1.unwrap(); + + let mut record2 = record.clone(); + record2.uuid = UUID2.to_string(); + let (r2, e2, _) = + unwrap_done(run_vendor_318(&root, &blobs, &installed, &record2, false).await); + assert!(r2.success, "re-vendor must succeed: {:?}", r2.error); + assert_eq!( + tokio::fs::read_to_string(root.join(GEMFILE_LOCK)) + .await + .unwrap(), + expected_lock_checksums().replace(UUID, UUID2) + ); + + let mut entry2 = e2.unwrap(); + assert_eq!(entry2.wiring.len(), 3, "{:?}", entry2.wiring); + let ck = &entry2.wiring[2]; + assert_eq!(ck.kind, LOCK_CHECKSUM_WIRING_KIND); + assert!(ck.original.is_none(), "{:?}", ck.original); + assert_eq!( + ck.new.as_ref().unwrap(), + &Value::String(" rack (3.1.8)".to_string()) + ); + + carry_forward_originals(&entry1, &mut entry2); + let outcome = revert_gem(&entry2, &root, false).await; + assert!(outcome.success, "{:?}", outcome.error); + assert!( + !outcome + .warnings + .iter() + .any(|w| w.code == "vendor_lock_entry_drifted"), + "clean revert must not report drift: {:?}", + outcome.warnings + ); + assert_eq!( + tokio::fs::read_to_string(root.join(GEMFILE_LOCK)) + .await + .unwrap(), + SPIKE_LOCK_CHECKSUMS_BEFORE, + "registry sha256 line restored" + ); + assert_eq!( + tokio::fs::read_to_string(root.join(GEMFILE)).await.unwrap(), + SPIKE_GEMFILE_CHECKSUMS + ); + } + + /// The GEM-specs twin of `test_checksums_platform_sibling_fails_closed`: + /// on a bundler < 2.6 lock (no CHECKSUMS section to catch it in) a + /// platform-suffixed sibling spec must fail the lift closed — lifting + /// only the plain entry would leave the sibling behind as a stale + /// registry spec. + #[test] + fn test_gem_specs_platform_sibling_fails_closed() { + let lock = "GEM\n remote: https://rubygems.org/\n specs:\n nokogiri (1.16.0)\n racc (~> 1.4)\n nokogiri (1.16.0-arm64-darwin)\n racc (~> 1.4)\n\nPLATFORMS\n arm64-darwin\n ruby\n\nDEPENDENCIES\n nokogiri\n\nBUNDLED WITH\n 2.5.22\n"; + let rel = format!(".socket/vendor/gem/{UUID}/nokogiri-1.16.0"); + let err = match edit_lock(lock, "nokogiri", "1.16.0", &rel) { + Err(e) => e, + Ok(_) => panic!("a platform-suffixed GEM specs sibling must fail closed"), + }; + assert!(err.contains("platform-suffixed"), "{err}"); + } + /// Trailing options on the declaration (`require: false`, `group: :test`, /// …) must survive the rewrite: dropping `require: false` auto-requires /// the gem at boot, changing app behavior while vendored (the redirect diff --git a/crates/socket-patch-core/src/vendor/registry_fetch.rs b/crates/socket-patch-core/src/vendor/registry_fetch.rs index 0f0c92a5..532f02f2 100644 --- a/crates/socket-patch-core/src/vendor/registry_fetch.rs +++ b/crates/socket-patch-core/src/vendor/registry_fetch.rs @@ -26,6 +26,7 @@ use sha2::{Digest, Sha256, Sha384, Sha512}; use crate::constants::USER_AGENT; use crate::crawlers::go_crawler::encode_module_path; use crate::patch::apply::is_safe_relative_subpath; +use crate::patch::path_safety::is_safe_single_segment; use super::lock_inventory::{LockIntegrity, LockfileEntry}; @@ -237,6 +238,18 @@ async fn fetch_gem( entry: &LockfileEntry, client: &reqwest::Client, ) -> Result { + // The staged leaf must be the canonical `{name}-{version}`: the gem + // vendor backend refuses any other leaf as a platform-suffixed install + // (`platform_gem_unsupported`), so a generic name would kill the whole + // auto-fetch path. The coordinates thereby become a tempdir path + // component — `inventory_gemfile_lock` already filters both, but + // re-assert locally (defense in depth), before any network I/O. + if !is_safe_single_segment(&entry.name) || !is_safe_single_segment(&entry.version) { + return Err(FetchError::Failed(format!( + "unsafe gem coordinates `{}` @ `{}` — refusing to stage", + entry.name, entry.version + ))); + } let Some(url) = entry.resolved.clone() else { return Err(FetchError::Unverifiable(format!( "no download URL for {}@{}", @@ -248,7 +261,7 @@ async fn fetch_gem( let tmp = tempfile::tempdir() .map_err(|e| FetchError::Failed(format!("cannot create fetch tempdir: {e}")))?; - let dir = tmp.path().join("gem"); + let dir = tmp.path().join(format!("{}-{}", entry.name, entry.version)); extract_gem_data(&bytes, &dir).map_err(FetchError::Failed)?; Ok(FetchedPackage { dir, @@ -1482,6 +1495,36 @@ mod tests { "data.tar.gz content extracts at the root (no strip)" ); assert!(fetched.dir().join("README.md").is_file()); + // The staged leaf must be the canonical `{name}-{version}`: + // vendor_gem's platform-suffix guard refuses any other leaf + // (`platform_gem_unsupported`), which killed lockfile auto-fetch + // when this dir was named `gem`. + assert_eq!( + fetched.dir().file_name().unwrap().to_string_lossy(), + "rails-7.1.0", + "staged dir leaf must satisfy vendor_gem's `{{name}}-{{version}}` check" + ); + } + + #[tokio::test] + async fn gem_fetch_refuses_unsafe_coordinates_without_network() { + // The coordinates become the staged-dir leaf, so a separator-bearing + // name must refuse — and BEFORE any I/O (the URL would hard-fail if + // contacted). + let entry = LockfileEntry { + ecosystem: "gem", + name: "ra/ils".into(), + version: "7.1.0".into(), + purl: "pkg:gem/ra/ils@7.1.0".into(), + resolved: Some("http://127.0.0.1:1/nope.gem".into()), + integrity: LockIntegrity::Sha256Hex("0".repeat(64)), + }; + match fetch_and_stage(&entry, &build_registry_client()).await { + Err(FetchError::Failed(msg)) => { + assert!(msg.contains("unsafe gem coordinates"), "{msg}") + } + other => panic!("expected coordinate refusal, got {other:?}"), + } } #[tokio::test] diff --git a/crates/socket-patch-core/tests/crawler_ruby_e2e.rs b/crates/socket-patch-core/tests/crawler_ruby_e2e.rs index ac137863..d611dded 100644 --- a/crates/socket-patch-core/tests/crawler_ruby_e2e.rs +++ b/crates/socket-patch-core/tests/crawler_ruby_e2e.rs @@ -290,6 +290,47 @@ async fn get_gem_paths_vendor_bundle_takes_precedence_over_global() { ); } +/// A JRuby deployment (`bundle install --deployment` under JRuby) puts +/// gems in `vendor/bundle/jruby//gems` — Bundler scopes the path +/// by `Gem.ruby_engine`, not the literal `ruby`. The crawler must discover +/// that layout end-to-end; hardcoding the `ruby` engine dir made JRuby and +/// TruffleRuby deployments silently yield zero gems. No Gemfile is staged, +/// so a regression cannot green via the `gem env` fallback. +#[tokio::test] +#[serial_test::parallel] +async fn get_gem_paths_vendor_bundle_jruby_engine_layout() { + let tmp = tempfile::tempdir().unwrap(); + let jruby_gems = tmp + .path() + .join("vendor") + .join("bundle") + .join("jruby") + .join("3.1.4.0") + .join("gems"); + tokio::fs::create_dir_all(jruby_gems.join("rails-7.1.0").join("lib")) + .await + .unwrap(); + + let crawler = RubyCrawler; + let paths = crawler + .get_gem_paths(&options_at(tmp.path())) + .await + .unwrap(); + assert_eq!( + paths, + vec![jruby_gems.clone()], + "vendor/bundle/jruby/*/gems must be discovered; got {paths:?}" + ); + + // End-to-end: the gem inside the jruby scope is crawled. + let crawled = crawler.crawl_all(&options_at(tmp.path())).await; + let purls: Vec<&str> = crawled.iter().map(|p| p.purl.as_str()).collect(); + assert!( + purls.contains(&"pkg:gem/rails@7.1.0"), + "gem in a jruby vendor scope must be crawled; got {purls:?}" + ); +} + #[tokio::test] #[serial_test::parallel] async fn get_gem_paths_no_gemfile_returns_empty() { diff --git a/crates/socket-patch-core/tests/fixtures/redirect/gem/bundler/basic/expected-edits.json b/crates/socket-patch-core/tests/fixtures/redirect/gem/bundler/basic/expected-edits.json index 9467dc3d..314c0958 100644 --- a/crates/socket-patch-core/tests/fixtures/redirect/gem/bundler/basic/expected-edits.json +++ b/crates/socket-patch-core/tests/fixtures/redirect/gem/bundler/basic/expected-edits.json @@ -12,6 +12,7 @@ "kind": "redirect_gemfile_lock_checksum", "action": "rewritten", "key": "rails", + "original": "rails (7.0.0) sha256=2222222222222222222222222222222222222222222222222222222222222222", "new": "rails (7.0.0) sha256=deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef" } ] From 99f9cd0cd476de62eec33a4de55b3752d9cd2d35 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Thu, 13 Aug 2026 17:15:32 -0700 Subject: [PATCH 2/5] fix(review): CRLF-tolerant source-block guard; dry-run VEX skip for all scan arms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses both Bugbot findings on #175: - The grant-agnostic Gemfile idempotency guard required LF (`do\n`), so a core.autocrlf checkout of a previously-redirected Gemfile was not recognized and the indented gem line inside the block got wrapped in a second, nested source block on re-run. The recognizer now accepts `do\r?\n`; pinned by gemfile_rerun_on_crlf_checkout_never_nests (verified red without the fix). - The dry-run VEX skip only covered the vendor JSON arm; the interactive `scan --vendor --dry-run --vex` path (embed_vex_human) and the JSON `scan --apply --dry-run --vex` path (embed_vex_into_json at the apply fall-through) still generated the document — exiting 1 on a not-yet-vendored project or writing the attestation during --dry-run. The guard now lives at the top of both embed helpers, covering every scan arm; pinned by scan_vendor_dry_run_with_vex_interactive_* and scan_apply_json_dry_run_with_vex_* (both verified red without it). Co-Authored-By: Claude Fable 5 --- .../socket-patch-cli/src/commands/scan/mod.rs | 16 ++++ .../socket-patch-cli/tests/in_process_scan.rs | 86 +++++++++++++++++++ .../src/patch/redirect/mod.rs | 53 +++++++++++- 3 files changed, 154 insertions(+), 1 deletion(-) diff --git a/crates/socket-patch-cli/src/commands/scan/mod.rs b/crates/socket-patch-cli/src/commands/scan/mod.rs index eefed677..e4a5320a 100644 --- a/crates/socket-patch-cli/src/commands/scan/mod.rs +++ b/crates/socket-patch-cli/src/commands/scan/mod.rs @@ -280,6 +280,15 @@ async fn embed_vex_into_json( if vex_args.vex.is_none() || base_code != 0 { return base_code; } + // A dry run is a non-mutating preview: generating here would verify the + // deliberately untouched tree (failing outright on a not-yet-vendored + // project) and write an attestation file to disk. The marker keeps the + // request visible to JSON consumers instead of silently dropping it + // (same shape as the vendor JSON arm's early return). + if common.dry_run { + result["vex"] = serde_json::json!({ "skipped": true, "reason": "dry_run" }); + return base_code; + } let params = vex_args.to_build_params(); match generate_vex_from_manifest_path(common, ¶ms, manifest_path).await { Ok(summary) => { @@ -314,6 +323,13 @@ async fn embed_vex_human( if vex_args.vex.is_none() || base_code != 0 { return base_code; } + // Dry-run twin of the JSON guard above: no generation, no file write. + if common.dry_run { + if !common.silent { + println!("[dry-run] VEX generation skipped. No attestation written."); + } + return base_code; + } let params = vex_args.to_build_params(); match generate_vex_from_manifest_path(common, ¶ms, manifest_path).await { Ok(summary) => { diff --git a/crates/socket-patch-cli/tests/in_process_scan.rs b/crates/socket-patch-cli/tests/in_process_scan.rs index 96cb0111..c0ea588f 100644 --- a/crates/socket-patch-cli/tests/in_process_scan.rs +++ b/crates/socket-patch-cli/tests/in_process_scan.rs @@ -1540,3 +1540,89 @@ async fn scan_vendor_dry_run_with_vex_does_not_write_attestation_file() { "vendor dry run must leave the manifest byte-for-byte unchanged" ); } + +/// The INTERACTIVE arm's twin: `scan --vendor --dry-run --vex` without +/// `--json` returns through `embed_vex_human`, which generated (and wrote) +/// the document for real — exit 1 on a not-yet-vendored project, an +/// attestation file on disk otherwise. The dry-run guard lives in the embed +/// helpers, so both output modes must skip. +#[tokio::test] +#[serial] +async fn scan_vendor_dry_run_with_vex_interactive_does_not_fail_or_write() { + let server = MockServer::start().await; + mock_batch_one(&server).await; + mock_by_package(&server).await; + + let tmp = tempfile::tempdir().unwrap(); + write_root_package_json(tmp.path()); + write_npm_package(tmp.path(), "in-proc-scan", "1.0.0"); + + let vex_path = tmp.path().join("vendor-dry-interactive.vex.json"); + let mut args = default_args(tmp.path()); + args.common.api_url = Some(server.uri()); + args.common.json = false; + args.vendor = true; + args.common.dry_run = true; + args.vex.vex = Some(vex_path.clone()); + + let code = run_scrubbed(args).await; + assert_eq!( + code, 0, + "an interactive vendor dry run must not fail for not-yet-vendored state" + ); + assert!( + !vex_path.exists(), + "interactive dry run must not write the VEX file" + ); +} + +/// `scan --apply --json --dry-run --vex`: the JSON apply arm synthesizes its +/// preview and falls through to `embed_vex_into_json` with apply_code 0, so +/// without the guard the dry run generated and wrote the attestation. +#[tokio::test] +#[serial] +async fn scan_apply_json_dry_run_with_vex_does_not_write_attestation() { + let server = MockServer::start().await; + mock_batch_one(&server).await; + mock_by_package(&server).await; + + let tmp = tempfile::tempdir().unwrap(); + write_root_package_json(tmp.path()); + write_npm_package(tmp.path(), "in-proc-scan", "1.0.0"); + + // Attestable manifest (same fixture as the vendor twin above): metadata + // for the statement, `setup.manual: ["npm"]`, `--vex-no-verify` below. + let socket = tmp.path().join(".socket"); + std::fs::create_dir_all(&socket).unwrap(); + std::fs::write( + socket.join("manifest.json"), + r#"{ "patches": { + "pkg:npm/in-proc-scan@1.0.0": { + "uuid": "11111111-1111-4111-8111-111111111111", + "exportedAt": "2024-01-01T00:00:00Z", + "files": {}, + "vulnerabilities": { "GHSA-aaaa-bbbb-cccc": { + "cves": ["CVE-2024-0001"], "summary": "s", + "severity": "HIGH", "description": "d" + }}, + "description": "x", "license": "MIT", "tier": "free" + } + }, "setup": { "manual": ["npm"] } }"#, + ) + .unwrap(); + + let vex_path = tmp.path().join("apply-dry.vex.json"); + let mut args = default_args(tmp.path()); + args.common.api_url = Some(server.uri()); + args.apply = true; + args.common.dry_run = true; + args.vex.vex = Some(vex_path.clone()); + args.vex.vex_no_verify = true; + + let code = run_scrubbed(args).await; + assert_eq!(code, 0, "apply dry run with attestable state must exit 0"); + assert!( + !vex_path.exists(), + "apply dry run must not write the VEX file even when generation would succeed" + ); +} diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 87740fcf..3f337eed 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -1783,10 +1783,14 @@ fn rewrite_gem( url_pat.replace(®ex::escape(&format!("/{rotating}/")), "/[^/\"]+/"); } } + // `\r?\n`: the rewriter emits LF, but a `core.autocrlf` checkout + // rewrites the working tree to CRLF — the guard must still + // recognize the block there, or the indented `gem` line inside + // it falls through to `gem_line_re` and gets wrapped again. let block_re = Regex::new( &(String::from(r#"(?m)^source "("#) + &url_pat - + r#")" do\n gem ["']"# + + r#")" do\r?\n gem ["']"# + ®ex::escape(&dep.name) + r#"["']"#), ) @@ -3888,6 +3892,53 @@ mod tests { ); } + /// A `core.autocrlf` checkout rewrites a previously-redirected Gemfile to + /// CRLF. The block recognizer must still see the Socket source block + /// there: if it misses, the indented `gem` line inside the block matches + /// `gem_line_re` and gets wrapped in a second, nested source block. + #[test] + fn gemfile_rerun_on_crlf_checkout_never_nests() { + fn ov(token: &str) -> DepOverride { + let mut o = gem_override("rails", "7.0.0"); + o.token = token.into(); + if let Some(r) = o.registry_override.as_mut() { + r.index_url = format!("https://patch.test/gem/{token}/uuid/"); + } + o + } + // The block exactly as run 1 writes it, after a CRLF checkout. + let crlf_gemfile = "source \"https://rubygems.org\"\r\n\r\n\ + source \"https://patch.test/gem/tok-one/uuid/\" do\r\n \ + gem \"rails\", \"7.0.0\"\r\nend\r\n"; + let mut files = BTreeMap::new(); + files.insert("Gemfile".to_string(), crlf_gemfile.to_string()); + + // Same grant: recognized in place, a true no-op. + let same = rewrite_registry_redirect(&files, &[ov("tok-one")]); + assert!( + !same.files.contains_key("Gemfile"), + "same-grant re-run on a CRLF checkout must not rewrite the Gemfile: {:?}", + same.files.get("Gemfile") + ); + + // Rotated grant: URL refreshed inside the existing block, never nested. + let rotated = rewrite_registry_redirect(&files, &[ov("tok-two")]); + let out = rotated + .files + .get("Gemfile") + .expect("rotated grant refreshes the URL on a CRLF checkout"); + assert_eq!( + out.matches("source \"https://patch.test/gem/").count(), + 1, + "exactly one Socket source block, never nested: {out}" + ); + assert!(!out.contains("tok-one"), "old grant token gone: {out}"); + assert!( + out.contains("source \"https://patch.test/gem/tok-two/uuid/\" do\r\n"), + "existing CRLF block body left intact: {out}" + ); + } + /// A gem-level source option (`git:` / `path:` / `github:` / `source:`) /// preserved into the Socket source block OVERRIDES it in bundler's DSL, /// leaving the redirect a silent no-op that still gets attested. Fail From 4f68bc4975ba9d546e44c600e7a5604c5d23d2ff Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Thu, 13 Aug 2026 17:43:54 -0700 Subject: [PATCH 3/5] =?UTF-8?q?fix(gem):=20production-safe=20bundler=20plu?= =?UTF-8?q?gin=20=E2=80=94=20tolerant=20bootstrap,=20honest=20digest,=20pr?= =?UTF-8?q?oject-scoped=20stamp?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The generated Bundler setup plugin could deadlock a project on its own bootstrap, its design comment described trigger behavior bundler does not have, and its digest stamp was a machine-global file blind to the actual gem-file state. All trigger claims below were re-derived empirically against real bundler 4.0.15 (host) and 2.7.2 (docker image). plugins.rb template (+ published socket-patch-bundler twin): - [P0] bootstrap deadlock: bundler evaluates plugins.rb at plugin REGISTRATION, before any project gem is installed; the load-time SocketPatch.apply! got apply's exit 1 ("No packages found") and raised Bundler::BundlerError, so the FIRST `bundle install` of every fresh clone of a setup-wired project died (exit 29 under 4.0.15, exit 1 under 2.7) and every retry failed identically (registration never completed). Patch failures now warn once per process — naming what failed and the manual remediation — and let the install continue; SOCKET_PATCH_STRICT=1 restores the raise. The load-time and per-gem triggers are additionally stamp-gated so the bootstrap install stays quiet and defers to the forced after-install-all pass. - [P1] trigger reality: the header claimed plugins.rb runs during the Gemfile pass "on EVERY bundle invocation" — false; bundler evaluates a plugin when a subscribed hook event first fires. Measured surface (identical on 2.7 and 4.0): every `bundle install` — fresh AND fully cached — fires before/after-install(-all); `bundle pristine` fires ONLY the per-gem events; `bundle exec`/`bundle check`/`gem pristine` fire nothing. The plugin now also subscribes `after-install` (digest+stamp-gated), which catches `bundle pristine`'s patch reversion in the same run, and the digest folds in the on-disk CONTENT of every gem-patch target (resolved from the manifest purls under Bundler.bundle_path/gems), recomputed after apply — so out-of-band reversion flips the digest even when every committed input is byte-identical. Header documents precisely which flows re-apply and which cannot, including the stale .bundle/plugin/index caveat for checkouts registered by an older plugin version. - [P2] stamp location: the digest stamp was a fixed-name file under Bundler.bundle_path — the interpreter's machine-global gem dir when no bundle path is configured, shared and clobbered across every socket-patch project on the host. It now lives at .socket/gem-plugin-stamp (project-local, excluded from its own digest inputs); the legacy global stamp is deleted best-effort and never read. launcher.rb (gem/socket-patch): - Windows arm now propagates the child's real exit code instead of collapsing every non-zero exit to 1. - the binary-cache install is atomic: staged as a temp file in the destination dir, chmodded, then renamed into place (cross-run race on Windows rename tolerated when the winner already published). - first-run failures outside LauncherError exit with a clean one-line message instead of a raw backtrace; the PowerShell Expand-Archive fallback quotes paths containing single quotes; `version`'s documented from-a-checkout fallback never engaged because Gem::MissingSpecError is a Gem::LoadError (ScriptError family), not a StandardError — found by the new launcher guard. - socket-patch-bundler.gemspec: stale `git:` comment corrected to `path:` (the source has been path: since #150). setup-matrix driver (gem-scoped, npm-family byte-identical — verified by diffing the fixtures the old and new driver produce for npm across all patchsets): - the gem fixture now serves the REAL git-blob beforeHash probed from the published .gem (`gem fetch` + `gem unpack`, mirroring docker_e2e_gem's probe; verified against an independent oracle), so hash-gated gem apply passes the variant gate without --force. With the deadlock fix this turns the formerly-gapped gem with-setup docker cases green: the full 6-case gem matrix passes in BOTH host mode (bundler 4.0.15, real rubygems.org installs) and docker mode (rebuilt image, bundler 2.7) — no dependency on any sibling apply change. Tests (each red without its fix): - core template invariants: test_plugin_template_failure_policy_and_ stamp_location (new) + test_templates_are_well_formed (extended) pin the tolerant reporter, strict hatch, stamp constants, legacy cleanup, target-content digest, and the published twin's parity — 2 failures against the old template. - setup_matrix_gem::plugin_runtime drives the plugin generated by the REAL binary through REAL `bundle install` runs with a fake apply: first_bundle_install_survives_failing_apply (the P0 repro: red at exit 29 on the old template), strict_mode_fails_bundle_install_on_apply_ failure, successful_apply_stamps_project_scoped (stamp path + exactly one forced apply per cached install), digest_tracks_gem_file_content_ and_legacy_stamp_is_removed (plain-ruby drive; red on the old manifest-only digest and old stamp path). - setup_matrix_gem::launcher_guard drives launcher.rb with host ruby: windows_branch_propagates_child_exit_code (red: 7 collapsed to 1), unexpected_download_errors_exit_cleanly (red: raw backtrace), powershell_quote_doubles_single_quotes and install_executable_is_atomic_into_place (red: helpers absent). 7 of 8 runtime/launcher guards fail against the base-branch code. Verified: core 2461/0, cli lib 350/0, setup_matrix_gem 11/11 (incl. the docker-mode 6-case matrix on a fresh image AND host-mode 4.0.15 run), docker_e2e_gem 2/2, docker_e2e_vendor_gem 1/1, e2e_gem 11/11 (incl. live lifecycle), clippy+fmt clean on both crates. Co-Authored-By: Claude Fable 5 --- .../tests/setup_matrix_gem.rs | 638 +++++++++++++++++- crates/socket-patch-core/src/setup/gem/mod.rs | 96 ++- .../src/setup/gem/templates/plugins.rb.tmpl | 269 +++++--- gem/socket-patch-bundler/plugins.rb | 262 +++++-- .../socket-patch-bundler.gemspec | 2 +- gem/socket-patch/lib/socket_patch/launcher.rb | 60 +- tests/setup_matrix/run-case.sh | 38 +- 7 files changed, 1178 insertions(+), 187 deletions(-) diff --git a/crates/socket-patch-cli/tests/setup_matrix_gem.rs b/crates/socket-patch-cli/tests/setup_matrix_gem.rs index c8d68164..d4fa72cb 100644 --- a/crates/socket-patch-cli/tests/setup_matrix_gem.rs +++ b/crates/socket-patch-cli/tests/setup_matrix_gem.rs @@ -2,20 +2,25 @@ //! — it appends a managed `plugin "socket-patch"` block to the Gemfile and //! generates a committed in-tree Bundler plugin under `.socket/bundler-plugin/` //! whose `plugins.rb` re-runs `socket-patch apply --ecosystems gem` on every -//! `bundle install` (load-time digest gate + `after-install-all` hook). +//! `bundle install` (digest-gated load-time + per-gem `after-install` +//! triggers, forced `after-install-all` re-apply). //! -//! The with-setup Docker cases (`baseline_with_setup`, `alt_content_patchset`) -//! are still a [BASELINE GAP], for two structural reasons (verified -//! 2026-08-13): (a) installing the plugin evaluates `plugins.rb` BEFORE any -//! project gems land, and the load-time `SocketPatch.apply!` treats apply's -//! exit 1 ("No packages found that match available patches") as a genuine -//! failure and raises `Bundler::BundlerError`, so the FIRST `bundle install` -//! after `setup` on a never-installed project always dies — this is the gem -//! twin of the documented apply exit-semantics issue; (b) the matrix fixture's -//! synthetic beforeHashes only pass hash-gated ecosystems via npm's -//! mismatch-warn-and-apply path, which gem apply does not have. The -//! `after-install-all` re-apply flow itself works (asserted in-container with -//! realistic hashes). +//! The two structural reasons the with-setup Docker cases +//! (`baseline_with_setup`, `alt_content_patchset`) used to be a +//! [BASELINE GAP] are both fixed (2026-08-13): (a) the bootstrap deadlock — +//! installing the plugin evaluates `plugins.rb` BEFORE any project gems land, +//! and the old load-time `SocketPatch.apply!` treated apply's exit 1 ("No +//! packages found") as fatal (`Bundler::BundlerError`), killing the FIRST +//! `bundle install` of every fresh checkout — is gone: the generated plugin +//! now warns-and-continues on apply failures (`SOCKET_PATCH_STRICT=1` +//! restores the raise), pinned by [`plugin_runtime`] below; (b) the fixture's +//! synthetic all-zeros beforeHash — which hash-gated gem apply (no npm-style +//! mismatch-warn-and-apply path) always rejected — is replaced by the real +//! git-blob hash probed from the published .gem (`resolve_before_hash` in +//! `run-case.sh`, mirroring docker_e2e_gem). NOTE: in Docker mode the matrix +//! runs the binary BAKED INTO the local image; an image built before this fix +//! generates the old raising plugin and still red-flags these cases — rebuild +//! the image (or run with `SOCKET_PATCH_TEST_HOST=1`) to see them pass. //! //! IMPORTANT — why this file carries a real assertion of its own: //! `smc::run_pm("gem", "bundler")` routes gem through the shared Docker @@ -257,9 +262,16 @@ mod host_guard { rb.contains("\"--ecosystems\", \"gem\", \"--offline\""), "plugins.rb must shell the gem-scoped offline apply:\n{rb}" ); + // Tolerant by default (a raise at plugin registration deadlocks a + // fresh checkout's first `bundle install`), with the strict escape + // hatch still raising Bundler::BundlerError. + assert!( + rb.contains("SOCKET_PATCH_STRICT"), + "plugins.rb must carry the strict-mode escape hatch:\n{rb}" + ); assert!( rb.contains("BundlerError"), - "plugins.rb must fail loud (raise Bundler::BundlerError) on a patch failure:\n{rb}" + "plugins.rb must still raise Bundler::BundlerError in strict mode:\n{rb}" ); // ── check (after setup): configured, exit 0 ───────────────────────── @@ -387,3 +399,601 @@ mod host_guard { ); } } + +// ───────────────────────────────────────────────────────────────────────── +// Runtime guards for the GENERATED plugin, driven through a REAL `bundle +// install` (host bundler; validated against 4.0.15, and the same flows +// against bundler 2.7 in the gem Docker image during development). Each test +// wires a scratch project with the actual CLI binary (`setup --yes`), points +// SOCKET_PATCH_BIN at a fake apply whose exit code and invocation log we +// control, and asserts on bundler's real exit status + the on-disk state. +// +// Soft-skips (loudly, mirroring the docker_e2e_* convention) when no +// `bundle`/`ruby` toolchain is on PATH — the CI setup-matrix job and any dev +// machine with ruby run them for real. +// ───────────────────────────────────────────────────────────────────────── +#[cfg(unix)] +mod plugin_runtime { + use std::os::unix::fs::PermissionsExt; + use std::path::{Path, PathBuf}; + use std::process::Command; + + /// Manifest fixture: one committed gem patch record (hashes are dummies — + /// the fake apply never checks them; what matters is that the manifest + /// EXISTS so the plugin's applier engages). + const MANIFEST: &str = r#"{ + "patches": { + "pkg:gem/colorize@1.1.0": { + "uuid": "aaaaaaaa-aaaa-4aaa-8aaa-aaaaaaaaaaaa", + "exportedAt": "2026-01-01T00:00:00Z", + "files": { "package/lib/colorize.rb": { "beforeHash": "0000000000000000000000000000000000000000000000000000000000000000", "afterHash": "1111111111111111111111111111111111111111111111111111111111111111" } }, + "vulnerabilities": {}, + "description": "plugin-runtime fixture", + "license": "MIT", + "tier": "free" + } + } +} +"#; + + /// Hand-pinned stamp locations (independent oracles, not copies of the + /// template constants): the project-scoped stamp the plugin must write, + /// and the legacy fixed-name file it must never write again. + const STAMP_REL: &str = ".socket/gem-plugin-stamp"; + const LEGACY_STAMP_NAME: &str = ".socket-patch-gem-stamp"; + + fn binary() -> PathBuf { + env!("CARGO_BIN_EXE_socket-patch").into() + } + + fn have(cmd: &str) -> bool { + Command::new(cmd) + .arg("--version") + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .map(|s| s.success()) + .unwrap_or(false) + } + + /// Strip every ambient var that could flip a verdict: the CLI's SOCKET_* + /// surface (a dev's SOCKET_PATCH_STRICT or SOCKET_DRY_RUN must not leak + /// into the child), and bundler/rubygems config that could retarget the + /// install (BUNDLE_GEMFILE, GEM_HOME, RUBYOPT). + fn scrub(cmd: &mut Command) { + for (key, _) in std::env::vars_os() { + let name = key.to_string_lossy().into_owned(); + let hit = (name.starts_with("SOCKET_") && name != "SOCKET_NO_CONFIG") + || name.starts_with("BUNDLE_") + || name.starts_with("GEM_") + || name == "RUBYOPT"; + if hit { + cmd.env_remove(&name); + } + } + cmd.env("SOCKET_TELEMETRY_DISABLED", "1"); + } + + fn run(mut cmd: Command) -> (i32, String, String) { + let out = cmd.output().expect("spawn child process"); + ( + out.status.code().unwrap_or(-1), + String::from_utf8_lossy(&out.stdout).to_string(), + String::from_utf8_lossy(&out.stderr).to_string(), + ) + } + + /// `bundle install` in `root` with the fake apply bin + extra env. + fn bundle_install(root: &Path, fake: &Path, extra: &[(&str, &str)]) -> (i32, String, String) { + let mut cmd = Command::new("bundle"); + cmd.arg("install").current_dir(root); + scrub(&mut cmd); + cmd.env("BUNDLE_PATH", "vendor/bundle"); + cmd.env("SOCKET_PATCH_BIN", fake); + for (k, v) in extra { + cmd.env(k, v); + } + run(cmd) + } + + /// A fake `socket-patch` that logs each invocation and exits `code`. + /// Returns (bin path, log path). + fn write_fake_apply(dir: &Path, code: i32) -> (PathBuf, PathBuf) { + let log = dir.join("apply.log"); + std::fs::write(&log, "").unwrap(); + let bin = dir.join("fake-socket-patch"); + std::fs::write( + &bin, + format!( + "#!/bin/sh\nprintf 'APPLY-CALLED %s\\n' \"$*\" >> '{}'\nexit {code}\n", + log.display() + ), + ) + .unwrap(); + std::fs::set_permissions(&bin, std::fs::Permissions::from_mode(0o755)).unwrap(); + (bin, log) + } + + fn apply_calls(log: &Path) -> Vec { + std::fs::read_to_string(log) + .unwrap_or_default() + .lines() + .map(str::to_string) + .collect() + } + + /// Scaffold a setup-wired project the way a fresh clone sees it: a + /// Gemfile, a committed manifest, and the plugin generated by the REAL + /// binary. A zero-dependency Gemfile keeps the install offline — bundler + /// still registers the plugin and fires `after-install-all` (verified on + /// bundler 2.7 and 4.0.15), which is all these guards need. + fn scaffold(root: &Path) { + std::fs::write(root.join("Gemfile"), "# no dependencies\n").unwrap(); + std::fs::create_dir_all(root.join(".socket")).unwrap(); + std::fs::write(root.join(".socket/manifest.json"), MANIFEST).unwrap(); + + let mut cmd = Command::new(binary()); + cmd.args(["setup", "--yes", "--json"]).current_dir(root); + scrub(&mut cmd); + let (code, out, err) = run(cmd); + assert_eq!(code, 0, "setup --yes must wire the plugin.\n{out}\n{err}"); + assert!( + root.join(".socket/bundler-plugin/plugins.rb").exists(), + "setup must generate plugins.rb" + ); + } + + /// [P0 bootstrap deadlock] On a fresh clone of a setup-wired project the + /// FIRST `bundle install` evaluates plugins.rb at plugin REGISTRATION, + /// before any project gem lands; apply legitimately finds nothing and + /// exits 1. The old plugin raised Bundler::BundlerError there, so that + /// first install (and every retry) died with "Failed to install plugin" + /// — reproduced at exit 29 under bundler 4.0.15 / exit 1 under 2.7. The + /// generated plugin must instead warn (with the manual remediation) and + /// let the install succeed. + #[test] + fn first_bundle_install_survives_failing_apply() { + if !have("bundle") { + eprintln!("skip plugin_runtime: bundler not on PATH"); + return; + } + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + scaffold(root); + let (fake, log) = write_fake_apply(root, 1); + + let (code, out, err) = bundle_install(root, &fake, &[]); + assert_eq!( + code, 0, + "the FIRST bundle install of a setup-wired fresh checkout must \ + succeed even when apply fails (bootstrap deadlock).\n{out}\n{err}" + ); + // Anti-vacuity: the failure was real — the plugin DID shell apply. + let calls = apply_calls(&log); + assert!( + calls + .iter() + .any(|c| c.contains("apply --ecosystems gem --offline --silent")), + "the plugin must have invoked the (failing) gem-scoped apply:\n{calls:?}" + ); + // The warning names what failed and how to remediate. + assert!( + err.contains("socket-patch:"), + "a failing apply must be surfaced on stderr:\n{err}" + ); + assert!( + err.contains("socket-patch apply --ecosystems gem"), + "the warning must name the manual remediation command:\n{err}" + ); + assert!( + err.contains("SOCKET_PATCH_STRICT"), + "the warning must mention the strict escape hatch:\n{err}" + ); + + // A retry is not poisoned either (the old failure mode repeated + // identically forever because plugin registration never completed). + let (code, out, err) = bundle_install(root, &fake, &[]); + assert_eq!( + code, 0, + "retried bundle install must also succeed.\n{out}\n{err}" + ); + } + + /// SOCKET_PATCH_STRICT=1 restores raise-on-failure for builds that must + /// not proceed with unpatched gems: the same failing-apply install must + /// break the build again. + #[test] + fn strict_mode_fails_bundle_install_on_apply_failure() { + if !have("bundle") { + eprintln!("skip plugin_runtime: bundler not on PATH"); + return; + } + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + scaffold(root); + let (fake, log) = write_fake_apply(root, 1); + + let (code, out, err) = bundle_install(root, &fake, &[("SOCKET_PATCH_STRICT", "1")]); + assert_ne!( + code, 0, + "strict mode must fail the build on a patch failure.\n{out}\n{err}" + ); + assert!( + !apply_calls(&log).is_empty(), + "the strict failure must come from a real apply invocation" + ); + assert!( + err.contains("socket-patch"), + "the strict failure must carry the socket-patch message:\n{err}" + ); + } + + /// [P2 stamp location] A successful apply stamps the PROJECT + /// (.socket/gem-plugin-stamp), not a fixed-name file under the bundle + /// path (machine-global when no path is configured — shared and clobbered + /// across every socket-patch project on the host). And the digest gate + /// holds: a second, fully-cached install runs exactly one more (forced + /// after-install-all) apply — the gated triggers stay quiet. + #[test] + fn successful_apply_stamps_project_scoped() { + if !have("bundle") { + eprintln!("skip plugin_runtime: bundler not on PATH"); + return; + } + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + scaffold(root); + let (fake, log) = write_fake_apply(root, 0); + + let (code, out, err) = bundle_install(root, &fake, &[]); + assert_eq!(code, 0, "install must succeed.\n{out}\n{err}"); + assert!( + !err.contains("socket-patch:"), + "a successful apply must not warn:\n{err}" + ); + + let stamp = root.join(STAMP_REL); + assert!( + stamp.is_file(), + "the digest stamp must land at the project-scoped {STAMP_REL}" + ); + let content = std::fs::read_to_string(&stamp).unwrap(); + let content = content.trim(); + assert!( + content.len() == 64 && content.bytes().all(|b| b.is_ascii_hexdigit()), + "the stamp must hold one sha256 hex digest, got: {content:?}" + ); + // No legacy fixed-name stamp anywhere under the bundle path. + let legacy_hits: Vec<_> = walk(&root.join("vendor")) + .into_iter() + .filter(|p| p.file_name().is_some_and(|n| n == LEGACY_STAMP_NAME)) + .collect(); + assert!( + legacy_hits.is_empty(), + "no legacy bundle-path stamp may be written: {legacy_hits:?}" + ); + + let after_first = apply_calls(&log).len(); + let (code, out, err) = bundle_install(root, &fake, &[]); + assert_eq!(code, 0, "cached install must succeed.\n{out}\n{err}"); + assert_eq!( + apply_calls(&log).len(), + after_first + 1, + "a fully-cached install runs exactly the one forced \ + after-install-all apply; the digest-gated triggers must not \ + shell out again" + ); + } + + /// [P1 digest honesty + migration] Drive the applier directly with plain + /// ruby (no bundler process, no network): the digest stamp must reflect + /// the ACTUAL on-disk gem-file state, so an out-of-band reversion + /// (`bundle pristine`, `gem pristine`, a manual edit) flips the digest + /// and the next trigger re-applies; and the legacy machine-global stamp + /// must be cleaned up, never read. + #[test] + fn digest_tracks_gem_file_content_and_legacy_stamp_is_removed() { + if !have("ruby") { + eprintln!("skip plugin_runtime: ruby not on PATH"); + return; + } + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + scaffold(root); + let (fake, log) = write_fake_apply(root, 0); + + // Where bundler would install this project's gems (BUNDLE_PATH set, + // so /vendor/bundle/ruby/). + let mut cmd = Command::new("ruby"); + cmd.args(["-e", "require \"bundler\"; print Bundler.bundle_path"]) + .current_dir(root); + scrub(&mut cmd); + cmd.env("BUNDLE_PATH", "vendor/bundle"); + let (code, bundle_path, err) = run(cmd); + assert_eq!(code, 0, "Bundler.bundle_path probe failed: {err}"); + let bundle_path = PathBuf::from(bundle_path.trim()); + + // The installed file the manifest's patch record targets. + let target = bundle_path.join("gems/colorize-1.1.0/lib/colorize.rb"); + std::fs::create_dir_all(target.parent().unwrap()).unwrap(); + std::fs::write(&target, "UPSTREAM CONTENT\n").unwrap(); + // A stale stamp from the old plugin version in the shared location. + let legacy = bundle_path.join(LEGACY_STAMP_NAME); + std::fs::write(&legacy, "stale digest from another project\n").unwrap(); + + let drive = |label: &str| { + let mut cmd = Command::new("ruby"); + cmd.args([ + "-e", + "require \"bundler\"; load ARGV[0]; SocketPatch.apply!", + "--", + ]) + .arg(root.join(".socket/bundler-plugin/plugins.rb")) + .current_dir(root); + scrub(&mut cmd); + cmd.env("BUNDLE_PATH", "vendor/bundle"); + cmd.env("SOCKET_PATCH_BIN", &fake); + let (code, out, err) = run(cmd); + assert_eq!( + code, 0, + "{label}: driving the applier failed.\n{out}\n{err}" + ); + }; + + drive("initial apply"); + assert_eq!( + apply_calls(&log).len(), + 1, + "first drive must shell apply (nothing stamped yet)" + ); + assert!(root.join(STAMP_REL).is_file(), "stamp written"); + assert!( + !legacy.exists(), + "the legacy bundle-path stamp must be deleted on the first run" + ); + + drive("stamped no-op"); + assert_eq!( + apply_calls(&log).len(), + 1, + "unchanged state must be digest-gated to a no-op" + ); + + // Out-of-band reversion: the committed inputs (manifest, blobs, lock) + // are untouched — only the installed gem file changed back. + std::fs::write(&target, "REVERTED BY PRISTINE\n").unwrap(); + drive("after reversion"); + assert_eq!( + apply_calls(&log).len(), + 2, + "reverting the installed gem file must flip the digest and \ + re-run apply — a manifest-only digest misses this" + ); + } + + fn walk(dir: &Path) -> Vec { + let mut out = Vec::new(); + let Ok(entries) = std::fs::read_dir(dir) else { + return out; + }; + for entry in entries.flatten() { + let p = entry.path(); + if p.is_dir() { + out.extend(walk(&p)); + } else { + out.push(p); + } + } + out + } +} + +// ───────────────────────────────────────────────────────────────────────── +// Guards for the RubyGems CLI launcher (gem/socket-patch), driven with the +// host ruby. Ruby-gated with a loud skip, like `plugin_runtime` above. +// ───────────────────────────────────────────────────────────────────────── +#[cfg(unix)] +mod launcher_guard { + use std::os::unix::fs::PermissionsExt; + use std::path::{Path, PathBuf}; + use std::process::Command; + + fn have_ruby() -> bool { + Command::new("ruby") + .arg("--version") + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .map(|s| s.success()) + .unwrap_or(false) + } + + /// The launcher under test, resolved from the workspace checkout. + fn launcher_path() -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("../../gem/socket-patch/lib/socket_patch/launcher.rb") + .canonicalize() + .expect("launcher.rb must exist in the workspace") + } + + /// Run `script` (which `load`s the launcher via the SP_LAUNCHER env) with + /// a scratch HOME/cache so no real launcher cache is consulted. + fn run_ruby(script: &Path, cache: &Path, envs: &[(&str, &str)]) -> (i32, String, String) { + let mut cmd = Command::new("ruby"); + cmd.arg(script); + cmd.env_remove("SOCKET_PATCH_BIN"); + cmd.env("SP_LAUNCHER", launcher_path()); + cmd.env("XDG_CACHE_HOME", cache); + for (k, v) in envs { + cmd.env(k, v); + } + let out = cmd.output().expect("spawn ruby"); + ( + out.status.code().unwrap_or(-1), + String::from_utf8_lossy(&out.stdout).to_string(), + String::from_utf8_lossy(&out.stderr).to_string(), + ) + } + + fn write_executable(path: &Path, body: &str) { + std::fs::write(path, body).unwrap(); + std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o755)).unwrap(); + } + + /// [P2a] The Windows arm must propagate the child's REAL exit code — the + /// old `system(...) ? $?.exitstatus : 1` collapsed every non-zero exit to + /// 1, erasing meaningful codes like `setup --check`'s needs-configuration + /// signal. Forced onto the Windows branch by stubbing `Gem.win_platform?`. + #[test] + fn windows_branch_propagates_child_exit_code() { + if !have_ruby() { + eprintln!("skip launcher_guard: ruby not on PATH"); + return; + } + let tmp = tempfile::tempdir().unwrap(); + let child = tmp.path().join("exit7"); + write_executable(&child, "#!/bin/sh\nexit 7\n"); + let script = tmp.path().join("drive.rb"); + std::fs::write( + &script, + "require \"rubygems\"\n\ + def Gem.win_platform?; true; end\n\ + load ENV.fetch(\"SP_LAUNCHER\")\n\ + SocketPatch::Launcher.run([\"anything\"])\n", + ) + .unwrap(); + + let (code, out, err) = run_ruby( + &script, + &tmp.path().join("cache"), + &[("SOCKET_PATCH_BIN", child.to_str().unwrap())], + ); + assert_eq!( + code, 7, + "the child's exit 7 must survive the spawn+wait arm, not \ + collapse to 1.\nstdout:\n{out}\nstderr:\n{err}" + ); + } + + /// [nit] First-run failures outside LauncherError (DNS outages, TLS + /// errors, ...) must exit with a clean one-line message, not a raw ruby + /// backtrace. + #[test] + fn unexpected_download_errors_exit_cleanly() { + if !have_ruby() { + eprintln!("skip launcher_guard: ruby not on PATH"); + return; + } + let tmp = tempfile::tempdir().unwrap(); + let script = tmp.path().join("drive.rb"); + std::fs::write( + &script, + "require \"net/http\"\n\ + def (Net::HTTP).start(*args, &block)\n\ + raise SocketError, \"simulated dns failure\"\n\ + end\n\ + load ENV.fetch(\"SP_LAUNCHER\")\n\ + SocketPatch::Launcher.run([\"--version\"])\n", + ) + .unwrap(); + + let (code, out, err) = run_ruby(&script, &tmp.path().join("cache"), &[]); + assert_eq!(code, 1, "a download failure exits 1.\n{out}\n{err}"); + assert!( + err.contains("socket-patch:") && err.contains("SocketError"), + "the failure must be reported as a clean launcher message:\n{err}" + ); + assert!( + !err.contains("launcher.rb:"), + "no raw backtrace frames may escape to the user:\n{err}" + ); + } + + /// [nit] PowerShell quoting: a path containing a single quote must be + /// escaped by doubling it, or the Expand-Archive fallback command breaks. + #[test] + fn powershell_quote_doubles_single_quotes() { + if !have_ruby() { + eprintln!("skip launcher_guard: ruby not on PATH"); + return; + } + let tmp = tempfile::tempdir().unwrap(); + let script = tmp.path().join("drive.rb"); + std::fs::write( + &script, + "load ENV.fetch(\"SP_LAUNCHER\")\n\ + print SocketPatch::Launcher.powershell_quote(\"C:/it's a dir/x.zip\")\n", + ) + .unwrap(); + + let (code, out, err) = run_ruby(&script, &tmp.path().join("cache"), &[]); + assert_eq!(code, 0, "quoting helper must exist and run.\n{err}"); + assert_eq!( + out, "'C:/it''s a dir/x.zip'", + "single quotes must be doubled inside the single-quoted literal" + ); + // And the Expand-Archive fallback actually routes through it. + let launcher = std::fs::read_to_string(launcher_path()).unwrap(); + assert!( + launcher.contains("-LiteralPath #{powershell_quote(archive_path)}") + && launcher.contains("-DestinationPath #{powershell_quote(dir)}"), + "extract's PowerShell fallback must quote both paths via the helper" + ); + } + + /// [P2b] The binary-cache install must be atomic: staged in the + /// destination dir and renamed into place, leaving no temp litter — a + /// concurrent first run can then never exec a torn or not-yet-chmodded + /// binary. (The rename mechanism itself is pinned by inspection since a + /// mid-write race cannot be scheduled deterministically from a test.) + #[test] + fn install_executable_is_atomic_into_place() { + if !have_ruby() { + eprintln!("skip launcher_guard: ruby not on PATH"); + return; + } + let tmp = tempfile::tempdir().unwrap(); + let src = tmp.path().join("extracted-binary"); + std::fs::write(&src, "BINARY CONTENT\n").unwrap(); + let dest = tmp.path().join("cache/1.0.0/target/socket-patch"); + let script = tmp.path().join("drive.rb"); + std::fs::write( + &script, + "load ENV.fetch(\"SP_LAUNCHER\")\n\ + SocketPatch::Launcher.install_executable(ARGV[0], ARGV[1])\n", + ) + .unwrap(); + + let mut cmd = Command::new("ruby"); + cmd.arg(&script).arg(&src).arg(&dest); + cmd.env("SP_LAUNCHER", launcher_path()); + let out = cmd.output().expect("spawn ruby"); + assert!( + out.status.success(), + "install_executable must exist and succeed:\n{}", + String::from_utf8_lossy(&out.stderr) + ); + assert_eq!( + std::fs::read_to_string(&dest).unwrap(), + "BINARY CONTENT\n", + "the cached binary must be byte-identical to the extracted one" + ); + let mode = std::fs::metadata(&dest).unwrap().permissions().mode(); + assert_ne!(mode & 0o111, 0, "the cached binary must be executable"); + let litter: Vec<_> = std::fs::read_dir(dest.parent().unwrap()) + .unwrap() + .flatten() + .map(|e| e.file_name().to_string_lossy().into_owned()) + .filter(|n| n != "socket-patch") + .collect(); + assert!( + litter.is_empty(), + "no staging temp files may be left behind: {litter:?}" + ); + let launcher = std::fs::read_to_string(launcher_path()).unwrap(); + assert!( + launcher.contains("File.rename(tmp, dest)"), + "the cache publish must go through a same-dir rename" + ); + } +} diff --git a/crates/socket-patch-core/src/setup/gem/mod.rs b/crates/socket-patch-core/src/setup/gem/mod.rs index c4cbf8e1..563eb375 100644 --- a/crates/socket-patch-core/src/setup/gem/mod.rs +++ b/crates/socket-patch-core/src/setup/gem/mod.rs @@ -1,15 +1,20 @@ //! Gem (Bundler) `setup` support: wire a Ruby project for automatic patching. //! -//! Bundler has no after-each-install hook that survives a cached/no-op -//! `bundle install`, but it loads any declared **plugin** during the Gemfile -//! pass on every `bundle` invocation. So setup delivers the gate as a -//! generated, git-committed Bundler plugin plus a `plugin` directive in the -//! Gemfile: +//! Bundler loads a declared **plugin** whenever one of its subscribed hook +//! events fires — on every `bundle install`, fresh AND fully cached (verified +//! against bundler 2.7 and 4.0). So setup delivers the gate as a generated, +//! git-committed Bundler plugin plus a `plugin` directive in the Gemfile: //! //! * `.socket/bundler-plugin/{plugins.rb, socket-patch.gemspec}` — a generated //! plugin whose `plugins.rb` re-runs `socket-patch apply --ecosystems gem` -//! on every `bundle install` (load-time digest gate + `after-install-all` -//! hook), failing the build loudly on a patch failure; +//! on every `bundle install` (digest-gated load-time + per-gem +//! `after-install` triggers, forced `after-install-all` re-apply). A patch +//! failure warns with a remediation and lets the install continue — +//! bundler evaluates `plugins.rb` at plugin REGISTRATION, before any +//! project gem is installed, so raising there would deadlock a fresh +//! clone on its own first `bundle install` (plugin registration fails and +//! every retry fails identically). `SOCKET_PATCH_STRICT=1` restores +//! raise-on-failure (`Bundler::BundlerError`); //! * a managed block appended to the `Gemfile` that references the plugin via //! `plugin "socket-patch", path: File.expand_path(".socket/bundler-plugin", //! __dir__)`. The source must be `path:` — Bundler fetches `git:` plugin @@ -405,21 +410,22 @@ mod tests { #[test] fn test_templates_are_well_formed() { - // The plugin must carry the ownership marker and both triggers. + // The plugin must carry the ownership marker and all three triggers. assert!(PLUGINS_RB.starts_with(GENERATED_MARKER)); assert!(PLUGINS_RB.contains("def apply!")); - // Load-time trigger + after-install-all hook. + // Load-time trigger + the per-gem after-install hook (the only event + // bundler fires during `bundle pristine`) + after-install-all hook. assert!(PLUGINS_RB.contains("SocketPatch.apply!")); + assert!(PLUGINS_RB.contains("Bundler::Plugin.add_hook(\"after-install\")")); assert!(PLUGINS_RB.contains("Bundler::Plugin.add_hook(\"after-install-all\")")); - // The applier shells the gem-scoped offline apply and fails loud. + // The applier shells the gem-scoped offline apply. assert!(PLUGINS_RB.contains("\"apply\"")); assert!(PLUGINS_RB.contains("\"--ecosystems\", \"gem\", \"--offline\"")); - assert!(PLUGINS_RB.contains("BundlerError")); - // Stamp travels with the gems (under Bundler.bundle_path). - assert!(PLUGINS_RB.contains("Bundler.bundle_path")); - // Digest folds in Gemfile.lock + the manifest. + // Digest folds in Gemfile.lock + the manifest + the on-disk state of + // the patch target files (so an out-of-band reversion is detected). assert!(PLUGINS_RB.contains("Gemfile.lock")); assert!(PLUGINS_RB.contains("manifest.json")); + assert!(PLUGINS_RB.contains("def patch_target_files")); // The gemspec names the plugin the Gemfile directive references. assert!(GEMSPEC.starts_with(GENERATED_MARKER)); assert!(GEMSPEC.contains("\"socket-patch\"")); @@ -430,6 +436,68 @@ mod tests { assert!(GEMSPEC.contains("s.require_paths = [\".\"]")); } + #[test] + fn test_plugin_template_failure_policy_and_stamp_location() { + // Failure policy: tolerant by default — a patch failure WARNS (with + // the manual-apply remediation) and lets `bundle install` continue. + // Bundler evaluates plugins.rb at plugin REGISTRATION, before any + // project gem is installed; a raise there deadlocks a fresh clone on + // its own first `bundle install` (plugin registration fails, every + // retry fails identically — reproduced against bundler 2.7 and 4.0). + assert!( + PLUGINS_RB.contains("def report_failure"), + "the applier must route failures through the tolerant reporter" + ); + assert!( + !PLUGINS_RB.contains("def fail!"), + "the unconditional raise helper must be gone — it is what \ + deadlocked bootstrap installs" + ); + assert!( + PLUGINS_RB.contains("warn(message)"), + "tolerant mode must surface the failure as a stderr warning" + ); + assert!( + PLUGINS_RB.contains("socket-patch apply --ecosystems gem"), + "the warning must name the manual remediation command" + ); + // Strict escape hatch: SOCKET_PATCH_STRICT=1 restores raise-on-failure. + assert!(PLUGINS_RB.contains("SOCKET_PATCH_STRICT")); + assert!( + PLUGINS_RB.contains("BundlerError"), + "strict mode must still raise Bundler::BundlerError" + ); + // Stamp location: project-scoped under .socket/, NOT a fixed-name file + // in Bundler.bundle_path (machine-global with no bundle path + // configured, shared and clobbered across every project on the host). + assert!(PLUGINS_RB.contains("STAMP_NAME = \"gem-plugin-stamp\"")); + assert!(PLUGINS_RB.contains("File.join(socket_dir, STAMP_NAME)")); + // The legacy global stamp is cleaned up, never read. + assert!(PLUGINS_RB.contains("LEGACY_STAMP_NAME = \".socket-patch-gem-stamp\"")); + assert!(PLUGINS_RB.contains("def remove_legacy_stamp")); + // The stamp must be excluded from its own digest inputs, or every + // write would invalidate the digest it records. + assert!(PLUGINS_RB.contains("p != stamp_path")); + + // The published-gem twin must carry the same applier contract. + let published = include_str!("../../../../../gem/socket-patch-bundler/plugins.rb"); + for needle in [ + "def report_failure", + "SOCKET_PATCH_STRICT", + "STAMP_NAME = \"gem-plugin-stamp\"", + "def remove_legacy_stamp", + "def patch_target_files", + "Bundler::Plugin.add_hook(\"after-install\")", + "Bundler::Plugin.add_hook(\"after-install-all\")", + ] { + assert!( + published.contains(needle), + "published plugins.rb drifted from the template: missing {needle:?}" + ); + } + assert!(!published.contains("def fail!")); + } + #[tokio::test] async fn test_add_then_remove_plugin_files_roundtrip() { let dir = tempfile::tempdir().unwrap(); diff --git a/crates/socket-patch-core/src/setup/gem/templates/plugins.rb.tmpl b/crates/socket-patch-core/src/setup/gem/templates/plugins.rb.tmpl index 9103b26f..0c39c30b 100644 --- a/crates/socket-patch-core/src/setup/gem/templates/plugins.rb.tmpl +++ b/crates/socket-patch-core/src/setup/gem/templates/plugins.rb.tmpl @@ -1,39 +1,65 @@ # Code generated by `socket-patch setup`. DO NOT EDIT. # # socket-patch Bundler plugin. Keeps the gem patches recorded in -# .socket/manifest.json applied on every `bundle install` — including a -# cached/no-op install that restores a previously-installed gem set — by -# re-running the socket-patch CLI. Without it, `bundle install` reinstalls a gem +# .socket/manifest.json applied by re-running the socket-patch CLI whenever +# Bundler touches the gem set. Without it, `bundle install` reinstalls a gem # from its cached .gem and silently reverts any applied patch. # -# Two complementary triggers feed one idempotent applier: -# * load-time — this file is evaluated during Bundler's Gemfile pass on EVERY -# `bundle` invocation, even when no gem needs installing, so it covers the -# cached/no-op install the after-install-all hook would miss; -# * the `after-install-all` hook — fires after the installer finishes, so it -# covers the fresh install where gems exist only afterwards (at load time -# there was nothing on disk to patch yet). +# When each trigger actually runs (verified against bundler 2.7 and 4.0; hook +# subscriptions are recorded in .bundle/plugin/index at plugin REGISTRATION, +# and bundler evaluates this file whenever a subscribed event first fires in a +# bundle process): # -# A digest of (manifest + every committed patch file under .socket/ + -# Gemfile.lock) gates the load-time work: identical to the last applied state -> -# fast exit; otherwise shell out and re-stamp. Folding Gemfile.lock into the -# digest forces a reapply when a version bump reinstalls a gem and wipes its -# patch even though the manifest is byte-identical. The stamp lives under -# Bundler.bundle_path so it travels WITH the gems: a cached gem dir carries the -# stamp alongside the patched gems (stays in sync); a wiped vendor/bundle drops -# the stamp too, so patches reapply. +# * plugin registration — the FIRST `bundle install` after `setup` (or on a +# fresh clone) evaluates this file BEFORE any project gem is installed, so +# the load-time trigger is stamp-gated and quietly no-ops there; the +# install hooks below re-apply once the gems land. +# * every `bundle install` — fresh AND fully cached — fires the per-gem +# `after-install` events and then `after-install-all`; the forced +# `after-install-all` re-apply is the actual patch point. +# * `bundle pristine` fires ONLY the per-gem `after-install` events, so that +# digest-gated hook is what catches pristine's patch reversion in the same +# run. (A checkout registered by an older plugin version keeps its old +# subscription set until it re-registers — fresh clones and CI always +# re-register, a dev checkout can `rm -rf .bundle/plugin`.) +# * nothing fires on `bundle exec` / `bundle check` / plain `ruby`, and +# `gem pristine` bypasses bundler entirely — a reversion via those is only +# healed at the NEXT `bundle install`. # -# On any patch failure it raises Bundler::BundlerError so the build breaks -# loudly rather than proceeding with stale/unpatched gems. The socket-patch CLI -# must be on PATH (or pointed at by SOCKET_PATCH_BIN) wherever `bundle install` -# runs — the same requirement as the cargo build-script guard. +# A digest of (manifest + every committed file under .socket/ + Gemfile.lock + +# the on-disk content of every gem-patch target file) gates the non-forced +# triggers: identical to the last applied state -> fast exit; otherwise shell +# out and re-stamp. Folding the targets' actual content in means an +# out-of-band reversion (pristine, manual edit, wiped bundle path) flips the +# digest even when every committed input is byte-identical. The stamp lives at +# .socket/gem-plugin-stamp — project-local state, safe to gitignore or delete +# (deleting only forces one re-probe). Older plugin versions stamped a +# fixed-name file under Bundler.bundle_path — the machine-global gem dir when +# no bundle path is configured, shared and clobbered across every project on +# the host — so that legacy stamp is deleted best-effort when seen. +# +# A patch failure NEVER breaks `bundle install`: it prints a warning naming +# what failed and the remediation (run `socket-patch apply --ecosystems gem` +# manually). Set SOCKET_PATCH_STRICT=1 to restore raise-on-failure +# (Bundler::BundlerError) for builds that must not proceed with unpatched +# gems. The socket-patch CLI must be on PATH (or pointed at by +# SOCKET_PATCH_BIN) wherever `bundle install` runs. require "digest" require "fileutils" +require "json" module SocketPatch - BIN_ENV = "SOCKET_PATCH_BIN".freeze - STAMP_NAME = ".socket-patch-gem-stamp".freeze + # Bundler evaluates this file twice in a bootstrap install (registration + + # first hook load), so constant assignments are guarded against re-runs. + BIN_ENV = "SOCKET_PATCH_BIN".freeze unless defined?(BIN_ENV) + STRICT_ENV = "SOCKET_PATCH_STRICT".freeze unless defined?(STRICT_ENV) + STAMP_NAME = "gem-plugin-stamp".freeze unless defined?(STAMP_NAME) + LEGACY_STAMP_NAME = ".socket-patch-gem-stamp".freeze unless defined?(LEGACY_STAMP_NAME) + # Bundler's parallel installer can fire per-gem hooks from worker threads; + # one applier runs at a time so a single bundle process never races + # concurrent `socket-patch apply` children against each other. + APPLY_LOCK = Mutex.new unless defined?(APPLY_LOCK) module_function @@ -43,8 +69,12 @@ module SocketPatch File.expand_path("../..", __dir__) end + def socket_dir + File.join(project_root, ".socket") + end + def manifest_path - File.join(project_root, ".socket", "manifest.json") + File.join(socket_dir, "manifest.json") end def socket_bin @@ -52,18 +82,63 @@ module SocketPatch env && !env.empty? ? env : "socket-patch" end + def strict? + %w[1 true].include?(ENV[STRICT_ENV].to_s) + end + + def bundle_path + Bundler.bundle_path.to_s + rescue StandardError + File.join(project_root, "vendor", "bundle") + end + + def stamp_path + File.join(socket_dir, STAMP_NAME) + end + + # The on-disk files the manifest's gem patches target: + # /gems/-[-]/. + # Paths are collected whether or not the file exists — `current_digest` + # folds an absence marker, so a gem appearing or vanishing flips the digest. + def patch_target_files + records = begin + JSON.parse(File.read(manifest_path)).fetch("patches", {}) + rescue StandardError + return [] + end + return [] unless records.is_a?(Hash) + gems_dir = File.join(bundle_path, "gems") + targets = [] + records.each do |purl, record| + next unless purl.is_a?(String) && purl.start_with?("pkg:gem/") + coordinate = purl.split("pkg:gem/", 2).last.split("?", 2).first + name, at, version = coordinate.rpartition("@") + next if at.empty? || name.empty? || version.empty? + files = record.is_a?(Hash) ? record["files"] : nil + next unless files.is_a?(Hash) + files.each_key do |key| + rel = key.to_s.sub(%r{\Apackage/}, "") + targets << File.join(gems_dir, "#{name}-#{version}", rel) + targets.concat(Dir.glob(File.join(gems_dir, "#{name}-#{version}-*", rel))) + end + end + targets.uniq.sort + end + # Files whose change must force a reapply: the manifest, every committed file - # under .socket/ (patch blobs etc.), and Gemfile.lock. + # under .socket/ (patch blobs etc. — the stamp itself excluded, or each write + # would invalidate the digest it records), Gemfile.lock, and the current + # on-disk state of every patch target. def digest_inputs inputs = [manifest_path] lock = File.join(project_root, "Gemfile.lock") inputs << lock if File.file?(lock) - socket_dir = File.join(project_root, ".socket") if File.directory?(socket_dir) Dir.glob(File.join(socket_dir, "**", "*")).sort.each do |p| - inputs << p if File.file?(p) + inputs << p if File.file?(p) && p != stamp_path end end + inputs.concat(patch_target_files) inputs.uniq end @@ -72,27 +147,18 @@ module SocketPatch digest_inputs.each do |path| d.update(path) d.update("\0") + d.update(File.file?(path) ? "+" : "-") begin d.update(File.binread(path)) rescue StandardError - # Unreadable now -> contributes only its path; a later readable state - # changes the digest and forces a reapply. + # Unreadable now -> contributes only its path + absence marker; a later + # readable state changes the digest and forces a reapply. end d.update("\0") end d.hexdigest end - def bundle_path - Bundler.bundle_path.to_s - rescue StandardError - File.join(project_root, "vendor", "bundle") - end - - def stamp_path - File.join(bundle_path, STAMP_NAME) - end - def stamped?(digest) File.file?(stamp_path) && File.read(stamp_path).strip == digest rescue StandardError @@ -106,56 +172,105 @@ module SocketPatch # Best-effort: a missing/unwritable stamp just means we re-probe next time. end - def fail!(message) - raise(defined?(Bundler::BundlerError) ? Bundler::BundlerError.new(message) : message) + # Older plugin versions stamped under Bundler.bundle_path. It is never read + # anymore; delete it (best-effort, once per process) so it does not linger + # as an orphan in a shared gem dir. + def remove_legacy_stamp + return if @legacy_stamp_checked + @legacy_stamp_checked = true + legacy = File.join(bundle_path, LEGACY_STAMP_NAME) + File.delete(legacy) if File.file?(legacy) + rescue StandardError + # Best-effort cleanup only. end - # Idempotent, missing-gem-tolerant. No manifest -> the project does not use - # socket-patch, nothing to do. When `force` is false the digest stamp short- - # circuits already-applied state; the after-install-all hook passes force:true - # because the installer just changed the on-disk gem set. - def apply!(force: false) - return unless File.file?(manifest_path) - - digest = current_digest - return if !force && stamped?(digest) - - ok = system( - socket_bin, "apply", - "--ecosystems", "gem", "--offline", "--silent", - "--cwd", project_root - ) - - if ok.nil? - fail!( - "socket-patch: could not run `#{socket_bin} apply` to apply gem patches; " \ - "the socket-patch CLI is required. Install it or set #{BIN_ENV} to its path." - ) - elsif !ok - fail!( - "socket-patch: `#{socket_bin} apply --ecosystems gem` failed; the gem patches " \ - "in .socket/manifest.json are NOT applied. The build was failed to avoid " \ - "shipping unpatched gems." - ) + # Tolerant by default: a patch failure must never break `bundle install` — + # the first install of a fresh checkout runs the applier before any project + # gem exists, and raising there deadlocks the project on its own bootstrap + # (plugin registration fails, so every retry fails identically). Warn once + # per process with the remediation; SOCKET_PATCH_STRICT=1 restores the raise + # for builds that must not proceed unpatched. + def report_failure(message) + if strict? + raise(defined?(Bundler::BundlerError) ? Bundler::BundlerError.new(message) : message) end + return if @warned + @warned = true + warn(message) + end + + # Idempotent applier behind every trigger. No manifest -> the project does + # not use socket-patch, nothing to do. + # force: skip the digest gate (the installer just changed the gem set). + # require_stamp: bail unless a previous apply stamped this project — the + # load-time trigger and the per-gem after-install hook use it so a + # bootstrap install's early evaluations don't shell out (and warn) before + # the target gems even exist; the forced after-install-all pass does the + # first real apply and lays down the stamp. + def apply!(force: false, require_stamp: false) + APPLY_LOCK.synchronize do + return unless File.file?(manifest_path) + remove_legacy_stamp + return if require_stamp && !File.file?(stamp_path) + return if !force && stamped?(current_digest) + + ok = system( + socket_bin, "apply", + "--ecosystems", "gem", "--offline", "--silent", + "--cwd", project_root + ) + + if ok.nil? + report_failure( + "socket-patch: could not run `#{socket_bin} apply` — the gem patches in " \ + ".socket/manifest.json are NOT applied. Install the socket-patch CLI (or set " \ + "#{BIN_ENV} to its path), then run `socket-patch apply --ecosystems gem` " \ + "manually. `bundle install` continues; set #{STRICT_ENV}=1 to make this fatal." + ) + return + elsif !ok + report_failure( + "socket-patch: `#{socket_bin} apply --ecosystems gem` failed — the gem patches " \ + "in .socket/manifest.json may NOT be applied. Run `socket-patch apply " \ + "--ecosystems gem` in #{project_root} to apply them manually. " \ + "`bundle install` continues; set #{STRICT_ENV}=1 to make patch failures fatal." + ) + return + end - write_stamp(digest) + if @warned + @warned = false + warn("socket-patch: gem patches applied; the earlier warning is resolved.") + end + # Recompute: the apply just rewrote the target files the digest folds in. + write_stamp(current_digest) + end end end -# Trigger 1 — load-time (covers the cached/no-op `bundle install`). On a fresh -# install the gems are not on disk yet; `apply!` is a tolerant no-op there and -# Trigger 2 does the real work once they exist. A genuine patch failure +# Trigger 1 — load time. Runs at plugin registration and whenever a subscribed +# hook event first loads the plugin in a bundle process. Stamp-gated: on the +# bootstrap install nothing is stamped yet (and no gems exist to patch), so +# this quietly defers to Trigger 3. In strict mode a genuine patch failure # (Bundler::BundlerError) still propagates. begin - SocketPatch.apply! + SocketPatch.apply!(require_stamp: true) rescue StandardError => e raise if defined?(Bundler::BundlerError) && e.is_a?(Bundler::BundlerError) end -# Trigger 2 — after the installer finishes (covers the fresh install). Forced, -# because the install just changed the gem set; the applier is idempotent so a -# redundant run on an already-patched tree is a cheap no-op. +# Trigger 2 — after each individual gem (re)install. The only event bundler +# fires during `bundle pristine`, so this is what catches pristine's patch +# reversion in the same run. Digest- and stamp-gated: on a fresh install's +# per-gem events the project is not stamped yet and Trigger 3 is about to do +# the real work. +Bundler::Plugin.add_hook("after-install") do |_spec_install| + SocketPatch.apply!(require_stamp: true) +end + +# Trigger 3 — after the installer finishes (fresh AND fully-cached installs). +# Forced, because the install just changed the gem set; the applier is +# idempotent so a redundant run on an already-patched tree is a cheap no-op. Bundler::Plugin.add_hook("after-install-all") do |_install| SocketPatch.apply!(force: true) end diff --git a/gem/socket-patch-bundler/plugins.rb b/gem/socket-patch-bundler/plugins.rb index f2d65fcd..a11c87c9 100644 --- a/gem/socket-patch-bundler/plugins.rb +++ b/gem/socket-patch-bundler/plugins.rb @@ -1,32 +1,61 @@ # socket-patch Bundler plugin (published-gem form). # -# Keeps the gem patches recorded in .socket/manifest.json applied on every -# `bundle install` — including a cached/no-op install — by re-running the -# socket-patch CLI. This is the Phase-2 published-gem counterpart of the in-tree -# plugin generated by `socket-patch setup` under .socket/bundler-plugin/; the -# applier logic is identical, but because a published plugin is loaded from the -# gem cache (not from inside the repo) it resolves the project root from the -# bundle context rather than relative to its own location. +# Keeps the gem patches recorded in .socket/manifest.json applied by +# re-running the socket-patch CLI whenever Bundler touches the gem set. This +# is the Phase-2 published-gem counterpart of the in-tree plugin generated by +# `socket-patch setup` under .socket/bundler-plugin/; the applier logic is +# identical, but because a published plugin is loaded from the gem cache (not +# from inside the repo) it resolves the project root from the bundle context +# rather than relative to its own location. # -# Two complementary triggers feed one idempotent applier: -# * load-time — evaluated during Bundler's Gemfile pass on EVERY `bundle` -# invocation, covering the cached/no-op install the after-install-all hook -# would miss; -# * the `after-install-all` hook — fires after the installer finishes, -# covering the fresh install where gems exist only afterwards. +# When each trigger actually runs (verified against bundler 2.7 and 4.0; hook +# subscriptions are recorded in .bundle/plugin/index at plugin REGISTRATION, +# and bundler evaluates this file whenever a subscribed event first fires in a +# bundle process): # -# A digest of (manifest + every committed file under .socket/ + Gemfile.lock) -# gates the load-time work. The stamp lives under Bundler.bundle_path so it -# travels WITH the gems. On any patch failure it raises Bundler::BundlerError so -# the build breaks loudly rather than shipping stale/unpatched gems. The -# socket-patch CLI must be on PATH (or pointed at by SOCKET_PATCH_BIN). +# * plugin registration — the FIRST `bundle install` evaluates this file +# BEFORE any project gem is installed, so the load-time trigger is +# stamp-gated and quietly no-ops there; the install hooks below re-apply +# once the gems land. +# * every `bundle install` — fresh AND fully cached — fires the per-gem +# `after-install` events and then `after-install-all`; the forced +# `after-install-all` re-apply is the actual patch point. +# * `bundle pristine` fires ONLY the per-gem `after-install` events, so that +# digest-gated hook is what catches pristine's patch reversion in the same +# run. (A checkout registered by an older plugin version keeps its old +# subscription set until it re-registers — fresh clones and CI always +# re-register, a dev checkout can `rm -rf .bundle/plugin`.) +# * nothing fires on `bundle exec` / `bundle check` / plain `ruby`, and +# `gem pristine` bypasses bundler entirely — a reversion via those is only +# healed at the NEXT `bundle install`. +# +# A digest of (manifest + every committed file under .socket/ + Gemfile.lock + +# the on-disk content of every gem-patch target file) gates the non-forced +# triggers. The stamp lives at .socket/gem-plugin-stamp — project-local state, +# safe to gitignore or delete. Older plugin versions stamped a fixed-name file +# under Bundler.bundle_path (machine-global when no bundle path is +# configured); that legacy stamp is deleted best-effort when seen. +# +# A patch failure NEVER breaks `bundle install`: it prints a warning naming +# what failed and the remediation. Set SOCKET_PATCH_STRICT=1 to restore +# raise-on-failure (Bundler::BundlerError). The socket-patch CLI must be on +# PATH (or pointed at by SOCKET_PATCH_BIN). require "digest" require "fileutils" +require "json" module SocketPatch - BIN_ENV = "SOCKET_PATCH_BIN".freeze - STAMP_NAME = ".socket-patch-gem-stamp".freeze + # Bundler evaluates this file twice in a bootstrap install (registration + + # first hook load), so constant assignments are guarded against re-runs. + BIN_ENV = "SOCKET_PATCH_BIN".freeze unless defined?(BIN_ENV) + STRICT_ENV = "SOCKET_PATCH_STRICT".freeze unless defined?(STRICT_ENV) + STAMP_NAME = "gem-plugin-stamp".freeze unless defined?(STAMP_NAME) + LEGACY_STAMP_NAME = ".socket-patch-gem-stamp".freeze unless defined?(LEGACY_STAMP_NAME) + # Bundler's parallel installer can fire per-gem hooks from worker threads; + # one applier runs at a time so a single bundle process never races + # concurrent `socket-patch apply` children against each other. + APPLY_LOCK = Mutex.new unless defined?(APPLY_LOCK) module_function @@ -50,8 +79,12 @@ def project_root Dir.pwd end + def socket_dir + File.join(project_root, ".socket") + end + def manifest_path - File.join(project_root, ".socket", "manifest.json") + File.join(socket_dir, "manifest.json") end def socket_bin @@ -59,18 +92,63 @@ def socket_bin env && !env.empty? ? env : "socket-patch" end + def strict? + %w[1 true].include?(ENV[STRICT_ENV].to_s) + end + + def bundle_path + Bundler.bundle_path.to_s + rescue StandardError + File.join(project_root, "vendor", "bundle") + end + + def stamp_path + File.join(socket_dir, STAMP_NAME) + end + + # The on-disk files the manifest's gem patches target: + # /gems/-[-]/. + # Paths are collected whether or not the file exists — `current_digest` + # folds an absence marker, so a gem appearing or vanishing flips the digest. + def patch_target_files + records = begin + JSON.parse(File.read(manifest_path)).fetch("patches", {}) + rescue StandardError + return [] + end + return [] unless records.is_a?(Hash) + gems_dir = File.join(bundle_path, "gems") + targets = [] + records.each do |purl, record| + next unless purl.is_a?(String) && purl.start_with?("pkg:gem/") + coordinate = purl.split("pkg:gem/", 2).last.split("?", 2).first + name, at, version = coordinate.rpartition("@") + next if at.empty? || name.empty? || version.empty? + files = record.is_a?(Hash) ? record["files"] : nil + next unless files.is_a?(Hash) + files.each_key do |key| + rel = key.to_s.sub(%r{\Apackage/}, "") + targets << File.join(gems_dir, "#{name}-#{version}", rel) + targets.concat(Dir.glob(File.join(gems_dir, "#{name}-#{version}-*", rel))) + end + end + targets.uniq.sort + end + # Files whose change must force a reapply: the manifest, every committed file - # under .socket/ (patch blobs etc.), and Gemfile.lock. + # under .socket/ (patch blobs etc. — the stamp itself excluded, or each write + # would invalidate the digest it records), Gemfile.lock, and the current + # on-disk state of every patch target. def digest_inputs inputs = [manifest_path] lock = File.join(project_root, "Gemfile.lock") inputs << lock if File.file?(lock) - socket_dir = File.join(project_root, ".socket") if File.directory?(socket_dir) Dir.glob(File.join(socket_dir, "**", "*")).sort.each do |p| - inputs << p if File.file?(p) + inputs << p if File.file?(p) && p != stamp_path end end + inputs.concat(patch_target_files) inputs.uniq end @@ -79,27 +157,18 @@ def current_digest digest_inputs.each do |path| d.update(path) d.update("\0") + d.update(File.file?(path) ? "+" : "-") begin d.update(File.binread(path)) rescue StandardError - # Unreadable now -> contributes only its path; a later readable state - # changes the digest and forces a reapply. + # Unreadable now -> contributes only its path + absence marker; a later + # readable state changes the digest and forces a reapply. end d.update("\0") end d.hexdigest end - def bundle_path - Bundler.bundle_path.to_s - rescue StandardError - File.join(project_root, "vendor", "bundle") - end - - def stamp_path - File.join(bundle_path, STAMP_NAME) - end - def stamped?(digest) File.file?(stamp_path) && File.read(stamp_path).strip == digest rescue StandardError @@ -113,52 +182,105 @@ def write_stamp(digest) # Best-effort: a missing/unwritable stamp just means we re-probe next time. end - def fail!(message) - raise(defined?(Bundler::BundlerError) ? Bundler::BundlerError.new(message) : message) + # Older plugin versions stamped under Bundler.bundle_path. It is never read + # anymore; delete it (best-effort, once per process) so it does not linger + # as an orphan in a shared gem dir. + def remove_legacy_stamp + return if @legacy_stamp_checked + @legacy_stamp_checked = true + legacy = File.join(bundle_path, LEGACY_STAMP_NAME) + File.delete(legacy) if File.file?(legacy) + rescue StandardError + # Best-effort cleanup only. end - # Idempotent, missing-gem-tolerant. No manifest -> the project does not use - # socket-patch, nothing to do. When `force` is false the digest stamp short- - # circuits already-applied state; the after-install-all hook passes force:true - # because the installer just changed the on-disk gem set. - def apply!(force: false) - return unless File.file?(manifest_path) - - digest = current_digest - return if !force && stamped?(digest) - - ok = system( - socket_bin, "apply", - "--ecosystems", "gem", "--offline", "--silent", - "--cwd", project_root - ) - - if ok.nil? - fail!( - "socket-patch: could not run `#{socket_bin} apply` to apply gem patches; " \ - "the socket-patch CLI is required. Install it or set #{BIN_ENV} to its path." - ) - elsif !ok - fail!( - "socket-patch: `#{socket_bin} apply --ecosystems gem` failed; the gem patches " \ - "in .socket/manifest.json are NOT applied. The build was failed to avoid " \ - "shipping unpatched gems." - ) + # Tolerant by default: a patch failure must never break `bundle install` — + # the first install of a fresh checkout runs the applier before any project + # gem exists, and raising there deadlocks the project on its own bootstrap + # (plugin registration fails, so every retry fails identically). Warn once + # per process with the remediation; SOCKET_PATCH_STRICT=1 restores the raise + # for builds that must not proceed unpatched. + def report_failure(message) + if strict? + raise(defined?(Bundler::BundlerError) ? Bundler::BundlerError.new(message) : message) end + return if @warned + @warned = true + warn(message) + end + + # Idempotent applier behind every trigger. No manifest -> the project does + # not use socket-patch, nothing to do. + # force: skip the digest gate (the installer just changed the gem set). + # require_stamp: bail unless a previous apply stamped this project — the + # load-time trigger and the per-gem after-install hook use it so a + # bootstrap install's early evaluations don't shell out (and warn) before + # the target gems even exist; the forced after-install-all pass does the + # first real apply and lays down the stamp. + def apply!(force: false, require_stamp: false) + APPLY_LOCK.synchronize do + return unless File.file?(manifest_path) + remove_legacy_stamp + return if require_stamp && !File.file?(stamp_path) + return if !force && stamped?(current_digest) + + ok = system( + socket_bin, "apply", + "--ecosystems", "gem", "--offline", "--silent", + "--cwd", project_root + ) + + if ok.nil? + report_failure( + "socket-patch: could not run `#{socket_bin} apply` — the gem patches in " \ + ".socket/manifest.json are NOT applied. Install the socket-patch CLI (or set " \ + "#{BIN_ENV} to its path), then run `socket-patch apply --ecosystems gem` " \ + "manually. `bundle install` continues; set #{STRICT_ENV}=1 to make this fatal." + ) + return + elsif !ok + report_failure( + "socket-patch: `#{socket_bin} apply --ecosystems gem` failed — the gem patches " \ + "in .socket/manifest.json may NOT be applied. Run `socket-patch apply " \ + "--ecosystems gem` in #{project_root} to apply them manually. " \ + "`bundle install` continues; set #{STRICT_ENV}=1 to make patch failures fatal." + ) + return + end - write_stamp(digest) + if @warned + @warned = false + warn("socket-patch: gem patches applied; the earlier warning is resolved.") + end + # Recompute: the apply just rewrote the target files the digest folds in. + write_stamp(current_digest) + end end end -# Trigger 1 — load-time (covers the cached/no-op `bundle install`). +# Trigger 1 — load time. Runs at plugin registration and whenever a subscribed +# hook event first loads the plugin in a bundle process. Stamp-gated: on the +# bootstrap install nothing is stamped yet (and no gems exist to patch), so +# this quietly defers to Trigger 3. In strict mode a genuine patch failure +# (Bundler::BundlerError) still propagates. begin - SocketPatch.apply! + SocketPatch.apply!(require_stamp: true) rescue StandardError => e raise if defined?(Bundler::BundlerError) && e.is_a?(Bundler::BundlerError) end -# Trigger 2 — after the installer finishes (covers the fresh install). Forced, -# because the install just changed the gem set; the applier is idempotent. +# Trigger 2 — after each individual gem (re)install. The only event bundler +# fires during `bundle pristine`, so this is what catches pristine's patch +# reversion in the same run. Digest- and stamp-gated: on a fresh install's +# per-gem events the project is not stamped yet and Trigger 3 is about to do +# the real work. +Bundler::Plugin.add_hook("after-install") do |_spec_install| + SocketPatch.apply!(require_stamp: true) +end + +# Trigger 3 — after the installer finishes (fresh AND fully-cached installs). +# Forced, because the install just changed the gem set; the applier is +# idempotent so a redundant run on an already-patched tree is a cheap no-op. Bundler::Plugin.add_hook("after-install-all") do |_install| SocketPatch.apply!(force: true) end diff --git a/gem/socket-patch-bundler/socket-patch-bundler.gemspec b/gem/socket-patch-bundler/socket-patch-bundler.gemspec index bf130b4e..01359d65 100644 --- a/gem/socket-patch-bundler/socket-patch-bundler.gemspec +++ b/gem/socket-patch-bundler/socket-patch-bundler.gemspec @@ -2,7 +2,7 @@ # Published form of the socket-patch Bundler plugin (CLI_CONTRACT property: # "gem" support matrix, Phase 2). `socket-patch setup` today references the -# in-tree plugin under `.socket/bundler-plugin/` via `git:`; once this gem is +# in-tree plugin under `.socket/bundler-plugin/` via `path:`; once this gem is # published, a follow-up switches the Gemfile directive to # `plugin "socket-patch-bundler", "~> "`. The version is kept in # sync with the workspace by `scripts/version-sync.sh`. diff --git a/gem/socket-patch/lib/socket_patch/launcher.rb b/gem/socket-patch/lib/socket_patch/launcher.rb index bed9b1db..0b0a706f 100644 --- a/gem/socket-patch/lib/socket_patch/launcher.rb +++ b/gem/socket-patch/lib/socket_patch/launcher.rb @@ -31,14 +31,24 @@ def run(argv) bin = resolve_binary if Gem.win_platform? # Windows has no exec() that replaces the process cleanly for console - # apps; spawn + wait and propagate the child's exit status. - exit(system(bin, *argv) ? $?.exitstatus : 1) + # apps; spawn + wait and propagate the child's real exit status (a + # blanket 1 would erase the CLI's meaningful non-zero codes, e.g. + # `setup --check`'s needs-configuration signal). + ok = system(bin, *argv) + raise LauncherError, "could not run #{bin}" if ok.nil? + exit($?.exitstatus || 1) else exec([bin, bin], *argv) end rescue LauncherError => e warn("socket-patch: #{e.message}") exit(1) + rescue StandardError => e + # First-run download/extract failures outside our own error type (DNS + # outages, TLS errors, ...) must exit cleanly, not escape as raw + # backtraces. + warn("socket-patch: #{e.class}: #{e.message}") + exit(1) end class LauncherError < StandardError; end @@ -74,7 +84,10 @@ def version return spec.version.to_s end Gem::Specification.find_by_name("socket-patch").version.to_s - rescue StandardError + rescue StandardError, Gem::LoadError + # Gem::MissingSpecError (the gem isn't installed at all — running from + # a checkout) is a Gem::LoadError, which is NOT a StandardError; without + # naming it the documented fallback never engaged. VERSION end @@ -152,9 +165,33 @@ def download_binary(ver, target, ext, dest) raise LauncherError, "release archive #{archive} did not contain #{exe}" end - FileUtils.mkdir_p(File.dirname(dest)) - FileUtils.cp(extracted, dest) - File.chmod(0o755, dest) unless Gem.win_platform? + install_executable(extracted, dest) + end + end + + # Publish the verified binary into the cache atomically: copy to a temp + # file in the destination dir, set the exec bit, then rename over the + # final path — a concurrent first run can only ever see a complete, + # executable binary, never a torn or not-yet-chmodded one. + def install_executable(src, dest) + FileUtils.mkdir_p(File.dirname(dest)) + tmp = File.join(File.dirname(dest), ".#{File.basename(dest)}.#{Process.pid}.tmp") + begin + FileUtils.cp(src, tmp) + File.chmod(0o755, tmp) unless Gem.win_platform? + begin + File.rename(tmp, dest) + rescue SystemCallError + # Windows rename cannot replace an existing file: a concurrent + # first run already published the (identical, verified) binary. + raise unless File.exist?(dest) + end + ensure + begin + File.delete(tmp) if File.file?(tmp) + rescue StandardError + # Leftover temp cleanup is best-effort. + end end end @@ -223,6 +260,14 @@ def verify_sha256!(path, archive, sums) raise LauncherError, "checksum mismatch for #{archive} (expected #{expected}, got #{actual})" end + # Quote a value for interpolation into a PowerShell command: single-quoted + # strings are literal except for embedded single quotes, which are escaped + # by doubling them (paths like `it's here` would otherwise break the + # command). + def powershell_quote(value) + "'#{value.gsub("'", "''")}'" + end + def extract(archive_path, ext, dir) ok = if ext == "zip" @@ -230,7 +275,8 @@ def extract(archive_path, ext, dir) # PowerShell Expand-Archive. system("tar", "-xf", archive_path, "-C", dir) || system("powershell", "-NoProfile", "-Command", - "Expand-Archive -Force -LiteralPath '#{archive_path}' -DestinationPath '#{dir}'") + "Expand-Archive -Force -LiteralPath #{powershell_quote(archive_path)} " \ + "-DestinationPath #{powershell_quote(dir)}") else system("tar", "xzf", archive_path, "-C", dir) end diff --git a/tests/setup_matrix/run-case.sh b/tests/setup_matrix/run-case.sh index c399b675..b4713c3a 100755 --- a/tests/setup_matrix/run-case.sh +++ b/tests/setup_matrix/run-case.sh @@ -150,14 +150,15 @@ marker_blob() { # $1 = marker -> runnable payload on stdout esac } -write_manifest() { # $1=purl $2=key $3=afterHash +write_manifest() { # $1=purl $2=key $3=afterHash $4=beforeHash (default: zero) + local before="${4:-$ZEROHASH}" cat > .socket/manifest.json </dev/null 2>&1 \ + && gem unpack "${SM_PACKAGE}-${SM_VERSION}.gem" >/dev/null 2>&1) \ + && [ -f "$target" ]; then + git_sha256 "$target" + else + # The function runs inside $(...): route the log PAST the capture pipe. + log "gem beforeHash probe failed; falling back to the zero placeholder" >&2 + printf '%s' "$ZEROHASH" + fi + rm -rf "$dir" +} + build_fixture() { # Ablation: no patch set committed at all (no .socket/). Even with a # working install hook, apply finds no manifest and no-ops, so the @@ -196,11 +226,11 @@ build_fixture() { alt) marker_blob "$SM_ALT_MARKER" > "$blob_tmp" local h; h="$(git_sha256 "$blob_tmp")"; cp "$blob_tmp" ".socket/blobs/$h" - write_manifest "$SM_PURL" "$SM_MANIFEST_KEY" "$h" ;; + write_manifest "$SM_PURL" "$SM_MANIFEST_KEY" "$h" "$(resolve_before_hash)" ;; *) # primary marker_blob "$SM_MARKER" > "$blob_tmp" local h; h="$(git_sha256 "$blob_tmp")"; cp "$blob_tmp" ".socket/blobs/$h" - write_manifest "$SM_PURL" "$SM_MANIFEST_KEY" "$h" ;; + write_manifest "$SM_PURL" "$SM_MANIFEST_KEY" "$h" "$(resolve_before_hash)" ;; esac rm -f "$blob_tmp" } From 9b0fd73054f6b8e2e76f02a2e2020d34c2326fdb Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Thu, 13 Aug 2026 18:30:17 -0700 Subject: [PATCH 4/5] fix(gem): bootstrap gate keys on target presence, not the committed-dir stamp; gitignore + lifecycle it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The require_stamp gate trusted the EXISTENCE of .socket/gem-plugin-stamp — a file living in the directory every other workflow file tells users to COMMIT. Reproduced against bundler 4.0.15: a stale stamp that reaches version control passes the gate at plugin REGISTRATION on a fresh clone, digest-mismatches (targets absent), shells apply, and under SOCKET_PATCH_STRICT=1 resurrects the exact bootstrap deadlock this plugin exists to avoid (exit 29, "Failed to install plugin", no plugin index, every retry identical). Deleting the stamp had the inverse sharp edge: the gated triggers went dead, so `bundle pristine` left the patches reverted until the next `bundle install`. - plugins.rb template + published twin: the bootstrap gate now bails while NONE of the manifest's gem-patch targets exist on disk, reading the live gem tree and never the stamp (which is now a pure digest cache). Registration on a fresh clone stays quiet regardless of stamp state, and pristine heals in the same run even with the stamp deleted — both verified against real bundler 4.0.15. - report_failure: the trailer now states what the ACTIVE mode does — the strict raise says the install is failing because SOCKET_PATCH_STRICT is set, instead of claiming "`bundle install` continues". - setup wires /gem-plugin-stamp into .socket/.gitignore (append-only, sparing user lines) so the stamp never lands in git status or a blanket `git add .socket`; `--check` demands the entry (check/setup agreement); `--remove` best-effort deletes the stamp and strips our line. - matrix.json: the gem row records reality — hook_family bundler-plugin, baseline_supported true — so a future regression of the with-setup flow classifies as blocking regression, not a known gap. - launcher_guard::run_ruby scrubs RUBYOPT/BUNDLE_*/GEM_*/SOCKET_* like plugin_runtime::scrub, so the suite survives `bundle exec`. New pins: plugin_runtime::committed_stale_stamp_does_not_deadlock_strict_ fresh_clone (registration recorded, hook-only failure, retry converges), plugin_runtime::bootstrap_gate_keys_on_target_presence_not_stamp (both gate directions), strict-trailer asserts in the strict-mode test, gitignore and stamp-lifecycle asserts in host_guard + core gem tests. Co-Authored-By: Claude Fable 5 --- .../tests/setup_matrix_gem.rs | 182 ++++++++++++++++- crates/socket-patch-core/src/setup/gem/mod.rs | 191 +++++++++++++++++- .../src/setup/gem/templates/plugins.rb.tmpl | 78 ++++--- gem/socket-patch-bundler/plugins.rb | 72 ++++--- tests/setup_matrix/matrix.json | 4 +- 5 files changed, 460 insertions(+), 67 deletions(-) diff --git a/crates/socket-patch-cli/tests/setup_matrix_gem.rs b/crates/socket-patch-cli/tests/setup_matrix_gem.rs index d4fa72cb..3009687a 100644 --- a/crates/socket-patch-cli/tests/setup_matrix_gem.rs +++ b/crates/socket-patch-cli/tests/setup_matrix_gem.rs @@ -273,6 +273,15 @@ mod host_guard { rb.contains("BundlerError"), "plugins.rb must still raise Bundler::BundlerError in strict mode:\n{rb}" ); + // The plugin's digest stamp is machine-local, but it lives in the + // otherwise-committed .socket/ — setup must gitignore it, or every + // install litters `git status` and a blanket `git add .socket` + // commits one machine's stamp to every clone. + let gitignore = std::fs::read_to_string(root.join(".socket/.gitignore")).unwrap(); + assert!( + gitignore.lines().any(|l| l == "/gem-plugin-stamp"), + ".socket/.gitignore must carry the stamp entry:\n{gitignore}" + ); // ── check (after setup): configured, exit 0 ───────────────────────── let (code, out, err) = run(root, &["setup", "--check", "--cwd", root_s, "--json"]); @@ -301,6 +310,9 @@ mod host_guard { ); // ── remove: byte-for-byte restore + plugin dir gone ───────────────── + // A stamp left behind by a previous apply: `--remove`'s no-residue + // contract covers it (it sits in the committed .socket/ dir). + std::fs::write(root.join(".socket/gem-plugin-stamp"), "e".repeat(64)).unwrap(); let (code, out, err) = run( root, &["setup", "--remove", "--cwd", root_s, "--yes", "--json"], @@ -321,6 +333,15 @@ mod host_guard { !root.join(PLUGIN_DIR).exists(), "remove must delete the generated plugin dir" ); + assert!( + !root.join(".socket/gem-plugin-stamp").exists(), + "remove must delete the plugin's digest stamp — unwiring must not \ + orphan it in the committed .socket/" + ); + assert!( + !root.join(".socket/.gitignore").exists(), + "remove must delete the .gitignore setup created (it held only our line)" + ); // ── check (after remove): needs_configuration again, exit 1 ───────── let (code, out, _) = run(root, &["setup", "--check", "--cwd", root_s, "--json"]); @@ -626,6 +647,146 @@ mod plugin_runtime { err.contains("socket-patch"), "the strict failure must carry the socket-patch message:\n{err}" ); + // The strict raise must tell the truth about the active mode: the + // tolerant trailer ("`bundle install` continues; set + // SOCKET_PATCH_STRICT=1 ...") is false on both counts while the + // install is failing and the var is already set. + assert!( + err.contains("because SOCKET_PATCH_STRICT is set"), + "the strict failure must say WHY the install is failing:\n{err}" + ); + assert!( + !err.contains("`bundle install` continues"), + "the strict failure must not claim the install continues:\n{err}" + ); + } + + /// [P0 regression: committed stale stamp] `.socket/` is a committed + /// directory, so a stamp that reaches version control (a blanket + /// `git add .socket`) arrives on every fresh clone BEFORE the first + /// `bundle install`. If the bootstrap gate keyed on the stamp's + /// existence, plugin REGISTRATION would shell the applier (targets + /// absent -> apply fails) and a strict-mode raise there resurrects the + /// exact deadlock this plugin exists to avoid — exit 29, "Failed to + /// install plugin", every retry identical (reproduced on bundler + /// 4.0.15). The gate must key on the patch targets existing on disk: + /// registration completes, strict enforcement waits for the + /// post-install hooks. + #[test] + fn committed_stale_stamp_does_not_deadlock_strict_fresh_clone() { + if !have("bundle") { + eprintln!("skip plugin_runtime: bundler not on PATH"); + return; + } + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + scaffold(root); + // The stale stamp a teammate committed, exactly as a fresh clone sees it. + std::fs::write(root.join(STAMP_REL), "a".repeat(64)).unwrap(); + let (fake, log) = write_fake_apply(root, 1); + + let (code, out, err) = bundle_install(root, &fake, &[("SOCKET_PATCH_STRICT", "1")]); + // Registration must complete — the strict failure may only come from + // the post-install hooks, never from plugin registration. + assert!( + !err.contains("Failed to install plugin"), + "a committed stale stamp must not fail plugin REGISTRATION.\n{out}\n{err}" + ); + assert!( + root.join(".bundle/plugin/index").is_file(), + "registration must be recorded despite the strict failure.\n{out}\n{err}" + ); + assert_ne!( + code, 0, + "strict mode still fails the install — from the hook.\n{out}\n{err}" + ); + assert!( + !apply_calls(&log).is_empty(), + "anti-vacuity: the forced post-install apply ran (and failed)" + ); + + // The deadlock is gone: with apply working, the SAME checkout (stale + // stamp still in place) converges on retry. + let (fake, _log) = write_fake_apply(root, 0); + let (code, out, err) = bundle_install(root, &fake, &[("SOCKET_PATCH_STRICT", "1")]); + assert_eq!( + code, 0, + "retry with a working apply must succeed — registration was never \ + poisoned.\n{out}\n{err}" + ); + } + + /// The bootstrap gate reads the LIVE gem tree, never the stamp. Both + /// directions matter: with no patch target on disk the gated triggers + /// must not shell out no matter what a (stale, possibly committed) stamp + /// says; with the target present they must re-apply even when the stamp + /// is missing — a `bundle pristine` run after deleting the stamp used to + /// leave the patches silently reverted until the next `bundle install` + /// (reproduced on bundler 4.0.15). + #[test] + fn bootstrap_gate_keys_on_target_presence_not_stamp() { + if !have("ruby") { + eprintln!("skip plugin_runtime: ruby not on PATH"); + return; + } + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + scaffold(root); + let (fake, log) = write_fake_apply(root, 0); + + // Drive the gated path exactly as the load-time / per-gem hooks do. + let drive = |label: &str| { + let mut cmd = Command::new("ruby"); + cmd.args([ + "-e", + "require \"bundler\"; load ARGV[0]; SocketPatch.apply!(bootstrap_gate: true)", + "--", + ]) + .arg(root.join(".socket/bundler-plugin/plugins.rb")) + .current_dir(root); + scrub(&mut cmd); + cmd.env("BUNDLE_PATH", "vendor/bundle"); + cmd.env("SOCKET_PATCH_BIN", &fake); + let (code, out, err) = run(cmd); + assert_eq!(code, 0, "{label}: gated drive failed.\n{out}\n{err}"); + }; + + // Fresh clone: no target on disk, stale stamp committed. + std::fs::write(root.join(STAMP_REL), "b".repeat(64)).unwrap(); + drive("no target, stale stamp"); + assert_eq!( + apply_calls(&log).len(), + 0, + "no target on disk -> the gated trigger must not shell out, \ + whatever the stamp says" + ); + + // Target installed, stamp deleted: the pristine-heal case. + let mut cmd = Command::new("ruby"); + cmd.args(["-e", "require \"bundler\"; print Bundler.bundle_path"]) + .current_dir(root); + scrub(&mut cmd); + cmd.env("BUNDLE_PATH", "vendor/bundle"); + let (code, bundle_path, err) = run(cmd); + assert_eq!(code, 0, "Bundler.bundle_path probe failed: {err}"); + let target = PathBuf::from(bundle_path.trim()).join("gems/colorize-1.1.0/lib/colorize.rb"); + std::fs::create_dir_all(target.parent().unwrap()).unwrap(); + std::fs::write(&target, "REVERTED BY PRISTINE\n").unwrap(); + std::fs::remove_file(root.join(STAMP_REL)).unwrap(); + + drive("target present, no stamp"); + assert_eq!( + apply_calls(&log).len(), + 1, + "with the target on disk and nothing validly stamped, the gated \ + trigger must re-apply — deleting the stamp defers nothing" + ); + drive("digest-gated no-op"); + assert_eq!( + apply_calls(&log).len(), + 1, + "the re-apply stamped the state; the next gated probe is a no-op" + ); } /// [P2 stamp location] A successful apply stamps the PROJECT @@ -816,12 +977,30 @@ mod launcher_guard { .expect("launcher.rb must exist in the workspace") } + /// Strip the ambient vars that could flip a verdict, mirroring + /// `plugin_runtime::scrub`: the CLI's SOCKET_* surface, and the + /// bundler/rubygems config a `bundle exec cargo test` run injects + /// (RUBYOPT=-rbundler/setup, BUNDLE_*, GEM_*) — which would activate a + /// foreign bundle inside the child ruby under test. + fn scrub(cmd: &mut Command) { + for (key, _) in std::env::vars_os() { + let name = key.to_string_lossy().into_owned(); + let hit = (name.starts_with("SOCKET_") && name != "SOCKET_NO_CONFIG") + || name.starts_with("BUNDLE_") + || name.starts_with("GEM_") + || name == "RUBYOPT"; + if hit { + cmd.env_remove(&name); + } + } + } + /// Run `script` (which `load`s the launcher via the SP_LAUNCHER env) with /// a scratch HOME/cache so no real launcher cache is consulted. fn run_ruby(script: &Path, cache: &Path, envs: &[(&str, &str)]) -> (i32, String, String) { let mut cmd = Command::new("ruby"); cmd.arg(script); - cmd.env_remove("SOCKET_PATCH_BIN"); + scrub(&mut cmd); cmd.env("SP_LAUNCHER", launcher_path()); cmd.env("XDG_CACHE_HOME", cache); for (k, v) in envs { @@ -966,6 +1145,7 @@ mod launcher_guard { let mut cmd = Command::new("ruby"); cmd.arg(&script).arg(&src).arg(&dest); + scrub(&mut cmd); cmd.env("SP_LAUNCHER", launcher_path()); let out = cmd.output().expect("spawn ruby"); assert!( diff --git a/crates/socket-patch-core/src/setup/gem/mod.rs b/crates/socket-patch-core/src/setup/gem/mod.rs index 563eb375..5cfe2ea9 100644 --- a/crates/socket-patch-core/src/setup/gem/mod.rs +++ b/crates/socket-patch-core/src/setup/gem/mod.rs @@ -21,7 +21,12 @@ //! sources with `git clone`, and the generated dir is a plain directory //! (committing it to the parent repo does not give it a `.git`), so `git:` //! fails every `bundle install`. The directory still must be committed so -//! clones and CI have the plugin on disk. +//! clones and CI have the plugin on disk; +//! * a `.socket/.gitignore` entry for the plugin's digest stamp +//! (`gem-plugin-stamp`) — everything else under `.socket/` is meant to be +//! committed, and an untracked stamp in every wired repo's `git status` +//! invites a `git add .socket` that ships one machine's digest to every +//! clone. //! //! The actual gem patching is done by `apply` (unchanged); this module only //! manages the setup wiring. Phase 2 (follow-up) replaces the in-tree plugin @@ -43,6 +48,15 @@ const PLUGIN_DIR: &str = ".socket/bundler-plugin"; /// First line of every generated plugin file — the ownership signal for removal /// (we never delete a file that lacks it). const GENERATED_MARKER: &str = "# Code generated by `socket-patch setup`. DO NOT EDIT."; +/// The generated plugin's digest stamp, relative to the project root +/// (machine-local state; the plugin owns its content, setup owns its lifecycle). +const STAMP_REL: &str = ".socket/gem-plugin-stamp"; +/// The `.socket/.gitignore` line that keeps the stamp out of version control. +/// Everything else under `.socket/` is meant to be committed, so without this +/// line every install drops an untracked stamp into `git status` — and a stamp +/// committed by a blanket `git add .socket` ships one machine's digest to +/// every clone. +const STAMP_IGNORE_LINE: &str = "/gem-plugin-stamp"; /// The generated `plugins.rb` body (the two-trigger idempotent applier). const PLUGINS_RB: &str = include_str!("templates/plugins.rb.tmpl"); @@ -131,6 +145,63 @@ fn gemspec_path(root: &Path) -> PathBuf { plugin_dir(root).join("socket-patch.gemspec") } +fn stamp_path(root: &Path) -> PathBuf { + root.join(STAMP_REL) +} + +fn stamp_gitignore_path(root: &Path) -> PathBuf { + root.join(".socket").join(".gitignore") +} + +/// Whether `.socket/.gitignore` is missing the stamp entry (so `setup` still +/// has a write to make). Shared by [`add_plugin_files`] and +/// [`plugin_files_present`] to keep `setup` and `--check` in agreement. +async fn stamp_ignore_missing(root: &Path) -> bool { + match fs::read_to_string(stamp_gitignore_path(root)).await { + Ok(c) => !c.lines().any(|l| l.trim() == STAMP_IGNORE_LINE), + Err(_) => true, + } +} + +/// Append the stamp entry to `.socket/.gitignore`, preserving any existing +/// (user or other-tool) lines. Creates the file when absent. +async fn add_stamp_gitignore(root: &Path) -> Result<(), String> { + let path = stamp_gitignore_path(root); + let existing = fs::read_to_string(&path).await.unwrap_or_default(); + let mut body = existing; + if !body.is_empty() && !body.ends_with('\n') { + body.push('\n'); + } + body.push_str(STAMP_IGNORE_LINE); + body.push('\n'); + write_file(&path, &body).await +} + +/// Best-effort cleanup of the stamp artifacts on `--remove`: delete the stamp +/// itself and strip our line from `.socket/.gitignore` (deleting the file when +/// nothing else is left, sparing any other lines). Best-effort by design — a +/// leftover stamp is inert machine-local state, not worth failing an +/// otherwise-successful unwire over. +async fn remove_stamp_artifacts(root: &Path) { + let _ = fs::remove_file(stamp_path(root)).await; + let path = stamp_gitignore_path(root); + let Ok(content) = fs::read_to_string(&path).await else { + return; + }; + let kept: Vec<&str> = content + .lines() + .filter(|l| l.trim() != STAMP_IGNORE_LINE) + .collect(); + if kept.len() == content.lines().count() { + return; + } + if kept.iter().all(|l| l.trim().is_empty()) { + let _ = fs::remove_file(&path).await; + } else { + let _ = fs::write(&path, format!("{}\n", kept.join("\n"))).await; + } +} + /// Whether the generated plugin files are present *and* match the templates the /// current CLI generates (the `setup --check` "configured" signal, paired with /// the Gemfile directive check). @@ -147,6 +218,7 @@ fn gemspec_path(root: &Path) -> PathBuf { pub async fn plugin_files_present(root: &Path) -> bool { !needs_write(&plugins_rb_path(root), PLUGINS_RB).await && !needs_write(&gemspec_path(root), GEMSPEC).await + && !stamp_ignore_missing(root).await } /// True if the file is absent or its content differs from `desired`. @@ -168,15 +240,17 @@ async fn write_file(path: &Path, body: &str) -> Result<(), String> { .map_err(|e| format!("write {}: {e}", path.display())) } -/// Generate `.socket/bundler-plugin/{plugins.rb, socket-patch.gemspec}`. -/// Idempotent: `AlreadyConfigured` when both already match the templates byte -/// for byte. `kind = "gem_plugin"`. +/// Generate `.socket/bundler-plugin/{plugins.rb, socket-patch.gemspec}` and +/// make sure `.socket/.gitignore` keeps the plugin's digest stamp untracked. +/// Idempotent: `AlreadyConfigured` when everything already matches. `kind = +/// "gem_plugin"`. async fn add_plugin_files(root: &Path, dry_run: bool) -> GemEditResult { let dir = plugin_dir(root); let result = async { let rb_changed = needs_write(&plugins_rb_path(root), PLUGINS_RB).await; let spec_changed = needs_write(&gemspec_path(root), GEMSPEC).await; - if !rb_changed && !spec_changed { + let ignore_missing = stamp_ignore_missing(root).await; + if !rb_changed && !spec_changed && !ignore_missing { return Ok(false); } if !dry_run { @@ -186,6 +260,9 @@ async fn add_plugin_files(root: &Path, dry_run: bool) -> GemEditResult { if spec_changed { write_file(&gemspec_path(root), GEMSPEC).await?; } + if ignore_missing { + add_stamp_gitignore(root).await?; + } } Ok(true) } @@ -214,8 +291,11 @@ async fn remove_generated(path: &Path) -> Result<(), String> { /// Remove the generated plugin files — each only when it carries our /// [`GENERATED_MARKER`], so a user-authored file at either path is never -/// deleted (and an orphaned generated file is still cleaned up). Idempotent: -/// `AlreadyConfigured` when nothing of ours is there. +/// deleted (and an orphaned generated file is still cleaned up) — plus the +/// plugin's digest stamp and its `.socket/.gitignore` entry (the plugin that +/// maintained them is being unwired; leaving the stamp behind would orphan it +/// in the otherwise-committed `.socket/`). Idempotent: `AlreadyConfigured` +/// when nothing of ours is there. async fn remove_plugin_files(root: &Path, dry_run: bool) -> GemEditResult { let dir = plugin_dir(root); let result = async { @@ -231,6 +311,7 @@ async fn remove_plugin_files(root: &Path, dry_run: bool) -> GemEditResult { if spec_ours { remove_generated(&gemspec_path(root)).await?; } + remove_stamp_artifacts(root).await; // Prune the now-empty plugin dir (leave .socket/ — apply uses it). let _ = fs::remove_dir(&dir).await; } @@ -467,6 +548,9 @@ mod tests { PLUGINS_RB.contains("BundlerError"), "strict mode must still raise Bundler::BundlerError" ); + // The strict raise must carry a strict trailer, not the tolerant + // "`bundle install` continues" text (which is false mid-raise). + assert!(PLUGINS_RB.contains("def failure_trailer")); // Stamp location: project-scoped under .socket/, NOT a fixed-name file // in Bundler.bundle_path (machine-global with no bundle path // configured, shared and clobbered across every project on the host). @@ -478,15 +562,28 @@ mod tests { // The stamp must be excluded from its own digest inputs, or every // write would invalidate the digest it records. assert!(PLUGINS_RB.contains("p != stamp_path")); + // The bootstrap gate keys on the patch targets existing on disk, NEVER + // on the stamp: `.socket/` is a committed directory, so a stale stamp + // that reaches version control would otherwise pass the gate at plugin + // REGISTRATION on a fresh clone and resurrect the strict-mode + // bootstrap deadlock this plugin exists to avoid. + assert!(PLUGINS_RB.contains("bootstrap_gate && patch_target_files.none?")); + assert!( + !PLUGINS_RB.contains("require_stamp"), + "the stamp-existence gate must be gone — a committed stamp made it \ + a remote-controlled registration deadlock" + ); // The published-gem twin must carry the same applier contract. let published = include_str!("../../../../../gem/socket-patch-bundler/plugins.rb"); for needle in [ "def report_failure", + "def failure_trailer", "SOCKET_PATCH_STRICT", "STAMP_NAME = \"gem-plugin-stamp\"", "def remove_legacy_stamp", "def patch_target_files", + "bootstrap_gate && patch_target_files.none?", "Bundler::Plugin.add_hook(\"after-install\")", "Bundler::Plugin.add_hook(\"after-install-all\")", ] { @@ -496,6 +593,7 @@ mod tests { ); } assert!(!published.contains("def fail!")); + assert!(!published.contains("require_stamp")); } #[tokio::test] @@ -509,16 +607,32 @@ mod tests { fs::read_to_string(plugins_rb_path(root)).await.unwrap(), PLUGINS_RB ); + // The stamp is gitignored: everything else under .socket/ is committed, + // so without this line every install litters `git status` and a blanket + // `git add .socket` commits one machine's stamp to every clone. + assert_eq!( + fs::read_to_string(stamp_gitignore_path(root)) + .await + .unwrap(), + "/gem-plugin-stamp\n" + ); // Idempotent. assert_eq!( add_plugin_files(root, false).await.status, GemSetupStatus::AlreadyConfigured ); - // Remove. + // Remove — including the stamp a previous apply left behind, which + // would otherwise be orphaned in the committed .socket/ dir. + fs::write(stamp_path(root), "f".repeat(64)).await.unwrap(); let rr = remove_plugin_files(root, false).await; assert_eq!(rr.status, GemSetupStatus::Updated); assert!(!plugin_files_present(root).await); assert!(!plugin_dir(root).exists(), "empty plugin dir pruned"); + assert!(!stamp_path(root).exists(), "stamp removed with the plugin"); + assert!( + !stamp_gitignore_path(root).exists(), + "the .gitignore we created (nothing but our line) is removed too" + ); // Remove again → already gone. assert_eq!( remove_plugin_files(root, false).await.status, @@ -754,8 +868,9 @@ mod tests { } #[tokio::test] - async fn test_plugin_files_present_requires_both() { - // The "configured" signal must demand BOTH files, not either one. + async fn test_plugin_files_present_requires_all_three() { + // The "configured" signal must demand BOTH generated files AND the + // stamp's .gitignore entry, not any subset. let dir = tempfile::tempdir().unwrap(); let root = dir.path(); write(&plugins_rb_path(root), PLUGINS_RB).await; @@ -764,6 +879,62 @@ mod tests { "plugins.rb alone is not 'configured' — the gemspec is required too" ); write(&gemspec_path(root), GEMSPEC).await; + assert!( + !plugin_files_present(root).await, + "check and setup must agree: setup would still write the stamp's \ + .gitignore entry, so this is not 'configured' yet" + ); + assert_eq!( + add_plugin_files(root, true).await.status, + GemSetupStatus::Updated, + "setup agrees it still has the .gitignore write to make" + ); + write(&stamp_gitignore_path(root), "/gem-plugin-stamp\n").await; + assert!(plugin_files_present(root).await); + } + + #[tokio::test] + async fn test_stamp_gitignore_add_and_remove_spare_user_lines() { + // A user's own .socket/.gitignore entries must survive both the add + // (line appended, not the file clobbered) and the remove (only our + // line stripped, file kept). + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + write(&stamp_gitignore_path(root), "my-scratch-dir/\n").await; + + add_plugin_files(root, false).await; + assert_eq!( + fs::read_to_string(stamp_gitignore_path(root)) + .await + .unwrap(), + "my-scratch-dir/\n/gem-plugin-stamp\n", + "our line is appended after the user's" + ); + + remove_plugin_files(root, false).await; + assert_eq!( + fs::read_to_string(stamp_gitignore_path(root)) + .await + .unwrap(), + "my-scratch-dir/\n", + "only our line is stripped; the user's file survives" + ); + } + + #[tokio::test] + async fn test_add_appends_gitignore_line_to_unterminated_file() { + // A .gitignore whose last line has no trailing newline must not have + // our entry glued onto it ("vendor//gem-plugin-stamp" ignores nothing). + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + write(&stamp_gitignore_path(root), "vendor/").await; + add_plugin_files(root, false).await; + assert_eq!( + fs::read_to_string(stamp_gitignore_path(root)) + .await + .unwrap(), + "vendor/\n/gem-plugin-stamp\n" + ); assert!(plugin_files_present(root).await); } } diff --git a/crates/socket-patch-core/src/setup/gem/templates/plugins.rb.tmpl b/crates/socket-patch-core/src/setup/gem/templates/plugins.rb.tmpl index 0c39c30b..97dcc87c 100644 --- a/crates/socket-patch-core/src/setup/gem/templates/plugins.rb.tmpl +++ b/crates/socket-patch-core/src/setup/gem/templates/plugins.rb.tmpl @@ -12,8 +12,9 @@ # # * plugin registration — the FIRST `bundle install` after `setup` (or on a # fresh clone) evaluates this file BEFORE any project gem is installed, so -# the load-time trigger is stamp-gated and quietly no-ops there; the -# install hooks below re-apply once the gems land. +# the load-time trigger is bootstrap-gated (no patch target exists yet) +# and quietly no-ops there; the install hooks below re-apply once the +# gems land. # * every `bundle install` — fresh AND fully cached — fires the per-gem # `after-install` events and then `after-install-all`; the forced # `after-install-all` re-apply is the actual patch point. @@ -31,12 +32,16 @@ # triggers: identical to the last applied state -> fast exit; otherwise shell # out and re-stamp. Folding the targets' actual content in means an # out-of-band reversion (pristine, manual edit, wiped bundle path) flips the -# digest even when every committed input is byte-identical. The stamp lives at -# .socket/gem-plugin-stamp — project-local state, safe to gitignore or delete -# (deleting only forces one re-probe). Older plugin versions stamped a -# fixed-name file under Bundler.bundle_path — the machine-global gem dir when -# no bundle path is configured, shared and clobbered across every project on -# the host — so that legacy stamp is deleted best-effort when seen. +# digest even when every committed input is byte-identical. The stamp is a +# pure digest cache at .socket/gem-plugin-stamp — machine-local state that +# `setup` keeps out of version control via .socket/.gitignore. Deleting it +# only forces one re-probe, and a stale copy that reaches version control +# anyway is harmless on a fresh clone: the bootstrap gate below keys on the +# patch targets existing on disk, never on the stamp. Older plugin versions +# stamped a fixed-name file under Bundler.bundle_path — the machine-global +# gem dir when no bundle path is configured, shared and clobbered across +# every project on the host — so that legacy stamp is deleted best-effort +# when seen. # # A patch failure NEVER breaks `bundle install`: it prints a warning naming # what failed and the remediation (run `socket-patch apply --ecosystems gem` @@ -189,8 +194,20 @@ module SocketPatch # gem exists, and raising there deadlocks the project on its own bootstrap # (plugin registration fails, so every retry fails identically). Warn once # per process with the remediation; SOCKET_PATCH_STRICT=1 restores the raise - # for builds that must not proceed unpatched. + # for builds that must not proceed unpatched. The trailer states what the + # ACTIVE mode does — the strict raise must not claim the install continues. + def failure_trailer + if strict? + "Failing `bundle install` because #{STRICT_ENV} is set; unset it to " \ + "warn and continue instead." + else + "`bundle install` continues; set #{STRICT_ENV}=1 to make patch " \ + "failures fatal." + end + end + def report_failure(message) + message = "#{message} #{failure_trailer}" if strict? raise(defined?(Bundler::BundlerError) ? Bundler::BundlerError.new(message) : message) end @@ -202,16 +219,20 @@ module SocketPatch # Idempotent applier behind every trigger. No manifest -> the project does # not use socket-patch, nothing to do. # force: skip the digest gate (the installer just changed the gem set). - # require_stamp: bail unless a previous apply stamped this project — the - # load-time trigger and the per-gem after-install hook use it so a - # bootstrap install's early evaluations don't shell out (and warn) before - # the target gems even exist; the forced after-install-all pass does the - # first real apply and lays down the stamp. - def apply!(force: false, require_stamp: false) + # bootstrap_gate: bail while NONE of the manifest's gem-patch targets exist + # on disk. The load-time trigger and the per-gem after-install hook use it + # so a bootstrap install's early evaluations (plugin REGISTRATION runs + # before any project gem lands) never shell out, warn, or — in strict mode — + # raise while there is nothing to patch; the forced after-install-all pass + # does the first real apply once the gems exist. The gate reads only the + # live gem tree, never the stamp: a stale stamp committed by mistake cannot + # re-open the bootstrap deadlock on a fresh clone, and deleting the stamp + # costs one re-probe instead of disabling these triggers. + def apply!(force: false, bootstrap_gate: false) APPLY_LOCK.synchronize do return unless File.file?(manifest_path) remove_legacy_stamp - return if require_stamp && !File.file?(stamp_path) + return if bootstrap_gate && patch_target_files.none? { |t| File.file?(t) } return if !force && stamped?(current_digest) ok = system( @@ -225,15 +246,14 @@ module SocketPatch "socket-patch: could not run `#{socket_bin} apply` — the gem patches in " \ ".socket/manifest.json are NOT applied. Install the socket-patch CLI (or set " \ "#{BIN_ENV} to its path), then run `socket-patch apply --ecosystems gem` " \ - "manually. `bundle install` continues; set #{STRICT_ENV}=1 to make this fatal." + "manually." ) return elsif !ok report_failure( "socket-patch: `#{socket_bin} apply --ecosystems gem` failed — the gem patches " \ "in .socket/manifest.json may NOT be applied. Run `socket-patch apply " \ - "--ecosystems gem` in #{project_root} to apply them manually. " \ - "`bundle install` continues; set #{STRICT_ENV}=1 to make patch failures fatal." + "--ecosystems gem` in #{project_root} to apply them manually." ) return end @@ -249,23 +269,25 @@ module SocketPatch end # Trigger 1 — load time. Runs at plugin registration and whenever a subscribed -# hook event first loads the plugin in a bundle process. Stamp-gated: on the -# bootstrap install nothing is stamped yet (and no gems exist to patch), so -# this quietly defers to Trigger 3. In strict mode a genuine patch failure -# (Bundler::BundlerError) still propagates. +# hook event first loads the plugin in a bundle process. Bootstrap-gated on +# the patch targets existing on disk (never on the stamp — a committed stale +# stamp must not re-open the registration deadlock): on the bootstrap install +# no gems exist to patch, so this quietly defers to Trigger 3. In strict mode +# a genuine patch failure (Bundler::BundlerError) still propagates. begin - SocketPatch.apply!(require_stamp: true) + SocketPatch.apply!(bootstrap_gate: true) rescue StandardError => e raise if defined?(Bundler::BundlerError) && e.is_a?(Bundler::BundlerError) end # Trigger 2 — after each individual gem (re)install. The only event bundler # fires during `bundle pristine`, so this is what catches pristine's patch -# reversion in the same run. Digest- and stamp-gated: on a fresh install's -# per-gem events the project is not stamped yet and Trigger 3 is about to do -# the real work. +# reversion in the same run — even when the stamp was deleted, since the gate +# reads the gem tree, not the stamp. Digest- and bootstrap-gated: on a fresh +# install's per-gem events the targets are only just landing and Trigger 3 is +# about to do the real work. Bundler::Plugin.add_hook("after-install") do |_spec_install| - SocketPatch.apply!(require_stamp: true) + SocketPatch.apply!(bootstrap_gate: true) end # Trigger 3 — after the installer finishes (fresh AND fully-cached installs). diff --git a/gem/socket-patch-bundler/plugins.rb b/gem/socket-patch-bundler/plugins.rb index a11c87c9..c62762f2 100644 --- a/gem/socket-patch-bundler/plugins.rb +++ b/gem/socket-patch-bundler/plugins.rb @@ -15,8 +15,8 @@ # # * plugin registration — the FIRST `bundle install` evaluates this file # BEFORE any project gem is installed, so the load-time trigger is -# stamp-gated and quietly no-ops there; the install hooks below re-apply -# once the gems land. +# bootstrap-gated (no patch target exists yet) and quietly no-ops there; +# the install hooks below re-apply once the gems land. # * every `bundle install` — fresh AND fully cached — fires the per-gem # `after-install` events and then `after-install-all`; the forced # `after-install-all` re-apply is the actual patch point. @@ -31,10 +31,13 @@ # # A digest of (manifest + every committed file under .socket/ + Gemfile.lock + # the on-disk content of every gem-patch target file) gates the non-forced -# triggers. The stamp lives at .socket/gem-plugin-stamp — project-local state, -# safe to gitignore or delete. Older plugin versions stamped a fixed-name file -# under Bundler.bundle_path (machine-global when no bundle path is -# configured); that legacy stamp is deleted best-effort when seen. +# triggers. The stamp is a pure digest cache at .socket/gem-plugin-stamp — +# machine-local state, safe to gitignore or delete (deleting only forces one +# re-probe); a stale copy that reaches version control anyway is harmless on +# a fresh clone, because the bootstrap gate keys on the patch targets +# existing on disk, never on the stamp. Older plugin versions stamped a +# fixed-name file under Bundler.bundle_path (machine-global when no bundle +# path is configured); that legacy stamp is deleted best-effort when seen. # # A patch failure NEVER breaks `bundle install`: it prints a warning naming # what failed and the remediation. Set SOCKET_PATCH_STRICT=1 to restore @@ -199,8 +202,20 @@ def remove_legacy_stamp # gem exists, and raising there deadlocks the project on its own bootstrap # (plugin registration fails, so every retry fails identically). Warn once # per process with the remediation; SOCKET_PATCH_STRICT=1 restores the raise - # for builds that must not proceed unpatched. + # for builds that must not proceed unpatched. The trailer states what the + # ACTIVE mode does — the strict raise must not claim the install continues. + def failure_trailer + if strict? + "Failing `bundle install` because #{STRICT_ENV} is set; unset it to " \ + "warn and continue instead." + else + "`bundle install` continues; set #{STRICT_ENV}=1 to make patch " \ + "failures fatal." + end + end + def report_failure(message) + message = "#{message} #{failure_trailer}" if strict? raise(defined?(Bundler::BundlerError) ? Bundler::BundlerError.new(message) : message) end @@ -212,16 +227,20 @@ def report_failure(message) # Idempotent applier behind every trigger. No manifest -> the project does # not use socket-patch, nothing to do. # force: skip the digest gate (the installer just changed the gem set). - # require_stamp: bail unless a previous apply stamped this project — the - # load-time trigger and the per-gem after-install hook use it so a - # bootstrap install's early evaluations don't shell out (and warn) before - # the target gems even exist; the forced after-install-all pass does the - # first real apply and lays down the stamp. - def apply!(force: false, require_stamp: false) + # bootstrap_gate: bail while NONE of the manifest's gem-patch targets exist + # on disk. The load-time trigger and the per-gem after-install hook use it + # so a bootstrap install's early evaluations (plugin REGISTRATION runs + # before any project gem lands) never shell out, warn, or — in strict mode — + # raise while there is nothing to patch; the forced after-install-all pass + # does the first real apply once the gems exist. The gate reads only the + # live gem tree, never the stamp: a stale stamp committed by mistake cannot + # re-open the bootstrap deadlock on a fresh clone, and deleting the stamp + # costs one re-probe instead of disabling these triggers. + def apply!(force: false, bootstrap_gate: false) APPLY_LOCK.synchronize do return unless File.file?(manifest_path) remove_legacy_stamp - return if require_stamp && !File.file?(stamp_path) + return if bootstrap_gate && patch_target_files.none? { |t| File.file?(t) } return if !force && stamped?(current_digest) ok = system( @@ -235,15 +254,14 @@ def apply!(force: false, require_stamp: false) "socket-patch: could not run `#{socket_bin} apply` — the gem patches in " \ ".socket/manifest.json are NOT applied. Install the socket-patch CLI (or set " \ "#{BIN_ENV} to its path), then run `socket-patch apply --ecosystems gem` " \ - "manually. `bundle install` continues; set #{STRICT_ENV}=1 to make this fatal." + "manually." ) return elsif !ok report_failure( "socket-patch: `#{socket_bin} apply --ecosystems gem` failed — the gem patches " \ "in .socket/manifest.json may NOT be applied. Run `socket-patch apply " \ - "--ecosystems gem` in #{project_root} to apply them manually. " \ - "`bundle install` continues; set #{STRICT_ENV}=1 to make patch failures fatal." + "--ecosystems gem` in #{project_root} to apply them manually." ) return end @@ -259,23 +277,25 @@ def apply!(force: false, require_stamp: false) end # Trigger 1 — load time. Runs at plugin registration and whenever a subscribed -# hook event first loads the plugin in a bundle process. Stamp-gated: on the -# bootstrap install nothing is stamped yet (and no gems exist to patch), so -# this quietly defers to Trigger 3. In strict mode a genuine patch failure -# (Bundler::BundlerError) still propagates. +# hook event first loads the plugin in a bundle process. Bootstrap-gated on +# the patch targets existing on disk (never on the stamp — a committed stale +# stamp must not re-open the registration deadlock): on the bootstrap install +# no gems exist to patch, so this quietly defers to Trigger 3. In strict mode +# a genuine patch failure (Bundler::BundlerError) still propagates. begin - SocketPatch.apply!(require_stamp: true) + SocketPatch.apply!(bootstrap_gate: true) rescue StandardError => e raise if defined?(Bundler::BundlerError) && e.is_a?(Bundler::BundlerError) end # Trigger 2 — after each individual gem (re)install. The only event bundler # fires during `bundle pristine`, so this is what catches pristine's patch -# reversion in the same run. Digest- and stamp-gated: on a fresh install's -# per-gem events the project is not stamped yet and Trigger 3 is about to do -# the real work. +# reversion in the same run — even when the stamp was deleted, since the gate +# reads the gem tree, not the stamp. Digest- and bootstrap-gated: on a fresh +# install's per-gem events the targets are only just landing and Trigger 3 is +# about to do the real work. Bundler::Plugin.add_hook("after-install") do |_spec_install| - SocketPatch.apply!(require_stamp: true) + SocketPatch.apply!(bootstrap_gate: true) end # Trigger 3 — after the installer finishes (fresh AND fully-cached installs). diff --git a/tests/setup_matrix/matrix.json b/tests/setup_matrix/matrix.json index c5b4c308..101316fe 100644 --- a/tests/setup_matrix/matrix.json +++ b/tests/setup_matrix/matrix.json @@ -155,8 +155,8 @@ }, { - "ecosystem": "gem", "pm": "bundler", "image": "gem", "hook_family": "none", - "baseline_supported": false, + "ecosystem": "gem", "pm": "bundler", "image": "gem", "hook_family": "bundler-plugin", + "baseline_supported": true, "package": "colorize", "version": "1.1.0", "purl": "pkg:gem/colorize@1.1.0", "manifest_key": "package/lib/colorize.rb", "apply_ecosystems": "gem" }, From c954115175514405c2497dfcd4b4c9de52d059fe Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Thu, 13 Aug 2026 18:48:35 -0700 Subject: [PATCH 5/5] fix(gem): normalize the platform-gem glob base to forward slashes Dir.glob treats backslash as an escape on EVERY platform, and Bundler.bundle_path carries Windows backslash separators through verbatim (verified: BUNDLE_PATH='vendor\bundle' yields /vendor\bundle/ruby/3.4.0). The platform-install wildcard in patch_target_files (/--*/) therefore escape-ate the separator and matched nothing on Windows: platform installs (nokogiri-1.15.0-x64-mingw-ucrt) dropped out of the digest, so a bundle pristine reversion of them left the stamp matching and the re-apply skipped. Fix: glob a slash-normalized base (forward slashes are valid separators on Windows); the direct non-glob join stays byte-faithful. Applied to both the setup template and the published gem twin, pinned by new needles in the core parity test. Regression test (verified red without the fix): plugin_runtime::backslash_bundle_path_still_digests_platform_gem_files drives the generated plugins.rb with plain ruby under a backslash-bearing BUNDLE_PATH while the real tree lives at the slash spelling (the two-spellings-one-directory situation Windows creates): the platform install must be enumerated as a patch target and its reversion must flip the digest and re-run apply. Co-Authored-By: Claude Fable 5 --- .../tests/setup_matrix_gem.rs | 116 ++++++++++++++++++ crates/socket-patch-core/src/setup/gem/mod.rs | 6 + .../src/setup/gem/templates/plugins.rb.tmpl | 10 +- gem/socket-patch-bundler/plugins.rb | 10 +- 4 files changed, 140 insertions(+), 2 deletions(-) diff --git a/crates/socket-patch-cli/tests/setup_matrix_gem.rs b/crates/socket-patch-cli/tests/setup_matrix_gem.rs index 3009687a..cb0c202a 100644 --- a/crates/socket-patch-cli/tests/setup_matrix_gem.rs +++ b/crates/socket-patch-cli/tests/setup_matrix_gem.rs @@ -932,6 +932,122 @@ mod plugin_runtime { ); } + /// [P2 Windows platform-gem glob] `Bundler.bundle_path` carries + /// backslash separators through verbatim (Windows spelling), and + /// `Dir.glob` treats `\` as an escape on EVERY platform — so the + /// platform-install wildcard (`/--*/`) built + /// from that base escape-eats the separator, matches nothing, and + /// platform installs (colorize-1.1.0-x64-mingw-ucrt) silently drop out + /// of the digest: a `bundle pristine` reversion of them leaves the stamp + /// matching and the re-apply skipped. Simulated on this host by feeding + /// bundler a backslash-bearing BUNDLE_PATH while the real tree lives at + /// the forward-slash spelling — exactly the two-spellings-one-directory + /// situation Windows creates (glob escape semantics are identical + /// everywhere). The plugin must glob a slash-normalized base; forward + /// slashes are valid separators on Windows. + #[test] + fn backslash_bundle_path_still_digests_platform_gem_files() { + if !have("ruby") { + eprintln!("skip plugin_runtime: ruby not on PATH"); + return; + } + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + scaffold(root); + let (fake, log) = write_fake_apply(root, 0); + + // Where bundler puts this project's gems under the backslash spelling. + let mut cmd = Command::new("ruby"); + cmd.args(["-e", "require \"bundler\"; print Bundler.bundle_path"]) + .current_dir(root); + scrub(&mut cmd); + cmd.env("BUNDLE_PATH", "vendor\\bundle"); + let (code, bundle_path, err) = run(cmd); + assert_eq!(code, 0, "Bundler.bundle_path probe failed: {err}"); + let bundle_path = bundle_path.trim().to_string(); + assert!( + bundle_path.contains('\\'), + "precondition: bundler must carry the backslash spelling through \ + verbatim (the Windows behavior this test simulates), got: \ + {bundle_path:?}" + ); + + // On Windows both spellings denote the SAME directory; materialize + // the real tree at the slash spelling — the one the normalized glob + // must reach from the backslash-bearing base. Only a PLATFORM install + // exists: the plain `colorize-1.1.0` direct join never globs and is + // not at issue. + let target = PathBuf::from(bundle_path.replace('\\', "/")) + .join("gems/colorize-1.1.0-x64-mingw-ucrt/lib/colorize.rb"); + std::fs::create_dir_all(target.parent().unwrap()).unwrap(); + std::fs::write(&target, "PATCHED PLATFORM CONTENT\n").unwrap(); + + let drive = |label: &str| { + let mut cmd = Command::new("ruby"); + cmd.args([ + "-e", + "require \"bundler\"; load ARGV[0]; SocketPatch.apply!", + "--", + ]) + .arg(root.join(".socket/bundler-plugin/plugins.rb")) + .current_dir(root); + scrub(&mut cmd); + cmd.env("BUNDLE_PATH", "vendor\\bundle"); + cmd.env("SOCKET_PATCH_BIN", &fake); + let (code, out, err) = run(cmd); + assert_eq!( + code, 0, + "{label}: driving the applier failed.\n{out}\n{err}" + ); + }; + + drive("initial apply"); + assert_eq!( + apply_calls(&log).len(), + 1, + "first drive must shell apply (nothing stamped yet)" + ); + assert!(root.join(STAMP_REL).is_file(), "stamp written"); + drive("stamped no-op"); + assert_eq!( + apply_calls(&log).len(), + 1, + "unchanged state must be digest-gated to a no-op" + ); + + // The pristine reversion the digest exists to catch — of the + // PLATFORM install this time. + std::fs::write(&target, "REVERTED BY PRISTINE\n").unwrap(); + drive("after platform-install reversion"); + assert_eq!( + apply_calls(&log).len(), + 2, + "reverting the platform gem install must flip the digest and \ + re-run apply — an escape-eaten glob omits platform installs \ + from the digest and skips this re-apply" + ); + + // And directly: the platform install is enumerated as a patch target. + let mut cmd = Command::new("ruby"); + cmd.args([ + "-e", + "require \"bundler\"; load ARGV[0]; puts SocketPatch.patch_target_files", + "--", + ]) + .arg(root.join(".socket/bundler-plugin/plugins.rb")) + .current_dir(root); + scrub(&mut cmd); + cmd.env("BUNDLE_PATH", "vendor\\bundle"); + cmd.env("SOCKET_PATCH_BIN", &fake); + let (code, out, err) = run(cmd); + assert_eq!(code, 0, "patch_target_files probe failed.\n{out}\n{err}"); + assert!( + out.contains("colorize-1.1.0-x64-mingw-ucrt"), + "patch_target_files must enumerate the platform install under a \ + backslash-bearing bundle path:\n{out}" + ); + } + fn walk(dir: &Path) -> Vec { let mut out = Vec::new(); let Ok(entries) = std::fs::read_dir(dir) else { diff --git a/crates/socket-patch-core/src/setup/gem/mod.rs b/crates/socket-patch-core/src/setup/gem/mod.rs index 5cfe2ea9..871aa399 100644 --- a/crates/socket-patch-core/src/setup/gem/mod.rs +++ b/crates/socket-patch-core/src/setup/gem/mod.rs @@ -507,6 +507,11 @@ mod tests { assert!(PLUGINS_RB.contains("Gemfile.lock")); assert!(PLUGINS_RB.contains("manifest.json")); assert!(PLUGINS_RB.contains("def patch_target_files")); + // The platform-gem wildcard must glob a forward-slash base: Dir.glob + // treats `\` as an escape on every platform, so a Windows bundle path + // (backslash separators) would otherwise never match platform installs + // and they would silently drop out of the digest. + assert!(PLUGINS_RB.contains(r#"glob_gems_dir = gems_dir.tr("\\", "/")"#)); // The gemspec names the plugin the Gemfile directive references. assert!(GEMSPEC.starts_with(GENERATED_MARKER)); assert!(GEMSPEC.contains("\"socket-patch\"")); @@ -583,6 +588,7 @@ mod tests { "STAMP_NAME = \"gem-plugin-stamp\"", "def remove_legacy_stamp", "def patch_target_files", + r#"glob_gems_dir = gems_dir.tr("\\", "/")"#, "bootstrap_gate && patch_target_files.none?", "Bundler::Plugin.add_hook(\"after-install\")", "Bundler::Plugin.add_hook(\"after-install-all\")", diff --git a/crates/socket-patch-core/src/setup/gem/templates/plugins.rb.tmpl b/crates/socket-patch-core/src/setup/gem/templates/plugins.rb.tmpl index 97dcc87c..4c5fc5b9 100644 --- a/crates/socket-patch-core/src/setup/gem/templates/plugins.rb.tmpl +++ b/crates/socket-patch-core/src/setup/gem/templates/plugins.rb.tmpl @@ -113,6 +113,14 @@ module SocketPatch end return [] unless records.is_a?(Hash) gems_dir = File.join(bundle_path, "gems") + # Dir.glob treats `\` as an escape on EVERY platform, so a Windows-style + # bundle path (Bundler.bundle_path carries backslash separators through + # verbatim) would never match the platform-gem wildcard below: platform + # installs (nokogiri-1.15.0-x64-mingw-ucrt) drop out of the digest and a + # `bundle pristine` reversion of them leaves the stamp matching. Forward + # slashes are valid separators on Windows, so normalize the GLOB BASE + # only — the direct join below is not a pattern and stays byte-faithful. + glob_gems_dir = gems_dir.tr("\\", "/") targets = [] records.each do |purl, record| next unless purl.is_a?(String) && purl.start_with?("pkg:gem/") @@ -124,7 +132,7 @@ module SocketPatch files.each_key do |key| rel = key.to_s.sub(%r{\Apackage/}, "") targets << File.join(gems_dir, "#{name}-#{version}", rel) - targets.concat(Dir.glob(File.join(gems_dir, "#{name}-#{version}-*", rel))) + targets.concat(Dir.glob(File.join(glob_gems_dir, "#{name}-#{version}-*", rel))) end end targets.uniq.sort diff --git a/gem/socket-patch-bundler/plugins.rb b/gem/socket-patch-bundler/plugins.rb index c62762f2..32b3bfdb 100644 --- a/gem/socket-patch-bundler/plugins.rb +++ b/gem/socket-patch-bundler/plugins.rb @@ -121,6 +121,14 @@ def patch_target_files end return [] unless records.is_a?(Hash) gems_dir = File.join(bundle_path, "gems") + # Dir.glob treats `\` as an escape on EVERY platform, so a Windows-style + # bundle path (Bundler.bundle_path carries backslash separators through + # verbatim) would never match the platform-gem wildcard below: platform + # installs (nokogiri-1.15.0-x64-mingw-ucrt) drop out of the digest and a + # `bundle pristine` reversion of them leaves the stamp matching. Forward + # slashes are valid separators on Windows, so normalize the GLOB BASE + # only — the direct join below is not a pattern and stays byte-faithful. + glob_gems_dir = gems_dir.tr("\\", "/") targets = [] records.each do |purl, record| next unless purl.is_a?(String) && purl.start_with?("pkg:gem/") @@ -132,7 +140,7 @@ def patch_target_files files.each_key do |key| rel = key.to_s.sub(%r{\Apackage/}, "") targets << File.join(gems_dir, "#{name}-#{version}", rel) - targets.concat(Dir.glob(File.join(gems_dir, "#{name}-#{version}-*", rel))) + targets.concat(Dir.glob(File.join(glob_gems_dir, "#{name}-#{version}-*", rel))) end end targets.uniq.sort