From 9d7d7e401878e98b411a1c84e2ff707026ba4c7f Mon Sep 17 00:00:00 2001 From: Landon James Date: Tue, 16 Jul 2024 11:20:27 -0700 Subject: [PATCH 1/3] Fix maps/lists w sensitive members not redacted --- .../smithy/rust/codegen/core/util/Smithy.kt | 15 ++++--- .../generators/StructureGeneratorTest.kt | 43 +++++++++++++++---- 2 files changed, 43 insertions(+), 15 deletions(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/util/Smithy.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/util/Smithy.kt index 975416d72f..53876cf33c 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/util/Smithy.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/util/Smithy.kt @@ -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 @@ -99,13 +101,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() + when { + this is MemberShape -> model.expectShape(target).shouldRedact(model) + hasTrait() -> true + this is ListShape -> member.shouldRedact(model) + this is MapShape -> key.shouldRedact(model) || value.shouldRedact(model) + else -> false } const val REDACTION = "\"*** Sensitive Data Redacted ***\"" diff --git a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/StructureGeneratorTest.kt b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/StructureGeneratorTest.kt index c197a7cb4c..d8a4fed3ec 100644 --- a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/StructureGeneratorTest.kt +++ b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/StructureGeneratorTest.kt @@ -62,8 +62,10 @@ class StructureGeneratorTest { structure Credentials { username: String, password: Password, - - secretKey: SecretKey + secretKey: SecretKey, + secretValueMap: MapThatContainsSecretValues, + secretKeyMap: MapThatContainsSecretKeys, + secretList: ListThatContainsSecrets } structure StructWithDoc { @@ -79,6 +81,20 @@ class StructureGeneratorTest { public: String, private: SecretStructure, } + + map MapThatContainsSecretKeys { + key: SecretKey + value: String + } + + map MapThatContainsSecretValues { + key: String + value: SecretKey + } + + list ListThatContainsSecrets { + member: Password + } """.asSmithyModel() val struct = model.lookup("com.test#MyStruct") val structWithDoc = model.lookup("com.test#StructWithDoc") @@ -159,15 +175,27 @@ class StructureGeneratorTest { TestWorkspace.testProject().unitTest { structureGenerator(model, provider, this, credentials).render() - this.unitTest( - "sensitive_fields_redacted", + rust( """ + use std::collections::HashMap; + + let mut secret_map = HashMap::new(); + secret_map.insert("FirstSecret".to_string(), "don't leak me".to_string()); + secret_map.insert("SecondSecret".to_string(), "don't leak me".to_string()); + + let secret_list = vec!["don't leak me".to_string()]; + let creds = Credentials { username: Some("not_redacted".to_owned()), password: Some("don't leak me".to_owned()), - secret_key: Some("don't leak me".to_owned()) + secret_key: Some("don't leak me".to_owned()), + secret_key_map: Some(secret_map.clone()), + secret_value_map: Some(secret_map), + secret_list: Some(secret_list), }; - assert_eq!(format!("{:?}", creds), "Credentials { username: Some(\"not_redacted\"), password: \"*** Sensitive Data Redacted ***\", secret_key: \"*** Sensitive Data Redacted ***\" }"); + + assert_eq!(format!("{:?}", creds), + "Credentials { username: Some(\"not_redacted\"), 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 ***\" }"); """, ) }.compileAndTest() @@ -179,8 +207,7 @@ class StructureGeneratorTest { TestWorkspace.testProject().unitTest { structureGenerator(model, provider, this, secretStructure).render() - this.unitTest( - "sensitive_structure_redacted", + rust( """ let secret_structure = SecretStructure { secret_field: Some("secret".to_owned()), From 4b398c752e573dd4fa55abb9ad6fa802b1d1263a Mon Sep 17 00:00:00 2001 From: Landon James Date: Tue, 16 Jul 2024 14:50:55 -0700 Subject: [PATCH 2/3] Fix bug with @sensitive structs --- .../smithy/generators/BuilderGenerator.kt | 13 +++++++- .../smithy/generators/StructureGenerator.kt | 14 ++++++++- .../smithy/rust/codegen/core/util/Smithy.kt | 2 +- .../smithy/generators/BuilderGeneratorTest.kt | 30 +++++++++++-------- 4 files changed, 43 insertions(+), 16 deletions(-) diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGenerator.kt index bf99e73790..44f5e83949 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGenerator.kt @@ -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);", diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/StructureGenerator.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/StructureGenerator.kt index 61ea75bb35..17452da38d 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/StructureGenerator.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/StructureGenerator.kt @@ -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);", diff --git a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/util/Smithy.kt b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/util/Smithy.kt index 53876cf33c..87f9537f3f 100644 --- a/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/util/Smithy.kt +++ b/codegen-core/src/main/kotlin/software/amazon/smithy/rust/codegen/core/util/Smithy.kt @@ -102,8 +102,8 @@ fun ServiceShape.hasEventStreamOperations(model: Model): Boolean = fun Shape.shouldRedact(model: Model): Boolean = when { - this is MemberShape -> model.expectShape(target).shouldRedact(model) hasTrait() -> 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 diff --git a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGeneratorTest.kt b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGeneratorTest.kt index fc5fe91b4e..64b07f7e14 100644 --- a/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGeneratorTest.kt +++ b/codegen-core/src/test/kotlin/software/amazon/smithy/rust/codegen/core/smithy/generators/BuilderGeneratorTest.kt @@ -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,18 +279,21 @@ internal class BuilderGeneratorTest { implBlock(provider.toSymbol(inner)) { BuilderGenerator.renderConvenienceMethod(this, provider, inner) } - 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 - // fail the compilation. - // - // This piece of code would fail though if the Builder inherits the attributes from Inner. - """ - let _ = test_inner::Builder::default(); - """, - ) - }) + 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 + // fail the compilation. + // + // This piece of code would fail though if the Builder inherits the attributes from Inner. + """ + let _ = test_inner::Builder::default(); + """, + ) + }, + ) } project.withModule(provider.moduleForBuilder(inner)) { BuilderGenerator(model, provider, inner, emptyList()).render(this) From 3c37ca14595a612371c74a6696ff225cf9df7027 Mon Sep 17 00:00:00 2001 From: Landon James Date: Wed, 17 Jul 2024 09:07:55 -0700 Subject: [PATCH 3/3] Update CHANGELOG --- CHANGELOG.next.toml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/CHANGELOG.next.toml b/CHANGELOG.next.toml index a168393287..68f8864d92 100644 --- a/CHANGELOG.next.toml +++ b/CHANGELOG.next.toml @@ -34,3 +34,15 @@ message = "`aws_smithy_runtime_api::client::orchestrator::HttpRequest` and `aws_ 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"