Add URL validation for organization, repository, and enterprise arguments #1485
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…resh2003/gh-gei into 1180-fix-url-validation
…resh2003/gh-gei into 1180-fix-url-validation
…tory, and enterprise name arguments and remove trailing blank lines from test files
Added validation for URL arguments and improved error messages for path options.
…o 1180-fix-url-validation # Conflicts: # RELEASENOTES.md
…ory, and enterprise arguments
Unit Test Results 1 files 1 suites 10m 25s ⏱️ Results for commit 94d4c0a. ♻️ This comment has been updated with latest results. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds validation to detect when users accidentally provide URLs instead of names for organization, repository, and enterprise arguments across multiple CLI commands. The implementation uses .NET's built-in Uri.TryCreate() method for robust URL detection and provides clear, actionable error messages.
Key Changes:
- Added
IsUrl()extension method toStringExtensionsthat validates http/https URLs using .NET's standard URI parsing - Implemented validation checks in 14 CommandArgs classes across gei, ado2gh, and shared commands
- Added 47 comprehensive unit tests covering all validation scenarios
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/Octoshift/Extensions/StringExtensions.cs |
Added IsUrl() extension method using Uri.TryCreate() for URL detection |
src/gei/Commands/MigrateSecretAlerts/MigrateSecretAlertsCommandArgs.cs |
Added URL validation for source/target org and repo parameters |
src/gei/Commands/MigrateRepo/MigrateRepoCommandArgs.cs |
Added URL validation for github-source-org, github-target-org, source-repo, and target-repo |
src/gei/Commands/MigrateOrg/MigrateOrgCommandArgs.cs |
Added URL validation for github-source-org, github-target-org, and github-target-enterprise |
src/gei/Commands/MigrateCodeScanningAlerts/MigrateCodeScanningAlertsCommandArgs.cs |
Added URL validation for source/target org and repo parameters |
src/gei/Commands/GenerateScript/GenerateScriptCommandArgs.cs |
Added URL validation for github-source-org and github-target-org |
src/ado2gh/Commands/MigrateRepo/MigrateRepoCommandArgs.cs |
Added URL validation for github-org and github-repo parameters |
src/ado2gh/Commands/IntegrateBoards/IntegrateBoardsCommandArgs.cs |
Added URL validation for github-org and github-repo parameters |
src/Octoshift/Commands/RevokeMigratorRole/RevokeMigratorRoleCommandArgs.cs |
Added URL validation for github-org parameter |
src/Octoshift/Commands/ReclaimMannequin/ReclaimMannequinCommandArgs.cs |
Added URL validation for github-org parameter |
src/Octoshift/Commands/GrantMigratorRole/GrantMigratorRoleCommandArgs.cs |
Added URL validation for github-org parameter |
src/Octoshift/Commands/GenerateMannequinCsv/GenerateMannequinCsvCommandArgs.cs |
Added URL validation for github-org parameter |
src/Octoshift/Commands/DownloadLogs/DownloadLogsCommandArgs.cs |
Added URL validation for github-org and github-repo parameters |
src/Octoshift/Commands/CreateTeam/CreateTeamCommandArgs.cs |
Added URL validation for github-org parameter |
src/OctoshiftCLI.Tests/StringExtensionsTests.cs |
Added 14 test cases covering URL detection edge cases |
src/OctoshiftCLI.Tests/gei/Commands/MigrateSecretAlerts/MigrateSecretAlertsCommandArgsTests.cs |
Added 4 test cases for URL validation in MigrateSecretAlerts command |
src/OctoshiftCLI.Tests/gei/Commands/MigrateRepo/MigrateRepoCommandArgsTests.cs |
Added 4 test cases for URL validation in MigrateRepo command |
src/OctoshiftCLI.Tests/gei/Commands/MigrateOrg/MigrateOrgCommandArgsTests.cs |
Added 3 test cases for URL validation in MigrateOrg command |
src/OctoshiftCLI.Tests/gei/Commands/MigrateCodeScanningAlerts/MigrateCodeScanningAlertsCommandArgsTests.cs |
Added 4 test cases for URL validation in MigrateCodeScanningAlerts command |
src/OctoshiftCLI.Tests/gei/Commands/GenerateScript/GenerateScriptCommandArgsTests.cs |
Added 2 test cases for URL validation in GenerateScript command |
src/OctoshiftCLI.Tests/ado2gh/Commands/MigrateRepo/MigrateRepoCommandArgsTests.cs |
Created new test file with 3 test cases for ado2gh MigrateRepo validation |
src/OctoshiftCLI.Tests/ado2gh/Commands/IntegrateBoards/IntegrateBoardsCommandArgsTests.cs |
Created new test file with 3 test cases for IntegrateBoards validation |
src/OctoshiftCLI.Tests/Octoshift/Commands/RevokeMigratorRole/RevokeMigratorRoleCommandArgsTests.cs |
Added 1 test case for URL validation in RevokeMigratorRole command |
src/OctoshiftCLI.Tests/Octoshift/Commands/ReclaimMannequin/ReclaimMannequinCommandArgsTests.cs |
Added 1 test case for URL validation in ReclaimMannequin command |
src/OctoshiftCLI.Tests/Octoshift/Commands/GrantMigratorRole/GrantMigratorRoleCommandArgsTests.cs |
Added 1 test case for URL validation in GrantMigratorRole command |
src/OctoshiftCLI.Tests/Octoshift/Commands/GenerateMannequinCsv/GenerateMannequinCsvCommandArgsTests.cs |
Created new test file with 2 test cases for GenerateMannequinCsv validation |
src/OctoshiftCLI.Tests/Octoshift/Commands/DownloadLogs/DownloadLogsCommandArgsTests.cs |
Created new test file with 3 test cases for DownloadLogs validation |
src/OctoshiftCLI.Tests/Octoshift/Commands/CreateTeam/CreateTeamCommandArgsTests.cs |
Created new test file with 2 test cases for CreateTeam validation |
RELEASENOTES.md |
Added user-friendly release note describing the new validation feature |
| { | ||
| if (GithubOrg.IsUrl()) | ||
| { | ||
| throw new OctoshiftCliException("The --github-org option expects an organization name, not a URL. Please provide just the organization name (e.g., 'my-org' instead of 'https://github.com/my-org')."); |
There was a problem hiding this comment.
The error message should use string interpolation ($"...") for consistency with similar error messages throughout the codebase. Even though no variables are currently being interpolated, using string interpolation maintains consistency with other validation error messages in this PR.
Suggested change
| throw new OctoshiftCliException("The --github-org option expects an organization name, not a URL. Please provide just the organization name (e.g., 'my-org' instead of 'https://github.com/my-org')."); | |
| throw new OctoshiftCliException($"The --github-org option expects an organization name, not a URL. Please provide just the organization name (e.g., 'my-org' instead of 'https://github.com/my-org')."); |
Comment on lines
+21
to
+26
| throw new OctoshiftCliException("The --github-org option expects an organization name, not a URL. Please provide just the organization name (e.g., 'my-org' instead of 'https://github.com/my-org')."); | ||
| } | ||
|
|
||
| if (GithubRepo.IsUrl()) | ||
| { | ||
| throw new OctoshiftCliException("The --github-repo option expects a repository name, not a URL. Please provide just the repository name (e.g., 'my-repo' instead of 'https://github.com/my-org/my-repo')."); |
There was a problem hiding this comment.
The error messages should use string interpolation ($"...") for consistency with similar error messages throughout the codebase. Even though no variables are currently being interpolated, using string interpolation maintains consistency with other validation error messages in this PR.
Suggested change
| throw new OctoshiftCliException("The --github-org option expects an organization name, not a URL. Please provide just the organization name (e.g., 'my-org' instead of 'https://github.com/my-org')."); | |
| } | |
| if (GithubRepo.IsUrl()) | |
| { | |
| throw new OctoshiftCliException("The --github-repo option expects a repository name, not a URL. Please provide just the repository name (e.g., 'my-repo' instead of 'https://github.com/my-org/my-repo')."); | |
| throw new OctoshiftCliException($"The --github-org option expects an organization name, not a URL. Please provide just the organization name (e.g., 'my-org' instead of 'https://github.com/my-org')."); | |
| } | |
| if (GithubRepo.IsUrl()) | |
| { | |
| throw new OctoshiftCliException($"The --github-repo option expects a repository name, not a URL. Please provide just the repository name (e.g., 'my-repo' instead of 'https://github.com/my-org/my-repo')."); |
| { | ||
| if (GithubOrg.IsUrl()) | ||
| { | ||
| throw new OctoshiftCliException("The --github-org option expects an organization name, not a URL. Please provide just the organization name (e.g., 'my-org' instead of 'https://github.com/my-org')."); |
There was a problem hiding this comment.
The error message should use string interpolation ($"...") for consistency with similar error messages throughout the codebase. Even though no variables are currently being interpolated, using string interpolation maintains consistency with other validation error messages in this PR.
Suggested change
| throw new OctoshiftCliException("The --github-org option expects an organization name, not a URL. Please provide just the organization name (e.g., 'my-org' instead of 'https://github.com/my-org')."); | |
| throw new OctoshiftCliException($"The --github-org option expects an organization name, not a URL. Please provide just the organization name (e.g., 'my-org' instead of 'https://github.com/my-org')."); |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
External PR from @AakashSuresh2003 (#1464) to add validation that detects when users accidentally provide URLs instead of names for org/repo/enterprise arguments.
This addresses issue #1180 by providing clear, user-friendly error messages when URL patterns are detected.
Changes:
IsUrl()extension method using .NET'sUri.TryCreate()for robust URL detectionKey features:
✅ Uses standard .NET
Uri.TryCreate()validation (not custom pattern matching)✅ Only detects proper http/https URLs (no false positives)
✅ Clear error messages explaining expected format
✅ 47 new test cases covering all affected commands
Fixes #1180
All 1019 tests passing ✅