Skip to content

Commit

Permalink
Enforce private on traits (smithy-lang#1406)
Browse files Browse the repository at this point in the history
Fixes: smithy-lang#1369

PrivateAccessValidator was not checking trait relationships which allowed private traits to be referenced in any namespace. The HttpConfiguration mixin in aws.protocols was not including private in its localTraits, which caused the private trait to be applied to any shape that mixed in HttpConfiguration. This issue wasn't apparent because trait relationships were not being validated for private access. The private access validation test was updated to check for this.

This could be a breaking change for any Smithy models that are using private traits in other namespaces, either directly or through a mixin that doesn't have private included in its localTraits.

Commits:
* Fix bug where `private` would be applied to every shape that mixes in `HttpConfiguration`

* Fix bug where private access wasn't enforced on trait applications
  • Loading branch information
milesziemer authored Sep 20, 2022
1 parent 0827159 commit 88e2d1f
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ structure awsJson1_1 with [HttpConfiguration] {}

/// Contains HTTP protocol configuration for HTTP-based protocols.
@private
@mixin
@mixin(localTraits: [private])
structure HttpConfiguration {
/// The priority ordered list of supported HTTP protocol versions.
http: StringList
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public final class PrivateAccessValidator extends AbstractValidator {

@Override
public List<ValidationEvent> validate(Model model) {
NeighborProvider provider = NeighborProviderIndex.of(model).getReverseProvider();
NeighborProvider provider = NeighborProviderIndex.of(model).getReverseProviderWithTraitRelationships();

List<ValidationEvent> events = new ArrayList<>();
for (Shape privateShape : model.getShapesWithTrait(PrivateTrait.class)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@
[ERROR] smithy.example#InvalidOperation: This shape has an invalid output relationship that targets a private shape, `smithy.private#InvalidOperationOutput`, in another namespace. | PrivateAccess
[ERROR] smithy.example#InvalidService: This shape has an invalid operation relationship that targets a private shape, `smithy.private#PrivateOperation`, in another namespace. | PrivateAccess
[ERROR] smithy.example#InvalidStructure$invalid: This shape has an invalid member_target relationship that targets a private shape, `smithy.private#PrivateString`, in another namespace. | PrivateAccess
[ERROR] smithy.example#InvalidTraitApplication: This shape has an invalid trait relationship that targets a private shape, `smithy.private#privateTrait`, in another namespace. | PrivateAccess
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,20 @@
}
]
},
"smithy.example#InvalidTraitApplication": {
"type": "structure",
"members": {},
"traits": {
"smithy.private#privateTrait": {}
}
},
"smithy.private#privateTrait": {
"type": "structure",
"traits": {
"smithy.api#trait": {},
"smithy.api#private": {}
}
},
"smithy.private#InvalidOperationInput": {
"type": "structure",
"members": {
Expand All @@ -64,6 +78,9 @@
},
"map": {
"target": "smithy.example#InvalidMap"
},
"traitApplication": {
"target": "smithy.example#InvalidTraitApplication"
}
},
"traits": {
Expand All @@ -85,6 +102,9 @@
},
"map": {
"target": "smithy.example#InvalidMap"
},
"traitApplication": {
"target": "smithy.example#InvalidTraitApplication"
}
},
"traits": {
Expand Down

0 comments on commit 88e2d1f

Please sign in to comment.