Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.<base>.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 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

- `apm install --ssh` / `--https` flags and `APM_GIT_PROTOCOL=ssh|https` env to pick the initial transport for shorthand dependencies (#778)
Expand Down
12 changes: 12 additions & 0 deletions docs/src/content/docs/getting-started/authentication.md
Original file line number Diff line number Diff line change
Expand Up @@ -331,3 +331,15 @@ 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 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. 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.
:::
13 changes: 13 additions & 0 deletions docs/src/content/docs/guides/dependencies.md
Original file line number Diff line number Diff line change
Expand Up @@ -484,6 +484,19 @@ 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 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
resolution, see
[Authentication](../../getting-started/authentication/#choosing-transport-ssh-vs-https).
Expand Down
2 changes: 1 addition & 1 deletion docs/src/content/docs/reference/cli-commands.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. MCP servers are only installed for global-capable runtimes (Copilot CLI, Codex CLI); workspace-only runtimes are skipped.
- `--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.<base>.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:**

Expand Down
2 changes: 1 addition & 1 deletion src/apm_cli/commands/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,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; 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):
Expand Down
53 changes: 53 additions & 0 deletions src/apm_cli/deps/github_downloader.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -199,6 +208,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()
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

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

self._fallback_port_warned is used as a set of (host, repo, port) tuples, but it is annotated as a plain set, which loses useful type information. Consider annotating it as a typed set (e.g., set[tuple[Optional[str], str, int]]) to improve readability and static checking.

This issue also appears on line 764 of the same file.

Suggested change
self._fallback_port_warned: set = set()
self._fallback_port_warned: set[tuple[Optional[str], str, int]] = set()

Copilot uses AI. Check for mistakes.

def _setup_git_environment(self) -> Dict[str, Any]:
"""Set up Git environment with authentication using centralized token manager.
Expand Down Expand Up @@ -739,6 +753,45 @@ 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 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
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)
):
# 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} 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",
)

prev_label: Optional[str] = None
prev_scheme: Optional[str] = None
for attempt in plan.attempts:
Expand Down
Loading
Loading