Skip to content

Commit 816f5cc

Browse files
mergify[bot]mergify[bot]
authored
fix: mask both source and role credentials (#40)
1 parent a5771a0 commit 816f5cc

File tree

2 files changed

+75
-41
lines changed

2 files changed

+75
-41
lines changed

‎index.js

+34-23
Original file line numberDiff line numberDiff line change
@@ -16,17 +16,15 @@ async function assumeRole(params) {
1616
const isDefined = i => !!i;
1717

1818
const {
19+
sourceAccountId,
1920
roleToAssume,
2021
roleExternalId,
2122
roleDurationSeconds,
2223
roleSessionName,
23-
accessKeyId,
24-
secretAccessKey,
25-
sessionToken,
2624
region,
2725
} = params;
2826
assert(
29-
[roleToAssume, roleDurationSeconds, roleSessionName, accessKeyId, secretAccessKey, region].every(isDefined),
27+
[sourceAccountId, roleToAssume, roleDurationSeconds, roleSessionName, region].every(isDefined),
3028
"Missing required input when assuming a Role."
3129
);
3230

@@ -36,18 +34,12 @@ async function assumeRole(params) {
3634
'Missing required environment value. Are you running in GitHub Actions?'
3735
);
3836

39-
const endpoint = util.format('https://sts.%s.amazonaws.com', region);
40-
41-
const sts = new aws.STS({
42-
accessKeyId, secretAccessKey, sessionToken, region, endpoint, customUserAgent: USER_AGENT
43-
});
37+
const sts = getStsClient(region);
4438

4539
let roleArn = roleToAssume;
4640
if (!roleArn.startsWith('arn:aws')) {
47-
const identity = await sts.getCallerIdentity().promise();
48-
const accountId = identity.Account;
4941
// Supports only 'aws' partition. Customers in other partitions ('aws-cn') will need to provide full ARN
50-
roleArn = `arn:aws:iam::${accountId}:role/${roleArn}`;
42+
roleArn = `arn:aws:iam::${sourceAccountId}:role/${roleArn}`;
5143
}
5244

5345
const assumeRoleRequest = {
@@ -125,15 +117,25 @@ function exportRegion(region) {
125117
core.exportVariable('AWS_REGION', region);
126118
}
127119

128-
async function exportAccountId(maskAccountId) {
120+
async function exportAccountId(maskAccountId, region) {
129121
// Get the AWS account ID
130-
const sts = new aws.STS({customUserAgent: USER_AGENT});
122+
const sts = getStsClient(region);
131123
const identity = await sts.getCallerIdentity().promise();
132124
const accountId = identity.Account;
133125
core.setOutput('aws-account-id', accountId);
134126
if (!maskAccountId || maskAccountId.toLowerCase() == 'true') {
135127
core.setSecret(accountId);
136128
}
129+
return accountId;
130+
}
131+
132+
function getStsClient(region) {
133+
const endpoint = util.format('https://sts.%s.amazonaws.com', region);
134+
return new aws.STS({
135+
region,
136+
endpoint,
137+
customUserAgent: USER_AGENT
138+
});
137139
}
138140

139141
async function run() {
@@ -149,19 +151,28 @@ async function run() {
149151
const roleDurationSeconds = core.getInput('role-duration-seconds', {required: false}) || MAX_ACTION_RUNTIME;
150152
const roleSessionName = core.getInput('role-session-name', { required: false }) || ROLE_SESSION_NAME;
151153

154+
// Always export the source credentials and account ID.
155+
// The STS client for calling AssumeRole pulls creds from the environment.
156+
// Plus, in the assume role case, if the AssumeRole call fails, we want
157+
// the source credentials and accound ID to already be masked as secrets
158+
// in any error messages.
159+
exportRegion(region);
160+
exportCredentials({accessKeyId, secretAccessKey, sessionToken});
161+
const sourceAccountId = await exportAccountId(maskAccountId, region);
162+
152163
// Get role credentials if configured to do so
153164
if (roleToAssume) {
154-
const roleCredentials = await assumeRole(
155-
{accessKeyId, secretAccessKey, sessionToken, region, roleToAssume, roleExternalId, roleDurationSeconds, roleSessionName}
156-
);
165+
const roleCredentials = await assumeRole({
166+
sourceAccountId,
167+
region,
168+
roleToAssume,
169+
roleExternalId,
170+
roleDurationSeconds,
171+
roleSessionName
172+
});
157173
exportCredentials(roleCredentials);
158-
} else {
159-
exportCredentials({accessKeyId, secretAccessKey, sessionToken});
174+
await exportAccountId(maskAccountId, region);
160175
}
161-
162-
exportRegion(region);
163-
164-
await exportAccountId(maskAccountId);
165176
}
166177
catch (error) {
167178
core.setFailed(error.message);

‎index.test.js

+41-18
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,9 @@ const FAKE_STS_SECRET_ACCESS_KEY = 'STS-AWS-SECRET-ACCESS-KEY';
1313
const FAKE_STS_SESSION_TOKEN = 'STS-AWS-SESSION-TOKEN';
1414
const FAKE_REGION = 'fake-region-1';
1515
const FAKE_ACCOUNT_ID = '123456789012';
16+
const FAKE_ROLE_ACCOUNT_ID = '111111111111';
1617
const ROLE_NAME = 'MY-ROLE';
17-
const ROLE_ARN = 'arn:aws:iam::123456789012:role/MY-ROLE';
18+
const ROLE_ARN = 'arn:aws:iam::111111111111:role/MY-ROLE';
1819
const ENVIRONMENT_VARIABLE_OVERRIDES = {
1920
SHOW_STACK_TRACE: 'true',
2021
GITHUB_REPOSITORY: 'MY-REPOSITORY-NAME',
@@ -68,13 +69,18 @@ describe('Configure AWS Credentials', () => {
6869
.fn()
6970
.mockImplementation(mockGetInput(DEFAULT_INPUTS));
7071

71-
mockStsCallerIdentity.mockImplementation(() => {
72-
return {
72+
mockStsCallerIdentity.mockReset();
73+
mockStsCallerIdentity
74+
.mockReturnValueOnce({
7375
promise() {
7476
return Promise.resolve({ Account: FAKE_ACCOUNT_ID });
7577
}
76-
};
77-
});
78+
})
79+
.mockReturnValueOnce({
80+
promise() {
81+
return Promise.resolve({ Account: FAKE_ROLE_ACCOUNT_ID });
82+
}
83+
});
7884

7985
mockStsAssumeRole.mockImplementation(() => {
8086
return {
@@ -154,6 +160,7 @@ describe('Configure AWS Credentials', () => {
154160
test('error is caught by core.setFailed and caught', async () => {
155161
process.env.SHOW_STACK_TRACE = 'false';
156162

163+
mockStsCallerIdentity.mockReset();
157164
mockStsCallerIdentity.mockImplementation(() => {
158165
throw new Error();
159166
});
@@ -165,6 +172,7 @@ describe('Configure AWS Credentials', () => {
165172

166173
test('error is caught by core.setFailed and passed', async () => {
167174

175+
mockStsCallerIdentity.mockReset();
168176
mockStsCallerIdentity.mockImplementation(() => {
169177
throw new Error();
170178
});
@@ -181,18 +189,33 @@ describe('Configure AWS Credentials', () => {
181189

182190
await run();
183191
expect(mockStsAssumeRole).toHaveBeenCalledTimes(1);
184-
expect(core.exportVariable).toHaveBeenCalledTimes(5);
185-
expect(core.setSecret).toHaveBeenCalledTimes(4);
186-
expect(core.exportVariable).toHaveBeenCalledWith('AWS_ACCESS_KEY_ID', FAKE_STS_ACCESS_KEY_ID);
187-
expect(core.setSecret).toHaveBeenCalledWith(FAKE_STS_ACCESS_KEY_ID);
188-
expect(core.exportVariable).toHaveBeenCalledWith('AWS_SECRET_ACCESS_KEY', FAKE_STS_SECRET_ACCESS_KEY);
189-
expect(core.setSecret).toHaveBeenCalledWith(FAKE_STS_SECRET_ACCESS_KEY);
190-
expect(core.exportVariable).toHaveBeenCalledWith('AWS_SESSION_TOKEN', FAKE_STS_SESSION_TOKEN);
191-
expect(core.setSecret).toHaveBeenCalledWith(FAKE_STS_SESSION_TOKEN);
192-
expect(core.exportVariable).toHaveBeenCalledWith('AWS_DEFAULT_REGION', FAKE_REGION);
193-
expect(core.exportVariable).toHaveBeenCalledWith('AWS_REGION', FAKE_REGION);
194-
expect(core.setOutput).toHaveBeenCalledWith('aws-account-id', FAKE_ACCOUNT_ID);
195-
expect(core.setSecret).toHaveBeenCalledWith(FAKE_ACCOUNT_ID);
192+
expect(core.exportVariable).toHaveBeenCalledTimes(7);
193+
expect(core.setSecret).toHaveBeenCalledTimes(7);
194+
expect(core.setOutput).toHaveBeenCalledTimes(2);
195+
196+
// first the source credentials are exported and masked
197+
expect(core.setSecret).toHaveBeenNthCalledWith(1, FAKE_ACCESS_KEY_ID);
198+
expect(core.setSecret).toHaveBeenNthCalledWith(2, FAKE_SECRET_ACCESS_KEY);
199+
expect(core.setSecret).toHaveBeenNthCalledWith(3, FAKE_ACCOUNT_ID);
200+
201+
expect(core.exportVariable).toHaveBeenNthCalledWith(1, 'AWS_DEFAULT_REGION', FAKE_REGION);
202+
expect(core.exportVariable).toHaveBeenNthCalledWith(2, 'AWS_REGION', FAKE_REGION);
203+
expect(core.exportVariable).toHaveBeenNthCalledWith(3, 'AWS_ACCESS_KEY_ID', FAKE_ACCESS_KEY_ID);
204+
expect(core.exportVariable).toHaveBeenNthCalledWith(4, 'AWS_SECRET_ACCESS_KEY', FAKE_SECRET_ACCESS_KEY);
205+
206+
expect(core.setOutput).toHaveBeenNthCalledWith(1, 'aws-account-id', FAKE_ACCOUNT_ID);
207+
208+
// then the role credentials are exported and masked
209+
expect(core.setSecret).toHaveBeenNthCalledWith(4, FAKE_STS_ACCESS_KEY_ID);
210+
expect(core.setSecret).toHaveBeenNthCalledWith(5, FAKE_STS_SECRET_ACCESS_KEY);
211+
expect(core.setSecret).toHaveBeenNthCalledWith(6, FAKE_STS_SESSION_TOKEN);
212+
expect(core.setSecret).toHaveBeenNthCalledWith(7, FAKE_ROLE_ACCOUNT_ID);
213+
214+
expect(core.exportVariable).toHaveBeenNthCalledWith(5, 'AWS_ACCESS_KEY_ID', FAKE_STS_ACCESS_KEY_ID);
215+
expect(core.exportVariable).toHaveBeenNthCalledWith(6, 'AWS_SECRET_ACCESS_KEY', FAKE_STS_SECRET_ACCESS_KEY);
216+
expect(core.exportVariable).toHaveBeenNthCalledWith(7, 'AWS_SESSION_TOKEN', FAKE_STS_SESSION_TOKEN);
217+
218+
expect(core.setOutput).toHaveBeenNthCalledWith(2, 'aws-account-id', FAKE_ROLE_ACCOUNT_ID);
196219
});
197220

198221
test('role assumption tags', async () => {
@@ -268,7 +291,7 @@ describe('Configure AWS Credentials', () => {
268291

269292
await run();
270293
expect(mockStsAssumeRole).toHaveBeenCalledWith({
271-
RoleArn: ROLE_ARN,
294+
RoleArn: 'arn:aws:iam::123456789012:role/MY-ROLE',
272295
RoleSessionName: 'GitHubActions',
273296
DurationSeconds: 6 * 3600,
274297
Tags: [

0 commit comments

Comments
 (0)