From 62060b8f083ae9bf4c766affb6e29821209acb84 Mon Sep 17 00:00:00 2001 From: Katia Bulatova Date: Thu, 27 Aug 2026 07:01:56 +0000 Subject: [PATCH 01/25] feat(webapp): add a profile photo editor modal with circular crop and zoom --- .../app/components/ProfilePhotoEditor.tsx | 198 ++++++++++++++++++ .../storybook.profile-photo-editor/route.tsx | 43 ++++ apps/webapp/app/routes/storybook/route.tsx | 1 + apps/webapp/app/tailwind.css | 1 + apps/webapp/package.json | 1 + pnpm-lock.yaml | 20 ++ 6 files changed, 264 insertions(+) create mode 100644 apps/webapp/app/components/ProfilePhotoEditor.tsx create mode 100644 apps/webapp/app/routes/storybook.profile-photo-editor/route.tsx diff --git a/apps/webapp/app/components/ProfilePhotoEditor.tsx b/apps/webapp/app/components/ProfilePhotoEditor.tsx new file mode 100644 index 00000000000..dd48178192e --- /dev/null +++ b/apps/webapp/app/components/ProfilePhotoEditor.tsx @@ -0,0 +1,198 @@ +import { MagnifyingGlassMinusIcon, MagnifyingGlassPlusIcon } from "@heroicons/react/20/solid"; +import { useEffect, useRef, useState } from "react"; +import Cropper, { type Area, type Point } from "react-easy-crop"; +import { Button } from "./primitives/Buttons"; +import { + Dialog, + DialogContent, + DialogFooter, + DialogHeader, + DialogTitle, +} from "./primitives/Dialog"; +import { Paragraph } from "./primitives/Paragraph"; +import { Slider } from "./primitives/Slider"; + +const ACCEPTED_TYPES = ["image/png", "image/jpeg", "image/webp"]; +const OUTPUT_SIZE = 512; +const MIN_ZOOM = 1; +const MAX_ZOOM = 3; +const ZOOM_STEP = 0.01; +const CENTER: Point = { x: 0, y: 0 }; + +async function cropImageToBlob(imageSrc: string, area: Area): Promise { + const image = await loadImage(imageSrc); + const canvas = document.createElement("canvas"); + canvas.width = OUTPUT_SIZE; + canvas.height = OUTPUT_SIZE; + + const context = canvas.getContext("2d"); + if (!context) { + throw new Error("Could not create a canvas to crop the image"); + } + + context.drawImage(image, area.x, area.y, area.width, area.height, 0, 0, OUTPUT_SIZE, OUTPUT_SIZE); + + return await new Promise((resolve, reject) => { + canvas.toBlob((blob) => { + if (blob) { + resolve(blob); + } else { + reject(new Error("Could not crop the image")); + } + }, "image/png"); + }); +} + +function loadImage(src: string): Promise { + return new Promise((resolve, reject) => { + const image = new Image(); + image.addEventListener("load", () => resolve(image)); + image.addEventListener("error", () => reject(new Error("Could not load the image"))); + image.src = src; + }); +} + +type ProfilePhotoEditorProps = { + open: boolean; + onOpenChange: (open: boolean) => void; + onSave: (blob: Blob) => void; + isSaving?: boolean; +}; + +export function ProfilePhotoEditor({ + open, + onOpenChange, + onSave, + isSaving = false, +}: ProfilePhotoEditorProps) { + return ( + + + + Profile picture + + {/* Radix unmounts the content when closed, so the crop state resets with it. */} + + + + ); +} + +function Editor({ onSave, isSaving }: Pick) { + const fileInputRef = useRef(null); + const [imageSrc, setImageSrc] = useState(); + const [crop, setCrop] = useState(CENTER); + const [zoom, setZoom] = useState(MIN_ZOOM); + const [croppedArea, setCroppedArea] = useState(); + const [error, setError] = useState(); + + useEffect(() => { + if (!imageSrc) return; + return () => URL.revokeObjectURL(imageSrc); + }, [imageSrc]); + + function selectFile(file: File | undefined) { + if (!file) return; + + if (!ACCEPTED_TYPES.includes(file.type)) { + setError("Choose a PNG, JPEG or WebP image."); + return; + } + + setCrop(CENTER); + setZoom(MIN_ZOOM); + setCroppedArea(undefined); + setError(undefined); + setImageSrc(URL.createObjectURL(file)); + } + + async function save() { + if (!imageSrc || !croppedArea) return; + + try { + onSave(await cropImageToBlob(imageSrc, croppedArea)); + } catch { + setError("Could not crop that image. Try another one."); + } + } + + return ( + <> +
+ { + selectFile(event.target.files?.[0]); + // Or re-picking the same file after an error fires no change event. + event.target.value = ""; + }} + /> + {imageSrc ? ( + <> +
+ setCroppedArea(areaPixels)} + /> +
+ setZoom(value)} + disabled={isSaving} + LeadingIcon={MagnifyingGlassMinusIcon} + TrailingIcon={MagnifyingGlassPlusIcon} + /> + + ) : ( + + )} + {error && ( + + {error} + + )} +
+ + + + + + ); +} diff --git a/apps/webapp/app/routes/storybook.profile-photo-editor/route.tsx b/apps/webapp/app/routes/storybook.profile-photo-editor/route.tsx new file mode 100644 index 00000000000..9eba2f3b57c --- /dev/null +++ b/apps/webapp/app/routes/storybook.profile-photo-editor/route.tsx @@ -0,0 +1,43 @@ +import { useState } from "react"; +import { ProfilePhotoEditor } from "~/components/ProfilePhotoEditor"; +import { Button } from "~/components/primitives/Buttons"; +import { Story, StoryGrid, StoryPage, StorySection } from "../storybook/StoryKit"; + +function EditorStory({ isSaving }: { isSaving?: boolean }) { + const [open, setOpen] = useState(false); + + return ( + <> + + setOpen(false)} + isSaving={isSaving} + /> + + ); +} + +export default function Story_() { + return ( + + + + + + + + + + + + + ); +} diff --git a/apps/webapp/app/routes/storybook/route.tsx b/apps/webapp/app/routes/storybook/route.tsx index 35f51682938..d962c9b309d 100644 --- a/apps/webapp/app/routes/storybook/route.tsx +++ b/apps/webapp/app/routes/storybook/route.tsx @@ -70,6 +70,7 @@ const sections: StorySection[] = [ { name: "Popover", slug: "popover" }, { name: "Filter", slug: "filter" }, { name: "Dialog", slug: "dialog" }, + { name: "Profile photo editor", slug: "profile-photo-editor" }, { name: "Sheet", slug: "sheet" }, { name: "Tooltip", slug: "tooltip" }, ], diff --git a/apps/webapp/app/tailwind.css b/apps/webapp/app/tailwind.css index fdacb677ca1..6d8d6c6a6b5 100644 --- a/apps/webapp/app/tailwind.css +++ b/apps/webapp/app/tailwind.css @@ -1,5 +1,6 @@ @import "react-grid-layout/css/styles.css" layer(base); @import "react-resizable/css/styles.css" layer(base); +@import "react-easy-crop/react-easy-crop.css" layer(base); @import "tailwindcss"; @import "tw-animate-css"; diff --git a/apps/webapp/package.json b/apps/webapp/package.json index 788342f23bb..5ff62b45d68 100644 --- a/apps/webapp/package.json +++ b/apps/webapp/package.json @@ -190,6 +190,7 @@ "react": "^18.2.0", "react-day-picker": "^9.13.0", "react-dom": "^18.2.0", + "react-easy-crop": "^6.2.3", "react-grid-layout": "^2.2.2", "react-hotkeys-hook": "^4.4.1", "react-markdown": "^10.1.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a7a0a3e5d54..851545d078a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -684,6 +684,9 @@ importers: react-dom: specifier: 18.3.1 version: 18.3.1(react@18.3.1) + react-easy-crop: + specifier: ^6.2.3 + version: 6.2.3(react-dom@18.3.1(react@18.3.1))(react@18.3.1) react-grid-layout: specifier: ^2.2.2 version: 2.2.2(react-dom@18.3.1(react@18.3.1))(react@18.3.1) @@ -12176,6 +12179,9 @@ packages: resolution: {integrity: sha512-6eZs5Ls3WtCisHWp9S2GUy8dqkpGi4BVSz3GaqiE6ezub0512ESztXUwUB6C6IKbQkY2Pnb/mD4WYojCRwcwLA==} engines: {node: '>=0.10.0'} + normalize-wheel@1.0.1: + resolution: {integrity: sha512-1OnlAPZ3zgrk8B91HyRj+eVv+kS5u+Z0SCsak6Xil/kmgEia50ga7zfkumayonZrImffAxPU/5WcyGhzetHNPA==} + notepack.io@3.0.1: resolution: {integrity: sha512-TKC/8zH5pXIAMVQio2TvVDTtPRX+DJPHDqjRbxogtFiByHyzKmy96RA0JtCQJ+WouyyL4A10xomQzgbUT+1jCg==} @@ -13026,6 +13032,12 @@ packages: react: 18.3.1 react-dom: 18.3.1 + react-easy-crop@6.2.3: + resolution: {integrity: sha512-ebimG3OGlzizjxEZ77Cj9CVLcg5vDZ6QVz8ud1rx4WmsCDWHIROEhgMi0GpJ/jKAuwHrH0M+QZUK4hyvxbYhOA==} + peerDependencies: + react: 18.3.1 + react-dom: 18.3.1 + react-email@6.5.0: resolution: {integrity: sha512-WrJ+XPW87O1dabF4RJNGnTr7VTGsNa+BlMiinAZdH5fg8Kepwk++ZzX+LEieTlk+a3r13TaTJ4DfI9gv++y02g==} engines: {node: '>=20.0.0'} @@ -27098,6 +27110,8 @@ snapshots: normalize-path@3.0.0: {} + normalize-wheel@1.0.1: {} + notepack.io@3.0.1: {} npm-install-checks@6.2.0: @@ -28044,6 +28058,12 @@ snapshots: react: 18.3.1 react-dom: 18.3.1(react@18.3.1) + react-easy-crop@6.2.3(react-dom@18.3.1(react@18.3.1))(react@18.3.1): + dependencies: + normalize-wheel: 1.0.1 + react: 18.3.1 + react-dom: 18.3.1(react@18.3.1) + react-email@6.5.0(react-dom@18.3.1(react@18.3.1))(react@18.3.1): dependencies: '@babel/parser': 7.27.0 From 01619a933fcdfaab653767e2df39d652b7e90d85 Mon Sep 17 00:00:00 2001 From: Katia Bulatova Date: Thu, 27 Aug 2026 07:02:42 +0000 Subject: [PATCH 02/25] feat(webapp): store profile photos in S3 and serve them presigned --- apps/webapp/app/models/user.server.ts | 7 ++ ...ources.account.avatar.$userId.$filename.ts | 19 +++ .../app/routes/resources.account.avatar.ts | 28 +++++ .../services/dashboardAgentBodyCap.server.ts | 61 ++++++++-- apps/webapp/app/services/userAvatar.server.ts | 108 +++++++++++++++++ apps/webapp/app/utils/avatarLimits.ts | 13 ++ apps/webapp/app/v3/objectStore.server.ts | 2 +- .../webapp/app/v3/objectStoreClient.server.ts | 19 ++- .../webapp/test/dashboardAgentBodyCap.test.ts | 77 +++++++++++- apps/webapp/test/userAvatar.test.ts | 114 ++++++++++++++++++ 10 files changed, 433 insertions(+), 15 deletions(-) create mode 100644 apps/webapp/app/routes/resources.account.avatar.$userId.$filename.ts create mode 100644 apps/webapp/app/routes/resources.account.avatar.ts create mode 100644 apps/webapp/app/services/userAvatar.server.ts create mode 100644 apps/webapp/app/utils/avatarLimits.ts create mode 100644 apps/webapp/test/userAvatar.test.ts diff --git a/apps/webapp/app/models/user.server.ts b/apps/webapp/app/models/user.server.ts index 302e16c2953..139df9db4e3 100644 --- a/apps/webapp/app/models/user.server.ts +++ b/apps/webapp/app/models/user.server.ts @@ -416,6 +416,13 @@ export function updateUserEmail({ id, email }: Pick) { }); } +export function updateUserAvatarUrl({ id, avatarUrl }: Pick) { + return prisma.user.update({ + where: { id }, + data: { avatarUrl }, + }); +} + /** * `updateMany` so the WHERE does the comparing: a redundant request updates zero * rows rather than churning the row and its updatedAt. diff --git a/apps/webapp/app/routes/resources.account.avatar.$userId.$filename.ts b/apps/webapp/app/routes/resources.account.avatar.$userId.$filename.ts new file mode 100644 index 00000000000..3c7059a2ab4 --- /dev/null +++ b/apps/webapp/app/routes/resources.account.avatar.$userId.$filename.ts @@ -0,0 +1,19 @@ +import { redirect, type LoaderFunctionArgs } from "@remix-run/node"; +import { requireUser } from "~/services/session.server"; +import { presignUserAvatarUrl, resolveUserAvatarObjectPath } from "~/services/userAvatar.server"; + +/** + * Presigned URLs expire, so the stored avatarUrl points here and we sign on each request. + */ +export async function loader({ request, params }: LoaderFunctionArgs) { + await requireUser(request); + + const { userId, filename } = params; + const objectPath = userId && filename ? resolveUserAvatarObjectPath(userId, filename) : undefined; + + if (!objectPath) { + throw new Response("Not found", { status: 404 }); + } + + return redirect(await presignUserAvatarUrl(objectPath)); +} diff --git a/apps/webapp/app/routes/resources.account.avatar.ts b/apps/webapp/app/routes/resources.account.avatar.ts new file mode 100644 index 00000000000..22db23993e6 --- /dev/null +++ b/apps/webapp/app/routes/resources.account.avatar.ts @@ -0,0 +1,28 @@ +import { json, type ActionFunctionArgs } from "@remix-run/node"; +import { updateUserAvatarUrl } from "~/models/user.server"; +import { requireUser } from "~/services/session.server"; +import { + isAvatarUploadRejection, + parseAvatarUpload, + uploadUserAvatar, +} from "~/services/userAvatar.server"; + +export async function action({ request }: ActionFunctionArgs) { + if (request.method.toUpperCase() !== "POST") { + return json({ error: "Method not allowed" }, { status: 405 }); + } + + const user = await requireUser(request); + + const upload = await parseAvatarUpload(await request.formData()); + + if (isAvatarUploadRejection(upload)) { + return json({ error: upload.error }, { status: upload.status }); + } + + const { avatarUrl } = await uploadUserAvatar({ userId: user.id, ...upload }); + + await updateUserAvatarUrl({ id: user.id, avatarUrl }); + + return json({ avatarUrl }); +} diff --git a/apps/webapp/app/services/dashboardAgentBodyCap.server.ts b/apps/webapp/app/services/dashboardAgentBodyCap.server.ts index 0ca6dd65b4d..125f513e7f0 100644 --- a/apps/webapp/app/services/dashboardAgentBodyCap.server.ts +++ b/apps/webapp/app/services/dashboardAgentBodyCap.server.ts @@ -4,6 +4,7 @@ import { MESSAGE_TOO_LARGE_CODE, MESSAGE_TOO_LARGE_ERROR, } from "~/components/dashboard-agent/message-limits"; +import { MAX_AVATAR_SIZE_IN_BYTES } from "~/utils/avatarLimits"; /** * The ingress cap for the agent's chat paths. A route can only refuse a body after it has read @@ -23,12 +24,53 @@ export const DASHBOARD_AGENT_MAX_INGRESS_BYTES = MAX_MESSAGE_BODY_BYTES + INGRES const AGENT_PATH = /^(?:\/api\/v1\/dashboard-agent|\/resources\/orgs\/[^/]+\/projects\/[^/]+\/env\/[^/]+\/dashboard-agent)(\/|$)/; +/** Headroom over the avatar cap for multipart framing. */ +const AVATAR_INGRESS_SLACK_BYTES = 8 * 1024; + +export const AVATAR_MAX_INGRESS_BYTES = MAX_AVATAR_SIZE_IN_BYTES + AVATAR_INGRESS_SLACK_BYTES; + +/** The avatar upload only: the presigned-redirect routes below it take two more segments. */ +const AVATAR_PATH = /^\/resources\/account\/avatar\/*$/; + /** Methods that can carry one. GET and HEAD cannot, and streaming them would be wasted work. */ const BODY_METHODS = new Set(["POST", "PUT", "PATCH", "DELETE"]); -function refuse(res: Response): void { +type Cap = { + limit: number; + body: { error: string; code?: string }; +}; + +const AGENT_CAP: Cap = { + limit: DASHBOARD_AGENT_MAX_INGRESS_BYTES, + body: { error: MESSAGE_TOO_LARGE_ERROR, code: MESSAGE_TOO_LARGE_CODE }, +}; + +const AVATAR_CAP: Cap = { + limit: AVATAR_MAX_INGRESS_BYTES, + body: { error: "Image is too large" }, +}; + +/** + * Matched against the path Remix will route, not the raw one: the express adapter rebuilds the + * request through `new URL`, so `/…/avatar/.` reaches the action as `/…/avatar/`. + */ +function pathToMatch(req: Request): string { + try { + return new URL(req.originalUrl || req.url, "http://localhost").pathname.toLowerCase(); + } catch { + return req.path.toLowerCase(); + } +} + +function capForPath(path: string): Cap | undefined { + if (AGENT_PATH.test(path)) return AGENT_CAP; + if (AVATAR_PATH.test(path)) return AVATAR_CAP; + return undefined; +} + +function refuse(res: Response, cap: Cap): void { if (res.headersSent) return; - res.status(413).json({ error: MESSAGE_TOO_LARGE_ERROR, code: MESSAGE_TOO_LARGE_CODE }); + res.status(413).json(cap.body); } /** @@ -36,10 +78,11 @@ function refuse(res: Response): void { * reader still receives every chunk while nothing flows until it asks for it. Crossing the * limit ends the request: pausing alone wouldn't stop the route resuming the stream itself. */ -function capRequestBody(req: Request, res: Response, limit: number): void { +function capRequestBody(req: Request, res: Response, cap: Cap): void { + const { limit } = cap; const declared = Number.parseInt(req.headers["content-length"] ?? "", 10); if (Number.isFinite(declared) && declared > limit) { - refuse(res); + refuse(res, cap); return; } @@ -49,7 +92,7 @@ function capRequestBody(req: Request, res: Response, limit: number): void { if (received <= limit) return; req.off("data", onData); req.pause(); - refuse(res); + refuse(res, cap); // Torn down only once the refusal is on the wire, or the client never reads it. res.once("finish", () => req.destroy()); }; @@ -60,16 +103,18 @@ function capRequestBody(req: Request, res: Response, limit: number): void { } /** - * Only the agent's own paths: every other route keeps the body handling it had. Matched + * Only the capped paths: every other route keeps the body handling it had. Matched * case-insensitively because Remix routes are, and on every method — a DELETE reads a body too. */ export function dashboardAgentBodyCap(req: Request, res: Response, next: NextFunction): void { - if (!BODY_METHODS.has(req.method) || !AGENT_PATH.test(req.path.toLowerCase())) { + const cap = BODY_METHODS.has(req.method) ? capForPath(pathToMatch(req)) : undefined; + + if (!cap) { next(); return; } - capRequestBody(req, res, DASHBOARD_AGENT_MAX_INGRESS_BYTES); + capRequestBody(req, res, cap); if (res.headersSent) return; next(); } diff --git a/apps/webapp/app/services/userAvatar.server.ts b/apps/webapp/app/services/userAvatar.server.ts new file mode 100644 index 00000000000..c4d1f01c3da --- /dev/null +++ b/apps/webapp/app/services/userAvatar.server.ts @@ -0,0 +1,108 @@ +import { createHash } from "node:crypto"; +import { + AVATAR_EXTENSIONS, + type AvatarContentType, + isAvatarContentType, + MAX_AVATAR_SIZE_IN_BYTES, +} from "~/utils/avatarLimits"; +import { getObjectStoreClient } from "~/v3/objectStore.server"; + +/** Avatars always live in plain S3, never the default/R2 protocol. */ +const AVATAR_STORE_PROTOCOL = "s3"; +const AVATAR_PRESIGN_EXPIRY_IN_SECONDS = 300; + +const AVATAR_FILENAME_REGEX = /^[0-9a-f]{32}\.(png|jpg|webp)$/; +const USER_ID_REGEX = /^[A-Za-z0-9_-]+$/; + +/** The first segment of a logical key is the bucket, as with `packets/…`. */ +function requireAvatarObjectStore() { + const client = getObjectStoreClient(AVATAR_STORE_PROTOCOL); + + if (!client) { + throw new Error(`Object store is not configured for protocol: ${AVATAR_STORE_PROTOCOL}`); + } + + if (!client.bucket) { + throw new Error("OBJECT_STORE_S3_BUCKET is required to store avatars"); + } + + return { client, objectKey: (path: string) => `${client.bucket}/${path}` }; +} + +export function buildUserAvatarUrl(userId: string, filename: string) { + return `/resources/account/avatar/${userId}/${filename}`; +} + +export function buildUserAvatarFilename(contentType: AvatarContentType, data: Uint8Array) { + const hash = createHash("sha256").update(data).digest("hex").slice(0, 32); + return `${hash}.${AVATAR_EXTENSIONS[contentType]}`; +} + +/** Undefined when the params can't name an avatar object, so callers 404 instead of signing. */ +export function resolveUserAvatarObjectPath(userId: string, filename: string): string | undefined { + if (!USER_ID_REGEX.test(userId) || !AVATAR_FILENAME_REGEX.test(filename)) { + return undefined; + } + + return `avatars/${userId}/${filename}`; +} + +export type AvatarUpload = { contentType: AvatarContentType; data: Uint8Array }; +export type AvatarUploadRejection = { error: string; status: 400 | 413 | 415 }; + +/** + * Nothing here can name the key's user: the id comes from the session, never from the body. + */ +export async function parseAvatarUpload( + formData: FormData +): Promise { + const image = formData.get("image"); + + if (!(image instanceof File)) { + return { error: "Missing image", status: 400 }; + } + + if (!isAvatarContentType(image.type)) { + return { error: "Unsupported image type", status: 415 }; + } + + if (image.size > MAX_AVATAR_SIZE_IN_BYTES) { + return { error: "Image is too large", status: 413 }; + } + + return { contentType: image.type, data: new Uint8Array(await image.arrayBuffer()) }; +} + +export function isAvatarUploadRejection( + upload: AvatarUpload | AvatarUploadRejection +): upload is AvatarUploadRejection { + return "error" in upload; +} + +export async function uploadUserAvatar({ + userId, + contentType, + data, +}: { + userId: string; + contentType: AvatarContentType; + data: Uint8Array; +}) { + const filename = buildUserAvatarFilename(contentType, data); + const path = resolveUserAvatarObjectPath(userId, filename); + + if (!path) { + throw new Error("Invalid avatar object path"); + } + + const { client, objectKey } = requireAvatarObjectStore(); + await client.putObject(objectKey(path), data, contentType); + + return { avatarUrl: buildUserAvatarUrl(userId, filename) }; +} + +export function presignUserAvatarUrl(objectPath: string) { + const { client, objectKey } = requireAvatarObjectStore(); + + return client.presign(objectKey(objectPath), "GET", AVATAR_PRESIGN_EXPIRY_IN_SECONDS); +} diff --git a/apps/webapp/app/utils/avatarLimits.ts b/apps/webapp/app/utils/avatarLimits.ts new file mode 100644 index 00000000000..9acf1d6f63f --- /dev/null +++ b/apps/webapp/app/utils/avatarLimits.ts @@ -0,0 +1,13 @@ +export const MAX_AVATAR_SIZE_IN_BYTES = 5 * 1024 * 1024; + +export const AVATAR_EXTENSIONS = { + "image/png": "png", + "image/jpeg": "jpg", + "image/webp": "webp", +} as const; + +export type AvatarContentType = keyof typeof AVATAR_EXTENSIONS; + +export function isAvatarContentType(contentType: string): contentType is AvatarContentType { + return contentType in AVATAR_EXTENSIONS; +} diff --git a/apps/webapp/app/v3/objectStore.server.ts b/apps/webapp/app/v3/objectStore.server.ts index 159f2f41c6f..bb2efbe7ae4 100644 --- a/apps/webapp/app/v3/objectStore.server.ts +++ b/apps/webapp/app/v3/objectStore.server.ts @@ -210,7 +210,7 @@ const objectStoreClients = singleton( () => new Map() ); -function getObjectStoreClient(protocol?: string): ObjectStoreClient | undefined { +export function getObjectStoreClient(protocol?: string): ObjectStoreClient | undefined { const config = getObjectStoreConfig(protocol); if (!config) return undefined; diff --git a/apps/webapp/app/v3/objectStoreClient.server.ts b/apps/webapp/app/v3/objectStoreClient.server.ts index 790530de352..6b11b6af250 100644 --- a/apps/webapp/app/v3/objectStoreClient.server.ts +++ b/apps/webapp/app/v3/objectStoreClient.server.ts @@ -13,7 +13,11 @@ export function normalizeObjectStoreLogicalKeyPathname(logicalKey: string): stri } interface IObjectStoreClient { - putObject(key: string, body: ReadableStream | string, contentType: string): Promise; + putObject( + key: string, + body: ReadableStream | Uint8Array | string, + contentType: string + ): Promise; getObject(key: string): Promise; presign(key: string, method: "PUT" | "GET", expiresIn: number): Promise; } @@ -48,14 +52,15 @@ class Aws4FetchClient implements IObjectStoreClient { async putObject( key: string, - body: ReadableStream | string, + body: ReadableStream | Uint8Array | string, contentType: string ): Promise { const objectUrl = this.buildUrl(key); const response = await this.awsClient.fetch(objectUrl, { method: "PUT", headers: { "Content-Type": contentType }, - body, + // Byte bodies are valid BodyInit at runtime; the ambient fetch types don't say so. + body: body instanceof Uint8Array ? (body as unknown as BodyInit) : body, }); if (!response.ok) { throw new Error(`Failed to upload to object store: ${response.statusText}`); @@ -120,7 +125,7 @@ class AwsSdkClient implements IObjectStoreClient { async putObject( key: string, - body: ReadableStream | string, + body: ReadableStream | Uint8Array | string, contentType: string ): Promise { const s3Key = this.toS3ObjectKey(key); @@ -204,7 +209,11 @@ export class ObjectStoreClient implements IObjectStoreClient { ); } - putObject(key: string, body: ReadableStream | string, contentType: string): Promise { + putObject( + key: string, + body: ReadableStream | Uint8Array | string, + contentType: string + ): Promise { return this.impl.putObject(key, body, contentType); } diff --git a/apps/webapp/test/dashboardAgentBodyCap.test.ts b/apps/webapp/test/dashboardAgentBodyCap.test.ts index aec6fa8b40a..786db416ba4 100644 --- a/apps/webapp/test/dashboardAgentBodyCap.test.ts +++ b/apps/webapp/test/dashboardAgentBodyCap.test.ts @@ -1,12 +1,14 @@ import express from "express"; -import type { Server } from "node:http"; +import http, { type Server } from "node:http"; import type { AddressInfo } from "node:net"; import { Readable } from "node:stream"; import { afterEach, describe, expect, it } from "vitest"; import { + AVATAR_MAX_INGRESS_BYTES, DASHBOARD_AGENT_MAX_INGRESS_BYTES, dashboardAgentBodyCap, } from "~/services/dashboardAgentBodyCap.server"; +import { MAX_AVATAR_SIZE_IN_BYTES } from "~/utils/avatarLimits"; // The cap has to hold for a body with no `content-length`: that is the case a route-level // check can't cover, because by then the body is already in memory. @@ -60,6 +62,31 @@ function postChunked(url: string, totalBytes: number, chunkBytes = 16 * 1024) { }); } +/** + * `node:http` sends the path verbatim; `fetch` resolves dot segments client-side, so it cannot + * express this request at all. + */ +function postRawPath(url: string, path: string, bytes: number) { + return new Promise((resolve, reject) => { + let settled = false; + const request = http.request( + { port: Number(new URL(url).port), method: "POST", path }, + (response) => { + response.resume(); + settled = true; + resolve(response.statusCode ?? 0); + request.destroy(); + } + ); + + // A refusal tears the socket down mid-write; that is the pass, not an error. + request.on("error", (error) => { + if (!settled) reject(error); + }); + request.end(Buffer.alloc(bytes, "a")); + }); +} + afterEach(async () => { await new Promise((resolve) => (server ? server.close(resolve) : resolve(undefined))); server = undefined; @@ -169,6 +196,54 @@ describe("the dashboard agent's ingress cap", () => { expect(buffered()).toBe(size); }); + it("refuses an oversized avatar upload before it is buffered", async () => { + const { url, buffered } = await listen(); + const oversized = AVATAR_MAX_INGRESS_BYTES + 512 * 1024; + + const response = await postChunked(`${url}/resources/account/avatar`, oversized).catch( + () => undefined + ); + + if (response) expect(response.status).toBe(413); + expect(buffered()).toBeLessThan(oversized); + }); + + it("caps a dot segment, which the adapter normalizes away before the action runs", async () => { + const { url, buffered } = await listen(); + + // `/…/avatar/.` reaches the route as `/…/avatar/`, so the cap has to see it that way too. + const status = await postRawPath( + url, + "/resources/account/avatar/.", + AVATAR_MAX_INGRESS_BYTES + 1 + ); + + expect(status).toBe(413); + expect(buffered()).toBe(0); + }); + + it("passes an avatar body exactly at the image cap through untouched", async () => { + const { url, buffered } = await listen(); + + const response = await postChunked(`${url}/resources/account/avatar`, MAX_AVATAR_SIZE_IN_BYTES); + + expect(response.status).toBe(200); + expect(buffered()).toBe(MAX_AVATAR_SIZE_IN_BYTES); + }); + + it("leaves the presigned avatar GET path uncapped", async () => { + const { url, buffered } = await listen(); + const size = AVATAR_MAX_INGRESS_BYTES + 1024; + + const response = await fetch(`${url}/resources/account/avatar/user_1/abc.png`, { + method: "POST", + body: "x".repeat(size), + }); + + expect(response.status).toBe(200); + expect(buffered()).toBe(size); + }); + it("does not cap a task whose id is literally dashboard-agent", async () => { const { url, buffered } = await listen(); const size = DASHBOARD_AGENT_MAX_INGRESS_BYTES + 1024; diff --git a/apps/webapp/test/userAvatar.test.ts b/apps/webapp/test/userAvatar.test.ts new file mode 100644 index 00000000000..81b7934552e --- /dev/null +++ b/apps/webapp/test/userAvatar.test.ts @@ -0,0 +1,114 @@ +import { describe, expect, it } from "vitest"; +import { + buildUserAvatarFilename, + buildUserAvatarUrl, + isAvatarUploadRejection, + parseAvatarUpload, + resolveUserAvatarObjectPath, +} from "~/services/userAvatar.server"; +import { MAX_AVATAR_SIZE_IN_BYTES } from "~/utils/avatarLimits"; + +const USER_ID = "clzabc123"; + +function filenameFor(bytes: number[]) { + return buildUserAvatarFilename("image/png", new Uint8Array(bytes)); +} + +describe("resolveUserAvatarObjectPath", () => { + it("accepts a content-addressed filename", () => { + const filename = filenameFor([1, 2, 3]); + + expect(resolveUserAvatarObjectPath(USER_ID, filename)).toBe(`avatars/${USER_ID}/${filename}`); + }); + + it.each([ + ["traversal in the filename", USER_ID, ".."], + ["encoded traversal in the filename", USER_ID, "%2e%2e"], + ["traversal in the user id", "..", filenameFor([1])], + ["encoded traversal in the user id", "%2e%2e", filenameFor([1])], + ["a disallowed extension", USER_ID, `${"a".repeat(32)}.svg`], + ["a nested filename", USER_ID, "a/b"], + ["a nested user id", `${USER_ID}/other`, filenameFor([1])], + ["a non-hex filename", USER_ID, "not-a-hash.png"], + ["an empty filename", USER_ID, ""], + ])("rejects %s", (_case, userId, filename) => { + expect(resolveUserAvatarObjectPath(userId, filename)).toBeUndefined(); + }); +}); + +describe("buildUserAvatarFilename", () => { + it("is content-addressed, so the URL changes when the image does", () => { + const first = filenameFor([1, 2, 3]); + const second = filenameFor([4, 5, 6]); + + expect(first).not.toBe(second); + expect(filenameFor([1, 2, 3])).toBe(first); + expect(first).toMatch(/^[0-9a-f]{32}\.png$/); + }); + + it("uses the extension of the content type", () => { + expect(buildUserAvatarFilename("image/jpeg", new Uint8Array([1]))).toMatch(/\.jpg$/); + expect(buildUserAvatarFilename("image/webp", new Uint8Array([1]))).toMatch(/\.webp$/); + }); +}); + +/** The route's whole body handling, minus the session lookup it wraps. */ +function pngForm(bytes: number[], extra?: Record) { + const form = new FormData(); + for (const [key, value] of Object.entries(extra ?? {})) form.set(key, value); + form.set("image", new File([new Uint8Array(bytes)], "avatar.png", { type: "image/png" })); + return form; +} + +describe("parseAvatarUpload", () => { + it("takes nothing from the body that could name the key's user", async () => { + const upload = await parseAvatarUpload( + pngForm([1, 2, 3], { userId: "usr_attacker", avatarUrl: "/resources/account/avatar/x/y" }) + ); + + if (isAvatarUploadRejection(upload)) throw new Error("expected the upload to be accepted"); + + // Only the caller's authenticated id reaches the key and the URL. + const filename = buildUserAvatarFilename(upload.contentType, upload.data); + const url = buildUserAvatarUrl(USER_ID, filename); + + expect(url).toBe(`/resources/account/avatar/${USER_ID}/${filename}`); + expect(url).not.toContain("usr_attacker"); + expect(resolveUserAvatarObjectPath(USER_ID, filename)).toBe(`avatars/${USER_ID}/${filename}`); + expect(Object.keys(upload)).toEqual(["contentType", "data"]); + }); + + it("rejects a missing image with 400", async () => { + expect(await parseAvatarUpload(new FormData())).toEqual({ + error: "Missing image", + status: 400, + }); + }); + + it("rejects a disallowed content type with 415", async () => { + const form = new FormData(); + form.set("image", new File([""], "a.svg", { type: "image/svg+xml" })); + + expect(await parseAvatarUpload(form)).toMatchObject({ status: 415 }); + }); + + it("rejects an image over the cap with 413", async () => { + const form = new FormData(); + form.set( + "image", + new File([new Uint8Array(MAX_AVATAR_SIZE_IN_BYTES + 1)], "a.png", { type: "image/png" }) + ); + + expect(await parseAvatarUpload(form)).toMatchObject({ status: 413 }); + }); + + it("accepts an image exactly at the cap", async () => { + const form = new FormData(); + form.set( + "image", + new File([new Uint8Array(MAX_AVATAR_SIZE_IN_BYTES)], "a.png", { type: "image/png" }) + ); + + expect(isAvatarUploadRejection(await parseAvatarUpload(form))).toBe(false); + }); +}); From 8068b39e1de84a6320c00ff04fe44a02bdf8b828 Mon Sep 17 00:00:00 2001 From: Katia Bulatova Date: Thu, 27 Aug 2026 07:22:30 +0000 Subject: [PATCH 03/25] feat(webapp): change your profile picture from the account page --- .../app/routes/account._index/route.tsx | 59 ++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/apps/webapp/app/routes/account._index/route.tsx b/apps/webapp/app/routes/account._index/route.tsx index 9187d361dba..825e26ed5df 100644 --- a/apps/webapp/app/routes/account._index/route.tsx +++ b/apps/webapp/app/routes/account._index/route.tsx @@ -8,6 +8,7 @@ import { } from "@remix-run/server-runtime"; import { z } from "zod"; import { EditPencilIcon } from "~/assets/icons/EditPencilIcon"; +import { ProfilePhotoEditor } from "~/components/ProfilePhotoEditor"; import { UserProfilePhoto } from "~/components/UserProfilePhoto"; import { MainHorizontallyCenteredContainer, @@ -447,6 +448,62 @@ function useProfileFieldUpdate({ return { fetcher, error, setError, isSubmitting: fetcher.state !== "idle" }; } +function ChangeProfilePhotoButton() { + const [isOpen, setIsOpen] = useState(false); + const fetcher = useFetcher<{ avatarUrl?: string; error?: string }>(); + const toast = useToast(); + const isSaving = fetcher.state !== "idle"; + const submitSeenRef = useRef(false); + + useEffect(() => { + if (fetcher.state !== "idle") { + submitSeenRef.current = true; + return; + } + if (!submitSeenRef.current) return; + submitSeenRef.current = false; + + if (fetcher.data?.avatarUrl) { + // oxlint-disable-next-line react/set-state-in-effect -- Closes the modal once the upload has landed. + setIsOpen(false); + toast.success("Your profile picture has been updated."); + return; + } + + toast.error(fetcher.data?.error ?? "Something went wrong. Please try again."); + }, [fetcher.state, fetcher.data, toast]); + + const save = (blob: Blob) => { + const formData = new FormData(); + formData.append("image", blob, "avatar.png"); + fetcher.submit(formData, { + method: "post", + action: "/resources/account/avatar", + encType: "multipart/form-data", + }); + }; + + return ( + <> + + + + ); +} + function EditNameButton() { const user = useUser(); const [isOpen, setIsOpen] = useState(false); @@ -900,7 +957,7 @@ export default function Page() {
- +
From 92450920d2caa1e7ff08a980614e3ce20e34cd21 Mon Sep 17 00:00:00 2001 From: Katia Bulatova Date: Thu, 27 Aug 2026 07:36:45 +0000 Subject: [PATCH 04/25] feat(webapp): delete the previous profile photo after a new one is stored --- .../app/routes/resources.account.avatar.ts | 7 ++- apps/webapp/app/services/userAvatar.server.ts | 56 ++++++++++++++++++- .../webapp/app/v3/objectStoreClient.server.ts | 25 ++++++++- apps/webapp/test/userAvatar.test.ts | 46 +++++++++++++++ 4 files changed, 131 insertions(+), 3 deletions(-) diff --git a/apps/webapp/app/routes/resources.account.avatar.ts b/apps/webapp/app/routes/resources.account.avatar.ts index 22db23993e6..101bbe2cbd2 100644 --- a/apps/webapp/app/routes/resources.account.avatar.ts +++ b/apps/webapp/app/routes/resources.account.avatar.ts @@ -2,6 +2,7 @@ import { json, type ActionFunctionArgs } from "@remix-run/node"; import { updateUserAvatarUrl } from "~/models/user.server"; import { requireUser } from "~/services/session.server"; import { + deleteStaleUserAvatar, isAvatarUploadRejection, parseAvatarUpload, uploadUserAvatar, @@ -20,9 +21,13 @@ export async function action({ request }: ActionFunctionArgs) { return json({ error: upload.error }, { status: upload.status }); } - const { avatarUrl } = await uploadUserAvatar({ userId: user.id, ...upload }); + const previousAvatarUrl = user.avatarUrl; + + const { avatarUrl, filename } = await uploadUserAvatar({ userId: user.id, ...upload }); await updateUserAvatarUrl({ id: user.id, avatarUrl }); + await deleteStaleUserAvatar({ previousAvatarUrl, userId: user.id, filename }); + return json({ avatarUrl }); } diff --git a/apps/webapp/app/services/userAvatar.server.ts b/apps/webapp/app/services/userAvatar.server.ts index c4d1f01c3da..4bcda626205 100644 --- a/apps/webapp/app/services/userAvatar.server.ts +++ b/apps/webapp/app/services/userAvatar.server.ts @@ -1,4 +1,5 @@ import { createHash } from "node:crypto"; +import { logger } from "~/services/logger.server"; import { AVATAR_EXTENSIONS, type AvatarContentType, @@ -98,7 +99,60 @@ export async function uploadUserAvatar({ const { client, objectKey } = requireAvatarObjectStore(); await client.putObject(objectKey(path), data, contentType); - return { avatarUrl: buildUserAvatarUrl(userId, filename) }; + return { filename, avatarUrl: buildUserAvatarUrl(userId, filename) }; +} + +const AVATAR_URL_REGEX = /^\/resources\/account\/avatar\/([^/]+)\/([^/]+)$/; + +/** + * Undefined unless the stored URL is this user's own avatar route and names a different object: + * an OAuth avatar elsewhere is not ours to delete, and the same content hash is the same file. + */ +export function resolveStaleAvatarObjectPath({ + previousAvatarUrl, + userId, + filename, +}: { + previousAvatarUrl: string | null; + userId: string; + filename: string; +}): string | undefined { + const match = previousAvatarUrl?.match(AVATAR_URL_REGEX); + + if (!match) { + return undefined; + } + + const [, previousUserId, previousFilename] = match; + + if (previousUserId !== userId || previousFilename === filename) { + return undefined; + } + + return resolveUserAvatarObjectPath(previousUserId, previousFilename); +} + +export async function deleteStaleUserAvatar(options: { + previousAvatarUrl: string | null; + userId: string; + filename: string; +}) { + const path = resolveStaleAvatarObjectPath(options); + + if (!path) { + return; + } + + try { + const { client, objectKey } = requireAvatarObjectStore(); + await client.deleteObject(objectKey(path)); + } catch (error) { + logger.warn("Failed to delete the previous avatar", { + userId: options.userId, + path, + error: error instanceof Error ? error.message : String(error), + }); + } } export function presignUserAvatarUrl(objectPath: string) { diff --git a/apps/webapp/app/v3/objectStoreClient.server.ts b/apps/webapp/app/v3/objectStoreClient.server.ts index 6b11b6af250..7e671566f96 100644 --- a/apps/webapp/app/v3/objectStoreClient.server.ts +++ b/apps/webapp/app/v3/objectStoreClient.server.ts @@ -1,5 +1,10 @@ import { AwsClient } from "aws4fetch"; -import { GetObjectCommand, PutObjectCommand, S3Client } from "@aws-sdk/client-s3"; +import { + DeleteObjectCommand, + GetObjectCommand, + PutObjectCommand, + S3Client, +} from "@aws-sdk/client-s3"; import { getSignedUrl } from "@aws-sdk/s3-request-presigner"; /** @@ -19,6 +24,7 @@ interface IObjectStoreClient { contentType: string ): Promise; getObject(key: string): Promise; + deleteObject(key: string): Promise; presign(key: string, method: "PUT" | "GET", expiresIn: number): Promise; } @@ -76,6 +82,13 @@ class Aws4FetchClient implements IObjectStoreClient { return response.text(); } + async deleteObject(key: string): Promise { + const response = await this.awsClient.fetch(this.buildUrl(key), { method: "DELETE" }); + if (!response.ok) { + throw new Error(`Failed to delete from object store: ${response.statusText}`); + } + } + async presign(key: string, method: "PUT" | "GET", expiresIn: number): Promise { const url = new URL(this.config.baseUrl); url.pathname = normalizeObjectStoreLogicalKeyPathname(key); @@ -151,6 +164,12 @@ class AwsSdkClient implements IObjectStoreClient { return response.Body.transformToString(); } + async deleteObject(key: string): Promise { + await this.s3Client.send( + new DeleteObjectCommand({ Bucket: this.config.bucket, Key: this.toS3ObjectKey(key) }) + ); + } + async presign(key: string, method: "PUT" | "GET", expiresIn: number): Promise { const s3Key = this.toS3ObjectKey(key); const command = @@ -221,6 +240,10 @@ export class ObjectStoreClient implements IObjectStoreClient { return this.impl.getObject(key); } + deleteObject(key: string): Promise { + return this.impl.deleteObject(key); + } + presign(key: string, method: "PUT" | "GET", expiresIn: number): Promise { return this.impl.presign(key, method, expiresIn); } diff --git a/apps/webapp/test/userAvatar.test.ts b/apps/webapp/test/userAvatar.test.ts index 81b7934552e..a0b63d2da39 100644 --- a/apps/webapp/test/userAvatar.test.ts +++ b/apps/webapp/test/userAvatar.test.ts @@ -4,6 +4,7 @@ import { buildUserAvatarUrl, isAvatarUploadRejection, parseAvatarUpload, + resolveStaleAvatarObjectPath, resolveUserAvatarObjectPath, } from "~/services/userAvatar.server"; import { MAX_AVATAR_SIZE_IN_BYTES } from "~/utils/avatarLimits"; @@ -112,3 +113,48 @@ describe("parseAvatarUpload", () => { expect(isAvatarUploadRejection(await parseAvatarUpload(form))).toBe(false); }); }); + +describe("resolveStaleAvatarObjectPath", () => { + const previous = filenameFor([1, 2, 3]); + const next = filenameFor([4, 5, 6]); + + it("derives the old object from the stored URL", () => { + expect( + resolveStaleAvatarObjectPath({ + previousAvatarUrl: buildUserAvatarUrl(USER_ID, previous), + userId: USER_ID, + filename: next, + }) + ).toBe(`avatars/${USER_ID}/${previous}`); + }); + + it("keeps the object when the content hash is unchanged", () => { + expect( + resolveStaleAvatarObjectPath({ + previousAvatarUrl: buildUserAvatarUrl(USER_ID, previous), + userId: USER_ID, + filename: previous, + }) + ).toBeUndefined(); + }); + + it.each([ + ["no previous avatar", null], + ["an OAuth avatar hosted elsewhere", "https://avatars.githubusercontent.com/u/1?v=4"], + [ + "an absolute URL onto our own path", + `https://evil.test/resources/account/avatar/${USER_ID}/${previous}`, + ], + ["another user's avatar", `/resources/account/avatar/usr_other/${previous}`], + [ + "a filename that is not content-addressed", + `/resources/account/avatar/${USER_ID}/../../secret.png`, + ], + ["a deeper path", `/resources/account/avatar/${USER_ID}/${previous}/extra`], + ["an unrelated app path", "/resources/account/photo"], + ])("leaves %s alone", (_case, previousAvatarUrl) => { + expect( + resolveStaleAvatarObjectPath({ previousAvatarUrl, userId: USER_ID, filename: next }) + ).toBeUndefined(); + }); +}); From 41dbfadd3715a4bea15ce355bff5286ae85a8c8f Mon Sep 17 00:00:00 2001 From: Katia Bulatova Date: Thu, 27 Aug 2026 07:55:13 +0000 Subject: [PATCH 05/25] feat(webapp): verify profile photo bytes and return fetchable avatar URLs --- .server-changes/profile-picture-upload.md | 6 + .../routes/api.v1.orgs.$orgParam.members.ts | 3 +- .../services/dashboardAgentBodyCap.server.ts | 9 +- apps/webapp/app/services/userAvatar.server.ts | 18 +- apps/webapp/app/utils/avatarLimits.ts | 18 ++ apps/webapp/server.ts | 2 +- .../webapp/test/dashboardAgentBodyCap.test.ts | 16 ++ apps/webapp/test/userAvatar.test.ts | 157 +++++++++++++++--- 8 files changed, 197 insertions(+), 32 deletions(-) create mode 100644 .server-changes/profile-picture-upload.md diff --git a/.server-changes/profile-picture-upload.md b/.server-changes/profile-picture-upload.md new file mode 100644 index 00000000000..97b1e065d3e --- /dev/null +++ b/.server-changes/profile-picture-upload.md @@ -0,0 +1,6 @@ +--- +area: webapp +type: feature +--- + +You can now upload and crop your own profile picture from your account page. diff --git a/apps/webapp/app/routes/api.v1.orgs.$orgParam.members.ts b/apps/webapp/app/routes/api.v1.orgs.$orgParam.members.ts index 2f95bc6b900..9d912781166 100644 --- a/apps/webapp/app/routes/api.v1.orgs.$orgParam.members.ts +++ b/apps/webapp/app/routes/api.v1.orgs.$orgParam.members.ts @@ -4,6 +4,7 @@ import { prisma } from "~/db.server"; import { getTeamMembersAndInvites } from "~/models/member.server"; import { resolveOrganizationForApiUser } from "~/services/organizationApiAccess.server"; import { createLoaderPATApiRoute } from "~/services/routeBuilders/apiBuilder.server"; +import { absoluteUserAvatarUrl } from "~/services/userAvatar.server"; const ParamsSchema = z.object({ orgParam: z.string(), @@ -51,7 +52,7 @@ export const loader = createLoaderPATApiRoute( id: member.user.id, name: member.user.name, email: member.user.email, - avatarUrl: member.user.avatarUrl, + avatarUrl: absoluteUserAvatarUrl(member.user.avatarUrl), }, })), invites: result.invites.map((invite) => ({ diff --git a/apps/webapp/app/services/dashboardAgentBodyCap.server.ts b/apps/webapp/app/services/dashboardAgentBodyCap.server.ts index 125f513e7f0..5053d6565ff 100644 --- a/apps/webapp/app/services/dashboardAgentBodyCap.server.ts +++ b/apps/webapp/app/services/dashboardAgentBodyCap.server.ts @@ -24,8 +24,8 @@ export const DASHBOARD_AGENT_MAX_INGRESS_BYTES = MAX_MESSAGE_BODY_BYTES + INGRES const AGENT_PATH = /^(?:\/api\/v1\/dashboard-agent|\/resources\/orgs\/[^/]+\/projects\/[^/]+\/env\/[^/]+\/dashboard-agent)(\/|$)/; -/** Headroom over the avatar cap for multipart framing. */ -const AVATAR_INGRESS_SLACK_BYTES = 8 * 1024; +/** Headroom over the avatar cap: multipart framing, part headers and the crop metadata. */ +const AVATAR_INGRESS_SLACK_BYTES = 64 * 1024; export const AVATAR_MAX_INGRESS_BYTES = MAX_AVATAR_SIZE_IN_BYTES + AVATAR_INGRESS_SLACK_BYTES; @@ -52,11 +52,12 @@ const AVATAR_CAP: Cap = { /** * Matched against the path Remix will route, not the raw one: the express adapter rebuilds the - * request through `new URL`, so `/…/avatar/.` reaches the action as `/…/avatar/`. + * request as `new URL(origin + originalUrl)`, so `/…/avatar/.` reaches the action as `/…/avatar/`. + * Built the same way here, or a protocol-relative path would normalize to a different one. */ function pathToMatch(req: Request): string { try { - return new URL(req.originalUrl || req.url, "http://localhost").pathname.toLowerCase(); + return new URL(`http://localhost${req.originalUrl || req.url}`).pathname.toLowerCase(); } catch { return req.path.toLowerCase(); } diff --git a/apps/webapp/app/services/userAvatar.server.ts b/apps/webapp/app/services/userAvatar.server.ts index 4bcda626205..a7ff14f33bf 100644 --- a/apps/webapp/app/services/userAvatar.server.ts +++ b/apps/webapp/app/services/userAvatar.server.ts @@ -1,8 +1,10 @@ import { createHash } from "node:crypto"; +import { env } from "~/env.server"; import { logger } from "~/services/logger.server"; import { AVATAR_EXTENSIONS, type AvatarContentType, + hasAvatarMagicBytes, isAvatarContentType, MAX_AVATAR_SIZE_IN_BYTES, } from "~/utils/avatarLimits"; @@ -71,7 +73,21 @@ export async function parseAvatarUpload( return { error: "Image is too large", status: 413 }; } - return { contentType: image.type, data: new Uint8Array(await image.arrayBuffer()) }; + const data = new Uint8Array(await image.arrayBuffer()); + + if (!hasAvatarMagicBytes(image.type, data)) { + return { error: "Unsupported image type", status: 415 }; + } + + return { contentType: image.type, data }; +} + +export function absoluteUserAvatarUrl(avatarUrl: string | null) { + if (!avatarUrl || !avatarUrl.startsWith("/")) { + return avatarUrl; + } + + return `${env.APP_ORIGIN}${avatarUrl}`; } export function isAvatarUploadRejection( diff --git a/apps/webapp/app/utils/avatarLimits.ts b/apps/webapp/app/utils/avatarLimits.ts index 9acf1d6f63f..4ba3e59764a 100644 --- a/apps/webapp/app/utils/avatarLimits.ts +++ b/apps/webapp/app/utils/avatarLimits.ts @@ -11,3 +11,21 @@ export type AvatarContentType = keyof typeof AVATAR_EXTENSIONS; export function isAvatarContentType(contentType: string): contentType is AvatarContentType { return contentType in AVATAR_EXTENSIONS; } + +function startsWith(data: Uint8Array, signature: number[], offset = 0) { + return signature.every((byte, index) => data[offset + index] === byte); +} + +/** A declared content type is a claim; the bytes have to back it. */ +export function hasAvatarMagicBytes(contentType: AvatarContentType, data: Uint8Array) { + switch (contentType) { + case "image/png": + return startsWith(data, [0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); + case "image/jpeg": + return startsWith(data, [0xff, 0xd8, 0xff]); + case "image/webp": + return ( + startsWith(data, [0x52, 0x49, 0x46, 0x46]) && startsWith(data, [0x57, 0x45, 0x42, 0x50], 8) + ); + } +} diff --git a/apps/webapp/server.ts b/apps/webapp/server.ts index 51afe0c90ca..822dbef27a5 100644 --- a/apps/webapp/server.ts +++ b/apps/webapp/server.ts @@ -257,7 +257,7 @@ async function startServer() { app.use(tenantContextMiddleware); - // Before the Remix handler: the agent's chat body is refused while it streams, so a + // Before the Remix handler: a capped path's body is refused while it streams, so a // route never buffers one that was already too large. app.use(dashboardAgentBodyCap); diff --git a/apps/webapp/test/dashboardAgentBodyCap.test.ts b/apps/webapp/test/dashboardAgentBodyCap.test.ts index 786db416ba4..fbef3b7761c 100644 --- a/apps/webapp/test/dashboardAgentBodyCap.test.ts +++ b/apps/webapp/test/dashboardAgentBodyCap.test.ts @@ -231,6 +231,22 @@ describe("the dashboard agent's ingress cap", () => { expect(buffered()).toBe(MAX_AVATAR_SIZE_IN_BYTES); }); + it("passes a real multipart upload whose image is exactly at the cap", async () => { + const { url, buffered } = await listen(); + + const form = new FormData(); + form.set( + "image", + new File([new Uint8Array(MAX_AVATAR_SIZE_IN_BYTES)], "avatar.png", { type: "image/png" }) + ); + + const response = await fetch(`${url}/resources/account/avatar`, { method: "POST", body: form }); + + expect(response.status).toBe(200); + expect(buffered()).toBeGreaterThan(MAX_AVATAR_SIZE_IN_BYTES); + expect(buffered()).toBeLessThanOrEqual(AVATAR_MAX_INGRESS_BYTES); + }); + it("leaves the presigned avatar GET path uncapped", async () => { const { url, buffered } = await listen(); const size = AVATAR_MAX_INGRESS_BYTES + 1024; diff --git a/apps/webapp/test/userAvatar.test.ts b/apps/webapp/test/userAvatar.test.ts index a0b63d2da39..a5d46e7d09e 100644 --- a/apps/webapp/test/userAvatar.test.ts +++ b/apps/webapp/test/userAvatar.test.ts @@ -1,9 +1,12 @@ -import { describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it } from "vitest"; +import { env } from "~/env.server"; import { buildUserAvatarFilename, buildUserAvatarUrl, isAvatarUploadRejection, + absoluteUserAvatarUrl, parseAvatarUpload, + presignUserAvatarUrl, resolveStaleAvatarObjectPath, resolveUserAvatarObjectPath, } from "~/services/userAvatar.server"; @@ -11,10 +14,25 @@ import { MAX_AVATAR_SIZE_IN_BYTES } from "~/utils/avatarLimits"; const USER_ID = "clzabc123"; +const PNG_MAGIC = [0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]; +const JPEG_MAGIC = [0xff, 0xd8, 0xff]; + function filenameFor(bytes: number[]) { return buildUserAvatarFilename("image/png", new Uint8Array(bytes)); } +function imageFile(type: string, bytes: number[], padTo = 0) { + const data = new Uint8Array(Math.max(padTo, bytes.length)); + data.set(bytes); + return new File([data], "avatar.bin", { type }); +} + +function formWith(file: File) { + const form = new FormData(); + form.set("image", file); + return form; +} + describe("resolveUserAvatarObjectPath", () => { it("accepts a content-addressed filename", () => { const filename = filenameFor([1, 2, 3]); @@ -53,23 +71,16 @@ describe("buildUserAvatarFilename", () => { }); }); -/** The route's whole body handling, minus the session lookup it wraps. */ -function pngForm(bytes: number[], extra?: Record) { - const form = new FormData(); - for (const [key, value] of Object.entries(extra ?? {})) form.set(key, value); - form.set("image", new File([new Uint8Array(bytes)], "avatar.png", { type: "image/png" })); - return form; -} - describe("parseAvatarUpload", () => { it("takes nothing from the body that could name the key's user", async () => { - const upload = await parseAvatarUpload( - pngForm([1, 2, 3], { userId: "usr_attacker", avatarUrl: "/resources/account/avatar/x/y" }) - ); + const form = formWith(imageFile("image/png", PNG_MAGIC)); + form.set("userId", "usr_attacker"); + form.set("avatarUrl", "/resources/account/avatar/x/y"); + + const upload = await parseAvatarUpload(form); if (isAvatarUploadRejection(upload)) throw new Error("expected the upload to be accepted"); - // Only the caller's authenticated id reaches the key and the URL. const filename = buildUserAvatarFilename(upload.contentType, upload.data); const url = buildUserAvatarUrl(USER_ID, filename); @@ -87,31 +98,60 @@ describe("parseAvatarUpload", () => { }); it("rejects a disallowed content type with 415", async () => { - const form = new FormData(); - form.set("image", new File([""], "a.svg", { type: "image/svg+xml" })); + const form = formWith(new File([""], "a.svg", { type: "image/svg+xml" })); expect(await parseAvatarUpload(form)).toMatchObject({ status: 415 }); }); it("rejects an image over the cap with 413", async () => { - const form = new FormData(); - form.set( - "image", - new File([new Uint8Array(MAX_AVATAR_SIZE_IN_BYTES + 1)], "a.png", { type: "image/png" }) - ); + const form = formWith(imageFile("image/png", PNG_MAGIC, MAX_AVATAR_SIZE_IN_BYTES + 1)); expect(await parseAvatarUpload(form)).toMatchObject({ status: 413 }); }); it("accepts an image exactly at the cap", async () => { - const form = new FormData(); - form.set( - "image", - new File([new Uint8Array(MAX_AVATAR_SIZE_IN_BYTES)], "a.png", { type: "image/png" }) - ); + const form = formWith(imageFile("image/png", PNG_MAGIC, MAX_AVATAR_SIZE_IN_BYTES)); + + expect(isAvatarUploadRejection(await parseAvatarUpload(form))).toBe(false); + }); + + it.each([ + ["png", "image/png", PNG_MAGIC], + ["jpeg", "image/jpeg", JPEG_MAGIC], + ["webp", "image/webp", [0x52, 0x49, 0x46, 0x46, 0, 0, 0, 0, 0x57, 0x45, 0x42, 0x50]], + ])("accepts %s bytes matching their declared type", async (_case, type, magic) => { + const form = formWith(imageFile(type, magic)); expect(isAvatarUploadRejection(await parseAvatarUpload(form))).toBe(false); }); + + it.each([ + ["png bytes declared as jpeg", "image/jpeg", PNG_MAGIC], + ["jpeg bytes declared as png", "image/png", JPEG_MAGIC], + ["garbage declared as png", "image/png", [1, 2, 3, 4, 5, 6, 7, 8]], + ["an html payload declared as webp", "image/webp", [0x3c, 0x21, 0x64, 0x6f, 0x63, 0x74]], + ["a truncated png header", "image/png", PNG_MAGIC.slice(0, 4)], + ["an empty file", "image/png", []], + ])("rejects %s with 415", async (_case, type, bytes) => { + const form = formWith(imageFile(type, bytes)); + + expect(await parseAvatarUpload(form)).toMatchObject({ status: 415 }); + }); +}); + +describe("absoluteUserAvatarUrl", () => { + it("absolutises an uploaded avatar so an API client can fetch it", () => { + expect(absoluteUserAvatarUrl(`/resources/account/avatar/${USER_ID}/a.png`)).toBe( + `${env.APP_ORIGIN}/resources/account/avatar/${USER_ID}/a.png` + ); + }); + + it.each([ + ["an OAuth avatar", "https://avatars.githubusercontent.com/u/1?v=4"], + ["no avatar", null], + ])("leaves %s untouched", (_case, avatarUrl) => { + expect(absoluteUserAvatarUrl(avatarUrl)).toBe(avatarUrl); + }); }); describe("resolveStaleAvatarObjectPath", () => { @@ -158,3 +198,70 @@ describe("resolveStaleAvatarObjectPath", () => { ).toBeUndefined(); }); }); + +const S3_ENV_KEYS = [ + "OBJECT_STORE_S3_BASE_URL", + "OBJECT_STORE_S3_BUCKET", + "OBJECT_STORE_S3_ACCESS_KEY_ID", + "OBJECT_STORE_S3_SECRET_ACCESS_KEY", + "OBJECT_STORE_S3_REGION", +] as const; + +describe("the avatar object store", () => { + const originalS3Env = Object.fromEntries(S3_ENV_KEYS.map((key) => [key, process.env[key]])); + const originalDefaultBaseUrl = env.OBJECT_STORE_BASE_URL; + const originalDefaultBucket = env.OBJECT_STORE_BUCKET; + + function setS3Env(values: Partial>) { + for (const key of S3_ENV_KEYS) delete process.env[key]; + for (const [key, value] of Object.entries(values)) process.env[key] = value; + } + + afterEach(() => { + for (const key of S3_ENV_KEYS) { + const original = originalS3Env[key]; + if (original === undefined) delete process.env[key]; + else process.env[key] = original; + } + env.OBJECT_STORE_BASE_URL = originalDefaultBaseUrl; + env.OBJECT_STORE_BUCKET = originalDefaultBucket; + }); + + it("reads OBJECT_STORE_S3_*, never the default protocol", () => { + setS3Env({}); + env.OBJECT_STORE_BASE_URL = "https://default-store.test"; + env.OBJECT_STORE_BUCKET = "packets"; + process.env.OBJECT_STORE_BASE_URL = "https://default-store.test"; + process.env.OBJECT_STORE_BUCKET = "packets"; + + expect(() => presignUserAvatarUrl(`avatars/${USER_ID}/a.png`)).toThrow(/protocol: s3/); + }); + + it("requires its own bucket", () => { + setS3Env({ + OBJECT_STORE_S3_BASE_URL: "https://s3-no-bucket.test", + OBJECT_STORE_S3_ACCESS_KEY_ID: "key", + OBJECT_STORE_S3_SECRET_ACCESS_KEY: "secret", + }); + + expect(() => presignUserAvatarUrl(`avatars/${USER_ID}/a.png`)).toThrow( + /OBJECT_STORE_S3_BUCKET/ + ); + }); + + it("signs a short-lived URL under the S3 bucket", async () => { + setS3Env({ + OBJECT_STORE_S3_BASE_URL: "https://s3-signing.test", + OBJECT_STORE_S3_BUCKET: "avatars-bucket", + OBJECT_STORE_S3_ACCESS_KEY_ID: "key", + OBJECT_STORE_S3_SECRET_ACCESS_KEY: "secret", + OBJECT_STORE_S3_REGION: "us-east-1", + }); + + const url = await presignUserAvatarUrl(`avatars/${USER_ID}/a.png`); + + expect(url).toContain(`/avatars-bucket/avatars/${USER_ID}/a.png`); + expect(url).toContain("X-Amz-Expires=300"); + expect(url).toContain("X-Amz-Signature="); + }); +}); From 569c46c171d10f5125e460360cb41e235f306f42 Mon Sep 17 00:00:00 2001 From: Katia Bulatova Date: Thu, 27 Aug 2026 11:27:16 +0000 Subject: [PATCH 06/25] fix(webapp): size the avatar image to its container --- apps/webapp/app/components/UserProfilePhoto.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/webapp/app/components/UserProfilePhoto.tsx b/apps/webapp/app/components/UserProfilePhoto.tsx index 92c435aa109..4c33658f428 100644 --- a/apps/webapp/app/components/UserProfilePhoto.tsx +++ b/apps/webapp/app/components/UserProfilePhoto.tsx @@ -48,7 +48,7 @@ export function UserAvatar({ return (
{name Date: Thu, 27 Aug 2026 11:33:24 +0000 Subject: [PATCH 07/25] fix(webapp): allow the avatar object store origin in the image policy --- apps/webapp/app/entry.server.tsx | 9 +++-- apps/webapp/app/env.server.ts | 4 ++ apps/webapp/app/services/userAvatar.server.ts | 6 +++ apps/webapp/app/utils/cspImageOrigins.test.ts | 37 +++++++++++++++++++ apps/webapp/app/utils/cspImageOrigins.ts | 22 +++++++++++ apps/webapp/test/userAvatar.test.ts | 27 ++++++++++++++ 6 files changed, 102 insertions(+), 3 deletions(-) diff --git a/apps/webapp/app/entry.server.tsx b/apps/webapp/app/entry.server.tsx index 074bc39d760..2c662fde678 100644 --- a/apps/webapp/app/entry.server.tsx +++ b/apps/webapp/app/entry.server.tsx @@ -20,6 +20,7 @@ import { assertRunOpsSplitSentinel, Prisma } from "./db.server"; import { env } from "./env.server"; import { eventLoopMonitor, eventLoopUtilizationMonitor } from "./eventLoopMonitor.server"; import { logger } from "./services/logger.server"; +import { avatarObjectStoreImageOrigin } from "./services/userAvatar.server"; import { buildImgSrcDirective, parseCspImageOrigins, withImgSrc } from "./utils/cspImageOrigins"; import { singleton } from "./utils/singleton"; import { remoteBuildsEnabled } from "./v3/remoteImageBuilder.server"; @@ -66,8 +67,8 @@ const ABORT_DELAY = 30000; * ships in the stacked UI PR, so on this branch the policy is the only thing stopping * a model- or customer-authored image from reaching a remote host. * - * The hosts we store avatar URLs for, plus whatever `CSP_IMG_SRC_ALLOWLIST` adds - * (e.g. a self-hosted SSO avatar host). + * The hosts we store avatar URLs for, the object store uploaded avatars are presigned + * from, plus whatever `CSP_IMG_SRC_ALLOWLIST` adds (e.g. a self-hosted SSO avatar host). */ const IMG_SRC_DIRECTIVE = buildImgSrcDirective( singleton("CspImageOrigins", () => { @@ -81,7 +82,9 @@ const IMG_SRC_DIRECTIVE = buildImgSrcDirective( ); } - return origins; + const avatarOrigin = avatarObjectStoreImageOrigin(); + + return avatarOrigin && !origins.includes(avatarOrigin) ? [...origins, avatarOrigin] : origins; }) ); diff --git a/apps/webapp/app/env.server.ts b/apps/webapp/app/env.server.ts index 2b1fba86980..e89f03ecc99 100644 --- a/apps/webapp/app/env.server.ts +++ b/apps/webapp/app/env.server.ts @@ -860,6 +860,10 @@ const EnvironmentSchema = z .regex(/^[a-z0-9]+$/) .optional(), + // Declared because avatars are stored under the "s3" protocol and its origin has to + // reach the image policy. The rest of the OBJECT_STORE_S3_* set is read by protocol name. + OBJECT_STORE_S3_BASE_URL: z.string().optional(), + ARTIFACTS_OBJECT_STORE_BUCKET: z.string().optional(), ARTIFACTS_OBJECT_STORE_BASE_URL: z.string().optional(), ARTIFACTS_OBJECT_STORE_ACCESS_KEY_ID: z.string().optional(), diff --git a/apps/webapp/app/services/userAvatar.server.ts b/apps/webapp/app/services/userAvatar.server.ts index a7ff14f33bf..472b28112f5 100644 --- a/apps/webapp/app/services/userAvatar.server.ts +++ b/apps/webapp/app/services/userAvatar.server.ts @@ -8,6 +8,7 @@ import { isAvatarContentType, MAX_AVATAR_SIZE_IN_BYTES, } from "~/utils/avatarLimits"; +import { imageOriginFromUrl } from "~/utils/cspImageOrigins"; import { getObjectStoreClient } from "~/v3/objectStore.server"; /** Avatars always live in plain S3, never the default/R2 protocol. */ @@ -17,6 +18,11 @@ const AVATAR_PRESIGN_EXPIRY_IN_SECONDS = 300; const AVATAR_FILENAME_REGEX = /^[0-9a-f]{32}\.(png|jpg|webp)$/; const USER_ID_REGEX = /^[A-Za-z0-9_-]+$/; +/** Undefined when no avatar store is configured, so the policy stays unchanged. */ +export function avatarObjectStoreImageOrigin() { + return imageOriginFromUrl(env.OBJECT_STORE_S3_BASE_URL); +} + /** The first segment of a logical key is the bucket, as with `packets/…`. */ function requireAvatarObjectStore() { const client = getObjectStoreClient(AVATAR_STORE_PROTOCOL); diff --git a/apps/webapp/app/utils/cspImageOrigins.test.ts b/apps/webapp/app/utils/cspImageOrigins.test.ts index c08c6cde4b0..f6d03fa3c79 100644 --- a/apps/webapp/app/utils/cspImageOrigins.test.ts +++ b/apps/webapp/app/utils/cspImageOrigins.test.ts @@ -3,6 +3,7 @@ import { faviconUrl } from "./favicon"; import { BASE_IMG_SRC_SOURCES, buildImgSrcDirective, + imageOriginFromUrl, parseCspImageOrigins, withImgSrc, } from "./cspImageOrigins"; @@ -189,3 +190,39 @@ describe("withImgSrc", () => { ); }); }); + +describe("imageOriginFromUrl", () => { + it("keeps a plain http object store, which local and self-hosted setups run", () => { + expect(imageOriginFromUrl("http://localhost:9005")).toBe("http://localhost:9005"); + }); + + it("drops the path and query a presigned URL carries", () => { + expect(imageOriginFromUrl("https://s3.example.com/bucket/key.png?X-Amz-Signature=abc")).toBe( + "https://s3.example.com" + ); + }); + + it.each([ + ["unset", undefined], + ["empty", ""], + ["not a URL", "s3.example.com"], + ["a non-http scheme", "s3://bucket"], + ])("is undefined when the base URL is %s", (_case, value) => { + expect(imageOriginFromUrl(value)).toBeUndefined(); + }); + + it("permits a presigned image once it is in the directive", () => { + const origin = imageOriginFromUrl("http://localhost:9005"); + const directive = buildImgSrcDirective(origin ? [origin] : []); + + expect( + directivePermits( + directive, + "http://localhost:9005/avatars-local/avatars/usr_1/abc.png?X-Amz-Expires=300" + ) + ).toBe(true); + expect( + directivePermits(buildImgSrcDirective(), "http://localhost:9005/avatars-local/a.png") + ).toBe(false); + }); +}); diff --git a/apps/webapp/app/utils/cspImageOrigins.ts b/apps/webapp/app/utils/cspImageOrigins.ts index 5d52c4b77fe..1c89321d640 100644 --- a/apps/webapp/app/utils/cspImageOrigins.ts +++ b/apps/webapp/app/utils/cspImageOrigins.ts @@ -112,6 +112,28 @@ function rejectionReason(value: string, allowHttp: boolean): string | undefined return undefined; } +/** + * The origin of a URL the operator configured themselves, keeping its scheme: an object + * store on plain http is a normal local or self-hosted setup. Origin only — CSP matches + * the host and ignores the presigned query string. + */ +export function imageOriginFromUrl(baseUrl: string | undefined | null): string | undefined { + if (!baseUrl) return undefined; + + let url: URL; + try { + url = new URL(baseUrl); + } catch { + return undefined; + } + + if ((url.protocol !== "http:" && url.protocol !== "https:") || url.host.length === 0) { + return undefined; + } + + return `${url.protocol}//${url.host}`; +} + /** The full directive: the base sources plus any configured extra origins. */ export function buildImgSrcDirective(extraOrigins: readonly string[] = []): string { return ["img-src", ...BASE_IMG_SRC_SOURCES, ...extraOrigins].join(" "); diff --git a/apps/webapp/test/userAvatar.test.ts b/apps/webapp/test/userAvatar.test.ts index a5d46e7d09e..45d7ba86178 100644 --- a/apps/webapp/test/userAvatar.test.ts +++ b/apps/webapp/test/userAvatar.test.ts @@ -5,6 +5,7 @@ import { buildUserAvatarUrl, isAvatarUploadRejection, absoluteUserAvatarUrl, + avatarObjectStoreImageOrigin, parseAvatarUpload, presignUserAvatarUrl, resolveStaleAvatarObjectPath, @@ -265,3 +266,29 @@ describe("the avatar object store", () => { expect(url).toContain("X-Amz-Signature="); }); }); + +describe("avatarObjectStoreImageOrigin", () => { + const originalBaseUrl = env.OBJECT_STORE_S3_BASE_URL; + + afterEach(() => { + env.OBJECT_STORE_S3_BASE_URL = originalBaseUrl; + }); + + it("is the store's origin when one is configured, http included", () => { + env.OBJECT_STORE_S3_BASE_URL = "http://localhost:9005"; + + expect(avatarObjectStoreImageOrigin()).toBe("http://localhost:9005"); + }); + + it("keeps only the origin of a store URL that carries a path", () => { + env.OBJECT_STORE_S3_BASE_URL = "https://s3.eu-west-1.amazonaws.com/avatars"; + + expect(avatarObjectStoreImageOrigin()).toBe("https://s3.eu-west-1.amazonaws.com"); + }); + + it("is undefined when no avatar store is configured", () => { + env.OBJECT_STORE_S3_BASE_URL = undefined; + + expect(avatarObjectStoreImageOrigin()).toBeUndefined(); + }); +}); From cefdc340f25f2feb190d77ff28bb201a422e0805 Mon Sep 17 00:00:00 2001 From: Katia Bulatova Date: Thu, 27 Aug 2026 11:40:09 +0000 Subject: [PATCH 08/25] fix(webapp): reject unsafe hosts when deriving an image policy origin --- apps/webapp/app/entry.server.tsx | 11 +++++---- apps/webapp/app/utils/cspImageOrigins.test.ts | 24 +++++++++++++++++++ apps/webapp/app/utils/cspImageOrigins.ts | 13 ++++++++++ 3 files changed, 44 insertions(+), 4 deletions(-) diff --git a/apps/webapp/app/entry.server.tsx b/apps/webapp/app/entry.server.tsx index 2c662fde678..661ef0f238f 100644 --- a/apps/webapp/app/entry.server.tsx +++ b/apps/webapp/app/entry.server.tsx @@ -21,7 +21,12 @@ import { env } from "./env.server"; import { eventLoopMonitor, eventLoopUtilizationMonitor } from "./eventLoopMonitor.server"; import { logger } from "./services/logger.server"; import { avatarObjectStoreImageOrigin } from "./services/userAvatar.server"; -import { buildImgSrcDirective, parseCspImageOrigins, withImgSrc } from "./utils/cspImageOrigins"; +import { + appendImageOrigin, + buildImgSrcDirective, + parseCspImageOrigins, + withImgSrc, +} from "./utils/cspImageOrigins"; import { singleton } from "./utils/singleton"; import { remoteBuildsEnabled } from "./v3/remoteImageBuilder.server"; import { @@ -82,9 +87,7 @@ const IMG_SRC_DIRECTIVE = buildImgSrcDirective( ); } - const avatarOrigin = avatarObjectStoreImageOrigin(); - - return avatarOrigin && !origins.includes(avatarOrigin) ? [...origins, avatarOrigin] : origins; + return appendImageOrigin(origins, avatarObjectStoreImageOrigin()); }) ); diff --git a/apps/webapp/app/utils/cspImageOrigins.test.ts b/apps/webapp/app/utils/cspImageOrigins.test.ts index f6d03fa3c79..399ef1d621b 100644 --- a/apps/webapp/app/utils/cspImageOrigins.test.ts +++ b/apps/webapp/app/utils/cspImageOrigins.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from "vitest"; import { faviconUrl } from "./favicon"; import { + appendImageOrigin, BASE_IMG_SRC_SOURCES, buildImgSrcDirective, imageOriginFromUrl, @@ -207,6 +208,10 @@ describe("imageOriginFromUrl", () => { ["empty", ""], ["not a URL", "s3.example.com"], ["a non-http scheme", "s3://bucket"], + ["a wildcard host", "http://*.evil.com"], + ["a host carrying a directive separator", "http://evil.com;script-src"], + ["a host carrying a source separator", "http://evil.com,https://other.test"], + ["a host with whitespace", "http://evil.com script-src"], ])("is undefined when the base URL is %s", (_case, value) => { expect(imageOriginFromUrl(value)).toBeUndefined(); }); @@ -226,3 +231,22 @@ describe("imageOriginFromUrl", () => { ).toBe(false); }); }); + +describe("appendImageOrigin", () => { + it("leaves the directive unchanged when no origin is configured", () => { + expect(buildImgSrcDirective(appendImageOrigin([], undefined))).toBe(buildImgSrcDirective()); + }); + + it("does not list an origin twice", () => { + expect(appendImageOrigin(["http://localhost:9005"], "http://localhost:9005")).toEqual([ + "http://localhost:9005", + ]); + }); + + it("appends a new origin after the configured ones", () => { + expect(appendImageOrigin(["https://sso.example.com"], "http://localhost:9005")).toEqual([ + "https://sso.example.com", + "http://localhost:9005", + ]); + }); +}); diff --git a/apps/webapp/app/utils/cspImageOrigins.ts b/apps/webapp/app/utils/cspImageOrigins.ts index 1c89321d640..c2113c64764 100644 --- a/apps/webapp/app/utils/cspImageOrigins.ts +++ b/apps/webapp/app/utils/cspImageOrigins.ts @@ -120,6 +120,10 @@ function rejectionReason(value: string, allowHttp: boolean): string | undefined export function imageOriginFromUrl(baseUrl: string | undefined | null): string | undefined { if (!baseUrl) return undefined; + // `new URL` keeps these in the host, and a ";" or "," would truncate or inject a + // directive once the sources are space-joined. + if (/[*;,]|\s/.test(baseUrl)) return undefined; + let url: URL; try { url = new URL(baseUrl); @@ -134,6 +138,15 @@ export function imageOriginFromUrl(baseUrl: string | undefined | null): string | return `${url.protocol}//${url.host}`; } +/** Adds an optional origin to a source list, keeping it free of duplicates. */ +export function appendImageOrigin( + origins: readonly string[], + origin: string | undefined +): string[] { + if (!origin || origins.includes(origin)) return [...origins]; + return [...origins, origin]; +} + /** The full directive: the base sources plus any configured extra origins. */ export function buildImgSrcDirective(extraOrigins: readonly string[] = []): string { return ["img-src", ...BASE_IMG_SRC_SOURCES, ...extraOrigins].join(" "); From b4cf0ae1b28c2e4e84f619d10becc11fa8bfb420 Mon Sep 17 00:00:00 2001 From: Katia Bulatova Date: Thu, 27 Aug 2026 11:46:58 +0000 Subject: [PATCH 09/25] feat(webapp): remove your profile photo from the account page --- .../app/routes/resources.account.avatar.ts | 12 +++++++++- apps/webapp/app/services/userAvatar.server.ts | 5 +++-- apps/webapp/test/userAvatar.test.ts | 22 +++++++++++++++++++ 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/apps/webapp/app/routes/resources.account.avatar.ts b/apps/webapp/app/routes/resources.account.avatar.ts index 101bbe2cbd2..4b9c3581b69 100644 --- a/apps/webapp/app/routes/resources.account.avatar.ts +++ b/apps/webapp/app/routes/resources.account.avatar.ts @@ -9,12 +9,22 @@ import { } from "~/services/userAvatar.server"; export async function action({ request }: ActionFunctionArgs) { - if (request.method.toUpperCase() !== "POST") { + const method = request.method.toUpperCase(); + + if (method !== "POST" && method !== "DELETE") { return json({ error: "Method not allowed" }, { status: 405 }); } const user = await requireUser(request); + if (method === "DELETE") { + await updateUserAvatarUrl({ id: user.id, avatarUrl: null }); + + await deleteStaleUserAvatar({ previousAvatarUrl: user.avatarUrl, userId: user.id }); + + return json({ avatarUrl: null }); + } + const upload = await parseAvatarUpload(await request.formData()); if (isAvatarUploadRejection(upload)) { diff --git a/apps/webapp/app/services/userAvatar.server.ts b/apps/webapp/app/services/userAvatar.server.ts index 472b28112f5..99d3cdd3106 100644 --- a/apps/webapp/app/services/userAvatar.server.ts +++ b/apps/webapp/app/services/userAvatar.server.ts @@ -129,6 +129,7 @@ const AVATAR_URL_REGEX = /^\/resources\/account\/avatar\/([^/]+)\/([^/]+)$/; /** * Undefined unless the stored URL is this user's own avatar route and names a different object: * an OAuth avatar elsewhere is not ours to delete, and the same content hash is the same file. + * Without a replacement filename the object is always stale — the avatar is being removed. */ export function resolveStaleAvatarObjectPath({ previousAvatarUrl, @@ -137,7 +138,7 @@ export function resolveStaleAvatarObjectPath({ }: { previousAvatarUrl: string | null; userId: string; - filename: string; + filename?: string; }): string | undefined { const match = previousAvatarUrl?.match(AVATAR_URL_REGEX); @@ -157,7 +158,7 @@ export function resolveStaleAvatarObjectPath({ export async function deleteStaleUserAvatar(options: { previousAvatarUrl: string | null; userId: string; - filename: string; + filename?: string; }) { const path = resolveStaleAvatarObjectPath(options); diff --git a/apps/webapp/test/userAvatar.test.ts b/apps/webapp/test/userAvatar.test.ts index 45d7ba86178..83192329e08 100644 --- a/apps/webapp/test/userAvatar.test.ts +++ b/apps/webapp/test/userAvatar.test.ts @@ -292,3 +292,25 @@ describe("avatarObjectStoreImageOrigin", () => { expect(avatarObjectStoreImageOrigin()).toBeUndefined(); }); }); + +describe("resolveStaleAvatarObjectPath on removal", () => { + const stored = filenameFor([1, 2, 3]); + + it("derives the object to drop when there is no replacement", () => { + expect( + resolveStaleAvatarObjectPath({ + previousAvatarUrl: buildUserAvatarUrl(USER_ID, stored), + userId: USER_ID, + }) + ).toBe(`avatars/${USER_ID}/${stored}`); + }); + + it.each([ + ["no avatar", null], + ["an OAuth avatar", "https://avatars.githubusercontent.com/u/1?v=4"], + ["another user's avatar", `/resources/account/avatar/usr_other/${stored}`], + ["a traversal filename", `/resources/account/avatar/${USER_ID}/../../secret.png`], + ])("deletes nothing for %s", (_case, previousAvatarUrl) => { + expect(resolveStaleAvatarObjectPath({ previousAvatarUrl, userId: USER_ID })).toBeUndefined(); + }); +}); From 5aca64680f4f5679531b53ed2006fe3527135c34 Mon Sep 17 00:00:00 2001 From: Katia Bulatova Date: Thu, 27 Aug 2026 11:46:25 +0000 Subject: [PATCH 10/25] feat(webapp): show, remove and drag-drop the profile picture in the photo editor --- .../app/components/ProfilePhotoEditor.tsx | 95 ++++++++++++++++--- .../storybook.profile-photo-editor/route.tsx | 32 ++++++- 2 files changed, 108 insertions(+), 19 deletions(-) diff --git a/apps/webapp/app/components/ProfilePhotoEditor.tsx b/apps/webapp/app/components/ProfilePhotoEditor.tsx index dd48178192e..2bfd1121c14 100644 --- a/apps/webapp/app/components/ProfilePhotoEditor.tsx +++ b/apps/webapp/app/components/ProfilePhotoEditor.tsx @@ -1,6 +1,7 @@ import { MagnifyingGlassMinusIcon, MagnifyingGlassPlusIcon } from "@heroicons/react/20/solid"; import { useEffect, useRef, useState } from "react"; import Cropper, { type Area, type Point } from "react-easy-crop"; +import { cn } from "~/utils/cn"; import { Button } from "./primitives/Buttons"; import { Dialog, @@ -56,14 +57,16 @@ type ProfilePhotoEditorProps = { open: boolean; onOpenChange: (open: boolean) => void; onSave: (blob: Blob) => void; + currentAvatarUrl?: string; + onRemove?: () => void; isSaving?: boolean; }; export function ProfilePhotoEditor({ open, onOpenChange, - onSave, isSaving = false, + ...editorProps }: ProfilePhotoEditorProps) { return ( @@ -72,26 +75,42 @@ export function ProfilePhotoEditor({ Profile picture {/* Radix unmounts the content when closed, so the crop state resets with it. */} - + ); } -function Editor({ onSave, isSaving }: Pick) { +type EditorProps = Omit; + +function Editor({ onSave, currentAvatarUrl, onRemove, isSaving }: EditorProps) { const fileInputRef = useRef(null); const [imageSrc, setImageSrc] = useState(); const [crop, setCrop] = useState(CENTER); const [zoom, setZoom] = useState(MIN_ZOOM); const [croppedArea, setCroppedArea] = useState(); const [error, setError] = useState(); + const [isDraggingOver, setIsDraggingOver] = useState(false); useEffect(() => { if (!imageSrc) return; return () => URL.revokeObjectURL(imageSrc); }, [imageSrc]); + // A drop landing outside our own handlers would navigate the tab to the file + // and lose the crop. Editor only exists while the dialog is open. + useEffect(() => { + const suppress = (event: DragEvent) => event.preventDefault(); + window.addEventListener("dragover", suppress); + window.addEventListener("drop", suppress); + return () => { + window.removeEventListener("dragover", suppress); + window.removeEventListener("drop", suppress); + }; + }, []); + function selectFile(file: File | undefined) { + if (isSaving) return; if (!file) return; if (!ACCEPTED_TYPES.includes(file.type)) { @@ -117,7 +136,23 @@ function Editor({ onSave, isSaving }: Pick +
{ + event.preventDefault(); + setIsDraggingOver(true); + }} + onDragLeave={(event) => { + // Moving between children fires dragleave too, so ignore inside targets. + if (event.currentTarget.contains(event.relatedTarget as Node | null)) return; + setIsDraggingOver(false); + }} + onDrop={(event) => { + event.preventDefault(); + setIsDraggingOver(false); + selectFile(event.dataTransfer.files[0]); + }} + >
{imageSrc ? ( <> -
+
+ ) : currentAvatarUrl ? ( +
+ + Drop an image here to replace it +
) : ( )} @@ -177,13 +235,20 @@ function Editor({ onSave, isSaving }: Pick - +
+ + {onRemove && currentAvatarUrl && ( + + )} +
); } diff --git a/apps/webapp/app/routes/storybook.profile-photo-editor/route.tsx b/apps/webapp/app/routes/storybook.profile-photo-editor/route.tsx index 9eba2f3b57c..efae92569a9 100644 --- a/apps/webapp/app/routes/storybook.profile-photo-editor/route.tsx +++ b/apps/webapp/app/routes/storybook.profile-photo-editor/route.tsx @@ -3,7 +3,23 @@ import { ProfilePhotoEditor } from "~/components/ProfilePhotoEditor"; import { Button } from "~/components/primitives/Buttons"; import { Story, StoryGrid, StoryPage, StorySection } from "../storybook/StoryKit"; -function EditorStory({ isSaving }: { isSaving?: boolean }) { +// Data URI, not a remote image: the document `img-src` CSP allowlist has no +// placeholder host. +const PLACEHOLDER_AVATAR = + "data:image/svg+xml;utf8," + + encodeURIComponent( + `` + ); + +function EditorStory({ + isSaving, + currentAvatarUrl, + withRemove, +}: { + isSaving?: boolean; + currentAvatarUrl?: string; + withRemove?: boolean; +}) { const [open, setOpen] = useState(false); return ( @@ -15,6 +31,8 @@ function EditorStory({ isSaving }: { isSaving?: boolean }) { open={open} onOpenChange={setOpen} onSave={() => setOpen(false)} + currentAvatarUrl={currentAvatarUrl} + onRemove={withRemove ? () => setOpen(false) : undefined} isSaving={isSaving} /> @@ -26,15 +44,21 @@ export default function Story_() { - + + + + + + + - + From 4516205463d847d99a701b5b0d4e078323925386 Mon Sep 17 00:00:00 2001 From: Katia Bulatova Date: Thu, 27 Aug 2026 11:51:24 +0000 Subject: [PATCH 11/25] feat(webapp): show and remove the current profile picture from the account page --- .server-changes/profile-picture-upload.md | 2 +- .../app/routes/account._index/route.tsx | 27 ++++++++++++++++--- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/.server-changes/profile-picture-upload.md b/.server-changes/profile-picture-upload.md index 97b1e065d3e..0f861c8437b 100644 --- a/.server-changes/profile-picture-upload.md +++ b/.server-changes/profile-picture-upload.md @@ -3,4 +3,4 @@ area: webapp type: feature --- -You can now upload and crop your own profile picture from your account page. +You can now upload and crop your own profile picture from your account page, and remove it again whenever you like. diff --git a/apps/webapp/app/routes/account._index/route.tsx b/apps/webapp/app/routes/account._index/route.tsx index 825e26ed5df..32e0fcb9782 100644 --- a/apps/webapp/app/routes/account._index/route.tsx +++ b/apps/webapp/app/routes/account._index/route.tsx @@ -449,11 +449,13 @@ function useProfileFieldUpdate({ } function ChangeProfilePhotoButton() { + const user = useUser(); const [isOpen, setIsOpen] = useState(false); - const fetcher = useFetcher<{ avatarUrl?: string; error?: string }>(); + const fetcher = useFetcher<{ avatarUrl?: string | null; error?: string }>(); const toast = useToast(); const isSaving = fetcher.state !== "idle"; const submitSeenRef = useRef(false); + const actionRef = useRef<"save" | "remove">("save"); useEffect(() => { if (fetcher.state !== "idle") { @@ -463,10 +465,19 @@ function ChangeProfilePhotoButton() { if (!submitSeenRef.current) return; submitSeenRef.current = false; - if (fetcher.data?.avatarUrl) { - // oxlint-disable-next-line react/set-state-in-effect -- Closes the modal once the upload has landed. + const removing = actionRef.current === "remove"; + const succeeded = removing + ? fetcher.data?.avatarUrl === null + : Boolean(fetcher.data?.avatarUrl); + + if (succeeded) { + // oxlint-disable-next-line react/set-state-in-effect -- Closes the modal once the change has landed. setIsOpen(false); - toast.success("Your profile picture has been updated."); + toast.success( + removing + ? "Your profile picture has been removed." + : "Your profile picture has been updated." + ); return; } @@ -474,6 +485,7 @@ function ChangeProfilePhotoButton() { }, [fetcher.state, fetcher.data, toast]); const save = (blob: Blob) => { + actionRef.current = "save"; const formData = new FormData(); formData.append("image", blob, "avatar.png"); fetcher.submit(formData, { @@ -483,6 +495,11 @@ function ChangeProfilePhotoButton() { }); }; + const remove = () => { + actionRef.current = "remove"; + fetcher.submit(null, { method: "delete", action: "/resources/account/avatar" }); + }; + return ( <> - {onRemove && currentAvatarUrl && ( + {onRemove && currentAvatarUrl && !imageSrc && ( From 89421bc69bf1ae12961b007b233dab21950d1d76 Mon Sep 17 00:00:00 2001 From: Katia Bulatova Date: Thu, 27 Aug 2026 12:10:55 +0000 Subject: [PATCH 13/25] feat(webapp): serve avatar bytes from our own origin for re-cropping --- ...ources.account.avatar.$userId.$filename.ts | 36 +++++++++++++++++-- apps/webapp/app/services/userAvatar.server.ts | 6 ++++ apps/webapp/app/utils/avatarLimits.ts | 9 +++++ .../webapp/app/v3/objectStoreClient.server.ts | 32 +++++++++++++++++ apps/webapp/test/userAvatar.test.ts | 28 ++++++++++++++- 5 files changed, 107 insertions(+), 4 deletions(-) diff --git a/apps/webapp/app/routes/resources.account.avatar.$userId.$filename.ts b/apps/webapp/app/routes/resources.account.avatar.$userId.$filename.ts index 3c7059a2ab4..11f647e9afc 100644 --- a/apps/webapp/app/routes/resources.account.avatar.$userId.$filename.ts +++ b/apps/webapp/app/routes/resources.account.avatar.$userId.$filename.ts @@ -1,9 +1,18 @@ import { redirect, type LoaderFunctionArgs } from "@remix-run/node"; import { requireUser } from "~/services/session.server"; -import { presignUserAvatarUrl, resolveUserAvatarObjectPath } from "~/services/userAvatar.server"; +import { + presignUserAvatarUrl, + readUserAvatarBytes, + resolveUserAvatarObjectPath, +} from "~/services/userAvatar.server"; +import { avatarContentTypeForFilename } from "~/utils/avatarLimits"; + +/** Content-hashed filename, so a hit never goes stale. */ +const RAW_CACHE_CONTROL = "private, max-age=31536000, immutable"; /** * Presigned URLs expire, so the stored avatarUrl points here and we sign on each request. + * `?raw` serves the bytes from this origin instead, so a canvas reading them stays untainted. */ export async function loader({ request, params }: LoaderFunctionArgs) { await requireUser(request); @@ -11,9 +20,30 @@ export async function loader({ request, params }: LoaderFunctionArgs) { const { userId, filename } = params; const objectPath = userId && filename ? resolveUserAvatarObjectPath(userId, filename) : undefined; - if (!objectPath) { + if (!objectPath || !filename) { + throw new Response("Not found", { status: 404 }); + } + + if (!new URL(request.url).searchParams.has("raw")) { + return redirect(await presignUserAvatarUrl(objectPath)); + } + + const contentType = avatarContentTypeForFilename(filename); + const bytes = contentType ? await readUserAvatarBytes(objectPath) : undefined; + + if (!bytes || !contentType) { throw new Response("Not found", { status: 404 }); } - return redirect(await presignUserAvatarUrl(objectPath)); + // Byte bodies are valid BodyInit at runtime; the ambient fetch types don't say so. + return new Response(bytes as unknown as BodyInit, { + headers: { + "Content-Type": contentType, + "Content-Length": String(bytes.byteLength), + "Cache-Control": RAW_CACHE_CONTROL, + // User-supplied bytes on our own origin, which CSP 'self' trusts. + "X-Content-Type-Options": "nosniff", + "Content-Disposition": "inline", + }, + }); } diff --git a/apps/webapp/app/services/userAvatar.server.ts b/apps/webapp/app/services/userAvatar.server.ts index 99d3cdd3106..26be97e91be 100644 --- a/apps/webapp/app/services/userAvatar.server.ts +++ b/apps/webapp/app/services/userAvatar.server.ts @@ -178,6 +178,12 @@ export async function deleteStaleUserAvatar(options: { } } +export function readUserAvatarBytes(objectPath: string) { + const { client, objectKey } = requireAvatarObjectStore(); + + return client.getObjectBytes(objectKey(objectPath)); +} + export function presignUserAvatarUrl(objectPath: string) { const { client, objectKey } = requireAvatarObjectStore(); diff --git a/apps/webapp/app/utils/avatarLimits.ts b/apps/webapp/app/utils/avatarLimits.ts index 4ba3e59764a..c6d6a63cb97 100644 --- a/apps/webapp/app/utils/avatarLimits.ts +++ b/apps/webapp/app/utils/avatarLimits.ts @@ -12,6 +12,15 @@ export function isAvatarContentType(contentType: string): contentType is AvatarC return contentType in AVATAR_EXTENSIONS; } +/** The stored filename is content-hash + ext, so its ext is the only type signal we keep. */ +export function avatarContentTypeForFilename(filename: string): AvatarContentType | undefined { + const ext = filename.split(".").pop(); + + return (Object.keys(AVATAR_EXTENSIONS) as AvatarContentType[]).find( + (contentType) => AVATAR_EXTENSIONS[contentType] === ext + ); +} + function startsWith(data: Uint8Array, signature: number[], offset = 0) { return signature.every((byte, index) => data[offset + index] === byte); } diff --git a/apps/webapp/app/v3/objectStoreClient.server.ts b/apps/webapp/app/v3/objectStoreClient.server.ts index 7e671566f96..ff41f4c01d6 100644 --- a/apps/webapp/app/v3/objectStoreClient.server.ts +++ b/apps/webapp/app/v3/objectStoreClient.server.ts @@ -2,6 +2,7 @@ import { AwsClient } from "aws4fetch"; import { DeleteObjectCommand, GetObjectCommand, + NoSuchKey, PutObjectCommand, S3Client, } from "@aws-sdk/client-s3"; @@ -24,6 +25,8 @@ interface IObjectStoreClient { contentType: string ): Promise; getObject(key: string): Promise; + /** Undefined when the object is not there, so a caller can 404 instead of throwing. */ + getObjectBytes(key: string): Promise; deleteObject(key: string): Promise; presign(key: string, method: "PUT" | "GET", expiresIn: number): Promise; } @@ -82,6 +85,17 @@ class Aws4FetchClient implements IObjectStoreClient { return response.text(); } + async getObjectBytes(key: string): Promise { + const response = await this.awsClient.fetch(this.buildUrl(key)); + if (response.status === 404) { + return undefined; + } + if (!response.ok) { + throw new Error(`Failed to download from object store: ${response.statusText}`); + } + return new Uint8Array(await response.arrayBuffer()); + } + async deleteObject(key: string): Promise { const response = await this.awsClient.fetch(this.buildUrl(key), { method: "DELETE" }); if (!response.ok) { @@ -164,6 +178,20 @@ class AwsSdkClient implements IObjectStoreClient { return response.Body.transformToString(); } + async getObjectBytes(key: string): Promise { + try { + const response = await this.s3Client.send( + new GetObjectCommand({ Bucket: this.config.bucket, Key: this.toS3ObjectKey(key) }) + ); + return await response.Body?.transformToByteArray(); + } catch (error) { + if (error instanceof NoSuchKey || (error as { name?: string }).name === "NotFound") { + return undefined; + } + throw error; + } + } + async deleteObject(key: string): Promise { await this.s3Client.send( new DeleteObjectCommand({ Bucket: this.config.bucket, Key: this.toS3ObjectKey(key) }) @@ -240,6 +268,10 @@ export class ObjectStoreClient implements IObjectStoreClient { return this.impl.getObject(key); } + getObjectBytes(key: string): Promise { + return this.impl.getObjectBytes(key); + } + deleteObject(key: string): Promise { return this.impl.deleteObject(key); } diff --git a/apps/webapp/test/userAvatar.test.ts b/apps/webapp/test/userAvatar.test.ts index 83192329e08..4cf42ed1ea1 100644 --- a/apps/webapp/test/userAvatar.test.ts +++ b/apps/webapp/test/userAvatar.test.ts @@ -11,7 +11,7 @@ import { resolveStaleAvatarObjectPath, resolveUserAvatarObjectPath, } from "~/services/userAvatar.server"; -import { MAX_AVATAR_SIZE_IN_BYTES } from "~/utils/avatarLimits"; +import { avatarContentTypeForFilename, MAX_AVATAR_SIZE_IN_BYTES } from "~/utils/avatarLimits"; const USER_ID = "clzabc123"; @@ -314,3 +314,29 @@ describe("resolveStaleAvatarObjectPath on removal", () => { expect(resolveStaleAvatarObjectPath({ previousAvatarUrl, userId: USER_ID })).toBeUndefined(); }); }); + +describe("avatarContentTypeForFilename", () => { + it.each([ + ["image/png", "png"], + ["image/jpeg", "jpg"], + ["image/webp", "webp"], + ])("serves %s for a stored .%s object", (contentType, ext) => { + const filename = `${"a".repeat(32)}.${ext}`; + + expect(avatarContentTypeForFilename(filename)).toBe(contentType); + }); + + it("round-trips the filename the upload produced", () => { + const filename = buildUserAvatarFilename("image/webp", new Uint8Array([1, 2, 3])); + + expect(avatarContentTypeForFilename(filename)).toBe("image/webp"); + }); + + it.each([ + ["an ext we never store", `${"a".repeat(32)}.svg`], + ["no ext at all", "a".repeat(32)], + ["an empty name", ""], + ])("is undefined for %s", (_case, filename) => { + expect(avatarContentTypeForFilename(filename)).toBeUndefined(); + }); +}); From a651d06890a9bdef06a282c9cb2e01c5379cbb57 Mon Sep 17 00:00:00 2001 From: Katia Bulatova Date: Thu, 27 Aug 2026 12:11:05 +0000 Subject: [PATCH 14/25] feat(webapp): load the existing profile picture straight into the cropper --- .../app/components/ProfilePhotoEditor.tsx | 72 ++++++++++++++----- .../storybook.profile-photo-editor/route.tsx | 10 +-- 2 files changed, 61 insertions(+), 21 deletions(-) diff --git a/apps/webapp/app/components/ProfilePhotoEditor.tsx b/apps/webapp/app/components/ProfilePhotoEditor.tsx index 44a54d74bad..d0ecd878224 100644 --- a/apps/webapp/app/components/ProfilePhotoEditor.tsx +++ b/apps/webapp/app/components/ProfilePhotoEditor.tsx @@ -12,6 +12,7 @@ import { } from "./primitives/Dialog"; import { Paragraph } from "./primitives/Paragraph"; import { Slider } from "./primitives/Slider"; +import { Spinner } from "./primitives/Spinner"; const ACCEPTED_TYPES = ["image/png", "image/jpeg", "image/webp"]; const OUTPUT_SIZE = 512; @@ -62,6 +63,10 @@ type ProfilePhotoEditorProps = { isSaving?: boolean; }; +function isInlineImage(url: string) { + return url.startsWith("data:"); +} + export function ProfilePhotoEditor({ open, onOpenChange, @@ -85,18 +90,60 @@ type EditorProps = Omit; function Editor({ onSave, currentAvatarUrl, onRemove, isSaving }: EditorProps) { const fileInputRef = useRef(null); - const [imageSrc, setImageSrc] = useState(); + // Ref for the async load guard, state for rendering. + const hasPickedRef = useRef(false); + const [hasPicked, setHasPicked] = useState(false); + const isInline = currentAvatarUrl !== undefined && isInlineImage(currentAvatarUrl); + const [imageSrc, setImageSrc] = useState(isInline ? currentAvatarUrl : undefined); const [crop, setCrop] = useState(CENTER); const [zoom, setZoom] = useState(MIN_ZOOM); const [croppedArea, setCroppedArea] = useState(); const [error, setError] = useState(); const [isDraggingOver, setIsDraggingOver] = useState(false); + const [isLoadingCurrent, setIsLoadingCurrent] = useState( + currentAvatarUrl !== undefined && !isInline + ); useEffect(() => { if (!imageSrc) return; return () => URL.revokeObjectURL(imageSrc); }, [imageSrc]); + // The cropper exports through a canvas, so the current photo has to come in as + // same-origin bytes rather than a remote URL. A missing one is just no photo. + useEffect(() => { + if (currentAvatarUrl === undefined || isInlineImage(currentAvatarUrl)) return; + + let cancelled = false; + + async function loadCurrentAvatar(url: string) { + try { + const response = await fetch(`${url}?raw`); + if (!response.ok) return; + + const blob = await response.blob(); + if (cancelled || hasPickedRef.current) return; + // An expired session redirects to the login HTML, which fetch follows + // with response.ok still true and would leave a blank cropper. + if (response.redirected || !ACCEPTED_TYPES.includes(blob.type)) return; + + setImageSrc(URL.createObjectURL(blob)); + } catch { + // Leaves the empty state in place. + } finally { + if (!cancelled) { + setIsLoadingCurrent(false); + } + } + } + + void loadCurrentAvatar(currentAvatarUrl); + + return () => { + cancelled = true; + }; + }, [currentAvatarUrl]); + // A drop landing outside our own handlers would navigate the tab to the file // and lose the crop. Editor only exists while the dialog is open. useEffect(() => { @@ -118,10 +165,13 @@ function Editor({ onSave, currentAvatarUrl, onRemove, isSaving }: EditorProps) { return; } + hasPickedRef.current = true; + setHasPicked(true); setCrop(CENTER); setZoom(MIN_ZOOM); setCroppedArea(undefined); setError(undefined); + setIsLoadingCurrent(false); setImageSrc(URL.createObjectURL(file)); } @@ -200,20 +250,9 @@ function Editor({ onSave, currentAvatarUrl, onRemove, isSaving }: EditorProps) { TrailingIcon={MagnifyingGlassPlusIcon} /> - ) : currentAvatarUrl ? ( -
- - Drop an image here to replace it + ) : isLoadingCurrent ? ( +
+
) : ( - {onRemove && currentAvatarUrl && !imageSrc && ( + {/* Only while the existing photo is showing, or it would discard a pending crop. */} + {onRemove && imageSrc && !hasPicked && ( diff --git a/apps/webapp/app/routes/storybook.profile-photo-editor/route.tsx b/apps/webapp/app/routes/storybook.profile-photo-editor/route.tsx index efae92569a9..a0e139deebc 100644 --- a/apps/webapp/app/routes/storybook.profile-photo-editor/route.tsx +++ b/apps/webapp/app/routes/storybook.profile-photo-editor/route.tsx @@ -3,12 +3,12 @@ import { ProfilePhotoEditor } from "~/components/ProfilePhotoEditor"; import { Button } from "~/components/primitives/Buttons"; import { Story, StoryGrid, StoryPage, StorySection } from "../storybook/StoryKit"; -// Data URI, not a remote image: the document `img-src` CSP allowlist has no -// placeholder host. +// A data URI skips the `?raw` fetch, so the story needs no backend. Explicit +// width/height too, or the SVG has no intrinsic size to crop against. const PLACEHOLDER_AVATAR = "data:image/svg+xml;utf8," + encodeURIComponent( - `` + `` ); function EditorStory({ @@ -51,10 +51,10 @@ export default function Story_() { - + - + From fea2e6cb20d678cc249627ba4d6a10b11d967fe8 Mon Sep 17 00:00:00 2001 From: Katia Bulatova Date: Thu, 27 Aug 2026 12:06:13 +0000 Subject: [PATCH 15/25] feat(webapp): show a tooltip on the account page profile picture --- .../app/routes/account._index/route.tsx | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/apps/webapp/app/routes/account._index/route.tsx b/apps/webapp/app/routes/account._index/route.tsx index 32e0fcb9782..a6c186fec8b 100644 --- a/apps/webapp/app/routes/account._index/route.tsx +++ b/apps/webapp/app/routes/account._index/route.tsx @@ -34,6 +34,7 @@ import { Label } from "~/components/primitives/Label"; import { Switch } from "~/components/primitives/Switch"; import { Paragraph } from "~/components/primitives/Paragraph"; import { useToast } from "~/components/primitives/Toast"; +import { SimpleTooltip } from "~/components/primitives/Tooltip"; import { NavBar, PageTitle } from "~/components/primitives/PageHeader"; import { SETTINGS_ROW_TITLE_GAP, @@ -502,15 +503,25 @@ function ChangeProfilePhotoButton() { return ( <> - + setIsOpen(true)} + aria-label="Change your profile picture" + className="focus-custom group cursor-pointer rounded-full outline-hidden" + > + + + } + /> Date: Thu, 27 Aug 2026 12:34:40 +0000 Subject: [PATCH 16/25] feat(webapp): show the saved profile picture statically in the photo editor --- .../app/components/ProfilePhotoEditor.tsx | 94 ++++++------------- .../storybook.profile-photo-editor/route.tsx | 12 +-- 2 files changed, 35 insertions(+), 71 deletions(-) diff --git a/apps/webapp/app/components/ProfilePhotoEditor.tsx b/apps/webapp/app/components/ProfilePhotoEditor.tsx index d0ecd878224..dba0f27819b 100644 --- a/apps/webapp/app/components/ProfilePhotoEditor.tsx +++ b/apps/webapp/app/components/ProfilePhotoEditor.tsx @@ -12,7 +12,6 @@ import { } from "./primitives/Dialog"; import { Paragraph } from "./primitives/Paragraph"; import { Slider } from "./primitives/Slider"; -import { Spinner } from "./primitives/Spinner"; const ACCEPTED_TYPES = ["image/png", "image/jpeg", "image/webp"]; const OUTPUT_SIZE = 512; @@ -63,10 +62,6 @@ type ProfilePhotoEditorProps = { isSaving?: boolean; }; -function isInlineImage(url: string) { - return url.startsWith("data:"); -} - export function ProfilePhotoEditor({ open, onOpenChange, @@ -90,60 +85,18 @@ type EditorProps = Omit; function Editor({ onSave, currentAvatarUrl, onRemove, isSaving }: EditorProps) { const fileInputRef = useRef(null); - // Ref for the async load guard, state for rendering. - const hasPickedRef = useRef(false); - const [hasPicked, setHasPicked] = useState(false); - const isInline = currentAvatarUrl !== undefined && isInlineImage(currentAvatarUrl); - const [imageSrc, setImageSrc] = useState(isInline ? currentAvatarUrl : undefined); + const [imageSrc, setImageSrc] = useState(); const [crop, setCrop] = useState(CENTER); const [zoom, setZoom] = useState(MIN_ZOOM); const [croppedArea, setCroppedArea] = useState(); const [error, setError] = useState(); const [isDraggingOver, setIsDraggingOver] = useState(false); - const [isLoadingCurrent, setIsLoadingCurrent] = useState( - currentAvatarUrl !== undefined && !isInline - ); useEffect(() => { if (!imageSrc) return; return () => URL.revokeObjectURL(imageSrc); }, [imageSrc]); - // The cropper exports through a canvas, so the current photo has to come in as - // same-origin bytes rather than a remote URL. A missing one is just no photo. - useEffect(() => { - if (currentAvatarUrl === undefined || isInlineImage(currentAvatarUrl)) return; - - let cancelled = false; - - async function loadCurrentAvatar(url: string) { - try { - const response = await fetch(`${url}?raw`); - if (!response.ok) return; - - const blob = await response.blob(); - if (cancelled || hasPickedRef.current) return; - // An expired session redirects to the login HTML, which fetch follows - // with response.ok still true and would leave a blank cropper. - if (response.redirected || !ACCEPTED_TYPES.includes(blob.type)) return; - - setImageSrc(URL.createObjectURL(blob)); - } catch { - // Leaves the empty state in place. - } finally { - if (!cancelled) { - setIsLoadingCurrent(false); - } - } - } - - void loadCurrentAvatar(currentAvatarUrl); - - return () => { - cancelled = true; - }; - }, [currentAvatarUrl]); - // A drop landing outside our own handlers would navigate the tab to the file // and lose the crop. Editor only exists while the dialog is open. useEffect(() => { @@ -165,13 +118,10 @@ function Editor({ onSave, currentAvatarUrl, onRemove, isSaving }: EditorProps) { return; } - hasPickedRef.current = true; - setHasPicked(true); setCrop(CENTER); setZoom(MIN_ZOOM); setCroppedArea(undefined); setError(undefined); - setIsLoadingCurrent(false); setImageSrc(URL.createObjectURL(file)); } @@ -250,9 +200,20 @@ function Editor({ onSave, currentAvatarUrl, onRemove, isSaving }: EditorProps) { TrailingIcon={MagnifyingGlassPlusIcon} /> - ) : isLoadingCurrent ? ( -
- + ) : currentAvatarUrl ? ( +
+ {/* Fills the box like the cropper's circle, so switching doesn't jump. */} +
) : ( - {/* Only while the existing photo is showing, or it would discard a pending crop. */} - {onRemove && imageSrc && !hasPicked && ( + {/* Only while the saved photo is showing, or it would discard a pending crop. */} + {onRemove && currentAvatarUrl && !imageSrc && ( )}
- + {/* Nothing to save until a new file is cropped. */} + {imageSrc && ( + + )}
); diff --git a/apps/webapp/app/routes/storybook.profile-photo-editor/route.tsx b/apps/webapp/app/routes/storybook.profile-photo-editor/route.tsx index a0e139deebc..433a52da797 100644 --- a/apps/webapp/app/routes/storybook.profile-photo-editor/route.tsx +++ b/apps/webapp/app/routes/storybook.profile-photo-editor/route.tsx @@ -3,8 +3,8 @@ import { ProfilePhotoEditor } from "~/components/ProfilePhotoEditor"; import { Button } from "~/components/primitives/Buttons"; import { Story, StoryGrid, StoryPage, StorySection } from "../storybook/StoryKit"; -// A data URI skips the `?raw` fetch, so the story needs no backend. Explicit -// width/height too, or the SVG has no intrinsic size to crop against. +// Data URI, not a remote image: the document `img-src` CSP allowlist has no +// placeholder host. const PLACEHOLDER_AVATAR = "data:image/svg+xml;utf8," + encodeURIComponent( @@ -44,20 +44,20 @@ export default function Story_() { - + - + - + From a614903ff5c209948b865f88891bebeae14f324c Mon Sep 17 00:00:00 2001 From: Katia Bulatova Date: Thu, 27 Aug 2026 12:43:51 +0000 Subject: [PATCH 17/25] fix(webapp): move the remove button to the footer's right slot --- .../app/components/ProfilePhotoEditor.tsx | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/apps/webapp/app/components/ProfilePhotoEditor.tsx b/apps/webapp/app/components/ProfilePhotoEditor.tsx index dba0f27819b..048504610b0 100644 --- a/apps/webapp/app/components/ProfilePhotoEditor.tsx +++ b/apps/webapp/app/components/ProfilePhotoEditor.tsx @@ -235,23 +235,16 @@ function Editor({ onSave, currentAvatarUrl, onRemove, isSaving }: EditorProps) { )}
-
- - {/* Only while the saved photo is showing, or it would discard a pending crop. */} - {onRemove && currentAvatarUrl && !imageSrc && ( - - )} -
- {/* Nothing to save until a new file is cropped. */} - {imageSrc && ( + + {/* Nothing to save until a new file is cropped, so the saved photo offers + Remove in the same slot instead. */} + {imageSrc ? ( + ) : ( + onRemove && + currentAvatarUrl && ( + + ) )}
From 3eac9a37b5123baca459734fb1cc72c88d53c294 Mon Sep 17 00:00:00 2001 From: Katia Bulatova Date: Thu, 27 Aug 2026 12:58:17 +0000 Subject: [PATCH 18/25] fix(webapp): drop refused uploads and block avatar writes while impersonating --- .../app/routes/resources.account.avatar.ts | 7 ++ .../services/dashboardAgentBodyCap.server.ts | 3 + .../webapp/test/dashboardAgentBodyCap.test.ts | 69 ++++++++++++++++--- 3 files changed, 70 insertions(+), 9 deletions(-) diff --git a/apps/webapp/app/routes/resources.account.avatar.ts b/apps/webapp/app/routes/resources.account.avatar.ts index 4b9c3581b69..ba09676954c 100644 --- a/apps/webapp/app/routes/resources.account.avatar.ts +++ b/apps/webapp/app/routes/resources.account.avatar.ts @@ -17,6 +17,13 @@ export async function action({ request }: ActionFunctionArgs) { const user = await requireUser(request); + if (user.isImpersonating) { + return json( + { error: "You can't change this while impersonating another user." }, + { status: 403 } + ); + } + if (method === "DELETE") { await updateUserAvatarUrl({ id: user.id, avatarUrl: null }); diff --git a/apps/webapp/app/services/dashboardAgentBodyCap.server.ts b/apps/webapp/app/services/dashboardAgentBodyCap.server.ts index 5053d6565ff..1218305772e 100644 --- a/apps/webapp/app/services/dashboardAgentBodyCap.server.ts +++ b/apps/webapp/app/services/dashboardAgentBodyCap.server.ts @@ -84,6 +84,9 @@ function capRequestBody(req: Request, res: Response, cap: Cap): void { const declared = Number.parseInt(req.headers["content-length"] ?? "", 10); if (Number.isFinite(declared) && declared > limit) { refuse(res, cap); + // Same teardown as the overflow branch: a refused client must not keep the + // connection open trickling a body nobody will read. + res.once("finish", () => req.destroy()); return; } diff --git a/apps/webapp/test/dashboardAgentBodyCap.test.ts b/apps/webapp/test/dashboardAgentBodyCap.test.ts index fbef3b7761c..29aac8d6012 100644 --- a/apps/webapp/test/dashboardAgentBodyCap.test.ts +++ b/apps/webapp/test/dashboardAgentBodyCap.test.ts @@ -1,4 +1,4 @@ -import express from "express"; +import express, { type Request as ExpressRequest } from "express"; import http, { type Server } from "node:http"; import type { AddressInfo } from "node:net"; import { Readable } from "node:stream"; @@ -16,9 +16,18 @@ import { MAX_AVATAR_SIZE_IN_BYTES } from "~/utils/avatarLimits"; let server: Server | undefined; /** A server whose route stands in for Remix: it reads the whole body, like `text()` would. */ -async function listen(): Promise<{ url: string; buffered: () => number }> { +async function listen(): Promise<{ + url: string; + buffered: () => number; + requestDestroyed: () => boolean; +}> { let buffered = 0; + let lastRequest: ExpressRequest | undefined; const app = express(); + app.use((req, _res, next) => { + lastRequest = req; + next(); + }); app.use(dashboardAgentBodyCap); app.all("*", async (req, res) => { try { @@ -35,9 +44,19 @@ async function listen(): Promise<{ url: string; buffered: () => number }> { return { url: `http://127.0.0.1:${(server!.address() as AddressInfo).port}`, buffered: () => buffered, + requestDestroyed: () => lastRequest?.destroyed === true, }; } +/** Teardown happens on the response's "finish", which lands just after `fetch` resolves. */ +async function eventually(assertion: () => boolean) { + for (let attempt = 0; attempt < 50; attempt++) { + if (assertion()) return true; + await new Promise((resolve) => setTimeout(resolve, 10)); + } + return assertion(); +} + /** A chunked POST: `fetch` omits `content-length` for a stream body. */ function postChunked(url: string, totalBytes: number, chunkBytes = 16 * 1024) { let left = totalBytes; @@ -66,24 +85,41 @@ function postChunked(url: string, totalBytes: number, chunkBytes = 16 * 1024) { * `node:http` sends the path verbatim; `fetch` resolves dot segments client-side, so it cannot * express this request at all. */ -function postRawPath(url: string, path: string, bytes: number) { +function postRawPath(url: string, path: string, declaredBytes: number) { return new Promise((resolve, reject) => { let settled = false; + const finish = (status: number) => { + if (settled) return; + settled = true; + resolve(status); + }; + const request = http.request( - { port: Number(new URL(url).port), method: "POST", path }, + { + port: Number(new URL(url).port), + method: "POST", + path, + headers: { "content-length": String(declaredBytes) }, + }, (response) => { response.resume(); - settled = true; - resolve(response.statusCode ?? 0); + finish(response.statusCode ?? 0); request.destroy(); } ); - // A refusal tears the socket down mid-write; that is the pass, not an error. + // A refusal answers and then tears the socket down; a write error after that is + // expected. Anything else still fails the test. request.on("error", (error) => { - if (!settled) reject(error); + const code = (error as NodeJS.ErrnoException).code; + if (code === "EPIPE" || code === "ECONNRESET") finish(0); + else if (!settled) reject(error); }); - request.end(Buffer.alloc(bytes, "a")); + + // Only a token of the declared body: an uncapped server waits for the rest, which + // resolves as 0 rather than hanging the test. + request.write(Buffer.alloc(1024, "a")); + setTimeout(() => finish(0), 1500).unref(); }); } @@ -135,6 +171,21 @@ describe("the dashboard agent's ingress cap", () => { expect(buffered()).toBe(0); }); + it("drops a refused client that never sends the body it declared", async () => { + const { url, buffered, requestDestroyed } = await listen(); + + const status = await postRawPath( + url, + "/api/v1/dashboard-agent/watches/batch-check", + DASHBOARD_AGENT_MAX_INGRESS_BYTES + 1 + ); + + expect(status).toBe(413); + expect(buffered()).toBe(0); + // Otherwise the connection stays occupied while the client trickles the rest. + expect(await eventually(requestDestroyed)).toBe(true); + }); + it("passes a body under the cap through untouched", async () => { const { url } = await listen(); const size = 32 * 1024; From 237e84d1dc9a79bbcc4e24f694f66c6513bb2d7a Mon Sep 17 00:00:00 2001 From: Katia Bulatova Date: Thu, 27 Aug 2026 12:55:03 +0000 Subject: [PATCH 19/25] fix(webapp): fall back to the picker when the saved photo fails to load --- apps/webapp/app/components/ProfilePhotoEditor.tsx | 14 ++++++++++---- .../storybook.profile-photo-editor/route.tsx | 3 +++ 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/apps/webapp/app/components/ProfilePhotoEditor.tsx b/apps/webapp/app/components/ProfilePhotoEditor.tsx index 048504610b0..69a4a95c265 100644 --- a/apps/webapp/app/components/ProfilePhotoEditor.tsx +++ b/apps/webapp/app/components/ProfilePhotoEditor.tsx @@ -91,6 +91,10 @@ function Editor({ onSave, currentAvatarUrl, onRemove, isSaving }: EditorProps) { const [croppedArea, setCroppedArea] = useState(); const [error, setError] = useState(); const [isDraggingOver, setIsDraggingOver] = useState(false); + // Holding the url rather than a flag resets the fallback when it changes. + const [failedUrl, setFailedUrl] = useState(); + + const savedPhotoUrl = currentAvatarUrl === failedUrl ? undefined : currentAvatarUrl; useEffect(() => { if (!imageSrc) return; @@ -200,7 +204,7 @@ function Editor({ onSave, currentAvatarUrl, onRemove, isSaving }: EditorProps) { TrailingIcon={MagnifyingGlassPlusIcon} /> - ) : currentAvatarUrl ? ( + ) : savedPhotoUrl ? (
{/* Fills the box like the cropper's circle, so switching doesn't jump. */} setFailedUrl(savedPhotoUrl)} />
) : ( @@ -240,10 +245,11 @@ function Editor({ onSave, currentAvatarUrl, onRemove, isSaving }: EditorProps) { onClick={() => fileInputRef.current?.click()} disabled={isSaving} > - {imageSrc || currentAvatarUrl ? "Choose another" : "Choose image"} + {imageSrc || savedPhotoUrl ? "Choose another" : "Choose image"} {/* Nothing to save until a new file is cropped, so the saved photo offers - Remove in the same slot instead. */} + Remove in the same slot instead. Still offered when the preview failed + to load: there is a stored photo worth removing. */} {imageSrc ? (
diff --git a/apps/webapp/app/routes/resources.account.avatar.$userId.$filename.ts b/apps/webapp/app/routes/resources.account.avatar.$userId.$filename.ts index 23cab39df13..0130832f993 100644 --- a/apps/webapp/app/routes/resources.account.avatar.$userId.$filename.ts +++ b/apps/webapp/app/routes/resources.account.avatar.$userId.$filename.ts @@ -2,6 +2,7 @@ import { redirect } from "@remix-run/node"; import { z } from "zod"; import { dashboardLoader } from "~/services/routeBuilders/dashboardBuilder"; import { + isAvatarUploadsEnabled, presignUserAvatarUrl, readUserAvatarBytes, resolveUserAvatarObjectPath, @@ -24,7 +25,9 @@ const ParamsSchema = z.object({ export const loader = dashboardLoader( { params: ParamsSchema }, async ({ params: { userId, filename }, request }) => { - const objectPath = resolveUserAvatarObjectPath(userId, filename); + const objectPath = isAvatarUploadsEnabled() + ? resolveUserAvatarObjectPath(userId, filename) + : undefined; if (!objectPath) { throw new Response("Not found", { status: 404 }); diff --git a/apps/webapp/app/routes/resources.account.avatar.ts b/apps/webapp/app/routes/resources.account.avatar.ts index 36d436bdb64..6189ca78b23 100644 --- a/apps/webapp/app/routes/resources.account.avatar.ts +++ b/apps/webapp/app/routes/resources.account.avatar.ts @@ -5,6 +5,7 @@ import { dashboardAction } from "~/services/routeBuilders/dashboardBuilder"; import { deleteStaleUserAvatar, isAvatarUploadRejection, + isAvatarUploadsEnabled, parseAvatarUpload, uploadUserAvatar, } from "~/services/userAvatar.server"; @@ -20,6 +21,11 @@ export const action = dashboardAction({}, async ({ request, user }) => { return json({ error: "Method not allowed" }, { status: 405 }); } + // An install with no avatar store hides this UI entirely; a stray request still answers. + if (!isAvatarUploadsEnabled()) { + return json({ error: "Profile pictures are not available on this instance." }, { status: 400 }); + } + // Read from the cookie: the builder's session user reports isImpersonating false. const { isImpersonating } = await getImpersonationState(request, user.id); diff --git a/apps/webapp/app/services/userAvatar.server.ts b/apps/webapp/app/services/userAvatar.server.ts index 19a53185ae3..56fd6fd1f6f 100644 --- a/apps/webapp/app/services/userAvatar.server.ts +++ b/apps/webapp/app/services/userAvatar.server.ts @@ -17,6 +17,14 @@ const AVATAR_PRESIGN_EXPIRY_IN_SECONDS = 300; const AVATAR_FILENAME_REGEX = /^[0-9a-f]{32}\.(png|jpg|webp)$/; const USER_ID_REGEX = /^[A-Za-z0-9_-]+$/; +/** + * Whether this deployment can store profile pictures at all. Self-hosted installs that + * configure no avatar store keep the account page exactly as it was before the feature. + */ +export function isAvatarUploadsEnabled() { + return Boolean(env.AVATARS_OBJECT_STORE_BASE_URL && env.AVATARS_OBJECT_STORE_BUCKET); +} + /** Undefined when no avatar store is configured, so the policy stays unchanged. */ export function avatarObjectStoreImageOrigin() { return imageOriginFromUrl(env.AVATARS_OBJECT_STORE_BASE_URL); diff --git a/apps/webapp/test/userAvatar.test.ts b/apps/webapp/test/userAvatar.test.ts index 42f423b3636..30430ad2ebc 100644 --- a/apps/webapp/test/userAvatar.test.ts +++ b/apps/webapp/test/userAvatar.test.ts @@ -6,6 +6,7 @@ import { isAvatarUploadRejection, absoluteUserAvatarUrl, avatarObjectStoreImageOrigin, + isAvatarUploadsEnabled, parseAvatarUpload, presignUserAvatarUrl, resolveStaleAvatarObjectPath, @@ -373,3 +374,42 @@ describe("avatarContentTypeForFilename", () => { expect(avatarContentTypeForFilename(filename)).toBeUndefined(); }); }); + +describe("isAvatarUploadsEnabled", () => { + const original = { + baseUrl: env.AVATARS_OBJECT_STORE_BASE_URL, + bucket: env.AVATARS_OBJECT_STORE_BUCKET, + }; + + afterEach(() => { + env.AVATARS_OBJECT_STORE_BASE_URL = original.baseUrl; + env.AVATARS_OBJECT_STORE_BUCKET = original.bucket; + }); + + it("is on when the store is fully configured", () => { + env.AVATARS_OBJECT_STORE_BASE_URL = "http://localhost:9005"; + env.AVATARS_OBJECT_STORE_BUCKET = "avatars"; + + expect(isAvatarUploadsEnabled()).toBe(true); + }); + + it.each([ + ["nothing is configured", undefined, undefined], + ["only the base URL is set", "http://localhost:9005", undefined], + ["only the bucket is set", undefined, "avatars"], + ["the base URL is blank", "", "avatars"], + ["the bucket is blank", "http://localhost:9005", ""], + ])("is off when %s", (_case, baseUrl, bucket) => { + env.AVATARS_OBJECT_STORE_BASE_URL = baseUrl; + env.AVATARS_OBJECT_STORE_BUCKET = bucket; + + expect(isAvatarUploadsEnabled()).toBe(false); + }); + + it("never builds a client, so an unconfigured install can ask freely", () => { + env.AVATARS_OBJECT_STORE_BASE_URL = undefined; + env.AVATARS_OBJECT_STORE_BUCKET = undefined; + + expect(() => isAvatarUploadsEnabled()).not.toThrow(); + }); +}); From 11af3dc62cb13ff9811da93ffa6c9a51b5f92cfb Mon Sep 17 00:00:00 2001 From: Katia Bulatova Date: Fri, 28 Aug 2026 06:49:20 +0000 Subject: [PATCH 24/25] fix(webapp): reject protocol-relative avatar urls in the photo editor --- apps/webapp/app/routes/account._index/route.tsx | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/apps/webapp/app/routes/account._index/route.tsx b/apps/webapp/app/routes/account._index/route.tsx index f71b09dc20b..08a7feaa1c1 100644 --- a/apps/webapp/app/routes/account._index/route.tsx +++ b/apps/webapp/app/routes/account._index/route.tsx @@ -502,7 +502,11 @@ function ChangeProfilePhotoButton() { }; // Only our own uploads are app-relative; OAuth avatars are absolute URLs. - const uploadedAvatarUrl = user.avatarUrl?.startsWith("/") ? user.avatarUrl : undefined; + // "//host/path" is protocol-relative, so it would point off-origin. + const uploadedAvatarUrl = + user.avatarUrl?.startsWith("/") && !user.avatarUrl.startsWith("//") + ? user.avatarUrl + : undefined; const remove = () => { actionRef.current = "remove"; From c4b7ae4b817c72643df7b8f3d4740b98b5ff199d Mon Sep 17 00:00:00 2001 From: Katia Bulatova Date: Fri, 28 Aug 2026 06:53:44 +0000 Subject: [PATCH 25/25] fix(webapp): sign avatar store requests as s3 and ignore blank config --- apps/webapp/app/env.server.ts | 3 + apps/webapp/app/services/userAvatar.server.ts | 24 ++++--- apps/webapp/test/objectStore.test.ts | 62 ++++++++++++++++++- apps/webapp/test/userAvatar.test.ts | 22 +++++++ 4 files changed, 103 insertions(+), 8 deletions(-) diff --git a/apps/webapp/app/env.server.ts b/apps/webapp/app/env.server.ts index c7ceead1a65..eda5d56bec3 100644 --- a/apps/webapp/app/env.server.ts +++ b/apps/webapp/app/env.server.ts @@ -867,6 +867,9 @@ const EnvironmentSchema = z AVATARS_OBJECT_STORE_ACCESS_KEY_ID: z.string().optional(), AVATARS_OBJECT_STORE_SECRET_ACCESS_KEY: z.string().optional(), AVATARS_OBJECT_STORE_REGION: z.string().optional(), + // Signed as "s3" unless told otherwise, like the shared store: aws4fetch otherwise + // guesses the SigV4 service from the hostname and gets it wrong off amazonaws.com. + AVATARS_OBJECT_STORE_SERVICE: z.string().default("s3"), ARTIFACTS_OBJECT_STORE_BUCKET: z.string().optional(), ARTIFACTS_OBJECT_STORE_BASE_URL: z.string().optional(), diff --git a/apps/webapp/app/services/userAvatar.server.ts b/apps/webapp/app/services/userAvatar.server.ts index 56fd6fd1f6f..36eb00e0a0d 100644 --- a/apps/webapp/app/services/userAvatar.server.ts +++ b/apps/webapp/app/services/userAvatar.server.ts @@ -22,12 +22,22 @@ const USER_ID_REGEX = /^[A-Za-z0-9_-]+$/; * configure no avatar store keep the account page exactly as it was before the feature. */ export function isAvatarUploadsEnabled() { - return Boolean(env.AVATARS_OBJECT_STORE_BASE_URL && env.AVATARS_OBJECT_STORE_BUCKET); + const { baseUrl, bucket } = avatarObjectStoreSettings(); + + return Boolean(baseUrl && bucket); +} + +/** Trimmed at the read point: a whitespace-only value is unset, not a usable URL. */ +function avatarObjectStoreSettings() { + return { + baseUrl: env.AVATARS_OBJECT_STORE_BASE_URL?.trim(), + bucket: env.AVATARS_OBJECT_STORE_BUCKET?.trim(), + }; } /** Undefined when no avatar store is configured, so the policy stays unchanged. */ export function avatarObjectStoreImageOrigin() { - return imageOriginFromUrl(env.AVATARS_OBJECT_STORE_BASE_URL); + return imageOriginFromUrl(avatarObjectStoreSettings().baseUrl); } /** Keyed by config so a changed base URL builds a fresh client instead of reusing a stale one. */ @@ -41,8 +51,7 @@ const avatarObjectStoreClients = singleton( * bucket, as with `packets/…`. */ function requireAvatarObjectStore() { - const baseUrl = env.AVATARS_OBJECT_STORE_BASE_URL; - const bucket = env.AVATARS_OBJECT_STORE_BUCKET; + const { baseUrl, bucket } = avatarObjectStoreSettings(); if (!baseUrl) { throw new Error("AVATARS_OBJECT_STORE_BASE_URL is required to store avatars"); @@ -59,9 +68,10 @@ function requireAvatarObjectStore() { client = ObjectStoreClient.create({ baseUrl, bucket, - accessKeyId: env.AVATARS_OBJECT_STORE_ACCESS_KEY_ID || undefined, - secretAccessKey: env.AVATARS_OBJECT_STORE_SECRET_ACCESS_KEY || undefined, - region: env.AVATARS_OBJECT_STORE_REGION || undefined, + accessKeyId: env.AVATARS_OBJECT_STORE_ACCESS_KEY_ID?.trim() || undefined, + secretAccessKey: env.AVATARS_OBJECT_STORE_SECRET_ACCESS_KEY?.trim() || undefined, + region: env.AVATARS_OBJECT_STORE_REGION?.trim() || undefined, + service: env.AVATARS_OBJECT_STORE_SERVICE?.trim() || undefined, }); avatarObjectStoreClients.set(cacheKey, client); } diff --git a/apps/webapp/test/objectStore.test.ts b/apps/webapp/test/objectStore.test.ts index 617e6b08b9c..9bb584ea505 100644 --- a/apps/webapp/test/objectStore.test.ts +++ b/apps/webapp/test/objectStore.test.ts @@ -1,4 +1,4 @@ -import { postgresAndMinioTest } from "@internal/testcontainers"; +import { minioTest, postgresAndMinioTest } from "@internal/testcontainers"; import { type IOPacket } from "@trigger.dev/core/v3"; import { type PrismaClient } from "@trigger.dev/database"; import { afterAll, describe, expect, it, vi } from "vitest"; @@ -22,6 +22,11 @@ import { resolveStoreProtocolForPacketPresign, uploadPacketToObjectStore, } from "~/v3/objectStore.server"; +import { + presignUserAvatarUrl, + readUserAvatarBytes, + uploadUserAvatar, +} from "~/services/userAvatar.server"; // Extend the timeout for container tests vi.setConfig({ testTimeout: 60_000 }); @@ -838,3 +843,58 @@ describe("Object Storage", () => { env.TASK_PAYLOAD_OFFLOAD_THRESHOLD = originalEnvObj.TASK_PAYLOAD_OFFLOAD_THRESHOLD; }); }); + +const PNG_BYTES = new Uint8Array([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 1, 2, 3, 4]); + +/** + * The avatar store signs its own requests: aws4fetch guesses the SigV4 service from the + * hostname unless it is told, and guesses wrong for anything that is not amazonaws.com. + * Only a real S3-compatible host rejects that signature, so this has to run against MinIO. + */ +describe("the avatar object store against MinIO", () => { + const original = { + baseUrl: env.AVATARS_OBJECT_STORE_BASE_URL, + bucket: env.AVATARS_OBJECT_STORE_BUCKET, + accessKeyId: env.AVATARS_OBJECT_STORE_ACCESS_KEY_ID, + secretAccessKey: env.AVATARS_OBJECT_STORE_SECRET_ACCESS_KEY, + region: env.AVATARS_OBJECT_STORE_REGION, + }; + + afterAll(() => { + env.AVATARS_OBJECT_STORE_BASE_URL = original.baseUrl; + env.AVATARS_OBJECT_STORE_BUCKET = original.bucket; + env.AVATARS_OBJECT_STORE_ACCESS_KEY_ID = original.accessKeyId; + env.AVATARS_OBJECT_STORE_SECRET_ACCESS_KEY = original.secretAccessKey; + env.AVATARS_OBJECT_STORE_REGION = original.region; + }); + + minioTest("uploads, presigns and serves an avatar", async ({ minioConfig, minioContainer }) => { + await minioContainer.resetBucket("avatars"); + + env.AVATARS_OBJECT_STORE_BASE_URL = minioConfig.baseUrl; + env.AVATARS_OBJECT_STORE_BUCKET = "avatars"; + env.AVATARS_OBJECT_STORE_ACCESS_KEY_ID = minioConfig.accessKeyId; + env.AVATARS_OBJECT_STORE_SECRET_ACCESS_KEY = minioConfig.secretAccessKey; + env.AVATARS_OBJECT_STORE_REGION = minioConfig.region; + + const userId = `usr_${Date.now().toString(36)}`; + + const { avatarUrl, filename } = await uploadUserAvatar({ + userId, + contentType: "image/png", + data: PNG_BYTES, + }); + + expect(avatarUrl).toBe(`/resources/account/avatar/${userId}/${filename}`); + + const objectPath = `avatars/${userId}/${filename}`; + + const presigned = await presignUserAvatarUrl(objectPath); + const response = await fetch(presigned); + + expect(response.status).toBe(200); + expect(new Uint8Array(await response.arrayBuffer())).toEqual(PNG_BYTES); + + expect(await readUserAvatarBytes(objectPath)).toEqual(PNG_BYTES); + }); +}); diff --git a/apps/webapp/test/userAvatar.test.ts b/apps/webapp/test/userAvatar.test.ts index 30430ad2ebc..17f37f0ef2d 100644 --- a/apps/webapp/test/userAvatar.test.ts +++ b/apps/webapp/test/userAvatar.test.ts @@ -267,6 +267,25 @@ describe("the avatar object store", () => { ); }); + it("treats a whitespace-only base URL as unset", () => { + setAvatarEnv({ AVATARS_OBJECT_STORE_BASE_URL: " ", AVATARS_OBJECT_STORE_BUCKET: "avatars" }); + + expect(() => presignUserAvatarUrl(`avatars/${USER_ID}/a.png`)).toThrow( + /AVATARS_OBJECT_STORE_BASE_URL/ + ); + }); + + it("treats a whitespace-only bucket as unset", () => { + setAvatarEnv({ + AVATARS_OBJECT_STORE_BASE_URL: "https://avatars-blank-bucket.test", + AVATARS_OBJECT_STORE_BUCKET: " ", + }); + + expect(() => presignUserAvatarUrl(`avatars/${USER_ID}/a.png`)).toThrow( + /AVATARS_OBJECT_STORE_BUCKET/ + ); + }); + it("requires its own bucket", () => { setAvatarEnv({ AVATARS_OBJECT_STORE_BASE_URL: "https://avatars-no-bucket.test", @@ -399,6 +418,9 @@ describe("isAvatarUploadsEnabled", () => { ["only the bucket is set", undefined, "avatars"], ["the base URL is blank", "", "avatars"], ["the bucket is blank", "http://localhost:9005", ""], + ["the base URL is only whitespace", " ", "avatars"], + ["the bucket is only whitespace", "http://localhost:9005", " "], + ["both are only whitespace", " ", "\t"], ])("is off when %s", (_case, baseUrl, bucket) => { env.AVATARS_OBJECT_STORE_BASE_URL = baseUrl; env.AVATARS_OBJECT_STORE_BUCKET = bucket;