Questions on @sensitive

[david-perez]

Suppose we have:

@sensitive
structure Struct {
    member: List
}

list List {
    member: Password
}

@sensitive
string Password

I have a few questions on the @sensitive trait; let me know if you'd like me to create separate issues for any of them.

1. Transitivity of @sensitive

Is @sensitive transitive? Is List to be considered sensitive in the same respects as Struct and Password (i.e. redacted from logs)? Or only when it is being hosted in the programming language type represented by Struct? Note that:

• If List were @sensitive, it would be logged as "*** Sensitive Data Redacted ***".
• If List were not @sensitive and had two members, it would be logged as ["*** Sensitive Data Redacted ***", "*** Sensitive Data Redacted ***"].

2. Backwards compatibility of @sensitive

Currently, in smithy-rs we implement the Debug trait on structs so that if the structure shape is @sensitive, we redact it; likewise, if any of the structure members targets a shape that is @sensitive, we redact them too.

let st = Struct { member: vec![String::from("password")] };
dbg!(&st); // Prints `"*** Sensitive Data Redacted ***"`.

Is this behavior compliant with the spec? I assume, from the lack of a mention in the Evolving models page, that it is backwards-compatible to add or remove @sensitive in the model. However, in smithy-rs it might not be, because the actual implementation of Debug would change.

3. On newtyping @sensitive shapes

In smithy-rs, Password is currently rendered as a regular std::string::String type from the standard library. So when a user writes:

let password = String::from("secret");
dbg!(&password);
println!("{}", &password);

They'd be printing out the contents. In this particular example it might be OK to go ahead and log the string, since the user's intention is clear. However, consider that:

• Password may be very nested in a (non-@sensitive) shape hierarchy. When the user logs the root shape, they might be inadvertently logging Password.
• The @sensitive trait may be added to a shape at any time. Things that are currently being logged might become sensitive in the future and the user's application would then be inadvertently logging them!

I doubt this problem is solvable in all programming languages, but in Rust newtypes are a widespread pattern that may offer a solution. Instead of rendering the Password shape as a standard library String type, we'd render it as a code-generated Password unit struct type we have control over. We'd be able to implement Debug and Display on said type to redact its contents if the shape is @sensitive. We're considering implementing such a thing over in smithy-lang/smithy-rs#1833 for server SDKs. As experts in the spec, do you have any thoughts about whether we should or shouldn't do this?

[mtdowling]

Is @sensitive transitive

No. It only applies to the direct shape and it's members.

Is this behavior compliant with the spec? I assume, from the lack of a mention in the Evolving models page, that it is backwards-compatible to add or remove @sensitive in the model. However, in smithy-rs it might not be, because the actual implementation of Debug would change.

It is backward compatible to add/remove the sensitive trait, yes. Changing the output of Debug shouldn't be an issue and is actually desirable IMO if something truly is sensitive and a team adds the sensitive trait. Changing the type signature based on sensitive traits would be something to avoid though since it would break previously generated code, and we don't want to discourage service teams from retroactively fixing their models by applying missing sensitive traits.

re: Evolving models page: This page doesn't contain every possible backward or forward compatible change, and I don't think it ever could. Maybe we need to make that more clear. The best place to look for trait compatibility is the spec for the trait and the prelude model. Trait defintions describe their backward compatibility constraints, so you can find them in the prelude. We need to better publish the prelude somehow too.

3. On newtyping @sensitive shapes

The problems I can think of here are:

1. You'd be changing the generated type from, say a normal String, to a different type. If a service omitted a sensitive trait and later needs to add the trait, that would be a breaking change. We don't want to discourage adding the sensitive trait.
2. If you're generating code based on the type name, then that's a risk. It's actually documented as something you don't need to worry about changing in a few places in Smithy specs and guides. smithy-diff won't care if you change a member target from one compatible string to another (you could, say, change a member from targeting a sensitive marked "Password" to a senstive marked "SecretKey", and it would be considered backward compatible).

[david-perez]

Thanks for answering my questions.

The best place to look for trait compatibility is the spec for the trait and the prelude model.

Ah, I didn't know that. It'd be neat if we parsed the backwards compatibility info and surfaced it in the docs.

re: the problems on newtyping you call out, it's clear we can't do this on generated clients. Our doubt is whether to do it in server SDKs, which are authoritative model consumers, and where it might even be a good thing to break the build, since allowing the application author to inadvertently log these may be seen as a security risk. We already newtype constrained shapes (with a codegen option to disable such behavior), so newtyping on @sensitive would follow a similar approach.

[kstich]

Closing as I believe the main question is answered and the note for checking compatibility has been added to the spec.