fix: Mount lib_mounts to root in sandbox

Bug: N/A
Change-Id: Icc210b91acd807ce27c819b856faec253e82ff90
GitOrigin-RevId: a89678ad03480c5aa0c5a179b01df78f4f4bb640

Author: Privacy Sandbox Team

src/roma/byob/config/config.h

@@ -77,6 +77,7 @@ struct Config {
   std::uint64_t memory_limit_soft = 0;
   std::uint64_t memory_limit_hard = 0;
   std::string roma_container_name;
+  // Mounts /x -> /x and /y/z -> /z.
   std::string lib_mounts = LIB_MOUNTS;
 };
 }  // namespace privacy_sandbox::server_common::byob

src/roma/byob/container/run_workers.cc

@@ -226,14 +226,16 @@ struct ReloaderImplArg {
 };
 
 std::vector<std::pair<std::filesystem::path, std::filesystem::path>>
-GetSourcesAndTargets(const std::filesystem::path& pivot_root_dir,
-                     absl::Span<const std::string> mounts) {
+GetSourcesAndTargets(absl::Span<const std::string> mounts) {
   std::vector<std::pair<std::filesystem::path, std::filesystem::path>>
       sources_and_targets;
   sources_and_targets.reserve(mounts.size());
   for (const std::filesystem::path mount : mounts) {
+    // Mount /x -> /x and /y/z -> /z.
     sources_and_targets.push_back(
-        {mount, pivot_root_dir / mount.relative_path()});
+        {mount,
+         "/" /
+             (mount.has_filename() ? mount : mount.parent_path()).filename()});
   }
   return sources_and_targets;
 }
@@ -248,21 +250,32 @@ int ReloaderImpl(void* arg) {
   const absl::StatusOr<std::filesystem::path> pivot_root_dir =
       CreatePivotRootDir();
   CHECK_OK(pivot_root_dir);
-  const std::vector<std::pair<std::filesystem::path, std::filesystem::path>>
-      sources_and_targets_read_only =
-          GetSourcesAndTargets(*pivot_root_dir, reloader_impl_arg.mounts);
   {
     const std::filesystem::path socket_dir =
         std::filesystem::path(reloader_impl_arg.socket_name).parent_path();
     std::vector<std::pair<std::filesystem::path, std::filesystem::path>>
-        socket_dir_read_and_write = {
-            {socket_dir, *pivot_root_dir / socket_dir.relative_path()}};
+        sources_and_targets_read_only =
+            GetSourcesAndTargets(reloader_impl_arg.mounts);
+    sources_and_targets_read_only.push_back({socket_dir, socket_dir});
+
     // SetupPivotRoot reduces the base filesystem image size. This pivot root
     // includes the socket_dir, which must not be shared with the pivot_root
     // created by the worker.
     CHECK_OK(::privacy_sandbox::server_common::byob::SetupPivotRoot(
-        *pivot_root_dir, socket_dir_read_and_write,
-        /*cleanup_pivot_root_dir=*/true, sources_and_targets_read_only));
+        *pivot_root_dir, sources_and_targets_read_only,
+        /*cleanup_pivot_root_dir=*/true,
+        /*sources_and_targets_read_and_write=*/
+        {{reloader_impl_arg.log_dir_name, reloader_impl_arg.log_dir_name}}));
+  }
+
+  // Reloader mounts /x -> /x and /y/z -> /z.
+  // Workers mounts /a -> /a.
+  std::vector<std::pair<std::filesystem::path, std::filesystem::path>>
+      sources_and_targets_read_only;
+  sources_and_targets_read_only.reserve(reloader_impl_arg.mounts.size());
+  for (const auto& [_, target] :
+       GetSourcesAndTargets(reloader_impl_arg.mounts)) {
+    sources_and_targets_read_only.push_back({target, target});
   }
   while (true) {
     // Start a new worker.
@@ -488,7 +501,7 @@ class WorkerRunner final : public WorkerRunnerService::Service {
     return absl::OkStatus();
   }
 
-  absl::Status CreateWorkerPool(std::filesystem::path binary_path,
+  absl::Status CreateWorkerPool(const std::filesystem::path binary_path,
                                 std::string_view code_token,
                                 const int num_workers,
                                 const bool enable_log_egress)
@@ -498,15 +511,14 @@ class WorkerRunner final : public WorkerRunnerService::Service {
       code_token_to_reloader_pids_[code_token].reserve(num_workers);
     }
     std::vector<std::string> mounts = mounts_;
-    mounts.push_back(binary_path.parent_path());
-    if (enable_log_egress) {
-      mounts.push_back(log_dir_name_);
-    }
+    const std::filesystem::path binary_dir = binary_path.parent_path();
+    mounts.push_back(binary_dir);
     ReloaderImplArg reloader_impl_arg{
         .mounts = std::move(mounts),
         .socket_name = socket_name_,
         .code_token = std::string(code_token),
-        .binary_path = std::move(binary_path),
+        // Within the pivot root, binary_dir is a child of root, not progdir.
+        .binary_path = binary_dir.filename() / binary_path.filename(),
         .dev_null_fd = dev_null_fd_,
         .enable_log_egress = enable_log_egress,
         .log_dir_name = log_dir_name_,

src/roma/byob/interface/roma_service.cc

@@ -48,29 +48,21 @@ LocalHandle::LocalHandle(int pid, std::string_view mounts,
         std::filesystem::path(CONTAINER_PATH) / CONTAINER_ROOT_RELPATH;
     std::vector<std::pair<std::filesystem::path, std::filesystem::path>>
         sources_and_targets = {
-            {log_dir,
-             root_dir / std::filesystem::path(log_dir).relative_path()},
-            {socket_dir,
-             root_dir / std::filesystem::path(socket_dir).relative_path()},
+            {log_dir, "/log_dir"},
+            {socket_dir, "/socket_dir"},
             // Needs to be mounted for Cancel to work (kill by cmdline)
-            {"/proc",
-             root_dir / std::filesystem::path("/proc").relative_path()},
-            {"/dev", root_dir / std::filesystem::path("/dev").relative_path()}};
+            {"/proc", "/proc"},
+            {"/dev", "/dev"}};
     CHECK_OK(::privacy_sandbox::server_common::byob::SetupPivotRoot(
         root_dir, /*sources_and_targets_read_only=*/{},
         /*cleanup_pivot_root_dir=*/false, sources_and_targets));
     const std::string mounts_flag = absl::StrCat("--mounts=", mounts);
-    const std::string control_socket_name_flag =
-        absl::StrCat("--control_socket_name=", control_socket_path);
-    const std::string udf_socket_name_flag =
-        absl::StrCat("--udf_socket_name=", udf_socket_path);
-    const std::string log_dir_flag = absl::StrCat("--log_dir=", log_dir);
     const char* argv[] = {
         "/server/bin/run_workers",
         mounts_flag.c_str(),
-        control_socket_name_flag.c_str(),
-        udf_socket_name_flag.c_str(),
-        log_dir_flag.c_str(),
+        "--control_socket_name=/socket_dir/control.sock",
+        "--udf_socket_name=/socket_dir/byob_rpc.sock",
+        "--log_dir=/log_dir",
         nullptr,
     };
     const char* envp[] = {
@@ -123,9 +115,9 @@ ByobHandle::ByobHandle(int pid, std::string_view mounts,
     config["process"]["args"] = {
         "/server/bin/run_workers",
         absl::StrCat("--mounts=", mounts),
-        absl::StrCat("--control_socket_name=", control_socket_path),
-        absl::StrCat("--udf_socket_name=", udf_socket_path),
-        absl::StrCat("--log_dir=", log_dir),
+        "--control_socket_name=/socket_dir/control.sock",
+        "--udf_socket_name=/socket_dir/byob_rpc.sock",
+        "--log_dir=/log_dir",
     };
     config["process"]["rlimits"] = {};
     // If a memory limit has been configured, apply it.
@@ -146,13 +138,13 @@ ByobHandle::ByobHandle(int pid, std::string_view mounts,
     config["mounts"] = {
         {
             {"source", socket_dir},
-            {"destination", socket_dir},
+            {"destination", "/socket_dir"},
             {"type", "bind"},
             {"options", {"rbind", "rprivate"}},
         },
         {
             {"source", log_dir},
-            {"destination", log_dir},
+            {"destination", "/log_dir"},
             {"type", "bind"},
             {"options", {"rbind", "rprivate"}},
         },

src/roma/byob/sample_udf/BUILD.bazel

@@ -169,6 +169,16 @@ cc_binary(
     ],
 )
 
+cc_binary(
+    name = "socket_finder_udf",
+    srcs = ["socket_finder_udf.cc"],
+    visibility = ["//visibility:public"],
+    deps = [
+        ":sample_byob_sdk_cc_proto",
+        "@com_google_protobuf//:protobuf",
+    ],
+)
+
 cc_binary(
     name = "pause_udf",
     srcs = ["pause_udf.cc"],
@@ -365,6 +375,7 @@ filegroup(
         ":sample_go_udf",
         ":sample_java_native_udf",
         ":sample_udf",
+        ":socket_finder_udf",
         ":sort_list_100k_udf",
         ":sort_list_10k_udf",
         ":sort_list_1m_udf",

src/roma/byob/sample_udf/socket_finder_udf.cc

@@ -0,0 +1,61 @@
+// Copyright 2025 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+#include <fstream>
+#include <iostream>
+
+#include "google/protobuf/util/delimited_message_util.h"
+#include "src/roma/byob/sample_udf/sample_udf_interface.pb.h"
+
+using ::google::protobuf::io::FileInputStream;
+using ::google::protobuf::util::ParseDelimitedFromZeroCopyStream;
+using ::google::protobuf::util::SerializeDelimitedToFileDescriptor;
+using ::privacy_sandbox::roma_byob::example::SampleRequest;
+using ::privacy_sandbox::roma_byob::example::SampleResponse;
+
+void ReadRequestFromFd(int fd) {
+  SampleRequest bin_request;
+  FileInputStream input(fd);
+  ParseDelimitedFromZeroCopyStream(&bin_request, &input, nullptr);
+}
+
+void WriteResponseToFd(int fd, SampleResponse resp) {
+  google::protobuf::util::SerializeDelimitedToFileDescriptor(resp, fd);
+}
+
+bool CanFindSocketInFileSystem() {
+  for (const auto& entry : std::filesystem::recursive_directory_iterator("/")) {
+    if (std::filesystem::is_socket(entry.path())) {
+      return true;
+    }
+  }
+  return false;
+}
+
+int main(int argc, char* argv[]) {
+  if (argc < 2) {
+    std::cerr << "Not enough arguments!" << std::endl;
+    return -1;
+  }
+  int fd = std::stoi(argv[1]);
+  ReadRequestFromFd(fd);
+  SampleResponse bin_response;
+  if (CanFindSocketInFileSystem()) {
+    bin_response.set_greeting("Failure. Able to find a socket file.");
+  } else {
+    bin_response.set_greeting("Success.");
+  }
+  WriteResponseToFd(fd, std::move(bin_response));
+  return 0;
+}

src/roma/byob/test/roma_byob_test.cc

@@ -51,6 +51,8 @@ const std::filesystem::path kUdfPath = "/udf";
 const std::filesystem::path kGoLangBinaryFilename = "sample_go_udf";
 const std::filesystem::path kCPlusPlusBinaryFilename = "sample_udf";
 const std::filesystem::path kCPlusPlusCapBinaryFilename = "cap_udf";
+const std::filesystem::path kCPlusPlusSocketFinderBinaryFilename =
+    "socket_finder_udf";
 const std::filesystem::path kCPlusPlusFileSystemModificationFilename =
     "filesystem_udf";
 const std::filesystem::path kCPlusPlusNewBinaryFilename = "new_udf";
@@ -179,8 +181,33 @@ std::pair<SampleResponse, absl::Status> GetResponseAndLogStatus(
   return {*bin_response, log_status};
 }
 
-// TODO: b/387989444 - Enable once the filesystem egression issue is fixed.
-TEST(RomaByobTest, DISABLED_NoFileSystemChangeEgressionInNonSandboxMode) {
+TEST(RomaByobTest, NoSocketFileInNonSandboxMode) {
+  Mode mode = Mode::kModeNoSandbox;
+  if (!HasClonePermissionsByobWorker(mode)) {
+    GTEST_SKIP() << "HasClonePermissionsByobWorker check returned false";
+  }
+  ByobSampleService<> roma_service = GetRomaService(mode);
+
+  std::string code_token =
+      LoadCode(roma_service, kUdfPath / kCPlusPlusSocketFinderBinaryFilename,
+               /*enable_log_egress=*/true, /*num_workers=*/1);
+
+  EXPECT_THAT(SendRequestAndGetResponse(roma_service, code_token).greeting(),
+              ::testing::StrEq("Success."));
+}
+
+TEST(RomaByobTest, NoSocketFileInSandboxMode) {
+  ByobSampleService<> roma_service = GetRomaService(Mode::kModeSandbox);
+
+  std::string code_token =
+      LoadCode(roma_service, kUdfPath / kCPlusPlusSocketFinderBinaryFilename,
+               /*enable_log_egress=*/true, /*num_workers=*/1);
+
+  EXPECT_THAT(SendRequestAndGetResponse(roma_service, code_token).greeting(),
+              ::testing::StrEq("Success."));
+}
+
+TEST(RomaByobTest, NoFileSystemChangeEgressionInNonSandboxMode) {
   Mode mode = Mode::kModeNoSandbox;
   if (!HasClonePermissionsByobWorker(mode)) {
     GTEST_SKIP() << "HasClonePermissionsByobWorker check returned false";
@@ -189,11 +216,10 @@ TEST(RomaByobTest, DISABLED_NoFileSystemChangeEgressionInNonSandboxMode) {
 
   std::string code_token = LoadCode(
       roma_service, kUdfPath / kCPlusPlusFileSystemModificationFilename,
-      /*enable_log_egress=*/false, /*num_workers=*/1);
+      /*enable_log_egress=*/true, /*num_workers=*/1);
 
-  EXPECT_THAT(
-      SendRequestAndGetResponse(roma_service, code_token).greeting(),
-      ::testing::StrEq("Success. Could not write file in any directory."));
+  EXPECT_THAT(SendRequestAndGetResponse(roma_service, code_token).greeting(),
+              ::testing::StrEq("Success."));
 }
 
 TEST(RomaByobTest, LoadBinaryInSandboxMode) {

src/roma/byob/utility/utils.cc

@@ -97,13 +97,18 @@ absl::Status SetupPivotRoot(
   // https://man7.org/linux/man-pages/man2/pivot_root.2.html.
   PS_RETURN_IF_ERROR(Mount(nullptr, "/", nullptr, MS_REC | MS_PRIVATE));
   for (const auto& [source, target] : sources_and_targets_read_only) {
-    PS_RETURN_IF_ERROR(CreateDirectories(target));
-    PS_RETURN_IF_ERROR(
-        Mount(source.c_str(), target.c_str(), nullptr, MS_BIND | MS_RDONLY));
+    const std::filesystem::path mount_target =
+        pivot_root_dir / target.relative_path();
+    PS_RETURN_IF_ERROR(CreateDirectories(mount_target));
+    PS_RETURN_IF_ERROR(Mount(source.c_str(), mount_target.c_str(), nullptr,
+                             MS_BIND | MS_RDONLY));
   }
   for (const auto& [source, target] : sources_and_targets_read_and_write) {
-    PS_RETURN_IF_ERROR(CreateDirectories(target));
-    PS_RETURN_IF_ERROR(Mount(source.c_str(), target.c_str(), nullptr, MS_BIND));
+    const std::filesystem::path mount_target =
+        pivot_root_dir / target.relative_path();
+    PS_RETURN_IF_ERROR(CreateDirectories(mount_target));
+    PS_RETURN_IF_ERROR(
+        Mount(source.c_str(), mount_target.c_str(), nullptr, MS_BIND));
   }
 
   // MS_REC needed here to get other mounts (/lib, /lib64 etc)
@@ -129,13 +134,13 @@ absl::Status SetupPivotRoot(
   if (::rmdir("/pivot") == -1) {
     return absl::ErrnoToStatus(errno, "rmdir('/pivot')");
   }
-  for (const auto& [source, _] : sources_and_targets_read_only) {
-    PS_RETURN_IF_ERROR(Mount(source.c_str(), source.c_str(), nullptr,
+  for (const auto& [_, target] : sources_and_targets_read_only) {
+    PS_RETURN_IF_ERROR(Mount(target.c_str(), target.c_str(), nullptr,
                              MS_REMOUNT | MS_BIND | MS_RDONLY));
   }
-  for (const auto& [source, _] : sources_and_targets_read_and_write) {
+  for (const auto& [_, target] : sources_and_targets_read_and_write) {
     PS_RETURN_IF_ERROR(
-        Mount(source.c_str(), source.c_str(), nullptr, MS_REMOUNT | MS_BIND));
+        Mount(target.c_str(), target.c_str(), nullptr, MS_REMOUNT | MS_BIND));
   }
   return absl::OkStatus();
 }