Merge the updated legacy cleanup (#109, with main) into the Phase 2 integration branch
The onboarding scan keeps the consent-first rewrite. Since main's folder check moved out of wireProjectWorkspace, applying a scan now checks that each chosen project is still a folder (not only inside the scan folder) before anything is written; a discovery test covers a project replaced by a file after the scan. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MRtGG9ARfqqhma3t85Ro2u
This commit is contained in:
commit
adb4dff052
@ -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<string> {
|
||||
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);
|
||||
|
||||
@ -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,
|
||||
|
||||
@ -72,7 +72,7 @@ describe("project workspace wiring", () => {
|
||||
|
||||
const wired = await wireProjectWorkspace(prisma as unknown as Pick<PrismaClient, "space">, {
|
||||
spaceId: "space-1",
|
||||
workspacePath: root,
|
||||
workspacePath: await resolveProjectWorkspaceDirectory(root),
|
||||
});
|
||||
expect(wired.path).toBe(realpathSync(root));
|
||||
expect(wired.git).toEqual({
|
||||
|
||||
@ -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<PrismaClient, "space">,
|
||||
input: { spaceId: string; workspacePath: string },
|
||||
): Promise<WiredProjectWorkspace> {
|
||||
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
|
||||
? {
|
||||
|
||||
@ -2267,15 +2267,25 @@ describeWithDatabase("API authorization and resource isolation", () => {
|
||||
const legacyNavigation = await rpc<SpaceNavigation>(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<Space>(app, owner, "spaces/create", {
|
||||
name: "Owner folder",
|
||||
workspacePath: hostFolder,
|
||||
});
|
||||
expect(linked.workspace?.path).toBe(hostFolder);
|
||||
const ownerNavigation = await rpc<SpaceNavigation>(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<Bot>(
|
||||
app,
|
||||
owner,
|
||||
|
||||
@ -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<UserProfileState>(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");
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user