diff --git a/bridge/hopper_bridge.py b/bridge/hopper_bridge.py index 46f8515c3..a22f20637 100644 --- a/bridge/hopper_bridge.py +++ b/bridge/hopper_bridge.py @@ -18,6 +18,9 @@ _selected_document = None _search_inventory_cache = {} MAX_SEARCH_PATTERN_LENGTH = 256 MAX_SEARCH_VALUE_LENGTH = 4096 +MAX_REGEX_BACKTRACKING_PATHS = 10000 +MAX_REGEX_CANDIDATE_LENGTH = 4096 +MAX_REGEX_SEARCH_WORK_UNITS = 1000000 def _hex(value): @@ -216,33 +219,150 @@ def _search_inventory(document, kind): return inventory +def _checked_regex_paths(left, right, operation): + """Apply one path-count operation without crossing the static work budget.""" + if operation == "add": + exceeded = left > MAX_REGEX_BACKTRACKING_PATHS - right + result = left + right + else: + exceeded = right != 0 and left > MAX_REGEX_BACKTRACKING_PATHS // right + result = left * right + if exceeded or result > MAX_REGEX_BACKTRACKING_PATHS: + raise ValueError( + "Regex exceeds the %d-path backtracking budget" + % MAX_REGEX_BACKTRACKING_PATHS + ) + return result + + +def _repeat_regex_paths(child_paths, minimum, maximum): + """Count every bounded repetition path, including alternative child paths.""" + paths = 0 + repeated_paths = 1 + for count in range(maximum + 1): + if count >= minimum: + paths = _checked_regex_paths(paths, repeated_paths, "add") + if count < maximum: + repeated_paths = _checked_regex_paths( + repeated_paths, child_paths, "multiply" + ) + return paths + + +def _validate_regex_class(items): + """Accept only constant-time character-class operations.""" + allowed = { + sre_parse.CATEGORY, + sre_parse.LITERAL, + sre_parse.NEGATE, + sre_parse.RANGE, + } + if any(operation not in allowed for operation, _ in items): + raise ValueError("Regex operation is not supported by the bounded matcher") + + def _validate_regex_node(node, inside_repeat=False): - """Reject regex structures with disproportionate or non-local evaluation cost.""" + """Return capped path and step bounds for Python regex evaluation.""" + leaf_operations = { + sre_parse.ANY, + sre_parse.AT, + sre_parse.CATEGORY, + sre_parse.LITERAL, + sre_parse.NOT_LITERAL, + } forbidden = { sre_parse.ASSERT, sre_parse.ASSERT_NOT, sre_parse.GROUPREF, sre_parse.GROUPREF_EXISTS, } + for name in ("GROUPREF_IGNORE", "GROUPREF_LOC_IGNORE", "GROUPREF_UNI_IGNORE"): + operation = getattr(sre_parse, name, None) + if operation is not None: + forbidden.add(operation) repeat_tokens = {sre_parse.MAX_REPEAT, sre_parse.MIN_REPEAT} possessive = getattr(sre_parse, "POSSESSIVE_REPEAT", None) if possessive is not None: repeat_tokens.add(possessive) + atomic = getattr(sre_parse, "ATOMIC_GROUP", None) + + paths = 1 + steps = 0 for operation, argument in node: if operation in forbidden: raise ValueError("Regex lookarounds and backreferences are not supported") - if operation in repeat_tokens: + if operation in leaf_operations: + operation_paths = 1 + operation_steps = 1 + elif operation == sre_parse.IN: + _validate_regex_class(argument) + operation_paths = 1 + operation_steps = 1 + elif operation in repeat_tokens: if inside_repeat: raise ValueError("Nested regex repetitions are not supported") minimum, maximum, child = argument if maximum == sre_parse.MAXREPEAT or maximum > 1000: - raise ValueError("Unbounded or excessive regex repetitions are not supported") - _validate_regex_node(child, True) + raise ValueError( + "Unbounded or excessive regex repetitions are not supported" + ) + child_paths, child_steps = _validate_regex_node(child, True) + operation_paths = _repeat_regex_paths( + child_paths, minimum, maximum + ) + operation_steps = maximum * child_steps elif operation == sre_parse.SUBPATTERN: - _validate_regex_node(argument[-1], inside_repeat) + operation_paths, operation_steps = _validate_regex_node( + argument[-1], inside_repeat + ) elif operation == sre_parse.BRANCH: + operation_paths = 0 + operation_steps = 0 for branch in argument[1]: - _validate_regex_node(branch, inside_repeat) + branch_paths, branch_steps = _validate_regex_node( + branch, inside_repeat + ) + operation_paths = _checked_regex_paths( + operation_paths, + branch_paths, + "add", + ) + operation_steps = max(operation_steps, branch_steps) + elif atomic is not None and operation == atomic: + operation_paths, operation_steps = _validate_regex_node( + argument, inside_repeat + ) + else: + raise ValueError("Regex operation is not supported by the bounded matcher") + paths = _checked_regex_paths(paths, operation_paths, "multiply") + steps += operation_steps + return paths, steps + + +def _bounded_regex_matcher(expression, backtracking_paths, steps_per_path): + """Create a matcher with per-candidate and cumulative work bounds.""" + remaining_work = MAX_REGEX_SEARCH_WORK_UNITS + work_per_character = backtracking_paths * max(steps_per_path, 1) + + def matches(value): + nonlocal remaining_work + if not isinstance(value, str): + raise ValueError("Regex candidates must be strings") + if len(value) > MAX_REGEX_CANDIDATE_LENGTH: + raise ValueError( + "Regex candidate exceeds the %d-character safety limit" + % MAX_REGEX_CANDIDATE_LENGTH + ) + required_work = work_per_character * max(len(value), 1) + if required_work > remaining_work: + raise ValueError( + "Regex search exceeds the %d-unit work budget" + % MAX_REGEX_SEARCH_WORK_UNITS + ) + remaining_work -= required_work + return expression.search(value) is not None + + return matches def _search_page(document, kind, params): @@ -268,11 +388,13 @@ def _search_page(document, kind, params): else: try: parsed = sre_parse.parse(pattern) - _validate_regex_node(parsed) + backtracking_paths, steps_per_path = _validate_regex_node(parsed) expression = re.compile(pattern, 0 if case_sensitive else re.IGNORECASE) - except re.error as error: + except (re.error, OverflowError) as error: raise ValueError("Invalid regex pattern") from error - matches = lambda value: expression.search(value) is not None + matches = _bounded_regex_matcher( + expression, backtracking_paths, steps_per_path + ) selected = [] total = 0 diff --git a/scripts/verify-package.mjs b/scripts/verify-package.mjs index ef193b86f..c111edb41 100644 --- a/scripts/verify-package.mjs +++ b/scripts/verify-package.mjs @@ -15,6 +15,7 @@ import { Client } from "@modelcontextprotocol/client"; import { StdioClientTransport } from "@modelcontextprotocol/client/stdio"; import { TextReader, Uint8ArrayWriter, ZipWriter } from "@zip.js/zip.js"; import { TOOL_CONTRACTS } from "../dist/contracts/toolContracts.js"; +import { verifyPackedBridge } from "./verify-packed-bridge.mjs"; const expectedToolCount = TOOL_CONTRACTS.length; @@ -41,6 +42,7 @@ try { ) { throw new Error("package contained generated Python bytecode"); } + await verifyPackedBridge({ root, workspace, tarball, packedFiles }); const prefix = join(workspace, "prefix"); const home = join(workspace, "home"); const fakeBin = join(workspace, "bin"); @@ -484,7 +486,7 @@ try { } process.stdout.write( - `${JSON.stringify({ cli: true, analysisCli: true, artifactCli: true, evidenceCli: true, incurMcpCommand: "npx -y rea-agents mcp", doctor: "platform-appropriate", setup: supportedSetupHost ? "planned-then-idempotent" : "unsupported-host-rejected", setupPlanReadOnly: supportedSetupHost, existingHopperPreserved: supportedSetupHost, clients: supportedSetupHost ? 2 : 0, backupReadback: supportedSetupHost, failureRecovery: supportedSetupHost, skill: supportedSetupHost, mcpTools: expectedToolCount, evidenceMcp: true, targetFree: true, targetLifecycle: true })}\n`, + `${JSON.stringify({ cli: true, analysisCli: true, artifactCli: true, evidenceCli: true, incurMcpCommand: "npx -y rea-agents mcp", doctor: "platform-appropriate", setup: supportedSetupHost ? "planned-then-idempotent" : "unsupported-host-rejected", setupPlanReadOnly: supportedSetupHost, existingHopperPreserved: supportedSetupHost, clients: supportedSetupHost ? 2 : 0, backupReadback: supportedSetupHost, failureRecovery: supportedSetupHost, skill: supportedSetupHost, mcpTools: expectedToolCount, evidenceMcp: true, targetFree: true, targetLifecycle: true, boundedRegexBridge: true })}\n`, ); } finally { if (tarball) await rm(join(root, tarball), { force: true }); diff --git a/scripts/verify-packed-bridge.mjs b/scripts/verify-packed-bridge.mjs new file mode 100644 index 000000000..ca9fc3224 --- /dev/null +++ b/scripts/verify-packed-bridge.mjs @@ -0,0 +1,40 @@ +import { execFile } from "node:child_process"; +import { join } from "node:path"; +import { promisify } from "node:util"; + +const exec = promisify(execFile); +const packedBridge = "package/bridge/hopper_bridge.py"; + +/** Verify that the packed production bridge rejects a catastrophic regex. */ +export async function verifyPackedBridge({ + root, + workspace, + tarball, + packedFiles, +}) { + if (!packedFiles.includes(packedBridge)) + throw new Error("package omitted the Hopper bridge"); + await exec("tar", ["-xf", join(root, tarball), "-C", workspace]); + const probe = JSON.parse( + ( + await exec("python3", [ + join(root, "tests/fixtures/bridgeRegexProbe.py"), + join(workspace, packedBridge), + JSON.stringify({ + action: "match", + pattern: "(a|aa){1,35}b", + value: "a".repeat(35), + case_sensitive: true, + }), + ]) + ).stdout, + ); + if ( + probe.ok !== false || + probe.diagnostic_type !== "invalid_request" || + probe.message !== "Regex exceeds the 10000-path backtracking budget" + ) + throw new Error( + `packaged Hopper bridge omitted regex bounds: ${JSON.stringify(probe)}`, + ); +} diff --git a/src/contracts/toolContracts.ts b/src/contracts/toolContracts.ts index b2c4f0e9f..b92c97fc9 100644 --- a/src/contracts/toolContracts.ts +++ b/src/contracts/toolContracts.ts @@ -293,12 +293,12 @@ export const OFFICIAL_TOOL_CONTRACTS = [ ), official( "search_procedures", - "Search analyzed procedure names using literal matching by default or a structurally bounded regex. Returns a deterministic, offset-paginated page; continue at next_offset while has_more is true.", + "Search analyzed procedure names using literal matching by default or regex opt-in. Regex rejects unsupported constructs, more than 10,000 static backtracking paths, candidates over 4,096 characters, and searches over 1,000,000 work units. Results are deterministic and offset-paginated.", z.object(searchInput), ), official( "search_strings", - "Search analyzed strings using literal matching by default or a structurally bounded regex. Returns a deterministic, offset-paginated page with explicit value truncation; follow matches with xrefs.", + "Search analyzed strings using literal matching by default or regex opt-in. Regex rejects unsupported constructs, more than 10,000 static backtracking paths, candidates over 4,096 characters, and searches over 1,000,000 work units. Results are deterministic, offset-paginated, and explicitly truncated.", z.object(searchInput), ), official( diff --git a/tests/bridgeRegex.test.ts b/tests/bridgeRegex.test.ts new file mode 100644 index 000000000..1d70a59c0 --- /dev/null +++ b/tests/bridgeRegex.test.ts @@ -0,0 +1,372 @@ +import { execFile } from "node:child_process"; +import { promisify } from "node:util"; +import { fileURLToPath } from "node:url"; + +import { describe, expect, it } from "vitest"; +import { z } from "zod"; + +const execFileAsync = promisify(execFile); +const bridgePath = fileURLToPath( + new URL("../bridge/hopper_bridge.py", import.meta.url), +); +const probePath = fileURLToPath( + new URL("./fixtures/bridgeRegexProbe.py", import.meta.url), +); + +const errorResultSchema = z.object({ + action: z.enum(["match", "search"]), + ok: z.literal(false), + type: z.string(), + diagnostic_type: z.string(), + message: z.string(), +}); +const matchResultSchema = z.object({ + action: z.literal("match"), + ok: z.literal(true), + backtracking_paths: z.number().int().positive(), + matched: z.boolean(), +}); +const searchResultSchema = z.object({ + action: z.literal("search"), + ok: z.literal(true), + result: z.object({ + items: z.array( + z.object({ + address: z.string(), + value: z.string(), + value_truncated: z.boolean(), + }), + ), + offset: z.number().int().nonnegative(), + limit: z.number().int().positive(), + total: z.number().int().nonnegative(), + next_offset: z.number().int().nonnegative().nullable(), + has_more: z.boolean(), + }), +}); +const probeResultSchema = z.union([ + errorResultSchema, + matchResultSchema, + searchResultSchema, +]); + +type ProbeInput = + | { + readonly action: "match"; + readonly pattern: string; + readonly value: string; + readonly case_sensitive?: boolean; + } + | { + readonly action: "search"; + readonly items: readonly (readonly [string, string])[]; + readonly params: Readonly>; + }; + +const probe = async (input: ProbeInput) => { + const result = await execFileAsync( + "python3", + [probePath, bridgePath, JSON.stringify(input)], + { timeout: 3_000, maxBuffer: 1024 * 1024 }, + ); + return probeResultSchema.parse(JSON.parse(result.stdout)); +}; + +const invalidRequest = (action: "match" | "search", message: string) => ({ + action, + ok: false, + type: "ValueError", + diagnostic_type: "invalid_request", + message, +}); + +describe("Hopper bridge regex bounds", () => { + it("accepts useful patterns whose static backtracking budget is bounded", async () => { + await expect( + probe({ + action: "match", + pattern: String.raw`^rea_(?:main|helper)_[0-9]{1,3}$`, + value: "REA_helper_42", + }), + ).resolves.toEqual({ + action: "match", + ok: true, + backtracking_paths: 6, + matched: true, + }); + }); + + it("applies bounded regex matching through deterministic pagination", async () => { + await expect( + probe({ + action: "search", + items: [ + ["0x1000", "REA_main_1"], + ["0x2000", "unrelated"], + ["0x3000", "rea_helper_42"], + ], + params: { + pattern: String.raw`^rea_(?:main|helper)_[0-9]{1,3}$`, + mode: "regex", + case_sensitive: false, + offset: 1, + limit: 1, + }, + }), + ).resolves.toEqual({ + action: "search", + ok: true, + result: { + items: [ + { + address: "0x3000", + value: "rea_helper_42", + value_truncated: false, + }, + ], + offset: 1, + limit: 1, + total: 2, + next_offset: null, + has_more: false, + }, + }); + }); + + it.each([ + [ + "lookaround", + "(?=a)a", + "Regex lookarounds and backreferences are not supported", + ], + [ + "backreference", + String.raw`(a)\1`, + "Regex lookarounds and backreferences are not supported", + ], + [ + "nested repeat", + "(a{1,2}){1,2}", + "Nested regex repetitions are not supported", + ], + [ + "unbounded repeat", + "a+", + "Unbounded or excessive regex repetitions are not supported", + ], + [ + "excessive repeat", + "a{1001}", + "Unbounded or excessive regex repetitions are not supported", + ], + ])("rejects unsupported %s constructs", async (_name, pattern, message) => { + await expect( + probe({ action: "match", pattern, value: "aaaa", case_sensitive: true }), + ).resolves.toEqual(invalidRequest("match", message)); + }); + + it.each(["(", "a{999999999999999999999999}"])( + "normalizes parser failure %s through the search boundary", + async (pattern) => { + await expect( + probe({ + action: "search", + items: [], + params: { pattern, mode: "regex", offset: 0, limit: 1 }, + }), + ).resolves.toEqual(invalidRequest("search", "Invalid regex pattern")); + }, + ); + + it("accepts the maximum bounded repeat at the exact work budget", async () => { + await expect( + probe({ + action: "match", + pattern: "a{1000}", + value: "a".repeat(1_000), + case_sensitive: true, + }), + ).resolves.toEqual({ + action: "match", + ok: true, + backtracking_paths: 1, + matched: true, + }); + }); + + it("accepts expensive matching just below the cumulative work limit", async () => { + await expect( + probe({ + action: "match", + pattern: "(?:a|aa){1,8}b", + value: "a".repeat(115), + case_sensitive: true, + }), + ).resolves.toEqual({ + action: "match", + ok: true, + backtracking_paths: 510, + matched: false, + }); + }); + + it("rejects ambiguous bounded repetition before Python regex execution", async () => { + await expect( + probe({ + action: "match", + pattern: "(a|aa){1,35}b", + value: "a".repeat(35), + case_sensitive: true, + }), + ).resolves.toEqual( + invalidRequest( + "match", + "Regex exceeds the 10000-path backtracking budget", + ), + ); + }); + + it("rejects excessive combinations across adjacent optional atoms", async () => { + await expect( + probe({ + action: "match", + pattern: `${"a?".repeat(14)}b`, + value: "a".repeat(14), + case_sensitive: true, + }), + ).resolves.toEqual( + invalidRequest( + "match", + "Regex exceeds the 10000-path backtracking budget", + ), + ); + }); + + it("bounds cumulative work before evaluating a costly candidate", async () => { + await expect( + probe({ + action: "match", + pattern: "(?:a|aa){1,12}b", + value: "a".repeat(4_096), + case_sensitive: true, + }), + ).resolves.toEqual( + invalidRequest( + "match", + "Regex search exceeds the 1000000-unit work budget", + ), + ); + }); + + it("applies the regex work budget across the full inventory", async () => { + await expect( + probe({ + action: "search", + items: [ + ["0x1000", "a".repeat(60)], + ["0x2000", "a".repeat(60)], + ], + params: { + pattern: "(?:a|aa){1,8}b", + mode: "regex", + case_sensitive: true, + offset: 0, + limit: 1, + }, + }), + ).resolves.toEqual( + invalidRequest( + "search", + "Regex search exceeds the 1000000-unit work budget", + ), + ); + }); + + it("rejects oversized regex candidates instead of matching a prefix", async () => { + await expect( + probe({ + action: "match", + pattern: "needle", + value: `${"x".repeat(4_096)}needle`, + }), + ).resolves.toEqual( + invalidRequest( + "match", + "Regex candidate exceeds the 4096-character safety limit", + ), + ); + }); + + it("accepts a regex candidate at the exact character limit", async () => { + await expect( + probe({ + action: "match", + pattern: "z$", + value: `${"a".repeat(4_095)}z`, + case_sensitive: true, + }), + ).resolves.toEqual({ + action: "match", + ok: true, + backtracking_paths: 1, + matched: true, + }); + }); + + it.each([ + [256, true], + [257, false], + ])("enforces the %i-character pattern boundary", async (length, accepted) => { + const pattern = "a".repeat(length); + const result = await probe({ + action: "search", + items: [["0x1000", pattern]], + params: { + pattern, + mode: "regex", + case_sensitive: true, + offset: 0, + limit: 1, + }, + }); + if (accepted) { + expect(result).toMatchObject({ + action: "search", + ok: true, + result: { total: 1 }, + }); + return; + } + expect(result).toEqual( + invalidRequest( + "search", + "pattern must contain between 1 and 256 characters", + ), + ); + }); + + it("keeps literal matching exhaustive while bounding returned text", async () => { + const result = await probe({ + action: "search", + items: [["0x1000", `${"x".repeat(4_096)}needle`]], + params: { + pattern: "needle", + mode: "literal", + case_sensitive: true, + offset: 0, + limit: 1, + }, + }); + expect(result).toMatchObject({ + action: "search", + ok: true, + result: { + total: 1, + has_more: false, + items: [{ address: "0x1000", value_truncated: true }], + }, + }); + if (result.action === "search" && result.ok) + expect(result.result.items[0]?.value).toHaveLength(4_096); + }); +}); diff --git a/tests/bridgeTruthfulness.test.ts b/tests/bridgeTruthfulness.test.ts index 2a46717aa..29df93b85 100644 --- a/tests/bridgeTruthfulness.test.ts +++ b/tests/bridgeTruthfulness.test.ts @@ -72,6 +72,11 @@ describe("Hopper bridge truthfulness", () => { ); expect(bridgeSource).toContain("MAX_SEARCH_PATTERN_LENGTH = 256"); expect(bridgeSource).toContain("MAX_SEARCH_VALUE_LENGTH = 4096"); + expect(bridgeSource).toContain("MAX_REGEX_BACKTRACKING_PATHS = 10000"); + expect(bridgeSource).toContain("MAX_REGEX_CANDIDATE_LENGTH = 4096"); + expect(bridgeSource).toContain("MAX_REGEX_SEARCH_WORK_UNITS = 1000000"); + expect(bridgeSource).toContain("_checked_regex_paths("); + expect(bridgeSource).toContain("_bounded_regex_matcher("); expect(bridgeSource).toContain('"value_truncated"'); expect(bridgeSource).toContain("page_end = offset + limit"); expect(bridgeSource).toContain("if offset <= total < page_end:"); diff --git a/tests/fixtures/bridgeRegexProbe.py b/tests/fixtures/bridgeRegexProbe.py new file mode 100644 index 000000000..48509dba0 --- /dev/null +++ b/tests/fixtures/bridgeRegexProbe.py @@ -0,0 +1,62 @@ +"""Execute REA's bridge regex helpers without starting a Hopper socket.""" + +import json +from pathlib import Path +import sys + + +def _load_bridge(path): + source = Path(path).read_text(encoding="utf-8") + prefix, marker, suffix = source.rpartition("\n_run()") + if not marker or suffix.strip(): + raise RuntimeError("bridge entrypoint shape changed") + namespace = {"__file__": path} + exec(compile(prefix, path, "exec"), namespace) + return namespace + + +def _probe(namespace, payload): + action = payload["action"] + try: + if action == "match": + parsed = namespace["sre_parse"].parse(payload["pattern"]) + paths, steps = namespace["_validate_regex_node"](parsed) + flags = ( + 0 + if payload.get("case_sensitive", False) + else namespace["re"].IGNORECASE + ) + expression = namespace["re"].compile(payload["pattern"], flags) + matcher = namespace["_bounded_regex_matcher"](expression, paths, steps) + matched = matcher(payload["value"]) + return { + "action": action, + "ok": True, + "backtracking_paths": paths, + "matched": matched, + } + if action == "search": + inventory = tuple(tuple(item) for item in payload["items"]) + namespace["_search_inventory"] = lambda _document, _kind: inventory + return { + "action": action, + "ok": True, + "result": namespace["_search_page"]( + object(), "string", payload["params"] + ), + } + raise ValueError("unknown probe action") + except Exception as error: + return { + "action": action, + "ok": False, + "type": type(error).__name__, + "diagnostic_type": namespace["_diagnostic_type"](error), + "message": str(error), + } + + +if __name__ == "__main__": + bridge = _load_bridge(sys.argv[1]) + request = json.loads(sys.argv[2]) + print(json.dumps(_probe(bridge, request), sort_keys=True))