feat: add SAFETY_NET_ASK mode to prompt user instead of blocking#45
feat: add SAFETY_NET_ASK mode to prompt user instead of blocking#45c2keesey wants to merge 2 commits intokenryu42:mainfrom
Conversation
When SAFETY_NET_ASK=1 is set, dangerous commands return 'ask' instead of 'deny', prompting the user for confirmation rather than blocking outright. Supported in Claude Code and Copilot CLI hooks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fafa8d881c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const sessionId = `copilot-${input.timestamp ?? Date.now()}`; | ||
| writeAuditLog(sessionId, command, result.segment, result.reason, cwd); | ||
| outputCopilotDeny(result.reason, command, result.segment); | ||
| outputCopilotDecision(askMode ? 'ask' : 'deny', result.reason, command, result.segment); |
There was a problem hiding this comment.
When SAFETY_NET_ASK=1 is set, this code emits permissionDecision: 'ask' for Copilot CLI. I checked GitHub’s Copilot hooks reference, and it states PreToolUse currently processes only deny, so returning ask will not trigger an interactive confirmation there. In that environment, dangerous commands that were previously blocked can execute instead, which is a fail-open regression for users who enable ask mode.
Useful? React with 👍 / 👎.
Greptile SummaryThis PR adds Confidence Score: 5/5Safe to merge — logic is correct, strict-mode invariant is properly enforced, and all key interactions are tested. The only finding is a P2 cosmetic issue where the statusline shows ❓ even when strict mode renders ask mode inactive. All remaining changes are well-tested and the strict-mode/ask-mode guard (askMode && !strict) is implemented correctly at the decision point. Prior thread concerns about strict parse failures are fully addressed. src/bin/statusline.ts — ❓ emoji could be suppressed when SAFETY_NET_STRICT=1 to match actual runtime behavior. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Hook receives Bash command] --> B{Parse stdin JSON}
B -- "parse error + STRICT" --> Z1[outputDecision 'deny']
B -- "parse error, no STRICT" --> Z2[return, allow through]
B -- success --> C{tool_name == 'Bash'?}
C -- No --> Z3[return, allow through]
C -- Yes --> D[analyzeCommand]
D -- "no match" --> Z4[return, allow through]
D -- "match (blocked)" --> E{SAFETY_NET_ASK && !SAFETY_NET_STRICT?}
E -- Yes --> F[outputDecision 'ask' - FLAGGED header + confirm footer - Audit logged]
E -- No --> G[outputDecision 'deny' - BLOCKED header + manual-run footer - Audit logged]
Reviews (2): Last reviewed commit: "fix: address review feedback — strict ov..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughA new "ask mode" feature is introduced via the Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Hook as Claude Code Hook
participant SafetyNet as Safety Net Logic
participant Format as Message Formatter
User->>Hook: Execute command
Hook->>SafetyNet: Check if dangerous & read env flags
alt Command is safe
SafetyNet->>Hook: permit
Hook->>User: Execute
else Command is dangerous
SafetyNet->>SafetyNet: Check SAFETY_NET_ASK & SAFETY_NET_STRICT
alt Strict mode enabled
SafetyNet->>Format: Prepare "BLOCKED" message
Format->>User: Display blocked notification
SafetyNet->>Hook: deny
else Ask mode enabled
SafetyNet->>Format: Prepare "FLAGGED" message with confirmation
Format->>User: Prompt for approval/denial
User->>Hook: Approve or Cancel
SafetyNet->>Hook: ask (pending user decision)
else Default (block)
SafetyNet->>Format: Prepare "BLOCKED" message
Format->>User: Display blocked notification
SafetyNet->>Hook: deny
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/bin/cli-statusline.test.ts (1)
54-67: LGTM!Test case follows the established pattern and correctly verifies the ask mode emoji output.
Consider adding a test for combined modes (e.g.,
SAFETY_NET_ASK=1+SAFETY_NET_STRICT=1→❓🔒) to verify emoji concatenation works correctly with ask mode, similar to the existing "strict + paranoid" combination test at lines 99-117.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/bin/cli-statusline.test.ts` around lines 54 - 67, Add a new test in tests/bin/cli-statusline.test.ts that spawns the CLI with --statusline and both environment vars set (SAFETY_NET_ASK: '1' and SAFETY_NET_STRICT: '1') using the same pattern as the existing test 'shows ask mode emoji when SAFETY_NET_ASK=1' (reuse enabledSettingsPath and Bun.spawn call), then assert output.trim() equals '🛡️ Safety Net ❓🔒' and exit code is 0 to verify emoji concatenation when ask + strict are enabled.tests/bin/hooks/copilot-cli-hook.test.ts (1)
23-58: Add strict+ask regression coverage for Copilot hook.Nice ask-mode path coverage. Please also add tests for
SAFETY_NET_ASK=1+SAFETY_NET_STRICT=1with invalid outer JSON and invalidtoolArgsJSON to lock in the “strict parse failures always deny” invariant for Copilot too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/bin/hooks/copilot-cli-hook.test.ts` around lines 23 - 58, Add two tests under the same hook tests that run runCopilotHook with SAFETY_NET_ASK=1 and SAFETY_NET_STRICT=1: one where the outer input JSON is malformed and one where toolArgs contains invalid JSON; both tests should parse the hook stdout and assert the permissionDecision is "deny" and the permissionDecisionReason contains a parse/JSON error message (i.e., strict parse failures always deny), and ensure the hook emits a JSON decision object so the test can inspect permissionDecision and permissionDecisionReason; reference runCopilotHook, SAFETY_NET_ASK, SAFETY_NET_STRICT and the existing "ask mode" tests as the place to add these cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/bin/cli-statusline.test.ts`:
- Around line 54-67: Add a new test in tests/bin/cli-statusline.test.ts that
spawns the CLI with --statusline and both environment vars set (SAFETY_NET_ASK:
'1' and SAFETY_NET_STRICT: '1') using the same pattern as the existing test
'shows ask mode emoji when SAFETY_NET_ASK=1' (reuse enabledSettingsPath and
Bun.spawn call), then assert output.trim() equals '🛡️ Safety Net ❓🔒' and exit
code is 0 to verify emoji concatenation when ask + strict are enabled.
In `@tests/bin/hooks/copilot-cli-hook.test.ts`:
- Around line 23-58: Add two tests under the same hook tests that run
runCopilotHook with SAFETY_NET_ASK=1 and SAFETY_NET_STRICT=1: one where the
outer input JSON is malformed and one where toolArgs contains invalid JSON; both
tests should parse the hook stdout and assert the permissionDecision is "deny"
and the permissionDecisionReason contains a parse/JSON error message (i.e.,
strict parse failures always deny), and ensure the hook emits a JSON decision
object so the test can inspect permissionDecision and permissionDecisionReason;
reference runCopilotHook, SAFETY_NET_ASK, SAFETY_NET_STRICT and the existing
"ask mode" tests as the place to add these cases.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f5fe88d7-c049-438c-8eb3-117d08b06786
⛔ Files ignored due to path filters (4)
dist/bin/cc-safety-net.jsis excluded by!**/dist/**dist/core/format.d.tsis excluded by!**/dist/**dist/index.jsis excluded by!**/dist/**dist/types.d.tsis excluded by!**/dist/**
📒 Files selected for processing (14)
AGENTS.mdCLAUDE.mdREADME.mdsrc/bin/doctor/environment.tssrc/bin/help.tssrc/bin/hooks/claude-code.tssrc/bin/hooks/copilot-cli.tssrc/bin/statusline.tssrc/core/format.tssrc/types.tstests/bin/cli-statusline.test.tstests/bin/hooks/claude-code-hook.test.tstests/bin/hooks/copilot-cli-hook.test.tstests/core/format.test.ts
…o deny-only - Strict mode now overrides ask mode (askMode && !strict) so unparseable commands in strict mode always hard-deny - Reverted Copilot CLI hook to deny-only since Copilot may not support 'ask' as a permissionDecision (fail-open risk) - Added tests: strict+ask command parse failures, combined statusline emoji, Copilot ignores ask mode - Updated README to document Claude Code-only support Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Manual TestingTested locally using a clean Claude Code session with the local plugin build: Setup# Build the plugin
cd claude-code-safety-net && bun run build
# Create a wrapper script to set env + load local plugin
cat > /tmp/claude-safety-test.sh << 'SCRIPT'
#!/bin/bash
export SAFETY_NET_ASK=1
exec claude --plugin-dir /path/to/claude-code-safety-net "$@"
SCRIPT
chmod +x /tmp/claude-safety-test.shLaunched the session via agent-deck with the wrapper registered as a custom tool, providing an isolated Claude Code instance with only the local safety-net plugin. Test: Dangerous command triggers ask promptSent: Result — Claude Code showed an interactive confirmation dialog: The user can approve with Enter or deny with Esc — no need to Review feedback addressed
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/bin/hooks/claude-code.ts`:
- Around line 7-12: Rename the module-private helper outputDecision to
_outputDecision and change its parameter types from optional to explicit
undefined types: command: string | undefined and segment: string | undefined;
then update every call site that uses outputDecision(...) to call
_outputDecision(...) with the same arguments (e.g., the call that logs parse
failure and the call that forwards result.reason, command, result.segment) so
the new name and signature are consistent across the module.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e45693d1-bae8-4d37-82a4-af85b6878f9c
⛔ Files ignored due to path filters (1)
dist/bin/cc-safety-net.jsis excluded by!**/dist/**
📒 Files selected for processing (5)
README.mdsrc/bin/hooks/claude-code.tstests/bin/cli-statusline.test.tstests/bin/hooks/claude-code-hook.test.tstests/bin/hooks/copilot-cli-hook.test.ts
✅ Files skipped from review due to trivial changes (1)
- README.md
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/bin/cli-statusline.test.ts
- tests/bin/hooks/claude-code-hook.test.ts
- tests/bin/hooks/copilot-cli-hook.test.ts
| function outputDecision( | ||
| decision: 'deny' | 'ask', | ||
| reason: string, | ||
| command?: string, | ||
| segment?: string, | ||
| ): void { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify module-private naming and optional-parameter usage in this hook file.
rg -nP --type=ts '^\s*function\s+(?!_)[A-Za-z][A-Za-z0-9_]*\s*\(' src/bin/hooks/claude-code.ts -C1
rg -nP --type=ts '^\s*function\s+[A-Za-z_][A-Za-z0-9_]*\s*\([^)]*\?:' src/bin/hooks/claude-code.ts -C1Repository: kenryu42/claude-code-safety-net
Length of output: 134
🏁 Script executed:
cat -n src/bin/hooks/claude-code.ts | head -100Repository: kenryu42/claude-code-safety-net
Length of output: 2886
Rename module-private helper and use explicit undefined typing per coding guidelines.
The outputDecision function (lines 7-12) should be _outputDecision and parameters should use command: string | undefined and segment: string | undefined instead of optional syntax (command?: string, segment?: string).
Suggested refactor
-function outputDecision(
+function _outputDecision(
decision: 'deny' | 'ask',
reason: string,
- command?: string,
- segment?: string,
+ command: string | undefined = undefined,
+ segment: string | undefined = undefined,
): void {Update call sites:
- Line 50:
_outputDecision('deny', 'Failed to parse hook input JSON (strict mode)'); - Line 86:
_outputDecision(askMode && !strict ? 'ask' : 'deny', result.reason, command, result.segment);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/bin/hooks/claude-code.ts` around lines 7 - 12, Rename the module-private
helper outputDecision to _outputDecision and change its parameter types from
optional to explicit undefined types: command: string | undefined and segment:
string | undefined; then update every call site that uses outputDecision(...) to
call _outputDecision(...) with the same arguments (e.g., the call that logs
parse failure and the call that forwards result.reason, command, result.segment)
so the new name and signature are consistent across the module.
1 participant
What
Adds
SAFETY_NET_ASK=1environment variable. When set, dangerous commands returnpermissionDecision: 'ask'instead of'deny', prompting the user for interactive confirmation rather than blocking outright.Why
When Safety Net blocks a command, there's no hands-on-keyboard way to approve it. The user has to either:
/copyto grab the blocked command and paste it backThis creates unnecessary friction when you do want the agent to run a flagged command. Ask mode keeps the safety analysis and warning visible while letting the user approve with a single keypress — similar to how
--force-with-leaseis safer than--forcebut still requires intent.This is opt-in — default behavior is unchanged (hard deny). And it's not truly "soft" — the user must actively confirm each flagged command. The safety analysis still runs, the warning is still shown, and the audit log still records every flagged command.
Claude Code's hook protocol explicitly supports
'ask'as apermissionDecisionalongside'allow'and'deny'(docs).Changes
src/types.ts— WidenHookOutput.permissionDecisionto include'ask'src/core/format.ts— Context-aware message: "FLAGGED by Safety Net" (ask) vs "BLOCKED by Safety Net" (deny), with appropriate footer textsrc/bin/hooks/claude-code.ts— ReadSAFETY_NET_ASK, return'ask'instead of'deny'src/bin/hooks/copilot-cli.ts— Samesrc/bin/statusline.ts— Show ❓ emoji for ask modesrc/bin/doctor/environment.ts— Added to env var diagnosticssrc/bin/help.ts— Added to help outputCLAUDE.md,AGENTS.md,README.md— Documented the new env varGemini CLI and OpenCode are left as hard-deny only — their hook protocols don't support interactive confirmation.
Strict mode parse failures (
SAFETY_NET_STRICT=1) always hard-deny regardless of ask mode.Test coverage
'ask', safe commands still pass through, strict parse failures still hard deny'ask', safe commands still pass throughSAFETY_NET_ASK=1FLAGGEDheader and confirmation footer in ask mode,BLOCKEDheader in default modeSummary by CodeRabbit
Release Notes
New Features
SAFETY_NET_ASK=1) that prompts users to approve or deny flagged commands instead of automatic blocking. Strict mode takes precedence.Documentation