feat: enhance repository name validation and expose validation function#280
feat: enhance repository name validation and expose validation function#280
Conversation
|
|
Deploying repomix with
|
| Latest commit: |
76d625e
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://59ea84ba.repomix.pages.dev |
| Branch Preview URL: | https://fix-remote-url.repomix.pages.dev |
📝 WalkthroughWalkthroughThe pull request introduces a new validation mechanism for repository URLs in the remote action functionality. A new function Changes
Sequence DiagramsequenceDiagram
participant User
participant RemoteAction
participant URLValidator
participant GitClone
User->>RemoteAction: Provide repository URL
RemoteAction->>URLValidator: Validate URL
alt URL is valid
URLValidator-->>RemoteAction: Validation successful
RemoteAction->>GitClone: Proceed with cloning
else URL is invalid
URLValidator-->>RemoteAction: Throw RepomixError
RemoteAction-->>User: Display error message
end
Possibly related PRs
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/core/file/gitCommand.ts (1)
32-36: Consider removing redundant URL validation.This URL validation is redundant since
isValidRemoteValueis already called inrunRemoteActionbefore reaching this point. Consider removing this try-catch block to avoid duplicate validation.- // Check if the URL is valid - try { - new URL(url); - } catch (error) { - throw new RepomixError(`Invalid repository URL. Please provide a valid URL. url: ${url}`); - }tests/cli/actions/remoteAction.test.ts (1)
133-139: Enhance URL test coverage.Consider adding more test cases for URLs with:
- Query parameters (e.g.,
https://example.com/repo?ref=main)- URL fragments (e.g.,
https://example.com/repo#readme)- Port numbers (e.g.,
https://localhost:8080/repo)- Special characters in path (e.g.,
https://example.com/org/repo%20name)const validUrls = [ 'https://example.com', 'http://localhost', 'https://github.com/user/repo', 'https://gitlab.com/user/repo', 'https://domain.com/path/to/something', + 'https://example.com/repo?ref=main', + 'https://example.com/repo#readme', + 'https://localhost:8080/repo', + 'https://example.com/org/repo%20name', ];src/cli/actions/remoteAction.ts (2)
24-26: Enhance error message with validation rules.The error message could be more helpful by including the expected format rules.
- throw new RepomixError('Invalid repository URL or user/repo format'); + throw new RepomixError( + 'Invalid repository URL or user/repo format. Expected format: ' + + 'owner/repo (e.g., user/project) or a valid Git repository URL' + );
121-122: Consider more restrictive repository name validation.The current regex pattern allows some characters that GitHub might not support. Consider using GitHub's official repository name restrictions.
- const namePattern = '[a-zA-Z0-9](?:[a-zA-Z0-9._-]*[a-zA-Z0-9])?'; + // GitHub username: 1-39 characters, alphanumeric or single hyphens, cannot begin/end with hyphen + const usernamePattern = '[a-zA-Z0-9](?:[a-zA-Z0-9]|-(?=[a-zA-Z0-9])){0,38}'; + // GitHub repository: similar rules but allows dots + const repoPattern = '[a-zA-Z0-9](?:[a-zA-Z0-9]|-(?=[a-zA-Z0-9])|[._](?=[a-zA-Z0-9])){0,99}'; + const shortFormRegex = new RegExp(`^${usernamePattern}/${repoPattern}$`);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/cli/actions/remoteAction.ts(2 hunks)src/core/file/gitCommand.ts(2 hunks)src/index.ts(1 hunks)tests/cli/actions/remoteAction.test.ts(2 hunks)tests/core/file/gitCommand.test.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- tests/core/file/gitCommand.test.ts
- src/index.ts
⏰ Context from checks skipped due to timeout of 90000ms (15)
- GitHub Check: Test (macos-latest, 23.x)
- GitHub Check: Test (macos-latest, 21.x)
- GitHub Check: Test (macos-latest, 19.x)
- GitHub Check: Test (windows-latest, 23.x)
- GitHub Check: Test (windows-latest, 22.x)
- GitHub Check: Test (windows-latest, 21.x)
- GitHub Check: Test (windows-latest, 20.x)
- GitHub Check: Test (windows-latest, 18.x)
- GitHub Check: Test (ubuntu-latest, 23.x)
- GitHub Check: Test (ubuntu-latest, 22.x)
- GitHub Check: Test (ubuntu-latest, 21.x)
- GitHub Check: Test (ubuntu-latest, 20.x)
- GitHub Check: Test (ubuntu-latest, 19.x)
- GitHub Check: Lint Renovate config
- GitHub Check: Cloudflare Pages
| // Check the direct form of the GitHub URL. e.g. https://github.com/yamadashy/repomix or https://gist.github.com/yamadashy/1234567890abcdef | ||
| try { | ||
| new URL(remoteValue); | ||
| return true; | ||
| } catch (error) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve URL validation for Git repositories.
The current URL validation accepts any valid URL. Consider adding specific checks for Git repository URLs.
- // Check the direct form of the GitHub URL. e.g. https://github.com/yamadashy/repomix or https://gist.github.com/yamadashy/1234567890abcdef
try {
- new URL(remoteValue);
- return true;
+ const url = new URL(remoteValue);
+ // Check if the URL is a valid Git repository URL
+ const isGitUrl = /^https?:\/\/([^/]+\/){2}/.test(url.toString()) || // Has at least two path segments
+ /^git@[^:]+:.+\/.+$/.test(remoteValue); // SSH format
+ return isGitUrl;
} catch (error) {
return false;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Check the direct form of the GitHub URL. e.g. https://github.com/yamadashy/repomix or https://gist.github.com/yamadashy/1234567890abcdef | |
| try { | |
| new URL(remoteValue); | |
| return true; | |
| } catch (error) { | |
| return false; | |
| } | |
| try { | |
| const url = new URL(remoteValue); | |
| // Check if the URL is a valid Git repository URL | |
| const isGitUrl = /^https?:\/\/([^/]+\/){2}/.test(url.toString()) || // Has at least two path segments | |
| /^git@[^:]+:.+\/.+$/.test(remoteValue); // SSH format | |
| return isGitUrl; | |
| } catch (error) { | |
| return false; | |
| } |
e7e5064 to
76d625e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/cli/actions/remoteAction.ts (1)
127-133: 🛠️ Refactor suggestionImprove URL validation for Git repositories.
The current URL validation accepts any valid URL. Consider adding specific checks for Git repository URLs.
- // Check the direct form of the GitHub URL. e.g. https://github.com/yamadashy/repomix or https://gist.github.com/yamadashy/1234567890abcdef try { - new URL(remoteValue); - return true; + const url = new URL(remoteValue); + // Check if the URL is a valid Git repository URL + return /^https?:\/\/(?:github\.com|gitlab\.com|bitbucket\.org)\/[^/]+\/[^/]+(?:\.git)?$/.test(url.toString()) || + /^git@(?:github\.com|gitlab\.com|bitbucket\.org):.+\/.+(?:\.git)?$/.test(remoteValue); } catch (error) { return false; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/cli/actions/remoteAction.ts(2 hunks)src/core/file/gitCommand.ts(2 hunks)src/index.ts(1 hunks)tests/cli/actions/remoteAction.test.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/core/file/gitCommand.ts
- src/index.ts
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: Test (windows-latest, 23.x)
- GitHub Check: Build and run (windows-latest, 23.x)
- GitHub Check: Test (windows-latest, 22.x)
- GitHub Check: Build and run (windows-latest, 22.x)
- GitHub Check: Test (windows-latest, 21.x)
- GitHub Check: Build and run (windows-latest, 21.x)
- GitHub Check: Test (windows-latest, 20.x)
- GitHub Check: Test (windows-latest, 19.x)
- GitHub Check: Test (windows-latest, 18.x)
- GitHub Check: Test (windows-latest, 18.0.0)
- GitHub Check: Lint Renovate config
- GitHub Check: Cloudflare Pages
🔇 Additional comments (3)
src/cli/actions/remoteAction.ts (2)
24-26: LGTM! Early validation is well implemented.Good placement of validation before any operations, with a clear error message using the appropriate error type.
120-125: LGTM! Robust shorthand format validation.The regex pattern correctly implements GitHub's repository naming rules, ensuring names:
- Start and end with alphanumeric characters
- Can contain dots, hyphens, and underscores in between
tests/cli/actions/remoteAction.test.ts (1)
86-128: LGTM! Comprehensive test coverage for shorthand format.Excellent test coverage with thorough validation of:
- Various valid patterns including edge cases
- Invalid patterns and disallowed characters
- Clear test descriptions and helpful error messages
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #280 +/- ##
==========================================
+ Coverage 92.00% 92.12% +0.11%
==========================================
Files 44 44
Lines 2140 2146 +6
Branches 467 469 +2
==========================================
+ Hits 1969 1977 +8
+ Misses 171 169 -2 ☔ View full report in Codecov by Sentry. |
Improve repository URL validation logic and expose it for website use. Update validation patterns to strictly follow GitHub's repository name rules.
related: #278
Checklist
npm run testnpm run lint