Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

aws-s3-deployment: Source.jsonData does not escape quotes #22661

Open
revmischa opened this issue Oct 26, 2022 · 2 comments · May be fixed by #33698
Open

aws-s3-deployment: Source.jsonData does not escape quotes #22661

revmischa opened this issue Oct 26, 2022 · 2 comments · May be fixed by #33698
Assignees
Labels
@aws-cdk/aws-s3-deployment bug This issue is a bug. effort/small Small work item – less than a day of effort p1

Comments

@revmischa
Copy link
Contributor

Describe the bug

Doing something like

 new BucketDeployment(this, 'EnvJsonDeployment', {
      sources: [
        // warning: this doesn't escape quotes in unresolved tokens
        Source.jsonData(CONFIG_ENV_JSON_PATH, replacementParams),
      ],
      destinationBucket: bucket,
    });

Where replacementParams contains an unresolved value, for example a secrets manager token in an SSM param, then that value will be interpolated into the JSON file without escaping quotes.

e.g. if you make a secret:

const secret = new Secret(stack, 'Secret', {
    description: app.logicalPrefixedName('app'),
    generateSecretString: {
      secretStringTemplate: JSON.stringify({      }),
      excludeCharacters: '"@/\\',  // without this it may have a " which doesn't get escaped
      generateStringKey: 'RANDOM', 
    },
  });

Then you have to put that secret value into SSM (need to use SSM as a workaround for #21503) like

SECRET: secrets.secret.secretValueFromJson('RANDOM').toString()

then the resulting S3 object will have {"SECRET": "a21"vxC@29%9!"} which is not valid JSON

Expected Behavior

Escaped JSON

Current Behavior

Resolved token in SSM is not escaped when used as a JSON value in Source.jsonData

Reproduction Steps

See above

Possible Solution

No response

Additional Information/Context

No response

CDK CLI Version

2.39.1

Framework Version

No response

Node.js Version

16

OS

macos

Language

Typescript

Language Version

No response

Other information

No response

@revmischa revmischa added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Oct 26, 2022
@peterwoodworth peterwoodworth added p1 effort/small Small work item – less than a day of effort and removed needs-triage This issue or PR still needs to be triaged. labels Dec 7, 2022
@peterwoodworth
Copy link
Contributor

Thanks for reporting this @revmischa, we should be sure account for these cases. I think we would need to do so in the private CloudFormationLang.toJSON(), which is what's used under the hood in Source.jsonData()

return Source.data(objectKey, Stack.of(scope).toJsonString(obj)).bind(scope, context);

@scorbiere scorbiere self-assigned this Mar 4, 2025
@scorbiere
Copy link
Contributor

Hi, I have successfully reproduce he issue by updating the integration test packages/@aws-cdk-testing/framework-integ/test/aws-s3-deployment/test/integ.bucket-deployment-data.ts. I will work on a possible fix, focusing on the file packages/aws-cdk-lib/core/lib/private/cloudformation-lang.ts for now.

Here is the updated test:

import { Bucket } from 'aws-cdk-lib/aws-s3';
import { App, CfnOutput, Stack, Token } from 'aws-cdk-lib';
import { ExpectedResult, IntegTest } from '@aws-cdk/integ-tests-alpha';
import { BucketDeployment, Source } from 'aws-cdk-lib/aws-s3-deployment';
import * as secretsmanager from 'aws-cdk-lib/aws-secretsmanager';
import * as ssm from 'aws-cdk-lib/aws-ssm';
import * as path from 'path';

const app = new App();
const stack = new Stack(app, 'TestBucketDeploymentContent');
const bucket = new Bucket(stack, 'Bucket');

const file1 = Source.data('file1.txt', 'boom');
const file2 = Source.data('path/to/file2.txt', `bam! ${bucket.bucketName}`);
const file3 = Source.jsonData('my-json/config.json', { website_url: bucket.bucketWebsiteUrl });
const file4 = Source.yamlData('my-yaml/config.yaml', { website_url: bucket.bucketWebsiteUrl });
const file5 = Source.jsonData('my-json/config2.json', { bucket_domain_name: bucket.bucketWebsiteDomainName });

// Add new test case for secret value with quotes
const secret = new secretsmanager.Secret(stack, 'TestSecret', {
  generateSecretString: {
    secretStringTemplate: JSON.stringify({
      value: 'test"with"quotes',
    }),
    generateStringKey: 'password',
  },
});

// Store secret in SSM (workaround for #21503)
const param = new ssm.StringParameter(stack, 'SecretParam', {
  stringValue: secret.secretValueFromJson('value').unsafeUnwrap(),
});

const tokenizedValue = param.stringValue; // This should be a Token
new CfnOutput(stack, 'IsToken', { value: Token.isUnresolved(tokenizedValue).toString() });

// Add new file with secret value that needs proper escaping
const file6 = Source.jsonData('my-json/secret-config.json', {
  secret_value: tokenizedValue, // Using the tokenized value explicitly
});

const deployment = new BucketDeployment(stack, 'DeployMeHere', {
  destinationBucket: bucket,
  sources: [file1, file2],
  destinationKeyPrefix: 'deploy/here/',
  retainOnDelete: false, // default is true, which will block the integration test cleanup
});
deployment.addSource(file3);
deployment.addSource(file4);
deployment.addSource(file5);
deployment.addSource(file6);

new CfnOutput(stack, 'BucketName', { value: bucket.bucketName });

const integ = new IntegTest(app, 'integ-test-bucket-deployment-data', {
  testCases: [stack],
});

// Add assertions to verify the JSON file
const assertionProvider = integ.assertions.awsApiCall('S3', 'getObject', {
  Bucket: bucket.bucketName,
  Key: path.join('deploy/here', 'my-json/secret-config.json'),
});

// Verify the content is valid JSON and properly escaped
assertionProvider.expect(ExpectedResult.objectLike({
  Body: JSON.stringify({
    secret_value: 'test"with"quotes', // Properly escaped JSON.
  }),
}));

app.synth();

And the actual failure:

  ASSERT     aws-s3-deployment/test/integ.bucket-deployment-data-integ-test-bucket-deployment-data/DefaultTest (undefined/us-east-1) 510.311s
      AssertionResultsAwsApiCallS3getObject780d452f7611fd1c0e0b1c423e1bda6d
{
  "AcceptRanges": "bytes",
!! Expected {"secret_value":"test\"with\"quotes"} but received {"secret_value":"test"with"quotes"}
  "Body": "{\"secret_value\":\"test\"with\"quotes\"}",
  "ContentLength": 35,
  "ContentType": "application/json",
  "ETag": "\"e08754d5d686baf71219d00b7f9424a9\"",
  "LastModified": {},
  "Metadata": {},
  "ServerSideEncryption": "AES256"
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-s3-deployment bug This issue is a bug. effort/small Small work item – less than a day of effort p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants