fix(rgs-004): add author_association guard to pre_activation jobs for comment-triggered workflows#29481
Merged
Merged
Conversation
… comment-triggered workflows
Resolves the RGS-004 static-analysis finding by adding an explicit
`author_association` check to the `if:` condition of the `pre_activation`
job in compiled workflows that:
- are triggered by `issue_comment` or `pull_request_review_comment` events, AND
- have permission checks enabled (i.e. `roles` is NOT set to `all`)
The new condition:
(github.event_name != 'issue_comment' && github.event_name != 'pull_request_review_comment')
|| contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.comment.author_association)
prevents the `pre_activation` job from even starting for commenters with no
affiliation to the repository, complementing the existing `check_membership`
step that provides the authoritative per-user role check inside the job.
Workflows with `roles: all` intentionally opt out of permission checks and
are left unchanged.
All 205 workflow lock files have been recompiled to reflect the change.
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/42a49fdd-afdd-4956-846b-7028f3bf4f91
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…precision Addresses code review feedback by matching 'issue_comment:' and 'pull_request_review_comment:' (with colon) instead of bare substrings, making it unambiguous that we are checking for YAML event keys in the compiled on: section. Agent-Logs-Url: https://github.com/github/gh-aw/sessions/42a49fdd-afdd-4956-846b-7028f3bf4f91 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot
AI
changed the title
[WIP] Fix comment-triggered workflow without author authorization check
fix(rgs-004): add author_association guard to pre_activation jobs for comment-triggered workflows
May 1, 2026
Collaborator
|
@copilot merge main and recompile |
…omment-triggered-workflow # Conflicts: # .github/workflows/grumpy-reviewer.lock.yml Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/2f777bd5-8d1d-4b13-8271-db54ae3a2acc Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Done — merged main (resolved a heredoc-hash conflict in Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses RGS-004 by adding an early authorization gate for comment-triggered workflows so pre_activation is skipped (instead of run-then-deny) when the commenter isn’t sufficiently associated with the repo/org.
Changes:
- Updated the workflow compiler to inject an
author_association-based guard into thepre_activationjobif:forissue_comment/pull_request_review_commenttriggers when permission checks are enabled. - Added helper functions to detect comment triggers in the compiled
on:YAML and to build the author association condition expression. - Added a unit test validating presence/absence of the guard and regenerated affected workflow lockfiles.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/compiler_pre_activation_job.go | Injects a job-level author_association guard into pre_activation when comment triggers are present and permission checks are enabled; adds helper builders. |
| pkg/workflow/role_checks_test.go | Adds a compiler test ensuring author_association gating appears for comment-triggered workflows and is bypassed for roles: all / non-comment triggers. |
| .github/workflows/ace-editor.lock.yml | Regenerates compiled lockfile with updated pre_activation.if guard. |
| .github/workflows/archie.lock.yml | Regenerates compiled lockfile with updated pre_activation.if guard. |
| .github/workflows/brave.lock.yml | Regenerates compiled lockfile with updated pre_activation.if guard. |
| .github/workflows/cloclo.lock.yml | Regenerates compiled lockfile with updated pre_activation.if guard. |
| .github/workflows/grumpy-reviewer.lock.yml | Regenerates compiled lockfile with updated pre_activation.if guard (and related generated heredoc IDs). |
| .github/workflows/mergefest.lock.yml | Regenerates compiled lockfile with updated pre_activation.if guard. |
| .github/workflows/pdf-summary.lock.yml | Regenerates compiled lockfile with updated pre_activation.if guard. |
| .github/workflows/plan.lock.yml | Regenerates compiled lockfile with updated pre_activation.if guard. |
| .github/workflows/pr-nitpick-reviewer.lock.yml | Regenerates compiled lockfile with updated pre_activation.if guard. |
| .github/workflows/q.lock.yml | Regenerates compiled lockfile with updated pre_activation.if guard. |
| .github/workflows/scout.lock.yml | Regenerates compiled lockfile with updated pre_activation.if guard. |
| .github/workflows/security-review.lock.yml | Regenerates compiled lockfile with updated pre_activation.if guard. |
| .github/workflows/tidy.lock.yml | Regenerates compiled lockfile with updated pre_activation.if guard. |
| .github/workflows/unbloat-docs.lock.yml | Regenerates compiled lockfile with updated pre_activation.if guard. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 16/16 changed files
- Comments generated: 2
Comment on lines
+447
to
+456
| if needsPermissionCheck && hasCommentEventInOn(data.On) { | ||
| commentAuthCondition := RenderCondition(buildCommentAuthorAssociationCondition()) | ||
| if jobIfCondition != "" { | ||
| jobIfCondition = RenderCondition(BuildAnd( | ||
| &ExpressionNode{Expression: commentAuthCondition}, | ||
| &ExpressionNode{Expression: jobIfCondition}, | ||
| )) | ||
| } else { | ||
| jobIfCondition = commentAuthCondition | ||
| } |
There was a problem hiding this comment.
The new job-level author_association guard can unintentionally block workflows that rely on on.bots allowlisted bot/app actors. check_membership supports GH_AW_ALLOWED_BOTS, but with this change the job will be skipped before check_membership runs whenever the actor’s author_association is not OWNER/MEMBER/COLLABORATOR (common for bots and GitHub Apps). Consider extending buildCommentAuthorAssociationCondition (or the surrounding logic) to also allow github.actor values from data.Bots (including both slug and [bot] forms) so existing bot-trigger behavior remains intact.
|
|
||
| compiledStr := string(compiledContent) | ||
|
|
||
| hasCheck := strings.Contains(compiledStr, "author_association") |
There was a problem hiding this comment.
This test only checks for the substring author_association anywhere in the compiled workflow. To make it resilient and ensure the guard is applied specifically to the pre_activation job-level if: (and not, for example, in a commented section or a different job), consider extracting the pre_activation job YAML (e.g., via the existing extractJobSection helper) and asserting on that section’s if: line(s).
Collaborator
|
@copilot review all comments |
…ghten test to use pre_activation section Agent-Logs-Url: https://github.com/github/gh-aw/sessions/5b214baf-6bf3-4d89-a7f3-03169377d187 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…st assertions data-driven Agent-Logs-Url: https://github.com/github/gh-aw/sessions/5b214baf-6bf3-4d89-a7f3-03169377d187 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Addressed both reviewer threads in c6892c9 + c90e438:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Collaborator
|
@copilot disable this optimization if you detect github action expression. Ensure runtime check is ALWAYS present (unless roles: all) |
…in expressions Agent-Logs-Url: https://github.com/github/gh-aw/sessions/17eae8a2-66f1-4171-ab82-34f214a90bb8 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Done in 292dcd0. Added Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Collaborator
|
@copilot a github action expression also disable this optimization make sure this works with imported shared agentic workflows |
… expression; add import test Agent-Logs-Url: https://github.com/github/gh-aw/sessions/9a652c4f-a3e3-4aa3-8765-8eb235c7c3e2 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Done in 639a117. Two changes:
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Collaborator
|
@copilot merge main and recompile |
…omment-triggered-workflow # Conflicts: # .github/workflows/grumpy-reviewer.lock.yml Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/f36ae35e-ecee-4354-9948-d13bb8f1212d Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Done — merged main (resolved a conflict in Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
5 tasks
11 tasks
5 tasks
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.
Workflows triggered by
issue_commentorpull_request_review_commenthad no early authorization gate — any GitHub user could trigger thepre_activationjob. The existingcheck_membershipstep handled denial inside the job, but the job itself still ran, consuming resources and failing the RGS-004 static analysis rule.Changes
Compiler (
compiler_pre_activation_job.go): WhenneedsPermissionCheckis true and the compiledon:section includesissue_comment:orpull_request_review_comment:, thepre_activationjob now receives an explicitauthor_associationguard in itsif:condition:Workflows with
roles: allopt out ofneedsPermissionCheckand remain unrestricted.Bot actor exemption: Actors listed in
on.botswith static names are also exempted from the guard via|| github.actor == '<bot>'clauses, preserving existing bot/app-triggered workflow behaviour. Bot conditions are combined usingBuildDisjunctionto produce a flat OR expression rather than a deeply nested binary tree.Expression safety — guard disabled when static analysis is not possible: The static job-level guard is skipped entirely (leaving
check_membershipas the sole runtime gate) in two cases:on.botsis a GitHub Actions expression (contains${{) — the bot identity cannot be resolved at compile time. This applies to bots defined directly in the workflow and to bots contributed viaimports:from shared agentic workflows.on:section itself contains a GitHub Actions expression — event detection is unreliable at compile time.Tests (
role_checks_test.go): The test suite usesextractJobSection("pre_activation")to assert the guard is present on thepre_activationjob-levelif:specifically (not anywhere in the compiled YAML). Test cases cover: default roles,slash_command,pull_request_review_comment,roles: all, push-only,workflow_dispatch, static bots, expression-based bots (inline), and expression-based bots imported from a shared agentic workflow. Bot name assertions are data-driven via awantBotNamesfield on the test struct.