🔒 Fix potential Server-Side DoS via Unauthenticated GitHub API Call - #516
🔒 Fix potential Server-Side DoS via Unauthenticated GitHub API Call#516is0692vs wants to merge 2 commits into
Conversation
…b API call Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
|
👋 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 New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📄 Knowledge reviewDosu skipped reviewing this PR because your organization has used its |
|
Warning Review limit reached
Next review available in: 52 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
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 |
PR Summary by QodoAuthenticate GitHub OG endpoint requests to prevent rate-limit DoS
AI Description
Diagram
High-Level Assessment
Files changed (1)
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Code Review by Qodo
Context used✅ Compliance rules (platform):
30 rules 1. Untrimmed GitHub token
|
| try { | ||
| const res = await fetch(`https://api.github.com/users/${encodeURIComponent(username)}`, { | ||
| headers: { | ||
| ...(process.env.GITHUB_TOKEN && { Authorization: `Bearer ${process.env.GITHUB_TOKEN}` }), |
There was a problem hiding this comment.
複数の送信元IPからキャッシュされていない異なるユーザー名へ毎分50件ずつ要求すると、IP単位の制限を通過した合計リクエストが単一の GITHUB_TOKEN に集中し、通常の認証済み時間クォータを超えます。クォータ枯渇後のGitHubの403応答はデフォルト値へフォールスルーするため、アバターや統計が欠落したOG画像が200で返され、最大24時間キャッシュされます。
How this was verified: IP単位の毎分50件制限から、異なるユーザー名への共有トークン付きAPI呼び出しと非OK応答時のフォールバックまでを追跡しました。
Context Used: 日本語で!!! (source)
Knowledge Base Used: Card Data Pipeline
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/api/og/[username]/route.tsx
Line: 47
Comment:
**共有トークンのクォータ枯渇**
複数の送信元IPからキャッシュされていない異なるユーザー名へ毎分50件ずつ要求すると、IP単位の制限を通過した合計リクエストが単一の `GITHUB_TOKEN` に集中し、通常の認証済み時間クォータを超えます。クォータ枯渇後のGitHubの403応答はデフォルト値へフォールスルーするため、アバターや統計が欠落したOG画像が200で返され、最大24時間キャッシュされます。
**How this was verified:** IP単位の毎分50件制限から、異なるユーザー名への共有トークン付きAPI呼び出しと非OK応答時のフォールバックまでを追跡しました。
**Context Used:** 日本語で!!! ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))
**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.| try { | ||
| const res = await fetch(`https://api.github.com/users/${encodeURIComponent(username)}`, { | ||
| headers: { | ||
| ...(process.env.GITHUB_TOKEN && { Authorization: `Bearer ${process.env.GITHUB_TOKEN}` }), |
There was a problem hiding this comment.
1. No tests for github_token 📘 Rule violation ▣ Testability
The new conditional Authorization header branch is not covered by tests, so regressions (missing/always-sent auth header) could go unnoticed. Add tests that assert request headers with and without process.env.GITHUB_TOKEN.
Agent Prompt
## Issue description
`src/app/api/og/[username]/route.tsx` now conditionally adds an `Authorization` header when `process.env.GITHUB_TOKEN` is set, but the existing tests do not assert this behavior.
## Issue Context
This PR changes auth behavior for the GitHub API request; tests should verify both branches:
- When `GITHUB_TOKEN` is set, `fetch` receives `Authorization: Bearer <token>`.
- When `GITHUB_TOKEN` is unset/empty, `Authorization` is not present.
## Fix Focus Areas
- src/app/api/og/[username]/route.tsx[45-52]
- src/app/api/og/[username]/route.test.ts[45-79]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| try { | ||
| const res = await fetch(`https://api.github.com/users/${encodeURIComponent(username)}`, { | ||
| headers: { | ||
| ...(process.env.GITHUB_TOKEN && { Authorization: `Bearer ${process.env.GITHUB_TOKEN}` }), |
There was a problem hiding this comment.
2. Untrimmed github token 🐞 Bug ≡ Correctness
The OG route sends Authorization: Bearer ${process.env.GITHUB_TOKEN} without trimming/validating,
so a whitespace-padded secret (common when copy/pasting) will produce an invalid auth header and the
call will still be effectively unauthenticated—undermining the PR’s DoS mitigation. The current
...(process.env.GITHUB_TOKEN && {...}) pattern is also brittle; prefer a normalized token +
explicit ternary/object construction.
Agent Prompt
### Issue description
`src/app/api/og/[username]/route.tsx` conditionally adds the GitHub `Authorization` header using the raw `process.env.GITHUB_TOKEN` value. If the token contains leading/trailing whitespace, the route will send an invalid `Authorization` header and GitHub will treat the request as unauthenticated (or reject it), negating the intended rate-limit increase.
### Issue Context
The repo already has a GitHub fetcher that normalizes the token via `.trim()` before setting `Authorization`.
### Fix Focus Areas
- src/app/api/og/[username]/route.tsx[44-52]
### Suggested change
1. Normalize once:
- `const token = process.env.GITHUB_TOKEN?.trim();`
2. Build headers with a stable object type:
- `...(token ? { Authorization: `Bearer ${token}` } : {})`
This matches the approach in `src/lib/cardDataFetcher.ts` and avoids sending an invalid header for whitespace-only tokens.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
…b API call Co-authored-by: is0692vs <135803462+is0692vs@users.noreply.github.com>
🎯 What: Adds
⚠️ Risk: The unauthenticated API call is subject to a strict rate limit (60 req/hr), making the endpoint vulnerable to Server-Side DoS by exhausting the rate limit.
Authorizationheader to the GitHub API call insrc/app/api/og/[username]/route.tsxifprocess.env.GITHUB_TOKENis set.🛡️ Solution: Passes the
GITHUB_TOKENto the GitHub API call, upgrading to the authenticated rate limit (5000 req/hr).PR created automatically by Jules for task 9905796814203422129 started by @is0692vs
Greptile Summary
GitHubユーザー情報を取得するOG画像APIについて、
GITHUB_TOKENが設定されている場合にBearer認証を追加する変更です。Confidence Score: 4/5
共有GitHubトークンのクォータを分散リクエストで枯渇させられるため、この経路を修正してからマージする必要があります。
IP単位の制限は複数IPからの合計リクエスト数を抑えられず、2つのIPから許容量いっぱいに異なるユーザー名を要求するだけで、共有トークンの通常の時間クォータを超えて後続のOG画像を不完全にできます。
Files Needing Attention: src/app/api/og/[username]/route.tsx
Security Review
共有トークンの追加によってGitHub APIクォータが全利用者で共有されますが、制限は送信元IP単位のままです。異なるユーザー名を使う複数IPからのリクエストで共有クォータを枯渇させ、後続のOG画像を最大24時間、不完全な内容にできます。
How this was verified: IP単位の毎分50件制限から、異なるユーザー名へのGitHub API呼び出しと共有トークンの非OK応答時のフォールバックまでを追跡しました。
Important Files Changed
Sequence Diagram
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "Fix potential Server-Side DoS by adding ..." | Re-trigger Greptile
Context used: