fix: harden execution and archive handling (#1469)

* fix: harden execution and archive handling

* style: sort security fix imports

* fix: contain workspace file operations

* docs: document execution security boundaries

* fix: enforce JSON grading output contract

* fix: close security hardening regressions

Author: XianBW

docs/development.rst

@@ -83,3 +83,56 @@ File Naming Convention
      - The configuration for the module, app, and project.
 
 .. <!-- TODO: renaming files -->
+
+
+Security boundaries for generated code and artifacts
+====================================================
+
+RD-Agent executes generated code and processes files produced by external
+tools. Treat filenames, archive members, subprocess output, competition names,
+and environment names as untrusted input even when the surrounding workflow is
+started by a trusted operator.
+
+Workspace file operations
+-------------------------
+
+Use :meth:`FBWorkspace.inject_files <rdagent.core.experiment.FBWorkspace.inject_files>`
+and :meth:`FBWorkspace.remove_files <rdagent.core.experiment.FBWorkspace.remove_files>`
+for files owned by an experiment workspace. These methods accept relative paths
+inside ``workspace_path``. Parent traversal, absolute paths, and paths that
+escape through a symbolic link are rejected before a file is written or
+deleted.
+
+Do not construct an unchecked path from a generated filename and then call
+``write_text()``, ``unlink()``, or another filesystem operation directly. New
+workspace APIs should apply the same resolved-path containment rule to every
+write and delete branch.
+
+Archive extraction
+------------------
+
+Use ``rdagent.utils.archive.safe_extract_zip`` and
+``rdagent.utils.archive.safe_extract_tar`` for archives that may contain
+externally supplied entries. The helpers reject:
+
+* absolute and parent-traversal member paths;
+* symbolic links and hard links;
+* device nodes and other special file types; and
+* archives containing more than 10,000 members by default.
+
+Avoid ``ZipFile.extractall()`` and ``TarFile.extractall()`` in these paths. If a
+workflow legitimately needs links, special files, or a larger archive, handle
+that input in a separately reviewed trusted-data path rather than weakening the
+shared helpers.
+
+Commands and parsed process output
+----------------------------------
+
+Pass subprocess arguments as a list with ``shell=False`` whenever any argument
+can vary. Conda environment names are limited to letters, digits, ``_``, ``-``,
+and ``.``, and Python versions must be numeric dotted versions. Kaggle
+competition identifiers use the corresponding competition-slug validator.
+
+Never use ``eval()`` to parse subprocess or container output. Score output must
+be finite numeric JSON. Legacy Python dictionary-shaped training metrics may be
+parsed with ``ast.literal_eval()`` only when JSON cannot be used.

rdagent/components/coder/finetune/unified_validator.py

@@ -6,6 +6,7 @@
 2. Micro-batch testing - Runtime validation with small dataset
 """
 
+import ast
 import json
 import re
 import time
@@ -226,10 +227,10 @@ def _parse_execution_log(self, stdout: str, exit_code: int, failed_stage: str =
                 result["num_epochs"] = int(num_epochs.group(1).replace(",", ""))
 
             # Extract final metrics (JSON format from trainer output)
-            final_metrics = re.search(r"\{'train_runtime':[^}]+\}", stdout)
+            final_metrics = re.search(r"\{[\"']train_runtime[\"']:[^}]+\}", stdout)
             if final_metrics:
                 try:
-                    metrics = eval(final_metrics.group(0))  # Safe: only numbers and strings
+                    metrics = ast.literal_eval(final_metrics.group(0))
                     result["final_metrics"] = {
                         "train_loss": metrics.get("train_loss"),
                         "train_runtime": metrics.get("train_runtime"),

rdagent/core/experiment.py

@@ -220,6 +220,27 @@ def link_all_files_in_folder_to_workspace(data_path: Path, workspace_path: Path)
 
     DEL_KEY = "__DEL__"
 
+    def _resolve_workspace_path(self, file_name: str, *, follow_leaf_symlink: bool = True) -> Path:
+        """Resolve a caller-provided file name without allowing workspace escape."""
+        relative_path = Path(file_name)
+        if relative_path.is_absolute() or ".." in relative_path.parts:
+            message = f"File path must be relative and contained in the workspace: {file_name}"
+            raise ValueError(message)
+
+        workspace_root = self.workspace_path.resolve()
+        target_file_path = workspace_root / relative_path
+        resolved_path = (
+            target_file_path.resolve()
+            if follow_leaf_symlink
+            else target_file_path.parent.resolve() / target_file_path.name
+        )
+        try:
+            resolved_path.relative_to(workspace_root)
+        except ValueError as exc:
+            message = f"File path escapes workspace: {file_name}"
+            raise ValueError(message) from exc
+        return resolved_path if follow_leaf_symlink else target_file_path
+
     def inject_files(self, **files: str) -> None:
         """
         Inject the code into the folder.
@@ -232,12 +253,13 @@ def inject_files(self, **files: str) -> None:
         """
         self.prepare()
         for k, v in files.items():
-            target_file_path = self.workspace_path / k  # Define target_file_path before using it
             if v == self.DEL_KEY:  # Use self.DEL_KEY to access the class variable
-                if target_file_path.exists():
+                target_file_path = self._resolve_workspace_path(k, follow_leaf_symlink=False)
+                if target_file_path.exists() or target_file_path.is_symlink():
                     target_file_path.unlink()  # Unlink the file if it exists
                 self.file_dict.pop(k, None)  # Safely remove the key from file_dict
             else:
+                target_file_path = self._resolve_workspace_path(k)
                 self.file_dict[k] = v
                 target_file_path.parent.mkdir(parents=True, exist_ok=True)
                 target_file_path.write_text(v)
@@ -249,8 +271,8 @@ def remove_files(self, file_names: str | list[str]) -> None:
         if isinstance(file_names, str):
             file_names = [file_names]
         for file_name in file_names:
-            target_file_path = self.workspace_path / file_name
-            if target_file_path.exists():
+            target_file_path = self._resolve_workspace_path(file_name, follow_leaf_symlink=False)
+            if target_file_path.exists() or target_file_path.is_symlink():
                 target_file_path.unlink()  # Unlink the file if it exists
             self.file_dict.pop(file_name, None)  # Safely remove the key from file_dict
 

rdagent/scenarios/data_science/proposal/exp_gen/select/prompts.yaml

@@ -107,7 +107,10 @@ grade:
     Write a Python script named `grade.py` to evaluate `submission.csv` produced by a model.
     Input files (relative to current working directory):
     - `{{ input_folder }}/label.csv` and `submission.csv`
-    Output format: `{'score': float, 'metric': str}`
+    Output exactly one valid JSON object on one line, for example:
+    `{"score": 0.75, "metric": "auc"}`
+    Use `json.dumps(...)` to produce the output. Do not print a Python dictionary representation.
+    `score` must be a finite JSON number, not a string, boolean, NaN, or Infinity.
     {% if error %} 
     {{ error }}
     {% endif %}

rdagent/scenarios/data_science/proposal/exp_gen/select/submit.py

@@ -1,9 +1,9 @@
 import json
+import math
 import os
 import pickle
 import re
 import shutil
-import tarfile
 import time
 from pathlib import Path
 from typing import Any, Dict, List, Optional, Tuple
@@ -26,6 +26,7 @@
 from rdagent.scenarios.data_science.experiment.experiment import DSExperiment
 from rdagent.utils.agent.ret import PythonAgentOut
 from rdagent.utils.agent.tpl import T
+from rdagent.utils.archive import safe_extract_tar
 from rdagent.utils.fmt import shrink_text
 from rdagent.utils.workflow import wait_retry
 
@@ -350,6 +351,34 @@ def print_code(self, data_py_code: str, grade_py_code: str):
         print(grade_py_code)
         print("======== code end ========")
 
+    def _validate_grade_script(self, grade_py_code: str, reference_exp: DSExperiment, mock_folder: str) -> None:
+        """Run a reusable grade script and verify its output contract."""
+        input_folder = T("scenarios.data_science.share:scen.input_path").r()
+        submission_path = Path(mock_folder) / "submission.csv"
+        if not submission_path.exists():
+            message = f"Cannot validate grade.py because {submission_path} does not exist."
+            raise RuntimeError(message)
+
+        ws = FBWorkspace()
+        ws.inject_code_from_file_dict(reference_exp.experiment_workspace)
+        ws.inject_files(**{"grade.py": grade_py_code})
+        shutil.copy(str(submission_path), str(ws.workspace_path / "submission.csv"))
+        env = get_ds_env(extra_volumes={str(Path(mock_folder) / input_folder): {"bind": input_folder, "mode": "rw"}})
+        result = ws.run(env=env, entry=f"python grade.py --cache-buster={time.time()}")
+        stdout = re.sub(r"^chmod:.*\n?", "", result.stdout, flags=re.MULTILINE)
+
+        if result.exit_code != 0:
+            output = shrink_text(stdout, context_lines=20, line_len=500)
+            message = f"grade.py validation failed with exit code {result.exit_code}: {output}"
+            raise RuntimeError(message)
+        if _parsing_score(stdout) is None:
+            output = shrink_text(stdout, context_lines=20, line_len=500)
+            message = (
+                "grade.py must print a valid JSON object whose 'score' is a finite numeric value; "
+                f"received stdout: {output}"
+            )
+            raise RuntimeError(message)
+
     def _prepare_validation_scripts(
         self, reference_exp: DSExperiment, competition: str, mock_folder: str
     ) -> Tuple[str, str]:
@@ -361,6 +390,7 @@ def _prepare_validation_scripts(
         data_py_path = Path(mock_folder) / "data.py"
         grade_py_path = Path(mock_folder) / "grade.py"
         label_path = Path(mock_folder) / "workspace_input/label.csv"
+        submission_path = Path(mock_folder) / "submission.csv"
         reference_code = reference_exp.experiment_workspace.file_dict.get("main.py", "")
         if not reference_code:
             raise RuntimeError("ValidationSelector: No code found in the reference experiment.")
@@ -370,7 +400,7 @@ def _prepare_validation_scripts(
             shutil.copy(self.sample_code_path / competition / "grade.py", grade_py_path)
             data_py_code = data_py_path.read_text()
             grade_py_code = grade_py_path.read_text()
-            if not label_path.exists():
+            if not label_path.exists() or not submission_path.exists():
                 ws = FBWorkspace()
                 if self.sample_rate != 0.8:
                     data_py_code = data_py_code.replace("0.8", str(self.sample_rate)).replace(
@@ -390,10 +420,11 @@ def _prepare_validation_scripts(
                 )  # Do not cache the result
                 if result.exit_code == 0:
                     self.print_code(data_py_code, grade_py_code)
+            self._validate_grade_script(grade_py_code, reference_exp, mock_folder)
             return data_py_code, grade_py_code
 
         # --- Generate data.py if needed ---
-        if not data_py_path.exists() or not label_path.exists():
+        if not data_py_path.exists() or not label_path.exists() or not submission_path.exists():
             logger.info(f"Generating synthetic data script: {data_py_path}")
             data_py_code = self._generate_and_run_script(
                 script_type="data",
@@ -408,23 +439,31 @@ def _prepare_validation_scripts(
         data_py_code = data_py_path.read_text()
 
         # --- Generate grade.py if needed ---
-        if not grade_py_path.exists():
-            logger.info(f"Generating grading script: {grade_py_path}")
-            grade_py_code = self._generate_and_run_script(
-                script_type="grade",
-                prompt_template_key="grade",
-                reference_exp=reference_exp,
-                competition=competition,
-                mock_folder=mock_folder,
-                prompt_kwargs={
-                    "reference_code": reference_code,
-                    "sample_code": data_py_code,
-                    "input_folder": input_folder,
-                },
-            )
-            grade_py_path.write_text(grade_py_code)
-            self.print_code(data_py_code, grade_py_code)
-        return data_py_code, grade_py_path.read_text()
+        if grade_py_path.exists():
+            grade_py_code = grade_py_path.read_text()
+            try:
+                self._validate_grade_script(grade_py_code, reference_exp, mock_folder)
+            except RuntimeError as exc:
+                logger.warning(f"Cached grade.py is incompatible and will be regenerated: {exc}")
+            else:
+                return data_py_code, grade_py_code
+
+        logger.info(f"Generating grading script: {grade_py_path}")
+        grade_py_code = self._generate_and_run_script(
+            script_type="grade",
+            prompt_template_key="grade",
+            reference_exp=reference_exp,
+            competition=competition,
+            mock_folder=mock_folder,
+            prompt_kwargs={
+                "reference_code": reference_code,
+                "sample_code": data_py_code,
+                "input_folder": input_folder,
+            },
+        )
+        grade_py_path.write_text(grade_py_code)
+        self.print_code(data_py_code, grade_py_code)
+        return data_py_code, grade_py_code
 
     def _generate_and_run_script(
         self,
@@ -582,19 +621,14 @@ def _parsing_score(grade_stdout: str) -> Optional[float]:
             continue
         json_str = m.group(0)
         try:
-            # Priority 1: JSON parsing
-            return float(json.loads(json_str)["score"])
-        except:
-            pass
-        try:
-            # Priority 2: Eval dict
-            return float(eval(json_str)["score"])
-        except:
-            pass
-        try:
-            # Priority 3: Regex for the last number in the string
-            return float(re.findall(r"[-+]?\d*\.\d+|\d+", json_str)[-1])
-        except:
+            score = json.loads(json_str)["score"]
+            if isinstance(score, bool) or not isinstance(score, (int, float)):
+                continue
+            score = float(score)
+            if not math.isfinite(score):
+                continue
+            return score
+        except (KeyError, TypeError, ValueError):
             pass
     return None
 
@@ -626,9 +660,8 @@ def try_get_loop_id(trace: Trace, exp: DSExperiment):
     return index
 
 
-def extract_tar(tar_path: str, to_dir: str = "log") -> str:
-    with tarfile.open(tar_path, mode="r:*") as tar:
-        tar.extractall(path=to_dir)
+def extract_tar(tar_path: str, to_dir: str = "log") -> None:
+    safe_extract_tar(tar_path, to_dir)
 
 
 # ==============================================================================