fix: SecurityValidator confirm handler silently passes through#933
Open
SaintPepsi wants to merge 1 commit intodanielmiessler:mainfrom
Open
fix: SecurityValidator confirm handler silently passes through#933SaintPepsi wants to merge 1 commit intodanielmiessler:mainfrom
SaintPepsi wants to merge 1 commit intodanielmiessler:mainfrom
Conversation
Two bugs fixed in SecurityValidator.hook.ts:
1. The "confirm" category used {"decision": "ask"} as stdout output,
but Claude Code PreToolUse hooks only recognize {"continue": true}
(allow) and exit code 2 (hard block). The "ask" format is silently
ignored, meaning operations that should prompt for confirmation are
allowed without any user interaction.
Fix: confirm-level operations now use process.exit(2) to hard-block,
with guidance to run the command manually outside Claude Code.
2. Pattern file paths used paiPath('skills', 'PAI', 'USER', ...) but
the correct path has no 'skills' prefix: paiPath('PAI', 'USER', ...).
This meant patterns.yaml was never found, so SecurityValidator
fell through to EMPTY_PATTERNS (fail-open, everything allowed).
Co-Authored-By: Claude Opus 4.6 <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.
Summary
Two security bugs in SecurityValidator.hook.ts (v3.0) that cause the hook to be effectively non-functional:
{"decision": "ask", "message": "..."}to stdout, but Claude Code PreToolUse hooks require a specific JSON structure withhookSpecificOutput.permissionDecision. The{"decision": "ask"}format is not recognized and silently ignored, meaning operations matching confirm patterns execute without any user interaction.paiPath('skills', 'PAI', 'USER', ...)resolves to~/.claude/skills/PAI/USER/..., butpatterns.yamllives at~/.claude/PAI/USER/...(noskills/prefix). This means the patterns file is never found, SecurityValidator falls through toEMPTY_PATTERNS(fail-open, everything allowed).Proof: Claude Code only recognizes specific PreToolUse output formats
From the official Claude Code hooks documentation:
Exit codes:
Structured JSON output (exit 0 + stdout):
The upstream SecurityValidator outputs
{"decision": "ask", "message": "..."}which does NOT match either recognized format. It's not exit code 2, and it's not thehookSpecificOutputstructure. Claude Code silently ignores it and proceeds with the operation.Changes
{"decision": "ask"}to usingprocess.exit(2)with descriptive stderr messages — matching the hard-block pattern used forblockedcategory'skills'from bothpaiPath()calls sopatterns.yamlis actually found at~/.claude/PAI/USER/PAISECURITYSYSTEM/patterns.yamlImpact
Without this fix, SecurityValidator's confirm-level protections provide zero actual protection — they silently pass through. The path bug compounds this by preventing ALL pattern matching from working, meaning even blocked-level patterns don't fire.
Alternative fix
Instead of
process.exit(2), the confirm handlers could use the correct JSON structure to actually prompt the user:{ "hookSpecificOutput": { "hookEventName": "PreToolUse", "permissionDecision": "ask", "permissionDecisionReason": "This operation requires confirmation" } }This PR uses
exit(2)(hard block) as the safer default, since confirm-level operations are inherently risky. But switching to"permissionDecision": "ask"would restore the original intent of prompting the user.— 🍁 Maple