Skip to content

fix: gate 文件清单三防线——edited/rename 溯源/3000 fail-closed(ADR-0016) - #22

Merged
randypanding merged 3 commits into
mainfrom
harden-adr-gate
Aug 19, 2026
Merged

fix: gate 文件清单三防线——edited/rename 溯源/3000 fail-closed(ADR-0016)#22
randypanding merged 3 commits into
mainfrom
harden-adr-gate

Conversation

@randypanding

Copy link
Copy Markdown
Contributor

背景

CI-Workflows #8 的 qodo code review 发现 adr-required 实现有三个可绕过路径。本仓 validate.yml 的 standards-diff detection 用的是同一模式(同模板移植),同款缺陷同样存在,本 PR 同步加固。

三防线

# 缺陷 修复
1 workflow 未订阅 edited——PR check 通过后可编辑 title/body 移除 ADR 引用,不触发重验 on.pull_request.typesedited
2 files API 对 rename 只在 previous_filename 暴露原路径——把 standards/CODEOWNERS 等改名移出受管路径可绕过 C1 判定 C1 匹配纳入 previous_filename
3 files API 3000 文件硬上限——超大 PR 中 C1 文件可能被截断漏检 返回数 < changed_files 即 fail-closed

复核

  • 缺陷客观性:GitHub REST API 文档明确 files endpoint 上限 3000、rename 条目含 previous_filenameon.pull_request 默认 types 为 opened/synchronize/reopened 不含 edited——三项均为平台行为,非推测。
  • 本仓 gate.yml 与 CI-Workflows ci.yml 为同一模型的移植(ADR-0013 实装),故同款修复三仓同步。

ADR: ADR-0016

CI-Workflows #8 qodo review 发现的同款缺陷在本仓 validate.yml 同样存在:
1. on.pull_request.types 增 edited:PR title/body 是可变输入,check 通过后
   编辑掉 ADR 引用必须触发重验(反向:补引用救活 check 同理)
2. previous_filename 纳入 C1 判定:standards/CODEOWNERS 等 C1 资产改名移出
   受管路径同样是 C1 变更
3. files API 3000 文件硬上限:返回数 < changed_files 即 fail-closed
@coderabbitai

coderabbitai Bot commented Aug 19, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@randypanding, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 3 minutes

Limit details: You’ve used all 3 included reviews currently available.

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: fa85983f-f8b4-482c-9156-1e8337c66719

📥 Commits

Reviewing files that changed from the base of the PR and between 2457a8f and 061c64a.

📒 Files selected for processing (1)
  • .github/workflows/validate.yml

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

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Harden ADR gate against edited metadata, renames, and API truncation

🐞 Bug fix ⚙️ Configuration changes 🕐 10-20 Minutes

Grey Divider

AI Description

• Re-run ADR validation when pull request title or body is edited.
• Detect governed files through both current and pre-rename paths.
• Fail closed when GitHub's files API returns an incomplete list.
Diagram

graph TD
  A["PR event"] --> B["Gate workflow"] --> C["Files API"] --> D{"List complete?"}
  D -- "No" --> E["Fail closed"]
  D -- "Yes" --> F["Current + prior paths"] --> G{"C1 path?"}
  G -- "Yes" --> H["ADR required"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Compute paths with git diff
  • ➕ Avoids the REST files endpoint's 3,000-file ceiling
  • ➕ Reports rename source and destination paths in one operation
  • ➕ Eliminates repeated files API requests
  • ➖ Requires reliable base and head history in the workflow checkout
  • ➖ Introduces merge-base and shallow-clone edge cases
  • ➖ Expands the scope beyond the targeted gate hardening

Recommendation: Keep the PR's API-based fail-closed approach for this targeted fix. It preserves the existing workflow model, directly addresses all three bypasses, and safely rejects inventories whose completeness cannot be proven; a local git-diff implementation is worth considering only if legitimate pull requests routinely approach 3,000 files.

Files changed (1) +20 / -2

Bug fix (1) +20 / -2
validate.ymlClose three ADR gate bypass paths +20/-2

Close three ADR gate bypass paths

• Adds the 'edited' pull request trigger so ADR checks rerun after title or body changes. File classification now includes rename origins and rejects truncated GitHub file inventories before deciding whether a governed C1 path changed.

.github/workflows/validate.yml

@qodo-code-review

qodo-code-review Bot commented Aug 19, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Newline filenames bypass truncation ✓ Resolved 🐞 Bug ⛨ Security
Description
GOT counts rendered filename lines, so a valid Git filename containing a newline is counted as
multiple files. In a PR exceeding the API's 3000-file limit, an attacker can inflate GOT to
changed_files, hide a C1 path beyond the truncation boundary, and bypass the intended fail-closed
ADR gate.
Code

.github/workflows/validate.yml[62]

+          GOT=$(grep -c . <<<"$FILES_CUR" || true)
Relevance

●●● Strong

Recent PR #10 accepted the same fail-closed file-list integrity issue; line counting remains a
deterministic bypass.

PR-#10

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflow renders .[].filename and then uses grep -c ., making the count dependent on
embedded newline characters rather than the number of returned API objects. Git documents
NUL-delimited output as necessary for handling unusual pathnames, while the GitHub files endpoint
returns at most 3000 records; together these establish a concrete way to make the new completeness
check pass despite truncation.

.github/workflows/validate.yml[60-67]
🌐 Git documents NUL-delimited path output for safely handling unusual pathnames, including paths that cannot be represented reliably as newline-delimited records.
🌐 The documented pull-request files endpoint returns a maximum of 3000 files.

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The truncation guard counts lines produced from filenames. Git permits newline characters in paths, so one API record can contribute multiple counted lines and defeat the comparison against `changed_files`.

## Issue Context
Keep the paginated API responses structured, count their JSON objects, and derive both current and previous paths from the same captured response. Prefer performing the C1 path classification in `jq` as well, so filename delimiters cannot affect counting or matching.

## Fix Focus Areas
- .github/workflows/validate.yml[61-68]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
✅ Compliance rules (platform): 8 rules
✅ Web pages:
  +12 more
Review mode: ⚖️ Balanced: This modifies a CI enforcement workflow and its fail-closed file/API detection logic, so it has meaningful correctness and bypass risk; however, the change is localized enough for one careful review pass.

Grey Divider

Tip of the day
💡 Did you know, you can show, collapse, or hide each part of a finding: code, evidence, and all

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread .github/workflows/validate.yml Outdated
randypanding and others added 2 commits August 19, 2026 12:17
Git 允许文件名含换行——行流计数(grep -c)可被单文件多行虚增 GOT,
使 3000 截断 fail-closed 失效(GOT 虚增到 == changed_files 后,藏在
截断边界后的 C1 文件不再触发报错)。改为逐页 jq 按 JSON 对象计数
(length)与匹配(test 正则),文件名中的换行不影响任何一层判定。
逐页拉取同时天然覆盖 >100 文件分页场景。
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