Unverified Commit c39792b3 authored by Landon James's avatar Landon James Committed by GitHub
Browse files

Fix maps/lists with sensitive members not redacted (#3765) 

## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here -->
Lists and maps with sensitive members were not being properly redacted.
Closes https://github.com/smithy-lang/smithy-rs/issues/3757

## Description
<!--- Describe your changes in detail -->
Updated `Shape.shouldRedact` method to properly check for list and map
shapes with sensitive members. Also updated the test definition so it
would actually run since previously the test code was generated in a
nested function inside a no-op function and never run.

## Testing
<!--- Please describe in detail how you tested your changes -->
<!--- Include details of your testing environment, and the tests you ran
to -->
<!--- see how your change affects other areas of the code, etc. -->
Added test cases for lists with sensitive members, maps with sensitive
keys, and maps with sensitive values.

## Checklist
<!--- If a checkbox below is not applicable, then please DELETE it
rather than leaving it unchecked -->
- [x] I have updated `CHANGELOG.next.toml` if I made changes to the
smithy-rs codegen or runtime crates


----

_By submitting this pull request, I confirm that you can use, modify,
copy, and redistribute this contribution, under the terms of your
choice._
parent c63e7924

CHANGELOG.next.toml

+37 −1
Original line number Diff line number Diff line
@@ -10,3 +10,39 @@ 
# references = ["smithy-rs#920"]

# meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client | server | all"}

# author = "rcoh"



[[smithy-rs]]

message = "Support `stringArray` type in endpoints params"

references = ["smithy-rs#3742"]

meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client"}

author = "landonxjames"



[[smithy-rs]]

message = "Add support for `operationContextParams` Endpoints trait"

references = ["smithy-rs#3755"]

meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client"}

author = "landonxjames"



[[aws-sdk-rust]]

message = "`aws_smithy_runtime_api::client::orchestrator::HttpRequest` and `aws_smithy_runtime_api::client::orchestrator::HttpResponse` are now re-exported in AWS SDK clients so that using these types does not require directly depending on `aws-smithy-runtime-api`."

references = ["smithy-rs#3591"]

meta = { "breaking" = false, "tada" = false, "bug" = false }

author = "ysaito1001"



[[smithy-rs]]

message = "`aws_smithy_runtime_api::client::orchestrator::HttpRequest` and `aws_smithy_runtime_api::client::orchestrator::HttpResponse` are now re-exported in generated clients so that using these types does not require directly depending on `aws-smithy-runtime-api`."

references = ["smithy-rs#3591"]

meta = { "breaking" = false, "tada" = false, "bug" = false, "target" = "client" }

author = "ysaito1001"



[[aws-sdk-rust]]

message = "Fix incorrect redaction of `@sensitive` types in maps and lists."

references = ["smithy-rs#3765",  "smithy-rs#3757"]

meta = { "breaking" = false, "tada" = false, "bug" = true }

author = "landonxjames"



[[smithy-rs]]

message = "Fix incorrect redaction of `@sensitive` types in maps and lists."

references = ["smithy-rs#3765",  "smithy-rs#3757"]

meta = { "breaking" = false, "tada" = false, "bug" = true, "target" = "client" }

author = "landonxjames"

codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGenerator.kt

+12 −1
Original line number Diff line number Diff line
@@ -45,10 +45,12 @@ import software.amazon.smithy.rust.codegen.core.smithy.isOptional 
import software.amazon.smithy.rust.codegen.core.smithy.makeOptional

import software.amazon.smithy.rust.codegen.core.smithy.rustType

import software.amazon.smithy.rust.codegen.core.smithy.traits.SyntheticInputTrait

import software.amazon.smithy.rust.codegen.core.util.REDACTION

import software.amazon.smithy.rust.codegen.core.util.dq

import software.amazon.smithy.rust.codegen.core.util.hasTrait

import software.amazon.smithy.rust.codegen.core.util.letIf

import software.amazon.smithy.rust.codegen.core.util.redactIfNecessary

import software.amazon.smithy.rust.codegen.core.util.shouldRedact

import software.amazon.smithy.rust.codegen.core.util.toSnakeCase



// TODO(https://github.com/smithy-lang/smithy-rs/issues/1401) This builder generator is only used by the client.
@@ -353,7 +355,16 @@ class BuilderGenerator( 
                rust("""let mut formatter = f.debug_struct(${builderName.dq()});""")

                members.forEach { member ->

                    val memberName = symbolProvider.toMemberName(member)

                    val fieldValue = member.redactIfNecessary(model, "self.$memberName")

                    // If the struct is marked sensitive all fields get redacted, otherwise each field is determined on its own

                    val fieldValue =

                        if (shape.shouldRedact(model)) {

                            REDACTION

                        } else {

                            member.redactIfNecessary(

                                model,

                                "self.$memberName",

                            )

                        }



                    rust(

                        "formatter.field(${memberName.dq()}, &$fieldValue);",

codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/StructureGenerator.kt

+13 −1
Original line number Diff line number Diff line
@@ -33,9 +33,11 @@ import software.amazon.smithy.rust.codegen.core.smithy.customize.writeCustomizat 
import software.amazon.smithy.rust.codegen.core.smithy.expectRustMetadata

import software.amazon.smithy.rust.codegen.core.smithy.renamedFrom

import software.amazon.smithy.rust.codegen.core.smithy.rustType

import software.amazon.smithy.rust.codegen.core.util.REDACTION

import software.amazon.smithy.rust.codegen.core.util.dq

import software.amazon.smithy.rust.codegen.core.util.getTrait

import software.amazon.smithy.rust.codegen.core.util.redactIfNecessary

import software.amazon.smithy.rust.codegen.core.util.shouldRedact



/** StructureGenerator customization sections */

sealed class StructureSection(name: String) : Section(name) {
@@ -102,9 +104,19 @@ open class StructureGenerator( 
        ) {

            writer.rustBlock("fn fmt(&self, f: &mut #1T::Formatter<'_>) -> #1T::Result", RuntimeType.stdFmt) {

                rust("""let mut formatter = f.debug_struct(${name.dq()});""")



                members.forEach { member ->

                    val memberName = symbolProvider.toMemberName(member)

                    val fieldValue = member.redactIfNecessary(model, "self.$memberName")

                    // If the struct is marked sensitive all fields get redacted, otherwise each field is determined on its own

                    val fieldValue =

                        if (shape.shouldRedact(model)) {

                            REDACTION

                        } else {

                            member.redactIfNecessary(

                                model,

                                "self.$memberName",

                            )

                        }



                    rust(

                        "formatter.field(${memberName.dq()}, &$fieldValue);",

codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/util/Smithy.kt

+8 −7
Original line number Diff line number Diff line
@@ -9,6 +9,8 @@ import software.amazon.smithy.aws.traits.ServiceTrait 
import software.amazon.smithy.codegen.core.CodegenException

import software.amazon.smithy.model.Model

import software.amazon.smithy.model.shapes.BooleanShape

import software.amazon.smithy.model.shapes.ListShape

import software.amazon.smithy.model.shapes.MapShape

import software.amazon.smithy.model.shapes.MemberShape

import software.amazon.smithy.model.shapes.NumberShape

import software.amazon.smithy.model.shapes.OperationShape
@@ -84,13 +86,12 @@ fun ServiceShape.hasEventStreamOperations(model: Model): Boolean = 
    }



fun Shape.shouldRedact(model: Model): Boolean =

    when (this) {

        is MemberShape ->

            model.expectShape(target).shouldRedact(model) ||

                model.expectShape(container)

                    .shouldRedact(model)



        else -> this.hasTrait<SensitiveTrait>()

    when {

        hasTrait<SensitiveTrait>() -> true

        this is MemberShape -> model.expectShape(target).shouldRedact(model)

        this is ListShape -> member.shouldRedact(model)

        this is MapShape -> key.shouldRedact(model) || value.shouldRedact(model)

        else -> false

    }



const val REDACTION = "\"*** Sensitive Data Redacted ***\""

codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGeneratorTest.kt

+17 −13
Original line number Diff line number Diff line
@@ -124,7 +124,8 @@ internal class BuilderGeneratorTest { 
                        .username("admin")

                        .password("pswd")

                        .secret_key("12345");

                         assert_eq!(format!("{:?}", builder), "Builder { username: Some(\"admin\"), password: \"*** Sensitive Data Redacted ***\", secret_key: \"*** Sensitive Data Redacted ***\" }");

                         assert_eq!(format!("{:?}", builder),

                         "Builder { username: Some(\"admin\"), password: \"*** Sensitive Data Redacted ***\", secret_key: \"*** Sensitive Data Redacted ***\", secret_value_map: \"*** Sensitive Data Redacted ***\", secret_key_map: \"*** Sensitive Data Redacted ***\", secret_list: \"*** Sensitive Data Redacted ***\" }");

                    """,

                )

            }
@@ -278,7 +279,9 @@ internal class BuilderGeneratorTest { 
            implBlock(provider.toSymbol(inner)) {

                BuilderGenerator.renderConvenienceMethod(this, provider, inner)

            }

            unitTest("test", additionalAttributes = listOf(Attribute.DenyDeprecated), block = {

            unitTest(

                "test", additionalAttributes = listOf(Attribute.DenyDeprecated),

                block = {

                    rust(

                        // Notice that the builder is instantiated directly, and not through the Inner::builder() method.

                        // This is because Inner is marked with `deprecated`, so any usage of `Inner` inside the test will
@@ -289,7 +292,8 @@ internal class BuilderGeneratorTest { 
                        let _ = test_inner::Builder::default();

                        """,

                    )

            })

                },

            )

        }

        project.withModule(provider.moduleForBuilder(inner)) {

            BuilderGenerator(model, provider, inner, emptyList()).render(this)