From 50647de0143e68fde051eb1a0ccf4fe63f9947b2 Mon Sep 17 00:00:00 2001 From: Julius Marminge Date: Sat, 10 Oct 2026 09:36:20 -0700 Subject: [PATCH] fix(server): check the specific scope for scripts, preview input, and full-access MCP grants (#17772) Co-authored-by: Claude Opus 5.5 --- apps/server/src/auth/EnvironmentAuth.ts | 16 +++++++-- apps/server/src/auth/McpOAuth.test.ts | 9 +++++ apps/server/src/auth/RpcAuthorization.ts | 7 ++++ apps/server/src/preview/ServerBrowser.test.ts | 29 ++++++++++++++++ apps/server/src/preview/ServerBrowser.ts | 34 ++++++++++++++++--- .../src/preview/ServerBrowserStream.test.ts | 8 +++-- .../server/src/preview/ServerBrowserStream.ts | 8 ++--- apps/server/src/project/http.ts | 6 ++-- packages/contracts/src/settings.test.ts | 21 +++++++++++- packages/contracts/src/settings.ts | 20 ++++++++++- 10 files changed, 140 insertions(+), 18 deletions(-) diff --git a/apps/server/src/auth/EnvironmentAuth.ts b/apps/server/src/auth/EnvironmentAuth.ts index ac90e67b6f26df3d2d93d5e7484466f6d7a167ba..d66cdbb62c3650f0fd3317daa93b8915773d941a 100644 --- a/apps/server/src/auth/EnvironmentAuth.ts +++ b/apps/server/src/auth/EnvironmentAuth.ts @@ -4,6 +4,8 @@ import { AuthAdministrativeScopes, AuthOrchestrationOperateScope, AuthOrchestrationReadScope, + AuthSettingsWriteScope, + AuthSourceControlWriteScope, AuthStandardClientScopes, type AuthAccessTokenResult, type AuthBrowserSessionResult, @@ -74,7 +76,10 @@ export interface IssuedBearerSession { * token cannot reach the full RPC surface around that cap. * * A read-only grant holds `orchestration:read` alone. Any other grant also - * holds `orchestration:operate` and carries its runtime-mode ceiling. + * holds `orchestration:operate` and carries its runtime-mode ceiling. A + * full-access client may also change environment preferences and clone + * repositories, so it holds the scopes those need over RPC. Approving a grant + * requires every scope it holds. */ const MCP_CLIENT_SUBJECT = "mcp-client"; const MCP_CLIENT_SESSION_TTL = Duration.days(30); @@ -84,7 +89,14 @@ export const mcpClientScopes = ( ): ReadonlyArray => access === "read-only" ? [AuthOrchestrationReadScope] - : [AuthOrchestrationReadScope, AuthOrchestrationOperateScope]; + : access === "full-access" + ? [ + AuthOrchestrationReadScope, + AuthOrchestrationOperateScope, + AuthSettingsWriteScope, + AuthSourceControlWriteScope, + ] + : [AuthOrchestrationReadScope, AuthOrchestrationOperateScope]; export interface McpClientSession { readonly sessionId: AuthSessionId; diff --git a/apps/server/src/auth/McpOAuth.test.ts b/apps/server/src/auth/McpOAuth.test.ts index 97e4a4d824ceff9851eeed2ca6d8c85ecdc880de..cb58294fb2bf85a91bfef9a1823a3979bf5dae16 100644 --- a/apps/server/src/auth/McpOAuth.test.ts +++ b/apps/server/src/auth/McpOAuth.test.ts @@ -451,6 +451,15 @@ it.live( const readOnlyResponse = yield* approveWith(readOnly.credential); expect(readOnlyResponse.status).toBe(400); expect(readOnlyResponse.message).toContain("cannot grant this access"); + const operator = yield* auth.issuePairingCredential({ + scopes: ["orchestration:read", "orchestration:operate"], + }); + const operatorFull = yield* decide(handler, params, { + _tag: "pairing-code", + access: "full-access", + code: operator.credential, + }); + expect(operatorFull.status).toBe(400); // A read-only code can approve read-only access. const readOnlyCode = yield* auth.issuePairingCredential({ scopes: ["orchestration:read"] }); diff --git a/apps/server/src/auth/RpcAuthorization.ts b/apps/server/src/auth/RpcAuthorization.ts index 11756026f6d1b9013ef221c37af1c7217cd73905..9c6243fde2300b8ab9599fe563da3810da17aa07 100644 --- a/apps/server/src/auth/RpcAuthorization.ts +++ b/apps/server/src/auth/RpcAuthorization.ts @@ -8,6 +8,8 @@ import { ServerSettingsPatch, ProviderInstanceMutation, requiredScopesForServerSettingsPatch, + requiredScopesForProjectMutation, + ProjectMutation, AuthSettingsWriteScope, AuthProvidersManageScope, AuthEnvironmentMaintainScope, @@ -242,6 +244,8 @@ const requiredScopesForSettingsUpdate = (payload: unknown) => { : [...new Set([...scopes, AuthProvidersManageScope])]; }; +const decodeProjectMutation = Schema.decodeUnknownSync(ProjectMutation); + const requiredScopesForRpcCall = ( method: string, payload: unknown, @@ -260,6 +264,9 @@ const requiredScopesForRpcCall = ( ]; } if (method === WS_METHODS.serverUpdateSettings) return requiredScopesForSettingsUpdate(payload); + if (method === WS_METHODS.projectsMutate) { + return requiredScopesForProjectMutation(decodeProjectMutation(payload)); + } const guarded = clientRpcRequiredScopes(method, payload); if (guarded.length > 0) return guarded; return [requiredScopeForRpcMethod(method)]; diff --git a/apps/server/src/preview/ServerBrowser.test.ts b/apps/server/src/preview/ServerBrowser.test.ts index f66e255c51b4fe5daea63ac33cfc1c2402a66dc8..385223a42d397034d4eddde2b35afbe2f4219a83 100644 --- a/apps/server/src/preview/ServerBrowser.test.ts +++ b/apps/server/src/preview/ServerBrowser.test.ts @@ -1126,6 +1126,35 @@ it.live("a page's file picker goes to the controller and takes its uploaded file ).pipe(Effect.provide(layer)), ); +it.live("only the current controller can answer a file picker", () => + Effect.scoped( + Effect.gen(function* () { + const { browser, tabId } = yield* ready; + const first = yield* browser.attachViewer(viewerInput(tabId, true)); + yield* first.input({ type: "takeControl" }); + const setFiles = vi.fn(async (_files: unknown) => {}); + contexts[0]!.page.emit("filechooser", { + isMultiple: () => false, + element: () => ({ getAttribute: async () => "" }), + setFiles, + }); + let offered = yield* Queue.take(first.output); + while (offered._tag !== "fileChooser") offered = yield* Queue.take(first.output); + const second = yield* browser.attachViewer(viewerInput(tabId, true)); + yield* first.input({ type: "releaseControl" }); + yield* second.input({ type: "takeControl" }); + const answered = yield* browser.answerFileChooser({ + threadId: scope.thread.threadId, + tabId, + chooserId: offered.id, + files: [{ name: "a.txt", mimeType: "text/plain", buffer: Buffer.from("a") }], + }); + expect(answered).toBe(false); + expect(setFiles).not.toHaveBeenCalled(); + }), + ).pipe(Effect.provide(layer)), +); + it.live("a file picker replaces a stalled viewer backlog instead of being dropped", () => Effect.scoped( Effect.gen(function* () { diff --git a/apps/server/src/preview/ServerBrowser.ts b/apps/server/src/preview/ServerBrowser.ts index 072a022f90c0853bdad3c115ccf2472bb1ff700d..939d112c4908105863cf38e3603a1240576dfd5e 100644 --- a/apps/server/src/preview/ServerBrowser.ts +++ b/apps/server/src/preview/ServerBrowser.ts @@ -310,11 +310,15 @@ interface ServerTab { readonly openerTabId: string | undefined; /** Finished downloads, newest last; files live until the tab closes. */ readonly downloads: Array; - /** A page's open file picker, waiting for the controlling viewer's files. */ + /** + * A page's open file picker, waiting for the controlling viewer's files. Its + * id is reissued for each viewer it is offered to, so only that viewer can answer. + */ fileChooser: { readonly id: string; readonly chooser: FileChooser; readonly accept: string; + readonly offeredTo: string | null; } | null; dialog: Dialog | null; setting: PreviewViewportSetting; @@ -1027,14 +1031,27 @@ const make = Effect.gen(function* () { .catch(() => null)) ?? ""; // A newer picker replaces an unanswered one, as a real browser allows only one. if (previous) closeFileChooser(tab); - tab.fileChooser = { id: NodeCrypto.randomUUID(), chooser, accept: accept.slice(0, 1024) }; + tab.fileChooser = { + id: NodeCrypto.randomUUID(), + chooser, + accept: accept.slice(0, 1024), + offeredTo: null, + }; pushFileChooser(tab); }; const pushFileChooser = (tab: ServerTab) => { - const message = fileChooserMessage(tab); + const open = tab.fileChooser; const controller = [...tab.viewers].find((viewer) => viewer.id === tab.control.controller); - if (message) controller?.push(message); + if (!open || !controller) return; + if (open.offeredTo !== controller.id) { + // A new controller gets a fresh id; the previous one can no longer answer. + tab.fileChooser = { ...open, id: NodeCrypto.randomUUID(), offeredTo: controller.id }; + if (open.offeredTo !== null) + for (const viewer of tab.viewers) viewer.push({ _tag: "fileChooserClosed", id: open.id }); + } + const message = fileChooserMessage(tab); + if (message) controller.push(message); }; const closeFileChooser = (tab: ServerTab) => { @@ -1050,7 +1067,14 @@ const make = Effect.gen(function* () { ) => { const tab = tabs.get(tabKey(input.threadId, input.tabId)); const open = tab?.fileChooser; - if (!tab || !open || open.id !== input.chooserId) return false; + if ( + !tab || + !open || + open.id !== input.chooserId || + open.offeredTo === null || + open.offeredTo !== tab.control.controller + ) + return false; if (input.files.length > 0) { await open.chooser.setFiles( open.chooser.isMultiple() ? [...input.files] : input.files.slice(0, 1), diff --git a/apps/server/src/preview/ServerBrowserStream.test.ts b/apps/server/src/preview/ServerBrowserStream.test.ts index 0654ea62a2b82817bd524ef69999510338664b73..8fc153d9e2f5135ad562be7afbfe8d585bfecb80 100644 --- a/apps/server/src/preview/ServerBrowserStream.test.ts +++ b/apps/server/src/preview/ServerBrowserStream.test.ts @@ -5,6 +5,7 @@ import { expect, it } from "@effect/vitest"; import { AuthOrchestrationOperateScope, AuthOrchestrationReadScope, + AuthPreviewOperateScope, AuthSessionId, PREVIEW_STREAM_HOST_SETUP_CLOSE_CODE, PreviewStreamHostSetup, @@ -71,7 +72,7 @@ it.effect.each([ Effect.gen(function* () { const canOperate = hasOperateScope && interactive; const scopes = hasOperateScope - ? [AuthOrchestrationReadScope, AuthOrchestrationOperateScope] + ? [AuthOrchestrationReadScope, AuthPreviewOperateScope] : [AuthOrchestrationReadScope]; const auth = makeAuth(scopes); const inputs: unknown[] = []; @@ -293,11 +294,12 @@ it.effect("passes uploaded files to the page's open picker and needs operate sco }); expect((yield* upload([AuthOrchestrationReadScope], "chooser-1")).status).toBe(403); expect(answers).toEqual([]); - expect((yield* upload([AuthOrchestrationOperateScope], "chooser-1")).status).toBe(204); + expect((yield* upload([AuthOrchestrationOperateScope], "chooser-1")).status).toBe(403); + expect((yield* upload([AuthPreviewOperateScope], "chooser-1")).status).toBe(204); expect(answers).toEqual([ { chooserId: "chooser-1", files: [{ name: "notes.txt", text: "hello" }] }, ]); - expect((yield* upload([AuthOrchestrationOperateScope], "stale")).status).toBe(409); + expect((yield* upload([AuthPreviewOperateScope], "stale")).status).toBe(409); }).pipe(Effect.scoped), ); diff --git a/apps/server/src/preview/ServerBrowserStream.ts b/apps/server/src/preview/ServerBrowserStream.ts index 964f143f5ef60c5cbf0fc00f11dfbff8ab8f4b38..b2960c2401efa454b42f0f1c5151ad05e4d4af55 100644 --- a/apps/server/src/preview/ServerBrowserStream.ts +++ b/apps/server/src/preview/ServerBrowserStream.ts @@ -1,7 +1,7 @@ import * as NodeHttpServerRequest from "@effect/platform-node/NodeHttpServerRequest"; import { - AuthOrchestrationOperateScope, AuthOrchestrationReadScope, + AuthPreviewOperateScope, PREVIEW_STREAM_HOST_SETUP_CLOSE_CODE, PreviewStreamHostSetup, } from "@t3tools/contracts"; @@ -71,9 +71,9 @@ const makeHandler = (browser: ServerBrowser.ServerBrowser["Service"]) => } const session = yield* authenticateMediaRequest(AuthOrchestrationReadScope); const params = url.value.searchParams; + // Watching needs read access; driving the page is a preview mutation. const canOperate = - session.scopes.includes(AuthOrchestrationOperateScope) && - params.get("interactive") !== "false"; + session.scopes.includes(AuthPreviewOperateScope) && params.get("interactive") !== "false"; const threadId = params.get("threadId") ?? ""; const tabId = params.get("tabId") ?? ""; if (threadId.length === 0 || tabId.length === 0) { @@ -238,7 +238,7 @@ const UPLOAD_MAX_FILES = 20; */ const receiveUpload = (browser: ServerBrowser.ServerBrowser["Service"], params: URLSearchParams) => Effect.gen(function* () { - yield* authenticateMediaRequest(AuthOrchestrationOperateScope); + yield* authenticateMediaRequest(AuthPreviewOperateScope); const request = yield* HttpServerRequest.HttpServerRequest; const fs = yield* FileSystem.FileSystem; const parts = yield* request.multipart.pipe( diff --git a/apps/server/src/project/http.ts b/apps/server/src/project/http.ts index ed383d95b98aa3bfe0fc71d501367ba5803a6dc8..e1a04655d8353d1449ee18262dbbf09e890d033f 100644 --- a/apps/server/src/project/http.ts +++ b/apps/server/src/project/http.ts @@ -1,7 +1,7 @@ import { - AuthOrchestrationOperateScope, AuthOrchestrationReadScope, EnvironmentHttpApi, + requiredScopesForProjectMutation, } from "@t3tools/contracts"; import * as Effect from "effect/Effect"; import * as HttpApiBuilder from "effect/http-api/HttpApiBuilder"; @@ -53,7 +53,9 @@ export const layer = HttpApiBuilder.group( "mutate", Effect.fn("environment.projects.mutate")(function* (args) { yield* annotateEnvironmentRequest(args.endpoint.name); - yield* requireEnvironmentScope(AuthOrchestrationOperateScope); + for (const scope of requiredScopesForProjectMutation(args.payload)) { + yield* requireEnvironmentScope(scope); + } const operation = projectMutationOperation(projects, args.payload); return yield* startup .enqueueCommand(operation) diff --git a/packages/contracts/src/settings.test.ts b/packages/contracts/src/settings.test.ts index b5d39323a23d1c3451be7065d8414fa50ca55be2..b9446389d08321b15a6911abb23e17105efbbf52 100644 --- a/packages/contracts/src/settings.test.ts +++ b/packages/contracts/src/settings.test.ts @@ -1,13 +1,14 @@ import { describe, expect, it } from "vite-plus/test"; import * as Schema from "effect/Schema"; -import type { ProjectId } from "./baseSchemas.ts"; +import type { CommandId, ProjectId } from "./baseSchemas.ts"; import { ProviderDriverKind, ProviderInstanceId } from "./providerInstance.ts"; import { ClientSettingsSchema, ClientSettingsPatch, ClaudeSettings, DEFAULT_SERVER_SETTINGS, + requiredScopesForProjectMutation, resolveProviderInstanceEnabled, ServerSettings, ServerSettingsPatch, @@ -1068,3 +1069,21 @@ describe("ServerSettings.removeAgentCreditsOnMerge", () => { ).toBe(true); }); }); + +describe("requiredScopesForProjectMutation", () => { + it("adds the settings scope only when a mutation carries scripts", () => { + const projectId = "project-1" as ProjectId; + const commandId = "command-1" as CommandId; + expect( + requiredScopesForProjectMutation({ type: "project.update", commandId, projectId }), + ).toEqual(["orchestration:operate"]); + expect( + requiredScopesForProjectMutation({ + type: "project.update", + commandId, + projectId, + scripts: [], + }), + ).toEqual(["orchestration:operate", "settings:write"]); + }); +}); diff --git a/packages/contracts/src/settings.ts b/packages/contracts/src/settings.ts index 0038ede86c2d4dde583c151d24b20b2446160db0..0e5be96e740af0e9dfcfcc81fa3e3e0cb5885df4 100644 --- a/packages/contracts/src/settings.ts +++ b/packages/contracts/src/settings.ts @@ -1,5 +1,6 @@ import { SshDeviceHostConfigs } from "./device.ts"; import { + AuthOrchestrationOperateScope, AuthSettingsWriteScope, AuthProvidersManageScope, type AuthEnvironmentScope, @@ -27,7 +28,7 @@ import { ProviderOptionSelections, } from "./model.ts"; import { ModelSelection } from "./modelSelection.ts"; -import { ProjectScript } from "./project.ts"; +import { ProjectScript, type ProjectMutation } from "./project.ts"; import { DEFAULT_RUNTIME_MODE, RuntimeMode } from "./providerPolicy.ts"; import { BrowserProfile, BrowserProfileId, DEFAULT_BROWSER_PROFILE_ID } from "./browserProfile.ts"; import { @@ -1583,6 +1584,23 @@ export function requiredScopesForServerSettingsPatch( ]; } +/** + * Project scripts run on the host when a worktree is created or a thread + * settles, so a project mutation that carries them needs the same grant as + * saving project scripts through settings, on top of `orchestration:operate`. + */ +export function requiredScopesForProjectMutation( + mutation: ProjectMutation, +): ReadonlyArray { + if (mutation.type === "project.delete" || mutation.scripts === undefined) { + return [AuthOrchestrationOperateScope]; + } + return [ + AuthOrchestrationOperateScope, + ...requiredScopesForServerSettingsPatch({ defaultProjectScripts: mutation.scripts }), + ]; +} + export const ClientSettingsPatch = Schema.Struct({ notificationMode: Schema.optionalKey(NotificationMode), inAppNotificationsEnabled: Schema.optionalKey(Schema.Boolean), -- 2.54.0