Always check $config_dependencies visibility at use

All dependencies for `select` expressions are collected in an implicit `$config_dependencies` attribute. Even with `--incompatible_visibility_private_attributes_at_definition`, visibility for this attribute is now checked relative to the use rather than the definition of the rule. Fixes bazelbuild#19126 Closes bazelbuild#19139. PiperOrigin-RevId: 553830083 Change-Id: Ic4af0f5ad9a0c627c973cd5ce88dd1e1bf51474e
iancha1992 · Aug 7, 2023 · 2a39d6f · 2a39d6f
1 parent 90e8ffd
commit 2a39d6f
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 0 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/CommonPrerequisiteValidator.java b/src/main/java/com/google/devtools/build/lib/analysis/CommonPrerequisiteValidator.java
@@ -26,6 +26,7 @@
 import com.google.devtools.build.lib.packages.RawAttributeMapper;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.Target;
+import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.Type;
 import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
 import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
@@ -89,6 +90,7 @@ private void validateDirectPrerequisiteVisibility(
 
       if (!toolCheckAtDefinition
           || !attribute.isImplicit()
+          || attribute.getName().equals(RuleClass.CONFIG_SETTING_DEPS_ATTRIBUTE)
           || rule.getRuleClassObject().getRuleDefinitionEnvironmentLabel() == null) {
         // Default check: The attribute must be visible from the target.
         if (!context.isVisible(prerequisite.getConfiguredTarget())) {

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/VisibilityTest.java b/src/test/java/com/google/devtools/build/lib/analysis/VisibilityTest.java
@@ -166,6 +166,42 @@ public void testDataVisibilityPrivateCheckPrivateAtUse() throws Exception {
     assertThrows(ViewCreationFailedException.class, () -> update("//use:world"));
   }
 
+  @Test
+  public void testConfigSettingVisibilityAlwaysCheckedAtUse() throws Exception {
+    scratch.file(
+        "BUILD",
+        "load('//build_defs:defs.bzl', 'my_rule')",
+        "my_rule(",
+        "    name = 'my_target',",
+        "    value = select({",
+        "        '//config_setting:my_setting': 'foo',",
+        "        '//conditions:default': 'bar',",
+        "    }),",
+        ")");
+    scratch.file("build_defs/BUILD");
+    scratch.file(
+        "build_defs/defs.bzl",
+        "def _my_rule_impl(ctx):",
+        "    pass",
+        "my_rule = rule(",
+        "    implementation = _my_rule_impl,",
+        "    attrs = {",
+        "        'value': attr.string(mandatory = True),",
+        "    },",
+        ")");
+    scratch.file(
+        "config_setting/BUILD",
+        "config_setting(",
+        "    name = 'my_setting',",
+        "    values = {'cpu': 'does_not_matter'},",
+        "    visibility = ['//:__pkg__'],",
+        ")");
+    useConfiguration("--incompatible_visibility_private_attributes_at_definition");
+
+    update("//:my_target");
+    assertThat(hasErrors(getConfiguredTarget("//:my_target"))).isFalse();
+  }
+
   void setupFilesScenario(String wantRead) throws Exception {
     scratch.file("src/source.txt", "source");
     scratch.file("src/BUILD", "exports_files(['source.txt'], visibility=['//pkg:__pkg__'])");