security: gate the \translate-resync trigger on commenter trust (3 languages) - #586
Merged
Conversation
…3 languages) The issue_comment clause checked only that the comment body contained the magic string. issue_comment workflows run in default-branch context with full access to secrets, so on a public repo any GitHub account could comment \translate-resync on a merged PR and spend Anthropic credits and runner minutes, repeatedly. The clause now also requires the comment to be on a pull request (plain issues raise the same event) and its author to be an OWNER, MEMBER or COLLABORATOR. Adds permissions: contents: read — the action authenticates with QUANTECON_SERVICES_PAT, so the ambient GITHUB_TOKEN never needs write. Applied identically to the zh-cn, fa and fr sync workflows. Ports the shape from QuantEcon/action-translation#192. Triggers, paths filters, inputs and the @v0 pin are unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the \translate-resync manual trigger in the translation sync GitHub Actions workflows to prevent untrusted users from starting secret-bearing workflow runs via issue_comment events. It aligns the workflow-level gating with the action’s internal trust checks and reduces ambient token privileges.
Changes:
- Gate the
issue_commentexecution path on: comment is on a PR, comment contains\translate-resync, and commenter association isOWNER/MEMBER/COLLABORATOR. - Add job-level
permissions: contents: readto keepGITHUB_TOKENread-only (checkout-only use).
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| .github/workflows/sync-translations-zh-cn.yml | Tightens issue_comment gating + sets read-only contents permission for the sync job. |
| .github/workflows/sync-translations-fr.yml | Tightens issue_comment gating + sets read-only contents permission for the sync job. |
| .github/workflows/sync-translations-fa.yml | Tightens issue_comment gating + sets read-only contents permission for the sync job. |
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.
Applies the
\translate-resynctrust gate from QuantEcon/action-translation#192 (step 3 of that rollout, tracked in QuantEcon/action-translation#220). No behaviour changes for normal merges.Why
The
issue_commentclause checked only that the comment body contained the magic string.issue_commentworkflows run in default-branch context with full access to secrets, and GitHub cannot filter the event by comment body at the trigger level — so thisif:is the only gate. On a public repo that means any GitHub account could comment\translate-resyncon a merged PR and spend Anthropic credits plus runner minutes, repeatedly.The action already rejects untrusted commenters internally, so no translation happens — but the run still starts, checks out, loads the action, and holds
ANTHROPIC_API_KEYand the services PAT in its environment while doing it. The workflow-level gate is the only thing that stops the job from starting at all.What changed
Two edits per file:
issue_commentclause now requires the comment to be on a pull request (github.event.issue.pull_request— plain issues raise the same event, so a comment on any issue used to fire a run too), to carry the command, and to come from anOWNER,MEMBERorCOLLABORATOR.permissions: contents: read. The action authenticates to the target repo withQUANTECON_SERVICES_PAT; checkout is the ambientGITHUB_TOKEN's only consumer, so it never needs write.Triggers,
paths:filters, action inputs and the@v0pin are all untouched.Who can still trigger a resync
Everyone who could before, in practice: org members and repo collaborators.
CONTRIBUTOR— anyone with one merged PR — is deliberately excluded, and the action enforces the same three-way set internally, so widening the workflow alone would only produce a billed run that no-ops. The sync bot posts as an org member today, so its comments are unaffected.Verification
The folded
if:is not a theory. This exact shape has been running on QuantEcon/lecture-python.myst since 2026-07-22, and it was E2E-confirmed on the action's test harness on 2026-07-26 across three languages: the merge path fired three green syncs, and a\translate-resynccomment from aMEMBERwas admitted by the gate (the job started and logged the resync line). Each file here was also parsed with a YAML parser post-edit to confirm the folded expression is balanced and carries all four conditions.Worth knowing about the failure mode: a malformed
if:does not error, it silently stops the workflow firing — which looks exactly like "no PRs have been merged lately". GitHub's workflow linter rejects a malformedif:on push, so a clean push is already meaningful evidence; the next merged lecture PR is the real confirmation.Refs QuantEcon/action-translation#192, QuantEcon/action-translation#220.
🤖 Generated with Claude Code