This is an automated email from the ASF dual-hosted git repository.
jscheffl pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new 23e971600a Remove RAW HTML support from Trigger Form UI (#40029)
23e971600a is described below
commit 23e971600a7530bf787eac59e652ccd8062445b0
Author: Jens Scheffler <95105677+jsche...@users.noreply.github.com>
AuthorDate: Wed Aug 21 10:25:32 2024 +0200
Remove RAW HTML support from Trigger Form UI (#40029)
* Remove RAW HTML support from Trigger Form UI
* Updates since main branch changed to 3.0.0
* Force really no HTML in code, fix unit tests
* Fix version in deprecation message
---
airflow/config_templates/config.yml | 15 -------
airflow/www/templates/airflow/trigger.html | 4 +-
airflow/www/utils.py | 3 +-
airflow/www/views.py | 43 --------------------
docs/apache-airflow/core-concepts/params.rst | 12 ++----
newsfragments/40029.significant.rst | 1 +
tests/www/test_utils.py | 59 ++++++----------------------
tests/www/views/test_views_trigger_dag.py | 51 +-----------------------
8 files changed, 20 insertions(+), 168 deletions(-)
diff --git a/airflow/config_templates/config.yml
b/airflow/config_templates/config.yml
index 007ceadd67..621589d2a5 100644
--- a/airflow/config_templates/config.yml
+++ b/airflow/config_templates/config.yml
@@ -2042,21 +2042,6 @@ webserver:
type: integer
example: "10"
default: "5"
- allow_raw_html_descriptions:
- description: |
- 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
future version.
- version_added: 2.8.0
- type: boolean
- example: "False"
- default: "False"
allowed_payload_size:
description: |
The maximum size of the request payload (in MB) that can be sent.
diff --git a/airflow/www/templates/airflow/trigger.html
b/airflow/www/templates/airflow/trigger.html
index eb098dd21d..86e0b4eb45 100644
--- a/airflow/www/templates/airflow/trigger.html
+++ b/airflow/www/templates/airflow/trigger.html
@@ -42,9 +42,7 @@
: </label>
</td>
<td>
- {% if "custom_html_form" in form_details.schema %}
- {{ form_details.schema.custom_html_form | replace("{name}", "element_" +
form_key) | replace("{value}", form_details.value) }}
- {% elif "type" in form_details.schema and form_details.schema.type ==
"boolean" %}
+ {% if "type" in form_details.schema and form_details.schema.type ==
"boolean" %}
<label for="element_{{ form_key }}" control-label="">
<input class="switch-input" name="element_{{ form_key }}"
id="element_{{ form_key }}" type="checkbox"
{%- if form_details.value %} checked="checked"{%- endif -%}/>
diff --git a/airflow/www/utils.py b/airflow/www/utils.py
index 413d9fe2b6..37c5590558 100644
--- a/airflow/www/utils.py
+++ b/airflow/www/utils.py
@@ -39,7 +39,6 @@ from pygments.formatters import HtmlFormatter
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.dagrun import DagRun
from airflow.models.dagwarning import DagWarning
@@ -639,7 +638,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_raw_html_descriptions")})
+ md = MarkdownIt("gfm-like", {"html": False})
if s is None:
return None
s = textwrap.dedent(s)
diff --git a/airflow/www/views.py b/airflow/www/views.py
index d7b670303c..c34554d508 100644
--- a/airflow/www/views.py
+++ b/airflow/www/views.py
@@ -2059,8 +2059,6 @@ class Airflow(AirflowBaseView):
# Prepare form fields with param struct details to render a proper
form with schema information
form_fields = {}
- 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()
form_field: dict = form_fields[k]
@@ -2078,19 +2076,6 @@ class Airflow(AirflowBaseView):
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
- 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:
- form_field_schema["custom_html_form"] =
Markup(form_field_schema["custom_html_form"])
- else:
- 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"])
# Check for default values and pre-populate
@@ -2110,34 +2095,6 @@ class Airflow(AirflowBaseView):
)
else:
form_field["value"] = request.values.get(k)
- if form_trust_problems:
- flash(
- Markup(
- "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
<code>description_md</code>. "
- "Raw HTML is deprecated and must be enabled via "
- "<code>webserver.allow_raw_html_descriptions</code>
configuration parameter. Using plain text "
- "as fallback for these fields. "
-
f"<ul><li>{'</li><li>'.join(form_trust_problems)}</li></ul>"
- ),
- "warning",
- )
- 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
<code>description_html</code> which is deprecated. "
- "Please migrate to <code>description_md</code>."
- ),
- "warning",
- )
- 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 <code>custom_html_form</code>
definition. "
- "This is deprecated with Airflow 2.8.0 and will be removed
in a future 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")
diff --git a/docs/apache-airflow/core-concepts/params.rst
b/docs/apache-airflow/core-concepts/params.rst
index a4efceb366..9de7e0df0e 100644
--- a/docs/apache-airflow/core-concepts/params.rst
+++ b/docs/apache-airflow/core-concepts/params.rst
@@ -388,14 +388,10 @@ For examples, please take a look at the two example DAGs
provided: :ref:`Params
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
- 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_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
- custom HTML form elements are deprecated as of version 2.8.0.
+.. versionchanged:: 3.0.0
+ By default custom HTML is not allowed to prevent injection of scripts or
other malicious HTML code. The previous field named
+ ``description_html`` is now super-seeded with the attribute
``description_md``. ``description_html`` is not supported anymore.
+ Custom form elements using the attribute ``custom_html_form`` was
deprecated in version 2.8.0 and support was removed in 3.0.0.
Disabling Runtime Param Modification
------------------------------------
diff --git a/newsfragments/40029.significant.rst
b/newsfragments/40029.significant.rst
new file mode 100644
index 0000000000..e64f5170ef
--- /dev/null
+++ b/newsfragments/40029.significant.rst
@@ -0,0 +1 @@
+Removed deprecated ``allow_raw_html_descriptions`` option from UI Trigger
forms.
diff --git a/tests/www/test_utils.py b/tests/www/test_utils.py
index 669f08a2b6..f7470c8c68 100644
--- a/tests/www/test_utils.py
+++ b/tests/www/test_utils.py
@@ -45,7 +45,6 @@ from airflow.www.utils import (
wrapped_markdown,
)
from airflow.www.widgets import AirflowDateTimePickerROWidget,
BS3TextAreaROWidget, BS3TextFieldROWidget
-from tests.test_utils.config import conf_vars
class TestUtils:
@@ -504,53 +503,19 @@ class TestWrappedMarkdown:
== rendered
)
- def test_wrapped_markdown_with_collapsible_section(self):
- with conf_vars({("webserver", "allow_raw_html_descriptions"): "true"}):
- rendered = wrapped_markdown(
- """
-# A collapsible section with markdown
-<details>
- <summary>Click to expand!</summary>
-
- ## Heading
- 1. A numbered
- 2. list
- * With some
- * Sub bullets
-</details>
- """
- )
-
- assert (
- """<div class="rich_doc" ><h1>A collapsible section with
markdown</h1>
-<details>
- <summary>Click to expand!</summary>
-<h2>Heading</h2>
-<ol>
-<li>A numbered</li>
-<li>list
-<ul>
-<li>With some</li>
-<li>Sub bullets</li>
-</ul>
-</li>
-</ol>
-</details>
-</div>"""
- == rendered
- )
-
- @pytest.mark.parametrize("allow_html", [False, True])
- def test_wrapped_markdown_with_raw_html(self, allow_html):
- with conf_vars({("webserver", "allow_raw_html_descriptions"):
str(allow_html)}):
- HTML = "test <code>raw HTML</code>"
- rendered = wrapped_markdown(HTML)
- if allow_html:
- assert HTML in rendered
- else:
- from markupsafe import escape
+ @pytest.mark.parametrize(
+ "html",
+ [
+ "test <code>raw HTML</code>",
+ "hidden <script>alert(1)</script> nuggets.",
+ ],
+ )
+ def test_wrapped_markdown_with_raw_html(self, html):
+ """Ensure that HTML code is not ending-up in markdown but is always
escaped."""
+ from markupsafe import escape
- assert escape(HTML) in rendered
+ rendered = wrapped_markdown(html)
+ assert escape(html) in rendered
@pytest.mark.parametrize(
"dag_run,expected_val",
diff --git a/tests/www/views/test_views_trigger_dag.py
b/tests/www/views/test_views_trigger_dag.py
index 9b2b297198..01b6713600 100644
--- a/tests/www/views/test_views_trigger_dag.py
+++ b/tests/www/views/test_views_trigger_dag.py
@@ -34,7 +34,7 @@ from airflow.utils.session import create_session
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,
check_content_not_in_response
+from tests.test_utils.www import check_content_in_response
pytestmark = pytest.mark.db_test
@@ -273,55 +273,6 @@ 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 escaped per default in description.
- """
- from markupsafe import escape
-
- DAG_ID = "params_dag"
- HTML_DESCRIPTION1 = "HTML <code>raw code</code>."
- HTML_DESCRIPTION2 = "HTML <code>in md text</code>."
- expect_escape = not allow_html
- with conf_vars({("webserver", "allow_raw_html_descriptions"):
str(allow_html)}):
- param1 = Param(
- 42,
- 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, "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_DESCRIPTION1), resp)
- check_content_in_response(escape(HTML_DESCRIPTION2), resp)
- check_content_in_response(
- "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 the trigger form uses a raw 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
