Conversation
|
@RihanArfan is attempting to deploy a commit to the Nitro Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds a new "basicAuth" route rule that invokes h3's requireBasicAuth when configured; updates runtime route-rules mapping, TypeScript types, tests, fixture config, and docs to expose and verify route-level basic authentication. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 🚥 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)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. 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 |
commit: |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/build/virtual/routing.ts (1)
92-106:⚠️ Potential issue | 🟠 Major
authroute rule passwords are serialized in plaintext into the compiled bundle.
JSON.stringify(options)embeds the entire options object — includingpassword— into the generated JavaScript at build time. Every consumer of the build artifact (minified or not) can read the credential in plaintext. This applies specifically to theauthrule becausepasswordis a secret, unlike headers or redirect targets.Consider either:
- Filtering sensitive fields before serialization and resolving them at runtime via
useRuntimeConfig, or- Prominently documenting this limitation (the compiled bundle will contain the literal password string) and recommending that users pass
process.env.SECRET_PASSWORDin theirnitro.config.tsso the secret comes from the build environment rather than being committed to source.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/build/virtual/routing.ts` around lines 92 - 106, serializeRouteRule currently embeds full options (including auth.password) into the compiled bundle via JSON.stringify; update serializeRouteRule to strip or replace sensitive fields for the "auth" rule before serialization (e.g., if name === "auth", clone options and delete or replace the password field with a runtime reference token) and ensure consumers resolve the actual secret at runtime (via useRuntimeConfig or process.env) instead of emitting the literal password; keep existing behavior for other rule names and preserve RuntimeRouteRules and handler emission (__routeRules__, h._route, h._method) while serializing the sanitized options.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/1.docs/5.routing.md`:
- Around line 363-376: The example imports defineNitroConfig but calls
defineConfig — fix by making the symbols consistent: either import defineConfig
from "nitro" and leave the calls as defineConfig, or keep the import of
defineNitroConfig from "nitro/config" and rename the calls to defineNitroConfig;
update both basic-auth examples (the blocks that use
defineNitroConfig/defineConfig and the routeRules with auth) so the import and
the function call match (references: defineNitroConfig, defineConfig,
routeRules).
- Around line 359-401: Add a security note to the "Basic authentication" docs
warning that the `password` value is inlined verbatim into the compiled bundle
(because `serializeRouteRule` in src/build/virtual/routing.ts uses
JSON.stringify to embed route rules), and instruct users to supply secrets via
build-time environment variables (e.g., process.env.ADMIN_PASSWORD) while
explicitly warning that the resolved string will still appear in the generated
JS artifact; include guidance similar to other security-sensitive docs advising
against committing secrets to source and recommending runtime proxying or
external secret management if secrecy is required.
In `@src/types/route-rules.ts`:
- Line 34: Update the NitroBasicAuthOptions type to match h3's BasicAuthOptions
by adding optional realm?: string and validate?: (username: string, password:
string) => boolean | Promise<boolean>; then ensure wherever
NitroBasicAuthOptions is forwarded into requireBasicAuth (the call in
route-rules handling) accepts and passes these fields through unchanged so realm
customization and custom validation functions are preserved.
In `@test/tests.ts`:
- Around line 433-436: Add a new test that verifies child route rules can
override a parent auth rule: create overlapping fixtures analogous to the docs
(e.g., a parent rule matching "/rules/auth/**" that requires auth and a child
rule matching "/rules/auth/public/**" with auth: false), then use the existing
callHandler helper to request a child path like "/rules/auth/public/test" and
assert a 200 response (instead of the parent's 401). Place the test alongside
the existing "skips auth when set to false" test and reference the same
callHandler helper and route patterns to ensure the route-rule merger correctly
prefers the child auth: false override.
---
Outside diff comments:
In `@src/build/virtual/routing.ts`:
- Around line 92-106: serializeRouteRule currently embeds full options
(including auth.password) into the compiled bundle via JSON.stringify; update
serializeRouteRule to strip or replace sensitive fields for the "auth" rule
before serialization (e.g., if name === "auth", clone options and delete or
replace the password field with a runtime reference token) and ensure consumers
resolve the actual secret at runtime (via useRuntimeConfig or process.env)
instead of emitting the literal password; keep existing behavior for other rule
names and preserve RuntimeRouteRules and handler emission (__routeRules__,
h._route, h._method) while serializing the sanitized options.
docs/1.docs/5.routing.md
Outdated
| ### Basic authentication | ||
|
|
||
| The `auth` route rule protects matching routes with [HTTP Basic Authentication](https://developer.mozilla.org/en-US/docs/Web/HTTP/Guides/Authentication). When a request hits a protected route, the browser prompts the user for credentials before granting access. | ||
|
|
||
| ```ts [nitro.config.ts] | ||
| import { defineNitroConfig } from "nitro/config"; | ||
|
|
||
| export default defineConfig({ | ||
| routeRules: { | ||
| '/admin/**': { | ||
| auth: { | ||
| username: 'admin', | ||
| password: 'supersecret', | ||
| } | ||
| } | ||
| } | ||
| }); | ||
| ``` | ||
|
|
||
| The `auth` option accepts the following properties: | ||
|
|
||
| | Property | Type | Required | Description | | ||
| | ---------- | -------- | -------- | ------------------------------------------------------------------ | | ||
| | `password` | `string` | Yes | Password for basic auth validation. | | ||
| | `username` | `string` | No | Username for basic auth validation. If omitted, any username is accepted. | | ||
| | `realm` | `string` | No | Realm for the basic auth challenge. Defaults to `""`. | | ||
|
|
||
| You can also explicitly disable authentication for a sub-route by setting `auth: false`: | ||
|
|
||
| ```ts [nitro.config.ts] | ||
| import { defineNitroConfig } from "nitro/config"; | ||
|
|
||
| export default defineConfig({ | ||
| routeRules: { | ||
| '/admin/**': { | ||
| auth: { password: 'supersecret' } | ||
| }, | ||
| '/admin/public/**': { | ||
| auth: false, | ||
| } | ||
| } | ||
| }); | ||
| ``` |
There was a problem hiding this comment.
Document that the password is embedded verbatim in the compiled bundle.
serializeRouteRule in src/build/virtual/routing.ts calls JSON.stringify(options) to inline every route rule into the generated JavaScript, so password ends up in the build artifact in plaintext. Consider adding a note directing users to supply the value at build time via an environment variable (process.env.ADMIN_PASSWORD) and warning that the resolved string will still be present in the bundle, similar to the guidance in other security-sensitive integrations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/1.docs/5.routing.md` around lines 359 - 401, Add a security note to the
"Basic authentication" docs warning that the `password` value is inlined
verbatim into the compiled bundle (because `serializeRouteRule` in
src/build/virtual/routing.ts uses JSON.stringify to embed route rules), and
instruct users to supply secrets via build-time environment variables (e.g.,
process.env.ADMIN_PASSWORD) while explicitly warning that the resolved string
will still appear in the generated JS artifact; include guidance similar to
other security-sensitive docs advising against committing secrets to source and
recommending runtime proxying or external secret management if secrecy is
required.
|
Will push fixes for those soon |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/build/virtual/routing.ts`:
- Line 3: RuntimeRouteRules currently includes "basicAuth" which causes
serializeRouteRule to JSON.stringify basicAuth options and embed plaintext
credentials in the generated virtual module; update the code to avoid embedding
secrets by either removing "basicAuth" from RuntimeRouteRules or changing
serializeRouteRule to treat basicAuth specially: detect when ruleName ===
"basicAuth" and if options contains a literal non-empty password/username do not
stringify them into the bundle but instead emit a consola.warn (use
consola.warn) informing the user to supply credentials via environment variables
or runtime secret injection, and when emitting the route rule reference ensure
it references placeholders or env-based lookups rather than inlined credentials;
reference RuntimeRouteRules and serializeRouteRule in your change so reviewers
can find the logic.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/1.docs/5.routing.mdsrc/build/virtual/routing.tssrc/runtime/internal/route-rules.tssrc/types/route-rules.tstest/fixture/nitro.config.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- test/fixture/nitro.config.ts
- src/types/route-rules.ts
- src/runtime/internal/route-rules.ts
- docs/1.docs/5.routing.md
| import type { Nitro, NitroEventHandler, NitroRouteRules } from "nitro/types"; | ||
|
|
||
| export const RuntimeRouteRules = ["headers", "redirect", "proxy", "cache"] as string[]; | ||
| export const RuntimeRouteRules = ["headers", "redirect", "proxy", "cache", "basicAuth"] as string[]; |
There was a problem hiding this comment.
Credentials will be embedded in plaintext in the compiled bundle.
Adding "basicAuth" to RuntimeRouteRules means serializeRouteRule (line 101) will call JSON.stringify(options) on the basicAuth options object, embedding password and username values directly as a string literal in the generated virtual module and therefore in the compiled server bundle.
Unlike other entries in RuntimeRouteRules (headers, redirect, proxy, cache), basicAuth explicitly carries secrets. Any party with read access to the bundle artifact (e.g., a deployed Docker image layer, edge-worker script, or accidental bundle exposure) can extract the credentials trivially.
Consider documenting this trade-off prominently and/or guiding users to supply credentials via environment variables, e.g.:
- "/admin/**": { basicAuth: { username: "admin", password: "secret" } }
+ "/admin/**": { basicAuth: { username: process.env.BASIC_AUTH_USER, password: process.env.BASIC_AUTH_PASS } }At minimum, a warning during the build step (via consola.warn) when basicAuth options contain a non-empty literal password would alert users who accidentally hardcode credentials. As per coding guidelines, consola should be used for logging in build/dev code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/build/virtual/routing.ts` at line 3, RuntimeRouteRules currently includes
"basicAuth" which causes serializeRouteRule to JSON.stringify basicAuth options
and embed plaintext credentials in the generated virtual module; update the code
to avoid embedding secrets by either removing "basicAuth" from RuntimeRouteRules
or changing serializeRouteRule to treat basicAuth specially: detect when
ruleName === "basicAuth" and if options contains a literal non-empty
password/username do not stringify them into the bundle but instead emit a
consola.warn (use consola.warn) informing the user to supply credentials via
environment variables or runtime secret injection, and when emitting the route
rule reference ensure it references placeholders or env-based lookups rather
than inlined credentials; reference RuntimeRouteRules and serializeRouteRule in
your change so reviewers can find the logic.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/tests.ts`:
- Line 398: The 401 lacks the WWW-Authenticate header because the route-rule
middleware calling h3's requireBasicAuth is not propagating the challenge
headers from the requireBasicAuth result/exception; inspect the middleware in
src/runtime/internal/route-rules.ts (the function that forwards options to
requireBasicAuth at around the referenced call) and update it so when
requireBasicAuth returns/throws it copies any WWW-Authenticate header(s) into
the outgoing response (e.g., setHeader('WWW-Authenticate', value) on the
event/res) before returning/throwing, or rethrow the original h3 response object
so its headers are preserved; after that, re-run the test in test/tests.ts to
confirm expect(headers["www-authenticate"]) receives 'Basic realm="Secure
Area"'.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
test/fixture/nitro.config.tstest/tests.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- test/fixture/nitro.config.ts
| }, | ||
| }); | ||
| expect(status).toBe(401); | ||
| expect(headers["www-authenticate"]).toBe('Basic realm="Secure Area"'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the fixture basicAuth config
find test -type f -name "*.ts" -o -name "*.js" | head -20Repository: nitrojs/nitro
Length of output: 773
🏁 Script executed:
# Search for basicAuth in test fixtures
rg -n "basicAuth" test/ --type=ts -A5 -B2Repository: nitrojs/nitro
Length of output: 721
🏁 Script executed:
# Search for WWW-Authenticate header handling in src
rg -n "WWW-Authenticate|www-authenticate" src/ --type=ts -A2 -B2Repository: nitrojs/nitro
Length of output: 39
🏁 Script executed:
# Check route-rules implementation
fd -t f "route-rules" src/ --exec cat -n {} \;Repository: nitrojs/nitro
Length of output: 9644
🏁 Script executed:
# Look for basicAuth in route-rules
rg -n "basicAuth" src/ --type=ts -A5 -B2Repository: nitrojs/nitro
Length of output: 1743
🏁 Script executed:
# Check the test file to see exact test setup at line 393-398
sed -n '393,398p' test/tests.tsRepository: nitrojs/nitro
Length of output: 265
🏁 Script executed:
# Look for any h3 version or documentation in package.json
cat package.json | grep -A2 -B2 "h3"Repository: nitrojs/nitro
Length of output: 460
🏁 Script executed:
# Check if there are any other basicAuth tests or examples in the codebase
rg -n "requireBasicAuth|basicAuth" test/ --type=ts -B2 -A2 | head -50Repository: nitrojs/nitro
Length of output: 571
🏁 Script executed:
# Check if there's a test setup or fixture initialization that might affect headers
sed -n '389,415p' test/tests.tsRepository: nitrojs/nitro
Length of output: 942
🏁 Script executed:
# Look for any callHandler function that processes the response
rg -n "callHandler" test/tests.ts -B3 -A10 | head -40Repository: nitrojs/nitro
Length of output: 1496
🏁 Script executed:
# Check the complete callHandler implementation to see how headers are extracted
sed -n '203,240p' test/tests.tsRepository: nitrojs/nitro
Length of output: 1175
🏁 Script executed:
# Look for the TestHandlerResult type definition to understand header structure
rg -n "TestHandlerResult" test/ --type=ts -B5 -A5Repository: nitrojs/nitro
Length of output: 1364
🏁 Script executed:
# Check if there's any middleware or handler setup that might affect basicAuth
rg -n "basicAuth|requireBasicAuth" test/fixture/ --type=ts -A5Repository: nitrojs/nitro
Length of output: 568
🏁 Script executed:
# Let's check h3 documentation or type definition for requireBasicAuth behavior
rg -n "requireBasicAuth" --type=ts -B3 -A10 | head -30Repository: nitrojs/nitro
Length of output: 1427
🏁 Script executed:
# Check if there are other basicAuth tests that pass to understand expected behavior
rg -n "www-authenticate|WWW-Authenticate" test/ --type=ts -B5 -A5Repository: nitrojs/nitro
Length of output: 617
🏁 Script executed:
# Let's check if there's any test output or debug logs that show actual header values
rg -n "Basic realm\|www-authenticate" test/ --type=tsRepository: nitrojs/nitro
Length of output: 39
🏁 Script executed:
# Check if there are any comments in the PR or test about expected h3 behavior
rg -n "h3|requireBasicAuth" test/tests.ts -B2 -A2 | grep -i "comment\|todo\|fixme\|note"Repository: nitrojs/nitro
Length of output: 39
🏁 Script executed:
# Let's check what h3 version might have this issue or if there are any known issues
cat package.json | grep -E "h3|version"Repository: nitrojs/nitro
Length of output: 250
🏁 Script executed:
# See if there's a test that sends no credentials to understand expected behavior difference
sed -n '380,420p' test/tests.tsRepository: nitrojs/nitro
Length of output: 1489
🏁 Script executed:
# Check if there's any documentation or comments about the basicAuth implementation
head -20 src/runtime/internal/route-rules.tsRepository: nitrojs/nitro
Length of output: 936
🏁 Script executed:
# Let's check if h3's requireBasicAuth is even being exported from h3 in package.json exports
rg -n "requireBasicAuth" package.jsonRepository: nitrojs/nitro
Length of output: 39
🏁 Script executed:
# Check if there are any setup/teardown issues or middleware that might strip headers
sed -n '140,200p' test/tests.ts | grep -A10 -B10 "describe\|middleware"Repository: nitrojs/nitro
Length of output: 39
🌐 Web query:
h3 requireBasicAuth WWW-Authenticate header behavior
💡 Result:
In h3 (the UnJS/h3 framework), requireBasicAuth(event, opts) enforces HTTP Basic Auth for the current request; when credentials are missing or invalid, it fails the request with HTTP 401 Unauthorized and (to comply with the HTTP auth challenge flow / to trigger browser credential prompts) it sends a WWW-Authenticate challenge for the Basic scheme. The docs list requireBasicAuth as the utility for applying Basic Auth to a request. [1]
Per the HTTP authentication spec, a 401 response for a protected resource must include at least one WWW-Authenticate header describing the challenge (e.g., Basic realm="..."). [2]
Sources:
[1] h3 Security utils (basicAuth, requireBasicAuth) — https://www.h3.dev/utils/security
[2] MDN: WWW-Authenticate header (401 must include it; Basic uses realm) — https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/WWW-Authenticate
🌐 Web query:
h3 2.0.1-rc.14 basicAuth implementation
💡 Result:
In h3 2.0.1-rc.14, basicAuth() is a middleware factory for HTTP Basic Authentication that’s meant to be used either:
- as route middleware:
app.get("/", handler, [basicAuth({ password })])[1], or - imperatively inside a handler via
await requireBasicAuth(event, opts)[1].
What it does (behavior / flow)
At request time, the middleware:
- Reads the
Authorizationheader (expects theBasic <base64>scheme). - Decodes the base64 payload into
username:password. - Validates the provided password against the configured password (and/or other allowed credentials depending on options).
- On success, it stores auth info on the event context so you can access the username at
event.context.basicAuth.username[1]. - On failure, it responds with 401 Unauthorized and a
WWW-Authenticate: Basic …challenge header (standard Basic Auth behavior).
Security-related implementation details present in the v2 RCs
The v2 RC line includes hardening specifically for Basic Auth comparisons:
- It uses random jitter and a constant-time string comparison to reduce timing side-channels (added in the
2.0.1-rc.9fixes). [2] - In
2.0.1-rc.14, the jitter generator was further strengthened (“Enhance randomJitter function for cryptographic security”). [3]
Minimal “shape” of the implementation (pseudocode)
export function basicAuth(opts) {
return async (event) => {
await requireBasicAuth(event, opts) // throws / returns 401 response on failure
}
}Where requireBasicAuth(event, opts) performs the header parsing + decode, then validates using constant-time comparison plus jitter, and sets event.context.basicAuth = { username } on success. [1] [2] [3]
Sources: [1] (H3 security utils docs), [2] (release note mentioning jitter + constant-time compare), [3] (rc.14 release note about stronger random jitter).
www-authenticate header is missing from 401 response — blocking test failure across all CI environments
The test at line 398 expects the header but receives undefined, while the preceding line's 401 status check passes. This confirms the response has the correct status but not the required HTTP challenge header.
The fixture configuration is correct (realm: "Secure Area" at test/fixture/nitro.config.ts:110), and the route-rules implementation properly forwards the entire options object—including realm—to h3's requireBasicAuth (src/runtime/internal/route-rules.ts:89). Per HTTP spec and h3 v2.0.1-rc.14 documentation, requireBasicAuth should emit a WWW-Authenticate header when credentials are invalid.
The actual root cause lies either in h3's requireBasicAuth behavior in rc.14 or in how the route-rule middleware chain applies basicAuth to the event. This requires investigation of the actual header value being returned or verification that the basicAuth route rule is properly invoking requireBasicAuth and propagating its response headers.
🧰 Tools
🪛 GitHub Check: tests-rolldown (ubuntu-latest)
[failure] 398-398: test/presets/standard.test.ts > nitro:standard > handles route rules - basic auth > rejects request with bad creds
AssertionError: expected undefined to be 'Basic realm="Secure Area"' // Object.is equality
- Expected:
"Basic realm="Secure Area""
- Received:
undefined
❯ test/tests.ts:398:43
[failure] 398-398: test/presets/node.test.ts > nitro:preset:node-middleware > handles route rules - basic auth > rejects request with bad creds
AssertionError: expected undefined to be 'Basic realm="Secure Area"' // Object.is equality
- Expected:
"Basic realm="Secure Area""
- Received:
undefined
❯ test/tests.ts:398:43
[failure] 398-398: test/presets/nitro-dev.test.ts > nitro:preset:nitro-dev > handles route rules - basic auth > rejects request with bad creds
AssertionError: expected undefined to be 'Basic realm="Secure Area"' // Object.is equality
- Expected:
"Basic realm="Secure Area""
- Received:
undefined
❯ test/tests.ts:398:43
[failure] 398-398: test/presets/netlify.test.ts > nitro:preset:netlify > handles route rules - basic auth > rejects request with bad creds
AssertionError: expected undefined to be 'Basic realm="Secure Area"' // Object.is equality
- Expected:
"Basic realm="Secure Area""
- Received:
undefined
❯ test/tests.ts:398:43
[failure] 398-398: test/presets/deno-server.test.ts > nitro:preset:deno-server > handles route rules - basic auth > rejects request with bad creds
AssertionError: expected undefined to be 'Basic realm="Secure Area"' // Object.is equality
- Expected:
"Basic realm="Secure Area""
- Received:
undefined
❯ test/tests.ts:398:43
[failure] 398-398: test/presets/cloudflare-pages.test.ts > nitro:preset:cloudflare-pages > handles route rules - basic auth > rejects request with bad creds
AssertionError: expected undefined to be 'Basic realm="Secure Area"' // Object.is equality
- Expected:
"Basic realm="Secure Area""
- Received:
undefined
❯ test/tests.ts:398:43
[failure] 398-398: test/presets/cloudflare-module.test.ts > nitro:preset:cloudflare-module > handles route rules - basic auth > rejects request with bad creds
AssertionError: expected undefined to be 'Basic realm="Secure Area"' // Object.is equality
- Expected:
"Basic realm="Secure Area""
- Received:
undefined
❯ test/tests.ts:398:43
[failure] 398-398: test/presets/bun.test.ts > nitro:preset:bun > handles route rules - basic auth > rejects request with bad creds
AssertionError: expected undefined to be 'Basic realm="Secure Area"' // Object.is equality
- Expected:
"Basic realm="Secure Area""
- Received:
undefined
❯ test/tests.ts:398:43
[failure] 398-398: test/presets/aws-lambda.test.ts > nitro:preset:aws-lambda-v1 > handles route rules - basic auth > rejects request with bad creds
AssertionError: expected undefined to be 'Basic realm="Secure Area"' // Object.is equality
- Expected:
"Basic realm="Secure Area""
- Received:
undefined
❯ test/tests.ts:398:43
[failure] 398-398: test/presets/aws-lambda.test.ts > nitro:preset:aws-lambda-v2 > handles route rules - basic auth > rejects request with bad creds
AssertionError: expected undefined to be 'Basic realm="Secure Area"' // Object.is equality
- Expected:
"Basic realm="Secure Area""
- Received:
undefined
❯ test/tests.ts:398:43
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/tests.ts` at line 398, The 401 lacks the WWW-Authenticate header because
the route-rule middleware calling h3's requireBasicAuth is not propagating the
challenge headers from the requireBasicAuth result/exception; inspect the
middleware in src/runtime/internal/route-rules.ts (the function that forwards
options to requireBasicAuth at around the referenced call) and update it so when
requireBasicAuth returns/throws it copies any WWW-Authenticate header(s) into
the outgoing response (e.g., setHeader('WWW-Authenticate', value) on the
event/res) before returning/throwing, or rethrow the original h3 response object
so its headers are preserved; after that, re-run the test in test/tests.ts to
confirm expect(headers["www-authenticate"]) receives 'Basic realm="Secure
Area"'.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
docs/1.docs/5.routing.md (1)
359-401:⚠️ Potential issue | 🟠 MajorAdd a security note about the password being inlined verbatim into the compiled bundle.
Because
serializeRouteRuleinsrc/build/virtual/routing.tsusesJSON.stringifyto embed route rules into the generated JavaScript, anypasswordvalue provided directly innitro.config.tswill appear in plaintext in the build artifact. Warn users to supply credentials via build-time environment variables (e.g.process.env.ADMIN_PASSWORD) and note that the resolved string will still be present in the bundle.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/1.docs/5.routing.md` around lines 359 - 401, Add a security note to the "Basic authentication" docs warning that passwords provided in nitro.config.ts are inlined verbatim into the compiled bundle because serializeRouteRule in src/build/virtual/routing.ts uses JSON.stringify to embed route rules; instruct users to avoid hardcoding secrets and instead reference build-time environment variables (e.g. process.env.ADMIN_PASSWORD) or supply credentials at runtime, and explicitly note that resolving an env var at build time still results in the plaintext string appearing in the bundle.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/1.docs/5.routing.md`:
- Around line 361-401: The docs' "Basic authentication" section incorrectly uses
the auth key; update all occurrences to basicAuth: replace mentions in prose
("the `auth` route rule", "The `auth` option accepts...", "setting `auth:
false`") to use `basicAuth`, and change every code example and object key
(`auth: { ... }`, `auth: false`) to `basicAuth: { ... }` or `basicAuth: false`
so they match the NitroRouteConfig and runtime handling (references: basicAuth,
auth).
- Line 340: The "Basic authentication" examples use the wrong property name
`auth`; update all examples in that section to use `basicAuth` (the actual
NitroRouteConfig property) instead of `auth` so the config examples match the
overview and compile; search for occurrences of `auth:` in the "Basic
authentication" examples and replace them with `basicAuth:` while keeping the
same values and surrounding structure (e.g., in the route config objects shown
alongside defineConfig/NitroRouteConfig).
---
Duplicate comments:
In `@docs/1.docs/5.routing.md`:
- Around line 359-401: Add a security note to the "Basic authentication" docs
warning that passwords provided in nitro.config.ts are inlined verbatim into the
compiled bundle because serializeRouteRule in src/build/virtual/routing.ts uses
JSON.stringify to embed route rules; instruct users to avoid hardcoding secrets
and instead reference build-time environment variables (e.g.
process.env.ADMIN_PASSWORD) or supply credentials at runtime, and explicitly
note that resolving an env var at build time still results in the plaintext
string appearing in the bundle.
docs/1.docs/5.routing.md
Outdated
|
|
||
| ```ts [nitro.config.ts] | ||
| import { defineNitroConfig } from "nitro/config"; | ||
| import { defineConfig } from "nitro/config"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the config.ts file for defineConfig export
echo "=== Checking src/config.ts for defineConfig export ==="
fd "config\.ts" src --exec grep -n "export.*defineConfig" {} \;
echo -e "\n=== Checking src/index.ts for defineConfig export ==="
rg -n "defineConfig" src/index.ts
echo -e "\n=== Checking package.json exports map ==="
cat package.json | jq '.exports | select(. != null)'
echo -e "\n=== Checking for defineNitroConfig in src/config.ts ==="
rg -n "defineNitroConfig" src/config.ts -A 2
echo -e "\n=== Checking for defineConfig definition in src ==="
rg -n "export.*defineConfig\s*=" src/Repository: nitrojs/nitro
Length of output: 1324
🏁 Script executed:
#!/bin/bash
# Check the actual documentation file for auth vs basicAuth usage
echo "=== Lines 340-401 of docs/1.docs/5.routing.md ==="
sed -n '340,401p' docs/1.docs/5.routing.md
echo -e "\n=== Count of 'auth' vs 'basicAuth' in the relevant section ==="
sed -n '340,401p' docs/1.docs/5.routing.md | grep -o "basicAuth\|'auth'" | sort | uniq -cRepository: nitrojs/nitro
Length of output: 2313
🏁 Script executed:
#!/bin/bash
# Check the route rules type definition to see what the actual key is
echo "=== Searching for basicAuth in source code ==="
rg -n "basicAuth" src/ -t ts
echo -e "\n=== Searching for auth in route rules context ==="
rg -n "auth.*?:" src/ -t ts -A 1 -B 1 | head -50
echo -e "\n=== Check NitroConfig or route rules interface ==="
rg -n "interface.*RouteRule\|type.*RouteRule" src/ -t ts -A 20 | head -60Repository: nitrojs/nitro
Length of output: 1676
🏁 Script executed:
#!/bin/bash
# Get the full route rules type definition
echo "=== Full route-rules.ts type definitions ==="
cat src/types/route-rules.ts | head -50
echo -e "\n=== Context around basicAuth export in route-rules.ts ==="
rg -n "basicAuth" src/runtime/internal/route-rules.ts -B 2 -A 5Repository: nitrojs/nitro
Length of output: 2152
Fix incorrect property name in "Basic authentication" section: use basicAuth instead of auth.
The overview example (line 352) correctly uses basicAuth, but the dedicated "Basic authentication" section (lines 361-401) inconsistently uses auth. The actual property name in NitroRouteConfig is basicAuth (not auth). This will cause users following the dedicated section examples to encounter configuration errors. All three code examples in the section (lines 364, 388, 393) should use basicAuth instead of auth.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/1.docs/5.routing.md` at line 340, The "Basic authentication" examples
use the wrong property name `auth`; update all examples in that section to use
`basicAuth` (the actual NitroRouteConfig property) instead of `auth` so the
config examples match the overview and compile; search for occurrences of
`auth:` in the "Basic authentication" examples and replace them with
`basicAuth:` while keeping the same values and surrounding structure (e.g., in
the route config objects shown alongside defineConfig/NitroRouteConfig).
docs/1.docs/5.routing.md
Outdated
| The `auth` route rule protects matching routes with [HTTP Basic Authentication](https://developer.mozilla.org/en-US/docs/Web/HTTP/Guides/Authentication). When a request hits a protected route, the browser prompts the user for credentials before granting access. | ||
|
|
||
| ```ts [nitro.config.ts] | ||
| import { defineConfig } from "nitro/config"; | ||
|
|
||
| export default defineConfig({ | ||
| routeRules: { | ||
| '/admin/**': { | ||
| auth: { | ||
| username: 'admin', | ||
| password: 'supersecret', | ||
| } | ||
| } | ||
| } | ||
| }); | ||
| ``` | ||
|
|
||
| The `auth` option accepts the following properties: | ||
|
|
||
| | Property | Type | Required | Description | | ||
| | ---------- | -------- | -------- | ------------------------------------------------------------------ | | ||
| | `password` | `string` | Yes | Password for basic auth validation. | | ||
| | `username` | `string` | No | Username for basic auth validation. If omitted, any username is accepted. | | ||
| | `realm` | `string` | No | Realm for the basic auth challenge. Defaults to `""`. | | ||
|
|
||
| You can also explicitly disable authentication for a sub-route by setting `auth: false`: | ||
|
|
||
| ```ts [nitro.config.ts] | ||
| import { defineConfig } from "nitro/config"; | ||
|
|
||
| export default defineConfig({ | ||
| routeRules: { | ||
| '/admin/**': { | ||
| auth: { password: 'supersecret' } | ||
| }, | ||
| '/admin/public/**': { | ||
| auth: false, | ||
| } | ||
| } | ||
| }); | ||
| ``` |
There was a problem hiding this comment.
All code examples and prose in the dedicated section use auth, but the correct route rule key is basicAuth.
The overview snippet at line 354 (and the actual NitroRouteConfig type from the AI summary) uses basicAuth, but every code block and every piece of inline text in the "Basic authentication" section uses auth instead. Users following this section will configure a auth rule that the runtime doesn't recognise, silently receiving no protection.
Affected locations:
- Line 361:
"The \auth` route rule…"→basicAuth` - Line 369 & 394:
auth: {…}→basicAuth: {…} - Line 378:
"The \auth` option accepts…"→basicAuth` - Line 386:
"setting \auth: false`"→basicAuth: false` - Line 397:
auth: false→basicAuth: false
📝 Proposed fix — align the dedicated section with the `basicAuth` key
-The `auth` route rule protects matching routes with [HTTP Basic Authentication]...
+The `basicAuth` route rule protects matching routes with [HTTP Basic Authentication]...
```ts [nitro.config.ts]
import { defineConfig } from "nitro/config";
export default defineConfig({
routeRules: {
'/admin/**': {
- auth: {
+ basicAuth: {
username: 'admin',
password: 'supersecret',
}
}
}
});-The auth option accepts the following properties:
+The basicAuth option accepts the following properties:
...
-You can also explicitly disable authentication for a sub-route by setting auth: false:
+You can also explicitly disable authentication for a sub-route by setting basicAuth: false:
import { defineConfig } from "nitro/config";
export default defineConfig({
routeRules: {
'/admin/**': {
- auth: { password: 'supersecret' }
+ basicAuth: { password: 'supersecret' }
},
'/admin/public/**': {
- auth: false,
+ basicAuth: false,
}
}
});</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @docs/1.docs/5.routing.md around lines 361 - 401, The docs' "Basic
authentication" section incorrectly uses the auth key; update all occurrences to
basicAuth: replace mentions in prose ("the auth route rule", "The auth
option accepts...", "setting auth: false") to use basicAuth, and change
every code example and object key (auth: { ... }, auth: false) to
basicAuth: { ... } or basicAuth: false so they match the NitroRouteConfig
and runtime handling (references: basicAuth, auth).
</details>
<!-- fingerprinting:phantom:triton:churro -->
<!-- This is an auto-generated comment by CodeRabbit -->
RihanArfan
force-pushed
the
feat/auth-route-rules
branch
from
dcc0b08 to
8206228
Compare
2 participants
🔗 Linked issue
Closes #3924
❓ Type of change
📚 Description
Adds support for configuring basic auth through route rules
📝 Checklist