kayg/calternal
Watch
1
0
Fork
You've already forked calternal
 0
Code Issues 191 Pull requests 61 Projects Releases Packages Wiki Activity Actions 20

Typed DAV recorder and replay v2 #820

Closed
kayg wants to merge 40 commits from feat/dav-recorder-v2 into dev
pull from: feat/dav-recorder-v2
merge into: kayg:dev
Conversation 8 Commits 40 Files changed 21 +6761
kayg commented 2026-10-31 23:43:51 +00:00
Owner
Copy link

Implements the approved v2 plan in #818 and supersedes #789.

What changed

  • Replaces raw exchange persistence with typed allowlisted request and response records and in-memory per-capture placeholder maps.
  • Bounds bodies, connections, deadlines, and XML/card nesting; aborts capture and removes partial output on schema or synthetic-data failures.
  • Checks decoded card and calendar data inside XML and normalizes percent-encoded URI values before placeholder binding.
  • Replays against target-owned identifiers and checks sync-token progression, ETag/If-Match flows, stale-write 412s, and multistatus namespace, order, and shape.
  • Adds randomized no-verbatim coverage for identifiers in every fixture position, including percent, double-percent, URL-safe Base64, and uppercase forms.

Gates on dev-02

  • cargo fmt --all --check passed.
  • cargo clippy -j4 -p calternal-dav-replay --locked -- -D warnings passed.
  • cargo test -j4 -p calternal-dav-replay --locked passed.
  • cargo run -j4 -p calternal-dav-replay --locked -- check crates/dav-replay/fixtures returns PENDING (exit 3) for contacts-crud and contacts-initial-sync.

The real-capture replay gate remains PENDING because a real Mac capture is blocked on Mac GUI permission. No Mac capture was run.

Deferred minor: main.rs may say partial output was removed even when cleanup itself fails.

Implements the approved v2 plan in #818 and supersedes #789. ## What changed - Replaces raw exchange persistence with typed allowlisted request and response records and in-memory per-capture placeholder maps. - Bounds bodies, connections, deadlines, and XML/card nesting; aborts capture and removes partial output on schema or synthetic-data failures. - Checks decoded card and calendar data inside XML and normalizes percent-encoded URI values before placeholder binding. - Replays against target-owned identifiers and checks sync-token progression, ETag/If-Match flows, stale-write 412s, and multistatus namespace, order, and shape. - Adds randomized no-verbatim coverage for identifiers in every fixture position, including percent, double-percent, URL-safe Base64, and uppercase forms. ## Gates on dev-02 - `cargo fmt --all --check` passed. - `cargo clippy -j4 -p calternal-dav-replay --locked -- -D warnings` passed. - `cargo test -j4 -p calternal-dav-replay --locked` passed. - `cargo run -j4 -p calternal-dav-replay --locked -- check crates/dav-replay/fixtures` returns `PENDING` (exit 3) for contacts-crud and contacts-initial-sync. The real-capture replay gate remains PENDING because a real Mac capture is blocked on Mac GUI permission. No Mac capture was run. Deferred minor: `main.rs` may say partial output was removed even when cleanup itself fails.
kayg added 40 commits 2026-10-31 23:43:51 +00:00
perf(dav-replay): index unique target bindings
Some checks failed
CI / server (pull_request) Has been cancelled
Details
CI / rust (pull_request) Has been cancelled
Details
CI / web (pull_request) Has been cancelled
Details
565b50e397
kayg commented 2026-10-31 23:51:43 +00:00
Author
Owner
Copy link

Independent review, round 1

Head: 565b50e3. Read the approved #818 plan and all three #789 review rounds.

Verdict: fix first.

P0

  • None found.

P1

  1. Initial response data can establish an incorrect expected value, including duplicate UIDs. schema.rs:1473–1495 puts UID and all other vCard values into generic CardValue slots; wire.rs:50–98,1121,1404 learns unseeded values without enforcing distinct UID identities. Independent HTTP replay probes changed only FN from Test One to Test Two, or collapsed two distinct UIDs into one: both reported ok. During initial sync, an incorrect contact or duplicate identity can therefore become the replay binding and pass. Require independently seeded expectations for synthetic contact content, give UID its own correlation rules, and reject distinct identity slots collapsing to one value. Preserve privacy through target seed data rather than storing captured values.

  2. Broken live hrefs are repaired by normalization and request rendering. schema.rs:687–708 drops an optional /dav prefix and merges contacts with addressbook; wire.rs:209 always reconstructs canonical target paths. With principal/resource bindings seeded, changing only a response href from /dav/u/contacts/a.vcf to /u/contacts/a.vcf, or to /dav/u/addressbook/a.vcf, still reported ok. If those alternate paths are not served, Apple follows the advertised URL and gets a failure while replay silently uses its reconstructed URL. Validate target response hrefs against the actual target routes and follow/check the advertised target href. Source-to-target route translation must not erase errors in the target's own response.

  3. Ordinary DAV capability responses stop recording. schema.rs:434–531 neither models nor explicitly ignores DAV and Allow. A normal OPTIONS response containing these headers produces unknown header in response; the writer aborts/removes the session. The documented Apple-to-Radicale capture can therefore stop during capability discovery before producing a fixture. Add typed capability/method-list support and a recorder regression using a representative OPTIONS exchange.

P2

  1. Protocol values are treated as arbitrary XML text. schema.rs:1178–1199 placeholders DAV content lengths and falls back to generic text for getcontenttype. Independent HTTP replay probes changing only length 123 → 999999 or media type text/vcard → text/calendar both reported ok. Preserve media types as protocol enums; validate advertised lengths against the relevant target representation or an explicit derived expectation. Rebinding any first-seen number does not check correctness.

  2. ETag syntax and strength are lost inside an opaque binding. schema.rs:579–587 and wire.rs:947 accept an arbitrary nonempty ETag slot. Changing only "one" to W/"one" reported ok. Weak versus strong changes If-Match semantics, even when later opaque-value correlations remain consistent. Parse and preserve ETag strength and syntax, with a mutation regression.

