From 472e549195ba1f153e9fb72e39dc2a094e5de13e Mon Sep 17 00:00:00 2001 From: Clare Liguori Date: Wed, 3 Jun 2020 13:05:51 -0700 Subject: [PATCH] feat: Refresh and validate credentials after setting env var creds (#71) * feat: Refresh and validate credentials after setting env var creds * Positive test case Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- dist/index.js | 52 +++++++++++++++++++++++++++++ index.js | 52 +++++++++++++++++++++++++++++ index.test.js | 91 +++++++++++++++++++++++++++++++++++++++++++++++++-- 3 files changed, 193 insertions(+), 2 deletions(-) diff --git a/dist/index.js b/dist/index.js index 2d4c879df..6eb5742ba 100644 --- a/dist/index.js +++ b/dist/index.js @@ -265,6 +265,50 @@ async function exportAccountId(maskAccountId, region) { return accountId; } +function loadCredentials() { + // Force the SDK to re-resolve credentials with the default provider chain. + // + // This action typically sets credentials in the environment via environment variables. + // The SDK never refreshes those env-var-based credentials after initial load. + // In case there were already env-var creds set in the actions environment when this action + // loaded, this action needs to refresh the SDK creds after overwriting those environment variables. + // + // The credentials object needs to be entirely recreated (instead of simply refreshed), + // because the credential object type could change when this action writes env var creds. + // For example, the first load could return EC2 instance metadata credentials + // in a self-hosted runner, and the second load could return environment credentials + // from an assume-role call in this action. + aws.config.credentials = null; + + return new Promise((resolve, reject) => { + aws.config.getCredentials((err) => { + if (err) { + reject(err); + } + resolve(aws.config.credentials); + }) + }); +} + +async function validateCredentials(expectedAccessKeyId) { + let credentials; + try { + credentials = await loadCredentials(); + + if (!credentials.accessKeyId) { + throw new Error('Access key ID empty after loading credentials'); + } + } catch (error) { + throw new Error(`Credentials could not be loaded, please check your action inputs: ${error.message}`); + } + + const actualAccessKeyId = credentials.accessKeyId; + + if (expectedAccessKeyId && expectedAccessKeyId != actualAccessKeyId) { + throw new Error('Unexpected failure: Credentials loaded by the SDK do not match the access key ID configured by the action'); + } +} + function getStsClient(region) { return new aws.STS({ region, @@ -305,6 +349,13 @@ async function run() { exportCredentials({accessKeyId, secretAccessKey, sessionToken}); } + // Regardless of whether any source credentials were provided as inputs, + // validate that the SDK can actually pick up credentials. This validates + // cases where this action is on a self-hosted runner that doesn't have credentials + // configured correctly, and cases where the user intended to provide input + // credentials but the secrets inputs resolved to empty strings. + await validateCredentials(accessKeyId); + const sourceAccountId = await exportAccountId(maskAccountId, region); // Get role credentials if configured to do so @@ -318,6 +369,7 @@ async function run() { roleSessionName }); exportCredentials(roleCredentials); + await validateCredentials(roleCredentials.accessKeyId); await exportAccountId(maskAccountId, region); } } diff --git a/index.js b/index.js index 089a0818a..9b0cc52b9 100644 --- a/index.js +++ b/index.js @@ -132,6 +132,50 @@ async function exportAccountId(maskAccountId, region) { return accountId; } +function loadCredentials() { + // Force the SDK to re-resolve credentials with the default provider chain. + // + // This action typically sets credentials in the environment via environment variables. + // The SDK never refreshes those env-var-based credentials after initial load. + // In case there were already env-var creds set in the actions environment when this action + // loaded, this action needs to refresh the SDK creds after overwriting those environment variables. + // + // The credentials object needs to be entirely recreated (instead of simply refreshed), + // because the credential object type could change when this action writes env var creds. + // For example, the first load could return EC2 instance metadata credentials + // in a self-hosted runner, and the second load could return environment credentials + // from an assume-role call in this action. + aws.config.credentials = null; + + return new Promise((resolve, reject) => { + aws.config.getCredentials((err) => { + if (err) { + reject(err); + } + resolve(aws.config.credentials); + }) + }); +} + +async function validateCredentials(expectedAccessKeyId) { + let credentials; + try { + credentials = await loadCredentials(); + + if (!credentials.accessKeyId) { + throw new Error('Access key ID empty after loading credentials'); + } + } catch (error) { + throw new Error(`Credentials could not be loaded, please check your action inputs: ${error.message}`); + } + + const actualAccessKeyId = credentials.accessKeyId; + + if (expectedAccessKeyId && expectedAccessKeyId != actualAccessKeyId) { + throw new Error('Unexpected failure: Credentials loaded by the SDK do not match the access key ID configured by the action'); + } +} + function getStsClient(region) { return new aws.STS({ region, @@ -172,6 +216,13 @@ async function run() { exportCredentials({accessKeyId, secretAccessKey, sessionToken}); } + // Regardless of whether any source credentials were provided as inputs, + // validate that the SDK can actually pick up credentials. This validates + // cases where this action is on a self-hosted runner that doesn't have credentials + // configured correctly, and cases where the user intended to provide input + // credentials but the secrets inputs resolved to empty strings. + await validateCredentials(accessKeyId); + const sourceAccountId = await exportAccountId(maskAccountId, region); // Get role credentials if configured to do so @@ -185,6 +236,7 @@ async function run() { roleSessionName }); exportCredentials(roleCredentials); + await validateCredentials(roleCredentials.accessKeyId); await exportAccountId(maskAccountId, region); } } diff --git a/index.test.js b/index.test.js index 89e82d901..8fb01a753 100644 --- a/index.test.js +++ b/index.test.js @@ -1,6 +1,6 @@ const core = require('@actions/core'); const assert = require('assert'); - +const aws = require('aws-sdk'); const run = require('.'); jest.mock('@actions/core'); @@ -49,6 +49,9 @@ const mockStsAssumeRole = jest.fn(); jest.mock('aws-sdk', () => { return { + config: { + getCredentials: jest.fn() + }, STS: jest.fn(() => ({ getCallerIdentity: mockStsCallerIdentity, assumeRole: mockStsAssumeRole, @@ -82,6 +85,27 @@ describe('Configure AWS Credentials', () => { } }); + aws.config.getCredentials.mockReset(); + aws.config.getCredentials + .mockImplementationOnce(callback => { + if (!aws.config.credentials) { + aws.config.credentials = { + accessKeyId: FAKE_ACCESS_KEY_ID, + secretAccessKey: FAKE_SECRET_ACCESS_KEY + } + } + callback(null); + }) + .mockImplementationOnce(callback => { + if (!aws.config.credentials) { + aws.config.credentials = { + accessKeyId: FAKE_STS_ACCESS_KEY_ID, + secretAccessKey: FAKE_STS_SECRET_ACCESS_KEY + } + } + callback(null); + }); + mockStsAssumeRole.mockImplementation(() => { return { promise() { @@ -134,6 +158,59 @@ describe('Configure AWS Credentials', () => { expect(core.setSecret).toHaveBeenCalledWith(FAKE_ACCOUNT_ID); }); + test('action with no accessible credentials fails', async () => { + process.env.SHOW_STACK_TRACE = 'false'; + const mockInputs = {'aws-region': FAKE_REGION}; + core.getInput = jest + .fn() + .mockImplementation(mockGetInput(mockInputs)); + aws.config.getCredentials.mockReset(); + aws.config.getCredentials.mockImplementation(callback => { + callback(new Error('No credentials to load')); + }); + + await run(); + + expect(core.setFailed).toHaveBeenCalledWith("Credentials could not be loaded, please check your action inputs: No credentials to load"); + }); + + test('action with empty credentials fails', async () => { + process.env.SHOW_STACK_TRACE = 'false'; + const mockInputs = {'aws-region': FAKE_REGION}; + core.getInput = jest + .fn() + .mockImplementation(mockGetInput(mockInputs)); + aws.config.getCredentials.mockReset(); + aws.config.getCredentials.mockImplementation(callback => { + aws.config.credentials = { + accessKeyId: '' + } + callback(null); + }); + + await run(); + + expect(core.setFailed).toHaveBeenCalledWith("Credentials could not be loaded, please check your action inputs: Access key ID empty after loading credentials"); + }); + + test('action fails when credentials are not set in the SDK correctly', async () => { + process.env.SHOW_STACK_TRACE = 'false'; + core.getInput = jest + .fn() + .mockImplementation(mockGetInput(ASSUME_ROLE_INPUTS)); + aws.config.getCredentials.mockReset(); + aws.config.getCredentials.mockImplementation(callback => { + aws.config.credentials = { + accessKeyId: FAKE_ACCESS_KEY_ID + } + callback(null); + }); + + await run(); + + expect(core.setFailed).toHaveBeenCalledWith("Unexpected failure: Credentials loaded by the SDK do not match the access key ID configured by the action"); + }); + test('session token is optional', async () => { const mockInputs = {...CREDS_INPUTS, 'aws-region': 'eu-west-1'}; core.getInput = jest @@ -154,12 +231,19 @@ describe('Configure AWS Credentials', () => { expect(core.setSecret).toHaveBeenCalledWith(FAKE_ACCOUNT_ID); }); - test('session token is cleared if necessary', async () => { + test('existing env var creds are cleared', async () => { const mockInputs = {...CREDS_INPUTS, 'aws-region': 'eu-west-1'}; core.getInput = jest .fn() .mockImplementation(mockGetInput(mockInputs)); + process.env.AWS_ACCESS_KEY_ID = 'foo'; + process.env.AWS_SECRET_ACCESS_KEY = 'bar'; process.env.AWS_SESSION_TOKEN = 'helloworld'; + aws.config.credentials = { + accessKeyId: 'foo', + secretAccessKey: 'bar', + sessionToken: 'helloworld' + }; await run(); expect(mockStsAssumeRole).toHaveBeenCalledTimes(0); @@ -174,6 +258,9 @@ describe('Configure AWS Credentials', () => { expect(core.exportVariable).toHaveBeenCalledWith('AWS_REGION', 'eu-west-1'); expect(core.setOutput).toHaveBeenCalledWith('aws-account-id', FAKE_ACCOUNT_ID); expect(core.setSecret).toHaveBeenCalledWith(FAKE_ACCOUNT_ID); + expect(aws.config.credentials.accessKeyId).toBe(FAKE_ACCESS_KEY_ID); + expect(aws.config.credentials.secretAccessKey).toBe(FAKE_SECRET_ACCESS_KEY); + expect(aws.config.credentials.sessionToken).toBeUndefined(); }); test('validates region name', async () => {