Fix gem source-option guard missing git sources (#652) by mikolalysenko · Pull Request #731 · SocketDev/socket-patch

Mikola Lysenko (mikolalysenko) · 2026-10-03T23:29:15Z

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

Fixes #652

Summary

In hosted mode, a gem pulled from a git source that the old token list didn't recognize got "redirected" into a Socket source block that its own option still overrides. Bundler kept loading the unpatched git checkout, the scan reported success, and VEX attested not_affected. Hosted and vendored modes now refuse every source-selecting option, in every Ruby key spelling. Hosted mode also refuses a gem that the lock resolves from a non-registry section.

Root cause

Both modes decided whether a gem line already picks its own source by substring-matching a fixed token list (path:, git:, github:, gist:, bitbucket:, source:). The list was duplicated in patch/redirect/mod.rs and vendor/gem.rs, and it missed:

Bundler's built-in gitlab: source.
Any custom git_source(:name) { … } key.
String-keyed spellings ("git" => …, "git": …). gem_line_trailing_options also read these as a version constraint, so string-keyed options such as "require" => false were silently dropped.

Fix

New formats::gem::gemfile module: the one reader of a gem line's argument tail. It splits the tail at top-level commas, outside strings (including escapes) and brackets, and reads each option key in all four spellings (key:, "key":, :key =>, "key" =>).
- source_option treats any key outside Bundler's non-source keys (group, require, platforms, install_if, branch/ref/tag, …) as source-selecting. Bundler rejects unknown keys that aren't git sources, so this covers gitlab: and every custom git_source without parsing the git_source declarations. Arguments it can't read (**opts, a hash literal, a method call) fail closed.
- trailing_options replaces gem_line_trailing_options and keeps string-keyed options.
Hosted (redirect_gem_source_option) and vendored (rest_blocks_edit) modes both use the shared reader, so they can't drift again.
Hosted mode also refuses when Gemfile.lock lists the gem under a GIT / PATH / PLUGIN SOURCE section (new GemfileLock::non_registry_section_of). That is what Bundler actually resolves from, and it catches declarations the line reader can't see, e.g. a plain gem line inside a git "…" do block. Socket's own vendored PATH wiring still gets the existing "un-vendor first" remedy.

Test evidence

Red before the fix, green after:

patch::redirect::tests::gemfile_git_source_options_in_every_spelling_fail_closed: gitlab:, a custom local: git source, :internal =>, "git" =>, 'path' =>, "git":. Failed on main, passes now.
patch::redirect::tests::gemfile_string_keyed_options_survive_the_redirect: "require" => false is kept inside the block. Failed on main.
patch::redirect::tests::gem_locked_from_git_or_path_section_fails_closed: a gem in a git … do / path … do block that the lock resolves from GIT / PATH. Failed on main.
vendor::gem::tests::plan_gemfile_edit_refusal_grammar (extended): vendored mode refuses gitlab:, a custom git source and "git" =>. Failed on main.
formats::gem::gemfile::tests::*: unit tests for the reader (spellings, comments, #{} interpolation, escaped quotes, fail-closed cases, option preservation).
e2e e2e_redirect_gem_build::gem_hosted_custom_git_source_is_refused_and_still_installs: real gem build + bundle install (Ruby 3.3.6, Bundler 4.0.17) of a gem from a local git repo through git_source(:local). The scan must name redirect_gem_source_option, leave Gemfile/lock byte-identical and attest nothing, and bundle install must still succeed. With the old token list put back, it fails ("status":"success", nothing refused). With the fix, it passes.

Commands run locally:

cargo clippy --workspace --all-features -- -D warnings: clean.
cargo test -p socket-patch-core --all-features --lib -- gem: 271 passed.
cargo test -p socket-patch-cli --all-features --test e2e_redirect_gem_build --test e2e_vendor_gem_build -- --ignored (Bundler 4.0.17): 12 + 6 passed.
cargo test --workspace --all-features --no-fail-fast: all gem tests pass. The only failures are write-failure / read-only-permission tests (*_state_write_failure_*, *_unremovable_*, wire_*_failure_*, relax_loop_must_not_traverse_symlinked_root, …). They rely on chmod making files read-only and can't fail as designed when the suite runs as root, as it does in this sandbox. None of them touch gem code.
cargo fmt: the new module is rustfmt-clean, and the hunks in existing files follow their surrounding format. main itself is not cargo fmt-clean, so I didn't run a repo-wide reformat.

Performance

The first CI run flagged scan performance: bundler/hosted +15.1% and bundler/rescan +15.6%, because the lock check re-parsed Gemfile.lock once per patched gem. Commit 72db141 reads the non-registry sections once per rewrite. Our own edits only touch GEM sections and CHECKSUMS, so membership read up front stays accurate. A local socket-patch-bench compare --filter bundler (perf profile, base = main 045d7ec) now shows bundler/hosted -2.2% [-6.5, +2.1] and bundler/rescan -2.7% [-7.4, +6.7]: unchanged.

Checklist

Hosted gem redirect treats a gitlab: or custom git_source gem as patched, so Bundler keeps loading the unpatched git checkout while VEX attests not_affected #652 custom git_source arm: unit test + e2e capstone
Hosted gem redirect treats a gitlab: or custom git_source gem as patched, so Bundler keeps loading the unpatched git checkout while VEX attests not_affected #652 gitlab: arm: unit tests (hosted + vendored)
Hosted gem redirect treats a gitlab: or custom git_source gem as patched, so Bundler keeps loading the unpatched git checkout while VEX attests not_affected #652 string-keyed "git" => arm (refused) and "require" => false (preserved): unit tests
Vendored token list (vendor/gem.rs), also named in the report: same shared reader, vendored test extended

Notes

Merged with Fix vendored gem rewrite breaking positional args (#847) #849 (Vendored gem rewrite puts non-string arguments after path:, so gem "x", *V, gem "x", ENV.fetch(…) or gem "x", VERSION becomes a Gemfile syntax error and every bundle command fails #847), commit 7db1412: the shared reader now classifies positional arguments (*V, constants, ENV.fetch(…), Rack::VERSION) as version constraints, which never select a source. **opts splats and hash literals are flagged dynamic: hosted mode refuses them because a hidden git: would make the redirect a silent, attested no-op, while vendored mode keeps them after path: as Vendored gem rewrite puts non-string arguments after path:, so gem "x", *V, gem "x", ENV.fetch(…) or gem "x", VERSION becomes a Gemfile syntax error and every bundle command fails #847 intends. New unit test: positional_constraints_are_not_sources.
Out of scope: Hosted gem stale-install warning calls the project's own vendor/bundle a "shared gem home" when --cwd is left at its default (or relative), so it gives the wrong remedy and drops the committed cache archive from the delete list #729 (relative --cwd stale-install wording) touches neighboring hosted gem code but has a separate cause.

🤖 Generated with Claude Code

Note

High Risk
Changes hosted gem redirect and VEX refusal logic; incorrect handling could silently attest patches while bundler still loads unpatched git or path sources.

Overview
Fixes #652 by replacing duplicated Gemfile substring checks with a shared formats::gem::gemfile parser for gem "name", … tails. Hosted redirect and vendored rewrite now fail closed on any source-selecting option—including gitlab:, custom git_source keys, and "git" => / "git": spellings—and trailing_options keeps string-keyed options like "require" => false instead of dropping them.

Hosted mode also skips redirect when Gemfile.lock resolves the gem from a GIT / PATH / PLUGIN SOURCE section (via non_registry_sources), even when the Gemfile line looks registry-safe. New unit tests and an e2e capstone (ScanVexCustomGitSource) pin the refuse-and-attest-nothing contract.

Separately, the PDM compatibility backtest retries cases up to three times on transport-only failures (PyPI/API 5xx/429), records evidence under attempts/, logs failed-check details, and uploads those artifacts in CI.

^{Reviewed by Cursor Bugbot for commit 7db1412. Configure here.}

Generated by Claude Code

Assisted-by: Claude Code:claude-opus-5-5

Hosted scans moved a gem declared with `gitlab:`, a custom `git_source(:name)` key or a string-keyed `"git" =>` option into the Socket source block. The option still overrode the block, so bundler kept loading the unpatched git checkout while scan reported the gem redirected and VEX attested it not_affected. String-keyed options such as `"require" => false` were also silently dropped. Both hosted and vendored modes now read gem options through one shared reader that understands every key spelling and treats any key outside bundler's non-source options as a source. Hosted mode also refuses a gem the lock resolves from a GIT, PATH or plugin section. Fixes #652 Assisted-by: Claude Code:claude-opus-5-5

Adds a real-bundler capstone where the gem comes from a custom `git_source` key: the hosted scan must refuse it, write nothing and attest nothing, and bundler must still install the project. The option reader now also honors backslash escapes in single-quoted strings, so a quote inside a value cannot hide a later git option. Refs #652 Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) · 2026-10-04T00:00:14Z

BugBot review

Generated by Claude Code

cursor

Stale Bugbot comment from a previous run.

The new git/path refusal re-parsed Gemfile.lock for every patched gem, which made hosted bundler scans about 15% slower on the bench fixture (800 gems, 20 patched). The lock is now parsed once per rewrite; our own edits only touch GEM sections and CHECKSUMS, so the GIT/PATH membership read up front stays accurate. Refs #652 Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) · 2026-10-04T00:08:54Z

BugBot review

Generated by Claude Code

cursor

Stale Bugbot comment from a previous run.

Mikola Lysenko (mikolalysenko) · 2026-10-04T01:14:53Z

[burn-down agent] Ready for review at head 72db14167c.

CI: 485/485 completed checks green (3 skipped), no failures or pending runs.
Mergeable: no conflicts (blocked only on required review).
Bugbot reviewed 72db141: no new issues; no unresolved review threads.
Reviewer focus: the shared formats::gem::gemfile option reader treats any unknown key as source-selecting (fail-closed), and the new lock-section refusal for GIT/PATH/PLUGIN SOURCE gems.

Generated by Claude Code

Conflicts: kept main's gem_line_tail_blocks_edit (#340) alongside the PR's shared gemfile::source_option reader (dropped main's duplicated token lists), ran the #340 structural check before the fail-closed source-option reader in redirect, and kept both sides' e2e drivers/tests. Co-Authored-By: Claude <noreply@anthropic.com>

Mikola Lysenko (mikolalysenko) · 2026-10-05T12:16:04Z

bugbot run

Generated by Claude Code

cursor

Stale Bugbot comment from a previous run.

…option-guard

#605 taught the name-keyed npm resolver to probe bundled store trees, so it now finds aliased copies (node_modules/lp) and a nested host's store peers itself. Two vex_consumed tests from #738 assumed that set never held aliases, so main's CI went red after both merged. The tests now feed the alias-free set explicitly to keep covering alias expansion, and also check the resolver's own set reaches the same copies with no duplicates. No production code changes. Assisted-by: Claude Code:claude-opus-5-5 (cherry picked from commit 40dac07)

Mikola Lysenko (mikolalysenko) · 2026-10-05T12:45:20Z

[agent] CI was red on 99a6c95 (coverage and test-release). Both fail on the same two tests in socket-patch-cli --lib: commands::vex_consumed::tests::hosted_expands_alias_only_copies and hosted_reuses_expanded_npm_copies_and_merges_alias_variants.

The cause is outside this PR. Both tests also fail on main at 4646693, right after #605 (npm store copies) merged; I reproduced that locally on a plain origin/main checkout. CI runs the PR merged with the latest main, which is how they reached this PR.

What I pushed in c7d18c9:

Merged the latest main into this branch.
Cherry-picked the test-only fix from Fix vex alias tests broken by store-copy merge #851 (agent/ci-vex-alias-store-tests, 40dac07). It adds assertions and loosens none, and it will be a no-op once Fix vex alias tests broken by store-copy merge #851 lands on main.

Checked locally on c7d18c9:

cargo test -p socket-patch-cli --lib: 840/840 pass.
gem unit tests: 284 pass.
clippy -D warnings: clean.
e2e_redirect_gem_build and e2e_vendor_gem_build with --ignored: 16 + 7 pass.

Generated by Claude Code

Mikola Lysenko (mikolalysenko) · 2026-10-05T12:45:21Z

BugBot review

Generated by Claude Code

cursor

Stale Bugbot comment from a previous run.

The PDM matrix runs against production PyPI and the public patch API. Over the last 7 days 25 pdm-compatibility runs failed on one random cell each, on unrelated PRs. The version, OS, shape, mode and check differed every time (rescanIdempotent, appliedExactlyOne, rescanAfterRelockApplies, ...). Each check judges a CLI scan, install or rollback. `Run` retries a command once, and only on a non-zero exit. The CLI usually reports an exhausted patch API fetch in its JSON while exiting zero, so the cell just fails a later check. Port backtest-poetry.py's case-level retry (#596). A case is re-run from a fresh directory, at most three attempts, only when every failed check recorded transport evidence from the operation it judged. Evidence is a failed command's request error, PyPI give-up, patch API 5xx or exhausted 429, or the same in the CLI's JSON error records. Functional failures are never retried, even when a later step raises a transport error. Failed attempts' logs go under attempts/ and are uploaded. A failing case now prints its failed checks' notes, since the job log alone never said why. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N8YeCUhdg2tKdqyyY7z3sV (cherry picked from commit 4329170)

Bugbot: the final hosted/vendored rollback checks, the unverifiable- write rollback, the refused-lock VEX and the reverted-lock VEX runs named no operation, so a transport failure there never made the case retryable. installedBytesPatched fails together with a blipped pdm sync and blocked the retry the same way. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N8YeCUhdg2tKdqyyY7z3sV (cherry picked from commit 6b0302a)

Mikola Lysenko (mikolalysenko) · 2026-10-05T13:34:38Z

[agent] One more red check, also not from this PR: PDM patch compatibility / native (macos-latest, 0.12.3) on c7d18c9.

What failed: a single cell, pep582 hosted (rescanIdempotent), out of a 33-cell matrix that is otherwise green. The run before it failed a different single cell, 2.29.2 extras hosted (rescanIdempotent) on macOS.
Why it isn't this PR's: this PR changes no PDM code. Fix PDM matrix flakes on PyPI/patch API transport blips #860 (ci-janitor/pdm-transport-retry) records this exact pattern across 29 failed runs on unrelated PRs, and lists this PR's earlier run 37308344193 as one of its examples. Its diagnosis is that a patch API or PyPI network error makes the CLI exit 0 with the error recorded in its JSON output. The harness only retries non-zero exits, so the case goes on to fail a later check.
What I pushed: I couldn't re-run the job (the API returned 403), so I cherry-picked Fix PDM matrix flakes on PyPI/patch API transport blips #860's two commits (4329170 and 6b0302a) as 4846c0a and 678a1ff. They retry a case only when every failed check's own operation shows a terminal network error, and they never retry a functional failure. They will be no-ops once Fix PDM matrix flakes on PyPI/patch API transport blips #860 merges.
Local check: python3 -m unittest scripts/tests/test_backtest_harnesses.py passes 72/72.

Generated by Claude Code

Mikola Lysenko (mikolalysenko) · 2026-10-05T13:34:48Z

BugBot review

Generated by Claude Code

Mikola Lysenko (mikolalysenko) · 2026-10-05T13:36:39Z

[agent] composer 2.2.30 / php 8.3 / windows-latest failed on 678a1ff during setup, before any test ran. The runner's curl couldn't download the Composer phar from getcomposer.org because Windows certificate revocation checking was unavailable: curl: (35) schannel: ... CRYPT_E_REVOCATION_OFFLINE (0x80092013) - The revocation function was unable to check revocation because the revocation server was offline. This is a runner network problem and has nothing to do with this PR.

A single re-run should clear it, but this session can't trigger one (the re-run API returns 403). Could a maintainer re-run that job? I'm still watching the PR.

Generated by Claude Code

cursor

Stale Bugbot comment from a previous run.

Resolves the vendored Gemfile rewrite conflict with #849 (#847). The merged rewrite keeps #847's handling of positional version arguments (`*V`, constants, method calls) and reads the tail through the shared option reader. That reader now classifies positional arguments as version constraints, never sources, and flags `**opts` splats and hash literals as dynamic. Hosted mode still refuses dynamic options, while vendored mode keeps them after `path:` as #847 intends. Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) · 2026-10-05T14:37:36Z

BugBot review

Generated by Claude Code

Clears the merge conflict GitHub reported against main. Co-Authored-By: Claude <noreply@anthropic.com>

Mikola Lysenko (mikolalysenko) · 2026-10-05T14:40:17Z

bugbot run

Generated by Claude Code

cursor

✅ 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 d902d81. Configure here.}

Mikola Lysenko (mikolalysenko) · 2026-10-05T16:19:34Z

Burn-down agent: labeled Ready for review at d902d81.

CI: 486 success + 6 skipped, 0 failing; mergeable clean against main.
Bugbot: Cursor Bugbot check passed on d902d81; 0 unresolved review threads.
Linked issue(s) still open and not fixed on main.

Generated by Claude Code

Start fix for #652

6d53479

Assisted-by: Claude Code:claude-opus-5-5

Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 3, 2026

Hosted gem redirect treats a gitlab: or custom git_source gem as patched, so Bundler keeps loading the unpatched git checkout while VEX attests not_affected #652

Closed

Claude (claude) added 2 commits October 3, 2026 23:47

Mikola Lysenko (mikolalysenko) marked this pull request as ready for review October 4, 2026 00:00

cursor Bot reviewed Oct 4, 2026

View reviewed changes

Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 4, 2026

Fix human scan skipping re-apply of recorded patches (#732) #733

Merged

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

Tanmay Singla (Tanmay182003) approved these changes Oct 5, 2026

View reviewed changes

Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 5, 2026

Vendored gem rewrite puts non-string arguments after path:, so gem "x", *V, gem "x", ENV.fetch(…) or gem "x", VERSION becomes a Gemfile syntax error and every bundle command fails #847

Closed

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

cursor Bot reviewed Oct 5, 2026

View reviewed changes

Claude (claude) added 2 commits October 5, 2026 12:36

Merge remote-tracking branch 'origin/main' into agent/fix-gem-source-…

044a3fc

…option-guard

Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 5, 2026

Fix gem lock readers ignoring gems.locked (#736) #750

Merged

6 tasks done

cursor Bot reviewed Oct 5, 2026

View reviewed changes

Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 5, 2026

Fix pip rollback refusing all-hosted requirements (#410) #827

Open

Claude (claude) added 2 commits October 5, 2026 13:34

cursor Bot reviewed Oct 5, 2026

View reviewed changes

Merge origin/main into agent/fix-gem-source-option-guard

d902d81

Clears the merge conflict GitHub reported against main. Co-Authored-By: Claude <noreply@anthropic.com>

Mikola Lysenko (mikolalysenko) mentioned this pull request Oct 5, 2026

Retry Composer/Maven/Gradle downloads on connect and TLS errors #868

Merged

cursor Bot reviewed Oct 5, 2026

View reviewed changes

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

Mikola Lysenko (mikolalysenko) merged commit 33ddc26 into main Oct 5, 2026
493 checks passed

Mikola Lysenko (mikolalysenko) deleted the agent/fix-gem-source-option-guard branch October 5, 2026 17:25

This was referenced Oct 5, 2026

Fix gem rewrite deleting a ;-joined declaration (#826) #875

Open

Read Gemfile.lock sections and DEPENDENCIES entries through formats::gem in hosted and vendored modes #780

Open

Conversation

Mikola Lysenko (mikolalysenko) commented Oct 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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

Summary

Root cause

Fix

Test evidence

Performance

Checklist

Notes

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 4, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 4, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 4, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 5, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 5, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 5, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 5, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 5, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 5, 2026

Uh oh!

cursor Bot left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 5, 2026

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 5, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Mikola Lysenko (mikolalysenko) commented Oct 5, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Mikola Lysenko (mikolalysenko) commented Oct 3, 2026 •

edited

Loading

cursor Bot left a comment •

edited

Loading

cursor Bot left a comment •

edited

Loading

cursor Bot left a comment •

edited

Loading

cursor Bot left a comment •

edited

Loading

cursor Bot left a comment •

edited

Loading