fix: comprehensive security, bug, performance, and UI/UX audit
Lint & Test / test (push) Successful in 20s
Lint & Test / test (push) Successful in 20s
Security
- Default bind 127.0.0.1; first-run bootstrap generates random api_token
and refuses to bind non-loopback without auth unless explicitly opted in
- Path-traversal hardened: BrowserService.validate_path rejects absolute
paths, drive letters, UNC, NUL bytes. /api/browser/{play,metadata,
thumbnail} now require folder_id and a folder-relative path
- Pydantic validators on links: http(s) URLs only, mdi:<slug> icons only
- Scripts/callbacks/links create/update/delete gated by *_management flags
- Strict CSP, X-Frame-Options DENY, Referrer-Policy no-referrer,
X-Content-Type-Options nosniff
- CORS locked to localhost:<port> + 127.0.0.1:<port> by default; configurable
- config.yaml writes atomic (tmp + os.replace) and 0o600 on POSIX
- Subprocesses spawned in their own process group / new session so timeout
kills the whole tree (Windows CREATE_NEW_PROCESS_GROUP, POSIX
start_new_session=True)
- Frontend XSS: monitor name + details escapeHtml'd; power button moved to
delegated data-action handler; remote MDI SVGs parsed and sanitized
(strip script/foreignObject/on*/javascript: hrefs) before innerHTML
- All dynamic URL segments now wrapped in encodeURIComponent
Bugs
- WebSocket reconnect: close previous socket before opening new, clear
ping interval per-socket, clear reconnectTimeout up-front, retry on
online/visibilitychange, try/catch JSON.parse
- Artwork fetch race: AbortController + generation guard
- _broadcast_after_open: initialize status, swallow per-poll errors,
background tasks tracked in a strong-ref set with done-callback cleanup
- Audio analyzer: sticky _unavailable flag prevents infinite start/stop
spin when no loopback device exists; cleared by set_device()
- Volume short-circuit cache invalidated when server reports remote volume
- Browser thumbnail race: per-folder generation counter + isConnected
checks; aborts in-flight fetches on navigation
- Track-skip uses cached title instead of full WinRT status round-trip
Performance
- Linux MPRIS/pactl and /api/display DDC-CI handlers wrapped in
asyncio.to_thread so blocking IO never stalls the event loop
- browse_directory moved off the event loop (SMB shares could freeze it)
- Windows status poll caches one asyncio loop per worker thread via
threading.local instead of new_event_loop/close on every 0.5s tick
- broadcast() serializes JSON once and uses send_text to all clients
- Hourly thumbnail cache cleanup scheduled in lifespan (was never invoked
— cache grew unbounded)
- Progress drag listeners attached only while dragging
Quality
- All asyncio.get_event_loop() in coroutines → get_running_loop()
- ThreadPoolExecutors shut down cleanly during lifespan teardown
- config_manager dedup: 12 near-identical methods collapsed onto generic
_upsert/_delete helpers (~290 lines removed)
- Service worker no longer pass-throughs every fetch
- M3U playlist written via NamedTemporaryFile (no fixed-path symlink
clobber race)
- __version__ now prefers live pyproject.toml in dev checkouts so
pip install -e . users see the source-of-truth version, not the stale
package-metadata version baked in at install time
UI/UX (Studio Reference)
- Green leftover focus rings (rgba(29,185,84,...)) all replaced with
copper accent (rgba(var(--copper-rgb),...))
- Dialogs: square corners, copper top hairline, unified with editorial
chrome
- .browser-item: transparent with copper hover border (was filled card)
- Audio device select uses var(--sans) instead of generic system font
- Mobile container padding tuned for ≤480px screens
- Breadcrumb home is a real <button> with aria-label; aria-current on root
- i18n: filled display.msg.power_*, execution.*, scripts.params.execute,
callbacks.empty in both en + ru
This commit is contained in:
+100
-80
@@ -23,6 +23,17 @@ logger = logging.getLogger(__name__)
|
||||
|
||||
router = APIRouter(prefix="/api/browser", tags=["browser"])
|
||||
|
||||
# Strong refs to background tasks so they don't get garbage-collected mid-flight.
|
||||
_background_tasks: set[asyncio.Task] = set()
|
||||
|
||||
|
||||
def _spawn_background(coro) -> asyncio.Task:
|
||||
"""Schedule a background coroutine and keep a strong ref to its Task."""
|
||||
task = asyncio.create_task(coro)
|
||||
_background_tasks.add(task)
|
||||
task.add_done_callback(_background_tasks.discard)
|
||||
return task
|
||||
|
||||
|
||||
def _require_folder_management() -> None:
|
||||
"""Raise 403 if media folder management is disabled in config."""
|
||||
@@ -38,16 +49,23 @@ async def _broadcast_after_open(controller, label: str, max_wait: float = 2.0) -
|
||||
|
||||
Fires as a background task so the HTTP response returns immediately.
|
||||
"""
|
||||
status = None
|
||||
try:
|
||||
interval = 0.3
|
||||
elapsed = 0.0
|
||||
while elapsed < max_wait:
|
||||
await asyncio.sleep(interval)
|
||||
elapsed += interval
|
||||
status = await controller.get_status()
|
||||
try:
|
||||
status = await controller.get_status()
|
||||
except Exception as poll_err: # noqa: BLE001 — broadcast is best-effort
|
||||
logger.debug("get_status during broadcast poll failed: %s", poll_err)
|
||||
continue
|
||||
if status.state in ("playing", "paused"):
|
||||
break
|
||||
|
||||
if status is None:
|
||||
return
|
||||
status_dict = status.model_dump()
|
||||
await ws_manager.broadcast({"type": "status", "data": status_dict})
|
||||
logger.info(f"Broadcasted status update after opening: {label}")
|
||||
@@ -74,9 +92,14 @@ class FolderUpdateRequest(BaseModel):
|
||||
|
||||
|
||||
class PlayRequest(BaseModel):
|
||||
"""Request model for playing a media file."""
|
||||
"""Request model for playing a media file.
|
||||
|
||||
path: str = Field(..., description="Full path to the media file")
|
||||
Both ``folder_id`` and ``path`` are required so the server can validate
|
||||
the file lives inside a configured media folder.
|
||||
"""
|
||||
|
||||
folder_id: str = Field(..., description="Media folder ID")
|
||||
path: str = Field(..., description="Path relative to folder root")
|
||||
|
||||
|
||||
class PlayFolderRequest(BaseModel):
|
||||
@@ -128,8 +151,10 @@ async def create_folder(
|
||||
"""
|
||||
_require_folder_management()
|
||||
try:
|
||||
# Validate folder_id format (alphanumeric and underscore only)
|
||||
if not request.folder_id.replace("_", "").isalnum():
|
||||
# Validate folder_id format (alphanumeric and underscore only).
|
||||
# Same constraint is enforced when validating paths so traversal can't
|
||||
# be smuggled through the ID itself.
|
||||
if not request.folder_id or not request.folder_id.replace("_", "").isalnum():
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail="Folder ID must contain only alphanumeric characters and underscores",
|
||||
@@ -277,13 +302,15 @@ async def browse(
|
||||
# URL decode the path
|
||||
decoded_path = unquote(path)
|
||||
|
||||
# Browse directory
|
||||
result = BrowserService.browse_directory(
|
||||
folder_id=folder_id,
|
||||
path=decoded_path,
|
||||
offset=offset,
|
||||
limit=limit,
|
||||
nocache=nocache,
|
||||
# Browse directory in a thread — iterdir() + stat() can block on
|
||||
# network shares for many seconds; never run on the event loop.
|
||||
result = await asyncio.to_thread(
|
||||
BrowserService.browse_directory,
|
||||
folder_id,
|
||||
decoded_path,
|
||||
offset,
|
||||
limit,
|
||||
nocache,
|
||||
)
|
||||
|
||||
return result
|
||||
@@ -307,41 +334,40 @@ async def browse(
|
||||
# Metadata Endpoint
|
||||
@router.get("/metadata")
|
||||
async def get_metadata(
|
||||
path: str = Query(..., description="Full path to media file (URL-encoded)"),
|
||||
folder_id: str = Query(..., description="Media folder ID"),
|
||||
path: str = Query(..., description="Path relative to folder root (URL-encoded)"),
|
||||
_: str = Depends(verify_token),
|
||||
):
|
||||
"""Get metadata for a media file.
|
||||
"""Get metadata for a media file inside a configured media folder.
|
||||
|
||||
Args:
|
||||
path: Full path to the media file (URL-encoded).
|
||||
folder_id: ID of the media folder.
|
||||
path: Path relative to folder root (URL-encoded).
|
||||
|
||||
Returns:
|
||||
Media file metadata.
|
||||
|
||||
Raises:
|
||||
HTTPException: If file not found or metadata extraction fails.
|
||||
"""
|
||||
try:
|
||||
# URL decode the path
|
||||
decoded_path = unquote(path)
|
||||
file_path = Path(decoded_path)
|
||||
|
||||
if not file_path.exists():
|
||||
raise HTTPException(status_code=404, detail="File not found")
|
||||
file_path = BrowserService.validate_path(folder_id, decoded_path)
|
||||
|
||||
if not file_path.is_file():
|
||||
raise HTTPException(status_code=400, detail="Path is not a file")
|
||||
if not BrowserService.is_media_file(file_path):
|
||||
raise HTTPException(status_code=400, detail="File is not a media file")
|
||||
|
||||
# Extract metadata in executor (blocking operation)
|
||||
loop = asyncio.get_event_loop()
|
||||
loop = asyncio.get_running_loop()
|
||||
metadata = await loop.run_in_executor(
|
||||
None,
|
||||
MetadataService.extract_metadata,
|
||||
file_path,
|
||||
)
|
||||
|
||||
return metadata
|
||||
|
||||
except ValueError as e:
|
||||
raise HTTPException(status_code=400, detail=str(e))
|
||||
except FileNotFoundError as e:
|
||||
raise HTTPException(status_code=404, detail=str(e))
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception as e:
|
||||
@@ -352,59 +378,47 @@ async def get_metadata(
|
||||
# Thumbnail Endpoint
|
||||
@router.get("/thumbnail")
|
||||
async def get_thumbnail(
|
||||
path: str = Query(..., description="Full path to media file (URL-encoded)"),
|
||||
folder_id: str = Query(..., description="Media folder ID"),
|
||||
path: str = Query(..., description="Path relative to folder root (URL-encoded)"),
|
||||
size: str = Query(default="medium", description='Thumbnail size: "small" or "medium"'),
|
||||
_: str = Depends(verify_token),
|
||||
):
|
||||
"""Get thumbnail for a media file.
|
||||
|
||||
Args:
|
||||
path: Full path to the media file (URL-encoded).
|
||||
size: Thumbnail size ("small" or "medium").
|
||||
|
||||
Returns:
|
||||
JPEG image bytes.
|
||||
|
||||
Raises:
|
||||
HTTPException: If file not found or thumbnail generation fails.
|
||||
"""
|
||||
"""Get thumbnail for a media file inside a configured media folder."""
|
||||
try:
|
||||
# URL decode the path
|
||||
decoded_path = unquote(path)
|
||||
file_path = Path(decoded_path)
|
||||
|
||||
if not file_path.exists():
|
||||
raise HTTPException(status_code=404, detail="File not found")
|
||||
file_path = BrowserService.validate_path(folder_id, decoded_path)
|
||||
|
||||
if not file_path.is_file():
|
||||
raise HTTPException(status_code=400, detail="Path is not a file")
|
||||
if not BrowserService.is_media_file(file_path):
|
||||
raise HTTPException(status_code=400, detail="File is not a media file")
|
||||
|
||||
# Validate size
|
||||
if size not in ("small", "medium"):
|
||||
size = "medium"
|
||||
|
||||
# Get thumbnail
|
||||
thumbnail_data = await ThumbnailService.get_thumbnail(file_path, size)
|
||||
|
||||
if thumbnail_data is None:
|
||||
return Response(status_code=204)
|
||||
|
||||
# Calculate ETag (hash of path + mtime)
|
||||
import hashlib
|
||||
stat = file_path.stat()
|
||||
etag_data = f"{file_path}:{stat.st_mtime}:{size}".encode()
|
||||
etag = hashlib.md5(etag_data).hexdigest()
|
||||
|
||||
# Return image with caching headers
|
||||
return Response(
|
||||
content=thumbnail_data,
|
||||
media_type="image/jpeg",
|
||||
headers={
|
||||
"ETag": f'"{etag}"',
|
||||
"Cache-Control": "public, max-age=86400", # 24 hours
|
||||
"Cache-Control": "public, max-age=86400",
|
||||
},
|
||||
)
|
||||
|
||||
except ValueError as e:
|
||||
raise HTTPException(status_code=400, detail=str(e))
|
||||
except FileNotFoundError as e:
|
||||
raise HTTPException(status_code=404, detail=str(e))
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception as e:
|
||||
@@ -420,44 +434,37 @@ async def play_file(
|
||||
):
|
||||
"""Open a media file with the default system player.
|
||||
|
||||
Args:
|
||||
request: Play request with file path.
|
||||
|
||||
Returns:
|
||||
Success message.
|
||||
|
||||
Raises:
|
||||
HTTPException: If file not found or playback fails.
|
||||
Requires both ``folder_id`` and a folder-relative ``path``; the resolved
|
||||
file must live inside the configured media folder and be a recognized
|
||||
media file. This prevents arbitrary OS-handler invocation (e.g.,
|
||||
``os.startfile`` on Windows ``.lnk``/UNC paths).
|
||||
"""
|
||||
try:
|
||||
file_path = Path(request.path)
|
||||
|
||||
# Validate file exists
|
||||
if not file_path.exists():
|
||||
raise HTTPException(status_code=404, detail="File not found")
|
||||
decoded_path = unquote(request.path)
|
||||
file_path = BrowserService.validate_path(request.folder_id, decoded_path)
|
||||
|
||||
if not file_path.is_file():
|
||||
raise HTTPException(status_code=400, detail="Path is not a file")
|
||||
|
||||
# Validate file is a media file
|
||||
if not BrowserService.is_media_file(file_path):
|
||||
raise HTTPException(status_code=400, detail="File is not a media file")
|
||||
|
||||
# Get media controller and open file
|
||||
controller = get_media_controller()
|
||||
success = await controller.open_file(str(file_path))
|
||||
|
||||
if not success:
|
||||
raise HTTPException(status_code=500, detail="Failed to open file")
|
||||
|
||||
# Poll until player registers with media session API (up to 2s)
|
||||
asyncio.create_task(_broadcast_after_open(controller, file_path.name))
|
||||
_spawn_background(_broadcast_after_open(controller, file_path.name))
|
||||
|
||||
return {
|
||||
"success": True,
|
||||
"message": f"Playing {file_path.name}",
|
||||
}
|
||||
|
||||
except ValueError as e:
|
||||
raise HTTPException(status_code=400, detail=str(e))
|
||||
except FileNotFoundError as e:
|
||||
raise HTTPException(status_code=404, detail=str(e))
|
||||
except HTTPException:
|
||||
raise
|
||||
except Exception as e:
|
||||
@@ -489,26 +496,38 @@ async def play_folder(
|
||||
if not full_path.is_dir():
|
||||
raise HTTPException(status_code=400, detail="Path is not a directory")
|
||||
|
||||
# Collect all media files sorted by name
|
||||
media_files = sorted(
|
||||
[f for f in full_path.iterdir() if f.is_file() and BrowserService.is_media_file(f)],
|
||||
key=lambda f: f.name.lower(),
|
||||
)
|
||||
def _scan(directory: Path) -> list[Path]:
|
||||
return sorted(
|
||||
(
|
||||
f for f in directory.iterdir()
|
||||
if f.is_file() and BrowserService.is_media_file(f)
|
||||
),
|
||||
key=lambda f: f.name.lower(),
|
||||
)
|
||||
|
||||
media_files = await asyncio.to_thread(_scan, full_path)
|
||||
|
||||
if not media_files:
|
||||
raise HTTPException(status_code=404, detail="No media files found in this folder")
|
||||
|
||||
# Generate M3U playlist with absolute paths and EXTINF entries
|
||||
# Written to local temp dir to avoid extra SMB file handle on network shares
|
||||
# Uses utf-8-sig (BOM) so players detect encoding properly
|
||||
# Generate M3U playlist with absolute paths and EXTINF entries.
|
||||
# Use NamedTemporaryFile to get a fresh per-call path — prevents
|
||||
# symlink-clobber races between concurrent /play-folder requests
|
||||
# and any local user pre-creating a fixed temp filename.
|
||||
lines = ["#EXTM3U"]
|
||||
for f in media_files:
|
||||
lines.append(f"#EXTINF:-1,{f.stem}")
|
||||
lines.append(str(f))
|
||||
m3u_content = "\r\n".join(lines) + "\r\n"
|
||||
m3u_content = ("\r\n".join(lines) + "\r\n").encode("utf-8-sig")
|
||||
|
||||
playlist_path = Path(tempfile.gettempdir()) / ".media_server_playlist.m3u"
|
||||
playlist_path.write_text(m3u_content, encoding="utf-8-sig")
|
||||
with tempfile.NamedTemporaryFile(
|
||||
mode="wb",
|
||||
prefix=".media_server_playlist_",
|
||||
suffix=".m3u",
|
||||
delete=False,
|
||||
) as f:
|
||||
f.write(m3u_content)
|
||||
playlist_path = Path(f.name)
|
||||
|
||||
# Open playlist with default player
|
||||
controller = get_media_controller()
|
||||
@@ -517,8 +536,9 @@ async def play_folder(
|
||||
if not success:
|
||||
raise HTTPException(status_code=500, detail="Failed to open playlist")
|
||||
|
||||
# Poll until player registers with media session API (up to 2s)
|
||||
asyncio.create_task(_broadcast_after_open(controller, f"playlist ({len(media_files)} files)"))
|
||||
_spawn_background(
|
||||
_broadcast_after_open(controller, f"playlist ({len(media_files)} files)")
|
||||
)
|
||||
|
||||
return {
|
||||
"success": True,
|
||||
|
||||
Reference in New Issue
Block a user