Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/apm_cli/models/dependency/reference.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 '..'"
Copy link

Copilot AI Mar 25, 2026

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).

Suggested change
f"path segments must not be '.' or '..'"
f"path segments must not be empty or be '.' or '..'"

Copilot uses AI. Check for mistakes.
)
Comment on lines +659 to +665
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change tightens accepted SSH repo paths (rejecting '.'/'..' and empty segments). Please update the Starlight docs that describe dependency URL grammar/patterns (e.g., the repository path segment pattern in the dependency guide / manifest schema) to explicitly state that '.' and '..' segments are invalid, so the docs match the parser behavior.

Copilot generated this review using guidance from repository custom instructions.

return host, repo_url, reference, alias

@classmethod
Expand Down
33 changes: 33 additions & 0 deletions tests/unit/test_path_security.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link

Copilot AI Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new SSH traversal guard explicitly rejects empty segments (e.g., "git@github.com:owner//repo"), but the added tests only cover '.' and '..'. Add a test that double slashes (or leading/trailing slash) in the SSH repo path raises PathTraversalError so this behavior is locked in.

Copilot generated this review using guidance from repository custom instructions.

# ---------------------------------------------------------------------------
# DependencyReference.get_install_path containment
Expand Down
Loading