Skip to content

sarif: configurable rule severity per finding #58

@Fieldnote-Echo

Description

SARIF rule severity should be configurable per finding

Problem

The two SARIF rules in src/navi_bootstrap/sarif.py (AUDIT_RULES
constant) hardcode defaultConfiguration.level: "warning":

AUDIT_RULES: tuple[dict[str, Any], ...] = (
    {
        "id": "pack-drift-missing",
        ...
        "defaultConfiguration": {"level": "warning"},
    },
    {
        "id": "pack-drift-changed",
        ...
        "defaultConfiguration": {"level": "warning"},
    },
)

SarifResult already carries a level: str = "warning" field that
nothing currently sets, so per-finding override exists in the
dataclass but is unreachable from audit_cmd.

This blocks legitimate severity expressiveness: a missing
SECURITY.md from the security-scanning pack should arguably
land as error, while stylistic README drift should be note.
SARIF supports both per-rule defaults and per-result overrides;
we expose neither.

Proposed fix

Two options, in order of decreasing scope:

A. Per-pack severity in the manifest schema (preferred)

Extend schema/manifest-schema.yaml so each template entry can
declare a severity: key (error | warning | note). Plumb
through RenderEntry → DiffResult → AuditFinding → SarifResult.level.

templates:
  - src: SECURITY.md.j2
    dest: SECURITY.md
    severity: error  # new field; defaults to "warning" if absent

B. CLI-level uniform override (cheaper, less expressive)

Add --severity {error,warning,note} to audit_cmd; applies to
all findings emitted by that run. Simpler but loses the
"missing SECURITY.md is more serious than missing README badge"
distinction.

Acceptance criteria

  • At least one rule in the SARIF run can emit a finding at
    level != "warning".
  • findings_to_sarif() and SarifResult.to_dict() faithfully
    propagate the level.
  • partialFingerprints.primaryLocationLineHash is unchanged
    regardless of level (level is metadata; identity is rule+path).
  • All existing 0.2.0 packs still produce identical SARIF output
    when no severity is declared (default = warning).
  • Tests in tests/test_sarif.py covering the level pass-through.

Out of scope (companion follow-ups, not bundled)

These were considered for inclusion but rejected per audit:

  • Stdout audit-log line — stdout already emits the full report;
    the report IS the log, so a redundant line on stderr would just
    add CI noise. Skip unless concrete demand surfaces.
  • "No drift" success message in --output file — empty SARIF
    already serializes as a valid run with results: [], which any
    spec-compliant consumer parses correctly. Spec-compliant; skip.
  • CLI error-message hints with doc URLs — covered partially by
    the existing "file an issue" hint added in 83d9376; if more is
    needed it's a 5-minute follow-up to file separately.
  • SARIF extensibility caveat in audit.md — once this issue lands,
    the doc should describe the new level mechanism; a separate doc
    issue isn't needed.

Effort

  • Path A: ~2 hours (schema change + plumbing + tests + doc).
  • Path B: ~30 minutes (CLI flag + tests).

Source

Bot review on PR #51 (Grippy sarif.py:23/50 MEDIUM/LOW
"SARIF rule severity is always 'warning'"). Originally drafted as
a polish bundle; sharpened post-Codex review which correctly
identified that the other "polish" items were either already
covered or bot-noise. This is the one item with genuine product
value.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions