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>
This commit is contained in:
@@ -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",
|
||||
|
||||
@@ -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}
|
||||
|
||||
@@ -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">
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user