Skip to content
Open
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
23 changes: 23 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

### Security

- **BREAKING:** `apm install` against a `*.ghe.com` marketplace now refuses bare cross-repo `repo:` fields in `marketplace.json` before any network request runs, closing a dependency-confusion attack vector. Previously a bare `repo: owner/repo` on an enterprise marketplace could silently resolve to an attacker-controlled namespace on public `github.com`; the install now fail-closes with an actionable error. Qualify the `repo:` field to fix:

```yaml
# Before (ambiguous -- refused on enterprise marketplaces):
source:
type: git
repo: owner/repo

# After -- enterprise dep on the same host:
source:
type: git
repo: corp.ghe.com/owner/repo

# After -- declared cross-host dep on public github.com:
source:
type: git
repo: github.com/owner/repo
```

(closes #1326) -- by @edenfunf (#1459)

### Fixed

- Copilot, Codex, Cursor, Claude, Windsurf, OpenCode, and Gemini adapters handle MCP v0.1 `runtimeArguments`/`packageArguments` with `variables` (no `type` key), matching the VS Code fix from #1444. (#1461, closes #1452, thanks @sergio-sisternes-epam)
Expand Down
25 changes: 25 additions & 0 deletions docs/src/content/docs/producer/publish-to-a-marketplace.md
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,31 @@ consumers running `apm install --update` on their own cadence.
- **No `versions[]` array.** Each compiled package carries one
resolved ref -- the highest tag matching the range at build time.
Re-run `apm pack` and re-tag to publish a new version.
- **Bare cross-repo `repo:` on enterprise (`*.ghe.com`) marketplaces is
refused at install time.** Dict-form plugin sources (the `source:`
mapping with nested `type:` and `repo:` keys) that point to a
different repo than the marketplace project must host-qualify the
`repo:` field. A bare `owner/repo` cannot be disambiguated from a
dependency-confusion attempt where an attacker pre-stages the namespace
on public `github.com`, so the install command fail-closes before
validating.

```yaml
# Refused -- ambiguous bare form on enterprise marketplace:
source:
type: git
repo: owner/repo

# Accepted -- enterprise dep on the same host:
source:
type: git
repo: corp.ghe.com/owner/repo

# Accepted -- declared cross-host dep on public github.com:
source:
type: git
repo: github.com/owner/repo
```

## Governance

Expand Down
1 change: 1 addition & 0 deletions docs/src/content/docs/reference/cli/install.md
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ Transport env vars: `APM_GIT_PROTOCOL` (`ssh` or `https`) sets the default initi
- **Frozen mode.** With `--frozen`, install resolves only what is in `apm.lock.yaml`. A direct dependency missing from the lockfile, or a missing lockfile entirely, exits `1`. Orphan lockfile entries (locked but no longer in `apm.yml`) are tolerated; local-path deps are skipped. This is a structural check, not a content check -- run `apm audit --ci` for hash verification.
- **Local `.apm/` deployment.** After dependencies are integrated, primitives in the project's own `.apm/` directory are deployed to the same targets. Local files win on collision. Skipped at `--global` and with `--only mcp`.
- **Stale-file cleanup.** Files a still-present package previously deployed but no longer produces are removed from the workspace, gated by per-file content hashes recorded in the lockfile (user-edited files are kept with a warning).
- **Enterprise marketplace gate.** When installing from a `*.ghe.com` marketplace, bare cross-repo `repo:` fields (e.g. `repo: owner/repo`) are refused before any network request runs, preventing dependency-confusion attacks. Host-qualify the field to proceed: `repo: corp.ghe.com/owner/repo` for an enterprise dep, or `repo: github.com/owner/repo` for a declared cross-host dep.
- **Security scan.** Source files are scanned for hidden Unicode and other tag-character / bidi-override patterns before deployment. Critical findings block the package; the install exits `1`. Use `--force` to deploy anyway, or run `apm audit --strip` first to remediate.
- **Diagnostic summary.** Output is grouped at the end (collisions, replacements, warnings, errors) instead of inline. Use `--verbose` to expand individual file paths.

Expand Down
31 changes: 31 additions & 0 deletions packages/apm-guide/.apm/skills/apm-usage/package-authoring.md
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,37 @@ Schema rules:
checks that `version:` is present). Omit entirely to skip the gate.
- Unknown keys raise a schema error -- do not invent fields.

