From db92131fe914a558fd90d0658cae58ea3fc8134c Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 17 Aug 2026 22:24:50 -0700 Subject: [PATCH] fix(gateway): allow write-scoped worktree creation (#125598) --- docs/concepts/managed-worktrees.md | 2 +- src/gateway/method-scopes.test.ts | 2 +- src/gateway/methods/core-descriptors.ts | 2 +- .../worktrees.authorization.test.ts | 113 ++++++++++++++++++ src/gateway/server-methods/worktrees.test.ts | 32 ++++- src/gateway/server-methods/worktrees.ts | 73 ++++++----- 6 files changed, 187 insertions(+), 37 deletions(-) create mode 100644 src/gateway/server-methods/worktrees.authorization.test.ts diff --git a/docs/concepts/managed-worktrees.md b/docs/concepts/managed-worktrees.md index 6fd441a18a87..30edd06b4013 100644 --- a/docs/concepts/managed-worktrees.md +++ b/docs/concepts/managed-worktrees.md @@ -97,7 +97,7 @@ The Control UI **Worktrees** page under Settings provides the same actions plus | `worktrees.restore` | Restore a removed worktree from its snapshot. | | `worktrees.gc` | Run idle, orphan, and retention cleanup now. | -`worktrees.list` requires `operator.read`, and the mutating methods require `operator.admin`. `worktrees.branches` needs `operator.write` for configured agent workspaces, while any other host path requires `operator.admin` (matching the `sessions.create` cwd bar). It reads existing refs only and never fetches, and remote-only branches come back remote-qualified (`origin/feature-a`) so every returned name resolves as a base ref. New Session can also request a typed repository status from this method; a plain directory or unavailable checkout returns no branches instead of forcing the UI to infer Git capability from an error string. +`worktrees.list` requires `operator.read`. `worktrees.create` and `worktrees.branches` require `operator.write` for configured agent workspaces and registered projects; arbitrary host paths still require `operator.admin`. Write-scoped creation skips repository checkout hooks and `.openclaw/worktree-setup.sh`. Removing, restoring, and garbage-collecting worktrees remain admin-only. Branch listing reads existing refs only and never fetches, and remote-only branches come back remote-qualified (`origin/feature-a`) so every returned name resolves as a base ref. New Session can also request a typed repository status from this method; a plain directory or unavailable checkout returns no branches instead of forcing the UI to infer Git capability from an error string. ## Workboard workspaces diff --git a/src/gateway/method-scopes.test.ts b/src/gateway/method-scopes.test.ts index 36ff45bbc272..79da3f252690 100644 --- a/src/gateway/method-scopes.test.ts +++ b/src/gateway/method-scopes.test.ts @@ -95,7 +95,7 @@ describe("method scope resolution", () => { ["environments.destroy", ["operator.admin"]], ["worktrees.list", ["operator.read"]], ["worktrees.branches", ["operator.write"]], - ["worktrees.create", ["operator.admin"]], + ["worktrees.create", ["operator.write"]], ["projects.list", ["operator.read"]], ["users.prefs.get", ["operator.read"]], ["users.prefs.set", ["operator.write"]], diff --git a/src/gateway/methods/core-descriptors.ts b/src/gateway/methods/core-descriptors.ts index 1195d6f0b827..0247e2a4c5d2 100644 --- a/src/gateway/methods/core-descriptors.ts +++ b/src/gateway/methods/core-descriptors.ts @@ -167,7 +167,7 @@ const CORE_GATEWAY_METHOD_SPECS = [ // Params-aware: Gateway paths start at write scope and are containment-checked // by the handler; node browsing remains admin-only. ["fs.listDir", "fs", "dynamic", "<=2026.7"], - ["worktrees.create", "worktrees", "operator.admin", "2026.7", { controlPlaneWrite: true }], + ["worktrees.create", "worktrees", "operator.write", "2026.7", { controlPlaneWrite: true }], ["worktrees.remove", "worktrees", "operator.admin", "2026.7", { controlPlaneWrite: true }], ["worktrees.restore", "worktrees", "operator.admin", "2026.7", { controlPlaneWrite: true }], ["worktrees.gc", "worktrees", "operator.admin", "2026.7", { controlPlaneWrite: true }], diff --git a/src/gateway/server-methods/worktrees.authorization.test.ts b/src/gateway/server-methods/worktrees.authorization.test.ts new file mode 100644 index 000000000000..2c417402f13f --- /dev/null +++ b/src/gateway/server-methods/worktrees.authorization.test.ts @@ -0,0 +1,113 @@ +import fs from "node:fs/promises"; +import path from "node:path"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { useAutoCleanupTempDirTracker } from "../../../test/helpers/temp-dir.js"; +import type { ManagedWorktreeRecord } from "../../agents/worktrees/types.js"; +import { handleGatewayRequest } from "../server-methods.js"; +import { createWorktreesHandlers } from "./worktrees.js"; + +const tempDirs = useAutoCleanupTempDirTracker(afterEach); + +function worktreeRecord(repoRoot: string): ManagedWorktreeRecord { + return { + id: "worktree-id", + name: "task-one", + repoFingerprint: "0123456789abcdef", + repoRoot, + path: "/state/worktrees/0123456789abcdef/task-one", + branch: "openclaw/task-one", + baseRef: "HEAD", + ownerKind: "manual", + createdAt: 1, + lastActiveAt: 2, + }; +} + +async function dispatchCreate(params: { repoRoot: string; scopes: string[]; workspace: string }) { + const create = vi.fn(async () => worktreeRecord(params.repoRoot)); + const handler = createWorktreesHandlers({ create } as never)["worktrees.create"]; + if (!handler) { + throw new Error("worktrees.create handler is not registered"); + } + const respond = vi.fn(); + await handleGatewayRequest({ + req: { + type: "req", + id: "req-worktree-create", + method: "worktrees.create", + params: { repoRoot: params.repoRoot }, + }, + respond, + client: { + connId: "conn-worktree-create", + connect: { + role: "operator", + scopes: params.scopes, + client: { id: "test", version: "1", platform: "test", mode: "test" }, + minProtocol: 1, + maxProtocol: 1, + }, + } as Parameters[0]["client"], + isWebchatConnect: () => false, + context: { + getRuntimeConfig: () => ({ + agents: { list: [{ id: "main", default: true, workspace: params.workspace }] }, + }), + logGateway: { warn: vi.fn() }, + } as unknown as Parameters[0]["context"], + extraHandlers: { "worktrees.create": handler }, + }); + return { create, respond }; +} + +describe("worktrees.create authorization", () => { + it("allows write-scoped creation inside an agent workspace", async () => { + const workspace = await fs.realpath(tempDirs.make("openclaw-worktree-create-auth-")); + const repoRoot = path.join(workspace, "project"); + await fs.mkdir(repoRoot); + + const { create, respond } = await dispatchCreate({ + repoRoot, + scopes: ["operator.write"], + workspace, + }); + + expect(create).toHaveBeenCalledWith({ + repoRoot, + name: undefined, + baseRef: undefined, + ownerKind: "manual", + runSetupScript: false, + }); + expect(respond).toHaveBeenCalledWith(true, worktreeRecord(repoRoot), undefined); + }); + + it("keeps arbitrary host paths admin-only", async () => { + const root = await fs.realpath(tempDirs.make("openclaw-worktree-create-host-auth-")); + const workspace = path.join(root, "workspace"); + const outside = path.join(root, "outside"); + await Promise.all([fs.mkdir(workspace), fs.mkdir(outside)]); + + const write = await dispatchCreate({ + repoRoot: outside, + scopes: ["operator.write"], + workspace, + }); + expect(write.create).not.toHaveBeenCalled(); + expect(write.respond).toHaveBeenCalledWith( + false, + undefined, + expect.objectContaining({ code: "INVALID_REQUEST" }), + ); + + const admin = await dispatchCreate({ + repoRoot: outside, + scopes: ["operator.admin"], + workspace, + }); + expect(admin.create).toHaveBeenCalledWith( + expect.objectContaining({ repoRoot: outside, runSetupScript: true }), + ); + expect(admin.respond).toHaveBeenCalledWith(true, worktreeRecord(outside), undefined); + }); +}); diff --git a/src/gateway/server-methods/worktrees.test.ts b/src/gateway/server-methods/worktrees.test.ts index 04f7b9b7faa2..23c863270c00 100644 --- a/src/gateway/server-methods/worktrees.test.ts +++ b/src/gateway/server-methods/worktrees.test.ts @@ -75,11 +75,16 @@ describe("worktrees gateway methods", () => { undefined, ]); expect( - await call(handlers, "worktrees.create", { - repoRoot: "/repo", - name: "task-one", - baseRef: "main", - }), + await call( + handlers, + "worktrees.create", + { + repoRoot: "/repo", + name: "task-one", + baseRef: "main", + }, + { client: adminClient, context: emptyConfigContext }, + ), ).toEqual([true, record, undefined]); expect(await call(handlers, "worktrees.remove", { id: record.id, force: true })).toEqual([ true, @@ -106,6 +111,7 @@ describe("worktrees gateway methods", () => { name: "task-one", baseRef: "main", ownerKind: "manual", + runSetupScript: true, }); expect(service.remove).toHaveBeenCalledWith({ id: record.id, @@ -195,6 +201,7 @@ describe("worktrees gateway methods", () => { await fs.mkdir(outside); const project = await registerProjectRegistry({ path: repoRoot, name: "Registered" }); const service = { + create: vi.fn(async () => record), listRepositoryBranches: vi.fn(async () => ({ branches: [] })), }; const handlers = createWorktreesHandlers(service as never); @@ -208,6 +215,21 @@ describe("worktrees gateway methods", () => { expect(allowed?.[0]).toBe(true); expect(service.listRepositoryBranches).toHaveBeenCalledWith(repoRoot); + const created = await call( + handlers, + "worktrees.create", + { repoRoot: alias, name: "registered-task" }, + { client: writeClient, context: emptyConfigContext }, + ); + expect(created?.[0]).toBe(true); + expect(service.create).toHaveBeenCalledWith({ + repoRoot, + name: "registered-task", + baseRef: undefined, + ownerKind: "manual", + runSetupScript: false, + }); + const denied = await call( handlers, "worktrees.branches", diff --git a/src/gateway/server-methods/worktrees.ts b/src/gateway/server-methods/worktrees.ts index b99460143f9d..58a015a268c8 100644 --- a/src/gateway/server-methods/worktrees.ts +++ b/src/gateway/server-methods/worktrees.ts @@ -29,6 +29,35 @@ function invalidParams(respond: Parameters[0]["r respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "invalid worktrees parameters")); } +async function resolveAuthorizedRepoRoot( + method: string, + repoRoot: string, + opts: Parameters[0], +): Promise { + const scopes = Array.isArray(opts.client?.connect.scopes) ? opts.client.connect.scopes : []; + if (scopes.includes(ADMIN_SCOPE)) { + return repoRoot; + } + const containment = await resolveWorkspacePathContainment( + repoRoot, + opts.context.getRuntimeConfig(), + ); + // A stored project row authorizes its canonical repo root for write-scoped clients. + const authorizedRoot = containment?.path ?? (await resolveRecordedProjectRoot(repoRoot)); + if (authorizedRoot) { + return authorizedRoot; + } + opts.respond( + false, + undefined, + errorShape( + ErrorCodes.INVALID_REQUEST, + `${method} outside configured agent workspaces requires gateway scope: ${ADMIN_SCOPE}`, + ), + ); + return undefined; +} + export function createWorktreesHandlers(service: WorktreeService): GatewayRequestHandlers { return { "worktrees.list": async ({ params, respond }) => { @@ -38,18 +67,26 @@ export function createWorktreesHandlers(service: WorktreeService): GatewayReques } respond(true, { worktrees: await service.list() }, undefined); }, - "worktrees.create": async ({ params, respond }) => { + "worktrees.create": async (opts) => { + const { params, respond } = opts; if (!validateWorktreesCreateParams(params)) { invalidParams(respond); return; } + const repoRoot = await resolveAuthorizedRepoRoot("worktrees.create", params.repoRoot, opts); + if (!repoRoot) { + return; + } + const scopes = Array.isArray(opts.client?.connect.scopes) ? opts.client.connect.scopes : []; respond( true, await service.create({ - repoRoot: params.repoRoot, + repoRoot, name: params.name, baseRef: params.baseRef, ownerKind: "manual", + // Repository hooks and .openclaw/worktree-setup.sh execute repo code. + runSetupScript: scopes.includes(ADMIN_SCOPE), }), undefined, ); @@ -91,37 +128,15 @@ export function createWorktreesHandlers(service: WorktreeService): GatewayReques } respond(true, await service.restore({ id: params.id }), undefined); }, - "worktrees.branches": async ({ params, respond, context, client }) => { + "worktrees.branches": async (opts) => { + const { params, respond } = opts; if (!validateWorktreesBranchesParams(params)) { invalidParams(respond); return; } - let repoRoot = params.repoRoot; - const scopes = Array.isArray(client?.connect.scopes) ? client.connect.scopes : []; - if (!scopes.includes(ADMIN_SCOPE)) { - const containment = await resolveWorkspacePathContainment( - params.repoRoot, - context.getRuntimeConfig(), - ); - if (!containment) { - const projectRoot = await resolveRecordedProjectRoot(params.repoRoot); - if (!projectRoot) { - respond( - false, - undefined, - errorShape( - ErrorCodes.INVALID_REQUEST, - `worktrees.branches outside configured agent workspaces requires gateway scope: ${ADMIN_SCOPE}`, - ), - ); - return; - } - // The stored project row is the authorization boundary, so write-scoped clients - // may inspect its canonical repo root without workspace containment. - repoRoot = projectRoot; - } else { - repoRoot = containment.path; - } + const repoRoot = await resolveAuthorizedRepoRoot("worktrees.branches", params.repoRoot, opts); + if (!repoRoot) { + return; } const result = params.includeRepositoryStatus ? await service.listRepositoryBranches(repoRoot, {