Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion acestep/api/train_api_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,21 @@

try:
export_path = request.export_path.strip()
os.makedirs(os.path.dirname(export_path) if os.path.dirname(export_path) else ".", exist_ok=True)
# Constrain export_path to a safe base directory and prevent path traversal
safe_base = os.path.abspath("./exports")
# Strip leading separators to prevent os.path.join from treating input as absolute
export_path = export_path.lstrip("/").lstrip("\\")
export_path = os.path.abspath(os.path.join(safe_base, export_path))
if not export_path.startswith(safe_base + os.sep) and export_path != safe_base:
raise HTTPException(status_code=400, detail="Invalid export path")

os.makedirs(os.path.dirname(export_path), exist_ok=True)

Check failure

Code scanning / CodeQL

Uncontrolled data used in path expression High

This path depends on a
user-provided value
.

Copilot Autofix

AI 2 days ago

In general, to fix uncontrolled-path issues, normalize the path relative to a trusted base directory and then verify that the normalized path is still within that base. Use os.path.abspath/os.path.normpath or os.path.realpath, and compare using robust directory containment checks (e.g., os.path.commonpath) instead of plain string-prefix checks, which can be error‑prone (e.g., /exports2 vs /exports).

In this specific function (export_lora in acestep/api/train_api_service.py), we should: (1) keep treating ./exports as the only allowed root; (2) normalize the user-provided request.export_path relative to safe_base; (3) verify containment using os.path.commonpath([safe_base, export_path]) == safe_base; and (4) optionally handle the case where export_path is empty or only whitespace. After we have a validated export_path, we can safely call os.path.dirname(export_path) and the write operations (os.makedirs, shutil.copytree) without risking writes outside safe_base. We can implement this entirely within the shown snippet: no new imports are needed, as os is already imported, and we only adjust the logic around lines 175–183 to strengthen validation while leaving the external behavior (export under ./exports/...) the same.

Concretely:

  • In the try block of export_lora, replace the current sanitization code (lines 176–183) with a more robust version:
    • Strip and check that request.export_path is non-empty.
    • Strip leading slashes/backslashes.
    • Build export_path with os.path.join(safe_base, user_path) and normalize with os.path.abspath.
    • Use os.path.commonpath([safe_base, export_path]) to ensure export_path stays under safe_base.
  • Keep the rest of the logic (creating parent directory, removing existing tree, copying) unchanged.

This hardens the path validation and should eliminate the CodeQL alert because all sinks now consume a value that has passed a strict containment check.


Suggested changeset 1
acestep/api/train_api_service.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/acestep/api/train_api_service.py b/acestep/api/train_api_service.py
--- a/acestep/api/train_api_service.py
+++ b/acestep/api/train_api_service.py
@@ -173,20 +173,27 @@
             raise HTTPException(status_code=404, detail=f"No trained model found in {request.lora_output_dir}")
 
         try:
-            export_path = request.export_path.strip()
+            raw_export_path = (request.export_path or "").strip()
+            if not raw_export_path:
+                raise HTTPException(status_code=400, detail="Export path must not be empty")
+
             # Constrain export_path to a safe base directory and prevent path traversal
             safe_base = os.path.abspath("./exports")
             # Strip leading separators to prevent os.path.join from treating input as absolute
-            export_path = export_path.lstrip("/").lstrip("\\")
-            export_path = os.path.abspath(os.path.join(safe_base, export_path))
-            if not export_path.startswith(safe_base + os.sep) and export_path != safe_base:
+            sanitized_relative = raw_export_path.lstrip("/").lstrip("\\")
+            export_path = os.path.abspath(os.path.join(safe_base, sanitized_relative))
+
+            # Ensure the final path is within the safe base directory
+            if os.path.commonpath([safe_base, export_path]) != safe_base:
                 raise HTTPException(status_code=400, detail="Invalid export path")
 
             os.makedirs(os.path.dirname(export_path), exist_ok=True)
             if os.path.exists(export_path):
                 shutil.rmtree(export_path)
             shutil.copytree(source_path, export_path)
-            return wrap_response({"message": "LoRA exported successfully", "export_path": export_path, "source": source_path})
+            return wrap_response(
+                {"message": "LoRA exported successfully", "export_path": export_path, "source": source_path}
+            )
         except HTTPException:
             # Re-raise HTTP exceptions to preserve intended status codes
             raise
EOF
@@ -173,20 +173,27 @@
raise HTTPException(status_code=404, detail=f"No trained model found in {request.lora_output_dir}")

try:
export_path = request.export_path.strip()
raw_export_path = (request.export_path or "").strip()
if not raw_export_path:
raise HTTPException(status_code=400, detail="Export path must not be empty")

# Constrain export_path to a safe base directory and prevent path traversal
safe_base = os.path.abspath("./exports")
# Strip leading separators to prevent os.path.join from treating input as absolute
export_path = export_path.lstrip("/").lstrip("\\")
export_path = os.path.abspath(os.path.join(safe_base, export_path))
if not export_path.startswith(safe_base + os.sep) and export_path != safe_base:
sanitized_relative = raw_export_path.lstrip("/").lstrip("\\")
export_path = os.path.abspath(os.path.join(safe_base, sanitized_relative))

# Ensure the final path is within the safe base directory
if os.path.commonpath([safe_base, export_path]) != safe_base:
raise HTTPException(status_code=400, detail="Invalid export path")

os.makedirs(os.path.dirname(export_path), exist_ok=True)
if os.path.exists(export_path):
shutil.rmtree(export_path)
shutil.copytree(source_path, export_path)
return wrap_response({"message": "LoRA exported successfully", "export_path": export_path, "source": source_path})
return wrap_response(
{"message": "LoRA exported successfully", "export_path": export_path, "source": source_path}
)
except HTTPException:
# Re-raise HTTP exceptions to preserve intended status codes
raise
Copilot is powered by AI and may make mistakes. Always verify output.
if os.path.exists(export_path):
shutil.rmtree(export_path)
shutil.copytree(source_path, export_path)
return wrap_response({"message": "LoRA exported successfully", "export_path": export_path, "source": source_path})
except HTTPException:
# Re-raise HTTP exceptions to preserve intended status codes
raise
except Exception as exc:
return wrap_response(None, code=500, error=f"Export failed: {exc}")