-
-
Notifications
You must be signed in to change notification settings - Fork 40
Distrust Green Gate misreports a collection error as "ALL tests skip" and offers a waiver that cannot apply #3106
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,15 +43,27 @@ jobs: | |
| with: | ||
| github-token: ${{ secrets.GITHUB_TOKEN }} | ||
| script: | | ||
| const body = [ | ||
| "**Distrust Green Gate**: this PR adds or modifies test file(s) where ALL", | ||
| "tests skip (e.g. `pytest.importorskip` on a module not yet available), so", | ||
| "CI reports GREEN while asserting nothing. See the check-all-skip job log", | ||
| "for the file and the guard that caused it.", | ||
| "", | ||
| "Either implement the guarded code, or waive deliberately with a", | ||
| "`Tests-Skipped-Intentionally: <file>, <why>` trailer in the PR body.", | ||
| ].join("\n"); | ||
| const reason = `${{ steps.all-skip.outputs.failure_reason }}`; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🛡️ Analyzed with Security Review | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: sed -n '1,130p' .github/workflows/distrust-green-gate.yml
sed -n '1,120p' .github/scripts/check_all_skip.py
sed -n '260,415p' .github/scripts/check_all_skip.py
rg -n 'distrust-green-gate|check_all_skip|GITHUB_OUTPUT|pull_request_target|permissions:' .github tests/scriptsRepository: jaylfc/taOS Length of output: 16869 🏁 Script executed: #!/bin/bash
set -e
printf '%s\n' '--- workflow ---'
cat -n .github/workflows/distrust-green-gate.yml | sed -n '1,100p'
printf '%s\n' '--- checker lifecycle ---'
cat -n .github/scripts/check_all_skip.py | sed -n '60,125p'
cat -n .github/scripts/check_all_skip.py | sed -n '280,410p'
printf '%s\n' '--- relevant tests/helpers ---'
cat -n tests/scripts/test_check_all_skip.py | sed -n '390,520p'Repository: jaylfc/taOS Length of output: 18445 🌐 Web query:
💡 Result: <search_synthesis> <source_evidence> Citations:
🌐 Web query:
💡 Result: <search_synthesis> <source_evidence> Citations:
Injection Reachability: External Keep The direct Pass the value through the environment: Proposed fix uses: actions/github-script@v9
+ env:
+ FAILURE_REASON: ${{ steps.all-skip.outputs.failure_reason }}
with:
github-token: ${{ secrets.GITHUB_TOKEN }}
script: |
- const reason = `${{ steps.all-skip.outputs.failure_reason }}`;
+ const reason = process.env.FAILURE_REASON;🧰 Tools🪛 zizmor (1.30.0)[info] 46-46: code injection via template expansion (template-injection): may expand into attacker-controllable code (template-injection) 🤖 Prompt for AI Agents |
||
| let body; | ||
| if (reason === 'collection_error') { | ||
| body = [ | ||
| "**Distrust Green Gate**: this PR adds or modifies a test file that", | ||
| "fails to collect (crash or import error during collection). See the", | ||
| "check-all-skip job log for the file and error detail.", | ||
| "", | ||
| "Fix the collection error before requesting review.", | ||
| ].join("\n"); | ||
| } else { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WARNING: The Reply with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Handle
Proposed fix- } else {
+ } else if (reason === 'all_skip') {
body = [
"**Distrust Green Gate**: this PR adds or modifies test file(s) where ALL",
"tests skip (e.g. `pytest.importorskip` on a module not yet available), so",
"CI reports GREEN while asserting nothing. See the check-all-skip job log",
"for the file and the guard that caused it.",
"",
"Either implement the guarded code, or waive deliberately with a",
"`Tests-Skipped-Intentionally: <file>, <why>` trailer in the PR body.",
].join("\n");
+ } else {
+ body = [
+ "**Distrust Green Gate**: the all-skip check failed for a reason other",
+ "than all skipped tests. See the check-all-skip job log for details.",
+ ].join("\n");
}🤖 Prompt for AI Agents |
||
| body = [ | ||
| "**Distrust Green Gate**: this PR adds or modifies test file(s) where ALL", | ||
| "tests skip (e.g. `pytest.importorskip` on a module not yet available), so", | ||
| "CI reports GREEN while asserting nothing. See the check-all-skip job log", | ||
| "for the file and the guard that caused it.", | ||
| "", | ||
| "Either implement the guarded code, or waive deliberately with a", | ||
| "`Tests-Skipped-Intentionally: <file>, <why>` trailer in the PR body.", | ||
| ].join("\n"); | ||
| } | ||
| await github.rest.issues.createComment({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| ### Fixed | ||
|
|
||
| - Distrust Green Gate now reports collection errors separately from all-skip violations and no longer offers the `Tests-Skipped-Intentionally` waiver for files that crash on collection. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WARNING:
failure_detailis not sanitized before writing toGITHUB_OUTPUT. A filepath containing newlines or=could corrupt the output format.Reply with
@kilocode-bot fix itto have Kilo Code address this issue.