From 1a4cf741d2a6c6cef7ee1f6d8be9109db17ec861 Mon Sep 17 00:00:00 2001 From: albin-george-kurian Date: Thu, 10 Sep 2026 13:49:42 +0530 Subject: [PATCH 1/2] feat: add restricted execution context (#139) --- Architecture.md | 8 +- CHANGELOG.md | 45 +++ SECURITY.md | 113 +++++- docs/architecture/runtime-adapters.md | 16 +- docs/project/security.md | 109 ++++++ docs/user-guide/configuration.md | 30 +- .../adapters/registry/catalog_registry.py | 14 +- src/modeldock/adapters/runtimes/base.py | 13 + src/modeldock/adapters/runtimes/gpt4all.py | 3 + src/modeldock/adapters/runtimes/registry.py | 76 +++- src/modeldock/adapters/runtimes/vllm.py | 3 + src/modeldock/cli/commands/config.py | 1 + src/modeldock/cli/console.py | 12 + src/modeldock/cli/factory.py | 12 +- src/modeldock/common/config.py | 18 + src/modeldock/common/errors.py | 17 + src/modeldock/core/execution.py | 106 ++++++ src/modeldock/core/lifecycle.py | 10 + src/modeldock/core/manager.py | 24 +- src/modeldock/ports/runtime.py | 6 + tests/unit/test_execution_policy.py | 350 ++++++++++++++++++ tests/unit/test_security.py | 30 +- 22 files changed, 987 insertions(+), 29 deletions(-) create mode 100644 src/modeldock/core/execution.py create mode 100644 tests/unit/test_execution_policy.py diff --git a/Architecture.md b/Architecture.md index bb47d7f..8e00e2e 100644 --- a/Architecture.md +++ b/Architecture.md @@ -361,11 +361,17 @@ modeldock --help - **Format:** TOML for the file (human-friendly, stdlib `tomllib` in 3.11+). - **Model:** a frozen `Settings` dataclass/pydantic model: `default_backend`, `cache_dir`, `registry_url`, `catalog_source`, `log_level`, `progress_style`, - `auto_install`, `ollama_host`, etc. + `auto_install`, `execution_policy`, `ollama_host`, etc. - **Cross-platform paths:** resolved via `common/platform.py` using `platformdirs` (the de-facto standard for user/config/cache dirs across OSes). - **Validation:** config loaded through a validator; unknown keys warn, invalid values fall back to defaults with a logged warning (never crash on bad config). +- **Execution policy:** `execution_policy` (`unrestricted` | `warn` | `strict`) + is the one restricted-execution knob. It is applied in `core/execution.py` + (`ExecutionGuard`), which both `LifecycleOrchestrator.load` and + `ModelManager.run` consult, and which gates entry-point plugin discovery in + `RuntimeRegistry`/`CatalogProviderRegistry`. Adapters never implement it. + See SECURITY.md, "Model Execution & Native Code". --- diff --git a/CHANGELOG.md b/CHANGELOG.md index 7889531..34592bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,51 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/), and this ## [Unreleased] +### Added + +- `execution_policy` setting (`unrestricted` | `warn` | `strict`, default + `warn`) — the restricted-execution context for issue #139. Configurable via + `config.toml`, `MODELDOCK_EXECUTION_POLICY`, and `modeldock config show`. +- `ExecutionGuard` (`core/execution.py`) — the single place the policy is + applied, consulted by both `LifecycleOrchestrator.load` and + `ModelManager.run` so the two execution entry points cannot diverge. +- A one-per-session warning, before a model is executed, that the runtime runs + it as native code with the user's full privileges and that ModelDock does not + sandbox it. Delivered to stderr by the CLI via a new + `cli.console.print_warning`; the SDK stays silent unless a caller passes + `ModelManager(notify=...)`. +- `execution_policy="strict"` refuses third-party entry-point plugins + (`modeldock.runtimes`, `modeldock.model_sources`, + `modeldock.catalog_providers`) — they are not imported or instantiated at + all — and refuses backends that load model weights into ModelDock's own + process, raising the new typed `ExecutionPolicyError`. +- `BaseRuntime.executes_in_process`, declaring whether an adapter loads weights + into ModelDock's interpreter rather than driving a separate server. Shipped + HTTP-backed adapters are `False`; `gpt4all` and `vllm` declare `True`. +- SECURITY.md and `docs/project/security.md` — a "Model Execution & Native + Code" section: threat model for model artifacts and plugins, what + `execution_policy` does and does not enforce, and a concrete container recipe + for confining the runtime itself. + +### Changed + +- `RuntimeRegistry` and `CatalogProviderRegistry` take `allow_plugins`, and log + plugin provenance: a plugin that shadows a built-in adapter is reported at + WARNING, since nothing else revealed that the shipped adapter was replaced. +- `RuntimeRegistry` imports `entry_points` at module scope, matching + `CatalogProviderRegistry` and making the discovery call site visible. +- `RuntimeRegistry.detect_available` logs why a backend failed to probe instead + of discarding the exception silently. + +### Fixed + +- `tests/unit/test_security.py` resolved its source root to a directory that + does not exist, so the no-shell-execution audit walked **zero** files and + passed vacuously. It now walks `src/modeldock/{adapters,common}`, and a new + test asserts the file list is non-empty so it cannot silently degrade again. +- `test_model_names_are_treated_as_data` asserted a string literal against + itself; it now routes the hostile name through `ModelRef.parse`. + ## [0.2.0] - 2026-08-28 Live GGUF catalogs, composite registry, third-party catalog plugins, and diff --git a/SECURITY.md b/SECURITY.md index c6386d7..0a3c150 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -44,7 +44,9 @@ Send an email to **opensource@openagenthq.com** with: - Keep ModelDock and its dependencies up to date (`pip install -U modeldock`). - Use environment variables for any secrets; never hardcode them. -- Download models only from trusted runtimes/registries. +- Download models only from trusted runtimes/registries. A model file is + executed as native code — see [Model Execution & Native + Code](#model-execution--native-code). - Review the dynamic catalog source (`ollama.com`) and any bundled registry sources. ### For Contributors @@ -53,6 +55,8 @@ Send an email to **opensource@openagenthq.com** with: - Never commit secrets or `.env` files (see `.gitignore`). - Raise typed `ModelDockError` subclasses — never swallow errors silently. - Run `bandit -r src` as part of local checks. +- Never bypass the execution policy in `core/execution.py` from an adapter + or the CLI; see [Model Execution & Native Code](#model-execution--native-code). - Treat output from adapters as untrusted; see [Prompt-Injection & Untrusted Model Output](#prompt-injection--untrusted-model-output) below. ## Prompt-Injection & Untrusted Model Output @@ -106,6 +110,113 @@ If you embed ModelDock in a larger agent, copilot, or automation pipeline: make network requests without an explicit human-in-the-loop approval step. - Assume every string originating from an adapter response could be adversarial. +## Model Execution & Native Code + +### Threat Model + +ModelDock does not perform inference itself. `load()` and `run()` hand a model +to a runtime — Ollama, LM Studio, llama.cpp and others — which loads the +weights and executes them as **native machine code**, in a process owned by +the user who invoked ModelDock, with that user's full privileges. + +A model artifact is therefore not inert data: + +- **Weight files are parsed by native C/C++ loaders.** A malformed GGUF header + is a memory-safety bug in that loader, not a Python exception you can catch. +- **Some formats carry executable content by construction.** Pickle-based + `.bin` checkpoints deserialize arbitrary Python objects; repositories that + ship custom operators or conversion scripts execute code by design. +- **Model metadata drives the runtime.** Chat templates and tokenizer + configuration embedded in a model file are interpreted by the runtime, not + validated by ModelDock. +- **A runtime is a separate program.** Once ModelDock has asked it to load a + model, ModelDock has no further control over what that process reads, + writes, or connects to. +- **Installed plugins run inside ModelDock itself.** Any distribution that + advertises a `modeldock.runtimes`, `modeldock.model_sources`, or + `modeldock.catalog_providers` entry point is imported *and instantiated* in + ModelDock's own process the moment a registry is built. Installing such a + package is equivalent to granting it arbitrary code execution. + +**ModelDock cannot sandbox any of this.** Python cannot confine a native +library already mapped into its address space, and it cannot restrain a server +process it does not supervise. Real containment comes from the operating +system. What ModelDock *can* do is decline to take part, and tell you when it +is about to — which is what the setting below controls. + +### The `execution_policy` Setting + +Set it in `config.toml`, or as `MODELDOCK_EXECUTION_POLICY`: + +| Value | Warns about native execution | Third-party plugins | Backends that load models in-process | +|-------|------------------------------|---------------------|--------------------------------------| +| `unrestricted` | No | Loaded | Allowed | +| `warn` (default) | Once per session | Loaded | Allowed | +| `strict` | Once per session | **Not imported or executed** | **Refused** | + +Be clear about what `strict` does and does not buy you. It is not a sandbox. +It restricts what *ModelDock's own process* will execute: no third-party +plugin code, and no backend that maps model weights into that process. A +runtime server such as Ollama or llama-server still runs your model with your +full privileges — `strict` does not change that, and cannot. Confine the +runtime with the operating system, as below. + +### Rules + +1. **Treat a model file as a program, not a document.** Apply the same + scrutiny to its origin that you would to an executable you downloaded. +2. **Prefer runtimes that execute out-of-process.** A separate server process + can be confined by the OS; a native library inside your own interpreter + cannot. +3. **Never install a ModelDock plugin you would not accept as arbitrary code.** + Entry-point discovery grants it exactly that. Use `execution_policy = + "strict"` when running untrusted or unaudited environments. +4. **Do not rely on ModelDock for isolation.** It reports and refuses; it does + not contain. + +### Running a Model in a Restricted Context + +Confine the *runtime*, not ModelDock. A reasonable baseline, using Ollama as +the example — the same shape applies to `llama-server` and LM Studio: + +```bash +docker run --rm \ + --user "$(id -u):$(id -g)" \ + --read-only --tmpfs /tmp \ + --cap-drop ALL --security-opt no-new-privileges \ + -v "$PWD/models:/models:ro" \ + -p 127.0.0.1:11434:11434 \ + ollama/ollama +``` + +What each part is for: + +- `--user` — never run the runtime as root. +- `--read-only` plus a read-only model mount — the model directory is the only + filesystem the runtime needs, and it does not need to write to it. +- `--cap-drop ALL`, `--security-opt no-new-privileges` — inference needs no + capabilities. +- `-p 127.0.0.1:...` — bind the API to loopback so it is not exposed to the + network. Add `--network none` once the model is downloaded if the runtime + does not need to fetch anything at inference time. + +Then point ModelDock at it (`ollama_host`, `lmstudio_host`, or +`MODELDOCK_OLLAMA_HOST`) and set `execution_policy = "strict"` so ModelDock +itself executes nothing beyond its own shipped code. + +If you launch `llama-server` directly, note that ModelDock only ever *suggests* +that command in an error hint — it never runs it for you. Apply the same +confinement to the command you actually run. + +### Guidance for Contributors + +- A runtime adapter must not spawn a model process without documenting it. + Today every shipped adapter is an HTTP client to a server the user started. +- Declare `executes_in_process = True` on any adapter that loads weights into + ModelDock's interpreter, so `strict` can refuse it. +- The execution policy is decided once, in `core/execution.py`. Do not + re-implement or bypass it in an adapter or in the CLI. + ## Contact For security-related questions or concerns, contact: diff --git a/docs/architecture/runtime-adapters.md b/docs/architecture/runtime-adapters.md index aff80f0..a0b345e 100644 --- a/docs/architecture/runtime-adapters.md +++ b/docs/architecture/runtime-adapters.md @@ -119,7 +119,21 @@ extending a runtime adapter, keep the following in mind: mislead downstream tooling in agent/copilot pipelines. Treat it as adversarial user input. -See [SECURITY.md](https://github.com/OpenAgentHQ/modeldock/blob/main/SECURITY.md) for the full prompt-injection guidance. +### Native Code & the Execution Boundary + +`get_model_client()` and `run()` are a second, different boundary: past them a +runtime loads model weights and executes them as native code with the invoking +user's full privileges. ModelDock cannot sandbox that. When writing an adapter: + +- **Document how the model process is started and confined.** Every shipped + adapter is an HTTP client to a server the user launched; an adapter that + spawns a process itself must say so. +- **Declare `executes_in_process = True`** if the adapter loads weights into + ModelDock's own interpreter, so `execution_policy="strict"` can refuse it. +- **Do not implement the policy yourself.** It is decided once, in + `core/execution.py`, so `load` and `run` cannot diverge. + +See [SECURITY.md](https://github.com/OpenAgentHQ/modeldock/blob/main/SECURITY.md) for the full prompt-injection and model-execution guidance. --- ## Next Steps diff --git a/docs/project/security.md b/docs/project/security.md index d00aa14..7635df4 100644 --- a/docs/project/security.md +++ b/docs/project/security.md @@ -106,6 +106,115 @@ If you embed ModelDock in a larger agent, copilot, or automation pipeline: --- +## Model Execution & Native Code + +### Threat Model + +ModelDock does not perform inference itself. `load()` and `run()` hand a model +to a runtime — Ollama, LM Studio, llama.cpp and others — which loads the +weights and executes them as **native machine code**, in a process owned by +the user who invoked ModelDock, with that user's full privileges. + +A model artifact is therefore not inert data: + +- **Weight files are parsed by native C/C++ loaders.** A malformed GGUF header + is a memory-safety bug in that loader, not a Python exception you can catch. +- **Some formats carry executable content by construction.** Pickle-based + `.bin` checkpoints deserialize arbitrary Python objects; repositories that + ship custom operators or conversion scripts execute code by design. +- **Model metadata drives the runtime.** Chat templates and tokenizer + configuration embedded in a model file are interpreted by the runtime, not + validated by ModelDock. +- **A runtime is a separate program.** Once ModelDock has asked it to load a + model, ModelDock has no further control over what that process reads, + writes, or connects to. +- **Installed plugins run inside ModelDock itself.** Any distribution that + advertises a `modeldock.runtimes`, `modeldock.model_sources`, or + `modeldock.catalog_providers` entry point is imported *and instantiated* in + ModelDock's own process the moment a registry is built. Installing such a + package is equivalent to granting it arbitrary code execution. + +**ModelDock cannot sandbox any of this.** Python cannot confine a native +library already mapped into its address space, and it cannot restrain a server +process it does not supervise. Real containment comes from the operating +system. What ModelDock *can* do is decline to take part, and tell you when it +is about to — which is what the setting below controls. + +### The `execution_policy` Setting + +Set it in `config.toml`, or as `MODELDOCK_EXECUTION_POLICY`: + +| Value | Warns about native execution | Third-party plugins | Backends that load models in-process | +|-------|------------------------------|---------------------|--------------------------------------| +| `unrestricted` | No | Loaded | Allowed | +| `warn` (default) | Once per session | Loaded | Allowed | +| `strict` | Once per session | **Not imported or executed** | **Refused** | + +Be clear about what `strict` does and does not buy you. It is not a sandbox. +It restricts what *ModelDock's own process* will execute: no third-party +plugin code, and no backend that maps model weights into that process. A +runtime server such as Ollama or llama-server still runs your model with your +full privileges — `strict` does not change that, and cannot. Confine the +runtime with the operating system, as below. + +### Rules + +1. **Treat a model file as a program, not a document.** Apply the same + scrutiny to its origin that you would to an executable you downloaded. +2. **Prefer runtimes that execute out-of-process.** A separate server process + can be confined by the OS; a native library inside your own interpreter + cannot. +3. **Never install a ModelDock plugin you would not accept as arbitrary code.** + Entry-point discovery grants it exactly that. Use `execution_policy = + "strict"` when running untrusted or unaudited environments. +4. **Do not rely on ModelDock for isolation.** It reports and refuses; it does + not contain. + +### Running a Model in a Restricted Context + +Confine the *runtime*, not ModelDock. A reasonable baseline, using Ollama as +the example — the same shape applies to `llama-server` and LM Studio: + +```bash +docker run --rm \ + --user "$(id -u):$(id -g)" \ + --read-only --tmpfs /tmp \ + --cap-drop ALL --security-opt no-new-privileges \ + -v "$PWD/models:/models:ro" \ + -p 127.0.0.1:11434:11434 \ + ollama/ollama +``` + +What each part is for: + +- `--user` — never run the runtime as root. +- `--read-only` plus a read-only model mount — the model directory is the only + filesystem the runtime needs, and it does not need to write to it. +- `--cap-drop ALL`, `--security-opt no-new-privileges` — inference needs no + capabilities. +- `-p 127.0.0.1:...` — bind the API to loopback so it is not exposed to the + network. Add `--network none` once the model is downloaded if the runtime + does not need to fetch anything at inference time. + +Then point ModelDock at it (`ollama_host`, `lmstudio_host`, or +`MODELDOCK_OLLAMA_HOST`) and set `execution_policy = "strict"` so ModelDock +itself executes nothing beyond its own shipped code. + +If you launch `llama-server` directly, note that ModelDock only ever *suggests* +that command in an error hint — it never runs it for you. Apply the same +confinement to the command you actually run. + +### Guidance for Contributors + +- A runtime adapter must not spawn a model process without documenting it. + Today every shipped adapter is an HTTP client to a server the user started. +- Declare `executes_in_process = True` on any adapter that loads weights into + ModelDock's interpreter, so `strict` can refuse it. +- The execution policy is decided once, in `core/execution.py`. Do not + re-implement or bypass it in an adapter or in the CLI. + +--- + ## Contact - **Email**: opensource@openagenthq.com diff --git a/docs/user-guide/configuration.md b/docs/user-guide/configuration.md index 65becc6..aa364c1 100644 --- a/docs/user-guide/configuration.md +++ b/docs/user-guide/configuration.md @@ -16,10 +16,11 @@ ModelDock is zero-config by default. Customize when needed. ## Config File Format ```toml -default_backend = "ollama" -auto_install = true -log_level = "INFO" -progress_style = "rich" +default_backend = "ollama" +auto_install = true +log_level = "INFO" +progress_style = "rich" +execution_policy = "warn" ``` --- @@ -35,6 +36,7 @@ Override config with `MODELDOCK_*` env vars: | `MODELDOCK_AUTO_INSTALL` | Auto-download missing models | `false` | | `MODELDOCK_CACHE_DIR` | Override cache location | platform default | | `MODELDOCK_CATALOG_SOURCE` | `auto`/`ollama`/`bundled` | `auto` | +| `MODELDOCK_EXECUTION_POLICY` | `unrestricted`/`warn`/`strict` | `warn` | --- @@ -101,6 +103,26 @@ Set via config file or `MODELDOCK_CATALOG_SOURCE` env var. --- +## Restricted Execution + +Loading a model makes a runtime execute it as native code with your user +account's full privileges. `execution_policy` controls how much ModelDock is +willing to do on your behalf: + +| Value | Behavior | +|-------|----------| +| `warn` | Warn once per session before a model is executed (default) | +| `unrestricted` | No warning; previous behavior | +| `strict` | Also refuse third-party plugins and in-process model loading | + +`strict` is not a sandbox — it restricts what ModelDock's own process executes, +not what a runtime server does with your model. Set via config file or +`MODELDOCK_EXECUTION_POLICY`. See +[SECURITY.md](https://github.com/OpenAgentHQ/modeldock/blob/main/SECURITY.md) +for how to confine the runtime itself. + +--- + ## Next Steps - [SDK Reference](../sdk/python-api.md) — full API reference diff --git a/src/modeldock/adapters/registry/catalog_registry.py b/src/modeldock/adapters/registry/catalog_registry.py index 34136c4..975cc77 100644 --- a/src/modeldock/adapters/registry/catalog_registry.py +++ b/src/modeldock/adapters/registry/catalog_registry.py @@ -49,13 +49,23 @@ class CatalogProviderRegistry: only ``cache_dir``. Entry points take priority over built-ins, so a plugin can also replace the shipped Hugging Face provider for LM Studio/llama.cpp if it wants to. + + Security Note + ------------- + ``ep.load()`` imports third-party code into ModelDock's own process at + construction time, so installing a catalog-provider plugin grants it + arbitrary code execution. Pass ``allow_plugins=False`` (what + ``execution_policy="strict"`` does) to skip discovery entirely. """ - def __init__(self) -> None: + def __init__(self, allow_plugins: bool = True) -> None: self._logger = get_logger("registry.catalog_provider_registry") _register_builtins() self._entry_points: Dict[RuntimeBackend, Callable[[Path], RegistryPort]] = {} - self._discover_entry_points() + if allow_plugins: + self._discover_entry_points() + else: + self._logger.debug("Catalog provider plugin discovery disabled by execution policy") def _discover_entry_points(self) -> None: try: diff --git a/src/modeldock/adapters/runtimes/base.py b/src/modeldock/adapters/runtimes/base.py index 30999fa..436ef92 100644 --- a/src/modeldock/adapters/runtimes/base.py +++ b/src/modeldock/adapters/runtimes/base.py @@ -63,10 +63,23 @@ class BaseRuntime: objects. Never pass raw runtime output to ``exec()``, ``eval()``, ``subprocess``, or any code-execution primitive. See SECURITY.md for the full prompt-injection guidance. + + ``get_model_client`` and ``run`` are the *execution* boundary: past them a + runtime loads model weights and executes them as native code with the + invoking user's full privileges. Adapters declare which side of that + boundary they sit on via ``executes_in_process``; the policy decision + itself lives in ``core/execution.py``, never in an adapter. """ backend: RuntimeBackend = RuntimeBackend.OLLAMA + #: True when this adapter loads model weights into ModelDock's own Python + #: process (a native extension or ctypes binding) instead of talking to a + #: separate runtime server. Consulted by ``ExecutionGuard``: once native + #: code is mapped into this process, Python can no longer restrain it, so + #: ``execution_policy="strict"`` refuses these adapters outright. + executes_in_process: bool = False + def __init__(self) -> None: self._logger = get_logger(f"runtime.{self.backend.value}") self._availability: Optional[bool] = None diff --git a/src/modeldock/adapters/runtimes/gpt4all.py b/src/modeldock/adapters/runtimes/gpt4all.py index 19f1f8c..49d4855 100644 --- a/src/modeldock/adapters/runtimes/gpt4all.py +++ b/src/modeldock/adapters/runtimes/gpt4all.py @@ -19,6 +19,9 @@ class Gpt4AllRuntime(BaseRuntime): """Discover models already present in a GPT4All models directory.""" backend: RuntimeBackend = RuntimeBackend.GPT4ALL + # Loads weights into this process through a native extension rather than + # driving a separate server, so ``execution_policy="strict"`` refuses it. + executes_in_process: bool = True def __init__(self, models_dir: Path | None = None) -> None: super().__init__() diff --git a/src/modeldock/adapters/runtimes/registry.py b/src/modeldock/adapters/runtimes/registry.py index 2d14ea8..85b98ed 100644 --- a/src/modeldock/adapters/runtimes/registry.py +++ b/src/modeldock/adapters/runtimes/registry.py @@ -6,6 +6,7 @@ from __future__ import annotations +from importlib.metadata import entry_points from pathlib import Path from typing import Any, Callable, Dict, List, cast @@ -16,6 +17,26 @@ # Built-in registry: first-party adapters shipped in-repo. _BUILTIN: Dict[RuntimeBackend, Callable[[], RuntimePort]] = {} +#: Our own distribution name. ModelDock advertises its ``ollama`` adapter as a +#: ``modeldock.runtimes`` entry point, so discovery legitimately finds a +#: first-party entry point on every run. +_OWN_DISTRIBUTION = "modeldock" + + +def _distribution_name(ep: Any) -> str: + """Return the distribution that provides ``ep``, or "" when unknown. + + ``EntryPoint.dist`` is only populated for entry points obtained from + ``entry_points()``, and is absent on older interpreters, so this degrades + to "" rather than assuming provenance it cannot establish. + """ + return str(getattr(getattr(ep, "dist", None), "name", "") or "") + + +def _is_first_party(ep: Any) -> bool: + """Whether ``ep`` was advertised by ModelDock's own distribution.""" + return _distribution_name(ep).replace("_", "-").lower() == _OWN_DISTRIBUTION + def _register_builtins() -> None: from modeldock.adapters.runtimes.gpt4all import Gpt4AllRuntime @@ -34,18 +55,31 @@ def _register_builtins() -> None: class RuntimeRegistry: - """Resolves a RuntimeBackend to a runtime instance.""" - - def __init__(self) -> None: + """Resolves a RuntimeBackend to a runtime instance. + + Security Note + ------------- + Entry-point discovery imports and instantiates code from any installed + distribution advertising ``modeldock.runtimes``, inside ModelDock's own + process, at construction time. Installing such a package is equivalent to + granting it arbitrary code execution. Pass ``allow_plugins=False`` (what + ``execution_policy="strict"`` does) for a built-ins-only registry. + """ + + def __init__(self, allow_plugins: bool = True) -> None: self._logger = get_logger("runtime.registry") _register_builtins() self._entry_points: Dict[RuntimeBackend, Callable[[], RuntimePort]] = {} - self._discover_entry_points() + if allow_plugins: + self._discover_entry_points() + else: + # Discovery imports and instantiates third-party code inside this + # process. Skipping it entirely is the enforcement half of + # ``execution_policy="strict"`` — see core/execution.py. + self._logger.debug("Runtime plugin discovery disabled by execution policy") def _discover_entry_points(self) -> None: try: - from importlib.metadata import entry_points - eps = entry_points() if hasattr(eps, "select"): group: Any = eps.select(group="modeldock.runtimes") @@ -57,6 +91,33 @@ def _discover_entry_points(self) -> None: runtime_cls = ep.load() loaded = cast(RuntimePort, runtime_cls()) self._entry_points[backend] = self._make_factory(loaded) + # Loading a plugin runs third-party code in this process, + # and an entry point named after a built-in backend + # silently displaces the first-party adapter. Record the + # shadowing case loudly, because nothing else reveals that + # the shipped adapter is no longer the one in use. + # + # ModelDock registers its own ``ollama`` runtime this way + # (pyproject.toml), so provenance is checked before + # warning: a first-party entry point re-registering a + # first-party adapter is not a security event, and warning + # on every invocation would teach users to ignore the one + # that matters. + if _is_first_party(ep): + self._logger.debug("Registered first-party runtime entry point %r", ep.name) + elif backend in _BUILTIN: + self._logger.warning( + "Runtime plugin %r from %r replaces the built-in %s adapter", + ep.name, + _distribution_name(ep) or "an unknown distribution", + backend.value, + ) + else: + self._logger.info( + "Loaded third-party runtime plugin %r for backend %s", + ep.name, + backend.value, + ) except Exception as exc: # skip bad plugins self._logger.warning("Skipping runtime plugin %s: %s", ep.name, exc) except Exception as exc: @@ -112,7 +173,8 @@ def detect_available(self) -> List[RuntimeBackend]: runtime = self.get(backend) if runtime.is_available(): result.append(backend) - except Exception: # nosec B112 - skip backends that fail to probe + except Exception as exc: # nosec B112 - one bad adapter must not hide the rest + self._logger.warning("Backend %s failed to probe: %s", backend.value, exc) continue return result diff --git a/src/modeldock/adapters/runtimes/vllm.py b/src/modeldock/adapters/runtimes/vllm.py index aae1dc8..213e60c 100644 --- a/src/modeldock/adapters/runtimes/vllm.py +++ b/src/modeldock/adapters/runtimes/vllm.py @@ -14,6 +14,9 @@ class VllmRuntime(BaseRuntime): """Planned runtime adapter for vLLM.""" backend: RuntimeBackend = RuntimeBackend.VLLM + # Loads weights into this process through a native extension rather than + # driving a separate server, so ``execution_policy="strict"`` refuses it. + executes_in_process: bool = True def _check_available(self) -> bool: return False diff --git a/src/modeldock/cli/commands/config.py b/src/modeldock/cli/commands/config.py index 9cdbe4e..d81ed87 100644 --- a/src/modeldock/cli/commands/config.py +++ b/src/modeldock/cli/commands/config.py @@ -23,6 +23,7 @@ def config_show(debug: bool = typer.Option(False, "--debug", help="Show tracebac typer.echo(f"log_level: {settings.log_level}") typer.echo(f"progress_style: {settings.progress_style}") typer.echo(f"auto_install: {settings.auto_install}") + typer.echo(f"execution_policy: {settings.execution_policy}") typer.echo(f"ollama_host: {settings.ollama_host}") typer.echo(f"lmstudio_host: {settings.lmstudio_host}") typer.echo(f"llamacpp_gpu_layers: {settings.llamacpp_gpu_layers}") diff --git a/src/modeldock/cli/console.py b/src/modeldock/cli/console.py index 4830fb9..bd0ad2e 100644 --- a/src/modeldock/cli/console.py +++ b/src/modeldock/cli/console.py @@ -70,6 +70,18 @@ def print_error(exc: Exception, debug: bool = False, as_json: bool = False) -> N traceback.print_exc() +def print_warning(message: str) -> None: + """Print a security/advisory warning to stderr. + + Always stderr, never stdout: a warning must not land in the middle of + ``--json`` output or piped model tokens, which is exactly where the + native-code notice would otherwise appear. + """ + import sys + + sys.stderr.write(f"Warning: {message}\n") + + def render_models(models: List[Any]) -> None: """Render a list of ModelSpec as a rich table.""" from rich.console import Console diff --git a/src/modeldock/cli/factory.py b/src/modeldock/cli/factory.py index bd86994..27ffbd0 100644 --- a/src/modeldock/cli/factory.py +++ b/src/modeldock/cli/factory.py @@ -8,6 +8,7 @@ from typing import Optional +from modeldock.cli.console import print_warning from modeldock.common.errors import ConfigError from modeldock.core.manager import ModelManager from modeldock.domain.model import RuntimeBackend @@ -25,11 +26,16 @@ def resolve_backend(backend: Optional[str]) -> Optional[RuntimeBackend]: def manager_for(backend: Optional[str] = None) -> ModelManager: - """Build a ``ModelManager`` for the requested backend (config default if None).""" + """Build a ``ModelManager`` for the requested backend (config default if None). + + Supplies the CLI's warning channel: the execution policy itself is decided + in ``core``, but only an interactive front end should print to the user, so + the library stays silent unless a caller opts in like this. + """ resolved = resolve_backend(backend) if resolved is None: - return ModelManager() - return ModelManager(backend=resolved) + return ModelManager(notify=print_warning) + return ModelManager(backend=resolved, notify=print_warning) __all__ = ["manager_for", "resolve_backend"] diff --git a/src/modeldock/common/config.py b/src/modeldock/common/config.py index 6ca4121..af75cf4 100644 --- a/src/modeldock/common/config.py +++ b/src/modeldock/common/config.py @@ -43,6 +43,10 @@ class Settings(BaseModel): log_level: str = "ERROR" progress_style: str = "rich" auto_install: bool = False + # "unrestricted" | "warn" | "strict" — how much ModelDock is willing to + # execute on the user's behalf. See SECURITY.md, "Model Execution & + # Native Code". + execution_policy: str = "warn" ollama_host: Optional[str] = None lmstudio_host: Optional[str] = None llamacpp_gpu_layers: Optional[int] = None @@ -78,6 +82,16 @@ def _validate_catalog_source(cls, value: str) -> str: ) return value + @field_validator("execution_policy") + @classmethod + def _validate_execution_policy(cls, value: str) -> str: + allowed = {"unrestricted", "warn", "strict"} + if value not in allowed: + raise ConfigError( + f"Invalid execution_policy {value!r}; expected one of {sorted(allowed)}" + ) + return value + @field_validator("llamacpp_gpu_layers", mode="before") @classmethod def _validate_llamacpp_gpu_layers(cls, value: Any) -> Optional[int]: @@ -102,6 +116,7 @@ def to_env_overrides(self) -> Dict[str, str]: f"{_ENV_PREFIX}LOG_LEVEL": self.log_level, f"{_ENV_PREFIX}DEFAULT_BACKEND": self.default_backend.value, f"{_ENV_PREFIX}CATALOG_SOURCE": self.catalog_source, + f"{_ENV_PREFIX}EXECUTION_POLICY": self.execution_policy, f"{_ENV_PREFIX}AUTO_INSTALL": str(self.auto_install).lower(), f"{_ENV_PREFIX}CACHE_DIR": str(self.cache_dir), f"{_ENV_PREFIX}GPT4ALL_MODELS_DIR": "" @@ -172,6 +187,8 @@ def _apply_mapping(settings: Settings, data: Dict[str, Any], source: str = "conf _safe_set(settings, "registry_url", data["registry_url"] or None, source) if "catalog_source" in data and data["catalog_source"]: _safe_set(settings, "catalog_source", str(data["catalog_source"]), source) + if "execution_policy" in data and data["execution_policy"]: + _safe_set(settings, "execution_policy", str(data["execution_policy"]), source) if "log_level" in data and data["log_level"]: _safe_set(settings, "log_level", _coerce_log_level(data["log_level"]), source) if "progress_style" in data and data["progress_style"]: @@ -231,6 +248,7 @@ def load_settings( f"{_ENV_PREFIX}CACHE_DIR": "cache_dir", f"{_ENV_PREFIX}REGISTRY_URL": "registry_url", f"{_ENV_PREFIX}CATALOG_SOURCE": "catalog_source", + f"{_ENV_PREFIX}EXECUTION_POLICY": "execution_policy", f"{_ENV_PREFIX}LOG_LEVEL": "log_level", f"{_ENV_PREFIX}PROGRESS_STYLE": "progress_style", f"{_ENV_PREFIX}OLLAMA_HOST": "ollama_host", diff --git a/src/modeldock/common/errors.py b/src/modeldock/common/errors.py index f67b03a..a4acbdc 100644 --- a/src/modeldock/common/errors.py +++ b/src/modeldock/common/errors.py @@ -84,6 +84,22 @@ def __init__(self, message: str) -> None: super().__init__(f"Configuration error: {message}") +class ExecutionPolicyError(ModelDockError): + """An operation was refused by the configured execution policy. + + Raised when ``execution_policy="strict"`` forbids something that would + execute code ModelDock cannot vouch for — a third-party plugin, or a + backend that loads a model into ModelDock's own process. The message names + what was refused and how to allow it deliberately. + """ + + def __init__(self, what: str, hint: str = "") -> None: + message = f"Refused by execution_policy='strict': {what}." + if hint: + message += f" {hint}" + super().__init__(message) + + class AliasResolutionError(ModelDockError): """A friendly model alias could not be resolved to a spec.""" @@ -99,5 +115,6 @@ def __init__(self, message: str) -> None: "DownloadError", "CacheError", "ConfigError", + "ExecutionPolicyError", "AliasResolutionError", ] diff --git a/src/modeldock/core/execution.py b/src/modeldock/core/execution.py new file mode 100644 index 0000000..af36382 --- /dev/null +++ b/src/modeldock/core/execution.py @@ -0,0 +1,106 @@ +"""ExecutionGuard — the one place ModelDock's execution policy is applied. + +Loading a model is not like loading a data file. Every runtime ModelDock +drives ultimately hands the weights to native machine code — llama.cpp's GGUF +loader, an Ollama server, a Python extension module — which then runs with the +invoking user's full privileges. ModelDock cannot sandbox that: a separate +runtime server is outside this process entirely, and a native extension loaded +into this process is past the point where Python can restrain it. + +What ModelDock *can* do honestly is refuse, and say so. This module holds that +decision so ``load`` and ``run`` cannot drift apart, and so no adapter has to +re-implement it. See SECURITY.md, "Model Execution & Native Code", and +Architecture.md §4. +""" + +from __future__ import annotations + +from typing import Any, Callable, Optional + +from modeldock.common.errors import ExecutionPolicyError +from modeldock.common.logging import get_logger +from modeldock.domain.model import ModelRef + +#: Policy values, in increasing order of restriction. Mirrors +#: ``Settings.execution_policy``; validated there, not here. +UNRESTRICTED = "unrestricted" +WARN = "warn" +STRICT = "strict" + + +class ExecutionGuard: + """Applies the configured execution policy at the execution boundary. + + Constructed once per ``ModelManager`` and consulted immediately before a + model is handed to a runtime. The guard deliberately does no I/O of its + own beyond logging: a user-visible channel is supplied by the caller + through ``notify``, so the library stays quiet by default and only the CLI + prints. + """ + + def __init__( + self, + policy: str = WARN, + notify: Optional[Callable[[str], None]] = None, + ) -> None: + self._policy = policy + self._notify = notify + self._logger = get_logger("core.execution") + self._warned = False + + @property + def policy(self) -> str: + """The configured policy value.""" + return self._policy + + def allows_plugins(self) -> bool: + """Whether third-party entry-point plugins may be imported and run. + + A plugin is arbitrary Python from any installed distribution, executed + inside ModelDock's own process the moment a registry is built. Under + ``strict`` it is not loaded at all — this is the part of a "restricted + execution context" ModelDock can genuinely enforce. + """ + return self._policy != STRICT + + def check(self, ref: ModelRef, runtime: Any) -> None: + """Apply the policy before ``runtime`` executes ``ref``. + + Raises ``ExecutionPolicyError`` when the policy forbids the execution; + otherwise emits the native-code notice once per guard and returns. + """ + if self._policy == STRICT and getattr(runtime, "executes_in_process", False): + backend = getattr(getattr(runtime, "backend", None), "value", "unknown") + raise ExecutionPolicyError( + f"the {backend!r} runtime loads model weights into ModelDock's own " + f"process as native code", + hint=( + "Use a runtime that runs the model in a separate server process, " + "or set execution_policy to 'warn' if you accept the risk." + ), + ) + self._warn_once(ref, runtime) + + def _warn_once(self, ref: ModelRef, runtime: Any) -> None: + """Emit the native-code notice at most once per guard. + + Once per guard rather than once per call: a session that loads several + models should say this clearly one time, not turn it into noise the + user learns to skip past. + """ + if self._policy == UNRESTRICTED or self._warned: + return + self._warned = True + backend = getattr(getattr(runtime, "backend", None), "value", "unknown") + message = ( + f"{ref.qualified_name()} will be executed by the {backend!r} runtime as " + f"native code, with your user account's full privileges. ModelDock does " + f"not sandbox it. Run only models you trust; see SECURITY.md " + f'("Model Execution & Native Code") for how to confine the runtime.' + ) + self._logger.warning("%s", message) + if self._notify is not None: + self._notify(message) + + +__all__ = ["ExecutionGuard", "UNRESTRICTED", "WARN", "STRICT"] diff --git a/src/modeldock/core/lifecycle.py b/src/modeldock/core/lifecycle.py index a866e56..78a3678 100644 --- a/src/modeldock/core/lifecycle.py +++ b/src/modeldock/core/lifecycle.py @@ -6,6 +6,7 @@ from modeldock.common.errors import ModelNotFoundError, ModelNotInstalledError from modeldock.common.logging import get_logger +from modeldock.core.execution import ExecutionGuard from modeldock.domain.model import ModelRef, ModelSpec from modeldock.ports.cache import CachePort from modeldock.ports.events import EventPort @@ -25,6 +26,7 @@ def __init__( progress: Optional[ProgressPort] = None, events: Optional[EventPort] = None, auto_install: bool = False, + guard: Optional[ExecutionGuard] = None, ) -> None: self._runtime = runtime self._registry = registry @@ -32,6 +34,10 @@ def __init__( self._progress = progress self._events = events self._auto_install = auto_install + # Default rather than optional: a caller who builds the orchestrator + # directly still gets the shipped policy instead of silently opting out + # of it. + self._guard = guard or ExecutionGuard() self._logger = get_logger("core.lifecycle") def load(self, name: str, auto_install: Optional[bool] = None) -> Any: @@ -56,6 +62,10 @@ def load(self, name: str, auto_install: Optional[bool] = None) -> Any: if ev is not None: ev.after_install(ref, None) + # The execution boundary: past this call the runtime loads and runs + # the model as native code. Checked here, and in ModelManager.run, so + # both entry points share one policy. + self._guard.check(ref, self._runtime) return self._runtime.get_model_client(ref) def _resolve(self, ref: ModelRef) -> ModelSpec: diff --git a/src/modeldock/core/manager.py b/src/modeldock/core/manager.py index c2f8368..070136c 100644 --- a/src/modeldock/core/manager.py +++ b/src/modeldock/core/manager.py @@ -9,7 +9,7 @@ from __future__ import annotations from pathlib import Path -from typing import Any, List, Optional, cast +from typing import Any, Callable, List, Optional, cast from modeldock.adapters.downloaders.factory import needs_http_download from modeldock.adapters.downloaders.http import HttpDownloader @@ -25,6 +25,7 @@ from modeldock.core.cache import CacheService from modeldock.core.config import ConfigService from modeldock.core.download import DownloadService +from modeldock.core.execution import ExecutionGuard from modeldock.core.lifecycle import LifecycleOrchestrator from modeldock.core.registry import RegistryService from modeldock.domain.model import ( @@ -56,6 +57,7 @@ def __init__( cache: Optional[CachePort] = None, events: Optional[EventPort] = None, settings: Optional[Settings] = None, + notify: Optional[Callable[[str], None]] = None, ) -> None: self._logger = get_logger("core.manager") # ``settings`` carries only the caller's deliberate overrides. Dumping it @@ -69,8 +71,13 @@ def __init__( cfg = self._config.settings self._backend = backend or cfg.default_backend + # Built before anything that could execute third-party code: both + # registries below discover entry points, and the guard decides whether + # that discovery may run at all. + self._guard = ExecutionGuard(cfg.execution_policy, notify=notify) + self._registry_port = registry or self._resolve_registry(cfg) - self._runtime_registry = RuntimeRegistry() + self._runtime_registry = RuntimeRegistry(allow_plugins=self._guard.allows_plugins()) self._runtime = runtime or self._resolve_runtime(self._backend, cfg) self._cache_port = cache or self._default_cache(cfg) @@ -87,6 +94,7 @@ def __init__( self._progress, events, auto_install=cfg.auto_install, + guard=self._guard, ) # --- resolution helpers ---------------------------------------------- @@ -153,7 +161,9 @@ def _resolve_backend_catalog(self, cfg: Settings) -> Optional[RegistryPort]: """ from modeldock.adapters.registry.catalog_registry import CatalogProviderRegistry - return CatalogProviderRegistry().get(self._backend, cfg.cache_dir) + return CatalogProviderRegistry(allow_plugins=self._guard.allows_plugins()).get( + self._backend, cfg.cache_dir + ) #: Config field holding the host override for each backend that has one. _HOST_SETTING_FOR = { @@ -452,8 +462,14 @@ def remove(self, name: str) -> None: self._runtime.remove(ref) def run(self, name: str, prompt: Optional[str] = None, **opts: Any) -> Any: - """Run an interactive session for a model in the active runtime.""" + """Run an interactive session for a model in the active runtime. + + ``run`` does not go through ``LifecycleOrchestrator``, so the execution + policy is applied here explicitly — otherwise ``run`` would be the one + path that executes a model without it. + """ ref = ModelRef.parse(name, backend=self._backend) + self._guard.check(ref, self._runtime) return self._runtime.run(ref, prompt=prompt, **opts) def verify(self, name: str) -> bool: diff --git a/src/modeldock/ports/runtime.py b/src/modeldock/ports/runtime.py index da6d9cb..d9ab151 100644 --- a/src/modeldock/ports/runtime.py +++ b/src/modeldock/ports/runtime.py @@ -31,6 +31,12 @@ class RuntimePort(Protocol): validate and sanitise all responses before constructing domain objects. Consumers must never execute, ``eval()``, or otherwise treat port output as trusted instructions. See SECURITY.md for full guidance. + + ``get_model_client`` and ``run`` additionally cross an *execution* + boundary: they cause a runtime to load a model artifact and execute it as + native code with the invoking user's full privileges. ModelDock cannot + sandbox that; it can only warn or refuse, which ``core.execution`` does on + every caller's behalf. See SECURITY.md, "Model Execution & Native Code". """ @property diff --git a/tests/unit/test_execution_policy.py b/tests/unit/test_execution_policy.py new file mode 100644 index 0000000..104ed53 --- /dev/null +++ b/tests/unit/test_execution_policy.py @@ -0,0 +1,350 @@ +"""Tests for the restricted execution context (issue #139). + +Two things are being defended here, and they are deliberately different in +kind. ModelDock can genuinely *refuse* to run third-party plugin code and to +drive a backend that loads native code into its own process — those are +assertions about behaviour. It cannot sandbox a runtime server it does not +own, so the rest is a warning, and these tests pin the warning down to exactly +once per session on the paths that actually execute a model. +""" + +from __future__ import annotations + +import logging +from pathlib import Path +from typing import Any, List +from unittest.mock import MagicMock, patch + +import pytest + +from modeldock.adapters.registry.catalog_registry import CatalogProviderRegistry +from modeldock.adapters.runtimes.registry import RuntimeRegistry +from modeldock.common.config import Settings, load_settings +from modeldock.common.errors import ConfigError, ExecutionPolicyError +from modeldock.core.execution import STRICT, UNRESTRICTED, WARN, ExecutionGuard +from modeldock.core.manager import ModelManager +from modeldock.domain.model import ModelRef, RuntimeBackend +from tests.conftest import FakeCache, FakeRegistry, FakeRuntime + + +class _InProcessRuntime(FakeRuntime): + """A runtime that loads model weights into ModelDock's own process.""" + + backend = RuntimeBackend.GPT4ALL + executes_in_process = True + + +def _fake_entry_point(name: str, target: Any, dist: str = "third-party-pkg") -> MagicMock: + ep = MagicMock() + ep.name = name + ep.load.return_value = target + # Provenance decides whether a registration is a security event; the + # default here is a foreign distribution, since that is the case worth + # testing. + ep.dist.name = dist + return ep + + +def _patched_entry_points(entries: List[MagicMock]) -> MagicMock: + """Mock importlib.metadata.entry_points() supporting .select().""" + eps = MagicMock() + eps.select.return_value = entries + return eps + + +# --------------------------------------------------------------------------- +# Settings +# --------------------------------------------------------------------------- + + +def test_default_policy_warns() -> None: + """The shipped default must satisfy "warn about native model code".""" + assert Settings().execution_policy == WARN + + +@pytest.mark.parametrize("value", [UNRESTRICTED, WARN, STRICT]) +def test_every_policy_value_is_accepted(value: str) -> None: + assert Settings(execution_policy=value).execution_policy == value + + +def test_invalid_policy_raises_config_error() -> None: + with pytest.raises(ConfigError): + Settings(execution_policy="sandboxed") + + +def test_policy_is_read_from_the_environment(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("MODELDOCK_EXECUTION_POLICY", STRICT) + assert load_settings().execution_policy == STRICT + + +def test_invalid_env_policy_falls_back_instead_of_crashing( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Per the config contract, a bad env value warns and keeps the default.""" + monkeypatch.setenv("MODELDOCK_EXECUTION_POLICY", "nonsense") + assert load_settings().execution_policy == WARN + + +def test_policy_is_read_from_a_config_file(tmp_path: Path) -> None: + cfg = tmp_path / "config.toml" + cfg.write_text('execution_policy = "unrestricted"\n', encoding="utf-8") + assert load_settings(config_path=cfg).execution_policy == UNRESTRICTED + + +def test_policy_is_exported_to_subprocesses() -> None: + env = Settings(execution_policy=STRICT).to_env_overrides() + assert env["MODELDOCK_EXECUTION_POLICY"] == STRICT + + +# --------------------------------------------------------------------------- +# ExecutionGuard +# --------------------------------------------------------------------------- + + +def test_warn_policy_notifies_once_per_session() -> None: + """A session loading several models should say this once, not become noise.""" + seen: List[str] = [] + guard = ExecutionGuard(WARN, notify=seen.append) + + guard.check(ModelRef.parse("llama3"), FakeRuntime()) + guard.check(ModelRef.parse("mistral"), FakeRuntime()) + + assert len(seen) == 1 + + +def test_the_warning_names_the_mechanism_and_the_backend() -> None: + seen: List[str] = [] + ExecutionGuard(WARN, notify=seen.append).check(ModelRef.parse("llama3"), FakeRuntime()) + + message = seen[0] + assert "llama3:latest" in message + assert "ollama" in message + assert "native code" in message + # It must not claim a containment ModelDock does not provide. + assert "does not sandbox" in message + + +def test_unrestricted_policy_is_silent() -> None: + seen: List[str] = [] + ExecutionGuard(UNRESTRICTED, notify=seen.append).check(ModelRef.parse("llama3"), FakeRuntime()) + assert seen == [] + + +def test_strict_refuses_a_runtime_that_executes_in_process() -> None: + with pytest.raises(ExecutionPolicyError) as excinfo: + ExecutionGuard(STRICT).check(ModelRef.parse("llama3"), _InProcessRuntime()) + + message = str(excinfo.value) + assert "gpt4all" in message + # The error has to say how to proceed deliberately, not just refuse. + assert "execution_policy" in message + + +def test_strict_still_allows_a_server_backed_runtime() -> None: + """Strict restricts what ModelDock itself executes, not all model use.""" + ExecutionGuard(STRICT).check(ModelRef.parse("llama3"), FakeRuntime()) + + +@pytest.mark.parametrize( + ("policy", "allowed"), + [(UNRESTRICTED, True), (WARN, True), (STRICT, False)], +) +def test_plugin_permission_follows_the_policy(policy: str, allowed: bool) -> None: + assert ExecutionGuard(policy).allows_plugins() is allowed + + +def test_an_undeclared_runtime_is_not_assumed_to_be_in_process() -> None: + """A duck-typed runtime without the attribute must not break strict mode.""" + + class _Bare: + backend = RuntimeBackend.OLLAMA + + ExecutionGuard(STRICT).check(ModelRef.parse("llama3"), _Bare()) + + +# --------------------------------------------------------------------------- +# Plugin discovery gating — the part that is real enforcement +# --------------------------------------------------------------------------- + + +def test_runtime_plugins_are_not_even_looked_up_when_disallowed() -> None: + """Not "loaded but ignored" — entry_points() is never consulted at all.""" + with patch("modeldock.adapters.runtimes.registry.entry_points") as eps: + RuntimeRegistry(allow_plugins=False) + + eps.assert_not_called() + + +def test_runtime_plugin_code_never_executes_when_disallowed() -> None: + plugin = _fake_entry_point("vllm", MagicMock()) + + with patch( + "modeldock.adapters.runtimes.registry.entry_points", + return_value=_patched_entry_points([plugin]), + ): + RuntimeRegistry(allow_plugins=False) + + plugin.load.assert_not_called() + + +def test_runtime_plugins_load_when_allowed() -> None: + """The gate must be a gate, not a permanent block.""" + plugin = _fake_entry_point("vllm", lambda: FakeRuntime()) + + with patch( + "modeldock.adapters.runtimes.registry.entry_points", + return_value=_patched_entry_points([plugin]), + ): + RuntimeRegistry(allow_plugins=True) + + plugin.load.assert_called_once() + + +def test_a_plugin_shadowing_a_builtin_is_logged( + caplog: pytest.LogCaptureFixture, monkeypatch: pytest.MonkeyPatch +) -> None: + """Nothing else reveals that the shipped adapter is no longer in use.""" + # ``configure_logging`` sets propagate=False on the "modeldock" logger, and + # any earlier CLI test in the session leaves it that way. caplog only sees + # records that reach the root logger, so restore propagation for this test; + # ``at_level(..., logger="modeldock")`` lifts that logger's ERROR level so a + # WARNING is not filtered before it propagates. + monkeypatch.setattr(logging.getLogger("modeldock"), "propagate", True) + plugin = _fake_entry_point("ollama", lambda: FakeRuntime()) + + with ( + patch( + "modeldock.adapters.runtimes.registry.entry_points", + return_value=_patched_entry_points([plugin]), + ), + caplog.at_level("WARNING", logger="modeldock"), + ): + RuntimeRegistry(allow_plugins=True) + + assert "replaces the built-in ollama adapter" in caplog.text + assert "third-party-pkg" in caplog.text + + +def test_modeldocks_own_entry_point_does_not_warn( + caplog: pytest.LogCaptureFixture, monkeypatch: pytest.MonkeyPatch +) -> None: + """ModelDock advertises its own ``ollama`` runtime as an entry point. + + Warning on that would fire on every single invocation and teach users to + ignore the warning that actually matters. + """ + monkeypatch.setattr(logging.getLogger("modeldock"), "propagate", True) + plugin = _fake_entry_point("ollama", lambda: FakeRuntime(), dist="modeldock") + + with ( + patch( + "modeldock.adapters.runtimes.registry.entry_points", + return_value=_patched_entry_points([plugin]), + ), + caplog.at_level("WARNING", logger="modeldock"), + ): + RuntimeRegistry(allow_plugins=True) + + assert caplog.text == "" + + +def test_a_real_registry_construction_is_quiet( + caplog: pytest.LogCaptureFixture, monkeypatch: pytest.MonkeyPatch +) -> None: + """Guards the above against the installed distribution, not a mock.""" + monkeypatch.setattr(logging.getLogger("modeldock"), "propagate", True) + + with caplog.at_level("WARNING", logger="modeldock"): + RuntimeRegistry(allow_plugins=True) + + assert "replaces the built-in" not in caplog.text + + +def test_catalog_plugins_are_not_looked_up_when_disallowed() -> None: + with patch("modeldock.adapters.registry.catalog_registry.entry_points") as eps: + CatalogProviderRegistry(allow_plugins=False) + + eps.assert_not_called() + + +def test_catalog_registry_still_serves_builtins_when_plugins_are_off(tmp_path: Path) -> None: + """Refusing plugins must not disable first-party catalogs.""" + registry = CatalogProviderRegistry(allow_plugins=False) + assert RuntimeBackend.LM_STUDIO in registry.available_backends() + + +# --------------------------------------------------------------------------- +# ModelManager wiring — both execution entry points +# --------------------------------------------------------------------------- + + +def _manager(policy: str, runtime: Any, notify: Any = None) -> ModelManager: + return ModelManager( + runtime=runtime, + registry=FakeRegistry(), + cache=FakeCache(), + settings=Settings(catalog_source="bundled", execution_policy=policy), + notify=notify, + ) + + +def test_load_applies_the_policy() -> None: + seen: List[str] = [] + runtime = FakeRuntime(installed=[ModelRef.parse("llama3")]) + + _manager(WARN, runtime, notify=seen.append).load("llama3") + + assert len(seen) == 1 + + +def test_run_applies_the_policy_too() -> None: + """``run`` bypasses LifecycleOrchestrator, so it needs its own check.""" + seen: List[str] = [] + runtime = FakeRuntime(installed=[ModelRef.parse("llama3")]) + + manager = _manager(WARN, runtime, notify=seen.append) + try: + manager.run("llama3") + except NotImplementedError: + pass # FakeRuntime has no interactive session; the guard ran first. + + assert len(seen) == 1 + + +def test_strict_refuses_before_the_runtime_is_touched() -> None: + runtime = _InProcessRuntime(installed=[ModelRef.parse("llama3")]) + + with pytest.raises(ExecutionPolicyError): + _manager(STRICT, runtime).load("llama3") + + # Refused, not merely warned about after the fact. + assert runtime.clients == [] + + +def test_strict_manager_builds_a_plugin_free_runtime_registry() -> None: + with patch("modeldock.adapters.runtimes.registry.entry_points") as eps: + _manager(STRICT, FakeRuntime()) + + eps.assert_not_called() + + +def test_the_library_is_silent_without_a_notify_channel( + capsys: pytest.CaptureFixture[str], +) -> None: + """SDK use must not print; only a front end that opts in does.""" + runtime = FakeRuntime(installed=[ModelRef.parse("llama3")]) + + _manager(WARN, runtime).load("llama3") + + assert capsys.readouterr().err == "" + + +def test_cli_warning_goes_to_stderr(capsys: pytest.CaptureFixture[str]) -> None: + """Never stdout: it would corrupt --json output and piped model tokens.""" + from modeldock.cli.console import print_warning + + print_warning("native code ahead") + + captured = capsys.readouterr() + assert captured.out == "" + assert "native code ahead" in captured.err diff --git a/tests/unit/test_security.py b/tests/unit/test_security.py index 5d7123f..f8a468f 100644 --- a/tests/unit/test_security.py +++ b/tests/unit/test_security.py @@ -5,7 +5,13 @@ import ast from pathlib import Path -SRC_ROOT = Path(__file__).parents[1] / "src" / "modeldock" +from modeldock.domain.model import ModelRef, RuntimeBackend + +# ``parents[2]`` — tests/unit/ -> tests/unit -> tests -> repo root. +# This was ``parents[1]`` and resolved to a directory that does not exist, so +# every audit below walked an empty file list and passed vacuously. +# ``test_audited_source_tree_is_discovered`` now makes that failure loud. +SRC_ROOT = Path(__file__).resolve().parents[2] / "src" / "modeldock" AUDITED_DIRS = ( SRC_ROOT / "adapters", SRC_ROOT / "common", @@ -21,6 +27,13 @@ def _python_files() -> list[Path]: return files +def test_audited_source_tree_is_discovered() -> None: + """The audits below are only meaningful if they actually walk source files.""" + for directory in AUDITED_DIRS: + assert directory.is_dir(), f"audited directory missing: {directory}" + assert _python_files(), "security audit walked zero files" + + def test_no_shell_execution_in_model_runtime_paths() -> None: """Model/runtime code must not invoke commands through a shell.""" forbidden_calls = { @@ -57,10 +70,15 @@ def test_no_shell_execution_in_model_runtime_paths() -> None: def test_model_names_are_treated_as_data() -> None: - """Shell metacharacters in model names must remain ordinary model-name data.""" + """Shell metacharacters in a model name must survive parsing as inert data. + + Routed through the real ``ModelRef.parse`` rather than asserted against a + literal: the boundary being defended is that ModelDock parses a name into a + domain object instead of ever letting it become a command fragment. + """ malicious_model_name = "model;echo injected && whoami | cat" - # This test documents the security boundary: model names may contain - # characters that have shell meaning, but the application must treat them - # as ordinary strings rather than command fragments. - assert malicious_model_name == "model;echo injected && whoami | cat" + ref = ModelRef.parse(malicious_model_name, backend=RuntimeBackend.OLLAMA) + + assert ref.name == malicious_model_name + assert ref.qualified_name() == f"{malicious_model_name}:latest" From 3cb15e9a02cf9830568d958ed078fe1d8b360405 Mon Sep 17 00:00:00 2001 From: albin-george-kurian Date: Thu, 10 Sep 2026 14:22:05 +0530 Subject: [PATCH 2/2] fix: detect first-party entry points without EntryPoint.dist --- src/modeldock/adapters/runtimes/registry.py | 31 ++++++++-- tests/unit/test_execution_policy.py | 67 +++++++++++++++++++-- 2 files changed, 89 insertions(+), 9 deletions(-) diff --git a/src/modeldock/adapters/runtimes/registry.py b/src/modeldock/adapters/runtimes/registry.py index 85b98ed..4d03239 100644 --- a/src/modeldock/adapters/runtimes/registry.py +++ b/src/modeldock/adapters/runtimes/registry.py @@ -26,16 +26,37 @@ def _distribution_name(ep: Any) -> str: """Return the distribution that provides ``ep``, or "" when unknown. - ``EntryPoint.dist`` is only populated for entry points obtained from - ``entry_points()``, and is absent on older interpreters, so this degrades - to "" rather than assuming provenance it cannot establish. + ``EntryPoint.dist`` does not exist at all before Python 3.10, and even + where it does it is only populated for entry points obtained from + ``entry_points()``. This degrades to "" rather than asserting provenance it + cannot establish. """ return str(getattr(getattr(ep, "dist", None), "name", "") or "") +def _target_root_package(ep: Any) -> str: + """Return the top-level package ``ep`` resolves into, or "". + + ``"modeldock.adapters.runtimes.ollama:OllamaRuntime"`` -> ``"modeldock"``. + """ + module = str(getattr(ep, "value", "") or "").split(":", 1)[0] + return module.strip().split(".", 1)[0] + + def _is_first_party(ep: Any) -> bool: - """Whether ``ep`` was advertised by ModelDock's own distribution.""" - return _distribution_name(ep).replace("_", "-").lower() == _OWN_DISTRIBUTION + """Whether ``ep`` was advertised by ModelDock itself. + + The distribution name is exact, so it is preferred where available. On + Python 3.9 ``EntryPoint`` carries no distribution at all, so fall back to + where the entry point actually points: one resolving into the ``modeldock`` + package is our own registration, not a third-party override. Without this + fallback every 3.9 invocation warns about ModelDock's own ``ollama`` entry + point — exactly the false alarm this check exists to prevent. + """ + distribution = _distribution_name(ep) + if distribution: + return distribution.replace("_", "-").lower() == _OWN_DISTRIBUTION + return _target_root_package(ep) == _OWN_DISTRIBUTION def _register_builtins() -> None: diff --git a/tests/unit/test_execution_policy.py b/tests/unit/test_execution_policy.py index 104ed53..bb15258 100644 --- a/tests/unit/test_execution_policy.py +++ b/tests/unit/test_execution_policy.py @@ -34,9 +34,15 @@ class _InProcessRuntime(FakeRuntime): executes_in_process = True -def _fake_entry_point(name: str, target: Any, dist: str = "third-party-pkg") -> MagicMock: +def _fake_entry_point( + name: str, + target: Any, + dist: str = "third-party-pkg", + value: str = "third_party_pkg.runtime:Runtime", +) -> MagicMock: ep = MagicMock() ep.name = name + ep.value = value ep.load.return_value = target # Provenance decides whether a registration is a security event; the # default here is a foreign distribution, since that is the case worth @@ -45,6 +51,15 @@ def _fake_entry_point(name: str, target: Any, dist: str = "third-party-pkg") -> return ep +def _entry_point_without_dist(name: str, target: Any, value: str) -> MagicMock: + """An entry point shaped like Python 3.9's, which carries no distribution.""" + ep = MagicMock(spec=["name", "value", "load"]) + ep.name = name + ep.value = value + ep.load.return_value = target + return ep + + def _patched_entry_points(entries: List[MagicMock]) -> MagicMock: """Mock importlib.metadata.entry_points() supporting .select().""" eps = MagicMock() @@ -189,7 +204,7 @@ def test_runtime_plugin_code_never_executes_when_disallowed() -> None: def test_runtime_plugins_load_when_allowed() -> None: """The gate must be a gate, not a permanent block.""" - plugin = _fake_entry_point("vllm", lambda: FakeRuntime()) + plugin = _fake_entry_point("vllm", FakeRuntime) with patch( "modeldock.adapters.runtimes.registry.entry_points", @@ -210,7 +225,7 @@ def test_a_plugin_shadowing_a_builtin_is_logged( # ``at_level(..., logger="modeldock")`` lifts that logger's ERROR level so a # WARNING is not filtered before it propagates. monkeypatch.setattr(logging.getLogger("modeldock"), "propagate", True) - plugin = _fake_entry_point("ollama", lambda: FakeRuntime()) + plugin = _fake_entry_point("ollama", FakeRuntime) with ( patch( @@ -234,7 +249,32 @@ def test_modeldocks_own_entry_point_does_not_warn( ignore the warning that actually matters. """ monkeypatch.setattr(logging.getLogger("modeldock"), "propagate", True) - plugin = _fake_entry_point("ollama", lambda: FakeRuntime(), dist="modeldock") + plugin = _fake_entry_point("ollama", FakeRuntime, dist="modeldock") + + with ( + patch( + "modeldock.adapters.runtimes.registry.entry_points", + return_value=_patched_entry_points([plugin]), + ), + caplog.at_level("WARNING", logger="modeldock"), + ): + RuntimeRegistry(allow_plugins=True) + + assert caplog.text == "" + + +def test_first_party_detection_survives_a_missing_distribution( + caplog: pytest.LogCaptureFixture, monkeypatch: pytest.MonkeyPatch +) -> None: + """Python 3.9's EntryPoint has no ``dist``; provenance falls back to the target. + + Without the fallback, every 3.9 invocation warns about ModelDock's own + ``ollama`` entry point. + """ + monkeypatch.setattr(logging.getLogger("modeldock"), "propagate", True) + plugin = _entry_point_without_dist( + "ollama", FakeRuntime, "modeldock.adapters.runtimes.ollama:OllamaRuntime" + ) with ( patch( @@ -248,6 +288,25 @@ def test_modeldocks_own_entry_point_does_not_warn( assert caplog.text == "" +def test_a_foreign_plugin_without_a_distribution_still_warns( + caplog: pytest.LogCaptureFixture, monkeypatch: pytest.MonkeyPatch +) -> None: + """The 3.9 fallback must not become a blanket exemption.""" + monkeypatch.setattr(logging.getLogger("modeldock"), "propagate", True) + plugin = _entry_point_without_dist("ollama", FakeRuntime, "evil_pkg.runtime:Evil") + + with ( + patch( + "modeldock.adapters.runtimes.registry.entry_points", + return_value=_patched_entry_points([plugin]), + ), + caplog.at_level("WARNING", logger="modeldock"), + ): + RuntimeRegistry(allow_plugins=True) + + assert "replaces the built-in ollama adapter" in caplog.text + + def test_a_real_registry_construction_is_quiet( caplog: pytest.LogCaptureFixture, monkeypatch: pytest.MonkeyPatch ) -> None: