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 index 573064244445f7..0bdec0fc8716bb 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedSpawnRunner.java @@ -66,6 +66,8 @@ final class LinuxSandboxedSpawnRunner extends AbstractSandboxSpawnRunner { private static final Map isSupportedMap = new HashMap<>(); private static final AtomicBoolean warnedAboutNonHermeticTmp = new AtomicBoolean(); + private static final AtomicBoolean warnedAboutUnsupportedModificationCheck = new AtomicBoolean(); + /** * Returns whether the linux sandbox is supported on the local machine by running a small command * in it. @@ -216,9 +218,7 @@ protected SandboxedSpawn prepareSpawn(Spawn spawn, SpawnExecutionContext context ImmutableSet writableDirs = getWritableDirs(sandboxExecRoot, environment); SandboxInputs inputs = - helpers.processInputFiles( - context.getInputMapping(PathFragment.EMPTY_FRAGMENT), - execRoot); + helpers.processInputFiles(context.getInputMapping(PathFragment.EMPTY_FRAGMENT), execRoot); SandboxOutputs outputs = helpers.getOutputs(spawn); Duration timeout = context.getTimeout(); @@ -439,22 +439,39 @@ private void checkForConcurrentModifications(SpawnExecutionContext context) throws IOException, ForbiddenActionInputException { for (ActionInput input : context.getInputMapping(PathFragment.EMPTY_FRAGMENT).values()) { if (input instanceof VirtualActionInput) { + // Virtual inputs are not existing in file system and can't be tampered with via sandbox. No + // need to check them. continue; } FileArtifactValue metadata = context.getMetadataProvider().getMetadata(input); - Path path = execRoot.getRelative(input.getExecPath()); + if (!metadata.getType().isFile()) { + // The hermetic sandbox creates hardlinks from files inside sandbox to files outside + // sandbox. The content of the files outside the sandbox could have been tampered with via + // the hardlinks. Therefore files are checked for modifications. But directories and + // unresolved symlinks are not represented as hardlinks in sandbox and don't need to be + // checked. By continue and not checking them, we avoid UnsupportedOperationException and + // IllegalStateException. + continue; + } + Path path = execRoot.getRelative(input.getExecPath()); try { if (wasModifiedSinceDigest(metadata.getContentsProxy(), path)) { throw new IOException("input dependency " + path + " was modified during execution."); } } catch (UnsupportedOperationException e) { - throw new IOException( - "input dependency " - + path - + " could not be checked for modifications during execution.", - e); + // Some FileArtifactValue implementations are ignored safely and silently already by the + // isFile check above. The remaining ones should probably be checked, but some are not + // supporting necessary operations. + if (warnedAboutUnsupportedModificationCheck.compareAndSet(false, true)) { + reporter.handle( + Event.warn( + String.format( + "Input dependency %s of type %s could not be checked for modifications during" + + " execution. Suppressing similar warnings.", + path, metadata.getClass().getSimpleName()))); + } } } } diff --git a/src/test/shell/bazel/bazel_hermetic_sandboxing_test.sh b/src/test/shell/bazel/bazel_hermetic_sandboxing_test.sh index a67f4be7421ebb..51fdf239f25ce7 100755 --- a/src/test/shell/bazel/bazel_hermetic_sandboxing_test.sh +++ b/src/test/shell/bazel/bazel_hermetic_sandboxing_test.sh @@ -95,6 +95,37 @@ import import_module EOF cat << 'EOF' > examples/hermetic/BUILD + +load( + "test.bzl", + "overwrite_via_symlink", + "overwrite_file_from_declared_directory", + "subdirectories_in_declared_directory", + "other_artifacts", +) + +overwrite_via_symlink( + name = "overwrite_via_resolved_symlink", + resolve_symlink = True +) + +overwrite_via_symlink( + name = "overwrite_via_unresolved_symlink", + resolve_symlink = False +) + +overwrite_file_from_declared_directory( + name = "overwrite_file_from_declared_directory" +) + +subdirectories_in_declared_directory( + name = "subdirectories_in_declared_directory" +) + +other_artifacts( + name = "other_artifacts" +) + genrule( name = "absolute_path", srcs = ["script_absolute_path.sh"], # unknown_file.txt not referenced. @@ -129,6 +160,141 @@ genrule( (echo overwrite text > $(location :input_file)) && \ (echo success > $@)) || (echo fail > $@)", ) +EOF + + cat << 'EOF' > examples/hermetic/test.bzl + +def _overwrite_via_symlink_impl(ctx): + file = ctx.actions.declare_file(ctx.attr.name + ".file") + if ctx.attr.resolve_symlink: + symlink = ctx.actions.declare_file(ctx.attr.name + ".symlink") + else: + symlink = ctx.actions.declare_symlink(ctx.attr.name + ".symlink") + + ctx.actions.write(file, "") + + if ctx.attr.resolve_symlink: + ctx.actions.symlink( + output = symlink, + target_file = file + ) + # Symlink become resolved to RegularFileArtifactValue. + needed_inputs = [symlink] + else: + ctx.actions.symlink( + output = symlink, + target_path = file.basename + ) + # Symlink become UnresolvedSymlinkArtifactValue and would be + # dangling unless also providing the actual file as input to sandbox. + needed_inputs = [symlink, file] + + result_file = ctx.actions.declare_file(ctx.attr.name + ".result") + + # Try invalid write to the input file via the symlink + ctx.actions.run_shell( + command = "chmod u+w $1 && echo hello >> $1 && ls -lR > $2", + arguments = [symlink.path, result_file.path], + inputs = needed_inputs, + outputs = [result_file], + ) + + return [DefaultInfo(files = depset([result_file]))] + +overwrite_via_symlink = rule( + attrs = { + "resolve_symlink" : attr.bool(), + }, + implementation = _overwrite_via_symlink_impl, +) + + +def _overwrite_file_from_declared_directory_impl(ctx): + dir = ctx.actions.declare_directory(ctx.attr.name + ".dir") + + ctx.actions.run_shell( + command = "mkdir -p $1/subdir && touch $1/subdir/file", + arguments = [dir.path], + outputs = [dir], + ) + + # Try invalid write to input file, with file as implicit input + # from declared directory. + result_file = ctx.actions.declare_file(ctx.attr.name + ".result") + ctx.actions.run_shell( + command = "chmod -R u+w $1 && echo hello >> $1/subdir/file && touch $2", + arguments = [dir.path, result_file.path], + inputs = [dir], + outputs = [result_file], + ) + + return [DefaultInfo(files = depset([result_file]))] + +overwrite_file_from_declared_directory = rule( + implementation = _overwrite_file_from_declared_directory_impl, +) + + +def _subdirectories_in_declared_directory_impl(ctx): + dir = ctx.actions.declare_directory(ctx.attr.name + ".dir") + + ctx.actions.run_shell( + command = "mkdir -p %s/subdir1/subdir2" % dir.path, + outputs = [dir], + ) + + result_file = ctx.actions.declare_file(ctx.attr.name + ".result") + ctx.actions.run_shell( + command = "ls -lRH %s > %s" % (dir.path, result_file.path), + inputs = [dir], + outputs = [result_file], + ) + + return [DefaultInfo(files = depset([result_file]))] + +subdirectories_in_declared_directory = rule( + implementation = _subdirectories_in_declared_directory_impl, +) + + +def _other_artifacts_impl(ctx): + + # Produce artifacts of other types + + regular_file_artifact = ctx.actions.declare_file(ctx.attr.name + ".regular_file_artifact") + directory_artifact = ctx.actions.declare_file(ctx.attr.name + ".directory_artifact") + tree_artifact = ctx.actions.declare_directory(ctx.attr.name + ".tree_artifact") + unresolved_symlink_artifact = ctx.actions.declare_symlink(ctx.attr.name + ".unresolved_symlink_artifact") + + ctx.actions.run_shell( + command = "touch %s && mkdir %s" % (regular_file_artifact.path, directory_artifact.path), + outputs = [regular_file_artifact, tree_artifact, directory_artifact], + ) + + ctx.actions.symlink( + output = unresolved_symlink_artifact, + target_path="dangling" + ) + + # Test other artifact types as input to hermetic sandbox. + + all_artifacts = [regular_file_artifact, + directory_artifact, + tree_artifact, + unresolved_symlink_artifact] + input_paths_string = " ".join([a.path for a in all_artifacts]) + result_file = ctx.actions.declare_file(ctx.attr.name + ".result") + ctx.actions.run_shell( + command = "ls -lR %s > %s" % (input_paths_string, result_file.path), + inputs = all_artifacts, + outputs = [result_file], + ) + + return [DefaultInfo(files = depset([result_file]))] + +other_artifacts = rule( + implementation = _other_artifacts_impl, +) EOF } @@ -141,8 +307,6 @@ function test_absolute_path() { # Test that the build can't escape the sandbox by resolving symbolic link. function test_symbolic_link() { - [ "$PLATFORM" != "darwin" ] || return 0 - bazel build examples/hermetic:symbolic_link &> $TEST_log \ && fail "Fail due to non hermetic sandbox: examples/hermetic:symbolic_link" || true expect_log "cat: \/execroot\/main\/examples\/hermetic\/unknown_file.txt: No such file or directory" @@ -150,8 +314,6 @@ function test_symbolic_link() { # Test that the sandbox discover if the bazel python rule miss dependencies. function test_missing_python_deps() { - [ "$PLATFORM" != "darwin" ] || return 0 - bazel test examples/hermetic:py_module_test --test_output=all &> $TEST_TMPDIR/log \ && fail "Fail due to non hermetic sandbox: examples/hermetic:py_module_test" || true @@ -160,7 +322,6 @@ function test_missing_python_deps() { # Test that the intermediate corrupt input file gets re:evaluated function test_writing_input_file() { - [ "$PLATFORM" != "darwin" ] || return 0 # Write an input file, this should cause the hermetic sandbox to fail with an exception bazel build examples/hermetic:write_input_test &> $TEST_log \ && fail "Fail due to non hermetic sandbox: examples/hermetic:write_input_test" || true @@ -177,7 +338,48 @@ function test_writing_input_file() { expect_log "original text input" } +# Test that invalid write of input file is detected, when file is accessed via resolved symlink. +function test_overwrite_via_resolved_symlink() { + bazel build examples/hermetic:overwrite_via_resolved_symlink &> $TEST_log \ + && fail "Hermetic sandbox did not detect invalid write to input file" + expect_log "input dependency .* was modified during execution." +} + +# Test that invalid write of input file is detected, when file is accessed via unresolved symlink. +function test_overwrite_via_unresolved_symlink() { + bazel build examples/hermetic:overwrite_via_unresolved_symlink &> $TEST_log \ + && fail "Hermetic sandbox did not detect invalid write to input file" + expect_log "input dependency .* was modified during execution." +} + +# Test that invalid write of input file is detected, when file is found implicit via declared directory. +function test_overwrite_file_from_declared_directory() { + bazel build examples/hermetic:overwrite_file_from_declared_directory &> $TEST_log \ + && fail "Hermetic sandbox did not detect invalid write to input file" + expect_log "input dependency .* was modified during execution." +} + +# Test that the sandbox can handle deep directory trees from declared directory. +function test_subdirectories_in_declared_directory() { + bazel build examples/hermetic:subdirectories_in_declared_directory &> $TEST_log + cat bazel-bin/examples/hermetic/subdirectories_in_declared_directory.result + assert_contains "dir/subdir1/subdir2" "bazel-bin/examples/hermetic/subdirectories_in_declared_directory.result" +} + +# Test that the sandbox is not crashing and not producing warnings for various types of artifacts. +# Regression test for Issue #15340 +function test_other_artifacts() { + bazel shutdown # Clear memory about duplicated warnings + bazel build examples/hermetic:other_artifacts &> $TEST_log + expect_not_log "WARNING" + assert_contains "regular_file_artifact" "bazel-bin/examples/hermetic/other_artifacts.result" + assert_contains "unresolved_symlink_artifact" "bazel-bin/examples/hermetic/other_artifacts.result" + assert_contains "directory_artifact" "bazel-bin/examples/hermetic/other_artifacts.result" + assert_contains "tree_artifact" "bazel-bin/examples/hermetic/other_artifacts.result" +} + # The test shouldn't fail if the environment doesn't support running it. check_sandbox_allowed || exit 0 +[ "$PLATFORM" != "darwin" ] || exit 0 run_suite "hermetic_sandbox"