test-integrity: P2-1 测试篡改检测门（ADR-0035 / .github #86） - #20
Conversation
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
…39 的 main 重建（替代 #17）
|
Warning Review limit reached
Next review available in: 1 minute Limit details: You’ve used all 10 included reviews currently available. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (41)
Comment |
| - name: Checkout 检测器（CI-Workflows 同 ref，不取 caller 仓内副本） | ||
| if: github.event_name == 'pull_request' | ||
| uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| with: | ||
| repository: Cloudbird-Software/CI-Workflows | ||
| ref: ${{ steps.toolref.outputs.ref }} | ||
| path: ciw | ||
| persist-credentials: false | ||
|
|
||
| - uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 |
PR Summary by QodoAdd fail-closed reusable test-integrity gate
AI Description
Diagram
High-Level Assessment
Files changed (41)
|
…己的 ref——refs/pull/N/merge，不可用于钉源）
Code Review by Qodo
1.
|
| else | ||
| is_meta "$path" || impl_changed=$((impl_changed + 1)) || true | ||
| fi |
There was a problem hiding this comment.
1. Non-code changes suppress ti-r4 🐞 Bug ≡ Correctness
Every changed path that is neither a test nor in the narrow metadata regex is counted as an implementation file, including arbitrary configs, lockfiles, data, or junk files. Adding or touching any such file alongside an expectation rewrite makes impl_changed nonzero and bypasses TI-R4 without an implementation change.
Agent Prompt
## Issue description
TI-R4 treats all non-test, non-metadata files as implementation source, allowing unrelated file changes to suppress the rule.
## Issue Context
Use an explicit policy-controlled implementation/source classification, or conservatively exclude known config, generated, asset, and data paths without treating the complement of a small metadata list as source.
## Fix Focus Areas
- scripts/test-integrity.sh[27-30]
- scripts/test-integrity.sh[107-145]
- scripts/test-integrity.sh[245-249]
- scripts/parse-test-integrity-policy.py[58-62]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| got_rules="$(printf '%s\n' "$out" | { grep -oE '\[TI-[A-Z0-9-]+\]' || true; } | tr -d '[]' | sort -u | paste -sd, -)" | ||
|
|
||
| ok=1 | ||
| [ "$rc" = "$exp_exit" ] || ok=0 | ||
| [ "$got_rules" = "$exp_rules" ] || ok=0 | ||
| while IFS= read -r pat; do | ||
| [ -n "$pat" ] || continue | ||
| printf '%s\n' "$out" | grep -qE "$pat" || ok=0 | ||
| done < <(sed -n 's/^contains=//p' "$d/expected.txt") |
There was a problem hiding this comment.
2. Self-test omits count validation 🐞 Bug ⚙ Maintainability
The fixture runner compares exit status, emitted rule names, and a few optional regex fragments, but it does not systematically compare the detector's assertion, suppression, deletion, or implementation counts. Incorrect ledger accounting can therefore pass the required self-test even though the job is documented as fully validating those counts.
Agent Prompt
## Issue description
Fixture validation does not fully compare the detector's advertised count outputs with expected values.
## Issue Context
Give each fixture explicit expected `TI-COUNT` values and parse/compare every relevant counter, rather than relying on sparse `contains=` patterns.
## Fix Focus Areas
- scripts/test-integrity-fixtures/run.sh[18-36]
- scripts/test-integrity-fixtures/cases/01-delete-test-file/expected.txt[1-4]
- scripts/test-integrity-fixtures/cases/05-normal-evolution/expected.txt[1-2]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| policy-repo: | ||
| type: string | ||
| default: "Cloudbird-Software/.github" # 规则/阈值声明来源 | ||
| policy-ref: | ||
| type: string | ||
| default: "main" # C1 保护的默认分支；调用方可钉 SHA 提高不可变性 |
There was a problem hiding this comment.
4. Caller can replace policy 🐞 Bug ⛨ Security
A PR can modify its caller workflow to override the unrestricted policy-repo and policy-ref inputs, redirecting the gate from the claimed immutable central source to attacker-controlled policy regexes. Those patterns are exported to the detector and can be made permissive enough to classify no files or assertions, bypassing R1–R3 and potentially R4 so test tampering passes.
Agent Prompt
## Issue description
An audited PR can alter the reusable-workflow invocation to supply an attacker-controlled `policy-repo` or `policy-ref`, redirecting enforcement to untrusted policy regexes. The parser accepts and exports those regexes, allowing the detector's matching rules to be disabled and test tampering to pass.
## Issue Context
The policy is part of the gate's trust boundary and is intended to come from a location that the reviewed PR cannot modify. The current caller-selectable workflow inputs conflict with that boundary; hard-code or strictly allowlist the governance repository, and derive or validate an immutable trusted ref.
## Fix Focus Areas
- .github/workflows/test-integrity.yml[21-28]
- .github/workflows/test-integrity.yml[95-106]
- scripts/parse-test-integrity-policy.py[39-62]
- README.md[87-89]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| [ "$old_test" = 1 ] && assert_rem=$((assert_rem + f_ar)) || true | ||
| [ "$new_test" = 1 ] && assert_add=$((assert_add + f_aa)) || true | ||
| [ "$new_test" = 1 ] && supp_add=$((supp_add + f_sa)) || true |
There was a problem hiding this comment.
5. Renames bypass rewrite detection 🐞 Bug ≡ Correctness
An edited test-to-test rename is handled by the R branch, which records assertion and suppression counts but never increments test_mod_removed for deleted lines; TI-R4 is raised only through the ordinary modification path. Consequently, renaming a test while rewriting or removing an expectation—with no net assertion decrease and no implementation change—can incorrectly exit TI-OK and evade TI-R4.
Agent Prompt
## Issue description
A `git diff -M` rename with content edits is classified as `R`, but the rename handler excludes deleted test lines from TI-R4 activity accounting. As a result, expectation rewrites can evade TI-R4 merely by renaming the test file.
## Issue Context
`git diff -M` emits sufficiently similar edited renames with rename metadata and normal hunks, and the parser already counts removed diff lines in `f_r`. Only the `M` path currently consumes that value for TI-R4; apply equivalent `f_r`/R4 suspect handling when either side of a rename remains a test path, and add an edited-rename fixture.
## Fix Focus Areas
- scripts/test-integrity.sh[97-110]
- scripts/test-integrity.sh[132-145]
- scripts/test-integrity.sh[245-249]
- scripts/test-integrity-fixtures/run.sh[15-36]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| "diff --git "*) finalize_file ;; | ||
| "deleted file mode "*) cur_status=D ;; | ||
| "new file mode "*) cur_status=A ;; |
There was a problem hiding this comment.
6. Binary deletions evade r1 🐞 Bug ≡ Correctness
A binary test asset deletion has deleted file mode but no textual --- a/path header, and the parser does not recover its old path from the preceding diff --git header, leaving old_test=0. As a result, binary files under test directories or binary .snap assets are classified as changed implementation files instead of being added to deleted_tests, allowing them to finish as TI-OK and bypass TI-R1.
Agent Prompt
## Issue description
Binary file deletions set deletion status but do not provide the textual `---`/`+++` file headers currently used to recover the old path. Update the parser so it can recognize a deleted binary file under a test path and record TI-R1 rather than allowing it to finish as TI-OK.
## Issue Context
TI-R1 promises to reject test-file deletion regardless of file contents, and its test-path regex intentionally includes testdata and snapshots, which can contain binary test assets. Parse and validate old/new paths from `diff --git` headers or supplement parsing with a fail-closed `git diff --name-status -M` inventory.
## Fix Focus Areas
- scripts/test-integrity.sh[151-170]
- scripts/test-integrity.sh[90-120]
- scripts/test-integrity-fixtures/run.sh[15-36]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
摘要
落地 P2-1（.github #86，ADR-0035）：新增 test-integrity 可复用工作流，四类测试篡改直接红——
\bADR-NNNN\b（agent-registry/decisions 存在性校验，防幽灵）→ 豁免但计数入账（TI-COUNT escape_hatch_waived + step summary）github.workflow_ref解析后从同 ref checkout（不取 caller 副本；实测github.action_ref在 reusable 上下文随步骤动作漂移，不可用）；规则/阈值读 .github main policy（缺节用内置同值缺省，拉取失败即红）test-integrity-selftestjob + 执法 workflow 每次运行前置 fixture 自检（15 case 预标注全比对）本 PR 替代 #17（原分支与并行落地的 ADR-0037/#16、ADR-0039 产生文本冲突，基于含两者的 main 重建，内容等价 + 上述钉源/policy 解析改进）。
变更文件（41）
.github/workflows/test-integrity.yml（reusable workflow）scripts/test-integrity.sh（检测器，bash，语言无关 regex 级）scripts/parse-test-integrity-policy.py（policy 解析，fail-closed；独立 .py 而非内联 heredoc——CodeQL Analyze(python) 曾因内联片段 fatal）.github/requirements-test-integrity.txt（PyYAML 版本+sha256 双锚定，同 diff-coverage 钉法）scripts/test-integrity-fixtures/（T8：15 case + run.sh）.github/workflows/ci.yml（selftest job + gate needs，ADR-0032 严格断言）README.md（工作流表 + test-integrity 接入章节）验证清单（全部已执行）