DEV Community

Would you block a PR that changes GitHub Actions contents permission from read to write?

JinHyuk Sung on June 30, 2026

A sandbox PR changed one GitHub Actions workflow permission: permissions: contents: write The base branch had: permiss...
Collapse
 
unitbuilds profile image
UnitBuilds

I think it'd be useful if it did a check to see whether the PR actually requires the permission change, or not. Whether it's scoped or not and what sectors are affected by the permission change.

These can catch mistakes, drafts and help plug newly created holes before it's published to production

Collapse
 
sjh9714 profile image
JinHyuk Sung

Thanks — this is a useful way to frame it.

I agree that read -> write by itself should not automatically mean “bad PR.” A workflow may legitimately need contents: write for things like releases, tags, generated commits, or publishing flows.

The first-run behavior I’m aiming for is narrower:

  • surface the permission escalation
  • show the exact workflow path
  • keep it in warn mode by default
  • ask a human to decide whether the increased permission is expected

The next evidence layer is probably what you’re pointing at:

  • what job or workflow gained the permission
  • whether the changed workflow appears to contain steps that need that permission
  • whether the permission is scoped to the smallest job/workflow possible
  • which repo areas or release paths could be affected
  • whether the right reviewer approved that boundary change

I’d be careful about claiming Agent Gate can fully prove “this permission is required,” because that can become semantic judgment pretty quickly. But I do think it can make the review much more actionable by saying something like:

contents: write was added to .github/workflows/release.yml; this can affect release/tag/repo-content writes; no matching reviewer evidence was found; please confirm whether this is expected.

That feels like a better direction than either silently allowing the change or blocking every permission increase by default.

Appreciate the feedback — this points pretty directly toward a “scope + reviewer evidence” layer after the basic warning.

Collapse
 
unitbuilds profile image
UnitBuilds

Never blindly accept, instead, these are the parameters passed to the reviewer. Chances are these changes are either done by a senior dev, who understands their purpose and this just validates for them that it's done properly, or it's a junior that messed up and created a leak, which with this, the senior dev can quickly see if it's done correctly based on the task and scope assigned.

Imo, this is the kind of thing I'd put as a pre-commit too, so the person creating the PR can look at it first to validate themselves and maybe add notes to each/ each violation on why it was done, maybe with a section where they can dump their scoped assignment, so it's more practical to understand what each commit was made for, who signed off on it, what were the potential problems and what was the criteria of the task. Could actually be a good way to seed training data for a LLM now that I think of it?

Thread Thread
 
sjh9714 profile image
JinHyuk Sung

Yeah, I agree with this framing.

I don’t want Agent Gate to blindly accept or block the PR by itself. The more useful behavior is to put the right facts in front of the reviewer:

  • what changed
  • which workflow/job changed
  • which permission changed
  • what that permission can affect
  • whether the author explained why it was needed
  • whether someone appropriate signed off on it

That way, if it’s an intentional release/publishing change, the report helps validate the boundary. If it’s an accidental permission leak, the reviewer can spot it much faster.

The author-side idea is interesting too. I probably wouldn’t call these “violations” by default, because contents: write can be legitimate. But I can imagine the author seeing the finding before opening the PR and adding a note like “needed for release tags” or “needed for generated commit push.” Then the CI report can carry that context into review.

The training-data angle is interesting, but I’d keep that separate and explicitly opt-in. The first thing I’d want to trust is a clean review/audit record.

So yes, I think the next useful layer is not smarter auto-blocking. It’s better review context around the finding.

Thread Thread
 
unitbuilds profile image
UnitBuilds

And it creates an audit trail. If a vulnerability surfaces, they can inspect why, when, where, by who and who signed off on it.

Agreed, commits are a touchy field, because some people work in a broken branch just to make sure they save all progress states, but on submit ready commits, they want to still be able to denote it's a stable state and validate it. Eg. maybe they left some things open for their particular dev environment that is different in production and the detect is there to make sure they dont accidentally put their opening on a live server? So definitely focus on the review-phase, because that's what people will be using most. The detect just needs to work, but the review stage is accountability... If a dev signs off on it, it means they've reviewed the code + understand the reasoning of it all, the reviewer does the same when accepting.

Thread Thread
 
sjh9714 profile image
JinHyuk Sung

Yes — “review-phase accountability” feels like the right center of gravity.

Detection matters, but the durable value is the audit trail around the decision:

  • what changed
  • why it was accepted
  • who understood the boundary change
  • who signed off
  • what decision was made at review time

I’d want that to be a decision record, not a blame trail. Agent Gate should not pretend it can infer intent by itself. It should surface the boundary change, preserve the reasoning around it, and make the human review easier to trust later.

I also agree that commit-time/pre-commit behavior can get noisy, because branches often represent messy working state. The review phase is cleaner: by then the author is saying “this is ready to evaluate,” so asking for justification/sign-off makes more sense.

This is pushing me toward treating reviewer/sign-off evidence as a separate layer after the raw finding evidence:

finding -> author reason -> reviewer sign-off -> decision record