Guide 051 pillar

How to add security checks to pull requests without blocking every merge

Design pull-request security checks around honest results, revision binding and a narrow merge policy, with deeper work assigned to a separate assessment lane.

Put a small, reliable security decision in the pull request (PR), then give deeper investigation its own clock and owner. Define which new signals require a correction or human review before merge, which are advisory, and what happens when a required check cannot finish. Do not turn every scanner alert into a confirmed vulnerability or every timeout into a pass. The balance changes for a sensitive change, such as authorization or payment code: a plausible high-impact claim may justify a temporary hold while a person checks it. A documentation change should not wait for the same investigation. This is a workflow policy to design and measure, not a universal severity threshold.

NIST's Secure Software Development Framework calls for integrating security practices into development and reviewing or analyzing code for vulnerabilities. It does not prescribe that every security tool must become a blocking PR job. The engineering manager's task is to make the merge decision explicit, bounded and reviewable.

Separate the signal from the merge decision

A scan result answers only what that tool examined at a particular revision. A merge policy answers whether the team has enough evidence to ship this change under its own risk rules. Keep these layers separate:

Layer Question Typical output
Check execution Did the intended job run on this revision and declared scope? Completed, failed to run, partial or timed out.
Security signal What possible issue did it identify? Rule match, dependency advisory, secret candidate or source-supported claim.
Review Is the claim applicable, and what would disprove it? Supported, rejected or still uncertain, with evidence.
Merge decision May this exact revision merge now? Allow, hold for fix/review, or authorized time-bound exception.

If the scanner says “critical” while the relevant route is unused, the title alone cannot decide the merge. If it reports nothing after examining only one directory, the absence of an alert cannot decide it either. GitHub's code scanning documentation explains that PR annotations depend on configuration and on the alert's lines appearing in the PR diff. A clean PR annotation view is therefore narrower than a claim that the whole application was reviewed.

Build the policy around three paths. Routine changes get quick checks with clear ownership. Sensitive changes or strong new signals get a short human review before merge. Longer, broader assessments run on the default branch or a schedule and create owned work without holding an unrelated PR open. A later full assessment can still discover a problem in code that previously merged; its finding needs a route back into the backlog.

Illustrative case: a billing download change

Illustrative scenario, not a customer result: Bluefin Billing has 40 engineers and one security engineer. Its PR 248 changes an invoice download handler, a shared authorization helper and a user guide. An ordinary account user must only download invoices owned by that account. The team already has a known dependency alert in another service, and its full code assessment takes much longer than a normal PR review.

At first, Bluefin makes every security job required. PR 248 waits on the broad scan, and a docs-only PR later waits on the same job. The manager changes the policy, not the standard of proof. A quick check still runs for every PR; it records revision and applicability, scans configured changed material and returns a visible result. The invoice handler and shared authorization helper trigger a human-owned review because they change a trust boundary. The broader scan runs after merge on a declared schedule and is linked to the deployed revision. A long-running scan's result never silently substitutes for the required PR decision.

Here is Bluefin's draft merge matrix:

PR state at the exact head revision Decision Owner and evidence
Required checks completed; no new blocked signal; no sensitive-path trigger Merge under normal code review. Platform records check run IDs and scope; author keeps normal reviewer approval.
New high-impact signal on a reachable path, or authorization logic changed Hold for a bounded AppSec or designated senior-engineer review. Reviewer records actor, entry, control, effect and contrary evidence; manager sees the decision deadline.
Candidate proves false for this revision Merge after reasoned dismissal; keep the alert and counterevidence. Security reviewer cites the specific guard or deployment fact.
Required check incomplete, unavailable or skipped Treat result as inconclusive; use the documented manual fallback or hold. Platform identifies failure and reruns if safe; authorized reviewer signs the fallback scope.
Verified issue remains but urgent release is justified by an exception Follow the exception process with an owner, controls and expiry. Named risk owner accepts the temporary exposure; engineering records follow-up and reassessment.

The existing dependency alert in another service remains open work. It does not automatically fail PR 248 merely because the scanner repeats it. If PR 248 changes the vulnerable dependency's reachability or exposes it to the invoice route, the team must re-evaluate it as part of this PR. “Old alert” is a scheduling fact, not proof of safety.

Put the policy into the repository workflow

1. Inventory the current checks. For the last representative set of PRs, list each job's trigger, runtime, failure reasons, output, repository permissions and owner. Identify which jobs find new issues, which repeat the default-branch backlog and which run only because their trigger matches every file. Record the baseline before changing required checks.

2. Write the blocking rule before configuring a tool. State the narrow conditions that deserve a merge hold: for example, a verified new cross-account data exposure; a high-confidence secret committed in the diff; or a sensitive-path change requiring a named reviewer. State whether a scanner candidate is a hold for review or a fail for a confirmed issue. Define the security owner and maximum review window. Different repositories may need different thresholds, but the same class of decision should be consistent within a repository.

3. Decide where each analysis runs. The fast PR lane might include configuration checks and tools that finish within the team's chosen wait budget. The deeper lane might inspect complete service relationships, historical code or a large monorepo partition. GitHub's CodeQL workflow options support PR, push and scheduled triggers; the documentation describes scheduled scans as a way to surface problems discovered even when developers are not actively changing code. Use that distinction as a planning model even if the team uses another provider.

4. Make required status checks honest. If using GitHub, rulesets can require checks or code scanning results, and code scanning merge protection lets eligible repositories select tool and alert thresholds. Check the current plan and permissions before assuming those controls are available. Configure a required policy job that actually reports on every PR for which it is required. GitHub warns that a workflow skipped by path or branch filtering can leave a required check pending and block merge; path filters decide whether a workflow runs, not whether a particular file was analyzed (trigger documentation).

The policy job itself needs a trust boundary. Review changes to its workflow and policy inputs, and give it only the permissions it needs. Do not make an always-reporting check by running untrusted PR code in a privileged workflow; GitHub's secure-use guidance describes the risks of privileged triggers with untrusted checkout.

5. Give incomplete work its own result. Define three business meanings even if a CI provider shows only a binary check: pass means applicable required work completed and found no condition requiring a hold; fail/hold means a prohibited condition or pending reviewer decision; inconclusive means the check did not finish or the needed scope was missing. Map inconclusive to a visible hold or documented manual route. Do not let an error-handling script change it to success just to reduce friction.

6. Roll out in observation mode, then require a narrow slice. During a declared observation period, compare docs, dependency, feature and sensitive-path PRs. Check revision binding, actionable signals and review timeliness before requiring the rule. Keep ownership of the broader scan and its backlog.

Follow one pull request through changing revisions

Bluefin's matrix needs evidence tied to the candidate it is deciding. The following event trace is a proposed policy exercise, not an implemented GitHub integration. H1, H2 and H3 stand for immutable revisions. Record the actual analyzed SHA as well as the PR head: when a provider analyzes a generated merge commit, the record must bind that result to the intended head and base rather than pretending they are the same object.

Event Evidence available Policy result
09:00, H1 Ordinary handler cleanup; applicable quick checks complete; no sensitive change identified Allow H1 under ordinary review
09:10, H2 Author adds authorization-helper change; H1's green result still exists in history Hold H2 for fresh checks and sensitive-path review; H1 cannot authorize it
09:22, H2 Required analysis times out before shared helper examination Inconclusive, still held; no “zero findings” conclusion
09:40, H2 Reviewer obtains handler, mandatory storage wrapper and effective route binding; fallback covers the required question Manual fallback may authorize H2 only if this policy and repository enforcement permit that route
09:45, H3 Author replaces the mandatory wrapper with a raw storage call to fix a compatibility issue Invalidate the H2 decision; re-open the precise account-binding question

At H2, the supplied wrapper always derives the account from authenticated context and checks the requested invoice before returning content. The reviewer records that counterevidence and rejects the suspected gap for H2. The fallback record identifies the timed-out tool, the uncompleted automated scope, the independently inspected paths and the authorizing person. It does not report the scanner as successful.

At H3, the decisive control has changed. The next review finds no account check in the raw call or its downstream implementation. Assume independent review supports that gap in the release candidate. The author restores the compulsory account-bound wrapper in H4; checks and the focused review complete against H4 and its relevant base. The decision becomes “allow H4 for the reviewed invariant,” with authorized and foreign-account validation evidence required by the team's chosen policy. This article supplies the example reasoning, not actual run results.

If GitHub directly requires a specific scanner result, a note saying “manual fallback complete” does not satisfy that check. The team must either obtain the required result or use its separately authorized, recorded exception mechanism. Design fallback authority before making a check mandatory; do not improvise a green result during a timeout.

Make a docs-only decision visible too

PR 249 changes only user-guide prose. The always-reporting policy job checks the complete changed-file list and records: “Documentation-only; no executable examples, build configuration or generated-route inputs changed; source-security analysis not applicable under policy version 3.” It can return the policy's allowed outcome while normal document review continues. This is a decision about applicability, not a source scan that somehow completed in zero seconds.

Now change the example: the documentation PR edits an executable example copied into a release build. The prose-only assumption fails, so the policy routes it through the relevant checks. A simple directory name is not a sufficient classifier when files under that directory affect shipped behavior. The policy-job implementation must handle missing diffs and unrecognized change classes as inconclusive rather than defaulting them to documentation.

Measure friction and missed scope together

Review the policy after a small batch of real PRs. Keep a ledger with the head revision, change class, check completion, scan scope, merge decision, review owner, time waiting for the security decision and later findings tied to that revision. Report at least median and slow-tail wait, the number of inconclusive runs, review reversals and known findings discovered outside the PR lane. Do not use “zero alerts” as the success metric: it could mean the detector skipped the relevant code.

For example, a later broad assessment finds that a generated route bypasses the shared guard. The rollout review discovers that policy version 3 classified its route-definition file as documentation. Version 4 adds that generator input and its emitted route binding to the sensitive-change trigger. Recheck the affected merged revision and route the supported issue separately; this policy correction does not retroactively make old reviews complete.

If quick checks regularly time out, investigate the critical path and move suitable broad work to the deeper lane. If the deferred lane repeatedly discovers newly introduced high-impact issues, tighten the PR trigger or add an explicit human check for that change class. If reviewers keep dismissing the same rule for a documented control, tune the rule or its context and keep the dismissal evidence. These are policy changes with measured effects, not blanket exemptions.

The next action is to write a one-page merge matrix for one repository and trial it on recent PRs before changing branch rules. For a concrete finding handoff, use the developer-friendly vulnerability comment template. If deciding which work belongs in each lane is the main problem, use the PR-versus-scheduled assessment guide.