fix(web): post OG cards no longer crash on webp images - #145
Conversation
Satori can't decode webp; sniff image format server-side and only pass satori-supported formats (PNG/JPEG/GIF) through as data URIs, falling back to the no-image layout otherwise. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughOpenGraph routes now fetch images through scoped safe loaders. The loaders enforce storage allowlists, timeouts, size limits, and magic-byte checks. Valid images become data URIs. Invalid or unsupported images produce no-image fallbacks. ChangesOpenGraph image safety
Changelog updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Merge Risk: ⚪ Minimal · up to The change prevents unsupported or unavailable images from crashing OG card generation by falling back to the existing no-image layout. No actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant OpenGraphRoute
participant SafeImageLoader
participant ApprovedStorage
participant ShareCardImage
participant ImageResponse
OpenGraphRoute->>SafeImageLoader: Load hero image and avatar concurrently
SafeImageLoader->>ApprovedStorage: Fetch approved URLs with limits
ApprovedStorage-->>SafeImageLoader: Return image bytes
SafeImageLoader-->>OpenGraphRoute: Return data URIs or null
OpenGraphRoute->>ShareCardImage: Pass validated assets
ShareCardImage->>ImageResponse: Render OpenGraph card
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Full details: Docstring CoverageExplanation No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (1 skipped: 1 unsupported.) ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
The webp fix fetched whatever URL was in users.avatar_url or posts.images. avatar_url is written verbatim by PATCH /api/users/me, so that fetch was an SSRF probe an authenticated user fully controlled — and the sibling /api/posts/[id]/share-image route already defended against exactly this with a storage allowlist. - Split loadSafeOgImage into loadSafeOgAvatar / loadSafeOgPostImage, each applying the allowlist the share-image route already used. Nothing outside it is fetched at all. - Bound the fetch with a 3s deadline and a 5 MiB cap (content-length plus a post-read byte check, since content-length is advisory). - Apply the loaders to the two other routes that render the same user-supplied images and crash the same way: /api/posts/[id]/share-image and /join/[username]. - Add unit coverage: webp rejection, allowlist enforcement (link-local and third-party hosts, wrong bucket), CDN avatar hosts, non-ok, fetch rejection, abort signal, both size caps. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/web/app/(landing)/join/[username]/opengraph-image.tsx (1)
29-31: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winStart the avatar load before the statistics queries.
Line 31 waits for the avatar fetch before the independent usage and RPC calls start at Line 40. A slow allowed avatar adds its deadline to the route latency.
Store the promise here. Await it immediately before creating
ImageResponse.Proposed fix
- const safeAvatarUrl = await loadSafeOgAvatar(referrer.avatar_url); + const safeAvatarUrlPromise = loadSafeOgAvatar(referrer.avatar_url); ... + const safeAvatarUrl = await safeAvatarUrlPromise; return new ImageResponse(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/app/`(landing)/join/[username]/opengraph-image.tsx around lines 29 - 31, Update the avatar-loading flow around loadSafeOgAvatar to start the request without awaiting it, storing its promise before the independent statistics queries. Await that promise immediately before constructing ImageResponse, while preserving the existing safe avatar value and error-handling behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/lib/og-safe-image.ts`:
- Around line 57-59: Update the response-reading logic around the current
Buffer.from and MAX_IMAGE_BYTES check to consume res.body incrementally, track
accumulated bytes, and cancel the stream and return null immediately when the
cap is exceeded. Preserve successful buffering for responses within the limit,
and update the oversized-body test to verify reading stops once MAX_IMAGE_BYTES
is crossed.
- Around line 47-49: Update the fetch options in the image-loading function
around the visible fetch call to set redirect handling to "error", preventing
automatic redirects to unvalidated targets while preserving the existing timeout
and null-fallback behavior.
---
Nitpick comments:
In `@apps/web/app/`(landing)/join/[username]/opengraph-image.tsx:
- Around line 29-31: Update the avatar-loading flow around loadSafeOgAvatar to
start the request without awaiting it, storing its promise before the
independent statistics queries. Await that promise immediately before
constructing ImageResponse, while preserving the existing safe avatar value and
error-handling behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3934f927-5096-4227-8b49-f13427285b0d
📒 Files selected for processing (6)
apps/web/__tests__/unit/og-safe-image.test.tsapps/web/app/(app)/post/[id]/opengraph-image.tsxapps/web/app/(landing)/join/[username]/opengraph-image.tsxapps/web/app/api/posts/[id]/share-image/route.tsxapps/web/lib/og-safe-image.tsdocs/CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/app/(app)/post/[id]/opengraph-image.tsx
| const res = await fetch(url, { | ||
| signal: AbortSignal.timeout(FETCH_TIMEOUT_MS), | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and inspect the target file around the reported lines.
if [ -f apps/web/lib/og-safe-image.ts ]; then
echo "== file exists =="
wc -l apps/web/lib/og-safe-image.ts
echo "== lines 1-140 =="
sed -n '1,140p' apps/web/lib/og-safe-image.ts | nl -ba
else
echo "target file not found"
fd -a 'og-safe-image\.ts$' .
fi
echo "== related usages =="
rg -n "loadSafeOgImage|og-safe-image|isAllowed|FETCH_TIMEOUT_MS" apps/web/lib apps/web -g '*.ts' -g '*.tsx' || trueRepository: ohong/straude
Length of output: 260
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== target file =="
sed -n '1,120p' apps/web/lib/og-safe-image.ts
echo "== related usages and fallback handling =="
rg -n "loadSafeOgImage|og-safe-image|isAllowed|FETCH_TIMEOUT_MS|onError|image.*onError|avatar|hero" apps/web/lib apps/web -g '*.ts' -g '*.tsx' || true
echo "== fetch redirect semantics probe under strict security context =="
node - <<'JS'
try {
fetch("https://httpbin.org/redirect-to?url=https://blocked", { redirect: "follow" })
.then(async (res) => {
console.log("follow final URL:", res.url);
await res.arrayBuffer();
})
.catch((error) => console.log("follow error:", error && error.message));
fetch("https://httpbin.org/redirect-to?url=https://blocked", { redirect: "error" })
.then((res) => console.log("error final URL:", res.url))
.catch((error) => console.log("error:", error && error.message));
} catch (error) {
console.log("probe unavailable:", error && error.message);
}
JSRepository: 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:
- 1: https://fetch.spec.whatwg.org/
- 2: https://github.com/whatwg/fetch/blob/main/fetch.bs
- 3: https://developer.mozilla.org/en-US/docs/Web/API/RequestInit
Block redirects to unvalidated targets.
fetch follows redirects by default, so isAllowed checks 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
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/lib/og-safe-image.ts` around lines 47 - 49, Update the fetch options
in the image-loading function around the visible fetch call to set redirect
handling to "error", preventing automatic redirects to unvalidated targets while
preserving the existing timeout and null-fallback behavior.
| 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; |
There was a problem hiding this comment.
🩺 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 content-length, or a false small value, can allocate far more than 5 MiB before the loader returns null.
Read res.body incrementally. Track received bytes. Cancel and return null as soon as the total exceeds MAX_IMAGE_BYTES. Update the oversized-body test to assert that reading stops at the limit.
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
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 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); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/lib/og-safe-image.ts` around lines 57 - 59, Update the
response-reading logic around the current Buffer.from and MAX_IMAGE_BYTES check
to consume res.body incrementally, track accumulated bytes, and cancel the
stream and return null immediately when the cap is exceeded. Preserve successful
buffering for responses within the limit, and update the oversized-body test to
verify reading stops once MAX_IMAGE_BYTES is crossed.
|
Reviewed this adversarially and pushed d932f2e on top. The webp diagnosis is right, but the fix as written opened a worse hole than the one it closed, and it only covered one of the three routes that crash. The fetch was unbounded and the URL is attacker-controlled.
Two sibling routes had the identical crash. Added the tests the PR was missing. Typecheck and lint clean. I'd merge this. |
|
CI red here is not this PR. Every open PR is failing the same single assertion: That test pins hardcoded LiteLLM dollar amounts for the GPT-5.6 family, and LiteLLM repriced #150 replaces the pinned rates with the invariant (every model resolves to a non-zero price, day total equals the sum of the breakdown). Merge that first and this should go green on a re-run. |
# Conflicts: # docs/CHANGELOG.md
# Conflicts: # docs/CHANGELOG.md
1 participant
Summary
Vercel runtime errors showed the
/post/[id]twitter/OG image route failing in production withUnsupported image type: image/webp(3 hits, 3 users over the last months, most recently yesterday). Satori (next/og) can't decode webp, and user-uploaded hero images/avatars can be webp regardless of file extension — the failing file was a webp stored as.jpg.Fix
New
lib/og-safe-image.tshelper fetches the image server-side, sniffs the real format from magic bytes, and returns a data URI only for satori-supported formats (PNG/JPEG/GIF). Unsupported, missing, or unfetchable images resolve tonull, and the card falls back to its existing no-image layout instead of 500ing. Wired into the post opengraph-image route for both the hero image and the avatar.Verification
null(fallback) instead of crashing; a real PNG round-trips to a data URI; a 404 returnsnull.tsc --noEmitclean for app/lib code (pre-existing test-file errors untouched).🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Documentation