From a2ff6cb52bc2a3f62424e4564f35dd3155b7d6b6 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 27 Jul 2026 05:00:58 -0400 Subject: [PATCH] fix(ui): consistently load and cache user avatars (#114444) * fix(ui): consistently load and cache user avatars * fix(ui): retain avatar images across profile updates --- ui/src/app/gateway-store.ts | 17 +- ui/src/app/user-profile.test.ts | 2 +- ui/src/app/user-profile.ts | 9 +- ui/src/components/viewer-facepile.test.ts | 58 ++++++ ui/src/components/viewer-facepile.ts | 45 +++-- ui/src/e2e/profile-page.e2e.test.ts | 99 ++++++++-- ui/src/lib/identity-avatar.test.ts | 180 +++++++++++++++++- ui/src/lib/identity-avatar.ts | 151 ++++++++++++++- ui/src/pages/chat/chat-avatar.test.ts | 54 ++++++ ui/src/pages/chat/chat-avatar.ts | 33 +++- .../chat/components/chat-author-avatar.ts | 36 +++- 11 files changed, 617 insertions(+), 67 deletions(-) diff --git a/ui/src/app/gateway-store.ts b/ui/src/app/gateway-store.ts index 5e204365a6c1..d576d4837a73 100644 --- a/ui/src/app/gateway-store.ts +++ b/ui/src/app/gateway-store.ts @@ -18,6 +18,7 @@ import type { ApplicationGatewayConnection, ApplicationGatewaySnapshot, } from "./context.ts"; +import { resolveControlUiAuthHeader } from "./control-ui-auth.ts"; import { loadSettings, patchSettings, persistSessionToken } from "./settings.ts"; import { readPresenceEntries, resolveSelfPresenceUser } from "./user-profile.ts"; @@ -272,7 +273,13 @@ export function createApplicationGateway( 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); + setAvatarGatewayOrigin( + nextConnection.gatewayUrl, + resolveControlUiAuthHeader({ + settings: { token: nextConnection.token }, + password: nextConnection.password, + }), + ); updateSettings( { gatewayUrl: nextConnection.gatewayUrl, @@ -306,6 +313,14 @@ export function createApplicationGateway( if (client !== nextClient) { return; } + setAvatarGatewayOrigin( + nextConnection.gatewayUrl, + resolveControlUiAuthHeader({ + hello, + settings: { token: nextConnection.token }, + password: nextConnection.password, + }), + ); connection = { ...connection, bootstrapToken: "" }; if (persistConnectionSettings) { settings = loadSettings(); diff --git a/ui/src/app/user-profile.test.ts b/ui/src/app/user-profile.test.ts index 3346166c4e1b..2cf2d9bfceb7 100644 --- a/ui/src/app/user-profile.test.ts +++ b/ui/src/app/user-profile.test.ts @@ -61,6 +61,6 @@ describe("connection user profile helpers", () => { 42, "https://gateway.example.test/control/profile", ), - ).toBeNull(); + ).toBe("https://remote.example.test/api/users/profile-1/avatar?v=42"); }); }); diff --git a/ui/src/app/user-profile.ts b/ui/src/app/user-profile.ts index d3e6744ec938..646f89c51b11 100644 --- a/ui/src/app/user-profile.ts +++ b/ui/src/app/user-profile.ts @@ -58,12 +58,9 @@ export function userProfileAvatarUrl( } else if (url.protocol === "wss:") { url.protocol = "https:"; } - // The authenticated avatar endpoint is HTTP-only and the Control UI CSP - // permits images from its own origin. Cross-origin gateways keep initials. - if ( - !["http:", "https:"].includes(url.protocol) || - url.origin !== new URL(documentHref).origin - ) { + // The shared avatar loader authenticates cross-origin Gateway requests and + // turns their response into a local blob accepted by the Control UI CSP. + if (!["http:", "https:"].includes(url.protocol)) { return null; } url.username = ""; diff --git a/ui/src/components/viewer-facepile.test.ts b/ui/src/components/viewer-facepile.test.ts index 30cea3499945..f3cb375ce3dc 100644 --- a/ui/src/components/viewer-facepile.test.ts +++ b/ui/src/components/viewer-facepile.test.ts @@ -53,6 +53,64 @@ it("renders trusted presence avatar routes directly", async () => { }); }); +it("derives a missing presence avatar from the durable profile id, not the email", async () => { + const profileId = "c3e32452-0467-47e5-aafa-233cd5dae29f"; + const avatar = document.createElement("openclaw-viewer-avatar") as ViewerAvatarElement; + avatar.user = { + id: profileId, + email: "ada@example.test", + name: "Ada Lovelace", + watchedSessions: [], + }; + document.body.append(avatar); + + await vi.waitFor(async () => { + await avatar.updateComplete; + expect(avatar.querySelector("img")?.getAttribute("src")).toBe(`/api/users/${profileId}/avatar`); + }); +}); + +it("shares an authenticated avatar blob between the same user in the roster and profile", async () => { + setAvatarGatewayOrigin("https://gateway.example.test", "Bearer viewer-token"); + const fetchAvatar = vi.spyOn(globalThis, "fetch").mockResolvedValue( + new Response(new Uint8Array([1, 2, 3]), { + headers: { "content-type": "image/png" }, + }), + ); + vi.spyOn(URL, "createObjectURL").mockReturnValue("blob:shared-viewer-avatar"); + const user: PresenceViewer = { + id: "profile-ada", + email: "ada@example.test", + name: "Ada Lovelace", + avatarUrl: "/api/users/profile-ada/avatar?v=7", + watchedSessions: [], + }; + const avatars = Array.from({ length: 2 }, () => { + const avatar = document.createElement("openclaw-viewer-avatar") as ViewerAvatarElement; + avatar.user = user; + document.body.append(avatar); + return avatar; + }); + + await vi.waitFor(async () => { + await Promise.all(avatars.map((avatar) => avatar.updateComplete)); + expect(avatars.map((avatar) => avatar.querySelector("img")?.getAttribute("src"))).toEqual([ + "blob:shared-viewer-avatar", + "blob:shared-viewer-avatar", + ]); + }); + + expect(fetchAvatar).toHaveBeenCalledOnce(); + expect(fetchAvatar).toHaveBeenCalledWith( + "https://gateway.example.test/api/users/profile-ada/avatar?v=7", + expect.objectContaining({ headers: { Authorization: "Bearer viewer-token" } }), + ); + for (const avatar of avatars) { + avatar.querySelector("img")?.dispatchEvent(new Event("load")); + expect(avatar.querySelector(".viewer-avatar")?.classList.contains("is-fallback")).toBe(false); + } +}); + type ViewerFacepileElement = HTMLElement & { presencePayload: unknown; selfInstanceId?: string; diff --git a/ui/src/components/viewer-facepile.ts b/ui/src/components/viewer-facepile.ts index 77974c3ecd38..9b70fab9bbdf 100644 --- a/ui/src/components/viewer-facepile.ts +++ b/ui/src/components/viewer-facepile.ts @@ -1,9 +1,16 @@ import { html, nothing } from "lit"; import { property } from "lit/decorators.js"; +import { live } from "lit/directives/live.js"; +import { until } from "lit/directives/until.js"; import type { PresenceEntry } from "../api/types.ts"; import { CONTROL_UI_BUILD_INFO, type ControlUiBuildInfo } from "../build-info.ts"; import { t } from "../i18n/index.ts"; -import { resolveAvatar } from "../lib/identity-avatar.ts"; +import { + resolveAvatar, + resolveAvatarImageUrl, + settleAvatarImageUrl, + type ResolvedIdentityAvatar, +} from "../lib/identity-avatar.ts"; import { OpenClawLightDomContentsElement } from "../lit/openclaw-element.ts"; import { renderSidebarServerDetails } from "./sidebar-build-chip-format.ts"; import "./tooltip.ts"; @@ -135,28 +142,34 @@ function renderAvatarInitials(user: PresenceViewer) { return html`${initialsFor(user)}`; } -function resolveViewerAvatar(user: PresenceViewer) { - const avatar = resolveAvatar({ - id: user.email ?? user.id, - name: user.name, - profileAvatarUrl: user.avatarUrl, - }); - if (avatar.kind === "initials") { +function renderViewerAvatar( + user: PresenceViewer, + avatar: ResolvedIdentityAvatar, + imageUrl: string | Promise | null, +) { + if (avatar.kind === "initials" || !imageUrl) { return renderAvatarInitials(user); } return html` url ?? nothing), + nothing, + )} alt="" referrerpolicy="no-referrer" @error=${(event: Event) => { const image = event.currentTarget; if (image instanceof HTMLImageElement) { + settleAvatarImageUrl(image.getAttribute("src")); image.closest(".viewer-avatar")?.classList.add("is-fallback"); } }} @load=${(event: Event) => { const image = event.currentTarget; if (image instanceof HTMLImageElement) { + settleAvatarImageUrl(image.getAttribute("src")); image.closest(".viewer-avatar")?.classList.remove("is-fallback"); } }} @@ -178,12 +191,22 @@ class ViewerAvatar extends OpenClawLightDomContentsElement { return nothing; } const label = presenceViewerLabel(user); + const avatar = resolveAvatar({ + id: user.id, + name: user.name, + username: user.email, + profileAvatarUrl: user.avatarUrl, + }); + const imageUrl = avatar.kind === "initials" ? null : resolveAvatarImageUrl(avatar.url); + const pending = imageUrl !== null && typeof imageUrl !== "string"; + // Image events toggle fallback imperatively; live() compares actual DOM so + // a changed avatar restores initials without replacing its existing image. return html` - ${resolveViewerAvatar(user)} + ${renderViewerAvatar(user, avatar, imageUrl)} `; } } diff --git a/ui/src/e2e/profile-page.e2e.test.ts b/ui/src/e2e/profile-page.e2e.test.ts index e0ae62b1a06a..1cefe570f931 100644 --- a/ui/src/e2e/profile-page.e2e.test.ts +++ b/ui/src/e2e/profile-page.e2e.test.ts @@ -3,6 +3,10 @@ import { mkdir } from "node:fs/promises"; import path from "node:path"; import { chromium, type Browser, type Page } from "playwright"; import { afterAll, beforeAll, describe, expect, it } from "vitest"; +import { + buildControlUiCspHeader, + computeInlineScriptHashes, +} from "../../../src/gateway/control-ui-csp.ts"; import { canRunPlaywrightChromium, installMockGateway, @@ -148,6 +152,7 @@ const testPresenceUsers = [ id: testProfile.id, name: testProfile.displayName, email: testProfile.emails[0], + avatarUrl: `/api/users/${testProfile.id}/avatar?v=${testProfile.updatedAt}`, }, ]; @@ -215,9 +220,31 @@ describeControlUiE2e("Control UI profile page mocked Gateway E2E", () => { } }); - it("renders the gateway avatar route in the profile preview", async () => { - const context = await browser.newContext(); + it("shares one authenticated avatar between the sidebar and profile preview", async () => { + if (captureUiProof) { + await mkdir(proofDir, { recursive: true }); + } + const context = await browser.newContext({ + ...(captureUiProof + ? { recordVideo: { dir: proofDir, size: { width: 1280, height: 800 } } } + : {}), + viewport: { width: 1280, height: 800 }, + }); const page = await context.newPage(); + await page.route(`${server.baseUrl}settings/profile`, async (route) => { + const response = await route.fetch(); + const body = await response.text(); + await route.fulfill({ + body, + headers: { + ...response.headers(), + "content-security-policy": buildControlUiCspHeader({ + inlineScriptHashes: computeInlineScriptHashes(body), + }), + }, + response, + }); + }); const gatewayUrl = server.baseUrl.replace(/^http/u, "ws").replace(/\/$/u, ""); await page.addInitScript((sameOriginGatewayUrl) => { ( @@ -229,16 +256,20 @@ describeControlUiE2e("Control UI profile page mocked Gateway E2E", () => { token: "test", }; }, gatewayUrl); - 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. + const avatarRequests: Array<{ authorization?: string; url: string }> = []; + // Profile images require the same bearer auth as gateway RPCs. One cached + // blob keeps the sidebar and preview inside the Control UI's image CSP. await page.route(`**/api/users/${testProfile.id}/avatar*`, async (route) => { - avatarRequests.push(route.request().url()); + avatarRequests.push({ + authorization: route.request().headers().authorization, + url: route.request().url(), + }); await route.fulfill({ - body: '', - contentType: "image/svg+xml", + body: Buffer.from( + "iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mP8/x8AAwMCAO+/a6kAAAAASUVORK5CYII=", + "base64", + ), + contentType: "image/png", status: 200, }); }); @@ -254,19 +285,49 @@ describeControlUiE2e("Control UI profile page mocked Gateway E2E", () => { try { const response = await page.goto(`${server.baseUrl}settings/profile`); expect(response?.status()).toBe(200); + expect(response?.headers()["content-security-policy"]).toContain( + "img-src 'self' data: blob:", + ); const profileAvatar = page.locator("#settings-profile-identity openclaw-viewer-avatar img"); await profileAvatar.waitFor({ timeout: 10_000 }); - // 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")).toContain( - `/api/users/${testProfile.id}/avatar?v=2`, - ); + const imageUrl = await profileAvatar.getAttribute("src"); + expect(imageUrl).toMatch(/^blob:/u); await expect - .poll(() => - avatarRequests.some((url) => url.includes(`/api/users/${testProfile.id}/avatar`)), - ) - .toBe(true); + .poll(() => profileAvatar.evaluate((image) => (image as HTMLImageElement).naturalWidth)) + .toBe(1); + expect( + await profileAvatar.evaluate((image) => + image.closest(".viewer-avatar")?.classList.contains("is-fallback"), + ), + ).toBe(false); + if (captureUiProof) { + await page.screenshot({ + animations: "disabled", + path: path.join(proofDir, "03-authenticated-profile-avatar.png"), + }); + } + + await page.getByRole("button", { name: "Back to app" }).click(); + const sidebarAvatar = page.locator(".sidebar-identity-card openclaw-viewer-avatar img"); + await sidebarAvatar.waitFor({ timeout: 10_000 }); + await expect.poll(() => avatarRequests.length).toBe(1); + expect(avatarRequests[0]).toEqual({ + authorization: "Bearer e2e-device-token", + url: expect.stringContaining(`/api/users/${testProfile.id}/avatar?v=2`), + }); + expect(await sidebarAvatar.getAttribute("src")).toBe(imageUrl); + expect( + await sidebarAvatar.evaluate((image) => + image.closest(".viewer-avatar")?.classList.contains("is-fallback"), + ), + ).toBe(false); + if (captureUiProof) { + await page.screenshot({ + animations: "disabled", + path: path.join(proofDir, "03-authenticated-user-avatar-cache.png"), + }); + } } finally { await context.close(); } diff --git a/ui/src/lib/identity-avatar.test.ts b/ui/src/lib/identity-avatar.test.ts index db6e38060b65..7cf4a19f292c 100644 --- a/ui/src/lib/identity-avatar.test.ts +++ b/ui/src/lib/identity-avatar.test.ts @@ -1,9 +1,17 @@ // @vitest-environment node -import { afterEach, describe, expect, it } from "vitest"; -import { resolveAvatar, resolveIdentityHue, setAvatarGatewayOrigin } from "./identity-avatar.ts"; +import { afterEach, describe, expect, it, vi } from "vitest"; +import { + resolveAvatar, + resolveAvatarImageUrl, + resolveIdentityHue, + setAvatarGatewayOrigin, + settleAvatarImageUrl, +} from "./identity-avatar.ts"; afterEach(() => { setAvatarGatewayOrigin(null); + vi.restoreAllMocks(); + vi.unstubAllGlobals(); }); describe("resolveAvatar", () => { @@ -170,3 +178,171 @@ describe("resolveAvatar profile-id senders", () => { ).toEqual({ kind: "profile", url: "/api/users/other-profile/avatar?v=9" }); }); }); + +describe("authenticated profile avatar cache", () => { + it("shares one authenticated image fetch and blob across avatar surfaces", async () => { + setAvatarGatewayOrigin("wss://gateway.example.test/ws", "Bearer profile-token"); + const fetchAvatar = vi.spyOn(globalThis, "fetch").mockResolvedValue( + new Response(new Uint8Array([1, 2, 3]), { + headers: { "content-type": "image/png" }, + }), + ); + const createObjectURL = vi.spyOn(URL, "createObjectURL").mockReturnValue("blob:profile-ada"); + + const first = resolveAvatarImageUrl("/api/users/profile-ada/avatar?v=7"); + const second = resolveAvatarImageUrl("/api/users/profile-ada/avatar?v=7"); + + expect(first).toBe(second); + await expect(first).resolves.toBe("blob:profile-ada"); + await expect(second).resolves.toBe("blob:profile-ada"); + expect(fetchAvatar).toHaveBeenCalledOnce(); + expect(fetchAvatar).toHaveBeenCalledWith( + "https://gateway.example.test/api/users/profile-ada/avatar?v=7", + expect.objectContaining({ + credentials: "include", + headers: { Authorization: "Bearer profile-token" }, + signal: expect.any(AbortSignal), + }), + ); + expect(createObjectURL).toHaveBeenCalledOnce(); + }); + + it("refetches when the gateway publishes a newer avatar revision", async () => { + setAvatarGatewayOrigin("https://gateway.example.test", "Bearer profile-token"); + const fetchAvatar = vi.spyOn(globalThis, "fetch").mockImplementation( + async () => + new Response(new Uint8Array([1, 2, 3]), { + headers: { "content-type": "image/png" }, + }), + ); + vi.spyOn(URL, "createObjectURL") + .mockReturnValueOnce("blob:profile-v7") + .mockReturnValueOnce("blob:profile-v8"); + + await expect(resolveAvatarImageUrl("/api/users/profile-ada/avatar?v=7")).resolves.toBe( + "blob:profile-v7", + ); + await expect(resolveAvatarImageUrl("/api/users/profile-ada/avatar?v=8")).resolves.toBe( + "blob:profile-v8", + ); + + expect(fetchAvatar).toHaveBeenCalledTimes(2); + }); + + it("does not cache a missing avatar or a transient image failure", async () => { + setAvatarGatewayOrigin("https://gateway.example.test", "Bearer profile-token"); + const fetchAvatar = vi + .spyOn(globalThis, "fetch") + .mockResolvedValueOnce(new Response(null, { status: 404 })) + .mockResolvedValueOnce( + new Response(new Uint8Array([1, 2, 3]), { + headers: { "content-type": "image/png" }, + }), + ); + vi.spyOn(URL, "createObjectURL").mockReturnValue("blob:profile-uploaded"); + + await expect(resolveAvatarImageUrl("/api/users/profile-ada/avatar")).resolves.toBeNull(); + await expect(resolveAvatarImageUrl("/api/users/profile-ada/avatar")).resolves.toBe( + "blob:profile-uploaded", + ); + + expect(fetchAvatar).toHaveBeenCalledTimes(2); + }); + + it("keeps active avatar requests valid when a roster exceeds the cache limit", async () => { + setAvatarGatewayOrigin("https://gateway.example.test", "Bearer profile-token"); + const finishRequests: Array<(response: Response) => void> = []; + const fetchAvatar = vi.spyOn(globalThis, "fetch").mockImplementation( + async () => + await new Promise((resolve) => { + finishRequests.push(resolve); + }), + ); + let blobIndex = 0; + vi.spyOn(URL, "createObjectURL").mockImplementation(() => `blob:profile-${blobIndex++}`); + const revokeObjectURL = vi.spyOn(URL, "revokeObjectURL"); + + const pending = Array.from({ length: 130 }, (_, index) => + Promise.resolve(resolveAvatarImageUrl(`/api/users/profile-${index}/avatar?v=1`)), + ); + expect(fetchAvatar).toHaveBeenCalledTimes(130); + for (const finishRequest of finishRequests) { + finishRequest( + new Response(new Uint8Array([1, 2, 3]), { + headers: { "content-type": "image/png" }, + }), + ); + } + + const imageUrls = await Promise.all(pending); + expect(imageUrls).toHaveLength(130); + expect(imageUrls.every((url) => url?.startsWith("blob:profile-"))).toBe(true); + expect(new Set(imageUrls).size).toBe(130); + expect(revokeObjectURL).not.toHaveBeenCalled(); + + settleAvatarImageUrl(imageUrls[0] ?? null); + settleAvatarImageUrl(imageUrls[1] ?? null); + + expect(revokeObjectURL).toHaveBeenCalledTimes(2); + expect(revokeObjectURL).toHaveBeenNthCalledWith(1, imageUrls[0]); + expect(revokeObjectURL).toHaveBeenNthCalledWith(2, imageUrls[1]); + }); + + it("rejects non-image responses from the authenticated avatar endpoint", async () => { + setAvatarGatewayOrigin("https://gateway.example.test", "Bearer profile-token"); + vi.spyOn(globalThis, "fetch").mockResolvedValue( + new Response("not an image", { headers: { "content-type": "text/html" } }), + ); + const createObjectURL = vi.spyOn(URL, "createObjectURL"); + + await expect(resolveAvatarImageUrl("/api/users/profile-ada/avatar?v=7")).resolves.toBeNull(); + expect(createObjectURL).not.toHaveBeenCalled(); + }); + + it("revokes cached blobs and refetches after a gateway credential changes", async () => { + setAvatarGatewayOrigin("https://gateway.example.test", "Bearer first-token"); + const fetchAvatar = vi.spyOn(globalThis, "fetch").mockImplementation( + async () => + new Response(new Uint8Array([1, 2, 3]), { + headers: { "content-type": "image/png" }, + }), + ); + vi.spyOn(URL, "createObjectURL") + .mockReturnValueOnce("blob:first-profile") + .mockReturnValueOnce("blob:second-profile"); + const revokeObjectURL = vi.spyOn(URL, "revokeObjectURL"); + + await expect(resolveAvatarImageUrl("/api/users/profile-ada/avatar?v=7")).resolves.toBe( + "blob:first-profile", + ); + setAvatarGatewayOrigin("https://gateway.example.test", "Bearer second-token"); + await expect(resolveAvatarImageUrl("/api/users/profile-ada/avatar?v=7")).resolves.toBe( + "blob:second-profile", + ); + + expect(revokeObjectURL).toHaveBeenCalledWith("blob:first-profile"); + expect(fetchAvatar).toHaveBeenLastCalledWith( + "https://gateway.example.test/api/users/profile-ada/avatar?v=7", + expect.objectContaining({ headers: { Authorization: "Bearer second-token" } }), + ); + }); + + it("never forwards gateway credentials to a sender-controlled avatar origin", () => { + setAvatarGatewayOrigin("https://gateway.example.test", "Bearer profile-token"); + const fetchAvatar = vi.spyOn(globalThis, "fetch"); + + for (const avatarUrl of [ + "https://evil.example/api/users/profile-ada/avatar?v=7", + "https://gateway.example.test.evil.example/api/users/profile-ada/avatar?v=7", + "https://gateway.example.test@evil.example/api/users/profile-ada/avatar?v=7", + "//evil.example/api/users/profile-ada/avatar?v=7", + "/api/secrets", + ]) { + expect(resolveAvatarImageUrl(avatarUrl)).toBeNull(); + expect(resolveAvatar({ id: "profile-ada", profileAvatarUrl: avatarUrl })).toMatchObject({ + kind: "initials", + }); + } + expect(fetchAvatar).not.toHaveBeenCalled(); + }); +}); diff --git a/ui/src/lib/identity-avatar.ts b/ui/src/lib/identity-avatar.ts index ef100a258994..f70424d987ef 100644 --- a/ui/src/lib/identity-avatar.ts +++ b/ui/src/lib/identity-avatar.ts @@ -1,3 +1,4 @@ +import { AVATAR_MAX_BYTES } from "../../../src/shared/avatar-limits.js"; import { formatSenderLabel, type SenderIdentity } from "./chat/sender-label.ts"; import { fnv1aUtf16 } from "./fnv1a.ts"; @@ -11,6 +12,48 @@ export type IdentityAvatarInput = SenderIdentity & { const ORIGIN_PROBE = "https://origin-probe.invalid"; let appGatewayOrigin: string | null = null; +let appGatewayAuthHeader: string | null = null; + +const IDENTITY_AVATAR_CACHE_MAX_ENTRIES = 128; +const IDENTITY_AVATAR_FETCH_TIMEOUT_MS = 30_000; +const IDENTITY_AVATAR_MIME_TYPES = new Set(["image/gif", "image/jpeg", "image/png", "image/webp"]); + +type CachedIdentityAvatar = { + blobUrl: string | null; + loaded: boolean; + promise: Promise; +}; + +const identityAvatarCache = new Map(); + +function clearIdentityAvatarCache(): void { + for (const entry of identityAvatarCache.values()) { + if (entry.blobUrl) { + URL.revokeObjectURL(entry.blobUrl); + } + } + identityAvatarCache.clear(); +} + +function trimIdentityAvatarCache(protectedEntry?: CachedIdentityAvatar): void { + while (identityAvatarCache.size > IDENTITY_AVATAR_CACHE_MAX_ENTRIES) { + let evicted = false; + for (const [key, entry] of identityAvatarCache) { + // Pending consumers still need their eventual blob. Only completed LRU + // entries may be evicted; the request currently resolving stays valid. + if (!entry.blobUrl || !entry.loaded || entry === protectedEntry) { + continue; + } + identityAvatarCache.delete(key); + URL.revokeObjectURL(entry.blobUrl); + evicted = true; + break; + } + if (!evicted) { + break; + } + } +} function toHttpOrigin(url: string | null | undefined): string | null { if (!url) { @@ -26,9 +69,18 @@ function toHttpOrigin(url: string | null | undefined): string | null { } } -/** Records the connected gateway URL so avatar routes resolve to its origin. */ -export function setAvatarGatewayOrigin(gatewayUrl: string | null | undefined): void { - appGatewayOrigin = toHttpOrigin(gatewayUrl); +/** Keeps avatar routes, credentials, and cached images scoped to the current gateway. */ +export function setAvatarGatewayOrigin( + gatewayUrl: string | null | undefined, + authHeader: string | null = null, +): void { + const nextOrigin = toHttpOrigin(gatewayUrl); + const nextAuthHeader = authHeader?.trim() || null; + if (appGatewayOrigin !== nextOrigin || appGatewayAuthHeader !== nextAuthHeader) { + clearIdentityAvatarCache(); + } + appGatewayOrigin = nextOrigin; + appGatewayAuthHeader = nextAuthHeader; } // Mirrors the server's user-profiles-http-path matcher. Sender metadata may @@ -39,10 +91,9 @@ 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. + * replacing an image invalidates its bounded authenticated blob-cache entry. + * Relative paths resolve against the trusted gateway origin; absolute URLs + * must match that origin. */ function toTrustedAvatarUrl(value: string, gatewayOrigin: string | null): string | null { try { @@ -60,9 +111,93 @@ function toTrustedAvatarUrl(value: string, gatewayOrigin: string | null): string } } +function loadIdentityAvatar(url: string): string | Promise { + const cacheKey = url; + const cached = identityAvatarCache.get(cacheKey); + if (cached) { + // Map order is the LRU order; concurrent roster, profile, and chat views + // must share both the authenticated request and its resulting blob. + identityAvatarCache.delete(cacheKey); + identityAvatarCache.set(cacheKey, cached); + return cached.loaded && cached.blobUrl ? cached.blobUrl : cached.promise; + } + + const entry: CachedIdentityAvatar = { + blobUrl: null, + loaded: false, + promise: Promise.resolve(null), + }; + const authHeader = appGatewayAuthHeader; + entry.promise = (async () => { + try { + const response = await fetch(url, { + credentials: "include", + ...(authHeader ? { headers: { Authorization: authHeader } } : {}), + signal: AbortSignal.timeout(IDENTITY_AVATAR_FETCH_TIMEOUT_MS), + }); + if (!response.ok) { + return null; + } + const blob = await response.blob(); + if ( + blob.size === 0 || + blob.size > AVATAR_MAX_BYTES || + !IDENTITY_AVATAR_MIME_TYPES.has(blob.type.toLowerCase()) + ) { + return null; + } + const blobUrl = URL.createObjectURL(blob); + // A gateway or credential change can finish while its old request is in + // flight. Never publish an image into the replacement security context. + if (identityAvatarCache.get(cacheKey) !== entry) { + URL.revokeObjectURL(blobUrl); + return null; + } + entry.blobUrl = blobUrl; + trimIdentityAvatarCache(entry); + return blobUrl; + } catch { + return null; + } finally { + if (!entry.blobUrl && identityAvatarCache.get(cacheKey) === entry) { + // Transient failures and uncached 404s must not hide a later upload. + identityAvatarCache.delete(cacheKey); + } + } + })(); + identityAvatarCache.set(cacheKey, entry); + trimIdentityAvatarCache(entry); + + return entry.promise; +} + +/** Fetch protected or cross-origin profile images once and render CSP-safe blobs. */ +export function resolveAvatarImageUrl(value: string): string | Promise | null { + const trusted = toTrustedAvatarUrl(value, appGatewayOrigin); + if (!trusted) { + return null; + } + const pageOrigin = globalThis.location?.origin; + const crossOrigin = pageOrigin ? new URL(trusted, pageOrigin).origin !== pageOrigin : false; + return appGatewayAuthHeader || crossOrigin ? loadIdentityAvatar(trusted) : trusted; +} + +/** A blob stays live until its image has finished loading or definitively failed. */ +export function settleAvatarImageUrl(value: string | null): void { + if (!value?.startsWith("blob:")) { + return; + } + for (const entry of identityAvatarCache.values()) { + if (entry.blobUrl === value) { + entry.loaded = true; + trimIdentityAvatarCache(); + return; + } + } +} + export type ResolvedIdentityAvatar = | { kind: "profile"; url: string } - | { kind: "gravatar"; url: string } | { kind: "initials"; initials: string; colorSeed: number }; function initialsFromLabel(label: string): string { diff --git a/ui/src/pages/chat/chat-avatar.test.ts b/ui/src/pages/chat/chat-avatar.test.ts index 69c9505436ce..c26c5d062bc3 100644 --- a/ui/src/pages/chat/chat-avatar.test.ts +++ b/ui/src/pages/chat/chat-avatar.test.ts @@ -2,6 +2,7 @@ import { render } from "lit"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { setAvatarGatewayOrigin } from "../../lib/identity-avatar.ts"; import { refreshChatAvatar, renderChatAvatar } from "./chat-avatar.ts"; function renderAvatar(params: Parameters) { @@ -37,7 +38,9 @@ function pendingUntilAbort(signal: AbortSignal | null | undefined): Promise { + setAvatarGatewayOrigin(null); vi.useRealTimers(); + vi.restoreAllMocks(); vi.unstubAllGlobals(); }); @@ -147,6 +150,57 @@ describe("refreshChatAvatar", () => { }); describe("attributed sender avatars", () => { + it("restores pending initials when the authenticated sender avatar changes", async () => { + setAvatarGatewayOrigin("https://gateway.example.test", "Bearer profile-token"); + vi.spyOn(globalThis, "fetch").mockImplementation( + async () => + new Response(new Uint8Array([1, 2, 3]), { + headers: { "content-type": "image/png" }, + }), + ); + vi.spyOn(URL, "createObjectURL") + .mockReturnValueOnce("blob:first-sender") + .mockReturnValueOnce("blob:second-sender"); + const container = document.createElement("div"); + const firstSender = { + id: "c3e32452-0467-47e5-aafa-233cd5dae29f", + name: "Ada Lovelace", + profileAvatarUrl: "/api/users/c3e32452-0467-47e5-aafa-233cd5dae29f/avatar?v=1", + }; + + render(renderChatAvatar("user", undefined, undefined, "", null, firstSender), container); + const firstImage = await vi.waitFor(() => { + const image = container.querySelector(".chat-avatar-slot img"); + expect(image?.getAttribute("src")).toBe("blob:first-sender"); + return image!; + }); + firstImage.dispatchEvent(new Event("load")); + expect(container.querySelector(".chat-avatar-slot")?.classList.contains("is-fallback")).toBe( + false, + ); + + render( + renderChatAvatar("user", undefined, undefined, "", null, { + ...firstSender, + profileAvatarUrl: "/api/users/c3e32452-0467-47e5-aafa-233cd5dae29f/avatar?v=2", + }), + container, + ); + expect(container.querySelector(".chat-avatar-slot")?.classList.contains("is-fallback")).toBe( + true, + ); + const secondImage = await vi.waitFor(() => { + const image = container.querySelector(".chat-avatar-slot img"); + expect(image?.getAttribute("src")).toBe("blob:second-sender"); + return image!; + }); + expect(secondImage).toBe(firstImage); + secondImage.dispatchEvent(new Event("load")); + expect(container.querySelector(".chat-avatar-slot")?.classList.contains("is-fallback")).toBe( + false, + ); + }); + it("renders the sender's profile avatar route for user messages", () => { const avatar = renderAvatar([ "user", diff --git a/ui/src/pages/chat/chat-avatar.ts b/ui/src/pages/chat/chat-avatar.ts index 7ef2d3f4911c..2c80df9d2dc5 100644 --- a/ui/src/pages/chat/chat-avatar.ts +++ b/ui/src/pages/chat/chat-avatar.ts @@ -1,5 +1,7 @@ // Control UI chat module implements chat avatar behavior. -import { html } from "lit"; +import { html, nothing } from "lit"; +import { live } from "lit/directives/live.js"; +import { until } from "lit/directives/until.js"; import type { GatewayHelloOk } from "../../api/gateway.ts"; import { normalizeBasePath } from "../../app-route-paths.ts"; import { resolveControlUiAuthHeader } from "../../app/control-ui-auth.ts"; @@ -19,8 +21,10 @@ import type { SenderIdentity } from "../../lib/chat/sender-label.ts"; import { formatSenderLabel } from "../../lib/chat/sender-label.ts"; import { resolveAvatar, + resolveAvatarImageUrl, resolveAvatarInitials, resolveIdentityHue, + settleAvatarImageUrl, } from "../../lib/identity-avatar.ts"; import { DEFAULT_AGENT_ID, @@ -54,23 +58,34 @@ export function renderChatAvatar( if (resolved.kind === "initials") { return initialsAvatar; } + const imageUrl = resolveAvatarImageUrl(resolved.url); + if (!imageUrl) { + return initialsAvatar; + } // The derived route may 404 (no upload, no Gravatar); swap to initials // instead of a broken image. Lit reuses DOM parts, so a load must clear a // prior sender's error state. - return html` + return html` url ?? nothing), + nothing, + )} alt="${label}" @error=${(event: Event) => { - (event.currentTarget as HTMLElement) - .closest(".chat-avatar-slot") - ?.classList.add("is-fallback"); + const image = event.currentTarget as HTMLImageElement; + settleAvatarImageUrl(image.getAttribute("src")); + image.closest(".chat-avatar-slot")?.classList.add("is-fallback"); }} @load=${(event: Event) => { - (event.currentTarget as HTMLElement) - .closest(".chat-avatar-slot") - ?.classList.remove("is-fallback"); + const image = event.currentTarget as HTMLImageElement; + settleAvatarImageUrl(image.getAttribute("src")); + image.closest(".chat-avatar-slot")?.classList.remove("is-fallback"); }} /> ${initialsAvatar} diff --git a/ui/src/pages/chat/components/chat-author-avatar.ts b/ui/src/pages/chat/components/chat-author-avatar.ts index a7d77a89aa87..92ed5bfde781 100644 --- a/ui/src/pages/chat/components/chat-author-avatar.ts +++ b/ui/src/pages/chat/components/chat-author-avatar.ts @@ -1,8 +1,12 @@ import { html, nothing, type TemplateResult } from "lit"; +import { live } from "lit/directives/live.js"; +import { until } from "lit/directives/until.js"; import { formatSenderLabel } from "../../../lib/chat/sender-label.ts"; import { resolveAvatar, + resolveAvatarImageUrl, resolveAvatarInitials, + settleAvatarImageUrl, type IdentityAvatarInput, type ResolvedIdentityAvatar, } from "../../../lib/identity-avatar.ts"; @@ -24,21 +28,27 @@ function renderInitialsAvatar( } function renderResolvedAvatar( - avatar: ResolvedIdentityAvatar, fallback: Extract, + imageUrl: string | Promise | null, ): TemplateResult { - if (avatar.kind === "initials") { - return renderInitialsAvatar(avatar); + if (!imageUrl) { + return renderInitialsAvatar(fallback); } return html` url ?? nothing), + nothing, + )} alt="" aria-hidden="true" @error=${(event: Event) => { const image = event.currentTarget; if (image instanceof HTMLImageElement) { + settleAvatarImageUrl(image.getAttribute("src")); image.closest(".chat-author-avatar")?.classList.add("is-fallback"); } }} @@ -47,6 +57,7 @@ function renderResolvedAvatar( // must not hide a successfully loaded avatar for the next source. const image = event.currentTarget; if (image instanceof HTMLImageElement) { + settleAvatarImageUrl(image.getAttribute("src")); image.closest(".chat-author-avatar")?.classList.remove("is-fallback"); } }} @@ -65,13 +76,18 @@ export function renderChatAuthorAvatar( } const fallback = resolveAvatarInitials(sender); const avatar = resolveAvatar(sender); + const imageUrl = avatar.kind === "initials" ? null : resolveAvatarImageUrl(avatar.url); + const pending = imageUrl !== null && typeof imageUrl !== "string"; const resolved = avatar.kind === "initials" ? renderInitialsAvatar(avatar) - : renderResolvedAvatar(avatar, fallback); - return html` - - ${resolved} - - `; + : renderResolvedAvatar(fallback, imageUrl); + return html` + ${resolved} + `; }