feat(secretsmanager): create secrets with specified values #18098

njlynch · 2021-12-20T16:51:15Z

Enables customers to supply their own secret value in the cases where an auto-
generated value is not viable. The secret value is typed to highlight the
inheret lack of safety with creating secret values via CloudFormation; if a
plaintext secret is provided, this secret will be visible anywhere the
CloudFormation template is, including the AWS Console, SDKs, and CLIs.

An unsafe fromUnsafePlaintext method and slightly safer fromToken method are
exposed to highlight the potential risks and hopefully encourage safe usage.
The latter is intended to be used directly with a Ref or GetAtt call from
another (Custom) Resource, such as storing the value of a User SecretAccessKey
or storing a password generated from a custom resource.

As an implementation detail, this API has been created using the new standard
for experimental APIs, via suffixing with Beta1. This allow us to make
breaking changes by deprecating the Beta1 version and creating an improved
Beta2 version. I've chosen to do this in this case because this has been a
relatively controversial feature to decide to implement, and the criteria for
what makes a secret "safe" may evolve over time. I am open to feedback on
whether this is necessitated.

fixes #5810

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

Enables customers to supply their own secret value in the cases where an auto- generated value is not viable. The secret value is typed to highlight the inheret lack of safety with creating secret values via CloudFormation; if a plaintext secret is provided, this secret will be visible anywhere the CloudFormation template is, including the AWS Console, SDKs, and CLIs. An unsafe `fromUnsafePlaintext` method and slightly safer `fromToken` method are exposed to highlight the potential risks and hopefully encourage safe usage. The latter is intended to be used directly with a Ref or GetAtt call from another (Custom) Resource, such as storing the value of a User SecretAccessKey or storing a password generated from a custom resource. As an implementation detail, this API has been created using the new standard for experimental APIs, via suffixing with `Beta1`. This allow us to make breaking changes by deprecating the `Beta1` version and creating an improved `Beta2` version. I've chosen to do this in this case because this has been a relatively controversial feature to decide to implement, and the criteria for what makes a secret "safe" may evolve over time. I am open to feedback on whether this is necessitated. fixes #5810

gitpod-io · 2021-12-20T16:51:17Z

eladb · 2021-12-20T18:29:16Z

packages/@aws-cdk/aws-secretsmanager/lib/secret.ts

