From 223b57eb875524b3c2994e6c811d5f6d0cc146f0 Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Fri, 9 Oct 2026 19:05:10 +0200 Subject: [PATCH] crdt-sync 0.9.2: per-write temp file for the Firebase credential cache **FileCredentialStore.save wrote every save through one fixed `.tmp`.** Two processes share a credential cache (a long-running stream and a periodic sync), so one could rename the other's half-written temp file into place and leave a truncated refresh token behind. Each save now gets its own mkstemp file in the same directory -- created 0600 from the outset, so the token is never world-readable -- and replaces the cache atomically. A failed write unlinks its temp file and leaves the previous cache untouched. Tests cover: no temp file left after a save, distinct temp names per write, and an interrupted write keeping the old credentials with no leaked temp. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_018RHwVPyiiQMPh4M3JQwQku --- crdt-sync/crdt_sync/_credentials.py | 30 ++++++++----- .../tests/test_firebase_auth_session.py | 45 +++++++++++++++++++ crdt-sync/pyproject.toml | 2 +- 3 files changed, 66 insertions(+), 11 deletions(-) diff --git a/crdt-sync/crdt_sync/_credentials.py b/crdt-sync/crdt_sync/_credentials.py index f87ec87..392634c 100644 --- a/crdt-sync/crdt_sync/_credentials.py +++ b/crdt-sync/crdt_sync/_credentials.py @@ -15,10 +15,10 @@ from dataclasses import dataclass import datetime as dt import json import logging -from typing import TYPE_CHECKING, Protocol - -if TYPE_CHECKING: - from pathlib import Path +import os +from pathlib import Path +import tempfile +from typing import Protocol _logger = logging.getLogger(__name__) @@ -132,12 +132,22 @@ class FileCredentialStore: def save(self, credentials: FirebaseCredentials) -> None: """Write ``credentials`` atomically, readable only by this user.""" self._path.parent.mkdir(parents=True, exist_ok=True) - temp = self._path.with_suffix(f"{self._path.suffix}.tmp") - # Create with 0600 from the outset -- writing then chmod'ing would - # leave the refresh token world-readable for the gap in between. - temp.touch(mode=0o600) - temp.write_text(json.dumps(credentials.to_json()), encoding="utf-8") - temp.replace(self._path) + # A unique temp name per write, not a fixed ``.tmp``: two processes + # (a long-running stream and a periodic sync) share one cache, and a + # fixed name let one rename the other's half-written file into place. + # mkstemp creates it 0600 from the outset -- writing then chmod'ing + # would leave the refresh token world-readable for the gap between. + fd, name = tempfile.mkstemp( + dir=self._path.parent, prefix=f".{self._path.name}.", suffix=".tmp" + ) + temp = Path(name) + try: + with os.fdopen(fd, "w", encoding="utf-8") as handle: + handle.write(json.dumps(credentials.to_json())) + temp.replace(self._path) + except BaseException: + temp.unlink(missing_ok=True) + raise def clear(self) -> None: """Delete the credentials file if it exists.""" diff --git a/crdt-sync/crdt_sync/tests/test_firebase_auth_session.py b/crdt-sync/crdt_sync/tests/test_firebase_auth_session.py index 848d282..a50181c 100644 --- a/crdt-sync/crdt_sync/tests/test_firebase_auth_session.py +++ b/crdt-sync/crdt_sync/tests/test_firebase_auth_session.py @@ -21,6 +21,7 @@ from crdt_sync import ( FirebaseTokenProvider, MemoryCredentialStore, ) +from crdt_sync import _credentials as fa_credentials from crdt_sync import _firebase_auth as fa if TYPE_CHECKING: @@ -172,3 +173,47 @@ class TestFileCredentialStore: loaded = store.load() assert loaded is not None assert loaded.id_token == _ID_2 + + def test_leaves_no_temp_file_behind(self, tmp_path: Path) -> None: + """A finished save leaves only the credentials file in the directory.""" + path = tmp_path / "creds.json" + FileCredentialStore(path).save(_credentials()) + assert [p.name for p in tmp_path.iterdir()] == ["creds.json"] + + def test_each_write_uses_its_own_temp_file(self, tmp_path: Path) -> None: + """Two writers never share a temp name, so neither renames the other's. + + A fixed ``creds.json.tmp`` let a stream and a periodic sync writing + the same cache collide; mkstemp hands each save a fresh name. + """ + path = tmp_path / "creds.json" + seen: list[str] = [] + real_mkstemp = fa_credentials.tempfile.mkstemp + + def spy(**kwargs: str | Path) -> tuple[int, str]: + fd, name = real_mkstemp(**kwargs) + seen.append(name) + return fd, name + + with patch.object(fa_credentials.tempfile, "mkstemp", side_effect=spy): + FileCredentialStore(path).save(_credentials()) + FileCredentialStore(path).save(_credentials(id_value=_ID_2)) + assert len(set(seen)) == 2 + assert all(name != f"{path}.tmp" for name in seen) + + def test_a_failed_write_keeps_the_old_file_and_cleans_up( + self, tmp_path: Path + ) -> None: + """An interrupted save neither clobbers the cache nor leaks a temp.""" + path = tmp_path / "creds.json" + store = FileCredentialStore(path) + store.save(_credentials()) + with ( + patch.object(fa_credentials.json, "dumps", side_effect=OSError("full")), + pytest.raises(OSError, match="full"), + ): + store.save(_credentials(id_value=_ID_2)) + assert [p.name for p in tmp_path.iterdir()] == ["creds.json"] + loaded = store.load() + assert loaded is not None + assert loaded.id_token == _ID_1 diff --git a/crdt-sync/pyproject.toml b/crdt-sync/pyproject.toml index 6b7a13f..7fb4cd4 100644 --- a/crdt-sync/pyproject.toml +++ b/crdt-sync/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "crdt-sync" -version = "0.9.1" +version = "0.9.2" description = "Shared CRDT merge scheme + GitHub-Contents-API sync transport for cross-device apps" requires-python = ">=3.14" dependencies = ["requests==2.34.2"]