GeezerVison is showing github.com — Fix gem source-option guard missing git sources (#652) by mikolalysenko · Pull Request #731 · SocketDev/socket-patch
Conversation
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
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
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
cursor
Bot
left a comment
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
cursor
Bot
left a comment
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
[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
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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 <[email protected]>
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
cursor
Bot
left a comment
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
#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)
|
[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:
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
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
cursor
Bot
left a comment
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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) <[email protected]>
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) <[email protected]>
Claude-Session: https://claude.ai/code/session_01N8YeCUhdg2tKdqyyY7z3sV
(cherry picked from commit 6b0302a)
|
[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
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
[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
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
cursor
Bot
left a comment
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
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
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Clears the merge conflict GitHub reported against main.
Co-Authored-By: Claude <[email protected]>
cursor[bot] reacted with thumbs up emoji
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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 d902d81. Configure here.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
|
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
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Oct 5, 2026
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Labels
Ready for review
Agent-verified: mergeable, CI green, Bugbot clean — awaiting human review
Milestone
No milestone
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|