sandbox2: reject writable root in EnableSharedMountNamespace()

sandboxed_api/sandbox2/mounts.cc

@@ -94,9 +94,20 @@ 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 would silently become cross-instance shared state,
+    // like tmpfs above. For per-instance writable storage, bind-mount a
+    // per-instance directory instead (PolicyBuilder::AddDirectoryAt(),
+    // is_ro=false); AddTmpfs() is rejected too, at any path.
+    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/policybuilder.h

@@ -972,6 +972,8 @@ class PolicyBuilder final {
   // Not recommended
   //
   // NOTE: Requires namespace support.
+  // NOTE: Does not work with EnableSharedMountNamespace() -- a writable
+  // root is rejected there.
   PolicyBuilder& SetRootWritable();
 
   // Changes mounts propagation from MS_PRIVATE to MS_SLAVE.

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 =