⚡ Bolt: optimize security utilities, MDX evaluation, and metadata#3
⚡ Bolt: optimize security utilities, MDX evaluation, and metadata#3administrakt0r wants to merge 1 commit into
Conversation
- Hoist regexes and implement single-pass replacements in `src/lib/security.ts`. - Implement global `mdxCache` in `src/components/app/mdx-components.tsx` to avoid redundant MDX evaluations. - Pre-calculate static JSON-LD strings in `src/app/layout.tsx` and `src/app/page.tsx` to minimize rendering overhead. - Update `.jules/bolt.md` with performance learnings.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
There was a problem hiding this comment.
I've got 2 comments for you to consider
Risk: 🟠 High
Risk analysis
This PR introduces performance optimizations but includes two high-severity security concerns. First, the global MDX component cache in mdx-components.tsx caches the result of new Function(code), which poses a critical risk if code can ever come from untrusted input, due to potential cross-user cache poisoning and memory exhaustion. Second, the shared, stateful regex COLOR_FUNCTION_RE in security.ts may lead to incorrect behavior or exceptions under concurrent access or error conditions, despite attempts to reset lastIndex. These issues elevate the merge risk significantly, warranting careful multi-reviewer attention before landing.
Did you know we can integrate this feedback directly into 50+ IDEs? Get setup in just one command
| return mdxCache.get(code)! | ||
| } | ||
|
|
||
| const exports = new Function(code)({ ...runtime }) |
There was a problem hiding this comment.
The new Function(code) call executes arbitrary JavaScript code. Since mdxCache is a module-level singleton, in a server-side or SSR context this cache is shared across all requests/users. If the code string ever originates from user-controlled input (e.g., user-uploaded MDX), a malicious user could:
- Inject and cache a malicious component under a key that another user's request hits.
- Cause unbounded memory growth before the eviction threshold is reached.
Even in a purely client-side context, caching the result of new Function(code) at module scope means the cache persists for the entire lifetime of the module (across navigations in an SPA). If code can be influenced by untrusted content, this is a critical security concern. Ensure code is always sourced exclusively from trusted, build-time-generated MDX bundles and never from user input or dynamic network responses.
Why did I show this?
Category: security
Comment Quality: high
Based on general best practices
Tools used:
get_file_lines,{'file_path': 'src/app/layout.tsx', 'start_line': '1', 'end_line': '200'}get_file_lines,{'file_path': 'src/app/page.tsx', 'start_line': '1', 'end_line': '200'}get_file_lines,{'file_path': 'src/lib/security.ts', 'start_line': '1', 'end_line': '200'}get_file_lines,{'file_path': 'src/components/app/mdx-components.tsx', 'start_line': '1', 'end_line': '100'}
| * ⚡ Bolt: Hoisted regexes for performance optimization. | ||
| */ | ||
| const COLOR_VALIDATION_RE = /^[-a-zA-Z0-9#(),.%\s+*\/]+$/ | ||
| const COLOR_FUNCTION_RE = /([a-zA-Z-]+)\s*\(/g |
There was a problem hiding this comment.
The COLOR_FUNCTION_RE regex is stateful (uses the g flag) and is shared at the module level. While lastIndex is reset to 0 before each use (line 87 in the whole file), this is not thread-safe in environments where the same module instance could be accessed concurrently (e.g., Node.js worker threads or edge runtimes). More critically, if sanitizeColor throws an exception between lastIndex = 0 and the while loop completing, the lastIndex will be left in a non-zero state for the next call, causing incorrect results (skipping matches from the beginning of the string).
Since the regex is only used in a single while loop, the safest fix is to not use the g flag on the shared instance, or to create a new RegExp per call:
| const COLOR_FUNCTION_RE = /([a-zA-Z-]+)\s*\(/g | |
| const COLOR_FUNCTION_RE = /([a-zA-Z-]+)\s*\(/g |
The current code does reset lastIndex before use, but the exception-safety concern remains. Consider wrapping the while loop in a try/finally to reset lastIndex, or avoid the shared stateful regex entirely by constructing it locally inside sanitizeColor.
Why did I show this?
Category: security
Comment Quality: high
Based on general best practices
Tools used:
get_file_lines,{'file_path': 'src/components/app/mdx-components.tsx', 'start_line': '1', 'end_line': '200'}get_file_lines,{'file_path': 'src/app/layout.tsx', 'start_line': '1', 'end_line': '200'}get_file_lines,{'file_path': 'src/app/page.tsx', 'start_line': '1', 'end_line': '200'}get_file_lines,{'file_path': 'src/components/app/mdx-components.tsx', 'start_line': '1', 'end_line': '200'}get_file_lines,{'file_path': 'src/app/layout.tsx', 'start_line': '1', 'end_line': '200'}get_file_lines,{'file_path': 'src/app/page.tsx', 'start_line': '1', 'end_line': '200'}list_changed_files,{'pattern': '*.ts'}
This PR implements several performance optimizations to reduce rendering overhead and improve efficiency in hot paths:
src/lib/security.tsand refactoredsafeJsonLdandescapeCSSStringto use single-pass regex replacements with callback mappings. This reduces the cost of string sanitization used in nearly every page.mdxCacheinsrc/components/app/mdx-components.tsx. Evaluating MDX code usingnew Function()is expensive; caching the results ensures that navigating back to previously viewed documentation pages is much faster.src/app/layout.tsxandsrc/app/page.tsx. This eliminates the need to callsafeJsonLdon every render of the main layout or home page.These changes are low-risk and maintain the existing security and correctness of the application while providing measurable efficiency gains in the React render cycle.
PR created automatically by Jules for task 3723018164091190952 started by @administrakt0r