From 8edaa2916fd7810e020b87d5ecfe9eb4696b9358 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 08:51:38 +0000 Subject: [PATCH] triage: accept findings on the record, and gate on the rest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the oldest item in Known gaps. The two halves ship together because either one alone is worse than neither. --fail-on by itself turns a single unfixable transitive advisory into a build that is red every week forever, since the scheduled scan exists precisely to catch advisories published against code that has not changed. A job that is always red gets `|| true` appended or gets deleted, and both outcomes are what the exit-code design was built to prevent. So gating only becomes safe once there is a way to say "we have looked at this one". .icebergsca.toml records that decision. Every constraint in core/triage.py follows from this being the one feature whose job is to remove findings, which makes it the one most able to undermine the rest of the tool: - A suppressed finding stays in ScanReport.findings, marked. It is never filtered out, so table.py's empty-findings branch cannot print "no known vulnerabilities" for a scan where everything was accepted. There is a test asserting exactly that, and it fails if the filtering is ever moved upstream. - A reason is required, and an expiry always exists, defaulting to 90 days rather than being optional, so no entry becomes permanent by accident. Past it the finding returns; expiry never fails a scan by itself. - Matching is exact on advisory-or-alias plus package, so a rule can never grow to swallow a second, unrelated finding. The negative tests are the point. - parse_ignore_file is strict where every other parser here is forgiving. A third-party manifest is not ours to police; the file that decides which vulnerabilities you stop seeing is a different case, so an unknown key is a ConfigError rather than a silent skip. - A rule that matches nothing, or has expired, becomes a warning — a typo suppresses nothing and looks identical to a rule that is working. Gating checks completeness before severity, so it can never pass a scan that failed to look anything up, and counts only unsuppressed findings. It exits 3 rather than reusing 1: a pipeline that cannot tell "the scanner broke" from "the scanner found something" ends up treating both as noise. SARIF uses its own suppressions property, so GitHub shows an accepted finding as dismissed rather than closing it as fixed. CycloneDX gets analysis.detail only — state and justification are closed enums of security claims that a free-text reason cannot back, and asserting one would invent an analysis nobody performed. Verified end to end against a local OSV stub: a finding appears, is accepted by a rule written as the CVE where OSV answered with a GHSA, stays visible in every format while dropping out of the counts and the gate, and returns with a warning once the entry expires. JSON schema moves to 1.1 — additive only. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01By6nqX2dcU351oZn6y6Xoq --- CHANGELOG.md | 14 +- CLAUDE.md | 26 +- README.md | 6 +- .../.agents/skills/icebergsca/SKILL.md | 33 ++- .../icebergsca/references/ci-integration.md | 49 +++- .../icebergsca/references/json-report.md | 41 ++- src/icebergsca/cli/main.py | 101 ++++++- src/icebergsca/core/errors.py | 17 ++ src/icebergsca/core/models.py | 74 +++++- src/icebergsca/core/scanner.py | 14 +- src/icebergsca/core/triage.py | 229 ++++++++++++++++ src/icebergsca/report/csv_.py | 9 + src/icebergsca/report/cyclonedx.py | 14 + src/icebergsca/report/json_.py | 46 +++- src/icebergsca/report/sarif.py | 17 ++ src/icebergsca/report/table.py | 25 +- tests/test_cli.py | 185 ++++++++++++- tests/test_report.py | 103 +++++++- tests/test_report_formats.py | 47 +++- tests/test_skill.py | 3 + tests/test_triage.py | 247 ++++++++++++++++++ website/docs/cli.md | 27 +- website/docs/output.md | 31 ++- 23 files changed, 1311 insertions(+), 47 deletions(-) create mode 100644 src/icebergsca/core/triage.py create mode 100644 tests/test_triage.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 85403ed..e2e8dd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -61,7 +61,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 and output says "lookup did not run" rather than "no vulnerabilities found" whenever that is the truth. - Exit codes: `0` completed (regardless of findings), `1` scan failed or partial, `2` usage - error. Findings alone never fail a build. + error, `3` `--fail-on` threshold met. Findings alone never fail a build. +- `--fail-on ` gates a build on unsuppressed findings, checking scan completeness + first so it can never pass a scan that failed to look anything up. +- A triage file, `.icebergsca.toml`, records findings that have been assessed and accepted, so + gating is safe to leave switched on rather than being disabled the first week an unfixable + transitive advisory appears. Each entry requires a reason, matches exactly on advisory (or + alias) and package, and carries an expiry that defaults to 90 days. An accepted finding stays + in the report marked `accepted`, is counted by `summary.suppressed`, appears in SARIF as + dismissed rather than fixed, and returns when the entry expires. Rules that match nothing, or + have expired, are reported as warnings. +- JSON schema `1.1`: adds a top-level `suppressed` audit trail, `summary.suppressed`, and + `findings[].suppression`. Additive only — `summary.findings` and `by_severity` now count + active findings, which is unchanged for any scan with no ignore file. - Maven graphs are marked approximate — they are reconstructed, not read, and we never shell out to `mvn`. Each module of a multi-module build resolves against its own POM, so module boundaries hold: one module's nearest-wins choice cannot decide another's versions, and each diff --git a/CLAUDE.md b/CLAUDE.md index a0ab812..c7650e4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -146,6 +146,27 @@ schema validation in `tests/test_report_formats.py` against the official schemas **Exit code 2 is usage, 1 is scan failure** — the reverse of the original plan, because Click reserves 2 for usage errors and remapping it means overriding framework internals for nothing. +`--fail-on` gets its own code 3 rather than reusing 1: a pipeline that cannot tell "the scanner +broke" from "the scanner found something" ends up treating both as noise, which is the same +failure `|| true` represents. + +**Suppression and gating shipped together, deliberately.** `--fail-on` alone turns a single +unfixable transitive advisory into a build that is red every week forever, and a job that is +always red gets `|| true` or gets deleted. The ignore file is what makes the gate survivable. +The safeguards in `core/triage.py` all follow from it being the one feature whose job is to +remove findings: a suppressed finding stays in `ScanReport.findings` marked rather than being +filtered out (so `table.py`'s `if not report.findings` branch cannot print "no known +vulnerabilities" for an all-suppressed scan — `test_a_fully_suppressed_report_never_reads_as_clean` +guards exactly that), `reason` is mandatory, expiry defaults rather than being optional, matching +is exact on advisory-or-alias plus package, and a rule matching nothing becomes a warning. + +**`parse_ignore_file` is strict where every other parser is forgiving.** Ecosystem parsers skip +what they cannot read, because a third-party manifest is not ours to police. The ignore file is +the user's own and decides which vulnerabilities they stop seeing, so an unknown key is a +`ConfigError` rather than a silent skip. + +**`SeverityLevel` is a `StrEnum`, so `<` compares strings** — `HIGH < LOW` is True. Use +`at_or_above()` or `.rank`; never compare members directly. ## Testing @@ -168,7 +189,10 @@ and versions. This has caught two real bugs (a missing `coverage[toml]` extra tr ## Known gaps -- `--fail-on ` and an ignore/triage file — the obvious next feature. +- The ignore file suppresses; it does not yet express "not exploitable" in VEX terms. + `cyclonedx.py` emits `analysis.detail` only, because `state` and `justification` are closed + enums of security claims a free-text reason cannot back. An optional validated `state` key on + the entry is the obvious next step. - No SPDX output; CycloneDX only. - No SBOM *ingest*. - npm/yarn v1 lockfiles record no scope, so dev transitives are reported as runtime unless a diff --git a/README.md b/README.md index 5d12958..ba50f14 100644 --- a/README.md +++ b/README.md @@ -80,9 +80,11 @@ follows from this: | `0` | Scan completed. Vulnerabilities may have been found — that is not an error | | `1` | Scan failed, or completed only partially — some files or packages went unchecked | | `2` | Usage error: bad flag, unknown format (Click's reserved code) | +| `3` | Only with `--fail-on`: the scan completed and found something at or above the threshold | -Severity-based CI gating (`--fail-on`) is planned; the report model already carries everything -needed for it. +`--fail-on ` gates a build, and `.icebergsca.toml` records the findings you have +assessed and accepted so the gate is safe to leave switched on. An accepted finding stays in +the report, marked and counted separately — it is never silently dropped. ## Ecosystem support diff --git a/src/icebergsca/.agents/skills/icebergsca/SKILL.md b/src/icebergsca/.agents/skills/icebergsca/SKILL.md index 5674f93..7f7b36c 100644 --- a/src/icebergsca/.agents/skills/icebergsca/SKILL.md +++ b/src/icebergsca/.agents/skills/icebergsca/SKILL.md @@ -78,6 +78,7 @@ icebergsca scan . --format json | jq '{ | `scan.vulnerabilities_checked` | `false` means the OSV lookup never ran. No conclusion can be drawn at all | | `summary.unchecked_packages` | Above `0` means some packages could not be looked up. The result is partial | | `scan.complete` | `false` means either of the above, or a file failed to parse | +| `summary.suppressed` | Above `0` means findings were **accepted, not fixed**. Say so; never fold them into a clean summary | A correct summary: @@ -93,6 +94,9 @@ elif not report["summary"]["findings"]: verdict = f"No known vulnerabilities in {report['summary']['packages']} packages." else: verdict = f"{report['summary']['findings']} finding(s): {report['summary']['by_severity']}" + +if report["summary"]["suppressed"]: + verdict += f" ({report['summary']['suppressed']} accepted via the ignore file)" ``` ```python @@ -115,11 +119,36 @@ icebergsca scan .; echo "exit=$?" | `0` | The scan completed. Vulnerabilities may have been found — that is not an error | | `1` | The scan failed, or completed only partially. Treat the report as incomplete | | `2` | Usage error: unknown flag or format | +| `3` | Only with `--fail-on`: the scan completed and found something at or above the threshold | -Findings deliberately never change the exit code, so a non-zero exit always means *the tool -could not do its job*, not *your project has problems*. Do not use the exit code to decide +Findings deliberately never change the exit code unless `--fail-on` was passed, and even then +under a code of their own. So `1` always means *the tool could not do its job*, and `3` always +means *it did its job and you did not like the answer*. Do not use the exit code to decide whether vulnerabilities exist — read `summary.findings`. +## Accepting a finding + +Some findings cannot be fixed and should not be reported forever: an unfixable transitive +advisory, or one that is genuinely unreachable. `.icebergsca.toml` in the project root records +that decision. + +```toml +[[ignore]] +advisory = "GHSA-6757-jp84-gxfx" # or the CVE — aliases match too +package = "pyyaml" # bare name, or a purl with or without a version +reason = "Unreachable: we never call yaml.load on untrusted input" +expires = 2026-10-27 # optional; defaults to 90 days out +``` + +What this does **not** do is hide anything. The finding stays in `findings` with a +`suppression` object, `summary.suppressed` counts it, SARIF marks it dismissed rather than +fixed, and it stops tripping `--fail-on`. When reporting a scan, say how many findings were +accepted and by which rule — an accepted finding is a decision somebody made, not a solved +problem, and it comes back when the entry expires. + +A rule that matches nothing, or has expired, produces a `warnings` entry. Surface those: they +mean either a typo or an entry left behind after the upgrade that fixed it. + ## Reading a finding ```json diff --git a/src/icebergsca/.agents/skills/icebergsca/references/ci-integration.md b/src/icebergsca/.agents/skills/icebergsca/references/ci-integration.md index 8f46d2d..320e8b4 100644 --- a/src/icebergsca/.agents/skills/icebergsca/references/ci-integration.md +++ b/src/icebergsca/.agents/skills/icebergsca/references/ci-integration.md @@ -4,16 +4,54 @@ Findings never change the exit code. A non-zero exit means the scan itself did not complete. +Findings never change the exit code unless you ask them to with `--fail-on`. + | Code | Meaning | Pipeline response | |---|---|---| | `0` | Scan completed | Read `summary.findings` to decide what to do | | `1` | Scan failed or partial | Fail the job — the result is not trustworthy | | `2` | Usage error | Fix the invocation | +| `3` | `--fail-on` threshold met | Fail the job — the scan worked, the result did not pass | + +`1` and `3` are separate on purpose. A pipeline that cannot tell "the scanner broke" from "the +scanner found something" ends up treating both as noise. + +```bash +icebergsca scan . --fail-on critical +``` + +The gate checks completeness first, so it can never pass a scan that failed to look anything +up, and it ignores findings accepted in the ignore file. Do not add `|| true`: that is what +the ignore file is for. + +## Accepting a finding -This is deliberate: a scanner that exits non-zero on findings gets `|| true` appended within a -week, at which point genuine tool failures also go unnoticed. +The scheduled run below matters because most new findings arrive when an advisory is published, +not when the code changes. Which means one unfixable transitive advisory would otherwise turn +the build red every week forever — and a job that is red every week gets deleted. -To gate on severity today, read the JSON: +`.icebergsca.toml` in the project root records the assessment instead: + +```toml +[[ignore]] +advisory = "GHSA-6757-jp84-gxfx" # or the CVE — aliases are matched too +package = "pyyaml" # bare name, or a purl, optionally versioned +reason = "Unreachable: we never call yaml.load on untrusted input" +expires = 2026-10-27 # optional; defaults to 90 days from the scan +``` + +Point elsewhere with `--ignore-file`. What it does not do is hide anything: the finding stays +in the report with a `suppression` object, `summary.suppressed` counts it, SARIF marks it +dismissed rather than fixed, and `--fail-on` skips it. When the entry expires the finding comes +back — expiry never fails a scan by itself. + +A rule that matches nothing, or one past its date, lands in `warnings`. Watch for those: they +mean a typo, or an entry left behind after the upgrade that already fixed it. + +### Gating without `--fail-on` + +Reading the JSON still works, and is the way to gate on something more specific. Check +`scan.complete` **first** — gating on findings alone would pass a scan that checked nothing: ```bash icebergsca scan . --format json --output report.json @@ -22,10 +60,7 @@ jq -e '(.summary.by_severity.critical // 0) == 0' report.json > /dev/null \ || { echo "critical vulnerabilities found"; exit 1; } ``` -Check `scan.complete` **first**. Gating on findings alone would pass a scan that failed to -check anything. - -A built-in `--fail-on ` flag is planned but not yet implemented. +`summary.by_severity` counts active findings only, so accepted ones are already excluded. ## GitHub code scanning diff --git a/src/icebergsca/.agents/skills/icebergsca/references/json-report.md b/src/icebergsca/.agents/skills/icebergsca/references/json-report.md index 60c508a..ef809a4 100644 --- a/src/icebergsca/.agents/skills/icebergsca/references/json-report.md +++ b/src/icebergsca/.agents/skills/icebergsca/references/json-report.md @@ -12,13 +12,14 @@ icebergsca scan . --format json --output report.json ```json { - "schema_version": "1.0", + "schema_version": "1.1", "tool": { "name": "icebergsca", "version": "0.1.0" }, "scan": { ... }, "summary": { ... }, "manifests": [ ... ], "dependencies": [ ... ], "findings": [ ... ], + "suppressed": [ ... ], "unchecked_packages": [ ... ], "skipped": [ ... ], "warnings": [ ... ] @@ -53,6 +54,7 @@ it means the tool did its job. { "dependencies": 21, "direct": 9, "transitive": 12, "packages": 21, "findings": 12, + "suppressed": 2, "by_severity": { "critical": 3, "high": 4, "medium": 4, "low": 1 }, "unchecked_packages": 0, "skipped_files": 0 @@ -63,7 +65,12 @@ it means the tool did its job. manifests appears twice. `packages` counts unique packages. Use `packages` when reporting "we checked N packages". -`by_severity` omits levels with no findings and is ordered most severe first. +`findings` counts only findings nobody has accepted; `suppressed` counts the rest. The total in +the `findings` array is the two added together. **A suppressed finding is not a fixed one** — +somebody recorded a reason for accepting it and a date to revisit. Say so when reporting. + +`by_severity` omits levels with no findings and is ordered most severe first. It counts active +findings only, for the same reason. ## `manifests` @@ -144,11 +151,12 @@ One entry per ecosystem-and-directory scan unit. "direct": true, "introduced_by": [ { "path": "requirements.txt", "line": 3, "scope": "runtime", "direct": true } - ] + ], + "suppression": null } ``` -Sorted most severe first, then by package name. +Sorted active first, then most severe, then by package name. * `severity.level`: `critical`, `high`, `medium`, `low`, `none`, `unknown`. `unknown` means the advisory carries no rating — distinct from `none`, which is a scored zero. @@ -159,6 +167,31 @@ Sorted most severe first, then by package name. removal, not upgrade. * One CVE reported by several advisory databases is merged into a **single** finding, keeping whichever record carried the severity and fix version. The other IDs appear in `aliases`. +* `suppression` is `null` unless an ignore-file entry accepted the finding, in which case it + carries `reason`, `expires` and `rule`. **Read this field.** A suppressed finding is still in + this array — it is deliberately not removed — so code that ignores `suppression` counts it as + outstanding, and code that filters on it must not describe the result as "no vulnerabilities". + +## `suppressed` + +The ignore rules that fired, and what each accepted. This is an audit trail, not a second copy +of the findings — the per-finding state is `findings[].suppression`. + +```json +"suppressed": [ + { + "rule": "GHSA-6757-jp84-gxfx@pkg:pypi/pyyaml", + "reason": "Unreachable: we never call yaml.load on untrusted input", + "expires": "2026-10-27", + "findings": [{ "advisory": "GHSA-6757-jp84-gxfx", "purl": "pkg:pypi/pyyaml@5.1" }] + } +] +``` + +A rule that matched nothing, or one that has expired, does not appear here — it produces a +`warnings` entry instead, because a rule doing no work is usually a typo or an entry left behind +after the upgrade that fixed it. An expired entry stops suppressing, so the finding returns to +the active count. ## `unchecked_packages`, `skipped` and `warnings` diff --git a/src/icebergsca/cli/main.py b/src/icebergsca/cli/main.py index 2634160..924389a 100644 --- a/src/icebergsca/cli/main.py +++ b/src/icebergsca/cli/main.py @@ -23,17 +23,26 @@ from icebergsca.cache import Cache, cache_path from icebergsca.core.discovery import DiscoveryOptions from icebergsca.core.errors import IcebergSCAError -from icebergsca.core.models import EcosystemId, ScanReport, ScanStatus, Scope +from icebergsca.core.models import ( + EcosystemId, + ScanReport, + ScanStatus, + Scope, + SeverityLevel, +) from icebergsca.core.scanner import DEFAULT_SCOPES, ScanOptions, scan +from icebergsca.core.triage import IgnoreRule, parse_ignore_file from icebergsca.report import OutputFormat, cyclonedx, render class ExitCode(IntEnum): """Process exit codes. - Findings deliberately do not appear here. A scan that reports forty critical - vulnerabilities has done its job and exits :attr:`OK`; severity gating arrives - later as an explicit ``--fail-on`` flag. + Findings still do not affect the exit code on their own: a scan reporting forty + critical vulnerabilities has done its job and exits :attr:`OK`. They change it only + when the caller asked for a gate with ``--fail-on``, and even then under a code of + their own, so that "the gate tripped" stays distinguishable from "the scan broke". + A pipeline that cannot tell those apart eventually treats both as noise. :attr:`USAGE` is 2 rather than 1 because Typer's vendored Click reserves that code for usage errors, and remapping it would mean overriding framework @@ -45,6 +54,10 @@ class ExitCode(IntEnum): #: to check. Results are incomplete and must not be read as a clean result. SCAN_FAILED = 1 USAGE = 2 + #: Findings at or above the ``--fail-on`` threshold. Only ever returned when that + #: flag was passed, and only for a scan that completed — an incomplete scan is + #: :attr:`SCAN_FAILED` regardless of what it managed to find. + FINDINGS = 3 app = typer.Typer( @@ -57,6 +70,9 @@ class ExitCode(IntEnum): rich_markup_mode="rich", ) +#: The ignore file looked for beside the scan target when --ignore-file is absent. +IGNORE_FILENAME = ".icebergsca.toml" + logger = logging.getLogger("icebergsca") @@ -65,6 +81,60 @@ def _fail(message: str) -> NoReturn: raise typer.Exit(ExitCode.SCAN_FAILED) +def _ignore_rules(path: Path, ignore_file: Path | None) -> tuple[IgnoreRule, ...]: + """Locate and read the ignore file, if there is one. + + Reading it belongs here rather than in the scanner: it is tool configuration, not + scan input, and ``cli/`` is the only layer that touches either the filesystem for + its own purposes or the user directly. + + An explicitly named file that does not exist is an error — the caller asked for it + by name and a silent fall-through to "nothing is suppressed" would be a surprise in + the dangerous direction. A missing default file is simply the normal case. + """ + explicit = ignore_file is not None + target = ignore_file or _default_ignore_file(path) + + if not target.is_file(): + if explicit: + _fail(f"ignore file not found: {target}") + return () + + try: + content = target.read_text(encoding="utf-8") + except OSError as exc: + _fail(f"could not read {target}: {exc.strerror or exc}") + + try: + return parse_ignore_file(str(target), content) + except IcebergSCAError as exc: + _fail(str(exc)) + + +def _default_ignore_file(path: Path) -> Path: + """``.icebergsca.toml`` beside the scan target, or in it when it is a directory.""" + return (path if path.is_dir() else path.parent) / IGNORE_FILENAME + + +def _gate(report: ScanReport, threshold: SeverityLevel) -> None: + """Exit non-zero when an *unsuppressed* finding meets the threshold. + + Suppressed findings are excluded by design: being able to accept one is what makes + gating safe to turn on in the first place, since otherwise a single unfixable + transitive advisory turns the build red every week until somebody deletes the job. + """ + tripped = [f for f in report.active_findings if f.level.at_or_above(threshold)] + if not tripped: + return + + typer.secho( + f"{len(tripped)} finding(s) at or above {threshold.value}", + fg=typer.colors.RED, + err=True, + ) + raise typer.Exit(ExitCode.FINDINGS) + + def _configure_logging(verbosity: int, quiet: bool) -> None: """Send logs to stderr so that ``--format json | jq`` is never polluted.""" level = ( @@ -175,6 +245,22 @@ def scan_command( no_color: Annotated[ bool, typer.Option("--no-color", help="Disable coloured output.") ] = False, + fail_on: Annotated[ + SeverityLevel | None, + typer.Option( + "--fail-on", + help="Exit 3 when a finding at or above this severity is not suppressed. " + "An incomplete scan still exits 1, whatever it found.", + ), + ] = None, + ignore_file: Annotated[ + Path | None, + typer.Option( + "--ignore-file", + help=f"Accepted findings. Defaults to {IGNORE_FILENAME} beside the scan " + "target when it exists.", + ), + ] = None, verbose: Annotated[ int, typer.Option("-v", "--verbose", count=True, help="Increase log verbosity.") ] = 0, @@ -207,6 +293,7 @@ def scan_command( concurrency=concurrency, no_cache=no_cache, resolve_ranges=not no_resolve, + ignore_rules=_ignore_rules(path, ignore_file), ) try: @@ -216,6 +303,9 @@ def scan_command( _write(report, output_format, output, color=_use_color(no_color, output)) + # Completeness first, always. A gate that passed an incomplete scan because it + # happened to find nothing is the false clean this tool exists to avoid, and it is + # the single easiest way to build one. if report.status is not ScanStatus.OK or report.scan_failed: typer.secho( "scan incomplete — some files or packages could not be checked", @@ -224,6 +314,9 @@ def scan_command( ) raise typer.Exit(ExitCode.SCAN_FAILED) + if fail_on is not None: + _gate(report, fail_on) + def _use_color(no_color: bool, output: Path | None) -> bool: """Colour only when writing to a terminal, honouring the NO_COLOR convention.""" diff --git a/src/icebergsca/core/errors.py b/src/icebergsca/core/errors.py index ab75376..0b0836c 100644 --- a/src/icebergsca/core/errors.py +++ b/src/icebergsca/core/errors.py @@ -56,3 +56,20 @@ class UpstreamError(IcebergSCAError): class CacheError(IcebergSCAError): """The on-disk cache could not be opened or written.""" + + +class ConfigError(IcebergSCAError): + """The user's own configuration file is malformed. + + Deliberately strict, unlike the parsers: a third-party manifest we did not write + gets the benefit of the doubt and anything unrecognised is skipped, because the + alternative is refusing to scan a project over a field we never needed. An ignore + file is the opposite case. It exists to *remove* findings, so a mistyped key that + silently suppresses nothing — or everything — is precisely the failure this tool + must not have. Say so and stop. + """ + + def __init__(self, path: str, reason: str) -> None: + super().__init__(f"{path}: {reason}") + self.path = path + self.reason = reason diff --git a/src/icebergsca/core/models.py b/src/icebergsca/core/models.py index e3ed6b0..6512569 100644 --- a/src/icebergsca/core/models.py +++ b/src/icebergsca/core/models.py @@ -16,13 +16,13 @@ from __future__ import annotations from dataclasses import dataclass, field -from datetime import datetime +from datetime import date, datetime from enum import StrEnum from pathlib import Path from packageurl import PackageURL -SCHEMA_VERSION = "1.0" +SCHEMA_VERSION = "1.1" # --------------------------------------------------------------------------- # Ecosystems @@ -149,6 +149,15 @@ def rank(self) -> int: """Sort key, highest severity first when sorting descending.""" return _SEVERITY_RANK[self] + def at_or_above(self, threshold: SeverityLevel) -> bool: + """True when this level is at least as severe as ``threshold``. + + Use this rather than comparing members. ``SeverityLevel`` is a ``StrEnum``, so + ``<`` and ``>`` compare the *strings*: ``HIGH < LOW`` is True because ``"high"`` + sorts before ``"low"``, which is exactly backwards and silently so. + """ + return self.rank >= threshold.rank + _SEVERITY_RANK: dict[SeverityLevel, int] = { SeverityLevel.CRITICAL: 5, @@ -332,6 +341,25 @@ def cve_ids(self) -> tuple[str, ...]: return tuple(a for a in self.aliases if a.startswith("CVE-")) +@dataclass(frozen=True, slots=True) +class Suppression: + """A human's recorded decision to accept a finding, from the ignore file. + + Note what this is not: a statement that the finding is wrong, or fixed, or that the + package is unaffected. It records only that someone looked and wrote down why, with + a date by which they intend to look again. + """ + + #: Why the finding was accepted. Required — an entry with no justification is + #: indistinguishable from a mistake six months later. + reason: str + #: When the decision lapses. After this date the finding surfaces again. + expires: date + #: The ignore entry that matched, as ``advisory@package``, so a reader can find + #: the rule responsible without diffing the file. + rule: str + + @dataclass(frozen=True, slots=True) class Finding: """One advisory affecting one package, with every path that introduced it.""" @@ -341,11 +369,20 @@ class Finding: #: Lowest fixed version above the installed one, or ``None`` if no fix is known. fixed_version: str | None = None introduced_by: tuple[Dependency, ...] = () + #: Set when an ignore rule matched. A suppressed finding stays in the report and + #: stays in :attr:`ScanReport.findings`; only the counts and the exit-code gate + #: treat it differently. Removing it from the list would let an all-suppressed + #: scan render as "no known vulnerabilities". + suppression: Suppression | None = None @property def level(self) -> SeverityLevel: return self.advisory.level + @property + def is_suppressed(self) -> bool: + return self.suppression is not None + @property def is_direct(self) -> bool: return any(dep.direct for dep in self.introduced_by) @@ -448,18 +485,43 @@ def is_complete(self) -> bool: and self.status is ScanStatus.OK ) + @property + def active_findings(self) -> tuple[Finding, ...]: + """Findings nobody has accepted — what a build should be judged on.""" + return tuple(f for f in self.findings if not f.is_suppressed) + + @property + def suppressed_findings(self) -> tuple[Finding, ...]: + """Findings an ignore rule accepted. Still reported, never discarded.""" + return tuple(f for f in self.findings if f.is_suppressed) + def counts_by_severity(self) -> dict[SeverityLevel, int]: - """Finding counts per severity, highest first, omitting empty levels.""" + """Active finding counts per severity, highest first, omitting empty levels. + + Suppressed findings are excluded — they are counted separately rather than + folded in, so that "3 high" means three someone still has to deal with. With + no ignore file in play this is every finding, exactly as before. + """ counts: dict[SeverityLevel, int] = {} - for finding in self.findings: + for finding in self.active_findings: counts[finding.level] = counts.get(finding.level, 0) + 1 return dict(sorted(counts.items(), key=lambda item: item[0].rank, reverse=True)) def sorted_findings(self) -> tuple[Finding, ...]: - """Findings ordered most severe first, then by package for stable output.""" + """Findings ordered active first, then most severe, then by package. + + Suppressed findings sort last rather than being dropped: every renderer walks + this list, and one that stopped seeing them would report an accepted finding + as a fixed one. + """ return tuple( sorted( self.findings, - key=lambda f: (-f.level.rank, f.package.name, f.advisory.id), + key=lambda f: ( + f.is_suppressed, + -f.level.rank, + f.package.name, + f.advisory.id, + ), ) ) diff --git a/src/icebergsca/core/scanner.py b/src/icebergsca/core/scanner.py index 89d4f19..0160f0d 100644 --- a/src/icebergsca/core/scanner.py +++ b/src/icebergsca/core/scanner.py @@ -22,6 +22,7 @@ from icebergsca import __version__ from icebergsca.cache import Cache +from icebergsca.core import triage from icebergsca.core.discovery import Discovery, DiscoveryOptions, ScanUnit, discover from icebergsca.core.errors import CacheError, ParseError from icebergsca.core.graph import most_included @@ -37,6 +38,7 @@ Scope, SkippedFile, ) +from icebergsca.core.triage import IgnoreRule from icebergsca.ecosystems import get as get_ecosystem from icebergsca.ecosystems.base import FileParser from icebergsca.ecosystems.maven import MavenResolver, MavenResult, MavenUnit @@ -75,6 +77,10 @@ class ScanOptions: #: findings — and the report says so via ``vulnerabilities_checked`` rather than #: presenting an empty finding list as a clean result. check_vulnerabilities: bool = True + #: Accepted findings, already parsed. A plain value like every other option: the + #: CLI locates and reads ``.icebergsca.toml``, because tool configuration is its + #: business, not the scanner's. + ignore_rules: tuple[IgnoreRule, ...] = () async def scan(root: Path, options: ScanOptions | None = None) -> ScanReport: @@ -232,12 +238,18 @@ async def _resolve_and_check( for ref, hits in result.advisories.items() for hit in hits ) + # Applied here, at the one place findings are built, so the accepted ones are the + # same objects the report ships. Note this is below the ``check_vulnerabilities`` + # early return above: there is nothing to accept when nothing was looked up, and + # marking an empty list as triaged would be a clean result nobody earned. + findings, ignore_warnings = triage.apply(findings, options.ignore_rules) + return _VulnOutcome( dependencies, findings, result, checked=True, - warnings=warnings, + warnings=warnings + ignore_warnings, maven_approximate=maven.approximate, ) diff --git a/src/icebergsca/core/triage.py b/src/icebergsca/core/triage.py new file mode 100644 index 0000000..7764a15 --- /dev/null +++ b/src/icebergsca/core/triage.py @@ -0,0 +1,229 @@ +"""Accepting a finding, on the record. + +A weekly scheduled scan is the right way to run this tool, because most new findings +arrive when an advisory is published rather than when code changes. But it means a +project with one unfixable transitive advisory gets a red build every week forever, +with nowhere to say that somebody assessed it. That ends in ``|| true`` on the command +or the job quietly deleted, which is worse than the finding. + +So: an ignore file. Everything here is shaped by the fact that it is the one feature +whose job is to make findings go away, and therefore the one most able to undermine the +rest of the tool: + +* A suppressed finding stays in the report, marked. It never vanishes. +* A reason is required. Without one, an entry is indistinguishable from a mistake. +* An expiry always exists, defaulting rather than being optional, so that no entry + becomes permanent by accident. Past it, the finding comes back. +* Matching is exact. A rule names one advisory and one package, so it can never grow + to swallow a second, unrelated finding. +* A rule that matches nothing says so, because a typo that silently suppresses nothing + looks identical to a rule that is working. + +Parsing takes text, never a path — file reading lives in the layer above, as it does +for every other parser here. +""" + +from __future__ import annotations + +import tomllib +from dataclasses import dataclass, replace +from datetime import date, timedelta +from typing import Any + +from icebergsca.core.errors import ConfigError +from icebergsca.core.models import Finding, PackageRef, Suppression + +#: How long an entry lasts when it does not say. Long enough not to be busywork, short +#: enough that a yearly audit is not the first time anyone revisits it. +DEFAULT_EXPIRY_DAYS = 90 + +_ENTRY_KEYS = frozenset({"advisory", "package", "reason", "expires"}) +_REQUIRED_KEYS = ("advisory", "package", "reason") + + +@dataclass(frozen=True, slots=True) +class IgnoreRule: + """One ``[[ignore]]`` entry.""" + + #: An OSV ID or any of its aliases — ``GHSA-...``, ``CVE-...``, ``PYSEC-...``. + #: Matching either means a user can write the CVE they were sent even though OSV + #: answers with a GHSA. + advisory: str + #: A bare ``name`` (``group:artifact`` for Maven), or a purl with or without a + #: version. Never a pattern. The versioned form pins the decision to the version + #: assessed, so an upgrade brings the finding back for a fresh look; the other two + #: forms outlive it. Both are legitimate — which one is right depends on whether + #: the reason was about this version or about how the package is used. + package: str + reason: str + expires: date + + @property + def key(self) -> str: + return f"{self.advisory}@{self.package}" + + def is_expired(self, today: date) -> bool: + return today > self.expires + + def matches(self, finding: Finding) -> bool: + """Exact match on advisory (or alias) and package (purl or bare name). + + Deliberately not a substring or glob test. A rule for ``GHSA-aaaa`` must not + reach ``GHSA-aaaa-bbbb``, and one written for a package must not travel to + another that merely shares a prefix. + """ + advisory = self.advisory.lower() + known = {finding.advisory.id.lower()} + known.update(alias.lower() for alias in finding.advisory.aliases) + if advisory not in known: + return False + + ref = finding.package + unversioned = PackageRef(ref.ecosystem, ref.name).purl + return self.package in (ref.purl, unversioned, ref.name) + + +def parse_ignore_file( + path: str, content: str, *, today: date | None = None +) -> tuple[IgnoreRule, ...]: + """Parse ``.icebergsca.toml`` into rules, refusing anything it does not understand. + + Every parser in this codebase is forgiving, skipping what it cannot read, because a + third-party manifest is not ours to police and refusing to scan over an unknown + field would be worse. This one is the opposite: the file is the user's own, it + exists to remove findings, and a key silently ignored because it was misspelled is + the difference between a finding being assessed and being invisible. + """ + today = today or date.today() + + try: + data = tomllib.loads(content) + except tomllib.TOMLDecodeError as exc: + raise ConfigError(path, f"invalid TOML: {exc}") from exc + + raw = data.get("ignore", []) + if not isinstance(raw, list): + raise ConfigError(path, "'ignore' must be a list of [[ignore]] entries") + + rules: list[IgnoreRule] = [] + for index, entry in enumerate(raw, start=1): + rules.append(_rule(path, index, entry, today)) + + _reject_duplicates(path, rules) + return tuple(rules) + + +def apply( + findings: tuple[Finding, ...], + rules: tuple[IgnoreRule, ...], + *, + today: date | None = None, +) -> tuple[tuple[Finding, ...], tuple[str, ...]]: + """Mark findings their rules accept, and report on the rules themselves. + + Returns the findings — all of them, with the accepted ones marked — and warnings + about rules that did no work. A rule matching nothing is worth saying out loud: it + is either a typo, or an entry left behind after the upgrade that fixed it, and both + look exactly like a rule that is quietly working. + """ + if not rules: + return findings, () + + today = today or date.today() + live = [rule for rule in rules if not rule.is_expired(today)] + expired = [rule for rule in rules if rule.is_expired(today)] + + used: set[str] = set() + marked: list[Finding] = [] + for finding in findings: + rule = next((r for r in live if r.matches(finding)), None) + if rule is None: + marked.append(finding) + continue + used.add(rule.key) + marked.append( + replace( + finding, + suppression=Suppression( + reason=rule.reason, expires=rule.expires, rule=rule.key + ), + ) + ) + + warnings: list[str] = [] + for rule in expired: + # Not an error, and emphatically not a reason to fail the scan: the finding + # simply stops being accepted and shows up again, which is the point. + warnings.append( + f"ignore entry {rule.key} expired on {rule.expires.isoformat()}; " + "the finding is reported again" + ) + for rule in live: + if rule.key not in used: + warnings.append( + f"ignore entry {rule.key} matched no finding; " + "it may be misspelled, or the finding may already be resolved" + ) + + return tuple(marked), tuple(warnings) + + +def _rule(path: str, index: int, entry: Any, today: date) -> IgnoreRule: + where = f"ignore entry {index}" + if not isinstance(entry, dict): + raise ConfigError(path, f"{where} must be a table") + + unknown = sorted(set(entry) - _ENTRY_KEYS) + if unknown: + known = ", ".join(sorted(_ENTRY_KEYS)) + raise ConfigError( + path, f"{where} has unknown key(s) {', '.join(unknown)}; expected {known}" + ) + + values: dict[str, str] = {} + for key in _REQUIRED_KEYS: + value = entry.get(key) + if not isinstance(value, str) or not value.strip(): + raise ConfigError(path, f"{where} needs a non-empty '{key}'") + values[key] = value.strip() + + return IgnoreRule( + advisory=values["advisory"], + package=values["package"], + reason=values["reason"], + expires=_expires(path, where, entry.get("expires"), today), + ) + + +def _expires(path: str, where: str, value: Any, today: date) -> date: + """A date, or a default — never "no expiry". + + TOML has a native date type, so an unquoted ``2026-10-27`` arrives as a ``date``. A + quoted one arrives as a string and is parsed, because the difference is invisible in + a text editor and refusing it would teach nothing. + """ + if value is None: + return today + timedelta(days=DEFAULT_EXPIRY_DAYS) + if isinstance(value, date): + return value + if isinstance(value, str): + try: + return date.fromisoformat(value.strip()) + except ValueError as exc: + raise ConfigError( + path, f"{where} has an unparseable 'expires': {exc}" + ) from exc + raise ConfigError(path, f"{where} 'expires' must be a date like 2026-10-27") + + +def _reject_duplicates(path: str, rules: list[IgnoreRule]) -> None: + """Two entries for the same advisory and package mean one of them does nothing. + + Which one wins is an implementation detail nobody should have to know, and the + second is usually an edit that was meant to replace the first. + """ + seen: set[str] = set() + for rule in rules: + if rule.key in seen: + raise ConfigError(path, f"duplicate ignore entry for {rule.key}") + seen.add(rule.key) diff --git a/src/icebergsca/report/csv_.py b/src/icebergsca/report/csv_.py index b11064f..7625981 100644 --- a/src/icebergsca/report/csv_.py +++ b/src/icebergsca/report/csv_.py @@ -36,6 +36,10 @@ "source_file", "source_line", "summary", + # Appended, never inserted: consumers index these columns positionally and a + # spreadsheet built last quarter should still line up. + "suppressed", + "suppression_reason", ) @@ -84,6 +88,11 @@ def render(report: ScanReport, *, color: bool = True, width: int | None = None) str(dep.source.path) if dep else "", (dep.source.line or "") if dep else "", advisory.summary.replace("\n", " ").strip(), + "true" if finding.is_suppressed else "false", + # User-supplied text bound for a spreadsheet. It goes through _defuse + # with every other cell below, which is what keeps a reason beginning + # "=" from becoming a formula. + finding.suppression.reason if finding.suppression else "", ) writer.writerow([_defuse(cell) for cell in row]) diff --git a/src/icebergsca/report/cyclonedx.py b/src/icebergsca/report/cyclonedx.py index 3bc2753..c6a9a8f 100644 --- a/src/icebergsca/report/cyclonedx.py +++ b/src/icebergsca/report/cyclonedx.py @@ -99,6 +99,10 @@ def _metadata(report: ScanReport) -> dict[str, Any]: "value": str(report.vulnerabilities_checked).lower(), }, {"name": "icebergsca:complete", "value": str(report.is_complete).lower()}, + { + "name": "icebergsca:suppressedCount", + "value": str(len(report.suppressed_findings)), + }, ], } @@ -188,6 +192,16 @@ def _vulnerability(finding: Finding) -> dict[str, Any]: entry["recommendation"] = ( f"Upgrade {finding.package.name} to {finding.fixed_version} or later." ) + if finding.suppression is not None: + # Only ``detail``. CycloneDX's ``state`` and ``justification`` are closed + # enumerations of security claims — ``not_affected``, ``code_not_reachable`` + # and so on — and all this tool actually knows is that a human wrote a sentence + # and a date. Asserting one of those values from free text would be inventing + # an analysis nobody performed. + entry["analysis"] = { + "detail": f"Accepted until {finding.suppression.expires.isoformat()}: " + f"{finding.suppression.reason}" + } return entry diff --git a/src/icebergsca/report/json_.py b/src/icebergsca/report/json_.py index 772ebbb..872b551 100644 --- a/src/icebergsca/report/json_.py +++ b/src/icebergsca/report/json_.py @@ -45,7 +45,11 @@ def to_dict(report: ScanReport) -> dict[str, Any]: "direct": report.direct_count, "transitive": report.transitive_count, "packages": len(report.packages), - "findings": len(report.findings), + # Outstanding findings. ``suppressed`` is reported beside it rather than + # inside it, so a consumer reading only ``findings`` under-states nothing: + # the total is findings + suppressed, and both are named. + "findings": len(report.active_findings), + "suppressed": len(report.suppressed_findings), "by_severity": { level.value: count for level, count in report.counts_by_severity().items() @@ -56,6 +60,12 @@ def to_dict(report: ScanReport) -> dict[str, Any]: "manifests": [_manifest(result) for result in report.manifests], "dependencies": [_dependency(dep) for dep in report.dependencies], "findings": [_finding(finding) for finding in report.sorted_findings()], + # The rules, not a second copy of the findings. Duplicating finding objects + # across two arrays invites double-counting, and lets a consumer read + # ``findings`` alone and never learn some were accepted — which is why the + # per-finding state lives on the finding itself. What is here instead is the + # audit trail: which entry accepted what, and which did no work at all. + "suppressed": _suppressions(report), "unchecked_packages": [_package(ref) for ref in report.scan_failed], "skipped": [ {"path": str(entry.path), "reason": entry.reason} @@ -65,6 +75,28 @@ def to_dict(report: ScanReport) -> dict[str, Any]: } +def _suppressions(report: ScanReport) -> list[dict[str, Any]]: + """One entry per ignore rule that fired, with what it accepted.""" + grouped: dict[str, dict[str, Any]] = {} + for finding in report.sorted_findings(): + rule = finding.suppression + if rule is None: + continue + entry = grouped.setdefault( + rule.rule, + { + "rule": rule.rule, + "reason": rule.reason, + "expires": rule.expires.isoformat(), + "findings": [], + }, + ) + entry["findings"].append( + {"advisory": finding.advisory.id, "purl": finding.package.purl} + ) + return list(grouped.values()) + + def _package(ref: PackageRef) -> dict[str, Any]: return { "ecosystem": ref.ecosystem.osv_name, @@ -138,4 +170,16 @@ def _finding(finding: Finding) -> dict[str, Any]: } for dep in finding.introduced_by ], + # Present on every finding, ``null`` when nothing accepted it, so that reading + # this field is never optional. A consumer that skips it counts an accepted + # finding as outstanding, which is the harmless direction. + "suppression": ( + None + if finding.suppression is None + else { + "reason": finding.suppression.reason, + "expires": finding.suppression.expires.isoformat(), + "rule": finding.suppression.rule, + } + ), } diff --git a/src/icebergsca/report/sarif.py b/src/icebergsca/report/sarif.py index 9d7f242..db5d31a 100644 --- a/src/icebergsca/report/sarif.py +++ b/src/icebergsca/report/sarif.py @@ -181,6 +181,23 @@ def _help_markdown(finding: Finding) -> str: def _result(finding: Finding) -> dict[str, Any]: + result = _base_result(finding) + if finding.suppression is not None: + # SARIF's own slot for this. Kept in ``results`` rather than filtered out, so + # GitHub renders the alert as dismissed rather than closing it as fixed — and + # so the rule it points at survives, since the rule list is derived from the + # same findings. + result["suppressions"] = [ + { + "kind": "external", + "status": "accepted", + "justification": finding.suppression.reason, + } + ] + return result + + +def _base_result(finding: Finding) -> dict[str, Any]: return { "ruleId": _rule_id(finding), "level": _LEVEL[finding.level], diff --git a/src/icebergsca/report/table.py b/src/icebergsca/report/table.py index e2ea1da..a6bff63 100644 --- a/src/icebergsca/report/table.py +++ b/src/icebergsca/report/table.py @@ -144,19 +144,28 @@ def _render_findings(console: Console, report: ScanReport) -> None: table.add_column("Advisory") table.add_column("Fixed in") table.add_column("Via") + table.add_column("") for finding in report.sorted_findings(): - style = _SEVERITY_STYLE[finding.level] + # Suppressed rows are dimmed rather than dropped, and keep their real severity + # so that nobody reads an accepted critical as a medium. + style = "dim" if finding.is_suppressed else _SEVERITY_STYLE[finding.level] table.add_row( f"[{style}]{finding.level.value}[/{style}]", - escape(finding.package.name), + f"[{style}]{escape(finding.package.name)}[/{style}]", escape(finding.package.version or "?"), escape(_advisory_label(finding)), escape(finding.fixed_version or "—"), escape(_via_label(finding)), + "[dim]accepted[/dim]" if finding.is_suppressed else "", ) console.print(table) + if report.suppressed_findings: + console.print( + "[dim]accepted — matched an entry in the ignore file; still reported, " + "and excluded from the counts below and from --fail-on[/dim]" + ) console.print() _render_summary(console, report) @@ -179,9 +188,15 @@ def _render_summary(console: Console, report: ScanReport) -> None: f"[{_SEVERITY_STYLE[level]}]{count} {level.value}[/{_SEVERITY_STYLE[level]}]" for level, count in report.counts_by_severity().items() ] - console.print( - f"[bold]{len(report.findings)} finding(s):[/bold] " + " ".join(parts) - ) + # The headline number counts what is still outstanding. Accepted findings are named + # separately rather than folded in or dropped: rolled into the total they would look + # like work nobody has done, and omitted entirely they would look like work that + # never existed. + suppressed = len(report.suppressed_findings) + line = f"[bold]{len(report.active_findings)} finding(s):[/bold] " + " ".join(parts) + if suppressed: + line += f" [dim]· {suppressed} accepted[/dim]" + console.print(line) console.print() diff --git a/tests/test_cli.py b/tests/test_cli.py index bee7f73..c7e7e65 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -6,6 +6,7 @@ import re from pathlib import Path +import httpx import pytest from typer.testing import CliRunner @@ -71,7 +72,7 @@ def test_output_file(tmp_path: Path) -> None: ], ) assert result.exit_code == ExitCode.OK - assert json.loads(destination.read_text())["schema_version"] == "1.0" + assert json.loads(destination.read_text())["schema_version"] == "1.1" def test_dev_dependencies_are_excluded_by_default(tmp_path: Path) -> None: @@ -241,3 +242,185 @@ def explode(self: Cache) -> int: assert result.exit_code == ExitCode.SCAN_FAILED assert isinstance(result.exception, SystemExit) assert "disk is full" in result.stderr + + +# --------------------------------------------------------------------------- +# Suppression and --fail-on +# --------------------------------------------------------------------------- + + +class _FindingTransport(httpx.AsyncBaseTransport): + """Answer OSV as though every queried package had one high-severity advisory.""" + + async def handle_async_request(self, request: httpx.Request) -> httpx.Response: + if request.url.path.endswith("/querybatch"): + body = json.loads(request.content or b'{"queries": []}') + results = [ + {"vulns": [{"id": "GHSA-test", "modified": "2026-01-01T00:00:00Z"}]} + for _ in body.get("queries", []) + ] + return httpx.Response(200, json={"results": results}, request=request) + if "/v1/vulns/" in request.url.path: + return httpx.Response( + 200, + json={ + "id": "GHSA-test", + "modified": "2026-01-01T00:00:00Z", + "aliases": ["CVE-2026-9999"], + "summary": "Example", + "severity": [ + { + "type": "CVSS_V3", + "score": "CVSS:3.1/AV:N/AC:L/PR:N/UI:N/S:U/C:H/I:N/A:N", + } + ], + }, + request=request, + ) + return httpx.Response(404, json={"error": "not mocked"}, request=request) + + +@pytest.fixture +def with_findings(monkeypatch: pytest.MonkeyPatch) -> None: + from icebergsca.core import scanner + + monkeypatch.setattr( + scanner, + "build_http_client", + lambda options: httpx.AsyncClient(transport=_FindingTransport()), + ) + + +def one_package(tmp_path: Path) -> Path: + """A project with a single dependency, so one ignore entry covers everything.""" + (tmp_path / "requirements.txt").write_text("requests==2.31.0\n") + return tmp_path + + +def ignore_file(tmp_path: Path, **fields: str) -> Path: + entry = { + "advisory": "GHSA-test", + "package": "requests", + "reason": "Assessed: not reachable from our entry points", + } | fields + body = "\n".join(f'{key} = "{value}"' for key, value in entry.items()) + target = tmp_path / ".icebergsca.toml" + target.write_text(f"[[ignore]]\n{body}\n") + return target + + +def test_findings_alone_still_exit_zero(tmp_path: Path, with_findings: None) -> None: + """The design this feature must not undo: a finding is data, not a failure.""" + result = runner.invoke(app, ["scan", str(project(tmp_path))]) + assert result.exit_code == ExitCode.OK + + +def test_fail_on_exits_three_above_the_threshold( + tmp_path: Path, with_findings: None +) -> None: + result = runner.invoke(app, ["scan", str(project(tmp_path)), "--fail-on", "high"]) + assert result.exit_code == ExitCode.FINDINGS + + +def test_fail_on_exits_zero_below_the_threshold( + tmp_path: Path, with_findings: None +) -> None: + result = runner.invoke( + app, ["scan", str(project(tmp_path)), "--fail-on", "critical"] + ) + assert result.exit_code == ExitCode.OK + + +def test_an_accepted_finding_does_not_trip_the_gate( + tmp_path: Path, with_findings: None +) -> None: + """The whole point of pairing the two: gating is only safe once you can accept.""" + target = one_package(tmp_path) + ignore_file(target) + result = runner.invoke(app, ["scan", str(target), "--fail-on", "high"]) + assert result.exit_code == ExitCode.OK + + +def test_an_expired_entry_trips_the_gate_again( + tmp_path: Path, with_findings: None +) -> None: + target = one_package(tmp_path) + ignore_file(target, expires="2020-01-01") + result = runner.invoke(app, ["scan", str(target), "--fail-on", "high"]) + assert result.exit_code == ExitCode.FINDINGS + + +def test_an_incomplete_scan_exits_one_not_three( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """The false-clean trap, from the other side. + + Completeness is checked before the gate, so a scan that could not look anything up + fails as a broken scan — never passing because it happened to find nothing, and + never reported as a policy failure it did not actually establish. + """ + from icebergsca.core import scanner + from icebergsca.osv import client as client_module + + # Same collapse as tests/test_osv.py: the retry schedule is asserted there, and + # paying it in real seconds here would put the suite over its time budget. + monkeypatch.setattr(client_module, "_BACKOFF_BASE", 0.0001) + monkeypatch.setattr(client_module, "_BACKOFF_CAP", 0.001) + + class _Unreachable(httpx.AsyncBaseTransport): + async def handle_async_request(self, request: httpx.Request) -> httpx.Response: + raise httpx.ConnectError("no route to host") + + monkeypatch.setattr( + scanner, + "build_http_client", + lambda options: httpx.AsyncClient(transport=_Unreachable()), + ) + result = runner.invoke( + app, ["scan", str(project(tmp_path)), "--fail-on", "critical"] + ) + assert result.exit_code == ExitCode.SCAN_FAILED + + +def test_a_suppressed_finding_is_still_reported( + tmp_path: Path, with_findings: None +) -> None: + target = one_package(tmp_path) + ignore_file(target) + result = runner.invoke(app, ["scan", str(target), "--format", "json"]) + document = json.loads(result.stdout) + + assert document["summary"]["suppressed"] == 1 + assert any(f["suppression"] for f in document["findings"]) + + +def test_an_unreadable_ignore_file_named_explicitly_is_an_error( + tmp_path: Path, +) -> None: + """Asking for a file by name and silently getting no rules is the wrong surprise.""" + result = runner.invoke( + app, + ["scan", str(project(tmp_path)), "--ignore-file", str(tmp_path / "nope.toml")], + ) + assert result.exit_code == ExitCode.SCAN_FAILED + assert "not found" in result.stderr + + +def test_a_malformed_ignore_file_fails_loudly(tmp_path: Path) -> None: + target = project(tmp_path) + (target / ".icebergsca.toml").write_text('[[ignore]]\nadvisroy = "x"\n') + result = runner.invoke(app, ["scan", str(target)]) + assert result.exit_code == ExitCode.SCAN_FAILED + assert "advisroy" in result.stderr + + +def test_unknown_fail_on_severity_is_a_usage_error(tmp_path: Path) -> None: + result = runner.invoke( + app, ["scan", str(project(tmp_path)), "--fail-on", "catastrophic"] + ) + assert result.exit_code == ExitCode.USAGE + + +def test_fail_on_choices_are_listed_in_help() -> None: + result = runner.invoke(app, ["scan", "--help"], env={"COLUMNS": "200"}) + assert "critical|high|medium|low|none|unknown" in plain(result.stdout) diff --git a/tests/test_report.py b/tests/test_report.py index f859f0e..9920eaa 100644 --- a/tests/test_report.py +++ b/tests/test_report.py @@ -8,6 +8,7 @@ import csv import json +from datetime import date from io import StringIO from pathlib import Path @@ -169,7 +170,7 @@ def test_json_document_shape() -> None: ) document = json.loads(render(report, OutputFormat.JSON)) - assert document["schema_version"] == "1.0" + assert document["schema_version"] == "1.1" assert document["tool"]["name"] == "icebergsca" assert document["summary"]["findings"] == 1 assert document["summary"]["by_severity"] == {"high": 1} @@ -352,3 +353,103 @@ def test_json_records_what_a_dependency_excludes() -> None: ) payload = json.loads(render(report, OutputFormat.JSON)) assert payload["dependencies"][0]["exclusions"] == ["com.lowagie:itext"] + + +def suppressed_finding(**overrides: object) -> Finding: + from icebergsca.core.models import Suppression + + defaults: dict[str, object] = { + "package": PackageRef(EcosystemId.PYPI, "pyyaml", "5.1"), + "advisory": make_advisory(), + "suppression": Suppression( + reason="Unreachable from our code paths", + expires=date(2026, 10, 27), + rule="GHSA-xxxx-yyyy-zzzz@pyyaml", + ), + } + return Finding(**{**defaults, **overrides}) # type: ignore[arg-type] + + +def test_a_fully_suppressed_report_never_reads_as_clean() -> None: + """The load-bearing one. + + If suppressed findings were filtered out of ``findings`` rather than marked, this + report would take the empty-findings branch and print "No known vulnerabilities" — + turning an accepted vulnerability into a clean bill of health. + """ + output = table_of( + { + "dependencies": (make_dependency(),), + "vulnerabilities_checked": True, + "findings": (suppressed_finding(),), + } + ) + assert "No known vulnerabilities" not in output + assert "accepted" in output + + +def test_the_summary_counts_accepted_findings_separately() -> None: + output = table_of( + { + "dependencies": (make_dependency(),), + "vulnerabilities_checked": True, + "findings": ( + suppressed_finding(), + Finding( + package=PackageRef(EcosystemId.PYPI, "flask", "2.0"), + advisory=make_advisory("GHSA-live"), + ), + ), + } + ) + assert "1 finding(s)" in output + assert "1 accepted" in output + + +def test_the_accepted_marker_is_explained_rather_than_left_bare() -> None: + output = table_of( + { + "dependencies": (make_dependency(),), + "vulnerabilities_checked": True, + "findings": (suppressed_finding(),), + } + ) + assert "ignore file" in output + + +def test_json_carries_the_suppression_on_the_finding_and_in_the_audit_trail() -> None: + report = make_report(vulnerabilities_checked=True, findings=(suppressed_finding(),)) + document = json.loads(render(report, OutputFormat.JSON)) + + entry = document["findings"][0]["suppression"] + assert entry["reason"] == "Unreachable from our code paths" + assert entry["expires"] == "2026-10-27" + + assert document["summary"]["findings"] == 0 + assert document["summary"]["suppressed"] == 1 + assert document["suppressed"][0]["rule"] == "GHSA-xxxx-yyyy-zzzz@pyyaml" + + +def test_json_suppression_is_null_when_nothing_accepted_the_finding() -> None: + """Always present, so reading it is never optional.""" + report = make_report( + vulnerabilities_checked=True, + findings=( + Finding( + package=PackageRef(EcosystemId.PYPI, "flask", "2.0"), + advisory=make_advisory(), + ), + ), + ) + document = json.loads(render(report, OutputFormat.JSON)) + assert document["findings"][0]["suppression"] is None + assert document["suppressed"] == [] + + +def test_csv_records_suppression_without_disturbing_existing_columns() -> None: + report = make_report(vulnerabilities_checked=True, findings=(suppressed_finding(),)) + text = render(report, OutputFormat.CSV) + assert text.startswith("severity,cvss_score,advisory_id") + row = next(csv.DictReader(StringIO(text))) + assert row["suppressed"] == "true" + assert row["suppression_reason"] == "Unreachable from our code paths" diff --git a/tests/test_report_formats.py b/tests/test_report_formats.py index 0c8139f..854f344 100644 --- a/tests/test_report_formats.py +++ b/tests/test_report_formats.py @@ -8,6 +8,7 @@ from __future__ import annotations import json +from datetime import date from functools import cache from pathlib import Path from typing import Any @@ -23,6 +24,7 @@ PackageRef, Severity, SeverityLevel, + Suppression, ) from icebergsca.report import OutputFormat, cyclonedx, render, sarif from tests.conftest import make_advisory, make_dependency, make_report @@ -80,6 +82,7 @@ def sample_report(**overrides: Any) -> Any: unscored = make_dependency( "left-pad", "1.0.0", ecosystem=EcosystemId.NPM, path="package.json" ) + accepted = make_dependency("jinja2", "3.1.2", path="requirements.txt", line=7) findings = ( Finding( @@ -104,10 +107,22 @@ def sample_report(**overrides: Any) -> Any: advisory=make_advisory("MAL-2024-1", level=None), introduced_by=(unscored,), ), + Finding( + package=accepted.ref, + advisory=make_advisory("GHSA-cccc", level=SeverityLevel.HIGH, score=7.5), + introduced_by=(accepted,), + # Suppressed findings are rendered, not dropped, so they must satisfy the + # official schemas like any other — which is what this entry buys. + suppression=Suppression( + reason="Unreachable: the affected parser is never called", + expires=date(2026, 10, 27), + rule="GHSA-cccc@jinja2", + ), + ), ) defaults: dict[str, Any] = { - "dependencies": (scored, transitive, unscored), + "dependencies": (scored, transitive, unscored, accepted), "findings": findings, "vulnerabilities_checked": True, "warnings": ("an example warning",), @@ -350,3 +365,33 @@ def test_severity_is_carried_through_from_a_vector() -> None: rating = document["vulnerabilities"][0]["ratings"][0] assert rating["method"] == "CVSSv31" assert rating["score"] == 7.5 + + +def test_sarif_marks_a_suppressed_result_rather_than_dropping_it() -> None: + """Dropped, GitHub would close the alert as fixed. Marked, it shows as dismissed.""" + document = sarif.to_dict(sample_report()) + result = result_for(document, "GHSA-cccc") + assert result["suppressions"] == [ + { + "kind": "external", + "status": "accepted", + "justification": "Unreachable: the affected parser is never called", + } + ] + + +def test_sarif_omits_suppressions_entirely_when_nothing_was_accepted() -> None: + assert "suppressions" not in result_for(sarif.to_dict(sample_report()), "GHSA-aaaa") + + +def test_cyclonedx_records_an_accepted_finding_without_claiming_it_is_unaffected() -> ( + None +): + """``state`` is a closed enum of security claims we cannot make from free text.""" + document = cyclonedx.to_dict(sample_report(), include_vulnerabilities=True) + entry = next(v for v in document["vulnerabilities"] if v["id"] == "GHSA-cccc") + assert "Unreachable" in entry["analysis"]["detail"] + assert "state" not in entry["analysis"] + + properties = {p["name"]: p["value"] for p in document["metadata"]["properties"]} + assert properties["icebergsca:suppressedCount"] == "1" diff --git a/tests/test_skill.py b/tests/test_skill.py index 6928c4e..d6544e8 100644 --- a/tests/test_skill.py +++ b/tests/test_skill.py @@ -107,6 +107,9 @@ def test_skill_name_matches_the_distribution() -> None: "vulnerabilities_checked", "unchecked_packages", "--format json", + # An accepted finding read as a fixed one is the second way to produce a + # false clean, so it belongs in the same guard as the first. + "suppressed", ], ) def test_skill_teaches_the_false_clean_checks(claim: str) -> None: diff --git a/tests/test_triage.py b/tests/test_triage.py new file mode 100644 index 0000000..71a526c --- /dev/null +++ b/tests/test_triage.py @@ -0,0 +1,247 @@ +"""The ignore file. + +The negative assertions carry the weight here. This is the one feature whose purpose is +to make findings disappear, so a rule that reaches further than it was written to reach +hides a real vulnerability — the failure this codebase is otherwise arranged to make +impossible. +""" + +from __future__ import annotations + +from datetime import date, timedelta + +import pytest + +from icebergsca.core.errors import ConfigError +from icebergsca.core.models import ( + Advisory, + EcosystemId, + Finding, + PackageRef, + Severity, + SeverityLevel, +) +from icebergsca.core.triage import DEFAULT_EXPIRY_DAYS, apply, parse_ignore_file + +TODAY = date(2026, 7, 29) + + +def finding( + advisory_id: str = "GHSA-aaaa", + name: str = "pyyaml", + *, + aliases: tuple[str, ...] = (), + ecosystem: EcosystemId = EcosystemId.PYPI, + version: str = "5.1", +) -> Finding: + return Finding( + package=PackageRef(ecosystem, name, version), + advisory=Advisory( + id=advisory_id, + modified="2026-01-01T00:00:00Z", + aliases=aliases, + severity=Severity(SeverityLevel.HIGH, 7.5), + ), + ) + + +def entry(**overrides: str) -> str: + fields = { + "advisory": "GHSA-aaaa", + "package": "pyyaml", + "reason": "Unreachable from our code paths", + } | overrides + body = "\n".join(f'{key} = "{value}"' for key, value in fields.items()) + return f"[[ignore]]\n{body}\n" + + +def rules(text: str, *, today: date = TODAY) -> tuple: + return parse_ignore_file(".icebergsca.toml", text, today=today) + + +# --------------------------------------------------------------------------- +# Parsing +# --------------------------------------------------------------------------- + + +def test_a_minimal_entry_parses() -> None: + (rule,) = rules(entry()) + assert rule.advisory == "GHSA-aaaa" + assert rule.package == "pyyaml" + assert rule.reason == "Unreachable from our code paths" + + +def test_no_ignore_file_content_is_no_rules() -> None: + assert rules("") == () + + +def test_expiry_defaults_rather_than_being_absent() -> None: + """An ignore file with no dates becomes permanent by accident. This is the guard.""" + (rule,) = rules(entry()) + assert rule.expires == TODAY + timedelta(days=DEFAULT_EXPIRY_DAYS) + + +def test_an_explicit_expiry_is_honoured() -> None: + (rule,) = rules(entry() + 'expires = "2026-12-25"\n') + assert rule.expires == date(2026, 12, 25) + + +def test_a_native_toml_date_is_accepted_as_well_as_a_quoted_one() -> None: + """Quoted and unquoted look identical in an editor; refusing one teaches nothing.""" + (rule,) = rules(entry() + "expires = 2026-12-25\n") + assert rule.expires == date(2026, 12, 25) + + +@pytest.mark.parametrize("missing", ["advisory", "package", "reason"]) +def test_a_missing_required_field_is_an_error(missing: str) -> None: + fields = { + "advisory": "GHSA-aaaa", + "package": "pyyaml", + "reason": "because", + } + del fields[missing] + text = "[[ignore]]\n" + "\n".join(f'{k} = "{v}"' for k, v in fields.items()) + with pytest.raises(ConfigError, match=missing): + rules(text) + + +def test_an_empty_reason_is_an_error() -> None: + """An entry with no justification is indistinguishable from a mistake later.""" + with pytest.raises(ConfigError, match="reason"): + rules(entry(reason=" ")) + + +def test_an_unknown_key_is_an_error_rather_than_being_skipped() -> None: + """The deliberate divergence from every other parser here. + + A misspelled key in a third-party manifest should be skipped. A misspelled key in + the file that decides which vulnerabilities you stop seeing should not. + """ + with pytest.raises(ConfigError, match="advisroy"): + rules('[[ignore]]\nadvisroy = "GHSA-aaaa"\npackage = "p"\nreason = "r"\n') + + +def test_an_unparseable_expiry_is_an_error() -> None: + with pytest.raises(ConfigError, match="expires"): + rules(entry() + 'expires = "next tuesday"\n') + + +def test_invalid_toml_names_the_file() -> None: + with pytest.raises(ConfigError, match="invalid TOML"): + rules("[[ignore]\n") + + +def test_a_duplicate_entry_is_an_error() -> None: + """Two entries for the same pair mean one silently does nothing.""" + with pytest.raises(ConfigError, match="duplicate"): + rules(entry() + entry()) + + +def test_ignore_must_be_a_list_of_tables() -> None: + with pytest.raises(ConfigError, match="list"): + rules('ignore = "GHSA-aaaa"\n') + + +# --------------------------------------------------------------------------- +# Matching — the half that must never over-reach +# --------------------------------------------------------------------------- + + +def test_a_matching_rule_suppresses() -> None: + marked, warnings = apply((finding(),), rules(entry()), today=TODAY) + assert marked[0].is_suppressed + assert marked[0].suppression is not None + assert marked[0].suppression.reason == "Unreachable from our code paths" + assert warnings == () + + +def test_a_cve_alias_matches_the_ghsa_it_belongs_to() -> None: + """Users are sent CVE numbers; OSV answers in GHSA. Both must work.""" + target = finding("GHSA-aaaa", aliases=("CVE-2026-1234",)) + marked, _ = apply((target,), rules(entry(advisory="CVE-2026-1234")), today=TODAY) + assert marked[0].is_suppressed + + +def test_an_unversioned_purl_matches_as_well_as_a_bare_name() -> None: + marked, _ = apply( + (finding(),), rules(entry(package="pkg:pypi/pyyaml")), today=TODAY + ) + assert marked[0].is_suppressed + + +def test_a_versioned_purl_pins_the_decision_to_that_version() -> None: + """Writing the version means the acceptance does not survive an upgrade.""" + accepted, _ = apply( + (finding(),), rules(entry(package="pkg:pypi/pyyaml@5.1")), today=TODAY + ) + assert accepted[0].is_suppressed + + upgraded, _ = apply( + (finding(version="5.4"),), + rules(entry(package="pkg:pypi/pyyaml@5.1")), + today=TODAY, + ) + assert not upgraded[0].is_suppressed + + +def test_a_rule_does_not_reach_a_longer_advisory_id() -> None: + """``GHSA-aaaa`` must not swallow ``GHSA-aaaa-bbbb``. No substrings, ever.""" + marked, _ = apply((finding("GHSA-aaaa-bbbb"),), rules(entry()), today=TODAY) + assert not marked[0].is_suppressed + + +def test_a_rule_does_not_reach_a_longer_package_name() -> None: + marked, _ = apply((finding(name="pyyaml-extra"),), rules(entry()), today=TODAY) + assert not marked[0].is_suppressed + + +def test_a_rule_for_one_package_does_not_suppress_the_same_advisory_elsewhere() -> None: + """The same CVE often affects several packages. A rule accepts one of them.""" + same_advisory = finding("GHSA-aaaa", name="other-package") + marked, _ = apply((finding(), same_advisory), rules(entry()), today=TODAY) + assert marked[0].is_suppressed + assert not marked[1].is_suppressed + + +def test_a_rule_for_one_advisory_does_not_suppress_another_on_the_same_package() -> ( + None +): + marked, _ = apply((finding("GHSA-bbbb"),), rules(entry()), today=TODAY) + assert not marked[0].is_suppressed + + +def test_matching_is_case_insensitive_on_the_advisory_id() -> None: + marked, _ = apply((finding(),), rules(entry(advisory="ghsa-aaaa")), today=TODAY) + assert marked[0].is_suppressed + + +# --------------------------------------------------------------------------- +# Rules that do no work +# --------------------------------------------------------------------------- + + +def test_an_expired_entry_stops_suppressing_and_says_so() -> None: + """Expiry surfaces the finding again. It never fails the scan.""" + text = entry() + 'expires = "2026-01-01"\n' + marked, warnings = apply((finding(),), rules(text), today=TODAY) + assert not marked[0].is_suppressed + assert any("expired" in w for w in warnings) + + +def test_an_entry_expiring_today_still_applies() -> None: + text = entry() + f'expires = "{TODAY.isoformat()}"\n' + marked, _ = apply((finding(),), rules(text), today=TODAY) + assert marked[0].is_suppressed + + +def test_a_rule_matching_nothing_is_reported() -> None: + """A typo suppresses nothing and looks exactly like a rule that is working.""" + _, warnings = apply((finding("GHSA-bbbb"),), rules(entry()), today=TODAY) + assert any("matched no finding" in w for w in warnings) + + +def test_no_rules_means_no_warnings_and_no_changes() -> None: + original = (finding(),) + marked, warnings = apply(original, (), today=TODAY) + assert marked == original + assert warnings == () diff --git a/website/docs/cli.md b/website/docs/cli.md index 78e6073..d3f20e5 100644 --- a/website/docs/cli.md +++ b/website/docs/cli.md @@ -50,6 +50,29 @@ icebergsca scan ./requirements.txt # a single file Logs go to stderr and the report to stdout, so piping the report is always safe. +### Gating and triage + +| Flag | Effect | +|---|---| +| `--fail-on ` | Exit `3` when an unsuppressed finding is at or above this severity. An incomplete scan still exits `1`, whatever it found | +| `--ignore-file ` | Accepted findings. Defaults to `.icebergsca.toml` beside the scan target when it exists | + +`.icebergsca.toml` records findings somebody has assessed and accepted, so that gating is safe +to leave switched on rather than being disabled the first week an unfixable advisory appears: + +```toml +[[ignore]] +advisory = "GHSA-6757-jp84-gxfx" # or the CVE — aliases are matched too +package = "pyyaml" # bare name, or a purl, optionally versioned +reason = "Unreachable: we never call yaml.load on untrusted input" +expires = 2026-10-27 # optional; defaults to 90 days out +``` + +`reason` is required, matching is exact rather than pattern-based, and an entry that expires +stops suppressing rather than failing the scan. Nothing is hidden: the finding stays in the +report marked `accepted`, `summary.suppressed` counts it, and SARIF reports it as dismissed +rather than fixed. A rule matching nothing produces a warning. + ### Network and cache | Flag | Effect | @@ -94,5 +117,7 @@ pure waste. | `0` | Scan completed. Vulnerabilities may have been found — that is not an error | | `1` | Scan failed, or completed only partially | | `2` | Usage error: bad flag, unknown format | +| `3` | Only with `--fail-on`: an unsuppressed finding met the threshold | -Findings never change the exit code. See [Output and CI](output.md#exit-codes). +Findings never change the exit code unless you ask with `--fail-on`, and even then +under a code of their own. See [Output and CI](output.md#exit-codes). diff --git a/website/docs/output.md b/website/docs/output.md index 733ab41..2774e4d 100644 --- a/website/docs/output.md +++ b/website/docs/output.md @@ -29,16 +29,17 @@ icebergsca scan . --format json --output report.json ```json { - "schema_version": "1.0", + "schema_version": "1.1", "tool": { "name": "icebergsca", "version": "0.1.0" }, "scan": { "root": "...", "status": "ok", "vulnerabilities_checked": true, "complete": true }, - "summary": { "dependencies": 21, "packages": 21, "findings": 12, + "summary": { "dependencies": 21, "packages": 21, "findings": 12, "suppressed": 2, "by_severity": { "critical": 3, "high": 4, "medium": 4, "low": 1 }, "unchecked_packages": 0, "skipped_files": 0 }, "manifests": [ ... ], "dependencies": [ ... ], "findings": [ ... ], + "suppressed": [ ... ], "unchecked_packages": [ ... ], "skipped": [ ... ], "warnings": [ ... ] @@ -58,6 +59,10 @@ Three things are easy to get wrong: - **`dependencies[].exclusions` explains an absence.** A coordinate listed there was removed from the graph on purpose, which is the one case where something missing is not something overlooked. +- **`summary.findings` counts only what is outstanding.** Findings accepted in + the ignore file are counted by `summary.suppressed` and still appear in the + `findings` array, each with a `suppression` object. An accepted finding is a + decision, not a fix. The last three arrays — `unchecked_packages`, `skipped` and `warnings` — are the report's account of what it could not do. They are the difference between @@ -112,10 +117,13 @@ one as evidence that anything was checked. | `0` | Scan completed. Vulnerabilities may have been found — that is not an error | Read `summary.findings` to decide what to do | | `1` | Scan failed, or completed only partially — some files or packages went unchecked | Fail the job; the result is not trustworthy | | `2` | Usage error: bad flag, unknown format (Click's reserved code) | Fix the invocation | +| `3` | `--fail-on` threshold met by an unsuppressed finding | Fail the job; the scan worked, the result did not pass | -Findings deliberately never change the exit code. A scanner that exits non-zero -on findings sooner or later gets `|| true` appended, at which point genuine tool -failures go unnoticed too. +Findings never change the exit code unless you ask for it with `--fail-on`, and +even then under a code of their own. A scanner that exits non-zero on findings by +default sooner or later gets `|| true` appended, at which point genuine tool +failures go unnoticed too — and a pipeline that cannot tell `1` from `3` ends up +treating both as noise. !!! note "Exit 2 is usage, exit 1 is scan failure" @@ -124,10 +132,15 @@ failures go unnoticed too. ## Gating a build on severity -A built-in `--fail-on ` is planned; the report model already carries -everything it needs. Until then, read the JSON — and check `scan.complete` -**first**, because gating on findings alone would pass a scan that failed to -check anything: +```bash +icebergsca scan . --fail-on critical +``` + +The gate checks completeness before severity, so it cannot pass a scan that failed +to look anything up, and it ignores findings accepted in `.icebergsca.toml`. + +For anything more specific, read the JSON — and check `scan.complete` **first**, +because gating on findings alone would pass a scan that failed to check anything: ```bash icebergsca scan . --format json --output report.json