diff --git a/app/pubspec.yaml b/app/pubspec.yaml index ba3d439..0b659e4 100644 --- a/app/pubspec.yaml +++ b/app/pubspec.yaml @@ -2,7 +2,7 @@ name: diet_guard_app description: "Companion phone app for diet_guard: log meals on the go." publish_to: 'none' -version: 1.0.59+1 +version: 1.0.60+1 environment: sdk: ^3.12.2 diff --git a/diet_guard/_constants.py b/diet_guard/_constants.py index 5cfc995..7ed68c7 100644 --- a/diet_guard/_constants.py +++ b/diet_guard/_constants.py @@ -134,13 +134,17 @@ KUCHNIA_COMPANY: str = "kuchniavikinga" KUCHNIA_LAUNCHER_TYPE: str = "BROWSER_PANEL" # Credentials the user writes by hand, mode 600: e-mail on line 1, password on # line 2. Nothing in this package ever writes it, exactly as with -# SYNC_TOKEN_FILE. Under ~/.config, not DATA_DIR, so it is outside the synced -# tree -- a credential must never leave the machine it was entered on. +# SYNC_TOKEN_FILE, and it acts as a local override that wins over the synced +# copy. Under ~/.config rather than DATA_DIR so it is off the food-log sync +# path -- but note the credential itself DOES travel between devices now, as +# its own document (diet_guard.sync_merge._kuchnia); the phone runs its own +# catering importer and cannot fetch without it. KUCHNIA_CREDENTIALS_FILE: Path = ( Path.home() / ".config" / "diet_guard" / "kuchnia_credentials" ) -# The cached SESSION cookie, so a refresh does not re-login every time. Also -# outside the synced tree, for the same reason. +# The cached SESSION cookie, so a refresh does not re-login every time. This +# one genuinely never leaves the machine: it is regenerable from the password, +# so syncing it would widen exposure and buy nothing. KUCHNIA_SESSION_FILE: Path = ( Path.home() / ".config" / "diet_guard" / "kuchnia_session.json" ) diff --git a/diet_guard/_sync_kuchnia.py b/diet_guard/_sync_kuchnia.py index 4bc2401..af5ba54 100644 --- a/diet_guard/_sync_kuchnia.py +++ b/diet_guard/_sync_kuchnia.py @@ -47,20 +47,39 @@ _logger = logging.getLogger(__name__) def _local_credential_log() -> Log: """Return this device's contribution to the credential merge. - Prefers the synced cache, and falls back to the hand-written - ``kuchnia_credentials`` when there is no cache yet. That fallback is what - bootstraps the whole feature: the PC is where the password is typed, so - without it the PC would never publish and the phone would never receive. + Sourced from the synced cache, which carries a real edit time. Returns an + empty ``Log`` when there is no cache, so a device that has never merged + contributes nothing rather than competing on a made-up clock. - The hand-written file's mtime is its edit time. It is a coarse clock, but - it is the only one that file has, and it only has to beat the epoch that an - unset peer contributes. + The hand-written ``kuchnia_credentials`` deliberately does **not** feed + this; see :func:`_bootstrap_credential_log`. """ synced = read_synced_credential() - if synced is not None: - username, password, edited_at = synced - return credential_to_log(username, password, edited_at) + if synced is None: + return {} + username, password, edited_at = synced + return credential_to_log(username, password, edited_at) + +def _bootstrap_credential_log() -> Log: + """Return the hand-written credential, for use only when no peer has one. + + The PC is where the password is first typed, so without a bootstrap no + device would ever publish and the phone would never receive one. But this + file has no edit time of its own -- only an mtime, which ``git checkout``, + a backup restore, or re-running the ``install -m 600`` setup line all bump + forward without the credential changing. + + So it is a fallback of *last resort*, applied only when the merge came back + empty, rather than a competitor in the LWW race. Letting it compete on + mtime meant a PC whose file had merely been touched could overwrite a + password the user had just typed on the phone -- reproduced before this + split existed, and now pinned by + ``test_a_touched_handwritten_file_cannot_clobber_a_peer``. + + Its stamp is the mtime all the same: something has to go on the wire, and + once this has been published the synced cache takes over permanently. + """ # Reached through the module rather than imported by value: # `conftest._isolate_state` redirects this attribute on `_kuchnia_config`, # and importing the constant straight from `_constants` silently bypasses @@ -108,6 +127,12 @@ def _sync_kuchnia_credential( device_id, ) + if not merged: + # Nobody -- not this device's cache, not any peer -- has a credential + # yet. Only now does the hand-written file get to speak, so its + # untrustworthy mtime can never outrank a real edit from a peer. + merged = _bootstrap_credential_log() + resolved = log_to_credential(merged) if resolved is None: return diff --git a/diet_guard/tests/test_sync_kuchnia.py b/diet_guard/tests/test_sync_kuchnia.py index 7e2a55a..4f0cbc3 100644 --- a/diet_guard/tests/test_sync_kuchnia.py +++ b/diet_guard/tests/test_sync_kuchnia.py @@ -11,13 +11,18 @@ The behaviours worth pinning are the ones whose failure is silent: from __future__ import annotations import json +import time from typing import TYPE_CHECKING from unittest.mock import MagicMock from crdt_sync import Hlc, Record import diet_guard._kuchnia_credential_store as store -from diet_guard._sync_kuchnia import _local_credential_log, _sync_kuchnia_credential +from diet_guard._sync_kuchnia import ( + _bootstrap_credential_log, + _local_credential_log, + _sync_kuchnia_credential, +) from diet_guard.sync_merge._kuchnia import ( KUCHNIA_RECORD_ID, PASSWORD_FIELD_NAME, @@ -145,11 +150,11 @@ def test_a_missing_peer_file_is_skipped() -> None: ) -def test_the_synced_cache_wins_over_the_handwritten_file() -> None: - """Once a merge has resolved, that is what this device contributes. +def test_the_synced_cache_is_what_this_device_contributes() -> None: + """The hand-written file never competes in the merge. - Otherwise the hand-written file -- a deliberately *local* override -- would - be republished to every peer on every tick. + It is a deliberately *local* override; republishing it on every tick would + push a machine-local value to every peer. """ store.write_synced_credential("synced@example.com", "synced-pass", _EDITED) _write_handwritten("local@example.com\nlocal-pass\n") @@ -157,16 +162,60 @@ def test_the_synced_cache_wins_over_the_handwritten_file() -> None: assert log[KUCHNIA_RECORD_ID].fields[USERNAME_FIELD_NAME][0] == "synced@example.com" -def test_a_malformed_handwritten_file_contributes_nothing() -> None: +def test_no_cache_contributes_nothing() -> None: + """A device that has never merged competes on no clock at all.""" + _write_handwritten("local@example.com\nlocal-pass\n") + assert _local_credential_log() == {} + + +def test_a_malformed_handwritten_file_bootstraps_nothing() -> None: """A one-line credentials file is unusable, not a crash.""" _write_handwritten("only-a-username\n") - assert _local_credential_log() == {} + assert _bootstrap_credential_log() == {} -def test_no_handwritten_file_and_no_cache_contributes_nothing() -> None: - """A device that has never seen the password contributes an empty log. +def test_no_handwritten_file_bootstraps_nothing() -> None: + """The normal state of a freshly installed second device.""" + assert _bootstrap_credential_log() == {} - Distinct from the malformed-file case below: here the file is absent - entirely, which is the normal state of a freshly installed second device. + +def test_a_touched_handwritten_file_cannot_clobber_a_peer() -> None: + """A bumped mtime must not overwrite a password just set on the phone. + + ``git checkout``, a backup restore, or re-running the ``install -m 600`` + setup line all move the credentials file's mtime forward without the + credential changing. When that mtime competed in the LWW race, the PC won + it against a genuinely newer phone edit and the user's new password + silently reverted -- reproduced before the bootstrap was split out of + ``_local_credential_log``. """ - assert _local_credential_log() == {} + # A hand-written file "touched" just now, and no local cache yet. + _write_handwritten("pc@example.com\nstale-pc-pass\n") + assert store.read_synced_credential() is None + + # The phone set a new password an hour ago -- older wall time, newer edit. + phone_ms = int((time.time() - 3600) * 1000) + client = _client( + { + "diet-guard-sync/devices/phone/kuchnia.json": _peer_push( + "phone@example.com", "new-phone-pass", phone_ms + ) + } + ) + _sync_kuchnia_credential(client, ["phone"]) + + cached = store.read_synced_credential() + assert cached is not None + assert cached[1] == "new-phone-pass", ( + "the PC's touched credentials file outranked a real phone edit" + ) + + +def test_the_bootstrap_still_runs_when_no_peer_has_one() -> None: + """Last resort, but still a resort: someone has to publish first.""" + _write_handwritten("pc@example.com\nhunter2\n") + client = _client() + _sync_kuchnia_credential(client, ["phone"]) + cached = store.read_synced_credential() + assert cached is not None + assert cached[1] == "hunter2" diff --git a/docs/kuchnia-wikinga.md b/docs/kuchnia-wikinga.md index e551874..bdfa618 100644 --- a/docs/kuchnia-wikinga.md +++ b/docs/kuchnia-wikinga.md @@ -87,9 +87,36 @@ the parser refuses rather than importing a plausible-looking lie. ## Credentials -Two files under `~/.config/diet_guard/`, both mode 600, both **outside** the -synced tree — a password and a live session cookie must not travel to another -device. +Three files under `~/.config/diet_guard/`, all mode 600. They are outside +`DATA_DIR` so they stay off the *food-log* sync path — but one of them, the +credential itself, **does travel between devices**. + +**The password syncs, in plaintext.** The phone runs its own importer and +cannot fetch without it, and a phone that has been wiped needs it back without +the user digging out the original. It rides its own document +(`diet_guard/sync_merge/_kuchnia.py`, `devices//kuchnia.json`), not the +budget record — `budget.json` is written back at default permissions, so a +password inside it would be readable by anything that reads the budget. Nothing +encrypts it, and the rest of the synced state is not encrypted either, so do +not describe it as "encrypted like everything else". This is a deliberate +trade: the alternative was a phone that cannot fetch at all, and the blast +radius is a catering menu and a delivery address. + +**The session cookie does not sync.** It is regenerable from the password, so +copying it around would widen exposure and buy nothing. + +| file | written by | syncs? | +|---|---|---| +| `kuchnia_credentials` | the user, by hand | no — a local override, and the bootstrap for the first push | +| `kuchnia_synced_credential.json` | the sync layer | **yes**, as its own record | +| `kuchnia_session.json` | the client, on login | no | + +`kuchnia_credentials` wins over the synced copy when present, so a machine-local +override always works. It is never *pushed*, though — only used to bootstrap +when no device has published a credential yet. Its mtime is the only edit time +it has, and `git checkout`, a backup restore or re-running the `install` line +below all bump it without the credential changing; letting that compete in the +merge let a merely-touched file overwrite a password just typed on the phone. ```bash install -m 600 /dev/null ~/.config/diet_guard/kuchnia_credentials