sandbox: Make /tmp and /dev/shm writable by default on Linux.

Also refactor the way we compute writable dirs so that they're computed
only once per running action, not twice.

Fixes #2056, fixes #1973, fixes #1460.

RELNOTES: /tmp and /dev/shm are now writable by default inside the
Linux sandbox.

--
PiperOrigin-RevId: 151123543
MOS_MIGRATED_REVID=151123543

4 files changed, 52 insertions, 18 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java
index 6a892dae9c..d17f5760f9 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java
@@ -207,14 +207,13 @@ public class DarwinSandboxedStrategy extends SandboxStrategy {
     ImmutableMap<String, String> spawnEnvironment =
         StandaloneSpawnStrategy.locallyDeterminedEnv(execRoot, productName, spawn.getEnvironment());
 
-    Set<Path> writableDirs = getWritableDirs(sandboxExecRoot, spawn.getEnvironment());
-
+    Set<Path> writableDirs;
     Path runUnderPath = getRunUnderPath(spawn);
-
     HardlinkedExecRoot hardlinkedExecRoot =
         new HardlinkedExecRoot(execRoot, sandboxPath, sandboxExecRoot, errWriter);
     ImmutableSet<PathFragment> outputs = SandboxHelpers.getOutputFiles(spawn);
     try {
+      writableDirs = getWritableDirs(sandboxExecRoot, spawn.getEnvironment());
       hardlinkedExecRoot.createFileSystem(
           getMounts(spawn, actionExecutionContext), outputs, writableDirs);
     } catch (IOException e) {
@@ -228,11 +227,7 @@ public class DarwinSandboxedStrategy extends SandboxStrategy {
 
     DarwinSandboxRunner runner =
         new DarwinSandboxRunner(
-            sandboxPath,
-            sandboxExecRoot,
-            getWritableDirs(sandboxExecRoot, spawnEnvironment),
-            runUnderPath,
-            verboseFailures);
+            sandboxPath, sandboxExecRoot, writableDirs, runUnderPath, verboseFailures);
     try {
       runSpawn(
           spawn,
@@ -260,11 +255,12 @@ public class DarwinSandboxedStrategy extends SandboxStrategy {
   }
 
   @Override
-  protected ImmutableSet<Path> getWritableDirs(Path sandboxExecRoot, Map<String, String> env) {
-    FileSystem fs = sandboxExecRoot.getFileSystem();
+  protected ImmutableSet<Path> getWritableDirs(Path sandboxExecRoot, Map<String, String> env)
+      throws IOException {
     ImmutableSet.Builder<Path> writableDirs = ImmutableSet.builder();
-
     writableDirs.addAll(super.getWritableDirs(sandboxExecRoot, env));
+
+    FileSystem fs = sandboxExecRoot.getFileSystem();
     writableDirs.add(fs.getPath("/dev"));
 
     String sysTmpDir = System.getenv("TMPDIR");
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 cc7da502b0..b2fdfacf6a 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
@@ -33,10 +33,12 @@ import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.buildtool.BuildRequest;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
+import com.google.devtools.build.lib.vfs.FileSystem;
 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 java.io.IOException;
+import java.util.Map;
 import java.util.Set;
 import java.util.SortedMap;
 import java.util.UUID;
@@ -130,18 +132,18 @@ public class LinuxSandboxedStrategy extends SandboxStrategy {
     Path sandboxPath = SandboxHelpers.getSandboxRoot(blazeDirs, productName, uuid, execCounter);
     Path sandboxExecRoot = sandboxPath.getRelative("execroot").getRelative(execRoot.getBaseName());
 
-    Set<Path> writableDirs = getWritableDirs(sandboxExecRoot, spawn.getEnvironment());
-
+    Set<Path> writableDirs;
     SymlinkedExecRoot symlinkedExecRoot = new SymlinkedExecRoot(sandboxExecRoot);
     ImmutableSet<PathFragment> outputs = SandboxHelpers.getOutputFiles(spawn);
     try {
+      writableDirs = getWritableDirs(sandboxExecRoot, spawn.getEnvironment());
       symlinkedExecRoot.createFileSystem(
           getMounts(spawn, actionExecutionContext), outputs, writableDirs);
     } catch (IOException e) {
       throw new UserExecException("I/O error during sandboxed execution", e);
     }
 
-    SandboxRunner runner = getSandboxRunner(spawn, sandboxPath, sandboxExecRoot);
+    SandboxRunner runner = getSandboxRunner(spawn, sandboxPath, sandboxExecRoot, writableDirs);
     try {
       runSpawn(
           spawn,
@@ -168,14 +170,15 @@ public class LinuxSandboxedStrategy extends SandboxStrategy {
     }
   }
 
-  private SandboxRunner getSandboxRunner(Spawn spawn, Path sandboxPath, Path sandboxExecRoot)
+  private SandboxRunner getSandboxRunner(
+      Spawn spawn, Path sandboxPath, Path sandboxExecRoot, Set<Path> writableDirs)
       throws UserExecException {
     if (fullySupported) {
       return new LinuxSandboxRunner(
           execRoot,
           sandboxPath,
           sandboxExecRoot,
-          getWritableDirs(sandboxExecRoot, spawn.getEnvironment()),
+          writableDirs,
           getTmpfsPaths(),
           getReadOnlyBindMounts(blazeDirs, sandboxExecRoot),
           verboseFailures,
@@ -185,6 +188,19 @@ public class LinuxSandboxedStrategy extends SandboxStrategy {
     }
   }
 
+  @Override
+  protected ImmutableSet<Path> getWritableDirs(Path sandboxExecRoot, Map<String, String> env)
+      throws IOException {
+    ImmutableSet.Builder<Path> writableDirs = ImmutableSet.builder();
+    writableDirs.addAll(super.getWritableDirs(sandboxExecRoot, env));
+
+    FileSystem fs = sandboxExecRoot.getFileSystem();
+    writableDirs.add(fs.getPath("/dev/shm").resolveSymbolicLinks());
+    writableDirs.add(fs.getPath("/tmp"));
+
+    return writableDirs.build();
+  }
+
   private ImmutableSet<Path> getTmpfsPaths() {
     ImmutableSet.Builder<Path> tmpfsPaths = ImmutableSet.builder();
     for (String tmpfsPath : sandboxOptions.sandboxTmpfsPath) {
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStrategy.java
index ba88c8fa54..778b8e5a00 100644
--- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStrategy.java
@@ -120,8 +120,13 @@ abstract class SandboxStrategy implements SandboxedSpawnActionContext {
     }
   }
 
-  /** Gets the list of directories that the spawn will assume to be writable. */
-  protected ImmutableSet<Path> getWritableDirs(Path sandboxExecRoot, Map<String, String> env) {
+  /**
+   * Gets the list of directories that the spawn will assume to be writable.
+   *
+   * @throws IOException because we might resolve symlinks, which throws {@link IOException}.
+   */
+  protected ImmutableSet<Path> getWritableDirs(Path sandboxExecRoot, Map<String, String> env)
+      throws IOException {
     Builder<Path> writableDirs = ImmutableSet.builder();
     // We have to make the TEST_TMPDIR directory writable if it is specified.
     if (env.containsKey("TEST_TMPDIR")) {
diff --git a/src/test/shell/bazel/linux-sandbox_test.sh b/src/test/shell/bazel/linux-sandbox_test.sh
index 6529ae2de4..c6642be252 100755
--- a/src/test/shell/bazel/linux-sandbox_test.sh
+++ b/src/test/shell/bazel/linux-sandbox_test.sh
@@ -207,6 +207,23 @@ function test_redirect_output() {
   assert_equals "err" "$(cat $ERR)"
 }
 
+function test_tmp_is_writable() {
+  # If /tmp is not writable on the host, it won't be inside the sandbox.
+  test -w /tmp || return 0
+
+  $linux_sandbox $SANDBOX_DEFAULT_OPTS -w /tmp -- /bin/bash -c "rm -f $(mktemp --tmpdir=/tmp)" \
+    &> $TEST_log || fail
+}
+
+function test_dev_shm_is_writable() {
+  # If /dev/shm is not writable on the host, it won't be inside the sandbox.
+  test -w /dev/shm || return 0
+
+  # /dev/shm is often a symlink to /run/shm, thus we use readlink to get the canonical path.
+  $linux_sandbox $SANDBOX_DEFAULT_OPTS -w "$(readlink -f /dev/shm)" -- /bin/bash -c "rm -f $(mktemp --tmpdir=/dev/shm)" \
+    &> $TEST_log || fail
+}
+
 # The test shouldn't fail if the environment doesn't support running it.
 check_supported_platform || exit 0
 check_sandbox_allowed || exit 0