feat: hard harness approval gate for dangerous tool calls - #17
Merged
Conversation
Phase 1+2 of channel approval gate: - Add approval_timeout_secs config field (default: 120s) - Define ChannelApprovalAdapter async trait with send/receive methods - Define PendingApproval + PlatformRef for per-channel correlation - Extend ApprovalRequest with request_id, channel, risk_level, created_at - Add Timeout variant to ApprovalResponse - Extend ApprovalLogEntry with risk_level, request_id, response_time_ms - Add async request_approval() to ApprovalManager with timeout wrapper - Add needs_shell_approval() for harness-enforced risk classification - Add parse_approval_input() for reusable response parsing - Make CommandRiskLevel serializable - Add MockApprovalAdapter, HangingApprovalAdapter, FailingApprovalAdapter test fixtures
…agent loop Phase 3 (US1) — auto-approve path + CLI adapter: - Implement CliApprovalAdapter using spawn_blocking for stdin reads - Add approval_adapter parameter to run_tool_call_loop - Route through adapter.request_approval() when adapter is available - Preserve legacy is_non_interactive/prompt_cli fallback for backward compat - Wire CliApprovalAdapter in CLI interactive mode - Channel callers pass None for adapter (will get adapters in Phase 4)
Phase 5 (US3) — shell command risk is now harness-enforced: - Refactor validate_command_execution to return (CommandRiskLevel, bool) where bool indicates whether approval is needed, instead of accepting an approved param from the LLM - Remove approved parameter from shell tool JSON schema - Remove approved parameter from all cron tool schemas (cron_add, cron_run, cron_update, schedule) - Update all callers: shell.rs, cron/mod.rs, cron/scheduler.rs, gateway/api.rs, tools/skill_tool.rs, fuzz target - Make CommandRiskLevel serializable for use in ApprovalRequest - High-risk commands blocked by policy still return hard Err (no approval can override) - Medium/high-risk approval is now the agent loop's responsibility via ApprovalManager.needs_shell_approval()
…hannel wiring Phase 4 (US2) — channel approval adapters: - Add TelegramApprovalAdapter: sendMessage with force_reply, polls getUpdates with reply_to_message correlation and chat_id guard - Add SlackApprovalAdapter: chat.postMessage in thread, polls conversations.replies for newer messages - Add GatewayApprovalAdapter: sends approval_request JSON frame via mpsc, awaits approval_response via oneshot channel - Add create_approval_adapter() to Channel trait (default: None) - Implement create_approval_adapter for TelegramChannel and SlackChannel - Wire adapter creation in channel message processing path - Change channel approval_manager from for_non_interactive to from_config since channels now have interactive adapters - Sequential approval for multi-tool turns preserved via existing should_execute_tools_in_parallel logic - "Always" flow already handled in ApprovalManager.request_approval
Phase 6 (US4) — timeout handling:
- Agent loop now handles ApprovalResponse::Timeout alongside No, denying
the tool call with "Approval timed out after {N}s" message
- Timeout wrapping was already implemented in ApprovalManager.request_approval
from Phase 2 (tokio::time::timeout around adapter.receive_approval_response)
- Late responses after timeout are naturally ignored because the
oneshot/correlation handle is dropped
Phase 7 (US5) — audit trail: - Add record_auto_approved() for logging auto-approved tool calls with tracing::debug level - Add structured tracing events for all 4 decision types: info for prompted/denied/timed-out, debug for auto-approved - ApprovalLogEntry already captures risk_level, request_id, and response_time_ms from Phase 2 implementation - request_approval() already records all prompted/timeout decisions with extended audit fields
Phase 8 (Polish): - Fix cast_possible_truncation: use i32::try_from for Telegram message_id, i64::from for comparison, and #[allow] for elapsed_ms u128→u64 - Fix format! appended to String: use write! macro instead - Remove unused re-exports (GatewayApprovalAdapter, PendingApproval, PlatformRef from approval/mod.rs) - Apply cargo fmt across all changed files - All 4275 tests pass, clippy clean, fmt clean
- Extract format_approval_prompt() shared by CLI, Telegram, Slack adapters (eliminates 3 near-duplicate message formatting blocks) - Consolidate "Always" allowlist logic into maybe_add_to_allowlist() helper (was duplicated in request_approval and record_decision) - Store reqwest::Client in SlackApprovalAdapter instead of creating per-call (avoids repeated TLS/connection pool setup) - Extract telegram_api_url() free function (was duplicated in adapter and TelegramChannel) - Delegate needs_shell_approval() to validate_command_execution() instead of reimplementing risk→approval mapping - Accept channel parameter in ApprovalRequest::new() instead of post-mutation - Remove legacy prompt_cli_interactive formatting (now uses shared helper)
github-actions
Bot
added
agent
channel
tool
tool:shell
gateway
cron
security
channel:telegram
config
tool:cron
channel:slack
labels
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
1 participant
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
ChannelApprovalAdapterasync trait with 4 implementations: CLI (stdin/stderr), Telegram (reply-to correlation), Slack (thread replies), Gateway (JSON frames via oneshot)approvedparameter from shell tool — risk classification is now entirely harness-enforced viaSecurityPolicyapproval_timeout_secs, default 120s) with auto-denial on expirycreate_approval_adapter()toChanneltrait for per-message adapter creationTest plan
-D warnings)cargo fmt --checkcleanneeds_approval,needs_shell_approval,parse_approval_input,ApprovalResponse::Timeoutserde roundtrip, extended audit fieldsrequest_approvalwith MockAdapter (yes/no/always/timeout flows)