P3

  1. The deferred cleanup report is inaccurate. main.rs:137 replaces every recorder error with “partial output was removed”, including cleanup failures and startup failures where an existing directory was deliberately retained. Preserve the recorder's error and report removal only after successful cleanup. Otherwise someone may rely on the message while partial files remain.

Checks and limits

  • On dev-01, detached uncommitted worktree ~/Developer/Worktrees/review820-r1: nix develop -c bash -c '~/bin/slot cargo test -j4 -p calternal-dav-replay --locked -- --nocapture' passed, including all 38 committed tests and the initial seven probes.
  • Final targeted command: nix develop -c bash -c '~/bin/slot cargo test -j4 -p calternal-dav-replay --test review_r1_probe --locked -- --nocapture': 10/10 probe assertions passed. Seven response mutations were accepted incorrectly; numeric 412→204 and XML-shape mutations were rejected; the remaining probe confirmed capability-header rejection. Probe assertions deliberately assert these observed outcomes, so a green probe suite does not mean replay caught all mutations.
  • Existing tests also reject repeated sync tokens and seeded resource-order changes, verify target-owned If-Match and stale-write 412 flows, and exercise percent-encoded href bindings.
  • Resource review found 4 MiB body collection caps, eight connections, client/upstream/shutdown deadlines, XML/card nesting checks, and DTD/entity rejection. Embedded XML card text is parsed structurally and checked for synthetic data. Corresponding committed limit/deadline/depth tests passed. The lifetime placeholder map remains uncapped, explicitly acknowledged in #818; the per-exchange limits do not bound total session memory.
  • Committed fixture inventory contains only required-scenarios.json; no captured personal data or credentials are committed there. Typed serialization omits auth payloads/cookies and replaces variable content, with the randomized no-verbatim tests passing.
  • nix develop -c bash -c '~/bin/slot cargo run -j4 -p calternal-dav-replay --locked -- check crates/dav-replay/fixtures' reports PENDING for contacts-crud and contacts-initial-sync. No real Mac capture was run. No code committed or pushed; probe source/logs remain on the VM.
## Independent review, round 1 Head: `565b50e3`. Read the approved #818 plan and all three #789 review rounds. **Verdict: fix first.** ### P0 - None found. ### P1 1. **Initial response data can establish an incorrect expected value, including duplicate UIDs.** `schema.rs:1473–1495` puts UID and all other vCard values into generic `CardValue` slots; `wire.rs:50–98,1121,1404` learns unseeded values without enforcing distinct UID identities. Independent HTTP replay probes changed only FN from Test One to Test Two, or collapsed two distinct UIDs into one: both reported `ok`. During initial sync, an incorrect contact or duplicate identity can therefore become the replay binding and pass. Require independently seeded expectations for synthetic contact content, give UID its own correlation rules, and reject distinct identity slots collapsing to one value. Preserve privacy through target seed data rather than storing captured values. 2. **Broken live hrefs are repaired by normalization and request rendering.** `schema.rs:687–708` drops an optional `/dav` prefix and merges `contacts` with `addressbook`; `wire.rs:209` always reconstructs canonical target paths. With principal/resource bindings seeded, changing only a response href from `/dav/u/contacts/a.vcf` to `/u/contacts/a.vcf`, or to `/dav/u/addressbook/a.vcf`, still reported `ok`. If those alternate paths are not served, Apple follows the advertised URL and gets a failure while replay silently uses its reconstructed URL. Validate target response hrefs against the actual target routes and follow/check the advertised target href. Source-to-target route translation must not erase errors in the target's own response. 3. **Ordinary DAV capability responses stop recording.** `schema.rs:434–531` neither models nor explicitly ignores `DAV` and `Allow`. A normal OPTIONS response containing these headers produces `unknown header in response`; the writer aborts/removes the session. The documented Apple-to-Radicale capture can therefore stop during capability discovery before producing a fixture. Add typed capability/method-list support and a recorder regression using a representative OPTIONS exchange. ### P2 4. **Protocol values are treated as arbitrary XML text.** `schema.rs:1178–1199` placeholders DAV content lengths and falls back to generic text for `getcontenttype`. Independent HTTP replay probes changing only length `123 → 999999` or media type `text/vcard → text/calendar` both reported `ok`. Preserve media types as protocol enums; validate advertised lengths against the relevant target representation or an explicit derived expectation. Rebinding any first-seen number does not check correctness. 5. **ETag syntax and strength are lost inside an opaque binding.** `schema.rs:579–587` and `wire.rs:947` accept an arbitrary nonempty ETag slot. Changing only `"one"` to `W/"one"` reported `ok`. Weak versus strong changes If-Match semantics, even when later opaque-value correlations remain consistent. Parse and preserve ETag strength and syntax, with a mutation regression. ### P3 6. **The deferred cleanup report is inaccurate.** `main.rs:137` replaces every recorder error with “partial output was removed”, including cleanup failures and startup failures where an existing directory was deliberately retained. Preserve the recorder's error and report removal only after successful cleanup. Otherwise someone may rely on the message while partial files remain. ### Checks and limits - On dev-01, detached uncommitted worktree `~/Developer/Worktrees/review820-r1`: `nix develop -c bash -c '~/bin/slot cargo test -j4 -p calternal-dav-replay --locked -- --nocapture'` passed, including all 38 committed tests and the initial seven probes. - Final targeted command: `nix develop -c bash -c '~/bin/slot cargo test -j4 -p calternal-dav-replay --test review_r1_probe --locked -- --nocapture'`: 10/10 probe assertions passed. Seven response mutations were accepted incorrectly; numeric 412→204 and XML-shape mutations were rejected; the remaining probe confirmed capability-header rejection. Probe assertions deliberately assert these observed outcomes, so a green probe suite does not mean replay caught all mutations. - Existing tests also reject repeated sync tokens and seeded resource-order changes, verify target-owned If-Match and stale-write 412 flows, and exercise percent-encoded href bindings. - Resource review found 4 MiB body collection caps, eight connections, client/upstream/shutdown deadlines, XML/card nesting checks, and DTD/entity rejection. Embedded XML card text is parsed structurally and checked for synthetic data. Corresponding committed limit/deadline/depth tests passed. The lifetime placeholder map remains uncapped, explicitly acknowledged in #818; the per-exchange limits do not bound total session memory. - Committed fixture inventory contains only `required-scenarios.json`; no captured personal data or credentials are committed there. Typed serialization omits auth payloads/cookies and replaces variable content, with the randomized no-verbatim tests passing. - `nix develop -c bash -c '~/bin/slot cargo run -j4 -p calternal-dav-replay --locked -- check crates/dav-replay/fixtures'` reports **PENDING** for contacts-crud and contacts-initial-sync. No real Mac capture was run. No code committed or pushed; probe source/logs remain on the VM.
kayg commented 2026-11-01 00:23:15 +00:00
Author
Owner
Copy link

