Skip to content

🔒 Security: Prevent SSRF vulnerability when fetching fonts - #562

Open
is0692vs wants to merge 1 commit into
mainfrom
jules-18188617785768465598-68be9b16
Open

🔒 Security: Prevent SSRF vulnerability when fetching fonts#562
is0692vs wants to merge 1 commit into
mainfrom
jules-18188617785768465598-68be9b16

Conversation

@is0692vs

@is0692vs is0692vs commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

🎯 What:
Fixed a Server-Side Request Forgery (SSRF) vulnerability caused by dynamically fetching fonts based on the APP_URL environment variable.

⚠️ Risk:
If an attacker exploited a misconfiguration in APP_URL to point to an internal network address (or used a DNS rebinding attack if validation bypassed), the edge worker could be coerced into making arbitrary internal requests, potentially accessing private or sensitive data.

🛡️ Solution:
Removed dynamic font fetching entirely. The getFontData function now strictly uses the hardcoded DEFAULT_FONT_URL (JSDelivr). Removed fontUrl and allowedOrigin propagation from the API route and the cardRenderer function signatures. Updated associated tests.


PR created automatically by Jules for task 18188617785768465598 started by @is0692vs

Greptile Summary

APP_URL 由来のフォント URL 伝播を廃止し、SVG レンダリング用フォントを固定の JSDelivr URL から取得することで SSRF 経路を除去しています。

  • API ルートから fontUrl と allowedOrigin の生成・伝播を削除
  • カードレンダラーの公開引数と内部 SVG レンダリングを簡素化
  • エラー応答テストを新しい引数契約に更新

Confidence Score: 4/5

SVG の外部フォント取得に失敗すると 503 フォールバックも同じ理由で失敗するため、この経路を修正してからマージすべきです。

SSRF の原因となった動的 URL は除去されていますが、すべての SVG 描画を単一の外部 CDN に固定したことで、その取得障害が通常描画だけでなく catch 節内のエラーカード描画にも伝播し、意図した 503 応答を返せません。

Files Needing Attention: src/lib/cardRenderer.tsx、src/app/api/card/[username]/route.ts

Important Files Changed

Filename Overview
src/lib/cardRenderer.tsx フォント URL を固定して SSRF 経路を除去した一方、SVG の通常描画とエラー描画が同じ CDN 障害で失敗し、未使用 import も残っています。
src/app/api/card/[username]/route.ts APP_URL 由来のフォント指定を全分岐から削除していますが、SVG レンダリング失敗後のエラーカード生成には二重障害へのフォールバックがありません。
src/app/api/card/[username]/route.test.ts エラーレンダラーの期待値から fontUrl を削除し、新しい呼び出し契約に整合させています。

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A["GET /api/card/:username?format=svg"] --> B["renderCardResponse"]
  B --> C["getFontData"]
  C --> D["固定 JSDelivr URL"]
  D -->|成功| E["SVG 応答"]
  D -->|失敗| F["route の catch"]
  F --> G["renderErrorCardResponse"]
  G --> C
  D -->|再失敗| H["未処理例外 / 500"]
Loading

Comments Outside Diff (1)

  1. src/lib/cardRenderer.tsx, line 9 (link)

    P2 URL 検証の未使用 import

    getFontData から URL 検証を削除したことで isTrustedFontUrl は参照されなくなっています。残したままだと静的解析の警告や lint エラーになり、現在も URL 検証が使われているとの誤解を招くため削除してください。

    Prompt To Fix With AI
    This is a comment left during a code review.
    Path: src/lib/cardRenderer.tsx
    Line: 9
    
    Comment:
    **URL 検証の未使用 import**
    
    `getFontData` から URL 検証を削除したことで `isTrustedFontUrl` は参照されなくなっています。残したままだと静的解析の警告や lint エラーになり、現在も URL 検証が使われているとの誤解を招くため削除してください。
    
    
    
    ---
    
    For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix All With AI
### Issue 1
src/lib/cardRenderer.tsx:21
**SVG フォールバックも CDN 障害で失敗**

JSDelivr に到達できない環境では、`format=svg` の通常描画が失敗した後、503 エラーカードも同じ `DEFAULT_FONT_URL` を再取得して失敗するため、クライアントには意図した 503 カードではなく未処理例外による 500 応答が返ります。

