Skip to content

Commit

Permalink
feat: add config option for special character handling (#795)
Browse files Browse the repository at this point in the history
  • Loading branch information
kellertk authored Aug 24, 2023
1 parent ef93a73 commit fbbf385
Show file tree
Hide file tree
Showing 6 changed files with 105 additions and 74 deletions.
9 changes: 9 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,15 @@ Your account ID is not masked by default in workflow logs since it's not conside
#### Unset current credentials
Sometimes, existing credentials in your runner can get in the way of the intended outcome, and the recommended solution is to include another step in your workflow which unsets the environment variables set by this action. Now if you set the `unset-current-credentials` input to `true`, the workaround is made eaiser

#### Special characters in AWS_SECRET_ACCESS_KEY
Some edge cases are unable to properly parse an `AWS_SECRET_ACCESS_KEY` if it
contains special characters. For more information, please see the
[AWS CLI documentation](https://docs.aws.amazon.com/cli/latest/userguide/cli-chap-troubleshooting.html#tshoot-signature-does-not-match).
If you set the `special-characters-workaround` option, this action will
continually retry fetching credentials until we get one that does not have
special characters. This option overrides the `disable-retry` and
`retry-max-attempts` options.

## OIDC

We recommend using [GitHub's OIDC provider](https://docs.github.com/en/actions/deployment/security-hardening-your-deployments/configuring-openid-connect-in-amazon-web-services) to get short-lived AWS credentials needed for your actions. When using OIDC, this action will create a JWT unique to the workflow run, and it will use this JWT to assume the role. For this action to create the JWT, it is required for your workflow to have the `id-token: write` permission:
Expand Down
3 changes: 3 additions & 0 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,9 @@ inputs:
retry-max-attempts:
description: The maximum number of attempts it will attempt to retry the assume role call. By default it will retry 12 times
required: false
special-characters-workaround:
description: Some environments do not support special characters in AWS_SECRET_ACCESS_KEY. This option will retry fetching credentials until the secret access key does not contain special characters. This option overrides disable-retry and retry-max-attempts. This option is disabled by default
required: false
outputs:
aws-account-id:
description: The AWS account ID for the provided credentials
Expand Down
5 changes: 1 addition & 4 deletions src/assumeRole.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ 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, verifyKeys } from './helpers';
import { errorMessage, isDefined, sanitizeGitHubVariables } from './helpers';

async function assumeRoleWithOIDC(params: AssumeRoleCommandInput, client: STSClient, webIdentityToken: string) {
delete params.Tags;
Expand All @@ -17,7 +17,6 @@ async function assumeRoleWithOIDC(params: AssumeRoleCommandInput, client: STSCli
WebIdentityToken: webIdentityToken,
})
);
verifyKeys(creds.Credentials);
return creds;
} catch (error) {
throw new Error(`Could not assume role with OIDC: ${errorMessage(error)}`);
Expand Down Expand Up @@ -49,7 +48,6 @@ async function assumeRoleWithWebIdentityTokenFile(
WebIdentityToken: webIdentityToken,
})
);
verifyKeys(creds.Credentials);
return creds;
} catch (error) {
throw new Error(`Could not assume role with web identity token file: ${errorMessage(error)}`);
Expand All @@ -60,7 +58,6 @@ async function assumeRoleWithCredentials(params: AssumeRoleCommandInput, client:
core.info('Assuming role with user credentials');
try {
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)}`);
Expand Down
9 changes: 6 additions & 3 deletions src/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,18 +93,21 @@ export function reset() {

export function verifyKeys(creds: Partial<Credentials> | undefined) {
if (!creds) {
return;
return false;
}
if (creds.AccessKeyId) {
if (SPECIAL_CHARS_REGEX.test(creds.AccessKeyId)) {
throw new Error('AccessKeyId contains special characters.');
core.debug('AccessKeyId contains special characters.');
return false;
}
}
if (creds.SecretAccessKey) {
if (SPECIAL_CHARS_REGEX.test(creds.SecretAccessKey)) {
throw new Error('SecretAccessKey contains special characters.');
core.debug('SecretAccessKey contains special characters.');
return false;
}
}
return true;
}

// Retries the promise with exponential backoff if the error isRetryable up to maxRetries time.
Expand Down
62 changes: 40 additions & 22 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import * as core from '@actions/core';
import type { AssumeRoleCommandOutput } from '@aws-sdk/client-sts';
import { assumeRole } from './assumeRole';
import { CredentialsClient } from './CredentialsClient';
import {
Expand All @@ -8,6 +9,7 @@ import {
exportCredentials,
exportAccountId,
unsetCredentials,
verifyKeys,
} from './helpers';

const DEFAULT_ROLE_DURATION = 3600; // One hour (seconds)
Expand Down Expand Up @@ -43,10 +45,20 @@ export async function run() {
const unsetCurrentCredentialsInput = core.getInput('unset-current-credentials', { required: false }) || 'false';
const unsetCurrentCredentials = unsetCurrentCredentialsInput.toLowerCase() === 'true';
const disableRetryInput = core.getInput('disable-retry', { required: false }) || 'false';
const disableRetry = disableRetryInput.toLowerCase() === 'true';
let disableRetry = disableRetryInput.toLowerCase() === 'true';
const specialCharacterWorkaroundInput =
core.getInput('special-characters-workaround', { required: false }) || 'false';
const specialCharacterWorkaround = specialCharacterWorkaroundInput.toLowerCase() === 'true';
let maxRetries = parseInt(core.getInput('retry-max-attempts', { required: false })) || 12;
if (maxRetries < 1) {
maxRetries = 1;
switch (true) {
case specialCharacterWorkaround:
// 😳
maxRetries = Number.MAX_SAFE_INTEGER;
disableRetry = false;
break;
case maxRetries < 1:
maxRetries = 1;
break;
}
for (const managedSessionPolicy of managedSessionPoliciesInput) {
managedSessionPolicies.push({ arn: managedSessionPolicy });
Expand Down Expand Up @@ -129,25 +141,30 @@ export async function run() {

// Get role credentials if configured to do so
if (roleToAssume) {
const roleCredentials = await retryAndBackoff(
async () => {
return assumeRole({
credentialsClient,
sourceAccountId,
roleToAssume,
roleExternalId,
roleDuration,
roleSessionName,
roleSkipSessionTagging,
webIdentityTokenFile,
webIdentityToken,
inlineSessionPolicy,
managedSessionPolicies,
});
},
!disableRetry,
maxRetries
);
let roleCredentials: AssumeRoleCommandOutput;
do {
// eslint-disable-next-line no-await-in-loop
roleCredentials = await retryAndBackoff(
async () => {
return assumeRole({
credentialsClient,
sourceAccountId,
roleToAssume,
roleExternalId,
roleDuration,
roleSessionName,
roleSkipSessionTagging,
webIdentityTokenFile,
webIdentityToken,
inlineSessionPolicy,
managedSessionPolicies,
});
},
!disableRetry,
maxRetries
);
// eslint-disable-next-line no-unmodified-loop-condition
} while (specialCharacterWorkaround && !verifyKeys(roleCredentials.Credentials));
core.info(`Authenticated as assumedRoleId ${roleCredentials.AssumedRoleUser!.AssumedRoleId!}`);
exportCredentials(roleCredentials.Credentials, outputCredentials);
// We need to validate the credentials in 2 of our use-cases
Expand All @@ -173,6 +190,7 @@ export async function run() {
}

/* c8 ignore start */
/* istanbul ignore next */
if (require.main === module) {
(async () => {
await run();
Expand Down
91 changes: 46 additions & 45 deletions test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -556,61 +556,62 @@ 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 }));
test('special character workaround works for AWS_ACCESS_KEY_ID', async () => {
jest
.spyOn(core, 'getInput')
.mockImplementation(mockGetInput({ ...ASSUME_ROLE_INPUTS, 'special-characters-workaround': 'true' }));

mockedSTS.on(AssumeRoleCommand).resolves({
Credentials: {
AccessKeyId: FAKE_STS_ACCESS_KEY_ID,
SecretAccessKey: 'asdf+',
SessionToken: FAKE_STS_SESSION_TOKEN,
Expiration: new Date(8640000000000000),
},
});
mockedSTS
.on(AssumeRoleCommand)
.resolvesOnce({
Credentials: {
AccessKeyId: FAKE_STS_ACCESS_KEY_ID,
SecretAccessKey: 'asdf+',
SessionToken: FAKE_STS_SESSION_TOKEN,
Expiration: new Date(8640000000000000),
},
})
.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.'
);
expect(mockedSTS.commandCalls(AssumeRoleCommand).length).toEqual(2);
});

test('role assumption succeeds if keys have no special characters', async () => {
jest.spyOn(core, 'getInput').mockImplementation(mockGetInput({ ...ASSUME_ROLE_INPUTS }));
test('special character workaround works for AWS_SECRET_ACCESS_KEY', async () => {
jest
.spyOn(core, 'getInput')
.mockImplementation(mockGetInput({ ...ASSUME_ROLE_INPUTS, 'special-characters-workaround': 'true' }));

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),
},
});
mockedSTS
.on(AssumeRoleCommand)
.resolvesOnce({
Credentials: {
AccessKeyId: FAKE_STS_ACCESS_KEY_ID,
SecretAccessKey: 'asdf+',
SessionToken: FAKE_STS_SESSION_TOKEN,
Expiration: new Date(8640000000000000),
},
})
.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(1);
expect(mockedSTS.commandCalls(AssumeRoleCommand).length).toEqual(2);
});

test('max retries is configurable', async () => {
Expand Down

0 comments on commit fbbf385

Please sign in to comment.