Changes to Fileset traversal code for internal purposes
PiperOrigin-RevId: 628051123
Change-Id: I35697db03e13fa866801ef81f36aaa8b2caab306
diff --git a/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParams.java b/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParams.java
index e214868..c14aa7f 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParams.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParams.java
@@ -182,6 +182,9 @@
/** Returns whether Filesets treat outputs in a strict manner, assuming regular files. */
public abstract boolean isStrictFilesetOutput();
+ /** Returns whether directories are permitted and can be traversed. */
+ public abstract boolean permitDirectories();
+
@Memoized
@Override
public abstract int hashCode();
@@ -193,6 +196,7 @@
fp.addBoolean(isFollowingSymlinks());
fp.addBoolean(isGenerated());
fp.addBoolean(isStrictFilesetOutput());
+ fp.addBoolean(permitDirectories());
getPackageBoundaryMode().fingerprint(fp);
return fp.digestAndReset();
}
@@ -202,9 +206,15 @@
boolean followingSymlinks,
PackageBoundaryMode packageBoundaryMode,
boolean isStrictFilesetOutput,
+ boolean permitSourceDirectories,
boolean isGenerated) {
return new AutoValue_FilesetTraversalParams_DirectTraversal(
- root, isGenerated, followingSymlinks, packageBoundaryMode, isStrictFilesetOutput);
+ root,
+ isGenerated,
+ followingSymlinks,
+ packageBoundaryMode,
+ isStrictFilesetOutput,
+ permitSourceDirectories);
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParamsFactory.java b/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParamsFactory.java
index afe9d74..ea14899 100644
--- a/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParamsFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/actions/FilesetTraversalParamsFactory.java
@@ -63,9 +63,14 @@
* @param pkgBoundaryMode what to do when the traversal hits a subdirectory that is also a
* @param strictFilesetOutput whether Fileset assumes that output Artifacts are regular files.
*/
- public static FilesetTraversalParams fileTraversal(Label ownerLabel, Artifact fileToTraverse,
- PathFragment destPath, SymlinkBehavior symlinkBehaviorMode,
- PackageBoundaryMode pkgBoundaryMode, boolean strictFilesetOutput) {
+ public static FilesetTraversalParams fileTraversal(
+ Label ownerLabel,
+ Artifact fileToTraverse,
+ PathFragment destPath,
+ SymlinkBehavior symlinkBehaviorMode,
+ PackageBoundaryMode pkgBoundaryMode,
+ boolean strictFilesetOutput,
+ boolean permitDirectories) {
return DirectoryTraversalParams.getDirectoryTraversalParams(
ownerLabel,
DirectTraversalRoot.forFileOrDirectory(fileToTraverse),
@@ -74,6 +79,7 @@
symlinkBehaviorMode,
pkgBoundaryMode,
strictFilesetOutput,
+ permitDirectories,
!fileToTraverse.isSourceArtifact());
}
@@ -138,6 +144,7 @@
SymlinkBehavior symlinkBehaviorMode,
PackageBoundaryMode pkgBoundaryMode,
boolean strictFilesetOutput,
+ boolean permitDirectories,
boolean isGenerated) {
DirectTraversal traversal =
DirectTraversal.getDirectTraversal(
@@ -145,6 +152,7 @@
symlinkBehaviorMode == SymlinkBehavior.DEREFERENCE,
pkgBoundaryMode,
strictFilesetOutput,
+ permitDirectories,
isGenerated);
return new AutoValue_FilesetTraversalParamsFactory_DirectoryTraversalParams(
ownerLabelForErrorMessages, destPath, getOrderedExcludes(excludes), traversal);
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java
index ecdbbd9..ebb565a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunction.java
@@ -83,6 +83,18 @@
return FilesetEntryValue.EMPTY;
}
+ // Check if directory traversal is permitted
+ if (resolvedRoot.getType().isDirectory() && !params.getDirectTraversal().permitDirectories()) {
+ throw new FilesetEntryFunctionException(
+ new RecursiveFilesystemTraversalException(
+ String.format(
+ "%s contains a directory artifact '%s' and is restricted by %s",
+ params.getOwnerLabelForErrorMessages(),
+ params.getDestPath(),
+ "tools/allowlists/fileset_dir_in_files_allowlist"),
+ RecursiveFilesystemTraversalException.Type.FILE_OPERATION_FAILURE));
+ }
+
// The "direct" traversal params are present, which is the case when the FilesetEntry
// specifies a package's BUILD file, a directory or a list of files.
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java
index bf763af..5375433 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java
@@ -234,12 +234,13 @@
Artifact file = createSourceArtifact("foo/file.real");
FilesetTraversalParams params =
FilesetTraversalParamsFactory.fileTraversal(
- /*ownerLabel=*/ label("//foo"),
- /*fileToTraverse=*/ file,
+ /* ownerLabel= */ label("//foo"),
+ /* fileToTraverse= */ file,
PathFragment.create("output-name"),
- /*symlinkBehaviorMode=*/ FilesetTraversalParamsFactory.SymlinkBehavior.COPY,
- /*pkgBoundaryMode=*/ DONT_CROSS,
- /*strictFilesetOutput=*/ false);
+ /* symlinkBehaviorMode= */ FilesetTraversalParamsFactory.SymlinkBehavior.COPY,
+ /* pkgBoundaryMode= */ DONT_CROSS,
+ /* strictFilesetOutput= */ false,
+ /* permitDirectories= */ false);
assertSymlinksCreatedInOrder(params, symlink("output-name", file));
}
@@ -250,12 +251,13 @@
FilesetTraversalParams params =
FilesetTraversalParamsFactory.fileTraversal(
- /*ownerLabel=*/ label("//foo"),
- /*fileToTraverse=*/ symlink,
+ /* ownerLabel= */ label("//foo"),
+ /* fileToTraverse= */ symlink,
PathFragment.create("output-name"),
- /*symlinkBehaviorMode=*/ symlinks,
- /*pkgBoundaryMode=*/ DONT_CROSS,
- /*strictFilesetOutput=*/ false);
+ /* symlinkBehaviorMode= */ symlinks,
+ /* pkgBoundaryMode= */ DONT_CROSS,
+ /* strictFilesetOutput= */ false,
+ /* permitDirectories= */ false);
switch (symlinks) {
case COPY:
assertSymlinksCreatedInOrder(params, symlink("output-name", "file.real"));
@@ -286,16 +288,41 @@
FilesetTraversalParams params =
FilesetTraversalParamsFactory.fileTraversal(
- /*ownerLabel=*/ label("//foo"),
- /*fileToTraverse=*/ dir,
+ /* ownerLabel= */ label("//foo"),
+ /* fileToTraverse= */ dir,
PathFragment.create("output-name"),
- /*symlinkBehaviorMode=*/ FilesetTraversalParamsFactory.SymlinkBehavior.COPY,
- /*pkgBoundaryMode=*/ DONT_CROSS,
- /*strictFilesetOutput*/ false);
+ /* symlinkBehaviorMode= */ FilesetTraversalParamsFactory.SymlinkBehavior.COPY,
+ /* pkgBoundaryMode= */ DONT_CROSS,
+ /* strictFilesetOutput= */ false,
+ /* permitDirectories= */ true);
assertSymlinksCreatedInOrder(
params, symlink("output-name/file.a", fileA), symlink("output-name/sub/file.b", fileB));
}
+ @Test
+ public void testFileTraversalForDisallowedDirectoryThrows() throws Exception {
+ Artifact dir = getSourceArtifact("foo/dir_real");
+ createFile(childOf(dir, "file.a"), "hello");
+ createFile(childOf(dir, "sub/file.b"), "world");
+
+ FilesetTraversalParams params =
+ FilesetTraversalParamsFactory.fileTraversal(
+ /* ownerLabel= */ label("//foo"),
+ /* fileToTraverse= */ dir,
+ PathFragment.create("output-name"),
+ /* symlinkBehaviorMode= */ FilesetTraversalParamsFactory.SymlinkBehavior.COPY,
+ /* pkgBoundaryMode= */ DONT_CROSS,
+ /* strictFilesetOutput= */ false,
+ /* permitDirectories= */ false);
+
+ SkyKey key = FilesetEntryKey.key(params);
+ EvaluationResult<FilesetEntryValue> result = eval(key);
+ assertThat(result.hasError()).isTrue();
+ assertThat(result.getError(key).getException())
+ .hasMessageThat()
+ .contains("foo contains a directory");
+ }
+
private void assertFileTraversalForDirectorySymlink(SymlinkBehavior symlinks) throws Exception {
Artifact dir = getSourceArtifact("foo/dir_real");
Artifact symlink = getSourceArtifact("foo/dir_sym");
@@ -305,12 +332,13 @@
FilesetTraversalParams params =
FilesetTraversalParamsFactory.fileTraversal(
- /*ownerLabel=*/ label("//foo"),
- /*fileToTraverse=*/ symlink,
+ /* ownerLabel= */ label("//foo"),
+ /* fileToTraverse= */ symlink,
PathFragment.create("output-name"),
- /*symlinkBehaviorMode=*/ symlinks,
- /*pkgBoundaryMode=*/ DONT_CROSS,
- /*strictFilesetOutput*/ false);
+ /* symlinkBehaviorMode= */ symlinks,
+ /* pkgBoundaryMode= */ DONT_CROSS,
+ /* strictFilesetOutput= */ false,
+ /* permitDirectories= */ true);
switch (symlinks) {
case COPY:
assertSymlinksCreatedInOrder(params, symlink("output-name", "dir_real"));
@@ -343,12 +371,13 @@
FilesetTraversalParams params =
FilesetTraversalParamsFactory.fileTraversal(
- /*ownerLabel=*/ label("//foo"),
- /*fileToTraverse=*/ linkName,
+ /* ownerLabel= */ label("//foo"),
+ /* fileToTraverse= */ linkName,
PathFragment.create("output-name"),
- /*symlinkBehaviorMode=*/ FilesetTraversalParamsFactory.SymlinkBehavior.COPY,
- /*pkgBoundaryMode=*/ DONT_CROSS,
- /*strictFilesetOutput=*/ false);
+ /* symlinkBehaviorMode= */ FilesetTraversalParamsFactory.SymlinkBehavior.COPY,
+ /* pkgBoundaryMode= */ DONT_CROSS,
+ /* strictFilesetOutput= */ false,
+ /* permitDirectories= */ false);
assertSymlinksCreatedInOrder(params); // expect empty results
}
@@ -357,12 +386,13 @@
Artifact path = getSourceArtifact("foo/non-existent");
FilesetTraversalParams params =
FilesetTraversalParamsFactory.fileTraversal(
- /*ownerLabel=*/ label("//foo"),
- /*fileToTraverse=*/ path,
+ /* ownerLabel= */ label("//foo"),
+ /* fileToTraverse= */ path,
PathFragment.create("output-name"),
- /*symlinkBehaviorMode=*/ FilesetTraversalParamsFactory.SymlinkBehavior.COPY,
- /*pkgBoundaryMode=*/ DONT_CROSS,
- /*strictFilesetOutput=*/ false);
+ /* symlinkBehaviorMode= */ FilesetTraversalParamsFactory.SymlinkBehavior.COPY,
+ /* pkgBoundaryMode= */ DONT_CROSS,
+ /* strictFilesetOutput= */ false,
+ /* permitDirectories= */ false);
assertSymlinksCreatedInOrder(params); // expect empty results
}
@@ -469,6 +499,7 @@
FilesetTraversalParamsFactory.SymlinkBehavior.DEREFERENCE))
.put("pkgBoundaryMode", partOfFingerprint(CROSS, DONT_CROSS))
.put("strictFilesetOutput", partOfFingerprint(true, false))
+ .put("permitDirectories", partOfFingerprint(true, false))
.buildOrThrow()) {
@Override
FilesetTraversalParams create(Map<String, ?> kwArgs) throws Exception {
@@ -478,7 +509,8 @@
PathFragment.create((String) kwArgs.get("destPath")),
((SymlinkBehavior) kwArgs.get("symlinkBehaviorMode")),
(PackageBoundaryMode) kwArgs.get("pkgBoundaryMode"),
- (Boolean) kwArgs.get("strictFilesetOutput"));
+ (Boolean) kwArgs.get("strictFilesetOutput"),
+ (Boolean) kwArgs.get("permitDirectories"));
}
}.doTest();
}
