fix(core): avoid unidiff panic on bash xtrace (#1506)

## Summary
- preflight unified diff inputs before calling the Rust unidiff parser
- catch parser panics so malformed-but-diff-looking input falls back to
non-diff
- add regressions for Bash xtrace lines like `+++ test.sh` and `+++
dirname test.sh`

## Repro
`detect_content_type("+++ test.sh")` could panic through the Rust
detector because unidiff treated the lone `+++` line as a target header
without a preceding source header.

## Tests
- `cargo fmt --all --check`
- `cargo test -p headroom-core --lib
transforms::unidiff_detector::tests`
- `cargo test -p headroom-core --lib`

Co-authored-by: Michael Stattmann <mstattma@users.noreply.github.com>
Co-authored-by: JerrettDavis <mxjerrett@gmail.com>
This commit is contained in:
mstattma 2026-07-15 21:58:45 +02:00 committed by GitHub
parent 05932d7165
commit 3757a7cef3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -48,8 +48,9 @@ use unidiff::PatchSet;
/// real change content?
///
/// Empty input is **not** a diff (returns `false`) — saves a parser
/// call. Otherwise we hand off to [`PatchSet::parse`] and check that
/// the result has at least one file with at least one hunk.
/// call. Inputs must also have a real unified-diff source/target/hunk
/// shape before we hand off to [`PatchSet::parse`]. This keeps shell
/// traces like `+++ test.sh` from being treated as diff headers.
///
/// Why "at least one hunk" instead of "parsed without error":
/// `unidiff::PatchSet::parse` returns `Ok(())` even on plain text
@ -57,7 +58,7 @@ use unidiff::PatchSet;
/// passthrough through the diff compressor — a silent regression.
/// The explicit hunk check makes the contract honest.
pub fn is_diff(content: &str) -> bool {
if content.is_empty() {
if content.is_empty() || !looks_like_unified_diff(content) {
return false;
}
@ -85,6 +86,57 @@ pub fn is_diff(content: &str) -> bool {
.unwrap_or(false)
}
fn looks_like_unified_diff(content: &str) -> bool {
let mut saw_source_header = false;
let mut saw_target_header = false;
for line in content.lines() {
if line.starts_with("--- ") {
saw_source_header = true;
saw_target_header = false;
continue;
}
if saw_source_header && line.starts_with("+++ ") {
saw_target_header = true;
continue;
}
if saw_target_header && is_unified_hunk_header(line) {
return true;
}
}
false
}
fn is_unified_hunk_header(line: &str) -> bool {
let Some(rest) = line.strip_prefix("@@ -") else {
return false;
};
let Some((source_range, rest)) = rest.split_once(" +") else {
return false;
};
let Some((target_range, suffix)) = rest.split_once(" @@") else {
return false;
};
is_hunk_range(source_range)
&& is_hunk_range(target_range)
&& (suffix.is_empty() || suffix.starts_with(' '))
}
fn is_hunk_range(range: &str) -> bool {
let Some((start, length)) = range.split_once(',') else {
return !range.is_empty() && range.chars().all(|c| c.is_ascii_digit());
};
!start.is_empty()
&& !length.is_empty()
&& start.chars().all(|c| c.is_ascii_digit())
&& length.chars().all(|c| c.is_ascii_digit())
}
/// [`ContentType`]-typed wrapper. Returns `Some(ContentType::GitDiff)`
/// when [`is_diff`] is true, `None` otherwise. The router (PR5)
/// chains this after Magika and uses the `Option` to cleanly fall
@ -128,6 +180,20 @@ mod tests {
assert!(!is_diff(py));
}
#[test]
fn bash_xtrace_target_header_is_not_a_diff() {
assert!(!is_diff("+++ test.sh"));
assert_eq!(detect_diff("+++ test.sh"), None);
}
#[test]
fn bash_xtrace_output_is_not_a_diff() {
let output = "+ set -euo pipefail\n\
+++ dirname test.sh\n\
++ cd scripts/..\n";
assert!(!is_diff(output));
}
#[test]
fn standard_git_diff_detected() {
let diff = "diff --git a/foo.py b/foo.py\n\