mirror of
https://github.com/kuhyx/diet-guard.git
synced 2026-11-11 19:33:55 +01:00
fix: stop a touched credentials file clobbering a newer peer edit
`_local_credential_log` fell back to the hand-written `kuchnia_credentials` and used its **mtime** as the edit clock. That clock is not trustworthy: `git checkout`, a backup restore, and re-running the `install -m 600` setup line all bump it forward without the credential changing. Reproduced before fixing: a PC whose file had merely been touched, syncing against a phone that had set a new password an hour earlier, resolved to the PC's stale password. The phone user would have seen their edit silently revert. The bootstrap is still needed -- someone has to publish first, and the PC is where the password is typed -- so it moves to `_bootstrap_credential_log` and runs only when the merge came back completely empty. It can no longer outrank a real edit from a peer; it can only fill a vacuum. Once anything has been published, the synced cache (which carries a real edit time) takes over permanently. `test_a_touched_handwritten_file_cannot_clobber_a_peer` pins it, and was checked against the pre-fix code: reintroducing the fallback fails it. Also rewrites the two remaining places that still claimed a credential "must never leave the machine it was entered on" -- `_constants.py`'s comment and the doc's Credentials section, which now has a table of which of the three files syncs and which does not. Rewritten rather than given an exception clause. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QQpCiPaSbyRpwsLkH3dgsX
This commit is contained in:
parent
fd0a35e69e
commit
04f5d75aab
@ -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
|
||||
|
||||
@ -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"
|
||||
)
|
||||
|
||||
@ -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
|
||||
|
||||
@ -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"
|
||||
|
||||
@ -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/<id>/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
|
||||
|
||||
Loading…
Reference in New Issue
Block a user