Skip to content

feat(calendar): groundwork for CalDAV calendar integration (1/4) - #49

Merged
ArtyomSavchenko merged 3 commits into
Platform-Collective:developfrom
UncleDoomVSSP:caldav/pr1-groundwork
Oct 8, 2026
Merged

ArtyomSavchenko merged 3 commits into
Platform-Collective:developfrom
UncleDoomVSSP:caldav/pr1-groundwork

Conversation

@UncleDoomVSSP

Copy link
Copy Markdown
Contributor

Summary

First of four PRs adding a CalDAV client integration to the calendar service, so users can sync calendars from Nextcloud, iCloud, Fastmail, Radicale and Baikal alongside Google Calendar. This PR is groundwork only: it contains no CalDAV behaviour and no user-visible change for existing deployments.

Planned sequence:

  1. This PR: safe-fetch package, optional Google module, provider marker, Google guards.
  2. Read-only CalDAV inbound sync, connect endpoint and settings UI.
  3. Two-way sync via the existing CalendarEventCUD queue topic, with reconciliation.
  4. Compose block, local docker script, debug config and documentation.

Changes

  • New package packages/safe-fetch (@hcengineering/safe-fetch): a fetch wrapper for services that request user-supplied URLs. It resolves every DNS record and rejects private, loopback, link-local, CGNAT, multicast and reserved ranges in both IP families, pins the socket to the checked address so DNS rebinding cannot redirect the connection, validates every redirect hop, drops Authorization and Cookie across origins, switches to GET where the HTTP specification requires it, and enforces a timeout and a body size cap. Operators can allowlist hosts or CIDR ranges for servers on private networks. Dependencies: undici and ipaddr.js, both MIT. 87 unit tests, including end-to-end runs against local HTTP servers with an injected resolver.
  • pod-calendar: Google module becomes optional. Credentials and WATCH_URL are no longer required at startup. When either is missing or blank, the service logs that Google is disabled, skips the push handler, watch controller and calendar controller, keeps /event available, and answers /signin and /signout with 501 { error: 'google-disabled' }. Deployments that set both variables see no change.
  • plugins/calendar, models/calendar: new integration kind caldav-calendar (distinct from the existing caldav kind, which is the platform acting as a CalDAV server) and a CalDavCalendar mixin on ExternalCalendar with hidden accountKey, href and ctag fields. Google calendars never carry the mixin, so no migration is needed.
  • Provider guards in the Google paths that previously loaded every ExternalCalendar: IncomingSyncManager.getMyCalendars, WorkspaceClient.init, and getTokenByEvent in the outbound client. Each now ignores calendars carrying the mixin.
  • rush.json entry for the new package and the matching lockfile update.

Verification

  • rush validate --to @hcengineering/pod-calendar --to @hcengineering/model-calendar through the full upstream chain.
  • rushx _phase:validate for safe-fetch run twice, to confirm the incremental pass is stable.
  • jest for safe-fetch: 87 passed. ESLint clean on every touched file. Formatting via rush fast-format.
  • gitleaks on the staged diff and semgrep (--config auto) on the 24 changed files, both clean.
  • Not yet verified: a manual Google sync against a live deployment with both configurations. I would welcome a maintainer running that if a test environment is to hand.

Notes for reviewers

  • The source files in safe-fetch are named safe-url.ts and safe-fetch-types.ts rather than url.ts and types.ts on purpose. The rig's validate step adds a package's own types/ directory to the compiler type roots, and @types/node imports the bare specifier url, so a file named url.ts makes the second validate pass fail with TS5055.
  • Several pre-existing lines in the touched files are not in Prettier shape on develop; the formatter rewrapped only the lines this PR added.
  • New files carry the SPDX identifier only, per the current AGENTS.md.

🤖 Generated with Claude Code

Preparation for a CalDAV client integration in pod-calendar, with no
user-visible change for existing deployments.

