diff --git a/apps/ai/src/chat/agents.test.ts b/apps/ai/src/chat/agents.test.ts index 8fe2c0e099..6b34e60ab2 100644 --- a/apps/ai/src/chat/agents.test.ts +++ b/apps/ai/src/chat/agents.test.ts @@ -20,7 +20,8 @@ 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 { DASHBOARD_AUTHORING_TOOLS } from "../mcp/tools/types" import { CONNECTOR_SYSTEM_PROMPT } from "./prompts" import { buildChatToolkit } from "./maple-toolkit" @@ -137,6 +138,29 @@ describe("profileForTurn", () => { } }) + it("proposes dashboard changes to an analyst and offers no other tool that changes data", () => { + const profile = profileForRoles(profileForTurn(agentForSession(appSession), { kind: "app" }), [ + "org:analyst", + ]) + const { tools } = buildChatToolkit(executor, tenant, profile.ruleset, profile.surface).toolkit + assert.property(tools, "search_traces") + for (const name of MUTATING_TOOL_NAMES) { + if (DASHBOARD_AUTHORING_TOOLS.has(name)) { + assert.equal(evaluatePermission(profile.ruleset, name), "ask", name) + } else { + assert.notProperty(tools, name, `${name} was offered to an analyst`) + } + } + assert.property(tools, "create_dashboard") + // A member's turn on the same session still proposes every one of them. + const member = profileForRoles(profileForTurn(agentForSession(appSession), { kind: "app" }), [ + "org:member", + ]) + 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..43d30a2446 100644 --- a/apps/ai/src/chat/profiles.ts +++ b/apps/ai/src/chat/profiles.ts @@ -6,7 +6,10 @@ * 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 { DASHBOARD_AUTHORING_TOOLS } from "../mcp/tools/types" import type { AgentDefinition } from "./agents" import { READ_ONLY_RULESET } from "./permissions" import { CONNECTOR_SYSTEM_PROMPT } from "./prompts" @@ -21,6 +24,18 @@ export interface TurnProfile { readonly prompt: string } +/** The ruleset an analyst's turn runs under: only dashboard changes are still proposed to them. */ +const forAnalyst = (ruleset: PermissionRuleset): PermissionRuleset => + Arr.map(ruleset, (rule) => + rule.action === "ask" && !DASHBOARD_AUTHORING_TOOLS.has(rule.tool) + ? new PermissionRule({ tool: rule.tool, action: "deny" }) + : rule, + ) + +/** The profile for whoever the turn's tools run as: an analyst is offered no change outside dashboards. */ +export const profileForRoles = (profile: TurnProfile, roles: ReadonlyArray): TurnProfile => + Policy.isAnalyst(roles) ? { ...profile, ruleset: forAnalyst(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/app.test.ts b/apps/ai/src/mcp/app.test.ts index f40e9ec61f..dc2a479f6c 100644 --- a/apps/ai/src/mcp/app.test.ts +++ b/apps/ai/src/mcp/app.test.ts @@ -736,7 +736,7 @@ describe("MCP HTTP authorization", () => { }) describe("MCP tools/list per credential", () => { - it("hides write tools from a read-only key, in the same isolate that shows them to a full key", async () => { + it("hides write tools from a read-only key and all but the dashboard tools from an analyst, in the isolate that shows them to a full key", async () => { const db = createTestDb(createdDbs) const base = Layer.mergeAll(db.layer, Env.layer.pipe(Layer.provide(testConfig()))) const services = Layer.mergeAll( @@ -748,7 +748,7 @@ describe("MCP tools/list per credential", () => { ).pipe(Layer.provideMerge(base)) const orgId = Schema.decodeSync(OrgId)("org_test") const userId = Schema.decodeSync(UserId)("user_test") - const [full, readOnly] = await Effect.runPromise( + const [full, readOnly, analyst] = await Effect.runPromise( Effect.gen(function* () { const apiKeys = yield* ApiKeysService return [ @@ -757,6 +757,11 @@ describe("MCP tools/list per credential", () => { name: "Read-only key", scopes: Arr.map(INVESTIGATION_READ_FAMILIES, (family) => `${family}:read`), }), + // A key minted for a person acts with that person's org roles. + yield* apiKeys.create(orgId, userId, { + name: "Analyst's key", + metadataJson: { source: "maple_cli", roles: ["org:analyst"], deviceName: "laptop" }, + }), ] as const }).pipe(Effect.provide(services)), ) @@ -783,6 +788,7 @@ describe("MCP tools/list per credential", () => { return { status: response.status, write: names.includes("create_alert_rule"), + dashboard: names.includes("create_dashboard"), read: names.includes("inspect_trace"), feedback: names.includes("send_maple_feedback"), } @@ -791,12 +797,22 @@ describe("MCP tools/list per credential", () => { expect(await listFor(full.secret)).toEqual({ status: 200, write: true, + dashboard: true, read: true, feedback: true, }) expect(await listFor(readOnly.secret)).toEqual({ status: 200, write: false, + dashboard: false, + read: true, + feedback: true, + }) + // An analyst builds dashboards and operates nothing else. + expect(await listFor(analyst.secret)).toEqual({ + status: 200, + write: false, + dashboard: true, read: true, feedback: true, }) diff --git a/apps/ai/src/mcp/server.ts b/apps/ai/src/mcp/server.ts index fadaaec288..3742a6b265 100644 --- a/apps/ai/src/mcp/server.ts +++ b/apps/ai/src/mcp/server.ts @@ -7,8 +7,9 @@ import { McpToolsUnavailableError, } from "./executor-loader" import { CurrentMcpRequestTenant } from "./tenant" -import { refusesReadOnlyCredential, type McpToolResult } from "./tools/types" +import { refusesAnalyst, refusesReadOnlyCredential, type McpToolResult } from "./tools/types" import type { McpToolDescriptor } from "@maple/domain/mcp-tool-contract" +import * as Policy from "@maple/domain/policy" /** * The text blocks are what a model reads; `structuredContent` is the typed output the tool's @@ -69,15 +70,19 @@ 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: - * the registry would refuse every such call anyway, and a listed tool invites it. + * Hides a tool from the `tools/list` of a caller the registry would refuse, and answers a call to + * it as unknown: a listed tool invites the call. */ -const HiddenFromReadOnlyCredentials = EffectMcpServer.visibility(() => - Effect.gen(function* () { - const tenant = yield* CurrentMcpRequestTenant - return tenant?.readOnly !== true - }), -) +const hiddenFrom = (isRefused: (tenant: { readOnly?: true; roles: ReadonlyArray }) => boolean) => + EffectMcpServer.visibility(() => + Effect.gen(function* () { + const tenant = yield* CurrentMcpRequestTenant + return tenant === undefined || !isRefused(tenant) + }), + ) + +const HiddenFromReadOnlyKeys = hiddenFrom((tenant) => tenant.readOnly === true) +const HiddenFromAnalysts = hiddenFrom((tenant) => tenant.readOnly === true || Policy.isAnalyst(tenant.roles)) const toBoundaryErrorResult = (error: { readonly _tag: string; readonly message: string }) => toCallToolResult({ @@ -104,12 +109,16 @@ export class McpToolRegistration extends Context.Service< make: Effect.gen(function* () { const server = yield* EffectMcpServer.McpServer const loader = yield* McpToolExecutorLoader - const readOnlyGate = yield* Layer.build(HiddenFromReadOnlyCredentials.layer) + const keyGate = yield* Layer.build(HiddenFromReadOnlyKeys.layer) + const analystGate = yield* Layer.build(HiddenFromAnalysts.layer) + // A dashboard tool is hidden from read-only keys alone; every other change from analysts too. + const gateFor = (name: string, readOnly: boolean) => { + if (refusesAnalyst(name, readOnly)) return { services: analystGate } + return refusesReadOnlyCredential(name, readOnly) ? { services: keyGate } : undefined + } const addTool = (descriptor: McpToolDescriptor) => server.addTool({ - ...(refusesReadOnlyCredential(descriptor.name, descriptor.annotations?.readOnlyHint === true) - ? { services: readOnlyGate } - : undefined), + ...gateFor(descriptor.name, descriptor.annotations?.readOnlyHint === true), tool: new McpSchema.Tool({ name: descriptor.name, title: descriptor.title, 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..27847f8e37 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 @@ -6,26 +6,27 @@ import { cleanupTestDbs, createTestDb, type TestDb } from "@maple/backend/testin import { AlertDestinationsService } from "@maple/backend/services/alerts/AlertDestinationsService" import { ErrorPolicyService } from "@maple/backend/services/errors/ErrorPolicyService" import { CurrentMcpTenant } from "../../tenant" -import { executeRegisteredMcpToolUnscoped } from "../registry" +import { executeRegisteredMcpToolUnscoped, mapleToolCatalog } from "../registry" +import { DASHBOARD_AUTHORING_TOOLS, FEEDBACK_TOOL, refusalFor } from "../types" 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 +72,52 @@ describe("read-only API key over MCP", () => { ) }) +describe("an analyst over MCP", () => { + // A layer per test: each owns a database that `afterEach` removes. + const analyst = () => makeLayer(false, undefined, "org:analyst") + + 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(analyst())), + ) + + 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(analyst())), + ) +}) + +describe("what an analyst may change over MCP", () => { + const changing = Arr.filter(mapleToolCatalog, (definition) => !definition.hints.readOnly) + const openTo = (caller: Parameters[0]) => + Arr.filter( + Arr.map(changing, (definition) => definition.name), + (name) => refusalFor(caller, name, false) === undefined, + ).sort() + + it("is the dashboard tools and Maple's feedback channel, out of every tool that changes data", () => { + expect(openTo({ roles: ["org:analyst"] })).toEqual( + [...DASHBOARD_AUTHORING_TOOLS, FEEDBACK_TOOL].sort(), + ) + }) + + it("is only the feedback channel when their key is read-only", () => { + expect(openTo({ roles: ["org:analyst"], readOnly: true })).toEqual([FEEDBACK_TOOL]) + expect(refusalFor({ roles: ["org:analyst"], readOnly: true }, "create_dashboard", false)).toBe( + "read_only_key", + ) + }) +}) + 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..0bd7b926f4 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, + refusalFor, refusesReadOnlyCredential, } from "./types" import { unknownToolMessage } from "../lib/tool-suggestions" @@ -750,9 +752,13 @@ export const executeRegisteredMcpToolUnscoped = Effect.fn("McpToolRegistry.execu if (refusesReadOnlyCredential(definition.name, definition.hints.readOnly)) { const tenant = yield* CurrentMcpTenant - if (tenant.readOnly === true) { + const refusal = refusalFor(tenant, definition.name, definition.hints.readOnly) + if (refusal !== undefined) { 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: + refusal === "analyst" + ? `\`${definition.name}\` changes data. ${Policy.analystMessage}` + : `\`${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..a466a3a9d9 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,32 @@ export const FEEDBACK_TOOL = "send_maple_feedback" export const refusesReadOnlyCredential = (name: string, readOnly: boolean): boolean => !readOnly && name !== FEEDBACK_TOOL +/** The tools that build dashboards: the only changes an analyst's role reaches. */ +export const DASHBOARD_AUTHORING_TOOLS: ReadonlySet = new Set([ + "create_dashboard", + "update_dashboard", + "add_dashboard_widget", + "update_dashboard_widget", + "remove_dashboard_widget", + "reorder_dashboard_widgets", + "replace_dashboard_widgets", +]) + +/** Whether an analyst is refused this tool: one that changes data and does not build a dashboard. */ +export const refusesAnalyst = (name: string, readOnly: boolean): boolean => + refusesReadOnlyCredential(name, readOnly) && !DASHBOARD_AUTHORING_TOOLS.has(name) + +/** Why this caller is refused the tool, and so never shown it: their key, their role, or nothing. */ +export const refusalFor = ( + tenant: { readonly readOnly?: true; readonly roles: ReadonlyArray }, + name: string, + readOnly: boolean, +): "analyst" | "read_only_key" | undefined => { + if (Policy.isAnalyst(tenant.roles) && refusesAnalyst(name, readOnly)) return "analyst" + if (tenant.readOnly === true && refusesReadOnlyCredential(name, readOnly)) return "read_only_key" + return undefined +} + /** * Who a tool is for. * diff --git a/apps/ai/src/routes/chat-sessions.http.test.ts b/apps/ai/src/routes/chat-sessions.http.test.ts index dc498c99fc..ed9cd662fc 100644 --- a/apps/ai/src/routes/chat-sessions.http.test.ts +++ b/apps/ai/src/routes/chat-sessions.http.test.ts @@ -1,5 +1,5 @@ import { afterEach, assert, describe, it } from "@effect/vitest" -import { AuthorizationUnavailableError, OrgId, UserId } from "@maple/domain/http" +import { AuthorizationUnavailableError, OrgId, RoleName, UserId } from "@maple/domain/http" import { fakeChatSessionsLayer } from "@maple/backend/testing/chat-sessions-fake" import { envPorts } from "@maple/backend/platform/env-ports" import { ConfigProvider, Effect, Layer, Schema } from "effect" @@ -314,6 +314,48 @@ describe("ChatSessionsRouter", () => { ) }) + it.effect("lets an analyst read an investigation's thread but not drive or stop it", () => { + const testDb = createTestDb(trackedDbs) + const turns: Array = [] + const asAnalyst = Layer.effect( + AuthService, + Effect.gen(function* () { + const auth = yield* AuthService + return { + ...auth, + resolveTenant: () => + Effect.succeed({ + orgId: ORG, + userId: USER, + roles: [Schema.decodeSync(RoleName)("org:analyst")], + authMode: "clerk" as const, + }), + } + }), + ).pipe(Layer.provide(AuthService.layer)) + const call = (handler: (request: Request) => Promise, method: string, path: string) => + Effect.promise(() => + handler( + new Request(`http://api.localhost${SESSION_PATH}${path}`, { + method, + headers: { authorization: "Bearer aaa.bbb.ccc" }, + }), + ), + ) + return withHandler( + testDb, + turns, + Effect.fnUntraced(function* (handler) { + const sent = yield* send(handler, { authorization: "Bearer aaa.bbb.ccc" }) + assert.strictEqual(sent.status, 403) + assert.strictEqual(turns.length, 0) + assert.strictEqual((yield* call(handler, "POST", "/abort")).status, 403) + assert.strictEqual((yield* call(handler, "GET", "/history")).status, 200) + }), + asAnalyst, + ) + }) + it.effect("hides another org's session as 404", () => { const testDb = createTestDb(trackedDbs) const turns: Array = [] diff --git a/apps/ai/src/routes/chat-sessions.http.ts b/apps/ai/src/routes/chat-sessions.http.ts index 222383c2ac..c0a9bd122a 100644 --- a/apps/ai/src/routes/chat-sessions.http.ts +++ b/apps/ai/src/routes/chat-sessions.http.ts @@ -40,6 +40,7 @@ import { AuthService } from "@maple/backend/services/auth/AuthService" import { Env } from "@maple/backend/platform/Env" import { resolveChatSessionTenantContext } from "../mcp/lib/resolve-tenant" import type { TenantContext } from "@maple/domain/http" +import * as Policy from "@maple/domain/policy" const encodeJson = Schema.encodeSync(Schema.fromJsonString(Schema.Unknown)) @@ -147,6 +148,12 @@ const resolveSession = Effect.fn("ChatSessionsRouter.resolveSession")(function* return { ok: false, failure: problem("Chat session not found", 404) } as const } + // Driving an investigation is a member's job (`investigations:run`): a follow-up can file a new + // diagnosis, and a stop ends a running pass. An analyst reads the thread. + if (request.method !== "GET" && Policy.isAnalyst(tenant.roles)) { + return { ok: false, failure: problem(Policy.analystMessage, 403) } as const + } + const stub = (yield* ChatSessions).session(sessionId) const session: ResolvedSession = { sessionId, tenant, stub, url }