jedcunningham commented on code in PR #35460:
URL: https://github.com/apache/airflow/pull/35460#discussion_r1385810730
##########
docs/apache-airflow/core-concepts/params.rst:
##########
@@ -324,9 +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``.
+ 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
Review Comment:
```suggestion
By default custom HTML is not allowed to prevent injection of scripts or
other malicious HTML code. If you trust your DAG authors
```
##########
newsfragments/35460.significant.rst:
##########
@@ -0,0 +1,10 @@
+Per default raw HTML code in DAG docs and DAG params descriptions is disabled
with 2.8.0.
Review Comment:
```suggestion
Raw HTML code in DAG docs and DAG params descriptions is disabled by default
```
No need to duplicate the version, it's in the 2.8.0 release section.
##########
airflow/www/views.py:
##########
@@ -1952,30 +1952,52 @@ 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()
+ 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
+ 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 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:
Review Comment:
Are we missing a deprecation warning when folks do use this?
##########
airflow/config_templates/config.yml:
##########
@@ -1821,6 +1821,17 @@ webserver:
type: boolean
example: ~
default: "False"
+ allow_html_in_dag_docs:
Review Comment:
Not sure about the name of this. It implies its only dag docs, but it covers
more than that. Maybe `allow_unsafe_html`?
##########
docs/apache-airflow/core-concepts/params.rst:
##########
@@ -324,9 +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``.
+ 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
Review Comment:
```suggestion
Custom form elements using the attribute ``custom_html_form`` allow a
DAG author to specify raw HTML form templates. These
```
##########
docs/apache-airflow/core-concepts/params.rst:
##########
@@ -324,9 +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``.
+ 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.
Review Comment:
```suggestion
custom HTML form elements are deprecated as of version 2.8.0.
```
Do we have a plan for replacing this with something else? I'd rather we
don't mention it if we don't know if/what we will do.
##########
newsfragments/35460.significant.rst:
##########
@@ -0,0 +1,10 @@
+Per default raw HTML code in DAG docs and DAG params descriptions is disabled
with 2.8.0.
+
+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 ``description_md``.
+The ``custom_html_form`` is now deprecated and will be replaced by a more
secure feature in a future release.
Review Comment:
```suggestion
The ``custom_html_form`` is now deprecated.
```
Same here.
##########
newsfragments/35460.significant.rst:
##########
@@ -0,0 +1,10 @@
+Per default raw HTML code in DAG docs and DAG params descriptions is disabled
with 2.8.0.
+
+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``.
Review Comment:
```suggestion
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``.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@airflow.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
