Support array arguments for process credentials (#3048)

Author: mullermp

gems/aws-sdk-core/CHANGELOG.md

@@ -1,6 +1,8 @@
 Unreleased Changes
 ------------------
 
+* Issue - Support an array of string arguments for `Aws::ProcessCredentials` to be executed by `system`.
+
 3.197.0 (2024-06-05)
 ------------------
 

gems/aws-sdk-core/lib/aws-sdk-core/process_credentials.rb

@@ -2,9 +2,15 @@
 
 module Aws
   # A credential provider that executes a given process and attempts
-  # to read its stdout to recieve a JSON payload containing the credentials.
+  # to read its stdout to receive a JSON payload containing the credentials.
   #
-  #     credentials = Aws::ProcessCredentials.new('/usr/bin/credential_proc')
+  #     credentials = Aws::ProcessCredentials.new(['/usr/bin/credential_proc'])
+  #     ec2 = Aws::EC2::Client.new(credentials: credentials)
+  #
+  # Arguments should be provided as strings in the array, for example:
+  #
+  #     process = ['/usr/bin/credential_proc', 'arg1', 'arg2']
+  #     credentials = Aws::ProcessCredentials.new(process)
   #     ec2 = Aws::EC2::Client.new(credentials: credentials)
   #
   # Automatically handles refreshing credentials if an Expiration time is
@@ -19,40 +25,49 @@ class ProcessCredentials
     # Creates a new ProcessCredentials object, which allows an
     # external process to be used as a credential provider.
     #
-    # @param [String] process Invocation string for process
-    # credentials provider.
+    # @param [Array<String>, String] process An array of strings including
+    #  the process name and its arguments to execute, or a single string to be
+    #  executed by the shell (deprecated and insecure).
     def initialize(process)
+      if process.is_a?(String)
+        warn('Passing a single string to Aws::ProcessCredentials.new '\
+             'is insecure, please use use an array of system arguments instead')
+      end
       @process = process
-      @credentials = credentials_from_process(@process)
+      @credentials = credentials_from_process
       @async_refresh = false
 
       super
     end
 
     private
-    def credentials_from_process(proc_invocation)
-      begin
-        raw_out = `#{proc_invocation}`
-        process_status = $?
-      rescue Errno::ENOENT
-        raise Errors::InvalidProcessCredentialsPayload.new("Could not find process #{proc_invocation}")
+
+    def credentials_from_process
+      r, w = IO.pipe
+      success = system(*@process, out: w)
+      w.close
+      raw_out = r.read
+      r.close
+
+      unless success
+        raise Errors::InvalidProcessCredentialsPayload.new(
+          'credential_process provider failure, the credential process had '\
+          'non zero exit status and failed to provide credentials'
+        )
       end
 
-      if process_status.success?
-        begin
-          creds_json = Aws::Json.load(raw_out)
-        rescue Aws::Json::ParseError
-          raise Errors::InvalidProcessCredentialsPayload.new("Invalid JSON response")
-        end
-        payload_version = creds_json['Version']
-        if payload_version == 1
-          _parse_payload_format_v1(creds_json)
-        else
-          raise Errors::InvalidProcessCredentialsPayload.new("Invalid version #{payload_version} for credentials payload")
-        end
-      else
-        raise Errors::InvalidProcessCredentialsPayload.new('credential_process provider failure, the credential process had non zero exit status and failed to provide credentials')
+      begin
+        creds_json = Aws::Json.load(raw_out)
+      rescue Aws::Json::ParseError
+        raise Errors::InvalidProcessCredentialsPayload.new('Invalid JSON response')
       end
+
+      payload_version = creds_json['Version']
+      return _parse_payload_format_v1(creds_json) if payload_version == 1
+
+      raise Errors::InvalidProcessCredentialsPayload.new(
+        "Invalid version #{payload_version} for credentials payload"
+      )
     end
 
     def _parse_payload_format_v1(creds_json)
@@ -64,11 +79,14 @@ def _parse_payload_format_v1(creds_json)
 
       @expiration = creds_json['Expiration'] ? Time.iso8601(creds_json['Expiration']) : nil
       return creds if creds.set?
-      raise Errors::InvalidProcessCredentialsPayload.new("Invalid payload for JSON credentials version 1")
+
+      raise Errors::InvalidProcessCredentialsPayload.new(
+        'Invalid payload for JSON credentials version 1'
+      )
     end
 
     def refresh
-      @credentials = credentials_from_process(@process)
+      @credentials = credentials_from_process
     end
 
     def near_expiration?(expiration_length)

gems/aws-sdk-core/spec/aws/process_credentials_spec.rb

@@ -5,47 +5,87 @@
 module Aws
   describe ProcessCredentials do
 
-    before(:each) do
-      stub_const('ENV', {})
+    before do
       allow(Dir).to receive(:home).and_raise(ArgumentError)
     end
-    
+
     it 'will read credentials from a process' do
-      creds = ProcessCredentials.new('echo \'{"Version":1,"AccessKeyId":"AK_PROC1","SecretAccessKey":"SECRET_AK_PROC1","SessionToken":"TOKEN_PROC1"}\'').credentials
+      process = %w[echo {"Version":1,"AccessKeyId":"AK_PROC1","SecretAccessKey":"SECRET_AK_PROC1","SessionToken":"TOKEN_PROC1"}]
+      creds = ProcessCredentials.new(process).credentials
       expect(creds.access_key_id).to eq('AK_PROC1')
       expect(creds.secret_access_key).to eq('SECRET_AK_PROC1')
       expect(creds.session_token).to eq('TOKEN_PROC1')
     end
 
     it 'will throw an error when invalid JSON is returned' do
+      process = %w[echo {"Version":3,"AccessKeyId":"","SecretAccessKey":""\']
       expect {
-        creds = ProcessCredentials.new('echo \'{"Version":3,"AccessKeyId":"","SecretAccessKey":"","SessionToken":""\'').credentials
+        ProcessCredentials.new(process).credentials
       }.to raise_error(Errors::InvalidProcessCredentialsPayload)
     end
 
-    it 'will throw an error when the process credentials payload version is invalid' do 
+    it 'will throw an error when the process credentials payload version is invalid' do
+      process = %w[echo {"Version":3,"AccessKeyId":"","SecretAccessKey":""}]
       expect {
-        creds = ProcessCredentials.new('echo \'{"Version":3,"AccessKeyId":"","SecretAccessKey":"","SessionToken":""}\'').credentials
+        ProcessCredentials.new(process).credentials
       }.to raise_error(Errors::InvalidProcessCredentialsPayload)
     end
 
-    it 'will throw an error when the process credentials payload is malformed' do 
+    it 'will throw an error when the process credentials payload is malformed' do
+      process = %w[echo {"Version":1}]
       expect {
-        creds = ProcessCredentials.new('echo \'{"Version":1}\'').credentials
+        ProcessCredentials.new(process).credentials
       }.to raise_error(Errors::InvalidProcessCredentialsPayload)
     end
 
-    it 'will throw an error and expose the stderr output when the credential process has a nonzero exit status' do
+    it 'will throw an error when the credential process has a nonzero exit status' do
+      process = ['fake_proc']
       expect {
-        creds = ProcessCredentials.new('>&2 echo "Credential Provider Error"; false').credentials
+        ProcessCredentials.new(process).credentials
       }.to raise_error(Errors::InvalidProcessCredentialsPayload)
-      .and output("Credential Provider Error\n").to_stderr_from_any_process
     end
 
-    it 'will throw an error when the credential process cant be found' do
-      expect {
-        creds = ProcessCredentials.new('fake_proc').credentials
-      }.to raise_error(Errors::InvalidProcessCredentialsPayload)
+    context 'legacy process string' do
+      before do
+        expect_any_instance_of(ProcessCredentials)
+          .to receive(:warn).with(/array of system arguments/)
+      end
+
+      it 'will read credentials from a process' do
+        process = 'echo \'{"Version":1,"AccessKeyId":"AK_PROC1","SecretAccessKey":"SECRET_AK_PROC1","SessionToken":"TOKEN_PROC1"}\''
+        creds = ProcessCredentials.new(process).credentials
+        expect(creds.access_key_id).to eq('AK_PROC1')
+        expect(creds.secret_access_key).to eq('SECRET_AK_PROC1')
+        expect(creds.session_token).to eq('TOKEN_PROC1')
+      end
+
+      it 'will throw an error when invalid JSON is returned' do
+        process = 'echo \'{"Version":3,"AccessKeyId":"","SecretAccessKey":""\''
+        expect {
+          ProcessCredentials.new(process).credentials
+        }.to raise_error(Errors::InvalidProcessCredentialsPayload)
+      end
+
+      it 'will throw an error when the process credentials payload version is invalid' do
+        process = 'echo \'{"Version":3,"AccessKeyId":"","SecretAccessKey":""}\''
+        expect {
+          ProcessCredentials.new(process).credentials
+        }.to raise_error(Errors::InvalidProcessCredentialsPayload)
+      end
+
+      it 'will throw an error when the process credentials payload is malformed' do
+        process = 'echo \'{"Version":1}\''
+        expect {
+          ProcessCredentials.new(process).credentials
+        }.to raise_error(Errors::InvalidProcessCredentialsPayload)
+      end
+
+      it 'will throw an error when the credential process has a nonzero exit status' do
+        process = 'fake_proc'
+        expect {
+          ProcessCredentials.new(process).credentials
+        }.to raise_error(Errors::InvalidProcessCredentialsPayload)
+      end
     end
   end
 end

gems/aws-sdk-core/spec/shared_spec_helper.rb

@@ -23,8 +23,7 @@
 
   config.before(:each) do
     # Clear the current ENV to avoid loading credentials.
-    # This was previously mocked with stub_const but was provided a hash.
-    ENV.clear
+    ENV.keep_if { |k, _| k == 'PATH' }
 
     # disable loading credentials from shared file
     allow(Dir).to receive(:home).and_raise(ArgumentError)