fix: marketplace install auth host on *.ghe.com (closes #1285)

src/apm_cli/marketplace/resolver.py

@@ -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``.
@@ -180,6 +183,37 @@ 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.
+
+    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()
+
+
 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]
@@ -546,6 +580,20 @@ 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}"
+
     # ---- 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.

tests/unit/marketplace/test_marketplace_resolver.py

@@ -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."""