From f5c0d118f7cc8becac850c01be16ade9ca81001d Mon Sep 17 00:00:00 2001 From: Max Truxa Date: Thu, 13 Aug 2026 21:46:57 +0200 Subject: [PATCH] Fix: fix validation of workflow title assignment (#13659) --- src/documents/serialisers.py | 28 +---- src/documents/templating/workflows.py | 45 ++++++++ src/documents/tests/test_api_workflows.py | 126 +++++++++++++++++++++- 3 files changed, 169 insertions(+), 30 deletions(-) diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index c81640b0d..30df87455 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -85,6 +85,7 @@ from documents.permissions import set_permissions_for_object from documents.regex import validate_regex_pattern from documents.templating.filepath import validate_filepath_template_and_render from documents.templating.utils import convert_format_str_to_template_format +from documents.templating.workflows import validate_workflow_template from documents.validators import uri_validator from documents.validators import url_validator @@ -3185,33 +3186,10 @@ class WorkflowActionSerializer(serializers.ModelSerializer[WorkflowAction]): attrs["assign_title"] = None else: try: - # test against all placeholders, see consumer.py `parse_doc_title_w_placeholders` - attrs["assign_title"].format( - correspondent="", - document_type="", - added="", - added_year="", - added_year_short="", - added_month="", - added_month_name="", - added_month_name_short="", - added_day="", - added_time="", - owner_username="", - original_filename="", - filename="", - created="", - created_year="", - created_year_short="", - created_month="", - created_month_name="", - created_month_name_short="", - created_day="", - created_time="", - ) + validate_workflow_template(attrs["assign_title"]) except (ValueError, KeyError) as e: raise serializers.ValidationError( - {"assign_title": f'Invalid f-string detected: "{e.args[0]}"'}, + {"assign_title": f"{e.args[0]}"}, ) if attrs.get("assign_custom_fields_values"): diff --git a/src/documents/templating/workflows.py b/src/documents/templating/workflows.py index 66fd97e01..f1c94913a 100644 --- a/src/documents/templating/workflows.py +++ b/src/documents/templating/workflows.py @@ -6,9 +6,11 @@ from pathlib import Path from django.utils.text import slugify as django_slugify from jinja2 import StrictUndefined from jinja2 import Template +from jinja2 import TemplateAssertionError from jinja2 import TemplateSyntaxError from jinja2 import UndefinedError from jinja2 import make_logging_undefined +from jinja2.meta import find_undeclared_variables from jinja2.sandbox import SecurityError from documents.templating.environment import _template_environment @@ -29,6 +31,49 @@ _template_environment.filters["slugify"] = django_slugify _template_environment.filters["localize_date"] = localize_date +_known_placeholder_names = { + "correspondent", + "document_type", + "added", + "added_year", + "added_year_short", + "added_month", + "added_month_name", + "added_month_name_short", + "added_day", + "added_time", + "owner_username", + "original_filename", + "filename", + "created", + "created_year", + "created_year_short", + "created_month", + "created_month_name", + "created_month_name_short", + "created_day", + "created_time", + "doc_title", + "doc_url", + "doc_id", +} + + +def validate_workflow_template(text: str) -> None: + try: + ast = _template_environment.parse(text) + undeclared_vars = find_undeclared_variables(ast) + except TemplateAssertionError as e: + raise ValueError(f"Template assertion error: {e}") + except TemplateSyntaxError as e: + raise ValueError(f"Template syntax error: {e}") + unknown_vars = undeclared_vars - _known_placeholder_names + if unknown_vars: + raise KeyError( + f"Template references unknown placeholders: {', '.join(unknown_vars)}", + ) + + def parse_w_workflow_placeholders( text: str, correspondent_name: str, diff --git a/src/documents/tests/test_api_workflows.py b/src/documents/tests/test_api_workflows.py index e2689001a..f3d51d686 100644 --- a/src/documents/tests/test_api_workflows.py +++ b/src/documents/tests/test_api_workflows.py @@ -351,11 +351,45 @@ class TestApiWorkflows(DirectoriesMixin, APITestCase): self.assertEqual(WorkflowTrigger.objects.count(), 1) - def test_api_create_invalid_assign_title(self) -> None: + def test_api_create_complex_assign_title(self) -> None: """ GIVEN: - API request to create a workflow - - Invalid f-string for assign_title + - Template using Jinja flow control statements + WHEN: + - API is called + THEN: + - Workflow is created + """ + response = self.client.post( + self.ENDPOINT, + json.dumps( + { + "name": "Workflow 2", + "order": 1, + "triggers": [ + { + "type": WorkflowTrigger.WorkflowTriggerType.DOCUMENT_UPDATED, + }, + ], + "actions": [ + { + "assign_title": '{# this is a comment #}foo{% if created_year < 2000 %}bar{% endif %}{{ "{:04d}".format(42) }}', + }, + ], + }, + ), + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + self.assertEqual(Workflow.objects.count(), 2) + + def test_api_create_invalid_assign_title_syntax_error(self) -> None: + """ + GIVEN: + - API request to create a workflow + - Invalid template for assign_title WHEN: - API is called THEN: @@ -366,7 +400,7 @@ class TestApiWorkflows(DirectoriesMixin, APITestCase): self.ENDPOINT, json.dumps( { - "name": "Workflow 1", + "name": "Workflow 2", "order": 1, "triggers": [ { @@ -375,7 +409,7 @@ class TestApiWorkflows(DirectoriesMixin, APITestCase): ], "actions": [ { - "assign_title": "{created_year]", + "assign_title": "{{created_year}", }, ], }, @@ -384,7 +418,89 @@ class TestApiWorkflows(DirectoriesMixin, APITestCase): ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertIn( - "Invalid f-string detected", + "Template syntax error", + response.data["actions"][0]["assign_title"][0], + ) + + self.assertEqual(Workflow.objects.count(), 1) + + def test_api_create_invalid_assign_title_assertion_error(self) -> None: + """ + GIVEN: + - API request to create a workflow + - Template using unknown filters for assign_title + WHEN: + - API is called + THEN: + - Correct HTTP 400 response + - No objects are created + """ + response = self.client.post( + self.ENDPOINT, + json.dumps( + { + "name": "Workflow 2", + "order": 1, + "triggers": [ + { + "type": WorkflowTrigger.WorkflowTriggerType.DOCUMENT_UPDATED, + }, + ], + "actions": [ + { + "assign_title": "{{ created_year | foo }}", + }, + ], + }, + ), + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn( + "Template assertion error", + response.data["actions"][0]["assign_title"][0], + ) + + self.assertEqual(Workflow.objects.count(), 1) + + def test_api_create_invalid_assign_title_unknown_placeholder(self) -> None: + """ + GIVEN: + - API request to create a workflow + - Template with unknown placeholders for assign_title + WHEN: + - API is called + THEN: + - Correct HTTP 400 response + - No objects are created + """ + response = self.client.post( + self.ENDPOINT, + json.dumps( + { + "name": "Workflow 2", + "order": 1, + "triggers": [ + { + "type": WorkflowTrigger.WorkflowTriggerType.DOCUMENT_UPDATED, + }, + ], + "actions": [ + { + "assign_title": "{{creation_year}}", + }, + ], + }, + ), + content_type="application/json", + ) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertIn( + "Template references unknown placeholders", + response.data["actions"][0]["assign_title"][0], + ) + self.assertIn( + "creation_year", response.data["actions"][0]["assign_title"][0], )