- Add @hcengineering/safe-fetch, an SSRF-guarded fetch for services that
  request user-supplied URLs: DNS resolution with blocked private and
  reserved ranges, pinned connections, per-hop redirect validation,
  Authorization stripping across origins, timeouts and body size caps.
  Unit tested with stubbed DNS and local HTTP servers.
- Make the Google Calendar module in pod-calendar optional: the service
  starts without Credentials and WATCH_URL, logs that Google is disabled,
  keeps /event available and answers the Google endpoints with 501.
- Add the caldav-calendar integration kind and the CalDavCalendar mixin on
  ExternalCalendar, so provider-specific code can tell CalDAV calendars
  apart from Google ones. No data migration is needed.
- Restrict the Google sync and outbound paths to calendars without that
  mixin.

Signed-off-by: UncleDoomVSSP <uncledoom@pm.me>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@ArtyomSavchenko

Copy link
Copy Markdown
Member

Hi @UncleDoomVSSP
Thank you for the contribution.
Could you please take a look at the following review comments?

Should fix before merge

1. /event still runs Google sync when Google is disabled

services/calendar/pod-calendar/src/main.ts:197

When GoogleEnabled is false, /event still calls OutcomingClient.push on every event write. Each call takes the workspace lock, opens a workspace connection and looks up the Google secret.

If a deployment once had Google configured, old secrets still exist in the account service. Then new OutcomingClient() throws Google Credentials not provided, and the error is logged on every event.

Suggestion: make /event return immediately when !config.GoogleEnabled, or check that flag at the top of OutcomingClient.push. PR 3 sends CalDAV writes through the CalendarEventCUD queue, so /event stays Google-only.

2. Saved addresses are never cleared

packages/safe-fetch/src/fetch.ts:30

PinnedAddresses.pinned keeps one entry for every hostname a createSafeFetch instance has contacted, and never removes any. That is fine for a handful of CalDAV servers. But the README offers the package for link previews and webhooks, where hostnames are unlimited and controlled by users, so memory keeps growing.

Suggestion: delete the entry when the request finishes, or use a small cache with expiry.


Worth addressing (low severity)

3. A Request object loses its method, headers and body

packages/safe-fetch/src/fetch.ts:117

Only input.url is read, so safeFetch(new Request(url, { method: 'PROPFIND', headers })) quietly sends a plain GET. The function is typed as typeof fetch, so callers will expect fetch behaviour.

Suggestion: merge the Request's method, headers and body into init, or reject a Request input.

4. The body size limit is incomplete

packages/safe-fetch/src/fetch.ts:68–101, :159

  • A HEAD response (or a 304) with a large Content-Length throws BODY_TOO_LARGE, even though there is no body.
  • formData() and clone() are not overridden, so they fail on the locked original stream.
  • A redirect with no Location header comes back with its body already cancelled (fetch.ts:159).

5. A timeout during the body read gives the wrong error

If the timer fires after the headers arrive, the caller probably gets Node's own TimeoutError (a DOMException) instead of SafeFetchError('TIMEOUT'). The README says the timeout covers the whole request, so callers will expect the same error code either way.

6. A bad blockedRanges entry fails late

packages/safe-fetch/src/ranges.ts:50

ipaddr.parseCIDR only runs when the first request is checked, so an operator's typo shows up as a generic error at request time.

Suggestion: parse and validate allowlist and blockedRanges once, inside createSafeFetch.

7. Two IPv6 forms that can reach IPv4 addresses are not blocked

packages/safe-fetch/src/ranges.ts:28–38

  • 64:ff9b:1::/48: the local NAT64 prefix (RFC 8215).
  • ::ffff:0:0/96: the IPv4-translated form ::ffff:0:a.b.c.d.
    Both only matter on networks with that translation set up, but they are cheap to add next to 64:ff9b::/96.

8. Proxy-Authorization is not dropped on cross-origin redirects

packages/safe-fetch/src/fetch.ts:175

Add it next to authorization and cookie.


Nits

  • services/calendar/pod-calendar/src/config.ts:52: GoogleEnabled: 'GOOGLE_ENABLED' in envMap suggests a GOOGLE_ENABLED environment variable exists, but nothing reads it. Leave that key out of envMap's type instead of adding a placeholder entry.
  • models/calendar/src/index.ts: href and ctag both use calendar.string.Calendar as their label. They are hidden, so this is harmless, but proper labels would help later debugging.
  • services/calendar/pod-calendar/src/utils.ts:287: the new blank-value check in getGoogleClient repeats what config already does. Harmless, but it can be removed.

@UncleDoomVSSP

Copy link
Copy Markdown
Contributor Author

Thanks for the careful review, @ArtyomSavchenko. All eleven points are addressed in the follow-up commit; details below.

Should fix

  1. /event now returns immediately when GoogleEnabled is false, before the workspace lock and the secret lookup. CalDAV writes will come through the CalendarEventCUD queue in PR 3, so /event stays Google-only.
  2. Pinned addresses are now reference-counted per host and removed when the last in-flight request for that host has established its connection. A unit test covers acquire and release.

Low severity

  1. A Request input now contributes its method, headers, body and signal, with init taking precedence, matching fetch. The body is buffered so it can be re-sent after a redirect. Tested with PROPFIND and an init override.
  2. Body limiting now skips responses that cannot carry a body (HEAD, 101, 204, 205, 304) regardless of Content-Length. The limited body is wrapped in a native Response, so formData() and clone() work as usual; url and redirected are carried over. A redirect without Location is returned as an ordinary readable response. Each case has a test.
  3. A timeout that fires while the body is being read now surfaces as SafeFetchError('TIMEOUT'); tested against a server that sends headers and then stalls.
  4. createSafeFetch validates allowlist and blockedRanges once at creation and throws a descriptive error on a malformed entry.
  5. Added 64:ff9b:1::/48 (blocked as a whole, like the well-known NAT64 prefix) and the IPv4-translated form, which I read as ::ffff:0:0:0/96 (::ffff:0:a.b.c.d), checked through its embedded IPv4 address like the mapped form. Tests cover both.
  6. Proxy-Authorization is dropped alongside Authorization and Cookie on cross-origin redirects, with a test.

Nits

  • GoogleEnabled is excluded from the envMap type instead of carrying a placeholder.
  • The mixin fields now have distinct embedded labels.
  • The blank-value check in getGoogleClient is reverted; config handles it.

safe-fetch tests: 101 passing (was 87). Full validation of safe-fetch, pod-calendar and model-calendar is clean.

UncleDoomVSSP and others added 2 commits October 7, 2026 11:30
Follow-up to review comments on Platform-Collective#49.

pod-calendar:
- /event returns immediately when the Google module is disabled, instead
  of taking the workspace lock and looking up Google secrets per event.
- GoogleEnabled is excluded from the envMap type; no placeholder entry.
- Revert the duplicate blank-value check in getGoogleClient.

safe-fetch:
- Pinned addresses are reference-counted and released once a request has
  connected, so the map no longer grows with the hosts contacted.
- A Request input contributes method, headers, body and signal; init wins.
- Body limiting skips HEAD and null-body statuses, wraps the limited
  stream in a native Response so formData() and clone() work, and keeps
  url and redirected. Redirects without Location are returned readable.
- A timeout during the body read surfaces as SafeFetchError('TIMEOUT').
- allowlist and blockedRanges are validated once in createSafeFetch.
- Block 64:ff9b:1::/48 and check the IPv4-translated ::ffff:0:a.b.c.d
  form through its embedded address.
- Drop Proxy-Authorization on cross-origin redirects.
- Tests for each change; 101 in total.

models/calendar:
- Distinct embedded labels for the CalDavCalendar mixin fields.

Signed-off-by: UncleDoomVSSP <uncledoom@pm.me>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: UncleDoomVSSP <uncledoom@pm.me>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@ArtyomSavchenko
ArtyomSavchenko merged commit 3271181 into Platform-Collective:develop Oct 8, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants