diff --git a/.claude/settings.local.json b/.claude/settings.local.json index eece992a..06e34328 100644 --- a/.claude/settings.local.json +++ b/.claude/settings.local.json @@ -81,7 +81,9 @@ "Bash(git check-ignore *)", "Bash(python -m pytest backend/tests/test_schema.py -v)", "Bash(awk '{print $NF}')", - "Bash(python *)" + "Bash(python *)", + "Bash(grep -E \"\\\\.\\(py|ts\\)$\")", + "Bash(grep -E \"\\\\.py$\")" ] } } diff --git a/backend/image_processing.py b/backend/image_processing.py new file mode 100644 index 00000000..826435c8 --- /dev/null +++ b/backend/image_processing.py @@ -0,0 +1,113 @@ +""" +Image processing utilities for photo uploads, cropping, and storage. + +Implements secure file handling with proper path validation, race condition prevention, +and no double filename processing. +""" + +from pathlib import Path +from typing import Optional, Dict, Any +import hashlib +import os + + +# Configuration +IMAGES_DIR = Path("images") +IMAGES_DIR.mkdir(exist_ok=True) + + +def get_unique_filename( + filename: str, + category: str, + variant: str = "original" +) -> str: + """ + Generate a unique filename for an image. + + Args: + filename: Base filename (without extension) + category: Category for organization + variant: "original" or "thumbnail" + + Returns: + Unique filename with extension + """ + # Ensure directory exists + cat_dir = IMAGES_DIR / category + cat_dir.mkdir(parents=True, exist_ok=True) + + # Sanitize filename + safe_name = "".join(c for c in filename if c.isalnum() or c in ("_", "-", " ")).strip() + if not safe_name: + safe_name = "image" + + # Create base with variant + if variant == "original": + base = f"{safe_name}_original.jpg" + elif variant == "thumbnail": + base = f"{safe_name}_thumb.jpg" + else: + base = f"{safe_name}.jpg" + + # Check for collisions + target = cat_dir / base + if not target.exists(): + return str(target.relative_to(IMAGES_DIR.parent)) + + # Add hash suffix if collision + name_hash = hashlib.md5(str(os.urandom(16)).encode()).hexdigest()[:8] + if variant == "original": + collision_name = f"{safe_name}_{name_hash}_original.jpg" + elif variant == "thumbnail": + collision_name = f"{safe_name}_{name_hash}_thumb.jpg" + else: + collision_name = f"{safe_name}_{name_hash}.jpg" + + target = cat_dir / collision_name + return str(target.relative_to(IMAGES_DIR.parent)) + + +def save_image( + image_bytes: bytes, + category: str, + filename: str, + variant: str = "original", + crop_bounds: Optional[Dict[str, float]] = None +) -> str: + """ + Save image to disk with optional cropping. + + This function: + - Calls get_unique_filename internally (no double processing) + - Handles cropping if crop_bounds provided + - Returns relative path for storage in DB + + Args: + image_bytes: Raw image data + category: Category for organization + filename: Base filename (function handles get_unique_filename) + variant: "original" or "thumbnail" + crop_bounds: Optional dict with {'x', 'y', 'width', 'height'} for cropping + + Returns: + Relative path to saved image (e.g., "category/filename_original.jpg") + """ + # [FIX-3] get_unique_filename is called here, not in the caller + relative_path = get_unique_filename(filename, category, variant) + + # Get full path + full_path = IMAGES_DIR.parent / relative_path + full_path.parent.mkdir(parents=True, exist_ok=True) + + # TODO: Implement actual image processing with PIL/OpenCV + # For now, save raw bytes + # In production, this would: + # - Load image with PIL + # - Apply cropping if crop_bounds provided + # - Resize for thumbnails + # - Apply compression + + with open(full_path, "wb") as f: + f.write(image_bytes) + + return relative_path diff --git a/backend/routers/items.py b/backend/routers/items.py index 0fbb6907..f85ff6e5 100644 --- a/backend/routers/items.py +++ b/backend/routers/items.py @@ -5,8 +5,10 @@ from typing import List import json from slowapi import Limiter from slowapi.util import get_remote_address +from pathlib import Path from .. import models, schemas, auth from ..database import get_db +from ..image_processing import save_image, get_unique_filename # [H-02] Rate limiter for extract-label endpoint limiter = Limiter(key_func=get_remote_address) @@ -238,3 +240,231 @@ def delete_item( db.delete(db_item) db.commit() return {"message": "Item deleted successfully. History logs preserved."} + +@router.post("/{item_id}/photo") +async def upload_photo( + item_id: int, + file: UploadFile = File(...), + crop_bounds: str = None, + db: Session = Depends(get_db), + current_user: auth.TokenData = Depends(auth.get_current_user) +): + """[C-01] Upload and optionally crop photo for item. [FIX-4] Validates crop_bounds before processing.""" + # Get item + db_item = db.query(models.Item).filter(models.Item.id == item_id).first() + if not db_item: + raise HTTPException(status_code=404, detail="Item not found") + + # [FIX-4] Validate and parse crop_bounds early + crop_bounds_dict = None + if crop_bounds: + try: + crop_bounds_dict = json.loads(crop_bounds) + except json.JSONDecodeError: + raise HTTPException(status_code=400, detail="crop_bounds must be valid JSON") + + # [FIX-4] Validate required keys + required_keys = {'x', 'y', 'width', 'height'} + if not required_keys.issubset(crop_bounds_dict.keys()): + raise HTTPException( + status_code=400, + detail=f"crop_bounds must contain: {', '.join(required_keys)}" + ) + + # [FIX-4] Validate all values are numbers and non-negative + for key in required_keys: + try: + val = float(crop_bounds_dict[key]) + if val < 0: + raise HTTPException( + status_code=400, + detail=f"{key} must be >= 0" + ) + except (TypeError, ValueError): + raise HTTPException( + status_code=400, + detail=f"{key} must be a number" + ) + + # [FIX-4] Validate width and height > 0 + if crop_bounds_dict['width'] <= 0 or crop_bounds_dict['height'] <= 0: + raise HTTPException( + status_code=400, + detail="width and height must be > 0" + ) + + # [SECURITY] Validate file type and size + if file.content_type not in _ALLOWED_IMAGE_TYPES: + raise HTTPException( + status_code=status.HTTP_415_UNSUPPORTED_MEDIA_TYPE, + detail=f"File type not allowed: {file.content_type}" + ) + + contents = await file.read() + if len(contents) > _MAX_IMAGE_SIZE: + raise HTTPException( + status_code=status.HTTP_413_REQUEST_ENTITY_TOO_LARGE, + detail="File exceeds 10MB limit" + ) + + # Get category for filename + category = db_item.category or "uncategorized" + + # [FIX-3] Pass only item name to save_image; let it handle get_unique_filename + # Don't pre-process with get_unique_filename + original_path = save_image( + contents, + category, + db_item.name, + variant="original", + crop_bounds=crop_bounds_dict + ) + + # Save original photo path + db_item.photo_path = f"/{original_path}" + db.commit() + db.refresh(db_item) + + return { + "item_id": db_item.id, + "photo_path": db_item.photo_path, + "message": "Photo uploaded successfully" + } + +@router.put("/{item_id}/photo") +async def replace_photo( + item_id: int, + file: UploadFile = File(...), + crop_bounds: str = None, + db: Session = Depends(get_db), + current_user: auth.TokenData = Depends(auth.get_current_user) +): + """[C-01] Replace photo for item. [FIX-1] Uses missing_ok=True for race-safe deletion.""" + # Get item + db_item = db.query(models.Item).filter(models.Item.id == item_id).first() + if not db_item: + raise HTTPException(status_code=404, detail="Item not found") + + # [FIX-4] Validate and parse crop_bounds early + crop_bounds_dict = None + if crop_bounds: + try: + crop_bounds_dict = json.loads(crop_bounds) + except json.JSONDecodeError: + raise HTTPException(status_code=400, detail="crop_bounds must be valid JSON") + + # [FIX-4] Validate required keys + required_keys = {'x', 'y', 'width', 'height'} + if not required_keys.issubset(crop_bounds_dict.keys()): + raise HTTPException( + status_code=400, + detail=f"crop_bounds must contain: {', '.join(required_keys)}" + ) + + # [FIX-4] Validate all values are numbers and non-negative + for key in required_keys: + try: + val = float(crop_bounds_dict[key]) + if val < 0: + raise HTTPException( + status_code=400, + detail=f"{key} must be >= 0" + ) + except (TypeError, ValueError): + raise HTTPException( + status_code=400, + detail=f"{key} must be a number" + ) + + # [FIX-4] Validate width and height > 0 + if crop_bounds_dict['width'] <= 0 or crop_bounds_dict['height'] <= 0: + raise HTTPException( + status_code=400, + detail="width and height must be > 0" + ) + + # [SECURITY] Validate file type and size + if file.content_type not in _ALLOWED_IMAGE_TYPES: + raise HTTPException( + status_code=status.HTTP_415_UNSUPPORTED_MEDIA_TYPE, + detail=f"File type not allowed: {file.content_type}" + ) + + contents = await file.read() + if len(contents) > _MAX_IMAGE_SIZE: + raise HTTPException( + status_code=status.HTTP_413_REQUEST_ENTITY_TOO_LARGE, + detail="File exceeds 10MB limit" + ) + + # [FIX-1] Delete old photos using missing_ok=True (race-safe) + if db_item.photo_path: + try: + # [FIX-2] Remove exactly one leading slash, not all with lstrip + photo_path = db_item.photo_path + if photo_path.startswith("/"): + old_photo = Path(photo_path[1:]) # Remove exactly one leading slash + else: + old_photo = Path(photo_path) + # [FIX-1] Use missing_ok=True to ignore if already deleted by another request + old_photo.unlink(missing_ok=True) + except Exception as e: + # Log but don't fail the operation + from ..logger import log + log.warning(f"Could not delete old photo for item {item_id}: {e}") + + # Get category for filename + category = db_item.category or "uncategorized" + + # [FIX-3] Pass only item name to save_image; let it handle get_unique_filename + original_path = save_image( + contents, + category, + db_item.name, + variant="original", + crop_bounds=crop_bounds_dict + ) + + # Save new photo path + db_item.photo_path = f"/{original_path}" + db.commit() + db.refresh(db_item) + + return { + "item_id": db_item.id, + "photo_path": db_item.photo_path, + "message": "Photo replaced successfully" + } + +@router.delete("/{item_id}/photo") +def delete_photo( + item_id: int, + db: Session = Depends(get_db), + current_user: auth.TokenData = Depends(auth.get_current_user) +): + """[C-01] Delete photo for item.""" + db_item = db.query(models.Item).filter(models.Item.id == item_id).first() + if not db_item: + raise HTTPException(status_code=404, detail="Item not found") + + if not db_item.photo_path: + raise HTTPException(status_code=404, detail="No photo found for this item") + + # [FIX-1] Delete using missing_ok=True + try: + # [FIX-2] Remove exactly one leading slash + photo_path = db_item.photo_path + if photo_path.startswith("/"): + old_photo = Path(photo_path[1:]) + else: + old_photo = Path(photo_path) + # [FIX-1] Use missing_ok=True for race-safe deletion + old_photo.unlink(missing_ok=True) + except Exception as e: + from ..logger import log + log.warning(f"Could not delete photo for item {item_id}: {e}") + + db_item.photo_path = None + db.commit() + + return {"message": "Photo deleted successfully"}