From e91f98f49a997f1a02e07222376636f715d0c938 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Tue, 18 Aug 2026 21:05:20 -0400 Subject: [PATCH 1/6] =?UTF-8?q?test(gem):=20red=20=E2=80=94=20setup=20--re?= =?UTF-8?q?move=20must=20clear=20bundler's=20.bundle/plugin=20registration?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pins the 2026-08 e2e campaign finding (D3/C3, P2): setup --remove unwires the Gemfile and deletes the generated plugin files, but leaves bundler's machine-local .bundle/plugin/index registration (hook subscriptions + plugin/load paths) dangling, so every later bundle install prints bundler's 5-line "plugin paths don't exist ... Continuing without installing plugin socket-patch" block with a misleading reinstall suggestion. Three failing guards: - host_guard::gem_setup_roundtrip_host now seeds the index exactly as bundler writes it and asserts remove leaves no socket-patch entry (index deleted when nothing else is registered); - host_guard::gem_setup_remove_strips_registration_surgically_under_ bundle_app_config: surgical strip that preserves another plugin's entries verbatim, drops emptied hook events, and follows bundler's BUNDLE_APP_CONFIG resolution (relative value resolves against the project root); - plugin_runtime::setup_remove_clears_bundler_plugin_registration: real host-bundler flow — install registers, remove must clear, and the next bundle install must print neither "plugin paths don't exist" nor "Continuing without installing plugin". Co-Authored-By: Claude Fable 5 --- .../tests/setup_matrix_gem.rs | 207 ++++++++++++++++++ 1 file changed, 207 insertions(+) diff --git a/crates/socket-patch-cli/tests/setup_matrix_gem.rs b/crates/socket-patch-cli/tests/setup_matrix_gem.rs index cb0c202a..014e1d43 100644 --- a/crates/socket-patch-cli/tests/setup_matrix_gem.rs +++ b/crates/socket-patch-cli/tests/setup_matrix_gem.rs @@ -97,6 +97,13 @@ mod host_guard { /// explicit flags alone — nothing reaches authed endpoints and no ambient /// var can stand in for a flag. fn run(cwd: &Path, args: &[&str]) -> (i32, String, String) { + run_env(cwd, args, &[]) + } + + /// [`run`] with extra environment variables for the child (e.g. bundler's + /// `BUNDLE_APP_CONFIG`, which relocates the machine-local registration + /// `--remove` must clean). + fn run_env(cwd: &Path, args: &[&str], envs: &[(&str, &str)]) -> (i32, String, String) { let mut cmd = Command::new(binary()); cmd.args(args).current_dir(cwd); // Prefix-scrub the whole ambient `SOCKET_*` surface (mirrors @@ -119,6 +126,13 @@ mod host_guard { // would strip a developer's own opt-out. Force it off for the child — // no assertion here concerns telemetry. cmd.env("SOCKET_TELEMETRY_DISABLED", "1"); + // An ambient BUNDLE_APP_CONFIG would relocate where `--remove` looks + // for bundler's plugin registration — strip it so only the explicit + // per-test env below can steer that resolution. + cmd.env_remove("BUNDLE_APP_CONFIG"); + for (k, v) in envs { + cmd.env(k, v); + } let out = cmd.output().expect("failed to execute socket-patch binary"); ( out.status.code().unwrap_or(-1), @@ -313,6 +327,27 @@ mod host_guard { // 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(); + // Bundler's machine-local plugin registration, exactly as the first + // `bundle install` after `setup` writes it (bundler's YAMLSerializer + // dialect, verified against bundler 2.7.2 / 4.0.18): the hook + // subscriptions + load/plugin paths in `.bundle/plugin/index`. + // `--remove` must clear it too — a dangling registration makes every + // later `bundle install` print bundler's "The following plugin paths + // don't exist ... Continuing without installing plugin socket-patch" + // block (with a misleading reinstall suggestion) forever. + let plugin_reg_dir = root.join(".bundle").join("plugin"); + std::fs::create_dir_all(&plugin_reg_dir).unwrap(); + let index_path = plugin_reg_dir.join("index"); + std::fs::write( + &index_path, + format!( + "---\ncommands:\nhooks:\n after-install:\n - \"socket-patch\"\n \ + after-install-all:\n - \"socket-patch\"\nload_paths:\n socket-patch:\n \ + - \"{root_s}/.socket/bundler-plugin/.\"\nplugin_paths:\n \ + socket-patch: \"{root_s}/.socket/bundler-plugin\"\nsources:\n" + ), + ) + .unwrap(); let (code, out, err) = run( root, &["setup", "--remove", "--cwd", root_s, "--yes", "--json"], @@ -342,6 +377,23 @@ mod host_guard { !root.join(".socket/.gitignore").exists(), "remove must delete the .gitignore setup created (it held only our line)" ); + // The machine-local registration must be gone too. socket-patch was + // the ONLY registered plugin, so nothing in the index is left worth + // keeping — the index (and thereby every socket-patch entry) must not + // survive. + let residue = std::fs::read_to_string(&index_path).unwrap_or_default(); + assert!( + !residue.contains("socket-patch"), + "remove must clear bundler's machine-local plugin registration \ + (.bundle/plugin/index) — a dangling entry makes every later \ + `bundle install` warn \"plugin paths don't exist ... Continuing \ + without installing plugin socket-patch\":\n{residue}" + ); + assert!( + !index_path.exists(), + "socket-patch was the only registered plugin: the emptied index \ + must be deleted, not left as an all-empty husk" + ); // ── check (after remove): needs_configuration again, exit 1 ───────── let (code, out, _) = run(root, &["setup", "--check", "--cwd", root_s, "--json"]); @@ -356,6 +408,88 @@ mod host_guard { ); } + /// `setup --remove` must clear bundler's machine-local plugin + /// registration SURGICALLY: only the socket-patch entries leave the + /// index; another plugin's registration (its hook subscriptions and + /// paths) survives byte-intact. And the index location must follow + /// bundler's own `BUNDLE_APP_CONFIG` resolution — a relative value + /// resolves against the project root (`Bundler.app_config_path`), not + /// the process cwd or a hardcoded `.bundle`. + #[test] + fn gem_setup_remove_strips_registration_surgically_under_bundle_app_config() { + let tmp = tempfile::tempdir().unwrap(); + let root = tmp.path(); + std::fs::write(root.join("Gemfile"), GEMFILE).unwrap(); + let root_s = root.to_str().unwrap(); + + // Wire the project (the registration below is what bundler would + // write on the first `bundle install` after this). + let (code, out, err) = run_env( + root, + &["setup", "--cwd", root_s, "--yes", "--json"], + &[("BUNDLE_APP_CONFIG", "bundle-config")], + ); + assert_eq!(code, 0, "setup must exit 0.\n{out}\n{err}"); + + // The registration lives under the RELATIVE app-config dir, resolved + // against the project root — bundler's own resolution rule. + let plugin_reg_dir = root.join("bundle-config").join("plugin"); + std::fs::create_dir_all(&plugin_reg_dir).unwrap(); + let index_path = plugin_reg_dir.join("index"); + std::fs::write( + &index_path, + format!( + "---\ncommands:\nhooks:\n after-install:\n - \"other-plugin\"\n \ + - \"socket-patch\"\n after-install-all:\n - \"socket-patch\"\n \ + before-install-all:\n - \"other-plugin\"\nload_paths:\n other-plugin:\n \ + - \"{root_s}/plugins/other-plugin/.\"\n socket-patch:\n \ + - \"{root_s}/.socket/bundler-plugin/.\"\nplugin_paths:\n \ + other-plugin: \"{root_s}/plugins/other-plugin\"\n \ + socket-patch: \"{root_s}/.socket/bundler-plugin\"\nsources:\n" + ), + ) + .unwrap(); + + let (code, out, err) = run_env( + root, + &["setup", "--remove", "--cwd", root_s, "--yes", "--json"], + &[("BUNDLE_APP_CONFIG", "bundle-config")], + ); + assert_eq!(code, 0, "remove must exit 0.\n{out}\n{err}"); + assert_eq!( + json_str(&parse_json(&out, "remove"), "status", "remove"), + "success" + ); + + let index = std::fs::read_to_string(&index_path).unwrap_or_else(|e| { + panic!( + "the index must SURVIVE (another plugin is still registered), \ + not be deleted wholesale: {e}" + ) + }); + assert!( + !index.contains("socket-patch"), + "every socket-patch registration entry must be stripped:\n{index}" + ); + for kept in [ + " after-install:\n - \"other-plugin\"", + " before-install-all:\n - \"other-plugin\"", + &format!(" other-plugin:\n - \"{root_s}/plugins/other-plugin/.\"") as &str, + &format!(" other-plugin: \"{root_s}/plugins/other-plugin\"") as &str, + ] { + assert!( + index.contains(kept), + "the OTHER plugin's registration must survive verbatim — \ + missing {kept:?} in:\n{index}" + ); + } + assert!( + !index.contains("after-install-all:"), + "a hook event left with NO subscribers must be dropped, not left \ + as an empty key bundler chokes on:\n{index}" + ); + } + /// `bundle` resolves the Gemfile by walking UP from the invocation dir, /// and `discover_bundler_project` documents the same contract. Run from a /// subdirectory with NO `--cwd` flag the CLI defaults to the RELATIVE @@ -846,6 +980,79 @@ mod plugin_runtime { ); } + /// [P2 remove leaves registration dangling] Bundler records the plugin + /// machine-locally at first install (`.bundle/plugin/index`: hook + /// subscriptions + plugin/load paths). `setup --remove` unwires the + /// Gemfile and deletes the generated plugin dir — if it leaves that + /// registration behind, EVERY later `bundle install` prints bundler's + /// 5-line "The following plugin paths don't exist ... Continuing without + /// installing plugin socket-patch" block with a misleading reinstall + /// suggestion (install still exits 0, so nothing ever heals it). + /// Reproduced against real bundler 2.7.2 and 4.0.18 in the 2026-08 e2e + /// campaign; this drives the same flow with the host bundler. + #[test] + fn setup_remove_clears_bundler_plugin_registration() { + 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); + + // First install: bundler registers the plugin machine-locally. + let (code, out, err) = bundle_install(root, &fake, &[]); + assert_eq!(code, 0, "wired install must succeed.\n{out}\n{err}"); + let index_path = root.join(".bundle/plugin/index"); + assert!( + std::fs::read_to_string(&index_path) + .unwrap_or_default() + .contains("socket-patch"), + "precondition: bundler must have registered the plugin at {}", + index_path.display() + ); + + // Unwire. + let mut cmd = Command::new(binary()); + cmd.args(["setup", "--remove", "--yes", "--json"]) + .current_dir(root); + scrub(&mut cmd); + let (code, out, err) = run(cmd); + assert_eq!(code, 0, "setup --remove must exit 0.\n{out}\n{err}"); + assert!( + out.contains("\"status\": \"success\""), + "setup --remove must report success:\n{out}" + ); + + // No socket-patch registration may survive under .bundle/plugin. + let residue = std::fs::read_to_string(&index_path).unwrap_or_default(); + assert!( + !residue.contains("socket-patch"), + ".bundle/plugin must hold no socket-patch entry after remove:\n{residue}" + ); + + // And the REAL oracle: the next bundle install is silent about the + // unwired plugin — no "plugin paths don't exist", no "Continuing + // without installing plugin", on either stream. + let (code, out, err) = bundle_install(root, &fake, &[]); + assert_eq!( + code, 0, + "post-remove install must still succeed.\n{out}\n{err}" + ); + let combined = format!("{out}\n{err}"); + assert!( + !combined.contains("plugin paths don't exist"), + "post-remove `bundle install` must not warn about the unwired \ + plugin's missing paths:\n{combined}" + ); + assert!( + !combined.contains("Continuing without installing plugin"), + "post-remove `bundle install` must not print bundler's \ + skipped-plugin block:\n{combined}" + ); + } + /// [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 From f8ee008de40cce7e76f7499f66fd1723713a0fdf Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Tue, 18 Aug 2026 21:16:07 -0400 Subject: [PATCH 2/6] fix(gem): setup --remove clears bundler's machine-local .bundle/plugin registration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Campaign finding D3/C3 (P2, bundler 2.7.2 + 4.0.18 repros): the unwire deleted the generated plugin files/stamp and restored the Gemfile, but never touched the .bundle/plugin/index registration (hook subscriptions and plugin/load paths) bundler wrote at the wired plugin's first install, so every later bundle install printed bundler's 5-line "plugin paths don't exist ... Continuing without installing plugin socket-patch" block with a misleading reinstall suggestion, forever. remove_plugin_directive now runs a third step after the Gemfile un-wire and plugin-file removal (never before — while the directive is wired the registration is live state, not residue): - resolves the index location exactly like Bundler.app_config_path: $BUNDLE_APP_CONFIG when set (relative values resolve against the project root — the official ruby images export BUNDLE_APP_CONFIG=/usr/local/bundle), else /.bundle; - surgically strips socket-patch from the index, parsing only the exact dialect bundler's YAMLSerializer writes: plugin_paths/ load_paths keys, hooks subscriptions (dropping an event key left with no subscribers), commands/sources mappings — every other plugin's line is kept byte-verbatim, and unknown sections survive untouched; - deletes a bundler-plugin-installed copy only when the recorded dir sits inside the plugin root, never a path elsewhere; - deletes the index outright when no plugin remains (bundler treats a missing index as empty) and prunes the emptied plugin/app-config dirs (remove_dir, so a .bundle/config keeps its parent); - an index in any OTHER shape is never rewritten on a guess: the cleanup reports residue as a gem_plugin_registration files[] error whose message carries the remedy (`bundler plugin uninstall socket-patch`). The cleanup is reported as its own files[] entry (kind gem_plugin_registration, remove-only, emitted only when a registration existed) — contract + README updated; a project whose only leftover is the registration now counts as removable, so a re-run of setup --remove heals residue left by older CLIs. Co-Authored-By: Claude Fable 5 --- README.md | 2 +- crates/socket-patch-cli/CLI_CONTRACT.md | 5 +- crates/socket-patch-core/src/setup/gem/mod.rs | 605 ++++++++++++++++++ .../socket-patch-core/src/setup/gem/update.rs | 99 ++- 4 files changed, 705 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 8ddaeddc..ee3a94e9 100644 --- a/README.md +++ b/README.md @@ -766,7 +766,7 @@ socket-patch setup --remove # revert what setup added | Flag | Env var | Description | |------|---------|-------------| | `--check` | — | Read-only verification that every manifest is configured **and** every installed patch is still applied on disk (each file matches its recorded `afterHash`); exits non-zero if any manifest still needs setup or a patch has drifted. Never writes (safe in CI). Conflicts with `--remove`. | -| `--remove` | — | Revert every install hook `setup` added (npm `package.json` scripts, the Python `socket-patch[hook]` dependency, the gem Bundler plugin wiring, and the Composer `post-install-cmd`/`post-update-cmd` script entries). | +| `--remove` | — | Revert every install hook `setup` added (npm `package.json` scripts, the Python `socket-patch[hook]` dependency, the gem Bundler plugin wiring — including bundler's machine-local `.bundle/plugin` registration, so later `bundle install`s don't warn about the unwired plugin — and the Composer `post-install-cmd`/`post-update-cmd` script entries). If the registration can't be cleared automatically (unexpected index format), the error names the fallback: `bundler plugin uninstall socket-patch`. | | `--exclude ` | `SOCKET_SETUP_EXCLUDE` | Workspace-member path(s) to exclude from setup (comma-separated, relative to the repo root). The exclusion is persisted in `.socket/manifest.json`, so `setup --check` and a fresh clone honor it without re-passing the flag. | #### Disabling / opting out (Python hook) diff --git a/crates/socket-patch-cli/CLI_CONTRACT.md b/crates/socket-patch-cli/CLI_CONTRACT.md index 30e0e1dd..1f26008a 100644 --- a/crates/socket-patch-cli/CLI_CONTRACT.md +++ b/crates/socket-patch-cli/CLI_CONTRACT.md @@ -294,7 +294,10 @@ wins, pinned by `find_by_purls_prefers_root_copy_over_nested_duplicate`). `setup` predates the v3.0 unified envelope and emits its own three shapes. They are stable as of v3.0; consumers may rely on these keys. All three share a `files[*]` entry shape; `kind` is one of -`package_json`, `pth`, `gemfile`, `gem_plugin`, `composer`. +`package_json`, `pth`, `gemfile`, `gem_plugin`, `composer`, `gem_plugin_registration` (the last is +`setup --remove`-only: clearing bundler's machine-local `.bundle/plugin` registration of the wired +plugin — emitted only when a registration existed; `status: error` carries the +`bundler plugin uninstall socket-patch` remedy when it could not be cleared safely). **`setup`:** diff --git a/crates/socket-patch-core/src/setup/gem/mod.rs b/crates/socket-patch-core/src/setup/gem/mod.rs index 871aa399..9f64d995 100644 --- a/crates/socket-patch-core/src/setup/gem/mod.rs +++ b/crates/socket-patch-core/src/setup/gem/mod.rs @@ -202,6 +202,338 @@ async fn remove_stamp_artifacts(root: &Path) { } } +// ───────────────────────────────────────────────────────────────────────── +// Bundler's machine-local plugin registration (`.bundle/plugin/index`) +// ───────────────────────────────────────────────────────────────────────── + +/// The plugin name our Gemfile directive registers — the key/value bundler +/// records in its machine-local plugin index at first install. +const PLUGIN_NAME: &str = "socket-patch"; + +/// Outcome of clearing bundler's machine-local plugin registration on +/// `setup --remove`. +#[derive(Debug)] +pub enum GemRegistrationCleanup { + /// socket-patch entries were found and removed (or would be, on dry-run). + Cleaned { + /// The registration index that held (or holds, on dry-run) them. + index: PathBuf, + }, + /// No socket-patch registration exists — nothing to clean. + NotRegistered, + /// A socket-patch registration is (probably) present but could not be + /// cleared safely; the human remedy must be surfaced. + Residue { + /// The registration index carrying the leftover entries. + index: PathBuf, + /// Why the surgical cleanup refused/failed. + reason: String, + }, +} + +/// Bundler's app-config dir for `root`, following `Bundler.app_config_path` +/// exactly: `$BUNDLE_APP_CONFIG` when set (a relative value resolves against +/// the project root, NOT the process cwd), else `/.bundle`. The +/// machine-local plugin registration lives at `/plugin/index`, so +/// getting this resolution wrong means `--remove` cleans (or misses) the +/// wrong directory — e.g. under the official ruby Docker images, which export +/// `BUNDLE_APP_CONFIG=/usr/local/bundle`. +fn bundler_app_config_dir(root: &Path, env_value: Option<&std::ffi::OsStr>) -> PathBuf { + match env_value { + Some(v) if !v.is_empty() => { + let p = PathBuf::from(v); + if p.is_absolute() { + p + } else { + root.join(p) + } + } + _ => root.join(".bundle"), + } +} + +/// One `key:` entry of a section in bundler's plugin index, carrying the raw +/// lines it spans so kept entries are rebuilt byte-verbatim. +struct IndexEntry { + /// The entry key (unquoted): a plugin name, or a hook event name. + key: String, + key_line: String, + /// Inline scalar value (`plugin_paths`-style `key: "value"`), unquoted. + value: Option, + /// `- "item"` lines under the key, with their unquoted values. + items: Vec<(String, String)>, +} + +/// One top-level section (`commands` / `hooks` / `load_paths` / +/// `plugin_paths` / `sources`) of the index. +struct IndexSection { + name: String, + header_line: String, + entries: Vec, +} + +/// Strip a matching pair of quotes, mirroring bundler's `YAMLSerializer` +/// loader (its regexes accept an optional `'`/`"` wrapper around values). +fn unquote(s: &str) -> &str { + let s = s.trim(); + let b = s.as_bytes(); + if s.len() >= 2 && ((b[0] == b'"' && b[s.len() - 1] == b'"') || (b[0] == b'\'' && b[s.len() - 1] == b'\'')) { + &s[1..s.len() - 1] + } else { + s + } +} + +/// Parse bundler's plugin index into sections/entries, refusing anything +/// outside the exact dialect bundler's `YAMLSerializer` writes (`---`, then +/// two-level `key:` maps with 2-space indents and `- "item"` array lines). +/// The refusal matters more than the acceptance: a hand-edited or +/// future-format index must fall through to the "residue remains" remedy +/// path, never be rewritten on a guess and corrupted. +fn parse_plugin_index(content: &str) -> Result, String> { + let mut sections: Vec = Vec::new(); + for (n, line) in content.lines().enumerate() { + let lineno = n + 1; + if n == 0 && line.trim_end() == "---" { + continue; + } + if let Some(body) = line.strip_prefix(" ") { + // Second level: an array item or an entry key. + if body.starts_with(' ') || body.starts_with('\t') { + return Err(format!("line {lineno}: unexpected indentation")); + } + let section = sections + .last_mut() + .ok_or_else(|| format!("line {lineno}: entry before any section"))?; + if let Some(item) = body.strip_prefix("- ") { + let entry = section + .entries + .last_mut() + .ok_or_else(|| format!("line {lineno}: array item before any key"))?; + entry + .items + .push((line.to_string(), unquote(item).to_string())); + } else if let Some((key, value)) = body.split_once(": ") { + section.entries.push(IndexEntry { + key: unquote(key).to_string(), + key_line: line.to_string(), + value: Some(unquote(value).to_string()), + items: Vec::new(), + }); + } else if let Some(key) = body.strip_suffix(':') { + section.entries.push(IndexEntry { + key: unquote(key).to_string(), + key_line: line.to_string(), + value: None, + items: Vec::new(), + }); + } else { + return Err(format!("line {lineno}: unrecognized entry line")); + } + } else if let Some(name) = line.strip_suffix(':') { + if name.is_empty() || name.contains(' ') || line.starts_with(' ') { + return Err(format!("line {lineno}: unrecognized section line")); + } + sections.push(IndexSection { + name: name.to_string(), + header_line: line.to_string(), + entries: Vec::new(), + }); + } else { + return Err(format!("line {lineno}: unrecognized line")); + } + } + Ok(sections) +} + +/// The result of surgically removing socket-patch from a parsed index. +struct StrippedIndex { + /// The index content with every socket-patch entry removed; kept lines + /// are byte-verbatim. + content: String, + /// Whether any OTHER plugin's registration remains (the index must then + /// survive; an all-empty index is deleted instead). + plugins_remain: bool, + /// The dir bundler recorded as the plugin's install location + /// (`plugin_paths`), if present. + installed_dir: Option, +} + +/// Pure transform: drop every socket-patch entry from the index — its +/// `plugin_paths`/`load_paths` keys, its `hooks` subscriptions (removing an +/// event key left with no subscribers), and any `commands`/`sources` mapping +/// to it — while preserving every other plugin's lines byte-verbatim. +/// `Ok(None)` when nothing of ours is registered; `Err` when the file is not +/// in bundler's dialect (the caller must warn, never write). +fn strip_plugin_registration(content: &str) -> Result, String> { + let mut sections = parse_plugin_index(content)?; + let mut changed = false; + let mut installed_dir = None; + for section in &mut sections { + match section.name.as_str() { + "plugin_paths" | "load_paths" => { + let is_plugin_paths = section.name == "plugin_paths"; + section.entries.retain(|e| { + if e.key != PLUGIN_NAME { + return true; + } + if is_plugin_paths { + if let Some(v) = &e.value { + installed_dir = Some(PathBuf::from(v)); + } + } + changed = true; + false + }); + } + "hooks" => { + section.entries.retain_mut(|e| { + let before = e.items.len(); + e.items.retain(|(_, v)| v != PLUGIN_NAME); + let lost = e.items.len() != before; + changed |= lost; + // Drop an event key we just emptied — bundler never + // writes a subscriber-less event, so leaving one behind + // is not round-trippable. Entries empty on arrival are + // not ours to judge and stay. + !(lost && e.items.is_empty() && e.value.is_none()) + }); + } + "commands" | "sources" => { + section.entries.retain(|e| { + if e.value.as_deref() == Some(PLUGIN_NAME) { + changed = true; + false + } else { + true + } + }); + } + // Unknown section (a future bundler's addition): keep verbatim. + _ => {} + } + } + if !changed { + return Ok(None); + } + let plugins_remain = sections.iter().any(|s| !s.entries.is_empty()); + let mut out = String::from("---\n"); + for section in §ions { + out.push_str(§ion.header_line); + out.push('\n'); + for entry in §ion.entries { + out.push_str(&entry.key_line); + out.push('\n'); + for (line, _) in &entry.items { + out.push_str(line); + out.push('\n'); + } + } + } + Ok(Some(StrippedIndex { + content: out, + plugins_remain, + installed_dir, + })) +} + +/// Clear bundler's machine-local plugin registration for socket-patch on +/// `setup --remove` — the `.bundle/plugin/index` entries (hook subscriptions +/// and plugin/load paths) bundler wrote when the wired plugin first installed. +/// `remove_plugin_files` deletes the plugin *source*; without this step the +/// registration dangles and every later `bundle install` prints bundler's +/// "The following plugin paths don't exist ... Continuing without installing +/// plugin socket-patch" block (with a misleading reinstall suggestion) +/// forever. Surgical: only socket-patch entries leave the index; another +/// plugin's registration survives byte-verbatim, and an index in an +/// unexpected format is never rewritten on a guess — that reports +/// [`GemRegistrationCleanup::Residue`] so the caller surfaces the +/// `bundler plugin uninstall socket-patch` remedy instead. +pub async fn remove_plugin_registration(root: &Path, dry_run: bool) -> GemRegistrationCleanup { + let env = std::env::var_os("BUNDLE_APP_CONFIG"); + remove_plugin_registration_at(root, env.as_deref(), dry_run).await +} + +/// [`remove_plugin_registration`] with the `BUNDLE_APP_CONFIG` resolution +/// input made explicit (tests inject it; the public entry reads the process +/// env, exactly like bundler itself). +async fn remove_plugin_registration_at( + root: &Path, + app_config_env: Option<&std::ffi::OsStr>, + dry_run: bool, +) -> GemRegistrationCleanup { + let plugin_root = bundler_app_config_dir(root, app_config_env).join("plugin"); + let index = plugin_root.join("index"); + let content = match fs::read_to_string(&index).await { + Ok(c) => c, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => { + return GemRegistrationCleanup::NotRegistered; + } + Err(e) => { + return GemRegistrationCleanup::Residue { + index, + reason: format!("could not read it: {e}"), + }; + } + }; + if !content.contains(PLUGIN_NAME) { + return GemRegistrationCleanup::NotRegistered; + } + let stripped = match strip_plugin_registration(&content) { + Ok(Some(s)) => s, + // Mentioned only incidentally (e.g. inside another plugin's path): + // nothing registered under our name. + Ok(None) => return GemRegistrationCleanup::NotRegistered, + Err(reason) => { + return GemRegistrationCleanup::Residue { + index, + reason: format!("unexpected index format ({reason})"), + }; + } + }; + if dry_run { + return GemRegistrationCleanup::Cleaned { index }; + } + // The dir bundler recorded as the plugin's install location. For our + // `path:`-sourced wiring that is the project's `.socket/bundler-plugin` + // (already deleted by `remove_plugin_files`); a `bundler plugin install`ed + // copy lives under the plugin root itself. Delete it only inside that + // root — never a recorded path elsewhere on the machine. + if let Some(dir) = &stripped.installed_dir { + if dir.starts_with(&plugin_root) && dir != &plugin_root { + let _ = fs::remove_dir_all(dir).await; + } + } + let write_result = if stripped.plugins_remain { + // Another plugin is still registered: rewrite the index without our + // entries (staged + renamed — a torn index would break EVERY plugin). + crate::utils::fs::atomic_write_bytes_preserving_mode(&index, stripped.content.as_bytes()) + .await + .map_err(|e| format!("could not rewrite it: {e}")) + } else { + // Nothing registered anymore: delete the index outright (bundler + // treats a missing index as empty and regenerates it on demand). + match fs::remove_file(&index).await { + Ok(()) => Ok(()), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(e) => Err(format!("could not delete it: {e}")), + } + }; + if let Err(reason) = write_result { + return GemRegistrationCleanup::Residue { index, reason }; + } + if !stripped.plugins_remain { + // Prune the now-empty machine-local plugin dir (and an app-config dir + // that held nothing else). `remove_dir` refuses non-empty dirs, so a + // `.bundle/config` or another plugin's `gems/` cache keeps its parent. + let _ = fs::remove_dir(&plugin_root).await; + if let Some(parent) = plugin_root.parent() { + let _ = fs::remove_dir(parent).await; + } + } + GemRegistrationCleanup::Cleaned { index } +} + /// 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). @@ -927,6 +1259,279 @@ mod tests { ); } + // ── bundler machine-local plugin registration cleanup ──────────── + + /// The exact index bundler 4.0.18 wrote in the 2026-08 e2e campaign + /// repro (agent-b4/eject-setup-remove), path root substituted. + fn solo_index(root: &str) -> String { + format!( + "---\ncommands:\nhooks:\n after-install:\n - \"socket-patch\"\n \ + after-install-all:\n - \"socket-patch\"\nload_paths:\n socket-patch:\n \ + - \"{root}/.socket/bundler-plugin/.\"\nplugin_paths:\n \ + socket-patch: \"{root}/.socket/bundler-plugin\"\nsources:\n" + ) + } + + #[test] + fn test_strip_registration_solo_plugin_empties_index() { + let stripped = strip_plugin_registration(&solo_index("/proj")) + .expect("bundler's own dump must parse") + .expect("socket-patch entries must be found"); + assert!( + !stripped.plugins_remain, + "socket-patch was the only plugin — nothing may remain" + ); + assert!( + !stripped.content.contains(PLUGIN_NAME), + "no socket-patch line may survive:\n{}", + stripped.content + ); + assert_eq!( + stripped.installed_dir.as_deref(), + Some(Path::new("/proj/.socket/bundler-plugin")), + "the recorded install dir is surfaced for containment-gated deletion" + ); + // The emptied hook events are dropped with their subscribers. + assert!(!stripped.content.contains("after-install")); + } + + #[test] + fn test_strip_registration_preserves_other_plugins_verbatim() { + let index = "---\ncommands:\n mycmd: \"other-plugin\"\nhooks:\n after-install:\n \ + - \"other-plugin\"\n - \"socket-patch\"\n after-install-all:\n \ + - \"socket-patch\"\nload_paths:\n other-plugin:\n - \"/x/other/.\"\n \ + socket-patch:\n - \"/proj/.socket/bundler-plugin/.\"\nplugin_paths:\n \ + other-plugin: \"/x/other\"\n socket-patch: \"/proj/.socket/bundler-plugin\"\nsources:\n"; + let stripped = strip_plugin_registration(index) + .expect("parses") + .expect("has our entries"); + assert!(stripped.plugins_remain, "other-plugin is still registered"); + assert_eq!( + stripped.content, + "---\ncommands:\n mycmd: \"other-plugin\"\nhooks:\n after-install:\n \ + - \"other-plugin\"\nload_paths:\n other-plugin:\n - \"/x/other/.\"\n\ + plugin_paths:\n other-plugin: \"/x/other\"\nsources:\n", + "only socket-patch lines leave; every kept line is byte-verbatim, \ + and the after-install-all event we emptied is dropped" + ); + } + + #[test] + fn test_strip_registration_none_when_not_ours() { + // Another plugin whose PATH merely mentions socket-patch: nothing + // registered under our name — nothing to strip, nothing to write. + let index = "---\ncommands:\nhooks:\n after-install:\n - \"other\"\nload_paths:\n \ + other:\n - \"/mono/socket-patch-fork/other/.\"\nplugin_paths:\n \ + other: \"/mono/socket-patch-fork/other\"\nsources:\n"; + assert!(strip_plugin_registration(index) + .expect("parses") + .is_none()); + } + + #[test] + fn test_strip_registration_refuses_unknown_shape() { + // Psych-style deeper nesting is NOT bundler's dialect: refuse rather + // than rewrite on a guess (the caller warns with the remedy). + let psych = "---\nhooks:\n after-install:\n - socket-patch\n"; + assert!(strip_plugin_registration(psych).is_err()); + let garbage = "socket-patch says hi\n"; + assert!(strip_plugin_registration(garbage).is_err()); + } + + #[test] + fn test_bundler_app_config_dir_resolution() { + use std::ffi::OsStr; + let root = Path::new("/proj"); + // Unset / empty → /.bundle. + assert_eq!(bundler_app_config_dir(root, None), Path::new("/proj/.bundle")); + assert_eq!( + bundler_app_config_dir(root, Some(OsStr::new(""))), + Path::new("/proj/.bundle") + ); + // Relative → resolved against the PROJECT ROOT (Bundler.app_config_path). + assert_eq!( + bundler_app_config_dir(root, Some(OsStr::new("bundle-config"))), + Path::new("/proj/bundle-config") + ); + // Absolute → taken as-is (the official ruby images' /usr/local/bundle). + assert_eq!( + bundler_app_config_dir(root, Some(OsStr::new("/usr/local/bundle"))), + Path::new("/usr/local/bundle") + ); + } + + #[tokio::test] + async fn test_remove_registration_missing_index_is_not_registered() { + let dir = tempfile::tempdir().unwrap(); + assert!(matches!( + remove_plugin_registration_at(dir.path(), None, false).await, + GemRegistrationCleanup::NotRegistered + )); + } + + #[tokio::test] + async fn test_remove_registration_solo_deletes_index_and_prunes_dirs() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let index = root.join(".bundle/plugin/index"); + write(&index, &solo_index(&root.display().to_string())).await; + + let r = remove_plugin_registration_at(root, None, false).await; + assert!(matches!(r, GemRegistrationCleanup::Cleaned { .. }), "{r:?}"); + assert!(!index.exists(), "emptied index deleted"); + assert!( + !root.join(".bundle").exists(), + "the plugin dir and an app-config dir holding nothing else are pruned" + ); + } + + #[tokio::test] + async fn test_remove_registration_keeps_nonempty_app_config_dir() { + // A real project's .bundle/ holds a config file too: the index (and + // the emptied plugin dir) go, the user's config survives. + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + write(&root.join(".bundle/config"), "---\nBUNDLE_PATH: \"vendor/bundle\"\n").await; + let index = root.join(".bundle/plugin/index"); + write(&index, &solo_index(&root.display().to_string())).await; + + let r = remove_plugin_registration_at(root, None, false).await; + assert!(matches!(r, GemRegistrationCleanup::Cleaned { .. }), "{r:?}"); + assert!(!root.join(".bundle/plugin").exists(), "plugin dir pruned"); + assert!( + root.join(".bundle/config").is_file(), + "the user's bundler config must survive" + ); + } + + #[tokio::test] + async fn test_remove_registration_rewrites_index_when_others_remain() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let index = root.join(".bundle/plugin/index"); + write( + &index, + "---\ncommands:\nhooks:\n after-install:\n - \"other\"\n - \"socket-patch\"\n\ + load_paths:\n other:\n - \"/x/other/.\"\n socket-patch:\n - \"/proj/p/.\"\n\ + plugin_paths:\n other: \"/x/other\"\n socket-patch: \"/proj/p\"\nsources:\n", + ) + .await; + + let r = remove_plugin_registration_at(root, None, false).await; + assert!(matches!(r, GemRegistrationCleanup::Cleaned { .. }), "{r:?}"); + let body = fs::read_to_string(&index).await.unwrap(); + assert!(!body.contains("socket-patch"), "ours gone:\n{body}"); + assert!(body.contains("- \"other\""), "theirs kept:\n{body}"); + } + + #[tokio::test] + async fn test_remove_registration_dry_run_writes_nothing() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let index = root.join(".bundle/plugin/index"); + let body = solo_index(&root.display().to_string()); + write(&index, &body).await; + + let r = remove_plugin_registration_at(root, None, true).await; + assert!(matches!(r, GemRegistrationCleanup::Cleaned { .. }), "{r:?}"); + assert_eq!( + fs::read_to_string(&index).await.unwrap(), + body, + "dry-run must not touch the index" + ); + } + + #[tokio::test] + async fn test_remove_registration_honors_bundle_app_config() { + // Relative BUNDLE_APP_CONFIG resolves against the project root — + // the same rule bundler applies (`Bundler.app_config_path`). + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let index = root.join("bundle-config/plugin/index"); + write(&index, &solo_index(&root.display().to_string())).await; + // A decoy at the default location must NOT be the one cleaned. + let decoy = root.join(".bundle/plugin/index"); + write(&decoy, &solo_index("/elsewhere")).await; + + let r = remove_plugin_registration_at( + root, + Some(std::ffi::OsStr::new("bundle-config")), + false, + ) + .await; + assert!(matches!(r, GemRegistrationCleanup::Cleaned { .. }), "{r:?}"); + assert!(!index.exists(), "the app-config index is the one cleared"); + assert!( + decoy.is_file(), + "the default-location index is out of scope when BUNDLE_APP_CONFIG points elsewhere" + ); + } + + #[tokio::test] + async fn test_remove_registration_unparseable_reports_residue() { + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let index = root.join(".bundle/plugin/index"); + // Mentions socket-patch but is not bundler's dialect: never rewrite + // on a guess — report residue so the caller surfaces the remedy. + let body = "%TAG !u! tag:example\n---\nplugins: [socket-patch]\n"; + write(&index, body).await; + + let r = remove_plugin_registration_at(root, None, false).await; + assert!(matches!(r, GemRegistrationCleanup::Residue { .. }), "{r:?}"); + assert_eq!( + fs::read_to_string(&index).await.unwrap(), + body, + "an unrecognized index must survive byte-identical" + ); + } + + #[tokio::test] + async fn test_remove_registration_deletes_installed_dir_only_inside_plugin_root() { + // A `bundler plugin install`ed copy lives under .bundle/plugin — that + // dir goes. A recorded path OUTSIDE the plugin root (our path-sourced + // project dir, or anything else on the machine) is never touched here. + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let inside = root.join(".bundle/plugin/gems/socket-patch-0.1.0"); + fs::create_dir_all(&inside).await.unwrap(); + write(&inside.join("plugins.rb"), "# installed copy\n").await; + let index = root.join(".bundle/plugin/index"); + write( + &index, + &format!( + "---\ncommands:\nhooks:\n after-install:\n - \"socket-patch\"\n\ + load_paths:\n socket-patch:\n - \"{0}/.\"\nplugin_paths:\n \ + socket-patch: \"{0}\"\nsources:\n", + inside.display() + ), + ) + .await; + let r = remove_plugin_registration_at(root, None, false).await; + assert!(matches!(r, GemRegistrationCleanup::Cleaned { .. }), "{r:?}"); + assert!(!inside.exists(), "the in-root installed copy is deleted"); + + // Outside the plugin root: left alone. + let outside = root.join("elsewhere/socket-patch"); + fs::create_dir_all(&outside).await.unwrap(); + let index2 = root.join(".bundle/plugin/index"); + write( + &index2, + &format!( + "---\ncommands:\nhooks:\nload_paths:\n socket-patch:\n - \"{0}/.\"\n\ + plugin_paths:\n socket-patch: \"{0}\"\nsources:\n", + outside.display() + ), + ) + .await; + let r = remove_plugin_registration_at(root, None, false).await; + assert!(matches!(r, GemRegistrationCleanup::Cleaned { .. }), "{r:?}"); + assert!( + outside.is_dir(), + "a recorded dir outside the plugin root must never be deleted" + ); + } + #[tokio::test] async fn test_add_appends_gitignore_line_to_unterminated_file() { // A .gitignore whose last line has no trailing newline must not have diff --git a/crates/socket-patch-core/src/setup/gem/update.rs b/crates/socket-patch-core/src/setup/gem/update.rs index 18c6f210..e58d4f51 100644 --- a/crates/socket-patch-core/src/setup/gem/update.rs +++ b/crates/socket-patch-core/src/setup/gem/update.rs @@ -10,7 +10,10 @@ use std::path::Path; use tokio::fs; -use super::{add_plugin_files, remove_plugin_files, BundlerProject}; +use super::{ + add_plugin_files, remove_plugin_files, remove_plugin_registration, BundlerProject, + GemRegistrationCleanup, +}; use crate::utils::fs::atomic_write_bytes_preserving_mode; /// Outcome of one setup edit. @@ -23,7 +26,8 @@ pub enum GemSetupStatus { #[derive(Debug)] pub struct GemEditResult { - /// Envelope `files[].kind` (`gemfile` | `gem_plugin`). + /// Envelope `files[].kind` (`gemfile` | `gem_plugin` | + /// `gem_plugin_registration`). pub kind: &'static str, pub path: String, pub status: GemSetupStatus, @@ -232,13 +236,22 @@ pub async fn add_plugin_directive(project: &BundlerProject, dry_run: bool) -> Ve } /// Unwire the project: strip the Gemfile block (byte-for-byte restore), then -/// delete the generated plugin directory. +/// delete the generated plugin directory, then clear bundler's machine-local +/// `.bundle/plugin` registration of the plugin. /// /// Mirror of [`add_plugin_directive`]'s ordering contract, from the other end: /// the files are deleted only once the directive referencing them is gone. A /// failed un-wire that still deleted the plugin dir would leave the Gemfile /// pointing at a path that no longer exists, breaking every later /// `bundle install` (exit 13) on a project that installed fine before. +/// +/// The registration comes last (and only after the Gemfile un-wire held): +/// while the `plugin` directive is still in the Gemfile the registration is +/// live state bundler needs, not residue. Left behind after a successful +/// unwire, it makes every later `bundle install` print bundler's "plugin +/// paths don't exist ... Continuing without installing plugin socket-patch" +/// block forever, so the cleanup failure/refusal path surfaces the +/// `bundler plugin uninstall socket-patch` remedy as a `files[]` error. pub async fn remove_plugin_directive( project: &BundlerProject, dry_run: bool, @@ -247,7 +260,30 @@ pub async fn remove_plugin_directive( if gemfile.status == GemSetupStatus::Error { return vec![gemfile]; } - vec![gemfile, remove_plugin_files(&project.root, dry_run).await] + let mut results = vec![gemfile, remove_plugin_files(&project.root, dry_run).await]; + match remove_plugin_registration(&project.root, dry_run).await { + GemRegistrationCleanup::Cleaned { index } => results.push(GemEditResult { + kind: "gem_plugin_registration", + path: index.display().to_string(), + status: GemSetupStatus::Updated, + error: None, + }), + // The common pre-first-install case (bundler never registered the + // plugin): no entry — there was nothing machine-local to remove. + GemRegistrationCleanup::NotRegistered => {} + GemRegistrationCleanup::Residue { index, reason } => results.push(GemEditResult { + kind: "gem_plugin_registration", + path: index.display().to_string(), + status: GemSetupStatus::Error, + error: Some(format!( + "could not clear bundler's machine-local plugin registration at {} \ + ({reason}); run `bundler plugin uninstall socket-patch` to remove it, \ + or every later `bundle install` will warn about the unwired plugin", + index.display() + )), + }), + } + results } #[cfg(test)] @@ -714,6 +750,61 @@ mod tests { ); } + #[tokio::test] + async fn test_remove_clears_bundler_plugin_registration_entry() { + // A project bundler has already installed once: the machine-local + // `.bundle/plugin/index` registration exists. `remove` must clear it + // and report the cleanup as its own `gem_plugin_registration` entry. + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + fs::write(root.join("Gemfile"), GEMFILE).await.unwrap(); + let project = super::super::discover_bundler_project(root).await.unwrap(); + assert!(add_plugin_directive(&project, false) + .await + .iter() + .all(|r| r.status == GemSetupStatus::Updated)); + let index = root.join(".bundle").join("plugin").join("index"); + fs::create_dir_all(index.parent().unwrap()).await.unwrap(); + fs::write( + &index, + format!( + "---\ncommands:\nhooks:\n after-install:\n - \"socket-patch\"\n \ + after-install-all:\n - \"socket-patch\"\nload_paths:\n socket-patch:\n \ + - \"{0}/.socket/bundler-plugin/.\"\nplugin_paths:\n \ + socket-patch: \"{0}/.socket/bundler-plugin\"\nsources:\n", + root.display() + ), + ) + .await + .unwrap(); + + let removed = remove_plugin_directive(&project, false).await; + assert!( + removed + .iter() + .any(|r| r.kind == "gem_plugin_registration" + && r.status == GemSetupStatus::Updated), + "the registration cleanup must be reported: {removed:?}" + ); + assert!( + !index.exists(), + "the socket-patch-only registration index must be gone" + ); + // Absent registration (the pre-first-install case): no entry at all. + fs::write(root.join("Gemfile"), gemfile_add(GEMFILE).unwrap()) + .await + .unwrap(); + assert!(add_plugin_directive(&project, false) + .await + .iter() + .all(|r| r.status != GemSetupStatus::Error)); + let removed = remove_plugin_directive(&project, false).await; + assert!( + removed.iter().all(|r| r.kind != "gem_plugin_registration"), + "no machine-local registration -> no registration entry: {removed:?}" + ); + } + #[tokio::test] async fn test_full_roundtrip_via_project() { let dir = tempfile::tempdir().unwrap(); From 0a0295ef14c8352268071f5f16ca6ffa647ad03d Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Tue, 18 Aug 2026 21:42:22 -0400 Subject: [PATCH 3/6] =?UTF-8?q?test(gem):=20red=20=E2=80=94=20'..'=20trave?= =?UTF-8?q?rsal=20in=20a=20recorded=20plugin=20install=20dir=20escapes=20t?= =?UTF-8?q?he=20delete=20gate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding (P2): Path::starts_with is a purely lexical component compare, so a crafted committed .bundle/plugin/index recording plugin_paths /.bundle/plugin/../../victim passes the containment gate and remove_dir_all deletes outside the plugin root. Co-Authored-By: Claude Fable 5 --- crates/socket-patch-core/src/setup/gem/mod.rs | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/crates/socket-patch-core/src/setup/gem/mod.rs b/crates/socket-patch-core/src/setup/gem/mod.rs index 9f64d995..75f881b8 100644 --- a/crates/socket-patch-core/src/setup/gem/mod.rs +++ b/crates/socket-patch-core/src/setup/gem/mod.rs @@ -1532,6 +1532,42 @@ mod tests { ); } + #[tokio::test] + async fn test_remove_registration_rejects_traversal_in_recorded_dir() { + // A committed `.bundle/plugin/index` is attacker-authored input: it + // can record ANY path as the plugin's install dir. `..` components + // let `/../../victim` pass a purely lexical + // `starts_with(plugin_root)` check while pointing outside the plugin + // root — such a dir must never be deleted. Bundler itself never + // records traversal paths, so rejecting them loses nothing. + let dir = tempfile::tempdir().unwrap(); + let root = dir.path(); + let victim = root.join("victim"); + fs::create_dir_all(&victim).await.unwrap(); + write(&victim.join("precious.txt"), "do not delete\n").await; + // Lexically under the plugin root, physically the victim dir. + let evil = root.join(".bundle/plugin/../../victim"); + assert!( + evil.starts_with(root.join(".bundle/plugin")), + "precondition: the traversal path passes the lexical prefix check" + ); + write( + &root.join(".bundle/plugin/index"), + &format!( + "---\ncommands:\nhooks:\nload_paths:\n socket-patch:\n - \"{0}/.\"\n\ + plugin_paths:\n socket-patch: \"{0}\"\nsources:\n", + evil.display() + ), + ) + .await; + let r = remove_plugin_registration_at(root, None, false).await; + assert!(matches!(r, GemRegistrationCleanup::Cleaned { .. }), "{r:?}"); + assert!( + victim.join("precious.txt").is_file(), + "a recorded dir with `..` traversal must never be deleted" + ); + } + #[tokio::test] async fn test_add_appends_gitignore_line_to_unterminated_file() { // A .gitignore whose last line has no trailing newline must not have From 11d007c9c8da89c1a294ea4cfb0cb63986e6c356 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Tue, 18 Aug 2026 21:42:56 -0400 Subject: [PATCH 4/6] fix(gem): reject '..' components in bundler's recorded install dir before deleting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Path::starts_with is lexical, so the containment gate alone lets a crafted index's /../../victim path through to remove_dir_all. Refuse any recorded dir containing a ParentDir component — bundler never writes traversal paths, so only hostile input is rejected. Co-Authored-By: Claude Fable 5 --- crates/socket-patch-core/src/setup/gem/mod.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/crates/socket-patch-core/src/setup/gem/mod.rs b/crates/socket-patch-core/src/setup/gem/mod.rs index 75f881b8..526bf41d 100644 --- a/crates/socket-patch-core/src/setup/gem/mod.rs +++ b/crates/socket-patch-core/src/setup/gem/mod.rs @@ -498,9 +498,17 @@ async fn remove_plugin_registration_at( // `path:`-sourced wiring that is the project's `.socket/bundler-plugin` // (already deleted by `remove_plugin_files`); a `bundler plugin install`ed // copy lives under the plugin root itself. Delete it only inside that - // root — never a recorded path elsewhere on the machine. + // root — never a recorded path elsewhere on the machine. The recorded + // path is attacker-authored input (the index can be committed), and + // `starts_with` compares components lexically, so a `..` traversal like + // `/../../victim` would pass the prefix check while pointing + // anywhere on the machine: reject any `..` component outright (bundler + // never records traversal paths, so nothing legitimate is lost). if let Some(dir) = &stripped.installed_dir { - if dir.starts_with(&plugin_root) && dir != &plugin_root { + let traversal_free = dir + .components() + .all(|c| !matches!(c, std::path::Component::ParentDir)); + if traversal_free && dir.starts_with(&plugin_root) && dir != &plugin_root { let _ = fs::remove_dir_all(dir).await; } } From 75084dbd7b9250cf6651795442a696c0f64f62cb Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Tue, 18 Aug 2026 21:45:02 -0400 Subject: [PATCH 5/6] fix(gem): hermetic BUNDLE_APP_CONFIG for remove_plugin_directive's unit-test path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding (P2): remove_plugin_directive read the ambient BUNDLE_APP_CONFIG, so the -core unit tests exercising it failed spuriously on machines exporting it (official ruby images export /usr/local/bundle) — and could have pointed the cleanup at a real machine-local index outside the tempdir. Thread the env through an explicit remove_plugin_directive_at (mirroring remove_plugin_registration_at); the public entry still reads the process env exactly like bundler. Verified red: with BUNDLE_APP_CONFIG=/usr/local/bundle, test_remove_clears_bundler_plugin_registration_entry failed before this change and passes after (69/69 with and without the export). Co-Authored-By: Claude Fable 5 --- .../socket-patch-core/src/setup/gem/update.rs | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/crates/socket-patch-core/src/setup/gem/update.rs b/crates/socket-patch-core/src/setup/gem/update.rs index e58d4f51..82ba1903 100644 --- a/crates/socket-patch-core/src/setup/gem/update.rs +++ b/crates/socket-patch-core/src/setup/gem/update.rs @@ -11,7 +11,7 @@ use std::path::Path; use tokio::fs; use super::{ - add_plugin_files, remove_plugin_files, remove_plugin_registration, BundlerProject, + add_plugin_files, remove_plugin_files, remove_plugin_registration_at, BundlerProject, GemRegistrationCleanup, }; use crate::utils::fs::atomic_write_bytes_preserving_mode; @@ -255,13 +255,27 @@ pub async fn add_plugin_directive(project: &BundlerProject, dry_run: bool) -> Ve pub async fn remove_plugin_directive( project: &BundlerProject, dry_run: bool, +) -> Vec { + let env = std::env::var_os("BUNDLE_APP_CONFIG"); + remove_plugin_directive_at(project, env.as_deref(), dry_run).await +} + +/// [`remove_plugin_directive`] with the `BUNDLE_APP_CONFIG` resolution input +/// made explicit (tests inject it so a machine's exported value — e.g. the +/// official ruby images' `/usr/local/bundle` — can neither fail them +/// spuriously nor point the cleanup at a real machine-local index; the public +/// entry reads the process env, exactly like bundler itself). +async fn remove_plugin_directive_at( + project: &BundlerProject, + app_config_env: Option<&std::ffi::OsStr>, + dry_run: bool, ) -> Vec { let gemfile = edit_gemfile_remove(&project.gemfile, dry_run).await; if gemfile.status == GemSetupStatus::Error { return vec![gemfile]; } let mut results = vec![gemfile, remove_plugin_files(&project.root, dry_run).await]; - match remove_plugin_registration(&project.root, dry_run).await { + match remove_plugin_registration_at(&project.root, app_config_env, dry_run).await { GemRegistrationCleanup::Cleaned { index } => results.push(GemEditResult { kind: "gem_plugin_registration", path: index.display().to_string(), @@ -728,7 +742,7 @@ mod tests { .await .unwrap(); - let results = remove_plugin_directive(&project, false).await; + let results = remove_plugin_directive_at(&project, None, false).await; // Restore before any assertion can unwind, so the tempdir cleans up. fs::set_permissions(root, std::fs::Permissions::from_mode(0o755)) @@ -778,7 +792,7 @@ mod tests { .await .unwrap(); - let removed = remove_plugin_directive(&project, false).await; + let removed = remove_plugin_directive_at(&project, None, false).await; assert!( removed .iter() @@ -798,7 +812,7 @@ mod tests { .await .iter() .all(|r| r.status != GemSetupStatus::Error)); - let removed = remove_plugin_directive(&project, false).await; + let removed = remove_plugin_directive_at(&project, None, false).await; assert!( removed.iter().all(|r| r.kind != "gem_plugin_registration"), "no machine-local registration -> no registration entry: {removed:?}" @@ -825,7 +839,7 @@ mod tests { .iter() .all(|r| r.status == GemSetupStatus::AlreadyConfigured)); - let removed = remove_plugin_directive(&project, false).await; + let removed = remove_plugin_directive_at(&project, None, false).await; assert!(removed.iter().all(|r| r.status == GemSetupStatus::Updated)); assert_eq!( fs::read_to_string(root.join("Gemfile")).await.unwrap(), From 499e498e8373c8747c9a1c93ac508d63141bed74 Mon Sep 17 00:00:00 2001 From: Mikola Lysenko Date: Tue, 18 Aug 2026 21:45:35 -0400 Subject: [PATCH 6/6] style(gem): rustfmt the new gem setup code Review nit: 5 rustfmt divergences introduced by this branch (the unquote condition width plus 4 test-code spots). cargo fmt --check on the workspace is back to main's pre-existing count; no touched-file divergence remains. Co-Authored-By: Claude Fable 5 --- crates/socket-patch-core/src/setup/gem/mod.rs | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/crates/socket-patch-core/src/setup/gem/mod.rs b/crates/socket-patch-core/src/setup/gem/mod.rs index 526bf41d..3b1f6b74 100644 --- a/crates/socket-patch-core/src/setup/gem/mod.rs +++ b/crates/socket-patch-core/src/setup/gem/mod.rs @@ -277,7 +277,9 @@ struct IndexSection { fn unquote(s: &str) -> &str { let s = s.trim(); let b = s.as_bytes(); - if s.len() >= 2 && ((b[0] == b'"' && b[s.len() - 1] == b'"') || (b[0] == b'\'' && b[s.len() - 1] == b'\'')) { + if s.len() >= 2 + && ((b[0] == b'"' && b[s.len() - 1] == b'"') || (b[0] == b'\'' && b[s.len() - 1] == b'\'')) + { &s[1..s.len() - 1] } else { s @@ -1331,9 +1333,7 @@ mod tests { let index = "---\ncommands:\nhooks:\n after-install:\n - \"other\"\nload_paths:\n \ other:\n - \"/mono/socket-patch-fork/other/.\"\nplugin_paths:\n \ other: \"/mono/socket-patch-fork/other\"\nsources:\n"; - assert!(strip_plugin_registration(index) - .expect("parses") - .is_none()); + assert!(strip_plugin_registration(index).expect("parses").is_none()); } #[test] @@ -1351,7 +1351,10 @@ mod tests { use std::ffi::OsStr; let root = Path::new("/proj"); // Unset / empty → /.bundle. - assert_eq!(bundler_app_config_dir(root, None), Path::new("/proj/.bundle")); + assert_eq!( + bundler_app_config_dir(root, None), + Path::new("/proj/.bundle") + ); assert_eq!( bundler_app_config_dir(root, Some(OsStr::new(""))), Path::new("/proj/.bundle") @@ -1399,7 +1402,11 @@ mod tests { // the emptied plugin dir) go, the user's config survives. let dir = tempfile::tempdir().unwrap(); let root = dir.path(); - write(&root.join(".bundle/config"), "---\nBUNDLE_PATH: \"vendor/bundle\"\n").await; + write( + &root.join(".bundle/config"), + "---\nBUNDLE_PATH: \"vendor/bundle\"\n", + ) + .await; let index = root.join(".bundle/plugin/index"); write(&index, &solo_index(&root.display().to_string())).await; @@ -1461,12 +1468,9 @@ mod tests { let decoy = root.join(".bundle/plugin/index"); write(&decoy, &solo_index("/elsewhere")).await; - let r = remove_plugin_registration_at( - root, - Some(std::ffi::OsStr::new("bundle-config")), - false, - ) - .await; + let r = + remove_plugin_registration_at(root, Some(std::ffi::OsStr::new("bundle-config")), false) + .await; assert!(matches!(r, GemRegistrationCleanup::Cleaned { .. }), "{r:?}"); assert!(!index.exists(), "the app-config index is the one cleared"); assert!(