From 1b11ceeb5582394a7dd01c8da185d3dad080530e Mon Sep 17 00:00:00 2001 From: Brett Walker Date: Tue, 30 Sep 2025 16:54:58 -0500 Subject: [PATCH 1/2] Iniital step in rendering an iframe --- .../behaviors/markdown/render_gfm.js | 5 ++ .../markdown/render_sandboxed_iframe.js | 67 +++++++++++++++++++ .../content_security_policy/config_loader.rb | 8 +++ 3 files changed, 80 insertions(+) create mode 100644 app/assets/javascripts/behaviors/markdown/render_sandboxed_iframe.js diff --git a/app/assets/javascripts/behaviors/markdown/render_gfm.js b/app/assets/javascripts/behaviors/markdown/render_gfm.js index 4d3d9fd91fef4b..89956b1d736aa4 100644 --- a/app/assets/javascripts/behaviors/markdown/render_gfm.js +++ b/app/assets/javascripts/behaviors/markdown/render_gfm.js @@ -3,6 +3,7 @@ import highlightCurrentUser from './highlight_current_user'; import { renderKroki } from './render_kroki'; import renderMath from './render_math'; import renderSandboxedMermaid from './render_sandboxed_mermaid'; +import renderSandboxedIframe from './render_sandboxed_iframe'; import { renderGlql } from './render_glql'; import { renderJSONTable, renderJSONTableHTML } from './render_json_table'; import { addAriaLabels } from './accessibility'; @@ -28,6 +29,7 @@ export function renderGFM(element) { krokiEls, mathEls, mermaidEls, + iframeEls, tableEls, tableHTMLEls, glqlEls, @@ -40,6 +42,7 @@ export function renderGFM(element) { '.js-render-kroki[hidden]', '.js-render-math', '.js-render-mermaid', + '.js-render-iframe', '[data-canonical-lang="json"][data-lang-params~="table"]', 'table[data-table-fields]', '[data-canonical-lang="glql"]', @@ -50,10 +53,12 @@ export function renderGFM(element) { 'a>img', ].map((selector) => Array.from(element.querySelectorAll(selector))); + debugger; syntaxHighlight(highlightEls); renderKroki(krokiEls); renderMath(mathEls); renderSandboxedMermaid(mermaidEls); + renderSandboxedIframe(iframeEls); renderJSONTable(tableEls.map((e) => e.parentNode)); renderJSONTableHTML(tableHTMLEls); highlightCurrentUser(userEls); diff --git a/app/assets/javascripts/behaviors/markdown/render_sandboxed_iframe.js b/app/assets/javascripts/behaviors/markdown/render_sandboxed_iframe.js new file mode 100644 index 00000000000000..0949abaf82a8aa --- /dev/null +++ b/app/assets/javascripts/behaviors/markdown/render_sandboxed_iframe.js @@ -0,0 +1,67 @@ +import { setAttributes } from '~/lib/utils/dom_utils'; + +const SANDBOX_ATTRIBUTES = 'allow-scripts allow-popups allow-same-origin'; + +// Keep a map of iframes we've already rendered. +const elsProcessingMap = new WeakMap(); +let renderedIframeBlocks = 0; + +function renderIframeEl(el) { + debugger; + const iframeEl = document.createElement('iframe'); + setAttributes(iframeEl, { + src: el.src, + sandbox: SANDBOX_ATTRIBUTES, + frameBorder: 0, + // scrolling: 'no', + // width: '1000px', + class: 'gl-inset-0 gl-h-full gl-w-full', + allowfullscreen: 'true', + referrerpolicy: 'strict-origin-when-cross-origin', + }); + + const wrapper = document.createElement('div'); + wrapper.appendChild(iframeEl); + + const container = el.closest('.media-container'); + while (container.firstChild) { + container.removeChild(container.firstChild); + } + container.appendChild(wrapper); + + // Event Listeners + iframeEl.addEventListener('load', () => { + // Potential risk associated with '*' discussed in below thread + // https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74414#note_735183398 + iframeEl.contentWindow.postMessage(source, '*'); + }); + + window.addEventListener( + 'message', + (event) => { + if (event.origin !== 'null' || event.source !== iframeEl.contentWindow) { + return; + } + const { h } = event.data; + iframeEl.height = `${h + BUFFER_IFRAME_HEIGHT}px`; + }, + false, + ); +} + +export default function renderIframes(els) { + if (!els.length) return; + + els.forEach((el) => { + // Skipping all the elements which we've already queued in requestIdleCallback + if (elsProcessingMap.has(el)) { + return; + } + + const requestId = window.requestIdleCallback(() => { + renderIframeEl(el); + }); + + elsProcessingMap.set(el, requestId); + }); +} diff --git a/lib/gitlab/content_security_policy/config_loader.rb b/lib/gitlab/content_security_policy/config_loader.rb index 26c844cc1fe8a0..a59b27bb4716f7 100644 --- a/lib/gitlab/content_security_policy/config_loader.rb +++ b/lib/gitlab/content_security_policy/config_loader.rb @@ -28,6 +28,7 @@ def default_directives allow_sentry(directives) allow_framed_gitlab_paths(directives) allow_customersdot(directives) + allow_iframes(directives) csp_level_3_backport(directives) directives @@ -168,6 +169,13 @@ def allow_customersdot(directives) append_to_directive(directives, 'frame_src', customersdot_host) end + def allow_iframes(directives) + # return unless Gitlab::CurrentSettings.allow_iframes_in_markdown? + + # append_to_directive(directives, 'frame_src', Gitlab::CurrentSettings.iframe_src_allowlist.join(' ')) + append_to_directive(directives, 'frame_src', 'https://www.youtube.com/ https://www.figma.com/') + end + # The follow contains workarounds to patch Safari's lack of support for CSP Level 3 def csp_level_3_backport(directives) # See https://gitlab.com/gitlab-org/gitlab/-/issues/343579 -- GitLab From 46218cdf1c166ccd141d12b3746c502dab9a9f43 Mon Sep 17 00:00:00 2001 From: Asherah Connor Date: Wed, 22 Oct 2025 14:13:34 +1100 Subject: [PATCH 2/2] Small cleanup; remove Mermaid-specific code I've renamed it to 'render_iframe.js'. 'render_sandboxed_mermaid.js' actually renders in *our* sandbox, /-/sandbox/mermaid, whereas we just use an iframe's regular sandbox controls. --- .../behaviors/markdown/render_gfm.js | 50 +++++++------------ ...r_sandboxed_iframe.js => render_iframe.js} | 30 ++--------- lib/banzai/filter/iframe_link_filter.rb | 9 +++- 3 files changed, 29 insertions(+), 60 deletions(-) rename app/assets/javascripts/behaviors/markdown/{render_sandboxed_iframe.js => render_iframe.js} (51%) diff --git a/app/assets/javascripts/behaviors/markdown/render_gfm.js b/app/assets/javascripts/behaviors/markdown/render_gfm.js index 89956b1d736aa4..4627b3e58f0dc7 100644 --- a/app/assets/javascripts/behaviors/markdown/render_gfm.js +++ b/app/assets/javascripts/behaviors/markdown/render_gfm.js @@ -3,7 +3,7 @@ import highlightCurrentUser from './highlight_current_user'; import { renderKroki } from './render_kroki'; import renderMath from './render_math'; import renderSandboxedMermaid from './render_sandboxed_mermaid'; -import renderSandboxedIframe from './render_sandboxed_iframe'; +import renderSandboxedIframe from './render_iframe'; import { renderGlql } from './render_glql'; import { renderJSONTable, renderJSONTableHTML } from './render_json_table'; import { addAriaLabels } from './accessibility'; @@ -18,42 +18,30 @@ function initPopovers(elements) { .catch(() => {}); } -// Render GitLab flavored Markdown +// Render GitLab Flavored Markdown export function renderGFM(element) { if (!element) { return; } - const [ - highlightEls, - krokiEls, - mathEls, - mermaidEls, - iframeEls, - tableEls, - tableHTMLEls, - glqlEls, - userEls, - popoverEls, - taskListCheckboxEls, - imageEls, - ] = [ - '.js-syntax-highlight', - '.js-render-kroki[hidden]', - '.js-render-math', - '.js-render-mermaid', - '.js-render-iframe', - '[data-canonical-lang="json"][data-lang-params~="table"]', - 'table[data-table-fields]', - '[data-canonical-lang="glql"]', - '.gfm-project_member', - '.gfm-issue, .gfm-work_item, .gfm-merge_request, .gfm-epic, .gfm-milestone', - '.task-list-item-checkbox', - // eslint-disable-next-line @gitlab/require-i18n-strings - 'a>img', - ].map((selector) => Array.from(element.querySelectorAll(selector))); + function arrayFromAll(selector) { + return Array.from(element.querySelectorAll(selector)); + } + + const highlightEls = arrayFromAll('.js-syntax-highlight'); + const krokiEls = arrayFromAll('.js-render-kroki[hidden]'); + const mathEls = arrayFromAll('.js-render-math'); + const mermaidEls = arrayFromAll('.js-render-mermaid'); + const iframeEls = arrayFromAll('.js-render-iframe'); + const tableEls = arrayFromAll('[data-canonical-lang="json"][data-lang-params~="table"]'); + const tableHTMLEls = arrayFromAll('table[data-table-fields]'); + const glqlEls = arrayFromAll('[data-canonical-lang="glql"]'); + const userEls = arrayFromAll('.gfm-project_member'); + const popoverEls = arrayFromAll('.gfm-issue, .gfm-work_item, .gfm-merge_request, .gfm-epic, .gfm-milestone'); + const taskListCheckboxEls = arrayFromAll('.task-list-item-checkbox'); + // eslint-disable-next-line @gitlab/require-i18n-strings + const imageEls = arrayFromAll('a>img'); - debugger; syntaxHighlight(highlightEls); renderKroki(krokiEls); renderMath(mathEls); diff --git a/app/assets/javascripts/behaviors/markdown/render_sandboxed_iframe.js b/app/assets/javascripts/behaviors/markdown/render_iframe.js similarity index 51% rename from app/assets/javascripts/behaviors/markdown/render_sandboxed_iframe.js rename to app/assets/javascripts/behaviors/markdown/render_iframe.js index 0949abaf82a8aa..dc1d26c9623b68 100644 --- a/app/assets/javascripts/behaviors/markdown/render_sandboxed_iframe.js +++ b/app/assets/javascripts/behaviors/markdown/render_iframe.js @@ -1,20 +1,16 @@ import { setAttributes } from '~/lib/utils/dom_utils'; -const SANDBOX_ATTRIBUTES = 'allow-scripts allow-popups allow-same-origin'; +// https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/iframe#sandbox +const IFRAME_SANDBOX_RESTRICTIONS = 'allow-scripts allow-popups allow-same-origin'; -// Keep a map of iframes we've already rendered. const elsProcessingMap = new WeakMap(); -let renderedIframeBlocks = 0; function renderIframeEl(el) { - debugger; const iframeEl = document.createElement('iframe'); setAttributes(iframeEl, { src: el.src, - sandbox: SANDBOX_ATTRIBUTES, + sandbox: IFRAME_SANDBOX_RESTRICTIONS, frameBorder: 0, - // scrolling: 'no', - // width: '1000px', class: 'gl-inset-0 gl-h-full gl-w-full', allowfullscreen: 'true', referrerpolicy: 'strict-origin-when-cross-origin', @@ -28,32 +24,12 @@ function renderIframeEl(el) { container.removeChild(container.firstChild); } container.appendChild(wrapper); - - // Event Listeners - iframeEl.addEventListener('load', () => { - // Potential risk associated with '*' discussed in below thread - // https://gitlab.com/gitlab-org/gitlab/-/merge_requests/74414#note_735183398 - iframeEl.contentWindow.postMessage(source, '*'); - }); - - window.addEventListener( - 'message', - (event) => { - if (event.origin !== 'null' || event.source !== iframeEl.contentWindow) { - return; - } - const { h } = event.data; - iframeEl.height = `${h + BUFFER_IFRAME_HEIGHT}px`; - }, - false, - ); } export default function renderIframes(els) { if (!els.length) return; els.forEach((el) => { - // Skipping all the elements which we've already queued in requestIdleCallback if (elsProcessingMap.has(el)) { return; } diff --git a/lib/banzai/filter/iframe_link_filter.rb b/lib/banzai/filter/iframe_link_filter.rb index 5c355c5b502d6d..574b2970f77d37 100644 --- a/lib/banzai/filter/iframe_link_filter.rb +++ b/lib/banzai/filter/iframe_link_filter.rb @@ -19,10 +19,15 @@ def media_type 'img' end + # Note that every value _must_ contain at least one forward slash `/` in it + # separating the host from the path, and _should_ contain a final `/` or `?` + # if possible. Otherwise, e.g. if just "www.youtube.com" was given, a user + # could embed "https://www.youtube.com.my.malicious.domain.info/anything/". def safe_media_ext # TODO: will change to use the administrator defined allow list # Gitlab::CurrentSettings.iframe_src_allowlist - ['www.youtube.com/embed'] + + ['www.youtube.com/embed/'] end override :has_allowed_media? @@ -34,7 +39,7 @@ def has_allowed_media?(element) return unless src.present? - src.start_with?('https://') && safe_media_ext.any? { |domain| src.start_with?("https://#{domain}") } + src.start_with?('https://') && safe_media_ext.any? { |url_prefix| src.start_with?("https://#{url_prefix}") } end def extra_element_attrs(element) -- GitLab