feat(cli): Implement report baseline comparison and suppression features - #324
Draft
misonijnik wants to merge 29 commits into
Draft
feat(cli): Implement report baseline comparison and suppression features#324misonijnik wants to merge 29 commits into
misonijnik wants to merge 29 commits into
Conversation
misonijnik
force-pushed
the
misonijnik/supressions
branch
from
July 30, 2026 11:17
82d35fc to
5cbbc0c
Compare
misonijnik
marked this pull request as draft
July 30, 2026 11:18
misonijnik
force-pushed
the
misonijnik/supressions
branch
2 times, most recently
from
August 1, 2026 21:33
e8b01aa to
4e8e975
Compare
misonijnik
force-pushed
the
misonijnik/supressions
branch
3 times, most recently
from
August 24, 2026 02:48
efee5a1 to
b9b7f28
Compare
misonijnik
force-pushed
the
misonijnik/supressions
branch
from
August 24, 2026 08:58
b9b7f28 to
1f5eeca
Compare
misonijnik
force-pushed
the
misonijnik/supressions
branch
from
August 25, 2026 12:35
1f5eeca to
3818a3c
Compare
Adds the report-level engine for baselines and suppressions: - identity.go: fingerprint-key resolution (source/sink hash by default), identity lookup and git-style prefix resolution - baseline.go: new/unchanged/updated/absent classification, baselineState and baselineGuid application, run GUID stamping - suppress.go: the read rule for suppression status, accept/defer/unsuppress, verbatim inheritance from a baseline, and summary counters - save.go: atomic report writing PropertyBag now preserves unknown keys as raw JSON. It previously modelled only "tags", so any read-modify-write of a report silently dropped the rest of every property bag — which the round-trip test caught.
internal/triage applies one pass of baseline inheritance, accept/defer decisions and baseline comparison over a report, and is the single implementation behind the triage command, scan's annotation step and summary's read-only view. Gate decides whether findings fail the build: suppressed findings never count, and with a baseline only new (or uncomparable) ones do. The summary grows Baseline and Suppressions subsections, rendered only when they apply, and the finding listing hides suppressed results unless asked for them.
- opentaint triage: compare a report against a baseline, record accept
("won't fix") and defer ("not fixing for now") decisions by fingerprint
prefix, and write the annotated report
- scan: --baseline, --baseline-state, --error-on-findings,
--error-on-severity, --fingerprint-key; the report the analyzer wrote is
rewritten only when triage actually changed it
- summary: --baseline, --baseline-state, --suppressed; runs the same engine
read-only so the file is never touched
Read-only mode still annotates the in-memory report, otherwise
--baseline-state could not filter on a state nobody had written.
Rule lists are scan-time rule selection, not suppression: an excluded rule never loads, so it produces nothing to suppress. The analyzer only supports inclusion, so exclusion is implemented by enumerating every rule id in the ruleset and subtracting, then re-expanding join refs so that excluding a library rule cannot silently break a rule that joins against it. Resolution happens before the --dry-run bail-out, so a list that selects nothing is reported by --dry-run rather than after a full compile. The --rule-id flag continues to win over the config file.
Adds the opentaint triage reference, the baseline/gating flags on scan and summary, a Baselines and suppressions section explaining that presence in a baseline is not acceptance, and the rules.only/rules.exclude configuration keys. Suggestion builders learn the new flags so rebuilt commands keep them.
- rename TestGateCountsUpdatedFindingsAsNew: the body asserts the opposite (updated findings do not trip the gate) - Gate.inScope reuses sarif.MatchesSeverity (exported) instead of re-implementing the case-insensitive level match - fingerprintValue delegates to Identity instead of duplicating the lookup - scan loads the baseline once, before compile, and passes it through to the triage step instead of reading the file a second time
Rule exclusion was config-only (rules.exclude); this adds the flag analogue on scan. It overrides the configured exclude list, and composes with --rule-id — both were asked for explicitly, so exclusion subtracts from the selection instead of one flag silently winning. Emptying the selection is an error either way, caught by --dry-run before any compile.
…matcher matchesPattern in rules/select.go had invented a fourth match form, globbing the bare leaf name, which summary's --rule-id filter does not support. Export the string-level matcher as sarif.MatchesRuleID and delegate rule selection (rules.only/rules.exclude, --exclude-rule-id) to it, so one grammar holds everywhere: exact full path.yaml:id, exact bare name, or a doublestar glob over the full id only. Docs examples updated off the removed leaf-glob form.
Verifying that --exclude-rule-id reaches the analyzer as concrete --semgrep-rule-id args surfaced a silence: excluding a typo'd rule id matched nothing and looked effective. Selection.Unmatched reports patterns that select no rule, and scan warns for each — on both the config-list path and the --rule-id/--exclude-rule-id flag path. The configuration example also used a rule id that does not exist in the shipped ruleset; replaced with a real one.
…-exclude
An exclusion-only selection lowers to just the excluded concrete ids, passed
via --semgrep-rule-id-exclude, instead of expanding into the ~150-rule
inclusion complement. rules.Select now returns Resolved{Include, Exclude};
the allow-list path keeps CLI-side subtraction and join-ref re-expansion,
since inclusion must be exact ids anyway. --rule-id composed with
--exclude-rule-id still subtracts CLI-side (tiny explicit lists).
Requires the analyzer's --semgrep-rule-id-exclude option (separate core
branch); older jars reject it, so exclusion fails fast until the analyzer
version is bumped.
… filter The single --baseline-state name carried two grammars: a boolean on scan/triage that persisted result.baselineState into the file, and a value list on summary that filtered the listing. Rename the scan/triage switch to --write-baseline-state (matching its WriteBaselineState field and its intent), leaving --baseline-state <state> exclusively as the summary filter. The filter help now spells out the accepted values (new | unchanged | updated | absent). No behavior change beyond the flag name; --error-on-severity is unchanged.
The flag validated each repeatable value as a single token, so the natural --error-on-severity error,warning was rejected with "invalid severity". Split each value on commas so the comma form and the repeated-flag form are equivalent. Purely CLI-side (the gate never reaches the analyzer); the docs already used the comma form, which now works.
A dedicated task-oriented guide for the new capabilities: the two-axis mental model (baseline state vs. suppression), the full lifecycle from establishing a baseline through triage inheritance to gating, and reference tables for baseline states, suppression statuses, fingerprint identity, the failure gate + exit codes, and rule selection. Includes copy-paste GitHub Actions (actions/cache to carry the baseline) and GitLab recipes, plus the SARIF 2.1.0 conformance notes. Linked from the docs index (Guides + CI/CD) and the usage guide, whose long inline section is trimmed to a concise pointer keeping the flag tables.
…ounts Field-testing baselines on three Stirling-PDF releases surfaced six defects. The fingerprint summary printed was not the one triage resolved: the listing showed vulnerabilityWithTraceHash/v1 while triage --accept matched vulnerabilitySourceSinkHash/v1, so the documented copy-paste workflow always failed. One --fingerprint-key now governs baseline matching, the printed value, --partial-fingerprint, and the triage prefix; --partial-fingerprint-key is a deprecated alias. summary --baseline-state with no states to read printed "0 findings" and exited 0 — a clean bill of health for a report nobody compared. It now reads persisted states (so it works standalone on a --write-baseline-state report) and errors when there are none. --baseline-state absent could never match, since fixed findings live only in the baseline; it now lists them. Excluding a rule reported its baseline findings as Fixed, indistinguishable from real fixes. Findings whose rule did not run in the current scan are counted separately as "Rule not run". Under a display filter the Baseline and Suppressions groups still described the whole report; the counts are now recomputed over what was printed. A repeated --justification silently applied the last one to every decision in the run. It is now an error. --error-on-severity reported "invalid --severity"; it names the flag the user typed.
Nine runs of one unchanged project show that vulnerabilitySourceSinkHash/v1 changes between runs: 28 of 36 fingerprints stay the same. The report gives the cause chain from the graph construction to the fingerprint, the measurements at each step, the two causes the tests exclude, the one question that stays open, and the recommended change. Written in ASD-STE100 Simplified Technical English.
Analyzer 908e924 emits a third fingerprint, vulnerabilitySinkHash/v1, which hashes the sink statement; 06c8d25 puts the rule id into it, so it is now the "rule and sink only" identity that docs/reports/fingerprint-stability.md asked for. The CLI accepts it, so a decision can be made about a vulnerable statement rather than about one route into it: it survives a change to where the untrusted data comes from, and one entry then covers every caller. --fingerprint-key also takes short names now — trace, source-sink, sink — so a user does not have to type a versioned SARIF key to choose an identity. Any other value still passes through unchanged, because a report may carry fingerprints this build does not know. All three keys hash the rule id, so matching stays a plain fingerprint comparison: no key can make two rules that fire on one statement look like one finding. The default stays source-sink. The stability report recommended promoting the sink hash to default, but that recommendation assumed the source/sink hash was unstable, and 908e924 fixed that; the report now carries a status section saying so.
The sink hash becomes the default --fingerprint-key. It names the vulnerable statement and the rule, and nothing else, so a baseline entry or a triage decision survives every edit to how the untrusted data reaches the sink. It costs no precision. The analyzer reports one finding per rule and sink (VulnerabilityIdentity in TaintAnalysisUnitStorage.kt), so the coarsest key is still one fingerprint per finding: on the reproduction project, 180 findings and 180 distinct sink hashes. docs/reports/fingerprint-stability.md measured the same on Stirling-PDF, 36 of 36, and asked for exactly this change. A coarser identity leaves more underneath it, so a matched finding now reports what moved. SARIF has one word for all of it, "updated", which does not distinguish a sink that acquired a new source from a call path that was refactored — the first deserves a look, the second usually does not. The summary now reads "Updated, source changed" and "Updated, path changed", while result.baselineState stays "updated" for both, so --baseline-state keeps selecting either. The refinement follows the key ladder sink -> source-sink -> trace, so it also answers for a non-default identity: under source-sink only a moved path is reportable, because a moved source is already a different finding, and under trace nothing refines further. The coarsest difference wins, since a source that moves drags its trace along and the source is the more meaningful of the two.
The status section said the cause was repaired. It is not, and the commit was never meant to repair it. Measured on the reproduction project, 3 runs each: 278/300/314 statements with differing facts before #336, 268/392/364 after, out of 5244. The measurement that produced the wrong claim compared a probe hash that mixes in a processed-event counter. That counter varies between runs of both builds, so it could never separate them; the delayed-analyzer composition it was meant to sense is in fact identical in both. The reproduction project also turns out not to answer the fingerprint question at all: its fingerprints are stable before and after, because each of its findings has a single route from source to sink. Reproducing the drift needs a project where a finding has several. This makes the sink hash the mitigation rather than a convenience, which is noted where it is documented as the default.
The line is derived from vulnerabilityWithTraceHash/v1, and measurement on Stirling-PDF says that fingerprint is not reproducible: over 20 runs of the same analyzer on the same code, 19 disagree with the modal set, and ~1540 of ~1850 trace fingerprints are absent from at least one run. Comparing two such runs reports ~131 findings as "Updated, path changed" when nothing changed at all. So the line was close to 100% false positives, and it presented a known-unstable value as if it were a property of the code. Those findings now fall through to the plain "Updated" count, which is honest: something below the identity moved, and we cannot yet say what. ChangeSource keeps being reported. It comes from the source/sink hash, which is stable once duplicate sources are excluded from the digest. The classification itself is kept, with the reason it is unreported recorded on the constant, so restoring the line is a one-word change once the trace hash is deterministic.
… is noise" This reverts commit cbf058ceb127c0c06d488cff069eaff3fee6171a.
triage gets the structured Long (prose paragraphs, positional-arg sentence, cross-command pointers, exit-code block) and a cobra Example section instead of inline Examples. scan documents exit code 2 via a gate-aware exit-codes helper and gains a baseline-gating example; summary explains --baseline and --baseline-state in its Long and examples. Flag help is normalized to the house grammar: comma lists instead of pipes, '; defaults to X' instead of '(default X)', and the --fingerprint-key help no longer claims source-sink is the default when sink is. docs/usage.md tables are synced with the changed strings.
Same rule as on the base branch: no semicolons in written prose. Semicolon joins in the triage/scan/summary help text, flag descriptions, error messages, comments, and the two docs pages are rewritten as separate sentences, commas, or parentheticals. Go statement syntax is untouched.
…baseline safety Rule selection: a ruleset root that is a single YAML file now produces the ':<id>' ids the analyzer matches (it names a file root by the empty relative path), so exclusions and only-lists work for file rulesets. The built-in rules are fetched before an active selection resolves, so a fresh install no longer fails with 'no rules were found'. Identity: the sink/source-sink/trace aliases are expanded in summary and triage before the listing and filter paths consume them, so the fingerprint on screen is always the one a decision resolves. Results sharing one fingerprint value resolve together — one decision covers every duplicate — instead of being permanently ambiguous. Baseline safety: triage refuses an --output that would overwrite the baseline, scan validates the baseline carries the identity key before compiling instead of failing after the analysis, and --baseline-state absent without --baseline is an error rather than a silently empty listing. A filtered summary keeps the source-changed/path-changed attribution, recovered by identity value since filtered results are copies. Suggested follow-up commands no longer repeat the deprecated --partial-fingerprint-key next to --fingerprint-key.
golangci-lint (errcheck) flags the unchecked os.Remove and tmp.Close on the error paths of the atomic write. Both are deliberate best-effort calls whose errors carry no signal, so they are discarded explicitly.
The triage Long follows the ASD-STE100 style of the base branch: short sentences, one instruction per sentence, and simple words. The examples gain two recipes, the decision-by-decision triage loop and the baseline roll-forward after a release. The baseline paragraphs that this branch adds to the scan and summary help follow the same style, and scan gains a CI-gate recipe.
… of it An absent baseline finding was always summarized as Fixed, but a fingerprint disappears whenever the code it hashes moves, so a gone hash does not prove a gone finding. The comparison now looks for what remains before the summary makes that claim: - a current result matching under a coarser fingerprint proves the sink is still reported, shown as "Gone, sink still reported" - a new result of the same rule in the same file hints the finding moved with its hash, shown as "Gone, possibly moved" - only absences with nothing left behind keep the Fixed label The listing prints the qualifier next to the absent state, and updated findings get their source/path-changed note there too.
…int-key The identity of a finding is now always the sink hash. The finer hashes (source-sink, trace) no longer ever decide identity: the comparison reads them only to describe what happened to a finding — the existing "Updated, source changed" and "Updated, path changed" attribution. A configurable identity bought little and cost a lot: under a finer key every source or path change split one finding into a new + absent pair, and decisions silently detached from findings that never went away. With the key fixed, the absent-finding attribution also simplifies, and its labels change per review: - "Fixed" becomes "Absent": the finding is gone and nothing points at it, but the summary no longer claims someone fixed it - "Gone, possibly moved" becomes "Possibly drifted": a new finding of the same rule in the same file hints the sink hash moved with the code - "Gone, sink still reported" is gone with the finer keys that made it possible Removed flags: --fingerprint-key (scan, triage, summary) and the deprecated --partial-fingerprint-key. --partial-fingerprint still filters, by the identity value. A baseline without sink hashes is now rejected with a pointer at the analyzer being too old.
misonijnik
force-pushed
the
misonijnik/supressions
branch
from
September 4, 2026 19:04
cec50d1 to
74a9538
Compare
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.
No description provided.