Gating a pull request on what it introduced

In plain terms#

Scan the branch behind a pull request and you get everything wrong with that branch. Almost all of it was wrong before the pull request existed, and the author is being shown a backlog they did not create, at the moment they are least able to act on it.

A diff compares two scans, the branch against what it is merging into, and reports the difference. That difference is the part the change is answerable for.

Why it matters#

The reason this is worth building rather than approximating is adoption. A gate that reads every open finding fails the first pull request it sees, and every one after that, because the backlog was already there. Teams respond the two ways available to them:

A gate on the delta fails only the pull request that introduced something, which is a bar a team can actually hold, on the day they turn it on, with the backlog untouched.

Outcomes#

Every finding in a diff has one outcome. New, fixed and unchanged are somebody's work; accepted and unaccepted are somebody's decision, and the two are worth keeping apart. Moved is the same finding reported under a different component, which is what dividing a repository into components does to every finding it held.

Accepted is a finding somebody excused rather than repaired: an exclusion was added, or the finding arrived already covered by one. Unaccepted is the reverse. An exclusion was removed or reached its expiry date, so the finding counts again.

Accepting a risk is not fixing it, and the two are counted apart for that reason. The first is a decision worth a reviewer's attention and the second is work somebody did. A tool that reports them as one number lets a team clear a backlog by writing exclusions and shows the same green as a team that fixed things.

What a gate should read#

A gate should read only what the change introduced, and nothing else in the report should be able to trip it. An accepted finding does not, which is the point of accepting it. Neither does an unaccepted one, and that is the less obvious half: a gate exists to stop a change bringing something in, and an acceptance reaching its expiry is a decision to make again rather than a regression. Failing the merge for it punishes whoever happened to open a pull request that week.

So the verdict and the report diverge, and both are correct:

DRAUGR DIFF  pass  1 unaccepted  28 unchanged

 unaccepted      1 P1 0 P2 0 P3 0 P4
 unchanged       13 P1 13 P2 1 P3 0 P4
 still accepted  1 P1 0 P2 0 P3 0 P4

Gate: fails on any P1 this change introduces.

CHANGED  1, by priority
  Change        Priority  Severity  Rule            Scanner  Location                Upgrade
  ! unaccepted  P1        critical  CVE-2019-20477  trivy    app/requirements.txt:4  PyYAML 5.1 → 5.2
              command execution through python/object/apply constructor in FullLoader

A critical finding, named in full, on a run that passes. The gate is answering the question it was asked, and the unaccepted row is telling somebody that an acceptance they made a year ago has run out. The still accepted strip counts the findings excluded in both scans.

What counts as the same finding#

Two scans produce two lists, and matching them is where this quietly goes wrong. A finding matched too strictly appears as one thing fixed and one thing new, so a diff of a change that touched nothing relevant fills up with both.

Line numbers are the usual culprit. Add an import at the top of a file and every finding below it moves, which is not a security event. Severity is the other: a CVE rescored overnight is the same issue with a different number attached.

Draugr treats two findings as the same when either of two keys matches, and neither key includes the line number or the severity:

The component is part of both keys because the same flaw in two components is two findings. A finding matched by neither key is tried again with the component left out, and a pair found that way is moved: the same repository, file, tool and rule, now under a different component. Splitting one component into three with paths moves every finding it held, and a pull request that does only that passes a gate on new findings. A moved finding whose priority changed is listed on its own row, because the component it moved to declares a different exposure or criticality.

Common pitfalls#

Comparing a working tree against itself. A repository is cloned before it is scanned, whether it was given as a URL or a local path, so each scan describes a commit rather than whatever is saved on disk. In CI that is the intent. Locally it means scanning, editing and re-scanning produces two identical files and an empty diff. Commit between the two scans.

Losing the base. A diff needs something to compare against, so wiring it by hand means scanning the default branch on every push and keeping the result where the pull-request job can fetch it. Forget the second half and the comparison silently has nothing to compare to. It is also the part most worth handing to a first-party integration, which keeps both scans and the artifact between them out of your pipeline entirely.

Reading the diff as a posture report. It says what one change did. The backlog it declined to mention is still there, and something has to look at the whole picture on a schedule.

How Draugr fits#

draugr diff takes the two results.sarif files a scan writes and classifies everything the change touched into one table, ranked by priority:

draugr diff base/results.sarif head/results.sarif --fail-on-new P1

A scan can also compare itself with a base:

draugr scan --base base/results.sarif --fail-on-new P1

That run is judged on what the change introduced. The console prints the change as draugr diff does, and the report.json the scan writes records the base it compared against, the new findings by band and the verdict. The reference covers the rest.

The threshold can also live in the descriptor as config.gate.failOnNew, reviewed with the rest of the policy, and both commands read it. The GitHub Action's fail-on-new input is empty by default, so the descriptor applies there. The GitLab and Azure templates default their input to P1, which takes precedence until you empty it. --fail-on-new overrides the descriptor for one run.

On most pipelines there is nothing to wire#

Running the command yourself is the general case rather than the expected one. Both first-party integrations already do the whole loop:

The two differ in what they add rather than in how much work they save. GitHub gets native code scanning, where a pull request shows the findings it introduced and nothing else. GitLab has the gate on by default, where on GitHub fail-on-new is a line you uncomment, and its template also handles something invisible until it bites: GitLab clones twenty commits deep, so the merge base a comparison needs is frequently not in the checkout, and the template fetches that one commit.

Gate PRs on new findings is the how-to for all three, and every flag is in the draugr diff reference.

The band it gates on is not severity. Draugr ranks every finding into P1 through P4 from the severity together with the component's declared exposure and criticality, so --fail-on-new P1 stops a change that puts a critical flaw on an internet-facing service without also stopping one that puts the same flaw in a batch job nothing reaches. The prioritization guide covers how the band is computed.

The loop this closes#

An entry in an ignore file stays until somebody deletes it, and the whole problem with ignore files is that nobody ever does. An exclusion with an expiry stops suppressing on its own, and the diff is what puts that in front of somebody.

Keep learning#

• Risk acceptance and exceptions covers what a defensible exclusion carries and why suppressing beats deleting.
• Vulnerability prioritization covers the band the gate reads.
• Shift left, and what it cost covers where a gate belongs in a pipeline that already has several.
• SARIF covers the format both sides of the diff are written in.