Repository navigation
Add the rejected outcome to base-security-audit. - #5
Merged
Merged
Conversation
… investigation rules out.
samuel-williams-shopify
force-pushed
the
rejected-outcome
branch
from
November 5, 2026 03:51
64e6f39 to
cfd9449
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
base-security-audithas 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:
// 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.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.mdandreleases.mdupdated.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_trailerwould report it again.