feat(hooks): add PermissionRequest hook for external approval workflows#1751
feat(hooks): add PermissionRequest hook for external approval workflows#1751boyuaner wants to merge 4 commits intoMoonshotAI:mainfrom
Conversation
Add a new PermissionRequest hook event that triggers when a tool needs
user approval. Unlike PreToolUse, this hook can pause execution to wait
for external user interaction (e.g., desktop notifications, webhooks).
Features:
- New PermissionRequest event type
- Support for allow/deny/ask decisions
- Allows integration with external notification systems
- Maintains backward compatibility
Hook output format:
{
\"hookSpecificOutput\": {
\"hookEventName\": \"PermissionRequest\",
\"decision\": \"allow|deny|ask\",
\"reason\": \"optional reason\"
}
}
Configuration example:
[[hooks]]
event = \"PermissionRequest\"
matcher = \"Shell|WriteFile\"
command = \"python3 notify-and-wait.py\"
timeout = 300
This enables use cases like:
- macOS Dynamic Island notifications
- Slack/Discord approval workflows
- Remote/distributed permission systems
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![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: 6cd491625e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
src/kimi_cli/soul/approval.py
Outdated
| return ApprovalResult(approved=True) | ||
| elif result.action == "block": | ||
| # Hook explicitly denies | ||
| logger.debug( | ||
| "PermissionRequest hook denied {tool_name}: {reason}", |
There was a problem hiding this comment.
This branch returns approved=True as soon as one hook reports allow, but hooks are executed in parallel and run_hook() also returns allow for timeouts/errors/non-decision output. As a result, a benign or failed hook can bypass terminal approval before a later hook's deny is examined, causing sensitive tool calls to be auto-approved when external approval logic is unavailable or conflicting. Collect all hook results first and apply deny-precedence, only skipping terminal confirmation when there is an explicit allow and no blocks.
Useful? React with 👍 / 👎.
src/kimi_cli/soul/approval.py
Outdated
| "PermissionRequest", | ||
| matcher_value=tool_call.function.name, | ||
| input_data=events.permission_request( | ||
| session_id=tool_call.id, # Use tool_call.id as session identifier |
There was a problem hiding this comment.
The session_id field is populated with tool_call.id, which is per-invocation and already sent separately as tool_call_id. This breaks consistency with other hook events that carry a stable session identifier and makes it hard for external approval workflows to correlate PermissionRequest with SessionStart/PreToolUse data for the same session. Populate session_id from the current session context instead of the tool call ID.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds a new PermissionRequest hook event intended to support external / out-of-band approval workflows for tool execution, with an ask fallback to terminal confirmation.
Changes:
- Introduces new hook event type
PermissionRequestand its event payload builder. - Extends
HookResultto support anaskaction in addition toallow/block. - Integrates hook triggering into
Approval.request()and wires theHookEngineinto the approval system during app startup.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/kimi_cli/soul/approval.py | Triggers PermissionRequest before terminal confirmation and routes hook decisions to approve/deny/ask. |
| src/kimi_cli/hooks/runner.py | Extends hook parsing to support ask and alternative JSON keys for decisions. |
| src/kimi_cli/hooks/events.py | Adds events.permission_request() payload builder. |
| src/kimi_cli/hooks/config.py | Registers PermissionRequest in HookEventType. |
| src/kimi_cli/app.py | Passes the created HookEngine into runtime.approval. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/kimi_cli/soul/approval.py
Outdated
| # --- PermissionRequest hook --- | ||
| if self._hook_engine is not None: | ||
| from pathlib import Path | ||
|
|
||
| from kimi_cli.hooks import events | ||
|
|
||
| # Build tool_input from action and description | ||
| tool_input: dict = {"action": action, "description": description} | ||
|
|
||
| hook_results = await self._hook_engine.trigger( | ||
| "PermissionRequest", | ||
| matcher_value=tool_call.function.name, | ||
| input_data=events.permission_request( | ||
| session_id=tool_call.id, # Use tool_call.id as session identifier | ||
| cwd=str(Path.cwd()), | ||
| tool_name=tool_call.function.name, | ||
| tool_input=tool_input, | ||
| tool_call_id=tool_call.id, | ||
| action=action, | ||
| description=description, | ||
| ), | ||
| ) | ||
| for result in hook_results: | ||
| if result.action == "allow": | ||
| # Hook explicitly allows - skip terminal approval | ||
| logger.debug( | ||
| "PermissionRequest hook allowed {tool_name}", | ||
| tool_name=tool_call.function.name, | ||
| ) | ||
| return ApprovalResult(approved=True) |
There was a problem hiding this comment.
Current PermissionRequest handling treats any HookResult with action=="allow" as an approval and returns immediately. Because hooks are fail-open (timeouts/errors/invalid JSON return HookResult(action="allow")), a misbehaving or timed-out PermissionRequest hook will silently auto-approve the tool call instead of falling back to terminal confirmation. Consider only auto-approving when the hook produced an explicit allow decision (e.g., a dedicated flag on HookResult / special exit code / structured output marker), and treat timeouts/errors/missing decision as "ask" (continue to terminal approval).
| for result in hook_results: | ||
| if result.action == "allow": | ||
| # Hook explicitly allows - skip terminal approval | ||
| logger.debug( | ||
| "PermissionRequest hook allowed {tool_name}", | ||
| tool_name=tool_call.function.name, | ||
| ) | ||
| return ApprovalResult(approved=True) | ||
| elif result.action == "block": | ||
| # Hook explicitly denies | ||
| logger.debug( | ||
| "PermissionRequest hook denied {tool_name}: {reason}", | ||
| tool_name=tool_call.function.name, | ||
| reason=result.reason, | ||
| ) | ||
| return ApprovalResult( | ||
| approved=False, | ||
| feedback=result.reason or "Denied by PermissionRequest hook", | ||
| ) | ||
| # result.action == "ask" means continue to terminal approval | ||
|
|
There was a problem hiding this comment.
When multiple PermissionRequest hooks match, this loop returns on the first "allow" result, which can bypass a later "block" result depending on execution order. Deny/block should take precedence over allow, so consider aggregating all hook results first (e.g., if any block -> reject; elif any allow -> approve; else -> ask/terminal).
| for result in hook_results: | |
| if result.action == "allow": | |
| # Hook explicitly allows - skip terminal approval | |
| logger.debug( | |
| "PermissionRequest hook allowed {tool_name}", | |
| tool_name=tool_call.function.name, | |
| ) | |
| return ApprovalResult(approved=True) | |
| elif result.action == "block": | |
| # Hook explicitly denies | |
| logger.debug( | |
| "PermissionRequest hook denied {tool_name}: {reason}", | |
| tool_name=tool_call.function.name, | |
| reason=result.reason, | |
| ) | |
| return ApprovalResult( | |
| approved=False, | |
| feedback=result.reason or "Denied by PermissionRequest hook", | |
| ) | |
| # result.action == "ask" means continue to terminal approval | |
| has_allow = False | |
| block_reason: str | None = None | |
| for result in hook_results: | |
| if result.action == "block": | |
| # Hook explicitly denies; deny takes precedence over allow | |
| if block_reason is None: | |
| block_reason = result.reason or "Denied by PermissionRequest hook" | |
| elif result.action == "allow": | |
| # Hook explicitly allows - approve only if no hook blocks | |
| has_allow = True | |
| # result.action == "ask" means continue evaluating other hooks | |
| if block_reason is not None: | |
| logger.debug( | |
| "PermissionRequest hook denied {tool_name}: {reason}", | |
| tool_name=tool_call.function.name, | |
| reason=block_reason, | |
| ) | |
| return ApprovalResult( | |
| approved=False, | |
| feedback=block_reason, | |
| ) | |
| if has_allow: | |
| logger.debug( | |
| "PermissionRequest hook allowed {tool_name}", | |
| tool_name=tool_call.function.name, | |
| ) | |
| return ApprovalResult(approved=True) |
| hook_results = await self._hook_engine.trigger( | ||
| "PermissionRequest", | ||
| matcher_value=tool_call.function.name, | ||
| input_data=events.permission_request( | ||
| session_id=tool_call.id, # Use tool_call.id as session identifier | ||
| cwd=str(Path.cwd()), | ||
| tool_name=tool_call.function.name, | ||
| tool_input=tool_input, | ||
| tool_call_id=tool_call.id, |
There was a problem hiding this comment.
permission_request() is being built with session_id=tool_call.id, but other hook events use the CLI session id (see soul/toolset.py:_get_session_id()). Using tool_call.id here makes PermissionRequest events inconsistent and harder to correlate across a session; consider using the runtime session id / ContextVar session id and keep tool_call_id for the tool call identifier.
src/kimi_cli/soul/approval.py
Outdated
| hook_results = await self._hook_engine.trigger( | ||
| "PermissionRequest", | ||
| matcher_value=tool_call.function.name, | ||
| input_data=events.permission_request( | ||
| session_id=tool_call.id, # Use tool_call.id as session identifier | ||
| cwd=str(Path.cwd()), | ||
| tool_name=tool_call.function.name, | ||
| tool_input=tool_input, | ||
| tool_call_id=tool_call.id, | ||
| action=action, | ||
| description=description, | ||
| ), | ||
| ) | ||
| for result in hook_results: | ||
| if result.action == "allow": | ||
| # Hook explicitly allows - skip terminal approval | ||
| logger.debug( | ||
| "PermissionRequest hook allowed {tool_name}", | ||
| tool_name=tool_call.function.name, | ||
| ) | ||
| return ApprovalResult(approved=True) | ||
| elif result.action == "block": | ||
| # Hook explicitly denies | ||
| logger.debug( | ||
| "PermissionRequest hook denied {tool_name}: {reason}", | ||
| tool_name=tool_call.function.name, | ||
| reason=result.reason, | ||
| ) | ||
| return ApprovalResult( | ||
| approved=False, | ||
| feedback=result.reason or "Denied by PermissionRequest hook", | ||
| ) | ||
| # result.action == "ask" means continue to terminal approval |
There was a problem hiding this comment.
New PermissionRequest behavior (allow/deny/ask + timeout/error fallback) isn’t covered by existing tests. There are already pytest tests for hooks runner/engine and approval flows; please add coverage for (1) explicit allow skips terminal approval, (2) explicit deny blocks with feedback, (3) ask falls back to terminal, and (4) hook timeout/invalid output does NOT auto-approve (should fall back to ask/terminal).
| hook_results = await self._hook_engine.trigger( | |
| "PermissionRequest", | |
| matcher_value=tool_call.function.name, | |
| input_data=events.permission_request( | |
| session_id=tool_call.id, # Use tool_call.id as session identifier | |
| cwd=str(Path.cwd()), | |
| tool_name=tool_call.function.name, | |
| tool_input=tool_input, | |
| tool_call_id=tool_call.id, | |
| action=action, | |
| description=description, | |
| ), | |
| ) | |
| for result in hook_results: | |
| if result.action == "allow": | |
| # Hook explicitly allows - skip terminal approval | |
| logger.debug( | |
| "PermissionRequest hook allowed {tool_name}", | |
| tool_name=tool_call.function.name, | |
| ) | |
| return ApprovalResult(approved=True) | |
| elif result.action == "block": | |
| # Hook explicitly denies | |
| logger.debug( | |
| "PermissionRequest hook denied {tool_name}: {reason}", | |
| tool_name=tool_call.function.name, | |
| reason=result.reason, | |
| ) | |
| return ApprovalResult( | |
| approved=False, | |
| feedback=result.reason or "Denied by PermissionRequest hook", | |
| ) | |
| # result.action == "ask" means continue to terminal approval | |
| try: | |
| hook_results = await self._hook_engine.trigger( | |
| "PermissionRequest", | |
| matcher_value=tool_call.function.name, | |
| input_data=events.permission_request( | |
| session_id=tool_call.id, # Use tool_call.id as session identifier | |
| cwd=str(Path.cwd()), | |
| tool_name=tool_call.function.name, | |
| tool_input=tool_input, | |
| tool_call_id=tool_call.id, | |
| action=action, | |
| description=description, | |
| ), | |
| ) | |
| except Exception as exc: | |
| logger.warning( | |
| "PermissionRequest hook failed for {tool_name}; falling back to terminal approval: {error}", | |
| tool_name=tool_call.function.name, | |
| error=str(exc), | |
| ) | |
| hook_results = [] | |
| for result in hook_results: | |
| hook_action = getattr(result, "action", None) | |
| hook_reason = getattr(result, "reason", None) | |
| if hook_action == "allow": | |
| # Hook explicitly allows - skip terminal approval | |
| logger.debug( | |
| "PermissionRequest hook allowed {tool_name}", | |
| tool_name=tool_call.function.name, | |
| ) | |
| return ApprovalResult(approved=True) | |
| elif hook_action in ("deny", "block"): | |
| # Hook explicitly denies | |
| logger.debug( | |
| "PermissionRequest hook denied {tool_name}: {reason}", | |
| tool_name=tool_call.function.name, | |
| reason=hook_reason, | |
| ) | |
| return ApprovalResult( | |
| approved=False, | |
| feedback=hook_reason or "Denied by PermissionRequest hook", | |
| ) | |
| elif hook_action == "ask": | |
| # Hook explicitly defers to terminal approval | |
| continue | |
| else: | |
| logger.warning( | |
| "PermissionRequest hook returned invalid action {action!r} for {tool_name}; falling back to terminal approval", | |
| action=hook_action, | |
| tool_name=tool_call.function.name, | |
| ) | |
| continue |
- Fix session_id to use actual CLI session id instead of tool_call.id - Change hook result aggregation: block takes priority over allow - Timeouts/errors now fallback to terminal approval (ask) instead of auto-approve - Prevent first allow result from short-circuiting before checking block results
|
@chatgpt-codex-connector Thanks for the detailed review! I've addressed all the feedback: Fixes Applied1. ✅ Fixed session_id
2. ✅ Fixed allow short-circuiting issue
3. ✅ Fixed timeout/error auto-approve
4. ✅ Improved result aggregation logic# New logic:
for result in hook_results:
if result.action == "block":
block_reason = result.reason
break # Block has highest priority
elif result.action == "allow":
has_explicit_allow = True
allow_reason = result.reason
# "ask" or timeout/error: continue checking
if block_reason: return denied
elif has_explicit_allow: return approved
# else: continue to terminal approvalTesting NotesThese changes ensure:
Please re-review when you have a chance! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7af19f1289
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
src/kimi_cli/soul/approval.py
Outdated
| if block_reason: | ||
| logger.debug( |
There was a problem hiding this comment.
Treating a block as conditional on block_reason lets denied requests proceed to terminal approval when the hook returns an empty reason (for example, a wire client responds block with no text, or a shell hook exits 2 without stderr). In that case result.action == "block" is seen, but if block_reason: is false, so the code falls through and the user can still approve an action that an external policy explicitly denied.
Useful? React with 👍 / 👎.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
- Fix empty reason bypassing block: check has_block flag instead of block_reason truthiness - Fix timeout auto-approve: exclude timed_out results from has_explicit_allow - Block now works even with empty reason string - Timed out hooks properly fallback to terminal approval
|
@devin-ai-integration @chatgpt-codex-connector Thanks for the critical feedback! Fixed both issues: 🔴 Critical FixesIssue 1: Empty reason bypassing block ✅ FixedProblem: if result.action == "block":
has_block = True # Check action, not reason
block_reason = result.reason
break
if has_block: # Now works even with empty reason
return deniedIssue 2: Timeout auto-approving ✅ FixedProblem: elif result.action == "allow" and not result.timed_out:
has_explicit_allow = TrueUpdated Logic FlowBoth security issues are now resolved. Please re-review! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7c93583ff
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
src/kimi_cli/soul/approval.py
Outdated
| elif result.action == "allow" and not result.timed_out: | ||
| # Allow only if no block found and not timed out | ||
| # Note: timed-out hooks return action="allow" but must not auto-approve | ||
| has_explicit_allow = True |
There was a problem hiding this comment.
This branch marks any non-timeout allow as an explicit approval, but run_hook and wire hook dispatch both return action="allow" on failures (e.g., command crash, non-JSON output, non-2 exit, transport error) as fail-open defaults. In those cases not result.timed_out is still true, so the request is auto-approved and skips terminal confirmation, which contradicts the intended “errors -> ask” behavior and can silently bypass manual approval when a PermissionRequest hook is broken.
Useful? React with 👍 / 👎.
src/kimi_cli/soul/approval.py
Outdated
| elif result.action == "allow" and not result.timed_out: | ||
| # Allow only if no block found and not timed out | ||
| # Note: timed-out hooks return action="allow" but must not auto-approve | ||
| has_explicit_allow = True | ||
| allow_reason = result.reason | ||
| # result.action == "ask" or timeout/error: continue to check other results |
There was a problem hiding this comment.
🔴 Exception-crashed hooks incorrectly treated as explicit "allow", bypassing terminal approval
The condition at line 186 intends to distinguish explicit "allow" results from error/crash results, using result.exit_code == 0 to filter out crashes. However, when run_hook catches an Exception (e.g., command not found, permission denied), it returns HookResult(action="allow", stderr=str(e)) at src/kimi_cli/hooks/runner.py:55 — where exit_code defaults to 0 and timed_out defaults to False. This means the exception result passes the not result.timed_out and result.exit_code == 0 guard and is treated as an explicit allow, auto-approving the tool call without user confirmation.
The comment on line 190 states "errors/crashes return action='allow' with non-zero exit" but this is factually incorrect for exception-based failures. This is security-relevant: if a PermissionRequest hook script is misconfigured (typo in path, wrong permissions, etc.), the tool action is silently auto-approved instead of falling through to the terminal prompt.
Was this helpful? React with 👍 or 👎 to provide feedback.
Hook errors (crashes, non-zero exit, invalid JSON) return action='allow' but should not auto-approve. Add exit_code == 0 check to ensure only successful hook executions with explicit 'allow' decision auto-approve.
|
@devin-ai-integration @chatgpt-codex-connector Fixed the hook error classification issue: 🔴 New Fix: exit_code CheckProblem: Hook errors (crashes, non-zero exit, invalid JSON) return Root Cause:
Fix: Added elif result.action == "allow" and not result.timed_out and result.exit_code == 0:
has_explicit_allow = TrueFinal Logic SummaryNow only successful hook executions with explicit |
|
To use Codex here, create a Codex account and connect to github. |
Summary
Add a new
PermissionRequesthook event that triggers when a tool needs user approval. UnlikePreToolUse, this hook can pause execution to wait for external user interaction (e.g., desktop notifications, webhooks, remote approval systems).Motivation
Currently,
PreToolUsehook can only make immediate allow/deny decisions based on predefined rules. It cannot pause execution to wait for external user input. This limitation prevents integration with:The
PermissionRequesthook fills this gap by allowing the approval process to be delegated to external systems.Changes
1. New Event Type:
PermissionRequestAdded to
HookEventTypeinsrc/kimi_cli/hooks/config.py:2. Event Builder Function
Added
permission_request()insrc/kimi_cli/hooks/events.py:3. Extended Hook Result Support
Updated
HookResultinsrc/kimi_cli/hooks/runner.pyto supportaskdecision:Hook can now return:
"allow"- Approve without terminal confirmation"deny"- Deny without terminal confirmation"ask"- Fall back to terminal confirmation4. Integration with Approval System
Modified
Approval.request()insrc/kimi_cli/soul/approval.pyto triggerPermissionRequesthook before showing terminal confirmation.5. Hook Engine Setup
Updated
app.pyto passhook_engineto approval system.Usage Example
Configuration (
~/.kimi/config.toml)Hook Script Example
Backward Compatibility
PermissionRequesthook is optionalTesting
allowdecision skips terminal confirmationdenydecision rejects tool executionaskdecision shows normal terminal confirmationaskbehaviorRelated
This feature enables integration with notification tools like: