jedcunningham commented on code in PR #35460:
URL: https://github.com/apache/airflow/pull/35460#discussion_r1388642381
##########
airflow/config_templates/config.yml:
##########
@@ -1828,6 +1828,21 @@ webserver:
type: boolean
example: ~
default: "False"
+ 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
next version.
Review Comment:
```suggestion
``custom_html_form`` in DAG params until the feature is removed in a
future version.
```
##########
airflow/www/views.py:
##########
@@ -1956,30 +1956,69 @@ 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_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]
# 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_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"])
+ if form_trust_problems:
+ flash(
+ Markup(
+ "At least one field in trigger form uses 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 next release."
Review Comment:
```suggestion
"This is deprecated with Airflow 2.8.0 and will be
removed in a future release."
```
##########
airflow/www/views.py:
##########
@@ -1956,30 +1956,69 @@ 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_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]
# 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_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"])
+ if form_trust_problems:
+ flash(
+ Markup(
+ "At least one field in trigger form uses raw HTML form
definition. This is not allowed for "
Review Comment:
```suggestion
"At least one field in the trigger form uses a raw HTML
form definition. This is not allowed for "
```
--
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
