From 067fdcd0a5b6b63a689f9fe4fa322c690ed263fb Mon Sep 17 00:00:00 2001 From: Joshua Rogers Date: Tue, 5 Aug 2025 11:18:09 +0200 Subject: [PATCH 1/3] techdocs: Disallow javascript URLs Signed-off-by: Joshua Rogers --- .changeset/great-adults-stare.md | 5 ++++ .../transformers/rewriteDocLinks.test.ts | 27 +++++++++++++++++++ .../reader/transformers/rewriteDocLinks.ts | 11 ++++++++ 3 files changed, 43 insertions(+) create mode 100644 .changeset/great-adults-stare.md diff --git a/.changeset/great-adults-stare.md b/.changeset/great-adults-stare.md new file mode 100644 index 0000000000..05d196ac62 --- /dev/null +++ b/.changeset/great-adults-stare.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-techdocs': minor +--- + +Ensure that techdocs rewritten URLs do not contain potentially dangerous javascript: URLs. diff --git a/plugins/techdocs/src/reader/transformers/rewriteDocLinks.test.ts b/plugins/techdocs/src/reader/transformers/rewriteDocLinks.test.ts index 66fc5f29c1..864129164c 100644 --- a/plugins/techdocs/src/reader/transformers/rewriteDocLinks.test.ts +++ b/plugins/techdocs/src/reader/transformers/rewriteDocLinks.test.ts @@ -71,6 +71,33 @@ describe('rewriteDocLinks', () => { expect(getSample(shadowDom, 'a', 'href')).toEqual([]); expect(shadowDom.innerHTML).toContain(expectedText); }); + + it('should rewrite javascript hrefs as text', async () => { + const samples: Array<[string, string]> = [ + // eslint-disable-next-line no-script-url + ['javascript:alert(1)', 'JS 1'], + [' javascript:alert(2)', 'JS 2 (leading space)'], + ['\n\tjavascript:alert(3)', 'JS 3 (whitespace)'], + // eslint-disable-next-line no-script-url + ['JaVaScRiPt:alert(4)', 'JS 4 (mixed case)'], + ['javascript:alert(5)', 'JS 5 (entity-encoded colon)'], + ]; + + const html = samples + .map(([href, text]) => `${text}`) + .join('\n'); + + const shadowDom = await createTestShadowDom(html, { + preTransformers: [rewriteDocLinks()], + postTransformers: [], + }); + + // There should be no tags, but the link text should remain. + expect(getSample(shadowDom, 'a', 'href')).toEqual([]); + for (const [, text] of samples) { + expect(shadowDom.innerHTML).toContain(text); + } + }); }); describe('normalizeUrl', () => { diff --git a/plugins/techdocs/src/reader/transformers/rewriteDocLinks.ts b/plugins/techdocs/src/reader/transformers/rewriteDocLinks.ts index e43dda1815..a1005fe1d6 100644 --- a/plugins/techdocs/src/reader/transformers/rewriteDocLinks.ts +++ b/plugins/techdocs/src/reader/transformers/rewriteDocLinks.ts @@ -16,6 +16,11 @@ import type { Transformer } from './transformer'; +// See https://github.com/facebook/react/blob/f0cf832e1d0c8544c36aa8b310960885a11a847c/packages/react-dom-bindings/src/shared/sanitizeURL.js +const scriptProtocolPattern = + // eslint-disable-next-line no-control-regex + /^[\u0000-\u001F ]*j[\r\n\t]*a[\r\n\t]*v[\r\n\t]*a[\r\n\t]*s[\r\n\t]*c[\r\n\t]*r[\r\n\t]*i[\r\n\t]*p[\r\n\t]*t[\r\n\t]*\:/i; + export const rewriteDocLinks = (): Transformer => { return dom => { const updateDom = ( @@ -33,6 +38,12 @@ export const rewriteDocLinks = (): Transformer => { } try { + if (scriptProtocolPattern.test(elemAttribute)) { + throw new TypeError( + `Invalid location ref '${elemAttribute}', target is a javascript: URL`, + ); + } + const normalizedWindowLocation = normalizeUrl( window.location.href, ); From cb9e2d11552773a0b323575bbd94daed400c86a8 Mon Sep 17 00:00:00 2001 From: Joshua Rogers Date: Thu, 11 Sep 2025 08:26:59 +0200 Subject: [PATCH 2/3] Revert "techdocs: Disallow javascript URLs" This reverts commit 067fdcd0a5b6b63a689f9fe4fa322c690ed263fb. Signed-off-by: Joshua Rogers --- .changeset/great-adults-stare.md | 5 ---- .../transformers/rewriteDocLinks.test.ts | 27 ------------------- .../reader/transformers/rewriteDocLinks.ts | 11 -------- 3 files changed, 43 deletions(-) delete mode 100644 .changeset/great-adults-stare.md diff --git a/.changeset/great-adults-stare.md b/.changeset/great-adults-stare.md deleted file mode 100644 index 05d196ac62..0000000000 --- a/.changeset/great-adults-stare.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@backstage/plugin-techdocs': minor ---- - -Ensure that techdocs rewritten URLs do not contain potentially dangerous javascript: URLs. diff --git a/plugins/techdocs/src/reader/transformers/rewriteDocLinks.test.ts b/plugins/techdocs/src/reader/transformers/rewriteDocLinks.test.ts index 864129164c..66fc5f29c1 100644 --- a/plugins/techdocs/src/reader/transformers/rewriteDocLinks.test.ts +++ b/plugins/techdocs/src/reader/transformers/rewriteDocLinks.test.ts @@ -71,33 +71,6 @@ describe('rewriteDocLinks', () => { expect(getSample(shadowDom, 'a', 'href')).toEqual([]); expect(shadowDom.innerHTML).toContain(expectedText); }); - - it('should rewrite javascript hrefs as text', async () => { - const samples: Array<[string, string]> = [ - // eslint-disable-next-line no-script-url - ['javascript:alert(1)', 'JS 1'], - [' javascript:alert(2)', 'JS 2 (leading space)'], - ['\n\tjavascript:alert(3)', 'JS 3 (whitespace)'], - // eslint-disable-next-line no-script-url - ['JaVaScRiPt:alert(4)', 'JS 4 (mixed case)'], - ['javascript:alert(5)', 'JS 5 (entity-encoded colon)'], - ]; - - const html = samples - .map(([href, text]) => `${text}`) - .join('\n'); - - const shadowDom = await createTestShadowDom(html, { - preTransformers: [rewriteDocLinks()], - postTransformers: [], - }); - - // There should be no tags, but the link text should remain. - expect(getSample(shadowDom, 'a', 'href')).toEqual([]); - for (const [, text] of samples) { - expect(shadowDom.innerHTML).toContain(text); - } - }); }); describe('normalizeUrl', () => { diff --git a/plugins/techdocs/src/reader/transformers/rewriteDocLinks.ts b/plugins/techdocs/src/reader/transformers/rewriteDocLinks.ts index a1005fe1d6..e43dda1815 100644 --- a/plugins/techdocs/src/reader/transformers/rewriteDocLinks.ts +++ b/plugins/techdocs/src/reader/transformers/rewriteDocLinks.ts @@ -16,11 +16,6 @@ import type { Transformer } from './transformer'; -// See https://github.com/facebook/react/blob/f0cf832e1d0c8544c36aa8b310960885a11a847c/packages/react-dom-bindings/src/shared/sanitizeURL.js -const scriptProtocolPattern = - // eslint-disable-next-line no-control-regex - /^[\u0000-\u001F ]*j[\r\n\t]*a[\r\n\t]*v[\r\n\t]*a[\r\n\t]*s[\r\n\t]*c[\r\n\t]*r[\r\n\t]*i[\r\n\t]*p[\r\n\t]*t[\r\n\t]*\:/i; - export const rewriteDocLinks = (): Transformer => { return dom => { const updateDom = ( @@ -38,12 +33,6 @@ export const rewriteDocLinks = (): Transformer => { } try { - if (scriptProtocolPattern.test(elemAttribute)) { - throw new TypeError( - `Invalid location ref '${elemAttribute}', target is a javascript: URL`, - ); - } - const normalizedWindowLocation = normalizeUrl( window.location.href, ); From b87e54c355dac759cb006c5fd6a34a47b051a196 Mon Sep 17 00:00:00 2001 From: Joshua Rogers Date: Thu, 11 Sep 2025 08:42:51 +0200 Subject: [PATCH 3/3] tests(techdocs): add transformer tests for sanitizing javascript: hrefs Signed-off-by: Joshua Rogers --- .../html/transformer.sanitizer.test.tsx | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/plugins/techdocs/src/reader/transformers/html/transformer.sanitizer.test.tsx b/plugins/techdocs/src/reader/transformers/html/transformer.sanitizer.test.tsx index 39d3d229aa..ed02f86060 100644 --- a/plugins/techdocs/src/reader/transformers/html/transformer.sanitizer.test.tsx +++ b/plugins/techdocs/src/reader/transformers/html/transformer.sanitizer.test.tsx @@ -143,4 +143,35 @@ describe('Transformers > Html > Sanitizer Custom Elements', () => { expect(elements).toHaveLength(1); expect(elements[0].hasAttribute('dominant-baseline')).toBe(true); }); + + it('removes javascript: hrefs while preserving link text', async () => { + const { result } = renderHook(() => useSanitizerTransformer(), { wrapper }); + const dirtyDom = document.createElement('html'); + const dirtyHTML = ` + + + JS 1 + JS 2 (leading space) + JS 3 (whitespace) + + JS 4 (mixed case) + JS 5 (entity-encoded colon) + `; + dirtyDom.innerHTML = dirtyHTML; + const clearDom = await result.current(dirtyDom); // calling html transformer + const elements = Array.from( + clearDom.querySelectorAll('body > a'), + ); + expect(elements).toHaveLength(5); + for (const el of elements) { + // DOMPurify strips the dangerous href attribute + expect(el.getAttribute('href')).toBeNull(); + } + // link text remains + expect(clearDom.textContent).toContain('JS 1'); + expect(clearDom.textContent).toContain('JS 2 (leading space)'); + expect(clearDom.textContent).toContain('JS 3 (whitespace)'); + expect(clearDom.textContent).toContain('JS 4 (mixed case)'); + expect(clearDom.textContent).toContain('JS 5 (entity-encoded colon)'); + }); });