diff options
author | Robert Speicher <robert@gitlab.com> | 2016-09-20 05:42:03 +0000 |
---|---|---|
committer | Ruben Davila <rdavila84@gmail.com> | 2016-09-20 11:07:49 -0500 |
commit | 30e98e486a61ab52573b5b5cdce43966af4268ce (patch) | |
tree | ddd66b931c47d07c053ee7f62eef3a1026a8febd | |
parent | 3d26b41c4a4a8e3cc1b5c6419cd0b2a7f8d4f945 (diff) | |
download | gitlab-ce-30e98e486a61ab52573b5b5cdce43966af4268ce.tar.gz |
Merge branch 'strip-key-comments-for-gitlab-shell' into 'master'
Strip comments before sending keys to gitlab-shell
## Why was this MR needed?
https://gitlab.com/gitlab-org/gitlab-ce/issues/22167 encoding issues in comment text.
## What are the relevant issue numbers?
https://gitlab.com/gitlab-org/gitlab-ce/issues/22167
See merge request !6381
-rw-r--r-- | CHANGELOG | 1 | ||||
-rw-r--r-- | lib/gitlab/backend/shell.rb | 13 | ||||
-rw-r--r-- | spec/lib/gitlab/backend/shell_spec.rb | 32 |
3 files changed, 40 insertions, 6 deletions
diff --git a/CHANGELOG b/CHANGELOG index ab74f6ea03b..a5f0f95bfe0 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -58,6 +58,7 @@ v 8.12.0 (unreleased) - Add hover color to emoji icon (ClemMakesApps) - Increase ci_builds artifacts_size column to 8-byte integer to allow larger files - Add textarea autoresize after comment (ClemMakesApps) + - Do not write SSH public key 'comments' to authorized_keys !6381 - Refresh todos count cache when an Issue/MR is deleted - Fix branches page dropdown sort alignment (ClemMakesApps) - Hides merge request button on branches page is user doesn't have permissions diff --git a/lib/gitlab/backend/shell.rb b/lib/gitlab/backend/shell.rb index c412249a01e..79eac66b364 100644 --- a/lib/gitlab/backend/shell.rb +++ b/lib/gitlab/backend/shell.rb @@ -6,7 +6,12 @@ module Gitlab KeyAdder = Struct.new(:io) do def add_key(id, key) - key.gsub!(/[[:space:]]+/, ' ').strip! + key = Gitlab::Shell.strip_key(key) + # Newline and tab are part of the 'protocol' used to transmit id+key to the other end + if key.include?("\t") || key.include?("\n") + raise Error.new("Invalid key: #{key.inspect}") + end + io.puts("#{id}\t#{key}") end end @@ -16,6 +21,10 @@ module Gitlab @version_required ||= File.read(Rails.root. join('GITLAB_SHELL_VERSION')).strip end + + def strip_key(key) + key.split(/ /)[0, 2].join(' ') + end end # Init new repository @@ -107,7 +116,7 @@ module Gitlab # def add_key(key_id, key_content) Gitlab::Utils.system_silent([gitlab_shell_keys_path, - 'add-key', key_id, key_content]) + 'add-key', key_id, self.class.strip_key(key_content)]) end # Batch-add keys to authorized_keys diff --git a/spec/lib/gitlab/backend/shell_spec.rb b/spec/lib/gitlab/backend/shell_spec.rb index 6e5ba211382..07407f212aa 100644 --- a/spec/lib/gitlab/backend/shell_spec.rb +++ b/spec/lib/gitlab/backend/shell_spec.rb @@ -1,4 +1,5 @@ require 'spec_helper' +require 'stringio' describe Gitlab::Shell, lib: true do let(:project) { double('Project', id: 7, path: 'diaspora') } @@ -44,15 +45,38 @@ describe Gitlab::Shell, lib: true do end end + describe '#add_key' do + it 'removes trailing garbage' do + allow(gitlab_shell).to receive(:gitlab_shell_keys_path).and_return(:gitlab_shell_keys_path) + expect(Gitlab::Utils).to receive(:system_silent).with( + [:gitlab_shell_keys_path, 'add-key', 'key-123', 'ssh-rsa foobar'] + ) + + gitlab_shell.add_key('key-123', 'ssh-rsa foobar trailing garbage') + end + end + describe Gitlab::Shell::KeyAdder, lib: true do describe '#add_key' do - it 'normalizes space characters in the key' do - io = spy + it 'removes trailing garbage' do + io = spy(:io) adder = described_class.new(io) - adder.add_key('key-42', "sha-rsa foo\tbar\tbaz") + adder.add_key('key-42', "ssh-rsa foo bar\tbaz") + + expect(io).to have_received(:puts).with("key-42\tssh-rsa foo") + end + + it 'raises an exception if the key contains a tab' do + expect do + described_class.new(StringIO.new).add_key('key-42', "ssh-rsa\tfoobar") + end.to raise_error(Gitlab::Shell::Error) + end - expect(io).to have_received(:puts).with("key-42\tsha-rsa foo bar baz") + it 'raises an exception if the key contains a newline' do + expect do + described_class.new(StringIO.new).add_key('key-42', "ssh-rsa foobar\nssh-rsa pawned") + end.to raise_error(Gitlab::Shell::Error) end end end |