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"]