Skip to content

Commit fbbf385

Browse files
kellertkkellertk
authored
feat: add config option for special character handling (#795)
1 parent ef93a73 commit fbbf385

File tree

6 files changed

+105
-74
lines changed

6 files changed

+105
-74
lines changed

‎README.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,15 @@ Your account ID is not masked by default in workflow logs since it's not conside
268268
#### Unset current credentials
269269
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
270270

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

273282
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:

‎action.yml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,9 @@ inputs:
7070
retry-max-attempts:
7171
description: The maximum number of attempts it will attempt to retry the assume role call. By default it will retry 12 times
7272
required: false
73+
special-characters-workaround:
74+
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
75+
required: false
7376
outputs:
7477
aws-account-id:
7578
description: The AWS account ID for the provided credentials

‎src/assumeRole.ts

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import * as core from '@actions/core';
55
import type { AssumeRoleCommandInput, STSClient, Tag } from '@aws-sdk/client-sts';
66
import { AssumeRoleCommand, AssumeRoleWithWebIdentityCommand } from '@aws-sdk/client-sts';
77
import type { CredentialsClient } from './CredentialsClient';
8-
import { errorMessage, isDefined, sanitizeGitHubVariables, verifyKeys } from './helpers';
8+
import { errorMessage, isDefined, sanitizeGitHubVariables } from './helpers';
99

1010
async function assumeRoleWithOIDC(params: AssumeRoleCommandInput, client: STSClient, webIdentityToken: string) {
1111
delete params.Tags;
@@ -17,7 +17,6 @@ async function assumeRoleWithOIDC(params: AssumeRoleCommandInput, client: STSCli
1717
WebIdentityToken: webIdentityToken,
1818
})
1919
);
20-
verifyKeys(creds.Credentials);
2120
return creds;
2221
} catch (error) {
2322
throw new Error(`Could not assume role with OIDC: ${errorMessage(error)}`);
@@ -49,7 +48,6 @@ async function assumeRoleWithWebIdentityTokenFile(
4948
WebIdentityToken: webIdentityToken,
5049
})
5150
);
52-
verifyKeys(creds.Credentials);
5351
return creds;
5452
} catch (error) {
5553
throw new Error(`Could not assume role with web identity token file: ${errorMessage(error)}`);
@@ -60,7 +58,6 @@ async function assumeRoleWithCredentials(params: AssumeRoleCommandInput, client:
6058
core.info('Assuming role with user credentials');
6159
try {
6260
const creds = await client.send(new AssumeRoleCommand({ ...params }));
63-
verifyKeys(creds.Credentials);
6461
return creds;
6562
} catch (error) {
6663
throw new Error(`Could not assume role with user credentials: ${errorMessage(error)}`);

‎src/helpers.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,18 +93,21 @@ export function reset() {
9393

9494
export function verifyKeys(creds: Partial<Credentials> | undefined) {
9595
if (!creds) {
96-
return;
96+
return false;
9797
}
9898
if (creds.AccessKeyId) {
9999
if (SPECIAL_CHARS_REGEX.test(creds.AccessKeyId)) {
100-
throw new Error('AccessKeyId contains special characters.');
100+
core.debug('AccessKeyId contains special characters.');
101+
return false;
101102
}
102103
}
103104
if (creds.SecretAccessKey) {
104105
if (SPECIAL_CHARS_REGEX.test(creds.SecretAccessKey)) {
105-
throw new Error('SecretAccessKey contains special characters.');
106+
core.debug('SecretAccessKey contains special characters.');
107+
return false;
106108
}
107109
}
110+
return true;
108111
}
109112

110113
// Retries the promise with exponential backoff if the error isRetryable up to maxRetries time.

‎src/index.ts

Lines changed: 40 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import * as core from '@actions/core';
2+
import type { AssumeRoleCommandOutput } from '@aws-sdk/client-sts';
23
import { assumeRole } from './assumeRole';
34
import { CredentialsClient } from './CredentialsClient';
45
import {
@@ -8,6 +9,7 @@ import {
89
exportCredentials,
910
exportAccountId,
1011
unsetCredentials,
12+
verifyKeys,
1113
} from './helpers';
1214

1315
const DEFAULT_ROLE_DURATION = 3600; // One hour (seconds)
@@ -43,10 +45,20 @@ export async function run() {
4345
const unsetCurrentCredentialsInput = core.getInput('unset-current-credentials', { required: false }) || 'false';
4446
const unsetCurrentCredentials = unsetCurrentCredentialsInput.toLowerCase() === 'true';
4547
const disableRetryInput = core.getInput('disable-retry', { required: false }) || 'false';
46-
const disableRetry = disableRetryInput.toLowerCase() === 'true';
48+
let disableRetry = disableRetryInput.toLowerCase() === 'true';
49+
const specialCharacterWorkaroundInput =
50+
core.getInput('special-characters-workaround', { required: false }) || 'false';
51+
const specialCharacterWorkaround = specialCharacterWorkaroundInput.toLowerCase() === 'true';
4752
let maxRetries = parseInt(core.getInput('retry-max-attempts', { required: false })) || 12;
48-
if (maxRetries < 1) {
49-
maxRetries = 1;
53+
switch (true) {
54+
case specialCharacterWorkaround:
55+
// 😳
56+
maxRetries = Number.MAX_SAFE_INTEGER;
57+
disableRetry = false;
58+
break;
59+
case maxRetries < 1:
60+
maxRetries = 1;
61+
break;
5062
}
5163
for (const managedSessionPolicy of managedSessionPoliciesInput) {
5264
managedSessionPolicies.push({ arn: managedSessionPolicy });
@@ -129,25 +141,30 @@ export async function run() {
129141

130142
// Get role credentials if configured to do so
131143
if (roleToAssume) {
132-
const roleCredentials = await retryAndBackoff(
133-
async () => {
134-
return assumeRole({
135-
credentialsClient,
136-
sourceAccountId,
137-
roleToAssume,
138-
roleExternalId,
139-
roleDuration,
140-
roleSessionName,
141-
roleSkipSessionTagging,
142-
webIdentityTokenFile,
143-
webIdentityToken,
144-
inlineSessionPolicy,
145-
managedSessionPolicies,
146-
});
147-
},
148-
!disableRetry,
149-
maxRetries
150-
);
144+
let roleCredentials: AssumeRoleCommandOutput;
145+
do {
146+
// eslint-disable-next-line no-await-in-loop
147+
roleCredentials = await retryAndBackoff(
148+
async () => {
149+
return assumeRole({
150+
credentialsClient,
151+
sourceAccountId,
152+
roleToAssume,
153+
roleExternalId,
154+
roleDuration,
155+
roleSessionName,
156+
roleSkipSessionTagging,
157+
webIdentityTokenFile,
158+
webIdentityToken,
159+
inlineSessionPolicy,
160+
managedSessionPolicies,
161+
});
162+
},
163+
!disableRetry,
164+
maxRetries
165+
);
166+
// eslint-disable-next-line no-unmodified-loop-condition
167+
} while (specialCharacterWorkaround && !verifyKeys(roleCredentials.Credentials));
151168
core.info(`Authenticated as assumedRoleId ${roleCredentials.AssumedRoleUser!.AssumedRoleId!}`);
152169
exportCredentials(roleCredentials.Credentials, outputCredentials);
153170
// We need to validate the credentials in 2 of our use-cases
@@ -173,6 +190,7 @@ export async function run() {
173190
}
174191

175192
/* c8 ignore start */
193+
/* istanbul ignore next */
176194
if (require.main === module) {
177195
(async () => {
178196
await run();

‎test/index.test.ts

Lines changed: 46 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -556,61 +556,62 @@ describe('Configure AWS Credentials', () => {
556556
expect(mockedSTS.commandCalls(AssumeRoleWithWebIdentityCommand).length).toEqual(1);
557557
});
558558

559-
test('role assumption fails if access key id contains special characters', async () => {
560-
jest.spyOn(core, 'getInput').mockImplementation(mockGetInput({ ...ASSUME_ROLE_INPUTS }));
561-
562-
mockedSTS.on(AssumeRoleCommand).resolves({
563-
Credentials: {
564-
AccessKeyId: 'asdf+',
565-
SecretAccessKey: FAKE_STS_SECRET_ACCESS_KEY,
566-
SessionToken: FAKE_STS_SESSION_TOKEN,
567-
Expiration: new Date(8640000000000000),
568-
},
569-
});
570-
571-
await run();
572-
573-
expect(mockedSTS.commandCalls(AssumeRoleCommand).length).toEqual(12);
574-
expect(core.setFailed).toHaveBeenCalledWith(
575-
'Could not assume role with user credentials: AccessKeyId contains special characters.'
576-
);
577-
});
578-
579-
test('role assumption fails if secret access key contains special characters', async () => {
580-
jest.spyOn(core, 'getInput').mockImplementation(mockGetInput({ ...ASSUME_ROLE_INPUTS }));
559+
test('special character workaround works for AWS_ACCESS_KEY_ID', async () => {
560+
jest
561+
.spyOn(core, 'getInput')
562+
.mockImplementation(mockGetInput({ ...ASSUME_ROLE_INPUTS, 'special-characters-workaround': 'true' }));
581563

582-
mockedSTS.on(AssumeRoleCommand).resolves({
583-
Credentials: {
584-
AccessKeyId: FAKE_STS_ACCESS_KEY_ID,
585-
SecretAccessKey: 'asdf+',
586-
SessionToken: FAKE_STS_SESSION_TOKEN,
587-
Expiration: new Date(8640000000000000),
588-
},
589-
});
564+
mockedSTS
565+
.on(AssumeRoleCommand)
566+
.resolvesOnce({
567+
Credentials: {
568+
AccessKeyId: FAKE_STS_ACCESS_KEY_ID,
569+
SecretAccessKey: 'asdf+',
570+
SessionToken: FAKE_STS_SESSION_TOKEN,
571+
Expiration: new Date(8640000000000000),
572+
},
573+
})
574+
.resolves({
575+
Credentials: {
576+
AccessKeyId: FAKE_STS_ACCESS_KEY_ID,
577+
SecretAccessKey: 'asdf',
578+
SessionToken: FAKE_STS_SESSION_TOKEN,
579+
Expiration: new Date(8640000000000000),
580+
},
581+
});
590582

591583
await run();
592584

593-
expect(mockedSTS.commandCalls(AssumeRoleCommand).length).toEqual(12);
594-
expect(core.setFailed).toHaveBeenCalledWith(
595-
'Could not assume role with user credentials: SecretAccessKey contains special characters.'
596-
);
585+
expect(mockedSTS.commandCalls(AssumeRoleCommand).length).toEqual(2);
597586
});
598587

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

602-
mockedSTS.on(AssumeRoleCommand).resolves({
603-
Credentials: {
604-
AccessKeyId: FAKE_STS_ACCESS_KEY_ID,
605-
SecretAccessKey: FAKE_STS_SECRET_ACCESS_KEY,
606-
SessionToken: FAKE_STS_SESSION_TOKEN,
607-
Expiration: new Date(8640000000000000),
608-
},
609-
});
593+
mockedSTS
594+
.on(AssumeRoleCommand)
595+
.resolvesOnce({
596+
Credentials: {
597+
AccessKeyId: FAKE_STS_ACCESS_KEY_ID,
598+
SecretAccessKey: 'asdf+',
599+
SessionToken: FAKE_STS_SESSION_TOKEN,
600+
Expiration: new Date(8640000000000000),
601+
},
602+
})
603+
.resolves({
604+
Credentials: {
605+
AccessKeyId: FAKE_STS_ACCESS_KEY_ID,
606+
SecretAccessKey: 'asdf',
607+
SessionToken: FAKE_STS_SESSION_TOKEN,
608+
Expiration: new Date(8640000000000000),
609+
},
610+
});
610611

611612
await run();
612613

613-
expect(mockedSTS.commandCalls(AssumeRoleCommand).length).toEqual(1);
614+
expect(mockedSTS.commandCalls(AssumeRoleCommand).length).toEqual(2);
614615
});
615616

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

0 commit comments

Comments
 (0)