-
Notifications
You must be signed in to change notification settings - Fork 44
fix: prevent path traversal via task_id in cleaning task log/delete e… #508
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 7 commits
fa71c86
d16bf0b
4105654
0e51ab7
d8c0005
23e7799
fce6886
867097f
92b66b0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,5 @@ | ||
| import json | ||
| import os | ||
| import re | ||
| import shutil | ||
| import uuid | ||
|
|
@@ -380,13 +381,20 @@ | |
| self, db: AsyncSession, task_id: str, retry_count: int | ||
| ) -> List[CleaningTaskLog]: | ||
| """Get task log""" | ||
| self.validator.check_task_id(task_id) | ||
| safe_task_id = self.validator.sanitize_task_id(task_id) | ||
|
|
||
| log_path = Path(f"{FLOW_PATH}/{task_id}/output.log") | ||
| flow_root = Path(FLOW_PATH).resolve() | ||
| log_path = flow_root / safe_task_id / "output.log" | ||
| if retry_count > 0: | ||
| log_path = Path(f"{FLOW_PATH}/{task_id}/output.log.{retry_count}") | ||
| log_path = flow_root / safe_task_id / f"output.log.{retry_count}" | ||
|
|
||
| if not log_path.exists(): | ||
| # 防止路径穿越:规范化后校验仍在 FLOW_PATH 下 | ||
| resolved_log_path = log_path.resolve() | ||
Check failureCode scanning / CodeQL Uncontrolled data used in path expression High
This path depends on a
user-provided value Error loading related location Loading This path depends on a user-provided value. |
||
|
github-advanced-security[bot] marked this conversation as resolved.
Fixed
MoeexT marked this conversation as resolved.
Dismissed
|
||
| if flow_root not in resolved_log_path.parents: | ||
| logger.warning(f"Path traversal attempt detected: task_id={task_id}") | ||
| return [] | ||
|
|
||
| if not resolved_log_path.exists(): | ||
Check failureCode scanning / CodeQL Uncontrolled data used in path expression High
This path depends on a
user-provided value Error loading related location Loading This path depends on a user-provided value. |
||
|
MoeexT marked this conversation as resolved.
Dismissed
|
||
| return [] | ||
|
|
||
| logs = [] | ||
|
|
@@ -397,7 +405,7 @@ | |
| ) | ||
| exception_suffix_pattern = re.compile(r"\b\w+(Warning|Error|Exception)\b") | ||
|
|
||
| with open(log_path, "r", encoding="utf-8") as f: | ||
| with open(resolved_log_path, "r", encoding="utf-8") as f: | ||
Check failureCode scanning / CodeQL Uncontrolled data used in path expression High
This path depends on a
user-provided value Error loading related location Loading This path depends on a user-provided value. |
||
|
MoeexT marked this conversation as resolved.
Dismissed
|
||
| for line in f: | ||
| last_level = self._get_log_level( | ||
| line, last_level, standard_level_pattern, exception_suffix_pattern | ||
|
|
@@ -429,9 +437,9 @@ | |
|
|
||
| async def delete_task(self, db: AsyncSession, task_id: str) -> None: | ||
| """Delete task""" | ||
| self.validator.check_task_id(task_id) | ||
| safe_task_id = self.validator.sanitize_task_id(task_id) | ||
|
|
||
| task = await self.task_repo.find_task_by_id(db, task_id) | ||
| task = await self.task_repo.find_task_by_id(db, safe_task_id) | ||
| if not task: | ||
| raise BusinessError(ErrorCodes.CLEANING_TASK_NOT_FOUND, task_id) | ||
|
|
||
|
|
@@ -442,12 +450,21 @@ | |
| "Task is running, cannot be deleted. Please stop the task first." | ||
| ) | ||
|
|
||
| await self.task_repo.delete_task_by_id(db, task_id) | ||
| await self.operator_instance_repo.delete_by_instance_id(db, task_id) | ||
| await self.result_repo.delete_by_instance_id(db, task_id) | ||
| await self.task_repo.delete_task_by_id(db, safe_task_id) | ||
| await self.operator_instance_repo.delete_by_instance_id(db, safe_task_id) | ||
| await self.result_repo.delete_by_instance_id(db, safe_task_id) | ||
|
|
||
| # 删除任务相关文件 | ||
| task_path = Path(f"{FLOW_PATH}/{task_id}") | ||
| flow_root = Path(FLOW_PATH).resolve() | ||
| task_path = (flow_root / safe_task_id).resolve() | ||
|
MoeexT marked this conversation as resolved.
Dismissed
|
||
| # 防止路径穿越:parents 校验目标路径仍在 flow_root 下 | ||
| if flow_root not in task_path.parents: | ||
| logger.warning(f"Path traversal attempt in delete_task: task_id={task_id}") | ||
| raise BusinessError( | ||
| ErrorCodes.CLEANING_TASK_NOT_FOUND, | ||
| f"Invalid task_id: {task_id}", | ||
| ) | ||
|
|
||
| if task_path.exists(): | ||
| try: | ||
| shutil.rmtree(task_path) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.