Conversation
…d rate limiting - Add clustering support based on available CPU cores and environment settings - Integrate PostHog analytics for API request and server metrics tracking - Implement rate limiting with IP validation and bounded in-memory storage - Enhance VercelRequest and VercelResponse interfaces with robust parsing and security headers - Improve CORS handling with origin allowlists and credential support - Validate and sanitize API endpoint paths to prevent directory traversal attacks - Add request body size limit and enforce request timeout handling - Provide structured logging for requests, responses, errors, and server lifecycle events - Add health endpoint with uptime, metrics, environment, and version info - Support graceful shutdown with analytics capture on termination signals - Update create-checkout-session API with stricter CORS origin checks and OPTIONS method handling - Refine hono-polar API subscription syncing with date object conversions and improved checkout flow - Enhance secret-chat API error handling with detailed status codes and messages - Update service worker cache revision for production deployment
- Replace `existsSync` with `statSync` for safer file existence checks in API endpoints. - Improve error handling for non-file responses and inaccessible directories. - Update CORS handling in `create-checkout-session` to validate origins more robustly. - Introduce helper functions for validating environment variables in `hono-polar` API. - Refactor input sanitization and validation logic in various components for enhanced security. - Clean up unused imports and optimize component structures across multiple files.
- Update analytics configuration to enable based on the presence of the PostHog API key. - Improve IP hashing method for better privacy using SHA-256. - Refine IP validation logic with comprehensive regex for IPv6 support. - Enhance error responses in API endpoints to include decoded endpoint information. - Implement structured logging for API requests and errors, ensuring sensitive data is scrubbed. - Update README to reflect enhanced security features and request timeout settings. - Add new environment variables for Sentry error monitoring and privacy consent management. - Optimize dependency versions in package.json and bun.lock for improved stability.
- Simplified regex patterns for email and phone number validation. - Improved handling of sensitive headers in the scrubHeaders function. - Updated error sanitization methods to prevent sensitive data leaks. - Enhanced URL validation patterns for GitHub links. - Refined object sanitization to prevent prototype pollution and ensure safe handling of sensitive fields.
|
| GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
|---|---|---|---|---|---|
| 20372498 | Triggered | Generic High Entropy Secret | 72993ac | .env.deployment.template | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secret safely. Learn here the best practices.
- Revoke and rotate this secret.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a unified multi-platform deployment system (Netlify/Vercel) with domain management APIs, a security-aware dev API server, privacy consent + Sentry PII controls, Polar billing integration and success sync, substantial chat UI upgrades (SmartPrompts, LivePreview), broader sanitization and type-safety hardening, improved GitHub token/storage flows, and extensive docs/templates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant API as /api/deploy
participant Manager as ZapdevDeploymentManager
participant Service as Platform Service (Netlify/Vercel)
rect rgba(200,235,255,0.25)
note over Client,API: Deploy or Domain Action
Client->>API: POST { action, platform, ... }
API->>Manager: validate + route(action)
alt action == deploy
Manager->>Service: deploy(config)
Service-->>Manager: DeploymentResult
else action == status/setup/verify/delete/list
Manager->>Service: corresponding method(...)
Service-->>Manager: Result
end
Manager-->>API: Result
API-->>Client: JSON { success, ... }
end
sequenceDiagram
autonumber
participant App
participant Banner as PrivacyConsentBanner
participant Store as privacyConsent
participant Sentry as Sentry SDK
App->>Banner: Render if consent required
Banner->>Store: setConsent(preferences)
Store-->>Banner: persisted ok
Banner-->>App: onConsentChange(flag)
App->>Sentry: Init with sendDefaultPii=shouldSendPII()
Sentry->>Sentry: Scrub events/breadcrumbs per consent
sequenceDiagram
autonumber
participant FE as Frontend (/pricing,/success)
participant Success as /api/success
participant Auth as verifyAuth
participant Sub as /api/get-subscription
FE->>Success: POST { userId } (+Authorization?)
alt Auth header present
Success->>Auth: verifyAuth(headers)
Auth-->>Success: user or null
Success-->>FE: 401/403 on failure
end
Success->>Sub: fetch (fwd Authorization if present)
alt Sub ok
Sub-->>Success: { planId, status }
Success-->>FE: 200 { planId,status }
else error
Success-->>FE: 200 { planId:'free',status:'none' }
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
|
|
||
| private extractGitProvider(url: string): string { | ||
| if (url.includes('github.com')) return 'github'; |
Check failure
Code scanning / CodeQL
Incomplete URL substring sanitization High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
The best way to fix this is to properly parse the input URL and only consider the host portion when matching the domain. This avoids false positives arising from substring searches in the path, query, or other components of URLs. You should use Node's standard URL class (available in the global runtime since Node.js 10+ and also in browsers) to parse the URL and extract the host property. For extra correctness, this method should handle any possible parsing errors gracefully. The test for supported git providers should check for an exact host match or allow for subdomains if that's desired (e.g., github.com or www.github.com).
You should update the extractGitProvider method in lib/deployment/netlify.ts to parse the URL and match only using the host portion, replacing the substring checks.
| @@ -618,9 +618,27 @@ | ||
| } | ||
|
|
||
| private extractGitProvider(url: string): string { | ||
| if (url.includes('github.com')) return 'github'; | ||
| if (url.includes('gitlab.com')) return 'gitlab'; | ||
| if (url.includes('bitbucket.org')) return 'bitbucket'; | ||
| let host = ''; | ||
| try { | ||
| // Use built-in URL class to parse; fallback for SSH/short syntax | ||
| if (/^(https?:\/\/|git@)/i.test(url)) { | ||
| host = new URL( | ||
| url.startsWith('git@') ? url.replace(':', '/').replace(/^git@/, 'https://') : url | ||
| ).host; | ||
| } else { | ||
| // Fallback for short syntax or incomplete URLs | ||
| host = url.split('/')[0]; | ||
| } | ||
| } catch (e) { | ||
| // Fall back to substring matching if parsing fails (very rare) | ||
| if (url.includes('github.com')) return 'github'; | ||
| if (url.includes('gitlab.com')) return 'gitlab'; | ||
| if (url.includes('bitbucket.org')) return 'bitbucket'; | ||
| return 'github'; | ||
| } | ||
| if (host === 'github.com' || host.endsWith('.github.com')) return 'github'; | ||
| if (host === 'gitlab.com' || host.endsWith('.gitlab.com')) return 'gitlab'; | ||
| if (host === 'bitbucket.org' || host.endsWith('.bitbucket.org')) return 'bitbucket'; | ||
| return 'github'; // default | ||
| } | ||
|
|
|
|
||
| private extractGitProvider(url: string): string { | ||
| if (url.includes('github.com')) return 'github'; | ||
| if (url.includes('gitlab.com')) return 'gitlab'; |
Check failure
Code scanning / CodeQL
Incomplete URL substring sanitization High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
The best way to fix this problem is to properly parse the URL and inspect its hostname (host) property, rather than doing a substring search. We should use the standard WHATWG URL constructor (built-in in Node.js and all browsers) to extract the hostname, then compare it against an explicit list of permissible hosts (github.com, gitlab.com, bitbucket.org). For extra safety and usability, we could also support "www." prefixed versions, but for standard repository URLs, these three hosts are the relevant ones.
Specifically, in lib/deployment/netlify.ts, in the extractGitProvider method (lines 620-625), replace the substring checks with proper extraction and matching of the hostname. We need to handle malformed URLs gracefully (e.g., if url is not a valid URL), possibly by falling back to the default provider or attempting to extract a host with regex only if the URL constructor fails. Also, no extra dependencies are needed; the global URL class can be used directly.
| @@ -618,9 +618,21 @@ | ||
| } | ||
|
|
||
| private extractGitProvider(url: string): string { | ||
| if (url.includes('github.com')) return 'github'; | ||
| if (url.includes('gitlab.com')) return 'gitlab'; | ||
| if (url.includes('bitbucket.org')) return 'bitbucket'; | ||
| try { | ||
| const parsed = new URL(url); | ||
| const host = parsed.hostname.toLowerCase(); | ||
| if (host === 'github.com') return 'github'; | ||
| if (host === 'gitlab.com') return 'gitlab'; | ||
| if (host === 'bitbucket.org') return 'bitbucket'; | ||
| } catch (_) { | ||
| // Malformed URLs may reach here (e.g. git@github.com:user/repo.git format) | ||
| // Attempt to extract host using regex as fallback | ||
| const match = url.match(/(?:@|\/\/)([^/:]+)/); | ||
| const host = match?.[1]?.toLowerCase(); | ||
| if (host === 'github.com') return 'github'; | ||
| if (host === 'gitlab.com') return 'gitlab'; | ||
| if (host === 'bitbucket.org') return 'bitbucket'; | ||
| } | ||
| return 'github'; // default | ||
| } | ||
|
|
| private extractGitProvider(url: string): string { | ||
| if (url.includes('github.com')) return 'github'; | ||
| if (url.includes('gitlab.com')) return 'gitlab'; | ||
| if (url.includes('bitbucket.org')) return 'bitbucket'; |
Check failure
Code scanning / CodeQL
Incomplete URL substring sanitization High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To fix the problem, URLs should be parsed and the provider check should be performed on the hostname component rather than a substring search. Specifically, use the built-in URL class from Node.js or the browser standard to safely extract the hostname, then check for an exact match with the correct host. For example, to detect GitHub, confirm that hostname is github.com (or add additional handling for accepted subdomains, if needed). The fix should be implemented within the extractGitProvider method only, without altering the function's contract or externally observable behavior. If the URL cannot be parsed, fall back to the current default provider.
No other changes are necessary in the given code.
| @@ -618,9 +618,15 @@ | ||
| } | ||
|
|
||
| private extractGitProvider(url: string): string { | ||
| if (url.includes('github.com')) return 'github'; | ||
| if (url.includes('gitlab.com')) return 'gitlab'; | ||
| if (url.includes('bitbucket.org')) return 'bitbucket'; | ||
| try { | ||
| const parsed = new URL(url); | ||
| const host = parsed.hostname.toLowerCase(); | ||
| if (host === 'github.com') return 'github'; | ||
| if (host === 'gitlab.com') return 'gitlab'; | ||
| if (host === 'bitbucket.org') return 'bitbucket'; | ||
| } catch { | ||
| // Ignore parse errors, fallback to default | ||
| } | ||
| return 'github'; // default | ||
| } | ||
|
|
| success: false, | ||
| error: error instanceof Error ? error.message : String(error) | ||
| }; | ||
| } |
Check failure
Code scanning / CodeQL
Incomplete URL substring sanitization High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To fix the problem, we must replace the substring match on the entire URL (url.includes('github.com')) with a proper parse that examines the host portion of the URL. We can do this by using the built-in URL class (available in Node.js and modern browsers) to parse the URL, then compare the hostname to our known git provider hosts. Where possible, we should handle a full match as well as any supported subdomains, but for git service URLs it is safe to expect the host to exactly match (or at least end with) github.com, gitlab.com, or bitbucket.org. This change affects only the extractGitProvider method: we need to parse the URL, catch malformed ones, get the hostname, and then compare hostnames rather than using the unsafe substring match. We will also need to add imports for the URL class if it is not already available (but in a TypeScript/Node.js context it should be).
| @@ -568,9 +568,21 @@ | ||
| } | ||
|
|
||
| private extractGitProvider(url: string): string { | ||
| if (url.includes('github.com')) return 'github'; | ||
| if (url.includes('gitlab.com')) return 'gitlab'; | ||
| if (url.includes('bitbucket.org')) return 'bitbucket'; | ||
| try { | ||
| const u = new URL(url); | ||
| const host = u.hostname.replace(/^\./, ''); // remove leading dot if any | ||
| if (host === 'github.com') return 'github'; | ||
| if (host === 'gitlab.com') return 'gitlab'; | ||
| if (host === 'bitbucket.org') return 'bitbucket'; | ||
| } catch (_) { | ||
| // fallback for non-URL git-addresses, e.g. git@github.com:owner/repo.git | ||
| // Try to match in host segment after '@' | ||
| const match = url.match(/@([^:\/]+)/); | ||
| const host = match?.[1] ?? ''; | ||
| if (host === 'github.com') return 'github'; | ||
| if (host === 'gitlab.com') return 'gitlab'; | ||
| if (host === 'bitbucket.org') return 'bitbucket'; | ||
| } | ||
| return 'github'; // default | ||
| } | ||
|
|
| error: error instanceof Error ? error.message : String(error) | ||
| }; | ||
| } | ||
| } |
Check failure
Code scanning / CodeQL
Incomplete URL substring sanitization High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To correctly identify the provider from a git URL, we should parse the URL and examine its host part rather than performing substring checks. In TypeScript/JavaScript, the standard URL class can be used to reliably extract the host from a URL string. This can be used to match against exact hostnames like github.com, gitlab.com, or bitbucket.org. We'll modify extractGitProvider() so that it parses the URL, collects its host, and compares it strictly (or with some allowance for subdomains if needed, but only in a controlled way). For malformed URLs, fallback logic may be used.
Required changes:
- Import or use the
URLglobal class. - Update
extractGitProvider()to parse the URL and compare its host against known hostnames. - Only edit the code within
extractGitProvider().
| @@ -568,9 +568,19 @@ | ||
| } | ||
|
|
||
| private extractGitProvider(url: string): string { | ||
| if (url.includes('github.com')) return 'github'; | ||
| if (url.includes('gitlab.com')) return 'gitlab'; | ||
| if (url.includes('bitbucket.org')) return 'bitbucket'; | ||
| try { | ||
| const parsed = new URL(url.trim()); | ||
| const host = parsed.host.toLowerCase(); | ||
| if (host === 'github.com') return 'github'; | ||
| if (host === 'gitlab.com') return 'gitlab'; | ||
| if (host === 'bitbucket.org') return 'bitbucket'; | ||
| } catch { | ||
| // If parsing fails, fallback to substring (legacy support) | ||
| const maybeUrl = url.toLowerCase(); | ||
| if (maybeUrl.includes('github.com')) return 'github'; | ||
| if (maybeUrl.includes('gitlab.com')) return 'gitlab'; | ||
| if (maybeUrl.includes('bitbucket.org')) return 'bitbucket'; | ||
| } | ||
| return 'github'; // default | ||
| } | ||
|
|
| }; | ||
| } | ||
| } | ||
|
|
Check failure
Code scanning / CodeQL
Incomplete URL substring sanitization High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
The best way to fix this issue is to properly parse the input URL and perform host checks only on the hostname portion, rather than the raw URL string. The ideal approach is to use the built-in URL class from Node.js (new URL(url)), extract the hostname property, then match it against the known provider domains: github.com, gitlab.com, and bitbucket.org. Optionally, allow for subdomains by either exact match or suffix match using .endsWith().
To implement this:
- In the
extractGitProvidermethod:- Parse the URL string safely (with try/catch).
- Check the provider against the parsed host.
- Return the correct provider string.
- Update logic for all providers (GitHub, GitLab, Bitbucket) to use hostname comparison.
- If the URL is invalid or the hostname cannot be determined, default to
'github'as before.
Only code in extractGitProvider on lines 570-575 should be changed; no additional imports are necessary since the built-in URL is available in Node.js, and no other context is being modified.
| @@ -568,9 +568,17 @@ | ||
| } | ||
|
|
||
| private extractGitProvider(url: string): string { | ||
| if (url.includes('github.com')) return 'github'; | ||
| if (url.includes('gitlab.com')) return 'gitlab'; | ||
| if (url.includes('bitbucket.org')) return 'bitbucket'; | ||
| try { | ||
| const { hostname } = new URL(url); | ||
| if (hostname === 'github.com' || hostname.endsWith('.github.com')) return 'github'; | ||
| if (hostname === 'gitlab.com' || hostname.endsWith('.gitlab.com')) return 'gitlab'; | ||
| if (hostname === 'bitbucket.org' || hostname.endsWith('.bitbucket.org')) return 'bitbucket'; | ||
| } catch (_) { | ||
| // If input is not a valid URL, fallback to substring match for robustness | ||
| if (url.includes('github.com')) return 'github'; | ||
| if (url.includes('gitlab.com')) return 'gitlab'; | ||
| if (url.includes('bitbucket.org')) return 'bitbucket'; | ||
| } | ||
| return 'github'; // default | ||
| } | ||
|
|
| return htmlString | ||
| .replace(/<script\b[^<]*(?:(?!<\/script>)<[^<]*)*<\/script>/gi, '') // Remove script tags with content | ||
| .replace(/<style\b[^<]*(?:(?!<\/style>)<[^<]*)*<\/style>/gi, '') // Remove style tags with content | ||
| .replace(/<script[^>]*>[\s\S]*?<\/script>/gi, '') // Remove script tags with content | ||
| .replace(/<style[^>]*>[\s\S]*?<\/style>/gi, '') // Remove style tags with content | ||
| .replace(/<[^>]+>/g, '') // Remove all remaining HTML tags |
Check failure
Code scanning / CodeQL
Incomplete multi-character sanitization High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
The best way to fix the incomplete multi-character sanitization is to repeatedly apply the key regular expressions for <script> and <style> tag removal until no further replacements occur. This ensures that all instances, including oddly nested or fragmented tags created by initial replacements, are fully removed. The replacement should happen in a loop for both patterns before the remainder of the function logic executes.
How to fix:
- Replace lines 10–13 in the function to use a loop for
<script>and<style>tag removal. - Initialize a variable to hold the iterative result.
- Do-while loop to keep applying regex replacements for
<script>and<style>tags until the string no longer changes. - Follow with the existing tag and entity removal code.
- No additional imports are needed—use only standard JavaScript techniques.
This fix is localized to the sanitizeHtmlText function in src/lib/firecrawl.ts.
| @@ -5,11 +5,19 @@ | ||
| // Secure HTML text sanitization function | ||
| function sanitizeHtmlText(htmlString: string): string { | ||
| if (!htmlString) return '' | ||
|
|
||
| // More robust HTML tag removal that handles edge cases | ||
| return htmlString | ||
| .replace(/<script[^>]*>[\s\S]*?<\/script>/gi, '') // Remove script tags with content | ||
| .replace(/<style[^>]*>[\s\S]*?<\/style>/gi, '') // Remove style tags with content | ||
|
|
||
| // Robust HTML tag removal: repeat script/style tag removal for complete sanitization | ||
| let sanitized = htmlString; | ||
| let prevSanitized; | ||
| do { | ||
| prevSanitized = sanitized; | ||
| sanitized = sanitized | ||
| .replace(/<script[^>]*>[\s\S]*?<\/script>/gi, '') | ||
| .replace(/<style[^>]*>[\s\S]*?<\/style>/gi, ''); | ||
| } while (sanitized !== prevSanitized); | ||
|
|
||
| // Remove all remaining HTML tags, decode common entities, trim & limit length | ||
| return sanitized | ||
| .replace(/<[^>]+>/g, '') // Remove all remaining HTML tags | ||
| .replace(/&[#\w]+;/g, (entity) => { // Decode common HTML entities | ||
| const entities: { [key: string]: string } = { |
| .replace(/<script\b[^<]*(?:(?!<\/script>)<[^<]*)*<\/script>/gi, '') // Remove script tags with content | ||
| .replace(/<style\b[^<]*(?:(?!<\/style>)<[^<]*)*<\/style>/gi, '') // Remove style tags with content | ||
| .replace(/<script[^>]*>[\s\S]*?<\/script>/gi, '') // Remove script tags with content | ||
| .replace(/<style[^>]*>[\s\S]*?<\/style>/gi, '') // Remove style tags with content |
Check failure
Code scanning / CodeQL
Bad HTML filtering regexp High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To fix this issue, remove the custom regular-expression-based sanitization logic and instead use a well-tested HTML sanitization library. In Node.js/TypeScript, one option is the sanitize-html package. This library thoroughly parses HTML and strips dangerous tags and attributes, reliably addressing tricky cases such as malformed tags or XSS vectors the regex approach misses.
Steps:
- Add an import for
sanitize-html. - Rewrite the
sanitizeHtmlTextfunction so that it processeshtmlStringusingsanitizeHtml(configured to strip tags and decode entities appropriately), then trims and cuts to the desired length. - Remove all regex-based tag removal in this function.
Edits to do:
- In
src/lib/firecrawl.ts, add an import forsanitize-html. - Replace the body of
sanitizeHtmlTextwith a call tosanitizeHtml, appropriate configuration, and post-processing as before.
| @@ -1,3 +1,4 @@ | ||
| import sanitizeHtml from 'sanitize-html' | ||
| import * as Sentry from '@sentry/react' | ||
|
|
||
| const { logger } = Sentry | ||
| @@ -5,26 +6,13 @@ | ||
| // Secure HTML text sanitization function | ||
| function sanitizeHtmlText(htmlString: string): string { | ||
| if (!htmlString) return '' | ||
|
|
||
| // More robust HTML tag removal that handles edge cases | ||
| return htmlString | ||
| .replace(/<script[^>]*>[\s\S]*?<\/script>/gi, '') // Remove script tags with content | ||
| .replace(/<style[^>]*>[\s\S]*?<\/style>/gi, '') // Remove style tags with content | ||
| .replace(/<[^>]+>/g, '') // Remove all remaining HTML tags | ||
| .replace(/&[#\w]+;/g, (entity) => { // Decode common HTML entities | ||
| const entities: { [key: string]: string } = { | ||
| '&': '&', | ||
| '<': '<', | ||
| '>': '>', | ||
| '"': '"', | ||
| ''': "'", | ||
| ' ': ' ' | ||
| } | ||
| const safeEntities = entities; | ||
| return Object.prototype.hasOwnProperty.call(safeEntities, entity) ? safeEntities[entity as keyof typeof safeEntities] : entity; | ||
| }) | ||
| .trim() | ||
| .substring(0, 500) // Limit length for safety | ||
| // Use sanitize-html to remove all tags and decode entities. | ||
| const sanitized = sanitizeHtml(htmlString, { | ||
| allowedTags: [], // remove all tags | ||
| allowedAttributes: {}, | ||
| textFilter: function(text) { return text }, // preserve decoded text | ||
| }) | ||
| return sanitized.trim().substring(0, 500) // Limit length for safety | ||
| } | ||
|
|
||
| interface NavigationItem { |
| @@ -130,7 +130,8 @@ | ||
| "undici": "^7.13.0", | ||
| "vaul": "^1.1.2", | ||
| "vercel": "^44.7.3", | ||
| "zod": "^4.0.17" | ||
| "zod": "^4.0.17", | ||
| "sanitize-html": "^2.17.0" | ||
| }, | ||
| "devDependencies": { | ||
| "@eslint/js": "^9.33.0", |
| Package | Version | Security advisories |
| sanitize-html (npm) | 2.17.0 | None |
…for credentials transfer Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 45
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (41)
src/components/UserSync.tsx (2)
26-26: Avoid leaking PII in fallback username generationDeriving the username from the email local-part exposes PII in a public-facing handle. Replace with a non-email-based fallback.
Apply this diff:
- username: clerkUser.username || `${email.split('@')[0]}_${Date.now()}`, + // Avoid PII in public handles + username: clerkUser.username || `user_${Date.now().toString(36)}`,Follow-up: see my broader refactor on Lines 13-36 for sanitization and stricter username rules.
13-36: Sanitize inputs, prevent duplicate upserts, and provide consistent user feedback
- Sanitizes
fullName/usernameand normalizes email before sending to Convex.- Prevents redundant writes by diffing a stable payload snapshot.
- Uses Sonner to provide loading/success/error feedback per guidelines.
Apply this diff:
useEffect(() => { - if (isAuthenticated && !isLoading && clerkUser) { - // Validate required user data - const email = clerkUser.primaryEmailAddress?.emailAddress; - if (!email) { - console.error('User missing primary email address'); - return; - } - - upsertUser({ - email, - fullName: clerkUser.fullName || undefined, - avatarUrl: clerkUser.imageUrl || undefined, - username: clerkUser.username || `${email.split('@')[0]}_${Date.now()}`, - bio: '', - }).catch((error) => { - console.error('Failed to sync user to Convex:', error); - // In production, you might want to show a user-friendly error - if (import.meta.env.PROD) { - console.error('Authentication sync failed. Please refresh the page or contact support.'); - } - }); - } - }, [isAuthenticated, isLoading, clerkUser?.id, clerkUser?.primaryEmailAddress?.emailAddress, clerkUser?.imageUrl, clerkUser?.fullName, clerkUser?.username, upsertUser, clerkUser]); + // Build a stable, sanitized payload + const payload = (() => { + if (!clerkUser) return null; + const emailRaw = clerkUser.primaryEmailAddress?.emailAddress; + if (!emailRaw) return null; + const email = emailRaw.trim().toLowerCase(); + const fullName = clerkUser.fullName + ? sanitizeText(clerkUser.fullName, MAX_NAME_LENGTH) + : undefined; + const fallbackUsername = `user_${Date.now().toString(36)}`; + const usernameRaw = clerkUser.username ?? fallbackUsername; + const username = sanitizeText( + usernameRaw.replace(/[^a-z0-9_]/gi, '_'), + MAX_USERNAME_LENGTH + ); + const avatarUrl = clerkUser.imageUrl || undefined; + return { email, fullName, avatarUrl, username }; + })(); + + // Skip until auth and payload are ready + if (!isAuthenticated || isLoading || !payload) return; + + // Prevent duplicate upserts if nothing relevant changed + const json = JSON.stringify(payload); + if (lastPayloadJsonRef.current === json) return; + lastPayloadJsonRef.current = json; + + // Execute with consistent user feedback + (async () => { + try { + await toast.promise( + upsertUser(payload), + { + loading: 'Syncing your account…', + success: 'Account synced.', + error: 'Failed to sync your account. Please try again.', + } + ); + } catch (err) { + // Log minimal details in dev; avoid leaking PII + if (typeof process !== 'undefined' && process.env.NODE_ENV !== 'production') { + console.error('[UserSync] upsertUser failed:', (err as Error)?.message ?? err); + } + } + })(); + // Depend on stable signals only + }, [isAuthenticated, isLoading, clerkUser, upsertUser]);Add these above the effect (once) to support the change:
+// Track last sent payload to avoid repeated writes +const lastPayloadJsonRef = useRef<string | null>(null);src/lib/github-token-storage.ts (5)
7-10: Sentry logger usage will throw at runtime; replace with a safe logging wrapper.
@sentry/reactdoes not export aloggerobject.const { logger } = Sentry;setsloggertoundefined, and everylogger.info/warn/errorcall will crash. Replace with a small wrapper that uses Sentry APIs (and console in dev) and never logs token material.Apply:
-import * as Sentry from '@sentry/react'; - -const { logger } = Sentry; +import * as Sentry from '@sentry/react'; + +const log = { + info(message: string, context?: Record<string, unknown>) { + Sentry.addBreadcrumb({ category: 'github-token', level: 'info', message, data: context }); + if (process.env.NODE_ENV !== 'production') console.info(message, context); + }, + warn(message: string, context?: Record<string, unknown>) { + Sentry.addBreadcrumb({ category: 'github-token', level: 'warning', message, data: context }); + if (process.env.NODE_ENV !== 'production') console.warn(message, context); + }, + error(message: string, err?: unknown, context?: Record<string, unknown>) { + Sentry.withScope(scope => { + if (context) scope.setContext('github-token', context); + Sentry.captureException(err instanceof Error ? err : new Error(message)); + }); + if (process.env.NODE_ENV !== 'production') console.error(message, err, context); + }, +};
22-33: Tighten token validation: avoid brittle length checks, remove XSS heuristics, and forbid logging token substrings.
- Length equality for classic tokens and “>=82” for fine‑grained are brittle.
- XSS string checks are irrelevant here; enforce allowed charset instead.
- Don’t reassign params; avoid side effects.
-export function validateGitHubToken(token: string): boolean { - if (!token || typeof token !== 'string') { - return false; - } - - // Remove whitespace - token = token.trim(); - - // Check for proper GitHub token formats - const isClassicToken = token.startsWith('ghp_') && token.length === 40; - const isFineGrainedToken = token.startsWith('github_pat_') && token.length >= 82; - - // Additional security checks - if (token.includes('<script>') || token.includes('javascript:') || token.includes('data:')) { - logger.warn('Potentially malicious token detected', { tokenPrefix: token.substring(0, 10) }); - return false; - } - - return isClassicToken || isFineGrainedToken; -} +const GH_CLASSIC_RE = /^ghp_[A-Za-z0-9]{36}$/; +const GH_FINE_RE = /^github_pat_[A-Za-z0-9_]{82,}$/; // fine-grained PATs are long and may include underscores +const MAX_TOKEN_LENGTH = 256; // hard cap to avoid pathological inputs + +export function validateGitHubToken(token: string): boolean { + if (!token || typeof token !== 'string') return false; + const t = token.trim(); + if (t.length === 0 || t.length > MAX_TOKEN_LENGTH) return false; + // forbid whitespace or non-expected characters globally + if (/\s/.test(t)) return false; + // validate supported PAT formats + const isClassicToken = GH_CLASSIC_RE.test(t); + const isFineGrainedToken = GH_FINE_RE.test(t); + // If neither, reject without logging any token material + if (!isClassicToken && !isFineGrainedToken) { + log.warn('Rejected GitHub token input (unsupported format)', { length: t.length }); + return false; + } + return true; +}
47-51: Update logging calls to use the safe wrapper and avoid leaking secrets.Replace
logger.info/errorwith the wrapper and ensure no token data is logged.- await setSecureApiKey(`github_${token.trim()}`); - logger.info('GitHub token stored securely'); + await setSecureApiKey(`github_${token.trim()}`); + log.info('GitHub token stored securely'); } catch (error) { - logger.error('Failed to store GitHub token:', error); + log.error('Failed to store GitHub token', error); throw new Error('Failed to securely store GitHub token'); }
76-80: Replace logger error with safe wrapper and keep error details out of thrown messages.Current code uses
logger.errorwhich is broken and would leak error objects if it weren’t. Switch to the wrapper.- } catch (error) { - logger.error('Failed to retrieve GitHub token:', error); + } catch (error) { + log.error('Failed to retrieve GitHub token', error); return null; }
86-93: Replace logger calls and ensure idempotent clearing does not throw.Use wrapper; no token details in logs.
try { await clearSecureApiKey(); - logger.info('GitHub token cleared from storage'); + log.info('GitHub token cleared from storage'); } catch (error) { - logger.error('Failed to clear GitHub token:', error); + log.error('Failed to clear GitHub token', error); }src/lib/webcontainer-templates.ts (1)
1396-1401: Fix mismatched braces in template factory functions and optionally refinetemplatestypingThere are parse errors flagged by Biome in the three
create…Templatefunctions—these must be corrected first before any changes to thetemplatesexport will parse cleanly.• In
src/lib/webcontainer-templates.tsaround lines 211–213, updatecreateLandingPageTemplateso the arrow function returns the object literal with exactly one}followed by). For example:-export const createLandingPageTemplate = (): WebContainerTemplate => ({ - /* … */ - } -}); +export const createLandingPageTemplate = (): WebContainerTemplate => ({ + /* … */ +});• Around lines 505–507, apply the same fix to
createNextJsTemplate:-export const createNextJsTemplate = (): WebContainerTemplate => ({ - /* … */ - } -}); +export const createNextJsTemplate = (): WebContainerTemplate => ({ + /* … */ +});• Around lines 1392–1394, correct
createPortfolioTemplatesimilarly:-export const createPortfolioTemplate = (): WebContainerTemplate => ({ - /* … */ - } -}); +export const createPortfolioTemplate = (): WebContainerTemplate => ({ + /* … */ +});Once those syntax errors are resolved, you can also strengthen the
templatesexport to preserve literal key types:-export const templates: Record<string, () => WebContainerTemplate> = { +export const templates = { nextjs: createNextJsTemplate, basic: createBasicExpressTemplate, landing: createLandingPageTemplate, portfolio: createPortfolioTemplate, -}; +} as const satisfies Record<string, () => WebContainerTemplate>;This ensures
export type TemplateKey = keyof typeof templates;yields'nextjs' | 'basic' | 'landing' | 'portfolio'instead ofstring(requires TypeScript 4.9+).After applying these fixes, please re-run Biome (
npx biome check) and TypeScript (tsc --noEmit) to confirm there are no remaining parse or type errors.src/components/E2BCodeExecution.tsx (6)
433-436: Sanitize artifact download URLs to prevent javascript: and data: URL XSSArtifact URLs are rendered directly into an anchor href. An attacker could inject a javascript: URL here. Validate and whitelist protocols before rendering, and add rel attributes.
Apply this diff to harden the link, and add a small helper:
@@ - <Button + <Button size="sm" variant="ghost" asChild > - <a href={artifact.url} download={artifact.name}> - <Download className="w-3 h-3" /> - </a> + <a + href={toSafeHref(artifact.url) ?? '#'} + download={artifact.name} + rel="noopener noreferrer nofollow" + onClick={(e) => { + if (!toSafeHref(artifact.url)) e.preventDefault(); + }} + > + <Download className="w-3 h-3" /> + </a> </Button>Add this helper near the top-level of the file (outside the component):
// Only allow http(s) or site-internal relative URLs function toSafeHref(url: string): string | null { if (!url) return null; const trimmed = url.trim(); if (trimmed.startsWith('/')) return trimmed; if (/^https?:\/\//i.test(trimmed)) return trimmed; return null; }
303-304: Use SafeText for all user-generated content per security guidelinesCode, output, logs, error text, and artifact names are user-controlled. React escapes by default, but our guidelines mandate SafeText usage for consistency and future-proofing.
Apply these diffs and import SafeText:
@@ -import WebContainerFailsafe from './WebContainerFailsafe'; +import WebContainerFailsafe from './WebContainerFailsafe'; +import { SafeText } from '@/components/SafeText'; @@ - <pre className="text-gray-300 whitespace-pre-wrap">{code}</pre> + <pre className="text-gray-300 whitespace-pre-wrap"><SafeText text={code} /></pre> @@ - {result.output || 'No output'} + <SafeText text={result.output ?? 'No output'} /> @@ - {result.logs.map((log, index) => ( - <div key={index} className="text-sm font-mono text-gray-600 dark:text-gray-400"> - {log} - </div> - ))} + {result.logs.map((log, index) => ( + <div key={index} className="text-sm font-mono text-gray-600 dark:text-gray-400"> + <SafeText text={log} /> + </div> + ))} @@ - {result.error} + <SafeText text={result.error} /> @@ - <span className="text-sm font-medium">{artifact.name}</span> + <span className="text-sm font-medium"><SafeText text={artifact.name} /></span>Also applies to: 389-391, 400-403, 446-448, 426-427
120-125: Clipboard: add error handling and fallback for non-secure contextsnavigator.clipboard may fail (HTTP, denied permission, unsupported). Wrap with try/catch and provide a fallback plus user feedback.
- const handleCopy = () => { - navigator.clipboard.writeText(code); - setCopied(true); - toast.success('Code copied to clipboard!'); - setTimeout(() => setCopied(false), 2000); - }; + const handleCopy = async (): Promise<void> => { + try { + await navigator.clipboard.writeText(code); + setCopied(true); + toast.success('Code copied to clipboard!'); + setTimeout(() => setCopied(false), 2000); + } catch (err) { + // Fallback for older browsers or blocked permissions + try { + const el = document.createElement('textarea'); + el.value = code; + el.setAttribute('readonly', ''); + el.style.position = 'absolute'; + el.style.left = '-9999px'; + document.body.appendChild(el); + el.select(); + document.execCommand('copy'); + document.body.removeChild(el); + setCopied(true); + toast.success('Code copied to clipboard!'); + setTimeout(() => setCopied(false), 2000); + } catch { + toast.error('Unable to copy code. Please copy manually.'); + } + } + };
60-71: Fix ESLint hook deps; stabilize handlers with useCallback; remove rule disableYou’re disabling react-hooks/exhaustive-deps; wrap handlers in useCallback and depend on them in useEffect. This improves correctness (autoRun re-triggers when code/language change) and removes the linter suppression.
- import React, { useState, useEffect } from 'react'; + import React, { useState, useEffect, useCallback } from 'react'; @@ - useEffect(() => { - if (autoRun) { - // fire and forget, errors are handled inside - // slight delay to ensure mount animation completes - const t = setTimeout(() => { - handleExecute(); - }, 200); - return () => clearTimeout(t); - } - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [autoRun]); + useEffect(() => { + if (!autoRun) return; + const t = setTimeout(() => { void handleExecute(); }, 200); + return () => clearTimeout(t); + }, [autoRun, handleExecute]); @@ - const handleExecute = async () => { + const handleExecute = useCallback(async (): Promise<void> => { setIsExecuting(true); setE2bFailed(false); setShowFailsafe(false); @@ - } finally { + } finally { setIsExecuting(false); } - }; + }, [code, language, onExecute]); @@ - const activateFailsafe = () => { + const activateFailsafe = useCallback((): void => { setShowFailsafe(true); toast.info('Switching to WebContainer failsafe...'); - }; + }, []); @@ - const handleCopy = () => { - navigator.clipboard.writeText(code); - setCopied(true); - toast.success('Code copied to clipboard!'); - setTimeout(() => setCopied(false), 2000); - }; + const handleCopy = useCallback(handleCopy, [code]);Note: the last hunk assumes you’ve applied the clipboard refactor above; if not, wrap the final implementation of handleCopy in useCallback with [code].
Also applies to: 72-113, 115-119, 120-125, 1-1
86-113: Avoid leaking internal error details; route specifics to logs/Sentry; default to Error tab on failureRaw error strings are shown in the UI. Per guidelines, avoid information leakage. Show a generic message to users, optionally include a non-sensitive correlation ID, and set the active tab to "error" on failures.
@@ - } catch (error) { - console.error('E2B execution error:', error); - const errorMessage = String(error); + } catch (error) { + console.error('E2B execution error:', error); + const errorMessage = String(error); @@ - if (isE2bError) { + if (isE2bError) { toast.error('E2B service unavailable. Activating WebContainer failsafe...'); setE2bFailed(true); setShowFailsafe(true); } else { - toast.error('Code execution failed'); + toast.error('Code execution failed. Please try again or use the failsafe.'); } @@ - setResult({ - success: false, - error: errorMessage, - logs: [] - }); + // Do not surface raw error details to end-users + setResult({ + success: false, + error: 'Execution failed. See logs or try the failsafe.', + logs: [] + }); + setActiveTab('error');Additionally, when onExecute returns success: false (Line 80-85), also setActiveTab('error') for consistency.
Also applies to: 347-355
72-79: Enforce input size limits before executionImplement MAX_CODE_BLOCK_LENGTH to guard against abuse and excessive memory usage, with user feedback via Sonner. Our guidelines require defined security constants.
const handleExecute = async () => { setIsExecuting(true); setE2bFailed(false); setShowFailsafe(false); try { + // Guard rail: code size limit + const MAX_CODE_BLOCK_LENGTH = 50000; // TODO: import from a shared security constants module + if (code.length > MAX_CODE_BLOCK_LENGTH) { + toast.error(`Code is too large (max ${MAX_CODE_BLOCK_LENGTH} chars).`); + setIsExecuting(false); + return; + } const executionResult = await onExecute(code, language);If a shared module exists (e.g., '@/lib/security'), replace the inline constant with the import.
src/components/Navigation.tsx (2)
160-162: Sanitize user-generated name with SafeText (XSS hardening and guideline compliance)
user.fullNameoriginates from user data and is rendered directly. Per guidelines, all user-generated content must be sanitized and rendered viaSafeText. Wrap the displayed first name withSafeText.Apply these diffs:
- {user?.fullName?.split(' ')[0] || 'Profile'} + <SafeText text={user?.fullName?.split(' ')[0] ?? 'Profile'} />- {user?.fullName?.split(' ')[0] || 'Profile'} + <SafeText text={user?.fullName?.split(' ')[0] ?? 'Profile'} />Add the import (and prefer @/ alias for ui imports, see next comment):
import { useState, useEffect } from "react"; import { Menu, User, Zap } from "lucide-react"; -import { Button } from "./ui/button"; -import { Sheet, SheetContent, SheetTrigger } from "./ui/sheet"; +import { Button } from "@/components/ui/button"; +import { Sheet, SheetContent, SheetTrigger } from "@/components/ui/sheet"; import { Link, useNavigate } from "react-router-dom"; import { SignInButton, useClerk } from "@clerk/clerk-react"; import { useAuth } from "@/hooks/useAuth"; import { motion } from "framer-motion"; +import { SafeText } from "@/components/ui/SafeText";Also applies to: 273-275
151-192: Handle the auth “loading” state explicitly (spinner/skeleton) per guidelinesComponents under pages/components must show a loading indicator while auth is resolving, then render sign-in when unauthenticated and protected content when authenticated.
useAuth()usage here doesn’t account for a loading state.Suggested pattern:
- const { isAuthenticated, user } = useAuth(); + const { isAuthenticated, user, isLoading } = useAuth();Desktop actions:
- {isAuthenticated ? ( + {isLoading ? ( + <Button size="sm" variant="outline" disabled className="glass-hover border-white/20"> + Loading... + </Button> + ) : isAuthenticated ? ( <div className="flex items-center gap-2"> ... </div> ) : ( <SignInButton mode="redirect" forceRedirectUrl="/chat"> ... </SignInButton> )}Mobile actions:
- {isAuthenticated ? ( + {isLoading ? ( + <Button variant="outline" disabled className="glass-hover border-white/20 w-full"> + Loading... + </Button> + ) : isAuthenticated ? ( <div className="flex flex-col gap-3"> ... </div> ) : ( <SignInButton mode="redirect" forceRedirectUrl="/chat"> ... </SignInButton> )}Also applies to: 263-297
src/components/WebsiteCloneDialog.tsx (1)
401-406: Render untrusted analysis data with SafeText to meet project XSS rulesAnalysis fields (title, description, url, technologies, components, designPatterns, color labels) originate from external content and should be rendered via SafeText per repo rules. This keeps us consistent and future-proof even though React escapes text by default.
@@ -import { toast } from 'sonner'; +import { toast } from 'sonner'; +import { SafeText } from '@/components/ui/SafeText'; @@ - <h3 className="text-lg font-semibold text-gradient-static mb-2"> - {analysis.title || 'Website Analysis'} - </h3> + <h3 className="text-lg font-semibold text-gradient-static mb-2"> + <SafeText>{analysis.title || 'Website Analysis'}</SafeText> + </h3> @@ - <p className="text-muted-foreground text-sm mb-3"> - {analysis.description || 'No description available'} - </p> + <p className="text-muted-foreground text-sm mb-3"> + <SafeText>{analysis.description || 'No description available'}</SafeText> + </p> @@ - <span>{analysis.url}</span> + <span><SafeText>{analysis.url}</SafeText></span> @@ - {analysis.technologies.map((tech, index) => ( - <Badge key={index} variant="secondary" className="text-xs"> - {tech} - </Badge> - ))} + {analysis.technologies.map((tech, index) => ( + <Badge key={index} variant="secondary" className="text-xs"> + <SafeText>{tech}</SafeText> + </Badge> + ))} @@ - {analysis.components.map((component, index) => ( - <Badge key={index} variant="outline" className="text-xs"> - {component} - </Badge> - ))} + {analysis.components.map((component, index) => ( + <Badge key={index} variant="outline" className="text-xs"> + <SafeText>{component}</SafeText> + </Badge> + ))} @@ - {analysis.designPatterns.map((pattern, index) => ( - <Badge key={index} variant="secondary" className="text-xs"> - {pattern} - </Badge> - ))} + {analysis.designPatterns.map((pattern, index) => ( + <Badge key={index} variant="secondary" className="text-xs"> + <SafeText>{pattern}</SafeText> + </Badge> + ))} @@ - <span className="text-xs text-muted-foreground font-mono"> - {color} - </span> + <span className="text-xs text-muted-foreground font-mono"> + <SafeText>{color}</SafeText> + </span>Follow-ups (optional, not shown above to keep the diff focused):
- Consider validating and normalizing color strings before applying them to style.backgroundColor.
- When adding new surfaces that render analysis content, prefer by default.
Also applies to: 408-412, 462-466, 479-483, 496-500, 519-521, 22-25
convex/messages.ts (3)
7-7: Use isomorphic DOMPurify in Convex/server code to avoid runtime/bundling failuresImporting dompurify directly can break in non-DOM environments (Convex server). Switch to isomorphic-dompurify or lazy-import inside the browser-only branch to prevent reference errors during bundling/runtime.
-import DOMPurify from 'dompurify'; +import DOMPurify from 'isomorphic-dompurify';Optionally, defer the import to the browser-only path in createTrustedTypesPolicy to avoid pulling DOMPurify into server bundles.
Also applies to: 20-30
280-287: atob is not available in Node/Convex; replace with safe base64 validation/decodingConvex/Node doesn’t provide atob, leading to ReferenceError. Use Buffer-based validation instead.
- // Basic validation of base64 encoding - try { - atob(args.encryptedContent); - atob(args.encryptionSalt); - atob(args.encryptionIv); - } catch { - throw new Error("Invalid base64 encoding in encryption fields"); - } + // Basic validation of base64 encoding (Node-safe) + const isValidBase64 = (s: string): boolean => { + // normalize padding and whitespace + const t = s.replace(/\s+/g, ''); + if (t.length % 4 !== 0) return false; + // re-encode round-trip check + const buf = Buffer.from(t, 'base64'); + return buf.length > 0 && buf.toString('base64') === t; + }; + if ( + !isValidBase64(args.encryptedContent) || + !isValidBase64(args.encryptionSalt) || + !isValidBase64(args.encryptionIv) + ) { + throw new Error("Invalid base64 encoding in encryption fields"); + }
818-892: Client/browser helpers should not live in Convex moduleCSP builders, Trusted Types policy, storage wrappers, CSRF storage, and File-based validators are browser-only concerns. Keeping them in a Convex server file increases bundle surface and risk. Move these to src/utils (client) and import where needed; keep convex/messages.ts focused on DB/business logic.
Concretely, consider extracting:
- getCSPConfig, generateCSPHeader, getSecurityHeaders
- validateFileUpload (relies on DOM File)
- browserStorage, sessionStorage, csrf
- createTrustedTypesPolicy, isSecureContext
Then re-export from an index on the client side.
Also applies to: 897-1098, 1102-1165
src/lib/firecrawl.ts (1)
1-3: Incorrect logger import: Sentry doesn’t exportlogger
@sentry/reactdoes not expose alogger. This destructuring yieldsundefined, and everylogger.*call will throw at runtime. Import the project’s logger from@/lib/error-handlerinstead, and (optionally) keep Sentry for telemetry.-import * as Sentry from '@sentry/react' - -const { logger } = Sentry +import * as Sentry from '@sentry/react' +import { logger } from '@/lib/error-handler'src/components/GitHubIntegration.tsx (3)
27-30: Incorrect logger import: Sentry doesn’t exportloggerThis pattern yields
loggerasundefinedand will crash onlogger.error(...). Import the project logger from@/lib/error-handler.-import * as Sentry from '@sentry/react'; - -const { logger } = Sentry; +import * as Sentry from '@sentry/react'; +import { logger } from '@/lib/error-handler';
216-225: PR head owner is wrong; PR will target base owner’s branch, not the fork
createPullRequestcomputesheadas${owner}:${headBranch}whenoriginalOwneris provided. You’re passingowner: originalOwner, so the head becomesoriginalOwner:branch, which doesn’t exist. It should be the fork owner (userLogin).-const prOptions: CreatePullRequestOptions = { - owner: originalOwner, +const prOptions: CreatePullRequestOptions = { + owner: userLogin, // fork owner repo: originalRepo, title: prTitle, body: prDescription, headBranch: branchName, baseBranch: currentRepo.default_branch, originalOwner: originalOwner };
498-506: Use SafeText for user-generated content per project security rulesRepo metadata and dynamic messages are user-generated/external. Wrap them with
SafeTextto enforce consistent XSS protection.+import { SafeText } from '@/components/ui/SafeText'; ... -<h3 className="font-semibold">{currentRepo.full_name}</h3> -<p className="text-sm text-muted-foreground">{currentRepo.description}</p> +<h3 className="font-semibold"><SafeText text={currentRepo.full_name} /></h3> +<p className="text-sm text-muted-foreground"><SafeText text={currentRepo.description ?? ''} /></p> ... -<span className="text-sm">{operationStatus.message}</span> +<span className="text-sm"><SafeText text={operationStatus.message} /></span>Optional: also sanitize any other externally sourced strings rendered in this component.
Also applies to: 321-323, 475-477
src/lib/github-service.ts (3)
1-7: Incorrect logger import: Sentry doesn’t exportloggerSame issue as other files—this will crash on first
logger.*use.-// Remove unused import -// import { toast } from 'sonner'; -import * as Sentry from '@sentry/react'; +// Remove unused import +// import { toast } from 'sonner'; +import * as Sentry from '@sentry/react'; +import { logger } from '@/lib/error-handler'; - -const { logger } = Sentry;
205-207: Wrong endpoint: should be/git/refs/heads/{branch}(pluralrefs)
/git/ref/heads/...does not exist. This will 404 and break branch creation.-`/repos/${owner}/${repo}/git/ref/heads/${fromBranch}` +`/repos/${owner}/${repo}/git/refs/heads/${fromBranch}`
249-252: Avoid deprecatedunescapeand ensure UTF-8-safe Base64 encoding
unescapeis deprecated andbtoabreaks on non-ASCII. UseTextEncoderin the browser, andBufferon server.- content: btoa(unescape(encodeURIComponent(file.content))), // Base64 encode + content: (typeof window === 'undefined' + ? Buffer.from(file.content, 'utf8').toString('base64') + : btoa(String.fromCharCode(...new TextEncoder().encode(file.content))) + ), // UTF-8 safe Base64src/components/LoadingStates.tsx (1)
305-307: isCompleted can become string | boolean; force boolean
const isCompleted = currentStep && (findIndex > index)yieldsstring | booleanunder strict TS. Wrap with a ternary for a pure boolean.Apply:
- const isCompleted = currentStep && steps.findIndex(s => s.id === currentStep) > index; + const isCompleted = currentStep ? (steps.findIndex(s => s.id === currentStep) > index) : false;src/lib/ai.ts (1)
21-21: Bug: Sentry doesn’t exportlogger; this will crash at runtime/TS
@sentry/reacthas nologgerexport. Use the app’s logger (seesrc/lib/error-handler.ts) and keep Sentry for spans/telemetry.Apply:
-import * as Sentry from '@sentry/react' +import * as Sentry from '@sentry/react' +import { logger } from './error-handler' @@ -const { logger } = Sentry; +// Use app logger; Sentry remains for spans and captureExceptionsrc/lib/__tests__/ai-production.test.ts (2)
257-279: Test relies on spy but now uses mock implementationThe test creates
recordSpyas a spy on Line 257 but then immediately overrides it with a mock implementation on Line 355, which disconnects the spy from the actual implementation. This makes the assertions on Lines 273-278 potentially unreliable sinceaiMonitoring.getMetricsmay not reflect the mocked calls.Apply this diff to properly test the monitoring functionality:
describe('AI Monitoring', () => { it('should record successful operations', () => { - const recordSpy = vi.spyOn(aiMonitoring, 'recordOperation'); - aiMonitoring.recordOperation({ operation: 'test', model: 'test-model', duration: 1000, success: true, inputTokens: 100, outputTokens: 200, cost: 0.01 }); - expect(recordSpy).toHaveBeenCalled(); - const metrics = aiMonitoring.getMetrics('test', 'test-model'); expect(metrics).toBeTruthy(); if (metrics && typeof metrics === 'object' && 'totalRequests' in metrics) { expect(metrics.totalRequests).toBe(1); expect(metrics.successfulRequests).toBe(1); expect(metrics.avgResponseTime).toBe(1000); expect(metrics.totalCost).toBe(0.01); } });
353-367: Inconsistent mock setup approachThe mock monitoring call is created but never used or verified. The test sets up the mock but doesn't complete the integration test flow, leaving commented-out code that should either be implemented or removed.
Either complete the integration test or remove the incomplete setup:
describe('Integration Tests', () => { it('should handle complete AI request flow with monitoring', async () => { // Mock the AI SDK response const mockGenerateText = vi.fn().mockResolvedValue({ text: 'Test response', usage: { promptTokens: 100, completionTokens: 200 } }); vi.doMock('ai', () => ({ generateText: mockGenerateText })); - // Track monitoring calls - const mockMonitoringCall = vi.fn(); - vi.spyOn(aiMonitoring, 'recordOperation').mockImplementation(mockMonitoringCall); - - // Make request (would need proper mocking setup) - // const response = await generateAIResponse('Test prompt'); - - // Verify monitoring was called - // expect(monitoringSpy).toHaveBeenCalledWith( - // expect.objectContaining({ - // operation: 'generateText', - // success: true - // }) - // ); + // TODO: Implement full integration test once generateAIResponse is properly exported + expect(mockGenerateText).toBeDefined(); }); });src/lib/error-handler.ts (2)
309-316: Fix triggerRetry signature/call mismatchtriggerRetry() no longer accepts an argument, but showUserNotification still calls it with error. This will fail type-checking and at runtime in JS strict mode.
Apply this diff to remove the argument at the call site:
- action: error.isRetryable ? { - label: 'Retry', - onClick: () => this.triggerRetry(error) - } : undefined + action: error.isRetryable ? { + label: 'Retry', + onClick: () => this.triggerRetry() + } : undefinedAlso applies to: 339-343
363-385: Gate Sentry reporting behind privacy consent; avoid sending PII without consentGiven the new PrivacyConsentBanner and privacy-consent helpers in this PR, Sentry reporting should respect user choices. Add a consent check before capturing exceptions; also avoid setting user identifiers unless consent is granted.
Suggested patch:
private reportToSentry(error: AppError): void { + // Respect client-side privacy consent + if (typeof window !== 'undefined') { + try { + const consent = privacyConsent?.getConsent?.(); + if (!consent?.errorMonitoring) { + return; + } + } catch { + // If consent state is unavailable, default to not reporting + return; + } + } Sentry.withScope((scope) => { // Set error context scope.setTag('errorType', error.type); scope.setLevel(this.getSentryLevel(error.severity)); scope.setContext('errorDetails', { code: error.code, statusCode: error.statusCode, isRetryable: error.isRetryable, userMessage: error.userMessage, technicalMessage: error.technicalMessage }); if (error.context) { scope.setContext('appContext', error.context); - if (error.context.userId) { - scope.setUser({ id: error.context.userId }); - } + // Only attach PII when error monitoring consent is enabled + if (error.context.userId) { + scope.setUser({ id: error.context.userId }); + } } Sentry.captureException(error); }); }Add this import near the top:
import { privacyConsent } from '@/lib/privacy-consent';If you also have sanitizeError/logSanitizedError utility, consider using it to reduce potentially sensitive payloads in errorDetails/context.
src/pages/Pricing.tsx (1)
392-414: Don’t trust userId from localStorage; send an authenticated request insteadPosting a client-supplied userId allows spoofing. The API treats auth as optional, so your call proceeds unauthenticated. Use Clerk’s JWT in the Authorization header and avoid relying on localStorage user IDs.
Apply this diff to send a Bearer token and remove dependency on localStorage identity where possible (keep userId only if the server currently requires it):
+import { useAuth as useClerkAuth } from "@clerk/clerk-react"; +import { toast } from "sonner"; @@ const Pricing = () => { + const { getToken, isLoaded: clerkLoaded } = useClerkAuth(); // Trigger KV sync after successful checkout redirect useEffect(() => { const params = new URLSearchParams(window.location.search); if (params.get('billing') === 'success') { - const id = localStorage.getItem('convexUserId'); - if (id) { - fetch('/api/success', { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - }, - body: JSON.stringify({ userId: id }) - }) - .catch((error) => { - console.error('Failed to sync billing success:', error); - Sentry.captureException(error, { - tags: { feature: 'pricing', action: 'billing_success_sync' }, - extra: { userId: 'redacted' } // Avoid logging raw userId - }); - }); - } + (async () => { + try { + const id = localStorage.getItem('convexUserId'); // TODO: remove when API no longer requires body.userId + const token = clerkLoaded ? await getToken() : null; + const res = await fetch('/api/success', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + ...(token ? { 'Authorization': `Bearer ${token}` } : {}), + }, + body: JSON.stringify({ userId: id ?? 'me' }) + }); + if (!res.ok) { + throw new Error(`HTTP ${res.status}`); + } + } catch (error) { + console.error('Failed to sync billing success:', error); + Sentry.captureException(error, { + tags: { feature: 'pricing', action: 'billing_success_sync' }, + extra: { userId: 'redacted' } // Avoid logging raw userId + }); + toast.error('We could not verify your subscription yet. If you just completed checkout, this will retry shortly.'); + } finally { + window.history.replaceState({}, '', window.location.pathname); + } + })(); + return; // ensure we don't fall through + } - window.history.replaceState({}, '', window.location.pathname); - } }, []);Follow-up: Update api/success.ts to require auth and ignore userId from the body. I’ve left the body field for backward compatibility until the server is tightened.
Also applies to: 397-404
src/pages/Settings.tsx (2)
373-397: Authenticate invoice requests like other billing endpoints/api/list-invoices likely requires user context. Use Clerk’s token similarly to create-portal-session and forward it via Authorization.
useEffect(() => { async function loadInvoices() { if (!user) return; try { setLoadingInvoices(true); - const res = await fetch('/api/list-invoices', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({}), - }); + const token = await getToken(); + if (!token) { + throw new Error('Not authenticated'); + } + const res = await fetch('/api/list-invoices', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'Authorization': `Bearer ${token}`, + }, + body: JSON.stringify({}), + });If the endpoint is intentionally public, please confirm and add a comment explaining why. Otherwise, this change enforces the “proper authentication” guideline.
Also applies to: 378-383
223-231: Replace Invalid Model ID & Add User FeedbackThe model identifier
"openai/gpt-oss-120b"is not listed as a valid public model in Groq’s Chat Completions API as of August 24, 2025. You’ll need to:
- Update the
modelfield to a currently supported Groq model ID (e.g. one from the/v1/modelsendpoint).
• Verify the exact model name against Groq’s official docs or by callingGET https://api.groq.ai/v1/modelswith your API key.- Implement toast notifications (or another UI feedback mechanism) so users see immediate success or failure messages when the request completes.
Please replace the placeholder model with the correct one and add toasts on both the
response.okand!response.okbranches to surface errors.src/hono-server.ts (2)
83-93: Fix Clerk token verification options; use issuer (and audience) instead of jwtKeyPassing
{ jwtKey: issuer }is incorrect for@clerk/backend’sverifyToken. Use{ issuer, audience }to avoid false negatives/positives in auth.- const { verifyToken } = await import('@clerk/backend'); - - const options: { jwtKey?: string; audience?: string } = { jwtKey: issuer }; - if (audience) options.audience = audience; - - const verified = await verifyToken(token, options) as { sub?: string; email?: string }; + const { verifyToken } = await import('@clerk/backend'); + const options: { issuer: string; audience?: string } = { issuer }; + if (audience) options.audience = audience; + const verified = await verifyToken(token, options) as { sub?: string; email?: string };
288-297: Customer portal: don’t trust query param; derive customerId from the authenticated userReturning
event.req.query('customerId') || ''allows users to open arbitrary customer portals. Compute thecustomerIdfrom the authenticated user (like in/subscription) and ignore the query param.CustomerPortal({ accessToken: (() => { const token = process.env.POLAR_ACCESS_TOKEN; if (!token) throw new Error('POLAR_ACCESS_TOKEN environment variable is required'); return token; })(), - getCustomerId: async (event) => { - // Extract customer ID from query params or user context - const customerId = event.req.query('customerId'); - return customerId || ''; - }, + getCustomerId: async (event) => { + const auth = event.req.header('Authorization') || ''; + const user = await verifyClerkToken(auth); + if (!user) throw new Error('Unauthorized'); + const polar = getPolarClient(); + // Minimal mapping: prefer email + const customers = await polar.customers.list({ email: user.email || undefined, limit: 10 }); + const list = Array.isArray(customers) ? customers : (customers as { data?: Array<{ id: string; email?: string; metadata?: { userId?: string } }> }).data || []; + const match = list.find(c => c.metadata?.userId === user.id || (user.email && c.email === user.email)); + if (!match) throw new Error('No customer'); + return match.id; + }, server: process.env.NODE_ENV === 'production' ? 'production' : 'sandbox', })src/components/ChatInterface.tsx (1)
149-194: DRY/security: reuse shared sanitization/validation utilities instead of redefining them hereImport
sanitizeText,validateInput, andMAX_MESSAGE_LENGTHfrom@/utils/securityto keep one source of truth and ensure consistent rules across the app.-// Security constants for input validation -const MAX_MESSAGE_LENGTH = 10000; - -// XSS protection: sanitize text input -const sanitizeText = (text: string): string => { - return text - .replace(/[<>'"&]/g, (char) => { - const chars: { [key: string]: string } = { - '<': '<', - '>': '>', - "'": ''', - '"': '"', - '&': '&' - }; - // eslint-disable-next-line security/detect-object-injection - return chars[char] || char; - }) - .trim(); -}; - -// Validate input length and content -const validateInput = (text: string, maxLength: number): { isValid: boolean; error?: string } => { - if (!text || text.trim().length === 0) { - return { isValid: false, error: 'Input cannot be empty' }; - } - if (text.length > maxLength) { - return { isValid: false, error: `Input too long. Maximum ${maxLength} characters allowed` }; - } - // Check for potentially malicious patterns - const suspiciousPatterns = [ - /<script/i, - /javascript:/i, - /vbscript:/i, - /onload=/i, - /onerror=/i, - /onclick=/i - ]; - - for (const pattern of suspiciousPatterns) { - if (pattern.test(text)) { - return { isValid: false, error: 'Invalid content detected' }; - } - } - - return { isValid: true }; -}; +import { MAX_MESSAGE_LENGTH, sanitizeText, validateInput } from '@/utils/security';api/hono-polar.ts (1)
98-118: Plan inference drops Enterprise to StarterWhen syncing to Convex,
planIdfallback uses a binaryincludes('pro') ? 'pro' : 'starter'. If you sell Enterprise, add it here; otherwise customers may be downgraded.- const planId = subscription.metadata?.planId || - (subscription.product?.name?.toLowerCase().includes('pro') ? 'pro' : 'starter'); + const productName = subscription.product?.name?.toLowerCase() || ''; + const planId = + subscription.metadata?.planId || + (productName.includes('enterprise') ? 'enterprise' + : productName.includes('pro') ? 'pro' + : 'starter');api-dev-server.ts (1)
561-565: CORS: Only set Allow-Credentials when origin is allowedCurrently
Access-Control-Allow-Credentials: trueis set unconditionally, contradicting your own comment and risking credentialed requests from disallowed origins.Apply this diff:
- // Always only set credentials if a specific, trusted origin is matched - res.setHeader('Access-Control-Allow-Credentials', 'true'); + // Only set credentials when a specific, trusted origin matched + if (allowedOrigin) { + res.setHeader('Access-Control-Allow-Credentials', 'true'); + }
♻️ Duplicate comments (4)
convex/messages.ts (1)
114-173: Strengthen disallowed script pattern and dotall handlingThe current regexp may miss cases like “</script >” and multi-line matches. Use a dotall-safe pattern and allow optional whitespace before the end tag’s “>”.
- /<script[^>]*>.*?<\/script>/gi, // Script tags (safer pattern) + /<script[^>]*>[\s\S]*?<\/script\s*>/gi, // Script tags (safer + end-tag whitespace)src/lib/firecrawl.ts (1)
11-13: Regex still misses edge-case closing tags like</script >and</style >The non-greedy patterns are an improvement, but they still fail when there’s whitespace before
>in the closing tag. CodeQL flagged this earlier and it remains applicable.- .replace(/<script[^>]*>[\s\S]*?<\/script>/gi, '') // Remove script tags with content - .replace(/<style[^>]*>[\s\S]*?<\/style>/gi, '') // Remove style tags with content + .replace(/<script\b[^>]*>[\s\S]*?<\/script\s*>/gi, '') // Remove script tags (handles </script >) + .replace(/<style\b[^>]*>[\s\S]*?<\/style\s*>/gi, '') // Remove style tags (handles </style >)Follow-up: Consider using a proper HTML sanitizer/parser (e.g., DOMPurify in the browser or sanitize-html on the server) instead of regex for robust coverage. I can provide a safe wrapper if you want.
lib/deployment/vercel.ts (1)
570-575: Harden git host detection; substring checks are exploitable (fix CodeQL finding)
includes('github.com')can be bypassed with attacker domains likeevil-github.com. Parse the URL or SSH form and strictly match the hostname. Throw on unsupported hosts.Apply this diff:
- private extractGitProvider(url: string): string { - if (url.includes('github.com')) return 'github'; - if (url.includes('gitlab.com')) return 'gitlab'; - if (url.includes('bitbucket.org')) return 'bitbucket'; - return 'github'; // default - } + private extractGitProvider(url: string): string { + try { + if (url.startsWith('git@')) { + const m = url.match(/^git@([^:]+):/); + const host = m?.[1]?.toLowerCase(); + if (host === 'github.com') return 'github'; + if (host === 'gitlab.com') return 'gitlab'; + if (host === 'bitbucket.org') return 'bitbucket'; + throw new DeploymentError(`Unsupported git host: ${host ?? 'unknown'}`, 'vercel', 'UNSUPPORTED_GIT_HOST'); + } + const u = new URL(url); + const host = u.hostname.toLowerCase(); + if (host === 'github.com') return 'github'; + if (host === 'gitlab.com') return 'gitlab'; + if (host === 'bitbucket.org') return 'bitbucket'; + } catch { + // fallthrough + } + throw new DeploymentError('Invalid or unsupported git repository URL', 'vercel', 'INVALID_GIT_HOST'); + }lib/deployment/netlify.ts (1)
621-625: Harden git host detection; avoid substring checks (matches CodeQL finding)Although
validateGitUrlconstrains hosts, replacing substring checks with exact hostname parsing further reduces risk and satisfies scanners.Apply this diff:
- private extractGitProvider(url: string): string { - if (url.includes('github.com')) return 'github'; - if (url.includes('gitlab.com')) return 'gitlab'; - if (url.includes('bitbucket.org')) return 'bitbucket'; - return 'github'; // default - } + private extractGitProvider(url: string): string { + try { + const host = new URL(url).hostname.toLowerCase(); + if (host === 'github.com') return 'github'; + if (host === 'gitlab.com') return 'gitlab'; + if (host === 'bitbucket.org') return 'bitbucket'; + } catch { + // fallthrough + } + throw new DeploymentError('Invalid or unsupported git host', 'netlify', 'INVALID_GIT_HOST'); + }
Pull request was closed
…m our branch Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
1 participant
Summary by CodeRabbit