Merge branch 'main' into dataset/code-review/gold-audit-639655

Author: gggdttt

docs/_data/code-review.json

@@ -87,6 +87,11 @@ ignore = [
     "TRY301",  # raise-within-try: suggested "abstract to inner function" refactor adds indirection
 ]
 
+[tool.ruff.lint.flake8-tidy-imports.banned-api]
+# Force sandboxed rendering:
+"jinja2.Template".msg = "Use jinja2.sandbox.SandboxedEnvironment instead; the bare Template is not sandboxed."
+"jinja2.Environment".msg = "Use jinja2.sandbox.SandboxedEnvironment instead; the bare Environment is not sandboxed."
+
 [tool.ruff.lint.per-file-ignores]
 "tests/**" = ["ANN"]
 "notebooks/**" = ["ANN", "T20"]   # notebooks routinely print() for analysis output

pyproject.toml

@@ -4,7 +4,7 @@
 from pathlib import Path
 from typing import Any
 
-from jinja2 import Template
+from jinja2.sandbox import SandboxedEnvironment
 
 from bcbench.agent.shared.altool_paths import build_assembly_probing_paths, compiler_symbol_folder_for_container
 from bcbench.dataset import BaseDatasetEntry
@@ -13,6 +13,8 @@
 
 logger = get_logger(__name__)
 
+_jinja = SandboxedEnvironment(autoescape=False)
+
 
 def _build_server_entry(server: dict[str, Any], template_context: dict[str, Any]) -> tuple[str, dict[str, Any]]:
     server_type: str = server["type"]
@@ -26,7 +28,7 @@ def _build_server_entry(server: dict[str, Any], template_context: dict[str, Any]
             }
         case "stdio":
             args: list[str] = server["args"]
-            rendered_args = [Template(arg).render(**template_context) for arg in args]
+            rendered_args = [_jinja.from_string(arg).render(**template_context) for arg in args]
             command: str = shutil.which(server["command"]) or server["command"]
             entry: dict[str, Any] = {
                 "type": server_type,

src/bcbench/agent/shared/mcp.py

@@ -1,14 +1,16 @@
 import re
 from pathlib import Path
 
-from jinja2 import Template
+from jinja2.sandbox import SandboxedEnvironment
 
 from bcbench.config import get_config
 from bcbench.dataset import BaseDatasetEntry
 from bcbench.types import EvaluationCategory
 
 _config = get_config()
 
+_jinja = SandboxedEnvironment(autoescape=False)
+
 
 def _transform_image_paths(content: str) -> str:
     dest_dir = _config.file_patterns.problem_statement_dest_dir
@@ -26,8 +28,7 @@ def build_prompt(entry: BaseDatasetEntry, repo_path: Path, config: dict, categor
 
     task = _transform_image_paths(entry.get_task())
 
-    template = Template(template_str)
-    return template.render(
+    return _jinja.from_string(template_str).render(
         repo_path=repo_path,
         task=task,
         project_paths=", ".join(entry.project_paths),

src/bcbench/agent/shared/prompt.py

@@ -11,7 +11,7 @@
 # Note: Defaults are provided in function signatures, not here
 RepoPath = Annotated[Path, typer.Option(help="Path to repository")]
 
-OutputDir = Annotated[Path, typer.Option(help="Directory to save evaluation results")]
+OutputDir = Annotated[Path, typer.Option(help="Directory to save evaluation results", file_okay=False, dir_okay=True)]
 
 RunId = Annotated[str, typer.Option(envvar="GITHUB_RUN_ID", help="Unique identifier for this evaluation run")]
 

src/bcbench/cli_options.py

@@ -1,6 +1,7 @@
 from __future__ import annotations
 
 from enum import StrEnum
+from typing import Annotated
 
 from pydantic import BaseModel, ConfigDict, Field, field_validator
 
@@ -46,11 +47,11 @@ def from_input(cls, value: str) -> Severity:
 class ReviewComment(BaseModel):
     model_config = ConfigDict(frozen=True)
 
-    file: str
-    line_start: int
-    line_end: int | None = None
+    file: Annotated[str, Field(pattern=r"^[A-Za-z0-9][A-Za-z0-9./_-]*\.(al|json)$")]
+    line_start: Annotated[int, Field(ge=1)]
+    line_end: Annotated[int, Field(ge=1)] | None = None
     domain: str | None = None
-    body: str
+    body: Annotated[str, Field(min_length=1)]
     severity: Severity | None = None
 
     @field_validator("severity", mode="before")

src/bcbench/dataset/codereview.py

@@ -20,15 +20,15 @@
 class TestEntry(BaseModel):
     model_config = ConfigDict(frozen=True)
 
-    codeunitID: int
+    codeunitID: Annotated[int, Field(gt=0)]
     functionName: Annotated[frozenset[str], Field(min_length=1)]
 
 
 class EntryMetadata(BaseModel):
     model_config = ConfigDict(frozen=True)
 
     area: str | None = None
-    image_count: int | None = None
+    image_count: Annotated[int, Field(ge=0)] | None = None
     persona: str | None = None
 
 
@@ -44,8 +44,8 @@ class BaseDatasetEntry(BaseModel):
     base_commit: str = Field(pattern=r"^[a-fA-F0-9]{40}$")
     created_at: Annotated[str, Field(min_length=1)]
     environment_setup_version: str = Field(pattern=r"^[0-9]{2}\.[0-9]{1}$")
-    project_paths: list[str] = []
-    patch: Annotated[str, Field(min_length=1)]
+    project_paths: list[Annotated[str, Field(pattern=r"^[A-Za-z0-9][A-Za-z0-9 \\/-]*$")]] = []
+    patch: Annotated[str, Field(min_length=1, pattern=r"^[^\x00]*$")]
 
     @classmethod
     def load(cls, dataset_path: Path, entry_id: str | None = None, random: int | None = None) -> list[Self]:
@@ -112,7 +112,7 @@ class _BugFixTestGenBase(BaseDatasetEntry):
 
     fail_to_pass: Annotated[list[TestEntry], Field(alias="FAIL_TO_PASS", min_length=1)]
     pass_to_pass: Annotated[list[TestEntry], Field(alias="PASS_TO_PASS")] = []
-    test_patch: Annotated[str, Field(min_length=1)]
+    test_patch: Annotated[str, Field(min_length=1, pattern=r"^[^\x00]*$")]
 
     @property
     def problem_statement_dir(self) -> Path:
@@ -158,9 +158,9 @@ class NL2ALEntry(BaseDatasetEntry):
     """Dataset entry for NL2AL category — generate AL code from natural language."""
 
     base_commit: str | None = None
-    nl_prompt: Annotated[str, Field(min_length=1)]
+    nl_prompt: Annotated[str, Field(min_length=1, pattern=r"^[^\x00]*$")]
     expected: Annotated[list[ChecklistAssertion], Field(min_length=1)]
-    page: str
+    page: Annotated[str, Field(pattern=r"^[A-Za-z0-9][A-Za-z0-9 ./]*$")]
     audience: Literal["Business", "Technical", "Both"]
 
     def get_task(self) -> str:

src/bcbench/dataset/dataset_entry.py

@@ -1,5 +1,6 @@
 import contextlib
 import json
+import shlex
 from pathlib import Path
 
 from bcbench.config import get_config
@@ -60,15 +61,16 @@ def _setup_claude_hooks(repo_path: Path, script_path: str, tool_log_path: Path)
         with contextlib.suppress(json.JSONDecodeError):
             existing_settings = json.loads(settings_file.read_text(encoding="utf-8"))
 
-    tool_log_resolved = str(tool_log_path.resolve())
+    tool_log_quoted = shlex.quote(str(tool_log_path.resolve()))
+    script_path_quoted = shlex.quote(script_path)
     existing_settings["hooks"] = {
         "PreToolUse": [
             {
                 "matcher": "",
                 "hooks": [
                     {
                         "type": "command",
-                        "command": f'BCBENCH_TOOL_LOG="{tool_log_resolved}" pwsh -ExecutionPolicy Bypass -File "{script_path}"',
+                        "command": f"BCBENCH_TOOL_LOG={tool_log_quoted} pwsh -ExecutionPolicy Bypass -File {script_path_quoted}",
                     }
                 ],
             }

src/bcbench/operations/hooks_operations.py

@@ -1,4 +1,5 @@
 import json
+import shlex
 from pathlib import Path
 
 from bcbench.operations.hooks_operations import setup_hooks
@@ -102,3 +103,19 @@ def test_tool_log_path_is_absolute_in_env(self, tmp_path: Path):
         log_path = hooks_config["hooks"]["preToolUse"][0]["env"]["BCBENCH_TOOL_LOG"]
 
         assert Path(log_path).is_absolute()
+
+    def test_claude_command_escapes_shell_metacharacters(self, tmp_path: Path):
+        repo_path = tmp_path / "repo"
+        repo_path.mkdir()
+        output_dir = tmp_path / "results$(id)`whoami`"
+        output_dir.mkdir()
+
+        setup_hooks(repo_path, AgentType.CLAUDE, output_dir)
+
+        settings = json.loads((repo_path / ".claude" / "settings.local.json").read_text(encoding="utf-8"))
+        command = settings["hooks"]["PreToolUse"][0]["hooks"][0]["command"]
+
+        # shlex.quote wraps the path in single quotes so $() and backticks are inert.
+        assert command.startswith("BCBENCH_TOOL_LOG='")
+        assert 'BCBENCH_TOOL_LOG="' not in command
+        assert shlex.quote(str((output_dir / "tool_usage.jsonl").resolve())) in command