chore(ci): pin third-party actions to commit SHAs (#440) - #451
Merged
Conversation
#437's mechanism was a moving tag: `actions/checkout@v4` silently began refusing fork checkouts under `pull_request_target`, and Repo Guard failed on every fork PR with no change on our side. The regression test written for it asserts workflow *text*, and Actions only warns on unknown `with:` keys rather than failing — so if the tag moves again and `allow-unsafe-pr-checkout` is renamed or dropped, checkout breaks while `test:workflows` stays green. That is the same mechanism that hid #437 in the first place. Every third-party action now carries a SHA plus a trailing version comment, and a contract test requires both halves — the SHA so behavior is frozen, the comment so a future upgrade can tell how far it is jumping. Verified that reverting either half fails the test. Pinned to each action's *current* major, not the latest release: `checkout` is at v7.0.1 upstream and this stays on v6.1.0. A pin commit that also bumps majors is a behavior change wearing a chore's clothes. The two `actions/checkout@v4` usages (repo-guard, update-npm-downloads) move to the same v6 SHA as everything else. Confirmed `allow-unsafe-pr-checkout` is present in action.yml at both the old v4.4.0 and the new v6.1.0 SHAs, so the input #438 depends on survives the jump. `ceilf6/repo-guard@main` stays on a moving ref and is allowlisted explicitly. It is first-party, and pinning it would mean a bump PR here for every repo-guard change; it is also the step that runs on the self-hosted runner with `secrets.LLM_API_KEY`, so the trade is real. That is a maintainer policy call — recorded next to the allowlist, not decided here. Closes #440 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
代码评审报告: chore(ci): pin third-party actions to commit SHAs (#440)
风险等级: 中
处理建议: 需要人工判断
决策摘要: 钉 SHA 本身干净且有契约测试保护，但 actions/checkout v4→v6 的两处跨大版本迁移只验证了 allow-unsafe-pr-checkout 这一个输入，两个真正决定这两条流程能否跑通的前提（自托管 runner 的 node24 支持、update-npm-downloads.yml 依赖的持久化凭据）在仓库内无法验证，需要维护者确认后再合。
级联分析
- 变更符号: 无应用符号。变更面为 6 个 workflow 的
uses:ref、scripts/tests/workflow-rules.test.mjs新增一条契约测试、CHANGELOG 一行。 - 受影响流程: CI（check/package）、contract-guard、gitguardian、release（upload/download/gh-release）、repo-guard（自托管 runner +
secrets.LLM_API_KEY）、update-npm-downloads（定时任务，会git push）。 - 变更集外调用方: 仅 GitHub Actions 运行时。仓库内 grep 确认
.github/下全部 20 条uses:均在本 diff 覆盖范围内，无遗漏的 workflow 文件、无本地 composite action、无 reusable workflow 调用 (text search)。 - 置信度: medium — 变更面已穷举，但上游 action 在钉定 SHA 处的实际默认行为无法在本 checkout 内验证，只能依赖 PR 作者提供的证据。
问题发现
-
[高] repo-guard 的 checkout 从 v4 跨到 v6，把自托管 runner 的 node24 支持变成了新的前提
- 证据:
.github/workflows/repo-guard.yml:75由@v4（node20 运行时）改为 v6.1.0 SHA。该 job 跑在runs-on: ${{ vars.REPO_GUARD_RUNNER || 'ubuntu-latest' }}（repo-guard.yml:52，注释指明是自托管repo-guard-claude）。actions/checkout在 v5.0.0 起改用 node24 运行时，对 runner 版本有下限要求；GitHub 托管 runner 自动满足，自托管 runner 只有在开启自动更新且已升级过时才满足。 - 受影响调用方/流程: Repo Guard 对所有 PR（含 fork PR）的 checkout step。失败形态与 #437 完全一致——job 在 checkout 处红，review step skipped，仓库这边零改动。
- 最小可行修复: 合并前确认
repo-guard-claude的 runner 版本满足 node24（或确认未--disableupdate）。若无法确认，repo-guard 这一处保留在 v4 的 SHA（例如 PR 中已核对过allow-unsafe-pr-checkout的 v4.4.0 SHA），把对齐 v6 拆成单独变更——这样本 PR 仍完成 #440 的第 1 项（消除移动 tag），风险面为零。
- 证据:
-
[中]
update-npm-downloads.yml依赖 checkout 持久化的凭据做git push，而这次跨大版本迁移只验证了另一个输入- 证据:
.github/workflows/update-npm-downloads.yml:15同样 v4→v6，该 workflow 在:29直接git push，没有显式token:、也没有persist-credentials设置，完全依赖 checkout 写入的默认凭据。PR 的验证段落只覆盖了allow-unsafe-pr-checkout在两个 checkout SHA 上的存在性，没有覆盖凭据持久化这条完全不同的默认行为；persist-credentials的默认值正是 checkout 跨大版本最容易发生的硬化型变更。 - 受影响调用方/流程: 每日 01:17 的定时任务。它没有任何断言或告警，退化后只在 Actions 历史里可见，README 计数会静默停更。
- 最小可行修复: 在 v6.1.0 的
action.yml上确认persist-credentials默认仍为true；如果不是，在这一步显式加persist-credentials: true（或改用显式 token）。这是一行修改，与本 PR 的 chore 性质相容。
- 证据:
-
[低] 契约测试的枚举面窄于它要守护的契约
- 证据:
scripts/tests/workflow-rules.test.mjs:452只收.yml；新增的x.yamlworkflow 会静默豁免整条钉 SHA 规则。:467用split('@')[1] ?? ''求 ref，本地 action / reusable workflow（uses: ./.github/actions/foo）没有@，会被判为 offender，而它们在结构上无法钉 SHA，唯一的绕过途径是往FIRST_PARTY_UNPINNED里塞一个语义不符的条目。 - 受影响调用方/流程: 仅
pnpm test:workflows。当前仓库没有.yamlworkflow 也没有本地 action，所以今天不影响任何流程；这是这条门禁未来失效/误报的方式。 - 最小可行修复: 过滤改为
/\.ya?ml$/u，并在取 ref 前if (match[1].startsWith('./')) continue;。两行。
- 证据:
行级发现
- [.github/workflows/repo-guard.yml:75] v4→v6 把 checkout 带到 node24 运行时，而这一 job 跑在自托管 runner 上；合并前确认该 runner 版本支持 node24，否则这一步会对所有 PR 红在 checkout，复现 #437 的现象。
- [.github/workflows/update-npm-downloads.yml:15] 这一步的凭据被
:29的git push使用，跨大版本迁移需要额外确认 v6.1.0 的persist-credentials默认值仍是true，否则改为显式置位。 - [scripts/tests/workflow-rules.test.mjs:452] 只匹配
.yml，新增.yamlworkflow 会绕过整条钉 SHA 契约；改为/\.ya?ml$/u。 - [scripts/tests/workflow-rules.test.mjs:467] 无
@的本地 action / reusable workflow ref 会被误判为未钉；在取 ref 前跳过./开头的uses。
Karpathy 评审
- 假设: 核心未验证假设有两个，都与 v4→v6 相关——自托管 runner 满足新运行时要求（发现 1），以及默认凭据行为不变（发现 2）。PR 明确验证了第三个假设（
allow-unsafe-pr-checkout在新旧 SHA 均存在），但把它当成了整个大版本跳跃的充分证据。「钉在当前 major 而非最新 release」的取舍说明清楚且正确。 - 简洁性: 无猜测性抽象。契约测试是一个自包含函数，约 25 行，无新增依赖，仅新增
readdirSync一个 import。 - 结构质量: 无退化。测试文件维持原有「逐条 test + 中文根因注释」的模式，没有引入 helper 层或共享状态；workflow 侧只改
uses:行，逻辑、触发条件、权限、job 条件均未动，与 PR 描述一致（grep 复核确认）。 - 变更范围: 超出 #440 字面提到的两处
@v4，但 issue 第 2 项显式把「全部钉 SHA」列为待决选项之一，因此在授权范围内。ceilf6/repo-guard@main留白并把理由写进测试注释、标为维护者策略决定，处理得当——且该豁免以完整字符串匹配，ref 一旦变动就会 fail-closed。 - 验证: 契约测试做了双向证伪（回退一条到
@v6报ci.yml: actions/checkout@v6；去掉版本注释报(no version comment)），这是这类文本断言应有的强度。缺口在于：新测试守的是「refs 是 SHA 形式」，而本 PR 引入的真实风险是「跳过两个 major 后行为是否不变」，后者没有、也难有自动化覆盖，只能靠合并前的人工确认。
缺失覆盖
- 合并前需人工确认（无法用测试代替）：
repo-guard-clauderunner 版本满足 checkout v6 的 node24 要求；v6.1.0 的persist-credentials默认值。 - 契约测试建议补两个用例：
.yaml扩展名的 workflow 会被检查；uses: ./...形式不被误判。 - Repo Guard 对 fork PR 的端到端确认只能在合并后由一个真实 fork PR 提供，这一点 PR 已如实说明，不作为阻塞项。
| # 一个无条件置位的开关会把 issue_comment 这条边界更宽的路径也静默放行。 | ||
| # 表达式渲染为布尔字面量 true / false。 | ||
| - uses: actions/checkout@v4 | ||
| - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 |
There was a problem hiding this comment.
v4→v6 把 checkout 带到 node24 运行时，而这一 job 跑在自托管 runner 上；合并前确认该 runner 版本支持 node24，否则这一步会对所有 PR 红在 checkout，复现 #437 的现象。
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: actions/checkout@d23441a48e516b6c34aea4fa41551a30e30af803 # v6.1.0 |
There was a problem hiding this comment.
这一步的凭据被 :29 的 git push 使用，跨大版本迁移需要额外确认 v6.1.0 的 persist-credentials 默认值仍是 true，否则改为显式置位。
| // 改名或删掉,fork PR 会在 checkout 处坏掉而 test:workflows 照旧全绿,正是藏住 | ||
| // #437 的那套机制。所以这里要求的是 SHA,不是「某个 tag 名」。 | ||
| test('third-party actions are pinned to a commit SHA', () => { | ||
| const workflows = readdirSync('.github/workflows').filter((file) => file.endsWith('.yml')); |
There was a problem hiding this comment.
只匹配 .yml，新增 .yaml workflow 会绕过整条钉 SHA 契约；改为 /\.ya?ml$/u。
| for (const line of text.split('\n')) { | ||
| const match = /^\s*(?:-\s*)?uses:\s*(\S+)/u.exec(line); | ||
| if (!match) continue; | ||
| const [owner, ref] = [match[1], match[1].split('@')[1] ?? '']; |
There was a problem hiding this comment.
无 @ 的本地 action / reusable workflow ref 会被误判为未钉；在取 ref 前跳过 ./ 开头的 uses。
repo-guard on 311e178, all three findings. 1. Aligning repo-guard's checkout to v6 quietly added a precondition I cannot check: v5.0.0 moved the action to a node24 runtime, and that job runs on `vars.REPO_GUARD_RUNNER` — a self-hosted runner whose version is not visible from this repo. The failure mode would be identical to #437: red at checkout, review skipped, nothing changed on our side. Both `@v4` usages now pin to the v4.4.0 SHA instead. #440's first item (kill the moving tag) is still done; its second (align to v6) is deferred to a change that can actually verify the runner, which is the honest answer rather than a guess. 2. Same reasoning covers `update-npm-downloads.yml`, which `git push`es on credentials `checkout` persists with no explicit token. Staying on v4.4.0 means that default does not move. For the record, `persist-credentials` defaults to `true` at both the v4.4.0 and v6.1.0 SHAs — checked — so the alignment would have been safe on that axis; it is the node24 axis that is unverifiable, and the two usages should move together. 3. The contract test now matches `/\.ya?ml$/` (a new `x.yaml` workflow would have been silently exempt) and skips local actions and reusable workflows (`uses: ./…`), which have no `@ref` and were being reported as offenders with no legitimate way to comply. Refs #440 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
Author
|
三条都已处理（ 发现 1（高，v4→v6 把自托管 runner 的 node24 支持变成新前提） — 采纳，两处 checkout 保留在 v4.4.0 的 SHA。 我原本的理由是「仓库其余五处已经在 v6，落下两处是 #440 点名的不一致」——但那是审美一致性，而你指出的是一个我在这个仓库里无从验证的前提：v5.0.0 起 checkout 改用 node24 运行时，而这个 job 跑在 所以本 PR 现在只做 #440 的第 1 项（消灭移动 tag），行为零变化；第 2 项（对齐 v6）留给一个能真正验证 runner 的独立变更。这是我对那个开放问题的答复，附理由。 发现 2（中， 顺带把你要的那项查了，记录在案： 发现 3（低，契约测试枚举面偏窄） — 已修，两处都按你给的写法：文件过滤改为 门禁： |
There was a problem hiding this comment.
代码评审报告: chore(ci): pin third-party actions to commit SHAs (#440)
风险等级: 中
处理建议: 评论
决策摘要: 可以合并——#440 的两项都被正面回答（item 1 钉 SHA、item 2 选择「就地全部钉 SHA」而非升 v6），全部 20 条 uses: 已核对无遗漏、无行为改动；剩余问题都是非阻塞的：新契约测试的版本注释正则未锚定、release.yml 三条 pin 在合并前不会被执行，以及「刻意留在 v4」这个决定只写在 PR/CHANGELOG 里、没有落到 dependabot 会撞到的地方。
级联分析
- 变更符号: 无应用符号。仅 6 个 workflow 的
uses:ref +scripts/tests/workflow-rules.test.mjs新增 1 个契约测试 + CHANGELOG 一行。 - 受影响流程: GitHub Actions 自身是唯一消费者。已确认
.github/actions/不存在（无本地 composite action 会携带未被扫描的uses:）；仓库内引用.github/workflows的只有scripts/workflows/contract-rules.mjs（按路径前缀分类，不解析文件内容）、contract-guard.yml/release.yml的 paths 过滤、CODEOWNERS与本测试文件——都不读uses:值。(text search) - 变更集外调用方: 无。
grep uses: .github/workflows返回 20 行，与 diff 一一对应，无遗漏、无run: |块内的伪uses:会被新测试误报。(text search) - 既有断言未被打破:
repo guard can check out fork PRs from allowlisted contributors（:840）用/- uses: actions\/checkout@[\s\S]*?(?=\n {6}- |$)/抓 step 块，SHA 形式仍匹配，终止 lookahead 落在repo-guard.yml:81的 6 空格- uses:上，仍然正确切分。(read) - 置信度: medium — diff 侧的调用面已穷尽核对；但 SHA↔版本号的映射无法在本 checkout 内离线验证，只能采信作者的
action.yml拉取记录。
问题发现
-
[中] 版本注释断言未锚定，
# 见 #440这类注释即可冒充版本号- 证据:
scripts/tests/workflow-rules.test.mjs:475的/#\s*v?\d+(\.\d+)*/u在整行上做无锚点匹配。本仓库注释里 issue 引用（#437、#440、#441）随处可见，- uses: some/action@<sha> # pinned per #440会匹配#440而通过。 - 受影响调用方/流程: 该断言的唯一目的（:474 注释「升级时无从判断跨了多少」）在这种写法下失效，而 offender 列表仍为空。
- 最小可行修复: 锚到行尾，要求版本号是末尾 token：
/#\s*v?\d+(\.\d+)*\s*$/u。当前 8 种 pin 全部是# vX.Y.Z结尾，改后仍全绿。
- 证据:
-
[中] 「刻意停在 v4」的约束没有落到 dependabot 会撞到的位置
- 证据:
.github/dependabot.yml:27-33的github-actions生态没有ignore规则（major 忽略只写在 npm 那一段，:18-21），所以 dependabot 会照常提actions/checkout跨 major 的 bump PR。而.github/workflows/repo-guard.yml:75与update-npm-downloads.yml:15的 pin 行上，没有任何文字说明为什么停在 v4.4.0——理由只存在于 PR 描述和 CHANGELOG 里。新契约测试只校验「是 SHA + 有版本注释」，不校验 major，因此 bump 后仍然全绿。 - 受影响调用方/流程: 叠加 #443 之后，dependabot 作者的 PR 被 Contract Guard 跳过，而 CI 是唯一 required check，且 CI 不执行
repo-guard.yml/update-npm-downloads.yml。这条路径上没有任何门禁能发现 node24 runtime 跳变——正是本 PR 论证要避免的失败形态。 - 最小可行修复: 在这两行 pin 上各加一句「hold at v4.4.0：v5.0.0 起为 node24 runtime，self-hosted runner 版本不可见（#440 item 2）」；或在
dependabot.yml的 github-actions 段加ignore: actions/checkout / version-update:semver-major。二选一即可，前者成本更低且解释出现在 review diff 里。
- 证据:
-
[低]
release.yml的三条 pin 在合并前不会被任何 job 执行- 证据:
release.yml:58,74,80只在 tag 推送时运行（:68startsWith(github.ref, 'refs/tags/')），gitguardian.yml:22,27只在 push 后运行，update-npm-downloads.yml:15只在 cron/dispatch 运行。本 PR 自身的检查只覆盖ci.yml、contract-guard.yml与repo-guard.yml的 pin。SHA 是否真的指向注释所声明的 tag，在本 checkout 内无法离线核验。 - 受影响调用方/流程: 若
upload-artifact@043fb46d/download-artifact@d3f86a10/action-gh-release@3d0d9888中任一 SHA 有误，首次暴露点是发版当天的 release 流水线。 - 最小可行修复: 合并前贴一次
git ls-remote https://github.com/<owner>/<repo> refs/tags/vX.Y.Z的输出（逐条对照 SHA），作为不可自动化的那部分证据留档。不建议把网络校验塞进契约测试。
- 证据:
-
[低]
uses:值被引号包裹时会误报为未 pin- 证据:
scripts/tests/workflow-rules.test.mjs:466-468用(\S+)捕获后直接split('@')[1]。uses: 'actions/checkout@<40hex>'得到的 ref 末尾带'，/^[0-9a-f]{40}$/不匹配，一个完全正确的 pin 会被列进 offenders。 - 受影响调用方/流程: 今天没有引号写法，所以是潜在误报而非现存失败；但它会把一次合法改动变成硬红，而报错信息（
file: 'actions/checkout@sha'）不指向真正原因。 - 最小可行修复: 捕获后剥一层引号，例如
match[1].replace(/^['"]|['"]$/gu, '')。
- 证据:
-
[低] 唯一留下的移动 ref 恰是权限最高的那一个
- 证据:
repo-guard.yml:81的ceilf6/repo-guard@main被FIRST_PARTY_UNPINNED（:459）显式豁免。它跑在vars.REPO_GUARD_RUNNER自托管 runner 上、env 携带secrets.LLM_API_KEY（:88）。 - 受影响调用方/流程: 本 PR 消除的是第三方漂移；第一方漂移的爆炸半径反而更大。这不在 #440 的 scope 内，PR 也明确写了「recorded, not decided」，因此不作为阻塞项。
- 最小可行修复: 无需在本 PR 处理；建议维护者在合并时给出明确表态，或开一条独立 issue，避免这个豁免因为写在测试里而被默认为已定论。
- 证据:
行级发现
- [scripts/tests/workflow-rules.test.mjs:475] 版本注释正则在整行上无锚点匹配，
# per #440这类注释里的 issue 号会被当成版本号通过；锚到行尾/#\s*v?\d+(\.\d+)*\s*$/u。 - [scripts/tests/workflow-rules.test.mjs:468]
(\S+)捕获后直接split('@')，带引号的uses: 'owner/action@<sha>'会因尾引号被误判为未 pin；捕获后先剥一层引号。 - [.github/workflows/repo-guard.yml:75] 停在 v4.4.0 的理由（v5 起 node24、self-hosted runner 版本不可见）只写在 PR/CHANGELOG，dependabot 的 github-actions 段无 major ignore、新契约测试也不校验 major；在本行加一句 hold 说明，或加 dependabot ignore（
update-npm-downloads.yml:15同因，两处应一起）。
Karpathy 评审
- 假设: 两个关键假设无法在本 checkout 内证伪——
allow-unsafe-pr-checkout存在于11d5960a，以及@v4当前恰好解析到 v4.4.0（否则「零行为变化」会有一个小落差）。作者声明已逐个拉取action.yml核对，采信但不能复现；这是把置信度定在 medium 而非 high 的原因。反过来说，node24 这条不可验证的前提被识别出来并主动回避，判断是对的。 - 简洁性: 契约测试是单层循环、无抽象、无配置，与文件里其余直接断言的风格一致。没有猜测性通用化。
- 结构质量: 无退化。测试文件从 931 → 970 行，仍在 1000 行以下，本次不构成拆分阈值问题；但已逼近，下一个同类测试进来时值得先拆（例如把 workflow 文本类断言分出一个文件）。新测试没有复制既有 helper，也没有把逻辑放到错误层。
- 变更范围: 严格贴着 #440。没有顺手升 major、没有格式噪声、没有无关重构。「pin where they are, not to latest」这个克制是本 PR 最正确的一处决定：把 pin 和 bump 分开，让后者成为可独立验证的变更。
- 验证: 契约测试双向证伪过（回退一条 →
ci.yml: actions/checkout@v6；去掉注释 →(no version comment)），这比只报一次绿更有说服力。缺口在上面第 1、3、4 条：正则本身有一条未被证伪的松口，且一半的 pin 在合并前不会被执行。
缺失覆盖
- 版本注释断言缺一个「注释里只有 issue 号、没有版本号」的反例；补上就能挡住第 1 条。
release.yml/gitguardian.yml/update-npm-downloads.yml的 5 条 pin 缺合并前的 SHA↔tag 对照证据（git ls-remote输出即可），这部分无法由测试覆盖。- fork PR 上 Repo Guard 的端到端确认按定义只能在合并后由一个 fork PR 给出——这一点 PR 已如实声明，不要求在合并前解决。
| if (FIRST_PARTY_UNPINNED.has(owner)) continue; | ||
| if (!pinned.test(ref)) offenders.push(`${file}: ${owner}`); | ||
| // 光有 SHA 读不出这是哪个版本,升级时无从判断跨了多少。要求尾注版本号。 | ||
| else if (!/#\s*v?\d+(\.\d+)*/u.test(line)) |
There was a problem hiding this comment.
版本注释正则在整行上无锚点匹配，# per #440 这类注释里的 issue 号会被当成版本号通过；锚到行尾 /#\s*v?\d+(\.\d+)*\s*$/u。
| for (const line of text.split('\n')) { | ||
| const match = /^\s*(?:-\s*)?uses:\s*(\S+)/u.exec(line); | ||
| if (!match) continue; | ||
| const [owner, ref] = [match[1], match[1].split('@')[1] ?? '']; |
There was a problem hiding this comment.
(\S+) 捕获后直接 split('@')，带引号的 uses: 'owner/action@<sha>' 会因尾引号被误判为未 pin；捕获后先剥一层引号。
| # 一个无条件置位的开关会把 issue_comment 这条边界更宽的路径也静默放行。 | ||
| # 表达式渲染为布尔字面量 true / false。 | ||
| - uses: actions/checkout@v4 | ||
| - uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0 |
There was a problem hiding this comment.
停在 v4.4.0 的理由（v5 起 node24、self-hosted runner 版本不可见）只写在 PR/CHANGELOG，dependabot 的 github-actions 段无 major ignore、新契约测试也不校验 major；在本行加一句 hold 说明，或加 dependabot ignore（update-npm-downloads.yml:15 同因，两处应一起）。
…where dependabot lands repo-guard on 2c2a71f, findings 1 and 3. The version-comment assertion matched `#\s*v?\d+` anywhere on the line, so a comment like `# 见 #440` satisfied it and the check was decorative. Anchored to the text immediately following the pinned SHA; verified that a non-version comment now fails. The "deliberately staying on v4" reasoning lived only in the PR body and CHANGELOG — neither of which is in front of the person who has to act on it. It now sits directly above both pins, which is exactly what a dependabot PR bumping them will show in its diff: v5.0.0 moved checkout to node24, the repo-guard job runs on a self-hosted runner whose version is not visible from here, and guessing wrong reproduces #437 byte for byte. The note names the precondition to check before merging such a bump. Not addressed: `release.yml`'s three pins do not execute on PRs (its publish job is tag-gated), so they stay unexercised until the next version tag. That is a property of the workflow's triggers, not something this change can fix — flagged rather than papered over. Refs #440 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Owner
Author
|
三条都已处理（ 发现 1（版本注释正则未锚定） — 说得对，这条断言此前是装饰性的： 发现 3（「刻意留在 v4」只写在 PR/CHANGELOG 里） — 这条最实用。理由此前只在两个要动手的人看不到的地方。现在直接写在两处 pin 的正上方，也就是 dependabot 提升级 PR 时 diff 里会显示的位置，并写明合之前要先确认什么（自托管 runner 的 node24 支持）。 发现 2（ 门禁： |
There was a problem hiding this comment.
代码评审报告: chore(ci): pin third-party actions to commit SHAs (#440)
风险等级: 中
处理建议: 评论
决策摘要: ** 可以合并——diff 严格限于 uses: ref、一条契约测试和 changelog，所有 major 版本原地保留，我用 grep 核对了 6 个 workflow 的全部 20 条 uses: 行，除显式允许的 ceilf6/repo-guard@main 外都是 40-hex SHA + 尾注版本，新测试应当通过；合并前建议修正 update-npm-downloads.yml 里那段事实不成立的保留理由注释，并确认 release publish job 的两个 pin 确实解析得到。
级联分析
- 变更符号: 原始模型未提供结构化级联字段。
- 受影响流程: 原始模型未提供结构化级联字段。
- 变更集外调用方: unknown
- 置信度: degraded
问题发现
- [中]
update-npm-downloads.yml的保留理由与该 workflow 实际情况不符- 证据:
.github/workflows/update-npm-downloads.yml:15-17写「同 repo-guard.yml 的理由…升级前先确认自托管 runner 满足 node24」，但该 job 在:13是runs-on: ubuntu-latest，是 GitHub 托管 runner，node24 前置条件不适用；第二条理由「git push依赖 checkout 持久化的默认凭据」也被 PR 描述自己的核对结果否定（v4.4.0 与 v6.1.0 的persist-credentials默认都是true）。 - 受影响调用方/流程: 这条注释不是装饰——
.github/dependabot.yml:27-33的 github-actions 生态没有 npm 生态那样的 major ignore（:18-21只覆盖 npm），所以 dependabot 一定会对这两处 v4 提 major 升级 PR，而这段注释是评审者当时唯一的判断依据。理由写错会让人去追一个不存在的前置条件，或反过来认为「查过 runner 了」就把两处一起合掉。 - 最小可行修复: 把该注释改成本 workflow 成立的理由——与 repo-guard 的 checkout 同版本、按 #440 item 2 一起动，托管 runner 侧无 node24 阻塞；
v5.0.0 → node24 + 自托管 runner的论证只保留在repo-guard.yml:75-78。
- 证据:
- [低] release publish job 的两个 pin 在合并前不会被任何一次运行触及
- 证据:
release.yml:64-68的 publish job 是if: startsWith(github.ref, 'refs/tags/')，只在 tag push 时跑；:74的download-artifact@d3f86a10和:80的action-gh-release@3d0d9888因此不在本 PR 的 CI 里执行。build job 会跑（release.yml在pull_request的 paths 里），所以:35/:37/:39/:58四个 pin 有 pre-merge 证据；publish 侧两个没有。 - 受影响调用方/流程: SHA 写错的失败形态是发布时
Unable to resolve action——构建 zip 成功、资产没挂上 Release，且发生在拿了contents: write的那一步。 - 最小可行修复: 合并前对这两个 SHA 各做一次
gh api repos/actions/download-artifact/commits/<sha>/repos/softprops/action-gh-release/commits/<sha>确认存在并落在对应 tag 上；成本一分钟，覆盖唯一没有执行证据的部分。
- 证据:
- [低] 契约测试的扫描面止于
.github/workflows/- 证据:
scripts/tests/workflow-rules.test.mjs:453只readdirSync('.github/workflows')。测试注释已经专门论证过「只收.yml会让新增的x.yaml静默豁免」，但同样的静默豁免对.github/actions/**下的复合 action 仍然成立——复合 action 的action.yml里的uses:一样能引入移动 tag。 - 受影响调用方/流程: 今天没有复合 action（Glob 确认
.github/下只有 ISSUE_TEMPLATE、dependabot.yml 和 workflows），所以现在是零影响；风险只在有人日后新增该目录时兑现。 - 最小可行修复: 把扫描源从固定目录改成「
.github/workflows/*.y[a]ml+.github/actions/**/action.y[a]ml（若存在）」，或在注释里明确记录这条规则的边界，让扩展时不必重新推导。
- 证据:
行级发现
- [.github/workflows/update-npm-downloads.yml:15] 这段保留理由对本文件不成立：该 job 在
:13是runs-on: ubuntu-latest，node24/自托管 runner 的前置条件不适用，而git push的凭据依赖也被 PR 自己核对的「v4.4.0 与 v6.1.0 的persist-credentials默认同为 true」否定。由于 dependabot 的 github-actions 生态没有 major ignore，这段注释是未来那次升级 PR 的唯一判断依据，改成本 workflow 真正成立的理由（与 repo-guard 同步、按 #440 item 2 一起动）。 - [scripts/tests/workflow-rules.test.mjs:477] 版本注释正则要求版本号后紧跟空白或行尾，
# v1.2.0-rc1、# v2.0.0-beta这类带后缀的 tag 会被判成(no version comment)。dependabot 默认不提预发布版本，所以概率低，但这是全仓门禁，误红的代价是拦住一次合法 bump；把结尾放宽成[\w.+-]*(\s|$)即可。
Karpathy 评审
- 假设: 模型输出需要归一化为固定 Markdown 契约。
- 简洁性: 已提取 summary、finding、evidence 与 fix；原始 prose 不再附在评论中，避免占用下游解析与代理上下文。
- 变更范围: 原始模型未提供结构化范围字段。
- 验证: 需要查看 CI、测试或人工 CR 证据补强合并信心。
缺失覆盖
- 输出未命中 Repo Guard Markdown 契约；建议补充真实模型质量评估覆盖。
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| # 刻意留在 v4:同 repo-guard.yml 的理由(v5.0.0 起改用 node24),而且本 |
There was a problem hiding this comment.
这段保留理由对本文件不成立：该 job 在 :13 是 runs-on: ubuntu-latest，node24/自托管 runner 的前置条件不适用，而 git push 的凭据依赖也被 PR 自己核对的「v4.4.0 与 v6.1.0 的 persist-credentials 默认同为 true」否定。由于 dependabot 的 github-actions 生态没有 major ignore，这段注释是未来那次升级 PR 的唯一判断依据，改成本 workflow 真正成立的理由（与 repo-guard 同步、按 #440 item 2 一起动）。
| // 光有 SHA 读不出这是哪个版本,升级时无从判断跨了多少。要求尾注版本号。 | ||
| // 锚定到 SHA 之后的注释:不锚定的话,行内任何位置出现的数字(比如一句 | ||
| // 「# 见 #440」)都会被当成版本号,这条断言就形同虚设。 | ||
| else if (!new RegExp(`@${ref}\\s*#\\s*v?\\d+(\\.\\d+)*(\\s|$)`, 'u').test(line)) |
There was a problem hiding this comment.
版本注释正则要求版本号后紧跟空白或行尾，# v1.2.0-rc1、# v2.0.0-beta 这类带后缀的 tag 会被判成 (no version comment)。dependabot 默认不提预发布版本，所以概率低，但这是全仓门禁，误红的代价是拦住一次合法 bump；把结尾放宽成 [\w.+-]*(\s|$) 即可。
…reasons were wrong repo-guard on 2ff6b39, all three findings. 1. The note I put above `update-npm-downloads.yml`'s pin was wrong twice. That job runs on `ubuntu-latest`, not the self-hosted runner, so the node24 precondition does not apply to it; and the `git push` credential concern was already answered in this same PR — `persist-credentials` defaults to `true` at both v4.4.0 and v6.1.0. This matters because `.github/dependabot.yml` has no major-version ignore for the github-actions ecosystem, so a bump PR for these two is certain, and that comment is the reviewer's only context. A wrong reason sends them chasing a precondition that does not exist — or, worse, lets "I checked the runner" justify merging both. The note now states the real reason: it stays on v4 to move together with repo-guard's checkout under #440 item 2, and says plainly that this one could be upgraded on its own merits — there is just no reason to. The node24 argument lives only where it is true, in repo-guard.yml. 2. `release.yml`'s publish job is tag-gated, so `download-artifact` and `action-gh-release` get no pre-merge execution. Both SHAs verified to exist upstream (HTTP 200 on the commits API). A wrong SHA there fails as "Unable to resolve action" during a release, in the step holding `contents: write`. 3. The test scans only `.github/workflows/`. A composite action under `.github/actions/**` could reintroduce a moving tag and be silently exempt — zero impact today since that directory does not exist, so the boundary is recorded next to the existing `.yml`/`.yaml` reasoning rather than speculatively coded for. Refs #440 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
代码评审报告: chore(ci): pin third-party actions to commit SHAs (#440)
风险等级: 中
处理建议: 评论
决策摘要: ** diff 本身没有发现缺陷，可以合并；但三处 pin（repo-guard、update-npm-downloads、release publish 两个 action）在本 PR 的 CI 里不会被执行，而 SHA↔版本↔allow-unsafe-pr-checkout 存在性这组事实在仓库内无法复现验证——合并前请维护者对着上游 tag 眼验这三处，尤其是 actions/checkout v4.4.0 确实带 allow-unsafe-pr-checkout。
级联分析
- 变更符号: 原始模型未提供结构化级联字段。
- 受影响流程: 原始模型未提供结构化级联字段。
- 变更集外调用方: unknown
- 置信度: degraded
问题发现
- [中] 三处 pin 在合并前不会被执行，其中一处正是 #440 item 1
- 证据:
pull_request_target使用 base 分支的 workflow 文件，所以本次 Repo Guard 运行执行的是 develop 上的@v4，不是repo-guard.yml:79的新 SHA。update-npm-downloads.yml只有schedule/workflow_dispatch。release.yml:74,80在if: startsWith(github.ref, 'refs/tags/')的 publish job 里。其余（ci、contract-guard、gitguardian、release build）都有pull_request触发，其 SHA 由本 PR 自己的 CI 验证。 - 受影响调用方/流程: SHA 若错，repo-guard 的失败形态与 #437 逐字相同（checkout 红、评审 step skipped）；release publish 的失败要等到打 tag 才暴露；npm-downloads 的失败在定时任务里，没人盯。
- 最小可行修复: 合并前用
git ls-remote --tags或 GitHub UI 核对这三处 SHA 与注释版本一致，并确认actions/checkoutv4.4.0 的action.yml含allow-unsafe-pr-checkout（PR 描述称已做，但该证据无法从仓库复现）。这是外部事实核对，不需要改 diff。
- 证据:
- [低] 被显式推迟的两项决定在 #440 关闭后失去 tracker
- 证据:
repo-guard.yml:75-78、update-npm-downloads.yml:15-18、workflow-rules.test.mjs:462三处注释都把「对齐到 v6」和「是否钉ceilf6/repo-guard@main」指向 #440，而 PR 声明Closes #440。#438 的评审已经要求过「follow-up 不要只活在别的 issue 正文里」，这里会重演同一模式。 - 受影响调用方/流程: 后续维护者读到注释后追到一个已关闭 issue；
ceilf6/repo-guard@main是唯一跑在自托管 runner、携带secrets.LLM_API_KEY的移动 ref，恰恰是这批 pin 里安全权重最高的一处，却没有活跃 tracker。 - 最小可行修复: 开一个 follow-up issue 覆盖（a）runner 确认 node24 后两处 checkout 一起对齐 v6、（b）repo-guard@main 的 pin 策略，并把三处注释与 CHANGELOG 重新指向它；或者本 PR 不关闭 #440。
- 证据:
- [低] dependabot 会对两处 v4 checkout 提跨大版本 SHA 升级，唯一防线是 YAML 注释
- 证据:
.github/dependabot.yml:27-33开启了github-actions每周更新，且 major 的ignore规则只挂在 npm ecosystem 上。contract-guard.yml:34现在跳过 dependabot 作者的 PR，所以这类 PR 只剩 CI 和 CODEOWNERS 兜底——而新契约测试对@<sha> # v7.0.1同样是绿的（\s*#允许单空格，dependabot 的写法能通过）。 - 受影响调用方/流程: repo-guard 的 checkout 被顺手合到 v6/v7 = node24，正是 PR 判定为不可验证的那条风险。
- 最小可行修复: 在
dependabot.yml的 github-actions 条目下为actions/checkout加version-update:semver-major的 ignore，注释指向 follow-up issue；或明确接受由人工评审拦截。
- 证据:
- [低] 契约测试只校验版本注释存在，不校验它与 SHA 一致
- 证据:
workflow-rules.test.mjs:481的正则要求@<sha>后跟# vX.Y.Z，但注释内容与 SHA 的对应关系无法离线验证；一个复制粘贴错的标签会静默通过，而这条注释的全部价值就是「升级时知道跨了多远」。 - 受影响调用方/流程: 未来的升级者按错误标签判断跨度。
- 最小可行修复: 离线可查的那部分加断言即可——同一 SHA 必须对应同一版本注释、同一 (action, 版本注释) 必须对应同一 SHA。这允许 checkout 刻意的 v4/v6 分叉，同时能抓住
pnpm/action-setup、setup-node（各重复 3–4 处）里最现实的手抖错误。
- 证据:
行级发现
- [.github/workflows/repo-guard.yml:78] 注释把「先确认 runner 版本再升 v6/v7」这条前提挂在 #440 上，而本 PR
Closes #440；建议改指向一个新开的 follow-up issue（update-npm-downloads.yml:15与workflow-rules.test.mjs:462同源，一并改），否则这条推迟决定合并当天就失去 tracker。 - [scripts/tests/workflow-rules.test.mjs:481] 只校验「有版本注释」，错标签静默通过；补一条离线可查的一致性断言（同 SHA ⇒ 同注释、同 action+同注释 ⇒ 同 SHA）即可覆盖重复 4 次的
setup-node/action-setup，且不妨碍 checkout 刻意的 v4/v6 分叉。
Karpathy 评审
- 假设: 模型输出需要归一化为固定 Markdown 契约。
- 简洁性: 已提取 summary、finding、evidence 与 fix；原始 prose 不再附在评论中，避免占用下游解析与代理上下文。
- 变更范围: 原始模型未提供结构化范围字段。
- 验证: 需要查看 CI、测试或人工 CR 证据补强合并信心。
缺失覆盖
- 输出未命中 Repo Guard Markdown 契约；建议补充真实模型质量评估覆盖。
| # 刻意留在 v4:v5.0.0 起 actions/checkout 改用 node24 运行时,而本 job 跑在 | ||
| # 自托管 runner(vars.REPO_GUARD_RUNNER),它是否满足 node24 从本仓库看不到。 | ||
| # 猜错的失败形态与 #437 逐字相同——checkout 处红、评审 step skipped、我们这边 | ||
| # 零改动。dependabot 提 v6/v7 升级时:先确认 runner 版本,再合。见 #440。 |
There was a problem hiding this comment.
注释把「先确认 runner 版本再升 v6/v7」这条前提挂在 #440 上，而本 PR Closes #440；建议改指向一个新开的 follow-up issue（update-npm-downloads.yml:15 与 workflow-rules.test.mjs:462 同源，一并改），否则这条推迟决定合并当天就失去 tracker。
| // 光有 SHA 读不出这是哪个版本,升级时无从判断跨了多少。要求尾注版本号。 | ||
| // 锚定到 SHA 之后的注释:不锚定的话,行内任何位置出现的数字(比如一句 | ||
| // 「# 见 #440」)都会被当成版本号,这条断言就形同虚设。 | ||
| else if (!new RegExp(`@${ref}\\s*#\\s*v?\\d+(\\.\\d+)*(\\s|$)`, 'u').test(line)) |
There was a problem hiding this comment.
只校验「有版本注释」，错标签静默通过；补一条离线可查的一致性断言（同 SHA ⇒ 同注释、同 action+同注释 ⇒ 同 SHA）即可覆盖重复 4 次的 setup-node/action-setup，且不妨碍 checkout 刻意的 v4/v6 分叉。
Owner
Author
|
三条都已处理（ 发现 1（中，保留理由与实际不符） — 我写错了，而且错了两处。 你指出的后果是对的、也是我没想到的一层： 现在写的是真实理由：留在 v4 是为了与 repo-guard 的 checkout 同版本、按 #440 item 2 一起动；并明说这一处本身可以升，只是没有理由单独升。node24 的论证只留在它成立的地方（ 发现 2（低，publish 侧两个 pin 无执行证据） — 已按你说的成本一分钟的方式补了证据： 两个 SHA 都在上游存在。这覆盖的是「写错 SHA」这一类；publish job 本身要到下次打 tag 才会真正跑，那部分仍无 pre-merge 证据，如实记在 commit 里。 发现 3（低，扫描面止于 门禁： |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Linked Issue Or Context
Summary
#437 was caused by a moving tag.
actions/checkout@v4silently began refusing fork checkouts underpull_request_target, and Repo Guard failed on every fork PR with no change on our side. The regression test written for it asserts workflow text, and Actions only warns on unknownwith:keys rather than failing — so if the tag moves again andallow-unsafe-pr-checkoutis renamed or dropped, checkout breaks whiletest:workflowsstays green. That is the mechanism that hid #437.Every third-party action is now
@<40-hex SHA> # vX.Y.Z, and a contract test requires both halves: the SHA freezes behavior, the comment lets a future upgrade see how far it is jumping.actions/checkoutd23441a4v6.1.0 ×5,11d5960av4.4.0 ×2@v6×5,@v4×2actions/setup-node24997072v6.5.0@v6actions/upload-artifact043fb46dv7.0.1@v7actions/download-artifactd3f86a10v4.3.0@v4pnpm/action-setup0ebf4713v6.0.9@v6softprops/action-gh-release3d0d9888v3.0.2@v3GitGuardian/ggshield/actions/secretfaa02e4bv1.53.0@v1.53.0Pinned to each action's current major, not the latest release.
actions/checkoutis at v7.0.1 upstream; this stays on v6.1.0. A pin commit that also bumps majors is a behavior change wearing a chore's clothes — and #440 explicitly asked to check theallow-unsafe-pr-checkoutsemantics before moving repo-guard off@v4.On the two
@v4usages (#440's open question — align to v6, or pin where they are): pinned where they are, on the v4.4.0 SHA.Aligning them to v6 would have added a precondition this repo cannot check.
actions/checkoutv5.0.0 moved to a node24 runtime, andrepo-guard.ymlruns onvars.REPO_GUARD_RUNNER— a self-hosted runner whose version is not visible from here. If it does not satisfy node24, the failure is byte-for-byte #437 again: red at checkout, review step skipped, nothing changed on our side.update-npm-downloads.ymlcarries the same jump and additionallygit pushes on credentialscheckoutpersists.So this PR does #440's item 1 (kill the moving tag) with zero behavior change, and leaves item 2 (align to v6) to a change that can verify the runner. For the record,
persist-credentialsdefaults totrueat both the v4.4.0 and v6.1.0 SHAs — checked — so the alignment is safe on that axis; it is the node24 axis that is unverifiable, and both usages should move together when it is.ceilf6/repo-guard@mainis deliberately left unpinned and allowlisted explicitly in the test, with the reason next to it. It is first-party, so pinning means a bump PR here for every repo-guard change — but it is also the step that runs on the self-hosted runner carryingsecrets.LLM_API_KEY, so the trade is real. That is a maintainer policy call; recorded, not decided.Also checked, since the pins made the pairing explicit:
download-artifact@v4readsupload-artifact@v4+artifacts, so the v7/v4 pairing inrelease.ymlis fine. (Thearchive: falsedirect-download path would need upload v7 and is not used here.)Impact Scope
.github/workflows/,uses:lines only.scripts/tests/workflow-rules.test.mjs— one new contract test.CHANGELOG.md.No workflow logic, triggers, permissions or job conditions change.
GitNexus Impact Summary
actions/checkout..github/workflows/classifies asrepo-harness, so this PR gates itself and ships the required contract test.detect_changesreports no symbol-level change — the diff isuses:refs, one test, one changelog line.contexton the Harness surface confirmsscripts/tests/workflow-rules.test.mjshas no importers (anode --testentry point) and that nothing inpackages/,apps/orscripts/reads workflow files at runtime; the only consumer is GitHub Actions itself. Noimpactquery applies — there is no call graph here, and the real risk is behavioral drift in third-party code, which is exactly what pinning removes.pnpm quality:precommitgreen (43 workflow tests, up from 42);pnpm quality:localgreen.allow-unsafe-pr-checkoutpresence confirmed at both checkout SHAs by fetchingaction.ymlat each.Verification
third-party actions are pinned to a commit SHA— walks everyuses:in every.yml/.yamlworkflow, skipping local actions and reusable workflows (uses: ./…), which have no@refand so cannot comply. Falsified both ways: reverting one entry to@v6fails withci.yml: actions/checkout@v6; dropping just the version comment fails with(no version comment).repo guard can check out fork PRs from allowlisted contributorstest still passes, so fix(ci): let Repo Guard check out fork PRs from allowlisted contributors (#437) #438's opt-in survived the v4→v6 move.End-to-end confirmation that Repo Guard still checks out fork PRs correctly can only come from a fork PR after merge; the input's presence at the pinned SHA is the pre-merge evidence.
Checklist
pnpm quality:precommit, or explained why it could not run.pnpm quality:localfor critical skeleton changes, or explained why it could not run.