Repository navigation
🐛 fix(regression): strict +4 pipelines, multi-entry validation & refactor - #97
Merged
Merged
Conversation
… export `regression.add_json_output` writes an auxiliary `<rule_id>.json` next to the data file, but its shape leaked into `regression_tests_info[0].type`: enabling it made every entry claim `type: ndjson` while `path` still named a `.evtx` file. SigmaHQ rejects that combination, so no Windows entry generated with the option could be merged. - `type` is now always the data file's own extension (`evtx`/`log`), so it always agrees with `path`. - The auxiliary export follows the json/ndjson rule: a single record is written as a pretty-printed `.json` document (reviewable in a diff, like the rest of the repo), several records stay newline-delimited with one compact document per line. Pretty-printing every record would have produced a file that is no longer valid ndjson. Two read-side defects made the bad metadata worse than a cosmetic mismatch: - `get_json_data` parsed the whole export as one value, so a multi-record export read as absent and silently disabled the `match_count` cross-check in `regressiondata-check`. Replaced by `get_json_records`, backed by a streaming parser that accepts both layouts. - An unrecognised `type` defaulted to `Json`, so an entry carrying the already committed `type: ndjson` parsed an EVTX blob as JSON and failed with a misleading "EMPTY". The data file on disk is now the ground truth for unknown types, and a recognised type that disagrees with the extension warns.
The format reference listed the accepted `type` values but never said what the field qualifies, which is what let `ndjson` — the layout of the auxiliary export — be written there. Documents the rule in both language mirrors: `type` matches the data file's extension, `add_json_output` never changes it, and readers fall back to the extension when `type` is unrecognised.
`clone_via_git_cli` called `find_git_executable()`, tested `is_none()`, then `unwrap()`ed two lines later; `find_git_executable` unwrapped `lines().next()` behind an `is_empty()` guard. Both unwraps were provably unreachable, so this is a contract change, not a behaviour fix. The `where git` parsing moves into a pure `first_executable_path` helper whose invariant — first non-empty trimmed line, or `None` — is now pinned by three tests instead of resting on an implicit `.trim()`. `None` is what lets the caller fall back to the grit-lib path, so it must never become an empty path. Non-test `unwrap()` count: 2 -> 0.
`test_signed_commit_accepted_by_real_git` was marked `#[ignore = "requires git binary on PATH"]`, leaving the ground-truth check for SSH signing permanently dark: nothing would catch a broken signature header or signed payload. The stated reason does not hold — git is on PATH in CI and the test completes in 0.57s, so the ignore only hid a passing oracle. Verified green before unignoring. Last `#[ignore]` in the tree; ignored tests: 1 -> 0.
…ages `clone_via_git_cli` degrades to a grit-lib depth-1 shallow clone when no git executable is found — which is every Linux and macOS run, since the lookup shells out to `where`. That fallback passed `false` for `sparse_checkout`, discarding the caller's request even though `clone_repo_inner_impl` implements it. With `git.sparse_checkout` defaulting to true, enabling `git.partial_clone` materialised the full working tree instead of rules/, rules-emerging-threats/ and regression_data/. The blobless filter is genuinely unavailable in the fallback, so the degradation itself stays; the warning now says so instead of only mentioning the shallow fallback. `sparse_checkout` is forwarded. Not unit-tested on purpose: both failure branches of `clone_repo_inner_impl` `remove_dir_all` the evidence (the sparse-checkout file lives under `.git`), so a black-box test could only assert something vacuous. The reasoning is recorded at the call site instead.
`inputs/ebpf.rs` and `inputs/ebpf_event.rs` are gated behind `feature = "ebpf"`, and the coverage gate runs `--features auditd,builtin, sysmon,evtx`. Those files therefore appeared in no coverage number in the project, and neither did `ebpf_common.rs` — 1 341 lines of shipped code measured by nothing. Adds a companion `coverage-ebpf` job on nightly + bpf-linker, mirroring the existing gate's pinned actions. Measured 78.43% (12 110/15 440) on 2026-09-26; the bar is 77 rather than the stable gate's 78 to absorb line-count drift between nightly releases. Scope, recorded in the workflow so the number is not over-read: this measures host-side ebpf code only. The nested `sigmacatch/ebpf` kernel probe is a separate workspace built for `bpfel-unknown-none` and llvm-cov cannot instrument those objects, so its 478 lines remain unmeasured — closing that needs in-kernel verification, not a coverage gate. Within the measured set, `inputs/ebpf_event.rs` sits at 96.8% while the privileged attach paths (`inputs/ebpf.rs` 62.9%, `ebpf_common.rs` 32.8%) drag the average down. Verified: 385 tests pass under the ebpf feature set, and zizmor reports no findings on the workflow.
…spec The upstream regression spec defines only `evtx`, `json`, `ndjson` and `jsonl`; its runner skips any other value as an unknown test type. sigmacatch additionally stores line-oriented Linux data as `log` and unprocessed Cisco output as `raw`, so those entries validate and replay here but are invisible upstream. That gap was silent. Add SIGMA_SPEC_TYPES / is_sigma_spec_type and have regressiondata-check report the out-of-spec types, one line per distinct type so a Linux run does not bury the real failures under hundreds of entries. Advisory only: the exit status is unchanged. `from_declared` deliberately keeps refusing `ndjson`/`jsonl`. They are spec spellings that map to no LogType, and accepting them would make a legacy `ndjson` declaration over an `.evtx` path parse an EVTX blob as JSON and fail as "EMPTY".
Every info.yml example in the regression-data-format guides used 2-space sequences, but the SigmaHQ layout — and regressiondata-check, which re-indents to it — use 4 spaces for the sequence and 6 for the item keys. `--fix` now reproduces a conformant file byte for byte, so the examples showed a shape the tooling rejects. Align all 16 sequences with the canonical form and document that `log` and `raw` are sigmacatch extensions outside the upstream spec, with the warning regressiondata-check now emits.
…e pipelines at +4 - Add to so the key is not silently dropped - New checks every entry: path exists & non-empty, pipelines/filters resolved via upstream walk-up - Blocking indentation validation for / sequences at +4 (README form); 3 upstream files at +0 now FAIL until - reindents / from +0 → +4, idempotent at +4 - 6 new logic-validation tests + existing suite green - clippy / fmt / typos / xwin build clean
- Config::validate: split into 8 focused helper methods (190→~25 lines each) - cli.rs: extract compare_filter_results, print_filter_result, build_rule_infos, compute_coverage_info, print_list_rules_output; filter_tests() data fn - evtx.rs: merge duplicated SAFETY comment block in write_evtx_winevt - evtx_writer.rs: decompose system_time_to_filetime into parse_datetime_components, parse_fraction_ns, parse_timezone (114→<30 lines) - regression/mod.rs: extract write_data_files, write_json_output, write_primary_data, cleanup_on_failure from generate() (110→~25 lines) - All clippy::too_many_lines resolved except filter_tests() data fn - cargo clippy --all-targets / fmt / typos / xwin build clean - Full test suite passes
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary
This PR fixes the regression data validation to match the upstream SigmaHQ runner behavior and canonicalizes `pipelines`/`filters` indentation to the README's strict `+4` form.
Changes
Validation Improvements
Refactoring (clippy `too_many_lines` resolved)
Quality Gates
Testing
Notes