[7.4.0] Create writable dirs under hermetic tmp in the sandbox (#23796)

Fixes #23754 Closes #23755. PiperOrigin-RevId: 679472028 Change-Id: I0ea922ee6edf28c5643c6f2b524371f1d5405c9c Commit 765d5e0 Co-authored-by: Fabian Meumertzheim <[email protected]>
bazelbuild · Oct 8, 2024 · 952ef86 · 952ef86
1 parent a6ad182
commit 952ef86
Show file tree

Hide file tree

Showing 2 changed files with 24 additions and 9 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java
@@ -261,6 +261,10 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context
             context.getInputMapping(PathFragment.EMPTY_FRAGMENT, /* willAccessRepeatedly= */ true),
             execRoot);
 
+    ImmutableMap<String, String> environment =
+        localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), binTools, "/tmp");
+    ImmutableSet<Path> writableDirs = getWritableDirs(sandboxExecRoot, environment);
+
     Path sandboxTmp = null;
     ImmutableSet<Path> pathsUnderTmpToMount = ImmutableSet.of();
     if (useHermeticTmp()) {
@@ -272,21 +276,21 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context
       sandboxTmp = sandboxPath.getRelative("_hermetic_tmp");
       sandboxTmp.createDirectoryAndParents();
 
-      for (PathFragment pathFragment : getSandboxOptions().sandboxTmpfsPath) {
+      for (PathFragment pathFragment :
+          Iterables.concat(
+              getSandboxOptions().sandboxTmpfsPath,
+              Iterables.transform(writableDirs, Path::asFragment))) {
         Path path = fileSystem.getPath(pathFragment);
         if (path.startsWith(slashTmp)) {
-          // tmpfs mount points must exist, which is usually the user's responsibility. But if the
-          // user requests a tmpfs mount under /tmp, we have to create it under the sandbox tmp
-          // directory.
+          // tmpfs mount points and writable dirs must exist, which is usually the user's
+          // responsibility. But if the user requests a path mount under /tmp, we have to create it
+          // under the sandbox tmp directory.
           sandboxTmp.getRelative(path.relativeTo(slashTmp)).createDirectoryAndParents();
         }
       }
     }
 
     SandboxOutputs outputs = helpers.getOutputs(spawn);
-    ImmutableMap<String, String> environment =
-        localEnvProvider.rewriteLocalEnv(spawn.getEnvironment(), binTools, "/tmp");
-    ImmutableSet<Path> writableDirs = getWritableDirs(sandboxExecRoot, environment);
     Duration timeout = context.getTimeout();
     SandboxOptions sandboxOptions = getSandboxOptions();
 
@@ -371,8 +375,7 @@ public String getName() {
   @Override
   protected ImmutableSet<Path> getWritableDirs(Path sandboxExecRoot, Map<String, String> env)
       throws IOException {
-    Set<Path> writableDirs = new TreeSet<>();
-    writableDirs.addAll(super.getWritableDirs(sandboxExecRoot, env));
+    Set<Path> writableDirs = new TreeSet<>(super.getWritableDirs(sandboxExecRoot, env));
     if (getSandboxOptions().memoryLimitMb > 0) {
       CgroupsInfo cgroupsInfo = CgroupsInfo.getInstance();
       writableDirs.add(fileSystem.getPath(cgroupsInfo.getMountPoint().getAbsolutePath()));

diff --git a/src/test/shell/integration/sandboxing_test.sh b/src/test/shell/integration/sandboxing_test.sh
@@ -961,6 +961,18 @@ EOF
         || fail "Expected build to succeed"
 }
 
+function test_hermetic_tmp_with_tmpdir_under_tmp() {
+  mkdir pkg
+  cat >pkg/BUILD <<EOF
+genrule(name = "pkg", outs = ["pkg.out"], cmd = "echo >\$@")
+EOF
+  mkdir /tmp/my_tmpdir
+  TMPDIR=/tmp/my_tmpdir \
+    bazel build --incompatible_sandbox_hermetic_tmp \
+     //pkg >"${TEST_log}" 2>&1 \
+        || fail "Expected build to succeed"
+}
+
 function test_runfiles_from_tests_get_reused_and_tmp_clean() {
   do_test_runfiles_from_tests_get_reused_and_tmp_clean \
     "--noexperimental_inmemory_sandbox_stashes"