Skip to content

Commit 789e8d2

Browse files
rix0rrrrix0rrr
authored
feat: check for accidental exposure of secrets (#19543)
Introduces the following changes to secrets handling: - Deprecate `SecretValue.plainText()` in favor of `SecretValue.unsafePlainText()` to highlight its issues - Introduce `SecretValue.resourceAttribute()` for cases where we know we want to use a value produced at deployment time - Introduce a new feature flag (`@aws-cdk/core:checkSecretUsage`) which, when enabled, will make it impossible to use secrets without explicitly unwrapping them first (`secretValue.unsafeUnwrap`). This prevents people from naively sticking secrets into vulnerable locations like environment variables. - Deprecate `secretsmanager.SecretStringValueBeta1` in favor of just accepting a `SecretValue`. Since this behavior would constitute a breaking change, it will only be enabled if the feature flag `@aws-cdk/core:checkSecretUsage` is turned on. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent e217c79 commit 789e8d2

File tree

76 files changed

+475
-220
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

76 files changed

+475
-220
lines changed

‎packages/@aws-cdk/app-delivery/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ const source = new codepipeline_actions.GitHubSourceAction({
8383
output: sourceOutput,
8484
owner: 'myName',
8585
repo: 'myRepo',
86-
oauthToken: cdk.SecretValue.plainText('secret'),
86+
oauthToken: cdk.SecretValue.unsafePlainText('secret'),
8787
});
8888
pipeline.addStage({
8989
stageName: 'source',

‎packages/@aws-cdk/app-delivery/test/integ.cicd.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ const source = new cpactions.GitHubSourceAction({
1818
actionName: 'GitHub',
1919
owner: 'awslabs',
2020
repo: 'aws-cdk',
21-
oauthToken: cdk.SecretValue.plainText('DummyToken'),
21+
oauthToken: cdk.SecretValue.unsafePlainText('DummyToken'),
2222
trigger: cpactions.GitHubTrigger.POLL,
2323
output: sourceOutput,
2424
});

‎packages/@aws-cdk/aws-amplify/lib/app.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ export interface SourceCodeProviderConfig {
3737
* to create webhook and read-only deploy key. OAuth token is not stored.
3838
*
3939
* Either `accessToken` or `oauthToken` must be specified if `repository`
40-
* is sepcified.
40+
* is specified.
4141
*
4242
* @default - do not use a token
4343
*/
@@ -223,7 +223,7 @@ export class App extends Resource implements IApp, iam.IGrantable {
223223
const sourceCodeProviderOptions = props.sourceCodeProvider?.bind(this);
224224

225225
const app = new CfnApp(this, 'Resource', {
226-
accessToken: sourceCodeProviderOptions?.accessToken?.toString(),
226+
accessToken: sourceCodeProviderOptions?.accessToken?.unsafeUnwrap(), // Safe usage
227227
autoBranchCreationConfig: props.autoBranchCreation && {
228228
autoBranchCreationPatterns: props.autoBranchCreation.patterns,
229229
basicAuthConfig: props.autoBranchCreation.basicAuth
@@ -247,7 +247,7 @@ export class App extends Resource implements IApp, iam.IGrantable {
247247
environmentVariables: Lazy.any({ produce: () => renderEnvironmentVariables(this.environmentVariables) }, { omitEmptyArray: true }),
248248
iamServiceRole: role.roleArn,
249249
name: props.appName || this.node.id,
250-
oauthToken: sourceCodeProviderOptions?.oauthToken?.toString(),
250+
oauthToken: sourceCodeProviderOptions?.oauthToken?.unsafeUnwrap(), // Safe usage
251251
repository: sourceCodeProviderOptions?.repository,
252252
customHeaders: props.customResponseHeaders ? renderCustomResponseHeaders(props.customResponseHeaders) : undefined,
253253
});

‎packages/@aws-cdk/aws-amplify/lib/basic-auth.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ export class BasicAuth {
9191
if (this.props.password) {
9292
return {
9393
...config,
94-
password: this.props.password.toString(),
94+
password: this.props.password.unsafeUnwrap(), // Safe usage
9595
};
9696
}
9797

@@ -103,7 +103,7 @@ export class BasicAuth {
103103
});
104104
return {
105105
...config,
106-
password: secret.secretValueFromJson('password').toString(),
106+
password: secret.secretValueFromJson('password').unsafeUnwrap(),
107107
};
108108
}
109109
}

‎packages/@aws-cdk/aws-amplify/test/app.test.ts

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ test('create an app connected to a GitHub repository', () => {
1515
sourceCodeProvider: new amplify.GitHubSourceCodeProvider({
1616
owner: 'aws',
1717
repository: 'aws-cdk',
18-
oauthToken: SecretValue.plainText('secret'),
18+
oauthToken: SecretValue.unsafePlainText('secret'),
1919
}),
2020
buildSpec: codebuild.BuildSpec.fromObjectToYaml({
2121
version: '1.0',
@@ -70,7 +70,7 @@ test('create an app connected to a GitLab repository', () => {
7070
sourceCodeProvider: new amplify.GitLabSourceCodeProvider({
7171
owner: 'aws',
7272
repository: 'aws-cdk',
73-
oauthToken: SecretValue.plainText('secret'),
73+
oauthToken: SecretValue.unsafePlainText('secret'),
7474
}),
7575
buildSpec: codebuild.BuildSpec.fromObject({
7676
version: '1.0',
@@ -194,9 +194,9 @@ test('with basic auth from credentials', () => {
194194
sourceCodeProvider: new amplify.GitHubSourceCodeProvider({
195195
owner: 'aws',
196196
repository: 'aws-cdk',
197-
oauthToken: SecretValue.plainText('secret'),
197+
oauthToken: SecretValue.unsafePlainText('secret'),
198198
}),
199-
basicAuth: amplify.BasicAuth.fromCredentials('username', SecretValue.plainText('password')),
199+
basicAuth: amplify.BasicAuth.fromCredentials('username', SecretValue.unsafePlainText('password')),
200200
});
201201

202202
// THEN
@@ -215,7 +215,7 @@ test('with basic auth from generated password', () => {
215215
sourceCodeProvider: new amplify.GitHubSourceCodeProvider({
216216
owner: 'aws',
217217
repository: 'aws-cdk',
218-
oauthToken: SecretValue.plainText('secret'),
218+
oauthToken: SecretValue.unsafePlainText('secret'),
219219
}),
220220
basicAuth: amplify.BasicAuth.fromGeneratedPassword('username'),
221221
});
@@ -254,7 +254,7 @@ test('with env vars', () => {
254254
sourceCodeProvider: new amplify.GitHubSourceCodeProvider({
255255
owner: 'aws',
256256
repository: 'aws-cdk',
257-
oauthToken: SecretValue.plainText('secret'),
257+
oauthToken: SecretValue.unsafePlainText('secret'),
258258
}),
259259
environmentVariables: {
260260
key1: 'value1',
@@ -283,7 +283,7 @@ test('with custom rules', () => {
283283
sourceCodeProvider: new amplify.GitHubSourceCodeProvider({
284284
owner: 'aws',
285285
repository: 'aws-cdk',
286-
oauthToken: SecretValue.plainText('secret'),
286+
oauthToken: SecretValue.unsafePlainText('secret'),
287287
}),
288288
customRules: [
289289
{
@@ -322,7 +322,7 @@ test('with SPA redirect', () => {
322322
sourceCodeProvider: new amplify.GitHubSourceCodeProvider({
323323
owner: 'aws',
324324
repository: 'aws-cdk',
325-
oauthToken: SecretValue.plainText('secret'),
325+
oauthToken: SecretValue.unsafePlainText('secret'),
326326
}),
327327
customRules: [amplify.CustomRule.SINGLE_PAGE_APPLICATION_REDIRECT],
328328
});
@@ -345,7 +345,7 @@ test('with auto branch creation', () => {
345345
sourceCodeProvider: new amplify.GitHubSourceCodeProvider({
346346
owner: 'aws',
347347
repository: 'aws-cdk',
348-
oauthToken: SecretValue.plainText('secret'),
348+
oauthToken: SecretValue.unsafePlainText('secret'),
349349
}),
350350
autoBranchCreation: {
351351
environmentVariables: {
@@ -384,7 +384,7 @@ test('with auto branch deletion', () => {
384384
sourceCodeProvider: new amplify.GitHubSourceCodeProvider({
385385
owner: 'aws',
386386
repository: 'aws-cdk',
387-
oauthToken: SecretValue.plainText('secret'),
387+
oauthToken: SecretValue.unsafePlainText('secret'),
388388
}),
389389
autoBranchDeletion: true,
390390
});
@@ -401,7 +401,7 @@ test('with custom headers', () => {
401401
sourceCodeProvider: new amplify.GitHubSourceCodeProvider({
402402
owner: 'aws',
403403
repository: 'aws-cdk',
404-
oauthToken: SecretValue.plainText('secret'),
404+
oauthToken: SecretValue.unsafePlainText('secret'),
405405
}),
406406
customResponseHeaders: [
407407
{

‎packages/@aws-cdk/aws-amplify/test/branch.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ beforeEach(() => {
1212
sourceCodeProvider: new amplify.GitHubSourceCodeProvider({
1313
owner: 'aws',
1414
repository: 'aws-cdk',
15-
oauthToken: SecretValue.plainText('secret'),
15+
oauthToken: SecretValue.unsafePlainText('secret'),
1616
}),
1717
});
1818
});
@@ -38,7 +38,7 @@ test('create a branch', () => {
3838
test('with basic auth from credentials', () => {
3939
// WHEN
4040
app.addBranch('dev', {
41-
basicAuth: amplify.BasicAuth.fromCredentials('username', SecretValue.plainText('password')),
41+
basicAuth: amplify.BasicAuth.fromCredentials('username', SecretValue.unsafePlainText('password')),
4242
});
4343

4444
// THEN

‎packages/@aws-cdk/aws-amplify/test/domain.test.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ test('create a domain', () => {
1010
sourceCodeProvider: new amplify.GitHubSourceCodeProvider({
1111
owner: 'aws',
1212
repository: 'aws-cdk',
13-
oauthToken: SecretValue.plainText('secret'),
13+
oauthToken: SecretValue.unsafePlainText('secret'),
1414
}),
1515
});
1616
const prodBranch = app.addBranch('master');
@@ -71,7 +71,7 @@ test('map a branch to the domain root', () => {
7171
sourceCodeProvider: new amplify.GitHubSourceCodeProvider({
7272
owner: 'aws',
7373
repository: 'aws-cdk',
74-
oauthToken: SecretValue.plainText('secret'),
74+
oauthToken: SecretValue.unsafePlainText('secret'),
7575
}),
7676
});
7777
const prodBranch = app.addBranch('master');
@@ -111,7 +111,7 @@ test('throws at synthesis without subdomains', () => {
111111
sourceCodeProvider: new amplify.GitHubSourceCodeProvider({
112112
owner: 'aws',
113113
repository: 'aws-cdk',
114-
oauthToken: SecretValue.plainText('secret'),
114+
oauthToken: SecretValue.unsafePlainText('secret'),
115115
}),
116116
});
117117

@@ -129,7 +129,7 @@ test('auto subdomain all branches', () => {
129129
sourceCodeProvider: new amplify.GitHubSourceCodeProvider({
130130
owner: 'aws',
131131
repository: 'aws-cdk',
132-
oauthToken: SecretValue.plainText('secret'),
132+
oauthToken: SecretValue.unsafePlainText('secret'),
133133
}),
134134
});
135135
const prodBranch = app.addBranch('master');
@@ -163,7 +163,7 @@ test('auto subdomain some branches', () => {
163163
sourceCodeProvider: new amplify.GitHubSourceCodeProvider({
164164
owner: 'aws',
165165
repository: 'aws-cdk',
166-
oauthToken: SecretValue.plainText('secret'),
166+
oauthToken: SecretValue.unsafePlainText('secret'),
167167
}),
168168
});
169169
const prodBranch = app.addBranch('master');
@@ -195,7 +195,7 @@ test('auto subdomain with IAM role', () => {
195195
sourceCodeProvider: new amplify.GitHubSourceCodeProvider({
196196
owner: 'aws',
197197
repository: 'aws-cdk',
198-
oauthToken: SecretValue.plainText('secret'),
198+
oauthToken: SecretValue.unsafePlainText('secret'),
199199
}),
200200
role: iam.Role.fromRoleArn(
201201
stack,
@@ -230,4 +230,4 @@ test('auto subdomain with IAM role', () => {
230230
],
231231
},
232232
});
233-
});
233+
});

‎packages/@aws-cdk/aws-apigatewayv2-authorizers/test/http/integ.iam.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ new cdk.CfnOutput(stack, 'TESTACCESSKEYID', {
4848
});
4949

5050
new cdk.CfnOutput(stack, 'TESTSECRETACCESSKEY', {
51-
value: userAccessKey.secretAccessKey.toString(),
51+
value: userAccessKey.secretAccessKey.unsafeUnwrap(),
5252
});
5353

5454
new cdk.CfnOutput(stack, 'TESTREGION', {

‎packages/@aws-cdk/aws-codebuild/lib/source-credentials.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ export class GitHubSourceCredentials extends Resource {
2828
new CfnSourceCredential(this, 'Resource', {
2929
serverType: 'GITHUB',
3030
authType: 'PERSONAL_ACCESS_TOKEN',
31-
token: props.accessToken.toString(),
31+
token: props.accessToken.unsafeUnwrap(), // Safe usage
3232
});
3333
}
3434
}
@@ -60,7 +60,7 @@ export class GitHubEnterpriseSourceCredentials extends Resource {
6060
new CfnSourceCredential(this, 'Resource', {
6161
serverType: 'GITHUB_ENTERPRISE',
6262
authType: 'PERSONAL_ACCESS_TOKEN',
63-
token: props.accessToken.toString(),
63+
token: props.accessToken.unsafeUnwrap(), // Safe usage
6464
});
6565
}
6666
}
@@ -92,8 +92,8 @@ export class BitBucketSourceCredentials extends Resource {
9292
new CfnSourceCredential(this, 'Resource', {
9393
serverType: 'BITBUCKET',
9494
authType: 'BASIC_AUTH',
95-
username: props.username.toString(),
96-
token: props.password.toString(),
95+
username: props.username.unsafeUnwrap(), // Safe usage
96+
token: props.password.unsafeUnwrap(), // Safe usage
9797
});
9898
}
9999
}

‎packages/@aws-cdk/aws-codebuild/test/project.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,7 @@ describe('GitHub source', () => {
202202

203203
// WHEN
204204
new codebuild.GitHubSourceCredentials(stack, 'GitHubSourceCredentials', {
205-
accessToken: cdk.SecretValue.plainText('my-access-token'),
205+
accessToken: cdk.SecretValue.unsafePlainText('my-access-token'),
206206
});
207207

208208
// THEN
@@ -239,7 +239,7 @@ describe('GitHub Enterprise source', () => {
239239

240240
// WHEN
241241
new codebuild.GitHubEnterpriseSourceCredentials(stack, 'GitHubEnterpriseSourceCredentials', {
242-
accessToken: cdk.SecretValue.plainText('my-access-token'),
242+
accessToken: cdk.SecretValue.unsafePlainText('my-access-token'),
243243
});
244244

245245
// THEN
@@ -277,8 +277,8 @@ describe('BitBucket source', () => {
277277

278278
// WHEN
279279
new codebuild.BitBucketSourceCredentials(stack, 'BitBucketSourceCredentials', {
280-
username: cdk.SecretValue.plainText('my-username'),
281-
password: cdk.SecretValue.plainText('password'),
280+
username: cdk.SecretValue.unsafePlainText('my-username'),
281+
password: cdk.SecretValue.unsafePlainText('password'),
282282
});
283283

284284
// THEN

0 commit comments

Comments
 (0)