Skip to content

Commit

Permalink
fix: Default username in RoleSessionName (#20188)
Browse files Browse the repository at this point in the history
In case user does not have entry in `/etc/passwd` the `os.userInfo()`
call will throw `SystemError` exception as documented:
https://nodejs.org/docs/latest-v16.x/api/os.html#osuserinfooptions

Fixes #19401 issue.

It can be tested inside Docker for ad-hoc 1234 user ID:
```sh
docker run -u 1234 -e CDK_HOME=/tmp npm run cdk diff
```

The `CDK_HOME=/tmp` is a workaround for #7937 issue, where CDK complains
that it can't write cached info in user homedir, because it does not
exists.

Once #7937 will be fixed then #19401 will most likely hit users. However
above workaround is a viable option. Hence those two issues are related,
but not duplicated.


----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

Yes, followed the guide.

### Adding new Unconventional Dependencies:

* [x] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

No new dependencies.

### New Features

* [x] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [x] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

No, it's a bugfix, not a feature.

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
adambro committed May 27, 2022
1 parent 6f4aba8 commit b7bc10c
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 2 deletions.
6 changes: 5 additions & 1 deletion packages/aws-cdk/lib/api/aws-auth/sdk-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -459,7 +459,11 @@ function readIfPossible(filename: string): string | undefined {
* @see https://docs.aws.amazon.com/STS/latest/APIReference/API_AssumeRole.html#API_AssumeRole_RequestParameters
*/
function safeUsername() {
return os.userInfo().username.replace(/[^\w+=,.@-]/g, '@');
try {
return os.userInfo().username.replace(/[^\w+=,.@-]/g, '@');
} catch (e) {
return 'noname';
}
}

/**
Expand Down
28 changes: 28 additions & 0 deletions packages/aws-cdk/test/api/sdk-provider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,34 @@ describe('with intercepted network calls', () => {
});
});

test('assuming a role does not fail when OS username cannot be read', async () => {
// GIVEN
prepareCreds({
fakeSts,
config: {
default: { aws_access_key_id: 'foo', $account: '11111' },
},
});

await withMocked(os, 'userInfo', async (userInfo) => {
userInfo.mockImplementation(() => {
// SystemError thrown as documented: https://nodejs.org/docs/latest-v16.x/api/os.html#osuserinfooptions
throw new Error('SystemError on Linux: uv_os_get_passwd returned ENOENT. See #19401 issue.');
});

// WHEN
const provider = await providerFromProfile(undefined);

const sdk = (await provider.forEnvironment(env(uniq('88888')), Mode.ForReading, { assumeRoleArn: 'arn:aws:role' })).sdk as SDK;
await sdk.currentAccount();

// THEN
expect(fakeSts.assumedRoles[0]).toEqual(expect.objectContaining({
roleSessionName: 'aws-cdk-noname',
}));
});
});

test('even if current credentials are for the wrong account, we will still use them to AssumeRole', async () => {
// GIVEN
prepareCreds({
Expand Down
6 changes: 5 additions & 1 deletion packages/cdk-assets/lib/aws.ts
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,10 @@ export class DefaultAwsClient implements IAws {
* @see https://docs.aws.amazon.com/STS/latest/APIReference/API_AssumeRole.html#API_AssumeRole_RequestParameters
*/
function safeUsername() {
return os.userInfo().username.replace(/[^\w+=,.@-]/g, '@');
try {
return os.userInfo().username.replace(/[^\w+=,.@-]/g, '@');
} catch (e) {
return 'noname';
}
}

0 comments on commit b7bc10c

Please sign in to comment.