sandbox:

- add flag --sandbox_add_path, which takes a list of additional paths as argument and mount these paths to sandbox. Fixes #884.
- mount target of /etc/resolv.conf if it is a symlink. Fixes #738.

RELNOTES:
- add flag --sandbox_add_path, which takes a list of additional paths as argument and mount these paths to sandbox.
- mount target of /etc/resolv.conf if it is a symlink.

--
MOS_MIGRATED_REVID=117364211

3 files changed, 115 insertions, 3 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java
index 76e18e7e09..69ed5666a7 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java
@@ -15,6 +15,7 @@ package com.google.devtools.build.lib.sandbox;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Predicates;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.io.Files;
@@ -74,6 +75,7 @@ public class LinuxSandboxedStrategy implements SpawnActionContext {
   private final boolean verboseFailures;
   private final boolean sandboxDebug;
   private final StandaloneSpawnStrategy standaloneStrategy;
+  private final List<String> sandboxAddPath;
   private final UUID uuid = UUID.randomUUID();
   private final AtomicInteger execCounter = new AtomicInteger();
 
@@ -82,13 +84,15 @@ public class LinuxSandboxedStrategy implements SpawnActionContext {
       BlazeDirectories blazeDirs,
       ExecutorService backgroundWorkers,
       boolean verboseFailures,
-      boolean sandboxDebug) {
+      boolean sandboxDebug,
+      List<String> sandboxAddPath) {
     this.clientEnv = ImmutableMap.copyOf(clientEnv);
     this.blazeDirs = blazeDirs;
     this.execRoot = blazeDirs.getExecRoot();
     this.backgroundWorkers = Preconditions.checkNotNull(backgroundWorkers);
     this.verboseFailures = verboseFailures;
     this.sandboxDebug = sandboxDebug;
+    this.sandboxAddPath = sandboxAddPath;
     this.standaloneStrategy = new StandaloneSpawnStrategy(blazeDirs.getExecRoot(), verboseFailures);
   }
 
@@ -221,6 +225,8 @@ public class LinuxSandboxedStrategy implements SpawnActionContext {
       throws IOException, ExecException {
     ImmutableMap.Builder<Path, Path> result = new ImmutableMap.Builder<>();
     result.putAll(mountUsualUnixDirs());
+    result.putAll(mountUserDefinedPath());
+
     MountMap mounts = new MountMap();
     mounts.putAll(setupBlazeUtils());
     mounts.putAll(mountRunfilesFromManifests(spawn));
@@ -303,7 +309,19 @@ public class LinuxSandboxedStrategy implements SpawnActionContext {
     FileSystem fs = blazeDirs.getFileSystem();
     mounts.put(fs.getPath("/bin"), fs.getPath("/bin"));
     mounts.put(fs.getPath("/sbin"), fs.getPath("/sbin"));
-    mounts.put(fs.getPath("/etc"), fs.getPath("/etc"));
+
+    // Check if /etc/resolv.conf is a symlink and mount its target
+    // Fix #738
+    Path resolv = fs.getPath("/etc/resolv.conf");
+    if (resolv.exists() && resolv.isSymbolicLink()) {
+      mounts.put(resolv, resolv.resolveSymbolicLinks());
+
+      List<Path> resolvList = ImmutableList.of(resolv);
+      mounts.putAll(mountDirExclude(fs.getPath("/etc"), resolvList));
+    } else {
+      mounts.put(fs.getPath("/etc"), fs.getPath("/etc"));
+    }
+
     for (String entry : NativePosixFiles.readdir("/")) {
       if (entry.startsWith("lib")) {
         Path libDir = fs.getRootDirectory().getRelative(entry);
@@ -498,6 +516,86 @@ public class LinuxSandboxedStrategy implements SpawnActionContext {
     return mounts;
   }
 
+  /**
+   * Mount all user defined path in --sandbox_add_path.
+   */
+  private MountMap mountUserDefinedPath() throws IOException {
+    MountMap mounts = new MountMap();
+    FileSystem fs = blazeDirs.getFileSystem();
+
+    ImmutableList<Path> exclude =
+        ImmutableList.of(blazeDirs.getWorkspace(), blazeDirs.getOutputBase());
+
+    for (String pathStr : sandboxAddPath) {
+      Path path = fs.getPath(pathStr);
+
+      // Check if path is in {workspace, outputBase}
+      for (Path exc : exclude) {
+        if (path.startsWith(exc)) {
+          throw new IllegalArgumentException(
+              "Mounting subdirectory of WORKSPACE or OUTPUTBASE to sandbox is not allowed.");
+        }
+      }
+
+      // Check if path is ancestor of {workspace, outputBase}
+      // Mount subdirectory of path except {workspace, outputBase}
+      mounts.putAll(mountChildDirExclude(path, exclude));
+    }
+
+    return mounts;
+  }
+
+  /**
+   * Mount all subdirectories recursively except some paths
+   */
+  private MountMap mountDirExclude(Path path, List<Path> exclude) throws IOException {
+    MountMap mounts = new MountMap();
+
+    if (!path.isDirectory(Symlinks.NOFOLLOW)) {
+      if (!exclude.contains(path)) {
+        mounts.put(path, path);
+      }
+      return mounts;
+    }
+
+    try {
+      for (Path child : path.getDirectoryEntries()) {
+        // Ignore broken symlink
+        if (!child.exists()) {
+          continue;
+        }
+
+        mounts.putAll(mountChildDirExclude(child, exclude));
+      }
+    } catch (IOException e) {
+      throw new IOException("Illegal additional path for mount", e);
+    }
+
+    return mounts;
+  }
+
+  /**
+   * Helper function of mountDirExclude and mountUserDefinedPath
+   */
+  private MountMap mountChildDirExclude(Path child, List<Path> exclude) throws IOException {
+    MountMap mounts = new MountMap();
+
+    boolean startsWithFlag = false;
+    for (Path exc : exclude) {
+      if (exc.startsWith(child)) {
+        startsWithFlag = true;
+        break;
+      }
+    }
+    if (!startsWithFlag) {
+      mounts.put(child, child);
+    } else if (!exclude.contains(child)) {
+      mounts.putAll(mountDirExclude(child, exclude));
+    }
+
+    return mounts;
+  }
+
   @Override
   public boolean willExecuteRemotely(boolean remotable) {
     return false;
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java
index 03476912a1..914b0c56e4 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java
@@ -22,6 +22,7 @@ import com.google.devtools.build.lib.exec.ExecutionOptions;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
 import com.google.devtools.build.lib.util.OS;
 
+import java.util.List;
 import java.util.concurrent.ExecutorService;
 
 /**
@@ -36,6 +37,7 @@ public class SandboxActionContextProvider extends ActionContextProvider {
       CommandEnvironment env, BuildRequest buildRequest, ExecutorService backgroundWorkers) {
     boolean verboseFailures = buildRequest.getOptions(ExecutionOptions.class).verboseFailures;
     boolean sandboxDebug = buildRequest.getOptions(SandboxOptions.class).sandboxDebug;
+    List<String> sandboxAddPath = buildRequest.getOptions(SandboxOptions.class).sandboxAddPath;
     Builder<ActionContext> strategies = ImmutableList.builder();
 
     if (OS.getCurrent() == OS.LINUX) {
@@ -45,7 +47,8 @@ public class SandboxActionContextProvider extends ActionContextProvider {
               env.getDirectories(),
               backgroundWorkers,
               verboseFailures,
-              sandboxDebug));
+              sandboxDebug,
+              sandboxAddPath));
     }
 
     this.strategies = strategies.build();
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java
index 32b715f229..f1291d2cbb 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxOptions.java
@@ -16,6 +16,8 @@ package com.google.devtools.build.lib.sandbox;
 import com.google.devtools.common.options.Option;
 import com.google.devtools.common.options.OptionsBase;
 
+import java.util.List;
+
 /**
  * Options for sandboxed execution.
  */
@@ -38,4 +40,13 @@ public class SandboxOptions extends OptionsBase {
             + "Bazel or Skylark rules with debugging failures due to missing input files, etc."
   )
   public boolean sandboxDebug;
+
+  @Option(
+    name = "sandbox_add_path",
+    allowMultiple = true,
+    defaultValue = "",
+    category = "config",
+    help = "Add additional path to mount to sandbox. Path including workspace is not allowed."
+  )
+  public List<String> sandboxAddPath;
 }