From e2ec658d2026be705e7cf0dd3a6fb83b2b5a4386 Mon Sep 17 00:00:00 2001 From: Jens Scheffler Date: Wed, 1 Nov 2023 21:50:48 +0100 Subject: [PATCH 01/13] Add config option to trust HTML code in param descriptions --- airflow/config_templates/config.yml | 12 ++++ airflow/www/views.py | 61 +++++++++++++------- docs/apache-airflow/core-concepts/params.rst | 15 ++++- newsfragments/tbd.significant.rst | 6 ++ 4 files changed, 71 insertions(+), 23 deletions(-) create mode 100644 newsfragments/tbd.significant.rst diff --git a/airflow/config_templates/config.yml b/airflow/config_templates/config.yml index ffb9dce073ad5..c87865ac5025c 100644 --- a/airflow/config_templates/config.yml +++ b/airflow/config_templates/config.yml @@ -1821,6 +1821,18 @@ webserver: type: boolean example: ~ default: "False" + trigger_form_param_html_trust_level: + description: | + Level of trust which is given to DAG authors for raw HTML that is passed from trigger form + descriptions and custom HTML forms. As a DAG author is able to provide any HTML also + including potentially unsafe javascript in params description, displaying the trigger form + might provide potential to inject code into clients browsers. + Current version provides two options, "FullTrust" allows unfiltered HTML to be used, "None" + will block HTML code in descriptions. + version_added: 2.8.0 + type: string + example: FullTrust + default: None email: description: | Configuration email backend and whether to diff --git a/airflow/www/views.py b/airflow/www/views.py index 386c7e9350a30..5117e7ecdaa53 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -1952,30 +1952,49 @@ def trigger(self, dag_id: str, session: Session = NEW_SESSION): # Prepare form fields with param struct details to render a proper form with schema information form_fields = {} + form_trust_problems = [] for k, v in dag.params.items(): form_fields[k] = v.dump() + form_field: dict = form_fields[k] # If no schema is provided, auto-detect on default values - if "schema" not in form_fields[k]: - form_fields[k]["schema"] = {} - if "type" not in form_fields[k]["schema"]: - if isinstance(form_fields[k]["value"], bool): - form_fields[k]["schema"]["type"] = "boolean" - elif isinstance(form_fields[k]["value"], int): - form_fields[k]["schema"]["type"] = ["integer", "null"] - elif isinstance(form_fields[k]["value"], list): - form_fields[k]["schema"]["type"] = ["array", "null"] - elif isinstance(form_fields[k]["value"], dict): - form_fields[k]["schema"]["type"] = ["object", "null"] - # Mark markup fields as safe - if ( - "description_html" in form_fields[k]["schema"] - and form_fields[k]["schema"]["description_html"] - ): - form_fields[k]["description"] = Markup(form_fields[k]["schema"]["description_html"]) - if "custom_html_form" in form_fields[k]["schema"]: - form_fields[k]["schema"]["custom_html_form"] = Markup( - form_fields[k]["schema"]["custom_html_form"] - ) + if "schema" not in form_field: + form_field["schema"] = {} + form_field_schema: dict = form_field["schema"] + if "type" not in form_field_schema: + form_field_value = form_field["value"] + if isinstance(form_field_value, bool): + form_field_schema["type"] = "boolean" + elif isinstance(form_field_value, int): + form_field_schema["type"] = ["integer", "null"] + elif isinstance(form_field_value, list): + form_field_schema["type"] = ["array", "null"] + elif isinstance(form_field_value, dict): + form_field_schema["type"] = ["object", "null"] + # Mark HTML fields as safe if allowed + trust_level = conf.get("webserver", "trigger_form_param_html_trust_level") + if trust_level == "FullTrust": + if "description_html" in form_field_schema: + form_field["description"] = Markup(form_field_schema["description_html"]) + if "custom_html_form" in form_field_schema: + form_field_schema["custom_html_form"] = Markup(form_field_schema["custom_html_form"]) + else: + if "description_html" in form_field_schema: + form_trust_problems.append(f"Field {k} uses HTML description") + form_field["description"] = form_field_schema.pop("description_html") + if "custom_html_form" in form_field_schema: + form_trust_problems.append(f"Field {k} uses custom HTML form definition") + form_field_schema.pop("custom_html_form") + if form_trust_problems: + flash( + Markup( + f"At least one field in trigger form uses custom HTML form definition. This is not allowed per " + "configured trust level. Change trigger_form_param_html_trust_level to enable HTML." + "Using plain text as fallback for these fields." + f"" + ), + "warning", + ) + ui_fields_defined = any("const" not in f["schema"] for f in form_fields.values()) show_trigger_form_if_no_params = conf.getboolean("webserver", "show_trigger_form_if_no_params") diff --git a/docs/apache-airflow/core-concepts/params.rst b/docs/apache-airflow/core-concepts/params.rst index b2b95252ec719..a9a3579b050f3 100644 --- a/docs/apache-airflow/core-concepts/params.rst +++ b/docs/apache-airflow/core-concepts/params.rst @@ -173,6 +173,8 @@ JSON Schema Validation Use Params to Provide a Trigger UI Form --------------------------------------- +.. versionadded:: 2.6.0 + :class:`~airflow.models.dag.DAG` level params are used to render a user friendly trigger form. This form is provided when a user clicks on the "Trigger DAG" button. @@ -190,7 +192,8 @@ The following features are supported in the Trigger UI Form: If no ``title`` is defined the parameter name/key is used instead. - The :class:`~airflow.models.param.Param` attribute ``description`` is rendered below an entry field as help text in gray color. If you want to provide HTML tags for special formatting or links you need to use the Param attribute - ``description_html``, see tutorial DAG ``example_params_ui_tutorial`` for an example. + ``description_html`` and adjust the webserver configuration ``trigger_form_param_html_trust_level`` allowing raw HTML, + see tutorial DAG ``example_params_ui_tutorial`` for an example. - The :class:`~airflow.models.param.Param` attribute ``type`` influences how a field is rendered. The following types are supported: .. list-table:: @@ -313,7 +316,8 @@ The following features are supported in the Trigger UI Form: The ``const`` value must match the default value to pass `JSON Schema validation `_. - On the bottom of the form the generated JSON configuration can be expanded. If you want to change values manually, the JSON configuration can be adjusted. Changes are overridden when form fields change. -- If you want to render custom HTML as form on top of the provided features, you can use the ``custom_html_form`` attribute. +- If you want to render custom HTML as form on top of the provided features, you can use the ``custom_html_form`` attribute it the + webserver configuration ``trigger_form_param_html_trust_level`` allowing raw HTML. .. note:: If the field is required the default value must be valid according to the schema as well. If the DAG is defined with @@ -328,6 +332,13 @@ For examples also please take a look to two example DAGs provided: ``example_par The trigger form can also be forced to be displayed also if no params are defined using the configuration switch ``webserver.show_trigger_form_if_no_params``. +.. versionadded:: 2.8.0 + +Per default custom HTML is not allowed to prevent injection of scripts or other maliceus HTML code. If you trust your DAG authors +you can change the trust level of parameter descriptions to allow raw HTML by setting the configuration entry +``webserver.trigger_form_param_html_trust_level`` to ``FullTrust``. With the default of ``None`` all HTML will be displayed as +plain text. In future maybe more HTML filtering options might be added. + Disabling Runtime Param Modification ------------------------------------ diff --git a/newsfragments/tbd.significant.rst b/newsfragments/tbd.significant.rst new file mode 100644 index 0000000000000..dfc8a06ef96a3 --- /dev/null +++ b/newsfragments/tbd.significant.rst @@ -0,0 +1,6 @@ +Per default raw HTML code in parameter descriptions is disabled with 2.8.0. + +To ensure that no maliceus javascript can be injected with Trigger UI form by DAG authors +a new parameter ``webserver.trigger_form_param_html_trust_level`` was added with default value of ``None``. +If you trust your DAG authors code and want to allow using raw HTML in DAG params and restore the previous +behavior you must set the configuration value to ``FullTrust``. From 51cc5bad650dfccf40f4d009851f0d761bca7e0c Mon Sep 17 00:00:00 2001 From: Jens Scheffler Date: Wed, 1 Nov 2023 22:46:16 +0100 Subject: [PATCH 02/13] Add pytest for parameter trigger_form_param_html_trust_level --- tests/www/views/test_views_trigger_dag.py | 44 ++++++++++++++++++++++- 1 file changed, 43 insertions(+), 1 deletion(-) diff --git a/tests/www/views/test_views_trigger_dag.py b/tests/www/views/test_views_trigger_dag.py index c48e053639fd7..4f1f96870ef51 100644 --- a/tests/www/views/test_views_trigger_dag.py +++ b/tests/www/views/test_views_trigger_dag.py @@ -31,7 +31,7 @@ from airflow.utils.types import DagRunType from tests.test_utils.api_connexion_utils import create_test_client from tests.test_utils.config import conf_vars -from tests.test_utils.www import check_content_in_response +from tests.test_utils.www import check_content_in_response, check_content_not_in_response pytestmark = pytest.mark.db_test @@ -236,6 +236,48 @@ def test_trigger_dag_params_render(admin_client, dag_maker, session, app, monkey ) +@pytest.mark.parametrize( + "trust, expect_escape", + [ + ["None", True], + ["FullTrust", False], + ], +) +def test_trigger_dag_html_trust(admin_client, dag_maker, session, app, monkeypatch, trust, expect_escape): + """ + Test that HTML is masked per default in description. + """ + from markupsafe import escape + + with conf_vars({("webserver", "trigger_form_param_html_trust_level"): trust}): + DAG_ID = "params_dag" + HTML_DESCRIPTION = "HTML raw code." + param1 = Param( + 42, + description_html=HTML_DESCRIPTION, + type="integer", + minimum=1, + maximum=100, + ) + with monkeypatch.context() as m: + with dag_maker(dag_id=DAG_ID, serialized=True, session=session, params={"param1": param1}): + EmptyOperator(task_id="task1") + + m.setattr(app, "dag_bag", dag_maker.dagbag) + resp = admin_client.get(f"dags/{DAG_ID}/trigger") + + if expect_escape: + check_content_in_response(escape(HTML_DESCRIPTION), resp) + check_content_in_response( + "At least one field in trigger form uses custom HTML form definition.", resp + ) + else: + check_content_in_response(HTML_DESCRIPTION, resp) + check_content_not_in_response( + "At least one field in trigger form uses custom HTML form definition.", resp + ) + + def test_trigger_endpoint_uses_existing_dagbag(admin_client): """ Test that Trigger Endpoint uses the DagBag already created in views.py From 6771eef2b49b49112863d8a0802cc561d6d48cc2 Mon Sep 17 00:00:00 2001 From: Jens Scheffler Date: Sun, 5 Nov 2023 16:53:52 +0100 Subject: [PATCH 03/13] Implement markdown support for trigger ui forms --- airflow/config_templates/config.yml | 21 ++-- .../example_params_ui_tutorial.py | 101 ++++-------------- airflow/www/utils.py | 15 +-- airflow/www/views.py | 12 ++- docs/apache-airflow/core-concepts/params.rst | 15 +-- newsfragments/tbd.significant.rst | 14 ++- 6 files changed, 61 insertions(+), 117 deletions(-) diff --git a/airflow/config_templates/config.yml b/airflow/config_templates/config.yml index c87865ac5025c..3bec46c184788 100644 --- a/airflow/config_templates/config.yml +++ b/airflow/config_templates/config.yml @@ -1821,18 +1821,17 @@ webserver: type: boolean example: ~ default: "False" - trigger_form_param_html_trust_level: - description: | - Level of trust which is given to DAG authors for raw HTML that is passed from trigger form - descriptions and custom HTML forms. As a DAG author is able to provide any HTML also - including potentially unsafe javascript in params description, displaying the trigger form - might provide potential to inject code into clients browsers. - Current version provides two options, "FullTrust" allows unfiltered HTML to be used, "None" - will block HTML code in descriptions. + allow_html_in_dag_docs: + description: | + A DAG author is able to provide any raw HTML into ``doc_md`` or params description for text + formatting. This is including potentially unsafe javascript. Displaying the DAG or trigger + form in web UI provides the DAG author the potential to inject malicieus code into clients + browsers. To ensure the web UI is safe by default, raw HTML is disabled by default. If you + trust your DAG authors, you can enable HTML support by setting this option to True. version_added: 2.8.0 - type: string - example: FullTrust - default: None + type: boolean + example: "False" + default: "False" email: description: | Configuration email backend and whether to diff --git a/airflow/example_dags/example_params_ui_tutorial.py b/airflow/example_dags/example_params_ui_tutorial.py index 12992c545c25a..1ca607a6e3f9b 100644 --- a/airflow/example_dags/example_params_ui_tutorial.py +++ b/airflow/example_dags/example_params_ui_tutorial.py @@ -47,18 +47,17 @@ "flag": False, "a_simple_list": ["one", "two", "three", "actually one value is made per line"], # But of course you might want to have it nicer! Let's add some description to parameters. - # Note if you can add any HTML formatting to the description, you need to use the description_html + # Note if you can add any MD formatting to the description, you need to use the description_md # attribute. "most_loved_number": Param( 42, type="integer", title="Your favorite number", - description_html="""Everybody should have a favorite number. Not only math teachers. - If you can not think of any at the moment please think of the 42 which is very famous because - of the book - - The Hitchhiker's Guide to the Galaxy""", + description_md="Everybody should have a **favorite** number. Not only _math teachers_. " + "If you can not think of any at the moment please think of the 42 which is very famous because" + "of the book [The Hitchhiker's Guide to the Galaxy]" + "(https://en.wikipedia.org/wiki/Phrases_from_The_Hitchhiker%27s_Guide_to_the_Galaxy#" + "The_Answer_to_the_Ultimate_Question_of_Life,_the_Universe,_and_Everything_is_42).", ), # If you want to have a selection list box then you can use the enum feature of JSON schema "pick_one": Param( @@ -177,8 +176,8 @@ "optional text, you can trigger also w/o text", type=["null", "string"], title="Optional text field", - description_html="This field is optional. As field content is JSON schema validated you must " - "allow the null type.", + description_md="This field is optional. As field content is JSON schema validated you must " + "allow the `null` type.", ), # You can arrange the entry fields in sections so that you can have a better overview for the user # Therefore you can add the "section" attribute. @@ -188,10 +187,10 @@ "length-checked-field", type="string", title="Text field with length check", - description_html="""This field is required. And you need to provide something between 10 and 30 - characters. See the - - JSON schema description (string) in for more details""", + description_md="""This field is required. And you need to provide something between 10 and 30 + characters. See the JSON + [schema description (string)](https://json-schema.org/understanding-json-schema/reference/string.html) + for more details""", minLength=10, maxLength=20, section="JSON Schema validation options", @@ -200,9 +199,10 @@ 100, type="number", title="Number field with value check", - description_html="""This field is required. You need to provide any number between 64 and 128. - See the - JSON schema description (numbers) in for more details""", + description_md="""This field is required. You need to provide any number between 64 and 128. + See the JSON + [schema description (numbers)](https://json-schema.org/understanding-json-schema/reference/numeric.html) + for more details""", minimum=64, maximum=128, section="JSON Schema validation options", @@ -217,9 +217,9 @@ ), "array_of_objects": Param( [{"name": "account_name", "country": "country_name"}], - "Array with complex objects and validation rules. " - "See JSON Schema validation options in specs.", + description_md="Array with complex objects and validation rules. " + "See [JSON Schema validation options in specs]" + "(https://json-schema.org/understanding-json-schema/reference/array.html#items).", type="array", title="JSON array field", items={ @@ -233,69 +233,6 @@ # then you can use the JSON schema option of passing constant values. These parameters # will not be displayed but passed to the DAG "hidden_secret_field": Param("constant value", const="constant value"), - # Finally besides the standard provided field generator you can have you own HTML form code - # injected - but be careful, you can also mess-up the layout! - "color_picker": Param( - "#FF8800", - type="string", - title="Pick a color", - description_html="""This is a special HTML widget as custom implementation in the DAG code. - It is templated with the following parameter to render proper HTML form fields: -
    -
  • {name}: Name of the HTML input field that is expected.
    • -
  • {value}: - (Default) value that should be displayed when showing/loading the form.
    • -
  • Note: If you have elements changing a value, call updateJSONconf() to update - the form data to be posted as dag_run.conf.
    • -
