diff --git a/server/src/__tests__/environment-selection-route-guards.test.ts b/server/src/__tests__/environment-selection-route-guards.test.ts index 195753a126..26e1fec3bc 100644 --- a/server/src/__tests__/environment-selection-route-guards.test.ts +++ b/server/src/__tests__/environment-selection-route-guards.test.ts @@ -126,6 +126,7 @@ function buildApp(routerFactory: (app: express.Express) => void) { (req as any).actor = { type: "board", userId: "user-1", + companyIds: ["company-1"], source: "local_implicit", }; next(); @@ -179,6 +180,10 @@ describe.sequential("execution environment route guards", () => { mockProjectService.createWorkspace.mockReset(); mockProjectService.remove.mockReset(); mockProjectService.resolveByReference.mockReset(); + mockProjectService.resolveByReference.mockResolvedValue({ + ambiguous: false, + project: { id: "project-1" }, + }); mockProjectService.listWorkspaces.mockReset(); mockIssueService.create.mockReset(); mockIssueService.getById.mockReset(); diff --git a/server/src/__tests__/project-get-reference-routes.test.ts b/server/src/__tests__/project-get-reference-routes.test.ts new file mode 100644 index 0000000000..99a8362563 --- /dev/null +++ b/server/src/__tests__/project-get-reference-routes.test.ts @@ -0,0 +1,136 @@ +import { randomUUID } from "node:crypto"; +import express from "express"; +import request from "supertest"; +import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest"; +import { companies, createDb, projects } from "@paperclipai/db"; +import { + getEmbeddedPostgresTestSupport, + startEmbeddedPostgresTestDatabase, +} from "./helpers/embedded-postgres.js"; +import { errorHandler } from "../middleware/index.js"; +import { projectRoutes } from "../routes/projects.js"; + +const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); +const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; + +if (!embeddedPostgresSupport.supported) { + console.warn( + `Skipping embedded Postgres project reference tests on this host: ${ + embeddedPostgresSupport.reason ?? "unsupported environment" + }`, + ); +} + +function boardActor(companyId: string): Express.Request["actor"] { + return { + type: "board", + userId: "user-1", + source: "session", + isInstanceAdmin: true, + companyIds: [companyId], + memberships: [{ companyId, membershipRole: "admin", status: "active" }], + }; +} + +function createApp(db: ReturnType, actor: Express.Request["actor"]) { + const app = express(); + app.use(express.json()); + app.use((req, _res, next) => { + req.actor = actor; + next(); + }); + app.use("/api", projectRoutes(db)); + app.use(errorHandler); + return app; +} + +describeEmbeddedPostgres("GET /projects/:id reference resolution", () => { + let db!: ReturnType; + let tempDb: Awaited> | null = null; + + beforeAll(async () => { + tempDb = await startEmbeddedPostgresTestDatabase("paperclip-project-get-reference-"); + db = createDb(tempDb.connectionString); + }, 20_000); + + afterEach(async () => { + await db.delete(projects); + await db.delete(companies); + }); + + afterAll(async () => { + await tempDb?.cleanup(); + }); + + async function seed() { + const companyId = randomUUID(); + const projectId = randomUUID(); + await db.insert(companies).values({ + id: companyId, + name: "Paperclip", + issuePrefix: `T${companyId.replace(/-/g, "").slice(0, 6).toUpperCase()}`, + requireBoardApprovalForNewAgents: false, + }); + await db.insert(projects).values({ + id: projectId, + companyId, + name: "Demo Project", + status: "in_progress", + }); + return { companyId, projectId }; + } + + it("resolves a project shortname to the project when companyId is given", async () => { + const { companyId, projectId } = await seed(); + const app = createApp(db, boardActor(companyId)); + + const res = await request(app).get(`/api/projects/demo-project?companyId=${companyId}`); + + expect(res.status).toBe(200); + expect(res.body.id).toBe(projectId); + }); + + it("returns 404 instead of 500 for an unknown shortname", async () => { + const { companyId } = await seed(); + const app = createApp(db, boardActor(companyId)); + + const res = await request(app).get(`/api/projects/no-such-project?companyId=${companyId}`); + + expect(res.status).toBe(404); + }); + + it("returns 404 instead of 500 for a non-uuid ref without company context", async () => { + await seed(); + const app = createApp(db, { + type: "board", + userId: "user-1", + source: "session", + isInstanceAdmin: true, + companyIds: [], + memberships: [], + }); + + const res = await request(app).get("/api/projects/demo-project"); + + expect(res.status).toBe(404); + }); + + it("resolves a shortname for a single-company actor without ?companyId=", async () => { + const { companyId, projectId } = await seed(); + const app = createApp(db, boardActor(companyId)); + + const res = await request(app).get("/api/projects/demo-project"); + + expect(res.status).toBe(200); + expect(res.body.id).toBe(projectId); + }); + + it("still returns 404 for an unknown uuid", async () => { + const { companyId } = await seed(); + const app = createApp(db, boardActor(companyId)); + + const res = await request(app).get(`/api/projects/${randomUUID()}?companyId=${companyId}`); + + expect(res.status).toBe(404); + }); +}); diff --git a/server/src/__tests__/project-routes-env.test.ts b/server/src/__tests__/project-routes-env.test.ts index 25072e1491..133a72ad4e 100644 --- a/server/src/__tests__/project-routes-env.test.ts +++ b/server/src/__tests__/project-routes-env.test.ts @@ -163,7 +163,7 @@ describe("project env routes", () => { explanation: "Allowed by test mock.", }); mockGetTelemetryClient.mockReturnValue({ track: vi.fn() }); - mockProjectService.resolveByReference.mockResolvedValue({ ambiguous: false, project: null }); + mockProjectService.resolveByReference.mockResolvedValue({ ambiguous: false, project: buildProject() }); mockProjectService.createWorkspace.mockResolvedValue(null); mockProjectService.listWorkspaces.mockResolvedValue([]); mockEnvironmentService.getById.mockReset(); diff --git a/server/src/__tests__/project-workspace-managed-sandbox-routes.test.ts b/server/src/__tests__/project-workspace-managed-sandbox-routes.test.ts index 9dfc578172..c73d82e567 100644 --- a/server/src/__tests__/project-workspace-managed-sandbox-routes.test.ts +++ b/server/src/__tests__/project-workspace-managed-sandbox-routes.test.ts @@ -208,7 +208,7 @@ describe("project workspace host-path floor", () => { explanation: "Allowed by test mock.", }); mockGetTelemetryClient.mockReturnValue({ track: vi.fn() }); - mockProjectService.resolveByReference.mockResolvedValue({ ambiguous: false, project: null }); + mockProjectService.resolveByReference.mockResolvedValue({ ambiguous: false, project: buildProject() }); mockProjectService.getById.mockResolvedValue(buildProject()); mockProjectService.create.mockResolvedValue(buildProject()); mockProjectService.createWorkspace.mockResolvedValue(buildWorkspace()); diff --git a/server/src/routes/projects.ts b/server/src/routes/projects.ts index acbccf5bd1..dd0f09ac69 100644 --- a/server/src/routes/projects.ts +++ b/server/src/routes/projects.ts @@ -22,7 +22,7 @@ import type { WorkspaceRuntimeDesiredState, WorkspaceRuntimeServiceStateMap } fr import { trackProjectCreated } from "@paperclipai/shared/telemetry"; import { validate } from "../middleware/validate.js"; import { accessService, projectService, logActivity, workspaceOperationService } from "../services/index.js"; -import { conflict, forbidden, unprocessable } from "../errors.js"; +import { conflict, forbidden, notFound, unprocessable } from "../errors.js"; import { externalObjectService } from "../services/external-objects.js"; import { instanceSettingsService } from "../services/instance-settings.js"; import { assertBoard, assertCompanyAccess, getAccessibleResource, getActorInfo } from "./authz.js"; @@ -128,18 +128,23 @@ export function projectRoutes(db: Db) { if (req.actor.type === "agent" && req.actor.companyId) { return req.actor.companyId; } - return null; + // A single-company actor (the common self-hosted case) has an unambiguous + // company context without a `?companyId=` query — shortnames only resolve + // inside one company anyway, so require exactly one. + const actorCompanyIds = req.actor.companyIds ?? []; + return actorCompanyIds.length === 1 ? actorCompanyIds[0] : null; } async function normalizeProjectReference(req: Request, rawId: string) { if (isUuidLike(rawId)) return rawId; const companyId = await resolveCompanyIdForProjectReference(req); - if (!companyId) return rawId; + if (!companyId) throw notFound("Project not found"); const resolved = await svc.resolveByReference(companyId, rawId); if (resolved.ambiguous) { throw conflict("Project shortname is ambiguous in this company. Use the project ID."); } - return resolved.project?.id ?? rawId; + if (!resolved.project) throw notFound("Project not found"); + return resolved.project.id; } async function assertProjectReadAllowed(req: Request, res: Response, project: { id: string; companyId: string }) {