diff --git a/frontend/src/agents/ApiWriteAllowlist.test.tsx b/frontend/src/agents/ApiWriteAllowlist.test.tsx index 189e5f47..161bcb8b 100644 --- a/frontend/src/agents/ApiWriteAllowlist.test.tsx +++ b/frontend/src/agents/ApiWriteAllowlist.test.tsx @@ -11,10 +11,14 @@ vi.mock('react-i18next', () => ({ })); const getUserTools = vi.fn(); +const getAgent = vi.fn(); +const getWorkflow = vi.fn(); const updateAgent = vi.fn(); vi.mock('../api/services/userService', () => ({ default: { getUserTools: (...args: unknown[]) => getUserTools(...args), + getAgent: (...args: unknown[]) => getAgent(...args), + getWorkflow: (...args: unknown[]) => getWorkflow(...args), updateAgent: (...args: unknown[]) => updateAgent(...args), }, })); @@ -24,7 +28,7 @@ import actionToastReducer, { } from '../notifications/actionToastSlice'; import { prefSlice } from '../preferences/preferenceSlice'; import ApiWriteAllowlist from './ApiWriteAllowlist'; -import type { Agent } from './types'; +import type { Agent, ResourceState } from './types'; Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }); @@ -57,6 +61,58 @@ const TOOLS = { ], }; +// What the agent read says each tool may write on stored credentials: the +// owner's own tools, one an editor sponsored that the owner can't see, and +// one that stopped. +const state = (over: Partial): ResourceState => ({ + key: `tool:${over.id}`, + type: 'tool', + id: 'x', + name: 'Tool', + state: 'active', + reason: null, + owner_credential_writes: [], + ...over, +}); + +const STATES: ResourceState[] = [ + state({ + id: 'tg', + name: 'Telegram', + owner_credential_writes: ['telegram_send_message'], + }), + state({ id: 'memory', name: 'Memory' }), + state({ + id: 'crm', + name: 'CRM API', + owner_credential_writes: ['create_lead'], + }), + state({ + id: 'bob-jira', + name: 'Bob Jira', + runs_as: { user_id: 'bob', label: 'bob@example.com' }, + owner_credential_writes: ['create_issue'], + }), + state({ + id: 'gone', + name: 'Gone', + state: 'stopped', + reason: 'deleted', + owner_credential_writes: ['delete_all'], + }), +]; + +const readAgent = (tools: string[], extra: Partial = {}) => ({ + ok: true, + json: async () => ({ + id: 'agent-1', + agent_type: 'classic', + tools, + resource_states: STATES.filter((s) => tools.includes(s.id)), + ...extra, + }), +}); + const agent = (overrides: Partial = {}): Agent => ({ id: 'agent-1', @@ -79,6 +135,10 @@ describe('ApiWriteAllowlist', () => { beforeEach(() => { getUserTools.mockResolvedValue({ json: async () => TOOLS }); + getAgent + .mockReset() + .mockImplementation(async () => readAgent(currentTools)); + getWorkflow.mockReset(); updateAgent.mockReset(); container = document.createElement('div'); document.body.appendChild(container); @@ -90,11 +150,13 @@ describe('ApiWriteAllowlist', () => { container.remove(); }); + let currentTools: string[] = []; const render = async ( a: Agent, onConfigChange = vi.fn(), getSavedConfig?: () => Agent['config'], ) => { + currentTools = a.tools ?? []; store = makeStore(); await act(async () => { root.render( @@ -183,6 +245,39 @@ describe('ApiWriteAllowlist', () => { expect(labels).toEqual(['CRM API: Create lead']); }); + it("lists a sponsor's tool the owner can't see, but not a stopped one", async () => { + await render(agent({ tools: ['bob-jira', 'gone'] })); + const labels = Array.from(container.querySelectorAll('label')).map( + (l) => l.textContent, + ); + expect(labels).toEqual(['Bob Jira: Create issue']); + }); + + it("lists writes of a workflow agent's node tools", async () => { + getAgent.mockResolvedValue( + readAgent([], { agent_type: 'workflow', workflow: 'w1' }), + ); + getWorkflow.mockResolvedValue({ + ok: true, + json: async () => ({ + data: { + resource_states: [ + state({ + id: 'node-tool', + name: 'Node Slack', + owner_credential_writes: ['post_message'], + }), + ], + }, + }), + }); + await render(agent({ tools: [] })); + const labels = Array.from(container.querySelectorAll('label')).map( + (l) => l.textContent, + ); + expect(labels).toEqual(['Node Slack: Post message']); + }); + it('renders nothing for an agent without connected tools', async () => { await render(agent({ tools: ['memory'] })); expect(container.textContent).toBe(''); diff --git a/frontend/src/agents/ApiWriteAllowlist.tsx b/frontend/src/agents/ApiWriteAllowlist.tsx index 1e319643..bb03dae0 100644 --- a/frontend/src/agents/ApiWriteAllowlist.tsx +++ b/frontend/src/agents/ApiWriteAllowlist.tsx @@ -10,6 +10,7 @@ import { SectionHeader } from '../components/ui/section-header'; import { showActionToast } from '../notifications/actionToastSlice'; import { selectToken } from '../preferences/preferenceSlice'; import type { Agent, AgentConfig } from './types'; +import useAgentResourceStates from './useAgentResourceStates'; import { actionTitle } from '../connectors/i18n'; type WriteAction = { @@ -21,16 +22,7 @@ type WriteAction = { type UserTool = { id: string; - displayName?: string; - customName?: string; - connection_id?: string | null; - owner_credential_writes?: string[]; - actions?: { - name: string; - description?: string; - access?: string; - active?: boolean; - }[]; + actions?: { name: string; description?: string }[]; }; /** @@ -38,7 +30,9 @@ type UserTool = { * anyone reaching this agent through its API key, widget or its public link * may run. Nobody there can approve an action for the owner, so * the server refuses every other such write. The server names these writes - * per tool (`owner_credential_writes`). + * per running tool in the agent's `resource_states` (`owner_credential_writes`), + * which covers tools an editor sponsored and a workflow agent's node tools + * too; the owner's own tool list only adds action descriptions. * * A toggle saves at once, on top of the agent's last saved config * (`getSavedConfig`), so edits still pending in the form are not saved with @@ -56,47 +50,47 @@ export default function ApiWriteAllowlist({ const { t } = useTranslation(); const dispatch = useDispatch(); const token = useSelector(selectToken); - const [actions, setActions] = useState([]); + const [descriptions, setDescriptions] = useState>({}); const [allowed, setAllowed] = useState( agent.config?.api_write_allowlist ?? [], ); + const loaded = useAgentResourceStates(agent.id, (agent.tools ?? []).join()); useEffect(() => { setAllowed(agent.config?.api_write_allowlist ?? []); }, [agent.config?.api_write_allowlist]); + // Descriptions of the actions of tools the owner can open themselves. useEffect(() => { - if (!agent.id || !agent.tools?.length) { - setActions([]); - return; - } let cancelled = false; userService .getUserTools(token) .then((response: Response) => response.json()) .then((data: { tools?: UserTool[] }) => { if (cancelled) return; - const agentTools = new Set(agent.tools); - setActions( - (data.tools ?? []) - .filter((tool) => agentTools.has(tool.id)) - .flatMap((tool) => - (tool.owner_credential_writes ?? []).map((name) => ({ - entry: `${tool.id}:${name}`, - tool: tool.customName || tool.displayName || '', - action: name, - description: - tool.actions?.find((action) => action.name === name) - ?.description ?? '', - })), - ), - ); + const found: Record = {}; + for (const tool of data.tools ?? []) + for (const action of tool.actions ?? []) + if (action.description) + found[`${tool.id}:${action.name}`] = action.description; + setDescriptions(found); }) - .catch(() => !cancelled && setActions([])); + .catch(() => undefined); return () => { cancelled = true; }; - }, [agent.id, agent.tools, token]); + }, [token]); + + const actions: WriteAction[] = (loaded?.items ?? []) + .filter((item) => item.type === 'tool' && item.state === 'active') + .flatMap((item) => + (item.owner_credential_writes ?? []).map((name) => ({ + entry: `${item.id}:${name}`, + tool: item.name || t('agents.form.sponsors.unknownItem'), + action: name, + description: descriptions[`${item.id}:${name}`] ?? '', + })), + ); if (actions.length === 0) return null; diff --git a/frontend/src/agents/components/AgentUsesSection.test.tsx b/frontend/src/agents/components/AgentUsesSection.test.tsx index aa65cf07..4b25a689 100644 --- a/frontend/src/agents/components/AgentUsesSection.test.tsx +++ b/frontend/src/agents/components/AgentUsesSection.test.tsx @@ -136,7 +136,7 @@ describe('AgentUsesSection', () => { item({ id: 't2', name: 'Bobs', - sponsor: { user_id: 'bob', label: 'bob@example.com' }, + runs_as: { user_id: 'bob', label: 'bob@example.com' }, }), item({ id: 't3', @@ -166,6 +166,7 @@ describe('AgentUsesSection', () => { expect(rowOf('Bobs')?.textContent).toContain( `${K}.access.person(person=bob@example.com)`, ); + // API and widget callers run it as the owner: the owner's account. expect(rowOf('Slack tool')?.textContent).toContain( `${K}.access.member(service=Slack)`, ); @@ -173,7 +174,7 @@ describe('AgentUsesSection', () => { `${K}.access.yourAccount(service=Notion)`, ); expect(rowOf('Jira tool')?.textContent).toContain( - `${K}.access.personAccount(person=carol@example.com,service=Jira)`, + `${K}.access.personAccount(service=Jira,person=carol@example.com)`, ); expect(rowOf('Tone')?.textContent).toContain(`${K}.access.you`); }); @@ -186,7 +187,13 @@ describe('AgentUsesSection', () => { item({ id: 't2', name: 'Editors', - sponsor: { user_id: 'me', label: 'me@example.com' }, + runs_as: { user_id: 'me', label: 'me@example.com' }, + }), + item({ + id: 't3', + name: 'Slack tool', + credential_mode: 'member', + connection: { id: null, connector_key: 'slack', name: 'Slack' }, }), ], { access: 'editor', allowed_actions: ['edit', 'share', 'use'] }, @@ -195,6 +202,67 @@ describe('AgentUsesSection', () => { await open(); expect(rowOf('Owners')?.textContent).toContain(`${K}.access.owner`); expect(rowOf('Editors')?.textContent).toContain(`${K}.access.you`); + expect(rowOf('Slack tool')?.textContent).toContain( + `${K}.access.memberShared(service=Slack)`, + ); + }); + + it('follows who it runs as now, not a sponsor on record', async () => { + await render( + agentWith([ + item({ + id: 't1', + name: 'Once sponsored', + sponsor: { user_id: 'bob', label: 'bob@example.com' }, + runs_as: null, + }), + ]), + ); + await open(); + expect(rowOf('Once sponsored')?.textContent).toContain(`${K}.access.you`); + expect(rowOf('Once sponsored')?.textContent).not.toContain('bob'); + }); + + it('names whose saved credentials a tool without a connection uses', async () => { + await render( + agentWith([ + item({ + id: 't1', + name: 'My API', + account: { user_id: 'me', label: 'me@example.com' }, + }), + item({ + id: 't2', + name: 'Carols API', + account: { user_id: 'carol', label: 'carol@example.com' }, + }), + item({ + id: 't3', + name: 'Hidden API', + account: { user_id: null, label: null }, + }), + item({ + id: 't4', + name: 'Hidden Jira', + credential_mode: 'owner', + account: { user_id: null, label: null }, + connection: { id: null, connector_key: 'jira', name: 'Jira' }, + }), + ]), + ); + await open(); + expect(rowOf('My API')?.textContent).toContain( + `${K}.access.yourCredentials`, + ); + expect(rowOf('Carols API')?.textContent).toContain( + `${K}.access.personCredentials(person=carol@example.com)`, + ); + expect(rowOf('Hidden API')?.textContent).toContain( + `${K}.access.otherCredentials`, + ); + expect(rowOf('Hidden Jira')?.textContent).toContain( + `${K}.access.otherAccount(service=Jira)`, + ); }); it('opens by itself and marks a stopped item with why it stopped', async () => { @@ -234,11 +302,76 @@ describe('AgentUsesSection', () => { ), ); await open(); - expect(rowOf('Blocked')?.textContent).toContain(`${K}.writesOff`); - expect(rowOf('Allowed')?.textContent).not.toContain(`${K}.writesOff`); - expect(rowOf('Reads')?.textContent).not.toContain(`${K}.writesOff`); + // One of two writes still blocked: some, not all. + expect(rowOf('Blocked')?.textContent).toContain(`${K}.writesSomeOff`); + expect(rowOf('Allowed')?.textContent).not.toContain(`${K}.writes`); + expect(rowOf('Reads')?.textContent).not.toContain(`${K}.writes`); const alert = container.querySelector('[data-slot="alert"]'); expect(alert?.textContent).toContain(`${K}.writesNote`); + expect(alert?.textContent).not.toContain(`${K}.writesNoteMemberTail`); + }); + + it('marks a tool with every write blocked', async () => { + await render( + agentWith([ + item({ id: 't1', name: 'Blocked', owner_credential_writes: ['a'] }), + ]), + ); + await open(); + expect(rowOf('Blocked')?.textContent).toContain(`${K}.writesOff`); + expect(rowOf('Blocked')?.textContent).not.toContain(`${K}.writesSomeOff`); + }); + + it('names only API and widget users for tools each person connects', async () => { + await render( + agentWith([ + item({ + id: 't1', + name: 'Slack tool', + credential_mode: 'member', + connection: { id: null, connector_key: 'slack', name: 'Slack' }, + owner_credential_writes: ['send'], + }), + ]), + ); + await open(); + const alert = container.querySelector('[data-slot="alert"]'); + expect(alert?.textContent).toBe(`${K}.writesNoteApi`); + }); + + it('says public-link users use their own account on mixed tools', async () => { + await render( + agentWith([ + item({ id: 't1', name: 'Mine', owner_credential_writes: ['a'] }), + item({ + id: 't2', + name: 'Slack tool', + credential_mode: 'member', + owner_credential_writes: ['send'], + }), + ]), + ); + await open(); + const alert = container.querySelector('[data-slot="alert"]'); + expect(alert?.textContent).toBe( + `${K}.writesNote ${K}.writesNoteMemberTail`, + ); + }); + + it('says when an admin turned changes off', async () => { + await render( + agentWith([ + item({ + id: 't1', + name: 'GitHub tool', + writes_allowed: false, + owner_credential_writes: [], + }), + ]), + ); + await open(); + expect(rowOf('GitHub tool')?.textContent).toContain(`${K}.adminOff`); + expect(container.querySelector('[data-slot="alert"]')).toBeNull(); }); it('tells an editor the owner allows the writes', async () => { diff --git a/frontend/src/agents/components/AgentUsesSection.tsx b/frontend/src/agents/components/AgentUsesSection.tsx index 88fb832c..8a554767 100644 --- a/frontend/src/agents/components/AgentUsesSection.tsx +++ b/frontend/src/agents/components/AgentUsesSection.tsx @@ -5,9 +5,8 @@ import { TriangleAlert, Wrench, } from 'lucide-react'; -import { useEffect, useState } from 'react'; +import { useState } from 'react'; import { useTranslation } from 'react-i18next'; -import { useSelector } from 'react-redux'; import { Alert, AlertDescription } from '@/components/ui/alert'; import { Badge } from '@/components/ui/badge'; @@ -16,10 +15,9 @@ import { Card } from '@/components/ui/card'; import { ListRow, ListRows } from '@/components/ui/list-row'; import { cn } from '@/lib/utils'; -import userService from '../../api/services/userService'; -import { selectToken } from '../../preferences/preferenceSlice'; import { can, isOwner } from '../../utils/accessUtils'; -import type { Agent, ResourceState } from '../types'; +import type { ResourceState } from '../types'; +import useAgentResourceStates from '../useAgentResourceStates'; import { reasonKey } from './ResourceStatusNotice'; type AgentUsesSectionProps = { @@ -38,15 +36,6 @@ const TYPE_ICONS: Record = { const K = 'settings.teams.share.uses'; const plain = { interpolation: { escapeValue: false } }; -/** The agent's own items, then its workflow nodes' ones it doesn't have. */ -function mergeStates( - own: ResourceState[], - nodes: ResourceState[], -): ResourceState[] { - const seen = new Set(own.map((item) => item.key.toLowerCase())); - return [...own, ...nodes.filter((item) => !seen.has(item.key.toLowerCase()))]; -} - /** Write actions outside callers can't take: not in the API write allowlist. */ function blockedWrites(item: ResourceState, allowlist: string[]): string[] { const allowed = new Set(allowlist.map((entry) => entry.toLowerCase())); @@ -58,66 +47,32 @@ function blockedWrites(item: ResourceState, allowlist: string[]): string[] { /** * "What this agent uses" in the agent's share dialog: each attached tool, * source and prompt (and a workflow agent's node tools and sources), with - * whose access it runs with for the people the agent is shared with. + * whose access, account or saved credentials it runs with for the people + * the agent is shared with. * * Built from `resource_states` on the agent read (and the workflow read), * which only owners and editors get; the section is hidden from anyone else, - * and while it loads or when it fails. A stopped item says why. A tool whose - * writes act on stored credentials and aren't in the API write allowlist is - * marked, since API, widget and public-link users can't make those changes. + * and while it loads or when it fails. A stopped item says why. A tool with + * writes on stored credentials that aren't in the API write allowlist is + * marked (all or some of them), since API and widget users, and public-link + * users on the owner's accounts, can't make those changes; so is a tool an + * admin allows no changes through. */ export default function AgentUsesSection({ agentId, readerId, }: AgentUsesSectionProps) { const { t } = useTranslation(); - const token = useSelector(selectToken); - const [loaded, setLoaded] = useState<{ - agent: Agent; - items: ResourceState[]; - } | null>(null); - const [open, setOpen] = useState(false); - - useEffect(() => { - let cancelled = false; - setLoaded(null); - setOpen(false); - const load = async () => { - let agent: Agent; - try { - const response = await userService.getAgent(agentId, token); - if (!response.ok) return; - agent = await response.json(); - } catch { - return; - } - let items = agent.resource_states ?? []; - if (agent.agent_type === 'workflow' && agent.workflow) { - try { - const response = await userService.getWorkflow(agent.workflow, token); - if (response.ok) { - const body = await response.json(); - items = mergeStates(items, body?.data?.resource_states ?? []); - } - } catch { - // The agent's own items still show. - } - } - if (cancelled) return; - setLoaded({ agent, items }); - // Like Access settings: open when there is something to look at. - if (items.some((item) => item.state === 'stopped')) setOpen(true); - }; - void load(); - return () => { - cancelled = true; - }; - }, [agentId, token]); + const loaded = useAgentResourceStates(agentId); + // Like Access settings: open once by itself when something stopped, then + // follow the reader's clicks. + const [open, setOpen] = useState(null); if (!loaded || !can(loaded.agent, 'edit') || loaded.items.length === 0) return null; const { agent, items } = loaded; + const expanded = open ?? items.some((item) => item.state === 'stopped'); const ownerReads = isOwner(agent); // Without a user id (authentication off) the one local user is everyone. const isYou = (userId: string) => @@ -126,32 +81,43 @@ export default function AgentUsesSection({ const nameOf = (item: ResourceState) => item.name || t('agents.form.sponsors.unknownItem'); - const accessLabel = (item: ResourceState): string => { + // Whose account or saved credentials a tool acts with. + const credentialsLabel = (item: ResourceState): string | null => { const service = item.connection?.name; + const suffix = service ? '' : 'NoService'; + const named = service ? { ...plain, service } : plain; if (item.credential_mode === 'member') - return service - ? t(`${K}.access.member`, { ...plain, service }) - : t(`${K}.access.memberNoService`); - if (item.credential_mode === 'owner' && item.account) { - if (isYou(item.account.user_id)) - return service - ? t(`${K}.access.yourAccount`, { ...plain, service }) - : t(`${K}.access.yourAccountNoService`); - return service - ? t(`${K}.access.personAccount`, { - ...plain, - person: item.account.label, - service, - }) - : t(`${K}.access.personAccountNoService`, { - ...plain, - person: item.account.label, - }); - } - if (item.sponsor) - return isYou(item.sponsor.user_id) + // API and widget callers run the agent as its owner, so they use the + // owner's own account. + return t( + `${K}.access.member${ownerReads ? '' : 'Shared'}${suffix}`, + named, + ); + const account = item.account; + if (!account) return null; + // A connection names its service; saved credentials (an API key, an + // MCP sign-in) have none. + const connected = item.credential_mode === 'owner'; + const key = connected ? `Account${suffix}` : 'Credentials'; + const opts = connected ? named : plain; + if (!account.user_id) return t(`${K}.access.other${key}`, opts); + if (isYou(account.user_id)) return t(`${K}.access.your${key}`, opts); + return t(`${K}.access.person${key}`, { + ...opts, + person: account.label || account.user_id, + }); + }; + + const accessLabel = (item: ResourceState): string => { + const credentials = credentialsLabel(item); + if (credentials) return credentials; + if (item.runs_as) + return isYou(item.runs_as.user_id) ? t(`${K}.access.you`) - : t(`${K}.access.person`, { ...plain, person: item.sponsor.label }); + : t(`${K}.access.person`, { + ...plain, + person: item.runs_as.label || item.runs_as.user_id, + }); return ownerReads ? t(`${K}.access.you`) : t(`${K}.access.owner`); }; @@ -165,10 +131,46 @@ export default function AgentUsesSection({ person: item.sponsor?.label || item.sponsor?.user_id, }); - const anyBlocked = items.some( + const badgeFor = (item: ResourceState) => { + if (item.state === 'stopped') + return {t(`${K}.stopped`)}; + if (item.writes_allowed === false) + return {t(`${K}.adminOff`)}; + const blocked = blockedWrites(item, allowlist).length; + if (blocked === 0) return null; + const all = blocked === (item.owner_credential_writes ?? []).length; + return ( + + {t(all ? `${K}.writesOff` : `${K}.writesSomeOff`)} + + ); + }; + + // Public-link users act with their own account on a tool each person + // connects, so only API and widget users are held back there. + const blocked = items.filter( (item) => - item.state === 'active' && blockedWrites(item, allowlist).length > 0, + item.state === 'active' && + item.writes_allowed !== false && + blockedWrites(item, allowlist).length > 0, ); + const blockedMember = blocked.some( + (item) => item.credential_mode === 'member', + ); + const blockedOwned = blocked.some( + (item) => item.credential_mode !== 'member', + ); + const editor = ownerReads ? '' : 'Editor'; + const writesNote = blockedOwned + ? [ + t(`${K}.writesNote${editor}`), + blockedMember ? t(`${K}.writesNoteMemberTail`) : '', + ] + .filter(Boolean) + .join(' ') + : blockedMember + ? t(`${K}.writesNoteApi${editor}`) + : null; return (
@@ -177,32 +179,30 @@ export default function AgentUsesSection({ type="button" variant="link" size="sm" - aria-expanded={open} + aria-expanded={expanded} className="-ml-3 w-fit justify-start" - onClick={() => setOpen((value) => !value)} + onClick={() => setOpen(!expanded)} >