From 241fa9d0e0013eb31cbc9b9fda5ff0415ce8b35c Mon Sep 17 00:00:00 2001 From: coder-bot Date: Mon, 17 Aug 2026 23:42:56 +0000 Subject: [PATCH] WIP checkpoint: Address review feedback: Address review feedback: Scaffold FastAPI backend project structure --- backend/app/routers/assets.py | 37 +++++++++++++++++++++++++---------- backend/app/routers/users.py | 19 +++++++++--------- 2 files changed, 37 insertions(+), 19 deletions(-) diff --git a/backend/app/routers/assets.py b/backend/app/routers/assets.py index 0d38860..1f23e9b 100644 --- a/backend/app/routers/assets.py +++ b/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, file: UploadFile = File(...), - current_user: CurrentUser = None, + current_user: CurrentUser = Depends(), 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 diff --git a/backend/app/routers/users.py b/backend/app/routers/users.py index 1ed37cd..6ba19b5 100644 --- a/backend/app/routers/users.py +++ b/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)