From 6b6d336123c96326e64a94a6df5632117dc8832b Mon Sep 17 00:00:00 2001 From: chloecrozier Date: Tue, 4 Aug 2026 10:57:57 -0700 Subject: [PATCH 1/2] fix: prevent path traversal writes on /documents (NVBug 6553697) Sanitize collection_name/filename and confine uploads to INGESTOR_UPLOAD_ROOT so unauthenticated POST/PATCH /documents cannot write arbitrary files. --- community/ai-vws-sizing-advisor/CHANGELOG.md | 4 + community/ai-vws-sizing-advisor/README.md | 9 ++ .../src/ingestor_server/path_security.py | 94 +++++++++++++++++++ .../src/ingestor_server/server.py | 53 +++++++++-- community/ai-vws-sizing-advisor/src/utils.py | 13 ++- 5 files changed, 162 insertions(+), 11 deletions(-) create mode 100644 community/ai-vws-sizing-advisor/src/ingestor_server/path_security.py diff --git a/community/ai-vws-sizing-advisor/CHANGELOG.md b/community/ai-vws-sizing-advisor/CHANGELOG.md index 9fae02c9a..ecbb298f6 100644 --- a/community/ai-vws-sizing-advisor/CHANGELOG.md +++ b/community/ai-vws-sizing-advisor/CHANGELOG.md @@ -2,6 +2,10 @@ All notable changes to this project will be documented in this file. The format is based on Keep a Changelog, and this project adheres to Semantic Versioning. +## [2.5] - 2026-08-04 + +### Security +- **Path traversal fix (NVBug 6553697)** — POST/PATCH `/documents` no longer allows arbitrary filesystem writes via malicious `collection_name` or filename values. Uploads are confined to `INGESTOR_UPLOAD_ROOT` with strict collection-name validation and resolved-path checks. ## [2.3] - 2026-01-08 diff --git a/community/ai-vws-sizing-advisor/README.md b/community/ai-vws-sizing-advisor/README.md index b12830350..e0be94d36 100644 --- a/community/ai-vws-sizing-advisor/README.md +++ b/community/ai-vws-sizing-advisor/README.md @@ -193,6 +193,15 @@ curl -X POST -F "file=@./vgpu_docs/your-document.pdf" http://localhost:8082/v1/i --- +## Security Notes + +This is a local development / demo example. Do not expose the ingestor (`:8082`) or RAG APIs to untrusted networks. + +- Document uploads are confined to `INGESTOR_UPLOAD_ROOT` (default `/tmp-data/uploaded_files`). +- `collection_name` values are validated as single path segments; path-traversal sequences are rejected (NVBug 6553697). + +--- + ## License Licensed under the Apache License, Version 2.0. diff --git a/community/ai-vws-sizing-advisor/src/ingestor_server/path_security.py b/community/ai-vws-sizing-advisor/src/ingestor_server/path_security.py new file mode 100644 index 000000000..9a2b09d0e --- /dev/null +++ b/community/ai-vws-sizing-advisor/src/ingestor_server/path_security.py @@ -0,0 +1,94 @@ +# SPDX-FileCopyrightText: Copyright (c) 2025 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +# SPDX-License-Identifier: Apache-2.0 +"""Path validation helpers to prevent path-traversal on document uploads.""" + +from __future__ import annotations + +import os +import re +from pathlib import Path + +# Upload root for ingested files. Never write outside this directory. +UPLOAD_ROOT = Path(os.getenv("INGESTOR_UPLOAD_ROOT", "/tmp-data/uploaded_files")).resolve() + +# Collection names are used as directory segments — keep them strict. +_SAFE_COLLECTION_RE = re.compile(r"^[A-Za-z0-9_][A-Za-z0-9._-]{0,254}$") + + +class UnsafePathError(ValueError): + """Raised when a user-supplied path component fails validation.""" + + +def validate_safe_collection_name(name: str | None, field: str = "collection_name") -> str: + """Reject empty, traversal, and separator-bearing collection names.""" + if name is None: + raise UnsafePathError(f"Invalid {field}: missing value") + + if "\x00" in name or "/" in name or "\\" in name: + raise UnsafePathError(f"Invalid {field}: path separators not allowed") + + candidate = os.path.basename(name.strip()) + if not candidate or candidate in {".", ".."} or not _SAFE_COLLECTION_RE.fullmatch(candidate): + raise UnsafePathError( + f"Invalid {field}: must be 1-255 characters, start with a letter, digit, or '_', " + "and contain only letters, digits, '.', '_' or '-'" + ) + return candidate + + +def validate_safe_filename(name: str | None, field: str = "filename") -> str: + """ + Sanitize an upload filename to a single path segment. + + Allows common document name characters (spaces, parentheses, etc.) but + rejects traversal sequences and path separators. + """ + if name is None: + raise UnsafePathError(f"Invalid {field}: missing value") + + if "\x00" in name: + raise UnsafePathError(f"Invalid {field}: null bytes not allowed") + + candidate = os.path.basename(name.strip()) + if not candidate or candidate in {".", ".."}: + raise UnsafePathError(f"Invalid {field}: empty or traversal name") + + # Defense in depth if basename behavior differs across platforms + if "/" in candidate or "\\" in candidate: + raise UnsafePathError(f"Invalid {field}: path separators not allowed") + + if len(candidate) > 255: + raise UnsafePathError(f"Invalid {field}: exceeds 255 characters") + + return candidate + + +def validate_safe_name(name: str | None, field: str = "name") -> str: + """Validate a collection-like or filename identifier based on field name.""" + if field in {"filename", "document_name"}: + return validate_safe_filename(name, field) + return validate_safe_collection_name(name, field) + + +def safe_collection_dir(collection_name: str) -> Path: + """Return a directory under UPLOAD_ROOT for the given collection.""" + safe_collection = validate_safe_collection_name(collection_name, "collection_name") + collection_dir = (UPLOAD_ROOT / safe_collection).resolve() + if not collection_dir.is_relative_to(UPLOAD_ROOT): + raise UnsafePathError("Invalid collection_name: path traversal detected") + return collection_dir + + +def safe_upload_file_path(collection_name: str, filename: str | None) -> tuple[Path, str]: + """ + Build a write destination confined to UPLOAD_ROOT//. + + Returns: + (absolute_file_path, sanitized_filename) + """ + collection_dir = safe_collection_dir(collection_name) + safe_filename = validate_safe_filename(filename, "filename") + file_path = (collection_dir / safe_filename).resolve() + if not file_path.is_relative_to(UPLOAD_ROOT): + raise UnsafePathError("Invalid filename: path traversal detected") + return file_path, safe_filename diff --git a/community/ai-vws-sizing-advisor/src/ingestor_server/server.py b/community/ai-vws-sizing-advisor/src/ingestor_server/server.py index 87687f2eb..189025736 100644 --- a/community/ai-vws-sizing-advisor/src/ingestor_server/server.py +++ b/community/ai-vws-sizing-advisor/src/ingestor_server/server.py @@ -20,7 +20,6 @@ import shutil from inspect import getmembers from inspect import isclass -from pathlib import Path from typing import List, Dict, Any, Union from uuid import uuid4 @@ -38,6 +37,12 @@ from src.utils import get_config from .main import NVIngestIngestor from .ingestion_task_handler import INGESTION_TASK_HANDLER +from .path_security import ( + UnsafePathError, + safe_collection_dir, + safe_upload_file_path, + validate_safe_name, +) logging.basicConfig(level=os.environ.get('LOGLEVEL', 'INFO').upper()) logger = logging.getLogger(__name__) @@ -102,7 +107,8 @@ class DocumentUploadRequest(BaseModel): collection_name: str = Field( "multimodal_data", - description="Name of the collection in the vector database." + description="Name of the collection in the vector database.", + pattern=r"^[A-Za-z0-9_][A-Za-z0-9._-]{0,254}$", ) blocking: bool = Field( @@ -289,11 +295,13 @@ async def upload_document(documents: List[UploadFile] = File(...), temp_dirs = [] try: - base_upload_folder = Path(f"/tmp-data/uploaded_files/{request.collection_name}") + # Confine writes to UPLOAD_ROOT// (blocks path traversal via collection_name) + base_upload_folder = safe_collection_dir(request.collection_name) base_upload_folder.mkdir(parents=True, exist_ok=True) for file in documents: - upload_file = os.path.basename(file.filename) + # Sanitize filename and resolve destination under the collection directory + file_path, upload_file = safe_upload_file_path(request.collection_name, file.filename) # Check for unsupported file formats (.rst, .rtf, etc.) not_supported_formats = ('.rst', '.rtf', '.org') @@ -310,15 +318,11 @@ async def upload_document(documents: List[UploadFile] = File(...), logger.info(dockerfile_instructions) raise Exception(f"File format for {upload_file} is not supported.") - if not upload_file: - raise RuntimeError("Error parsing uploaded filename.") - # Create a unique directory for each file unique_dir = base_upload_folder #/ str(uuid4()) unique_dir.mkdir(parents=True, exist_ok=True) temp_dirs.append(unique_dir) - file_path = unique_dir / upload_file if not (hasattr(NV_INGEST_INGESTOR, "get_documents") and callable(NV_INGEST_INGESTOR.get_documents)): raise NotImplementedError("Example class has not implemented get_documents method.") @@ -364,6 +368,10 @@ async def upload_document(documents: List[UploadFile] = File(...), except asyncio.CancelledError as e: logger.warning(f"Request cancelled while uploading document {e}") return JSONResponse(content={"message": "Request was cancelled by the client"}, status_code=499) + except UnsafePathError as e: + raise HTTPException(status_code=400, detail=str(e)) from e + except HTTPException: + raise except Exception as e: logger.error(f"Error from POST /documents endpoint. Ingestion of file failed with error: {e}") return JSONResponse(content={"message": f"Ingestion of files failed with error: {e}"}, status_code=500) @@ -444,8 +452,10 @@ async def delete_and_upload_document(documents: List[UploadFile] = File(...), """Upload a document to the vector store. If the document already exists, it will be replaced.""" try: + # Validate collection_name before any filesystem / VDB operations + validate_safe_name(request.collection_name, "collection_name") for file in documents: - file_name = os.path.basename(file.filename) + file_name = validate_safe_name(file.filename, "filename") # Delete the existing document if not (hasattr(NV_INGEST_INGESTOR, "delete_documents") and callable(NV_INGEST_INGESTOR.delete_documents)): @@ -462,6 +472,10 @@ async def delete_and_upload_document(documents: List[UploadFile] = File(...), except asyncio.CancelledError as e: logger.error(f"Request cancelled while deleting and uploading document") return JSONResponse(content={"message": "Request was cancelled by the client"}, status_code=499) + except UnsafePathError as e: + raise HTTPException(status_code=400, detail=str(e)) from e + except HTTPException: + raise except Exception as e: logger.error("Error from PATCH /documents endpoint. Ingestion failed with error.") return JSONResponse(content={"message": f"Ingestion of files failed with error. {e}"}, status_code=500) @@ -501,6 +515,7 @@ async def get_documents( ) -> DocumentListResponse: """Get list of document ingested in vectorstore.""" try: + collection_name = validate_safe_name(collection_name, "collection_name") if hasattr(NV_INGEST_INGESTOR, "get_documents") and callable(NV_INGEST_INGESTOR.get_documents): documents = NV_INGEST_INGESTOR.get_documents(collection_name, vdb_endpoint) return DocumentListResponse(**documents) @@ -509,6 +524,10 @@ async def get_documents( except asyncio.CancelledError as e: logger.warning(f"Request cancelled while fetching documents. {str(e)}") return JSONResponse(content={"message": "Request was cancelled by the client."}, status_code=499) + except UnsafePathError as e: + raise HTTPException(status_code=400, detail=str(e)) from e + except HTTPException: + raise except Exception as e: logger.error("Error from GET /documents endpoint. Error details: %s", e) return JSONResponse(content={"message": f"Error occurred while fetching documents: {e}"}, status_code=500) @@ -544,6 +563,8 @@ async def get_documents( async def delete_documents(_: Request, document_names: List[str] = [], collection_name: str = os.getenv("COLLECTION_NAME"), vdb_endpoint: str = Query(default=os.getenv("APP_VECTORSTORE_URL"), include_in_schema=False)) -> DocumentListResponse: """Delete a document from vectorstore.""" try: + collection_name = validate_safe_name(collection_name, "collection_name") + document_names = [validate_safe_name(name, "document_name") for name in document_names] if hasattr(NV_INGEST_INGESTOR, "delete_documents") and callable(NV_INGEST_INGESTOR.delete_documents): response = NV_INGEST_INGESTOR.delete_documents(document_names=document_names, document_ids=[], collection_name=collection_name, vdb_endpoint=vdb_endpoint) return DocumentListResponse(**response) @@ -553,6 +574,10 @@ async def delete_documents(_: Request, document_names: List[str] = [], collectio except asyncio.CancelledError as e: logger.warning(f"Request cancelled while deleting document:, {document_names}, {str(e)}") return JSONResponse(content={"message": "Request was cancelled by the client."}, status_code=499) + except UnsafePathError as e: + raise HTTPException(status_code=400, detail=str(e)) from e + except HTTPException: + raise except Exception as e: logger.error("Error from DELETE /documents endpoint. Error details: %s", e) return JSONResponse(content={"message": f"Error deleting document {document_names}: {e}"}, status_code=500) @@ -642,6 +667,7 @@ async def create_collections( Returns status message. """ try: + collection_names = [validate_safe_name(name, "collection_name") for name in collection_names] if hasattr(NV_INGEST_INGESTOR, "create_collections") and callable(NV_INGEST_INGESTOR.create_collections): response = NV_INGEST_INGESTOR.create_collections(collection_names, vdb_endpoint, embedding_dimension, collection_type) return CollectionResponse(**response) @@ -650,6 +676,10 @@ async def create_collections( except asyncio.CancelledError as e: logger.warning(f"Request cancelled while fetching collections. {str(e)}") return JSONResponse(content={"message": "Request was cancelled by the client."}, status_code=499) + except UnsafePathError as e: + raise HTTPException(status_code=400, detail=str(e)) from e + except HTTPException: + raise except Exception as e: logger.error("Error from POST /collections endpoint. Error details: %s", e) return JSONResponse(content={"message": f"Error occurred while creating collections. Error: {e}"}, status_code=500) @@ -688,6 +718,7 @@ async def delete_collections(vdb_endpoint: str = Query(default=os.getenv("APP_VE Returns status message. """ try: + collection_names = [validate_safe_name(name, "collection_name") for name in collection_names] if hasattr(NV_INGEST_INGESTOR, "delete_collections") and callable(NV_INGEST_INGESTOR.delete_collections): response = NV_INGEST_INGESTOR.delete_collections(collection_names, vdb_endpoint) return CollectionResponse(**response) @@ -696,6 +727,10 @@ async def delete_collections(vdb_endpoint: str = Query(default=os.getenv("APP_VE except asyncio.CancelledError as e: logger.warning(f"Request cancelled while fetching collections. {str(e)}") return JSONResponse(content={"message": "Request was cancelled by the client."}, status_code=499) + except UnsafePathError as e: + raise HTTPException(status_code=400, detail=str(e)) from e + except HTTPException: + raise except Exception as e: logger.error("Error from DELETE /collections endpoint. Error details: %s", e) return JSONResponse(content={"message": f"Error occurred while deleting collections. Error: {e}"}, status_code=500) diff --git a/community/ai-vws-sizing-advisor/src/utils.py b/community/ai-vws-sizing-advisor/src/utils.py index c3bcaff55..3d864d129 100644 --- a/community/ai-vws-sizing-advisor/src/utils.py +++ b/community/ai-vws-sizing-advisor/src/utils.py @@ -604,11 +604,20 @@ def del_docs_vectorstore_langchain(vectorstore: VectorStore, filenames: List[str """Delete documents from the vector index implemented in LangChain.""" settings = get_config() - upload_folder = f"/tmp-data/uploaded_files/{collection_name}" + # Keep collection segment as a single path component to avoid traversal in source metadata keys + safe_collection = os.path.basename(collection_name or "") + if not safe_collection or safe_collection in {".", ".."} or "/" in collection_name or "\\" in collection_name: + logger.error("Invalid collection_name for document deletion: %s", collection_name) + return False + upload_folder = f"/tmp-data/uploaded_files/{safe_collection}" deleted = False try: for filename in filenames: - source_value = os.path.join(upload_folder, filename) + safe_filename = os.path.basename(filename) + if not safe_filename or safe_filename in {".", ".."}: + logger.error("Invalid filename for document deletion: %s", filename) + return False + source_value = os.path.join(upload_folder, safe_filename) if settings.vector_store.name == "milvus": # Delete Milvus Entities resp = vectorstore.col.delete(f"source['source_name'] == '{source_value}'") From 60d7cbf1c46bd3d2d5e5cae7bf3daa5e275196f4 Mon Sep 17 00:00:00 2001 From: Chloe Crozier <40836363+chloecrozier@users.noreply.github.com> Date: Tue, 4 Aug 2026 11:28:15 -0700 Subject: [PATCH 2/2] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- community/ai-vws-sizing-advisor/src/utils.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/community/ai-vws-sizing-advisor/src/utils.py b/community/ai-vws-sizing-advisor/src/utils.py index 3d864d129..5860badfe 100644 --- a/community/ai-vws-sizing-advisor/src/utils.py +++ b/community/ai-vws-sizing-advisor/src/utils.py @@ -605,8 +605,9 @@ def del_docs_vectorstore_langchain(vectorstore: VectorStore, filenames: List[str settings = get_config() # Keep collection segment as a single path component to avoid traversal in source metadata keys - safe_collection = os.path.basename(collection_name or "") - if not safe_collection or safe_collection in {".", ".."} or "/" in collection_name or "\\" in collection_name: + raw_collection = (collection_name or "").strip() + safe_collection = os.path.basename(raw_collection) + if not safe_collection or safe_collection in {".", ".."} or "/" in raw_collection or "\\" in raw_collection: logger.error("Invalid collection_name for document deletion: %s", collection_name) return False upload_folder = f"/tmp-data/uploaded_files/{safe_collection}"