Skip to content

Commit

Permalink
policybuilder: Defensive handling of COVERAGE_DIR.
Browse files Browse the repository at this point in the history
If `COVERAGE_DIR` is not set, the PolicyBuilder will fail because it attempts to add an empty string as directory.

We're adding a check to emit a warning if the `COVERAGE_DIR` envvar is not present or empty.

PiperOrigin-RevId: 705453437
Change-Id: I15ac7f4600ef0a7d2f95bb0e82693a5116232458
  • Loading branch information
okunz authored and copybara-github committed Dec 12, 2024
1 parent 47dcb51 commit dfe5ac0
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 1 deletion.
10 changes: 9 additions & 1 deletion sandboxed_api/sandbox2/policybuilder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,15 @@ PolicyBuilder& PolicyBuilder::AllowLlvmCoverage() {
LABEL(&labels, mmap_end),
};
});
AddDirectoryIfNamespaced(getenv("COVERAGE_DIR"), /*is_ro=*/false);
const char* coverage_dir = std::getenv("COVERAGE_DIR");
if (!coverage_dir || absl::string_view(coverage_dir).empty()) {
LOG(WARNING)
<< "Environment variable COVERAGE is set but COVERAGE_DIR is not set. "
"No directory to collect coverage data will be added to the "
"sandbox.";
return *this;
}
AddDirectoryIfNamespaced(coverage_dir, /*is_ro=*/false);
return *this;
}

Expand Down
18 changes: 18 additions & 0 deletions sandboxed_api/sandbox2/policybuilder_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -176,5 +176,23 @@ TEST(PolicyBuilderTest, AddPolicyOnSyscallJumpOutOfBounds) {
{BPF_JUMP(BPF_JMP | BPF_JEQ | BPF_K, 1, 2, 0)});
EXPECT_THAT(builder.TryBuild(), StatusIs(absl::StatusCode::kInvalidArgument));
}

TEST(PolicyBuilderTest, TestAllowLlvmCoverage) {
ASSERT_THAT(setenv("COVERAGE", "1", 0), Eq(0));
ASSERT_THAT(setenv("COVERAGE_DIR", "/tmp", 0), Eq(0));
PolicyBuilder builder;
builder.AllowLlvmCoverage();
EXPECT_THAT(builder.TryBuild(), IsOk());
ASSERT_THAT(unsetenv("COVERAGE"), Eq(0));
ASSERT_THAT(unsetenv("COVERAGE_DIR"), Eq(0));
}

TEST(PolicyBuilderTest, TestAllowLlvmCoverageWithoutCoverageDir) {
ASSERT_THAT(setenv("COVERAGE", "1", 0), Eq(0));
PolicyBuilder builder;
builder.AllowLlvmCoverage();
EXPECT_THAT(builder.TryBuild(), IsOk());
ASSERT_THAT(unsetenv("COVERAGE"), Eq(0));
}
} // namespace
} // namespace sandbox2

0 comments on commit dfe5ac0

Please sign in to comment.