[7.2.0] Preserve paths under hermetic /tmp in the sandbox (#22407)

The bind mounting scheme used with the Linux sandbox' hermetic `/tmp`
feature is modified to preserve all paths as they are outside the
sandbox, which removes the need to rewrite paths when staging inputs
into and, crucially, moving outputs out of the sandbox.

Source roots and output base paths under `/tmp` are now treated just
like any user-specified bind mount under `/tmp`: They are mounted under
the hermetic tmp directory with their path relativized against `/tmp`
before the hermetic tmp directory is mounted as `/tmp` as the final
step.

There is one caveat compared to user-specified mounts: Source roots,
which may themselves not lie under `/tmp`, can be symlinks to
directories under `/tmp` (e.g., when they arise from a
`local_repository`). To handle this situation in the common case, all
parent directories of package path entries (up to direct children of
`/tmp`) are mounted into the sandbox. If users use `local_repository`s
with fixed target paths under `/tmp`, they will need to specify
`--sandbox_add_mount_pair`.

Overlayfs has been considered as an alternative to this approach, but
ultimately doesn't seem to work for this use case since its `lowerpath`,
which would be `/tmp`, is not allowed to have child mounts from a
different user namespace (see
https://unix.stackexchange.com/questions/776030/mounting-overlayfs-in-a-user-namespace-with-child-mounts).
However, this is exactly the situation created by a Bazel-in-Bazel test
and can also arise if the user has existing mounts under `/tmp` when
using Bazel (e.g. the JetBrains toolbox on Linux uses such mounts).

This replaces and mostly reverts the following commits, but keeps their
tests:
*
bf6ebe9
*
fb6658c
*
bc1d9d3
*
1829883
*
70691f2
*
a556969
*
8e32f44
(had its test lost in an incorrect merge conflict resolution, this PR
adds it back)

Fixes #20533
Work towards #20753
Fixes #21215
Fixes #22117
Fixes #22226
Fixes #22290

RELNOTES: Paths in the Linux sandbox are now again identical to those
outside the sandbox, even with `--incompatible_sandbox_hermetic_tmp`.

Closes #22001.

PiperOrigin-RevId: 634381503
Change-Id: I9f7f3948c705be120c55c9b0c51204e5bea45f61

Fixes #22291

Author: fmeum

src/main/java/com/google/devtools/build/lib/actions/FileArtifactValue.java

@@ -170,19 +170,11 @@ protected boolean couldBeModifiedByMetadata(FileArtifactValue lastKnown) {
   /**
    * Optional materialization path.
    *
-   * <p>If present, this artifact is a copy of another artifact whose contents live at this path.
-   * This can happen when it is declared as a file and not as an unresolved symlink but the action
-   * that creates it materializes it in the filesystem as a symlink to another output artifact. This
-   * information is useful in two situations:
-   *
-   * <ol>
-   *   <li>When the symlink target is a remotely stored artifact, we can avoid downloading it
-   *       multiple times when building without the bytes (see AbstractActionInputPrefetcher).
-   *   <li>When the symlink target is inaccessible from the sandboxed environment an action runs
-   *       under, we can rewrite it accordingly (see SandboxHelpers).
-   * </ol>
-   *
-   * @see com.google.devtools.build.lib.skyframe.TreeArtifactValue#getMaterializationExecPath().
+   * <p>If present, this artifact is a copy of another artifact. It is still tracked as a
+   * non-symlink by Bazel, but materialized in the local filesystem as a symlink to the original
+   * artifact, whose contents live at this location. This is used by {@link
+   * com.google.devtools.build.lib.remote.AbstractActionInputPrefetcher} to implement zero-cost
+   * copies of remotely stored artifacts.
    */
   public Optional<PathFragment> getMaterializationExecPath() {
     return Optional.empty();
@@ -223,12 +215,6 @@ public static FileArtifactValue createForSourceArtifact(
         xattrProvider);
   }
 
-  public static FileArtifactValue createForResolvedSymlink(
-      PathFragment realPath, FileArtifactValue metadata, @Nullable byte[] digest) {
-    return new ResolvedSymlinkFileArtifactValue(
-        realPath, digest, metadata.getContentsProxy(), metadata.getSize());
-  }
-
   public static FileArtifactValue createFromInjectedDigest(
       FileArtifactValue metadata, @Nullable byte[] digest) {
     return createForNormalFile(digest, metadata.getContentsProxy(), metadata.getSize());
@@ -459,25 +445,7 @@ public String toString() {
     }
   }
 
-  private static final class ResolvedSymlinkFileArtifactValue extends RegularFileArtifactValue {
-    private final PathFragment realPath;
-
-    private ResolvedSymlinkFileArtifactValue(
-        PathFragment realPath,
-        @Nullable byte[] digest,
-        @Nullable FileContentsProxy proxy,
-        long size) {
-      super(digest, proxy, size);
-      this.realPath = realPath;
-    }
-
-    @Override
-    public Optional<PathFragment> getMaterializationExecPath() {
-      return Optional.of(realPath);
-    }
-  }
-
-  private static class RegularFileArtifactValue extends FileArtifactValue {
+  private static final class RegularFileArtifactValue extends FileArtifactValue {
     private final byte[] digest;
     @Nullable private final FileContentsProxy proxy;
     private final long size;
@@ -500,8 +468,7 @@ public boolean equals(Object o) {
       RegularFileArtifactValue that = (RegularFileArtifactValue) o;
       return Arrays.equals(digest, that.digest)
           && Objects.equals(proxy, that.proxy)
-          && size == that.size
-          && Objects.equals(getMaterializationExecPath(), that.getMaterializationExecPath());
+          && size == that.size;
     }
 
     @Override

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

@@ -27,7 +27,6 @@
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
-import com.google.devtools.build.lib.vfs.RootedPath;
 import java.io.IOException;
 import java.util.LinkedHashSet;
 import java.util.Set;
@@ -163,9 +162,9 @@ void createInputs(Iterable<PathFragment> inputsToCreate, SandboxInputs inputs)
       }
       Path key = sandboxExecRoot.getRelative(fragment);
       if (inputs.getFiles().containsKey(fragment)) {
-        RootedPath fileDest = inputs.getFiles().get(fragment);
+        Path fileDest = inputs.getFiles().get(fragment);
         if (fileDest != null) {
-          copyFile(fileDest.asPath(), key);
+          copyFile(fileDest, key);
         } else {
           FileSystemUtils.createEmptyFile(key);
         }

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

@@ -360,21 +360,19 @@ private boolean wasTimeout(Duration timeout, Duration wallTime) {
   /**
    * Gets the list of directories that the spawn will assume to be writable.
    *
-   * @param sandboxExecRoot the exec root of the sandbox from the point of view of the Bazel process
-   * @param withinSandboxExecRoot the exec root from the point of view of the sandboxed processes
+   * @param sandboxExecRoot the exec root of the sandbox
    * @param env the environment of the sandboxed processes
    * @throws IOException because we might resolve symlinks, which throws {@link IOException}.
    */
-  protected ImmutableSet<Path> getWritableDirs(
-      Path sandboxExecRoot, Path withinSandboxExecRoot, Map<String, String> env)
+  protected ImmutableSet<Path> getWritableDirs(Path sandboxExecRoot, Map<String, String> env)
       throws IOException {
     // We have to make the TEST_TMPDIR directory writable if it is specified.
     ImmutableSet.Builder<Path> writablePaths = ImmutableSet.builder();
 
     // On Windows, sandboxExecRoot is actually the main execroot. We will specify
     // exactly which output path is writable.
     if (OS.getCurrent() != OS.WINDOWS) {
-      writablePaths.add(withinSandboxExecRoot);
+      writablePaths.add(execRoot);
     }
 
     String testTmpdir = env.get("TEST_TMPDIR");

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

@@ -37,7 +37,6 @@
 import com.google.devtools.build.lib.vfs.FileSystem;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
-import com.google.devtools.build.lib.vfs.Root;
 import java.io.BufferedWriter;
 import java.io.File;
 import java.io.IOException;
@@ -103,7 +102,6 @@ private static boolean computeIsSupported() throws InterruptedException {
 
   private final SandboxHelpers helpers;
   private final Path execRoot;
-  private final ImmutableList<Root> packageRoots;
   private final boolean allowNetwork;
   private final ProcessWrapper processWrapper;
   private final Path sandboxBase;
@@ -134,7 +132,6 @@ private static boolean computeIsSupported() throws InterruptedException {
     super(cmdEnv);
     this.helpers = helpers;
     this.execRoot = cmdEnv.getExecRoot();
-    this.packageRoots = cmdEnv.getPackageLocator().getPathEntries();
     this.allowNetwork = helpers.shouldAllowNetwork(cmdEnv.getOptions());
     this.alwaysWritableDirs = getAlwaysWritableDirs(cmdEnv.getRuntime().getFileSystem());
     this.processWrapper = ProcessWrapper.fromCommandEnvironment(cmdEnv);
@@ -221,18 +218,13 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context
         localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), binTools, "/tmp");
 
     final HashSet<Path> writableDirs = new HashSet<>(alwaysWritableDirs);
-    ImmutableSet<Path> extraWritableDirs =
-        getWritableDirs(sandboxExecRoot, sandboxExecRoot, environment);
+    ImmutableSet<Path> extraWritableDirs = getWritableDirs(sandboxExecRoot, environment);
     writableDirs.addAll(extraWritableDirs);
 
     SandboxInputs inputs =
         helpers.processInputFiles(
             context.getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true),
-            context.getInputMetadataProvider(),
-            execRoot,
-            execRoot,
-            packageRoots,
-            null);
+            execRoot);
     SandboxOutputs outputs = helpers.getOutputs(spawn);
 
     final Path sandboxConfigPath = sandboxPath.getRelative("sandbox.sb");

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

@@ -45,7 +45,6 @@
 import com.google.devtools.build.lib.util.ProcessUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
-import com.google.devtools.build.lib.vfs.Root;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
@@ -143,7 +142,6 @@ public static boolean isSupported(CommandEnvironment cmdEnv, Path dockerClient)
 
   private final SandboxHelpers helpers;
   private final Path execRoot;
-  private final ImmutableList<Root> packageRoots;
   private final boolean allowNetwork;
   private final Path dockerClient;
   private final ProcessWrapper processWrapper;
@@ -181,7 +179,6 @@ public static boolean isSupported(CommandEnvironment cmdEnv, Path dockerClient)
     super(cmdEnv);
     this.helpers = helpers;
     this.execRoot = cmdEnv.getExecRoot();
-    this.packageRoots = cmdEnv.getPackageLocator().getPathEntries();
     this.allowNetwork = helpers.shouldAllowNetwork(cmdEnv.getOptions());
     this.dockerClient = dockerClient;
     this.processWrapper = ProcessWrapper.fromCommandEnvironment(cmdEnv);
@@ -226,11 +223,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context
     SandboxInputs inputs =
         helpers.processInputFiles(
             context.getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true),
-            context.getInputMetadataProvider(),
-            execRoot,
-            execRoot,
-            packageRoots,
-            null);
+            execRoot);
     SandboxOutputs outputs = helpers.getOutputs(spawn);
 
     Duration timeout = context.getTimeout();

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

@@ -17,9 +17,9 @@
 import static com.google.devtools.build.lib.sandbox.LinuxSandboxCommandLineBuilder.NetworkNamespace.NETNS;
 import static com.google.devtools.build.lib.sandbox.LinuxSandboxCommandLineBuilder.NetworkNamespace.NETNS_WITH_LOOPBACK;
 
-import com.google.auto.value.AutoValue;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.ExecutionRequirements;
 import com.google.devtools.build.lib.vfs.Path;
@@ -36,20 +36,6 @@
  * linux-sandbox} tool.
  */
 public class LinuxSandboxCommandLineBuilder {
-  /** A bind mount that needs to be present when the sandboxed command runs. */
-  @AutoValue
-  public abstract static class BindMount {
-    public static BindMount of(Path mountPoint, Path source) {
-      return new AutoValue_LinuxSandboxCommandLineBuilder_BindMount(mountPoint, source);
-    }
-
-    /** "target" in mount(2) */
-    public abstract Path getMountPoint();
-
-    /** "source" in mount(2) */
-    public abstract Path getContent();
-  }
-
   private final Path linuxSandboxPath;
   private Path hermeticSandboxPath;
   private Path workingDirectory;
@@ -60,7 +46,7 @@ public static BindMount of(Path mountPoint, Path source) {
   private Path stderrPath;
   private Set<Path> writableFilesAndDirectories = ImmutableSet.of();
   private ImmutableSet<PathFragment> tmpfsDirectories = ImmutableSet.of();
-  private List<BindMount> bindMounts = ImmutableList.of();
+  private Map<Path, Path> bindMounts = ImmutableMap.of();
   private Path statisticsPath;
   private boolean useFakeHostname = false;
   private NetworkNamespace createNetworkNamespace = NetworkNamespace.NO_NETNS;
@@ -164,7 +150,7 @@ public LinuxSandboxCommandLineBuilder setTmpfsDirectories(
    * if any.
    */
   @CanIgnoreReturnValue
-  public LinuxSandboxCommandLineBuilder setBindMounts(List<BindMount> bindMounts) {
+  public LinuxSandboxCommandLineBuilder setBindMounts(Map<Path, Path> bindMounts) {
     this.bindMounts = bindMounts;
     return this;
   }
@@ -273,11 +259,12 @@ public ImmutableList<String> buildForCommand(List<String> commandArguments) {
     for (PathFragment tmpfsPath : tmpfsDirectories) {
       commandLineBuilder.add("-e", tmpfsPath.getPathString());
     }
-    for (BindMount bindMount : bindMounts) {
-      commandLineBuilder.add("-M", bindMount.getContent().getPathString());
+    for (Path bindMountTarget : bindMounts.keySet()) {
+      Path bindMountSource = bindMounts.get(bindMountTarget);
+      commandLineBuilder.add("-M", bindMountSource.getPathString());
       // The file is mounted in a custom location inside the sandbox.
-      if (!bindMount.getContent().equals(bindMount.getMountPoint())) {
-        commandLineBuilder.add("-m", bindMount.getMountPoint().getPathString());
+      if (!bindMountSource.equals(bindMountTarget)) {
+        commandLineBuilder.add("-m", bindMountTarget.getPathString());
       }
     }
     if (statisticsPath != null) {