M9 S4: sync adopts serialization/parse_dt toolkit + normalizes push oracle
DRY: - serialize.py: serialize_label_sync(label) = base serialize_label + the delta-only fields (sync_revision/purged_at/created_at via iso()). sync's changes() adopts it; the local _serialize_label_row near-dup is gone. - sync adopts common.parse_dt (drops the byte-identical _parse_client_dt; 4 call sites) and common.iso for the note delta augmentation. (Manual-label reconciliation was already shared in S3.) Fully folding the note re-augmentation into the serializer waits on the notes.py split. - test_sync: drops the now-redundant _parse_client_dt test (parse_dt is covered in test_notes) + its dead import. Security (issue — push existence-oracle): a foreign-owned id on push was rejected with "not yours", distinguishing "another user's note" from a free id. A legit client only pushes ids of notes it created, so that branch is only hit by a probe (or ~0-prob UUID collision) — now a GENERIC "cannot apply" rejection that doesn't confirm the id exists. The residual create-vs-reject status difference is inherent to client-chosen ids over a global PK and is practically unexploitable (a shared note already exposes its id to recipients). Sync behavior operator-verified on deploy (no Postgres CI lane). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FRgehjoz7Yv8LkUfADxACm
This commit is contained in:
@@ -5,10 +5,22 @@ extras their surface needs (usage count in the labels API, sync_revision/purged_
|
||||
in sync deltas). Grows as sections adopt it (labels here; note/user/device later)."""
|
||||
from __future__ import annotations
|
||||
|
||||
from .common import iso
|
||||
from .models.label import Label
|
||||
|
||||
|
||||
def serialize_label(label: Label) -> dict:
|
||||
"""Base label shape {id, name, color}. The labels API adds `count`; sync deltas
|
||||
(S4) add `sync_revision`/`purged_at`/`created_at` on top of this."""
|
||||
add `sync_revision`/`purged_at`/`created_at` on top of this."""
|
||||
return {"id": str(label.id), "name": label.name, "color": label.color}
|
||||
|
||||
|
||||
def serialize_label_sync(label: Label) -> dict:
|
||||
"""Label as a sync delta row: the base shape + the fields native clients reconcile
|
||||
on (monotonic revision, tombstone marker, creation time)."""
|
||||
return {
|
||||
**serialize_label(label),
|
||||
"sync_revision": label.sync_revision,
|
||||
"purged_at": iso(label.purged_at),
|
||||
"created_at": iso(label.created_at),
|
||||
}
|
||||
|
||||
+18
-28
@@ -19,6 +19,7 @@ from sqlalchemy import delete as sa_delete
|
||||
from sqlalchemy import func, select
|
||||
|
||||
from .auth import login_required
|
||||
from .common import iso, parse_dt
|
||||
from .config import Config
|
||||
from .db import session_scope
|
||||
from .labeling import reconcile_manual_labels, resolve_owned_label_ids
|
||||
@@ -37,6 +38,7 @@ from .notes import (
|
||||
normalize_color,
|
||||
normalize_recurrence,
|
||||
)
|
||||
from .serialize import serialize_label_sync
|
||||
|
||||
bp = Blueprint("sync", __name__, url_prefix="/api/sync")
|
||||
|
||||
@@ -80,17 +82,6 @@ def _page_cursor(note_revs: list[int], label_revs: list[int], since: int, limit:
|
||||
return (max(all_revs) if all_revs else since), False
|
||||
|
||||
|
||||
def _serialize_label_row(lb: Label) -> dict:
|
||||
return {
|
||||
"id": str(lb.id),
|
||||
"name": lb.name,
|
||||
"color": lb.color,
|
||||
"sync_revision": lb.sync_revision,
|
||||
"purged_at": lb.purged_at.isoformat() if lb.purged_at else None,
|
||||
"created_at": lb.created_at.isoformat() if lb.created_at else None,
|
||||
}
|
||||
|
||||
|
||||
@bp.get("/changes")
|
||||
@login_required
|
||||
async def changes():
|
||||
@@ -126,15 +117,18 @@ async def changes():
|
||||
note_rows = [n for n in note_rows if n.sync_revision <= cursor]
|
||||
label_rows = [lb for lb in label_rows if lb.sync_revision <= cursor]
|
||||
|
||||
# Note bodies come from the shared note serializer; sync adds the two
|
||||
# delta-only fields on top (folding these into the serializer itself waits on
|
||||
# the notes.py serialization split).
|
||||
notes_out = await _serialize_notes(db, note_rows)
|
||||
for data, n in zip(notes_out, note_rows):
|
||||
data["sync_revision"] = n.sync_revision
|
||||
data["purged_at"] = n.purged_at.isoformat() if n.purged_at else None
|
||||
data["purged_at"] = iso(n.purged_at)
|
||||
|
||||
return jsonify(
|
||||
{
|
||||
"notes": notes_out,
|
||||
"labels": [_serialize_label_row(lb) for lb in label_rows],
|
||||
"labels": [serialize_label_sync(lb) for lb in label_rows],
|
||||
"cursor": cursor,
|
||||
"has_more": has_more,
|
||||
}
|
||||
@@ -144,15 +138,6 @@ async def changes():
|
||||
# --- Push: apply client mutations (LWW by client edit-time, non-destructive) ---
|
||||
|
||||
|
||||
def _parse_client_dt(raw: object) -> datetime | None:
|
||||
if not isinstance(raw, str) or not raw:
|
||||
return None
|
||||
try:
|
||||
return datetime.fromisoformat(raw.replace("Z", "+00:00"))
|
||||
except ValueError:
|
||||
return None
|
||||
|
||||
|
||||
def client_wins(client_edited_at: datetime | None, server_edited_at: datetime | None) -> bool:
|
||||
"""Last-write-wins: the client's version is applied iff its edit-time is at least
|
||||
the server's. A missing client time never overwrites a real server edit; a missing
|
||||
@@ -179,7 +164,7 @@ def _assign_note_fields(note: Note, ch: dict) -> None:
|
||||
note.deleted_at = datetime.now(timezone.utc)
|
||||
else:
|
||||
note.deleted_at = None
|
||||
note.remind_at = _parse_client_dt(ch.get("remind_at"))
|
||||
note.remind_at = parse_dt(ch.get("remind_at"))
|
||||
note.recurrence = normalize_recurrence(ch.get("recurrence"))
|
||||
if isinstance(ch.get("position"), int):
|
||||
note.position = ch["position"]
|
||||
@@ -248,11 +233,15 @@ async def _apply_note(db, ch: dict) -> dict:
|
||||
except (ValueError, TypeError):
|
||||
return {"id": raw_id, "entity": "note", "status": "rejected", "error": "invalid id"}
|
||||
op = ch.get("op", "upsert")
|
||||
edited_at = _parse_client_dt(ch.get("edited_at"))
|
||||
edited_at = parse_dt(ch.get("edited_at"))
|
||||
|
||||
note = await db.scalar(select(Note).where(Note.id == nid))
|
||||
if note is not None and note.owner_id != g.user_id:
|
||||
return {"id": str(nid), "entity": "note", "status": "rejected", "error": "not yours"}
|
||||
# A client only ever pushes ids of notes IT created, so this branch is only
|
||||
# reached by a probe (or a ~0-probability UUID collision). Reject with a
|
||||
# GENERIC message so the response doesn't confirm the id belongs to another
|
||||
# user (don't leak existence via a distinctive "not yours").
|
||||
return {"id": str(nid), "entity": "note", "status": "rejected", "error": "cannot apply"}
|
||||
|
||||
if op == "delete":
|
||||
if note is None:
|
||||
@@ -267,7 +256,7 @@ async def _apply_note(db, ch: dict) -> dict:
|
||||
creating = note is None
|
||||
if creating:
|
||||
note = Note(id=nid, owner_id=g.user_id, body="", display_title="")
|
||||
created = _parse_client_dt(ch.get("created_at"))
|
||||
created = parse_dt(ch.get("created_at"))
|
||||
if created is not None:
|
||||
note.created_at = created
|
||||
db.add(note)
|
||||
@@ -309,11 +298,12 @@ async def _apply_label(db, ch: dict) -> dict:
|
||||
except (ValueError, TypeError):
|
||||
return {"id": raw_id, "entity": "label", "status": "rejected", "error": "invalid id"}
|
||||
op = ch.get("op", "upsert")
|
||||
edited_at = _parse_client_dt(ch.get("edited_at"))
|
||||
edited_at = parse_dt(ch.get("edited_at"))
|
||||
|
||||
label = await db.scalar(select(Label).where(Label.id == lid))
|
||||
if label is not None and label.owner_id != g.user_id:
|
||||
return {"id": str(lid), "entity": "label", "status": "rejected", "error": "not yours"}
|
||||
# Generic rejection (see _apply_note): don't confirm a foreign-owned id exists.
|
||||
return {"id": str(lid), "entity": "label", "status": "rejected", "error": "cannot apply"}
|
||||
|
||||
if op == "delete":
|
||||
if label is None:
|
||||
|
||||
@@ -8,7 +8,6 @@ from thoughtsync.sync import (
|
||||
MAX_LIMIT,
|
||||
_clamp_limit,
|
||||
_page_cursor,
|
||||
_parse_client_dt,
|
||||
_parse_since,
|
||||
client_wins,
|
||||
)
|
||||
@@ -42,14 +41,6 @@ def test_client_wins():
|
||||
assert client_wins(None, None) is True
|
||||
|
||||
|
||||
def test_parse_client_dt():
|
||||
assert _parse_client_dt("2026-07-22T00:00:00Z").year == 2026
|
||||
assert _parse_client_dt("2026-07-22T00:00:00+00:00").tzinfo is not None
|
||||
assert _parse_client_dt("garbage") is None
|
||||
assert _parse_client_dt(None) is None
|
||||
assert _parse_client_dt(123) is None
|
||||
|
||||
|
||||
def test_parse_since():
|
||||
assert _parse_since(None) == 0
|
||||
assert _parse_since("42") == 42
|
||||
|
||||
Reference in New Issue
Block a user