### Cross-repo plugin sources on enterprise marketplaces

When a marketplace published on a `*.ghe.com` host references a plugin
in a different repo via the YAML mapping form of `source:` -- with
nested `type:` and `repo:` keys (rather than the simple `source: owner/repo`
string) -- the `repo:` field **must be host-qualified**. A bare
`owner/repo` value is refused at install time because it cannot be
disambiguated from a public-github.com dependency-confusion attempt
(see CHANGELOG entry for #1326). Two valid forms:

```yaml
plugins:
- name: shared-tool
source:
type: github
# Enterprise dep (most common): host-qualify to the marketplace host
repo: corp.ghe.com/platform-team/shared-tool
path: plugins/shared

- name: opensource-helper
source:
type: github
# Declared cross-host dep: host-qualify to github.com explicitly
repo: github.com/opensource-org/helper
path: plugins/helper
```

In-marketplace plugins (`source: ./...` or `source: owner/marketplace-repo`
when it matches the marketplace project) are unaffected -- the resolver
backfills the host automatically.

### Build semantics

`apm pack` runs `git ls-remote` against each remote plugin source, picks the
Expand Down
81 changes: 39 additions & 42 deletions src/apm_cli/commands/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -352,13 +352,6 @@ 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 @@ -407,19 +400,51 @@ def warning_handler(msg):
canonical_str, _resolved_plugin = resolution
if logger:
logger.verbose_detail(f" Resolved to: {canonical_str}")
# #1326: dependency-confusion fail-closed gate.
# When the resolver attaches ``CrossRepoMisconfigRisk``,
# the marketplace declared a bare ``owner/repo`` on a
# ``*.ghe.com`` host and the canonical falls back to
# ``github.com`` -- the same syntactic form that an
# attacker pre-staging the namespace on public github.com
# would exploit. Refuse before any outbound validation so
# no probe reaches the potentially-attacker-controlled
# URL (information leak + RCE both shut at one boundary).
# Escape hatch: marketplace.json author host-qualifies
# ``repo:`` (either to the enterprise host for same-host
# intent or to github.com for declared cross-host intent).
# That prevents the sentinel from attaching at resolver
# layer -- no new flag, env var, or schema field needed.
_risk = resolution.cross_repo_misconfig_risk
if _risk is not None:
# Two explicit-host options are alternatives, not a
# sequence, so they read clearer as separate bullets.
# ``validation_fail`` prepends the package name; the
# body below is the remediation. Each list element is
# one logical clause so individual edits stay local.
_lead = (
f"refused (dependency-confusion risk #1326): bare"
f" `repo: {_risk.bare_repo_field}` on enterprise"
f" marketplace '{_risk.marketplace_host}' is ambiguous."
f" Host-qualify the plugin `repo` field in"
f" marketplace.json to one of:"
)
reason = "\n".join(
[
_lead,
f" - '{_risk.suggested_qualified_repo}' (enterprise dep on this marketplace)",
f" - 'github.com/{_risk.bare_repo_field}' (declared cross-host dep on public github.com)",
]
)
invalid_outcomes.append((package, reason))
if logger:
logger.validation_fail(package, reason)
continue
marketplace_provenance = {
"discovered_via": marketplace_name,
"marketplace_plugin_name": plugin_name,
}
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 @@ -557,34 +582,6 @@ 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
66 changes: 58 additions & 8 deletions src/apm_cli/marketplace/resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,11 @@
from urllib.parse import quote, urlparse

from ..models.dependency.reference import DependencyReference
from ..utils.github_host import is_azure_devops_hostname, is_github_hostname
from ..utils.github_host import (
is_azure_devops_hostname,
is_github_hostname,
is_supported_git_host,
)
from ..utils.path_security import PathTraversalError, validate_path_segments
from .client import fetch_or_cache
from .errors import PluginNotFoundError
Expand All @@ -50,7 +54,9 @@
@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).
GitHub-family marketplace declares a bare ``owner/repo`` whose canonical
falls back to ``github.com`` -- the same syntactic ambiguity that powers
a dependency-confusion attack (#1326, formerly diagnosed only as #1305).

Attached to :class:`MarketplacePluginResolution` when the marketplace is on
``*.ghe.com`` and the plugin's dict source declares a bare ``owner/repo``
Expand All @@ -60,9 +66,18 @@ class CrossRepoMisconfigRisk:
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.
distinguish them.

Consumer contract (#1326): the install command consults this sentinel
BEFORE any outbound validation HTTP call and refuses the package
fail-closed when it is non-``None``. The earlier #1305 design surfaced
only an advisory hint on validation failure, which left the success
path (attacker pre-stages the bare namespace on public github.com)
silently exploitable. Cross-host explicit qualification by the
marketplace author -- ``repo: github.com/owner/repo`` -- prevents
the sentinel from attaching at the resolver layer (see
:func:`_compute_cross_repo_misconfig_risk`), which is the supported
escape hatch for declared cross-host intent.
"""

marketplace_host: str
Expand All @@ -80,9 +95,10 @@ 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.
:attr:`cross_repo_misconfig_risk` is non-``None`` only for the
cross-repo bare-on-enterprise pattern (#1305 / #1326); the install
command consumes it as a pre-validation fail-closed signal so a
dependency-confusion attempt cannot reach an outbound HTTP probe.
"""

canonical: str
Expand Down Expand Up @@ -290,6 +306,40 @@ def _compute_cross_repo_misconfig_risk(
bare = repo_field.strip().lstrip("/")
if "/" not in bare:
return None
# #1326: an already-host-qualified `repo:` field declares explicit intent
# (e.g. ``repo: github.com/owner/repo`` on a ``*.ghe.com`` marketplace is
# an unambiguous declared cross-host dependency). Only the truly-bare
# ``owner/repo`` form is the dependency-confusion vector this sentinel
# flags. ``_needs_canonical_host_prefix`` above already returns False
# for SAME-host qualification (its idempotency clause) and for URL /
# SSH SCP shorthand canonicals; this is the symmetric guard for the
# remaining case -- CROSS-host shorthand qualification (``github.com/...``
# on a ``*.ghe.com`` marketplace), which the idempotency check cannot
# detect because the canonical starts with a different host than
# ``source.host``.
#
# Defense in depth: extract the host from URL and SCP shorthand forms
# too, so the guard is robust even if a future upstream refactor lets
# those forms reach this point. A bare ``split("/", 1)[0]`` would
# otherwise classify ``https://...`` as having a ``https:`` first
# segment (not a host) and incorrectly attach the sentinel.
explicit_host = ""
bare_lower = bare.lower()
if bare_lower.startswith(("https://", "http://", "ssh://")):
explicit_host = (urlparse(bare).hostname or "").strip()
elif bare.startswith("git@") and ":" in bare:
# SCP shorthand: ``git@host:owner/repo``
explicit_host = bare[4:].split(":", 1)[0].strip()
else:
explicit_host = bare.split("/", 1)[0]
# ``is_supported_git_host`` accepts any valid FQDN, not an allowlist.
# This is intentional: the goal is to distinguish "looks like a
# hostname" (explicit intent) from "bare owner/repo" (ambiguous).
# Restricting to known hosts would silently refuse legitimate
# self-hosted Git servers and create a false sense of security --
# the real protection is the fail-closed refusal of the bare form.
if is_supported_git_host(explicit_host):
return None
Comment thread
edenfunf marked this conversation as resolved.
return CrossRepoMisconfigRisk(
marketplace_host=source.host,
bare_repo_field=bare,
Expand Down
Loading
Loading