From 7b893ba14bef5ccd014073d438444b85c3113fee Mon Sep 17 00:00:00 2001 From: peterwoodworth Date: Wed, 5 Jul 2023 18:55:04 -0700 Subject: [PATCH] feat: getIDToken retry, feat: special character in key retry --- .github/workflows/development.yml | 3 +- .github/workflows/test.yml | 16 ------ .github/workflows/unit-tests.yml | 26 +++++++++ dist/cleanup/index.js | 19 ++++++- dist/cleanup/src/helpers.d.ts | 1 + dist/index.js | 40 ++++++++++++-- src/assumeRole.ts | 14 +++-- src/helpers.ts | 17 ++++++ src/index.ts | 12 ++++- test/index.test.ts | 87 ++++++++++++++++++++++++++++--- 10 files changed, 200 insertions(+), 35 deletions(-) delete mode 100644 .github/workflows/test.yml create mode 100644 .github/workflows/unit-tests.yml diff --git a/.github/workflows/development.yml b/.github/workflows/development.yml index aa93fb9..0cba2db 100644 --- a/.github/workflows/development.yml +++ b/.github/workflows/development.yml @@ -1,5 +1,4 @@ -## WIP -name: Devel_workflow +name: Run tests on: workflow_dispatch: diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml deleted file mode 100644 index c43dcd6..0000000 --- a/.github/workflows/test.yml +++ /dev/null @@ -1,16 +0,0 @@ -on: - [pull_request] - -name: Run Unit Tests - -jobs: - test: - name: Run Unit Tests - runs-on: ubuntu-latest - steps: - - name: Checkout - uses: actions/checkout@v3 - - name: Run tests - run: | - npm ci - npm run test diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml new file mode 100644 index 0000000..2f2ea76 --- /dev/null +++ b/.github/workflows/unit-tests.yml @@ -0,0 +1,26 @@ +on: + [pull_request] + +name: Run unit tests + +jobs: + unit-test: + strategy: + fail-fast: false + matrix: + os: [windows-latest, ubuntu-latest, macos-latest] + node: [14, 16, 18] + name: Run unit tests + runs-on: ${{ matrix.os }} + timeout-minutes: 5 + steps: + - name: "Checkout repository" + uses: actions/checkout@v3 + - name: "Setup node" + uses: actions/setup-node@v3 + with: + node-version: ${{ matrix.node }} + - name: "Install dependencies" + uses: bahmutov/npm-install@v1 + - name: "Run tests" + run: npm run test --if-present diff --git a/dist/cleanup/index.js b/dist/cleanup/index.js index 44a58ba..ef82639 100644 --- a/dist/cleanup/index.js +++ b/dist/cleanup/index.js @@ -17583,11 +17583,12 @@ var __importStar = (this && this.__importStar) || function (mod) { return result; }; Object.defineProperty(exports, "__esModule", ({ value: true })); -exports.isDefined = exports.errorMessage = exports.retryAndBackoff = exports.reset = exports.withsleep = exports.defaultSleep = exports.sanitizeGitHubVariables = exports.exportAccountId = exports.exportRegion = exports.unsetCredentials = exports.exportCredentials = void 0; +exports.isDefined = exports.errorMessage = exports.retryAndBackoff = exports.verifyKeys = exports.reset = exports.withsleep = exports.defaultSleep = exports.sanitizeGitHubVariables = exports.exportAccountId = exports.exportRegion = exports.unsetCredentials = exports.exportCredentials = void 0; const core = __importStar(__nccwpck_require__(2186)); const client_sts_1 = __nccwpck_require__(2209); const MAX_TAG_VALUE_LENGTH = 256; const SANITIZATION_CHARACTER = '_'; +const SPECIAL_CHARS_REGEX = /[!@#$%^&*()_+\-=[\]{};':"\\|,.<>/?]+/; // 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 function exportCredentials(creds, outputCredentials) { @@ -17670,6 +17671,22 @@ function reset() { sleep = defaultSleep; } exports.reset = reset; +function verifyKeys(creds) { + if (!creds) { + return; + } + if (creds.AccessKeyId) { + if (SPECIAL_CHARS_REGEX.test(creds.AccessKeyId)) { + throw new Error('AccessKeyId contains special characters.'); + } + } + if (creds.SecretAccessKey) { + if (SPECIAL_CHARS_REGEX.test(creds.SecretAccessKey)) { + throw new Error('SecretAccessKey contains special characters.'); + } + } +} +exports.verifyKeys = verifyKeys; // Retries the promise with exponential backoff if the error isRetryable up to maxRetries time. async function retryAndBackoff(fn, isRetryable, maxRetries = 12, retries = 0, base = 50) { try { diff --git a/dist/cleanup/src/helpers.d.ts b/dist/cleanup/src/helpers.d.ts index e3f4c09..1df20f0 100644 --- a/dist/cleanup/src/helpers.d.ts +++ b/dist/cleanup/src/helpers.d.ts @@ -9,6 +9,7 @@ export declare function defaultSleep(ms: number): Promise; declare let sleep: typeof defaultSleep; export declare function withsleep(s: typeof sleep): void; export declare function reset(): void; +export declare function verifyKeys(creds: Partial | undefined): void; export declare function retryAndBackoff(fn: () => Promise, isRetryable: boolean, maxRetries?: number, retries?: number, base?: number): Promise; export declare function errorMessage(error: unknown): string; export declare function isDefined(i: T | undefined | null): i is T; diff --git a/dist/index.js b/dist/index.js index 346dda6..6627b32 100644 --- a/dist/index.js +++ b/dist/index.js @@ -109,10 +109,12 @@ async function assumeRoleWithOIDC(params, client, webIdentityToken) { delete params.Tags; core.info('Assuming role with OIDC'); try { - return await client.send(new client_sts_1.AssumeRoleWithWebIdentityCommand({ + const creds = await client.send(new client_sts_1.AssumeRoleWithWebIdentityCommand({ ...params, WebIdentityToken: webIdentityToken, })); + (0, helpers_1.verifyKeys)(creds.Credentials); + return creds; } catch (error) { throw new Error(`Could not assume role with OIDC: ${(0, helpers_1.errorMessage)(error)}`); @@ -130,10 +132,12 @@ async function assumeRoleWithWebIdentityTokenFile(params, client, webIdentityTok try { const webIdentityToken = fs_1.default.readFileSync(webIdentityTokenFilePath, 'utf8'); delete params.Tags; - return await client.send(new client_sts_1.AssumeRoleWithWebIdentityCommand({ + const creds = await client.send(new client_sts_1.AssumeRoleWithWebIdentityCommand({ ...params, WebIdentityToken: webIdentityToken, })); + (0, helpers_1.verifyKeys)(creds.Credentials); + return creds; } catch (error) { throw new Error(`Could not assume role with web identity token file: ${(0, helpers_1.errorMessage)(error)}`); @@ -142,7 +146,9 @@ async function assumeRoleWithWebIdentityTokenFile(params, client, webIdentityTok async function assumeRoleWithCredentials(params, client) { core.info('Assuming role with user credentials'); try { - return await client.send(new client_sts_1.AssumeRoleCommand({ ...params })); + const creds = await client.send(new client_sts_1.AssumeRoleCommand({ ...params })); + (0, helpers_1.verifyKeys)(creds.Credentials); + return creds; } catch (error) { throw new Error(`Could not assume role with user credentials: ${(0, helpers_1.errorMessage)(error)}`); @@ -241,11 +247,12 @@ var __importStar = (this && this.__importStar) || function (mod) { return result; }; Object.defineProperty(exports, "__esModule", ({ value: true })); -exports.isDefined = exports.errorMessage = exports.retryAndBackoff = exports.reset = exports.withsleep = exports.defaultSleep = exports.sanitizeGitHubVariables = exports.exportAccountId = exports.exportRegion = exports.unsetCredentials = exports.exportCredentials = void 0; +exports.isDefined = exports.errorMessage = exports.retryAndBackoff = exports.verifyKeys = exports.reset = exports.withsleep = exports.defaultSleep = exports.sanitizeGitHubVariables = exports.exportAccountId = exports.exportRegion = exports.unsetCredentials = exports.exportCredentials = void 0; const core = __importStar(__nccwpck_require__(2186)); const client_sts_1 = __nccwpck_require__(2209); const MAX_TAG_VALUE_LENGTH = 256; const SANITIZATION_CHARACTER = '_'; +const SPECIAL_CHARS_REGEX = /[!@#$%^&*()_+\-=[\]{};':"\\|,.<>/?]+/; // 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 function exportCredentials(creds, outputCredentials) { @@ -328,6 +335,22 @@ function reset() { sleep = defaultSleep; } exports.reset = reset; +function verifyKeys(creds) { + if (!creds) { + return; + } + if (creds.AccessKeyId) { + if (SPECIAL_CHARS_REGEX.test(creds.AccessKeyId)) { + throw new Error('AccessKeyId contains special characters.'); + } + } + if (creds.SecretAccessKey) { + if (SPECIAL_CHARS_REGEX.test(creds.SecretAccessKey)) { + throw new Error('SecretAccessKey contains special characters.'); + } + } +} +exports.verifyKeys = verifyKeys; // Retries the promise with exponential backoff if the error isRetryable up to maxRetries time. async function retryAndBackoff(fn, isRetryable, maxRetries = 12, retries = 0, base = 50) { try { @@ -465,7 +488,14 @@ async function run() { // If OIDC is being used, generate token // Else, validate that the SDK can pick up credentials if (useGitHubOIDCProvider()) { - webIdentityToken = await core.getIDToken(audience); + try { + webIdentityToken = await (0, helpers_1.retryAndBackoff)(async () => { + return core.getIDToken(audience); + }, !disableRetry, maxRetries); + } + catch (error) { + throw new Error(`getIDToken call failed: ${(0, helpers_1.errorMessage)(error)}`); + } } else if (AccessKeyId) { if (!SecretAccessKey) { diff --git a/src/assumeRole.ts b/src/assumeRole.ts index a5a1fe7..e756ace 100644 --- a/src/assumeRole.ts +++ b/src/assumeRole.ts @@ -5,18 +5,20 @@ import * as core from '@actions/core'; import type { AssumeRoleCommandInput, STSClient, Tag } from '@aws-sdk/client-sts'; import { AssumeRoleCommand, AssumeRoleWithWebIdentityCommand } from '@aws-sdk/client-sts'; import type { CredentialsClient } from './CredentialsClient'; -import { errorMessage, isDefined, sanitizeGitHubVariables } from './helpers'; +import { errorMessage, isDefined, sanitizeGitHubVariables, verifyKeys } from './helpers'; async function assumeRoleWithOIDC(params: AssumeRoleCommandInput, client: STSClient, webIdentityToken: string) { delete params.Tags; core.info('Assuming role with OIDC'); try { - return await client.send( + const creds = await client.send( new AssumeRoleWithWebIdentityCommand({ ...params, WebIdentityToken: webIdentityToken, }) ); + verifyKeys(creds.Credentials); + return creds; } catch (error) { throw new Error(`Could not assume role with OIDC: ${errorMessage(error)}`); } @@ -41,12 +43,14 @@ async function assumeRoleWithWebIdentityTokenFile( try { const webIdentityToken = fs.readFileSync(webIdentityTokenFilePath, 'utf8'); delete params.Tags; - return await client.send( + const creds = await client.send( new AssumeRoleWithWebIdentityCommand({ ...params, WebIdentityToken: webIdentityToken, }) ); + verifyKeys(creds.Credentials); + return creds; } catch (error) { throw new Error(`Could not assume role with web identity token file: ${errorMessage(error)}`); } @@ -55,7 +59,9 @@ async function assumeRoleWithWebIdentityTokenFile( async function assumeRoleWithCredentials(params: AssumeRoleCommandInput, client: STSClient) { core.info('Assuming role with user credentials'); try { - return await client.send(new AssumeRoleCommand({ ...params })); + const creds = await client.send(new AssumeRoleCommand({ ...params })); + verifyKeys(creds.Credentials); + return creds; } catch (error) { throw new Error(`Could not assume role with user credentials: ${errorMessage(error)}`); } diff --git a/src/helpers.ts b/src/helpers.ts index f4ba97a..3b84d16 100644 --- a/src/helpers.ts +++ b/src/helpers.ts @@ -5,6 +5,7 @@ import type { CredentialsClient } from './CredentialsClient'; const MAX_TAG_VALUE_LENGTH = 256; const SANITIZATION_CHARACTER = '_'; +const SPECIAL_CHARS_REGEX = /[!@#$%^&*()_+\-=[\]{};':"\\|,.<>/?]+/; // 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 @@ -90,6 +91,22 @@ export function reset() { sleep = defaultSleep; } +export function verifyKeys(creds: Partial | undefined) { + if (!creds) { + return; + } + if (creds.AccessKeyId) { + if (SPECIAL_CHARS_REGEX.test(creds.AccessKeyId)) { + throw new Error('AccessKeyId contains special characters.'); + } + } + if (creds.SecretAccessKey) { + if (SPECIAL_CHARS_REGEX.test(creds.SecretAccessKey)) { + throw new Error('SecretAccessKey contains special characters.'); + } + } +} + // Retries the promise with exponential backoff if the error isRetryable up to maxRetries time. export async function retryAndBackoff( fn: () => Promise, diff --git a/src/index.ts b/src/index.ts index 7b32a70..30c34f4 100644 --- a/src/index.ts +++ b/src/index.ts @@ -92,7 +92,17 @@ export async function run() { // If OIDC is being used, generate token // Else, validate that the SDK can pick up credentials if (useGitHubOIDCProvider()) { - webIdentityToken = await core.getIDToken(audience); + try { + webIdentityToken = await retryAndBackoff( + async () => { + return core.getIDToken(audience); + }, + !disableRetry, + maxRetries + ); + } catch (error) { + throw new Error(`getIDToken call failed: ${errorMessage(error)}`); + } } else if (AccessKeyId) { if (!SecretAccessKey) { throw new Error("'aws-secret-access-key' must be provided if 'aws-access-key-id' is provided"); diff --git a/test/index.test.ts b/test/index.test.ts index 37bfce9..04aae16 100644 --- a/test/index.test.ts +++ b/test/index.test.ts @@ -12,12 +12,12 @@ import { withsleep, reset } from '../src/helpers'; import { run } from '../src/index'; // #region -const FAKE_ACCESS_KEY_ID = 'MY-AWS-ACCESS-KEY-ID'; -const FAKE_SECRET_ACCESS_KEY = 'MY-AWS-SECRET-ACCESS-KEY'; -const FAKE_SESSION_TOKEN = 'MY-AWS-SESSION-TOKEN'; -const FAKE_STS_ACCESS_KEY_ID = 'STS-AWS-ACCESS-KEY-ID'; -const FAKE_STS_SECRET_ACCESS_KEY = 'STS-AWS-SECRET-ACCESS-KEY'; -const FAKE_STS_SESSION_TOKEN = 'STS-AWS-SESSION-TOKEN'; +const FAKE_ACCESS_KEY_ID = 'MYAWSACCESSKEYID'; +const FAKE_SECRET_ACCESS_KEY = 'MYAWSSECRETACCESSKEY'; +const FAKE_SESSION_TOKEN = 'MYAWSSESSIONTOKEN'; +const FAKE_STS_ACCESS_KEY_ID = 'STSAWSACCESSKEYID'; +const FAKE_STS_SECRET_ACCESS_KEY = 'STSAWSSECRETACCESSKEY'; +const FAKE_STS_SESSION_TOKEN = 'STSAWSSESSIONTOKEN'; const FAKE_REGION = 'fake-region-1'; const FAKE_ACCOUNT_ID = '123456789012'; const FAKE_ROLE_ACCOUNT_ID = '111111111111'; @@ -453,6 +453,24 @@ describe('Configure AWS Credentials', () => { DurationSeconds: 3600, WebIdentityToken: 'testtoken', }); + expect(core.getIDToken).toHaveBeenCalledTimes(1); + }); + + test('getIDToken call retries when failing', async () => { + process.env['GITHUB_ACTIONS'] = 'true'; + process.env['ACTIONS_ID_TOKEN_REQUEST_TOKEN'] = 'test-token'; + jest.spyOn(core, 'getIDToken').mockImplementation(() => { + throw new Error('test error'); + }); + + jest + .spyOn(core, 'getInput') + .mockImplementation(mockGetInput({ 'role-to-assume': ROLE_ARN, 'aws-region': FAKE_REGION })); + + await run(); + + expect(core.getIDToken).toHaveBeenCalledTimes(12); + expect(core.setFailed).toHaveBeenCalledWith('getIDToken call failed: test error'); }); test('GH OIDC With custom role duration', async () => { @@ -507,6 +525,63 @@ describe('Configure AWS Credentials', () => { expect(mockedSTS.commandCalls(AssumeRoleWithWebIdentityCommand).length).toEqual(1); }); + test('role assumption fails if access key id contains special characters', async () => { + jest.spyOn(core, 'getInput').mockImplementation(mockGetInput({ ...ASSUME_ROLE_INPUTS })); + + mockedSTS.on(AssumeRoleCommand).resolves({ + Credentials: { + AccessKeyId: 'asdf+', + SecretAccessKey: FAKE_STS_SECRET_ACCESS_KEY, + SessionToken: FAKE_STS_SESSION_TOKEN, + Expiration: new Date(8640000000000000), + }, + }); + + await run(); + + expect(mockedSTS.commandCalls(AssumeRoleCommand).length).toEqual(12); + expect(core.setFailed).toHaveBeenCalledWith( + 'Could not assume role with user credentials: AccessKeyId contains special characters.' + ); + }); + + test('role assumption fails if secret access key contains special characters', async () => { + jest.spyOn(core, 'getInput').mockImplementation(mockGetInput({ ...ASSUME_ROLE_INPUTS })); + + mockedSTS.on(AssumeRoleCommand).resolves({ + Credentials: { + AccessKeyId: FAKE_STS_ACCESS_KEY_ID, + SecretAccessKey: 'asdf+', + SessionToken: FAKE_STS_SESSION_TOKEN, + Expiration: new Date(8640000000000000), + }, + }); + + await run(); + + expect(mockedSTS.commandCalls(AssumeRoleCommand).length).toEqual(12); + expect(core.setFailed).toHaveBeenCalledWith( + 'Could not assume role with user credentials: SecretAccessKey contains special characters.' + ); + }); + + test('role assumption succeeds if keys have no special characters', async () => { + jest.spyOn(core, 'getInput').mockImplementation(mockGetInput({ ...ASSUME_ROLE_INPUTS })); + + mockedSTS.on(AssumeRoleCommand).resolves({ + Credentials: { + AccessKeyId: FAKE_STS_ACCESS_KEY_ID, + SecretAccessKey: FAKE_STS_SECRET_ACCESS_KEY, + SessionToken: FAKE_STS_SESSION_TOKEN, + Expiration: new Date(8640000000000000), + }, + }); + + await run(); + + expect(mockedSTS.commandCalls(AssumeRoleCommand).length).toEqual(1); + }); + test('max retries is configurable', async () => { process.env['GITHUB_ACTIONS'] = 'true'; process.env['ACTIONS_ID_TOKEN_REQUEST_TOKEN'] = 'test-token';