From 6b2a41a600bd0e1b260c3c80494f2d50f2b1d2fa Mon Sep 17 00:00:00 2001 From: XuanRui LI Date: Tue, 2 Jun 2026 20:42:33 +0800 Subject: [PATCH 1/4] Add WebHarbor task validation script --- README.md | 13 +- scripts/test_validate_tasks.py | 94 ++++++ scripts/validate_tasks.py | 519 +++++++++++++++++++++++++++++++++ 3 files changed, 625 insertions(+), 1 deletion(-) create mode 100644 scripts/test_validate_tasks.py create mode 100644 scripts/validate_tasks.py diff --git a/README.md b/README.md index dce3f934..46d7c87f 100644 --- a/README.md +++ b/README.md @@ -99,6 +99,17 @@ Any other improvement — bug fixes, UI polish, data enrichment, task suggestion | 📊 Contribution Track Sheet | [Google Sheet](https://docs.google.com/spreadsheets/d/1vZsrQjy9nJKze58fx4kbQtFi85NjVXIWCFyu3ShD7gk/edit?gid=0#gid=0) | | 📝 Contribution Request Form | [Google Form](https://forms.gle/ngcD1rzAfUEphNmRA) | +## Validate Tasks + +Use the repository task validator to check task JSONL files and localhost metadata before opening a review or PR: + +```bash +python scripts/validate_tasks.py +python scripts/validate_tasks.py --site amazon +python scripts/validate_tasks.py --strict +python scripts/validate_tasks.py --json +``` + ## Citation WebHarbor is initiated by UNC-Chapel Hill and Microsoft, with contributions from the broader community. If you have any questions, please contact us via `webharborcomm at gmail dot com` or `zhaoyang at cs dot unc dot edu`. @@ -111,4 +122,4 @@ WebHarbor is initiated by UNC-Chapel Hill and Microsoft, with contributions from url = {https://aiming-lab.github.io/webharbor.github.io}, note = {Project website.} } -``` \ No newline at end of file +``` diff --git a/scripts/test_validate_tasks.py b/scripts/test_validate_tasks.py new file mode 100644 index 00000000..03290dd3 --- /dev/null +++ b/scripts/test_validate_tasks.py @@ -0,0 +1,94 @@ +#!/usr/bin/env python3 +"""Lightweight tests for scripts/validate_tasks.py.""" + +from __future__ import annotations + +import io +import json +import sys +import tempfile +import unittest +from contextlib import redirect_stdout +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) +import validate_tasks as vt + + +def task_line( + *, + task_id: str = "Demo--0", + web_name: str = "Demo", + web: str = "http://localhost:40000/", + upstream_url: str = "https://example.com/demo", + ques: str = "Find the lowest priced demo item with at least two filters applied.", +) -> str: + return json.dumps( + { + "id": task_id, + "web_name": web_name, + "web": web, + "upstream_url": upstream_url, + "ques": ques, + } + ) + + +class ValidateTasksTests(unittest.TestCase): + def make_root(self, task_contents: str) -> Path: + temp_root = Path(tempfile.mkdtemp(prefix="validate-tasks-")) + (temp_root / "sites" / "demo").mkdir(parents=True) + (temp_root / "sites" / "demo" / "tasks.jsonl").write_text(task_contents, encoding="utf-8") + (temp_root / "websyn_start.sh").write_text("#!/bin/bash\nSITES=(demo other)\n", encoding="utf-8") + (temp_root / "control_server.py").write_text("SITES = ['demo', 'other']\n", encoding="utf-8") + return temp_root + + def test_valid_jsonl_passes(self) -> None: + root = self.make_root(task_line() + "\n") + summary = vt.run_validation(root=root) + self.assertEqual(summary["errors"], 0) + self.assertEqual(summary["warnings"], 0) + self.assertEqual(summary["task_count"], 1) + self.assertEqual(summary["exit_code"], 0) + + def test_malformed_jsonl_fails(self) -> None: + root = self.make_root("{this is not json}\n") + summary = vt.run_validation(root=root) + self.assertGreater(summary["errors"], 0) + self.assertEqual(summary["exit_code"], 1) + + def test_duplicate_ids_fail(self) -> None: + root = self.make_root(task_line(task_id="Demo--1") + "\n" + task_line(task_id="Demo--1") + "\n") + summary = vt.run_validation(root=root) + self.assertGreater(summary["errors"], 0) + self.assertTrue(any(f["code"] == "duplicate-id-cross-site" or f["code"] == "duplicate-id" for f in summary["findings"])) + + def test_missing_required_field_fails(self) -> None: + bad_line = json.dumps({"id": "Demo--2", "web_name": "Demo", "web": "http://localhost:40000/", "ques": "Missing upstream."}) + root = self.make_root(bad_line + "\n") + summary = vt.run_validation(root=root) + self.assertGreater(summary["errors"], 0) + self.assertTrue(any(f["code"] == "missing-field" for f in summary["findings"])) + + def test_warnings_do_not_fail_unless_strict(self) -> None: + root = self.make_root(task_line(task_id="Demo--3", ques="The answer is already visible on the page.") + "\n") + non_strict = vt.run_validation(root=root, strict=False) + strict = vt.run_validation(root=root, strict=True) + self.assertEqual(non_strict["errors"], 0) + self.assertGreater(non_strict["warnings"], 0) + self.assertEqual(non_strict["exit_code"], 0) + self.assertEqual(strict["exit_code"], 1) + + def test_json_output_is_valid_json(self) -> None: + root = self.make_root(task_line() + "\n") + buffer = io.StringIO() + with redirect_stdout(buffer): + exit_code = vt.main(["--json"], root=root) + payload = json.loads(buffer.getvalue()) + self.assertEqual(exit_code, 0) + self.assertEqual(payload["task_count"], 1) + self.assertIn("files", payload) + + +if __name__ == "__main__": + unittest.main() diff --git a/scripts/validate_tasks.py b/scripts/validate_tasks.py new file mode 100644 index 00000000..133ecff1 --- /dev/null +++ b/scripts/validate_tasks.py @@ -0,0 +1,519 @@ +#!/usr/bin/env python3 +"""Validate WebHarbor task files and site metadata.""" + +from __future__ import annotations + +import argparse +import json +import re +import sys +from collections import defaultdict +from dataclasses import asdict, dataclass +from pathlib import Path +from typing import Iterable +from urllib.parse import urlparse + +REQUIRED_FIELDS = ("id", "web_name", "web", "upstream_url", "ques") +LOCAL_HOSTS = {"localhost", "127.0.0.1"} +SUSPICIOUS_PATTERNS = ( + (re.compile(r"\breal payment\b", re.IGNORECASE), "real payment"), + (re.compile(r"\breal booking\b", re.IGNORECASE), "real booking"), + (re.compile(r"\blive api\b", re.IGNORECASE), "live api"), + (re.compile(r"\bapi[_ -]?key\b", re.IGNORECASE), "api_key"), + (re.compile(r"\bsecret\b", re.IGNORECASE), "secret"), + (re.compile(r"\baccess token\b", re.IGNORECASE), "access token"), + (re.compile(r"\bpassword leak\b", re.IGNORECASE), "password leak"), + (re.compile(r"\bproduction (environment|system|server)\b", re.IGNORECASE), "production environment"), + (re.compile(r"\bexternal runtime call\b", re.IGNORECASE), "external runtime call"), + (re.compile(r"\bcredit card\b", re.IGNORECASE), "credit card"), + (re.compile(r"\bssn\b", re.IGNORECASE), "ssn"), + (re.compile(r"\bsocial security number\b", re.IGNORECASE), "social security number"), +) +BAD_MARKERS = ("todo", "fixme", "lorem", "placeholder") +ANSWER_LEAK_PATTERNS = ( + (re.compile(r"\bthe answer is\b", re.IGNORECASE), "question appears to reveal the answer directly"), + ( + re.compile(r"\bselect the option named exactly\b", re.IGNORECASE), + "question may leak the exact UI text of the answer", + ), +) +CONFIRMATION_CODE_PATTERN = re.compile(r"\b[A-Z0-9]{5,}\b") +DEFAULT_ROOT = Path(__file__).resolve().parents[1] + + +@dataclass +class Finding: + level: str + path: str + line: int | None + code: str + message: str + + +@dataclass +class FileSummary: + path: str + site: str + task_count: int = 0 + errors: int = 0 + warnings: int = 0 + + +def normalize_token(value: str) -> str: + return re.sub(r"[^a-z0-9]+", "", value.lower()) + + +def relative_to_root(path: Path, root: Path) -> str: + try: + return path.resolve().relative_to(root.resolve()).as_posix() + except ValueError: + return path.resolve().as_posix() + + +def add_finding( + findings: list[Finding], + file_summaries: dict[Path, FileSummary], + path: Path, + root: Path, + level: str, + code: str, + message: str, + line: int | None = None, +) -> None: + findings.append(Finding(level=level, path=relative_to_root(path, root), line=line, code=code, message=message)) + if path in file_summaries: + if level == "error": + file_summaries[path].errors += 1 + elif level == "warning": + file_summaries[path].warnings += 1 + + +def parse_websyn_sites(path: Path) -> list[str]: + text = path.read_text(encoding="utf-8") + match = re.search(r"SITES=\((.*?)\)", text, re.DOTALL) + if not match: + return [] + return re.findall(r"[A-Za-z0-9_]+", match.group(1)) + + +def parse_control_server_sites(path: Path) -> list[str]: + text = path.read_text(encoding="utf-8") + match = re.search(r"SITES\s*=\s*\[(.*?)\]", text, re.DOTALL) + if not match: + return [] + values: list[str] = [] + for single, double in re.findall(r"'([^']+)'|\"([^\"]+)\"", match.group(1)): + values.append(single or double) + return values + + +def load_port_map(root: Path) -> tuple[dict[str, int], list[Finding]]: + websyn_path = root / "websyn_start.sh" + control_path = root / "control_server.py" + findings: list[Finding] = [] + websyn_sites = parse_websyn_sites(websyn_path) if websyn_path.exists() else [] + control_sites = parse_control_server_sites(control_path) if control_path.exists() else [] + if websyn_sites and control_sites and websyn_sites != control_sites: + findings.append( + Finding( + level="warning", + path=relative_to_root(control_path, root), + line=None, + code="registry-mismatch", + message="site order differs between websyn_start.sh and control_server.py; port validation may be unreliable", + ) + ) + sites = websyn_sites or control_sites + return {site: 40000 + idx for idx, site in enumerate(sites)}, findings + + +def parse_args(argv: list[str] | None = None) -> argparse.Namespace: + parser = argparse.ArgumentParser(description="Validate WebHarbor tasks.jsonl files.") + parser.add_argument("--site", help="Validate only sites//tasks.jsonl") + parser.add_argument("--tasks", help="Validate a specific tasks.jsonl file") + parser.add_argument("--strict", action="store_true", help="Treat warnings as failures") + parser.add_argument("--json", action="store_true", help="Emit machine-readable JSON") + args = parser.parse_args(argv) + if args.site and args.tasks: + parser.error("--site and --tasks are mutually exclusive") + return args + + +def discover_task_files(root: Path, site: str | None, tasks_path: str | None) -> list[Path]: + if tasks_path: + path = Path(tasks_path) + if not path.is_absolute(): + path = (root / path).resolve() + return [path] + if site: + return [root / "sites" / site / "tasks.jsonl"] + return sorted((root / "sites").glob("*/tasks.jsonl")) + + +def validate_non_empty_string( + obj: dict, + field: str, + path: Path, + line_no: int, + root: Path, + findings: list[Finding], + file_summaries: dict[Path, FileSummary], +) -> str | None: + value = obj.get(field) + if not isinstance(value, str) or not value.strip(): + add_finding( + findings, + file_summaries, + path, + root, + "error", + "missing-field", + f"required field '{field}' is missing or empty", + line=line_no, + ) + return None + return value.strip() + + +def validate_local_web( + value: str, + site_slug: str, + expected_port: int | None, + path: Path, + line_no: int, + root: Path, + findings: list[Finding], + file_summaries: dict[Path, FileSummary], +) -> None: + parsed = urlparse(value) + if parsed.scheme not in {"http", "https"}: + add_finding(findings, file_summaries, path, root, "error", "bad-web-url", "web must use http or https", line_no) + return + if parsed.hostname not in LOCAL_HOSTS: + add_finding( + findings, + file_summaries, + path, + root, + "error", + "bad-web-host", + "web must point to localhost or 127.0.0.1 rather than a live site", + line_no, + ) + if parsed.port is None: + add_finding(findings, file_summaries, path, root, "error", "missing-web-port", "web must include an explicit localhost port", line_no) + elif expected_port is not None and parsed.port != expected_port: + add_finding( + findings, + file_summaries, + path, + root, + "error", + "port-mismatch", + f"web port {parsed.port} does not match registered port {expected_port} for site '{site_slug}'", + line_no, + ) + + +def validate_upstream_url( + value: str, + path: Path, + line_no: int, + root: Path, + findings: list[Finding], + file_summaries: dict[Path, FileSummary], +) -> None: + parsed = urlparse(value) + if parsed.scheme not in {"http", "https"}: + add_finding(findings, file_summaries, path, root, "error", "bad-upstream-url", "upstream_url must use http or https", line_no) + return + if not parsed.netloc: + add_finding(findings, file_summaries, path, root, "error", "bad-upstream-url", "upstream_url must include a hostname", line_no) + return + if parsed.hostname in LOCAL_HOSTS: + add_finding( + findings, + file_summaries, + path, + root, + "error", + "bad-upstream-host", + "upstream_url must point to the real upstream site, not localhost", + line_no, + ) + + +def scan_question_quality( + question: str, + path: Path, + line_no: int, + root: Path, + findings: list[Finding], + file_summaries: dict[Path, FileSummary], +) -> None: + for pattern, message in ANSWER_LEAK_PATTERNS: + if pattern.search(question): + add_finding(findings, file_summaries, path, root, "warning", "answer-leak", message, line_no) + + if len(question.strip()) < 20: + add_finding( + findings, + file_summaries, + path, + root, + "warning", + "short-question", + "question is unusually short and may be underspecified", + line_no, + ) + + lower = question.lower() + if ("confirmation code" in lower or "booking code" in lower) and CONFIRMATION_CODE_PATTERN.search(question): + add_finding( + findings, + file_summaries, + path, + root, + "warning", + "answer-leak", + "question includes a code-like token that may leak the lookup target directly", + line_no, + ) + + for pattern, label in SUSPICIOUS_PATTERNS: + if pattern.search(question): + add_finding( + findings, + file_summaries, + path, + root, + "warning", + "suspicious-term", + f"question contains suspicious phrase '{label}'", + line_no, + ) + + for marker in BAD_MARKERS: + if marker in lower: + add_finding( + findings, + file_summaries, + path, + root, + "warning", + "bad-marker", + f"question contains marker '{marker}'", + line_no, + ) + + +def id_relates_to_site(task_id: str, site_slug: str, web_name: str) -> bool: + normalized_id = normalize_token(task_id) + return normalized_id.startswith(normalize_token(web_name)) or normalized_id.startswith(normalize_token(site_slug)) + + +def validate_file( + path: Path, + root: Path, + port_map: dict[str, int], + findings: list[Finding], + file_summaries: dict[Path, FileSummary], + id_occurrences: defaultdict[str, list[tuple[Path, int]]], +) -> None: + site_slug = path.parent.name + file_summaries[path] = FileSummary(path=relative_to_root(path, root), site=site_slug) + + if not path.exists(): + add_finding(findings, file_summaries, path, root, "error", "missing-file", "tasks file does not exist", None) + return + + seen_ids: dict[str, int] = {} + with path.open("r", encoding="utf-8") as handle: + for line_no, raw_line in enumerate(handle, 1): + if not raw_line.strip(): + continue + file_summaries[path].task_count += 1 + try: + obj = json.loads(raw_line) + except json.JSONDecodeError as exc: + add_finding( + findings, + file_summaries, + path, + root, + "error", + "invalid-json", + f"invalid JSON: {exc.msg}", + line_no, + ) + continue + if not isinstance(obj, dict): + add_finding(findings, file_summaries, path, root, "error", "wrong-type", "task line must decode to a JSON object", line_no) + continue + + values: dict[str, str | None] = {} + for field in REQUIRED_FIELDS: + values[field] = validate_non_empty_string(obj, field, path, line_no, root, findings, file_summaries) + + task_id = values["id"] + web_name = values["web_name"] + question = values["ques"] + web_url = values["web"] + upstream_url = values["upstream_url"] + + if task_id: + if task_id in seen_ids: + add_finding( + findings, + file_summaries, + path, + root, + "error", + "duplicate-id", + f"duplicate task id '{task_id}' in the same file (first seen on line {seen_ids[task_id]})", + line_no, + ) + else: + seen_ids[task_id] = line_no + id_occurrences[task_id].append((path, line_no)) + + if task_id and web_name and not id_relates_to_site(task_id, site_slug, web_name): + add_finding( + findings, + file_summaries, + path, + root, + "warning", + "id-convention", + f"task id '{task_id}' does not appear to relate to site slug '{site_slug}' or web_name '{web_name}'", + line_no, + ) + + if web_url: + validate_local_web(web_url, site_slug, port_map.get(site_slug), path, line_no, root, findings, file_summaries) + if upstream_url: + validate_upstream_url(upstream_url, path, line_no, root, findings, file_summaries) + if question: + scan_question_quality(question, path, line_no, root, findings, file_summaries) + + +def apply_cross_file_duplicate_checks( + id_occurrences: defaultdict[str, list[tuple[Path, int]]], + root: Path, + findings: list[Finding], + file_summaries: dict[Path, FileSummary], +) -> None: + for task_id, locations in sorted(id_occurrences.items()): + unique_locations = {(path.resolve(), line_no) for path, line_no in locations} + if len(unique_locations) <= 1: + continue + location_summary = ", ".join(f"{relative_to_root(path, root)}:{line_no}" for path, line_no in locations) + for path, line_no in locations: + add_finding( + findings, + file_summaries, + path, + root, + "error", + "duplicate-id-cross-site", + f"task id '{task_id}' is duplicated across files ({location_summary})", + line_no, + ) + + +def summarize( + root: Path, + files: list[Path], + findings: list[Finding], + file_summaries: dict[Path, FileSummary], + strict: bool, + port_findings: Iterable[Finding] = (), +) -> dict: + combined_findings = list(port_findings) + findings + errors = sum(1 for finding in combined_findings if finding.level == "error") + warnings = sum(1 for finding in combined_findings if finding.level == "warning") + total_tasks = sum(summary.task_count for summary in file_summaries.values()) + exit_code = 1 if errors or (strict and warnings) else 0 + return { + "root": str(root), + "sites_checked": len({summary.site for summary in file_summaries.values()}), + "task_files_checked": len(files), + "task_count": total_tasks, + "errors": errors, + "warnings": warnings, + "strict": strict, + "exit_code": exit_code, + "files": [asdict(file_summaries[path]) for path in sorted(file_summaries, key=lambda item: file_summaries[item].path)], + "findings": [asdict(finding) for finding in sorted(combined_findings, key=lambda item: (item.path, item.line or 0, item.level, item.code))], + } + + +def print_human(summary: dict) -> None: + print( + f"Checked {summary['sites_checked']} site(s), {summary['task_files_checked']} task file(s), " + f"{summary['task_count']} task(s)" + ) + print(f"Errors: {summary['errors']} Warnings: {summary['warnings']}") + print("") + for file_summary in summary["files"]: + label = "OK" if file_summary["errors"] == 0 else "FAIL" + print( + f"[{label}] {file_summary['path']}: " + f"tasks={file_summary['task_count']} errors={file_summary['errors']} warnings={file_summary['warnings']}" + ) + if summary["findings"]: + print("") + print("Findings:") + for finding in summary["findings"]: + location = finding["path"] + if finding["line"] is not None: + location = f"{location}:{finding['line']}" + print(f"- {finding['level'].upper()} {location} [{finding['code']}] {finding['message']}") + + +def run_validation( + *, + root: Path = DEFAULT_ROOT, + site: str | None = None, + tasks_path: str | None = None, + strict: bool = False, +) -> dict: + root = root.resolve() + files = discover_task_files(root, site, tasks_path) + findings: list[Finding] = [] + file_summaries: dict[Path, FileSummary] = {} + id_occurrences: defaultdict[str, list[tuple[Path, int]]] = defaultdict(list) + port_map, port_findings = load_port_map(root) + + if not files: + missing_path = root / "sites" + findings.append( + Finding( + level="error", + path=relative_to_root(missing_path, root), + line=None, + code="no-task-files", + message="no tasks.jsonl files were found for the requested scope", + ) + ) + return summarize(root, files, findings, file_summaries, strict, port_findings=port_findings) + + for path in files: + validate_file(path.resolve(), root, port_map, findings, file_summaries, id_occurrences) + + apply_cross_file_duplicate_checks(id_occurrences, root, findings, file_summaries) + return summarize(root, files, findings, file_summaries, strict, port_findings=port_findings) + + +def main(argv: list[str] | None = None, *, root: Path = DEFAULT_ROOT) -> int: + args = parse_args(argv) + summary = run_validation(root=root, site=args.site, tasks_path=args.tasks, strict=args.strict) + if args.json: + print(json.dumps(summary, indent=2, sort_keys=True)) + else: + print_human(summary) + return int(summary["exit_code"]) + + +if __name__ == "__main__": + raise SystemExit(main()) From 142bae2c32c4f3fc8b1ceae51b1b63511b401f7d Mon Sep 17 00:00:00 2001 From: JackJin <1037461232@qq.com> Date: Thu, 10 Sep 2026 14:02:39 +0800 Subject: [PATCH 2/4] fix(tasks): harden task validation contract --- README.md | 3 +- scripts/test_validate_tasks.py | 425 ++++++++++++++++++- scripts/validate_tasks.py | 726 ++++++++++++++++++++++++++++----- 3 files changed, 1029 insertions(+), 125 deletions(-) diff --git a/README.md b/README.md index 973837c0..3752be70 100644 --- a/README.md +++ b/README.md @@ -101,11 +101,12 @@ Any other improvement — bug fixes, UI polish, data enrichment, task suggestion ## Validate Tasks -Use the repository task validator to check task JSONL files and localhost metadata before opening a review or PR: +Use the repository task validator to check task JSONL files, site registration, localhost ports, and optional reviewer grading metadata before opening a review or PR. Ground-truth fields in agent-facing task rows are rejected; unrecognized metadata is reported as a warning and fails under `--strict`. ```bash python scripts/validate_tasks.py python scripts/validate_tasks.py --site amazon +python scripts/validate_tasks.py --tasks sites/amazon/tasks.jsonl python scripts/validate_tasks.py --strict python scripts/validate_tasks.py --json ``` diff --git a/scripts/test_validate_tasks.py b/scripts/test_validate_tasks.py index 03290dd3..f48a6a4f 100644 --- a/scripts/test_validate_tasks.py +++ b/scripts/test_validate_tasks.py @@ -5,6 +5,7 @@ import io import json +import shutil import sys import tempfile import unittest @@ -22,27 +23,51 @@ def task_line( web: str = "http://localhost:40000/", upstream_url: str = "https://example.com/demo", ques: str = "Find the lowest priced demo item with at least two filters applied.", + **extra: object, ) -> str: - return json.dumps( - { - "id": task_id, - "web_name": web_name, - "web": web, - "upstream_url": upstream_url, - "ques": ques, - } - ) + task: dict[str, object] = { + "id": task_id, + "web_name": web_name, + "web": web, + "upstream_url": upstream_url, + "ques": ques, + } + task.update(extra) + return json.dumps(task) class ValidateTasksTests(unittest.TestCase): - def make_root(self, task_contents: str) -> Path: + def make_root( + self, + task_contents: str, + *, + site: str = "demo", + websyn_sites: tuple[str, ...] = ("demo",), + control_sites: tuple[str, ...] = ("demo",), + ) -> Path: temp_root = Path(tempfile.mkdtemp(prefix="validate-tasks-")) - (temp_root / "sites" / "demo").mkdir(parents=True) - (temp_root / "sites" / "demo" / "tasks.jsonl").write_text(task_contents, encoding="utf-8") - (temp_root / "websyn_start.sh").write_text("#!/bin/bash\nSITES=(demo other)\n", encoding="utf-8") - (temp_root / "control_server.py").write_text("SITES = ['demo', 'other']\n", encoding="utf-8") + self.addCleanup(shutil.rmtree, temp_root) + (temp_root / "sites" / site).mkdir(parents=True) + (temp_root / "sites" / site / "tasks.jsonl").write_text( + task_contents, encoding="utf-8" + ) + (temp_root / "websyn_start.sh").write_text( + "#!/bin/bash\nSITES=(" + " ".join(websyn_sites) + ")\n", + encoding="utf-8", + ) + (temp_root / "control_server.py").write_text( + "SITES = " + repr(list(control_sites)) + "\n", + encoding="utf-8", + ) return temp_root + def add_verifier( + self, root: Path, path: str = "sites/demo/verify/verify_0.py" + ) -> None: + verifier = root / path + verifier.parent.mkdir(parents=True, exist_ok=True) + verifier.write_text("#!/usr/bin/env python3\n", encoding="utf-8") + def test_valid_jsonl_passes(self) -> None: root = self.make_root(task_line() + "\n") summary = vt.run_validation(root=root) @@ -58,20 +83,39 @@ def test_malformed_jsonl_fails(self) -> None: self.assertEqual(summary["exit_code"], 1) def test_duplicate_ids_fail(self) -> None: - root = self.make_root(task_line(task_id="Demo--1") + "\n" + task_line(task_id="Demo--1") + "\n") + root = self.make_root( + task_line(task_id="Demo--1") + "\n" + task_line(task_id="Demo--1") + "\n" + ) summary = vt.run_validation(root=root) self.assertGreater(summary["errors"], 0) - self.assertTrue(any(f["code"] == "duplicate-id-cross-site" or f["code"] == "duplicate-id" for f in summary["findings"])) + self.assertTrue( + any( + f["code"] == "duplicate-id-cross-site" or f["code"] == "duplicate-id" + for f in summary["findings"] + ) + ) def test_missing_required_field_fails(self) -> None: - bad_line = json.dumps({"id": "Demo--2", "web_name": "Demo", "web": "http://localhost:40000/", "ques": "Missing upstream."}) + bad_line = json.dumps( + { + "id": "Demo--2", + "web_name": "Demo", + "web": "http://localhost:40000/", + "ques": "Missing upstream.", + } + ) root = self.make_root(bad_line + "\n") summary = vt.run_validation(root=root) self.assertGreater(summary["errors"], 0) self.assertTrue(any(f["code"] == "missing-field" for f in summary["findings"])) def test_warnings_do_not_fail_unless_strict(self) -> None: - root = self.make_root(task_line(task_id="Demo--3", ques="The answer is already visible on the page.") + "\n") + root = self.make_root( + task_line( + task_id="Demo--3", ques="The answer is already visible on the page." + ) + + "\n" + ) non_strict = vt.run_validation(root=root, strict=False) strict = vt.run_validation(root=root, strict=True) self.assertEqual(non_strict["errors"], 0) @@ -89,6 +133,351 @@ def test_json_output_is_valid_json(self) -> None: self.assertEqual(payload["task_count"], 1) self.assertIn("files", payload) + def test_invalid_ports_are_reported_without_crashing(self) -> None: + for port in ("not-a-port", "70000"): + with self.subTest(port=port): + root = self.make_root(task_line(web=f"http://localhost:{port}/") + "\n") + summary = vt.run_validation(root=root) + self.assertEqual(summary["exit_code"], 1) + self.assertTrue( + any(f["code"] == "bad-web-port" for f in summary["findings"]) + ) + + def test_empty_task_file_fails(self) -> None: + root = self.make_root("") + summary = vt.run_validation(root=root) + self.assertEqual(summary["exit_code"], 1) + self.assertTrue( + any(f["code"] == "empty-task-file" for f in summary["findings"]) + ) + + def test_registered_site_without_task_file_fails_full_scan(self) -> None: + root = self.make_root( + task_line() + "\n", + websyn_sites=("demo", "other"), + control_sites=("demo", "other"), + ) + summary = vt.run_validation(root=root) + self.assertEqual(summary["exit_code"], 1) + self.assertTrue( + any( + finding["code"] == "missing-file" + and finding["path"] == "sites/other/tasks.jsonl" + for finding in summary["findings"] + ) + ) + + def test_unregistered_site_fails(self) -> None: + root = self.make_root( + task_line(web="http://localhost:49999/") + "\n", + site="rogue", + ) + summary = vt.run_validation(root=root) + self.assertEqual(summary["exit_code"], 1) + self.assertTrue( + any(f["code"] == "unregistered-site" for f in summary["findings"]) + ) + + def test_registry_mismatch_is_an_error(self) -> None: + root = self.make_root( + task_line() + "\n", + control_sites=("other", "demo"), + ) + summary = vt.run_validation(root=root) + self.assertEqual(summary["exit_code"], 1) + finding = next( + f for f in summary["findings"] if f["code"] == "registry-mismatch" + ) + self.assertEqual(finding["level"], "error") + + def test_duplicate_registry_entries_fail(self) -> None: + root = self.make_root( + task_line(web="http://localhost:40001/") + "\n", + websyn_sites=("demo", "demo"), + control_sites=("demo", "demo"), + ) + summary = vt.run_validation(root=root) + self.assertEqual(summary["exit_code"], 1) + self.assertTrue( + any(f["code"] == "registry-duplicate" for f in summary["findings"]) + ) + + def test_missing_registries_fail(self) -> None: + root = self.make_root(task_line() + "\n") + (root / "websyn_start.sh").unlink() + (root / "control_server.py").unlink() + summary = vt.run_validation(root=root) + self.assertEqual(summary["exit_code"], 1) + self.assertTrue( + any(f["code"] == "registry-missing" for f in summary["findings"]) + ) + + def test_invalid_utf8_is_reported_without_crashing(self) -> None: + root = self.make_root(task_line() + "\n") + (root / "sites" / "demo" / "tasks.jsonl").write_bytes(b"\xff\n") + summary = vt.run_validation(root=root) + self.assertEqual(summary["exit_code"], 1) + self.assertTrue( + any(f["code"] == "unreadable-file" for f in summary["findings"]) + ) + + def test_ground_truth_fields_fail(self) -> None: + for field in ("answer", "expected_answer", "ground_truth"): + with self.subTest(field=field): + root = self.make_root(task_line(**{field: "SECRET"}) + "\n") + summary = vt.run_validation(root=root) + self.assertEqual(summary["exit_code"], 1) + self.assertTrue( + any( + f["code"] == "forbidden-answer-field" + for f in summary["findings"] + ) + ) + + def test_unknown_field_warns_and_strict_mode_fails(self) -> None: + root = self.make_root(task_line(future_metadata="value") + "\n") + non_strict = vt.run_validation(root=root) + strict = vt.run_validation(root=root, strict=True) + self.assertEqual(non_strict["errors"], 0) + self.assertEqual(non_strict["exit_code"], 0) + self.assertTrue( + any(f["code"] == "unexpected-field" for f in non_strict["findings"]) + ) + self.assertEqual(strict["exit_code"], 1) + + def test_human_labels_reflect_warning_and_strict_failure(self) -> None: + root = self.make_root(task_line(future_metadata="value") + "\n") + non_strict = vt.run_validation(root=root) + strict = vt.run_validation(root=root, strict=True) + non_strict_output = io.StringIO() + strict_output = io.StringIO() + with redirect_stdout(non_strict_output): + vt.print_human(non_strict) + with redirect_stdout(strict_output): + vt.print_human(strict) + self.assertIn("[WARN] sites/demo/tasks.jsonl", non_strict_output.getvalue()) + self.assertIn("[FAIL] sites/demo/tasks.jsonl", strict_output.getvalue()) + + def test_quality_markers_do_not_match_inside_legitimate_words(self) -> None: + root = self.make_root( + task_line( + ques="Open the Mastodon profile for Victoria's Secret and report its visible identifier." + ) + + "\n" + ) + summary = vt.run_validation(root=root, strict=True) + self.assertEqual(summary["errors"], 0) + self.assertEqual(summary["warnings"], 0) + self.assertEqual(summary["exit_code"], 0) + + def test_explicit_placeholder_and_client_secret_still_warn(self) -> None: + root = self.make_root( + task_line( + ques="Replace the TODO placeholder and paste the client secret into the production form." + ) + + "\n" + ) + summary = vt.run_validation(root=root) + codes = [finding["code"] for finding in summary["findings"]] + self.assertIn("bad-marker", codes) + self.assertIn("suspicious-term", codes) + + def test_grading_fields_must_be_a_pair(self) -> None: + for extra in ( + {"verifier_path": "sites/demo/verify/verify_0.py"}, + {"judge_rubric": "Require a detail-page visit and a non-empty answer."}, + ): + with self.subTest(extra=extra): + root = self.make_root(task_line(**extra) + "\n") + summary = vt.run_validation(root=root) + self.assertEqual(summary["exit_code"], 1) + self.assertTrue( + any( + f["code"] == "incomplete-grading-contract" + for f in summary["findings"] + ) + ) + + def test_valid_grading_contract_passes(self) -> None: + root = self.make_root( + task_line( + verifier_path="sites/demo/verify/verify_0.py", + judge_rubric="Require a detail-page visit and a non-empty answer.", + ) + + "\n" + ) + self.add_verifier(root) + summary = vt.run_validation(root=root) + self.assertEqual(summary["errors"], 0) + self.assertEqual(summary["warnings"], 0) + + def test_verifier_must_exist_under_the_same_site(self) -> None: + cases = ( + "sites/demo/verify/missing.py", + "/tmp/verify_0.py", + "sites/demo/verify/../verify_0.py", + "sites/other/verify/verify_0.py", + ) + for verifier_path in cases: + with self.subTest(verifier_path=verifier_path): + root = self.make_root( + task_line( + verifier_path=verifier_path, + judge_rubric="Require a detail-page visit and a non-empty answer.", + ) + + "\n" + ) + summary = vt.run_validation(root=root) + self.assertEqual(summary["exit_code"], 1) + self.assertTrue( + any( + f["code"].startswith("bad-verifier") + for f in summary["findings"] + ) + ) + + def test_two_tasks_cannot_share_one_verifier(self) -> None: + rubric = "Require a detail-page visit and a non-empty answer." + root = self.make_root( + task_line( + task_id="Demo--0", + verifier_path="sites/demo/verify/verify_0.py", + judge_rubric=rubric, + ) + + "\n" + + task_line( + task_id="Demo--1", + verifier_path="sites/demo/verify/verify_0.py", + judge_rubric=rubric, + ) + + "\n" + ) + self.add_verifier(root) + summary = vt.run_validation(root=root) + self.assertEqual(summary["exit_code"], 1) + self.assertTrue( + any(f["code"] == "duplicate-verifier" for f in summary["findings"]) + ) + + def test_task_identity_matches_site_and_numeric_suffix(self) -> None: + cases = ( + {"task_id": "DemoExtra--0"}, + {"task_id": "Demo--not-a-number"}, + {"task_id": "Other--0", "web_name": "Other"}, + ) + for overrides in cases: + with self.subTest(overrides=overrides): + root = self.make_root(task_line(**overrides) + "\n") + summary = vt.run_validation(root=root) + self.assertEqual(summary["exit_code"], 1) + self.assertTrue( + any(f["code"] == "bad-task-identity" for f in summary["findings"]) + ) + + def test_acronym_site_identity_passes(self) -> None: + root = self.make_root( + task_line( + task_id="Ohio State University--0", + web_name="Ohio State University", + ) + + "\n", + site="osu", + websyn_sites=("osu",), + control_sites=("osu",), + ) + summary = vt.run_validation(root=root) + self.assertEqual(summary["errors"], 0) + self.assertEqual(summary["warnings"], 0) + + def test_web_name_is_consistent_within_a_task_file(self) -> None: + root = self.make_root( + task_line( + task_id="Ohio State University--0", + web_name="Ohio State University", + ) + + "\n" + + task_line(task_id="OSU--1", web_name="OSU") + + "\n", + site="osu", + websyn_sites=("osu",), + control_sites=("osu",), + ) + summary = vt.run_validation(root=root) + self.assertEqual(summary["exit_code"], 1) + self.assertTrue( + any(f["code"] == "inconsistent-web-name" for f in summary["findings"]) + ) + + def test_same_file_duplicate_does_not_claim_cross_site_duplication(self) -> None: + root = self.make_root(task_line() + "\n" + task_line() + "\n") + summary = vt.run_validation(root=root) + codes = [finding["code"] for finding in summary["findings"]] + self.assertIn("duplicate-id", codes) + self.assertNotIn("duplicate-id-cross-site", codes) + + def test_true_cross_file_duplicate_is_reported(self) -> None: + root = self.make_root( + task_line() + "\n", + websyn_sites=("demo", "other"), + control_sites=("demo", "other"), + ) + other = root / "sites" / "other" + other.mkdir() + (other / "tasks.jsonl").write_text( + task_line(web="http://localhost:40001/") + "\n", + encoding="utf-8", + ) + summary = vt.run_validation(root=root) + self.assertTrue( + any(f["code"] == "duplicate-id-cross-site" for f in summary["findings"]) + ) + + def test_registry_parsers_ignore_comments(self) -> None: + root = self.make_root( + task_line( + task_id="Other--0", + web_name="Other", + web="http://localhost:40001/", + ) + + "\n", + site="other", + ) + (root / "websyn_start.sh").write_text( + "#!/bin/bash\nSITES=(demo # primary site\n other)\n", + encoding="utf-8", + ) + (root / "control_server.py").write_text( + "SITES = ['demo', # 'phantom'\n 'other']\n", + encoding="utf-8", + ) + summary = vt.run_validation(root=root, site="other") + self.assertEqual(summary["errors"], 0) + self.assertEqual(summary["warnings"], 0) + + def test_websyn_parser_ignores_commented_assignment(self) -> None: + root = self.make_root(task_line() + "\n") + (root / "websyn_start.sh").write_text( + "#!/bin/bash\n# SITES=(phantom)\nSITES=(demo)\n", + encoding="utf-8", + ) + summary = vt.run_validation(root=root) + self.assertEqual(summary["errors"], 0) + self.assertEqual(summary["warnings"], 0) + + def test_upstream_url_rejects_all_loopback_and_missing_hosts(self) -> None: + for upstream_url in ( + "http://127.0.0.2:8080/", + "http://[::1]:8080/", + "https://:443/", + ): + with self.subTest(upstream_url=upstream_url): + root = self.make_root(task_line(upstream_url=upstream_url) + "\n") + summary = vt.run_validation(root=root) + self.assertEqual(summary["exit_code"], 1) + self.assertTrue( + any(f["code"] == "bad-upstream-host" for f in summary["findings"]) + ) + if __name__ == "__main__": unittest.main() diff --git a/scripts/validate_tasks.py b/scripts/validate_tasks.py index 133ecff1..d93fa521 100644 --- a/scripts/validate_tasks.py +++ b/scripts/validate_tasks.py @@ -4,9 +4,11 @@ from __future__ import annotations import argparse +import ast +import ipaddress import json import re -import sys +import shlex from collections import defaultdict from dataclasses import asdict, dataclass from pathlib import Path @@ -14,24 +16,39 @@ from urllib.parse import urlparse REQUIRED_FIELDS = ("id", "web_name", "web", "upstream_url", "ques") +GRADING_FIELDS = ("verifier_path", "judge_rubric") +ALLOWED_FIELDS = frozenset((*REQUIRED_FIELDS, *GRADING_FIELDS)) +FORBIDDEN_ANSWER_FIELDS = frozenset(("answer", "expected_answer", "ground_truth")) LOCAL_HOSTS = {"localhost", "127.0.0.1"} SUSPICIOUS_PATTERNS = ( (re.compile(r"\breal payment\b", re.IGNORECASE), "real payment"), (re.compile(r"\breal booking\b", re.IGNORECASE), "real booking"), (re.compile(r"\blive api\b", re.IGNORECASE), "live api"), (re.compile(r"\bapi[_ -]?key\b", re.IGNORECASE), "api_key"), - (re.compile(r"\bsecret\b", re.IGNORECASE), "secret"), + ( + re.compile(r"\b(?:client secret|secret key)\b", re.IGNORECASE), + "secret credential", + ), (re.compile(r"\baccess token\b", re.IGNORECASE), "access token"), (re.compile(r"\bpassword leak\b", re.IGNORECASE), "password leak"), - (re.compile(r"\bproduction (environment|system|server)\b", re.IGNORECASE), "production environment"), + ( + re.compile(r"\bproduction (environment|system|server)\b", re.IGNORECASE), + "production environment", + ), (re.compile(r"\bexternal runtime call\b", re.IGNORECASE), "external runtime call"), (re.compile(r"\bcredit card\b", re.IGNORECASE), "credit card"), (re.compile(r"\bssn\b", re.IGNORECASE), "ssn"), - (re.compile(r"\bsocial security number\b", re.IGNORECASE), "social security number"), + ( + re.compile(r"\bsocial security number\b", re.IGNORECASE), + "social security number", + ), ) BAD_MARKERS = ("todo", "fixme", "lorem", "placeholder") ANSWER_LEAK_PATTERNS = ( - (re.compile(r"\bthe answer is\b", re.IGNORECASE), "question appears to reveal the answer directly"), + ( + re.compile(r"\bthe answer is\b", re.IGNORECASE), + "question appears to reveal the answer directly", + ), ( re.compile(r"\bselect the option named exactly\b", re.IGNORECASE), "question may leak the exact UI text of the answer", @@ -80,7 +97,15 @@ def add_finding( message: str, line: int | None = None, ) -> None: - findings.append(Finding(level=level, path=relative_to_root(path, root), line=line, code=code, message=message)) + findings.append( + Finding( + level=level, + path=relative_to_root(path, root), + line=line, + code=code, + message=message, + ) + ) if path in file_summaries: if level == "error": file_summaries[path].errors += 1 @@ -90,37 +115,97 @@ def add_finding( def parse_websyn_sites(path: Path) -> list[str]: text = path.read_text(encoding="utf-8") - match = re.search(r"SITES=\((.*?)\)", text, re.DOTALL) + match = re.search(r"^[ \t]*SITES=\((.*?)\)", text, re.DOTALL | re.MULTILINE) if not match: return [] - return re.findall(r"[A-Za-z0-9_]+", match.group(1)) + try: + tokens = shlex.split(match.group(1), comments=True, posix=True) + except ValueError: + return [] + return [token for token in tokens if re.fullmatch(r"[A-Za-z0-9_]+", token)] def parse_control_server_sites(path: Path) -> list[str]: - text = path.read_text(encoding="utf-8") - match = re.search(r"SITES\s*=\s*\[(.*?)\]", text, re.DOTALL) - if not match: + try: + tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) + except (OSError, SyntaxError, UnicodeError): + return [] + for node in tree.body: + value_node = None + if isinstance(node, ast.Assign) and any( + isinstance(target, ast.Name) and target.id == "SITES" + for target in node.targets + ): + value_node = node.value + elif ( + isinstance(node, ast.AnnAssign) + and isinstance(node.target, ast.Name) + and node.target.id == "SITES" + ): + value_node = node.value + if value_node is None: + continue + try: + value = ast.literal_eval(value_node) + except (ValueError, TypeError): + return [] + if isinstance(value, (list, tuple)) and all( + isinstance(site, str) for site in value + ): + return list(value) return [] - values: list[str] = [] - for single, double in re.findall(r"'([^']+)'|\"([^\"]+)\"", match.group(1)): - values.append(single or double) - return values + return [] def load_port_map(root: Path) -> tuple[dict[str, int], list[Finding]]: websyn_path = root / "websyn_start.sh" control_path = root / "control_server.py" findings: list[Finding] = [] - websyn_sites = parse_websyn_sites(websyn_path) if websyn_path.exists() else [] - control_sites = parse_control_server_sites(control_path) if control_path.exists() else [] - if websyn_sites and control_sites and websyn_sites != control_sites: + try: + websyn_sites = parse_websyn_sites(websyn_path) if websyn_path.exists() else [] + except (OSError, UnicodeError): + websyn_sites = [] + control_sites = ( + parse_control_server_sites(control_path) if control_path.exists() else [] + ) + for registry_path, sites in ( + (websyn_path, websyn_sites), + (control_path, control_sites), + ): + duplicates = sorted({site for site in sites if sites.count(site) > 1}) + if duplicates: + findings.append( + Finding( + level="error", + path=relative_to_root(registry_path, root), + line=None, + code="registry-duplicate", + message=f"SITES registry contains duplicate entries: {', '.join(duplicates)}", + ) + ) + if not websyn_sites or not control_sites: + missing = [] + if not websyn_sites: + missing.append("websyn_start.sh") + if not control_sites: + missing.append("control_server.py") + findings.append( + Finding( + level="error", + path=relative_to_root(root, root), + line=None, + code="registry-missing", + message=f"could not read a non-empty SITES registry from {', '.join(missing)}", + ) + ) + elif websyn_sites != control_sites: findings.append( Finding( - level="warning", + level="error", path=relative_to_root(control_path, root), line=None, code="registry-mismatch", - message="site order differs between websyn_start.sh and control_server.py; port validation may be unreliable", + message="site order differs between websyn_start.sh and control_server.py; port validation is unreliable", ) ) sites = websyn_sites or control_sites @@ -128,18 +213,26 @@ def load_port_map(root: Path) -> tuple[dict[str, int], list[Finding]]: def parse_args(argv: list[str] | None = None) -> argparse.Namespace: - parser = argparse.ArgumentParser(description="Validate WebHarbor tasks.jsonl files.") + parser = argparse.ArgumentParser( + description="Validate WebHarbor tasks.jsonl files." + ) parser.add_argument("--site", help="Validate only sites//tasks.jsonl") parser.add_argument("--tasks", help="Validate a specific tasks.jsonl file") - parser.add_argument("--strict", action="store_true", help="Treat warnings as failures") - parser.add_argument("--json", action="store_true", help="Emit machine-readable JSON") + parser.add_argument( + "--strict", action="store_true", help="Treat warnings as failures" + ) + parser.add_argument( + "--json", action="store_true", help="Emit machine-readable JSON" + ) args = parser.parse_args(argv) if args.site and args.tasks: parser.error("--site and --tasks are mutually exclusive") return args -def discover_task_files(root: Path, site: str | None, tasks_path: str | None) -> list[Path]: +def discover_task_files( + root: Path, site: str | None, tasks_path: str | None +) -> list[Path]: if tasks_path: path = Path(tasks_path) if not path.is_absolute(): @@ -185,11 +278,34 @@ def validate_local_web( findings: list[Finding], file_summaries: dict[Path, FileSummary], ) -> None: - parsed = urlparse(value) + try: + parsed = urlparse(value) + hostname = parsed.hostname + except ValueError as error: + add_finding( + findings, + file_summaries, + path, + root, + "error", + "bad-web-url", + f"web is not a valid URL: {error}", + line_no, + ) + return if parsed.scheme not in {"http", "https"}: - add_finding(findings, file_summaries, path, root, "error", "bad-web-url", "web must use http or https", line_no) + add_finding( + findings, + file_summaries, + path, + root, + "error", + "bad-web-url", + "web must use http or https", + line_no, + ) return - if parsed.hostname not in LOCAL_HOSTS: + if hostname is None or hostname.lower() not in LOCAL_HOSTS: add_finding( findings, file_summaries, @@ -200,9 +316,32 @@ def validate_local_web( "web must point to localhost or 127.0.0.1 rather than a live site", line_no, ) - if parsed.port is None: - add_finding(findings, file_summaries, path, root, "error", "missing-web-port", "web must include an explicit localhost port", line_no) - elif expected_port is not None and parsed.port != expected_port: + try: + port = parsed.port + except ValueError as error: + add_finding( + findings, + file_summaries, + path, + root, + "error", + "bad-web-port", + f"web has an invalid port: {error}", + line_no, + ) + return + if port is None: + add_finding( + findings, + file_summaries, + path, + root, + "error", + "missing-web-port", + "web must include an explicit localhost port", + line_no, + ) + elif expected_port is not None and port != expected_port: add_finding( findings, file_summaries, @@ -210,7 +349,7 @@ def validate_local_web( root, "error", "port-mismatch", - f"web port {parsed.port} does not match registered port {expected_port} for site '{site_slug}'", + f"web port {port} does not match registered port {expected_port} for site '{site_slug}'", line_no, ) @@ -223,14 +362,57 @@ def validate_upstream_url( findings: list[Finding], file_summaries: dict[Path, FileSummary], ) -> None: - parsed = urlparse(value) + try: + parsed = urlparse(value) + hostname = parsed.hostname + parsed.port + except ValueError as error: + add_finding( + findings, + file_summaries, + path, + root, + "error", + "bad-upstream-url", + f"upstream_url is not a valid URL: {error}", + line_no, + ) + return if parsed.scheme not in {"http", "https"}: - add_finding(findings, file_summaries, path, root, "error", "bad-upstream-url", "upstream_url must use http or https", line_no) + add_finding( + findings, + file_summaries, + path, + root, + "error", + "bad-upstream-url", + "upstream_url must use http or https", + line_no, + ) return - if not parsed.netloc: - add_finding(findings, file_summaries, path, root, "error", "bad-upstream-url", "upstream_url must include a hostname", line_no) + if not parsed.netloc or hostname is None: + add_finding( + findings, + file_summaries, + path, + root, + "error", + "bad-upstream-host", + "upstream_url must include a hostname", + line_no, + ) return - if parsed.hostname in LOCAL_HOSTS: + normalized_hostname = hostname.lower().rstrip(".") + is_local = normalized_hostname in LOCAL_HOSTS or normalized_hostname.endswith( + ".localhost" + ) + try: + address = ipaddress.ip_address(normalized_hostname) + except ValueError: + address = None + if address is not None and (address.is_loopback or address.is_unspecified): + is_local = True + if is_local: add_finding( findings, file_summaries, @@ -253,7 +435,16 @@ def scan_question_quality( ) -> None: for pattern, message in ANSWER_LEAK_PATTERNS: if pattern.search(question): - add_finding(findings, file_summaries, path, root, "warning", "answer-leak", message, line_no) + add_finding( + findings, + file_summaries, + path, + root, + "warning", + "answer-leak", + message, + line_no, + ) if len(question.strip()) < 20: add_finding( @@ -268,7 +459,9 @@ def scan_question_quality( ) lower = question.lower() - if ("confirmation code" in lower or "booking code" in lower) and CONFIRMATION_CODE_PATTERN.search(question): + if ( + "confirmation code" in lower or "booking code" in lower + ) and CONFIRMATION_CODE_PATTERN.search(question): add_finding( findings, file_summaries, @@ -294,7 +487,7 @@ def scan_question_quality( ) for marker in BAD_MARKERS: - if marker in lower: + if re.search(rf"\b{re.escape(marker)}\b", lower): add_finding( findings, file_summaries, @@ -307,9 +500,164 @@ def scan_question_quality( ) -def id_relates_to_site(task_id: str, site_slug: str, web_name: str) -> bool: - normalized_id = normalize_token(task_id) - return normalized_id.startswith(normalize_token(web_name)) or normalized_id.startswith(normalize_token(site_slug)) +def task_identity_is_valid(task_id: str, site_slug: str, web_name: str) -> bool: + normalized_site = normalize_token(site_slug) + normalized_name = normalize_token(web_name) + words = re.findall(r"[A-Za-z0-9]+", web_name) + initialism = "".join(word[0] for word in words).lower() + if normalized_name != normalized_site and initialism != normalized_site: + return False + return re.fullmatch(rf"{re.escape(web_name)}--[0-9]+", task_id) is not None + + +def validate_verifier_path( + value: object, + site_slug: str, + path: Path, + line_no: int, + root: Path, + findings: list[Finding], + file_summaries: dict[Path, FileSummary], +) -> None: + if not isinstance(value, str) or not value.strip(): + add_finding( + findings, + file_summaries, + path, + root, + "error", + "bad-verifier-path", + "verifier_path must be a non-empty repository-relative path", + line_no, + ) + return + + verifier_path = Path(value.strip()) + expected_prefix = ("sites", site_slug, "verify") + if ( + verifier_path.is_absolute() + or ".." in verifier_path.parts + or verifier_path.parts[:3] != expected_prefix + or len(verifier_path.parts) < 4 + or verifier_path.suffix != ".py" + ): + add_finding( + findings, + file_summaries, + path, + root, + "error", + "bad-verifier-path", + f"verifier_path must point to a Python file under sites/{site_slug}/verify/", + line_no, + ) + return + + expected_root = (root / "sites" / site_slug / "verify").resolve() + resolved = (root / verifier_path).resolve() + try: + resolved.relative_to(expected_root) + except ValueError: + add_finding( + findings, + file_summaries, + path, + root, + "error", + "bad-verifier-path", + f"verifier_path escapes sites/{site_slug}/verify/", + line_no, + ) + return + if not resolved.is_file(): + add_finding( + findings, + file_summaries, + path, + root, + "error", + "bad-verifier-missing", + f"verifier_path does not name an existing file: {value.strip()}", + line_no, + ) + + +def validate_task_schema( + obj: dict, + site_slug: str, + path: Path, + line_no: int, + root: Path, + findings: list[Finding], + file_summaries: dict[Path, FileSummary], +) -> None: + for field in sorted(obj): + normalized = normalize_token(field) + if ( + field.lower() in FORBIDDEN_ANSWER_FIELDS + or "answer" in normalized + or "groundtruth" in normalized + ): + add_finding( + findings, + file_summaries, + path, + root, + "error", + "forbidden-answer-field", + f"agent-facing task rows must not contain ground-truth field '{field}'", + line_no, + ) + elif field not in ALLOWED_FIELDS: + add_finding( + findings, + file_summaries, + path, + root, + "warning", + "unexpected-field", + f"task contains field '{field}' outside the current contributor/reviewer schema", + line_no, + ) + + has_verifier = "verifier_path" in obj + has_rubric = "judge_rubric" in obj + if has_verifier != has_rubric: + add_finding( + findings, + file_summaries, + path, + root, + "error", + "incomplete-grading-contract", + "verifier_path and judge_rubric must either both be present or both be absent", + line_no, + ) + return + if not has_verifier: + return + + validate_verifier_path( + obj.get("verifier_path"), + site_slug, + path, + line_no, + root, + findings, + file_summaries, + ) + rubric = obj.get("judge_rubric") + if not isinstance(rubric, str) or not rubric.strip(): + add_finding( + findings, + file_summaries, + path, + root, + "error", + "bad-judge-rubric", + "judge_rubric must be a non-empty string", + line_no, + ) def validate_file( @@ -321,80 +669,211 @@ def validate_file( id_occurrences: defaultdict[str, list[tuple[Path, int]]], ) -> None: site_slug = path.parent.name - file_summaries[path] = FileSummary(path=relative_to_root(path, root), site=site_slug) + file_summaries[path] = FileSummary( + path=relative_to_root(path, root), site=site_slug + ) if not path.exists(): - add_finding(findings, file_summaries, path, root, "error", "missing-file", "tasks file does not exist", None) + add_finding( + findings, + file_summaries, + path, + root, + "error", + "missing-file", + "tasks file does not exist", + None, + ) + return + + if site_slug not in port_map: + add_finding( + findings, + file_summaries, + path, + root, + "error", + "unregistered-site", + f"site '{site_slug}' is not registered in the WebHarbor site registries", + None, + ) + + try: + raw_lines = path.read_text(encoding="utf-8").splitlines() + except (OSError, UnicodeError) as error: + add_finding( + findings, + file_summaries, + path, + root, + "error", + "unreadable-file", + f"could not read tasks file as UTF-8 text: {error}", + None, + ) return seen_ids: dict[str, int] = {} - with path.open("r", encoding="utf-8") as handle: - for line_no, raw_line in enumerate(handle, 1): - if not raw_line.strip(): - continue - file_summaries[path].task_count += 1 - try: - obj = json.loads(raw_line) - except json.JSONDecodeError as exc: + seen_verifiers: dict[str, int] = {} + expected_web_name: str | None = None + for line_no, raw_line in enumerate(raw_lines, 1): + if not raw_line.strip(): + continue + file_summaries[path].task_count += 1 + try: + obj = json.loads(raw_line) + except json.JSONDecodeError as exc: + add_finding( + findings, + file_summaries, + path, + root, + "error", + "invalid-json", + f"invalid JSON: {exc.msg}", + line_no, + ) + continue + if not isinstance(obj, dict): + add_finding( + findings, + file_summaries, + path, + root, + "error", + "wrong-type", + "task line must decode to a JSON object", + line_no, + ) + continue + + validate_task_schema( + obj, site_slug, path, line_no, root, findings, file_summaries + ) + + values: dict[str, str | None] = {} + for field in REQUIRED_FIELDS: + values[field] = validate_non_empty_string( + obj, + field, + path, + line_no, + root, + findings, + file_summaries, + ) + + task_id = values["id"] + web_name = values["web_name"] + question = values["ques"] + web_url = values["web"] + upstream_url = values["upstream_url"] + + if web_name: + if expected_web_name is None: + expected_web_name = web_name + elif web_name != expected_web_name: add_finding( findings, file_summaries, path, root, "error", - "invalid-json", - f"invalid JSON: {exc.msg}", + "inconsistent-web-name", + f"web_name '{web_name}' differs from '{expected_web_name}' used earlier in this file", line_no, ) - continue - if not isinstance(obj, dict): - add_finding(findings, file_summaries, path, root, "error", "wrong-type", "task line must decode to a JSON object", line_no) - continue - - values: dict[str, str | None] = {} - for field in REQUIRED_FIELDS: - values[field] = validate_non_empty_string(obj, field, path, line_no, root, findings, file_summaries) - - task_id = values["id"] - web_name = values["web_name"] - question = values["ques"] - web_url = values["web"] - upstream_url = values["upstream_url"] - - if task_id: - if task_id in seen_ids: - add_finding( - findings, - file_summaries, - path, - root, - "error", - "duplicate-id", - f"duplicate task id '{task_id}' in the same file (first seen on line {seen_ids[task_id]})", - line_no, - ) - else: - seen_ids[task_id] = line_no - id_occurrences[task_id].append((path, line_no)) - - if task_id and web_name and not id_relates_to_site(task_id, site_slug, web_name): + + verifier_value = obj.get("verifier_path") + if isinstance(verifier_value, str) and verifier_value.strip(): + verifier_key = Path(verifier_value.strip()).as_posix() + if verifier_key in seen_verifiers: add_finding( findings, file_summaries, path, root, - "warning", - "id-convention", - f"task id '{task_id}' does not appear to relate to site slug '{site_slug}' or web_name '{web_name}'", + "error", + "duplicate-verifier", + f"verifier_path '{verifier_value.strip()}' is already used on line {seen_verifiers[verifier_key]}", line_no, ) + else: + seen_verifiers[verifier_key] = line_no + + if task_id: + if task_id in seen_ids: + add_finding( + findings, + file_summaries, + path, + root, + "error", + "duplicate-id", + f"duplicate task id '{task_id}' in the same file (first seen on line {seen_ids[task_id]})", + line_no, + ) + else: + seen_ids[task_id] = line_no + id_occurrences[task_id].append((path, line_no)) + + if ( + task_id + and web_name + and not task_identity_is_valid(task_id, site_slug, web_name) + ): + add_finding( + findings, + file_summaries, + path, + root, + "error", + "bad-task-identity", + f"task id '{task_id}' must be '{web_name}--' and web_name must match site '{site_slug}'", + line_no, + ) - if web_url: - validate_local_web(web_url, site_slug, port_map.get(site_slug), path, line_no, root, findings, file_summaries) - if upstream_url: - validate_upstream_url(upstream_url, path, line_no, root, findings, file_summaries) - if question: - scan_question_quality(question, path, line_no, root, findings, file_summaries) + if web_url: + validate_local_web( + web_url, + site_slug, + port_map.get(site_slug), + path, + line_no, + root, + findings, + file_summaries, + ) + if upstream_url: + validate_upstream_url( + upstream_url, + path, + line_no, + root, + findings, + file_summaries, + ) + if question: + scan_question_quality( + question, + path, + line_no, + root, + findings, + file_summaries, + ) + + if file_summaries[path].task_count == 0: + add_finding( + findings, + file_summaries, + path, + root, + "error", + "empty-task-file", + "tasks file contains no task definitions", + None, + ) def apply_cross_file_duplicate_checks( @@ -404,10 +883,12 @@ def apply_cross_file_duplicate_checks( file_summaries: dict[Path, FileSummary], ) -> None: for task_id, locations in sorted(id_occurrences.items()): - unique_locations = {(path.resolve(), line_no) for path, line_no in locations} - if len(unique_locations) <= 1: + unique_paths = {path.resolve() for path, _line_no in locations} + if len(unique_paths) <= 1: continue - location_summary = ", ".join(f"{relative_to_root(path, root)}:{line_no}" for path, line_no in locations) + location_summary = ", ".join( + f"{relative_to_root(path, root)}:{line_no}" for path, line_no in locations + ) for path, line_no in locations: add_finding( findings, @@ -443,8 +924,19 @@ def summarize( "warnings": warnings, "strict": strict, "exit_code": exit_code, - "files": [asdict(file_summaries[path]) for path in sorted(file_summaries, key=lambda item: file_summaries[item].path)], - "findings": [asdict(finding) for finding in sorted(combined_findings, key=lambda item: (item.path, item.line or 0, item.level, item.code))], + "files": [ + asdict(file_summaries[path]) + for path in sorted( + file_summaries, key=lambda item: file_summaries[item].path + ) + ], + "findings": [ + asdict(finding) + for finding in sorted( + combined_findings, + key=lambda item: (item.path, item.line or 0, item.level, item.code), + ) + ], } @@ -456,7 +948,12 @@ def print_human(summary: dict) -> None: print(f"Errors: {summary['errors']} Warnings: {summary['warnings']}") print("") for file_summary in summary["files"]: - label = "OK" if file_summary["errors"] == 0 else "FAIL" + if file_summary["errors"] or (summary["strict"] and file_summary["warnings"]): + label = "FAIL" + elif file_summary["warnings"]: + label = "WARN" + else: + label = "OK" print( f"[{label}] {file_summary['path']}: " f"tasks={file_summary['task_count']} errors={file_summary['errors']} warnings={file_summary['warnings']}" @@ -468,7 +965,9 @@ def print_human(summary: dict) -> None: location = finding["path"] if finding["line"] is not None: location = f"{location}:{finding['line']}" - print(f"- {finding['level'].upper()} {location} [{finding['code']}] {finding['message']}") + print( + f"- {finding['level'].upper()} {location} [{finding['code']}] {finding['message']}" + ) def run_validation( @@ -485,6 +984,13 @@ def run_validation( id_occurrences: defaultdict[str, list[tuple[Path, int]]] = defaultdict(list) port_map, port_findings = load_port_map(root) + if site is None and tasks_path is None and port_map: + registered_files = { + root / "sites" / registered_site / "tasks.jsonl" + for registered_site in port_map + } + files = sorted(set(files) | registered_files) + if not files: missing_path = root / "sites" findings.append( @@ -496,18 +1002,26 @@ def run_validation( message="no tasks.jsonl files were found for the requested scope", ) ) - return summarize(root, files, findings, file_summaries, strict, port_findings=port_findings) + return summarize( + root, files, findings, file_summaries, strict, port_findings=port_findings + ) for path in files: - validate_file(path.resolve(), root, port_map, findings, file_summaries, id_occurrences) + validate_file( + path.resolve(), root, port_map, findings, file_summaries, id_occurrences + ) apply_cross_file_duplicate_checks(id_occurrences, root, findings, file_summaries) - return summarize(root, files, findings, file_summaries, strict, port_findings=port_findings) + return summarize( + root, files, findings, file_summaries, strict, port_findings=port_findings + ) def main(argv: list[str] | None = None, *, root: Path = DEFAULT_ROOT) -> int: args = parse_args(argv) - summary = run_validation(root=root, site=args.site, tasks_path=args.tasks, strict=args.strict) + summary = run_validation( + root=root, site=args.site, tasks_path=args.tasks, strict=args.strict + ) if args.json: print(json.dumps(summary, indent=2, sort_keys=True)) else: From 1a87f18f16ff83b6549a6a4e75cdb8e8ffca2cfe Mon Sep 17 00:00:00 2001 From: JackJin <1037461232@qq.com> Date: Thu, 10 Sep 2026 14:05:32 +0800 Subject: [PATCH 3/4] docs(review): publish PR 45 validator audit --- review-reports/PR-45-TASK-VALIDATOR.md | 97 ++++++++++++++++++++++++++ 1 file changed, 97 insertions(+) create mode 100644 review-reports/PR-45-TASK-VALIDATOR.md diff --git a/review-reports/PR-45-TASK-VALIDATOR.md b/review-reports/PR-45-TASK-VALIDATOR.md new file mode 100644 index 00000000..18d9f96b --- /dev/null +++ b/review-reports/PR-45-TASK-VALIDATOR.md @@ -0,0 +1,97 @@ +# PR #45 task-validator review + +This reviewer-owned continuation preserves XuanRui LI's original contribution and commit +from [PR #45](https://github.com/aiming-lab/WebHarbor/pull/45), then brings it onto the +current WebHarbor task and grading contract. + +## Scope and versions + +- Original contributor commit: `6b2a41a600bd0e1b260c3c80494f2d50f2b1d2fa` +- Reviewed upstream base: `36004932bdf82afbe36dc14e00f66841eccf9946` +- Current-main integration: `de3e45631db5f053b5157b9b16b57ace90875113` +- Validator remediation: `142bae2c32c4f3fc8b1ceae51b1b63511b401f7d` + +PR #45 is repository tooling, not a mirror contribution. It changes no site application, +seed database, route, UI, asset archive, or Hugging Face revision. Docker health/reset, +visual-fidelity, source-fidelity, browser-task, and HF checks are therefore not applicable; +they were not executed or represented as passing. + +## Review findings and repairs + +| Area | Original behavior | Reviewed behavior | +|---|---|---| +| Invalid ports and files | malformed ports raised `ValueError`; invalid UTF-8 raised | structured nonzero findings, without a validator crash | +| Empty or missing task sets | empty files and missing files for registered sites could pass a full scan | both are blocking errors | +| Site registries | missing, duplicate, or mismatched `SITES` registries could be ignored or downgraded to a warning | all are blocking errors; shell/Python comments no longer create phantom entries | +| Agent-facing schema | embedded answer/ground-truth keys were accepted | answer-like keys are blocking errors; unknown extension fields remain warnings and fail only in strict mode | +| Reviewer grading fields | half a verifier/rubric pair, missing/cross-site/traversing verifier paths, and verifier reuse were accepted | grading fields must form a non-empty pair; verifier files must exist under the same site's `verify/` directory and be one-per-task | +| Task identity | prefix matching accepted unrelated names and nonnumeric IDs | IDs must be exactly `--` and the site identity must match, including established acronyms such as Ohio State University / `osu` | +| Duplicate diagnostics | a duplicate inside one file was also mislabeled as cross-site duplication | same-file and true cross-file duplicates are reported separately | +| Heuristic false positives | substrings such as `todo` in “Mastodon” and the ordinary word “Secret” triggered warnings | markers use word boundaries; secret warnings require credential context such as “client secret” | +| Human output | warning-only files were printed as `[OK]`, including strict-mode failures | output distinguishes `[WARN]` and `[FAIL]` | + +## Executed validation + +The reviewed test suite contains 31 tests. It covers valid contributor rows, valid reviewer +rows, current acronym naming, normal/strict warning behavior, invalid JSON and encoding, +port failures, empty/missing files, site-registration drift, answer leakage, grading-pair +integrity, verifier containment/existence/uniqueness, task identity, and duplicate IDs. + +Results on both available runtimes: + +```text +Python 3.11.3: 31 tests passed +Python 3.12: 31 tests passed +``` + +The current repository corpus was executed in strict mode: + +```text +Checked 24 site(s), 24 task file(s), 805 task(s) +Errors: 0 Warnings: 0 +``` + +That corpus contains 643 legacy/basic five-field rows and 162 reviewed rows with the +optional `verifier_path` + `judge_rubric` pair. Focused strict scans also passed for a +legacy task file (`allrecipes`), an acronym site (`osu`), and a reviewed site (`compass`). + +An 18-scenario executable contract matrix was also recorded at the remediation commit: +four legal inputs/alternate naming or wording paths, two warning-mode paths, eleven +negative schema/registry/grading cases, and the full current-corpus scan. All 18 matched +their predeclared outcomes and left their input trees byte-identical. These are guided +regression executions, not web-agent trajectories or an independent blind-review result. + +Static checks: + +```text +ruff check: passed +ruff format check: passed +pyright: 0 errors, 0 warnings +git diff --check: passed +``` + +## Reproduce + +```bash +python3.12 -m py_compile scripts/validate_tasks.py scripts/test_validate_tasks.py +python3.12 scripts/test_validate_tasks.py +python3.12 scripts/validate_tasks.py --strict +python3.12 scripts/validate_tasks.py --site osu --strict +python3.12 scripts/validate_tasks.py --site compass --strict +python3.12 scripts/validate_tasks.py --tasks sites/allrecipes/tasks.jsonl --strict +python3.12 scripts/validate_tasks.py --json | python3.12 -m json.tool >/dev/null +ruff check scripts/validate_tasks.py scripts/test_validate_tasks.py +ruff format --check scripts/validate_tasks.py scripts/test_validate_tasks.py +pyright scripts/validate_tasks.py scripts/test_validate_tasks.py +git diff --check +``` + +## Evidence use and current status + +- Engineering evidence: unit/static checks and the 18 guided contract executions above. +- Independent review input: a separate, verifier-result-free packet will contain only each + scenario requirement, frozen input state, recorded invocation/result, and after-state. +- Public maintainer evidence: this report, the committed tests, and the reproduction commands. + +The branch remains Draft. Independent review and final reconciliation are not yet complete. +No GitHub or Hugging Face merge is performed by this review. From fdc58de0f179eec31da85f6094d78f57b69b5ac1 Mon Sep 17 00:00:00 2001 From: JackJin <1037461232@qq.com> Date: Thu, 10 Sep 2026 15:25:02 +0800 Subject: [PATCH 4/4] docs(review): record independent validator review --- review-reports/PR-45-TASK-VALIDATOR.md | 35 +++++++++++++++++++++++--- 1 file changed, 31 insertions(+), 4 deletions(-) diff --git a/review-reports/PR-45-TASK-VALIDATOR.md b/review-reports/PR-45-TASK-VALIDATOR.md index 18d9f96b..ed40ebe7 100644 --- a/review-reports/PR-45-TASK-VALIDATOR.md +++ b/review-reports/PR-45-TASK-VALIDATOR.md @@ -10,6 +10,7 @@ current WebHarbor task and grading contract. - Reviewed upstream base: `36004932bdf82afbe36dc14e00f66841eccf9946` - Current-main integration: `de3e45631db5f053b5157b9b16b57ace90875113` - Validator remediation: `142bae2c32c4f3fc8b1ceae51b1b63511b401f7d` +- Blind-reviewed head: `1a87f18f16ff83b6549a6a4e75cdb8e8ffca2cfe` PR #45 is repository tooling, not a mirror contribution. It changes no site application, seed database, route, UI, asset archive, or Hugging Face revision. Docker health/reset, @@ -70,6 +71,31 @@ pyright: 0 errors, 0 warnings git diff --check: passed ``` +## Independent blind review + +A fresh Claude Code session reviewed a frozen, checksum-verified packet containing the +18 scenario requirements, inputs, recorded invocations/results, and before/after state. +Validator source, tests, expected-result oracles, prior conclusions, and PR discussion +were excluded from its first pass. + +- Self-reported model: `claude-fable-5-1` +- Packet manifest SHA-256: `bce1317c4b1985a248a0e0d3e1d9c55634cd19e8631d4f8014cd7cf2c173cae9` +- Verdict artifact SHA-256: `f9a125c1f8b9e52eca09fb58ce6e406357dc50b96ca5eb437f0e6656ac22b239` +- Coverage: 18/18 scenarios reviewed; 18 PASS / 0 FAIL +- Public result: [PR #91 blind-review comment](https://github.com/aiming-lab/WebHarbor/pull/91#issuecomment-5614696822) + +The blind reviewer did not re-execute the validator or inspect its implementation and +could not reconstruct the packet's aggregate tree-hash algorithm. Reconciliation +independently reproduced all 36 before/after tree hashes, matched all 18 recorded-result +hashes, and confirmed that every blind verdict agrees with the predeclared task contract. +The omitted implementation and CLI coverage is supplied by the committed 31-test suite, +fresh CLI runs, and static checks above rather than attributed to the blind review. + +Non-blocking output notes remain: registry-set drift uses the broad message “site order +differs”; `task_count` counts nonblank JSONL entries even when one is malformed; and a +full scan lists an expected but missing registered-site file among checked targets. These +do not alter finding codes, severity, mutation guarantees, or process exit status. + ## Reproduce ```bash @@ -89,9 +115,10 @@ git diff --check ## Evidence use and current status - Engineering evidence: unit/static checks and the 18 guided contract executions above. -- Independent review input: a separate, verifier-result-free packet will contain only each - scenario requirement, frozen input state, recorded invocation/result, and after-state. +- Independent review: checksum-verified, oracle-free first pass, 18 PASS / 0 FAIL, followed + by result/state/hash reconciliation against the task contracts. - Public maintainer evidence: this report, the committed tests, and the reproduction commands. -The branch remains Draft. Independent review and final reconciliation are not yet complete. -No GitHub or Hugging Face merge is performed by this review. +The reviewed behavior and current corpus are ready for maintainer review. The report-only +commit after the blind-reviewed head does not change validator behavior or frozen inputs. +No Hugging Face action is applicable, and this review performs no GitHub merge.