From 88e2d1f17c3e202de109e584904204d0c532f0e5 Mon Sep 17 00:00:00 2001
From: Miles Ziemer <45497130+milesziemer@users.noreply.github.com>
Date: Tue, 20 Sep 2022 19:25:51 -0400
Subject: [PATCH] Enforce private on traits (#1406)

Fixes: #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
---
 .../META-INF/smithy/aws.protocols.smithy      |  2 +-
 .../validators/PrivateAccessValidator.java    |  2 +-
 .../validators/private-access.errors          |  1 +
 .../errorfiles/validators/private-access.json | 20 +++++++++++++++++++
 4 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/smithy-aws-traits/src/main/resources/META-INF/smithy/aws.protocols.smithy b/smithy-aws-traits/src/main/resources/META-INF/smithy/aws.protocols.smithy
index 5099667c77c..577fe722391 100644
--- a/smithy-aws-traits/src/main/resources/META-INF/smithy/aws.protocols.smithy
+++ b/smithy-aws-traits/src/main/resources/META-INF/smithy/aws.protocols.smithy
@@ -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
diff --git a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/PrivateAccessValidator.java b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/PrivateAccessValidator.java
index bc1764e727f..7099f66873a 100644
--- a/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/PrivateAccessValidator.java
+++ b/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/PrivateAccessValidator.java
@@ -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)) {
diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/private-access.errors b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/private-access.errors
index 93760752a55..d34ffe6ed64 100644
--- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/private-access.errors
+++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/private-access.errors
@@ -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
diff --git a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/private-access.json b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/private-access.json
index d845c8dd6aa..cc13dab1c12 100644
--- a/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/private-access.json
+++ b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/private-access.json
@@ -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": {
@@ -64,6 +78,9 @@
                 },
                 "map": {
                     "target": "smithy.example#InvalidMap"
+                },
+                "traitApplication": {
+                    "target": "smithy.example#InvalidTraitApplication"
                 }
             },
             "traits": {
@@ -85,6 +102,9 @@
                 },
                 "map": {
                     "target": "smithy.example#InvalidMap"
+                },
+                "traitApplication": {
+                    "target": "smithy.example#InvalidTraitApplication"
                 }
             },
             "traits": {