Repository: aws-actions/configure-aws-credentials
Pull request #19 (2020-01-27 02:44:56)

fix: set role credentials as secrets to mask them in logs
It's super cool that this action now supports assuming a role (#17), it prompted me to start using it instead of a custom solution.
One thing I think was overlooked is that now if you assume a role, the temporary role credentials will be in plain text in Github Actions logs. Even if they're short-lived, they're still normally powerful and I think it's better if they're masked.

This PR marks the AWS credentials from an assumed role as secrets (access key id, secret access key and session token), which will mask them in the logs. This isn't an issue using AWS credentials straight from Github Actions secrets because of course they are already marked as secrets and hence are already masked.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.


mattsb42-aws: Yup, this was a miss on my part. I hadn't dug into what exactly marking an environment variable as "secret" did.

>This isn't an issue using AWS credentials straight from Github Actions secrets because of course they are already marked as secrets and hence are already masked.

Would it hurt anything to mark them secret anyway? My preference would be to just always mark them as secret, regardless of whether they are already. That way a) we don't need to rely on externally defined behavior, and b) we don't need to rely on the reader to know that external behavior exists. It would also simplify the code and reduce branching. :)
clareliguori: Thanks for this contribution!

I agree with @mattsb42-aws that it would be valuable to always mask credentials.  Getting the original creds as secrets is more of a convention; they could actually come from anywhere (like from an encrypted file in the repo, or as an output from another action) and we would always want them to be masked.
LaurenceGA: Yeah true both good points. I don't think it would do any harm. I've updated it.

Patch:
diff --git a/index.js b/index.js
index ef4072f..744f8ea 100644
--- a/index.js
+++ b/index.js
@@ -54,21 +54,25 @@ async function assumeRole(params) {
 }
 
 function exportCredentials(params){
-  // Configure the AWS CLI and AWS SDKs using environment variables
+  // Configure the AWS CLI and AWS SDKs using environment variables and set them as secrets.
+  // Setting the credentials as secrets masks them in Github Actions logs
   const {accessKeyId, secretAccessKey, sessionToken} = params;
 
   // AWS_ACCESS_KEY_ID:
   // Specifies an AWS access key associated with an IAM user or role
   core.exportVariable('AWS_ACCESS_KEY_ID', accessKeyId);
+  core.setSecret('AWS_ACCESS_KEY_ID', accessKeyId);
 
   // AWS_SECRET_ACCESS_KEY:
   // Specifies the secret key associated with the access key. This is essentially the "password" for the access key.
   core.exportVariable('AWS_SECRET_ACCESS_KEY', secretAccessKey);
+  core.setSecret('AWS_SECRET_ACCESS_KEY', secretAccessKey);
 
   // AWS_SESSION_TOKEN:
   // Specifies the session token value that is required if you are using temporary security credentials.
   if (sessionToken) {
     core.exportVariable('AWS_SESSION_TOKEN', sessionToken);
+    core.setSecret('AWS_SESSION_TOKEN', sessionToken);
   }
 }
 


