Fix gem pair model ignoring custom lockfile and Bundler 1 twins (#749, #751)

.github/workflows/ci.yml

@@ -1397,7 +1397,7 @@ jobs:
         # The composer capstones shell out to a real composer; `composer:`
         # pins the release line (1, 2.2 LTS, 2) so the composer.lock grammar
         # the edits assert stays stable across runners.
-        uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # v2
+        uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # 2.37.2
         with:
           php-version: '8.2'
           tools: composer:${{ matrix.composer }}

crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs

@@ -457,6 +457,17 @@ enum Driver {
     /// environment, so bundler still loads `Gemfile.next` and the run must
     /// still redirect and attest nothing.
     ScanVexDualBootEnvGemfile,
+    /// [`Driver::ScanVex`] on a bundler 4 project whose `.bundle/config`
+    /// sets `lockfile custom.lock` beside a leftover `Gemfile.lock` (#749):
+    /// bundler reads `custom.lock`, which the rewriter never pins, so the
+    /// run must redirect nothing and attest nothing. Bundler >= 4 only; the
+    /// fixture asserts the contract itself and yields `None`.
+    ScanVexCustomLockfile,
+    /// [`Driver::ScanVex`] on a `Gemfile` + `gems.rb` twin (#751): bundler
+    /// 1.x loads the `Gemfile` and >= 2 loads `gems.rb`, and the scan cannot
+    /// see which runs, so it must redirect and attest nothing and leave all
+    /// four files byte-identical. Every bundler line.
+    ScanVexTwin,
     /// [`Driver::ScanVex`] on a Gemfile that declares the gem inside a
     /// `group :development do … end` block (#775): hosted mode wraps it in
     /// a source block inside the group, but vendored mode cannot edit an
@@ -508,6 +519,8 @@ impl Driver {
             Driver::ScanVexDualBootEnvGemfile => {
                 "scan --mode hosted (config Gemfile.next, env BUNDLE_GEMFILE=Gemfile)"
             }
+            Driver::ScanVexCustomLockfile => "scan --mode hosted (lockfile custom.lock)",
+            Driver::ScanVexTwin => "scan --mode hosted (Gemfile + gems.rb twin)",
             Driver::ScanVexGroupBlock => "scan --mode hosted (gem in a group block)",
             Driver::ScanVexSemicolonJoinedDeclaration => {
                 "scan --mode hosted (two `;`-joined gem declarations)"
@@ -622,6 +635,20 @@ async fn redirect_scanned_project(
     let bundler = bundler_e2e::gate("e2e_redirect_gem_build", tag, floor, &|c| {
         cache_env::isolate(c);
     })?;
+    // Drivers that only mean something on one bundler line.
+    let only = match driver {
+        Driver::ScanVexCustomLockfile if !bundler.at_least(4, 0) => {
+            Some("custom lockfiles need bundler >= 4")
+        }
+        _ => None,
+    };
+    if let Some(why) = only {
+        println!(
+            "SKIP e2e_redirect_gem_build ({tag}): bundler {}: {why}",
+            bundler.version
+        );
+        return None;
+    }
 
     let tmp = tempfile::tempdir().unwrap();
     let (gemfile_name, lock_name) = spelling.pair();
@@ -975,6 +1002,27 @@ async fn redirect_scanned_project(
             String::from_utf8_lossy(&cfg.stderr)
         );
     }
+    let custom_lockfile = driver == Driver::ScanVexCustomLockfile;
+    if custom_lockfile {
+        // `bundle config set --local lockfile custom.lock`; the default
+        // lock stays behind as a leftover bundler 4 ignores.
+        std::fs::copy(proj.join(lock_name), proj.join("custom.lock")).unwrap();
+        let args = bundler.config_local_args("lockfile", "custom.lock");
+        let args: Vec<&str> = args.iter().map(String::as_str).collect();
+        let cfg = bundle(&proj, &args);
+        assert!(
+            cfg.status.success(),
+            "bundle config set --local lockfile failed:\n{}",
+            String::from_utf8_lossy(&cfg.stderr)
+        );
+    }
+    let twin = driver == Driver::ScanVexTwin;
+    if twin {
+        // Identical twins: which pair installs depends only on the bundler
+        // that runs.
+        std::fs::copy(proj.join(gemfile_name), proj.join("gems.rb")).unwrap();
+        std::fs::copy(proj.join(lock_name), proj.join("gems.locked")).unwrap();
+    }
     // Synthetic credentials must never appear in the scan's automatic
     // diagnostics. The loopback mirror itself serves the unpatched gem.
     let mirror = format!("{}/upstream/", server.uri()).replacen(
@@ -1007,6 +1055,8 @@ async fn redirect_scanned_project(
         | Driver::ScanVexMirrorSource
         | Driver::ScanVexMirrorSourceEnv
         | Driver::ScanVexMirrorAllEnv
+        | Driver::ScanVexCustomLockfile
+        | Driver::ScanVexTwin
         | Driver::ScanVexDualBoot
         | Driver::ScanVexDualBootEnvGemfile
         | Driver::ScanVexDuplicateDeclaration
@@ -1082,6 +1132,35 @@ async fn redirect_scanned_project(
         assert_dual_boot_redirects_nothing(&env, &proj, &pristine_gemfile, &pristine_lock);
         return None;
     }
+    if custom_lockfile {
+        assert_custom_lockfile_redirects_nothing(
+            &bundler,
+            "redirect_gem_bundle_lockfile_unsupported",
+            (code, &stdout, &stderr),
+            &proj,
+            &[
+                ("Gemfile", &pristine_gemfile),
+                ("Gemfile.lock", &pristine_lock),
+                ("custom.lock", &pristine_lock),
+            ],
+        );
+        return None;
+    }
+    if twin {
+        assert_custom_lockfile_redirects_nothing(
+            &bundler,
+            "redirect_gem_twin_manifest_ambiguous",
+            (code, &stdout, &stderr),
+            &proj,
+            &[
+                ("Gemfile", &pristine_gemfile),
+                ("Gemfile.lock", &pristine_lock),
+                ("gems.rb", &pristine_gemfile),
+                ("gems.locked", &pristine_lock),
+            ],
+        );
+        return None;
+    }
     if let Some(warning) = match driver {
         Driver::ScanVexDuplicateDeclaration => Some("redirect_gem_declared_more_than_once"),
         Driver::ScanVexEvalGemfile => Some("redirect_gem_declaration_not_visible"),
@@ -1194,6 +1273,8 @@ async fn redirect_scanned_project(
         }
         Driver::ScanVexDualBoot
         | Driver::ScanVexDualBootEnvGemfile
+        | Driver::ScanVexCustomLockfile
+        | Driver::ScanVexTwin
         | Driver::ScanVexDuplicateDeclaration
         | Driver::ScanVexEvalGemfile
         | Driver::ScanVexMirrorAll
@@ -1306,6 +1387,56 @@ fn assert_unwirable_declaration_redirects_nothing(
     );
 }
 
+/// The contract of a hosted scan that must refuse every gem: #749's
+/// `custom.lock` named in `.bundle/config` (bundler 4), or #751's
+/// `Gemfile` + `gems.rb` twin. The scan reports `refusal`, redirects and
+/// attests nothing, leaves every one of `files` byte-identical, and bundler
+/// still installs the untouched project frozen.
+fn assert_custom_lockfile_redirects_nothing(
+    bundler: &bundler_e2e::Bundler,
+    refusal: &str,
+    (code, stdout, stderr): (i32, &str, &str),
+    proj: &Path,
+    files: &[(&str, &[u8])],
+) {
+    let env: serde_json::Value = serde_json::from_str(stdout)
+        .unwrap_or_else(|e| panic!("not JSON: {e}\nstdout:\n{stdout}\nstderr:\n{stderr}"));
+    let warning_codes: Vec<&str> = env["redirect"]["warnings"]
+        .as_array()
+        .map(|a| a.iter().filter_map(|w| w["code"].as_str()).collect())
+        .unwrap_or_default();
+    assert!(
+        warning_codes.contains(&refusal),
+        "the {refusal} refusal must be reported: {env}"
+    );
+    assert_ne!(code, 0, "nothing was patched or attested: {env}");
+    assert_eq!(
+        env["redirect"]["redirected"], 0,
+        "nothing redirected: {env}"
+    );
+    assert!(
+        env["vex"]["statements"].as_u64().unwrap_or(0) == 0,
+        "no in-run attestation for a lock that was never pinned: {env}"
+    );
+    for (file, want) in files {
+        assert_eq!(
+            std::fs::read(proj.join(file)).unwrap(),
+            *want,
+            "{file} must be byte-untouched"
+        );
+    }
+    let args = bundler.config_local_args("frozen", "true");
+    let args: Vec<&str> = args.iter().map(String::as_str).collect();
+    assert!(bundle(proj, &args).status.success());
+    let install = bundle(proj, &["install"]);
+    assert!(
+        install.status.success(),
+        "bundler {} must still install the untouched project frozen:\n{}",
+        bundler.version,
+        String::from_utf8_lossy(&install.stderr)
+    );
+}
+
 /// #390's contract on a `BUNDLE_GEMFILE: Gemfile.next` project: the hosted
 /// scan names the setting, rewrites neither the `Gemfile` pair (which
 /// bundler ignores) nor `Gemfile.next`, and its in-run VEX attests nothing.
@@ -2065,6 +2196,49 @@ async fn gem_hosted_bundle_gemfile_dual_boot_redirects_nothing() {
     assert!(fx.is_none(), "the dual-boot driver asserts in place");
 }
 
+/// #749: bundler 4's `bundle config set --local lockfile custom.lock`
+/// makes bundler read `custom.lock`. The hosted scan used to wire the
+/// Gemfile (and the ignored leftover `Gemfile.lock`), report success and
+/// attest, while every frozen install then failed; it must redirect and
+/// attest nothing.
+#[tokio::test(flavor = "multi_thread")]
+#[ignore = "host capstone: shells out to a real ruby/gem/bundler (>= 4.0 for this arm); \
+            the unpinned `test` job skips it, an e2e job with a pinned toolchain runs it via --ignored"]
+async fn gem_hosted_bundler4_custom_lockfile_redirects_nothing() {
+    let fx = redirect_scanned_project(
+        "custom-lockfile",
+        Spelling::Gemfile,
+        true,
+        true,
+        None,
+        Driver::ScanVexCustomLockfile,
+    )
+    .await;
+    assert!(fx.is_none(), "the custom-lockfile driver asserts in place");
+}
+
+/// #751: bundler 1.x loads a twin's `Gemfile` and bundler >= 2 its
+/// `gems.rb`. The hosted scan used to wire `gems.rb` and attest while
+/// bundler 1.17 installed the unpatched gem from the `Gemfile`; since a
+/// lock's `BUNDLED WITH` does not say which bundler installs, it must
+/// refuse the twin on every bundler line, and the untouched twin must
+/// still install.
+#[tokio::test(flavor = "multi_thread")]
+#[ignore = "host capstone: shells out to a real ruby/gem/bundler (>= 1.17); \
+            the unpinned `test` job skips it, an e2e job with a pinned toolchain runs it via --ignored"]
+async fn gem_hosted_twin_redirects_nothing() {
+    let fx = redirect_scanned_project(
+        "twin",
+        Spelling::Gemfile,
+        false,
+        true,
+        None,
+        Driver::ScanVexTwin,
+    )
+    .await;
+    assert!(fx.is_none(), "the twin driver asserts in place");
+}
+
 /// #548: a gem declared in two `group` blocks must not be half-rewritten
 /// (bundler refuses `= 1.0.0` next to `>= 0` on every install).
 #[tokio::test(flavor = "multi_thread")]

crates/socket-patch-cli/tests/hosted_memory_engine.rs

@@ -991,6 +991,150 @@ async fn a_vlt_project_is_withheld_as_offline() {
     assert!(output.changed_files.is_empty());
 }
 
+const GEM_BASIC_FIXTURE: &str = "redirect/gem/bundler/basic";
+
+/// The gem fixture's API mocks and input files.
+async fn gem_server_and_input() -> (MockServer, BTreeMap<String, Vec<u8>>) {
+    let server = MockServer::start().await;
+    let patches = patches_from_overrides(
+        &fixtures_root()
+            .join(GEM_BASIC_FIXTURE)
+            .join("overrides.json"),
+        None,
+    );
+    mount_api(&server, &patches).await;
+    let input = fixture_files(&fixtures_root().join(GEM_BASIC_FIXTURE).join("input"));
+    (server, input)
+}
+
+fn warning_codes(redirect: &Value) -> Vec<String> {
+    redirect["warnings"]
+        .as_array()
+        .unwrap()
+        .iter()
+        .map(|w| w["code"].as_str().unwrap().to_string())
+        .collect()
+}
+
+fn changed_paths(output: &HostedScanOutput) -> Vec<&str> {
+    output
+        .changed_files
+        .iter()
+        .map(|f| f.path.as_str())
+        .collect()
+}
+
+/// #749: bundler 4 reads the lock `bundle config set lockfile custom.lock`
+/// names, which the rewriter never pins. With a leftover `Gemfile.lock`
+/// beside it, the run used to rewrite that ignored lock, report success,
+/// and break every frozen install; the project is refused with nothing
+/// written instead. A memory tree only finds gem candidates through the
+/// lock bundler loads (#736), and that lock is none of the default ones
+/// here, so the project yields no gem candidate; the run reports
+/// `gem_lock_unsupported` instead (the per-candidate
+/// `redirect_gem_bundle_lockfile_unsupported` refusal is covered by the
+/// engine unit tests).
+#[tokio::test]
+async fn a_bundler4_custom_lockfile_is_refused() {
+    let (server, input) = gem_server_and_input().await;
+    let mut files = input.clone();
+    files.insert("custom.lock".to_string(), input["Gemfile.lock"].clone());
+    files.insert(
+        ".bundle/config".to_string(),
+        b"---\nBUNDLE_LOCKFILE: \"custom.lock\"\n".to_vec(),
+    );
+    let output = run_engine(&server, build_input(&files, &[], options(false))).await;
+    let project = &output.projects[0];
+    assert!(project.error.is_none(), "{:?}", project.error);
+    assert!(project.redirected.is_empty(), "{:?}", project.redirected);
+    assert!(
+        changed_paths(&output).is_empty(),
+        "{:?}",
+        changed_paths(&output)
+    );
+    assert_gem_lock_unsupported(&output);
+}
+
+/// The run says the project's gems were not scanned, rather than finding
+/// none: the lock inventory's `gem_lock_unsupported` diagnosis.
+fn assert_gem_lock_unsupported(output: &HostedScanOutput) {
+    let codes: Vec<&str> = output.warnings.iter().map(|w| w.code.as_str()).collect();
+    assert!(codes.contains(&"gem_lock_unsupported"), "{codes:?}");
+}
+
+/// #749: a configured lockfile naming the pair's own default lock is the
+/// lock the rewriter pins anyway, so the project is wired as usual.
+#[tokio::test]
+async fn a_lockfile_setting_naming_the_default_lock_is_wired() {
+    let (server, input) = gem_server_and_input().await;
+    let mut files = input.clone();
+    files.insert(
+        ".bundle/config".to_string(),
+        b"---\nBUNDLE_LOCKFILE: \"Gemfile.lock\"\n".to_vec(),
+    );
+    let output = run_engine(&server, build_input(&files, &[], options(false))).await;
+    let project = &output.projects[0];
+    assert_eq!(
+        project.redirected.len(),
+        1,
+        "{:?}",
+        warning_codes(&project.redirect)
+    );
+    assert_eq!(changed_paths(&output), vec!["Gemfile", "Gemfile.lock"]);
+}
+
+/// The fixture lock re-stamped `BUNDLED WITH <version>`.
+fn bundled_with(lock: &[u8], version: &str) -> Vec<u8> {
+    let text = String::from_utf8(lock.to_vec()).unwrap();
+    let (head, _) = text.split_once("BUNDLED WITH").unwrap();
+    format!("{head}BUNDLED WITH\n   {version}\n").into_bytes()
+}
+
+/// A `Gemfile` + `gems.rb` twin with each lock bundled by `versions`.
+fn gem_twin(
+    input: &BTreeMap<String, Vec<u8>>,
+    versions: (&str, &str),
+) -> BTreeMap<String, Vec<u8>> {
+    let mut files = BTreeMap::new();
+    files.insert("Gemfile".to_string(), input["Gemfile"].clone());
+    files.insert("gems.rb".to_string(), input["Gemfile"].clone());
+    files.insert(
+        "Gemfile.lock".to_string(),
+        bundled_with(&input["Gemfile.lock"], versions.0),
+    );
+    files.insert(
+        "gems.locked".to_string(),
+        bundled_with(&input["Gemfile.lock"], versions.1),
+    );
+    files
+}
+
+/// #751: bundler 1.x loads a twin's `Gemfile` and bundler >= 2 its
+/// `gems.rb`, and a lock's `BUNDLED WITH` records who wrote it, not who
+/// installs it, so no twin is wired whatever its locks say: refused,
+/// nothing written. No lock is the one bundler loads (#736), so the
+/// memory tree yields no gem candidate and the run reports
+/// `gem_lock_unsupported`; the per-candidate
+/// `redirect_gem_twin_manifest_ambiguous` refusal is covered by the engine
+/// unit tests.
+#[tokio::test]
+async fn a_gemfile_gems_rb_twin_is_refused() {
+    let (server, input) = gem_server_and_input().await;
+    for versions in [
+        ("1.17.3", "1.17.3"),
+        ("2.6.2", "2.6.2"),
+        ("1.17.3", "2.6.2"),
+        ("2.6.2", "1.17.3"),
+    ] {
+        let files = gem_twin(&input, versions);
+        let output = run_engine(&server, build_input(&files, &[], options(false))).await;
+        let project = &output.projects[0];
+        assert!(project.redirected.is_empty(), "{versions:?}");
+        assert!(changed_paths(&output).is_empty(), "{versions:?}");
+        assert_gem_lock_unsupported(&output);
+    }
+}
+
 /// #736: the engine's purl set comes from the lock bundler loads. A
 /// `gems.rb` project's gems live in `gems.locked`; reading only
 /// `Gemfile.lock` found nothing to redirect, and a leftover `Gemfile.lock`