Add a name property to SAPI Sandbox class and update the code generator to propagate sandbox class names for both embedded and non-embedded libraries.

PiperOrigin-RevId: 964101509
Change-Id: I9db14c8260c32da79ac7785b9b11ba3ab4ee937a

Author: Sandboxed API Team

sandboxed_api/BUILD

@@ -168,9 +168,9 @@ cc_library(
         ":vars",
         "@abseil-cpp//absl/base:core_headers",
         "@abseil-cpp//absl/base:dynamic_annotations",
+        "@abseil-cpp//absl/base:no_destructor",
         "@abseil-cpp//absl/cleanup",
         "@abseil-cpp//absl/container:flat_hash_map",
-        "@abseil-cpp//absl/container:flat_hash_set",
         "@abseil-cpp//absl/functional:any_invocable",
         "@abseil-cpp//absl/log",
         "@abseil-cpp//absl/log:check",
@@ -188,7 +188,6 @@ cc_library(
         "//sandboxed_api/sandbox2:fork_client",
         "//sandboxed_api/sandbox2:sandbox_config",
         "//sandboxed_api/util:fileops",
-        "//sandboxed_api/util:status",
     ],
 )
 

sandboxed_api/bazel/embed_data.bzl

@@ -77,6 +77,11 @@ def sapi_cc_embed_data(name, srcs = [], namespace = "", **kwargs):
       **kwargs: extra arguments like testonly, visibility, etc.
     """
     embed_rule = "_%s_sapi" % name
+    testonly = kwargs.get("testonly", None)
+    common = {}
+    if testonly != None:
+        common["testonly"] = testonly
+
     _sapi_cc_embed_data(
         name = embed_rule,
         srcs = srcs,
@@ -86,6 +91,7 @@ def sapi_cc_embed_data(name, srcs = [], namespace = "", **kwargs):
             "%s.h" % name,
             "%s.cc" % name,
         ],
+        **common
     )
     cc_library(
         name = name,

sandboxed_api/bazel/sapi.bzl

@@ -283,14 +283,16 @@ def symbol_list_gen(name, lib, out, **kwargs):
         **kwargs
     )
 
-def _common_kwargs(tags, visibility, compatible_with):
+def _common_kwargs(tags, visibility, compatible_with, testonly = None):
     common = {
         "tags": tags,
     }
     if visibility:
         common["visibility"] = visibility
     if compatible_with != None:
         common["compatible_with"] = compatible_with
+    if testonly != None:
+        common["testonly"] = testonly
     return common
 
 def sapi_library(
@@ -313,6 +315,7 @@ def sapi_library(
         deps = [],
         tags = [],
         generator_version = 2,
+        testonly = None,
         visibility = None,
         compatible_with = None,
         default_copts = [],
@@ -359,7 +362,7 @@ def sapi_library(
       sandbox_mode: Sandbox mode to use for the generated library. Either "sandbox2" (default) or "passthrough".
     """
 
-    common = _common_kwargs(tags, visibility, compatible_with)
+    common = _common_kwargs(tags, visibility, compatible_with, testonly)
     generated_file_prefix = name + ".sapi"
     generated_header = generated_file_prefix + ".h"
     generated_sandboxee_src = generated_file_prefix + ".sandboxee.cc"
@@ -587,7 +590,8 @@ def cc_sandboxed_library(
     # TODO(dvyukov): add hash/flattening of the full library /path:name, just the name is not
     # necessarily globally unique.
     wrapper_name = "Sapi" + name
-    common = _common_kwargs(tags, visibility, compatible_with)
+    testonly = kwargs.get("testonly", None)
+    common = _common_kwargs(tags, visibility, compatible_with, testonly)
 
     cc_library(
         name = "_unsandboxed_" + name,

sandboxed_api/examples/sum/BUILD

@@ -104,3 +104,19 @@ sh_test(
     srcs = ["main_sum_test.sh"],
     data = [":main_sum"],
 )
+
+sapi_library(
+    name = "sum_sapi_noembed",
+    embed = False,
+    functions = [
+        "sum",
+    ],
+    input_files = [
+        "sum.c",
+        "sum_cpp.cc",
+    ],
+    lib = ":sum",
+    lib_name = "SumNoEmbed",
+    visibility = ["//visibility:public"],
+    deps = [":sum_params_cc_proto"],
+)

sandboxed_api/passthrough_backend.h

@@ -35,9 +35,12 @@ class PassthroughBackend {
 
   PassthroughBackend(SandboxConfig config, CallFunctionT call_function,
                      SymbolFunctionT symbol_function)
-      : rpc_channel_(std::make_unique<PassthroughRPCChannel>(
+      : name_(std::move(config.name)),
+        rpc_channel_(std::make_unique<PassthroughRPCChannel>(
             std::move(call_function), std::move(symbol_function))) {}
 
+  const std::string& name() const { return name_; }
+
   // Initializes a new sandboxing session.
   absl::Status Init() { return absl::OkStatus(); }
 
@@ -60,6 +63,7 @@ class PassthroughBackend {
   void Terminate(bool attempt_graceful_exit = true) {}
 
  private:
+  std::string name_;
   std::unique_ptr<PassthroughRPCChannel> rpc_channel_;
 };
 

sandboxed_api/sandbox.h

@@ -28,6 +28,7 @@
 #include "absl/status/status.h"
 #include "absl/status/status_macros.h"
 #include "absl/status/statusor.h"
+#include "absl/strings/string_view.h"
 #include "absl/time/time.h"
 #include "absl/types/span.h"
 #include "sandboxed_api/call.h"
@@ -39,6 +40,7 @@
 #include "sandboxed_api/vars.h"
 
 namespace sapi {
+
 namespace sandbox_internal {
 
 class PtrOrCallable {
@@ -64,7 +66,7 @@ class PtrOrCallable {
 // means to communicate with it (make function calls, transfer memory).
 class SandboxBase {
  public:
-  SandboxBase() = default;
+  explicit SandboxBase(std::string name = "unknown") : name_(std::move(name)) {}
 
   virtual ~SandboxBase() = default;
 
@@ -156,6 +158,8 @@ class SandboxBase {
   // sandboxee is not running or we're using an in-process sandbox.
   virtual absl::StatusOr<int> GetPid() const = 0;
 
+  const std::string& name() const { return name_; }
+
  protected:
   // WrapCallStatus is called with the status returned by a Call. The default
   // implementation simply returns the status as is.
@@ -166,6 +170,7 @@ class SandboxBase {
   absl::Status Call(
       const std::string& func, v::Callable* ret,
       std::initializer_list<sandbox_internal::PtrOrCallable> args);
+  std::string name_;
 };
 
 // The Sandbox class represents the sandboxed library. It provides users with
@@ -174,23 +179,26 @@ template <typename Backend>
 class Sandbox : public SandboxBase {
  public:
   explicit Sandbox(SandboxConfig config)
-      : SandboxBase(), backend_(std::move(config), [this] {
+      : SandboxBase(config.name), backend_(std::move(config), [this] {
           return CreateNotifier();  // NOLINT
         }) {}
 
   // This constructor should only be used for special cases, e.g. when using the
   // CreateNotifier() method of the Sandbox2Backend. Otherwise, prefer to use
   // the SandboxConfig constructor above.
   explicit Sandbox(Backend backend)
-      : SandboxBase(), backend_(std::move(backend)) {}
+      : SandboxBase(backend.name()), backend_(std::move(backend)) {}
 
   Sandbox(const Sandbox&) = delete;
   Sandbox& operator=(const Sandbox&) = delete;
 
   virtual ~Sandbox() = default;
 
   // Initializes a new sandboxing session.
-  absl::Status Init() override { return backend().Init(); }
+  absl::Status Init() override {
+    ABSL_RETURN_IF_ERROR(backend().Init());
+    return absl::OkStatus();
+  }
 
   // Returns whether the current sandboxing session is active.
   bool is_active() const override { return backend().is_active(); }

sandboxed_api/sandbox2_backend.h

@@ -48,6 +48,8 @@ class Sandbox2Backend {
   Sandbox2Backend(Sandbox2Backend&&);
   Sandbox2Backend& operator=(Sandbox2Backend&&);
 
+  const std::string& name() const { return config_.name; }
+
   virtual ~Sandbox2Backend();
 
   // Initializes a new sandboxing session.

sandboxed_api/sandbox_config.h

@@ -104,6 +104,13 @@ struct Sandbox2Config {
 };
 
 struct SandboxConfig {
+  std::string name = "unknown";
+
+  SandboxConfig& set_name(std::string n) {
+    name = std::move(n);
+    return *this;
+  }
+
   std::optional<std::vector<std::string>> environment_variables;
   std::optional<absl::flat_hash_map<std::string, std::string>>
       command_line_flags;

sandboxed_api/tests/BUILD

@@ -25,6 +25,9 @@ cc_test(
     name = "sapi_test",
     srcs = ["sapi_test.cc"],
     copts = sapi_platform_copts(),
+    data = [
+        "//sandboxed_api/examples/sum:sum_sapi_noembed.bin",
+    ],
     tags = ["local"],
     deps = [
         ":sapi_test-sapi",
@@ -36,6 +39,7 @@ cc_test(
         "//sandboxed_api/examples/stringop:stringop-sapi",
         "//sandboxed_api/examples/stringop:stringop_params_cc_proto",
         "//sandboxed_api/examples/sum:sum-sapi",
+        "//sandboxed_api/examples/sum:sum_sapi_noembed",
         "//sandboxed_api/sandbox2:result",
         "//sandboxed_api/util:fileops",
         "//sandboxed_api/util:thread",
@@ -55,17 +59,20 @@ cc_test(
 
 cc_library(
     name = "sapi_test_lib",
+    testonly = 1,
     srcs = [
         "sapi_test_lib_cpp.cc",
     ],
     deps = [
         "@abseil-cpp//absl/algorithm:container",
         "@abseil-cpp//absl/types:span",
     ],
+    alwayslink = 1,
 )
 
 sapi_library(
     name = "sapi_test-sapi",
+    testonly = 1,
     functions = [
         "accumulate",
         "compare_self_symbol",

sandboxed_api/tools/clang_generator/emitter.cc

@@ -89,7 +89,7 @@ class %1$s : public ::sapi::Sandbox<::sapi::Sandbox2Backend> {
       : %1$s(::sapi::SandboxConfig::DefaultConfig()) {}
   explicit %1$s(::sapi::SandboxConfig config)
       : %1$s(::sapi::Sandbox2Backend(
-                ConfigWithForkClientContext(std::move(config)),
+                ConfigWithForkClientContext(std::move(config.set_name("%1$s"))),
                 [this] { return CreateNotifier(); })) {}
   // This constructor should only be used for special cases, e.g. when using the
   // CreateNotifier() method. Otherwise, prefer to use the SandboxConfig
@@ -107,13 +107,19 @@ class %1$s : public ::sapi::Sandbox<::sapi::Sandbox2Backend> {
     return config;
   }
 };
-
 )";
 
 // Text template arguments:
 //   1. Class name
-constexpr absl::string_view kSandboxTypedefTemplate = R"(
-using %1$s = ::sapi::Sandbox<::sapi::Sandbox2Backend>;
+constexpr absl::string_view kSandboxClassTemplate = R"(
+// Sandbox class with default policy (non-embedded)
+class %1$s : public ::sapi::Sandbox<::sapi::Sandbox2Backend> {
+ public:
+  explicit %1$s(::sapi::SandboxConfig config)
+      : ::sapi::Sandbox<::sapi::Sandbox2Backend>(std::move(config.set_name("%1$s"))) {}
+  explicit %1$s(::sapi::Sandbox2Backend backend)
+      : ::sapi::Sandbox<::sapi::Sandbox2Backend>(std::move(backend)) {}
+};
 )";
 
 // Text template arguments:
@@ -135,7 +141,7 @@ class %1$s : public ::sapi::Sandbox<::sapi::PassthroughBackend> {
   %1$s()
       : %1$s(::sapi::SandboxConfig{}) {}
   explicit %1$s(::sapi::SandboxConfig config)
-      : %1$s(::sapi::PassthroughBackend(std::move(config), %2$s, %3$s)) {}
+      : %1$s(::sapi::PassthroughBackend(std::move(config.set_name("%1$s")), %2$s, %3$s)) {}
   explicit %1$s(::sapi::PassthroughBackend backend)
       : ::sapi::Sandbox<::sapi::PassthroughBackend>(std::move(backend)) {}
 };
@@ -621,9 +627,8 @@ absl::StatusOr<std::string> Emitter::DoEmitHeader() {
             &out, kEmbedClassTemplate, sandbox_class_name,
             absl::StrReplaceAll(options_.embed_name, {{"-", "_"}}));
       } else {
-        // Or a typedef for the sandbox class if no embedded sandboxee is used.
-        absl::StrAppendFormat(&out, kSandboxTypedefTemplate,
-                              sandbox_class_name);
+        // Or a class for the sandbox if no embedded sandboxee is used.
+        absl::StrAppendFormat(&out, kSandboxClassTemplate, sandbox_class_name);
       }
       break;
     }