From 4faaa5f95694f8fd77c4a3767d19a7ce0a7636a0 Mon Sep 17 00:00:00 2001 From: "claude[bot]" <41898282+claude[bot]@users.noreply.github.com> Date: Thu, 21 May 2026 02:35:54 +0000 Subject: [PATCH 1/2] fix(curation): fix Export Dataset button always reporting 'undefined' files Three bugs combined to break the Export Dataset feature: 1. check_album_lock() was called with no argument in export_dataset, which blocks all exports whenever PHOTOMAP_ALBUM_LOCKED is set -- even for albums that are in the allowed list. Export is not a destructive album-management operation; the correct per-album lock check is already done inside validate_album_exists() which is called immediately after. 2. An overly-restrictive is_within_base_dir check required the export destination to be inside Path.home(). Users exporting to external drives or network mounts (e.g. /Volumes/..., /mnt/...) received a 400 error silently. Source-path security is handled by validate_image_access(); no restriction on the destination is necessary. 3. The frontend export handler did not check response.ok before accessing data.exported. Any server-side error returned {"detail": "..."} without an exported field, causing the dialog to read "Exported undefined files" with no indication of what went wrong. Fixes #245 Co-authored-by: Lincoln Stein --- photomap/backend/routers/curation.py | 25 ++---- .../frontend/static/javascript/curation.js | 4 + tests/backend/test_curation.py | 79 +++++++------------ 3 files changed, 39 insertions(+), 69 deletions(-) diff --git a/photomap/backend/routers/curation.py b/photomap/backend/routers/curation.py index 6442f9c..11f85e9 100644 --- a/photomap/backend/routers/curation.py +++ b/photomap/backend/routers/curation.py @@ -278,8 +278,8 @@ async def export_dataset(request: ExportRequest): Returns: JSON response with success count and any errors. """ - check_album_lock() # May raise a 403 exception - # Validate and sanitize the output folder to prevent path traversal + # Export is not a destructive album-management operation; the per-album + # lock check is already handled inside validate_album_exists() below. if not request.output_folder: raise HTTPException(status_code=400, detail="Output folder required") @@ -290,26 +290,11 @@ async def export_dataset(request: ExportRequest): except Exception as e: raise HTTPException(status_code=400, detail=f"Invalid output folder: {e}") from e - # Define the base directory under which exports are allowed - # Use user's home directory as the base to prevent system-wide access - base_dir = Path.home().resolve() - - # Ensure the export directory is within the allowed base directory - def is_within_base_dir(target_dir: Path, base: Path) -> bool: - """Check if target directory is within the base directory.""" - if os.name == "nt": - # On Windows, also ensure the drive matches - return target_dir.drive.lower() == base.drive.lower() and (target_dir == base or base in target_dir.parents) - else: - return target_dir == base or base in target_dir.parents - - if not is_within_base_dir(output_dir, base_dir): - raise HTTPException(status_code=400, detail="Output folder is outside the allowed export directory") - # Resolve the album so we can verify each source path lives inside it — # otherwise a caller could ask us to copy /etc/passwd into their export - # dir. Validated after the output-folder checks so cheap input errors - # surface as 400 even when the album key is bogus. + # dir. Source-path security is handled by validate_image_access(); no + # home-dir restriction is placed on the destination so users can export + # to external drives, network mounts, etc. album_config = validate_album_exists(request.album) if not output_dir.exists(): diff --git a/photomap/frontend/static/javascript/curation.js b/photomap/frontend/static/javascript/curation.js index 688f5d5..f875da9 100644 --- a/photomap/frontend/static/javascript/curation.js +++ b/photomap/frontend/static/javascript/curation.js @@ -585,11 +585,15 @@ function setupEventListeners() { }), }); const data = await response.json(); + if (!response.ok) { + throw new Error(data.detail || `Export failed (${response.status})`); + } alert(`Exported ${data.exported} files.`); setStatus("Export Complete.", "success"); } catch (e) { console.error(e); alert("Export failed: " + e.message); + setStatus("Export failed.", "error"); } }; diff --git a/tests/backend/test_curation.py b/tests/backend/test_curation.py index e86cd3c..8f8d95b 100644 --- a/tests/backend/test_curation.py +++ b/tests/backend/test_curation.py @@ -3,9 +3,7 @@ Tests for the curation functionality (Model Training Dataset Curator). """ -import tempfile import time -from pathlib import Path import pytest from fixtures import build_index @@ -269,29 +267,27 @@ def test_export_endpoint(client, new_album, monkeypatch, tmp_path): data = response.json() selected_files = data["selected_files"] - # Create export folder within home directory (as required by endpoint) - with tempfile.TemporaryDirectory(dir=Path.home()) as temp_dir: - export_folder = Path(temp_dir) / "exported_images" + export_folder = tmp_path / "exported_images" - # Export the files - response = client.post( - "/api/curation/export", - json={ - "album": new_album["key"], - "filenames": selected_files, - "output_folder": str(export_folder) - } - ) - assert response.status_code == 200 - result = response.json() - assert result["status"] == "success" - assert "exported" in result - assert result["exported"] > 0 + # Export the files + response = client.post( + "/api/curation/export", + json={ + "album": new_album["key"], + "filenames": selected_files, + "output_folder": str(export_folder) + } + ) + assert response.status_code == 200 + result = response.json() + assert result["status"] == "success" + assert "exported" in result + assert result["exported"] > 0 - # Verify files were actually exported - assert export_folder.exists() - exported_files = list(export_folder.iterdir()) - assert len(exported_files) > 0 + # Verify files were actually exported + assert export_folder.exists() + exported_files = list(export_folder.iterdir()) + assert len(exported_files) > 0 def test_export_validation(client, tmp_path): @@ -321,37 +317,22 @@ def test_export_validation(client, tmp_path): assert response.status_code == 400 -def test_export_path_traversal_protection(client): - """Test that export prevents path traversal attacks.""" - # Output-folder validation runs before album resolution. +def test_export_nonexistent_files(client, new_album, tmp_path): + """Test export with nonexistent files.""" + export_folder = tmp_path / "export_test" + response = client.post( "/api/curation/export", json={ - "album": "any-key", - "filenames": ["some_file.jpg"], - "output_folder": "/etc" + "album": new_album["key"], + "filenames": ["/nonexistent/file1.jpg", "/nonexistent/file2.jpg"], + "output_folder": str(export_folder) } ) - assert response.status_code == 400 - - -def test_export_nonexistent_files(client, new_album): - """Test export with nonexistent files.""" - with tempfile.TemporaryDirectory(dir=Path.home()) as temp_dir: - export_folder = Path(temp_dir) / "export_test" - - response = client.post( - "/api/curation/export", - json={ - "album": new_album["key"], - "filenames": ["/nonexistent/file1.jpg", "/nonexistent/file2.jpg"], - "output_folder": str(export_folder) - } - ) - assert response.status_code == 200 - result = response.json() - # Should succeed but with 0 exported - assert result["exported"] == 0 + assert response.status_code == 200 + result = response.json() + # Should succeed but with 0 exported + assert result["exported"] == 0 def test_curate_multiple_iterations(client, new_album, monkeypatch): From 39b7b222f46cd0e166b40b660ac99ffd3286aaf7 Mon Sep 17 00:00:00 2001 From: Lincoln Stein Date: Thu, 21 May 2026 08:07:31 -0400 Subject: [PATCH 2/2] chore(backend): ruff --- photomap/backend/routers/curation.py | 1 - 1 file changed, 1 deletion(-) diff --git a/photomap/backend/routers/curation.py b/photomap/backend/routers/curation.py index 11f85e9..6a7617e 100644 --- a/photomap/backend/routers/curation.py +++ b/photomap/backend/routers/curation.py @@ -16,7 +16,6 @@ from ..embeddings import _open_npz_file, get_fps_indices_global, get_kmeans_indices_global from ..progress import IndexStatus, progress_tracker from .album import validate_album_exists, validate_image_access -from .index import check_album_lock router = APIRouter() logger = logging.getLogger(__name__)