Repository navigation
Gate security findings on a cited guard check to cut false positives - #2294
Closed
remigiusz-antczak wants to merge 2 commits into
Closed
remigiusz-antczak wants to merge 2 commits into
remigiusz-antczak wants to merge 2 commits into
Conversation
A bench of whole-app security audits showed the reviewer filing env-gated dev bypasses, admin routes behind auth, local-only paths, same-bearer JWT claim "mismatches" and opaque IDs as findings. Root cause: security-appsec.md said a plausible-but-unproven issue is still a finding, and the whole-app mode never routed to the refuter pass in appsec-verify.md. - security-appsec.md: unproven issues go through appsec-verify.md section 1 first, are reported `unverified`, and rank below proven findings. - appsec-verify.md section 1: enumerate guard classes to search and cite (middleware, env/feature gate, prod-disabled path, validator/cap, opaque ID, same-bearer claims, documented decision); a held guard drops the finding unless bypassable; an unread guard is `❓`. - security-review-mode.md: new step 5 runs that refuter on every candidate before the report. - evals.json: env-gated-dev-bypass-not-reported case with a fictional fixture. Tests: 4/4 (routing, rules, privacy, run-evals dry-run); size gate fails only on review-config.md, untouched here and already over budget on origin/main. size-budget-raise: .claude/skills/deep-code-review/references/security-appsec.md 28245→28322 guard-check pointer size-budget-raise: .claude/skills/deep-code-review/references/appsec-verify.md 8075→8768 guard classes and unverified ranking size-budget-raise: .claude/skills/deep-code-review/references/security-review-mode.md 13589→13799 refute-before-reporting step Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The first measurement showed the model read security-review-mode.md but never opened appsec-verify.md, so the pointer never fired. Step 5 now carries the rule itself: search five guard classes, cite the guard file:line or "no guard found at <paths>", drop a held guard or an unproven-in-production env gate, and mark unreadable guards unverified below proven findings. appsec-verify.md is restored to its previous text (no duplicate list). A new commit rather than an amend, because the branch is already pushed and amending needs a force push. Tests: 4/4 (routing, rules, privacy, size gate only fails on review-config.md, already over budget on origin/main) size-budget-raise: .claude/skills/deep-code-review/references/security-review-mode.md 13589→14545 inline guard check size-budget-raise: .claude/skills/deep-code-review/references/appsec-verify.md 8075→8075 unchanged after revert Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Collaborator
Author
|
Superseded by #2295 (single squashed commit; drops the security-appsec.md edit that broke the appsec-split and must-load gates). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
BLUF: Security audits now must find and cite the guard for each candidate before filing it. Held guard -> drop; unread or unproven ->
unverified, ranked below proven findings. Targets the false-positive classes seen in a private audit bench: env-gated dev bypass, admin console behind auth, local-only media paths, JWT claims vs the checked identity, opaque IDs flagged as IDOR.Root cause:
security-appsec.mdsaid a plausible-but-unproven issue is still a finding, and whole-app mode never routed to the refuter pass inappsec-verify.md.Changes (reuses the existing refuter step, no new section):
security-appsec.md: unproven issues go throughappsec-verify.mdsection 1, reportedunverified, ranked below proven.appsec-verify.mdsection 1: guard classes to search and cite; held guard drops the finding unless bypassable; "did not read the guard" is a question mark, never a finding.security-review-mode.md: step 5 runs that refuter on every candidate.evals.json:env-gated-dev-bypass-not-reported(fictional fixture).size-budgets.tsv: three rows raised withsize-budget-raise:markers.Tests: routing, rules, privacy, run-evals dry-run all pass.
ci-gates.sh sizefails onreview-config.md(6888 vs 6472), a file this PR does not touch and which is already over budget on main.Not merged. A paid before/after re-measure is reported separately.
🤖 Generated with Claude Code