fix(nitro): gate chrome devtools workspace endpoint to local requests#35201
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis change adds access control to the Chrome devtools JSON endpoint in the Nitro server. A type-only import of 🚥 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 docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@packages/nitro-server/src/index.ts`:
- Around line 1071-1078: The host extraction in isLocalDevRequest is incorrectly
splitting IPv6 Host headers by ':'; update the parsing to correctly handle IPv6
bracketed hosts and ports: if hostHeader starts with '[' extract the substring
between '[' and ']' as the host, otherwise strip the port by taking the
substring before the last ':' (or use URL/authority parsing). Then use that
parsed host when checking allowedHosts.has(host) so IPv6 addresses like
`[::1]:3000` are recognized by the allowedHosts check.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9eaf028c-263f-4033-a34f-d4e64b2710e5
📒 Files selected for processing (1)
packages/nitro-server/src/index.ts
| function isLocalDevRequest (event: H3Event, allowedHosts: ReadonlySet<string> | true): boolean { | ||
| const hostHeader = event.req.headers.get('host') | ||
| if (allowedHosts !== true) { | ||
| const host = hostHeader?.split(':')[0] | ||
| if (!host || !allowedHosts.has(host)) { | ||
| return false | ||
| } | ||
| } |
There was a problem hiding this comment.
IPv6 host parsing is broken.
Using hostHeader?.split(':')[0] to strip the port does not work for IPv6 addresses. For a Host header like [::1]:3000, splitting on : yields ['[', '', '1]', '3000'], so the extracted host becomes [ instead of [::1]. This would incorrectly reject all IPv6 loopback requests.
🐛 Proposed fix for IPv6 handling
function isLocalDevRequest (event: H3Event, allowedHosts: ReadonlySet<string> | true): boolean {
const hostHeader = event.req.headers.get('host')
if (allowedHosts !== true) {
- const host = hostHeader?.split(':')[0]
+ let host: string | undefined
+ if (hostHeader?.startsWith('[')) {
+ // IPv6: [::1] or [::1]:port
+ const closingBracket = hostHeader.indexOf(']')
+ host = closingBracket !== -1 ? hostHeader.slice(0, closingBracket + 1) : undefined
+ } else {
+ // IPv4 or hostname: strip port if present
+ host = hostHeader?.split(':')[0]
+ }
if (!host || !allowedHosts.has(host)) {
return false
}
}📝 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.
| function isLocalDevRequest (event: H3Event, allowedHosts: ReadonlySet<string> | true): boolean { | |
| const hostHeader = event.req.headers.get('host') | |
| if (allowedHosts !== true) { | |
| const host = hostHeader?.split(':')[0] | |
| if (!host || !allowedHosts.has(host)) { | |
| return false | |
| } | |
| } | |
| function isLocalDevRequest (event: H3Event, allowedHosts: ReadonlySet<string> | true): boolean { | |
| const hostHeader = event.req.headers.get('host') | |
| if (allowedHosts !== true) { | |
| let host: string | undefined | |
| if (hostHeader?.startsWith('[')) { | |
| // IPv6: [::1] or [::1]:port | |
| const closingBracket = hostHeader.indexOf(']') | |
| host = closingBracket !== -1 ? hostHeader.slice(0, closingBracket + 1) : undefined | |
| } else { | |
| // IPv4 or hostname: strip port if present | |
| host = hostHeader?.split(':')[0] | |
| } | |
| if (!host || !allowedHosts.has(host)) { | |
| return false | |
| } | |
| } |
🤖 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 `@packages/nitro-server/src/index.ts` around lines 1071 - 1078, The host
extraction in isLocalDevRequest is incorrectly splitting IPv6 Host headers by
':'; update the parsing to correctly handle IPv6 bracketed hosts and ports: if
hostHeader starts with '[' extract the substring between '[' and ']' as the
host, otherwise strip the port by taking the substring before the last ':' (or
use URL/authority parsing). Then use that parsed host when checking
allowedHosts.has(host) so IPv6 addresses like `[::1]:3000` are recognized by the
allowedHosts check.
|
Actionable comments posted: 0 |
@nuxt/kit
@nuxt/nitro-server
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
Merging this PR will not alter performance
Comparing Footnotes
|
…#35201) (cherry picked from commit 0289381) Refs: GHSA-rq7w-g337-39qq
…#35201) (cherry picked from commit 0289381) Refs: GHSA-rq7w-g337-39qq
🔗 Linked issue
📚 Description
out of an abundance of caution, this prevents us from serving the chrome devtools json integration to a non-allowed-host, just as we do for the vite dev server.