Skip to content

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

Merged
randypanding merged 1 commit into
mainfrom
harden-adr-gate
Aug 19, 2026
Merged

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

Conversation

@randypanding

Copy link
Copy Markdown
Contributor

背景

CI-Workflows #8 的 qodo code review 发现 adr-required 实现有三个可绕过路径。本仓 gate.yml 的 adr-required 是同一模型的原始实现(agent-registry validate.yml 由其移植),同款缺陷存在,本 PR 同步加固。

三防线

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

客观依据(平台行为,非推测)

  • GitHub REST API 文档:List pull requests files 端点响应上限 3000 文件;rename 条目含 previous_filename 字段
  • on.pull_request 默认 activity types 为 opened/synchronize/reopened,不含 edited

三仓同款修复:CI-Workflows #8(首发)/ agent-registry #22 / 本 PR。

ADR: ADR-0016

CI-Workflows #8 qodo review 发现的同款缺陷在本仓 gate.yml 同样存在:
1. on.pull_request.types 增 edited:PR title/body 是可变输入,check 通过后
   编辑掉 ADR 引用必须触发重验(反向:补引用救活 check 同理)
2. previous_filename 纳入 C1 判定:governance/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: 23 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: 45a2c912-2d03-40f3-b286-b405b5d5b7fe

📥 Commits

Reviewing files that changed from the base of the PR and between 0ffe714 and a87421d.

📒 Files selected for processing (1)
  • .github/workflows/gate.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, Renamed, and Truncated PRs

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

Grey Divider

AI Description

• Revalidate ADR references whenever pull request metadata is edited.
• Detect governed assets renamed out of protected paths using original filenames.
• Fail closed when GitHub returns an incomplete changed-file list.
Diagram

