From 205a00cf500c186067e58466a7a547b9eb7c4e46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=A8=B1=E5=85=83=E8=B1=AA?= <146086744+edenfunf@users.noreply.github.com> Date: Mon, 20 Apr 2026 22:29:53 +0800 Subject: [PATCH 1/3] fix: detect port-like segment in SCP shorthand and suggest ssh:// form (#784) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- src/apm_cli/models/dependency/reference.py | 30 ++++++- tests/unit/test_generic_git_urls.py | 98 ++++++++++++++++++++++ 2 files changed, 127 insertions(+), 1 deletion(-) diff --git a/src/apm_cli/models/dependency/reference.py b/src/apm_cli/models/dependency/reference.py index bcdb1ffd..feb58f5d 100644 --- a/src/apm_cli/models/dependency/reference.py +++ b/src/apm_cli/models/dependency/reference.py @@ -665,11 +665,39 @@ def _parse_ssh_url(dependency_str: str): else: repo_part = ssh_repo_part - if repo_part.endswith(".git"): + had_git_suffix = repo_part.endswith(".git") + if had_git_suffix: repo_part = repo_part[:-4] repo_url = repo_part.strip() + # SCP syntax (git@host:path) uses ':' as the path separator, so it + # cannot carry a port. Detect when the first segment is a valid TCP + # port number (1-65535) and raise an actionable error instead of + # silently misparsing the port as part of the repo path. + segments = repo_url.split("/", 1) + first_segment = segments[0] + if re.fullmatch(r"[0-9]+", first_segment): + port_candidate = int(first_segment) + if 1 <= port_candidate <= 65535: + if len(segments) > 1: + remaining_path = segments[1] + git_suffix = ".git" if had_git_suffix else "" + suggested = f"ssh://git@{host}:{port_candidate}/{remaining_path}{git_suffix}" + raise ValueError( + f"It looks like '{first_segment}' in 'git@{host}:{repo_url}' " + f"is a port number, but SCP-style URLs (git@host:path) cannot " + f"carry a port. Use the ssh:// URL form instead:\n" + f" {suggested}" + ) + else: + raise ValueError( + f"It looks like '{first_segment}' in 'git@{host}:{first_segment}' " + f"is a port number, but no repository path follows it. " + f"SCP-style URLs (git@host:path) cannot carry a port. " + f"Use the ssh:// URL form: ssh://git@{host}:{port_candidate}//.git" + ) + # Security: reject traversal sequences in SSH repo paths validate_path_segments( repo_url, context="SSH repository path", reject_empty=True diff --git a/tests/unit/test_generic_git_urls.py b/tests/unit/test_generic_git_urls.py index 76d8acdc..efb172c5 100644 --- a/tests/unit/test_generic_git_urls.py +++ b/tests/unit/test_generic_git_urls.py @@ -781,3 +781,101 @@ def test_https_nested_group_with_virtual_ext_rejected(self): """HTTPS URLs can't embed virtual paths even with nested groups.""" with pytest.raises(ValueError, match="virtual file extension"): DependencyReference.parse("https://gitlab.com/group/subgroup/file.prompt.md") + + +class TestSCPPortDetection: + """Detect port-like first path segment in SCP shorthand (git@host:port/path). + + SCP shorthand uses ':' as the path separator and cannot carry a port. + When the first path segment is a valid TCP port (1-65535), APM should + raise a ValueError with an actionable suggestion to use ssh:// instead. + """ + + def test_scp_with_port_7999_raises(self): + """Bitbucket Datacenter: git@host:7999/project/repo.git.""" + with pytest.raises(ValueError, match="ssh://"): + DependencyReference.parse("git@bitbucket.example.com:7999/project/repo.git") + + def test_scp_with_port_22_raises(self): + """Default SSH port 22 should still be detected.""" + with pytest.raises(ValueError, match="ssh://"): + DependencyReference.parse("git@host.example.com:22/owner/repo.git") + + def test_scp_with_port_65535_raises(self): + """Max valid TCP port should trigger detection.""" + with pytest.raises(ValueError, match="ssh://"): + DependencyReference.parse("git@host.example.com:65535/owner/repo.git") + + def test_scp_with_port_1_raises(self): + """Min valid TCP port should trigger detection.""" + with pytest.raises(ValueError, match="ssh://"): + DependencyReference.parse("git@host.example.com:1/owner/repo.git") + + def test_scp_with_leading_zeros_raises(self): + """Leading zeros: 007999 -> int 7999, still a valid port.""" + with pytest.raises(ValueError, match="ssh://"): + DependencyReference.parse("git@host.example.com:007999/project/repo.git") + + def test_scp_port_only_no_path_raises(self): + """git@host:7999 with no repo path after the port.""" + with pytest.raises(ValueError, match="no repository path follows"): + DependencyReference.parse("git@host.example.com:7999") + + def test_scp_port_with_ref_raises(self): + """Port-like segment with #ref should still be caught.""" + with pytest.raises(ValueError, match="ssh://"): + DependencyReference.parse("git@host.example.com:7999/project/repo.git#main") + + def test_scp_port_with_alias_raises(self): + """Port-like segment with @alias should still be caught.""" + with pytest.raises(ValueError, match="ssh://"): + DependencyReference.parse("git@host.example.com:7999/project/repo.git@my-alias") + + def test_suggestion_includes_git_suffix(self): + """When the user wrote .git, the suggestion should preserve it.""" + with pytest.raises( + ValueError, + match=r"ssh://git@host\.example\.com:7999/project/repo\.git", + ): + DependencyReference.parse("git@host.example.com:7999/project/repo.git") + + def test_suggestion_omits_git_suffix_when_absent(self): + """When the user omitted .git, the suggestion should not add it.""" + try: + DependencyReference.parse("git@host.example.com:7999/project/repo") + assert False, "should have raised ValueError" + except ValueError as e: + msg = str(e) + assert "ssh://git@host.example.com:7999/project/repo" in msg + assert not msg.endswith(".git") + + def test_port_zero_not_detected(self): + """Port 0 is invalid -- should NOT trigger port detection, parses as org name.""" + dep = DependencyReference.parse("git@host.example.com:0/repo") + assert dep.repo_url == "0/repo" + assert dep.port is None + + def test_port_out_of_range_not_detected(self): + """99999 > 65535 -- not a valid port, should NOT trigger port detection.""" + dep = DependencyReference.parse("git@host.example.com:99999/repo") + assert dep.repo_url == "99999/repo" + assert dep.port is None + + def test_normal_org_name_not_detected(self): + """Non-numeric org name should parse normally.""" + dep = DependencyReference.parse("git@gitlab.com:acme/repo.git") + assert dep.repo_url == "acme/repo" + assert dep.port is None + + def test_alphanumeric_first_segment_not_detected(self): + """'v2' is not purely numeric -- should parse normally.""" + dep = DependencyReference.parse("git@gitlab.com:v2/repo.git") + assert dep.repo_url == "v2/repo" + assert dep.port is None + + def test_ssh_protocol_with_port_still_works(self): + """ssh:// URL form with port must continue working (regression guard).""" + dep = DependencyReference.parse("ssh://git@bitbucket.example.com:7999/project/repo.git") + assert dep.host == "bitbucket.example.com" + assert dep.port == 7999 + assert dep.repo_url == "project/repo" From ef33f3037a46716fa0ec3b5f440ddf24944bad78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=A8=B1=E5=85=83=E8=B1=AA?= <146086744+edenfunf@users.noreply.github.com> Date: Mon, 20 Apr 2026 22:49:33 +0800 Subject: [PATCH 2/3] 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. --- CHANGELOG.md | 1 + src/apm_cli/models/dependency/reference.py | 4 +++- tests/unit/test_generic_git_urls.py | 26 +++++++++++++++++----- 3 files changed, 24 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 173a4659..1307da03 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `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) - 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) +- 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) ## [0.8.12] - 2026-04-19 diff --git a/src/apm_cli/models/dependency/reference.py b/src/apm_cli/models/dependency/reference.py index feb58f5d..ec09cb66 100644 --- a/src/apm_cli/models/dependency/reference.py +++ b/src/apm_cli/models/dependency/reference.py @@ -683,7 +683,9 @@ def _parse_ssh_url(dependency_str: str): if len(segments) > 1: remaining_path = segments[1] git_suffix = ".git" if had_git_suffix else "" - suggested = f"ssh://git@{host}:{port_candidate}/{remaining_path}{git_suffix}" + ref_suffix = f"#{reference}" if reference else "" + alias_suffix = f"@{alias}" if alias else "" + suggested = f"ssh://git@{host}:{port_candidate}/{remaining_path}{git_suffix}{ref_suffix}{alias_suffix}" raise ValueError( f"It looks like '{first_segment}' in 'git@{host}:{repo_url}' " f"is a port number, but SCP-style URLs (git@host:path) cannot " diff --git a/tests/unit/test_generic_git_urls.py b/tests/unit/test_generic_git_urls.py index efb172c5..eb880323 100644 --- a/tests/unit/test_generic_git_urls.py +++ b/tests/unit/test_generic_git_urls.py @@ -821,16 +821,30 @@ def test_scp_port_only_no_path_raises(self): with pytest.raises(ValueError, match="no repository path follows"): DependencyReference.parse("git@host.example.com:7999") - def test_scp_port_with_ref_raises(self): - """Port-like segment with #ref should still be caught.""" - with pytest.raises(ValueError, match="ssh://"): + def test_scp_port_with_ref_raises_and_preserves_ref(self): + """Port-like segment with #ref should be caught; suggestion preserves the ref.""" + with pytest.raises( + ValueError, + match=r"ssh://git@host\.example\.com:7999/project/repo\.git#main", + ): DependencyReference.parse("git@host.example.com:7999/project/repo.git#main") - def test_scp_port_with_alias_raises(self): - """Port-like segment with @alias should still be caught.""" - with pytest.raises(ValueError, match="ssh://"): + def test_scp_port_with_alias_raises_and_preserves_alias(self): + """Port-like segment with @alias should be caught; suggestion preserves the alias.""" + with pytest.raises( + ValueError, + match=r"ssh://git@host\.example\.com:7999/project/repo\.git@my-alias", + ): DependencyReference.parse("git@host.example.com:7999/project/repo.git@my-alias") + def test_scp_port_with_ref_and_alias_preserves_both(self): + """Suggestion should include both #ref and @alias when present.""" + with pytest.raises( + ValueError, + match=r"ssh://git@host\.example\.com:7999/project/repo\.git#v1\.0@my-alias", + ): + DependencyReference.parse("git@host.example.com:7999/project/repo.git#v1.0@my-alias") + def test_suggestion_includes_git_suffix(self): """When the user wrote .git, the suggestion should preserve it.""" with pytest.raises( From da23da25e5a9d6e72ad8978fda058ef1596004f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E8=A8=B1=E5=85=83=E8=B1=AA?= <146086744+edenfunf@users.noreply.github.com> Date: Mon, 20 Apr 2026 22:51:41 +0800 Subject: [PATCH 3/3] 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. --- src/apm_cli/models/dependency/reference.py | 4 ++-- tests/unit/test_generic_git_urls.py | 15 +++++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/apm_cli/models/dependency/reference.py b/src/apm_cli/models/dependency/reference.py index ec09cb66..742c9452 100644 --- a/src/apm_cli/models/dependency/reference.py +++ b/src/apm_cli/models/dependency/reference.py @@ -680,8 +680,8 @@ def _parse_ssh_url(dependency_str: str): if re.fullmatch(r"[0-9]+", first_segment): port_candidate = int(first_segment) if 1 <= port_candidate <= 65535: - if len(segments) > 1: - remaining_path = segments[1] + remaining_path = segments[1] if len(segments) > 1 else "" + if remaining_path: git_suffix = ".git" if had_git_suffix else "" ref_suffix = f"#{reference}" if reference else "" alias_suffix = f"@{alias}" if alias else "" diff --git a/tests/unit/test_generic_git_urls.py b/tests/unit/test_generic_git_urls.py index eb880323..0fca7d9f 100644 --- a/tests/unit/test_generic_git_urls.py +++ b/tests/unit/test_generic_git_urls.py @@ -821,6 +821,11 @@ def test_scp_port_only_no_path_raises(self): with pytest.raises(ValueError, match="no repository path follows"): DependencyReference.parse("git@host.example.com:7999") + def test_scp_port_trailing_slash_no_path_raises(self): + """git@host:7999/ — trailing slash but empty remaining path.""" + with pytest.raises(ValueError, match="no repository path follows"): + DependencyReference.parse("git@host.example.com:7999/") + def test_scp_port_with_ref_raises_and_preserves_ref(self): """Port-like segment with #ref should be caught; suggestion preserves the ref.""" with pytest.raises( @@ -855,13 +860,11 @@ def test_suggestion_includes_git_suffix(self): def test_suggestion_omits_git_suffix_when_absent(self): """When the user omitted .git, the suggestion should not add it.""" - try: + with pytest.raises(ValueError) as excinfo: DependencyReference.parse("git@host.example.com:7999/project/repo") - assert False, "should have raised ValueError" - except ValueError as e: - msg = str(e) - assert "ssh://git@host.example.com:7999/project/repo" in msg - assert not msg.endswith(".git") + msg = str(excinfo.value) + assert "ssh://git@host.example.com:7999/project/repo" in msg + assert not msg.endswith(".git") def test_port_zero_not_detected(self): """Port 0 is invalid -- should NOT trigger port detection, parses as org name."""