Harden MCP tool-listing permissions and fix Provider CLI dialog gap

[pikann]

Summary

Hardens the Paca MCP server's tool-listing permission filter, which had three separate gaps letting the wrong tools show up in tools/list, and adds a small UX fix to the Create Agent dialog.

1. Unpinned super-admin saw only 10 of 76 tools (fixes #461)

For a human user with no PACA_PROJECT_ID (unpinned mode), requiresProject tools were gated solely by scanning permissionMap.projects — which is only ever populated for a pinned project or a global agent's discovered invited projects. An unpinned personal API key never populates it, so a global role grant like {"*": true} was never consulted and every project-scoped tool stayed hidden, even for a super-admin (10/76 tools visible).

isToolVisible (extracted from the inline filter in server.ts for testability) now also checks the caller's global permissions before falling back to scanning per-project permissions — the same hasPermission check already used for non-project-scoped tools.

2. Seven tools were unconditionally listed regardless of permissions

list_task_links, create_task_link, delete_task_link, list_doc_activities, add_doc_comment, update_doc_comment, delete_doc_comment had no TOOL_PERMISSIONS entry at all — added in earlier feature commits without a matching permission mapping. Since an unmapped tool is shown to every caller by default, these were visible regardless of the caller's actual permissions, even though the backend itself always enforced tasks.read/tasks.write/docs.read/docs.write on the corresponding endpoints. Not a backend vulnerability, but an MCP-side least-privilege gap. Added the missing entries, mirroring their already-gated siblings exactly.

3. read_conversation gated on agents.read

Previously left deliberately unmapped (relying on the backend's own agent_id-match check on GET /agents/me/conversations/:id, which is real and correct). Now also gated on agents.read — the same permission domain the backend already uses for managing Agent entities, both globally and per-project — so a regular user without that permission doesn't see the tool at all, rather than seeing it and hitting a 401 (or, in a pinned-mode scenario, a global role grant not intended to cover this being enough to show it).

4. Create Agent dialog: Provider CLI option for global agents

The "Provider CLI" agent type was silently omitted from the type picker when creating a global agent (no project), since it requires a project's own static environment. It's now shown but disabled, with a tooltip explaining it's project-only — discoverable instead of silently missing.

Testing

• apps/mcp: vitest run — 629/629 passing (16 new/updated tests covering isToolVisible's unpinned/pinned branches and the new TOOL_PERMISSIONS entries); tsc --noEmit and biome check clean.
• apps/web: vitest run — full suite passing including new create-agent-dialog.test.tsx coverage (Provider CLI enabled/disabled states); tsc -b and biome check clean.

Fixes #461

🤖 Generated with Claude Code

[pullfrog]

Important

Gating read_conversation on agents.read hides the tool from the very callers who use it — global agents created without a global role (the create dialog's default) and project agents with non-default roles — silently breaking the attached-context feature for them, even though the backend's self-scoped authorization would have allowed every such call.

Reviewed changes — single-commit PR (e92fe1b) touching the MCP server and the web create-agent dialog:

• Permission mappings for new feature tools — permissions.ts now maps the task-link tools (list_task_links/create_task_link/delete_task_link) and doc-activity/comment tools (list_doc_activities/add_doc_comment/update_doc_comment/delete_doc_comment) to tasks.*/docs.*. I cross-checked every key against the backend routers: tasks/{taskId}/links GET is gated on PermissionTasksRead, POST/DELETE on PermissionTasksWrite; docs/{docId}/activities on docs.read, comments on docs.write. The new entries mirror those gates exactly, and the harness that flags "listed without a mapping" is a good regression guard.
• read_conversation now gated on agents.read — previously deliberately unmapped ("is always listed, since its real authorization is unconditional and self-scoped"), and the test that pinned the old behavior was inverted to pin the new expectation. The rationale is documented at length in permissions.ts, but see the inline comment — gating visibility on agents.read is a poor proxy for this tool's authorization model and regresses a core flow. This is the one issue I'd like resolved before merge.
• isToolVisible extraction + unpinned-mode global-grant fix — the tool-listing filter is refactored into an exported isToolVisible, and a requiresProject tool is now also granted when the caller's global permission map covers it in unpinned mode, fixing issue #461 (an unpinned human with a * global role saw only 10 of 76 tools). I traced all 10 new unit tests against the implementation — each pins a distinct branch outcome and would fail if that branch regressed. The refactor preserves pinned-mode behavior exactly.
• Provider CLI discoverability in the create-agent dialog — the card is now always rendered (3-column grid) but disabled with a tooltip at global scope, pointing users at a project's Agents page. I verified the TooltipTrigger render={<button aria-disabled />} + children pattern against @base-ui/react@1.4.1: evaluateRenderProp merges component props (including children) into the cloned render element, so the card content renders inside the trigger as the new tests assume. New tooltip copy is present in all 9 locales.

⚠️ read_conversation gated on agents.read — an orthogonal domain

GET /agents/me/conversations/:conversationId (the tool's backing endpoint) carries no permission middleware at all — it sits behind only Authn + RequireFreshPassword, and its entire authorization model is the self-scoped check in GetConversationForAgent ("an agent may always read the conversation it's currently in", plus the current-conversation-confined cross-read rule). agents.read is never consulted server-side, so gating the tool's visibility on it does not add a security boundary — it only changes who sees the tool. And the callers it hides it from are exactly the ones that use it:

• A global agent created without a global role (the create dialog's default NO_GLOBAL_ROLE) has an empty global permission map and no invited projects in unpinned mode → read_conversation becomes invisible even though the backend would authorize its self-scoped reads without question.
• A project agent assigned a custom project role that omits agents.read → same. (The shipped default roles all grant agents.read, so the common path is spared, but nothing forces it.)

Meanwhile the human personal-key callers the mapping is aimed at never could use the tool anyway — they have no X-Agent-ID, so the handler 401s them ("agent identity required"). The concrete cost of this change is FormatAttachedContext's "call read_conversation" instruction (agent-runner context_item.go) going unfulfillable for the affected agent classes, with no error surfaced — the tool is just absent from the list.

If the intent is to keep the tool reachable for every agent that's server-side-allowed while still hiding it from personal-key humans, the only reliable discriminating signal is the presence of agent identity / X-Conversation-ID in config (i.e. config.agentId), not a permission the agent may or may not hold.

Technical details

# read_conversation visibility gated on an orthogonal permission key

## Affected sites
- apps/mcp/src/permissions.ts:428 — `read_conversation` mapped to `agents.read` + `requiresProject`
- apps/mcp/src/server.ts:214-224 — the `requiresProject` branch of `isToolVisible` that turns that mapping into tool-list visibility
- apps/mcp/src/__tests__/permissions.test.ts:168-172 — inverted test now pins `agents.read`
- services/agent-runner/internal/agent/context_item.go:66-67 — the "call `read_conversation`" instruction that becomes unfulfillable when the tool is hidden

## Required outcome
- An agent that the backend would authorize for a self-scoped conversation read must still have `read_conversation` in its tool list; a caller that the backend will 401 anyway (no agent identity) must not see it.

## Suggested approach (optional)
- Gate on agent identity available in config (`config.agentId` set ⇒ treat as own-conversation capability) instead of a permission key, or leave the tool unmapped and rely on the server-side self-scope (as before this PR), or introduce a dedicated self-scoped key. Confirm whichever path keeps global agents created with no global role able to read attached-context conversations.

## Open questions for the human
- Is hiding the tool from a personal-key human worth breaking the attached-context read for any agent whose role omits `agents.read`?

ℹ️ Nitpicks

• create-agent-dialog.test.tsx mocks globalRolesQueryOptions and llmModelsQueryOptions but not projectRolesQueryOptions, so the two project-agent cases fire a real HTTP request to the roles endpoint (a jsdom URL-resolution failure, swallowed into React Query's isError). Mock it too for a hermetic test — the pattern's already in the file.
• isToolVisible's requiresProject branch console.errors "requires project permission X, granted: true" even when the grant came from the global map — the message misleads when reading server logs. Cosmetic.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

ℹ️ No new issues in this delta — formatting-only change.

Reviewed changes — one new commit since the prior review (553e26b), covering only the changes it introduced:

• Import formatting fix in create-agent-dialog.tsx — the tooltip import was wrapped across multiple lines to satisfy the locked biome check (line-length enforcement in apps/web's CI). No behavioral change; verified the incremental diff contains nothing else. The earlier [!IMPORTANT] concern about read_conversation gating on agents.read remains open and is unaffected by this commit.

  ^{｜ Fix it ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

Important

The new backend enforcement in commit 70027538 closes the original "hidden but callable" gap, but it opens a reversed mismatch: read_conversation is still listed whenever the caller holds agents.read in any project, while the backend now only allows a read scoped to the conversation's own project or the agent's global role. An unpinned global agent invited into a project that grants agents.read, reading a global conversation (ProjectID == uuid.Nil), now sees the tool advertised but every call 404s — a call that this very commit previously authorized.

Reviewed changes — two commits since the prior pullfrog review (553e26b):

• Backend agents.read enforcement for read_conversation — GetConversationForAgent now calls a new authorizeAgentsReadForConversation, checking the agent's global role first, then (for a project-scoped conversation) its role in the conversation's own project; the agent service gains WithAuthorizer, wired in bootstrap/app.go. Production wiring is complete — AuthzPermissionStore implements AgentPermissionStore and the authorizer is constructed with WithAgentRoleResolver(permissionStore) — and the nil-authorizer skip is confined to the bare-Service test convention. The 4 new Go tests pin global-grant→allowed, own-project-grant→allowed, no-grant→denied (even for the same-conversation shortcut), and wrong-project-grant→denied; each would fail if that branch regressed.
• Both prior nitpicks resolved — isToolVisible's requiresProject log line now reads (global or any project), granted:, and create-agent-dialog.test.tsx mocks projectRolesQueryOptions so no project-agent case fires a real request in jsdom.

The reversed mismatch (inline) is the one thing I'd like settled before merge — the PR's explicit goal is "tool-list visibility and backend enforcement agree," and the new doc comment repeats that claim, but the unpinned requiresProject listing is broader than the new enforcement, in the opposite direction of issue #461.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

Important

The new conversations.* domain re-gates the conversation and chat-session routes off agents.*, but migration 000051 backfills only the built-in role names — existing projects' custom roles that granted agents.read will silently lose conversation and chat access on deploy.

Reviewed changes — delta since the prior pullfrog review at commit 2142091:

• Introduced a dedicated conversations permission domain — permissions.go adds conversations.read/write/.*, and the MCP read_conversation gate, the backend GetConversationForAgent authorization, and the project-scoped /conversations/* + /{agentId}/chat-sessions* route gates all move from agents.* onto it, leaving agents.* to govern agent-entity configuration only.
• Added migration 000051 (backfill conversations.* onto the built-in role names) and 000052 (structural dedupe of wildcard-redundant project_roles keys), with the dedupe SQL matching the new frontend dedupeGrantedPermissions semantics.
• Re-gated the web conversation UI — New-conversation button, composer, reply/stop/pause controls, and the project chat float now require conversations.write for project members; a viewer keeps a read-only list/view; heartbeat stays conversations.read; new permission group and labels land in all 9 locales.
• Role badge display (RolesSettings, global-roles activePermissions) now filters through dedupeGrantedPermissions.

Both prior read_conversation important threads remain open: re-orienting the key to conversations.read changed the domain, but neither the no-global-role attached-context gap nor the any-project listing vs own-project+global enforcement mismatch moved.

  ^{｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}