fix(dev): restrict /_nitro/tasks endpoint to local requests#4389
fix(dev): restrict /_nitro/tasks endpoint to local requests#4389winstoncrooker wants to merge 1 commit into
Conversation
The dev task runner (/_nitro/tasks and /_nitro/tasks/:name) executes server tasks with caller-supplied payload and had no access control, while the dev server binds all interfaces by default (devServer.hostname is unset). Any host that can reach the dev server could enumerate and run tasks. The VFS viewer already restricts itself to loopback requests; this applies the same check to the task runner, sharing the logic in a new isLocalDevRequest helper. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@winstoncrooker is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds ChangesLocal dev access control
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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.
Actionable comments posted: 2
🤖 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 `@src/dev/app.ts`:
- Line 73: The 403 thrown from the development app bootstrap should use an
actionable message instead of the current generic “Forbidden IP” text. Update
the HTTPError in app.ts so it clearly tells local developers that access is
restricted to localhost/loopback and what to change, and keep the logic in the
dev server access check that rejects non-local requests without echoing the
client IP.
In `@src/dev/vfs.ts`:
- Line 8: The VFS access denial in the code path that throws HTTPError should
use an actionable localhost-focused message instead of the generic “Forbidden
IP”. Update the statusText in the vfs denial logic so it tells local developers
that only localhost access is allowed, without exposing the computed IP, and
keep the change within the existing HTTPError throw site.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 782191b5-dd67-4b23-88ee-99ab1d4f6b99
📒 Files selected for processing (3)
src/dev/_request.tssrc/dev/app.tssrc/dev/vfs.ts
| // all interfaces by default (`devServer.hostname` is unset). | ||
| const assertLocalTaskRequest = (event: H3Event) => { | ||
| if (!isLocalDevRequest(event)) { | ||
| throw new HTTPError({ statusText: "Forbidden IP", status: 403 }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use an actionable 403 message.
Line 73 returns only "Forbidden IP", which doesn’t tell a local developer what to fix. Prefer a message that states the localhost requirement without echoing the client IP. As per coding guidelines, "Include actionable context in error messages."
Proposed fix
- throw new HTTPError({ statusText: "Forbidden IP", status: 403 });
+ throw new HTTPError({
+ message: "Dev task runner is only available from localhost.",
+ status: 403,
+ });📝 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.
| throw new HTTPError({ statusText: "Forbidden IP", status: 403 }); | |
| throw new HTTPError({ | |
| message: "Dev task runner is only available from localhost.", | |
| status: 403, | |
| }); |
🤖 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 `@src/dev/app.ts` at line 73, The 403 thrown from the development app bootstrap
should use an actionable message instead of the current generic “Forbidden IP”
text. Update the HTTPError in app.ts so it clearly tells local developers that
access is restricted to localhost/loopback and what to change, and keep the
logic in the dev server access check that rejects non-local requests without
echoing the client IP.
Source: Coding guidelines
| status: 403, | ||
| }); | ||
| if (!isLocalDevRequest(event)) { | ||
| throw new HTTPError({ statusText: "Forbidden IP", status: 403 }); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Make the VFS denial message actionable.
"Forbidden IP" is generic and doesn’t tell a local developer the expected access pattern. Prefer a localhost-specific message without exposing the computed IP. As per coding guidelines, "Include actionable context in error messages."
Proposed fix
- throw new HTTPError({ statusText: "Forbidden IP", status: 403 });
+ throw new HTTPError({
+ message: "VFS viewer is only available from localhost.",
+ status: 403,
+ });🤖 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 `@src/dev/vfs.ts` at line 8, The VFS access denial in the code path that throws
HTTPError should use an actionable localhost-focused message instead of the
generic “Forbidden IP”. Update the statusText in the vfs denial logic so it
tells local developers that only localhost access is allowed, without exposing
the computed IP, and keep the change within the existing HTTPError throw site.
Source: Coding guidelines
1 participant
Summary
The development task runner endpoints,
/_nitro/tasksand/_nitro/tasks/:name, execute server tasks with caller-supplied payload ({ ...getQuery(event), ...body }) and have no access control. The dev server binds all network interfaces by default (devServer.hostnameis unset insrc/config/defaults.ts), so any host that can reach the dev server can enumerate the project's tasks and invoke them — for example adb:*migration or seed task — with attacker-controlled input. A co-located process or a different local user can do the same over loopback.The VFS debug viewer already restricts itself to loopback requests (
createVFSHandlerinsrc/dev/vfs.ts), so the framework already treats dev debug surfaces as local-only; the task runner just did not follow the same rule. (The preview server also hard-codes--host localhost, while the dev server defaults to all interfaces.)Change
vfs.tsinto a sharedisLocalDevRequesthelper (src/dev/_request.ts)./_nitro/tasksand/_nitro/tasks/**at the dev-app layer — the same layer as the VFS viewer, where the real client socket is visible (the task routes are otherwise handled by the worker via the catch-all).Verification
With
test/fixture(which enablesexperimental.tasks), running the dev server:GET /_nitro/tasksandGET /_nitro/tasks/db:migratecontinue to work (HTTP 200) over loopback (127.0.0.1).403 Forbidden IPinstead of executing./_vfs/behaves identically to before (loopback 200, non-loopback 403), now via the shared helper.Scope and alternatives
Dev-server only; production output is unaffected, and the task routes require the experimental tasks feature. An
Origin/CSRF check would additionally harden the drive-by-from-a-visited-page vector, and binding the dev server tolocalhostby default (as the preview server already does) would be a broader option. This change keeps to the minimal, behavior-preserving fix that mirrors the existing VFS gate. Happy to add a regression test or take a different approach if you prefer.🤖 Generated with Claude Code