- Example: <input name='{name}' value='{value}' onchange='updateJSONconf()' /> - """, - custom_html_form=""" -
- - - - -
 
- -
- - - -
- - - -
- """, - section="Special advanced stuff with form fields", - ), }, ) as dag: diff --git a/airflow/www/utils.py b/airflow/www/utils.py index 9d7728798e7bb..390b747bd71ff 100644 --- a/airflow/www/utils.py +++ b/airflow/www/utils.py @@ -38,6 +38,7 @@ from sqlalchemy import delete, func, select, types from sqlalchemy.ext.associationproxy import AssociationProxy +from airflow.configuration import conf from airflow.exceptions import RemovedInAirflow3Warning from airflow.models import errors from airflow.models.dagrun import DagRun @@ -152,16 +153,16 @@ def get_mapped_summary(parent_instance, task_instances): def get_dag_run_conf( dag_run_conf: Any, *, json_encoder: type[json.JSONEncoder] = json.JSONEncoder ) -> tuple[str | None, bool]: - conf: str | None = None + result: str | None = None conf_is_json: bool = False if isinstance(dag_run_conf, str): - conf = dag_run_conf + result = dag_run_conf elif isinstance(dag_run_conf, (dict, list)) and any(dag_run_conf): - conf = json.dumps(dag_run_conf, sort_keys=True, cls=json_encoder, ensure_ascii=False) + result = json.dumps(dag_run_conf, sort_keys=True, cls=json_encoder, ensure_ascii=False) conf_is_json = True - return conf, conf_is_json + return result, conf_is_json def encode_dag_run( @@ -170,7 +171,7 @@ def encode_dag_run( if not dag_run: return None - conf, conf_is_json = get_dag_run_conf(dag_run.conf, json_encoder=json_encoder) + dag_run_conf, conf_is_json = get_dag_run_conf(dag_run.conf, json_encoder=json_encoder) return { "run_id": dag_run.run_id, @@ -184,7 +185,7 @@ def encode_dag_run( "run_type": dag_run.run_type, "last_scheduling_decision": datetime_to_string(dag_run.last_scheduling_decision), "external_trigger": dag_run.external_trigger, - "conf": conf, + "conf": dag_run_conf, "conf_is_json": conf_is_json, "note": dag_run.note, } @@ -611,7 +612,7 @@ def json_render(obj, lexer): def wrapped_markdown(s, css_class="rich_doc"): """Convert a Markdown string to HTML.""" - md = MarkdownIt("gfm-like") + md = MarkdownIt("gfm-like", {"html": conf.getboolean("webserver", "allow_html_in_dag_docs")}) if s is None: return None s = textwrap.dedent(s) diff --git a/airflow/www/views.py b/airflow/www/views.py index 5117e7ecdaa53..19fc3d776a7ed 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -1952,6 +1952,7 @@ def trigger(self, dag_id: str, session: Session = NEW_SESSION): # Prepare form fields with param struct details to render a proper form with schema information form_fields = {} + allow_html_in_dag_docs = conf.getboolean("webserver", "allow_html_in_dag_docs") form_trust_problems = [] for k, v in dag.params.items(): form_fields[k] = v.dump() @@ -1971,25 +1972,26 @@ def trigger(self, dag_id: str, session: Session = NEW_SESSION): elif isinstance(form_field_value, dict): form_field_schema["type"] = ["object", "null"] # Mark HTML fields as safe if allowed - trust_level = conf.get("webserver", "trigger_form_param_html_trust_level") - if trust_level == "FullTrust": + if allow_html_in_dag_docs: if "description_html" in form_field_schema: form_field["description"] = Markup(form_field_schema["description_html"]) if "custom_html_form" in form_field_schema: form_field_schema["custom_html_form"] = Markup(form_field_schema["custom_html_form"]) else: - if "description_html" in form_field_schema: + if "description_html" in form_field_schema and "description_md" not in form_field_schema: form_trust_problems.append(f"Field {k} uses HTML description") form_field["description"] = form_field_schema.pop("description_html") if "custom_html_form" in form_field_schema: form_trust_problems.append(f"Field {k} uses custom HTML form definition") form_field_schema.pop("custom_html_form") + if "description_md" in form_field_schema: + form_field["description"] = wwwutils.wrapped_markdown(form_field_schema["description_md"]) if form_trust_problems: flash( Markup( f"At least one field in trigger form uses custom HTML form definition. This is not allowed per " - "configured trust level. Change trigger_form_param_html_trust_level to enable HTML." - "Using plain text as fallback for these fields." + "configuration for security. Change allow_html_in_dag_docs to enable HTML. " + "Using plain text as fallback for these fields. " f"
  • {'
  • '.join(form_trust_problems)}
" ), "warning", diff --git a/docs/apache-airflow/core-concepts/params.rst b/docs/apache-airflow/core-concepts/params.rst index a9a3579b050f3..c927899b001e9 100644 --- a/docs/apache-airflow/core-concepts/params.rst +++ b/docs/apache-airflow/core-concepts/params.rst @@ -191,9 +191,8 @@ The following features are supported in the Trigger UI Form: - The :class:`~airflow.models.param.Param` attribute ``title`` is used to render the form field label of the entry box. If no ``title`` is defined the parameter name/key is used instead. - The :class:`~airflow.models.param.Param` attribute ``description`` is rendered below an entry field as help text in gray color. - If you want to provide HTML tags for special formatting or links you need to use the Param attribute - ``description_html`` and adjust the webserver configuration ``trigger_form_param_html_trust_level`` allowing raw HTML, - see tutorial DAG ``example_params_ui_tutorial`` for an example. + If you want to provide special formatting or links you need to use the Param attribute + ``description_md``. See tutorial DAG ``example_params_ui_tutorial`` for an example. - The :class:`~airflow.models.param.Param` attribute ``type`` influences how a field is rendered. The following types are supported: .. list-table:: @@ -316,8 +315,6 @@ The following features are supported in the Trigger UI Form: The ``const`` value must match the default value to pass `JSON Schema validation `_. - On the bottom of the form the generated JSON configuration can be expanded. If you want to change values manually, the JSON configuration can be adjusted. Changes are overridden when form fields change. -- If you want to render custom HTML as form on top of the provided features, you can use the ``custom_html_form`` attribute it the - webserver configuration ``trigger_form_param_html_trust_level`` allowing raw HTML. .. note:: If the field is required the default value must be valid according to the schema as well. If the DAG is defined with @@ -336,8 +333,12 @@ The trigger form can also be forced to be displayed also if no params are define Per default custom HTML is not allowed to prevent injection of scripts or other maliceus HTML code. If you trust your DAG authors you can change the trust level of parameter descriptions to allow raw HTML by setting the configuration entry -``webserver.trigger_form_param_html_trust_level`` to ``FullTrust``. With the default of ``None`` all HTML will be displayed as -plain text. In future maybe more HTML filtering options might be added. +``webserver.allow_html_in_dag_docs`` to ``True``. With the default setting all HTML will be displayed as plain text. +This relates to the previous feature to enable rich formatting with the attribute ``description_html`` which is now super-seeded +with the attribute ``description_md``. +Custom form elements using the attribute ``custom_html_form`` are allowing a DAG author to specify raw HTML form templates. These +custom HTML form elements are with version 2.8.0 deprecated and will be replaced with a newer and more secure option in a future +release. Disabling Runtime Param Modification ------------------------------------ diff --git a/newsfragments/tbd.significant.rst b/newsfragments/tbd.significant.rst index dfc8a06ef96a3..5407569d273e2 100644 --- a/newsfragments/tbd.significant.rst +++ b/newsfragments/tbd.significant.rst @@ -1,6 +1,10 @@ -Per default raw HTML code in parameter descriptions is disabled with 2.8.0. +Per default raw HTML code in DAG docs and DAG params descriptions is disabled with 2.8.0. -To ensure that no maliceus javascript can be injected with Trigger UI form by DAG authors -a new parameter ``webserver.trigger_form_param_html_trust_level`` was added with default value of ``None``. -If you trust your DAG authors code and want to allow using raw HTML in DAG params and restore the previous -behavior you must set the configuration value to ``FullTrust``. +To ensure that no maliceus javascript can be injected with DAG descriptions or trigger UI forms by DAG authors +a new parameter ``webserver.allow_html_in_dag_docs`` was added with default value of ``False``. +If you trust your DAG authors code and want to allow using raw HTML in DAG descriptions and params and restore the previous +behavior you must set the configuration value to ``True``. + +To ensure Airflow is secure by default, the raw HTML support in trigger UI has been super-seeded by markdown support via +the ``description_md`` attribute. If you have been using ``description_html`` please migrate to ``escription_md``. +The ``custom_html_form`` is now deprecated and will be replaced by a more secure feature in a future release. From e2d025148672fbcf6219e812dc9ff8930cfd0c89 Mon Sep 17 00:00:00 2001 From: Jens Scheffler Date: Sun, 5 Nov 2023 17:38:08 +0100 Subject: [PATCH 04/13] Fix pytests for allow HTML parameters --- tests/www/test_utils.py | 28 ++++++++++++----- tests/www/views/test_views_trigger_dag.py | 37 ++++++++++++++--------- 2 files changed, 43 insertions(+), 22 deletions(-) diff --git a/tests/www/test_utils.py b/tests/www/test_utils.py index 19941c15e0d2d..488eecf7e5dad 100644 --- a/tests/www/test_utils.py +++ b/tests/www/test_utils.py @@ -32,6 +32,7 @@ from airflow.utils import json as utils_json from airflow.www import utils from airflow.www.utils import DagRunCustomSQLAInterface, json_f, wrapped_markdown +from tests.test_utils.config import conf_vars class TestUtils: @@ -386,8 +387,9 @@ def test_wrapped_markdown_with_nested_list(self): ) def test_wrapped_markdown_with_collapsible_section(self): - rendered = wrapped_markdown( - """ + with conf_vars({("webserver", "allow_html_in_dag_docs"): "true"}): + rendered = wrapped_markdown( + """ # A collapsible section with markdown
Click to expand! @@ -399,10 +401,10 @@ def test_wrapped_markdown_with_collapsible_section(self): * Sub bullets
""" - ) + ) - assert ( - """

A collapsible section with markdown

+ assert ( + """

A collapsible section with markdown

Click to expand!

Heading

@@ -417,8 +419,20 @@ def test_wrapped_markdown_with_collapsible_section(self):
""" - == rendered - ) + == rendered + ) + + @pytest.mark.parametrize("allow_html", [False, True]) + def test_wrapped_markdown_with_raw_html(self, allow_html): + with conf_vars({("webserver", "allow_html_in_dag_docs"): str(allow_html)}): + HTML = "test raw HTML" + rendered = wrapped_markdown(HTML) + if allow_html: + assert HTML in rendered + else: + from markupsafe import escape + + assert escape(HTML) in rendered @pytest.mark.db_test diff --git a/tests/www/views/test_views_trigger_dag.py b/tests/www/views/test_views_trigger_dag.py index 4f1f96870ef51..212fc0908adfb 100644 --- a/tests/www/views/test_views_trigger_dag.py +++ b/tests/www/views/test_views_trigger_dag.py @@ -236,43 +236,50 @@ def test_trigger_dag_params_render(admin_client, dag_maker, session, app, monkey ) -@pytest.mark.parametrize( - "trust, expect_escape", - [ - ["None", True], - ["FullTrust", False], - ], -) -def test_trigger_dag_html_trust(admin_client, dag_maker, session, app, monkeypatch, trust, expect_escape): +@pytest.mark.parametrize("allow_html", [False, True]) +def test_trigger_dag_html_allow(admin_client, dag_maker, session, app, monkeypatch, allow_html): """ Test that HTML is masked per default in description. """ from markupsafe import escape - with conf_vars({("webserver", "trigger_form_param_html_trust_level"): trust}): - DAG_ID = "params_dag" - HTML_DESCRIPTION = "HTML raw code." + DAG_ID = "params_dag" + HTML_DESCRIPTION1 = "HTML raw code." + HTML_DESCRIPTION2 = "HTML in md text." + expect_escape = not allow_html + with conf_vars({("webserver", "allow_html_in_dag_docs"): str(allow_html)}): param1 = Param( 42, - description_html=HTML_DESCRIPTION, + description_html=HTML_DESCRIPTION1, + type="integer", + minimum=1, + maximum=100, + ) + param2 = Param( + 42, + description_md=HTML_DESCRIPTION2, type="integer", minimum=1, maximum=100, ) with monkeypatch.context() as m: - with dag_maker(dag_id=DAG_ID, serialized=True, session=session, params={"param1": param1}): + with dag_maker( + dag_id=DAG_ID, serialized=True, session=session, params={"param1": param1, "param2": param2} + ): EmptyOperator(task_id="task1") m.setattr(app, "dag_bag", dag_maker.dagbag) resp = admin_client.get(f"dags/{DAG_ID}/trigger") if expect_escape: - check_content_in_response(escape(HTML_DESCRIPTION), resp) + check_content_in_response(escape(HTML_DESCRIPTION1), resp) + check_content_in_response(escape(HTML_DESCRIPTION2), resp) check_content_in_response( "At least one field in trigger form uses custom HTML form definition.", resp ) else: - check_content_in_response(HTML_DESCRIPTION, resp) + check_content_in_response(HTML_DESCRIPTION1, resp) + check_content_in_response(HTML_DESCRIPTION2, resp) check_content_not_in_response( "At least one field in trigger form uses custom HTML form definition.", resp ) From 13004519b61392171ff3c77412dfdecdbe057cdb Mon Sep 17 00:00:00 2001 From: Jens Scheffler Date: Sun, 5 Nov 2023 18:49:34 +0100 Subject: [PATCH 05/13] Move newsfragment to PR number --- newsfragments/{tbd.significant.rst => 35460.significant.rst} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename newsfragments/{tbd.significant.rst => 35460.significant.rst} (100%) diff --git a/newsfragments/tbd.significant.rst b/newsfragments/35460.significant.rst similarity index 100% rename from newsfragments/tbd.significant.rst rename to newsfragments/35460.significant.rst From 7882fa44179e866d934a466c80ec1dab6690e5f7 Mon Sep 17 00:00:00 2001 From: Jens Scheffler Date: Sun, 5 Nov 2023 22:41:05 +0100 Subject: [PATCH 06/13] Review feedback --- airflow/www/views.py | 5 +++-- docs/apache-airflow/core-concepts/params.rst | 2 +- newsfragments/35460.significant.rst | 4 ++-- tests/www/views/test_views_trigger_dag.py | 2 +- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/airflow/www/views.py b/airflow/www/views.py index 19fc3d776a7ed..2d95685de6e3f 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -1990,8 +1990,9 @@ def trigger(self, dag_id: str, session: Session = NEW_SESSION): flash( Markup( f"At least one field in trigger form uses custom HTML form definition. This is not allowed per " - "configuration for security. Change allow_html_in_dag_docs to enable HTML. " - "Using plain text as fallback for these fields. " + "configuration for security. Switch to markdown description via description_md " + "or ask your deployment manager to change webserver.allow_html_in_dag_docs " + "configuration parameter to enable HTML. Using plain text as fallback for these fields. " f"
  • {'
  • '.join(form_trust_problems)}
" ), "warning", diff --git a/docs/apache-airflow/core-concepts/params.rst b/docs/apache-airflow/core-concepts/params.rst index c927899b001e9..e5efc33b72f85 100644 --- a/docs/apache-airflow/core-concepts/params.rst +++ b/docs/apache-airflow/core-concepts/params.rst @@ -331,7 +331,7 @@ The trigger form can also be forced to be displayed also if no params are define .. versionadded:: 2.8.0 -Per default custom HTML is not allowed to prevent injection of scripts or other maliceus HTML code. If you trust your DAG authors +Per default custom HTML is not allowed to prevent injection of scripts or other malicious HTML code. If you trust your DAG authors you can change the trust level of parameter descriptions to allow raw HTML by setting the configuration entry ``webserver.allow_html_in_dag_docs`` to ``True``. With the default setting all HTML will be displayed as plain text. This relates to the previous feature to enable rich formatting with the attribute ``description_html`` which is now super-seeded diff --git a/newsfragments/35460.significant.rst b/newsfragments/35460.significant.rst index 5407569d273e2..eadcf08f6952a 100644 --- a/newsfragments/35460.significant.rst +++ b/newsfragments/35460.significant.rst @@ -1,10 +1,10 @@ Per default raw HTML code in DAG docs and DAG params descriptions is disabled with 2.8.0. -To ensure that no maliceus javascript can be injected with DAG descriptions or trigger UI forms by DAG authors +To ensure that no malicious javascript can be injected with DAG descriptions or trigger UI forms by DAG authors a new parameter ``webserver.allow_html_in_dag_docs`` was added with default value of ``False``. If you trust your DAG authors code and want to allow using raw HTML in DAG descriptions and params and restore the previous behavior you must set the configuration value to ``True``. To ensure Airflow is secure by default, the raw HTML support in trigger UI has been super-seeded by markdown support via -the ``description_md`` attribute. If you have been using ``description_html`` please migrate to ``escription_md``. +the ``description_md`` attribute. If you have been using ``description_html`` please migrate to ``description_md``. The ``custom_html_form`` is now deprecated and will be replaced by a more secure feature in a future release. diff --git a/tests/www/views/test_views_trigger_dag.py b/tests/www/views/test_views_trigger_dag.py index 212fc0908adfb..adc109510e75b 100644 --- a/tests/www/views/test_views_trigger_dag.py +++ b/tests/www/views/test_views_trigger_dag.py @@ -239,7 +239,7 @@ def test_trigger_dag_params_render(admin_client, dag_maker, session, app, monkey @pytest.mark.parametrize("allow_html", [False, True]) def test_trigger_dag_html_allow(admin_client, dag_maker, session, app, monkeypatch, allow_html): """ - Test that HTML is masked per default in description. + Test that HTML is escaped per default in description. """ from markupsafe import escape From 81d13643731553963dbb7719b225776ce2f174f7 Mon Sep 17 00:00:00 2001 From: Jens Scheffler Date: Mon, 6 Nov 2023 19:49:11 +0100 Subject: [PATCH 07/13] Review feedback 2 --- .../example_params_ui_tutorial.py | 2 +- docs/apache-airflow/core-concepts/params.rst | 26 +++++++++---------- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/airflow/example_dags/example_params_ui_tutorial.py b/airflow/example_dags/example_params_ui_tutorial.py index 1ca607a6e3f9b..489a4681c91b7 100644 --- a/airflow/example_dags/example_params_ui_tutorial.py +++ b/airflow/example_dags/example_params_ui_tutorial.py @@ -47,7 +47,7 @@ "flag": False, "a_simple_list": ["one", "two", "three", "actually one value is made per line"], # But of course you might want to have it nicer! Let's add some description to parameters. - # Note if you can add any MD formatting to the description, you need to use the description_md + # Note if you can add any Markdown formatting to the description, you need to use the description_md # attribute. "most_loved_number": Param( 42, diff --git a/docs/apache-airflow/core-concepts/params.rst b/docs/apache-airflow/core-concepts/params.rst index e5efc33b72f85..8141c01b65c16 100644 --- a/docs/apache-airflow/core-concepts/params.rst +++ b/docs/apache-airflow/core-concepts/params.rst @@ -325,20 +325,18 @@ For examples also please take a look to two example DAGs provided: ``example_par .. image:: ../img/trigger-dag-tutorial-form.png .. versionadded:: 2.7.0 - -The trigger form can also be forced to be displayed also if no params are defined using the configuration switch -``webserver.show_trigger_form_if_no_params``. - -.. versionadded:: 2.8.0 - -Per default custom HTML is not allowed to prevent injection of scripts or other malicious HTML code. If you trust your DAG authors -you can change the trust level of parameter descriptions to allow raw HTML by setting the configuration entry -``webserver.allow_html_in_dag_docs`` to ``True``. With the default setting all HTML will be displayed as plain text. -This relates to the previous feature to enable rich formatting with the attribute ``description_html`` which is now super-seeded -with the attribute ``description_md``. -Custom form elements using the attribute ``custom_html_form`` are allowing a DAG author to specify raw HTML form templates. These -custom HTML form elements are with version 2.8.0 deprecated and will be replaced with a newer and more secure option in a future -release. + The trigger form can also be forced to be displayed also if no params are defined using the configuration switch + ``webserver.show_trigger_form_if_no_params``. + +.. versionchanged:: 2.8.0 + Per default custom HTML is not allowed to prevent injection of scripts or other malicious HTML code. If you trust your DAG authors + you can change the trust level of parameter descriptions to allow raw HTML by setting the configuration entry + ``webserver.allow_html_in_dag_docs`` to ``True``. With the default setting all HTML will be displayed as plain text. + This relates to the previous feature to enable rich formatting with the attribute ``description_html`` which is now super-seeded + with the attribute ``description_md``. + Custom form elements using the attribute ``custom_html_form`` are allowing a DAG author to specify raw HTML form templates. These + custom HTML form elements are with version 2.8.0 deprecated and will be replaced with a newer and more secure option in a future + release. Disabling Runtime Param Modification ------------------------------------ From c5bee252d6a01fd84158115abe2034d3e7e8e1d9 Mon Sep 17 00:00:00 2001 From: Jens Scheffler Date: Wed, 8 Nov 2023 23:45:27 +0100 Subject: [PATCH 08/13] Add more deprecation warnings for HTML --- airflow/www/views.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/airflow/www/views.py b/airflow/www/views.py index 2d95685de6e3f..fdebb19bac40b 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -1997,6 +1997,22 @@ def trigger(self, dag_id: str, session: Session = NEW_SESSION): ), "warning", ) + if allow_html_in_dag_docs and any("description_html" in p.schema for p in dag.params.values()): + flash( + Markup( + "The form params use raw HTML in description_html which is deprecated. " + "Please migrate to description_md." + ), + "warning", + ) + if allow_html_in_dag_docs and any("custom_html_form" in p.schema for p in dag.params.values()): + flash( + Markup( + "The form params use custom_html_form definition. " + "This is deprecated with Airflow 2.8.0 and will be removed in a next release." + ), + "warning", + ) ui_fields_defined = any("const" not in f["schema"] for f in form_fields.values()) show_trigger_form_if_no_params = conf.getboolean("webserver", "show_trigger_form_if_no_params") From 90c5719c8cf8ccd375926fafef936c16ca218509 Mon Sep 17 00:00:00 2001 From: Jens Scheffler Date: Wed, 8 Nov 2023 23:45:48 +0100 Subject: [PATCH 09/13] Review feedback - wording --- docs/apache-airflow/core-concepts/params.rst | 7 +++---- newsfragments/35460.significant.rst | 8 ++++---- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/docs/apache-airflow/core-concepts/params.rst b/docs/apache-airflow/core-concepts/params.rst index 8141c01b65c16..1d2806ca13996 100644 --- a/docs/apache-airflow/core-concepts/params.rst +++ b/docs/apache-airflow/core-concepts/params.rst @@ -329,14 +329,13 @@ For examples also please take a look to two example DAGs provided: ``example_par ``webserver.show_trigger_form_if_no_params``. .. versionchanged:: 2.8.0 - Per default custom HTML is not allowed to prevent injection of scripts or other malicious HTML code. If you trust your DAG authors + By default custom HTML is not allowed to prevent injection of scripts or other malicious HTML code. If you trust your DAG authors you can change the trust level of parameter descriptions to allow raw HTML by setting the configuration entry ``webserver.allow_html_in_dag_docs`` to ``True``. With the default setting all HTML will be displayed as plain text. This relates to the previous feature to enable rich formatting with the attribute ``description_html`` which is now super-seeded with the attribute ``description_md``. - Custom form elements using the attribute ``custom_html_form`` are allowing a DAG author to specify raw HTML form templates. These - custom HTML form elements are with version 2.8.0 deprecated and will be replaced with a newer and more secure option in a future - release. + Custom form elements using the attribute ``custom_html_form`` allow a DAG author to specify raw HTML form templates. These + custom HTML form elements are deprecated as of version 2.8.0. Disabling Runtime Param Modification ------------------------------------ diff --git a/newsfragments/35460.significant.rst b/newsfragments/35460.significant.rst index eadcf08f6952a..412c59595212f 100644 --- a/newsfragments/35460.significant.rst +++ b/newsfragments/35460.significant.rst @@ -1,10 +1,10 @@ -Per default raw HTML code in DAG docs and DAG params descriptions is disabled with 2.8.0. +Raw HTML code in DAG docs and DAG params descriptions is disabled by default To ensure that no malicious javascript can be injected with DAG descriptions or trigger UI forms by DAG authors a new parameter ``webserver.allow_html_in_dag_docs`` was added with default value of ``False``. -If you trust your DAG authors code and want to allow using raw HTML in DAG descriptions and params and restore the previous -behavior you must set the configuration value to ``True``. +If you trust your DAG authors code and want to allow using raw HTML in DAG descriptions and params, you can restore the previous +behavior by setting the configuration value to ``True``. To ensure Airflow is secure by default, the raw HTML support in trigger UI has been super-seeded by markdown support via the ``description_md`` attribute. If you have been using ``description_html`` please migrate to ``description_md``. -The ``custom_html_form`` is now deprecated and will be replaced by a more secure feature in a future release. +The ``custom_html_form`` is now deprecated. From 6b6a3bc8d631ce2b3a765d7daab4a00011617c79 Mon Sep 17 00:00:00 2001 From: Jens Scheffler Date: Wed, 8 Nov 2023 23:48:29 +0100 Subject: [PATCH 10/13] Review feedback - rename allow_html_in_dag_docs to allow_raw_html_descriptions --- airflow/config_templates/config.yml | 2 +- airflow/www/utils.py | 2 +- airflow/www/views.py | 10 +++++----- docs/apache-airflow/core-concepts/params.rst | 2 +- newsfragments/35460.significant.rst | 2 +- tests/www/test_utils.py | 4 ++-- tests/www/views/test_views_trigger_dag.py | 2 +- 7 files changed, 12 insertions(+), 12 deletions(-) diff --git a/airflow/config_templates/config.yml b/airflow/config_templates/config.yml index 3bec46c184788..9905d46c51a81 100644 --- a/airflow/config_templates/config.yml +++ b/airflow/config_templates/config.yml @@ -1821,7 +1821,7 @@ webserver: type: boolean example: ~ default: "False" - allow_html_in_dag_docs: + allow_raw_html_descriptions: description: | A DAG author is able to provide any raw HTML into ``doc_md`` or params description for text formatting. This is including potentially unsafe javascript. Displaying the DAG or trigger diff --git a/airflow/www/utils.py b/airflow/www/utils.py index 390b747bd71ff..e8837b86f6dcd 100644 --- a/airflow/www/utils.py +++ b/airflow/www/utils.py @@ -612,7 +612,7 @@ def json_render(obj, lexer): def wrapped_markdown(s, css_class="rich_doc"): """Convert a Markdown string to HTML.""" - md = MarkdownIt("gfm-like", {"html": conf.getboolean("webserver", "allow_html_in_dag_docs")}) + md = MarkdownIt("gfm-like", {"html": conf.getboolean("webserver", "allow_raw_html_descriptions")}) if s is None: return None s = textwrap.dedent(s) diff --git a/airflow/www/views.py b/airflow/www/views.py index fdebb19bac40b..bf43127d41653 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -1952,7 +1952,7 @@ def trigger(self, dag_id: str, session: Session = NEW_SESSION): # Prepare form fields with param struct details to render a proper form with schema information form_fields = {} - allow_html_in_dag_docs = conf.getboolean("webserver", "allow_html_in_dag_docs") + allow_raw_html_descriptions = conf.getboolean("webserver", "allow_raw_html_descriptions") form_trust_problems = [] for k, v in dag.params.items(): form_fields[k] = v.dump() @@ -1972,7 +1972,7 @@ def trigger(self, dag_id: str, session: Session = NEW_SESSION): elif isinstance(form_field_value, dict): form_field_schema["type"] = ["object", "null"] # Mark HTML fields as safe if allowed - if allow_html_in_dag_docs: + if allow_raw_html_descriptions: if "description_html" in form_field_schema: form_field["description"] = Markup(form_field_schema["description_html"]) if "custom_html_form" in form_field_schema: @@ -1991,13 +1991,13 @@ def trigger(self, dag_id: str, session: Session = NEW_SESSION): Markup( f"At least one field in trigger form uses custom HTML form definition. This is not allowed per " "configuration for security. Switch to markdown description via description_md " - "or ask your deployment manager to change webserver.allow_html_in_dag_docs " + "or ask your deployment manager to change webserver.allow_raw_html_descriptions " "configuration parameter to enable HTML. Using plain text as fallback for these fields. " f"
  • {'
  • '.join(form_trust_problems)}
" ), "warning", ) - if allow_html_in_dag_docs and any("description_html" in p.schema for p in dag.params.values()): + if allow_raw_html_descriptions and any("description_html" in p.schema for p in dag.params.values()): flash( Markup( "The form params use raw HTML in description_html which is deprecated. " @@ -2005,7 +2005,7 @@ def trigger(self, dag_id: str, session: Session = NEW_SESSION): ), "warning", ) - if allow_html_in_dag_docs and any("custom_html_form" in p.schema for p in dag.params.values()): + if allow_raw_html_descriptions and any("custom_html_form" in p.schema for p in dag.params.values()): flash( Markup( "The form params use custom_html_form definition. " diff --git a/docs/apache-airflow/core-concepts/params.rst b/docs/apache-airflow/core-concepts/params.rst index 1d2806ca13996..72eb058d4b74c 100644 --- a/docs/apache-airflow/core-concepts/params.rst +++ b/docs/apache-airflow/core-concepts/params.rst @@ -331,7 +331,7 @@ For examples also please take a look to two example DAGs provided: ``example_par .. versionchanged:: 2.8.0 By default custom HTML is not allowed to prevent injection of scripts or other malicious HTML code. If you trust your DAG authors you can change the trust level of parameter descriptions to allow raw HTML by setting the configuration entry - ``webserver.allow_html_in_dag_docs`` to ``True``. With the default setting all HTML will be displayed as plain text. + ``webserver.allow_raw_html_descriptions`` to ``True``. With the default setting all HTML will be displayed as plain text. This relates to the previous feature to enable rich formatting with the attribute ``description_html`` which is now super-seeded with the attribute ``description_md``. Custom form elements using the attribute ``custom_html_form`` allow a DAG author to specify raw HTML form templates. These diff --git a/newsfragments/35460.significant.rst b/newsfragments/35460.significant.rst index 412c59595212f..d29481d219a6f 100644 --- a/newsfragments/35460.significant.rst +++ b/newsfragments/35460.significant.rst @@ -1,7 +1,7 @@ Raw HTML code in DAG docs and DAG params descriptions is disabled by default To ensure that no malicious javascript can be injected with DAG descriptions or trigger UI forms by DAG authors -a new parameter ``webserver.allow_html_in_dag_docs`` was added with default value of ``False``. +a new parameter ``webserver.allow_raw_html_descriptions`` was added with default value of ``False``. If you trust your DAG authors code and want to allow using raw HTML in DAG descriptions and params, you can restore the previous behavior by setting the configuration value to ``True``. diff --git a/tests/www/test_utils.py b/tests/www/test_utils.py index 488eecf7e5dad..dfd8b563dc415 100644 --- a/tests/www/test_utils.py +++ b/tests/www/test_utils.py @@ -387,7 +387,7 @@ def test_wrapped_markdown_with_nested_list(self): ) def test_wrapped_markdown_with_collapsible_section(self): - with conf_vars({("webserver", "allow_html_in_dag_docs"): "true"}): + with conf_vars({("webserver", "allow_raw_html_descriptions"): "true"}): rendered = wrapped_markdown( """ # A collapsible section with markdown @@ -424,7 +424,7 @@ def test_wrapped_markdown_with_collapsible_section(self): @pytest.mark.parametrize("allow_html", [False, True]) def test_wrapped_markdown_with_raw_html(self, allow_html): - with conf_vars({("webserver", "allow_html_in_dag_docs"): str(allow_html)}): + with conf_vars({("webserver", "allow_raw_html_descriptions"): str(allow_html)}): HTML = "test raw HTML" rendered = wrapped_markdown(HTML) if allow_html: diff --git a/tests/www/views/test_views_trigger_dag.py b/tests/www/views/test_views_trigger_dag.py index adc109510e75b..9428fc09b29eb 100644 --- a/tests/www/views/test_views_trigger_dag.py +++ b/tests/www/views/test_views_trigger_dag.py @@ -247,7 +247,7 @@ def test_trigger_dag_html_allow(admin_client, dag_maker, session, app, monkeypat HTML_DESCRIPTION1 = "HTML raw code." HTML_DESCRIPTION2 = "HTML in md text." expect_escape = not allow_html - with conf_vars({("webserver", "allow_html_in_dag_docs"): str(allow_html)}): + with conf_vars({("webserver", "allow_raw_html_descriptions"): str(allow_html)}): param1 = Param( 42, description_html=HTML_DESCRIPTION1, From 582dce0d05969a6bb702de9b2451aa2078098200 Mon Sep 17 00:00:00 2001 From: Jens Scheffler Date: Thu, 9 Nov 2023 23:07:39 +0100 Subject: [PATCH 11/13] Review wording, round 3 --- airflow/config_templates/config.yml | 14 +++++++++----- airflow/www/views.py | 9 +++++---- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/airflow/config_templates/config.yml b/airflow/config_templates/config.yml index 863bcaa793a01..73c26b89ec219 100644 --- a/airflow/config_templates/config.yml +++ b/airflow/config_templates/config.yml @@ -1830,11 +1830,15 @@ webserver: default: "False" allow_raw_html_descriptions: description: | - A DAG author is able to provide any raw HTML into ``doc_md`` or params description for text - formatting. This is including potentially unsafe javascript. Displaying the DAG or trigger - form in web UI provides the DAG author the potential to inject malicieus code into clients - browsers. To ensure the web UI is safe by default, raw HTML is disabled by default. If you - trust your DAG authors, you can enable HTML support by setting this option to True. + A DAG author is able to provide any raw HTML into ``doc_md`` or params description in + ``description_md`` for text formatting. This is including potentially unsafe javascript. + Displaying the DAG or trigger form in web UI provides the DAG author the potential to + inject malicious code into clients browsers. To ensure the web UI is safe by default, + raw HTML is disabled by default. If you trust your DAG authors, you can enable HTML + support in markdown by setting this option to True. + + This parameter also enables the deprecated fields ``description_html`` and + ``custom_html_form`` in DAG params until the feature is removed in a next version. version_added: 2.8.0 type: boolean example: "False" diff --git a/airflow/www/views.py b/airflow/www/views.py index 9243679d0722d..9b4172570d6f9 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -1993,10 +1993,11 @@ def trigger(self, dag_id: str, session: Session = NEW_SESSION): if form_trust_problems: flash( Markup( - f"At least one field in trigger form uses custom HTML form definition. This is not allowed per " - "configuration for security. Switch to markdown description via description_md " - "or ask your deployment manager to change webserver.allow_raw_html_descriptions " - "configuration parameter to enable HTML. Using plain text as fallback for these fields. " + "At least one field in trigger form uses raw HTML form definition. This is not allowed for " + "security. Please switch to markdown description via description_md. " + "Raw HTML is deprecated and must be enabled via " + "webserver.allow_raw_html_descriptions configuration parameter. Using plain text " + "as fallback for these fields. " f"
  • {'
  • '.join(form_trust_problems)}
" ), "warning", From 13b36cb0df78bc1ecbeb40e8f9dca253fe0002c2 Mon Sep 17 00:00:00 2001 From: Jens Scheffler Date: Sat, 11 Nov 2023 20:03:18 +0100 Subject: [PATCH 12/13] Review wording, round 4 --- airflow/config_templates/config.yml | 2 +- airflow/www/views.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/airflow/config_templates/config.yml b/airflow/config_templates/config.yml index 230f82932224b..ee246e1c0156e 100644 --- a/airflow/config_templates/config.yml +++ b/airflow/config_templates/config.yml @@ -1838,7 +1838,7 @@ webserver: support in markdown by setting this option to True. This parameter also enables the deprecated fields ``description_html`` and - ``custom_html_form`` in DAG params until the feature is removed in a next version. + ``custom_html_form`` in DAG params until the feature is removed in a future version. version_added: 2.8.0 type: boolean example: "False" diff --git a/airflow/www/views.py b/airflow/www/views.py index 9b4172570d6f9..3e4ed75a5953c 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -1993,7 +1993,7 @@ def trigger(self, dag_id: str, session: Session = NEW_SESSION): if form_trust_problems: flash( Markup( - "At least one field in trigger form uses raw HTML form definition. This is not allowed for " + "At least one field in the trigger form uses a raw HTML form definition. This is not allowed for " "security. Please switch to markdown description via description_md. " "Raw HTML is deprecated and must be enabled via " "webserver.allow_raw_html_descriptions configuration parameter. Using plain text " @@ -2014,7 +2014,7 @@ def trigger(self, dag_id: str, session: Session = NEW_SESSION): flash( Markup( "The form params use custom_html_form definition. " - "This is deprecated with Airflow 2.8.0 and will be removed in a next release." + "This is deprecated with Airflow 2.8.0 and will be removed in a future release." ), "warning", ) From 65f8037865f68a2a20154a045a7fc0c86a331fdd Mon Sep 17 00:00:00 2001 From: Jens Scheffler Date: Sun, 12 Nov 2023 12:13:24 +0100 Subject: [PATCH 13/13] Fix pytest --- tests/www/views/test_views_trigger_dag.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/www/views/test_views_trigger_dag.py b/tests/www/views/test_views_trigger_dag.py index 9428fc09b29eb..65ad8734d5140 100644 --- a/tests/www/views/test_views_trigger_dag.py +++ b/tests/www/views/test_views_trigger_dag.py @@ -275,13 +275,13 @@ def test_trigger_dag_html_allow(admin_client, dag_maker, session, app, monkeypat check_content_in_response(escape(HTML_DESCRIPTION1), resp) check_content_in_response(escape(HTML_DESCRIPTION2), resp) check_content_in_response( - "At least one field in trigger form uses custom HTML form definition.", resp + "At least one field in the trigger form uses a raw HTML form definition.", resp ) else: check_content_in_response(HTML_DESCRIPTION1, resp) check_content_in_response(HTML_DESCRIPTION2, resp) check_content_not_in_response( - "At least one field in trigger form uses custom HTML form definition.", resp + "At least one field in the trigger form uses a raw HTML form definition.", resp )