-
Notifications
You must be signed in to change notification settings - Fork 61
fix: reject path traversal in SSH URL parsing #458
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -623,6 +623,15 @@ def _parse_ssh_url(dependency_str: str): | |
| repo_part = repo_part[:-4] | ||
|
|
||
| repo_url = repo_part.strip() | ||
|
|
||
| # Reject path traversal sequences in SSH URLs | ||
| for segment in repo_url.split('/'): | ||
| if not segment or segment in ('.', '..'): | ||
| raise PathTraversalError( | ||
| f"Invalid SSH repository path '{repo_url}': " | ||
| f"path segments must not be '.' or '..'" | ||
| ) | ||
|
Comment on lines
+659
to
+665
|
||
|
|
||
| return host, repo_url, reference, alias | ||
|
|
||
| @classmethod | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -177,6 +177,39 @@ def test_parse_accepts_normal_virtual_package(self): | |
| dep = DependencyReference.parse("owner/repo/prompts/my-file.prompt.md") | ||
| assert dep.is_virtual is True | ||
|
|
||
| # --- SSH URL traversal rejection --- | ||
|
|
||
| def test_ssh_parse_rejects_dotdot_in_repo(self): | ||
| """SSH URLs with '..' traversal in the repo path must be rejected.""" | ||
| with pytest.raises(PathTraversalError): | ||
| DependencyReference.parse("git@github.com:owner/../evil") | ||
|
|
||
| def test_ssh_parse_rejects_nested_dotdot(self): | ||
| with pytest.raises(PathTraversalError): | ||
| DependencyReference.parse("git@github.com:org/../../etc/passwd") | ||
|
|
||
| def test_ssh_parse_rejects_single_dot(self): | ||
| with pytest.raises(PathTraversalError): | ||
| DependencyReference.parse("git@github.com:owner/./repo") | ||
|
|
||
| def test_ssh_parse_accepts_normal_url(self): | ||
| dep = DependencyReference.parse("git@github.com:owner/repo#main") | ||
| assert dep.repo_url == "owner/repo" | ||
| assert dep.reference == "main" | ||
|
|
||
| def test_ssh_parse_accepts_url_with_git_suffix(self): | ||
| dep = DependencyReference.parse("git@gitlab.com:team/project.git#v1.0") | ||
| assert dep.repo_url == "team/project" | ||
| assert dep.reference == "v1.0" | ||
|
|
||
| def test_ssh_parse_rejects_dotdot_with_alias(self): | ||
| with pytest.raises(PathTraversalError): | ||
| DependencyReference.parse("git@github.com:owner/../evil@my-alias") | ||
|
|
||
| def test_ssh_parse_rejects_dotdot_with_reference(self): | ||
| with pytest.raises(PathTraversalError): | ||
| DependencyReference.parse("git@github.com:owner/../../etc#main") | ||
|
|
||
|
Comment on lines
+180
to
+212
|
||
|
|
||
| # --------------------------------------------------------------------------- | ||
| # DependencyReference.get_install_path containment | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The traversal check rejects empty path segments (e.g., double slashes), but the error message only mentions '.' and '..'. Update the message (or the condition) so users get an accurate reason (empty segments vs traversal).