Skip to content

fix(ci): add allow-unsafe-pr-checkout for pull_request_target#33

Open
watney1024 wants to merge 2 commits into
mainfrom
zmz_dev
Open

fix(ci): add allow-unsafe-pr-checkout for pull_request_target#33
watney1024 wants to merge 2 commits into
mainfrom
zmz_dev

Conversation

@watney1024

Copy link
Copy Markdown
Collaborator

背景

PR #31 将 AI Code Review workflow 的 trigger 从 pull_request 改为 pull_request_target,使 fork PR 能访问 SENSENOVA_API_KEY。但 actions/checkout@v4 新增了安全保护,默认拒绝在 pull_request_target 下 checkout fork 代码。

变更

在 checkout 步骤添加 allow-unsafe-pr-checkout: true,显式放行。

安全说明

此 workflow 对 fork 代码只做只读操作,不存在 "pwn request" 风险:

操作 风险 说明
git diff ✅ 无 纯文本 diff,不执行任何代码
file -b --mime-encoding ✅ 无 读文件头判断类型
curl → SenseNova API ✅ 无 API key 在 HTTP Header,diff 在 body
make / python / ./script.sh ✅ 不存在 workflow 中没有编译或执行步骤
脚本来源 ✅ base 分支 所有 run: 命令 inline 在 workflow yml 中,fork 无法篡改

@github-actions

github-actions Bot commented Jul 26, 2026

Copy link
Copy Markdown

🤖 AI Code Review

共审查 1 个变更文件

📁 .github/workflows/ai-code-review.yml

🔴 jq 注入风险 — 第 2 行(AI Code Review 步骤):jq -r '.[] | select(.filename == "'"$FILE"'") | .patch // empty' 使用字符串拼接传递文件名,若文件名含引号或反斜杠会导致 jq 解析错误或意外行为。建议使用 jq --arg f "$FILE" '.[] | select(.filename == $f) | .patch // empty'

🔴 缺失分页处理gh api "/repos/.../pulls/$PR_NUMBER/files" 默认只返回前 100 个文件。若 PR 超过 100 个文件,后续文件会被忽略。应添加 --paginate 选项或循环处理分页。

🟡 API 调用失败未处理gh api 命令没有检查退出状态。若网络错误或权限不足导致调用失败,pr_files.json 可能为空或不存在,后续步骤会引发未定义行为。建议添加 || exit 1 或更友好的错误处理。

🟡 wc -l 计数可能不准确wc -l < changed_files.txt 如果文件最后一行无换行符会少计一行(虽常见 jq 输出有换行,但更健壮的做法是 jq 'length' 直接获取文件总数)。

💭 错误日志丢失文件大小信息 — 之前输出 FILE_SIZE 有助于调试大文件问题,现在移除了,对排查性能或 API 限制问题不够友好。


Replace git diff from fork checkout with gh api to get PR file diffs remotely. This eliminates the need for allow-unsafe-pr-checkout and ref: head.sha entirely. The API handles binary/large file detection automatically via null .patch fields.

Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent)

Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
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.

1 participant