feat: add restricted execution context (#139) - #232
Open
albin-george-kurian wants to merge 2 commits into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #139.
Model execution wasn't sandboxed or even flagged.
load()andrun()hand a model to aruntime 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.runtimesormodeldock.model_sourcesentry point wasimported and instantiated inside ModelDock's own process on every
ModelManagerconstruction, with no allowlist, no opt-out, and no log line recording it.
This adds an
execution_policysetting that does something real, a once-per-sessionnative-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_policyrestricts 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_policyunrestrictedwarn(default)strictChanges
src/modeldock/core/execution.py(new) —ExecutionGuard, the single point where thepolicy is applied.
ModelManager.run()bypassesLifecycleOrchestratorentirely, so bothcore/lifecycle.pyandcore/manager.pyconsult the guard; otherwiserunwould be theone path that executes a model unchecked.
src/modeldock/common/config.py—execution_policy(unrestricted|warn|strict,default
warn) threaded through all five layers: field,field_validatorraisingConfigError,_apply_mappingforconfig.toml,env_mapforMODELDOCK_EXECUTION_POLICY, andto_env_overrides. An enumerated string rather than abool, so an env value cannot invert its own meaning via
bool("false").src/modeldock/common/errors.py—ExecutionPolicyError, naming what was refused and howto proceed deliberately.
src/modeldock/adapters/runtimes/registry.pyandsrc/modeldock/adapters/registry/catalog_registry.py—allow_pluginsgate. Understrict,entry_points()is never consulted, so plugin code never executes. Pluginprovenance is now logged: a foreign distribution shadowing a built-in adapter warns,
since nothing else revealed the shipped adapter was replaced.
entry_pointshoisted tomodule scope to match its sibling module and make the call site visible.
src/modeldock/adapters/runtimes/base.py—executes_in_process, declaring whether anadapter loads weights into ModelDock's interpreter rather than driving a separate server.
Shipped HTTP-backed adapters are
False;gpt4all.pyandvllm.pydeclareTrue.src/modeldock/cli/console.py/factory.py/commands/config.py—print_warning(stderr, so it cannot corrupt
--jsonoutput or piped tokens), wired as the CLI's notifychannel, and the setting surfaced in
modeldock config show. The policy decision stays incore; the CLI only supplies the output channel.src/modeldock/ports/runtime.py— documentsget_model_client/runas the executionboundary. 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_policydoesand 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.pyusedPath(__file__).parents[1], resolving totests/src/modeldock— a directory that does notexist. The no-shell-execution audit walked zero files. Fixed to
parents[2](matchingtest_workflow_permissions.py), plus a new test asserting the file list is non-empty so itcannot 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
ollamaruntime as an entry point (
pyproject.toml), so the "plugin replaces a built-in" warningfired 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
pytest— 645 passed, 4 skipped. The skips are pre-existing (Ollama CLI not installedon this machine); no test was skipped or weakened by this change.
tests/unit/test_execution_policy.py(new, 32 tests) — settings validation acrossconfig file, env var, invalid-value fallback and
to_env_overrides; the guard warningfiring exactly once per session and naming the mechanism, backend and the "does not
sandbox" caveat;
strictrefusing an in-process backend before the runtime is touched;strictstill allowing a server-backed backend;entry_points()never being consulted whenplugins are disallowed; plugin code never executing; the gate still permitting plugins when
allowed; first-party vs foreign entry-point logging; both
loadandrunapplying thepolicy; 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 realsrc/modeldock/{adapters,common}tree.ruff check src tests— All checks passed.ruff format --check— 117 files alreadyformatted.
mypy --strict src— Success, no issues in 81 source files.bandit -c pyproject.toml -r src— 0 issues (0 high / 0 medium / 0 low).modeldock config showreflects the setting from config andMODELDOCK_EXECUTION_POLICY, and falls back to the default on an invalid value; on the realload path
unrestrictedis silent,warnwarns once across two loads,strictraisesExecutionPolicyErrorwith the runtime never reached (clients=[]) while still allowing aserver-backed backend; and
allow_plugins=Falseyieldsentry_points() consulted: False, plugin code executed: False.Checklist
feature/,fix/,docs/,refactor/,test/,chore/)mainAGENT.mdcoding standards (type hints, Pydantic v2, no genericException, no business logic in CLI)domain/andports/stay pure (no I/O, no framework imports)ruff,mypy --strict,bandit,pytestpyproject.tomlandsrc/modeldock/__init__.pyversions match (if release-related)