Skip to content

Commit c4be498

Browse files
kellertklehmanmj
kellertk
and
lehmanmj
authored
feat: support account id allowlist (#1456)
* feat: support account id allowlist * chore: update readme --------- Co-authored-by: Michael Lehmann <[email protected]>
1 parent c5a43c3 commit c4be498

File tree

6 files changed

+201
-13
lines changed

6 files changed

+201
-13
lines changed

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,7 @@ See [action.yml](./action.yml) for more detail.
150150
| retry-max-attempts | Limits the number of retry attempts before giving up. Defaults to 12. | No |
151151
| special-characters-workaround | Uncommonly, some environments cannot tolerate special characters in a secret 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. | No |
152152
| use-existing-credentials | When set, the action will check if existing credentials are valid and exit if they are. Defaults to false. | No |
153+
| allowed-account-ids | A comma-delimited list of expected AWS account IDs. The action will fail if we receive credentials for the wrong account. | No |
153154
| force-skip-oidc | When set, the action will skip using GitHub OIDC provider even if the id-token permission is set. | No |
154155
</details>
155156

‎action.yml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,11 @@ inputs:
7878
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
7979
required: false
8080
use-existing-credentials:
81+
required: false
8182
description: When enabled, this option will check if there are already valid credentials in the environment. If there are, new credentials will not be fetched. If there are not, the action will run as normal.
83+
allowed-account-ids:
84+
required: false
85+
description: An option comma-delimited list of expected AWS account IDs. The action will fail if we receive credentials for the wrong account.
8286
force-skip-oidc:
8387
required: false
8488
description: When enabled, this option will skip using GitHub OIDC provider even if the id-token permission is set. This is sometimes useful when using IAM instance credentials.

‎src/CredentialsClient.ts‎

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { STSClient } from '@aws-sdk/client-sts';
33
import type { AwsCredentialIdentity } from '@aws-sdk/types';
44
import { NodeHttpHandler } from '@smithy/node-http-handler';
55
import { HttpsProxyAgent } from 'https-proxy-agent';
6-
import { errorMessage } from './helpers';
6+
import { errorMessage, getCallerIdentity } from './helpers';
77

88
const USER_AGENT = 'configure-aws-credentials-for-github-actions';
99

@@ -40,7 +40,11 @@ export class CredentialsClient {
4040
return this._stsClient;
4141
}
4242

43-
public async validateCredentials(expectedAccessKeyId?: string, roleChaining?: boolean) {
43+
public async validateCredentials(
44+
expectedAccessKeyId?: string,
45+
roleChaining?: boolean,
46+
expectedAccountIds?: string[],
47+
) {
4448
let credentials: AwsCredentialIdentity;
4549
try {
4650
credentials = await this.loadCredentials();
@@ -50,13 +54,27 @@ export class CredentialsClient {
5054
} catch (error) {
5155
throw new Error(`Credentials could not be loaded, please check your action inputs: ${errorMessage(error)}`);
5256
}
57+
if (expectedAccountIds && expectedAccountIds.length > 0 && expectedAccountIds[0] !== '') {
58+
let callerIdentity: Awaited<ReturnType<typeof getCallerIdentity>>;
59+
try {
60+
callerIdentity = await getCallerIdentity(this.stsClient);
61+
} catch (error) {
62+
throw new Error(`Could not validate account ID of credentials: ${errorMessage(error)}`);
63+
}
64+
if (!callerIdentity.Account || !expectedAccountIds.includes(callerIdentity.Account)) {
65+
throw new Error(
66+
`The account ID of the provided credentials (${
67+
callerIdentity.Account ?? 'unknown'
68+
}) does not match any of the expected account IDs: ${expectedAccountIds.join(', ')}`,
69+
);
70+
}
71+
}
5372

5473
if (!roleChaining) {
5574
const actualAccessKeyId = credentials.accessKeyId;
56-
5775
if (expectedAccessKeyId && expectedAccessKeyId !== actualAccessKeyId) {
5876
throw new Error(
59-
'Unexpected failure: Credentials loaded by the SDK do not match the access key ID configured by the action',
77+
'Credentials loaded by the SDK do not match the expected access key ID configured by the action',
6078
);
6179
}
6280
}

‎src/helpers.ts‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import * as core from '@actions/core';
2-
import type { Credentials } from '@aws-sdk/client-sts';
2+
import type { Credentials, STSClient } from '@aws-sdk/client-sts';
33
import { GetCallerIdentityCommand } from '@aws-sdk/client-sts';
44
import type { CredentialsClient } from './CredentialsClient';
55

@@ -109,15 +109,19 @@ export function exportRegion(region: string, outputEnvCredentials?: boolean) {
109109
}
110110
}
111111

112+
export async function getCallerIdentity(client: STSClient): Promise<{ Account: string; Arn: string; UserId?: string }> {
113+
const identity = await client.send(new GetCallerIdentityCommand({}));
114+
if (!identity.Account || !identity.Arn) {
115+
throw new Error('Could not get Account ID or ARN from STS. Did you set credentials?');
116+
}
117+
return { Account: identity.Account, Arn: identity.Arn, UserId: identity.UserId };
118+
}
119+
112120
// Obtains account ID from STS Client and sets it as output
113121
export async function exportAccountId(credentialsClient: CredentialsClient, maskAccountId?: boolean) {
114-
const client = credentialsClient.stsClient;
115-
const identity = await client.send(new GetCallerIdentityCommand({}));
122+
const identity = await getCallerIdentity(credentialsClient.stsClient);
116123
const accountId = identity.Account;
117124
const arn = identity.Arn;
118-
if (!accountId || !arn) {
119-
throw new Error('Could not get Account ID or ARN from STS. Did you set credentials?');
120-
}
121125
if (maskAccountId) {
122126
core.setSecret(accountId);
123127
core.setSecret(arn);

‎src/index.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@ export async function run() {
5151
const specialCharacterWorkaround = getBooleanInput('special-characters-workaround', { required: false });
5252
const useExistingCredentials = core.getInput('use-existing-credentials', { required: false });
5353
let maxRetries = Number.parseInt(core.getInput('retry-max-attempts', { required: false })) || 12;
54+
const expectedAccountIds = core
55+
.getInput('allowed-account-ids', { required: false })
56+
.split(',')
57+
.map((s) => s.trim());
5458
const forceSkipOidc = getBooleanInput('force-skip-oidc', { required: false });
5559

5660
if (forceSkipOidc && roleToAssume && !AccessKeyId && !webIdentityTokenFile) {
@@ -144,15 +148,15 @@ export async function run() {
144148
exportCredentials({ AccessKeyId, SecretAccessKey, SessionToken }, outputCredentials, outputEnvCredentials);
145149
} else if (!webIdentityTokenFile && !roleChaining) {
146150
// Proceed only if credentials can be picked up
147-
await credentialsClient.validateCredentials();
151+
await credentialsClient.validateCredentials(undefined, roleChaining, expectedAccountIds);
148152
sourceAccountId = await exportAccountId(credentialsClient, maskAccountId);
149153
}
150154

151155
if (AccessKeyId || roleChaining) {
152156
// Validate that the SDK can actually pick up credentials.
153157
// This validates cases where this action is using existing environment credentials,
154158
// and cases where the user intended to provide input credentials but the secrets inputs resolved to empty strings.
155-
await credentialsClient.validateCredentials(AccessKeyId, roleChaining);
159+
await credentialsClient.validateCredentials(AccessKeyId, roleChaining, expectedAccountIds);
156160
sourceAccountId = await exportAccountId(credentialsClient, maskAccountId);
157161
}
158162

@@ -187,7 +191,11 @@ export async function run() {
187191
// is set to `true` then we are NOT in a self-hosted runner.
188192
// Second: Customer provided credentials manually (IAM User keys stored in GH Secrets)
189193
if (!process.env.GITHUB_ACTIONS || AccessKeyId) {
190-
await credentialsClient.validateCredentials(roleCredentials.Credentials?.AccessKeyId);
194+
await credentialsClient.validateCredentials(
195+
roleCredentials.Credentials?.AccessKeyId,
196+
roleChaining,
197+
expectedAccountIds,
198+
);
191199
}
192200
if (outputEnvCredentials) {
193201
await exportAccountId(credentialsClient, maskAccountId);

‎test/index.test.ts‎

Lines changed: 153 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,11 +399,13 @@ describe('Configure AWS Credentials', {}, () => {
399399
'force-skip-oidc': 'true'
400400
}));
401401
vi.spyOn(core, 'getIDToken').mockResolvedValue('testoidctoken');
402+
402403
mockedSTSClient.on(GetCallerIdentityCommand).resolves({ ...mocks.outputs.GET_CALLER_IDENTITY });
403404
// biome-ignore lint/suspicious/noExplicitAny: any required to mock private method
404405
vi.spyOn(CredentialsClient.prototype as any, 'loadCredentials').mockResolvedValue({
405406
accessKeyId: 'MYAWSACCESSKEYID',
406407
});
408+
407409
process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN = 'fake-token';
408410

409411
await run();
@@ -463,6 +465,157 @@ describe('Configure AWS Credentials', {}, () => {
463465
});
464466
});
465467

468+
describe('Account ID Validation', {}, () => {
469+
beforeEach(() => {
470+
vi.clearAllMocks();
471+
mockedSTSClient.reset();
472+
});
473+
474+
it('succeeds when account ID matches allowed list', async () => {
475+
vi.spyOn(core, 'getInput').mockImplementation(mocks.getInput({
476+
...mocks.IAM_USER_INPUTS,
477+
'allowed-account-ids': '111111111111'
478+
}));
479+
mockedSTSClient.on(GetCallerIdentityCommand).resolves({ ...mocks.outputs.GET_CALLER_IDENTITY });
480+
// biome-ignore lint/suspicious/noExplicitAny: any required to mock private method
481+
vi.spyOn(CredentialsClient.prototype as any, 'loadCredentials').mockResolvedValue({
482+
accessKeyId: 'MYAWSACCESSKEYID',
483+
});
484+
485+
await run();
486+
expect(core.setFailed).not.toHaveBeenCalled();
487+
expect(core.info).toHaveBeenCalledWith('Proceeding with IAM user credentials');
488+
});
489+
490+
it('succeeds with multiple allowed account IDs when account matches', async () => {
491+
vi.spyOn(core, 'getInput').mockImplementation(mocks.getInput({
492+
...mocks.IAM_USER_INPUTS,
493+
'allowed-account-ids': '999999999999,111111111111,222222222222'
494+
}));
495+
mockedSTSClient.on(GetCallerIdentityCommand).resolves({ ...mocks.outputs.GET_CALLER_IDENTITY });
496+
// biome-ignore lint/suspicious/noExplicitAny: any required to mock private method
497+
vi.spyOn(CredentialsClient.prototype as any, 'loadCredentials').mockResolvedValue({
498+
accessKeyId: 'MYAWSACCESSKEYID',
499+
});
500+
501+
await run();
502+
expect(core.setFailed).not.toHaveBeenCalled();
503+
});
504+
505+
it('fails when account ID does not match allowed list', async () => {
506+
vi.spyOn(core, 'getInput').mockImplementation(mocks.getInput({
507+
...mocks.IAM_USER_INPUTS,
508+
'allowed-account-ids': '999999999999'
509+
}));
510+
mockedSTSClient.on(GetCallerIdentityCommand).resolves({ ...mocks.outputs.GET_CALLER_IDENTITY });
511+
// biome-ignore lint/suspicious/noExplicitAny: any required to mock private method
512+
vi.spyOn(CredentialsClient.prototype as any, 'loadCredentials').mockResolvedValue({
513+
accessKeyId: 'MYAWSACCESSKEYID',
514+
});
515+
516+
await run();
517+
expect(core.setFailed).toHaveBeenCalledWith(
518+
'The account ID of the provided credentials (111111111111) does not match any of the expected account IDs: 999999999999'
519+
);
520+
});
521+
522+
it('fails when account ID does not match any in multiple allowed accounts', async () => {
523+
vi.spyOn(core, 'getInput').mockImplementation(mocks.getInput({
524+
...mocks.IAM_USER_INPUTS,
525+
'allowed-account-ids': '999999999999,888888888888'
526+
}));
527+
528+
mockedSTSClient.on(GetCallerIdentityCommand).resolves({ ...mocks.outputs.GET_CALLER_IDENTITY });
529+
// biome-ignore lint/suspicious/noExplicitAny: any required to mock private method
530+
vi.spyOn(CredentialsClient.prototype as any, 'loadCredentials').mockResolvedValue({
531+
accessKeyId: 'MYAWSACCESSKEYID',
532+
});
533+
534+
await run();
535+
expect(core.setFailed).toHaveBeenCalledWith(
536+
'The account ID of the provided credentials (111111111111) does not match any of the expected account IDs: 999999999999, 888888888888'
537+
);
538+
});
539+
540+
it('works with assume role when account ID matches', async () => {
541+
vi.spyOn(core, 'getInput').mockImplementation(mocks.getInput({
542+
...mocks.IAM_ASSUMEROLE_INPUTS,
543+
'allowed-account-ids': '111111111111'
544+
}));
545+
mockedSTSClient.on(AssumeRoleCommand).resolves(mocks.outputs.STS_CREDENTIALS);
546+
mockedSTSClient.on(GetCallerIdentityCommand).resolves({ ...mocks.outputs.GET_CALLER_IDENTITY });
547+
// biome-ignore lint/suspicious/noExplicitAny: any required to mock private method
548+
vi.spyOn(CredentialsClient.prototype as any, 'loadCredentials')
549+
.mockResolvedValueOnce({ accessKeyId: 'MYAWSACCESSKEYID' })
550+
.mockResolvedValueOnce({ accessKeyId: 'STSAWSACCESSKEYID' });
551+
552+
await run();
553+
expect(core.setFailed).not.toHaveBeenCalled();
554+
expect(core.info).toHaveBeenCalledWith('Authenticated as assumedRoleId AROAFAKEASSUMEDROLEID');
555+
});
556+
557+
it('works with OIDC when account ID matches', async () => {
558+
vi.spyOn(core, 'getInput').mockImplementation(mocks.getInput({
559+
...mocks.GH_OIDC_INPUTS,
560+
'allowed-account-ids': '111111111111'
561+
}));
562+
vi.spyOn(core, 'getIDToken').mockResolvedValue('testoidctoken');
563+
mockedSTSClient.on(AssumeRoleWithWebIdentityCommand).resolves(mocks.outputs.STS_CREDENTIALS);
564+
mockedSTSClient.on(GetCallerIdentityCommand).resolves({ ...mocks.outputs.GET_CALLER_IDENTITY });
565+
process.env.ACTIONS_ID_TOKEN_REQUEST_TOKEN = 'fake-token';
566+
567+
await run();
568+
expect(core.setFailed).not.toHaveBeenCalled();
569+
expect(core.info).toHaveBeenCalledWith('Authenticated as assumedRoleId AROAFAKEASSUMEDROLEID');
570+
});
571+
572+
it('handles GetCallerIdentity API failure gracefully', async () => {
573+
vi.spyOn(core, 'getInput').mockImplementation(mocks.getInput({
574+
...mocks.IAM_USER_INPUTS,
575+
'allowed-account-ids': '111111111111'
576+
}));
577+
mockedSTSClient.on(GetCallerIdentityCommand).rejects(new Error('API Error'));
578+
// biome-ignore lint/suspicious/noExplicitAny: any required to mock private method
579+
vi.spyOn(CredentialsClient.prototype as any, 'loadCredentials').mockResolvedValue({
580+
accessKeyId: 'MYAWSACCESSKEYID',
581+
});
582+
583+
await run();
584+
expect(core.setFailed).toHaveBeenCalledWith('Could not validate account ID of credentials: API Error');
585+
});
586+
587+
it('ignores validation when allowed-account-ids is empty', async () => {
588+
vi.spyOn(core, 'getInput').mockImplementation(mocks.getInput({
589+
...mocks.IAM_USER_INPUTS,
590+
'allowed-account-ids': ''
591+
}));
592+
mockedSTSClient.on(GetCallerIdentityCommand).resolves({ ...mocks.outputs.GET_CALLER_IDENTITY });
593+
// biome-ignore lint/suspicious/noExplicitAny: any required to mock private method
594+
vi.spyOn(CredentialsClient.prototype as any, 'loadCredentials').mockResolvedValue({
595+
accessKeyId: 'MYAWSACCESSKEYID',
596+
});
597+
598+
await run();
599+
expect(core.setFailed).not.toHaveBeenCalled();
600+
expect(core.info).toHaveBeenCalledWith('Proceeding with IAM user credentials');
601+
});
602+
603+
it('handles whitespace in allowed-account-ids input', async () => {
604+
vi.spyOn(core, 'getInput').mockImplementation(mocks.getInput({
605+
...mocks.IAM_USER_INPUTS,
606+
'allowed-account-ids': ' 111111111111 , 222222222222 '
607+
}));
608+
mockedSTSClient.on(GetCallerIdentityCommand).resolves({ ...mocks.outputs.GET_CALLER_IDENTITY });
609+
// biome-ignore lint/suspicious/noExplicitAny: any required to mock private method
610+
vi.spyOn(CredentialsClient.prototype as any, 'loadCredentials').mockResolvedValue({
611+
accessKeyId: 'MYAWSACCESSKEYID',
612+
});
613+
614+
await run();
615+
expect(core.setFailed).not.toHaveBeenCalled();
616+
});
617+
});
618+
466619
describe('HTTP Proxy Configuration', {}, () => {
467620
beforeEach(() => {
468621
vi.spyOn(core, 'getInput').mockImplementation(mocks.getInput(mocks.GH_OIDC_INPUTS));

0 commit comments

Comments
 (0)