security(ci): repo-guard's issue_comment path checks out any fork's head based on the commenter, not the PR author

[ceilf6]

Context

Split out of #437 per the repo-guard review on #438. Pre-existing behavior, documented but deliberately not changed there — #438 only corrected the description.

Behavior

.github/workflows/repo-guard.yml gates issue_comment on the commenter:

github.event_name == 'issue_comment' &&
(
  contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.comment.author_association) ||
  contains(fromJSON('["NanluQingshi","HaveNiceDa"]'), github.event.comment.user.login)
)

The checkout step then resolves that branch to:

ref: ${{ ... || format('refs/pull/{0}/head', github.event.issue.number) }}

So any MEMBER/COLLABORATOR (or the two named external contributors) commenting on any third-party fork PR pulls that fork's head onto the self-hosted runner (vars.REPO_GUARD_RUNNER), in a job whose env carries secrets.LLM_API_KEY.

By contrast the pull_request_target path gates on the PR author and is restricted to repo branches plus the named allowlist.

Why this is not covered by the #438 opt-in

actions/checkout's assertSafePrCheckout returns early unless the event is pull_request_target or workflow_run, so the issue_comment path never hit the refusal and is unaffected by allow-unsafe-pr-checkout. #438 scopes that flag to pull_request_target only, so this path stays opted out even if the action's guard widens later.

Mitigating context

Confirmed from ceilf6/repo-guard@main's action.yml: the composite steps only ever execute from github.action_path (node "${{ github.action_path }}/scripts/review.mjs"), never from the checked-out workspace, which is passed as an env var and documented as explored "with read-only tools". So the exposure is not arbitrary code execution.

The residual risk is prompt injection against the reviewing agent — fork-authored files are read by Claude Code on a persistent host with an API key in env.

Also note the trigger requires a deliberate act by a trusted member, which is itself a form of vetting.

Options

1. Leave as-is, now that it is documented in the workflow comment.
2. Additionally require the PR author to pass the same allowlist on this path — needs an API lookup, since the issue_comment payload's issue.pull_request carries only URLs and no head repo info.
3. Resolve the head SHA via gh api first and check out that fixed SHA, closing the TOCTOU gap that refs/pull/{n}/head leaves open.

Maintainer call — this is a trust decision, not a bug fix.

[github-actions]

🛡️ ceilf6/repo-guard

Issue 分析: security(ci): repo-guard's issue_comment path checks out any fork's head based on the commenter, not the PR author

质量评分: 4/5
优先级建议: P1-高
类型: 功能请求
维护者下一步动作: 可以开始

完整性

• 问题陈述: 未在模型非契约输出中提供
• 复现步骤: 未在模型非契约输出中提供
• 预期与实际: 未在模型非契约输出中提供
• 环境信息: 未在模型非契约输出中提供
• 支撑证据: 未在模型非契约输出中提供

清晰度

• 标题质量: 未在模型非契约输出中提供
• 单一关注点: 未在模型非契约输出中提供
• 表达精确度: 未在模型非契约输出中提供
• 范围: 未在模型非契约输出中提供

可执行性

• 是否可开始: 未在模型非契约输出中提供
• 验收标准: 未在模型非契约输出中提供
• 依赖: 未在模型非契约输出中提供

建议

• Verified against the repo. Every code claim in the issue matches .github/workflows/repo-guard.yml exactly: the commenter-based gate (lines 45-46), the refs/pull/{n}/head ref (line 79), the author-based pull_request_target gate (lines 31-32), the scoped opt-in (line 81), and the runner/secret exposure (lines 52, 89). I also found existing test coverage in scripts/tests/workflow-rules.test.mjs:548-569 that pins the current scoping.

总结

Verified against the repo. Every code claim in the issue matches .github/workflows/repo-guard.yml exactly: the commenter-based gate (lines 45-46), the refs/pull/{n}/head ref (line 79), the author-based pull_request_target gate (lines 31-32), the scoped opt-in (line 81), and the runner/secret exposure (lines 52, 89). I also found existing test coverage in scripts/tests/workflow-rules.test.mjs:548-569 that pins the current scoping.