Run more tests with coverage and sanitizers

Running with a permissive test policy should not interfere with sanitizers
or coverage.
Most tests should run with such a permissive policy.
The exception are tests which actually tests policy enforcement.

PiperOrigin-RevId: 513548936
Change-Id: I9a4c2cc8074997cff08cc22d15f4736219ce4d63

sandboxed_api/BUILD.bazel

@@ -224,6 +224,9 @@ cc_library(
     copts = sapi_platform_copts(),
     visibility = ["//visibility:public"],
     deps = [
+        ":config",
+        "//sandboxed_api/sandbox2:policybuilder",
+        "//sandboxed_api/sandbox2:testonly_allow_all_syscalls",
         "//sandboxed_api/util:file_base",
         "@com_google_absl//absl/strings",
     ],

sandboxed_api/CMakeLists.txt

@@ -198,10 +198,13 @@ if(BUILD_TESTING AND SAPI_BUILD_TESTING AND NOT CMAKE_CROSSCOMPILING)
     testing.h
   )
   add_library(sapi::testing ALIAS sapi_testing)
-  target_link_libraries(sapi_testing PRIVATE
-    absl::strings
+  target_link_libraries(sapi_testing
+   PRIVATE absl::strings
            sapi::file_base
            sapi::base
+   PUBLIC sapi::config
+          sandbox2::allow_all_syscalls
+          sandbox2::policybuilder
   )
 
   # sandboxed_api:sapi_test

sandboxed_api/sandbox2/BUILD.bazel

@@ -911,7 +911,6 @@ cc_test(
     ],
     deps = [
         ":sandbox2",
-        ":testonly_allow_all_syscalls",
         "//sandboxed_api:config",
         "//sandboxed_api:testing",
         "//sandboxed_api/util:status_matchers",

sandboxed_api/sandbox2/CMakeLists.txt

@@ -998,7 +998,6 @@ if(BUILD_TESTING AND SAPI_BUILD_TESTING)
   )
   target_link_libraries(sandbox2_sandbox2_test PRIVATE
     absl::strings
-    sandbox2::allow_all_syscalls
     sapi::config
     sandbox2::sandbox2
     sapi::testing

sandboxed_api/sandbox2/sandbox2_test.cc

@@ -27,7 +27,6 @@
 #include "gtest/gtest.h"
 #include "absl/strings/str_cat.h"
 #include "sandboxed_api/config.h"
-#include "sandboxed_api/sandbox2/allow_all_syscalls.h"
 #include "sandboxed_api/sandbox2/executor.h"
 #include "sandboxed_api/sandbox2/policy.h"
 #include "sandboxed_api/sandbox2/policybuilder.h"
@@ -38,19 +37,7 @@
 namespace sandbox2 {
 namespace {
 
-PolicyBuilder CreateDefaultPolicyBuilder(absl::string_view path) {
-  PolicyBuilder builder;
-  // Don't restrict the syscalls at all.
-  builder.DefaultAction(AllowAllSyscalls());
-  if constexpr (sapi::sanitizers::IsAny()) {
-    builder.AddLibrariesForBinary(path);
-  }
-  if constexpr (sapi::sanitizers::IsAny()) {
-    builder.AddDirectory("/proc");
-  }
-  return builder;
-}
-
+using ::sapi::CreateDefaultPermissiveTestPolicy;
 using ::sapi::GetTestSourcePath;
 using ::testing::Eq;
 using ::testing::IsEmpty;
@@ -60,14 +47,13 @@ using ::testing::Lt;
 // Test that aborting inside a sandbox with all userspace core dumping
 // disabled reports the signal.
 TEST(SandboxCoreDumpTest, AbortWithoutCoreDumpReturnsSignaled) {
-  SKIP_SANITIZERS_AND_COVERAGE;
   const std::string path = GetTestSourcePath("sandbox2/testcases/abort");
   std::vector<std::string> args = {
       path,
   };
   auto executor = std::make_unique<Executor>(path, args);
 
-  SAPI_ASSERT_OK_AND_ASSIGN(auto policy, CreateDefaultPolicyBuilder(path)
+  SAPI_ASSERT_OK_AND_ASSIGN(auto policy, CreateDefaultPermissiveTestPolicy(path)
                                              .TryBuild());
   Sandbox2 sandbox(std::move(executor), std::move(policy));
   auto result = sandbox.Run();
@@ -79,7 +65,6 @@ TEST(SandboxCoreDumpTest, AbortWithoutCoreDumpReturnsSignaled) {
 // Test that with TSYNC we are able to sandbox when multithreaded and with no
 // memory checks. If TSYNC is not supported, then no.
 TEST(TsyncTest, TsyncNoMemoryChecks) {
-  SKIP_SANITIZERS_AND_COVERAGE;
   const std::string path = GetTestSourcePath("sandbox2/testcases/tsync");
 
   auto executor =
@@ -87,7 +72,7 @@ TEST(TsyncTest, TsyncNoMemoryChecks) {
   executor->set_enable_sandbox_before_exec(false);
 
   SAPI_ASSERT_OK_AND_ASSIGN(auto policy,
-                            CreateDefaultPolicyBuilder(path).TryBuild());
+                            CreateDefaultPermissiveTestPolicy(path).TryBuild());
   Sandbox2 sandbox(std::move(executor), std::move(policy));
   auto result = sandbox.Run();
 
@@ -99,8 +84,6 @@ TEST(TsyncTest, TsyncNoMemoryChecks) {
 // Tests whether Executor(fd, std::vector<std::string>{path}, envp) constructor
 // works as expected.
 TEST(ExecutorTest, ExecutorFdConstructor) {
-  SKIP_SANITIZERS_AND_COVERAGE;
-
   const std::string path = GetTestSourcePath("sandbox2/testcases/minimal");
   int fd = open(path.c_str(), O_RDONLY);
   ASSERT_NE(fd, -1);
@@ -110,7 +93,7 @@ TEST(ExecutorTest, ExecutorFdConstructor) {
   auto executor = std::make_unique<Executor>(fd, args, envs);
 
   SAPI_ASSERT_OK_AND_ASSIGN(auto policy,
-                            CreateDefaultPolicyBuilder(path).TryBuild());
+                            CreateDefaultPermissiveTestPolicy(path).TryBuild());
   Sandbox2 sandbox(std::move(executor), std::move(policy));
   auto result = sandbox.Run();
 
@@ -127,7 +110,7 @@ TEST(RunAsyncTest, SandboxeeExternalKill) {
   auto executor = std::make_unique<Executor>(path, args, envs);
 
   SAPI_ASSERT_OK_AND_ASSIGN(auto policy,
-                            CreateDefaultPolicyBuilder(path).TryBuild());
+                            CreateDefaultPermissiveTestPolicy(path).TryBuild());
   Sandbox2 sandbox(std::move(executor), std::move(policy));
   ASSERT_TRUE(sandbox.RunAsync());
   sleep(1);
@@ -145,7 +128,7 @@ TEST(RunAsyncTest, SandboxeeTimeoutDisabledStacktraces) {
   std::vector<std::string> envs;
   auto executor = std::make_unique<Executor>(path, args, envs);
 
-  SAPI_ASSERT_OK_AND_ASSIGN(auto policy, CreateDefaultPolicyBuilder(path)
+  SAPI_ASSERT_OK_AND_ASSIGN(auto policy, CreateDefaultPermissiveTestPolicy(path)
                                              .CollectStacktracesOnTimeout(false)
                                              .TryBuild());
   Sandbox2 sandbox(std::move(executor), std::move(policy));
@@ -181,8 +164,8 @@ TEST(RunAsyncTest, SandboxeeNotKilledWhenStartingThreadFinishes) {
   std::vector<std::string> args = {path};
   auto executor = std::make_unique<Executor>(path, args);
 
-  SAPI_ASSERT_OK_AND_ASSIGN(auto policy, CreateDefaultPolicyBuilder(path)
-                                             .TryBuild());
+  SAPI_ASSERT_OK_AND_ASSIGN(auto policy,
+                            CreateDefaultPermissiveTestPolicy(path).TryBuild());
   Sandbox2 sandbox(std::move(executor), std::move(policy));
   std::thread sandbox_start_thread([&sandbox]() { sandbox.RunAsync(); });
   sandbox_start_thread.join();
@@ -199,7 +182,7 @@ TEST(StarvationTest, MonitorIsNotStarvedByTheSandboxee) {
   executor->limits()->set_walltime_limit(absl::Seconds(5));
 
   SAPI_ASSERT_OK_AND_ASSIGN(auto policy,
-                            CreateDefaultPolicyBuilder(path).TryBuild());
+                            CreateDefaultPermissiveTestPolicy(path).TryBuild());
   Sandbox2 sandbox(std::move(executor), std::move(policy));
 
   auto start = absl::Now();

sandboxed_api/testing.cc

@@ -14,11 +14,39 @@
 
 #include "sandboxed_api/testing.h"
 
+#include <cstdlib>
+
 #include "absl/strings/string_view.h"
+#include "sandboxed_api/config.h"
+#include "sandboxed_api/sandbox2/allow_all_syscalls.h"
 #include "sandboxed_api/util/path.h"
 
 namespace sapi {
 
+bool IsCoverageRun() {
+  return getenv("COVERAGE") != nullptr;
+}
+
+sandbox2::PolicyBuilder CreateDefaultPermissiveTestPolicy(
+    absl::string_view bin_path) {
+  sandbox2::PolicyBuilder builder;
+  // Don't restrict the syscalls at all.
+  builder.DefaultAction(sandbox2::AllowAllSyscalls());
+  if (sapi::host_os::IsAndroid()) {
+    builder.DisableNamespaces();
+  }
+  if (IsCoverageRun()) {
+    builder.AddDirectory(getenv("COVERAGE_DIR"), /*is_ro=*/false);
+  }
+  if constexpr (sapi::sanitizers::IsAny()) {
+    builder.AddLibrariesForBinary(bin_path);
+  }
+  if constexpr (sapi::sanitizers::IsAny()) {
+    builder.AddDirectory("/proc");
+  }
+  return builder;
+}
+
 std::string GetTestTempPath(absl::string_view name) {
   // When using Bazel, the environment variable TEST_TMPDIR is guaranteed to be
   // set.

sandboxed_api/testing.h

@@ -15,10 +15,11 @@
 #ifndef SANDBOXED_API_TESTING_H_
 #define SANDBOXED_API_TESTING_H_
 
-#include <cstdlib>
 #include <string>
 
 #include "absl/strings/string_view.h"
+#include "sandboxed_api/config.h"
+#include "sandboxed_api/sandbox2/policybuilder.h"
 
 // The macro SKIP_ANDROID can be used in tests to skip running a
 // given test (by emitting 'retrun') when running on Android. Example:
@@ -30,11 +31,12 @@
 //
 // The reason for this is because certain unit tests require the use of user
 // namespaces which are not present on Android.
-#if defined(__ANDROID__)
-#define SKIP_ANDROID return
-#else
-#define SKIP_ANDROID
-#endif
+#define SKIP_ANDROID                            \
+  do {                                          \
+    if constexpr (sapi::host_os::IsAndroid()) { \
+      return;                                   \
+    }                                           \
+  } while (0)
 
 // The macro SKIP_SANITIZERS_AND_COVERAGE can be used in tests to skip running
 // a given test (by emitting 'return') when running under one of the sanitizers
@@ -57,21 +59,21 @@
 // The downside of this approach is that no coverage will be collected.
 // To still have coverage, pre-compile sandboxees and add them as test data,
 // then there will be no need to skip tests.
-#if defined(ABSL_HAVE_ADDRESS_SANITIZER) || \
-    defined(ABSL_HAVE_MEMORY_SANITIZER) || defined(ABSL_HAVE_THREAD_SANITIZER)
-#define SKIP_SANITIZERS_AND_COVERAGE return
-#else
-#define SAPI_BUILDDATA_COVERAGE_ENABLED false
 #define SKIP_SANITIZERS_AND_COVERAGE                          \
   do {                                                        \
-    if (SAPI_BUILDDATA_COVERAGE_ENABLED || getenv("COVERAGE") != nullptr) { \
+    if (sapi::sanitizers::IsAny() || sapi::IsCoverageRun()) { \
       return;                                                 \
     }                                                         \
   } while (0)
-#endif
 
 namespace sapi {
 
+// Returns whether the executable running under code coverage.
+bool IsCoverageRun();
+
+sandbox2::PolicyBuilder CreateDefaultPermissiveTestPolicy(
+    absl::string_view bin_path);
+
 // Returns a writable path usable in tests. If the name argument is specified,
 // returns a name under that path. This can then be used for creating temporary
 // test files and/or directories.