From e2273fad9ca469b6463b8e4e7d84e4cea0feb745 Mon Sep 17 00:00:00 2001 From: Igor Lins e Silva <4753812+igorls@users.noreply.github.com> Date: Tue, 12 May 2026 19:19:43 -0300 Subject: [PATCH] fix(benchmarks): validate --language input + correct --embed-endpoint defaulting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses Copilot + gemini-code-assist review on #1483. 1. Path-traversal guard for --language. The value is interpolated into the dataset filename (`dataset.{language}.jsonl`), so unvalidated input could escape `task_dir`. Now: - regex `^[A-Za-z][A-Za-z0-9]*(?:[_-][A-Za-z0-9]+)?$` accepts en, pt-BR, zh-CN, fr_CA, etc. and rejects anything with path separators or `..` - belt-and-suspenders `Path.resolve().is_relative_to(task_dir)` check before opening the file 2. --embed-endpoint now defaults to None and is resolved after parsing: uses --endpoint when --llm-provider=ollama (so remote benchmark runs score against the same host), else http://localhost:11434. Help text now matches behavior. runner.py's CLI was also missing the flag entirely — added so single-task runs honor remote endpoints. --- benchmarks/model_eval/orchestrator.py | 12 ++++++-- benchmarks/model_eval/runner.py | 41 +++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) diff --git a/benchmarks/model_eval/orchestrator.py b/benchmarks/model_eval/orchestrator.py index 5cdcce1..9cf5618 100644 --- a/benchmarks/model_eval/orchestrator.py +++ b/benchmarks/model_eval/orchestrator.py @@ -148,8 +148,10 @@ def main(): parser.add_argument("--llm-provider", default="ollama", choices=["ollama", "openai-compat", "anthropic"], help="LLM provider (default: ollama)") - parser.add_argument("--embed-endpoint", default="http://localhost:11434", - help="Endpoint for the embedding model (always Ollama). Defaults to --endpoint when using ollama provider.") + parser.add_argument("--embed-endpoint", default=None, + help="Endpoint for the embedding model (always Ollama). " + "When omitted: defaults to --endpoint if --llm-provider=ollama, " + "else http://localhost:11434.") parser.add_argument("--embed-model", default=_EMBED_MODEL, help=f"Embedding model for semantic-similarity scoring. Default: {_EMBED_MODEL}.") parser.add_argument("--num-ctx", type=int, default=None, @@ -172,6 +174,12 @@ def main(): ) args = parser.parse_args() + # Resolve --embed-endpoint default: it lives on Ollama always, but should + # follow --endpoint when the LLM provider is Ollama so a remote benchmark + # run scores against the same host. + if args.embed_endpoint is None: + args.embed_endpoint = args.endpoint if args.llm_provider == "ollama" else "http://localhost:11434" + candidates = load_candidates(args.candidates_file, args.candidates) if not candidates: print(f"No candidates matched: {args.candidates}", file=sys.stderr) diff --git a/benchmarks/model_eval/runner.py b/benchmarks/model_eval/runner.py index 24c0f26..9027d3b 100644 --- a/benchmarks/model_eval/runner.py +++ b/benchmarks/model_eval/runner.py @@ -16,6 +16,7 @@ import argparse import dataclasses import json import os +import re import subprocess import sys import time @@ -62,6 +63,13 @@ class Result: _EMBED_MODEL = "embeddinggemma" +# Language code validator. Accepts ISO-639-style codes with optional region +# subtag (en, pt-BR, zh-CN, fr_CA, etc.). Strict pattern is required because +# `language` is interpolated into the dataset filename — without validation a +# caller could pass values containing path separators or `..` and the loader +# would read files outside the task directory. +_LANGUAGE_RE = re.compile(r"^[A-Za-z][A-Za-z0-9]*(?:[_-][A-Za-z0-9]+)?$") + # Tasks that score via semantic similarity and require an embedding model. _EMBED_TASKS: set[tuple[str, str]] = { ("memory_extraction", "default"), @@ -247,9 +255,31 @@ def run( host = gather_host_info() run_date = time.strftime("%Y-%m-%d %H:%M:%S UTC", time.gmtime()) + if not _LANGUAGE_RE.match(language): + return Result( + model_tag=model_tag, task=task, mode=mode, + accuracy=0.0, extras={}, timing=aggregate_timings([]), + vram_resident_mb=None, vram_peak_mb=None, host=host, + run_date=run_date, n_samples=0, language=language, + error=f"Invalid language code: {language!r}. Expected pattern like 'en', 'pt-BR', 'zh-CN'.", + ) + task_dir = dataset_dir / task dataset_file = "dataset.jsonl" if language == "en" else f"dataset.{language}.jsonl" dataset_path = task_dir / dataset_file + # Belt-and-suspenders: even with the regex guard above, confirm the resolved + # path lives inside task_dir before opening anything on disk. + try: + if not dataset_path.resolve().is_relative_to(task_dir.resolve()): + raise ValueError("resolved path escapes task_dir") + except (ValueError, OSError) as e: + return Result( + model_tag=model_tag, task=task, mode=mode, + accuracy=0.0, extras={}, timing=aggregate_timings([]), + vram_resident_mb=None, vram_peak_mb=None, host=host, + run_date=run_date, n_samples=0, language=language, + error=f"Refused to load dataset outside task_dir: {dataset_path} ({e})", + ) if not dataset_path.exists(): return Result( model_tag=model_tag, task=task, mode=mode, @@ -391,6 +421,10 @@ def main(): parser.add_argument("--llm-provider", default="ollama", choices=["ollama", "openai-compat", "anthropic"], help="LLM provider (default: ollama)") + parser.add_argument("--embed-endpoint", default=None, + help="Endpoint for the embedding model (always Ollama). " + "When omitted: defaults to --endpoint if --llm-provider=ollama, " + "else http://localhost:11434.") parser.add_argument("--language", default="en", help="Dataset language suffix. 'en' loads dataset.jsonl; " "other values load dataset.{lang}.jsonl (e.g. pt-BR, es, zh).") @@ -407,6 +441,12 @@ def main(): if args.task != "room_classification" and args.mode in ("closed", "open"): args.mode = "default" + # Resolve --embed-endpoint default: it lives on Ollama always, but should + # follow --endpoint when the LLM provider is Ollama so a remote benchmark + # run scores against the same host. + if args.embed_endpoint is None: + args.embed_endpoint = args.endpoint if args.llm_provider == "ollama" else "http://localhost:11434" + result = run( model_tag=args.model, task=args.task, @@ -417,6 +457,7 @@ def main(): n_samples=args.n, strip_thinking=not args.no_strip_thinking, llm_provider=args.llm_provider, + embed_endpoint=args.embed_endpoint, language=args.language, embed_model=args.embed_model, num_ctx=args.num_ctx,