diff options
3 files changed, 62 insertions, 4 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxfsSandboxedSpawn.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxfsSandboxedSpawn.java index a33785f648..09b63f0054 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxfsSandboxedSpawn.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxfsSandboxedSpawn.java @@ -165,8 +165,9 @@ class SandboxfsSandboxedSpawn implements SandboxedSpawn { * as a map of mapped path to target path. The target path may be null, in which case an empty * read-only file is mapped. * @return the collection of mappings to use for reconfiguration + * @throws IOException if we fail to resolve symbolic links */ - private List<Mapping> createMappings(Map<PathFragment, Path> inputs) { + private List<Mapping> createMappings(Map<PathFragment, Path> inputs) throws IOException { List<Mapping> mappings = new ArrayList<>(); mappings.add(Mapping.builder() @@ -176,9 +177,24 @@ class SandboxfsSandboxedSpawn implements SandboxedSpawn { .build()); for (Entry<PathFragment, Path> entry : inputs.entrySet()) { + PathFragment target; + if (entry.getValue() == null) { + target = EMPTY_FILE; + } else if (entry.getValue().isSymbolicLink()) { + // If an input is a symlink, we don't necessarily have its target as an input as well. To + // ensure the target is reachable within the sandbox, we have two choices: we can either + // expose the target in the sandbox and respect the symlink, or we can resolve what the + // actual target is and point the mapping there. The former has higher fidelity, as the + // sandbox will respect the file's type as a symlink. The latter is easier to implement + // and is slightly faster, as we avoid having to resolve symlinks later via sandboxfs. + // Therefore, do the latter until proven insufficient. + target = entry.getValue().resolveSymbolicLinks().asFragment(); + } else { + target = entry.getValue().asFragment(); + } mappings.add(Mapping.builder() .setPath(innerExecRoot.getRelative(entry.getKey())) - .setTarget(entry.getValue() == null ? EMPTY_FILE : entry.getValue().asFragment()) + .setTarget(target) .setWritable(false) .build()); } 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); + } } |