From 5eb3130898ad0c54e5f278b82510e0b00358ff40 Mon Sep 17 00:00:00 2001 From: HubEight Date: Tue, 15 Sep 2026 06:54:47 +0000 Subject: [PATCH 1/3] Only delete orphan birthday events after a truly complete fetch fetch_complete compared fetched vCards with listed ones, so a failed addressbook listing (HTTP error, exception, broken XML) was never counted and still yielded complete=True. Unparseable birthdays were counted as fetched, and an empty contact list with a complete fetch deleted every birthday event. - Clear fetch_complete on any listing, download or parse failure; _parse_vcard now raises on unreadable data and returns None only when there is no BDAY. Listing PROPFIND gets a timeout. - Restore main_sync returning False when no contacts were found, so orphan delete never runs with an empty set. - Match orphans on name slug plus month/day, so a changed birthday replaces the old event instead of leaving a duplicate. - Share the UID slug between event creation, lookup and orphan delete. - The retry docstring blamed IPv6: urllib3 tries every resolved address and IPv4 sorts first, so ENETUNREACH is only the last error. Add test_sync.py covering each case. Co-Authored-By: Claude Opus 5 --- bdaysync/caldav_client.py | 28 ++++---- bdaysync/cardav_client.py | 50 +++++--------- bdaysync/main.py | 35 +++++----- bdaysync/test_sync.py | 140 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 190 insertions(+), 63 deletions(-) create mode 100644 bdaysync/test_sync.py diff --git a/bdaysync/caldav_client.py b/bdaysync/caldav_client.py index 8b4619e..bff3cd7 100644 --- a/bdaysync/caldav_client.py +++ b/bdaysync/caldav_client.py @@ -8,12 +8,17 @@ from typing import Dict, List, Optional import vobject import caldav - -BIRTHDAY_UID_RE = re.compile(r'^birthday-(.+)-(\d{8})$') from config import get_birthday_config logger = logging.getLogger(__name__) +BIRTHDAY_UID_RE = re.compile(r'^birthday-(.+)-\d{4}(\d{4})$') + + +def birthday_slug(name: str) -> str: + """Name part of a birthday event UID: birthday-{slug}-{YYYYMMDD}""" + return name.replace(' ', '-').lower() + class CalDAVClient: """Client for creating events in CalDAV server""" @@ -93,7 +98,7 @@ def create_birthday_event(self, contact: Dict, year: int = None) -> bool: return False # Create unique UID - event_uid = f"birthday-{name.replace(' ', '-').lower()}-{event_date.strftime('%Y%m%d')}" + event_uid = f"birthday-{birthday_slug(name)}-{event_date.strftime('%Y%m%d')}" # Create iCalendar event cal = vobject.iCalendar() @@ -168,15 +173,11 @@ def _format_reminder_message(self, name: str, days_before: int) -> str: return f"{name}'s birthday is in {days_before} days!" def delete_orphans(self, contacts: List[Dict]) -> int: - """Delete birthday-* events whose name slug is not in the current BDAY set.""" - wanted = {contact['name'].replace(' ', '-').lower() for contact in contacts} - events = self.calendar.events() - if not events: - logger.warning("No calendar events listed; skipping orphan delete") - return 0 + """Delete birthday-* events whose name and month/day match no current contact.""" + wanted = {(birthday_slug(c['name']), c['birthday'].strftime('%m%d')) for c in contacts} deleted = 0 - for ev in events: + for ev in self.calendar.events(): try: parsed = vobject.readOne(ev.data) if not hasattr(parsed, 'vevent') or not hasattr(parsed.vevent, 'uid'): @@ -185,8 +186,7 @@ def delete_orphans(self, contacts: List[Dict]) -> int: match = BIRTHDAY_UID_RE.match(uid) if not match: continue - slug = match.group(1) - if slug in wanted: + if match.groups() in wanted: continue logger.info(f"Deleting orphan birthday event: {uid}") ev.delete() @@ -229,7 +229,7 @@ def _find_existing_event(self, name: str, date) -> Optional: # Also check by UID pattern if hasattr(cal.vevent, 'uid'): uid = cal.vevent.uid.value - expected_uid = f"birthday-{name.replace(' ', '-').lower()}" + expected_uid = f"birthday-{birthday_slug(name)}" if uid.startswith(expected_uid): return event except Exception as e: @@ -254,7 +254,7 @@ def _find_existing_event(self, name: str, date) -> Optional: return event if hasattr(cal.vevent, 'uid'): uid = cal.vevent.uid.value - expected_uid = f"birthday-{name.replace(' ', '-').lower()}" + expected_uid = f"birthday-{birthday_slug(name)}" if uid.startswith(expected_uid): return event except Exception as e: diff --git a/bdaysync/cardav_client.py b/bdaysync/cardav_client.py index 2099980..ff64f1c 100644 --- a/bdaysync/cardav_client.py +++ b/bdaysync/cardav_client.py @@ -29,8 +29,6 @@ def __init__(self, server_url: str, username: str, password: str): # Discover addressbooks self.addressbook_urls = [] - self.vcard_listed = 0 - self.vcard_fetched_ok = 0 self.fetch_complete = False self._test_auth_and_discover() @@ -138,9 +136,8 @@ def _find_addressbooks(self, xml_response: str) -> List[str]: def get_contacts(self) -> List[Dict]: """Fetch all contacts from all discovered addressbooks""" all_contacts = [] - self.vcard_listed = 0 - self.vcard_fetched_ok = 0 - self.fetch_complete = False + # Cleared by any listing, download or parse failure. Orphan delete relies on it. + self.fetch_complete = True for addressbook_url in self.addressbook_urls: logger.info(f"Processing addressbook: {addressbook_url}") @@ -148,18 +145,12 @@ def get_contacts(self) -> List[Dict]: all_contacts.extend(contacts) logger.info(f"Found {len(contacts)} contacts with birthdays in this addressbook") - self.fetch_complete = ( - self.vcard_listed > 0 and self.vcard_fetched_ok == self.vcard_listed - ) - logger.info( - f"CardDAV fetch {self.vcard_fetched_ok}/{self.vcard_listed} vCards " - f"(complete={self.fetch_complete})" - ) + logger.info(f"CardDAV fetch complete: {self.fetch_complete}") logger.info(f"Total contacts with birthdays across all addressbooks: {len(all_contacts)}") return all_contacts def _http_get_retry(self, url: str, attempts: int = 3): - """GET with retries for transient connection errors (e.g. IPv6 unreachable).""" + """GET with retries for transient connection errors.""" last_error = None for i in range(1, attempts + 1): try: @@ -191,7 +182,8 @@ def _get_contacts_from_addressbook(self, addressbook_url: str) -> List[Dict]: logger.debug(f"Discovering resources in addressbook: {addressbook_url}") response = requests.request('PROPFIND', addressbook_url, - auth=self.auth, headers=headers, data=propfind_body) + auth=self.auth, headers=headers, data=propfind_body, + timeout=30) logger.debug(f"PROPFIND response status: {response.status_code}") @@ -205,8 +197,6 @@ def _get_contacts_from_addressbook(self, addressbook_url: str) -> List[Dict]: if not vcard_urls: logger.debug("No vCard URLs found in this addressbook") return contacts - - self.vcard_listed += len(vcard_urls) # Fetch each vCard for i, vcard_url in enumerate(vcard_urls): @@ -218,7 +208,6 @@ def _get_contacts_from_addressbook(self, addressbook_url: str) -> List[Dict]: logger.debug(f"vCard response status: {vcard_response.status_code}") if vcard_response.status_code == 200: - self.vcard_fetched_ok += 1 logger.debug(f"vCard content preview: {vcard_response.text[:200]}...") contact = self._parse_vcard(vcard_response.text) if contact: @@ -229,15 +218,19 @@ def _get_contacts_from_addressbook(self, addressbook_url: str) -> List[Dict]: logger.debug(f"No birthday found in vCard: {vcard_url}") else: logger.warning(f"Failed to fetch vCard {vcard_url}: {vcard_response.status_code}") + self.fetch_complete = False except Exception as e: logger.warning(f"Error processing vCard {vcard_url}: {e}") + self.fetch_complete = False continue else: logger.error(f"Failed to discover resources in {addressbook_url}: {response.status_code}") logger.error(f"Response: {response.text[:500]}") + self.fetch_complete = False except Exception as e: logger.error(f"Error fetching contacts from {addressbook_url}: {e}") + self.fetch_complete = False if logger.getEffectiveLevel() <= logging.DEBUG: import traceback logger.debug(traceback.format_exc()) @@ -247,12 +240,7 @@ def _get_contacts_from_addressbook(self, addressbook_url: str) -> List[Dict]: def _extract_vcard_urls(self, xml_response: str) -> List[str]: """Extract vCard URLs from PROPFIND response""" dav_namespace = 'DAV:' - - try: - root = ElementTree.fromstring(xml_response) - except ElementTree.ParseError as error: - logger.warning(f"Could not parse vCard discovery XML: {error}") - return [] + root = ElementTree.fromstring(xml_response) urls = [] for response in root.findall(f'{{{dav_namespace}}}response'): @@ -286,13 +274,12 @@ def _resolve_url(self, url: str) -> str: return f"{self.server_url.rstrip('/')}/{url.lstrip('/')}" def _parse_vcard(self, vcard_text: str) -> Optional[Dict]: - """Parse individual vCard""" + """Parse individual vCard. Returns None without BDAY, raises on unreadable data.""" try: # Clean up the vCard text vcard_text = vcard_text.strip() if not vcard_text.startswith('BEGIN:VCARD'): - logger.debug("Invalid vCard: doesn't start with BEGIN:VCARD") - return None + raise ValueError("Invalid vCard: doesn't start with BEGIN:VCARD") vcard = vobject.readOne(vcard_text) contact = {} @@ -337,12 +324,10 @@ def _parse_vcard(self, vcard_text: str) -> Optional[Dict]: month_day = bday_clean[2:] # Remove -- contact['birthday'] = datetime.strptime(f"2000-{month_day}", '%Y-%m-%d').date() else: - logger.warning(f"Unknown birthday format for {contact['name']}: {bday}") - return None + raise ValueError("unknown format") except ValueError as e: - logger.warning(f"Could not parse birthday for {contact['name']}: {bday} - {e}") - return None + raise ValueError(f"Could not parse birthday for {contact['name']}: {bday} - {e}") from e elif hasattr(bday, 'date'): contact['birthday'] = bday.date() @@ -364,7 +349,6 @@ def _parse_vcard(self, vcard_text: str) -> Optional[Dict]: logger.debug(f"No birthday found for contact: {contact['name']}") return None - except Exception as e: - logger.warning(f"Error parsing vCard: {e}") + except Exception: logger.debug(f"vCard content: {vcard_text[:500]}...") - return None + raise diff --git a/bdaysync/main.py b/bdaysync/main.py index 2c1d838..0353af5 100644 --- a/bdaysync/main.py +++ b/bdaysync/main.py @@ -98,27 +98,30 @@ def main_sync(): if not contacts: logger.warning("No contacts with birthdays found") - else: - logger.info(f"Found {len(contacts)} contacts with birthdays") - created_count = 0 - current_year = datetime.now().year - for contact in contacts: - logger.info(f"Processing birthday for: {contact['name']} ({contact['birthday']})") - if caldav_client.create_birthday_event(contact, current_year): - created_count += 1 - if caldav_client.create_birthday_event(contact, current_year + 1): - created_count += 1 - logger.info(f"Successfully created {created_count} birthday events") + return False + + logger.info(f"Found {len(contacts)} contacts with birthdays") + + # Create birthday events + created_count = 0 + current_year = datetime.now().year + + for contact in contacts: + logger.info(f"Processing birthday for: {contact['name']} ({contact['birthday']})") + if caldav_client.create_birthday_event(contact, current_year): + created_count += 1 + + # Also create for next year + if caldav_client.create_birthday_event(contact, current_year + 1): + created_count += 1 + + logger.info(f"Successfully created {created_count} birthday events") if cardav_client.fetch_complete: deleted = caldav_client.delete_orphans(contacts) logger.info(f"Deleted {deleted} orphan birthday events") else: - logger.warning( - f"Incomplete CardDAV fetch " - f"({cardav_client.vcard_fetched_ok}/{cardav_client.vcard_listed}); " - f"skipping orphan delete" - ) + logger.warning("Incomplete CardDAV fetch; skipping orphan delete") return True diff --git a/bdaysync/test_sync.py b/bdaysync/test_sync.py new file mode 100644 index 0000000..0e346d5 --- /dev/null +++ b/bdaysync/test_sync.py @@ -0,0 +1,140 @@ +""" +Safety tests for orphan deletion. Run from bdaysync/: python -m unittest test_sync +""" + +import logging +import unittest +from datetime import date +from unittest import mock + +import requests + +import caldav_client +import cardav_client +import main + +logging.disable(logging.CRITICAL) + +LISTING = ''' + + {ab} + {ab}a.vcf +''' + +VCARD = "BEGIN:VCARD\r\nVERSION:3.0\r\nFN:Anna\r\n{bday}END:VCARD\r\n" + + +def response(status, text): + return mock.Mock(status_code=status, text=text) + + +def fetch(listings, vcard=VCARD.format(bday="BDAY:1990-01-02\r\n"), vcard_status=200): + """Run get_contacts against fake addressbooks; a listing is a response or an exception.""" + client = cardav_client.CardDAVClient.__new__(cardav_client.CardDAVClient) + client.server_url = 'https://dav.example' + client.auth = None + client.addressbook_urls = list(listings) + + def propfind(method, url, **kwargs): + listing = listings[url] + if isinstance(listing, Exception): + raise listing + return listing + + with mock.patch.object(cardav_client.requests, 'request', side_effect=propfind), \ + mock.patch.object(cardav_client.requests, 'get', return_value=response(vcard_status, vcard)): + contacts = client.get_contacts() + return contacts, client.fetch_complete + + +class CardDAVFetchComplete(unittest.TestCase): + OK = {'https://dav.example/ab1/': response(207, LISTING.format(ab='/ab1/'))} + + def test_all_vcards_fetched_is_complete(self): + contacts, complete = fetch(self.OK) + self.assertEqual(len(contacts), 1) + self.assertTrue(complete) + + def test_contact_without_birthday_keeps_fetch_complete(self): + _, complete = fetch(self.OK, vcard=VCARD.format(bday="")) + self.assertTrue(complete) + + def test_failed_addressbook_listing_is_incomplete(self): + failures = { + 'exception': requests.exceptions.ConnectionError('down'), + 'http error': response(503, 'unavailable'), + 'broken xml': response(207, ' Date: Tue, 15 Sep 2026 06:54:47 +0000 Subject: [PATCH 2/3] Scan the image tag that was actually pushed and pin Trivy Tag pushes publish 1.2.3 via type=semver, but Trivy looked for v1.2.3. Take the first tag from docker/metadata-action, which is already lowercased and sanitized. Pin trivy-action to the v0.36.0 commit instead of master, run the new unit tests in the PR job and bump setup-python to v5. Co-Authored-By: Claude Opus 5 --- .github/workflows/docker.yml | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/.github/workflows/docker.yml b/.github/workflows/docker.yml index d45e1db..1a9e594 100644 --- a/.github/workflows/docker.yml +++ b/.github/workflows/docker.yml @@ -23,6 +23,9 @@ env: jobs: build-and-push: runs-on: ubuntu-latest + outputs: + # First pushed tag, already lowercased and sanitized by metadata-action + image: ${{ fromJSON(steps.meta.outputs.json).tags[0] }} permissions: contents: read packages: write @@ -92,7 +95,7 @@ jobs: uses: actions/checkout@v4 - name: Set up Python - uses: actions/setup-python@v4 + uses: actions/setup-python@v5 with: python-version: "3.11" @@ -110,6 +113,9 @@ jobs: # Test help output python main.py --help + # Orphan-delete safety tests + python -m unittest test_sync + # Test config validation (should fail without env vars) python -c "from config import validate_environment; exit(0 if not validate_environment() else 1)" @@ -123,16 +129,10 @@ jobs: security-events: write steps: - - name: Resolve image reference - run: | - repo="${GITHUB_REPOSITORY,,}" - tag="${GITHUB_REF_NAME//\//-}" - echo "TRIVY_IMAGE=${REGISTRY}/${repo}:${tag}" >> "$GITHUB_ENV" - - name: Run Trivy vulnerability scanner - uses: aquasecurity/trivy-action@master + uses: aquasecurity/trivy-action@ed142fd0673e97e23eac54620cfb913e5ce36c25 # v0.36.0 with: - image-ref: ${{ env.TRIVY_IMAGE }} + image-ref: ${{ needs.build-and-push.outputs.image }} format: "sarif" output: "trivy-results.sarif" From b4c6d0d68c543f0f6a376e5bf1b4e6d50a2e1b85 Mon Sep 17 00:00:00 2001 From: HubEight Date: Tue, 15 Sep 2026 07:00:15 +0000 Subject: [PATCH 3/3] Add BIRTHDAY_DELETE_ORPHANS switch, off by default Orphan delete removes calendar data, so it is opt-in. Wired like the other BIRTHDAY_* settings: config, .env.template, docker-compose and README, and logged with the event configuration. Co-Authored-By: Claude Opus 5 --- .env.template | 3 +++ README.md | 1 + bdaysync/caldav_client.py | 2 ++ bdaysync/config.py | 3 ++- bdaysync/main.py | 11 ++++++----- bdaysync/test_sync.py | 17 +++++++++++++++++ docker-compose.yaml | 1 + 7 files changed, 32 insertions(+), 6 deletions(-) diff --git a/.env.template b/.env.template index 2423abd..0d34aae 100644 --- a/.env.template +++ b/.env.template @@ -36,6 +36,9 @@ BIRTHDAY_EVENT_CATEGORY=Birthday # Whether to update existing events when templates change BIRTHDAY_UPDATE_EXISTING=true +# Delete birthday events whose contact or birthday is gone (only after a complete fetch) +BIRTHDAY_DELETE_ORPHANS=false + # ============================================================================ # OPTIONAL: Scheduling Configuration # ============================================================================ diff --git a/README.md b/README.md index 1dc5cf6..0d11e15 100644 --- a/README.md +++ b/README.md @@ -73,6 +73,7 @@ docker-compose up -d | `BIRTHDAY_REMINDER_MESSAGE` | `Reminder: {name}'s birthday is in {days} days!` | Reminder message template | | `BIRTHDAY_EVENT_CATEGORY` | `Birthday` | Event category | | `BIRTHDAY_UPDATE_EXISTING` | `true` | Update existing events | +| `BIRTHDAY_DELETE_ORPHANS` | `false` | Delete orphaned events | ### Logging & Debug diff --git a/bdaysync/caldav_client.py b/bdaysync/caldav_client.py index bff3cd7..3d60e26 100644 --- a/bdaysync/caldav_client.py +++ b/bdaysync/caldav_client.py @@ -63,6 +63,7 @@ def _load_config(self): self.reminder_template = config['reminder_template'] self.event_category = config['event_category'] self.update_existing = config['update_existing'] + self.delete_orphans_enabled = config['delete_orphans'] logger.info("Birthday event configuration:") logger.info(f" Title template: {self.event_title_template}") @@ -71,6 +72,7 @@ def _load_config(self): logger.info(f" Reminder message: {self.reminder_template}") logger.info(f" Category: {self.event_category}") logger.info(f" Update existing: {self.update_existing}") + logger.info(f" Delete orphans: {self.delete_orphans_enabled}") def create_birthday_event(self, contact: Dict, year: int = None) -> bool: """Create a birthday event for a contact""" diff --git a/bdaysync/config.py b/bdaysync/config.py index 6a2385f..fc344ce 100644 --- a/bdaysync/config.py +++ b/bdaysync/config.py @@ -81,7 +81,8 @@ def get_birthday_config(): 'reminder_days_str': os.getenv('BIRTHDAY_REMINDER_DAYS', '1'), 'reminder_template': os.getenv('BIRTHDAY_REMINDER_MESSAGE', 'Reminder: {name}\'s birthday is in {days} days!'), 'event_category': os.getenv('BIRTHDAY_EVENT_CATEGORY', 'Birthday'), - 'update_existing': os.getenv('BIRTHDAY_UPDATE_EXISTING', 'true').lower() == 'true' + 'update_existing': os.getenv('BIRTHDAY_UPDATE_EXISTING', 'true').lower() == 'true', + 'delete_orphans': os.getenv('BIRTHDAY_DELETE_ORPHANS', 'false').lower() == 'true' } def get_scheduler_config(): diff --git a/bdaysync/main.py b/bdaysync/main.py index 0353af5..e7c4239 100644 --- a/bdaysync/main.py +++ b/bdaysync/main.py @@ -117,11 +117,12 @@ def main_sync(): logger.info(f"Successfully created {created_count} birthday events") - if cardav_client.fetch_complete: - deleted = caldav_client.delete_orphans(contacts) - logger.info(f"Deleted {deleted} orphan birthday events") - else: - logger.warning("Incomplete CardDAV fetch; skipping orphan delete") + if caldav_client.delete_orphans_enabled: + if cardav_client.fetch_complete: + deleted = caldav_client.delete_orphans(contacts) + logger.info(f"Deleted {deleted} orphan birthday events") + else: + logger.warning("Incomplete CardDAV fetch; skipping orphan delete") return True diff --git a/bdaysync/test_sync.py b/bdaysync/test_sync.py index 0e346d5..09d42ae 100644 --- a/bdaysync/test_sync.py +++ b/bdaysync/test_sync.py @@ -11,6 +11,7 @@ import caldav_client import cardav_client +import config import main logging.disable(logging.CRITICAL) @@ -127,6 +128,22 @@ def test_deletes_removed_contacts_and_changed_dates_only(self): class MainSync(unittest.TestCase): + def test_orphan_delete_is_off_by_default(self): + with mock.patch.dict('os.environ', clear=True): + self.assertFalse(config.get_birthday_config()['delete_orphans']) + + def test_orphan_delete_follows_switch(self): + for enabled in (False, True): + with self.subTest(enabled=enabled), \ + mock.patch.object(main, 'CardDAVClient') as carddav, \ + mock.patch.object(main, 'CalDAVClient') as caldav: + carddav.return_value.get_contacts.return_value = [{'name': 'Anna', 'birthday': date(1990, 1, 2)}] + carddav.return_value.fetch_complete = True + caldav.return_value.delete_orphans_enabled = enabled + caldav.return_value.delete_orphans.return_value = 0 + self.assertTrue(main.main_sync()) + self.assertEqual(caldav.return_value.delete_orphans.called, enabled) + def test_no_contacts_fails_and_deletes_nothing(self): with mock.patch.object(main, 'CardDAVClient') as carddav, \ mock.patch.object(main, 'CalDAVClient') as caldav: diff --git a/docker-compose.yaml b/docker-compose.yaml index d4ddc2c..0986daf 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -25,6 +25,7 @@ services: BIRTHDAY_REMINDER_MESSAGE: "${BIRTHDAY_REMINDER_MESSAGE:-Reminder: {name}'s birthday is in {days} days!}" BIRTHDAY_EVENT_CATEGORY: "${BIRTHDAY_EVENT_CATEGORY:-Birthday}" BIRTHDAY_UPDATE_EXISTING: "${BIRTHDAY_UPDATE_EXISTING:-true}" + BIRTHDAY_DELETE_ORPHANS: "${BIRTHDAY_DELETE_ORPHANS:-false}" # Scheduling Configuration RUN_MODE: "${RUN_MODE:-daemon}" # Options: daemon, once