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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Override `Path.home()` itself in the root test conftest so the 46 remaining Windows `RuntimeError: Could not determine home directory` failures on xdist worker `gw2` cannot recur regardless of which conftest the worker imports first; per-test `monkeypatch.setenv("HOME", ...)` continues to work because the override consults env vars before falling back to the hermetic tmp dir. (#1272)
- Retry the `apm mcp search` and `apm mcp show` integration tests on the documented "Could not reach MCP registry" transient (with backoff and a final skip) so a brief `api.mcp.github.com` outage no longer red-marks the Windows integration job. (#1274)
- Also wrap `Path.expanduser()` in the root test conftest so the `windows-2025-vs2026` runner cannot raise `RuntimeError("Could not determine home directory.")` from `ntpath.expanduser` when production code (e.g. `install.package_resolution.user_scope_rejection_reason`) calls `Path("~/pkg").expanduser()`. Falls back to the hermetic tmp dir; assertions about `~/pkg` being absolute still hold. (#1276)
- `apm install` from marketplaces registered on `*.ghe.com` (GHE Cloud) hosts now routes auth at the registered enterprise host instead of silently defaulting to `github.com` and failing with 401; the marketplace resolver backfills the enterprise host onto the canonical so downstream `DependencyReference.parse` recovers it, and the resulting `apm.yml` entry records the correct enterprise `git:` URL instead of `https://github.com/...`. (#1292)

## [0.13.0] - 2026-05-11

Expand Down
63 changes: 62 additions & 1 deletion src/apm_cli/marketplace/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@
:class:`~apm_cli.models.dependency.reference.DependencyReference` built like explicit
``git:`` + ``path:``; clone target
is only the registered marketplace project; the plugin directory is ``virtual_path``.
``github.com`` / ``*.ghe.com`` keep shorthand (no structured ref). :func:`resolve_marketplace_plugin` returns
``github.com`` and ``*.ghe.com`` keep shorthand (no structured ref); ``*.ghe.com``
canonicals additionally carry a host prefix so downstream auth resolves at the
enterprise host instead of falling back to ``github.com`` (#1285).
:func:`resolve_marketplace_plugin` returns
:class:`MarketplacePluginResolution`, which iterates as ``(canonical, plugin)`` so
existing ``canonical, plugin = resolve_marketplace_plugin(...)`` call sites keep
working; consumers that need the structured ref use ``result.dependency_reference``.
Expand Down Expand Up @@ -180,6 +183,44 @@ def _marketplace_host_needs_explicit_git_path(host: str) -> bool:
return not is_github_hostname(h)


def _needs_canonical_host_prefix(canonical: str, host: str) -> bool:
"""True when a GitHub-family enterprise host must be prefixed to ``canonical``.

GitHub-family hosts (``github.com`` + ``*.ghe.com``) keep virtual shorthand --
``resolve_plugin_source`` emits a bare ``owner/repo[/path]`` canonical because
there is no nested-group ambiguity to disambiguate. ``DependencyReference.parse``
defaults missing hosts to ``github.com``, which is correct for ``github.com`` but
silently mis-routes auth for every ``*.ghe.com`` marketplace.

Returns True only for enterprise GitHub hosts (``*.ghe.com``) so the caller can
backfill the host while preserving shorthand semantics. Idempotent: when the
canonical already starts with ``host`` (case-insensitive) -- as happens when the
manifest's dict source carries a host-qualified ``repo`` -- this returns False
so the prefix is not duplicated.

GHES (GitHub Enterprise Server, configured via ``GITHUB_HOST``) is not handled
here. Those hosts return True from ``_marketplace_host_needs_explicit_git_path``
(neither GitHub-family nor ADO) so ``resolve_marketplace_plugin`` builds a
structured ``dep_ref`` upstream and this helper is never reached. The
``is_github_hostname`` check below is defense-in-depth that would also reject
them if a future change ever bypassed the upstream guard.

Also returns False when ``canonical`` is in URL form (``https://...``) or SSH
SCP shorthand (``git@host:owner/repo``). Manifests that put a full URL in the
``repo`` field reach this point via ``_resolve_github_source`` (which only
requires a ``/``); detecting those by ``":"`` in the first slash-split segment
avoids producing malformed ``host/https://...`` canonicals. Those forms already
carry a host and ``DependencyReference.parse`` resolves them natively.
"""
h = (host or "").strip()
if not h or not is_github_hostname(h) or h.lower() == "github.com":
return False
first_segment = canonical.split("/", 1)[0]
if ":" in first_segment:
return False
return first_segment.lower() != h.lower()
Comment thread
edenfunf marked this conversation as resolved.


def _marketplace_https_git_url(source: MarketplaceSource) -> str:
"""HTTPS clone URL for the registered marketplace project (same project as ``marketplace.json``)."""
segments = [p for p in f"{source.owner}/{source.repo}".split("/") if p]
Expand Down Expand Up @@ -546,6 +587,26 @@ def _emit_warning(msg: str) -> None:
)
canonical = dep_ref.to_canonical()

# ---- Backfill host on canonical for GitHub-family enterprise hosts ----
# ``*.ghe.com`` marketplaces keep virtual shorthand (no structured ``dep_ref``)
# because there is no nested-group ambiguity to disambiguate, but the bare
# canonical drops the host that ``DependencyReference.parse`` needs to route auth
# at the enterprise host instead of falling back to ``github.com``. Backfill the
# host so the canonical self-routes, scoped to in-marketplace sources where the
# host is unambiguously the registered marketplace host (#1285).
if (
dep_ref is None
and _is_in_marketplace_source(plugin, source)
and _needs_canonical_host_prefix(canonical, source.host)
):
canonical = f"{source.host}/{canonical}"
logger.debug(
"Backfilled marketplace host '%s' onto canonical for %s@%s (auth routing #1285)",
source.host,
plugin_name,
marketplace_name,
)

# ---- Raw ref override ----
# When version_spec is provided it is treated as a raw git ref that
# overrides whatever ref came from the marketplace source field.
Expand Down
225 changes: 225 additions & 0 deletions tests/unit/marketplace/test_marketplace_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -794,6 +794,231 @@ def test_kind_key_github_dict_in_marketplace_gets_structured_ref(
assert dep.repo_url == "epm-ease/ai-apm-registry"


class TestResolveMarketplacePluginGHECloud:
"""GHE Cloud (``*.ghe.com``) marketplaces must carry host in canonical (issue #1285).

GitHub-family hosts keep virtual shorthand (no ``dependency_reference``) because
they have no GitLab-style nested-group ambiguity. ``github.com`` is the
``DependencyReference.parse`` default so a bare ``owner/repo`` canonical
self-routes; ``*.ghe.com`` is not, so canonical must carry the host forward or
downstream auth lands on ``github.com`` with the wrong credentials.
"""

@pytest.fixture
def ghe_marketplace_source(self) -> MarketplaceSource:
return MarketplaceSource(
name="my-marketplace",
owner="myorg",
repo="my-marketplace",
host="corp.ghe.com",
branch="main",
)

@staticmethod
def _manifest_with_plugin(plugin: MarketplacePlugin) -> MarketplaceManifest:
return MarketplaceManifest(
name="my-marketplace",
plugins=(plugin,),
plugin_root="",
)

@patch("apm_cli.marketplace.resolver.fetch_or_cache")
@patch("apm_cli.marketplace.resolver.get_marketplace_by_name")
def test_relative_source_carries_host_in_canonical(
self, mock_get, mock_fetch, ghe_marketplace_source
):
plugin = MarketplacePlugin(name="my-plugin", source="./plugins/my-plugin")
mock_get.return_value = ghe_marketplace_source
mock_fetch.return_value = self._manifest_with_plugin(plugin)

result = resolve_marketplace_plugin("my-plugin", "my-marketplace")

assert result.canonical == "corp.ghe.com/myorg/my-marketplace/plugins/my-plugin"
# GHE keeps shorthand semantics -- no structured dep_ref, only canonical
assert result.dependency_reference is None
# The whole point: parse must recover the GHE host instead of defaulting to github.com
dep = DependencyReference.parse(result.canonical)
assert dep.host == "corp.ghe.com"
assert dep.repo_url == "myorg/my-marketplace"
assert dep.virtual_path == "plugins/my-plugin"

@patch("apm_cli.marketplace.resolver.fetch_or_cache")
@patch("apm_cli.marketplace.resolver.get_marketplace_by_name")
def test_dict_github_source_bare_repo_carries_host(
self, mock_get, mock_fetch, ghe_marketplace_source
):
"""Dict ``github`` source whose bare ``repo`` matches the marketplace project."""
plugin = MarketplacePlugin(
name="dict-bare",
source={
"type": "github",
"repo": "myorg/my-marketplace",
"path": "plugins/my-plugin",
},
)
mock_get.return_value = ghe_marketplace_source
mock_fetch.return_value = self._manifest_with_plugin(plugin)

result = resolve_marketplace_plugin("dict-bare", "my-marketplace")
assert result.canonical == "corp.ghe.com/myorg/my-marketplace/plugins/my-plugin"

@patch("apm_cli.marketplace.resolver.fetch_or_cache")
@patch("apm_cli.marketplace.resolver.get_marketplace_by_name")
def test_dict_github_source_host_qualified_repo_not_double_prefixed(
self, mock_get, mock_fetch, ghe_marketplace_source
):
"""Idempotent: dict source already carrying the host in ``repo`` keeps a single prefix."""
plugin = MarketplacePlugin(
name="dict-qualified",
source={
"type": "github",
"repo": "corp.ghe.com/myorg/my-marketplace",
"path": "plugins/my-plugin",
},
)
mock_get.return_value = ghe_marketplace_source
mock_fetch.return_value = self._manifest_with_plugin(plugin)

result = resolve_marketplace_plugin("dict-qualified", "my-marketplace")
assert result.canonical == "corp.ghe.com/myorg/my-marketplace/plugins/my-plugin"

@patch("apm_cli.marketplace.resolver.fetch_or_cache")
@patch("apm_cli.marketplace.resolver.get_marketplace_by_name")
def test_dict_github_source_mixed_case_host_qualified_not_double_prefixed(
self, mock_get, mock_fetch, ghe_marketplace_source
):
"""Case-insensitive idempotent check: manifests may write host in any case."""
plugin = MarketplacePlugin(
name="dict-mixed",
source={
"type": "github",
"repo": "Corp.GHE.com/myorg/my-marketplace",
"path": "plugins/my-plugin",
},
)
mock_get.return_value = ghe_marketplace_source
mock_fetch.return_value = self._manifest_with_plugin(plugin)

result = resolve_marketplace_plugin("dict-mixed", "my-marketplace")
# Manifest-supplied case is preserved; no second prefix is added
assert result.canonical == "Corp.GHE.com/myorg/my-marketplace/plugins/my-plugin"

@patch("apm_cli.marketplace.resolver.fetch_or_cache")
@patch("apm_cli.marketplace.resolver.get_marketplace_by_name")
def test_cross_repo_source_not_prefixed(self, mock_get, mock_fetch, ghe_marketplace_source):
"""Cross-repo dict source is out of scope; canonical is left to the existing parse default.

Bare unqualified ``repo`` pointing to a different project carries no signal that
it lives on the marketplace host; treating it as such would silently change the
host routing of every cross-repo plugin entry. Manifest authors must
host-qualify cross-host references explicitly.
"""
plugin = MarketplacePlugin(
name="cross-repo",
source={
"type": "github",
"repo": "anotherorg/anothertool",
"path": "plugins/my-plugin",
},
)
mock_get.return_value = ghe_marketplace_source
mock_fetch.return_value = self._manifest_with_plugin(plugin)

result = resolve_marketplace_plugin("cross-repo", "my-marketplace")
assert result.canonical == "anotherorg/anothertool/plugins/my-plugin"

@patch("apm_cli.marketplace.resolver.fetch_or_cache")
@patch("apm_cli.marketplace.resolver.get_marketplace_by_name")
def test_version_spec_override_preserves_host_prefix(
self, mock_get, mock_fetch, ghe_marketplace_source
):
"""``version_spec`` raw-ref override stacks correctly on a host-prefixed canonical."""
plugin = MarketplacePlugin(name="ref-overridden", source="./plugins/ref-overridden")
mock_get.return_value = ghe_marketplace_source
mock_fetch.return_value = self._manifest_with_plugin(plugin)

result = resolve_marketplace_plugin(
"ref-overridden", "my-marketplace", version_spec="release-2.0"
)
assert (
result.canonical
== "corp.ghe.com/myorg/my-marketplace/plugins/ref-overridden#release-2.0"
)
dep = DependencyReference.parse(result.canonical)
assert dep.host == "corp.ghe.com"
assert dep.reference == "release-2.0"

@patch("apm_cli.marketplace.resolver.fetch_or_cache")
@patch("apm_cli.marketplace.resolver.get_marketplace_by_name")
def test_url_form_repo_not_prefixed(self, mock_get, mock_fetch, ghe_marketplace_source):
"""Dict source whose ``repo`` is a full ``https://`` URL must not be host-prefixed.

``_resolve_github_source`` only validates ``/`` in ``repo`` so a full URL slips
through and produces a canonical that already carries a scheme + host. Prefixing
again would yield a malformed ``corp.ghe.com/https://...`` string that
``DependencyReference.parse`` rejects with ValueError.
"""
plugin = MarketplacePlugin(
name="url-form",
source={
"type": "github",
"repo": "https://corp.ghe.com/myorg/my-marketplace",
"path": "plugins/url-form",
},
)
mock_get.return_value = ghe_marketplace_source
mock_fetch.return_value = self._manifest_with_plugin(plugin)

result = resolve_marketplace_plugin("url-form", "my-marketplace")
assert result.canonical == "https://corp.ghe.com/myorg/my-marketplace/plugins/url-form"
# Downstream parse still recovers the GHE host from the URL form natively.
dep = DependencyReference.parse(result.canonical)
assert dep.host == "corp.ghe.com"

@patch("apm_cli.marketplace.resolver.fetch_or_cache")
@patch("apm_cli.marketplace.resolver.get_marketplace_by_name")
def test_ssh_form_repo_not_prefixed(self, mock_get, mock_fetch, ghe_marketplace_source):
"""Dict source whose ``repo`` is an SSH SCP shorthand must not be host-prefixed.

Same class as the URL-form regression: ``git@host:owner/repo`` carries its own
host and an SCP-style ``:`` path separator that breaks a naive ``/`` split.
"""
plugin = MarketplacePlugin(
name="ssh-form",
source={
"type": "github",
"repo": "git@corp.ghe.com:myorg/my-marketplace",
"path": "plugins/ssh-form",
},
)
mock_get.return_value = ghe_marketplace_source
mock_fetch.return_value = self._manifest_with_plugin(plugin)

result = resolve_marketplace_plugin("ssh-form", "my-marketplace")
assert result.canonical == "git@corp.ghe.com:myorg/my-marketplace/plugins/ssh-form"
dep = DependencyReference.parse(result.canonical)
assert dep.host == "corp.ghe.com"

@patch("apm_cli.marketplace.resolver.fetch_or_cache")
@patch("apm_cli.marketplace.resolver.get_marketplace_by_name")
def test_github_com_canonical_remains_bare(self, mock_get, mock_fetch):
"""Regression: github.com marketplace canonical stays bare (parse default applies)."""
gh_source = MarketplaceSource(
name="my-marketplace",
owner="myorg",
repo="my-marketplace",
host="github.com",
branch="main",
)
plugin = MarketplacePlugin(name="my-plugin", source="./plugins/my-plugin")
mock_get.return_value = gh_source
mock_fetch.return_value = self._manifest_with_plugin(plugin)

result = resolve_marketplace_plugin("my-plugin", "my-marketplace")
assert result.canonical == "myorg/my-marketplace/plugins/my-plugin"
assert result.dependency_reference is None


class TestGitLabShorthandParseVsStructuredRef:
"""``DependencyReference.parse`` on a long FQDN does not split monorepo paths on GitLab hosts."""

Expand Down
Loading