From 0d0231ff97ad9a73894161dab5f73c403277fce4 Mon Sep 17 00:00:00 2001 From: elhoim Date: Thu, 24 Sep 2026 11:02:41 +0000 Subject: [PATCH] Report collection errors and broken filters in sigma check load_and_check_rules() only looked at the errors of the rules in SigmaCollection.rules. Errors that are not attached to a rule, such as an unknown collection 'action:' or an invalid filter (filters are kept in SigmaCollection.filters), were never reported, so 'sigma check' passed inputs that 'sigma convert' rejects. These errors are now reported, counted as rule errors (so --fail-on-error applies) and added to the JUnit report. Rule errors are also contained in SigmaCollection.errors; they are skipped there so they are not counted twice. Co-Authored-By: Claude Opus 5.5 (1M context) --- sigma/cli/check.py | 22 +++++++++++++++++++ tests/test_check.py | 51 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+) diff --git a/sigma/cli/check.py b/sigma/cli/check.py index 1a6359b..2c865b4 100644 --- a/sigma/cli/check.py +++ b/sigma/cli/check.py @@ -172,6 +172,28 @@ def load_and_check_rules(input, file_pattern, rule_errors, cond_errors, junit_re }) else: check_rules.append(rule) + + # Errors that are not attached to any loaded rule, e.g. an unknown collection action or + # an invalid filter (filters are kept in rule_collection.filters, not in .rules). + rule_error_ids = { + id(error) for rule in rule_collection.rules for error in rule.errors + } + for error in rule_collection.errors: + if id(error) in rule_error_ids: + continue + if first_error: + click.echo("=== Sigma Rule Errors ===") + first_error = False + click.echo(error) + rule_errors.update((error.__class__.__name__,)) + if junit_results is not None: + error_type = error.__class__.__name__ + source = getattr(error, "source", None) + file_path = str(source) if source else "unknown" + junit_results.append({ + "rule_name": file_path, "file_path": file_path, "status": "failed", + "issue_type": error_type, "severity": "error", "description": str(error) + }) return check_rules @click.command() diff --git a/tests/test_check.py b/tests/test_check.py index 006da08..ad514f3 100644 --- a/tests/test_check.py +++ b/tests/test_check.py @@ -149,3 +149,54 @@ def test_check_cli_generates_junitxml(tmp_path): assert out.exists() tree = ET.parse(str(out)) assert tree.getroot().tag == "testsuites" + + +def test_check_unknown_collection_action(tmp_path): + """Collection-level errors (not attached to any rule) must be reported and fail the check.""" + (tmp_path / "typo.yml").write_text("action: repaet\ntitle: typo in action keyword\n") + cli = CliRunner() + result = cli.invoke(check, [str(tmp_path)]) + assert result.exit_code == 1 + assert "Unknown Sigma collection action 'repaet'" in result.stdout + assert "Found 1 errors" in result.stdout + + +def test_check_invalid_filter(tmp_path): + """Errors in filters (kept in SigmaCollection.filters, not .rules) must fail the check.""" + (tmp_path / "filter.yml").write_text( + "title: filter with invalid id\n" + "id: not-a-uuid\n" + "logsource:\n" + " category: test\n" + "filter:\n" + " rules: []\n" + " selection:\n" + " User: x\n" + " condition: not selection\n" + ) + cli = CliRunner() + result = cli.invoke(check, [str(tmp_path)]) + assert result.exit_code == 1 + assert "SigmaIdentifierError" in result.stdout + assert "Found 1 errors" in result.stdout + + +def test_check_rule_errors_not_double_counted(): + """Per-rule errors are also contained in SigmaCollection.errors and must be counted once.""" + cli = CliRunner() + result = cli.invoke(check, ["tests/files/invalid"]) + assert result.exit_code == 1 + assert "Found 6 errors, 1 condition errors" in result.stdout + + +def test_check_collection_error_junitxml(tmp_path): + rules = tmp_path / "rules" + rules.mkdir() + (rules / "typo.yml").write_text("action: repaet\ntitle: typo in action keyword\n") + out = tmp_path / "report.xml" + cli = CliRunner() + result = cli.invoke(check, ["--junitxml", str(out), str(rules)]) + assert result.exit_code == 1 + tree = ET.parse(str(out)) + failures = tree.getroot().findall(".//failure") + assert any("repaet" in (f.text or "") + (f.get("message") or "") for f in failures)