diff --git a/server/src/__tests__/auth-session-route.test.ts b/server/src/__tests__/auth-session-route.test.ts index 65f6cbd3..549ebd71 100644 --- a/server/src/__tests__/auth-session-route.test.ts +++ b/server/src/__tests__/auth-session-route.test.ts @@ -1,6 +1,7 @@ import express from "express"; import request from "supertest"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { instanceUserRoles } from "@paperclipai/db"; import { actorMiddleware } from "../middleware/auth.js"; function createSelectChain(rows: unknown[]) { @@ -92,6 +93,7 @@ describe("actorMiddleware authenticated session profile", () => { }; return chain; }), + delete: vi.fn(() => ({ where: () => Promise.resolve(undefined) })), select: vi.fn(), } as any; const app = express(); @@ -122,10 +124,12 @@ describe("actorMiddleware authenticated session profile", () => { userName: "Stack Owner", userEmail: "owner@example.com", source: "cloud_tenant", - isInstanceAdmin: true, + isInstanceAdmin: false, memberships: [expect.objectContaining({ membershipRole: "owner", status: "active" })], }); expect(res.body.companyIds[0]).toMatch(/^[0-9a-f-]{36}$/); + // authUsers, companies, companyMemberships, and the role-default + // principalPermissionGrants seeded in place of instance-admin elevation. expect(inserts).toHaveLength(4); expect(inserts[0]?.values).toMatchObject({ id: "global-user-1", @@ -133,4 +137,82 @@ describe("actorMiddleware authenticated session profile", () => { emailVerified: true, }); }); + + it("purges a stale instance_admin row so the session path stops elevating the cloud-tenant user", async () => { + process.env.PAPERCLIP_CLOUD_TENANT_SERVER_TOKEN = "tenant-token"; + // Simulates a deployment that previously ran the pre-hardening cloud_tenant + // path: instance_user_roles still holds an instance_admin row for the + // tenant user, who can also resolve a BetterAuth session for the same id. + const state = { staleInstanceAdminRow: true }; + const insertChain = { + values() { + return insertChain; + }, + onConflictDoUpdate() { + return insertChain; + }, + onConflictDoNothing() { + return insertChain; + }, + returning() { + return Promise.resolve([{ companyId: "company-1", membershipRole: "owner", status: "active" }]); + }, + then(resolve: (value: unknown) => unknown) { + return Promise.resolve(undefined).then(resolve); + }, + }; + const db = { + select: vi.fn(() => ({ + from: (table: unknown) => ({ + where: () => + Promise.resolve( + table === instanceUserRoles && state.staleInstanceAdminRow ? [{ id: "stale-role-row" }] : [], + ), + }), + })), + insert: vi.fn(() => insertChain), + delete: vi.fn((table: unknown) => ({ + where: () => { + if (table === instanceUserRoles) state.staleInstanceAdminRow = false; + return Promise.resolve(undefined); + }, + })), + } as any; + const app = express(); + app.use( + actorMiddleware(db, { + deploymentMode: "authenticated", + resolveSession: async () => ({ + session: { id: "session-1", userId: "global-user-1" }, + user: { id: "global-user-1", name: "Stack Owner", email: "owner@example.com" }, + }), + }), + ); + app.get("/actor", (req, res) => { + res.json(req.actor); + }); + + // Control: while the stale row exists, the session path still elevates. + const before = await request(app).get("/actor"); + expect(before.body).toMatchObject({ source: "session", isInstanceAdmin: true }); + + // One trusted-header authentication purges the stale grant. + const cloud = await request(app) + .get("/actor") + .set("x-paperclip-cloud-tenant-token", "tenant-token") + .set("x-paperclip-cloud-user-id", "global-user-1") + .set("x-paperclip-cloud-user-email", "owner@example.com") + .set("x-paperclip-cloud-stack-id", "stack-alpha") + .set("x-paperclip-cloud-stack-role", "owner"); + expect(cloud.body).toMatchObject({ source: "cloud_tenant", isInstanceAdmin: false }); + expect(state.staleInstanceAdminRow).toBe(false); + + // The same user no longer gets instance admin via the session path. + const after = await request(app).get("/actor"); + expect(after.body).toMatchObject({ + source: "session", + userId: "global-user-1", + isInstanceAdmin: false, + }); + }); }); diff --git a/server/src/__tests__/authorization-service.test.ts b/server/src/__tests__/authorization-service.test.ts index 1d156105..426ab9c8 100644 --- a/server/src/__tests__/authorization-service.test.ts +++ b/server/src/__tests__/authorization-service.test.ts @@ -681,6 +681,48 @@ describeEmbeddedPostgres("authorization service", () => { }); }); + it("never elevates cloud_tenant actors through stale instance_admin rows", async () => { + const tenantCompany = await createCompany(db, "CloudTenantStale"); + const otherCompany = await createCompany(db, "CloudTenantOther"); + const userId = `user-${randomUUID()}`; + const targetAgent = await createAgent(db, otherCompany.id, { role: "engineer" }); + await db.insert(companyMemberships).values({ + companyId: tenantCompany.id, + principalType: "user", + principalId: userId, + status: "active", + membershipRole: "owner", + }); + // Stale grant left behind by a pre-hardening cloud_tenant deployment. + await db.insert(instanceUserRoles).values({ userId, role: "instance_admin" }); + + const decision = await authorizationService(db).decide({ + actor: { + type: "board", + userId, + companyIds: [tenantCompany.id], + isInstanceAdmin: false, + source: "cloud_tenant", + }, + action: "tasks:assign", + resource: { type: "issue", companyId: otherCompany.id, assigneeAgentId: targetAgent.id }, + scope: { assigneeAgentId: targetAgent.id }, + }); + + expect(decision.allowed).toBe(false); + expect(decision.reason).not.toBe("allow_instance_admin"); + + // Control: the instanceUserRoles lookup still elevates non-cloud_tenant + // board actors, so the carve-out is scoped to the tenant contract only. + const sessionDecision = await authorizationService(db).decide({ + actor: { type: "board", userId, companyIds: [tenantCompany.id], source: "session" }, + action: "tasks:assign", + resource: { type: "issue", companyId: otherCompany.id, assigneeAgentId: targetAgent.id }, + scope: { assigneeAgentId: targetAgent.id }, + }); + expect(sessionDecision).toMatchObject({ allowed: true, reason: "allow_instance_admin" }); + }); + it("denies simple-mode assignment to a target agent from another company", async () => { const sourceCompany = await createCompany(db, "AssignmentSource"); const targetCompany = await createCompany(db, "AssignmentTarget"); diff --git a/server/src/__tests__/issue-comment-redaction.test.ts b/server/src/__tests__/issue-comment-redaction.test.ts index 45e08d18..d5339205 100644 --- a/server/src/__tests__/issue-comment-redaction.test.ts +++ b/server/src/__tests__/issue-comment-redaction.test.ts @@ -5,6 +5,7 @@ import { sql } from "drizzle-orm"; import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; import { companies, + companyMemberships, createDb, issueComments, issueReferenceMentions, @@ -46,6 +47,7 @@ describeEmbeddedPostgres("deleted issue comment redaction", () => { await db.delete(issueReferenceMentions); await db.delete(issueComments); await db.delete(issues); + await db.delete(companyMemberships); await db.delete(companies); }); @@ -70,6 +72,14 @@ describeEmbeddedPostgres("deleted issue comment redaction", () => { status: "todo", priority: "medium", }); + await db.insert(companyMemberships).values({ + companyId, + principalType: "user", + principalId: "board-user-1", + status: "active", + membershipRole: "owner", + updatedAt: new Date(), + }); return { companyId, issueId }; } @@ -83,7 +93,9 @@ describeEmbeddedPostgres("deleted issue comment redaction", () => { companyIds: [companyId], memberships: [{ companyId, membershipRole: "owner", status: "active" }], source: "cloud_tenant", - isInstanceAdmin: true, + // cloud_tenant actors are never instance admins — reads flow through + // the active company membership seeded in seedIssue(). + isInstanceAdmin: false, }; next(); }); diff --git a/server/src/__tests__/issue-identifier-routes.test.ts b/server/src/__tests__/issue-identifier-routes.test.ts index 1a204db5..2ad2e467 100644 --- a/server/src/__tests__/issue-identifier-routes.test.ts +++ b/server/src/__tests__/issue-identifier-routes.test.ts @@ -3,13 +3,14 @@ import express from "express"; import request from "supertest"; import { afterAll, beforeAll, describe, expect, it } from "vitest"; import { eq } from "drizzle-orm"; -import { companies, createDb, issues } from "@paperclipai/db"; +import { companies, companyMemberships, createDb, issues } from "@paperclipai/db"; import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase, } from "./helpers/embedded-postgres.js"; import { errorHandler } from "../middleware/index.js"; import { issueRoutes } from "../routes/issues.js"; +import { ensureHumanRoleDefaultGrants } from "../services/principal-access-compatibility.js"; const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport(); const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip; @@ -43,7 +44,9 @@ describeEmbeddedPostgres("issue identifier routes", () => { companyIds: [companyId], memberships: [{ companyId, membershipRole: "owner", status: "active" }], source: "cloud_tenant", - isInstanceAdmin: true, + // cloud_tenant actors are never instance admins — access flows through + // company-scoped membership grants, seeded per test company below. + isInstanceAdmin: false, }; next(); }); @@ -52,6 +55,23 @@ describeEmbeddedPostgres("issue identifier routes", () => { return app; } + async function seedCloudTenantMember(companyId: string) { + await db.insert(companyMemberships).values({ + companyId, + principalType: "user", + principalId: "cloud-user-1", + status: "active", + membershipRole: "owner", + updatedAt: new Date(), + }); + await ensureHumanRoleDefaultGrants(db, { + companyId, + principalId: "cloud-user-1", + membershipRole: "owner", + grantedByUserId: null, + }); + } + it("resolves alphanumeric Cloud tenant issue identifiers for detail reads and updates", async () => { const companyId = randomUUID(); const issueId = randomUUID(); @@ -62,6 +82,7 @@ describeEmbeddedPostgres("issue identifier routes", () => { issuePrefix: "PC1A2", requireBoardApprovalForNewAgents: false, }); + await seedCloudTenantMember(companyId); await db.insert(issues).values({ id: issueId, companyId, diff --git a/server/src/__tests__/multilingual-issues-routes.test.ts b/server/src/__tests__/multilingual-issues-routes.test.ts index da9f0aa3..4b0eac47 100644 --- a/server/src/__tests__/multilingual-issues-routes.test.ts +++ b/server/src/__tests__/multilingual-issues-routes.test.ts @@ -2,7 +2,7 @@ import { randomUUID } from "node:crypto"; import express from "express"; import request from "supertest"; import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; -import { companies, createDb } from "@paperclipai/db"; +import { companies, companyMemberships, createDb } from "@paperclipai/db"; import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase, @@ -62,6 +62,14 @@ describeEmbeddedPostgres("multilingual issue routes", () => { issuePrefix: "LNG", requireBoardApprovalForNewAgents: false, }); + await db.insert(companyMemberships).values({ + companyId, + principalType: "user", + principalId: "cloud-user-1", + status: "active", + membershipRole: "owner", + updatedAt: new Date(), + }); }, 20_000); afterAll(async () => { @@ -92,7 +100,9 @@ describeEmbeddedPostgres("multilingual issue routes", () => { companyIds: [companyId], memberships: [{ companyId, membershipRole: "owner", status: "active" }], source: "cloud_tenant", - isInstanceAdmin: true, + // cloud_tenant actors are never instance admins — reads flow through + // the active company membership seeded in beforeAll. + isInstanceAdmin: false, }; next(); }); diff --git a/server/src/middleware/auth.ts b/server/src/middleware/auth.ts index ad0b4289..c0cbf954 100644 --- a/server/src/middleware/auth.ts +++ b/server/src/middleware/auth.ts @@ -8,6 +8,7 @@ import type { DeploymentMode } from "@paperclipai/shared"; import type { BetterAuthSessionResult } from "../auth/better-auth.js"; import { logger } from "./logger.js"; import { boardAuthService } from "../services/board-auth.js"; +import { ensureHumanRoleDefaultGrants } from "../services/principal-access-compatibility.js"; function hashToken(token: string) { return createHash("sha256").update(token).digest("hex"); @@ -199,7 +200,7 @@ export function actorMiddleware(db: Db, opts: ActorMiddlewareOptions): RequestHa }; } -async function resolveCloudTenantActor(db: Db, req: Request): Promise { +export async function resolveCloudTenantActor(db: Db, req: Request): Promise { const expectedToken = process.env.PAPERCLIP_CLOUD_TENANT_SERVER_TOKEN?.trim(); if (!expectedToken) return null; @@ -237,16 +238,14 @@ async function resolveCloudTenantActor(db: Db, req: Request): Promise = {}; + chain.values = () => chain; + chain.onConflictDoUpdate = () => chain; + chain.onConflictDoNothing = () => chain; + chain.where = () => chain; + chain.returning = async () => [membershipRow]; + chain.then = (resolve: (v: unknown) => unknown) => Promise.resolve(undefined).then(resolve); + const db = { + insert: (table: unknown) => { + insertedTables.push(table); + return chain; + }, + delete: (table: unknown) => { + deletedTables.push(table); + return chain; + }, + } as unknown as Db; + return { db, insertedTables, deletedTables }; +} + +function fakeReq(headers: Record): Request { + const lower: Record = {}; + for (const [k, v] of Object.entries(headers)) lower[k.toLowerCase()] = v; + return { header: (name: string) => lower[name.toLowerCase()] } as unknown as Request; +} + +const VALID_HEADERS = { + "x-paperclip-cloud-tenant-token": "test-server-token", + "x-paperclip-cloud-user-id": "user-123", + "x-paperclip-cloud-user-email": "Owner@Example.com", + "x-paperclip-cloud-stack-id": "stack-abc", + "x-paperclip-cloud-stack-role": "owner", +}; + +describe("resolveCloudTenantActor (shared-pool hardening)", () => { + beforeEach(() => { + process.env.PAPERCLIP_CLOUD_TENANT_SERVER_TOKEN = "test-server-token"; + }); + afterEach(() => { + delete process.env.PAPERCLIP_CLOUD_TENANT_SERVER_TOKEN; + }); + + it("never grants instance admin", async () => { + const { db, insertedTables } = createFakeDb(); + const actor = await resolveCloudTenantActor(db, fakeReq(VALID_HEADERS)); + expect(actor).not.toBeNull(); + expect(actor!.isInstanceAdmin).toBe(false); + expect(insertedTables).not.toContain(instanceUserRoles); + }); + + it("is scoped to exactly the one company from its stack", async () => { + const { db } = createFakeDb(); + const actor = await resolveCloudTenantActor(db, fakeReq(VALID_HEADERS)); + expect(actor!.companyIds).toHaveLength(1); + expect(actor!.memberships).toHaveLength(1); + expect(actor?.memberships?.[0]?.companyId).toBe(actor?.companyIds?.[0]); + expect(actor?.memberships?.[0]?.membershipRole).toBe("owner"); + expect(actor!.source).toBe("cloud_tenant"); + }); + + it("purges stale instance_admin rows left by pre-hardening deployments", async () => { + const { db, deletedTables } = createFakeDb(); + const actor = await resolveCloudTenantActor(db, fakeReq(VALID_HEADERS)); + expect(actor).not.toBeNull(); + expect(deletedTables).toContain(instanceUserRoles); + }); + + it("still upserts the user, company, and membership", async () => { + const { db, insertedTables } = createFakeDb(); + await resolveCloudTenantActor(db, fakeReq(VALID_HEADERS)); + expect(insertedTables).toContain(authUsers); + expect(insertedTables).toContain(companies); + expect(insertedTables).toContain(companyMemberships); + }); + + it("returns null when the server token is unset", async () => { + delete process.env.PAPERCLIP_CLOUD_TENANT_SERVER_TOKEN; + const { db } = createFakeDb(); + const actor = await resolveCloudTenantActor(db, fakeReq(VALID_HEADERS)); + expect(actor).toBeNull(); + }); + + it("maps a non-owner stack role through to the membership without elevating", async () => { + const { db } = createFakeDb({ companyId: "company-y", membershipRole: "member", status: "active" }); + const actor = await resolveCloudTenantActor( + db, + fakeReq({ ...VALID_HEADERS, "x-paperclip-cloud-stack-role": "member" }), + ); + expect(actor!.isInstanceAdmin).toBe(false); + expect(actor?.memberships?.[0]?.membershipRole).toBe("member"); + }); +}); diff --git a/server/src/services/authorization.ts b/server/src/services/authorization.ts index df7af2a5..50aff187 100644 --- a/server/src/services/authorization.ts +++ b/server/src/services/authorization.ts @@ -78,6 +78,7 @@ export type AuthorizationDecision = { | "allow_legacy_agent_creator" | "allow_self" | "allow_company_agent" + | "allow_company_member" | "allow_simple_company_member" | "allow_manager_chain" | "deny_unauthenticated" @@ -922,13 +923,48 @@ export function authorizationService(db: Db) { explanation: "Allowed because the actor is the local implicit board.", }); } - if (input.actor.isInstanceAdmin || await isInstanceAdmin(input.actor.userId)) { + // cloud_tenant actors are company-scoped by contract and must never be + // elevated — not even via stale instance_admin rows left behind by + // deployments that ran the pre-hardening cloud_tenant path. + if ( + input.actor.source !== "cloud_tenant" && + (input.actor.isInstanceAdmin || await isInstanceAdmin(input.actor.userId)) + ) { return allow({ action: input.action, reason: "allow_instance_admin", explanation: "Allowed because the actor is an instance admin.", }); } + // What instance-admin elevation used to give cloud tenant users is + // replaced by company-scoped visibility: an active membership in the + // resource company grants the same read surface a same-company agent + // gets, and non-viewer members may mutate issues inside their company. + // Cross-company access stays denied. + if (input.actor.source === "cloud_tenant" && input.actor.userId) { + const membership = await getActiveMembership(companyId, "user", input.actor.userId); + if (membership) { + if ( + input.action === "agent:read" || + input.action === "company_scope:read" || + input.action === "issue:read" || + input.action === "project:read" + ) { + return allow({ + action: input.action, + reason: "allow_company_member", + explanation: "Allowed by active cloud tenant company membership.", + }); + } + if (input.action === "issue:mutate" && membership.membershipRole !== "viewer") { + return allow({ + action: input.action, + reason: "allow_company_member", + explanation: "Allowed by active cloud tenant company membership.", + }); + } + } + } if (!input.actor.userId) { return deny({ action: input.action,