Skip to content

Add the rejected outcome to base-security-audit. - #5

Merged
samuel-williams-shopify merged 1 commit into
mainfrom
rejected-outcome
Nov 5, 2026
Merged

samuel-williams-shopify merged 1 commit into
mainfrom
rejected-outcome

Conversation

@samuel-williams-shopify

Copy link
Copy Markdown
Contributor

base-security-audit has four outcomes: advisory, fixed, accepted, and tracked. A finding that looks plausible but is ruled out on investigation fits none of them. "Classify findings" says a defect without attacker leverage is "not an audit finding … leave it out", so the rejection is recorded nowhere, the "already known" check cannot find it, and the next reviewer re-derives the same finding and repeats the investigation. That is the repeated-review cost the readme says the records exist to prevent.

A rejection also rests on reasoning that can stop holding, so it deserves the same durable note as an accepted risk, labelled honestly as "not a weakness" rather than "tolerated weakness".

This adds a fifth outcome, Rejected, and threads it through:

  • Decide each outcome: defines rejected, requires a // SECURITY: comment stating the reasoning that rules the finding out, and distinguishes it from accepted (a weakness whose risk is tolerated under an assumption). Rejecting is listed as a maintainer decision alongside accepting.
  • Classify findings: a plausible finding ruled out only after investigation is recorded as rejected rather than left out; the "already known" check covers rejected findings and re-reports only when the reasoning no longer holds.
  • Record the audit: adds the rejected, {reason} outcome form and an example line; a rejected finding keeps the severity it would have had if real, so the record shows what was ruled out.
  • design/record-format.md: notes that adding an outcome form is additive to existing records, but tools that recognize the forms must be updated first.
  • bake/tests/audit.rs: accepts the new form.
  • readme.md and releases.md updated.

Motivating case: socketry/protocol-http1#64 and socketry/async-http#259 were opened for a finding ("received trailer fields are merged into the header section") and closed after investigation showed it is not a weakness, because a field is only accepted as a trailer when its policy permits it. Without this outcome there is no place to record that, and the next audit of Body::Chunked#read_trailer would report it again.

@samuel-williams-shopify
samuel-williams-shopify merged commit 78f84e0 into main Nov 5, 2026
4 checks passed
@samuel-williams-shopify
samuel-williams-shopify deleted the rejected-outcome branch November 5, 2026 03:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant