rbrooks/Iris-WLED

Watch

Fork 0

Import personal events from .ics files and subscriptions (#51) #121

Merged

claude-bot merged 2 commits from feat/51-ical-import into main

2026-09-04 22:41:44 +00:00

claude-bot commented

2026-09-04 22:19:07 +00:00

Contributor

Closes #51.

The other direction from #16: upload a .ics or subscribe to a URL, and birthdays and anniversaries become events that drive the lights. Subscriptions refresh as part of the weekly candidate job, so an event added to a personal calendar this week gets a candidate scheme on the same run rather than waiting for the next one.

De-dup needs no new column

Event ids are already {event_key}_{year}, so deriving event_key deterministically from the VEVENT UID makes a re-import collide on the primary key by construction. That's exactly the "de-dup on re-import/refresh by UID" the issue asks for, with no UID column and no second uniqueness rule to keep in step with the first.

UIDs are hashed rather than sanitised — real ones from Google and Apple (040000008200E00074C5B7101A82E008/...@corp) contain characters that aren't id-safe and carry no meaning worth preserving. All that matters is that the same UID always maps to the same key.

One load-bearing detail worth your eye

priority_resolver._SOURCE_RANK maps a source to a tiebreak rank and defaults anything unrecognised to 0 — below system. Adding the new "ical" source without registering it there would have made every imported event lose a priority tie against a builtin, silently, with nothing to show why.

That's the same shape as the leds.segs bug from #62: a lookup with a quiet default that turns a wrong answer into a plausible-looking one. Registered at the same rank as user, and I confirmed two tests fail if the entry is removed rather than assuming they would.

Upload and subscription behave differently, on purpose

A subscription is authoritative. A refresh overwrites name and dates, because the remote calendar is where those events are edited. That's what subscribing means.
An upload never overwrites. Nothing will ever refresh it, so clobbering an event someone has since edited would be a silent loss with no counterpart benefit.
Neither touches a non-ical event. A hash collision, or a hand-made event that happens to share an id, is left alone.
Deleting a subscription keeps its events. They're on the calendar and may already have approved schemes; removing a feed isn't the same as saying the birthdays never happened.

Recurrence

Yearly RRULEs expand per year, honouring UNTIL, COUNT and INTERVAL. Two decisions:

29 February falls back to the 28th rather than vanishing three years in four.
Non-yearly rules are skipped, not partially supported. Birthdays and anniversaries are the case the issue names and they're all yearly; a weekly rule would flood the calendar with things that aren't holidays.

DTEND is read as exclusive — the same off-by-one the exporter has to avoid in the other direction, here meaning every imported event would otherwise be a day too long.

Robustness

A failing feed is recorded against its subscription row rather than raised, so one dead calendar can't stop the weekly job for the rest, and the error shows in the UI instead of only in the container log. The manual sync endpoint still surfaces it as a 502. Fetches are bounded (20s, 5MB), and webcal:// is rewritten to https:// since that's what Apple and Google hand out.

Dependency change

icalendar moves from dev-only to runtime. #16 needed it only to validate the exported feed in tests; parsing an upload needs it in production. Flagged in the #16 close comment so it wouldn't be a surprise here.

Verification

488 backend tests pass (38 new), ruff, mypy clean
Migration 0009 applies on a fresh DB and alembic check reports no drift
Frontend in node:22: tsc -b, eslint, vitest (76), npm run build all clean
uvx pre-commit run --all-files passes

🤖 Generated with Claude Code

Closes #51. The other direction from #16: upload a `.ics` or subscribe to a URL, and birthdays and anniversaries become events that drive the lights. Subscriptions refresh as part of the **weekly candidate job**, so an event added to a personal calendar this week gets a candidate scheme on the same run rather than waiting for the next one. ### De-dup needs no new column Event ids are already `{event_key}_{year}`, so deriving `event_key` deterministically from the VEVENT `UID` makes a re-import collide on the **primary key by construction**. That's exactly the "de-dup on re-import/refresh by UID" the issue asks for, with no UID column and no second uniqueness rule to keep in step with the first. UIDs are hashed rather than sanitised — real ones from Google and Apple (`040000008200E00074C5B7101A82E008/...@corp`) contain characters that aren't id-safe and carry no meaning worth preserving. All that matters is that the same UID always maps to the same key. ### One load-bearing detail worth your eye `priority_resolver._SOURCE_RANK` maps a source to a tiebreak rank and **defaults anything unrecognised to `0`** — below `system`. Adding the new `"ical"` source without registering it there would have made every imported event lose a priority tie against a builtin, silently, with nothing to show why. That's the same shape as the `leds.segs` bug from #62: a lookup with a quiet default that turns a wrong answer into a plausible-looking one. Registered at the same rank as `user`, and I confirmed two tests fail if the entry is removed rather than assuming they would. ### Upload and subscription behave differently, on purpose - **A subscription is authoritative.** A refresh overwrites name and dates, because the remote calendar is where those events are edited. That's what subscribing means. - **An upload never overwrites.** Nothing will ever refresh it, so clobbering an event someone has since edited would be a silent loss with no counterpart benefit. - **Neither touches a non-`ical` event.** A hash collision, or a hand-made event that happens to share an id, is left alone. - **Deleting a subscription keeps its events.** They're on the calendar and may already have approved schemes; removing a feed isn't the same as saying the birthdays never happened. ### Recurrence Yearly RRULEs expand per year, honouring `UNTIL`, `COUNT` and `INTERVAL`. Two decisions: - **29 February falls back to the 28th** rather than vanishing three years in four. - **Non-yearly rules are skipped**, not partially supported. Birthdays and anniversaries are the case the issue names and they're all yearly; a weekly rule would flood the calendar with things that aren't holidays. `DTEND` is read as **exclusive** — the same off-by-one the exporter has to avoid in the other direction, here meaning every imported event would otherwise be a day too long. ### Robustness A failing feed is **recorded against its subscription row rather than raised**, so one dead calendar can't stop the weekly job for the rest, and the error shows in the UI instead of only in the container log. The manual sync endpoint still surfaces it as a 502. Fetches are bounded (20s, 5MB), and `webcal://` is rewritten to `https://` since that's what Apple and Google hand out. ### Dependency change `icalendar` moves from **dev-only to runtime**. #16 needed it only to validate the exported feed in tests; parsing an upload needs it in production. Flagged in the #16 close comment so it wouldn't be a surprise here. ### Verification - 488 backend tests pass (38 new), `ruff`, `mypy` clean - Migration `0009` applies on a fresh DB and `alembic check` reports no drift - Frontend in `node:22`: `tsc -b`, `eslint`, `vitest` (76), `npm run build` all clean - `uvx pre-commit run --all-files` passes 🤖 Generated with [Claude Code](https://claude.com/claude-code)

claude-bot added 1 commit

2026-09-04 22:19:08 +00:00

Import personal events from .ics files and subscriptions (#51 )

CI / Dockerfile lint (pull_request) Successful in 8s

Details

CI / Alembic migration check (pull_request) Successful in 34s

Details

CI / Pre-commit hooks (pull_request) Successful in 51s

Details

CI / Python lint & type-check (pull_request) Successful in 1m4s

Details

CI / Frontend lint, test & build (pull_request) Successful in 1m22s

Details

CI / Python tests (pull_request) Successful in 2m27s

Details

CI / Docker build, health smoke & E2E (pull_request) Successful in 3m56s

Details

5ffca40cf6

The other direction from #16: upload a .ics or subscribe to a URL, and
birthdays and anniversaries become events that drive the lights.
Subscriptions refresh as part of the weekly candidate job, so an event
added to a personal calendar this week gets a candidate scheme on the
same run rather than waiting for the next one.

De-duplication needs no new column. Event ids are already
{event_key}_{year}, so deriving event_key deterministically from the
VEVENT UID makes a re-import collide on the primary key by construction
-- exactly the "de-dup by UID" the issue asks for, with no UID column and
no second uniqueness rule to keep in step with the first. UIDs are hashed
rather than sanitised: real ones from Google and Apple contain characters
that are not id-safe and carry no meaning worth preserving.

A subscription is authoritative, so a refresh overwrites name and dates
-- the remote calendar is where those events are edited. A one-off upload
never overwrites, because nothing will refresh it and clobbering a
since-edited event would be a silent loss with no counterpart benefit.
Neither path touches an event whose source is not "ical", so a hash
collision or a hand-made event sharing an id is safe.

Registering the new source in priority_resolver._SOURCE_RANK is load-
bearing, not tidiness: that map defaults an unrecognised source to 0,
below "system", so an unregistered "ical" would have lost every priority
tie against a builtin with nothing to show why. Same shape as reading a
JSON field that does not exist and taking the fallback. Two tests fail if
the entry is removed.

Yearly RRULEs are expanded per year, honouring UNTIL, COUNT and INTERVAL;
29 February falls back to the 28th rather than vanishing three years in
four. Non-yearly rules are skipped rather than partially supported -- a
weekly rule would flood the calendar with things that are not holidays.
DTEND is read as exclusive, the same off-by-one the exporter has to avoid
in the other direction.

A failing feed is recorded against its subscription row rather than
raised, so one dead calendar cannot stop the weekly job for the rest, and
the error is visible in the UI instead of only in the container log.

icalendar moves from a dev-only to a runtime dependency; #16 needed it
only to validate the exported feed in tests.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

rbrooks added 1 commit

2026-09-04 22:30:04 +00:00

Close three security findings in the calendar importer (#51 )

CI / Dockerfile lint (pull_request) Successful in 16s

Details

CI / Alembic migration check (pull_request) Successful in 1m20s

Details

CI / Pre-commit hooks (pull_request) Successful in 1m44s

Details

CI / Python lint & type-check (pull_request) Successful in 2m7s

Details

CI / Frontend lint, test & build (pull_request) Successful in 2m31s

Details

CI / Python tests (pull_request) Successful in 5m31s

Details

CI / Docker build, health smoke & E2E (pull_request) Successful in 5m30s

Details

6f8eb0ffed

From a background security review of 5ffca40. All three are in code that
commit introduced.

SSRF. fetch_ics took an operator-supplied URL and fetched it server-side
with redirects followed automatically. From inside the container that
reaches sibling containers, host-only services and cloud metadata at
169.254.169.254 -- none of which the operator can necessarily reach
themselves -- and parse failures are stored in last_error and rendered in
the UI, so a blocked service's response can leak back through the error
text. Admin-only was not sufficient on its own.

URLs are now restricted to http(s), the host is resolved, and every
address it maps to must be global. Redirects are followed by hand so each
hop is re-validated: an allowed host that 302s to a link-local address
would otherwise walk straight past a check on the original URL. DNS
rebinding between the check and the request is still possible; closing
that needs the connection pinned to a vetted IP, which httpx does not
expose cleanly. Said so in the docstring rather than implying the guard
is total.

Size cap bypass. MAX_ICS_BYTES was compared against resp.content, which
materialises the entire body first -- so the limit did not limit
anything, and a hostile or merely broken feed could exhaust memory before
the check ran. The body is now streamed and abandoned once the cap is
passed.

Credential exposure. GET /subscriptions was require_viewer. A subscription
URL is a credential rather than a setting: the private links Google and
Apple hand out embed a secret in the path, and anyone holding one can read
that whole calendar, including everything unrelated to Iris. Now
require_admin.

Resolution is behind a seam so the guard is testable without the network
-- a suite that needs DNS to check a security control is one that will
eventually skip the check. The stub resolves literals to themselves and
only maps names, because a stub that returned a public address for
everything would let every private literal walk through the guard and
make these tests worse than useless.

Verified by neutering the guard: all seven SSRF cases fail without it,
including the redirect bypass.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

claude-bot merged commit 0cd749f2f5 into main

2026-09-04 22:41:44 +00:00

claude-bot deleted branch feat/51-ical-import

2026-09-04 22:41:45 +00:00

claude-bot referenced this pull request

2026-09-04 22:42:03 +00:00

iCal (.ics) import / subscription for personal events #51