mirror of
https://github.com/kuhyx/utils.git
synced 2026-11-10 19:27:16 +01:00
crdt-sync 0.9.2: per-write temp file for the Firebase credential cache
Some checks are pending
crdt-sync pre-commit / pre-commit (push) Waiting to run
crdt-sync tests / test (push) Waiting to run
dependency freshness / dependencies on newest stable (push) Waiting to run
file length / file-length (push) Waiting to run
md-naming / markdown naming convention (push) Waiting to run
palette drift / palette (push) Waiting to run
repo-contract / repo contract (push) Waiting to run
shared gate tests / gate unit tests (push) Waiting to run
Some checks are pending
crdt-sync pre-commit / pre-commit (push) Waiting to run
crdt-sync tests / test (push) Waiting to run
dependency freshness / dependencies on newest stable (push) Waiting to run
file length / file-length (push) Waiting to run
md-naming / markdown naming convention (push) Waiting to run
palette drift / palette (push) Waiting to run
repo-contract / repo contract (push) Waiting to run
shared gate tests / gate unit tests (push) Waiting to run
**FileCredentialStore.save wrote every save through one fixed `<cache>.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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018RHwVPyiiQMPh4M3JQwQku
This commit is contained in:
parent
abda028b05
commit
223b57eb87
@ -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")
|
||||
# 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."""
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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"]
|
||||
|
||||
Loading…
Reference in New Issue
Block a user