Prevent Skylark actions from setting arbitrary execution info

PiperOrigin-RevId: 194236287

2 files changed, 28 insertions, 8 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java
index 4f09dde901..0985afb108 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java
@@ -805,7 +805,7 @@ public class SkylarkActionFactory implements SkylarkValue {
     }
     if (executionRequirementsUnchecked != Runtime.NONE) {
       builder.setExecutionInfo(
-          ImmutableMap.copyOf(
+          TargetUtils.filter(
               SkylarkDict.castSkylarkDictOrNoneToDict(
                   executionRequirementsUnchecked,
                   String.class,
diff --git a/src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java b/src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java
index 756df750a0..133cb71e9c 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/TargetUtils.java
@@ -20,6 +20,7 @@ import static com.google.devtools.build.lib.syntax.Type.BOOLEAN;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Predicate;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.events.Location;
 import com.google.devtools.build.lib.syntax.Type;
@@ -40,6 +41,23 @@ public final class TargetUtils {
   // *_test / test_suite attribute that used to specify constraint keywords.
   private static final String CONSTRAINTS_ATTR = "tags";
 
+  // We don't want to pollute the execution info with random things, and we also need to reserve
+  // some internal tags that we don't allow to be set on targets. We also don't want to
+  // exhaustively enumerate all the legal values here. Right now, only a ~small set of tags is
+  // recognized by Bazel.
+  private static final Predicate<String> LEGAL_EXEC_INFO_KEYS = new Predicate<String>() {
+    @Override
+    public boolean apply(String tag) {
+      return tag.startsWith("block-")
+          || tag.startsWith("requires-")
+          || tag.startsWith("no-")
+          || tag.startsWith("supports-")
+          || tag.startsWith("disable-")
+          || tag.equals("local")
+          || tag.startsWith("cpu:");
+    }
+  };
+
   private TargetUtils() {} // Uninstantiable.
 
   public static boolean isTestRuleName(String name) {
@@ -215,13 +233,7 @@ public final class TargetUtils {
       // some internal tags that we don't allow to be set on targets. We also don't want to
       // exhaustively enumerate all the legal values here. Right now, only a ~small set of tags is
       // recognized by Bazel.
-      if (tag.startsWith("block-")
-          || tag.startsWith("requires-")
-          || tag.startsWith("no-")
-          || tag.startsWith("supports-")
-          || tag.startsWith("disable-")
-          || tag.equals("local")
-          || tag.startsWith("cpu:")) {
+      if (LEGAL_EXEC_INFO_KEYS.apply(tag)) {
         map.put(tag, "");
       }
     }
@@ -229,6 +241,14 @@ public final class TargetUtils {
   }
 
   /**
+   * Returns the execution info. These include execution requirement tags ('block-*', 'requires-*',
+   * 'no-*', 'supports-*', 'disable-*', 'local', and 'cpu:*') as keys with empty values.
+   */
+  public static Map<String, String> filter(Map<String, String> executionInfo) {
+    return Maps.filterKeys(executionInfo, LEGAL_EXEC_INFO_KEYS);
+  }
+
+  /**
    * Returns the language part of the rule name (e.g. "foo" for foo_test or foo_binary).
    *
    * <p>In practice this is the part before the "_", if any, otherwise the entire rule class name.