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

Record and replay native Apple CardDAV traffic #789

Closed
kayg wants to merge 3 commits from feat/dav-recorder into dev
pull from: feat/dav-recorder
merge into: kayg:dev
Conversation 8 Commits 3 Files changed 19 +1717
kayg commented 2026-10-31 16:02:24 +00:00
Owner
Copy link

Plan (before implementation)

Goal

Capture one macOS Contacts session that creates, edits, and deletes only synthetic contacts; preserve each request/response with credentials redacted; replay the exchanges as a repeatable CardDAV compatibility gate.

Design and decisions

  • Keep the existing Rust recorder, fixture loader, scrubber, and replayer; make capture completion reliable and make credential handling usable without placing the Radicale password on a process command line.
  • Reuse the existing empty Radicale address book on dev-02. It is a learning-only stand-in and stays outside product tests.
  • Capture from the macOS 26.5.2 guest while holding /tmp/mac-bench.lock. Add only fake names and .test email addresses. Remove the fixture account before releasing the lock.
  • Commit the synthetic, audited fixture here because this task explicitly requests it. ADR 0067 normally stores recordings on the issue; this fixture will contain no real data and will pass the secret audit.
  • Keep Calternal replay invokable against a target address; the current dev branch does not contain the CardDAV server yet, so the live-server replay gate will run on dev-02 against the available CardDAV test server and the README will state its exact setup.

Work and gates

  1. Add regression coverage for recorder shutdown/error behavior and secret-file handling, then implement only what the tests require.
  2. Run the crate tests on dev-02 through its Nix dev shell and build slot.
  3. Record the fake Contacts.app create/edit/delete session, remove the account, audit every saved exchange, and replay the fixture against the fixture answerer and the Calternal CardDAV server available on dev-02.
  4. Commit the fixture and recording instructions; update this plan with exact gate results.

Final round 2 closure plan

  1. Replace synthetic URL host matching with an exact whole-value allowlist of fixed synthetic forms; add an allowed-form test and a capture-abort regression for an unapproved path/query.
  2. Audit both raw and canonicalized views of every stored field. Percent-decode repeatedly to stability (maximum 8 rounds, fail closed if another decode remains), case-fold both views, decode Basic authorization payloads, and check secret patterns plus every configured secret. Add path/body regressions and a property test for nested percent encoding, mixed case, Basic encoding, and fragments split across query values.
  3. Bound each request and response body to 4 MiB, active client connections to 8, and the complete upstream request/response to 30 seconds. Any exceeded limit or timeout aborts capture and removes the partial session; add targeted regressions.
  4. Pass the Radicale throwaway password through a mode 0600 curl config file instead of curl arguments. Add the multi-item partial scenario coverage regression before relying on the fixture gate.

Size and complexity

The recorder buffers one request and response at a time, so transient memory is O(request bytes + response bytes) per active exchange. Fixture storage is O(total captured bytes). Replay is O(total fixture bytes) plus XML sibling sorting, O(n log n) for n XML nodes at each sibling level. The intended capture is a small CRUD session, not a bulk address book.

Failure table

Failure Effect Detection Recovery Test
Radicale or recorder cannot bind/reach upstream No valid session Service readiness check and recorder error Use a fresh scenario directory and retry before opening Contacts Recorder forwards an upstream failure visibly
Shutdown races an in-flight request Last exchange may be missing Session exchange count and parse/audit Drain accepted work before returning Delayed upstream response is persisted before shutdown returns
Fixture write fails Session is incomplete Nonzero recorder failure and missing sequence Stop capture; retry into a new directory after resolving the write error Inject a write failure and assert capture reports it
Secret remains in a header/body Fixture is unsafe to publish dav-replay check with the known password file plus fixture audit Remove the fixture and recapture with a fresh password Secret audit rejects a deliberately unredacted value
Mac lock is held Capture could disrupt another Mac task mkdir /tmp/mac-bench.lock fails Wait for the existing owner; do not touch Contacts Lock acquisition/release procedure in the capture checklist

Gate commands

  • ~/bin/slot /nix/var/nix/profiles/default/bin/nix develop -c bash -c 'cargo test -j4 -p calternal-dav-replay' on dev-02.
  • dav-replay check over the committed fixture with its throwaway password supplied through a protected file.
  • Replay every committed fixture against its recorded answerer 20 times, then replay the captured session against the dev-02 Calternal CardDAV test server and retain the result with the PR.
  • Confirm the Mac fixture account is gone and the lock directory has been removed.

