Add .scl environment restrictions https://github.com/bazelbuild/bazel/commit/a0cd355347b57b17f28695a84af168f9fd200ba1 added the Starlark Configuration Language (SCL), a restricted subset of .bzl that is basically pure Starlark + Bazel-style load(), visibility(), and struct(). But that CL only made .scl files loadable. This CL completes the job by actually restricting .scl files to only access those symbols. The .scl predeclared environment is determined in StarlarkGlobals, like other Starlark evaluation contexts. BzlLoadFunction and BzlCompileFunction branch on whether the key is for .scl, and if so use that environment instead of their normal logic. Added tests that .scl can use the mentioned symbols and cannot use anything else. PiperOrigin-RevId: 534144656 Change-Id: I6cca8db234beaf74529a2a7efbdb40d0420fa9c1
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkGlobalsImpl.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkGlobalsImpl.java index a2d546a..2a5d42d 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkGlobalsImpl.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkGlobalsImpl.java
@@ -102,4 +102,22 @@ return env.buildOrThrow(); } + + @Override + public ImmutableMap<String, Object> getSclToplevels() { + // TODO(bazel-team): We only want the visibility() symbol from BazelBuildApiGlobals, nothing + // else, but Starlark#addMethods doesn't allow that kind of granularity, and the Starlark + // interpreter doesn't provide any other way to turn a Java method definition into a + // callable symbol. So we hack it by building the map of all symbols in that class and + // retrieving just the one we want. The alternative of refactoring the class is more churn than + // its worth, given the starlarkbuildapi/ split. + ImmutableMap.Builder<String, Object> bazelBuildApiGlobalsSymbols = ImmutableMap.builder(); + Starlark.addMethods(bazelBuildApiGlobalsSymbols, new BazelBuildApiGlobals()); + Object visibilitySymbol = bazelBuildApiGlobalsSymbols.buildOrThrow().get("visibility"); + + ImmutableMap.Builder<String, Object> env = ImmutableMap.builder(); + env.put("visibility", visibilitySymbol); + env.put("struct", StructProvider.STRUCT); + return env.buildOrThrow(); + } }
diff --git a/src/main/java/com/google/devtools/build/lib/packages/StarlarkGlobals.java b/src/main/java/com/google/devtools/build/lib/packages/StarlarkGlobals.java index cde8a97f..a27d630 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/StarlarkGlobals.java +++ b/src/main/java/com/google/devtools/build/lib/packages/StarlarkGlobals.java
@@ -61,4 +61,7 @@ /** Returns the fixed top-levels for .bzl files, excluding the {@code native} object. */ ImmutableMap<String, Object> getFixedBzlToplevels(); + + /** Returns the top-levels for .scl files. */ + ImmutableMap<String, Object> getSclToplevels(); }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BzlCompileFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BzlCompileFunction.java index 8291406..82e9119 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BzlCompileFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BzlCompileFunction.java
@@ -144,7 +144,9 @@ } Map<String, Object> predeclared; - if (key.kind == BzlCompileValue.Kind.BUILTINS) { + if (key.isSclDialect()) { + predeclared = bazelStarlarkEnvironment.getStarlarkGlobals().getSclToplevels(); + } else if (key.kind == BzlCompileValue.Kind.BUILTINS) { predeclared = bazelStarlarkEnvironment.getBuiltinsBzlEnv(); } else { // Use the predeclared environment for BUILD-loaded bzl files, ignoring injection. It is not
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BzlCompileValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/BzlCompileValue.java index 02e52f3..b6a1393 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BzlCompileValue.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BzlCompileValue.java
@@ -186,6 +186,13 @@ return kind == Kind.BUILTINS; } + /** Returns true if the requested file follows the .scl dialect. */ + // See comment in BzlLoadValue#isSclDialect about distinguishing .scl keys by label as opposed + // to by Kind. + final boolean isSclDialect() { + return label != null && label.getName().endsWith(".scl"); + } + boolean isBuildPrelude() { return kind == Kind.PRELUDE || kind == Kind.EMPTY_PRELUDE; }
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java index 87d9576..876d56c 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java
@@ -1267,8 +1267,8 @@ } /** - * Obtains the predeclared environment for a .bzl file, based on the type of .bzl and (if - * applicable) the injected builtins. + * Obtains the predeclared environment for a .bzl (or .scl) file, based on the type of .bzl and + * (if applicable) the injected builtins. * * <p>Returns null if there was a missing Skyframe dep or unspecified exception. * @@ -1279,7 +1279,10 @@ private ImmutableMap<String, Object> getAndDigestPredeclaredEnvironment( BzlLoadValue.Key key, StarlarkBuiltinsValue builtins, Fingerprint fp) { BazelStarlarkEnvironment starlarkEnv = ruleClassProvider.getBazelStarlarkEnvironment(); - if (key instanceof BzlLoadValue.KeyForBuild) { + if (key.isSclDialect()) { + // .scl doesn't use injection and doesn't care what kind of key it is. + return starlarkEnv.getStarlarkGlobals().getSclToplevels(); + } else if (key instanceof BzlLoadValue.KeyForBuild) { // TODO(#11437): Remove ability to disable injection by setting flag to empty string. if (builtins .starlarkSemantics
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/BzlLoadFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/BzlLoadFunctionTest.java index cee118c..833508f 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/BzlLoadFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/skyframe/BzlLoadFunctionTest.java
@@ -305,6 +305,43 @@ assertContainsEvent("in .scl files, load labels must begin with \"//\""); } + @Test + public void testSclSupportsStructAndVisibility() throws Exception { + setBuildLanguageOptions("--experimental_enable_scl_dialect=true"); + + scratch.file("pkg/BUILD"); + scratch.file( + "pkg/ext1.scl", // + "visibility('private')", + "a = struct()"); + scratch.file( + "pkg/ext2.scl", // + "load('//pkg:ext1.scl', 'a')"); + scratch.file("pkg2/BUILD"); + scratch.file( + "pkg2/ext3.scl", // + "load('//pkg:ext1.scl', 'a')"); + + checkSuccessfulLookup("//pkg:ext2.scl"); + reporter.removeHandler(failFastHandler); + checkFailingLookup( + "//pkg2:ext3.scl", "module //pkg2:ext3.scl contains .bzl load visibility violations"); + } + + @Test + public void testSclDoesNotSupportOtherBazelSymbols() throws Exception { + setBuildLanguageOptions("--experimental_enable_scl_dialect=true"); + + scratch.file("pkg/BUILD"); + scratch.file( + "pkg/ext.scl", // + "a = depset([])"); + + reporter.removeHandler(failFastHandler); + checkFailingLookup("//pkg:ext.scl", "compilation of module 'pkg/ext.scl' failed"); + assertContainsEvent("name 'depset' is not defined"); + } + private EvaluationResult<BzlLoadValue> get(SkyKey skyKey) throws Exception { EvaluationResult<BzlLoadValue> result = SkyframeExecutorTestUtils.evaluate(