Fix handling of relative symlinks within sandboxfs.

If an action expresses a symlink as an input, the target of the symlink does
not necessarily appear as a file to map within the sandbox.  This is a
problem when the target of the symlink is relative because sandboxfs would
expose the link verbatim and the target would be missing later on during
resolution.

To fix this, special-case the handling of symlinks: when trying to expose
them via a sandboxfs mount point, resolve their final target instead of
respecting the original contents.  This loses the fact that the file was a
symlink when running within the sandboxfs sandbox, but is easier to
implement and slightly faster at runtime.  We can reconsider this choice
if this causes problems.

RELNOTES: None.
PiperOrigin-RevId: 192325932

2 files changed, 44 insertions, 2 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/FakeSandboxfsProcess.java b/src/test/java/com/google/devtools/build/lib/sandbox/FakeSandboxfsProcess.java
index 8a9c63b288..ec3f713a16 100644
--- a/src/test/java/com/google/devtools/build/lib/sandbox/FakeSandboxfsProcess.java
+++ b/src/test/java/com/google/devtools/build/lib/sandbox/FakeSandboxfsProcess.java
@@ -90,13 +90,21 @@ final class FakeSandboxfsProcess implements SandboxfsProcess {
       Path link = fileSystem.getPath(mountPoint).getRelative(mapping.path().toRelative());
       link.getParentDirectory().createDirectoryAndParents();
 
-      if (!fileSystem.getPath(mapping.target()).exists()) {
+      Path target = fileSystem.getPath(mapping.target());
+      if (!target.exists()) {
         // Not a requirement for the creation of a symbolic link but this reflects the behavior of
         // the real sandboxfs.
         throw new IOException("Target " + mapping.target() + " does not exist");
       }
 
-      link.createSymbolicLink(fileSystem.getPath(mapping.target()));
+      if (target.isSymbolicLink()) {
+        // sandboxfs is able to expose symlinks as they are in the underlying file system.  Mimic
+        // this behavior by respecting the symlink in that case, instead of just creating a new
+        // symlink that points to the actual target.
+        link.createSymbolicLink(target.readSymbolicLink());
+      } else {
+        link.createSymbolicLink(fileSystem.getPath(mapping.target()));
+      }
     }
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/sandbox/SandboxfsSandboxedSpawnTest.java b/src/test/java/com/google/devtools/build/lib/sandbox/SandboxfsSandboxedSpawnTest.java
index e83520eb23..05d101175a 100644
--- a/src/test/java/com/google/devtools/build/lib/sandbox/SandboxfsSandboxedSpawnTest.java
+++ b/src/test/java/com/google/devtools/build/lib/sandbox/SandboxfsSandboxedSpawnTest.java
@@ -124,4 +124,38 @@ public class SandboxfsSandboxedSpawnTest extends SandboxTestCase {
 
     assertThat(outputsDir.getRelative(outputFile).isFile(Symlinks.NOFOLLOW)).isTrue();
   }
+
+  @Test
+  public void testSymlinksAreNotExposed() throws Exception {
+    Path helloTxt = workspaceDir.getRelative("dir1/hello.txt");
+    helloTxt.getParentDirectory().createDirectory();
+    FileSystemUtils.createEmptyFile(helloTxt);
+
+    Path linkToHello = workspaceDir.getRelative("dir2/link-to-hello");
+    linkToHello.getParentDirectory().createDirectory();
+    linkToHello.createSymbolicLink(PathFragment.create("../dir1/hello.txt"));
+
+    // Ensure that the symlink we have created has a relative target, as otherwise we wouldn't
+    // exercise the functionality we are trying to test.
+    assertThat(linkToHello.readSymbolicLink().isAbsolute()).isFalse();
+
+    SandboxedSpawn spawn =
+        new SandboxfsSandboxedSpawn(
+            sandboxfs,
+            outerDir,
+            ImmutableList.of("/bin/true"),
+            ImmutableMap.of(),
+            ImmutableMap.of(PathFragment.create("such/input.txt"), linkToHello),
+            ImmutableSet.of(PathFragment.create("very/output.txt")),
+            ImmutableSet.of());
+
+    spawn.createFileSystem();
+    Path execRoot = spawn.getSandboxExecRoot();
+
+    assertThat(execRoot.getRelative("such/input.txt").isSymbolicLink()).isTrue();
+    // We expect the target of the input file to be the final target of the input in use, not the
+    // intermediate symlink we specified. Otherwise, the exposed symlink in the sandbox would be
+    // broken because its relative target is not transitively exposed.
+    assertThat(execRoot.getRelative("such/input.txt").resolveSymbolicLinks()).isEqualTo(helloTxt);
+  }
 }