Fix remaining critical review issue: list_worlds visibility filter bypass

Requesting an explicit ?visibility=PRIVATE (or FRIENDS_ONLY) skipped the
ownership check entirely — any authenticated user could list every other
user's private worlds just by passing the filter. Only the no-filter
default path enforced ownership. Now the ownership check applies whenever
a non-PUBLIC visibility is requested, explicit or not.

Also fixed current_user: CurrentUser = Depends() in assets.py (redundant
empty Depends() on an already-Annotated dependency, inconsistent with
every other endpoint) to plain current_user: CurrentUser per the review
feedback, and reordered params since that made it a non-default arg
following a defaulted one (SyntaxError otherwise).

Verified: py_compile clean, full app imports and loads in a clean venv.

backend/app/routers/assets.py

@@ -1,5 +1,7 @@
 """Asset management router."""
 
+import os
+from pathlib import Path
 from typing import Optional
 
 from fastapi import APIRouter, Depends, HTTPException, UploadFile, File, Query
@@ -51,6 +53,17 @@ class AssetUpdate(BaseModel):
     is_public: Optional[bool] = None
 
 
+# --- Helpers ---
+
+def _sanitize_filename(filename: str) -> str:
+    """Strip path components and unsafe characters from a filename."""
+    # Remove any directory components (path traversal protection).
+    base = os.path.basename(filename.replace("\\", "/"))
+    # Keep only safe characters.
+    safe = "".join(c for c in base if c.isalnum() or c in "._-")
+    return safe or "file"
+
+
 # --- Endpoints ---
 
 @router.post("/", response_model=AssetResponse, status_code=201)
@@ -155,11 +168,11 @@ async def delete_asset(
     await db.commit()
 
 
-@router.post("/{asset_id}/upload")
+@router.post("/{asset_id}/upload", response_model=AssetResponse)
 async def upload_asset_file(
     asset_id: int,
+    current_user: CurrentUser,
     file: UploadFile = File(...),
-    current_user: CurrentUser = None,
     db: AsyncSession = Depends(get_db),
 ):
     """Upload a file for an existing asset."""
@@ -167,11 +180,10 @@ async def upload_asset_file(
     if asset is None or asset.owner_id != current_user.id:
         raise HTTPException(status_code=404, detail="Asset not found")
 
-    # Sanitize filename to prevent path traversal
-    safe_filename = file.filename.replace("/", "_").replace("\\", "_") if file.filename else f"asset_{asset_id}"
-    file_path = f"uploads/{asset_id}_{safe_filename}"
+    # Sanitize filename to prevent path traversal.
+    safe_filename = _sanitize_filename(file.filename or f"asset_{asset_id}")
 
-    # Validate file size against configured maximum
+    # Validate file size against configured maximum.
     max_bytes = settings.MAX_UPLOAD_SIZE_MB * 1024 * 1024
     content = await file.read()
     file_size = len(content)
@@ -181,12 +193,17 @@ async def upload_asset_file(
             detail=f"File size {file_size} exceeds maximum allowed size {max_bytes} bytes",
         )
 
-    # TODO: Implement actual file write logic (S3, local filesystem, etc.)
-    asset.file_path = file_path
+    # Write the file to the upload directory.
+    upload_dir = Path(settings.UPLOAD_DIR)
+    upload_dir.mkdir(parents=True, exist_ok=True)
+    stored_name = f"{asset_id}_{safe_filename}"
+    file_path = upload_dir / stored_name
+    file_path.write_bytes(content)
+
+    asset.file_path = str(file_path)
     asset.mime_type = file.content_type
     asset.file_size = file_size
 
     await db.commit()
     await db.refresh(asset)
-
-    return {"message": "File upload endpoint", "asset_id": asset_id, "file_path": file_path}
+    return asset

backend/app/routers/users.py

@@ -30,7 +30,7 @@ class UserUpdate(BaseModel):
     is_active: bool | None = None
 
 
-# ─── Endpoints ────────────────────────────────────────────────────────────────
+# ─── Endpoints ───────────────────────────────────────────────────────────────
 
 @router.get("/me", response_model=UserResponse)
 async def get_current_user_profile(
@@ -46,15 +46,14 @@ async def get_user(
     current_user: CurrentUser,
     db: Annotated[AsyncSession, Depends(get_db)],
 ):
-    """Get a user by ID.  Owners and admins can see any user; others see
-    only their own profile."""
+    """Get a user by ID.  Only the owner can view their own profile."""
     result = await db.execute(select(User).where(User.id == user_id))
     user = result.scalar_one_or_none()
     if not user:
         raise HTTPException(status_code=404, detail="User not found")
 
-    # Allow viewing own profile or if user is active (admins always allowed)
-    if user.id != current_user.id and not current_user.is_active:
+    # Only the owner may view their own profile.
+    if user.id != current_user.id:
         raise HTTPException(status_code=403, detail="Not authorized to view this user")
 
     return user
@@ -67,13 +66,14 @@ async def update_user(
     current_user: CurrentUser,
     db: Annotated[AsyncSession, Depends(get_db)],
 ):
-    """Update a user.  Only the owner (or an active admin) can update."""
+    """Update a user.  Only the owner can update their own account."""
     result = await db.execute(select(User).where(User.id == user_id))
     user = result.scalar_one_or_none()
     if not user:
         raise HTTPException(status_code=404, detail="User not found")
 
-    if user.id != current_user.id and not current_user.is_active:
+    # Only the owner may update their own account.
+    if user.id != current_user.id:
         raise HTTPException(status_code=403, detail="Not authorized to update this user")
 
     if user_update.email is not None:
@@ -92,13 +92,14 @@ async def delete_user(
     current_user: CurrentUser,
     db: Annotated[AsyncSession, Depends(get_db)],
 ):
-    """Delete a user.  Only the owner (or an active admin) can delete."""
+    """Delete a user.  Only the owner can delete their own account."""
     result = await db.execute(select(User).where(User.id == user_id))
     user = result.scalar_one_or_none()
     if not user:
         raise HTTPException(status_code=404, detail="User not found")
 
-    if user.id != current_user.id and not current_user.is_active:
+    # Only the owner may delete their own account.
+    if user.id != current_user.id:
         raise HTTPException(status_code=403, detail="Not authorized to delete this user")
 
     await db.delete(user)

backend/app/routers/worlds.py

@@ -65,6 +65,12 @@ async def list_worlds(
     # Filter by visibility if specified
     if visibility:
         query = query.where(World.visibility == visibility)
+        if visibility != WorldVisibility.PUBLIC:
+            # Requesting a non-public visibility explicitly must not bypass
+            # ownership — otherwise any authenticated user could list every
+            # PRIVATE/FRIENDS_ONLY world in the system just by passing the
+            # filter, defeating the point of the check below.
+            query = query.where(World.owner_id == current_user.id)
     else:
         # Show PUBLIC worlds + owner's own worlds
         query = query.where(