From c6c22626fd9e0edbfe16f11372c759388b6927ff 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: Tue, 21 Apr 2026 00:43:08 +0800
Subject: [PATCH 1/3] fix(install): warn when --allow-protocol-fallback reuses
a custom port across schemes (#786)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
When cross-protocol fallback is enabled and a dependency carries a
custom port, `_build_repo_url` passes the same `dep_ref.port` to both
the SSH and HTTPS URL builders. On servers that serve each protocol on
a different port (e.g. Bitbucket Datacenter: SSH 7999, HTTPS 7990), the
off-protocol URL is wrong.
Strict-by-default (#778) already prevents this for the default path.
The bug only surfaces when the user opts into fallback via
`--allow-protocol-fallback` or `APM_ALLOW_PROTOCOL_FALLBACK=1`.
Per the maintainer's Option D direction (warning + docs, no schema
change), emit a default-visibility `[!]` warning before the first clone
attempt when a custom port is set and the plan includes both SSH and
HTTPS. The message tells the user to pin the protocol with an explicit
`ssh://` or `https://` URL. The warning is routed through the existing
`_rich_warning` idiom and deduped per (host, repo, port) on the
downloader instance, so a single install run with multiple internal
`_clone_with_fallback` calls (ref resolution + actual clone) still
emits exactly one warning per dep.
Docs callouts under the transport-selection and authentication guides
cover the limitation and the pin-the-protocol workaround; the
`--allow-protocol-fallback` help text picks up a one-line caveat.
No schema changes (`DependencyReference` / `LockedDependency` untouched,
no `ssh_port` / `https_port` / `fallback:` fields). The existing
`test_https_attempt_preserves_same_port_across_protocols` invariant is
unchanged — the warning is what makes that behaviour loud.
---
CHANGELOG.md | 4 +
.../docs/getting-started/authentication.md | 10 +
docs/src/content/docs/guides/dependencies.md | 10 +
src/apm_cli/commands/install.py | 2 +-
src/apm_cli/deps/github_downloader.py | 31 +++
tests/unit/test_protocol_fallback_warning.py | 244 ++++++++++++++++++
6 files changed, 300 insertions(+), 1 deletion(-)
create mode 100644 tests/unit/test_protocol_fallback_warning.py
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1307da034..40f044610 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Strict-by-default transport selection: explicit `ssh://`/`https://` URLs no longer silently fall back to the other protocol; shorthand consults `git config url..insteadOf` and otherwise defaults to HTTPS. Set `APM_ALLOW_PROTOCOL_FALLBACK=1` (or pass `--allow-protocol-fallback`) to restore the legacy permissive chain; cross-protocol retries then emit a `[!]` warning. Closes #328 (#778)
+### Changed
+
+- `apm install --allow-protocol-fallback` now emits a `[!]` warning before the first clone attempt when a dependency has a custom port and both SSH and HTTPS attempts are planned, explaining that fallback reuses the same port across schemes and suggesting a pinned `ssh://` or `https://` URL for servers that use different ports per protocol (e.g. Bitbucket Datacenter: SSH 7999, HTTPS 7990). Docs note the limitation in the transport-selection and authentication guides. Closes #786
+
### Added
- `apm install --ssh` / `--https` flags and `APM_GIT_PROTOCOL=ssh|https` env to pick the initial transport for shorthand dependencies (#778)
diff --git a/docs/src/content/docs/getting-started/authentication.md b/docs/src/content/docs/getting-started/authentication.md
index c852a2d32..79db170b3 100644
--- a/docs/src/content/docs/getting-started/authentication.md
+++ b/docs/src/content/docs/getting-started/authentication.md
@@ -318,3 +318,13 @@ URL scheme honored exactly; shorthand uses HTTPS unless
selection matrix, the `--ssh` / `--https` flags, the `APM_GIT_PROTOCOL`
env var, and the `--allow-protocol-fallback` escape hatch, see
[Dependencies: Transport selection](../../guides/dependencies/#transport-selection-ssh-vs-https).
+
+:::caution[Custom ports and cross-protocol fallback]
+When `--allow-protocol-fallback` is in effect, APM reuses the
+dependency's port on both SSH and HTTPS attempts. On servers that use
+different ports per protocol (e.g. Bitbucket Datacenter: SSH 7999,
+HTTPS 7990), the off-protocol URL will be wrong. APM emits a `[!]`
+warning before the first clone attempt to flag this. Pin the protocol
+with an explicit `ssh://...` or `https://...` URL in `apm.yml` to
+bypass fallback entirely.
+:::
diff --git a/docs/src/content/docs/guides/dependencies.md b/docs/src/content/docs/guides/dependencies.md
index 1a5532d6b..acbaf4700 100644
--- a/docs/src/content/docs/guides/dependencies.md
+++ b/docs/src/content/docs/guides/dependencies.md
@@ -484,6 +484,16 @@ When fallback runs, each cross-protocol retry emits a `[!]` warning naming
both protocols. Use this to unblock a pipeline while you fix the root
cause -- not as a long-term setting.
+:::caution[Cross-protocol fallback reuses the same port]
+Fallback reuses `dep_ref.port` for both schemes. On servers that use
+different ports per protocol (e.g. Bitbucket Datacenter: SSH 7999,
+HTTPS 7990), the off-protocol URL will be wrong. APM emits a `[!]`
+warning before the first clone attempt when a custom port is set and
+fallback is enabled. The fix is to pin the protocol with an explicit
+`ssh://...` or `https://...` URL in `apm.yml`, which bypasses the
+fallback chain entirely.
+:::
+
For SSH key selection (ssh-agent, `~/.ssh/config`) and HTTPS token
resolution, see
[Authentication](../../getting-started/authentication/#choosing-transport-ssh-vs-https).
diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py
index 4d6f6808e..aefd5cdec 100644
--- a/src/apm_cli/commands/install.py
+++ b/src/apm_cli/commands/install.py
@@ -415,7 +415,7 @@ def _validate_and_add_packages_to_apm_yml(packages, dry_run=False, dev=False, lo
"allow_protocol_fallback",
is_flag=True,
default=False,
- help="Restore the legacy permissive cross-protocol fallback chain (escape hatch for migrating users; also: APM_ALLOW_PROTOCOL_FALLBACK=1).",
+ help="Restore the legacy permissive cross-protocol fallback chain (escape hatch for migrating users; also: APM_ALLOW_PROTOCOL_FALLBACK=1). Caveat: fallback reuses the same port across schemes, so pin the protocol with an explicit ssh:// or https:// URL on servers that use different SSH and HTTPS ports.",
)
@click.pass_context
def install(ctx, packages, runtime, exclude, only, update, dry_run, force, verbose, trust_transitive_mcp, parallel_downloads, dev, target, global_, use_ssh, use_https, allow_protocol_fallback):
diff --git a/src/apm_cli/deps/github_downloader.py b/src/apm_cli/deps/github_downloader.py
index 3c4e6d762..78fb27128 100644
--- a/src/apm_cli/deps/github_downloader.py
+++ b/src/apm_cli/deps/github_downloader.py
@@ -199,6 +199,11 @@ def __init__(
self._allow_fallback = (
allow_fallback if allow_fallback is not None else is_fallback_allowed()
)
+ # Dedup set for the issue #786 cross-protocol port warning: one install
+ # run calls _clone_with_fallback multiple times per dep (ref-resolution
+ # clone, then the actual dep clone). We want the warning exactly once
+ # per (host, repo, port) identity across all those calls.
+ self._fallback_port_warned: set = set()
def _setup_git_environment(self) -> Dict[str, Any]:
"""Set up Git environment with authentication using centralized token manager.
@@ -739,6 +744,32 @@ def _env_for(use_token_attempt: bool) -> Dict[str, str]:
f"strict={plan.strict}, attempts={[(a.scheme, a.use_token, a.label) for a in plan.attempts]}"
)
+ # Cross-protocol fallback reuses dep_ref.port for every attempt. On
+ # servers that serve SSH and HTTPS on different ports (e.g. Bitbucket
+ # Datacenter: SSH 7999, HTTPS 7990), the off-protocol URL will be
+ # wrong. Warn once per dep, before the first attempt, so the user can
+ # pin the protocol with an explicit ssh:// or https:// URL. See #786.
+ # A single install may call this method multiple times for the same
+ # dep (ref resolution + actual clone), so dedup on (host, repo, port).
+ dep_port = getattr(dep_ref, "port", None) if dep_ref else None
+ if (
+ not plan.strict
+ and dep_port is not None
+ and any(a.scheme == "ssh" for a in plan.attempts)
+ and any(a.scheme == "https" for a in plan.attempts)
+ ):
+ warn_key = (dep_host, repo_url_base, dep_port)
+ if warn_key not in self._fallback_port_warned:
+ self._fallback_port_warned.add(warn_key)
+ _rich_warning(
+ f"Custom port {dep_port} is set on this dependency. "
+ f"Cross-protocol fallback will reuse the same port for the "
+ f"other scheme. If your server uses different ports per "
+ f"protocol (e.g., Bitbucket Datacenter: SSH 7999, HTTPS 7990), "
+ f"pin the protocol with an explicit ssh:// or https:// URL.",
+ symbol="warning",
+ )
+
prev_label: Optional[str] = None
prev_scheme: Optional[str] = None
for attempt in plan.attempts:
diff --git a/tests/unit/test_protocol_fallback_warning.py b/tests/unit/test_protocol_fallback_warning.py
new file mode 100644
index 000000000..ffbcde793
--- /dev/null
+++ b/tests/unit/test_protocol_fallback_warning.py
@@ -0,0 +1,244 @@
+"""Unit tests for the cross-protocol-fallback port warning (issue #786).
+
+When the user opts into cross-protocol fallback via
+``--allow-protocol-fallback`` / ``APM_ALLOW_PROTOCOL_FALLBACK=1`` AND a
+dependency carries a custom port AND the plan attempts both SSH and
+HTTPS, APM emits a ``[!]`` warning before the first clone attempt. The
+same port is reused across schemes, which is incorrect for servers that
+serve SSH and HTTPS on different ports (e.g. Bitbucket Datacenter: SSH
+7999, HTTPS 7990). The warning directs the user to pin the protocol
+with an explicit ``ssh://`` or ``https://`` URL.
+"""
+
+import os
+import tempfile
+from pathlib import Path
+from unittest.mock import Mock, patch
+
+import pytest
+from git.exc import GitCommandError
+
+from apm_cli.deps.github_downloader import GitHubPackageDownloader
+from apm_cli.models.apm_package import DependencyReference
+
+
+def _make_downloader():
+ with patch.dict(os.environ, {}, clear=True), patch(
+ "apm_cli.core.token_manager.GitHubTokenManager.resolve_credential_from_git",
+ return_value=None,
+ ):
+ return GitHubPackageDownloader()
+
+
+def _run_clone_capture_warnings(dep, allow_fallback=False):
+ """Run _clone_with_fallback and return every ``_rich_warning`` call.
+
+ Returns a list of (message, symbol) tuples, one per call. Clones
+ always fail so any chained attempts run.
+ """
+ dl = _make_downloader()
+ dl.auth_resolver._cache.clear()
+ if allow_fallback:
+ dl._allow_fallback = True
+
+ def _fake_clone(url, *a, **kw):
+ raise GitCommandError("clone", "failed")
+
+ captured = []
+
+ def _capture(message, symbol=None):
+ captured.append((message, symbol))
+
+ 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, patch(
+ "apm_cli.deps.github_downloader._rich_warning", side_effect=_capture
+ ):
+ 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, GitCommandError):
+ pass
+ finally:
+ import shutil
+ shutil.rmtree(target, ignore_errors=True)
+ return captured
+
+
+class TestProtocolFallbackPortWarning:
+ """Issue #786: warn once per dep when fallback reuses a custom port."""
+
+ def _port_warnings(self, calls):
+ return [m for m, _s in calls if "Custom port" in m]
+
+ def test_warning_fires_on_ssh_url_with_port_when_fallback_allowed(self):
+ """ssh:// URL with port + allow_fallback => plan has SSH and HTTPS =>
+ exactly one ``Custom port`` warning, citing the port."""
+ dep = DependencyReference.parse(
+ "ssh://git@bitbucket.example.com:7999/project/repo.git"
+ )
+ assert dep.port == 7999
+
+ calls = _run_clone_capture_warnings(dep, allow_fallback=True)
+ port_warnings = self._port_warnings(calls)
+ assert len(port_warnings) == 1, (
+ f"expected exactly one port warning, got {len(port_warnings)}: {port_warnings!r}"
+ )
+ msg = port_warnings[0]
+ assert "7999" in msg, f"port missing from warning: {msg!r}"
+ assert "ssh://" in msg and "https://" in msg, (
+ f"warning should suggest pinning with ssh:// or https://: {msg!r}"
+ )
+ assert "Bitbucket Datacenter" in msg, (
+ f"warning should cite the Bitbucket Datacenter example: {msg!r}"
+ )
+
+ def test_warning_fires_on_https_url_with_port_when_fallback_allowed(self):
+ """https:// URL with port + allow_fallback => plan has HTTPS and SSH =>
+ warning fires."""
+ dep = DependencyReference.parse(
+ "https://git.company.internal:8443/team/repo.git"
+ )
+ assert dep.port == 8443
+
+ calls = _run_clone_capture_warnings(dep, allow_fallback=True)
+ port_warnings = self._port_warnings(calls)
+ assert len(port_warnings) == 1, (
+ f"expected exactly one port warning, got: {port_warnings!r}"
+ )
+ assert "8443" in port_warnings[0]
+
+ def test_warning_silent_in_strict_mode_even_with_custom_port(self):
+ """Strict mode (default) only plans one attempt; the warning must not
+ fire even when a custom port is set. This is the whole point of
+ strict-by-default (#778)."""
+ dep = DependencyReference.parse(
+ "ssh://git@bitbucket.example.com:7999/project/repo.git"
+ )
+ assert dep.port == 7999
+
+ calls = _run_clone_capture_warnings(dep, allow_fallback=False)
+ port_warnings = self._port_warnings(calls)
+ assert port_warnings == [], (
+ f"strict mode must not emit the port warning, got: {port_warnings!r}"
+ )
+
+ def test_warning_silent_when_no_port_set(self):
+ """Shorthand with no port => no warning, even under allow_fallback."""
+ dep = DependencyReference.parse("owner/repo")
+ assert dep.port is None
+
+ calls = _run_clone_capture_warnings(dep, allow_fallback=True)
+ port_warnings = self._port_warnings(calls)
+ assert port_warnings == [], (
+ f"no-port deps must not emit the port warning, got: {port_warnings!r}"
+ )
+
+ def test_warning_fires_once_not_per_attempt(self):
+ """Regression guard: the warning must be emitted once per dep (before
+ the attempt loop), not per clone attempt in the plan."""
+ dep = DependencyReference.parse(
+ "ssh://git@bitbucket.example.com:7999/project/repo.git"
+ )
+ calls = _run_clone_capture_warnings(dep, allow_fallback=True)
+ port_warnings = self._port_warnings(calls)
+ assert len(port_warnings) == 1, (
+ f"warning must fire exactly once per dep, got {len(port_warnings)}"
+ )
+
+ def test_warning_dedup_across_multiple_clone_calls_for_same_dep(self):
+ """Regression guard: a single install run can call
+ ``_clone_with_fallback`` multiple times for the same dep (ref-
+ resolution clone, then the actual dep clone). The warning must still
+ fire only once per (host, repo, port) across all those calls."""
+ dep = DependencyReference.parse(
+ "ssh://git@bitbucket.example.com:7999/project/repo.git"
+ )
+ dl = _make_downloader()
+ dl.auth_resolver._cache.clear()
+ dl._allow_fallback = True
+
+ def _fake_clone(url, *a, **kw):
+ raise GitCommandError("clone", "failed")
+
+ captured = []
+
+ def _capture(message, symbol=None):
+ captured.append((message, symbol))
+
+ 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, patch(
+ "apm_cli.deps.github_downloader._rich_warning", side_effect=_capture
+ ):
+ MockRepo.clone_from.side_effect = _fake_clone
+ for _ in range(3):
+ target = Path(tempfile.mkdtemp())
+ try:
+ dl._clone_with_fallback(dep.repo_url, target, dep_ref=dep)
+ except (RuntimeError, GitCommandError):
+ pass
+ finally:
+ import shutil
+ shutil.rmtree(target, ignore_errors=True)
+
+ port_warnings = [m for m, _s in captured if "Custom port" in m]
+ assert len(port_warnings) == 1, (
+ f"warning must dedup across repeated _clone_with_fallback calls "
+ f"for the same dep, got {len(port_warnings)}: {port_warnings!r}"
+ )
+
+ def test_warning_fires_again_for_different_dep(self):
+ """Dedup must be per-dep, not global: a second dep with a different
+ (host, repo, port) identity gets its own warning."""
+ dep_a = DependencyReference.parse(
+ "ssh://git@bitbucket.example.com:7999/project/repo.git"
+ )
+ dep_b = DependencyReference.parse(
+ "https://git.other.example:8443/team/repo.git"
+ )
+ dl = _make_downloader()
+ dl.auth_resolver._cache.clear()
+ dl._allow_fallback = True
+
+ def _fake_clone(url, *a, **kw):
+ raise GitCommandError("clone", "failed")
+
+ captured = []
+
+ def _capture(message, symbol=None):
+ captured.append((message, symbol))
+
+ 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, patch(
+ "apm_cli.deps.github_downloader._rich_warning", side_effect=_capture
+ ):
+ MockRepo.clone_from.side_effect = _fake_clone
+ for dep in (dep_a, dep_a, dep_b, dep_b):
+ target = Path(tempfile.mkdtemp())
+ try:
+ dl._clone_with_fallback(dep.repo_url, target, dep_ref=dep)
+ except (RuntimeError, GitCommandError):
+ pass
+ finally:
+ import shutil
+ shutil.rmtree(target, ignore_errors=True)
+
+ port_warnings = [m for m, _s in captured if "Custom port" in m]
+ assert len(port_warnings) == 2, (
+ f"expected one warning per distinct dep identity (2), "
+ f"got {len(port_warnings)}: {port_warnings!r}"
+ )
+ assert any("7999" in m for m in port_warnings), port_warnings
+ assert any("8443" in m for m in port_warnings), port_warnings
From 9a0b022534334e3ffb3dcfab7b93832b5aa735ad 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: Tue, 21 Apr 2026 12:39:59 +0800
Subject: [PATCH 2/3] fix(install): address review feedback on PR #789
Polish pass from maintainer review of the cross-protocol-fallback port
warning. No functional surface change; message content, docs accuracy,
and CLI reference sync only.
Warning text (blocker 1 -- unactionable wording):
- Name the offender: "Custom port {port} on {host}/{repo}" instead of
"this dependency" (ambiguous under N-dep installs).
- Name the planned initial + fallback schemes so the user sees which
scheme will be tried first and which one the port will be reused on.
- Offer both remediations in one line: "Pin the URL scheme, or drop
--allow-protocol-fallback to fail fast."
- Link to the public docs anchor under the dependencies guide where
the corresponding :::caution block lives.
- Add a NOTE that the (host, repo, port) dedup key is case-sensitive;
follow-up tracks case-insensitive normalization.
Reference doc sync (blocker 2 -- cli-commands.md:101 stale):
- Extend the `--allow-protocol-fallback` entry in the CLI reference to
describe the new pre-attempt port warning, not only the existing
per-retry warning. Required by the cli.instructions contract for CLI
behaviour changes.
Docs accuracy (Copilot review findings):
- dependencies.md / authentication.md cautions previously claimed an
explicit `ssh://` or `https://` URL "bypasses the fallback chain
entirely". False under `--allow-protocol-fallback`: explicit URLs
still go through the permissive chain (transport_selection.py:255).
Both cautions now state the invariant correctly -- pinning only
hard-stops cross-protocol retries in strict mode.
- Drop the internal `dep_ref.port` identifier from user docs; refer to
"the dependency's custom port" / "the dependency URL's port".
- `apm install --allow-protocol-fallback` help text picks up the same
correction (pin AND omit the flag).
CHANGELOG formatting:
- End the entry with "Closes #786 (#789)" to match the convention used
by the surrounding BREAKING and Fixed entries ("Closes #328 (#778)",
"(#780)", "(#784)") instead of the bare "Closes #786" trailer.
Tests:
- Refresh assertions to cover the new shape: host named, repo named,
both schemes named, both remediations present, docs URL present.
- Drop assertions on the removed Bitbucket-Datacenter example string
and literal "ssh://"/"https://" substrings (no longer in the terse
three-line message).
- Existing dedup / strict-mode-silent / no-port-silent / distinct-dep
coverage untouched.
Full unit suite: 4170 passed. Manual `apm install` verified the four
scenarios (port + fallback, strict + port, fallback + no-port, --help).
---
CHANGELOG.md | 2 +-
.../docs/getting-started/authentication.md | 12 +++---
docs/src/content/docs/guides/dependencies.md | 17 ++++----
.../content/docs/reference/cli-commands.md | 2 +-
src/apm_cli/commands/install.py | 2 +-
src/apm_cli/deps/github_downloader.py | 42 ++++++++++++++-----
tests/unit/test_protocol_fallback_warning.py | 36 ++++++++++++----
7 files changed, 79 insertions(+), 34 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 40f044610..707e9d251 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,7 +14,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed
-- `apm install --allow-protocol-fallback` now emits a `[!]` warning before the first clone attempt when a dependency has a custom port and both SSH and HTTPS attempts are planned, explaining that fallback reuses the same port across schemes and suggesting a pinned `ssh://` or `https://` URL for servers that use different ports per protocol (e.g. Bitbucket Datacenter: SSH 7999, HTTPS 7990). Docs note the limitation in the transport-selection and authentication guides. Closes #786
+- `apm install --allow-protocol-fallback` now emits a one-shot `[!]` warning before the first clone attempt when a dependency carries a custom port and both SSH and HTTPS attempts are planned, naming the dependency and recommending either pinning the URL scheme (with fallback disabled) or dropping the flag to fail fast. Servers like Bitbucket Datacenter that serve SSH and HTTPS on different ports can otherwise hit a silent port mismatch. Closes #786 (#789)
### Added
diff --git a/docs/src/content/docs/getting-started/authentication.md b/docs/src/content/docs/getting-started/authentication.md
index 79db170b3..74a6a5484 100644
--- a/docs/src/content/docs/getting-started/authentication.md
+++ b/docs/src/content/docs/getting-started/authentication.md
@@ -321,10 +321,12 @@ env var, and the `--allow-protocol-fallback` escape hatch, see
:::caution[Custom ports and cross-protocol fallback]
When `--allow-protocol-fallback` is in effect, APM reuses the
-dependency's port on both SSH and HTTPS attempts. On servers that use
-different ports per protocol (e.g. Bitbucket Datacenter: SSH 7999,
+dependency URL's port on both SSH and HTTPS attempts. On servers that
+use different ports per protocol (e.g. Bitbucket Datacenter: SSH 7999,
HTTPS 7990), the off-protocol URL will be wrong. APM emits a `[!]`
-warning before the first clone attempt to flag this. Pin the protocol
-with an explicit `ssh://...` or `https://...` URL in `apm.yml` to
-bypass fallback entirely.
+warning before the first clone attempt to flag this. To avoid
+cross-protocol retries, leave `--allow-protocol-fallback` disabled
+(strict mode) and pin the dependency with an explicit `ssh://...` or
+`https://...` URL. If the flag is enabled, APM may still try the
+other protocol even when the URL uses an explicit scheme.
:::
diff --git a/docs/src/content/docs/guides/dependencies.md b/docs/src/content/docs/guides/dependencies.md
index acbaf4700..6dab1c59b 100644
--- a/docs/src/content/docs/guides/dependencies.md
+++ b/docs/src/content/docs/guides/dependencies.md
@@ -485,13 +485,16 @@ both protocols. Use this to unblock a pipeline while you fix the root
cause -- not as a long-term setting.
:::caution[Cross-protocol fallback reuses the same port]
-Fallback reuses `dep_ref.port` for both schemes. On servers that use
-different ports per protocol (e.g. Bitbucket Datacenter: SSH 7999,
-HTTPS 7990), the off-protocol URL will be wrong. APM emits a `[!]`
-warning before the first clone attempt when a custom port is set and
-fallback is enabled. The fix is to pin the protocol with an explicit
-`ssh://...` or `https://...` URL in `apm.yml`, which bypasses the
-fallback chain entirely.
+Fallback reuses the dependency's custom port for both schemes. On
+servers that use different ports per protocol (e.g. Bitbucket
+Datacenter: SSH 7999, HTTPS 7990), the off-protocol URL will be
+wrong. APM emits a `[!]` warning before the first clone attempt when
+a custom port is set and fallback is enabled. To avoid cross-protocol
+retries entirely, leave `--allow-protocol-fallback` disabled (strict
+mode) and pin the dependency with an explicit `ssh://...` or
+`https://...` URL in `apm.yml`. If fallback is enabled, APM may still
+try the other protocol even when the URL uses an explicit scheme --
+pinning only hard-stops cross-protocol retries in strict mode.
:::
For SSH key selection (ssh-agent, `~/.ssh/config`) and HTTPS token
diff --git a/docs/src/content/docs/reference/cli-commands.md b/docs/src/content/docs/reference/cli-commands.md
index be886e455..b8c631a2e 100644
--- a/docs/src/content/docs/reference/cli-commands.md
+++ b/docs/src/content/docs/reference/cli-commands.md
@@ -98,7 +98,7 @@ apm install [PACKAGES...] [OPTIONS]
- `-g, --global` - Install to user scope (`~/.apm/`) instead of the current project. Primitives deploy to `~/.copilot/`, `~/.claude/`, etc.
- `--ssh` - Force SSH for shorthand (`owner/repo`) dependencies. Mutually exclusive with `--https`. Ignored for URLs with an explicit scheme.
- `--https` - Force HTTPS for shorthand dependencies. Mutually exclusive with `--ssh`. Default unless `git config url..insteadOf` rewrites the candidate to SSH.
-- `--allow-protocol-fallback` - Restore the legacy permissive cross-protocol fallback chain (HTTPS-then-SSH or vice-versa). Strict-by-default otherwise. Each retry emits a `[!]` warning naming both protocols.
+- `--allow-protocol-fallback` - Restore the legacy permissive cross-protocol fallback chain (HTTPS-then-SSH or vice-versa). Strict-by-default otherwise. Each retry emits a `[!]` warning naming both protocols. When the dependency URL carries a custom port, APM also emits a one-shot `[!]` warning before the first clone attempt noting that the same port will be reused across schemes (wrong on servers like Bitbucket Datacenter that serve SSH and HTTPS on different ports) -- to avoid the mismatch, omit this flag and pin the dependency with an explicit `ssh://` or `https://` URL.
**Transport env vars:**
diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py
index aefd5cdec..f325e20e6 100644
--- a/src/apm_cli/commands/install.py
+++ b/src/apm_cli/commands/install.py
@@ -415,7 +415,7 @@ def _validate_and_add_packages_to_apm_yml(packages, dry_run=False, dev=False, lo
"allow_protocol_fallback",
is_flag=True,
default=False,
- help="Restore the legacy permissive cross-protocol fallback chain (escape hatch for migrating users; also: APM_ALLOW_PROTOCOL_FALLBACK=1). Caveat: fallback reuses the same port across schemes, so pin the protocol with an explicit ssh:// or https:// URL on servers that use different SSH and HTTPS ports.",
+ help="Restore the legacy permissive cross-protocol fallback chain (escape hatch for migrating users; also: APM_ALLOW_PROTOCOL_FALLBACK=1). Caveat: fallback reuses the same port across schemes; on servers that use different SSH and HTTPS ports, omit this flag and pin the dependency with an explicit ssh:// or https:// URL.",
)
@click.pass_context
def install(ctx, packages, runtime, exclude, only, update, dry_run, force, verbose, trust_transitive_mcp, parallel_downloads, dev, target, global_, use_ssh, use_https, allow_protocol_fallback):
diff --git a/src/apm_cli/deps/github_downloader.py b/src/apm_cli/deps/github_downloader.py
index 78fb27128..b979fa218 100644
--- a/src/apm_cli/deps/github_downloader.py
+++ b/src/apm_cli/deps/github_downloader.py
@@ -54,6 +54,15 @@
protocol_pref_from_env,
)
+# Public docs anchor for the cross-protocol fallback caveat surfaced by the
+# #786 warning. Lives under the dependencies guide, next to the canonical
+# `--allow-protocol-fallback` section (Starlight site defined in
+# docs/astro.config.mjs).
+_PROTOCOL_FALLBACK_DOCS_URL = (
+ "https://microsoft.github.io/apm/guides/dependencies/"
+ "#restoring-the-legacy-permissive-chain"
+)
+
def normalize_collection_path(virtual_path: str) -> str:
"""Normalize a collection virtual path by stripping any existing extension.
@@ -744,11 +753,12 @@ def _env_for(use_token_attempt: bool) -> Dict[str, str]:
f"strict={plan.strict}, attempts={[(a.scheme, a.use_token, a.label) for a in plan.attempts]}"
)
- # Cross-protocol fallback reuses dep_ref.port for every attempt. On
- # servers that serve SSH and HTTPS on different ports (e.g. Bitbucket
- # Datacenter: SSH 7999, HTTPS 7990), the off-protocol URL will be
- # wrong. Warn once per dep, before the first attempt, so the user can
- # pin the protocol with an explicit ssh:// or https:// URL. See #786.
+ # Cross-protocol fallback reuses the dependency's port for every
+ # attempt. On servers that serve SSH and HTTPS on different ports
+ # (e.g. Bitbucket Datacenter: SSH 7999, HTTPS 7990), the off-protocol
+ # URL will be wrong. Warn once per dep, before the first attempt, so
+ # the user can pin the URL scheme (and leave fallback disabled) or
+ # fail fast by dropping --allow-protocol-fallback. See #786.
# A single install may call this method multiple times for the same
# dep (ref resolution + actual clone), so dedup on (host, repo, port).
dep_port = getattr(dep_ref, "port", None) if dep_ref else None
@@ -758,15 +768,27 @@ def _env_for(use_token_attempt: bool) -> Dict[str, str]:
and any(a.scheme == "ssh" for a in plan.attempts)
and any(a.scheme == "https" for a in plan.attempts)
):
+ # NOTE: dedup key is case-sensitive. GitHub/Bitbucket hostnames
+ # are case-insensitive per RFC, so "Example.com" and
+ # "example.com" dedup as distinct identities -- worst case is a
+ # duplicate warning. Follow-up issue tracks normalization.
warn_key = (dep_host, repo_url_base, dep_port)
if warn_key not in self._fallback_port_warned:
self._fallback_port_warned.add(warn_key)
+ initial_scheme = plan.attempts[0].scheme.upper()
+ fallback_scheme = next(
+ a.scheme.upper()
+ for a in plan.attempts
+ if a.scheme != plan.attempts[0].scheme
+ )
+ host_display = dep_host or "host"
_rich_warning(
- f"Custom port {dep_port} is set on this dependency. "
- f"Cross-protocol fallback will reuse the same port for the "
- f"other scheme. If your server uses different ports per "
- f"protocol (e.g., Bitbucket Datacenter: SSH 7999, HTTPS 7990), "
- f"pin the protocol with an explicit ssh:// or https:// URL.",
+ f"Custom port {dep_port} on {host_display}/{repo_url_base}: "
+ f"if {initial_scheme} fails, APM will retry over "
+ f"{fallback_scheme} on the same port.\n"
+ f" Pin the URL scheme, or drop "
+ f"--allow-protocol-fallback to fail fast.\n"
+ f" See: {_PROTOCOL_FALLBACK_DOCS_URL}",
symbol="warning",
)
diff --git a/tests/unit/test_protocol_fallback_warning.py b/tests/unit/test_protocol_fallback_warning.py
index ffbcde793..b97b39b4a 100644
--- a/tests/unit/test_protocol_fallback_warning.py
+++ b/tests/unit/test_protocol_fallback_warning.py
@@ -6,8 +6,10 @@
HTTPS, APM emits a ``[!]`` warning before the first clone attempt. The
same port is reused across schemes, which is incorrect for servers that
serve SSH and HTTPS on different ports (e.g. Bitbucket Datacenter: SSH
-7999, HTTPS 7990). The warning directs the user to pin the protocol
-with an explicit ``ssh://`` or ``https://`` URL.
+7999, HTTPS 7990). The warning names the offending dependency
+(``{host}/{repo}``), names the planned initial + fallback schemes,
+lists two remediations (pin the URL scheme, or drop
+``--allow-protocol-fallback`` to fail fast), and links to the docs.
"""
import os
@@ -77,7 +79,8 @@ def _port_warnings(self, calls):
def test_warning_fires_on_ssh_url_with_port_when_fallback_allowed(self):
"""ssh:// URL with port + allow_fallback => plan has SSH and HTTPS =>
- exactly one ``Custom port`` warning, citing the port."""
+ exactly one warning naming the offender, both schemes, both
+ remediations, and the docs URL."""
dep = DependencyReference.parse(
"ssh://git@bitbucket.example.com:7999/project/repo.git"
)
@@ -90,16 +93,25 @@ def test_warning_fires_on_ssh_url_with_port_when_fallback_allowed(self):
)
msg = port_warnings[0]
assert "7999" in msg, f"port missing from warning: {msg!r}"
- assert "ssh://" in msg and "https://" in msg, (
- f"warning should suggest pinning with ssh:// or https://: {msg!r}"
+ assert "bitbucket.example.com" in msg, f"host missing from warning: {msg!r}"
+ assert "project/repo" in msg, f"repo missing from warning: {msg!r}"
+ assert "SSH" in msg and "HTTPS" in msg, (
+ f"warning must name both planned schemes: {msg!r}"
)
- assert "Bitbucket Datacenter" in msg, (
- f"warning should cite the Bitbucket Datacenter example: {msg!r}"
+ assert "Pin the URL scheme" in msg, (
+ f"warning must offer the 'pin the URL scheme' remediation: {msg!r}"
+ )
+ assert "--allow-protocol-fallback" in msg, (
+ f"warning must offer the 'drop --allow-protocol-fallback' escape "
+ f"hatch as an alternative: {msg!r}"
+ )
+ assert "microsoft.github.io/apm/" in msg, (
+ f"warning must link to the public docs: {msg!r}"
)
def test_warning_fires_on_https_url_with_port_when_fallback_allowed(self):
"""https:// URL with port + allow_fallback => plan has HTTPS and SSH =>
- warning fires."""
+ warning fires, naming the offender and schemes."""
dep = DependencyReference.parse(
"https://git.company.internal:8443/team/repo.git"
)
@@ -110,7 +122,13 @@ def test_warning_fires_on_https_url_with_port_when_fallback_allowed(self):
assert len(port_warnings) == 1, (
f"expected exactly one port warning, got: {port_warnings!r}"
)
- assert "8443" in port_warnings[0]
+ msg = port_warnings[0]
+ assert "8443" in msg, f"port missing from warning: {msg!r}"
+ assert "git.company.internal" in msg, f"host missing from warning: {msg!r}"
+ assert "team/repo" in msg, f"repo missing from warning: {msg!r}"
+ assert "SSH" in msg and "HTTPS" in msg, (
+ f"warning must name both planned schemes: {msg!r}"
+ )
def test_warning_silent_in_strict_mode_even_with_custom_port(self):
"""Strict mode (default) only plans one attempt; the warning must not
From d032ba006ec2a1c2e5fb5a586f9ac7a5f03852cd 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: Tue, 21 Apr 2026 14:38:41 +0800
Subject: [PATCH 3/3] test(protocol-fallback-warning): tighten assertions to
anchor on emit format
CodeQL flagged the bare "bitbucket.example.com" in msg substring check
as "Incomplete URL substring sanitization" -- a false positive here
(this is a test, not URL validation), but the fix strengthens the
assertion at the same time.
Replace `assert "{host}" in msg` + `assert "{repo}" in msg` with a
single anchored check against the exact emitted prefix:
assert "Custom port {port} on {host}/{repo}:" in msg
Same treatment for the docs URL: `"microsoft.github.io/apm/" in msg`
becomes `"See: https://microsoft.github.io/apm/" in msg`, anchoring on
the `See: ` emit prefix.
Two wins: the CodeQL finding goes away (no bare hostname substring
check), and the tests now document the exact wire format so a
regression that re-splits host and repo, or changes the "Custom port
... on ..." phrasing, will fail loudly instead of silently passing via
substring containment.
---
tests/unit/test_protocol_fallback_warning.py | 22 +++++++++++++-------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/tests/unit/test_protocol_fallback_warning.py b/tests/unit/test_protocol_fallback_warning.py
index b97b39b4a..79102e19f 100644
--- a/tests/unit/test_protocol_fallback_warning.py
+++ b/tests/unit/test_protocol_fallback_warning.py
@@ -92,9 +92,14 @@ def test_warning_fires_on_ssh_url_with_port_when_fallback_allowed(self):
f"expected exactly one port warning, got {len(port_warnings)}: {port_warnings!r}"
)
msg = port_warnings[0]
- assert "7999" in msg, f"port missing from warning: {msg!r}"
- assert "bitbucket.example.com" in msg, f"host missing from warning: {msg!r}"
- assert "project/repo" in msg, f"repo missing from warning: {msg!r}"
+ # Assertions anchor on the emitted format ("Custom port ... on
+ # host/repo:" + "See: https://...") instead of bare host/URL
+ # substrings, so the test both documents the wire format and avoids
+ # CodeQL's "incomplete URL substring sanitization" false positive.
+ assert "Custom port 7999 on bitbucket.example.com/project/repo:" in msg, (
+ f"warning must name the offender in 'Custom port {{port}} on "
+ f"{{host}}/{{repo}}:' form: {msg!r}"
+ )
assert "SSH" in msg and "HTTPS" in msg, (
f"warning must name both planned schemes: {msg!r}"
)
@@ -105,8 +110,8 @@ def test_warning_fires_on_ssh_url_with_port_when_fallback_allowed(self):
f"warning must offer the 'drop --allow-protocol-fallback' escape "
f"hatch as an alternative: {msg!r}"
)
- assert "microsoft.github.io/apm/" in msg, (
- f"warning must link to the public docs: {msg!r}"
+ assert "See: https://microsoft.github.io/apm/" in msg, (
+ f"warning must link to the public docs via the 'See: ' prefix: {msg!r}"
)
def test_warning_fires_on_https_url_with_port_when_fallback_allowed(self):
@@ -123,9 +128,10 @@ def test_warning_fires_on_https_url_with_port_when_fallback_allowed(self):
f"expected exactly one port warning, got: {port_warnings!r}"
)
msg = port_warnings[0]
- assert "8443" in msg, f"port missing from warning: {msg!r}"
- assert "git.company.internal" in msg, f"host missing from warning: {msg!r}"
- assert "team/repo" in msg, f"repo missing from warning: {msg!r}"
+ assert "Custom port 8443 on git.company.internal/team/repo:" in msg, (
+ f"warning must name the offender in 'Custom port {{port}} on "
+ f"{{host}}/{{repo}}:' form: {msg!r}"
+ )
assert "SSH" in msg and "HTTPS" in msg, (
f"warning must name both planned schemes: {msg!r}"
)