From 50738b139a877c9ef78f5731cf2d8d89f03f5d1e Mon Sep 17 00:00:00 2001
From: Athena Kaminsky <thegiantsnail@gmail.com>
Date: Tue, 25 Aug 2026 19:26:08 -0500
Subject: [PATCH] fix(security): close icons path-prefix bypass, log swallowed
 exceptions

/icons/image used a bare string startswith() against allowed roots, so
a sibling dir like /usr/share/icons_evil would pass as if it were under
/usr/share/icons. Switched to the pathlib parents-based check already
used correctly in icons/compositor.py, plus a regression test.

Also stopped three bare `except Exception: pass` blocks (auto model
select fallback, conversation titling) from swallowing errors silently
- they now log to the existing chat trace helper. Behavior unchanged,
just visible when something's actually failing.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---
 synapse/main.py     | 20 +++++++++++++-------
 tests/test_smoke.py | 30 ++++++++++++++++++++++++++++++
 2 files changed, 43 insertions(+), 7 deletions(-)

diff --git a/synapse/main.py b/synapse/main.py
index cf32dd3..5880d9b 100644
--- a/synapse/main.py
+++ b/synapse/main.py
@@ -146,7 +146,8 @@ async def _auto_select_model(message: str = "") -> str:
         if remap:
             return remap
         return await get_ollama_manager().select_best_model(intent)
-    except Exception:
+    except Exception as e:
+        _synapse_trace(f"⚠ auto model selection failed, falling back to default: {e}\n")
         return DEFAULT_CHAT_MODEL
 
 
@@ -198,7 +199,8 @@ async def _generate_conversation_title(first_message: str, model: str) -> Option
         if not title:
             return None
         return title[:120]
-    except Exception:
+    except Exception as e:
+        _synapse_trace(f"⚠ title generation failed: {e}\n")
         return None
 
 
@@ -600,8 +602,8 @@ async def chat_stream_endpoint(payload: Dict[str, Any]):
                         if title:
                             store.set_conversation_title(conversation_id, title)
                             yield f"event: title\ndata: {_json.dumps({'title': title})}\n\n"
-                except Exception:
-                    pass
+                except Exception as e:
+                    _synapse_trace(f"⚠ conversation titling step failed: {e}\n")
 
             # Ask the memory service curator to evaluate this exchange
             if response_chunks:
@@ -1492,10 +1494,14 @@ async def list_icon_apps():
 @app.get("/icons/image")
 async def get_icon_image(path: str):
     """Serve an icon file after verifying it's in an allowed root."""
-    real = _os.path.realpath(path)
-    if not any(real.startswith(r) for r in _ALLOWED_ICON_ROOTS):
+    real = Path(_os.path.realpath(path))
+    allowed = any(
+        real == root or root in real.parents
+        for root in (Path(r).resolve() for r in _ALLOWED_ICON_ROOTS)
+    )
+    if not allowed:
         raise HTTPException(status_code=403, detail="Path not allowed")
-    if not _os.path.isfile(real):
+    if not real.is_file():
         raise HTTPException(status_code=404, detail="Icon not found")
     return FileResponse(real)
 
diff --git a/tests/test_smoke.py b/tests/test_smoke.py
index 751bcc6..d04f493 100644
--- a/tests/test_smoke.py
+++ b/tests/test_smoke.py
@@ -216,6 +216,36 @@ def test_icon_source_requires_real_allowed_file_boundary(tmp_path):
         module._ALLOWED_ROOTS[:] = old_roots
 
 
+def test_icons_image_endpoint_requires_real_allowed_root_boundary(tmp_path):
+    # Sibling directories that merely share a string prefix with an allowed
+    # root (e.g. "icons-other" vs "icons") must not pass the check.
+    from synapse import main
+
+    allowed = tmp_path / "icons"
+    allowed.mkdir()
+    source = allowed / "app.svg"
+    source.write_text("<svg />")
+    sibling = tmp_path / "icons-other"
+    sibling.mkdir()
+    evil = sibling / "app.svg"
+    evil.write_text("<svg />")
+
+    old_roots = list(main._ALLOWED_ICON_ROOTS)
+    main._ALLOWED_ICON_ROOTS[:] = [str(allowed)]
+    try:
+        client = TestClient(app)
+        ok = client.get("/icons/image", params={"path": str(source)})
+        assert ok.status_code == 200
+
+        blocked = client.get("/icons/image", params={"path": str(evil)})
+        assert blocked.status_code == 403
+
+        missing = client.get("/icons/image", params={"path": str(allowed / "missing.svg")})
+        assert missing.status_code in (403, 404)
+    finally:
+        main._ALLOWED_ICON_ROOTS[:] = old_roots
+
+
 def test_ollama_stream_propagates_transport_errors(monkeypatch):
     """A failing stream must surface, not be swallowed into an empty reply —
     and it must carry Ollama's own explanation, since that is the only part the