feat(approval): consolidate on ApprovalGate + persistent "Always allow" list (#2706)

This commit is contained in:
sanil-23
2026-05-27 05:54:24 +05:30
committed by GitHub
parent a3023d0947
commit 0fddf1120f
54 changed files with 657 additions and 674 deletions
@@ -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<Props> = ({ threadId, approval }) => {
const { t } = useT();
const dispatch = useAppDispatch();
const [deciding, setDeciding] = useState<BinaryDecision | null>(null);
const [deciding, setDeciding] = useState<Decision | null>(null);
const [errorMsg, setErrorMsg] = useState<string | null>(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<Props> = ({ threadId, approval }) =>
{errorMsg && <p className="mt-2 text-xs text-coral"> {errorMsg}</p>}
<div className="mt-3 flex items-center gap-2">
<div className="mt-3 flex flex-wrap items-center gap-2">
<Button
variant="primary"
size="sm"
@@ -90,6 +93,16 @@ export const ApprovalRequestCard: React.FC<Props> = ({ threadId, approval }) =>
? t('chat.approval.deciding')
: t('chat.approval.approve')}
</Button>
<Button
variant="secondary"
size="sm"
onClick={() => void decide('approve_always_for_tool')}
disabled={deciding !== null}
title={t('chat.approval.alwaysAllowHint')}>
{deciding === 'approve_always_for_tool'
? t('chat.approval.deciding')
: t('chat.approval.alwaysAllow')}
</Button>
<Button
variant="secondary"
size="sm"
@@ -80,6 +80,21 @@ describe('ApprovalRequestCard', () => {
});
});
it('Always allow routes approve_always_for_tool to approval_decide and clears the pending state', async () => {
vi.mocked(callCoreRpc).mockResolvedValueOnce({});
const { store } = renderCard();
fireEvent.click(screen.getByText('Always allow'));
expect(callCoreRpc).toHaveBeenCalledWith({
method: 'openhuman.approval_decide',
params: { request_id: 'req-approval-1', decision: 'approve_always_for_tool' },
});
await waitFor(() => {
expect(store.getState().chatRuntime.pendingApprovalByThread[THREAD]).toBeUndefined();
});
});
it('keeps the prompt and shows an error when the decide RPC fails', async () => {
vi.mocked(callCoreRpc).mockRejectedValueOnce(new Error('gate not installed'));
const { store } = renderCard();
@@ -51,6 +51,9 @@ const AgentAccessPanel = () => {
const [level, setLevel] = useState<AutonomyLevel>('supervised');
const [workspaceOnly, setWorkspaceOnly] = useState(false);
const [trustedRoots, setTrustedRoots] = useState<TrustedRoot[]>([]);
// "Always allow" allowlist — populated by the in-chat "Always allow" button;
// shown here read-only with a Remove action (the re-protect path).
const [autoApprove, setAutoApprove] = useState<string[]>([]);
const [newRootPath, setNewRootPath] = useState('');
const [newRootAccess, setNewRootAccess] = useState<TrustedAccess>('read');
@@ -76,6 +79,7 @@ const AgentAccessPanel = () => {
setLevel(resp.result.level);
setWorkspaceOnly(resp.result.workspace_only);
setTrustedRoots(resp.result.trusted_roots ?? []);
setAutoApprove(resp.result.auto_approve ?? []);
} catch (e) {
if (!cancelled)
setError(e instanceof Error ? e.message : t('settings.agentAccess.loadError'));
@@ -97,6 +101,11 @@ const AgentAccessPanel = () => {
level: AutonomyLevel;
workspaceOnly: boolean;
trustedRoots: TrustedRoot[];
// Only sent when the allowlist itself is being changed. Omitting it leaves
// the server's `auto_approve` untouched (partial patch) — important so a
// tier/folder change here can't clobber a tool the user just added via the
// in-chat "Always allow" button.
autoApprove?: string[];
}) => {
const seq = ++persistSeqRef.current;
if (!isTauri()) return;
@@ -109,6 +118,7 @@ const AgentAccessPanel = () => {
workspace_only: next.workspaceOnly,
trusted_roots: next.trustedRoots,
allow_tool_install: ALLOW_TOOL_INSTALL,
...(next.autoApprove !== undefined ? { auto_approve: next.autoApprove } : {}),
});
// Only the most recent persist may write UI state back.
if (persistSeqRef.current === seq) {
@@ -155,6 +165,12 @@ const AgentAccessPanel = () => {
void persist({ level, workspaceOnly, trustedRoots: nextRoots });
};
const removeAutoApprove = (tool: string) => {
const nextList = autoApprove.filter(name => name !== tool);
setAutoApprove(nextList);
void persist({ level, workspaceOnly, trustedRoots, autoApprove: nextList });
};
return (
<div>
<SettingsHeader
@@ -290,6 +306,35 @@ const AgentAccessPanel = () => {
</div>
</section>
{/* "Always allow" allowlist — tools the user chose to stop being
prompted for, via the in-chat approval card. Read-only here with
a Remove action to re-enable prompting for a tool. */}
<section className="space-y-2">
<h2 className="text-sm font-semibold text-ink">
{t('settings.agentAccess.alwaysAllow')}
</h2>
<p className="text-xs text-ink-soft">{t('settings.agentAccess.alwaysAllowDesc')}</p>
{autoApprove.length === 0 ? (
<p className="text-xs text-ink-soft">{t('settings.agentAccess.alwaysAllowNone')}</p>
) : (
<ul className="space-y-1">
{autoApprove.map(tool => (
<li
key={tool}
className="flex items-center justify-between rounded border border-line px-2 py-1">
<span className="font-mono text-xs text-ink truncate">{tool}</span>
<button
type="button"
onClick={() => removeAutoApprove(tool)}
className="text-xs text-coral hover:underline">
{t('settings.agentAccess.remove')}
</button>
</li>
))}
</ul>
)}
</section>
{/* Auto-save status — changes persist on selection; no manual save. */}
<div className="min-h-[1.25rem] text-sm" aria-live="polite">
{error ? (
@@ -18,6 +18,7 @@ const autonomy = (overrides: Partial<AutonomySettings> = {}): 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(<AgentAccessPanel />);
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(<AgentAccessPanel />);
// 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(<AgentAccessPanel />);
@@ -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', () => ({
+2
View File
@@ -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.',
+4
View File
@@ -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.',
+2
View File
@@ -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.',
+4
View File
@@ -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.',
+2
View File
@@ -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.',
+4
View File
@@ -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.',
+2
View File
@@ -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.',
+4
View File
@@ -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.',
+2
View File
@@ -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.',
+4
View File
@@ -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.',
+2
View File
@@ -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.',
+4
View File
@@ -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.',
+2
View File
@@ -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.',
+4
View File
@@ -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.',
+2
View File
@@ -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.',
+4
View File
@@ -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.',
+2
View File
@@ -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.',
+4
View File
@@ -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.',
+2
View File
@@ -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.',
+4
View File
@@ -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.',
+2
View File
@@ -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.',
+4
View File
@@ -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.',
+2
View File
@@ -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.',
+4
View File
@@ -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.',
+2
View File
@@ -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.',
+4
View File
@@ -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.',
+6
View File
@@ -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.',
+4
View File
@@ -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<CommandResponse<AutonomySettings>> {
+18
View File
@@ -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
+13
View File
@@ -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",
-5
View File
@@ -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<Arc<ApprovalManager>>` and pass
// it through here.
None,
&channel_name,
&multimodal,
max_tool_iterations,
@@ -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,
@@ -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,
@@ -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,
-3
View File
@@ -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,
+4 -43
View File
@@ -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<tokio::sync::mpsc::Sender<String>>,
@@ -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,
"<tool_result name=\"{}\">\n{denied}\n</tool_result>",
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
+123 -76
View File
@@ -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("<tool_call>{\"name\":\"echo\",\"arguments\":{}}</tool_call>".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<Box<dyn Tool>> = 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<std::sync::atomic::AtomicBool>,
}
#[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<ToolResult> {
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!(
"<tool_call>{{\"name\":\"{tool_name}\",\"arguments\":{{}}}}</tool_call>"
)),
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<Box<dyn Tool>> = 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)"
);
}
+74 -56
View File
@@ -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<HashMap<String, oneshot::Sender<ApprovalDecision>>>,
always_allowlist: Mutex<HashSet<String>>,
/// 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<String>) {
// 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<Option<PendingApproval>> {
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);
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();
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;
// 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"
);
}
}
+11 -13
View File
@@ -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;
-421
View File
@@ -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<String>,
/// Tools that always need approval, ignoring session allowlist.
always_ask: HashSet<String>,
/// Autonomy level from config.
autonomy_level: AutonomyLevel,
/// Session-scoped allowlist built from "Always" responses.
session_allowlist: Mutex<HashSet<String>>,
/// Audit trail of approval decisions.
audit_log: Mutex<Vec<ApprovalLogEntry>>,
}
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<ApprovalLogEntry> {
self.audit_log.lock().clone()
}
/// Get the current session allowlist.
pub fn session_allowlist(&self) -> HashSet<String> {
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<String> = 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");
}
}
+41 -7
View File
@@ -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))
}
+43 -1
View File
@@ -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<Vec<crate::openhuman::security::TrustedRoot>>,
pub allow_tool_install: Option<bool>,
pub max_actions_per_hour: Option<u32>,
/// "Always allow" allowlist — tool names the gate skips prompting for.
pub auto_approve: Option<Vec<String>>,
}
#[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<tokio::sync::Mutex<()>> = 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
+62
View File
@@ -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");
}
}
+4 -7
View File
@@ -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<String>,
#[serde(default = "default_always_ask")]
pub always_ask: Vec<String>,
/// 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<String> {
]
}
fn default_always_ask() -> Vec<String> {
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,
}
+10
View File
@@ -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<u64>,
/// Replaces the "Always allow" allowlist wholesale — tool names the agent
/// may run without an approval prompt. Empty list clears it.
auto_approve: Option<Vec<String>>,
}
pub fn all_controller_schemas() -> Vec<ControllerSchema> {
@@ -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<String, Value>) -> 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?)
})
+6
View File
@@ -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,
+13 -5
View File
@@ -182,6 +182,11 @@ pub struct SecurityPolicy {
pub trusted_roots: Vec<TrustedRoot>,
/// 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<String>,
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(),
}
}
+4
View File
@@ -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 -----------------------------------------------
+36
View File
@@ -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<String> = 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();
}