diff --git a/README.md b/README.md index 2c5f2ea..18294d0 100644 --- a/README.md +++ b/README.md @@ -33,6 +33,10 @@ The official Python SDK & CLI for [ModelScope Hub](https://modelscope.cn) — do ## News +**Unreleased** +- **Feature**: `ms-hub agent install -r owner/name` fetches the official framework plugin from a model repository and delegates to the entry operation it declares, plus the `modelscope_hub.agent.install_agent` SDK entry. The hub gains no framework knowledge. +- **Quality**: loading a plugin executes code this package did not ship, so it is gated by a compile-time owner allow-list checked before any download and a `content_sha256` manifest verified against every file before import. + **v0.4.0** (2026-09-01) - **Feature**: complete OpenAPI coverage for Agent-IDP, MCP, and Studios — Agent Ed25519 identities, OIDC discovery/JWKS and signed JWT issuance (`HubApi`, `ms-hub agent-idp`); Studio lists, variables and configuration options; hosted MCP discovery; protected visibility and runtime metadata; read-only tokens can log in and rejected writes name the required tier. Agent private JWKs are only written to an explicitly requested owner-only file. - **Fix**: Studio compat calls no longer leak connection options or API tokens, or drop cover images; errors distinguish permission, quota and conflicts; anonymous Studio info and log pagination work @@ -640,13 +644,18 @@ ms-hub cache clear --repo-id my-org/old-model --repo-type model --yes ### `ms-hub agent` -Low-level raw file transfer for remote agent repositories: `download`, `upload`, `list`. This command transfers files as-is, with **no framework awareness**. +Remote agent repositories: raw file transfer (`download`, `upload`, `list`) and plugin-driven `install`. ```bash ms-hub agent download -r user/my-agent --local-dir ./my-agent # download raw files ms-hub agent upload -r user/my-agent --local-dir ./my-agent # upload raw +ms-hub agent install -r user/my-agent # install into the framework ``` +`download` / `upload` / `list` transfer files as-is, with **no framework awareness**. + +`install` is different: it fetches an official plugin and hands the agent id to it, leaving every framework decision to the plugin. See [`ms-hub agent install`](#ms-hub-agent-install) for the details and the security model. + > **Framework-aware operations** (cross-framework `convert`, `watch`/bidirectional sync, `status`, `backups`, `restore`, `stop`) live in **[modelscope-agent](https://github.com/modelscope/ms-agent)** — use `ms-agent agent ...` instead. For example, to download and convert in one step: `ms-agent agent download -f qoder -r user/my-agent --target-framework qwenpaw`.
@@ -683,6 +692,59 @@ ms-hub agent upload -r user/my-agent --local-dir ./my-agent --dry-run | `--revision REV` | no | Repository revision (default: `master`) | | `--dry-run` | no | List files that would be uploaded without uploading | +#### `ms-hub agent install` + +Download an agent and hand it to its **framework plugin**. A plugin with an `install` entry point places the agent into the framework's workspace; one that only transports bytes writes the repository's files into a destination directory and leaves placement to whatever runs next. The command reports which happened (`Installed …` vs `Fetched …`). + +```bash +ms-hub agent install -r user/my-agent +``` + +| Option | Required | Description | +|--------|----------|-------------| +| `-r, --repo REPO` | yes | Agent repository to install (`owner/name`) | +| `--plugin-revision REV` | no | Plugin revision (default: `master`; pin a tag for reproducible installs) | +| `-n, --name NAME` | no | Sub-agent name, passed through to the plugin | +| `--framework FW` | no | Override the plugin's framework detection | +| `--local-dir DIR` | no | Where the agent repository is **downloaded**, not where it is installed. Omitted, downloads go to `$MODELSCOPE_CACHE/agent/agent-staging/---/` and are cleaned up on success | +| `--dry-run` | no | Ask the plugin to report instead of change anything. The plugin is still downloaded, imported and run — only its writes are suppressed | +| `-y, --yes` / `--force` / `-q, --quiet` | no | Passed through to the plugin | + +Exit codes: `0` success, `2` a gate refused or the command line is wrong, otherwise **the plugin's own code** — the install layer uses `3` (already exists), `4` (refused to overwrite), `5` (install or self-check failed), `6` (framework mismatch). + +##### Supported scope + +This package supports **no frameworks**; what can be installed is a property of the plugin build it fetches, so the authoritative list is printed every run: + +``` +plugin: modelscope/agent-hub-plugin@v0.3.1 (version 0.3.1) +entry : agent_hub_plugin.install() # negotiated: install > fetch_raw > download +scope : frameworks ms-agent, qwenpaw | operations fetch_raw, install, list_backups, restore | planned convert (P2), upload (P1) +Installed user/my-agent +``` + +`planned` names operations the plugin declares but has not implemented; calling one returns `ok=False` naming the planned release. + +##### Security model + +Where the plugin may come from is fixed at compile time, not at the command line: + +1. **Owner allow-list** — `modelscope_hub.constants.AGENT_PLUGIN_TRUSTED_OWNERS`, currently `modelscope` and `AI-ModelScope`. Checked before anything is downloaded, with no environment override and no per-invocation confirmation: an allow-listed plugin is fetched and run. Widening the list is a reviewed code change. +2. **Manifest integrity** — `plugin.json`'s `content_sha256` is verified against the files on disk before import. That proves the bytes are the ones the manifest described; it is **not** authenticity, since the manifest is unsigned and ships beside the code it describes. Origin rests on the allow-list alone. + +Which build is about to run is logged before the import. The plugin receives your `--endpoint` and your API token, since it needs credentials to fetch the agent. + +Plugins from other owners are not supported. The package format is documented for maintainers in the `modelscope_hub.agent._plugin` module docstring. + +##### Python API + +```python +from modelscope_hub.agent import install_agent + +outcome = install_agent("owner/my-agent", plugin_revision="v0.3.1") +print(outcome.ok, outcome.operation, outcome.exit_code, outcome.error) +``` +
### `ms-hub agent-idp` diff --git a/src/modelscope_hub/agent/__init__.py b/src/modelscope_hub/agent/__init__.py index 025d267..8669699 100644 --- a/src/modelscope_hub/agent/__init__.py +++ b/src/modelscope_hub/agent/__init__.py @@ -1,9 +1,11 @@ # Copyright (c) Alibaba, Inc. and its affiliates. """Agent repository transport SDK for ModelScope Hub. -This package provides only the low-level HTTP client for agent repositories. -Framework-aware workspace management (frameworks, conversion, sync, watch, -backups) lives in **modelscope-agent** (``ms_agent.agent_hub``). +This package provides the low-level HTTP client for agent repositories, plus the +plugin loader behind ``ms agent install``. Neither carries framework knowledge: +workspace management (frameworks, conversion, sync, watch, backups) lives in +**modelscope-agent** (``ms_agent.agent_hub``), and the plugin loader only fetches +and invokes a plugin that a publisher ships as a model repository. Public API ---------- @@ -14,9 +16,27 @@ - ``agent_visibility_label`` / ``agent_last_modified`` -- read renamed agent metadata fields from an API item, tolerating both JSON spellings (snake_case and PascalCase) and legacy keys. +- :func:`install_agent` -- fetch or install an agent through its framework + plugin, the choice being negotiated with the plugin. +- :func:`default_staging_dir` -- where a fetch-only plugin's files land when the + caller named no directory. """ from ._api import AgentApi, RemoteFileInfo, agent_last_modified, agent_visibility_label, is_lfs_file +from ._plugin import ( + ENTRY_OPERATIONS, + InstallOutcome, + PluginSpec, + assert_trusted_owner, + default_staging_dir, + fetch_plugin, + install_agent, + load_plugin, + plugin_syspath, + resolve_plugin_repo, + select_operation, + verify_manifest, +) __all__ = [ "AgentApi", @@ -24,4 +44,16 @@ "is_lfs_file", "agent_visibility_label", "agent_last_modified", + "install_agent", + "PluginSpec", + "InstallOutcome", + "ENTRY_OPERATIONS", + "resolve_plugin_repo", + "assert_trusted_owner", + "fetch_plugin", + "verify_manifest", + "load_plugin", + "plugin_syspath", + "select_operation", + "default_staging_dir", ] diff --git a/src/modelscope_hub/agent/_plugin.py b/src/modelscope_hub/agent/_plugin.py new file mode 100644 index 0000000..3033564 --- /dev/null +++ b/src/modelscope_hub/agent/_plugin.py @@ -0,0 +1,718 @@ +# Copyright (c) Alibaba, Inc. and its affiliates. +"""Locate, fetch, verify and run an agent plugin. + +``ms agent install`` resolves which plugin to use, downloads it from a model +repository, verifies it, and hands the agent id to the plugin's entry point. No +framework knowledge lives here: how an agent is registered and what its workspace +looks like are the plugin's decisions. The one exception is the destination +directory, which this module resolves for a plugin that only transports bytes -- +it cannot know where such a plugin should write, and the plugin deliberately has +no default of its own. + +Trust model +----------- +The **owner allow-list is the authorisation**. It is a compile-time constant +naming only official organisations, and nothing lets a caller point this command +at a plugin whose owner is not on it, so by the time a package has been fetched +the decision to run it was already made by whoever shipped this release. There is +consequently no per-invocation opt-in: an allow-listed plugin is always downloaded +*and* executed, and there is no inspect-only mode. A ``--trust-remote-code`` style +flag is deferred to whichever release supports third-party plugins; exposing one +now would imply a choice the allow-list has already made. + +Integrity comes from ``plugin.json``'s ``content_sha256``, not from the hub's own +file listing -- that listing has been observed reporting a git blob SHA-1 in a +``sha256`` field. Be precise about what that buys: it proves the bytes on disk are +the bytes the manifest described, and it gives the audit line in +:func:`log_execution` a stable fingerprint. It is **not** authenticity. The +manifest ships inside the same unsigned repository as the code it describes, so +whoever controls the repository controls the hashes and can make anything verify. +The allow-list is what vouches for the plugin's origin; nothing here vouches for +its contents beyond "unchanged since it was listed". Signing would change that and +is not done yet. + +Plugin package contract +----------------------- +Maintainer-facing record of the format; it is deliberately not in the README, +which documents only the supported path of installing an official plugin. + +A plugin is a **model** repository (``snapshot_download`` rejects +``repo_type='agent'``) with ``plugin.json`` at its root beside an importable +package or module named by ``entry_module``. + +``plugin.json`` -- two fields are required, the rest are display only and never +validated: + +* ``entry_module`` (str) -- imported from the download root via + ``sys.path.insert(0, root)``, so relative imports inside a package work. +* ``content_sha256`` (dict) -- sha256 of every file, keyed by posix path relative + to the root. Checked in both directions: missing, mismatched and unlisted files + all fail. Exempt: ``plugin.json`` itself (it cannot hash itself), + ``.gitattributes`` (the hub injects it) and ``__pycache__``. Keys are validated + as paths before use, since they are attacker-controlled. +* ``version``, ``frameworks``, ``api``, ``roadmap`` -- feed :meth:`PluginSpec.describe` + and :meth:`PluginSpec.scope`, nothing else. + +The entry module must expose at least one of ``install``, ``fetch_raw``, +``download``, tried in that order. ``capabilities()`` returning +``{"operations": [...], "frameworks": [...], "planned": {...}}`` is authoritative +when present, so a name that is shipped but not implemented is skipped rather than +selected; when it is absent, selection falls back to presence, and when it raises +that is an error rather than an empty declaration. + +The chosen operation is called with keyword arguments narrowed to its signature, +from: ``repo``, ``name``, ``framework``, ``source_framework``, ``local_dir``, +``dest``, ``dry_run``, ``yes``, ``force``, ``quiet``, ``endpoint``, ``token``. +Unset optionals are dropped so the plugin's own defaults apply; ``False`` booleans +are kept; ``dest`` is always resolved. Its return value must carry ``ok`` -- +required, not defaulted, because it is the only signal deciding whether the user +is told the agent was installed -- plus ``error`` and ``exit_code`` on failure and +``files_written`` / ``root`` for the success message. Raising is also handled. + +Two constraints follow from how loading works. The package must use **relative +imports** internally, because it is registered under a directory-scoped alias +rather than its own name so two plugins cannot be served each other's cached +code. And it must not assume its directory stays on ``sys.path`` after the +operation returns: the entry is scoped to the call, since a directory parked at +``sys.path[0]`` lets any file it ships shadow the standard library. Cleaning up +its own staging directory is the plugin's job, not this module's. +""" + +from __future__ import annotations + +import hashlib +import importlib +import importlib.util +import inspect +import json +import logging +import os +import re +import sys +from collections.abc import Iterator +from contextlib import contextmanager +from dataclasses import dataclass +from datetime import datetime +from pathlib import Path, PurePosixPath +from typing import Any + +from .. import constants +from ..api import HubApi +from ..errors import InvalidParameter, NotSupportedError +from ..utils.file_utils import compute_hash, get_cache_dir + +logger = logging.getLogger("modelscope_hub.agent") + +MANIFEST_NAME = "plugin.json" + +#: Negotiated rather than hard-coded, so a plugin growing a richer entry point +#: does not require re-releasing the hub. Order is preference: a plugin that +#: implements ``install`` owns placement, registration and completion, so it +#: wins. ``fetch_raw`` is a transport that writes the repository's bytes into a +#: directory the caller names and never touches a framework workspace, which is +#: what keeps a user's own credentials intact. ``download`` is the 0.1.x name +#: for an operation that did install into the workspace. +ENTRY_OPERATIONS: tuple[str, ...] = ("install", "fetch_raw", "download") + +#: Staging root for a fetch-only plugin, relative to ``MODELSCOPE_CACHE``. +#: Matches the plugin's own ``staging_dir()`` so one convention covers both +#: sides and there are not two places agent files can land. +AGENT_STAGING_SUBDIR: tuple[str, ...] = ("agent", "agent-staging") + + +@dataclass(frozen=True, slots=True) +class PluginSpec: + """A plugin that has been downloaded and verified, but not yet imported.""" + + repo_id: str + owner: str + name: str + revision: str + directory: Path + manifest: dict[str, Any] + entry_module: str + + @property + def version(self) -> str: + return str(self.manifest.get("version", "unknown")) + + def describe(self) -> str: + frameworks = self.manifest.get("frameworks") or [] + operations = self.manifest.get("api") or [] + digest = _manifest_digest(self.manifest) + return ( + f" plugin : {self.repo_id}\n" + f" revision : {self.revision}\n" + f" version : {self.version}\n" + f" entry : {self.entry_module}\n" + f" frameworks : {', '.join(map(str, frameworks)) or '-'}\n" + f" operations : {', '.join(map(str, operations)) or '-'}\n" + f" planned : {self._planned()}\n" + f" directory : {self.directory}\n" + f" manifest : {len(self.manifest.get('content_sha256') or {})} file(s), " + f"sha256 {digest}" + ) + + def scope(self) -> str: + """One line naming what this build covers, for the success path. + + A command that exits 0 otherwise tells a user nothing about which + frameworks it handled or which operations this plugin version actually + implements, and both decide whether the result is what they wanted. + """ + frameworks = ", ".join(map(str, self.manifest.get("frameworks") or [])) or "-" + operations = ", ".join(map(str, self.manifest.get("api") or [])) or "-" + return f"frameworks {frameworks} | operations {operations} | planned {self._planned()}" + + def _planned(self) -> str: + """Operations the manifest declares as not yet implemented, and when.""" + roadmap = self.manifest.get("roadmap") or {} + if not isinstance(roadmap, dict): + return "-" + return ", ".join(f"{name} ({when})" for name, when in sorted(roadmap.items())) or "-" + + +@dataclass(frozen=True, slots=True) +class InstallOutcome: + ok: bool + error: str | None = None + operation: str | None = None + plugin: PluginSpec | None = None + result: Any = None + exit_code: int = 0 + + +def _manifest_digest(manifest: dict[str, Any]) -> str: + entries = manifest.get("content_sha256") or {} + if not isinstance(entries, dict) or not entries: + return "unavailable" + blob = "\n".join(f"{k}:{entries[k]}" for k in sorted(entries)) + return hashlib.sha256(blob.encode("utf-8")).hexdigest()[:16] + + +def resolve_plugin_repo(explicit: str | None = None) -> str: + """Return the plugin repository id: argument, then environment, then default. + + The default is :data:`~modelscope_hub.constants.DEFAULT_AGENT_PLUGIN_REPO`, + the plugin published under the ModelScope organisation. It is a default and + not a hard-coded call site because who publishes the plugin is a deployment + decision -- an override is one flag or one environment variable away, and the + owner allow-list applies to whichever id wins. + """ + repo_id = (explicit or "").strip() + if not repo_id: + repo_id = (os.environ.get(constants.ENV_AGENT_PLUGIN_REPO) or "").strip() + if not repo_id: + repo_id = constants.DEFAULT_AGENT_PLUGIN_REPO + HubApi._parse_repo_id(repo_id) + return repo_id + + +def assert_trusted_owner(repo_id: str) -> tuple[str, str]: + """Split *repo_id* and require its owner on the allow-list. + + Matching is case-insensitive because that is how the registry treats + identity: it resolves ``ModelScope/x`` and ``modelscope/x`` to the same + repository and normalises the owner, so two owners differing only in case + cannot both exist. An exact comparison would therefore not stop a look-alike + account -- it would only reject the casing somebody copied from the website. + """ + owner, name = HubApi._parse_repo_id(repo_id) + trusted = constants.AGENT_PLUGIN_TRUSTED_OWNERS + if owner.casefold() not in {entry.casefold() for entry in trusted}: + error = InvalidParameter( + f"owner {owner!r} is not allowed to provide the agent plugin. " + f"Trusted owners: {', '.join(sorted(trusted)) or '(none)'}." + ) + error.suggestion = ( + "The allow-list is a compile-time constant " + "(modelscope_hub.constants.AGENT_PLUGIN_TRUSTED_OWNERS), not an " + "environment variable: it is the trust anchor for a command that runs " + "downloaded code, so widening it is a reviewed code change." + ) + raise error + return owner, name + + +def fetch_plugin( + repo_id: str, + *, + revision: str | None = None, + token: str | None = None, + endpoint: str | None = None, + cache_dir: str | None = None, +) -> Path: + """Download the plugin package and return its directory. + + Transfer executes nothing, and the owner gate in :func:`assert_trusted_owner` + has already run by the time this is reached -- a package outside the + allow-list is refused without touching the network. + """ + from ..compat import snapshot_download + + rev = revision or constants.DEFAULT_AGENT_PLUGIN_REVISION + try: + directory = snapshot_download( + repo_id, + repo_type="model", + revision=rev, + token=token, + endpoint=endpoint, + cache_dir=cache_dir, + ) + except Exception as exc: + # ``snapshot_download`` re-raises hub errors as + # ``requests.exceptions.HTTPError``, so the original type is not a + # reliable discriminator; keep the cause chain instead. + raise NotSupportedError(f"failed to download agent plugin {repo_id}@{rev}: {exc}") from exc + path = Path(directory) + if not path.is_dir(): + raise NotSupportedError(f"agent plugin {repo_id}@{rev} did not resolve to a directory: {path}") + return path + + +#: Present in a downloaded plugin directory but not plugin content, so their +#: absence from ``content_sha256`` is expected. ``.gitattributes`` is injected by +#: the hub for LFS tracking and therefore never appears in an author's manifest. +NOT_PLUGIN_CONTENT: frozenset[str] = frozenset({MANIFEST_NAME, ".gitattributes"}) + + +def verify_manifest(directory: Path, repo_id: str) -> dict[str, Any]: + """Check the downloaded package against its own ``plugin.json``. + + Strict on purpose, and worth being clear about what strictness buys: it proves + the files on disk are the files the manifest described, and it makes the + digest in the :func:`log_execution` audit line mean something, so what was + recorded as about to run and what actually got imported cannot diverge. It + does not prove anything about authorship -- the manifest is unsigned and ships + beside the code it describes, so a repository's owner can make any content + verify. That is the owner allow-list's job. + """ + manifest_path = directory / MANIFEST_NAME + if not manifest_path.is_file(): + raise NotSupportedError(f"{repo_id} is not an agent plugin: no {MANIFEST_NAME} at the repository root.") + try: + manifest = json.loads(manifest_path.read_text(encoding="utf-8")) + except (OSError, json.JSONDecodeError) as exc: + raise NotSupportedError(f"{repo_id}: cannot read {MANIFEST_NAME}: {exc}") from exc + if not isinstance(manifest, dict): + raise NotSupportedError(f"{repo_id}: {MANIFEST_NAME} must be a JSON object.") + + entry_module = manifest.get("entry_module") + if not isinstance(entry_module, str) or not entry_module: + raise NotSupportedError(f"{repo_id}: {MANIFEST_NAME} has no 'entry_module'; cannot know what to import.") + + recorded = manifest.get("content_sha256") + if not isinstance(recorded, dict) or not recorded: + raise NotSupportedError( + f"{repo_id}: {MANIFEST_NAME} has no 'content_sha256', so the plugin's integrity " + "cannot be verified. Refusing to load it." + ) + + missing, mismatched, unexpected, escaped = [], [], [], [] + directory_resolved = directory.resolve() + for rel, expected in sorted(recorded.items()): + # Manifest keys are attacker-controlled. ``directory / "/etc/passwd"`` + # discards the directory entirely, so an absolute or parent-climbing key + # would have this loader read -- and hash -- a file outside the package. + # Nothing is returned to the caller, so it is not a disclosure, but it is + # a read the manifest has no business requesting. + parts = PurePosixPath(rel).parts + if PurePosixPath(rel).is_absolute() or ".." in parts: + escaped.append(rel) + continue + target = directory / rel + if not target.resolve().is_relative_to(directory_resolved): + escaped.append(rel) + continue + if not target.is_file(): + missing.append(rel) + continue + actual = compute_hash(target) + if actual != expected: + mismatched.append(rel) + recorded_set = set(recorded) + for path in sorted(p for p in directory.rglob("*") if p.is_file()): + relative = path.relative_to(directory) + rel = relative.as_posix() + if rel in NOT_PLUGIN_CONTENT or "__pycache__" in relative.parts: + continue + if rel not in recorded_set: + unexpected.append(rel) + + problems = [] + if escaped: + problems.append(f"{len(escaped)} manifest key(s) point outside the package: {', '.join(escaped[:5])}") + if missing: + problems.append(f"missing {len(missing)} file(s): {', '.join(missing[:5])}") + if mismatched: + problems.append(f"sha256 mismatch for {len(mismatched)} file(s): {', '.join(mismatched[:5])}") + if unexpected: + problems.append(f"not listed in the manifest: {', '.join(unexpected[:5])}") + if problems: + raise NotSupportedError(f"{repo_id}: plugin integrity check failed -- " + "; ".join(problems)) + return manifest + + +def log_execution(spec: PluginSpec) -> None: + """Record which build is about to be imported, before it is imported. + + There is no per-invocation opt-in to wait for any more. The owner allow-list + is the authorisation: it is compile-time, it names only official + organisations, and nothing lets a user point this command at a plugin whose + owner is not on it. Executing downloaded code still deserves an audit line + naming the exact build, emitted *before* the import so that a crash during it + leaves a trace of what was being loaded. + + A per-invocation opt-in (``--trust-remote-code``) is deliberately deferred: + it belongs with third-party plugins, which this release does not support, and + exposing it now would imply a choice the allow-list has already made. + Reintroducing it means gating here again and refusing instead of logging. + """ + logger.info("Executing agent plugin:\n%s", spec.describe()) + + +def _module_alias(spec: PluginSpec) -> str: + """A ``sys.modules`` name unique to the code being loaded. + + ``importlib.import_module(entry_module)`` goes through the global cache, so + loading two plugins whose entry modules share a name in one process would + silently run the first one's code for the second. + + The discriminator has to be the *directory*, not the repository and revision: + those do not identify the bytes. Two checkouts of one repository at one + revision -- a re-download into a different cache, a development tree beside a + published one -- are different code with the same identity, and aliasing on + the pair collides. The entry module name stays in the alias so a traceback is + still readable. + """ + resolved = Path(spec.directory).resolve() + digest = hashlib.sha256(str(resolved).encode("utf-8")).hexdigest()[:12] + safe = re.sub(r"[^A-Za-z0-9_]", "_", spec.entry_module) + return f"_ms_agent_plugin_{safe}_{digest}" + + +@contextmanager +def plugin_syspath(directory: Path) -> Iterator[None]: + """Keep *directory* importable for the duration of the block, then undo it. + + Scoped rather than permanent on purpose. A plugin directory parked at + ``sys.path[0]`` lets any module it ships shadow the standard library or a + dependency for the rest of the process, and shipping one is not even a rule + violation -- every file has to be listed in ``content_sha256``, so a + ``json.py`` or ``requests.py`` passes the integrity check like anything else. + A one-shot CLI barely notices; a long-lived process calling + :func:`install_agent` would stay poisoned. + + It cannot be narrowed to the import alone: plugins import their own sibling + packages lazily, at call time, so the directory has to stay reachable until + the operation returns. + """ + root = str(directory) + inserted = root not in sys.path + if inserted: + sys.path.insert(0, root) + try: + yield + finally: + if inserted: + try: + sys.path.remove(root) + except ValueError: + pass + + +def load_plugin(spec: PluginSpec) -> Any: + """Import the plugin's entry module from its downloaded directory. + + The directory goes at the *front* of ``sys.path`` so the fetched revision + wins over any same-named installed distribution, and the module is registered + under :func:`_module_alias` rather than its own name so a second plugin cannot + be served the first one's cached code. Callers that will also *invoke* the + plugin should hold :func:`plugin_syspath` open for the whole operation. + + Residual limitation, not solved here: a plugin's sibling top-level packages + (``agent_hub_core`` beside ``agent_hub_plugin``, say) are imported by their own + names and still land in the global cache, so two plugins shipping different + copies of one would collide. Isolating that needs a subprocess, which in turn + needs a serialisable result contract. + """ + root = str(spec.directory) + if root not in sys.path: + sys.path.insert(0, root) + + alias = _module_alias(spec) + cached = sys.modules.get(alias) + if cached is not None: + return cached + + package_init = Path(spec.directory) / spec.entry_module / "__init__.py" + single_file = Path(spec.directory) / f"{spec.entry_module}.py" + if package_init.is_file(): + target, search_locations = package_init, [str(package_init.parent)] + elif single_file.is_file(): + target, search_locations = single_file, None + else: + if root in sys.path: + sys.path.remove(root) + raise ImportError(f"entry module {spec.entry_module!r} is neither a package nor a module in {root}") + + module_spec = importlib.util.spec_from_file_location(alias, target, submodule_search_locations=search_locations) + if module_spec is None or module_spec.loader is None: + if root in sys.path: + sys.path.remove(root) + raise ImportError(f"cannot build an import spec for {target}") + + module = importlib.util.module_from_spec(module_spec) + # Registered before executing so the plugin's own relative and recursive + # imports resolve to this module rather than re-entering it. + sys.modules[alias] = module + try: + module_spec.loader.exec_module(module) + except BaseException: + sys.modules.pop(alias, None) + if root in sys.path: + sys.path.remove(root) + raise + return module + + +def select_operation(module: Any) -> tuple[str, Any]: + """Pick the entry operation the plugin actually supports. + + ``capabilities()`` is authoritative when present, so a plugin that ships a + name without implementing it is not selected. When it is *absent* selection + falls back to presence, which is what lets a minimal plugin work. When it is + present but fails, that is a broken plugin rather than an undeclaring one, so + it is an error: silently falling back to presence would pick whatever name + happens to exist, including a placeholder the plugin chose not to declare. + """ + declared = None + capabilities = getattr(module, "capabilities", None) + if callable(capabilities): + try: + payload = capabilities() + except Exception as exc: + raise NotSupportedError( + f"plugin {module.__name__}.capabilities() raised " + f"{exc.__class__.__name__}: {exc}. Without it there is no way to tell " + "which operations the plugin implements, so refusing to guess." + ) from exc + operations = payload.get("operations") if isinstance(payload, dict) else None + if operations is None: + raise NotSupportedError( + f"plugin {module.__name__}.capabilities() returned no 'operations' " + f"(got {type(payload).__name__}); cannot tell which entry points it implements." + ) + declared = set(operations) + + for name in ENTRY_OPERATIONS: + func = getattr(module, name, None) + if not callable(func): + continue + if declared is not None and name not in declared: + continue + return name, func + + available = ", ".join(sorted(declared)) if declared else "none reported" + raise NotSupportedError( + f"plugin {module.__name__} exposes none of {', '.join(ENTRY_OPERATIONS)}. Its capabilities are: {available}." + ) + + +def _accepted_kwargs(func: Any, candidates: dict[str, Any]) -> dict[str, Any]: + """Narrow *candidates* to what *func* accepts. + + A plugin's entry signature is its own contract and may grow keywords the hub + knows nothing about; filtering keeps older hubs working instead of raising + ``TypeError``. A function taking ``**kwargs`` gets everything. + """ + try: + parameters = inspect.signature(func).parameters + except (TypeError, ValueError): + return dict(candidates) + if any(p.kind is inspect.Parameter.VAR_KEYWORD for p in parameters.values()): + return dict(candidates) + return {key: value for key, value in candidates.items() if key in parameters} + + +def default_staging_dir(repo: str) -> Path: + """Where a fetch-only plugin's files land when the caller named no directory. + + The staging directory itself is not created -- the plugin makes it when it + writes, so an operation that ignores ``dest`` leaves no empty directory + behind. Resolving the path does create the SDK cache root, as any download + would. The repository id contains a slash and so is flattened; the timestamp + keeps repeated fetches of one repository apart, to one-second resolution. + """ + slug = repo.replace("/", "--") + stamp = datetime.now().strftime("%Y%m%d_%H%M%S") + return get_cache_dir().joinpath(*AGENT_STAGING_SUBDIR, f"{slug}-{stamp}") + + +def install_agent( + repo: str, + *, + name: str | None = None, + framework: str | None = None, + local_dir: str | None = None, + dry_run: bool = False, + yes: bool = False, + force: bool = False, + quiet: bool = False, + plugin_repo: str | None = None, + plugin_revision: str | None = None, + endpoint: str | None = None, + token: str | None = None, + cache_dir: str | None = None, +) -> InstallOutcome: + """Fetch or install *repo*'s agent through its framework plugin. + + An allow-listed plugin is always downloaded **and executed** -- there is no + inspect-only mode. Authorisation is the compile-time owner allow-list, not a + per-invocation opt-in, so by the time this function is past + :func:`assert_trusted_owner` the decision has already been made by whoever + shipped this package. + + Which of fetch or install happens is the plugin's answer, not this function's: + the entry operation is negotiated in :func:`select_operation`, so a plugin + that installs into the workspace installs, and one that only transports bytes + stages them in ``local_dir`` (or :func:`default_staging_dir`) for the install + layer to place. + + *repo* is passed through uninterpreted beyond requiring ``owner/name``. + Plugin failures come back as data (``ok`` False); the two gates + (:func:`resolve_plugin_repo`, :func:`assert_trusted_owner`) raise instead, so + the CLI can map a misconfigured command line to exit 2 and keep it distinct + from a failed install. + """ + if not repo or not repo.strip(): + raise InvalidParameter("--repo is required, in 'owner/name' form.") + repo = repo.strip() + HubApi._parse_repo_id(repo) + + plugin_repo_id = resolve_plugin_repo(plugin_repo) + owner, plugin_name = assert_trusted_owner(plugin_repo_id) + + try: + directory = fetch_plugin( + plugin_repo_id, + revision=plugin_revision, + token=token, + endpoint=endpoint, + cache_dir=cache_dir, + ) + except NotSupportedError as exc: + if plugin_repo_id == constants.DEFAULT_AGENT_PLUGIN_REPO: + # The user never named this repository, so a bare download error + # leaves them nothing to act on. + exc.suggestion = ( + f"{plugin_repo_id} is the built-in default. If it is not published yet, " + f"or you built your own, pass --plugin-repo owner/name (or set " + f"{constants.ENV_AGENT_PLUGIN_REPO}). Its owner must be one of " + f"{', '.join(sorted(constants.AGENT_PLUGIN_TRUSTED_OWNERS))}." + ) + raise + manifest = verify_manifest(directory, plugin_repo_id) + spec = PluginSpec( + repo_id=plugin_repo_id, + owner=owner, + name=plugin_name, + revision=plugin_revision or constants.DEFAULT_AGENT_PLUGIN_REVISION, + directory=directory, + manifest=manifest, + entry_module=str(manifest["entry_module"]), + ) + log_execution(spec) + + # The plugin directory is importable for exactly as long as the plugin runs, + # not for the rest of the process -- see :func:`plugin_syspath`. + with plugin_syspath(spec.directory): + try: + module = load_plugin(spec) + operation, func = select_operation(module) + except NotSupportedError as exc: + # Imported but nothing was runnable. Say that the download succeeded + # and that no agent work happened, or the message reads like a + # network failure and sends the user off checking the wrong thing. + raise NotSupportedError( + f"plugin {plugin_repo_id}@{spec.revision} downloaded and verified, but was " + f"NOT executed -- no agent was fetched or installed. {exc}" + ) from exc + except Exception as exc: + return InstallOutcome( + ok=False, + error=( + f"plugin {plugin_repo_id}@{spec.revision} downloaded and verified, but could " + f"not be imported, so it was NOT executed and no agent was fetched or " + f"installed: {exc.__class__.__name__}: {exc}" + ), + plugin=spec, + exit_code=1, + ) + + candidates: dict[str, Any] = { + "repo": repo, + "name": name, + "framework": framework, + "source_framework": framework, + "local_dir": local_dir, + # A fetch-only plugin writes where it is told and has no default, so the + # destination is always resolved here: the caller's --local-dir, else a + # staging directory. Operations that do not declare ``dest`` never see it. + "dest": local_dir or str(default_staging_dir(repo)), + "dry_run": dry_run, + "yes": yes, + "force": force, + "quiet": quiet, + "endpoint": endpoint, + "token": token, + } + # Unset optionals are dropped so the plugin applies its own defaults; a False + # boolean is kept because that is a decision the caller made. + provided = {key: value for key, value in candidates.items() if value is not None} + try: + result = func(**_accepted_kwargs(func, provided)) + except Exception as exc: + return InstallOutcome( + ok=False, + error=f"plugin {operation}() failed: {exc.__class__.__name__}: {exc}", + operation=operation, + plugin=spec, + exit_code=1, + ) + + # ``ok`` is required, not defaulted. It is the only signal deciding whether + # the user is told the agent was installed, so defaulting it to True let a + # plugin returning None, a bare string or an empty dict report success. Every + # other gate here fails closed; this one has to as well. + if not hasattr(result, "ok"): + return InstallOutcome( + ok=False, + error=( + f"plugin {operation}() returned {type(result).__name__} with no 'ok' attribute. " + "An entry operation must return a result carrying at least 'ok', plus 'error' " + "and 'exit_code' on failure. Refusing to report an unverifiable install as success." + ), + operation=operation, + plugin=spec, + result=result, + exit_code=1, + ) + + if bool(result.ok): + return InstallOutcome(ok=True, operation=operation, plugin=spec, result=result) + + error = getattr(result, "error", None) or f"plugin {operation}() reported failure" + try: + exit_code = int(getattr(result, "exit_code", 0) or 0) + except (TypeError, ValueError): + exit_code = 0 + return InstallOutcome( + ok=False, + error=error, + operation=operation, + plugin=spec, + result=result, + exit_code=exit_code or 1, + ) diff --git a/src/modelscope_hub/cli/agent.py b/src/modelscope_hub/cli/agent.py index 1f0621a..0aced6c 100644 --- a/src/modelscope_hub/cli/agent.py +++ b/src/modelscope_hub/cli/agent.py @@ -1,11 +1,15 @@ # Copyright (c) Alibaba, Inc. and its affiliates. -"""``ms agent`` command -- low-level raw file transfer for agent repositories. +"""``ms agent`` command -- agent repository transfer and plugin-driven install. -This is the *slim* Hub CLI. It supports only ``download``/``upload``/``list`` -for raw file transfer to and from remote agent repositories; Agent-IDP identity, -Ed25519-key, and token operations live in ``ms agent-idp``. Framework-aware -operations (convert, watch/sync, status, backups, restore, stop) live in -**modelscope-agent** -- use ``ms-agent agent ...``. +``download`` / ``upload`` / ``list`` are the *slim* Hub CLI: raw file transfer to +and from remote agent repositories, with no framework awareness. Agent-IDP +identity, Ed25519-key and token operations live in ``ms agent-idp``. +Framework-aware operations (convert, watch/sync, status, backups, restore, stop) +live in **modelscope-agent** -- use ``ms-agent agent ...``. + +``install`` is the exception, and it keeps that boundary by delegating rather +than knowing: it fetches a framework plugin and hands the agent id over, so no +framework file layout enters this distribution. """ from __future__ import annotations @@ -15,10 +19,15 @@ from argparse import RawDescriptionHelpFormatter from pathlib import Path -from ..agent import AgentApi, agent_last_modified, agent_visibility_label, is_lfs_file -from ..constants import Visibility +from ..agent import AgentApi, agent_last_modified, agent_visibility_label, install_agent, is_lfs_file +from ..constants import ( + AGENT_PLUGIN_TRUSTED_OWNERS, + DEFAULT_AGENT_PLUGIN_REPO, + Visibility, +) from ..errors import APIError -from .base import CLICommand, SubParsers +from .base import CLICommand, SubParsers, info, success +from .compat import add_subcmd_token_endpoint _CONVERT_HINT = ( "This command transfers raw files only. For framework-aware conversion, " @@ -250,11 +259,80 @@ def _cmd_upload(repo, local_dir, revision, dry_run, *, endpoint, token, username return 0 +def _cmd_install( + repo, + *, + name, + framework, + local_dir, + dry_run, + yes, + force, + quiet, + plugin_repo, + plugin_revision, + endpoint, + token, +) -> int: + """Install an agent through its framework plugin. + + The gates in :func:`install_agent` raise rather than return a code, and are + deliberately not caught here so ``run_cmd`` maps them to exit 2: a + misconfigured command line is a different failure from a failed install. + + The plugin's exit code passes through unchanged. The install layer gives + 3/4/5/6 distinct meanings (already exists, refused to overwrite, install or + self-check failed, framework mismatch); collapsing them to 1 would discard + the only machine-readable signal a caller has. + """ + outcome = install_agent( + repo, + name=name, + framework=framework, + local_dir=local_dir, + dry_run=dry_run, + yes=yes, + force=force, + quiet=quiet, + plugin_repo=plugin_repo, + plugin_revision=plugin_revision, + endpoint=endpoint, + token=token, + ) + + plugin = outcome.plugin + if plugin is not None and not quiet: + info(f"plugin: {plugin.repo_id}@{plugin.revision} (version {plugin.version})") + if outcome.operation: + info(f"entry : {plugin.entry_module}.{outcome.operation}()") + info(f"scope : {plugin.scope()}") + + if not outcome.ok: + _fail(outcome.error or "install failed") + return outcome.exit_code or 1 + if outcome.exit_code: + # Reported success but a non-zero code; trust the code. + return outcome.exit_code + + if not quiet: + result = outcome.result + written = getattr(result, "files_written", None) + root = getattr(result, "root", None) + # ``fetch_raw`` stages files for the install layer to place; reporting + # "Installed" would hide that no framework was touched. + verb, where = ("Fetched", "to") if outcome.operation == "fetch_raw" else ("Installed", "under") + if written is not None and root is not None: + success(f"{verb} {repo}: {len(written)} file(s) {where} {root}") + else: + success(f"{verb} {repo}") + return 0 + + # --------------------------------------------------------------------------- # CLI command # --------------------------------------------------------------------------- class AgentCommand(CLICommand): - """Raw agent-repository file transfer: download, upload, list.""" + """Agent repositories: raw file transfer, plus plugin-driven install.""" @staticmethod def register(subparsers: SubParsers) -> None: @@ -263,19 +341,28 @@ def register(subparsers: SubParsers) -> None: " download -r REPO [--local-dir DIR] [--revision REV]\n" " upload -r REPO [--local-dir DIR] [--revision REV] [--dry-run]\n" " list [--owner OWNER] [--page N] [--page-size N]\n" + " install -r REPO [--plugin-repo OWNER/NAME]\n" + " [-n NAME] [--framework FW] [--local-dir DIR] [--plugin-revision REV]\n" + " [--dry-run] [-y] [--force] [-q]\n" "\n" "note:\n" f" {_CONVERT_HINT}\n" + " `install` delegates to a framework plugin; see `ms agent install --help`.\n" "\n" "examples:\n" " ms agent download -r user/my-agent --local-dir ./my-agent\n" " ms agent upload -r user/my-agent --local-dir ./my-agent\n" " ms agent list --owner user\n" + " ms agent install -r user/my-agent\n" ) agent_parser = subparsers.add_parser( "agent", - help="Transfer raw agent repository files (download, upload, list).", - description="Low-level raw file transfer for remote agent repositories. " + _CONVERT_HINT, + help="Agent repositories: raw file transfer (download, upload, list) and install.", + description=( + "Work with remote agent repositories. `download`/`upload`/`list` are low-level raw " + "file transfer. " + _CONVERT_HINT + " `install` instead resolves a framework plugin, " + "fetches it from a model repository, and delegates the install to it." + ), epilog=_epilog, formatter_class=RawDescriptionHelpFormatter, ) @@ -342,6 +429,64 @@ def register(subparsers: SubParsers) -> None: "--page-size", dest="page_size", type=int, default=10, help="Number of items per page (default: 10)" ) + # ---- install ---- + p_install = agent_sub.add_parser( + "install", + help="Install an agent into its framework via the agent plugin", + formatter_class=RawDescriptionHelpFormatter, + description=( + "Download an agent repository and hand it to the framework plugin. A plugin with an " + "install entry point places the agent into the framework's workspace; one that only " + "transports bytes writes the files into a destination directory and leaves placement " + "to whatever runs next. Every run prints a 'scope :' line with what that plugin build " + "supports.\n\n" + f"The plugin is official code chosen by a compile-time owner allow-list " + f"({', '.join(sorted(AGENT_PLUGIN_TRUSTED_OWNERS))}), checked before any download and " + f"the whole authorisation: an allow-listed plugin is fetched and run with no separate " + f"confirmation. It receives your --endpoint and API token, since it needs credentials " + f"to fetch the agent." + ), + ) + p_install.add_argument( + "-r", + "--repo", + required=True, + help="Agent repository to install, in owner/name format (e.g. user/my-agent)", + ) + p_install.add_argument( + "-n", "--name", default=None, help="Sub-agent name to install (default: the plugin's choice)" + ) + p_install.add_argument("--framework", default=None, help="Override framework detection") + p_install.add_argument( + "--local-dir", + default=None, + help="Where the agent repository is downloaded, not where it is installed: an installing " + "plugin still puts the agent in the framework's own home (e.g. ~/.ms_agent, ~/.qwenpaw) " + "and leaves your directory alone. Omitted, downloads go to " + "$MODELSCOPE_CACHE/agent/agent-staging/ and are cleaned up on success.", + ) + p_install.add_argument( + "--plugin-repo", + default=None, + help=f"Plugin model repository, owner/name (default: {DEFAULT_AGENT_PLUGIN_REPO}). " + f"Its owner must be on the allow-list.", + ) + p_install.add_argument( + "--plugin-revision", + default=None, + help="Plugin revision to fetch (default: master; pin a tag for reproducible installs)", + ) + p_install.add_argument( + "--dry-run", + action="store_true", + help="Ask the plugin to report instead of change anything. The plugin is still downloaded, " + "imported and run -- only its writes are suppressed", + ) + p_install.add_argument("-y", "--yes", action="store_true", help="Answer the plugin's prompts yes") + p_install.add_argument("--force", action="store_true", help="Let the plugin overwrite an existing agent") + p_install.add_argument("-q", "--quiet", action="store_true", help="Suppress the plugin's progress output") + add_subcmd_token_endpoint(p_install) + def execute(self) -> None: args = self.args action = args.agent_command @@ -396,6 +541,21 @@ def execute(self) -> None: endpoint=endpoint, token=token, ) + elif action == "install": + rc = _cmd_install( + args.repo, + name=args.name, + framework=args.framework, + local_dir=args.local_dir, + dry_run=args.dry_run, + yes=args.yes, + force=args.force, + quiet=args.quiet, + plugin_repo=args.plugin_repo, + plugin_revision=args.plugin_revision, + endpoint=endpoint, + token=token, + ) else: print(f"Unknown agent action: {action}") rc = 1 diff --git a/src/modelscope_hub/constants.py b/src/modelscope_hub/constants.py index 1a9f970..5bfdc0a 100644 --- a/src/modelscope_hub/constants.py +++ b/src/modelscope_hub/constants.py @@ -953,13 +953,48 @@ def get_upload_ignore_file_pattern() -> str | None: USER_INFO_FILE_NAME: str = "user" +# --------------------------------------------------------------------------- +# Agent plugin loading (``ms agent install``) +# +# These constrain where the plugin that ``ms agent install`` imports may come +# from. The security model they serve is documented in +# :mod:`modelscope_hub.agent._plugin`. +# --------------------------------------------------------------------------- +ENV_AGENT_PLUGIN_REPO: str = "MODELSCOPE_AGENT_PLUGIN_REPO" + +#: Owners allowed to provide the agent plugin. +#: +#: A compile-time constant with **no environment override**, on purpose. This list +#: is the trust anchor for a command that executes downloaded code, and an anchor +#: any parent process can rewrite through the environment is not an anchor: a +#: script that can set env vars could point ``ms agent install`` at a repository +#: it controls. Deciding who is trusted is a reviewed code change. +#: +#: ``MODELSCOPE_AGENT_PLUGIN_REPO`` *is* overridable and that is safe: it chooses +#: which repository to fetch, but the owner still has to appear here, so it can +#: pick among already-trusted owners without widening trust. +AGENT_PLUGIN_TRUSTED_OWNERS: frozenset[str] = frozenset({"modelscope", "AI-ModelScope"}) + +DEFAULT_AGENT_PLUGIN_REPO: str = "modelscope/agent-hub-plugin" +DEFAULT_AGENT_PLUGIN_REVISION: str = "master" + +_env_register( + ENV_AGENT_PLUGIN_REPO, + DEFAULT_AGENT_PLUGIN_REPO, + "Model repository id ('owner/name') of the agent plugin used by 'ms agent install'", + "Core", +) + __all__ = [ + "AGENT_PLUGIN_TRUSTED_OWNERS", "API_CONNECT_TIMEOUT", "API_MAX_RETRIES", "API_TIMEOUT", "CATEGORY_ORDER", "CONFIG_DIR_NAME", "DATASET_LFS_SUFFIX", + "DEFAULT_AGENT_PLUGIN_REPO", + "DEFAULT_AGENT_PLUGIN_REVISION", "DEFAULT_CACHE_DIR_NAME", "DEFAULT_CREDENTIALS_PATH", "DEFAULT_DATASET_REVISION", @@ -979,6 +1014,7 @@ def get_upload_ignore_file_pattern() -> str | None: "DOWNLOAD_PART_SIZE", "DOWNLOAD_RETRY_TIMES", "DOWNLOAD_TIMEOUT", + "ENV_AGENT_PLUGIN_REPO", "ENV_FILE_LOCK", "ENV_CACHE", "ENV_INTRA_CLOUD_ACCELERATION", diff --git a/tests/cli/test_agent_install.py b/tests/cli/test_agent_install.py new file mode 100644 index 0000000..e5dbb69 --- /dev/null +++ b/tests/cli/test_agent_install.py @@ -0,0 +1,344 @@ +# Copyright (c) Alibaba, Inc. and its affiliates. +"""CLI tests for ``ms agent install``. + +These cover what the CLI layer owns: argument wiring, gate failures mapping to +exit 2, the plugin's exit code passing through unchanged, and output routing +(including ``-q``). The gates' own logic is tested in ``tests/test_agent_plugin.py`` +and is not repeated here. + +Mock-only: CI runs with ``MODELSCOPE_RUN_REMOTE_TESTS=false``. The trust gate +fires after the package is on disk, so it stubs ``fetch_plugin`` and points at a +real plugin tree -- manifest verification and the refusal message are genuine. +""" + +from __future__ import annotations + +import hashlib +import json +import sys +import textwrap +from pathlib import Path +from typing import Any +from unittest.mock import MagicMock + +import pytest + +from modelscope_hub import constants +from modelscope_hub.agent import InstallOutcome, PluginSpec, _plugin +from modelscope_hub.cli.agent import AgentCommand +from modelscope_hub.errors import NotSupportedError + +from .conftest import run_cli + +TRUSTED = "mushenL" +PLUGIN_REPO = f"{TRUSTED}/agent-hub-plugin" +AGENT_REPO = "owner/my-agent" + +ENTRY = textwrap.dedent( + """ + def capabilities(): + return {"operations": ("install",)} + + def install(repo, **kwargs): + return type("R", (), {"ok": True, "error": None, "exit_code": 0, + "files_written": ("SOUL.md",), "root": "/tmp/ws"})() + """ +).lstrip() + +ALL_OPTIONS = [ + "agent", + "install", + "-r", + AGENT_REPO, + "-n", + "sub", + "--framework", + "qwenpaw", + "--local-dir", + "/tmp/ws", + "--plugin-repo", + PLUGIN_REPO, + "--plugin-revision", + "v1.0.0", + "--dry-run", + "-y", + "--force", + "-q", +] + +MINIMAL = ["agent", "install", "-r", AGENT_REPO, "--plugin-repo", PLUGIN_REPO] + + +def build_plugin(root: Path, *, entry_module: str = "cli_fake_plugin") -> Path: + directory = root / "plugin" + directory.mkdir(parents=True, exist_ok=True) + entry = directory / f"{entry_module}.py" + entry.write_text(ENTRY, encoding="utf-8") + manifest = { + "name": "agent-hub-plugin", + "version": "9.9.9", + "entry_module": entry_module, + "frameworks": ["qwenpaw", "ms-agent"], + "api": ["install"], + "content_sha256": {f"{entry_module}.py": hashlib.sha256(entry.read_bytes()).hexdigest()}, + } + (directory / "plugin.json").write_text(json.dumps(manifest, indent=2), encoding="utf-8") + return directory + + +def outcome(**kwargs) -> InstallOutcome: + return InstallOutcome(**kwargs) + + +def spec(revision: str = "v1.0.0") -> PluginSpec: + return PluginSpec( + repo_id=PLUGIN_REPO, + owner=TRUSTED, + name="agent-hub-plugin", + revision=revision, + directory=Path("/tmp/nowhere"), + manifest={"version": "9.9.9"}, + entry_module="mod", + ) + + +@pytest.fixture(autouse=True) +def _clean_env(monkeypatch): + monkeypatch.setattr(constants, "AGENT_PLUGIN_TRUSTED_OWNERS", frozenset({TRUSTED, "modelscope"})) + monkeypatch.delenv(constants.ENV_AGENT_PLUGIN_REPO, raising=False) + yield + sys.modules.pop("cli_fake_plugin", None) + + +@pytest.fixture +def stub_sdk(monkeypatch): + """Replace the SDK entry point and capture what the CLI forwarded.""" + seen: dict[str, Any] = {} + + def fake_install(repo, **kwargs): + seen.update(repo=repo, **kwargs) + return InstallOutcome(ok=True, operation="install") + + monkeypatch.setattr("modelscope_hub.cli.agent.install_agent", fake_install) + return seen + + +# --------------------------------------------------------------------------- +# argument wiring +# --------------------------------------------------------------------------- +def test_parser_wires_install(parser): + args = parser.parse_args(ALL_OPTIONS) + assert args._command is AgentCommand + assert args.agent_command == "install" + assert (args.repo, args.name, args.framework, args.local_dir) == ( + AGENT_REPO, + "sub", + "qwenpaw", + "/tmp/ws", + ) + assert (args.plugin_repo, args.plugin_revision) == (PLUGIN_REPO, "v1.0.0") + assert args.dry_run and args.yes and args.force and args.quiet + + +def test_parser_install_defaults(parser): + args = parser.parse_args(["agent", "install", "-r", AGENT_REPO]) + assert args.plugin_repo is None + assert args.plugin_revision is None + assert args.dry_run is False + assert args.yes is False + assert args.force is False + assert args.quiet is False + assert args.name is None + assert args.framework is None + assert args.local_dir is None + + +def test_forwards_every_option_to_the_sdk(stub_sdk): + code, _, err = run_cli(ALL_OPTIONS) + assert code == 0, err + assert stub_sdk["repo"] == AGENT_REPO + assert stub_sdk["name"] == "sub" + assert stub_sdk["framework"] == "qwenpaw" + assert stub_sdk["local_dir"] == "/tmp/ws" + assert stub_sdk["plugin_repo"] == PLUGIN_REPO + assert stub_sdk["plugin_revision"] == "v1.0.0" + assert stub_sdk["dry_run"] and stub_sdk["yes"] + assert stub_sdk["force"] and stub_sdk["quiet"] + + +def test_credentials_reach_the_sdk(stub_sdk): + code, _, err = run_cli(MINIMAL, token="tok-123", endpoint="https://pre.modelscope.cn") + assert code == 0, err + assert stub_sdk["token"] == "tok-123" + assert stub_sdk["endpoint"] == "https://pre.modelscope.cn" + + +def test_install_does_not_resolve_a_username(monkeypatch, stub_sdk): + """``install`` always receives ``owner/name``, so it must not pay for a whoami + round trip -- nor fail when that endpoint is unavailable.""" + from modelscope_hub import _openapi + + client = MagicMock(side_effect=AssertionError("whoami must not be called")) + monkeypatch.setattr(_openapi, "OpenAPIClient", client) + + code, _, err = run_cli(MINIMAL, token="tok-123") + assert code == 0, err + client.assert_not_called() + + +# --------------------------------------------------------------------------- +# gates: exit code 2, and where the message lands +# --------------------------------------------------------------------------- +def test_unfetchable_default_plugin_repo_points_at_the_override(monkeypatch): + """The default repository is not the user's choice, so a bare download error + would leave them nothing to act on. Stubbed: this must not reach the network.""" + + def refused(repo_id, **kwargs): + assert repo_id == constants.DEFAULT_AGENT_PLUGIN_REPO, "the built-in default should be used" + raise NotSupportedError(f"failed to download agent plugin {repo_id}@master: record not found") + + monkeypatch.setattr(_plugin, "fetch_plugin", refused) + code, out, err = run_cli(["agent", "install", "-r", AGENT_REPO]) + assert code != 0 + combined = out + err + assert constants.DEFAULT_AGENT_PLUGIN_REPO in combined + assert "--plugin-repo" in combined + for owner in sorted(constants.AGENT_PLUGIN_TRUSTED_OWNERS): + assert owner in combined + + +def test_untrusted_plugin_owner_exits_2(): + code, out, err = run_cli(["agent", "install", "-r", AGENT_REPO, "--plugin-repo", "evil/plugin"]) + assert code == 2 + assert "evil" in err + combined = out + err + # The refusal must say the list is compiled in, so nobody hunts for an + # environment variable that no longer exists. + assert "AGENT_PLUGIN_TRUSTED_OWNERS" in combined + assert "compile-time" in combined + + +def test_an_allow_listed_plugin_runs_with_no_confirmation(monkeypatch, tmp_path): + """The allow-list is the whole authorisation, so an allow-listed plugin is + downloaded and executed with nothing for the user to confirm -- and the + command no longer accepts a flag that would imply otherwise.""" + directory = build_plugin(tmp_path) + monkeypatch.setattr(_plugin, "fetch_plugin", lambda repo_id, **kwargs: directory) + + code, out, err = run_cli(["agent", "install", "-r", AGENT_REPO, "--plugin-repo", PLUGIN_REPO]) + assert code == 0, err + assert "Installed" in out + + +def test_the_trust_flag_is_gone(monkeypatch, tmp_path): + """Deferred with third-party plugin support. Passing it must be an argparse + error rather than a silently ignored extra, so nobody believes they opted in.""" + directory = build_plugin(tmp_path) + monkeypatch.setattr(_plugin, "fetch_plugin", lambda repo_id, **kwargs: directory) + + code, out, err = run_cli( + ["agent", "install", "-r", AGENT_REPO, "--plugin-repo", PLUGIN_REPO, "--trust-remote-code"] + ) + assert code != 0 + assert "unrecognized arguments" in (out + err) or "--trust-remote-code" in (out + err) + + +# --------------------------------------------------------------------------- +# reporting and exit-code mapping +# --------------------------------------------------------------------------- +def test_reports_plugin_and_entry(monkeypatch): + monkeypatch.setattr( + "modelscope_hub.cli.agent.install_agent", + lambda *a, **k: outcome(ok=True, operation="install", plugin=spec()), + ) + code, out, _ = run_cli(MINIMAL) + assert code == 0 + assert f"{PLUGIN_REPO}@v1.0.0" in out + assert "9.9.9" in out + assert "mod.install()" in out + + +@pytest.mark.parametrize( + ("operation", "verb", "where"), + [ + ("install", "Installed", "under"), + ("download", "Installed", "under"), + ("fetch_raw", "Fetched", "to"), + ], +) +def test_success_wording_follows_the_negotiated_operation(monkeypatch, operation, verb, where): + """A transport-only plugin stages files; reporting "Installed" would hide + that no framework was touched.""" + result = type("R", (), {"files_written": ("SOUL.md", "AGENTS.md"), "root": "/tmp/staged"})() + monkeypatch.setattr( + "modelscope_hub.cli.agent.install_agent", + lambda *a, **k: outcome(ok=True, operation=operation, plugin=spec(), result=result), + ) + code, out, _ = run_cli(MINIMAL) + assert code == 0 + assert f"{verb} {AGENT_REPO}: 2 file(s) {where} /tmp/staged" in out + + +def test_reports_the_supported_scope(monkeypatch): + """A successful run must show what that plugin build covers, not just an exit + code -- which frameworks, which operations work, and which are declared but + not implemented yet.""" + rich = PluginSpec( + repo_id=PLUGIN_REPO, + owner=TRUSTED, + name="agent-hub-plugin", + revision="v0.2.0", + directory=Path("/tmp/nowhere"), + manifest={ + "version": "0.2.0", + "frameworks": ["ms-agent", "qwenpaw"], + "api": ["fetch_raw", "list_backups", "restore"], + "roadmap": {"install": "entry package", "upload": "P1", "convert": "P2"}, + "content_sha256": {"agent_hub_core/__init__.py": "0" * 64}, + }, + entry_module="agent_hub_core", + ) + result = type("R", (), {"files_written": ("SOUL.md",), "root": "/tmp/staged"})() + monkeypatch.setattr( + "modelscope_hub.cli.agent.install_agent", + lambda *a, **k: outcome(ok=True, operation="fetch_raw", plugin=rich, result=result), + ) + code, out, _ = run_cli(MINIMAL) + assert code == 0 + assert "scope : frameworks ms-agent, qwenpaw" in out + assert "operations fetch_raw, list_backups, restore" in out + assert "planned convert (P2), install (entry package), upload (P1)" in out + + +def test_quiet_suppresses_all_hub_output(monkeypatch): + monkeypatch.setattr( + "modelscope_hub.cli.agent.install_agent", + lambda *a, **k: outcome(ok=True, operation="install", plugin=spec()), + ) + code, out, _ = run_cli([*MINIMAL, "-q"]) + assert code == 0 + assert out == "" + + +@pytest.mark.parametrize("plugin_code", [1, 3, 6]) +def test_plugin_exit_code_is_passed_through(monkeypatch, plugin_code): + """The install layer gives 3/4/5/6 distinct meanings (already exists, refused + to overwrite, install or self-check failed, framework mismatch); collapsing + them to 1 would discard the only machine-readable signal a caller has.""" + monkeypatch.setattr( + "modelscope_hub.cli.agent.install_agent", + lambda *a, **k: outcome(ok=False, error="nope", exit_code=plugin_code), + ) + code, _, err = run_cli(MINIMAL) + assert code == plugin_code + assert "nope" in err + + +def test_failure_without_an_exit_code_becomes_1(monkeypatch): + monkeypatch.setattr( + "modelscope_hub.cli.agent.install_agent", + lambda *a, **k: outcome(ok=False, error="download failed"), + ) + code, _, err = run_cli(MINIMAL) + assert code == 1 + assert "download failed" in err diff --git a/tests/test_agent_plugin.py b/tests/test_agent_plugin.py new file mode 100644 index 0000000..f32e6c0 --- /dev/null +++ b/tests/test_agent_plugin.py @@ -0,0 +1,924 @@ +# Copyright (c) Alibaba, Inc. and its affiliates. +"""Unit tests for the agent plugin loader (``modelscope_hub.agent._plugin``). + +Happy paths build a real plugin package on disk -- manifest, entry module and all +-- so verification, import and operation negotiation are exercised rather than +mocked. Only the network hop (``snapshot_download``) is stubbed, per the rule +that CI runs with ``MODELSCOPE_RUN_REMOTE_TESTS=false``. + +The gates themselves are tested here at function level; that they are *wired +into* ``install_agent`` and map to the right exit codes is covered by +``tests/cli/test_agent_install.py``, so it is not repeated at both levels. +""" + +from __future__ import annotations + +import hashlib +import importlib +import json +import re +import sys +import textwrap +from pathlib import Path +from typing import Any + +import pytest + +from modelscope_hub import constants +from modelscope_hub.agent import _plugin +from modelscope_hub.errors import InvalidParameter, NotSupportedError + +TRUSTED = "mushenL" +PLUGIN_REPO = f"{TRUSTED}/agent-hub-plugin" + + +def _sha256(path: Path) -> str: + return hashlib.sha256(path.read_bytes()).hexdigest() + + +ENTRY_SOURCE = textwrap.dedent( + """ + from dataclasses import dataclass, field + + + @dataclass(frozen=True) + class Result: + ok: bool = True + error: str | None = None + files_written: tuple = field(default_factory=tuple) + root: str = "/tmp/ws" + exit_code: int = 0 + + + CALLS = [] + + + def capabilities(): + return {"operations": ("install", "download"), "version": "9.9.9"} + + + def install(repo, **kwargs): + CALLS.append(("install", repo, kwargs)) + return Result(files_written=("SOUL.md",)) + + + def download(repo, **kwargs): + CALLS.append(("download", repo, kwargs)) + return Result(files_written=("SOUL.md",)) + """ +).lstrip() + + +def make_plugin( + root: Path, + *, + dirname: str = "plugin", + entry_module: str = "fake_plugin", + version: str = "9.9.9", + operations: tuple[str, ...] = ("install", "download"), + entry_source: str | None = None, + extra_files: dict[str, str] | None = None, + omit_hashes: bool = False, + omit_entry: bool = False, +) -> Path: + """Write a plugin package tree and return its directory.""" + directory = root / dirname + directory.mkdir(parents=True, exist_ok=True) + + source = entry_source if entry_source is not None else ENTRY_SOURCE + (directory / f"{entry_module}.py").write_text(source, encoding="utf-8") + for rel, content in (extra_files or {}).items(): + target = directory / rel + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(content, encoding="utf-8") + + manifest: dict[str, Any] = { + "name": "agent-hub-plugin", + "version": version, + "kind": "agent-hub-plugin", + "frameworks": ["qwenpaw", "ms-agent"], + "api": list(operations), + } + if not omit_entry: + manifest["entry_module"] = entry_module + if not omit_hashes: + files = sorted(p for p in directory.rglob("*") if p.is_file()) + manifest["content_sha256"] = {p.relative_to(directory).as_posix(): _sha256(p) for p in files} + (directory / "plugin.json").write_text(json.dumps(manifest, indent=2), encoding="utf-8") + return directory + + +def spec_for(directory: Path, *, manifest: dict | None = None) -> _plugin.PluginSpec: + resolved = manifest + if resolved is None: + resolved = json.loads((directory / "plugin.json").read_text(encoding="utf-8")) + return _plugin.PluginSpec( + repo_id=PLUGIN_REPO, + owner=TRUSTED, + name="agent-hub-plugin", + revision="master", + directory=directory, + manifest=resolved, + entry_module=str(resolved.get("entry_module", "fake_plugin")), + ) + + +def load_entry(directory: Path, module_name: str): + """Import a generated plugin entry module the way ``load_plugin`` does.""" + return _plugin.load_plugin(spec_for(directory, manifest={"entry_module": module_name})) + + +def loaded(directory: Path, module_name: str): + """The module ``install_agent`` loaded from *directory*. + + Reached through the alias, not ``import ``: registration is + directory-scoped on purpose, so the plain name is never in ``sys.modules`` and + the plugin directory is off ``sys.path`` again once the call returns. + """ + alias = _plugin._module_alias(spec_for(directory, manifest={"entry_module": module_name})) + return sys.modules[alias] + + +# --------------------------------------------------------------------------- +# resolve_plugin_repo +# --------------------------------------------------------------------------- +def test_resolve_plugin_repo_resolution_order(monkeypatch): + """Argument beats environment beats the built-in default.""" + monkeypatch.setenv(constants.ENV_AGENT_PLUGIN_REPO, "env-owner/env-plugin") + assert _plugin.resolve_plugin_repo("arg-owner/arg-plugin") == "arg-owner/arg-plugin" + assert _plugin.resolve_plugin_repo(None) == "env-owner/env-plugin" + assert _plugin.resolve_plugin_repo(" ") == "env-owner/env-plugin" + + monkeypatch.delenv(constants.ENV_AGENT_PLUGIN_REPO, raising=False) + assert _plugin.resolve_plugin_repo(None) == constants.DEFAULT_AGENT_PLUGIN_REPO + + +@pytest.mark.parametrize("value", ["noslash", "/noname", "owner/"]) +def test_resolve_plugin_repo_requires_owner_slash_name(monkeypatch, value): + monkeypatch.setenv(constants.ENV_AGENT_PLUGIN_REPO, value) + with pytest.raises(InvalidParameter): + _plugin.resolve_plugin_repo(None) + + +# --------------------------------------------------------------------------- +# assert_trusted_owner +# --------------------------------------------------------------------------- +@pytest.fixture +def allow_list(monkeypatch): + monkeypatch.setattr(constants, "AGENT_PLUGIN_TRUSTED_OWNERS", frozenset({"mushenL", "modelscope"})) + + +def test_assert_trusted_owner_accepts_allow_listed(allow_list): + assert _plugin.assert_trusted_owner("mushenL/agent-hub-plugin") == ("mushenL", "agent-hub-plugin") + assert _plugin.assert_trusted_owner("modelscope/agent-hub-plugin") == ("modelscope", "agent-hub-plugin") + + +@pytest.mark.parametrize("owner", ["evil", "mushenL-x", "modelscope2", "ai_modelscope"]) +def test_assert_trusted_owner_rejects_others(allow_list, owner): + with pytest.raises(InvalidParameter) as excinfo: + _plugin.assert_trusted_owner(f"{owner}/agent-hub-plugin") + assert owner in str(excinfo.value) + assert "AGENT_PLUGIN_TRUSTED_OWNERS" in excinfo.value.suggestion + + +def test_the_allow_list_cannot_be_widened_from_the_environment(monkeypatch): + """The allow-list is the trust anchor for a command that executes downloaded + code, so a parent process must not be able to move it. The override that used + to exist is gone; this pins that it stays gone rather than being reintroduced + as a convenience.""" + monkeypatch.setenv("MODELSCOPE_AGENT_PLUGIN_TRUSTED_OWNERS", "evilcorp") + importlib.reload(constants) + try: + assert "evilcorp" not in constants.AGENT_PLUGIN_TRUSTED_OWNERS + assert not hasattr(constants, "ENV_AGENT_PLUGIN_TRUSTED_OWNERS") + with pytest.raises(InvalidParameter): + _plugin.assert_trusted_owner("evilcorp/agent-hub-plugin") + finally: + importlib.reload(constants) + + +def test_the_shipped_allow_list_has_no_personal_account(): + """A personal account in the default list is a supply-chain entry point: if it + is compromised, anything it publishes passes the owner gate.""" + assert constants.AGENT_PLUGIN_TRUSTED_OWNERS == frozenset({"modelscope", "AI-ModelScope"}) + + +@pytest.mark.parametrize("owner", ["mushenl", "MUSHENL", "ModelScope", "MODELSCOPE", "ai-modelscope"]) +def test_assert_trusted_owner_matches_case_insensitively(monkeypatch, owner): + """The registry resolves ids case-insensitively and normalises the owner -- + ``ModelScope/x`` and ``modelscope/x`` are one repository -- so two owners + differing only in case cannot both exist. Matching exactly would not stop a + look-alike; it would only reject the casing somebody copied from the website, + which is how the product writes ``ModelScope``.""" + monkeypatch.setattr(constants, "AGENT_PLUGIN_TRUSTED_OWNERS", frozenset({"mushenL", "modelscope", "AI-ModelScope"})) + got_owner, got_name = _plugin.assert_trusted_owner(f"{owner}/agent-hub-plugin") + assert got_name == "agent-hub-plugin" + # Echoed as typed, so messages and PluginSpec keep the user's spelling. + assert got_owner == owner + + +def test_assert_trusted_owner_empty_list_blocks_everything(monkeypatch): + monkeypatch.setattr(constants, "AGENT_PLUGIN_TRUSTED_OWNERS", frozenset()) + with pytest.raises(InvalidParameter): + _plugin.assert_trusted_owner("mushenL/agent-hub-plugin") + + +# --------------------------------------------------------------------------- +# verify_manifest +# --------------------------------------------------------------------------- +def test_verify_manifest_accepts_a_consistent_package(tmp_path): + directory = make_plugin(tmp_path) + manifest = _plugin.verify_manifest(directory, PLUGIN_REPO) + assert manifest["entry_module"] == "fake_plugin" + assert manifest["version"] == "9.9.9" + + +@pytest.mark.parametrize( + "case,expected", + [ + ("absent", "not an agent plugin"), + ("no-entry", "entry_module"), + # Without a digest the import would be unconditional code execution. + ("no-hashes", "content_sha256"), + ], +) +def test_verify_manifest_rejects_an_incomplete_manifest(tmp_path, case, expected): + if case == "absent": + directory = tmp_path / "empty" + directory.mkdir() + elif case == "no-entry": + directory = make_plugin(tmp_path, omit_entry=True) + else: + directory = make_plugin(tmp_path, omit_hashes=True) + with pytest.raises(NotSupportedError) as excinfo: + _plugin.verify_manifest(directory, PLUGIN_REPO) + assert expected in str(excinfo.value) + + +@pytest.mark.parametrize( + "case,expected", + [ + ("tampered", "sha256 mismatch"), + ("missing", "missing"), + ("unlisted", "not listed in the manifest"), + ], +) +def test_verify_manifest_rejects_inconsistent_content(tmp_path, case, expected): + directory = make_plugin(tmp_path, extra_files={"pkg/mod.py": "x = 1\n"}) + if case == "tampered": + target = directory / "fake_plugin.py" + target.write_text(target.read_text(encoding="utf-8") + "\n# tampered\n", encoding="utf-8") + elif case == "missing": + (directory / "pkg" / "mod.py").unlink() + else: + (directory / "surprise.py").write_text("import os\n", encoding="utf-8") + with pytest.raises(NotSupportedError) as excinfo: + _plugin.verify_manifest(directory, PLUGIN_REPO) + assert expected in str(excinfo.value) + + +def test_verify_manifest_exempts_non_plugin_files(tmp_path): + """``.gitattributes`` is injected by the hub into every repository and so is + never in an author's manifest; refusing it made every real package + unverifiable. ``plugin.json`` cannot hash itself and ``__pycache__`` is + written locally by a previous import. + + The exemption is exact -- a genuinely unlisted file is still refused. + """ + directory = make_plugin(tmp_path) + (directory / ".gitattributes").write_text("*.bin filter=lfs\n", encoding="utf-8") + cache = directory / "__pycache__" + cache.mkdir() + (cache / "fake_plugin.cpython-311.pyc").write_bytes(b"\x00\x01") + + manifest = _plugin.verify_manifest(directory, PLUGIN_REPO) + listed = manifest["content_sha256"] + assert "plugin.json" not in listed + assert ".gitattributes" not in listed + assert not any("__pycache__" in key for key in listed) + + (directory / "surprise.py").write_text("import os\n", encoding="utf-8") + with pytest.raises(NotSupportedError) as excinfo: + _plugin.verify_manifest(directory, PLUGIN_REPO) + assert "surprise.py" in str(excinfo.value) + assert ".gitattributes" not in str(excinfo.value) + + +@pytest.mark.parametrize("key", ["/etc/hosts", "../../etc/hosts", "pkg/../../outside.py"]) +def test_verify_manifest_refuses_keys_pointing_outside_the_package(tmp_path, key): + """Manifest keys are attacker-controlled and become paths. ``directory / key`` + with an absolute key discards the directory outright, so a hostile manifest + could have the loader hash a file anywhere on disk. Nothing is returned to the + caller, so it is not a disclosure -- but it is a read the manifest has no + business requesting, and a package that asks for it is not a corrupt download. + """ + directory = make_plugin(tmp_path) + manifest = json.loads((directory / "plugin.json").read_text()) + manifest["content_sha256"][key] = "0" * 64 + (directory / "plugin.json").write_text(json.dumps(manifest)) + + with pytest.raises(NotSupportedError) as excinfo: + _plugin.verify_manifest(directory, PLUGIN_REPO) + assert "outside the package" in str(excinfo.value) + + +# --------------------------------------------------------------------------- +# execution audit / no opt-in +# --------------------------------------------------------------------------- +def test_log_execution_records_the_build_before_import(tmp_path, caplog): + """There is no opt-in to wait for any more -- the allow-list is the + authorisation -- but executing downloaded code still deserves an audit line + naming the exact build, emitted before the import so a crash during it leaves + a trace of what was being loaded.""" + spec = spec_for(make_plugin(tmp_path)) + with caplog.at_level("INFO", logger="modelscope_hub.agent"): + _plugin.log_execution(spec) + for expected in (spec.repo_id, spec.revision, spec.entry_module, "9.9.9"): + assert expected in caplog.text + + +def test_no_trust_opt_in_is_exposed(): + """Deferred on purpose: the allow-list already decided, so a per-invocation + flag would imply a choice the user does not have. Pin that it stays gone until + third-party plugins are supported.""" + import inspect + + assert not hasattr(constants, "AGENT_TRUST_REMOTE_CODE") + assert not hasattr(constants, "ENV_AGENT_TRUST_REMOTE_CODE") + assert not hasattr(_plugin, "require_trust") + assert "trust_remote_code" not in inspect.signature(_plugin.install_agent).parameters + + # Nor in the user-facing docs: naming an opt-in that does not exist invites + # users to reach for it, and a stale example is a call that raises TypeError. + readme = (Path(__file__).resolve().parents[1] / "README.md").read_text(encoding="utf-8") + assert "trust_remote_code" not in readme + assert "trust-remote-code" not in readme + + +# --------------------------------------------------------------------------- +# fetch_plugin +# --------------------------------------------------------------------------- +def test_fetch_plugin_requests_a_model_repo(monkeypatch, tmp_path): + seen: list[dict[str, Any]] = [] + + def fake_snapshot_download(repo_id, **kwargs): + seen.append({"repo_id": repo_id, **kwargs}) + return str(tmp_path) + + import modelscope_hub.compat as compat + + monkeypatch.setattr(compat, "snapshot_download", fake_snapshot_download) + + got = _plugin.fetch_plugin(PLUGIN_REPO, revision="v1.2.3", token="tok", endpoint="https://ep") + assert got == tmp_path + assert seen[0]["repo_id"] == PLUGIN_REPO + assert seen[0]["repo_type"] == "model" + assert seen[0]["revision"] == "v1.2.3" + assert (seen[0]["token"], seen[0]["endpoint"]) == ("tok", "https://ep") + + _plugin.fetch_plugin(PLUGIN_REPO) + assert seen[1]["revision"] == constants.DEFAULT_AGENT_PLUGIN_REVISION + + +def test_fetch_plugin_wraps_download_failure(monkeypatch): + """``snapshot_download`` re-raises hub errors as ``requests.HTTPError``, so + the original type is not a reliable discriminator.""" + + def boom(repo_id, **kwargs): + raise RuntimeError("404 not found") + + import modelscope_hub.compat as compat + + monkeypatch.setattr(compat, "snapshot_download", boom) + with pytest.raises(NotSupportedError) as excinfo: + _plugin.fetch_plugin(PLUGIN_REPO) + assert PLUGIN_REPO in str(excinfo.value) + assert isinstance(excinfo.value.__cause__, RuntimeError) + + +# --------------------------------------------------------------------------- +# load_plugin / select_operation +# --------------------------------------------------------------------------- +def test_load_plugin_imports_the_declared_entry_module(tmp_path): + directory = make_plugin(tmp_path, entry_module="entry_a") + module = _plugin.load_plugin(spec_for(directory)) + assert module.__file__ == str(directory / "entry_a.py") + # Registered under a directory-scoped alias, not its own name -- see the next + # test for why that matters. + assert module.__name__.startswith("_ms_agent_plugin_entry_a_") + assert str(directory) in sys.path + sys.path.remove(str(directory)) + sys.modules.pop(module.__name__, None) + + +def test_load_plugin_does_not_serve_one_directory_another(tmp_path): + """Two plugins sharing a repository id, revision and entry module name must + not share code. ``import_module`` would have returned the first from + ``sys.modules`` for the second, so installing plugin B ran plugin A.""" + first = make_plugin(tmp_path / "one", dirname="p", entry_module="same_name", entry_source="MARKER = 'first'\n") + second = make_plugin(tmp_path / "two", dirname="p", entry_module="same_name", entry_source="MARKER = 'second'\n") + loaded = [] + try: + for directory in (first, second): + sys.path.insert(0, str(directory)) + loaded.append(_plugin.load_plugin(spec_for(directory))) + assert [m.MARKER for m in loaded] == ["first", "second"] + assert loaded[0] is not loaded[1] + finally: + for m in loaded: + sys.modules.pop(m.__name__, None) + for directory in (first, second): + if str(directory) in sys.path: + sys.path.remove(str(directory)) + + +def test_load_plugin_restores_sys_path_on_failure(tmp_path): + directory = tmp_path / "plugin" + directory.mkdir() + spec = _plugin.PluginSpec( + repo_id=PLUGIN_REPO, + owner=TRUSTED, + name="p", + revision="master", + directory=directory, + manifest={}, + entry_module="does_not_exist_xyz", + ) + before = list(sys.path) + with pytest.raises(ImportError): + _plugin.load_plugin(spec) + assert sys.path == before + + +def test_plugin_syspath_is_scoped_to_the_block(tmp_path): + """A plugin directory left at sys.path[0] lets any file it ships shadow the + standard library or a dependency for the rest of the process -- and shipping + one is not a rule violation, since every file has to be listed in the + manifest. Harmless in a one-shot CLI; a long-lived process calling + install_agent would stay poisoned.""" + directory = make_plugin(tmp_path) + before = list(sys.path) + with _plugin.plugin_syspath(directory): + assert str(directory) in sys.path + assert sys.path[0] == str(directory) + assert sys.path == before + + +def test_install_agent_leaves_no_plugin_directory_on_sys_path(wired): + _plugin.install_agent("owner/my-agent", plugin_repo=PLUGIN_REPO) + assert str(wired) not in sys.path + + +def test_select_operation_prefers_install(tmp_path): + module = load_entry(make_plugin(tmp_path, entry_module="sel_install"), "sel_install") + name, func = _plugin.select_operation(module) + assert (name, func) == ("install", module.install) + + +def test_select_operation_falls_back_to_download(tmp_path): + source = textwrap.dedent( + """ + def capabilities(): + return {"operations": ("download",)} + + def install(repo, **kwargs): + raise AssertionError("must not be chosen") + + def download(repo, **kwargs): + return "downloaded" + """ + ).lstrip() + directory = make_plugin(tmp_path, entry_module="sel_download", entry_source=source) + module = load_entry(directory, "sel_download") + name, func = _plugin.select_operation(module) + assert (name, func) == ("download", module.download) + + +def test_select_operation_rejects_an_undeclared_name(tmp_path): + """A plugin shipping a name without declaring it in ``capabilities()`` is not + trusted to have implemented it.""" + source = textwrap.dedent( + """ + def capabilities(): + return {"operations": ()} + + def install(repo, **kwargs): + raise AssertionError("must not be chosen") + """ + ).lstrip() + directory = make_plugin(tmp_path, entry_module="sel_none", entry_source=source) + module = load_entry(directory, "sel_none") + with pytest.raises(NotSupportedError) as excinfo: + _plugin.select_operation(module) + assert "sel_none" in str(excinfo.value) + + +def test_select_operation_refuses_a_broken_capabilities(tmp_path): + """A ``capabilities()`` that exists and fails means the plugin is broken, not + that it declares nothing. Swallowing it downgraded selection to "first + callable attribute wins", which can pick a placeholder the plugin deliberately + left undeclared.""" + source = textwrap.dedent( + """ + def capabilities(): + raise RuntimeError("manifest and code disagree") + + def install(repo, **kwargs): + raise AssertionError("must not be chosen") + """ + ).lstrip() + directory = make_plugin(tmp_path, entry_module="broken_caps", entry_source=source) + module = load_entry(directory, "broken_caps") + with pytest.raises(NotSupportedError) as excinfo: + _plugin.select_operation(module) + assert "capabilities() raised" in str(excinfo.value) + assert "manifest and code disagree" in str(excinfo.value) + + +def test_select_operation_without_capabilities_uses_presence(tmp_path): + source = "def download(repo, **kwargs):\n return 'ok'\n" + directory = make_plugin(tmp_path, entry_module="sel_nocaps", entry_source=source) + assert _plugin.select_operation(load_entry(directory, "sel_nocaps"))[0] == "download" + + +def test_select_operation_prefers_install_over_fetch_raw(tmp_path): + """A plugin that owns placement wins over one that only transports bytes, so + the install layer taking over needs no hub release.""" + source = textwrap.dedent( + """ + def capabilities(): + return {"operations": ("install", "fetch_raw", "download")} + + def install(repo, **kwargs): + return "installed" + + def fetch_raw(repo, *, dest, **kwargs): + raise AssertionError("must not be chosen while install is declared") + """ + ).lstrip() + directory = make_plugin(tmp_path, entry_module="sel_pref", entry_source=source) + module = load_entry(directory, "sel_pref") + assert _plugin.select_operation(module) == ("install", module.install) + + +def test_select_operation_falls_back_to_fetch_raw(tmp_path): + """A transport-only plugin is usable: ``download`` is present but undeclared, + so it must not be chosen over the operation the plugin actually reports.""" + source = textwrap.dedent( + """ + def capabilities(): + return {"operations": ("fetch_raw", "restore", "list_backups")} + + def install(repo, **kwargs): + raise AssertionError("must not be chosen") + + def download(repo, **kwargs): + raise AssertionError("must not be chosen") + + def fetch_raw(repo, *, dest, **kwargs): + return "fetched" + """ + ).lstrip() + directory = make_plugin(tmp_path, entry_module="sel_fetch", entry_source=source) + module = load_entry(directory, "sel_fetch") + assert _plugin.select_operation(module) == ("fetch_raw", module.fetch_raw) + + +def test_accepted_kwargs_narrowing(): + def positional(repo, name=None): + return repo, name + + assert _plugin._accepted_kwargs(positional, {"repo": "a/b", "name": "x", "force": True}) == { + "repo": "a/b", + "name": "x", + } + + def variadic(**kwargs): + return kwargs + + payload = {"repo": "a/b", "anything": 1} + assert _plugin._accepted_kwargs(variadic, payload) == payload + + +# --------------------------------------------------------------------------- +# install_agent +# --------------------------------------------------------------------------- +@pytest.fixture +def wired(monkeypatch, tmp_path): + """Point ``install_agent`` at a real plugin tree with the network stubbed.""" + monkeypatch.setattr(constants, "AGENT_PLUGIN_TRUSTED_OWNERS", frozenset({TRUSTED})) + monkeypatch.delenv(constants.ENV_AGENT_PLUGIN_REPO, raising=False) + # install_agent resolves a default staging directory under the cache; keep it + # out of the real user home. + monkeypatch.setenv(constants.ENV_CACHE, str(tmp_path / "cache")) + directory = make_plugin(tmp_path, entry_module="e2e_plugin") + monkeypatch.setattr(_plugin, "fetch_plugin", lambda repo_id, **kwargs: directory) + yield directory + sys.modules.pop(_plugin._module_alias(spec_for(directory)), None) + + +def test_install_agent_happy_path_and_option_forwarding(wired): + outcome = _plugin.install_agent("owner/my-agent", plugin_repo=PLUGIN_REPO) + assert outcome.ok, outcome.error + assert (outcome.operation, outcome.exit_code) == ("install", 0) + assert outcome.plugin.repo_id == PLUGIN_REPO + assert outcome.plugin.version == "9.9.9" + + entry = loaded(wired, "e2e_plugin") + assert entry.CALLS[-1][:2] == ("install", "owner/my-agent") + forwarded = entry.CALLS[-1][2] + # Unset optionals are dropped so the plugin applies its own defaults, but a + # False boolean is a decision the caller made and is forwarded. + assert {key: forwarded[key] for key in ("dry_run", "yes", "force", "quiet")} == { + "dry_run": False, + "yes": False, + "force": False, + "quiet": False, + } + # A variadic entry also gets the resolved destination. With no --local-dir + # that is a fresh staging directory under the cache, not a workspace. + staged = Path(forwarded["dest"]) + assert staged.parent.name == "agent-staging" + assert staged.name.startswith("owner--my-agent-") + + _plugin.install_agent( + "owner/my-agent", + name="sub", + local_dir="/tmp/ws", + dry_run=True, + force=True, + endpoint="https://pre.modelscope.cn", + token="tok", + plugin_repo=PLUGIN_REPO, + ) + forwarded = entry.CALLS[-1][2] + assert forwarded["name"] == "sub" + assert forwarded["local_dir"] == "/tmp/ws" + assert forwarded["dest"] == "/tmp/ws" + assert forwarded["dry_run"] is True + assert forwarded["force"] is True + assert forwarded["endpoint"] == "https://pre.modelscope.cn" + assert forwarded["token"] == "tok" + + +@pytest.mark.parametrize("repo", ["", " ", "no-slash", "/noname", "owner/"]) +def test_install_agent_validates_the_agent_repo(wired, monkeypatch, repo): + """``/noname`` and ``owner/`` contain a slash but name no repository; they + must be rejected before any network call.""" + + def no_network(*args, **kwargs): + raise AssertionError(f"network reached for malformed repo id {repo!r}") + + monkeypatch.setattr(_plugin, "fetch_plugin", no_network) + import modelscope_hub.compat as compat + + monkeypatch.setattr(compat, "snapshot_download", no_network) + + with pytest.raises(InvalidParameter): + _plugin.install_agent(repo, plugin_repo=PLUGIN_REPO) + + +def test_install_agent_reports_plugin_failure(wired, monkeypatch): + source = textwrap.dedent( + """ + from dataclasses import dataclass + + + @dataclass(frozen=True) + class Result: + ok: bool = False + error: str = "framework not installed" + exit_code: int = 2 + + + def capabilities(): + return {"operations": ("install",)} + + + def install(repo, **kwargs): + return Result() + """ + ).lstrip() + directory = make_plugin(wired.parent, dirname="fail_plugin", entry_module="fail_plugin", entry_source=source) + monkeypatch.setattr(_plugin, "fetch_plugin", lambda repo_id, **kwargs: directory) + + outcome = _plugin.install_agent("owner/my-agent", plugin_repo=PLUGIN_REPO) + assert not outcome.ok + assert outcome.error == "framework not installed" + # The install layer's own codes (3/4/5/6) carry meaning and must survive. + assert outcome.exit_code == 2 + sys.modules.pop("fail_plugin", None) + + +def test_install_agent_contains_a_plugin_exception(wired, monkeypatch): + source = textwrap.dedent( + """ + def capabilities(): + return {"operations": ("install",)} + + + def install(repo, **kwargs): + raise RuntimeError("boom") + """ + ).lstrip() + directory = make_plugin(wired.parent, dirname="boom_plugin", entry_module="boom_plugin", entry_source=source) + monkeypatch.setattr(_plugin, "fetch_plugin", lambda repo_id, **kwargs: directory) + + outcome = _plugin.install_agent("owner/my-agent", plugin_repo=PLUGIN_REPO) + assert not outcome.ok + assert "boom" in outcome.error + assert outcome.exit_code == 1 + sys.modules.pop("boom_plugin", None) + + +def test_an_unimportable_plugin_says_it_was_downloaded_but_not_run(wired, monkeypatch): + """The download and the manifest check both passed, so a bare "failed to load" + reads like a network problem and hides the two facts that matter: the package + is fine, and no agent work happened.""" + source = "import definitely_not_installed_xyz\n" + directory = make_plugin(wired.parent, dirname="bad_import", entry_module="bad_import", entry_source=source) + monkeypatch.setattr(_plugin, "fetch_plugin", lambda repo_id, **kwargs: directory) + + outcome = _plugin.install_agent("owner/my-agent", plugin_repo=PLUGIN_REPO) + assert not outcome.ok + assert outcome.exit_code == 1 + assert "downloaded and verified" in outcome.error + assert "NOT executed" in outcome.error + assert "no agent was fetched or installed" in outcome.error + + +def test_a_plugin_with_no_usable_operation_says_it_was_not_run(wired, monkeypatch): + source = textwrap.dedent( + """ + def capabilities(): + return {"operations": ()} + + def install(repo, **kwargs): + raise AssertionError("must not be chosen") + """ + ).lstrip() + directory = make_plugin(wired.parent, dirname="no_op", entry_module="no_op", entry_source=source) + monkeypatch.setattr(_plugin, "fetch_plugin", lambda repo_id, **kwargs: directory) + + with pytest.raises(NotSupportedError) as excinfo: + _plugin.install_agent("owner/my-agent", plugin_repo=PLUGIN_REPO) + message = str(excinfo.value) + assert "downloaded and verified" in message + assert "NOT executed" in message + assert "no agent was fetched or installed" in message + sys.modules.pop("no_op", None) + + +@pytest.mark.parametrize("returned", ["None", "'done'", "{}"]) +def test_install_agent_refuses_a_result_without_ok(wired, monkeypatch, returned): + """``ok`` is the only signal that decides whether the user is told the agent + was installed, so a result without it must fail closed. Defaulting it to True + meant a plugin returning None, a bare string or an empty dict reported + "Installed" for an install nobody could verify.""" + source = textwrap.dedent( + f""" + def capabilities(): + return {{"operations": ("install",)}} + + def install(repo, **kwargs): + return {returned} + """ + ).lstrip() + directory = make_plugin(wired.parent, dirname="no_ok_plugin", entry_module="no_ok_plugin", entry_source=source) + monkeypatch.setattr(_plugin, "fetch_plugin", lambda repo_id, **kwargs: directory) + + outcome = _plugin.install_agent("owner/my-agent", plugin_repo=PLUGIN_REPO) + assert not outcome.ok + assert outcome.exit_code == 1 + assert "no 'ok' attribute" in outcome.error + sys.modules.pop("no_ok_plugin", None) + + +# --------------------------------------------------------------------------- +# fetch-only plugins +# --------------------------------------------------------------------------- +def test_default_staging_dir_computes_without_creating(tmp_path, monkeypatch): + monkeypatch.setenv(constants.ENV_CACHE, str(tmp_path / "cache")) + path = _plugin.default_staging_dir("owner/my-agent") + + assert path.parent == tmp_path / "cache" / "agent" / "agent-staging" + # The repository id contains a slash, so it has to be flattened, and the + # stamp keeps repeats of one repository apart. Resolution is one second, so + # this separates runs, not concurrent calls. + assert re.fullmatch(r"owner--my-agent-\d{8}_\d{6}", path.name) + assert not path.exists(), "computing a destination must not create it" + + +#: Mirrors the real 0.2.0 plugin: ``dest`` is keyword-only and required, and +#: there is no ``local_dir`` at all, so a hub that does not resolve a +#: destination cannot call it. +FETCH_ONLY_SOURCE = textwrap.dedent( + """ + from dataclasses import dataclass, field + + + @dataclass(frozen=True) + class Result: + ok: bool = True + error: str | None = None + files_written: tuple = field(default_factory=tuple) + root: str = "" + exit_code: int = 0 + + + CALLS = [] + + + def capabilities(): + return {"operations": ("fetch_raw", "restore", "list_backups")} + + + def install(**kwargs): + raise AssertionError("placeholder must not be chosen") + + + def download(repo, **kwargs): + raise AssertionError("legacy alias must not be chosen") + + + def fetch_raw(repo, *, dest, name=None, framework=None, dry_run=False, + quiet=False, endpoint=None, token=None): + CALLS.append({"repo": repo, "dest": dest, "framework": framework}) + return Result(files_written=("SOUL.md", "AGENTS.md"), root=dest) + """ +).lstrip() + + +@pytest.fixture +def fetch_only(wired, monkeypatch): + directory = make_plugin( + wired.parent, + dirname="fetch_plugin", + entry_module="fetch_plugin", + entry_source=FETCH_ONLY_SOURCE, + ) + monkeypatch.setattr(_plugin, "fetch_plugin", lambda repo_id, **kwargs: directory) + yield directory + sys.modules.pop(_plugin._module_alias(spec_for(directory)), None) + + +def test_install_agent_drives_a_fetch_only_plugin(fetch_only): + """The regression test for the joint-testing failure: a transport-only plugin + used to be rejected outright, and then called without its required ``dest``.""" + outcome = _plugin.install_agent( + "owner/my-agent", + framework="qwenpaw", + plugin_repo=PLUGIN_REPO, + ) + assert outcome.ok, outcome.error + assert outcome.operation == "fetch_raw" + + call = loaded(fetch_only, "fetch_plugin").CALLS[-1] + assert call["repo"] == "owner/my-agent" + assert call["framework"] == "qwenpaw" + assert Path(call["dest"]).parent.name == "agent-staging" + # The destination the plugin reports is the one the hub resolved. + assert str(outcome.result.root) == call["dest"] + + +def test_install_agent_maps_local_dir_onto_dest(fetch_only): + outcome = _plugin.install_agent( + "owner/my-agent", + local_dir="/tmp/joint/staging", + plugin_repo=PLUGIN_REPO, + ) + assert outcome.ok, outcome.error + + assert loaded(fetch_only, "fetch_plugin").CALLS[-1]["dest"] == "/tmp/joint/staging" + + +def test_dest_is_not_forwarded_to_an_operation_that_does_not_accept_it(wired, monkeypatch): + """Narrowing keeps the legacy path working: an operation with no ``dest`` + parameter must not be handed one, or every 0.1.x plugin would break.""" + source = textwrap.dedent( + """ + def capabilities(): + return {"operations": ("download",)} + + CALLS = [] + + def download(repo, *, local_dir=None, dry_run=False): + CALLS.append({"repo": repo, "local_dir": local_dir}) + return type("R", (), {"ok": True})() + """ + ).lstrip() + directory = make_plugin(wired.parent, dirname="legacy_plugin", entry_module="legacy_plugin", entry_source=source) + monkeypatch.setattr(_plugin, "fetch_plugin", lambda repo_id, **kwargs: directory) + + outcome = _plugin.install_agent( + "owner/my-agent", + local_dir="/tmp/ws", + plugin_repo=PLUGIN_REPO, + ) + assert outcome.ok, outcome.error + assert outcome.operation == "download" + + assert loaded(directory, "legacy_plugin").CALLS[-1] == { + "repo": "owner/my-agent", + "local_dir": "/tmp/ws", + }