Skip to content

fix: allow triage-permission actors to trigger review via allow-list - #10

Merged
lklimek merged 3 commits into
mainfrom
fix/allow-triage-actor-trigger
Jul 23, 2026
Merged

fix: allow triage-permission actors to trigger review via allow-list#10
lklimek merged 3 commits into
mainfrom
fix/allow-triage-actor-trigger

Conversation

@lklimek

@lklimek lklimek commented Jul 23, 2026

Copy link
Copy Markdown
Owner

TL;DR: Lets a trusted bot/service account without repo write access (e.g. a triage-only labeling bot) trigger the review, instead of always being rejected.

User story

As a repo maintainer, I want to let a specific triage-permission bot trigger the AI review by applying the trigger label, to achieve automated review triggering without granting that bot real write access to the repository.

Scenario

Base flow

A bot or human applies the trigger label (e.g. claudius-review) to a PR, which fires this action.

Actual behavior

The action's own preflight — and claude-code-action internally — only accept the GitHub collaborator-permission API's admin/write levels for the triggering actor. A fine-grained triage role rounds down to read via that API (it has no way to report "triage" directly), so any triage-only account is always rejected, even when it's a trusted account whose only job is applying the label. Confirmed on dashpay/dash-evo-tool run 29930647048: actor Claudius-Maginificent (triage role) reported permission read and was rejected.

Expected behavior

A repo can opt specific trusted non-write accounts into triggering the review, without loosening write access for anyone else.

Detailed discussion

What was done

  • Added allowed_non_write_users input (comma-separated usernames, or *) that:
    • Bypasses this action's own fast write-access preflight by exact actor-name match (mirrors claude-code-action's own bypass semantics).
    • Is passed through to claude-code-action's own allowed_non_write_users input, which also requires github_token to be explicitly provided (previously this action only set GH_TOKEN as an env var for its own gh calls, never passed github_token as an actual input to the claude-code-action step) — without both, the upstream bypass silently never activates.
  • Dropped maintain from the preflight's accepted permissions. claude-code-action itself only accepts admin/write, so a maintain-level actor was previously passing this fast preflight only to fail ~40s later inside claude-code-action with the exact opaque error this preflight exists to avoid.
  • Documented the new input in the README, including a "Triggering by non-write actors" section with a usage example.

Testing

  • action.yml and learn/action.yml validated with python3 -c "import yaml; yaml.safe_load(...)" (pass) and yamllint via pipx run (no new issues beyond this file's pre-existing 80-col line-length style, unrelated to this change).
  • Bash allow-list parsing logic (empty input, comma-separated with whitespace, exact match) verified standalone under set -e — no false matches, no spurious failures on empty/default input.
  • No functional CI in this repo (per CLAUDE.md); this action is exercised by consuming workflows once merged to main.

Breaking changes

None — new input defaults to "" (empty allow-list), identical behavior to before for anyone who doesn't set it. The only behavior change without opting in is the maintain → rejected fix (a permission level claude-code-action never actually honored anyway).

Checklist

  • YAML syntax validated
  • README updated
  • Verified end-to-end against a real triage-permission actor (pending: dashpay/dash-evo-tool#TBD opts Claudius-Maginificent into allowed_non_write_users once this merges to main)

Prior work

Attribution

🤖 Co-authored by Claudius the Magnificent AI Agent

The write-access preflight (and claude-code-action's own internal check)
only accept the collaborator-permission API's admin/write levels. A
fine-grained "triage" role rounds down to "read" via that API, so a
triage-only bot/service account applying the trigger label always got
rejected, even though it's a trusted account for this narrow purpose.

Add an allowed_non_write_users input (comma-separated usernames, or "*")
that bypasses the fast preflight by exact actor match and is threaded
through to claude-code-action's own allowed_non_write_users input (which
also needs github_token explicitly passed — previously only set via the
GH_TOKEN env var — to actually honor the bypass). Also drop "maintain"
from the preflight's accepted permissions: claude-code-action itself only
accepts admin/write, so the preflight was silently letting maintain-level
actors through to fail later with the exact opaque ~40s error it exists
to avoid.

Confirmed against dash-evo-tool run 29930647048: actor Claudius-Maginificent
(triage role) reported permission "read" and was rejected before this fix.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@lklimek
lklimek requested a review from Copilot July 23, 2026 11:12
@lklimek
lklimek marked this pull request as ready for review July 23, 2026 11:13

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the composite action to support allowing specific trusted non-write actors (e.g., triage-permission bots) to trigger the review via an explicit allow-list, while aligning the preflight permission gate with what anthropics/claude-code-action actually enforces.

Changes:

  • Adds a new allowed_non_write_users input and documents its use in the README.
  • Extends the action’s fast “triggering actor permission” preflight to honor the allow-list and removes maintain from the accepted permissions.
  • Passes github_token and allowed_non_write_users through to anthropics/claude-code-action@v1 so the upstream bypass can activate.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
README.md Documents the new allowed_non_write_users input and describes how triage-only actors can trigger reviews.
action.yml Adds the new input, implements the allow-list bypass in the preflight, tightens accepted permissions, and forwards inputs to claude-code-action.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread action.yml Outdated
The write-access preflight's allowed_non_write_users match was both
whitespace- and case-sensitive: a wildcard value with a stray space
(e.g. "* ") wouldn't hit the exact "*" comparison, and GitHub usernames
are case-insensitive so a differently-cased list entry (or actor login)
would silently fail to match and reject a trusted actor. Trim the whole
input before the wildcard check and lowercase both sides of the
per-entry comparison. Verified with 7 standalone bash test cases
covering the two failure modes flagged.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (1)

action.yml:239

  • To keep the fast preflight and claude-code-action in sync, pass the same normalized allowed_non_write_users value to the underlying action (e.g. the ALLOWED_NON_WRITE_USERS env var exported by the preflight). Otherwise whitespace/casing normalization only applies to the preflight, and the upstream bypass may still fail unexpectedly.
        github_token: ${{ inputs.github_token }}
        allowed_non_write_users: ${{ inputs.allowed_non_write_users }}

Comment thread action.yml Outdated
…value

Two related gaps from the previous whitespace/case fix, both flagged by
Copilot review:

- The preflight normalized allowed_non_write_users for its own matching,
  but the claude-code-action step still received the raw, unnormalized
  input — the two steps could disagree on edge cases (e.g. trailing
  whitespace in "* "), reintroducing the opaque ~40s failure this
  preflight exists to prevent. Fixed by exporting the trimmed value via
  GITHUB_ENV (ALLOWED_NON_WRITE_USERS) and having both steps read that
  single normalized value instead of computing it twice.

- The case-insensitive matching added last commit was actually a
  regression: claude-code-action compares the actor string with exact
  case (no case-folding), so a differently-cased list entry would now
  pass this preflight but still get rejected 40s later inside
  claude-code-action itself — worse than before, since it converts a
  fast, clear failure into a slow, opaque one. Reverted preflight
  matching to trim-only, exact-case — now structurally identical to
  claude-code-action's own comparison, so the two can never disagree.

Re-verified with the same 7 standalone bash test cases, updated to
assert the case-mismatch scenario is now correctly rejected rather than
falsely accepted.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

action.yml:109

  • The normalized allowed_non_write_users value is only exported to GITHUB_ENV after the ACTOR empty guard. When github.event.sender.login is empty, this step exits early and the downstream claude-code-action step receives an empty ${{ env.ALLOWED_NON_WRITE_USERS }}, even if the user configured inputs.allowed_non_write_users. Export the normalized value before the early-exit so pass-through remains consistent when the preflight is skipped.
        ACTOR="${{ github.event.sender.login }}"
        if [ -z "$ACTOR" ]; then
          echo "::warning::Could not determine the triggering actor (github.event.sender.login was empty) — skipping the write-access preflight. The underlying claude-code-action will still enforce this itself."
          exit 0
        fi

@lklimek
lklimek merged commit e926845 into main Jul 23, 2026
2 checks passed
@lklimek
lklimek deleted the fix/allow-triage-actor-trigger branch July 23, 2026 12:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants