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: 964265477
Change-Id: I296910f90c62cb0bf8eb80a487f2ddf1937ef812

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,6 +188,7 @@ 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,11 +77,6 @@ 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,
@@ -91,7 +86,6 @@ 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,16 +283,14 @@ def symbol_list_gen(name, lib, out, **kwargs):
         **kwargs
     )
 
-def _common_kwargs(tags, visibility, compatible_with, testonly = None):
+def _common_kwargs(tags, visibility, compatible_with):
     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(
@@ -315,7 +313,6 @@ def sapi_library(
         deps = [],
         tags = [],
         generator_version = 2,
-        testonly = None,
         visibility = None,
         compatible_with = None,
         default_copts = [],
@@ -362,7 +359,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, testonly)
+    common = _common_kwargs(tags, visibility, compatible_with)
     generated_file_prefix = name + ".sapi"
     generated_header = generated_file_prefix + ".h"
     generated_sandboxee_src = generated_file_prefix + ".sandboxee.cc"
@@ -590,8 +587,7 @@ 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
-    testonly = kwargs.get("testonly", None)
-    common = _common_kwargs(tags, visibility, compatible_with, testonly)
+    common = _common_kwargs(tags, visibility, compatible_with)
 
     cc_library(
         name = "_unsandboxed_" + name,

sandboxed_api/examples/sum/BUILD

@@ -104,19 +104,3 @@ 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,12 +35,9 @@ class PassthroughBackend {
 
   PassthroughBackend(SandboxConfig config, CallFunctionT call_function,
                      SymbolFunctionT symbol_function)
-      : name_(std::move(config.name)),
-        rpc_channel_(std::make_unique<PassthroughRPCChannel>(
+      : 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(); }
 
@@ -63,7 +60,6 @@ class PassthroughBackend {
   void Terminate(bool attempt_graceful_exit = true) {}
 
  private:
-  std::string name_;
   std::unique_ptr<PassthroughRPCChannel> rpc_channel_;
 };
 

sandboxed_api/sandbox.h

@@ -28,7 +28,6 @@
 #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"
@@ -40,7 +39,6 @@
 #include "sandboxed_api/vars.h"
 
 namespace sapi {
-
 namespace sandbox_internal {
 
 class PtrOrCallable {
@@ -66,7 +64,7 @@ class PtrOrCallable {
 // means to communicate with it (make function calls, transfer memory).
 class SandboxBase {
  public:
-  explicit SandboxBase(std::string name = "unknown") : name_(std::move(name)) {}
+  SandboxBase() = default;
 
   virtual ~SandboxBase() = default;
 
@@ -158,8 +156,6 @@ 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.
@@ -170,7 +166,6 @@ 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
@@ -179,26 +174,23 @@ template <typename Backend>
 class Sandbox : public SandboxBase {
  public:
   explicit Sandbox(SandboxConfig config)
-      : SandboxBase(config.name), backend_(std::move(config), [this] {
+      : SandboxBase(), 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.name()), backend_(std::move(backend)) {}
+      : SandboxBase(), 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 {
-    ABSL_RETURN_IF_ERROR(backend().Init());
-    return absl::OkStatus();
-  }
+  absl::Status Init() override { return backend().Init(); }
 
   // Returns whether the current sandboxing session is active.
   bool is_active() const override { return backend().is_active(); }

sandboxed_api/sandbox2_backend.h

@@ -48,8 +48,6 @@ 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,13 +104,6 @@ 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,9 +25,6 @@ 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",
@@ -39,7 +36,6 @@ 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",
@@ -59,20 +55,17 @@ 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.set_name("%1$s"))),
+                ConfigWithForkClientContext(std::move(config)),
                 [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,19 +107,13 @@ class %1$s : public ::sapi::Sandbox<::sapi::Sandbox2Backend> {
     return config;
   }
 };
+
 )";
 
 // Text template arguments:
 //   1. Class name
-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)) {}
-};
+constexpr absl::string_view kSandboxTypedefTemplate = R"(
+using %1$s = ::sapi::Sandbox<::sapi::Sandbox2Backend>;
 )";
 
 // Text template arguments:
@@ -141,7 +135,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.set_name("%1$s")), %2$s, %3$s)) {}
+      : %1$s(::sapi::PassthroughBackend(std::move(config), %2$s, %3$s)) {}
   explicit %1$s(::sapi::PassthroughBackend backend)
       : ::sapi::Sandbox<::sapi::PassthroughBackend>(std::move(backend)) {}
 };
@@ -627,8 +621,9 @@ absl::StatusOr<std::string> Emitter::DoEmitHeader() {
             &out, kEmbedClassTemplate, sandbox_class_name,
             absl::StrReplaceAll(options_.embed_name, {{"-", "_"}}));
       } else {
-        // Or a class for the sandbox if no embedded sandboxee is used.
-        absl::StrAppendFormat(&out, kSandboxClassTemplate, sandbox_class_name);
+        // Or a typedef for the sandbox class if no embedded sandboxee is used.
+        absl::StrAppendFormat(&out, kSandboxTypedefTemplate,
+                              sandbox_class_name);
       }
       break;
     }