diff --git a/crates/socket-patch-cli/src/commands/scan/hosted.rs b/crates/socket-patch-cli/src/commands/scan/hosted.rs index a3b24ef8..524865e2 100644 --- a/crates/socket-patch-cli/src/commands/scan/hosted.rs +++ b/crates/socket-patch-cli/src/commands/scan/hosted.rs @@ -275,12 +275,33 @@ pub(super) async fn run_redirect( .as_ref() .and_then(|o| o.identifiers.go_module_path.clone()), )); + // The grant token is never a top-level reference field — it only + // rides the URLs the reference endpoint hands back, as the path + // level before the patch uuid. Recover it so the rewriters' + // rotation-idempotency guards (which wildcard the token path + // level of a previously-written URL) don't depend on it being + // derivable from the URL alone: with an empty token the gem + // guard used to miss the previous grant's source block and NEST + // a new one around it on every re-scan. + let token = reference + .registry_override + .as_ref() + .and_then(|o| { + socket_patch_core::patch::redirect::grant_token_path_segment( + &o.index_url, + &sel.uuid, + ) + }) + .or_else(|| { + socket_patch_core::patch::redirect::grant_token_path_segment(&url, &sel.uuid) + }) + .unwrap_or_default(); overrides.push(DepOverride { ecosystem, name, namespace: None, version, - token: String::new(), + token, patch_uuid: sel.uuid.clone(), artifact_url: url, berry_zip_url: berry_zip.and_then(|a| a.url.clone()), 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 b017f277..47171330 100644 --- a/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs +++ b/crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs @@ -47,16 +47,19 @@ //! exact live-CI signature — so any server or fixture that stops declaring //! runtime deps turns this suite red. //! -//! KNOWN LIMITATION, pinned as a canary: on a lock that carries a CHECKSUMS -//! section (bundler >= 4 writes one by default), today's rewrite (Gemfile -//! block + CHECKSUMS pin, GEM section left on the upstream remote) makes the -//! prescribed unfrozen install fail with "Bundler found mismatched checksums" -//! — bundler still attributes the gem to the upstream source and refuses the -//! lockfile-vs-API disagreement (exit 37, verified on bundler 4.0.15). The -//! canary test pins that reality; the verified fix shape is the fully -//! converged lock (patched-registry GEM section + ` (= )!` -//! DEPENDENCIES pin + patched CHECKSUMS sha — a frozen install of that shape -//! passes), which must land in the TS twin + golden fixtures together. +//! CHECKSUMS locks (bundler >= 4 writes the section by default) come out +//! FULLY CONVERGED: patch-registry GEM section holding the dep's spec, +//! ` (= )!` DEPENDENCIES pin, patched CHECKSUMS sha. The flipped +//! canary proves the converged pair installs patched bytes on a fresh +//! checkout both FROZEN (`BUNDLE_FROZEN=true`, lock byte-identical — no +//! unfrozen two-step) and unfrozen (the historical exit 37 "mismatched +//! checksums" mixed-state refusal is gone; it was pinned here as a known +//! limitation until the converged rewrite landed). The depscan TS twin +//! (registry-rewrite gem.ts) must be ported to match. +//! +//! The grant-rotation capstone drives token A -> A -> B re-scans through the +//! real binary: byte-idempotent under the same grant, in-place URL refresh +//! (Gemfile source block + converged-lock remote) under a rotated one. //! //! Skips (with a println) when `ruby`/`gem`/`bundle` are missing or the host //! bundler predates 2.6 (the CHECKSUMS-aware floor); everything after that is @@ -168,6 +171,13 @@ fn run_socket(cwd: &Path, args: &[&str]) -> (i32, String, String) { /// cold (the fresh-checkout install must be forced through the wiremock /// registry, never satisfied from the scan project's cache). fn bundle(cwd: &Path, args: &[&str]) -> Output { + bundle_env(cwd, args, &[]) +} + +/// `bundle` with extra environment on top of the isolated surface — e.g. +/// `BUNDLE_FROZEN=true` for bundler's frozen/deployment contract (exit 16 on +/// any Gemfile-vs-lock drift, lock never written). +fn bundle_env(cwd: &Path, args: &[&str], envs: &[(&str, &str)]) -> Output { let mut cmd = Command::new("bundle"); cmd.args(args).current_dir(cwd); for (k, _) in std::env::vars_os() { @@ -179,6 +189,9 @@ fn bundle(cwd: &Path, args: &[&str]) -> Output { cache_env::isolate(&mut cmd); cmd.env("BUNDLE_APP_CONFIG", cwd.join(".bundle")); cmd.env("BUNDLE_USER_HOME", cwd.join(".bundle-user-home")); + for (k, v) in envs { + cmd.env(k, v); + } cmd.output().expect("failed to run bundle") } @@ -348,6 +361,77 @@ impl Spelling { } } +/// The Socket patches API reference endpoint for one grant token: granted, +/// carrying the rubygems-compact-index registry override (the identifier +/// shape the TS reference builder emits — name / version / +/// gemChecksumSha256). `limit` caps how many requests this grant answers +/// (wiremock falls through to later-mounted mocks after that), which is how +/// the rotation tests hand out token A first and the rotated token B after. +async fn mount_reference_mock( + server: &MockServer, + token: &str, + patched_sha: &str, + limit: Option, +) { + let hosted_url = format!( + "{}/patch/gem/{DEP}/{DEP_VERSION}/{token}/{UUID}/{DEP}-{DEP_VERSION}.gem", + server.uri() + ); + let index_url = format!("{}/patch-registry/gem/{token}/{UUID}/", server.uri()); + let mock = Mock::given(method("POST")) + .and(path(format!("/v0/orgs/{ORG}/patches/package"))) + .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ + "results": { + UUID: { + "status": "granted", + "url": hosted_url, + "purl": PURL, + "artifacts": [{ + "kind": "tarball", + "url": hosted_url, + "integrity": { "sha256": patched_sha } + }], + "registryOverride": { + "kind": "rubygems-compact-index", + "indexUrl": index_url, + "identifiers": { + "name": DEP, + "version": DEP_VERSION, + "gemChecksumSha256": patched_sha, + } + } + } + } + }))); + match limit { + Some(n) => mock.up_to_n_times(n).mount(server).await, + None => mock.mount(server).await, + } +} + +/// A bare `scan --mode hosted --json` re-scan (no VEX legs) — what a periodic +/// or CI re-run looks like. +fn run_hosted_scan(proj: &Path, api: &str) -> (i32, String, String) { + run_socket( + proj, + &[ + "scan", + "--mode", + "hosted", + "--json", + "--yes", + "--cwd", + proj.to_str().expect("utf8 tmp path"), + "--api-url", + api, + "--org", + ORG, + "--api-token", + "fake", + ], + ) +} + /// Build the hermetic fixture and run `scan --mode hosted` through the real /// binary: author + `gem build` the three gems, mount both compact indexes /// and the patches API, `bundle install` from the mock upstream, scan, and @@ -355,12 +439,17 @@ impl Spelling { /// fixture lock into a CHECKSUMS section (`bundle lock --add-checksums`); /// `registry_declares_deps` toggles the patch registry's `/info` between the /// CORRECT contract (runtime deps declared) and today's production-like -/// deps-less answer. `None` = skip (message already printed). +/// deps-less answer. `rotated_token` = Some(token B) arms a grant-rotation +/// plan: the `TOKEN` grant answers the first two reference calls, token B +/// (same uuid) every later one, and the patch registry serves both token +/// paths (production keeps a grant alive until it expires). `None` = skip +/// (message already printed). async fn redirect_scanned_project( tag: &str, spelling: Spelling, checksums_lock: bool, registry_declares_deps: bool, + rotated_token: Option<&str>, ) -> Option { for cmd in ["ruby", "gem", "bundle"] { if !has_command(cmd) { @@ -444,17 +533,13 @@ async fn redirect_scanned_project( } else { vec![] }, - gem: patched_gem, + gem: patched_gem.clone(), }], ) .await; let orig = orig_lib().into_bytes(); let patched = patched_lib().into_bytes(); - let hosted_url = format!( - "{}/patch/gem/{DEP}/{DEP_VERSION}/{TOKEN}/{UUID}/{DEP}-{DEP_VERSION}.gem", - server.uri() - ); // Batch discovery: the crawled gem has one free patch. Mock::given(method("POST")) .and(path(format!("/v0/orgs/{ORG}/patches/batch"))) @@ -489,34 +574,29 @@ async fn redirect_scanned_project( .await; // Reference endpoint: granted, carrying the rubygems-compact-index // registry override (the identifier shape the TS reference builder - // emits — name / version / gemChecksumSha256). - Mock::given(method("POST")) - .and(path(format!("/v0/orgs/{ORG}/patches/package"))) - .respond_with(ResponseTemplate::new(200).set_body_json(serde_json::json!({ - "results": { - UUID: { - "status": "granted", - "url": hosted_url, - "purl": PURL, - "artifacts": [{ - "kind": "tarball", - "url": hosted_url, - "integrity": { "sha256": patched_sha } - }], - "registryOverride": { - "kind": "rubygems-compact-index", - "indexUrl": index_url, - "identifiers": { - "name": DEP, - "version": DEP_VERSION, - "gemChecksumSha256": patched_sha, - } - } - } - } - }))) - .mount(&server) + // emits — name / version / gemChecksumSha256). With a rotation plan the + // first grant answers exactly twice (scan 1 + the same-grant re-scan), + // then the rotated grant takes over — production rotates the token path + // segment per request. + mount_reference_mock(&server, TOKEN, &patched_sha, rotated_token.map(|_| 2)).await; + if let Some(token_b) = rotated_token { + mount_compact_index( + &server, + &format!("/patch-registry/gem/{token_b}/{UUID}"), + &[IndexGem { + name: DEP, + version: DEP_VERSION, + deps: if registry_declares_deps { + vec![format!("{TRANSITIVE}:>= 0")] + } else { + vec![] + }, + gem: patched_gem.clone(), + }], + ) .await; + mount_reference_mock(&server, token_b, &patched_sha, None).await; + } // View endpoint: the patch record (REAL before/after hashes of the // authored vs patched lib) the redirect run persists for VEX. Mock::given(method("GET")) @@ -668,16 +748,23 @@ async fn redirect_scanned_project( .iter() .filter_map(|w| w["code"].as_str()) .collect(); - assert!( - warning_codes.contains(&"redirect_gem_frozen_install"), - "the frozen-install caveat must be surfaced: {env}" - ); if checksums_lock { + // CHECKSUMS-era locks converge (patch-registry GEM section + + // dependency pin + patched sha), so the pair is frozen-installable + // as written — the caveat would be a lie. + assert!( + !warning_codes.contains(&"redirect_gem_frozen_install"), + "a converged CHECKSUMS pair must not carry the frozen-install caveat: {env}" + ); assert!( rewritten.contains(&lock_name), "the CHECKSUMS pin must land in {lock_name}: {env}" ); } else { + assert!( + warning_codes.contains(&"redirect_gem_frozen_install"), + "the frozen-install caveat must be surfaced on a mixed (no-CHECKSUMS) pair: {env}" + ); assert!( warning_codes.contains(&"redirect_gem_no_checksums_section"), "a no-CHECKSUMS lock cannot be pinned and must say so: {env}" @@ -729,12 +816,10 @@ async fn redirect_scanned_project( }) } -/// New dir holding ONLY what a git checkout would carry — the manifest pair, -/// `.socket/`, `.bundle/` — then the UNFROZEN `bundle install` the rewriter's -/// `redirect_gem_frozen_install` warning prescribes, with a cold per-dir -/// bundler home. Returns the fresh dir and the install output. -fn fresh_checkout_bundle_install(fx: &RedirectFixture) -> (PathBuf, Output) { - let fresh = fx.tmp.path().join("fresh"); +/// New dir named `name` holding ONLY what a git checkout would carry — the +/// manifest pair, `.socket/`, `.bundle/` — with a cold per-dir bundler home. +fn stage_fresh_checkout(fx: &RedirectFixture, name: &str) -> PathBuf { + let fresh = fx.tmp.path().join(name); std::fs::create_dir_all(&fresh).unwrap(); std::fs::copy(fx.proj.join(fx.gemfile_name), fresh.join(fx.gemfile_name)).unwrap(); std::fs::copy(fx.proj.join(fx.lock_name), fresh.join(fx.lock_name)).unwrap(); @@ -744,6 +829,13 @@ fn fresh_checkout_bundle_install(fx: &RedirectFixture) -> (PathBuf, Output) { !fresh.join("vendor").exists(), "fresh checkout must not carry an installed tree (test bug)" ); + fresh +} + +/// Fresh checkout + the UNFROZEN `bundle install` the redirect prescribes on +/// a not-yet-converged lock. Returns the fresh dir and the install output. +fn fresh_checkout_bundle_install(fx: &RedirectFixture) -> (PathBuf, Output) { + let fresh = stage_fresh_checkout(fx, "fresh"); let install = bundle(&fresh, &["install"]); (fresh, install) } @@ -829,7 +921,8 @@ fn assert_patched_install(fx: &RedirectFixture, fresh: &Path) { #[ignore = "host capstone: shells out to a real ruby/gem/bundler >= 2.6; the unpinned `test` \ job skips it, an e2e job with a pinned toolchain runs it via --ignored"] async fn gem_hosted_fresh_checkout_bundle_install_installs_patched_bytes_and_vex_verifies() { - let Some(fx) = redirect_scanned_project("main", Spelling::Gemfile, false, true).await else { + let Some(fx) = redirect_scanned_project("main", Spelling::Gemfile, false, true, None).await + else { return; }; @@ -901,7 +994,8 @@ async fn gem_hosted_fresh_checkout_bundle_install_installs_patched_bytes_and_vex #[ignore = "host capstone: shells out to a real ruby/gem/bundler >= 2.6; the unpinned `test` \ job skips it, an e2e job with a pinned toolchain runs it via --ignored"] async fn gem_hosted_gems_rb_spelling_redirects_and_installs() { - let Some(fx) = redirect_scanned_project("gems.rb", Spelling::GemsRb, false, true).await else { + let Some(fx) = redirect_scanned_project("gems.rb", Spelling::GemsRb, false, true, None).await + else { return; }; assert!( @@ -934,7 +1028,8 @@ async fn gem_hosted_gems_rb_spelling_redirects_and_installs() { #[ignore = "host capstone: shells out to a real ruby/gem/bundler >= 2.6; the unpinned `test` \ job skips it, an e2e job with a pinned toolchain runs it via --ignored"] async fn gem_hosted_registry_info_without_deps_breaks_install_like_production() { - let Some(fx) = redirect_scanned_project("nodeps", Spelling::Gemfile, false, false).await else { + let Some(fx) = redirect_scanned_project("nodeps", Spelling::Gemfile, false, false, None).await + else { return; }; @@ -965,22 +1060,22 @@ async fn gem_hosted_registry_info_without_deps_breaks_install_like_production() ); } -/// KNOWN-LIMITATION CANARY — CHECKSUMS locks (bundler >= 4 default): the -/// current rewrite (source block + CHECKSUMS pin, GEM section left on the -/// upstream remote) makes the prescribed unfrozen install FAIL: bundler -/// still attributes the gem to the upstream source and refuses the -/// lockfile-vs-upstream-API checksum disagreement ("Bundler found mismatched -/// checksums", exit 37 — verified on bundler 4.0.15). This test pins the -/// rewrite half (the pin lands, its ledger edit records the upstream sha for -/// revert) AND the current install failure. When the rewriter learns the -/// verified fix — the fully converged lock: patched-registry GEM section, -/// ` (= )!` DEPENDENCIES pin, patched CHECKSUMS sha, which a -/// FROZEN install accepts — this canary must flip to asserting success. +/// FLIPPED CANARY — CHECKSUMS locks (bundler >= 4 default) must come out +/// FULLY CONVERGED: patch-registry GEM section holding the dep's spec, +/// ` (= )!` DEPENDENCIES pin, patched CHECKSUMS sha (upstream sha +/// recorded in the ledger for revert). The old mixed-state rewrite (pin only, +/// GEM section left upstream) made the prescribed unfrozen install fail with +/// "Bundler found mismatched checksums" (exit 37 — the bundler-4 DEFAULT +/// lock, i.e. the mainstream hosted-gem path) and forced a frozen-install +/// two-step (exit 16) on deployment setups. The converged pair must now +/// install patched bytes BOTH ways on a fresh checkout: under +/// `BUNDLE_FROZEN=true` with the lock byte-untouched (no two-step), and +/// unfrozen (no exit 37). #[tokio::test(flavor = "multi_thread")] #[ignore = "host capstone: shells out to a real ruby/gem/bundler >= 2.6; the unpinned `test` \ job skips it, an e2e job with a pinned toolchain runs it via --ignored"] -async fn gem_hosted_checksums_lock_pins_patched_sha_but_bundler_refuses_mixed_state() { - let Some(fx) = redirect_scanned_project("checksums", Spelling::Gemfile, true, true).await +async fn gem_hosted_checksums_lock_converges_and_installs_frozen_and_unfrozen() { + let Some(fx) = redirect_scanned_project("checksums", Spelling::Gemfile, true, true, None).await else { return; }; @@ -991,9 +1086,8 @@ async fn gem_hosted_checksums_lock_pins_patched_sha_but_bundler_refuses_mixed_st &std::fs::read_to_string(fx.proj.join(".socket/vendor/redirect-state.json")).unwrap(), ) .unwrap(); - let edit = ledger["edits"] - .as_array() - .expect("ledger edits") + let edits = ledger["edits"].as_array().expect("ledger edits"); + let edit = edits .iter() .find(|e| e["kind"] == "redirect_gemfile_lock_checksum") .expect("CHECKSUMS pin edit recorded in the ledger"); @@ -1003,30 +1097,170 @@ async fn gem_hosted_checksums_lock_pins_patched_sha_but_bundler_refuses_mixed_st original.starts_with(&format!("{DEP} ({DEP_VERSION}) sha256=")), "original must be the pre-edit registry line: {original}" ); + let lock = std::fs::read_to_string(fx.proj.join("Gemfile.lock")).unwrap(); assert!( - !std::fs::read_to_string(fx.proj.join("Gemfile.lock")) - .unwrap() - .contains(original), + !lock.contains(original), "the upstream sha line must actually have been replaced (else the pin is vacuous)" ); - // The install half — today's reality on a CHECKSUMS lock. - let (_fresh, install) = fresh_checkout_bundle_install(&fx); + // The converged half: GEM section attribution + bundler's own `!` pin, + // with the move and the pin recorded in the ledger. assert!( - !install.status.success(), - "KNOWN LIMITATION pinned: if this fresh install now SUCCEEDS, the mixed-state lock \ - handling was fixed — flip this canary to assert success + patched bytes (see the \ - test doc for the verified converged-lock shape).\nstdout:\n{}\nstderr:\n{}", + lock.contains(&format!( + "GEM\n remote: {}\n specs:\n {DEP} ({DEP_VERSION})", + fx.index_url + )), + "the lock must attribute the dep to the patch-registry GEM section:\n{lock}" + ); + assert!( + lock.contains(&format!(" {DEP} (= {DEP_VERSION})!")), + "DEPENDENCIES must carry the source-pinned entry:\n{lock}" + ); + assert!( + edits + .iter() + .any(|e| e["kind"] == "redirect_gemfile_lock_gem_source"), + "the GEM-section move must be a ledger edit: {edits:?}" + ); + + // FROZEN fresh checkout: the converged pair needs no unfrozen two-step — + // bundler's deployment contract accepts it as-is and the lock stays + // byte-identical. + let frozen = stage_fresh_checkout(&fx, "fresh-frozen"); + let lock_before = std::fs::read(frozen.join(fx.lock_name)).unwrap(); + let install = bundle_env(&frozen, &["install"], &[("BUNDLE_FROZEN", "true")]); + assert!( + install.status.success(), + "FROZEN fresh-checkout install of the converged pair must succeed (the exit-16 \ + two-step is gone).\nstdout:\n{}\nstderr:\n{}", String::from_utf8_lossy(&install.stdout), String::from_utf8_lossy(&install.stderr), ); - let chatter = format!( - "{}\n{}", + assert_eq!( + std::fs::read(frozen.join(fx.lock_name)).unwrap(), + lock_before, + "a frozen install must leave the lock byte-identical" + ); + assert_patched_install(&fx, &frozen); + + // UNFROZEN fresh checkout: the previously-pinned exit 37 "mismatched + // checksums" refusal is gone too. + let (fresh, install) = fresh_checkout_bundle_install(&fx); + assert!( + install.status.success(), + "unfrozen fresh-checkout install of the converged pair must succeed (the pinned \ + exit-37 mixed-state refusal is fixed).\nstdout:\n{}\nstderr:\n{}", String::from_utf8_lossy(&install.stdout), - String::from_utf8_lossy(&install.stderr) + String::from_utf8_lossy(&install.stderr), + ); + assert_patched_install(&fx, &fresh); +} + +/// GRANT ROTATION, end to end (token A -> A -> B, same patch uuid): the +/// production reference endpoint rotates the grant-token path segment of the +/// index URL per request, so a periodic/CI re-scan sees a NEW index URL for +/// the SAME redirect. The re-scan must (1) be byte-idempotent under the same +/// grant, (2) refresh the source block's URL IN PLACE under a rotated grant — +/// exactly one Socket source block, a `redirect_gemfile_source_url` ledger +/// edit, no stale token anywhere — and (3) leave a pair a fresh checkout +/// installs the patched bytes from. Before the fix, the rotated re-scan +/// wrapped the old block's indented gem line in a new NESTED source block +/// (+1 nesting per re-scan), kept the stale token URL live, and still +/// reported success. +#[tokio::test(flavor = "multi_thread")] +#[ignore = "host capstone: shells out to a real ruby/gem/bundler >= 2.6; the unpinned `test` \ + job skips it, an e2e job with a pinned toolchain runs it via --ignored"] +async fn gem_hosted_rotated_grant_rescan_refreshes_source_block_and_installs() { + const TOKEN_B: &str = "55555555-5555-4555-8555-555555555555"; + let Some(fx) = + redirect_scanned_project("rotation", Spelling::Gemfile, false, true, Some(TOKEN_B)).await + else { + return; + }; + let api = fx._server.uri(); + let index_url_b = format!("{api}/patch-registry/gem/{TOKEN_B}/{UUID}/"); + let gemfile_after_run1 = std::fs::read_to_string(fx.proj.join("Gemfile")) + .expect("read Gemfile after initial hosted scan"); + let ledger_edits = |proj: &Path| -> Vec { + serde_json::from_str::( + &std::fs::read_to_string(proj.join(".socket/vendor/redirect-state.json")) + .expect("read redirect ledger"), + ) + .expect("ledger is JSON")["edits"] + .as_array() + .expect("ledger edits array") + .clone() + }; + let edits_after_run1 = ledger_edits(&fx.proj).len(); + + // Re-scan 2, SAME grant: byte-idempotent, no ledger growth. + let (code, stdout, stderr) = run_hosted_scan(&fx.proj, &api); + assert_eq!( + code, 0, + "same-grant re-scan failed.\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + let env: serde_json::Value = serde_json::from_str(&stdout).expect("re-scan envelope JSON"); + assert_eq!(env["redirect"]["redirected"], 1, "envelope: {env}"); + assert_eq!( + std::fs::read_to_string(fx.proj.join("Gemfile")) + .expect("read Gemfile after same-grant re-scan"), + gemfile_after_run1, + "same-grant re-scan must leave the Gemfile byte-identical" + ); + assert_eq!( + ledger_edits(&fx.proj).len(), + edits_after_run1, + "same-grant re-scan must not grow the ledger" + ); + + // Re-scan 3, ROTATED grant (token B, same uuid): refresh in place. + let (code, stdout, stderr) = run_hosted_scan(&fx.proj, &api); + assert_eq!( + code, 0, + "rotated-grant re-scan failed.\nstdout:\n{stdout}\nstderr:\n{stderr}" + ); + let env: serde_json::Value = serde_json::from_str(&stdout).expect("rotation envelope JSON"); + assert_eq!(env["redirect"]["redirected"], 1, "envelope: {env}"); + let gemfile = std::fs::read_to_string(fx.proj.join("Gemfile")) + .expect("read Gemfile after rotated-grant re-scan"); + assert_eq!( + gemfile.matches("/patch-registry/gem/").count(), + 1, + "exactly one Socket source block, never nested:\n{gemfile}" + ); + assert!( + gemfile.contains(&format!( + "source \"{index_url_b}\" do\n gem \"{DEP}\", \"{DEP_VERSION}\"\nend" + )), + "the block's URL must be refreshed to the rotated grant in place:\n{gemfile}" + ); + assert!( + !gemfile.contains(TOKEN), + "the stale grant token must be gone from the Gemfile:\n{gemfile}" ); + let refresh = ledger_edits(&fx.proj) + .into_iter() + .find(|e| e["kind"] == "redirect_gemfile_source_url") + .expect("rotation must be recorded as a redirect_gemfile_source_url ledger edit"); + assert_eq!( + refresh["original"], + serde_json::Value::String(fx.index_url.clone()), + "refresh edit original: {refresh}" + ); + assert_eq!( + refresh["new"], + serde_json::Value::String(index_url_b), + "refresh edit new: {refresh}" + ); + + // Fresh checkout of the rotated pair: the prescribed unfrozen install + // resolves the patched gem from the rotated registry path. + let (fresh, install) = fresh_checkout_bundle_install(&fx); assert!( - chatter.to_lowercase().contains("mismatched checksums"), - "the refusal must be bundler's checksum-conflict check, not something incidental:\n{chatter}" + install.status.success(), + "fresh-checkout `bundle install` after rotation must succeed.\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&install.stdout), + String::from_utf8_lossy(&install.stderr), ); + assert_patched_install(&fx, &fresh); } diff --git a/crates/socket-patch-core/src/patch/redirect/mod.rs b/crates/socket-patch-core/src/patch/redirect/mod.rs index 4ebb1150..2263bfc6 100644 --- a/crates/socket-patch-core/src/patch/redirect/mod.rs +++ b/crates/socket-patch-core/src/patch/redirect/mod.rs @@ -759,6 +759,18 @@ fn is_valid_cargo_index_url(url: &str) -> bool { && !url.chars().any(char::is_control) } +/// Gem index URLs land verbatim inside a quoted Ruby `source "" do` +/// Gemfile string and on unquoted Gemfile.lock `remote:` lines — refuse +/// anything that could break out of either (quote, backslash, whitespace; +/// control chars cover newline injection into the lock) or that is not an +/// http(s) URL at all. Twin of [`is_valid_cargo_index_url`]. +fn is_valid_gem_index_url(url: &str) -> bool { + (url.starts_with("https://") || url.starts_with("http://")) + && !url.contains('"') + && !url.contains('\\') + && !url.chars().any(|c| c.is_control() || c == ' ') +} + /// The exact shape `hex::encode(sha256)` / the TS `Buffer.toString('hex')` /// produce: 64 lowercase hex chars. Anything else written as a Cargo.lock /// `checksum` breaks the next fetch. @@ -2968,12 +2980,45 @@ fn gem_tail_source_option(tail: &str) -> Option<&'static str> { .find(|tok| code.contains(tok)) } +/// The grant-token path segment of a hosted patch URL: the path level +/// immediately preceding the patch-uuid level (production shape +/// `…/patch-registry/gem/{token}/{uuid}/…`, same layout on the artifact +/// URLs). The reference endpoint hands the token back only inside its URLs, +/// so this is how a caller recovers it for `DepOverride.token`. Only path +/// levels count — the scheme/host prefix is skipped so a uuid sitting in the +/// first path segment can never elect the host as its "token". `None` when +/// the uuid is absent or nothing precedes it. +pub fn grant_token_path_segment(url: &str, patch_uuid: &str) -> Option { + if patch_uuid.is_empty() { + return None; + } + let after_scheme = url.split_once("://").map(|(_, rest)| rest).unwrap_or(url); + let (_, path) = after_scheme.split_once('/')?; + let before = path + .split_once(&format!("/{patch_uuid}/")) + .map(|(before, _)| before) + .or_else(|| path.strip_suffix(&format!("/{patch_uuid}")))?; + let token = before.rsplit('/').next().unwrap_or(""); + (!token.is_empty()).then(|| token.to_string()) +} + /// A dep's Socket index URL as a regex source with the per-request rotating /// segments (grant token, patch uuid) wildcarded — an exact-URL pattern -/// misses the URL a previous run wrote under an older grant. +/// misses the URL a previous run wrote under an older grant. The grant token +/// is wildcarded even when the caller left `dep.token` empty (the CLI +/// historically never populated it): the token path level is derived from +/// the index URL itself as the segment immediately preceding the patch-uuid +/// level, so the idempotency guard never silently degrades into the +/// nesting-corruption failure mode when a caller forgets the token. fn gem_index_url_pattern(dep: &DepOverride, index_url: &str) -> String { let mut url_pat = regex::escape(index_url); - for rotating in [&dep.token, &dep.patch_uuid] { + let derived_token = grant_token_path_segment(index_url, &dep.patch_uuid); + let rotating = [ + Some(dep.token.as_str()), + derived_token.as_deref(), + Some(dep.patch_uuid.as_str()), + ]; + for rotating in rotating.into_iter().flatten() { if !rotating.is_empty() { url_pat = url_pat.replace(®ex::escape(&format!("/{rotating}/")), "/[^/\"]+/"); } @@ -3023,6 +3068,232 @@ fn gem_spelling_residue(content: &str, deps: &[&DepOverride]) -> String { residue.trim_end().to_string() } +/// A lock line without its `\r?\n` ending (never more than one of each). +fn gem_lock_line_content(line: &str) -> &str { + let line = line.strip_suffix('\n').unwrap_or(line); + line.strip_suffix('\r').unwrap_or(line) +} + +/// The gem name of a 2-space DEPENDENCIES entry (` rails`, ` rails!`, +/// ` rails (= 7.0.0)!`) — the text before any constraint, sans source pin. +fn gem_lock_dependency_name(entry: &str) -> &str { + let entry = entry.trim_start(); + let entry = entry.split(" (").next().unwrap_or(entry); + entry.trim_end_matches('!') +} + +/// One parsed `GEM` section of a Gemfile.lock: its `remote:` lines (index + +/// URL) and the exclusive end index — the start of the next column-0 header +/// (trailing blank separator included) or EOF. +struct GemLockSection { + remotes: Vec<(usize, String)>, + end: usize, +} + +/// Converge the lock's source attribution for one redirected dep so the +/// Gemfile + lock pair is what bundler itself would write after an install +/// from the redirected Gemfile (verified frozen-installable on bundler 4): +/// the dep's spec entry (+ its dependency sublines) moves out of the +/// upstream `GEM` section into a patch-registry `GEM` section +/// (`remote: `), and DEPENDENCIES pins ` (= )!` +/// (bundler's source-pin spelling for a block-scoped exact-version gem) — +/// added in sorted position when the dep was transitive. Without this the +/// CHECKSUMS pin leaves a MIXED state bundler refuses: the lock still +/// attributes the gem to the upstream remote, so the prescribed unfrozen +/// install exits 37 "mismatched checksums" and a frozen install exits 16. +/// +/// Idempotent and rotation-aware: a section whose remote matches the +/// token-wildcard pattern is recognized as ours (never duplicated) and its +/// remote is refreshed in place under a rotated grant +/// (`redirect_gemfile_lock_source_url`, mirroring the Gemfile refresh). +/// +/// Returns true when the lock ends converged (already, or via edits recorded +/// into `result`); false when the dep cannot be attributed safely — spec +/// entry absent or duplicated, a legacy multi-remote `GEM` section, or no +/// DEPENDENCIES section — in which case nothing is touched and the caller +/// surfaces the frozen-install caveat exactly as before. +fn converge_gem_lock_source( + lk: &mut String, + dep: &DepOverride, + index_url: &str, + lock_name: &str, + lock_changed: &mut bool, + result: &mut RewriteResult, +) -> bool { + let eol = if lk.contains("\r\n") { "\r\n" } else { "\n" }; + let mut lines: Vec = lk.split_inclusive('\n').map(str::to_string).collect(); + let is_header = |c: &str| !c.is_empty() && !c.starts_with(' '); + + // Parse: GEM sections, the dep's 4-space spec entry, DEPENDENCIES range. + let spec_content = format!(" {} ({})", dep.name, dep.version); + let mut sections: Vec = Vec::new(); + let mut spec_at: Vec<(usize, usize)> = Vec::new(); // (section idx, line idx) + let mut deps_range: Option<(usize, usize)> = None; // exclusive of header + let mut i = 0; + while i < lines.len() { + let c = gem_lock_line_content(&lines[i]); + if !is_header(c) { + i += 1; + continue; + } + let header_is_gem = c == "GEM"; + let start = i; + let mut remotes = Vec::new(); + let mut j = i + 1; + while j < lines.len() && !is_header(gem_lock_line_content(&lines[j])) { + let cj = gem_lock_line_content(&lines[j]); + if header_is_gem { + if let Some(url) = cj.strip_prefix(" remote: ") { + remotes.push((j, url.to_string())); + } + if cj == spec_content { + spec_at.push((sections.len(), j)); + } + } + j += 1; + } + if header_is_gem { + sections.push(GemLockSection { remotes, end: j }); + } else if c == "DEPENDENCIES" { + deps_range = Some((start + 1, j)); + } + i = j; + } + + let spec_pos = if spec_at.len() == 1 { + Some(spec_at[0]) + } else { + None + }; + let (Some((sec_idx, spec_idx)), Some((deps_start, deps_end))) = (spec_pos, deps_range) else { + return false; + }; + if sections[sec_idx].remotes.len() != 1 { + return false; + } + // Bundler always writes source sections before DEPENDENCIES — the pin + // edit below runs first on that premise (its lines sit after the parsed + // spec/remote/end indices, so they never shift). A hand-edited lock with + // DEPENDENCIES before the dep's GEM section breaks the premise: the + // transitive-dep pin INSERT would leave the spec-move splicing on stale + // indices. Fail soft to the mixed state instead. + if deps_start < sections[sec_idx].end { + return false; + } + let (remote_idx, remote_url) = sections[sec_idx].remotes[0].clone(); + let socket_remote_re = Regex::new(&format!("^{}$", gem_index_url_pattern(dep, index_url))) + .expect("anchored index-url pattern from the escaped URL is valid"); + let mut changed = false; + + // DEPENDENCIES pin first — its lines sit AFTER the GEM sections, so the + // spec move below never invalidates these indices (and vice versa would). + let target = format!(" {} (= {})!", dep.name, dep.version); + let is_entry = |c: &str| c.starts_with(" ") && !c.starts_with(" "); + let entry_idx = (deps_start..deps_end).find(|&k| { + let ck = gem_lock_line_content(&lines[k]); + is_entry(ck) && gem_lock_dependency_name(ck) == dep.name + }); + match entry_idx { + Some(k) if gem_lock_line_content(&lines[k]) == target => {} + Some(k) => { + let old = gem_lock_line_content(&lines[k]).trim_start().to_string(); + let ending = lines[k][gem_lock_line_content(&lines[k]).len()..].to_string(); + lines[k] = format!("{target}{ending}"); + result.edits.push(FileEdit { + path: lock_name.into(), + kind: "redirect_gemfile_lock_dependency_pin".into(), + action: "rewritten".into(), + key: Some(dep.name.clone()), + original: Some(Value::String(old)), + new: Some(Value::String(target.trim_start().to_string())), + }); + changed = true; + } + None => { + // Transitive dep: bundler keeps DEPENDENCIES sorted by name. + let mut at = deps_end; + for (k, line) in lines.iter().enumerate().take(deps_end).skip(deps_start) { + let ck = gem_lock_line_content(line); + if ck.is_empty() + || (is_entry(ck) && gem_lock_dependency_name(ck) > dep.name.as_str()) + { + at = k; + break; + } + } + lines.insert(at, format!("{target}{eol}")); + result.edits.push(FileEdit { + path: lock_name.into(), + kind: "redirect_gemfile_lock_dependency_pin".into(), + action: "added".into(), + key: Some(dep.name.clone()), + original: None, + new: Some(Value::String(target.trim_start().to_string())), + }); + changed = true; + } + } + + if socket_remote_re.is_match(&remote_url) { + // Already ours. Rotated grant: refresh the remote in place. + if remote_url != index_url { + let ending = + lines[remote_idx][gem_lock_line_content(&lines[remote_idx]).len()..].to_string(); + lines[remote_idx] = format!(" remote: {index_url}{ending}"); + result.edits.push(FileEdit { + path: lock_name.into(), + kind: "redirect_gemfile_lock_source_url".into(), + action: "rewritten".into(), + key: Some(dep.name.clone()), + original: Some(Value::String(remote_url)), + new: Some(Value::String(index_url.to_string())), + }); + changed = true; + } + } else { + // Move the spec (+ sublines) into a patch-registry section of its + // own, inserted where the section it leaves ends. + let mut last = spec_idx; + while last + 1 < lines.len() + && gem_lock_line_content(&lines[last + 1]).starts_with(" ") + { + last += 1; + } + let moved: Vec = lines.drain(spec_idx..=last).collect(); + let insert_at = sections[sec_idx].end - moved.len(); + let mut block: Vec = Vec::with_capacity(moved.len() + 4); + block.push(format!("GEM{eol}")); + block.push(format!(" remote: {index_url}{eol}")); + block.push(format!(" specs:{eol}")); + for line in moved { + // Moved lines keep their own bytes; only a final line that lacked + // a newline (EOF) gains the file's ending. + if line.ends_with('\n') { + block.push(line); + } else { + block.push(format!("{line}{eol}")); + } + } + block.push(eol.to_string()); + lines.splice(insert_at..insert_at, block); + result.edits.push(FileEdit { + path: lock_name.into(), + kind: "redirect_gemfile_lock_gem_source".into(), + action: "rewritten".into(), + key: Some(dep.name.clone()), + original: Some(Value::String(remote_url)), + new: Some(Value::String(index_url.to_string())), + }); + changed = true; + } + + if changed { + *lk = lines.concat(); + *lock_changed = true; + } + true +} + fn rewrite_gem( files: &BTreeMap, overrides: &[DepOverride], @@ -3076,6 +3347,12 @@ fn rewrite_gem( // misdiagnosing the lock as bundler <2.6. let checksums_re = Regex::new(r"(?m)^CHECKSUMS(\r?)$").expect("static CHECKSUMS header regex is valid"); + // True once any redirected dep leaves the pair MIXED: the lock still + // attributes the dep to the upstream source (no CHECKSUMS section to key + // the convergence on, or a lock shape the convergence refused). Only + // that state earns the frozen-install caveat — a converged pair is + // frozen-installable as written. + let mut mixed_state = false; for dep in &gem { let Some(ov) = &dep.registry_override else { @@ -3088,6 +3365,19 @@ fn rewrite_gem( if ov.kind != "rubygems-compact-index" { continue; } + // The URL is interpolated into the Gemfile's quoted source string and + // the lock's `remote:` lines — gate it before any write, like the + // cargo arm gates sparse index URLs. + if !is_valid_gem_index_url(&ov.index_url) { + result.warnings.push(RewriteWarning { + code: "redirect_gem_invalid_index_url".into(), + detail: format!( + "{} has a malformed patch-registry index URL; dependency skipped", + dep.name + ), + }); + continue; + } let Some(sha256) = ov .identifiers .gem_checksum_sha256 @@ -3229,15 +3519,37 @@ fn rewrite_gem( }; // 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. + // silent no-op that still gets attested. Fail closed — + // and when the blocking `path:` is socket-patch's OWN + // vendored wiring, prescribe the eject path instead of + // leaving the user to puzzle over their own Gemfile. if let Some(tok) = gem_tail_source_option(&tail) { - result.warnings.push(RewriteWarning { - code: "redirect_gem_source_option".into(), - detail: format!( + let socket_vendored = matches!(tok, "path:" | ":path") + && tail + .split('#') + .next() + .unwrap_or("") + .contains(".socket/vendor/"); + let detail = if socket_vendored { + format!( + "the `gem \"{}\"` declaration carries `{tok}` pointing into \ + .socket/vendor — socket-patch's own vendored wiring, which \ + would override the Socket source block; un-vendor this gem \ + first (`socket-patch remove pkg:gem/{}@{}`, or `socket-patch \ + vendor --revert` to revert EVERY vendored dependency in the \ + project), then re-run the hosted scan", + dep.name, dep.name, dep.version + ) + } else { + format!( "the `gem \"{}\"` declaration carries `{tok}`, which would \ override the Socket source block; redirect skipped", dep.name - ), + ) + }; + result.warnings.push(RewriteWarning { + code: "redirect_gem_source_option".into(), + detail, }); continue; } @@ -3334,6 +3646,7 @@ fn rewrite_gem( let already_re = Regex::new(&(String::from(r"(?m)^ ") + ®ex::escape(&new_val) + r"\r?$")) .expect("already-redirected regex from the escaped line is valid"); + let mut checksums_era = true; if already_re.is_match(lk) { // no-op } else if let Some(m) = sum_line_re.captures(lk) { @@ -3387,15 +3700,35 @@ fn rewrite_gem( dep.name ), }); + checksums_era = false; + } + // A CHECKSUMS-era lock must end FULLY CONVERGED — with only the + // sha pinned, bundler still attributes the gem to the upstream + // remote and refuses the pair outright (unfrozen: exit 37 + // "mismatched checksums"; frozen: exit 16). A pre-CHECKSUMS lock + // has no sha to converge around, so it keeps today's + // mixed-but-installable state + the frozen-install caveat. + if !checksums_era + || !converge_gem_lock_source( + lk, + dep, + &ov.index_url, + lock_name, + &mut lock_changed, + result, + ) + { + mixed_state = true; } } } - // The rewritten pair breaks bundler's frozen/deployment mode: the lock's - // GEM section still records the upstream source, so `bundle install` with + // A MIXED 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 { + // Mirror of the CLI's pnpm trust-lockfile warning. A converged pair (the + // CHECKSUMS-era path) is frozen-installable as written — no caveat. + if (gemfile_changed || lock_changed) && mixed_state { result.warnings.push(RewriteWarning { code: "redirect_gem_frozen_install".into(), detail: format!( @@ -6526,6 +6859,52 @@ mod tests { } } + /// A service-supplied index URL is interpolated into the Gemfile's quoted + /// source string and the lock's `remote:` lines — a quote, backslash, or + /// control character (a newline would inject whole lock lines) must be + /// refused at intake with nothing written, like the cargo sparse gate. + #[test] + fn gem_malformed_index_url_is_refused_before_any_write() { + for bad in [ + "https://patch.test/gem/tok\"/uuid/", + "https://patch.test/gem\\tok/uuid/", + "https://patch.test/gem/tok/uuid/\nGEM", + "https://patch.test/gem/t k/uuid/", + "ftp://patch.test/gem/tok/uuid/", + ] { + 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\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\ + BUNDLED WITH\n 2.6.2\n" + .to_string(), + ); + let mut ov = gem_override("rails", "7.0.0"); + ov.registry_override + .as_mut() + .expect("gem_override always carries a registry override") + .index_url = bad.into(); + let r = rewrite_registry_redirect(&files, &[ov]); + assert!( + r.files.is_empty() && r.edits.is_empty(), + "malformed index URL [{bad}] must write nothing: {:?}", + r.edits + ); + assert!( + r.warnings + .iter() + .any(|w| w.code == "redirect_gem_invalid_index_url"), + "malformed index URL [{bad}] must warn: {:?}", + r.warnings + ); + } + } + /// Trailing options on the original `gem` line (`require: false`, /// `group: …`) must survive the move into the source block — dropping /// `require: false` auto-requires the gem at boot, changing app behavior @@ -6645,6 +7024,133 @@ mod tests { ); } + /// The CLI's ONLY production `DepOverride` construction site + /// (`scan/hosted.rs`) builds every override with an EMPTY `token` — the + /// reference endpoint hands the grant token back only inside the URLs it + /// returns. The rotated-grant idempotency guard must therefore never + /// depend on the caller populating `token`: a re-scan under a rotated + /// grant must still recognize the source block a previous run wrote and + /// refresh its URL in place. With a token-dependent guard the recognizer + /// misses the old block, `gem_line_re` matches the INDENTED gem line + /// inside it, and every re-scan wraps it in one more nested source block + /// while keeping the stale (soon-dead) token URL live and reporting + /// success. + #[test] + fn gemfile_rerun_with_rotated_grant_and_cli_empty_token_never_nests() { + const PATCH_UUID: &str = "7c8d9e0f-1a2b-4a1b-8c2d-3e4f5a6b7c8d"; + fn ov(token: &str) -> DepOverride { + let mut o = gem_override("rails", "7.0.0"); + // Exactly as the CLI builds it: the grant token never populated. + o.token = String::new(); + o.patch_uuid = PATCH_UUID.into(); + if let Some(r) = o.registry_override.as_mut() { + r.index_url = + format!("https://patch.test/patch-registry/gem/{token}/{PATCH_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")]); + files.insert( + "Gemfile".to_string(), + first + .files + .get("Gemfile") + .expect("first run rewrites") + .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/patch-registry/gem/") + .count(), + 1, + "exactly one Socket source block, never nested: {out}" + ); + assert!(!out.contains("tok-one"), "old grant token gone: {out}"); + assert!( + out.contains(&format!( + "source \"https://patch.test/patch-registry/gem/tok-two/{PATCH_UUID}/\" do\n gem \"rails\", \"7.0.0\"\nend" + )), + "URL refreshed in place: {out}" + ); + assert!( + second + .edits + .iter() + .any(|e| e.kind == "redirect_gemfile_source_url"), + "the refresh must be recorded as a redirect_gemfile_source_url edit: {:?}", + 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 + ); + } + + /// The gems.rb/Gemfile divergence guard erases the redirect's own + /// footprint with the same token-wildcard pattern, so it too must not + /// depend on `DepOverride.token` being populated: identical twins + /// re-scanned under a rotated grant with the CLI's empty token must reach + /// the in-place refresh, not be trapped behind + /// `redirect_gem_gemfile_spellings_diverge` by run 1's own edit. + #[test] + fn gems_rb_twins_rotated_grant_with_cli_empty_token_refreshes() { + const PATCH_UUID: &str = "7c8d9e0f-1a2b-4a1b-8c2d-3e4f5a6b7c8d"; + fn ov(token: &str) -> DepOverride { + let mut o = gem_override("rails", "7.0.0"); + o.token = String::new(); + o.patch_uuid = PATCH_UUID.into(); + if let Some(r) = o.registry_override.as_mut() { + r.index_url = + format!("https://patch.test/patch-registry/gem/{token}/{PATCH_UUID}/"); + } + o + } + let gemfile = "source \"https://rubygems.org\"\n\ngem \"rails\", \"7.0.0\"\n".to_string(); + let mut files = BTreeMap::new(); + files.insert("gems.rb".to_string(), gemfile.clone()); + files.insert("Gemfile".to_string(), gemfile); + let first = rewrite_registry_redirect(&files, &[ov("tok-one")]); + for (name, content) in first.files { + files.insert(name, content); + } + let second = rewrite_registry_redirect(&files, &[ov("tok-two")]); + assert!( + !second + .warnings + .iter() + .any(|w| w.code == "redirect_gem_gemfile_spellings_diverge"), + "run 1's own edit must not read as divergence: {:?}", + second.warnings + ); + let out = second + .files + .get("gems.rb") + .expect("rotated grant refreshes gems.rb"); + assert_eq!( + out.matches("source \"https://patch.test/patch-registry/gem/") + .count(), + 1, + "exactly one Socket source block, never nested: {out}" + ); + assert!(!out.contains("tok-one"), "old grant token gone: {out}"); + } + /// 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 @@ -6725,6 +7231,113 @@ mod tests { ); } + /// When the blocking `path:` option is socket-patch's OWN vendored wiring + /// (`.socket/vendor/gem//…`), the refusal must prescribe the eject + /// paths instead of pointing the user at a Gemfile line the tool itself + /// wrote — and state their blast radius honestly: `remove ` is the + /// per-gem undo, `vendor --revert` reverts EVERY vendored dependency. + #[test] + fn gemfile_source_option_refusal_prescribes_vendor_revert_for_own_wiring() { + let mut files = BTreeMap::new(); + files.insert( + "Gemfile".to_string(), + "source \"https://rubygems.org\"\n\n\ + gem \"rails\", \"7.0.0\", path: \".socket/vendor/gem/11111111-1111-4111-8111-111111111111/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()); + let warning = r + .warnings + .iter() + .find(|w| w.code == "redirect_gem_source_option") + .unwrap_or_else(|| panic!("skip must warn: {:?}", r.warnings)); + assert!( + warning.detail.contains("socket-patch remove pkg:gem/rails@7.0.0") + && warning.detail.contains("socket-patch vendor --revert") + && warning.detail.contains("EVERY vendored dependency"), + "socket's own vendored wiring must prescribe the per-gem eject and \ + state vendor --revert's whole-project blast radius: {}", + warning.detail + ); + // A USER path: dep keeps the generic refusal — no bogus prescription. + files.insert( + "Gemfile".to_string(), + "source \"https://rubygems.org\"\n\n\ + gem \"rails\", \"7.0.0\", path: \"../rails\"\n" + .to_string(), + ); + let r = rewrite_registry_redirect(&files, &[gem_override("rails", "7.0.0")]); + let warning = r + .warnings + .iter() + .find(|w| w.code == "redirect_gem_source_option") + .unwrap_or_else(|| panic!("skip must warn: {:?}", r.warnings)); + assert!( + !warning.detail.contains("vendor --revert"), + "a user path: dep is not socket wiring: {}", + warning.detail + ); + } + + /// `grant_token_path_segment` recovers the grant token from the hosted + /// URL shapes the reference endpoint hands back (the path level before + /// the patch uuid) and answers `None` — never a host or empty segment — + /// on anything else. + #[test] + fn grant_token_path_segment_shapes() { + let uuid = "7c8d9e0f-1a2b-4a1b-8c2d-3e4f5a6b7c8d"; + assert_eq!( + grant_token_path_segment( + &format!("https://patch.socket.dev/patch-registry/gem/tok-a/{uuid}/"), + uuid + ) + .as_deref(), + Some("tok-a"), + "index-url shape" + ); + assert_eq!( + grant_token_path_segment( + &format!( + "https://patch.socket.dev/patch/gem/rails/7.0.0/tok-b/{uuid}/rails-7.0.0.gem" + ), + uuid + ) + .as_deref(), + Some("tok-b"), + "artifact-url shape" + ); + assert_eq!( + grant_token_path_segment( + &format!("https://patch.socket.dev/patch-registry/gem/tok-c/{uuid}"), + uuid + ) + .as_deref(), + Some("tok-c"), + "no trailing slash" + ); + assert_eq!( + grant_token_path_segment(&format!("https://patch.socket.dev/{uuid}/"), uuid), + None, + "uuid in the first path level has no token before it" + ); + assert_eq!( + grant_token_path_segment("https://patch.socket.dev/gem/tok/other/", uuid), + None, + "uuid absent" + ); + assert_eq!( + grant_token_path_segment(&format!("https://{uuid}/x/"), uuid), + None, + "a uuid-shaped HOST is not a path level" + ); + assert_eq!( + grant_token_path_segment("https://patch.socket.dev/gem/tok/x/", ""), + None, + "empty uuid never matches" + ); + } + /// 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 @@ -6853,9 +7466,13 @@ mod tests { ); } - /// 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. + /// A MIXED-state 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. + /// Only the pre-CHECKSUMS lock (bundler <2.6, or `lockfile_checksums + /// false`) stays mixed today; a CHECKSUMS-era lock converges instead and + /// must NOT carry the caveat (pinned in + /// `gem_checksums_lock_converges_gem_section_and_pins_dependency`). #[test] fn gem_redirect_warns_about_frozen_installs() { let mut files = BTreeMap::new(); @@ -6863,9 +7480,13 @@ mod tests { "Gemfile".to_string(), "source \"https://rubygems.org\"\n\ngem \"rails\", \"7.0.0\"\n".to_string(), ); + // No CHECKSUMS section: nothing to converge around, GEM attribution + // stays upstream — the caveat is truthful here. files.insert( "Gemfile.lock".to_string(), - gem_lock(&format!(" rails (7.0.0) sha256={}", "2".repeat(64))), + "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\nBUNDLED WITH\n 2.5.0\n" + .to_string(), ); let ovr = gem_override("rails", "7.0.0"); let first = rewrite_registry_redirect(&files, std::slice::from_ref(&ovr)); @@ -6925,6 +7546,263 @@ mod tests { ); } + /// CHECKSUMS-era locks (bundler >= 4 writes the section by default) must + /// come out FULLY CONVERGED, not mixed-state: the dep's spec entry moves + /// out of the upstream GEM section into a patch-registry GEM section + /// (`remote: `), DEPENDENCIES pins ` (= )!`, and + /// CHECKSUMS carries the patched sha. The old mixed rewrite (CHECKSUMS + /// pinned, GEM section left upstream) made bundler refuse the prescribed + /// unfrozen install with exit 37 "mismatched checksums" — and the + /// converged pair needs no frozen-install caveat at all. + #[test] + fn gem_checksums_lock_converges_gem_section_and_pins_dependency() { + 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 expected = format!( + "GEM\n remote: https://rubygems.org/\n specs:\n\n\ + GEM\n remote: https://patch.test/gem/tok/uuid/\n specs:\n rails (7.0.0)\n\n\ + PLATFORMS\n ruby\n\nDEPENDENCIES\n rails (= 7.0.0)!\n\n\ + CHECKSUMS\n rails (7.0.0) sha256={}\n\nBUNDLED WITH\n 2.6.2\n", + "f".repeat(64) + ); + assert_eq!( + r.files.get("Gemfile.lock"), + Some(&expected), + "the lock must converge: patch-registry GEM section + dependency pin + patched sha" + ); + let source_edit = r + .edits + .iter() + .find(|e| e.kind == "redirect_gemfile_lock_gem_source") + .unwrap_or_else(|| panic!("GEM-section move edit recorded: {:?}", r.edits)); + assert_eq!(source_edit.path, "Gemfile.lock"); + assert_eq!( + source_edit.original, + Some(Value::String("https://rubygems.org/".into())), + "the upstream remote is the revert original" + ); + assert_eq!( + source_edit.new, + Some(Value::String("https://patch.test/gem/tok/uuid/".into())) + ); + let dep_edit = r + .edits + .iter() + .find(|e| e.kind == "redirect_gemfile_lock_dependency_pin") + .unwrap_or_else(|| panic!("DEPENDENCIES pin edit recorded: {:?}", r.edits)); + assert_eq!( + dep_edit.original, + Some(Value::String("rails (= 7.0.0)".into())) + ); + assert_eq!(dep_edit.new, Some(Value::String("rails (= 7.0.0)!".into()))); + assert!( + !r.warnings + .iter() + .any(|w| w.code == "redirect_gem_frozen_install"), + "a converged pair is frozen-install-ready — the caveat would be a lie: {:?}", + r.warnings + ); + } + + /// Feeding the converged pair back must be a true no-op (the ledger would + /// otherwise grow forever) — and the converged lock shape must be + /// RECOGNIZED, not re-converged into a duplicate section. + #[test] + fn gem_checksums_converged_lock_rerun_is_noop() { + 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)); + let lock = first + .files + .get("Gemfile.lock") + .expect("run 1 rewrites the lock"); + assert!( + lock.contains( + "GEM\n remote: https://patch.test/gem/tok/uuid/\n specs:\n rails (7.0.0)" + ), + "run 1 must converge the lock: {lock}" + ); + 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.edits.is_empty(), + "converged re-run must be a no-op: files={:?} edits={:?}", + second.files.keys(), + second.edits + ); + } + + /// A rotated grant must refresh the CONVERGED lock's GEM remote in place + /// (token-wildcard recognition, exactly like the Gemfile source block) — + /// leaving the stale remote live would send every install to the dead + /// grant URL. + #[test] + fn gem_checksums_converged_lock_rotated_grant_refreshes_remote() { + 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(), + ); + files.insert( + "Gemfile.lock".to_string(), + gem_lock(&format!(" rails (7.0.0) sha256={}", "2".repeat(64))), + ); + let first = rewrite_registry_redirect(&files, &[ov("tok-one")]); + for (name, content) in first.files { + files.insert(name, content); + } + let second = rewrite_registry_redirect(&files, &[ov("tok-two")]); + let lock = second + .files + .get("Gemfile.lock") + .expect("rotated grant refreshes the lock remote"); + assert_eq!( + lock.matches("remote: https://patch.test/gem/").count(), + 1, + "exactly one Socket GEM section: {lock}" + ); + assert!( + lock.contains(" remote: https://patch.test/gem/tok-two/uuid/\n"), + "lock remote refreshed in place: {lock}" + ); + assert!(!lock.contains("tok-one"), "stale grant gone: {lock}"); + assert!( + second + .edits + .iter() + .any(|e| e.kind == "redirect_gemfile_lock_source_url" + && e.original + == Some(Value::String("https://patch.test/gem/tok-one/uuid/".into())) + && e.new == Some(Value::String("https://patch.test/gem/tok-two/uuid/".into()))), + "remote refresh recorded with the old URL as original: {:?}", + second.edits + ); + } + + /// A TRANSITIVE redirected dep (undeclared in the Gemfile, appended as a + /// source block) becomes a direct source-pinned dependency, so the + /// converged lock must gain its ` (= )!` DEPENDENCIES entry — + /// inserted in bundler's sorted position — and the spec's dependency + /// sublines must travel with the spec into the patch-registry section. + #[test] + fn gem_checksums_lock_transitive_dep_converges_with_sorted_dependency() { + let mut files = BTreeMap::new(); + files.insert( + "Gemfile".to_string(), + "source \"https://rubygems.org\"\n\ngem \"rack\", \"3.0.0\"\n".to_string(), + ); + files.insert( + "Gemfile.lock".to_string(), + format!( + "GEM\n remote: https://rubygems.org/\n specs:\n rack (3.0.0)\n rails (7.0.0)\n rack (>= 2)\n\n\ + PLATFORMS\n ruby\n\nDEPENDENCIES\n rack (= 3.0.0)\n\n\ + CHECKSUMS\n rack (3.0.0) sha256={}\n rails (7.0.0) sha256={}\n\nBUNDLED WITH\n 2.6.2\n", + "4".repeat(64), + "2".repeat(64) + ), + ); + let r = rewrite_registry_redirect(&files, &[gem_override("rails", "7.0.0")]); + let expected = format!( + "GEM\n remote: https://rubygems.org/\n specs:\n rack (3.0.0)\n\n\ + GEM\n remote: https://patch.test/gem/tok/uuid/\n specs:\n rails (7.0.0)\n rack (>= 2)\n\n\ + PLATFORMS\n ruby\n\nDEPENDENCIES\n rack (= 3.0.0)\n rails (= 7.0.0)!\n\n\ + CHECKSUMS\n rack (3.0.0) sha256={}\n rails (7.0.0) sha256={}\n\nBUNDLED WITH\n 2.6.2\n", + "4".repeat(64), + "f".repeat(64) + ); + assert_eq!( + r.files.get("Gemfile.lock"), + Some(&expected), + "spec + sublines moved, dependency added sorted, sibling gem untouched" + ); + let dep_edit = r + .edits + .iter() + .find(|e| e.kind == "redirect_gemfile_lock_dependency_pin") + .unwrap_or_else(|| panic!("DEPENDENCIES pin edit recorded: {:?}", r.edits)); + assert_eq!(dep_edit.action, "added"); + assert_eq!(dep_edit.original, None); + } + + /// Convergence orders its edits on bundler's invariant that source + /// sections precede DEPENDENCIES (the pin insert runs first because its + /// lines sit after the spec-move indices). A hand-edited lock with + /// DEPENDENCIES before GEM breaks that premise — it must fail soft to the + /// mixed state (checksum pinned, GEM attribution untouched, frozen-install + /// caveat), never splice with stale indices and corrupt the lock. + #[test] + fn gem_checksums_lock_dependencies_before_gem_fails_soft_to_mixed() { + 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(), + format!( + "DEPENDENCIES\n rails (= 7.0.0)\n\n\ + GEM\n remote: https://rubygems.org/\n specs:\n rails (7.0.0)\n\n\ + PLATFORMS\n ruby\n\nCHECKSUMS\n rails (7.0.0) sha256={}\n\n\ + BUNDLED WITH\n 2.6.2\n", + "2".repeat(64) + ), + ); + let r = rewrite_registry_redirect(&files, &[gem_override("rails", "7.0.0")]); + let expected = format!( + "DEPENDENCIES\n rails (= 7.0.0)\n\n\ + GEM\n remote: https://rubygems.org/\n specs:\n rails (7.0.0)\n\n\ + PLATFORMS\n ruby\n\nCHECKSUMS\n rails (7.0.0) sha256={}\n\n\ + BUNDLED WITH\n 2.6.2\n", + "f".repeat(64) + ); + assert_eq!( + r.files.get("Gemfile.lock"), + Some(&expected), + "only the CHECKSUMS pin lands — the unconvergeable lock keeps its shape" + ); + assert!( + !r.edits + .iter() + .any(|e| e.kind == "redirect_gemfile_lock_gem_source" + || e.kind == "redirect_gemfile_lock_dependency_pin"), + "no convergence edits on the fail-soft path: {:?}", + r.edits + ); + assert!( + r.warnings + .iter() + .any(|w| w.code == "redirect_gem_frozen_install"), + "the mixed pair keeps the frozen-install caveat: {:?}", + r.warnings + ); + } + /// Bundler's modern `gems.rb`/`gems.locked` spelling must be redirected /// exactly like the classic pair — before this, a gems.rb project was a /// silent no-op (the rewriter keyed on the literal "Gemfile" names). @@ -7295,12 +8173,18 @@ mod tests { "a CRLF CHECKSUMS section must be recognized: {:?}", r.warnings ); - let expected = - gem_lock(&format!(" rails (7.0.0) sha256={}", "f".repeat(64))).replace('\n', "\r\n"); + let expected = format!( + "GEM\n remote: https://rubygems.org/\n specs:\n\n\ + GEM\n remote: https://patch.test/gem/tok/uuid/\n specs:\n rails (7.0.0)\n\n\ + PLATFORMS\n ruby\n\nDEPENDENCIES\n rails (= 7.0.0)!\n\n\ + CHECKSUMS\n rails (7.0.0) sha256={}\n\nBUNDLED WITH\n 2.6.2\n", + "f".repeat(64) + ) + .replace('\n', "\r\n"); assert_eq!( r.files.get("Gemfile.lock"), Some(&expected), - "pin rewritten in place with every \\r\\n preserved" + "pin + convergence rewritten in place with every \\r\\n preserved" ); let edit = r .edits 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 314c0958..d1040579 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 @@ -14,5 +14,21 @@ "key": "rails", "original": "rails (7.0.0) sha256=2222222222222222222222222222222222222222222222222222222222222222", "new": "rails (7.0.0) sha256=deadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeefdeadbeef" + }, + { + "path": "Gemfile.lock", + "kind": "redirect_gemfile_lock_dependency_pin", + "action": "rewritten", + "key": "rails", + "original": "rails (= 7.0.0)", + "new": "rails (= 7.0.0)!" + }, + { + "path": "Gemfile.lock", + "kind": "redirect_gemfile_lock_gem_source", + "action": "rewritten", + "key": "rails", + "original": "https://rubygems.org/", + "new": "https://patch.socket.dev/patch-registry/gem/11111111-1111-1111-1111-111111111111/77777777-7777-7777-7777-777777777777/" } ] diff --git a/crates/socket-patch-core/tests/fixtures/redirect/gem/bundler/basic/expected/Gemfile.lock b/crates/socket-patch-core/tests/fixtures/redirect/gem/bundler/basic/expected/Gemfile.lock index ff6ec24f..cbc30ebd 100644 --- a/crates/socket-patch-core/tests/fixtures/redirect/gem/bundler/basic/expected/Gemfile.lock +++ b/crates/socket-patch-core/tests/fixtures/redirect/gem/bundler/basic/expected/Gemfile.lock @@ -2,6 +2,10 @@ GEM remote: https://rubygems.org/ specs: puma (6.0.0) + +GEM + remote: https://patch.socket.dev/patch-registry/gem/11111111-1111-1111-1111-111111111111/77777777-7777-7777-7777-777777777777/ + specs: rails (7.0.0) PLATFORMS @@ -9,7 +13,7 @@ PLATFORMS DEPENDENCIES puma (= 6.0.0) - rails (= 7.0.0) + rails (= 7.0.0)! CHECKSUMS puma (6.0.0) sha256=1111111111111111111111111111111111111111111111111111111111111111