Round 1 findings → fixes → tests

  • Contact values and UIDs: Replay now compares card values field by field and requires a bijection for UID placeholder bindings. Mutation tests reject changed values and collapsed UIDs.
  • Href structure: Replay compares the DAV prefix, collection path, and member structure. Mutation tests reject changed prefixes and collection paths.
  • DAV/Allow headers: Standard capability headers are typed and allowlisted. The recorder test covers them; unknown headers still abort capture and remove partial output.
  • Protocol checks: ETag syntax, strength, and identity mapping are checked; content type/length and timestamps use typed fields. Replay tests cover sync-token progression, ETag/If-Match, 412 responses, and multistatus shape.
  • Identifier privacy: The property test exercises random identifiers and percent, double-percent, base64, and case variants in every position; no tested value appears in fixture bytes.

Dev-02 gates: fmt, clippy with -D warnings, and crate tests pass. The real-capture replay gate remains PENDING until a Mac GUI capture is available.

Round 1 findings → fixes → tests - **Contact values and UIDs:** Replay now compares card values field by field and requires a bijection for UID placeholder bindings. Mutation tests reject changed values and collapsed UIDs. - **Href structure:** Replay compares the DAV prefix, collection path, and member structure. Mutation tests reject changed prefixes and collection paths. - **DAV/Allow headers:** Standard capability headers are typed and allowlisted. The recorder test covers them; unknown headers still abort capture and remove partial output. - **Protocol checks:** ETag syntax, strength, and identity mapping are checked; content type/length and timestamps use typed fields. Replay tests cover sync-token progression, ETag/If-Match, 412 responses, and multistatus shape. - **Identifier privacy:** The property test exercises random identifiers and percent, double-percent, base64, and case variants in every position; no tested value appears in fixture bytes. Dev-02 gates: fmt, clippy with -D warnings, and crate tests pass. The real-capture replay gate remains PENDING until a Mac GUI capture is available.
kayg commented 2026-11-01 00:30:32 +00:00
Author
Owner
Copy link

Independent review, round 2

Head: 3721d48e8c951819e0fc258c20feb58bf3fd1200. Reviewed against #818, #issuecomment-72055 and #issuecomment-72112.

Verdict: fix first. The contact/UID and route fixes work in independently seeded HTTP replay probes. The capability-header fix is incomplete for the documented Radicale capture source.

Status and mutation probes

Each replay case first passed an unchanged baseline with all required synthetic content/identity seeds. Then only one server response was changed. Multi-exchange probes retained the other responses.

Check Result Replay message / evidence
Contact field: FN Test One → Test Two Caught; round-1 P1 fixed target placeholder binding changed during replay
Swap two contact UIDs Caught; round-1 P1 fixed target placeholder binding changed during replay
Collapse two distinct UIDs Caught target placeholder binding changed during replay
Remove response href /dav prefix Caught; round-1 P1 fixed response URI route shape differs
Change contacts → addressbook collection path Caught; round-1 P1 fixed response URI route shape differs
Sync token fails to advance Caught at exchange 2 sync-token did not progress
Reuse previous ETag on the change PUT response Caught at exchange 2 target returned a duplicate identifier binding
Stale PUT returns 204 instead of 412 Caught status differs: expected 412, received 204
Strong ETag becomes weak Caught; round-1 P2 fixed response ETag shape differs
Unquoted ETag Caught invalid ETag syntax
DAV content type text/vcard → text/calendar Caught DAV XML value differs
Seeded DAV content length 123 → 999 Caught; round-1 P2 fixed for length target placeholder binding changed during replay
REV → 20269999T999999Z Missed replay reports ok
creationdate → 2026-99-99T99:99:99Z Missed replay reports ok
getlastmodified → Abc, 99 Jan 2026 99:99:99 GMT Missed replay reports ok
DAV content-type charset utf-8 → us-ascii Missed replay reports ok
Previously tested DAV/Allow subset Accepted HTTP replay control and committed recorder test pass
Radicale DAV including extended-mkcol Capture aborts; round-1 P1 remains recorder returns 502, unknown DAV capability, session directory removed
Radicale Allow including MKCALENDAR Capture aborts; round-1 P1 remains recorder returns 502, unknown HTTP method, session directory removed

Response mutations: 12 caught / 16 tried. The two capability failures are separate recorder compatibility probes, not mutations that replay ought to reject.

Findings

P0: None found.

P1: Capability support still stops the documented Radicale capture. schema.rs:628–647 excludes extended-mkcol, and Method::parse excludes MKCALENDAR. Radicale's DAV_HEADERS includes extended-mkcol; OPTIONS builds Allow from its implemented methods, including MKCALENDAR. Independent recorder probes changed only one header at a time from the accepted baseline and confirmed both failures and cleanup. A Contacts capture can encounter these advertised values even if it never creates a calendar. Model these known protocol values and test representative source headers; retain rejection of unknown values.

