Skip to content

Commit 860c608

Browse files
authored
fix: detect port-like segment in SCP shorthand and suggest ssh:// form (#787)
* fix: detect port-like segment in SCP shorthand and suggest ssh:// form (#784) When a user writes `git@host:7999/project/repo.git` (SCP shorthand intending port 7999), the parser silently treats `7999/project/repo` as the repo path. The subsequent git clone fails with a 404 — no actionable error from APM. Detect when the first path segment after `:` in SCP shorthand is a valid TCP port number (1-65535) and raise a ValueError with a suggested ssh:// URL that preserves the port correctly. * fix: preserve #ref and @alias in suggested ssh:// URL; add CHANGELOG entry Address review feedback: - Thread reference and alias into the suggested ssh:// URL so users don't have to re-add them manually after switching from SCP form. - Add CHANGELOG entry under [Unreleased] -> Fixed for #784. - Add test for combined #ref@alias preservation. * fix: treat empty remaining path as no-path case; use pytest.raises style Address Copilot review: - git@host:7999/ (trailing slash, empty remaining path) now falls into the "no repository path follows" branch instead of building a suggestion ending with '/'. - Switch test_suggestion_omits_git_suffix_when_absent from try/except to pytest.raises for consistency with the rest of the file.
1 parent 891be0e commit 860c608

3 files changed

Lines changed: 147 additions & 1 deletion

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2727

