← Back to PRs

#20435: fix(exec): prioritize user 'always allow' config over tool defaults (ai-assisted fix)

by ChisomUma open 2026-02-18 22:50 View on GitHub →
channel: whatsapp-web agents size: M
## Summary Problem: - exec tool calls were invoking the approval popup even when the user configured security: "full" and ask: "off". - Why it matters: This annoyed users who explicitly opted out of security prompts, effectively breaking "Always Allow". - What changed: Updated `src/agents/bash-tools.exec.ts` to respect the user's configuration as the final authority if the tool doesn't explicitly request stricter security. - What did NOT change (scope boundary): Did not change the default security for sandboxed environments or other tools; specific tool-level overrides still work if passed. ## Change Type (select all) - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Docs - [ ] Security hardening - [ ] Chore/infra ## Scope (select all touched areas) - [ ] Gateway / orchestration - [x] Skills / tool execution - [ ] Auth / tokens - [ ] Memory / storage - [ ] Integrations - [ ] API / contracts - [ ] UI / DX - [ ] CI/CD / infra ## Linked Issue/PR - Closes #20141 ## User-visible / Behavior Changes Users who have set their Exec Approvals to "Always Allow" / "Never Ask" will no longer be prompted for approval when the agent runs shell commands on the gateway host. ## Security Impact (required) - New permissions/capabilities? (`No`) - Secrets/tokens handling changed? (`No`) - New/changed network calls? (`No`) - Command/tool execution surface changed? (`Yes`) - Data access scope changed? (`No`) Risk: "Always Allow" users are now truly always allowing commands without prompts (as they intended). Mitigation: This only affects users who explicitly opted into security: full. Default configuration remains strict (deny or allowlist depending on context). ## Repro + Verification Environment OS: macOS 26.2 (Apple Silicon) Runtime/container: Node.js (local gateway) Relevant config (redacted): ~/.openclaw/exec-approvals.json: {"defaults":{"security":"full","ask":"off"},...} Steps Set OpenClaw permissions to "Always Allow" and "Never Ask". Ask the agent to run a simple command (e.g., echo hello). Observe that it runs immediately without a popup. Expected Command executes silently and returns output. Actual (Before fix) Popup appeared asking for approval. (After fix) Command executes silently. ## Evidence ``` ✓ src/agents/bash-tools.exec.permission.test.ts (1 test) ✓ exec permission bug reproduction (1) ✓ respects user 'Always Allow' / 'Never Ask' settings even if defaults are stricter ``` ## Human Verification (required) What you personally verified (not just CI), and how: - Verified scenarios: Ran the reproduction test case pnpm test src/agents/bash-tools.exec.permission.test.ts. - Edge cases checked: Verified that user configuration is respected even when defaults are strict. - What you did not verify: Did not manually click through the UI flow (relied on e2e test simulation). ## Compatibility / Migration - Backward compatible? (`Yes`) - Config/env changes? (`No`) - Migration needed? (`No`) ## Failure Recovery (if this breaks) - How to disable/revert this change quickly: - Files/config to restore: - Known bad symptoms reviewers should watch for: ## Risks and Mitigations - How to disable/revert this change quickly: Revert the PR. - Files/config to restore: src/agents/bash-tools.exec.ts - Known bad symptoms reviewers should watch for: Unexpected command executions if logic is too permissive (but it defaults to user config). <!-- greptile_comment --> <h3>Greptile Summary</h3> Fixed exec tool to respect user's "Always Allow" configuration instead of being overridden by tool defaults. However, the fix introduces a critical security regression. ## Key Changes - Modified `src/agents/bash-tools.exec.ts` to check if security/ask params were explicitly requested before applying `minSecurity`/`maxAsk` logic - Added test to verify user config is respected when no explicit params provided - Updated test harness type definitions for better type safety ## Critical Issue The fix correctly handles the case when NO security params are provided (respects user's `security: "full"`, `ask: "off"`), but introduces a security vulnerability when params ARE provided: **Bug**: If user configures `security: "deny"` but a tool explicitly requests `security: "allowlist"`, the current fix ignores the user's stricter "deny" setting and allows execution with "allowlist". **Root cause**: Lines 707-709 bypass `approvals.agent.security` entirely when `requestedSecurity` is truthy, only using `minSecurity(configuredSecurity, requestedSecurity)`. **Fix needed**: Always apply `minSecurity(effectiveConfiguredSecurity, approvals.agent.security)` to ensure user's stricter settings are never bypassed. <h3>Confidence Score: 2/5</h3> - This PR has a critical security logic flaw that could allow bypassing user-configured security restrictions - While the fix correctly addresses the reported bug (user "Always Allow" being overridden by tool defaults), it introduces a worse security regression by allowing tools with explicit security params to bypass stricter user-configured restrictions. A user with `security: "deny"` could have their setting ignored if a tool requests `security: "allowlist"`. - src/agents/bash-tools.exec.ts requires immediate attention to fix the security logic regression <sub>Last reviewed commit: fa5448b</sub> <!-- greptile_other_comments_section --> <sub>(2/5) Greptile learns from your feedback when you react with thumbs up/down!</sub> <!-- /greptile_comment -->

Most Similar PRs