Fix hosted gem redirect breaking multi-line and conditional gem lines (#340)

[Mikola Lysenko (mikolalysenko)]

Review follow-up 464896d: closes the remaining interpolated-heredoc case in the shared Gemfile tail guard. Hosted and vendored modes leave unsupported declarations unchanged; native CLI tests also require zero redirects and VEX statements and a successful Bundler install. Preserves the author’s colon-modifier and ordinary-heredoc fixes. Validated with 267 core tests and four native tests covering five fixtures (Ruby 3.4.10 / Bundler 4.0.15); targeted Clippy and changed-hunk formatting checks pass. Ready to merge as-is from this review. 482 successful checks, 6 skipped, and all twelve workflows successful. Bugbot is clear on this commit; no unresolved review threads or new actionable findings.

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

Fixes #340

Summary

Hosted scan no longer rewrites a Gemfile gem declaration that continues on the next line or carries a modifier. Before this fix:

• gem "vuln-gem", ↵ require: false became a source block followed by an orphaned require: false, so every later bundle install failed to parse the Gemfile, while scan exited 0 and VEX attested the patch;
• gem "vuln-gem" if ENV["WITH_VULN"] != "0" became an unconditional declaration, so the condition was silently dropped.

Both shapes now skip the redirect with the existing redirect_gem_unrecognized_declaration warning (its detail now names the reason) and leave the Gemfile and lock byte-identical.

Root cause

The hosted rewriter (patch/redirect/mod.rs, gem_line_re → gem_line_trailing_options) replaces the declaration's single line with a source … do … end block, but it had no check that the line holds the whole declaration. Vendored mode had one (vendor::gem::rest_blocks_edit), but it used substring checks (ends_with(','), contains(" if ")) that missed several shapes.

Fix

• New shared gem_line_tail_blocks_edit (next to gem_line_trailing_options) scans the tail outside string literals and comments. It refuses:
  • an unterminated string, an unclosed (/[/{, or a heredoc (<<);
  • a tail that doesn't end in a value character (dangling ,, =>, key:, \);
  • a non-,-led tail;
  • a standalone if/unless/while/until/rescue/and/or/do keyword. Symbols (:unless), string contents, and hash keys (if: only where an argument can start, after ,, ( or {, and never if::) don't count. So "1.0.0" if::FEATURE and "1.0.0" if:flag == … are refused as modifiers.
• Hosted mode calls it after the existing source-option check, so the redirect_gem_source_option and own-vendored-wiring messages are unchanged.
• Vendored rest_blocks_edit delegates its structural checks to the same helper and keeps its source-token refusal. Behaviour there is stricter only on shapes that were already broken: a [ continuation, => continuation, tab-separated unless, rescue modifier, label-style modifiers, heredocs.
• The npm/pypi/gem wrappers only dispatch to the binary, so they need no change.

Test evidence

Issue arm	Test	Without fix	With fix
#340 multi-line (unit, 11 continuation shapes incl. CRLF, comment, =>, key:, \, brackets, paren form, heredoc)	patch::redirect::tests::gemfile_multi_line_or_conditional_declaration_fails_closed	FAILED (Gemfile rewritten with orphaned require: false)	ok
#340 conditional (unit, 11 modifier shapes incl. if::X, unless::X, if:x == …)	same test	FAILED	ok
Control: lookalikes still rewrite with options kept	gemfile_single_line_declaration_lookalikes_still_rewrite	ok	ok
Vendored shared-guard shapes	vendor::gem test block after the conditional assertion	—	ok
#340 multi-line, real Bundler 4.0.17 / Ruby 3.3.6	e2e_redirect_gem_build::gem_hosted_multi_line_declaration_is_refused_and_still_installs	FAILED	ok
#340 conditional, real Bundler 4.0.17 / Ruby 3.3.6	e2e_redirect_gem_build::gem_hosted_conditional_declaration_is_refused_and_still_installs	FAILED	ok

I got the red results by checking out main's crates/socket-patch-core/src with only the new tests applied. For 84a43d4, the if::FEATURE case was confirmed red against 369daa5's guard.

Local runs:

• cargo clippy --workspace --all-features -- -D warnings: clean.
• cargo test --workspace --all-features --no-fail-fast (at 369daa5): 9726 passed, 12 failed. All 12 are permission-denial or write-failure simulations (covgap_commands_vendor *_state_write_failure_*, repair_*unremovable*, *write_failure*, relax_loop_must_not_traverse_symlinked_root, …). They can't fail as intended in this sandbox, which runs as root (uid 0), and none of them touch gem code. CI runs as non-root, and all of these pass there. At 84a43d4, the core lib tests (same 4 root-only failures) and 91 CLI gem tests were re-run.
• cargo fmt: main itself isn't rustfmt-clean (about 500 diffs) and CI doesn't gate on it, so this PR formats only its own hunks.

🤖 Generated with Claude Code

https://claude.ai/code/session_015ZK1CuReoar7pKs2eZwepT

---

Note

Medium Risk
Changes Gemfile rewrite eligibility for hosted and vendored gem flows; incorrect classification could skip needed redirects or still break rare Gemfile shapes, but behavior is intentionally fail-closed with warnings.

Overview
Fixes #340: hosted gem redirect no longer rewrites gem lines whose declaration is not confined to a single safe line. Multi-line continuations, if/unless modifiers (including if::CONST), heredocs, and interpolated heredoc openers now fail closed with redirect_gem_unrecognized_declaration (detail names the reason) instead of producing broken Gemfiles or dropping conditions.

Adds shared gem_line_tail_blocks_edit, which scans the argument tail outside strings and comments for continuations, modifiers, and heredocs. The hosted rewriter calls it before moving a line into a source … do block; vendored Gemfile editing delegates the same structural rules via rest_blocks_edit, replacing weaker substring checks.

Unit tests cover refused shapes, lookalikes that still rewrite, and tail-parser edge cases. E2E e2e_redirect_gem_build gains drivers for multi-line, conditional, scoped-constant modifier, and heredoc fixtures (unchanged Gemfile/lock, no VEX, bundler still installs).

^{Reviewed by Cursor Bugbot for commit 464896d. Configure here.}

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

Burn-down agent: Ready for review at 369daa5.

• CI: 97/97 non-skipped checks green on the head SHA (3 skipped by path filters); mergeable with main.
• Bugbot: reviewed 369daa5, no issues found; no unresolved review threads.
• Reviewer focus: the behaviour change described in the summary; no outstanding findings.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

Codex follow-up review: correction pushed as 464896d, on top of the author’s 84a43d4.

The remaining interpolated-heredoc case is fixed. The shared hosted/vendor guard now refuses a possible << opener hidden inside double-quoted interpolation, preserving the Gemfile and lock instead of inserting end into the heredoc body. The author’s colon-modifier and ordinary-heredoc handling is preserved. Literal/escaped interpolation, comments, and keyword-shaped hash keys remain accepted controls.

Validation: 267 core tests and four native CLI tests passed, with five Ruby 3.4.10 / Bundler 4.0.15 fixture runs. Native cases cover colon-adjacent modifiers, plain and interpolated heredocs, and the original multiline/conditional declarations; they require a named refusal, unchanged files, zero redirects/VEX statements, and a successful native install. The actual-rewriter and native interpolation regressions both failed on the exact author head before this correction. Targeted Clippy passes with the existing macOS unused_variables allowance; changed hunks are formatted. An independent final source review found no additional issue, tested file hashes match the commit, and it merges cleanly with current main 045d7ec.

Ready to merge as-is from this review. 482 successful checks and 7 skipped; 12 workflows succeeded and one label-triggered Benchmarks workflow was skipped. Bugbot is clear on this commit; no unresolved review threads or new actionable findings. Ready for review has been restored. GitHub still requires the normal human approval before merge.

[Mikola Lysenko (mikolalysenko)]

Both findings from the Codex review of 369daa5 are addressed in 84a43d4:

• if::FEATURE / unless::FEATURE / if:enabled == …: gem_line_tail_blocks_edit now counts word: as a hash key only where an argument can start (after ,, ( or {) and never as word::. Directly after a value, these are refused as conditional declarations.
• require: heredoc: any << outside strings and comments is refused as a declaration that continues on the next line.

All four shapes are added to gemfile_multi_line_or_conditional_declaration_fails_closed. The if::FEATURE case fails without the guard change (Gemfile rewritten with the condition dropped) and passes with it. The lookalike controls still rewrite. Clippy is clean, and the core lib tests plus 91 CLI gem tests pass, apart from the 4 permission tests that can't pass as root in this sandbox.

The other agent run said it was working on the same fix. I checked that the branch had no new commits before pushing, so please build on 84a43d4 rather than pushing a parallel change.

---

Generated by Claude Code

[Mikola Lysenko (mikolalysenko)]

BugBot review

---

Generated by Claude Code

[cursor]

Stale Bugbot comment from a previous run.

[Mikola Lysenko (mikolalysenko)]

Cursor (@cursor) review

Please review follow-up 464896d0d404d69f6f7fb213b9e6de0d21256e8e, including the interpolated-heredoc guard and native regression coverage.

[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 464896d. Configure here.}