fix: remove broken createNonceGetter() (v1.0.1)#8
Conversation
BREAKING CHANGE: Remove createNonceGetter() due to AsyncLocalStorage bug
## What Changed
- **Removed:** createNonceGetter() function (broken isomorphic wrapper)
- **Updated:** src/nonce.ts with deprecation notice and correct pattern
- **Updated:** src/index.ts exports (removed createNonceGetter export)
- **Updated:** README with v1.0.1 pattern (direct context access)
- **Added:** Comprehensive migration guide (docs/MIGRATION-1.0-to-1.0.1.md)
- **Added:** Changeset for v1.0.1 release
## Why This Change?
The createIsomorphicFn() wrapper broke AsyncLocalStorage context chain:
- Server getStartContext() failed with "No Start context found"
- Scripts rendered without nonce attributes
- All scripts blocked by CSP
## Fixed by
Using direct context access (official TanStack pattern):
```typescript
export async function getRouter() {
let nonce: string | undefined;
if (typeof window === 'undefined') {
const { getStartContext } = await import('@tanstack/start-storage-context');
nonce = getStartContext().contextAfterGlobalMiddlewares?.nonce;
}
return createRouter({ ssr: { nonce } });
}
```
## Migration
See docs/MIGRATION-1.0-to-1.0.1.md for complete guide.
## Quality Checks
✅ TypeScript compilation - PASSED
✅ Linting (Biome) - PASSED
✅ Tests (28 tests) - PASSED
Reference: TanStack/router#3028
Removed all specific version references (v0.1, v0.2, v1.0.0, v1.0.1) from README. Makes documentation version-agnostic and easier to maintain. Changes: - Replaced version-specific sections with descriptive names - 'v0.2 API' → 'Middleware API' - 'v0.1' → 'Handler Wrapper - Deprecated' - Removed version numbers from features list - Updated migration section titles - Kept migration guide link (still useful for users upgrading) README now focuses on current best practices without version coupling.
Clean CSP Level 3 implementation that removes directives ignored by browsers with 'strict-dynamic', eliminating 13 console warnings. Changes: - src/internal/csp-builder.ts: Clean script-src directives - Remove 'self', 'unsafe-inline', https:, http: (ignored with strict-dynamic) - Keep only: nonce, strict-dynamic, unsafe-eval (dev only) - src/internal/defaults.ts: Matching clean defaults - Same pattern as csp-builder for consistency - Fallback to old pattern only if no nonce provided Why these directives are ignored: - 'self' - Ignored with 'strict-dynamic' (use nonce-based trust) - 'unsafe-inline' - Ignored when nonce is present (CSP Level 2+) - https:/http: - Ignored with 'strict-dynamic' (overly permissive) - URL whitelists - Ignored with 'strict-dynamic' (use nonce chain) Result: - 13 CSP warnings → 0 warnings - Cleaner, standards-compliant CSP - No loss of functionality (directives were already ignored) Reference: CSP Level 3 spec - strict-dynamic behavior
According to CSP spec, 'unsafe-eval' applies to eval()/Function() execution, which is controlled by script-src, not script-src-elem (which controls <script> elements). This fixes the browser warning: "Ignoring 'unsafe-eval' inside script-src-elem" Changes: - csp-builder.ts: Remove 'unsafe-eval' from script-src-elem (keep in script-src) - defaults.ts: Add script-src-elem directive without 'unsafe-eval' Result: 3 CSP warnings → 1 CSP warning (only posthog URL whitelist remains) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The directive copying logic was copying ALL sources from script-src to script-src-elem, including 'unsafe-eval', which caused browser warnings. According to CSP spec: - 'unsafe-eval' controls eval()/Function() execution (script-src) - script-src-elem only controls <script> element loading - 'unsafe-eval' in script-src-elem causes "Ignoring 'unsafe-eval'" warning Fix: Skip 'unsafe-eval' when copying from script-src to script-src-elem Result: 1 CSP warning → 0 CSP warnings 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The documentation showed CSP directives that don't match the actual code. Fixed: - Removed 'self' from script-src examples (ignored with 'strict-dynamic') - Removed mention of https:/http: in dev mode (also ignored) - Added dev mode example showing 'unsafe-eval' only in script-src - Added note that 'unsafe-eval' NOT in script-src-elem (prevents warning) - Updated CSP Level 3 section with unsafe-eval copying exception Result: Documentation now accurately reflects zero-warning CSP implementation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 672a05b The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary of ChangesHello @Enalmada, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a critical bug in the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request addresses a critical bug in createNonceGetter() by removing the function and aligning with the official TanStack Router pattern for nonce retrieval. The changes are thorough, covering not only the code fix but also significant improvements to the CSP generation logic and comprehensive updates to all documentation, including a new migration guide. The fix for CSP warnings by adjusting how 'strict-dynamic' and 'unsafe-eval' are handled is a great improvement for security and correctness. The documentation updates are excellent and will be very helpful for users migrating to the new version. I have one minor suggestion to improve the example code in the new migration guide for better debuggability.
| } catch (error) { | ||
| // Context not available (shouldn't happen, but handle gracefully) | ||
| nonce = undefined; | ||
| } |
There was a problem hiding this comment.
In the complete example, the try...catch block around getStartContext is a good defensive measure. However, silently catching the error and setting nonce to undefined might hide a fundamental setup problem from the developer. If getStartContext fails, it indicates a significant issue with the TanStack Start context. Logging the error would provide valuable debugging information if this unlikely scenario occurs.
| } catch (error) { | |
| // Context not available (shouldn't happen, but handle gracefully) | |
| nonce = undefined; | |
| } | |
| } catch (error) { | |
| // Context not available (shouldn't happen, but handle gracefully) | |
| console.error('[start-secure] Failed to get start context for nonce:', error); | |
| nonce = undefined; | |
| } |
Summary
Removes broken
createNonceGetter()function and updates documentation with official TanStack Router pattern.Changes
Breaking Change
createNonceGetter()function (had critical AsyncLocalStorage bug)createIsomorphicFn()wrapper broke AsyncLocalStorage context chainBug Fixes
'unsafe-eval'from being copied toscript-src-elem'strict-dynamic'Documentation
What Still Works
No changes to these (all work perfectly):
createCspMiddleware()- Middleware nonce generationgenerateNonce()- Crypto-random nonce generationbuildCspHeader()- CSP header buildingMigration Path
Before (v1.0.0 - BROKEN):
After (v1.0.1 - WORKING):
Testing
References