summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRobert Speicher <robert@gitlab.com>2016-09-28 15:02:12 +0000
committerRémy Coutable <remy@rymai.me>2016-09-28 17:50:04 +0200
commit23c297e781da2f8584fd47aecc2544e1a01b886f (patch)
tree1d2f37ab13234704351f814175ce87116863b752
parent4f1a1bbc2b0501dd7ba227597e115517e3a1fb3f (diff)
downloadgitlab-ce-23c297e781da2f8584fd47aecc2544e1a01b886f.tar.gz
Merge branch 'fix/escape-builds-commands-in-ci-linter' into 'security'
Escape HTML nodes in builds commands in ci linter This MR removes call to `simple_format` that behaves like `String#html_safe`, thus it passes unescaped HTML tags to the view. Closes #22541 See merge request !2001 Signed-off-by: Rémy Coutable <remy@rymai.me>
-rw-r--r--CHANGELOG1
-rw-r--r--app/views/ci/lints/_create.html.haml3
-rw-r--r--spec/views/ci/lints/show.html.haml_spec.rb35
3 files changed, 37 insertions, 2 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 81f9ea2de5d..ae6b9114a75 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -4,6 +4,7 @@ v 8.11.8
- Respect the fork_project permission when forking projects
- Set a restrictive CORS policy on the API for credentialed requests
- API: disable rails session auth for non-GET/HEAD requests
+ - Escape HTML nodes in builds commands in CI linter
v 8.11.7
- Avoid conflict with admin labels when importing GitHub labels. !6158
diff --git a/app/views/ci/lints/_create.html.haml b/app/views/ci/lints/_create.html.haml
index f7875e68b7e..1545c00af45 100644
--- a/app/views/ci/lints/_create.html.haml
+++ b/app/views/ci/lints/_create.html.haml
@@ -16,8 +16,7 @@
%tr
%td #{stage.capitalize} Job - #{build[:name]}
%td
- %pre
- = simple_format build[:commands]
+ %pre= build[:commands]
%br
%b Tag list:
diff --git a/spec/views/ci/lints/show.html.haml_spec.rb b/spec/views/ci/lints/show.html.haml_spec.rb
new file mode 100644
index 00000000000..3a65a86cd88
--- /dev/null
+++ b/spec/views/ci/lints/show.html.haml_spec.rb
@@ -0,0 +1,35 @@
+require 'spec_helper'
+
+describe 'ci/lints/show' do
+ include Devise::TestHelpers
+
+ before do
+ assign(:status, true)
+ assign(:stages, %w[test])
+ assign(:builds, builds)
+ end
+
+ context 'when builds attrbiutes contain HTML nodes' do
+ let(:builds) do
+ [ { name: 'rspec', stage: 'test', commands: '<h1>rspec</h1>' } ]
+ end
+
+ it 'does not render HTML elements' do
+ render
+
+ expect(rendered).not_to have_css('h1', text: 'rspec')
+ end
+ end
+
+ context 'when builds attributes do not contain HTML nodes' do
+ let(:builds) do
+ [ { name: 'rspec', stage: 'test', commands: 'rspec' } ]
+ end
+
+ it 'shows configuration in the table' do
+ render
+
+ expect(rendered).to have_css('td pre', text: 'rspec')
+ end
+ end
+end