From ec2b8ec7af0c5d3c48a7945f66a39d0a9bb61cc8 Mon Sep 17 00:00:00 2001 From: TPTBusiness Date: Fri, 17 Apr 2026 21:59:03 +0200 Subject: [PATCH] fix(security): replace relative_to() with realpath+startswith for CodeQL sanitization Path injection (#22, #28, #29, #30): - Switch from Path.relative_to() to os.path.realpath() + str.startswith() in all four path-validation sites across finetune and rl UI data_loader.py and finetune app.py. CodeQL recognizes realpath+startswith as a path- traversal sanitizer and clears taint on the resulting Path object. - Also simplify finetune/app.py: replace try/except relative_to block with the same realpath+startswith guard. Missing workflow permissions (#32, #33, #34, #35): - Add top-level permissions: contents: read to ci.yml, docs.yml, lint.yml, and security.yml. The docs deploy job already had pages: write and id-token: write set correctly on the job level. Co-Authored-By: Claude Sonnet 4.6 --- .github/workflows/ci.yml | 3 +++ .github/workflows/docs.yml | 3 +++ .github/workflows/lint.yml | 3 +++ .github/workflows/security.yml | 3 +++ rdagent/app/finetune/llm/ui/app.py | 13 ++++++------ rdagent/app/finetune/llm/ui/data_loader.py | 23 ++++++++++------------ rdagent/app/rl/ui/data_loader.py | 23 ++++++++++------------ 7 files changed, 39 insertions(+), 32 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 47199407..eba123a9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -17,6 +17,9 @@ on: env: PYTHONUNBUFFERED: "1" +permissions: + contents: read + jobs: test: name: Test (Python ${{ matrix.python-version }}, ${{ matrix.os }}) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 2106679a..a541f3de 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -15,6 +15,9 @@ on: - 'README.md' - '**/*.rst' +permissions: + contents: read + jobs: docs: name: Build Documentation diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index bc55cdec..c741291f 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -6,6 +6,9 @@ on: pull_request: branches: [ main ] +permissions: + contents: read + jobs: lint: name: Lint & Format diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 79663897..7b79f6f4 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -9,6 +9,9 @@ on: # Weekly on Monday at 6:00 UTC - cron: '0 6 * * 1' +permissions: + contents: read + jobs: security: name: Security Analysis diff --git a/rdagent/app/finetune/llm/ui/app.py b/rdagent/app/finetune/llm/ui/app.py index 74d30179..9a8728fe 100644 --- a/rdagent/app/finetune/llm/ui/app.py +++ b/rdagent/app/finetune/llm/ui/app.py @@ -142,13 +142,14 @@ def main(): base_folder = st.text_input("Base Folder", value=default_log, key="base_folder_input") # Normalize and validate the base folder against the configured log root - safe_root = Path(default_log).expanduser().resolve() - try: - base_path = Path(base_folder).expanduser().resolve() - # Ensure the user-selected base path is within the safe root - base_path.relative_to(safe_root) - except (OSError, ValueError): + root_real = os.path.realpath(str(Path(default_log).expanduser())) + folder_real = os.path.realpath(str(Path(base_folder).expanduser())) + if folder_real == root_real or folder_real.startswith(root_real + os.sep): + base_path = Path(folder_real) + safe_root = Path(root_real) + else: st.error("Invalid base folder: must be within the configured log directory.") + safe_root = Path(root_real) base_path = safe_root # base_path is validated against safe_root – nosec B614 diff --git a/rdagent/app/finetune/llm/ui/data_loader.py b/rdagent/app/finetune/llm/ui/data_loader.py index ee2a0311..cb6b4778 100644 --- a/rdagent/app/finetune/llm/ui/data_loader.py +++ b/rdagent/app/finetune/llm/ui/data_loader.py @@ -3,6 +3,7 @@ FT UI Data Loader Load pkl logs and convert to hierarchical timeline structure """ +import os import re from dataclasses import dataclass, field from datetime import datetime @@ -88,13 +89,11 @@ def extract_stage(tag: str) -> str: def get_valid_sessions(log_folder: Path, safe_root: Path | None = None) -> list[str]: """Get list of valid session directories, optionally validating against a safe root.""" if safe_root is not None: - try: - resolved_root = safe_root.expanduser().resolve() - resolved_folder = log_folder.expanduser().resolve() - # Reconstruct from the trusted root so file ops use a root-derived path. - log_folder = resolved_root / resolved_folder.relative_to(resolved_root) - except ValueError: + root_real = os.path.realpath(str(safe_root.expanduser())) + folder_real = os.path.realpath(str(log_folder.expanduser())) + if not (folder_real == root_real or folder_real.startswith(root_real + os.sep)): return [] + log_folder = Path(folder_real) if not log_folder.exists(): return [] @@ -376,13 +375,11 @@ def load_ft_session(log_path: Path, safe_root: Path | None = None) -> Session: """Load events into hierarchical session structure, optionally validating against safe root.""" # Validate path is within safe_root if provided if safe_root is not None: - try: - resolved_root = safe_root.expanduser().resolve() - resolved_path = log_path.expanduser().resolve() - # Reconstruct from the trusted root so file ops use a root-derived path. - log_path = resolved_root / resolved_path.relative_to(resolved_root) - except ValueError: - return Session() # Return empty session if path is outside allowed root + root_real = os.path.realpath(str(safe_root.expanduser())) + path_real = os.path.realpath(str(log_path.expanduser())) + if not (path_real == root_real or path_real.startswith(root_real + os.sep)): + return Session() + log_path = Path(path_real) session = Session() storage = FileStorage(log_path) diff --git a/rdagent/app/rl/ui/data_loader.py b/rdagent/app/rl/ui/data_loader.py index e9138599..369395fb 100644 --- a/rdagent/app/rl/ui/data_loader.py +++ b/rdagent/app/rl/ui/data_loader.py @@ -4,6 +4,7 @@ Load pkl logs and convert to hierarchical timeline structure Simplified version: no EvoLoop (RL doesn't have evolution loops) """ +import os import pickle import re from dataclasses import dataclass, field @@ -75,13 +76,11 @@ def extract_stage(tag: str) -> str: def get_valid_sessions(log_folder: Path, safe_root: Path | None = None) -> list[str]: """Get list of valid session directories, optionally validating against a safe root.""" if safe_root is not None: - try: - resolved_root = safe_root.expanduser().resolve() - resolved_folder = log_folder.expanduser().resolve() - # Reconstruct from the trusted root so file ops use a root-derived path. - log_folder = resolved_root / resolved_folder.relative_to(resolved_root) - except ValueError: + root_real = os.path.realpath(str(safe_root.expanduser())) + folder_real = os.path.realpath(str(log_folder.expanduser())) + if not (folder_real == root_real or folder_real.startswith(root_real + os.sep)): return [] + log_folder = Path(folder_real) if not log_folder.exists(): return [] @@ -248,13 +247,11 @@ def load_session(log_path: Path, safe_root: Path | None = None) -> Session: """Load events into hierarchical session structure, optionally validating against safe root.""" # Validate path is within safe_root if provided if safe_root is not None: - try: - resolved_root = safe_root.expanduser().resolve() - resolved_path = log_path.expanduser().resolve() - # Reconstruct from the trusted root so file ops use a root-derived path. - log_path = resolved_root / resolved_path.relative_to(resolved_root) - except ValueError: - return Session() # Return empty session if path is outside allowed root + root_real = os.path.realpath(str(safe_root.expanduser())) + path_real = os.path.realpath(str(log_path.expanduser())) + if not (path_real == root_real or path_real.startswith(root_real + os.sep)): + return Session() + log_path = Path(path_real) session = Session()