Skip to content

Gate the \translate-resync comment trigger on commenter trust (author_association) — template-first rollout #192

Description

@mmcky

Problem

The \translate-resync comment trigger runs a secrets-using workflow (ANTHROPIC_API_KEY, services PAT) for any commenter. issue_comment workflows execute in default-branch context with full secrets access, and the sync workflows' if: conditions check only that the comment is on a PR and contains the magic string — not who wrote it. On public repos this is a cost-abuse vector: any GitHub account can comment \translate-resync on a merged PR and repeatedly trigger runner time plus Anthropic API spend. The commenter controls nothing but the trigger (translated content comes from the repo), so this is abuse/noise rather than code execution — but it should still be gated.

Surfaced by Copilot on QuantEcon/test-translation-sync#671; assessment there concluded the fix belongs in the canonical templates first, not in one harness file, because the harness exists to mirror production wiring and a one-file patch would both diverge it and leave the actual exposure open everywhere else.

Where the unguarded pattern lives

Location Role
src/cli/commands/setup.ts (generated source workflow, ~line 160) canonical — what --source-workflow ships
examples/README.md (3 occurrences) canonical — documented template
docs/user/quickstart.md canonical — documented template
test-translation-sync sync-translations-fa.yml / translation-sync.yml / sync-translations-ml.yml (#671) harness mirrors
production source repos (e.g. lecture-python-programming/sync-translations-fa.yml) estate

Proposed fix

Add a commenter-trust condition to the issue_comment branch of the if: in all of the above, e.g. contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.comment.author_association). Notes for implementation: MEMBER covers org members, COLLABORATOR covers repo collaborators, and CONTRIBUTOR (anyone with a merged PR) is deliberately excluded; if finer control is wanted, the alternative is an explicit permission-check step via the API, but the association gate is the standard low-ceremony fix.

Rollout order

  1. Canonical: setup.ts generated workflow + examples/README.md + docs/user/quickstart.md (one PR here).
  2. Harness: all three test-translation-sync workflows in one PR, keeping them mirrors of the updated template.
  3. Estate: production source repos.

Refs QuantEcon/test-translation-sync#671.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workinghigh-priorityAddress soonsecuritySecurity implications — needs a security-aware review bar

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions