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