sandbox2: reject writable root in EnableSharedMountNamespace()

sandboxed_api/sandbox2/mounts.cc

@@ -94,9 +94,38 @@ absl::Status VerifyProcMount(const MountTree& mount_tree) {
 }
 
 absl::Status VerifySharedMountNamespace(const MountTree& mount_tree) {
-  if (mount_tree.has_node() && mount_tree.node().has_tmpfs_node()) {
-    return absl::FailedPreconditionError(
-        "Shared mount namespace cannot be used with tmpfs mounts.");
+  if (mount_tree.has_node()) {
+    const MountTree::Node& node = mount_tree.node();
+    if (node.has_tmpfs_node()) {
+      return absl::FailedPreconditionError(
+          "Shared mount namespace cannot be used with tmpfs mounts.");
+    }
+    // A writable root is implicit, unnamespaced-looking state: nothing in
+    // the policy marks it as something instances end up sharing, so a
+    // caller has no reason to expect that a write from one Sandbox2
+    // instance becomes visible to every other instance sharing this mount
+    // namespace -- confirmed directly: one instance's write to its
+    // writable root is readable by a second, independently-launched
+    // instance. Rejected here for the same reason tmpfs is rejected above:
+    // both are per-instance-looking state that a shared mount namespace
+    // would silently turn into cross-instance state.
+    //
+    // If per-instance writable storage is needed alongside a shared mount
+    // namespace, bind-mount a separate, per-instance directory from the
+    // host instead of making the root writable, e.g.
+    // `PolicyBuilder::AddDirectoryAt(per_instance_outside_path,
+    // "/some/inside/path", /*is_ro=*/false)` with a distinct
+    // `per_instance_outside_path` per Sandbox2 instance. A regular
+    // directory bind-mount is not a tmpfs node and is not the root node,
+    // so it isn't rejected by either check above, and each instance's
+    // writes stay confined to its own outside directory. `AddTmpfs()` is
+    // not a substitute here: this function recurses into every entry in
+    // the mount tree, not only the root, so a tmpfs mounted at any
+    // interior path is rejected the same way a tmpfs root is.
+    if (node.has_root_node() && node.root_node().writable()) {
+      return absl::FailedPreconditionError(
+          "Shared mount namespace cannot be used with a writable root.");
+    }
   }
   ABSL_RETURN_IF_ERROR(VerifyProcMount(mount_tree));
   for (const auto& [name, subtree] : mount_tree.entries()) {

sandboxed_api/sandbox2/sandbox2_test.cc

@@ -389,6 +389,27 @@ TEST(Sandbox2Test, SharedMountNamespaceWorks) {
   EXPECT_EQ(result.reason_code(), 0);
 }
 
+TEST(Sandbox2Test, SharedMountNamespaceRejectsWritableRoot) {
+  SKIP_SANITIZERS;
+
+  // A writable root, like tmpfs, is implicit per-instance state that
+  // should not be silently sharable -- EnableSharedMountNamespace() must
+  // reject it the same way it already rejects tmpfs. The check runs
+  // during policy validation, before the sandboxee is ever executed, so
+  // this reuses the existing minimal testcase rather than needing a
+  // purpose-built binary.
+  const std::string path = GetTestSourcePath("sandbox2/testcases/minimal");
+  auto executor = std::make_unique<Executor>(path, std::vector<std::string>{path});
+  SAPI_ASSERT_OK_AND_ASSIGN(
+      auto policy, sandbox2::PolicyBuilder()
+                       .DefaultAction(sandbox2::AllowAllSyscalls())
+                       .SetRootWritable()
+                       .TryBuild());
+  Sandbox2 sandbox(std::move(executor), std::move(policy));
+  EXPECT_THAT(sandbox.EnableSharedMountNamespace(),
+              StatusIs(absl::StatusCode::kFailedPrecondition));
+}
+
 TEST(SharedMemoryTest, SharedMemoryDataTransferWorks) {
   SKIP_SANITIZERS;
   const std::string path =