fix: harden security and sleep PR follow-ups

scripts/train.py

@@ -386,17 +386,35 @@ def load_config(args: argparse.Namespace) -> dict:
     import warnings
     from skillopt.config import load_config as _load, flatten_config, is_structured
 
-    # F08: Warn when API keys are supplied on the CLI — prefer env vars or managed identity.
-    for _cli_key in (
-        "azure_api_key", "azure_openai_api_key",
-        "optimizer_azure_openai_api_key", "target_azure_openai_api_key",
-        "minimax_api_key",
-    ):
+    # F08: Warn when API keys are supplied on the CLI. Keep the replacement
+    # guidance specific to each backend and, where applicable, each role.
+    _credential_guidance = {
+        "azure_api_key": (
+            "AZURE_OPENAI_API_KEY or "
+            "--azure_openai_auth_mode=managed_identity"
+        ),
+        "azure_openai_api_key": (
+            "AZURE_OPENAI_API_KEY or "
+            "--azure_openai_auth_mode=managed_identity"
+        ),
+        "optimizer_azure_openai_api_key": (
+            "OPTIMIZER_AZURE_OPENAI_API_KEY or "
+            "--optimizer_azure_openai_auth_mode=managed_identity"
+        ),
+        "target_azure_openai_api_key": (
+            "TARGET_AZURE_OPENAI_API_KEY or "
+            "--target_azure_openai_auth_mode=managed_identity"
+        ),
+        "qwen_chat_api_key": "QWEN_CHAT_API_KEY",
+        "optimizer_qwen_chat_api_key": "OPTIMIZER_QWEN_CHAT_API_KEY",
+        "target_qwen_chat_api_key": "TARGET_QWEN_CHAT_API_KEY",
+        "minimax_api_key": "MINIMAX_API_KEY",
+    }
+    for _cli_key, _guidance in _credential_guidance.items():
         if getattr(args, _cli_key, None):
             warnings.warn(
-                f"--{_cli_key} is deprecated: provide credentials via the "
-                f"AZURE_OPENAI_API_KEY environment variable or set "
-                f"--azure_openai_auth_mode=managed_identity instead.",
+                f"--{_cli_key} is deprecated: provide credentials via "
+                f"{_guidance} instead.",
                 DeprecationWarning,
                 stacklevel=2,
             )

skillopt/envs/spreadsheetbench/codegen_agent.py

@@ -262,6 +262,7 @@ def _build_codex_driver() -> str:
         "import re\n"
         "import subprocess\n"
         "import sys\n\n"
+        "import tempfile\n\n"
         'INPUT_PATH = "input.xlsx"\n'
         'OUTPUT_PATH = "output.xlsx"\n'
         "code = pathlib.Path('solution.py').read_text(encoding='utf-8')\n"
@@ -277,15 +278,21 @@ def _build_codex_driver() -> str:
         "_safe_env = {\n"
         "    'PATH': _os.environ.get('PATH', '/usr/bin:/bin'),\n"
         "    'HOME': str(pathlib.Path('_driver_runner.py').parent.resolve()),\n"
+        "    'TMPDIR': tempfile.gettempdir(),\n"
         "}\n"
         "if _os.name == 'nt':\n"
         "    _safe_env['SYSTEMROOT'] = _os.environ.get('SYSTEMROOT', '')\n"
-        "    _safe_env['TEMP'] = _safe_env['HOME']\n"
-        "    _safe_env['TMP'] = _safe_env['HOME']\n"
+        "    _safe_env['TEMP'] = tempfile.gettempdir()\n"
+        "    _safe_env['TMP'] = tempfile.gettempdir()\n"
         "_safe_env = {k: v for k, v in _safe_env.items() if v}\n"
-        "_res = subprocess.run([sys.executable, str(_patched)], capture_output=True, text=True, env=_safe_env)\n"
+        "try:\n"
+        "    _res = subprocess.run([sys.executable, str(_patched)], capture_output=True, text=True, env=_safe_env)\n"
+        "finally:\n"
+        "    try:\n"
+        "        _patched.unlink()\n"
+        "    except OSError:\n"
+        "        pass\n"
         "if _res.returncode != 0:\n"
-        "    import traceback as _tb\n"
         "    print(_res.stdout, end='')\n"
         "    print(_res.stderr, end='')\n"
         "    sys.exit(2)\n"

skillopt/envs/spreadsheetbench/executor.py

