Permit Absolute Symlink Targets with configuration

Partial specification of the absolute symlink response per REAPI.
Remaining work will be in output identification.

_site/docs/configuration/configuration.md

@@ -28,13 +28,14 @@ For an example configuration containing all of the configuration values, see `ex
 
 ### Common
 
-| Configuration        | Accepted and _Default_ Values | Command Line Argument | Description                                       |
-|----------------------|-------------------------------|-----------------------|---------------------------------------------------|
-| digestFunction       | _SHA256_, SHA1                |                       | Digest function for this implementation           |
-| defaultActionTimeout | Integer, _600_                |                       | Default timeout value for an action (seconds)     |
-| maximumActionTimeout | Integer, _3600_               |                       | Maximum allowed action timeout (seconds)          |
-| maxEntrySizeBytes    | Long, _2147483648_            |                       | Maximum size of a single blob accepted (bytes)    |
-| prometheusPort       | Integer, _9090_               | --prometheus_port     | Listening port of the Prometheus metrics endpoint |
+| Configuration                | Accepted and _Default_ Values | Command Line Argument | Description                                                  |
+|------------------------------|-------------------------------|-----------------------|--------------------------------------------------------------|
+| digestFunction               | _SHA256_, SHA1                |                       | Digest function for this implementation                      |
+| defaultActionTimeout         | Integer, _600_                |                       | Default timeout value for an action (seconds)                |
+| maximumActionTimeout         | Integer, _3600_               |                       | Maximum allowed action timeout (seconds)                     |
+| maxEntrySizeBytes            | Long, _2147483648_            |                       | Maximum size of a single blob accepted (bytes)               |
+| prometheusPort               | Integer, _9090_               | --prometheus_port     | Listening port of the Prometheus metrics endpoint            |
+| allowSymlinkTargetAbsolute   | boolean, _false_              |                       | Permit inputs to contain symlinks with absolute path targets |
 
 Example:
 

examples/config.yml

@@ -3,6 +3,7 @@ defaultActionTimeout: 600
 maximumActionTimeout: 3600
 maxEntrySizeBytes: 2147483648 # 2 * 1024 * 1024 * 1024
 prometheusPort: 9090
+allowSymlinkTargetAbsolute: false
 server:
   instanceType: SHARD
   name: shard

src/main/java/build/buildfarm/common/config/BuildfarmConfigs.java

@@ -36,6 +36,7 @@ public final class BuildfarmConfigs {
   private long maximumActionTimeout = 3600;
   private long maxEntrySizeBytes = 2147483648L; // 2 * 1024 * 1024 * 1024
   private int prometheusPort = 9090;
+  private boolean allowSymlinkTargetAbsolute = false;
   private Server server = new Server();
   private Backplane backplane = new Backplane();
   private Worker worker = new Worker();

src/main/java/build/buildfarm/instance/server/AbstractServerInstance.java

@@ -178,6 +178,8 @@ public abstract class AbstractServerInstance implements Instance {
   public static final String ENVIRONMENT_VARIABLES_NOT_SORTED =
       "The `Command`'s `environment_variables` are not correctly sorted by `name`.";
 
+  public static final String SYMLINK_TARGET_ABSOLUTE = "A symlink target is absolute.";
+
   public static final String MISSING_ACTION = "The action was not found in the CAS.";
 
   public static final String MISSING_COMMAND = "The command was not found in the CAS.";
@@ -790,6 +792,7 @@ public static void validateActionInputDirectory(
       Stack<Digest> pathDigests,
       Set<Digest> visited,
       Map<Digest, Directory> directoriesIndex,
+      boolean allowSymlinkTargetAbsolute,
       Consumer<String> onInputFile,
       Consumer<String> onInputDirectory,
       Consumer<Digest> onInputDigest,
@@ -838,6 +841,14 @@ public static void validateActionInputDirectory(
             .setSubject("/" + directoryPath + ": " + lastSymlinkName + " > " + symlinkName)
             .setDescription(DIRECTORY_NOT_SORTED);
       }
+      String symlinkTarget = symlinkNode.getTarget();
+      if (!allowSymlinkTargetAbsolute && symlinkTarget.charAt(0) == '/') {
+        preconditionFailure
+            .addViolationsBuilder()
+            .setType(VIOLATION_TYPE_INVALID)
+            .setSubject("/" + directoryPath + ": " + symlinkName + " -> " + symlinkTarget)
+            .setDescription(SYMLINK_TARGET_ABSOLUTE);
+      }
       /* FIXME serverside validity check? regex?
       Preconditions.checkState(
           isValidFilename(symlinkName),
@@ -907,6 +918,7 @@ public static void validateActionInputDirectory(
               pathDigests,
               visited,
               directoriesIndex,
+              allowSymlinkTargetAbsolute,
               onInputFile,
               onInputDirectory,
               onInputDigest,
@@ -922,6 +934,7 @@ private static void validateActionInputDirectoryDigest(
       Stack<Digest> pathDigests,
       Set<Digest> visited,
       Map<Digest, Directory> directoriesIndex,
+      boolean allowSymlinkTargetAbsolute,
       Consumer<String> onInputFile,
       Consumer<String> onInputDirectory,
       Consumer<Digest> onInputDigest,
@@ -946,6 +959,7 @@ private static void validateActionInputDirectoryDigest(
           pathDigests,
           visited,
           directoriesIndex,
+          allowSymlinkTargetAbsolute,
           onInputFile,
           onInputDirectory,
           onInputDigest,
@@ -1203,12 +1217,16 @@ protected void validateAction(
     ImmutableSet.Builder<String> inputFilesBuilder = ImmutableSet.builder();
 
     inputDirectoriesBuilder.add(ACTION_INPUT_ROOT_DIRECTORY_PATH);
+    boolean allowSymlinkTargetAbsolute =
+        getCacheCapabilities().getSymlinkAbsolutePathStrategy()
+            == SymlinkAbsolutePathStrategy.Value.ALLOWED;
     validateActionInputDirectoryDigest(
         ACTION_INPUT_ROOT_DIRECTORY_PATH,
         action.getInputRootDigest(),
         new Stack<>(),
         new HashSet<>(),
         directoriesIndex,
+        allowSymlinkTargetAbsolute,
         inputFilesBuilder::add,
         inputDirectoriesBuilder::add,
         onInputDigest,

src/main/java/build/buildfarm/instance/shard/ShardInstance.java

@@ -47,6 +47,7 @@
 import build.bazel.remote.execution.v2.Action;
 import build.bazel.remote.execution.v2.ActionResult;
 import build.bazel.remote.execution.v2.BatchReadBlobsResponse.Response;
+import build.bazel.remote.execution.v2.CacheCapabilities;
 import build.bazel.remote.execution.v2.Command;
 import build.bazel.remote.execution.v2.Compressor;
 import build.bazel.remote.execution.v2.Digest;
@@ -60,6 +61,7 @@
 import build.bazel.remote.execution.v2.Platform.Property;
 import build.bazel.remote.execution.v2.RequestMetadata;
 import build.bazel.remote.execution.v2.ResultsCachePolicy;
+import build.bazel.remote.execution.v2.SymlinkAbsolutePathStrategy;
 import build.buildfarm.actioncache.ActionCache;
 import build.buildfarm.actioncache.ShardActionCache;
 import build.buildfarm.backplane.Backplane;
@@ -2726,4 +2728,16 @@ private boolean inDenyList(RequestMetadata requestMetadata) throws IOException {
     }
     return backplane.isBlacklisted(requestMetadata);
   }
+
+  @Override
+  protected CacheCapabilities getCacheCapabilities() {
+    SymlinkAbsolutePathStrategy.Value symlinkAbsolutePathStrategy =
+        configs.isAllowSymlinkTargetAbsolute()
+            ? SymlinkAbsolutePathStrategy.Value.ALLOWED
+            : SymlinkAbsolutePathStrategy.Value.DISALLOWED;
+    return super.getCacheCapabilities()
+        .toBuilder()
+        .setSymlinkAbsolutePathStrategy(symlinkAbsolutePathStrategy)
+        .build();
+  }
 }

src/main/java/build/buildfarm/worker/shard/CFCExecFileSystem.java

@@ -82,6 +82,9 @@ class CFCExecFileSystem implements ExecFileSystem {
   // indicate symlinking above for a set of matching paths
   private final Iterable<Pattern> linkedInputDirectories;
 
+  // permit symlinks to point to absolute paths in inputs
+  private final boolean allowSymlinkTargetAbsolute;
+
   private final Map<Path, Iterable<String>> rootInputFiles = new ConcurrentHashMap<>();
   private final Map<Path, Iterable<Digest>> rootInputDirectories = new ConcurrentHashMap<>();
   private final ExecutorService fetchService = BuildfarmExecutors.getFetchServicePool();
@@ -95,6 +98,7 @@ class CFCExecFileSystem implements ExecFileSystem {
       @Nullable UserPrincipal owner,
       boolean linkInputDirectories,
       Iterable<String> linkedInputDirectories,
+      boolean allowSymlinkTargetAbsolute,
       ExecutorService removeDirectoryService,
       ExecutorService accessRecorder) {
     this.root = root;
@@ -104,6 +108,7 @@ class CFCExecFileSystem implements ExecFileSystem {
     this.linkedInputDirectories =
         Iterables.transform(
             linkedInputDirectories, realInputDirectory -> Pattern.compile(realInputDirectory));
+    this.allowSymlinkTargetAbsolute = allowSymlinkTargetAbsolute;
     this.removeDirectoryService = removeDirectoryService;
     this.accessRecorder = accessRecorder;
   }
@@ -179,7 +184,7 @@ public InputStream newInput(Compressor.Value compressor, Digest digest, long off
   private ListenableFuture<Void> putSymlink(Path path, SymlinkNode symlinkNode) {
     Path symlinkPath = path.resolve(symlinkNode.getName());
     Path relativeTargetPath = path.getFileSystem().getPath(symlinkNode.getTarget());
-    checkState(!relativeTargetPath.isAbsolute());
+    checkState(allowSymlinkTargetAbsolute || !relativeTargetPath.isAbsolute());
     return listeningDecorator(fetchService)
         .submit(
             () -> {

src/main/java/build/buildfarm/worker/shard/Worker.java

@@ -337,6 +337,7 @@ private ExecFileSystem createCFCExecFileSystem(
         owner,
         configs.getWorker().isLinkInputDirectories(),
         configs.getWorker().getLinkedInputDirectories(),
+        configs.isAllowSymlinkTargetAbsolute(),
         removeDirectoryService,
         accessRecorder
         /* deadlineAfter=*/

src/test/java/build/buildfarm/instance/server/AbstractServerInstanceTest.java

@@ -23,6 +23,7 @@
 import static build.buildfarm.instance.server.AbstractServerInstance.INVALID_COMMAND;
 import static build.buildfarm.instance.server.AbstractServerInstance.OUTPUT_DIRECTORY_IS_OUTPUT_ANCESTOR;
 import static build.buildfarm.instance.server.AbstractServerInstance.OUTPUT_FILE_IS_OUTPUT_ANCESTOR;
+import static build.buildfarm.instance.server.AbstractServerInstance.SYMLINK_TARGET_ABSOLUTE;
 import static com.google.common.truth.Truth.assertThat;
 import static com.google.common.util.concurrent.Futures.immediateFuture;
 import static org.mockito.Mockito.any;
@@ -43,6 +44,7 @@
 import build.bazel.remote.execution.v2.OutputDirectory;
 import build.bazel.remote.execution.v2.Platform;
 import build.bazel.remote.execution.v2.RequestMetadata;
+import build.bazel.remote.execution.v2.SymlinkNode;
 import build.bazel.remote.execution.v2.Tree;
 import build.buildfarm.actioncache.ActionCache;
 import build.buildfarm.cas.ContentAddressableStorage;
@@ -270,6 +272,7 @@ public void duplicateFileInputIsInvalid() {
         /* pathDigests=*/ new Stack<>(),
         /* visited=*/ Sets.newHashSet(),
         /* directoriesIndex=*/ Maps.newHashMap(),
+        /* allowSymlinkTargetAbsolute=*/ false,
         /* onInputFile=*/ file -> {},
         /* onInputDirectorie=*/ directory -> {},
         /* onInputDigest=*/ digest -> {},
@@ -304,6 +307,7 @@ public void duplicateEmptyDirectoryCheckPasses() throws StatusException {
         /* pathDigests=*/ new Stack<>(),
         /* visited=*/ Sets.newHashSet(),
         /* directoriesIndex=*/ ImmutableMap.of(Digest.getDefaultInstance(), emptyDirectory),
+        /* allowSymlinkTargetAbsolute=*/ false,
         /* onInputFiles=*/ file -> {},
         /* onInputDirectories=*/ directory -> {},
         /* onInputDigests=*/ digest -> {},
@@ -327,6 +331,7 @@ public void unsortedFileInputIsInvalid() {
         /* pathDigests=*/ new Stack<>(),
         /* visited=*/ Sets.newHashSet(),
         /* directoriesIndex=*/ Maps.newHashMap(),
+        /* allowSymlinkTargetAbsolute=*/ false,
         /* onInputFiles=*/ file -> {},
         /* onInputDirectories=*/ directory -> {},
         /* onInputDigests=*/ digest -> {},
@@ -361,6 +366,7 @@ public void duplicateDirectoryInputIsInvalid() {
         /* pathDigests=*/ new Stack<>(),
         /* visited=*/ Sets.newHashSet(),
         /* directoriesIndex=*/ ImmutableMap.of(emptyDirectoryDigest, emptyDirectory),
+        /* allowSymlinkTargetAbsolute=*/ false,
         /* onInputFiles=*/ file -> {},
         /* onInputDirectories=*/ directory -> {},
         /* onInputDigests=*/ digest -> {},
@@ -395,6 +401,7 @@ public void unsortedDirectoryInputIsInvalid() {
         /* pathDigests=*/ new Stack<>(),
         /* visited=*/ Sets.newHashSet(),
         /* directoriesIndex=*/ ImmutableMap.of(emptyDirectoryDigest, emptyDirectory),
+        /* allowSymlinkTargetAbsolute=*/ false,
         /* onInputFiles=*/ file -> {},
         /* onInputDirectories=*/ directory -> {},
         /* onInputDigests=*/ digest -> {},
@@ -407,6 +414,48 @@ public void unsortedDirectoryInputIsInvalid() {
     assertThat(violation.getDescription()).isEqualTo(DIRECTORY_NOT_SORTED);
   }
 
+  @Test
+  public void shouldValidateIfSymlinkTargetAbsolute() {
+    // invalid for disallowed
+    PreconditionFailure.Builder preconditionFailure = PreconditionFailure.newBuilder();
+    Directory absoluteSymlinkDirectory =
+        Directory.newBuilder()
+            .addSymlinks(SymlinkNode.newBuilder().setName("foo").setTarget("/root/secret").build())
+            .build();
+    AbstractServerInstance.validateActionInputDirectory(
+        ACTION_INPUT_ROOT_DIRECTORY_PATH,
+        absoluteSymlinkDirectory,
+        /* pathDigests=*/ new Stack<>(),
+        /* visited=*/ Sets.newHashSet(),
+        /* directoriesIndex=*/ Maps.newHashMap(),
+        /* allowSymlinkTargetAbsolute=*/ false,
+        /* onInputFile=*/ file -> {},
+        /* onInputDirectorie=*/ directory -> {},
+        /* onInputDigest=*/ digest -> {},
+        preconditionFailure);
+
+    assertThat(preconditionFailure.getViolationsCount()).isEqualTo(1);
+    Violation violation = preconditionFailure.getViolationsList().get(0);
+    assertThat(violation.getType()).isEqualTo(VIOLATION_TYPE_INVALID);
+    assertThat(violation.getSubject()).isEqualTo("/: foo -> /root/secret");
+    assertThat(violation.getDescription()).isEqualTo(SYMLINK_TARGET_ABSOLUTE);
+
+    // valid for allowed
+    preconditionFailure = PreconditionFailure.newBuilder();
+    AbstractServerInstance.validateActionInputDirectory(
+        ACTION_INPUT_ROOT_DIRECTORY_PATH,
+        absoluteSymlinkDirectory,
+        /* pathDigests=*/ new Stack<>(),
+        /* visited=*/ Sets.newHashSet(),
+        /* directoriesIndex=*/ Maps.newHashMap(),
+        /* allowSymlinkTargetAbsolute=*/ true,
+        /* onInputFile=*/ file -> {},
+        /* onInputDirectorie=*/ directory -> {},
+        /* onInputDigest=*/ digest -> {},
+        preconditionFailure);
+    assertThat(preconditionFailure.getViolationsCount()).isEqualTo(0);
+  }
+
   @Test
   public void nestedOutputDirectoriesAreInvalid() {
     PreconditionFailure.Builder preconditionFailureBuilder = PreconditionFailure.newBuilder();
@@ -547,6 +596,7 @@ public void multipleIdenticalDirectoryMissingAreAllPreconditionFailures() {
         /* pathDigests=*/ new Stack<>(),
         /* visited=*/ Sets.newHashSet(),
         /* directoriesIndex=*/ ImmutableMap.of(),
+        /* allowSymlinkTargetAbsolute=*/ false,
         /* onInputFiles=*/ file -> {},
         /* onInputDirectories=*/ directory -> {},
         /* onInputDigests=*/ digest -> {},
@@ -608,6 +658,7 @@ public void validationRevisitReplicatesPreconditionFailures() {
         /* pathDigests=*/ new Stack<>(),
         /* visited=*/ Sets.newHashSet(),
         /* directoriesIndex=*/ ImmutableMap.of(fooDigest, foo),
+        /* allowSymlinkTargetAbsolute=*/ false,
         /* onInputFiles=*/ file -> {},
         /* onInputDirectories=*/ directory -> {},
         /* onInputDigests=*/ digest -> {},