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 @@ -33,6 +33,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
Expand Down
42 changes: 42 additions & 0 deletions src/apm_cli/commands/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,13 @@ def _resolve_package_references(
invalid_outcomes = [] # (package, reason) tuples
_marketplace_provenance = {} # canonical -> {discovered_via, marketplace_plugin_name}
_apm_yml_entries = {} # canonical -> apm.yml entry (str or dict for HTTP deps)
# #1305: canonical -> (marketplace_name, plugin_name, CrossRepoMisconfigRisk)
# for cross-repo dict ``type: github`` sources on enterprise marketplaces
# whose bare ``repo`` would mis-route auth at ``github.com``. Recorded
# before validation runs so the validation-fail branch can emit an
# actionable hint -- ``_marketplace_provenance`` is only written on
# validation success and cannot be relied on at the failure boundary.
_misconfig_risks = {}
validated_packages = []
dependencies_changed = False

Expand Down Expand Up @@ -403,6 +410,13 @@ def warning_handler(msg):
}
package = canonical_str
marketplace_dep_ref = getattr(resolution, "dependency_reference", None)
_risk = getattr(resolution, "cross_repo_misconfig_risk", None)
if _risk is not None:
_misconfig_risks[canonical_str] = (
marketplace_name,
plugin_name,
_risk,
)
except Exception as mkt_err:
reason = str(mkt_err)
invalid_outcomes.append((package, reason))
Expand Down Expand Up @@ -521,6 +535,34 @@ def warning_handler(msg):
invalid_outcomes.append((package, reason))
if logger:
logger.validation_fail(package, reason)
# #1305: when a cross-repo dict ``type: github`` source on an
# enterprise marketplace fails validation, the failure is most
# 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. ``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.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. "
"If this is intentionally a github.com dependency, "
"verify your github.com credentials and that the "
"repository is accessible."
)

return (
valid_outcomes,
Expand Down
94 changes: 93 additions & 1 deletion src/apm_cli/marketplace/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,29 @@
_SEMVER_RANGE_CHARS = re.compile(r"[~^<>=!]")


@dataclass(frozen=True)
class CrossRepoMisconfigRisk:
"""Signal that a cross-repo dict ``type: github`` source on an enterprise
GitHub-family marketplace resolved to a bare canonical (#1305).

Attached to :class:`MarketplacePluginResolution` when the marketplace is on
``*.ghe.com`` and the plugin's dict source declares a bare ``owner/repo``
that does not match the marketplace project. The resolver deliberately
leaves these canonicals bare (PR #1292 scoped its host backfill to
in-marketplace sources), so ``DependencyReference.parse`` defaults the host
to ``github.com``. Two intents share this syntax -- a legitimate cross-host
``github.com`` open-source dep, or a misconfigured same-host entry that
should have been ``corp.ghe.com/owner/repo`` -- and the resolver cannot
distinguish them. The install command consults this sentinel when the
package fails validation so an actionable hint surfaces only at the
failure boundary, never on the legitimate path.
"""

marketplace_host: str
bare_repo_field: str
suggested_qualified_repo: str


@dataclass
class MarketplacePluginResolution:
"""Outcome of :func:`resolve_marketplace_plugin`.
Expand All @@ -57,11 +80,15 @@ class MarketplacePluginResolution:
subdirectory plugins), install logic should prefer it over
:meth:`~apm_cli.models.dependency.reference.DependencyReference.parse`
on :attr:`canonical` to avoid mis-parsing nested paths as GitLab project segments.
:attr:`cross_repo_misconfig_risk` is non-``None`` only for the #1305
cross-repo bare-on-enterprise pattern; consumers emit it as a hint when the
package subsequently fails validation.
"""

canonical: str
plugin: MarketplacePlugin
dependency_reference: DependencyReference | None = None
cross_repo_misconfig_risk: CrossRepoMisconfigRisk | None = None

def __iter__(self) -> Iterator[str | MarketplacePlugin]:
yield self.canonical
Expand Down Expand Up @@ -221,6 +248,55 @@ def _needs_canonical_host_prefix(canonical: str, host: str) -> bool:
return first_segment.lower() != h.lower()


def _compute_cross_repo_misconfig_risk(
plugin: MarketplacePlugin,
source: MarketplaceSource,
canonical: str,
dep_ref: DependencyReference | None,
) -> CrossRepoMisconfigRisk | None:
"""Identify the #1305 misconfiguration: cross-repo dict ``type: github``
source with bare ``repo`` on an enterprise GitHub-family marketplace.

Returns a :class:`CrossRepoMisconfigRisk` when **all** of:

- ``dep_ref`` is ``None`` (GitHub-family virtual-shorthand path; GitLab and
self-managed FQDNs build a structured ref upstream and sidestep the bug)
- ``plugin.source`` is a dict whose normalized type is ``github`` (other
dict types -- ``gitlab``, ``git-subdir`` -- hit the same auth-routing
bug but the "host-qualify with marketplace host" remediation only
matches operator intent for the GitHub family)
- the source is **not** an in-marketplace reference (PR #1292 already
backfills the host for those)
- ``_needs_canonical_host_prefix`` agrees the canonical is bare and the
host is GitHub-family enterprise (``*.ghe.com``; idempotent against
already host-qualified, URL, and SSH forms)
- the ``repo`` field is a non-empty ``owner/repo`` shorthand

Otherwise returns ``None``. Pure -- no logging, no side effects.
"""
if dep_ref is not None:
return None
if not isinstance(plugin.source, dict):
return None
if _coerce_dict_plugin_type(plugin.source) != "github":
return None
if _is_in_marketplace_source(plugin, source):
return None
if not _needs_canonical_host_prefix(canonical, source.host):
return None
repo_field = plugin.source.get("repo", "")
if not isinstance(repo_field, str):
return None
bare = repo_field.strip().lstrip("/")
if "/" not in bare:
return None
return CrossRepoMisconfigRisk(
marketplace_host=source.host,
bare_repo_field=bare,
suggested_qualified_repo=f"{source.host}/{bare}",
)


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 @@ -607,6 +683,19 @@ def _emit_warning(msg: str) -> None:
marketplace_name,
)

# ---- Cross-repo misconfig sentinel (#1305) ----
# PR #1292's host backfill only covers in-marketplace sources. A cross-repo
# dict ``type: github`` source with a bare ``repo`` on an enterprise
# marketplace cannot be safely backfilled here -- the bare syntax also
# legitimately means "a github.com open-source dep from this enterprise
# marketplace" -- so the canonical stays bare and downstream auth routes at
# github.com. Attach a sentinel so the install command can emit an
# actionable hint ONLY when the package subsequently fails validation; the
# legitimate cross-host path validates fine and never sees the hint.
cross_repo_misconfig_risk = _compute_cross_repo_misconfig_risk(
plugin, source, canonical, dep_ref
)

# ---- 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 Expand Up @@ -674,5 +763,8 @@ def _emit_warning(msg: str) -> None:
logger.debug("Shadow detection failed", exc_info=True)

return MarketplacePluginResolution(
canonical=canonical, plugin=plugin, dependency_reference=dep_ref
canonical=canonical,
plugin=plugin,
dependency_reference=dep_ref,
cross_repo_misconfig_risk=cross_repo_misconfig_risk,
)
161 changes: 149 additions & 12 deletions tests/integration/test_ghe_marketplace_install_e2e.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -234,12 +240,143 @@ 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
Loading
Loading