P2: Timestamp typing validates layout but accepts impossible dates/times. schema.rs:1341–1395,1713–1722 checks digit positions rather than date/time ranges or valid weekday names. All three impossible timestamp mutations above passed end-to-end replay. Validate calendar dates, clock ranges and the HTTP-date grammar before assigning a timestamp slot. Legitimate target timestamp variation can remain supported.

P2: DAV content-type typing discards charset semantics. schema.rs:582–608,1317–1322 accepts UTF-8 and US-ASCII but maps both to the same MediaType. Changing only the declaration therefore passes replay. These encodings differ for non-ASCII contact content. Preserve the charset in the typed value, or explicitly validate its compatibility with the target representation. This is new to XML getcontenttype typing; the earlier cross-family media-type mismatch is now caught.

P3, carried forward: main.rs:137 still replaces recorder errors with “partial output was removed” regardless of cleanup outcome. This remains the acknowledged round-1 minor finding.

Test quality: Several committed round-1 mutation tests use empty TargetBindings, so contact/UID and content-length cases can fail for missing seeds before reaching the mutation. Add passing seeded baselines and check the intended rejection reason. The independent probes above supply those seeds.

Verification and limits

  • Detached, uncommitted worktree on dev-02: /home/cua/Developer/Worktrees/review820-r2. dev-01's queued job was stopped before moving verification to an available dev-02 slot.
  • Full crate command: CARGO_TARGET_DIR=/home/cua/Developer/Worktrees/dav-recorder-v2/target nix develop -c bash -c '~/bin/slot cargo test -j4 -p calternal-dav-replay --locked -- --nocapture': 52 tests passed, including the 48 committed tests and four local probe tests.
  • After strengthening the local ETag/412 probes to send PUTs with If-Match, reran cargo test -j4 -p calternal-dav-replay --test review_r2_probe --locked -- --nocapture through the same Nix/slot setup: 2 passed, with the same 12/16 detection result.
  • Probe assertions explicitly encode the observed misses and recorder failures. A green probe suite does not mean every mutation was caught.
  • Local evidence: ~/review820-r2-final.log, ~/review820-r2-flow.log, tests/review_r2_probe.rs and appended recorder probes in tests/record.rs on dev-02.
  • No code committed or pushed. No real Mac capture; the real-capture gate remains PENDING.
## Independent review, round 2 Head: `3721d48e8c951819e0fc258c20feb58bf3fd1200`. Reviewed against #818, #issuecomment-72055 and #issuecomment-72112. **Verdict: fix first.** The contact/UID and route fixes work in independently seeded HTTP replay probes. The capability-header fix is incomplete for the documented Radicale capture source. ### Status and mutation probes Each replay case first passed an unchanged baseline with all required synthetic content/identity seeds. Then only one server response was changed. Multi-exchange probes retained the other responses. | Check | Result | Replay message / evidence | |---|---|---| | Contact field: FN Test One → Test Two | Caught; round-1 P1 fixed | target placeholder binding changed during replay | | Swap two contact UIDs | Caught; round-1 P1 fixed | target placeholder binding changed during replay | | Collapse two distinct UIDs | Caught | target placeholder binding changed during replay | | Remove response href /dav prefix | Caught; round-1 P1 fixed | response URI route shape differs | | Change contacts → addressbook collection path | Caught; round-1 P1 fixed | response URI route shape differs | | Sync token fails to advance | Caught at exchange 2 | sync-token did not progress | | Reuse previous ETag on the change PUT response | Caught at exchange 2 | target returned a duplicate identifier binding | | Stale PUT returns 204 instead of 412 | Caught | status differs: expected 412, received 204 | | Strong ETag becomes weak | Caught; round-1 P2 fixed | response ETag shape differs | | Unquoted ETag | Caught | invalid ETag syntax | | DAV content type text/vcard → text/calendar | Caught | DAV XML value differs | | Seeded DAV content length 123 → 999 | Caught; round-1 P2 fixed for length | target placeholder binding changed during replay | | REV → 20269999T999999Z | **Missed** | replay reports ok | | creationdate → 2026-99-99T99:99:99Z | **Missed** | replay reports ok | | getlastmodified → Abc, 99 Jan 2026 99:99:99 GMT | **Missed** | replay reports ok | | DAV content-type charset utf-8 → us-ascii | **Missed** | replay reports ok | | Previously tested DAV/Allow subset | Accepted | HTTP replay control and committed recorder test pass | | Radicale DAV including extended-mkcol | **Capture aborts; round-1 P1 remains** | recorder returns 502, unknown DAV capability, session directory removed | | Radicale Allow including MKCALENDAR | **Capture aborts; round-1 P1 remains** | recorder returns 502, unknown HTTP method, session directory removed | **Response mutations: 12 caught / 16 tried.** The two capability failures are separate recorder compatibility probes, not mutations that replay ought to reject. ### Findings **P0:** None found. **P1: Capability support still stops the documented Radicale capture.** `schema.rs:628–647` excludes extended-mkcol, and `Method::parse` excludes MKCALENDAR. Radicale's [DAV_HEADERS](https://github.com/Kozea/Radicale/blob/master/radicale/httputils.py) includes extended-mkcol; [OPTIONS](https://github.com/Kozea/Radicale/blob/master/radicale/app/options.py) builds Allow from its implemented methods, including [MKCALENDAR](https://github.com/Kozea/Radicale/blob/master/radicale/app/mkcalendar.py). Independent recorder probes changed only one header at a time from the accepted baseline and confirmed both failures and cleanup. A Contacts capture can encounter these advertised values even if it never creates a calendar. Model these known protocol values and test representative source headers; retain rejection of unknown values. **P2: Timestamp typing validates layout but accepts impossible dates/times.** `schema.rs:1341–1395,1713–1722` checks digit positions rather than date/time ranges or valid weekday names. All three impossible timestamp mutations above passed end-to-end replay. Validate calendar dates, clock ranges and the HTTP-date grammar before assigning a timestamp slot. Legitimate target timestamp variation can remain supported. **P2: DAV content-type typing discards charset semantics.** `schema.rs:582–608,1317–1322` accepts UTF-8 and US-ASCII but maps both to the same MediaType. Changing only the declaration therefore passes replay. These encodings differ for non-ASCII contact content. Preserve the charset in the typed value, or explicitly validate its compatibility with the target representation. This is new to XML getcontenttype typing; the earlier cross-family media-type mismatch is now caught. **P3, carried forward:** `main.rs:137` still replaces recorder errors with “partial output was removed” regardless of cleanup outcome. This remains the acknowledged round-1 minor finding. **Test quality:** Several committed round-1 mutation tests use empty TargetBindings, so contact/UID and content-length cases can fail for missing seeds before reaching the mutation. Add passing seeded baselines and check the intended rejection reason. The independent probes above supply those seeds. ### Verification and limits - Detached, uncommitted worktree on dev-02: `/home/cua/Developer/Worktrees/review820-r2`. dev-01's queued job was stopped before moving verification to an available dev-02 slot. - Full crate command: `CARGO_TARGET_DIR=/home/cua/Developer/Worktrees/dav-recorder-v2/target nix develop -c bash -c '~/bin/slot cargo test -j4 -p calternal-dav-replay --locked -- --nocapture'`: **52 tests passed**, including the 48 committed tests and four local probe tests. - After strengthening the local ETag/412 probes to send PUTs with If-Match, reran `cargo test -j4 -p calternal-dav-replay --test review_r2_probe --locked -- --nocapture` through the same Nix/slot setup: **2 passed**, with the same 12/16 detection result. - Probe assertions explicitly encode the observed misses and recorder failures. A green probe suite does not mean every mutation was caught. - Local evidence: `~/review820-r2-final.log`, `~/review820-r2-flow.log`, `tests/review_r2_probe.rs` and appended recorder probes in `tests/record.rs` on dev-02. - No code committed or pushed. No real Mac capture; the real-capture gate remains PENDING.
kayg commented 2026-11-01 01:08:02 +00:00
Author
Owner
Copy link

