From ca10a02cb5472f1db2a1f16bb6cc939c23f6611e Mon Sep 17 00:00:00 2001 From: socket-patch agent Date: Wed, 28 Oct 2026 03:23:00 +0000 Subject: [PATCH 1/3] Start fix for #1761 Assisted-by: Claude Code:claude-opus-5-5 From 8de500d76e330fe5fe230f597485034046d04359 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 03:29:21 +0000 Subject: [PATCH 2/3] Preview gem refusals in vendor eject dry run `socket-patch vendor --dry-run` on a hosted Bundler project whose patched gem sits inside a `group ... do` block reported the eject as planned (exit 0), while the real `vendor` refused the same gem with `gemfile_declaration_not_editable` and rolled back (exit 1). CI gating on the dry run was told an eject would succeed that never could. The eject now asks the gem takeover gate (manifest and Gemfile declaration checks) for each gem pin before planning, as `scan` and `get --mode vendored` already do. Dry and wet runs both refuse the eject whole with `eject_refused`, and the wet run no longer restores and rolls back first. Fixes #1761 Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-cli/src/commands/vendor.rs | 15 +++ .../tests/e2e_redirect_gem_build.rs | 108 ++++++++++++++++++ 2 files changed, 123 insertions(+) diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index bd85b661f..68687169c 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -1850,12 +1850,27 @@ async fn run_eject(args: &VendorArgs, pins: Vec) -> i32 { let installed_sites = vendor::pypi::InstalledSiteListings::default(); let vendored_at = now_rfc3339(); let restore_refused: HashSet = refused.iter().map(|(p, _, _)| p.clone()).collect(); + // The gem backend's dry run does not evaluate its manifest and Gemfile + // declaration gates, so ask them directly, as the per-purl takeover + // and `scan` / `get --mode vendored` do (#1761). + let mut gem_refusals = gem_takeover_refusals_for( + &common.cwd, + pins.iter().map(|pin| pin.purl.as_str()), + &pins, + common.offline, + origins.clone(), + ) + .await; for pin in &pins { // A pin the restore already refused keeps its first, accurate // reason; previewing it again would add a second failure event. if restore_refused.contains(&pin.purl) { continue; } + if let Some((code, detail)) = gem_refusals.remove(&pin.purl) { + refused.push((pin.purl.clone(), code, detail)); + continue; + } if let Some((code, detail)) = takeover_dry_refusal( &plan, &pin.purl, diff --git a/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs b/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs index d7f382808..960a153fe 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs @@ -2588,6 +2588,114 @@ async fn gem_hosted_group_block_pin_survives_a_refused_vendored_takeover() { ); } } + // #1761: the standalone `vendor` eject of the same pin. Its dry run + // must preview the refusal the wet run raises, not `eject_planned`. + mount_vendored_grant(&fx).await; + for dry_run in [true, false] { + vendor_eject_keeps_the_hosted_group_pin(&fx, dry_run); + } +} + +/// #1761 on a CHECKSUMS-less lock that an unfrozen `bundle install` +/// converged (the lock now resolves the hosted gem from the patch +/// registry): `vendor --dry-run` previewed `eject_planned` (exit 0) while +/// the wet eject refused `gemfile_declaration_not_editable`. +#[tokio::test(flavor = "multi_thread")] +#[ignore = "host capstone: shells out to a real ruby/gem/bundler (>= 2.2); \ + the unpinned `test` job skips it, an e2e job with a pinned toolchain runs it via --ignored"] +async fn gem_hosted_group_block_pin_survives_a_refused_eject_of_a_converged_lock() { + let Some(fx) = redirect_scanned_project( + "group-block converged eject", + Spelling::Gemfile, + false, + true, + None, + Driver::ScanVexGroupBlock, + ) + .await + else { + return; + }; + if !fx.bundler.at_least(2, 2) { + println!( + "SKIP e2e_redirect_gem_build (group-block converged eject): bundler {} merges \ + every rubygems source into one GEM section", + fx.bundler.version + ); + return; + } + let install = bundle(&fx.proj, &["install"]); + assert!( + install.status.success(), + "converging install:\n{}", + String::from_utf8_lossy(&install.stderr) + ); + let lock = std::fs::read_to_string(fx.proj.join(fx.lock_name)).unwrap(); + assert!( + lock.contains(&format!("remote: {}", fx.index_url)), + "converged:\n{lock}" + ); + mount_vendored_grant(&fx).await; + for dry_run in [true, false] { + vendor_eject_keeps_the_hosted_group_pin(&fx, dry_run); + } +} + +/// One refused `vendor` eject of the group-block fixture (#1761): dry or +/// wet, the eject is refused whole with `gemfile_declaration_not_editable` +/// and `eject_refused` (exit 1), and the hosted `Gemfile` / +/// `Gemfile.lock` stay byte-untouched. +fn vendor_eject_keeps_the_hosted_group_pin(fx: &RedirectFixture, dry_run: bool) { + let before: Vec> = ["Gemfile", "Gemfile.lock"] + .iter() + .map(|f| std::fs::read(fx.proj.join(f)).unwrap()) + .collect(); + let proj = fx.proj.to_str().expect("utf8 tmp path"); + let api = fx._server.uri(); + let mut argv: Vec<&str> = vec![ + "vendor", + "--json", + "--yes", + "--cwd", + proj, + "--api-url", + &api, + "--org", + ORG, + "--api-token", + "fake", + "--patch-server-url", + &api, + ]; + if dry_run { + argv.push("--dry-run"); + } + let label = format!("vendor eject (dry_run={dry_run})"); + let upstream = format!("{api}/upstream"); + let (code, stdout, stderr) = + run_socket_env(&fx.proj, &argv, &[("SOCKET_RUBYGEMS_URL", &upstream)]); + let env: serde_json::Value = serde_json::from_str(&stdout) + .unwrap_or_else(|e| panic!("{label}: not JSON: {e}\nstdout:\n{stdout}\nstderr:\n{stderr}")); + assert_eq!(code, 1, "{label} must refuse: {env}\nstderr:\n{stderr}"); + let events = env["events"].as_array().expect("events"); + assert_eq!(events.len(), 1, "{label}: {env}"); + assert_eq!(events[0]["action"], "failed", "{label}: {env}"); + assert_eq!( + events[0]["errorCode"], "gemfile_declaration_not_editable", + "{label}: {env}" + ); + assert_eq!(env["error"]["code"], "eject_refused", "{label}: {env}"); + assert!( + !stdout.contains("eject_planned") && !stdout.contains("eject_rolled_back"), + "{label}: refused before any restore (or a preview of one): {env}" + ); + for (file, before) in ["Gemfile", "Gemfile.lock"].iter().zip(before) { + assert_eq!( + std::fs::read(fx.proj.join(file)).unwrap(), + before, + "{label}: {file} keeps its hosted wiring" + ); + } } /// #1056: a gem the user declared inside their own `source "…" do` block From 01c0895af264b2d032f4317e34a2c6cd086244ec Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 28 Oct 2026 03:48:33 +0000 Subject: [PATCH 3/3] Check gem edits in vendor dry runs instead Move the #1761 fix to the gem vendored backend: its dry run now computes the Gemfile and lock edits the wet run would make and returns the same refusal (for example a declaration inside a `group` block). Every dry-run caller benefits, not just the eject: a plain `vendor --dry-run` and the takeover previews over restored text now report what the wet run would refuse. This replaces the eject-only gate added in the previous commit, which checked each gem pin against its own one-pin restore and hid earlier backend refusals. Assisted-by: Claude Code:claude-opus-5-5 --- .../socket-patch-cli/src/commands/vendor.rs | 15 ----------- crates/socket-patch-core/src/vendor/gem.rs | 27 +++++++++++++++++++ 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/crates/socket-patch-cli/src/commands/vendor.rs b/crates/socket-patch-cli/src/commands/vendor.rs index 68687169c..bd85b661f 100644 --- a/crates/socket-patch-cli/src/commands/vendor.rs +++ b/crates/socket-patch-cli/src/commands/vendor.rs @@ -1850,27 +1850,12 @@ async fn run_eject(args: &VendorArgs, pins: Vec) -> i32 { let installed_sites = vendor::pypi::InstalledSiteListings::default(); let vendored_at = now_rfc3339(); let restore_refused: HashSet = refused.iter().map(|(p, _, _)| p.clone()).collect(); - // The gem backend's dry run does not evaluate its manifest and Gemfile - // declaration gates, so ask them directly, as the per-purl takeover - // and `scan` / `get --mode vendored` do (#1761). - let mut gem_refusals = gem_takeover_refusals_for( - &common.cwd, - pins.iter().map(|pin| pin.purl.as_str()), - &pins, - common.offline, - origins.clone(), - ) - .await; for pin in &pins { // A pin the restore already refused keeps its first, accurate // reason; previewing it again would add a second failure event. if restore_refused.contains(&pin.purl) { continue; } - if let Some((code, detail)) = gem_refusals.remove(&pin.purl) { - refused.push((pin.purl.clone(), code, detail)); - continue; - } if let Some((code, detail)) = takeover_dry_refusal( &plan, &pin.purl, diff --git a/crates/socket-patch-core/src/vendor/gem.rs b/crates/socket-patch-core/src/vendor/gem.rs index 787be553b..713cba07d 100644 --- a/crates/socket-patch-core/src/vendor/gem.rs +++ b/crates/socket-patch-core/src/vendor/gem.rs @@ -663,6 +663,14 @@ pub async fn vendor_gem<'a>( } if dry_run { + // An unwired gem previews the Gemfile and lock edit refusals the wet + // run raises (a declaration inside a `group` block, #1761), so a + // takeover or eject preview over the restored text refuses too. + if !lock_wired { + if let Err(outcome) = gem_edits(purl, &prelude) { + return *outcome; + } + } if let Err(outcome) = super::service_fetch::preview_service(service, record, extract_gem_data).await { @@ -3663,6 +3671,25 @@ mod tests { assert!(detail.contains("path:"), "{detail}"); } + /// #1761: a dry run raises the same declaration refusal as the wet run, + /// so a `vendor` eject preview does not report a refused gem as planned. + #[tokio::test] + async fn test_dry_run_previews_declaration_refusal() { + let grouped = + "source \"https://rubygems.org\"\n\ngroup :test do\n gem \"rack\", \"~> 3.1\"\nend\n"; + for dry_run in [true, false] { + let (_tmp, root, installed, blobs, record) = fixture(grouped, LOCK_DIRECT).await; + let (code, detail) = + unwrap_refused(run_vendor(&root, &blobs, &installed, &record, dry_run).await); + assert_eq!( + code, "gemfile_declaration_not_editable", + "dry_run={dry_run}" + ); + assert!(detail.contains("indented"), "dry_run={dry_run}: {detail}"); + assert!(!root.join(".socket").exists(), "dry_run={dry_run}"); + } + } + /// SECURITY: a traversal uuid (tampered manifest) must be refused before /// any disk access. #[tokio::test]