Test patch:
diff --git a/index.test.js b/index.test.js
index cf5ca3a..9db2f8d 100644
--- a/index.test.js
+++ b/index.test.js
@@ -97,9 +97,13 @@ describe('Configure AWS Credentials', () => {
         await run();
         expect(mockStsAssumeRole).toHaveBeenCalledTimes(0);
         expect(core.exportVariable).toHaveBeenCalledTimes(5);
+        expect(core.setSecret).toHaveBeenCalledTimes(4);
         expect(core.exportVariable).toHaveBeenCalledWith('AWS_ACCESS_KEY_ID', FAKE_ACCESS_KEY_ID);
+        expect(core.setSecret).toHaveBeenCalledWith('AWS_ACCESS_KEY_ID', FAKE_ACCESS_KEY_ID);
         expect(core.exportVariable).toHaveBeenCalledWith('AWS_SECRET_ACCESS_KEY', FAKE_SECRET_ACCESS_KEY);
+        expect(core.setSecret).toHaveBeenCalledWith('AWS_SECRET_ACCESS_KEY', FAKE_SECRET_ACCESS_KEY);
         expect(core.exportVariable).toHaveBeenCalledWith('AWS_SESSION_TOKEN', FAKE_SESSION_TOKEN);
+        expect(core.setSecret).toHaveBeenCalledWith('AWS_SESSION_TOKEN', FAKE_SESSION_TOKEN);
         expect(core.exportVariable).toHaveBeenCalledWith('AWS_DEFAULT_REGION', FAKE_REGION);
         expect(core.exportVariable).toHaveBeenCalledWith('AWS_REGION', FAKE_REGION);
         expect(core.setOutput).toHaveBeenCalledWith('aws-account-id', FAKE_ACCOUNT_ID);
@@ -115,8 +119,11 @@ describe('Configure AWS Credentials', () => {
         await run();
         expect(mockStsAssumeRole).toHaveBeenCalledTimes(0);
         expect(core.exportVariable).toHaveBeenCalledTimes(4);
+        expect(core.setSecret).toHaveBeenCalledTimes(3);
         expect(core.exportVariable).toHaveBeenCalledWith('AWS_ACCESS_KEY_ID', FAKE_ACCESS_KEY_ID);
+        expect(core.setSecret).toHaveBeenCalledWith('AWS_ACCESS_KEY_ID', FAKE_ACCESS_KEY_ID);
         expect(core.exportVariable).toHaveBeenCalledWith('AWS_SECRET_ACCESS_KEY', FAKE_SECRET_ACCESS_KEY);
+        expect(core.setSecret).toHaveBeenCalledWith('AWS_SECRET_ACCESS_KEY', FAKE_SECRET_ACCESS_KEY);
         expect(core.exportVariable).toHaveBeenCalledWith('AWS_DEFAULT_REGION', 'eu-west-1');
         expect(core.exportVariable).toHaveBeenCalledWith('AWS_REGION', 'eu-west-1');
         expect(core.setOutput).toHaveBeenCalledWith('aws-account-id', FAKE_ACCOUNT_ID);
@@ -133,11 +140,13 @@ describe('Configure AWS Credentials', () => {
         expect(mockStsAssumeRole).toHaveBeenCalledTimes(0);
         expect(core.exportVariable).toHaveBeenCalledTimes(4);
         expect(core.exportVariable).toHaveBeenCalledWith('AWS_ACCESS_KEY_ID', FAKE_ACCESS_KEY_ID);
+        expect(core.setSecret).toHaveBeenCalledWith('AWS_ACCESS_KEY_ID', FAKE_ACCESS_KEY_ID);
         expect(core.exportVariable).toHaveBeenCalledWith('AWS_SECRET_ACCESS_KEY', FAKE_SECRET_ACCESS_KEY);
+        expect(core.setSecret).toHaveBeenCalledWith('AWS_SECRET_ACCESS_KEY', FAKE_SECRET_ACCESS_KEY);
         expect(core.exportVariable).toHaveBeenCalledWith('AWS_DEFAULT_REGION', 'us-east-1');
         expect(core.exportVariable).toHaveBeenCalledWith('AWS_REGION', 'us-east-1');
         expect(core.setOutput).toHaveBeenCalledWith('aws-account-id', FAKE_ACCOUNT_ID);
-        expect(core.setSecret).toHaveBeenCalledTimes(0);
+        expect(core.setSecret).toHaveBeenCalledTimes(2);
     });
 
     test('error is caught by core.setFailed and caught', async () => {
@@ -171,9 +180,13 @@ describe('Configure AWS Credentials', () => {
         await run();
         expect(mockStsAssumeRole).toHaveBeenCalledTimes(1);
         expect(core.exportVariable).toHaveBeenCalledTimes(5);
+        expect(core.setSecret).toHaveBeenCalledTimes(4);
         expect(core.exportVariable).toHaveBeenCalledWith('AWS_ACCESS_KEY_ID', FAKE_STS_ACCESS_KEY_ID);
+        expect(core.setSecret).toHaveBeenCalledWith('AWS_ACCESS_KEY_ID', FAKE_STS_ACCESS_KEY_ID);
         expect(core.exportVariable).toHaveBeenCalledWith('AWS_SECRET_ACCESS_KEY', FAKE_STS_SECRET_ACCESS_KEY);
+        expect(core.setSecret).toHaveBeenCalledWith('AWS_SECRET_ACCESS_KEY', FAKE_STS_SECRET_ACCESS_KEY);
         expect(core.exportVariable).toHaveBeenCalledWith('AWS_SESSION_TOKEN', FAKE_STS_SESSION_TOKEN);
+        expect(core.setSecret).toHaveBeenCalledWith('AWS_SESSION_TOKEN', FAKE_STS_SESSION_TOKEN);
         expect(core.exportVariable).toHaveBeenCalledWith('AWS_DEFAULT_REGION', FAKE_REGION);
         expect(core.exportVariable).toHaveBeenCalledWith('AWS_REGION', FAKE_REGION);
         expect(core.setOutput).toHaveBeenCalledWith('aws-account-id', FAKE_ACCOUNT_ID);