From dfe5ac04a652739d14df348bac725086a2acb9af Mon Sep 17 00:00:00 2001
From: Oliver Kunz <okunz@google.com>
Date: Thu, 12 Dec 2024 04:15:19 -0800
Subject: [PATCH] policybuilder: Defensive handling of COVERAGE_DIR.

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
---
 sandboxed_api/sandbox2/policybuilder.cc      | 10 +++++++++-
 sandboxed_api/sandbox2/policybuilder_test.cc | 18 ++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/sandboxed_api/sandbox2/policybuilder.cc b/sandboxed_api/sandbox2/policybuilder.cc
index 010f9303..653ed3b9 100644
--- a/sandboxed_api/sandbox2/policybuilder.cc
+++ b/sandboxed_api/sandbox2/policybuilder.cc
@@ -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;
 }
 
diff --git a/sandboxed_api/sandbox2/policybuilder_test.cc b/sandboxed_api/sandbox2/policybuilder_test.cc
index 39a79dac..f16a99f7 100644
--- a/sandboxed_api/sandbox2/policybuilder_test.cc
+++ b/sandboxed_api/sandbox2/policybuilder_test.cc
@@ -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