From c1417ec840eca722ab29111e73a40a328fdf8087 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 16:25:35 +0000 Subject: [PATCH 1/4] Start fix for #1573 Assisted-by: Claude Code:claude-opus-5-5 From d2b3976ce7bba390adce0d34433be909b9bb8373 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 16:36:45 +0000 Subject: [PATCH 2/4] Skip Ruby comments when naming a gem VEX product A gem library's VEX product comes from its root *.gemspec. The reader took the first line that looked like `spec.name = "..."` or `spec.version = "..."`, comments included, so a commented-out `# spec.version = "0.0.1"` above `spec.version = Acme::VERSION` gave pkg:gem/acme@0.0.1, and a commented `# spec.name = "acme-legacy"` renamed the product. Consumers matching VEX statements to their SBOM then never applied them. The gemspec is now read as Ruby sees it: `#` comments outside strings, =begin/=end blocks and heredoc bodies are skipped, and the last live assignment wins, so a computed version drops the version as documented. Fixes #1573 Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/vex/product.rs | 216 ++++++++++++++++++-- 1 file changed, 201 insertions(+), 15 deletions(-) diff --git a/crates/socket-patch-core/src/vex/product.rs b/crates/socket-patch-core/src/vex/product.rs index 8a8cc9a4e..368e45ae2 100644 --- a/crates/socket-patch-core/src/vex/product.rs +++ b/crates/socket-patch-core/src/vex/product.rs @@ -368,23 +368,24 @@ fn xml_element_texts(xml: &str, paths: [&[&str]; N]) -> [Option< /// `.gemspec` → `pkg:gem/[@]` from literal /// `.name = "…"` / `.version = "…"` assignments; a version /// computed from a constant (`Foo::VERSION`) is dropped, and the name falls -/// back to the file stem. +/// back to the file stem. As in Ruby, comments, `=begin`/`=end` blocks and +/// heredoc bodies are not code, and the last assignment wins (#1573). fn parse_gemspec(content: &str, stem: &str) -> Option { + let code = ruby_code_lines(strip_bom(content)); + // The value of the last `. = …` assignment: `Some(literal)` + // for a quoted literal, `None` for anything computed. let literal = |attr: &str| -> Option { - strip_bom(content).lines().find_map(|raw| { - let line = raw.trim(); - let (lhs, rhs) = line.split_once('=')?; - let lhs = lhs.trim(); - if !lhs.ends_with(&format!(".{attr}")) || rhs.starts_with('=') { - return None; - } - let rhs = rhs.trim(); - let quote = rhs.chars().next().filter(|c| *c == '"' || *c == '\'')?; - let inner = &rhs[1..]; - let end = inner.find(quote)?; - let value = inner[..end].trim(); - (!value.is_empty() && !value.contains("#{")).then(|| value.to_string()) - }) + let suffix = format!(".{attr}"); + code.iter() + .filter_map(|line| { + let (lhs, rhs) = line.split_once('=')?; + if !lhs.trim().ends_with(&suffix) || rhs.starts_with(['=', '~']) { + return None; + } + Some(quoted_literal(rhs.trim())) + }) + .next_back() + .flatten() }; let name = literal("name").unwrap_or_else(|| stem.to_string()); if name.is_empty() || name.contains(['/', '\\', ' ']) { @@ -396,6 +397,116 @@ fn parse_gemspec(content: &str, stem: &str) -> Option { }) } +/// The non-empty, non-interpolated text of a leading `"…"` / `'…'` literal. +fn quoted_literal(rhs: &str) -> Option { + let quote = rhs.chars().next().filter(|c| *c == '"' || *c == '\'')?; + let inner = &rhs[1..]; + let value = inner[..inner.find(quote)?].trim(); + (!value.is_empty() && !value.contains("#{")).then(|| value.to_string()) +} + +/// The code of each line of a Ruby file: `#` comments (outside string +/// literals) are cut off, and `=begin`/`=end` blocks and heredoc bodies are +/// skipped. A line-level reader, not a Ruby parser — enough to keep text +/// Ruby never executes from passing for an assignment. +fn ruby_code_lines(content: &str) -> Vec<&str> { + let mut out = Vec::new(); + let mut heredocs: Vec = Vec::new(); + let mut in_block_comment = false; + for raw in content.lines() { + let raw = raw.strip_suffix('\r').unwrap_or(raw); + if in_block_comment { + in_block_comment = !is_ruby_doc_marker(raw, "=end"); + continue; + } + if let Some(terminator) = heredocs.first() { + if raw.trim() == terminator { + heredocs.remove(0); + } + continue; + } + if is_ruby_doc_marker(raw, "=begin") { + in_block_comment = true; + continue; + } + let code = strip_ruby_comment(raw); + heredocs.extend(heredoc_terminators(code)); + out.push(code); + } + out +} + +/// `=begin` / `=end` only count at column 0, alone or followed by space. +fn is_ruby_doc_marker(line: &str, marker: &str) -> bool { + line.strip_prefix(marker) + .is_some_and(|rest| rest.is_empty() || rest.starts_with(char::is_whitespace)) +} + +/// `line` up to its first `#` outside a `"…"` / `'…'` literal. +fn strip_ruby_comment(line: &str) -> &str { + let mut quote: Option = None; + let mut escaped = false; + for (i, c) in line.char_indices() { + match quote { + Some(_) if escaped => escaped = false, + Some(_) if c == '\\' => escaped = true, + Some(q) if c == q => quote = None, + Some(_) => {} + None if c == '"' || c == '\'' => quote = Some(c), + None if c == '#' => return &line[..i], + None => {} + } + } + line +} + +/// The terminators of the heredocs a code line opens (`<<~ID`, `<<-ID`, +/// `< Vec { + let mut found = Vec::new(); + let mut quote: Option = None; + let mut escaped = false; + let mut i = 0; + while let Some(c) = code[i..].chars().next() { + let mut next = i + c.len_utf8(); + match quote { + Some(_) if escaped => escaped = false, + Some(_) if c == '\\' => escaped = true, + Some(q) if c == q => quote = None, + Some(_) => {} + None if c == '"' || c == '\'' => quote = Some(c), + None if code[i..].starts_with("<<") => { + let after = &code[i + 2..]; + let after = after.strip_prefix(['~', '-']).unwrap_or(after); + let (ident, quoted) = match after.chars().next() { + Some(q @ ('"' | '\'')) => (&after[1..], Some(q)), + _ => (after, None), + }; + let len = ident + .find(|c: char| !(c.is_ascii_alphanumeric() || c == '_')) + .unwrap_or(ident.len()); + let id = &ident[..len]; + let valid = !id.starts_with(|c: char| c.is_ascii_digit()) + && match quoted { + Some(q) => !id.is_empty() && ident[len..].starts_with(q), + None => id.starts_with(|c: char| c.is_ascii_uppercase()), + }; + if valid { + found.push(id.to_string()); + // Resume after the identifier (and its closing quote). + next = code.len() - ident.len() + len + usize::from(quoted.is_some()); + } else { + next = i + 2; + } + } + None => {} + } + i = next; + } + found +} + /// The one regular file directly under `cwd` named `*`, or `None` when /// there are zero or several (sorted names, so the answer is stable). async fn single_root_file_with_extension(cwd: &Path, ext: &str) -> Option { @@ -2752,6 +2863,81 @@ mod tests { ); } + /// #1573: a commented-out assignment is not an assignment. Ruby + /// (`Gem::Specification.load`) ignores `#` comments, `=begin`/`=end` + /// blocks and heredoc bodies, and the last live assignment wins. + #[tokio::test] + async fn detect_gemspec_ignores_commented_assignments() { + let gemspec = |body: &str| format!("Gem::Specification.new do |spec|\n{body}end\n"); + let cases: &[(&str, &str, &str)] = &[ + ( + "commented literal version above a computed one", + " spec.name = \"acme\"\n # spec.version = \"0.0.1\" # pinned before VERSION\n \ + spec.version = Acme::VERSION\n", + "pkg:gem/acme", + ), + ( + "commented name above the live one", + " # spec.name = \"acme-legacy\"\n spec.name = \"acme\"\n \ + spec.version = \"2.3.4\"\n", + "pkg:gem/acme@2.3.4", + ), + ( + "assignment after code in a trailing comment", + " spec.name = \"acme\"\n spec.files = [] # spec.version = \"0.0.1\"\n \ + x = 1 # spec.name = \"other\"\n", + "pkg:gem/acme", + ), + ( + "=begin / =end block", + "=begin\n spec.name = \"acme-legacy\"\n spec.version = \"0.0.1\"\n=end\n \ + spec.name = \"acme\"\n spec.version = Acme::VERSION\n", + "pkg:gem/acme", + ), + ( + "heredoc body", + " spec.name = \"acme\"\n spec.description = <<~DESC\n \ + spec.version = \"0.0.1\" was the old pin\n DESC\n \ + spec.version = Acme::VERSION\n", + "pkg:gem/acme", + ), + ( + "a later live assignment wins, as in Ruby", + " spec.name = \"acme\"\n spec.version = \"0.0.1\"\n \ + spec.version = Acme::VERSION\n", + "pkg:gem/acme", + ), + ( + "`<<` inside a string opens no heredoc", + " spec.summary = \"see < Date: Thu, 15 Oct 2026 17:12:20 +0000 Subject: [PATCH 3/4] Reuse the shared Ruby lexer for gemspec products Read the gemspec through formats::gem's for_each_code_line, the lexer the Gemfile readers already use, instead of a second copy. It also stops at __END__ and knows lower-case and backtick heredocs, so a DATA section or such a heredoc can't set the product's version. Only assignments to the Gem::Specification.new block parameter count when the file names one, so a later `ext.name = ...` in the same file can't rename the product now that the last assignment wins. Assisted-by: Claude Code:claude-opus-5-5 --- .../src/formats/gem/manifest.rs | 2 +- crates/socket-patch-core/src/vex/product.rs | 154 +++++------------- 2 files changed, 41 insertions(+), 115 deletions(-) diff --git a/crates/socket-patch-core/src/formats/gem/manifest.rs b/crates/socket-patch-core/src/formats/gem/manifest.rs index 7a992b8f9..2e212e979 100644 --- a/crates/socket-patch-core/src/formats/gem/manifest.rs +++ b/crates/socket-patch-core/src/formats/gem/manifest.rs @@ -469,7 +469,7 @@ fn plain_literal(s: &str) -> Option<(&str, &str)> { /// `code` is the line before any `#` comment. Skips `=begin`/`=end` /// blocks and heredoc bodies, and stops at `__END__`. A leading BOM is /// dropped, as Ruby does. -fn for_each_code_line(contents: &str, mut f: impl FnMut(&str, &str)) { +pub(crate) fn for_each_code_line<'a>(contents: &'a str, mut f: impl FnMut(&'a str, &'a str)) { use crate::formats::gem::gemfile::code_of; let contents = crate::formats::text::strip_bom(contents); let mut in_comment_block = false; diff --git a/crates/socket-patch-core/src/vex/product.rs b/crates/socket-patch-core/src/vex/product.rs index 368e45ae2..98300e1c9 100644 --- a/crates/socket-patch-core/src/vex/product.rs +++ b/crates/socket-patch-core/src/vex/product.rs @@ -368,21 +368,26 @@ fn xml_element_texts(xml: &str, paths: [&[&str]; N]) -> [Option< /// `.gemspec` → `pkg:gem/[@]` from literal /// `.name = "…"` / `.version = "…"` assignments; a version /// computed from a constant (`Foo::VERSION`) is dropped, and the name falls -/// back to the file stem. As in Ruby, comments, `=begin`/`=end` blocks and -/// heredoc bodies are not code, and the last assignment wins (#1573). +/// back to the file stem. As in Ruby, comments, `=begin`/`=end` blocks, +/// heredoc bodies and anything after `__END__` are not code, and the last +/// assignment wins (#1573). fn parse_gemspec(content: &str, stem: &str) -> Option { - let code = ruby_code_lines(strip_bom(content)); - // The value of the last `. = …` assignment: `Some(literal)` + let mut code: Vec<&str> = Vec::new(); + crate::formats::gem::manifest::for_each_code_line(content, |_, c| code.push(c)); + // The `spec` of `Gem::Specification.new do |spec|`, when the file names + // it, so a helper's `ext.name = …` elsewhere can't pass for the spec's. + let receiver = code.iter().find_map(|c| spec_block_param(c)); + // The value of the last `. = …` assignment: `Some(literal)` // for a quoted literal, `None` for anything computed. let literal = |attr: &str| -> Option { - let suffix = format!(".{attr}"); code.iter() .filter_map(|line| { let (lhs, rhs) = line.split_once('=')?; - if !lhs.trim().ends_with(&suffix) || rhs.starts_with(['=', '~']) { - return None; - } - Some(quoted_literal(rhs.trim())) + let (recv, method) = lhs.trim().rsplit_once('.')?; + let assigns = method == attr + && receiver.is_none_or(|r| recv == r) + && !rhs.starts_with(['=', '~']); + assigns.then(|| quoted_literal(rhs.trim())) }) .next_back() .flatten() @@ -397,116 +402,25 @@ fn parse_gemspec(content: &str, stem: &str) -> Option { }) } +/// The block parameter of `Gem::Specification.new … do |spec|` (or +/// `{ |spec|`) on this line of code. +fn spec_block_param(code: &str) -> Option<&str> { + let after = &code[code.find("Gem::Specification.new")?..]; + let (_, param) = after.split_once('|')?; + let param = param.split('|').next()?.trim(); + let ident = !param.is_empty() + && param.chars().all(|c| c.is_ascii_alphanumeric() || c == '_') + && !param.starts_with(|c: char| c.is_ascii_digit()); + ident.then_some(param) +} + /// The non-empty, non-interpolated text of a leading `"…"` / `'…'` literal. fn quoted_literal(rhs: &str) -> Option { - let quote = rhs.chars().next().filter(|c| *c == '"' || *c == '\'')?; - let inner = &rhs[1..]; - let value = inner[..inner.find(quote)?].trim(); + let (value, _) = crate::formats::gem::gemfile::leading_quoted(rhs)?; + let value = value.trim(); (!value.is_empty() && !value.contains("#{")).then(|| value.to_string()) } -/// The code of each line of a Ruby file: `#` comments (outside string -/// literals) are cut off, and `=begin`/`=end` blocks and heredoc bodies are -/// skipped. A line-level reader, not a Ruby parser — enough to keep text -/// Ruby never executes from passing for an assignment. -fn ruby_code_lines(content: &str) -> Vec<&str> { - let mut out = Vec::new(); - let mut heredocs: Vec = Vec::new(); - let mut in_block_comment = false; - for raw in content.lines() { - let raw = raw.strip_suffix('\r').unwrap_or(raw); - if in_block_comment { - in_block_comment = !is_ruby_doc_marker(raw, "=end"); - continue; - } - if let Some(terminator) = heredocs.first() { - if raw.trim() == terminator { - heredocs.remove(0); - } - continue; - } - if is_ruby_doc_marker(raw, "=begin") { - in_block_comment = true; - continue; - } - let code = strip_ruby_comment(raw); - heredocs.extend(heredoc_terminators(code)); - out.push(code); - } - out -} - -/// `=begin` / `=end` only count at column 0, alone or followed by space. -fn is_ruby_doc_marker(line: &str, marker: &str) -> bool { - line.strip_prefix(marker) - .is_some_and(|rest| rest.is_empty() || rest.starts_with(char::is_whitespace)) -} - -/// `line` up to its first `#` outside a `"…"` / `'…'` literal. -fn strip_ruby_comment(line: &str) -> &str { - let mut quote: Option = None; - let mut escaped = false; - for (i, c) in line.char_indices() { - match quote { - Some(_) if escaped => escaped = false, - Some(_) if c == '\\' => escaped = true, - Some(q) if c == q => quote = None, - Some(_) => {} - None if c == '"' || c == '\'' => quote = Some(c), - None if c == '#' => return &line[..i], - None => {} - } - } - line -} - -/// The terminators of the heredocs a code line opens (`<<~ID`, `<<-ID`, -/// `< Vec { - let mut found = Vec::new(); - let mut quote: Option = None; - let mut escaped = false; - let mut i = 0; - while let Some(c) = code[i..].chars().next() { - let mut next = i + c.len_utf8(); - match quote { - Some(_) if escaped => escaped = false, - Some(_) if c == '\\' => escaped = true, - Some(q) if c == q => quote = None, - Some(_) => {} - None if c == '"' || c == '\'' => quote = Some(c), - None if code[i..].starts_with("<<") => { - let after = &code[i + 2..]; - let after = after.strip_prefix(['~', '-']).unwrap_or(after); - let (ident, quoted) = match after.chars().next() { - Some(q @ ('"' | '\'')) => (&after[1..], Some(q)), - _ => (after, None), - }; - let len = ident - .find(|c: char| !(c.is_ascii_alphanumeric() || c == '_')) - .unwrap_or(ident.len()); - let id = &ident[..len]; - let valid = !id.starts_with(|c: char| c.is_ascii_digit()) - && match quoted { - Some(q) => !id.is_empty() && ident[len..].starts_with(q), - None => id.starts_with(|c: char| c.is_ascii_uppercase()), - }; - if valid { - found.push(id.to_string()); - // Resume after the identifier (and its closing quote). - next = code.len() - ident.len() + len + usize::from(quoted.is_some()); - } else { - next = i + 2; - } - } - None => {} - } - i = next; - } - found -} - /// The one regular file directly under `cwd` named `*`, or `None` when /// there are zero or several (sorted names, so the answer is stable). async fn single_root_file_with_extension(cwd: &Path, ext: &str) -> Option { @@ -2919,6 +2833,18 @@ mod tests { spec.version = \"0.0.1\"\n TEXT\n spec.version = \"3.0.0\"\n", "pkg:gem/acme@3.0.0", ), + ( + "DATA section after __END__", + " spec.name = \"acme\"\n spec.version = Acme::VERSION\nend\n__END__\n\ + spec.version = \"0.0.1\"\n", + "pkg:gem/acme", + ), + ( + "another receiver's `.name` / `.version` is not the spec's", + " spec.name = \"acme\"\n spec.version = \"1.2.0\"\n \ + ext.name = \"acme_ext\"\n task.version = \"0.1\"\n", + "pkg:gem/acme@1.2.0", + ), ( "trailing comment after a live literal", " spec.name = \"acme\" # the gem\n spec.version = \"1.2.0\" # bump\n", From 9de072deeace9f206f207e1eb3bab1d1debf840b Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 15 Oct 2026 17:32:36 +0000 Subject: [PATCH 4/4] Read gemspec assignments per statement A one-line spec such as `Gem::Specification.new { |s| s.name = "a"; s.version = "1" }`, or an assignment after the `do |s|` on the opener line, was not read: the product fell back to the file name with no version. Each line's code is now split into statements on `;`, after any block opener. Assisted-by: Claude Code:claude-opus-5-5 --- crates/socket-patch-core/src/vex/product.rs | 53 ++++++++++++++++++--- 1 file changed, 46 insertions(+), 7 deletions(-) diff --git a/crates/socket-patch-core/src/vex/product.rs b/crates/socket-patch-core/src/vex/product.rs index 98300e1c9..be46a8011 100644 --- a/crates/socket-patch-core/src/vex/product.rs +++ b/crates/socket-patch-core/src/vex/product.rs @@ -372,11 +372,19 @@ fn xml_element_texts(xml: &str, paths: [&[&str]; N]) -> [Option< /// heredoc bodies and anything after `__END__` are not code, and the last /// assignment wins (#1573). fn parse_gemspec(content: &str, stem: &str) -> Option { - let mut code: Vec<&str> = Vec::new(); - crate::formats::gem::manifest::for_each_code_line(content, |_, c| code.push(c)); + let mut lines: Vec<&str> = Vec::new(); + crate::formats::gem::manifest::for_each_code_line(content, |_, c| lines.push(c)); // The `spec` of `Gem::Specification.new do |spec|`, when the file names // it, so a helper's `ext.name = …` elsewhere can't pass for the spec's. - let receiver = code.iter().find_map(|c| spec_block_param(c)); + let receiver = lines + .iter() + .find_map(|c| spec_block_param(c).map(|(p, _)| p)); + // One entry per statement: a line's code after any block opener, split + // on `;` (`Gem::Specification.new { |s| s.name = "a"; s.version = "1" }`). + let code: Vec<&str> = lines + .iter() + .flat_map(|c| ruby_statements(spec_block_param(c).map_or(c, |(_, rest)| rest))) + .collect(); // The value of the last `. = …` assignment: `Some(literal)` // for a quoted literal, `None` for anything computed. let literal = |attr: &str| -> Option { @@ -403,15 +411,38 @@ fn parse_gemspec(content: &str, stem: &str) -> Option { } /// The block parameter of `Gem::Specification.new … do |spec|` (or -/// `{ |spec|`) on this line of code. -fn spec_block_param(code: &str) -> Option<&str> { +/// `{ |spec|`) on this line of code, and the code after it. +fn spec_block_param(code: &str) -> Option<(&str, &str)> { let after = &code[code.find("Gem::Specification.new")?..]; let (_, param) = after.split_once('|')?; - let param = param.split('|').next()?.trim(); + let (param, rest) = param.split_once('|')?; + let param = param.trim(); let ident = !param.is_empty() && param.chars().all(|c| c.is_ascii_alphanumeric() || c == '_') && !param.starts_with(|c: char| c.is_ascii_digit()); - ident.then_some(param) + ident.then_some((param, rest)) +} + +/// `code` split on each `;` outside a string literal. +fn ruby_statements(code: &str) -> Vec<&str> { + let mut out = Vec::new(); + let (mut quote, mut escaped, mut start) = (None, false, 0); + for (i, c) in code.char_indices() { + match quote { + Some(_) if escaped => escaped = false, + Some(_) if c == '\\' => escaped = true, + Some(q) if c == q => quote = None, + Some(_) => {} + None if c == '"' || c == '\'' => quote = Some(c), + None if c == ';' => { + out.push(&code[start..i]); + start = i + 1; + } + None => {} + } + } + out.push(&code[start..]); + out } /// The non-empty, non-interpolated text of a leading `"…"` / `'…'` literal. @@ -2855,6 +2886,14 @@ mod tests { let r = detect_in(&[("acme.gemspec", &gemspec(body))]).await; assert_eq!(r.purl.as_deref(), Some(*want), "{what}"); } + // Assignments on the block-opener line and after a `;`. + for one_line in [ + "Gem::Specification.new do |s| s.name = \"acme\"; s.version = \"4.5.6\"\nend\n", + "Gem::Specification.new { |s| s.name = 'acme'; s.version = '4.5.6' }\n", + ] { + let r = detect_in(&[("other.gemspec", one_line)]).await; + assert_eq!(r.purl.as_deref(), Some("pkg:gem/acme@4.5.6"), "{one_line}"); + } // Only a commented name: fall back to the file stem. let r = detect_in(&[( "acme.gemspec",