Round 3 status: finding → fix → test

  • Radicale capability headers — The DAV and Allow values are stored raw, with no closed-value parser that can reject extended-mkcol or MKCALENDAR. The recorder mutation test covers each header independently; the replay mutation test rejects a changed capability value.
  • Invalid timestamps — HTTP dates and ISO/card timestamps are validated for real calendar dates, clock ranges, and weekday consistency before normalization. replay_rejects_invalid_timestamps covers impossible values in headers, XML properties, and card REV.
  • Content-Type charset changes — The complete header and DAV property strings are retained for exact comparison, including charset parameters. replay_rejects_content_type_charset_changes rejects utf-8 → us-ascii.
  • Default header comparison and redaction — Other header values compare exactly; names use Hyper's lowercase canonical form. Authorization, cookies, proxy authorization, and names containing token or session are represented by placeholders. replay_requires_unknown_headers_to_match_exactly, sensitive_header_values_are_not_serialized, and headers_of_preserves_non_utf8_octets_reversibly cover these paths.

Dev-02 gates:

  • nix develop -c bash -c "cargo fmt --all --check" — pass.
  • nix develop -c /home/cua/bin/slot cargo clippy -j4 -p calternal-dav-replay --locked -- -D warnings — pass.
  • nix develop -c /home/cua/bin/slot cargo test -j4 -p calternal-dav-replay --locked — pass, 55 tests.
  • nix develop -c /home/cua/bin/slot cargo run -j4 -p calternal-dav-replay --locked -- check crates/dav-replay/fixtures — PENDING (exit 3); real captures remain blocked on Mac GUI permission.
## Round 3 status: finding → fix → test - **Radicale capability headers** — The `DAV` and `Allow` values are stored raw, with no closed-value parser that can reject `extended-mkcol` or `MKCALENDAR`. The recorder mutation test covers each header independently; the replay mutation test rejects a changed capability value. - **Invalid timestamps** — HTTP dates and ISO/card timestamps are validated for real calendar dates, clock ranges, and weekday consistency before normalization. `replay_rejects_invalid_timestamps` covers impossible values in headers, XML properties, and card `REV`. - **Content-Type charset changes** — The complete header and DAV property strings are retained for exact comparison, including charset parameters. `replay_rejects_content_type_charset_changes` rejects `utf-8` → `us-ascii`. - **Default header comparison and redaction** — Other header values compare exactly; names use Hyper's lowercase canonical form. Authorization, cookies, proxy authorization, and names containing `token` or `session` are represented by placeholders. `replay_requires_unknown_headers_to_match_exactly`, `sensitive_header_values_are_not_serialized`, and `headers_of_preserves_non_utf8_octets_reversibly` cover these paths. Dev-02 gates: - `nix develop -c bash -c "cargo fmt --all --check"` — pass. - `nix develop -c /home/cua/bin/slot cargo clippy -j4 -p calternal-dav-replay --locked -- -D warnings` — pass. - `nix develop -c /home/cua/bin/slot cargo test -j4 -p calternal-dav-replay --locked` — pass, 55 tests. - `nix develop -c /home/cua/bin/slot cargo run -j4 -p calternal-dav-replay --locked -- check crates/dav-replay/fixtures` — PENDING (exit 3); real captures remain blocked on Mac GUI permission.
kayg commented 2026-11-01 01:16:01 +00:00
Author
Owner
Copy link

Independent review, round 3 (final)

Head: 58e503944542a8a84b7e9a6a4389433a877fa099. Reviewed #818, #issuecomment-72055, #issuecomment-72126 and the simplified design in #issuecomment-72178. Verdict: fix first. This is the final review round; no further round is proposed.

Status

Each replay mutation first passed an unchanged baseline with the required synthetic identity/content bindings. Requests for the ETag change and stale-write cases retain PUT/If-Match. The harness includes HTTP-generated Date and Content-Length headers so strict comparison reaches the intended mutation.

