-
Notifications
You must be signed in to change notification settings - Fork 7
fix(web): post OG cards no longer crash on webp images #145
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a16f0e1
d932f2e
b704e71
bc63f32
00c3a76
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,131 @@ | ||
| import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; | ||
| import { loadSafeOgAvatar, loadSafeOgPostImage } from "@/lib/og-safe-image"; | ||
|
|
||
| const STORAGE_ORIGIN = "https://test.supabase.co"; | ||
| const AVATAR_URL = `${STORAGE_ORIGIN}/storage/v1/object/public/avatars/user-1/avatar.jpg`; | ||
| const POST_IMAGE_URL = `${STORAGE_ORIGIN}/storage/v1/object/public/post-images/user-1/hero.jpg`; | ||
|
|
||
| const PNG = Buffer.concat([ | ||
| Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]), | ||
| Buffer.alloc(16), | ||
| ]); | ||
| const WEBP = Buffer.concat([ | ||
| Buffer.from("RIFF"), | ||
| Buffer.from([0x24, 0x00, 0x00, 0x00]), | ||
| Buffer.from("WEBPVP8 "), | ||
| Buffer.alloc(16), | ||
| ]); | ||
|
|
||
| function imageResponse(body: Buffer, init: ResponseInit = {}): Response { | ||
| return new Response(new Uint8Array(body), init); | ||
| } | ||
|
|
||
| let fetchMock: ReturnType<typeof vi.fn>; | ||
|
|
||
| beforeEach(() => { | ||
| vi.stubEnv("NEXT_PUBLIC_SUPABASE_URL", STORAGE_ORIGIN); | ||
| fetchMock = vi.fn(); | ||
| vi.stubGlobal("fetch", fetchMock); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| vi.unstubAllEnvs(); | ||
| vi.unstubAllGlobals(); | ||
| }); | ||
|
|
||
| describe("loadSafeOgAvatar / loadSafeOgPostImage", () => { | ||
| it("returns a data URI for a satori-decodable first-party image", async () => { | ||
| fetchMock.mockResolvedValue(imageResponse(PNG)); | ||
|
|
||
| await expect(loadSafeOgAvatar(AVATAR_URL)).resolves.toBe( | ||
| `data:image/png;base64,${PNG.toString("base64")}`, | ||
| ); | ||
| }); | ||
|
|
||
| it("returns null for a webp stored under a .jpg extension", async () => { | ||
| // The exact production failure: satori throws "Unsupported image type: | ||
| // image/webp" and takes the whole card render down with it. | ||
| fetchMock.mockResolvedValue(imageResponse(WEBP)); | ||
|
|
||
| await expect(loadSafeOgPostImage(POST_IMAGE_URL)).resolves.toBeNull(); | ||
| }); | ||
|
|
||
| it("never fetches a URL outside the storage allowlist", async () => { | ||
| // avatar_url is written verbatim by PATCH /api/users/me, so an attacker | ||
| // controls it end to end. Fetching it server-side would be an SSRF probe. | ||
| await expect( | ||
| loadSafeOgAvatar("http://169.254.169.254/latest/meta-data/"), | ||
| ).resolves.toBeNull(); | ||
| await expect( | ||
| loadSafeOgAvatar("https://attacker.example.com/pixel.png"), | ||
| ).resolves.toBeNull(); | ||
| await expect( | ||
| loadSafeOgPostImage( | ||
| `${STORAGE_ORIGIN}/storage/v1/object/public/dm-attachments/user-1/x.png`, | ||
| ), | ||
| ).resolves.toBeNull(); | ||
|
|
||
| expect(fetchMock).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("allows the avatar CDN hosts the profile flow accepts", async () => { | ||
| fetchMock.mockResolvedValue(imageResponse(PNG)); | ||
|
|
||
| await expect( | ||
| loadSafeOgAvatar("https://avatars.githubusercontent.com/u/1?v=4"), | ||
| ).resolves.toContain("data:image/png;base64,"); | ||
| }); | ||
|
|
||
| it("rejects a post hero image served from an avatar CDN host", async () => { | ||
| await expect( | ||
| loadSafeOgPostImage("https://avatars.githubusercontent.com/u/1?v=4"), | ||
| ).resolves.toBeNull(); | ||
| expect(fetchMock).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("returns null on a non-ok response", async () => { | ||
| fetchMock.mockResolvedValue(new Response("nope", { status: 404 })); | ||
|
|
||
| await expect(loadSafeOgAvatar(AVATAR_URL)).resolves.toBeNull(); | ||
| }); | ||
|
|
||
| it("returns null when the fetch rejects (timeout, DNS, reset)", async () => { | ||
| fetchMock.mockRejectedValue(new DOMException("timed out", "TimeoutError")); | ||
|
|
||
| await expect(loadSafeOgAvatar(AVATAR_URL)).resolves.toBeNull(); | ||
| }); | ||
|
|
||
| it("aborts the fetch on a deadline instead of holding the render open", async () => { | ||
| fetchMock.mockResolvedValue(imageResponse(PNG)); | ||
|
|
||
| await loadSafeOgAvatar(AVATAR_URL); | ||
|
|
||
| const init = fetchMock.mock.calls[0]?.[1] as RequestInit | undefined; | ||
| expect(init?.signal).toBeInstanceOf(AbortSignal); | ||
| }); | ||
|
|
||
| it("refuses an oversized image declared by content-length without buffering it", async () => { | ||
| fetchMock.mockResolvedValue( | ||
| imageResponse(PNG, { | ||
| headers: { "content-length": String(50 * 1024 * 1024) }, | ||
| }), | ||
| ); | ||
|
|
||
| await expect(loadSafeOgAvatar(AVATAR_URL)).resolves.toBeNull(); | ||
| }); | ||
|
|
||
| it("refuses an oversized image that lied about (or omitted) content-length", async () => { | ||
| const huge = Buffer.concat([PNG, Buffer.alloc(6 * 1024 * 1024)]); | ||
| fetchMock.mockResolvedValue(imageResponse(huge)); | ||
|
|
||
| await expect(loadSafeOgAvatar(AVATAR_URL)).resolves.toBeNull(); | ||
| }); | ||
|
|
||
| it("returns null for empty input without fetching", async () => { | ||
| await expect(loadSafeOgAvatar(null)).resolves.toBeNull(); | ||
| await expect(loadSafeOgAvatar(undefined)).resolves.toBeNull(); | ||
| await expect(loadSafeOgPostImage("")).resolves.toBeNull(); | ||
|
|
||
| expect(fetchMock).not.toHaveBeenCalled(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,79 @@ | ||||||||||||||||||||||||||||||||||||||||||
| import { isAllowedAvatarUrl, isFirstPartyPublicStorageUrl } from "@/lib/storage"; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| // Satori (next/og) only decodes PNG, JPEG, and GIF. User-uploaded images can | ||||||||||||||||||||||||||||||||||||||||||
| // be webp/avif regardless of file extension, which makes the whole OG image | ||||||||||||||||||||||||||||||||||||||||||
| // render throw. Fetch the image, sniff its real format from magic bytes, and | ||||||||||||||||||||||||||||||||||||||||||
| // return a data URI satori can decode — or null so callers fall back to their | ||||||||||||||||||||||||||||||||||||||||||
| // no-image layout. | ||||||||||||||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||||||||||||||
| // Both loaders fetch a URL that ultimately comes from user-controlled columns | ||||||||||||||||||||||||||||||||||||||||||
| // (`users.avatar_url` is written verbatim by PATCH /api/users/me), so each one | ||||||||||||||||||||||||||||||||||||||||||
| // applies the same allowlist the sibling /api/posts/[id]/share-image route | ||||||||||||||||||||||||||||||||||||||||||
| // applies before handing a URL to satori. Anything outside the allowlist is | ||||||||||||||||||||||||||||||||||||||||||
| // never fetched. | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| /** Satori decodes nothing larger than this comfortably, and a data URI costs | ||||||||||||||||||||||||||||||||||||||||||
| * ~4/3 of the raw bytes in memory. Real avatars/hero images are well under. */ | ||||||||||||||||||||||||||||||||||||||||||
| const MAX_IMAGE_BYTES = 5 * 1024 * 1024; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| /** OG cards are rendered on request; a slow third-party host must not hold the | ||||||||||||||||||||||||||||||||||||||||||
| * function open until the platform timeout kills the whole render. */ | ||||||||||||||||||||||||||||||||||||||||||
| const FETCH_TIMEOUT_MS = 3_000; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| /** Avatars may live in first-party storage or on the three avatar CDNs the | ||||||||||||||||||||||||||||||||||||||||||
| * profile flow accepts. */ | ||||||||||||||||||||||||||||||||||||||||||
| export function loadSafeOgAvatar( | ||||||||||||||||||||||||||||||||||||||||||
| url: string | null | undefined, | ||||||||||||||||||||||||||||||||||||||||||
| ): Promise<string | null> { | ||||||||||||||||||||||||||||||||||||||||||
| return loadSafeOgImage(url, isAllowedAvatarUrl); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| /** Post hero images are only ever uploaded to the post-images bucket. */ | ||||||||||||||||||||||||||||||||||||||||||
| export function loadSafeOgPostImage( | ||||||||||||||||||||||||||||||||||||||||||
| url: string | null | undefined, | ||||||||||||||||||||||||||||||||||||||||||
| ): Promise<string | null> { | ||||||||||||||||||||||||||||||||||||||||||
| return loadSafeOgImage(url, (candidate) => | ||||||||||||||||||||||||||||||||||||||||||
| isFirstPartyPublicStorageUrl(candidate, "post-images"), | ||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| async function loadSafeOgImage( | ||||||||||||||||||||||||||||||||||||||||||
| url: string | null | undefined, | ||||||||||||||||||||||||||||||||||||||||||
| isAllowed: (url: string) => boolean, | ||||||||||||||||||||||||||||||||||||||||||
| ): Promise<string | null> { | ||||||||||||||||||||||||||||||||||||||||||
| if (typeof url !== "string" || !url || !isAllowed(url)) return null; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||
| const res = await fetch(url, { | ||||||||||||||||||||||||||||||||||||||||||
| signal: AbortSignal.timeout(FETCH_TIMEOUT_MS), | ||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||
| if (!res.ok) return null; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| const declaredLength = Number(res.headers.get("content-length")); | ||||||||||||||||||||||||||||||||||||||||||
| if (Number.isFinite(declaredLength) && declaredLength > MAX_IMAGE_BYTES) { | ||||||||||||||||||||||||||||||||||||||||||
| return null; | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| const buf = Buffer.from(await res.arrayBuffer()); | ||||||||||||||||||||||||||||||||||||||||||
| // Re-check: content-length is advisory and absent on chunked responses. | ||||||||||||||||||||||||||||||||||||||||||
| if (buf.byteLength > MAX_IMAGE_BYTES) return null; | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+57
to
+59
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win Enforce the byte cap while reading the response. Line 57 buffers the complete body before Line 59 checks its size. A chunked response with no Read Proposed fix- const buf = Buffer.from(await res.arrayBuffer());
- // Re-check: content-length is advisory and absent on chunked responses.
- if (buf.byteLength > MAX_IMAGE_BYTES) return null;
+ const reader = res.body?.getReader();
+ if (!reader) return null;
+
+ const chunks: Uint8Array[] = [];
+ let receivedBytes = 0;
+ for (;;) {
+ const { done, value } = await reader.read();
+ if (done) break;
+ receivedBytes += value.byteLength;
+ if (receivedBytes > MAX_IMAGE_BYTES) {
+ await reader.cancel();
+ return null;
+ }
+ chunks.push(value);
+ }
+
+ const buf = Buffer.concat(chunks, receivedBytes);📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| const mime = sniffMime(buf); | ||||||||||||||||||||||||||||||||||||||||||
| if (!mime) return null; | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| return `data:${mime};base64,${buf.toString("base64")}`; | ||||||||||||||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||||||||||||||
| return null; | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| function sniffMime(buf: Buffer): string | null { | ||||||||||||||||||||||||||||||||||||||||||
| if (buf.length < 12) return null; | ||||||||||||||||||||||||||||||||||||||||||
| if (buf[0] === 0x89 && buf[1] === 0x50 && buf[2] === 0x4e && buf[3] === 0x47) | ||||||||||||||||||||||||||||||||||||||||||
| return "image/png"; | ||||||||||||||||||||||||||||||||||||||||||
| if (buf[0] === 0xff && buf[1] === 0xd8 && buf[2] === 0xff) | ||||||||||||||||||||||||||||||||||||||||||
| return "image/jpeg"; | ||||||||||||||||||||||||||||||||||||||||||
| if (buf[0] === 0x47 && buf[1] === 0x49 && buf[2] === 0x46) | ||||||||||||||||||||||||||||||||||||||||||
| return "image/gif"; | ||||||||||||||||||||||||||||||||||||||||||
| return null; | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: ohong/straude
Length of output: 260
🏁 Script executed:
Repository: ohong/straude
Length of output: 41223
🌐 Web query:
WHATWG Fetch default redirect mode follows redirect behavior URL💡 Result:
The default redirect mode in the WHATWG Fetch Standard is "follow" [1][2]. A request object in the Fetch API has an associated redirect mode, which determines how the browser handles HTTP redirects [1]. The available modes are: - follow: Automatically follows all redirects incurred when fetching a resource [1][3]. This is the default behavior when no other mode is specified [1][2]. - error: Returns a network error if the request encounters a redirect [1][3]. - manual: Retrieves an opaque-redirect filtered response when a request is met with a redirect [1][2]. This mode allows service workers to handle redirects manually [2]. Unless explicitly stated otherwise, a request defaults to "follow" [1][2].
Citations:
Block redirects to unvalidated targets.
fetchfollows redirects by default, soisAllowedchecks only the initial URL while later target URLs are not validated. This can let an allowed avatar or post image redirect to a host outside the safety policy before this function checks the response.Use
redirect: "error"here; existing callers already use null fallbacks when image loading fails.🤖 Prompt for AI Agents