From 7e6ac7d823217f50bb67700990177e8e27f4616f Mon Sep 17 00:00:00 2001 From: Universe Date: Wed, 3 Jun 2026 21:05:44 +0900 Subject: [PATCH 1/7] feat: organization avatar upload (set/update/remove) Add an avatar control to the org General settings form. The profile submit now goes through a "use server" action instead of the private API route: - updateOrganizationProfile reads/updates the org row with the user-authed, RLS-aware client (the organization SELECT/UPDATE membership policy is the authorization gate); the avatar object write uses the service_role storage client because the avatars bucket has no tracked storage RLS. - avatar-field.tsx: pick + local preview + remove, carried as the avatar file and a remove_avatar flag on the existing form. - avatar stored at org-{id}/avatar (upsert); only avatar_path is persisted, the URL is computed at read time via PublicUrls. png/jpeg/webp, <=2MB. - remove the now-dead /private/accounts/organizations/[org]/profile route. No new routes/pages; reuses the avatars bucket + existing render sites. --- .../organizations/[org]/profile/route.ts | 45 -------- .../settings/profile/actions.ts | 103 ++++++++++++++++++ .../settings/profile/avatar-field.tsx | 79 ++++++++++++++ .../settings/profile/page.tsx | 18 ++- 4 files changed, 197 insertions(+), 48 deletions(-) delete mode 100644 editor/app/(api)/private/accounts/organizations/[org]/profile/route.ts create mode 100644 editor/app/(site)/organizations/[organization_name]/settings/profile/actions.ts create mode 100644 editor/app/(site)/organizations/[organization_name]/settings/profile/avatar-field.tsx diff --git a/editor/app/(api)/private/accounts/organizations/[org]/profile/route.ts b/editor/app/(api)/private/accounts/organizations/[org]/profile/route.ts deleted file mode 100644 index 4ead8e4ce8..0000000000 --- a/editor/app/(api)/private/accounts/organizations/[org]/profile/route.ts +++ /dev/null @@ -1,45 +0,0 @@ -import { createClient } from "@/lib/supabase/server"; -import { NextRequest, NextResponse } from "next/server"; - -type Params = { org: string }; - -export async function POST( - req: NextRequest, - context: { - params: Promise; - } -) { - const origin = req.nextUrl.origin; - const { org } = await context.params; - - const client = await createClient(); - - const body = await req.formData(); - - const display_name = body.get("display_name"); - const email = body.get("email"); - const description = body.get("description"); - const blog = body.get("blog"); - - const { error } = await client - .from("organization") - .update({ - display_name: String(display_name), - email: String(email), - description: description ? String(description) : undefined, - blog: blog ? String(blog) : undefined, - }) - .eq("name", org); - - if (error) { - console.error("organization/profile", error); - return NextResponse.error(); - } - - return NextResponse.redirect( - origin + `/organizations/${org}/settings/profile`, - { - status: 302, - } - ); -} diff --git a/editor/app/(site)/organizations/[organization_name]/settings/profile/actions.ts b/editor/app/(site)/organizations/[organization_name]/settings/profile/actions.ts new file mode 100644 index 0000000000..c23852e9f4 --- /dev/null +++ b/editor/app/(site)/organizations/[organization_name]/settings/profile/actions.ts @@ -0,0 +1,103 @@ +"use server"; + +// Reads + the org-row UPDATE use `createClient()` (user-authed, RLS-aware): +// the `organization` SELECT/UPDATE policies (`rls_organization`) only yield / +// accept a row for members, so they double as the authorization gate. The +// avatar object write goes through `service_role.workspace.storage` because the +// `avatars` bucket has no tracked storage RLS — the membership gate below is +// what authorizes it. + +import { createClient, service_role } from "@/lib/supabase/server"; +import { revalidatePath } from "next/cache"; + +const AVATAR_MAX_BYTES = 2 * 1024 * 1024; // ~2MB +const AVATAR_ACCEPTED_TYPES = ["image/png", "image/jpeg", "image/webp"]; + +/** + * Update an organization's general profile (display name, contact email, + * description, blog) and, optionally, its avatar. + * + * Bound to the org name at the call site: `action={updateOrganizationProfile.bind(null, organization_name)}`. + */ +export async function updateOrganizationProfile( + organization_name: string, + formData: FormData +): Promise { + const client = await createClient(); + + const { data: auth } = await client.auth.getUser(); + if (!auth.user) { + throw new Error("unauthorized"); + } + + // `Enabled based on membership` RLS returns a row only to org members — this + // select is both the id lookup and the authorization gate for the upload. + const { data: org, error: orgError } = await client + .from("organization") + .select("id") + .eq("name", organization_name) + .single(); + + if (orgError || !org) { + throw new Error("organization not found or forbidden"); + } + + const display_name = formData.get("display_name"); + const email = formData.get("email"); + const description = formData.get("description"); + const blog = formData.get("blog"); + const remove_avatar = formData.get("remove_avatar") === "1"; + const avatar = formData.get("avatar"); + + // `undefined` => leave avatar_path untouched. `null` => clear it. + let avatar_path: string | null | undefined = undefined; + + if (remove_avatar) { + avatar_path = null; + } else if (avatar instanceof File && avatar.size > 0) { + if (!AVATAR_ACCEPTED_TYPES.includes(avatar.type)) { + throw new Error(`unsupported image type: ${avatar.type || "unknown"}`); + } + if (avatar.size > AVATAR_MAX_BYTES) { + throw new Error("image too large (max 2MB)"); + } + + // Stable per-org key + upsert: replacing the avatar overwrites in place. + const path = `org-${org.id}/avatar`; + + // service_role: the `avatars` bucket has no tracked storage RLS, so the + // membership gate above is the authorization boundary for this write. + const { error: uploadError } = await service_role.workspace.storage + .from("avatars") + .upload(path, avatar, { + contentType: avatar.type, + upsert: true, + }); + + if (uploadError) { + console.error("organization/profile avatar upload", uploadError); + throw new Error("failed to upload avatar"); + } + + avatar_path = path; + } + + // Mirrors the prior route's field handling (empty string => leave untouched). + const { error: updateError } = await client + .from("organization") + .update({ + display_name: String(display_name), + email: String(email), + description: description ? String(description) : undefined, + blog: blog ? String(blog) : undefined, + ...(avatar_path !== undefined ? { avatar_path } : {}), + }) + .eq("name", organization_name); + + if (updateError) { + console.error("organization/profile update", updateError); + throw new Error("failed to update organization"); + } + + revalidatePath(`/organizations/${organization_name}/settings/profile`); +} diff --git a/editor/app/(site)/organizations/[organization_name]/settings/profile/avatar-field.tsx b/editor/app/(site)/organizations/[organization_name]/settings/profile/avatar-field.tsx new file mode 100644 index 0000000000..bb4a193902 --- /dev/null +++ b/editor/app/(site)/organizations/[organization_name]/settings/profile/avatar-field.tsx @@ -0,0 +1,79 @@ +"use client"; + +import React, { useRef, useState } from "react"; +import { Avatar, AvatarFallback, AvatarImage } from "@/components/ui/avatar"; +import { Button } from "@/components/ui/button"; + +const ACCEPT = "image/png,image/jpeg,image/webp"; + +/** + * Avatar control inside the org General form. Renders the current avatar (or + * initials fallback), lets the user pick a replacement with a local preview, or + * remove it. The actual write happens on the form's submit via the server + * action — this only carries the `avatar` file and the `remove_avatar` flag. + */ +export function OrganizationAvatarField({ + current_avatar_url, + display_name, +}: { + current_avatar_url?: string | null; + display_name: string; +}) { + const inputRef = useRef(null); + const [preview, setPreview] = useState(null); + const [removed, setRemoved] = useState(false); + + const shown = removed ? null : (preview ?? current_avatar_url ?? null); + + const onPick = (e: React.ChangeEvent) => { + const file = e.target.files?.[0]; + if (!file) return; + setRemoved(false); + setPreview(URL.createObjectURL(file)); + }; + + const onRemove = () => { + setPreview(null); + setRemoved(true); + if (inputRef.current) inputRef.current.value = ""; + }; + + return ( +
+ + {shown ? ( + + ) : ( + + {display_name?.charAt(0).toUpperCase()} + + )} + +
+ + + {shown && ( + + )} +
+ {/* Tells the server action to clear avatar_path on submit. */} + {removed && } +
+ ); +} diff --git a/editor/app/(site)/organizations/[organization_name]/settings/profile/page.tsx b/editor/app/(site)/organizations/[organization_name]/settings/profile/page.tsx index 6be1a58e9f..73f64bae96 100644 --- a/editor/app/(site)/organizations/[organization_name]/settings/profile/page.tsx +++ b/editor/app/(site)/organizations/[organization_name]/settings/profile/page.tsx @@ -12,6 +12,9 @@ import { notFound, redirect } from "next/navigation"; import { DeleteOrganizationConfirm } from "./delete"; import { Badge } from "@/components/ui/badge"; import { createClient } from "@/lib/supabase/server"; +import { PublicUrls } from "@/services/public-urls"; +import { updateOrganizationProfile } from "./actions"; +import { OrganizationAvatarField } from "./avatar-field"; import Link from "next/link"; type Params = { organization_name: string }; @@ -44,6 +47,10 @@ export default async function OrganizationsSettingsProfilePage({ const iamowner = data.owner_id === auth.user.id; + const avatar_url = data.avatar_path + ? PublicUrls.organization_avatar_url(client)(data.avatar_path) + : null; + return (
@@ -53,12 +60,17 @@ export default async function OrganizationsSettingsProfilePage({
+ + Organization avatar + + Name From c18672239773c179db15b85fa86c559c36369f88 Mon Sep 17 00:00:00 2001 From: Universe Date: Wed, 3 Jun 2026 21:06:35 +0900 Subject: [PATCH 2/7] test: cover org profile update server action Membership gate, avatar type/size validation, org-{id}/avatar upsert upload, and the remove (avatar_path: null) path. Mocks the supabase clients and next/cache. --- .../profile/__tests__/actions.test.ts | 211 ++++++++++++++++++ 1 file changed, 211 insertions(+) create mode 100644 editor/app/(site)/organizations/[organization_name]/settings/profile/__tests__/actions.test.ts diff --git a/editor/app/(site)/organizations/[organization_name]/settings/profile/__tests__/actions.test.ts b/editor/app/(site)/organizations/[organization_name]/settings/profile/__tests__/actions.test.ts new file mode 100644 index 0000000000..ab4d646182 --- /dev/null +++ b/editor/app/(site)/organizations/[organization_name]/settings/profile/__tests__/actions.test.ts @@ -0,0 +1,211 @@ +/** + * Unit tests for `updateOrganizationProfile` — the org General settings server + * action. The supabase clients and `next/cache` are mocked; these assert the + * pure decision logic: the membership gate, avatar validation, the + * `org-{id}/avatar` path scheme + upsert upload, and the remove flow. + */ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +// Typed `vi.fn` shorthand (repo lint requires explicit mock type parameters). +const fn = () => vi.fn<(...args: never[]) => unknown>(); + +vi.mock("next/cache", () => ({ + revalidatePath: vi.fn<(...args: never[]) => void>(), +})); + +vi.mock("@/lib/supabase/server", () => ({ + createClient: vi.fn<(...args: never[]) => unknown>(), + service_role: { + workspace: { + storage: { + from: vi.fn<(...args: never[]) => unknown>(), + }, + }, + }, +})); + +import { createClient, service_role } from "@/lib/supabase/server"; +import { revalidatePath } from "next/cache"; +import { updateOrganizationProfile } from "../actions"; + +const mockedCreateClient = vi.mocked(createClient); + +const ORG_NAME = "acme"; +const ORG_ID = 42; + +/** Build a user-authed client stub whose `.from()` chain captures the update. */ +function makeUserClient(opts: { member?: boolean } = {}) { + const member = opts.member ?? true; + const update = fn().mockReturnValue({ + eq: fn().mockResolvedValue({ error: null }), + }); + + const client = { + auth: { + getUser: fn().mockResolvedValue({ data: { user: { id: "u1" } } }), + }, + from: vi.fn<(table: string) => unknown>((table: string) => { + if (table !== "organization") + throw new Error(`unexpected table ${table}`); + return { + // org lookup / membership gate + select: fn().mockReturnValue({ + eq: fn().mockReturnValue({ + single: fn().mockResolvedValue( + member + ? { data: { id: ORG_ID }, error: null } + : { data: null, error: { message: "forbidden" } } + ), + }), + }), + // profile update + update, + }; + }), + }; + return { client, update }; +} + +/** Capture the storage upload call. */ +function stubUpload(result: { error: unknown } = { error: null }) { + const upload = fn().mockResolvedValue(result); + vi.mocked(service_role.workspace.storage.from).mockReturnValue({ + upload, + // oxlint-disable-next-line typescript-eslint/no-explicit-any -- partial storage stub + } as any); + return upload; +} + +function form(fields: Record) { + const fd = new FormData(); + for (const [k, v] of Object.entries(fields)) fd.set(k, v as never); + return fd; +} + +function imageFile(type: string, bytes: number, name = "a.png") { + return new File([new Uint8Array(bytes)], name, { type }); +} + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe("updateOrganizationProfile", () => { + it("rejects when the caller is not an org member (RLS gate)", async () => { + const { client } = makeUserClient({ member: false }); + // oxlint-disable-next-line typescript-eslint/no-explicit-any -- partial client stub + mockedCreateClient.mockResolvedValue(client as any); + + await expect( + updateOrganizationProfile( + ORG_NAME, + form({ display_name: "Acme", email: "a@acme.com" }) + ) + ).rejects.toThrow(/not found or forbidden/); + }); + + it("updates text fields without touching avatar_path when no file/remove", async () => { + const { client, update } = makeUserClient(); + // oxlint-disable-next-line typescript-eslint/no-explicit-any -- partial client stub + mockedCreateClient.mockResolvedValue(client as any); + const upload = stubUpload(); + + await updateOrganizationProfile( + ORG_NAME, + form({ + display_name: "Acme Inc", + email: "a@acme.com", + description: "hello", + blog: "https://acme.com", + }) + ); + + expect(upload).not.toHaveBeenCalled(); + const payload = update.mock.calls[0][0]; + expect(payload).toMatchObject({ + display_name: "Acme Inc", + email: "a@acme.com", + description: "hello", + blog: "https://acme.com", + }); + expect("avatar_path" in payload).toBe(false); + expect(revalidatePath).toHaveBeenCalledWith( + `/organizations/${ORG_NAME}/settings/profile` + ); + }); + + it("uploads to org-{id}/avatar (upsert) and writes avatar_path on valid image", async () => { + const { client, update } = makeUserClient(); + // oxlint-disable-next-line typescript-eslint/no-explicit-any -- partial client stub + mockedCreateClient.mockResolvedValue(client as any); + const upload = stubUpload(); + + await updateOrganizationProfile( + ORG_NAME, + form({ + display_name: "Acme", + email: "a@acme.com", + avatar: imageFile("image/png", 1024), + }) + ); + + expect(service_role.workspace.storage.from).toHaveBeenCalledWith("avatars"); + const [path, , options] = upload.mock.calls[0]; + expect(path).toBe(`org-${ORG_ID}/avatar`); + expect(options).toMatchObject({ upsert: true, contentType: "image/png" }); + expect(update.mock.calls[0][0]).toMatchObject({ + avatar_path: `org-${ORG_ID}/avatar`, + }); + }); + + it("clears avatar_path on remove and does not upload", async () => { + const { client, update } = makeUserClient(); + // oxlint-disable-next-line typescript-eslint/no-explicit-any -- partial client stub + mockedCreateClient.mockResolvedValue(client as any); + const upload = stubUpload(); + + await updateOrganizationProfile( + ORG_NAME, + form({ display_name: "Acme", email: "a@acme.com", remove_avatar: "1" }) + ); + + expect(upload).not.toHaveBeenCalled(); + expect(update.mock.calls[0][0]).toMatchObject({ avatar_path: null }); + }); + + it("rejects unsupported image types", async () => { + const { client } = makeUserClient(); + // oxlint-disable-next-line typescript-eslint/no-explicit-any -- partial client stub + mockedCreateClient.mockResolvedValue(client as any); + stubUpload(); + + await expect( + updateOrganizationProfile( + ORG_NAME, + form({ + display_name: "Acme", + email: "a@acme.com", + avatar: imageFile("image/gif", 1024, "a.gif"), + }) + ) + ).rejects.toThrow(/unsupported image type/); + }); + + it("rejects images larger than 2MB", async () => { + const { client } = makeUserClient(); + // oxlint-disable-next-line typescript-eslint/no-explicit-any -- partial client stub + mockedCreateClient.mockResolvedValue(client as any); + stubUpload(); + + await expect( + updateOrganizationProfile( + ORG_NAME, + form({ + display_name: "Acme", + email: "a@acme.com", + avatar: imageFile("image/png", 2 * 1024 * 1024 + 1), + }) + ) + ).rejects.toThrow(/too large/); + }); +}); From 2509465f096854557cbb48e4761a80b3e4a081e8 Mon Sep 17 00:00:00 2001 From: Universe Date: Thu, 4 Jun 2026 00:26:06 +0900 Subject: [PATCH 3/7] refactor(org-avatar): RLS-based storage, drop service_role MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per owner: server actions must not use service_role — RLS is the boundary end to end. The avatar upload/remove now go through the same user-authed createClient() as the org row read/update; authorization is enforced by storage policies, not a code-side gate. - New migration 20260604120000_avatars_bucket_rls.sql: - idempotently creates the public 'avatars' bucket (no-op in prod, where it already exists; on conflict do nothing) for local parity. - adds org-membership storage.objects policies (insert/update/delete) scoped via public.rls_organization on the path's first segment, mirroring the storage/www bucket precedent. SELECT stays public for public-URL reads. - each policy is drop-if-exists guarded to coexist with any same-named prod policy; the membership check is CASE-guarded so non-numeric paths (the bucket is shared with uuid-keyed user_profile avatars) return false instead of raising on the ::bigint cast. - Path scheme changed org-{id}/avatar -> {id}/avatar so the policy can parse the org id via (storage.foldername(name))[1]. - Tests assert the upload/remove go through the user-auth client (no service_role) and the {id}/avatar path. --- .../profile/__tests__/actions.test.ts | 67 +++++++-------- .../settings/profile/actions.ts | 40 ++++++--- .../20260604120000_avatars_bucket_rls.sql | 85 +++++++++++++++++++ 3 files changed, 141 insertions(+), 51 deletions(-) create mode 100644 supabase/migrations/20260604120000_avatars_bucket_rls.sql diff --git a/editor/app/(site)/organizations/[organization_name]/settings/profile/__tests__/actions.test.ts b/editor/app/(site)/organizations/[organization_name]/settings/profile/__tests__/actions.test.ts index ab4d646182..de15f4b33d 100644 --- a/editor/app/(site)/organizations/[organization_name]/settings/profile/__tests__/actions.test.ts +++ b/editor/app/(site)/organizations/[organization_name]/settings/profile/__tests__/actions.test.ts @@ -1,8 +1,12 @@ /** * Unit tests for `updateOrganizationProfile` — the org General settings server - * action. The supabase clients and `next/cache` are mocked; these assert the + * action. The supabase client and `next/cache` are mocked; these assert the * pure decision logic: the membership gate, avatar validation, the - * `org-{id}/avatar` path scheme + upsert upload, and the remove flow. + * `{id}/avatar` path scheme + upsert upload, and the remove flow. + * + * Note: the upload/remove go through the SAME user-authed `createClient()` as + * the row read/update — there is no `service_role` in this feature (RLS is the + * boundary), so the mock exposes only `createClient`. */ import { beforeEach, describe, expect, it, vi } from "vitest"; @@ -15,16 +19,9 @@ vi.mock("next/cache", () => ({ vi.mock("@/lib/supabase/server", () => ({ createClient: vi.fn<(...args: never[]) => unknown>(), - service_role: { - workspace: { - storage: { - from: vi.fn<(...args: never[]) => unknown>(), - }, - }, - }, })); -import { createClient, service_role } from "@/lib/supabase/server"; +import { createClient } from "@/lib/supabase/server"; import { revalidatePath } from "next/cache"; import { updateOrganizationProfile } from "../actions"; @@ -33,12 +30,21 @@ const mockedCreateClient = vi.mocked(createClient); const ORG_NAME = "acme"; const ORG_ID = 42; -/** Build a user-authed client stub whose `.from()` chain captures the update. */ +/** + * Build a user-authed client stub. `.from("organization")` drives the + * membership gate + profile update; `.storage.from("avatars")` captures the + * upload/remove calls. + */ function makeUserClient(opts: { member?: boolean } = {}) { const member = opts.member ?? true; const update = fn().mockReturnValue({ eq: fn().mockResolvedValue({ error: null }), }); + const upload = fn().mockResolvedValue({ error: null }); + const remove = fn().mockResolvedValue({ error: null }); + const storageFrom = vi + .fn<(bucket: string) => unknown>() + .mockReturnValue({ upload, remove }); const client = { auth: { @@ -48,7 +54,6 @@ function makeUserClient(opts: { member?: boolean } = {}) { if (table !== "organization") throw new Error(`unexpected table ${table}`); return { - // org lookup / membership gate select: fn().mockReturnValue({ eq: fn().mockReturnValue({ single: fn().mockResolvedValue( @@ -58,22 +63,12 @@ function makeUserClient(opts: { member?: boolean } = {}) { ), }), }), - // profile update update, }; }), + storage: { from: storageFrom }, }; - return { client, update }; -} - -/** Capture the storage upload call. */ -function stubUpload(result: { error: unknown } = { error: null }) { - const upload = fn().mockResolvedValue(result); - vi.mocked(service_role.workspace.storage.from).mockReturnValue({ - upload, - // oxlint-disable-next-line typescript-eslint/no-explicit-any -- partial storage stub - } as any); - return upload; + return { client, update, upload, remove, storageFrom }; } function form(fields: Record) { @@ -105,10 +100,9 @@ describe("updateOrganizationProfile", () => { }); it("updates text fields without touching avatar_path when no file/remove", async () => { - const { client, update } = makeUserClient(); + const { client, update, upload, remove } = makeUserClient(); // oxlint-disable-next-line typescript-eslint/no-explicit-any -- partial client stub mockedCreateClient.mockResolvedValue(client as any); - const upload = stubUpload(); await updateOrganizationProfile( ORG_NAME, @@ -121,6 +115,7 @@ describe("updateOrganizationProfile", () => { ); expect(upload).not.toHaveBeenCalled(); + expect(remove).not.toHaveBeenCalled(); const payload = update.mock.calls[0][0]; expect(payload).toMatchObject({ display_name: "Acme Inc", @@ -134,11 +129,10 @@ describe("updateOrganizationProfile", () => { ); }); - it("uploads to org-{id}/avatar (upsert) and writes avatar_path on valid image", async () => { - const { client, update } = makeUserClient(); + it("uploads via the user-auth client to {id}/avatar (upsert) and writes avatar_path", async () => { + const { client, update, upload, storageFrom } = makeUserClient(); // oxlint-disable-next-line typescript-eslint/no-explicit-any -- partial client stub mockedCreateClient.mockResolvedValue(client as any); - const upload = stubUpload(); await updateOrganizationProfile( ORG_NAME, @@ -149,20 +143,20 @@ describe("updateOrganizationProfile", () => { }) ); - expect(service_role.workspace.storage.from).toHaveBeenCalledWith("avatars"); + // Goes through the user-authed client's storage, not service_role. + expect(storageFrom).toHaveBeenCalledWith("avatars"); const [path, , options] = upload.mock.calls[0]; - expect(path).toBe(`org-${ORG_ID}/avatar`); + expect(path).toBe(`${ORG_ID}/avatar`); expect(options).toMatchObject({ upsert: true, contentType: "image/png" }); expect(update.mock.calls[0][0]).toMatchObject({ - avatar_path: `org-${ORG_ID}/avatar`, + avatar_path: `${ORG_ID}/avatar`, }); }); - it("clears avatar_path on remove and does not upload", async () => { - const { client, update } = makeUserClient(); + it("clears avatar_path on remove, deletes the object, and does not upload", async () => { + const { client, update, upload, remove } = makeUserClient(); // oxlint-disable-next-line typescript-eslint/no-explicit-any -- partial client stub mockedCreateClient.mockResolvedValue(client as any); - const upload = stubUpload(); await updateOrganizationProfile( ORG_NAME, @@ -170,6 +164,7 @@ describe("updateOrganizationProfile", () => { ); expect(upload).not.toHaveBeenCalled(); + expect(remove).toHaveBeenCalledWith([`${ORG_ID}/avatar`]); expect(update.mock.calls[0][0]).toMatchObject({ avatar_path: null }); }); @@ -177,7 +172,6 @@ describe("updateOrganizationProfile", () => { const { client } = makeUserClient(); // oxlint-disable-next-line typescript-eslint/no-explicit-any -- partial client stub mockedCreateClient.mockResolvedValue(client as any); - stubUpload(); await expect( updateOrganizationProfile( @@ -195,7 +189,6 @@ describe("updateOrganizationProfile", () => { const { client } = makeUserClient(); // oxlint-disable-next-line typescript-eslint/no-explicit-any -- partial client stub mockedCreateClient.mockResolvedValue(client as any); - stubUpload(); await expect( updateOrganizationProfile( diff --git a/editor/app/(site)/organizations/[organization_name]/settings/profile/actions.ts b/editor/app/(site)/organizations/[organization_name]/settings/profile/actions.ts index c23852e9f4..3dc15ad0d3 100644 --- a/editor/app/(site)/organizations/[organization_name]/settings/profile/actions.ts +++ b/editor/app/(site)/organizations/[organization_name]/settings/profile/actions.ts @@ -1,13 +1,15 @@ "use server"; -// Reads + the org-row UPDATE use `createClient()` (user-authed, RLS-aware): -// the `organization` SELECT/UPDATE policies (`rls_organization`) only yield / -// accept a row for members, so they double as the authorization gate. The -// avatar object write goes through `service_role.workspace.storage` because the -// `avatars` bucket has no tracked storage RLS — the membership gate below is -// what authorizes it. - -import { createClient, service_role } from "@/lib/supabase/server"; +// Everything here runs through `createClient()` (user-authed, RLS-aware) — the +// org row read/update AND the avatar object read/write/delete. Authorization is +// enforced by RLS end to end: the `organization` SELECT/UPDATE policies +// (`rls_organization`) and the `avatars` bucket storage policies (added in +// migration `*_avatars_bucket_rls.sql`, scoped to org membership via the +// `{organization_id}/avatar` path). No service_role anywhere. The org select +// below is a lightweight gate kept only for a friendly error — RLS is the real +// boundary. + +import { createClient } from "@/lib/supabase/server"; import { revalidatePath } from "next/cache"; const AVATAR_MAX_BYTES = 2 * 1024 * 1024; // ~2MB @@ -49,10 +51,22 @@ export async function updateOrganizationProfile( const remove_avatar = formData.get("remove_avatar") === "1"; const avatar = formData.get("avatar"); + // Object path scheme: `{organization_id}/avatar`. The leading folder segment + // is what the storage RLS policy parses to authorize the write. + const path = `${org.id}/avatar`; + // `undefined` => leave avatar_path untouched. `null` => clear it. let avatar_path: string | null | undefined = undefined; if (remove_avatar) { + // Best-effort object delete (RLS-guarded by the same membership policy); + // the row going null is what drives display, so a missing object is fine. + const { error: removeError } = await client.storage + .from("avatars") + .remove([path]); + if (removeError) { + console.error("organization/profile avatar remove", removeError); + } avatar_path = null; } else if (avatar instanceof File && avatar.size > 0) { if (!AVATAR_ACCEPTED_TYPES.includes(avatar.type)) { @@ -62,12 +76,10 @@ export async function updateOrganizationProfile( throw new Error("image too large (max 2MB)"); } - // Stable per-org key + upsert: replacing the avatar overwrites in place. - const path = `org-${org.id}/avatar`; - - // service_role: the `avatars` bucket has no tracked storage RLS, so the - // membership gate above is the authorization boundary for this write. - const { error: uploadError } = await service_role.workspace.storage + // User-authed upload — the `avatars` bucket storage policy authorizes it by + // org membership (parsed from the path). Stable key + upsert overwrites in + // place on replace. + const { error: uploadError } = await client.storage .from("avatars") .upload(path, avatar, { contentType: avatar.type, diff --git a/supabase/migrations/20260604120000_avatars_bucket_rls.sql b/supabase/migrations/20260604120000_avatars_bucket_rls.sql new file mode 100644 index 0000000000..69993ba9fa --- /dev/null +++ b/supabase/migrations/20260604120000_avatars_bucket_rls.sql @@ -0,0 +1,85 @@ +-- Avatars bucket + org-scoped storage RLS. +-- +-- The `avatars` bucket holds organization avatars (and, by the same +-- public-URL read pattern, user_profile avatars). Organization avatar objects +-- live at the path `{organization_id}/avatar`; the first path segment is the +-- org id, authorized through the existing `public.rls_organization()` helper — +-- mirroring how the `storage` / `www` buckets scope writes via +-- `rls_*( (storage.foldername(name))[1]::... )`. +-- +-- Reconciliation notes: +-- * The bucket already exists in production (created via the dashboard), so +-- the insert is idempotent (`on conflict do nothing`) and a no-op there. +-- This statement exists for local-stack parity. +-- * Each policy is `drop ... if exists` first so this migration coexists with +-- any same-named policy that may already be present on the prod bucket and +-- is safe to (re)apply. +-- * The bucket is shared with non-org (e.g. user_profile, uuid-keyed) avatar +-- objects. The membership check is wrapped in a CASE so a non-numeric first +-- segment yields `false` instead of raising on the `::bigint` cast — those +-- objects fall through to whatever policy governs them, never to an error. + +insert into storage.buckets (id, name, public) +values ('avatars', 'avatars', true) +on conflict (id) do nothing; + +-- SELECT stays public to match the public-URL read pattern (public bucket). +drop policy if exists "avatars public read" on storage.objects; +create policy "avatars public read" +on storage.objects +for select +to public +using (bucket_id = 'avatars'); + +-- INSERT: an org member may create their org's avatar object. +drop policy if exists "avatars org member insert" on storage.objects; +create policy "avatars org member insert" +on storage.objects +for insert +to authenticated +with check ( + bucket_id = 'avatars' + and case + when (storage.foldername(name))[1] ~ '^[0-9]+$' + then public.rls_organization((storage.foldername(name))[1]::bigint) + else false + end +); + +-- UPDATE: covers the upsert (replace) path. +drop policy if exists "avatars org member update" on storage.objects; +create policy "avatars org member update" +on storage.objects +for update +to authenticated +using ( + bucket_id = 'avatars' + and case + when (storage.foldername(name))[1] ~ '^[0-9]+$' + then public.rls_organization((storage.foldername(name))[1]::bigint) + else false + end +) +with check ( + bucket_id = 'avatars' + and case + when (storage.foldername(name))[1] ~ '^[0-9]+$' + then public.rls_organization((storage.foldername(name))[1]::bigint) + else false + end +); + +-- DELETE: covers the remove path. +drop policy if exists "avatars org member delete" on storage.objects; +create policy "avatars org member delete" +on storage.objects +for delete +to authenticated +using ( + bucket_id = 'avatars' + and case + when (storage.foldername(name))[1] ~ '^[0-9]+$' + then public.rls_organization((storage.foldername(name))[1]::bigint) + else false + end +); From 47702bad4dde646b63b7939cbc41270983ecc87f Mon Sep 17 00:00:00 2001 From: Universe Date: Thu, 4 Jun 2026 16:35:33 +0900 Subject: [PATCH 4/7] fix(org-avatar): validate required fields + sniff image magic bytes - Reject missing display_name/email explicitly instead of persisting the literal string "null" via String(null). - Sniff png/jpeg/webp from the file's leading bytes rather than trusting the client-supplied File.type; reject mismatches and use the sniffed type as the upload contentType. - Extend tests: missing-required-field rejection, spoofed-MIME rejection, sniffed-type-wins, no-signature rejection. --- .../profile/__tests__/actions.test.ts | 93 ++++++++++++++++++- .../settings/profile/actions.ts | 92 +++++++++++++++--- 2 files changed, 168 insertions(+), 17 deletions(-) diff --git a/editor/app/(site)/organizations/[organization_name]/settings/profile/__tests__/actions.test.ts b/editor/app/(site)/organizations/[organization_name]/settings/profile/__tests__/actions.test.ts index de15f4b33d..2bd2226a93 100644 --- a/editor/app/(site)/organizations/[organization_name]/settings/profile/__tests__/actions.test.ts +++ b/editor/app/(site)/organizations/[organization_name]/settings/profile/__tests__/actions.test.ts @@ -77,8 +77,30 @@ function form(fields: Record) { return fd; } -function imageFile(type: string, bytes: number, name = "a.png") { - return new File([new Uint8Array(bytes)], name, { type }); +// Leading magic bytes the server action sniffs (it ignores `File.type`). +const MAGIC: Record = { + png: [0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a], + jpeg: [0xff, 0xd8, 0xff], + // "RIFF" + 4 size bytes + "WEBP" + webp: [0x52, 0x49, 0x46, 0x46, 0, 0, 0, 0, 0x57, 0x45, 0x42, 0x50], + // "GIF8" — a real format, but NOT one of the accepted types. + gif: [0x47, 0x49, 0x46, 0x38], +}; + +/** + * Build an image `File`. `magic` controls the real leading bytes (what the + * action sniffs); `type` is the client-declared MIME (which the action must + * NOT trust). Omit `magic` for a buffer of zero bytes (no valid signature). + */ +function imageFile( + type: string, + bytes: number, + name = "a.png", + magic?: number[] +) { + const buf = new Uint8Array(bytes); + if (magic) buf.set(magic.slice(0, bytes), 0); + return new File([buf], name, { type }); } beforeEach(() => { @@ -139,7 +161,7 @@ describe("updateOrganizationProfile", () => { form({ display_name: "Acme", email: "a@acme.com", - avatar: imageFile("image/png", 1024), + avatar: imageFile("image/png", 1024, "a.png", MAGIC.png), }) ); @@ -153,6 +175,25 @@ describe("updateOrganizationProfile", () => { }); }); + it("uses the sniffed type as contentType, ignoring a spoofed File.type", async () => { + const { client, upload } = makeUserClient(); + // oxlint-disable-next-line typescript-eslint/no-explicit-any -- partial client stub + mockedCreateClient.mockResolvedValue(client as any); + + // Declared as PNG, but the real bytes are JPEG — sniffed type wins. + await updateOrganizationProfile( + ORG_NAME, + form({ + display_name: "Acme", + email: "a@acme.com", + avatar: imageFile("image/png", 1024, "a.png", MAGIC.jpeg), + }) + ); + + const [, , options] = upload.mock.calls[0]; + expect(options).toMatchObject({ contentType: "image/jpeg" }); + }); + it("clears avatar_path on remove, deletes the object, and does not upload", async () => { const { client, update, upload, remove } = makeUserClient(); // oxlint-disable-next-line typescript-eslint/no-explicit-any -- partial client stub @@ -168,7 +209,26 @@ describe("updateOrganizationProfile", () => { expect(update.mock.calls[0][0]).toMatchObject({ avatar_path: null }); }); - it("rejects unsupported image types", async () => { + it("rejects a spoofed MIME — accepted File.type but disallowed magic bytes", async () => { + const { client, upload } = makeUserClient(); + // oxlint-disable-next-line typescript-eslint/no-explicit-any -- partial client stub + mockedCreateClient.mockResolvedValue(client as any); + + // Claims to be a PNG, but the bytes are a real GIF (not an accepted type). + await expect( + updateOrganizationProfile( + ORG_NAME, + form({ + display_name: "Acme", + email: "a@acme.com", + avatar: imageFile("image/png", 1024, "a.png", MAGIC.gif), + }) + ) + ).rejects.toThrow(/unsupported image type/); + expect(upload).not.toHaveBeenCalled(); + }); + + it("rejects a file whose bytes match no accepted image signature", async () => { const { client } = makeUserClient(); // oxlint-disable-next-line typescript-eslint/no-explicit-any -- partial client stub mockedCreateClient.mockResolvedValue(client as any); @@ -179,12 +239,35 @@ describe("updateOrganizationProfile", () => { form({ display_name: "Acme", email: "a@acme.com", - avatar: imageFile("image/gif", 1024, "a.gif"), + // Zero-filled buffer: no magic match. + avatar: imageFile("image/png", 1024), }) ) ).rejects.toThrow(/unsupported image type/); }); + it("rejects when display_name is missing", async () => { + const { client, update } = makeUserClient(); + // oxlint-disable-next-line typescript-eslint/no-explicit-any -- partial client stub + mockedCreateClient.mockResolvedValue(client as any); + + await expect( + updateOrganizationProfile(ORG_NAME, form({ email: "a@acme.com" })) + ).rejects.toThrow(/display name is required/); + expect(update).not.toHaveBeenCalled(); + }); + + it("rejects when email is missing", async () => { + const { client, update } = makeUserClient(); + // oxlint-disable-next-line typescript-eslint/no-explicit-any -- partial client stub + mockedCreateClient.mockResolvedValue(client as any); + + await expect( + updateOrganizationProfile(ORG_NAME, form({ display_name: "Acme" })) + ).rejects.toThrow(/email is required/); + expect(update).not.toHaveBeenCalled(); + }); + it("rejects images larger than 2MB", async () => { const { client } = makeUserClient(); // oxlint-disable-next-line typescript-eslint/no-explicit-any -- partial client stub diff --git a/editor/app/(site)/organizations/[organization_name]/settings/profile/actions.ts b/editor/app/(site)/organizations/[organization_name]/settings/profile/actions.ts index 3dc15ad0d3..108fc52379 100644 --- a/editor/app/(site)/organizations/[organization_name]/settings/profile/actions.ts +++ b/editor/app/(site)/organizations/[organization_name]/settings/profile/actions.ts @@ -13,7 +13,56 @@ import { createClient } from "@/lib/supabase/server"; import { revalidatePath } from "next/cache"; const AVATAR_MAX_BYTES = 2 * 1024 * 1024; // ~2MB -const AVATAR_ACCEPTED_TYPES = ["image/png", "image/jpeg", "image/webp"]; + +/** + * Sniff the real image type from the file's leading bytes instead of trusting + * the client-supplied `File.type` (which is attacker-controlled). Returns the + * canonical MIME for png/jpeg/webp, or `null` when the magic bytes match none + * of the accepted formats. + */ +function sniffImageType(bytes: Uint8Array): string | null { + // PNG: 89 50 4E 47 0D 0A 1A 0A + if ( + bytes.length >= 8 && + bytes[0] === 0x89 && + bytes[1] === 0x50 && + bytes[2] === 0x4e && + bytes[3] === 0x47 && + bytes[4] === 0x0d && + bytes[5] === 0x0a && + bytes[6] === 0x1a && + bytes[7] === 0x0a + ) { + return "image/png"; + } + + // JPEG: FF D8 FF + if ( + bytes.length >= 3 && + bytes[0] === 0xff && + bytes[1] === 0xd8 && + bytes[2] === 0xff + ) { + return "image/jpeg"; + } + + // WEBP: "RIFF" .... "WEBP" (RIFF container with a WEBP fourCC at offset 8) + if ( + bytes.length >= 12 && + bytes[0] === 0x52 && // R + bytes[1] === 0x49 && // I + bytes[2] === 0x46 && // F + bytes[3] === 0x46 && // F + bytes[8] === 0x57 && // W + bytes[9] === 0x45 && // E + bytes[10] === 0x42 && // B + bytes[11] === 0x50 // P + ) { + return "image/webp"; + } + + return null; +} /** * Update an organization's general profile (display name, contact email, @@ -44,13 +93,25 @@ export async function updateOrganizationProfile( throw new Error("organization not found or forbidden"); } - const display_name = formData.get("display_name"); - const email = formData.get("email"); + const display_name_raw = formData.get("display_name"); + const email_raw = formData.get("email"); const description = formData.get("description"); const blog = formData.get("blog"); const remove_avatar = formData.get("remove_avatar") === "1"; const avatar = formData.get("avatar"); + // `display_name` and `email` are required. Validate presence explicitly — + // `String(null)` would otherwise persist the literal string "null". + const display_name = + typeof display_name_raw === "string" ? display_name_raw.trim() : ""; + if (!display_name) { + throw new Error("display name is required"); + } + const email = typeof email_raw === "string" ? email_raw.trim() : ""; + if (!email) { + throw new Error("email is required"); + } + // Object path scheme: `{organization_id}/avatar`. The leading folder segment // is what the storage RLS policy parses to authorize the write. const path = `${org.id}/avatar`; @@ -69,20 +130,26 @@ export async function updateOrganizationProfile( } avatar_path = null; } else if (avatar instanceof File && avatar.size > 0) { - if (!AVATAR_ACCEPTED_TYPES.includes(avatar.type)) { - throw new Error(`unsupported image type: ${avatar.type || "unknown"}`); - } if (avatar.size > AVATAR_MAX_BYTES) { throw new Error("image too large (max 2MB)"); } + // Don't trust the client-reported `avatar.type` — sniff the real format + // from the leading bytes and reject anything that isn't png/jpeg/webp. + const buffer = await avatar.arrayBuffer(); + const sniffed = sniffImageType(new Uint8Array(buffer)); + if (!sniffed) { + throw new Error(`unsupported image type: ${avatar.type || "unknown"}`); + } + // User-authed upload — the `avatars` bucket storage policy authorizes it by // org membership (parsed from the path). Stable key + upsert overwrites in - // place on replace. + // place on replace. Upload the already-read buffer with the sniffed type as + // the authoritative contentType. const { error: uploadError } = await client.storage .from("avatars") - .upload(path, avatar, { - contentType: avatar.type, + .upload(path, buffer, { + contentType: sniffed, upsert: true, }); @@ -94,12 +161,13 @@ export async function updateOrganizationProfile( avatar_path = path; } - // Mirrors the prior route's field handling (empty string => leave untouched). + // `display_name`/`email` are validated non-empty above. Optional fields: + // empty/absent => leave untouched (undefined is omitted from the update). const { error: updateError } = await client .from("organization") .update({ - display_name: String(display_name), - email: String(email), + display_name, + email, description: description ? String(description) : undefined, blog: blog ? String(blog) : undefined, ...(avatar_path !== undefined ? { avatar_path } : {}), From 3be06a399b4bcc79329f98a4c44e23f7b5c57d20 Mon Sep 17 00:00:00 2001 From: Universe Date: Thu, 4 Jun 2026 16:35:35 +0900 Subject: [PATCH 5/7] fix(org-avatar): encode avatar upload through the server action The picked avatar File never reached the server action: the lacked encType="multipart/form-data", and Next 16's default server-action bodySizeLimit (1MB) is below the 2MB avatar cap, 413-ing 1-2MB files before the action runs. - page.tsx: add encType="multipart/form-data" to the profile form. - next.config.ts: set experimental.serverActions.bodySizeLimit to '3mb' to fit the 2MB cap plus multipart overhead. --- .../[organization_name]/settings/profile/page.tsx | 3 +++ editor/next.config.ts | 6 ++++++ 2 files changed, 9 insertions(+) diff --git a/editor/app/(site)/organizations/[organization_name]/settings/profile/page.tsx b/editor/app/(site)/organizations/[organization_name]/settings/profile/page.tsx index 73f64bae96..f6c532b7b2 100644 --- a/editor/app/(site)/organizations/[organization_name]/settings/profile/page.tsx +++ b/editor/app/(site)/organizations/[organization_name]/settings/profile/page.tsx @@ -61,6 +61,9 @@ export default async function OrganizationsSettingsProfilePage({ diff --git a/editor/next.config.ts b/editor/next.config.ts index c79549f695..cf6c6ac1e2 100644 --- a/editor/next.config.ts +++ b/editor/next.config.ts @@ -18,6 +18,12 @@ const nextConfig: NextConfig = { pageExtensions: ["js", "jsx", "mdx", "ts", "tsx"], experimental: { mdxRs: true, + serverActions: { + // Default server-action body limit is 1MB; the org avatar cap is 2MB, so + // a 2MB file + multipart overhead would be rejected before reaching the + // action. Raise the limit to comfortably fit the 2MB cap. + bodySizeLimit: "3mb", + }, }, // CI typechecks via .github/workflows/test.yml (pnpm typecheck). Skipping // the in-build TypeScript pass removes ~30s of redundant work from every From db011fd244c02a6a49d0cc1aa78434a2a01c4b14 Mon Sep 17 00:00:00 2001 From: Universe Date: Thu, 4 Jun 2026 16:35:40 +0900 Subject: [PATCH 6/7] fix(org-avatar): encode multipart form + revoke preview object URLs - Add encType="multipart/form-data" to the profile form so the picked avatar File is actually encoded into the server action request. - Revoke the createObjectURL preview on replace and unmount to stop the blob URL leak. - Widen display_name prop to string | null | undefined (it's nullable at the call site). --- .../settings/profile/avatar-field.tsx | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/editor/app/(site)/organizations/[organization_name]/settings/profile/avatar-field.tsx b/editor/app/(site)/organizations/[organization_name]/settings/profile/avatar-field.tsx index bb4a193902..5ae472eb5c 100644 --- a/editor/app/(site)/organizations/[organization_name]/settings/profile/avatar-field.tsx +++ b/editor/app/(site)/organizations/[organization_name]/settings/profile/avatar-field.tsx @@ -1,6 +1,6 @@ "use client"; -import React, { useRef, useState } from "react"; +import React, { useEffect, useRef, useState } from "react"; import { Avatar, AvatarFallback, AvatarImage } from "@/components/ui/avatar"; import { Button } from "@/components/ui/button"; @@ -17,7 +17,7 @@ export function OrganizationAvatarField({ display_name, }: { current_avatar_url?: string | null; - display_name: string; + display_name?: string | null; }) { const inputRef = useRef(null); const [preview, setPreview] = useState(null); @@ -25,6 +25,13 @@ export function OrganizationAvatarField({ const shown = removed ? null : (preview ?? current_avatar_url ?? null); + // Revoke the object URL when it's replaced (deps change) or on unmount, so + // picking multiple files (or leaving the page) doesn't leak blob URLs. + useEffect(() => { + if (!preview) return; + return () => URL.revokeObjectURL(preview); + }, [preview]); + const onPick = (e: React.ChangeEvent) => { const file = e.target.files?.[0]; if (!file) return; From db37bcdf99e75d3c5514066b3acdde4f1cc3841e Mon Sep 17 00:00:00 2001 From: Universe Date: Thu, 4 Jun 2026 16:35:47 +0900 Subject: [PATCH 7/7] docs(org-avatar): correct avatars bucket RLS reconciliation note MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These are the only write policies on the avatars bucket. Under deny-by-default RLS, a non-numeric (e.g. uuid-keyed) path is simply denied for insert/update/delete — it does not fall through to another policy, because none exists. No user_profile avatar upload path exists in the codebase today. --- .../migrations/20260604120000_avatars_bucket_rls.sql | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/supabase/migrations/20260604120000_avatars_bucket_rls.sql b/supabase/migrations/20260604120000_avatars_bucket_rls.sql index 69993ba9fa..855ecb3efc 100644 --- a/supabase/migrations/20260604120000_avatars_bucket_rls.sql +++ b/supabase/migrations/20260604120000_avatars_bucket_rls.sql @@ -14,10 +14,14 @@ -- * Each policy is `drop ... if exists` first so this migration coexists with -- any same-named policy that may already be present on the prod bucket and -- is safe to (re)apply. --- * The bucket is shared with non-org (e.g. user_profile, uuid-keyed) avatar --- objects. The membership check is wrapped in a CASE so a non-numeric first --- segment yields `false` instead of raising on the `::bigint` cast — those --- objects fall through to whatever policy governs them, never to an error. +-- * These are the ONLY write policies on the `avatars` bucket. The membership +-- check is wrapped in a CASE so a non-numeric first segment yields `false` +-- instead of raising on the `::bigint` cast. Because storage.objects is +-- deny-by-default under RLS and no other write policy exists, a non-numeric +-- (e.g. uuid-keyed) path is simply DENIED for insert/update/delete — it does +-- not "fall through" to anything. Org avatar objects live at +-- `{organization_id}/avatar`; only that numeric-prefixed shape is writable +-- here today. Reads stay public (see the public-read policy below). insert into storage.buckets (id, name, public) values ('avatars', 'avatars', true)