fix(services): create BrowserTracker storage directories private and usable

The service created its own credential directory with create_directories
and then refused it, because IsPrivateDirectory rejects a group-writable
path and mkdir masks its mode with the process umask. On a umask of 002
that failed every first run; a umask carrying owner bits left the
directory unwritable by its owner instead.

CreatePrivateDirectories creates each missing component and tightens it to
0700 through a descriptor opened O_DIRECTORY | O_NOFOLLOW. Components that
already exist keep their mode and are still judged by IsPrivateDirectory.

src/services/browser_tracker_service.cc

@@ -13,6 +13,7 @@
 #include <nlohmann/json.hpp>
 #include <random>
 #include <system_error>
+#include <vector>
 
 namespace mocktail {
 namespace services {
@@ -211,6 +212,48 @@ bool FindExistingDirectoryAncestor(const std::filesystem::path& directory,
   }
 }
 
+// Creates every component of `directory` below `existing_ancestor` with mode
+// 0700, whatever the process umask. mkdir masks its mode argument, so a umask
+// of 002 would leave the directory group writable and rejected by the
+// IsPrivateDirectory checks that guard this file's credentials, while a umask
+// carrying owner bits would leave it unwritable by its own owner. Only
+// components this call creates are tightened; one that already exists keeps
+// its mode and is judged by IsPrivateDirectory like any other.
+bool CreatePrivateDirectories(const std::filesystem::path& directory,
+                              const std::filesystem::path& existing_ancestor) {
+  std::vector<std::filesystem::path> missing;
+  for (std::filesystem::path candidate = directory;
+       candidate != existing_ancestor;) {
+    if (candidate.empty()) return false;
+    missing.push_back(candidate);
+    // Mirror FindExistingDirectoryAncestor's walk, including the implicit "."
+    // parent of a bare relative name, so the two agree on which components are
+    // missing.
+    std::filesystem::path parent = candidate.parent_path();
+    if (parent.empty() && candidate != ".") parent = ".";
+    if (parent.empty() || parent == candidate) return false;
+    candidate = parent;
+  }
+  for (auto component = missing.rbegin(); component != missing.rend();
+       ++component) {
+    if (mkdir(component->c_str(), 0700) != 0) {
+      if (errno != EEXIST) return false;
+      continue;
+    }
+    const int created = open(component->c_str(),
+                             O_RDONLY | O_DIRECTORY | O_CLOEXEC | O_NOFOLLOW);
+    if (created < 0) return false;
+    struct stat metadata = {};
+    const bool tightened = fstat(created, &metadata) == 0 &&
+                           S_ISDIR(metadata.st_mode) &&
+                           metadata.st_uid == geteuid() &&
+                           fchmod(created, 0700) == 0;
+    close(created);
+    if (!tightened) return false;
+  }
+  return true;
+}
+
 bool FsyncDirectoryChain(const std::filesystem::path& first,
                          const std::filesystem::path& last) {
   std::filesystem::path current = first;
@@ -322,8 +365,7 @@ bool AtomicWriteContents(const std::filesystem::path& path,
     return false;
   }
   const bool parent_existed = existing_ancestor == parent;
-  std::filesystem::create_directories(parent, filesystem_error);
-  if (filesystem_error) {
+  if (!CreatePrivateDirectories(parent, existing_ancestor)) {
     error->assign(directory_error);
     return false;
   }
@@ -449,10 +491,11 @@ BrowserTrackerResult BrowserTrackerService::EnsureInitialized(
     return result;
   }
   const bool lock_parent_existed = existing_lock_ancestor == lock_parent;
-  std::filesystem::create_directories(lock_parent, filesystem_error);
+  const bool lock_parent_created =
+      CreatePrivateDirectories(lock_parent, existing_lock_ancestor);
   struct stat lock_parent_status = {};
   const bool safe_lock_parent =
-      !filesystem_error &&
+      lock_parent_created &&
       lstat(lock_parent.c_str(), &lock_parent_status) == 0 &&
       IsPrivateDirectory(lock_parent_status);
   const int lock = safe_lock_parent

tests/browser_tracker_service_test.cc

@@ -41,18 +41,26 @@ class FakeHttpClient final : public HttpClient {
 class BrowserTrackerServiceTest : public testing::Test {
  protected:
   void SetUp() override {
+    // The service refuses to hold credentials in a group- or world-writable
+    // directory, so the directories these tests create must be private
+    // whatever umask the developer or CI runner happens to use.
+    previous_umask_ = umask(077);
     root_ = std::filesystem::temp_directory_path() /
             ("mocktail-browser-tracker-" + std::to_string(getpid()) + "-" +
              ::testing::UnitTest::GetInstance()->current_test_info()->name());
     std::filesystem::remove_all(root_);
   }
-  void TearDown() override { std::filesystem::remove_all(root_); }
+  void TearDown() override {
+    std::filesystem::remove_all(root_);
+    umask(previous_umask_);
+  }
 
   std::filesystem::path Storage() const {
     return root_ / "appData/LocalStorage/appStorage.json";
   }
   std::filesystem::path Cookies() const { return root_ / "roblox.cookie"; }
   std::filesystem::path root_;
+  mode_t previous_umask_ = 0;
 };
 
 TEST_F(BrowserTrackerServiceTest, ReusesValidStoredIdentityWithoutNetwork) {
@@ -171,6 +179,58 @@ TEST_F(BrowserTrackerServiceTest,
   EXPECT_FALSE(std::filesystem::exists(Storage()));
 }
 
+TEST_F(BrowserTrackerServiceTest, CreatesPrivateStorageDirectoryUnderAnyUmask) {
+  std::filesystem::create_directories(root_);
+  ASSERT_EQ(chmod(root_.c_str(), 0700), 0);
+  std::ofstream(Cookies()) << ".ROBLOSECURITY=secret";
+  FakeHttpClient http;
+  http.response = {true,
+                   200,
+                   R"({"browserTrackerId":987654321})",
+                   "",
+                   {"Set-Cookie: RBXEventTrackerV2="
+                    "CreateDate=1&rbxid=2&browserid=987654321; Path=/"}};
+
+  // A permissive umask must not widen the directories the service creates for
+  // its own credentials; it refuses to use any that are group or world
+  // writable.
+  const mode_t previous_umask = umask(0);
+  const BrowserTrackerResult result =
+      BrowserTrackerService(http).EnsureInitialized(Storage(), Cookies(), true);
+  umask(previous_umask);
+
+  ASSERT_TRUE(result) << result.error;
+  struct stat storage_parent = {};
+  ASSERT_EQ(stat(Storage().parent_path().c_str(), &storage_parent), 0);
+  EXPECT_EQ(storage_parent.st_mode & 0777, 0700U);
+}
+
+TEST_F(BrowserTrackerServiceTest,
+       CreatesUsableStorageDirectoryUnderRestrictiveUmask) {
+  std::filesystem::create_directories(root_);
+  ASSERT_EQ(chmod(root_.c_str(), 0700), 0);
+  std::ofstream(Cookies()) << ".ROBLOSECURITY=secret";
+  FakeHttpClient http;
+  http.response = {true,
+                   200,
+                   R"({"browserTrackerId":987654321})",
+                   "",
+                   {"Set-Cookie: RBXEventTrackerV2="
+                    "CreateDate=1&rbxid=2&browserid=987654321; Path=/"}};
+
+  // mkdir applies the umask to its mode argument, so a umask that strips owner
+  // bits would otherwise leave the service's own directory unwritable.
+  const mode_t previous_umask = umask(0200);
+  const BrowserTrackerResult result =
+      BrowserTrackerService(http).EnsureInitialized(Storage(), Cookies(), true);
+  umask(previous_umask);
+
+  ASSERT_TRUE(result) << result.error;
+  struct stat storage_parent = {};
+  ASSERT_EQ(stat(Storage().parent_path().c_str(), &storage_parent), 0);
+  EXPECT_EQ(storage_parent.st_mode & 0777, 0700U);
+}
+
 TEST_F(BrowserTrackerServiceTest, RejectsCorruptStoredIdentityFailClosed) {
   std::filesystem::create_directories(Storage().parent_path());
   std::ofstream(Storage()) << R"({"BrowserTrackerId":"0"})";