Skip to content

Fix Bundler settings resolution order (#483, #507) - #532

Merged
Mikola Lysenko (mikolalysenko) merged 4 commits into
mainfrom
agent/fix-bundler-settings-resolution
Oct 2, 2026
Merged

Mikola Lysenko (mikolalysenko) merged 4 commits into
mainfrom
agent/fix-bundler-settings-resolution

Conversation

@mikolalysenko

@mikolalysenko Mikola Lysenko (mikolalysenko) commented Oct 2, 2026 •

Copy link
Copy Markdown
Collaborator

LLM Description written by Claude Code:claude-opus-5-5

Fixes #483
Fixes #507

Summary

Gem modes now read Bundler settings in Bundler's own priority, with the local app config over the environment.

Root cause

socket-patch read Bundler settings one key at a time, with no resolver that follows Bundler::Settings priority (local app config $BUNDLE_APP_CONFIG/config / .bundle/config first, then ENV):

  • gemfile: formats/gem/manifest.rs::classify checked the environment before the app config, which is the reverse of Bundler's order. A unit test even pinned the inverted order.
  • cache_path: never read. scan/hosted.rs hard-coded <cwd>/vendor/cache in both guard flavors.

Changes

  • crawlers/ruby_crawler.rs: added bundle_config_setting(contents, key), one exact-key app-config reader that config_gemfile now uses. Added bundler_app_cache_dir[_with_env], which resolves cache_path in Bundler's priority (relative to the project root, absolute stands alone, read-only use so no containment needed). Both resolvers load the file through read_app_config, which returns nothing under BUNDLE_IGNORE_CONFIG (Bundler's load_config returns {} for any set value).
  • formats/gem/manifest.rs::classify: the app config value now outranks the env. The one exception: an env BUNDLE_GEMFILE naming a file in another directory still decides alone. That value moves Bundler.root, so Bundler reads that root's config and never the project's. The run still refuses, now naming the env var. The inverted unit test is replaced.
  • scan/hosted.rs: both stale-guard flavors use the resolved cache dir, and the warning text now says "its cache dir".
  • Docs: CLI_CONTRACT.md (the "Gem stale-install guard" section, the redirect_gem_bundle_gemfile_unsupported and redirect_gem_stale_install rows), docs/ecosystems.md, and CHANGELOG [Unreleased] / Fixed.
  • No wrapper changes are needed (npm/, pypi/, gem/ only dispatch to the binary).

Per-issue checklist

