Repository navigation
Add insecure-defaults and fail-open detector to review_checks.sh - #2298
Merged
Merged
Conversation
A deterministic pass now flags literal insecure defaults and fail-open paths that the review otherwise only catches by reading: a security flag whose default turns the protection off, a broad exception in an auth check that returns allow, TLS or JWT verification disabled in code, debug on by default, and a wildcard CORS origin with credentials. insecure_defaults.py emits the same TSV rows as the other detectors and is wired as tool insecure-defaults with the shared SKIPPED handling. Test paths, comments, localhost lines and fail-closed variants stay silent; a false default on an ENABLE_* flag is the safe direction and is not reported, and a JS catch inside an if block of an auth function is found. Whole-app security mode routes the rows to a shipped-config check, tool-hub.md and the script indexes list the tool, and one eval pins the keep-versus-drop behavior. Tests: 5/5 in scripts/test_insecure_defaults.py checks (13 positive fixtures exact, 15 decoy fixtures silent); 167/170 in scripts/test-ci-gates.sh locally, the 3 failures are environment-only (size: review-config.md is already over its stale budget row at base; dcr-gates opt-in all-three-off and refix_gate Refix-Reason, which hit sandbox limits). mustload ok, test_appsec_split 7/7. size-budget-raise: .claude/skills/deep-code-review/references/security-review-mode.md 14643→14821 route insecure-defaults rows size-budget-raise: .claude/skills/deep-code-review/references/tool-hub.md 19414→19926 list the insecure-defaults tool Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
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: adds a deterministic insecure-defaults / fail-open pass to
review_checks.sh --full(toolinsecure-defaults). Replaces #2297 after review.Classes: security flag defaulting off, broad exception in an auth check returning allow, TLS verify off, JWT alg none or verify off, debug on by default, wildcard CORS with credentials. Rows use the existing TSV format and shared SKIPPED handling.
Review fixes vs #2297: a false default on ENABLE_* is no longer reported (safe direction, decoy added); control-flow heads (
if,catch, ...) no longer count as the enclosing function, so a JS catch inside anifin an auth function is found (positive added);INDEX-scripts.mdandtool-hub.mdlist the tool.Tests:
scripts/test_insecure_defaults.py4/4 test methods (13 positive fixtures exact, 15 decoy fixtures silent; the commit message says 5/5, a typo).test-ci-gates.sh167/170 locally. The 3 environment-only failures: (1)size: the real repo's SKILL.md/references files are all within their frozen byte budget(review-config.md is over its stale budget row at base; not in this diff), (2)dcr-gates opt-in: all three gates are off by default, (3)dcr-gates opt-in: refix_gate passes with a Refix-Reason trailer(sandbox lock and network limits). mustload ok, test_appsec_split 7/7.Derived files: following #2292 (latest script-adding PR), INDEX files and the touched
size-budgets.tsvrows are committed;SHA256SUMSis left to land-time regen (scripts/regen-unlanded.sh). Size markers for security-review-mode.md and tool-hub.md are in the commit. Not done: thesecurity-appsec.mdTLS-off list is left as is, since it is a detection grep list pinned by the split test and a link would not replace it.🤖 Generated with Claude Code