review: warning-level hint, anchored test substrings, integration trap

Address PR microsoft#1319 review panel findings and the github-advanced-security
``Incomplete URL substring sanitization`` flag without changing the fix's
core design.

logger.info -> logger.warning
=============================

The PR microsoft#1292 review panel's top-five follow-up microsoft#3 (the seed for microsoft#1305)
explicitly recommended ``logger.warning`` for this exact diagnostic:

  "A single ``logger.warning`` (or structured install-time check) would
  close the UX gap... converts a silent failure into an actionable error
  and prevents repeat microsoft#1285-class support tickets."

The original PR microsoft#1319 used ``logger.info`` on the rationale that
``CommandLogger.info`` is documented for "persistent advisory context...
must survive quiet-mode suppression". The current panel's three-persona
convergence (Python Architect + CLI Logging Expert + DevX UX) is that
``info`` is still visually ambient at default verbosity -- an operator
scanning a red ``[x]`` line will not register an adjacent ``[i]`` as
the recovery action. ``warning`` renders ``[!]`` and matches the
traffic-light convention the codebase uses elsewhere. As a side benefit,
``warning`` is implemented on both ``CommandLogger`` and
``NullCommandLogger`` (``info`` is not on the latter), so the message
shape is now safe against any future caller variant.

Remove the ``Hint:`` prefix; the ``[!]`` symbol carries the advisory
signal on its own. Inline the resolved enterprise hostname into the
"registered on" clause so the test assertions can anchor on contextual
prose (e.g. ``"registered on 'corp.ghe.com'"``) instead of bare
hostname substrings -- which silences the CodeQL flag without weakening
what the assertion verifies.

Auth-expert second clause
=========================

The original hint read as if the misconfigured case was the only
explanation for a validation failure: "If you meant the enterprise host,
set the plugin's repo field to corp.ghe.com/...". A legitimate
``github.com`` cross-host dep that fails for a transient reason
(rate-limit, network, expired PAT) would read that hint and add an
enterprise host prefix that breaks a working config.

Append the auth-expert recommended second clause:

  "If this is intentionally a github.com dependency, verify your
  github.com credentials and that the repository is accessible."

Both clauses are explicitly conditional, so neither path is misdirected.
The original issue's "two intents" framing assumed validation success
vs 401; this clause covers the third path (validation failure on a
legitimate dep) that was not in the issue spec but is real.

Integration trap + new e2e test
===============================

``test_cross_repo_locks_known_silent_misroute`` in
``tests/integration/test_ghe_marketplace_install_e2e.py`` was authored
by PR microsoft#1292 specifically to give "the future microsoft#1305 fix an explicit
before/after diff to assert against". The microsoft#1305 fix deliberately
preserves the resolver-level routing (bare cross-repo -> github.com is
correct for legitimate cross-host deps) and adds a sentinel + an
install-time hint instead. Update the test's docstring to reflect this,
keep the routing-preservation assertions, and add three new sentinel
assertions so the metadata the install command consumes is locked at
the integration tier.

Add ``TestCrossRepoMisconfigHintIntegration`` with two scenarios:

- ``test_cross_repo_hint_emitted_on_validation_failure``: drives the
  real ``_resolve_package_references`` + ``InstallLogger`` through
  ``capsys`` and asserts the warning-level hint contains the
  plugin@marketplace identity, the enterprise host anchored to its
  "registered on" clause, the bare repo, the host-qualified fix value,
  and the auth-expert second clause.
- ``test_legitimate_cross_host_validation_passes_no_hint``: locks the
  no-pollution contract for the legitimate cross-host path that
  validates successfully.

This matches the convention PR microsoft#1292 established with PR microsoft#1312 (microsoft#1304
closer): panel-flagged ``outcome: missing`` integration findings on
secure-by-default surfaces should land an integration-tier trap, not
just unit coverage.

CHANGELOG
=========

Add the ``[Unreleased] Fixed`` entry naming GHE enterprise marketplace
explicitly so enterprise teams scanning the changelog for cross-repo
misconfiguration symptoms recognize the fix on upgrade. Mirrors the
PR microsoft#1292 entry style.

Out of scope
============

The supply-chain finding (cross-repo bare where the same owner/repo
exists on github.com with attacker-staged content) is a real
dependency-confusion vector but is not the diagnostic-surface problem
microsoft#1305 targets; tracked as a separate follow-up issue. The doc-writer
finding referenced ``docs/manifest-schema.md`` which does not exist in
this repository; documentation additions deferred to a focused docs PR.

Author: edenfunf

CHANGELOG.md

