mirror of
https://github.com/usestrix/strix.git
synced 2026-10-04 02:33:47 +00:00
Resolve PR Comments
This commit is contained in:
parent
3cd6c93fa0
commit
b11228192d
7 changed files with 46 additions and 21 deletions
|
|
@ -23,11 +23,11 @@ strix -n --target ./app --scan-mode quick --scope-mode diff --diff-base origin/m
|
|||
|
||||
| Code | Meaning |
|
||||
|------|---------|
|
||||
| 0 | No vulnerabilities found |
|
||||
| 0 | No vulnerabilities found (with `--fail-on`, none at or above the threshold) |
|
||||
| 1 | Execution error |
|
||||
| 2 | Vulnerabilities found |
|
||||
| 2 | Vulnerabilities found (with `--fail-on`, at least one at or above the threshold) |
|
||||
|
||||
To fail only on serious findings, pass `--fail-on` with a minimum severity. Lower findings are still reported but exit `0`:
|
||||
To fail only on serious findings, pass `--fail-on` with a minimum severity. Lower findings are still reported but exit `0`, so a passing job does not mean the report is empty. The threshold only applies to scans that completed; a scan that stopped early fails on any finding:
|
||||
|
||||
```bash
|
||||
strix -n --target ./app --scan-mode quick --fail-on high
|
||||
|
|
|
|||
|
|
@ -46,10 +46,10 @@ The workflow fails when vulnerabilities are found:
|
|||
|
||||
| Code | Result |
|
||||
|------|--------|
|
||||
| 0 | Pass — No vulnerabilities |
|
||||
| 0 | Pass — No vulnerabilities (with `--fail-on`, none at or above the threshold) |
|
||||
| 2 | Fail — Vulnerabilities found |
|
||||
|
||||
Add `--fail-on high` (or `critical`, `medium`, `low`) to fail only on findings at or above that severity. Lower findings still appear in the report.
|
||||
Add `--fail-on high` (or `critical`, `medium`, `low`) to fail only on findings at or above that severity. Lower findings still appear in the report, so a passing run is not necessarily finding-free. A scan that stops before completing fails on any finding.
|
||||
|
||||
## Scan Modes for CI
|
||||
|
||||
|
|
|
|||
|
|
@ -66,6 +66,10 @@ strix (--target <target> | --target-list <path>) [options]
|
|||
threshold are still written to every report artifact. A finding whose
|
||||
severity Strix does not recognize always counts, so the gate never passes on a
|
||||
value it cannot rank. Omit the flag to exit `2` on any finding.
|
||||
|
||||
The threshold only applies to a scan that completed. If the run stopped
|
||||
early (for example at `--max-budget` or `--max-turns`), any finding exits `2`,
|
||||
because the scan may not have reached its more serious findings.
|
||||
</ParamField>
|
||||
|
||||
<ParamField path="--config" type="string">
|
||||
|
|
@ -170,6 +174,6 @@ strix --target https://app.com --workspace-file ./openapi.yaml:specs/openapi.yam
|
|||
|
||||
| Code | Meaning |
|
||||
|------|---------|
|
||||
| 0 | Scan completed successfully (interactive mode always exits `0`; in headless mode, `0` means no vulnerabilities were found) |
|
||||
| 0 | Scan completed successfully (interactive mode always exits `0`; in headless mode, `0` means no vulnerabilities were found, or with `--fail-on`, none at or above the threshold; lower findings are still in the report) |
|
||||
| 1 | A fatal error occurred before or during the scan (e.g. missing environment variables, Docker unavailable, invalid config file, diff-scope resolution failure, or an unhandled error) |
|
||||
| 2 | Vulnerabilities found (headless mode only; with `--fail-on`, only findings at or above that severity) |
|
||||
| 2 | Vulnerabilities found (headless mode only; with `--fail-on`, at least one finding at or above that severity, or any finding if the scan stopped before completing) |
|
||||
|
|
|
|||
|
|
@ -67,7 +67,7 @@ Then tell the user to add two repository secrets: `STRIX_LLM` (model id, for exa
|
|||
|
||||
Notes:
|
||||
- In CI/headless runs Strix automatically scopes to the PR's changed files (`--scope-mode auto`). If diff resolution fails, keep `fetch-depth: 0` or set `--diff-base` to the PR's actual base branch — use `origin/${{ github.base_ref }}` in GitHub Actions rather than a hard-coded `origin/main`, since repos use different default branches.
|
||||
- Exit codes: `0` pass, `2` vulnerabilities found (fails the job), `1` setup error. Add `--fail-on high` to fail only on high/critical findings; lower ones are still reported.
|
||||
- Exit codes: `0` pass, `2` vulnerabilities found (fails the job), `1` setup error. Add `--fail-on high` to fail only on high/critical findings; lower ones are still reported, so a passing job is not a finding-free report. A scan that stops before completing fails on any finding.
|
||||
- The runner needs Docker (default GitHub-hosted Ubuntu runners have it).
|
||||
- **Size the budget so the scan completes — do not let it fail open.** A `0` exit means "no validated vulnerabilities in what was analyzed"; if `--max-budget` is hit before the diff is fully covered, the scan wraps up early and can still exit `0`. The "Fail unless the scan completed" step above narrows the gap: `strix_runs/<run>/run.json` is `"stopped"` when the scan was cut off at the hard budget limit without a final report. It is not a complete guard — the agents get graduated wrap-up warnings before that limit, and a run that wraps up on a warning still calls `finish_scan` and records `"completed"` with partial coverage. So keep that step in any pipeline that gates merges **and** give the scan real headroom (compare `run.json`'s `llm_usage.cost` against `--max-budget`; if it ran right up to the cap, raise it). For a `quick` diff-scoped PR scan `--max-budget 10` is usually ample, raise it for large diffs.
|
||||
|
||||
|
|
|
|||
|
|
@ -94,7 +94,7 @@ Key flags:
|
|||
| `--workspace-file PATH[:DEST]` | Copy a file from this machine into `/workspace` before the scan, for a wordlist, a spec, or notes. Repeatable. |
|
||||
| `--max-budget USD` | Hard LLM spend cap; scan wraps up cleanly at the limit. |
|
||||
| `--max-turns N` | Per-agent turn cap (default 500). |
|
||||
| `--fail-on SEVERITY` | Headless only: exit `2` only for findings at or above `critical`/`high`/`medium`/`low`/`info`. Default: any finding. |
|
||||
| `--fail-on SEVERITY` | Headless only: exit `2` only for findings at or above `critical`/`high`/`medium`/`low`/`info` on a completed scan (a stopped scan fails on any finding). Default: any finding. |
|
||||
| `--resume RUN_NAME` | Resume a prior run from `strix_runs/`, with its agent history and targets. Cannot be combined with `-t`. |
|
||||
| `--scope-mode` | For code targets: `auto` (diff-scope in CI/headless), `diff` (force changed files only), `full` (whole tree). |
|
||||
| `--diff-base REF` | Branch or commit that `diff` scope compares against. Defaults to the repo's default branch. |
|
||||
|
|
@ -103,9 +103,9 @@ Scans take minutes (`quick`) to hours (`deep`). Run them in the background and p
|
|||
|
||||
### Exit codes (headless)
|
||||
|
||||
- `0` — finished with no validated vulnerabilities **in what was analyzed**
|
||||
- `0` — finished with no validated vulnerabilities **in what was analyzed** (with `--fail-on`, none at or above the threshold; lower ones are still in the artifacts)
|
||||
- `1` — fatal error (missing env vars, Docker down, bad config)
|
||||
- `2` — vulnerabilities found (with `--fail-on`, only at or above that severity; lower findings still land in the artifacts)
|
||||
- `2` — vulnerabilities found (with `--fail-on`, at least one at or above that severity, or any finding if the scan stopped early)
|
||||
|
||||
A `0` is not proof of full coverage: if `--max-budget`/`--max-turns` is reached before the scan completes, it wraps up early and still exits `0`. When you need assurance the scan finished, give it enough budget and check `strix_runs/<run>/run.json`: a hard budget stop leaves `status: "stopped"`, but an agent that wrapped up early on a budget *warning* still calls `finish_scan` and records `"completed"` — so also sanity-check the run's cost against `--max-budget` and the report's stated coverage before treating a clean result as full coverage.
|
||||
|
||||
|
|
|
|||
|
|
@ -316,17 +316,23 @@ def display_completion_message(args: argparse.Namespace, results_path: Path) ->
|
|||
notify_update(console)
|
||||
|
||||
|
||||
def findings_fail_build(reports: list[dict[str, Any]], fail_on: str | None) -> bool:
|
||||
def findings_fail_build(
|
||||
reports: list[dict[str, Any]], fail_on: str | None, *, completed: bool
|
||||
) -> bool:
|
||||
"""Whether headless findings should exit 2 under the ``--fail-on`` threshold.
|
||||
|
||||
With no threshold any finding fails. Otherwise a finding fails when its
|
||||
severity is at or above the threshold. A severity outside the known scale
|
||||
fails too, so a gate never passes on a value it cannot rank. ``none`` is a
|
||||
known level below ``info`` and only fails without a threshold.
|
||||
|
||||
The threshold only applies to a completed run. A run that stopped early
|
||||
(budget, turn limit, interrupt) may not have reached its serious findings,
|
||||
so any finding fails it, as without ``--fail-on``.
|
||||
"""
|
||||
if not reports:
|
||||
return False
|
||||
if fail_on is None:
|
||||
if fail_on is None or not completed:
|
||||
return True
|
||||
threshold = FAIL_ON_SEVERITIES.index(fail_on)
|
||||
for report in reports:
|
||||
|
|
@ -526,7 +532,11 @@ def main() -> None:
|
|||
|
||||
if args.non_interactive:
|
||||
report_state = get_global_report_state()
|
||||
if report_state and findings_fail_build(report_state.vulnerability_reports, args.fail_on):
|
||||
if report_state and findings_fail_build(
|
||||
report_state.vulnerability_reports,
|
||||
args.fail_on,
|
||||
completed=report_state.run_record.get("status") == "completed",
|
||||
):
|
||||
sys.exit(2)
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -26,13 +26,13 @@ def _reports(*severities: str | None) -> list[dict[str, Any]]:
|
|||
|
||||
|
||||
def test_no_findings_never_fail() -> None:
|
||||
assert not cli_main.findings_fail_build([], None)
|
||||
assert not cli_main.findings_fail_build([], "info")
|
||||
assert not cli_main.findings_fail_build([], None, completed=True)
|
||||
assert not cli_main.findings_fail_build([], "info", completed=True)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("severity", ["critical", "high", "medium", "low", "info", "none"])
|
||||
def test_without_threshold_any_finding_fails(severity: str) -> None:
|
||||
assert cli_main.findings_fail_build(_reports(severity), None)
|
||||
assert cli_main.findings_fail_build(_reports(severity), None, completed=True)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
|
|
@ -52,20 +52,31 @@ def test_without_threshold_any_finding_fails(severity: str) -> None:
|
|||
],
|
||||
)
|
||||
def test_threshold_compares_severity(fail_on: str, severity: str, expected: bool) -> None:
|
||||
assert cli_main.findings_fail_build(_reports(severity), fail_on) is expected
|
||||
assert cli_main.findings_fail_build(_reports(severity), fail_on, completed=True) is expected
|
||||
|
||||
|
||||
def test_one_finding_at_threshold_fails_a_mixed_run() -> None:
|
||||
assert cli_main.findings_fail_build(_reports("info", "low", "high"), "high")
|
||||
assert cli_main.findings_fail_build(_reports("info", "low", "high"), "high", completed=True)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("severity", ["severe", "", None])
|
||||
def test_unrecognized_severity_fails_closed(severity: str | None) -> None:
|
||||
assert cli_main.findings_fail_build(_reports(severity), "critical")
|
||||
assert cli_main.findings_fail_build(_reports(severity), "critical", completed=True)
|
||||
|
||||
|
||||
def test_none_severity_passes_any_threshold() -> None:
|
||||
assert not cli_main.findings_fail_build(_reports("none"), "info")
|
||||
assert not cli_main.findings_fail_build(_reports("none"), "info", completed=True)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("severity", ["medium", "low", "info", "none"])
|
||||
def test_threshold_ignored_when_run_did_not_complete(severity: str) -> None:
|
||||
# A run stopped early (budget, turn limit) may not have reached its serious
|
||||
# findings, so the threshold must not let it pass.
|
||||
assert cli_main.findings_fail_build(_reports(severity), "high", completed=False)
|
||||
|
||||
|
||||
def test_incomplete_run_without_findings_does_not_fail() -> None:
|
||||
assert not cli_main.findings_fail_build([], "high", completed=False)
|
||||
|
||||
|
||||
def test_parse_fail_on_is_case_insensitive(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue