Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Mikola Lysenko (mikolalysenko) merged 4 commits into from
Oct 5, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
651df1c
Start fix for #340
claude Oct 3, 2026
369daa5
Refuse multi-line and conditional gem lines
claude Oct 3, 2026
84a43d4
Refuse gem modifiers written as labels, and heredocs
claude Oct 3, 2026
464896d
fix(gem): refuse interpolated heredoc declarations
mikolalysenko Oct 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 131 additions & 2 deletions crates/socket-patch-cli/tests/e2e_redirect_gem_build.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,22 @@ enum Driver {
/// appending a source block would declare it twice. Same contract as
/// [`Driver::ScanVexDuplicateDeclaration`].
ScanVexEvalGemfile,
/// [`Driver::ScanVex`] on a Gemfile whose declaration continues on the
/// next line (`gem "x",` ↵ `require: false`, #340): rewriting the first
/// line would orphan the continuation after the source block. Same
/// contract as [`Driver::ScanVexDuplicateDeclaration`].
ScanVexMultiLineDeclaration,
/// [`Driver::ScanVex`] on a Gemfile whose declaration carries an `if`
/// modifier (#340): rewriting it would drop the condition. Same contract
/// as [`Driver::ScanVexDuplicateDeclaration`].
ScanVexConditionalDeclaration,
/// A modifier adjacent to a top-level constant is still a modifier,
/// not a hash label (`if::ENV`, #340).
ScanVexScopedConstantModifier,
/// A heredoc option continues beyond the declaration's physical line.
ScanVexHeredocDeclaration,
/// A double-quoted interpolation can itself contain a heredoc opener.
ScanVexInterpolatedHeredocDeclaration,
/// [`Driver::ScanVexDualBoot`] with `BUNDLE_GEMFILE=Gemfile` exported to
/// socket-patch too (#507): bundler's local app config outranks the
/// environment, so bundler still loads `Gemfile.next` and the run must
Expand All @@ -439,6 +455,13 @@ impl Driver {
Driver::ScanVexDualBoot => "scan --mode hosted (BUNDLE_GEMFILE=Gemfile.next)",
Driver::ScanVexDuplicateDeclaration => "scan --mode hosted (gem in two groups)",
Driver::ScanVexEvalGemfile => "scan --mode hosted (gem via eval_gemfile)",
Driver::ScanVexMultiLineDeclaration => "scan --mode hosted (multi-line gem line)",
Driver::ScanVexConditionalDeclaration => "scan --mode hosted (gem line with `if`)",
Driver::ScanVexScopedConstantModifier => "scan --mode hosted (gem line with `if::ENV`)",
Driver::ScanVexHeredocDeclaration => "scan --mode hosted (heredoc gem option)",
Driver::ScanVexInterpolatedHeredocDeclaration => {
"scan --mode hosted (interpolated heredoc gem option)"
}
Driver::ScanVexDualBootEnvGemfile => {
"scan --mode hosted (config Gemfile.next, env BUNDLE_GEMFILE=Gemfile)"
}
Expand Down Expand Up @@ -717,6 +740,26 @@ async fn redirect_scanned_project(
server.uri()
)
}
Driver::ScanVexMultiLineDeclaration => format!(
"source \"{}/upstream\"\n\ngem \"{DEP}\",\n require: false\n",
server.uri()
),
Driver::ScanVexConditionalDeclaration => format!(
"source \"{}/upstream\"\n\ngem \"{DEP}\" if ENV[\"WITH_VULN\"] != \"0\"\n",
server.uri()
),
Driver::ScanVexScopedConstantModifier => format!(
"source \"{}/upstream\"\n\ngem \"{DEP}\", \"{DEP_VERSION}\" if::ENV[\"WITH_VULN\"] != \"0\"\n",
server.uri()
),
Driver::ScanVexHeredocDeclaration => format!(
"source \"{}/upstream\"\n\ngem \"{DEP}\", require: <<~REQUIRE_PATH.chomp\n vuln_gem\nREQUIRE_PATH\n",
server.uri()
),
Driver::ScanVexInterpolatedHeredocDeclaration => format!(
"source \"{}/upstream\"\n\ngem \"{DEP}\", require: \"#{{<<~REQUIRE_PATH}}\".chomp\n vuln_gem\nREQUIRE_PATH\n",
server.uri()
),
_ => format!("source \"{}/upstream\"\n\ngem \"{DEP}\"\n", server.uri()),
};
std::fs::write(proj.join(gemfile_name), gemfile_body).unwrap();
Expand Down Expand Up @@ -823,7 +866,12 @@ async fn redirect_scanned_project(
| Driver::ScanVexDualBoot
| Driver::ScanVexDualBootEnvGemfile
| Driver::ScanVexDuplicateDeclaration
| Driver::ScanVexEvalGemfile => vec![
| Driver::ScanVexEvalGemfile
| Driver::ScanVexMultiLineDeclaration
| Driver::ScanVexConditionalDeclaration
| Driver::ScanVexScopedConstantModifier
| Driver::ScanVexHeredocDeclaration
| Driver::ScanVexInterpolatedHeredocDeclaration => vec![
"scan",
"--mode",
"hosted",
Expand Down Expand Up @@ -880,6 +928,13 @@ async fn redirect_scanned_project(
if let Some(warning) = match driver {
Driver::ScanVexDuplicateDeclaration => Some("redirect_gem_declared_more_than_once"),
Driver::ScanVexEvalGemfile => Some("redirect_gem_declaration_not_visible"),
Driver::ScanVexMultiLineDeclaration
| Driver::ScanVexConditionalDeclaration
| Driver::ScanVexScopedConstantModifier
| Driver::ScanVexHeredocDeclaration
| Driver::ScanVexInterpolatedHeredocDeclaration => {
Some("redirect_gem_unrecognized_declaration")
}
_ => None,
} {
assert_unwirable_declaration_redirects_nothing(
Expand Down Expand Up @@ -968,7 +1023,14 @@ async fn redirect_scanned_project(
Driver::ScanVexDualBoot
| Driver::ScanVexDualBootEnvGemfile
| Driver::ScanVexDuplicateDeclaration
| Driver::ScanVexEvalGemfile => unreachable!("asserted and returned above"),
| Driver::ScanVexEvalGemfile
| Driver::ScanVexMultiLineDeclaration
| Driver::ScanVexConditionalDeclaration
| Driver::ScanVexScopedConstantModifier
| Driver::ScanVexHeredocDeclaration
| Driver::ScanVexInterpolatedHeredocDeclaration => {
unreachable!("asserted and returned above")
}
Driver::GetUuid => {
// get's hosted envelope (CLI_CONTRACT.md "get --mode and
// installed narrowing"): `found` counts the resolved patch;
Expand Down Expand Up @@ -1717,6 +1779,73 @@ async fn gem_hosted_eval_gemfile_direct_dep_is_refused_and_still_installs() {
assert!(fx.is_none(), "the eval_gemfile driver asserts in place");
}

/// #340: a `gem` declaration that continues on the next line must not be
/// rewritten (the orphaned `require: false` made bundler refuse the Gemfile).
#[tokio::test(flavor = "multi_thread")]
#[ignore = "host capstone: shells out to a real ruby/gem/bundler (>= 1.17; CHECKSUMS arm >= 2.6); \
the unpinned `test` job skips it, an e2e job with a pinned toolchain runs it via --ignored"]
async fn gem_hosted_multi_line_declaration_is_refused_and_still_installs() {
let fx = redirect_scanned_project(
"multi-line",
Spelling::Gemfile,
false,
true,
None,
Driver::ScanVexMultiLineDeclaration,
)
.await;
assert!(fx.is_none(), "the multi-line driver asserts in place");
}

/// #340: a `gem` declaration with an `if` modifier must not be rewritten
/// (the rewrite dropped the condition and declared the gem unconditionally).
#[tokio::test(flavor = "multi_thread")]
#[ignore = "host capstone: shells out to a real ruby/gem/bundler (>= 1.17; CHECKSUMS arm >= 2.6); \
the unpinned `test` job skips it, an e2e job with a pinned toolchain runs it via --ignored"]
async fn gem_hosted_conditional_declaration_is_refused_and_still_installs() {
let fx = redirect_scanned_project(
"conditional",
Spelling::Gemfile,
false,
true,
None,
Driver::ScanVexConditionalDeclaration,
)
.await;
assert!(fx.is_none(), "the conditional driver asserts in place");
}

#[tokio::test(flavor = "multi_thread")]
#[ignore = "host capstone: shells out to a real ruby/gem/bundler (>= 1.17); \
run with a pinned toolchain via --ignored"]
async fn gem_hosted_scoped_constant_modifier_is_refused_and_still_installs() {
let fx = redirect_scanned_project(
"scoped-constant-modifier",
Spelling::Gemfile,
false,
true,
None,
Driver::ScanVexScopedConstantModifier,
)
.await;
assert!(fx.is_none(), "the scoped modifier driver asserts in place");
}

#[tokio::test(flavor = "multi_thread")]
#[ignore = "host capstone: shells out to a real ruby/gem/bundler (>= 1.17); \
run with a pinned toolchain via --ignored"]
async fn gem_hosted_heredoc_declaration_is_refused_and_still_installs() {
for driver in [
Driver::ScanVexHeredocDeclaration,
Driver::ScanVexInterpolatedHeredocDeclaration,
] {
let fx =
redirect_scanned_project(driver.label(), Spelling::Gemfile, false, true, None, driver)
.await;
assert!(fx.is_none(), "the heredoc driver asserts in place");
}
}

/// #507: the same dual boot with `BUNDLE_GEMFILE=Gemfile` exported. Bundler
/// ranks the committed `.bundle/config` above the environment (it still
/// loads `Gemfile.next`), so socket-patch must not follow the env value and
Expand Down
Loading
Loading