@@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - 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)
+- `apm install` from `*.ghe.com` (GHE Cloud) marketplaces now surfaces a warning-level hint when a cross-repo dict `type: github` plugin source with a bare `repo` field fails validation, naming the marketplace's enterprise host and the exact host-qualified `repo` value to set in `marketplace.json`; the resolver attaches a typed misconfig sentinel that the install command consults at the validation-failure boundary, so the legitimate cross-host path (validation succeeds) emits no hint and the suggestion does not pollute working configs. (#1319)
 - `apm view --help` and the `view` row in `apm --help` now render in release binaries; PyInstaller's `optimize=2` was stripping `__doc__` from every Click command, and `view` was the only command that relied on its docstring instead of the explicit `help=` kwarg every other command defensively sets. Lowered the spec to `optimize=1` so asserts are still removed but docstrings survive, restoring Click's documented help-from-docstring fallback for all current and future commands. (#1298)
 
 ## [0.13.0] - 2026-05-11

src/apm_cli/commands/install.py

@@ -540,18 +540,28 @@ def warning_handler(msg):
             # likely the silent auth mis-route (bare canonical fell back to
             # ``github.com``). Surface the host-qualify hint inline so the
             # operator can correct ``marketplace.json`` without rerunning
-            # under ``--verbose`` to decode the auth trace. The legitimate
-            # cross-host case validates successfully and never reaches here.
+            # under ``--verbose`` to decode the auth trace. ``logger.warning``
+            # is used (not ``info``) per the PR #1292 panel review's explicit
+            # guidance for this exact follow-up: a misconfiguration that
+            # voids ``apm install`` should be at warning level, not buried
+            # in info-level ambient output. The second clause acknowledges
+            # the legitimate cross-host alternative so operators whose
+            # github.com dep failed for a transient reason (rate limit,
+            # network, expired PAT) are not misdirected into adding an
+            # enterprise host prefix that would break a working config.
             _risk_entry = _misconfig_risks.get(package)
             if _risk_entry is not None and logger:
                 _mp_name, _plugin_name, _risk = _risk_entry
-                logger.info(
-                    f"Hint: '{_plugin_name}@{_mp_name}' is registered on "
-                    f"'{_risk.marketplace_host}', but the plugin's bare "
+                logger.warning(
+                    f"'{_plugin_name}@{_mp_name}' is registered on "
+                    f"'{_risk.marketplace_host}' but the plugin's bare "
                     f"`repo: {_risk.bare_repo_field}` resolved to "
                     "'github.com'. If you meant the enterprise host, set "
                     "the plugin's `repo` field to "
-                    f"'{_risk.suggested_qualified_repo}' in marketplace.json."
+                    f"'{_risk.suggested_qualified_repo}' in marketplace.json. "
+                    "If this is intentionally a github.com dependency, "
+                    "verify your github.com credentials and that the "
+                    "repository is accessible."
                 )
 
     return (

tests/integration/test_ghe_marketplace_install_e2e.py

@@ -205,14 +205,20 @@ def test_github_com_marketplace_keeps_github_default(self):
         assert ctx.host_info.kind == "github"
 
     def test_cross_repo_locks_known_silent_misroute(self):
-        """Regression trap for the cross-repo bug class tracked separately in #1305.
-
-        A ``*.ghe.com`` marketplace with a cross-repo dict source bears the same
-        symptoms as #1285 -- canonical emerges bare, parse defaults to ``github.com``.
-        This is intentionally out of scope of PR #1292; #1305 tracks the fix
-        (which belongs in the install-time error handler, not the resolver).
-        The test locks the current behaviour so the future #1305 fix has an
-        explicit before/after diff to assert against.
+        """Regression trap for the cross-repo routing semantics + #1305 sentinel.
+
+        A ``*.ghe.com`` marketplace with a cross-repo dict source bears the
+        same superficial symptoms as #1285 -- canonical emerges bare, parse
+        defaults to ``github.com``. The #1305 fix deliberately preserves
+        that resolver-level routing (a bare cross-repo ``repo`` also
+        legitimately means "a github.com open-source dep from this enterprise
+        marketplace") and instead attaches a
+        :class:`~apm_cli.marketplace.resolver.CrossRepoMisconfigRisk` sentinel
+        that the install command consults at the validation-failure boundary
+        to emit an actionable host-qualify hint. This test locks both halves:
+        the routing preservation (so the legitimate path is not regressed)
+        and the sentinel attachment (so the hint emission has the metadata
+        it needs).
         """
         plugin = MarketplacePlugin(
             name="cross-repo",
@@ -234,12 +240,146 @@ def test_cross_repo_locks_known_silent_misroute(self):
         ):
             result = resolve_marketplace_plugin("cross-repo", _REPO)
 
-        # Pre-existing behaviour: no host prefix for cross-repo (#1305 to fix).
+        # Routing preservation: cross-repo canonical stays bare; parse still
+        # falls back to ``github.com``. This is intentional -- the legitimate
+        # cross-host path validates successfully and never needs to recover
+        # the enterprise host. #1305 surfaces the diagnostic at install time
+        # when the misconfigured path subsequently fails validation.
         assert result.canonical == "anotherorg/anothertool/plugins/x"
         dep_ref = DependencyReference.parse(result.canonical)
         auth = AuthResolver()
         ctx = auth.resolve_for_dep(dep_ref)
-        assert ctx.host_info.host == "github.com", (
-            "If this assertion fails, the cross-repo silent mis-route bug from "
-            "#1305 has been fixed -- update this test to reflect the new behaviour."
+        assert ctx.host_info.host == "github.com"
+
+        # #1305: sentinel must attach so the install command's
+        # validation-fail branch has the metadata to emit the hint.
+        risk = result.cross_repo_misconfig_risk
+        assert risk is not None
+        assert risk.marketplace_host == _GHE_HOST
+        assert risk.bare_repo_field == "anotherorg/anothertool"
+        assert (
+            risk.suggested_qualified_repo
+            == f"{_GHE_HOST}/anotherorg/anothertool"
         )
+
+
+@pytest.mark.integration
+class TestCrossRepoMisconfigHintIntegration:
+    """End-to-end: the #1305 hint surfaces when a cross-repo bare entry on
+    a ``*.ghe.com`` marketplace fails validation.
+
+    Unit tests in ``tests/unit/commands/`` mock ``DependencyReference`` and
+    ``resolve_marketplace_plugin``; this integration trap walks the real
+    ``_resolve_package_references`` + real ``InstallLogger`` and asserts on
+    the actual stdout the operator would see. Required by the PR review
+    panel (test-coverage-expert: ``outcome: missing`` on a secure-by-default
+    surface) and matches the e2e-integration convention PR #1292 established
+    with ``test_ghe_marketplace_backfills_host_on_bare_canonical`` above.
+
+    Stubs at one seam only:
+
+    - ``_validate_package_exists``: forces the failure outcome that triggers
+      the hint. The real validate path makes outbound HTTP calls; this stub
+      keeps the test deterministic. Everything between the resolver sentinel
+      and the logger render is the real code path.
+    """
+
+    @pytest.fixture(autouse=True)
+    def _isolate_github_host_env(self, monkeypatch):
+        monkeypatch.delenv("GITHUB_HOST", raising=False)
+
+    def test_cross_repo_hint_emitted_on_validation_failure(self, capsys):
+        """The canonical misconfiguration scenario from #1305 surfaces a
+        warning-level hint identifying the marketplace host and the exact
+        host-qualified ``repo`` value to use as a fix."""
+        from apm_cli.commands.install import _resolve_package_references
+        from apm_cli.core.command_logger import InstallLogger
+
+        plugin = MarketplacePlugin(
+            name="shared-tool",
+            source={
+                "type": "github",
+                "repo": "platform-team/shared-tool",
+                "path": "plugins/shared",
+            },
+        )
+
+        with (
+            patch(
+                "apm_cli.marketplace.resolver.get_marketplace_by_name",
+                return_value=_make_source(_GHE_HOST),
+            ),
+            patch(
+                "apm_cli.marketplace.resolver.fetch_or_cache",
+                return_value=_make_manifest(plugin),
+            ),
+            patch(
+                "apm_cli.commands.install._validate_package_exists",
+                return_value=False,
+            ),
+        ):
+            _resolve_package_references(
+                ["shared-tool@my-marketplace"],
+                [],
+                set(),
+                logger=InstallLogger(verbose=False),
+            )
+
+        captured = capsys.readouterr()
+        emitted = captured.out
+        # Hint identifies the plugin@marketplace
+        assert "'shared-tool@my-marketplace'" in emitted
+        # Marketplace host is named in the "registered on" clause
+        # (anchored substring sidesteps CodeQL bare-host pattern recognizers)
+        assert f"registered on '{_GHE_HOST}'" in emitted
+        # The bare repo from marketplace.json is echoed back
+        assert "`repo: platform-team/shared-tool`" in emitted
+        # Concrete remediation value the operator can copy-paste
+        assert f"'{_GHE_HOST}/platform-team/shared-tool'" in emitted
+        # Auth-expert clause acknowledges the legitimate-cross-host
+        # alternative so transient failures of real github.com deps are
+        # not misdirected into adding an enterprise host prefix.
+        assert "intentionally a github.com dependency" in emitted
+
+    def test_legitimate_cross_host_validation_passes_no_hint(self, capsys):
+        """The legitimate cross-host case (validation passes) emits no hint.
+
+        This is the entire reason the diagnostic lives at the
+        validation-failure boundary instead of resolver time."""
+        from apm_cli.commands.install import _resolve_package_references
+        from apm_cli.core.command_logger import InstallLogger
+
+        plugin = MarketplacePlugin(
+            name="shared-tool",
+            source={
+                "type": "github",
+                "repo": "platform-team/shared-tool",
+                "path": "plugins/shared",
+            },
+        )
+
+        with (
+            patch(
+                "apm_cli.marketplace.resolver.get_marketplace_by_name",
+                return_value=_make_source(_GHE_HOST),
+            ),
+            patch(
+                "apm_cli.marketplace.resolver.fetch_or_cache",
+                return_value=_make_manifest(plugin),
+            ),
+            patch(
+                "apm_cli.commands.install._validate_package_exists",
+                return_value=True,
+            ),
+        ):
+            _resolve_package_references(
+                ["shared-tool@my-marketplace"],
+                [],
+                set(),
+                logger=InstallLogger(verbose=False),
+            )
+
+        emitted = capsys.readouterr().out
+        # No hint substrings on the successful path.
+        assert "intentionally a github.com dependency" not in emitted
+        assert "If you meant the enterprise host" not in emitted

tests/unit/commands/test_install_resolve_refs.py

@@ -370,17 +370,30 @@ def test_hint_emitted_on_validation_failure_with_risk(
             logger=logger,
         )
 
-        # The hint must be emitted exactly once and include the
-        # actionable host-qualified suggestion.
-        info_calls = [c for c in logger.info.call_args_list]
-        assert len(info_calls) == 1
-        emitted = info_calls[0].args[0]
-        assert "shared-tool@my-marketplace" in emitted
-        assert "corp.ghe.com" in emitted
-        assert "platform-team/shared-tool" in emitted
+        # The hint must be emitted exactly once via ``logger.warning``
+        # (PR #1292 panel review's explicit guidance for this follow-up).
+        # Assertions are anchored to the surrounding prose so a CodeQL
+        # "incomplete URL substring sanitization" pattern recognizer does
+        # not flag a bare hostname substring check in this test file.
+        warn_calls = list(logger.warning.call_args_list)
+        assert len(warn_calls) == 1
+        # ``info`` must NOT be used for the hint (the original PR shipped
+        # ``logger.info`` and was caught by the 3-persona panel convergence).
+        assert logger.info.call_args_list == []
+        emitted = warn_calls[0].args[0]
+        # Hint identifies the plugin@marketplace and both intent branches.
+        assert "'shared-tool@my-marketplace'" in emitted
+        assert "registered on 'corp.ghe.com'" in emitted
+        assert "`repo: platform-team/shared-tool`" in emitted
         assert (
-            "corp.ghe.com/platform-team/shared-tool" in emitted
+            "'corp.ghe.com/platform-team/shared-tool'" in emitted
         )
+        # Second clause acknowledges the legitimate-cross-host path so a
+        # transient-failure on a real github.com dep is not misdirected.
+        assert "intentionally a github.com dependency" in emitted
+        # Stale ``Hint:`` prefix from the original PR must not return; the
+        # warning symbol carries the advisory signal on its own.
+        assert "Hint:" not in emitted
 
     @patch("apm_cli.commands.install._validate_package_exists", return_value=True)
     @patch("apm_cli.marketplace.resolver.resolve_marketplace_plugin")
@@ -416,7 +429,7 @@ def test_hint_not_emitted_when_validation_passes_even_with_risk(
             logger=logger,
         )
 
-        assert logger.info.call_args_list == []
+        assert logger.warning.call_args_list == []
 
     @patch("apm_cli.commands.install._validate_package_exists", return_value=False)
     @patch("apm_cli.marketplace.resolver.resolve_marketplace_plugin")
@@ -451,7 +464,7 @@ def test_no_hint_when_resolution_has_no_risk(
             logger=logger,
         )
 
-        assert logger.info.call_args_list == []
+        assert logger.warning.call_args_list == []
 
     @patch("apm_cli.commands.install._validate_package_exists", return_value=False)
     @patch("apm_cli.commands.install.DependencyReference")
@@ -481,4 +494,4 @@ def test_no_hint_for_plain_owner_repo_failure(
             logger=logger,
         )
 
-        assert logger.info.call_args_list == []
+        assert logger.warning.call_args_list == []