diff --git a/apps/ai/src/chat/agents.test.ts b/apps/ai/src/chat/agents.test.ts index 8fe2c0e099..6f206ceef8 100644 --- a/apps/ai/src/chat/agents.test.ts +++ b/apps/ai/src/chat/agents.test.ts @@ -20,7 +20,7 @@ import { mapleToolCatalog } from "../mcp/tools/registry" import { MUTATING_TOOL_NAMES } from "./permissions" import type { TenantContext } from "@maple/domain/http" import { AGENTS, agentForSession, isAutonomousTurn, sessionOf } from "./agents" -import { profileForTurn } from "./profiles" +import { profileForRoles, profileForTurn } from "./profiles" import { CONNECTOR_SYSTEM_PROMPT } from "./prompts" import { buildChatToolkit } from "./maple-toolkit" @@ -137,6 +137,25 @@ describe("profileForTurn", () => { } }) + it("offers an observer's turn the read tools and none that change data", () => { + const profile = profileForRoles(profileForTurn(agentForSession(appSession), { kind: "app" }), [ + "org:observer", + ]) + const { tools } = buildChatToolkit(executor, tenant, profile.ruleset, profile.surface).toolkit + assert.property(tools, "search_traces") + for (const name of MUTATING_TOOL_NAMES) { + assert.notProperty(tools, name, `${name} was offered to an observer`) + } + // A member's turn on the same session still proposes them. + const member = profileForRoles(profileForTurn(agentForSession(appSession), { kind: "app" }), [ + "org:member", + ]) + assert.isNotEmpty([...MUTATING_TOOL_NAMES]) + for (const name of MUTATING_TOOL_NAMES) { + assert.equal(evaluatePermission(member.ruleset, name), "ask", name) + } + }) + it("offers a connector the repository sandbox but no internal-audience tool", () => { // The reviewer's pull request tools act on the org's behalf; a channel never reaches them. const { tools } = toolsFor(connectorSession, CONNECTOR_ORIGIN) diff --git a/apps/ai/src/chat/profiles.ts b/apps/ai/src/chat/profiles.ts index de8ab9450c..bcdf976a03 100644 --- a/apps/ai/src/chat/profiles.ts +++ b/apps/ai/src/chat/profiles.ts @@ -6,7 +6,9 @@ * surface and be billed, traced or prompted as another. */ import type { ChatTurnOrigin } from "@maple/domain/chat-session" -import type { PermissionRuleset } from "@maple/domain/permission" +import { PermissionRule, type PermissionRuleset } from "@maple/domain/permission" +import * as Policy from "@maple/domain/policy" +import { Array as Arr } from "effect" import type { AgentDefinition } from "./agents" import { READ_ONLY_RULESET } from "./permissions" import { CONNECTOR_SYSTEM_PROMPT } from "./prompts" @@ -21,6 +23,16 @@ export interface TurnProfile { readonly prompt: string } +/** The ruleset with every approval-gated tool withheld: an observer could not approve one. */ +const withoutProposals = (ruleset: PermissionRuleset): PermissionRuleset => + Arr.map(ruleset, (rule) => + rule.action === "ask" ? new PermissionRule({ tool: rule.tool, action: "deny" }) : rule, + ) + +/** The profile for whoever the turn's tools run as: an observer is offered nothing that changes data. */ +export const profileForRoles = (profile: TurnProfile, roles: ReadonlyArray): TurnProfile => + Policy.isReadOnly(roles) ? { ...profile, ruleset: withoutProposals(profile.ruleset) } : profile + export const profileForTurn = (agent: AgentDefinition, origin: ChatTurnOrigin): TurnProfile => { switch (origin.kind) { case "app": diff --git a/apps/ai/src/chat/run.ts b/apps/ai/src/chat/run.ts index fb51d25155..5325a13548 100644 --- a/apps/ai/src/chat/run.ts +++ b/apps/ai/src/chat/run.ts @@ -34,7 +34,7 @@ import { agentSessionSpanAttributes, type ResolvedModel } from "../platform/Llm" import { withRunSessionAttributes } from "../platform/genai-spans" import { summarizeCause } from "@maple/backend/platform/describe-cause" import { type AgentDefinition, agentForSession, chatAgent } from "./agents" -import { profileForTurn } from "./profiles" +import { profileForRoles, profileForTurn } from "./profiles" import { MUTATING_TOOL_NAMES } from "./permissions" import { makeTextSanitizer, @@ -165,7 +165,7 @@ export const runChatTurn = (input: ChatRunInput) => const runChatTurnFrom = (input: ChatRunInput, startedAt: number) => { const agent = input.agent ?? agentForSession(input.sessionId) - const profile = profileForTurn(agent, input.origin) + const profile = profileForRoles(profileForTurn(agent, input.origin), input.tenant.roles) const sessionAttributes = agentSessionSpanAttributes(input.model.tags) const completion = input.completion?.({ turnId: input.messageId, diff --git a/apps/ai/src/mcp/server.ts b/apps/ai/src/mcp/server.ts index fadaaec288..c00bc1e4bd 100644 --- a/apps/ai/src/mcp/server.ts +++ b/apps/ai/src/mcp/server.ts @@ -7,7 +7,7 @@ import { McpToolsUnavailableError, } from "./executor-loader" import { CurrentMcpRequestTenant } from "./tenant" -import { refusesReadOnlyCredential, type McpToolResult } from "./tools/types" +import { isReadOnlyTenant, refusesReadOnlyCredential, type McpToolResult } from "./tools/types" import type { McpToolDescriptor } from "@maple/domain/mcp-tool-contract" /** @@ -69,13 +69,13 @@ const annotateToolFailure = (result: McpToolResult) => : Effect.void /** - * Hides a tool from a read-only credential's `tools/list`, and answers a call to it as unknown: + * Hides a tool from a read-only credential's or an observer's `tools/list`, and answers a call to it as unknown: * the registry would refuse every such call anyway, and a listed tool invites it. */ const HiddenFromReadOnlyCredentials = EffectMcpServer.visibility(() => Effect.gen(function* () { const tenant = yield* CurrentMcpRequestTenant - return tenant?.readOnly !== true + return tenant === undefined || !isReadOnlyTenant(tenant) }), ) diff --git a/apps/ai/src/mcp/tools/__tests__/read-only-key.test.ts b/apps/ai/src/mcp/tools/__tests__/read-only-key.test.ts index aa4a19780a..090ad00061 100644 --- a/apps/ai/src/mcp/tools/__tests__/read-only-key.test.ts +++ b/apps/ai/src/mcp/tools/__tests__/read-only-key.test.ts @@ -11,21 +11,21 @@ import { executeRegisteredMcpToolUnscoped } from "../registry" const createdDbs: TestDb[] = [] afterEach(() => cleanupTestDbs(createdDbs)) -const tenant = (readOnly: boolean, scopes?: ReadonlyArray): TenantContext => ({ +const tenant = (readOnly: boolean, scopes?: ReadonlyArray, role = "root"): TenantContext => ({ orgId: Schema.decodeSync(OrgId)("org_read_only_key"), userId: Schema.decodeSync(UserId)("user_read_only_key"), - roles: [Schema.decodeSync(RoleName)("root")], + roles: [Schema.decodeSync(RoleName)(role)], authMode: "self_hosted", ...(readOnly ? { readOnly: true as const } : undefined), ...(scopes === undefined ? undefined : { scopes }), }) -const makeLayer = (readOnly: boolean, scopes?: ReadonlyArray) => { +const makeLayer = (readOnly: boolean, scopes?: ReadonlyArray, role?: string) => { const database = createTestDb(createdDbs).layer return Layer.mergeAll( ErrorPolicyService.layer.pipe(Layer.provide(database)), database, - Layer.succeed(CurrentMcpTenant, tenant(readOnly, scopes)), + Layer.succeed(CurrentMcpTenant, tenant(readOnly, scopes, role)), Layer.succeed(AlertDestinationsService, { listDestinations: () => Effect.succeed({ destinations: [] }), } as never), @@ -71,6 +71,30 @@ describe("read-only API key over MCP", () => { ) }) +describe("an observer over MCP", () => { + // A layer per test: each owns a database that `afterEach` removes. + const observer = () => makeLayer(false, undefined, "org:observer") + + it.effect("runs read tools", () => + Effect.gen(function* () { + const result = yield* call("get_error_notification_policy", {}) + expect(Result.isSuccess(result)).toBe(true) + }).pipe(Effect.provide(observer())), + ) + + it.effect("refuses a tool that changes data and says which role can", () => + Effect.gen(function* () { + const result = yield* call("update_error_notification_policy", updatePolicy) + expect(Result.isFailure(result)).toBe(true) + if (Result.isFailure(result)) { + expect(result.failure._tag).toBe("@maple/mcp/errors/McpAuthInvalidError") + expect(result.failure.message).toContain("update_error_notification_policy") + expect(result.failure.message).toContain("Ask an org admin for the Member role") + } + }).pipe(Effect.provide(observer())), + ) +}) + describe("a scoped key without users:read over MCP", () => { const scopes = Arr.map(INVESTIGATION_READ_FAMILIES, (family) => `${family}:write`) diff --git a/apps/ai/src/mcp/tools/registry.ts b/apps/ai/src/mcp/tools/registry.ts index e1ca7df54f..e6bfa1b90f 100644 --- a/apps/ai/src/mcp/tools/registry.ts +++ b/apps/ai/src/mcp/tools/registry.ts @@ -1,4 +1,5 @@ // BOUNDARY-FILE: This module owns unparsed external values and narrows them before domain use. +import * as Policy from "@maple/domain/policy" import { McpToolNotFoundError } from "@maple/domain/mcp-tool-contract" import { scopeAllows } from "@maple/domain/http/v2" import type { McpToolSurface } from "@maple/domain/mcp-manifest" @@ -133,6 +134,7 @@ import { type McpToolPhrases, type McpToolRegistrar, type McpToolResult, + isReadOnlyTenant, refusesReadOnlyCredential, } from "./types" import { unknownToolMessage } from "../lib/tool-suggestions" @@ -750,9 +752,11 @@ export const executeRegisteredMcpToolUnscoped = Effect.fn("McpToolRegistry.execu if (refusesReadOnlyCredential(definition.name, definition.hints.readOnly)) { const tenant = yield* CurrentMcpTenant - if (tenant.readOnly === true) { + if (isReadOnlyTenant(tenant)) { return yield* new McpAuthInvalidError({ - message: `\`${definition.name}\` changes data, and this API key is read-only. Use a key with write access, or connect with OAuth.`, + message: Policy.isReadOnly(tenant.roles) + ? `\`${definition.name}\` changes data. ${Policy.readOnlyMessage}` + : `\`${definition.name}\` changes data, and this API key is read-only. Use a key with write access, or connect with OAuth.`, reason: "insufficient_scope", }) } diff --git a/apps/ai/src/mcp/tools/types.ts b/apps/ai/src/mcp/tools/types.ts index be77cec142..384e0240f6 100644 --- a/apps/ai/src/mcp/tools/types.ts +++ b/apps/ai/src/mcp/tools/types.ts @@ -4,6 +4,7 @@ import type { McpToolSurface } from "@maple/domain/mcp-manifest" import type { McpToolName, McpToolOutputs } from "@maple/domain/mcp-outputs" import type { McpToolRequirements } from "./runtime-services" import type { ToolDoc } from "../lib/tool-doc" +import * as Policy from "@maple/domain/policy" export class McpAuthMissingError extends Schema.TaggedError()( "@maple/mcp/errors/McpAuthMissingError", @@ -123,6 +124,12 @@ export const FEEDBACK_TOOL = "send_maple_feedback" export const refusesReadOnlyCredential = (name: string, readOnly: boolean): boolean => !readOnly && name !== FEEDBACK_TOOL +/** Whether the caller may only read: a read-only API key, or anyone whose role is observer. */ +export const isReadOnlyTenant = (tenant: { + readonly readOnly?: true + readonly roles: ReadonlyArray +}): boolean => tenant.readOnly === true || Policy.isReadOnly(tenant.roles) + /** * Who a tool is for. *