diff --git a/app/src/components/chat/ApprovalRequestCard.tsx b/app/src/components/chat/ApprovalRequestCard.tsx index 55d5b9bc3..c9dfe8ec3 100644 --- a/app/src/components/chat/ApprovalRequestCard.tsx +++ b/app/src/components/chat/ApprovalRequestCard.tsx @@ -8,13 +8,16 @@ import { useAppDispatch } from '../../store/hooks'; import Button from '../ui/Button'; /** - * Binary v1 decision surface. The backend `approval.decide` RPC also accepts - * `approve_always_for_tool`, but the locked v1 contract is yes / no — a typed - * `yes`/`no` chat reply is the equivalent server-side path. + * Decision surface for a parked tool call. `approve_once` / `deny` decide the + * current call only; `approve_always_for_tool` additionally persists the tool + * onto the user's `autonomy.auto_approve` ("Always allow") list so the gate + * skips prompting for it on future turns (managed/removable in Settings → Agent + * access). A typed `yes`/`no` chat reply is the equivalent server-side path for + * the once/deny decisions. */ const log = debug('openhuman:chat:approval-card'); -type BinaryDecision = 'approve_once' | 'deny'; +type Decision = 'approve_once' | 'approve_always_for_tool' | 'deny'; interface Props { threadId: string; @@ -31,10 +34,10 @@ interface Props { export const ApprovalRequestCard: React.FC = ({ threadId, approval }) => { const { t } = useT(); const dispatch = useAppDispatch(); - const [deciding, setDeciding] = useState(null); + const [deciding, setDeciding] = useState(null); const [errorMsg, setErrorMsg] = useState(null); - const decide = async (decision: BinaryDecision) => { + const decide = async (decision: Decision) => { if (deciding) return; setDeciding(decision); setErrorMsg(null); @@ -80,7 +83,7 @@ export const ApprovalRequestCard: React.FC = ({ threadId, approval }) => {errorMsg &&

⚠ {errorMsg}

} -
+
+ + + ))} + + )} + + {/* Auto-save status — changes persist on selection; no manual save. */}
{error ? ( diff --git a/app/src/components/settings/panels/__tests__/AgentAccessPanel.test.tsx b/app/src/components/settings/panels/__tests__/AgentAccessPanel.test.tsx index 0a8fcdb23..1d6960ee6 100644 --- a/app/src/components/settings/panels/__tests__/AgentAccessPanel.test.tsx +++ b/app/src/components/settings/panels/__tests__/AgentAccessPanel.test.tsx @@ -18,6 +18,7 @@ const autonomy = (overrides: Partial = {}): AutonomySettings = trusted_roots: [], allow_tool_install: true, max_actions_per_hour: 0, + auto_approve: [], ...overrides, }); @@ -113,6 +114,31 @@ describe('AgentAccessPanel', () => { expect((screen.getByRole('checkbox') as HTMLInputElement).checked).toBe(true); }); + it('shows the empty "always-allow" state when no tools are allow-listed', async () => { + renderWithProviders(); + expect(await screen.findByText('Always-allowed tools')).toBeInTheDocument(); + expect(screen.getByText('No always-allowed tools yet.')).toBeInTheDocument(); + }); + + it('lists always-allowed tools and removing one persists the trimmed list', async () => { + mockGet.mockResolvedValue({ result: autonomy({ auto_approve: ['shell', 'curl'] }), logs: [] }); + renderWithProviders(); + + // The allowlist renders each tool name. + expect(await screen.findByText('shell')).toBeInTheDocument(); + expect(screen.getByText('curl')).toBeInTheDocument(); + + // trusted_roots is empty, so the only Remove buttons belong to the + // allowlist. Removing the first entry persists the trimmed list via + // update_autonomy_settings (auto_approve only — other fields untouched). + fireEvent.click(screen.getAllByText('Remove')[0]); + await waitFor(() => + expect(mockUpdate).toHaveBeenLastCalledWith( + expect.objectContaining({ auto_approve: ['curl'] }) + ) + ); + }); + it('surfaces a load error without crashing', async () => { mockGet.mockRejectedValue(new Error('boom')); renderWithProviders(); diff --git a/app/src/components/settings/panels/__tests__/AutonomyPanel.test.tsx b/app/src/components/settings/panels/__tests__/AutonomyPanel.test.tsx index 7a6ceb699..98360b35c 100644 --- a/app/src/components/settings/panels/__tests__/AutonomyPanel.test.tsx +++ b/app/src/components/settings/panels/__tests__/AutonomyPanel.test.tsx @@ -20,6 +20,7 @@ const autonomy = (max_actions_per_hour: number): AutonomySettings => ({ trusted_roots: [], allow_tool_install: false, max_actions_per_hour, + auto_approve: [], }); vi.mock('../../hooks/useSettingsNavigation', () => ({ diff --git a/app/src/lib/i18n/chunks/ar-3.ts b/app/src/lib/i18n/chunks/ar-3.ts index 5496862b8..781460753 100644 --- a/app/src/lib/i18n/chunks/ar-3.ts +++ b/app/src/lib/i18n/chunks/ar-3.ts @@ -428,6 +428,8 @@ const ar3: TranslationMap = { 'channels.yuanbao.savedRestartRequired': 'Channel saved. Restart the app to activate it.', 'channels.yuanbao.unexpectedStatus': 'Unexpected connection status: {status}', 'chat.approval.approve': 'Approve', + 'chat.approval.alwaysAllow': 'Always allow', + 'chat.approval.alwaysAllowHint': 'Stop asking for this tool — add it to your Always-allow list', 'chat.approval.deciding': 'Working…', 'chat.approval.deny': 'Deny', 'chat.approval.error': 'Could not record your decision — try again.', diff --git a/app/src/lib/i18n/chunks/ar-5.ts b/app/src/lib/i18n/chunks/ar-5.ts index ddc3e3a13..a7e11c049 100644 --- a/app/src/lib/i18n/chunks/ar-5.ts +++ b/app/src/lib/i18n/chunks/ar-5.ts @@ -729,6 +729,10 @@ const ar5: TranslationMap = { 'settings.agentAccess.confine.desc': 'Restrict the agent to the workspace directory (plus any granted folders), whichever access mode is selected. When off, it can reach anywhere your user can — except the always-blocked credential and system directories.', 'settings.agentAccess.grantedFolders': 'Granted folders', + 'settings.agentAccess.alwaysAllow': 'Always-allowed tools', + 'settings.agentAccess.alwaysAllowDesc': + 'Tools you marked "Always allow" in chat run without asking. Remove one to be prompted again.', + 'settings.agentAccess.alwaysAllowNone': 'No always-allowed tools yet.', 'settings.agentAccess.grantedDesc': 'Folders the agent may read and write, in addition to the workspace. Credential stores (~/.ssh, ~/.gnupg, ~/.aws, keychains) and system directories (/etc, /System, C:\\Windows, …) are always blocked, even inside a granted folder.', 'settings.agentAccess.noneGranted': 'No folders granted.', diff --git a/app/src/lib/i18n/chunks/bn-3.ts b/app/src/lib/i18n/chunks/bn-3.ts index b52cc846a..d2e981816 100644 --- a/app/src/lib/i18n/chunks/bn-3.ts +++ b/app/src/lib/i18n/chunks/bn-3.ts @@ -432,6 +432,8 @@ const bn3: TranslationMap = { 'channels.yuanbao.savedRestartRequired': 'Channel saved. Restart the app to activate it.', 'channels.yuanbao.unexpectedStatus': 'Unexpected connection status: {status}', 'chat.approval.approve': 'Approve', + 'chat.approval.alwaysAllow': 'Always allow', + 'chat.approval.alwaysAllowHint': 'Stop asking for this tool — add it to your Always-allow list', 'chat.approval.deciding': 'Working…', 'chat.approval.deny': 'Deny', 'chat.approval.error': 'Could not record your decision — try again.', diff --git a/app/src/lib/i18n/chunks/bn-5.ts b/app/src/lib/i18n/chunks/bn-5.ts index a27b97d40..7be6835cf 100644 --- a/app/src/lib/i18n/chunks/bn-5.ts +++ b/app/src/lib/i18n/chunks/bn-5.ts @@ -742,6 +742,10 @@ const bn5: TranslationMap = { 'settings.agentAccess.confine.desc': 'Restrict the agent to the workspace directory (plus any granted folders), whichever access mode is selected. When off, it can reach anywhere your user can — except the always-blocked credential and system directories.', 'settings.agentAccess.grantedFolders': 'Granted folders', + 'settings.agentAccess.alwaysAllow': 'Always-allowed tools', + 'settings.agentAccess.alwaysAllowDesc': + 'Tools you marked "Always allow" in chat run without asking. Remove one to be prompted again.', + 'settings.agentAccess.alwaysAllowNone': 'No always-allowed tools yet.', 'settings.agentAccess.grantedDesc': 'Folders the agent may read and write, in addition to the workspace. Credential stores (~/.ssh, ~/.gnupg, ~/.aws, keychains) and system directories (/etc, /System, C:\\Windows, …) are always blocked, even inside a granted folder.', 'settings.agentAccess.noneGranted': 'No folders granted.', diff --git a/app/src/lib/i18n/chunks/de-3.ts b/app/src/lib/i18n/chunks/de-3.ts index e3ad2c889..6e8a480aa 100644 --- a/app/src/lib/i18n/chunks/de-3.ts +++ b/app/src/lib/i18n/chunks/de-3.ts @@ -444,6 +444,8 @@ const de3: TranslationMap = { 'channels.yuanbao.savedRestartRequired': 'Channel saved. Restart the app to activate it.', 'channels.yuanbao.unexpectedStatus': 'Unexpected connection status: {status}', 'chat.approval.approve': 'Approve', + 'chat.approval.alwaysAllow': 'Always allow', + 'chat.approval.alwaysAllowHint': 'Stop asking for this tool — add it to your Always-allow list', 'chat.approval.deciding': 'Working…', 'chat.approval.deny': 'Deny', 'chat.approval.error': 'Could not record your decision — try again.', diff --git a/app/src/lib/i18n/chunks/de-5.ts b/app/src/lib/i18n/chunks/de-5.ts index 325434486..7d19ab1ad 100644 --- a/app/src/lib/i18n/chunks/de-5.ts +++ b/app/src/lib/i18n/chunks/de-5.ts @@ -770,6 +770,10 @@ const de5: TranslationMap = { 'settings.agentAccess.confine.desc': 'Restrict the agent to the workspace directory (plus any granted folders), whichever access mode is selected. When off, it can reach anywhere your user can — except the always-blocked credential and system directories.', 'settings.agentAccess.grantedFolders': 'Granted folders', + 'settings.agentAccess.alwaysAllow': 'Always-allowed tools', + 'settings.agentAccess.alwaysAllowDesc': + 'Tools you marked "Always allow" in chat run without asking. Remove one to be prompted again.', + 'settings.agentAccess.alwaysAllowNone': 'No always-allowed tools yet.', 'settings.agentAccess.grantedDesc': 'Folders the agent may read and write, in addition to the workspace. Credential stores (~/.ssh, ~/.gnupg, ~/.aws, keychains) and system directories (/etc, /System, C:\\Windows, …) are always blocked, even inside a granted folder.', 'settings.agentAccess.noneGranted': 'No folders granted.', diff --git a/app/src/lib/i18n/chunks/en-3.ts b/app/src/lib/i18n/chunks/en-3.ts index 96c11c923..c507836dc 100644 --- a/app/src/lib/i18n/chunks/en-3.ts +++ b/app/src/lib/i18n/chunks/en-3.ts @@ -401,6 +401,8 @@ const en3: TranslationMap = { 'channels.telegram.savedRestartRequired': 'Channel saved. Restart the app to activate it.', 'channels.web.alwaysAvailable': 'Always available', 'chat.approval.approve': 'Approve', + 'chat.approval.alwaysAllow': 'Always allow', + 'chat.approval.alwaysAllowHint': 'Stop asking for this tool — add it to your Always-allow list', 'chat.approval.deciding': 'Working…', 'chat.approval.deny': 'Deny', 'chat.approval.error': 'Could not record your decision — try again.', diff --git a/app/src/lib/i18n/chunks/en-5.ts b/app/src/lib/i18n/chunks/en-5.ts index 121f2fe94..bb6ef13f6 100644 --- a/app/src/lib/i18n/chunks/en-5.ts +++ b/app/src/lib/i18n/chunks/en-5.ts @@ -229,6 +229,10 @@ const en5: TranslationMap = { 'settings.agentAccess.confine.desc': 'Restrict the agent to the workspace directory (plus any granted folders), whichever access mode is selected. When off, it can reach anywhere your user can — except the always-blocked credential and system directories.', 'settings.agentAccess.grantedFolders': 'Granted folders', + 'settings.agentAccess.alwaysAllow': 'Always-allowed tools', + 'settings.agentAccess.alwaysAllowDesc': + 'Tools you marked "Always allow" in chat run without asking. Remove one to be prompted again.', + 'settings.agentAccess.alwaysAllowNone': 'No always-allowed tools yet.', 'settings.agentAccess.grantedDesc': 'Folders the agent may read and write, in addition to the workspace. Credential stores (~/.ssh, ~/.gnupg, ~/.aws, keychains) and system directories (/etc, /System, C:\\Windows, …) are always blocked, even inside a granted folder.', 'settings.agentAccess.noneGranted': 'No folders granted.', diff --git a/app/src/lib/i18n/chunks/es-3.ts b/app/src/lib/i18n/chunks/es-3.ts index 3a750b07b..ed4f48afe 100644 --- a/app/src/lib/i18n/chunks/es-3.ts +++ b/app/src/lib/i18n/chunks/es-3.ts @@ -438,6 +438,8 @@ const es3: TranslationMap = { 'channels.yuanbao.savedRestartRequired': 'Channel saved. Restart the app to activate it.', 'channels.yuanbao.unexpectedStatus': 'Unexpected connection status: {status}', 'chat.approval.approve': 'Approve', + 'chat.approval.alwaysAllow': 'Always allow', + 'chat.approval.alwaysAllowHint': 'Stop asking for this tool — add it to your Always-allow list', 'chat.approval.deciding': 'Working…', 'chat.approval.deny': 'Deny', 'chat.approval.error': 'Could not record your decision — try again.', diff --git a/app/src/lib/i18n/chunks/es-5.ts b/app/src/lib/i18n/chunks/es-5.ts index 9451979c0..ccd5c3f92 100644 --- a/app/src/lib/i18n/chunks/es-5.ts +++ b/app/src/lib/i18n/chunks/es-5.ts @@ -756,6 +756,10 @@ const es5: TranslationMap = { 'settings.agentAccess.confine.desc': 'Restrict the agent to the workspace directory (plus any granted folders), whichever access mode is selected. When off, it can reach anywhere your user can — except the always-blocked credential and system directories.', 'settings.agentAccess.grantedFolders': 'Granted folders', + 'settings.agentAccess.alwaysAllow': 'Always-allowed tools', + 'settings.agentAccess.alwaysAllowDesc': + 'Tools you marked "Always allow" in chat run without asking. Remove one to be prompted again.', + 'settings.agentAccess.alwaysAllowNone': 'No always-allowed tools yet.', 'settings.agentAccess.grantedDesc': 'Folders the agent may read and write, in addition to the workspace. Credential stores (~/.ssh, ~/.gnupg, ~/.aws, keychains) and system directories (/etc, /System, C:\\Windows, …) are always blocked, even inside a granted folder.', 'settings.agentAccess.noneGranted': 'No folders granted.', diff --git a/app/src/lib/i18n/chunks/fr-3.ts b/app/src/lib/i18n/chunks/fr-3.ts index bf3e878ce..638502c7a 100644 --- a/app/src/lib/i18n/chunks/fr-3.ts +++ b/app/src/lib/i18n/chunks/fr-3.ts @@ -441,6 +441,8 @@ const fr3: TranslationMap = { 'channels.yuanbao.savedRestartRequired': 'Channel saved. Restart the app to activate it.', 'channels.yuanbao.unexpectedStatus': 'Unexpected connection status: {status}', 'chat.approval.approve': 'Approve', + 'chat.approval.alwaysAllow': 'Always allow', + 'chat.approval.alwaysAllowHint': 'Stop asking for this tool — add it to your Always-allow list', 'chat.approval.deciding': 'Working…', 'chat.approval.deny': 'Deny', 'chat.approval.error': 'Could not record your decision — try again.', diff --git a/app/src/lib/i18n/chunks/fr-5.ts b/app/src/lib/i18n/chunks/fr-5.ts index 8f2def12e..cbe9f508b 100644 --- a/app/src/lib/i18n/chunks/fr-5.ts +++ b/app/src/lib/i18n/chunks/fr-5.ts @@ -760,6 +760,10 @@ const fr5: TranslationMap = { 'settings.agentAccess.confine.desc': 'Restrict the agent to the workspace directory (plus any granted folders), whichever access mode is selected. When off, it can reach anywhere your user can — except the always-blocked credential and system directories.', 'settings.agentAccess.grantedFolders': 'Granted folders', + 'settings.agentAccess.alwaysAllow': 'Always-allowed tools', + 'settings.agentAccess.alwaysAllowDesc': + 'Tools you marked "Always allow" in chat run without asking. Remove one to be prompted again.', + 'settings.agentAccess.alwaysAllowNone': 'No always-allowed tools yet.', 'settings.agentAccess.grantedDesc': 'Folders the agent may read and write, in addition to the workspace. Credential stores (~/.ssh, ~/.gnupg, ~/.aws, keychains) and system directories (/etc, /System, C:\\Windows, …) are always blocked, even inside a granted folder.', 'settings.agentAccess.noneGranted': 'No folders granted.', diff --git a/app/src/lib/i18n/chunks/hi-3.ts b/app/src/lib/i18n/chunks/hi-3.ts index ebeea63f9..21193e4ff 100644 --- a/app/src/lib/i18n/chunks/hi-3.ts +++ b/app/src/lib/i18n/chunks/hi-3.ts @@ -436,6 +436,8 @@ const hi3: TranslationMap = { 'channels.yuanbao.savedRestartRequired': 'Channel saved. Restart the app to activate it.', 'channels.yuanbao.unexpectedStatus': 'Unexpected connection status: {status}', 'chat.approval.approve': 'Approve', + 'chat.approval.alwaysAllow': 'Always allow', + 'chat.approval.alwaysAllowHint': 'Stop asking for this tool — add it to your Always-allow list', 'chat.approval.deciding': 'Working…', 'chat.approval.deny': 'Deny', 'chat.approval.error': 'Could not record your decision — try again.', diff --git a/app/src/lib/i18n/chunks/hi-5.ts b/app/src/lib/i18n/chunks/hi-5.ts index be6233135..1fbcc8e17 100644 --- a/app/src/lib/i18n/chunks/hi-5.ts +++ b/app/src/lib/i18n/chunks/hi-5.ts @@ -743,6 +743,10 @@ const hi5: TranslationMap = { 'settings.agentAccess.confine.desc': 'Restrict the agent to the workspace directory (plus any granted folders), whichever access mode is selected. When off, it can reach anywhere your user can — except the always-blocked credential and system directories.', 'settings.agentAccess.grantedFolders': 'Granted folders', + 'settings.agentAccess.alwaysAllow': 'Always-allowed tools', + 'settings.agentAccess.alwaysAllowDesc': + 'Tools you marked "Always allow" in chat run without asking. Remove one to be prompted again.', + 'settings.agentAccess.alwaysAllowNone': 'No always-allowed tools yet.', 'settings.agentAccess.grantedDesc': 'Folders the agent may read and write, in addition to the workspace. Credential stores (~/.ssh, ~/.gnupg, ~/.aws, keychains) and system directories (/etc, /System, C:\\Windows, …) are always blocked, even inside a granted folder.', 'settings.agentAccess.noneGranted': 'No folders granted.', diff --git a/app/src/lib/i18n/chunks/id-3.ts b/app/src/lib/i18n/chunks/id-3.ts index 8e4dea1eb..ddd54f590 100644 --- a/app/src/lib/i18n/chunks/id-3.ts +++ b/app/src/lib/i18n/chunks/id-3.ts @@ -437,6 +437,8 @@ const id3: TranslationMap = { 'channels.yuanbao.savedRestartRequired': 'Channel saved. Restart the app to activate it.', 'channels.yuanbao.unexpectedStatus': 'Unexpected connection status: {status}', 'chat.approval.approve': 'Approve', + 'chat.approval.alwaysAllow': 'Always allow', + 'chat.approval.alwaysAllowHint': 'Stop asking for this tool — add it to your Always-allow list', 'chat.approval.deciding': 'Working…', 'chat.approval.deny': 'Deny', 'chat.approval.error': 'Could not record your decision — try again.', diff --git a/app/src/lib/i18n/chunks/id-5.ts b/app/src/lib/i18n/chunks/id-5.ts index d67adf455..eb798c8da 100644 --- a/app/src/lib/i18n/chunks/id-5.ts +++ b/app/src/lib/i18n/chunks/id-5.ts @@ -743,6 +743,10 @@ const id5: TranslationMap = { 'settings.agentAccess.confine.desc': 'Restrict the agent to the workspace directory (plus any granted folders), whichever access mode is selected. When off, it can reach anywhere your user can — except the always-blocked credential and system directories.', 'settings.agentAccess.grantedFolders': 'Granted folders', + 'settings.agentAccess.alwaysAllow': 'Always-allowed tools', + 'settings.agentAccess.alwaysAllowDesc': + 'Tools you marked "Always allow" in chat run without asking. Remove one to be prompted again.', + 'settings.agentAccess.alwaysAllowNone': 'No always-allowed tools yet.', 'settings.agentAccess.grantedDesc': 'Folders the agent may read and write, in addition to the workspace. Credential stores (~/.ssh, ~/.gnupg, ~/.aws, keychains) and system directories (/etc, /System, C:\\Windows, …) are always blocked, even inside a granted folder.', 'settings.agentAccess.noneGranted': 'No folders granted.', diff --git a/app/src/lib/i18n/chunks/it-3.ts b/app/src/lib/i18n/chunks/it-3.ts index f4350f260..25929368b 100644 --- a/app/src/lib/i18n/chunks/it-3.ts +++ b/app/src/lib/i18n/chunks/it-3.ts @@ -437,6 +437,8 @@ const it3: TranslationMap = { 'channels.yuanbao.savedRestartRequired': 'Channel saved. Restart the app to activate it.', 'channels.yuanbao.unexpectedStatus': 'Unexpected connection status: {status}', 'chat.approval.approve': 'Approve', + 'chat.approval.alwaysAllow': 'Always allow', + 'chat.approval.alwaysAllowHint': 'Stop asking for this tool — add it to your Always-allow list', 'chat.approval.deciding': 'Working…', 'chat.approval.deny': 'Deny', 'chat.approval.error': 'Could not record your decision — try again.', diff --git a/app/src/lib/i18n/chunks/it-5.ts b/app/src/lib/i18n/chunks/it-5.ts index 602a0efcd..abb59e21c 100644 --- a/app/src/lib/i18n/chunks/it-5.ts +++ b/app/src/lib/i18n/chunks/it-5.ts @@ -754,6 +754,10 @@ const it5: TranslationMap = { 'settings.agentAccess.confine.desc': 'Restrict the agent to the workspace directory (plus any granted folders), whichever access mode is selected. When off, it can reach anywhere your user can — except the always-blocked credential and system directories.', 'settings.agentAccess.grantedFolders': 'Granted folders', + 'settings.agentAccess.alwaysAllow': 'Always-allowed tools', + 'settings.agentAccess.alwaysAllowDesc': + 'Tools you marked "Always allow" in chat run without asking. Remove one to be prompted again.', + 'settings.agentAccess.alwaysAllowNone': 'No always-allowed tools yet.', 'settings.agentAccess.grantedDesc': 'Folders the agent may read and write, in addition to the workspace. Credential stores (~/.ssh, ~/.gnupg, ~/.aws, keychains) and system directories (/etc, /System, C:\\Windows, …) are always blocked, even inside a granted folder.', 'settings.agentAccess.noneGranted': 'No folders granted.', diff --git a/app/src/lib/i18n/chunks/ko-3.ts b/app/src/lib/i18n/chunks/ko-3.ts index 07f42c21b..e5bdca6dd 100644 --- a/app/src/lib/i18n/chunks/ko-3.ts +++ b/app/src/lib/i18n/chunks/ko-3.ts @@ -434,6 +434,8 @@ const ko3: TranslationMap = { 'channels.yuanbao.savedRestartRequired': 'Channel saved. Restart the app to activate it.', 'channels.yuanbao.unexpectedStatus': 'Unexpected connection status: {status}', 'chat.approval.approve': 'Approve', + 'chat.approval.alwaysAllow': 'Always allow', + 'chat.approval.alwaysAllowHint': 'Stop asking for this tool — add it to your Always-allow list', 'chat.approval.deciding': 'Working…', 'chat.approval.deny': 'Deny', 'chat.approval.error': 'Could not record your decision — try again.', diff --git a/app/src/lib/i18n/chunks/ko-5.ts b/app/src/lib/i18n/chunks/ko-5.ts index 762d446dd..89debe5ef 100644 --- a/app/src/lib/i18n/chunks/ko-5.ts +++ b/app/src/lib/i18n/chunks/ko-5.ts @@ -732,6 +732,10 @@ const ko5: TranslationMap = { 'settings.agentAccess.confine.desc': 'Restrict the agent to the workspace directory (plus any granted folders), whichever access mode is selected. When off, it can reach anywhere your user can — except the always-blocked credential and system directories.', 'settings.agentAccess.grantedFolders': 'Granted folders', + 'settings.agentAccess.alwaysAllow': 'Always-allowed tools', + 'settings.agentAccess.alwaysAllowDesc': + 'Tools you marked "Always allow" in chat run without asking. Remove one to be prompted again.', + 'settings.agentAccess.alwaysAllowNone': 'No always-allowed tools yet.', 'settings.agentAccess.grantedDesc': 'Folders the agent may read and write, in addition to the workspace. Credential stores (~/.ssh, ~/.gnupg, ~/.aws, keychains) and system directories (/etc, /System, C:\\Windows, …) are always blocked, even inside a granted folder.', 'settings.agentAccess.noneGranted': 'No folders granted.', diff --git a/app/src/lib/i18n/chunks/pt-3.ts b/app/src/lib/i18n/chunks/pt-3.ts index 536206a10..004434225 100644 --- a/app/src/lib/i18n/chunks/pt-3.ts +++ b/app/src/lib/i18n/chunks/pt-3.ts @@ -437,6 +437,8 @@ const pt3: TranslationMap = { 'channels.yuanbao.savedRestartRequired': 'Channel saved. Restart the app to activate it.', 'channels.yuanbao.unexpectedStatus': 'Unexpected connection status: {status}', 'chat.approval.approve': 'Approve', + 'chat.approval.alwaysAllow': 'Always allow', + 'chat.approval.alwaysAllowHint': 'Stop asking for this tool — add it to your Always-allow list', 'chat.approval.deciding': 'Working…', 'chat.approval.deny': 'Deny', 'chat.approval.error': 'Could not record your decision — try again.', diff --git a/app/src/lib/i18n/chunks/pt-5.ts b/app/src/lib/i18n/chunks/pt-5.ts index d17d44b38..169eed866 100644 --- a/app/src/lib/i18n/chunks/pt-5.ts +++ b/app/src/lib/i18n/chunks/pt-5.ts @@ -753,6 +753,10 @@ const pt5: TranslationMap = { 'settings.agentAccess.confine.desc': 'Restrict the agent to the workspace directory (plus any granted folders), whichever access mode is selected. When off, it can reach anywhere your user can — except the always-blocked credential and system directories.', 'settings.agentAccess.grantedFolders': 'Granted folders', + 'settings.agentAccess.alwaysAllow': 'Always-allowed tools', + 'settings.agentAccess.alwaysAllowDesc': + 'Tools you marked "Always allow" in chat run without asking. Remove one to be prompted again.', + 'settings.agentAccess.alwaysAllowNone': 'No always-allowed tools yet.', 'settings.agentAccess.grantedDesc': 'Folders the agent may read and write, in addition to the workspace. Credential stores (~/.ssh, ~/.gnupg, ~/.aws, keychains) and system directories (/etc, /System, C:\\Windows, …) are always blocked, even inside a granted folder.', 'settings.agentAccess.noneGranted': 'No folders granted.', diff --git a/app/src/lib/i18n/chunks/ru-3.ts b/app/src/lib/i18n/chunks/ru-3.ts index 962b9295a..a3b15a76e 100644 --- a/app/src/lib/i18n/chunks/ru-3.ts +++ b/app/src/lib/i18n/chunks/ru-3.ts @@ -436,6 +436,8 @@ const ru3: TranslationMap = { 'channels.yuanbao.savedRestartRequired': 'Channel saved. Restart the app to activate it.', 'channels.yuanbao.unexpectedStatus': 'Unexpected connection status: {status}', 'chat.approval.approve': 'Approve', + 'chat.approval.alwaysAllow': 'Always allow', + 'chat.approval.alwaysAllowHint': 'Stop asking for this tool — add it to your Always-allow list', 'chat.approval.deciding': 'Working…', 'chat.approval.deny': 'Deny', 'chat.approval.error': 'Could not record your decision — try again.', diff --git a/app/src/lib/i18n/chunks/ru-5.ts b/app/src/lib/i18n/chunks/ru-5.ts index ffbe58f5e..a67530fe0 100644 --- a/app/src/lib/i18n/chunks/ru-5.ts +++ b/app/src/lib/i18n/chunks/ru-5.ts @@ -749,6 +749,10 @@ const ru5: TranslationMap = { 'settings.agentAccess.confine.desc': 'Restrict the agent to the workspace directory (plus any granted folders), whichever access mode is selected. When off, it can reach anywhere your user can — except the always-blocked credential and system directories.', 'settings.agentAccess.grantedFolders': 'Granted folders', + 'settings.agentAccess.alwaysAllow': 'Always-allowed tools', + 'settings.agentAccess.alwaysAllowDesc': + 'Tools you marked "Always allow" in chat run without asking. Remove one to be prompted again.', + 'settings.agentAccess.alwaysAllowNone': 'No always-allowed tools yet.', 'settings.agentAccess.grantedDesc': 'Folders the agent may read and write, in addition to the workspace. Credential stores (~/.ssh, ~/.gnupg, ~/.aws, keychains) and system directories (/etc, /System, C:\\Windows, …) are always blocked, even inside a granted folder.', 'settings.agentAccess.noneGranted': 'No folders granted.', diff --git a/app/src/lib/i18n/chunks/zh-CN-3.ts b/app/src/lib/i18n/chunks/zh-CN-3.ts index 4d5f203c9..27dde20db 100644 --- a/app/src/lib/i18n/chunks/zh-CN-3.ts +++ b/app/src/lib/i18n/chunks/zh-CN-3.ts @@ -425,6 +425,8 @@ const zhCN3: TranslationMap = { 'channels.yuanbao.savedRestartRequired': 'Channel saved. Restart the app to activate it.', 'channels.yuanbao.unexpectedStatus': '意外的连接状态:{status}', 'chat.approval.approve': 'Approve', + 'chat.approval.alwaysAllow': 'Always allow', + 'chat.approval.alwaysAllowHint': 'Stop asking for this tool — add it to your Always-allow list', 'chat.approval.deciding': 'Working…', 'chat.approval.deny': 'Deny', 'chat.approval.error': 'Could not record your decision — try again.', diff --git a/app/src/lib/i18n/chunks/zh-CN-5.ts b/app/src/lib/i18n/chunks/zh-CN-5.ts index ce194b346..5950b8f7d 100644 --- a/app/src/lib/i18n/chunks/zh-CN-5.ts +++ b/app/src/lib/i18n/chunks/zh-CN-5.ts @@ -703,6 +703,10 @@ const zhCN5: TranslationMap = { 'settings.agentAccess.confine.desc': 'Restrict the agent to the workspace directory (plus any granted folders), whichever access mode is selected. When off, it can reach anywhere your user can — except the always-blocked credential and system directories.', 'settings.agentAccess.grantedFolders': 'Granted folders', + 'settings.agentAccess.alwaysAllow': 'Always-allowed tools', + 'settings.agentAccess.alwaysAllowDesc': + 'Tools you marked "Always allow" in chat run without asking. Remove one to be prompted again.', + 'settings.agentAccess.alwaysAllowNone': 'No always-allowed tools yet.', 'settings.agentAccess.grantedDesc': 'Folders the agent may read and write, in addition to the workspace. Credential stores (~/.ssh, ~/.gnupg, ~/.aws, keychains) and system directories (/etc, /System, C:\\Windows, …) are always blocked, even inside a granted folder.', 'settings.agentAccess.noneGranted': 'No folders granted.', diff --git a/app/src/lib/i18n/en.ts b/app/src/lib/i18n/en.ts index c1da2d797..bcff636ab 100644 --- a/app/src/lib/i18n/en.ts +++ b/app/src/lib/i18n/en.ts @@ -2022,6 +2022,8 @@ const en: TranslationMap = { 'channels.telegram.savedRestartRequired': 'Channel saved. Restart the app to activate it.', 'channels.web.alwaysAvailable': 'Always available', 'chat.approval.approve': 'Approve', + 'chat.approval.alwaysAllow': 'Always allow', + 'chat.approval.alwaysAllowHint': 'Stop asking for this tool — add it to your Always-allow list', 'chat.approval.deciding': 'Working…', 'chat.approval.deny': 'Deny', 'chat.approval.error': 'Could not record your decision — try again.', @@ -3055,6 +3057,10 @@ const en: TranslationMap = { 'settings.agentAccess.confine.desc': 'Restrict the agent to the workspace directory (plus any granted folders), whichever access mode is selected. When off, it can reach anywhere your user can — except the always-blocked credential and system directories.', 'settings.agentAccess.grantedFolders': 'Granted folders', + 'settings.agentAccess.alwaysAllow': 'Always-allowed tools', + 'settings.agentAccess.alwaysAllowDesc': + 'Tools you marked "Always allow" in chat run without asking. Remove one to be prompted again.', + 'settings.agentAccess.alwaysAllowNone': 'No always-allowed tools yet.', 'settings.agentAccess.grantedDesc': 'Folders the agent may read and write, in addition to the workspace. Credential stores (~/.ssh, ~/.gnupg, ~/.aws, keychains) and system directories (/etc, /System, C:\\Windows, …) are always blocked, even inside a granted folder.', 'settings.agentAccess.noneGranted': 'No folders granted.', diff --git a/app/src/utils/tauriCommands/config.ts b/app/src/utils/tauriCommands/config.ts index 34a5dea77..9cc2b254c 100644 --- a/app/src/utils/tauriCommands/config.ts +++ b/app/src/utils/tauriCommands/config.ts @@ -316,6 +316,8 @@ export interface AutonomySettings { trusted_roots: TrustedRoot[]; allow_tool_install: boolean; max_actions_per_hour: number; + /** "Always allow" allowlist — tool names the agent runs without a prompt. */ + auto_approve: string[]; } /** Partial update — omitted fields are left unchanged. */ @@ -327,6 +329,8 @@ export interface AutonomySettingsUpdate { trusted_roots?: TrustedRoot[]; allow_tool_install?: boolean; max_actions_per_hour?: number; + /** Replaces the "Always allow" allowlist wholesale. */ + auto_approve?: string[]; } export async function openhumanGetAutonomySettings(): Promise> { diff --git a/src/core/jsonrpc.rs b/src/core/jsonrpc.rs index 3c38a03ff..8393a2a75 100644 --- a/src/core/jsonrpc.rs +++ b/src/core/jsonrpc.rs @@ -1824,6 +1824,24 @@ pub async fn bootstrap_core_runtime(embedded_core: bool) { ); } + // --- Live SecurityPolicy --- + // Install the process-global live policy on the always-run serve boot, not + // only inside `start_channels` (which is skipped for web-chat-only cores + // with no messaging integrations). Without this, `live_policy::current()` + // would be empty on those cores, so the ApprovalGate's `auto_approve` + // allowlist and `config.update_autonomy_settings` reloads (`reload_from`) + // would be inert until a session with integrations starts. `from_config` + // injects the default projects root, so this matches what `start_channels` + // installs; idempotent — a later `start_channels` re-installs an equivalent + // policy. + crate::openhuman::security::live_policy::install( + std::sync::Arc::new(crate::openhuman::security::SecurityPolicy::from_config( + &cfg.autonomy, + &workspace_dir, + )), + workspace_dir.clone(), + ); + // --- Approval gate (#1339) --- // ON by default; opt out with `OPENHUMAN_APPROVAL_GATE=0` (or `false`). // Prompt-class `external_effect()` tool calls route through diff --git a/src/openhuman/about_app/catalog.rs b/src/openhuman/about_app/catalog.rs index 0d4986b0e..0a6b22ead 100644 --- a/src/openhuman/about_app/catalog.rs +++ b/src/openhuman/about_app/catalog.rs @@ -1334,6 +1334,19 @@ const CAPABILITIES: &[Capability] = &[ status: CapabilityStatus::Stable, privacy: None, }, + Capability { + id: "security.always_allow_tool", + name: "Always Allow a Tool", + domain: "security", + category: CapabilityCategory::Settings, + description: "On an approval prompt, choose \"Always allow\" to stop being asked for that \ + tool. The choice is saved to your allow-list and persists across restarts; \ + remove it any time under Settings → Agent OS access to be prompted again. \ + Policy still blocks forbidden paths and high-risk commands regardless.", + how_to: "Click \"Always allow\" on an approval prompt; manage the list in Settings → Agent OS access.", + status: CapabilityStatus::Stable, + privacy: None, + }, Capability { id: "tool.detect_tools", name: "Detect Installed Tools", diff --git a/src/openhuman/agent/bus.rs b/src/openhuman/agent/bus.rs index 70fa11e20..5bd8e95ae 100644 --- a/src/openhuman/agent/bus.rs +++ b/src/openhuman/agent/bus.rs @@ -243,11 +243,6 @@ pub fn register_agent_handlers() { &model, temperature, silent, - // Approval is not wired into the channel path today; if - // CLI migrates to the bus later, extend AgentTurnRequest - // with `approval: Option>` and pass - // it through here. - None, &channel_name, &multimodal, max_tool_iterations, diff --git a/src/openhuman/agent/harness/bughunt_tests.rs b/src/openhuman/agent/harness/bughunt_tests.rs index 725596691..dae9204d5 100644 --- a/src/openhuman/agent/harness/bughunt_tests.rs +++ b/src/openhuman/agent/harness/bughunt_tests.rs @@ -92,7 +92,6 @@ async fn native_tool_call_decodes_json_encoded_arguments_string() { "m", 0.0, true, - None, "channel", &mm(), 3, @@ -154,7 +153,6 @@ async fn documents_silent_drop_of_non_json_arguments_string() { "m", 0.0, true, - None, "channel", &mm(), 3, @@ -211,7 +209,6 @@ async fn parallel_tool_calls_in_single_iteration_all_execute() { "m", 0.0, true, - None, "channel", &mm(), 5, @@ -254,7 +251,6 @@ async fn same_named_tool_in_registry_first_match_wins() { "m", 0.0, true, - None, "channel", &mm(), 5, @@ -307,7 +303,6 @@ async fn markdown_fenced_tool_call_block_is_parsed() { "m", 0.0, true, - None, "channel", &mm(), 5, @@ -361,7 +356,6 @@ async fn native_tool_calls_take_precedence_over_xml_in_text() { "m", 0.0, true, - None, "channel", &mm(), 5, @@ -421,7 +415,6 @@ async fn per_tool_max_result_size_caps_history_payload() { "m", 0.0, true, - None, "channel", &mm(), 5, @@ -473,7 +466,6 @@ async fn empty_response_with_no_tool_calls_terminates_with_empty_text() { "m", 0.0, true, - None, "channel", &mm(), 5, @@ -517,7 +509,6 @@ async fn progress_sink_emits_lifecycle_events_in_order() { "m", 0.0, true, - None, "channel", &mm(), 5, diff --git a/src/openhuman/agent/harness/harness_gap_tests.rs b/src/openhuman/agent/harness/harness_gap_tests.rs index f93b00f54..c6277b30f 100644 --- a/src/openhuman/agent/harness/harness_gap_tests.rs +++ b/src/openhuman/agent/harness/harness_gap_tests.rs @@ -142,7 +142,6 @@ async fn full_turn_cycle_user_llm_tool_result_final() { "model", 0.0, true, - None, "channel", &multimodal_cfg(), 2, @@ -202,7 +201,6 @@ async fn max_iterations_exceeded_downcasts_to_typed_agent_error() { "model", 0.0, true, - None, "channel", &multimodal_cfg(), 1, @@ -278,7 +276,6 @@ async fn visible_tool_names_rejects_tool_outside_whitelist() { "model", 0.0, true, - None, "channel", &multimodal_cfg(), 2, @@ -336,7 +333,6 @@ async fn visible_tool_names_allows_tool_inside_whitelist() { "model", 0.0, true, - None, "channel", &multimodal_cfg(), 2, diff --git a/src/openhuman/agent/harness/test_support_test.rs b/src/openhuman/agent/harness/test_support_test.rs index 118343eb0..556f31e46 100644 --- a/src/openhuman/agent/harness/test_support_test.rs +++ b/src/openhuman/agent/harness/test_support_test.rs @@ -393,7 +393,6 @@ async fn keyword_provider_drives_prompt_guided_tool_loop_to_completion() { "test-model", 0.0, true, - None, "channel", &mm(), 5, @@ -443,7 +442,6 @@ async fn keyword_provider_drives_native_tool_calls_path() { "test-model", 0.0, true, - None, "channel", &mm(), 5, @@ -499,7 +497,6 @@ async fn keyword_provider_chains_multiple_tools_across_iterations() { "test-model", 0.0, true, - None, "channel", &mm(), 10, @@ -617,7 +614,6 @@ async fn crypto_wallet_send_flow_sequences_wallet_tools_and_confirmation_gate() "test-model", 0.0, true, - None, "web", &mm(), 10, @@ -730,7 +726,6 @@ async fn crypto_wallet_send_flow_does_not_execute_when_confirmation_is_not_grant "test-model", 0.0, true, - None, "telegram", &mm(), 8, @@ -791,7 +786,6 @@ async fn keyword_provider_uses_latest_tool_result_to_drive_the_next_tool_call() "test-model", 0.0, true, - None, "channel", &mm(), 10, @@ -865,7 +859,6 @@ async fn keyword_provider_executes_multiple_native_tool_calls_from_one_turn() { "test-model", 0.0, true, - None, "channel", &mm(), 10, @@ -914,7 +907,6 @@ async fn keyword_provider_unknown_tool_surfaces_error_and_loop_continues() { "test-model", 0.0, true, - None, "channel", &mm(), 5, @@ -964,7 +956,6 @@ async fn run_tool_call_loop_returns_max_iterations_error() { "test-model", 0.0, true, - None, "channel", &mm(), 3, @@ -1034,7 +1025,6 @@ async fn agent_loop_refuses_clirpconly_tools() { "test-model", 0.0, true, - None, "channel", &mm(), 5, @@ -1094,7 +1084,6 @@ async fn tool_error_result_is_surfaced_to_next_iteration() { "test-model", 0.0, true, - None, "channel", &mm(), 5, @@ -1150,7 +1139,6 @@ async fn tool_anyhow_error_surfaces_in_history() { "test-model", 0.0, true, - None, "channel", &mm(), 5, @@ -1195,7 +1183,6 @@ async fn visible_tool_names_whitelist_rejects_filtered_out_tools() { "test-model", 0.0, true, - None, "channel", &mm(), 5, @@ -1241,7 +1228,6 @@ async fn extra_tools_are_invokable_alongside_registry() { "test-model", 0.0, true, - None, "channel", &mm(), 5, @@ -1396,7 +1382,6 @@ async fn harness_invokes_composio_action_tool_against_fake_backend() { "test-model", 0.0, true, - None, "channel", &mm(), 5, @@ -1543,7 +1528,6 @@ impl Tool for TestDelegationTool { "test-model", 0.0, true, - None, "channel", &mm(), 5, @@ -1686,7 +1670,6 @@ async fn orchestrator_prompt_drives_composio_call_via_delegation_chain() { "test-model", 0.0, true, - None, "channel", &mm(), 5, diff --git a/src/openhuman/agent/harness/tests.rs b/src/openhuman/agent/harness/tests.rs index 1be2539f9..f26ae5b2d 100644 --- a/src/openhuman/agent/harness/tests.rs +++ b/src/openhuman/agent/harness/tests.rs @@ -119,7 +119,6 @@ async fn run_tool_call_loop_returns_structured_error_for_non_vision_provider() { "mock-model", 0.0, true, - None, "cli", &crate::openhuman::config::MultimodalConfig::default(), 3, @@ -165,7 +164,6 @@ async fn run_tool_call_loop_rejects_oversized_image_payload() { "mock-model", 0.0, true, - None, "cli", &multimodal, 3, @@ -205,7 +203,6 @@ async fn run_tool_call_loop_accepts_valid_multimodal_request_flow() { "mock-model", 0.0, true, - None, "cli", &crate::openhuman::config::MultimodalConfig::default(), 3, diff --git a/src/openhuman/agent/harness/tool_loop.rs b/src/openhuman/agent/harness/tool_loop.rs index 24512c1aa..7f1100295 100644 --- a/src/openhuman/agent/harness/tool_loop.rs +++ b/src/openhuman/agent/harness/tool_loop.rs @@ -2,7 +2,6 @@ use crate::openhuman::agent::cost::TurnCost; use crate::openhuman::agent::multimodal; use crate::openhuman::agent::progress::AgentProgress; use crate::openhuman::agent::stop_hooks::{current_stop_hooks, StopDecision, TurnState}; -use crate::openhuman::approval::{ApprovalManager, ApprovalRequest, ApprovalResponse}; use crate::openhuman::inference::provider::{ ChatMessage, ChatRequest, Provider, ProviderCapabilityError, ProviderDelta, }; @@ -203,7 +202,6 @@ pub(crate) async fn agent_turn( model, temperature, silent, - None, "channel", multimodal_config, max_tool_iterations, @@ -255,8 +253,10 @@ pub(crate) async fn run_tool_call_loop( model: &str, temperature: f64, silent: bool, - approval: Option<&ApprovalManager>, - channel_name: &str, + // Retained in the harness signature (callers pass their channel) but no + // longer consumed here since the legacy CLI approval prompt was removed — + // approval now flows through the process-global `ApprovalGate`. + _channel_name: &str, multimodal_config: &crate::openhuman::config::MultimodalConfig, max_tool_iterations: usize, on_delta: Option>, @@ -792,45 +792,6 @@ pub(crate) async fn run_tool_call_loop( continue; } - // ── Approval hook ──────────────────────────────── - if let Some(mgr) = approval { - if mgr.needs_approval(&call.name) { - let request = ApprovalRequest { - tool_name: call.name.clone(), - arguments: call.arguments.clone(), - }; - - // Only prompt interactively when approvals are supported; auto-approve on other channels. - let decision = if channel_name == "cli" { - mgr.prompt_cli(&request) - } else { - ApprovalResponse::Yes - }; - - mgr.record_decision(&call.name, &call.arguments, decision, channel_name); - - if decision == ApprovalResponse::No { - let denied = "Denied by user.".to_string(); - emit_failed_completion(&denied).await; - individual_results.push(denied.clone()); - let _ = writeln!( - tool_results, - "\n{denied}\n", - call.name - ); - if let Some(halt) = failure_guard.record( - &call.name, - &call.arguments.to_string(), - false, - &denied, - ) { - halt_reason = Some(halt); - } - continue; - } - } - } - // Look up the tool by name in the combined registry + extras, // subject to the visibility whitelist. If the model hallucinated // a filtered-out tool name we treat it as unknown — the error diff --git a/src/openhuman/agent/harness/tool_loop_tests.rs b/src/openhuman/agent/harness/tool_loop_tests.rs index d2f8151fb..9cb1e9852 100644 --- a/src/openhuman/agent/harness/tool_loop_tests.rs +++ b/src/openhuman/agent/harness/tool_loop_tests.rs @@ -1,9 +1,6 @@ use super::*; -use crate::openhuman::approval::ApprovalManager; -use crate::openhuman::config::AutonomyConfig; use crate::openhuman::inference::provider::traits::ProviderCapabilities; use crate::openhuman::inference::provider::ChatResponse; -use crate::openhuman::security::AutonomyLevel; use crate::openhuman::tools::{ToolResult, ToolScope}; use async_trait::async_trait; use parking_lot::Mutex; @@ -217,7 +214,6 @@ async fn run_tool_call_loop_intercepts_oversized_tool_results_via_summarizer() { "model", 0.0, true, - None, "channel", &crate::openhuman::config::MultimodalConfig::default(), 2, @@ -269,7 +265,6 @@ async fn run_tool_call_loop_rejects_vision_markers_for_non_vision_provider() { "model", 0.0, true, - None, "channel", &crate::openhuman::config::MultimodalConfig::default(), 1, @@ -308,7 +303,6 @@ async fn run_tool_call_loop_streams_final_text_chunks() { "model", 0.0, true, - None, "channel", &crate::openhuman::config::MultimodalConfig::default(), 1, @@ -362,7 +356,6 @@ async fn run_tool_call_loop_blocks_cli_rpc_only_tools_in_prompt_mode() { "model", 0.0, true, - None, "channel", &crate::openhuman::config::MultimodalConfig::default(), 2, @@ -419,7 +412,6 @@ async fn run_tool_call_loop_persists_native_tool_results_as_tool_messages() { "model", 0.0, true, - None, "channel", &crate::openhuman::config::MultimodalConfig::default(), 2, @@ -442,64 +434,6 @@ async fn run_tool_call_loop_persists_native_tool_results_as_tool_messages() { assert!(tool_msg.content.contains("echo-out")); } -#[tokio::test] -async fn run_tool_call_loop_auto_approves_supervised_tools_on_non_cli_channels() { - let provider = ScriptedProvider { - responses: Mutex::new(vec![ - Ok(ChatResponse { - text: Some("{\"name\":\"echo\",\"arguments\":{}}".into()), - tool_calls: vec![], - usage: None, - }), - Ok(ChatResponse { - text: Some("done".into()), - tool_calls: vec![], - usage: None, - }), - ]), - native_tools: false, - vision: false, - }; - let mut history = vec![ChatMessage::user("hello")]; - let tools: Vec> = vec![Box::new(EchoTool)]; - let approval = ApprovalManager::from_config(&AutonomyConfig { - level: AutonomyLevel::Supervised, - auto_approve: vec![], - always_ask: vec!["echo".into()], - ..AutonomyConfig::default() - }); - - let result = run_tool_call_loop( - &provider, - &mut history, - &tools, - "test-provider", - "model", - 0.0, - true, - Some(&approval), - "telegram", - &crate::openhuman::config::MultimodalConfig::default(), - 2, - None, - None, - &[], - None, - None, - &crate::openhuman::tools::policy::DefaultToolPolicy, - ) - .await - .expect("non-cli channels should auto-approve supervised tools"); - - assert_eq!(result, "done"); - let tool_results = history - .iter() - .find(|msg| msg.role == "user" && msg.content.contains("[Tool results]")) - .expect("tool results should be appended"); - assert!(tool_results.content.contains("echo-out")); - assert_eq!(approval.audit_log().len(), 1); -} - #[tokio::test] async fn run_tool_call_loop_reports_unknown_tool_and_uses_default_max_iterations() { let provider = ScriptedProvider { @@ -528,7 +462,6 @@ async fn run_tool_call_loop_reports_unknown_tool_and_uses_default_max_iterations "model", 0.0, true, - None, "channel", &crate::openhuman::config::MultimodalConfig::default(), 0, @@ -585,7 +518,6 @@ async fn run_tool_call_loop_formats_tool_error_paths() { "model", 0.0, true, - None, "channel", &crate::openhuman::config::MultimodalConfig::default(), 2, @@ -626,7 +558,6 @@ async fn run_tool_call_loop_propagates_provider_errors_and_max_iteration_failure "model", 0.0, true, - None, "channel", &crate::openhuman::config::MultimodalConfig::default(), 1, @@ -660,7 +591,6 @@ async fn run_tool_call_loop_propagates_provider_errors_and_max_iteration_failure "model", 0.0, true, - None, "channel", &crate::openhuman::config::MultimodalConfig::default(), 1, @@ -737,7 +667,6 @@ async fn run_tool_call_loop_aborts_when_stop_hook_returns_stop() { "model", 0.0, true, - None, "channel", &crate::openhuman::config::MultimodalConfig::default(), 10, @@ -790,7 +719,6 @@ async fn run_tool_call_loop_runs_unchanged_when_no_stop_hooks_installed() { "model", 0.0, true, - None, "channel", &crate::openhuman::config::MultimodalConfig::default(), 1, @@ -866,7 +794,6 @@ async fn run_tool_call_loop_applies_per_tool_max_result_size_cap() { "model", 0.0, true, - None, "channel", &crate::openhuman::config::MultimodalConfig::default(), 2, @@ -933,7 +860,6 @@ async fn run_tool_call_loop_halts_on_repeated_identical_failure() { "model", 0.0, true, - None, "channel", &crate::openhuman::config::MultimodalConfig::default(), 10, // max_iterations — must NOT be reached; breaker fires at 3 @@ -997,7 +923,6 @@ async fn run_tool_call_loop_halts_when_no_progress() { "model", 0.0, true, - None, "channel", &crate::openhuman::config::MultimodalConfig::default(), 20, @@ -1228,7 +1153,6 @@ async fn run_tool_call_loop_dedups_duplicate_tool_names_before_provider_call() { "model", 0.0, true, - None, "channel", &crate::openhuman::config::MultimodalConfig::default(), 2, @@ -1260,3 +1184,126 @@ async fn run_tool_call_loop_dedups_duplicate_tool_names_before_provider_call() { names ); } + +// ── End-to-end: agent loop → ApprovalGate → auto_approve short-circuit ── +// +// Exercises the real seam: a scripted LLM emits a tool call for an +// external-effect tool, the loop routes it through the process-global +// `ApprovalGate` (`try_global`), and the tool's presence on the +// `auto_approve` "Always allow" list short-circuits the gate to `Allow` +// *before* parking — so the tool executes without a prompt, even though a +// chat context is present (which would otherwise park it). + +/// A tool with an external side effect, so the loop gates it via the +/// `ApprovalGate`. Records whether `execute` actually ran. +struct ExternalEffectTool { + ran: std::sync::Arc, +} + +#[async_trait] +impl Tool for ExternalEffectTool { + fn name(&self) -> &str { + "ext_effect_e2e_tool" + } + fn description(&self) -> &str { + "external effect (e2e gate test)" + } + fn parameters_schema(&self) -> serde_json::Value { + serde_json::json!({"type":"object"}) + } + fn external_effect_with_args(&self, _args: &serde_json::Value) -> bool { + true + } + async fn execute(&self, _args: serde_json::Value) -> Result { + self.ran.store(true, std::sync::atomic::Ordering::SeqCst); + Ok(ToolResult::success("did-external-effect")) + } +} + +#[tokio::test] +async fn auto_approved_external_effect_tool_runs_through_loop_without_parking() { + use std::sync::atomic::Ordering; + use std::sync::Arc; + // Serialize live-policy / gate global access against the other tests that + // install or reload them (gate auto_approve test, live_policy test, autonomy + // ops tests) — all take this same lock. + let _env = crate::openhuman::config::TEST_ENV_LOCK + .lock() + .unwrap_or_else(|e| e.into_inner()); + + let tool_name = "ext_effect_e2e_tool"; + + // Always-allow the tool via the live policy the gate reads. + let policy = crate::openhuman::security::SecurityPolicy { + auto_approve: vec![tool_name.into()], + ..crate::openhuman::security::SecurityPolicy::default() + }; + crate::openhuman::security::live_policy::install(Arc::new(policy), std::env::temp_dir()); + + // Install the process-global gate so the loop's external-effect branch has a + // gate to route through (idempotent; the loop calls `ApprovalGate::try_global`). + let cfg = crate::openhuman::config::Config { + workspace_dir: std::env::temp_dir(), + ..crate::openhuman::config::Config::default() + }; + crate::openhuman::approval::ApprovalGate::init_global(cfg, "loop-gate-e2e-session"); + + let ran = Arc::new(std::sync::atomic::AtomicBool::new(false)); + let provider = ScriptedProvider { + responses: Mutex::new(vec![ + Ok(ChatResponse { + text: Some(format!( + "{{\"name\":\"{tool_name}\",\"arguments\":{{}}}}" + )), + tool_calls: vec![], + usage: None, + }), + Ok(ChatResponse { + text: Some("done".into()), + tool_calls: vec![], + usage: None, + }), + ]), + native_tools: false, + vision: false, + }; + let mut history = vec![ChatMessage::user("please act")]; + let tools: Vec> = vec![Box::new(ExternalEffectTool { ran: ran.clone() })]; + + // Run *inside* a chat context: without the allowlist the gate would park + // this external-effect call — so a clean completion proves the auto_approve + // shortcut (checked before chat-context parking) is what let it through. + let result = crate::openhuman::approval::APPROVAL_CHAT_CONTEXT + .scope( + crate::openhuman::approval::ApprovalChatContext { + thread_id: "t-e2e".into(), + client_id: "c-e2e".into(), + }, + run_tool_call_loop( + &provider, + &mut history, + &tools, + "test-provider", + "model", + 0.0, + true, + "channel", + &crate::openhuman::config::MultimodalConfig::default(), + 2, + None, + None, + &[], + None, + None, + &crate::openhuman::tools::policy::DefaultToolPolicy, + ), + ) + .await + .expect("loop should complete without parking on an auto-approved tool"); + + assert_eq!(result, "done"); + assert!( + ran.load(Ordering::SeqCst), + "auto-approved external-effect tool must execute (gate must not park it)" + ); +} diff --git a/src/openhuman/approval/gate.rs b/src/openhuman/approval/gate.rs index ca6c1a61e..e246490a4 100644 --- a/src/openhuman/approval/gate.rs +++ b/src/openhuman/approval/gate.rs @@ -4,8 +4,11 @@ //! Flow (issue #1339): //! 1. Agent harness calls [`ApprovalGate::intercept`] with the tool //! name, a redacted JSON of the arguments, and a short summary. -//! 2. Gate checks the session-scoped allowlist (built from prior -//! `ApproveAlwaysForTool` decisions). Hit → `Allow` immediately. +//! 2. Gate checks the user's "Always allow" allowlist +//! (`autonomy.auto_approve`, read live via +//! [`crate::openhuman::security::live_policy`]). Hit → `Allow` +//! immediately. An `ApproveAlwaysForTool` decision adds the tool to +//! that list via `approval_decide` (config save + policy reload). //! 3. Otherwise: persist a row in `pending_approvals`, publish a //! [`DomainEvent::ApprovalRequested`] event so the UI can pop a //! toast, and park the call on a `oneshot::Sender` keyed by @@ -24,7 +27,7 @@ //! across processes — no side effect can fire across launches, so //! the security invariant is preserved without auto-purging. -use std::collections::{HashMap, HashSet}; +use std::collections::HashMap; use std::sync::{Arc, OnceLock}; use std::time::Duration; @@ -82,7 +85,6 @@ pub struct ApprovalGate { session_id: String, ttl: Duration, waiters: Mutex>>, - always_allowlist: Mutex>, /// thread_id → request_id for the approval currently parked on that chat /// thread, so the web channel can route a yes/no reply to `approval_decide`. /// In-memory only (session-scoped — a parked approval doesn't survive a @@ -123,11 +125,24 @@ impl ApprovalGate { session_id, ttl, waiters: Mutex::new(HashMap::new()), - always_allowlist: Mutex::new(HashSet::new()), thread_to_request: Mutex::new(HashMap::new()), } } + /// Whether `tool_name` is on the user's "Always allow" list. Prefers the + /// process-global live policy (so a grant made this session is seen + /// immediately) and falls back to the gate's boot-time config snapshot. + fn tool_is_auto_approved(&self, tool_name: &str) -> bool { + if let Some(policy) = crate::openhuman::security::live_policy::current() { + return policy.auto_approve.iter().any(|t| t == tool_name); + } + self.config + .autonomy + .auto_approve + .iter() + .any(|t| t == tool_name) + } + /// Intercept a tool call. Blocks until the user decides or the /// TTL elapses (timeout → `Deny`). /// @@ -167,17 +182,18 @@ impl ApprovalGate { action_summary: &str, args_redacted: serde_json::Value, ) -> (GateOutcome, Option) { - // Session-scoped allowlist shortcut — set by prior - // ApproveAlwaysForTool decisions in this launch. - { - let list = self.always_allowlist.lock(); - if list.contains(tool_name) { - tracing::debug!( - tool = tool_name, - "[approval::gate] session-allowlist hit, skipping prompt" - ); - return (GateOutcome::Allow, None); - } + // "Always allow" allowlist shortcut — the user's persisted + // `autonomy.auto_approve` set. Read from the live policy first so a + // grant made earlier in this session (which writes config + reloads the + // live policy) takes effect on the very next tool call; fall back to the + // gate's boot-time config when no live policy is installed (e.g. a CLI + // invocation that never started a session runtime, or a unit test). + if self.tool_is_auto_approved(tool_name) { + tracing::debug!( + tool = tool_name, + "[approval::gate] auto_approve allowlist hit, skipping prompt" + ); + return (GateOutcome::Allow, None); } // Chat context (thread/client id) for routing the yes/no reply — set by @@ -414,10 +430,10 @@ impl ApprovalGate { ) -> anyhow::Result> { let decided = store::decide(&self.config, request_id, decision)?; if let Some(row) = &decided { - if decision == ApprovalDecision::ApproveAlwaysForTool { - let mut list = self.always_allowlist.lock(); - list.insert(row.tool_name.clone()); - } + // `ApproveAlwaysForTool` persistence (append to `autonomy.auto_approve` + // + reload the live policy) is handled by the `approval_decide` RPC + // handler, which is async and owns the config save+reload path. The + // gate only resolves the parked future and emits the audit event. if let Some(tx) = self.take_waiter(request_id) { let _ = tx.send(decision); } @@ -573,34 +589,43 @@ mod tests { } #[tokio::test] - async fn approve_always_for_tool_short_circuits_future_calls() { - let (gate, _dir) = test_gate(); - let gate = Arc::new(gate); - - let g = gate.clone(); - let first = tokio::spawn(async move { - APPROVAL_CHAT_CONTEXT - .scope( - chat_ctx(), - g.intercept("composio", "first", serde_json::json!({})), - ) - .await - }); - let pending = loop { - if let Some(p) = gate.list_pending().unwrap().into_iter().next() { - break p; - } - tokio::time::sleep(Duration::from_millis(10)).await; + async fn auto_approve_tool_skips_prompt() { + // The gate reads the "Always allow" allowlist from the process-global + // live policy. Serialize with the other tests that install/reload it + // (the `live_policy` module test + the autonomy `ops` tests, which all + // take this same lock) so a parallel install can't clobber ours mid-test. + let _env = crate::openhuman::config::TEST_ENV_LOCK + .lock() + .unwrap_or_else(|e| e.into_inner()); + let (gate, dir) = test_gate(); + + // A tool name unique to this test so leaving it in the global allowlist + // afterwards can't make a sibling gate test (which use "composio" / + // "pushover") skip its expected prompt. + let tool = "openhuman_test_always_allow_tool"; + let policy = crate::openhuman::security::SecurityPolicy { + auto_approve: vec![tool.into()], + ..crate::openhuman::security::SecurityPolicy::default() }; - gate.decide(&pending.request_id, ApprovalDecision::ApproveAlwaysForTool) - .unwrap(); - assert!(matches!(first.await.unwrap(), GateOutcome::Allow)); + crate::openhuman::security::live_policy::install( + Arc::new(policy), + dir.path().to_path_buf(), + ); - // Second call to the same tool must NOT block — allowlist hit. - let outcome = gate - .intercept("composio", "second", serde_json::json!({})) + // An allow-listed tool short-circuits the gate to `Allow` immediately — + // before any parking — even with a live chat context present, and + // without persisting a pending row. + let outcome = APPROVAL_CHAT_CONTEXT + .scope( + chat_ctx(), + gate.intercept(tool, "noop", serde_json::json!({})), + ) .await; assert!(matches!(outcome, GateOutcome::Allow)); + assert!( + gate.list_pending().unwrap().is_empty(), + "an auto-approved call must not create a pending approval row" + ); } #[tokio::test] @@ -748,7 +773,7 @@ mod tests { } #[tokio::test] - async fn intercept_audited_returns_none_id_for_denied_and_allowlisted() { + async fn intercept_audited_id_is_none_for_denied_some_for_approved() { let (gate, _dir) = test_gate(); let gate = Arc::new(gate); @@ -795,6 +820,11 @@ mod tests { } tokio::time::sleep(Duration::from_millis(10)).await; }; + // `ApproveAlwaysForTool` resolves the parked prompt to Allow and, because + // the prompt persisted a row, returns its id. (Persisting the tool onto + // the `auto_approve` allowlist for *future* calls is the RPC handler's + // job — see `approval::rpc::approval_decide` — and the gate's allowlist + // short-circuit is covered by `auto_approve_tool_skips_prompt`.) gate.decide(&pending.request_id, ApprovalDecision::ApproveAlwaysForTool) .unwrap(); let (first_outcome, first_id) = first.await.unwrap(); @@ -803,17 +833,5 @@ mod tests { first_id.is_some(), "the prompting call still persists a row" ); - - let (second_outcome, second_id) = APPROVAL_CHAT_CONTEXT - .scope( - chat_ctx(), - gate.intercept_audited("pushover", "second send", serde_json::json!({})), - ) - .await; - assert!(matches!(second_outcome, GateOutcome::Allow)); - assert!( - second_id.is_none(), - "session-allowlist shortcut must not persist a row, so no id to record against" - ); } } diff --git a/src/openhuman/approval/mod.rs b/src/openhuman/approval/mod.rs index 3c9bade1b..0eac1c435 100644 --- a/src/openhuman/approval/mod.rs +++ b/src/openhuman/approval/mod.rs @@ -1,20 +1,19 @@ //! Interactive approval workflow for supervised mode. //! -//! Two layers: +//! [`ApprovalGate`] (in [`gate`]) is the single approval path — async +//! middleware between the agent and any tool whose +//! [`crate::openhuman::tools::Tool::external_effect`] returns `true`. It +//! persists pending rows in SQLite, parks the tool-call future on a oneshot, +//! and resumes when the UI dispatches `approval_decide`. Introduced for issue +//! #1339 so external-channel writes (Slack post, email send, calendar create, +//! …) cannot fire without explicit user consent. //! -//! - [`ApprovalManager`] (legacy, in [`ops`]) — CLI-only synchronous -//! prompt + in-memory session allowlist + audit log. Still used by -//! the agent harness when running under `--channel cli`. -//! - [`ApprovalGate`] (new, in [`gate`]) — async middleware between the -//! agent and any tool whose [`crate::openhuman::tools::Tool::external_effect`] -//! returns `true`. Persists pending rows in SQLite, parks the -//! tool-call future on a oneshot, and resumes when the UI dispatches -//! `approval_decide`. Introduced for issue #1339 so external-channel -//! writes (Slack post, email send, calendar create, …) cannot fire -//! without explicit user consent. +//! The user's "Always allow" allowlist lives in `autonomy.auto_approve` (read +//! by the gate via [`crate::openhuman::security::SecurityPolicy`]); a prior +//! list-based `ApprovalManager` that consumed it was removed once the gate +//! became the sole control. pub mod gate; -pub mod ops; pub mod redact; pub mod rpc; pub mod schemas; @@ -22,7 +21,6 @@ pub mod store; pub mod types; pub use gate::{parse_approval_reply, ApprovalChatContext, ApprovalGate, APPROVAL_CHAT_CONTEXT}; -pub use ops::*; pub use redact::{redact_args, summarize_action}; pub use schemas::all_controller_schemas as all_approval_controller_schemas; pub use schemas::all_registered_controllers as all_approval_registered_controllers; diff --git a/src/openhuman/approval/ops.rs b/src/openhuman/approval/ops.rs deleted file mode 100644 index 19dc019a6..000000000 --- a/src/openhuman/approval/ops.rs +++ /dev/null @@ -1,421 +0,0 @@ -use crate::openhuman::config::AutonomyConfig; -use crate::openhuman::security::AutonomyLevel; -use chrono::Utc; -use parking_lot::Mutex; -use serde::{Deserialize, Serialize}; -use std::collections::HashSet; -use std::io::{self, BufRead, Write}; - -// ── Types ──────────────────────────────────────────────────────── - -/// A request to approve a tool call before execution. -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct ApprovalRequest { - pub tool_name: String, - pub arguments: serde_json::Value, -} - -/// The user's response to an approval request. -#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] -#[serde(rename_all = "lowercase")] -pub enum ApprovalResponse { - /// Execute this one call. - Yes, - /// Deny this call. - No, - /// Execute and add tool to session-scoped allowlist. - Always, -} - -/// A single audit log entry for an approval decision. -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct ApprovalLogEntry { - pub timestamp: String, - pub tool_name: String, - pub arguments_summary: String, - pub decision: ApprovalResponse, - pub channel: String, -} - -// ── ApprovalManager ────────────────────────────────────────────── - -/// Manages the interactive approval workflow. -/// -/// - Checks config-level `auto_approve` / `always_ask` lists -/// - Maintains a session-scoped "always" allowlist -/// - Records an audit trail of all decisions -pub struct ApprovalManager { - /// Tools that never need approval (from config). - auto_approve: HashSet, - /// Tools that always need approval, ignoring session allowlist. - always_ask: HashSet, - /// Autonomy level from config. - autonomy_level: AutonomyLevel, - /// Session-scoped allowlist built from "Always" responses. - session_allowlist: Mutex>, - /// Audit trail of approval decisions. - audit_log: Mutex>, -} - -impl ApprovalManager { - /// Create from autonomy config. - pub fn from_config(config: &AutonomyConfig) -> Self { - Self { - auto_approve: config.auto_approve.iter().cloned().collect(), - always_ask: config.always_ask.iter().cloned().collect(), - autonomy_level: config.level, - session_allowlist: Mutex::new(HashSet::new()), - audit_log: Mutex::new(Vec::new()), - } - } - - /// Check whether a tool call requires interactive approval. - /// - /// Returns `true` if the call needs a prompt, `false` if it can proceed. - pub fn needs_approval(&self, tool_name: &str) -> bool { - // Full autonomy never prompts. - if self.autonomy_level == AutonomyLevel::Full { - return false; - } - - // ReadOnly blocks everything — handled elsewhere; no prompt needed. - if self.autonomy_level == AutonomyLevel::ReadOnly { - return false; - } - - // always_ask overrides everything. - if self.always_ask.contains(tool_name) { - return true; - } - - // auto_approve skips the prompt. - if self.auto_approve.contains(tool_name) { - return false; - } - - // Session allowlist (from prior "Always" responses). - let allowlist = self.session_allowlist.lock(); - if allowlist.contains(tool_name) { - return false; - } - - // Default: supervised mode requires approval. - true - } - - /// Record an approval decision and update session state. - pub fn record_decision( - &self, - tool_name: &str, - args: &serde_json::Value, - decision: ApprovalResponse, - channel: &str, - ) { - // If "Always", add to session allowlist. - if decision == ApprovalResponse::Always { - let mut allowlist = self.session_allowlist.lock(); - allowlist.insert(tool_name.to_string()); - } - - // Append to audit log. - let summary = summarize_args(args); - let entry = ApprovalLogEntry { - timestamp: Utc::now().to_rfc3339(), - tool_name: tool_name.to_string(), - arguments_summary: summary, - decision, - channel: channel.to_string(), - }; - let mut log = self.audit_log.lock(); - log.push(entry); - } - - /// Get a snapshot of the audit log. - pub fn audit_log(&self) -> Vec { - self.audit_log.lock().clone() - } - - /// Get the current session allowlist. - pub fn session_allowlist(&self) -> HashSet { - self.session_allowlist.lock().clone() - } - - /// Prompt the user on the local console and return their decision. - /// - /// In the web UI, approvals are handled elsewhere; this is a fallback - /// for non-UI environments. - pub fn prompt_cli(&self, request: &ApprovalRequest) -> ApprovalResponse { - prompt_cli_interactive(request) - } -} - -// ── Console prompt ─────────────────────────────────────────────── - -/// Display the approval prompt and read user input from stdin. -fn prompt_cli_interactive(request: &ApprovalRequest) -> ApprovalResponse { - let summary = summarize_args(&request.arguments); - eprintln!(); - eprintln!("🔧 Agent wants to execute: {}", request.tool_name); - eprintln!(" {summary}"); - eprint!(" [Y]es / [N]o / [A]lways for {}: ", request.tool_name); - let _ = io::stderr().flush(); - - let stdin = io::stdin(); - let mut line = String::new(); - if stdin.lock().read_line(&mut line).is_err() { - return ApprovalResponse::No; - } - - match line.trim().to_ascii_lowercase().as_str() { - "y" | "yes" => ApprovalResponse::Yes, - "a" | "always" => ApprovalResponse::Always, - _ => ApprovalResponse::No, - } -} - -/// Produce a short human-readable summary of tool arguments. -fn summarize_args(args: &serde_json::Value) -> String { - match args { - serde_json::Value::Object(map) => { - let parts: Vec = map - .iter() - .map(|(k, v)| { - let val = match v { - serde_json::Value::String(s) => truncate_for_summary(s, 80), - other => { - let s = other.to_string(); - truncate_for_summary(&s, 80) - } - }; - format!("{k}: {val}") - }) - .collect(); - parts.join(", ") - } - other => { - let s = other.to_string(); - truncate_for_summary(&s, 120) - } - } -} - -fn truncate_for_summary(input: &str, max_chars: usize) -> String { - let mut chars = input.chars(); - let truncated: String = chars.by_ref().take(max_chars).collect(); - if chars.next().is_some() { - format!("{truncated}…") - } else { - input.to_string() - } -} - -// ── Tests ──────────────────────────────────────────────────────── - -#[cfg(test)] -mod tests { - use super::*; - use crate::openhuman::config::AutonomyConfig; - - fn supervised_config() -> AutonomyConfig { - AutonomyConfig { - level: AutonomyLevel::Supervised, - auto_approve: vec!["file_read".into(), "memory_recall".into()], - always_ask: vec!["shell".into()], - ..AutonomyConfig::default() - } - } - - fn full_config() -> AutonomyConfig { - AutonomyConfig { - level: AutonomyLevel::Full, - ..AutonomyConfig::default() - } - } - - // ── needs_approval ─────────────────────────────────────── - - #[test] - fn auto_approve_tools_skip_prompt() { - let mgr = ApprovalManager::from_config(&supervised_config()); - assert!(!mgr.needs_approval("file_read")); - assert!(!mgr.needs_approval("memory_recall")); - } - - #[test] - fn always_ask_tools_always_prompt() { - let mgr = ApprovalManager::from_config(&supervised_config()); - assert!(mgr.needs_approval("shell")); - } - - #[test] - fn unknown_tool_needs_approval_in_supervised() { - let mgr = ApprovalManager::from_config(&supervised_config()); - assert!(mgr.needs_approval("file_write")); - assert!(mgr.needs_approval("http_request")); - } - - #[test] - fn full_autonomy_never_prompts() { - let mgr = ApprovalManager::from_config(&full_config()); - assert!(!mgr.needs_approval("shell")); - assert!(!mgr.needs_approval("file_write")); - assert!(!mgr.needs_approval("anything")); - } - - #[test] - fn readonly_never_prompts() { - let config = AutonomyConfig { - level: AutonomyLevel::ReadOnly, - ..AutonomyConfig::default() - }; - let mgr = ApprovalManager::from_config(&config); - assert!(!mgr.needs_approval("shell")); - } - - // ── session allowlist ──────────────────────────────────── - - #[test] - fn always_response_adds_to_session_allowlist() { - let mgr = ApprovalManager::from_config(&supervised_config()); - assert!(mgr.needs_approval("file_write")); - - mgr.record_decision( - "file_write", - &serde_json::json!({"path": "test.txt"}), - ApprovalResponse::Always, - "cli", - ); - - // Now file_write should be in session allowlist. - assert!(!mgr.needs_approval("file_write")); - } - - #[test] - fn always_ask_overrides_session_allowlist() { - let mgr = ApprovalManager::from_config(&supervised_config()); - - // Even after "Always" for shell, it should still prompt. - mgr.record_decision( - "shell", - &serde_json::json!({"command": "ls"}), - ApprovalResponse::Always, - "cli", - ); - - // shell is in always_ask, so it still needs approval. - assert!(mgr.needs_approval("shell")); - } - - #[test] - fn yes_response_does_not_add_to_allowlist() { - let mgr = ApprovalManager::from_config(&supervised_config()); - mgr.record_decision( - "file_write", - &serde_json::json!({}), - ApprovalResponse::Yes, - "cli", - ); - assert!(mgr.needs_approval("file_write")); - } - - // ── audit log ──────────────────────────────────────────── - - #[test] - fn audit_log_records_decisions() { - let mgr = ApprovalManager::from_config(&supervised_config()); - - mgr.record_decision( - "shell", - &serde_json::json!({"command": "rm -rf ./build/"}), - ApprovalResponse::No, - "cli", - ); - mgr.record_decision( - "file_write", - &serde_json::json!({"path": "out.txt", "content": "hello"}), - ApprovalResponse::Yes, - "cli", - ); - - let log = mgr.audit_log(); - assert_eq!(log.len(), 2); - assert_eq!(log[0].tool_name, "shell"); - assert_eq!(log[0].decision, ApprovalResponse::No); - assert_eq!(log[1].tool_name, "file_write"); - assert_eq!(log[1].decision, ApprovalResponse::Yes); - } - - #[test] - fn audit_log_contains_timestamp_and_channel() { - let mgr = ApprovalManager::from_config(&supervised_config()); - mgr.record_decision( - "shell", - &serde_json::json!({"command": "ls"}), - ApprovalResponse::Yes, - "telegram", - ); - - let log = mgr.audit_log(); - assert_eq!(log.len(), 1); - assert!(!log[0].timestamp.is_empty()); - assert_eq!(log[0].channel, "telegram"); - } - - // ── summarize_args ─────────────────────────────────────── - - #[test] - fn summarize_args_object() { - let args = serde_json::json!({"command": "ls -la", "cwd": "/tmp"}); - let summary = summarize_args(&args); - assert!(summary.contains("command: ls -la")); - assert!(summary.contains("cwd: /tmp")); - } - - #[test] - fn summarize_args_truncates_long_values() { - let long_val = "x".repeat(200); - let args = serde_json::json!({"content": long_val}); - let summary = summarize_args(&args); - assert!(summary.contains('…')); - assert!(summary.len() < 200); - } - - #[test] - fn summarize_args_unicode_safe_truncation() { - let long_val = "🦀".repeat(120); - let args = serde_json::json!({"content": long_val}); - let summary = summarize_args(&args); - assert!(summary.contains("content:")); - assert!(summary.contains('…')); - } - - #[test] - fn summarize_args_non_object() { - let args = serde_json::json!("just a string"); - let summary = summarize_args(&args); - assert!(summary.contains("just a string")); - } - - // ── ApprovalResponse serde ─────────────────────────────── - - #[test] - fn approval_response_serde_roundtrip() { - let json = serde_json::to_string(&ApprovalResponse::Always).unwrap(); - assert_eq!(json, "\"always\""); - let parsed: ApprovalResponse = serde_json::from_str("\"no\"").unwrap(); - assert_eq!(parsed, ApprovalResponse::No); - } - - // ── ApprovalRequest ────────────────────────────────────── - - #[test] - fn approval_request_serde() { - let req = ApprovalRequest { - tool_name: "shell".into(), - arguments: serde_json::json!({"command": "echo hi"}), - }; - let json = serde_json::to_string(&req).unwrap(); - let parsed: ApprovalRequest = serde_json::from_str(&json).unwrap(); - assert_eq!(parsed.tool_name, "shell"); - } -} diff --git a/src/openhuman/approval/rpc.rs b/src/openhuman/approval/rpc.rs index 85c6c261b..82367a2a2 100644 --- a/src/openhuman/approval/rpc.rs +++ b/src/openhuman/approval/rpc.rs @@ -98,17 +98,51 @@ pub async fn approval_decide( ); anyhow!("no pending approval found for request_id '{request_id}'") })?; + + let mut logs = vec![format!( + "[approval] decided request_id={} tool={} decision={}", + row.request_id, + row.tool_name, + decision.as_str() + )]; + + // "Always allow": persist the tool onto the user's `autonomy.auto_approve` + // allowlist (config save + live-policy reload) so the gate skips prompting + // for it on future turns — this session and across restarts. Best-effort: + // `gate.decide` already resolved the current call, so a persistence failure + // must not fail the RPC. It degrades safely — the tool simply prompts again + // next time rather than being silently auto-approved. + if decision == ApprovalDecision::ApproveAlwaysForTool { + match crate::openhuman::config::ops::add_auto_approve_tool(&row.tool_name).await { + Ok(()) => { + tracing::info!( + tool = row.tool_name.as_str(), + "[rpc:approval_decide] tool persisted to auto_approve allowlist" + ); + logs.push(format!( + "[approval] '{}' added to the Always-allow list", + row.tool_name + )); + } + Err(err) => { + tracing::warn!( + tool = row.tool_name.as_str(), + error = %err, + "[rpc:approval_decide] failed to persist auto_approve; tool will prompt again next time" + ); + logs.push(format!( + "[approval] WARNING: could not save 'Always allow' for '{}': {err}", + row.tool_name + )); + } + } + } + tracing::info!( request_id = row.request_id.as_str(), tool = row.tool_name.as_str(), decision = decision.as_str(), "[rpc:approval_decide] exit" ); - let log = format!( - "[approval] decided request_id={} tool={} decision={}", - row.request_id, - row.tool_name, - decision.as_str() - ); - Ok(RpcOutcome::single_log(row, log)) + Ok(RpcOutcome::new(row, logs)) } diff --git a/src/openhuman/config/ops.rs b/src/openhuman/config/ops.rs index 8a3ae05d0..ff82eecce 100644 --- a/src/openhuman/config/ops.rs +++ b/src/openhuman/config/ops.rs @@ -363,7 +363,8 @@ pub struct RuntimeSettingsPatch { /// Partial update for the `[autonomy]` block — the agent's filesystem access /// mode. Each `None` field is left unchanged. `trusted_roots`, `allowed_commands`, -/// and `forbidden_paths`, when `Some`, REPLACE the corresponding array wholesale. +/// `forbidden_paths`, and `auto_approve`, when `Some`, REPLACE the corresponding +/// array wholesale. #[derive(Debug, Clone, Default)] pub struct AutonomySettingsPatch { /// `"readonly" | "supervised" | "full"` (case-insensitive). @@ -374,6 +375,8 @@ pub struct AutonomySettingsPatch { pub trusted_roots: Option>, pub allow_tool_install: Option, pub max_actions_per_hour: Option, + /// "Always allow" allowlist — tool names the gate skips prompting for. + pub auto_approve: Option>, } #[derive(Debug, Clone, Default)] @@ -860,6 +863,9 @@ pub async fn apply_autonomy_settings( } config.autonomy.max_actions_per_hour = max_actions_per_hour; } + if let Some(auto_approve) = update.auto_approve { + config.autonomy.auto_approve = auto_approve; + } config.save().await.map_err(|e| e.to_string())?; @@ -888,6 +894,42 @@ pub async fn load_and_apply_autonomy_settings( apply_autonomy_settings(&mut config, update).await } +/// Serializes the load-modify-save in [`add_auto_approve_tool`] so two +/// concurrent "Always allow" appends (different tools) can't read the same +/// `auto_approve`, each push their own, and clobber the other on save +/// (last-write-wins lost-update). Holding it across load→save makes the second +/// caller observe the first's write and union the entries. Process-local; the +/// allowlist lives in a single per-launch config file. (CodeRabbit, PR #2706.) +fn auto_approve_write_lock() -> &'static tokio::sync::Mutex<()> { + static LOCK: std::sync::OnceLock> = std::sync::OnceLock::new(); + LOCK.get_or_init(|| tokio::sync::Mutex::new(())) +} + +/// Append `tool_name` to `autonomy.auto_approve` ("Always allow") and persist + +/// reload the live policy. Idempotent — a no-op (no disk write) when the tool is +/// already allow-listed. Backs the `ApproveAlwaysForTool` approval decision. +pub async fn add_auto_approve_tool(tool_name: &str) -> Result<(), String> { + // Serialize the read-modify-write against concurrent appends (see lock doc). + let _guard = auto_approve_write_lock().lock().await; + let mut config = load_config_with_timeout().await?; + if config.autonomy.auto_approve.iter().any(|t| t == tool_name) { + tracing::debug!( + tool = tool_name, + "[config:auto_approve] tool already allow-listed; nothing to persist" + ); + return Ok(()); + } + let mut next = config.autonomy.auto_approve.clone(); + next.push(tool_name.to_string()); + let patch = AutonomySettingsPatch { + auto_approve: Some(next), + ..AutonomySettingsPatch::default() + }; + apply_autonomy_settings(&mut config, patch) + .await + .map(|_| ()) +} + /// Returns the current `[autonomy]` settings block as JSON (no secrets). /// /// Emits a log line so `into_cli_compatible_json` wraps the payload under diff --git a/src/openhuman/config/ops_tests.rs b/src/openhuman/config/ops_tests.rs index a96e31b7f..a4626122c 100644 --- a/src/openhuman/config/ops_tests.rs +++ b/src/openhuman/config/ops_tests.rs @@ -1198,6 +1198,7 @@ async fn apply_screen_intelligence_settings_clamps_baseline_fps() { #[tokio::test] async fn apply_autonomy_settings_persists_max_actions_per_hour() { + let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let tmp = tempdir().unwrap(); let mut cfg = tmp_config(&tmp); let outcome = apply_autonomy_settings( @@ -1222,6 +1223,7 @@ async fn apply_autonomy_settings_persists_max_actions_per_hour() { #[tokio::test] async fn apply_autonomy_settings_no_op_when_patch_empty() { + let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let tmp = tempdir().unwrap(); let mut cfg = tmp_config(&tmp); let prior = cfg.autonomy.max_actions_per_hour; @@ -1239,6 +1241,7 @@ async fn apply_autonomy_settings_no_op_when_patch_empty() { #[tokio::test] async fn apply_autonomy_settings_rejects_zero() { + let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let tmp = tempdir().unwrap(); let mut cfg = tmp_config(&tmp); let err = apply_autonomy_settings( @@ -1258,6 +1261,7 @@ async fn apply_autonomy_settings_rejects_zero() { #[tokio::test] async fn apply_autonomy_settings_accepts_unlimited_sentinel() { + let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); // u32::MAX is the new "unlimited" sentinel exposed by the UI as a // preset. The upper cap was lifted in the same PR that defaulted // fresh installs to u32::MAX; anything in [1, u32::MAX] should now @@ -1301,3 +1305,61 @@ async fn load_and_apply_autonomy_settings_roundtrip() { std::env::remove_var("OPENHUMAN_WORKSPACE"); } } + +#[tokio::test] +async fn apply_autonomy_settings_replaces_auto_approve() { + // ENV_LOCK serializes the `live_policy::reload_from` triggered by + // `apply_autonomy_settings` against other live-policy-touching tests. + let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let tmp = tempdir().unwrap(); + let mut cfg = tmp_config(&tmp); + apply_autonomy_settings( + &mut cfg, + AutonomySettingsPatch { + auto_approve: Some(vec!["shell".into(), "curl".into()]), + ..Default::default() + }, + ) + .await + .expect("apply auto_approve"); + assert_eq!(cfg.autonomy.auto_approve, vec!["shell", "curl"]); + // Persisted to the TOML, not just held in memory. + let on_disk = tokio::fs::read_to_string(&cfg.config_path).await.unwrap(); + assert!( + on_disk.contains("auto_approve") && on_disk.contains("shell") && on_disk.contains("curl"), + "auto_approve allowlist should round-trip to TOML, got:\n{on_disk}" + ); +} + +#[tokio::test] +async fn add_auto_approve_tool_appends_then_dedupes() { + let _g = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let tmp = tempdir().unwrap(); + unsafe { + std::env::set_var("OPENHUMAN_WORKSPACE", tmp.path()); + } + + add_auto_approve_tool("git_operations") + .await + .expect("first add"); + // Idempotent: a second add of the same tool must not create a duplicate. + add_auto_approve_tool("git_operations") + .await + .expect("second add (idempotent)"); + + let reloaded = load_config_with_timeout().await.expect("reload"); + let hits = reloaded + .autonomy + .auto_approve + .iter() + .filter(|t| t.as_str() == "git_operations") + .count(); + assert_eq!( + hits, 1, + "tool must appear exactly once after duplicate adds" + ); + + unsafe { + std::env::remove_var("OPENHUMAN_WORKSPACE"); + } +} diff --git a/src/openhuman/config/schema/autonomy.rs b/src/openhuman/config/schema/autonomy.rs index dc4952122..4b49fb3fc 100644 --- a/src/openhuman/config/schema/autonomy.rs +++ b/src/openhuman/config/schema/autonomy.rs @@ -25,10 +25,12 @@ pub struct AutonomyConfig { pub require_approval_for_medium_risk: bool, #[serde(default = "default_true")] pub block_high_risk_commands: bool, + /// Tool names the user has pre-approved ("Always allow"). The `ApprovalGate` + /// skips the interactive prompt for any tool listed here. Populated by the + /// "Always allow" approval decision or hand-edited, and surfaced in + /// Settings → Agent access. Read live by the gate via `SecurityPolicy`. #[serde(default = "default_auto_approve")] pub auto_approve: Vec, - #[serde(default = "default_always_ask")] - pub always_ask: Vec, /// Directories outside the workspace the agent may access. Each entry grants /// read (or read+write) to its subtree, taking precedence over `workspace_only` /// and `forbidden_paths` — except credential stores (~/.ssh, ~/.gnupg, ~/.aws), @@ -112,10 +114,6 @@ fn default_auto_approve() -> Vec { ] } -fn default_always_ask() -> Vec { - vec![] -} - impl Default for AutonomyConfig { fn default() -> Self { Self { @@ -128,7 +126,6 @@ impl Default for AutonomyConfig { require_approval_for_medium_risk: default_true(), block_high_risk_commands: default_true(), auto_approve: default_auto_approve(), - always_ask: default_always_ask(), trusted_roots: Vec::new(), allow_tool_install: false, } diff --git a/src/openhuman/config/schemas.rs b/src/openhuman/config/schemas.rs index 552374364..c6b32fa4b 100644 --- a/src/openhuman/config/schemas.rs +++ b/src/openhuman/config/schemas.rs @@ -214,6 +214,9 @@ struct AutonomySettingsUpdate { // Accept u64 to match the published schema (`TypeSchema::U64`); clamped to the // internal u32 at apply time. u32::MAX/hr is already effectively unlimited. max_actions_per_hour: Option, + /// Replaces the "Always allow" allowlist wholesale — tool names the agent + /// may run without an approval prompt. Empty list clears it. + auto_approve: Option>, } pub fn all_controller_schemas() -> Vec { @@ -600,6 +603,12 @@ pub fn schemas(function: &str) -> ControllerSchema { comment: "Rate limit for side-effecting actions per hour.", required: false, }, + FieldSchema { + name: "auto_approve", + ty: TypeSchema::Option(Box::new(TypeSchema::Array(Box::new(TypeSchema::String)))), + comment: "Replace the \"Always allow\" allowlist (array of tool names the agent runs without an approval prompt). Empty array clears it.", + required: false, + }, ], outputs: vec![json_output("snapshot", "Updated config snapshot.")], }, @@ -1212,6 +1221,7 @@ fn handle_update_autonomy_settings(params: Map) -> ControllerFutu max_actions_per_hour: update .max_actions_per_hour .map(|v| u32::try_from(v).unwrap_or(u32::MAX)), + auto_approve: update.auto_approve, }; to_json(config_rpc::load_and_apply_autonomy_settings(patch).await?) }) diff --git a/src/openhuman/security/live_policy.rs b/src/openhuman/security/live_policy.rs index ad26d78c5..b2ac5efd6 100644 --- a/src/openhuman/security/live_policy.rs +++ b/src/openhuman/security/live_policy.rs @@ -92,6 +92,12 @@ mod tests { #[test] fn install_then_reload_swaps_policy_and_bumps_generation() { + // Serialize against other tests that install/reload this process-global + // (the approval-gate auto_approve test and the autonomy `ops` tests), + // which all take this same lock — otherwise a parallel install races. + let _env = crate::openhuman::config::TEST_ENV_LOCK + .lock() + .unwrap_or_else(|e| e.into_inner()); let workspace = std::env::temp_dir().join("openhuman_live_policy_test"); let initial = Arc::new(SecurityPolicy { autonomy: AutonomyLevel::Supervised, diff --git a/src/openhuman/security/policy.rs b/src/openhuman/security/policy.rs index d25c9ff52..d9443d2cd 100644 --- a/src/openhuman/security/policy.rs +++ b/src/openhuman/security/policy.rs @@ -182,6 +182,11 @@ pub struct SecurityPolicy { pub trusted_roots: Vec, /// Whether the agent may install OS packages via the `install_tool` tool. pub allow_tool_install: bool, + /// Tool names the user has pre-approved ("Always allow"). The `ApprovalGate` + /// skips the interactive prompt for any tool in this set. Sourced from + /// `autonomy.auto_approve`; populated/cleared via `config.update_autonomy_settings` + /// (or an "Always allow" decision) and observed live via `live_policy`. + pub auto_approve: Vec, pub tracker: ActionTracker, } @@ -241,6 +246,7 @@ impl Default for SecurityPolicy { block_high_risk_commands: true, trusted_roots: Vec::new(), allow_tool_install: false, + auto_approve: Vec::new(), tracker: ActionTracker::new(), } } @@ -1942,11 +1948,12 @@ impl SecurityPolicy { autonomy_config.max_actions_per_hour ); - // NOTE: `autonomy_config.auto_approve` / `always_ask` are loaded from - // config (with non-empty defaults) but are NOT consumed here — the - // ApprovalGate has no always-allow / always-ask allowlist wired to them - // yet, so approval is driven purely by tier + `CommandClass`. These - // fields pre-date this PR; wiring them into the gate is a follow-up. + // `auto_approve` is the user's "Always allow" allowlist: the + // `ApprovalGate` reads it via `live_policy::current()` and skips the + // interactive prompt for any tool named in it. Tier + `CommandClass` + // (and the unconditional read-only / forbidden-path / high-risk denials) + // still run *before* the gate, so the allowlist can only suppress the + // human prompt — it can never override a hard policy denial. // The default projects home (`~/OpenHuman/projects`) is always a // read-write trusted root so the coding agent can create/edit projects @@ -1978,6 +1985,7 @@ impl SecurityPolicy { block_high_risk_commands: autonomy_config.block_high_risk_commands, trusted_roots, allow_tool_install: autonomy_config.allow_tool_install, + auto_approve: autonomy_config.auto_approve.clone(), tracker: ActionTracker::new(), } } diff --git a/src/openhuman/security/policy_tests.rs b/src/openhuman/security/policy_tests.rs index 6648ab66d..0a7bb9393 100644 --- a/src/openhuman/security/policy_tests.rs +++ b/src/openhuman/security/policy_tests.rs @@ -898,6 +898,7 @@ fn from_config_maps_all_fields() { max_cost_per_day_cents: 1000, require_approval_for_medium_risk: false, block_high_risk_commands: false, + auto_approve: vec!["shell".into(), "file_write".into()], ..crate::openhuman::config::AutonomyConfig::default() }; let workspace = PathBuf::from("/tmp/test-workspace"); @@ -912,6 +913,9 @@ fn from_config_maps_all_fields() { assert!(!policy.require_approval_for_medium_risk); assert!(!policy.block_high_risk_commands); assert_eq!(policy.workspace_dir, PathBuf::from("/tmp/test-workspace")); + // The "Always allow" allowlist is carried onto the policy so the gate can + // skip prompting for these tools. + assert_eq!(policy.auto_approve, vec!["shell", "file_write"]); } // -- Default policy ----------------------------------------------- diff --git a/tests/json_rpc_e2e.rs b/tests/json_rpc_e2e.rs index bf09b471d..f6833edd6 100644 --- a/tests/json_rpc_e2e.rs +++ b/tests/json_rpc_e2e.rs @@ -7703,6 +7703,42 @@ async fn json_rpc_config_autonomy_settings_roundtrip() { "expected validation error in: {err_message}" ); + // auto_approve ("Always allow" allowlist) round-trips through the same + // update/get path the Agent Access settings panel uses. + let update_allow = post_json_rpc( + &rpc_base, + 7005, + "openhuman.config_update_autonomy_settings", + json!({ "auto_approve": ["shell", "curl"] }), + ) + .await; + assert_no_jsonrpc_error(&update_allow, "update_autonomy_settings auto_approve"); + + let after_allow = post_json_rpc( + &rpc_base, + 7006, + "openhuman.config_get_autonomy_settings", + json!({}), + ) + .await; + let after_allow_outer = + assert_no_jsonrpc_error(&after_allow, "get_autonomy_settings auto_approve"); + let allow_list: Vec = after_allow_outer + .get("result") + .and_then(|r| r.get("auto_approve")) + .and_then(Value::as_array) + .map(|arr| { + arr.iter() + .filter_map(|v| v.as_str().map(str::to_string)) + .collect() + }) + .unwrap_or_default(); + assert_eq!( + allow_list, + vec!["shell".to_string(), "curl".to_string()], + "auto_approve allowlist should round-trip, got envelope: {after_allow_outer}" + ); + mock_join.abort(); rpc_join.abort(); }