[7.0.1] Add support for bind mounts under /tmp with hermetic tmp (#…

…20749)

This is achieved by rewriting the user-specified mounts to mounts onto
subdirectories of the hermetic sandbox tmp directory.

Fixes #20527

Closes #20583.

Commit
5e68afd

PiperOrigin-RevId: 595815029
Change-Id: Ibfe5f67fb8fb59131b6c82a826ed5200f2b10a94

Co-authored-by: Fabian Meumertzheim <[email protected]>

src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java

@@ -14,13 +14,13 @@
 
 package com.google.devtools.build.lib.sandbox;
 
+import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.devtools.build.lib.sandbox.LinuxSandboxCommandLineBuilder.NetworkNamespace.NETNS_WITH_LOOPBACK;
 import static com.google.devtools.build.lib.sandbox.LinuxSandboxCommandLineBuilder.NetworkNamespace.NO_NETNS;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Maps;
 import com.google.common.io.ByteStreams;
 import com.google.devtools.build.lib.actions.ActionInput;
 import com.google.devtools.build.lib.actions.ExecException;
@@ -61,6 +61,7 @@
 import java.util.Map;
 import java.util.Optional;
 import java.util.SortedMap;
+import java.util.TreeMap;
 import java.util.concurrent.atomic.AtomicBoolean;
 import javax.annotation.Nullable;
 
@@ -313,7 +314,8 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context
             .addExecutionInfo(spawn.getExecutionInfo())
             .setWritableFilesAndDirectories(writableDirs)
             .setTmpfsDirectories(ImmutableSet.copyOf(getSandboxOptions().sandboxTmpfsPath))
-            .setBindMounts(getBindMounts(blazeDirs, inputs, sandboxExecRootBase, sandboxTmp))
+            .setBindMounts(
+                prepareAndGetBindMounts(blazeDirs, inputs, sandboxExecRootBase, sandboxTmp))
             .setUseFakeHostname(getSandboxOptions().sandboxFakeHostname)
             .setEnablePseudoterminal(getSandboxOptions().sandboxExplicitPseudoterminal)
             .setCreateNetworkNamespace(createNetworkNamespace ? NETNS_WITH_LOOPBACK : NO_NETNS)
@@ -403,50 +405,73 @@ protected ImmutableSet<Path> getWritableDirs(
     return writableDirs.build();
   }
 
-  private ImmutableList<BindMount> getBindMounts(
+  private ImmutableList<BindMount> prepareAndGetBindMounts(
       BlazeDirectories blazeDirs,
       SandboxInputs inputs,
       Path sandboxExecRootBase,
       @Nullable Path sandboxTmp)
-      throws UserExecException {
-    Path tmpPath = fileSystem.getPath("/tmp");
-    final SortedMap<Path, Path> bindMounts = Maps.newTreeMap();
+      throws UserExecException, IOException {
+    Path tmpPath = fileSystem.getPath(SLASH_TMP);
+    final SortedMap<Path, Path> userBindMounts = new TreeMap<>();
     SandboxHelpers.mountAdditionalPaths(
-        getSandboxOptions().sandboxAdditionalMounts, sandboxExecRootBase, bindMounts);
+        getSandboxOptions().sandboxAdditionalMounts, sandboxExecRootBase, userBindMounts);
 
     for (Path inaccessiblePath : getInaccessiblePaths()) {
       if (inaccessiblePath.isDirectory(Symlinks.NOFOLLOW)) {
-        bindMounts.put(inaccessiblePath, inaccessibleHelperDir);
+        userBindMounts.put(inaccessiblePath, inaccessibleHelperDir);
       } else {
-        bindMounts.put(inaccessiblePath, inaccessibleHelperFile);
+        userBindMounts.put(inaccessiblePath, inaccessibleHelperFile);
       }
     }
 
-    LinuxSandboxUtil.validateBindMounts(bindMounts);
-    ImmutableList.Builder<BindMount> result = ImmutableList.builder();
-    bindMounts.forEach((k, v) -> result.add(BindMount.of(k, v)));
+    LinuxSandboxUtil.validateBindMounts(userBindMounts);
 
-    if (sandboxTmp != null) {
-      // First mount the real exec root and the empty directory created as the working dir of the
-      // action under $SANDBOX/_tmp
-      result.add(BindMount.of(sandboxTmp.getRelative(BAZEL_EXECROOT), blazeDirs.getExecRootBase()));
-      result.add(
-          BindMount.of(sandboxTmp.getRelative(BAZEL_WORKING_DIRECTORY), sandboxExecRootBase));
-
-      // Then mount the individual package roots under $SANDBOX/_tmp/bazel-source-roots
-      inputs
-          .getSourceRootBindMounts()
-          .forEach(
-              (withinSandbox, real) -> {
-                PathFragment sandboxTmpSourceRoot = withinSandbox.asPath().relativeTo(tmpPath);
-                result.add(BindMount.of(sandboxTmp.getRelative(sandboxTmpSourceRoot), real));
-              });
-
-      // Then mount $SANDBOX/_tmp at /tmp. At this point, even if the output base (and execroot)
-      // and individual source roots are under /tmp, they are accessible at /tmp/bazel-*
-      result.add(BindMount.of(tmpPath, sandboxTmp));
+    if (sandboxTmp == null) {
+      return userBindMounts.entrySet().stream()
+          .map(e -> BindMount.of(e.getKey(), e.getValue()))
+          .collect(toImmutableList());
     }
 
+    SortedMap<Path, Path> bindMounts = new TreeMap<>();
+    for (var entry : userBindMounts.entrySet()) {
+      Path mountPoint = entry.getKey();
+      Path content = entry.getValue();
+      if (mountPoint.startsWith(tmpPath)) {
+        // sandboxTmp should be null if /tmp is an explicit mount point since useHermeticTmp()
+        // returns false in that case.
+        if (mountPoint.equals(tmpPath)) {
+          throw new IOException(
+              "Cannot mount /tmp explicitly with hermetic /tmp. Please file a bug at"
+                  + " https://github.com/bazelbuild/bazel/issues/new/choose.");
+        }
+        // We need to rewrite the mount point to be under the sandbox tmp directory, which will be
+        // mounted onto /tmp as the final mount.
+        mountPoint = sandboxTmp.getRelative(mountPoint.relativeTo(tmpPath));
+        mountPoint.createDirectoryAndParents();
+      }
+      bindMounts.put(mountPoint, content);
+    }
+
+    ImmutableList.Builder<BindMount> result = ImmutableList.builder();
+    bindMounts.forEach((k, v) -> result.add(BindMount.of(k, v)));
+
+    // First mount the real exec root and the empty directory created as the working dir of the
+    // action under $SANDBOX/_tmp
+    result.add(BindMount.of(sandboxTmp.getRelative(BAZEL_EXECROOT), blazeDirs.getExecRootBase()));
+    result.add(BindMount.of(sandboxTmp.getRelative(BAZEL_WORKING_DIRECTORY), sandboxExecRootBase));
+
+    // Then mount the individual package roots under $SANDBOX/_tmp/bazel-source-roots
+    inputs
+        .getSourceRootBindMounts()
+        .forEach(
+            (withinSandbox, real) -> {
+              PathFragment sandboxTmpSourceRoot = withinSandbox.asPath().relativeTo(tmpPath);
+              result.add(BindMount.of(sandboxTmp.getRelative(sandboxTmpSourceRoot), real));
+            });
+
+    // Then mount $SANDBOX/_tmp at /tmp. At this point, even if the output base (and execroot) and
+    // individual source roots are under /tmp, they are accessible at /tmp/bazel-*
+    result.add(BindMount.of(tmpPath, sandboxTmp));
     return result.build();
   }
 

src/test/shell/bazel/bazel_sandboxing_test.sh

@@ -316,21 +316,84 @@ function test_add_mount_pair_tmp_source() {
 
   sed -i.bak '/sandbox_tmpfs_path/d' $TEST_TMPDIR/bazelrc
 
+  local mounted=$(mktemp -d "/tmp/bazel_mounted.XXXXXXXX")
+  trap "rm -fr $mounted" EXIT
+  echo GOOD > "$mounted/data.txt"
+
   mkdir -p pkg
-  cat > pkg/BUILD <<'EOF'
+  cat > pkg/BUILD <<EOF
+genrule(
+    name = "gen",
+    outs = ["gen.txt"],
+    # Verify that /tmp is still hermetic.
+    cmd = """[ ! -e "${mounted}/data.txt" ] && cp /etc/data.txt \$@""",
+)
+EOF
+
+  # This assumes the existence of /etc on the host system
+  bazel build --sandbox_add_mount_pair="$mounted:/etc" //pkg:gen || fail "build failed"
+  assert_contains GOOD bazel-bin/pkg/gen.txt
+}
+
+function test_add_mount_pair_tmp_target() {
+  if [[ "$PLATFORM" == "darwin" ]]; then
+    # Tests Linux-specific functionality
+    return 0
+  fi
+
+  create_workspace_with_default_repos WORKSPACE
+
+  sed -i.bak '/sandbox_tmpfs_path/d' $TEST_TMPDIR/bazelrc
+
+  local source_dir=$(mktemp -d "/tmp/bazel_mounted.XXXXXXXX")
+  trap "rm -fr $source_dir" EXIT
+  echo BAD > "$source_dir/data.txt"
+
+  mkdir -p pkg
+  cat > pkg/BUILD <<EOF
 genrule(
     name = "gen",
     outs = ["gen.txt"],
-    cmd = "cp /etc/data.txt $@",
+    # Verify that /tmp is still hermetic.
+    cmd = """[ ! -e "${source_dir}/data.txt" ] && ls "$source_dir" > \$@""",
 )
 EOF
 
+
+  # This assumes the existence of /etc on the host system
+  bazel build --sandbox_add_mount_pair="/etc:$source_dir" //pkg:gen || fail "build failed"
+  assert_contains passwd bazel-bin/pkg/gen.txt
+}
+
+function test_add_mount_pair_tmp_target_and_source() {
+  if [[ "$PLATFORM" == "darwin" ]]; then
+    # Tests Linux-specific functionality
+    return 0
+  fi
+
+  create_workspace_with_default_repos WORKSPACE
+
+  sed -i.bak '/sandbox_tmpfs_path/d' $TEST_TMPDIR/bazelrc
+
   local mounted=$(mktemp -d "/tmp/bazel_mounted.XXXXXXXX")
   trap "rm -fr $mounted" EXIT
   echo GOOD > "$mounted/data.txt"
 
-  # This assumes the existence of /etc on the host system
-  bazel build --sandbox_add_mount_pair="$mounted:/etc" //pkg:gen || fail "build failed"
+  local tmp_file=$(mktemp "/tmp/bazel_tmp.XXXXXXXX")
+  trap "rm $tmp_file" EXIT
+  echo BAD > "$tmp_file"
+
+  mkdir -p pkg
+  cat > pkg/BUILD <<EOF
+genrule(
+    name = "gen",
+    outs = ["gen.txt"],
+    # Verify that /tmp is still hermetic.
+    cmd = """[ ! -e "${tmp_file}" ] && cp "$mounted/data.txt" \$@""",
+)
+EOF
+
+  bazel build --sandbox_add_mount_pair="$mounted" //pkg:gen || fail "build failed"
   assert_contains GOOD bazel-bin/pkg/gen.txt
 }