No merge is requested.

## Plan (before implementation) ### Goal Capture one macOS Contacts session that creates, edits, and deletes only synthetic contacts; preserve each request/response with credentials redacted; replay the exchanges as a repeatable CardDAV compatibility gate. ### Design and decisions - Keep the existing Rust recorder, fixture loader, scrubber, and replayer; make capture completion reliable and make credential handling usable without placing the Radicale password on a process command line. - Reuse the existing empty Radicale address book on dev-02. It is a learning-only stand-in and stays outside product tests. - Capture from the macOS 26.5.2 guest while holding `/tmp/mac-bench.lock`. Add only fake names and `.test` email addresses. Remove the fixture account before releasing the lock. - Commit the synthetic, audited fixture here because this task explicitly requests it. ADR 0067 normally stores recordings on the issue; this fixture will contain no real data and will pass the secret audit. - Keep Calternal replay invokable against a target address; the current `dev` branch does not contain the CardDAV server yet, so the live-server replay gate will run on dev-02 against the available CardDAV test server and the README will state its exact setup. ### Work and gates 1. Add regression coverage for recorder shutdown/error behavior and secret-file handling, then implement only what the tests require. 2. Run the crate tests on dev-02 through its Nix dev shell and build slot. 3. Record the fake Contacts.app create/edit/delete session, remove the account, audit every saved exchange, and replay the fixture against the fixture answerer and the Calternal CardDAV server available on dev-02. 4. Commit the fixture and recording instructions; update this plan with exact gate results. ### Final round 2 closure plan 1. Replace synthetic URL host matching with an exact whole-value allowlist of fixed synthetic forms; add an allowed-form test and a capture-abort regression for an unapproved path/query. 2. Audit both raw and canonicalized views of every stored field. Percent-decode repeatedly to stability (maximum 8 rounds, fail closed if another decode remains), case-fold both views, decode Basic authorization payloads, and check secret patterns plus every configured secret. Add path/body regressions and a property test for nested percent encoding, mixed case, Basic encoding, and fragments split across query values. 3. Bound each request and response body to 4 MiB, active client connections to 8, and the complete upstream request/response to 30 seconds. Any exceeded limit or timeout aborts capture and removes the partial session; add targeted regressions. 4. Pass the Radicale throwaway password through a mode 0600 curl config file instead of curl arguments. Add the multi-item partial scenario coverage regression before relying on the fixture gate. ### Size and complexity The recorder buffers one request and response at a time, so transient memory is O(request bytes + response bytes) per active exchange. Fixture storage is O(total captured bytes). Replay is O(total fixture bytes) plus XML sibling sorting, O(n log n) for n XML nodes at each sibling level. The intended capture is a small CRUD session, not a bulk address book. ### Failure table | Failure | Effect | Detection | Recovery | Test | |---|---|---|---|---| | Radicale or recorder cannot bind/reach upstream | No valid session | Service readiness check and recorder error | Use a fresh scenario directory and retry before opening Contacts | Recorder forwards an upstream failure visibly | | Shutdown races an in-flight request | Last exchange may be missing | Session exchange count and parse/audit | Drain accepted work before returning | Delayed upstream response is persisted before shutdown returns | | Fixture write fails | Session is incomplete | Nonzero recorder failure and missing sequence | Stop capture; retry into a new directory after resolving the write error | Inject a write failure and assert capture reports it | | Secret remains in a header/body | Fixture is unsafe to publish | `dav-replay check` with the known password file plus fixture audit | Remove the fixture and recapture with a fresh password | Secret audit rejects a deliberately unredacted value | | Mac lock is held | Capture could disrupt another Mac task | `mkdir /tmp/mac-bench.lock` fails | Wait for the existing owner; do not touch Contacts | Lock acquisition/release procedure in the capture checklist | ### Gate commands - `~/bin/slot /nix/var/nix/profiles/default/bin/nix develop -c bash -c 'cargo test -j4 -p calternal-dav-replay'` on dev-02. - `dav-replay check` over the committed fixture with its throwaway password supplied through a protected file. - Replay every committed fixture against its recorded answerer 20 times, then replay the captured session against the dev-02 Calternal CardDAV test server and retain the result with the PR. - Confirm the Mac fixture account is gone and the lock directory has been removed. No merge is requested.
dav-replay: readme
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
2308c690cd
kayg commented 2026-10-31 20:07:45 +00:00
Author
Owner
Copy link

Independent review, round 1

Verdict: fix first

P0

  • None.

P1

  • The fixture gate can be empty. No fixture tree is committed at 255eca90. tests/fixtures.rs does not assert a session or exchange count, and Report::ok() is true for zero results. check also succeeds for an existing empty root or metadata-only session. On a clean checkout the missing root instead makes the test unwrap an I/O error. Require at least one session and one exchange per session, and make replay/check reject empty data. The current mock tests do not establish the macOS replay gate.
  • Synthetic-only capture is not enforced. The recorder writes complete vCard bodies after replacing configured host/user/secrets. It has no check that captured contacts are synthetic, and check does not inspect meta.json (including the free-form note). The README's fake-contact instruction is manual; a real contact body can still be written as a fixture.
  • App passwords in paths/bodies are fail-open. --secret-file is optional, and literal replacement only applies to configured secrets in text bodies. Binary bodies are saved as base64 and skipped by scrubbing; the audit scans them through lossy UTF-8. A password in a URL/body can therefore reach disk. Require the secret input or reject unreviewable bodies before writing.

P2

  • Capture memory and concurrency are unbounded. http::collect buffers full request and response bodies, while the connection JoinSet has no limit. A large or parallel sync can exhaust the VM. Add body and concurrency limits, plus an upstream timeout, and fail the capture clearly.
  • DAV error bodies are skipped. compare_body ignores every body for status 400+, including structured DAV XML. A changed precondition/error QName with the same status passes. Compare protocol error XML while ignoring only unstable human wording.
  • The throwaway server setup exposes its password in argv. radicale-throwaway.sh passes davtest:<password> via curl -u, which conflicts with the PR plan to keep that password off process command lines. Use a protected curl config or netrc file.

P3

  • None.
## Independent review, round 1 Verdict: **fix first** ### P0 - None. ### P1 - **The fixture gate can be empty.** No fixture tree is committed at `255eca90`. `tests/fixtures.rs` does not assert a session or exchange count, and `Report::ok()` is true for zero results. `check` also succeeds for an existing empty root or metadata-only session. On a clean checkout the missing root instead makes the test unwrap an I/O error. Require at least one session and one exchange per session, and make replay/check reject empty data. The current mock tests do not establish the macOS replay gate. - **Synthetic-only capture is not enforced.** The recorder writes complete vCard bodies after replacing configured host/user/secrets. It has no check that captured contacts are synthetic, and `check` does not inspect `meta.json` (including the free-form note). The README's fake-contact instruction is manual; a real contact body can still be written as a fixture. - **App passwords in paths/bodies are fail-open.** `--secret-file` is optional, and literal replacement only applies to configured secrets in text bodies. Binary bodies are saved as base64 and skipped by scrubbing; the audit scans them through lossy UTF-8. A password in a URL/body can therefore reach disk. Require the secret input or reject unreviewable bodies before writing. ### P2 - **Capture memory and concurrency are unbounded.** `http::collect` buffers full request and response bodies, while the connection `JoinSet` has no limit. A large or parallel sync can exhaust the VM. Add body and concurrency limits, plus an upstream timeout, and fail the capture clearly. - **DAV error bodies are skipped.** `compare_body` ignores every body for status 400+, including structured DAV XML. A changed precondition/error QName with the same status passes. Compare protocol error XML while ignoring only unstable human wording. - **The throwaway server setup exposes its password in argv.** `radicale-throwaway.sh` passes `davtest:<password>` via `curl -u`, which conflicts with the PR plan to keep that password off process command lines. Use a protected curl config or netrc file. ### P3 - None.
kayg commented 2026-10-31 20:54:17 +00:00
Author
Owner
Copy link

Round 1 status

Head: 901acd4f

Review item Status Evidence
Empty or incomplete replay fixture gate Fixed. Empty fixture roots report PENDING (check exit 3); once captures exist, every session must contain exchanges and the declared required scenarios must be covered. CLI tests cover pending, missing required list, zero exchanges, missing coverage, and a complete fixture set. The fixture test output reports PENDING: no real capture fixtures; required scenarios: contacts-crud, contacts-initial-sync.
Synthetic capture data Fixed. Request and response vCard/iCalendar bodies are checked against the allowlist; a failed check aborts capture and removes the partial directory. Phone annotations outside the reserved pattern are rejected. Recorder tests cover names, email domains, reserved phone numbers, annotations, calendar attendees, and response bodies.
Redaction before persistence Fixed. Recording requires --secret-file; mixed-case Authorization/Cookie headers, Basic/Bearer tokens, ca1_ patterns, and configured values are scrubbed across paths, headers, text/binary bodies, and metadata. Checked writes reject any remaining pattern. Scrubber, recorder, and CLI regression tests cover each location and final-write rejection.
cargo fmt --all --check Pass Ran on dev-02 in the Nix shell.
cargo clippy -j4 -p calternal-dav-replay --all-targets --locked -- -D warnings Pass Ran on dev-02.
cargo test -j4 -p calternal-dav-replay --locked -- --nocapture Pass; live-fixture replay remains pending Ran on dev-02. The committed fixture set is empty, so the gate reports pending rather than claiming a replay pass.

Open follow-ups from the review: capture size/concurrency/timeouts, structured DAV error-body comparison, and moving the throwaway server credential out of the curl command line.

## Round 1 status Head: `901acd4f` | Review item | Status | Evidence | |---|---|---| | Empty or incomplete replay fixture gate | Fixed. Empty fixture roots report `PENDING` (check exit 3); once captures exist, every session must contain exchanges and the declared required scenarios must be covered. | CLI tests cover pending, missing required list, zero exchanges, missing coverage, and a complete fixture set. The fixture test output reports `PENDING: no real capture fixtures; required scenarios: contacts-crud, contacts-initial-sync`. | | Synthetic capture data | Fixed. Request and response vCard/iCalendar bodies are checked against the allowlist; a failed check aborts capture and removes the partial directory. Phone annotations outside the reserved pattern are rejected. | Recorder tests cover names, email domains, reserved phone numbers, annotations, calendar attendees, and response bodies. | | Redaction before persistence | Fixed. Recording requires `--secret-file`; mixed-case Authorization/Cookie headers, Basic/Bearer tokens, `ca1_` patterns, and configured values are scrubbed across paths, headers, text/binary bodies, and metadata. Checked writes reject any remaining pattern. | Scrubber, recorder, and CLI regression tests cover each location and final-write rejection. | | `cargo fmt --all --check` | Pass | Ran on dev-02 in the Nix shell. | | `cargo clippy -j4 -p calternal-dav-replay --all-targets --locked -- -D warnings` | Pass | Ran on dev-02. | | `cargo test -j4 -p calternal-dav-replay --locked -- --nocapture` | Pass; live-fixture replay remains pending | Ran on dev-02. The committed fixture set is empty, so the gate reports pending rather than claiming a replay pass. | Open follow-ups from the review: capture size/concurrency/timeouts, structured DAV error-body comparison, and moving the throwaway server credential out of the curl command line.
kayg commented 2026-10-31 21:05:19 +00:00
Author
Owner
Copy link

Independent review, round 2

Head: 901acd4f

Verdict: fix first

Round 1 item Status Does its test demonstrate it?
Empty or incomplete replay fixture gate Resolved in dav-replay check. Empty fixtures return PENDING (exit 3); zero-exchange and missing-coverage fixtures fail. Yes for pending, zero exchanges, missing declarations, and a complete set. The passing coverage test has only one required scenario, so a nonempty but incomplete multi-scenario set is not directly tested.
Synthetic-only capture Not resolved. The URL allowlist checks the scheme and example.invalid host, but accepts arbitrary path, query, and fragment text. Existing tests cover names, domains, phone values, attendees, annotations, and direct card responses. None exercises an unapproved value in an otherwise allowed URL.
Credentials in paths/bodies Not fully resolved. Raw configured values and recognized credential patterns are scrubbed and checked, but percent-encoded forms are not decoded before scrub/audit. Tests demonstrate raw configured values in text and binary data and the final write refusal. They do not cover encoded values.

New findings

P1

  • crates/dav-replay/src/synthetic.rs: synthetic_url() accepts any path/query/fragment on example.invalid. For example, a vCard URL containing ?contact=person%40example.com passes validation, and the complete body is saved. Restrict the entire value to approved synthetic URLs and add a capture test that confirms rejection and cleanup.
  • crates/dav-replay/src/scrub.rs: configured secrets are matched as literal bytes. A path or body containing a percent-encoded secret such as %73ecret passes the final audit when secret is supplied in the secret file, then is persisted. Normalize encoded forms or reject them before writing, with a path/body regression test.

P2

  • The recorder still buffers full bodies and accepts unbounded concurrent connections; there is no upstream request timeout. Add size/concurrency limits and a timeout before the first real capture, especially for the initial-sync scenario.
  • radicale-throwaway.sh still supplies the generated password through curl -u, placing it in the curl argument list. Change this before running the helper for the first real capture.
  • There is no test where a multi-item required-scenario list is only partly covered. The gate code checks the missing set, but its tests use a one-item manifest for the positive case. Add a partial-coverage regression test before relying on this gate.

Follow-up timing

The capture limits and curl credential handling should be fixed before the first real capture. Structured XML error bodies already take the XML comparison path before the 4xx skip, so changing that comparison is not a first-capture blocker. Add a 4xx DAV XML regression test later; the current comparator also compares response-description text, so decide which human wording is unstable before treating those fixtures as a gate.

Review was source-only; no commands were built or tested on the read-only control host.

## Independent review, round 2 Head: `901acd4f` Verdict: **fix first** | Round 1 item | Status | Does its test demonstrate it? | |---|---|---| | Empty or incomplete replay fixture gate | Resolved in `dav-replay check`. Empty fixtures return `PENDING` (exit 3); zero-exchange and missing-coverage fixtures fail. | Yes for pending, zero exchanges, missing declarations, and a complete set. The passing coverage test has only one required scenario, so a nonempty but incomplete multi-scenario set is not directly tested. | | Synthetic-only capture | Not resolved. The URL allowlist checks the scheme and `example.invalid` host, but accepts arbitrary path, query, and fragment text. | Existing tests cover names, domains, phone values, attendees, annotations, and direct card responses. None exercises an unapproved value in an otherwise allowed URL. | | Credentials in paths/bodies | Not fully resolved. Raw configured values and recognized credential patterns are scrubbed and checked, but percent-encoded forms are not decoded before scrub/audit. | Tests demonstrate raw configured values in text and binary data and the final write refusal. They do not cover encoded values. | ### New findings #### P1 - `crates/dav-replay/src/synthetic.rs`: `synthetic_url()` accepts any path/query/fragment on `example.invalid`. For example, a vCard URL containing `?contact=person%40example.com` passes validation, and the complete body is saved. Restrict the entire value to approved synthetic URLs and add a capture test that confirms rejection and cleanup. - `crates/dav-replay/src/scrub.rs`: configured secrets are matched as literal bytes. A path or body containing a percent-encoded secret such as `%73ecret` passes the final audit when `secret` is supplied in the secret file, then is persisted. Normalize encoded forms or reject them before writing, with a path/body regression test. #### P2 - The recorder still buffers full bodies and accepts unbounded concurrent connections; there is no upstream request timeout. Add size/concurrency limits and a timeout before the first real capture, especially for the initial-sync scenario. - `radicale-throwaway.sh` still supplies the generated password through `curl -u`, placing it in the curl argument list. Change this before running the helper for the first real capture. - There is no test where a multi-item required-scenario list is only partly covered. The gate code checks the missing set, but its tests use a one-item manifest for the positive case. Add a partial-coverage regression test before relying on this gate. ### Follow-up timing The capture limits and curl credential handling should be fixed before the first real capture. Structured XML error bodies already take the XML comparison path before the 4xx skip, so changing that comparison is not a first-capture blocker. Add a 4xx DAV XML regression test later; the current comparator also compares response-description text, so decide which human wording is unstable before treating those fixtures as a gate. Review was source-only; no commands were built or tested on the read-only control host.
kayg commented 2026-10-31 21:39:16 +00:00
Author
Owner
Copy link

Round 2 status

Head: 203493d51106cf5a98b13dff5c83160d27da013e

Review item Status Evidence
Synthetic URL values Fixed. URL properties must equal an approved full synthetic value, including its fixed path and parameters. Other values stop recording and remove partial output. Regression covers an unapproved path and query.
Secret scanning Fixed. Audits check raw and bounded canonical views, repeatedly percent-decode up to eight passes, case-fold, decode Basic authorization payloads, and scan query values together. Writes fail on a credential pattern, configured secret, or encoding beyond the bound. A 128-case property test covers mixed case, one to three percent-encoding layers, Basic headers, and values split across query parameters. Path and body write regressions are included.
Capture limits Fixed. Request and response bodies are limited to 4 MiB, concurrent client connections to eight, and each upstream exchange to 30 seconds. Recorder limit and timeout tests pass.
Throwaway credential handling Fixed. The helper uses a mode-0600 temporary curl config; password and users files are mode 0600. The password is no longer passed as a curl argument. bash -n crates/dav-replay/scripts/radicale-throwaway.sh passes.
Partial scenario coverage Fixed. Added a regression for a nonempty fixture set that covers only part of a multi-scenario requirement. Replay crate test suite passes.
Dev-02 gates Pass: cargo fmt --all --check; cargo clippy -j4 -p calternal-dav-replay --all-targets --locked -- -D warnings; cargo test -j4 -p calternal-dav-replay --locked -- --nocapture (56 passed). Ran in the dev-02 Nix shell at this head.
Replay fixtures Pending until real capture fixtures exist; no real capture was run in this pass. The fixture gate reports PENDING on the empty fixture set.
Structured 4xx DAV XML wording Deferred as a later follow-up in the round 2 review; that review did not mark it as a first-capture blocker. No change in this pass.
## Round 2 status Head: `203493d51106cf5a98b13dff5c83160d27da013e` | Review item | Status | Evidence | |---|---|---| | Synthetic URL values | Fixed. URL properties must equal an approved full synthetic value, including its fixed path and parameters. Other values stop recording and remove partial output. | Regression covers an unapproved path and query. | | Secret scanning | Fixed. Audits check raw and bounded canonical views, repeatedly percent-decode up to eight passes, case-fold, decode Basic authorization payloads, and scan query values together. Writes fail on a credential pattern, configured secret, or encoding beyond the bound. | A 128-case property test covers mixed case, one to three percent-encoding layers, Basic headers, and values split across query parameters. Path and body write regressions are included. | | Capture limits | Fixed. Request and response bodies are limited to 4 MiB, concurrent client connections to eight, and each upstream exchange to 30 seconds. | Recorder limit and timeout tests pass. | | Throwaway credential handling | Fixed. The helper uses a mode-0600 temporary curl config; password and users files are mode 0600. The password is no longer passed as a curl argument. | `bash -n crates/dav-replay/scripts/radicale-throwaway.sh` passes. | | Partial scenario coverage | Fixed. Added a regression for a nonempty fixture set that covers only part of a multi-scenario requirement. | Replay crate test suite passes. | | Dev-02 gates | Pass: `cargo fmt --all --check`; `cargo clippy -j4 -p calternal-dav-replay --all-targets --locked -- -D warnings`; `cargo test -j4 -p calternal-dav-replay --locked -- --nocapture` (56 passed). | Ran in the dev-02 Nix shell at this head. | | Replay fixtures | Pending until real capture fixtures exist; no real capture was run in this pass. | The fixture gate reports `PENDING` on the empty fixture set. | | Structured 4xx DAV XML wording | Deferred as a later follow-up in the round 2 review; that review did not mark it as a first-capture blocker. | No change in this pass. |
kayg commented 2026-10-31 21:49:58 +00:00
Author
Owner
Copy link

Independent review, round 3 (final)

Head: 203493d511

Item Status Evidence
Empty or partial replay fixtures Resolved; real-capture gate remains pending by design Committed tests reject empty/incomplete coverage. The suite reports PENDING while no real sessions exist.
Synthetic URL property values Resolved for vCard URL properties Independent probe accepted the fixed form and rejected added query data, fragments, host-case changes, ports, encoded variants, and userinfo.
Secret audit Resolved for the requested encodings Independent probes caught mixed-case, double percent encoding split across query values, and a percent-encoded Basic header payload.
Capture limits Partial Body and connection caps work; the 30-second upstream timeout passes. Client request-body collection has no deadline (P2 below).
Throwaway credential handling Resolved The helper uses a mode-0600 temporary curl config and does not pass the password as a curl argument.
Multi-scenario coverage Resolved The committed partial-coverage regression passes.
Structured 4xx DAV XML wording Deferred as recorded in round 2 No change in this pass.

New findings

  • P0: None.
  • P1: URI fields bypass the synthetic-only check. The recorder validates request and response bodies, but stores the request target and response Location headers without a synthetic URI check; application/xml DAV href values are not parsed by the body validator. A temporary probe passed a valid synthetic vCard under /alice/addressbook/sample-person.vcf?email=sample%40example.com; after the username scrub, that request target was written to the fixture. Contact names or addresses in resource URIs can therefore be captured. Simpler design: limit capture to the exact per-scenario request paths and response href/Location forms derived from fixed synthetic resource IDs, and stop on any unrecognized URI before forwarding or writing.
  • P2: The request body is collected before the 30-second timeout begins. An incomplete client body can hold one of the eight connection slots until the client closes or capture shutdown runs. Apply a deadline to client body collection too.
  • P3: None.

Verdict: stop and simplify.

Verification on dev-02: the committed crate command cargo test -j4 -p calternal-dav-replay --locked -- --nocapture passed all 56 tests; the real-capture gate reported PENDING as expected. The temporary independent probe command cargo test -j4 -p calternal-dav-replay --test review_r3_probe --locked -- --nocapture passed 4/4 and its file was removed. No PR code changed.

## Independent review, round 3 (final) Head: 203493d51106cf5a98b13dff5c83160d27da013e | Item | Status | Evidence | |---|---|---| | Empty or partial replay fixtures | Resolved; real-capture gate remains pending by design | Committed tests reject empty/incomplete coverage. The suite reports PENDING while no real sessions exist. | | Synthetic URL property values | Resolved for vCard URL properties | Independent probe accepted the fixed form and rejected added query data, fragments, host-case changes, ports, encoded variants, and userinfo. | | Secret audit | Resolved for the requested encodings | Independent probes caught mixed-case, double percent encoding split across query values, and a percent-encoded Basic header payload. | | Capture limits | Partial | Body and connection caps work; the 30-second upstream timeout passes. Client request-body collection has no deadline (P2 below). | | Throwaway credential handling | Resolved | The helper uses a mode-0600 temporary curl config and does not pass the password as a curl argument. | | Multi-scenario coverage | Resolved | The committed partial-coverage regression passes. | | Structured 4xx DAV XML wording | Deferred as recorded in round 2 | No change in this pass. | ### New findings - P0: None. - P1: URI fields bypass the synthetic-only check. The recorder validates request and response bodies, but stores the request target and response Location headers without a synthetic URI check; application/xml DAV href values are not parsed by the body validator. A temporary probe passed a valid synthetic vCard under /alice/addressbook/sample-person.vcf?email=sample%40example.com; after the username scrub, that request target was written to the fixture. Contact names or addresses in resource URIs can therefore be captured. Simpler design: limit capture to the exact per-scenario request paths and response href/Location forms derived from fixed synthetic resource IDs, and stop on any unrecognized URI before forwarding or writing. - P2: The request body is collected before the 30-second timeout begins. An incomplete client body can hold one of the eight connection slots until the client closes or capture shutdown runs. Apply a deadline to client body collection too. - P3: None. Verdict: **stop and simplify**. Verification on dev-02: the committed crate command cargo test -j4 -p calternal-dav-replay --locked -- --nocapture passed all 56 tests; the real-capture gate reported PENDING as expected. The temporary independent probe command cargo test -j4 -p calternal-dav-replay --test review_r3_probe --locked -- --nocapture passed 4/4 and its file was removed. No PR code changed.
kayg commented 2026-10-31 21:56:38 +00:00
Author
Owner
Copy link

The Apple DAV recorder/replayer work is on hold pending review of the v2 plan in PR #818. The plan replaces capture-then-scrub with a typed allowlist and in-memory placeholders.

The Apple DAV recorder/replayer work is on hold pending review of the v2 plan in [PR #818](https://git.kayg.org/kayg/calternal/pulls/818). The plan replaces capture-then-scrub with a typed allowlist and in-memory placeholders.
kayg commented 2026-10-31 23:44:03 +00:00
Author
Owner
Copy link

Implementation work moved to PR #820: #820. It implements the approved plan in #818 and supersedes this PR. This PR remains open and on hold; no merge was performed.

Implementation work moved to PR #820: https://git.kayg.org/kayg/calternal/pulls/820. It implements the approved plan in #818 and supersedes this PR. This PR remains open and on hold; no merge was performed.
kayg commented 2026-11-04 13:04:42 +00:00
Author
Owner
Copy link

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

Closing: superseded by #820, itself replaced by #822 (named in the PR body or comments). Branch kept.
kayg closed this pull request 2026-11-04 13:04:42 +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!789
No description provided.