graph TD
  A["PR Event"] --> B["Files API"] --> C{"List Complete?"}
  C -- "No" --> D["Fail Closed"]
  C -- "Yes" --> E{"C1 Path?"}
  E -- "No" --> F["Skip Gate"]
  E -- "Yes" --> G["ADR Check"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Single-pass file metadata retrieval
  • ➕ Avoids paginating through the files endpoint twice.
  • ➕ Ensures current and previous filenames come from one API snapshot.
  • ➕ Reduces GitHub API usage for large pull requests.
  • ➖ Requires retaining and querying the full paginated JSON response.
  • ➖ Adds shell or temporary-file handling complexity to a small workflow step.

Recommendation: Keep the PR’s fail-closed security model and rename-aware matching. The current implementation is clear and suitable for this focused fix; consolidating file retrieval into one snapshot would be a worthwhile follow-up only if API efficiency or consistency becomes a concern.

Files changed (1) +21 / -2

Bug fix (1) +21 / -2
gate.ymlClose ADR gate bypasses in pull request file detection +21/-2

Close ADR gate bypasses in pull request file detection

• Adds the 'edited' pull request trigger so title and body changes revalidate ADR references. The ADR gate now checks renamed files through 'previous_filename' and rejects incomplete file lists caused by GitHub’s 3,000-file API limit.

.github/workflows/gate.yml

@randypanding
randypanding merged commit 445b170 into main Aug 19, 2026
6 checks passed
@randypanding
randypanding deleted the harden-adr-gate branch August 19, 2026 04:07
@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Newline filename bypasses cap 🐞 Bug ≡ Correctness
Description
GOT counts nonempty output lines rather than files, so a filename containing a newline can inflate
the count and conceal truncation above GitHub's 3000-file limit. An attacker can place such a
filename among the returned entries and a C1 file beyond the limit, causing the incomplete list to
pass and skip ADR enforcement.
Code

.github/workflows/gate.yml[R82-83]

+          FILES_CUR=$(gh api --paginate "$FILES_API" --jq '.[].filename')
+          GOT=$(grep -c . <<<"$FILES_CUR" || true)
Relevance

●●● Strong

PR #19 accepted fail-closed handling for incomplete file lists; this is a similarly exploitable
counting bypass.

PR-#19

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The workflow renders each filename as plain text and applies grep -c, making output lines—not API
entries—the unit being compared with changed_files. Git supports paths containing LF when quoted,
while jq documents that raw string output is newline-delimited and specifically provides
NUL-delimited output for strings that can contain newlines; the previously accepted pagination bug
demonstrates that an incomplete file list can bypass this same ADR gate.

.github/workflows/gate.yml[81-89]
🌐 Git path documentation explicitly describes LF-containing filenames and requires them to use quoted path representation.
🌐 The jq manual states that raw string results are written directly with newline separators and recommends raw-output0 when values may contain newlines.
PR-#19

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 fail-closed check counts lines emitted from filenames, but Git filenames may contain line feeds. This lets one returned file produce multiple counted lines and mask files omitted by the API's 3000-file cap.

## Issue Context
Keep the paginated response as structured JSON. Count response objects and evaluate `filename` and `previous_filename` directly with `jq`, rather than serializing filenames into newline-delimited shell text before counting or matching.

## Fix Focus Areas
- .github/workflows/gate.yml[79-89]

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



Remediation recommended

2. gh api uses workflow token 📘 Rule violation ⛨ Security
Description
The added GitHub API requests authenticate through the existing ${{ github.token }} rather than a
single-repository cloudbrid-agent token issued by scripts/gh-app-token.sh. This violates the
required authentication and identity policy for automated GitHub operations.
Code

.github/workflows/gate.yml[R81-82]

+          CHANGED=$(gh api "$PR_API" --jq '.changed_files')
+          FILES_CUR=$(gh api --paginate "$FILES_API" --jq '.[].filename')
Relevance

●●● Strong

PR #19 accepted enforcing cloudbrid-agent single-repository tokens, directly matching this
repository's stated authentication policy.

PR-#19

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 2778539 requires all automated GitHub API operations to obtain authentication
exclusively through scripts/gh-app-token.sh with single-repository scope. The added gh api calls
at lines 81-82 inherit GH_TOKEN from ${{ github.token }} at line 68, and the workflow does not
invoke the required token script.

Rule 2778539: Agent operations must use the cloudbrid-agent GitHub App identity via scripts/gh-app-token.sh with single-repo tokens
.github/workflows/gate.yml[67-82]
scripts/gh-app-token.sh[20-29]

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 new `gh api` requests use the workflow-provided GitHub token instead of a cloudbrid-agent GitHub App token.

## Issue Context
Obtain the token through `scripts/gh-app-token.sh`, setting its mandatory `REPO` input to the current repository name so the resulting installation token is restricted to one repository. Populate `GH_TOKEN` from that command's output rather than `${{ github.token }}`.

## Fix Focus Areas
- .github/workflows/gate.yml[67-82]

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


Grey Divider

Context sources
✅ Compliance rules (platform): 7 rules
✅ Web pages:
  +7 more
Review mode: ⚖️ Balanced: This changes a security-relevant CI gate’s triggering and file-completeness logic, so it warrants a complete single-pass review despite being localized to one workflow.

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 on lines +81 to +82
CHANGED=$(gh api "$PR_API" --jq '.changed_files')
FILES_CUR=$(gh api --paginate "$FILES_API" --jq '.[].filename')

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Remediation recommended

1. gh api uses workflow token 📘 Rule violation ⛨ Security

The added GitHub API requests authenticate through the existing ${{ github.token }} rather than a
single-repository cloudbrid-agent token issued by scripts/gh-app-token.sh. This violates the
required authentication and identity policy for automated GitHub operations.
Agent Prompt
## Issue description
The new `gh api` requests use the workflow-provided GitHub token instead of a cloudbrid-agent GitHub App token.

## Issue Context
Obtain the token through `scripts/gh-app-token.sh`, setting its mandatory `REPO` input to the current repository name so the resulting installation token is restricted to one repository. Populate `GH_TOKEN` from that command's output rather than `${{ github.token }}`.

## Fix Focus Areas
- .github/workflows/gate.yml[67-82]

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

Comment on lines +82 to +83
FILES_CUR=$(gh api --paginate "$FILES_API" --jq '.[].filename')
GOT=$(grep -c . <<<"$FILES_CUR" || true)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

2. Newline filename bypasses cap 🐞 Bug ≡ Correctness

GOT counts nonempty output lines rather than files, so a filename containing a newline can inflate
the count and conceal truncation above GitHub's 3000-file limit. An attacker can place such a
filename among the returned entries and a C1 file beyond the limit, causing the incomplete list to
pass and skip ADR enforcement.
Agent Prompt
## Issue description
The fail-closed check counts lines emitted from filenames, but Git filenames may contain line feeds. This lets one returned file produce multiple counted lines and mask files omitted by the API's 3000-file cap.

## Issue Context
Keep the paginated response as structured JSON. Count response objects and evaluate `filename` and `previous_filename` directly with `jq`, rather than serializing filenames into newline-delimited shell text before counting or matching.

## Fix Focus Areas
- .github/workflows/gate.yml[79-89]

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

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