Check Result
FN change; UID swap; UID collapse 3/3 caught
Remove href /dav; change contacts to addressbook 2/2 caught
Sync token does not advance; ETag reused after change PUT; missing 412 3/3 caught
Weak ETag; unquoted ETag 2/2 caught
DAV media type; seeded DAV content length 2/2 caught
Impossible REV; creationdate; getlastmodified 3/3 caught, all former misses fixed
DAV charset utf-8 to us-ascii 1/1 caught, former miss fixed
New unknown header added; unknown value changed; header removed 3/3 caught
Duplicate header added; repeated same-name values reversed 2/2 caught
Header-name case; order between different header names Both accepted correctly, equivalent controls
Redacted Set-Cookie value changed and Secure removed Missed
Redacted X-Token-Required: true changed to false Missed
Redacted Authorization scheme Basic to Bearer Caught
Body Content-Type charset utf-8 to us-ascii Caught
Live Radicale 3.8.3 OPTIONS /davtest/contacts/ 200, fixture saved; capability-header repair works
Live Radicale GET /.well-known/carddav Upstream 301, recorder 502; Location: / rejected as empty URI path segment
Live Radicale unauthenticated PROPFIND Upstream 401 text/plain, recorder 502; free-form error body rejected
Live Radicale authenticated depth-0 PROPFIND /davtest/ Upstream 207 XML, recorder 502; unknown namespace-qualified XML element
Saved-fixture credential probes Apple auth value, Digest nonce and URL credentials in Link all survive
Standard HTTPS Location; obsolete HTTP Date Both abort capture and remove the session
Own DAV CardDAV precondition response, source-derived wire probe Valid 403 {urn:ietf:params:xml:ns:carddav}no-uid-conflict aborts capture

Replay total: 23 caught / 25 meaningful differences tried. Round-2 set: 16/16, additions: 7/9. Two equivalent controls are excluded from that denominator. Recording/privacy probes are separate. A green probe suite asserts these observed outcomes, including misses; it does not mean every difference was caught.

New findings

P0: None found.

P1 — Credentials and session material are persisted by the raw-header fallback. schema.rs:543–566 redacts a few names and the substrings token/session, but leaves X-Apple-ID-Account-Web-Auth, X-Apple-Password, WWW-Authenticate and Authentication-Info raw. Serialization probes retained Apple credentials, Digest nonce and nextnonce values. More importantly, full recorder probes produced successfully loaded/saved fixture files containing the exact synthetic sentinel from an Apple auth header and a 401 Digest challenge. Link: <https://user:password@host/...> likewise saves URL credentials verbatim. HTTP Location userinfo is removed by URI templating, and Set-Cookie is redacted, but this protection does not cover URL-bearing raw headers. HTTPS Content-Location aborts instead. Expand the explicit sensitive field rules and parse/redact credentials and challenge/session scalars inside structured values; preserve nonsecret structure. Add persisted-fixture sentinel checks for each supported path. The current claim that fixtures cannot contain credentials is false.

P1 — Standard source responses still terminate recording, beyond the repaired capability headers. schema.rs:1002–1025, XML allowlisting, and parse_uri/parse_route still reject ordinary responses. With an actual disposable Radicale 3.8.3 server, OPTIONS now works, but well-known discovery returns Location: / and aborts; an ordinary initial 401 with a text/plain body aborts; authenticated depth-0 PROPFIND returns valid DAV/Radicale XML and aborts. The response includes standard principal-collection-set, report capability properties, root hrefs and a Radicale extension. Separately, the own-server response shape emitted by carddav-writes-r2 (crates/carddav/src/write.rs:90–95 through crates/dav/src/serve.rs:915–923) is a 403 CardDAV no-uid-conflict; feeding that exact shape through the recorder returns 502/unknown XML element. The recorder currently models no-uid-conflict only in the DAV namespace and only at 409 (schema.rs:445,1134). Standard HTTPS redirects also fail at schema.rs:660–661. Define safe representations for these normal discovery/authentication/protocol responses without persisting arbitrary bodies. Fixing DAV/Allow alone does not make the documented capture path usable.

P2 — Whole-value redaction hides protocol differences. wire.rs:971–996 learns the entire nonempty redacted value as an opaque slot. Changing Set-Cookie: sid=one; HttpOnly; Secure to sid=two; HttpOnly passes replay, hiding loss of Secure alongside legitimate session-ID variation. The substring rule also makes an unrelated boolean such as X-Token-Required opaque: true to false passes. Preserve and compare cookie names/attributes and other nonsecret structure; narrowly identify actual secret scalars. Authorization scheme changes are already caught.

P2 — Valid legacy HTTP dates abort. schema.rs:1352 accepts only the 29-character IMF-fixdate layout. A recorder probe with valid obsolete HTTP-date Sunday, 06-Nov-94 08:49:37 GMT returns 502/invalid HTTP Date header and removes the fixture. HTTP recipients must accept the obsolete date forms; normalize valid forms while retaining the new impossible-date checks.

P3, carried forward: The inaccurate unconditional “partial output was removed” message remains; this is the acknowledged earlier finding, not a new one.

Verification and limits

All verification ran on dev-02, detached worktree /home/cua/Developer/Worktrees/review820-r3; the control host remained read-only apart from the explicitly requested Forgejo comment. No code committed or pushed.

  • Full crate run before adding the live/wire recorder probes: CARGO_TARGET_DIR=/home/cua/Developer/Worktrees/dav-recorder-v2/target nix develop -c bash -c '~/bin/slot cargo test -j4 -p calternal-dav-replay --locked -- --nocapture': 58 passed, comprising 55 committed tests and 3 independent probe tests.
  • Recorder wire probes: same setup with cargo test -j4 -p calternal-dav-replay --test record --locked review_r3_standard_and_privacy -- --nocapture: 1 passed, six observed cases.
  • Live server probes: python3 ~/review820-r3-live.py launches disposable servers, invokes the same Nix/slot setup with cargo test -j4 -p calternal-dav-replay --test record --locked review_r3_live_servers -- --nocapture, then stops only its own child processes: 1 passed, five observed cases.
  • Evidence: ~/review820-r3-all.log, ~/review820-r3-record.log, ~/review820-r3-live.log, tests/review_r3_probe.rs and appended tests/record.rs probes on dev-02.
  • Own-server limit: the available carddav-r2 binary is not DAV-mounted. Its live well-known request returned an HTML 200, which also aborts capture, but this is not evidence of mounted own-DAV compatibility. The own-DAV 403 test above is a source-derived wire reproduction, not a full authenticated own-server capture. That compatibility remains unproven and already has a concrete schema mismatch.
  • No real Mac capture. The real-capture gate remains PENDING. No new fmt/clippy claim is made by this review.
