diff --git a/apps/api/src/onboarding-scan.ts b/apps/api/src/onboarding-scan.ts index 504cc84..9fa0fe1 100644 --- a/apps/api/src/onboarding-scan.ts +++ b/apps/api/src/onboarding-scan.ts @@ -22,6 +22,7 @@ import { InvalidProjectWorkspacePathError, InvalidSpaceNameError, InvalidUserFactError, + resolveProjectWorkspaceDirectory, SpaceLimitError, updateUserProfile, wireProjectWorkspace, @@ -336,12 +337,7 @@ const MISSING_FOLDER = "That folder does not exist or is not readable."; /** Resolves a found project again, so a folder swapped for a symlink cannot leave the scan folder. */ async function confinedProjectFolder(home: string, relative: string): Promise { - let folder: string; - try { - folder = await realpath(path.join(home, relative)); - } catch { - throw new InvalidProjectWorkspacePathError(MISSING_FOLDER); - } + const folder = await resolveProjectWorkspaceDirectory(path.join(home, relative)); const inside = path.relative(home, folder); if (!inside || inside === ".." || inside.startsWith(`..${path.sep}`) || path.isAbsolute(inside)) { throw new InvalidProjectWorkspacePathError(MISSING_FOLDER); diff --git a/apps/api/src/router.ts b/apps/api/src/router.ts index 859725b..1d567e3 100644 --- a/apps/api/src/router.ts +++ b/apps/api/src/router.ts @@ -142,6 +142,7 @@ import { removePushSubscriptionsOfKind, renewSpaceDeletionClaim, requireMembership, + resolveProjectWorkspaceDirectory, SPACE_DELETION_CLAIM_TIMEOUT_MS, SpaceDeletionInProgressError, SpaceLimitError, @@ -649,6 +650,18 @@ export function createRouter(deps: RouterDeps) { if (input.workspacePath && !context.actor.isDeploymentOwner) { throw new ORPCError("FORBIDDEN"); } + // Check the folder before the space exists, so a bad path leaves no space behind. + let workspacePath: string | null = null; + if (input.workspacePath) { + try { + workspacePath = await resolveProjectWorkspaceDirectory(input.workspacePath); + } catch (error) { + if (error instanceof InvalidProjectWorkspacePathError) { + throw new ORPCError("BAD_REQUEST", { message: error.message }); + } + throw error; + } + } let space: { id: string; name: string }; try { space = await createSpaceForMember(deps.prisma, { @@ -662,20 +675,9 @@ export function createRouter(deps: RouterDeps) { } throw error; } - let workspace = null; - if (input.workspacePath) { - try { - workspace = await wireProjectWorkspace(deps.prisma, { - spaceId: space.id, - workspacePath: input.workspacePath, - }); - } catch (error) { - if (error instanceof InvalidProjectWorkspacePathError) { - throw new ORPCError("BAD_REQUEST", { message: error.message }); - } - throw error; - } - } + const workspace = workspacePath + ? await wireProjectWorkspace(deps.prisma, { spaceId: space.id, workspacePath }) + : null; return { id: space.id, name: space.name, diff --git a/packages/db/src/project-workspace.test.ts b/packages/db/src/project-workspace.test.ts index 75f0693..1a20e8c 100644 --- a/packages/db/src/project-workspace.test.ts +++ b/packages/db/src/project-workspace.test.ts @@ -72,7 +72,7 @@ describe("project workspace wiring", () => { const wired = await wireProjectWorkspace(prisma as unknown as Pick, { spaceId: "space-1", - workspacePath: root, + workspacePath: await resolveProjectWorkspaceDirectory(root), }); expect(wired.path).toBe(realpathSync(root)); expect(wired.git).toEqual({ diff --git a/packages/db/src/project-workspace.ts b/packages/db/src/project-workspace.ts index 50b3c5f..fd0c04a 100644 --- a/packages/db/src/project-workspace.ts +++ b/packages/db/src/project-workspace.ts @@ -50,18 +50,17 @@ export interface WiredProjectWorkspace { } | null; } -/** Persist a host folder and optional git metadata on a newly created space. */ +/** Persist a folder from `resolveProjectWorkspaceDirectory`, and its git metadata, on a new space. */ export async function wireProjectWorkspace( prisma: Pick, input: { spaceId: string; workspacePath: string }, ): Promise { - const resolved = await resolveProjectWorkspaceDirectory(input.workspacePath); - const git = await readGitCheckout(resolved); + const git = await readGitCheckout(input.workspacePath); await prisma.space.update({ where: { id: input.spaceId }, data: { - workspacePath: resolved, + workspacePath: input.workspacePath, gitRemoteUrl: git.present && git.remoteUrl ? git.remoteUrl : null, gitDefaultBranch: git.present && git.defaultBranch ? git.defaultBranch : null, gitHeadCommit: git.present ? git.headCommit : null, @@ -69,7 +68,7 @@ export async function wireProjectWorkspace( }); return { - path: resolved, + path: input.workspacePath, git: git.present && git.remoteUrl && git.defaultBranch ? { diff --git a/packages/testkit/src/authorization.test.ts b/packages/testkit/src/authorization.test.ts index c0d98a3..6c3de1d 100644 --- a/packages/testkit/src/authorization.test.ts +++ b/packages/testkit/src/authorization.test.ts @@ -2267,15 +2267,25 @@ describeWithDatabase("API authorization and resource isolation", () => { const legacyNavigation = await rpc(app, member, "spaces/list"); expect(legacyNavigation.spaces.find((space) => space.id === legacy.id)?.workspace).toBeNull(); + // A bad folder is rejected before the project exists, so fixing it and retrying leaves one. + for (const badPath of [path.join(hostFolder, "missing"), path.join(hostFolder, ".env")]) { + const rejected = await raw(app, owner, "spaces/create", { + name: "Owner folder", + workspacePath: badPath, + }); + expect(rejected.status, badPath).toBe(400); + } const linked = await rpc(app, owner, "spaces/create", { name: "Owner folder", workspacePath: hostFolder, }); expect(linked.workspace?.path).toBe(hostFolder); const ownerNavigation = await rpc(app, owner, "spaces/list"); - expect(ownerNavigation.spaces.find((space) => space.id === linked.id)?.workspace?.path).toBe( - hostFolder, - ); + expect( + ownerNavigation.spaces + .filter((space) => space.name === "Owner folder") + .map((space) => space.workspace?.path), + ).toEqual([hostFolder]); const ownerBot = await rpc( app, owner, diff --git a/packages/testkit/src/onboarding-discovery.postgres.test.ts b/packages/testkit/src/onboarding-discovery.postgres.test.ts index e1656bc..68c3289 100644 --- a/packages/testkit/src/onboarding-discovery.postgres.test.ts +++ b/packages/testkit/src/onboarding-discovery.postgres.test.ts @@ -1,5 +1,5 @@ import { mkdtempSync, realpathSync, rmSync, symlinkSync } from "node:fs"; -import { mkdir, rm } from "node:fs/promises"; +import { mkdir, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import path from "node:path"; import { @@ -264,6 +264,30 @@ describeWithDatabase("onboarding discovery through the API", () => { }); }); + it("refuses a chosen project whose folder became a file and writes nothing", async () => { + const owner = await account("discovery-file"); + await makeDeploymentOwner(owner.actor); + const garden = path.join(home, "code/garden"); + await rm(garden, { recursive: true, force: true }); + await writeOnboardingScanFixture(home); + const scan = await finishedScan(owner.cookie); + await rm(garden, { recursive: true, force: true }); + await writeFile(garden, "not a folder\n"); + + const response = await raw(app, owner.cookie, "onboarding/applyScan", { + scanId: scan.scanId, + projectIds: ["project-0", "project-1"], + factIds: ["language:Python"], + }); + expect(response.status).toBe(400); + expect(await handles.prisma.spaceMember.count({ where: { userId: owner.actor.userId } })).toBe( + 1, + ); + await expect(rpc(app, owner.cookie, "profile/get")).resolves.toMatchObject({ + facts: [], + }); + }); + it("remembers each connected app once as a tools fact and keeps it after revoking", async () => { const user = await account("discovery-apps"); const other = await account("discovery-apps-other");