Skip to content

Commit fb34d05

Browse files
authored
Reduce PR security preflight false positives for CI metadata (#4170)
## Summary Closes #4167. Reduces `pr-security-preflight` false positives for trusted CI metadata while keeping the trust boundary conservative: - allow exact hosted-CI metadata comments from `github-actions[bot]` - avoid treating `github-actions[bot]` as a hidden/untrusted participant when that participant is explained by exact hosted-CI metadata - allow exact hosted-CI waiver audit comments from `github-actions[bot]` only when the echoed waiver reason has no suspicious terms - ignore resolved trusted-bot review-thread comments only when the resolver is also trusted - keep arbitrary bot comments, unresolved trusted-bot suspicious review comments, and trusted-bot review comments resolved by untrusted users blocking ## Rationale The replay targets from #4167 had CI/review metadata that should not be treated as actionable untrusted input. This keeps arbitrary bot comments, suspicious waiver reasons, untrusted resolvers, high-risk files, and incomplete GitHub API coverage fail-closed. ## Validation - `ruby .agents/skills/pr-batch/bin/pr-security-preflight-test.rb` -> 20 runs, 157 assertions, 0 failures - `ruby -c .agents/skills/pr-batch/bin/pr-security-preflight && ruby -c .agents/skills/pr-batch/bin/pr-security-preflight-test.rb` -> Syntax OK - `BUNDLE_GEMFILE=Gemfile bundle exec rubocop .agents/skills/pr-batch/bin/pr-security-preflight .agents/skills/pr-batch/bin/pr-security-preflight-test.rb` -> 2 files, no offenses - `git diff --check origin/main` -> pass - local suspicious-diff regex scan on the added test diff -> no added matching lines - `script/ci-changes-detector origin/main` -> documentation-only, recommended CI jobs: NONE - `codex review --base origin/main` -> final pass clean after fixing waiver-reason and resolver-trust findings - `claude -p '/simplify origin/main' --model claude-opus-4-8 --max-budget-usd 20` -> interrupted after a bounded silent wait; no files changed - `.agents/skills/pr-batch/bin/pr-security-preflight --repo shakacode/react_on_rails 4170` -> `SECURITY_PREFLIGHT_OK` - `.agents/skills/pr-batch/bin/pr-security-preflight --repo shakacode/react_on_rails 4145 4122` -> false-positive queues cleared for both replay targets; still exits blocked only because both targets exceed the current GraphQL coverage window: - #4145: `timelineItems fetched 100 of 223 nodes` - #4122: `timelineItems fetched 100 of 206 nodes` ## Confidence note - Validated: targeted unit tests, Ruby syntax checks, RuboCop on changed files, diff whitespace check, suspicious-diff scan, CI selector, live #4170 self-preflight, live #4145/#4122 replay, Codex review. - Evidence: validation commands listed above; branch pre-push hook also reran branch Ruby lint successfully. - UNKNOWN: hosted CI/checks and review-agent results for this draft PR are pending; replay targets remain blocked by the existing coverage-truncation guard. - Residual risk: exact CI metadata matching may need updates if the CI command workflow changes its generated comment text. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Changes trust-boundary logic in a security gate (`.agents/` tooling): it deliberately allows more `github-actions[bot]` and resolved bot review input, so incorrect template matching or resolver trust could weaken blocking. > > **Overview** > Tightens **`pr-security-preflight`** so routine hosted-CI and review metadata stops tripping batch gates, without opening the door to arbitrary bot text. > > **Hosted CI from `github-actions[bot]`:** Issue comments are recognized only when they match fixed templates (CI requested/stop/status/waiver audit). Those comments are dropped from the untrusted comment queue, and `github-actions[bot]` is no longer flagged as a hidden participant when that presence is explained by matching metadata. Waiver comments are allowed only if the echoed **Reason** line does not hit suspicious-term patterns; malformed or injection-shaped waiver text still blocks. > > **Trusted-bot review threads:** The preflight loads resolved review-thread state via GraphQL and skips **suspicious-text** findings on trusted-bot review comments only when the thread is resolved by a **trusted** actor. Unresolved bot comments, threads resolved by untrusted users, and resolution fetch failures remain **fail-closed** (still blocking). > > Tests and fake `gh` fixtures cover the new allowlist paths, negative cases (arbitrary Actions comments, bad waiver reasons, untrusted resolvers), and the resolved-thread behavior. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit d2062e8. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 7f28663 commit fb34d05

2 files changed

Lines changed: 526 additions & 27 deletions

File tree

0 commit comments

Comments
 (0)