fix(deploy): enhance API token handling and improve user feedback#475
fix(deploy): enhance API token handling and improve user feedback#475dynamo-pentester wants to merge 2 commits into
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
👋 Thanks for opening a PR, @dynamo-pentester!Your PR has entered the 🚦 PR Review Pipeline.
What happens next
A pipeline status comment will appear below and update automatically as your PR progresses. While you wait
This comment is posted only once. |
|
Linter diff in the way? Review this PR in Change Stack to focus on meaningful changes and expand context only when needed. Warning Review limit reached
More reviews will be available in 16 minutes and 3 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughBackend deploy route now requires a trimmed user-supplied Vercel API key, enforces per-user rate limiting (5 deploys/min) with 429/Retry-After/X-RateLimit headers, and returns readyState on success. Frontend deploy dialog always collects and validates the user token, sends it as ChangesVercel Deploy Security Enforcement
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
modules/playground/components/deploy-dialog.tsx (1)
83-90:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard against missing
urlin API response.If the backend returns a success response but
data.urlisundefinedornull,data.url.startsWith("http")will throw aTypeError, crashing the UI.🛡️ Proposed defensive check
if (!res.ok) { throw new Error(data.error || "Deployment failed"); } - setDeployedUrl(data.url.startsWith("http") ? data.url : `https://${data.url}`); + const url = data.url; + if (!url) { + throw new Error("Deployment succeeded but no URL was returned"); + } + setDeployedUrl(url.startsWith("http") ? url : `https://${url}`); toast.success("Successfully deployed!");🤖 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 `@modules/playground/components/deploy-dialog.tsx` around lines 83 - 90, The response handling assumes data.url is a string and calls data.url.startsWith, which will throw if url is null/undefined; update the deploy result handling (after const data = await res.json()) to defensively check that data.url is a non-empty string (e.g., typeof data.url === "string" && data.url) before using startsWith, and if it's missing either throw a meaningful Error or setDeployedUrl to a safe fallback (or skip calling setDeployedUrl/toast.success). Modify the logic around setDeployedUrl and toast.success to only run when the validated url exists, referencing data.url and setDeployedUrl.
🧹 Nitpick comments (1)
app/api/deploy/vercel/route.ts (1)
44-53: 💤 Low valueConsider explicit type guard for
userApiKey.If a malformed request sends a non-string
userApiKey(e.g., a number), calling.trim()on it will throw at runtime, resulting in a 500 instead of a clear 400 validation error.🛡️ Proposed defensive check
- const token = (userApiKey as string | undefined)?.trim(); + const token = typeof userApiKey === "string" ? userApiKey.trim() : undefined;🤖 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 `@app/api/deploy/vercel/route.ts` around lines 44 - 53, The code currently calls (userApiKey as string | undefined)?.trim() which will throw if userApiKey is not a string; update the validation in the route handler to first check typeof userApiKey === "string" before calling .trim(), and if it's not a string (or is missing/empty after trimming) return the same NextResponse.json 400 error; reference the userApiKey/token validation block (token variable and the NextResponse.json error response) and apply this explicit type guard to avoid runtime exceptions.
🤖 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.
Outside diff comments:
In `@modules/playground/components/deploy-dialog.tsx`:
- Around line 83-90: The response handling assumes data.url is a string and
calls data.url.startsWith, which will throw if url is null/undefined; update the
deploy result handling (after const data = await res.json()) to defensively
check that data.url is a non-empty string (e.g., typeof data.url === "string" &&
data.url) before using startsWith, and if it's missing either throw a meaningful
Error or setDeployedUrl to a safe fallback (or skip calling
setDeployedUrl/toast.success). Modify the logic around setDeployedUrl and
toast.success to only run when the validated url exists, referencing data.url
and setDeployedUrl.
---
Nitpick comments:
In `@app/api/deploy/vercel/route.ts`:
- Around line 44-53: The code currently calls (userApiKey as string |
undefined)?.trim() which will throw if userApiKey is not a string; update the
validation in the route handler to first check typeof userApiKey === "string"
before calling .trim(), and if it's not a string (or is missing/empty after
trimming) return the same NextResponse.json 400 error; reference the
userApiKey/token validation block (token variable and the NextResponse.json
error response) and apply this explicit type guard to avoid runtime exceptions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c1b81fd1-3801-48dd-aa83-43b2767a54eb
📒 Files selected for processing (2)
app/api/deploy/vercel/route.tsmodules/playground/components/deploy-dialog.tsx
Signed-off-by: dynamo-pentester <giveawaynino143@gmail.com>
dynamo-pentester
force-pushed
the
bug/vercel-master
branch
from
2996b73 to
99ed963
Compare
1 participant
Summary
VERCEL_MASTER_TOKENsilent fallback inapp/api/deploy/vercel/route.tsuserApiKey; missing or empty tokens return a clear400session?.user?.idinstead ofsession?.userdeploy-dialog.tsxsince it relied on the master token fallback; the dialog now always prompts for a personal API tokenType of change
Related issue
Closes #449
Validation
npm run lintnpm testnpm run buildManual verification:
POST /api/deploy/vercelwithoutuserApiKeywhile authenticated — confirmed400with error messagePOST /api/deploy/vercelwithuserApiKey: ""anduserApiKey: null— both correctly return400userApiKey— deployment proceeds normallyuserApiKey: undefinedto the routeChecklist
Summary by CodeRabbit
New Features
Improvements