From 859bb1ac87eed149ff8a77beee2cfebd201c9f77 Mon Sep 17 00:00:00 2001 From: Niko Lindroos Date: Wed, 10 Dec 2025 14:16:02 +0200 Subject: [PATCH 1/9] fix(sonar): test image error handler should use reason HCRC-162. Fixes "Expected the Promise rejection reason to be an Error." --- src/common/utils/testImage.ts | 48 ++++++++++++++++++++++------------- 1 file changed, 30 insertions(+), 18 deletions(-) diff --git a/src/common/utils/testImage.ts b/src/common/utils/testImage.ts index ea3d21c6..4dcc7253 100644 --- a/src/common/utils/testImage.ts +++ b/src/common/utils/testImage.ts @@ -1,32 +1,44 @@ /** - * Test that loading image is successful + * Tests if an image at a given URL loads successfully, + * using AbortController for clean event listener management. + * + * @param {string} [url] - The URL of the image to test. + * @return {Promise} - A promise that resolves if the image loads, and rejects otherwise. */ - -/** - * @param {url} url - url of the image to test. - * @return {boolean} - Returns promise. - */ -const testImage = (url?: string): Promise => { +const testImage = (url?: string): Promise => { if (!url) { return Promise.reject(new Error('No image URL given')); } - // Define the promise - const imgPromise = new Promise((resolve, reject) => { - // Create the image - const imgElement = new Image(); + const imgElement = new Image(); + // Create the controller instance + const controller = new AbortController(); + const { signal } = controller; - // When image is loaded, resolve the promise - imgElement.addEventListener('load', () => resolve()); + return new Promise((resolve, reject) => { + // The { signal } option ensures these listeners are grouped for cleanup + imgElement.addEventListener( + 'load', + () => { + controller.abort(); // Triggers removal of ALL listeners attached to this signal + resolve(); + }, + { signal }, + ); - // When there's an error during load, reject the promise - imgElement.addEventListener('error', () => reject()); + imgElement.addEventListener( + 'error', + () => { + controller.abort(); // Triggers removal of ALL listeners attached to this signal + // Reject with a standard Error object + reject(new Error(`Failed to load image from URL: ${url}`)); + }, + { signal }, + ); - // Assign URL imgElement.src = url; }); - - return imgPromise; + // Note: The controller is self-contained and cleaned up when the promise resolves/rejects }; export default testImage; From 4bf559e2fe14dc17284ce8bf90a68cbb9b7ec4ad Mon Sep 17 00:00:00 2001 From: Niko Lindroos Date: Wed, 10 Dec 2025 14:16:52 +0200 Subject: [PATCH 2/9] fix(sonar): remove duplicated CSS rule HCRC-162 --- src/core/imageGallery/imageGallery.module.scss | 1 - 1 file changed, 1 deletion(-) diff --git a/src/core/imageGallery/imageGallery.module.scss b/src/core/imageGallery/imageGallery.module.scss index c4223712..d76acf57 100644 --- a/src/core/imageGallery/imageGallery.module.scss +++ b/src/core/imageGallery/imageGallery.module.scss @@ -57,7 +57,6 @@ display: flex; justify-content: center; align-items: center; - display: flex; position: absolute; max-height: 90%; max-width: 100%; From dfc8c31d93e54badb4802bc61b119751c4e5c91d Mon Sep 17 00:00:00 2001 From: Niko Lindroos Date: Wed, 10 Dec 2025 14:21:40 +0200 Subject: [PATCH 3/9] fix(sonar): correct identical sub-expressions HCRC-162. Fixes 'Correct one of the identical sub-expressions on both sides of operator "&&"'. --- src/core/navigation/Navigation.tsx | 2 +- src/core/pageContent/PageContentLayout.tsx | 4 +--- src/core/pageContent/sidebarContent/SidebarContentCard.tsx | 2 +- 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/src/core/navigation/Navigation.tsx b/src/core/navigation/Navigation.tsx index 71725e0d..ca932082 100644 --- a/src/core/navigation/Navigation.tsx +++ b/src/core/navigation/Navigation.tsx @@ -196,7 +196,7 @@ export function Navigation({ logoAriaLabel={t('headerActionBarLogoAriaLabel')} > - {userNavigation && userNavigation} + {userNavigation} {menuItemChildren[TOP_LEVEL_MENU_ITEM_PARENT_ID]?.map( diff --git a/src/core/pageContent/PageContentLayout.tsx b/src/core/pageContent/PageContentLayout.tsx index 4caf47b5..b9d36b60 100644 --- a/src/core/pageContent/PageContentLayout.tsx +++ b/src/core/pageContent/PageContentLayout.tsx @@ -59,9 +59,7 @@ export function PageContentLayout({
{content}
-
- {shareLinks && shareLinks} -
+
{shareLinks}
diff --git a/src/core/pageContent/sidebarContent/SidebarContentCard.tsx b/src/core/pageContent/sidebarContent/SidebarContentCard.tsx index ddd57920..b3a20dc8 100644 --- a/src/core/pageContent/sidebarContent/SidebarContentCard.tsx +++ b/src/core/pageContent/sidebarContent/SidebarContentCard.tsx @@ -33,7 +33,7 @@ export default function SidebarContentCard({ alt={imageAlt ?? ''} />
-

{description && description}

+ {description &&

{description}

} {title} From 727174c3e60b489d8eabe3b6dad5e984be547c6f Mon Sep 17 00:00:00 2001 From: Niko Lindroos Date: Wed, 10 Dec 2025 14:34:07 +0200 Subject: [PATCH 4/9] fix(sonar): security hotspot in string utils HCRC-162. Fixes "Make sure the regex used here, which is vulnerable to super-linear runtime due to backtracking, cannot lead to denial of service." in `getTextFromHtml`. Also added JSdocs and cleaned up the whole module. --- src/core/utils/string.ts | 34 +++++++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/src/core/utils/string.ts b/src/core/utils/string.ts index 61b3e113..0f901a31 100644 --- a/src/core/utils/string.ts +++ b/src/core/utils/string.ts @@ -1,17 +1,41 @@ -/* eslint-disable import/no-extraneous-dependencies */ import { camelCase, startCase } from 'lodash-es'; import { decode } from 'html-entities'; export const SELECT_COLORS_LIGHT = ['coat-of-arms', 'brick', 'bus', 'tram']; -export const getColor = (color: string): string => - startCase(camelCase(color)).replace(/\s/g, ''); +// Helper to convert strings like 'coat-of-arms' to 'CoatOfArms' +const toPascalCase = (s: string): string => + startCase(camelCase(s)).replace(/\s/g, ''); +/** + * Converts a string (e.g., 'coat-of-arms') into PascalCase (e.g., 'CoatOfArms'). + * @param {string} color The color string to convert. + * @returns {string} The color string in PascalCase format. + */ +export const getColor = (color: string): string => toPascalCase(color); + +/** + * Prepends 'icon-' to a name and converts it to PascalCase (e.g., 'icon-alert' to 'IconAlert'). + * @param {string} name The icon name part. + * @returns {string} The icon name in PascalCase format. + */ export const getIconName = (name: string): string => - startCase(camelCase(`icon-${name}`)).replace(/\s/g, ''); + toPascalCase(`icon-${name}`); +/** + * Strips all HTML tags from a string and decodes HTML entities. + * Uses a safe regex (/<[^>]*>/gi) to prevent ReDoS (Regular Expression Denial of Service). + * + * @param {string} html The input string containing HTML markup. + * @returns {string} The resulting text string with all HTML tags removed and entities decoded. + */ export const getTextFromHtml = (html: string): string => - decode(html.toString().replace(/(<([^>]+)>)/gi, '')); + decode(html.replace(/<[^>]*>/gi, '')); +/** + * Checks if the text color should be white based on the background color. + * @param {string} color The background color string. + * @returns {boolean} True if white text should be used, false otherwise. + */ export const isWhiteText = (color: string): boolean => SELECT_COLORS_LIGHT.includes(color); From 02057abfbc291b554ef98a2094c0d08a803f99b7 Mon Sep 17 00:00:00 2001 From: Niko Lindroos Date: Wed, 10 Dec 2025 14:45:57 +0200 Subject: [PATCH 5/9] fix(sonar): fix weak cryptography hotspots in story book files HCRC-162. The fixed security hotspots were safe, because they were only in storybook demo files, but using Math.random() could still be replaced with an array index, since only the siblings needs to have unique keys in React (https://legacy.reactjs.org/docs/lists-and-keys.html #keys-must-only-be-unique-among-siblings). Also, for a random number in storybook, it's ok. --- sonar-project.properties | 11 +++++ src/core/page/Page.stories.tsx | 47 ++++++++++--------- src/core/pageContent/PageContent.stories.tsx | 5 +- .../CardsModule/CardsModule.stories.tsx | 1 + 4 files changed, 40 insertions(+), 24 deletions(-) diff --git a/sonar-project.properties b/sonar-project.properties index 31a784be..d7c3c711 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -17,3 +17,14 @@ sonar.test.inclusions=src/**/*.test.ts,src/**/*.test.tsx,src/**/__tests__/**/*.{ # Coverage report sonar.javascript.lcov.reportPaths=coverage/lcov.info + + +# Ignore "Using pseudorandom number generators (PRNGs) is security-sensitive" in stoyry book files. +# because it is safe in a non-security related Storybook demo file. +# +# Rule: typescript:S2245 +# Name: "Using pseudorandom number generators (PRNGs) is security-sensitive" +# URL: +# - https://sonarcloud.io/organizations/city-of-helsinki/rules?open=typescript%3AS2245&rule_key=typescript%3AS2245 +sonar.issue.ignore.multicriteria.e1.ruleKey=typescript:S2245 +sonar.issue.ignore.multicriteria.e1.resourceKey=**/*.stories.tsx diff --git a/src/core/page/Page.stories.tsx b/src/core/page/Page.stories.tsx index bec88d3d..f8d3bc7e 100644 --- a/src/core/page/Page.stories.tsx +++ b/src/core/page/Page.stories.tsx @@ -116,28 +116,31 @@ export const PageDefault = { { title: 'Root', uri: '/' }, { title: 'Nested', uri: '/nested' }, ])} - collections={getCollections(pageMock.modules)?.map((collection) => ( - ( - - ))} - /> - ))} + collections={getCollections(pageMock.modules)?.map( + (collection, index) => ( + ( + + ))} + /> + ), + )} shareLinks={
TODO: Implement share links diff --git a/src/core/pageContent/PageContent.stories.tsx b/src/core/pageContent/PageContent.stories.tsx index 7faf6fb1..936e4f8a 100644 --- a/src/core/pageContent/PageContent.stories.tsx +++ b/src/core/pageContent/PageContent.stories.tsx @@ -91,9 +91,10 @@ export const PageContentWithFunctions = { /> ), collections: (page: PageType | ArticleType) => - getCollections(page?.modules ?? [], false)?.map((collection) => ( + getCollections(page?.modules ?? [], false)?.map((collection, index) => ( { description: 'Haluatko järjestää liikuntatapahtuman tai etsitkö harjoitusvuoroa seurallesi? Helsingissä on runsaasti erilaisia ja erikokoisia liikuntatiloja, joita voi varata maksutta.', link: { target: '_blank', title: 'link title', url: 'https://hel.fi' }, + // // SonarCloud S2245: Math.random() is safe here as this is a non-security related Storybook demo file. backgroundColor: SELECT_COLORS[Math.floor(Math.random() * SELECT_COLORS.length)], icon: 'alert-circle', From cf854250dd8785d48cdad81623a8b2f1f88350cf Mon Sep 17 00:00:00 2001 From: Niko Lindroos Date: Wed, 10 Dec 2025 14:48:31 +0200 Subject: [PATCH 6/9] fix(sonar): ignore functions 4 level deep in tests HCRC-162 --- sonar-project.properties | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/sonar-project.properties b/sonar-project.properties index d7c3c711..118eeb74 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -18,6 +18,14 @@ sonar.test.inclusions=src/**/*.test.ts,src/**/*.test.tsx,src/**/__tests__/**/*.{ # Coverage report sonar.javascript.lcov.reportPaths=coverage/lcov.info +# Ignore "Refactor this code to not nest functions more than 4 levels deep." in test files, +# because it's mostly raised when describe-functions are used. +# +# Rule: typescript:S2004 +# Name: "Refactor this code to not nest functions more than 4 levels deep." +# URL: https://rules.sonarsource.com/typescript/RSPEC-2004/ +sonar.issue.ignore.multicriteria.e1.ruleKey=typescript:S2004 +sonar.issue.ignore.multicriteria.e1.resourceKey=**/*.test.tsx # Ignore "Using pseudorandom number generators (PRNGs) is security-sensitive" in stoyry book files. # because it is safe in a non-security related Storybook demo file. @@ -25,6 +33,7 @@ sonar.javascript.lcov.reportPaths=coverage/lcov.info # Rule: typescript:S2245 # Name: "Using pseudorandom number generators (PRNGs) is security-sensitive" # URL: +# - https://rules.sonarsource.com/typescript/RSPEC-2245/ # - https://sonarcloud.io/organizations/city-of-helsinki/rules?open=typescript%3AS2245&rule_key=typescript%3AS2245 -sonar.issue.ignore.multicriteria.e1.ruleKey=typescript:S2245 -sonar.issue.ignore.multicriteria.e1.resourceKey=**/*.stories.tsx +sonar.issue.ignore.multicriteria.e2.ruleKey=typescript:S2245 +sonar.issue.ignore.multicriteria.e2.resourceKey=**/*.stories.tsx From 90990070b9f1b9826aeb9f8d8aa9c523d3a3af37 Mon Sep 17 00:00:00 2001 From: Niko Lindroos Date: Wed, 10 Dec 2025 15:44:05 +0200 Subject: [PATCH 7/9] refactor(card): fix card's cognitive complexity issue HCRC-162. Sonarcloud reported "Refactor this function to reduce its Cognitive Complexity from 29 to the 15 allowed. Cognitive Complexity of functions should not be too high typescript:S3776". Splitted the code of Card component to make it easier to read, maintain and understand. --- src/core/card/Card.tsx | 357 +++++++++++++++++++++++++++-------------- 1 file changed, 238 insertions(+), 119 deletions(-) diff --git a/src/core/card/Card.tsx b/src/core/card/Card.tsx index 6b078476..88cef01a 100644 --- a/src/core/card/Card.tsx +++ b/src/core/card/Card.tsx @@ -3,7 +3,6 @@ import { IconArrowRight } from 'hds-react'; import type { MouseEventHandler } from 'react'; import React, { useState } from 'react'; -// eslint-disable-next-line import/no-extraneous-dependencies import classNames from 'classnames'; import styles from './card.module.scss'; @@ -127,11 +126,139 @@ export type CardProps = { flex?: boolean; }; -export function Card({ +const warnOnSSR = () => { + // eslint-disable-next-line no-console + console.warn('Attempted to navigate in a non-browser environment.'); +}; + +function CardTitle({ + children, + direction, + withTitleIcon, + titleIcon, +}: { + children: React.ReactNode; + direction?: CardProps['direction']; + withTitleIcon: CardProps['withTitleIcon']; + titleIcon: CardProps['titleIcon']; +}) { + if (!children) { + return null; + } + + return ( +
+ {children} + {withTitleIcon && titleIcon} +
+ ); +} + +function CardSubTitle({ + children, + backgroundColor, +}: { + children: React.ReactNode; + backgroundColor?: CardProps['backgroundColor']; +}): React.ReactElement | null { + if (!children) { + return null; + } + + return ( +
+ {children} +
+ ); +} + +function CardText({ + children, + clampText, + direction, +}: Pick & { + children: React.ReactNode; +}): React.ReactElement | null { + if (!children) { + return null; + } + + return ( +
+ {children} +
+ ); +} + +function CardLink({ + url, + backgroundColor, + openLinkInNewTab, + ariaLabel, + linkArrowLabel, +}: Pick< + CardProps, + | 'url' + | 'backgroundColor' + | 'openLinkInNewTab' + | 'ariaLabel' + | 'linkArrowLabel' +>) { + if (!url) { + return null; + } + + return ( +
+ } + showExternalIcon={false} + aria-label={ariaLabel} + > + {linkArrowLabel && ( + {linkArrowLabel} + )} + +
+ ); +} + +const getImagePosition = (alignment: CardProps['alignment']) => { + if (alignment === undefined) { + return 'image-left'; + } + return alignment.indexOf('left') === -1 ? 'image-right' : 'image-left'; +}; + +function CardInnerContent({ id, - alignment, ariaLabel, - className, imageUrl, imageLabel, title, @@ -151,60 +278,42 @@ export function Card({ style, backgroundColor, primaryContent = 'text', - flex, -}: CardProps) { - const [isHovered, setIsHovered] = useState(false); - const handleToggleActive: MouseEventHandler | undefined = () => - setIsHovered((val) => !val); - const isDelimited = alignment?.startsWith('delimited'); - const isCentered = alignment?.startsWith('center'); - - const getImagePosition = () => { - if (alignment === undefined) { - return 'image-left'; - } - return alignment.indexOf('left') === -1 ? 'image-right' : 'image-left'; - }; - - const imagePosition = getImagePosition(); - - const { - utils: { redirectToUrl, getIsHrefExternal }, - } = useConfig(); - - const openInNewTab = (): void => { - const newWindow = window.open(url, '_blank', 'noopener,noreferrer'); - if (newWindow) newWindow.opener = null; - }; - - const handleClick = () => { - if (url) { - if (getIsHrefExternal(url)) { - openInNewTab(); - } else { - redirectToUrl(url); - } - } - }; + isHovered, + handleToggleActive, + isDelimited, + isCentered, + imagePosition, + handleClick, +}: Omit & { + isHovered: boolean; + handleToggleActive: MouseEventHandler; + isDelimited: boolean; + isCentered: boolean; + imagePosition: 'image-left' | 'image-right'; + handleClick: () => void; +}) { + const dynamicBackgroundClass = backgroundColor + ? colorStyles[`background${getColor(backgroundColor)}`] + : colorStyles.backgroundDefault; - const content = ( + return (
{title && ( -
{title} - {withTitleIcon && titleIcon} -
- )} - {subTitle && ( -
- {subTitle} -
+ )} + + {subTitle} + {text && ( -
+ {getTextFromHtml(text)} -
+ )} {customContent && (
{customContent}
)}
- {url && hasLink && ( -
- } - showExternalIcon={false} - aria-label={ariaLabel} - > - {linkArrowLabel && ( - {linkArrowLabel} - )} - -
+ {hasLink && ( + )}
); +} + +export function Card(props: CardProps) { + const { id, ariaLabel, className, url, openLinkInNewTab, flex, alignment } = + props; + const [isHovered, setIsHovered] = useState(false); + const handleToggleActive: MouseEventHandler | undefined = () => + setIsHovered((val) => !val); + + const isDelimited = alignment?.startsWith('delimited') ?? false; + const isCentered = alignment?.startsWith('center') ?? false; + const imagePosition = getImagePosition(alignment); + + const { + utils: { redirectToUrl, getIsHrefExternal }, + } = useConfig(); + + const handleClick = () => { + if (!url) return; + + // Ensure we are in a client-side (browser) environment + if (typeof window === 'undefined') { + warnOnSSR(); + return; + } + + // Determine if we should open a new tab: either it's explicitly set, or it's an external URL. + const shouldOpenNewTab = openLinkInNewTab || getIsHrefExternal(url); + + if (shouldOpenNewTab) { + const newWindow = window.open(url, '_blank', 'noopener,noreferrer'); + if (newWindow) newWindow.opener = null; + } else { + redirectToUrl(url); + } + }; + + const content = ( + + ); if (url) { return ( From 81111223a4b9aa18bc6f964ea85183e99716551d Mon Sep 17 00:00:00 2001 From: Niko Lindroos Date: Wed, 10 Dec 2025 17:03:50 +0200 Subject: [PATCH 8/9] fix(sonar): fixes to multiple medium level sonar issues HCRC-162 --- src/core/card/card.module.scss | 30 +++++++------------ .../components/CarouselSlideButton.tsx | 8 +++-- .../components/CarouselSliderPage.tsx | 12 ++------ src/core/imageGallery/ImagesGrid.tsx | 2 ++ 4 files changed, 21 insertions(+), 31 deletions(-) diff --git a/src/core/card/card.module.scss b/src/core/card/card.module.scss index 91acc9f6..4e8d148e 100644 --- a/src/core/card/card.module.scss +++ b/src/core/card/card.module.scss @@ -140,6 +140,15 @@ flex-direction: row; } } + + &.horizontalBorder { + // overriding withborder param + border: none; + + @include breakpoints.respond_below(s) { + border: 1px solid var(--color-black-30); + } + } } & > :first-child { @@ -182,17 +191,6 @@ } } } - - &.responsive-reverse { - &.horizontalBorder { - // overriding withborder param - border: none; - - @include breakpoints.respond_below(s) { - border: 1px solid var(--color-black-30); - } - } - } } .imageWrapper { @@ -292,10 +290,6 @@ .content { flex: 1; - @include breakpoints.respond_above(s) { - // min-height: var(--height-image-desktop); - // height: 100%; - } } .textWrapper { @@ -303,10 +297,6 @@ flex-direction: column; padding: var(--spacing-s) var(--spacing-m); - @include breakpoints.respond_below(s) { - // background-color: var(--color-white); - } - .title { font-size: var(--fontsize-heading-s); line-height: var(--lineheight-m); @@ -341,6 +331,7 @@ display: -webkit-box; &.clamp { -webkit-line-clamp: 2; + line-clamp: 2; -webkit-box-orient: vertical; overflow: hidden; } @@ -353,6 +344,7 @@ margin-bottom: var(--spacing-2-xs); &.clamp { -webkit-line-clamp: 3; + line-clamp: 3; -webkit-box-orient: vertical; } } diff --git a/src/core/carousel/components/CarouselSlideButton.tsx b/src/core/carousel/components/CarouselSlideButton.tsx index 73643629..17e02318 100644 --- a/src/core/carousel/components/CarouselSlideButton.tsx +++ b/src/core/carousel/components/CarouselSlideButton.tsx @@ -25,10 +25,12 @@ export function CarouselPreviousSlideButton() { handleUpdateSlideProps(targetSlide); }; + const ariaLabelTitle = title ? ` - ${title}` : ''; + return (