From 251acf38d6d2f77d26ac8ad1b3491c7ddf1cc7f5 Mon Sep 17 00:00:00 2001
From: Charles de Dreuille <charles.dedreuille@gmail.com>
Date: Thu, 23 Apr 2026 08:52:08 +0100
Subject: [PATCH] fix(ui): address further PR review comments

- Replace custom UNSAFE_HREF_RE with @braintree/sanitize-url for robust XSS prevention
- Shorten renderInlineMarkdown JSDoc
- Single-user with href: collapse two adjacent links into one wrapping avatar + name
- Multi-user list: use href ?? index:name as key to avoid collisions on duplicate names
- Status dot: replace role="img"/aria-label with aria-hidden (text label is sufficient)

Signed-off-by: Charles de Dreuille <charles.dedreuille@gmail.com>
---
 packages/ui/package.json                      |  1 +
 packages/ui/src/components/Header/Header.tsx  | 22 ++-----
 .../Header/HeaderMetadataStatus.tsx           |  3 +-
 .../components/Header/HeaderMetadataUsers.tsx | 59 +++++++++----------
 yarn.lock                                     |  8 +++
 5 files changed, 44 insertions(+), 49 deletions(-)

diff --git a/packages/ui/package.json b/packages/ui/package.json
index 0c43476e6a..84b3ada891 100644
--- a/packages/ui/package.json
+++ b/packages/ui/package.json
@@ -47,6 +47,7 @@
   },
   "dependencies": {
     "@backstage/version-bridge": "workspace:^",
+    "@braintree/sanitize-url": "^7.1.2",
     "@remixicon/react": "^4.6.0",
     "@tanstack/react-table": "^8.21.3",
     "clsx": "^2.1.1",
diff --git a/packages/ui/src/components/Header/Header.tsx b/packages/ui/src/components/Header/Header.tsx
index 5d41d6ca0f..07a9ec5ab7 100644
--- a/packages/ui/src/components/Header/Header.tsx
+++ b/packages/ui/src/components/Header/Header.tsx
@@ -20,32 +20,22 @@ import { RiArrowRightSLine } from '@remixicon/react';
 import { HeaderNav } from './HeaderNav';
 import { useDefinition } from '../../hooks/useDefinition';
 import { HeaderDefinition } from './definition';
+import { sanitizeUrl } from '@braintree/sanitize-url';
 import { Container } from '../Container';
 import { Lexer } from 'marked';
 import { Link } from '../Link';
 import { Fragment } from 'react';
 
-// Reject javascript:/vbscript:/data: URIs to prevent XSS via description links.
-const UNSAFE_HREF_RE = /^(javascript:|vbscript:|data:)/i;
-
 /**
- * Renders a plain-text string that may contain inline Markdown links as an
- * array of React nodes (strings and Link elements). Links with unsafe URL
- * schemes (javascript:, vbscript:, data:) are rendered as plain text instead.
- *
- * We use `marked`'s `Lexer.lexInline()` rather than `react-markdown` because
- * `react-markdown` v8+ is ESM-only, which breaks Jest in Node-role packages
- * that transitively import `@backstage/ui` (e.g. via `core-app-api`). `marked`
- * ships CommonJS, has zero dependencies, and its inline lexer gives us a clean
- * token model without needing to maintain a custom regex.
+ * Parses inline Markdown links in a string and returns an array of React nodes.
+ * URLs are sanitized via `@braintree/sanitize-url`; unsafe URLs are rendered as
+ * plain text. Uses `marked` instead of `react-markdown` to avoid ESM issues.
  */
 function renderInlineMarkdown(text: string): React.ReactNode[] {
   return Lexer.lexInline(text).map((token, i) => {
     if (token.type === 'link') {
-      // Trim leading whitespace/control chars before scheme check to prevent
-      // bypass via inputs like " javascript:alert(1)".
-      const href = token.href.trimStart();
-      if (UNSAFE_HREF_RE.test(href)) return token.text;
+      const href = sanitizeUrl(token.href);
+      if (href === 'about:blank') return token.text;
       return (
         <Link key={i} href={href} standalone>
           {token.text}
diff --git a/packages/ui/src/components/Header/HeaderMetadataStatus.tsx b/packages/ui/src/components/Header/HeaderMetadataStatus.tsx
index 2bf2effa8f..e39b58d74e 100644
--- a/packages/ui/src/components/Header/HeaderMetadataStatus.tsx
+++ b/packages/ui/src/components/Header/HeaderMetadataStatus.tsx
@@ -33,8 +33,7 @@ export const HeaderMetadataStatus = ({
   return (
     <div className={styles.single}>
       <span
-        role="img"
-        aria-label={`${color} status`}
+        aria-hidden="true"
         className={`${styles.dot} ${styles[`dot-${color}`]}`}
       />
       <Text variant="body-medium">
diff --git a/packages/ui/src/components/Header/HeaderMetadataUsers.tsx b/packages/ui/src/components/Header/HeaderMetadataUsers.tsx
index ce1688643f..e4d5c2d07b 100644
--- a/packages/ui/src/components/Header/HeaderMetadataUsers.tsx
+++ b/packages/ui/src/components/Header/HeaderMetadataUsers.tsx
@@ -39,45 +39,42 @@ export const HeaderMetadataUsers = ({
 
   if (users.length === 1) {
     const user = users[0];
+    if (user.href) {
+      return (
+        <Link
+          href={user.href}
+          variant="body-medium"
+          standalone
+          className={styles.single}
+        >
+          <Avatar
+            src={user.src ?? 'data:,'}
+            name={user.name}
+            size="small"
+            purpose="decoration"
+          />
+          {user.name}
+        </Link>
+      );
+    }
+
     return (
       <div className={styles.single}>
-        {user.href ? (
-          <>
-            <Link
-              href={user.href}
-              aria-label={user.name}
-              className={styles.avatarLink}
-            >
-              <Avatar
-                src={user.src ?? 'data:,'}
-                name={user.name}
-                size="small"
-                purpose="decoration"
-              />
-            </Link>
-            <Link href={user.href} variant="body-medium" standalone>
-              {user.name}
-            </Link>
-          </>
-        ) : (
-          <>
-            <Avatar
-              src={user.src ?? 'data:,'}
-              name={user.name}
-              size="small"
-              purpose="decoration"
-            />
-            <Text variant="body-medium">{user.name}</Text>
-          </>
-        )}
+        <Avatar
+          src={user.src ?? 'data:,'}
+          name={user.name}
+          size="small"
+          purpose="decoration"
+        />
+        <Text variant="body-medium">{user.name}</Text>
       </div>
     );
   }
 
   return (
     <ul className={styles.stack}>
-      {users.map(user => (
-        <li key={user.name}>
+      {users.map((user, i) => (
+        <li key={user.href ?? `${i}:${user.name}`}>
           <TooltipTrigger>
             {user.href ? (
               <Link
diff --git a/yarn.lock b/yarn.lock
index bbd007be7f..b898ff0e76 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -7973,6 +7973,7 @@ __metadata:
   dependencies:
     "@backstage/cli": "workspace:^"
     "@backstage/version-bridge": "workspace:^"
+    "@braintree/sanitize-url": "npm:^7.1.2"
     "@remixicon/react": "npm:^4.6.0"
     "@storybook/react-vite": "npm:^10.3.3"
     "@tanstack/react-table": "npm:^8.21.3"
@@ -8046,6 +8047,13 @@ __metadata:
   languageName: node
   linkType: hard
 
+"@braintree/sanitize-url@npm:^7.1.2":
+  version: 7.1.2
+  resolution: "@braintree/sanitize-url@npm:7.1.2"
+  checksum: 10/d9626ff8f8eb5e192cd055e6e743449c21102c76bb59e405b7028fe56230fa080bfcc80dfb1e21850a6876e75adda9f7b3c888cf0685942bb74da4d2866d6ec3
+  languageName: node
+  linkType: hard
+
 "@bundled-es-modules/cookie@npm:^2.0.1":
   version: 2.0.1
   resolution: "@bundled-es-modules/cookie@npm:2.0.1"