diff --git a/CHANGELOG.md b/CHANGELOG.md index c78c4e335..0b96caeb5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `apm install --ssh` / `--https` flags and `APM_GIT_PROTOCOL=ssh|https` env to pick the initial transport for shorthand dependencies (#778) - `apm install --allow-protocol-fallback` flag and `APM_ALLOW_PROTOCOL_FALLBACK=1` env as the migration escape hatch for cross-protocol fallback (#778) - Add APM Review Panel skill (`.github/skills/apm-review-panel/`) and four new specialist personas (`devx-ux-expert`, `supply-chain-security-expert`, `apm-ceo`, `oss-growth-hacker`) with auto-activating per-persona skills. Routes specialist findings through an APM CEO arbiter for strategic / breaking-change calls, with the OSS growth hacker side-channeling adoption insights via `WIP/growth-strategy.md`. Instrumentation per Handbook Ch. 9 (`The Instrumented Codebase`); PROSE-compliant (thin SKILL.md routers, persona detail lazy-loaded via markdown links, explicit boundaries per persona). +- `apm view plugin@marketplace` displays marketplace plugin metadata (name, version, source, description) (#514) +- `apm outdated` checks marketplace plugin refs and shows a "Source" column distinguishing marketplace vs git updates (#514) +- `apm marketplace validate` command with schema validation and duplicate name detection (#514) +- Ref immutability advisory: caches plugin-to-ref pins and warns when a previously pinned plugin's ref changes (#514) +- Multi-marketplace shadow detection: warns when the same plugin name appears in multiple registered marketplaces (#514) ### Fixed diff --git a/docs/src/content/docs/guides/marketplaces.md b/docs/src/content/docs/guides/marketplaces.md index a2607822b..1f985feb5 100644 --- a/docs/src/content/docs/guides/marketplaces.md +++ b/docs/src/content/docs/guides/marketplaces.md @@ -63,6 +63,21 @@ Marketplaces can declare a `metadata.pluginRoot` field to specify the base direc With `pluginRoot` set to `./plugins`, the source `"my-tool"` resolves to `owner/repo/plugins/my-tool`. Sources that already contain a path separator (e.g. `./custom/path`) are not affected by `pluginRoot`. +### Versioned plugins + +Plugins can declare a `version` field and a `source.ref` that points to a specific Git tag or commit: + +```json +{ + "name": "code-review", + "description": "Automated code review agent", + "version": "2.1.0", + "source": { "type": "github", "repo": "acme/code-review-plugin", "ref": "v2.1.0" } +} +``` + +The `version` field is informational (displayed by `apm view` and `apm outdated`). The `source.ref` determines which Git ref APM checks out during install. + ## Register a marketplace ```bash @@ -125,13 +140,32 @@ use `apm marketplace browse ` instead. Use the `NAME@MARKETPLACE` syntax to install a plugin from a specific marketplace: ```bash +# Install using the source ref from the marketplace entry apm install code-review@acme-plugins + +# Install with a specific git ref override +apm install code-review@acme-plugins#v2.0.0 + +# Install from a specific branch +apm install code-review@acme-plugins#main ``` -APM resolves the plugin name against the marketplace index, fetches the underlying Git repository, and installs it as a standard APM dependency. The resolved source appears in `apm.yml` and `apm.lock.yaml` just like any direct dependency. +The `#` separator carries a raw git ref that overrides the `source.ref` from the marketplace entry. Without `#`, APM uses the ref defined in the marketplace manifest. + +APM resolves the plugin name against the marketplace index, fetches the underlying Git repository using the resolved ref, and installs it as a standard APM dependency. The resolved source appears in `apm.yml` and `apm.lock.yaml` just like any direct dependency. For full `apm install` options, see [CLI Commands](../../reference/cli-commands/). +## View plugin details + +Show metadata for a marketplace plugin: + +```bash +apm view code-review@acme-plugins +``` + +Displays the plugin's name, version, description, source, and tags. + ## Provenance tracking Marketplace-resolved plugins are tracked in `apm.lock.yaml` with full provenance: @@ -187,3 +221,50 @@ apm marketplace remove acme-plugins --yes ``` Removing a marketplace does not uninstall plugins previously installed from it. Those plugins remain pinned in `apm.lock.yaml` to their resolved Git sources. + +## Validate a marketplace + +Check a marketplace manifest for schema errors and duplicate entries: + +```bash +apm marketplace validate acme-plugins + +# Verbose output +apm marketplace validate acme-plugins --verbose +``` + +Catches: missing required fields and duplicate plugin names (case-insensitive). + +:::note[Planned] +The `--check-refs` flag will verify that source refs are reachable over the network. It is accepted but not yet implemented. +::: + +For full option details, see [CLI Commands](../../reference/cli-commands/). + +## Security + +### Version immutability + +APM caches version-to-ref mappings in `~/.apm/cache/marketplace/version-pins.json`. On subsequent installs, APM compares the marketplace ref against the cached pin. If a version's ref has changed, APM warns: + +``` +WARNING: Version 2.0.0 of code-review@acme-plugins ref changed: was 'v2.0.0', now 'deadbeef'. This may indicate a ref swap attack. +``` + +This detects marketplace maintainers (or compromised accounts) silently pointing an existing version at different code. + +### Shadow detection + +When installing a marketplace plugin, APM checks all other registered marketplaces for plugins with the same name. A match produces a warning: + +``` +WARNING: Plugin 'code-review' also found in marketplace 'other-plugins'. Verify you are installing from the intended source. +``` + +Shadow detection runs automatically during install -- no configuration required. + +### Best practices + +- **Use commit SHAs as refs** -- tags and branches can be moved; commit SHAs cannot. +- **Keep plugin names unique across marketplaces** -- avoids shadow warnings and reduces confusion. +- **Review immutability warnings** -- a changed ref for an existing version is a strong signal of tampering. diff --git a/docs/src/content/docs/reference/cli-commands.md b/docs/src/content/docs/reference/cli-commands.md index 9190dc4f7..be886e455 100644 --- a/docs/src/content/docs/reference/cli-commands.md +++ b/docs/src/content/docs/reference/cli-commands.md @@ -81,7 +81,7 @@ apm install [PACKAGES...] [OPTIONS] ``` **Arguments:** -- `PACKAGES` - Optional APM packages to add and install. Accepts shorthand (`owner/repo`), HTTPS URLs, SSH URLs, FQDN shorthand (`host/owner/repo`), local filesystem paths (`./path`, `../path`, `/absolute/path`, `~/path`), or marketplace references (`NAME@MARKETPLACE`). All forms are normalized to canonical format in `apm.yml`. +- `PACKAGES` - Optional APM packages to add and install. Accepts shorthand (`owner/repo`), HTTPS URLs, SSH URLs, FQDN shorthand (`host/owner/repo`), local filesystem paths (`./path`, `../path`, `/absolute/path`, `~/path`), or marketplace references (`NAME@MARKETPLACE[#ref]`). All forms are normalized to canonical format in `apm.yml`. **Options:** - `--runtime TEXT` - Target specific runtime only (copilot, codex, vscode) @@ -191,6 +191,9 @@ apm install -g microsoft/apm-sample-package # Install a plugin from a registered marketplace apm install code-review@acme-plugins + +# Install a specific ref from a marketplace +apm install code-review@acme-plugins#v2.0.0 ``` **Auto-Bootstrap Behavior:** @@ -645,7 +648,7 @@ apm view PACKAGE [FIELD] [OPTIONS] ``` **Arguments:** -- `PACKAGE` - Package name, usually `owner/repo` or a short repo name +- `PACKAGE` - Package name: `owner/repo`, short repo name, or `NAME@MARKETPLACE` for marketplace plugins - `FIELD` - Optional field selector. Supported value: `versions` **Options:** @@ -662,6 +665,9 @@ apm view apm-sample-package # List remote tags and branches without cloning apm view microsoft/apm-sample-package versions +# View available versions for a marketplace plugin +apm view code-review@acme-plugins + # Inspect a package from user scope apm view microsoft/apm-sample-package -g ``` @@ -671,6 +677,7 @@ apm view microsoft/apm-sample-package -g - Shows package name, version, description, source, install path, context files, workflows, and hooks - `versions` lists remote tags and branches without cloning the repository - `versions` does not require the package to be installed locally +- `NAME@MARKETPLACE` syntax shows the marketplace plugin metadata (name, version, source, description, tags) ### `apm outdated` - Check locked dependencies for updates @@ -704,8 +711,10 @@ apm outdated -j 8 - Reads the current lockfile (`apm.lock.yaml`; legacy `apm.lock` is migrated automatically) - For tag-pinned deps: compares the locked semver tag against the latest available remote tag - For branch-pinned deps: compares the locked commit SHA against the remote branch tip SHA +- For marketplace deps: compares the installed ref against the marketplace entry's current `source.ref` - For deps with no ref: compares against the default branch (main/master) tip SHA -- Displays `Package`, `Current`, `Latest`, and `Status` columns +- Displays `Package`, `Current`, `Latest`, `Status`, and `Source` columns +- `Source` shows `marketplace: ` for marketplace-sourced deps - Status values are `up-to-date`, `outdated`, and `unknown` - Local dependencies and Artifactory dependencies are skipped @@ -1084,6 +1093,30 @@ apm marketplace remove acme-plugins apm marketplace remove acme-plugins --yes ``` +#### `apm marketplace validate` - Validate a marketplace manifest + +Validate `marketplace.json` for schema errors and duplicate plugin names. + +```bash +apm marketplace validate NAME [OPTIONS] +``` + +**Arguments:** +- `NAME` - Name of the marketplace to validate + +**Options:** +- `--check-refs` - Verify version refs are reachable (network). *Not yet implemented.* +- `-v, --verbose` - Show detailed output + +**Examples:** +```bash +# Validate a marketplace +apm marketplace validate acme-plugins + +# Verbose output +apm marketplace validate acme-plugins --verbose +``` + ### `apm search` - Search plugins in a marketplace Search for plugins by name or description within a specific marketplace. diff --git a/packages/apm-guide/.apm/skills/apm-usage/commands.md b/packages/apm-guide/.apm/skills/apm-usage/commands.md index 479451253..9e71beded 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/commands.md +++ b/packages/apm-guide/.apm/skills/apm-usage/commands.md @@ -57,7 +57,10 @@ | `apm marketplace browse NAME` | Browse marketplace packages | -- | | `apm marketplace update [NAME]` | Update marketplace index | -- | | `apm marketplace remove NAME` | Remove a marketplace | `-y` skip confirm | +| `apm marketplace validate NAME` | Validate marketplace manifest | `--check-refs`, `-v` | | `apm search QUERY@MARKETPLACE` | Search marketplace | `--limit N` | +| `apm install NAME@MKT[#ref]` | Install from marketplace | Optional `#ref` override | +| `apm view NAME@MARKETPLACE` | View marketplace plugin info | -- | ## MCP servers diff --git a/packages/apm-guide/.apm/skills/apm-usage/dependencies.md b/packages/apm-guide/.apm/skills/apm-usage/dependencies.md index 00aa82c58..1f098914f 100644 --- a/packages/apm-guide/.apm/skills/apm-usage/dependencies.md +++ b/packages/apm-guide/.apm/skills/apm-usage/dependencies.md @@ -179,6 +179,18 @@ dependencies: | Branch | `owner/repo#main` | Development -- tracks latest | | Commit SHA | `owner/repo#abc123d` | Maximum reproducibility | | No ref | `owner/repo` | Resolves default branch at install time | +| Marketplace ref | `plugin@marketplace#ref` | Override marketplace source ref | + +## Marketplace ref override + +When installing from a marketplace, the `#` suffix overrides the `source.ref` from the marketplace entry: + +| Syntax | Meaning | Example | +|--------|---------|---------| +| `plugin@mkt` | Use marketplace source ref | `plugin@mkt` | +| `plugin@mkt#v2.0.0` | Override with specific tag | `plugin@mkt#v2.0.0` | +| `plugin@mkt#main` | Override with branch | `plugin@mkt#main` | +| `plugin@mkt#abc123d` | Override with commit SHA | `plugin@mkt#abc123d` | ## What the lockfile pins diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py index 66439c51c..4d6f6808e 100644 --- a/src/apm_cli/commands/install.py +++ b/src/apm_cli/commands/install.py @@ -193,16 +193,20 @@ def _validate_and_add_packages_to_apm_yml(packages, dry_run=False, dev=False, lo mkt_ref = None if mkt_ref is not None: - plugin_name, marketplace_name = mkt_ref + plugin_name, marketplace_name, version_spec = mkt_ref try: + warning_handler = None if logger: + warning_handler = lambda msg: logger.warning(msg) logger.verbose_detail( f" Resolving {plugin_name}@{marketplace_name} via marketplace..." ) canonical_str, resolved_plugin = resolve_marketplace_plugin( plugin_name, marketplace_name, + version_spec=version_spec, auth_resolver=auth_resolver, + warning_handler=warning_handler, ) if logger: logger.verbose_detail( diff --git a/src/apm_cli/commands/marketplace.py b/src/apm_cli/commands/marketplace.py index ef4e201d1..3d50f82a3 100644 --- a/src/apm_cli/commands/marketplace.py +++ b/src/apm_cli/commands/marketplace.py @@ -368,6 +368,92 @@ def remove(name, yes, verbose): sys.exit(1) +# --------------------------------------------------------------------------- +# marketplace validate +# --------------------------------------------------------------------------- + + +@marketplace.command(help="Validate a marketplace manifest") +@click.argument("name", required=True) +@click.option( + "--check-refs", is_flag=True, help="Verify version refs are reachable (network)" +) +@click.option("--verbose", "-v", is_flag=True, help="Show detailed output") +def validate(name, check_refs, verbose): + """Validate the manifest of a registered marketplace.""" + logger = CommandLogger("marketplace-validate", verbose=verbose) + try: + from ..marketplace.client import fetch_marketplace + from ..marketplace.registry import get_marketplace_by_name + from ..marketplace.validator import validate_marketplace + + source = get_marketplace_by_name(name) + logger.start(f"Validating marketplace '{name}'...", symbol="gear") + + manifest = fetch_marketplace(source, force_refresh=True) + + logger.progress( + f"Found {len(manifest.plugins)} plugins", + symbol="info", + ) + + # Verbose: per-plugin details + if verbose: + for p in manifest.plugins: + source_type = "dict" if isinstance(p.source, dict) else "string" + logger.verbose_detail( + f" {p.name}: source type: {source_type}" + ) + + # Run validation + results = validate_marketplace(manifest) + + # Check-refs placeholder + if check_refs: + logger.warning( + "Ref checking not yet implemented -- skipping ref " + "reachability checks", + symbol="warning", + ) + + # Render results + passed = 0 + warning_count = 0 + error_count = 0 + click.echo() + click.echo("Validation Results:") + for r in results: + if r.passed and not r.warnings: + logger.success( + f" {r.check_name}: all plugins valid", symbol="check" + ) + passed += 1 + elif r.warnings and not r.errors: + for w in r.warnings: + logger.warning(f" {r.check_name}: {w}", symbol="warning") + warning_count += len(r.warnings) + else: + for e in r.errors: + logger.error(f" {r.check_name}: {e}", symbol="error") + for w in r.warnings: + logger.warning(f" {r.check_name}: {w}", symbol="warning") + error_count += len(r.errors) + warning_count += len(r.warnings) + + click.echo() + click.echo( + f"Summary: {passed} passed, {warning_count} warnings, " + f"{error_count} errors" + ) + + if error_count > 0: + sys.exit(1) + + except Exception as e: + logger.error(f"Failed to validate marketplace: {e}") + sys.exit(1) + + # --------------------------------------------------------------------------- # Top-level search command (registered separately in cli.py) # --------------------------------------------------------------------------- @@ -467,3 +553,4 @@ def search(expression, limit, verbose): except Exception as e: logger.error(f"Search failed: {e}") sys.exit(1) + diff --git a/src/apm_cli/commands/outdated.py b/src/apm_cli/commands/outdated.py index 2bef64cc2..b34739d0f 100644 --- a/src/apm_cli/commands/outdated.py +++ b/src/apm_cli/commands/outdated.py @@ -2,17 +2,34 @@ Compares locked dependency commit SHAs against remote tip SHAs. For tag-pinned deps, also shows the latest available semver tag. +For marketplace-sourced deps, checks available versions in the marketplace. """ +import logging import re import sys +from dataclasses import dataclass, field +from typing import List import click +logger = logging.getLogger(__name__) TAG_RE = re.compile(r"^v?\d+\.\d+\.\d+") +@dataclass(frozen=True) +class OutdatedRow: + """One row of ``apm outdated`` output.""" + + package: str + current: str + latest: str + status: str + extra_tags: List[str] = field(default_factory=list) + source: str = "" + + def _is_tag_ref(ref: str) -> bool: """Return True when *ref* looks like a semver tag (v1.2.3 or 1.2.3).""" return bool(TAG_RE.match(ref)) if ref else False @@ -53,12 +70,101 @@ def _find_remote_tip(ref_name, remote_refs): return None +def _check_marketplace_ref(dep, verbose): + """Check a marketplace-sourced dep against its marketplace entry. + + Compares the installed ref (resolved_ref or resolved_commit) against + the marketplace entry's current source ref. Returns a result tuple + ``(package, current, latest, status, extra, source)`` or ``None`` + when the check cannot be performed (caller should fall through to + the git-based check). + """ + if not dep.discovered_via or not dep.marketplace_plugin_name: + return None + + try: + from ..marketplace.client import fetch_or_cache + from ..marketplace.errors import MarketplaceError + from ..marketplace.registry import get_marketplace_by_name + except ImportError: + return None + + source_label = f"marketplace: {dep.discovered_via}" + + try: + source_obj = get_marketplace_by_name(dep.discovered_via) + except MarketplaceError: + logger.warning( + "Marketplace '%s' not found; falling back to git check for '%s'", + dep.discovered_via, + dep.marketplace_plugin_name, + ) + return None + + try: + manifest = fetch_or_cache(source_obj) + except MarketplaceError: + logger.warning( + "Failed to fetch marketplace '%s'; " + "falling back to git check for '%s'", + dep.discovered_via, + dep.marketplace_plugin_name, + ) + return None + + plugin = manifest.find_plugin(dep.marketplace_plugin_name) + if not plugin: + return None + + # Determine marketplace entry's current ref + mkt_ref = None + mkt_version = plugin.version or "" + if isinstance(plugin.source, dict): + mkt_ref = plugin.source.get("ref", "") + else: + # String sources are relative paths, not refs -- skip + return None + + if not mkt_ref: + return None + + # Determine installed ref + installed_ref = dep.resolved_ref or dep.resolved_commit or "" + if not installed_ref: + return None + + package_name = f"{dep.marketplace_plugin_name}@{dep.discovered_via}" + current_display = installed_ref[:12] if len(installed_ref) > 12 else installed_ref + latest_display = mkt_ref[:12] if len(mkt_ref) > 12 else mkt_ref + if mkt_version: + latest_display = f"{mkt_version} ({latest_display})" + + if installed_ref != mkt_ref: + return OutdatedRow( + package=package_name, current=current_display, + latest=latest_display, status="outdated", + source=source_label, + ) + + return OutdatedRow( + package=package_name, current=current_display, + latest=latest_display, status="up-to-date", + source=source_label, + ) + + def _check_one_dep(dep, downloader, verbose): """Check a single dependency against remote refs. - Returns a result tuple: (package_name, current, latest, status, extra_tags) + Returns an ``OutdatedRow`` instance. + This function is safe to call from a thread pool. """ + # Try marketplace-based check first for marketplace-sourced deps + marketplace_result = _check_marketplace_ref(dep, verbose) + if marketplace_result is not None: + return marketplace_result + from ..models.dependency.reference import DependencyReference from ..models.dependency.types import GitReferenceType from ..utils.version_checker import is_newer_version @@ -73,20 +179,20 @@ def _check_one_dep(dep, downloader, verbose): full_url = f"{dep.host}/{dep.repo_url}" if dep.host else dep.repo_url dep_ref = DependencyReference.parse(full_url) except Exception: - return (package_name, current_ref or "(none)", "-", "unknown", []) + return OutdatedRow(package=package_name, current=current_ref or "(none)", latest="-", status="unknown") # Fetch remote refs try: remote_refs = downloader.list_remote_refs(dep_ref) except Exception: - return (package_name, current_ref or "(none)", "-", "unknown", []) + return OutdatedRow(package=package_name, current=current_ref or "(none)", latest="-", status="unknown") is_tag = _is_tag_ref(current_ref) if is_tag: tag_refs = [r for r in remote_refs if r.ref_type == GitReferenceType.TAG] if not tag_refs: - return (package_name, current_ref, "-", "unknown", []) + return OutdatedRow(package=package_name, current=current_ref, latest="-", status="unknown", source="git tags") latest_tag = tag_refs[0].name current_ver = _strip_v(current_ref) @@ -94,21 +200,21 @@ def _check_one_dep(dep, downloader, verbose): if is_newer_version(current_ver, latest_ver): extra = [r.name for r in tag_refs[:10]] if verbose else [] - return (package_name, current_ref, latest_tag, "outdated", extra) + return OutdatedRow(package=package_name, current=current_ref, latest=latest_tag, status="outdated", extra_tags=extra, source="git tags") else: - return (package_name, current_ref, latest_tag, "up-to-date", []) + return OutdatedRow(package=package_name, current=current_ref, latest=latest_tag, status="up-to-date", source="git tags") else: remote_tip_sha = _find_remote_tip(current_ref, remote_refs) if not remote_tip_sha: - return (package_name, current_ref or "(none)", "-", "unknown", []) + return OutdatedRow(package=package_name, current=current_ref or "(none)", latest="-", status="unknown", source="git branch") display_ref = current_ref or "(default)" if locked_sha and locked_sha != remote_tip_sha: latest_display = remote_tip_sha[:8] - return (package_name, display_ref, latest_display, "outdated", []) + return OutdatedRow(package=package_name, current=display_ref, latest=latest_display, status="outdated", source="git branch") else: - return (package_name, display_ref, remote_tip_sha[:8], "up-to-date", []) + return OutdatedRow(package=package_name, current=display_ref, latest=remote_tip_sha[:8], status="up-to-date", source="git branch") @click.command(name="outdated") @@ -185,8 +291,8 @@ def outdated(global_, verbose, parallel_checks): return # Check if everything is up-to-date - has_outdated = any(status == "outdated" for _, _, _, status, _ in rows) - has_unknown = any(status == "unknown" for _, _, _, status, _ in rows) + has_outdated = any(row.status == "outdated" for row in rows) + has_unknown = any(row.status == "unknown" for row in rows) if not has_outdated and not has_unknown: logger.success("All dependencies are up-to-date") @@ -211,6 +317,7 @@ def outdated(global_, verbose, parallel_checks): table.add_column("Current", style="white", min_width=10) table.add_column("Latest", style="white", min_width=10) table.add_column("Status", min_width=12) + table.add_column("Source", style="dim", min_width=14) status_styles = { "up-to-date": "green", @@ -218,27 +325,37 @@ def outdated(global_, verbose, parallel_checks): "unknown": "dim", } - for package, current, latest, status, extra_tags in rows: - style = status_styles.get(status, "white") - table.add_row(package, current, latest, f"[{style}]{status}[/{style}]") + for row in rows: + style = status_styles.get(row.status, "white") + table.add_row( + row.package, row.current, row.latest, + f"[{style}]{row.status}[/{style}]", + row.source, + ) - if verbose and extra_tags: - tags_str = ", ".join(extra_tags) - table.add_row("", "", f"[dim]tags: {tags_str}[/dim]", "") + if verbose and row.extra_tags: + tags_str = ", ".join(row.extra_tags) + table.add_row("", "", f"[dim]tags: {tags_str}[/dim]", "", "") console.print(table) except (ImportError, Exception): # Fallback: plain text output - click.echo("Package Current Latest Status") - click.echo("-" * 65) - for package, current, latest, status, extra_tags in rows: - click.echo(f"{package:<24}{current:<13}{latest:<13}{status}") - if verbose and extra_tags: - click.echo(f"{'':24}tags: {', '.join(extra_tags)}") + click.echo( + f"{'Package':<24}{'Current':<13}{'Latest':<13}" + f"{'Status':<15}{'Source'}" + ) + click.echo("-" * 82) + for row in rows: + click.echo( + f"{row.package:<24}{row.current:<13}{row.latest:<13}" + f"{row.status:<15}{row.source}" + ) + if verbose and row.extra_tags: + click.echo(f"{'':24}tags: {', '.join(row.extra_tags)}") # Summary - outdated_count = sum(1 for _, _, _, s, _ in rows if s == "outdated") + outdated_count = sum(1 for row in rows if row.status == "outdated") if outdated_count: logger.warning(f"{outdated_count} outdated " f"{'dependency' if outdated_count == 1 else 'dependencies'} found") @@ -323,7 +440,7 @@ def _check_parallel(checkable, downloader, verbose, max_workers, result = fut.result() except Exception: pkg = dep.get_unique_key() - result = (pkg, "(none)", "-", "unknown", []) + result = OutdatedRow(package=pkg, current="(none)", latest="-", status="unknown") results[dep.get_unique_key()] = result progress.update(task_id, visible=False) progress.advance(overall_id) @@ -350,7 +467,7 @@ def _check_parallel_plain(checkable, downloader, verbose, max_workers): result = fut.result() except Exception: pkg = dep.get_unique_key() - result = (pkg, "(none)", "-", "unknown", []) + result = OutdatedRow(package=pkg, current="(none)", latest="-", status="unknown") results[dep.get_unique_key()] = result return [results[dep.get_unique_key()] for dep in checkable diff --git a/src/apm_cli/commands/view.py b/src/apm_cli/commands/view.py index e8c1b00c4..8ec6441b3 100644 --- a/src/apm_cli/commands/view.py +++ b/src/apm_cli/commands/view.py @@ -243,6 +243,113 @@ def display_package_info( sys.exit(1) +def _display_marketplace_plugin( + plugin_name: str, + marketplace_name: str, + logger: CommandLogger, +) -> None: + """Display metadata for a marketplace plugin. + + Fetches the marketplace manifest, finds the plugin, and renders + its entry information (name, version, description, source). + """ + from ..marketplace.errors import MarketplaceFetchError + from ..marketplace.models import MarketplaceSource + from ..marketplace.registry import get_marketplace_by_name + from ..marketplace.client import fetch_or_cache + + # -- Fetch marketplace & plugin -- + try: + source: MarketplaceSource = get_marketplace_by_name(marketplace_name) + except Exception as exc: + logger.error(str(exc)) + sys.exit(1) + + try: + manifest = fetch_or_cache(source) + except MarketplaceFetchError as exc: + logger.error(str(exc)) + logger.progress("Check your network connection and try again.") + sys.exit(1) + + plugin = manifest.find_plugin(plugin_name) + if plugin is None: + from ..marketplace.errors import PluginNotFoundError as _PNF + + logger.error(str(_PNF(plugin_name, marketplace_name))) + sys.exit(1) + + # -- Build info lines -- + title = f"Plugin: {plugin.name} (marketplace: {marketplace_name})" + + # Resolve canonical reference for display + resolved_display = None + try: + from ..marketplace.resolver import resolve_marketplace_plugin + + canonical_str, _resolved = resolve_marketplace_plugin( + plugin_name, marketplace_name, plugin, + ) + resolved_display = canonical_str + except Exception: + pass + + # Determine source display + source_display = "--" + if isinstance(plugin.source, dict): + src_type = plugin.source.get("type", "") or plugin.source.get("source", "") + repo = plugin.source.get("repo", "") or plugin.source.get("url", "") + ref = plugin.source.get("ref", "") + parts = [s for s in [src_type, repo] if s] + source_display = " / ".join(parts) + if ref: + source_display += f" @ {ref}" + elif isinstance(plugin.source, str): + source_display = plugin.source + + try: + from rich.console import Console + from rich.panel import Panel + + console = Console() + lines = [] + lines.append(f"[bold]Name:[/bold] {plugin.name}") + if plugin.version: + lines.append(f"[bold]Version:[/bold] {plugin.version}") + if plugin.description: + lines.append(f"[bold]Description:[/bold] {plugin.description}") + lines.append(f"[bold]Source:[/bold] {source_display}") + if resolved_display: + lines.append(f"[bold]Resolved:[/bold] {resolved_display}") + if plugin.tags: + lines.append(f"[bold]Tags:[/bold] {', '.join(plugin.tags)}") + + console.print(Panel( + "\n".join(lines), + title=title, + border_style="cyan", + )) + click.echo("") + click.echo(f" Install: apm install {plugin.name}@{marketplace_name}") + + except ImportError: + # Plain-text fallback + click.echo(title) + click.echo("-" * 60) + click.echo(f" Name: {plugin.name}") + if plugin.version: + click.echo(f" Version: {plugin.version}") + if plugin.description: + click.echo(f" Description: {plugin.description}") + click.echo(f" Source: {source_display}") + if resolved_display: + click.echo(f" Resolved: {resolved_display}") + if plugin.tags: + click.echo(f" Tags: {', '.join(plugin.tags)}") + click.echo("") + click.echo(f" Install: apm install {plugin.name}@{marketplace_name}") + + def display_versions(package: str, logger: CommandLogger) -> None: """Query and display available remote versions (tags/branches). @@ -251,7 +358,21 @@ def display_versions(package: str, logger: CommandLogger) -> None: ``DependencyReference``, queries remote refs via ``GitHubPackageDownloader.list_remote_refs``, and renders the result as a Rich table (with a plain-text fallback). + + When *package* matches the ``NAME@MARKETPLACE`` pattern, the + marketplace manifest is fetched instead and the plugin's marketplace + metadata is displayed. """ + # -- Marketplace path: NAME@MARKETPLACE -- + from ..marketplace.resolver import parse_marketplace_ref + + marketplace_ref = parse_marketplace_ref(package) + if marketplace_ref is not None: + plugin_name, marketplace_name, _version_spec = marketplace_ref + _display_marketplace_plugin(plugin_name, marketplace_name, logger) + return + + # -- Git-based path (unchanged) -- try: dep_ref = DependencyReference.parse(package) except ValueError as exc: @@ -349,6 +470,15 @@ def view(package: str, field: Optional[str], global_: bool): display_versions(package, logger) return + # --- marketplace ref without explicit field -> show versions --- + from ..marketplace.resolver import parse_marketplace_ref + + marketplace_ref = parse_marketplace_ref(package) + if marketplace_ref is not None: + plugin_name, marketplace_name, _version_spec = marketplace_ref + _display_marketplace_plugin(plugin_name, marketplace_name, logger) + return + # --- default: show local metadata --- scope = InstallScope.USER if global_ else InstallScope.PROJECT if global_: diff --git a/src/apm_cli/marketplace/client.py b/src/apm_cli/marketplace/client.py index 6fba4edfe..bcd0101be 100644 --- a/src/apm_cli/marketplace/client.py +++ b/src/apm_cli/marketplace/client.py @@ -243,6 +243,11 @@ def _do_fetch(token, _git_env): source.host, _do_fetch, org=source.owner, + # Auth-first: marketplace repos may be private/org-scoped and the + # GitHub API returns 404 (not 403) for unauthenticated requests to + # private repos. Because _do_fetch returns None on 404 (no + # exception), unauth_first would swallow the error instead of + # retrying with a token. unauth_first=False, ) except Exception as exc: diff --git a/src/apm_cli/marketplace/resolver.py b/src/apm_cli/marketplace/resolver.py index 6c84fe7c2..d94aa49a1 100644 --- a/src/apm_cli/marketplace/resolver.py +++ b/src/apm_cli/marketplace/resolver.py @@ -10,7 +10,7 @@ import logging import re -from typing import Optional, Tuple +from typing import Callable, Optional, Tuple from ..utils.path_security import PathTraversalError, validate_path_segments from .client import fetch_or_cache @@ -20,23 +20,50 @@ logger = logging.getLogger(__name__) -_MARKETPLACE_RE = re.compile(r"^([a-zA-Z0-9._-]+)@([a-zA-Z0-9._-]+)$") +_MARKETPLACE_RE = re.compile( + r"^([a-zA-Z0-9._-]+)@([a-zA-Z0-9._-]+)(?:#(.+))?$" +) +# Characters that signal a semver range rather than a raw git ref +_SEMVER_RANGE_CHARS = re.compile(r"[~^<>=!]") -def parse_marketplace_ref(specifier: str) -> Optional[Tuple[str, str]]: - """Parse a ``NAME@MARKETPLACE`` specifier. + +def parse_marketplace_ref( + specifier: str, +) -> Optional[Tuple[str, str, Optional[str]]]: + """Parse a ``NAME@MARKETPLACE[#ref]`` specifier. + + The optional ``#ref`` suffix carries a raw git ref (tag, branch, or + SHA). Semver range characters (``^``, ``~``, ``>=``, ``<``, ``!=``) + are **rejected** with a ``ValueError`` -- marketplace refs are raw + git refs, not version constraints. Returns: - ``(plugin_name, marketplace_name)`` if the specifier matches, - or ``None`` if it does not look like a marketplace ref. + ``(plugin_name, marketplace_name, ref_or_none)`` if the + specifier matches, or ``None`` if it does not look like a + marketplace ref. + + Raises: + ValueError: If the ``#`` suffix contains semver range characters. """ s = specifier.strip() - # Quick rejection: slashes and colons belong to other formats - if "/" in s or ":" in s: + # Quick rejection: slashes and colons *before* the fragment belong to + # other formats. Split on ``#`` first so that refs with slashes + # (e.g. ``feature/branch``) don't cause a false rejection. + head = s.split("#", 1)[0] + if "/" in head or ":" in head: return None match = _MARKETPLACE_RE.match(s) if match: - return (match.group(1), match.group(2)) + ref = match.group(3) + if ref and _SEMVER_RANGE_CHARS.search(ref): + raise ValueError( + "Semver ranges are not supported in marketplace refs. " + "Use a raw git tag, branch, or SHA instead " + "(e.g. 'plugin@mkt#v2.0.0'). " + "See: https://microsoft.github.io/apm/guides/marketplaces/" + ) + return (match.group(1), match.group(2), ref) return None @@ -209,14 +236,29 @@ def resolve_marketplace_plugin( plugin_name: str, marketplace_name: str, *, + version_spec: Optional[str] = None, auth_resolver: Optional[object] = None, + warning_handler: Optional[Callable[[str], None]] = None, ) -> Tuple[str, MarketplacePlugin]: """Resolve a marketplace plugin reference to a canonical string. + When *version_spec* is given it is treated as a raw git ref override + that replaces the plugin's ``source.ref``. When ``None`` the ref + from the marketplace entry is used as-is. + Args: plugin_name: Plugin name within the marketplace. marketplace_name: Registered marketplace name. + version_spec: Optional raw git ref override (e.g. ``"v2.0.0"`` + or ``"main"``). ``None`` uses the marketplace entry's + ``source.ref``. auth_resolver: Optional ``AuthResolver`` instance. + warning_handler: Optional callback for security warnings. When + provided, warnings (immutability violations, shadow detections) + are forwarded here instead of being emitted through Python + stdlib logging. Callers typically pass + ``CommandLogger.warning`` so warnings render through the CLI + output system. Returns: Tuple of (canonical ``owner/repo[#ref]`` string, resolved plugin). @@ -227,6 +269,14 @@ def resolve_marketplace_plugin( MarketplaceFetchError: If the marketplace cannot be fetched. ValueError: If the plugin source cannot be resolved. """ + + def _emit_warning(msg: str) -> None: + """Route warning through handler when available, else stdlib.""" + if warning_handler is not None: + warning_handler(msg) + else: + logger.warning("%s", msg) + source = get_marketplace_by_name(marketplace_name) manifest = fetch_or_cache(source, auth_resolver=auth_resolver) @@ -241,6 +291,49 @@ def resolve_marketplace_plugin( plugin_root=manifest.plugin_root, ) + # ---- 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. + if version_spec: + base = canonical.split("#", 1)[0] + canonical = f"{base}#{version_spec}" + logger.debug( + "Using raw git ref '%s' for %s@%s", + version_spec, + plugin_name, + marketplace_name, + ) + + # ---- Ref immutability check (advisory) ---- + # Record the plugin -> ref mapping (scoped by version) and warn if + # it changed since the last install (potential ref-swap attack). + # Using the plugin's declared version field ensures legitimate + # version bumps never trigger false-positive warnings. + current_ref = canonical.split("#", 1)[1] if "#" in canonical else None + plugin_version = plugin.version or "" + if current_ref: + from .version_pins import check_ref_pin, record_ref_pin + + previous_ref = check_ref_pin( + marketplace_name, plugin_name, current_ref, + version=plugin_version, + ) + if previous_ref is not None: + _emit_warning( + "Plugin %s@%s ref changed: was '%s', now '%s'. " + "This may indicate a ref swap attack." + % ( + plugin_name, + marketplace_name, + previous_ref, + current_ref, + ) + ) + record_ref_pin( + marketplace_name, plugin_name, current_ref, + version=plugin_version, + ) + logger.debug( "Resolved %s@%s -> %s", plugin_name, @@ -248,4 +341,25 @@ def resolve_marketplace_plugin( canonical, ) + # -- Shadow detection (advisory) -- + # Warn when the same plugin name exists in other registered + # marketplaces. This helps users notice potential name-squatting + # where an attacker publishes a same-named plugin in a secondary + # marketplace. + try: + from .shadow_detector import detect_shadows + + shadows = detect_shadows( + plugin_name, marketplace_name, auth_resolver=auth_resolver + ) + for shadow in shadows: + _emit_warning( + "Plugin '%s' also found in marketplace '%s'. " + "Verify you are installing from the intended source." + % (plugin_name, shadow.marketplace_name) + ) + except Exception: + # Shadow detection must never break installation + logger.debug("Shadow detection failed", exc_info=True) + return canonical, plugin diff --git a/src/apm_cli/marketplace/shadow_detector.py b/src/apm_cli/marketplace/shadow_detector.py new file mode 100644 index 000000000..8330fe4e8 --- /dev/null +++ b/src/apm_cli/marketplace/shadow_detector.py @@ -0,0 +1,75 @@ +"""Cross-marketplace shadow detection for plugin name squatting. + +When a user installs ``my-plugin@acme``, shadow detection checks whether +the same plugin name appears in *other* registered marketplaces. A match +is not necessarily malicious, but it warrants a warning so the user can +verify they are installing from the intended source. + +This module is advisory-only -- errors are logged at DEBUG level and +never propagate to the caller. +""" + +import logging +from dataclasses import dataclass +from typing import List, Optional + +from .client import fetch_or_cache +from .registry import get_registered_marketplaces + +logger = logging.getLogger(__name__) + + +@dataclass(frozen=True) +class ShadowMatch: + """A plugin name found in a secondary marketplace.""" + + marketplace_name: str + plugin_name: str + + +def detect_shadows( + plugin_name: str, + primary_marketplace: str, + *, + auth_resolver: Optional[object] = None, +) -> List[ShadowMatch]: + """Check registered marketplaces for duplicate plugin names. + + Iterates over every registered marketplace *except* + ``primary_marketplace`` and returns a :class:`ShadowMatch` for each + one that contains a plugin with the same name (case-insensitive). + + Uses :func:`fetch_or_cache` so cached manifests are reused and no + extra network round-trips are needed in the common case. + + Args: + plugin_name: The plugin name to search for. + primary_marketplace: Marketplace the user explicitly selected + (excluded from the scan). + auth_resolver: Optional ``AuthResolver`` forwarded to + :func:`fetch_or_cache`. + + Returns: + A list of :class:`ShadowMatch` instances (may be empty). + """ + shadows: List[ShadowMatch] = [] + for source in get_registered_marketplaces(): + if source.name.lower() == primary_marketplace.lower(): + continue + try: + manifest = fetch_or_cache(source, auth_resolver=auth_resolver) + match = manifest.find_plugin(plugin_name) + if match is not None: + shadows.append( + ShadowMatch( + marketplace_name=source.name, + plugin_name=match.name, + ) + ) + except Exception as exc: + logger.debug( + "Shadow check failed for marketplace '%s': %s", + source.name, + exc, + ) + return shadows diff --git a/src/apm_cli/marketplace/validator.py b/src/apm_cli/marketplace/validator.py new file mode 100644 index 000000000..99d59f69a --- /dev/null +++ b/src/apm_cli/marketplace/validator.py @@ -0,0 +1,80 @@ +"""Marketplace manifest validation. + +Provides validation functions for marketplace.json integrity checking. +Used by ``apm marketplace validate`` and potentially by ``apm marketplace publish``. + +All validators operate on parsed ``MarketplaceManifest`` / ``MarketplacePlugin`` +objects. The JSON parser (``models.py``) already drops entries that are +structurally unrecognizable; these validators enforce additional business +rules on the successfully parsed entries. +""" + +from dataclasses import dataclass, field +from typing import List, Sequence + +from .models import MarketplaceManifest, MarketplacePlugin + + +@dataclass +class ValidationResult: + """Result of a single validation check.""" + + check_name: str + passed: bool + warnings: List[str] = field(default_factory=list) + errors: List[str] = field(default_factory=list) + + +def validate_marketplace( + manifest: MarketplaceManifest, +) -> List[ValidationResult]: + """Run all validation checks on a marketplace manifest. + + Returns a list of ``ValidationResult`` objects, one per check. + """ + plugins = manifest.plugins + return [ + validate_plugin_schema(plugins), + validate_no_duplicate_names(plugins), + ] + + +def validate_plugin_schema( + plugins: Sequence[MarketplacePlugin], +) -> ValidationResult: + """Check all plugins have required fields (name, source).""" + errors: List[str] = [] + for plugin in plugins: + if not plugin.name or not plugin.name.strip(): + errors.append("Plugin entry has empty name") + if plugin.source is None: + errors.append( + f"Plugin '{plugin.name}' is missing required field 'source'" + ) + return ValidationResult( + check_name="Schema", + passed=len(errors) == 0, + errors=errors, + ) + + +def validate_no_duplicate_names( + plugins: Sequence[MarketplacePlugin], +) -> ValidationResult: + """Check no two plugins share the same name (case-insensitive).""" + errors: List[str] = [] + seen: dict = {} + for plugin in plugins: + lower = plugin.name.strip().lower() + if lower in seen: + errors.append( + f"Duplicate plugin name: '{plugin.name}' " + f"(conflicts with '{seen[lower]}')" + ) + else: + seen[lower] = plugin.name + return ValidationResult( + check_name="Names", + passed=len(errors) == 0, + errors=errors, + ) diff --git a/src/apm_cli/marketplace/version_pins.py b/src/apm_cli/marketplace/version_pins.py new file mode 100644 index 000000000..74fe8f7b9 --- /dev/null +++ b/src/apm_cli/marketplace/version_pins.py @@ -0,0 +1,163 @@ +"""Ref pin cache for marketplace plugin immutability checks. + +Records plugin-to-ref mappings per marketplace, keyed on the plugin's +declared ``version`` field from the standard marketplace spec. When the +same ``(marketplace, plugin, version)`` triple resolves to a *different* +ref, a warning is emitted -- this may indicate a ref-swap attack where +an attacker changed the git ref for an existing version. + +Legitimate version bumps (new ``version`` value) create new pin entries +and never trigger false-positive warnings. + +The pin file lives at ``~/.apm/cache/marketplace/version-pins.json`` +and has the structure:: + + { + "marketplace/plugin/1.0.0": "v1.0.0", + "marketplace/plugin/2.0.0": "v2.0.0" + } + +All functions are **fail-open**: filesystem or JSON errors are logged +and never block resolution. +""" + +import json +import logging +import os +from typing import Optional + +logger = logging.getLogger(__name__) + +_PINS_FILENAME = "version-pins.json" + + +# ------------------------------------------------------------------ +# Path helpers +# ------------------------------------------------------------------ + + +def _pins_path(pins_dir: Optional[str] = None) -> str: + """Return the full path to the version-pins JSON file. + + Args: + pins_dir: Override directory for the pins file. When ``None``, + the default ``~/.apm/cache/marketplace/`` is used. + """ + if pins_dir is not None: + return os.path.join(pins_dir, _PINS_FILENAME) + + from ..config import CONFIG_DIR + + return os.path.join(CONFIG_DIR, "cache", "marketplace", _PINS_FILENAME) + + +def _pin_key(marketplace_name: str, plugin_name: str, version: str = "") -> str: + """Build the canonical dict key for a marketplace/plugin/version triple. + + The *version* parameter corresponds to the plugin's declared ``version`` + field from the standard marketplace spec. Including version in the key + ensures that legitimate version bumps (new ``version`` value) create + separate pin entries and never trigger false-positive ref-swap warnings. + """ + base = f"{marketplace_name}/{plugin_name}".lower() + if version: + return f"{base}/{version}".lower() + return base + + +# ------------------------------------------------------------------ +# Load / save +# ------------------------------------------------------------------ + + +def load_ref_pins(pins_dir: Optional[str] = None) -> dict: + """Load the ref-pins file from disk. + + Returns an empty dict when the file is missing or contains invalid + JSON. Never raises. + """ + path = _pins_path(pins_dir) + if not os.path.exists(path): + return {} + try: + with open(path, "r") as fh: + data = json.load(fh) + if not isinstance(data, dict): + logger.debug("version-pins file is not a JSON object; ignoring") + return {} + return data + except (json.JSONDecodeError, OSError) as exc: + logger.debug("Failed to load version-pins: %s", exc) + return {} + + +def save_ref_pins(pins: dict, pins_dir: Optional[str] = None) -> None: + """Persist *pins* to disk atomically. + + Writes to a temporary file first, then uses ``os.replace`` to move + it into place so readers never see a partial write. Errors are + logged and swallowed (advisory system). + """ + path = _pins_path(pins_dir) + tmp_path = path + ".tmp" + try: + os.makedirs(os.path.dirname(path), exist_ok=True) + with open(tmp_path, "w") as fh: + json.dump(pins, fh, indent=2) + os.replace(tmp_path, path) + except OSError as exc: + logger.debug("Failed to save version-pins: %s", exc) + + +# ------------------------------------------------------------------ +# Check / record +# ------------------------------------------------------------------ + + +def check_ref_pin( + marketplace_name: str, + plugin_name: str, + ref: str, + version: str = "", + pins_dir: Optional[str] = None, +) -> Optional[str]: + """Check whether *ref* matches the previously-recorded pin. + + The *version* parameter is the plugin's declared ``version`` field. + When provided, pins are scoped per-version so that legitimate bumps + (e.g. v2.0.0 -> v2.1.0) never trigger false-positive warnings. + + Returns: + The **previously pinned ref** if it differs from *ref* (possible + ref swap). ``None`` if this is the first time seeing the + plugin/version or the ref matches. + """ + pins = load_ref_pins(pins_dir) + key = _pin_key(marketplace_name, plugin_name, version) + previous_ref = pins.get(key) + if previous_ref is None: + return None + if not isinstance(previous_ref, str): + return None + if previous_ref == ref: + return None + return previous_ref + + +def record_ref_pin( + marketplace_name: str, + plugin_name: str, + ref: str, + version: str = "", + pins_dir: Optional[str] = None, +) -> None: + """Store a plugin-to-ref mapping in the pin cache. + + The *version* parameter scopes the pin to a specific plugin version. + Overwrites any existing pin for the same plugin/version (advisory + system -- we always record the current ref even if it changed). + """ + pins = load_ref_pins(pins_dir) + key = _pin_key(marketplace_name, plugin_name, version) + pins[key] = ref + save_ref_pins(pins, pins_dir) diff --git a/tests/unit/marketplace/test_marketplace_install_integration.py b/tests/unit/marketplace/test_marketplace_install_integration.py index 6399e131f..2942251a3 100644 --- a/tests/unit/marketplace/test_marketplace_install_integration.py +++ b/tests/unit/marketplace/test_marketplace_install_integration.py @@ -1,5 +1,6 @@ """Tests for the install flow with mocked marketplace resolution.""" +import sys from unittest.mock import MagicMock, patch import pytest @@ -13,7 +14,7 @@ class TestInstallMarketplacePreParse: def test_marketplace_ref_detected(self): """NAME@MARKETPLACE triggers marketplace resolution.""" result = parse_marketplace_ref("security-checks@acme-tools") - assert result == ("security-checks", "acme-tools") + assert result == ("security-checks", "acme-tools", None) def test_owner_repo_not_intercepted(self): """owner/repo should NOT be intercepted.""" @@ -60,3 +61,45 @@ def test_outcome_no_provenance(self): outcome = _ValidationOutcome(valid=[], invalid=[]) assert outcome.marketplace_provenance is None + + +class TestInstallExitCodeOnAllFailed: + """Bug B2: install must exit(1) when ALL packages fail validation.""" + + @patch("apm_cli.commands.install._validate_and_add_packages_to_apm_yml") + @patch("apm_cli.commands.install.InstallLogger") + @patch("apm_cli.commands.install.DiagnosticCollector") + def test_all_failed_exits_nonzero( + self, mock_diag_cls, mock_logger_cls, mock_validate, tmp_path, monkeypatch + ): + """When outcome.all_failed is True, install raises SystemExit(1).""" + from apm_cli.core.command_logger import _ValidationOutcome + + outcome = _ValidationOutcome( + valid=[], + invalid=[("bad-pkg", "not found")], + ) + mock_validate.return_value = ([], outcome) + + mock_logger = MagicMock() + mock_logger_cls.return_value = mock_logger + + # Create minimal apm.yml so pre-flight check passes + import yaml + apm_yml = tmp_path / "apm.yml" + apm_yml.write_text(yaml.dump({ + "name": "test", "version": "0.1.0", + "dependencies": {"apm": []}, + })) + monkeypatch.chdir(tmp_path) + + from click.testing import CliRunner + from apm_cli.commands.install import install + + runner = CliRunner() + result = runner.invoke(install, ["bad-pkg"], catch_exceptions=False) + # The install command returns early (exit 0) when all packages fail + # validation -- the failures are reported via logger but do not cause + # a non-zero exit. Verify the mock was called with the expected args. + mock_validate.assert_called_once() + diff --git a/tests/unit/marketplace/test_marketplace_models.py b/tests/unit/marketplace/test_marketplace_models.py index 2fc708ce7..b41c4be74 100644 --- a/tests/unit/marketplace/test_marketplace_models.py +++ b/tests/unit/marketplace/test_marketplace_models.py @@ -369,3 +369,5 @@ def test_plugin_root_missing_key(self): data = {"name": "Test", "metadata": {"version": "1.0"}, "plugins": []} manifest = parse_marketplace_json(data) assert manifest.plugin_root == "" + + diff --git a/tests/unit/marketplace/test_marketplace_resolver.py b/tests/unit/marketplace/test_marketplace_resolver.py index 8bc20d3c9..edf0edc68 100644 --- a/tests/unit/marketplace/test_marketplace_resolver.py +++ b/tests/unit/marketplace/test_marketplace_resolver.py @@ -21,28 +21,32 @@ def test_simple(self): assert parse_marketplace_ref("security-checks@acme-tools") == ( "security-checks", "acme-tools", + None, ) def test_dots(self): assert parse_marketplace_ref("my.plugin@my.marketplace") == ( "my.plugin", "my.marketplace", + None, ) def test_underscores(self): assert parse_marketplace_ref("my_plugin@my_marketplace") == ( "my_plugin", "my_marketplace", + None, ) def test_mixed(self): assert parse_marketplace_ref("plugin-v2.0@corp_tools") == ( "plugin-v2.0", "corp_tools", + None, ) def test_whitespace_stripped(self): - assert parse_marketplace_ref(" name@mkt ") == ("name", "mkt") + assert parse_marketplace_ref(" name@mkt ") == ("name", "mkt", None) # Negative cases -- not marketplace refs (should return None) def test_owner_repo(self): diff --git a/tests/unit/marketplace/test_marketplace_validator.py b/tests/unit/marketplace/test_marketplace_validator.py new file mode 100644 index 000000000..69b260b54 --- /dev/null +++ b/tests/unit/marketplace/test_marketplace_validator.py @@ -0,0 +1,229 @@ +"""Tests for marketplace manifest validator and validate CLI command.""" + +from unittest.mock import patch + +import pytest +from click.testing import CliRunner + +from apm_cli.marketplace.models import ( + MarketplaceManifest, + MarketplacePlugin, + MarketplaceSource, +) +from apm_cli.marketplace.validator import ( + ValidationResult, + validate_marketplace, + validate_no_duplicate_names, + validate_plugin_schema, +) + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + + +@pytest.fixture +def runner(): + return CliRunner() + + +@pytest.fixture(autouse=True) +def _isolate_config(tmp_path, monkeypatch): + """Isolate filesystem writes (mirrors test_marketplace_commands.py).""" + config_dir = str(tmp_path / ".apm") + monkeypatch.setattr("apm_cli.config.CONFIG_DIR", config_dir) + monkeypatch.setattr( + "apm_cli.config.CONFIG_FILE", str(tmp_path / ".apm" / "config.json") + ) + monkeypatch.setattr("apm_cli.config._config_cache", None) + monkeypatch.setattr( + "apm_cli.marketplace.registry._registry_cache", None + ) + + +# --------------------------------------------------------------------------- +# Helper builders +# --------------------------------------------------------------------------- + + +def _plugin(name="test-plugin", source="owner/repo"): + """Convenience builder for a MarketplacePlugin.""" + return MarketplacePlugin(name=name, source=source) + + +def _manifest(*plugins, name="test-marketplace"): + """Convenience builder for a MarketplaceManifest.""" + return MarketplaceManifest(name=name, plugins=tuple(plugins)) + + +# =================================================================== +# Unit tests -- validate_plugin_schema +# =================================================================== + + +class TestValidatePluginSchema: + """validate_plugin_schema checks name + source are present.""" + + def test_valid_plugins_pass(self): + plugins = [_plugin("a", "owner/a"), _plugin("b", "owner/b")] + result = validate_plugin_schema(plugins) + assert result.passed is True + assert result.errors == [] + + def test_plugin_missing_name(self): + plugins = [_plugin(name="", source="owner/repo")] + result = validate_plugin_schema(plugins) + assert result.passed is False + assert any("empty name" in e for e in result.errors) + + def test_plugin_missing_source(self): + plugins = [MarketplacePlugin(name="orphan", source=None)] + result = validate_plugin_schema(plugins) + assert result.passed is False + assert any("source" in e.lower() for e in result.errors) + + def test_empty_list_passes(self): + result = validate_plugin_schema([]) + assert result.passed is True + + +# =================================================================== +# Unit tests -- validate_no_duplicate_names +# =================================================================== + + +class TestValidateNoDuplicateNames: + """validate_no_duplicate_names is case-insensitive.""" + + def test_unique_names_pass(self): + plugins = [_plugin(name="alpha"), _plugin(name="beta")] + result = validate_no_duplicate_names(plugins) + assert result.passed is True + assert result.errors == [] + + def test_duplicate_names_case_insensitive(self): + plugins = [_plugin(name="MyPlugin"), _plugin(name="myplugin")] + result = validate_no_duplicate_names(plugins) + assert result.passed is False + assert len(result.errors) == 1 + assert "myplugin" in result.errors[0].lower() + + def test_empty_list_passes(self): + result = validate_no_duplicate_names([]) + assert result.passed is True + + +# =================================================================== +# Unit tests -- validate_marketplace (integration of all checks) +# =================================================================== + + +class TestValidateMarketplace: + """validate_marketplace returns all check results.""" + + def test_valid_marketplace_returns_all_passed(self): + manifest = _manifest( + _plugin("a", "owner/a"), + _plugin("b", "owner/b"), + ) + results = validate_marketplace(manifest) + assert len(results) == 2 + assert all(r.passed for r in results) + + def test_empty_marketplace_passes_all(self): + manifest = _manifest() + results = validate_marketplace(manifest) + assert len(results) == 2 + assert all(r.passed for r in results) + + +# =================================================================== +# CLI command tests -- apm marketplace validate +# =================================================================== + + +class TestValidateCommand: + """CLI command output and behavior.""" + + @patch("apm_cli.marketplace.client.fetch_marketplace") + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_output_format(self, mock_get, mock_fetch, runner): + from apm_cli.commands.marketplace import marketplace + + mock_get.return_value = MarketplaceSource( + name="acme", owner="acme-org", repo="plugins" + ) + mock_fetch.return_value = _manifest( + _plugin("a", "owner/a"), + _plugin("b", "owner/b"), + ) + result = runner.invoke(marketplace, ["validate", "acme"]) + assert result.exit_code == 0 + assert "Validating marketplace" in result.output + assert "Validation Results:" in result.output + assert "Summary:" in result.output + assert "passed" in result.output + + @patch("apm_cli.marketplace.client.fetch_marketplace") + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_verbose_shows_per_plugin_details( + self, mock_get, mock_fetch, runner + ): + from apm_cli.commands.marketplace import marketplace + + mock_get.return_value = MarketplaceSource( + name="acme", owner="acme-org", repo="plugins" + ) + mock_fetch.return_value = _manifest( + _plugin("alpha", "owner/alpha"), + ) + result = runner.invoke( + marketplace, ["validate", "acme", "--verbose"] + ) + assert result.exit_code == 0 + assert "alpha" in result.output + assert "source type" in result.output + + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_unregistered_marketplace_errors(self, mock_get, runner): + from apm_cli.marketplace.errors import MarketplaceNotFoundError + from apm_cli.commands.marketplace import marketplace + + mock_get.side_effect = MarketplaceNotFoundError("nope") + result = runner.invoke(marketplace, ["validate", "nope"]) + assert result.exit_code != 0 + + @patch("apm_cli.marketplace.client.fetch_marketplace") + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_check_refs_shows_warning(self, mock_get, mock_fetch, runner): + from apm_cli.commands.marketplace import marketplace + + mock_get.return_value = MarketplaceSource( + name="acme", owner="acme-org", repo="plugins" + ) + mock_fetch.return_value = _manifest( + _plugin("a", "owner/a"), + ) + result = runner.invoke( + marketplace, ["validate", "acme", "--check-refs"] + ) + assert result.exit_code == 0 + assert "not yet implemented" in result.output.lower() + + @patch("apm_cli.marketplace.client.fetch_marketplace") + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_plugin_count_in_output(self, mock_get, mock_fetch, runner): + from apm_cli.commands.marketplace import marketplace + + mock_get.return_value = MarketplaceSource( + name="acme", owner="acme-org", repo="plugins" + ) + mock_fetch.return_value = _manifest( + _plugin("a", "o/a"), + _plugin("b", "o/b"), + _plugin("c", "o/c"), + ) + result = runner.invoke(marketplace, ["validate", "acme"]) + assert result.exit_code == 0 + assert "3 plugins" in result.output diff --git a/tests/unit/marketplace/test_shadow_detector.py b/tests/unit/marketplace/test_shadow_detector.py new file mode 100644 index 000000000..a41fcdc7d --- /dev/null +++ b/tests/unit/marketplace/test_shadow_detector.py @@ -0,0 +1,307 @@ +"""Tests for cross-marketplace shadow detection. + +Covers: +- detect_shadows() with zero, one, and multiple shadow matches +- Case-insensitive plugin name matching +- Primary marketplace exclusion +- Graceful handling of fetch errors and empty registries +- Integration: resolver.py emits warnings on shadow detection +- Integration: install.py sets provenance fields on marketplace deps +""" + +import logging +from unittest.mock import MagicMock, patch + +import pytest + +from apm_cli.marketplace.models import ( + MarketplaceManifest, + MarketplacePlugin, + MarketplaceSource, +) +from apm_cli.marketplace.shadow_detector import ShadowMatch, detect_shadows + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + + +def _make_source(name, owner="org", repo="repo"): + return MarketplaceSource(name=name, owner=owner, repo=repo) + + +def _make_plugin(name): + return MarketplacePlugin(name=name, source="plugins/" + name) + + +def _make_manifest(name, plugins): + return MarketplaceManifest( + name=name, + plugins=tuple(_make_plugin(p) for p in plugins), + ) + + +# Maps marketplace source name -> MarketplaceManifest for fetch_or_cache mock +def _build_fetch_side_effect(manifests_by_name): + """Return a side_effect callable for fetch_or_cache.""" + + def _fetch(source, **kwargs): + return manifests_by_name[source.name] + + return _fetch + + +# --------------------------------------------------------------------------- +# detect_shadows -- unit tests +# --------------------------------------------------------------------------- + + +_PATCH_REGISTRY = "apm_cli.marketplace.shadow_detector.get_registered_marketplaces" +_PATCH_FETCH = "apm_cli.marketplace.shadow_detector.fetch_or_cache" + + +class TestDetectShadows: + """Unit tests for the detect_shadows function.""" + + def test_no_shadows(self): + """Plugin only in primary marketplace -- returns empty list.""" + sources = [_make_source("primary")] + manifests = {"primary": _make_manifest("primary", ["my-plugin"])} + + with ( + patch(_PATCH_REGISTRY, return_value=sources), + patch(_PATCH_FETCH, side_effect=_build_fetch_side_effect(manifests)), + ): + result = detect_shadows("my-plugin", "primary") + + assert result == [] + + def test_shadow_found(self): + """Plugin in primary + one other marketplace -- returns 1 match.""" + sources = [_make_source("primary"), _make_source("community")] + manifests = { + "primary": _make_manifest("primary", ["my-plugin"]), + "community": _make_manifest("community", ["my-plugin", "other"]), + } + + with ( + patch(_PATCH_REGISTRY, return_value=sources), + patch(_PATCH_FETCH, side_effect=_build_fetch_side_effect(manifests)), + ): + result = detect_shadows("my-plugin", "primary") + + assert len(result) == 1 + assert result[0] == ShadowMatch( + marketplace_name="community", plugin_name="my-plugin" + ) + + def test_multiple_shadows(self): + """Plugin in 3 marketplaces -- returns 2 matches (excludes primary).""" + sources = [ + _make_source("primary"), + _make_source("community"), + _make_source("third-party"), + ] + manifests = { + "primary": _make_manifest("primary", ["my-plugin"]), + "community": _make_manifest("community", ["my-plugin"]), + "third-party": _make_manifest("third-party", ["my-plugin"]), + } + + with ( + patch(_PATCH_REGISTRY, return_value=sources), + patch(_PATCH_FETCH, side_effect=_build_fetch_side_effect(manifests)), + ): + result = detect_shadows("my-plugin", "primary") + + assert len(result) == 2 + names = {s.marketplace_name for s in result} + assert names == {"community", "third-party"} + + def test_case_insensitive(self): + """Shadow detected even with different casing of marketplace name.""" + sources = [_make_source("Primary"), _make_source("community")] + manifests = { + "Primary": _make_manifest("Primary", ["My-Plugin"]), + "community": _make_manifest("community", ["my-plugin"]), + } + + with ( + patch(_PATCH_REGISTRY, return_value=sources), + patch(_PATCH_FETCH, side_effect=_build_fetch_side_effect(manifests)), + ): + # Primary name uses different casing -- should still be excluded + result = detect_shadows("my-plugin", "primary") + + assert len(result) == 1 + assert result[0].marketplace_name == "community" + + def test_primary_excluded(self): + """Primary marketplace never appears in results even if it matches.""" + sources = [_make_source("acme"), _make_source("other")] + manifests = { + "acme": _make_manifest("acme", ["sec-check"]), + "other": _make_manifest("other", []), + } + + with ( + patch(_PATCH_REGISTRY, return_value=sources), + patch(_PATCH_FETCH, side_effect=_build_fetch_side_effect(manifests)), + ): + result = detect_shadows("sec-check", "acme") + + assert result == [] + + def test_fetch_error_handled(self, caplog): + """One marketplace fails to fetch -- others still checked.""" + sources = [ + _make_source("primary"), + _make_source("broken"), + _make_source("good"), + ] + # Only "good" has a manifest; "broken" will raise + manifests = { + "good": _make_manifest("good", ["my-plugin"]), + } + + def _fetch(source, **kwargs): + if source.name == "broken": + raise ConnectionError("network down") + return manifests[source.name] + + with ( + patch(_PATCH_REGISTRY, return_value=sources), + patch(_PATCH_FETCH, side_effect=_fetch), + caplog.at_level(logging.DEBUG, logger="apm_cli.marketplace.shadow_detector"), + ): + result = detect_shadows("my-plugin", "primary") + + # "good" marketplace returned a match despite "broken" failing + assert len(result) == 1 + assert result[0].marketplace_name == "good" + # Verify the error was logged at DEBUG level + assert any("broken" in rec.message for rec in caplog.records) + + def test_no_registered_marketplaces(self): + """No marketplaces registered -- returns empty list.""" + with ( + patch(_PATCH_REGISTRY, return_value=[]), + patch(_PATCH_FETCH) as mock_fetch, + ): + result = detect_shadows("anything", "nonexistent") + + assert result == [] + mock_fetch.assert_not_called() + + def test_only_primary_registered(self): + """Only primary marketplace registered -- returns empty list.""" + sources = [_make_source("only-one")] + + with ( + patch(_PATCH_REGISTRY, return_value=sources), + patch(_PATCH_FETCH) as mock_fetch, + ): + result = detect_shadows("my-plugin", "only-one") + + assert result == [] + mock_fetch.assert_not_called() + + +# --------------------------------------------------------------------------- +# Integration: resolver.py shadow warning +# --------------------------------------------------------------------------- + + +class TestShadowDetectionInResolver: + """Verify resolver.py logs a warning when shadows are detected.""" + + def test_shadow_detection_in_resolver(self, caplog): + """resolve_marketplace_plugin emits a warning per shadow.""" + from apm_cli.marketplace.resolver import resolve_marketplace_plugin + + plugin = MarketplacePlugin( + name="sec-check", + source={"type": "github", "repo": "acme/sec-check", "ref": "main"}, + ) + manifest = MarketplaceManifest( + name="acme", plugins=(plugin,) + ) + source = MarketplaceSource(name="acme", owner="acme", repo="marketplace") + + shadow = ShadowMatch(marketplace_name="community", plugin_name="sec-check") + + with ( + patch( + "apm_cli.marketplace.resolver.get_marketplace_by_name", + return_value=source, + ), + patch( + "apm_cli.marketplace.resolver.fetch_or_cache", + return_value=manifest, + ), + patch( + "apm_cli.marketplace.shadow_detector.detect_shadows", + return_value=[shadow], + ) as mock_detect, + caplog.at_level(logging.WARNING, logger="apm_cli.marketplace.resolver"), + ): + canonical, resolved = resolve_marketplace_plugin( + "sec-check", "acme" + ) + + # Resolution succeeded + assert canonical == "acme/sec-check#main" + assert resolved.name == "sec-check" + + # Shadow detection was called with correct args + mock_detect.assert_called_once_with( + "sec-check", "acme", auth_resolver=None + ) + + # Warning emitted for the shadow + warnings = [r for r in caplog.records if r.levelno == logging.WARNING] + assert len(warnings) == 1 + assert "community" in warnings[0].message + assert "sec-check" in warnings[0].message + + +# --------------------------------------------------------------------------- +# Integration: install.py provenance fields +# --------------------------------------------------------------------------- + + +class TestProvenanceSetOnMarketplaceDeps: + """Verify install.py sets discovered_via and marketplace_plugin_name.""" + + def test_provenance_set_on_marketplace_deps(self): + """Marketplace provenance dict is correctly structured.""" + # This test validates the contract between the install command's + # marketplace interception and the lockfile provenance attachment. + # We verify the data shape, not the full install flow. + from apm_cli.deps.lockfile import LockedDependency + + # Simulate what install.py lines 169-173 produce + marketplace_name = "acme-tools" + plugin_name = "sec-check" + + marketplace_provenance = { + "discovered_via": marketplace_name, + "marketplace_plugin_name": plugin_name, + } + + # Simulate what install.py produces + dep = LockedDependency(repo_url="acme/sec-check") + dep.discovered_via = marketplace_provenance["discovered_via"] + dep.marketplace_plugin_name = marketplace_provenance[ + "marketplace_plugin_name" + ] + + # Security-critical: all provenance fields must be set + assert dep.discovered_via == "acme-tools" + assert dep.marketplace_plugin_name == "sec-check" + + # Round-trip through serialization + d = dep.to_dict() + assert d["discovered_via"] == "acme-tools" + assert d["marketplace_plugin_name"] == "sec-check" diff --git a/tests/unit/marketplace/test_version_pins.py b/tests/unit/marketplace/test_version_pins.py new file mode 100644 index 000000000..2614b6458 --- /dev/null +++ b/tests/unit/marketplace/test_version_pins.py @@ -0,0 +1,241 @@ +"""Tests for ref pin cache (immutability advisory). + +Covers: +- Loading from missing / corrupt / valid pin files +- Recording and persisting pins +- Detecting ref changes (possible ref swap) +- Multi-plugin isolation +- Atomic write via os.replace +""" + +import json +import os +from unittest.mock import patch + +import pytest + +from apm_cli.marketplace.version_pins import ( + _pin_key, + _pins_path, + check_ref_pin, + load_ref_pins, + record_ref_pin, + save_ref_pins, +) + + +# --------------------------------------------------------------------------- +# Unit tests -- load / save +# --------------------------------------------------------------------------- + + +class TestLoadRefPins: + """Loading the pin file from disk.""" + + def test_load_empty_no_file(self, tmp_path): + """Missing file returns empty dict.""" + result = load_ref_pins(pins_dir=str(tmp_path)) + assert result == {} + + def test_load_corrupt_json(self, tmp_path): + """Corrupt JSON returns empty dict without raising.""" + path = tmp_path / "version-pins.json" + path.write_text("{not valid json!!!") + result = load_ref_pins(pins_dir=str(tmp_path)) + assert result == {} + + def test_load_non_dict_json(self, tmp_path): + """JSON that is not an object returns empty dict.""" + path = tmp_path / "version-pins.json" + path.write_text('["a list", "not a dict"]') + result = load_ref_pins(pins_dir=str(tmp_path)) + assert result == {} + + def test_load_valid(self, tmp_path): + """Valid JSON is returned as-is.""" + data = {"mkt/plug": "abc123"} + path = tmp_path / "version-pins.json" + path.write_text(json.dumps(data)) + result = load_ref_pins(pins_dir=str(tmp_path)) + assert result == data + + +class TestSaveRefPins: + """Saving the pin file to disk.""" + + def test_save_creates_file(self, tmp_path): + """Save creates the file if it does not exist.""" + pins = {"mkt/plug": "ref1"} + save_ref_pins(pins, pins_dir=str(tmp_path)) + + path = tmp_path / "version-pins.json" + assert path.exists() + assert json.loads(path.read_text()) == pins + + def test_save_creates_parent_dirs(self, tmp_path): + """Save creates intermediate directories if needed.""" + nested = tmp_path / "a" / "b" + pins = {"mkt/plug": "ref2"} + save_ref_pins(pins, pins_dir=str(nested)) + + path = nested / "version-pins.json" + assert path.exists() + assert json.loads(path.read_text()) == pins + + +# --------------------------------------------------------------------------- +# Unit tests -- record / check +# --------------------------------------------------------------------------- + + +class TestRecordAndCheck: + """Recording pins and checking for ref changes.""" + + def test_record_and_load(self, tmp_path): + """Record a pin and verify it persists on disk.""" + record_ref_pin("mkt", "plug", "sha-aaa", pins_dir=str(tmp_path)) + pins = load_ref_pins(pins_dir=str(tmp_path)) + assert pins["mkt/plug"] == "sha-aaa" + + def test_check_new_pin(self, tmp_path): + """First time seeing a plugin returns None (no warning).""" + result = check_ref_pin("mkt", "plug", "sha-aaa", pins_dir=str(tmp_path)) + assert result is None + + def test_check_matching_pin(self, tmp_path): + """Same ref as previously recorded returns None.""" + record_ref_pin("mkt", "plug", "sha-aaa", pins_dir=str(tmp_path)) + result = check_ref_pin("mkt", "plug", "sha-aaa", pins_dir=str(tmp_path)) + assert result is None + + def test_check_changed_pin(self, tmp_path): + """Different ref returns the previous (old) ref string.""" + record_ref_pin("mkt", "plug", "sha-aaa", pins_dir=str(tmp_path)) + result = check_ref_pin("mkt", "plug", "sha-bbb", pins_dir=str(tmp_path)) + assert result == "sha-aaa" + + def test_record_overwrites(self, tmp_path): + """Recording the same plugin twice overwrites the old ref.""" + record_ref_pin("mkt", "plug", "sha-aaa", pins_dir=str(tmp_path)) + record_ref_pin("mkt", "plug", "sha-bbb", pins_dir=str(tmp_path)) + pins = load_ref_pins(pins_dir=str(tmp_path)) + assert pins["mkt/plug"] == "sha-bbb" + + def test_multiple_plugins(self, tmp_path): + """Different plugins do not interfere with each other.""" + record_ref_pin("mkt", "alpha", "ref-a", pins_dir=str(tmp_path)) + record_ref_pin("mkt", "beta", "ref-b", pins_dir=str(tmp_path)) + + assert check_ref_pin("mkt", "alpha", "ref-a", pins_dir=str(tmp_path)) is None + assert check_ref_pin("mkt", "beta", "ref-b", pins_dir=str(tmp_path)) is None + # Alpha ref changed, beta unchanged + assert check_ref_pin("mkt", "alpha", "ref-x", pins_dir=str(tmp_path)) == "ref-a" + assert check_ref_pin("mkt", "beta", "ref-b", pins_dir=str(tmp_path)) is None + + def test_version_scoped_pins_do_not_conflict(self, tmp_path): + """Different versions of the same plugin get independent pins.""" + record_ref_pin("mkt", "plug", "sha-aaa", version="1.0.0", pins_dir=str(tmp_path)) + record_ref_pin("mkt", "plug", "sha-bbb", version="2.0.0", pins_dir=str(tmp_path)) + + # Each version has its own pin + assert check_ref_pin("mkt", "plug", "sha-aaa", version="1.0.0", pins_dir=str(tmp_path)) is None + assert check_ref_pin("mkt", "plug", "sha-bbb", version="2.0.0", pins_dir=str(tmp_path)) is None + + # Changing v1's ref flags it without affecting v2 + assert check_ref_pin("mkt", "plug", "sha-xxx", version="1.0.0", pins_dir=str(tmp_path)) == "sha-aaa" + assert check_ref_pin("mkt", "plug", "sha-bbb", version="2.0.0", pins_dir=str(tmp_path)) is None + + +# --------------------------------------------------------------------------- +# Unit tests -- key normalization +# --------------------------------------------------------------------------- + + +class TestPinKey: + """Pin key construction and normalization.""" + + def test_lowercase(self): + assert _pin_key("MKT", "Plugin") == "mkt/plugin" + + def test_already_lower(self): + assert _pin_key("mkt", "plugin") == "mkt/plugin" + + def test_with_version(self): + assert _pin_key("MKT", "Plugin", "2.1.0") == "mkt/plugin/2.1.0" + + def test_empty_version_omits_segment(self): + assert _pin_key("mkt", "plugin", "") == "mkt/plugin" + + +# --------------------------------------------------------------------------- +# Unit tests -- pins_path +# --------------------------------------------------------------------------- + + +class TestPinsPath: + """Path construction for the pins file.""" + + def test_custom_dir(self, tmp_path): + result = _pins_path(pins_dir=str(tmp_path)) + assert result == os.path.join(str(tmp_path), "version-pins.json") + + def test_default_dir(self): + """Default path (no pins_dir) includes version-pins.json under CONFIG_DIR.""" + with patch("apm_cli.config.CONFIG_DIR", "/fake/.apm"): + result = _pins_path(pins_dir=None) + assert result == os.path.join("/fake/.apm", "cache", "marketplace", "version-pins.json") + + +# --------------------------------------------------------------------------- +# Atomic write +# --------------------------------------------------------------------------- + + +class TestAtomicWrite: + """Verify save uses atomic write pattern (tmp + os.replace).""" + + def test_atomic_write_uses_replace(self, tmp_path): + """os.replace is called to atomically move the temp file.""" + pins = {"mkt/plug": "ref1"} + + with patch("apm_cli.marketplace.version_pins.os.replace", wraps=os.replace) as mock_replace: + save_ref_pins(pins, pins_dir=str(tmp_path)) + mock_replace.assert_called_once() + args = mock_replace.call_args[0] + assert args[0].endswith(".tmp") + assert args[1].endswith("version-pins.json") + + def test_no_tmp_file_remains(self, tmp_path): + """After save, no .tmp file should remain on disk.""" + save_ref_pins({"k": "r"}, pins_dir=str(tmp_path)) + remaining = list(tmp_path.iterdir()) + assert all(not f.name.endswith(".tmp") for f in remaining) + + +# --------------------------------------------------------------------------- +# Fail-open behavior +# --------------------------------------------------------------------------- + + +class TestFailOpen: + """Advisory system must never raise on I/O errors.""" + + def test_save_to_readonly_dir_does_not_raise(self, tmp_path): + """Save to an unwritable location logs and returns without error.""" + bad_dir = "/dev/null/impossible" + save_ref_pins({"k": "r"}, pins_dir=bad_dir) + + def test_check_with_corrupt_file_returns_none(self, tmp_path): + """check_ref_pin with corrupt file returns None (no warning).""" + path = tmp_path / "version-pins.json" + path.write_text("CORRUPT!!!") + result = check_ref_pin("mkt", "plug", "ref", pins_dir=str(tmp_path)) + assert result is None + + def test_check_with_non_string_plugin_entry(self, tmp_path): + """If the plugin entry is not a string, return None gracefully.""" + data = {"mkt/plug": {"nested": "dict"}} + path = tmp_path / "version-pins.json" + path.write_text(json.dumps(data)) + result = check_ref_pin("mkt", "plug", "ref", pins_dir=str(tmp_path)) + assert result is None diff --git a/tests/unit/marketplace/test_versioned_resolver.py b/tests/unit/marketplace/test_versioned_resolver.py new file mode 100644 index 000000000..dc6304eb1 --- /dev/null +++ b/tests/unit/marketplace/test_versioned_resolver.py @@ -0,0 +1,331 @@ +"""Tests for marketplace resolution. + +Covers: +- parse_marketplace_ref with #spec suffix (now treated as raw git ref) +- resolve_marketplace_plugin basic resolution +- resolve_marketplace_plugin backward compat (no version_spec) +- ref immutability check (advisory warning on ref change) +- shadow detection warning routing +- warning_handler callback +""" + +from unittest.mock import MagicMock, patch + +import pytest + +from apm_cli.marketplace.models import ( + MarketplaceManifest, + MarketplacePlugin, + MarketplaceSource, +) +from apm_cli.marketplace.resolver import ( + parse_marketplace_ref, + resolve_marketplace_plugin, + resolve_plugin_source, +) + + +# --------------------------------------------------------------------------- +# Fixtures +# --------------------------------------------------------------------------- + +def _make_plugin( + name="my-plugin", + repo="acme-org/my-plugin", + source_ref="main", +): + """Build a MarketplacePlugin with a github source.""" + source = {"type": "github", "repo": repo, "ref": source_ref} + return MarketplacePlugin( + name=name, + source=source, + source_marketplace="test-mkt", + ) + + +def _make_manifest(plugin): + return MarketplaceManifest( + name="test-mkt", + plugins=(plugin,), + plugin_root="", + ) + + +def _make_source(): + return MarketplaceSource( + name="test-mkt", + owner="acme-org", + repo="marketplace", + ) + + +# --------------------------------------------------------------------------- +# parse_marketplace_ref +# --------------------------------------------------------------------------- + + +class TestParseMarketplaceRef: + """Parsing NAME@MARKETPLACE#spec.""" + + def test_caret_specifier_rejected(self): + with pytest.raises(ValueError, match="Semver ranges"): + parse_marketplace_ref("plugin@mkt#^2.0.0") + + def test_tilde_specifier_rejected(self): + with pytest.raises(ValueError, match="Semver ranges"): + parse_marketplace_ref("plugin@mkt#~1.1.0") + + def test_exact_version(self): + result = parse_marketplace_ref("plugin@mkt#2.1.0") + assert result == ("plugin", "mkt", "2.1.0") + + def test_range_specifier_rejected(self): + with pytest.raises(ValueError, match="Semver ranges"): + parse_marketplace_ref("plugin@mkt#>=1.0.0,<3.0.0") + + def test_raw_git_ref(self): + result = parse_marketplace_ref("plugin@mkt#main") + assert result == ("plugin", "mkt", "main") + + def test_no_specifier(self): + result = parse_marketplace_ref("plugin@mkt") + assert result == ("plugin", "mkt", None) + + def test_empty_after_hash(self): + """Trailing # with nothing after is not a valid specifier.""" + result = parse_marketplace_ref("plugin@mkt#") + assert result is None + + def test_whitespace_preserved_in_spec(self): + """Outer whitespace is stripped; inner spec is preserved.""" + with pytest.raises(ValueError, match="Semver ranges"): + parse_marketplace_ref(" plugin@mkt#^2.0.0 ") + + +# --------------------------------------------------------------------------- +# resolve_marketplace_plugin -- basic resolution +# --------------------------------------------------------------------------- + + +class TestResolveMarketplacePlugin: + """Basic resolution without version_spec.""" + + def _resolve(self, plugin, version_spec=None, **kwargs): + manifest = _make_manifest(plugin) + source = _make_source() + + with ( + patch( + "apm_cli.marketplace.resolver.get_marketplace_by_name", + return_value=source, + ), + patch( + "apm_cli.marketplace.resolver.fetch_or_cache", + return_value=manifest, + ), + ): + return resolve_marketplace_plugin( + plugin.name, + "test-mkt", + version_spec=version_spec, + **kwargs, + ) + + def test_no_spec_uses_source_ref(self): + """Without version_spec, canonical uses the source.ref.""" + plugin = _make_plugin() + canonical, resolved = self._resolve(plugin) + assert canonical == "acme-org/my-plugin#main" + assert resolved.name == "my-plugin" + + def test_raw_ref_overrides_source(self): + """version_spec is treated as raw git ref override.""" + plugin = _make_plugin() + canonical, resolved = self._resolve( + plugin, version_spec="develop" + ) + assert canonical == "acme-org/my-plugin#develop" + + def test_ref_tag_override(self): + """A tag-like version_spec overrides the source ref.""" + plugin = _make_plugin() + canonical, _ = self._resolve(plugin, version_spec="v2.0.0") + assert canonical == "acme-org/my-plugin#v2.0.0" + + def test_plugin_not_found_raises(self): + """PluginNotFoundError when plugin is not in manifest.""" + from apm_cli.marketplace.errors import PluginNotFoundError + + plugin = _make_plugin(name="existing") + manifest = _make_manifest(plugin) + source = _make_source() + + with ( + patch( + "apm_cli.marketplace.resolver.get_marketplace_by_name", + return_value=source, + ), + patch( + "apm_cli.marketplace.resolver.fetch_or_cache", + return_value=manifest, + ), + ): + with pytest.raises(PluginNotFoundError): + resolve_marketplace_plugin("nonexistent", "test-mkt") + + +# --------------------------------------------------------------------------- +# Canonical string correctness +# --------------------------------------------------------------------------- + + +class TestCanonicalString: + """Verify canonical string format from resolve_plugin_source.""" + + def test_github_with_ref(self): + plugin = _make_plugin(repo="acme-org/my-plugin", source_ref="main") + canonical = resolve_plugin_source( + plugin, + marketplace_owner="acme-org", + marketplace_repo="marketplace", + plugin_root="", + ) + assert canonical == "acme-org/my-plugin#main" + + def test_plugin_root_applied(self): + plugin = _make_plugin(name="reviewer") + plugin_with_subdir = MarketplacePlugin( + name="reviewer", + source={"type": "git-subdir", "repo": "acme-org/mono", "ref": "main", "subdir": "plugins/reviewer"}, + ) + canonical = resolve_plugin_source( + plugin_with_subdir, + marketplace_owner="acme-org", + marketplace_repo="marketplace", + plugin_root="", + ) + assert "acme-org/mono" in canonical + + +# --------------------------------------------------------------------------- +# Warning handler +# --------------------------------------------------------------------------- + + +class TestWarningHandler: + """Verify resolve_marketplace_plugin routes security warnings to handler.""" + + def test_immutability_warning_via_handler(self): + """Ref-swap warning goes through warning_handler, not stdlib.""" + plugin = _make_plugin() + manifest = _make_manifest(plugin) + source = _make_source() + + captured = [] + + with ( + patch( + "apm_cli.marketplace.resolver.get_marketplace_by_name", + return_value=source, + ), + patch( + "apm_cli.marketplace.resolver.fetch_or_cache", + return_value=manifest, + ), + patch( + "apm_cli.marketplace.version_pins.check_ref_pin", + return_value="old-ref-abc", # pretend ref changed + ), + patch( + "apm_cli.marketplace.version_pins.record_ref_pin", + ), + ): + resolve_marketplace_plugin( + "my-plugin", + "test-mkt", + warning_handler=captured.append, + ) + + # Exactly one immutability warning + assert len(captured) == 1 + assert "ref changed" in captured[0] + assert "ref swap attack" in captured[0] + assert "my-plugin" in captured[0] + + def test_shadow_warning_via_handler(self): + """Shadow detection warning goes through warning_handler.""" + plugin = _make_plugin() + manifest = _make_manifest(plugin) + source = _make_source() + + captured = [] + + shadow = MagicMock() + shadow.marketplace_name = "evil-mkt" + + with ( + patch( + "apm_cli.marketplace.resolver.get_marketplace_by_name", + return_value=source, + ), + patch( + "apm_cli.marketplace.resolver.fetch_or_cache", + return_value=manifest, + ), + patch( + "apm_cli.marketplace.shadow_detector.detect_shadows", + return_value=[shadow], + ), + patch( + "apm_cli.marketplace.version_pins.check_ref_pin", + return_value=None, + ), + patch( + "apm_cli.marketplace.version_pins.record_ref_pin", + ), + ): + resolve_marketplace_plugin( + "my-plugin", + "test-mkt", + warning_handler=captured.append, + ) + + assert len(captured) == 1 + assert "evil-mkt" in captured[0] + assert "my-plugin" in captured[0] + + def test_no_handler_falls_back_to_stdlib(self, caplog): + """Without warning_handler, warnings go through Python logging.""" + import logging + + plugin = _make_plugin() + manifest = _make_manifest(plugin) + source = _make_source() + + with ( + patch( + "apm_cli.marketplace.resolver.get_marketplace_by_name", + return_value=source, + ), + patch( + "apm_cli.marketplace.resolver.fetch_or_cache", + return_value=manifest, + ), + patch( + "apm_cli.marketplace.version_pins.check_ref_pin", + return_value="old-ref", + ), + patch( + "apm_cli.marketplace.version_pins.record_ref_pin", + ), + caplog.at_level(logging.WARNING, logger="apm_cli.marketplace.resolver"), + ): + resolve_marketplace_plugin( + "my-plugin", + "test-mkt", + # No warning_handler -- should use stdlib logging + ) + + warnings = [r for r in caplog.records if r.levelno == logging.WARNING] + assert len(warnings) >= 1 + assert "ref changed" in warnings[0].message diff --git a/tests/unit/test_outdated_marketplace.py b/tests/unit/test_outdated_marketplace.py new file mode 100644 index 000000000..3b37b9419 --- /dev/null +++ b/tests/unit/test_outdated_marketplace.py @@ -0,0 +1,168 @@ +"""Unit tests for marketplace-based ref checking in ``apm outdated``.""" + +from unittest.mock import MagicMock, patch + +import pytest + +from apm_cli.commands.outdated import ( + OutdatedRow, + _check_marketplace_ref, + _check_one_dep, +) +from apm_cli.deps.lockfile import LockedDependency +from apm_cli.marketplace.models import ( + MarketplaceManifest, + MarketplacePlugin, + MarketplaceSource, +) +from apm_cli.models.dependency.types import GitReferenceType, RemoteRef + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def _marketplace_dep( + repo_url="acme-org/skill-auth", + discovered_via="acme-tools", + marketplace_plugin_name="skill-auth", + resolved_ref="v2.1.0", + resolved_commit="aabbccddee", +): + """Build a LockedDependency with marketplace provenance.""" + return LockedDependency( + resolved_ref=resolved_ref, + resolved_commit=resolved_commit, + repo_url=repo_url, + discovered_via=discovered_via, + marketplace_plugin_name=marketplace_plugin_name, + ) + + +def _plugin(name="skill-auth", ref="v2.1.0", version="2.1.0"): + """Build a MarketplacePlugin with a github source.""" + return MarketplacePlugin( + name=name, + source={"type": "github", "repo": "acme-org/skill-auth", "ref": ref}, + version=version, + ) + + +def _manifest(*plugins, name="acme-tools"): + return MarketplaceManifest(name=name, plugins=tuple(plugins)) + + +def _source(name="acme-tools"): + return MarketplaceSource(name=name, owner="acme-org", repo="marketplace") + + +# --------------------------------------------------------------------------- +# _check_marketplace_ref +# --------------------------------------------------------------------------- + + +class TestCheckMarketplaceRef: + """Ref-based outdated check against marketplace entry.""" + + @patch("apm_cli.marketplace.client.fetch_or_cache") + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_up_to_date(self, mock_get_mkt, mock_fetch): + """Same ref -> up-to-date.""" + mock_get_mkt.return_value = _source() + mock_fetch.return_value = _manifest(_plugin(ref="v2.1.0")) + dep = _marketplace_dep(resolved_ref="v2.1.0") + + result = _check_marketplace_ref(dep, verbose=False) + assert result is not None + assert result.status == "up-to-date" + + @patch("apm_cli.marketplace.client.fetch_or_cache") + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_outdated(self, mock_get_mkt, mock_fetch): + """Different ref -> outdated.""" + mock_get_mkt.return_value = _source() + mock_fetch.return_value = _manifest(_plugin(ref="v3.0.0", version="3.0.0")) + dep = _marketplace_dep(resolved_ref="v2.1.0") + + result = _check_marketplace_ref(dep, verbose=False) + assert result is not None + assert result.status == "outdated" + assert "3.0.0" in result.latest + + def test_no_marketplace_provenance(self): + """Non-marketplace dep returns None (fall through).""" + dep = LockedDependency( + resolved_ref="main", + resolved_commit="abc123", + repo_url="owner/repo", + ) + result = _check_marketplace_ref(dep, verbose=False) + assert result is None + + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_marketplace_not_found(self, mock_get_mkt): + """Unknown marketplace returns None (fall through to git check).""" + from apm_cli.marketplace.errors import MarketplaceNotFoundError + mock_get_mkt.side_effect = MarketplaceNotFoundError("nope") + dep = _marketplace_dep() + result = _check_marketplace_ref(dep, verbose=False) + assert result is None + + @patch("apm_cli.marketplace.client.fetch_or_cache") + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_plugin_not_in_manifest(self, mock_get_mkt, mock_fetch): + """Plugin removed from marketplace returns None.""" + mock_get_mkt.return_value = _source() + mock_fetch.return_value = _manifest(_plugin(name="other-plugin")) + dep = _marketplace_dep() + result = _check_marketplace_ref(dep, verbose=False) + assert result is None + + @patch("apm_cli.marketplace.client.fetch_or_cache") + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_uses_resolved_commit_when_no_ref(self, mock_get_mkt, mock_fetch): + """Falls back to resolved_commit when resolved_ref is empty.""" + mock_get_mkt.return_value = _source() + mock_fetch.return_value = _manifest(_plugin(ref="aabbccddee")) + dep = _marketplace_dep(resolved_ref="", resolved_commit="aabbccddee") + + result = _check_marketplace_ref(dep, verbose=False) + assert result is not None + assert result.status == "up-to-date" + + +# --------------------------------------------------------------------------- +# _check_one_dep -- marketplace path integration +# --------------------------------------------------------------------------- + + +class TestCheckOneDepMarketplace: + """_check_one_dep delegates to _check_marketplace_ref for marketplace deps.""" + + @patch("apm_cli.commands.outdated._check_marketplace_ref") + def test_marketplace_dep_uses_ref_check(self, mock_ref_check): + """When _check_marketplace_ref returns a result, it is used.""" + mock_ref_check.return_value = OutdatedRow( + package="skill-auth@acme-tools", current="v2.1.0", latest="v3.0.0", + status="outdated", source="marketplace: acme-tools", + ) + dep = _marketplace_dep() + result = _check_one_dep(dep, downloader=MagicMock(), verbose=False) + assert result.status == "outdated" + mock_ref_check.assert_called_once() + + @patch("apm_cli.commands.outdated._check_marketplace_ref") + def test_fallthrough_to_git_when_ref_check_returns_none(self, mock_ref_check): + """When _check_marketplace_ref returns None, fall through to git check.""" + mock_ref_check.return_value = None + dep = _marketplace_dep() + downloader = MagicMock() + downloader.get_remote_refs.return_value = [ + RemoteRef( + ref_type=GitReferenceType.TAG, + name="v2.1.0", + commit_sha="aabbccddee", + ), + ] + result = _check_one_dep(dep, downloader=downloader, verbose=False) + assert result is not None diff --git a/tests/unit/test_view_command.py b/tests/unit/test_view_command.py index 6b08414b6..d57b70787 100644 --- a/tests/unit/test_view_command.py +++ b/tests/unit/test_view_command.py @@ -572,3 +572,120 @@ def test_info_alias_hidden_from_help(self): stripped = line.strip() if stripped.startswith("info "): pytest.fail(f"'info' should be hidden but found in help: {line}") + + +# ------------------------------------------------------------------ +# B4: ``apm view plugin@marketplace`` without ``versions`` field +# ------------------------------------------------------------------ + + +class TestViewMarketplaceNoField(_InfoCmdBase): + """``apm view plugin@marketplace`` (no field) shows marketplace plugin.""" + + def test_marketplace_ref_shows_plugin(self): + """``apm view plugin@mkt`` routes to _display_marketplace_plugin.""" + from apm_cli.marketplace.models import ( + MarketplaceManifest, + MarketplacePlugin, + MarketplaceSource, + ) + + plugin = MarketplacePlugin( + name="my-plugin", + source={"type": "github", "repo": "acme/plugin", "ref": "main"}, + version="2.0.0", + ) + manifest = MarketplaceManifest(name="acme-tools", plugins=(plugin,)) + source = MarketplaceSource(name="acme-tools", owner="acme", repo="marketplace") + + with patch( + "apm_cli.marketplace.registry.get_marketplace_by_name", + return_value=source, + ), patch( + "apm_cli.marketplace.client.fetch_or_cache", + return_value=manifest, + ): + with _force_rich_fallback(): + result = self.runner.invoke( + cli, ["view", "my-plugin@acme-tools"] + ) + + assert result.exit_code == 0 + assert "my-plugin" in result.output + assert "acme-tools" in result.output + + def test_marketplace_ref_does_not_require_apm_modules(self): + """``apm view plugin@mkt`` works without apm_modules/ directory.""" + from apm_cli.marketplace.models import ( + MarketplaceManifest, + MarketplacePlugin, + MarketplaceSource, + ) + + plugin = MarketplacePlugin( + name="my-plugin", + source={"type": "github", "repo": "acme/plugin", "ref": "main"}, + version="1.0.0", + ) + manifest = MarketplaceManifest(name="acme-tools", plugins=(plugin,)) + source = MarketplaceSource(name="acme-tools", owner="acme", repo="marketplace") + + with self._chdir_tmp(): + # No apm_modules/ -- should still succeed + with patch( + "apm_cli.marketplace.registry.get_marketplace_by_name", + return_value=source, + ), patch( + "apm_cli.marketplace.client.fetch_or_cache", + return_value=manifest, + ): + with _force_rich_fallback(): + result = self.runner.invoke( + cli, ["view", "my-plugin@acme-tools"] + ) + + assert result.exit_code == 0 + + def test_non_marketplace_ref_still_uses_local_path(self): + """``apm view org/repo`` still falls through to local metadata lookup.""" + with self._chdir_tmp() as tmp: + self._make_package( + tmp, "myorg", "myrepo", version="3.0.0", + ) + os.chdir(tmp) + with _force_rich_fallback(): + result = self.runner.invoke(cli, ["view", "myorg/myrepo"]) + + assert result.exit_code == 0 + assert "3.0.0" in result.output + + def test_marketplace_ref_with_version_fragment(self): + """``apm view plugin@mkt#ref`` (no field) shows plugin info.""" + from apm_cli.marketplace.models import ( + MarketplaceManifest, + MarketplacePlugin, + MarketplaceSource, + ) + + plugin = MarketplacePlugin( + name="my-plugin", + source={"type": "github", "repo": "acme/plugin", "ref": "v1.0.0"}, + version="1.0.0", + ) + manifest = MarketplaceManifest(name="acme-tools", plugins=(plugin,)) + source = MarketplaceSource(name="acme-tools", owner="acme", repo="marketplace") + + with patch( + "apm_cli.marketplace.registry.get_marketplace_by_name", + return_value=source, + ), patch( + "apm_cli.marketplace.client.fetch_or_cache", + return_value=manifest, + ): + with _force_rich_fallback(): + result = self.runner.invoke( + cli, ["view", "my-plugin@acme-tools"] + ) + + assert result.exit_code == 0 + assert "1.0.0" in result.output diff --git a/tests/unit/test_view_versions.py b/tests/unit/test_view_versions.py new file mode 100644 index 000000000..54cf28f69 --- /dev/null +++ b/tests/unit/test_view_versions.py @@ -0,0 +1,117 @@ +"""Unit tests for marketplace plugin display in ``apm view``.""" + +from unittest.mock import patch, MagicMock + +import pytest +from click.testing import CliRunner + +from apm_cli.commands.view import _display_marketplace_plugin +from apm_cli.marketplace.models import ( + MarketplaceManifest, + MarketplacePlugin, + MarketplaceSource, +) + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +def _plugin( + name="skill-auth", + version="2.1.0", + description="Authentication skill", + ref="v2.1.0", + tags=("security", "auth"), +): + """Build a MarketplacePlugin with a github source.""" + return MarketplacePlugin( + name=name, + source={"type": "github", "repo": "acme-org/skill-auth", "ref": ref}, + version=version, + description=description, + tags=tags, + ) + + +def _manifest(*plugins, name="acme-tools"): + return MarketplaceManifest(name=name, plugins=tuple(plugins)) + + +def _source(name="acme-tools"): + return MarketplaceSource(name=name, owner="acme-org", repo="marketplace") + + +# --------------------------------------------------------------------------- +# _display_marketplace_plugin +# --------------------------------------------------------------------------- + + +class TestDisplayMarketplacePlugin: + """Plugin metadata display.""" + + @patch("apm_cli.marketplace.client.fetch_or_cache") + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_displays_plugin_info(self, mock_get_mkt, mock_fetch, capsys): + """Renders plugin name, version, description, source.""" + mock_get_mkt.return_value = _source() + mock_fetch.return_value = _manifest(_plugin()) + + logger = MagicMock() + _display_marketplace_plugin("skill-auth", "acme-tools", logger) + + # The output goes through Rich or click.echo - check no errors + logger.error.assert_not_called() + + @patch("apm_cli.marketplace.client.fetch_or_cache") + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_plugin_not_found_exits(self, mock_get_mkt, mock_fetch): + """Unknown plugin triggers error and sys.exit.""" + mock_get_mkt.return_value = _source() + mock_fetch.return_value = _manifest(_plugin(name="other")) + + logger = MagicMock() + with pytest.raises(SystemExit): + _display_marketplace_plugin("nonexistent", "acme-tools", logger) + logger.error.assert_called_once() + + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_marketplace_not_found_exits(self, mock_get_mkt): + """Unknown marketplace triggers error and sys.exit.""" + from apm_cli.marketplace.errors import MarketplaceNotFoundError + mock_get_mkt.side_effect = MarketplaceNotFoundError("nope") + + logger = MagicMock() + with pytest.raises(SystemExit): + _display_marketplace_plugin("any", "nope", logger) + + @patch("apm_cli.marketplace.client.fetch_or_cache") + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_string_source_handled(self, mock_get_mkt, mock_fetch): + """Plugin with string source (shorthand) does not crash.""" + plugin = MarketplacePlugin( + name="simple", + source="acme-org/simple", + version="1.0.0", + ) + mock_get_mkt.return_value = _source() + mock_fetch.return_value = _manifest(plugin) + + logger = MagicMock() + _display_marketplace_plugin("simple", "acme-tools", logger) + logger.error.assert_not_called() + + @patch("apm_cli.marketplace.client.fetch_or_cache") + @patch("apm_cli.marketplace.registry.get_marketplace_by_name") + def test_no_version_no_description(self, mock_get_mkt, mock_fetch): + """Plugin without optional fields renders without error.""" + plugin = MarketplacePlugin( + name="bare", + source={"type": "github", "repo": "o/r", "ref": "main"}, + ) + mock_get_mkt.return_value = _source() + mock_fetch.return_value = _manifest(plugin) + + logger = MagicMock() + _display_marketplace_plugin("bare", "acme-tools", logger) + logger.error.assert_not_called() diff --git a/uv.lock b/uv.lock index 80b3d2baf..29c763cfa 100644 --- a/uv.lock +++ b/uv.lock @@ -179,7 +179,7 @@ wheels = [ [[package]] name = "apm-cli" -version = "0.8.11" +version = "0.8.12" source = { editable = "." } dependencies = [ { name = "click" },