Skip to content

feat: drift-check §10 ADR 实体性校验——空壳 ADR 检出(红队 RB-D5,ADR-0016) - #50

Merged
randypanding merged 3 commits into
mainfrom
adr-substantive-check
Aug 19, 2026
Merged

feat: drift-check §10 ADR 实体性校验——空壳 ADR 检出(红队 RB-D5,ADR-0016)#50
randypanding merged 3 commits into
mainfrom
adr-substantive-check

Conversation

@randypanding

Copy link
Copy Markdown
Contributor

ADR-0016(决策 6,解决 .github #45 RB-D5)

问题(复核属实):§10 后验仅 grep -q "^ADR-${num}-" <<<"$ADR_FILES"文件名存在——红队演示:提交者引用一个真实存在但内容为空的 ADR 文件(或占位符),gate 的格式正则(\bADR-[0-9]{4}\b)放行、§10 判"存在"OK——"有 ADR"被解释为"文件存在",GM-2 决策背书无实质记录。

修复:§10 从存在性升级为存在性+实体性:

  • contents API 响应自带 size 字段——零额外 API 请求完成校验
  • 文件存在但 <100B = 空壳 ADR → 计漂移(100B 是宽松下界:连标题+状态+一句话都装不下的文件不可能记录决策;真实 ADR 均 >1KB,现有 16 个 ADR 最小者远超此界)
  • 精确前缀匹配 .name | startswith("ADR-NNNN-") 顺带消除旧 grep 前缀匹配的边缘误报可能

本地实测(真实 org 数据):OK adr-reference-existence(窗口内合并 PR 的 ADR 引用全部真实)bash -n 通过。

旧版仅查文件名存在:引用真实存在但 <100B 的空壳 ADR 可同时骗过
gate 格式检查与 §10 存在性检查,GM-2 '无 ADR 不合并' 形同虚设。
contents API 自带 size 字段,零额外请求完成实体性校验。
@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: 20 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: 5632e3f1-3476-4746-9d95-d22602417160

📥 Commits

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

📒 Files selected for processing (1)
  • governance/drift-check.sh

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

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Detect Empty ADR Stubs in Governance Drift Checks

🐞 Bug fix ✨ Enhancement 🕐 Less than 10 minutes

Grey Divider

AI Description

• Validates referenced ADRs as files using exact filename-prefix matching.
• Flags ADR files below 100 bytes as non-substantive governance drift.
• Reuses Contents API metadata without additional network requests.
Diagram

graph TD
  A["Merged PRs"] --> B["Extract ADR refs"] --> D{"Matching file?"}
  C["Contents listing"] --> D
  D -- "No" --> F["Ghost ADR drift"]
  D -- "Yes" --> E{"At least 100B?"}
  E -- "No" --> G["Stub ADR drift"]
  E -- "Yes" --> H["Valid reference"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Fetch and parse ADR contents
  • ➕ Can validate required sections and meaningful decision text
  • ➕ Provides stronger assurance than a byte threshold
  • ➖ Requires additional API requests or repository checkout
  • ➖ Adds parsing rules, failure modes, and maintenance overhead
2. Validate ADRs at source
  • ➕ Rejects empty ADRs when they enter agent-registry
  • ➕ Centralizes substantive validation around the ADR source of truth
  • ➖ Does not independently protect consumers from historical placeholders
  • ➖ Cannot replace the cross-repository post-hoc check under current token constraints

Recommendation: Keep the proposed metadata-size check as the immediate defense: it closes the empty-file bypass with no extra API calls and minimal complexity. Source-repository schema validation can later complement it if stronger structural guarantees are required.

Files changed (1) +8 / -2

Bug fix (1) +8 / -2
drift-check.shReject missing and undersized referenced ADR files +8/-2

Reject missing and undersized referenced ADR files

• Replaces grep-based filename existence checks with a jq lookup requiring a matching file entry. References to files smaller than 100 bytes now produce drift, using size metadata already returned by the Contents API.

governance/drift-check.sh

@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. Byte padding bypasses validation ✓ Resolved 🐞 Bug ⛨ Security
Description
The new check treats every matching file of at least 100 bytes as substantive, so an empty
placeholder containing 100 bytes of whitespace, comments, or filler passes without recording any
decision. This preserves the RB-D5 bypass the PR is intended to close and can incorrectly report the
ADR reference as valid.
Code

governance/drift-check.sh[R295-298]

+        elif [[ "$(jq -r .size <<<"$ROW")" -lt 100 ]]; then
+          # 实体性校验(红队 RB-D5,ADR-0016 决策 6):文件存在但 <100B = 空壳 ADR
+          # ——同时骗过 gate 格式检查与旧版 §10 存在性检查;"有 ADR"须=有实质决策记录
+          drift "repo '$r' PR#$pnum 引用空壳 ADR ${ref}(文件存在但 $(jq -r .size <<<"$ROW")B < 100B——无实质决策记录,GM-2 形同虚设)"
Relevance

●●● Strong

The PR explicitly targets RB-D5 substantive validation; recent governance bypass findings were
accepted as security fixes.

PR-#19

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The lookup selects only directory metadata for a matching file, and the sole substantive rejection
condition is .size < 100; therefore every matching file with size >= 100 is accepted regardless
of its contents.

governance/drift-check.sh[291-299]
governance/drift-check.sh[273-277]

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 substantive ADR check uses only the Contents API `size` metadata. A contributor can pad an otherwise empty placeholder to 100 bytes, causing it to pass without containing a decision.

## Issue Context
Inspect the referenced document content rather than equating byte length with substance. Validate required ADR structure and meaningful non-whitespace content, such as a title, accepted status, context, and decision section; malformed or unavailable content must fail closed.

## Fix Focus Areas
- governance/drift-check.sh[273-299]

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



Remediation recommended

2. ROW uses broad token ✗ Dismissed 📘 Rule violation ⛨ Security
Description
The added ADR validation consumes GitHub API data fetched with the long-lived GOVERNANCE_TOKEN
rather than a single-repository cloudbrid-agent token from scripts/gh-app-token.sh. This bypasses
the required App identity, least-privilege scope, and associated audit trail.
Code

governance/drift-check.sh[291]

+        ROW=$(jq -c --arg p "ADR-${num}-" '[.[] | select((.name | startswith($p)) and (.type == "file"))] | first // empty' <<<"$ADR_DIR_LISTING")
Relevance

●●● Strong

Recent PR #19 accepted closely related App-identity and single-repository token hardening in this
governance automation.

PR-#19

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Rule 2778539 requires GitHub API automation to obtain credentials exclusively through
scripts/gh-app-token.sh with single-repository scope. The changed ROW logic reads
ADR_DIR_LISTING, which is fetched through api; that helper uses externally supplied GH_TOKEN,
and the workflow populates it directly from secrets.GOVERNANCE_TOKEN instead of invoking the
required App-token script.

Rule 2778539: Agent operations must use the cloudbrid-agent GitHub App identity via scripts/gh-app-token.sh with single-repo tokens
governance/drift-check.sh[19-22]
governance/drift-check.sh[273-291]
.github/workflows/governance-drift.yml[20-28]

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 ADR substantive check relies on API responses authenticated using `GOVERNANCE_TOKEN`, contrary to the required cloudbrid-agent GitHub App authentication mechanism.

## Issue Context
Generate tokens through `scripts/gh-app-token.sh` with an explicit repository scope. Use an `agent-registry`-scoped token for the decisions listing and an appropriate single-repository token for each repository's pull-request query; remove reliance on the generic governance token.

## Fix Focus Areas
- governance/drift-check.sh[19-22]
- governance/drift-check.sh[273-291]
- .github/workflows/governance-drift.yml[20-28]

ⓘ 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:
  +2 more
Review mode: ⚖️ Balanced: This is a runtime governance shell change that alters ADR validation behavior and depends on API JSON semantics and jq handling; the logic is localized, but the gate impact warrants a complete single-pass review.

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 governance/drift-check.sh Outdated
Comment thread governance/drift-check.sh Outdated
randypanding and others added 2 commits August 19, 2026 11:54
回应 qodo 意见(High 落码,Medium 凭据项以设计依据回复):

1. 字节填充绕过(High):size>=100B 可被空白/注释/填充字符绕过——空壳
   ADR 仍可骗过校验。改为拉取被引 ADR 全文做结构校验:H1 编号行、
   status/状态 行、背景/决策章节齐备且决策节有正文,四者缺一即判
   空壳;读取失败 fail-closed。同名多文件(ADR-0011 先例)任一满足
   即通过;按编号缓存避免重复拉取。

2. 凭据项(Medium,不落码):与 §1-§9 一致使用 AGENTS.md 声明的
   GOVERNANCE_TOKEN 接口("GH_TOKEN=<org admin> bash
   governance/drift-check.sh");agent-registry 私有仓 contents 读取
   本就需要跨仓权限,工作流仅 schedule/dispatch 于可信 main 运行。

验证:单元测试 4 场景(真实结构 ok/空壳 shell/130 字节 # 填充 shell/
缺失 missing)全过;真实 API 全量跑——既有 15 个 ADR 全部判 ok(含
中文"状态:"变体的 ADR-0010),仅 ADR-0016 正确报幽灵(PR #21 未合)。
@randypanding
randypanding merged commit 813e43b into main Aug 19, 2026
6 checks passed
@randypanding
randypanding deleted the adr-substantive-check branch August 19, 2026 04:15
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