## Independent review, round 3 (final) Head: `58e503944542a8a84b7e9a6a4389433a877fa099`. Reviewed #818, #issuecomment-72055, #issuecomment-72126 and the simplified design in #issuecomment-72178. **Verdict: fix first.** This is the final review round; no further round is proposed. ### Status Each replay mutation first passed an unchanged baseline with the required synthetic identity/content bindings. Requests for the ETag change and stale-write cases retain PUT/If-Match. The harness includes HTTP-generated Date and Content-Length headers so strict comparison reaches the intended mutation. | Check | Result | |---|---| | FN change; UID swap; UID collapse | 3/3 caught | | Remove href `/dav`; change contacts to addressbook | 2/2 caught | | Sync token does not advance; ETag reused after change PUT; missing 412 | 3/3 caught | | Weak ETag; unquoted ETag | 2/2 caught | | DAV media type; seeded DAV content length | 2/2 caught | | Impossible REV; creationdate; getlastmodified | 3/3 caught, all former misses fixed | | DAV charset utf-8 to us-ascii | 1/1 caught, former miss fixed | | New unknown header added; unknown value changed; header removed | 3/3 caught | | Duplicate header added; repeated same-name values reversed | 2/2 caught | | Header-name case; order between different header names | Both accepted correctly, equivalent controls | | Redacted Set-Cookie value changed and Secure removed | **Missed** | | Redacted `X-Token-Required: true` changed to `false` | **Missed** | | Redacted Authorization scheme Basic to Bearer | Caught | | Body Content-Type charset utf-8 to us-ascii | Caught | | Live Radicale 3.8.3 OPTIONS `/davtest/contacts/` | 200, fixture saved; capability-header repair works | | Live Radicale GET `/.well-known/carddav` | Upstream 301, recorder 502; `Location: /` rejected as empty URI path segment | | Live Radicale unauthenticated PROPFIND | Upstream 401 text/plain, recorder 502; free-form error body rejected | | Live Radicale authenticated depth-0 PROPFIND `/davtest/` | Upstream 207 XML, recorder 502; unknown namespace-qualified XML element | | Saved-fixture credential probes | Apple auth value, Digest nonce and URL credentials in Link all survive | | Standard HTTPS Location; obsolete HTTP Date | Both abort capture and remove the session | | Own DAV CardDAV precondition response, source-derived wire probe | Valid 403 `{urn:ietf:params:xml:ns:carddav}no-uid-conflict` aborts capture | **Replay total: 23 caught / 25 meaningful differences tried.** Round-2 set: **16/16**, additions: **7/9**. Two equivalent controls are excluded from that denominator. Recording/privacy probes are separate. A green probe suite asserts these observed outcomes, including misses; it does not mean every difference was caught. ### New findings **P0:** None found. **P1 — Credentials and session material are persisted by the raw-header fallback.** `schema.rs:543–566` redacts a few names and the substrings token/session, but leaves `X-Apple-ID-Account-Web-Auth`, `X-Apple-Password`, `WWW-Authenticate` and `Authentication-Info` raw. Serialization probes retained Apple credentials, Digest nonce and nextnonce values. More importantly, full recorder probes produced successfully loaded/saved fixture files containing the exact synthetic sentinel from an Apple auth header and a 401 Digest challenge. `Link: <https://user:password@host/...>` likewise saves URL credentials verbatim. HTTP Location userinfo is removed by URI templating, and Set-Cookie is redacted, but this protection does not cover URL-bearing raw headers. HTTPS Content-Location aborts instead. Expand the explicit sensitive field rules and parse/redact credentials and challenge/session scalars inside structured values; preserve nonsecret structure. Add persisted-fixture sentinel checks for each supported path. The current claim that fixtures cannot contain credentials is false. **P1 — Standard source responses still terminate recording, beyond the repaired capability headers.** `schema.rs:1002–1025`, XML allowlisting, and `parse_uri`/`parse_route` still reject ordinary responses. With an actual disposable Radicale 3.8.3 server, OPTIONS now works, but well-known discovery returns `Location: /` and aborts; an ordinary initial 401 with a text/plain body aborts; authenticated depth-0 PROPFIND returns valid DAV/Radicale XML and aborts. The response includes standard principal-collection-set, report capability properties, root hrefs and a Radicale extension. Separately, the own-server response shape emitted by `carddav-writes-r2` (`crates/carddav/src/write.rs:90–95` through `crates/dav/src/serve.rs:915–923`) is a 403 CardDAV no-uid-conflict; feeding that exact shape through the recorder returns 502/unknown XML element. The recorder currently models no-uid-conflict only in the DAV namespace and only at 409 (`schema.rs:445,1134`). Standard HTTPS redirects also fail at `schema.rs:660–661`. Define safe representations for these normal discovery/authentication/protocol responses without persisting arbitrary bodies. Fixing DAV/Allow alone does not make the documented capture path usable. **P2 — Whole-value redaction hides protocol differences.** `wire.rs:971–996` learns the entire nonempty redacted value as an opaque slot. Changing `Set-Cookie: sid=one; HttpOnly; Secure` to `sid=two; HttpOnly` passes replay, hiding loss of Secure alongside legitimate session-ID variation. The substring rule also makes an unrelated boolean such as X-Token-Required opaque: true to false passes. Preserve and compare cookie names/attributes and other nonsecret structure; narrowly identify actual secret scalars. Authorization scheme changes are already caught. **P2 — Valid legacy HTTP dates abort.** `schema.rs:1352` accepts only the 29-character IMF-fixdate layout. A recorder probe with valid obsolete HTTP-date `Sunday, 06-Nov-94 08:49:37 GMT` returns 502/invalid HTTP Date header and removes the fixture. HTTP recipients must accept the obsolete date forms; normalize valid forms while retaining the new impossible-date checks. **P3, carried forward:** The inaccurate unconditional “partial output was removed” message remains; this is the acknowledged earlier finding, not a new one. ### Verification and limits All verification ran on dev-02, detached worktree `/home/cua/Developer/Worktrees/review820-r3`; the control host remained read-only apart from the explicitly requested Forgejo comment. No code committed or pushed. - Full crate run before adding the live/wire recorder probes: `CARGO_TARGET_DIR=/home/cua/Developer/Worktrees/dav-recorder-v2/target nix develop -c bash -c '~/bin/slot cargo test -j4 -p calternal-dav-replay --locked -- --nocapture'`: **58 passed**, comprising 55 committed tests and 3 independent probe tests. - Recorder wire probes: same setup with `cargo test -j4 -p calternal-dav-replay --test record --locked review_r3_standard_and_privacy -- --nocapture`: **1 passed**, six observed cases. - Live server probes: `python3 ~/review820-r3-live.py` launches disposable servers, invokes the same Nix/slot setup with `cargo test -j4 -p calternal-dav-replay --test record --locked review_r3_live_servers -- --nocapture`, then stops only its own child processes: **1 passed**, five observed cases. - Evidence: `~/review820-r3-all.log`, `~/review820-r3-record.log`, `~/review820-r3-live.log`, `tests/review_r3_probe.rs` and appended `tests/record.rs` probes on dev-02. - Own-server limit: the available `carddav-r2` binary is not DAV-mounted. Its live well-known request returned an HTML 200, which also aborts capture, but this is **not** evidence of mounted own-DAV compatibility. The own-DAV 403 test above is a source-derived wire reproduction, not a full authenticated own-server capture. That compatibility remains unproven and already has a concrete schema mismatch. - No real Mac capture. The real-capture gate remains PENDING. No new fmt/clippy claim is made by this review.
kayg commented 2026-11-01 01:26:06 +00:00
Author
Owner
Copy link

