tippyjs: Avoid unsafe allowHTML API in favor of <template> elements.

Signed-off-by: Anders Kaseorg <anders@zulip.com>

frontend_tests/node_tests/rendered_markdown.js

@@ -254,11 +254,6 @@ run_test("timestamp", ({mock_template}) => {
         return html;
     });
 
-    mock_template("markdown_time_tooltip.hbs", true, (data, html) => {
-        assert.deepEqual(data, {tz_offset_str: "UTC"});
-        return html;
-    });
-
     // Setup
     const $content = get_content_element();
     const $timestamp = $.create("timestamp(valid)");
@@ -276,10 +271,6 @@ run_test("timestamp", ({mock_template}) => {
 
     // Final asserts
     assert.equal($timestamp.html(), '<i class="fa fa-clock-o"></i>\nThu, Jan 1 1970, 12:00 AM\n');
-    assert.equal(
-        $timestamp.attr("data-tippy-content"),
-        "Everyone sees this in their own time zone.\n<br/>\nYour time zone: UTC\n",
-    );
     assert.equal($timestamp_invalid.text(), "never-been-set");
 });
 
@@ -290,10 +281,6 @@ run_test("timestamp-twenty-four-hour-time", ({mock_template}) => {
         return html;
     });
 
-    mock_template("markdown_time_tooltip.hbs", false, (data) => {
-        assert.deepEqual(data, {tz_offset_str: "UTC"});
-    });
-
     const $content = get_content_element();
     const $timestamp = $.create("timestamp");
     $timestamp.attr("datetime", "2020-07-15T20:40:00Z");

static/js/click_handlers.js

@@ -43,6 +43,7 @@ import * as stream_list from "./stream_list";
 import * as stream_popover from "./stream_popover";
 import * as topic_list from "./topic_list";
 import * as ui_util from "./ui_util";
+import {parse_html} from "./ui_util";
 import * as unread_ops from "./unread_ops";
 import * as user_profile from "./user_profile";
 import * as util from "./util";
@@ -557,10 +558,9 @@ export function initialize() {
             // so that they don't stick and overlap with
             // each other.
             delay: 0,
-            content: render_buddy_list_tooltip_content(title_data),
+            content: () => parse_html(render_buddy_list_tooltip_content(title_data)),
             arrow: true,
             placement,
-            allowHTML: true,
             showOnCreate: true,
             onHidden: (instance) => {
                 instance.destroy();

static/js/popover_menus.js

@@ -17,6 +17,7 @@ import * as giphy from "./giphy";
 import * as narrow_state from "./narrow_state";
 import * as popovers from "./popovers";
 import * as settings_data from "./settings_data";
+import {parse_html} from "./ui_util";
 import {user_settings} from "./user_settings";
 
 let left_sidebar_stream_setting_popover_displayed = false;
@@ -59,7 +60,6 @@ export function initialize() {
     delegate("body", {
         ...default_popover_props,
         target: "#streams_inline_icon",
-        allowHTML: true,
         onShow(instance) {
             const can_create_streams =
                 settings_data.user_can_create_private_streams() ||
@@ -75,7 +75,7 @@ export function initialize() {
                 return false;
             }
 
-            instance.setContent(render_left_sidebar_stream_setting_popover());
+            instance.setContent(parse_html(render_left_sidebar_stream_setting_popover()));
             left_sidebar_stream_setting_popover_displayed = true;
             return true;
         },
@@ -89,13 +89,14 @@ export function initialize() {
         ...default_popover_props,
         target: ".compose_mobile_button",
         placement: "top",
-        allowHTML: true,
         onShow(instance) {
             on_show_prep(instance);
             instance.setContent(
-                render_mobile_message_buttons_popover_content({
-                    is_in_private_narrow: narrow_state.narrowed_to_pms(),
-                }),
+                parse_html(
+                    render_mobile_message_buttons_popover_content({
+                        is_in_private_narrow: narrow_state.narrowed_to_pms(),
+                    }),
+                ),
             );
             compose_mobile_button_popover_displayed = true;
 
@@ -126,12 +127,13 @@ export function initialize() {
         ...default_popover_props,
         target: ".compose_control_menu_wrapper",
         placement: "top",
-        allowHTML: true,
         onShow(instance) {
             instance.setContent(
-                render_compose_control_buttons_popover({
-                    giphy_enabled: giphy.is_giphy_enabled(),
-                }),
+                parse_html(
+                    render_compose_control_buttons_popover({
+                        giphy_enabled: giphy.is_giphy_enabled(),
+                    }),
+                ),
             );
             compose_control_buttons_popover_instance = instance;
             popovers.hide_all_except_sidebars(instance);
@@ -145,13 +147,14 @@ export function initialize() {
         ...default_popover_props,
         target: ".enter_sends",
         placement: "top",
-        allowHTML: true,
         onShow(instance) {
             on_show_prep(instance);
             instance.setContent(
-                render_compose_select_enter_behaviour_popover({
-                    enter_sends_true: user_settings.enter_sends,
-                }),
+                parse_html(
+                    render_compose_select_enter_behaviour_popover({
+                        enter_sends_true: user_settings.enter_sends,
+                    }),
+                ),
             );
             compose_enter_sends_popover_displayed = true;
         },

static/js/rendered_markdown.js

@@ -165,14 +165,10 @@ export const update_elements = (content) => {
 
         const timestamp = parseISO(time_str);
         if (isValid(timestamp)) {
-            const text = $(this).text();
-            const rendered_time = timerender.render_markdown_timestamp(timestamp, text);
             const rendered_timestamp = render_markdown_timestamp({
-                text: rendered_time.text,
+                text: timerender.format_markdown_time(timestamp),
             });
             $(this).html(rendered_timestamp);
-            $(this).attr("data-tippy-content", rendered_time.tooltip_content_html);
-            $(this).attr("data-tippy-allowHTML", "true");
         } else {
             // This shouldn't happen. If it does, we're very interested in debugging it.
             blueslip.error(`Could not parse datetime supplied by backend: ${time_str}`);

static/js/timerender.ts

@@ -15,6 +15,7 @@ import _ from "lodash";
 import render_markdown_time_tooltip from "../templates/markdown_time_tooltip.hbs";
 
 import {$t} from "./i18n";
+import {parse_html} from "./ui_util";
 import {user_settings} from "./user_settings";
 
 let next_timerender_id = 0;
@@ -224,20 +225,18 @@ export function render_date(time: Date, time_above: Date | undefined, today: Dat
 }
 
 // Renders the timestamp returned by the <time:> Markdown syntax.
-export function render_markdown_timestamp(time: number | Date): {
-    text: string;
-    tooltip_content_html: string;
-} {
+export function format_markdown_time(time: number | Date): string {
     const hourformat = user_settings.twenty_four_hour_time ? "HH:mm" : "h:mm a";
-    const timestring = format(time, "E, MMM d yyyy, " + hourformat);
+    return format(time, "E, MMM d yyyy, " + hourformat);
+}
 
-    const tz_offset_str = get_tz_with_UTC_offset(time);
-    const tooltip_content_html = render_markdown_time_tooltip({tz_offset_str});
-
-    return {
-        text: timestring,
-        tooltip_content_html,
-    };
+export function get_markdown_time_tooltip(reference: HTMLElement): DocumentFragment | string {
+    if (reference instanceof HTMLTimeElement) {
+        const time = parseISO(reference.dateTime);
+        const tz_offset_str = get_tz_with_UTC_offset(time);
+        return parse_html(render_markdown_time_tooltip({tz_offset_str}));
+    }
+    return "";
 }
 
 // This isn't expected to be called externally except manually for

static/js/tippyjs.js

@@ -8,6 +8,18 @@ import * as reactions from "./reactions";
 import * as rows from "./rows";
 import * as timerender from "./timerender";
 
+// For tooltips without data-tippy-content, we use the HTML content of
+// a <template> whose id is given by data-tooltip-template-id.
+function get_tooltip_content(reference) {
+    if ("tooltipTemplateId" in reference.dataset) {
+        const template = document.querySelector(
+            `template#${CSS.escape(reference.dataset.tooltipTemplateId)}`,
+        );
+        return template.content.cloneNode(true);
+    }
+    return "";
+}
+
 // We override the defaults set by tippy library here,
 // so make sure to check this too after checking tippyjs
 // documentation for default properties.
@@ -34,9 +46,10 @@ tippy.setDefaultProps({
     // tooltips.
     appendTo: "parent",
 
-    // html content is not supported by default
-    // enable it by passing data-tippy-allowHTML="true"
-    // in the tag or a parameter.
+    // To add a text tooltip, override this by setting data-tippy-content.
+    // To add an HTML tooltip, set data-tooltip-template-id to the id of a <template>.
+    // Or, override this with a function returning string (text) or DocumentFragment (HTML).
+    content: get_tooltip_content,
 });
 
 export function initialize() {
@@ -194,11 +207,20 @@ export function initialize() {
         appendTo: () => document.body,
     });
 
+    delegate("body", {
+        target: ".rendered_markdown time",
+        content: timerender.get_markdown_time_tooltip,
+        appendTo: () => document.body,
+        onHidden(instance) {
+            instance.destroy();
+        },
+    });
+
     delegate("body", {
         target: [
-            ".rendered_markdown time",
             ".rendered_markdown .copy_codeblock",
             "#compose_top_right [data-tippy-content]",
+            "#compose_top_right [data-tooltip-template-id]",
         ],
         appendTo: () => document.body,
         onHidden(instance) {

static/js/ui_util.ts

@@ -48,3 +48,17 @@ export function update_unread_count_in_dom(unread_count_elem: JQuery, count: num
     unread_count_span.show();
     unread_count_span.text(count);
 }
+
+/**
+ * Parse HTML and return a DocumentFragment.
+ *
+ * Like any consumer of HTML, this function must only be given input
+ * from trusted producers of safe HTML, such as auto-escaping
+ * templates; violating this expectation will introduce bugs that are
+ * likely to be security vulnerabilities.
+ */
+export function parse_html(html: string): DocumentFragment {
+    const template = document.createElement("template");
+    template.innerHTML = html;
+    return template.content;
+}

static/templates/compose.hbs

@@ -4,11 +4,12 @@
     minimal css and no JS. We keep it `position: absolute` to prevent
     it changing compose box layout in any way. --}}
     <div id="scroll-to-bottom-button-container">
-        <div id="scroll-to-bottom-button-clickable-area"  data-tippy-content="{{t 'Scroll to bottom' }}  &lt;span class='hotkey-hint'&gt;({{scroll_to_bottom_key_html}})&lt;/span&gt;" data-tippy-allowHTML="true">
+        <div id="scroll-to-bottom-button-clickable-area" data-tooltip-template-id="scroll-to-bottom-button-tooltip">
             <div id="scroll-to-bottom-button">
                 <i class="fa fa-chevron-down"></i>
             </div>
         </div>
+        <template id="scroll-to-bottom-button-tooltip">{{t 'Scroll to bottom' }} <span class="hotkey-hint">({{{scroll_to_bottom_key_html}}})</span></template>
     </div>
     <div id="compose_controls" class="new-style">
         <div id="compose_buttons">
@@ -69,7 +70,8 @@
                         <div id="compose_top_right" class="order-2">
                             <button type="button" class="expand_composebox_button fa fa-angle-up" aria-label="{{t 'Expand compose' }}" data-tippy-content="{{t 'Expand compose' }}"></button>
                             <button type="button" class="collapse_composebox_button fa fa-angle-down" aria-label="{{t 'Collapse compose' }}" data-tippy-content="{{t 'Collapse compose' }}"></button>
-                            <button type="button" class="close" id='compose_close' data-tippy-content="{{t 'Cancel compose' }} &lt;span class='hotkey-hint'&gt;(Esc)&lt;/span&gt;" data-tippy-allowHTML="true">&times;</button>
+                            <button type="button" class="close" id="compose_close" data-tooltip-template-id="compose_close_tooltip">&times;</button>
+                            <template id="compose_close_tooltip">{{t 'Cancel compose' }} <span class="hotkey-hint">(Esc)</span></template>
                         </div>
                         <div id="stream-message" class="order-1">
                             <div class="stream-selection-header-colorblock message_header_stream left_part" tab-index="-1"></div>
@@ -142,3 +144,5 @@
         </div>
     </div>
 </div>
+
+<template id="add-global-time-tooltip">{{#tr}}Add global time<br />Everyone sees global times in their own time zone.{{/tr}}</template>

static/templates/compose_control_buttons.hbs

@@ -7,7 +7,7 @@
     <a role="button" class="undo_markdown_preview compose_control_button fa fa-edit" aria-label="{{t 'Write' }}" tabindex=0 style="display:none;" data-tippy-content="{{t 'Write' }}"></a>
     <a role="button" class="compose_control_button fa fa-video-camera video_link" aria-label="{{t 'Add video call' }}" tabindex=0 data-tippy-content="{{t 'Add video call' }}"></a>
     <a role="button" class="compose_control_button fa fa-smile-o emoji_map" aria-label="{{t 'Add emoji' }}" tabindex=0 data-tippy-content="{{t 'Add emoji' }}"></a>
-    <a role="button" class="compose_control_button fa fa-clock-o time_pick" aria-label="{{t 'Add global time' }}" tabindex=0 data-tippy-content="{{t 'Add global time<br />Everyone sees global times in their own time zone.' }}" data-tippy-allowHTML="true" data-tippy-maxWidth="none"></a>
+    <a role="button" class="compose_control_button fa fa-clock-o time_pick" aria-label="{{t 'Add global time' }}" tabindex=0 data-tooltip-template-id="add-global-time-tooltip" data-tippy-maxWidth="none"></a>
     <a role="button" class="compose_control_button compose_gif_icon {{#unless giphy_enabled }} hide {{/unless}} zulip-icon zulip-icon-gif" aria-label="{{t 'Add GIF' }}" tabindex=0 data-tippy-content="{{t 'Add GIF' }}"></a>
     <div class="divider hide-sm">|</div>
     <div class="{{#if message_id}}hide-lg{{else}}hide-sm{{/if}}">

static/templates/recent_topic_row.hbs

@@ -32,9 +32,10 @@
     <td class='recent_topic_users'>
         <ul class="recent_topics_participants">
             {{#if other_senders_count}}
-            <li class="recent_topics_participant_item tippy-zulip-tooltip" data-tippy-content="{{other_sender_names_html}}" data-tippy-allowHTML="true">
+            <li class="recent_topics_participant_item tippy-zulip-tooltip" data-tooltip-template-id="recent_topics_participant_overflow_tooltip:{{topic_key}}">
                 <span class="recent_topics_participant_overflow">+{{other_senders_count}}</span>
             </li>
+            <template id="recent_topics_participant_overflow_tooltip:{{topic_key}}">{{{other_sender_names_html}}}</template>
             {{/if}}
             {{#each senders}}
                 {{#if this.is_muted}}

static/templates/right_sidebar.hbs

@@ -27,7 +27,8 @@
             <a id="invite-user-link" href="#invite"><i class="fa fa-user-plus" aria-hidden="true"></i>{{t 'Invite more users' }}</a>
             {{/if}}
             <a id="sidebar-keyboard-shortcuts" data-overlay-trigger="keyboard-shortcuts" class="hidden-for-spectators">
-                <i class="fa fa-keyboard-o fa-2x tippy-zulip-tooltip" id="keyboard-icon" data-tippy-allowHTML="true" data-tippy-content="{{t 'Keyboard shortcuts' }} &lt;span class='hotkey-hint'&gt;(?)&lt;/span&gt;"></i>
+                <i class="fa fa-keyboard-o fa-2x tippy-zulip-tooltip" id="keyboard-icon" data-tooltip-template-id="keyboard-icon-tooltip"></i>
+                <template id="keyboard-icon-tooltip">{{t 'Keyboard shortcuts' }} <span class="hotkey-hint">(?)</span></template>
             </a>
             <div class="only-visible-for-spectators">
                 <div class="realm-description">

tools/linter_lib/custom_check.py

@@ -216,6 +216,10 @@ js_rules = RuleList(
             "good_lines": ["assert.ok(...)"],
             "bad_lines": ["assert(...)"],
         },
+        {
+            "pattern": r"allowHTML|(?i:data-tippy-allowHTML)",
+            "description": "Never use Tippy.js allowHTML; for an HTML tooltip, get a DocumentFragment with ui_util.parse_html.",
+        },
         *whitespace_rules,
     ],
 )
@@ -724,6 +728,10 @@ html_rules: List["Rule"] = [
         "good_lines": ["#my-style {color: blue;}", 'style="display: none"', "style='display: none"],
         "bad_lines": ['<p style="color: blue;">Foo</p>', 'style = "color: blue;"'],
     },
+    {
+        "pattern": r"(?i:data-tippy-allowHTML)",
+        "description": "Never use data-tippy-allowHTML; for an HTML tooltip, set data-tooltip-template-id to the id of a <template>.",
+    },
 ]
 
 handlebars_rules = RuleList(