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
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
High Priority Fixes: - Replace vulnerable regex patterns in IP validation with safe string operations - Secure cookie parsing with Object.create(null) to prevent prototype pollution - Enhanced file system operations with additional validation layers - Add PostHog analytics payload size limits (32KB) and comprehensive PII sanitization - Implement error message sanitization to prevent information leakage Security Improvements: - Safe IPv4/IPv6 validation without regex DoS vulnerability - Cookie name/value validation with length limits and safe patterns - Multi-layer path traversal protection for API endpoint resolution - PII pattern detection and redaction for analytics - Development vs production error handling with safe messaging - ESLint security rule compliance with appropriate exemptions for validated cases 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
|
| 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.
|
|
Caution Review failedThe pull request is closed. WalkthroughAdds a multi-platform deployment system (Netlify/Vercel) with new deployment/domain APIs, a security- and analytics-focused clustered API dev server, a multi-process dev orchestrator, checkout/CORS updates, extensive frontend preview and smart-prompt UI, many utility hardenings, docs, and an environment template. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant FE as Frontend
participant API as /api/deploy
participant M as ZapdevDeploymentManager
participant S as PlatformService (Netlify/Vercel)
participant DNS as DNS Provider (instructions)
participant PH as PostHog
U->>FE: Request deploy (subdomain, files/git)
FE->>API: POST /api/deploy { action: "deploy", ... }
API->>M: deploy(config)
M-->>PH: capture deployment_started (async)
M->>S: deploy(config)
alt Custom subdomain requested
S->>S: platform domain setup
S-->>M: domain + dnsRecords
M->>DNS: compose DNS instructions
end
S-->>M: DeploymentResult { id, url, status }
M-->>PH: capture deployment_completed/failed (async)
M-->>API: DeploymentResult
API-->>FE: 200 OK { deploymentId, url, status, dnsInstructions? }
sequenceDiagram
autonumber
actor Dev as Developer
participant Orchestrator as dev-server.ts
participant Vite as Vite (Frontend)
participant Convex as Convex Dev
participant APISrv as api-dev-server.ts
participant PH as PostHog
Dev->>Orchestrator: npm run dev:all
Orchestrator->>Vite: spawn (npm run dev)
Orchestrator->>Convex: spawn (npx convex dev)
Orchestrator->>APISrv: dynamic import/start
APISrv->>APISrv: Cluster workers + rate limit + CORS
APISrv-->>PH: server_started + periodic metrics
Note over Orchestrator: Graceful shutdown on SIGINT/SIGTERM
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested labels
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (44)
✨ Finishing Touches
🧪 Generate unit tests
🪧 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 (
|
|
Claude finished @otdoges's task —— View job 🔍 Pull Request Review CompletedI've conducted a comprehensive review of this large PR (5811 additions, 351 deletions across 44 files). Here's my detailed analysis: Review Checklist:
🚨 Critical Security Issues1. GitGuardian Alerts - FALSE POSITIVES ✅The GitGuardian alerts in
2. Type Safety Violations
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| // Use sanitized error message for client response | ||
| const sanitizedError = sanitizeErrorForClient(error, CONFIG.NODE_ENV === 'development'); | ||
| res.writeHead(500, { 'Content-Type': 'application/json' }); | ||
| res.end(JSON.stringify(sanitizedError)); |
Check warning
Code scanning / CodeQL
Information exposure through a stack trace Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
The best way to fix this problem is to never include stack trace information (or excessive internal details, such as file paths or code excerpts) in error responses sent to clients, regardless of environment. Instead, the response should offer a generic error message, possibly with minimal extra context (such as an error code) to aid client debugging, but all internal or sensitive details should be logged only on the server side. To accomplish this:
- In the
sanitizeErrorForClientfunction, always produce a safe, minimal error object for the client, never revealing stack trace, code excerpts, file paths, or raw error messages. - Remove or greatly simplify the environment-based distinction: both development and production responses should be safe for external exposure.
- Retain all logging (including stack traces, file paths, and full error objects) on the server via existing logger calls to ensure adequate debugging support for developers.
- The code to modify is primarily within the
sanitizeErrorForClientfunction (lines 200–242). - You may update error messages, remove the development branch, and clarify comments as needed.
No new imports are required, and only changes within api-dev-server.ts (to sanitizeErrorForClient) are necessary.
| @@ -196,31 +196,18 @@ | ||
|
|
||
| const analytics = new PostHogAnalytics(); | ||
|
|
||
| // Utility function to sanitize error messages for client responses | ||
| function sanitizeErrorForClient(error: Error | unknown, isDevelopment: boolean = false): { error: string; code?: string } { | ||
| // Always return a generic error message for client responses, never expose stack trace or internal details. | ||
| function sanitizeErrorForClient(error: Error | unknown): { error: string } { | ||
| if (!error) { | ||
| return { error: 'Unknown error occurred' }; | ||
| } | ||
|
|
||
|
|
||
| // Log full error and stack trace on the server only (already handled via logger.error elsewhere) | ||
| // For the client, only provide safe, non-sensitive messages | ||
|
|
||
| const errorMessage = error instanceof Error ? error.message : String(error); | ||
|
|
||
| // In development, provide more details (but still sanitized) | ||
| if (isDevelopment && CONFIG.NODE_ENV === 'development') { | ||
| // Remove sensitive paths and internal details but keep useful info | ||
| const sanitized = errorMessage | ||
| .replace(/\/[^/\s]*\/([^/\s]*\/)*[^/\s]*(\.(js|ts|json))/g, '[FILE_PATH]') | ||
| .replace(/at\s+[^\s]+\s+\([^)]+\)/g, '[STACK_TRACE]') | ||
| .replace(/ENOENT.*'/g, 'File not found') | ||
| .replace(/EACCES.*'/g, 'Permission denied') | ||
| .replace(/Error:\s*/g, ''); | ||
|
|
||
| return { | ||
| error: sanitized.length > 200 ? sanitized.substring(0, 200) + '...' : sanitized, | ||
| code: 'DEVELOPMENT_ERROR' | ||
| }; | ||
| } | ||
|
|
||
| // In production, return generic errors for security | ||
|
|
||
| // Match known error patterns and return safe messages | ||
| const commonErrors: Record<string, string> = { | ||
| 'Request timeout': 'Request timed out', | ||
| 'Invalid API handler export': 'Service temporarily unavailable', | ||
| @@ -229,15 +212,14 @@ | ||
| 'ETIMEDOUT': 'Request timed out', | ||
| 'ECONNRESET': 'Connection interrupted' | ||
| }; | ||
|
|
||
| // Check for known error patterns | ||
|
|
||
| for (const [pattern, safeMessage] of Object.entries(commonErrors)) { | ||
| if (errorMessage.includes(pattern)) { | ||
| return { error: safeMessage }; | ||
| } | ||
| } | ||
| // Default safe error message | ||
|
|
||
| // Default generic error message | ||
| return { error: 'Internal server error' }; | ||
| } | ||
|
|
||
| @@ -784,7 +761,7 @@ | ||
|
|
||
| if (!res.headersSent) { | ||
| // Use sanitized error message for client response | ||
| const sanitizedError = sanitizeErrorForClient(error, CONFIG.NODE_ENV === 'development'); | ||
| const sanitizedError = sanitizeErrorForClient(error); | ||
| res.writeHead(500, { 'Content-Type': 'application/json' }); | ||
| res.end(JSON.stringify(sanitizedError)); | ||
| } |
| } | ||
|
|
||
| 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 correct fix is to parse the incoming URL using a robust URL parser (the standard URL object in Node.js/TypeScript), extract the host, and compare it against allowed hosts. This avoids matching 'github.com' when it appears in the path, query, or as a part of another domain name (for example, 'evilgithub.com').
We should update the extractGitProvider method in lib/deployment/vercel.ts:
- Use the
URLconstructor to parse incoming URLs safely. - Normalize the host for comparison (to lowercase).
- Accept exact matches and, if required, subdomain matches appropriately (e.g., 'github.com' and 'www.github.com').
- Fail gracefully if the URL is invalid.
If the URL cannot be parsed, just fall back to the default 'github' value as before.
We'll also need to importURLfrom the global scope—no extra imports are required, asURLis a standard part of JS/Node.
| @@ -564,10 +564,16 @@ | ||
| } | ||
|
|
||
| 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 | ||
| try { | ||
| const parsed = new URL(url); | ||
| const host = parsed.hostname.toLowerCase(); | ||
| 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 | ||
| } catch (e) { | ||
| return 'github'; // default | ||
| } | ||
| } | ||
|
|
||
| private extractRepoPath(url: string): string { |
|
|
||
| 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
To fix the problem, instead of checking whether .includes('gitlab.com'), the code should parse the URL, extract the hostname, and then compare it robustly to known values. This approach prevents attackers or malformed URLs from misleading the provider detection logic by placing substrings in other URL components. The fix requires changing the checks in extractGitProvider to parse the URL (using the Node.js standard URL object), and compare the hostname (and if needed, handle subdomains appropriately) to the expected domains.
Region to change: Only the body of the extractGitProvider method.
What is needed:
- Import or use the global
URLclass. - Parse the URL, handle exceptions for invalid URLs.
- Compare the
hostname(possibly usingendsWithfor subdomains). - Refactor the logic for accurate detection.
| @@ -564,9 +564,23 @@ | ||
| } | ||
|
|
||
| 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 not a valid URL, fallback to string includes (legacy behavior) | ||
| 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 { | ||
| 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, the function extractGitProvider should properly parse the URL and check the host component instead of checking if the provider's domain is a substring anywhere in the entire URL. The standard URL constructor in JavaScript/TypeScript can be used for robust URL parsing. The function should use the hostname from the parsed URL and compare it exactly against allowed provider hostnames (github.com, gitlab.com, bitbucket.org). If the URL is malformed, the function should either return a default (e.g., "github"), throw an error, or return a fallback value.
- Change only the implementation of
extractGitProviderwithinlib/deployment/vercel.ts. - Import anything needed (
URLis built-in and doesn't require an import in Node 10+ unless targeting legacy environments). - No change to outside logic or function signature.
| @@ -564,9 +564,14 @@ | ||
| } | ||
|
|
||
| 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); | ||
| if (u.hostname === 'github.com') return 'github'; | ||
| if (u.hostname === 'gitlab.com') return 'gitlab'; | ||
| if (u.hostname === 'bitbucket.org') return 'bitbucket'; | ||
| } catch { | ||
| // fallback for invalid URLs | ||
| } | ||
| return 'github'; // default | ||
| } | ||
|
|
…for credentials transfer Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
🚨 Bugbot Trial ExpiredYour Bugbot trial has expired. Please purchase a license in the Cursor dashboard to continue using Bugbot. |
…ng sanitization Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
|
Claude finished @otdoges's task —— View job 🔍 Comprehensive PR Review - CompleteI've conducted a thorough review of this substantial PR (5,842 additions, 354 deletions across 44 files). Here's my detailed feedback:
|
|
Claude finished @otdoges's task —— View job 🔍 Pull Request Review CompleteI've conducted a comprehensive review of this large PR (5,842 additions, 354 deletions across 44 files). Here are my findings: Review Checklist:
🚨 Critical Security Issues1. False Positive GitGuardian Alerts ✅The GitGuardian alerts for
Status: ✅ Safe - This is a legitimate environment configuration template. 2. URL Sanitization Vulnerabilities 🔴 CRITICALIssue: Multiple security alerts for incomplete URL substring sanitization in:
Problem: Current code uses Recommendation: Replace with proper URL parsing: private extractGitProvider(url: string): string {
try {
const parsedUrl = new URL(url);
const hostname = parsedUrl.hostname.toLowerCase();
if (hostname === 'github.com') return 'github';
if (hostname === 'gitlab.com') return 'gitlab';
if (hostname === 'bitbucket.org') return 'bitbucket';
return 'github';
} catch {
// Fallback for git@ style URLs
const match = url.match(/@([a-zA-Z0-9.-]+)[:/]/);
const hostname = match?.[1]?.toLowerCase();
if (hostname === 'github.com') return 'github';
if (hostname === 'gitlab.com') return 'gitlab';
if (hostname === 'bitbucket.org') return 'bitbucket';
return 'github';
}
}3. CORS Configuration Issues
|
| // Skip potentially sensitive keys | ||
| const isSensitive = PII_PATTERNS.some(pattern => pattern.test(key)); | ||
| if (isSensitive) { | ||
| sanitized[key] = '[REDACTED]'; |
Check failure
Code scanning / ESLint
Detects "variable[key]" as a left- or right-hand assignment operand. Error
| if (isDevelopment && CONFIG.NODE_ENV === 'development') { | ||
| // Remove sensitive paths and internal details but keep useful info | ||
| const sanitized = errorMessage | ||
| .replace(/\/[^/\s]*\/([^/\s]*\/)*[^/\s]*(\.(js|ts|json))/g, '[FILE_PATH]') |
Check failure
Code scanning / ESLint
Detects potentially unsafe regular expressions, which may take a very long time to run, blocking the event loop. Error
| const query: { [key: string]: string | string[] } = {}; | ||
| for (const [key, value] of searchParams.entries()) { | ||
| const safeKey = String(key); | ||
| const existingValue = query[safeKey]; |
Check failure
Code scanning / ESLint
Detects "variable[key]" as a left- or right-hand assignment operand. Error
| if (Array.isArray(existingValue)) { | ||
| (existingValue as string[]).push(value); | ||
| } else { | ||
| query[safeKey] = [existingValue as string, value]; |
Check failure
Code scanning / ESLint
Detects "variable[key]" as a left- or right-hand assignment operand. Error
| query[safeKey] = [existingValue as string, value]; | ||
| } | ||
| } else { | ||
| query[safeKey] = value; |
Check failure
Code scanning / ESLint
Detects "variable[key]" as a left- or right-hand assignment operand. Error
| lg: sizeClasses.lg, | ||
| xl: sizeClasses.xl | ||
| }; | ||
| return sizeMap[safeSize] || sizeClasses.md; |
Check failure
Code scanning / ESLint
Detects "variable[key]" as a left- or right-hand assignment operand. Error
| // Mark current step as error | ||
| if (steps[currentStep]) { | ||
| updateStepStatus(steps[currentStep]?.id || '', 'error'); | ||
| const currentStepData = steps.length > currentStep ? steps[currentStep] : null; |
Check failure
Code scanning / ESLint
Detects "variable[key]" as a left- or right-hand assignment operand. Error
| >(({ index, className, ...props }, ref) => { | ||
| const inputOTPContext = React.useContext(OTPInputContext) | ||
| const slot = inputOTPContext.slots[index] | ||
| const slot = inputOTPContext?.slots?.[index] || null |
Check failure
Code scanning / ESLint
Detects "variable[key]" as a left- or right-hand assignment operand. Error
| /(?:https?:\/\/)?(?:www\.)?github\.com\/([^/]+)\/([^/]+)(?:\/.*)?$/, | ||
| /(?:https?:\/\/)?(?:www\.)?github\.com\/([^/]+)\/([^/]+)\.git$/, | ||
| /git@github\.com:([^/]+)\/([^/]+)\.git$/, | ||
| /^(?:https?:\/\/)?(?:www\.)?github\.com\/([^/]+)\/([^/]+)(?:\/.*)?$/, |
Check failure
Code scanning / ESLint
Detects potentially unsafe regular expressions, which may take a very long time to run, blocking the event loop. Error
| /(?:https?:\/\/)?(?:www\.)?github\.com\/([^/]+)\/([^/]+)\.git$/, | ||
| /git@github\.com:([^/]+)\/([^/]+)\.git$/, | ||
| /^(?:https?:\/\/)?(?:www\.)?github\.com\/([^/]+)\/([^/]+)(?:\/.*)?$/, | ||
| /^(?:https?:\/\/)?(?:www\.)?github\.com\/([^/]+)\/([^/]+)\.git$/, |
Check failure
Code scanning / ESLint
Detects potentially unsafe regular expressions, which may take a very long time to run, blocking the event loop. Error
| const allowedOrigin = origin && CONFIG.CORS_ORIGINS.includes(origin) ? origin : null; | ||
|
|
||
| if (allowedOrigin) { | ||
| res.setHeader('Access-Control-Allow-Origin', allowedOrigin); |
Check failure
Code scanning / CodeQL
CORS misconfiguration for credentials transfer High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 months ago
To fix the problem, we need to ensure that the server never sets the value of Access-Control-Allow-Origin to any user-influenced value such as "null" (the string) or an empty string, even if such values sneak into CONFIG.CORS_ORIGINS via configuration or environment variable issues. Furthermore, in the extremely unlikely case that req.headers.origin is not a string, we should handle it safely.
Step-by-step fix:
- Add additional runtime validation of the
originheader before allowing it to be used, even if it’s found in the whitelist. - Explicitly check that
originis a non-empty string and is not"null". - Only add the CORS headers if the origin passes this stricter validation.
- (Optional, best practice) Return a clear comment and log if suspicious origins are detected for auditability.
This can be implemented in api-dev-server.ts, within the block where the CORS headers are set.
| @@ -644,9 +644,15 @@ | ||
| const server = createServer(async (req: IncomingMessage, res: ServerResponse) => { | ||
| // Enhanced CORS (only allow credentialed requests if origin matches sanitized whitelist) | ||
| const origin = req.headers.origin; | ||
| // Only allow origins that are explicitly whitelisted and valid | ||
| const allowedOrigin = origin && CONFIG.CORS_ORIGINS.includes(origin) ? origin : null; | ||
|
|
||
| // Only allow origins that are explicitly whitelisted and valid (defense in depth) | ||
| const isValidOrigin = ( | ||
| typeof origin === 'string' && | ||
| origin.length > 0 && | ||
| origin !== 'null' && | ||
| CONFIG.CORS_ORIGINS.includes(origin) | ||
| ); | ||
| const allowedOrigin = isValidOrigin ? origin : null; | ||
|
|
||
| if (allowedOrigin) { | ||
| res.setHeader('Access-Control-Allow-Origin', allowedOrigin); | ||
| res.setHeader('Access-Control-Allow-Credentials', 'true'); |
| return 'github'; // default | ||
| } catch { | ||
| // Fallback to original heuristic | ||
| 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
To fix the problem, we must ensure provider-detection always uses precise checks on the actual host portion of the URL (not substring checks). In the main try-block, this is already handled with new URL(url).hostname. But in the catch block, we should attempt to extract a host in a similar way, even from non-standard (e.g., git@...) URLs.
General fix:
Replace substring matching with either (a) attempts to extract a hostname, or (b) strict regular expression matching that only matches valid host positions, not arbitrary string locations.
Detailed steps:
- In the catch block, instead of substring
.includes, repeat the regex logic used above in the catch in the try-block: extract hostname after '@' (e.g.,git@github.com:givesgithub.com), and then compare the result precisely (===). - Only if we truly cannot extract a host, return a default.
- Make no functional changes outside this error.
What to change:
- In
extractGitProvider, lines 544-547, replace substring checks with a repeat of the fallback logic: extract the provider host, and strict-comparison with allowed hostnames. - If extraction fails, return 'github' (the previous default).
Needed imports/definitions:
No new imports needed (already uses regex and string methods).
No new method definitions needed.
| @@ -541,9 +541,12 @@ | ||
| return 'github'; // default | ||
| } catch { | ||
| // Fallback to original heuristic | ||
| if (url.includes('github.com')) return 'github'; | ||
| if (url.includes('gitlab.com')) return 'gitlab'; | ||
| if (url.includes('bitbucket.org')) return 'bitbucket'; | ||
| // Attempt to extract a host as in the try-block | ||
| const match = url.match(/@([a-zA-Z0-9.\-]+)[/:]/); | ||
| const hostname = match ? match[1].toLowerCase() : ''; | ||
| if (hostname === 'github.com') return 'github'; | ||
| if (hostname === 'gitlab.com') return 'gitlab'; | ||
| if (hostname === 'bitbucket.org') return 'bitbucket'; | ||
| return 'github'; | ||
| } | ||
| } |
| } catch { | ||
| // Fallback to original heuristic | ||
| 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
We need to remove the unsafe substring matches on lines 544–546 and instead attempt to always parse the hostname portion when extracting the provider, even in the fallback path. If direct URL parsing fails, we can use a regular expression to extract the hostname as in lines 535–536, ensuring it matches exactly for known hosts (github.com, gitlab.com, bitbucket.org). Only match those hostnames in the relevant part, never as a loose substring in the string.
What to do:
- In the fallback section of
extractGitProvider, replace the substring checks with a regex (using the same technique as above) to extract the hostname from the URL. Then compare the extracted hostname to the list of accepted values using equality, not substring search. - No changes needed outside the function; no additional dependencies needed.
| @@ -540,10 +540,12 @@ | ||
| if (hostname === 'bitbucket.org') return 'bitbucket'; | ||
| return 'github'; // default | ||
| } catch { | ||
| // Fallback to original heuristic | ||
| if (url.includes('github.com')) return 'github'; | ||
| if (url.includes('gitlab.com')) return 'gitlab'; | ||
| if (url.includes('bitbucket.org')) return 'bitbucket'; | ||
| // Fallback to original heuristic using regex for hostname extraction | ||
| const fallbackMatch = url.match(/@([a-zA-Z0-9.\-]+)[/:]/); | ||
| const fallbackHostname = fallbackMatch ? fallbackMatch[1].toLowerCase() : ''; | ||
| if (fallbackHostname === 'github.com') return 'github'; | ||
| if (fallbackHostname === 'gitlab.com') return 'gitlab'; | ||
| if (fallbackHostname === 'bitbucket.org') return 'bitbucket'; | ||
| return 'github'; | ||
| } | ||
| } |
| // Fallback to original heuristic | ||
| 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 issue, the unsafe substring checking within the fallback should be removed and replaced by a host-based parsing fallback, similar to the primary logic. We should attempt to extract the host from the URL using several robust regexes for the supported git URL formats (e.g., git@host:user/repo.git, ssh://host/user/repo.git, etc.), and check the full host string against a whitelist.
The only lines to change are the fallback part in the extractGitProvider function, specifically lines 544–547. We should instead extract the hostname using a regex, convert it to lowercase, and check if it exactly matches one of the supported hosts. This avoids substring recognition and ensures the host is compared accurately.
No new methods are strictly needed; the code in the try/catch block above already provides regexes for extracting the host from git URLs, which can be reused in the catch.
No additional dependencies are necessary — Node.js's built-in URL is sufficient, and the fallback regex does not require any external libraries.
| @@ -541,9 +541,12 @@ | ||
| return 'github'; // default | ||
| } catch { | ||
| // Fallback to original heuristic | ||
| if (url.includes('github.com')) return 'github'; | ||
| if (url.includes('gitlab.com')) return 'gitlab'; | ||
| if (url.includes('bitbucket.org')) return 'bitbucket'; | ||
| // Attempt to extract the host via regex, fall back to default. | ||
| const match = url.match(/@([a-zA-Z0-9.\-]+)[/:]/); | ||
| const hostname = match ? match[1].toLowerCase() : ''; | ||
| if (hostname === 'github.com') return 'github'; | ||
| if (hostname === 'gitlab.com') return 'gitlab'; | ||
| if (hostname === 'bitbucket.org') return 'bitbucket'; | ||
| return 'github'; | ||
| } | ||
| } |
Bugbot found 1 bugTo see it, activate your membership in the Cursor dashboard. |
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores