Use ImmutableSandboxedEnvironment in jinja.py to prevent SSTI#1346
Open
JAE0Y2N wants to merge 1 commit into
Open
Use ImmutableSandboxedEnvironment in jinja.py to prevent SSTI#1346JAE0Y2N wants to merge 1 commit into
JAE0Y2N wants to merge 1 commit into
Conversation
The previous JinjaEnvironment exposed `inspect` and `os.getcwd` as Jinja
globals and used the raw (non-sandboxed) Environment. Any string that
flows into `Template.render()` from external input could escape via
either explicit globals (`{{ inspect.getmembers(os) }}`) or Python's
implicit class-traversal vectors (`{{ ''.__class__.__mro__[1].__subclasses__() }}`).
Marvin exposes prompt_template as a public field on Task, Agent, Memory,
Team, and Actor. If any of those flow from external input (LLM-generated,
HTTP request, file load), the SSTI primitive reaches arbitrary Python
execution in the host process.
Three changes:
1. Replace `from jinja2 import Environment as JinjaEnvironment` with
`from jinja2.sandbox import ImmutableSandboxedEnvironment as JinjaEnvironment`.
This blocks `__class__`, `__init__`, `__globals__`, `__builtins__`,
`__import__`, `__subclasses__`, and the rest of the standard
class-traversal escape chain.
2. Remove `inspect` from global_fns. It was not referenced by any
shipped template (grepped src/marvin/templates/) and its only
plausible runtime use is the SSTI escape path itself.
3. Remove `getcwd` from global_fns. Same reasoning: not used by any
template, and exposes the host's working directory to template
consumers.
Verified that all 5 shipped templates (task, memory, system, agent,
team) load + parse cleanly under ImmutableSandboxedEnvironment. The
SecurityError raised by the sandbox is the expected behavior on
disallowed attribute access; non-malicious templates that only access
Marvin's own typed model attributes (task.id, agent.name, etc.) continue
to render normally.
Disclosed to bugbounty@prefect.io 2026-05-17; out of bounty scope per
the program (only Prefect Cloud / Prefect OSS / Horizon are in scope),
but the team explicitly invited a PR.
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
1 participant
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.
The Jinja env in
src/marvin/utilities/jinja.pywas a rawEnvironmentwithinspectandos.getcwdexposed as globals. Any string that reachesTemplate.render()from external input —prompt_templateon Task / Agent / Memory / Team / Actor — could escape to arbitrary Python via either the explicit globals ({{ inspect.getmembers(os) }}) or the implicit class-traversal vectors ({{ ''.__class__.__mro__[1].__subclasses__() }}and friends).Fix is three small changes in one file:
Environmentforjinja2.sandbox.ImmutableSandboxedEnvironment. That blocks the standard class-traversal chain (__class__/__init__/__globals__/__subclasses__) at the sandbox layer.inspectfromglobal_fns. Not referenced by any shipped template — greppedsrc/marvin/templates/— so removing it doesn't break anything and removes the most direct escape primitive.getcwdfromglobal_fns. Same reasoning — not used, and exposes host cwd to template consumers for no clear benefit.Verified all 5 shipped templates (task, memory, system, agent, team) load and parse under the sandboxed env. The sandbox raises
SecurityErroron disallowed attribute access, which is the right behavior — non-malicious templates that only read Marvin's typed model attributes (task.id,agent.name, etc.) keep working unchanged.Background: I emailed this to bugbounty@prefect.io on May 17. The team confirmed the finding is technically valid but PrefectHQ/marvin is out of bounty scope (only Prefect Cloud / Prefect OSS / Horizon are in scope), and invited a PR — this is that PR.