DEV: Raise exception when execute_command will spawn a shell (#12716)

DEV: Raise exception when execute_command will spawn a shell (#12716)

diff --git a/lib/discourse.rb b/lib/discourse.rb
index b13f785..34aca6d 100644
--- a/lib/discourse.rb
+++ b/lib/discourse.rb
@@ -95,10 +95,16 @@ module Discourse
 
       private
 
-      def execute_command(*command, timeout: nil, failure_message: "", success_status_codes: [0], chdir: ".")
+      def execute_command(*command, timeout: nil, failure_message: "", success_status_codes: [0], chdir: ".", unsafe_shell: false)
         env = nil
         env = command.shift if command[0].is_a?(Hash)
 
+        if !unsafe_shell && (command.length == 1) && command[0].include?(" ")
+          # Sending a single string to Process.spawn will launch a shell
+          # This means various things (e.g. subshells) are possible, and could present injection risk
+          raise "Arguments should be provided as separate strings"
+        end
+
         if timeout
           # will send a TERM after timeout
           # will send a KILL after timeout * 2
diff --git a/spec/components/discourse_spec.rb b/spec/components/discourse_spec.rb
index cc05124..99c458f 100644
--- a/spec/components/discourse_spec.rb
+++ b/spec/components/discourse_spec.rb
@@ -438,6 +438,21 @@ describe Discourse do
       has_checked_chdir = true
       thread.join
     end
+
+    it "raises error for unsafe shell" do
+      expect(Discourse::Utils.execute_command("pwd").strip).to eq(Rails.root.to_s)
+
+      expect do
+        Discourse::Utils.execute_command("echo a b c")
+      end.to raise_error(RuntimeError)
+
+      expect do
+        Discourse::Utils.execute_command({ "ENV1" => "VAL" }, "echo a b c")
+      end.to raise_error(RuntimeError)
+
+      expect(Discourse::Utils.execute_command("echo", "a", "b", "c").strip).to eq("a b c")
+      expect(Discourse::Utils.execute_command("echo a b c", unsafe_shell: true).strip).to eq("a b c")
+    end
   end
 
 end

GitHub sha: 0ec5fd52

This commit appears in #12716 which was approved by eviltrout. It was merged by davidtaylorhq.