diff --git a/src/google/adk/flows/llm_flows/prompt/_instructions_utils.py b/src/google/adk/flows/llm_flows/prompt/_instructions_utils.py index 4b194df3c9..0cb697ef81 100644 --- a/src/google/adk/flows/llm_flows/prompt/_instructions_utils.py +++ b/src/google/adk/flows/llm_flows/prompt/_instructions_utils.py @@ -97,7 +97,8 @@ async def build_instruction( use_jinja2: If True, render the template with Jinja2 instead of the default regex-based engine. Defaults to False for backward compatibility. Jinja2 is an optional dependency and must be installed - separately to use this. + separately to use this. Templates are rendered in Jinja2's immutable + sandbox, so they cannot reach Python internals or modify session state. Returns: The instruction template with values populated. @@ -192,7 +193,7 @@ async def _render_with_jinja2( template: str, readonly_context: ReadonlyContext, ) -> str: - """Renders *template* using a Jinja2 environment. + """Renders *template* using a sandboxed Jinja2 environment. Session state variables are exposed as top-level template variables. Artifacts can be loaded with the ``artifact(filename)`` async callable @@ -213,6 +214,7 @@ async def _render_with_jinja2( """ try: import jinja2 + from jinja2.sandbox import ImmutableSandboxedEnvironment except ImportError as e: raise ImportError( 'Rendering an instruction with Jinja2 requires the optional jinja2' @@ -237,7 +239,10 @@ async def _load_artifact(filename: str) -> str: ) return str(artifact) - env = jinja2.Environment( + # Template text is not always fully trusted, and the context holds the live + # session state objects. The immutable sandbox blocks access to Python + # internals and in-place mutation of those objects. + env = ImmutableSandboxedEnvironment( enable_async=True, undefined=jinja2.StrictUndefined, autoescape=False, diff --git a/tests/unittests/flows/llm_flows/prompt/test_instructions_utils.py b/tests/unittests/flows/llm_flows/prompt/test_instructions_utils.py index 942e492e98..4d32e62d10 100644 --- a/tests/unittests/flows/llm_flows/prompt/test_instructions_utils.py +++ b/tests/unittests/flows/llm_flows/prompt/test_instructions_utils.py @@ -23,6 +23,7 @@ from google.adk.flows.llm_flows.prompt._instructions_utils import _is_valid_state_name from google.adk.flows.llm_flows.prompt._instructions_utils import InstructionProvider from google.adk.sessions.session import Session +from jinja2.exceptions import SecurityError import pytest from .... import testing_utils @@ -390,6 +391,44 @@ async def test_inject_session_state_jinja2_artifact_with_filter(): assert populated_instruction == "Content: ARTIFACT DATA" +@pytest.mark.asyncio +@pytest.mark.parametrize( + "instruction_template", + [ + "{{ ''.__class__.__mro__[1].__subclasses__() }}", + "{{ artifact.__globals__['__builtins__'] }}", + ], +) +async def test_inject_session_state_jinja2_blocks_python_internals( + instruction_template, +): + invocation_context = await _create_test_readonly_context( + artifact_service=MockArtifactService({}) + ) + + with pytest.raises(SecurityError): + await instructions_utils.inject_session_state( + instruction_template, invocation_context, use_jinja2=True + ) + + +@pytest.mark.asyncio +async def test_inject_session_state_jinja2_cannot_mutate_state(): + state = {"items": ["a"], "user": {"name": "Foo"}} + invocation_context = await _create_test_readonly_context(state=state) + + for instruction_template in ( + "{{ items.append('b') }}", + "{{ user.update(name='Bar') }}", + ): + with pytest.raises(SecurityError): + await instructions_utils.inject_session_state( + instruction_template, invocation_context, use_jinja2=True + ) + + assert state == {"items": ["a"], "user": {"name": "Foo"}} + + def test_module_imports_without_jinja2_installed(): # Jinja2 ships only in the eval and test extras, but this module is on the # import path of google.adk.agents, so a module-scope import of it would