@simonrw Thank you for your review and feedback.

I have addressed all code comments. As for handling the caveats related to NoEcho parameters, I want to ensure that my implementation aligns with AWS best practices. Specifically, here is what I have done:

1. Outputs Section:

   • I have a test that ensures parameters referenced in the Outputs section are printed as-is in API responses.
   • This aligns with AWS documentation, which states that values in the Outputs section should not be masked.

2. Metadata Section:

   • Currently, my implementation does not mask any values in the Metadata section.
   • I plan to add tests to verify this behavior and ensure it remains consistent.

3. Resource Metadata Attribute:

   • Similarly, values in the Metadata attribute of resource definitions are not masked.
   • I plan to add tests to cover this aspect as well.

4. Primary Identifiers:

   • I understood that NoEcho parameters should not used in properties that form primary identifiers for resources.
   • If such a scenario is detected, I plan to print a warning message during stack creation or update operations.

5. Warnings for Sensitive Data:

   • While NoEcho does not mask data in the Metadata and Outputs sections and Resource Metadata Attribute, I will consider adding warnings when users create or update stacks with NoEcho parameters in these sections by modifying resolve_dependencies function.
   • This aligns with AWS best practices to avoid embedding sensitive information directly in templates.

Would you like me to add specific tests for the Metadata section and Resource Metadata attribute? Additionally, should I proceed with printing warnings for NoEcho parameters used in places mentioned above?

Thanks for the detailed response @rominf.

1. Outputs Section:

• I have a test that ensures parameters referenced in the Outputs section are printed as-is in API responses.
• This aligns with AWS documentation, which states that values in the Outputs section should not be masked.

👍

2. Metadata Section:

• Currently, my implementation does not mask any values in the Metadata section.
• I plan to add tests to verify this behavior and ensure it remains consistent.

3. Resource Metadata Attribute:

• Similarly, values in the Metadata attribute of resource definitions are not masked.
• I plan to add tests to cover this aspect as well.

That's great, thanks - updating the single test you have would be enough to cover both of these cases. The test already snapshots the Outputs section, so it's just the two Metadata sections that are not covered.

4. Primary Identifiers:

• I understood that NoEcho parameters should not used in properties that form primary identifiers for resources.
• If such a scenario is detected, I plan to print a warning message during stack creation or update operations.

5. Warnings for Sensitive Data:

• While NoEcho does not mask data in the Metadata and Outputs sections and Resource Metadata Attribute, I will consider adding warnings when users create or update stacks with NoEcho parameters in these sections by modifying resolve_dependencies function.
• This aligns with AWS best practices to avoid embedding sensitive information directly in templates.

I think this is a nice feature to have for our users. I am not sure if resolve_dependencies is the right place. I think we have two options:

1. perform a looser check that prints a warning if any of the Parameters have NoEcho set, with a message like: "NoEcho parameters are being used. This may expose sensitive information in the stack outputs and metadata. Please ensure that you are not embedding sensitive information directly in the template.", or
2. perform a stricter check that warns in these specific situations i.e. when a property is used in a position where it may be exposed. In which case, we could use either resolve_outputs, or resolve_ref (if we know where the resolution is happening). We don't have any helpers for resolution in the Metadata section, so we would likely skip this for now.

Which would you suggest?

@rominf I can see one of our integration tests failing with your latest commit. Could you fix this please?

@simonrw You are right; resolve_dependencies is not the correct place. I would perform stricter checks, as looser checks might give too many false positives. Since AWS documentation explicitly mentions that NoEcho should be avoided, I do not see much value in implementing a looser check. However, I would prefer to ship full strict checking, and since helpers are not fully ready, I would prioritize other tasks.

As for the template Metadata, it turns out that CloudFormation does not expose the fully processed or final template, so I implemented a check for resource Metadata only.

I can see one of our integration tests failing with your latest commit. Could you fix this please?

Thanks. I fixed it.