Fix double-escaped ampersands in breadcrumb names

What does this MR do and why?

Group, subgroup, project, and organization names that contain an ampersand show in the breadcrumb as & instead of & (a subgroup "Digital & Clients" reads "Digital & Clients"). Only legacy names are affected, since validation no longer allows an ampersand.

The breadcrumb helpers ran each name through simple_sanitize, which turns & into &. The Vue top bar and the JSON-LD block then escape it again, so the extra amp; becomes visible. The fix routes every name (group ancestors, group, project, personal project owner, and organization) through a strip_tags_and_unescape helper that strips tags but decodes HTML entities, so the value is plain text and the consumers do the single escaping.

Security

simple_sanitize was defense in depth here, not the XSS barrier:

  • The JSON-LD block goes through to_json, and escape_html_entities_in_json is enabled, so < and > are emitted as their unicode escapes. A </script> in a name cannot break out of the script tag.
  • The Vue data sits in an HTML-escaped data- attribute, and GlBreadcrumb renders the text with {{ }}, not v-html.

The page title is already pushed onto the same list without sanitizing, so this is the escaping the breadcrumb already relied on. strip_tags_and_unescape still strips tags (the personal-project owner has a malicious-owner spec that depends on it), but it no longer pre-escapes, so ampersands are not double-escaped. simple_sanitize had no other callers and is removed. Added specs in breadcrumbs_helper_spec for strip_tags_and_unescape and that a </script><img src=x onerror=alert(1)> name does not break out of either consumer.

Screen recording

Live before/after on a GDK, on a group named "Digital & Clients": the breadcrumb goes from "Digital & Clients" to "Digital & Clients".

Testing

Regression specs in the group, project, and organization breadcrumb helpers assert the ampersand is kept (including the personal-project owner), plus strip_tags_and_unescape unit specs and the XSS specs above.

Resolves #507321 (closed)

Edited by Eldar Dadashov

Merge request reports

Loading
Loading