Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(api): run rendering process in a Sandbox Environment #671

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
fschuch marked this conversation as resolved.
Show resolved Hide resolved

## 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 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.
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
Loading