From 593ef750b21435a445955710f6130676eab1dd83 Mon Sep 17 00:00:00 2001 From: Alex Southwell Date: Sat, 3 Oct 2026 13:24:42 +1000 Subject: [PATCH 1/2] fix(server): allow reviews within registered project roots --- apps/server/src/review/ReviewService.test.ts | 222 ++++++++++++++++++- apps/server/src/review/ReviewService.ts | 27 +++ apps/server/src/server.ts | 1 + 3 files changed, 249 insertions(+), 1 deletion(-) diff --git a/apps/server/src/review/ReviewService.test.ts b/apps/server/src/review/ReviewService.test.ts index 0802bac2d73a..d0405e7b099f 100644 --- a/apps/server/src/review/ReviewService.test.ts +++ b/apps/server/src/review/ReviewService.test.ts @@ -5,7 +5,10 @@ import * as FileSystem from "effect/FileSystem"; import * as Layer from "effect/Layer"; import * as PlatformError from "effect/PlatformError"; +import { ProjectId } from "@t3tools/contracts"; +import { symlinksSupported } from "@t3tools/shared/testing/symlinks"; import * as ServerConfig from "../config.ts"; +import * as ProjectStore from "../orchestration-v2/ProjectStore.ts"; import * as ServerSettings from "../serverSettings.ts"; import * as GitVcsDriver from "../vcs/GitVcsDriver.ts"; import * as VcsDriverRegistry from "../vcs/VcsDriverRegistry.ts"; @@ -14,11 +17,37 @@ import * as ReviewService from "./ReviewService.ts"; function layer(input: { readonly workspaceRoot: string; readonly baseDir: string; + readonly registeredRoots?: ReadonlyArray; + readonly readProjects?: ProjectStore.ProjectStoreV2["Service"]["listShells"]; + readonly fileSystem?: FileSystem.FileSystem; readonly detectCalls?: Array<{ readonly cwd: string }>; readonly worktreesDirectory?: string; readonly previousWorktreesDirectories?: ReadonlyArray; }) { return ReviewService.layer.pipe( + Layer.provide( + Layer.mock(ProjectStore.ProjectStoreV2)({ + listShells: + input.readProjects ?? + (() => + Effect.succeed( + (input.registeredRoots ?? []).map((workspaceRoot, index) => ({ + id: ProjectId.make(`p${index}`), + title: "Registered", + workspaceRoot, + repositoryIdentity: null, + defaultThreadEnvMode: null, + autoPull: false, + faviconPath: null, + projectIcon: null, + defaultModelSelection: null, + scripts: [], + createdAt: "2026-07-01T00:00:00Z", + updatedAt: "2026-07-01T00:00:00Z", + })), + )), + }), + ), Layer.provide( Layer.mock(VcsDriverRegistry.VcsDriverRegistry)({ get: () => Effect.die("unexpected VCS registry get"), @@ -38,6 +67,11 @@ function layer(input: { }), ), Layer.provide(ServerConfig.layerTest(input.workspaceRoot, input.baseDir)), + Layer.provide( + input.fileSystem === undefined + ? Layer.empty + : Layer.succeed(FileSystem.FileSystem, input.fileSystem), + ), Layer.provideMerge(NodeServices.layer), ); } @@ -144,7 +178,16 @@ describe("ReviewService", () => { const result = yield* Effect.gen(function* () { const review = yield* ReviewService.ReviewService; return yield* review.getDiffPreview({ cwd: workspaceRoot }); - }).pipe(Effect.provide(layer({ workspaceRoot, baseDir, detectCalls }))); + }).pipe( + Effect.provide( + layer({ + workspaceRoot, + baseDir, + detectCalls, + readProjects: () => Effect.die("configured roots must not query project metadata"), + }), + ), + ); assert.strictEqual(result.cwd, workspaceRoot); assert.deepStrictEqual(result.sources, []); @@ -174,4 +217,181 @@ describe("ReviewService", () => { assert.deepStrictEqual(detectCalls, []); }).pipe(Effect.provide(NodeServices.layer)), ); + + it.effect("allows review previews within a live registered project outside the launch root", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const workspaceRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-launch-" }); + const projectRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-project-" }); + const baseDir = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-base-" }); + const nested = `${projectRoot}/nested`; + yield* fs.makeDirectory(nested); + const registeredRoots = [projectRoot]; + const detectCalls: Array<{ readonly cwd: string }> = []; + yield* Effect.gen(function* () { + const review = yield* ReviewService.ReviewService; + assert.deepStrictEqual((yield* review.getDiffPreview({ cwd: projectRoot })).sources, []); + assert.deepStrictEqual((yield* review.getDiffPreview({ cwd: nested })).sources, []); + assert.deepStrictEqual(detectCalls, [{ cwd: projectRoot }, { cwd: nested }]); + registeredRoots.length = 0; + const removed = yield* review.getDiffPreview({ cwd: projectRoot }).pipe(Effect.flip); + assert.strictEqual(removed._tag, "VcsRepositoryDetectionError"); + assert.strictEqual(detectCalls.length, 2); + }).pipe(Effect.provide(layer({ workspaceRoot, baseDir, registeredRoots, detectCalls }))); + }).pipe(Effect.provide(NodeServices.layer)), + ); + + it.effect("allows registered project roots through file-content workspace validation", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const workspaceRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-launch-" }); + const projectRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-project-" }); + const baseDir = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-base-" }); + const detectCalls: Array<{ readonly cwd: string }> = []; + const error = yield* Effect.gen(function* () { + const review = yield* ReviewService.ReviewService; + return yield* review + .getDiffFileContents({ + cwd: projectRoot, + sourceKind: "working-tree", + changeType: "change", + baseRef: "HEAD", + headRef: null, + oldPath: "file.ts", + newPath: "file.ts", + }) + .pipe(Effect.flip); + }).pipe( + Effect.provide( + layer({ workspaceRoot, baseDir, registeredRoots: [projectRoot], detectCalls }), + ), + ); + // The fixture has no Git repository: validation succeeds and detection decides the error. + assert.strictEqual(error._tag, "VcsUnsupportedOperationError"); + assert.deepStrictEqual(detectCalls, [{ cwd: projectRoot }]); + }).pipe(Effect.provide(NodeServices.layer)), + ); + + it.effect.skipIf(!symlinksSupported)("rejects symlink escapes from registered review roots", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const workspaceRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-launch-" }); + const projectRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-project-" }); + const outsideRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-outside-" }); + const baseDir = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-base-" }); + yield* fs.symlink(outsideRoot, `${projectRoot}/escape`); + const detectCalls: Array<{ readonly cwd: string }> = []; + const error = yield* Effect.gen(function* () { + const review = yield* ReviewService.ReviewService; + return yield* review.getDiffPreview({ cwd: `${projectRoot}/escape` }).pipe(Effect.flip); + }).pipe( + Effect.provide( + layer({ workspaceRoot, baseDir, registeredRoots: [projectRoot], detectCalls }), + ), + ); + assert.strictEqual(error._tag, "VcsRepositoryDetectionError"); + assert.deepStrictEqual(detectCalls, []); + }).pipe(Effect.provide(NodeServices.layer)), + ); + + it.effect("rejects traversal, sibling, and ancestor paths around registered review roots", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const workspaceRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-launch-" }); + const parent = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-parent-" }); + const baseDir = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-base-" }); + const projectRoot = `${parent}/project`; + const sibling = `${parent}/project-sibling`; + yield* fs.makeDirectory(projectRoot); + yield* fs.makeDirectory(sibling); + const detectCalls: Array<{ readonly cwd: string }> = []; + yield* Effect.gen(function* () { + const review = yield* ReviewService.ReviewService; + for (const cwd of [`${projectRoot}/../project-sibling`, sibling, parent]) { + const error = yield* review.getDiffPreview({ cwd }).pipe(Effect.flip); + assert.strictEqual(error._tag, "VcsRepositoryDetectionError"); + } + assert.deepStrictEqual(detectCalls, []); + }).pipe( + Effect.provide( + layer({ workspaceRoot, baseDir, registeredRoots: [projectRoot], detectCalls }), + ), + ); + }).pipe(Effect.provide(NodeServices.layer)), + ); + + it.effect("fails closed on project-store errors while preserving the boundary error", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const workspaceRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-launch-" }); + const projectRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-project-" }); + const baseDir = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-base-" }); + const failure = new ProjectStore.ProjectStoreV2Error({ + operation: "list", + cause: "database unavailable", + }); + const detectCalls: Array<{ readonly cwd: string }> = []; + const error = yield* Effect.gen(function* () { + const review = yield* ReviewService.ReviewService; + return yield* review.getDiffPreview({ cwd: projectRoot }).pipe(Effect.flip); + }).pipe( + Effect.provide( + layer({ + workspaceRoot, + baseDir, + readProjects: () => Effect.fail(failure), + detectCalls, + }), + ), + ); + assert.strictEqual(error._tag, "VcsRepositoryDetectionError"); + if (error._tag !== "VcsRepositoryDetectionError") return; + assert.strictEqual(error.operation, "ReviewService.getDiffPreview"); + assert.match(error.detail, /must stay within the configured workspace root/); + assert.deepStrictEqual(detectCalls, []); + }).pipe(Effect.provide(NodeServices.layer)), + ); + + it.effect("an unreadable registered root does not block another project or grant access", () => + Effect.gen(function* () { + const fs = yield* FileSystem.FileSystem; + const workspaceRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-launch-" }); + const blockedRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-blocked-" }); + const projectRoot = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-project-" }); + const baseDir = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-base-" }); + const blockedNested = `${blockedRoot}/nested`; + yield* fs.makeDirectory(blockedNested); + const detectCalls: Array<{ readonly cwd: string }> = []; + yield* Effect.gen(function* () { + const review = yield* ReviewService.ReviewService; + yield* review.getDiffPreview({ cwd: projectRoot }); + const rejected = yield* review.getDiffPreview({ cwd: blockedNested }).pipe(Effect.flip); + assert.strictEqual(rejected._tag, "VcsRepositoryDetectionError"); + assert.deepStrictEqual(detectCalls, [{ cwd: projectRoot }]); + }).pipe( + Effect.provide( + layer({ + workspaceRoot, + baseDir, + registeredRoots: [blockedRoot, projectRoot], + detectCalls, + fileSystem: { + ...fs, + realPath: (path) => + path === blockedRoot + ? Effect.fail( + PlatformError.systemError({ + _tag: "PermissionDenied", + module: "FileSystem", + method: "realPath", + pathOrDescriptor: path, + }), + ) + : fs.realPath(path), + }, + }), + ), + ); + }).pipe(Effect.provide(NodeServices.layer)), + ); }); diff --git a/apps/server/src/review/ReviewService.ts b/apps/server/src/review/ReviewService.ts index 7aee5847001f..cb0a2822b67c 100644 --- a/apps/server/src/review/ReviewService.ts +++ b/apps/server/src/review/ReviewService.ts @@ -16,6 +16,7 @@ import { } from "@t3tools/contracts"; import * as ServerConfig from "../config.ts"; +import * as ProjectStore from "../orchestration-v2/ProjectStore.ts"; import * as GitVcsDriver from "../vcs/GitVcsDriver.ts"; import * as VcsDriverRegistry from "../vcs/VcsDriverRegistry.ts"; import * as ServerSettings from "../serverSettings.ts"; @@ -41,6 +42,7 @@ export const make = Effect.gen(function* () { const vcsRegistry = yield* VcsDriverRegistry.VcsDriverRegistry; const git = yield* GitVcsDriver.GitVcsDriver; const settings = yield* ServerSettings.ServerSettingsService; + const projectStore = yield* ProjectStore.ProjectStoreV2; const canonicalizePath = (value: string) => { const resolvedPath = path.resolve(value); @@ -95,6 +97,31 @@ export const make = Effect.gen(function* () { return; } + // Registered projects can live outside the server's launch cwd. Read active + // project metadata only when neither configured root covers the request. + const projects = yield* projectStore.listShells().pipe( + Effect.catch((cause) => + Effect.logWarning("Failed to read registered project roots for review validation", { + operation, + cwd, + cause, + }).pipe(Effect.as([])), + ), + ); + for (const project of projects) { + const root = yield* canonicalizePath(project.workspaceRoot).pipe( + Effect.catch((cause) => + Effect.logWarning("Failed to resolve a registered project root for review validation", { + workspaceRoot: project.workspaceRoot, + cause, + }).pipe(Effect.as(null)), + ), + ); + if (root !== null && isWithinRoot(candidate, root)) { + return; + } + } + return yield* new VcsRepositoryDetectionError({ operation, cwd, diff --git a/apps/server/src/server.ts b/apps/server/src/server.ts index 1294e6bdcf32..b6c8ae159bd3 100644 --- a/apps/server/src/server.ts +++ b/apps/server/src/server.ts @@ -352,6 +352,7 @@ const layerProjectCloneTracker = ProjectCloneTracker.layer.pipe( ); const layerReview = ReviewService.layer.pipe( + Layer.provide(ProjectStore.layer), Layer.provideMerge(GitVcsDriver.layer), Layer.provideMerge(layerVcsDriverRegistry), ); From 67ece9a46c7ed9c4c5a0099757f35c3a557b2380 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Tue, 6 Oct 2026 23:02:15 +1100 Subject: [PATCH 2/2] fix(server): accept review cwds under a child whose name starts with .. Co-Authored-By: Claude Opus 5.5 (1M context) --- apps/server/src/review/ReviewService.test.ts | 12 ++++++++++-- apps/server/src/review/ReviewService.ts | 6 +++++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/apps/server/src/review/ReviewService.test.ts b/apps/server/src/review/ReviewService.test.ts index d0405e7b099f..3f02d9101446 100644 --- a/apps/server/src/review/ReviewService.test.ts +++ b/apps/server/src/review/ReviewService.test.ts @@ -226,17 +226,25 @@ describe("ReviewService", () => { const baseDir = yield* fs.makeTempDirectoryScoped({ prefix: "t3-review-base-" }); const nested = `${projectRoot}/nested`; yield* fs.makeDirectory(nested); + // A child name that starts with `..` is still inside the root. + const dotted = `${projectRoot}/..cache`; + yield* fs.makeDirectory(dotted); const registeredRoots = [projectRoot]; const detectCalls: Array<{ readonly cwd: string }> = []; yield* Effect.gen(function* () { const review = yield* ReviewService.ReviewService; assert.deepStrictEqual((yield* review.getDiffPreview({ cwd: projectRoot })).sources, []); assert.deepStrictEqual((yield* review.getDiffPreview({ cwd: nested })).sources, []); - assert.deepStrictEqual(detectCalls, [{ cwd: projectRoot }, { cwd: nested }]); + assert.deepStrictEqual((yield* review.getDiffPreview({ cwd: dotted })).sources, []); + assert.deepStrictEqual(detectCalls, [ + { cwd: projectRoot }, + { cwd: nested }, + { cwd: dotted }, + ]); registeredRoots.length = 0; const removed = yield* review.getDiffPreview({ cwd: projectRoot }).pipe(Effect.flip); assert.strictEqual(removed._tag, "VcsRepositoryDetectionError"); - assert.strictEqual(detectCalls.length, 2); + assert.strictEqual(detectCalls.length, 3); }).pipe(Effect.provide(layer({ workspaceRoot, baseDir, registeredRoots, detectCalls }))); }).pipe(Effect.provide(NodeServices.layer)), ); diff --git a/apps/server/src/review/ReviewService.ts b/apps/server/src/review/ReviewService.ts index cb0a2822b67c..301fd758c6b4 100644 --- a/apps/server/src/review/ReviewService.ts +++ b/apps/server/src/review/ReviewService.ts @@ -65,7 +65,11 @@ export const make = Effect.gen(function* () { const isWithinRoot = (candidate: string, root: string) => { const relative = path.relative(root, candidate); - return relative === "" || (!relative.startsWith("..") && !path.isAbsolute(relative)); + // A child named like `..cache` is inside the root; only a `..` segment leaves it. + return ( + relative === "" || + (relative !== ".." && !relative.startsWith(`..${path.sep}`) && !path.isAbsolute(relative)) + ); }; const assertWorkspaceBoundCwd = Effect.fn("ReviewService.assertWorkspaceBoundCwd")(function* (