Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,10 @@ private SpawnResult runSpawn(
/** Override this method if you need to run a post condition after the action has executed */
public void verifyPostCondition(
Spawn originalSpawn, SandboxedSpawn sandbox, SpawnExecutionContext context)
throws IOException {}
throws IOException {
SandboxHelpers.validateOutputs(
SandboxHelpers.getOutputs(originalSpawn), sandbox.getSandboxExecRoot());
}

private String makeFailureMessage(Spawn originalSpawn, SandboxedSpawn sandbox) {
if (sandboxOptions.sandboxDebug) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,7 @@ private ImmutableMap<Path, Path> prepareAndGetBindMounts(
public void verifyPostCondition(
Spawn originalSpawn, SandboxedSpawn sandbox, SpawnExecutionContext context)
throws IOException {
super.verifyPostCondition(originalSpawn, sandbox, context);
if (getSandboxOptions().useHermetic) {
checkForConcurrentModifications(context);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,23 @@ public static void moveOutputs(SandboxOutputs outputs, Path sourceRoot, Path tar
}
}

/**
* Validates that all symbolic links in the outputs are not dangling.
*
* @param outputs outputs to validate as relative paths to a root
* @param sourceRoot root directory to validate from
* @throws IOException if a dangling symbolic link is found
*/
public static void validateOutputs(SandboxOutputs outputs, Path sourceRoot) throws IOException {
for (Entry<PathFragment, PathFragment> output :
Iterables.concat(outputs.files().entrySet(), outputs.dirs().entrySet())) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you move this logic into moveOutputs, you could reuse the stat performed there to avoid an additional stat for all outputs, which can be significant.

Path source = sourceRoot.getRelative(output.getValue());
if (source.isSymbolicLink() && !source.exists()) {
throw new IOException("declared output '" + output.getKey() + "' is a dangling symbolic link");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Outputs matched to artifacts declared with ctx.declare_symlink could legitimately be dangling.

}
}
}

private static void copyFile(Path source, Path target) throws IOException {
try (InputStream in = source.getInputStream();
OutputStream out = target.getOutputStream()) {
Expand Down
27 changes: 27 additions & 0 deletions src/test/shell/bazel/bazel_sandboxing_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -874,6 +874,33 @@ EOF
assert_equals world "$(cat bazel-bin/pkg/some_file2.json)"
}

function test_dangling_symlink_sandboxed_deterministic_failure() {
# Regression test for https://github.com/bazelbuild/bazel/issues/3759
cat << 'EOF' > BUILD
genrule(
name = "make-target",
outs = ["target"],
cmd = "touch $@",
)
genrule(
name = "make-link",
outs = ["link"],
cmd = "ln -s target $@",
)
EOF

# 1. Build :link first. It should FAIL because :target doesn't exist in the sandbox.
bazel build :link &> $TEST_log && fail "Expected :link to fail when :target is missing" || true
expect_log "is a dangling symbolic link"

# 2. Build :target.
bazel build :target &> $TEST_log || fail "Expected :target to build successfully"

# 3. Build :link again. This should still FAIL because :target is not a declared input.
bazel build :link &> $TEST_log && fail "Expected :link to fail even after :target is built" || true
expect_log "is a dangling symbolic link"
}

# The test shouldn't fail if the environment doesn't support running it.
check_sandbox_allowed || exit 0

Expand Down