XSS and URL injection

PoC (pre-fix vulnerable behavior): - A server could provide malicious data such as newPullRequestURL: 'javascript:alert(1)' or a username containing crafted input in author fields. - The frontend would render a link with href set to that value (e.g., a Pull Request button linking to javascript:alert(1)). When a user clicks the link, the browser executes the injected script, resulting in a cross-site scripting (XSS) payload. Attack Vector: - UI path: Provisioning UI -> PullRequestButtons (or HistoryView author links) -> link rendered with untrusted URL -> user clicks the link -> JavaScript payload executes. Reproduction steps (conceptual): 1) Set server data to include: - newPullRequestURL: 'javascript:alert(1)' - username fields containing values that could influence hrefs in author links, e.g., 'evilUser' or crafted strings. 2) Load the provisioning UI and render the components (e.g., PullRequestButtons, FileHistoryPage HistoryView). 3) Click the rendered link. If the app is vulnerable (pre-fix), the browser will execute alert(1) or similar XSS payload. PoC after fix (sanitized): - With the patch, URLs are sanitized via textUtil.sanitizeUrl and encodeURIComponent for user profiles. javascript: URLs are removed or sanitized, so the rendered href no longer executes. - Example before/after for a link: - Before: <a href="javascript:alert(1)">Open PR</a> - After: href is sanitized (e.g., removed or replaced with a safe '#'/undefined), and no JavaScript payload can execute. Additional safe-usage PoC (sanitized profile links): - A username like 'user name&special' is encoded to 'user%20name%26special' and used to form a canonical profile URL (e.g., https://github.com/user%20name%26special) without injecting HTML or breaking attributes.

diff --git a/public/app/features/provisioning/File/FileHistoryPage.test.tsx b/public/app/features/provisioning/File/FileHistoryPage.test.tsx new file mode 100644 index 0000000000000..991615e0e7682 --- /dev/null +++ b/public/app/features/provisioning/File/FileHistoryPage.test.tsx @@ -0,0 +1,39 @@ +import { getAuthorProfileUrl } from './FileHistoryPage'; + +describe('getAuthorProfileUrl', () => { + it('returns GitHub URL for github type', () => { + expect(getAuthorProfileUrl('github', 'octocat')).toBe('https://github.com/octocat'); + }); + + it('returns GitHub URL for githubEnterprise type', () => { + expect(getAuthorProfileUrl('githubEnterprise', 'octocat')).toBe('https://github.com/octocat'); + }); + + it('returns GitLab URL for gitlab type', () => { + expect(getAuthorProfileUrl('gitlab', 'user')).toBe('https://gitlab.com/user'); + }); + + it('returns Bitbucket URL for bitbucket type', () => { + expect(getAuthorProfileUrl('bitbucket', 'user')).toBe('https://bitbucket.org/user'); + }); + + it('returns undefined for git type', () => { + expect(getAuthorProfileUrl('git', 'user')).toBeUndefined(); + }); + + it('returns undefined for local type', () => { + expect(getAuthorProfileUrl('local', 'user')).toBeUndefined(); + }); + + it('returns undefined for undefined type', () => { + expect(getAuthorProfileUrl(undefined, 'user')).toBeUndefined(); + }); + + it('encodes special characters in username', () => { + expect(getAuthorProfileUrl('github', 'user name&special')).toBe('https://github.com/user%20name%26special'); + }); + + it('encodes slashes in username', () => { + expect(getAuthorProfileUrl('gitlab', 'org/user')).toBe('https://gitlab.com/org%2Fuser'); + }); +}); diff --git a/public/app/features/provisioning/File/FileHistoryPage.tsx b/public/app/features/provisioning/File/FileHistoryPage.tsx index 9d762d9c9b811..e9c7ce3ebfbcc 100644 --- a/public/app/features/provisioning/File/FileHistoryPage.tsx +++ b/public/app/features/provisioning/File/FileHistoryPage.tsx @@ -56,8 +56,14 @@ export default function FileHistoryPage() { </TextLink> </EmptyState> ) : ( - //@ts-expect-error TODO fix history response types - <div>{history.data ? <HistoryView history={history.data} path={path} repo={name} /> : <Spinner />}</div> + <div> + {history.data ? ( + //@ts-expect-error TODO fix history response types + <HistoryView history={history.data} path={path} repo={name} repoType={repoType ?? undefined} /> + ) : ( + <Spinner /> + )} + </div> )} </Page.Contents> </Page> @@ -68,9 +74,10 @@ interface Props { history: HistoryListResponse; path: string; repo: string; + repoType?: string; } -function HistoryView({ history, path, repo }: Props) { +function HistoryView({ history, path, repo, repoType }: Props) { if (!history.items) { return <Trans i18nKey="provisioning.history-view.not-found">Not found</Trans>; } @@ -78,27 +85,40 @@ function HistoryView({ history, path, repo }: Props) { return ( <Stack direction={'column'}> {history.items.map((item) => ( - <Card noMargin href={`${PROVISIONING_URL}/${repo}/file/${path}?ref=${item.ref}`} key={item.ref}> + <Card + noMargin + href={`${PROVISIONING_URL}/${encodeURIComponent(repo)}/file/${path}?${new URLSearchParams({ ref: item.ref }).toString()}`} + key={item.ref} + > <Card.Heading>{item.message}</Card.Heading> <Card.Meta> <span>{formatTimestamp(item.createdAt)}</span> </Card.Meta> <Card.Description> <Stack> - {item.authors.map((a) => ( - <span key={a.username} style={{ display: 'flex', alignItems: 'center', gap: '4px' }}> - {a.avatarURL && ( - <UserIcon - userView={{ - user: { name: a.name, avatarUrl: a.avatarURL }, - lastActiveAt: new Date().toISOString(), - }} - showTooltip={false} - /> - )} - <a href={`https://github.com/${a.username}`}>{a.name}</a> - </span> - ))} + {item.authors.map((a) => { + const profileUrl = getAuthorProfileUrl(repoType, a.username); + return ( + <span key={a.username} style={{ display: 'flex', alignItems: 'center', gap: '4px' }}> + {a.avatarURL && ( + <UserIcon + userView={{ + user: { name: a.name, avatarUrl: a.avatarURL }, + lastActiveAt: new Date().toISOString(), + }} + showTooltip={false} + /> + )} + {profileUrl ? ( + <TextLink href={profileUrl} external> + {a.name} + </TextLink> + ) : ( + <Text>{a.name}</Text> + )} + </span> + ); + })} </Stack> </Card.Description> </Card> @@ -106,3 +126,18 @@ function HistoryView({ history, path, repo }: Props) { </Stack> ); } + +export function getAuthorProfileUrl(repoType: string | undefined, username: string): string | undefined { + const encoded = encodeURIComponent(username); + switch (repoType) { + case 'github': + case 'githubEnterprise': + return `https://github.com/${encoded}`; + case 'gitlab': + return `https://gitlab.com/${encoded}`; + case 'bitbucket': + return `https://bitbucket.org/${encoded}`; + default: + return undefined; + } +} diff --git a/public/app/features/provisioning/Repository/PullRequestButtons.test.tsx b/public/app/features/provisioning/Repository/PullRequestButtons.test.tsx new file mode 100644 index 0000000000000..fad2ce917e9f0 --- /dev/null +++ b/public/app/features/provisioning/Repository/PullRequestButtons.test.tsx @@ -0,0 +1,46 @@ +import { render, screen } from 'test/test-utils'; + +import { PullRequestButtons } from './PullRequestButtons'; + +describe('PullRequestButtons', () => { + it('sanitizes javascript: URLs from server', () => { + render( + <PullRequestButtons + urls={{ + newPullRequestURL: 'javascript:alert(1)', + compareURL: 'javascript:alert(2)', + sourceURL: 'javascript:alert(3)', + }} + /> + ); + + const links = screen.getAllByRole('link'); + for (const link of links) { + expect(link).not.toHaveAttribute('href', expect.stringContaining('javascript:')); + } + }); + + it('preserves valid URLs', () => { + render( + <PullRequestButtons + urls={{ + newPullRequestURL: 'https://github.com/org/repo/compare/main...branch', + compareURL: 'https://github.com/org/repo/compare/main...branch', + sourceURL: 'https://github.com/org/repo/tree/branch', + }} + /> + ); + + const links = screen.getAllByRole('link'); + expect(links[0]).toHaveAttribute('href', 'https://github.com/org/repo/tree/branch'); + expect(links[1]).toHaveAttribute('href', 'https://github.com/org/repo/compare/main...branch'); + expect(links[2]).toHaveAttribute('href', 'https://github.com/org/repo/compare/main...branch'); + }); + + it('returns null for sync job type', () => { + const { container } = render( + <PullRequestButtons jobType="sync" urls={{ newPullRequestURL: 'https://example.com' }} /> + ); + expect(container).toBeEmptyDOMElement(); + }); +}); diff --git a/public/app/features/provisioning/Repository/PullRequestButtons.tsx b/public/app/features/provisioning/Repository/PullRequestButtons.tsx index 86b8417a1eb9e..ff4e6262b484d 100644 --- a/public/app/features/provisioning/Repository/PullRequestButtons.tsx +++ b/public/app/features/provisioning/Repository/PullRequestButtons.tsx @@ -1,3 +1,4 @@ +import { textUtil } from '@grafana/data'; import { Trans } from '@grafana/i18n'; import { LinkButton, Stack } from '@grafana/ui'; import { type RepositoryUrLs } from 'app/api/clients/provisioning/v0alpha1'; @@ -9,9 +10,9 @@ interface Props { urls?: RepositoryUrLs; } export function PullRequestButtons({ urls, jobType }: Props) { - const pullRequestURL = urls?.newPullRequestURL; - const compareURL = urls?.compareURL; - const branchURL = urls?.sourceURL; + const pullRequestURL = urls?.newPullRequestURL ? textUtil.sanitizeUrl(urls.newPullRequestURL) : undefined; + const compareURL = urls?.compareURL ? textUtil.sanitizeUrl(urls.compareURL) : undefined; + const branchURL = urls?.sourceURL ? textUtil.sanitizeUrl(urls.sourceURL) : undefined; if (jobType === 'sync') { return null; diff --git a/public/app/features/provisioning/Repository/RepositoryActions.tsx b/public/app/features/provisioning/Repository/RepositoryActions.tsx index 2355043b42f70..a93a0dd79039f 100644 --- a/public/app/features/provisioning/Repository/RepositoryActions.tsx +++ b/public/app/features/provisioning/Repository/RepositoryActions.tsx @@ -1,3 +1,4 @@ +import { textUtil } from '@grafana/data'; import { t, Trans } from '@grafana/i18n'; import { reportInteraction } from '@grafana/runtime'; import { Badge, Button, LinkButton, Stack } from '@grafana/ui'; @@ -17,7 +18,8 @@ interface RepositoryActionsProps { export function RepositoryActions({ repository }: RepositoryActionsProps) { const name = repository.metadata?.name ?? ''; - const repoHref = getRepoHrefForProvider(repository.spec); + const rawRepoHref = getRepoHrefForProvider(repository.spec); + const repoHref = rawRepoHref ? textUtil.sanitizeUrl(rawRepoHref) : undefined; const connectionName = repository.spec?.connection?.name; const repoType = repository.spec?.type; diff --git a/public/app/features/provisioning/Repository/RepositoryOverview.tsx b/public/app/features/provisioning/Repository/RepositoryOverview.tsx index 8b6ac699b4d1e..9c1cc75cb8ae3 100644 --- a/public/app/features/provisioning/Repository/RepositoryOverview.tsx +++ b/public/app/features/provisioning/Repository/RepositoryOverview.tsx @@ -2,7 +2,7 @@ import { css, cx } from '@emotion/css'; import { useBooleanFlagValue } from '@openfeature/react-sdk'; import { useMemo } from 'react'; -import { type GrafanaTheme2 } from '@grafana/data'; +import { textUtil, type GrafanaTheme2 } from '@grafana/data'; import { Trans } from '@grafana/i18n'; import { Box, Card, type CellProps, Grid, InteractiveTable, LinkButton, Stack, Text, useStyles2 } from '@grafana/ui'; import { type Repository, type ResourceCount } from 'app/api/clients/provisioning/v0alpha1'; @@ -214,7 +214,7 @@ const getStyles = (theme: GrafanaTheme2) => { function getWebhookURL(repo: Repository) { const { status, spec } = repo; if (spec?.type === 'github' && status?.webhook?.url && spec.github?.url) { - return `${spec.github.url}/settings/hooks/${status.webhook?.id}`; + return textUtil.sanitizeUrl(`${spec.github.url}/settings/hooks/${status.webhook?.id}`); } return undefined; } diff --git a/public/app/features/provisioning/Repository/ResourceTreeView.tsx b/public/app/features/provisioning/Repository/ResourceTreeView.tsx index 1dc057f703e71..f2457276c1b42 100644 --- a/public/app/features/provisioning/Repository/ResourceTreeView.tsx +++ b/public/app/features/provisioning/Repository/ResourceTreeView.tsx @@ -2,7 +2,7 @@ import { css } from '@emotion/css'; import { useBooleanFlagValue } from '@openfeature/react-sdk'; import { useMemo, useState } from 'react'; -import { type GrafanaTheme2 } from '@grafana/data'; +import { textUtil, type GrafanaTheme2 } from '@grafana/data'; import { Trans, t } from '@grafana/i18n'; import { type CellProps, @@ -161,13 +161,14 @@ export function ResourceTreeView({ repo }: ResourceTreeViewProps) { const spec = repo.spec; const config = spec.github || spec.gitlab || spec.bitbucket; if (config) { - sourceLink = getRepoFileUrl({ + const rawSourceLink = getRepoFileUrl({ repoType: spec.type, url: config.url, branch: config.branch, filePath: item.path, pathPrefix: config.path, }); + sourceLink = rawSourceLink ? textUtil.sanitizeUrl(rawSourceLink) : undefined; } } diff --git a/public/app/features/provisioning/components/Dashboards/DashboardPreviewBanner.test.tsx b/public/app/features/provisioning/components/Dashboards/DashboardPreviewBanner.test.tsx index 351c5fd366ace..f748a2b5da82d 100644 --- a/public/app/features/provisioning/components/Dashboards/DashboardPreviewBanner.test.tsx +++ b/public/app/features/provisioning/components/Dashboards/DashboardPreviewBanner.test.tsx @@ -52,7 +52,7 @@ interface PullRequestParamReturn { prURL?: string; newPrURL?: string; repoURL?: string; - repoType?: string; + repoType?: 'github' | 'githubEnterprise' | 'gitlab' | 'bitbucket' | 'git' | 'local'; } interface FileQueryData { diff --git a/public/app/features/provisioning/components/Folders/FolderReadmePanel.test.tsx b/public/app/features/provisioning/components/Folders/FolderReadmePanel.test.tsx index f0d044817460d..922531922a939 100644 --- a/public/app/features/provisioning/components/Folders/FolderReadmePanel.test.tsx +++ b/public/app/features/provisioning/components/Folders/FolderReadmePanel.test.tsx @@ -215,4 +215,17 @@ describe('FolderReadmePanel', () => { expect(screen.getByRole('link', { name: /Edit README/i })).toBeInTheDocument(); expect(screen.queryByRole('link', { name: /Add README/i })).not.toBeInTheDocument(); }); + + it('sanitizes mXSS payloads in README markdown', () => { + setReadmeResult({ + markdownContent: '<div><svg><style><img src=x onerror=alert(1)></style></svg></div>', + }); + + const { container } = setup(); + const markdownDiv = container.querySelector('.markdown-html'); + expect(markdownDiv).not.toBeNull(); + // DOMPurify strips the dangerous elements + expect(markdownDiv!.querySelector('img[onerror]')).toBeNull(); + expect(markdownDiv!.innerHTML).not.toContain('onerror'); + }); }); diff --git a/public/app/features/provisioning/components/Folders/FolderReadmePanel.tsx b/public/app/features/provisioning/components/Folders/FolderReadmePanel.tsx index 2508f9b91e625..7b4b2500ee8eb 100644 --- a/public/app/features/provisioning/components/Folders/FolderReadmePanel.tsx +++ b/public/app/features/provisioning/components/Folders/FolderReadmePanel.tsx @@ -3,7 +3,7 @@ import { useBooleanFlagValue } from '@openfeature/react-sdk'; import { useEffect, useRef } from 'react'; im ... [truncated]