Skip to content

ci: charge Copilot reviews to the review owner - #3139

Merged
hqhq1025 merged 1 commit into
mainfrom
codex/copilot-review-owner
Aug 17, 2026
Merged

ci: charge Copilot reviews to the review owner#3139
hqhq1025 merged 1 commit into
mainfrom
codex/copilot-review-owner

Conversation

@hqhq1025

Copy link
Copy Markdown
Contributor

Summary

Pin automatic GitHub Copilot review requests to the repository secret
COPILOT_REVIEW_TOKEN, which belongs to hqhq1025, instead of using the
event-triggering ${{ github.token }} identity.

The workflow now fails closed when the secret is missing or belongs to another
account. It also removes all permissions from the generated GITHUB_TOKEN.

Security

The workflow still uses pull_request_target so it can request reviews for
fork pull requests, but it never checks out, executes, or otherwise consumes
untrusted pull request code. The dedicated token is used only for GitHub API
identity checks and reviewer requests.

Verification

  • Parsed .github/workflows/copilot-auto-review.yml with Ruby YAML
  • Checked the embedded shell with bash -n
  • Ran structural assertions for the dedicated secret, expected owner, absent
    ${{ github.token }} fallback, and absent checkout/head-ref consumption
  • Ran git diff --check
  • Confirmed COPILOT_REVIEW_TOKEN is configured in repository Actions secrets
  • Previously smoke-tested the same hqhq1025 user-token identity by requesting
    and completing Copilot review on PR chore(deps): bump the official-actions group across 1 directory with 3 updates #2941

The final automatic attribution path can be observed after merge on the next
non-draft, non-bot pull request event.

AI use

Select exactly one:

  • No generative tool made a substantive contribution
  • Generative tooling made a substantive contribution

Tool(s) and scope: Codex audited the attribution path, implemented the
least-privileged identity binding, and ran the verification commands.

Checklist

  • Tests cover the change and fail without it
  • Lint, format, typecheck and the affected suites pass locally

Does this PR entail a change in behavior?

  • Yes - automatic Copilot review usage is attributed to hqhq1025.
  • No

@hqhq1025
hqhq1025 marked this pull request as ready for review August 17, 2026 05:47
@coderabbitai

coderabbitai Bot commented Aug 17, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 928a9806-502e-4dd3-add9-01a570a87646

📥 Commits

Reviewing files that changed from the base of the PR and between 9c5455b and 7973d7d.

📒 Files selected for processing (1)
  • .github/workflows/copilot-auto-review.yml

Included review availability: Your plan includes up to 3 reviews per rolling hour; 2 remain after this review.


📝 Walkthrough

What this PR solves

The workflow now attributes automatic Copilot review requests to hqhq1025 through COPILOT_REVIEW_TOKEN. It no longer uses the event-triggering ${{ github.token }} identity.

The workflow fails closed when the secret is missing or belongs to another account. It also removes permissions from the generated GITHUB_TOKEN.

Source of truth

This change extends the existing automatic review workflow. It does not create a parallel review path.

The workflow still uses pull_request_target for fork pull requests. It does not check out, execute, or consume untrusted pull-request code.

Scope and complexity

The change is the smallest coherent solution described by the diff. It adds token validation, identity checks, and least-privilege permissions because these controls are required to attribute reviews correctly and prevent unintended token use.

No public or exported entities changed.

Simplification opportunities

No deletion or simplification is evident without weakening token validation, security behavior, or regression coverage.

Risks and validation

The dedicated token is used for GitHub API identity checks and reviewer requests. A missing, invalid, or incorrectly owned secret prevents review requests.

The reported validation includes:

  • YAML parsing
  • Shell syntax validation
  • Workflow structural assertions
  • git diff --check
  • Confirmation that the repository secret is configured
  • A prior smoke test of token identity

The final status of required checks remains unverified without direct check results.

Review-relevant risks

The diff changes GitHub API identity and automatic review attribution. Any material security or governance change in this area requires independent human review under repository policy.

No other protected-area effect was identified in the current diff. The person performing the merge must review the final diff, and a maintainer makes the final determination.

Walkthrough

The workflow removes default GitHub token permissions. It uses COPILOT_REVIEW_TOKEN, checks that the secret exists, and verifies that the authenticated account matches the expected requester before requesting a review.

Changes

Copilot review security

Layer / File(s) Summary
Token permissions and validation
.github/workflows/copilot-auto-review.yml
The workflow disables default permissions. The review step uses COPILOT_REVIEW_TOKEN, validates the secret, verifies the authenticated account, and exits on failed checks.

Estimated code review effort: 3 (Moderate) | ~15 minutes

Merge Risk: ⚪ Minimal · up to 7973d

