feat(calendar): subscribe to a public iCal/ICS calendar by URL (#4)#212
Open
eibrahim wants to merge 8 commits into
Open
feat(calendar): subscribe to a public iCal/ICS calendar by URL (#4)#212eibrahim wants to merge 8 commits into
eibrahim wants to merge 8 commits into
Conversation
Add a read-only ICAL calendar feed type so users can subscribe to a public iCal/ICS URL (sports schedules, holidays, shared calendars) that isn't reachable via Google/Outlook/CalDAV. - src/lib/ical-feed.ts: normalizeIcalUrl (webcal->https, http(s) only) + parseIcalEvents (reuses ical.js convertVEventToCalendarEvent; recurring masters expand at render time) + fetchIcalEvents (server-side fetch, size cap) - src/app/api/feeds/[id]/ical-sync/route.ts: re-fetch + parse + replace events in a transaction; fetch/parse failures surface an error WITHOUT dropping existing events - store.addFeed/syncFeed wired for ICAL via the generic /api/feeds routes - AccountManager "Subscribe to iCal Calendar" form + FeedManager icon - ICAL stays out of WRITABLE_FEED_TYPES, so feeds are read-only Refresh is user-initiated (manual sync), matching existing open-source feed behavior; a scheduled background refresh is left as a SAAS //todo. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016mqdsn2va7teLRuNPdyeZy
Round-1 adversarial review (verdict needs-attention) raised 3 HIGH findings; all fixed via TDD: - SSRF/resource hardening (src/lib/ical-feed.ts): reject loopback/private/ link-local/cloud-metadata hosts as literal IPs and via DNS resolution (assertSafeIcalHost / assertResolvedHostSafe), handle redirects manually and re-validate each hop, add an AbortController timeout, and read the body with a streamed hard byte cap. - Recurring events now render: expandIcalEvents materializes occurrence rows within a bounded window at sync time (the render path does not expand masters), mirroring the Google provider. Anchors RRule on each event's start. - Read-only enforced on delete: store.removeEvent rejects ICAL feeds, and the generic server event APIs (/api/events POST/PATCH/DELETE and /api/events/[id] PATCH/DELETE) now return 403 for ICAL feeds via a shared isWritableFeedType() helper exported from calendar-drag.ts. Adds unit tests for assertSafeIcalHost, expandIcalEvents, and isWritableFeedType. OpenSpec proposal/design/spec updated to match. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016mqdsn2va7teLRuNPdyeZy
Addresses adversarial-review findings on the ICS-subscribe branch: - SSRF: IPv4-mapped IPv6 literals (e.g. http://[::ffff:127.0.0.1]/, which Node canonicalizes to ::ffff:7f00:1) bypassed the private-IP guard. Normalize both dotted and compressed-hex mapped forms back to IPv4 before classifying. - Read-only boundary: the generic POST /api/feeds/[id]/sync replaced a feed's events from caller JSON with no type check, letting a user wipe/forge their read-only ICAL mirror. Reject non-writable feed types before deleting. - Read-only boundary: PATCH /api/events/[id] spread the raw body into the update, so a smuggled feedId could retarget an event into a read-only/other feed after the writable check. Whitelist mutable presentation fields. - Recurrence: occurrences cancelled via EXDATE were still materialized. Honor EXDATE during expansion; left a //todo for RDATE/RECURRENCE-ID overrides. Adds ical-feed SSRF/EXDATE tests and a route test for both read-only guards. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016mqdsn2va7teLRuNPdyeZy
Second round of Codex adversarial-review hardening: - DoS: a hostile feed with a high-frequency unbounded rule (FREQ=SECONDLY) made expandIcalEvents enumerate every occurrence in the multi-year window before slicing to the cap. Use rrule's iterator callback so generation stops at MAX_EXPANDED_INSTANCES instead of materializing millions first. - Atomic subscribe: addFeed created the ICAL feed row, then syncFeed recorded a sync failure on it WITHOUT throwing, leaving a broken feed behind. Detect the recorded error after the first sync, roll the feed back, and rethrow so an invalid/unreachable/private URL creates no feed (matches the spec). - Documented the residual DNS-rebinding (validate-then-fetch TOCTOU) risk with a //todo to pin the vetted IP into the connection via a custom dispatcher. Adds a bounded-expansion test and a store test for the atomic-rollback path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016mqdsn2va7teLRuNPdyeZy
Third round of Codex adversarial-review hardening: - A byte-capped (10 MB) ICS can still carry many recurring masters; the per-master instance cap alone allowed expansion to masters*1000 rows before the bulk insert. Add a feed-level MAX_TOTAL_EXPANDED_EVENTS (50k) ceiling so expansion stops and the sync stays bounded regardless of master count. Two residual findings are deliberately deferred as documented scope boundaries (see //todo comments in src/lib/ical-feed.ts): - DNS-rebinding (validate-then-fetch TOCTOU): fully closing it needs connection- level IP pinning via a custom undici dispatcher, out of scope for this feature. - RECURRENCE-ID/RDATE override handling: moved/edited recurring instances; a larger recurrence-exception feature beyond the MVP subscribe path. Adds a test asserting the total-row cap holds across many recurring masters. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016mqdsn2va7teLRuNPdyeZy
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016mqdsn2va7teLRuNPdyeZy
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
1 participant
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 #4
Summary
Adds the ability to subscribe to a public iCal/ICS calendar by URL (e.g. a sports schedule or holiday calendar). Subscribed calendars are read-only, appear in the "Your Calendars" list alongside Google/Outlook/CalDAV, and refresh on sync.
Implementation: a new
ICALfeed type, a pure fetch+parse library (src/lib/ical-feed.ts), a sync route (/api/feeds/[id]/ical-sync), store wiring, and a subscribe form. Recurring masters are materialized into occurrence rows at sync time (the render path does not expand masters), mirroring the Google provider.Security & correctness hardening (3 Codex adversarial-review rounds)
Resolved findings (all with regression tests):
http://[::ffff:127.0.0.1]/, which Node canonicalizes to::ffff:7f00:1)./api/eventsPOST/PATCH/DELETE,/api/events/[id]PATCH/DELETE, and the generic/api/feeds/[id]/sync(which previously would have let a user wipe/forge a read-only mirror).PATCH /api/events/[id]now whitelists mutable fields so a smuggledfeedIdcan't retarget an event into a read-only/other feed.EXDATEcancellations are honored (cancelled occurrences are not shown).Deliberately deferred (documented
//todoinsrc/lib/ical-feed.ts)Two adversarial-review findings are out of scope for this MVP and left as documented follow-ups rather than blocking the feature:
fetchresolves it again. Fully closing it requires pinning the vetted IP into the connection (custom undici dispatcher / egress proxy). The layered guards above remain in place.RDATE/RECURRENCE-IDoverrides – moved/edited recurring instances.EXDATE(cancellations) is handled; full recurrence-exception support is a larger feature.Tests / gate
npm run test:unit– green except the 2 pre-existinggoogle-*timezone suites that already fail onmain(this branch touches no google files). +9 new tests acrossical-feed, the read-only routes, and the atomic-subscribe store path.npm run type-check– cleannpm run lint– cleanNote for reviewer
The Codex adversarial reviewer did not emit
approveafter 3 rounds — its remaining findings are the two documented deferrals above (DNS rebinding, RECURRENCE-ID), which an adversarial reviewer will always flag on an arbitrary-URL server-side fetcher. Flagging here so a human can make the merge call rather than treating it as auto-approved.🤖 Generated with Claude Code
https://claude.ai/code/session_016mqdsn2va7teLRuNPdyeZy