diff --git a/app/assets/javascripts/alert.js b/app/assets/javascripts/alert.js index fd060c918100d2a5a4fb7caafaddfdf6f90f9a5c..1af4315487e9cf45ff9f72e1b001b0aa09b8aadf 100644 --- a/app/assets/javascripts/alert.js +++ b/app/assets/javascripts/alert.js @@ -3,6 +3,7 @@ import isEmpty from 'lodash/isEmpty'; import { GlAlert, GlLink, GlSprintf } from '@gitlab/ui'; import * as Sentry from '~/sentry/sentry_browser_wrapper'; import { __ } from '~/locale'; +import { sanitize } from '~/lib/dompurify'; export const VARIANT_SUCCESS = 'success'; export const VARIANT_WARNING = 'warning'; @@ -76,6 +77,7 @@ export const createAlert = ({ error = null, messageLinks = null, dismissible = true, + renderMessageHTML = false, }) => { if (captureError && error) Sentry.captureException(error); @@ -90,6 +92,19 @@ export const createAlert = ({ } const createMessageNodes = (h) => { + if (renderMessageHTML) { + return [ + h('div', { + domProps: { + innerHTML: sanitize(message, { + ALLOWED_TAGS: ['a'], + ALLOWED_ATTR: ['href', 'rel', 'target'], + }), + }, + }), + ]; + } + if (isEmpty(messageLinks)) { return message; } diff --git a/ee/app/assets/javascripts/security_dashboard/components/shared/vulnerability_report/vulnerability_list_graphql.vue b/ee/app/assets/javascripts/security_dashboard/components/shared/vulnerability_report/vulnerability_list_graphql.vue index 0e8ea646abbf4ec052b8616b48a5bf1565d52180..1531c2cd1b1fde76e3c2210ef603db6c34b13472 100644 --- a/ee/app/assets/javascripts/security_dashboard/components/shared/vulnerability_report/vulnerability_list_graphql.vue +++ b/ee/app/assets/javascripts/security_dashboard/components/shared/vulnerability_report/vulnerability_list_graphql.vue @@ -22,43 +22,6 @@ const GRAPHQL_DATA_PATH = { [DASHBOARD_TYPES.PIPELINE]: 'project.pipeline.securityReportFindings', }; -// @NOTE: This is a temporary solution to extract links from the error message. -// It was specifically created to handle error messages returned by the GraphQL API when -// there are failures with the Jira integration (app/services/jira/requests/base.rb) and we -// need to render links to relevant docs/settings pages. -// There is a follow-up to improve the `createAlert`, which will make this obsolete: -// https://gitlab.com/gitlab-org/gitlab/-/issues/488393 -export const getErrorAlertFormatFromHtml = (htmlErrorMessage = '') => { - // Initialize variables - let linkCounter = 0; - let messageLinks = null; - - // Regular expression to match <a> tags and extract both the href and the text content - const linkRegex = /<a\s+[^>]*href="([^"]*)".*?>(.*?)<\/a>/g; - - // Replace all <a> tags with placeholders and extract the links - const message = htmlErrorMessage.replace(linkRegex, (match, href, text) => { - if (messageLinks === null) { - messageLinks = {}; - } - - // Generate dynamic placeholders for each link - const placeholderStart = `%{link${linkCounter}Start}`; - const placeholderEnd = `%{link${linkCounter}End}`; - - // Add the extracted href to the messageLinks object - messageLinks[`link${linkCounter}`] = href; - - // Increment the link counter - linkCounter += 1; - - // Return the placeholders wrapped around the link text - return `${placeholderStart}${text}${placeholderEnd}`; - }); - - return { message, messageLinks }; -}; - export default { components: { VulnerabilityList, @@ -131,14 +94,13 @@ export default { return vulnerabilities.nodes; }, error({ message }) { - createAlert( - getErrorAlertFormatFromHtml( - message || - s__( - 'SecurityReports|Error fetching the vulnerability list. Please check your network connection and try again.', - ), - ), + const fallbackMessage = s__( + 'SecurityReports|Error fetching the vulnerability list. Please check your network connection and try again.', ); + createAlert({ + message: message || fallbackMessage, + renderMessageHTML: true, + }); }, skip() { return !this.filters; diff --git a/ee/spec/frontend/security_dashboard/components/shared/vulnerability_report/vulnerability_list_graphql_spec.js b/ee/spec/frontend/security_dashboard/components/shared/vulnerability_report/vulnerability_list_graphql_spec.js index c844095044fbdfa98ad352742126e64c468a5b06..ada5097480886aa0f914d733f0795d0533ba109e 100644 --- a/ee/spec/frontend/security_dashboard/components/shared/vulnerability_report/vulnerability_list_graphql_spec.js +++ b/ee/spec/frontend/security_dashboard/components/shared/vulnerability_report/vulnerability_list_graphql_spec.js @@ -140,22 +140,21 @@ describe('Vulnerability list GraphQL component', () => { }); it.each` - errorMessage | expectedAlertMessage | expectedAlertmessageLinks - ${'Something went wrong'} | ${'Something went wrong'} | ${null} - ${'Something went wrong: <a href="http://docs.com">docs</a>'} | ${'Something went wrong: %{link0Start}docs%{link0End}'} | ${{ link0: 'http://docs.com' }} - ${'Something went wrong: <a href="http://docs.com">docs</a>'} | ${'Something went wrong: %{link0Start}docs%{link0End}'} | ${{ link0: 'http://docs.com' }} - ${undefined} | ${'Error fetching the vulnerability list. Please check your network connection and try again.'} | ${null} - ${''} | ${'Error fetching the vulnerability list. Please check your network connection and try again.'} | ${null} + errorMessage | expectedAlertMessage + ${'Something went wrong'} | ${'Something went wrong'} + ${'Something went wrong. <a href="http://documentation.com/describe-error">Learn more</a>'} | ${'Something went wrong. <a href="http://documentation.com/describe-error">Learn more</a>'} + ${undefined} | ${'Error fetching the vulnerability list. Please check your network connection and try again.'} + ${''} | ${'Error fetching the vulnerability list. Please check your network connection and try again.'} `( 'shows an error message if the query fails', - async ({ errorMessage, expectedAlertMessage, expectedAlertmessageLinks }) => { + async ({ errorMessage, expectedAlertMessage }) => { const vulnerabilitiesHandler = jest.fn().mockRejectedValue(new Error(errorMessage)); createWrapper({ vulnerabilitiesHandler }); await waitForPromises(); expect(createAlert).toHaveBeenCalledWith({ message: expectedAlertMessage, - messageLinks: expectedAlertmessageLinks, + renderMessageHTML: true, }); }, ); diff --git a/spec/frontend/alert_spec.js b/spec/frontend/alert_spec.js index 6a0d5843d9e0cbf6c399f015f7dc7072d7e00d0a..c8749f663950809752e8cda3c149ee3448a268c7 100644 --- a/spec/frontend/alert_spec.js +++ b/spec/frontend/alert_spec.js @@ -1,7 +1,11 @@ import * as Sentry from '~/sentry/sentry_browser_wrapper'; import { setHTMLFixture, resetHTMLFixture } from 'helpers/fixtures'; +import { sanitize } from '~/lib/dompurify'; import { createAlert, VARIANT_WARNING } from '~/alert'; +jest.mock('~/lib/dompurify', () => ({ + sanitize: jest.fn((val) => val), +})); jest.mock('~/sentry/sentry_browser_wrapper'); describe('Flash', () => { @@ -359,6 +363,36 @@ describe('Flash', () => { expect(messageLinks).toHaveLength(0); }); }); + + describe('when rendered as HTML', () => { + const findMessageHTML = () => document.querySelector('.gl-alert-body div').innerHTML; + const message = + 'error: <a href="https://documentation.com/further-information">learn more</a>'; + + it('renders the given message as HTML', () => { + alert = createAlert({ + message, + renderMessageHTML: true, + }); + + expect(findMessageHTML()).toBe(message); + }); + + it('sanitizes the given message', () => { + expect(sanitize).not.toHaveBeenCalled(); + + createAlert({ + message, + renderMessageHTML: true, + }); + + expect(sanitize).toHaveBeenCalledTimes(1); + expect(sanitize).toHaveBeenCalledWith(message, { + ALLOWED_TAGS: ['a'], + ALLOWED_ATTR: ['href', 'rel', 'target'], + }); + }); + }); }); }); });