Harden bigbonk workflow trigger authorization

[dknecht]

Motivation

• The bigbonk workflow could be triggered by any non-bot commenter while running with id-token: write, repo write permissions, and injected Cloudflare secrets, creating a CI privilege-escalation and secret-exfiltration risk.

Description

• Tighten the job-level if expression in .github/workflows/bigbonk.yml to require that the comment/review author association is one of OWNER, MEMBER, or COLLABORATOR in addition to the existing non-bot and /bigbonk mention checks.
• The new condition uses fromJSON('["OWNER","MEMBER","COLLABORATOR"]') and tests github.event.comment.author_association || github.event.review.author_association to block untrusted external commenters.

Testing

• Verified the updated YAML contents with sed -n '1,220p' .github/workflows/bigbonk.yml and nl -ba .github/workflows/bigbonk.yml, which showed the new if expression as intended (succeeded).
• Confirmed the change via git diff -- .github/workflows/bigbonk.yml to inspect the patch (succeeded).
• Attempted to verify the third-party action reference via git ls-remote but the network check failed due to environment/proxy restrictions (failed).

---

Codex Task

[ask-bonk]

Good. Now let me think about the actual security concern and verify correctness of the || behavior in GitHub Actions expressions.

In GitHub Actions expressions, || returns the first truthy value, similar to JavaScript. So for an issue_comment event:

• github.event.comment.author_association is e.g. "CONTRIBUTOR" (truthy) -- this is used
• github.event.review.author_association is '' (empty string / null) -- not reached

This is correct behavior.

Now the real security question: Is author_association trustworthy and non-spoofable? Yes -- author_association is set by GitHub based on the repository's collaborator list and the user's actual relationship to the repo. It cannot be spoofed by the commenter.

The allowed values OWNER, MEMBER, COLLABORATOR correctly restrict to trusted repo contributors. External contributors would have CONTRIBUTOR, FIRST_TIMER, FIRST_TIME_CONTRIBUTOR, or NONE -- all correctly excluded.

This is a legitimate security hardening. The change is correct and addresses a real privilege escalation vector. Let me check one more thing -- whether there's any edge case with the contains() on an array:

contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), value) -- GitHub Actions contains() on arrays checks if any element equals the value (case-insensitive). This is the correct pattern for checking membership.

The change looks correct and is a meaningful security improvement. There are no actionable issues.

LGTM

github run

[elithrar]

/bonk re-review after you read and research how the underlying ask-bonk action enforces write (by default) or CODEOWNERS permissions for any invocation. By default, Bonk does not allow actors without write permissions to invoke it.

You cannot only look at the workflow files within this repo. You must look at the full upstream action and code.

Throughly review https://github.com/ask-bonk/ask-bonk/blob/main/github/action.yml#L25-L28 before you come back.

[elithrar]

As a data point: Claude Code's GitHub Action does this the same way (write users only): https://github.com/anthropics/claude-code-action/blob/main/action.yml - it does not have to be re-defined in the consuming repo's action (which is easy to get wrong / puts the burden on the consumer/user to get right)

[ask-bonk]

The author_association check is redundant with the upstream ask-bonk action's built-in permission enforcement and has a functional correctness issue. Details in inline comment.

---

This review was generated by an AI assistant (OpenCode/Claude) and may contain errors. Please verify all findings independently.

[ask-bonk]

Two issues with this if expression:

1. The author_association check is redundant with the action's built-in enforcement.

The ask-bonk action already enforces permission checks server-side before OpenCode is installed or invoked. Looking at the action.yml inputs:

permissions:
  description: "Required permission level: 'admin', 'write', 'any', or 'CODEOWNERS'"
  required: false
  default: "write"

This workflow passes permissions: write on line 47, matching the default. The action's orchestrate.ts checkPermissions() function is the first thing executed in main(). It calls the GitHub API (/repos/{owner}/{repo}/collaborators/{actor}/permission) and halts the job via core.setFailed() if the actor doesn't have write access. This happens before OIDC token exchange, before OpenCode installation, and before any secrets are consumed by the model process.

As @elithrar noted, this is the same pattern used by claude-code-action — the action itself is the authorization boundary, not the workflow if condition. Duplicating authorization logic in the consuming workflow is fragile (as demonstrated by issue #2 below) and creates a false sense of security while the real enforcement happens elsewhere.

The PR description's framing of "privilege-escalation and secret-exfiltration risk" is inaccurate — that risk is already mitigated by the action's built-in check.

2. The mention check doesn't work for pull_request_review events.

Line 19 checks github.event.comment.body for /bigbonk, but for pull_request_review events the trigger text would be in github.event.review.body. This bug existed before this PR (the original if had the same issue), so it's not a regression, but it means the author_association guard on lines 20-23 is only exercised for issue_comment events since pull_request_review invocations are already blocked by the faulty mention check.

Recommendation: Revert this change. The action's built-in permissions: write enforcement is the correct and complete authorization boundary. If you want the minor optimization of avoiding runner allocation for unauthorized commenters, that should be handled at the action level (which already does mention matching in its own Check mentions step), not duplicated in every consuming workflow.

[ask-bonk]

Review posted successfully on PR #6207. Here's a summary of my findings:

Review Summary

REQUEST_CHANGES posted on PR #6207.

Key Findings

1. The author_association check is redundant with the action's built-in enforcement. The ask-bonk action's permissions input defaults to "write" and the workflow already passes permissions: write. The action's orchestrate.ts calls checkPermissions() as the first step in main(), which uses the GitHub API to verify the actor has write access and calls core.setFailed() before OIDC exchange, before OpenCode installation, and before any secrets reach the model process. The PR's framing as a "privilege-escalation and secret-exfiltration risk" fix is inaccurate — that risk was already mitigated.

2. The mention check doesn't work for pull_request_review events. The if checks github.event.comment.body for /bigbonk, but for pull_request_review events the text is in github.event.review.body. This is a pre-existing bug, not a regression from this PR.

3. Recommendation: Revert the change. The action is the correct authorization boundary, not the workflow if condition. Duplicating authorization in consuming workflows is fragile and easy to get wrong.

github run