Skip to content

Commit

Permalink
fix(api): run rendering process in a Sandbox Environment
Browse files Browse the repository at this point in the history
  • Loading branch information
fschuch committed Dec 16, 2024
1 parent 75b741a commit 5ecb3df
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 6 deletions.
1 change: 1 addition & 0 deletions jobbergate-api/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
18 changes: 15 additions & 3 deletions jobbergate-api/jobbergate_api/apps/services.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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: {}",
Expand Down
17 changes: 17 additions & 0 deletions jobbergate-api/tests/apps/test_services.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 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.
Expand Down
13 changes: 10 additions & 3 deletions jobbergate-cli/jobbergate_cli/subapps/job_scripts/tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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: {}",
Expand Down
8 changes: 8 additions & 0 deletions jobbergate-cli/tests/subapps/job_scripts/test_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit 5ecb3df

Please sign in to comment.