Enforcing @private on trait usage

[iancaffey]

Should referencing a @private trait be permitted outside the declared namespace?

I couldn't really tell from https://awslabs.github.io/smithy/1.0/spec/core/constraint-traits.html#private-trait.

And it looks like the private access validator isn't including trait relationships (and so allows you to use it wherever).

Example

$version: "2"

namespace a

@trait
@private
structure privateTrait {}

$version: "2"

namespace b

use a#privateTrait

@privateTrait
structure MyStructure {}

and no complaints during build.

---

Main reason I'm asking is because HttpConfiguration

/// Contains HTTP protocol configuration for HTTP-based protocols.
@private
@mixin
structure HttpConfiguration { ... }

doesn't add private to localTraits, so all of the mixed protocols now end up being @private too.

Didn't know if this was intentional, so wanted to get a better understanding of what @private is supposed to do.

No real issue here, just curious. :)

[mtdowling]

Yeah, private is supposed to not allow referencing private traits outside of the defined namespace. It's a bug if that's not the behavior now.

[milesziemer]

Upon looking into this further, there are other cases that exhibit this behavior as well. This example includes everything that seems to not enforce @private:

$version: "2"

namespace example.private

@private
@trait
structure privateTrait {}

@private
resource PrivateResource {
    identifiers: {
        id: String
    }
}

@mixin
structure MixinWithPrivateMember {
    @private
    member: String
}

$version: "2"

namespace example.public

use example.private#privateTrait
use example.private#PrivateResource
use example.private#MixinWithPrivateMember

// 1. Using a private trait
@privateTrait
structure PrivateTraitUsage {}

// 2. Target elision with private resource
structure PrivateResourceUsage for PrivateResource {
    $id
}

// 3. Target elision of private member of mixin
structure PrivateMemberOfMixinUsage with [MixinWithPrivateMember] {
    @required
    $member
}

Based on the documentation, I'm not sure if 3. is expected behavior, because while the mixin itself isn't private, the member being referenced is. Both 1. and 2. are clearly references to private shapes in another namespace though. For 1., we could use getReverseProviderWithTraitRelationships instead of just getReverseProvider like we do now in PrivateAccessValidator. This change breaks a whole bunch of protocol tests because like mentioned in the issue:

namespace aws.protocols
...
/// Contains HTTP protocol configuration for HTTP-based protocols.
@private
@mixin
structure HttpConfiguration { ... }

HttpConfiguration does not use localTraits. For 2. however, the relationship with PrivateResource isn't modeled so there doesn't seem to be a way to enforce @private. This wasn't caught in any tests because the validation tests use JSON AST representation, not Smithy IDL.

Unless I'm missing something, we would probably have to model these target elision relationships in order to fix 2. and possibly 3. If I'm not missing something, we might want to open a separate issue for that because it would be a more general change. In the meantime I think a solution to this specific problem would be to just check any applied traits to the shape and see if they're private.