From a2b8bed9d02359fefead919fb13898db45360e27 Mon Sep 17 00:00:00 2001 From: Tien Thanh Pham Date: Mon, 14 Sep 2026 14:44:49 +0800 Subject: [PATCH 1/6] [MOL-22452][TP] Fix asgard-0004, add sanitization --- .../notification-banner-hoc.tsx | 36 +++++++- .../notification-banner.spec.tsx | 89 +++++++++++++++++++ 2 files changed, 121 insertions(+), 4 deletions(-) diff --git a/src/notification-banner/notification-banner-hoc.tsx b/src/notification-banner/notification-banner-hoc.tsx index d2da66346e..7229f82a25 100644 --- a/src/notification-banner/notification-banner-hoc.tsx +++ b/src/notification-banner/notification-banner-hoc.tsx @@ -9,6 +9,26 @@ import type { NotificationContentAttributes, } from "./types"; +const SAFE_URL_SCHEMES = /^(https?:|mailto:|tel:)/i; + +function sanitizeAttributes( + attrs: Record +): Record { + const sanitized: Record = {}; + + for (const [key, value] of Object.entries(attrs)) { + if (/^on[A-Z]/i.test(key)) continue; + + if (key === "href" && typeof value === "string") { + if (!SAFE_URL_SCHEMES.test(value)) continue; + } + + sanitized[key] = value; + } + + return sanitized; +} + /** * Higher-order component that wraps `NotificationBanner` and renders its * content from a structured data array. @@ -26,8 +46,12 @@ export const withNotificationBanner = ( if (data.length > 0) { return data.map((attribute, index) => { if (attribute.type === "text") { - const otherAttributes = - attribute.otherAttributes as ContentTextAttributes; + const otherAttributes = sanitizeAttributes( + (attribute.otherAttributes ?? {}) as Record< + string, + unknown + > + ) as ContentTextAttributes; const sanitizedContent = DOMPurify.sanitize( attribute.content @@ -42,8 +66,12 @@ export const withNotificationBanner = ( /> ); } else { - const otherAttributes = - attribute.otherAttributes as ContentLinkAttributes; + const otherAttributes = sanitizeAttributes( + (attribute.otherAttributes ?? {}) as Record< + string, + unknown + > + ) as ContentLinkAttributes; return ( { expect(mockOnClick).toHaveBeenCalledTimes(1); }); + it("should strip javascript: href from link otherAttributes", () => { + const HOCElement = withNotificationBanner([ + { + type: "link", + content: "malicious link", + otherAttributes: { + href: "javascript:alert(document.cookie)", + }, + }, + ]); + render(); + + const anchor = document.querySelector("a"); + expect(anchor).toBeInTheDocument(); + expect(anchor).not.toHaveAttribute("href"); + }); + + it("should strip data: href from link otherAttributes", () => { + const HOCElement = withNotificationBanner([ + { + type: "link", + content: "data link", + otherAttributes: { + href: "data:text/html,", + }, + }, + ]); + render(); + + const anchor = document.querySelector("a"); + expect(anchor).toBeInTheDocument(); + expect(anchor).not.toHaveAttribute("href"); + }); + + it("should preserve safe href schemes in link otherAttributes", () => { + const HOCElement = withNotificationBanner([ + { + type: "link", + content: "safe link", + otherAttributes: { + href: "https://www.example.com", + }, + }, + ]); + render(); + + const anchor = document.querySelector("a"); + expect(anchor).toBeInTheDocument(); + expect(anchor).toHaveAttribute("href", "https://www.example.com"); + }); + + it("should strip event handler attributes from otherAttributes", () => { + const HOCElement = withNotificationBanner([ + { + type: "link", + content: "link with handler", + otherAttributes: { + href: "https://www.example.com", + onMouseOver: "alert(1)" as unknown, + } as Record, + }, + ]); + render(); + + const anchor = document.querySelector("a"); + expect(anchor).toBeInTheDocument(); + expect(anchor).toHaveAttribute("href", "https://www.example.com"); + expect(anchor).not.toHaveAttribute("onMouseOver"); + }); + + it("should strip event handlers from text otherAttributes", () => { + const HOCElement = withNotificationBanner([ + { + type: "text", + content: "safe text", + otherAttributes: { + onClick: "alert(1)" as unknown, + className: "custom-class", + } as Record, + }, + ]); + render(); + + const paragraph = document.querySelector("p"); + expect(paragraph).toBeInTheDocument(); + expect(paragraph).toHaveClass("custom-class"); + expect(paragraph).not.toHaveAttribute("onClick"); + }); + it("should sanitise the content", () => { const HOCElement = withNotificationBanner([ { From 210db78f22f56c07adfd0c021343ea989b3d7670 Mon Sep 17 00:00:00 2001 From: Tien Thanh Pham Date: Mon, 14 Sep 2026 14:47:55 +0800 Subject: [PATCH 2/6] [MOL-22452][TP] Fix asgard-0001, prevent script injection in trigger-gitlab-pipeline workflow using env block --- .github/workflows/trigger-gitlab-pipeline.yml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/.github/workflows/trigger-gitlab-pipeline.yml b/.github/workflows/trigger-gitlab-pipeline.yml index c7d934fac7..bdc8f4ad1f 100644 --- a/.github/workflows/trigger-gitlab-pipeline.yml +++ b/.github/workflows/trigger-gitlab-pipeline.yml @@ -14,10 +14,13 @@ jobs: steps: - name: Print Configs + env: + HEAD_COMMIT_MSG: ${{ github.event.head_commit.message }} + REF_NAME: ${{ github.ref_name }} run: | - BRANCH_NAME="${{ github.ref_name }}" + BRANCH_NAME="$REF_NAME" - COMMIT_MSG=$(echo -e "${{ github.event.head_commit.message }}" | head -n 1) + COMMIT_MSG=$(printf '%s' "$HEAD_COMMIT_MSG" | head -n 1) PIPELINE_PROJECT_URL="github.com/$GITHUB_REPOSITORY.git" From 01fd0ba1aa327e4f3ecfd92882660df8e69fe77e Mon Sep 17 00:00:00 2001 From: Tien Thanh Pham Date: Mon, 14 Sep 2026 16:42:36 +0800 Subject: [PATCH 3/6] [MOL-22452][TP] fix asgard-0003, use execFileSync instead of execSync --- codemods/run-codemod.ts | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/codemods/run-codemod.ts b/codemods/run-codemod.ts index c00feef92b..af8008bd60 100644 --- a/codemods/run-codemod.ts +++ b/codemods/run-codemod.ts @@ -1,5 +1,5 @@ import { checkbox, confirm, input, select } from "@inquirer/prompts"; -import { execSync } from "child_process"; +import { execFileSync } from "child_process"; import * as fs from "fs"; import * as path from "path"; @@ -54,14 +54,23 @@ function runCodemods(selection: UserSelection): void { selectedCodemods.forEach((codemod) => { const codemodPath = path.join(codemodsDir, codemod, "index.ts"); - let command = `npx --yes jscodeshift --parser=tsx -t ${codemodPath} ${targetPath}`; - console.log( `Running codemod: ${codemod} on target path: ${targetPath}` ); try { - execSync(command, { stdio: "inherit" }); + execFileSync( + "npx", + [ + "--yes", + "jscodeshift", + "--parser=tsx", + "-t", + codemodPath, + targetPath, + ], + { stdio: "inherit" } + ); console.log( `Codemod ${codemod} executed successfully on ${targetPath}` ); From da3f590f0d01a32ec44fbfa2f99181c012ae4bf4 Mon Sep 17 00:00:00 2001 From: Tien Thanh Pham Date: Wed, 16 Sep 2026 08:51:20 +0800 Subject: [PATCH 4/6] [MOL-22452][TP] Refactor code --- .github/workflows/trigger-gitlab-pipeline.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/trigger-gitlab-pipeline.yml b/.github/workflows/trigger-gitlab-pipeline.yml index bdc8f4ad1f..94f397cae9 100644 --- a/.github/workflows/trigger-gitlab-pipeline.yml +++ b/.github/workflows/trigger-gitlab-pipeline.yml @@ -20,7 +20,7 @@ jobs: run: | BRANCH_NAME="$REF_NAME" - COMMIT_MSG=$(printf '%s' "$HEAD_COMMIT_MSG" | head -n 1) + COMMIT_MSG=$(printf '%s\n' "$HEAD_COMMIT_MSG" | awk 'NR>1{exit};1') PIPELINE_PROJECT_URL="github.com/$GITHUB_REPOSITORY.git" From b7943f4d40419104ea44d5c6d75b9c705138756a Mon Sep 17 00:00:00 2001 From: Tien Thanh Pham Date: Wed, 16 Sep 2026 09:00:35 +0800 Subject: [PATCH 5/6] [MOL-22452][TP] Sanitize link attributes only --- .../notification-banner-hoc.tsx | 43 +++++++------------ .../notification-banner.spec.tsx | 38 ---------------- 2 files changed, 16 insertions(+), 65 deletions(-) diff --git a/src/notification-banner/notification-banner-hoc.tsx b/src/notification-banner/notification-banner-hoc.tsx index 7229f82a25..6a5b418013 100644 --- a/src/notification-banner/notification-banner-hoc.tsx +++ b/src/notification-banner/notification-banner-hoc.tsx @@ -9,24 +9,19 @@ import type { NotificationContentAttributes, } from "./types"; -const SAFE_URL_SCHEMES = /^(https?:|mailto:|tel:)/i; +function sanitizeLinkAttributes( + attrs: ContentLinkAttributes +): ContentLinkAttributes { + const { href, ...rest } = attrs; -function sanitizeAttributes( - attrs: Record -): Record { - const sanitized: Record = {}; - - for (const [key, value] of Object.entries(attrs)) { - if (/^on[A-Z]/i.test(key)) continue; - - if (key === "href" && typeof value === "string") { - if (!SAFE_URL_SCHEMES.test(value)) continue; - } - - sanitized[key] = value; + if ( + typeof href === "string" && + !DOMPurify.isValidAttribute("a", "href", href) + ) { + return rest; } - return sanitized; + return attrs; } /** @@ -46,12 +41,8 @@ export const withNotificationBanner = ( if (data.length > 0) { return data.map((attribute, index) => { if (attribute.type === "text") { - const otherAttributes = sanitizeAttributes( - (attribute.otherAttributes ?? {}) as Record< - string, - unknown - > - ) as ContentTextAttributes; + const otherAttributes = + attribute.otherAttributes as ContentTextAttributes; const sanitizedContent = DOMPurify.sanitize( attribute.content @@ -66,12 +57,10 @@ export const withNotificationBanner = ( /> ); } else { - const otherAttributes = sanitizeAttributes( - (attribute.otherAttributes ?? {}) as Record< - string, - unknown - > - ) as ContentLinkAttributes; + const otherAttributes = sanitizeLinkAttributes( + (attribute.otherAttributes ?? + {}) as ContentLinkAttributes + ); return ( { expect(anchor).toHaveAttribute("href", "https://www.example.com"); }); - it("should strip event handler attributes from otherAttributes", () => { - const HOCElement = withNotificationBanner([ - { - type: "link", - content: "link with handler", - otherAttributes: { - href: "https://www.example.com", - onMouseOver: "alert(1)" as unknown, - } as Record, - }, - ]); - render(); - - const anchor = document.querySelector("a"); - expect(anchor).toBeInTheDocument(); - expect(anchor).toHaveAttribute("href", "https://www.example.com"); - expect(anchor).not.toHaveAttribute("onMouseOver"); - }); - - it("should strip event handlers from text otherAttributes", () => { - const HOCElement = withNotificationBanner([ - { - type: "text", - content: "safe text", - otherAttributes: { - onClick: "alert(1)" as unknown, - className: "custom-class", - } as Record, - }, - ]); - render(); - - const paragraph = document.querySelector("p"); - expect(paragraph).toBeInTheDocument(); - expect(paragraph).toHaveClass("custom-class"); - expect(paragraph).not.toHaveAttribute("onClick"); - }); - it("should sanitise the content", () => { const HOCElement = withNotificationBanner([ { From 50a92691058919c2f9dd07fccfa151724b53acb1 Mon Sep 17 00:00:00 2001 From: Tien Thanh Pham Date: Wed, 16 Sep 2026 10:01:14 +0800 Subject: [PATCH 6/6] [MOL-22452][TP] Trip href if it's not string too --- src/notification-banner/notification-banner-hoc.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/notification-banner/notification-banner-hoc.tsx b/src/notification-banner/notification-banner-hoc.tsx index 6a5b418013..1a72cc7653 100644 --- a/src/notification-banner/notification-banner-hoc.tsx +++ b/src/notification-banner/notification-banner-hoc.tsx @@ -15,7 +15,7 @@ function sanitizeLinkAttributes( const { href, ...rest } = attrs; if ( - typeof href === "string" && + typeof href !== "string" || !DOMPurify.isValidAttribute("a", "href", href) ) { return rest;