Conversation
\x18# with '#' will be ignored, and an empty message aborts the commit.
c1d1aab to
5955834
Compare
| ) -> list[str]: | ||
| """Return a sorted list of TIFF images in the given directory.""" | ||
|
|
||
| dir_path = (Path(CEPH_DIR) / path).resolve() |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 8 days ago
General approach: treat user input as untrusted, resolve it against a fixed trusted base directory, and explicitly enforce that the resolved result is inside that base. Reject absolute user paths and traversal attempts before accessing the filesystem.
Best concrete fix in plotting_service/routers/imat.py (around lines 104–110 in list_imat_images):
- Create a trusted base path once in the function:
base_dir = Path(CEPH_DIR).resolve(). - Reject absolute user input (
Path(path).is_absolute()). - Resolve candidate path from base + relative input.
- Enforce containment with
dir_path.relative_to(base_dir)inside atry/except ValueError. - Return
HTTP 400for invalid/traversal paths, and keep existing404behavior for not-found directories. - Keep existing functionality otherwise (same successful output, same sorting/filtering).
No new imports or dependencies are required; Path and HTTPException/HTTPStatus are already present.
| @@ -101,7 +101,17 @@ | ||
| ) -> list[str]: | ||
| """Return a sorted list of TIFF images in the given directory.""" | ||
|
|
||
| dir_path = (Path(CEPH_DIR) / path).resolve() | ||
| base_dir = Path(CEPH_DIR).resolve() | ||
| requested_path = Path(path) | ||
| if requested_path.is_absolute(): | ||
| raise HTTPException(status_code=HTTPStatus.BAD_REQUEST, detail="Invalid directory path") | ||
|
|
||
| dir_path = (base_dir / requested_path).resolve() | ||
| try: | ||
| dir_path.relative_to(base_dir) | ||
| except ValueError as err: | ||
| raise HTTPException(status_code=HTTPStatus.BAD_REQUEST, detail="Invalid directory path") from err | ||
|
|
||
| # Security: Ensure path is within CEPH_DIR | ||
| try: | ||
| safe_check_filepath(dir_path, CEPH_DIR) |
| except FileNotFoundError as err: | ||
| raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Directory not found") from err | ||
|
|
||
| if not dir_path.exists() or not dir_path.is_dir(): |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 8 days ago
General fix: normalize the user-supplied relative path against a trusted base directory and explicitly verify the resolved target remains under that base before any filesystem access.
Best concrete fix in plotting_service/routers/imat.py:
- In
list_imat_images, resolve a trustedbase_dir = Path(CEPH_DIR).resolve(). - Reject absolute input paths early (
Path(path).is_absolute()). - Build
dir_path = (base_dir / path).resolve(). - Enforce containment with
dir_path.relative_to(base_dir)inside atry/except ValueError; if it fails, returnHTTP 400. - Keep existing not-found behavior for non-existing/non-directory targets.
- Keep existing functionality (listing image filenames) unchanged for valid in-scope paths.
No new imports are required.
| @@ -101,7 +101,17 @@ | ||
| ) -> list[str]: | ||
| """Return a sorted list of TIFF images in the given directory.""" | ||
|
|
||
| dir_path = (Path(CEPH_DIR) / path).resolve() | ||
| base_dir = Path(CEPH_DIR).resolve() | ||
| user_path = Path(path) | ||
| if user_path.is_absolute(): | ||
| raise HTTPException(status_code=HTTPStatus.BAD_REQUEST, detail="Invalid directory path") | ||
|
|
||
| dir_path = (base_dir / user_path).resolve() | ||
| try: | ||
| dir_path.relative_to(base_dir) | ||
| except ValueError as err: | ||
| raise HTTPException(status_code=HTTPStatus.BAD_REQUEST, detail="Invalid directory path") from err | ||
|
|
||
| # Security: Ensure path is within CEPH_DIR | ||
| try: | ||
| safe_check_filepath(dir_path, CEPH_DIR) |
| except FileNotFoundError as err: | ||
| raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Directory not found") from err | ||
|
|
||
| if not dir_path.exists() or not dir_path.is_dir(): |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 8 days ago
Use explicit path containment validation in list_imat_images before any filesystem access:
- Build a canonical base path:
base_dir = Path(CEPH_DIR).resolve(). - Build and resolve candidate path from user input.
- Enforce containment with
dir_path.relative_to(base_dir)(raisesValueErrorif outside base). - Reject invalid paths with
HTTP 400. - Keep existing not-found behavior for missing/non-directory paths.
This preserves functionality (valid relative paths under CEPH_DIR still work) while preventing traversal/absolute path escapes and making the sanitizer obvious to CodeQL.
Edit only plotting_service/routers/imat.py, inside list_imat_images around lines 104–112.
| @@ -15,8 +15,8 @@ | ||
| convert_image_to_rgb_array, | ||
| find_latest_image_in_directory, | ||
| ) | ||
| from plotting_service.utils import safe_check_filepath | ||
|
|
||
|
|
||
| ImatRouter = APIRouter() | ||
|
|
||
| IMAT_DIR: Path = Path(os.getenv("IMAT_DIR", "/imat")).resolve() | ||
| @@ -101,12 +100,14 @@ | ||
| ) -> list[str]: | ||
| """Return a sorted list of TIFF images in the given directory.""" | ||
|
|
||
| dir_path = (Path(CEPH_DIR) / path).resolve() | ||
| # Security: Ensure path is within CEPH_DIR | ||
| base_dir = Path(CEPH_DIR).resolve() | ||
| dir_path = (base_dir / path).resolve() | ||
|
|
||
| # Security: ensure resolved path stays under CEPH_DIR | ||
| try: | ||
| safe_check_filepath(dir_path, CEPH_DIR) | ||
| except FileNotFoundError as err: | ||
| raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Directory not found") from err | ||
| dir_path.relative_to(base_dir) | ||
| except ValueError as err: | ||
| raise HTTPException(status_code=HTTPStatus.BAD_REQUEST, detail="Invalid directory path") from err | ||
|
|
||
| if not dir_path.exists() or not dir_path.is_dir(): | ||
| raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Directory not found") |
| ) -> Response: | ||
| """Return the raw data of a specific TIFF image as binary.""" | ||
|
|
||
| image_path = (Path(CEPH_DIR) / path).resolve() |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 8 days ago
Use strict, local path validation before any filesystem access:
- Resolve a canonical base path once:
ceph_root = Path(CEPH_DIR).resolve(). - Reject absolute user paths early (
Path(path).is_absolute()). - Build candidate path and resolve:
image_path = (ceph_root / path).resolve(). - Enforce containment with
image_path.relative_to(ceph_root)(raisesValueErrorif traversal escapes root). - Keep existing
safe_check_filepathcall for defense in depth. - Keep functional behavior unchanged otherwise.
Apply this in plotting_service/routers/imat.py within get_imat_image around current lines 133–139.
| @@ -130,7 +130,17 @@ | ||
| ) -> Response: | ||
| """Return the raw data of a specific TIFF image as binary.""" | ||
|
|
||
| image_path = (Path(CEPH_DIR) / path).resolve() | ||
| ceph_root = Path(CEPH_DIR).resolve() | ||
| requested_path = Path(path) | ||
| if requested_path.is_absolute(): | ||
| raise HTTPException(status_code=HTTPStatus.BAD_REQUEST, detail="Path must be relative to CEPH_DIR") | ||
|
|
||
| image_path = (ceph_root / requested_path).resolve() | ||
| try: | ||
| image_path.relative_to(ceph_root) | ||
| except ValueError as err: | ||
| raise HTTPException(status_code=HTTPStatus.BAD_REQUEST, detail="Invalid path") from err | ||
|
|
||
| # Security: Ensure path is within CEPH_DIR | ||
| try: | ||
| safe_check_filepath(image_path, CEPH_DIR) |
| except FileNotFoundError as err: | ||
| raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Directory not found") from err | ||
|
|
||
| if not image_path.exists() or not image_path.is_file(): |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 8 days ago
General fix: for user-controlled file paths, normalize/canonicalize and explicitly enforce that the resolved path stays under a trusted root before any filesystem operations.
Best fix here (single-file, minimal behavior change): in plotting_service/routers/imat.py, inside get_imat_image, compute a resolved base_dir = Path(CEPH_DIR).resolve() and enforce containment with image_path.relative_to(base_dir) (or equivalent). If containment fails, return 404 (matching current not-found behavior style). Keep existing safe_check_filepath call for compatibility/defense-in-depth.
Changes needed:
- In
get_imat_imagearound lines 133–140:- Introduce
base_dir. - Build
image_pathfrombase_dir. - Add explicit
relative_tocontainment guard. - Keep existing
safe_check_filepathtry/except.
- Introduce
- No new imports or dependencies required.
| @@ -130,9 +130,17 @@ | ||
| ) -> Response: | ||
| """Return the raw data of a specific TIFF image as binary.""" | ||
|
|
||
| image_path = (Path(CEPH_DIR) / path).resolve() | ||
| # Security: Ensure path is within CEPH_DIR | ||
| base_dir = Path(CEPH_DIR).resolve() | ||
| image_path = (base_dir / path).resolve() | ||
|
|
||
| # Security: Ensure resolved path is contained within CEPH_DIR | ||
| try: | ||
| image_path.relative_to(base_dir) | ||
| except ValueError: | ||
| raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Image not found") | ||
|
|
||
| # Security: Keep existing centralized path safety validation | ||
| try: | ||
| safe_check_filepath(image_path, CEPH_DIR) | ||
| except FileNotFoundError as err: | ||
| raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Directory not found") from err |
| except FileNotFoundError as err: | ||
| raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Directory not found") from err | ||
|
|
||
| if not image_path.exists() or not image_path.is_file(): |
Check failure
Code scanning / CodeQL
Uncontrolled data used in path expression High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 8 days ago
Use a strict, explicit path containment check in get_imat_image (and similarly in list_imat_images for consistency) by resolving both the base directory and target path and verifying the target is within the base via Path.relative_to(...). Reject absolute user paths early. This preserves existing behavior (valid relative paths under CEPH_DIR still work) while making the security check clear and analyzer-friendly.
Best concrete fix in plotting_service/routers/imat.py:
- In
list_imat_imagesandget_imat_image, replace direct(Path(CEPH_DIR) / path).resolve()+safe_check_filepath(...)block with:base_dir = Path(CEPH_DIR).resolve()requested = Path(path)- reject if
requested.is_absolute() resolved = (base_dir / requested).resolve()- enforce containment using
resolved.relative_to(base_dir)in atry/except ValueError
- Keep existing existence/type checks and response logic unchanged.
No new imports are needed (already using Path, HTTPException, HTTPStatus).
| @@ -101,11 +101,15 @@ | ||
| ) -> list[str]: | ||
| """Return a sorted list of TIFF images in the given directory.""" | ||
|
|
||
| dir_path = (Path(CEPH_DIR) / path).resolve() | ||
| # Security: Ensure path is within CEPH_DIR | ||
| base_dir = Path(CEPH_DIR).resolve() | ||
| requested_path = Path(path) | ||
| if requested_path.is_absolute(): | ||
| raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Directory not found") | ||
|
|
||
| dir_path = (base_dir / requested_path).resolve() | ||
| try: | ||
| safe_check_filepath(dir_path, CEPH_DIR) | ||
| except FileNotFoundError as err: | ||
| dir_path.relative_to(base_dir) | ||
| except ValueError as err: | ||
| raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Directory not found") from err | ||
|
|
||
| if not dir_path.exists() or not dir_path.is_dir(): | ||
| @@ -130,12 +133,16 @@ | ||
| ) -> Response: | ||
| """Return the raw data of a specific TIFF image as binary.""" | ||
|
|
||
| image_path = (Path(CEPH_DIR) / path).resolve() | ||
| # Security: Ensure path is within CEPH_DIR | ||
| base_dir = Path(CEPH_DIR).resolve() | ||
| requested_path = Path(path) | ||
| if requested_path.is_absolute(): | ||
| raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Image not found") | ||
|
|
||
| image_path = (base_dir / requested_path).resolve() | ||
| try: | ||
| safe_check_filepath(image_path, CEPH_DIR) | ||
| except FileNotFoundError as err: | ||
| raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Directory not found") from err | ||
| image_path.relative_to(base_dir) | ||
| except ValueError as err: | ||
| raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Image not found") from err | ||
|
|
||
| if not image_path.exists() or not image_path.is_file(): | ||
| raise HTTPException(status_code=HTTPStatus.NOT_FOUND, detail="Image not found") |
Closes None, it's the backend support for this issue: fiaisis/frontend#622
Description
2 endpoints added, to list, and return images using smart methodologies for compression.