Error on newlines in sidekiq-cluster arguments

When using Helm charts, which are YAML files, it's relatively easy to
accidentally include a newline in the Sidekiq arguments. This will
typically be a trailing newline, which shouldn't be a problem, except
that in our cloud-native deployment we launch Sidekiq using the command
given by `sidekiq-cluster --dryrun` (to avoid having the wrapper
process), so we effectively do:

    $(bin/sidekiq-cluster --dryrun $ARGS)

Ruby's `Shellwords.escape` quotes newlines as `'\n'`:
https://github.com/ruby/shellwords/blob/v0.1.0/lib/shellwords.rb#L161-L163

When those are parsed back in the arguments array we get `foo'\n'` for
an original argument of `foo\n`. Then, when we apply splitting, we get
`foo'` and `'` as two separate arguments.

The quoting here is quite complicated but the below demonstrates (in
Bash) what's happening:

    $ ruby -e 'p ARGV; p ARGV.first.split' "foo'"$'\n'"'"
    ["foo'\n'"]
    ["foo'", "'"]

If we did care about newlines in this argument array, the solution would
be more involved. But we don't, because they are never valid, so we can
just error when we encounter them.

Changelog: fixed

lib/gitlab/sidekiq_cluster/cli.rb

@@ -57,6 +57,11 @@ module Gitlab
         worker_queues = SidekiqConfig::CliMethods.worker_queues(@rails_path)
 
         queue_groups = argv.map do |queues_or_query_string|
+          if queues_or_query_string =~ /[\r\n]/
+            raise CommandError,
+              'The queue arguments cannot contain newlines'
+          end
+
           next worker_queues if queues_or_query_string == SidekiqConfig::WorkerMatcher::WILDCARD_MATCH
 
           # When using the queue query syntax, we treat each queue group

spec/bin/sidekiq_cluster_spec.rb

 # frozen_string_literal: true
 
-require 'spec_helper'
+require 'fast_spec_helper'
 require 'shellwords'
+require 'rspec-parameterized'
 
-RSpec.describe 'bin/sidekiq-cluster' do
+RSpec.describe 'bin/sidekiq-cluster', :aggregate_failures do
   using RSpec::Parameterized::TableSyntax
 
+  let(:root) { File.expand_path('../..', __dir__) }
+
   context 'when selecting some queues and excluding others' do
     where(:args, :included, :excluded) do
       %w[--negate cronjob] | '-qdefault,1' | '-qcronjob,1'
@@ -13,10 +16,10 @@ RSpec.describe 'bin/sidekiq-cluster' do
     end
 
     with_them do
-      it 'runs successfully', :aggregate_failures do
+      it 'runs successfully' do
         cmd = %w[bin/sidekiq-cluster --dryrun] + args
 
-        output, status = Gitlab::Popen.popen(cmd, Rails.root.to_s)
+        output, status = Gitlab::Popen.popen(cmd, root)
 
         expect(status).to be(0)
         expect(output).to include('bundle exec sidekiq')
@@ -31,10 +34,10 @@ RSpec.describe 'bin/sidekiq-cluster' do
       %w[*],
       %w[--queue-selector *]
     ].each do |args|
-      it "runs successfully with `#{args}`", :aggregate_failures do
+      it "runs successfully with `#{args}`" do
         cmd = %w[bin/sidekiq-cluster --dryrun] + args
 
-        output, status = Gitlab::Popen.popen(cmd, Rails.root.to_s)
+        output, status = Gitlab::Popen.popen(cmd, root)
 
         expect(status).to be(0)
         expect(output).to include('bundle exec sidekiq')
@@ -43,4 +46,20 @@ RSpec.describe 'bin/sidekiq-cluster' do
       end
     end
   end
+
+  context 'when arguments contain newlines' do
+    it 'raises an error' do
+      [
+        ["default\n"],
+        ["defaul\nt"]
+      ].each do |args|
+        cmd = %w[bin/sidekiq-cluster --dryrun] + args
+
+        output, status = Gitlab::Popen.popen(cmd, root)
+
+        expect(status).to be(1)
+        expect(output).to include('cannot contain newlines')
+      end
+    end
+  end
 end

spec/lib/gitlab/sidekiq_cluster/cli_spec.rb

@@ -48,6 +48,18 @@ RSpec.describe Gitlab::SidekiqCluster::CLI do
         cli.run(%w(*))
       end
 
+      it 'raises an error when the arguments contain newlines' do
+        invalid_arguments = [
+          ["foo\n"],
+          ["foo\r"],
+          %W[foo b\nar]
+        ]
+
+        invalid_arguments.each do |arguments|
+          expect { cli.run(arguments) }.to raise_error(described_class::CommandError)
+        end
+      end
+
       context 'with --negate flag' do
         it 'starts Sidekiq workers for all queues in all_queues.yml except the ones in argv' do
           expect(Gitlab::SidekiqConfig::CliMethods).to receive(:worker_queues).and_return(['baz'])