@@ -46,8 +46,9 @@ def run_generated_code(code: str, input_path: str, output_path: str, timeout: in
     with tempfile.NamedTemporaryFile("w", suffix=".py", delete=False) as f:
         f.write(script)
         tmp = f.name
-    # Build a minimal environment so the generated code cannot read API keys,
-    # cloud credentials, or other secrets from the current process environment.
+    # Build a minimal environment so generated code does not directly inherit
+    # API keys, cloud credentials, or other parent-process environment values.
+    # This is environment isolation, not a filesystem or network sandbox.
     import platform as _platform
     _safe_env: dict[str, str] = {
         "PATH": os.environ.get("PATH", "/usr/bin:/bin"),
@@ -58,7 +59,7 @@ def run_generated_code(code: str, input_path: str, output_path: str, timeout: in
         _safe_env["SYSTEMROOT"] = os.environ.get("SYSTEMROOT", "")
         _safe_env["TEMP"] = tempfile.gettempdir()
         _safe_env["TMP"] = tempfile.gettempdir()
-    # Drop empty entries (env dict values must be non-empty strings)
+    # Omit platform-specific entries that are absent in the parent.
     _safe_env = {k: v for k, v in _safe_env.items() if v}
     try:
         proc = subprocess.run(

skillopt/envs/spreadsheetbench/react_agent.py

@@ -11,6 +11,7 @@
 import os
 import shlex
 import subprocess
+import sys
 
 from skillopt.model import chat_target_messages
 from skillopt.prompts import load_prompt
@@ -249,25 +250,35 @@ def _auto_verify(work_dir: str) -> str:
         return f"\n\n[AUTO-VERIFY] Could not inspect output: {e}"
 
 
-# Commands that the ReAct agent is allowed to run (benchmark only needs Python).
-_CMD_ALLOW = {"python", "python3"}
+# Command aliases that the ReAct agent may request. Every accepted alias is
+# resolved to this process's interpreter before execution, so behavior does not
+# depend on PATH and similarly named executables cannot bypass the allow-list.
+_PYTHON_ALIASES = {"python", "python3", "python.exe", "python3.exe"}
 
 
 # ── Bash execution ────────────────────────────────────────────────────────────
 
 def _run_bash(cmd: str, work_dir: str, timeout: int = 60) -> str:
     try:
-        parts = shlex.split(cmd)
+        parts = shlex.split(cmd, posix=os.name != "nt")
+        if os.name == "nt":
+            # shlex in non-POSIX mode retains surrounding quotes.
+            parts = [
+                part[1:-1]
+                if len(part) >= 2 and part[0] == part[-1] and part[0] in {'"', "'"}
+                else part
+                for part in parts
+            ]
         if not parts:
             return "[error: empty command]"
-        exe_name = os.path.basename(parts[0]).lower()
-        # Strip .exe suffix on Windows (python.exe → python)
-        exe_stem = exe_name.split(".")[0]
-        if exe_stem not in _CMD_ALLOW:
+        exe_name = parts[0].replace("\\", "/").rsplit("/", 1)[-1].lower()
+        if exe_name not in _PYTHON_ALIASES:
             return (
-                f"[blocked: '{parts[0]}' not in allow-list {sorted(_CMD_ALLOW)}; "
+                f"[blocked: '{parts[0]}' not in allow-list "
+                f"{sorted(_PYTHON_ALIASES)}; "
                 "use Python to manipulate spreadsheets]"
             )
+        parts[0] = sys.executable
         proc = subprocess.run(
             parts,
             shell=False,

skillopt_sleep/cycle.py

@@ -32,10 +32,26 @@
 
 # ── Model-swap detection (F16) ───────────────────────────────
 def _make_model_key(cfg: SleepConfig) -> str:
-    """Stable string identifying the backend+model combination for this cycle."""
-    return "{}::{}".format(
-        cfg.get("backend", "mock"),
-        cfg.get("model", ""),
+    """Stable string identifying the effective backend/model role(s)."""
+    backend = str(cfg.get("backend", "mock") or "mock")
+    model = str(cfg.get("model", "") or "")
+    split_keys = (
+        "optimizer_backend",
+        "optimizer_model",
+        "target_backend",
+        "target_model",
+    )
+    if not any(cfg.get(key, "") for key in split_keys):
+        # Preserve the original state format for ordinary single-backend runs.
+        return f"{backend}::{model}"
+
+    optimizer_backend = str(cfg.get("optimizer_backend", "") or backend)
+    optimizer_model = str(cfg.get("optimizer_model", "") or model)
+    target_backend = str(cfg.get("target_backend", "") or backend)
+    target_model = str(cfg.get("target_model", "") or model)
+    return (
+        f"optimizer={optimizer_backend}::{optimizer_model};"
+        f"target={target_backend}::{target_model}"
     )
 
 

skillopt_sleep/judges.py

@@ -75,7 +75,7 @@ def _check(op: str, arg: Any, response: str,
 })
 
 
-def validate_checks(judge: Dict[str, Any]) -> Tuple[List[str], List[str]]:
+def validate_checks(judge: Any) -> Tuple[List[str], List[str]]:
     """Return ``(errors, warnings)`` for a rule judge's checks.
 
     An *error* means the check can never behave as written — a regex that does
@@ -85,7 +85,7 @@ def validate_checks(judge: Dict[str, Any]) -> Tuple[List[str], List[str]]:
     """
     errors: List[str] = []
     warnings: List[str] = []
-    if judge and not isinstance(judge, dict):
+    if judge is not None and not isinstance(judge, dict):
         return [f"judge must be an object, got {type(judge).__name__}"], warnings
     checks = (judge or {}).get("checks", []) or []
     if not isinstance(checks, list):

skillopt_sleep/memory.py

@@ -117,6 +117,9 @@ def apply_edits_detailed(
             applied.append(e)
         elif op == "delete":
             anchor = _norm(e.anchor or e.content)
+            if not anchor:
+                unmatched.append(e)
+                continue
             keep = [line for line in lines if anchor not in _norm(line)]
             if len(keep) != len(lines):
                 lines = keep

skillopt_sleep/staging.py

@@ -33,8 +33,20 @@
      "[REDACTED_JWT]"),
     (re.compile(r"(?i)(Authorization:\s*Bearer\s+)[^\s\"']+"), r"\1[REDACTED]"),
     (re.compile(r"(?i)(Authorization:\s*Basic\s+)[^\s\"']+"), r"\1[REDACTED]"),
+    # Connection-string passwords. Handle quoted values (which may contain
+    # semicolons) before the generic name=value rule below, and retain the key
+    # plus all non-secret connection-string fields for useful diagnostics.
     (
-        re.compile(r"(?i)\b(api[_-]?key|token|password|secret)\b(\s*[:=]\s*)[^\s\"']+"),
+        re.compile(
+            r'''(?i)(\bPassword\s*=\s*)(?:"[^"]+"|'[^']+'|[^;"'\s&]+)'''
+        ),
+        r"\1[REDACTED_DB_PASS]",
+    ),
+    (
+        re.compile(
+            r"(?i)\b(api[_-]?key|token|password|secret)\b"
+            r"(\s*[:=]\s*)(?!\[REDACTED(?:_[A-Z_]+)?\])[^\s\"';&]+"
+        ),
         r"\1\2[REDACTED]",
     ),
     (
@@ -52,8 +64,6 @@
     (re.compile(r"(?i)\bsig=[A-Za-z0-9%+/]{10,}"), "[REDACTED_SAS_SIG]"),
     # Azure Storage account keys (base64, typically 88 chars)
     (re.compile(r"(?i)AccountKey=[A-Za-z0-9+/=]{20,}"), "[REDACTED_STORAGE_KEY]"),
-    # Connection-string passwords (Password=...; up to semicolon/quote/whitespace)
-    (re.compile(r"(?i)\bPassword=[^;\"'\s]{6,}"), "[REDACTED_DB_PASS]"),
 )
 
 

skillopt_sleep/tasks_file.py

@@ -86,7 +86,7 @@ def load_tasks_file(
     # complies — the run looks legitimate while one dimension is dead.
     errors: List[str] = []
     for task in tasks:
-        if task.reference_kind != "rule" or not task.judge:
+        if task.reference_kind != "rule":
             continue
         task_errors, task_warnings = validate_checks(task.judge)
         errors.extend(f"task {task.id}: {e}" for e in task_errors)

tests/test_executor_env_isolation.py

@@ -7,8 +7,10 @@
 """
 from __future__ import annotations
 
-import os
+import subprocess
+import sys
 
+from skillopt.envs.spreadsheetbench.codegen_agent import _build_codex_driver
 from skillopt.envs.spreadsheetbench.executor import run_generated_code
 
 
@@ -44,3 +46,32 @@ def test_path_still_available_to_generated_code(tmp_path) -> None:
 
     assert ok, err
     assert out.read_text(encoding="utf-8") == "YES"
+
+
+def test_codex_driver_scrubs_env_sets_tempdir_and_cleans_runner(
+    tmp_path, monkeypatch
+) -> None:
+    monkeypatch.setenv("SUPER_SECRET_TOKEN", "do-not-inherit")
+    (tmp_path / "solution.py").write_text(
+        "import os\n"
+        "with open(OUTPUT_PATH, 'w', encoding='utf-8') as f:\n"
+        "    f.write('|'.join([\n"
+        "        os.environ.get('SUPER_SECRET_TOKEN', 'ABSENT'),\n"
+        "        'TMPDIR' if os.environ.get('TMPDIR') else 'NO_TMPDIR',\n"
+        "    ]))\n",
+        encoding="utf-8",
+    )
+    driver = tmp_path / "run_solution.py"
+    driver.write_text(_build_codex_driver(), encoding="utf-8")
+
+    proc = subprocess.run(
+        [sys.executable, str(driver)],
+        cwd=tmp_path,
+        capture_output=True,
+        text=True,
+        timeout=30,
+    )
+
+    assert proc.returncode == 0, proc.stdout + proc.stderr
+    assert (tmp_path / "output.xlsx").read_text(encoding="utf-8") == "ABSENT|TMPDIR"
+    assert not (tmp_path / "_driver_runner.py").exists()

tests/test_judges.py

@@ -4,9 +4,13 @@
 A regex that does not compile returns False on every rollout, so the affected
 dimension scores 0.0 forever while the run still looks healthy.
 """
+import json
 import unittest
 
+import pytest
+
 from skillopt_sleep.judges import KNOWN_OPS, score_rule_judge, validate_checks
+from skillopt_sleep.tasks_file import load_tasks_file
 
 
 class TestCheckOperators(unittest.TestCase):
@@ -56,7 +60,7 @@ def test_empty_checks_score_zero(self) -> None:
 class TestMalformedRegexIsDistinguishable(unittest.TestCase):
     """A pattern Python cannot parse must not read like a plain miss."""
 
-    BAD = r"(?i)foo|(?i)bar"  # inline flag not at the start -> re.error
+    BAD = r"foo("  # unclosed group is invalid on every supported Python version
 
     def test_bad_pattern_still_fails_closed(self) -> None:
         # the response does contain both alternatives; the pattern is the problem
@@ -87,7 +91,7 @@ def test_sound_checks_produce_nothing(self) -> None:
         self.assertEqual(warnings, [])
 
     def test_uncompilable_regex_is_an_error(self) -> None:
-        errors, _warnings = validate_checks({"checks": [{"op": "regex", "arg": r"(?i)a|(?i)b"}]})
+        errors, _warnings = validate_checks({"checks": [{"op": "regex", "arg": r"foo("}]})
         self.assertEqual(len(errors), 1)
         self.assertIn("does not compile", errors[0])
 
@@ -108,7 +112,7 @@ def test_non_object_check_is_an_error(self) -> None:
 
     def test_malformed_judge_is_reported_not_raised(self) -> None:
         # a tasks file may carry any JSON here; validation must stay structured
-        for bad in (["not", "a", "dict"], "a string", 42):
+        for bad in (["not", "a", "dict"], [], "a string", "", 42, 0):
             errors, _warnings = validate_checks(bad)
             self.assertEqual(len(errors), 1, bad)
             self.assertIn("must be an object", errors[0])
@@ -129,5 +133,31 @@ def test_every_known_op_is_accepted(self) -> None:
             self.assertEqual((errors, warnings), ([], []), op)
 
 
+@pytest.mark.parametrize("bad_judge", [[], "", 0])
+def test_tasks_file_rejects_falsy_non_object_judges(
+    tmp_path, bad_judge
+) -> None:
+    path = tmp_path / "tasks.json"
+    path.write_text(
+        json.dumps(
+            {
+                "tasks": [
+                    {
+                        "id": "malformed-rule",
+                        "project": "test",
+                        "intent": "test",
+                        "reference_kind": "rule",
+                        "judge": bad_judge,
+                    }
+                ]
+            }
+        ),
+        encoding="utf-8",
+    )
+
+    with pytest.raises(ValueError, match="judge must be an object"):
+        load_tasks_file(str(path))
+
+
 if __name__ == "__main__":
     unittest.main()