pass PGPASSWORD via env directly, not via shell

lib/foreman_maintain/concerns/base_database.rb

@@ -55,22 +55,25 @@ def query_csv(sql, config = configuration)
 
       def psql(query, config = configuration)
         if ping(config)
-          execute(psql_command(config),
+          cmd, env = psql_command(config)
+          execute(cmd,
             :stdin => query,
-            :hidden_patterns => [config['password']])
+            :env => env)
         else
           raise_service_error
         end
       end
 
       def ping(config = configuration)
-        execute?(psql_command(config),
+        cmd, env = psql_command(config)
+        execute?(cmd,
           :stdin => 'SELECT 1 as ping',
-          :hidden_patterns => [config['password']])
+          :env => env)
       end
 
       def dump_db(file, config = configuration)
-        execute!(dump_command(config) + " > #{file}", :hidden_patterns => [config['password']])
+        cmd, env = dump_command(config)
+        execute!(cmd + " > #{file}", :env => env)
       end
 
       def restore_dump(file, localdb, config = configuration)
@@ -80,11 +83,10 @@ def restore_dump(file, localdb, config = configuration)
         else
           # TODO: figure out how to completely ignore errors. Currently this
           # sometimes exits with 1 even though errors are ignored by pg_restore
-          dump_cmd = base_command(config, 'pg_restore') +
-                     ' --no-privileges --clean --disable-triggers -n public ' \
-                     "-d #{config['database']} #{file}"
-          execute!(dump_cmd, :hidden_patterns => [config['password']],
-            :valid_exit_statuses => [0, 1])
+          cmd, env = base_command(config, 'pg_restore')
+          cmd += ' --no-privileges --clean --disable-triggers -n public ' \
+                 "-d #{config['database']} #{file}"
+          execute!(cmd, :valid_exit_statuses => [0, 1], :env => env)
         end
       end
 
@@ -125,8 +127,9 @@ def dropdb(config = configuration)
       def db_version(config = configuration)
         if ping(config)
           # Note - t removes headers, -A removes alignment whitespace
-          server_version_cmd = psql_command(config) + ' -c "SHOW server_version" -t -A'
-          version_string = execute!(server_version_cmd, :hidden_patterns => [config['password']])
+          cmd, env = psql_command(config)
+          cmd += ' -c "SHOW server_version" -t -A'
+          version_string = execute!(cmd, :env => env)
           version(version_string)
         else
           raise_service_error
@@ -146,17 +149,20 @@ def raise_psql_missing_error
       private
 
       def base_command(config, command = 'psql')
-        "PGPASSWORD='#{config[%(password)]}' "\
-        "#{command} -h #{config['host'] || 'localhost'} "\
+        env = { 'PGPASSWORD' => config['password'] }
+        cmd = "#{command} -h #{config['host'] || 'localhost'} "\
         " -p #{config['port'] || '5432'} -U #{config['username']}"
+        return cmd, env
       end
 
       def psql_command(config)
-        base_command(config, 'psql') + " -d #{config['database']}"
+        cmd, env = base_command(config, 'psql')
+        return cmd + " -d #{config['database']}", env
       end
 
       def dump_command(config)
-        base_command(config, 'pg_dump') + " -Fc #{config['database']}"
+        cmd, env = base_command(config, 'pg_dump')
+        return cmd + " -Fc #{config['database']}", env
       end
 
       def raise_service_error

lib/foreman_maintain/utils/command_runner.rb

@@ -8,26 +8,27 @@ class CommandRunner
       attr_reader :logger, :command
 
       def initialize(logger, command, options)
-        options.validate_options!(:stdin, :hidden_patterns, :interactive, :valid_exit_statuses)
+        options.validate_options!(:stdin, :interactive, :valid_exit_statuses, :env)
         options[:valid_exit_statuses] ||= [0]
+        options[:env] ||= {}
         @logger = logger
         @command = command
         @stdin = options[:stdin]
-        @hidden_patterns = Array(options[:hidden_patterns]).compact
         @interactive = options[:interactive]
         @options = options
         @valid_exit_statuses = options[:valid_exit_statuses]
+        @env = options[:env]
         raise ArgumentError, 'Can not pass stdin for interactive command' if @interactive && @stdin
       end
 
       def run
-        logger&.debug(hide_strings("Running command #{@command} with stdin #{@stdin.inspect}"))
+        logger&.debug("Running command #{@command} with stdin #{@stdin.inspect}")
         if @interactive
           run_interactively
         else
           run_non_interactively
         end
-        logger&.debug("output of the command:\n #{hide_strings(output)}")
+        logger&.debug("output of the command:\n #{output}")
       end
 
       def interactive?
@@ -49,10 +50,10 @@ def success?
       end
 
       def execution_error
-        raise Error::ExecutionError.new(hide_strings(@command),
+        raise Error::ExecutionError.new(@command,
           exit_status,
-          hide_strings(@stdin),
-          @interactive ? nil : hide_strings(@output))
+          @stdin,
+          @interactive ? nil : @output)
       end
 
       private
@@ -81,7 +82,7 @@ def run_interactively
       end
 
       def run_non_interactively
-        IO.popen(full_command, 'r+') do |f|
+        IO.popen(@env, full_command, 'r+') do |f|
           if @stdin
             f.puts(@stdin)
             f.close_write
@@ -94,13 +95,6 @@ def run_non_interactively
       def full_command
         "#{@command} 2>&1"
       end
-
-      def hide_strings(string)
-        return unless string
-        @hidden_patterns.reduce(string) do |result, hidden_pattern|
-          result.gsub(hidden_pattern, '[FILTERED]')
-        end
-      end
     end
   end
 end

test/lib/utils/command_runner_test.rb

@@ -5,12 +5,7 @@ module ForemanMaintain
     let(:log) { StringIO.new }
     let(:logger) { Logger.new(log) }
 
-    it 'hides passwords in the logs' do
-      command = Utils::CommandRunner.new(logger, 'echo "Password is secret"',
-        :hidden_patterns => [nil, 'secret'])
-      command.run
-      _(log.string).must_match "output of the command:\n Password is [FILTERED]\n"
-      _(log.string).must_match 'Running command echo "Password is [FILTERED]" with stdin nil'
+    it 'has no tests' do
     end
   end
 end