Skip to content

Security hardening, structured logging & regression tests for 1.0#9

Merged
olivierlambert merged 1 commit into
mainfrom
target-1.0
Mar 12, 2026
Merged

Security hardening, structured logging & regression tests for 1.0#9
olivierlambert merged 1 commit into
mainfrom
target-1.0

Conversation

@olivierlambert

Copy link
Copy Markdown
Owner

Summary

This PR prepares calrs for a 1.0 release with comprehensive security hardening, production observability, reliability improvements, and regression tests.

21 files changed, +1,583 / -244 lines — tests: 191 → 219 (+28)

Security

CSRF protection (double-submit cookie pattern)

All 31 POST handlers are now protected against cross-site request forgery:

  • Middleware (csrf_cookie_middleware) sets a calrs_csrf cookie on every response
  • Client-side JS in base.html reads the cookie and injects a hidden _csrf field into all forms
  • Server verifies cookie matches form field on every POST — rejects with 403 on mismatch
  • Multipart forms (avatar/logo upload) pass the token via query parameter
  • Auth forms (login, register, logout) also protected

Booking rate limiting

Per-IP rate limiter on all 4 booking endpoints (user, group, team link, legacy):

  • 10 requests per 5-minute window
  • Uses X-Forwarded-For header (reverse proxy aware)
  • Returns friendly error message when limited

Server-side input validation

  • Booking forms: name (1–255 chars), email (format + length), notes (max 5,000 chars)
  • Date range: bookings rejected if >365 days in the future
  • Registration: name length (1–255), email format validation
  • Settings: name length, booking email format validation
  • Avatar upload: strict content-type whitelist (JPEG, PNG, GIF, WebP) — rejects unknown types instead of defaulting to PNG
  • HTML templates: maxlength attributes on form inputs (defense in depth)

Double-booking prevention

  • New migration 016_booking_unique.sql: partial unique index on (event_type_id, start_at) WHERE status IN ('confirmed', 'pending')
  • All 4 booking handlers wrap availability check + INSERT in a BEGIN IMMEDIATE transaction
  • Two simultaneous requests for the same slot: one succeeds, the other gets "This slot is no longer available"

Reliability

Crash-proof web handlers

Replaced ~37 bare .unwrap() calls in web handlers with proper error handling:

  • Template rendering → match returning "Internal error" HTML
  • Date/time parsing → match returning "Invalid date/time" response
  • Response builders → unwrap_or_else with fallback
  • Troubleshoot page time constructors → unwrap_or(NaiveTime::MIN)
  • OIDC admin form → unwrap_or_default()

Graceful shutdown

calrs serve now handles SIGINT (Ctrl+C) and SIGTERM:

  • Drains in-flight requests before exiting
  • Logs "Shutdown signal received, stopping gracefully..."
  • Clean deploys with zero dropped connections

Error handling

No web handler can panic from user input. All paths return proper HTTP responses.

Observability

Structured logging (50 tracing points)

Replaced all eprintln!/println! in web paths with tracing structured logging. Added tower-http TraceLayer for HTTP request tracing.

Category Level Events
Auth info/warn Login success/failure (with email + IP), registration, logout, OIDC login/failures, rate limited
Bookings info Created, cancelled, approved, declined, guest self-cancel, confirmed, reminder sent
CalDAV info/error Sync started/completed, write-back/delete failures, source added/removed, write calendar set
Admin info/warn Role changes, user toggle, auth/OIDC config updates, impersonation start/stop
Email debug/error Delivery success, send failures
HTTP info Every request via TraceLayer (method, path, status, latency)
Database info Migration applied on startup
Event types info/debug Created, deleted, toggled
Server info Startup, shutdown

Default: RUST_LOG=calrs=info,tower_http=info. Visible in journalctl -u calrs -f or docker logs -f calrs.

Tests (+28 new, 191 → 219 total)

ICS regression tests (6) — prevent the bugs fixed in v0.18.2

  • convert_to_utc with Europe/Paris, UTC, and invalid timezone fallback
  • Location field has no trailing whitespace — prevents ORGANIZER leaking into LOCATION (the exact bug from Monday)
  • DTSTART/DTEND always end with Z — prevents floating-time bug
  • Cancel ICS also has UTC times

Input validation tests (14)

  • validate_booking_input: empty name, name too long, valid name, empty email, email without @, email without domain dot, valid email, notes too long, notes within limit, None notes
  • validate_date_not_too_far: within range, exactly 365 days, 367 days rejected, past date passes

CSRF tests (8)

  • Token generation returns valid UUID format
  • Cookie extraction from headers (present, missing, among other cookies)
  • Token verification (match, mismatch, missing form token, missing cookie)

Documentation updates

  • docs/src/security.md — Replaced "No CSRF tokens" limitation with full CSRF, rate limiting, input validation, double-booking prevention, error handling sections. Updated credential storage (now AES-256-GCM).
  • docs/src/architecture.md — Added migrations 015-016, test count 147→219, tracing/tower-http deps, middleware table.
  • docs/src/deployment.md — Added RUST_LOG env var, full Observability section with categories table and viewing instructions.
  • docs/src/booking-flow.md — Guest self-cancellation, reminder email row, double-booking prevention note.
  • docs/src/introduction.md — Structured logging and security hardening in feature list.
  • README.md — New Observability section with config, categories, example output.
  • CHANGELOG.md — v0.18.2 entry + feature table entries for all 1.0 items.
  • CLAUDE.md — tracing/tower-http in tech stack, migration 016, security and observability sections.

Files changed

File What
src/web/mod.rs CSRF infra + middleware, rate limiting on 4 handlers, input validation, crash-proof unwraps, tracing, double-booking transactions
src/auth.rs CSRF on login/register/logout, input validation, OIDC logging, tracing
src/email.rs Email delivery logging, 6 ICS regression tests
src/main.rs Graceful shutdown, tracing subscriber init
src/db.rs Migration 016 registered, migration logging
src/commands/sync.rs Sync completed + on-demand sync logging
migrations/016_booking_unique.sql Partial unique index for double-booking prevention
templates/base.html CSRF JS injection script
templates/book.html maxlength on form inputs
templates/settings.html maxlength on form inputs
templates/auth/register.html maxlength on form inputs
Cargo.toml Added tracing, tracing-subscriber, tower-http deps
docs/src/*.md Security, architecture, deployment, booking-flow, introduction
README.md Observability section
CHANGELOG.md v0.18.2 + 1.0 feature entries
CLAUDE.md Tech stack, migrations, security/observability docs

Test plan

  • cargo test — 219 tests pass
  • cargo clippy -- -D warnings — clean
  • cargo fmt --check — clean
  • Manual test: book a slot, verify CSRF cookie is set and form submits work
  • Manual test: try booking without CSRF token (should get 403)
  • Manual test: check journalctl output after login + booking
  • Manual test: two simultaneous bookings for same slot (one should fail)
  • Deploy to staging and verify no regressions

🤖 Generated with Claude Code

…r 1.0

CSRF protection (double-submit cookie) on all 31 POST handlers, per-IP
rate limiting on booking endpoints, server-side input validation,
double-booking prevention via SQLite unique index + transactions,
crash-proof web handlers (37 unwrap removals), graceful shutdown,
and 50 structured tracing points across auth, bookings, CalDAV, admin,
and email delivery.

28 new regression tests (191 → 219) covering ICS timezone/location bugs,
input validation, CSRF token verification, and date range validation.

Updated docs: security.md, architecture.md, deployment.md, booking-flow.md,
introduction.md, README.md, CHANGELOG.md, and CLAUDE.md.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 8.69565% with 462 lines in your changes missing coverage. Please review.
✅ Project coverage is 13.49%. Comparing base (a14bc75) to head (4eec17a).

Files with missing lines Patch % Lines
src/web/mod.rs 9.11% 389 Missing ⚠️
src/auth.rs 0.00% 50 Missing ⚠️
src/main.rs 0.00% 12 Missing ⚠️
src/email.rs 0.00% 9 Missing ⚠️
src/commands/sync.rs 0.00% 2 Missing ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files
@@            Coverage Diff             @@
##             main       #9      +/-   ##
==========================================
- Coverage   13.59%   13.49%   -0.11%     
==========================================
  Files          16       16              
  Lines        5552     5930     +378     
==========================================
+ Hits          755      800      +45     
- Misses       4797     5130     +333     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@olivierlambert olivierlambert merged commit d6df0e0 into main Mar 12, 2026
4 checks passed
@olivierlambert olivierlambert deleted the target-1.0 branch March 12, 2026 22:23
huntervcx pushed a commit to DYB-Corp/calrs that referenced this pull request Jun 3, 2026
Blockers (from review):
- Migration 050_provider_type renumbered to 054_provider_type to avoid
  collision with upstream main's 051-053. src/db.rs migration array +
  CLAUDE.md index updated.
- Migration-count assertions in db.rs tests bumped from 50 to 54.

Substantive review items folded in:
- sync_folder_items now caps at 200 SOAP iterations; a server that
  never sets IncludesLastItemInRange=true returns partial results and
  is resumed from the latest cursor next sync. (olivierlambert#8)
- synth_vcalendar emits DTSTAMP per RFC 5545; EWS doesn't expose a
  stable last-modified, so we use Utc::now(). (olivierlambert#6)
- EWS sync_delta comment rewritten to acknowledge "EWS sources rely
  on full fetches" rather than claiming the cursor gets bootstrapped.
  (olivierlambert#3, option A)
- fetch_events_since wired into sync_ews_source with the same 90-day
  lookback used by the CalDAV path. The EWS impl uses CalendarView
  (server-side window). remove_orphaned_ews_events now takes the same
  since_prefix scoping as the CalDAV variant so events outside the
  window are not flagged as orphans. (olivierlambert#4)
- format_dt TODO documenting naive-local TZID drift on non-recurring
  items. (olivierlambert#7)
- Autodiscover redirect policy now carries an explicit comment about
  the SSRF residual risk on intermediate Location headers. (olivierlambert#5)
- 054_provider_type.sql explains the calendars.href / calendars.ctag
  reuse for EWS folder ItemId / ChangeKey. (olivierlambert#9)
- extract_vcalendar comment corrected: the \r\n /\r\n\t replacements
  are RFC 5545 §3.1 line-fold unfolding, not "stray indentation".

Rebase side-effects:
- SSRF guard now uses upstream's CALRS_ALLOW_PRIVATE_HOSTS allowlist
  (hostname-scoped) instead of the previous boolean toggle. private_
  host_allowlist() is exposed for the serve startup WARN log.
- Sources flow (commands/source.rs, commands/sync.rs, web/mod.rs)
  reconciled with upstream's OAuth2 dispatch + orphan-cancel work:
  EWS sources go through the provider trait, CalDAV sources keep the
  CaldavClient path (basic-auth or OAuth2 unchanged).
- new sync_ews_source/upsert_calendar_provider/upsert_provider_events
  helpers in commands/sync.rs to keep the EWS path off the CalDAV
  CaldavClient signature.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
huntervcx pushed a commit to DYB-Corp/calrs that referenced this pull request Jun 3, 2026
Blockers (from review):
- Migration 050_provider_type renumbered to 054_provider_type to avoid
  collision with upstream main's 051-053. src/db.rs migration array +
  CLAUDE.md index updated.
- Migration-count assertions in db.rs tests bumped from 50 to 54.

Substantive review items folded in:
- sync_folder_items now caps at 200 SOAP iterations; a server that
  never sets IncludesLastItemInRange=true returns partial results and
  is resumed from the latest cursor next sync. (olivierlambert#8)
- synth_vcalendar emits DTSTAMP per RFC 5545; EWS doesn't expose a
  stable last-modified, so we use Utc::now(). (olivierlambert#6)
- EWS sync_delta comment rewritten to acknowledge "EWS sources rely
  on full fetches" rather than claiming the cursor gets bootstrapped.
  (olivierlambert#3, option A)
- fetch_events_since wired into sync_ews_source with the same 90-day
  lookback used by the CalDAV path. The EWS impl uses CalendarView
  (server-side window). remove_orphaned_ews_events now takes the same
  since_prefix scoping as the CalDAV variant so events outside the
  window are not flagged as orphans. (olivierlambert#4)
- format_dt TODO documenting naive-local TZID drift on non-recurring
  items. (olivierlambert#7)
- Autodiscover redirect policy now carries an explicit comment about
  the SSRF residual risk on intermediate Location headers. (olivierlambert#5)
- 054_provider_type.sql explains the calendars.href / calendars.ctag
  reuse for EWS folder ItemId / ChangeKey. (olivierlambert#9)
- extract_vcalendar comment corrected: the \r\n /\r\n\t replacements
  are RFC 5545 §3.1 line-fold unfolding, not "stray indentation".

Rebase side-effects:
- SSRF guard now uses upstream's CALRS_ALLOW_PRIVATE_HOSTS allowlist
  (hostname-scoped) instead of the previous boolean toggle. private_
  host_allowlist() is exposed for the serve startup WARN log.
- Sources flow (commands/source.rs, commands/sync.rs, web/mod.rs)
  reconciled with upstream's OAuth2 dispatch + orphan-cancel work:
  EWS sources go through the provider trait, CalDAV sources keep the
  CaldavClient path (basic-auth or OAuth2 unchanged).
- new sync_ews_source/upsert_calendar_provider/upsert_provider_events
  helpers in commands/sync.rs to keep the EWS path off the CalDAV
  CaldavClient signature.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
olivierlambert pushed a commit that referenced this pull request Jun 4, 2026
* feat(providers): add Microsoft Exchange (EWS) calendar provider

Introduces a generic CalendarProvider trait so calrs is no longer
hard-wired to CalDAV. Adds an EWS implementation targeting on-prem
Exchange 2019 (also compatible with 2016/2013) with Autodiscover,
calendar listing, full + windowed event fetch, create/delete, and
SyncFolderItems delta sync.

The CalDAV path is preserved unchanged behind a thin adapter; sync,
source management, and write-back now dispatch on a new provider_type
column (migration 050). HTTPS-only and SSRF-safe URL validation is
shared across both providers, and Autodiscover candidate URLs are
re-validated before they get hit.

Layout:
  src/providers/     trait + factory + caldav adapter
  src/ews/           soap, autodiscover, operations, parse, ical synth

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

* fixup(ews): drop unused Context import + dead summary_view, collapse
            duplicate connection-check arms, rustfmt

Cargo.lock is updated by Cargo's first compile after a fresh dependency
add (async-trait); committing it so a clean checkout doesn't churn it.

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

* wip: save ews provider work

* feat: clean sources presets filtered via backend

* fixup(ews): address review blockers + substantive items

Blockers (from review):
- Migration 050_provider_type renumbered to 054_provider_type to avoid
  collision with upstream main's 051-053. src/db.rs migration array +
  CLAUDE.md index updated.
- Migration-count assertions in db.rs tests bumped from 50 to 54.

Substantive review items folded in:
- sync_folder_items now caps at 200 SOAP iterations; a server that
  never sets IncludesLastItemInRange=true returns partial results and
  is resumed from the latest cursor next sync. (#8)
- synth_vcalendar emits DTSTAMP per RFC 5545; EWS doesn't expose a
  stable last-modified, so we use Utc::now(). (#6)
- EWS sync_delta comment rewritten to acknowledge "EWS sources rely
  on full fetches" rather than claiming the cursor gets bootstrapped.
  (#3, option A)
- fetch_events_since wired into sync_ews_source with the same 90-day
  lookback used by the CalDAV path. The EWS impl uses CalendarView
  (server-side window). remove_orphaned_ews_events now takes the same
  since_prefix scoping as the CalDAV variant so events outside the
  window are not flagged as orphans. (#4)
- format_dt TODO documenting naive-local TZID drift on non-recurring
  items. (#7)
- Autodiscover redirect policy now carries an explicit comment about
  the SSRF residual risk on intermediate Location headers. (#5)
- 054_provider_type.sql explains the calendars.href / calendars.ctag
  reuse for EWS folder ItemId / ChangeKey. (#9)
- extract_vcalendar comment corrected: the \r\n /\r\n\t replacements
  are RFC 5545 §3.1 line-fold unfolding, not "stray indentation".

Rebase side-effects:
- SSRF guard now uses upstream's CALRS_ALLOW_PRIVATE_HOSTS allowlist
  (hostname-scoped) instead of the previous boolean toggle. private_
  host_allowlist() is exposed for the serve startup WARN log.
- Sources flow (commands/source.rs, commands/sync.rs, web/mod.rs)
  reconciled with upstream's OAuth2 dispatch + orphan-cancel work:
  EWS sources go through the provider trait, CalDAV sources keep the
  CaldavClient path (basic-auth or OAuth2 unchanged).
- new sync_ews_source/upsert_calendar_provider/upsert_provider_events
  helpers in commands/sync.rs to keep the EWS path off the CalDAV
  CaldavClient signature.

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

---------

Co-authored-by: Arthur Perrot <aperrot@dyb.fr>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
huntervcx pushed a commit to DYB-Corp/calrs that referenced this pull request Jun 25, 2026
Security hardening, structured logging & regression tests for 1.0
huntervcx added a commit to DYB-Corp/calrs that referenced this pull request Jun 25, 2026
…ert#103)

* feat(providers): add Microsoft Exchange (EWS) calendar provider

Introduces a generic CalendarProvider trait so calrs is no longer
hard-wired to CalDAV. Adds an EWS implementation targeting on-prem
Exchange 2019 (also compatible with 2016/2013) with Autodiscover,
calendar listing, full + windowed event fetch, create/delete, and
SyncFolderItems delta sync.

The CalDAV path is preserved unchanged behind a thin adapter; sync,
source management, and write-back now dispatch on a new provider_type
column (migration 050). HTTPS-only and SSRF-safe URL validation is
shared across both providers, and Autodiscover candidate URLs are
re-validated before they get hit.

Layout:
  src/providers/     trait + factory + caldav adapter
  src/ews/           soap, autodiscover, operations, parse, ical synth

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

* fixup(ews): drop unused Context import + dead summary_view, collapse
            duplicate connection-check arms, rustfmt

Cargo.lock is updated by Cargo's first compile after a fresh dependency
add (async-trait); committing it so a clean checkout doesn't churn it.

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

* wip: save ews provider work

* feat: clean sources presets filtered via backend

* fixup(ews): address review blockers + substantive items

Blockers (from review):
- Migration 050_provider_type renumbered to 054_provider_type to avoid
  collision with upstream main's 051-053. src/db.rs migration array +
  CLAUDE.md index updated.
- Migration-count assertions in db.rs tests bumped from 50 to 54.

Substantive review items folded in:
- sync_folder_items now caps at 200 SOAP iterations; a server that
  never sets IncludesLastItemInRange=true returns partial results and
  is resumed from the latest cursor next sync. (olivierlambert#8)
- synth_vcalendar emits DTSTAMP per RFC 5545; EWS doesn't expose a
  stable last-modified, so we use Utc::now(). (olivierlambert#6)
- EWS sync_delta comment rewritten to acknowledge "EWS sources rely
  on full fetches" rather than claiming the cursor gets bootstrapped.
  (olivierlambert#3, option A)
- fetch_events_since wired into sync_ews_source with the same 90-day
  lookback used by the CalDAV path. The EWS impl uses CalendarView
  (server-side window). remove_orphaned_ews_events now takes the same
  since_prefix scoping as the CalDAV variant so events outside the
  window are not flagged as orphans. (olivierlambert#4)
- format_dt TODO documenting naive-local TZID drift on non-recurring
  items. (olivierlambert#7)
- Autodiscover redirect policy now carries an explicit comment about
  the SSRF residual risk on intermediate Location headers. (olivierlambert#5)
- 054_provider_type.sql explains the calendars.href / calendars.ctag
  reuse for EWS folder ItemId / ChangeKey. (olivierlambert#9)
- extract_vcalendar comment corrected: the \r\n /\r\n\t replacements
  are RFC 5545 §3.1 line-fold unfolding, not "stray indentation".

Rebase side-effects:
- SSRF guard now uses upstream's CALRS_ALLOW_PRIVATE_HOSTS allowlist
  (hostname-scoped) instead of the previous boolean toggle. private_
  host_allowlist() is exposed for the serve startup WARN log.
- Sources flow (commands/source.rs, commands/sync.rs, web/mod.rs)
  reconciled with upstream's OAuth2 dispatch + orphan-cancel work:
  EWS sources go through the provider trait, CalDAV sources keep the
  CaldavClient path (basic-auth or OAuth2 unchanged).
- new sync_ews_source/upsert_calendar_provider/upsert_provider_events
  helpers in commands/sync.rs to keep the EWS path off the CalDAV
  CaldavClient signature.

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

---------

Co-authored-by: Arthur Perrot <aperrot@dyb.fr>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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