From f8e35afecc67dd8a0429eff8190a9d02177edfab Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 19 Jul 2026 08:52:01 -0700 Subject: [PATCH] feat: avatar upload UI, self-service profiles, server-side avatar route (#111421) * feat: avatar upload UI, self-service profiles, server-side avatar proxy * refactor(ui): make resolveAvatar synchronous so avatar updates render immediately resolveAvatar no longer does async work (gravatar moved server-side), so the until()-deferred render left the identity chip img stale after updateSelfUser. Rendering synchronously reuses the img element and reflects avatarUrl changes on the next render. * fix(gateway): bound avatar Gravatar lookup to one timeout budget Linked-email Gravatar lookups ran sequentially, each with its own 5s timeout, so an upstream outage stalled the held connection for 5s x linked-email-count. Resolve them concurrently and pick the first hit in email order, keeping primary-email precedence while capping the request to a single timeout budget. * fix(gateway): cap per-profile Gravatar fan-out to bound sockets and memory A profile with many linked emails would fan out one concurrent fetch per email with no bound, each able to buffer up to MAX_GRAVATAR_BYTES before the cache budget applies. Cap the lookups to the first 8 primary-ordered emails so a single avatar request holds a bounded number of sockets and transient bytes. * test: reconcile avatar tests with server-side gravatar model after merge main landed a client-side Gravatar path (browser computes the gravatar.com URL); this branch resolves avatars through the same-origin gateway route, which is the only approach that works under the Control UI CSP (img-src 'self'). Update the identity-section, app-sidebar footer chip, and profile-page e2e expectations to assert the canonical /api/users//avatar route (gateway serves the Gravatar fallback behind it) instead of a CSP-blocked direct gravatar.com image. * test(gateway): fix fetchImpl mock param type for check-test-types The Gravatar mock typed its param as (url: string), narrower than the fetchImpl signature (URL | RequestInfo); function-param contravariance made it unassignable under tsgo:test. Widen to the fetch input type and extract the URL via a small helper that avoids no-base-to-string on a Request. * fix(gateway): short-circuit Gravatar lookups and version presence avatar URL Two autoreview findings on the avatar path: - Privacy: the concurrent Promise.all fan-out queried every linked email's hash against Gravatar even when the primary already had one, exposing secondary work/personal addresses. Resolve sequentially with short-circuit so a later hash is disclosed only after the earlier email is a definite miss, under one shared request deadline that still bounds total latency. - Staleness: presence published an unversioned /api/users//avatar, so a reconnecting viewer's kept the stale cached image after an upload. Carry the profile revision as ?v= so a changed avatar refetches. --- CHANGELOG.md | 1 + src/gateway/server-methods/shared-types.ts | 1 + src/gateway/server-methods/users.test.ts | 21 + .../server/ws-connection/connect-session.ts | 13 +- ...essage-handler.post-connect-health.test.ts | 10 +- src/gateway/server/ws-types.ts | 1 + src/gateway/user-profiles-http-path.ts | 4 + src/gateway/user-profiles-http.test.ts | 342 +++++++++++++++- src/gateway/user-profiles-http.ts | 370 +++++++++++++++++- ui/src/app/gateway-store.ts | 4 + ui/src/components/viewer-facepile.test.ts | 134 ++----- ui/src/components/viewer-facepile.ts | 157 ++------ ui/src/e2e/profile-page.e2e.test.ts | 24 +- ui/src/i18n/locales/en.ts | 12 + ui/src/lib/identity-avatar.test.ts | 129 ++++-- ui/src/lib/identity-avatar.ts | 139 ++++--- ui/src/pages/chat/chat-pane.ts | 1 + ui/src/pages/chat/chat-view.ts | 2 + .../chat/components/chat-author-avatar.ts | 19 +- .../chat/components/chat-message.test.ts | 13 +- ui/src/pages/chat/components/chat-thread.ts | 2 + .../pages/profile/avatar-processing.test.ts | 6 +- ui/src/pages/profile/avatar-processing.ts | 34 +- ui/src/pages/profile/identity-section.test.ts | 29 +- ui/src/pages/profile/profile-page.ts | 14 +- ui/src/styles/layout.css | 12 + .../test-helpers/app-sidebar-cases/basics.ts | 21 +- 27 files changed, 1106 insertions(+), 409 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b0dc20f7174..cd4db4dc6d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ Docs: https://docs.openclaw.ai ### Changes - **Discord and Slack native login:** register `/login` in native command menus while keeping pairing-code issuance limited to private chats and the Web UI. +- **Control UI user profiles:** let trusted-proxy users manage their own display name and avatar, resolve attributed chat and presence identities through uploaded avatars or a private cached Gravatar proxy, and keep other users' profiles admin-only. - **Trusted-proxy browser pairing:** optionally auto-approve new Control UI and WebChat devices from allowlisted proxy identities with non-admin scope caps, while keeping existing-device upgrades manual. - **Channel plugin ingress monitors:** add a shared plugin SDK monitor for durable admission, polling, pruning, claim identity validation, adoption handoff, and shutdown, and migrate IRC, Synology Chat, and Google Chat to the shared lifecycle. - **External gateway supervision:** add `OPENCLAW_SUPERVISOR_MODE=external` for lifecycle owners such as OCM, preserving verified restart and deferral behavior without exposing native service authority, blocking native service mutation and self-update, and providing a versioned atomic restart-handoff consume contract. Thanks @shakkernerd. diff --git a/src/gateway/server-methods/shared-types.ts b/src/gateway/server-methods/shared-types.ts index b1f991bda4c..6b365f2fa99 100644 --- a/src/gateway/server-methods/shared-types.ts +++ b/src/gateway/server-methods/shared-types.ts @@ -74,6 +74,7 @@ export type GatewayClient = { profileId: string; displayName: string | null; hasAvatar: boolean; + updatedAt: number; }; pluginSurfaceUrls?: Record; pluginNodeCapabilitySurfaces?: Record; diff --git a/src/gateway/server-methods/users.test.ts b/src/gateway/server-methods/users.test.ts index c9fc9cf9971..59840ee9579 100644 --- a/src/gateway/server-methods/users.test.ts +++ b/src/gateway/server-methods/users.test.ts @@ -215,6 +215,27 @@ describe("users gateway methods", () => { expect(ensureProfileForEmail).toHaveBeenCalledWith("ada@example.com"); }); + it("denies an identified write caller changing another profile's avatar", async () => { + ensureProfileForEmail.mockReturnValue(profile); + resolveUserProfileId.mockReturnValue("profile-2"); + + expect( + await runUsersHandler( + "users.setAvatar", + { profileId: "profile-2", mime: "image/png", avatarBase64: "AQ==" }, + selfClient, + ), + ).toHaveBeenCalledWith( + false, + undefined, + expect.objectContaining({ + code: "FORBIDDEN", + message: "profile edits require the owning user or operator.admin", + }), + ); + expect(setAvatar).not.toHaveBeenCalled(); + }); + it("allows an owner to edit through a tombstoned durable profile id", async () => { ensureProfileForEmail.mockReturnValue(profile); resolveUserProfileId.mockReturnValue(profile.id); diff --git a/src/gateway/server/ws-connection/connect-session.ts b/src/gateway/server/ws-connection/connect-session.ts index 479874e160f..19b95536bb1 100644 --- a/src/gateway/server/ws-connection/connect-session.ts +++ b/src/gateway/server/ws-connection/connect-session.ts @@ -32,6 +32,7 @@ import { type PluginNodeCapabilitySurface, } from "../../plugin-node-capability.js"; import { MAX_PAYLOAD_BYTES } from "../../server-constants.js"; +import { formatUserProfileAvatarPath } from "../../user-profiles-http-path.js"; import { formatForLog, logWs } from "../../ws-log.js"; import { truncateCloseReason } from "../close-reason.js"; import { incrementPresenceVersion } from "../health-state.js"; @@ -149,6 +150,7 @@ export async function attachAuthenticatedGatewayConnect( profileId: profile.id, displayName: profile.displayName, hasAvatar: profile.avatarMime !== null, + updatedAt: profile.updatedAt, }; } catch (error) { // Profile storage must not block login; retain the legacy email-only identity on failure. @@ -359,11 +361,12 @@ export async function attachAuthenticatedGatewayConnect( ...(authenticatedUserProfile.displayName ? { name: authenticatedUserProfile.displayName } : {}), - ...(authenticatedUserProfile.hasAvatar - ? { - avatarUrl: `/api/users/${authenticatedUserProfile.profileId}/avatar`, - } - : {}), + // This authenticated route resolves the uploaded avatar first, then the + // gateway-side Gravatar proxy, so clients never need an email-hash URL. + // The ?v= revision changes when the profile (avatar) is + // updated, so a reconnecting viewer's refetches instead of reusing + // a stale cached image for the unchanged route. + avatarUrl: `${formatUserProfileAvatarPath(authenticatedUserProfile.profileId)}?v=${authenticatedUserProfile.updatedAt}`, } : { id: authenticatedUserId, email: authenticatedUserId }, } diff --git a/src/gateway/server/ws-connection/message-handler.post-connect-health.test.ts b/src/gateway/server/ws-connection/message-handler.post-connect-health.test.ts index dc1ec248a79..e02a418df51 100644 --- a/src/gateway/server/ws-connection/message-handler.post-connect-health.test.ts +++ b/src/gateway/server/ws-connection/message-handler.post-connect-health.test.ts @@ -617,6 +617,12 @@ describe("attachGatewayWsMessageHandler post-connect health refresh", () => { id: profileId, email: "alice@example.com", name: "alice", + // Published route carries the profile revision (?v=) so a + // reconnecting viewer's refetches after an avatar upload instead + // of reusing the stale cached image for an unchanged URL. + avatarUrl: expect.stringMatching( + new RegExp(`^/api/users/${profileId}/avatar\\?v=\\d+$`, "u"), + ), }); expect(first.harness.client).toMatchObject({ authenticatedUserId: "alice@example.com", @@ -633,7 +639,9 @@ describe("attachGatewayWsMessageHandler post-connect health refresh", () => { id: profileId, email: "alice@example.com", name: "alice", - avatarUrl: `/api/users/${profileId}/avatar`, + avatarUrl: expect.stringMatching( + new RegExp(`^/api/users/${profileId}/avatar\\?v=\\d+$`, "u"), + ), }); expect(second.harness.client).toMatchObject({ authenticatedUserProfile: { profileId, hasAvatar: true }, diff --git a/src/gateway/server/ws-types.ts b/src/gateway/server/ws-types.ts index c3f2df4c9de..679fbff6f50 100644 --- a/src/gateway/server/ws-types.ts +++ b/src/gateway/server/ws-types.ts @@ -37,6 +37,7 @@ export type GatewayWsClient = PluginNodeCapabilityClient & { profileId: string; displayName: string | null; hasAvatar: boolean; + updatedAt: number; }; clientIp?: string; internal?: { diff --git a/src/gateway/user-profiles-http-path.ts b/src/gateway/user-profiles-http-path.ts index b34c97a1bd7..423db97d991 100644 --- a/src/gateway/user-profiles-http-path.ts +++ b/src/gateway/user-profiles-http-path.ts @@ -1,5 +1,9 @@ const USER_PROFILE_AVATAR_PATH = /^\/api\/users\/([^/]+)\/avatar$/u; +export function formatUserProfileAvatarPath(profileId: string): string { + return `/api/users/${encodeURIComponent(profileId)}/avatar`; +} + export function matchUserProfileAvatarPath(pathname: string): string | undefined { const profileId = USER_PROFILE_AVATAR_PATH.exec(pathname)?.[1]; if (!profileId) { diff --git a/src/gateway/user-profiles-http.test.ts b/src/gateway/user-profiles-http.test.ts index 25060b52ede..9ece2632f9c 100644 --- a/src/gateway/user-profiles-http.test.ts +++ b/src/gateway/user-profiles-http.test.ts @@ -1,20 +1,34 @@ +import { createHash } from "node:crypto"; import type { IncomingMessage, ServerResponse } from "node:http"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { handleUserProfileAvatarHttpRequest } from "./user-profiles-http.js"; const authorizeScopedGatewayHttpRequestOrReply = vi.hoisted(() => vi.fn()); +const getRuntimeConfig = vi.hoisted(() => vi.fn()); const getProfileAvatar = vi.hoisted(() => vi.fn()); +const getUserProfileListItem = vi.hoisted(() => vi.fn()); vi.mock("./http-utils.js", async (importOriginal) => ({ ...(await importOriginal()), authorizeScopedGatewayHttpRequestOrReply, })); +vi.mock("../config/io.js", () => ({ getRuntimeConfig })); vi.mock("../state/user-profiles.js", () => ({ formatUserProfileAvatarEtag: (sha256: string, mime: string) => `"${sha256}-${mime.slice("image/".length)}"`, getProfileAvatar, + getUserProfileListItem, + UserProfileNotFoundError: class UserProfileNotFoundError extends Error {}, })); +function emailHash(email: string): string { + return createHash("sha256").update(email.trim().toLowerCase()).digest("hex"); +} + +function fetchUrl(input: URL | RequestInfo): string { + return typeof input === "string" ? input : input instanceof URL ? input.href : input.url; +} + function response() { const end = vi.fn(); const setHeader = vi.fn(); @@ -22,6 +36,7 @@ function response() { return { end, response: { end, setHeader, writeHead } as unknown as ServerResponse, + setHeader, writeHead, }; } @@ -34,7 +49,34 @@ describe("profile avatar HTTP endpoint", () => { beforeEach(() => { authorizeScopedGatewayHttpRequestOrReply.mockReset(); getProfileAvatar.mockReset(); + getUserProfileListItem.mockReset(); + getRuntimeConfig.mockReset(); authorizeScopedGatewayHttpRequestOrReply.mockResolvedValue({}); + getRuntimeConfig.mockReturnValue({ + gateway: { controlUi: { allowedOrigins: ["https://control.example"] } }, + }); + }); + + it("answers allowed credentialed cross-origin preflights without avatar auth", async () => { + const res = response(); + const req = { + method: "OPTIONS", + url: "/ignored-by-handler", + headers: { origin: "https://control.example" }, + } as unknown as IncomingMessage; + + await handleUserProfileAvatarHttpRequest(req, res.response, "/api/users/profile-1/avatar", { + auth: {} as never, + }); + + expect(authorizeScopedGatewayHttpRequestOrReply).not.toHaveBeenCalled(); + expect(res.setHeader).toHaveBeenCalledWith( + "Access-Control-Allow-Origin", + "https://control.example", + ); + expect(res.setHeader).toHaveBeenCalledWith("Access-Control-Allow-Credentials", "true"); + expect(res.setHeader).toHaveBeenCalledWith("Access-Control-Allow-Headers", "Authorization"); + expect(res.writeHead).toHaveBeenCalledWith(204); }); it("serves avatars with their stored MIME type and representation ETag", async () => { @@ -79,7 +121,10 @@ describe("profile avatar HTTP endpoint", () => { { auth: {} as never }, ); - expect(res.writeHead).toHaveBeenCalledWith(304, { ETag: '"current-hash-png"' }); + expect(res.writeHead).toHaveBeenCalledWith(304, { + ETag: '"current-hash-png"', + "Cache-Control": "private, max-age=0, must-revalidate", + }); expect(res.end).toHaveBeenCalledWith(); }); @@ -142,7 +187,300 @@ describe("profile avatar HTTP endpoint", () => { { auth: {} as never }, ); - expect(res.writeHead).toHaveBeenCalledWith(304, { ETag: '"current-hash-png"' }); + expect(res.writeHead).toHaveBeenCalledWith(304, { + ETag: '"current-hash-png"', + "Cache-Control": "private, max-age=0, must-revalidate", + }); }, ); + + it("proxies and caches Gravatar by a profile's normalized email", async () => { + const profileId = "profile-gravatar-cache"; + const hash = emailHash(" Ada@Example.com "); + getProfileAvatar.mockReturnValue(undefined); + getUserProfileListItem.mockReturnValue({ + id: profileId, + emails: [" Ada@Example.com "], + hasAvatar: false, + }); + const fetchImpl = vi.fn().mockResolvedValue( + new Response(new Uint8Array([4, 5, 6]), { + status: 200, + headers: { "content-type": "image/png" }, + }), + ); + + const first = response(); + const second = response(); + await handleUserProfileAvatarHttpRequest( + request("/ignored-by-handler"), + first.response, + `/api/users/${profileId}/avatar`, + { auth: {} as never, fetchImpl }, + ); + await handleUserProfileAvatarHttpRequest( + request("/ignored-by-handler"), + second.response, + `/api/users/${profileId}/avatar`, + { auth: {} as never, fetchImpl }, + ); + + expect(fetchImpl).toHaveBeenCalledTimes(1); + expect(fetchImpl).toHaveBeenCalledWith( + `https://www.gravatar.com/avatar/${hash}?s=256&d=404`, + expect.objectContaining({ + headers: { Accept: "image/webp,image/png,image/jpeg,image/gif" }, + signal: expect.any(AbortSignal), + }), + ); + expect(first.writeHead).toHaveBeenCalledWith( + 200, + expect.objectContaining({ + "Content-Type": "image/png", + "Cache-Control": "private, max-age=0, must-revalidate", + }), + ); + expect(first.end).toHaveBeenCalledWith(new Uint8Array([4, 5, 6])); + expect(second.end).toHaveBeenCalledWith(new Uint8Array([4, 5, 6])); + }); + + it("negative-caches a Gravatar 404 so the UI can fall back to initials", async () => { + const profileId = "profile-gravatar-miss"; + getProfileAvatar.mockReturnValue(undefined); + getUserProfileListItem.mockReturnValue({ + id: profileId, + emails: ["missing-avatar@example.com"], + hasAvatar: false, + }); + const fetchImpl = vi.fn().mockResolvedValue(new Response(null, { status: 404 })); + + const first = response(); + const second = response(); + await handleUserProfileAvatarHttpRequest( + request("/ignored-by-handler"), + first.response, + `/api/users/${profileId}/avatar`, + { auth: {} as never, fetchImpl }, + ); + await handleUserProfileAvatarHttpRequest( + request("/ignored-by-handler"), + second.response, + `/api/users/${profileId}/avatar`, + { auth: {} as never, fetchImpl }, + ); + + expect(fetchImpl).toHaveBeenCalledTimes(1); + expect(first.response.statusCode).toBe(404); + expect(first.setHeader).toHaveBeenCalledWith("Content-Type", "application/json; charset=utf-8"); + // A cached 404 would hide a later uploaded avatar behind the stable route. + expect(first.setHeader).toHaveBeenCalledWith("Cache-Control", "no-store"); + expect(second.response.statusCode).toBe(404); + }); + + it("serves the primary email's Gravatar when several linked emails resolve", async () => { + const profileId = "profile-multi-email-primary"; + const primaryHash = emailHash("primary@example.com"); + getProfileAvatar.mockReturnValue(undefined); + getUserProfileListItem.mockReturnValue({ + id: profileId, + emails: ["primary@example.com", "secondary@example.com"], + hasAvatar: false, + }); + const secondaryHash = emailHash("secondary@example.com"); + // The primary email has a Gravatar, so its lookup short-circuits — the + // secondary email's hash must never be disclosed to Gravatar. + const fetchImpl = vi.fn(async (input: URL | RequestInfo) => + fetchUrl(input).includes(primaryHash) + ? new Response(new Uint8Array([1, 1, 1]), { + status: 200, + headers: { "content-type": "image/png" }, + }) + : new Response(new Uint8Array([2, 2, 2]), { + status: 200, + headers: { "content-type": "image/png" }, + }), + ); + const res = response(); + + await handleUserProfileAvatarHttpRequest( + request("/ignored-by-handler"), + res.response, + `/api/users/${profileId}/avatar`, + { auth: {} as never, fetchImpl }, + ); + + expect(res.end).toHaveBeenCalledWith(new Uint8Array([1, 1, 1])); + expect(fetchImpl).toHaveBeenCalledTimes(1); + expect(fetchImpl).not.toHaveBeenCalledWith( + expect.stringContaining(secondaryHash), + expect.anything(), + ); + }); + + it("falls through to a later linked email when the primary has no Gravatar", async () => { + const profileId = "profile-multi-email-fallthrough"; + const primaryHash = emailHash("primary-miss@example.com"); + getProfileAvatar.mockReturnValue(undefined); + getUserProfileListItem.mockReturnValue({ + id: profileId, + emails: ["primary-miss@example.com", "secondary-hit@example.com"], + hasAvatar: false, + }); + const fetchImpl = vi.fn(async (input: URL | RequestInfo) => + fetchUrl(input).includes(primaryHash) + ? new Response(null, { status: 404 }) + : new Response(new Uint8Array([2, 2, 2]), { + status: 200, + headers: { "content-type": "image/png" }, + }), + ); + const res = response(); + + await handleUserProfileAvatarHttpRequest( + request("/ignored-by-handler"), + res.response, + `/api/users/${profileId}/avatar`, + { auth: {} as never, fetchImpl }, + ); + + // A definite miss on the primary lets the request fall through to the + // secondary email under the shared deadline; the secondary hit is served. + expect(fetchImpl).toHaveBeenCalledTimes(2); + expect(res.end).toHaveBeenCalledWith(new Uint8Array([2, 2, 2])); + }); + + it("caps the Gravatar fan-out so a profile with many linked emails is bounded", async () => { + const profileId = "profile-many-emails"; + const emails = Array.from({ length: 12 }, (_, index) => `many-${index}@example.com`); + // Only the last email — beyond the fan-out cap — has a Gravatar. + const reachableHash = emailHash(emails[emails.length - 1] ?? ""); + getProfileAvatar.mockReturnValue(undefined); + getUserProfileListItem.mockReturnValue({ id: profileId, emails, hasAvatar: false }); + const fetchImpl = vi.fn(async (input: URL | RequestInfo) => + fetchUrl(input).includes(reachableHash) + ? new Response(new Uint8Array([9, 9, 9]), { + status: 200, + headers: { "content-type": "image/png" }, + }) + : new Response(null, { status: 404 }), + ); + const res = response(); + + await handleUserProfileAvatarHttpRequest( + request("/ignored-by-handler"), + res.response, + `/api/users/${profileId}/avatar`, + { auth: {} as never, fetchImpl }, + ); + + // Only the first 8 emails are looked up, so the request never fans out to + // all 12 and the beyond-cap avatar stays unreachable (404 fallback). + expect(fetchImpl).toHaveBeenCalledTimes(8); + expect(res.response.statusCode).toBe(404); + }); + + it("cancels a chunked Gravatar response as soon as it exceeds the byte cap", async () => { + const profileId = "profile-gravatar-oversized"; + getProfileAvatar.mockReturnValue(undefined); + getUserProfileListItem.mockReturnValue({ + id: profileId, + emails: ["oversized-avatar@example.com"], + hasAvatar: false, + }); + const cancel = vi.fn(); + const body = new ReadableStream({ + start(controller) { + controller.enqueue(new Uint8Array(600_000)); + controller.enqueue(new Uint8Array(600_000)); + }, + cancel, + }); + const fetchImpl = vi.fn().mockResolvedValue( + new Response(body, { + status: 200, + headers: { "content-type": "image/png" }, + }), + ); + const res = response(); + + await handleUserProfileAvatarHttpRequest( + request("/ignored-by-handler"), + res.response, + `/api/users/${profileId}/avatar`, + { auth: {} as never, fetchImpl }, + ); + + expect(cancel).toHaveBeenCalledTimes(1); + expect(res.response.statusCode).toBe(502); + }); + + it("cancels a Gravatar response rejected by its declared byte size", async () => { + const profileId = "profile-gravatar-declared-oversized"; + getProfileAvatar.mockReturnValue(undefined); + getUserProfileListItem.mockReturnValue({ + id: profileId, + emails: ["declared-oversized-avatar@example.com"], + hasAvatar: false, + }); + const cancel = vi.fn(); + const body = new ReadableStream({ cancel }); + const fetchImpl = vi.fn().mockResolvedValue( + new Response(body, { + status: 200, + headers: { + "content-length": "1000001", + "content-type": "image/png", + }, + }), + ); + const res = response(); + + await handleUserProfileAvatarHttpRequest( + request("/ignored-by-handler"), + res.response, + `/api/users/${profileId}/avatar`, + { auth: {} as never, fetchImpl }, + ); + + expect(cancel).toHaveBeenCalledTimes(1); + expect(res.response.statusCode).toBe(502); + }); + + it("evicts older Gravatar images when the cache reaches its byte budget", async () => { + getProfileAvatar.mockReturnValue(undefined); + const imageBytes = new Uint8Array(1_000_000); + const fetchImpl = vi.fn( + async () => + new Response(imageBytes.slice(), { + status: 200, + headers: { "content-type": "image/png" }, + }), + ); + const emails = Array.from({ length: 17 }, (_, index) => `cache-${index}@example.com`); + const profiles = emails.map((email, index) => ({ + id: `profile-cache-${index}`, + emails: [email], + hasAvatar: false, + })); + getUserProfileListItem.mockImplementation((profileId: string) => + profiles.find((profile) => profile.id === profileId), + ); + + for (const profile of profiles) { + await handleUserProfileAvatarHttpRequest( + request("/ignored-by-handler"), + response().response, + `/api/users/${profile.id}/avatar`, + { auth: {} as never, fetchImpl }, + ); + } + await handleUserProfileAvatarHttpRequest( + request("/ignored-by-handler"), + response().response, + `/api/users/${profiles[0]?.id}/avatar`, + { auth: {} as never, fetchImpl }, + ); + + expect(fetchImpl).toHaveBeenCalledTimes(18); + }); }); diff --git a/src/gateway/user-profiles-http.ts b/src/gateway/user-profiles-http.ts index f1cf68ccdb2..95f4d730244 100644 --- a/src/gateway/user-profiles-http.ts +++ b/src/gateway/user-profiles-http.ts @@ -1,6 +1,14 @@ -// Authenticated HTTP avatar serving for durable user profiles. +// Authenticated HTTP avatar serving and Gravatar proxying for durable user profiles. +import { createHash } from "node:crypto"; import type { IncomingMessage, ServerResponse } from "node:http"; -import { formatUserProfileAvatarEtag, getProfileAvatar } from "../state/user-profiles.js"; +import { getRuntimeConfig } from "../config/io.js"; +import type { OpenClawConfig } from "../config/types.openclaw.js"; +import { + formatUserProfileAvatarEtag, + getProfileAvatar, + getUserProfileListItem, + UserProfileNotFoundError, +} from "../state/user-profiles.js"; import type { AuthRateLimiter } from "./auth-rate-limit.js"; import type { ResolvedGatewayAuth } from "./auth.js"; import { sendJson, sendMethodNotAllowed } from "./http-common.js"; @@ -10,6 +18,271 @@ import { } from "./http-utils.js"; import { matchUserProfileAvatarPath } from "./user-profiles-http-path.js"; +const GRAVATAR_BASE_URL = "https://www.gravatar.com/avatar"; +const GRAVATAR_FETCH_TIMEOUT_MS = 5_000; +// Whole-request budget shared across a profile's linked emails. Lookups run +// sequentially (see the resolution loop) so a secondary email's hash is only +// disclosed to Gravatar after the earlier one is a definite miss; this deadline +// bounds the total wait so an unreachable Gravatar cannot stall the held +// connection by GRAVATAR_FETCH_TIMEOUT_MS × linked-email-count. +const GRAVATAR_TOTAL_TIMEOUT_MS = 6_000; +const GRAVATAR_CACHE_MAX_ENTRIES = 256; +const GRAVATAR_CACHE_MAX_BYTES = 16 * 1024 * 1024; +const GRAVATAR_HIT_TTL_MS = 24 * 60 * 60_000; +const GRAVATAR_MISS_TTL_MS = 15 * 60_000; +const MAX_GRAVATAR_BYTES = 1_000_000; +// Bound the Gravatar fan-out per avatar request. Linked emails are primary-first +// and resolved sequentially with short-circuit, so the cap only matters when +// every earlier email misses; it stops a profile with many linked addresses from +// probing an unbounded number of them against Gravatar. +const MAX_GRAVATAR_EMAIL_LOOKUPS = 8; +const GRAVATAR_MIME_TYPES = new Set(["image/gif", "image/jpeg", "image/png", "image/webp"]); + +function resolveAvatarCorsOrigin(req: IncomingMessage, cfg: OpenClawConfig): string | undefined { + const rawOrigin = typeof req.headers.origin === "string" ? req.headers.origin.trim() : ""; + if (!rawOrigin) { + return undefined; + } + let origin: string; + try { + const parsed = new URL(rawOrigin); + if (parsed.origin !== rawOrigin || parsed.username || parsed.password) { + return undefined; + } + origin = parsed.origin; + } catch { + return undefined; + } + const allowed = cfg.gateway?.controlUi?.allowedOrigins ?? []; + return allowed.some((candidate) => candidate.trim() === "*" || candidate.trim() === origin) + ? origin + : undefined; +} + +function setAvatarCorsHeaders( + req: IncomingMessage, + res: ServerResponse, + cfg: OpenClawConfig, +): boolean { + if (!req.headers.origin) { + return true; + } + const origin = resolveAvatarCorsOrigin(req, cfg); + if (!origin) { + return false; + } + res.setHeader("Access-Control-Allow-Origin", origin); + res.setHeader("Access-Control-Allow-Credentials", "true"); + res.setHeader("Vary", "Origin"); + return true; +} + +type GravatarHit = { + kind: "hit"; + bytes: Uint8Array; + mime: string; + etag: string; +}; + +type GravatarResult = GravatarHit | { kind: "miss" } | { kind: "error" }; +type CachedGravatarResult = Exclude & { expiresAtMs: number }; + +const gravatarCache = new Map(); +const gravatarRequests = new Map>(); +let gravatarCacheBytes = 0; + +function deleteCachedGravatar(hash: string): void { + const cached = gravatarCache.get(hash); + if (cached?.kind === "hit") { + gravatarCacheBytes -= cached.bytes.byteLength; + } + gravatarCache.delete(hash); +} + +function hashEmail(email: string): string { + return createHash("sha256").update(email.trim().toLowerCase()).digest("hex"); +} + +function getCachedGravatar(hash: string, nowMs: number): GravatarResult | undefined { + const cached = gravatarCache.get(hash); + if (!cached) { + return undefined; + } + if (cached.expiresAtMs <= nowMs) { + deleteCachedGravatar(hash); + return undefined; + } + // Map insertion order is the LRU order. Promote on every hit. + deleteCachedGravatar(hash); + gravatarCache.set(hash, cached); + if (cached.kind === "hit") { + gravatarCacheBytes += cached.bytes.byteLength; + } + return cached.kind === "hit" + ? { kind: "hit", bytes: cached.bytes, mime: cached.mime, etag: cached.etag } + : { kind: "miss" }; +} + +function cacheGravatar( + hash: string, + result: Exclude, + nowMs: number, +) { + const ttlMs = result.kind === "hit" ? GRAVATAR_HIT_TTL_MS : GRAVATAR_MISS_TTL_MS; + deleteCachedGravatar(hash); + const cached = { ...result, expiresAtMs: nowMs + ttlMs } satisfies CachedGravatarResult; + gravatarCache.set(hash, cached); + if (cached.kind === "hit") { + gravatarCacheBytes += cached.bytes.byteLength; + } + while ( + gravatarCache.size > GRAVATAR_CACHE_MAX_ENTRIES || + gravatarCacheBytes > GRAVATAR_CACHE_MAX_BYTES + ) { + const oldest = gravatarCache.keys().next().value; + if (oldest === undefined) { + break; + } + deleteCachedGravatar(oldest); + } +} + +function normalizeContentType(value: string | null): string { + return value?.split(";", 1)[0]?.trim().toLowerCase() ?? ""; +} + +async function readBoundedGravatarBody( + body: ReadableStream | null, +): Promise { + if (!body) { + return undefined; + } + const reader = body.getReader(); + const chunks: Uint8Array[] = []; + let totalBytes = 0; + try { + while (true) { + const next = await reader.read(); + if (next.done) { + break; + } + totalBytes += next.value.byteLength; + if (totalBytes > MAX_GRAVATAR_BYTES) { + await reader.cancel(); + return undefined; + } + chunks.push(next.value); + } + } finally { + reader.releaseLock(); + } + if (totalBytes === 0) { + return undefined; + } + const bytes = new Uint8Array(totalBytes); + let offset = 0; + for (const chunk of chunks) { + bytes.set(chunk, offset); + offset += chunk.byteLength; + } + return bytes; +} + +async function cancelGravatarBody(body: ReadableStream | null): Promise { + try { + await body?.cancel(); + } catch { + // The response is already unusable; cancellation is only best-effort cleanup. + } +} + +async function fetchGravatar( + hash: string, + fetchImpl: typeof globalThis.fetch, + deadline?: AbortSignal, +): Promise { + try { + const perCall = AbortSignal.timeout(GRAVATAR_FETCH_TIMEOUT_MS); + const response = await fetchImpl(`${GRAVATAR_BASE_URL}/${hash}?s=256&d=404`, { + headers: { Accept: "image/webp,image/png,image/jpeg,image/gif" }, + signal: deadline ? AbortSignal.any([deadline, perCall]) : perCall, + }); + if (response.status === 404) { + await cancelGravatarBody(response.body); + return { kind: "miss" }; + } + if (!response.ok) { + await cancelGravatarBody(response.body); + return { kind: "error" }; + } + const mime = normalizeContentType(response.headers.get("content-type")); + const declaredLength = Number(response.headers.get("content-length")); + if ( + !GRAVATAR_MIME_TYPES.has(mime) || + (Number.isFinite(declaredLength) && declaredLength > MAX_GRAVATAR_BYTES) + ) { + await cancelGravatarBody(response.body); + return { kind: "error" }; + } + const bytes = await readBoundedGravatarBody(response.body); + if (!bytes) { + return { kind: "error" }; + } + const etag = `"gravatar-${createHash("sha256").update(bytes).digest("hex")}"`; + return { kind: "hit", bytes, mime, etag }; + } catch { + return { kind: "error" }; + } +} + +async function resolveGravatar( + hash: string, + options: { fetchImpl: typeof globalThis.fetch; nowMs: () => number; deadline?: AbortSignal }, +): Promise { + const cached = getCachedGravatar(hash, options.nowMs()); + if (cached) { + return cached; + } + const inFlight = gravatarRequests.get(hash); + if (inFlight) { + return await inFlight; + } + const request = fetchGravatar(hash, options.fetchImpl, options.deadline).then((result) => { + if (result.kind !== "error") { + cacheGravatar(hash, result, options.nowMs()); + } + return result; + }); + gravatarRequests.set(hash, request); + try { + return await request; + } finally { + gravatarRequests.delete(hash); + } +} + +function sendAvatar( + req: IncomingMessage, + res: ServerResponse, + avatar: { bytes: Uint8Array; mime: string; etag: string }, + cacheControl: string, +): void { + if (ifNoneMatchMatches(req.headers["if-none-match"], avatar.etag)) { + // Carry the success cache policy so a 304 does not inherit the miss-path + // no-store and force the client to re-download an unchanged avatar. + res.writeHead(304, { ETag: avatar.etag, "Cache-Control": cacheControl }); + res.end(); + return; + } + res.writeHead(200, { + "Content-Type": avatar.mime, + "Content-Length": avatar.bytes.byteLength, + "Cache-Control": cacheControl, + ETag: avatar.etag, + }); + res.end(req.method === "HEAD" ? undefined : avatar.bytes); +} + /** Serves a profile avatar with the same HTTP operator auth as sibling gateway endpoints. */ export async function handleUserProfileAvatarHttpRequest( req: IncomingMessage, @@ -20,6 +293,8 @@ export async function handleUserProfileAvatarHttpRequest( trustedProxies?: string[]; allowRealIpFallback?: boolean; rateLimiter?: AuthRateLimiter; + fetchImpl?: typeof globalThis.fetch; + nowMs?: () => number; }, ): Promise { const profileId = matchUserProfileAvatarPath(pathname); @@ -27,6 +302,20 @@ export async function handleUserProfileAvatarHttpRequest( return false; } const method = req.method; + const cfg = getRuntimeConfig(); + const corsAllowed = setAvatarCorsHeaders(req, res, cfg); + if (method === "OPTIONS") { + if (!corsAllowed) { + sendJson(res, 403, { ok: false, error: { type: "origin_not_allowed" } }); + return true; + } + res.setHeader("Access-Control-Allow-Methods", "GET, HEAD"); + res.setHeader("Access-Control-Allow-Headers", "Authorization"); + res.setHeader("Access-Control-Max-Age", "600"); + res.writeHead(204); + res.end(); + return true; + } if (method !== "GET" && method !== "HEAD") { sendMethodNotAllowed(res, "GET, HEAD"); return true; @@ -44,24 +333,75 @@ export async function handleUserProfileAvatarHttpRequest( if (!authResult) { return true; } - const avatar = getProfileAvatar(profileId); - if (!avatar) { - sendJson(res, 404, { ok: false, error: { type: "not_found" } }); + // Avatars render as plain against a stable, unversioned route, so a + // heuristically-cached 404 miss would otherwise hide a later uploaded image. + // Misses must never be cached; the 200 path overrides this with must-revalidate. + res.setHeader("Cache-Control", "no-store"); + let uploadedAvatar: ReturnType; + try { + uploadedAvatar = getProfileAvatar(profileId); + } catch (error) { + if (error instanceof UserProfileNotFoundError) { + sendJson(res, 404, { ok: false, error: { type: "not_found" } }); + return true; + } + sendJson(res, 500, { ok: false, error: { type: "profile_lookup_failed" } }); return true; } - const etag = formatUserProfileAvatarEtag(avatar.sha256, avatar.mime); - if (ifNoneMatchMatches(req.headers["if-none-match"], etag)) { - res.writeHead(304, { ETag: etag }); - res.end(); + if (uploadedAvatar) { + sendAvatar( + req, + res, + { + bytes: uploadedAvatar.bytes, + mime: uploadedAvatar.mime, + etag: formatUserProfileAvatarEtag(uploadedAvatar.sha256, uploadedAvatar.mime), + }, + "private, max-age=0, must-revalidate", + ); return true; } - res.writeHead(200, { - "Content-Type": avatar.mime, - "Content-Length": avatar.bytes.byteLength, - "Cache-Control": "private, max-age=0, must-revalidate", - ETag: etag, + + let hashes: string[]; + try { + hashes = getUserProfileListItem(profileId) + .emails.slice(0, MAX_GRAVATAR_EMAIL_LOOKUPS) + .map(hashEmail); + } catch (error) { + if (error instanceof UserProfileNotFoundError) { + sendJson(res, 404, { ok: false, error: { type: "not_found" } }); + return true; + } + sendJson(res, 500, { ok: false, error: { type: "profile_lookup_failed" } }); + return true; + } + + // Resolve linked emails sequentially and stop at the first hit: the primary + // email keeps precedence, and a secondary email's hash is disclosed to + // Gravatar only once the earlier one is a definite miss. A single shared + // deadline bounds the total wait, so an unreachable Gravatar cannot stall the + // held connection by one timeout per linked email. + const deadline = AbortSignal.timeout(GRAVATAR_TOTAL_TIMEOUT_MS); + let transientFailure = false; + for (const hash of hashes) { + const result = await resolveGravatar(hash, { + fetchImpl: opts.fetchImpl ?? globalThis.fetch, + nowMs: opts.nowMs ?? Date.now, + deadline, + }); + if (result.kind === "hit") { + sendAvatar(req, res, result, "private, max-age=0, must-revalidate"); + return true; + } + transientFailure ||= result.kind === "error"; + if (deadline.aborted) { + break; + } + } + sendJson(res, transientFailure ? 502 : 404, { + ok: false, + error: { type: transientFailure ? "avatar_upstream_unavailable" : "not_found" }, }); - res.end(method === "HEAD" ? undefined : avatar.bytes); return true; } diff --git a/ui/src/app/gateway-store.ts b/ui/src/app/gateway-store.ts index a0d2b988936..84a4bbe4c8d 100644 --- a/ui/src/app/gateway-store.ts +++ b/ui/src/app/gateway-store.ts @@ -7,6 +7,7 @@ import { type GatewayEventListener, type GatewayHelloOk, } from "../api/gateway.ts"; +import { setAvatarGatewayOrigin } from "../lib/identity-avatar.ts"; import { resolveSessionKey } from "../lib/sessions/index.ts"; import { generateUUID } from "../lib/uuid.ts"; import type { @@ -141,6 +142,9 @@ export function createApplicationGateway( connectionOverrides.gatewayUrl !== undefined && connectionOverrides.gatewayUrl !== connection.gatewayUrl; connection = nextConnection; + // Trust the connected gateway's origin for avatar route resolution so + // split-origin Control UI deployments load uploaded/proxied avatars. + setAvatarGatewayOrigin(nextConnection.gatewayUrl); updateSettings( { gatewayUrl: nextConnection.gatewayUrl, diff --git a/ui/src/components/viewer-facepile.test.ts b/ui/src/components/viewer-facepile.test.ts index eb3a787a73b..6f068563c9c 100644 --- a/ui/src/components/viewer-facepile.test.ts +++ b/ui/src/components/viewer-facepile.test.ts @@ -1,126 +1,50 @@ /* @vitest-environment jsdom */ -import { afterEach, describe, expect, it, vi } from "vitest"; +import { afterEach, expect, it, vi } from "vitest"; +import { setAvatarGatewayOrigin } from "../lib/identity-avatar.ts"; import type { PresenceViewer } from "./viewer-facepile.ts"; import "./viewer-facepile.ts"; type ViewerAvatarElement = HTMLElement & { - user: PresenceViewer; + user: PresenceViewer | null; updateComplete: Promise; }; -function viewer(overrides: Partial = {}): PresenceViewer { - return { - id: "profile-1", - name: "Test Person", - watchedSessions: [], - ...overrides, - }; -} - -async function mountViewer(user: PresenceViewer): Promise { - const avatar = document.createElement("openclaw-viewer-avatar") as ViewerAvatarElement; - avatar.user = user; - document.body.append(avatar); - await avatar.updateComplete; - return avatar; -} - -async function waitForImage(avatar: ViewerAvatarElement): Promise { - await vi.waitFor(() => expect(avatar.querySelector("img")).not.toBeNull()); - return avatar.querySelector("img")!; -} - afterEach(() => { document.body.replaceChildren(); + setAvatarGatewayOrigin(null); vi.restoreAllMocks(); }); -describe("viewer avatar resolution", () => { - it("keeps an uploaded avatar ahead of the email fallback", async () => { - const digest = vi.spyOn(globalThis.crypto.subtle, "digest"); - const avatar = await mountViewer( - viewer({ email: "test@example.com", avatarUrl: "/api/users/profile-1/avatar?v=2" }), - ); - const image = await waitForImage(avatar); +it("uses the shared resolver and rejects cross-origin presence avatar metadata", async () => { + const avatar = document.createElement("openclaw-viewer-avatar") as ViewerAvatarElement; + avatar.user = { + id: "profile-mallory", + name: "Mallory", + avatarUrl: "https://evil.example/avatar.png", + watchedSessions: [], + }; + document.body.append(avatar); - expect(image.getAttribute("src")).toBe("/api/users/profile-1/avatar?v=2"); - expect(image.getAttribute("referrerpolicy")).toBe("no-referrer"); - expect(image.getAttribute("loading")).toBe("lazy"); - expect(digest).not.toHaveBeenCalled(); - }); - - it("retries a failed uploaded avatar after the user snapshot updates", async () => { - const avatarUrl = "/api/users/profile-1/avatar?v=2"; - const avatar = await mountViewer(viewer({ avatarUrl })); - const failedImage = await waitForImage(avatar); - - failedImage.dispatchEvent(new Event("error")); + await vi.waitFor(async () => { await avatar.updateComplete; expect(avatar.querySelector("img")).toBeNull(); - - avatar.user = viewer({ avatarUrl }); - await avatar.updateComplete; - expect(avatar.querySelector("img")?.getAttribute("src")).toBe(avatarUrl); - }); - - it("normalizes and hashes email with SHA-256 for the Gravatar URL", async () => { - const avatar = await mountViewer(viewer({ email: " TEST@example.com " })); - - expect(avatar.querySelector("img")).toBeNull(); - expect(avatar.textContent?.trim()).toBe("TP"); - const image = await waitForImage(avatar); - expect(image.getAttribute("src")).toBe( - "https://gravatar.com/avatar/973dfe463ec85785f5f95af5ba3906eedb2d931c24e69824a89ea65dba4e813b?d=404&s=128", - ); - }); - - it("falls back to initials when the Gravatar image fails", async () => { - const avatar = await mountViewer(viewer({ email: "missing-one@example.test" })); - const image = await waitForImage(avatar); - - image.dispatchEvent(new Event("error")); - await avatar.updateComplete; - - expect(avatar.querySelector("img")).toBeNull(); - expect(avatar.textContent?.trim()).toBe("TP"); + expect(avatar.textContent?.trim()).toBe("MA"); }); - - it("does not attribute a stale Gravatar error to the next user", async () => { - const avatar = await mountViewer(viewer({ email: "first-stale@example.test" })); - const staleImage = await waitForImage(avatar); - - avatar.user = viewer({ id: "profile-2", email: "second-current@example.test" }); - staleImage.dispatchEvent(new Event("error")); +}); + +it("renders trusted presence avatar routes directly", async () => { + const avatar = document.createElement("openclaw-viewer-avatar") as ViewerAvatarElement; + avatar.user = { + id: "profile-ada", + name: "Ada Lovelace", + avatarUrl: "/api/users/profile-ada/avatar", + watchedSessions: [], + }; + document.body.append(avatar); + + await vi.waitFor(async () => { await avatar.updateComplete; - - const image = await waitForImage(avatar); - expect(image.getAttribute("src")).toMatch( - /^https:\/\/gravatar\.com\/avatar\/[a-f0-9]{64}\?d=404&s=128$/u, - ); - }); - - it("renders initials without attempting a hash when email is absent", async () => { - const digest = vi.spyOn(globalThis.crypto.subtle, "digest"); - const avatar = await mountViewer(viewer()); - - expect(avatar.querySelector("img")).toBeNull(); - expect(avatar.textContent?.trim()).toBe("TP"); - expect(digest).not.toHaveBeenCalled(); - }); - - it("caches missing Gravatars by normalized email", async () => { - const digest = vi.spyOn(globalThis.crypto.subtle, "digest"); - const first = await mountViewer(viewer({ email: " Missing-Two@Example.Test " })); - const image = await waitForImage(first); - image.dispatchEvent(new Event("error")); - await first.updateComplete; - - const second = await mountViewer( - viewer({ id: "profile-2", email: "missing-two@example.test" }), - ); - - expect(second.querySelector("img")).toBeNull(); - expect(digest).toHaveBeenCalledTimes(1); + expect(avatar.querySelector("img")?.getAttribute("src")).toBe("/api/users/profile-ada/avatar"); }); }); diff --git a/ui/src/components/viewer-facepile.ts b/ui/src/components/viewer-facepile.ts index 92c2ed3182a..a7187014737 100644 --- a/ui/src/components/viewer-facepile.ts +++ b/ui/src/components/viewer-facepile.ts @@ -1,6 +1,7 @@ -import { html, nothing, type PropertyValues } from "lit"; -import { property, state } from "lit/decorators.js"; +import { html, nothing } from "lit"; +import { property } from "lit/decorators.js"; import type { PresenceEntry } from "../api/types.ts"; +import { resolveAvatar } from "../lib/identity-avatar.ts"; import { OpenClawLightDomContentsElement } from "../lit/openclaw-element.ts"; import "./tooltip.ts"; @@ -112,58 +113,39 @@ function avatarColor(userId: string): string { return `hsl(${(hash >>> 0) % 360} 48% 42%)`; } -type GravatarCacheEntry = string | Promise | { failedUntil: number }; - -// An error cannot distinguish a Gravatar 404 from a transient network or -// 5xx failure, so negative entries expire instead of poisoning the page lifetime. -// Accepted tradeoff: re-resolution happens on the next user-prop update (any -// presence/snapshot event), not on a timer — a fully idle page keeps initials -// until activity resumes rather than each avatar owning a retry timer. -const GRAVATAR_NEGATIVE_CACHE_MS = 10 * 60 * 1000; -const gravatarUrlCache = new Map(); - -function negativeEntry(): GravatarCacheEntry { - return { failedUntil: Date.now() + GRAVATAR_NEGATIVE_CACHE_MS }; +function renderAvatarInitials(user: PresenceViewer) { + return html`${initialsFor(user)}`; } -function normalizedEmail(email: string | null | undefined): string | undefined { - return normalized(email)?.toLowerCase(); -} - -function gravatarUrlForEmail(email: string): string | null | Promise { - const normalizedValue = normalizedEmail(email); - if (!normalizedValue) { - return null; +function resolveViewerAvatar(user: PresenceViewer) { + const avatar = resolveAvatar({ + id: user.email ?? user.id, + name: user.name, + profileAvatarUrl: user.avatarUrl, + }); + if (avatar.kind === "initials") { + return renderAvatarInitials(user); } - const cached = gravatarUrlCache.get(normalizedValue); - if (cached !== undefined) { - if (typeof cached === "object" && cached !== null && "failedUntil" in cached) { - if (cached.failedUntil > Date.now()) { - return null; - } - gravatarUrlCache.delete(normalizedValue); - } else { - return cached; - } - } - const pending = Promise.resolve() - .then(() => - globalThis.crypto.subtle.digest("SHA-256", new TextEncoder().encode(normalizedValue)), - ) - .then((digest) => { - const hash = [...new Uint8Array(digest)] - .map((byte) => byte.toString(16).padStart(2, "0")) - .join(""); - const url = `https://gravatar.com/avatar/${hash}?d=404&s=128`; - gravatarUrlCache.set(normalizedValue, url); - return url; - }) - .catch(() => { - gravatarUrlCache.set(normalizedValue, negativeEntry()); - return null; - }); - gravatarUrlCache.set(normalizedValue, pending); - return pending; + return html` { + const image = event.currentTarget; + if (image instanceof HTMLImageElement) { + image.closest(".viewer-avatar")?.classList.add("is-fallback"); + } + }} + @load=${(event: Event) => { + const image = event.currentTarget; + if (image instanceof HTMLImageElement) { + image.closest(".viewer-avatar")?.classList.remove("is-fallback"); + } + }} + /> + ${initialsFor(user)}`; } export type ViewerAvatarVariant = "session" | "footer" | "profile"; @@ -172,89 +154,18 @@ class ViewerAvatar extends OpenClawLightDomContentsElement { @property({ attribute: false }) user: PresenceViewer | null = null; @property() variant: ViewerAvatarVariant = "session"; - @state() private gravatar: { email: string; url: string } | null = null; - - private failedImageUrl: string | null = null; - private resolutionId = 0; - - protected override willUpdate(changedProperties: PropertyValues) { - if (changedProperties.has("user")) { - this.failedImageUrl = null; - } - } - - protected override updated(changedProperties: PropertyValues) { - if (!changedProperties.has("user")) { - return; - } - const resolutionId = ++this.resolutionId; - const email = this.user?.avatarUrl ? undefined : normalizedEmail(this.user?.email); - if (!email) { - return; - } - const applyResolvedUrl = (url: string | null) => { - if ( - resolutionId === this.resolutionId && - !this.user?.avatarUrl && - normalizedEmail(this.user?.email) === email - ) { - this.gravatar = url ? { email, url } : null; - } - }; - const resolved = gravatarUrlForEmail(email); - if (typeof resolved === "string") { - applyResolvedUrl(resolved); - return; - } - if (resolved) { - void resolved.then(applyResolvedUrl); - } - } - - private readonly handleImageError = (event: Event) => { - const imageUrl = (event.currentTarget as HTMLImageElement).getAttribute("src"); - const user = this.user; - if (!imageUrl || !user) { - return; - } - const email = normalizedEmail(user.email); - if ( - !user.avatarUrl && - email && - imageUrl === this.gravatar?.url && - email === this.gravatar.email - ) { - gravatarUrlCache.set(email, negativeEntry()); - this.gravatar = null; - return; - } - this.failedImageUrl = imageUrl; - this.requestUpdate(); - }; - override render() { const user = this.user; if (!user) { return nothing; } const label = presenceViewerLabel(user); - const email = normalizedEmail(user.email); - const gravatarUrl = this.gravatar && this.gravatar.email === email ? this.gravatar.url : null; - const imageUrl = user.avatarUrl ?? gravatarUrl; return html` - ${imageUrl && imageUrl !== this.failedImageUrl - ? html`` - : html`${initialsFor(user)}`} + ${resolveViewerAvatar(user)} `; } } diff --git a/ui/src/e2e/profile-page.e2e.test.ts b/ui/src/e2e/profile-page.e2e.test.ts index fcac1df5194..3ed8559bc34 100644 --- a/ui/src/e2e/profile-page.e2e.test.ts +++ b/ui/src/e2e/profile-page.e2e.test.ts @@ -181,12 +181,16 @@ describeControlUiE2e("Control UI profile page mocked Gateway E2E", () => { } }); - it("renders the shared Gravatar fallback in the profile preview", async () => { + it("renders the gateway avatar route in the profile preview", async () => { const context = await browser.newContext(); const page = await context.newPage(); - const gravatarRequests: string[] = []; - await page.route("https://gravatar.com/avatar/**", async (route) => { - gravatarRequests.push(route.request().url()); + const avatarRequests: string[] = []; + // The gateway serves the avatar (uploaded first, Gravatar fallback second) + // behind its own same-origin route; the Control UI renders only that route, + // so the preview never requests gravatar.com directly — the Control UI CSP + // (img-src 'self') would block it. + await page.route("**/api/users/profile-1/avatar*", async (route) => { + avatarRequests.push(route.request().url()); await route.fulfill({ body: '', contentType: "image/svg+xml", @@ -228,12 +232,16 @@ describeControlUiE2e("Control UI profile page mocked Gateway E2E", () => { ], }); - const expectedUrl = - "https://gravatar.com/avatar/973dfe463ec85785f5f95af5ba3906eedb2d931c24e69824a89ea65dba4e813b?d=404&s=128"; const profileAvatar = page.locator("#settings-profile-identity openclaw-viewer-avatar img"); await profileAvatar.waitFor({ timeout: 10_000 }); - expect(await profileAvatar.getAttribute("src")).toBe(expectedUrl); - await expect.poll(() => gravatarRequests.includes(expectedUrl)).toBe(true); + // profile-page derives the src from userProfileAvatarUrl(id, updatedAt); + // the gateway origin may absolutize it, so match the canonical path suffix. + expect(await profileAvatar.getAttribute("src")).toMatch( + /\/api\/users\/profile-1\/avatar\?v=2$/u, + ); + await expect + .poll(() => avatarRequests.some((url) => url.includes("/api/users/profile-1/avatar"))) + .toBe(true); } finally { await context.close(); } diff --git a/ui/src/i18n/locales/en.ts b/ui/src/i18n/locales/en.ts index f6ee1603ee6..fb052d0e795 100644 --- a/ui/src/i18n/locales/en.ts +++ b/ui/src/i18n/locales/en.ts @@ -1122,11 +1122,23 @@ export const en: TranslationMap = { }, personal: { title: "Personal", + profileTitle: "Profile", user: "User", you: "You", assistant: "Assistant", localIdentity: "Your local chat identity", + profileIdentity: "Your gateway profile", assistantIdentity: "Assistant identity", + displayName: "Display name", + displayNamePlaceholder: "Your name", + saveProfile: "Save name", + saving: "Saving…", + uploading: "Processing…", + profileHint: "JPEG, PNG, or WebP. Images are center-cropped and resized to 512px.", + profileLoadFailed: "Could not load your profile.", + profileSaveFailed: "Could not save your profile.", + avatarInvalid: "Choose a valid JPEG, PNG, or WebP image.", + avatarTooLarge: "The processed avatar is still too large.", avatarText: "Avatar text / emoji", avatarPlaceholder: "JD or 🦞", chooseImage: "Choose image", diff --git a/ui/src/lib/identity-avatar.test.ts b/ui/src/lib/identity-avatar.test.ts index e9ffa7e3602..b0e580b47fc 100644 --- a/ui/src/lib/identity-avatar.test.ts +++ b/ui/src/lib/identity-avatar.test.ts @@ -1,41 +1,29 @@ // @vitest-environment node -import { describe, expect, it } from "vitest"; -import { resolveAvatar } from "./identity-avatar.ts"; +import { afterEach, describe, expect, it } from "vitest"; +import { resolveAvatar, setAvatarGatewayOrigin } from "./identity-avatar.ts"; + +afterEach(() => { + setAvatarGatewayOrigin(null); +}); describe("resolveAvatar", () => { - it("uses a normalized email id for the proxied avatar hash", async () => { - await expect( - resolveAvatar({ id: " Alice@Example.com ", avatarProxyBaseUrl: "/api/avatars/" }), - ).resolves.toEqual({ - kind: "gravatar", - url: "/api/avatars/ff8d9819fc0e12bf0d24892e45987e249a28dce836a85cad60e28eaaa8c6d976?s=64", - }); - }); - - it("never contacts a third-party avatar host without a proxy base", async () => { - await expect(resolveAvatar({ id: "alice@example.com" })).resolves.toMatchObject({ - kind: "initials", - initials: "A", - }); - }); - - it("falls back to initials for a non-email id", async () => { - await expect(resolveAvatar({ id: "profile_123" })).resolves.toMatchObject({ + it("falls back to initials for a non-email id", () => { + expect(resolveAvatar({ id: "profile_123" })).toMatchObject({ kind: "initials", initials: "P", }); }); - it("derives up to two initials from a display name", async () => { - await expect(resolveAvatar({ name: "Ada Lovelace Byron" })).resolves.toMatchObject({ + it("derives up to two initials from a display name", () => { + expect(resolveAvatar({ name: "Ada Lovelace Byron" })).toMatchObject({ kind: "initials", initials: "AL", }); }); - it("keeps the initials color deterministic", async () => { - const first = await resolveAvatar({ id: "profile_123", name: "Ada Lovelace" }); - const second = await resolveAvatar({ id: "profile_123", name: "Renamed User" }); + it("keeps the initials color deterministic", () => { + const first = resolveAvatar({ id: "profile_123", name: "Ada Lovelace" }); + const second = resolveAvatar({ id: "profile_123", name: "Renamed User" }); expect(first.kind).toBe("initials"); expect(second.kind).toBe("initials"); if (first.kind === "initials" && second.kind === "initials") { @@ -43,42 +31,97 @@ describe("resolveAvatar", () => { } }); - it("lets an already-resolved profile avatar win", async () => { - await expect( - resolveAvatar({ id: "alice@example.com", profileAvatarUrl: "/avatars/alice.png" }), - ).resolves.toEqual({ kind: "profile", url: "/avatars/alice.png" }); + it("lets an already-resolved profile avatar win", () => { + expect( + resolveAvatar({ id: "alice@example.com", profileAvatarUrl: "/api/users/p1/avatar" }), + ).toEqual({ kind: "profile", url: "/api/users/p1/avatar" }); }); }); describe("resolveAvatar profile URL origin restriction", () => { - it("rejects absolute profile URLs from sender metadata", async () => { - await expect( + it("rejects absolute profile URLs from sender metadata", () => { + expect( resolveAvatar({ id: "alice@example.com", profileAvatarUrl: "https://evil.example/a.png" }), - ).resolves.toMatchObject({ kind: "initials" }); + ).toMatchObject({ kind: "initials" }); }); - it("rejects protocol-relative profile URLs", async () => { - await expect( + it("rejects protocol-relative profile URLs", () => { + expect( resolveAvatar({ id: "alice@example.com", profileAvatarUrl: "//evil.example/a.png" }), - ).resolves.toMatchObject({ kind: "initials" }); + ).toMatchObject({ kind: "initials" }); }); - it("rejects backslash and control-character parser bypasses", async () => { + it("rejects backslash and control-character parser bypasses", () => { for (const url of [ "/\\evil.example/a.png", "\\/evil.example/a.png", "/\t/evil.example/a.png", "htt\nps://evil.example/a.png", ]) { - await expect( - resolveAvatar({ id: "alice@example.com", profileAvatarUrl: url }), - ).resolves.toMatchObject({ kind: "initials" }); + expect(resolveAvatar({ id: "alice@example.com", profileAvatarUrl: url })).toMatchObject({ + kind: "initials", + }); } }); - it("accepts same-origin relative profile URLs", async () => { - await expect( - resolveAvatar({ id: "alice@example.com", profileAvatarUrl: "/avatars/alice.png" }), - ).resolves.toEqual({ kind: "profile", url: "/avatars/alice.png" }); + it("accepts the canonical same-origin avatar route", () => { + expect( + resolveAvatar({ id: "alice@example.com", profileAvatarUrl: "/api/users/p1/avatar" }), + ).toEqual({ kind: "profile", url: "/api/users/p1/avatar" }); + }); + + it("rejects a same-origin path that is not the avatar route", () => { + expect( + resolveAvatar({ id: "alice@example.com", profileAvatarUrl: "/api/secrets" }), + ).toMatchObject({ kind: "initials" }); + }); + + it("preserves the version query but drops the fragment on the avatar route", () => { + expect( + resolveAvatar({ id: "alice@example.com", profileAvatarUrl: "/api/users/p1/avatar?v=2#f" }), + ).toEqual({ kind: "profile", url: "/api/users/p1/avatar?v=2" }); + }); +}); + +describe("resolveAvatar gateway origin trust", () => { + it("keeps relative avatar paths relative when no gateway origin is set", () => { + expect( + resolveAvatar({ id: "alice@example.com", profileAvatarUrl: "/api/users/p1/avatar" }), + ).toEqual({ kind: "profile", url: "/api/users/p1/avatar" }); + }); + + it("resolves relative paths against the configured gateway origin", () => { + setAvatarGatewayOrigin("wss://gw.example.com/ws"); + expect( + resolveAvatar({ id: "alice@example.com", profileAvatarUrl: "/api/users/p1/avatar" }), + ).toEqual({ kind: "profile", url: "https://gw.example.com/api/users/p1/avatar" }); + }); + + it("allows an absolute URL only when it matches the gateway origin", () => { + setAvatarGatewayOrigin("https://gw.example.com"); + expect( + resolveAvatar({ + id: "a@example.com", + profileAvatarUrl: "https://gw.example.com/api/users/p1/avatar", + }), + ).toEqual({ kind: "profile", url: "https://gw.example.com/api/users/p1/avatar" }); + }); + + it("rejects an absolute URL from a different origin than the gateway", () => { + setAvatarGatewayOrigin("https://gw.example.com"); + expect( + resolveAvatar({ id: "a@example.com", profileAvatarUrl: "https://evil.example/a.png" }), + ).toMatchObject({ kind: "initials" }); + }); + + // NOTE: the trusted origin can only come from setAvatarGatewayOrigin — the + // IdentityAvatarInput type has no gatewayOrigin field, so sender metadata + // cannot influence it (compile-time enforced; no runtime test needed). + + it("honors the app-wide gateway origin set via setAvatarGatewayOrigin", () => { + setAvatarGatewayOrigin("wss://gw.example.com/ws"); + expect( + resolveAvatar({ id: "a@example.com", profileAvatarUrl: "/api/users/p1/avatar" }), + ).toEqual({ kind: "profile", url: "https://gw.example.com/api/users/p1/avatar" }); }); }); diff --git a/ui/src/lib/identity-avatar.ts b/ui/src/lib/identity-avatar.ts index fbbc76a1722..8734dd93911 100644 --- a/ui/src/lib/identity-avatar.ts +++ b/ui/src/lib/identity-avatar.ts @@ -1,40 +1,75 @@ import { formatSenderLabel, type SenderIdentity } from "./chat/sender-label.ts"; +// NOTE: this is sender-controlled metadata. It must never carry the trusted +// gateway origin — that comes only from the app connection via +// setAvatarGatewayOrigin(). export type IdentityAvatarInput = SenderIdentity & { profileAvatarUrl?: string; - /** - * Base URL of a gateway-side avatar proxy (same-origin). When absent, the - * email-hash avatar tier is disabled entirely: the browser must never - * contact a third-party avatar host directly, because that leaks a - * dictionary-recoverable sender email hash plus the viewer's IP per render. - */ - avatarProxyBaseUrl?: string; }; +const ORIGIN_PROBE = "https://origin-probe.invalid"; + +let appGatewayOrigin: string | null = null; + +function toHttpOrigin(url: string | null | undefined): string | null { + if (!url) { + return null; + } + try { + const parsed = new URL(url); + const scheme = + parsed.protocol === "wss:" ? "https:" : parsed.protocol === "ws:" ? "http:" : parsed.protocol; + return `${scheme}//${parsed.host}`; + } catch { + return null; + } +} + +/** Records the connected gateway URL so avatar routes resolve to its origin. */ +export function setAvatarGatewayOrigin(gatewayUrl: string | null | undefined): void { + appGatewayOrigin = toHttpOrigin(gatewayUrl); +} + +// Mirrors the server's user-profiles-http-path matcher. Sender metadata may +// point only at this image route, never another gateway endpoint. +const USER_AVATAR_PATHNAME = /^\/api\/users\/[^/]+\/avatar$/u; + +/** + * Returns a browser-safe avatar URL, or null. Only the canonical + * /api/users//avatar route is trusted (pathname pinned, fragment dropped). + * The query is preserved: the gateway stamps a ?v= revision there so + * the browser cache-busts a replaced avatar. Since avatars now render as plain + * with no attached credentials, a varied query cannot amplify any + * client cache — the browser bounds it. Relative paths resolve against the + * trusted gateway origin; absolute URLs must match that origin. + */ +function toTrustedAvatarUrl(value: string, gatewayOrigin: string | null): string | null { + try { + const parsed = new URL(value, ORIGIN_PROBE); + if (!USER_AVATAR_PATHNAME.test(parsed.pathname)) { + return null; + } + const suffix = parsed.pathname + parsed.search; + if (parsed.origin === ORIGIN_PROBE) { + return gatewayOrigin ? new URL(suffix, gatewayOrigin).toString() : suffix; + } + return gatewayOrigin && parsed.origin === gatewayOrigin ? gatewayOrigin + suffix : null; + } catch { + return null; + } +} + export type ResolvedIdentityAvatar = | { kind: "profile"; url: string } | { kind: "gravatar"; url: string } | { kind: "initials"; initials: string; colorSeed: number }; -const EMAIL_PATTERN = /^[^@\s]+@[^@\s]+$/; - function initialsFromLabel(label: string): string { const words = label.trim().split(/\s+/u).filter(Boolean).slice(0, 2); const initials = words.map((word) => Array.from(word)[0] ?? "").join(""); return initials.toUpperCase() || "?"; } -const ORIGIN_PROBE = "https://origin-probe.invalid"; - -/** True only when the value resolves inside the embedding origin for any base. */ -function isOriginRelativePath(value: string): boolean { - try { - return new URL(value, ORIGIN_PROBE).origin === ORIGIN_PROBE; - } catch { - return false; - } -} - function stableColorSeed(value: string): number { let hash = 0x811c9dc5; for (let index = 0; index < value.length; index += 1) { @@ -44,47 +79,10 @@ function stableColorSeed(value: string): number { return hash >>> 0; } -async function sha256Hex(value: string): Promise { - if (!globalThis.crypto?.subtle) { - return null; - } - try { - const digest = await globalThis.crypto.subtle.digest( - "SHA-256", - new TextEncoder().encode(value), - ); - return Array.from(new Uint8Array(digest), (byte) => byte.toString(16).padStart(2, "0")).join( - "", - ); - } catch { - return null; - } -} - -/** Resolves profile, Gravatar, then deterministic initials without fetching profile data. */ -export async function resolveAvatar(input: IdentityAvatarInput): Promise { - const profileAvatarUrl = input.profileAvatarUrl?.trim(); - // Same-origin only: profile URLs arrive via sender metadata, and an - // absolute URL would let a sender make every viewing browser contact an - // arbitrary host. Validate with the URL parser (not string prefixes) so - // browser normalization quirks — backslashes as slashes, stripped tab or - // newline characters — cannot smuggle in a cross-origin target. - if (profileAvatarUrl && isOriginRelativePath(profileAvatarUrl)) { - return { kind: "profile", url: profileAvatarUrl }; - } - +export function resolveAvatarInitials( + input: IdentityAvatarInput, +): Extract { const id = input.id?.trim(); - const proxyBase = input.avatarProxyBaseUrl?.trim().replace(/\/+$/, ""); - if (proxyBase && id && EMAIL_PATTERN.test(id)) { - const hash = await sha256Hex(id.toLowerCase()); - if (hash) { - return { - kind: "gravatar", - url: `${proxyBase}/${hash}?s=64`, - }; - } - } - const label = formatSenderLabel(input) ?? "?"; return { kind: "initials", @@ -92,3 +90,24 @@ export async function resolveAvatar(input: IdentityAvatarInput): Promise/avatar endpoint or falls back to initials. + */ +export function resolveAvatar(input: IdentityAvatarInput): ResolvedIdentityAvatar { + // Trusted origin comes only from the app connection, never from `input`. + const gatewayOrigin = appGatewayOrigin; + + const profileAvatarUrl = input.profileAvatarUrl?.trim(); + if (profileAvatarUrl) { + const trusted = toTrustedAvatarUrl(profileAvatarUrl, gatewayOrigin); + if (trusted) { + return { kind: "profile", url: trusted }; + } + } + + return resolveAvatarInitials(input); +} diff --git a/ui/src/pages/chat/chat-pane.ts b/ui/src/pages/chat/chat-pane.ts index f809e55a9c3..5378954c13d 100644 --- a/ui/src/pages/chat/chat-pane.ts +++ b/ui/src/pages/chat/chat-pane.ts @@ -3142,6 +3142,7 @@ class ChatPane extends OpenClawLightDomElement { chatMessageMaxWidth: state.chatMessageMaxWidth, assistantAttachmentAuthToken: resolveAssistantAttachmentAuthToken(state as never), basePath: state.basePath, + gatewayUrl: state.settings.gatewayUrl, }; const chat = renderChat(props); const content = diff --git a/ui/src/pages/chat/chat-view.ts b/ui/src/pages/chat/chat-view.ts index 9d67f034706..df7666e105b 100644 --- a/ui/src/pages/chat/chat-view.ts +++ b/ui/src/pages/chat/chat-view.ts @@ -209,6 +209,7 @@ export type ChatProps = { onSplitRatioChange?: (ratio: number) => void; onChatScroll?: (event: Event) => void; basePath?: string; + gatewayUrl?: string; composerControls?: TemplateResult | typeof nothing; replyTarget?: { messageId: string; @@ -335,6 +336,7 @@ export function renderChat(props: ChatProps) { questionPrompts: props.gatewayQuestionPrompts, sessions: props.sessions, sessionHost: props.sessionHost, + gatewayUrl: props.gatewayUrl, assistantName: props.assistantName, assistantAvatar: props.assistantAvatar, assistantAvatarUrl: props.assistantAvatarUrl, diff --git a/ui/src/pages/chat/components/chat-author-avatar.ts b/ui/src/pages/chat/components/chat-author-avatar.ts index 7473238d533..a7d77a89aa8 100644 --- a/ui/src/pages/chat/components/chat-author-avatar.ts +++ b/ui/src/pages/chat/components/chat-author-avatar.ts @@ -1,8 +1,8 @@ import { html, nothing, type TemplateResult } from "lit"; -import { until } from "lit/directives/until.js"; import { formatSenderLabel } from "../../../lib/chat/sender-label.ts"; import { resolveAvatar, + resolveAvatarInitials, type IdentityAvatarInput, type ResolvedIdentityAvatar, } from "../../../lib/identity-avatar.ts"; @@ -63,18 +63,15 @@ export function renderChatAuthorAvatar( if (!sender || !label) { return nothing; } - const resolved = Promise.all([resolveAvatar(sender), resolveAvatar({ name: label })]).then( - ([avatar, fallback]) => { - const initials = - fallback.kind === "initials" - ? fallback - : ({ kind: "initials", initials: "?", colorSeed: 0 } as const); - return renderResolvedAvatar(avatar, initials); - }, - ); + const fallback = resolveAvatarInitials(sender); + const avatar = resolveAvatar(sender); + const resolved = + avatar.kind === "initials" + ? renderInitialsAvatar(avatar) + : renderResolvedAvatar(avatar, fallback); return html` - ${until(resolved, nothing)} + ${resolved} `; } diff --git a/ui/src/pages/chat/components/chat-message.test.ts b/ui/src/pages/chat/components/chat-message.test.ts index 54e0b1b2665..abfe166be3e 100644 --- a/ui/src/pages/chat/components/chat-message.test.ts +++ b/ui/src/pages/chat/components/chat-message.test.ts @@ -5,6 +5,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import * as markdown from "../../../components/markdown.ts"; import type { MessageGroup } from "../../../lib/chat/chat-types.ts"; import { setUiTimeFormatPreference } from "../../../lib/format.ts"; +import { setAvatarGatewayOrigin } from "../../../lib/identity-avatar.ts"; import * as localStorageModule from "../../../local-storage.ts"; import * as chatAvatar from "../chat-avatar.ts"; import { renderMessageGroup, renderStreamGroup } from "./chat-message.ts"; @@ -461,6 +462,7 @@ afterEach(() => { }); clearDeleteConfirmSkip(); setUiTimeFormatPreference("auto"); + setAvatarGatewayOrigin(null); vi.useRealTimers(); vi.unstubAllGlobals(); vi.restoreAllMocks(); @@ -1225,13 +1227,14 @@ describe("grouped chat rendering", () => { expect(avatar?.tagName).toBe("DIV"); }); - it("renders a durable sender label in the user message metadata", () => { + it("renders a durable sender label and avatar chip in user message metadata", async () => { const container = document.createElement("div"); const group: MessageGroup = { kind: "group", key: "attributed-user-group", role: "user", senderLabel: "alice", + sender: { id: "profile-1", name: "Alice Example" }, messages: [ { key: "attributed-user-message", @@ -1256,6 +1259,11 @@ describe("grouped chat rendering", () => { expect( container.querySelector(".chat-group.user .chat-sender-name")?.textContent, ).toBe("alice"); + await vi.waitFor(() => { + expect( + container.querySelector(".chat-author-avatar__initials")?.textContent?.trim(), + ).toBe("AE"); + }); }); it("renders an author avatar for a user group with sender identity", async () => { @@ -1305,7 +1313,7 @@ describe("grouped chat rendering", () => { senderLabel: "alice", // profileAvatarUrl exercises the img tier; bare emails render initials // only (no third-party avatar fetch without a gateway proxy base). - sender: { id: "alice@example.com", profileAvatarUrl: "/avatars/alice.png" }, + sender: { id: "alice@example.com", profileAvatarUrl: "/api/users/alice/avatar" }, messages: [ { key: "gravatar-message", @@ -1327,6 +1335,7 @@ describe("grouped chat rendering", () => { const image = await vi.waitFor(() => { const result = container.querySelector(".chat-author-avatar__image"); expect(result).not.toBeNull(); + expect(result?.getAttribute("src")).toBe("/api/users/alice/avatar"); return result!; }); image.dispatchEvent(new Event("error")); diff --git a/ui/src/pages/chat/components/chat-thread.ts b/ui/src/pages/chat/components/chat-thread.ts index eb2ce22b29a..59d6652e673 100644 --- a/ui/src/pages/chat/components/chat-thread.ts +++ b/ui/src/pages/chat/components/chat-thread.ts @@ -120,6 +120,7 @@ type ChatThreadProps = { /** Host context resolving global-alias session keys (scope=global fleets). */ /** Includes assistantAgentId so bare-global welcome recents scope to the selected agent. */ sessionHost?: UiSessionDefaultsHost | null; + gatewayUrl?: string; assistantName: string; assistantAvatar: string | null; assistantAvatarUrl?: string | null; @@ -1254,6 +1255,7 @@ function renderChatThreadContents( Math.floor(Date.now() / 60_000), getToolTitlesVersion(), props.sessionKey, + props.gatewayUrl, props.fullMessageAgentId, showReasoning, props.showToolCalls, diff --git a/ui/src/pages/profile/avatar-processing.test.ts b/ui/src/pages/profile/avatar-processing.test.ts index 89b23cbafa5..742c31b94d4 100644 --- a/ui/src/pages/profile/avatar-processing.test.ts +++ b/ui/src/pages/profile/avatar-processing.test.ts @@ -44,7 +44,7 @@ describe("profile avatar processing", () => { ).rejects.toMatchObject({ code: "too-large" } satisfies Partial); }); - it("does not upscale smaller uploads through the upload surface", async () => { + it("center-crops without upscaling smaller uploads through the upload surface", async () => { class StubImage { decoding = "auto"; src = ""; @@ -67,7 +67,7 @@ describe("profile avatar processing", () => { await processProfileAvatar(new File(["source"], "avatar.png", { type: "image/png" })); - expect(drawImage).toHaveBeenCalledWith(expect.any(StubImage), 0, 0, 80, 60); + expect(drawImage).toHaveBeenCalledWith(expect.any(StubImage), 10, 0, 60, 60, 0, 0, 60, 60); }); it("decodes, downsizes, and encodes an uploaded image before the RPC payload", async () => { @@ -98,7 +98,7 @@ describe("profile avatar processing", () => { new File(["source"], "avatar.jpg", { type: "image/jpeg" }), ); - expect(drawImage).toHaveBeenCalledWith(expect.any(StubImage), 0, 0, 256, 128); + expect(drawImage).toHaveBeenCalledWith(expect.any(StubImage), 256, 0, 512, 512, 0, 0, 512, 512); expect(result).toEqual({ mime: "image/png", avatarBase64: "AQID", byteLength: 3 }); expect(revokeObjectURL).toHaveBeenCalledWith("blob:avatar"); }); diff --git a/ui/src/pages/profile/avatar-processing.ts b/ui/src/pages/profile/avatar-processing.ts index 97afbf53883..8757fc73ec1 100644 --- a/ui/src/pages/profile/avatar-processing.ts +++ b/ui/src/pages/profile/avatar-processing.ts @@ -1,5 +1,6 @@ -const MAX_PROFILE_AVATAR_EDGE = 256; +const MAX_PROFILE_AVATAR_EDGE = 512; const MAX_PROFILE_AVATAR_BYTES = 512 * 1024; +const MAX_PROFILE_AVATAR_BASE64_CHARS = 700_000; const MAX_PROFILE_AVATAR_SOURCE_BYTES = 10 * 1024 * 1024; type ProcessedProfileAvatar = { @@ -19,10 +20,13 @@ function fitAvatarDimensions(width: number, height: number) { if (!Number.isFinite(width) || !Number.isFinite(height) || width <= 0 || height <= 0) { throw new ProfileAvatarError("invalid-image"); } - const scale = Math.min(1, MAX_PROFILE_AVATAR_EDGE / Math.max(width, height)); + const sourceEdge = Math.min(width, height); + const scale = Math.min(1, MAX_PROFILE_AVATAR_EDGE / sourceEdge); return { - width: Math.max(1, Math.round(width * scale)), - height: Math.max(1, Math.round(height * scale)), + sourceEdge, + sourceX: Math.max(0, Math.round((width - sourceEdge) / 2)), + sourceY: Math.max(0, Math.round((height - sourceEdge) / 2)), + edge: Math.max(1, Math.round(sourceEdge * scale)), }; } @@ -67,7 +71,11 @@ async function encodeAvatarBlob( throw new ProfileAvatarError("too-large"); } const bytes = new Uint8Array(await blob.arrayBuffer()); - return { mime, avatarBase64: bytesToBase64(bytes), byteLength: bytes.byteLength }; + const avatarBase64 = bytesToBase64(bytes); + if (avatarBase64.length > MAX_PROFILE_AVATAR_BASE64_CHARS) { + throw new ProfileAvatarError("too-large"); + } + return { mime, avatarBase64, byteLength: bytes.byteLength }; } export async function processProfileAvatar(file: File): Promise { @@ -80,13 +88,23 @@ export async function processProfileAvatar(file: File): Promise[0]; @@ -35,6 +36,8 @@ function createProps(overrides: Partial = {}): IdentitySec describe("renderIdentitySection", () => { afterEach(() => { document.body.replaceChildren(); + setAvatarGatewayOrigin(null); + vi.restoreAllMocks(); }); it("renders the resolved profile through settings rows and the shared avatar", async () => { @@ -42,7 +45,13 @@ describe("renderIdentitySection", () => { document.body.append(container); render(renderIdentitySection(createProps()), container); const avatar = container.querySelector("openclaw-viewer-avatar"); - await (avatar as (HTMLElement & { updateComplete?: Promise }) | null)?.updateComplete; + await vi.waitFor(async () => { + await (avatar as (HTMLElement & { updateComplete?: Promise }) | null) + ?.updateComplete; + expect(avatar?.querySelector("img")?.getAttribute("src")).toBe( + "/api/users/profile-1/avatar?v=2", + ); + }); expect(container.querySelector("#settings-profile-identity")).not.toBeNull(); expect( @@ -50,13 +59,10 @@ describe("renderIdentitySection", () => { node.textContent?.trim(), ), ).toEqual(["Avatar", "Display name", "Linked emails"]); - expect(avatar?.querySelector("img")?.getAttribute("src")).toBe( - "/api/users/profile-1/avatar?v=2", - ); expect(container.textContent).toContain("ada@example.test, ada@work.test"); }); - it("uses the linked email fallback when the profile has no upload", async () => { + it("falls back to initials when no same-origin avatar route is available", async () => { const container = document.createElement("div"); document.body.append(container); render( @@ -69,13 +75,16 @@ describe("renderIdentitySection", () => { container, ); const avatar = container.querySelector("openclaw-viewer-avatar") as - | (HTMLElement & { updateComplete: Promise }) + | (HTMLElement & { updateComplete?: Promise }) | null; + await avatar?.updateComplete; - await vi.waitFor(() => expect(avatar?.querySelector("img")).not.toBeNull()); - expect(avatar?.querySelector("img")?.getAttribute("src")).toMatch( - /^https:\/\/gravatar\.com\/avatar\/[a-f0-9]{64}\?d=404&s=128$/u, - ); + // The gateway route (userProfileAvatarUrl) serves the Gravatar fallback + // server-side and stays same-origin under the Control UI CSP. When no route + // is available — e.g. a cross-origin gateway returns null — the chip shows + // deterministic initials rather than a CSP-blocked direct gravatar.com image. + expect(avatar?.querySelector("img")).toBeNull(); + expect(avatar?.textContent?.trim()).toBe("AL"); }); it("edits and saves the display name with the standard input pattern", () => { diff --git a/ui/src/pages/profile/profile-page.ts b/ui/src/pages/profile/profile-page.ts index f1a9087ddaf..a928e85e94f 100644 --- a/ui/src/pages/profile/profile-page.ts +++ b/ui/src/pages/profile/profile-page.ts @@ -447,13 +447,13 @@ export class ProfilePage extends OpenClawLightDomElement { )} `; } - const avatarUrl = this.ownProfile.hasAvatar - ? userProfileAvatarUrl( - this.context.gateway.connection.gatewayUrl, - this.ownProfile.id, - this.ownProfile.updatedAt, - ) - : null; + // The gateway route serves an uploaded avatar first and its private Gravatar + // fallback second, while a 404 still leaves the viewer-avatar initials visible. + const avatarUrl = userProfileAvatarUrl( + this.context.gateway.connection.gatewayUrl, + this.ownProfile.id, + this.ownProfile.updatedAt, + ); return renderIdentitySection({ profile: this.ownProfile, avatarUrl, diff --git a/ui/src/styles/layout.css b/ui/src/styles/layout.css index 5014b6e8f60..13d2412061e 100644 --- a/ui/src/styles/layout.css +++ b/ui/src/styles/layout.css @@ -1919,6 +1919,18 @@ html.openclaw-native-macos height: 100%; } +.viewer-avatar__fallback { + display: none; +} + +.viewer-avatar.is-fallback > img { + display: none; +} + +.viewer-avatar.is-fallback > .viewer-avatar__fallback { + display: grid; +} + .viewer-avatar > img { object-fit: cover; } diff --git a/ui/src/test-helpers/app-sidebar-cases/basics.ts b/ui/src/test-helpers/app-sidebar-cases/basics.ts index f06ebbea84e..e06fc72cc2a 100644 --- a/ui/src/test-helpers/app-sidebar-cases/basics.ts +++ b/ui/src/test-helpers/app-sidebar-cases/basics.ts @@ -45,7 +45,7 @@ describe("AppSidebar update card wiring", () => { }); describe("AppSidebar viewer presence", () => { - it("renders the self user's Gravatar in the footer identity chip", async () => { + it("renders the self user's avatar route in the footer identity chip", async () => { const client = { instanceId: "self-instance" } as GatewayBrowserClient; const gatewayHarness = createGatewayHarness(client); const { sidebar } = await mountSidebar( @@ -58,7 +58,16 @@ describe("AppSidebar viewer presence", () => { presence: [ { instanceId: "self-instance", - user: { id: "00-self", email: "test@example.com", name: "Self User" }, + // Presence publishes the canonical gateway avatar route; the gateway + // serves an uploaded avatar or its Gravatar fallback behind it, so the + // chip renders that same-origin route (CSP-safe) rather than a direct + // gravatar.com URL the Control UI CSP would block. + user: { + id: "00-self", + email: "test@example.com", + name: "Self User", + avatarUrl: "/api/users/00-self/avatar?v=7", + }, }, ], }); @@ -67,9 +76,7 @@ describe("AppSidebar viewer presence", () => { const avatar = sidebar.querySelector( ".sidebar-footer-bar__identity openclaw-viewer-avatar img", ); - expect(avatar?.src).toBe( - "https://gravatar.com/avatar/973dfe463ec85785f5f95af5ba3906eedb2d931c24e69824a89ea65dba4e813b?d=404&s=128", - ); + expect(avatar?.getAttribute("src")).toBe("/api/users/00-self/avatar?v=7"); }); }); @@ -97,7 +104,9 @@ describe("AppSidebar viewer presence", () => { }, { instanceId: "alice-1", - user: { id: "alice", name: "Alice", avatarUrl: "https://example.test/alice.png" }, + // Presence publishes avatars as the canonical gateway route; the + // resolver renders only that, falling back to initials otherwise. + user: { id: "alice", name: "Alice", avatarUrl: "/api/users/alice/avatar" }, watchedSessions: ["agent:main:work"], }, {