fix(trivy_operator): fix compliance severity logic and checkID comparison

[SergK]

Description

Fix two bugs in the Trivy Operator parser.

Bug 1: Compliance severity logic is inverted (compliance_handler.py)

The severity selection logic is inverted — when check_severity is present, the code discards it and uses result_severity instead. When check_severity is empty, the code tries to use it, causing a KeyError crash.

Example — the existing cis_benchmark.json test fixture already proves this:

Input data:
  result.severity = "HIGH"
  check.severity  = "CRITICAL"

Original code path:
  check_severity = "CRITICAL"
  if not check_severity:          # not "CRITICAL" → False
      ...                         # skipped
  else:
      severity = TRIVY_SEVERITIES[result_severity]  # uses "HIGH" ← wrong!

Expected: "Critical" (check-level severity should take precedence)
Actual:   "High" (result-level severity used instead)

When check_severity is empty (e.g., a compliance pass sentinel), the original code crashes:

  check_severity = ""
  if not check_severity:          # not "" → True
      severity = TRIVY_SEVERITIES[""]  # KeyError: ""

Fix: Use check-level severity when present, fall back to result-level otherwise:

severity = TRIVY_SEVERITIES[check_severity] if check_severity else TRIVY_SEVERITIES[result_severity]

Reference: In the Trivy Operator Go source (compliance_types.go:237-240), when a control has zero misconfigurations, a ComplianceCheck{Success: true} sentinel is created with empty severity — confirming the empty-severity path is reachable.

Bug 2: checkID string/int comparison (checks_handler.py)

Three related issues with checkID handling:

1. check_id != 0 — string vs int comparison (always True in Python):

check_id = check.get("checkID", "0")  # returns string "0"
if check_id != 0:                      # "0" != 0 → always True (different types)
    check_references = ".../kubernetes/0"  # bogus URL generated

2. check.get("checkID", "0") — doesn't guard against null: Python's dict.get(key, default) only uses the default when the key is absent. If the JSON contains "checkID": null, Python returns None, not "0".

3. if check_id: — string "0" is truthy: When checkID is missing (defaulted to "0"), a bogus vulnerability ID is still generated because bool("0") is True in Python.

Fix: check.get("checkID") or "0" for null-safety, != "0" for proper string comparison.

Reference: In the Trivy Operator Go source (config_audit_types.go:111), checkID is defined as string (not *string), so Go serializes it as "" not null. The CRD schema marks it as required. The or "0" guard is defensive for manual JSON uploads to DefectDojo.

Test results

• Updated test_cis_benchmark — first finding now correctly asserts "Critical" (check-level) instead of "High" (result-level). The test data in cis_benchmark.json already had check.severity = "CRITICAL" and result.severity = "HIGH" — the original assertion matched the buggy output.
• Added test_compliance_severity_logic — two checks: one with severity="MEDIUM" (should use it), one with severity="" (should fall back to result's "HIGH")
• Added test_configauditreport_missing_checkid — check with no checkID field: verifies references is "" (no bogus URL)
• All existing tests pass
• ruff check passes

Documentation

No documentation changes needed — bug fixes to existing parser behavior.

Checklist

• Bugfixes should be submitted against the bugfix branch.
• Give a meaningful name to your PR, as it may end up being used in the release notes.
• Your code is flake8 compliant.
• Your code is python 3.13 compliant.
• Add applicable tests to the unit tests.
• Add the proper label to categorize your PR.

[valentijnscholten]

@SergK Thanks. Could you assess the possible impact on deduplication?

[SergK]

@valentijnscholten As far as I can see from the code, the Trivy Operator parser uses hash-code based deduplication configured in settings.dist.py:

"Trivy Operator Scan": ["title", "severity", "vulnerability_ids", "description"],

service is also always included via HASH_CODE_FIELDS_ALWAYS (settings.dist.py).

For the checkID fix (checks_handler.py) — no dedup impact. references is not part of the hash. The vulnerability_ids field was effectively always populated before too — the old if check_id: guard was truthy for string "0", so it generated an ID either way, just a bogus one. The fix actually makes things cleaner by not pushing a fake ID into vulnerability_ids when there is no real checkID.

For the severity fix (compliance_handler.py) — this one does affect dedup, since severity is part of the hash. Compliance findings that previously had the wrong severity (e.g., "High" instead of "Critical") will produce a different hash on the next reimport. So those findings will show up as new, and the old ones will get auto-closed.

That said, this is a one-time correction — the old severity values were simply wrong (inverted logic), so having them re-deduplicate with the correct severity is the right outcome. After the first reimport post-upgrade, everything settles and stays stable.