### Issue 2
src/lib/cardRenderer.tsx:9
**URL 検証の未使用 import**

`getFontData` から URL 検証を削除したことで `isTrustedFontUrl` は参照されなくなっています。残したままだと静的解析の警告や lint エラーになり、現在も URL 検証が使われているとの誤解を招くため削除してください。

```suggestion

```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (1): Last reviewed commit: "🔒 security fix: prevent SSRF via fontUr..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

Context used:

This commit removes the ability for the card API route to dynamically fetch fonts from an origin dictated by the `APP_URL` environment variable. Instead, the font data fetcher is hardcoded to use the default JSDelivr URL, completely neutralizing the SSRF vector.

Risk: If an attacker exploited a misconfiguration to point `APP_URL` to an internal address, the edge worker could perform requests against internal services (SSRF) when fetching the font.

Solution: Removed dynamic font fetching entirely and rely exclusively on the hardcoded `DEFAULT_FONT_URL`.

Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
@vercel

vercel Bot commented Aug 7, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
github-user-summary Ignored Ignored Aug 7, 2026 7:04am

@google-labs-jules

Copy link
Copy Markdown
Contributor

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@is0692vs, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 37 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: ae1f7184-c8cc-43f8-8aab-0554512fe6dc

📥 Commits

Reviewing files that changed from the base of the PR and between eb95c48 and ff9400a.

📒 Files selected for processing (3)
  • src/app/api/card/[username]/route.test.ts
  • src/app/api/card/[username]/route.ts
  • src/lib/cardRenderer.tsx

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Aug 7, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Prevent SSRF by hardcoding font fetch to trusted CDN

🐞 Bug fix 🧪 Tests 🕐 10-20 Minutes

Grey Divider

AI Description

• Remove dynamic font URL construction from APP_URL to eliminate SSRF risk.
• Hardcode font fetching to the JSDelivr default font URL in the renderer.
• Update API route and tests to stop passing/expecting fontUrl and allowedOrigin.
Diagram

graph TD
  C[Client] --> R["/api/card/[username] route"] --> CR["cardRenderer"] --> REN["satori / ImageResponse"]
  R --> DF["cardDataFetcher"]
  CR --> CDN{{"JSDelivr font URL"}}
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Bundle the font (no network fetch)
  • ➕ Eliminates outbound fetch dependency entirely (strongest SSRF mitigation)
  • ➕ Avoids runtime failures if CDN is unavailable
  • ➖ Increases edge bundle size and potentially cold-start time
  • ➖ Requires updating/redeploying to change fonts
2. Keep configurability with strict allowlist + private-IP/DNS rebinding defenses
  • ➕ Retains flexibility for self-hosted fonts or custom domains
  • ➖ Hard to implement safely (DNS rebinding, redirects, IPv6, parsing edge cases)
  • ➖ Still expands attack surface compared to a fixed URL

Recommendation: Hardcoding the font URL to a trusted CDN is the most robust and lowest-maintenance mitigation for this SSRF class, and matches the PR’s goal of fully neutralizing the vector. If availability becomes a concern, bundling the font is the next safest option. (Optional follow-up: remove now-unused isTrustedFontUrl import/tests if they’re no longer needed.)

Files changed (3) +11 / -27

Bug fix (2) +10 / -25
route.tsRemove 'APP_URL'-derived font origin propagation +5/-9

Remove 'APP_URL'-derived font origin propagation

• Deletes 'allowedOrigin'/'fontUrl' construction from 'APP_URL' and stops passing these values into 'renderCardResponse'/'renderErrorCardResponse'. Keeps rate limiting, data fetching, and caching behavior unchanged.

src/app/api/card/[username]/route.ts

cardRenderer.tsxHardcode font fetching to DEFAULT_FONT_URL and simplify APIs +5/-16

Hardcode font fetching to DEFAULT_FONT_URL and simplify APIs

• Refactors 'getFontData' to always fetch the hardcoded JSDelivr URL and removes 'fontUrl'/'allowedOrigin' parameters from 'renderSvg', 'renderCardResponse', and 'renderErrorCardResponse' signatures. Preserves caching, timeout, and redirect-blocking behavior for the font fetch.

src/lib/cardRenderer.tsx

Tests (1) +1 / -2
route.test.tsStop asserting 'fontUrl' is passed to error renderer +1/-2

