From 43f83acda9a7ab885662a643296e83468123eeb1 Mon Sep 17 00:00:00 2001 From: "alexei.dolgolyov" Date: Thu, 19 Mar 2026 14:17:59 +0300 Subject: [PATCH] Fix Phase 5 review issues: SSTI, FK violation, sync rebuild Fixes 5 issues identified by code-reviewer agent: 1. (Critical) EventLog.tracker_id now nullable - use None instead of 0 when tracker name doesn't match, avoiding FK constraint violations on PostgreSQL 2. (Critical) Replace jinja2.Environment with SandboxedEnvironment in all 3 server template rendering locations to prevent SSTI 3. (Important) Rebuild sync_client in _async_update_listener when server URL/key options change, propagate to all coordinators 4. (Important) Validate partial server config - require both URL and API key or neither, with clear error message 5. (Important) Name fire-and-forget sync task for debugging Co-Authored-By: Claude Opus 4.6 (1M context) --- custom_components/immich_album_watcher/__init__.py | 10 ++++++++++ custom_components/immich_album_watcher/config_flow.py | 4 +++- custom_components/immich_album_watcher/coordinator.py | 3 ++- .../immich_album_watcher/translations/en.json | 3 ++- packages/server/src/immich_watcher_server/api/sync.py | 4 ++-- .../server/src/immich_watcher_server/api/templates.py | 2 +- .../src/immich_watcher_server/database/models.py | 2 +- .../src/immich_watcher_server/services/notifier.py | 2 +- 8 files changed, 22 insertions(+), 8 deletions(-) diff --git a/custom_components/immich_album_watcher/__init__.py b/custom_components/immich_album_watcher/__init__.py index cf9d64d..53f8e25 100644 --- a/custom_components/immich_album_watcher/__init__.py +++ b/custom_components/immich_album_watcher/__init__.py @@ -406,10 +406,20 @@ async def _async_update_listener( # Update hub data entry.runtime_data.scan_interval = new_interval + # Rebuild sync client if server URL/key changed + server_url = entry.options.get(CONF_SERVER_URL, "") + server_api_key = entry.options.get(CONF_SERVER_API_KEY, "") + sync_client = None + if server_url and server_api_key: + from .sync import ServerSyncClient + sync_client = ServerSyncClient(hass, server_url, server_api_key) + entry_data["sync_client"] = sync_client + # Update all subentry coordinators subentries_data = entry_data["subentries"] for subentry_data in subentries_data.values(): subentry_data.coordinator.update_scan_interval(new_interval) + subentry_data.coordinator._sync_client = sync_client _LOGGER.info("Updated hub options (scan_interval=%d)", new_interval) diff --git a/custom_components/immich_album_watcher/config_flow.py b/custom_components/immich_album_watcher/config_flow.py index 63e168c..e7b4d7f 100644 --- a/custom_components/immich_album_watcher/config_flow.py +++ b/custom_components/immich_album_watcher/config_flow.py @@ -252,7 +252,9 @@ class ImmichAlbumWatcherOptionsFlow(OptionsFlow): # Validate server connection if URL is provided server_url = user_input.get(CONF_SERVER_URL, "").strip() server_api_key = user_input.get(CONF_SERVER_API_KEY, "").strip() - if server_url and server_api_key: + if bool(server_url) != bool(server_api_key): + errors["base"] = "server_partial_config" + elif server_url and server_api_key: try: session = async_get_clientsession(self.hass) async with session.get( diff --git a/custom_components/immich_album_watcher/coordinator.py b/custom_components/immich_album_watcher/coordinator.py index 015005b..3ff8a67 100644 --- a/custom_components/immich_album_watcher/coordinator.py +++ b/custom_components/immich_album_watcher/coordinator.py @@ -538,5 +538,6 @@ class ImmichAlbumWatcherCoordinator(DataUpdateCoordinator[AlbumData | None]): "added_count": change.added_count, "removed_count": change.removed_count, }, - ) + ), + name=f"immich_sync_report_{change.album_id}", ) diff --git a/custom_components/immich_album_watcher/translations/en.json b/custom_components/immich_album_watcher/translations/en.json index 1624377..9f677a7 100644 --- a/custom_components/immich_album_watcher/translations/en.json +++ b/custom_components/immich_album_watcher/translations/en.json @@ -81,7 +81,8 @@ "invalid_auth": "Invalid API key", "no_albums": "No albums found on the server", "unknown": "Unexpected error occurred", - "server_connect_failed": "Failed to connect to Immich Watcher server" + "server_connect_failed": "Failed to connect to Immich Watcher server", + "server_partial_config": "Both server URL and API key are required (or leave both empty to disable sync)" }, "abort": { "already_configured": "This Immich server is already configured" diff --git a/packages/server/src/immich_watcher_server/api/sync.py b/packages/server/src/immich_watcher_server/api/sync.py index 16a9025..6395a53 100644 --- a/packages/server/src/immich_watcher_server/api/sync.py +++ b/packages/server/src/immich_watcher_server/api/sync.py @@ -136,7 +136,7 @@ async def render_template( raise HTTPException(status_code=404, detail="Template not found") try: - env = jinja2.Environment(autoescape=False) + env = jinja2.sandbox.SandboxedEnvironment(autoescape=False) tmpl = env.from_string(template.body) rendered = tmpl.render(**body.context) return {"rendered": rendered} @@ -161,7 +161,7 @@ async def report_event( tracker = result.first() event = EventLog( - tracker_id=tracker.id if tracker else 0, + tracker_id=tracker.id if tracker else None, event_type=body.event_type, album_id=body.album_id, album_name=body.album_name, diff --git a/packages/server/src/immich_watcher_server/api/templates.py b/packages/server/src/immich_watcher_server/api/templates.py index 182ba6e..e58ac79 100644 --- a/packages/server/src/immich_watcher_server/api/templates.py +++ b/packages/server/src/immich_watcher_server/api/templates.py @@ -124,7 +124,7 @@ async def preview_template( """Render a template with sample data.""" template = await _get_user_template(session, template_id, user.id) try: - env = jinja2.Environment(autoescape=False) + env = jinja2.sandbox.SandboxedEnvironment(autoescape=False) tmpl = env.from_string(template.body) rendered = tmpl.render(**_SAMPLE_CONTEXT) return {"rendered": rendered} diff --git a/packages/server/src/immich_watcher_server/database/models.py b/packages/server/src/immich_watcher_server/database/models.py index 5f4ebe6..c072f69 100644 --- a/packages/server/src/immich_watcher_server/database/models.py +++ b/packages/server/src/immich_watcher_server/database/models.py @@ -104,7 +104,7 @@ class EventLog(SQLModel, table=True): __tablename__ = "event_log" id: int | None = Field(default=None, primary_key=True) - tracker_id: int = Field(foreign_key="album_tracker.id") + tracker_id: int | None = Field(default=None, foreign_key="album_tracker.id") event_type: str album_id: str album_name: str diff --git a/packages/server/src/immich_watcher_server/services/notifier.py b/packages/server/src/immich_watcher_server/services/notifier.py index 6a04255..2d43d54 100644 --- a/packages/server/src/immich_watcher_server/services/notifier.py +++ b/packages/server/src/immich_watcher_server/services/notifier.py @@ -24,7 +24,7 @@ DEFAULT_TEMPLATE = ( def render_template(template_body: str, context: dict[str, Any]) -> str: """Render a Jinja2 template with the given context.""" - env = jinja2.Environment(autoescape=False) + env = jinja2.sandbox.SandboxedEnvironment(autoescape=False) tmpl = env.from_string(template_body) return tmpl.render(**context)