Skip to content

feat: add restricted execution context (#139) - #232

Open
albin-george-kurian wants to merge 2 commits into
OpenAgentHQ:mainfrom
albin-george-kurian:docs/model-execution-security
Open

feat: add restricted execution context (#139)#232
albin-george-kurian wants to merge 2 commits into
OpenAgentHQ:mainfrom
albin-george-kurian:docs/model-execution-security

Conversation

@albin-george-kurian

Copy link
Copy Markdown
Contributor

Summary

Closes #139.

Model execution wasn't sandboxed or even flagged. load() and run() hand a model to a
runtime that executes it as native machine code with the invoking user's full privileges,
and nothing in the codebase said so — SECURITY.md had a thorough threat model for untrusted
model output and nothing at all for model execution. Separately, any installed
distribution advertising a modeldock.runtimes or modeldock.model_sources entry point was
imported and instantiated inside ModelDock's own process on every ModelManager
construction, with no allowlist, no opt-out, and no log line recording it.

This adds an execution_policy setting that does something real, a once-per-session
native-code warning, and the documentation both acceptance criteria ask for.

On what is actually enforced. Python cannot sandbox a native library already mapped into
its address space, nor a runtime server it does not supervise. So execution_policy
restricts what ModelDock's own process executes — it is not a sandbox, and the docs say
that in those words rather than implying containment that does not exist.

execution_policy 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 Never imported or executed Refused

Changes

  • src/modeldock/core/execution.py (new)ExecutionGuard, the single point where the
    policy is applied. ModelManager.run() bypasses LifecycleOrchestrator entirely, so both
    core/lifecycle.py and core/manager.py consult the guard; otherwise run would be the
    one path that executes a model unchecked.
  • src/modeldock/common/config.pyexecution_policy (unrestricted | warn | strict,
    default warn) threaded through all five layers: field, field_validator raising
    ConfigError, _apply_mapping for config.toml, env_map for
    MODELDOCK_EXECUTION_POLICY, and to_env_overrides. An enumerated string rather than a
    bool, so an env value cannot invert its own meaning via bool("false").
  • src/modeldock/common/errors.pyExecutionPolicyError, naming what was refused and how
    to proceed deliberately.
  • src/modeldock/adapters/runtimes/registry.py and
    src/modeldock/adapters/registry/catalog_registry.pyallow_plugins gate. Under
    strict, entry_points() is never consulted, so plugin code never executes. Plugin
    provenance is now logged: a foreign distribution shadowing a built-in adapter warns,
    since nothing else revealed the shipped adapter was replaced. entry_points hoisted to
    module scope to match its sibling module and make the call site visible.
  • src/modeldock/adapters/runtimes/base.pyexecutes_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.py and vllm.py declare True.
  • src/modeldock/cli/console.py / factory.py / commands/config.pyprint_warning
    (stderr, so it cannot corrupt --json output or piped tokens), wired as the CLI's notify
    channel, and the setting surfaced in modeldock config show. The policy decision stays in
    core; the CLI only supplies the output channel.
  • src/modeldock/ports/runtime.py — documents get_model_client/run as the execution
    boundary. Docstring only; no I/O, no new imports.
  • SECURITY.md + docs/project/security.md — new "Model Execution & Native Code" section:
    threat model for weight files, plugins and runtime processes; what execution_policy does
    and does not enforce; and a copy-pasteable container recipe for confining the runtime
    itself. The docs mirror is generated from SECURITY.md so the two cannot drift further — they
    already had.
  • Architecture.md, docs/architecture/runtime-adapters.md,
    docs/user-guide/configuration.md, CHANGELOG.md — settings list, contributor guidance,
    config table and env var, and an [Unreleased] entry.

Two findings worth calling out

The repo's only security test was passing vacuously. tests/unit/test_security.py used
Path(__file__).parents[1], resolving to tests/src/modeldock — a directory that does not
exist. The no-shell-execution audit walked zero files. Fixed to parents[2] (matching
test_workflow_permissions.py), plus a new test asserting the file list is non-empty so it
cannot silently degrade again. It now walks the real tree and still passes. Its companion
test asserted a string literal against itself; it now routes the hostile name through
ModelRef.parse.

The first draft of the plugin warning cried wolf. ModelDock advertises its own ollama
runtime as an entry point (pyproject.toml), so the "plugin replaces a built-in" warning
fired on a first-party registration on every single invocation — which teaches users to
ignore the warning that matters. Added a distribution-provenance check, with tests on both
sides including one against the really-installed distribution rather than a mock.

Testing

  • pytest645 passed, 4 skipped. The skips are pre-existing (Ollama CLI not installed
    on this machine); no test was skipped or weakened by this change.
  • tests/unit/test_execution_policy.py (new, 32 tests) — settings validation across
    config file, env var, invalid-value fallback and to_env_overrides; the guard warning
    firing exactly once per session and naming the mechanism, backend and the "does not
    sandbox" caveat; strict refusing an in-process backend before the runtime is touched;
    strict still allowing a server-backed backend; entry_points() never being consulted when
    plugins are disallowed; plugin code never executing; the gate still permitting plugins when
    allowed; first-party vs foreign entry-point logging; both load and run applying the
    policy; the SDK staying silent without a notify channel; and the CLI warning going to stderr
    rather than stdout.
  • tests/unit/test_security.py — repaired as above; now audits the real
    src/modeldock/{adapters,common} tree.
  • ruff check src tests — All checks passed. ruff format --check — 117 files already
    formatted.
  • mypy --strict src — Success, no issues in 81 source files.
  • bandit -c pyproject.toml -r src — 0 issues (0 high / 0 medium / 0 low).
  • Manual end-to-end: modeldock config show reflects the setting from config and
    MODELDOCK_EXECUTION_POLICY, and falls back to the default on an invalid value; on the real
    load path unrestricted is silent, warn warns once across two loads, strict raises
    ExecutionPolicyError with the runtime never reached (clients=[]) while still allowing a
    server-backed backend; and allow_plugins=False yields entry_points() consulted: False, plugin code executed: False.

Checklist

  • Branch named per Git Workflow (feature/, fix/, docs/, refactor/, test/, chore/)
  • Not developed on main
  • Code follows AGENT.md coding standards (type hints, Pydantic v2, no generic Exception, no business logic in CLI)
  • domain/ and ports/ stay pure (no I/O, no framework imports)
  • Quality gates pass locally: ruff, mypy --strict, bandit, pytest
  • Docs updated if behavior changed
  • pyproject.toml and src/modeldock/__init__.py versions match (if release-related)

Comment thread tests/unit/test_execution_policy.py Fixed
Comment thread tests/unit/test_execution_policy.py Fixed
Comment thread tests/unit/test_execution_policy.py Fixed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security: sandbox model execution option

2 participants