diff --git a/jobbergate-api/CHANGELOG.md b/jobbergate-api/CHANGELOG.md index 8b0c0e1b..fd15095f 100644 --- a/jobbergate-api/CHANGELOG.md +++ b/jobbergate-api/CHANGELOG.md @@ -3,6 +3,7 @@ This file keeps track of all notable changes to jobbergate-api ## Unreleased +- Fixed issue on the Job Template rendering process by running it inside a Jinja Sandbox Environment ## 5.4.0 -- 2024-11-18 diff --git a/jobbergate-api/jobbergate_api/apps/services.py b/jobbergate-api/jobbergate_api/apps/services.py index 75af183d..8bfb6baf 100644 --- a/jobbergate-api/jobbergate_api/apps/services.py +++ b/jobbergate-api/jobbergate_api/apps/services.py @@ -12,8 +12,8 @@ from fastapi import HTTPException, UploadFile, status from fastapi_pagination import Page from fastapi_pagination.ext.sqlalchemy import paginate -from jinja2 import Template -from jinja2.exceptions import UndefinedError +from jinja2.sandbox import SandboxedEnvironment +from jinja2.exceptions import SecurityError, UndefinedError from loguru import logger from pydantic import AnyUrl from sqlalchemy import delete, func, not_, select, update @@ -720,13 +720,25 @@ async def render(self, instance: FileModel, parameters: dict[str, Any]) -> str: raise_exc_class=ServiceError, raise_kwargs=dict(status_code=status.HTTP_422_UNPROCESSABLE_ENTITY), ): - template = Template(file_content.decode("utf-8")) + sandbox_env = SandboxedEnvironment() + template = sandbox_env.from_string(file_content.decode("utf-8")) render_contexts = [parameters, {"data": parameters}] for context in render_contexts: try: return template.render(**context) + except SecurityError as e: + logger.debug( + "Security error rendering filename={} with context={} -- Error: {}", + instance.filename, + context, + str(e), + ) + raise ServiceError( + f"Jinja can not render filename={instance.filename}", + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + ) except UndefinedError as e: logger.debug( "Unable to render filename={} with context={} -- Error: {}", diff --git a/jobbergate-api/tests/apps/test_services.py b/jobbergate-api/tests/apps/test_services.py index 74aae684..fddc4e19 100644 --- a/jobbergate-api/tests/apps/test_services.py +++ b/jobbergate-api/tests/apps/test_services.py @@ -903,6 +903,23 @@ async def test_render__raises_422_on_bad_template(self, make_upload_file, dummy_ assert "TemplateSyntaxError" in exc_info.value.detail assert "Unable to process jinja template filename=file-one.txt" in exc_info.value.detail + async def test_render__raises_422_on_sandbox_violation(self, make_upload_file, dummy_file_service): + """ + Test that the ``render()`` method raises a 422 error if the template is unsecure. + """ + dummy_upload_file = make_upload_file() + with make_upload_file(content="dummy {{ foo.__str__() }} content") as dummy_upload_file: + upserted_instance = await dummy_file_service.upsert( + 13, + "file-one.txt", + dummy_upload_file, + ) + + with pytest.raises(HTTPException) as exc_info: + await dummy_file_service.render(upserted_instance, parameters=dict(foo="bar")) + assert exc_info.value.status_code == 422 + assert "Jinja can not render filename=file-one.txt" in exc_info.value.detail + async def test_render__backward_compatible(self, make_upload_file, dummy_file_service): """ Test that the ``render()`` works in different contexts. diff --git a/jobbergate-cli/jobbergate_cli/subapps/job_scripts/tools.py b/jobbergate-cli/jobbergate_cli/subapps/job_scripts/tools.py index 97d8cc96..2736e9d7 100644 --- a/jobbergate-cli/jobbergate_cli/subapps/job_scripts/tools.py +++ b/jobbergate-cli/jobbergate_cli/subapps/job_scripts/tools.py @@ -10,8 +10,8 @@ from functools import partial from typing import Any, Callable, Dict, List, Optional, cast -from jinja2 import Template -from jinja2.exceptions import UndefinedError +from jinja2.sandbox import SandboxedEnvironment +from jinja2.exceptions import SecurityError, UndefinedError from loguru import logger from jobbergate_cli.config import settings @@ -206,13 +206,20 @@ def render_template( log_message=f"Unable to process jinja template filename={template_path}", ), ): - template = Template(file_content) + sandbox_env = SandboxedEnvironment() + template = sandbox_env.from_string(file_content) render_contexts = [parameters, {"data": parameters}] for context in render_contexts: try: return template.render(**context) + except SecurityError: + raise Abort( + f"Security errors raised when rendering filename={template_path}", + subject="Unable to render jinja template", + log_message=f"Security error rendering filename={template_path}", + ) except UndefinedError as e: logger.debug( "Unable to render filename={} with context={} -- Error: {}", diff --git a/jobbergate-cli/tests/subapps/job_scripts/test_tools.py b/jobbergate-cli/tests/subapps/job_scripts/test_tools.py index 031914a4..581ab88a 100644 --- a/jobbergate-cli/tests/subapps/job_scripts/test_tools.py +++ b/jobbergate-cli/tests/subapps/job_scripts/test_tools.py @@ -182,6 +182,14 @@ def test_render_template__fail_when_unable_to_render(tmp_path): with pytest.raises(Abort, match="Unable to render"): render_template(template_path, parameters) +def test_render_template__fail_sandbox_violation(tmp_path): + template_path = tmp_path / "dummy.j2" + template_path.write_text("{{ foo.__str__() }}") + + parameters = dict(foo="bla") + + with pytest.raises(Abort, match="Security errors raised when rendering"): + render_template(template_path, parameters) def test_render_template__fails_if_template_does_not_exist(tmp_path): non_exist = tmp_path / "does/not/exist"