Conversation
…rrides Add pnpm overrides to pin vulnerable transitive dependencies pulled in by [email protected] and [email protected] (both already at their latest versions): - simple-git >=3.32.3 (critical: RCE via protocol bypass) - rollup >=4.59.0 (high: arbitrary file write) - serialize-javascript >=7.0.3 (high: RCE) - svgo >=4.0.1 (high: DoS) - tar >=7.5.11 (high: path traversal) - minimatch@5/9/10 (high: ReDoS, multiple ranges) - devalue >=5.6.3 (low: CPU/memory amplification) - qs >=6.14.2 (low: DoS, tighten existing override)
Upholds the library's zero-dependencies principle by eliminating all
runtime imports from h3 and defu in the nuxt package.
defu:
- Replace two defu merge calls with an inline mergeDefaults<T> utility
h3 (adapter.ts):
- Use event.method, event.headers, event.path natively (H3Event Web API)
- Inline cookie parsing (parseCookieHeader) and serialization (serializeCookie)
- Read body via event.node.req stream instead of readBody()
- Set response headers/cookies via event.node.res instead of h3 helpers
- Extract parseBody() as a standalone function for clarity
- Pass event.headers directly to CsrfRequest (no Map conversion needed)
- Read request.cookies from the pre-parsed Map instead of re-parsing
the cookie header inside getTokenFromRequest
h3 (middleware.ts):
- Import defineEventHandler from #imports (Nuxt auto-import) instead of h3
- Replace createError with an inline Error + statusCode assignment
All imports from h3 are now type-only (import type { H3Event }),
erased at compile time with zero runtime cost. h3 removed from
peerDependencies since it is always provided transitively by nuxt.
Tests rewritten to mock H3Event properties directly using real
node:stream Readable instances instead of mocking h3 module functions.
Contributor
WalkthroughA new Nuxt module for CSRF protection is added. It introduces a NuxtAdapter that uses native H3Event/Node APIs for request/response handling, server middleware enforcing CSRF, client composables (useCsrfToken, useCsrfFetch) and a client plugin, plus docs and extensive test rewrites using in-memory mocks. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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}
Contributor
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)
packages/nuxt/src/runtime/server/middleware.ts (1)
34-38:⚠️ Potential issue | 🔴 CriticalThe statusCode property shall not be honored upon this plain Error!
Alas, I must bring grave tidings. The H3 framework recognizes the
statusCodeproperty only when thrown as anH3Errorcreated viacreateError. Thy approach of adorning a plainErrorwith this property shall avail thee naught—H3 shall treat it as an unhandled error and return a 500 status to the client, not the intended 403 Forbidden.Use
createError({ statusCode: 403, statusMessage: 'CSRF validation failed', data: { reason: result.reason } })instead, for that is the magic that H3 understands. Without it, thy CSRF protection shall fail silently to the user's eye.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nuxt/src/runtime/server/middleware.ts` around lines 34 - 38, Replace the plain Error thrown in middleware.ts (the throw using Object.assign(new Error('CSRF validation failed'), { statusCode: 403, statusMessage: 'CSRF validation failed', data: { reason: result.reason } })) with an H3 error created via createError so H3 will honor the status; call createError({ statusCode: 403, statusMessage: 'CSRF validation failed', data: { reason: result.reason } }) in place of the current throw, ensuring you import createError from h3 if not already present and keep the same message and data payload.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/nuxt/README.md`:
- Around line 15-20: The TOC links for the security sections are invalid because
they include a leading hyphen from the emoji; update the links referenced as
'#-security-strategies' and '#-security-best-practices' to match the actual
GitHub anchors (e.g., '#security-strategies' and '#security-best-practices') or
add explicit HTML anchors next to the headings (e.g., an <a
name="security-strategies"> or <a id="security-strategies">) so the links from
the README TOC resolve correctly for the headings "🛡️ Security Strategies" and
"🔒 Security Best Practices".
In `@packages/nuxt/src/runtime/server/adapter.ts`:
- Around line 63-93: The code currently treats 'multipart/form-data' as
supported in the supportedTypes array but then returns the rawBody string for
non-JSON content, which is unsafe for multipart bodies; either remove
'multipart/form-data' from the supportedTypes array so multipart requests won't
be parsed by the function, or implement proper multipart parsing (using a
multipart parser library) before returning body data; update references in the
supportedTypes constant and the contentType checks (the supportedTypes array and
the code paths that use contentType.startsWith(...)) so multipart is no longer
accepted unless a full multipart parser is added.
In `@packages/nuxt/test/adapter.test.ts`:
- Around line 550-559: The non-null assertion on csrfResponses[i]! in the
Promise.all mapping for events can break static analysis; update the
adapter.applyResponse call to safely handle missing csrfResponses by removing
the '!' and explicitly guarding the index (e.g., compute const csrf =
csrfResponses[i]; if (!csrf) throw or pass a safe fallback) before calling
adapter.applyResponse, or better yet zip events and csrfResponses into pairs and
map over those pairs so adapter.applyResponse is always invoked with a defined
csrfResponse; reference adapter.applyResponse, events, csrfResponses and the
Promise.all mapping when making the change.
- Around line 500-509: The test uses a non-null assertion on cookies[0] which
triggers linter warnings; replace the assertion with a guarded access when
constructing cookieStr (e.g., const cookieStr = cookies[0] ?? ''; or const
cookieStr = cookies[0] ?? throw new Error('expected cookie')); and/or add an
explicit expect(cookies[0]).toBeDefined() before using it; update the reference
in the assertions that follow to use this guarded cookieStr
(functions/variables: getSetCookies, mockEvent.node.res, cookies, cookieStr) so
the linter no longer complains.
---
Outside diff comments:
In `@packages/nuxt/src/runtime/server/middleware.ts`:
- Around line 34-38: Replace the plain Error thrown in middleware.ts (the throw
using Object.assign(new Error('CSRF validation failed'), { statusCode: 403,
statusMessage: 'CSRF validation failed', data: { reason: result.reason } }))
with an H3 error created via createError so H3 will honor the status; call
createError({ statusCode: 403, statusMessage: 'CSRF validation failed', data: {
reason: result.reason } }) in place of the current throw, ensuring you import
createError from h3 if not already present and keep the same message and data
payload.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 874c8e08-caad-4566-98fa-2f782367e55c
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
.changeset/add-nuxt-module.mdpackage.jsonpackages/nuxt/README.mdpackages/nuxt/package.jsonpackages/nuxt/src/module.tspackages/nuxt/src/runtime/composables/useCsrfToken.tspackages/nuxt/src/runtime/server/adapter.tspackages/nuxt/src/runtime/server/middleware.tspackages/nuxt/test/adapter.test.tspackages/nuxt/test/middleware.test.ts
💤 Files with no reviewable changes (1)
- packages/nuxt/package.json
Multipart bodies cannot be correctly parsed by the URLSearchParams fallback, so the CSRF token could never be extracted from them anyway. Multipart requests should include the token via header or cookie instead.
- Remove multipart/form-data from body parser (cannot be correctly parsed) - Replace non-null assertions with ?? '' and biome-ignore comments - Use literal key access for headerInit.cookie
Contributor
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 `@packages/nuxt/test/adapter.test.ts`:
- Around line 134-136: The test falsely casts result.headers to
Map<string,string>; change the type assertion to the Web Headers type used by
the adapter (e.g., const headers = result.headers as Headers) and update the
variable name (headers or headersObj) and assertions to use that Headers
instance (headers.get('content-type'), headers.get('x-csrf-token')) so the test
type accurately reflects the adapter's return value (refer to result.headers and
headersMap in the test).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c43a14f1-0001-436a-af6e-c6b60fd4fd2c
📒 Files selected for processing (2)
packages/nuxt/src/runtime/server/adapter.tspackages/nuxt/test/adapter.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
feat(nuxt): polish @csrf-armor/nuxt — zero dependencies, docs, and security fixes
Summary
This PR makes some improvements to the
@csrf-armor/nuxtmodule (merged via #39, Thank you @Jorgagu). It covers three areas: transitive dependency vulnerability fixes, a full refactor to eliminate runtime dependencies, and documentation.1. Fix transitive dependency vulnerabilities (
package.json,pnpm-lock.yaml)All direct dependencies were already at their latest versions, so vulnerabilities were fixed by adding
pnpm.overridesto force patched versions of affected transitive packages:simple-gitrollupserialize-javascriptsvgotarminimatchdevalueqs2. Remove
h3anddefuruntime dependencies (packages/nuxt/**)The library's core principle is zero runtime dependencies beyond
@csrf-armor/core. The initial Nuxt module violated this by depending onh3helper functions anddefuat runtime.Changes:
adapter.ts— Replaced allh3utility calls (getCookie,setCookie,getHeader,readBody, etc.) with native H3Event Web API properties (event.method,event.headers,event.path,event.node.req/res) and inline helpers for cookie parsing/serialization and body reading.import type { H3Event } from 'h3'is type-only and erased at compile time.middleware.ts—defineEventHandleranduseRuntimeConfigare now resolved via Nuxt's#importsvirtual module (build-time, zero runtime cost).createErrorreplaced with an inlineObject.assign(new Error(...), { statusCode }). This also fixes a deprecation warning from h3 about implicit event handler conversion.module.ts—defureplaced with a 20-line inlinemergeDefaultsdeep-merge utility.h3anddefuremoved frompeerDependencies.vi.mock('h3', ...)mocking. Mock events now use realnode:streamReadable instances and the WebHeadersAPI, matching the adapter's actual expectations.3. Add README (
packages/nuxt/README.md)New README consistent with the
@csrf-armor/nextjspackage, covering installation, module registration, composable usage, all five security strategies, configuration reference, and security best practices.4. Add changeset (
.changeset/add-nuxt-module.md)minorbump for@csrf-armor/nuxtcovering the initial module addition and the zero-dependency refactor.Test plan
pnpm --filter @csrf-armor/nuxt test— all adapter and middleware integration tests passpnpm --filter @csrf-armor/nuxt type-check— no TypeScript errorspnpm build— full monorepo build succeedspnpm audit— no unresolved vulnerabilities at or above moderate severity@csrf-armor/nuxtpackagedependenciescontains only@csrf-armor/coreSummary by CodeRabbit
New Features
Documentation
Refactor
Tests
Chores