[8.6.0] Fix visibility for implicit deps of parent rules (https://github.com/… (#28722)

…/pull/28627)

The visibility of a default value of an implicit dep in an extended rule
should be checked relative to the definition of the rule that introduced
it, which may not be the child rule.

Fixes #28618

Closes #28627.

PiperOrigin-RevId: 871303947
Change-Id: I0027e277dc9f01396fa4674297d2a73e1e9d257e

<!--
Thank you for contributing to Bazel!
Please read the contribution guidelines:
https://bazel.build/contribute.html
-->

### Description
<!--
Please provide a brief summary of the changes in this PR.
-->

### Motivation
<!--
Why is this change important? Does it fix a specific bug or add a new
feature?
If this PR fixes an existing issue, please link it here (e.g. "Fixes
#1234").
-->

### Build API Changes
<!--
Does this PR affect the Build API? (e.g. Starlark API, providers,
command-line flags, native rules)
If yes, please answer the following:
1. Has this been discussed in a design doc or issue? (Please link it)
2. Is the change backward compatible?
3. If it's a breaking change, what is the migration plan?
-->

No

### Checklist

- [ ] I have added tests for the new use cases (if any).
- [ ] I have updated the documentation (if applicable).

### Release Notes

<!--
If this is a new feature, please add 'RELNOTES[NEW]: <description>'
here.
If this is a breaking change, please add 'RELNOTES[INC]: <reason>' here.
If this change should be mentioned in release notes, please add
'RELNOTES: <reason>' here.
-->

RELNOTES: None

Commit
a16489f

---------

Co-authored-by: Fabian Meumertzheim <fabian@meumertzhe.im>

Author: iancha1992

src/main/java/com/google/devtools/build/lib/analysis/CommonPrerequisiteValidator.java

@@ -135,9 +135,13 @@ private void validateDirectPrerequisiteVisibility(
         // Never null since we already checked that the aspect is Starlark-defined.
         implicitDefinition = checkNotNull(aspectClass.getExtensionLabel());
       } else {
-        // Never null since we already checked that the rule is a Starlark rule.
-        implicitDefinition =
-            checkNotNull(rule.getRuleClassObject().getRuleDefinitionEnvironmentLabel());
+        var owningRule = rule.getRuleClassObject();
+        com.google.devtools.build.lib.packages.RuleClass parent;
+        while ((parent = owningRule.getStarlarkParent()) != null
+            && parent.getAttributeByNameMaybe(attrName) != null) {
+          owningRule = parent;
+        }
+        implicitDefinition = checkNotNull(owningRule.getRuleDefinitionEnvironmentLabel());
       }
       // Validate with respect to the defining .bzl.
       if (!isVisibleToLocation(prerequisite, implicitDefinition.getPackageIdentifier())) {

src/test/java/com/google/devtools/build/lib/starlark/StarlarkRuleClassFunctionsTest.java

@@ -5364,6 +5364,64 @@ def _impl(ctx):
         "Error in rule: attribute `_tool`: private attributes cannot be overridden.");
   }
 
+  @Test
+  public void extendRule_parentPrivateAttrDefault_visibilityCheckedAgainstParent()
+      throws Exception {
+    scratch.file(
+        "extend_rule_testing/parent/BUILD",
+        """
+        exports_files(
+            ["parent_tool.txt"],
+            visibility = ["//extend_rule_testing/parent:__pkg__"],
+        )
+        """);
+    scratch.file("extend_rule_testing/parent/parent_tool.txt");
+    scratch.file(
+        "extend_rule_testing/parent/parent.bzl",
+        """
+        def _impl(ctx):
+            return []
+
+        parent_library = rule(
+            implementation = _impl,
+            extendable = True,
+            attrs = {
+                "_tool": attr.label(
+                    allow_single_file = True,
+                    default = "//extend_rule_testing/parent:parent_tool.txt",
+                ),
+            },
+        )
+        """);
+    scratch.file(
+        "extend_rule_testing/child.bzl",
+        """
+        load("//extend_rule_testing/parent:parent.bzl", "parent_library")
+
+        def _impl(ctx):
+            return ctx.super()
+
+        my_library = rule(
+            implementation = _impl,
+            parent = parent_library,
+        )
+        """);
+    scratch.file(
+        "extend_rule_testing/BUILD",
+        """
+        load(":child.bzl", "my_library")
+
+        my_library(name = "my_target")
+        """);
+
+    // This should succeed because the visibility of the parent's private attribute default should
+    // be checked against the parent rule's package, not the child rule's package.
+    // See https://github.com/bazelbuild/bazel/issues/28618
+    getConfiguredTarget("//extend_rule_testing:my_target");
+
+    assertNoEvents();
+  }
+
   @Test
   public void extendRule_attributeOverrideDefault() throws Exception {
     scratch.file("extend_rule_testing/parent/BUILD");