Test evidence (Linux, Ruby 3.3.6, Bundler 4.0.17)

  • cargo clippy --workspace --all-features -- -D warnings: clean.
  • cargo test -p socket-patch-core --all-features --lib: 4720 passed, 4 failed. The 4 failures (copy_tree::relax_loop_must_not_traverse_symlinked_root, vlt_heal::an_unremovable_hidden_lock_keeps_every_store_entry, pypi_poetry::wire_write_failure_…, pypi_requirements::wire_failure_rolls_back_…) fail identically on origin/main. They are permission tests that root bypasses in this sandbox, and they are unrelated to this change.
  • cargo test -p socket-patch-cli --all-features --lib: 830 passed.
  • --test e2e_redirect_gem_stale_install (25), in_process_gem_apply (11), in_process_gem_multi_platform (7), covgap_commands_scan_hosted (50), hosted_memory_parity (31), core crawler_ruby_e2e (25): all pass.
  • --test e2e_redirect_gem_build --include-ignored (19/19) and --test e2e_vendor_gem_build --include-ignored (15/15), with real Bundler: all pass on a757732.
  • cargo fmt --all -- --check: main itself is not rustfmt-clean (460 diffs with the pinned 1.93.1 rustfmt, and CI doesn't gate it), so I formatted only this PR's own hunks.
  • I did not run a full local cargo test --workspace --all-features: building every integration-test binary used up the sandbox's disk allowance. CI covers it.
  • CI on 477aae9: all green, 477 passed, 6 skipped, 0 failed. Bugbot: "no issues found", twice. CI on a757732 is pending.

Follow-ups

  • Bundler's global ~/.bundle/config (priority below ENV) is still not consulted for any key. That was already true and is unchanged here.
  • The BUNDLE_PATH app-config probe (RubyCrawler::app_config_bundle_path) doesn't honor BUNDLE_IGNORE_CONFIG yet. It only adds a discovery path, it predates this PR, and this PR doesn't touch it.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MWt5CXPnmqVEUZe4wnCfgX


Note

Medium Risk
Changes which Gemfile gets wired and where stale-cache warnings fire—incorrect resolution could skip patches or attest unpatched gems, but behavior now matches Bundler and is heavily tested.

Overview
Aligns Ruby gem hosted and vendored flows with Bundler’s settings order so socket-patch does not edit or attest manifests Bundler never loads.

BUNDLE_GEMFILE (#507): .bundle/config now wins over the environment (matching Bundler::Settings). Dual-boot setups with gemfile Gemfile.next locally plus BUNDLE_GEMFILE=Gemfile in the shell no longer get the ignored Gemfile redirected or VEX-attested; they hit redirect_gem_bundle_gemfile_unsupported instead. An env gemfile outside the project root still wins alone (Bundler’s root moves). BUNDLE_IGNORE_CONFIG skips reading .bundle/config for gemfile resolution.

Stale-install guard (#483): Hosted redirect_gem_stale_install probes the committed .gem in Bundler’s cache_path (app config BUNDLE_CACHE_PATH: → env BUNDLE_CACHE_PATH → default vendor/cache), not a hard-coded vendor/cache. Stale archives at the configured path warn and exclude the purl from same-run --vex; leftover archives under the default path when cache is relocated are ignored.

Shared helpers: bundle_config_setting, bundler_app_cache_dir, and updated manifest::classify. Docs (CLI_CONTRACT.md, ecosystems.md, CHANGELOG) and unit/e2e tests cover config-over-env and custom cache paths.

Reviewed by Cursor Bugbot for commit a757732. Configure here.

Assisted-by: Claude Code:claude-opus-5-5
When BUNDLE_GEMFILE was set both in .bundle/config and in the
environment, socket-patch followed the environment. Bundler does the
reverse: local app config outranks ENV. A dual-boot project with
`gemfile Gemfile.next` committed and BUNDLE_GEMFILE=Gemfile exported
got its Gemfile rewired and attested while bundler installed
Gemfile.next unpatched.

The app config value now wins, unless the environment names a
manifest in another directory (that moves bundler's root, so the
project's config is never read). Add one app-config reader shared by
every Bundler key, and a resolver for bundler's cache dir
(cache_path / BUNDLE_CACHE_PATH, default vendor/cache) in the same
priority.

Refs #507, #483

Assisted-by: Claude Code:claude-opus-5-5
The hosted gem stale-install guard only looked for committed archives
in vendor/cache. With `bundle config set --local cache_path
vendor/gems` (or BUNDLE_CACHE_PATH), a committed unpatched archive
there gave no warning, the same run's VEX attested the gem, and
bundle install then installed the unpatched bytes.

Both guard flavors now use bundler's configured cache dir, so the
stale archive warns, joins the delete-list remedy, and keeps the purl
out of the in-run attestation.

Fixes #483
Fixes #507

Assisted-by: Claude Code:claude-opus-5-5
@mikolalysenko
Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 2, 2026 06:08
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale Bugbot comment from a previous run.

@mikolalysenko Mikola Lysenko (mikolalysenko) added the Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review label Oct 2, 2026
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Ready for review at head 477aae9b35e696e0822a04da1cb2de846b453824.

  • CI: all required checks green on this head (0 failing; remainder skipped/neutral).
  • Bugbot: reviewed 477aae9 — no issues found; no unresolved review threads.
  • Mergeable against main; awaiting human approval.
  • Reviewer focus: Bundler settings priority (local app config over BUNDLE_* env) for BUNDLE_GEMFILE / BUNDLE_CACHE_PATH resolution.

Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Reviewed 477aae9b35e696e0822a04da1cb2de846b453824. Recommendation: changes needed.

P1 — Honor BUNDLE_IGNORE_CONFIG when resolving the cache directory (ruby_crawler.rs:1021). This always reads .bundle/config, but Bundler ignores config files when BUNDLE_IGNORE_CONFIG is set. With an ignored BUNDLE_CACHE_PATH: "vendor/custom", Bundler uses vendor/cache while the new stale guard searches vendor/custom. The previously checked default directory is now skipped.

Reproduced against Bundler 4.0.15 and the actual CLI: set BUNDLE_IGNORE_CONFIG=1, leave the custom cache path in .bundle/config, and place the upstream archive under vendor/cache. scan --mode hosted --vex returns exit 0, emits no redirect_gem_stale_install, and writes a not_affected VEX statement. Skip file settings when config is ignored and fall through to the environment/default cache. Bundler's configuration documentation describes this switch.

Validation: 10 Bundler-related core unit tests, 9 manifest-selection tests, and all 25 e2e_redirect_gem_stale_install tests passed. An additional review-only CLI regression failed, reproducing the missed archive and attestation above. Full workspace and Bundler install matrices were not rerun.

Bundler's load_config returns {} whenever BUNDLE_IGNORE_CONFIG is set,
so a cache_path or gemfile setting in .bundle/config is then ignored.
The stale-install guard still followed the ignored cache_path, skipped
the vendor/cache archive bundler actually installs from, and attested
the purl. Both settings now read the app config through one reader
that honors the switch.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MWt5CXPnmqVEUZe4wnCfgX
@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Confirmed the BUNDLE_IGNORE_CONFIG finding. Bundler 4.0.17's Settings#load_config returns {} whenever ENV["BUNDLE_IGNORE_CONFIG"] is set, to any value. Fixed in a757732, which I'm pushing next.

  • One shared app-config reader (read_app_config) now returns nothing under BUNDLE_IGNORE_CONFIG. Both settings this PR resolves use it: cache_path and gemfile. The ignored setting falls through to the env value, then to the default vendor/cache (or the default Gemfile discovery).
  • New ignore-config arm in e2e_redirect_gem_stale_install::gem_hosted_stale_archive_at_configured_cache_path_warns_and_is_not_attested: committed BUNDLE_CACHE_PATH: "vendor/gems", BUNDLE_IGNORE_CONFIG=1, and the upstream archive under vendor/cache. On 477aae9 it reproduces your result (status: success, no redirect_gem_stale_install, vex.statements: 1). With the fix it warns and doesn't attest. Unit assertions cover both resolvers.
  • Checked locally: the ruby_crawler/manifest unit tests, e2e_redirect_gem_stale_install (25/25), and the real-Bundler e2e_redirect_gem_build (19/19) and e2e_vendor_gem_build (15/15) suites pass, with --include-ignored. cargo clippy --workspace --all-features -D warnings is clean.

Left as is: the pre-existing BUNDLE_PATH app-config probe in RubyCrawler::app_config_bundle_path doesn't honor the switch either. It only adds an extra discovery path and isn't part of this PR's change, so it's a follow-up.


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

Follow-up review of a757732502ecd579f4c70419c54064c063e037c0: ready from code review. The previously reported BUNDLE_IGNORE_CONFIG issue is fixed; the shared config reader now skips the ignored file for both manifest and cache settings.

I reran the original independent CLI reproduction at this exact head: 1 passed. With a local config pointing at vendor/custom and BUNDLE_IGNORE_CONFIG=1, scan now detects the stale archive in vendor/cache, emits redirect_gem_stale_install, and withholds VEX. No remaining blocking finding in the updated code. This supersedes my earlier changes-needed recommendation.

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

CI on a757732: native (macos-latest, 1.1.15) failed. This is the Poetry native backtest (scripts/backtest-poetry.py). Its only failing case is 1.1.15 direct agent, check appliedExactlyOne. The other six cases in that job pass, and so do the other native Poetry versions on macOS.

I don't think this failure comes from this PR:

  • a757732 changes only crawlers/ruby_crawler.rs (the BUNDLE_IGNORE_CONFIG reader), one gem e2e test and docs. When BUNDLE_IGNORE_CONFIG is unset, which it is in that job, the Ruby code behaves exactly as it did on 477aae9.
  • The same job passed on 477aae9 (run 36970601903) and passes on current main (run 37020150627).
  • This case runs a real poetry install from PyPI before the scan. That makes an outside service the likeliest cause.

I couldn't read the failing envelope (native-poetry/captures/.../cli-output.json), because this sandbox can't download the run's artifact. No fix exists to port. I'm re-running the failed job once to confirm. If it fails again on this commit, I'll treat it as real and dig into the captured envelope.


Generated by Claude Code

@mikolalysenko

Copy link
Copy Markdown
Collaborator Author

BugBot review


Generated by Claude Code

@mikolalysenko
Mikola Lysenko (mikolalysenko) merged commit cbf1f74 into main Oct 2, 2026
513 of 514 checks passed
@mikolalysenko
Mikola Lysenko (mikolalysenko) deleted the agent/fix-bundler-settings-resolution branch October 2, 2026 15:32

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit a757732. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready for review Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review

Projects

None yet

3 participants