fix(auth): address multi-user review hardening

packages/db/src/migrations/0057_tidy_join_requests.sql

@@ -0,0 +1,57 @@
+WITH ranked_user_requests AS (
+  SELECT
+    id,
+    row_number() OVER (
+      PARTITION BY company_id, requesting_user_id
+      ORDER BY created_at ASC, id ASC
+    ) AS rank
+  FROM join_requests
+  WHERE request_type = 'human'
+    AND status = 'pending_approval'
+    AND requesting_user_id IS NOT NULL
+)
+UPDATE join_requests
+SET
+  status = 'rejected',
+  rejected_at = COALESCE(rejected_at, now()),
+  updated_at = now()
+WHERE id IN (
+  SELECT id
+  FROM ranked_user_requests
+  WHERE rank > 1
+);
+--> statement-breakpoint
+WITH ranked_email_requests AS (
+  SELECT
+    id,
+    row_number() OVER (
+      PARTITION BY company_id, lower(request_email_snapshot)
+      ORDER BY created_at ASC, id ASC
+    ) AS rank
+  FROM join_requests
+  WHERE request_type = 'human'
+    AND status = 'pending_approval'
+    AND request_email_snapshot IS NOT NULL
+)
+UPDATE join_requests
+SET
+  status = 'rejected',
+  rejected_at = COALESCE(rejected_at, now()),
+  updated_at = now()
+WHERE id IN (
+  SELECT id
+  FROM ranked_email_requests
+  WHERE rank > 1
+);
+--> statement-breakpoint
+CREATE UNIQUE INDEX IF NOT EXISTS "join_requests_pending_human_user_uq"
+ON "join_requests" USING btree ("company_id", "requesting_user_id")
+WHERE "request_type" = 'human'
+  AND "status" = 'pending_approval'
+  AND "requesting_user_id" IS NOT NULL;
+--> statement-breakpoint
+CREATE UNIQUE INDEX IF NOT EXISTS "join_requests_pending_human_email_uq"
+ON "join_requests" USING btree ("company_id", lower("request_email_snapshot"))
+WHERE "request_type" = 'human'
+  AND "status" = 'pending_approval'
+  AND "request_email_snapshot" IS NOT NULL;

packages/db/src/migrations/meta/_journal.json

@@ -400,6 +400,13 @@
       "when": 1776084034244,
       "tag": "0056_spooky_ultragirl",
       "breakpoints": true
+    },
+    {
+      "idx": 57,
+      "version": "7",
+      "when": 1776309613598,
+      "tag": "0057_tidy_join_requests",
+      "breakpoints": true
     }
   ]
-}
+}

packages/db/src/schema/join_requests.ts

@@ -1,3 +1,4 @@
+import { sql } from "drizzle-orm";
 import { pgTable, uuid, text, timestamp, jsonb, index, uniqueIndex } from "drizzle-orm/pg-core";
 import { companies } from "./companies.js";
 import { invites } from "./invites.js";
@@ -37,5 +38,11 @@ export const joinRequests = pgTable(
       table.requestType,
       table.createdAt,
     ),
+    pendingHumanUserUniqueIdx: uniqueIndex("join_requests_pending_human_user_uq")
+      .on(table.companyId, table.requestingUserId)
+      .where(sql`${table.requestType} = 'human' AND ${table.status} = 'pending_approval' AND ${table.requestingUserId} IS NOT NULL`),
+    pendingHumanEmailUniqueIdx: uniqueIndex("join_requests_pending_human_email_uq")
+      .on(table.companyId, sql`lower(${table.requestEmailSnapshot})`)
+      .where(sql`${table.requestType} = 'human' AND ${table.status} = 'pending_approval' AND ${table.requestEmailSnapshot} IS NOT NULL`),
   }),
 );

packages/shared/src/validators/access.ts