Paused at 58e503944542a8a84b7e9a6a4389433a877fa099 under the three-round rule, pending review and approval of plan v3 in #818. No round 4 or further recorder patches are proposed.

Plan v3 separates stock Nix mitmproxy raw capture from offline allowlist export, and retains the typed replay/comparison work and strengthened mutation tests from this PR. It covers every P1 from rounds 1–3, including standard source responses, Apple/Digest/Link credential persistence, independently seeded contact/UID checks, target href correctness, and the source-derived own-DAV CardDAV no-uid-conflict 403 mismatch. The mounted own-server gate and real Mac capture remain unproven.

Design only: no implementation code or fixtures changed; no build, test or capture run. The v3 description supersedes the older v2 plan file on #818’s branch.

Paused at `58e503944542a8a84b7e9a6a4389433a877fa099` under the three-round rule, pending review and approval of [plan v3 in #818](https://git.kayg.org/kayg/calternal/pulls/818). No round 4 or further recorder patches are proposed. Plan v3 separates stock Nix mitmproxy raw capture from offline allowlist export, and retains the typed replay/comparison work and strengthened mutation tests from this PR. It covers every P1 from rounds 1–3, including standard source responses, Apple/Digest/Link credential persistence, independently seeded contact/UID checks, target href correctness, and the source-derived own-DAV CardDAV no-uid-conflict 403 mismatch. The mounted own-server gate and real Mac capture remain unproven. Design only: no implementation code or fixtures changed; no build, test or capture run. The v3 description supersedes the older v2 plan file on #818’s branch.
kayg commented 2026-11-01 02:29:09 +00:00
Author
Owner
Copy link

Superseded as the v3 work destination by #822, following approved #818 (revision 72227 and review 72220). #820 remains paused at 58e5039445; no further v2 recorder patches or review round are proposed. The v3 task explicitly requires stopping for a material size overrun: source inspection estimates 340–550 incremental Rust changed lines versus 100–180 for closed header representations and the dependent replay normalization/rendering, URI origins, omitted error bodies and recursive loader/slot checks. #822 records that pre-implementation estimate and all gates as NOT RUN (real Apple capture PENDING). It is a blocked draft, not a completed working replacement; no implementation or captures were produced.

Superseded as the v3 work destination by #822, following approved #818 (revision 72227 and review 72220). #820 remains paused at 58e503944542a8a84b7e9a6a4389433a877fa099; no further v2 recorder patches or review round are proposed. The v3 task explicitly requires stopping for a material size overrun: source inspection estimates 340–550 incremental Rust changed lines versus 100–180 for closed header representations and the dependent replay normalization/rendering, URI origins, omitted error bodies and recursive loader/slot checks. #822 records that pre-implementation estimate and all gates as NOT RUN (real Apple capture PENDING). It is a blocked draft, not a completed working replacement; no implementation or captures were produced.
kayg commented 2026-11-04 13:04:49 +00:00
Author
Owner
Copy link

Closing: superseded by #822 (named in the PR body or comments). Branch kept.

Closing: superseded by #822 (named in the PR body or comments). Branch kept.
kayg closed this pull request 2026-11-04 13:04:49 +00:00
Some checks failed
CI / server (pull_request) Has been cancelled
Details
CI / rust (pull_request) Has been cancelled
Details
CI / web (pull_request) Has been cancelled
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
backend
Rust server

  
design
Mockups, UI and TASTE

  
docs
ADRs, CONTEXT, TASTE

  
idea
Parked idea, never worked until promoted

  
infra
Deploy, CI, VMs

  
perf
Performance and budgets

  
reliability
Data integrity, corruption traps

  
web
Web client

No labels
backend
design
docs
idea
infra
perf
reliability
web
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
kayg/calternal!820
No description provided.