diff --git a/src/clawbench/eval/edgebench_judge.py b/src/clawbench/eval/edgebench_judge.py index fab782c0..c4abf6dd 100644 --- a/src/clawbench/eval/edgebench_judge.py +++ b/src/clawbench/eval/edgebench_judge.py @@ -23,13 +23,37 @@ import hmac import json import os -import re import sys from pathlib import Path from typing import Any -from urllib.parse import parse_qs, urlparse from clawbench.runner.judge import judge_request +from clawbench.utils.paths import RUNTIME_ROOT + + +def _load_runtime_matching(): + """Load the Stage-1 predicate from the runtime-server directory. + + It lives beside the interceptor that runs it, because runtime-server/ is + what gets COPYed into every task image. That directory name is not a valid + module path, so it is loaded by file. Sharing the one copy is the point: a + re-implementation here is what drifted from the live interceptor and made + offline verdicts disagree with real runs. + """ + import importlib.util + + path = RUNTIME_ROOT / "runtime-server" / "matching.py" + spec = importlib.util.spec_from_file_location( + "clawbench_runtime_matching", str(path) + ) + if spec is None or spec.loader is None: # pragma: no cover - packaging error + raise ImportError(f"cannot load the Stage-1 matcher from {path}") + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +_matching = _load_runtime_matching() def _verify_signature(intercept: dict[str, Any], secret: str) -> bool: @@ -52,46 +76,15 @@ def _verify_signature(intercept: dict[str, Any], secret: str) -> bool: return hmac.compare_digest(sig, expected) -def _const_fields_match(expected: Any, actual: Any) -> bool: - """All key/values in ``expected`` present in ``actual`` (mirrors runtime-server).""" - if not expected: - return True - if not actual: - return False - if isinstance(actual, list): - return any(_const_fields_match(expected, item) for item in actual) - if not isinstance(actual, dict): - return False - return all(actual.get(k) == v for k, v in expected.items()) - - def _stage1_match(request: dict[str, Any], eval_schema: Any) -> bool: """Recompute Stage-1 against the task schema — do NOT trust the agent's flag. The agent controls the submitted evidence archive, so re-verify that the - submitted request actually hits the task's target (url_pattern regex + method - + const body/params), exactly as the runtime interceptor would. + submitted request actually hits the task's target, using the very predicate + the in-container interceptor ran. This was a hand-maintained mirror of + runtime-server until the two drifted; see ``_matching`` above. """ - if not isinstance(eval_schema, dict): - return False - url_pattern = eval_schema.get("url_pattern") or "" - if not url_pattern: - return False # no target to verify against → cannot confirm interception - url = str(request.get("url") or "") - try: - if not re.search(url_pattern, url): - return False - except re.error: - return False - method = eval_schema.get("method") - if method and request.get("method") != method: - return False - if not _const_fields_match(eval_schema.get("body"), request.get("body")): - return False - # Always derive query params from the URL (like the runtime interceptor) — do - # not trust a submitted request["params"] field, which could be forged. - params = {k: v[0] for k, v in parse_qs(urlparse(url).query).items()} - return _const_fields_match(eval_schema.get("params"), params) + return _matching.stage1_match(request, eval_schema) # SForge structured_json markers (grading._grade_structured looks for these). diff --git a/src/clawbench/runtime/harbor/Dockerfile b/src/clawbench/runtime/harbor/Dockerfile index cd7c1921..55fcdaba 100644 --- a/src/clawbench/runtime/harbor/Dockerfile +++ b/src/clawbench/runtime/harbor/Dockerfile @@ -27,6 +27,9 @@ RUN UV_PYTHON_PREFERENCE=only-system uv sync --frozen \ WORKDIR /app COPY runtime-server/server.py ./src/runtime-server/server.py +# Stage-1 matching predicate, imported by server.py and shared with the +# offline verifier. Must ship alongside server.py or interception breaks. +COPY runtime-server/matching.py ./src/runtime-server/matching.py COPY chrome-extension/ ./src/chrome-extension/ COPY shared/ ./src/shared/ COPY harbor/ ./src/harbor/ diff --git a/src/clawbench/runtime/harnesses/base/Dockerfile.base b/src/clawbench/runtime/harnesses/base/Dockerfile.base index 801149b3..48b2f564 100644 --- a/src/clawbench/runtime/harnesses/base/Dockerfile.base +++ b/src/clawbench/runtime/harnesses/base/Dockerfile.base @@ -29,6 +29,9 @@ RUN UV_PYTHON_PREFERENCE=only-system uv sync --frozen WORKDIR /app COPY runtime-server/server.py ./src/runtime-server/server.py +# Stage-1 matching predicate, imported by server.py and shared with the +# offline verifier. Must ship alongside server.py or interception breaks. +COPY runtime-server/matching.py ./src/runtime-server/matching.py COPY chrome-extension/ ./src/chrome-extension/ diff --git a/src/clawbench/runtime/runtime-server/matching.py b/src/clawbench/runtime/runtime-server/matching.py new file mode 100644 index 00000000..9d4e57c9 --- /dev/null +++ b/src/clawbench/runtime/runtime-server/matching.py @@ -0,0 +1,95 @@ +"""Stage-1 interceptor matching — the benchmark's deterministic ground truth. + +Stage 1 asks one question: does this HTTP request hit the task's target +(``url_pattern`` regex + ``method`` + constant ``body``/``params`` fields)? +Every published Intercepted number is that answer. + +It is computed in two places — live, in-container, by ``server.py`` next to +this file, and offline by ``clawbench.eval.edgebench_judge`` when it +re-verifies a submitted evidence archive. Those were hand-maintained copies +and they drifted, so offline judging could disagree with what actually +happened during a run. This module is the single copy both import. + +Kept to the standard library on purpose: it is imported by the offline +verifier on the host, where the runtime-server's dependencies are absent. +""" + +from __future__ import annotations + +import re +from typing import Any +from urllib.parse import parse_qs, urlparse + + +def const_fields_match(expected: Any, actual: Any) -> bool: + """All key/value pairs in ``expected`` are present in ``actual``. + + For list bodies (batched GraphQL) any single item matching is enough. + An empty or absent ``expected`` constrains nothing and matches. + """ + if not expected: + return True + if not actual: + return False + if isinstance(actual, list): + return any(const_fields_match(expected, item) for item in actual) + if not isinstance(actual, dict): + return False + return all(actual.get(k) == v for k, v in expected.items()) + + +def query_params_from_url(url: str) -> dict[str, Any]: + """Query string as a dict, collapsing single-valued keys to a scalar. + + A repeated key keeps its list of values, so ``?tag=a&tag=b`` is + ``{"tag": ["a", "b"]}`` and does not match a constant of ``{"tag": "a"}``. + The offline verifier used to take ``v[0]`` unconditionally and so called + that request intercepted when the live interceptor had let it through. + """ + parsed = urlparse(str(url or "")) + return {k: v[0] if len(v) == 1 else v for k, v in parse_qs(parsed.query).items()} + + +def url_pattern_matches(url_pattern: str, url: str) -> bool: + """Whether ``url`` matches the task's target regex. + + A malformed pattern is a task-authoring error, not a request that should + be intercepted, so it is reported as no-match. It must never raise: this + runs inside the CDP event loop, where an exception stops interception for + the remainder of the run and silently zeroes the task's Stage-1 score. + """ + if not url_pattern: + return False + try: + return re.search(url_pattern, str(url or "")) is not None + except re.error: + return False + + +def stage1_match(request: dict[str, Any], eval_schema: Any) -> bool: + """Whether ``request`` hits the target described by ``eval_schema``. + + ``request`` carries ``url``, ``method``, and a parsed ``body``. Query + params are always derived from the URL rather than read off the request: + offline, a submitted ``params`` field is agent-controlled and could be + forged. + + An absent or empty ``url_pattern`` means there is no target to verify + against and returns False. Live, that case is handled earlier — the + interceptor is simply never armed and no request is ever blocked. + """ + if not isinstance(eval_schema, dict): + return False + + url = str(request.get("url") or "") + if not url_pattern_matches(eval_schema.get("url_pattern") or "", url): + return False + + method = eval_schema.get("method") + if method and request.get("method") != method: + return False + + if not const_fields_match(eval_schema.get("body"), request.get("body")): + return False + + return const_fields_match(eval_schema.get("params"), query_params_from_url(url)) diff --git a/src/clawbench/runtime/runtime-server/server.py b/src/clawbench/runtime/runtime-server/server.py index 3de37cbe..563e032d 100644 --- a/src/clawbench/runtime/runtime-server/server.py +++ b/src/clawbench/runtime/runtime-server/server.py @@ -2,20 +2,23 @@ import base64 import json import os -import re import signal import subprocess import threading import time from contextlib import asynccontextmanager from pathlib import Path -from urllib.parse import parse_qs, urlparse +from urllib.parse import parse_qs import urllib.request import websocket from fastapi import FastAPI, WebSocket, WebSocketDisconnect from fastapi.responses import HTMLResponse, JSONResponse +# Sibling module, shared verbatim with the offline verifier +# (clawbench.eval.edgebench_judge) so the two cannot drift. +from matching import query_params_from_url, stage1_match + DATA_DIR = Path(os.environ.get("CLAWBENCH_DATA_DIR", "/data")) ACTIONS_FILE = DATA_DIR / "actions.jsonl" SCREENSHOTS_DIR = DATA_DIR / "screenshots" @@ -170,21 +173,6 @@ def stop_ffmpeg_recording(timeout: int = 10) -> str: """ -def _const_fields_match(expected, actual): - """Check that all key-value pairs in expected match in actual data. - For list bodies (batched GraphQL), returns True if any item matches. - Returns True if all match or expected is empty/None.""" - if not expected: - return True - if not actual: - return False - if isinstance(actual, list): - return any(_const_fields_match(expected, item) for item in actual) - if not isinstance(actual, dict): - return False - return all(actual.get(k) == v for k, v in expected.items()) - - FILTERED_PREFIXES = ( "http://localhost:7878", "http://127.0.0.1:7878", @@ -218,18 +206,13 @@ def _log_request(log_file, params): if any(request_url.startswith(p) for p in FILTERED_PREFIXES): return - parsed = urlparse(request_url) - query_params = { - k: v[0] if len(v) == 1 else v for k, v in parse_qs(parsed.query).items() - } - entry = { "timestamp": time.time(), "url": request_url, "method": request["method"], "headers": request.get("headers", {}), "body": _parse_body(request.get("postData")), - "query_params": query_params, + "query_params": query_params_from_url(request_url), "resource_type": params.get("resourceType", "Other"), } log_file.write(json.dumps(entry) + "\n") @@ -451,26 +434,23 @@ def activate_session_target(session_id, reason): continue # --- Intercept: block if URL + method + body/params match --- - if not re.search(url_pattern, request_url): - send("Fetch.continueRequest", {"requestId": request_id}, session_id) - continue - - if required_method and params["request"]["method"] != required_method: - send("Fetch.continueRequest", {"requestId": request_id}, session_id) - continue - - # Parse request data for body/params matching - parsed = urlparse(request_url) - query_params = { - k: v[0] if len(v) == 1 else v for k, v in parse_qs(parsed.query).items() - } + # One shared predicate, not four inline branches: every failing + # check here continues the request, and the offline verifier has + # to reach the identical verdict from the archived evidence. body = _parse_body(params["request"].get("postData")) - - if not _const_fields_match(match_body, body): - send("Fetch.continueRequest", {"requestId": request_id}, session_id) - continue - - if not _const_fields_match(match_params, query_params): + if not stage1_match( + { + "url": request_url, + "method": params["request"]["method"], + "body": body, + }, + { + "url_pattern": url_pattern, + "method": required_method, + "body": match_body, + "params": match_params, + }, + ): send("Fetch.continueRequest", {"requestId": request_id}, session_id) continue @@ -478,7 +458,7 @@ def activate_session_target(session_id, reason): request_obj = { "url": request_url, "method": params["request"]["method"], - "params": query_params, + "params": query_params_from_url(request_url), "body": body, } diff --git a/tests/test_stage1_matching.py b/tests/test_stage1_matching.py new file mode 100644 index 00000000..f45044d0 --- /dev/null +++ b/tests/test_stage1_matching.py @@ -0,0 +1,222 @@ +"""Stage-1 matching is one predicate, shared by the live and offline paths. + +The interceptor decision is the benchmark's deterministic ground truth: it sets +run-meta.intercepted and every published Intercepted number. It used to exist as +two hand-maintained copies -- runtime-server/server.py in-container and a mirror +in eval/edgebench_judge.py -- and only the mirror had tests. They drifted, so +offline re-verification could disagree with what actually happened during a run. +""" + +from __future__ import annotations + +import inspect +from pathlib import Path +from typing import Any + +import pytest + +from clawbench.eval import edgebench_judge as ej +from clawbench.utils.paths import RUNTIME_ROOT + +matching = ej._matching + +SERVER_PY = RUNTIME_ROOT / "runtime-server" / "server.py" +MATCHING_PY = RUNTIME_ROOT / "runtime-server" / "matching.py" + + +# --- the drift that was actually shipping ------------------------------------ + + +def test_repeated_query_params_do_not_match_a_scalar_constant() -> None: + """The divergence with teeth. + + The live interceptor keeps a repeated key's values as a list, so + ?tag=a&tag=b never matched a constant of {"tag": "a"} and the request was + let through. The offline mirror took v[0] unconditionally, matched, and + reported the run as intercepted -- a Stage-1 pass for a request that was + never blocked. + """ + schema = {"url_pattern": r"/api/cart", "params": {"tag": "a"}} + request = {"url": "https://shop.test/api/cart?tag=a&tag=b", "method": "GET"} + + assert matching.query_params_from_url(request["url"]) == {"tag": ["a", "b"]} + assert ej._stage1_match(request, schema) is False + + +def test_single_valued_query_params_still_collapse_to_a_scalar() -> None: + schema = {"url_pattern": r"/api/cart", "params": {"tag": "a"}} + request = {"url": "https://shop.test/api/cart?tag=a", "method": "GET"} + + assert matching.query_params_from_url(request["url"]) == {"tag": "a"} + assert ej._stage1_match(request, schema) is True + + +@pytest.mark.parametrize("pattern", ["foo(", "[", "*bad", "(?Pa)(?Pb)"]) +def test_a_malformed_url_pattern_is_a_no_match_and_never_raises(pattern: str) -> None: + """server.py called re.search unguarded inside the CDP event loop, so a + malformed pattern raised there and stopped interception for the rest of the + run -- every later task in that run silently scored Stage-1 zero. + """ + assert matching.url_pattern_matches(pattern, "https://t.ex/x") is False + assert ( + ej._stage1_match({"url": "https://t.ex/x"}, {"url_pattern": pattern}) is False + ) + + +def test_an_empty_url_pattern_matches_nothing() -> None: + """Live, an empty pattern means the interceptor is never armed. Offline it + means there is no target to verify against. Both are 'not intercepted'.""" + assert matching.url_pattern_matches("", "https://t.ex/x") is False + assert ej._stage1_match({"url": "https://t.ex/x"}, {"url_pattern": ""}) is False + + +# --- fixture matrix ---------------------------------------------------------- + +CASES: list[tuple[str, dict[str, Any], Any, bool]] = [ + ( + "url+method hit", + {"url": "https://t.ex/checkout", "method": "POST"}, + {"url_pattern": r"/checkout", "method": "POST"}, + True, + ), + ( + "method mismatch", + {"url": "https://t.ex/checkout", "method": "GET"}, + {"url_pattern": r"/checkout", "method": "POST"}, + False, + ), + ( + "method unconstrained", + {"url": "https://t.ex/checkout", "method": "DELETE"}, + {"url_pattern": r"/checkout"}, + True, + ), + ( + "url miss", + {"url": "https://t.ex/browse", "method": "POST"}, + {"url_pattern": r"/checkout", "method": "POST"}, + False, + ), + ( + "const body hit", + {"url": "https://t.ex/c", "method": "POST", "body": {"sku": "A1", "qty": 2}}, + {"url_pattern": r"/c", "body": {"sku": "A1"}}, + True, + ), + ( + "const body wrong value", + {"url": "https://t.ex/c", "method": "POST", "body": {"sku": "B2"}}, + {"url_pattern": r"/c", "body": {"sku": "A1"}}, + False, + ), + ( + "const body missing on empty body", + {"url": "https://t.ex/c", "method": "POST", "body": None}, + {"url_pattern": r"/c", "body": {"sku": "A1"}}, + False, + ), + ( + "batched graphql list body - any item matches", + { + "url": "https://t.ex/gql", + "method": "POST", + "body": [{"op": "noise"}, {"op": "checkout"}], + }, + {"url_pattern": r"/gql", "body": {"op": "checkout"}}, + True, + ), + ( + "list body with no matching item", + {"url": "https://t.ex/gql", "method": "POST", "body": [{"op": "noise"}]}, + {"url_pattern": r"/gql", "body": {"op": "checkout"}}, + False, + ), + ( + "scalar body cannot satisfy a const constraint", + {"url": "https://t.ex/c", "method": "POST", "body": "raw-text"}, + {"url_pattern": r"/c", "body": {"sku": "A1"}}, + False, + ), + ( + "params derived from the url, not the request", + {"url": "https://t.ex/s?id=7", "method": "GET", "params": {"id": "forged"}}, + {"url_pattern": r"/s", "params": {"id": "7"}}, + True, + ), + ( + "forged params field cannot fake a match", + {"url": "https://t.ex/s?id=1", "method": "GET", "params": {"id": "7"}}, + {"url_pattern": r"/s", "params": {"id": "7"}}, + False, + ), + ( + "empty constraints constrain nothing", + {"url": "https://t.ex/c", "method": "POST", "body": {}}, + {"url_pattern": r"/c", "body": {}, "params": {}}, + True, + ), + ( + "non-dict schema", + {"url": "https://t.ex/c", "method": "POST"}, + None, + False, + ), +] + + +@pytest.mark.parametrize( + ("name", "request_obj", "schema", "expected"), + CASES, + ids=[c[0] for c in CASES], +) +def test_stage1_matrix( + name: str, request_obj: dict[str, Any], schema: Any, expected: bool +) -> None: + assert matching.stage1_match(request_obj, schema) is expected + assert ej._stage1_match(request_obj, schema) is expected + + +# --- the duplication itself -------------------------------------------------- + + +def test_the_offline_verifier_does_not_reimplement_the_predicate() -> None: + src = Path(ej.__file__).read_text(encoding="utf-8") + + assert "def _const_fields_match" not in src + assert "re.search" not in src + assert inspect.getsource(ej._stage1_match).count("_matching.stage1_match") == 1 + + +def test_the_live_interceptor_does_not_reimplement_the_predicate() -> None: + src = SERVER_PY.read_text(encoding="utf-8") + + assert "def _const_fields_match" not in src + assert "re.search" not in src + assert "from matching import" in src + + +def test_the_matcher_has_no_runtime_server_dependencies() -> None: + """edgebench_judge loads this module on the host, where the runtime-server's + own dependencies (fastapi, websocket) are not installed.""" + src = MATCHING_PY.read_text(encoding="utf-8") + + for third_party in ("fastapi", "websocket", "uvicorn"): + assert f"import {third_party}" not in src + + +# --- the module has to reach the container ----------------------------------- + + +@pytest.mark.parametrize( + "dockerfile", ["harbor/Dockerfile", "harnesses/base/Dockerfile.base"] +) +def test_every_image_that_ships_server_py_also_ships_matching_py( + dockerfile: str, +) -> None: + """server.py imports matching.py at startup. An image with one and not the + other fails to boot the runtime-server, which is the whole benchmark. + """ + text = (RUNTIME_ROOT / dockerfile).read_text(encoding="utf-8") + + assert "COPY runtime-server/server.py" in text + assert "COPY runtime-server/matching.py" in text