feat(ci): 完善门禁签名去重、Fork隔离与依赖版本升级 - #9
0xTimi2233 wants to merge 8 commits into
Conversation
|
@coderabbitai review |
|
@greptileai review |
✅ Action performedReview finished.
|
|
| Filename | Overview |
|---|---|
| .github/workflows/ai-review.yml | 重构 AI 审查触发、凭据准备和脚本执行流程。 |
| scripts/ai-review.ts | 新增审查编排脚本，覆盖 PR 校验、会话管理、PI 执行和 GitHub 评论更新。 |
| .github/workflows/review-gate.yml | 改用 GitHub App token 发布带稳定标记的首次审查命令，避免重复触发。 |
| .github/workflows/cache-cleanup.yml | 在 PR 关闭时同时清理编译缓存和对应的 AI 审查会话工件。 |
Reviews (4): Last reviewed commit: "fix(ai-review): 区分历史会话不存在与工件下载解压异常" | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: QUIET Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. Walkthrough本次变更新增 Sequence Diagram(s)sequenceDiagram
participant PRComment
participant AIWorkflow
participant aiReview
participant GitHubAPI
participant pi
PRComment->>AIWorkflow: 提交包含 `@timi-ai` 的评论
AIWorkflow->>aiReview: 传入 PR 和评论环境变量
aiReview->>GitHubAPI: 获取 PR 元数据
aiReview->>GitHubAPI: 创建占位评论
aiReview->>pi: 执行审查或继续会话
pi-->>aiReview: 返回审查结果
aiReview->>GitHubAPI: 更新占位评论
Merge Risk: 🟠 High · up to 该 PR 调整 CI 审查、Fork 隔离和依赖版本；当前仍存在受信任脚本恢复失败后执行 PR 可控脚本并暴露 Bot 凭据、令牌权限过宽等安全风险，且审查评论去重和清理流程可能导致门禁遗漏或工件未及时删除，因此当前版本不宜直接合并。 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
scripts/ai-review.ts (1)
97-132: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value下载的
session.zip未清理，且spawnSync的启动错误未上报。两点建议：
- 解压后删除
zipPath。该文件位于runnerTemp，与pi-session/同级，不会被上传，但会占用磁盘。- 如果
gh或unzip无法启动，spawnSync返回status === null且error有值。当前日志不含proc.error.message，排查困难。♻️ 建议改动
writeFileSync(zipPath, proc.stdout); const unzipProc = spawnSync("unzip", ["-q", "-o", zipPath, "-d", sessionDir], { maxBuffer: MAX_BUFFER_SIZE, }); + rmSync(zipPath, { force: true }); + if (unzipProc.status !== 0) { + process.stderr.write( + `解压会话工件失败（exit=${unzipProc.status}）：${unzipProc.error?.message || ""}\n`, + ); + } return unzipProc.status === 0;
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Pro Plus
Run ID: 56010c0a-7783-4d1f-9a80-22acd28a159c
📒 Files selected for processing (8)
.github/workflows/ai-review.yml.github/workflows/cache-cleanup.yml.github/workflows/checks.yml.github/workflows/nightly.yml.github/workflows/release.yml.github/workflows/review-gate.yml.github/workflows/security.ymlscripts/ai-review.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review.
| - name: 获取 Timi AI Bot 凭据 | ||
| id: bot-token | ||
| uses: actions/create-github-app-token@bcd2ba49218906704ab6c1aa796996da409d3eb1 # v3.2.0 | ||
| with: | ||
| app-id: ${{ secrets.TIMI_AI_APP_ID }} | ||
| private-key: ${{ secrets.TIMI_AI_PRIVATE_KEY }} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
两个工作流签发的 App token 都未限定范围。 actions/create-github-app-token 在缺省配置下签发覆盖该 App 全部安装权限、全部可访问仓库的 token。请在两处都显式声明 owner、repositories 和 permission-*，按最小权限收窄。
.github/workflows/ai-review.yml#L48-L53：添加owner、repositories，并限定为permission-issues: write与permission-actions: read（restoreSessionArtifact需要读取工件）。该 token 通过BOT_TOKEN传给scripts/ai-review.ts执行gh api，暴露面最大。.github/workflows/review-gate.yml#L104-L109：添加owner、repositories，并限定为permission-pull-requests: read与permission-issues: write。
🧰 Tools
🪛 zizmor (1.29.0)
[error] 50-50: dangerous use of GitHub App tokens (github-app): app token inherits blanket installation permissions
(github-app)
📍 Affects 2 files
.github/workflows/ai-review.yml#L48-L53(this comment).github/workflows/review-gate.yml#L104-L109
Source: Linters/SAST tools
| # 2. 物理销毁当前 PR 的 AI 审查会话工件 | ||
| gh api --paginate "repos/${GH_REPO}/actions/artifacts" \ | ||
| --jq ".artifacts[] | select(.name | startswith(\"pi-session-pr-${PR_NUMBER}\")) | .id" \ | ||
| | xargs -r -n 1 gh api --method DELETE "repos/${GH_REPO}/actions/artifacts/{}" | ||
| --jq ".artifacts[] | select(.name == \"pi-session-pr-${PR_NUMBER}\") | .id" \ | ||
| | xargs -r -I{} gh api --method DELETE "repos/${GH_REPO}/actions/artifacts/{}" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- workflow files ---'
git ls-files '.github/workflows/*'
printf '%s\n' '--- cache-cleanup.yml ---'
cat -n .github/workflows/cache-cleanup.yml
printf '%s\n' '--- ai-review.yml relevant sections ---'
cat -n .github/workflows/ai-review.yml | sed -n '1,125p'
printf '%s\n' '--- references to session artifact and cleanup ---'
rg -n -C 3 'pi-session-pr-|cache-cleanup|workflow_run|pull_request|actions/artifacts|always\(\)' .github/workflowsRepository: 0xTimi-labs/rust-template
Length of output: 15905
🌐 Web query:
GitHub Actions documentation concurrency cancel-in-progress workflow cancellation always() upload-artifact overwrite artifact retention
💡 Result:
In GitHub Actions, concurrency control, artifact management, and job execution behavior work as follows: Concurrency and Cancel-in-Progress The concurrency keyword limits how many workflow runs or jobs within the same concurrency group execute at once [1][2]. By default, a concurrency group allows at most one running job/workflow and one pending job/workflow [3][4]. If a new job is queued while another is pending, the existing pending job is cancelled and replaced by the new one [1][3]. Setting cancel-in-progress: true will also cancel any currently running job in that concurrency group [1][3]. You can also use conditional expressions with cancel-in-progress to dynamically control this behavior based on the branch or other context [3][4]. Note that cancel-in-progress cannot be used with the queue: max option [1][3]. Artifact Upload and Overwrite In version 4 and later of the upload-artifact action, artifacts are immutable by default [5][6][7]. If you attempt to upload an artifact with a name that already exists in the same workflow run, the action will fail [7]. To effectively "overwrite" an artifact, you must set overwrite: true in the upload-artifact action; this deletes the existing artifact before the new one is uploaded [8][9]. Note that this assigns the new artifact a different ID [5][6]. Artifact Retention By default, artifacts are retained for 90 days [10][11]. This period can be customized at the organization or repository level, ranging from 1 to 90 days for public repositories, or up to 400 days for private repositories [10]. You can also set a custom retention period for an individual artifact using the retention-days parameter within the upload-artifact action [12][11]. This custom value cannot exceed the maximum retention limit configured for the repository or organization [12]. Always Function The always expression is a function used in job-level or step-level if conditionals [1]. It ensures that a job or step runs even if previous steps have failed, were canceled, or were skipped [1]. This is commonly used for cleanup steps or reporting, such as uploading logs or artifacts even after a test failure. Top Results: [1], [3], [8], [7], [10], [12]
Citations:
- 1: https://docs.github.com/en/actions/how-tos/write-workflows/choose-when-workflows-run/control-workflow-concurrency
- 2: https://docs.github.com/en/actions/concepts/workflows-and-actions/concurrency
- 3: https://docs.github.com/en/enterprise-cloud@latest/actions/how-tos/write-workflows/choose-when-workflows-run/control-workflow-concurrency
- 4: Clarify concurrency cancel-in-progress behaviour github/docs#30647
- 5: https://github.com/actions/upload-artifact/blob/main/docs/MIGRATION.md
- 6: https://github.com/actions/upload-artifact
- 7: https://github.blog/news-insights/product-news/get-started-with-v4-of-github-actions-artifacts/
- 8: https://github.com/actions/upload-artifact/blob/main/action.yml
- 9: https://github.com/actions/upload-artifact?tab=readme-ov-file
- 10: https://docs.github.com/en/organizations/managing-organization-settings/configuring-the-retention-period-for-github-actions-artifacts-and-logs-in-your-organization
- 11: https://docs.github.com/actions/managing-workflow-runs/removing-workflow-artifacts
- 12: https://docs.github.com/en/actions/tutorials/store-and-share-data
🌐 Web query:
GitHub Actions upload-artifact overwrite true artifact deletion race behavior
💡 Result:
The overwrite: true option in GitHub Actions upload-artifact (starting from v4) is designed for sequential operations, not concurrent ones [1][2]. When enabled, the action attempts to delete any existing artifact with the same name before uploading the new one [3][4][5]. Crucially, this process is not atomic [1][2]. The implementation follows a "check-then-act" pattern: the action checks for the artifact, issues a delete command if it exists, and then initiates the upload [5]. This behavior leads to the following race conditions and limitations: 1. Parallel Execution Failure: If multiple parallel jobs attempt to upload an artifact with the same name using overwrite: true, they will frequently encounter race conditions [1][2]. For example, one job may trigger a deletion while another is in the middle of an upload, or two jobs might simultaneously attempt to delete the same artifact, causing one to fail with a conflict error (often a 409 error) [1][6][7]. 2. Non-Atomic Deletion: Because the deletion and subsequent upload are separate steps, the artifact name may be momentarily unavailable or subject to unexpected state changes if other processes are interacting with the repository's artifacts [1][2]. 3. Intended Use Case: The overwrite feature is intended for serial workflows where an artifact is updated in distinct, non-overlapping steps (e.g., uploading a file, downloading it for modification, then re-uploading it with the same name) [1][2]. For parallel jobs that produce results meant to be combined, you should use unique artifact names (e.g., incorporating matrix variables) and use the actions/upload-artifact/merge action to combine them into a single artifact after all jobs have finished [1][2][7].
Citations:
- 1: [bug] v4: overwrite: true fails with parallel jobs writing to the artifact actions/upload-artifact#506
- 2: [bug] v4: overwrite: true fails with parallel jobs writing to the artifact actions/upload-artifact#506
- 3: https://github.com/actions/upload-artifact/blob/main/action.yml
- 4: https://github.com/actions/upload-artifact?tab=readme-ov-file
- 5: https://github.com/actions/upload-artifact/blob/ef09cdac3e2d3e60d8ccadda691f4f1cec5035cb/src/upload/upload-artifact.ts
- 6: [bug] Artifact overwrite doesn't always work actions/upload-artifact#571
- 7: [bug] (v4) Unable to upload to same artifact name from multiple jobs actions/upload-artifact#478
🏁 Script executed:
#!/bin/bash
set -e
python3 - <<'PY'
from pathlib import Path
import re
cleanup = Path(".github/workflows/cache-cleanup.yml").read_text()
review = Path(".github/workflows/ai-review.yml").read_text()
print("cleanup trigger:", re.search(r"on:\s*\n\s*pull_request:\s*\n\s*types:\s*\[([^\]]+)\]", cleanup).group(1))
print("review triggers:", re.search(r"on:\s*\n((?:\s{2,}.*\n)+?)\nconcurrency:", review).group(1).strip())
print("cleanup concurrency:", "present" if re.search(r"^concurrency:", cleanup, re.M) else "absent")
print("review concurrency group:",
re.search(r"group:\s*(.+)", review).group(1).strip())
print("review cancellation:",
re.search(r"cancel-in-progress:\s*(.+)", review).group(1).strip())
print("upload condition:",
re.search(r"^\s*if:\s*(.+)", review[review.index("uses: actions/upload-artifact"):], re.M).group(1).strip())
print("upload overwrite:",
re.search(r"^\s*overwrite:\s*(.+)", review[review.index("uses: actions/upload-artifact"):], re.M).group(1).strip())
print("upload retention:",
re.search(r"^\s*retention-days:\s*(.+)", review[review.index("uses: actions/upload-artifact"):], re.M).group(1).strip())
PYRepository: 0xTimi-labs/rust-template
Length of output: 591
协调 PR 关闭与 AI 审查工件上传
PR 关闭事件不会取消独立的 AI Review 运行。清理任务只删除查询时存在的工件；AI Review 随后可能通过 if: always() 和 overwrite: true 重新创建 pi-session-pr-${PR_NUMBER}，导致工件保留 15 天。
请在删除前取消并等待相关运行结束，并阻止已取消运行上传工件；或在 AI Review 完成事件中执行幂等清理，覆盖上传与删除之间的竞态。
| if (comments.some((c) => c.body?.includes(MARKER))) { | ||
| core.info('跳过：已存在自动审查命令评论。'); | ||
| return; | ||
| } | ||
|
|
||
| await github.rest.issues.createComment({ | ||
| owner, | ||
| repo, | ||
| issue_number: issueNumber, | ||
| body: '@coderabbitai review', | ||
| body: `${MARKER}\n@coderabbitai review`, | ||
| }); | ||
| await github.rest.issues.createComment({ | ||
| owner, | ||
| repo, | ||
| issue_number: issueNumber, | ||
| body: '@greptileai review', | ||
| body: `${MARKER}\n@greptileai review`, | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
两条评论共用同一个 marker，部分失败后无法补发。
第 151 行和第 157 行使用相同的 MARKER。第 142 行只要发现任意一条带 marker 的评论就整体跳过。
如果 @coderabbitai 评论创建成功而 @greptileai 评论失败（限流、网络错误），重跑该工作流会在第 142 行命中第一条评论并直接返回。@greptileai review 命令将永久缺失。
请为每个审查工具使用独立 marker，并分别判断。
🐛 建议修复：按工具分别去重
- if (comments.some((c) => c.body?.includes(MARKER))) {
- core.info('跳过：已存在自动审查命令评论。');
- return;
- }
-
- await github.rest.issues.createComment({
- owner,
- repo,
- issue_number: issueNumber,
- body: `${MARKER}\n@coderabbitai review`,
- });
- await github.rest.issues.createComment({
- owner,
- repo,
- issue_number: issueNumber,
- body: `${MARKER}\n@greptileai review`,
- });
+ const targets = [
+ { id: 'coderabbit', command: '`@coderabbitai` review' },
+ { id: 'greptile', command: '`@greptileai` review' },
+ ];
+ for (const target of targets) {
+ const marker = `<!-- review-gate: initial-trigger:${target.id} -->`;
+ if (comments.some((c) => c.body?.includes(marker))) {
+ core.info(`跳过：已存在 ${target.id} 审查命令评论。`);
+ continue;
+ }
+ await github.rest.issues.createComment({
+ owner,
+ repo,
+ issue_number: issueNumber,
+ body: `${marker}\n${target.command}`,
+ });
+ }同时删除第 120 行的 MARKER 常量。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (comments.some((c) => c.body?.includes(MARKER))) { | |
| core.info('跳过：已存在自动审查命令评论。'); | |
| return; | |
| } | |
| await github.rest.issues.createComment({ | |
| owner, | |
| repo, | |
| issue_number: issueNumber, | |
| body: '@coderabbitai review', | |
| body: `${MARKER}\n@coderabbitai review`, | |
| }); | |
| await github.rest.issues.createComment({ | |
| owner, | |
| repo, | |
| issue_number: issueNumber, | |
| body: '@greptileai review', | |
| body: `${MARKER}\n@greptileai review`, | |
| }); | |
| const targets = [ | |
| { id: 'coderabbit', command: '@coderabbitai review' }, | |
| { id: 'greptile', command: '@greptileai review' }, | |
| ]; | |
| for (const target of targets) { | |
| const marker = `<!-- review-gate: initial-trigger:${target.id} -->`; | |
| if (comments.some((c) => c.body?.includes(marker))) { | |
| core.info(`跳过：已存在 ${target.id} 审查命令评论。`); | |
| continue; | |
| } | |
| await github.rest.issues.createComment({ | |
| owner, | |
| repo, | |
| issue_number: issueNumber, | |
| body: `${marker}\n${target.command}`, | |
| }); | |
| } |
|
@coderabbitai review |
|
@greptileai review |
✅ Action performedReview finished.
|
|
|
||
| - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 | ||
| - name: 恢复受信任的审查脚本 | ||
| run: git checkout origin/main -- scripts/ai-review.ts 2>/dev/null || true |
There was a problem hiding this comment.
本 PR 合并前，origin/main 尚不存在新增的 scripts/ai-review.ts，因此恢复命令会失败；|| true 会吞掉该错误并保留 PR head 中的脚本，随后在 GitHub App token 和 PI 凭据已经准备好的情况下执行它，导致被审代码能够读取或外传这些凭据，且脚本内部的 Fork 检查也可被恶意版本删除。
How this was verified: 当前主分支不包含该路径，而工作流会在恢复失败后继续执行工作区中的 PR head 脚本。
There was a problem hiding this comment.
Actionable comments posted: 2
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/ai-review.ts (1)
198-205: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win不要声称已保存完整审查输出。
当输出超过
MAX_COMMENT_LENGTH时，代码只写入截断后的body。proc.stdout从未写入 Actions 日志，且.github/workflows/ai-review.yml的 Lines 85-90 只上传pi-session，不会上传review-output.md。因此完整审查内容会永久丢失。请将完整输出写入会话工件，或显式输出到日志，并更新评论中的存储位置说明。
🟠 Other critical/major comments (1)
.github/workflows/ai-review.yml-46-46 (1)
46-46: 🔒 Security & Privacy | 🟠 Major同时隔离 PR 可控的 Pi 项目配置。
这里只恢复
scripts/ai-review.ts。Line 83 仍在 PR 工作区执行pi --approve，因此.pi/settings.json、项目扩展和REVIEW_GUIDELINES.md仍可能来自 PR。这些资源仍可影响凭据化审查。请从origin/main恢复受信任的项目资源，或在干净目录运行 Pi，并确认--approve不加载 PR 资源。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: QUIET
Plan: Pro Plus
Run ID: 399f9e40-75fc-4766-9c3a-bd29e64ae200
📒 Files selected for processing (2)
.github/workflows/ai-review.ymlscripts/ai-review.ts
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.
|
|
||
| - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 | ||
| - name: 恢复受信任的审查脚本 | ||
| run: git checkout origin/main -- scripts/ai-review.ts 2>/dev/null || true |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
恢复受信任脚本失败时必须终止任务。
当前 git checkout origin/main -- scripts/ai-review.ts 的失败被 || true 忽略。若 origin/main 不可用或文件恢复失败，工作区仍会保留 PR 修改的 scripts/ai-review.ts，但后续 Line 83 会执行该文件，并将 BOT_TOKEN 注入其进程。恶意 PR 可借此执行任意脚本并窃取 Bot token。请移除 || true，让恢复失败立即终止任务。
|
@coderabbitai review |
|
@greptileai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
|
@greptileai review |
✅ Action performedReview finished.
|
1 participant
完善门禁签名去重、Fork 隔离与 Actions 依赖版本升级。
Closes #6