Prevent Bonk from pushing commits.#217
Conversation
|
commit: |
|
/bonk review this |
There was a problem hiding this comment.
Review: token_permissions: NO_PUSH
The change is correct and well-motivated. As the PR body notes, prompt instructions are not a security boundary, so enforcing NO_PUSH at the token level is the right approach. I confirmed against the ask-bonk docs that token_permissions is a supported input and that NO_PUSH sets contents: read while retaining issues: write and pull_requests: write — so Bonk can still comment, review, and suggest, but cannot push. The docs also note invalid values fail closed to NO_PUSH, which is the safe default.
Two things worth confirming before merge (non-blocking):
-
Different action SHAs across the two workflows.
bonk-pr-review.ymlpins24832587...(the commit the docs link references), butbonk.ymlpinsc39e982d.... Thetoken_permissionsfeature must exist in both pinned revisions to take effect. Please confirmc39e982dincludes token scoping — if it predates the feature, thebonk.ymlchange would be silently ignored and Bonk would retain push access on/bonkinvocations, which is exactly the case this PR is trying to close. Consider pinning both to the same known-good SHA. -
Stale job-level
contents: writeinbonk.yml(line 27, outside the diff). Now that the installation token is scopedNO_PUSH, the job still declarescontents: writefor the defaultGITHUB_TOKEN. It is not the token that pushes here, but it is now misleading. For defense-in-depth and to matchbonk-pr-review.yml(which usescontents: read), consider reducing it toread— unless another step relies on the default token for content writes.
Net: LGTM assuming point (1) checks out. This is a meaningful hardening.
|
Review posted successfully: #217 (review) SummaryI reviewed PR #217, which adds Verdict: LGTM — correct and well-motivated hardening. I verified against the ask-bonk docs that Two non-blocking follow-ups flagged:
|
|
It would be kind of nice if we could actually ask bonk to push changes when we explicitly want it to, maybe we could define a separate slash command for that or something. (No need to action in this PR.) |
Bonk ignored the review prompt on #201 and pushed a commit. Prompt instructions are not a reliable security boundary.
This sets token_permissions: NO_PUSH for both automatic reviews and /bonk invocations. Bonk retains permission to post comments, reviews, and suggestions, but cannot push commits.
Documentation: https://github.com/Cloudflare-Studio/ask-bonk/blob/24832587005550860c8ad13fd96519e731ca632a/README.md#token-scoping