Fix Phase 5 review issues: SSTI, FK violation, sync rebuild
Some checks failed
Validate / Hassfest (push) Has been cancelled
Some checks failed
Validate / Hassfest (push) Has been cancelled
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) <noreply@anthropic.com>
This commit is contained in:
@@ -406,10 +406,20 @@ async def _async_update_listener(
|
|||||||
# Update hub data
|
# Update hub data
|
||||||
entry.runtime_data.scan_interval = new_interval
|
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
|
# Update all subentry coordinators
|
||||||
subentries_data = entry_data["subentries"]
|
subentries_data = entry_data["subentries"]
|
||||||
for subentry_data in subentries_data.values():
|
for subentry_data in subentries_data.values():
|
||||||
subentry_data.coordinator.update_scan_interval(new_interval)
|
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)
|
_LOGGER.info("Updated hub options (scan_interval=%d)", new_interval)
|
||||||
|
|
||||||
|
|||||||
@@ -252,7 +252,9 @@ class ImmichAlbumWatcherOptionsFlow(OptionsFlow):
|
|||||||
# Validate server connection if URL is provided
|
# Validate server connection if URL is provided
|
||||||
server_url = user_input.get(CONF_SERVER_URL, "").strip()
|
server_url = user_input.get(CONF_SERVER_URL, "").strip()
|
||||||
server_api_key = user_input.get(CONF_SERVER_API_KEY, "").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:
|
try:
|
||||||
session = async_get_clientsession(self.hass)
|
session = async_get_clientsession(self.hass)
|
||||||
async with session.get(
|
async with session.get(
|
||||||
|
|||||||
@@ -538,5 +538,6 @@ class ImmichAlbumWatcherCoordinator(DataUpdateCoordinator[AlbumData | None]):
|
|||||||
"added_count": change.added_count,
|
"added_count": change.added_count,
|
||||||
"removed_count": change.removed_count,
|
"removed_count": change.removed_count,
|
||||||
},
|
},
|
||||||
)
|
),
|
||||||
|
name=f"immich_sync_report_{change.album_id}",
|
||||||
)
|
)
|
||||||
|
|||||||
@@ -81,7 +81,8 @@
|
|||||||
"invalid_auth": "Invalid API key",
|
"invalid_auth": "Invalid API key",
|
||||||
"no_albums": "No albums found on the server",
|
"no_albums": "No albums found on the server",
|
||||||
"unknown": "Unexpected error occurred",
|
"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": {
|
"abort": {
|
||||||
"already_configured": "This Immich server is already configured"
|
"already_configured": "This Immich server is already configured"
|
||||||
|
|||||||
@@ -136,7 +136,7 @@ async def render_template(
|
|||||||
raise HTTPException(status_code=404, detail="Template not found")
|
raise HTTPException(status_code=404, detail="Template not found")
|
||||||
|
|
||||||
try:
|
try:
|
||||||
env = jinja2.Environment(autoescape=False)
|
env = jinja2.sandbox.SandboxedEnvironment(autoescape=False)
|
||||||
tmpl = env.from_string(template.body)
|
tmpl = env.from_string(template.body)
|
||||||
rendered = tmpl.render(**body.context)
|
rendered = tmpl.render(**body.context)
|
||||||
return {"rendered": rendered}
|
return {"rendered": rendered}
|
||||||
@@ -161,7 +161,7 @@ async def report_event(
|
|||||||
tracker = result.first()
|
tracker = result.first()
|
||||||
|
|
||||||
event = EventLog(
|
event = EventLog(
|
||||||
tracker_id=tracker.id if tracker else 0,
|
tracker_id=tracker.id if tracker else None,
|
||||||
event_type=body.event_type,
|
event_type=body.event_type,
|
||||||
album_id=body.album_id,
|
album_id=body.album_id,
|
||||||
album_name=body.album_name,
|
album_name=body.album_name,
|
||||||
|
|||||||
@@ -124,7 +124,7 @@ async def preview_template(
|
|||||||
"""Render a template with sample data."""
|
"""Render a template with sample data."""
|
||||||
template = await _get_user_template(session, template_id, user.id)
|
template = await _get_user_template(session, template_id, user.id)
|
||||||
try:
|
try:
|
||||||
env = jinja2.Environment(autoescape=False)
|
env = jinja2.sandbox.SandboxedEnvironment(autoescape=False)
|
||||||
tmpl = env.from_string(template.body)
|
tmpl = env.from_string(template.body)
|
||||||
rendered = tmpl.render(**_SAMPLE_CONTEXT)
|
rendered = tmpl.render(**_SAMPLE_CONTEXT)
|
||||||
return {"rendered": rendered}
|
return {"rendered": rendered}
|
||||||
|
|||||||
@@ -104,7 +104,7 @@ class EventLog(SQLModel, table=True):
|
|||||||
__tablename__ = "event_log"
|
__tablename__ = "event_log"
|
||||||
|
|
||||||
id: int | None = Field(default=None, primary_key=True)
|
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
|
event_type: str
|
||||||
album_id: str
|
album_id: str
|
||||||
album_name: str
|
album_name: str
|
||||||
|
|||||||
@@ -24,7 +24,7 @@ DEFAULT_TEMPLATE = (
|
|||||||
|
|
||||||
def render_template(template_body: str, context: dict[str, Any]) -> str:
|
def render_template(template_body: str, context: dict[str, Any]) -> str:
|
||||||
"""Render a Jinja2 template with the given context."""
|
"""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)
|
tmpl = env.from_string(template_body)
|
||||||
return tmpl.render(**context)
|
return tmpl.render(**context)
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user