Skip to content

[P2] remediate: pr degrades silently — a downgraded remediation looks identical to a successful one #26

Description

@Ndevu12

Severity

P2 (medium). No wrong scan result, but an operator can believe auto-remediation is running when it is not — and only discover otherwise during an incident, which is the worst moment to learn it.

Context

remediate: pr asks the action to open a fix PR on an infected verdict. When the token cannot push, saw degrades down a fallback ladder — fork PR, else a git-am-able patch in sab-patches/ plus a deduplicated issue — and the step summary records which branch was taken.

That is reasonable behaviour for a CLI. The problem is what it looks like from CI: the job goes red on the verdict either way, so a downgraded remediation and a successful one produce the same visible outcome. The difference is a line in a summary nobody reads when the gate is already red for another reason.

remediate: pr additionally requires the repository setting "Allow GitHub Actions to create and approve pull requests". It is off by default in many orgs, it is set outside the workflow file, and nothing in the repo records that the workflow depends on it. Someone enables remediate: pr, sees red gates with findings, and reasonably concludes remediation is working.

Why this belongs to the action, not the scanner

  1. Intent is declared here — remediate: pr is a Strix input.

  2. Strix owns the CI signal surface: annotations, outputs, the step summary, and the gate.

  3. The action already applies this exact principle elsewhere. From the scan step's own comment on require-db:

    a missing --require-db when the caller asked for it is a hard error — the caller asked for a coverage guarantee this install cannot provide

    remediate: pr is the same shape — a caller asking for a guarantee the environment cannot provide — and is handled the opposite way.

A supporting scanner change may be needed if saw fix --pr does not already expose which rung of the ladder it landed on in machine-readable form. If so, that half belongs in stayAwakeBot and this issue depends on it.

Proposed

Proposed direction, not a settled design.

  1. Expose the outcome. Add a remediation output — pr, fork-pr, patch, skipped, failed — so a downgrade is inspectable by later steps rather than only narratable in prose.
  2. Annotate the downgrade. Emit ::warning:: (or ::error::, see open question) naming what was asked for, what happened, and the specific cause — missing repo setting, fork PR, absent permission. An annotation surfaces on the run and in the PR files view; a summary line does not.
  3. Preflight the requirement. When remediate: pr is set, check push capability before the scan and say plainly that remediation will degrade. Failing after a red gate teaches nothing; warning up front is actionable on the first run.
  4. Document the repository setting in the input description alongside the permissions it already lists, so the dependency is discoverable from the workflow file.

Scope

Any run with remediate: pr where the effective token lacks push or PR-creation capability — fork PRs, the org setting being off, contents/pull-requests write missing, or a restrictive org ruleset.

Safety / correctness invariants

  1. Never weakens the gate. A downgraded remediation must not turn a red gate green. Remediation is a courtesy; only a clean tree passes.
  2. Fork PRs still must not fail the job for lack of permission — consistent with upload-sarif and pr-comment. Fork degradation is expected and should warn, not error.
  3. No new permissions. Detecting the downgrade must not require broader scopes than remediate: pr already asks for.
  4. Escape untrusted text in any annotation, as [P2] Make findings unmissable even when the gate can't remediate: SARIF upload, report/patch artifacts, actionable step summary #10 established for the summary path — a crafted branch or path must not inject ::-workflow commands.

Open questions

  • Should a downgrade fail the step rather than warn? Failing matches the require-db precedent and the "asked for a guarantee, did not get it" reading. Against it: the gate is usually already red, so an extra failure adds noise without adding information — and on a fork PR, degradation is normal and unavoidable. A middle option is to fail only when the downgrade is fixable by the repo owner (setting off, permission missing) and warn when it is structural (fork PR).
  • Is the outcome already machine-readable from saw fix --pr, or does exposing it require a scanner change first?
  • Should remediate: pr on a fork PR be a configuration error at all, given it can never succeed there?

Non-goal

Does not change the fallback ladder itself. The degradation is sensible; only its visibility is at issue.

Related

Complements #10 (closed), which made findings unmissable on a red gate. This is the same concern applied to the remediation outcome — with the difference that #10 was explicitly additive and visibility-only, whereas this one raises a gate-semantics question in the first open question above.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions