diff options
8 files changed, 40 insertions, 16 deletions
diff --git a/app/assets/javascripts/notebook/cells/output/html.vue b/app/assets/javascripts/notebook/cells/output/html.vue index 2d1d8845e41..fdcea300388 100644 --- a/app/assets/javascripts/notebook/cells/output/html.vue +++ b/app/assets/javascripts/notebook/cells/output/html.vue @@ -40,6 +40,13 @@ export default { <template> <div class="output"> <prompt type="Out" :count="count" :show-output="showOutput" /> - <div v-safe-html:[$options.safeHtmlConfig]="rawCode" class="gl-overflow-auto"></div> + <iframe + sandbox + :srcdoc="rawCode" + frameborder="0" + scrolling="no" + width="100%" + class="gl-overflow-auto" + ></iframe> </div> </template> diff --git a/app/graphql/types/incident_management/timeline_event_type.rb b/app/graphql/types/incident_management/timeline_event_type.rb index a6d3f57404b..690facc8732 100644 --- a/app/graphql/types/incident_management/timeline_event_type.rb +++ b/app/graphql/types/incident_management/timeline_event_type.rb @@ -33,11 +33,6 @@ module Types null: true, description: 'Text note of the timeline event.' - field :note_html, - GraphQL::Types::String, - null: true, - description: 'HTML note of the timeline event.' - field :promoted_from_note, Types::Notes::NoteType, null: true, @@ -67,6 +62,8 @@ module Types Types::TimeType, null: false, description: 'Timestamp when the event updated.' + + markdown_field :note_html, null: true, description: 'HTML note of the timeline event.' end end end diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb index 2d0bc1bc63f..e865db128c1 100644 --- a/app/helpers/labels_helper.rb +++ b/app/helpers/labels_helper.rb @@ -247,7 +247,7 @@ module LabelsHelper class="#{css_class}" data-container="body" data-html="true" - #{"style=\"background-color: #{bg_color}\"" if bg_color} + #{"style=\"background-color: #{h bg_color}\"" if bg_color} >#{ERB::Util.html_escape_once(name)}#{suffix}</span> HTML end diff --git a/lib/gitlab/markdown_cache.rb b/lib/gitlab/markdown_cache.rb index 09ba95666de..f426f70800c 100644 --- a/lib/gitlab/markdown_cache.rb +++ b/lib/gitlab/markdown_cache.rb @@ -11,7 +11,7 @@ module Gitlab # this if the change to the renderer output is a new feature or a # minor bug fix. # See: https://gitlab.com/gitlab-org/gitlab/-/issues/330313 - CACHE_COMMONMARK_VERSION = 31 + CACHE_COMMONMARK_VERSION = 32 CACHE_COMMONMARK_VERSION_START = 10 BaseError = Class.new(StandardError) diff --git a/spec/frontend/notebook/cells/output/html_sanitize_fixtures.js b/spec/frontend/notebook/cells/output/html_sanitize_fixtures.js index 70c7f56b62f..296d01ddd99 100644 --- a/spec/frontend/notebook/cells/output/html_sanitize_fixtures.js +++ b/spec/frontend/notebook/cells/output/html_sanitize_fixtures.js @@ -38,7 +38,7 @@ export default [ '</tr>\n', '</table>', ].join(''), - output: '<table>', + output: '<table data-myattr="XSS">', }, ], // Note: style is sanitized out @@ -98,7 +98,7 @@ export default [ '</svg>', ].join(), output: - '<svg xmlns="http://www.w3.org/2000/svg" width="388.84pt" version="1.0" id="svg2" height="115.02pt">', + '<svg height="115.02pt" id="svg2" version="1.0" width="388.84pt" xmlns="http://www.w3.org/2000/svg">', }, ], ]; diff --git a/spec/frontend/notebook/cells/output/index_spec.js b/spec/frontend/notebook/cells/output/index_spec.js index 4d1d03e5e34..97a7e22be60 100644 --- a/spec/frontend/notebook/cells/output/index_spec.js +++ b/spec/frontend/notebook/cells/output/index_spec.js @@ -49,15 +49,17 @@ describe('Output component', () => { const htmlType = json.cells[4]; createComponent(htmlType.outputs[0]); - expect(wrapper.findAll('p')).toHaveLength(1); - expect(wrapper.text()).toContain('test'); + const iframe = wrapper.find('iframe'); + expect(iframe.exists()).toBe(true); + expect(iframe.element.getAttribute('sandbox')).toBe(''); + expect(iframe.element.getAttribute('srcdoc')).toBe('<p>test</p>'); }); it('renders multiple raw HTML outputs', () => { const htmlType = json.cells[4]; createComponent([htmlType.outputs[0], htmlType.outputs[0]]); - expect(wrapper.findAll('p')).toHaveLength(2); + expect(wrapper.findAll('iframe')).toHaveLength(2); }); }); @@ -84,7 +86,11 @@ describe('Output component', () => { }); it('renders as an svg', () => { - expect(wrapper.find('svg').exists()).toBe(true); + const iframe = wrapper.find('iframe'); + + expect(iframe.exists()).toBe(true); + expect(iframe.element.getAttribute('sandbox')).toBe(''); + expect(iframe.element.getAttribute('srcdoc')).toBe('<svg></svg>'); }); }); diff --git a/spec/helpers/labels_helper_spec.rb b/spec/helpers/labels_helper_spec.rb index 5efa88a2a7d..90366d7772c 100644 --- a/spec/helpers/labels_helper_spec.rb +++ b/spec/helpers/labels_helper_spec.rb @@ -112,6 +112,14 @@ RSpec.describe LabelsHelper do end end + describe 'render_label_text' do + it 'html escapes the bg_color correctly' do + xss_payload = '"><img src=x onerror=prompt(1)>' + label_text = render_label_text('xss', bg_color: xss_payload) + expect(label_text).to include(html_escape(xss_payload)) + end + end + describe 'text_color_for_bg' do it 'uses light text on dark backgrounds' do expect(text_color_for_bg('#222E2E')).to be_color('#FFFFFF') diff --git a/spec/requests/api/graphql/project/incident_management/timeline_events_spec.rb b/spec/requests/api/graphql/project/incident_management/timeline_events_spec.rb index 31fef75f679..bcbb1f11d43 100644 --- a/spec/requests/api/graphql/project/incident_management/timeline_events_spec.rb +++ b/spec/requests/api/graphql/project/incident_management/timeline_events_spec.rb @@ -6,11 +6,16 @@ RSpec.describe 'getting incident timeline events' do include GraphqlHelpers let_it_be(:project) { create(:project) } + let_it_be(:private_project) { create(:project, :private) } + let_it_be(:issue) { create(:issue, project: private_project) } let_it_be(:current_user) { create(:user) } let_it_be(:updated_by_user) { create(:user) } let_it_be(:incident) { create(:incident, project: project) } let_it_be(:another_incident) { create(:incident, project: project) } let_it_be(:promoted_from_note) { create(:note, project: project, noteable: incident) } + let_it_be(:issue_url) { project_issue_url(private_project, issue) } + let_it_be(:issue_ref) { "#{private_project.full_path}##{issue.iid}" } + let_it_be(:issue_link) { %Q(<a href="#{issue_url}">#{issue_url}</a>) } let_it_be(:timeline_event) do create( @@ -18,7 +23,8 @@ RSpec.describe 'getting incident timeline events' do incident: incident, project: project, updated_by_user: updated_by_user, - promoted_from_note: promoted_from_note + promoted_from_note: promoted_from_note, + note: "Referencing #{issue.to_reference(full: true)} - Full URL #{issue_url}" ) end @@ -89,7 +95,7 @@ RSpec.describe 'getting incident timeline events' do 'title' => incident.title }, 'note' => timeline_event.note, - 'noteHtml' => timeline_event.note_html, + 'noteHtml' => "<p>Referencing #{issue_ref} - Full URL #{issue_link}</p>", 'promotedFromNote' => { 'id' => promoted_from_note.to_global_id.to_s, 'body' => promoted_from_note.note |