@@ -124,8 +124,8 @@ function isValidProfileImage(value: string): boolean {
   if (profileImageAssetPathPattern.test(value)) return true;
 
   try {
-    new URL(value);
-    return true;
+    const url = new URL(value);
+    return url.protocol === "https:" || url.protocol === "http:";
   } catch {
     return false;
   }

server/src/__tests__/access-validators.test.ts

@@ -0,0 +1,22 @@
+import { describe, expect, it } from "vitest";
+import { updateCurrentUserProfileSchema } from "@paperclipai/shared";
+
+describe("access validators", () => {
+  it("accepts HTTP(S) and Paperclip asset image URLs", () => {
+    expect(updateCurrentUserProfileSchema.safeParse({
+      name: "Ada Lovelace",
+      image: "https://example.com/avatar.png",
+    }).success).toBe(true);
+    expect(updateCurrentUserProfileSchema.safeParse({
+      name: "Ada Lovelace",
+      image: "/api/assets/avatar/content",
+    }).success).toBe(true);
+  });
+
+  it("rejects data URI profile images", () => {
+    expect(updateCurrentUserProfileSchema.safeParse({
+      name: "Ada Lovelace",
+      image: "data:image/png;base64,AAAA",
+    }).success).toBe(false);
+  });
+});

server/src/__tests__/plugin-routes-authz.test.ts

@@ -5,11 +5,15 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
 const mockRegistry = vi.hoisted(() => ({
   getById: vi.fn(),
   getByKey: vi.fn(),
+  upsertConfig: vi.fn(),
 }));
 
 const mockLifecycle = vi.hoisted(() => ({
   load: vi.fn(),
   upgrade: vi.fn(),
+  unload: vi.fn(),
+  enable: vi.fn(),
+  disable: vi.fn(),
 }));
 
 vi.mock("../services/plugin-registry.js", () => ({
@@ -138,6 +142,31 @@ describe("plugin install and upgrade authz", () => {
     expect(mockLifecycle.upgrade).not.toHaveBeenCalled();
   }, 20_000);
 
+  it.each([
+    ["delete", "delete", "/api/plugins/11111111-1111-4111-8111-111111111111", undefined],
+    ["enable", "post", "/api/plugins/11111111-1111-4111-8111-111111111111/enable", {}],
+    ["disable", "post", "/api/plugins/11111111-1111-4111-8111-111111111111/disable", {}],
+    ["config", "post", "/api/plugins/11111111-1111-4111-8111-111111111111/config", { configJson: {} }],
+  ] as const)("rejects plugin %s for non-admin board users", async (_name, method, path, body) => {
+    const { app } = await createApp({
+      type: "board",
+      userId: "user-1",
+      source: "session",
+      isInstanceAdmin: false,
+      companyIds: ["company-1"],
+    });
+
+    const req = method === "delete" ? request(app).delete(path) : request(app).post(path).send(body);
+    const res = await req;
+
+    expect(res.status).toBe(403);
+    expect(mockRegistry.getById).not.toHaveBeenCalled();
+    expect(mockRegistry.upsertConfig).not.toHaveBeenCalled();
+    expect(mockLifecycle.unload).not.toHaveBeenCalled();
+    expect(mockLifecycle.enable).not.toHaveBeenCalled();
+    expect(mockLifecycle.disable).not.toHaveBeenCalled();
+  }, 20_000);
+
   it("allows instance admins to upgrade plugins", async () => {
     const pluginId = "11111111-1111-4111-8111-111111111111";
     mockRegistry.getById.mockResolvedValue({

server/src/routes/authz.ts

@@ -51,8 +51,12 @@ export function assertCompanyAccess(req: Request, companyId: string) {
     }
     const method = typeof req.method === "string" ? req.method.toUpperCase() : "GET";
     const isSafeMethod = ["GET", "HEAD", "OPTIONS"].includes(method);
-    if (!isSafeMethod && Array.isArray(req.actor.memberships) && req.actor.memberships.length > 0) {
-      const membership = req.actor.memberships.find((item) => item.companyId === companyId);
+    if (!isSafeMethod && !req.actor.isInstanceAdmin) {
+      const memberships = req.actor.memberships ?? [];
+      const membership = memberships.find((item) => item.companyId === companyId);
+      if (!membership) {
+        throw forbidden("User does not have active company access");
+      }
       if (membership?.status !== "active") {
         throw forbidden("User does not have active company access");
       }

server/src/routes/plugins.ts

@@ -1232,7 +1232,7 @@ export function pluginRoutes(
    * Errors: 404 if plugin not found, 400 for lifecycle errors
    */
   router.delete("/plugins/:pluginId", async (req, res) => {
-    assertBoardOrgAccess(req);
+    assertInstanceAdmin(req);
     const { pluginId } = req.params;
     const purge = req.query.purge === "true";
 
@@ -1268,7 +1268,7 @@ export function pluginRoutes(
    * Errors: 404 if plugin not found, 400 for lifecycle errors
    */
   router.post("/plugins/:pluginId/enable", async (req, res) => {
-    assertBoardOrgAccess(req);
+    assertInstanceAdmin(req);
     const { pluginId } = req.params;
 
     const plugin = await resolvePlugin(registry, pluginId);
@@ -1306,7 +1306,7 @@ export function pluginRoutes(
    * Errors: 404 if plugin not found, 400 for lifecycle errors
    */
   router.post("/plugins/:pluginId/disable", async (req, res) => {
-    assertBoardOrgAccess(req);
+    assertInstanceAdmin(req);
     const { pluginId } = req.params;
     const body = req.body as { reason?: string } | undefined;
     const reason = body?.reason;
@@ -1547,7 +1547,7 @@ export function pluginRoutes(
    * - 404 if plugin not found
    */
   router.post("/plugins/:pluginId/config", async (req, res) => {
-    assertBoardOrgAccess(req);
+    assertInstanceAdmin(req);
     const { pluginId } = req.params;
 
     const plugin = await resolvePlugin(registry, pluginId);