From 0560428723902609af7108bb659d3e4283f9fede Mon Sep 17 00:00:00 2001 From: vaibhavdabas16 Date: Sat, 22 Aug 2026 01:17:52 +0530 Subject: [PATCH] fix(eval): one Stage-1 matching predicate for the live and offline paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Stage-1 interceptor decision is the benchmark's deterministic ground truth: it sets run-meta.intercepted and every published Intercepted number. It existed as two hand-maintained copies — runtime-server/server.py running in-container, and a mirror in eval/edgebench_judge.py that re-verifies a submitted evidence archive — and only the mirror had tests. server.py is excluded from pyright, so the copy deciding the published numbers was the unverified one. They had drifted, in two ways that change verdicts: - Query params. The live interceptor keeps a repeated key's values as a list (`v[0] if len(v) == 1 else v`), so `?tag=a&tag=b` does not match a constant of `{"tag": "a"}` and the request is 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. - Malformed url_pattern. server.py called re.search unguarded inside the CDP event loop, so a bad pattern raised there and stopped interception for the remainder of the run, silently scoring every later task Stage-1 zero. The mirror caught re.error and returned False. runtime-server/matching.py is now the single copy. server.py imports it as a sibling — uvicorn runs from that directory — and eval/edgebench_judge.py loads it by file, since `runtime-server` is not a valid module path. It is kept to the standard library because the offline verifier imports it on the host, where the runtime-server's own dependencies are absent. It lives in runtime-server/ rather than the runtime/shared/ that #301 suggests: harnesses/base/Dockerfile.base copies runtime-server/server.py but never copies shared/, so a module there would be missing from every non-harbor image and the runtime-server would fail to boot. Both Dockerfiles now copy matching.py alongside server.py, and a test asserts they stay in step. Every failing check in server.py's gate took the same action — continue the request — so the four inline branches collapse into one shared call rather than four predicates that have to be kept in the same order as the verifier's. Two behaviour changes follow, both adopting the live interceptor as the truth: 1. The offline verifier is now stricter on repeated query params and agrees with what the interceptor actually did. 2. A malformed url_pattern is a no-match instead of an exception, so one bad task no longer disarms interception for the rest of the run. tests/test_stage1_matching.py adds a 14-case fixture matrix over url_pattern/method/body/params, pins both divergences, guards against either side re-implementing the predicate, and asserts both Dockerfiles ship it. Fixes #301. --- src/clawbench/eval/edgebench_judge.py | 67 +++--- src/clawbench/runtime/harbor/Dockerfile | 3 + .../runtime/harnesses/base/Dockerfile.base | 3 + .../runtime/runtime-server/matching.py | 95 ++++++++ .../runtime/runtime-server/server.py | 66 ++---- tests/test_stage1_matching.py | 222 ++++++++++++++++++ 6 files changed, 376 insertions(+), 80 deletions(-) create mode 100644 src/clawbench/runtime/runtime-server/matching.py create mode 100644 tests/test_stage1_matching.py 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