Create the offline event cache with owner-only (0600) permissions by bmehta001 · Pull Request #1506 · microsoft/cpp_client_telemetry

bmehta001 · 2026-07-15T00:02:27Z

Problem

The SQLite offline cache (OfflineStorage_SQLite / SQLiteWrapper) buffers pending telemetry/audit events — tenant ids, user identifiers, and serialized event payloads — awaiting upload. It is opened via:

sqlite3_open_v2(file, &m_db, SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE | SQLITE_OPEN_NOMUTEX, NULL);

with no permission handling anywhere in the storage layer, so SQLite's compiled-in default applies — SQLITE_DEFAULT_FILE_PERMISSIONS is 0644 (world-readable). On a POSIX system with a shared or world-accessible profile directory, any co-located user can read the buffered event stream, and any same-uid process can tamper with pending events before upload.

Fix

Restrict the cache database to owner read/write only (0600) immediately after opening it, in SQLiteWrapper::open:

Runs before WAL is enabled, so the -wal/-journal companion files inherit 0600 from the main database file — SQLite's findCreateFileMode derives their mode from the main db's st_mode, so no per-companion handling is needed.
Best-effort: a failure (e.g. an in-memory ":memory:" database, or a filesystem that ignores chmod) logs a warning but does not fail the open.
POSIX-only (#if !defined(_WIN32)): on Windows the Unix mode bits are meaningless — access is governed by NTFS ACLs.

The brief window between create (0644) and chmod (0600) is on a freshly-created empty database (events are only inserted after initialization completes), so it exposes no event data.

Test

Adds a POSIX unit test (OfflineStorageTests_SQLite.CacheFileCreatedOwnerReadWriteOnly) asserting the cache is 0600 and that any -wal/-journal/-shm companion grants no group/other access.

Verified on Linux (umask 022): a bare SQLite db is created 0644; with this change the cache and its -wal/-journal are 0600. All 32 OfflineStorageTests_SQLite pass.

The SQLite offline cache buffers pending telemetry events (tenant ids, user identifiers, serialized event payloads) but was created with SQLite's default file permissions (0644 -- world-readable), letting any co-located user on a POSIX system read the buffered event stream or tamper with pending events. Restrict the database file to 0600 immediately after opening it in SQLiteWrapper::open. This runs before WAL is enabled, so the -wal/-journal companion files inherit 0600 from the main database file (SQLite's findCreateFileMode derives their mode from the main db). The chmod is best-effort (a failure, e.g. an in-memory ":memory:" database, does not fail the open) and POSIX-only -- on Windows the Unix mode bits are meaningless (NTFS ACLs govern access). Adds a POSIX unit test asserting the cache and its companions are not group/world accessible. Verified on Linux (umask 022): a bare SQLite db is created 0644; with this change the cache and its -wal/-journal are 0600. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

This PR hardens the SQLite offline cache used by OfflineStorage_SQLite to reduce exposure of buffered telemetry/audit events on POSIX systems by ensuring the cache database file is created with owner-only permissions.

Changes:

Add a POSIX-only chmod(0600) step after opening the SQLite DB to restrict cache file access.
Add a POSIX unit test verifying the cache DB file permissions are 0600 and that any companion files do not grant group/other access.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
tests/unittests/OfflineStorageTests_SQLite.cpp	Adds a POSIX-only unit test asserting the cache DB is owner-read/write only and companions don’t grant group/other access.
lib/offline/SQLiteWrapper.hpp	Adds POSIX-only permission tightening (`chmod`) immediately after opening the SQLite DB.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+#if !defined(_WIN32)
+            if (::chmod(filename.c_str(), S_IRUSR | S_IWUSR) != 0) {
+                LOG_WARN("Could not restrict database file permissions to 0600 (errno %d)", errno);
+            }
+#endif


Address Copilot review: only the main database was chmod'd, so a cache created by an older SDK (or a companion left behind after a crash) could retain the old world-readable 0644 mode on its -wal/-shm/-journal files. Best-effort chmod any pre-existing companions to 0600 on open, ignoring ENOENT. Add ExistingFilesAreTightenedOnOpen covering the migration path (loosen db + plant a leftover -wal, then verify reopen re-tightens both). Files: - lib/offline/SQLiteWrapper.hpp - tests/unittests/OfflineStorageTests_SQLite.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

+#if !defined(_WIN32)
+            if (::chmod(filename.c_str(), S_IRUSR | S_IWUSR) != 0) {
+                LOG_WARN("Could not restrict database file permissions to 0600 (errno %d)", errno);
+            }
+            for (const char* suffix : { "-wal", "-shm", "-journal" }) {
+                std::string companion = filename + suffix;
+                if (::chmod(companion.c_str(), S_IRUSR | S_IWUSR) != 0 && errno != ENOENT) {
+                    LOG_WARN("Could not restrict %s file permissions to 0600 (errno %d)", suffix, errno);
+                }
+            }
+#endif


+    struct stat wst;
+    if (::stat(wal.c_str(), &wst) == 0)
+    {
+        EXPECT_EQ(0, static_cast<int>(wst.st_mode & (S_IRWXG | S_IRWXO)))
+            << "pre-existing -wal companion must be tightened to 0600";
+    }
+}


Address Copilot review round 2: - The main-database chmod now ignores ENOENT, so opening an in-memory (":memory:") database -- which LogManagerImpl uses when no tenant token is configured -- no longer logs a spurious permission-tightening warning on every open (there is no file to secure). - ExistingFilesAreTightenedOnOpen now removes the database and its companion files (including the planted -wal) at the end, so they don't leak into other tests that reuse the same storage filename. Files: - lib/offline/SQLiteWrapper.hpp - tests/unittests/OfflineStorageTests_SQLite.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

+                << "companion file " << suffix << " must not be group/world accessible";
+        }
+    }
+}


Address Copilot review round 3: CacheFileCreatedOwnerReadWriteOnly (and any other WAL-mode test) relied on shutdownAndRemoveFile(), which only removed the main database and could leave -wal/-shm/-journal companions behind to pollute the temp dir. Remove the companions in the shared teardown helper so every test is covered, and drop the now-redundant explicit cleanup from ExistingFilesAreTightenedOnOpen. Files: - tests/unittests/OfflineStorageTests_SQLite.cpp Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

bmehta001 requested a review from a team as a code owner July 15, 2026 00:02

bmehta001 requested a review from Copilot July 15, 2026 00:02

Copilot started reviewing on behalf of bmehta001 July 15, 2026 00:03 View session

Copilot AI reviewed Jul 15, 2026

View reviewed changes

Comment thread lib/offline/SQLiteWrapper.hpp

Comment on lines +317 to +321

#if !defined(_WIN32)

if (::chmod(filename.c_str(), S_IRUSR | S_IWUSR) != 0) {

LOG_WARN("Could not restrict database file permissions to 0600 (errno %d)", errno);

}

#endif

bmehta001 requested a review from Copilot July 15, 2026 02:06

Copilot started reviewing on behalf of bmehta001 July 15, 2026 02:08 View session

Copilot AI reviewed Jul 15, 2026

View reviewed changes

bmehta001 requested a review from Copilot July 15, 2026 02:30

Copilot started reviewing on behalf of bmehta001 July 15, 2026 02:31 View session

Copilot AI reviewed Jul 15, 2026

View reviewed changes

Comment thread tests/unittests/OfflineStorageTests_SQLite.cpp

<< "companion file " << suffix << " must not be group/world accessible";

}

}

}

bmehta001 requested a review from Copilot July 15, 2026 02:45

Copilot started reviewing on behalf of bmehta001 July 15, 2026 02:45 View session

Copilot AI reviewed Jul 15, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Create the offline event cache with owner-only (0600) permissions#1506

Create the offline event cache with owner-only (0600) permissions#1506
bmehta001 wants to merge 4 commits into
microsoft:mainfrom
bmehta001:bhamehta/secure-offline-cache-permissions

bmehta001 commented Jul 15, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

bmehta001 commented Jul 15, 2026

Problem

Fix

Test

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants