From 94d05180ec7223bb6f322464c226b8dfee4cf0ee 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: Fri, 10 Apr 2026 23:43:31 +0800 Subject: [PATCH 1/3] fix: preserve ssh URLs instead of forcing https fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #661. When a user specifies an explicit ssh:// URL in apm.yml (e.g. ssh://git@bitbucket.domain.ext:7999/project/repo.git), APM was silently stripping the port during ssh:// → git@ normalisation and then falling back to https:// after the portless SSH clone attempt failed. Store the original ssh:// string in DependencyReference.original_ssh_url before normalisation, and pass it verbatim to git clone in _clone_with_fallback so the port and protocol are preserved. --- src/apm_cli/deps/github_downloader.py | 9 +++-- src/apm_cli/models/dependency/reference.py | 10 ++++++ tests/unit/test_generic_git_urls.py | 42 ++++++++++++++++++++++ 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/src/apm_cli/deps/github_downloader.py b/src/apm_cli/deps/github_downloader.py index 469ddc9c..0e8b300d 100644 --- a/src/apm_cli/deps/github_downloader.py +++ b/src/apm_cli/deps/github_downloader.py @@ -677,9 +677,14 @@ def _clone_with_fallback(self, repo_url_base: str, target_path: Path, progress_r last_error = e # Continue to next method - # Method 2: Try SSH (works with SSH keys for any host) + # Method 2: Try SSH (works with SSH keys for any host). + # When the user supplied an explicit ssh:// URL (e.g. with a custom port for + # Bitbucket Datacenter), use it verbatim so the port is not silently dropped. try: - ssh_url = self._build_repo_url(repo_url_base, use_ssh=True, dep_ref=dep_ref) + if dep_ref and dep_ref.original_ssh_url: + ssh_url = dep_ref.original_ssh_url + else: + ssh_url = self._build_repo_url(repo_url_base, use_ssh=True, dep_ref=dep_ref) repo = Repo.clone_from(ssh_url, target_path, env=clone_env, progress=progress_reporter, **clone_kwargs) if verbose_callback: verbose_callback(f"Cloned from: {ssh_url}") diff --git a/src/apm_cli/models/dependency/reference.py b/src/apm_cli/models/dependency/reference.py index 1c6df16b..39bda7a8 100644 --- a/src/apm_cli/models/dependency/reference.py +++ b/src/apm_cli/models/dependency/reference.py @@ -56,6 +56,11 @@ class DependencyReference: None # e.g., "artifactory/github" (repo key path) ) + # Preserved verbatim when the user supplied an explicit ssh:// URL in apm.yml. + # Used by the downloader to clone with the exact URL (including any custom port) + # instead of the reconstructed https:// fallback URL. + original_ssh_url: Optional[str] = None + # Supported file extensions for virtual packages VIRTUAL_FILE_EXTENSIONS = ( ".prompt.md", @@ -904,6 +909,10 @@ def parse(cls, dependency_str: str) -> "DependencyReference": ) ) + # Preserve the original ssh:// URL verbatim before normalization so the + # downloader can clone with the exact user-supplied URL (e.g. custom port). + original_ssh_url = dependency_str if dependency_str.startswith("ssh://") else None + dependency_str = cls._normalize_ssh_protocol_url(dependency_str) # Phase 1: detect virtual packages @@ -986,6 +995,7 @@ def parse(cls, dependency_str: str) -> "DependencyReference": ado_project=ado_project, ado_repo=ado_repo, artifactory_prefix=artifactory_prefix, + original_ssh_url=original_ssh_url, ) def to_github_url(self) -> str: diff --git a/tests/unit/test_generic_git_urls.py b/tests/unit/test_generic_git_urls.py index 31e35381..c2a52a1b 100644 --- a/tests/unit/test_generic_git_urls.py +++ b/tests/unit/test_generic_git_urls.py @@ -250,6 +250,48 @@ def test_git_at_url_unchanged(self): assert result == "git@gitlab.com:acme/repo.git" +class TestBitbucketDatacenterSSH: + """Regression tests for issue #661: ssh:// URLs with custom ports must be preserved. + + Bitbucket Datacenter (and other self-hosted instances) commonly use non-standard + SSH ports (e.g. 7999). When a user explicitly specifies an ssh:// URL in apm.yml + the original URL must be kept verbatim so git clones against the correct port + instead of silently falling back to HTTPS. + """ + + def test_preserve_bitbucket_datacenter_ssh_url_with_port(self): + """ssh:// URL with custom port must be stored in original_ssh_url.""" + url = "ssh://git@bitbucket.domain.ext:7999/project/repo.git" + dep = DependencyReference.parse(url) + assert dep.original_ssh_url == url + + def test_bitbucket_datacenter_host_and_repo_still_parsed(self): + """Parsed host/repo_url fields should still be populated correctly.""" + dep = DependencyReference.parse( + "ssh://git@bitbucket.domain.ext:7999/project/repo.git" + ) + assert dep.host == "bitbucket.domain.ext" + assert dep.repo_url == "project/repo" + + def test_preserve_standard_ssh_protocol_url(self): + """ssh:// without a port also stores the original URL.""" + url = "ssh://git@github.com/org/repo.git" + dep = DependencyReference.parse(url) + assert dep.original_ssh_url == url + + def test_https_url_does_not_set_original_ssh_url(self): + """HTTPS dependencies must not set original_ssh_url.""" + dep = DependencyReference.parse( + "https://bitbucket.domain.ext/scm/project/repo.git" + ) + assert dep.original_ssh_url is None + + def test_git_at_url_does_not_set_original_ssh_url(self): + """git@ SSH shorthand does not go through ssh:// normalisation.""" + dep = DependencyReference.parse("git@bitbucket.org:acme/rules.git") + assert dep.original_ssh_url is None + + class TestCloneURLBuilding: """Test that clone URLs are correctly built for generic hosts.""" From 073ef920488eaaccdb0732c29843fe29cd7884c3 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: Fri, 10 Apr 2026 23:47:34 +0800 Subject: [PATCH 2/3] test: verify _clone_with_fallback uses original ssh:// URL verbatim --- tests/unit/test_auth_scoping.py | 78 +++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/tests/unit/test_auth_scoping.py b/tests/unit/test_auth_scoping.py index e0b21526..0a2969ea 100644 --- a/tests/unit/test_auth_scoping.py +++ b/tests/unit/test_auth_scoping.py @@ -255,6 +255,84 @@ def test_generic_host_error_message_mentions_credential_helpers(self): shutil.rmtree(target, ignore_errors=True) +# =========================================================================== +# Regression: ssh:// URLs with custom ports (issue #661) +# =========================================================================== + +class TestCloneWithFallbackSshUrl: + """Verify that an explicit ssh:// URL is passed verbatim to git clone. + + Regression for #661: Bitbucket Datacenter uses custom SSH ports (e.g. + 7999). APM was stripping the port during normalisation and then falling + back to https://. The fix stores the original url in + DependencyReference.original_ssh_url and uses it in Method 2 of + _clone_with_fallback so the port is never silently dropped. + """ + + def _run_clone_capture_urls(self, dep): + """Run _clone_with_fallback and return every URL passed to clone_from.""" + mock_repo = Mock() + mock_repo.head.commit.hexsha = "abc123" + dl = _make_downloader() + dl.auth_resolver._cache.clear() + + called_urls = [] + + def _fake_clone(url, *a, **kw): + called_urls.append(url) + return mock_repo + + with patch.dict(os.environ, {}, clear=True), \ + patch( + "apm_cli.core.token_manager.GitHubTokenManager.resolve_credential_from_git", + return_value=None, + ), \ + patch('apm_cli.deps.github_downloader.Repo') as MockRepo: + MockRepo.clone_from.side_effect = _fake_clone + target = Path(tempfile.mkdtemp()) + try: + dl._clone_with_fallback(dep.repo_url, target, dep_ref=dep) + except (RuntimeError, Exception): + pass + finally: + import shutil + shutil.rmtree(target, ignore_errors=True) + return called_urls + + def test_bitbucket_datacenter_ssh_with_port_used_verbatim(self): + """The first clone attempt must use the exact ssh:// URL including port.""" + original = "ssh://git@bitbucket.domain.ext:7999/project/repo.git" + dep = _dep(original) + + assert dep.original_ssh_url == original, "original_ssh_url not stored" + + urls = self._run_clone_capture_urls(dep) + assert len(urls) >= 1 + assert urls[0] == original, ( + f"Expected first clone URL to be the original ssh:// URL, got: {urls[0]!r}" + ) + + def test_bitbucket_datacenter_ssh_no_https_attempted_first(self): + """APM must not attempt https:// before the explicit ssh:// URL.""" + original = "ssh://git@bitbucket.domain.ext:7999/project/repo.git" + dep = _dep(original) + + urls = self._run_clone_capture_urls(dep) + assert len(urls) >= 1 + assert not urls[0].startswith("https://"), ( + f"First clone attempt must not be https://, got: {urls[0]!r}" + ) + + def test_standard_ssh_url_without_port_also_preserved(self): + """ssh:// without a custom port is also used verbatim.""" + original = "ssh://git@github.com/org/repo.git" + dep = _dep(original) + + urls = self._run_clone_capture_urls(dep) + assert len(urls) >= 1 + assert urls[0] == original + + # =========================================================================== # Object-style dependency entries (parse_from_dict) # =========================================================================== From ecff19597494366e43b7185e2aa14a29ee323017 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: Sat, 11 Apr 2026 17:27:05 +0800 Subject: [PATCH 3/3] fix: sanitize original_ssh_url; strip #ref/@alias before storing Raw dependency strings can include '#ref' or '@alias' suffixes which are not valid git clone URL syntax. Strip these from original_ssh_url at capture time so the port-preserving clone path does not silently fail and re-trigger the https fallback. Also narrows the over-broad except clause in regression tests from (RuntimeError, Exception) to (RuntimeError, GitCommandError), and adds the CHANGELOG entry requested in review. --- CHANGELOG.md | 1 + src/apm_cli/models/dependency/reference.py | 20 +++++++++++++++++--- tests/unit/test_auth_scoping.py | 2 +- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cc4bcd89..f82362be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Preserve `ssh://` dependency URLs with custom ports for Bitbucket Datacenter repositories instead of silently falling back to HTTPS (#661) - Pin codex setup to `rust-v0.118.0` for security and reproducibility; update config to `wire_api = "responses"` (#663) - Propagate headers and environment variables through OpenCode MCP adapter with defensive copies to prevent mutation (#622) ### Changed diff --git a/src/apm_cli/models/dependency/reference.py b/src/apm_cli/models/dependency/reference.py index 39bda7a8..8b6a95e9 100644 --- a/src/apm_cli/models/dependency/reference.py +++ b/src/apm_cli/models/dependency/reference.py @@ -909,9 +909,23 @@ def parse(cls, dependency_str: str) -> "DependencyReference": ) ) - # Preserve the original ssh:// URL verbatim before normalization so the - # downloader can clone with the exact user-supplied URL (e.g. custom port). - original_ssh_url = dependency_str if dependency_str.startswith("ssh://") else None + # Preserve the original ssh:// URL before normalization so the downloader can + # clone with the exact user-supplied URL (e.g. custom port for Bitbucket DC). + # Strip #ref and @alias suffixes — git clone does not accept these; the ref + # is already passed separately via clone_kwargs. + if dependency_str.startswith("ssh://"): + _clone_url = dependency_str.strip() + if "#" in _clone_url: + _clone_url = _clone_url.split("#")[0] + # @alias appears only in the path portion (after scheme://user@host:port/). + # Split on the first three slashes to isolate the path, then strip trailing @alias. + _parts = _clone_url.split("/", 3) + if len(_parts) == 4 and "@" in _parts[3]: + _parts[3] = _parts[3].rsplit("@", 1)[0] + _clone_url = "/".join(_parts) + original_ssh_url = _clone_url + else: + original_ssh_url = None dependency_str = cls._normalize_ssh_protocol_url(dependency_str) diff --git a/tests/unit/test_auth_scoping.py b/tests/unit/test_auth_scoping.py index 0a2969ea..8b5e4c13 100644 --- a/tests/unit/test_auth_scoping.py +++ b/tests/unit/test_auth_scoping.py @@ -292,7 +292,7 @@ def _fake_clone(url, *a, **kw): target = Path(tempfile.mkdtemp()) try: dl._clone_with_fallback(dep.repo_url, target, dep_ref=dep) - except (RuntimeError, Exception): + except (RuntimeError, GitCommandError): pass finally: import shutil