2828
- `apm install` no longer silently drops skills, agents, and commands when a Claude Code plugin also ships `hooks/*.json`. The package-type detection cascade now classifies plugin-shaped packages as `MARKETPLACE_PLUGIN` (which already maps hooks via the plugin synthesizer) before falling back to the hook-only classification, and emits a default-visibility `[!]` warning when a hook-only classification disagrees with the package's directory contents (#780)
2929
- Preserve custom git ports across protocols: non-default ports on `ssh://` and `https://` dependency URLs (e.g. Bitbucket Datacenter on SSH port 7999, self-hosted GitLab on HTTPS port 8443) are now captured as a first-class `port` field on `DependencyReference` and threaded through all clone URL builders. When the SSH clone fails, the HTTPS fallback reuses the same port instead of silently dropping it (#661, #731)
30+
- Detect port-like first path segment in SCP shorthand (`git@host:7999/path`) and raise an actionable error suggesting the `ssh://` URL form, instead of silently misparsing the port as part of the repository path (#784)
3031

3132
## [0.8.12] - 2026-04-19
3233

src/apm_cli/models/dependency/reference.py

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -665,11 +665,41 @@ def _parse_ssh_url(dependency_str: str):
665665
else:
666666
repo_part = ssh_repo_part
667667

668-
if repo_part.endswith(".git"):
668+
had_git_suffix = repo_part.endswith(".git")
669+
if had_git_suffix:
669670
repo_part = repo_part[:-4]
670671

671672
repo_url = repo_part.strip()
672673

674+
# SCP syntax (git@host:path) uses ':' as the path separator, so it
675+
# cannot carry a port. Detect when the first segment is a valid TCP
676+
# port number (1-65535) and raise an actionable error instead of
677+
# silently misparsing the port as part of the repo path.
678+
segments = repo_url.split("/", 1)
679+
first_segment = segments[0]
680+
if re.fullmatch(r"[0-9]+", first_segment):
681+
port_candidate = int(first_segment)
682+
if 1 <= port_candidate <= 65535:
683+
remaining_path = segments[1] if len(segments) > 1 else ""
684+
if remaining_path:
685+
git_suffix = ".git" if had_git_suffix else ""
686+
ref_suffix = f"#{reference}" if reference else ""
687+
alias_suffix = f"@{alias}" if alias else ""
688+
suggested = f"ssh://git@{host}:{port_candidate}/{remaining_path}{git_suffix}{ref_suffix}{alias_suffix}"
689+
raise ValueError(
690+
f"It looks like '{first_segment}' in 'git@{host}:{repo_url}' "
691+
f"is a port number, but SCP-style URLs (git@host:path) cannot "
692+
f"carry a port. Use the ssh:// URL form instead:\n"
693+
f" {suggested}"
694+
)
695+
else:
696+
raise ValueError(
697+
f"It looks like '{first_segment}' in 'git@{host}:{first_segment}' "
698+
f"is a port number, but no repository path follows it. "
699+
f"SCP-style URLs (git@host:path) cannot carry a port. "
700+
f"Use the ssh:// URL form: ssh://git@{host}:{port_candidate}/<owner>/<repo>.git"
701+
)
702+
673703
# Security: reject traversal sequences in SSH repo paths
674704
validate_path_segments(
675705
repo_url, context="SSH repository path", reject_empty=True

tests/unit/test_generic_git_urls.py

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -781,3 +781,118 @@ def test_https_nested_group_with_virtual_ext_rejected(self):
781781
"""HTTPS URLs can't embed virtual paths even with nested groups."""
782782
with pytest.raises(ValueError, match="virtual file extension"):
783783
DependencyReference.parse("https://gitlab.com/group/subgroup/file.prompt.md")
784+
785+
786+
class TestSCPPortDetection:
787+
"""Detect port-like first path segment in SCP shorthand (git@host:port/path).
788+
789+
SCP shorthand uses ':' as the path separator and cannot carry a port.
790+
When the first path segment is a valid TCP port (1-65535), APM should
791+
raise a ValueError with an actionable suggestion to use ssh:// instead.
792+
"""
793+
794+
def test_scp_with_port_7999_raises(self):
795+
"""Bitbucket Datacenter: git@host:7999/project/repo.git."""
796+
with pytest.raises(ValueError, match="ssh://"):
797+
DependencyReference.parse("git@bitbucket.example.com:7999/project/repo.git")
798+
799+
def test_scp_with_port_22_raises(self):
800+
"""Default SSH port 22 should still be detected."""
801+
with pytest.raises(ValueError, match="ssh://"):
802+
DependencyReference.parse("git@host.example.com:22/owner/repo.git")
803+
804+
def test_scp_with_port_65535_raises(self):
805+
"""Max valid TCP port should trigger detection."""
806+
with pytest.raises(ValueError, match="ssh://"):
807+
DependencyReference.parse("git@host.example.com:65535/owner/repo.git")
808+
809+
def test_scp_with_port_1_raises(self):
810+
"""Min valid TCP port should trigger detection."""
811+
with pytest.raises(ValueError, match="ssh://"):
812+
DependencyReference.parse("git@host.example.com:1/owner/repo.git")
813+
814+
def test_scp_with_leading_zeros_raises(self):
815+
"""Leading zeros: 007999 -> int 7999, still a valid port."""
816+
with pytest.raises(ValueError, match="ssh://"):
817+
DependencyReference.parse("git@host.example.com:007999/project/repo.git")
818+
819+
def test_scp_port_only_no_path_raises(self):
820+
"""git@host:7999 with no repo path after the port."""
821+
with pytest.raises(ValueError, match="no repository path follows"):
822+
DependencyReference.parse("git@host.example.com:7999")
823+
824+
def test_scp_port_trailing_slash_no_path_raises(self):
825+
"""git@host:7999/ — trailing slash but empty remaining path."""
826+
with pytest.raises(ValueError, match="no repository path follows"):
827+
DependencyReference.parse("git@host.example.com:7999/")
828+
829+
def test_scp_port_with_ref_raises_and_preserves_ref(self):
830+
"""Port-like segment with #ref should be caught; suggestion preserves the ref."""
831+
with pytest.raises(
832+
ValueError,
833+
match=r"ssh://git@host\.example\.com:7999/project/repo\.git#main",
834+
):
835+
DependencyReference.parse("git@host.example.com:7999/project/repo.git#main")
836+
837+
def test_scp_port_with_alias_raises_and_preserves_alias(self):
838+
"""Port-like segment with @alias should be caught; suggestion preserves the alias."""
839+
with pytest.raises(
840+
ValueError,
841+
match=r"ssh://git@host\.example\.com:7999/project/repo\.git@my-alias",
842+
):
843+
DependencyReference.parse("git@host.example.com:7999/project/repo.git@my-alias")
844+
845+
def test_scp_port_with_ref_and_alias_preserves_both(self):
846+
"""Suggestion should include both #ref and @alias when present."""
847+
with pytest.raises(
848+
ValueError,
849+
match=r"ssh://git@host\.example\.com:7999/project/repo\.git#v1\.0@my-alias",
850+
):
851+
DependencyReference.parse("git@host.example.com:7999/project/repo.git#v1.0@my-alias")
852+
853+
def test_suggestion_includes_git_suffix(self):
854+
"""When the user wrote .git, the suggestion should preserve it."""
855+
with pytest.raises(
856+
ValueError,
857+
match=r"ssh://git@host\.example\.com:7999/project/repo\.git",
858+
):
859+
DependencyReference.parse("git@host.example.com:7999/project/repo.git")
860+
861+
def test_suggestion_omits_git_suffix_when_absent(self):
862+
"""When the user omitted .git, the suggestion should not add it."""
863+
with pytest.raises(ValueError) as excinfo:
864+
DependencyReference.parse("git@host.example.com:7999/project/repo")
865+
msg = str(excinfo.value)
866+
assert "ssh://git@host.example.com:7999/project/repo" in msg
867+
assert not msg.endswith(".git")
868+
869+
def test_port_zero_not_detected(self):
870+
"""Port 0 is invalid -- should NOT trigger port detection, parses as org name."""
871+
dep = DependencyReference.parse("git@host.example.com:0/repo")
872+
assert dep.repo_url == "0/repo"
873+
assert dep.port is None
874+
875+
def test_port_out_of_range_not_detected(self):
876+
"""99999 > 65535 -- not a valid port, should NOT trigger port detection."""
877+
dep = DependencyReference.parse("git@host.example.com:99999/repo")
878+
assert dep.repo_url == "99999/repo"
879+
assert dep.port is None
880+
881+
def test_normal_org_name_not_detected(self):
882+
"""Non-numeric org name should parse normally."""
883+
dep = DependencyReference.parse("git@gitlab.com:acme/repo.git")
884+
assert dep.repo_url == "acme/repo"
885+
assert dep.port is None
886+
887+
def test_alphanumeric_first_segment_not_detected(self):
888+
"""'v2' is not purely numeric -- should parse normally."""
889+
dep = DependencyReference.parse("git@gitlab.com:v2/repo.git")
890+
assert dep.repo_url == "v2/repo"
891+
assert dep.port is None
892+
893+
def test_ssh_protocol_with_port_still_works(self):
894+
"""ssh:// URL form with port must continue working (regression guard)."""
895+
dep = DependencyReference.parse("ssh://git@bitbucket.example.com:7999/project/repo.git")
896+
assert dep.host == "bitbucket.example.com"
897+
assert dep.port == 7999
898+
assert dep.repo_url == "project/repo"

0 commit comments

Comments
 (0)