diff --git a/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/ProcessCredentialsProvider.java b/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/ProcessCredentialsProvider.java index 6bac35476e1a..07a561fd24eb 100644 --- a/core/auth/src/main/java/software/amazon/awssdk/auth/credentials/ProcessCredentialsProvider.java +++ b/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 commandAsListOfStringsFromBuilder; + private final Boolean asyncCredentialUpdateEnabled; /** * @see #builder() */ private ProcessCredentialsProvider(Builder builder) { - List 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 cacheBuilder = CachedSupplier.builder(this::refreshCredentials) @@ -112,6 +101,26 @@ private ProcessCredentialsProvider(Builder builder) { this.processCredentialCache = cacheBuilder.build(); } + private List executableCommand(Builder builder) { + if (builder.commandAsListOfStrings != null) { + return Collections.unmodifiableList(builder.commandAsListOfStrings); + } else { + List 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 { private Boolean asyncCredentialUpdateEnabled = false; private String command; + private List 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 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. diff --git a/core/auth/src/test/java/software/amazon/awssdk/auth/credentials/ProcessCredentialsProviderTest.java b/core/auth/src/test/java/software/amazon/awssdk/auth/credentials/ProcessCredentialsProviderTest.java index d794421c40bf..2c5278ad1201 100644 --- a/core/auth/src/test/java/software/amazon/awssdk/auth/credentials/ProcessCredentialsProviderTest.java +++ b/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"); + } + } } \ No newline at end of file