presets(vercel): integrate with scheduled tasks#4030
Conversation
|
@RihanArfan is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughImplements Vercel Cron Jobs support for Nitro by introducing a new cron handler route, handler logic with optional CRON_SECRET validation, and configuration updates. Includes supporting documentation and test coverage for the new scheduled tasks integration. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/presets/vercel/runtime/cron-handler.ts`:
- Line 9: The current check if (authHeader !== `Bearer ${cronSecret}`) leaks
timing information; replace it with a constant-time comparison using Node's
crypto.timingSafeEqual: normalize both sides to Buffers of equal length (e.g.,
compute a safe HMAC or hash of `authHeader` and `Bearer ${cronSecret}` or
zero-pad lengths) and use timingSafeEqual to compare, updating the validation
logic in the cron handler that reads authHeader and cronSecret so the handler
performs a constant-time comparison before accepting the request.
🧹 Nitpick comments (1)
docs/2.deploy/20.providers/vercel.md (1)
116-121: Consider adding a warning that the endpoint is unprotected whenCRON_SECRETis not set.Currently, the cron handler (in
cron-handler.ts) only validates theAuthorizationheader whenCRON_SECRETis set. If a user doesn't configure it, the/_nitro/tasks/vercelendpoint is publicly accessible. A brief warning here would help users avoid accidentally deploying an unprotected cron endpoint.
commit: |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/presets/vercel/runtime/cron-handler.ts`:
- Around line 18-21: The handler currently checks
event.req.headers.get("x-vercel-cron-schedule") (variable cron) and throws an
HTTPError when missing; Vercel does not send that header so the handler will
always 400 in production—replace reliance on that header by reading a schedule
identifier from the request path or query string (e.g., parse event.req.url or
use event.req.nextUrl.searchParams.get("scheduleId") or distinct endpoints per
schedule), fall back to validating User-Agent ("vercel-cron/1.0") and the
Authorization bearer token if needed, and remove the HTTPError throw that
assumes the header exists; update any references to cron variable accordingly
(e.g., in the function that dispatches cron tasks) so scheduleId is used
instead.
🧹 Nitpick comments (2)
src/presets/vercel/runtime/cron-handler.ts (1)
5-16: CRON_SECRET validation is optional — consider the security implication.When
CRON_SECRETis not set, the cron endpoint is completely unprotected and publicly accessible. Anyone can trigger scheduled tasks by sending a request with a validx-vercel-cron-scheduleheader. While this matches Vercel's own docs (CRON_SECRET is opt-in), consider logging a warning during build ifCRON_SECRETis not configured, or at minimum note this in the documentation.src/presets/vercel/preset.ts (1)
22-25: Consider logging when cron handler is registered.Other configuration steps in this hook log info messages (e.g., runtime, entry format). Adding a similar log for cron handler registration would improve build-time observability.
📝 Suggested addition (inside the cron tasks block, after pushing the handler)
nitro.options.handlers.push({ route: nitro.options.vercel!.cronHandlerPath!, lazy: true, handler: join(presetsDir, "vercel/runtime/cron-handler"), }); + logger.info(`Registered cron handler at \`${nitro.options.vercel!.cronHandlerPath!}\`.`);Based on learnings: "Use
consolafor logging in build/dev code; usenitro.loggerwhen available"
| }, | ||
| }); | ||
|
|
||
| return { cron, tasks: result }; |
There was a problem hiding this comment.
Is this object consumed by vercel? Why this specific shape?
There was a problem hiding this comment.
No particular reason. I've double checked and Vercel only shows things that were logged but not the response of this endpoint, so I'll update it so we return nothing. Should we log anything? Like Running cron schedule * * * * *
There was a problem hiding this comment.
We mighjt like success: true for example. My main concern is to not introduce a reponse and break it later only
🔗 Linked issue
Related #1974 - we should update the checklist
❓ Type of change
📚 Description
Support Vercel Cron within Nitro Tasks. Automatically creates Vercel Cron configuration based on Nitro scheduled tasks, so fully zero-config when deploying to Vercel.
📝 Checklist