+  /**
+   * Creates a `SecretStringValueBeta1` from a plaintext value.
+   * This approach is inherently unsafe, as the secret value will be visible in plaintext in the resulting
+   * CloudFormation template, including in the AWS Console or APIs.


It will also be visible in the source code, which is also considered a bad practice.

It may also be visible in the source code.

It may be visible, it may not:

const secretValue = SecretStringValueBeta1. fromUnsafePlaintext(os.env.MY_SECRET_NOT_STORED_IN_SOURCE);

Yes, still worth mentioning

packages/@aws-cdk/aws-secretsmanager/lib/secret.ts

eladb · 2021-12-20T18:30:53Z

packages/@aws-cdk/aws-secretsmanager/lib/secret.ts

+   * This method throws if it determines the input is an unsafe plaintext string.
+   *
+   * For example:
+   *     // Creates a new IAM user, access and secret keys, and stores the secret access key in a Secret.


Will this be considered an example by Rosetta? (@rix0rrr)

I don't believe it will in this format; however, the same example (from the tests) should be picked up.

I believe if you use backtick code blocks, Rosetta will pick it up and translate it:

```ts /// Creates a new IAM... bla bla ```

eladb · 2021-12-20T18:32:42Z

packages/@aws-cdk/aws-secretsmanager/lib/secret.ts

+   *     }));
+   *
+   * Note that the value being a Token does *not* guarantee safety. For example, a Lazy-evaluated string
+   * (e.g., `Lazy.string({ produce: () => 'myInsecurePassword' }))`) is a Token, but as the output is


We can technically check that (i.e resolve the token and see if it's eventually a Ref or a GetAtt). Resolution can be done inside a wrapping token.

Yes. This is a bit complex and/or error-prone, however, which is why I opted for the simplest solution to start.

Simply checking for the presence of Ref in the string doesn't guarantee it's a CloudFormation reference (e.g., passRef1234). I can try to regex it, but then I have to account for nuances in formatting, especially as this Ref/GetAtt can be embedded inside an arbitrary JSON structure. Something like /{\s*"?\s*(Ref|Fn::GetAtt)\s*"?\s*:/m might work. I worry about unintentionally blocking an edge case we haven't thought of this way. WDYT? Is it worth the ugly regex and potentially blocking something if we've missed vs protecting against people passing in a Lazy value?

mergify · 2021-12-21T17:41:41Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation · 2021-12-21T18:23:36Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
Commit ID: c21ef11
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify · 2021-12-21T18:24:02Z

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

Enables customers to supply their own secret value in the cases where an auto- generated value is not viable. The secret value is typed to highlight the inheret lack of safety with creating secret values via CloudFormation; if a plaintext secret is provided, this secret will be visible anywhere the CloudFormation template is, including the AWS Console, SDKs, and CLIs. An unsafe `fromUnsafePlaintext` method and slightly safer `fromToken` method are exposed to highlight the potential risks and hopefully encourage safe usage. The latter is intended to be used directly with a Ref or GetAtt call from another (Custom) Resource, such as storing the value of a User SecretAccessKey or storing a password generated from a custom resource. As an implementation detail, this API has been created using the new standard for experimental APIs, via suffixing with `Beta1`. This allow us to make breaking changes by deprecating the `Beta1` version and creating an improved `Beta2` version. I've chosen to do this in this case because this has been a relatively controversial feature to decide to implement, and the criteria for what makes a secret "safe" may evolve over time. I am open to feedback on whether this is necessitated. fixes aws#5810 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

njlynch added the pr/requires-two-approvers This PR is critical (e.g., security, broadly-impacting) and requires 2 approvers to be merged. label Dec 20, 2021

njlynch requested a review from a team December 20, 2021 16:51

njlynch self-assigned this Dec 20, 2021

github-actions bot added the @aws-cdk/aws-secretsmanager Related to AWS Secrets Manager label Dec 20, 2021

mergify bot added the contribution/core This is a PR that came from AWS. label Dec 20, 2021

kellertk mentioned this pull request Dec 20, 2021

Aws cdk don't provide a way to create secrets with specified values #5810

Closed

eladb reviewed Dec 20, 2021

View reviewed changes

njlynch requested a review from eladb December 21, 2021 10:28

chore: updating docstrings

4613f0c

eladb approved these changes Dec 21, 2021

View reviewed changes

njlynch removed the pr/requires-two-approvers This PR is critical (e.g., security, broadly-impacting) and requires 2 approvers to be merged. label Dec 21, 2021

chore: updating docstrings for rosetta

5f7e60d

Merge branch 'master' into njlynch/secret-value

c21ef11

mergify bot merged commit dd90b8e into master Dec 21, 2021

mergify bot deleted the njlynch/secret-value branch December 21, 2021 18:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(secretsmanager): create secrets with specified values #18098

feat(secretsmanager): create secrets with specified values #18098

njlynch commented Dec 20, 2021

gitpod-io bot commented Dec 20, 2021

eladb Dec 20, 2021

jlk Dec 20, 2021

njlynch Dec 21, 2021

eladb Dec 21, 2021

eladb Dec 20, 2021

njlynch Dec 21, 2021

eladb Dec 21, 2021

eladb Dec 20, 2021

njlynch Dec 21, 2021

mergify bot commented Dec 21, 2021

aws-cdk-automation commented Dec 21, 2021

mergify bot commented Dec 21, 2021

feat(secretsmanager): create secrets with specified values #18098

feat(secretsmanager): create secrets with specified values #18098

Conversation

njlynch commented Dec 20, 2021

gitpod-io bot commented Dec 20, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mergify bot commented Dec 21, 2021

aws-cdk-automation commented Dec 21, 2021

AWS CodeBuild CI Report

mergify bot commented Dec 21, 2021