Apply inbound iTIP METHOD:REPLY to the organizer's calendar#753
Open
mosa-riel wants to merge 1 commit into
Open
Apply inbound iTIP METHOD:REPLY to the organizer's calendar#753mosa-riel wants to merge 1 commit into
mosa-riel wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughAdds inbound iTIP ChangesInbound iTIP Reply Processing
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant InboundTask
participant ItipEvaluator
participant CalendarTask
participant CalDAVService
InboundTask->>ItipEvaluator: evaluate_inbound_reply(parsed_email, auth_verdict, apply_unverified)
ItipEvaluator-->>InboundTask: accepted ICS and attendee
InboundTask->>CalendarTask: enqueue calendar_apply_reply_task
CalendarTask->>CalDAVService: apply_reply(ics_data, attendee_email, organizer_email)
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/backend/core/mda/inbound_tasks.py`:
- Around line 231-234: The inbound-task fan-out currently allows one
authenticated sender to generate unbounded REPORTs across mailbox users and
writable calendars. Before enabling this dispatch, add abuse protection in the
inbound task flow around the TODO: enforce a per-sender-domain/mailbox rate
limit, or otherwise cap and coalesce REPORT dispatches, while preserving normal
delivery behavior within the configured limit.
In `@src/backend/core/services/calendar/service.py`:
- Around line 545-562: The _put_resource method must never issue an
unconditional PUT when etag is missing or empty. Validate the ETag before
sending the request, and either obtain a fresh non-empty ETag or return/raise
the established concurrency failure; only include If-Match and perform the write
when a valid ETag is available.
- Around line 848-853: Update _reply_query_window to validate that the reply’s
normalized start and end are ordered, clamp both values to the supported
calendar date bounds before applying the one-day expansion, and cap the
resulting query window to the permitted maximum span. Ensure invalid or extreme
attacker-controlled DTSTART/DTEND values cannot underflow arithmetic or produce
oversized REPORT queries.
- Around line 745-755: Update the reply parsing logic around the DTSTAMP
extraction to require a valid DTSTAMP instead of returning None when it is
absent. Ensure records without DTSTAMP are rejected or otherwise excluded before
_record_reply_dtstamp processes them, while preserving the existing sequence and
timestamp handling for valid replies.
In `@src/backend/core/services/calendar/tasks.py`:
- Around line 185-187: Remove PII from all three iTIP error paths: in
src/backend/core/services/calendar/tasks.py:185-187, update the exception
handling around the inbound iTIP REPLY to emit one redacted Sentry event without
task-frame locals or raw ICS, relying on logger.exception rather than calling
capture_exception separately; in src/backend/core/mda/itip.py:150-152, remove
from_addr from the INFO log; and in
src/backend/core/mda/inbound_tasks.py:260-264, replace the mailbox address with
an internal identifier or generic message.
- Around line 178-184: The CalDAVError handler in the task must retry transient
failures instead of returning FAILURE. Classify wrapped timeout/connection
errors and HTTP 429/5xx responses from CalDAVService as retryable, call
self.retry(...) for those cases, and preserve the existing failure result for
permanent 4xx and configuration errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5d9ab507-79f3-4adc-8cdc-8d110c582bff
📒 Files selected for processing (11)
src/backend/core/mda/inbound_auth.pysrc/backend/core/mda/inbound_tasks.pysrc/backend/core/mda/itip.pysrc/backend/core/models.pysrc/backend/core/services/calendar/ics_rebuild.pysrc/backend/core/services/calendar/service.pysrc/backend/core/services/calendar/tasks.pysrc/backend/core/tests/api/test_calendar.pysrc/backend/core/tests/mda/test_inbound_auth.pysrc/backend/core/tests/test_itip.pysrc/backend/messages/settings.py
Comment on lines
+178
to
+184
| except CalDAVError as e: | ||
| logger.warning("Apply-reply failed: %s", e) | ||
| return { | ||
| "status": "FAILURE", | ||
| "result": None, | ||
| "error": str(e), | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
ast-grep outline src/backend/core/services/calendar/tasks.py \
--match 'calendar_apply_reply_task|CalDAVError' --view expanded
rg -nP --type=py -C4 \
'\bself\.retry\s*\(|autoretry_for|retry_backoff' \
src/backend/coreRepository: suitenumerique/messages
Length of output: 6077
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '130,210p' src/backend/core/services/calendar/tasks.py | cat -n
echo
echo '--- search for retry / Sentry patterns in this file ---'
rg -n -C3 'self\.retry|autoretry_for|capture_exception|logger\.exception|logger\.warning|CalDAVError|Exception' src/backend/core/services/calendar/tasks.pyRepository: suitenumerique/messages
Length of output: 8266
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
path = Path("src/backend/core/services/calendar/tasks.py")
text = path.read_text()
for i, line in enumerate(text.splitlines(), 1):
if 130 <= i <= 210:
print(f"{i:4d}: {line}")
PYRepository: suitenumerique/messages
Length of output: 3346
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline src/backend/core/services/calendar/service.py --view expanded
echo
echo '--- CalDAVError definition and raise sites ---'
rg -n -C4 'class CalDAVError|raise CalDAVError|except .*CalDAVError|timeout|429|5\d\d|requests|ConnectionError|HTTPError' \
src/backend/core/services/calendar/service.py src/backend/core/services/calendar -g '*.py'Repository: suitenumerique/messages
Length of output: 42657
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
path = Path("src/backend/core/services/calendar/service.py")
text = path.read_text().splitlines()
for i, line in enumerate(text, 1):
if 1 <= i <= 260:
print(f"{i:4d}: {line}")
PYRepository: suitenumerique/messages
Length of output: 12193
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- call sites for calendar_apply_reply_task ---'
rg -n -C3 'calendar_apply_reply_task|apply_reply_task' src/backend
echo
echo '--- task-result status handling in calendar area ---'
rg -n -C3 '"status": "FAILURE"|status == "FAILURE"|status.*FAILURE|result.*FAILURE' src/backend/core/services/calendar src/backend -g '*.py'Repository: suitenumerique/messages
Length of output: 31145
Retry retryable CalDAV failures here. CalDAVError also wraps timeouts/connection errors and HTTP 429/5xx from CalDAVService; returning "FAILURE" still lets this Celery task complete, so those transient cases will never be retried. Split retryable network/server errors from permanent 4xx/configuration failures and self.retry(...) the former.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/backend/core/services/calendar/tasks.py` around lines 178 - 184, The
CalDAVError handler in the task must retry transient failures instead of
returning FAILURE. Classify wrapped timeout/connection errors and HTTP 429/5xx
responses from CalDAVService as retryable, call self.retry(...) for those cases,
and preserve the existing failure result for permanent 4xx and configuration
errors.
Comment on lines
+185
to
+187
| except Exception as e: # pylint: disable=broad-exception-caught | ||
| capture_exception(e) | ||
| logger.exception("Error applying inbound iTIP REPLY") |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Remove PII from iTIP logs and exception telemetry. The new path exposes attendee/mailbox addresses and potentially the complete ICS payload.
src/backend/core/services/calendar/tasks.py#L185-L187: use one redacted Sentry event without task-frame locals or raw ICS.src/backend/core/mda/itip.py#L150-L152: removefrom_addrfrom the INFO log.src/backend/core/mda/inbound_tasks.py#L260-L264: replace the mailbox address with an internal identifier or generic message.
As per coding guidelines, do not log PII. Based on learnings, logger.exception(...) already reports through Sentry LoggingIntegration.
📍 Affects 3 files
src/backend/core/services/calendar/tasks.py#L185-L187(this comment)src/backend/core/mda/itip.py#L150-L152src/backend/core/mda/inbound_tasks.py#L260-L264
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/backend/core/services/calendar/tasks.py` around lines 185 - 187, Remove
PII from all three iTIP error paths: in
src/backend/core/services/calendar/tasks.py:185-187, update the exception
handling around the inbound iTIP REPLY to emit one redacted Sentry event without
task-frame locals or raw ICS, relying on logger.exception rather than calling
capture_exception separately; in src/backend/core/mda/itip.py:150-152, remove
from_addr from the INFO log; and in
src/backend/core/mda/inbound_tasks.py:260-264, replace the mailbox address with
an internal identifier or generic message.
Sources: Coding guidelines, Learnings
mosa-riel
force-pushed
the
feat/itip-reply-apply
branch
2 times, most recently
from
July 18, 2026 23:45
a787e66 to
6d39074
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/backend/core/mda/inbound_tasks.py`:
- Around line 243-253: Update the channel lookup and dispatch flow in the
inbound task to handle every CalDAV channel matching the mailbox instead of
selecting only the first result. Iterate through all matching channels, enqueue
calendar_apply_reply_task for each channel, and return only after the full set
has been dispatched; preserve the existing payload for each task.
In `@src/backend/core/services/calendar/service.py`:
- Around line 796-802: Update the reply-processing flow around
_apply_reply_to_resource to carry the recipient mailbox identity through the
task, and require the stored event’s ORGANIZER to match it before accepting a
UID match. Reject non-matching copies and continue searching for the canonical
organizer event, then add a regression test covering duplicate
organizer/attendee copies.
- Around line 548-552: Update the response parsing loop around the DAV href
handling to resolve relative href values against the queried calendar URL before
appending them to out. Preserve absolute hrefs unchanged, and ensure subsequent
GET/PUT operations use the resolved resource URL.
In `@src/backend/core/services/calendar/tasks.py`:
- Around line 129-136: Update calendar_apply_reply_task so Celery does not
persist its raw task arguments, especially ics_data and attendee_email, in the
result backend: configure this task to ignore its result using the project’s
established Celery task pattern, or change the enqueueing flow to pass an
internal record ID and load the ICS data inside the task. Preserve the task’s
existing reply-processing behavior.
In `@src/backend/core/tests/api/test_calendar.py`:
- Around line 2571-2574: Update both attendee lookup helpers, including the
shown loop and the corresponding helper near the second location, to normalize
each address by stripping an optional mailto: prefix and lowercasing before
comparing exact equality with the normalized target. Replace the endswith-based
match while preserving the existing PARTSTAT return behavior.
- Around line 2895-2896: Update the clamp assertion in the relevant calendar
test to compare recorded against the current UTC time rather than the fixed
January 1, 2030 cutoff, while preserving the non-null assertion and verifying
the clamped timestamp remains before now.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2042f9b9-dce2-4d32-9c63-99711270885e
📒 Files selected for processing (11)
src/backend/core/mda/inbound_auth.pysrc/backend/core/mda/inbound_tasks.pysrc/backend/core/mda/itip.pysrc/backend/core/models.pysrc/backend/core/services/calendar/ics_rebuild.pysrc/backend/core/services/calendar/service.pysrc/backend/core/services/calendar/tasks.pysrc/backend/core/tests/api/test_calendar.pysrc/backend/core/tests/mda/test_inbound_auth.pysrc/backend/core/tests/test_itip.pysrc/backend/messages/settings.py
mosa-riel
force-pushed
the
feat/itip-reply-apply
branch
2 times, most recently
from
July 19, 2026 00:09
3456346 to
162095e
Compare
Close the RSVP loop: when an attendee's iTIP REPLY email lands in the organizer's mailbox, record their PARTSTAT on the organizer's stored copy of the event. Mirror of the attendee-side respond_to_event. - core/mda/itip.py: detect a text/calendar METHOD:REPLY part and gate it on the inbound pipeline's postmark["auth"] verdict. The DMARC-verified From is threaded through enqueue -> task -> apply_reply as the only ATTENDEE that may be modified, and must itself be an ATTENDEE carrying a PARTSTAT — so a crafted multi-ATTENDEE reply (aligned but PARTSTAT-less From + a PARTSTAT on a third party) cannot move anyone else. Oversized parts (>512 KB) and recurrence- instance replies (RECURRENCE-ID) are rejected at the gate. - Verdict policy: "fail" never applied; "none" skipped unless CALENDAR_ITIP_REPLY_APPLY_UNVERIFIED; verified applied and flagged. An absent verdict counts as verified ONLY when inbound auth ran with a SUPPORTED mode (inbound_auth_enabled) — DKIM/DMARC disabled or a typo'd mode downgrades to unverifiable, so an auth-less deployment can't be spoofed. Provenance in postmark["itip-reply"]. - CalDAVService.apply_reply: locate the event via a time-range REPORT that also returns d:getetag + d:href (resolved against the calendar URL), then write back to the event's OWN resource (filenames need not equal the UID) with If-Match; if the REPORT gave no etag one is fetched and the write refused rather than sent unconditionally. On 412 re-fetch, re-check staleness, re-apply and retry once, else concurrent-modification. Only a copy whose ORGANIZER matches the recipient mailbox is written — a same-UID copy on which the mailbox is merely an attendee is skipped. Apply only the verified attendee's PARTSTAT; SCHEDULE-AGENT=CLIENT to suppress SabreDAV re-fanout; SEQUENCE + clamped per-attendee DTSTAMP staleness guard (all VEVENTs). REPLYs without DTSTART or DTSTAMP rejected; the attacker-controlled time-range window is normalized, span-capped and bounds-clamped. Distinct no-op reasons. - calendar_apply_reply_task (ignore_result — fire-and-forget) + inbound hook fanning out across all of the mailbox's CalDAV channels (or its users, capped), only when a CalDAV backend is configured. Per-(sender,mailbox) rate limiting is a TODO. - CALENDAR_ITIP_REPLY_ENABLED flag (off by default). respond_to_event (interactive RSVP) has a pre-existing, independent variant of the same href hazard (UID-derived URL) — tracked upstream as a follow-up. Tests: unit trust matrix + two-ATTENDEE attack + recurrence + oversized + auth-mode validation + DTSTAMP clamp + window bounds; apply_reply against live Radicale incl. non-UID filename, If-Match, 412 retry, no-etag refusal, missing DTSTART/DTSTAMP, non-organizer-copy skip, all-channels fan-out; full-stack e2e.
mosa-riel
force-pushed
the
feat/itip-reply-apply
branch
from
July 19, 2026 17:39
162095e to
3be281a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the calendar RSVP loop: applies an inbound iTIP
METHOD:REPLYto theorganizer's stored event — the mirror of the attendee-side
respond_to_event.Off by default behind
CALENDAR_ITIP_REPLY_ENABLED.How: detect a
text/calendar; method=REPLYpart in the inbound pipeline →gate → async
calendar_apply_reply_task→CalDAVService.apply_replyfinds theevent by UID and writes the replying attendee's PARTSTAT back to its real
resource href with
If-Match.Trust: consumes the pipeline's existing
postmark["auth"]verdict — no newauth mechanism. The DMARC-verified From is the only ATTENDEE that can be moved,
and must itself be RSVPing, so a crafted multi-ATTENDEE reply can't move a third
party.
failnever applied;noneonly withCALENDAR_ITIP_REPLY_APPLY_UNVERIFIED;an absent verdict counts as verified only when a supported inbound-auth mode ran.
CalDAV correctness: write to the event's real href (filenames needn't equal
the UID) with
If-Match+ 412 refetch-retry;SCHEDULE-AGENT=CLIENTto suppressserver re-fanout; SEQUENCE + clamped per-attendee DTSTAMP staleness guard;
recurrence-instance and DTSTART-less replies rejected; oversized parts capped.
Tests: unit trust matrix + the two-ATTENDEE attack;
apply_replyagainst liveRadicale (non-UID filename, If-Match, 412 retry); full-stack e2e through the MTA
deliver endpoint.
Follow-up: #752 (
respond_to_eventshares the same href hazard on theinteractive RSVP path). Open question: apply in-backend as here, or hand the
reply to SabreDAV's scheduling inbox?
Summary by CodeRabbit
METHOD:REPLYhandling to update matching CalDAV attendee PARTSTATs.X-STMSG-REPLY-DTSTAMPduring calendar rebuild/sanitization.