From 663c3013d1d150fa44fef29bfe5d8348bab3dbdc Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Sun, 4 Oct 2026 03:44:35 -0700 Subject: [PATCH] feat: bind each Remie approval to its tool call and run it once, show every argument and the resolved send on dashboard and Slack approval cards, keep always-allow to settings managers with a revocable list and never for tools that start sending or change access, withhold secrets and unpermitted tool results from the assistant and shared conversations, gate /mcp, /ai/tools and the web and playbook tools on AI_AGENT or use_ai, and scope get_mailbox by organization --- cmd/cli/specs.go | 3 +- docs/content/docs/api/agent-tools.mdx | 2 +- docs/content/docs/api/endpoints.mdx | 20 +- docs/content/docs/api/error-codes.mdx | 1 + docs/content/docs/api/mcp.mdx | 12 +- docs/content/docs/api/permissions.mdx | 2 + docs/content/docs/guides/ai-assistant.mdx | 19 +- .../content/docs/guides/connect-mcp-tools.mdx | 2 +- docs/content/docs/guides/slack.mdx | 10 +- docs/public/openapi.json | 16 +- internal/api/handler/ai_agent.go | 62 +++- internal/api/handler/mcp_server.go | 18 +- internal/api/handler/mcp_server_gate_test.go | 26 ++ internal/api/middleware/apikey.go | 29 ++ internal/api/middleware/key_holder_test.go | 35 ++ internal/api/routes.go | 31 +- internal/app/aiagent/service.go | 307 +++++++++++++++--- internal/app/aiagent/service_test.go | 102 ++++++ internal/app/aitools/registry.go | 54 ++- internal/app/aitools/tools_ai_gates_test.go | 108 ++++++ internal/app/aitools/tools_apikeys.go | 2 + internal/app/aitools/tools_automation.go | 1 + internal/app/aitools/tools_campaigns.go | 2 + internal/app/aitools/tools_inbox_send.go | 149 +++++++-- internal/app/aitools/tools_mailboxes.go | 4 +- internal/app/aitools/tools_segments.go | 4 +- internal/app/aitools/tools_skills.go | 7 +- internal/app/aitools/tools_team.go | 5 +- internal/app/aitools/tools_web.go | 13 +- internal/app/aitools/tools_webhooks.go | 6 +- internal/app/email/service.go | 2 +- internal/app/research/service.go | 2 +- internal/app/slackapp/agent.go | 119 ++++++- internal/app/slackapp/approval_test.go | 67 ++++ internal/app/slackapp/interactivity.go | 6 +- internal/models/agent.go | 23 +- internal/models/audit.go | 3 + internal/repository/pg_agent.go | 72 +++- .../_components/RemieToolPolicies.tsx | 73 +++++ web/src/app/app/settings/workspace/page.tsx | 4 +- .../components/app/agent/AgentActivity.tsx | 109 ++++++- web/src/components/app/agent/AgentPanel.tsx | 21 ++ web/src/components/app/agent/agentDemo.ts | 1 + web/src/hooks/useRealtimeEvents.ts | 2 + .../lib/api/client/app/agent/toolPolicies.ts | 19 ++ .../api/hooks/app/agent/useToolPolicies.ts | 20 ++ web/src/lib/api/models/app/agent/Agent.ts | 31 ++ web/src/stores/slices/agentSlice.ts | 7 + 48 files changed, 1467 insertions(+), 166 deletions(-) create mode 100644 internal/api/handler/mcp_server_gate_test.go create mode 100644 internal/api/middleware/key_holder_test.go create mode 100644 internal/app/aiagent/service_test.go create mode 100644 internal/app/aitools/tools_ai_gates_test.go create mode 100644 internal/app/slackapp/approval_test.go create mode 100644 web/src/app/app/settings/_components/RemieToolPolicies.tsx create mode 100644 web/src/lib/api/client/app/agent/toolPolicies.ts create mode 100644 web/src/lib/api/hooks/app/agent/useToolPolicies.ts diff --git a/cmd/cli/specs.go b/cmd/cli/specs.go index 050712646..c4ae0912a 100644 --- a/cmd/cli/specs.go +++ b/cmd/cli/specs.go @@ -2126,7 +2126,8 @@ func toolSpec() resource { Long: `The same tool registry the dashboard agent and MCP use, exposed as plain REST for function-calling agents that do not speak MCP. -Everything a tool can do is bounded by the signed-in key's scopes.`, +The signed-in key needs the AI_AGENT scope, and everything a tool can do +is bounded by its scopes.`, Endpoints: []endpoint{ { Name: "list", Aliases: []string{"ls"}, Short: "List the tools this key may call", diff --git a/docs/content/docs/api/agent-tools.mdx b/docs/content/docs/api/agent-tools.mdx index 713951f13..48b0bde79 100644 --- a/docs/content/docs/api/agent-tools.mdx +++ b/docs/content/docs/api/agent-tools.mdx @@ -9,7 +9,7 @@ The rules are identical to MCP: every tool is gated by its own permission bits, - **List**: `GET https://api.warmbly.com/v1/ai/tools` - **Execute**: `POST https://api.warmbly.com/v1/ai/tools/{name}/call` -- **Auth**: an API key or OAuth access token as a bearer header, or a dashboard JWT. For keys, each tool checks its own scope from the [permissions](/api/permissions/) table; for JWT members, the matching organization permission. +- **Auth**: an API key or OAuth access token as a bearer header, or a dashboard JWT. Both endpoints need the `AI_AGENT` scope for keys and tokens (and, for an OAuth token, a member with **Use AI**), or the **Use AI** permission for JWT members; without it they answer `403`. Each tool then checks its own scope from the [permissions](/api/permissions/) table; for JWT members, the matching organization permission. ## List the tools diff --git a/docs/content/docs/api/endpoints.mdx b/docs/content/docs/api/endpoints.mdx index 4ec70ba3a..17b87d300 100644 --- a/docs/content/docs/api/endpoints.mdx +++ b/docs/content/docs/api/endpoints.mdx @@ -638,10 +638,16 @@ Remie, the dashboard AI assistant, is JWT only: sessions are private to the memb | GET | `/ai/sessions/:id/messages` | organization member (use AI) | | POST | `/ai/sessions/:id/messages` | organization member (use AI, SSE) | | POST | `/ai/sessions/:id/approve` | organization member (use AI, SSE) | +| GET | `/ai/tool-policies` | `manage_settings` | +| DELETE | `/ai/tool-policies/:tool` | `manage_settings` | + +`POST /ai/sessions/:id/approve` takes `{"decision": "approve" | "deny" | "always_allow", "tool_call_id": "..."}`. `tool_call_id` is required and must be the call the session is waiting on (the `tool_call_id` of its `approval_required` event, or `pending.tool_call_id` from `GET /ai/sessions/:id/messages`); any other answers `409` `approval_not_pending`, and so does a second decision on the same call. `always_allow` saves a workspace policy only for a member with `manage_settings` and only for a tool that may be always allowed; otherwise it approves this one call. The `approval_required` event and `pending` carry `arguments` (every argument as indented JSON with sorted keys, capped at 16 KiB, with `arguments_truncated`), `preview` for a send (`from`, `to`, `subject`, `body`, `body_html`), and `always_allow_offered`. + +`GET /ai/tool-policies` lists the tools the assistant runs without asking (`tool_name`, `created_by`, `created_by_name`, `created_at`); `DELETE /ai/tool-policies/:tool` revokes one. Both audit as `ai_tool_policy`. ### AI skills -Org playbooks the AI features follow (see the [AI skills](/guides/ai-skills/) guide). Gated on `manage_settings` for JWT callers, or the `AI_AGENT` scope for API keys. +Org playbooks the AI features follow (see the [AI skills](/guides/ai-skills/) guide). Gated on `manage_settings` for JWT callers, or the `AI_AGENT` scope for API keys. Creating, editing or deleting one with an API key also needs the member who created the key to hold `manage_settings`. | Method | Path | Permission | |--------|------|------------| @@ -652,12 +658,12 @@ Org playbooks the AI features follow (see the [AI skills](/guides/ai-skills/) gu ### Agent tools (REST) -The AI tool registry over plain HTTP for function-calling agents that do not speak MCP (see [Agent tools](/api/agent-tools/)). Like the MCP endpoint, there is no route-level scope on purpose: each tool enforces its own permission, the list reflects only what the caller may use, and send-class tools are never exposed. +The AI tool registry over plain HTTP for function-calling agents that do not speak MCP (see [Agent tools](/api/agent-tools/)). Like the MCP endpoint, it needs `AI_AGENT` (or `use_ai` for JWT callers); each tool then enforces its own permission, the list reflects only what the caller may use, and send-class tools are never exposed. -| Method | Path | API Permission | -|--------|------|----------------| -| GET | `/ai/tools` | permission of each listed tool | -| POST | `/ai/tools/:name/call` | permission of the tool being called | +| Method | Path | Permission | +|--------|------|------------| +| GET | `/ai/tools` | `use_ai` / `AI_AGENT`, then the permission of each listed tool | +| POST | `/ai/tools/:name/call` | `use_ai` / `AI_AGENT`, then the permission of the tool being called | ### Connected MCP servers @@ -690,7 +696,7 @@ The two `/integrations/slack/link` routes that take a code do not need a workspa ## MCP server -`POST /v1/mcp` exposes the tool registry over the [Model Context Protocol](/api/mcp/) streamable-HTTP transport. It accepts an API key or an OAuth 2.1 access token (an unauthenticated request gets the RFC 9728 discovery challenge). Each tool is gated by its scope, `tools/list` reflects only what the credential allows, and send-class tools are never exposed. Per-key rate limits apply. +`POST /v1/mcp` exposes the tool registry over the [Model Context Protocol](/api/mcp/) streamable-HTTP transport. It accepts an API key or an OAuth 2.1 access token holding `AI_AGENT` (an unauthenticated request gets the RFC 9728 discovery challenge). Each tool is gated by its scope, `tools/list` reflects only what the credential allows, and send-class tools are never exposed. Per-key rate limits apply. ## Public diff --git a/docs/content/docs/api/error-codes.mdx b/docs/content/docs/api/error-codes.mdx index ca555814b..b1c1cc84a 100644 --- a/docs/content/docs/api/error-codes.mdx +++ b/docs/content/docs/api/error-codes.mdx @@ -590,6 +590,7 @@ A few conflicts carry their own `code`. The mailbox import's are under [mailbox | `lead_cc_has_copies` | 409 | [Set a lead's CC](/api/reference/campaigns/#set-a-leads-cc) names a contact that has copies of their own in the campaign | | `mailbox_is_seed` | 409 | Warmup was started or resumed on a placement seed inbox. See [placement test refusals](#placement-test-refusals) | | `mailbox_not_google_signin` | 409 | `POST /emails/onboarding/app-password/:id` on a mailbox that is not connected with per-mailbox Google sign-in. See [app password switch refusals](#app-password-switch-refusals) | +| `approval_not_pending` | 409 | `POST /ai/sessions/:id/approve` named a tool call the conversation is not waiting on: it was already decided, in another tab or in Slack, or a newer message replaced it | | `mailbox_cloud_unenroll_failed` | 409 | The mailbox is linked to [Warmbly Cloud](/guides/warmbly-cloud/) and its link could not be released, so `DELETE /emails/{id}` would leave Warmbly Cloud holding its credential or its claim on it. The mailbox record remains, and restoration onto its worker is attempted | `mailbox_cloud_unenroll_failed` is a self-hosted instance losing contact with Warmbly Cloud mid-delete. Deleting an enrolled mailbox has to revoke its enrollment before the record goes, because the pool holds the mailbox's own SMTP/IMAP credentials and that record is the only thing that knows the enrollment exists. The same call releases a cloud-managed mirror before the record goes, so the mailbox returns to the cloud workspace and can be adopted again instead of staying claimed by an instance that no longer keeps it. The mailbox record remains. Warmbly attempts to restore it onto its worker immediately, and the worker reconciler may restore it later if that attempt fails. Retry the delete once the instance can reach the cloud again. Unenrolling under **Settings > Warmbly Cloud** first does not help: it makes the same call. diff --git a/docs/content/docs/api/mcp.mdx b/docs/content/docs/api/mcp.mdx index 55f1632f2..6306bd33c 100644 --- a/docs/content/docs/api/mcp.mdx +++ b/docs/content/docs/api/mcp.mdx @@ -48,7 +48,7 @@ Authorization: Bearer wmbly_... ``` -Create a dedicated API key for each MCP client and grant it only the scopes it needs (see [Permissions](/api/permissions/)). The `AI_AGENT` scope is a convenient way to grant assistant-style access; combine it with the read/write scopes for the data you want the client to reach. +Create a dedicated API key for each MCP client and grant it only the scopes it needs (see [Permissions](/api/permissions/)). The key needs the `AI_AGENT` scope to use `/v1/mcp` at all; combine it with the read/write scopes for the data you want the client to reach. ### Claude Code @@ -91,7 +91,7 @@ Add to `.cursor/mcp.json`: ## What is exposed -The server exposes exactly the tools your credential is allowed to use, whether that credential is an OAuth token or an API key. A credential with only read scopes sees only the read tools; one with write scopes sees those too, so an MCP client acts with precisely the permissions you granted, and nothing more. +The server exposes exactly the tools your credential is allowed to use, whether that credential is an OAuth token or an API key. Every MCP call needs the `AI_AGENT` scope, and for an OAuth token, a member who still holds the **Use AI** permission; without it the server answers each request with a JSON-RPC error (`-32000`) and lists no tools. A credential with only read scopes sees only the read tools; one with write scopes sees those too, so an MCP client acts with precisely the permissions you granted, and nothing more. This is a subset of what the in-product assistant can do. Two categories are dashboard-only and never reachable by an API key or OAuth token: sending email (`send_reply`, `compose_email`), and workspace governance (team, workspace settings, and billing). Those routes are session-only and have no API scope to grant, so they never appear over MCP. @@ -157,12 +157,14 @@ Every tool is gated by its API permission. `tools/list` returns only the tools y | `list_api_keys` / `create_api_key` / `update_api_key` / `revoke_api_key` | Manage API keys | `API_KEYS` | | `list_webhooks` / `list_webhook_deliveries` | Read webhook endpoints and deliveries | `WEBHOOKS` | | `create_webhook` / `update_webhook` / `delete_webhook` / `rotate_webhook_secret` / `verify_webhook` | Manage webhook endpoints | `WEBHOOKS` | -| `search_web` / `fetch_url` | Search and read public pages | any | -| `load_skill` | Read one of your playbooks | any | +| `search_web` / `fetch_url` | Search and read public pages | `AI_AGENT` | +| `load_skill` | Read one of your playbooks | `AI_AGENT` | Two form surfaces are deliberately absent: brand-asset uploads (logo, cover, background) are multipart images with dimension and MIME limits that a tool call cannot express, and the custom forms domain is workspace-wide branding behind a DNS check, so it lives in settings rather than in a tool. -Rate limits apply to MCP calls exactly as they do to the rest of the API (per-key for API keys, per-user for OAuth tokens). `SEND_CAMPAIGNS` appears in the table because `set_campaign_status` needs it, but that tool is still withheld from MCP: no send-class tool is ever exposed over the MCP transport regardless of scope. +Rate limits apply to MCP calls exactly as they do to the rest of the API (per-key for API keys, per-user for OAuth tokens). `SEND_CAMPAIGNS` appears in the table because `set_campaign_status` and the placement tools need it. A self-registered MCP client is never granted it, so only an API key or app that holds the scope can start or stop a campaign over MCP. Tools that send a message to a recipient (`send_reply`, `compose_email`) are never exposed over the MCP transport regardless of scope. + +Tools that return a secret (`create_webhook` and `rotate_webhook_secret`) return it to the MCP client that called them, which is the caller asking for it. The in-product assistant never sees these values. ## How OAuth discovery works diff --git a/docs/content/docs/api/permissions.mdx b/docs/content/docs/api/permissions.mdx index 8689b0ec0..858e0a53c 100644 --- a/docs/content/docs/api/permissions.mdx +++ b/docs/content/docs/api/permissions.mdx @@ -36,6 +36,8 @@ These same permissions are the [OAuth](/api/oauth/) scopes, lowercased: `READ_EM | `AI_AGENT` | 22 | 4194304 | special | Run the AI assistant and MCP tools | | `AI_RESEARCH` | 23 | 8388608 | special | Run AI contact research | +`AI_AGENT` is the entry to the AI tool surfaces: [`/v1/mcp`](/api/mcp/) and [`/ai/tools`](/api/agent-tools/) refuse a key or OAuth token without it, and the web and playbook tools (`search_web`, `fetch_url`, `load_skill`) need it on top. It also gates the [AI skills](/api/endpoints/#ai-skills) endpoints for keys; creating, editing or deleting a playbook with a key further needs the member who created the key to hold `manage_settings`. + `SEND_CAMPAIGNS` is intentionally separate from `WRITE_CAMPAIGNS`: editing a campaign draft and starting one that actually transmits mail are different blast radii, so a key can be granted the first without the second. An [inbox placement test](/guides/placement-tests/) sends real mail from a real mailbox too, so starting or cancelling one, and scheduling a campaign's placement monitor, takes `SEND_CAMPAIGNS`, while reading results takes `READ_ANALYTICS`. ## Categories diff --git a/docs/content/docs/guides/ai-assistant.mdx b/docs/content/docs/guides/ai-assistant.mdx index 900843063..051a4e2a0 100644 --- a/docs/content/docs/guides/ai-assistant.mdx +++ b/docs/content/docs/guides/ai-assistant.mdx @@ -10,7 +10,7 @@ Open Remie from the blue blob in the top bar or `Cmd/Ctrl + I`. The panel follow The same assistant also answers in Slack, from a DM, a mention or `/warmbly`. See [Slack](/guides/slack/#the-assistant). -Remie acts as you: only what your role allows, only in your workspace, never another organization's data. Every change asks first, and **sending is never auto-approved**, even with "Always allow" on. +Remie acts as you: only what your role allows, only in your workspace, never another organization's data. Every change asks first. **Sending, starting things that send, and changing who has access are never auto-approved**, even with "Always allow" on. Remie needs a paid plan; a free trial has no credit allowance and the AI surfaces stay locked. Beyond that, access is the **Use AI** role permission (on for admins and managers, off for viewers); without it Remie and AI drafting are hidden. Admins can cap per-member spend by day, week, or month. See [AI credits](/guides/ai-credits/) and [Team and roles](/guides/team-roles/). @@ -19,6 +19,8 @@ Remie needs a paid plan; a free trial has no credit allowance and the AI surface Conversations are **private to you by default**. An admin can enable **Shared history** under Settings > Workspace > Remie, after which every member with Use AI sees, opens, continues, and deletes any conversation, attributed by owner. Turning it on exposes existing conversations, so decide deliberately. Delete individually from the history rail, or **Clear history** to remove every conversation you own. +In a teammate's conversation, the results of tools your own role cannot use are withheld, from you and from Remie when you continue it; you see that a step ran, not what it returned. Secrets are never kept in any conversation: an invitation token or a webhook signing secret is shown once, to the member who asked for it, in the dashboard step that produced it. Remie itself never sees them, and Slack never shows them. + Conversations run as tabs: open several with **+**, and a run keeps streaming in its own tab while you read another. Expanding gives a full workspace with searchable history grouped by day. Docked, the panel resizes by dragging its inner edge, which also takes the keyboard once focused: arrow keys nudge it (hold `Shift` for a bigger step), `Home` and `End` go to the narrowest and widest the window allows, and `Enter` or a double-click restores the default. On desktop Remie opens as a floating window you can drag by its header and resize from any edge; it remembers position and size. Dock it to either screen edge with the dock button or `Alt + P`, and pop it out again the same way or by dragging the header away. @@ -65,9 +67,20 @@ It knows what page you are on, so "summarize this campaign" and "draft a reply h ## Approvals -**Reading runs automatically.** **Changing always asks**, showing an approval card with a plain-language summary. You can **Approve** that one action, **Skip** it, or **Always allow** that kind of action workspace-wide from then on. +**Reading runs automatically.** **Changing always asks**, showing an approval card with the action and every argument Remie passed (expand **All arguments** to read them). You can **Approve** that one action or **Skip** it. A member with **Manage settings** can also choose **Always allow**, which lets that kind of action run without asking for everyone in the workspace from then on; other members approve one action at a time. -Sending is stricter and never auto-approves. The card shows the full message and sending mailbox, and only that exact message goes out. It refuses suppressed addresses. Starting a campaign and enabling an automation carry their own approvals, since both put mail in motion. +Some actions always ask, and **Always allow** is never offered for them: + +- sending a reply or a new email, and placement tests +- starting or stopping a campaign, enabling an automation, starting warmup, releasing a mailbox's send hold or a lead's hold, and adding leads to a campaign, since each puts mail in motion +- inviting a member, changing a member's role, and creating or editing an API key +- tools from [connected MCP servers](/guides/connect-mcp-tools/) + +A send's card shows the sending mailbox, the recipient, the subject, and the plain and HTML bodies, and only that exact message goes out, from that mailbox, to that recipient. If the conversation gets a newer message before you approve a reply, the reply is refused and Remie asks again. It refuses suppressed addresses. + +Each approval decides one specific action: approving a card that is no longer waiting (because it was already decided, in another tab or in Slack) does nothing. + +Settings > Workspace > Remie lists every action that is always allowed, who allowed it and when. A member with Manage settings can **Revoke** one, after which it asks again. **Drafts stay drafts.** Campaigns Remie builds are never started and automations are left disabled; you get a button to open them in the real editor. Copy it writes is grounded in your workspace [voice profile](/guides/unibox/) and follows the same human-writing rules as the rest of Warmbly. diff --git a/docs/content/docs/guides/connect-mcp-tools.mdx b/docs/content/docs/guides/connect-mcp-tools.mdx index 99f383db1..b88536032 100644 --- a/docs/content/docs/guides/connect-mcp-tools.mdx +++ b/docs/content/docs/guides/connect-mcp-tools.mdx @@ -25,7 +25,7 @@ If the server could not be reached, the connection shows the error instead of a ## How Remie uses external tools -External tools behave like Warmbly's own write actions with one important difference: they are **always** approval-gated. Remie can never run an external tool automatically, even if you have used "always allow" for built-in actions. Every external tool call shows you what it will do and waits for your approval. +External tools behave like Warmbly's own write actions with one important difference: they are **always** approval-gated. Remie can never run an external tool automatically, even when a settings manager has chosen "Always allow" for built-in actions. Every external tool call shows you what it will do and waits for your approval. Each tool appears in Remie namespaced by your connection, so it is always clear which server a tool belongs to. diff --git a/docs/content/docs/guides/slack.mdx b/docs/content/docs/guides/slack.mdx index 66caf6ecb..049b06eb1 100644 --- a/docs/content/docs/guides/slack.mdx +++ b/docs/content/docs/guides/slack.mdx @@ -63,16 +63,18 @@ It runs as your linked Warmbly account with the permissions your role has at tha ### Approvals -Reading runs straight away. A change shows an approval card in the thread with a plain-language summary and three buttons: +Reading runs straight away. A change shows an approval card in the thread with the action, every argument the assistant passed, and these buttons: - **Approve** runs that one action. - **Deny** skips it. -- **Always allow** approves it and every later action of that kind, as in the dashboard. +- **Always allow** approves it and every later action of that kind, as in the dashboard. It is offered only to a member with **Manage settings**, and never for the actions that [always ask](/guides/ai-assistant/#approvals). -Only the person whose conversation it is can press them. Anyone else in the channel sees the card but cannot act on it. +Only the person whose conversation it is can press them. Anyone else in the channel sees the card but cannot act on it. Each card decides the one action it shows: once it is decided, here or in the dashboard, its buttons do nothing. + +When the details do not fit in a Slack message, the card says so and carries **Open in Warmbly**, so you can read all of it before you decide. Secrets a step returns, such as a webhook signing secret, are never shown in Slack. -Anything that sends mail to a real recipient needs your approval every time, and **Always allow** is never offered for it. The card shows the full message and the sending mailbox, and only that exact message goes out. +Anything that sends mail to a real recipient needs your approval every time, and **Always allow** is never offered for it. The card shows the sending mailbox, the recipient, the subject and the message, and only that exact message goes out. ### Credits and history diff --git a/docs/public/openapi.json b/docs/public/openapi.json index feef32624..05c0af5a7 100644 --- a/docs/public/openapi.json +++ b/docs/public/openapi.json @@ -22643,7 +22643,7 @@ "get": { "operationId": "ai_tools_list", "summary": "List agent tools", - "description": "The AI tool registry filtered to what the caller's credentials allow. `format=openai` (aliases `hermes`, `functions`) returns OpenAI function-calling objects; the default returns `{name, description, input_schema}` per tool. Send-class tools are never listed.", + "description": "The AI tool registry filtered to what the caller's credentials allow. Needs `AI_AGENT` for keys and OAuth tokens, or `use_ai` for dashboard members. `format=openai` (aliases `hermes`, `functions`) returns OpenAI function-calling objects; the default returns `{name, description, input_schema}` per tool. Send-class tools are never listed.", "tags": [ "ai" ], @@ -22713,6 +22713,16 @@ } } }, + "403": { + "description": "The credentials lack AI_AGENT, or the member lacks use_ai.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, "429": { "description": "Rate limited.", "content": { @@ -22730,7 +22740,7 @@ "post": { "operationId": "ai_tools_call", "summary": "Execute an agent tool", - "description": "Runs one registry tool. The request body is the tool's JSON argument object (empty body = no arguments). Each tool enforces its own permission; send-class tools are never callable here.", + "description": "Runs one registry tool. The request body is the tool's JSON argument object (empty body = no arguments). Needs `AI_AGENT` for keys and OAuth tokens, or `use_ai` for dashboard members, and each tool enforces its own permission; send-class tools are never callable here.", "tags": [ "ai" ], @@ -22816,7 +22826,7 @@ } }, "403": { - "description": "The credentials lack the permission for this tool.", + "description": "The credentials lack AI_AGENT (or use_ai), or the permission for this tool.", "content": { "application/json": { "schema": { diff --git a/internal/api/handler/ai_agent.go b/internal/api/handler/ai_agent.go index 387178ccc..b26873b02 100644 --- a/internal/api/handler/ai_agent.go +++ b/internal/api/handler/ai_agent.go @@ -18,6 +18,7 @@ import ( "github.com/warmbly/warmbly/internal/app/aiagent" "github.com/warmbly/warmbly/internal/app/aitools" "github.com/warmbly/warmbly/internal/errx" + "github.com/warmbly/warmbly/internal/models" "github.com/warmbly/warmbly/internal/utils/paging" ) @@ -36,10 +37,14 @@ func (h *Handler) jwtInvocation(c *gin.Context) (aitools.Invocation, *errx.Error if xerr != nil || member == nil { return aitools.Invocation{}, errx.New(errx.Forbidden, "not a member of this organization") } + perms := member.Permissions + if member.IsOwner() { + perms = models.AllPermissions + } return aitools.Invocation{ OrgID: *orgID, UserID: userID, - OrgPerms: member.Permissions, + OrgPerms: perms, IsAPIKey: false, IP: c.ClientIP(), UserAgent: c.Request.UserAgent(), @@ -131,7 +136,7 @@ func (h *Handler) AgentSessionMessages(c *gin.Context) { errx.JSON(c, errx.New(errx.NotFound, "session not found")) return } - turns, terr := h.AIAgentService.Transcript(c.Request.Context(), inv.OrgID, inv.UserID, sessionID) + turns, pending, terr := h.AIAgentService.Transcript(c.Request.Context(), inv, sessionID) if terr != nil { errx.JSON(c, terr) return @@ -139,7 +144,7 @@ func (h *Handler) AgentSessionMessages(c *gin.Context) { c.JSON(http.StatusOK, gin.H{ "title": sess.Title, "turns": turns, - "pending": sess.Context.Pending, + "pending": pending, "free_model": sess.Context.FreeModel, }) } @@ -241,7 +246,8 @@ func (h *Handler) AgentApprove(c *gin.Context) { return } var req struct { - Decision string `json:"decision"` // approve | deny | always_allow + Decision string `json:"decision"` // approve | deny | always_allow + ToolCallID string `json:"tool_call_id"` } if err := c.ShouldBindJSON(&req); err != nil { errx.JSON(c, errx.InvalidBody(err)) @@ -253,13 +259,54 @@ func (h *Handler) AgentApprove(c *gin.Context) { errx.JSON(c, errx.New(errx.BadRequest, "decision must be approve, deny, or always_allow")) return } + if req.ToolCallID == "" { + errx.JSON(c, errx.New(errx.BadRequest, "tool_call_id is required")) + return + } emit := sseEmitter(c) - if serr := h.AIAgentService.Resume(c.Request.Context(), inv, sessionID, req.Decision, emit); serr != nil { + if serr := h.AIAgentService.Resume(c.Request.Context(), inv, sessionID, req.ToolCallID, req.Decision, emit); serr != nil { emit(aiagent.StreamEvent{Type: "error", Code: string(codeIdentifier(serr)), Message: serr.Message}) } } +// ListAIToolPolicies — GET /ai/tool-policies lists the tools the assistant runs without asking. +func (h *Handler) ListAIToolPolicies(c *gin.Context) { + if h.AIAgentService == nil { + errx.JSON(c, errx.New(errx.ServiceUnavailable, "the AI assistant is not configured")) + return + } + inv, xerr := h.jwtInvocation(c) + if xerr != nil { + errx.JSON(c, xerr) + return + } + policies, lerr := h.AIAgentService.ListToolPolicies(c.Request.Context(), inv.OrgID) + if lerr != nil { + errx.JSON(c, lerr) + return + } + c.JSON(http.StatusOK, gin.H{"data": policies}) +} + +// RevokeAIToolPolicy — DELETE /ai/tool-policies/:tool makes the tool ask again. +func (h *Handler) RevokeAIToolPolicy(c *gin.Context) { + if h.AIAgentService == nil { + errx.JSON(c, errx.New(errx.ServiceUnavailable, "the AI assistant is not configured")) + return + } + inv, xerr := h.jwtInvocation(c) + if xerr != nil { + errx.JSON(c, xerr) + return + } + if rerr := h.AIAgentService.RevokeToolPolicy(c.Request.Context(), inv, c.Param("tool")); rerr != nil { + errx.JSON(c, rerr) + return + } + c.JSON(http.StatusOK, gin.H{"message": "policy revoked"}) +} + // sseEmitter prepares the response for Server-Sent Events and returns a // flush-per-event emitter. Safe to call once per request. func sseEmitter(c *gin.Context) func(aiagent.StreamEvent) { @@ -297,6 +344,11 @@ func codeIdentifier(e *errx.Error) string { return "forbidden" case errx.ServiceUnavailable: return "service_unavailable" + case errx.Conflict: + if e.Identifier != "" { + return e.Identifier + } + return "conflict" default: return "error" } diff --git a/internal/api/handler/mcp_server.go b/internal/api/handler/mcp_server.go index 2702144f0..d419684c3 100644 --- a/internal/api/handler/mcp_server.go +++ b/internal/api/handler/mcp_server.go @@ -1,7 +1,8 @@ // Warmbly MCP server (server direction). Exposes the shared tool registry over // the MCP streamable-HTTP transport at POST /api/v1/mcp, authenticated by an API -// key. Each tool is gated by its RequiredAPIPerm bits, tools/list reflects only -// what the key's permission mask allows, and send-class tools are never exposed. +// key or OAuth token holding AI_AGENT. Each tool is gated by its RequiredAPIPerm +// bits, tools/list reflects only what the key's permission mask allows, and +// send-class tools are never exposed. package handler import ( @@ -15,6 +16,7 @@ import ( "github.com/warmbly/warmbly/internal/api/middleware" "github.com/warmbly/warmbly/internal/app/aitools" "github.com/warmbly/warmbly/internal/errx" + "github.com/warmbly/warmbly/internal/models" "github.com/warmbly/warmbly/internal/pkg/generation" ) @@ -65,6 +67,10 @@ func (h *Handler) MCPEndpoint(c *gin.Context) { inv.UserID = uid } bindOAuthMember(c, &inv) + if !mcpAllowed(inv) { + c.JSON(http.StatusOK, rpcError(req.ID, -32000, "this credential lacks the AI_AGENT permission, which MCP needs")) + return + } switch req.Method { case "initialize": @@ -84,6 +90,14 @@ func (h *Handler) MCPEndpoint(c *gin.Context) { } } +// mcpAllowed is the AI_AGENT gate: the key's scope, and for an OAuth token its member's use_ai. +func mcpAllowed(inv aitools.Invocation) bool { + if !models.HasAPIPermission(inv.APIPerms, models.APIPermAIAgent) { + return false + } + return !inv.ActsForMember || inv.OrgPerms.HasPermission(models.PermUseAI) +} + // mcpToolList reflects only the static tools the key's mask allows, excluding // send-class tools (never exposed over MCP). func (h *Handler) mcpToolList(inv aitools.Invocation) []gin.H { diff --git a/internal/api/handler/mcp_server_gate_test.go b/internal/api/handler/mcp_server_gate_test.go new file mode 100644 index 000000000..23b79d5a1 --- /dev/null +++ b/internal/api/handler/mcp_server_gate_test.go @@ -0,0 +1,26 @@ +package handler + +import ( + "testing" + + "github.com/warmbly/warmbly/internal/app/aitools" + "github.com/warmbly/warmbly/internal/models" +) + +func TestMCPNeedsAIAgent(t *testing.T) { + cases := []struct { + name string + inv aitools.Invocation + want bool + }{ + {"key without AI_AGENT", aitools.Invocation{IsAPIKey: true, APIPerms: models.APIPermReadContacts}, false}, + {"key with AI_AGENT", aitools.Invocation{IsAPIKey: true, APIPerms: models.APIPermAIAgent}, true}, + {"oauth member without use_ai", aitools.Invocation{IsAPIKey: true, ActsForMember: true, APIPerms: models.APIPermAIAgent, OrgPerms: models.PermViewContacts}, false}, + {"oauth member with use_ai", aitools.Invocation{IsAPIKey: true, ActsForMember: true, APIPerms: models.APIPermAIAgent, OrgPerms: models.PermUseAI}, true}, + } + for _, tc := range cases { + if got := mcpAllowed(tc.inv); got != tc.want { + t.Errorf("%s: mcpAllowed = %v, want %v", tc.name, got, tc.want) + } + } +} diff --git a/internal/api/middleware/apikey.go b/internal/api/middleware/apikey.go index 355e2b198..7e54015f2 100644 --- a/internal/api/middleware/apikey.go +++ b/internal/api/middleware/apikey.go @@ -352,6 +352,35 @@ func (h *Handler) RequireAccess(orgPerm models.OrganizationPermission, apiPerm u } } +// RequireKeyHolder passes an API key only while the member who created it holds perm; other callers pass. +func (h *Handler) RequireKeyHolder(perm models.OrganizationPermission) gin.HandlerFunc { + return func(c *gin.Context) { + if c.GetString(AuthTypeKey) != AuthTypeAPIKey { + c.Next() + return + } + userID, err := GetUserUUID(c) + orgID := GetOrganizationID(c) + if err != nil || orgID == nil || h.OrganizationService == nil { + errx.JSON(c, errx.ErrForbidden) + c.Abort() + return + } + has, xerr := h.memberHasPermission(c, *orgID, userID, perm) + if xerr != nil { + errx.JSON(c, xerr) + c.Abort() + return + } + if !has { + errx.JSON(c, errx.New(errx.Forbidden, "the member who created this API key does not have this permission")) + c.Abort() + return + } + c.Next() + } +} + // orNotPermitted is the lookup failure when there is one, and the member's missing permission otherwise. func orNotPermitted(xerr *errx.Error) *errx.Error { if xerr != nil { diff --git a/internal/api/middleware/key_holder_test.go b/internal/api/middleware/key_holder_test.go new file mode 100644 index 000000000..f0b134412 --- /dev/null +++ b/internal/api/middleware/key_holder_test.go @@ -0,0 +1,35 @@ +package middleware + +import ( + "context" + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" + "github.com/google/uuid" + + "github.com/warmbly/warmbly/internal/models" +) + +// RequireKeyHolder leaves sessions and OAuth to the role gate and refuses a key it cannot check. +func TestRequireKeyHolderFailsClosed(t *testing.T) { + h := &Handler{} + for _, tc := range []struct { + authType string + want int + }{{AuthTypeJWT, http.StatusOK}, {AuthTypeOAuth, http.StatusOK}, {AuthTypeAPIKey, http.StatusForbidden}} { + r := gin.New() + r.Use(func(c *gin.Context) { + c.Set(AuthTypeKey, tc.authType) + c.Set(UserIDKey, uuid.NewString()) + c.Next() + }) + r.GET("/x", h.RequireKeyHolder(models.PermManageSettings), func(c *gin.Context) { c.Status(http.StatusOK) }) + w := httptest.NewRecorder() + r.ServeHTTP(w, httptest.NewRequestWithContext(context.Background(), http.MethodGet, "/x", nil)) + if w.Code != tc.want { + t.Errorf("%s: status = %d, want %d", tc.authType, w.Code, tc.want) + } + } +} diff --git a/internal/api/routes.go b/internal/api/routes.go index 673ddc0d0..c6cb193ee 100644 --- a/internal/api/routes.go +++ b/internal/api/routes.go @@ -489,9 +489,10 @@ func Run( // Warmbly MCP server: exposes the shared tool registry over the MCP // streamable-HTTP transport. Accepts an API key (static header) or an OAuth // 2.1 access token (one-command `claude mcp add` + browser sign-in); an - // unauthenticated request gets the RFC 9728 discovery challenge. Each tool is - // gated by its RequiredAPIPerm, send-class tools are never exposed, and - // per-key rate limits apply. + // unauthenticated request gets the RFC 9728 discovery challenge. The caller + // needs AI_AGENT (MCPEndpoint checks it), each tool is gated by its + // RequiredAPIPerm, send-class tools are never exposed, and per-key rate + // limits apply. mcpServer := base.Group("/mcp") mcpServer.Use(m.MCPAuthMiddleware(), m.APIKeyUsageMiddleware(), m.RateLimitMiddleware(models.RateLimitWrite)) mcpServer.POST("", h.MCPEndpoint) @@ -776,23 +777,25 @@ func Run( } // AI skills (org playbooks). CRUD gated on manage_settings (JWT) or - // the AI_AGENT scope (API key); every mutation audits (ai_skill). + // the AI_AGENT scope (API key); a key's writes also need its creator + // to hold manage_settings. Every mutation audits (ai_skill). skillsGroup := protected.Group("/ai/skills") skillsGroup.Use(m.RequireOrganization()) { + skillWrite := m.RequireKeyHolder(models.PermManageSettings) skillsGroup.GET("", m.RequireAccess(models.PermManageSettings, models.APIPermAIAgent), h.ListSkills) - skillsGroup.POST("", m.RequireAccess(models.PermManageSettings, models.APIPermAIAgent), h.CreateSkill) - skillsGroup.PATCH("/:id", m.RequireAccess(models.PermManageSettings, models.APIPermAIAgent), h.UpdateSkill) - skillsGroup.DELETE("/:id", m.RequireAccess(models.PermManageSettings, models.APIPermAIAgent), h.DeleteSkill) + skillsGroup.POST("", m.RequireAccess(models.PermManageSettings, models.APIPermAIAgent), skillWrite, h.CreateSkill) + skillsGroup.PATCH("/:id", m.RequireAccess(models.PermManageSettings, models.APIPermAIAgent), skillWrite, h.UpdateSkill) + skillsGroup.DELETE("/:id", m.RequireAccess(models.PermManageSettings, models.APIPermAIAgent), skillWrite, h.DeleteSkill) } // REST tool surface for non-MCP agents (Hermes/OpenAI-style function - // calling). No route-level permission gate on purpose, matching the - // MCP endpoint: the registry enforces each - // tool's own permission bits, the list reflects only what the caller - // may use, and send-class tools are never exposed. + // calling). Gated on use_ai (JWT) or AI_AGENT (key), matching the MCP + // endpoint; the registry then enforces each tool's own permission + // bits, the list reflects only what the caller may use, and + // send-class tools are never exposed. agentTools := protected.Group("/ai/tools") - agentTools.Use(m.RequireOrganization()) + agentTools.Use(m.RequireOrganization(), m.RequireAccess(models.PermUseAI, models.APIPermAIAgent)) { agentTools.GET("", m.RateLimitMiddleware(models.RateLimitRead), h.ListAgentTools) agentTools.POST("/:name/call", m.RateLimitMiddleware(models.RateLimitWrite), h.CallAgentTool) @@ -1577,6 +1580,10 @@ func Run( ai.POST("/sessions/:id/messages", useAI, h.AgentMessage) ai.POST("/sessions/:id/approve", useAI, h.AgentApprove) + // Workspace "always allow" tool policies, listed and revoked by settings managers. + ai.GET("/tool-policies", m.RequirePermission(models.PermManageSettings), h.ListAIToolPolicies) + ai.DELETE("/tool-policies/:tool", m.RequirePermission(models.PermManageSettings), h.RevokeAIToolPolicy) + // Connected MCP servers (external tools). Admin-only; sealing // credentials and exposing external tools is a settings action. ai.GET("/connections", m.RequirePermission(models.PermManageSettings), h.ListMCPServers) diff --git a/internal/app/aiagent/service.go b/internal/app/aiagent/service.go index 18f800418..9221e2b3e 100644 --- a/internal/app/aiagent/service.go +++ b/internal/app/aiagent/service.go @@ -10,9 +10,11 @@ import ( "errors" "fmt" "log" + "sort" "strconv" "strings" "time" + "unicode/utf8" "github.com/google/uuid" "github.com/jackc/pgx/v5" @@ -102,6 +104,13 @@ type StreamEvent struct { OpenURL string `json:"open_url,omitempty"` // Args is a tool_start's full arguments for in-process renderers; never sent to a client. Args json.RawMessage `json:"-"` + // Approval card: every argument (sorted, capped), the resolved send, and whether "always allow" is offered. + Arguments string `json:"arguments,omitempty"` + ArgumentsTruncated bool `json:"arguments_truncated,omitempty"` + Preview *models.AgentSendPreview `json:"preview,omitempty"` + AlwaysAllowOffered bool `json:"always_allow_offered,omitempty"` + // Secrets are a tool_secret's values, shown once to the member and never to the model or the transcript. + Secrets map[string]string `json:"secrets,omitempty"` } // toolResultEvent builds the tool_result SSE step, extracting a draft artifact @@ -130,6 +139,7 @@ const ( evTool = "tool_start" evToolDone = "tool_result" evApproval = "approval_required" + evSecret = "tool_secret" evError = "error" evDone = "done" ) @@ -146,18 +156,26 @@ type Service interface { // Transcript returns the session's persisted history hydrated into the same // turn/block shape the live stream renders, so a reopened tab looks identical - // to a fresh run. - Transcript(ctx context.Context, orgID, userID, sessionID uuid.UUID) ([]HydratedTurn, *errx.Error) + // to a fresh run, plus the approval it is waiting on as the reader sees it. + Transcript(ctx context.Context, inv aitools.Invocation, sessionID uuid.UUID) ([]HydratedTurn, *models.PendingAgentTool, *errx.Error) // RunMessage streams a new user message's run. inv carries the caller's // identity + org permission bits. emit is called for each SSE step. RunMessage(ctx context.Context, inv aitools.Invocation, sessionID uuid.UUID, messageID, text, page, resource string, emit func(StreamEvent)) *errx.Error // Resume continues a paused run after the user's decision - // (approve | deny | always_allow). - Resume(ctx context.Context, inv aitools.Invocation, sessionID uuid.UUID, decision string, emit func(StreamEvent)) *errx.Error + // (approve | deny | always_allow) on the tool call named by toolCallID. + Resume(ctx context.Context, inv aitools.Invocation, sessionID uuid.UUID, toolCallID, decision string, emit func(StreamEvent)) *errx.Error + + // ListToolPolicies lists the workspace's "always allow" tools that take effect. + ListToolPolicies(ctx context.Context, orgID uuid.UUID) ([]models.AIToolPolicy, *errx.Error) + // RevokeToolPolicy removes one, so the tool asks again. + RevokeToolPolicy(ctx context.Context, inv aitools.Invocation, toolName string) *errx.Error } +// errApprovalNotPending answers a decision on a tool call the session is not waiting on. +var errApprovalNotPending = errx.NewWithIdentifier(errx.Conflict, "approval_not_pending", "This action is no longer waiting for approval.") + type service struct { repo repository.AgentRepository registry *aitools.Registry @@ -265,12 +283,52 @@ type HydratedTurn struct { Blocks []HydratedBlock `json:"blocks"` } -func (s *service) Transcript(ctx context.Context, orgID, userID, sessionID uuid.UUID) ([]HydratedTurn, *errx.Error) { - msgs, xerr := s.loadTranscript(ctx, orgID, s.sessionScope(ctx, orgID, userID, sessionID), sessionID) - if xerr != nil { - return nil, xerr +func (s *service) Transcript(ctx context.Context, inv aitools.Invocation, sessionID uuid.UUID) ([]HydratedTurn, *models.PendingAgentTool, *errx.Error) { + owner := s.sessionScope(ctx, inv.OrgID, inv.UserID, sessionID) + sess, err := s.repo.GetSession(ctx, inv.OrgID, owner, sessionID) + if err != nil || sess == nil { + return nil, nil, errx.New(errx.NotFound, "session not found") } - return hydrateTranscript(msgs), nil + msgs, xerr := s.loadTranscript(ctx, inv.OrgID, owner, sessionID) + if xerr != nil { + return nil, nil, xerr + } + if owner != inv.UserID { + msgs = s.forReader(ctx, inv, msgs) + } + pending := sess.Context.Pending + if pending != nil { + p := *pending + if p.Arguments == "" { + p.Arguments, p.ArgumentsTruncated = canonicalArgs(p.Args) + } + p.AlwaysAllowOffered = s.alwaysAllowOffered(inv, p.ToolName, p.Risk) + pending = &p + } + return hydrateTranscript(msgs), pending, nil +} + +// withheldResult replaces a shared conversation's tool result for a member whose role cannot run that tool. +const withheldResult = `{"withheld":"This result came from a tool your role cannot use."}` + +// forReader withholds tool results from a member reading someone else's conversation when they may not run that tool. +func (s *service) forReader(ctx context.Context, inv aitools.Invocation, msgs []generation.AgentMessage) []generation.AgentMessage { + allowed := map[string]bool{} + for _, d := range s.registry.ToolDefs(ctx, inv) { + allowed[d.Name] = true + } + calls := map[string]string{} + out := make([]generation.AgentMessage, len(msgs)) + for i, m := range msgs { + for _, tc := range m.ToolCalls { + calls[tc.ID] = tc.Name + } + if m.Role == "tool" && !allowed[calls[m.ToolCallID]] { + m.Content = withheldResult + } + out[i] = m + } + return out } // hydrateTranscript folds the stored provider-agnostic messages into the @@ -345,6 +403,9 @@ func (s *service) RunMessage(ctx context.Context, inv aitools.Invocation, sessio if xerr != nil { return xerr } + if owner != inv.UserID { + genMsgs = s.forReader(ctx, inv, genMsgs) + } userMsg := generation.AgentMessage{Role: "user", Content: text} if perr := s.persist(ctx, inv.OrgID, owner, sessionID, []generation.AgentMessage{userMsg}, 0); perr != nil { return errx.New(errx.Internal, "failed to save message") @@ -362,26 +423,43 @@ func (s *service) RunMessage(ctx context.Context, inv aitools.Invocation, sessio return s.runLoop(ctx, inv, sess, genMsgs, len(genMsgs), messageID, emit) } -func (s *service) Resume(ctx context.Context, inv aitools.Invocation, sessionID uuid.UUID, decision string, emit func(StreamEvent)) *errx.Error { +func (s *service) Resume(ctx context.Context, inv aitools.Invocation, sessionID uuid.UUID, toolCallID, decision string, emit func(StreamEvent)) *errx.Error { owner := s.sessionScope(ctx, inv.OrgID, inv.UserID, sessionID) sess, err := s.repo.GetSession(ctx, inv.OrgID, owner, sessionID) if err != nil || sess == nil { return errx.New(errx.NotFound, "session not found") } - pending := sess.Context.Pending - if pending == nil { - return errx.New(errx.BadRequest, "no tool awaiting approval") + if strings.TrimSpace(toolCallID) == "" { + return errx.New(errx.BadRequest, "tool_call_id is required") } genMsgs, xerr := s.loadTranscript(ctx, inv.OrgID, owner, sessionID) if xerr != nil { return xerr } + if owner != inv.UserID { + genMsgs = s.forReader(ctx, inv, genMsgs) + } baseline := len(genMsgs) - // Always-allow persists an org policy for write tools (never send). - if decision == "always_allow" && pending.Risk == string(generation.RiskWrite) { - _ = s.repo.SetToolPolicy(ctx, inv.OrgID, pending.ToolName, "always_allow", inv.UserID) + // The decision binds to one tool call, and only the caller that clears it runs it. + pending, cerr := s.repo.ClaimPendingTool(ctx, inv.OrgID, sessionID, toolCallID) + if cerr != nil { + return errx.New(errx.Internal, "failed to resume") + } + if pending == nil { + return errApprovalNotPending + } + sess.Context.Pending = nil + + // A workspace policy is set only by a member who manages settings, and never for a tool that always asks. + if decision == "always_allow" { + if s.alwaysAllowOffered(inv, pending.ToolName, pending.Risk) { + if perr := s.repo.SetToolPolicy(ctx, inv.OrgID, pending.ToolName, "always_allow", inv.UserID); perr == nil && s.audit != nil { + s.audit.LogAction(ctx, inv.OrgID, inv.UserID, models.AuditActionCreate, models.AuditEntityAIToolPolicy, nil, inv.IP, inv.UserAgent, nil, map[string]string{"tool": pending.ToolName}) + } + } + decision = "approve" } assistant := generation.AgentMessage{Role: "assistant", ToolCalls: []generation.ToolCall{{ @@ -396,9 +474,9 @@ func (s *service) Resume(ctx context.Context, inv aitools.Invocation, sessionID // Resolve the pending tool from the invocation's full tool set (static // registry tools PLUS dynamic per-org tools like connected MCP servers), // which registry.Call does not cover. - out, cerr := s.executeTool(ctx, inv, pending.ToolName, pending.Args) - if cerr != nil { - b, _ := json.Marshal(map[string]string{"error": cerr.Error()}) + out, eerr := s.executeTool(ctx, inv, pending.ToolName, pending.Args, emit) + if eerr != nil { + b, _ := json.Marshal(map[string]string{"error": eerr.Error()}) out = string(b) } toolResult = out @@ -410,14 +488,10 @@ func (s *service) Resume(ctx context.Context, inv aitools.Invocation, sessionID generation.AgentMessage{Role: "tool", ToolCallID: pending.ToolCallID, Content: toolResult}, ) - // Clear the pending marker before continuing. - sess.Context.Pending = nil - _ = s.repo.UpdateSessionContext(ctx, inv.OrgID, inv.UserID, sessionID, sess.Context) - // Give the resumed segment its own credit-idempotency namespace. Reusing the // original message id would collide with the pre-pause iterations (which // already consumed messageID:1..N), so the resumed loop's iterations 1..N - // would replay for free. Pending is cleared above, so a resume is single- + // would replay for free. Pending was claimed above, so a resume is single- // shot and a fresh id is safe. return s.runLoop(ctx, inv, sess, genMsgs, baseline, "resume:"+uuid.NewString(), emit) } @@ -452,7 +526,7 @@ func (s *service) runLoop(ctx context.Context, inv aitools.Invocation, sess *mod req := generation.AgentRequest{ System: s.systemPrompt(sess, voiceBlock, skillsBlock), Messages: genMsgs, - Tools: s.registry.ToolDefs(ctx, inv), + Tools: s.boundTools(ctx, inv, emit), Model: model, MaxIterations: DefaultRunBudget, OnEvent: func(ev generation.AgentEvent) { @@ -495,23 +569,31 @@ func (s *service) runLoop(ctx context.Context, inv aitools.Invocation, sess *mod return nil }, Approve: func(ctx context.Context, tool generation.ToolDef, call generation.ToolCall) error { - // Send-class is always per-action (never auto-allowed). External MCP - // tools are also never auto-allowed. Write-class auto-runs only when - // an org policy says always_allow. - if tool.Risk == generation.RiskWrite && policies[tool.Name] == "always_allow" && !strings.HasPrefix(tool.Name, "mcp_") { + if s.autoAllowed(policies, tool) { return nil } - sess.Context.Pending = &models.PendingAgentTool{ - MessageID: messageID, - ToolCallID: call.ID, - ToolName: call.Name, - Risk: string(tool.Risk), - Args: call.Args, - ArgsSummary: summarizeArgs(call.Args), + p := &models.PendingAgentTool{ + MessageID: messageID, + ToolCallID: call.ID, + ToolName: call.Name, + Risk: string(tool.Risk), + Args: call.Args, } + // A send shows its resolved sender and recipients, and runs with exactly those. + if preview, pinned, perr := s.registry.PreviewSend(ctx, inv, call.Name, call.Args); perr == nil && preview != nil { + p.Preview = preview + if len(pinned) > 0 { + p.Args = pinned + } + } + p.ArgsSummary = summarizeArgs(p.Args) + p.Arguments, p.ArgumentsTruncated = canonicalArgs(p.Args) + p.AlwaysAllowOffered = s.alwaysAllowOffered(inv, call.Name, string(tool.Risk)) + sess.Context.Pending = p emit(StreamEvent{ - Type: evApproval, Tool: call.Name, Risk: string(tool.Risk), - ArgsSummary: summarizeArgs(call.Args), ToolCallID: call.ID, + Type: evApproval, Tool: call.Name, Risk: p.Risk, ToolCallID: call.ID, + ArgsSummary: p.ArgsSummary, Arguments: p.Arguments, ArgumentsTruncated: p.ArgumentsTruncated, + Preview: p.Preview, AlwaysAllowOffered: p.AlwaysAllowOffered, }) return generation.ErrApprovalRequired }, @@ -530,6 +612,8 @@ func (s *service) runLoop(ctx context.Context, inv aitools.Invocation, sess *mod lastRemaining = bal } } + sess.Context.Pending = nil + _ = s.repo.ClearPendingTool(ctx, sess.OrgID, sess.ID) emit(StreamEvent{Type: evError, Code: "provider_error", Message: "Remie hit an error. Please try again.", CreditsRemaining: lastRemaining}) return nil } @@ -578,8 +662,8 @@ func (s *service) emitStop(emit func(StreamEvent), reason string, remaining int) // executeTool runs an approved tool by name, resolving it from the invocation's // full tool set (static + dynamic per-org tools). This is how a resumed run // executes a paused write/MCP tool, since registry.Call only knows static tools. -func (s *service) executeTool(ctx context.Context, inv aitools.Invocation, name string, args json.RawMessage) (string, error) { - for _, d := range s.registry.ToolDefs(ctx, inv) { +func (s *service) executeTool(ctx context.Context, inv aitools.Invocation, name string, args json.RawMessage, emit func(StreamEvent)) (string, error) { + for _, d := range s.boundTools(ctx, inv, emit) { if d.Name == name { return d.Handler(ctx, args) } @@ -587,6 +671,107 @@ func (s *service) executeTool(ctx context.Context, inv aitools.Invocation, name return "", aitools.ErrToolNotFound } +// boundTools is the invocation's tool set, with secret-bearing results withheld from the model. +func (s *service) boundTools(ctx context.Context, inv aitools.Invocation, emit func(StreamEvent)) []generation.ToolDef { + defs := s.registry.ToolDefs(ctx, inv) + for i := range defs { + t, ok := s.registry.Get(defs[i].Name) + if !ok || len(t.SecretFields) == 0 { + continue + } + run, name, fields := defs[i].Handler, defs[i].Name, t.SecretFields + defs[i].Handler = func(ctx context.Context, args json.RawMessage) (string, error) { + out, err := run(ctx, args) + if err != nil { + return out, err + } + redacted, secrets := withholdSecrets(out, fields) + if len(secrets) > 0 { + emit(StreamEvent{Type: evSecret, Tool: name, Secrets: secrets}) + } + return redacted, nil + } + } + return defs +} + +// secretPlaceholder is what the model and the stored transcript hold in place of a secret. +const secretPlaceholder = "[withheld: shown once to the member in the dashboard]" + +// withholdSecrets replaces the named top-level result fields with a placeholder and returns their values. +func withholdSecrets(out string, fields []string) (string, map[string]string) { + var m map[string]any + if json.Unmarshal([]byte(out), &m) != nil { + return out, nil + } + secrets := map[string]string{} + for _, f := range fields { + v, ok := m[f].(string) + if !ok || v == "" { + continue + } + secrets[f] = v + m[f] = secretPlaceholder + } + if len(secrets) == 0 { + return out, nil + } + b, err := json.Marshal(m) + if err != nil { + return `{"withheld":"` + secretPlaceholder + `"}`, secrets + } + return string(b), secrets +} + +// autoAllowed reports whether a workspace policy lets this write tool run without asking. +func (s *service) autoAllowed(policies map[string]string, tool generation.ToolDef) bool { + if tool.Risk != generation.RiskWrite || policies[tool.Name] != "always_allow" || strings.HasPrefix(tool.Name, "mcp_") { + return false + } + t, ok := s.registry.Get(tool.Name) + return ok && !t.AlwaysAsk +} + +// alwaysAllowOffered reports whether inv may turn this tool into a workspace "always allow". +func (s *service) alwaysAllowOffered(inv aitools.Invocation, toolName, risk string) bool { + if inv.IsAPIKey || risk != string(generation.RiskWrite) || strings.HasPrefix(toolName, "mcp_") { + return false + } + if !inv.OrgPerms.HasPermission(models.PermManageSettings) { + return false + } + t, ok := s.registry.Get(toolName) + return ok && t.Risk == generation.RiskWrite && !t.AlwaysAsk +} + +func (s *service) ListToolPolicies(ctx context.Context, orgID uuid.UUID) ([]models.AIToolPolicy, *errx.Error) { + rows, err := s.repo.ListToolPolicies(ctx, orgID) + if err != nil { + return nil, errx.New(errx.Internal, "failed to list tool policies") + } + out := make([]models.AIToolPolicy, 0, len(rows)) + for _, p := range rows { + if s.autoAllowed(map[string]string{p.ToolName: p.Decision}, generation.ToolDef{Name: p.ToolName, Risk: generation.RiskWrite}) { + out = append(out, p) + } + } + return out, nil +} + +func (s *service) RevokeToolPolicy(ctx context.Context, inv aitools.Invocation, toolName string) *errx.Error { + removed, err := s.repo.DeleteToolPolicy(ctx, inv.OrgID, toolName) + if err != nil { + return errx.New(errx.Internal, "failed to revoke tool policy") + } + if !removed { + return errx.ErrNotFound + } + if s.audit != nil { + s.audit.LogAction(ctx, inv.OrgID, inv.UserID, models.AuditActionDelete, models.AuditEntityAIToolPolicy, nil, inv.IP, inv.UserAgent, nil, map[string]string{"tool": toolName}) + } + return nil +} + // --- helpers --- func (s *service) loadTranscript(ctx context.Context, orgID, userID, sessionID uuid.UUID) ([]generation.AgentMessage, *errx.Error) { @@ -690,9 +875,14 @@ func summarizeArgs(args json.RawMessage) string { if err := json.Unmarshal(args, &m); err != nil { return "" } - parts := make([]string, 0, len(m)) - for k, v := range m { - parts = append(parts, fmt.Sprintf("%s=%v", k, v)) + keys := make([]string, 0, len(m)) + for k := range m { + keys = append(keys, k) + } + sort.Strings(keys) + parts := make([]string, 0, 4) + for _, k := range keys { + parts = append(parts, fmt.Sprintf("%s=%v", k, m[k])) if len(parts) >= 4 { break } @@ -700,6 +890,34 @@ func summarizeArgs(args json.RawMessage) string { return truncateRunesTitle(strings.Join(parts, ", "), 160) } +// canonicalArgs renders every argument as indented JSON with sorted keys, capped at AgentApprovalArgsMax bytes. +func canonicalArgs(args json.RawMessage) (string, bool) { + if len(args) == 0 { + return "", false + } + var v any + if err := json.Unmarshal(args, &v); err != nil { + return truncateBytes(string(args), models.AgentApprovalArgsMax) + } + b, err := json.MarshalIndent(v, "", " ") + if err != nil { + return truncateBytes(string(args), models.AgentApprovalArgsMax) + } + return truncateBytes(string(b), models.AgentApprovalArgsMax) +} + +// truncateBytes cuts s to at most n bytes on a rune boundary and reports whether it cut. +func truncateBytes(s string, n int) (string, bool) { + if len(s) <= n { + return s, false + } + cut := n + for cut > 0 && !utf8.RuneStart(s[cut]) { + cut-- + } + return s[:cut], true +} + // summarize renders a short human line for a tool result step row. func summarize(tool, result string) string { var m map[string]any @@ -710,6 +928,9 @@ func summarize(tool, result string) string { if e, ok := m["error"]; ok { return fmt.Sprintf("error: %v", e) } + if _, ok := m["withheld"]; ok { + return "result withheld" + } if id, ok := m["campaign_id"]; ok { return fmt.Sprintf("created campaign %v", id) } diff --git a/internal/app/aiagent/service_test.go b/internal/app/aiagent/service_test.go new file mode 100644 index 000000000..97e525c6e --- /dev/null +++ b/internal/app/aiagent/service_test.go @@ -0,0 +1,102 @@ +package aiagent + +import ( + "context" + "encoding/json" + "strings" + "testing" + "unicode/utf8" + + "github.com/google/uuid" + + "github.com/warmbly/warmbly/internal/app/aitools" + "github.com/warmbly/warmbly/internal/models" + "github.com/warmbly/warmbly/internal/pkg/generation" +) + +func testRegistry() *aitools.Registry { + r := aitools.NewRegistry() + noop := func(context.Context, aitools.Invocation, json.RawMessage) (string, error) { return `{}`, nil } + r.Register(aitools.Tool{Name: "add_tag", Risk: generation.RiskWrite, RequiredOrgPerm: models.PermManageContacts, Handler: noop}) + r.Register(aitools.Tool{Name: "set_campaign_status", Risk: generation.RiskWrite, AlwaysAsk: true, RequiredOrgPerm: models.PermSendCampaigns, Handler: noop}) + r.Register(aitools.Tool{Name: "send_reply", Risk: generation.RiskSend, RequiredOrgPerm: models.PermAccessUnibox, Handler: noop}) + r.Register(aitools.Tool{Name: "list_contacts", Risk: generation.RiskRead, RequiredOrgPerm: models.PermViewContacts, Handler: noop}) + return r +} + +func TestCanonicalArgsSortedAndCapped(t *testing.T) { + got, cut := canonicalArgs(json.RawMessage(`{"b":1,"a":{"d":2,"c":3}}`)) + if cut { + t.Fatal("small args were cut") + } + if !(strings.Index(got, `"a"`) < strings.Index(got, `"b"`) && strings.Index(got, `"c"`) < strings.Index(got, `"d"`)) { + t.Fatalf("keys not sorted: %s", got) + } + big := `{"body":"` + strings.Repeat("é", models.AgentApprovalArgsMax) + `"}` + got, cut = canonicalArgs(json.RawMessage(big)) + if !cut || len(got) > models.AgentApprovalArgsMax { + t.Fatalf("large args: cut=%v len=%d", cut, len(got)) + } + if !utf8.ValidString(got) { + t.Fatalf("cut inside a rune: %q", got[len(got)-8:]) + } +} + +func TestWithholdSecrets(t *testing.T) { + out, secrets := withholdSecrets(`{"ok":true,"secret":"whsec_abc","webhook_id":"w"}`, []string{"secret"}) + if secrets["secret"] != "whsec_abc" { + t.Fatalf("secret not returned: %v", secrets) + } + if strings.Contains(out, "whsec_abc") || !strings.Contains(out, `"webhook_id":"w"`) { + t.Fatalf("model-facing result = %s", out) + } + if out, secrets := withholdSecrets(`{"error":"nope"}`, []string{"secret"}); secrets != nil || out != `{"error":"nope"}` { + t.Fatalf("a result without the field changed: %s %v", out, secrets) + } +} + +func TestAlwaysAllowNeedsSettingsAndANonSendingTool(t *testing.T) { + s := &service{registry: testRegistry()} + admin := aitools.Invocation{OrgID: uuid.New(), OrgPerms: models.PermManageSettings | models.PermManageContacts} + member := aitools.Invocation{OrgID: uuid.New(), OrgPerms: models.PermManageContacts} + if !s.alwaysAllowOffered(admin, "add_tag", "write") { + t.Error("a settings manager is not offered always allow on a write tool") + } + if s.alwaysAllowOffered(member, "add_tag", "write") { + t.Error("a member without manage_settings is offered always allow") + } + for _, name := range []string{"set_campaign_status", "send_reply", "mcp_x_y", "unknown"} { + if s.alwaysAllowOffered(admin, name, "write") { + t.Errorf("%s is offered always allow", name) + } + } + policies := map[string]string{"add_tag": "always_allow", "set_campaign_status": "always_allow", "mcp_x_y": "always_allow"} + if !s.autoAllowed(policies, generation.ToolDef{Name: "add_tag", Risk: generation.RiskWrite}) { + t.Error("a policy for a write tool does not apply") + } + for _, name := range []string{"set_campaign_status", "mcp_x_y"} { + if s.autoAllowed(policies, generation.ToolDef{Name: name, Risk: generation.RiskWrite}) { + t.Errorf("a policy auto-runs %s", name) + } + } +} + +func TestForReaderWithholdsResultsOfToolsTheReaderCannotRun(t *testing.T) { + s := &service{registry: testRegistry()} + reader := aitools.Invocation{OrgID: uuid.New(), OrgPerms: models.PermViewContacts} + msgs := []generation.AgentMessage{ + {Role: "assistant", ToolCalls: []generation.ToolCall{{ID: "1", Name: "list_contacts"}, {ID: "2", Name: "send_reply"}}}, + {Role: "tool", ToolCallID: "1", Content: `{"count":1}`}, + {Role: "tool", ToolCallID: "2", Content: `{"to":"someone@example.com"}`}, + } + out := s.forReader(context.Background(), reader, msgs) + if out[1].Content != `{"count":1}` { + t.Errorf("a permitted result was withheld: %s", out[1].Content) + } + if out[2].Content != withheldResult { + t.Errorf("an unpermitted result was kept: %s", out[2].Content) + } + if msgs[2].Content == withheldResult { + t.Error("the stored messages were modified") + } +} diff --git a/internal/app/aitools/registry.go b/internal/app/aitools/registry.go index 44171bdd0..88c01e00a 100644 --- a/internal/app/aitools/registry.go +++ b/internal/app/aitools/registry.go @@ -54,6 +54,12 @@ type Tool struct { // routes have no API-key permission bit, so a zero RequiredAPIPerm must NOT // be read as "open to keys". The dashboard agent (JWT) still gets them. JWTOnly bool + // AlwaysAsk keeps the tool out of workspace "always allow" policies: it starts sending, or changes who has access. + AlwaysAsk bool + // SecretFields are top-level result keys the assistant never sees; the member is shown them once. + SecretFields []string + // Preview resolves a send's sender, recipients and subject for the approval card, and pins them in the returned args. + Preview func(ctx context.Context, inv Invocation, args json.RawMessage) (*models.AgentSendPreview, json.RawMessage, error) Handler Handler } @@ -153,9 +159,29 @@ func (r *Registry) ToolDefs(ctx context.Context, inv Invocation) []generation.To // ToolDefsByName returns bound ToolDefs for only the named tools the invocation // is permitted to use (unknown or unpermitted names are skipped). Feature agents -// (e.g. contact research) use this to pull a specific subset like search_web + -// fetch_url without exposing the whole registry. +// (e.g. the advisor fixer) use this to pull a specific subset without exposing +// the whole registry. func (r *Registry) ToolDefsByName(inv Invocation, names ...string) []generation.ToolDef { + return r.bindNamed(inv, true, names...) +} + +// researchTools are the tools a feature agent may run under its own route's gate: public web lookups and playbooks. +var researchTools = map[string]bool{"search_web": true, "fetch_url": true, "load_skill": true} + +// ResearchTools binds search_web, fetch_url and load_skill for a feature whose +// own route or schedule already authorized the caller (contact research, AI +// variables at send time). Any other name is ignored. +func (r *Registry) ResearchTools(inv Invocation, names ...string) []generation.ToolDef { + keep := make([]string, 0, len(names)) + for _, n := range names { + if researchTools[n] { + keep = append(keep, n) + } + } + return r.bindNamed(inv, false, keep...) +} + +func (r *Registry) bindNamed(inv Invocation, gate bool, names ...string) []generation.ToolDef { want := make(map[string]bool, len(names)) for _, n := range names { want[n] = true @@ -166,7 +192,7 @@ func (r *Registry) ToolDefsByName(inv Invocation, names ...string) []generation. continue } t := r.tools[name] - if !t.allowed(inv) { + if gate && !t.allowed(inv) { continue } defs = append(defs, generation.ToolDef{ @@ -183,12 +209,26 @@ func (r *Registry) ToolDefsByName(inv Invocation, names ...string) []generation. } // WebResearchTools returns fresh read-only web tools (search_web, fetch_url) -// bound to an org, for feature agents that only need public-web lookups (e.g. -// research-mode campaign AI variables at send time). Read-only web tools require -// no org permission, so a bare org-scoped invocation suffices. Returned defs are +// bound to an org, for research-mode campaign AI variables at send time, which +// the campaign's own permissions already authorized. Returned defs are // unbudgeted; the caller wraps them with its own per-run budget. func (r *Registry) WebResearchTools(orgID uuid.UUID) []generation.ToolDef { - return r.ToolDefsByName(Invocation{OrgID: orgID}, "search_web", "fetch_url") + return r.ResearchTools(Invocation{OrgID: orgID}, "search_web", "fetch_url") +} + +// PreviewSend resolves a gated call's send preview and pinned args; nil when the tool has no preview. +func (r *Registry) PreviewSend(ctx context.Context, inv Invocation, name string, args json.RawMessage) (*models.AgentSendPreview, json.RawMessage, error) { + t, ok := r.tools[name] + if !ok || t.Preview == nil || !t.allowed(inv) { + return nil, nil, nil + } + return t.Preview(ctx, inv, args) +} + +// Permits reports whether inv may run the named static tool; unknown names are not permitted. +func (r *Registry) Permits(inv Invocation, name string) bool { + t, ok := r.tools[name] + return ok && t.allowed(inv) } // Call invokes a single tool by name under inv, enforcing its permission gate. diff --git a/internal/app/aitools/tools_ai_gates_test.go b/internal/app/aitools/tools_ai_gates_test.go new file mode 100644 index 000000000..01a6504d5 --- /dev/null +++ b/internal/app/aitools/tools_ai_gates_test.go @@ -0,0 +1,108 @@ +package aitools + +import ( + "context" + "testing" + + "github.com/google/uuid" + + "github.com/warmbly/warmbly/internal/app/apikey" + "github.com/warmbly/warmbly/internal/app/email" + "github.com/warmbly/warmbly/internal/app/organization" + "github.com/warmbly/warmbly/internal/app/webhook" + "github.com/warmbly/warmbly/internal/models" +) + +type stubSkillLookup struct{ SkillLookup } +type stubEmails struct{ email.EmailService } +type stubOrg struct { + organization.OrganizationService +} +type stubAPIKeys struct{ apikey.APIKeyService } +type stubWebhooks struct{ webhook.Service } + +func aiGateRegistry() *Registry { + return BuildRegistry(Deps{ + Skills: stubSkillLookup{}, Emails: stubEmails{}, Org: stubOrg{}, APIKeys: stubAPIKeys{}, Webhooks: stubWebhooks{}, + Segments: stubSegments{}, Forms: stubForms{}, Suppressions: stubSuppressions{}, + }) +} + +// The web and playbook tools act for the assistant, so they need the assistant's own permission. +func TestResearchToolsRequireAIAccess(t *testing.T) { + r := aiGateRegistry() + org := uuid.New() + for _, name := range []string{"search_web", "fetch_url", "load_skill"} { + if r.Permits(Invocation{OrgID: org, OrgPerms: models.PermViewContacts}, name) { + t.Errorf("%s: a member without use_ai may run it", name) + } + if !r.Permits(Invocation{OrgID: org, OrgPerms: models.PermUseAI}, name) { + t.Errorf("%s: a member with use_ai may not run it", name) + } + if r.Permits(Invocation{OrgID: org, IsAPIKey: true, APIPerms: models.APIPermReadContacts}, name) { + t.Errorf("%s: a key without AI_AGENT may run it", name) + } + if !r.Permits(Invocation{OrgID: org, IsAPIKey: true, APIPerms: models.APIPermAIAgent}, name) { + t.Errorf("%s: a key with AI_AGENT may not run it", name) + } + } +} + +// Feature agents bind the research tools under their own gate, and nothing else. +func TestResearchToolsBindOnlyResearch(t *testing.T) { + r := aiGateRegistry() + defs := r.ResearchTools(Invocation{OrgID: uuid.New()}, "search_web", "fetch_url", "load_skill", "list_contacts", "set_campaign_status") + got := map[string]bool{} + for _, d := range defs { + got[d.Name] = true + } + if len(got) != 3 || !got["search_web"] || !got["fetch_url"] || !got["load_skill"] { + t.Fatalf("ResearchTools bound %v", got) + } + if n := len(r.WebResearchTools(uuid.New())); n != 2 { + t.Fatalf("WebResearchTools bound %d tools, want 2", n) + } +} + +// Starting sending and changing who has access always ask, whatever a workspace policy says. +func TestAlwaysAskTools(t *testing.T) { + r := aiGateRegistry() + for _, name := range []string{ + "set_campaign_status", "set_automation_enabled", "set_mailbox_warmup", "set_mailbox_send_hold", + "set_lead_hold", "set_campaign_segments", "add_segment_to_campaign", + "invite_member", "update_member_role", "create_api_key", "update_api_key", + } { + tool, ok := r.Get(name) + if !ok { + t.Errorf("%s is not registered", name) + continue + } + if !tool.AlwaysAsk { + t.Errorf("%s can be always-allowed", name) + } + } +} + +// Tools that return a secret declare it, so the assistant never holds it. +func TestSecretFieldsDeclared(t *testing.T) { + r := aiGateRegistry() + for name, field := range map[string]string{"get_invitation_link": "token", "create_webhook": "secret", "rotate_webhook_secret": "secret"} { + tool, ok := r.Get(name) + if !ok { + t.Errorf("%s is not registered", name) + continue + } + if len(tool.SecretFields) != 1 || tool.SecretFields[0] != field { + t.Errorf("%s secret fields = %v, want [%s]", name, tool.SecretFields, field) + } + } +} + +// A preview that is not permitted to the caller resolves nothing. +func TestPreviewSendNeedsPermission(t *testing.T) { + r := aiGateRegistry() + p, pinned, err := r.PreviewSend(context.Background(), Invocation{OrgID: uuid.New()}, "send_reply", []byte(`{"thread_id":"x","body":"hi"}`)) + if p != nil || pinned != nil || err != nil { + t.Fatalf("preview for an unpermitted caller = %v %s %v", p, pinned, err) + } +} diff --git a/internal/app/aitools/tools_apikeys.go b/internal/app/aitools/tools_apikeys.go index 3706cf8f7..beab6ae20 100644 --- a/internal/app/aitools/tools_apikeys.go +++ b/internal/app/aitools/tools_apikeys.go @@ -38,6 +38,7 @@ func (d Deps) registerAPIKeyTools(r *Registry) { "preset": enumProp("Scope preset.", "read_only", "full_access"), }, "name"), Risk: generation.RiskWrite, + AlwaysAsk: true, RequiredOrgPerm: models.PermManageAPIKeys, RequiredAPIPerm: models.APIPermAPIKeys, Handler: d.createAPIKey, @@ -52,6 +53,7 @@ func (d Deps) registerAPIKeyTools(r *Registry) { "description": strProp("New description."), }, "key_id"), Risk: generation.RiskWrite, + AlwaysAsk: true, RequiredOrgPerm: models.PermManageAPIKeys, RequiredAPIPerm: models.APIPermAPIKeys, Handler: d.updateAPIKey, diff --git a/internal/app/aitools/tools_automation.go b/internal/app/aitools/tools_automation.go index 5ee5427db..b06041e77 100644 --- a/internal/app/aitools/tools_automation.go +++ b/internal/app/aitools/tools_automation.go @@ -74,6 +74,7 @@ func (d Deps) registerAutomationTools(r *Registry) { "enabled": boolProp("true to enable, false to disable."), }, "automation_id", "enabled"), Risk: generation.RiskWrite, + AlwaysAsk: true, RequiredOrgPerm: models.PermManageSettings, RequiredAPIPerm: models.APIPermIntegrations, Handler: d.setAutomationEnabled, diff --git a/internal/app/aitools/tools_campaigns.go b/internal/app/aitools/tools_campaigns.go index bf87c2aa7..293868d30 100644 --- a/internal/app/aitools/tools_campaigns.go +++ b/internal/app/aitools/tools_campaigns.go @@ -62,6 +62,7 @@ func (d Deps) registerCampaignTools(r *Registry) { "action": enumProp("start to activate/resume, stop to pause.", "start", "stop"), }, "campaign_id", "action"), Risk: generation.RiskWrite, + AlwaysAsk: true, RequiredOrgPerm: models.PermSendCampaigns, RequiredAPIPerm: models.APIPermSendCampaigns, Handler: d.setCampaignStatus, @@ -78,6 +79,7 @@ func (d Deps) registerCampaignTools(r *Registry) { "reason": strProp("For pause: a short note shown next to the hold."), }, "campaign_id", "contact_id", "action"), Risk: generation.RiskWrite, + AlwaysAsk: true, RequiredOrgPerm: models.PermManageCampaigns, RequiredAPIPerm: models.APIPermWriteCampaigns, Handler: d.setLeadHold, diff --git a/internal/app/aitools/tools_inbox_send.go b/internal/app/aitools/tools_inbox_send.go index 10afcb415..adea6aa2e 100644 --- a/internal/app/aitools/tools_inbox_send.go +++ b/internal/app/aitools/tools_inbox_send.go @@ -3,6 +3,7 @@ package aitools import ( "context" "encoding/json" + "errors" "strings" "github.com/google/uuid" @@ -31,6 +32,7 @@ func (d Deps) registerInboxSendTools(r *Registry) { Risk: generation.RiskSend, RequiredOrgPerm: models.PermAccessUnibox, RequiredAPIPerm: models.APIPermWriteUnibox, + Preview: d.previewSendReply, Handler: d.sendReply, }) @@ -48,20 +50,107 @@ func (d Deps) registerInboxSendTools(r *Registry) { Risk: generation.RiskSend, RequiredOrgPerm: models.PermAccessUnibox, RequiredAPIPerm: models.APIPermWriteUnibox, + Preview: d.previewCompose, Handler: d.composeEmail, }) } +type sendReplyArgs struct { + ThreadID string `json:"thread_id"` + Body string `json:"body"` + BodyHTML string `json:"body_html"` + AccountID string `json:"account_id"` + // ExpectTo is pinned at approval; a reply whose recipient has changed since is refused. + ExpectTo string `json:"expect_to,omitempty"` +} + +type composeArgs struct { + To string `json:"to"` + Subject string `json:"subject"` + Body string `json:"body"` + BodyHTML string `json:"body_html"` + AccountID string `json:"account_id"` + FromTagID string `json:"from_tag_id"` +} + +// errReplyTargetChanged refuses a reply whose recipient is no longer the one the member approved. +var errReplyTargetChanged = errors.New("the conversation has a newer message since this reply was approved; ask again so the member can approve the new recipient") + +// replyTarget resolves who a reply in threadID goes to, its subject, and the message it answers. +func (d Deps) replyTarget(ctx context.Context, inv Invocation, threadID string) (string, string, uuid.UUID, error) { + res, xerr := d.Unibox.GetByThread(ctx, inv.OrgID, uuid.Nil, threadID, "1", "") + if xerr != nil { + return "", "", uuid.Nil, fromErrx(xerr) + } + if len(res.Data) == 0 { + return "", "", uuid.Nil, ErrInvalidArgs + } + latest := res.Data[0] + to := firstNonEmpty(latest.FromAddr) + if to == "" { + return "", "", uuid.Nil, ErrInvalidArgs + } + return to, replySubject(latest.Subject), latest.ID, nil +} + +// previewSendReply resolves the reply's recipient and sender, and pins both in the args the member approves. +func (d Deps) previewSendReply(ctx context.Context, inv Invocation, args json.RawMessage) (*models.AgentSendPreview, json.RawMessage, error) { + if err := d.requireUnibox(ctx, inv); err != nil { + return nil, nil, err + } + in, err := decodeArgs[sendReplyArgs](args) + if err != nil { + return nil, nil, err + } + if strings.TrimSpace(in.ThreadID) == "" { + return nil, nil, ErrInvalidArgs + } + to, subject, _, err := d.replyTarget(ctx, inv, in.ThreadID) + if err != nil { + return nil, nil, err + } + acct, err := d.resolveSenderAccount(ctx, inv, in.AccountID, "", to) + if err != nil { + return nil, nil, err + } + in.AccountID, in.ExpectTo = acct.ID.String(), to + pinned, err := json.Marshal(in) + if err != nil { + return nil, nil, err + } + return &models.AgentSendPreview{From: acct.Email, To: []string{to}, Subject: subject, Body: in.Body, BodyHTML: in.BodyHTML}, pinned, nil +} + +// previewCompose resolves the sender of a new email and pins it in the args the member approves. +func (d Deps) previewCompose(ctx context.Context, inv Invocation, args json.RawMessage) (*models.AgentSendPreview, json.RawMessage, error) { + if err := d.requireUnibox(ctx, inv); err != nil { + return nil, nil, err + } + in, err := decodeArgs[composeArgs](args) + if err != nil { + return nil, nil, err + } + to := strings.TrimSpace(in.To) + if to == "" { + return nil, nil, ErrInvalidArgs + } + acct, err := d.resolveSenderAccount(ctx, inv, in.AccountID, in.FromTagID, to) + if err != nil { + return nil, nil, err + } + in.AccountID, in.FromTagID = acct.ID.String(), "" + pinned, err := json.Marshal(in) + if err != nil { + return nil, nil, err + } + return &models.AgentSendPreview{From: acct.Email, To: []string{to}, Subject: in.Subject, Body: in.Body, BodyHTML: in.BodyHTML}, pinned, nil +} + func (d Deps) sendReply(ctx context.Context, inv Invocation, args json.RawMessage) (string, error) { if err := d.requireUnibox(ctx, inv); err != nil { return "", err } - in, err := decodeArgs[struct { - ThreadID string `json:"thread_id"` - Body string `json:"body"` - BodyHTML string `json:"body_html"` - AccountID string `json:"account_id"` - }](args) + in, err := decodeArgs[sendReplyArgs](args) if err != nil { return "", err } @@ -70,17 +159,12 @@ func (d Deps) sendReply(ctx context.Context, inv Invocation, args json.RawMessag } // Resolve the reply target from the thread's latest message. - res, xerr := d.Unibox.GetByThread(ctx, inv.OrgID, uuid.Nil, in.ThreadID, "1", "") - if xerr != nil { - return "", fromErrx(xerr) + to, subject, latestID, err := d.replyTarget(ctx, inv, in.ThreadID) + if err != nil { + return "", err } - if len(res.Data) == 0 { - return "", ErrInvalidArgs - } - latest := res.Data[0] - to := firstNonEmpty(latest.FromAddr) - if to == "" { - return "", ErrInvalidArgs + if in.ExpectTo != "" && bareEmail(in.ExpectTo) != bareEmail(to) { + return "", errReplyTargetChanged } if err := d.assertNotSuppressed(ctx, inv, to); err != nil { return "", err @@ -93,7 +177,7 @@ func (d Deps) sendReply(ctx context.Context, inv Invocation, args json.RawMessag sendReq := &emailsend.SendEmailRequest{ To: []string{to}, - Subject: replySubject(latest.Subject), + Subject: subject, BodyHTML: in.BodyHTML, BodyPlain: in.Body, ThreadID: in.ThreadID, @@ -101,7 +185,7 @@ func (d Deps) sendReply(ctx context.Context, inv Invocation, args json.RawMessag } // Best-effort: pull the original Message-ID so the reply threads via // In-Reply-To. The thread preview does not carry it, so fetch the message. - if full, ferr := d.Unibox.GetByID(ctx, inv.OrgID, latest.ID); ferr == nil && full != nil && mailhdr.ValidMessageID(full.MessageID) { + if full, ferr := d.Unibox.GetByID(ctx, inv.OrgID, latestID); ferr == nil && full != nil && mailhdr.ValidMessageID(full.MessageID) { sendReq.InReplyTo = []string{full.MessageID} } resp, xerr := d.EmailSend.SendEmail(ctx, inv.UserID, inv.OrgID, accountID, sendReq) @@ -116,14 +200,7 @@ func (d Deps) composeEmail(ctx context.Context, inv Invocation, args json.RawMes if err := d.requireUnibox(ctx, inv); err != nil { return "", err } - in, err := decodeArgs[struct { - To string `json:"to"` - Subject string `json:"subject"` - Body string `json:"body"` - BodyHTML string `json:"body_html"` - AccountID string `json:"account_id"` - FromTagID string `json:"from_tag_id"` - }](args) + in, err := decodeArgs[composeArgs](args) if err != nil { return "", err } @@ -158,30 +235,38 @@ func (d Deps) composeEmail(ctx context.Context, inv Invocation, args json.RawMes // otherwise the best compose candidate for the recipient (auto-pick within a // tag when tagID is set). Mirrors the compose handler. func (d Deps) resolveSender(ctx context.Context, inv Invocation, accountID, tagID, recipient string) (uuid.UUID, error) { + acct, err := d.resolveSenderAccount(ctx, inv, accountID, tagID, recipient) + if err != nil { + return uuid.Nil, err + } + return acct.ID, nil +} + +func (d Deps) resolveSenderAccount(ctx context.Context, inv Invocation, accountID, tagID, recipient string) (*models.Email, error) { if strings.TrimSpace(accountID) != "" && accountID != "auto" { id, err := parseUUIDArg(accountID) if err != nil { - return uuid.Nil, err + return nil, err } cand, _, xerr := d.Compose.Resolve(ctx, inv.UserID, inv.OrgID, &id, nil, bareEmail(recipient)) if xerr != nil { - return uuid.Nil, fromErrx(xerr) + return nil, fromErrx(xerr) } - return cand.Account.ID, nil + return &cand.Account, nil } var tag *uuid.UUID if strings.TrimSpace(tagID) != "" { id, err := parseUUIDArg(tagID) if err != nil { - return uuid.Nil, err + return nil, err } tag = &id } cand, _, xerr := d.Compose.Resolve(ctx, inv.UserID, inv.OrgID, nil, tag, bareEmail(recipient)) if xerr != nil { - return uuid.Nil, fromErrx(xerr) + return nil, fromErrx(xerr) } - return cand.Account.ID, nil + return &cand.Account, nil } // assertNotSuppressed refuses to send to a recipient the org suppressed diff --git a/internal/app/aitools/tools_mailboxes.go b/internal/app/aitools/tools_mailboxes.go index 6e1793f9b..ef448f377 100644 --- a/internal/app/aitools/tools_mailboxes.go +++ b/internal/app/aitools/tools_mailboxes.go @@ -64,6 +64,7 @@ func (d Deps) registerMailboxTools(r *Registry) { "action": enumProp("Warmup lifecycle action.", "start", "pause", "resume", "stop"), }, "email_account_id", "action"), Risk: generation.RiskWrite, + AlwaysAsk: true, RequiredOrgPerm: models.PermManageEmails, RequiredAPIPerm: models.APIPermWriteEmails, Handler: d.setMailboxWarmup, @@ -77,6 +78,7 @@ func (d Deps) registerMailboxTools(r *Registry) { "hold": boolProp("true holds the mailbox out of campaigns, false puts it back."), }, "email_account_id", "hold"), Risk: generation.RiskWrite, + AlwaysAsk: true, RequiredOrgPerm: models.PermManageEmails, RequiredAPIPerm: models.APIPermWriteEmails, Handler: d.setMailboxSendHold, @@ -143,7 +145,7 @@ func (d Deps) getMailbox(ctx context.Context, inv Invocation, args json.RawMessa if _, err := parseUUIDArg(in.EmailAccountID); err != nil { return "", err } - mb, xerr := d.Emails.Get(ctx, inv.UserID.String(), in.EmailAccountID) + mb, xerr := d.Emails.Get(ctx, inv.OrgID.String(), in.EmailAccountID) if xerr != nil { return "", fromErrx(xerr) } diff --git a/internal/app/aitools/tools_segments.go b/internal/app/aitools/tools_segments.go index 91c4796e8..e916d1a88 100644 --- a/internal/app/aitools/tools_segments.go +++ b/internal/app/aitools/tools_segments.go @@ -142,7 +142,8 @@ func (d Deps) registerSegmentTools(r *Registry) { "campaign_id": strProp("The campaign's UUID."), "segment_ids": arrProp("The segment UUIDs to link, at most 20. An empty list detaches every segment.", map[string]any{"type": "string"}), }, "campaign_id", "segment_ids"), - Risk: generation.RiskWrite, + Risk: generation.RiskWrite, + AlwaysAsk: true, // Linking an audience changes who a campaign mails. RequiredOrgPerm: models.PermManageCampaigns, RequiredAPIPerm: models.APIPermWriteCampaigns, @@ -157,6 +158,7 @@ func (d Deps) registerSegmentTools(r *Registry) { "campaign_id": strProp("The campaign to enrol them into."), }, "segment_id", "campaign_id"), Risk: generation.RiskWrite, + AlwaysAsk: true, RequiredOrgPerm: models.PermManageCampaigns, RequiredAPIPerm: models.APIPermWriteCampaigns, Handler: d.addSegmentToCampaign, diff --git a/internal/app/aitools/tools_skills.go b/internal/app/aitools/tools_skills.go index 70742a0af..d359479b6 100644 --- a/internal/app/aitools/tools_skills.go +++ b/internal/app/aitools/tools_skills.go @@ -5,6 +5,7 @@ import ( "encoding/json" "strings" + "github.com/warmbly/warmbly/internal/models" "github.com/warmbly/warmbly/internal/pkg/generation" ) @@ -18,8 +19,10 @@ func (d Deps) registerSkillTools(r *Registry) { InputSchema: objectSchema(map[string]any{ "name": strProp("The playbook name, exactly as listed."), }, "name"), - Risk: generation.RiskRead, - Handler: d.loadSkill, + Risk: generation.RiskRead, + RequiredOrgPerm: models.PermUseAI, + RequiredAPIPerm: models.APIPermAIAgent, + Handler: d.loadSkill, }) } diff --git a/internal/app/aitools/tools_team.go b/internal/app/aitools/tools_team.go index fb1962e8e..3e011c295 100644 --- a/internal/app/aitools/tools_team.go +++ b/internal/app/aitools/tools_team.go @@ -37,6 +37,7 @@ func (d Deps) registerTeamTools(r *Registry) { "role_ids": arrProp("Role UUIDs to assign (use instead of role_id for multiple).", strProp("Role UUID.")), }, "email"), Risk: generation.RiskWrite, + AlwaysAsk: true, JWTOnly: true, RequiredOrgPerm: models.PermManageTeam, Handler: d.inviteMember, @@ -51,6 +52,7 @@ func (d Deps) registerTeamTools(r *Registry) { "role_ids": arrProp("Role UUIDs to set (replaces the member's roles).", strProp("Role UUID.")), }, "member_user_id"), Risk: generation.RiskWrite, + AlwaysAsk: true, JWTOnly: true, RequiredOrgPerm: models.PermManageTeam, Handler: d.updateMemberRole, @@ -92,13 +94,14 @@ func (d Deps) registerTeamTools(r *Registry) { r.Register(Tool{ Name: "get_invitation_link", - Description: "Get the shareable invite link/token for a pending invitation.", + Description: "Issue the shareable invite token for a pending invitation. The token is shown to the member, never to you.", InputSchema: objectSchema(map[string]any{ "invitation_id": strProp("The invitation UUID."), }, "invitation_id"), Risk: generation.RiskRead, JWTOnly: true, RequiredOrgPerm: models.PermManageTeam, + SecretFields: []string{"token"}, Handler: d.getInvitationLink, }) } diff --git a/internal/app/aitools/tools_web.go b/internal/app/aitools/tools_web.go index 9d0f7e575..98c611f41 100644 --- a/internal/app/aitools/tools_web.go +++ b/internal/app/aitools/tools_web.go @@ -14,6 +14,7 @@ import ( "github.com/microcosm-cc/bluemonday" "github.com/warmbly/warmbly/internal/app/webhook" + "github.com/warmbly/warmbly/internal/models" "github.com/warmbly/warmbly/internal/pkg/generation" "github.com/warmbly/warmbly/internal/pkg/safehttp" ) @@ -38,8 +39,10 @@ func (d Deps) registerWebTools(r *Registry) { "query": strProp("The search query."), "limit": intProp("Max results (1-10, default 5)."), }, "query"), - Risk: generation.RiskRead, - Handler: d.searchWeb, + Risk: generation.RiskRead, + RequiredOrgPerm: models.PermUseAI, + RequiredAPIPerm: models.APIPermAIAgent, + Handler: d.searchWeb, }) r.Register(Tool{ @@ -48,8 +51,10 @@ func (d Deps) registerWebTools(r *Registry) { InputSchema: objectSchema(map[string]any{ "url": strProp("The https URL to fetch."), }, "url"), - Risk: generation.RiskRead, - Handler: d.fetchURL, + Risk: generation.RiskRead, + RequiredOrgPerm: models.PermUseAI, + RequiredAPIPerm: models.APIPermAIAgent, + Handler: d.fetchURL, }) } diff --git a/internal/app/aitools/tools_webhooks.go b/internal/app/aitools/tools_webhooks.go index 01ba0a2f4..85b0583ef 100644 --- a/internal/app/aitools/tools_webhooks.go +++ b/internal/app/aitools/tools_webhooks.go @@ -29,7 +29,7 @@ func (d Deps) registerWebhookTools(r *Registry) { r.Register(Tool{ Name: "create_webhook", - Description: "Create a webhook endpoint subscribed to event types. Must be an HTTPS URL. Returns the signing secret once.", + Description: "Create a webhook endpoint subscribed to event types. Must be an HTTPS URL. The signing secret is shown once to the member, never to you.", InputSchema: objectSchema(map[string]any{ "url": strProp("HTTPS endpoint URL (required)."), "description": strProp("Optional description."), @@ -39,6 +39,7 @@ func (d Deps) registerWebhookTools(r *Registry) { Risk: generation.RiskWrite, RequiredOrgPerm: models.PermManageSettings, RequiredAPIPerm: models.APIPermWebhooks, + SecretFields: []string{"secret"}, Handler: d.createWebhook, }) @@ -72,13 +73,14 @@ func (d Deps) registerWebhookTools(r *Registry) { r.Register(Tool{ Name: "rotate_webhook_secret", - Description: "Rotate a webhook endpoint's signing secret. Returns the new secret once.", + Description: "Rotate a webhook endpoint's signing secret. The new secret is shown once to the member, never to you.", InputSchema: objectSchema(map[string]any{ "webhook_id": strProp("The endpoint's UUID."), }, "webhook_id"), Risk: generation.RiskWrite, RequiredOrgPerm: models.PermManageSettings, RequiredAPIPerm: models.APIPermWebhooks, + SecretFields: []string{"secret"}, Handler: d.rotateWebhookSecret, }) diff --git a/internal/app/email/service.go b/internal/app/email/service.go index 9b71d4485..34e69e323 100644 --- a/internal/app/email/service.go +++ b/internal/app/email/service.go @@ -31,7 +31,7 @@ import ( type EmailService interface { Search(ctx context.Context, userID, search, cursor, tag, limit string, allowedAccountIDs []uuid.UUID) (*models.EmailsResult, *errx.Error) - Get(ctx context.Context, userID, emailAccountID string) (*models.Email, *errx.Error) + Get(ctx context.Context, orgID, emailAccountID string) (*models.Email, *errx.Error) // Update writes a mailbox's settings. orgID scopes the write (the mailbox // is a workspace asset); userID only names who to tell the worker about. Update(ctx context.Context, orgID, userID, emailAccountID string, udata *models.UpdateEmail) (*models.Email, *errx.Error) diff --git a/internal/app/research/service.go b/internal/app/research/service.go index b163b31b1..83b9cb4d8 100644 --- a/internal/app/research/service.go +++ b/internal/app/research/service.go @@ -245,7 +245,7 @@ func (s *service) execute(ctx context.Context, inv aitools.Invocation, run *mode var captured *models.ResearchResult saveAttempts := 0 tools := make([]generation.ToolDef, 0, 4) - for _, t := range s.registry.ToolDefsByName(inv, "search_web", "fetch_url", "load_skill") { + for _, t := range s.registry.ResearchTools(inv, "search_web", "fetch_url", "load_skill") { switch t.Name { case "search_web": tools = append(tools, budgeted(t, &searchBudget)) diff --git a/internal/app/slackapp/agent.go b/internal/app/slackapp/agent.go index e29b316dd..a5ec1e3f9 100644 --- a/internal/app/slackapp/agent.go +++ b/internal/app/slackapp/agent.go @@ -51,10 +51,14 @@ type agentTurn struct { // invocation builds the tool identity from the member's live permissions. func invocation(m *models.OrganizationMember, link *models.SlackUserLink) aitools.Invocation { + perms := m.Permissions + if m.IsOwner() { + perms = models.AllPermissions + } return aitools.Invocation{ OrgID: link.OrganizationID, UserID: link.UserID, - OrgPerms: m.Permissions, + OrgPerms: perms, UserAgent: "Slack", } } @@ -131,7 +135,7 @@ func (s *Service) runTurn(ctx context.Context, t agentTurn) { } // resumeTurn continues a paused run after the owner's decision. -func (s *Service) resumeTurn(ctx context.Context, token string, row *models.SlackAgentThread, inv aitools.Invocation, decision string) { +func (s *Service) resumeTurn(ctx context.Context, token string, row *models.SlackAgentThread, inv aitools.Invocation, toolCallID, decision string) { if s.agent == nil { return } @@ -147,14 +151,14 @@ func (s *Service) resumeTurn(ctx context.Context, token string, row *models.Slac defer release() r := s.newRenderer(token, row.ChannelID, row.ThreadTS, row.SessionID, strings.HasPrefix(row.ChannelID, "D")) r.start(ctx) - xerr := s.agent.Resume(ctx, inv, row.SessionID, decision, r.emit) + xerr := s.agent.Resume(ctx, inv, row.SessionID, toolCallID, decision, r.emit) s.afterRun(ctx, token, row, r, xerr) } func (s *Service) afterRun(ctx context.Context, token string, row *models.SlackAgentThread, r *renderer, xerr *errx.Error) { r.finish(ctx, xerr) if a := r.pendingApproval(); a != nil { - ts, err := s.client.PostMessage(ctx, token, approvalCard(row.ChannelID, row.ThreadTS, row.ID, *a)) + ts, err := s.client.PostMessage(ctx, token, approvalCard(row.ChannelID, row.ThreadTS, row.ID, row.SessionID, *a)) if err != nil { log.Warn().Err(err).Msg("slack: approval card failed") } else if err := s.repo.SetAgentThreadApproval(ctx, row.OrganizationID, row.ID, ts); err != nil { @@ -285,9 +289,13 @@ func formatThreadContext(msgs []slackMessage, skipTS string) string { // pendingApprovalInfo is the paused tool the approval card shows. type pendingApprovalInfo struct { - Tool string - Risk string - ArgsSummary string + Tool string + Risk string + ToolCallID string + Arguments string + ArgumentsTruncated bool + Preview *models.AgentSendPreview + AlwaysAllowOffered bool } func riskLabel(risk string) string { @@ -300,25 +308,102 @@ func riskLabel(risk string) string { return "Needs your approval" } +const ( + // approvalChunkRunes keeps one preformatted block well inside Slack's per-block limit. + approvalChunkRunes = 2900 + // approvalMaxChunks bounds a card's detail blocks; the rest is read in the dashboard. + approvalMaxChunks = 10 +) + +// approvalValue binds a card's buttons to its Slack thread row and the one tool call it decides. +func approvalValue(rowID uuid.UUID, toolCallID string) string { + return rowID.String() + "|" + toolCallID +} + +// parseApprovalValue reads approvalValue; a value without a tool call id decides nothing. +func parseApprovalValue(v string) (uuid.UUID, string, bool) { + row, call, ok := strings.Cut(v, "|") + if !ok || call == "" { + return uuid.Nil, "", false + } + id, err := uuid.Parse(row) + if err != nil { + return uuid.Nil, "", false + } + return id, call, true +} + +// preformatted is a rich_text code block, which Slack shows verbatim with no mrkdwn escaping. +func preformatted(s string) Block { + return Block{"type": "rich_text", "elements": []any{ + Block{"type": "rich_text_preformatted", "elements": []any{Block{"type": "text", "text": s}}}, + }} +} + +// chunkRunes splits s into pieces of at most n runes. +func chunkRunes(s string, n int) []string { + r := []rune(s) + out := make([]string, 0, len(r)/n+1) + for len(r) > n { + out = append(out, string(r[:n])) + r = r[n:] + } + if len(r) > 0 { + out = append(out, string(r)) + } + return out +} + +// approvalDetails renders what the action will do, within the card's block budget. +func approvalDetails(a pendingApprovalInfo) ([]Block, bool) { + var out []Block + budget := approvalMaxChunks + truncated := a.ArgumentsTruncated + add := func(label, body string) { + if strings.TrimSpace(body) == "" { + return + } + out = append(out, contextBlock("*"+label+"*")) + for _, c := range chunkRunes(body, approvalChunkRunes) { + if budget == 0 { + truncated = true + return + } + out = append(out, preformatted(c)) + budget-- + } + } + if p := a.Preview; p != nil { + out = append(out, sectionBlock("*From:* "+escapeMrkdwn(p.From)+"\n*To:* "+escapeMrkdwn(strings.Join(p.To, ", "))+"\n*Subject:* "+escapeMrkdwn(p.Subject))) + add("Message", p.Body) + add("HTML body", p.BodyHTML) + } + add("All arguments", a.Arguments) + return out, truncated +} + // approvalCard asks the session owner to approve a paused tool. -func approvalCard(channel, threadTS string, rowID uuid.UUID, a pendingApprovalInfo) Message { - id := rowID.String() +func approvalCard(channel, threadTS string, rowID, sessionID uuid.UUID, a pendingApprovalInfo) Message { + id := approvalValue(rowID, a.ToolCallID) btns := []Block{ actionButton("Approve", ActionApprove, id, "primary"), actionButton("Deny", ActionDeny, id, "danger"), } - if a.Risk == riskWrite { + if a.AlwaysAllowOffered && a.Risk == riskWrite { btns = append(btns, actionButton("Always allow", ActionAlwaysAllow, id, "")) } text := "*Approve this action?*\n*" + escapeMrkdwn(friendlyToolName(a.Tool)) + "* · " + riskLabel(a.Risk) - var args Block - if a.ArgsSummary != "" { - args = contextBlock(escapeMrkdwn(truncateRunes(a.ArgsSummary, 300))) + details, truncated := approvalDetails(a) + bl := append([]Block{sectionBlock(text)}, details...) + if truncated { + bl = append(bl, contextBlock("Some of this is cut off here. Open the conversation in Warmbly to read all of it before you decide.")) + btns = append(btns, urlButton("Open in Warmbly", sessionURL(sessionID))) } + bl = append(bl, actionsBlock(btns...)) return Message{ Channel: channel, ThreadTS: threadTS, Text: "Approve " + friendlyToolName(a.Tool) + "?", - Blocks: blocks(sectionBlock(text), args, actionsBlock(btns...)), + Blocks: blocks(bl...), } } @@ -420,7 +505,11 @@ func (r *renderer) emit(ev aiagent.StreamEvent) { } } case "approval_required": - r.appr = &pendingApprovalInfo{Tool: ev.Tool, Risk: ev.Risk, ArgsSummary: ev.ArgsSummary} + r.appr = &pendingApprovalInfo{ + Tool: ev.Tool, Risk: ev.Risk, ToolCallID: ev.ToolCallID, + Arguments: ev.Arguments, ArgumentsTruncated: ev.ArgumentsTruncated, + Preview: ev.Preview, AlwaysAllowOffered: ev.AlwaysAllowOffered, + } case "error": r.errMsg = ev.Message r.bill = ev.Code == "insufficient_credits" || ev.Code == "usage_cap_exceeded" diff --git a/internal/app/slackapp/approval_test.go b/internal/app/slackapp/approval_test.go new file mode 100644 index 000000000..c6b3f1faa --- /dev/null +++ b/internal/app/slackapp/approval_test.go @@ -0,0 +1,67 @@ +package slackapp + +import ( + "encoding/json" + "strings" + "testing" + + "github.com/google/uuid" + + "github.com/warmbly/warmbly/internal/models" +) + +func TestApprovalValueBindsTheToolCall(t *testing.T) { + row := uuid.New() + gotRow, call, ok := parseApprovalValue(approvalValue(row, "call_1")) + if !ok || gotRow != row || call != "call_1" { + t.Fatalf("round trip = %v %q %v", gotRow, call, ok) + } + if _, _, ok := parseApprovalValue(row.String()); ok { + t.Fatal("a value without a tool call id decides something") + } +} + +func cardJSON(t *testing.T, m Message) string { + t.Helper() + b, err := json.Marshal(m.Blocks) + if err != nil { + t.Fatal(err) + } + return string(b) +} + +func TestApprovalCardShowsTheSendAndOffersAlwaysOnlyWhenAllowed(t *testing.T) { + a := pendingApprovalInfo{ + Tool: "send_reply", Risk: "send", ToolCallID: "c1", + Arguments: `{"body": "hello"}`, + Preview: &models.AgentSendPreview{From: "me@example.com", To: []string{"you@example.com"}, Subject: "Re: hi", Body: "hello", BodyHTML: "

hello

"}, + } + card := cardJSON(t, approvalCard("C1", "1.0", uuid.New(), uuid.New(), a)) + for _, want := range []string{"me@example.com", "you@example.com", "Re: hi", "\\u003cp\\u003ehello\\u003c/p\\u003e", "c1"} { + if !strings.Contains(card, want) { + t.Errorf("card is missing %q", want) + } + } + if strings.Contains(card, ActionAlwaysAllow) { + t.Error("a send offers always allow") + } + a = pendingApprovalInfo{Tool: "add_tag", Risk: "write", ToolCallID: "c2", Arguments: "{}"} + if strings.Contains(cardJSON(t, approvalCard("C1", "1.0", uuid.New(), uuid.New(), a)), ActionAlwaysAllow) { + t.Error("always allow offered without the server offering it") + } + a.AlwaysAllowOffered = true + if !strings.Contains(cardJSON(t, approvalCard("C1", "1.0", uuid.New(), uuid.New(), a)), ActionAlwaysAllow) { + t.Error("always allow missing when offered") + } +} + +func TestApprovalCardTruncatesWithinSlackLimits(t *testing.T) { + a := pendingApprovalInfo{Tool: "update_campaign", Risk: "write", ToolCallID: "c3", Arguments: strings.Repeat("x", approvalChunkRunes*(approvalMaxChunks+2))} + m := approvalCard("C1", "1.0", uuid.New(), uuid.New(), a) + if len(m.Blocks) > 50 { + t.Fatalf("%d blocks", len(m.Blocks)) + } + if !strings.Contains(cardJSON(t, m), "Some of this is cut off") { + t.Error("a cut card does not say so") + } +} diff --git a/internal/app/slackapp/interactivity.go b/internal/app/slackapp/interactivity.go index 656c1146b..7ad0c8027 100644 --- a/internal/app/slackapp/interactivity.go +++ b/internal/app/slackapp/interactivity.go @@ -179,8 +179,8 @@ func (s *Service) requireLinked(ctx context.Context, p *interaction) *actor { // handleApproval resumes a paused run; only the session owner's linked Slack // member may decide, and each card is decided once. func (s *Service) handleApproval(ctx context.Context, p *interaction, value, decision string) { - rowID, err := uuid.Parse(value) - if err != nil { + rowID, toolCallID, ok := parseApprovalValue(value) + if !ok { return } a := s.requireLinked(ctx, p) @@ -214,7 +214,7 @@ func (s *Service) handleApproval(ctx context.Context, p *interaction, value, dec Blocks: blocks(contextBlock(verb + " by <@" + p.User.ID + ">")), }) _ = s.repo.SetAgentThreadApproval(ctx, row.OrganizationID, row.ID, "") - s.resumeTurn(ctx, token, row, invocation(a.member, a.link), decision) + s.resumeTurn(ctx, token, row, invocation(a.member, a.link), toolCallID, decision) } // channelRefusal applies workspace settings and Slack Connect to an explicit diff --git a/internal/models/agent.go b/internal/models/agent.go index 5b660d1fd..fb8f37863 100644 --- a/internal/models/agent.go +++ b/internal/models/agent.go @@ -50,6 +50,25 @@ type PendingAgentTool struct { Risk string `json:"risk"` Args json.RawMessage `json:"args"` ArgsSummary string `json:"args_summary,omitempty"` + // Arguments is Args as indented JSON with sorted keys, capped at AgentApprovalArgsMax bytes. + Arguments string `json:"arguments,omitempty"` + ArgumentsTruncated bool `json:"arguments_truncated,omitempty"` + // Preview is what a send will do: sender, recipients, subject and bodies. + Preview *AgentSendPreview `json:"preview,omitempty"` + // AlwaysAllowOffered is whether the viewer may make this tool a workspace "always allow". + AlwaysAllowOffered bool `json:"always_allow_offered,omitempty"` +} + +// AgentApprovalArgsMax caps the arguments an approval card carries. +const AgentApprovalArgsMax = 16 << 10 + +// AgentSendPreview is the resolved shape of a send awaiting approval. +type AgentSendPreview struct { + From string `json:"from"` + To []string `json:"to"` + Subject string `json:"subject"` + Body string `json:"body"` + BodyHTML string `json:"body_html,omitempty"` } // AgentMessageRow is one persisted transcript turn. Content is the serialized @@ -71,5 +90,7 @@ type AIToolPolicy struct { ToolName string `json:"tool_name"` Decision string `json:"decision"` CreatedBy *uuid.UUID `json:"created_by,omitempty"` - CreatedAt time.Time `json:"created_at"` + // CreatedByName is the setter's display name, for the settings list. + CreatedByName string `json:"created_by_name,omitempty"` + CreatedAt time.Time `json:"created_at"` } diff --git a/internal/models/audit.go b/internal/models/audit.go index 5e57e0302..0f090ee25 100644 --- a/internal/models/audit.go +++ b/internal/models/audit.go @@ -133,6 +133,9 @@ const ( // AI skill (org playbook) create/update/delete. AuditEntityAISkill AuditEntityType = "ai_skill" + // Workspace "always allow" policy for one assistant tool, set or revoked. + AuditEntityAIToolPolicy AuditEntityType = "ai_tool_policy" + // Connected MCP server (external tools) connect/update/disconnect. AuditEntityMCPServer AuditEntityType = "mcp_server" diff --git a/internal/repository/pg_agent.go b/internal/repository/pg_agent.go index 1f66d6490..19dcd0176 100644 --- a/internal/repository/pg_agent.go +++ b/internal/repository/pg_agent.go @@ -38,8 +38,16 @@ type AgentRepository interface { AppendMessages(ctx context.Context, orgID, userID, sessionID uuid.UUID, msgs []models.AgentMessageRow) error LoadTranscript(ctx context.Context, orgID, userID, sessionID uuid.UUID) ([]models.AgentMessageRow, error) + // ClaimPendingTool clears the session's pending approval when it is for toolCallID and returns it; one caller wins. + ClaimPendingTool(ctx context.Context, orgID, sessionID uuid.UUID, toolCallID string) (*models.PendingAgentTool, error) + // ClearPendingTool drops whatever approval the session is waiting on. + ClearPendingTool(ctx context.Context, orgID, sessionID uuid.UUID) error + GetToolPolicies(ctx context.Context, orgID uuid.UUID) (map[string]string, error) + ListToolPolicies(ctx context.Context, orgID uuid.UUID) ([]models.AIToolPolicy, error) SetToolPolicy(ctx context.Context, orgID uuid.UUID, toolName, decision string, createdBy uuid.UUID) error + // DeleteToolPolicy reports whether a policy was removed. + DeleteToolPolicy(ctx context.Context, orgID uuid.UUID, toolName string) (bool, error) } type agentRepository struct { @@ -300,7 +308,69 @@ func (r *agentRepository) SetToolPolicy(ctx context.Context, orgID uuid.UUID, to _, err := r.DB.Exec(ctx, ` INSERT INTO ai_tool_policies (org_id, tool_name, decision, created_by) VALUES ($1, $2, $3, $4) - ON CONFLICT (org_id, tool_name) DO UPDATE SET decision = EXCLUDED.decision`, + ON CONFLICT (org_id, tool_name) DO UPDATE SET decision = EXCLUDED.decision, created_by = EXCLUDED.created_by, created_at = now()`, orgID, toolName, decision, createdBy) return err } + +func (r *agentRepository) ClaimPendingTool(ctx context.Context, orgID, sessionID uuid.UUID, toolCallID string) (*models.PendingAgentTool, error) { + var raw []byte + err := r.DB.QueryRow(ctx, ` + UPDATE agent_sessions s + SET context = s.context - 'pending', updated_at = now() + FROM ( + SELECT id, context->'pending' AS pending + FROM agent_sessions + WHERE id = $1 AND org_id = $2 AND context->'pending'->>'tool_call_id' = $3 + FOR UPDATE + ) old + WHERE s.id = old.id + RETURNING old.pending`, sessionID, orgID, toolCallID).Scan(&raw) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return nil, nil + } + return nil, err + } + var p models.PendingAgentTool + if err := json.Unmarshal(raw, &p); err != nil { + return nil, err + } + return &p, nil +} + +func (r *agentRepository) ClearPendingTool(ctx context.Context, orgID, sessionID uuid.UUID) error { + _, err := r.DB.Exec(ctx, `UPDATE agent_sessions SET context = context - 'pending', updated_at = now() WHERE id = $1 AND org_id = $2 AND context->'pending' IS NOT NULL`, sessionID, orgID) + return err +} + +func (r *agentRepository) ListToolPolicies(ctx context.Context, orgID uuid.UUID) ([]models.AIToolPolicy, error) { + rows, err := r.DB.Query(ctx, ` + SELECT p.org_id, p.tool_name, p.decision, p.created_by, + TRIM(COALESCE(u.first_name, '') || ' ' || COALESCE(u.last_name, '')), p.created_at + FROM ai_tool_policies p + LEFT JOIN users u ON u.id = p.created_by + WHERE p.org_id = $1 + ORDER BY p.created_at DESC, p.tool_name`, orgID) + if err != nil { + return nil, err + } + defer rows.Close() + out := make([]models.AIToolPolicy, 0) + for rows.Next() { + var p models.AIToolPolicy + if err := rows.Scan(&p.OrgID, &p.ToolName, &p.Decision, &p.CreatedBy, &p.CreatedByName, &p.CreatedAt); err != nil { + return nil, err + } + out = append(out, p) + } + return out, rows.Err() +} + +func (r *agentRepository) DeleteToolPolicy(ctx context.Context, orgID uuid.UUID, toolName string) (bool, error) { + tag, err := r.DB.Exec(ctx, `DELETE FROM ai_tool_policies WHERE org_id = $1 AND tool_name = $2`, orgID, toolName) + if err != nil { + return false, err + } + return tag.RowsAffected() > 0, nil +} diff --git a/web/src/app/app/settings/_components/RemieToolPolicies.tsx b/web/src/app/app/settings/_components/RemieToolPolicies.tsx new file mode 100644 index 000000000..c7e073c26 --- /dev/null +++ b/web/src/app/app/settings/_components/RemieToolPolicies.tsx @@ -0,0 +1,73 @@ +// The workspace's "always allow" choices for Remie: which actions run without +// asking, who allowed them and when, and a way to make each one ask again. +// Settings managers only; the list refreshes live through the audit spine. + +import toast from "react-hot-toast"; +import { Loader2Icon, ShieldCheckIcon } from "lucide-react"; +import { useRevokeToolPolicy, useToolPolicies } from "@/lib/api/hooks/app/agent/useToolPolicies"; +import type { AppError } from "@/lib/api/client/normalizeError"; +import buildError from "@/lib/helper/buildError"; +import { useConfirm } from "@/hooks/context/confirm"; +import { toolAction } from "@/components/app/agent/toolLabels"; + +export default function RemieToolPolicies({ canManage }: { canManage: boolean }) { + const policies = useToolPolicies(canManage); + const revoke = useRevokeToolPolicy(); + const confirm = useConfirm(); + + if (!canManage) return null; + const rows = policies.data?.data ?? []; + + function onRevoke(tool: string) { + confirm.show(`Make Remie ask before "${toolAction(tool)}" again?`, async () => { + try { + await revoke.mutateAsync(tool); + toast.success("Remie will ask again"); + } catch (e) { + toast.error(buildError(e as AppError)); + } + }); + } + + return ( +
+
Always allowed actions
+

+ Actions Remie runs without asking, for everyone in the workspace. Sending, starting things that send, and + changing access always ask and never appear here. +

+ {policies.isPending ? ( +
+ ) : rows.length === 0 ? ( +

Nothing is always allowed. Every change asks first.

+ ) : ( +
+ {rows.map((p) => ( +
+ +
+
{toolAction(p.tool_name)}
+
+ {p.created_by_name ? `Allowed by ${p.created_by_name}` : "Allowed"} + {" · "} + {new Date(p.created_at).toLocaleDateString()} +
+
+ +
+ ))} +
+ )} +
+ ); +} diff --git a/web/src/app/app/settings/workspace/page.tsx b/web/src/app/app/settings/workspace/page.tsx index 335baca09..b21a5466d 100644 --- a/web/src/app/app/settings/workspace/page.tsx +++ b/web/src/app/app/settings/workspace/page.tsx @@ -19,6 +19,7 @@ import useCurrentOrganization from "@/lib/api/hooks/app/organizations/useCurrent import { usePermission } from "@/hooks/usePermission"; import useAiMetered from "@/hooks/useAiMetered"; import AdvisorSettingsSection from "@/components/app/advisor/AdvisorSettingsSection"; +import RemieToolPolicies from "../_components/RemieToolPolicies"; // Keyed on the workspace id, which is what makes a switch re-seed the editors // below. Each of them takes its initial value from the org it mounted with, and @@ -315,11 +316,12 @@ function WorkspaceSettings({ org: currentOrg }: { org: StoreOrganization | null > + diff --git a/web/src/components/app/agent/AgentActivity.tsx b/web/src/components/app/agent/AgentActivity.tsx index e02a8c908..689501ddd 100644 --- a/web/src/components/app/agent/AgentActivity.tsx +++ b/web/src/components/app/agent/AgentActivity.tsx @@ -5,7 +5,7 @@ import React from "react"; import { AnimatePresence, motion } from "framer-motion"; -import { CheckIcon, ChevronDownIcon, ExternalLinkIcon, ShieldCheckIcon } from "lucide-react"; +import { CheckIcon, ChevronDownIcon, CopyIcon, ExternalLinkIcon, KeyRoundIcon, ShieldCheckIcon } from "lucide-react"; import { cn } from "@/lib/utils"; import type { AgentPending, AgentToolStep } from "@/stores/slices/agentSlice"; import AgentMark from "./AgentMark"; @@ -77,6 +77,47 @@ export function WorkingStatus({ } function StepRow({ step }: { step: AgentToolStep }) { + return ( + <> + + {step.secrets && Object.keys(step.secrets).length > 0 && } + + ); +} + +// A secret a step returned: shown once here, never to Remie and never stored. +function SecretBox({ secrets }: { secrets: Record }) { + const [copied, setCopied] = React.useState(null); + return ( +
+
+ + Shown once. Copy it now; Remie cannot see it. +
+ {Object.entries(secrets).map(([k, v]) => ( +
+ {k} + + {v} + + +
+ ))} +
+ ); +} + +function StepLine({ step }: { step: AgentToolStep }) { const detail = step.result || step.argsSummary; return ( +
{label}
+ 
+                {text}
+
+
+ ); +} + // The pause a write or send asks for. Nothing runs until one of the buttons -// is pressed; "always allow" is offered for writes only, never for sends. +// is pressed. "Always allow" appears only when the server offers it: to a +// settings manager, for a write that does not always ask. export function ApprovalCard({ pending, onDecide, @@ -234,6 +288,8 @@ export function ApprovalCard({ onDecide: (d: "approve" | "deny" | "always_allow") => void; }) { const isSend = pending.risk === "send"; + const preview = pending.preview; + const [argsOpen, setArgsOpen] = React.useState(!preview); return ( {toolAction(pending.tool)} - {pending.argsSummary && ( -
- {pending.argsSummary} + {preview && ( + <> +
+
From
+
{preview.from}
+
To
+
{preview.to.join(", ")}
+
Subject
+
{preview.subject || "(none)"}
+
+ + {preview.body_html && } + + )} + {pending.arguments ? ( +
+ + {argsOpen && ( + 
+                                {pending.arguments}
+
+ )} + {pending.argumentsTruncated && ( +

+ These arguments are too long to show in full. Skip if you cannot verify them. +

+ )}
+ ) : ( + pending.argsSummary && ( +
+ {pending.argsSummary} +
+ ) )}
@@ -275,8 +371,9 @@ export function ApprovalCard({ > Skip - {!isSend && ( + {!isSend && pending.alwaysAllowOffered && (