This localized workflow change updates automatic review attribution and is merge-ready after normal checks and review; no actionable merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: attributing Copilot reviews to the designated review owner.
Description check ✅ Passed The description covers the change, security considerations, verification, AI use, checklist, and behavior change with sufficient detail.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Ai Use Disclosure ✅ Passed The PR selects only substantive generative use, names Codex and its scope, and its sole introduced commit has the consistent standalone trailer Generated-by: Codex.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/copilot-review-owner

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the Copilot auto-review GitHub Actions workflow to ensure review requests are made using a dedicated repository secret token (COPILOT_REVIEW_TOKEN) tied to a specific owner account, rather than the event-scoped ${{ github.token }} identity. It also hardens the workflow by removing all permissions from the generated GITHUB_TOKEN and failing closed when the dedicated secret is missing or does not match the expected user.

Changes:

  • Set workflow permissions: {} to remove all GITHUB_TOKEN permissions.
  • Switch gh authentication from ${{ github.token }} to ${{ secrets.COPILOT_REVIEW_TOKEN }}.
  • Add a runtime identity check to ensure the secret token resolves to the expected requester (hqhq1025) and fail closed otherwise.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@M4n5ter M4n5ter left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

English

LGTM. The attribution and security boundaries are correct.

中文

LGTM,归属与安全边界均正确。

@hqhq1025
hqhq1025 merged commit e0e8026 into main Aug 17, 2026
15 checks passed
@M4n5ter
M4n5ter deleted the codex/copilot-review-owner branch August 17, 2026 06:06
@Astro-Han

Copy link
Copy Markdown
Contributor

Thanks for the fix — the core change is minimal (2 lines: github.tokensecrets.COPILOT_REVIEW_TOKEN), the identity check is cheap defense-in-depth, and fail-closed is the right call (a fail-open here would silently break billing attribution). I verified the secret exists and the check doesn't block merges (no required status checks on main).

Conclusion: PASS — no blocking findings.

P2-1 (governance, non-blocking) — a personal PAT as a repo-level automation credential with a hardcoded owner and no rotation/offboarding plan. copilot-auto-review.yml:27-28 pins GH_TOKEN to secrets.COPILOT_REVIEW_TOKEN and EXPECTED_REVIEW_REQUESTER: hqhq1025. If the token expires/revokes or hqhq1025 leaves, every non-draft PR's request-review goes red (fail-closed is intended, but the fix path is "rotate secret + edit the hardcoded username", two manual steps during which all PRs keep failing). Also, a personal token in a shared secret is readable by any repo admin, and the token type/scope isn't recorded in the PR so the blast radius can't be verified externally. Not a current-correctness defect — please either record the token type/scope/expiry and rotation owner in the PR, or explicitly accept this operational risk and defer.

Optional nits (P3): the new code path has never run in CI (the request-review run executed the old script on main; the "Tests cover the change" claim is local one-off assertions, not committed tests — fine, just observe the next non-draft PR after merge); a dead/expired token surfaces as a raw gh 401 rather than the friendly ::error:: (the -z check only covers a missing secret); the identity check doesn't verify the token's pull-requests: write scope (a 403 on POST would be raw); the checklist's "Tests cover the change and fail without it" is inaccurate for a no-test diff.


AI-assisted review disclosure: this review was produced with AI assistance (pi review subagent on ollama-cloud/deepseek-v4-flash). The subagent verified the secret exists, checked branch protection (no required checks), and compared against repo workflow conventions; the P2-1 governance point is static analysis, not an observed failure. Please weigh these findings with your own judgment.

中文摘要(AI 辅助审查)

结论:PASS,无阻塞项。核心改动最小(2 行:github.token → secrets.COPILOT_REVIEW_TOKEN),身份检查是廉价的纵深防御,fail-closed 取舍正确(fail-open 会让计费归属静默失效)。已核实 secret 存在、该 check 不阻塞合并(main 无 required checks)。P2-1(治理,不阻塞):个人 PAT 作为仓库级自动化凭据 + owner 硬编码,无轮换/离职预案——token 过期/吊销或 hqhq1025 离职时所有 PR 的 request-review 会红,修复需两步人工操作;个人 token 存共享 secret 任何 repo admin 可读,且 PR 未记录 token 类型/scope 无法外部核验泄露半径。非当前正确性缺陷,建议在 PR 记录 token 类型/scope/过期时间与轮换责任人,或显式接受该运维风险。P3:新代码路径合并前从未在 CI 执行过("Tests cover the change" 是本地一次性断言)、token 失效时是原始 401 而非友好错误、身份检查不验证 pull-requests:write scope、checklist 表述不准确。

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants