feat(security): HMAC signing infrastructure for proxy endpoints#699
Merged
feat(security): HMAC signing infrastructure for proxy endpoints#699
Conversation
Adds the foundation for locking down proxy endpoints (Google Maps, Gravatar, embed image proxies) against cost/quota abuse. Signed URLs are generated server-side during SSR/prerender and verified at the endpoint so only legitimate requests reach upstream APIs. - sign/verify primitives with canonical query form and constant-time compare - withSigning handler wrapper (opt-in per endpoint via requiresSigning flag) - Secret management: env var > config > dev auto-generate to .env - /_scripts/sign endpoint for reactive client-side URLs (origin + rate-limited) - nuxt-scripts CLI with generate-secret command for explicit secret setup No existing endpoints are wired to signing yet; this PR is infrastructure only.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
commit: |
|
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:
📝 WalkthroughWalkthroughAdds HMAC-based signing to the Nuxt Scripts package and a small CLI for secret management. New utilities canonicalize query parameters, compute/truncate HMAC-SHA256 signatures, build/verify signed proxy URLs, and generate/verify time-limited page tokens. Module changes resolve a proxy secret from config/env or auto-generate in dev (with optional .env persistence) and expose it via runtimeConfig. A withSigning wrapper enforces verification for handlers; multiple proxy/embed handlers and registry entries were marked as requiring signing. Tests for signing/token logic and package CLI/build/package.json updates were added. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
test/unit/sign.test.ts (1)
39-47: Minor: Test description casing.Test descriptions at lines 39 and 44 have unusual casing: "uRL-encodes" and "jSON-stringifies". Consider "URL-encodes" and "JSON-stringifies" for consistency.
📝 Suggested fix
- it('uRL-encodes keys and values', () => { + it('URL-encodes keys and values', () => { expect(canonicalizeQuery({ 'q': 'hello world', 'a+b': 'c&d' })) .toBe('a%2Bb=c%26d&q=hello%20world') }) - it('jSON-stringifies object values for stable comparison', () => { + it('JSON-stringifies object values for stable comparison', () => { expect(canonicalizeQuery({ style: { color: 'red' } })) .toBe('style=%7B%22color%22%3A%22red%22%7D') })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/sign.test.ts` around lines 39 - 47, Update the two test descriptions in sign.test.ts to use correct casing: change "uRL-encodes keys and values" to "URL-encodes keys and values" and "jSON-stringifies object values for stable comparison" to "JSON-stringifies object values for stable comparison"; these are the it(...) descriptions for the tests asserting canonicalizeQuery behavior, so edit the string arguments passed to the it() calls that reference canonicalizeQuery accordingly.packages/script/src/runtime/server/sign-proxy.ts (1)
110-110: Remove unnecessaryas anycast on storage options.The
setItemmethod fromuseStorage()(unstorage) acceptsopts?: TransactionOptionswhereTransactionOptions = Record<string, any>. The object{ ttl: 120 }already conforms to this type, making theas anycast redundant and masking potential type issues. Remove the cast.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/script/src/runtime/server/sign-proxy.ts` at line 110, The call to storage.setItem(rateLimitKey, currentCount + 1, { ttl: 120 } as any) uses an unnecessary and masking cast; remove the cast so the options object is passed with its natural type. Update the invocation of storage.setItem in sign-proxy.ts (referencing storage.setItem and rateLimitKey) to await storage.setItem(rateLimitKey, currentCount + 1, { ttl: 120 }) and ensure no other code relies on the as any cast.
🤖 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/script/src/module.ts`:
- Around line 119-120: The current PROXY_SECRET_ENV_VALUE_RE
/^NUXT_SCRIPTS_PROXY_SECRET=(.+)$/m will capture inline comments (e.g. "abc #
comment"); update the extraction so the captured value stops before a "#" or
end-of-line and still allows trimming. Replace or modify
PROXY_SECRET_ENV_VALUE_RE to capture only characters up to a "#" or line ending
(for example using a pattern like capturing [^#\r\n]*), and keep the existing
.trim() usage when reading the value; also ensure PROXY_SECRET_ENV_LINE_RE
remains consistent with the new value-detection logic.
In `@packages/script/src/runtime/server/sign-proxy.ts`:
- Around line 96-110: The rate limiter currently trusts X-Forwarded-For via
getRequestIP(event, { xForwardedFor: true }), which can be spoofed unless behind
a trusted proxy; make this behavior explicit and configurable: add a runtime
option (e.g., trustXForwardedFor or trustedProxy) to the sign-proxy
configuration and use that flag when calling getRequestIP instead of the
hard-coded { xForwardedFor: true }, and update the threat-model/docs text to
state the trusted-proxy assumption; keep existing defaults to preserve current
deployments (or set default to false) and ensure references to
RATE_LIMIT_KEY_PREFIX, RATE_LIMIT_PER_MINUTE, and useStorage remain unchanged.
In `@packages/script/src/runtime/server/utils/sign.ts`:
- Around line 137-151: The doc comment on constantTimeEqual is misleading: it
states the loop runs over the longer length to avoid leaking length, but the
implementation actually early-returns when lengths differ. Update the comment
for the function constantTimeEqual to accurately describe behavior (e.g., state
that it first checks lengths and returns false on mismatch, then performs a
constant-time comparison over a.length), and optionally mention that callers
like verifyProxyRequest and signProxyUrl ensure fixed-length inputs so the
length check is normally redundant.
---
Nitpick comments:
In `@packages/script/src/runtime/server/sign-proxy.ts`:
- Line 110: The call to storage.setItem(rateLimitKey, currentCount + 1, { ttl:
120 } as any) uses an unnecessary and masking cast; remove the cast so the
options object is passed with its natural type. Update the invocation of
storage.setItem in sign-proxy.ts (referencing storage.setItem and rateLimitKey)
to await storage.setItem(rateLimitKey, currentCount + 1, { ttl: 120 }) and
ensure no other code relies on the as any cast.
In `@test/unit/sign.test.ts`:
- Around line 39-47: Update the two test descriptions in sign.test.ts to use
correct casing: change "uRL-encodes keys and values" to "URL-encodes keys and
values" and "jSON-stringifies object values for stable comparison" to
"JSON-stringifies object values for stable comparison"; these are the it(...)
descriptions for the tests asserting canonicalizeQuery behavior, so edit the
string arguments passed to the it() calls that reference canonicalizeQuery
accordingly.
🪄 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: 0bfb584a-a88c-48fb-a41f-e28da3302414
📒 Files selected for processing (10)
packages/script/bin/cli.mjspackages/script/build.config.tspackages/script/package.jsonpackages/script/src/cli.tspackages/script/src/module.tspackages/script/src/runtime/server/sign-proxy.tspackages/script/src/runtime/server/utils/sign.tspackages/script/src/runtime/server/utils/withSigning.tspackages/script/src/runtime/types.tstest/unit/sign.test.ts
There was a problem hiding this comment.
Pull request overview
Adds opt-in HMAC signing infrastructure to protect proxy endpoints (that inject server-side API keys / forward third-party requests) from direct quota-abuse by requiring server-generated signed URLs.
Changes:
- Introduces signing primitives (
canonicalizeQuery, HMAC signing, request verification) plus awithSigning()middleware wrapper for proxy handlers. - Adds secret resolution/management in the Nuxt module (env/config + dev auto-generation) and publishes signable route allowlists to runtime config.
- Adds a
/_scripts/signserver endpoint and annpx @nuxt/scripts generate-secretCLI for obtaining signatures without exposing the secret.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unit/sign.test.ts | Adds unit tests for query canonicalization, signing determinism, and constant-time comparison. |
| packages/script/src/runtime/types.ts | Adds requiresSigning?: boolean flag to server handler metadata. |
| packages/script/src/runtime/server/utils/withSigning.ts | Adds middleware wrapper enforcing signature verification on handlers. |
| packages/script/src/runtime/server/utils/sign.ts | Implements canonicalization, signing, verification, and constant-time compare utilities. |
| packages/script/src/runtime/server/sign-proxy.ts | Adds /_scripts/sign endpoint with allowlist, same-origin check, and rate limiting. |
| packages/script/src/module.ts | Adds proxy-secret resolution (env/config/dev generation), signable route collection, and /sign handler registration. |
| packages/script/src/cli.ts | Adds CLI implementation for generate-secret. |
| packages/script/package.json | Publishes a nuxt-scripts bin entry and includes bin/ in package files. |
| packages/script/build.config.ts | Includes CLI entry in build. |
| packages/script/bin/cli.mjs | Adds Node entrypoint that loads the built CLI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Collaborator
Author
Code reviewFound 1 issue:
scripts/packages/script/src/cli.ts Lines 3 to 7 in e8434f1 scripts/packages/script/src/runtime/server/sign-proxy.ts Lines 5 to 8 in e8434f1 scripts/packages/script/src/module.ts Lines 147 to 149 in e8434f1 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
Remove the /_scripts/sign endpoint (rate limiter, origin check, server-side storage) and replace it with stateless page tokens that the server embeds in the SSR payload. Page tokens are HMAC-signed timestamps valid for 1 hour. The client attaches _pt + _ts params to proxy requests instead of needing a signing round-trip. verifyProxyRequest now accepts either a URL signature (exact params, from SSR) or a page token (any params, time-limited, from client). This removes the rate limiter from the module. Rate limiting is the site owner's concern, not the module's.
All 10 server handlers now enforce HMAC signature verification: - google-static-maps-proxy, google-maps-geocode-proxy, gravatar-proxy - x-embed, bluesky-embed, instagram-embed - createImageProxyHandler (covers x-embed-image, bluesky-embed-image, instagram-embed-image, instagram-embed-asset) Registry entries updated with requiresSigning: true so the module enforces NUXT_SCRIPTS_PROXY_SECRET in production when any of these scripts are enabled.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/script/src/runtime/server/instagram-embed.ts (1)
190-335:⚠️ Potential issue | 🔴 CriticalBlocker: returned Instagram asset/image proxy URLs are now unsigned but downstream routes require signatures.
After enabling signed-only proxy routes, this handler still emits plain
/_scripts/embed/instagram-image?...and/_scripts/embed/instagram-asset?...URLs. Browser follow-up requests will fail with 403.
Please sign these generated URLs server-side (or temporarily stop requiring signing on those two routes until signed URL generation is wired).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/script/src/runtime/server/instagram-embed.ts` around lines 190 - 335, The handler emits plain proxy URLs (e.g., `${prefix}/embed/instagram-asset?url=...` and image URLs rewritten via rewriteUrl) which will 403 because downstream proxy routes now require signed URLs; fix by signing those generated URLs server-side before inserting into HTML/CSS: locate where prefix is used to build asset URLs (the RSRC_RE replacement producing `.../embed/instagram-asset?url=...`) and the calls to rewriteUrl/rewriteUrlsInText that produce `.../embed/instagram-image?...`, and replace the plain `${prefix}/embed/...` strings with calls to your signing utility (e.g., signUrl/signPath/getSignedUrl) so the handler returns signed proxy links; ensure you import/use the same signing helper used by withSigning so all generated asset/image URLs include valid signatures.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/script/src/runtime/server/instagram-embed.ts`:
- Around line 190-335: The handler emits plain proxy URLs (e.g.,
`${prefix}/embed/instagram-asset?url=...` and image URLs rewritten via
rewriteUrl) which will 403 because downstream proxy routes now require signed
URLs; fix by signing those generated URLs server-side before inserting into
HTML/CSS: locate where prefix is used to build asset URLs (the RSRC_RE
replacement producing `.../embed/instagram-asset?url=...`) and the calls to
rewriteUrl/rewriteUrlsInText that produce `.../embed/instagram-image?...`, and
replace the plain `${prefix}/embed/...` strings with calls to your signing
utility (e.g., signUrl/signPath/getSignedUrl) so the handler returns signed
proxy links; ensure you import/use the same signing helper used by withSigning
so all generated asset/image URLs include valid signatures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 86ce41b5-011e-4cbd-80c8-82656b32a59e
📒 Files selected for processing (8)
packages/script/src/registry.tspackages/script/src/runtime/server/bluesky-embed.tspackages/script/src/runtime/server/google-maps-geocode-proxy.tspackages/script/src/runtime/server/google-static-maps-proxy.tspackages/script/src/runtime/server/gravatar-proxy.tspackages/script/src/runtime/server/instagram-embed.tspackages/script/src/runtime/server/utils/image-proxy.tspackages/script/src/runtime/server/x-embed.ts
withSigning now delegates to the inner handler without verification when NUXT_SCRIPTS_PROXY_SECRET is not set. This avoids breaking existing users who upgrade before components emit signed URLs. Signing enforcement activates automatically once a secret is set.
Collaborator
Author
Code reviewFound 2 issues:
scripts/packages/script/src/module.ts Lines 486 to 498 in deacf59 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
…rning Two fixes from code review: 1. resolveProxySecret now runs AFTER the handler registration loop, gated on anyHandlerRequiresSigning. Users with only client-side scripts no longer get a surprise NUXT_SCRIPTS_PROXY_SECRET in their .env. 2. Missing secret in production is now a warning, not a fatal throw. This avoids breaking nuxt prepare / nuxt build / CI where the env var isn't set. withSigning already passes through gracefully when no secret is configured, so endpoints remain functional but unsigned. Users opt in to enforcement by setting the env var.
Avoids pulling nitro virtual modules (#imports) into unit tests that import handler files for their exported utilities (e.g. instagram-embed exports rewriteUrl/scopeCss alongside its default handler). The dynamic import only runs at request time inside the nitro runtime where #imports is available.
harlan-zw
force-pushed
the
feat/proxy-url-signing
branch
from
5d20c9b to
eee5ffc
Compare
- resolveProxySecret: config precedence, env var fallback, .env auto-gen, existing key detection, newline handling, read-only FS fallback, process.env population (10 tests) - verifyProxyRequest: URL signature mode, page token mode, expired tokens, tampered sigs, empty secret, any-params page token, dual-mode precedence (8 tests) Total: 54 tests across sign.test.ts and resolve-proxy-secret.test.ts
Documents HMAC URL signing, page tokens, the generate-secret CLI, dev auto-generation, and production setup requirements.
…ubleshooting Lists which endpoints use signing, documents autoGenerateSecret option, and covers common issues: 403 after deploy, cross-replica mismatches, unexpected .env entries, and page token expiry.
…y endpoints Adds a brief callout linking to the security guide on Google Maps, Gravatar, Bluesky, Instagram, and X embed doc pages.
Signing requires a server runtime to verify HMACs. The module now detects ssr: false and static presets (github-pages, etc.) and skips secret resolution entirely, logging a clear warning instead. Also documents the SSG/SPA limitations in the security guide.
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
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.
🔗 Linked issue
Related to #594
❓ Type of change
📚 Description
Proxy endpoints like Google Static Maps and Geocode inject a server-side API key and forward arbitrary requests to third-party services. Without signing, anyone can call these endpoints directly and burn the site owner's API quota. This PR adds the signing infrastructure so individual endpoints can opt in.
What's included:
runtime/server/utils/sign.ts): canonical query serialization (sorted keys, sig-stripped, URL-encoded), HMAC-SHA256 with 16-char hex signatures, constant-time comparisonwithSigningwrapper (runtime/server/utils/withSigning.ts): opt-in middleware that returns 403 on missing/invalid signatures, 500 if secret not configuredmodule.ts):NUXT_SCRIPTS_PROXY_SECRETenv var,scripts.security.secretconfig, dev auto-generates to.env, prod throws if missing when signed endpoints are registered/_scripts/signendpoint (runtime/server/sign-proxy.ts): for reactive client-side URL updates (e.g. Google Static Maps recomputing size on mount); same-origin check + per-IP rate limiting (60/min)src/cli.ts+bin/cli.mjs):npx @nuxt/scripts generate-secretfor explicit secret managementrequiresSigningflag onRegistryScriptServerHandlerso endpoints can declare they need signingNot included (follow-up PRs):
requiresSigning: trueon actual handlers (google-static-maps, geocode, gravatar, embed image proxies)buildSignedProxyUrlat SSR timecreateImageProxyHandler(streaming size limit, DNS + private-IP check)/_scripts/p/**analytics reverse proxy