Stop asserting 'fontUrl' is passed to error renderer

• Updates error-response expectations to only verify message/status/cache headers. Removes the prior expectation that a localhost-derived 'fontUrl' is propagated.

src/app/api/card/[username]/route.test.ts

@qodo-code-review

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Informational

1. Unused validator import 🐞 Bug ⚙ Maintainability
Description
src/lib/cardRenderer.tsx still imports isTrustedFontUrl even though getFontData() no longer
calls it after the refactor, leaving an unused import that can cause npm run lint to fail under
the Next.js TypeScript ESLint config.
Code

src/lib/cardRenderer.tsx[L20-23]

-function getFontData(fontUrl?: string, allowedOrigin?: string): Promise<ArrayBuffer> {
-  const targetUrl =
-    fontUrl && isTrustedFontUrl(fontUrl, allowedOrigin)
-      ? fontUrl
Evidence
cardRenderer.tsx imports isTrustedFontUrl but the refactored getFontData() always uses
DEFAULT_FONT_URL and never references the validator. The repository ESLint config includes Next’s
TypeScript ruleset, which commonly reports unused imports.

src/lib/cardRenderer.tsx[1-23]
src/lib/cardRenderer.tsx[20-66]
eslint.config.mjs[1-16]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`src/lib/cardRenderer.tsx` no longer uses `isTrustedFontUrl` after `getFontData` was simplified to always fetch `DEFAULT_FONT_URL`, but the import remains. This creates an unused import that is typically flagged by the project’s ESLint setup.

### Issue Context
The PR removed `fontUrl`/`allowedOrigin` parameters and the conditional `isTrustedFontUrl(...)` selection logic; leaving the import behind is now dead code.

### Fix Focus Areas
- src/lib/cardRenderer.tsx[1-12]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context used
✅ Compliance rules (platform): 30 rules

To customize comments, go to the Qodo configuration screen, or learn more in the docs.

Qodo Logo

Comment thread src/lib/cardRenderer.tsx
Comment on lines -20 to -23
function getFontData(fontUrl?: string, allowedOrigin?: string): Promise<ArrayBuffer> {
const targetUrl =
fontUrl && isTrustedFontUrl(fontUrl, allowedOrigin)
? fontUrl

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Informational

1. Unused validator import 🐞 Bug ⚙ Maintainability

src/lib/cardRenderer.tsx still imports isTrustedFontUrl even though getFontData() no longer
calls it after the refactor, leaving an unused import that can cause npm run lint to fail under
the Next.js TypeScript ESLint config.
Agent Prompt
### Issue description
`src/lib/cardRenderer.tsx` no longer uses `isTrustedFontUrl` after `getFontData` was simplified to always fetch `DEFAULT_FONT_URL`, but the import remains. This creates an unused import that is typically flagged by the project’s ESLint setup.

### Issue Context
The PR removed `fontUrl`/`allowedOrigin` parameters and the conditional `isTrustedFontUrl(...)` selection logic; leaving the import behind is now dead code.

### Fix Focus Areas
- src/lib/cardRenderer.tsx[1-12]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment thread src/lib/cardRenderer.tsx
? fontUrl
: DEFAULT_FONT_URL;
function getFontData(): Promise<ArrayBuffer> {
const targetUrl = DEFAULT_FONT_URL;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 SVG フォールバックも CDN 障害で失敗

JSDelivr に到達できない環境では、format=svg の通常描画が失敗した後、503 エラーカードも同じ DEFAULT_FONT_URL を再取得して失敗するため、クライアントには意図した 503 カードではなく未処理例外による 500 応答が返ります。

Knowledge Base Used: Card Data Pipeline

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/cardRenderer.tsx
Line: 21

Comment:
**SVG フォールバックも CDN 障害で失敗**

JSDelivr に到達できない環境では、`format=svg` の通常描画が失敗した後、503 エラーカードも同じ `DEFAULT_FONT_URL` を再取得して失敗するため、クライアントには意図した 503 カードではなく未処理例外による 500 応答が返ります。

**Knowledge Base Used:** [Card Data Pipeline](https://app.greptile.com/hiroki-org/-/custom-context/knowledge-base/hiroki-org/github-user-summary/-/docs/card-data-pipeline.md)

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant