Make orphan delete safe and opt-in

.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
 # ============================================================================

.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"
 

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
 

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"""
 
@@ -58,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}")
@@ -66,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"""
@@ -93,7 +100,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 +175,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 +188,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 +231,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 +256,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:

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,28 +136,21 @@ 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}")
             contacts = self._get_contacts_from_addressbook(addressbook_url)
             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

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():

bdaysync/main.py

@@ -98,27 +98,31 @@ 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"
-            )
+        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