From 23a391d638ccfa3d58904676df43989f13734710 Mon Sep 17 00:00:00 2001 From: marcorivm Date: Fri, 7 Aug 2026 09:05:48 -0600 Subject: [PATCH] =?UTF-8?q?feat(api):=20POST=20/v1/projects=20=E2=80=94=20?= =?UTF-8?q?create=20a=20project,=20owned=20by=20its=20creator?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slice 2 of docs/project-lifecycle.md. Projects could only come into existence implicitly — bootstrapOrganization, joinSharedOrganization and ensureMemberDefaultProject — so a user could never deliberately make one. The owner binding is a NESTED create, not a follow-up write, so it lands in the same statement as the project row. That is Guard G's precondition: a project with no owner binding can only ever be managed by an org admin, so a create that succeeded with a binding that failed would strand an orphan. A test proves the end-to-end consequence — a plain member creates a project and can immediately rename it, with no admin involved. Seeded like every other creation site (defaultProjectSeed: an API key and a default agent) so the project is usable the moment it exists, with best-effort policy seeding for the same reason those sites treat it as best-effort. Slug collisions are resolved silently, not surfaced: project NAMES are deliberately non-unique (projectNameSchema), so two projects called "Alpha" must both be creatable and only the slug has to differ — "alpha" is taken, so they become alpha-2 and alpha-3. A P2002 from a concurrent create retries once with a random tail. MAX_PROJECTS_PER_ORG caps growth at 100. Enforced count-then-create without a lock, so a concurrent burst can overshoot by a few — deliberate: the cap bounds runaway growth rather than being exact, and a lock on a cold path costs more than the overshoot. Authorization lives in the service, not the route, because there is no resource to resolve yet: any active member may create, and a caller with no role is refused. Note that gate is defence-in-depth rather than route-reachable — a suspended session resolves no context and an org key whose user lost membership fails key auth, so both 401 before the handler. The test asserts that real behaviour rather than the 403 I first expected. 12 new cases; 1168 passing across @onecli/api. --- docs/project-lifecycle.md | 4 +- packages/api/src/routes/org/projects.test.ts | 211 +++++++++++++++++++ packages/api/src/routes/org/projects.ts | 30 +++ packages/api/src/services/project-service.ts | 131 +++++++++++- packages/api/src/validations/project.ts | 15 ++ 5 files changed, 388 insertions(+), 3 deletions(-) diff --git a/docs/project-lifecycle.md b/docs/project-lifecycle.md index 2f44f0fd..30d6a528 100644 --- a/docs/project-lifecycle.md +++ b/docs/project-lifecycle.md @@ -8,8 +8,8 @@ | # | Slice | Size | State | | --- | ----------------------------- | ---- | ---------------------------------------- | -| 1 | `GET /v1/projects` — list | S | in progress | -| 2 | `POST /v1/projects` — create | M | not started | +| 1 | `GET /v1/projects` — list | S | PR open | +| 2 | `POST /v1/projects` — create | M | PR open | | 3 | Web: switcher + create dialog | M | not started | | 4 | Org switching (follow-up) | M–L | not started, blocked on v1.45.0 adoption | diff --git a/packages/api/src/routes/org/projects.test.ts b/packages/api/src/routes/org/projects.test.ts index 8a39e75f..6e2a91a0 100644 --- a/packages/api/src/routes/org/projects.test.ts +++ b/packages/api/src/routes/org/projects.test.ts @@ -1,6 +1,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import type { Hono } from "hono"; import type { ApiEnv } from "../../types"; +import { MAX_PROJECTS_PER_ORG } from "../../validations/project"; // `/v1/projects` end-to-end through the real app: the OSS routes mounted on // the `eeRoutes` seam, the OSS role resolver wired as the RoleResolver, and @@ -154,6 +155,7 @@ vi.mock("@onecli/db", () => { interface ProjectWhere { id?: string | StringFilter; organizationId?: string; + slug?: { startsWith: string }; createdByUserId?: string; organization?: { members: { some: { userId: string; status?: { not?: string } } }; @@ -161,6 +163,17 @@ vi.mock("@onecli/db", () => { accessBindings?: { some: { OR: BindingClause[] } }; OR?: ProjectWhere[]; } + interface ProjectCreateData { + id: string; + name?: string | null; + slug?: string | null; + organizationId: string; + createdByUserId?: string | null; + createdByUserEmail?: string | null; + accessBindings?: { create: { userId: string; role: string } }; + apiKeys?: { create: { key: string } }; + agents?: { create: unknown }; + } interface AccessWhere { projectId?: string; userId?: string | StringFilter | null; @@ -219,6 +232,11 @@ vi.mock("@onecli/db", () => { row.createdByUserId !== where.createdByUserId ) return false; + if ( + where.slug?.startsWith !== undefined && + !(row.slug ?? "").startsWith(where.slug.startsWith) + ) + return false; if (where.organization) { const { userId, status } = where.organization.members.some; const membership = store.members.find( @@ -478,6 +496,65 @@ vi.mock("@onecli/db", () => { return picked; }); }, + // Mirrors the nested create `createProject` issues: the project row plus + // its owner binding, api key and default agent land together, which is + // exactly the atomicity Guard G depends on. + create: async ({ + data, + select, + }: { + data: ProjectCreateData; + select?: Record; + }) => { + if ( + store.projects.some( + (p) => + p.organizationId === data.organizationId && p.slug === data.slug, + ) + ) { + // Prisma's @@unique([organizationId, slug]). + throw Object.assign(new Error("Unique constraint failed"), { + code: "P2002", + }); + } + const row: ProjectRow = { + id: data.id, + organizationId: data.organizationId, + name: data.name ?? null, + slug: data.slug ?? null, + createdByUserId: data.createdByUserId ?? null, + createdAt: new Date(), + }; + store.projects.push(row); + if (data.accessBindings?.create) { + const b = data.accessBindings.create; + store.projectAccess.push( + access( + `pa-new-${row.id}`, + row.id, + { userId: b.userId }, + b.role, + row.createdAt, + ), + ); + } + if (data.apiKeys?.create) { + store.apiKeys.push({ + id: `k-new-${row.id}`, + projectId: row.id, + key: data.apiKeys.create.key, + }); + } + if (data.agents?.create) { + store.agents.push({ id: `ag-new-${row.id}`, projectId: row.id }); + } + if (!select) return { ...row }; + const picked: Record = {}; + for (const key of Object.keys(select)) { + if (select[key]) picked[key] = row[key as keyof ProjectRow]; + } + return picked; + }, count: async ({ where }: { where: ProjectWhere }) => store.projects.filter((p) => matchesProject(p, where)).length, updateMany: async ({ @@ -1095,6 +1172,140 @@ describe("GET /projects (list)", () => { }); }); +describe("POST /projects (create)", () => { + const create = (body: unknown, init: RequestInit = asAdmin) => + app.request("/v1/projects", { + ...init, + method: "POST", + body: JSON.stringify(body), + }); + + it("401s an unauthenticated caller and 403s a project-scoped key", async () => { + expect((await create({ name: "New" }, {})).status).toBe(401); + expect((await create({ name: "New" }, asProjectKey)).status).toBe(403); + expect(store.projects).toHaveLength(5); + expect(store.audits).toHaveLength(0); + }); + + it("creates the project and seeds the caller as OWNER in the same write", async () => { + // Guard G's precondition: a project that exists without an owner binding + // can never be managed by anyone but an org admin. + store.sessionUserId = MEMBER; + const res = await create({ name: "Fresh Start" }, {}); + expect(res.status).toBe(201); + const body = (await res.json()) as ProjectBody; + + const row = projectRow(body.id); + expect(row?.organizationId).toBe(ORG); + expect(row?.createdByUserId).toBe(MEMBER); + expect(userBinding(body.id, MEMBER)?.role).toBe("owner"); + }); + + it("makes the new project immediately manageable by its creator", async () => { + // The end-to-end point of the owner seed: a plain member creates a project + // and can then rename it, with no admin involvement. + store.sessionUserId = MEMBER; + const body = (await ( + await create({ name: "Mine" }, {}) + ).json()) as ProjectBody; + expect((await patch(body.id, { name: "Renamed" }, {})).status).toBe(200); + expect(projectRow(body.id)?.name).toBe("Renamed"); + }); + + it("makes the new project appear in the creator's list", async () => { + store.sessionUserId = MEMBER; + const body = (await ( + await create({ name: "Listed" }, {}) + ).json()) as ProjectBody; + expect(await listIds({})).toContain(body.id); + }); + + it("derives a slug from the name", async () => { + const body = (await ( + await create({ name: "My New Project" }) + ).json()) as ProjectBody; + expect(projectRow(body.id)?.slug).toBe("my-new-project"); + }); + + it("disambiguates a colliding slug instead of failing — names are not unique", async () => { + // `projectNameSchema` deliberately allows duplicate names, so two projects + // called "Alpha" must both be creatable; only the slug has to differ. + const first = (await ( + await create({ name: "Alpha" }) + ).json()) as ProjectBody; + const second = (await ( + await create({ name: "Alpha" }) + ).json()) as ProjectBody; + // "alpha" is already taken by the seeded proj-1. + expect(projectRow(first.id)?.slug).toBe("alpha-2"); + expect(projectRow(second.id)?.slug).toBe("alpha-3"); + expect(projectRow(first.id)?.name).toBe("Alpha"); + }); + + it("falls back to a usable slug when the name has no slug characters", async () => { + const body = (await (await create({ name: "!!!" })).json()) as ProjectBody; + expect(projectRow(body.id)?.slug).toBe("project"); + }); + + it("422s an empty or missing name and writes nothing", async () => { + expect((await create({ name: "" })).status).toBe(422); + expect((await create({})).status).toBe(422); + expect((await create({ name: "x".repeat(101) })).status).toBe(422); + expect(store.projects).toHaveLength(5); + expect(store.audits).toHaveLength(0); + }); + + it("401s a caller with no active membership, and writes nothing", async () => { + // Rejected a layer earlier than `createProject`'s own gate: an org key + // whose user has lost their membership fails key authentication outright + // (the org-key branch re-checks role >= admin on every request), and a + // suspended session resolves neither project nor org. So the service's + // `if (!role) throw FORBIDDEN` is defence-in-depth for direct callers + // rather than a path this route reaches — kept so the service is correct + // on its own terms, not because the route depends on it. + store.members = store.members.filter((m) => m.userId !== ADMIN); + const res = await create({ name: "Nope" }); + expect(res.status).toBe(401); + expect(store.projects).toHaveLength(5); + expect(store.audits).toHaveLength(0); + }); + + it("409s at the per-org cap, counting only this org", async () => { + // Fill ORG to the ceiling; OTHER_ORG's project must not count toward it. + for (let n = store.projects.length; n < MAX_PROJECTS_PER_ORG + 1; n++) { + store.projects.push({ + id: `bulk-${n}`, + organizationId: ORG, + name: `Bulk ${n}`, + slug: `bulk-${n}`, + createdByUserId: ADMIN, + createdAt: at(100 + n), + }); + } + const res = await create({ name: "One Too Many" }); + expect(res.status).toBe(409); + }); + + it("audits the create with the new project's id and name", async () => { + const body = (await ( + await create({ name: "Audited" }) + ).json()) as ProjectBody; + const row = store.audits.at(-1); + expect(row?.action).toBe("create"); + expect(row?.projectId).toBe(body.id); + expect(row?.organizationId).toBe(ORG); + expect(row?.metadata).toMatchObject({ + projectId: body.id, + name: "Audited", + }); + }); + + it("flushes the gateway org cache — ProjectAccess is authorization data", async () => { + await create({ name: "Flushed" }); + expect(flushes.orgs).toContain(ORG); + }); +}); + describe("guard stack", () => { it("401s an unauthenticated caller on every route", async () => { expect((await get("proj-1", {})).status).toBe(401); diff --git a/packages/api/src/routes/org/projects.ts b/packages/api/src/routes/org/projects.ts index a40034e6..4369ad0b 100644 --- a/packages/api/src/routes/org/projects.ts +++ b/packages/api/src/routes/org/projects.ts @@ -6,6 +6,7 @@ import { ServiceError } from "../../services/errors"; import { parse } from "./parse"; import { canAccessProjectAsUser } from "../../middleware/auth/resolve"; import { + createProject, deleteProject, getProject, listProjects, @@ -18,6 +19,7 @@ import { setProjectAccess, } from "../../services/project-access-service"; import { + createProjectSchema, renameProjectSchema, setProjectAccessSchema, } from "../../validations/project"; @@ -113,6 +115,34 @@ export const ossProjectRoutes = () => { return c.json(await listProjects(auth.organizationId, auth.userId)); }); + // POST /projects — create, with the caller as owner. + // + // Like GET /, there is no id to resolve, so authorization lives in the + // service (`createProject` refuses a caller with no active role). The route + // stays thin: validate, call, audit. + app.post("/", async (c) => { + const auth = c.get("auth"); + const body = await c.req.json().catch(() => null); + const input = parse(createProjectSchema, body); + + const project = await withAudit( + () => + createProject( + auth.organizationId, + auth.userId, + auth.userEmail, + input.name, + ), + (created) => ({ + ...auditBase(c), + projectId: created.id, + action: AUDIT_ACTIONS.CREATE, + metadata: { projectId: created.id, name: created.name }, + }), + ); + return c.json(project, 201); + }); + // GET /projects/:projectId — the sharing page's name/slug source. Nothing // else in the API exposes a project's name (the session route returns only // `projectId`). diff --git a/packages/api/src/services/project-service.ts b/packages/api/src/services/project-service.ts index 9a7787f5..6725f88c 100644 --- a/packages/api/src/services/project-service.ts +++ b/packages/api/src/services/project-service.ts @@ -1,10 +1,19 @@ import { db } from "@onecli/db"; import { ServiceError } from "./errors"; -import { getRoleResolver, ROLE_HIERARCHY } from "../providers"; +import { + getRoleResolver, + ROLE_HIERARCHY, + getNewOrgPolicySeeder, +} from "../providers"; import { activeMembershipWhere, + defaultProjectSeed, hasResolvableProjectExcluding, + slugify, } from "./organization-service"; +import { generateProjectId } from "../lib/ids"; +import { logger } from "../lib/logger"; +import { MAX_PROJECTS_PER_ORG } from "../validations/project"; import { invalidateGatewayCacheForKeys } from "../lib/gateway-invalidate"; import { CAPS } from "../lib/env"; @@ -223,6 +232,126 @@ export const listProjects = async ( ).map(toProjectRow); }; +/** Prisma's unique-constraint code, same test as `org-group-service.ts`. */ +const isUniqueViolation = (err: unknown) => + typeof err === "object" && + err !== null && + (err as { code?: string }).code === "P2002"; + +/** + * A free slug for `name` within the org. Slugs are + * `@@unique([organizationId, slug])` but project NAMES are deliberately not + * unique (see `projectNameSchema`), so a collision here is an ordinary, + * expected state — never a user-facing error. Disambiguate silently. + */ +const freeSlug = async ( + organizationId: string, + name: string, +): Promise => { + const base = slugify(name) || "project"; + const taken = new Set( + ( + await db.project.findMany({ + where: { organizationId, slug: { startsWith: base } }, + select: { slug: true }, + }) + ).map((row) => row.slug), + ); + if (!taken.has(base)) return base; + for (let n = 2; n <= taken.size + 2; n++) { + const candidate = `${base}-${n}`; + if (!taken.has(candidate)) return candidate; + } + // Unreachable by construction (the loop bound exceeds the taken set), but a + // random tail is a safer fallback than throwing on a naming detail. + return `${base}-${generateProjectId().slice(0, 8)}`; +}; + +/** + * Create a project, with the caller as its `owner`. + * + * The owner binding is a NESTED create, not a follow-up write, so it lands in + * the same statement as the project row. That is Guard G's precondition: a + * project with no owner binding can never be renamed, shared or deleted by + * anyone but an org admin, so a create that succeeded and a binding that + * failed would leave an orphan no member could manage. + * + * Seeded like every other project-creation site (`bootstrapOrganization`, + * `ensureMemberDefaultProject`): an API key and a default agent, so the project + * is usable the moment it exists rather than being an empty shell. Policy + * seeding is best-effort for the same reason it is there — a seeding hiccup + * must not fail the create. + * + * Authorization is HERE rather than in the route because there is no resource + * to resolve yet. Any active member may create; a suspended member reads as no + * role and is refused, the same suspension invariant every other gate applies. + */ +export const createProject = async ( + organizationId: string, + userId: string, + userEmail: string, + name: string, +): Promise => { + if (CAPS.rbac) { + const resolver = getRoleResolver(); + const role = resolver + ? await resolver.getUserRole(userId, organizationId) + : null; + if (!role) { + throw new ServiceError( + "FORBIDDEN", + "You do not have permission to create a project.", + ); + } + } + + const count = await db.project.count({ where: { organizationId } }); + if (count >= MAX_PROJECTS_PER_ORG) { + throw new ServiceError( + "CONFLICT", + `This organization has reached its limit of ${MAX_PROJECTS_PER_ORG} projects.`, + ); + } + + const create = (slug: string) => + db.project.create({ + data: { + id: generateProjectId(), + name, + slug, + organizationId, + createdByUserId: userId, + createdByUserEmail: userEmail, + ...defaultProjectSeed(userId, userEmail), + accessBindings: { create: { userId, role: "owner" } }, + }, + select: projectSelect, + }); + + let row; + try { + row = await create(await freeSlug(organizationId, name)); + } catch (err) { + // A concurrent create took the slug between our read and our write. Retry + // once with a random tail; a second failure is a genuine error. + if (!isUniqueViolation(err)) throw err; + row = await create( + `${slugify(name) || "project"}-${generateProjectId().slice(0, 8)}`, + ); + } + + try { + await getNewOrgPolicySeeder().seed(organizationId, row.id); + } catch (err) { + logger.warn( + { err, organizationId, projectId: row.id }, + "created project policy seed failed", + ); + } + + return toProjectRow(row); +}; + /** * Rename. `name` ONLY — `slug` is immutable (it is write-only provenance, * never read by api/web/gateway, and it is `@@unique([organizationId, slug])`, diff --git a/packages/api/src/validations/project.ts b/packages/api/src/validations/project.ts index d3eceb53..5fab6e4b 100644 --- a/packages/api/src/validations/project.ts +++ b/packages/api/src/validations/project.ts @@ -14,6 +14,21 @@ export const projectNameSchema = z.string().trim().min(1).max(100); export const renameProjectSchema = z.object({ name: projectNameSchema }); +export const createProjectSchema = z.object({ name: projectNameSchema }); + +/** + * Per-org ceiling on `POST /v1/projects`. Bounds what a single member can mint + * — a project is not a cheap row: each one seeds an API key, a default agent + * and a policy generation, and every one of them widens the gateway's + * per-connect resolution. + * + * Enforced as count-then-create without a lock, so a burst of concurrent + * creates can overshoot by a few. That is deliberate: the cap exists to stop + * runaway growth, not to be exact, and a lock on a cold path costs more than + * the overshoot is worth. + */ +export const MAX_PROJECTS_PER_ORG = 100; + /** * The management role on a USER binding (step 13c): "owner" may * rename/share/delete the project, "member" is a plain use grant. GROUP