Review fixes: pairing by UID, bounded reads, notes per reader, per-resource tolerance #35

Merged

tiagoagueda merged 2 commits from sec2-fixes into main

2026-10-19 09:47:48 +00:00

tiagoagueda commented

2026-10-19 07:13:15 +00:00

Owner

The review fixes for postulo-dav: the open bug and security issues, apart from the three below, in two commits (the first is the dev environment and CI; the second is everything that touches dav.py, text.py and sync.py, which call into one another and were rewritten together).

Closes #7, #19, #20, #21, #22, #23, #24, #25, #26, #27, #28, #30, #31, #32, #33, #34

Refs #6, #11, #13 (left open, see below)

What and why, per issue

#31 (SEC2-048) The lock is re-resolved (pyjwt 2.15.1, urllib3 2.8.0, oauthlib 4.0.0, django 6.1.2, ...) and the core pin moves to 42be52048. pip-audit 2.10.1 on the exported lock reports nothing, with no ignore. CI gains the audit step (Postulo is left out of the export: it is installed from git and --strict would stop at it) and installs uv from a pinned version whose install.sh is checked against a sha256 (same values as the core), in ci.yml and release.yml.
#30 A contact's note is read and written for the connection's owner (with_notes, note_for, write_note); contacts are those visible_to them, and one they cannot write is not changed from the phone (the change is a note, once).
#19 Addresses are compared in one canonical form (decoded, then encoded with one fixed set), so Radicale's %40 and a stored @ are one resource; new events are named for the UID's own part.
#20 Names and companies are cut to 200; each card is handled in its own transaction and a failure is a note and a count; the contacts and the interviews are two halves and one failing does not stop the other (report.error carries it).
#21 The remote ETag is stored as soon as a lost conflict is recorded, so read-only mode and a failed push do not record it again on every run (contacts and interviews).
#22 What was typed in an event's description on the phone is pulled into the notes (Postulo's own lines are taken out by knowing exactly what they are; never added twice), and a push reads the event first and keeps it.
#23 Discovery asks the address as typed first, then /.well-known/…, then /.
#24 A TZID resolves through IANA, a table of the CLDR Windows names, or the document's own VTIMEZONE (when it names its location or has one fixed offset); otherwise, as for a floating time and an all-day event, the event has no instant and the interview is not moved; a note says why.
#25 REV is read in the extended form, with fractions and offsets; a card that gives no time wins and the report says it was chosen blind.
#26 A number taken elsewhere is a note and the card stays changed (the link keeps the old ETag, so the note repeats until the sides agree and a later edit here meets the conflict rule); a card with no TEL/URL leaves the number/link here alone.
#27 The vCard writer drops every control character and folds a lone CR, in every value, email and URL included; a test compares the rule with Postulo's own iCalendar writer so they cannot drift.
#28 A refused password (401/403) ends discovery at once as AuthenticationRefused with a translated sentence and the status on the error; the other messages are translated; test() and sync() show the sentence with no class name in front.
#32 A card's URL that is not an http(s) address is dropped with a note (plain and grouped item1.URL: forms).
#33 Every request is streamed through read_body to a size (2 MB a listing, 256 KB a card or event) and a deadline; client(total=60, follow_redirects=False); redirects are followed by hand and an address the server names (principal, home, listing, redirect) must stay on the configured host; a listing is read only for what is under the collection. An oversized resource is a skip with a note and the next is read.
#34 An impossible date is no date and not an exception; an end before the start does not move an interview; an address over 500 or a UID over 200 is a skip with a note; every pull and adoption is tolerant per resource.
#7 Pairing is the UID with the address as a cache: a UID is never rewritten (an adopted card keeps its own), a moved card is found again by it, a flagged one that comes back is paired again, a record whose twin is already there (connection deleted and added again) is paired with it (Postulo's UID with the same name, or an email address exactly one contact and one card share) instead of being pushed twice, and a server's refusal of one PUT is a note. The fake server now enforces no-uid-conflict. Not done, because it needs the core (#326/#327): a pair undone by hand staying undone.
#11 (the part that needs nothing from the core) A component stack in the parser, so an alarm's DESCRIPTION no longer replaces the notes; DURATION; the application's link in the event when POSTULO_PUBLIC_URL is set.
Core #963, postulo-dav half Events no longer carry ATTENDEE or the notes. The document says METHOD:PUBLISH. RFC 4791 §4.1 asks a calendar object resource to carry no METHOD, and some servers refuse one, so a PUT answered 400/403/415 is repeated once without it (tested). The test that asserted the attendee line now asserts there is none.

Tests

uv run pytest: 100 passed (the repo had 29 collected of which one third failed against the new core). Against the old sources the new tests fail (58 of 77 in test_dav.py). ruff check, ruff format --check, postulo-messages extract --check, check, guard all pass. pytest -m release was not run: the new strings are untranslated until Weblate's pull request.

Left open, and why

#6 needs the core's backup-destination plugin kind (core #325, not implemented).
#13 Collections chosen from the server by href with a mode each needs the core's form support for it (postulo/postulo#332). What the issue's security half asks and needs nothing else is in: credentials go only to the configured host, requests stay under the chosen collection, a server with no calendar home still syncs the contacts. Not done: the per-collection modes, the switches, the privilege-set check, items 2–3.
#11 Pairing an event made elsewhere with an interview, making an interview from an event, whole days as all-day events: they need core #330/#327/#326 and a page to choose on. The parser fixes and the link are in.

Notes for the reviewer

77 django.po catalogues changed and 7 appeared: postulo-messages extract against the new core pin (more languages) and the new strings. No translation is written here.
The commit is large on purpose: splitting sync.py per issue would give commits that do not pass their own tests.

🤖 Generated with Claude Code

The review fixes for postulo-dav: the open `bug` and `security` issues, apart from the three below, in two commits (the first is the dev environment and CI; the second is everything that touches `dav.py`, `text.py` and `sync.py`, which call into one another and were rewritten together). Closes #7, #19, #20, #21, #22, #23, #24, #25, #26, #27, #28, #30, #31, #32, #33, #34 Refs #6, #11, #13 (left open, see below) ## What and why, per issue - **#31** (SEC2-048) The lock is re-resolved (`pyjwt 2.15.1`, `urllib3 2.8.0`, `oauthlib 4.0.0`, `django 6.1.2`, ...) and the core pin moves to `42be52048`. `pip-audit` 2.10.1 on the exported lock reports nothing, with no ignore. CI gains the audit step (Postulo is left out of the export: it is installed from git and `--strict` would stop at it) and installs uv from a pinned version whose `install.sh` is checked against a sha256 (same values as the core), in `ci.yml` and `release.yml`. - **#30** A contact's note is read and written for the connection's owner (`with_notes`, `note_for`, `write_note`); contacts are those `visible_to` them, and one they cannot write is not changed from the phone (the change is a note, once). - **#19** Addresses are compared in one canonical form (decoded, then encoded with one fixed set), so Radicale's `%40` and a stored `@` are one resource; new events are named for the UID's own part. - **#20** Names and companies are cut to 200; each card is handled in its own transaction and a failure is a note and a count; the contacts and the interviews are two halves and one failing does not stop the other (`report.error` carries it). - **#21** The remote ETag is stored as soon as a lost conflict is recorded, so read-only mode and a failed push do not record it again on every run (contacts and interviews). - **#22** What was typed in an event's description on the phone is pulled into the notes (Postulo's own lines are taken out by knowing exactly what they are; never added twice), and a push reads the event first and keeps it. - **#23** Discovery asks the address as typed first, then `/.well-known/…`, then `/`. - **#24** A TZID resolves through IANA, a table of the CLDR Windows names, or the document's own VTIMEZONE (when it names its location or has one fixed offset); otherwise, as for a floating time and an all-day event, the event has no instant and the interview is not moved; a note says why. - **#25** `REV` is read in the extended form, with fractions and offsets; a card that gives no time wins and the report says it was chosen blind. - **#26** A number taken elsewhere is a note and the card stays changed (the link keeps the old ETag, so the note repeats until the sides agree and a later edit here meets the conflict rule); a card with no `TEL`/`URL` leaves the number/link here alone. - **#27** The vCard writer drops every control character and folds a lone CR, in every value, email and URL included; a test compares the rule with Postulo's own iCalendar writer so they cannot drift. - **#28** A refused password (401/403) ends discovery at once as `AuthenticationRefused` with a translated sentence and the status on the error; the other messages are translated; `test()` and `sync()` show the sentence with no class name in front. - **#32** A card's `URL` that is not an http(s) address is dropped with a note (plain and grouped `item1.URL:` forms). - **#33** Every request is streamed through `read_body` to a size (2 MB a listing, 256 KB a card or event) and a deadline; `client(total=60, follow_redirects=False)`; redirects are followed by hand and an address the server names (principal, home, listing, redirect) must stay on the configured host; a listing is read only for what is under the collection. An oversized resource is a skip with a note and the next is read. - **#34** An impossible date is no date and not an exception; an end before the start does not move an interview; an address over 500 or a UID over 200 is a skip with a note; every pull and adoption is tolerant per resource. - **#7** Pairing is the UID with the address as a cache: a UID is never rewritten (an adopted card keeps its own), a moved card is found again by it, a flagged one that comes back is paired again, a record whose twin is already there (connection deleted and added again) is paired with it (Postulo's UID with the same name, or an email address exactly one contact and one card share) instead of being pushed twice, and a server's refusal of one PUT is a note. The fake server now enforces `no-uid-conflict`. Not done, because it needs the core (#326/#327): a pair undone by hand staying undone. - **#11** (the part that needs nothing from the core) A component stack in the parser, so an alarm's `DESCRIPTION` no longer replaces the notes; `DURATION`; the application's link in the event when `POSTULO_PUBLIC_URL` is set. - **Core #963, postulo-dav half** Events no longer carry `ATTENDEE` or the notes. The document says `METHOD:PUBLISH`. RFC 4791 §4.1 asks a calendar object resource to carry no `METHOD`, and some servers refuse one, so a PUT answered 400/403/415 is repeated once without it (tested). The test that asserted the attendee line now asserts there is none. ## Tests `uv run pytest`: 100 passed (the repo had 29 collected of which one third failed against the new core). Against the old sources the new tests fail (58 of 77 in `test_dav.py`). `ruff check`, `ruff format --check`, `postulo-messages extract --check`, `check`, `guard` all pass. `pytest -m release` was not run: the new strings are untranslated until Weblate's pull request. ## Left open, and why - **#6** needs the core's backup-destination plugin kind (core #325, not implemented). - **#13** Collections chosen from the server by href with a mode each needs the core's form support for it (postulo/postulo#332). What the issue's security half asks and needs nothing else is in: credentials go only to the configured host, requests stay under the chosen collection, a server with no calendar home still syncs the contacts. Not done: the per-collection modes, the switches, the privilege-set check, items 2–3. - **#11** Pairing an event made elsewhere with an interview, making an interview from an event, whole days as all-day events: they need core #330/#327/#326 and a page to choose on. The parser fixes and the link are in. ## Notes for the reviewer - 77 `django.po` catalogues changed and 7 appeared: `postulo-messages extract` against the new core pin (more languages) and the new strings. No translation is written here. - The commit is large on purpose: splitting `sync.py` per issue would give commits that do not pass their own tests. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

tiagoagueda added 2 commits

2026-10-19 07:13:15 +00:00

Re-lock the dev environment, pin the core's current main, and audit the lock in CI 81fbedc013

The lock had never been re-resolved: pyjwt 2.13.0 (14 advisories), urllib3 2.7.0 (3),
oauthlib 3.3.1 (1) and django 6.1.1 (4 fixes in 6.1.2) sat in it. `uv lock --upgrade`
brings pyjwt 2.15.1, urllib3 2.8.0, oauthlib 4.0.0 (with the django-allauth that wants it)
and django 6.1.2, and the core pin moves to 42be52048, the head of postulo/postulo main.
pip-audit 2.10.1 against the exported lock now reports nothing, with no ignore.

CI gets the core's audit step (Postulo itself is left out of the export: it is installed
from git and is not on PyPI, so --strict would stop at it) and installs uv from a pinned
version whose install.sh is checked against a sha256 before it runs, in both ci.yml and
release.yml, instead of piping whatever astral.sh answers into sh.

Closes #31

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Review fixes: pairing by UID, bounded reads, per-resource tolerance, notes per reader, and a calendar that carries no attendee

CI / test (pull_request) Successful in 2m29s

Details

22b5120a07

One change, because the client, the two readers and the sync call into one another and
none of it ran apart: the dev environment first did not run at all against the core it
was pinned to (a contact's note is read for somebody since postulo/postulo#896).

The client (dav.py)
- An address the server names is followed only on the host that was typed, and a listing
  is read only for what lies under the collection (no other host, no `..`); redirects are
  followed by hand, hop by hop, under the same rule. The client opens with `total=60`
  and `follow_redirects=False` (#33, #13).
- Every answer is streamed through `read_body` to a size (2 MB a listing, 256 KB a card or
  an event, 64 KB the rest) and a deadline; `BodyRefused` is a `DavError` naming the
  resource (#33).
- One canonical spelling of an address, so Radicale's `%40` and a stored `@` are one
  resource (#19), and new events are named for the UID's own part, with no reserved
  character in it.
- Discovery asks the address as typed first, then `/.well-known/…`, then `/`; a 401 or a
  403 ends it at once as an `AuthenticationRefused` with a translated sentence, and every
  other message is translated too (#23, #28). `test` and `sync` show the sentence, with no
  class name in front.

The readers (text.py)
- The vCard writer drops every control character and folds a lone CR into a newline in
  every value, email and URL included, as Postulo's own writer does (#27).
- REV is read in the extended form, with fractions and offsets (#25); an impossible date
  is no date and not an exception (#34); DTSTART/DTEND are an instant only in UTC or in a
  zone that can be found (IANA, the CLDR Windows names, the document's own VTIMEZONE where
  it names its location or has one fixed offset), never UTC by default; a floating time
  and an all-day event are not instants (#24); components are tracked, so an alarm's
  DESCRIPTION is the alarm's (#11); DURATION ends an event with no end.

The sync (sync.py)
- Notes are read and written for the connection's owner, contacts are those visible to
  them, and those not writable are not changed from the phone (#30, core #896).
- A pairing is the UID, the address a cache: a card or event that moved is found again
  and a flagged one that comes back is paired again; a record whose twin is already there
  (a connection deleted and added again) is paired with it, by Postulo's UID with the same
  name or by an email address exactly one contact and one card share; a UID is never
  rewritten; a server's refusal of one PUT is a note (#7).
- A card or event that cannot be read or saved is skipped with a note, inside its own
  transaction, and the next is tried; a name or company over 200 characters is cut; an
  address over 500 or a UID over 200 is a skip, not a cut; the contacts and the
  interviews are two halves and one failing does not stop the other (#20, #34).
- A conflict that ours wins is remembered at once, so read-only mode and a failed push
  do not record it again on every run (#21).
- What was typed in an event's description on the phone is pulled into the notes (Postulo's
  own lines are told apart by knowing exactly what they are) and a push keeps it (#22).
- A taken number is a note and the card stays changed; a card with no TEL or URL leaves
  the number and the link here alone (#26); a URL that is not http(s) is dropped with a
  note (#32).
- An unresolvable zone, a floating time, an all-day event and an end before the start do
  not move an interview and are a note (#24, #34).
- The event carries the application's link when POSTULO_PUBLIC_URL is set, and, with
  postulo/postulo#963 now in the core, no attendee and no notes: the document says
  METHOD:PUBLISH. RFC 4791 §4.1 asks a calendar object to carry none, so a server that
  answers 400, 403 or 415 is written to again without it.

Strings are extracted for Weblate; no translation is written here.

Closes #19, #20, #21, #22, #23, #24, #25, #26, #27, #28, #30, #32, #33, #34
Refs #7, #11, #13

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>