Process credentials list of strings args (#5033)

* Process credentials list of strings args

* Address comments

* Deprecate single-string command setter

* Update javadocs

* Update javadocs

* Add unit test

core/auth/src/main/java/software/amazon/awssdk/auth/credentials/ProcessCredentialsProvider.java

@@ -77,30 +77,19 @@ public final class ProcessCredentialsProvider
 
     private final String commandFromBuilder;
 
+    private final List<String> commandAsListOfStringsFromBuilder;
+
     private final Boolean asyncCredentialUpdateEnabled;
 
     /**
      * @see #builder()
      */
     private ProcessCredentialsProvider(Builder builder) {
-        List<String> cmd = new ArrayList<>();
-
-        if (Platform.isWindows()) {
-            cmd.add("cmd.exe");
-            cmd.add("/C");
-        } else {
-            cmd.add("sh");
-            cmd.add("-c");
-        }
-
-        String builderCommand = Validate.paramNotNull(builder.command, "command");
-
-        cmd.add(builderCommand);
-
-        this.executableCommand = Collections.unmodifiableList(cmd);
+        this.executableCommand = executableCommand(builder);
         this.processOutputLimit = Validate.isPositive(builder.processOutputLimit, "processOutputLimit");
         this.credentialRefreshThreshold = Validate.isPositive(builder.credentialRefreshThreshold, "expirationBuffer");
         this.commandFromBuilder = builder.command;
+        this.commandAsListOfStringsFromBuilder = builder.commandAsListOfStrings;
         this.asyncCredentialUpdateEnabled = builder.asyncCredentialUpdateEnabled;
 
         CachedSupplier.Builder<AwsCredentials> cacheBuilder = CachedSupplier.builder(this::refreshCredentials)
@@ -112,6 +101,26 @@ private ProcessCredentialsProvider(Builder builder) {
         this.processCredentialCache = cacheBuilder.build();
     }
 
+    private List<String> executableCommand(Builder builder) {
+        if (builder.commandAsListOfStrings != null) {
+            return Collections.unmodifiableList(builder.commandAsListOfStrings);
+        } else {
+            List<String> cmd = new ArrayList<>();
+
+            if (Platform.isWindows()) {
+                cmd.add("cmd.exe");
+                cmd.add("/C");
+            } else {
+                cmd.add("sh");
+                cmd.add("-c");
+            }
+
+            String builderCommand = Validate.paramNotNull(builder.command, "command");
+            cmd.add(builderCommand);
+            return Collections.unmodifiableList(cmd);
+        }
+    }
+
     /**
      * Retrieve a new builder that can be used to create and configure a {@link ProcessCredentialsProvider}.
      */
@@ -249,6 +258,7 @@ public Builder toBuilder() {
     public static class Builder implements CopyableBuilder<Builder, ProcessCredentialsProvider> {
         private Boolean asyncCredentialUpdateEnabled = false;
         private String command;
+        private List<String> commandAsListOfStrings;
         private Duration credentialRefreshThreshold = Duration.ofSeconds(15);
         private long processOutputLimit = 64000;
 
@@ -261,6 +271,7 @@ private Builder() {
         private Builder(ProcessCredentialsProvider provider) {
             this.asyncCredentialUpdateEnabled = provider.asyncCredentialUpdateEnabled;
             this.command = provider.commandFromBuilder;
+            this.commandAsListOfStrings = provider.commandAsListOfStringsFromBuilder;
             this.credentialRefreshThreshold = provider.credentialRefreshThreshold;
             this.processOutputLimit = provider.processOutputLimit;
         }
@@ -280,12 +291,27 @@ public Builder asyncCredentialUpdateEnabled(Boolean asyncCredentialUpdateEnabled
 
         /**
          * Configure the command that should be executed to retrieve credentials.
+         * See {@link ProcessBuilder} for details on how this command is used.
+         *
+         * @deprecated The recommended approach is to specify the command as a list of Strings, using {@link #command(List)}
+         * instead, which makes it easier to programmatically add parameters to commands without needing to escape those
+         * parameters to protect against command injection.
          */
+        @Deprecated
         public Builder command(String command) {
             this.command = command;
             return this;
         }
 
+        /**
+         * Configure the command that should be executed to retrieve credentials, as a list of strings.
+         * See {@link ProcessBuilder} for details on how this command is used.
+         */
+        public Builder command(List<String> commandAsListOfStrings) {
+            this.commandAsListOfStrings = commandAsListOfStrings;
+            return this;
+        }
+
         /**
          * Configure the amount of time between when the credentials expire and when the credentials should start to be
          * refreshed. This allows the credentials to be refreshed *before* they are reported to expire.

core/auth/src/test/java/software/amazon/awssdk/auth/credentials/ProcessCredentialsProviderTest.java

@@ -20,8 +20,10 @@
 import static software.amazon.awssdk.auth.credentials.internal.ProcessCredentialsTestUtils.copyHappyCaseProcessCredentialsScript;
 
 import java.io.File;
+import java.io.IOException;
 import java.time.Duration;
 import java.time.Instant;
+import java.util.Arrays;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.Test;
@@ -64,6 +66,20 @@ void staticCredentialsCanBeLoaded() {
         assertThat(credentials.secretAccessKey()).isEqualTo("secretAccessKey");
         assertThat(credentials.providerName()).isPresent().contains("ProcessCredentialsProvider");
     }
+
+    @Test
+    public void staticCredentials_commandAsListOfStrings_CanBeLoaded() {
+        AwsCredentials credentials =
+            ProcessCredentialsProvider.builder()
+                                      .command(Arrays.asList(scriptLocation, "accessKeyId", "secretAccessKey"))
+                                      .build()
+                                      .resolveCredentials();
+
+        assertThat(credentials).isInstanceOf(AwsBasicCredentials.class);
+        assertThat(credentials.accessKeyId()).isEqualTo("accessKeyId");
+        assertThat(credentials.secretAccessKey()).isEqualTo("secretAccessKey");
+        assertThat(credentials.providerName()).isPresent().contains("ProcessCredentialsProvider");
+    }
 
     @Test
     void sessionCredentialsCanBeLoaded() {
@@ -188,4 +204,35 @@ void closeDoesNotRaise() {
         credentialsProvider.resolveCredentials();
         credentialsProvider.close();
     }
+
+    @Test
+    void commandAsListOfStrings_isNotExecutedInAShell() {
+        ProcessCredentialsProvider providerWithSingleStringCommand =
+            ProcessCredentialsProvider.builder()
+                                      .command("echo \"Hello, World!\" > output.txt; rm output.txt")
+                                      .build();
+
+        try {
+            providerWithSingleStringCommand.resolveCredentials();
+        } catch (IllegalStateException e) {
+            // executed in a shell
+            assertThat(e.getCause()).isInstanceOf(NullPointerException.class);
+            assertThat(e.getMessage()).isEqualTo("Failed to refresh process-based credentials.");
+        }
+
+        ProcessCredentialsProvider providerWithCommandAsListOfStrings =
+            ProcessCredentialsProvider.builder()
+                                      .command(Arrays.asList("echo \"Hello, World!\" > output.txt; rm output.txt"))
+                                      .build();
+
+        try {
+            providerWithCommandAsListOfStrings.resolveCredentials();
+        } catch (IllegalStateException e) {
+            // executed not in a shell
+            assertThat(e.getCause()).isInstanceOf(IOException.class);
+            assertThat(e.getCause().getMessage())
+                .isEqualTo("Cannot run program \"echo \"Hello, World!\" > output.txt; rm output.txt\": error=2, "
+                           + "No such file or directory");
+        }
+    }
 }