From 37f2507d37c56dba749c54b4c926d025a2257211 Mon Sep 17 00:00:00 2001 From: rookiestar28 Date: Fri, 31 Jul 2026 07:03:52 +0800 Subject: [PATCH] fix(parameter-lab): bound experiment inputs --- services/parameter_lab.py | 219 +++++----- services/parameter_lab_policy.py | 253 ++++++++++++ tests/architecture_dependency_policy.json | 1 + tests/e2e/specs/parameter_lab.spec.js | 96 ++++- tests/static_analysis_policy.json | 21 - tests/test_architecture_dependency_policy.py | 2 +- tests/test_r238_parameter_lab_boundaries.py | 391 ++++++++++++++++++ web/openclaw_graph_host.js | 4 +- web/openclaw_parameter_lab_policy.js | 198 +++++++++ web/tabs/parameter_lab_tab.js | 124 ++++-- web/tests/unit/openclaw_graph_host.test.js | 29 ++ .../openclaw_parameter_lab_policy.test.js | 123 ++++++ 12 files changed, 1308 insertions(+), 153 deletions(-) create mode 100644 services/parameter_lab_policy.py create mode 100644 tests/test_r238_parameter_lab_boundaries.py create mode 100644 web/openclaw_parameter_lab_policy.js create mode 100644 web/tests/unit/openclaw_parameter_lab_policy.test.js diff --git a/services/parameter_lab.py b/services/parameter_lab.py index a1690fc..9a1cb5a 100644 --- a/services/parameter_lab.py +++ b/services/parameter_lab.py @@ -10,7 +10,7 @@ import json import logging import time import uuid -from dataclasses import asdict, dataclass, field +from dataclasses import asdict, dataclass, field, replace from pathlib import Path from typing import Any, Dict, List, Optional @@ -29,6 +29,44 @@ else: # pragma: no cover (test-only import mode) check_rate_limit, ) +if __package__ and "." in __package__: + from ..services import parameter_lab_policy as _parameter_lab_policy + from ..services.parameter_lab_policy import ( + ParameterLabValidationError, + serialize_plan_payload, + validate_compare_input, + validate_sweep_dimensions, + validate_workflow, + ) + from ..services.safe_io import safe_write_text +else: # pragma: no cover (test-only import mode) + from services import parameter_lab_policy as _parameter_lab_policy + from services.parameter_lab_policy import ( + ParameterLabValidationError, + serialize_plan_payload, + validate_compare_input, + validate_sweep_dimensions, + validate_workflow, + ) + from services.safe_io import safe_write_text + +PARAMETER_LAB_POLICY_VERSION = _parameter_lab_policy.PARAMETER_LAB_POLICY_VERSION +PARAMETER_LAB_POLICY = _parameter_lab_policy.PARAMETER_LAB_POLICY +MAX_PARAMETER_LAB_REQUEST_BYTES = _parameter_lab_policy.MAX_PARAMETER_LAB_REQUEST_BYTES +MAX_PARAMETER_LAB_WORKFLOW_UTF8_BYTES = ( + _parameter_lab_policy.MAX_PARAMETER_LAB_WORKFLOW_UTF8_BYTES +) +MAX_SWEEP_DIMENSIONS = _parameter_lab_policy.MAX_SWEEP_DIMENSIONS +MAX_VALUES_PER_DIMENSION = _parameter_lab_policy.MAX_VALUES_PER_DIMENSION +MAX_NODE_ID_UTF8_BYTES = _parameter_lab_policy.MAX_NODE_ID_UTF8_BYTES +MAX_WIDGET_NAME_UTF8_BYTES = _parameter_lab_policy.MAX_WIDGET_NAME_UTF8_BYTES +MAX_SCALAR_STRING_UTF8_BYTES = _parameter_lab_policy.MAX_SCALAR_STRING_UTF8_BYTES +MAX_PARAMETER_LAB_PLAN_UTF8_BYTES = ( + _parameter_lab_policy.MAX_PARAMETER_LAB_PLAN_UTF8_BYTES +) +MAX_SWEEP_COMBINATIONS = _parameter_lab_policy.MAX_SWEEP_COMBINATIONS +MAX_COMPARE_ITEMS = _parameter_lab_policy.MAX_COMPARE_ITEMS + # R98: Endpoint Metadata if __package__ and "." in __package__: from ..services.endpoint_manifest import ( @@ -48,8 +86,6 @@ else: logger = logging.getLogger("ComfyUI-OpenClaw.services.parameter_lab") # Configuration -MAX_SWEEP_COMBINATIONS = 50 # Hard cap to prevent queue flooding -MAX_COMPARE_ITEMS = 8 # F50: Hard cap for side-by-side comparison EXPERIMENT_RETENTION_COUNT = 20 @@ -79,48 +115,27 @@ class SweepPlan: class SweepPlanner: """Generates bounded sweep plans.""" - def generate(self, workflow: str, params: List[Dict[str, Any]]) -> SweepPlan: - if not isinstance(workflow, str) or not workflow.strip(): - raise ValueError("workflow_json is required") - if not isinstance(params, list): - raise ValueError("params must be a list") - - exp_id = f"exp_{uuid.uuid4().hex[:8]}" - dimensions: List[SweepDimension] = [] - - for p in params: - if not isinstance(p, dict): - continue - node_id = p.get("node_id") - widget_name = p.get("widget_name") - if node_id is None or not isinstance(widget_name, str) or not widget_name: - continue - - dim = SweepDimension( - node_id=str(node_id), - widget_name=widget_name, - values=( - p.get("values", []) if isinstance(p.get("values", []), list) else [] - ), - strategy=str(p.get("strategy", "grid")), - count=int(p.get("count", 0) or 0), + def generate(self, workflow: Any, params: List[Dict[str, Any]]) -> SweepPlan: + normalized_workflow = validate_workflow(workflow) + normalized_params = validate_sweep_dimensions(params) + dimensions: List[SweepDimension] = [ + SweepDimension( + node_id=dimension["node_id"], + widget_name=dimension["widget_name"], + values=dimension["values"], + strategy=dimension["strategy"], + count=dimension["count"], ) - dimensions.append(dim) - + for dimension in normalized_params + ] overrides_list = self._generate_combinations(dimensions) - # F52: Bounded Invariant Check - count = len(overrides_list) - if count > MAX_SWEEP_COMBINATIONS: - raise ValueError( - f"Sweep size {count} exceeds limit {MAX_SWEEP_COMBINATIONS}" - ) - return SweepPlan( - experiment_id=exp_id, - workflow_json=workflow, + # IMPORTANT: validate a same-length placeholder before allocating any experiment ID. + candidate = SweepPlan( + experiment_id="exp_00000000", + workflow_json=normalized_workflow, dimensions=dimensions, runs=overrides_list, - # F52: Schema V1 Lock schema_version="1.0", combination_cap=MAX_SWEEP_COMBINATIONS, budget_cap=MAX_SWEEP_COMBINATIONS, @@ -130,6 +145,8 @@ class SweepPlanner: "lock_reason": "f52_closeout", }, ) + serialize_plan_payload(asdict(candidate)) + return replace(candidate, experiment_id=f"exp_{uuid.uuid4().hex[:8]}") def _generate_combinations( self, dimensions: List[SweepDimension] @@ -173,37 +190,18 @@ class ComparePlanner: """ def generate( - self, workflow: str, items: List[Any], node_id: Any, widget_name: str + self, workflow: Any, items: List[Any], node_id: Any, widget_name: str ) -> SweepPlan: - if not isinstance(workflow, str) or not workflow.strip(): - raise ValueError("workflow_json is required") - if not isinstance(items, list) or not items: - raise ValueError("items must be a non-empty list") - if node_id is None: - raise ValueError("node_id is required") - if not isinstance(widget_name, str) or not widget_name.strip(): - raise ValueError("widget_name is required") - if len(items) > MAX_COMPARE_ITEMS: - raise ValueError(f"Too many items for comparison (max {MAX_COMPARE_ITEMS})") - - normalized_items: List[Any] = [] - for item in items: - if isinstance(item, str): - if not item.strip(): - raise ValueError("items must not contain empty strings") - normalized_items.append(item) - continue - if isinstance(item, (int, float, bool)): - normalized_items.append(item) - continue - raise ValueError("items must contain only scalar values") - - exp_id = f"cmp_{uuid.uuid4().hex[:8]}" + normalized_workflow = validate_workflow(workflow) + validated_compare = validate_compare_input(items, node_id, widget_name) + normalized_items: List[Any] = validated_compare[0] + normalized_node_id = validated_compare[1] + normalized_widget_name = validated_compare[2] # Create a single dimension for the model/item dim = SweepDimension( - node_id=str(node_id), - widget_name=widget_name, + node_id=normalized_node_id, + widget_name=normalized_widget_name, values=normalized_items, strategy="compare", ) @@ -211,23 +209,24 @@ class ComparePlanner: # Generate runs (1 per item) runs = [] for val in normalized_items: - runs.append({f"{node_id}.{widget_name}": val}) + runs.append({f"{normalized_node_id}.{normalized_widget_name}": val}) - return SweepPlan( - experiment_id=exp_id, - workflow_json=workflow, + candidate = SweepPlan( + experiment_id="cmp_00000000", + workflow_json=normalized_workflow, dimensions=[dim], runs=runs, - # F52: Schema V1 Lock schema_version="1.0", combination_cap=MAX_COMPARE_ITEMS, - budget_cap=MAX_COMPARE_ITEMS, # F50: Budget aligns with compare limit + budget_cap=MAX_COMPARE_ITEMS, replay_metadata={ "replay_input_version": "1.0", "compat_state": "supported", "lock_reason": "f50_closeout", }, ) + serialize_plan_payload(asdict(candidate)) + return replace(candidate, experiment_id=f"cmp_{uuid.uuid4().hex[:8]}") _compare_planner = ComparePlanner() @@ -264,9 +263,14 @@ class ExperimentStore: logger.warning("Retention check failed: %s", exc) def save_plan(self, plan: SweepPlan) -> None: - path = self.store_dir / f"{plan.experiment_id}.json" - with open(path, "w", encoding="utf-8") as handle: - json.dump(asdict(plan), handle, indent=2) + serialized = serialize_plan_payload(asdict(plan)) + # IMPORTANT: keep validation before file creation and retention mutation. + safe_write_text( + str(self.store_dir), + f"{plan.experiment_id}.json", + serialized, + atomic=True, + ) self._enforce_retention() def get_plan(self, exp_id: str) -> Optional[Dict[str, Any]]: @@ -286,7 +290,7 @@ class ExperimentStore: "note": "Legacy experiment; full replay guarantees not active", } - return data + return data # type: ignore[no-any-return] except Exception: return None @@ -393,6 +397,36 @@ def _require_admin(request: web.Request) -> Optional[web.Response]: return None +async def _read_creation_payload(request: web.Request) -> dict[str, Any]: + content_length = request.content_length + if content_length is not None and content_length > MAX_PARAMETER_LAB_REQUEST_BYTES: + raise ParameterLabValidationError("payload_too_large", status=413) + + raw_body = bytearray() + while True: + remaining = MAX_PARAMETER_LAB_REQUEST_BYTES + 1 - len(raw_body) + if remaining <= 0: + raise ParameterLabValidationError("payload_too_large", status=413) + chunk = await request.content.read(min(64 * 1024, remaining)) + if not chunk: + break + raw_body.extend(chunk) + if len(raw_body) > MAX_PARAMETER_LAB_REQUEST_BYTES: + raise ParameterLabValidationError("payload_too_large", status=413) + + try: + data = json.loads(raw_body.decode("utf-8")) + except (UnicodeDecodeError, json.JSONDecodeError) as exc: + raise ParameterLabValidationError("invalid_json") from exc + if not isinstance(data, dict): + raise ParameterLabValidationError("invalid_payload") + return data + + +def _validation_response(exc: ParameterLabValidationError) -> web.Response: + return web.json_response({"ok": False, "error": exc.code}, status=exc.status) + + @endpoint_metadata( auth=AuthTier.ADMIN, risk=RiskTier.MEDIUM, @@ -410,13 +444,9 @@ async def create_compare_handler(request: web.Request) -> web.Response: return deny try: - data = await request.json() - except Exception: - return web.json_response({"ok": False, "error": "invalid_json"}, status=400) - - # Input validation. - if not isinstance(data, dict): - return web.json_response({"ok": False, "error": "invalid_payload"}, status=400) + data = await _read_creation_payload(request) + except ParameterLabValidationError as exc: + return _validation_response(exc) workflow = data.get("workflow_json") items = data.get("items", []) # List of comparison values. @@ -438,10 +468,10 @@ async def create_compare_handler(request: web.Request) -> web.Response: plan = _compare_planner.generate(workflow, items, node_id, widget_name) get_store().save_plan(plan) return web.json_response({"ok": True, "plan": asdict(plan)}) - except ValueError as exc: - return web.json_response({"ok": False, "error": str(exc)}, status=400) + except ParameterLabValidationError as exc: + return _validation_response(exc) except Exception as exc: - logger.error("Compare creation failed: %s", exc) + logger.error("Compare creation failed (%s)", type(exc).__name__) return web.json_response({"ok": False, "error": "internal_error"}, status=500) @@ -462,12 +492,9 @@ async def create_sweep_handler(request: web.Request) -> web.Response: return deny try: - data = await request.json() - except Exception: - return web.json_response({"ok": False, "error": "invalid_json"}, status=400) - - if not isinstance(data, dict): - return web.json_response({"ok": False, "error": "invalid_payload"}, status=400) + data = await _read_creation_payload(request) + except ParameterLabValidationError as exc: + return _validation_response(exc) workflow = data.get("workflow_json") params = data.get("params", []) @@ -476,10 +503,10 @@ async def create_sweep_handler(request: web.Request) -> web.Response: plan = _planner.generate(workflow, params) get_store().save_plan(plan) return web.json_response({"ok": True, "plan": asdict(plan)}) - except ValueError as exc: - return web.json_response({"ok": False, "error": str(exc)}, status=400) + except ParameterLabValidationError as exc: + return _validation_response(exc) except Exception as exc: - logger.error("Sweep creation failed: %s", exc) + logger.error("Sweep creation failed (%s)", type(exc).__name__) return web.json_response({"ok": False, "error": "internal_error"}, status=500) diff --git a/services/parameter_lab_policy.py b/services/parameter_lab_policy.py new file mode 100644 index 0000000..d3ad4e1 --- /dev/null +++ b/services/parameter_lab_policy.py @@ -0,0 +1,253 @@ +"""Versioned, dependency-light validation policy for Parameter Lab creation.""" + +from __future__ import annotations + +import json +import math +from types import MappingProxyType +from typing import Any + +PARAMETER_LAB_POLICY_VERSION = "1.0" +MAX_PARAMETER_LAB_REQUEST_BYTES = 5 * 1024 * 1024 +MAX_PARAMETER_LAB_WORKFLOW_UTF8_BYTES = 4 * 1024 * 1024 +MAX_SWEEP_DIMENSIONS = 8 +MAX_VALUES_PER_DIMENSION = 50 +MAX_NODE_ID_UTF8_BYTES = 128 +MAX_WIDGET_NAME_UTF8_BYTES = 256 +MAX_SCALAR_STRING_UTF8_BYTES = 16 * 1024 +MAX_PARAMETER_LAB_PLAN_UTF8_BYTES = 8 * 1024 * 1024 +MAX_SWEEP_COMBINATIONS = 50 +MAX_COMPARE_ITEMS = 8 + +PARAMETER_LAB_POLICY = MappingProxyType( + { + "version": PARAMETER_LAB_POLICY_VERSION, + "max_request_bytes": MAX_PARAMETER_LAB_REQUEST_BYTES, + "max_workflow_utf8_bytes": MAX_PARAMETER_LAB_WORKFLOW_UTF8_BYTES, + "max_sweep_dimensions": MAX_SWEEP_DIMENSIONS, + "max_values_per_dimension": MAX_VALUES_PER_DIMENSION, + "max_node_id_utf8_bytes": MAX_NODE_ID_UTF8_BYTES, + "max_widget_name_utf8_bytes": MAX_WIDGET_NAME_UTF8_BYTES, + "max_scalar_string_utf8_bytes": MAX_SCALAR_STRING_UTF8_BYTES, + "max_plan_utf8_bytes": MAX_PARAMETER_LAB_PLAN_UTF8_BYTES, + "max_sweep_combinations": MAX_SWEEP_COMBINATIONS, + "max_compare_items": MAX_COMPARE_ITEMS, + } +) + +_ERROR_MESSAGES = MappingProxyType( + { + "payload_too_large": "Parameter Lab request exceeds the byte limit", + "invalid_json": "Request body must be valid JSON", + "invalid_payload": "Request payload must be an object", + "workflow_required": "workflow_json is required", + "workflow_too_large": "workflow_json exceeds the byte limit", + "params_must_be_list": "params must be a list", + "items_must_be_list": "items must be a non-empty list", + "dimensions_required": "At least one sweep dimension is required", + "too_many_dimensions": "Too many sweep dimensions", + "invalid_dimension": "Each sweep dimension must be an object", + "node_id_required": "node_id is required", + "invalid_node_id": "node_id is not a supported identifier", + "node_id_too_large": "node_id exceeds the byte limit", + "widget_name_required": "widget_name is required", + "invalid_widget_name": "widget_name is not a supported identifier", + "widget_name_too_large": "widget_name exceeds the byte limit", + "values_required": "values must be a non-empty list", + "too_many_values": ( + f"Values per dimension exceeds limit {MAX_VALUES_PER_DIMENSION}" + ), + "invalid_scalar_value": "items must contain only scalar values", + "scalar_string_too_large": "A scalar string exceeds the byte limit", + "duplicate_ambiguous_value": "Values contain a presentation-ambiguous duplicate", + "duplicate_dimension": "Duplicate node/widget dimension", + "invalid_strategy": "Only grid sweep strategy is supported", + "sweep_too_large": (f"Sweep size exceeds limit {MAX_SWEEP_COMBINATIONS}"), + "plan_too_large": "Serialized Parameter Lab plan exceeds the byte limit", + } +) + + +class ParameterLabValidationError(ValueError): + """Content-free creation validation error with a stable public reason code.""" + + def __init__(self, code: str, *, status: int = 400, message: str | None = None): + self.code = code + self.status = status + super().__init__( + message or _ERROR_MESSAGES.get(code, "Invalid Parameter Lab request") + ) + + +def utf8_size(value: str) -> int: + return len(value.encode("utf-8")) + + +def _contains_control(value: str) -> bool: + return any(ord(char) < 32 or ord(char) == 127 for char in value) + + +def validate_workflow(workflow: Any) -> str: + if not isinstance(workflow, str) or not workflow.strip(): + raise ParameterLabValidationError("workflow_required") + if utf8_size(workflow) > MAX_PARAMETER_LAB_WORKFLOW_UTF8_BYTES: + raise ParameterLabValidationError("workflow_too_large", status=413) + return workflow + + +def normalize_node_id(node_id: Any) -> str: + if node_id is None: + raise ParameterLabValidationError("node_id_required") + if isinstance(node_id, bool): + raise ParameterLabValidationError("invalid_node_id") + if isinstance(node_id, int): + normalized = str(node_id) + elif isinstance(node_id, str): + normalized = node_id + else: + raise ParameterLabValidationError("invalid_node_id") + if not normalized.strip(): + raise ParameterLabValidationError("node_id_required") + # IMPORTANT: run override keys use the first "." as the node/widget separator. + if "." in normalized or _contains_control(normalized): + raise ParameterLabValidationError("invalid_node_id") + if utf8_size(normalized) > MAX_NODE_ID_UTF8_BYTES: + raise ParameterLabValidationError("node_id_too_large") + return normalized + + +def normalize_widget_name(widget_name: Any) -> str: + if not isinstance(widget_name, str) or not widget_name.strip(): + raise ParameterLabValidationError("widget_name_required") + if _contains_control(widget_name): + raise ParameterLabValidationError("invalid_widget_name") + if utf8_size(widget_name) > MAX_WIDGET_NAME_UTF8_BYTES: + raise ParameterLabValidationError("widget_name_too_large") + return widget_name + + +def _normalize_scalar(value: Any, *, allow_empty_string: bool) -> Any: + if isinstance(value, str): + if not allow_empty_string and not value.strip(): + raise ParameterLabValidationError("invalid_scalar_value") + if utf8_size(value) > MAX_SCALAR_STRING_UTF8_BYTES: + raise ParameterLabValidationError("scalar_string_too_large") + return value + if isinstance(value, bool): + return value + if isinstance(value, int): + return value + if isinstance(value, float) and math.isfinite(value): + return value + raise ParameterLabValidationError("invalid_scalar_value") + + +def _presentation_key(value: Any) -> str: + if isinstance(value, str): + return value + if isinstance(value, bool): + return "true" if value else "false" + if isinstance(value, int): + return str(value) + if value == 0: + return "0" + if float(value).is_integer(): + return str(int(value)) + return json.dumps(value, allow_nan=False, separators=(",", ":")) + + +def validate_scalar_values( + values: Any, + *, + max_values: int = MAX_VALUES_PER_DIMENSION, + allow_empty_string: bool = True, +) -> list[Any]: + if not isinstance(values, list) or not values: + raise ParameterLabValidationError("values_required") + if len(values) > max_values: + raise ParameterLabValidationError("too_many_values") + normalized: list[Any] = [] + seen_presentations = set() + for value in values: + scalar = _normalize_scalar(value, allow_empty_string=allow_empty_string) + presentation = _presentation_key(scalar) + if presentation in seen_presentations: + raise ParameterLabValidationError("duplicate_ambiguous_value") + seen_presentations.add(presentation) + normalized.append(scalar) + return normalized + + +def validate_sweep_dimensions(params: Any) -> list[dict[str, Any]]: + if not isinstance(params, list): + raise ParameterLabValidationError("params_must_be_list") + if not params: + raise ParameterLabValidationError("dimensions_required") + if len(params) > MAX_SWEEP_DIMENSIONS: + raise ParameterLabValidationError("too_many_dimensions") + + normalized: list[dict[str, Any]] = [] + seen_dimensions = set() + combinations = 1 + for raw_dimension in params: + if not isinstance(raw_dimension, dict): + raise ParameterLabValidationError("invalid_dimension") + node_id = normalize_node_id(raw_dimension.get("node_id")) + widget_name = normalize_widget_name(raw_dimension.get("widget_name")) + dimension_key = (node_id, widget_name) + if dimension_key in seen_dimensions: + raise ParameterLabValidationError("duplicate_dimension") + seen_dimensions.add(dimension_key) + + strategy = raw_dimension.get("strategy", "grid") + if strategy != "grid": + raise ParameterLabValidationError("invalid_strategy") + values = validate_scalar_values(raw_dimension.get("values")) + combinations *= len(values) + if combinations > MAX_SWEEP_COMBINATIONS: + raise ParameterLabValidationError("sweep_too_large") + normalized.append( + { + "node_id": node_id, + "widget_name": widget_name, + "values": values, + "strategy": "grid", + "count": 0, + } + ) + return normalized + + +def validate_compare_input( + items: Any, node_id: Any, widget_name: Any +) -> tuple[list[Any], str, str]: + if not isinstance(items, list) or not items: + raise ParameterLabValidationError("items_must_be_list") + if len(items) > MAX_COMPARE_ITEMS: + raise ParameterLabValidationError( + "too_many_values", + message=f"Too many items for comparison (max {MAX_COMPARE_ITEMS})", + ) + normalized_node_id = normalize_node_id(node_id) + normalized_widget_name = normalize_widget_name(widget_name) + normalized_items = validate_scalar_values( + items, + max_values=MAX_COMPARE_ITEMS, + allow_empty_string=False, + ) + return normalized_items, normalized_node_id, normalized_widget_name + + +def serialize_plan_payload(payload: Any) -> str: + try: + serialized = json.dumps( + payload, + indent=2, + ensure_ascii=False, + allow_nan=False, + ) + except (TypeError, ValueError) as exc: + raise ParameterLabValidationError("invalid_scalar_value") from exc + if utf8_size(serialized) > MAX_PARAMETER_LAB_PLAN_UTF8_BYTES: + raise ParameterLabValidationError("plan_too_large", status=413) + return serialized diff --git a/tests/architecture_dependency_policy.json b/tests/architecture_dependency_policy.json index 5ca4140..d2579b0 100644 --- a/tests/architecture_dependency_policy.json +++ b/tests/architecture_dependency_policy.json @@ -232,6 +232,7 @@ "services/packs/pack_registry.py", "services/packs/pack_types.py", "services/parameter_lab.py", + "services/parameter_lab_policy.py", "services/paths.py", "services/permission_posture.py", "services/planner.py", diff --git a/tests/e2e/specs/parameter_lab.spec.js b/tests/e2e/specs/parameter_lab.spec.js index 7429b6e..95344b6 100644 --- a/tests/e2e/specs/parameter_lab.spec.js +++ b/tests/e2e/specs/parameter_lab.spec.js @@ -19,7 +19,13 @@ test.describe('Parameter Lab - Dynamic Dimensions', () => { widgets: [ { name: "seed", type: "number", value: 1234, options: {} }, { name: "steps", type: "number", value: 20, options: { values: [20, 30, 40] } }, - { name: "sampler_name", type: "combo", value: "euler", options: { values: ["euler", "ddim", "uni_pc"] } } + { name: "sampler_name", type: "combo", value: "euler", options: { values: ["euler", "ddim", "uni_pc"] } }, + { + name: "video_edit", + type: "VIDEO_EDIT", + value: { trim: [0, 1] }, + options: { values: [{ trim: [0, 1] }, ["structured"]] } + } ] }, { @@ -87,6 +93,94 @@ test.describe('Parameter Lab - Dynamic Dimensions', () => { await expect(page.locator('.openclaw-chip >> text=9999')).toBeVisible(); }); + test('rejects oversized manual scalar values before chip state mutation', async ({ page }) => { + await page.click('#lab-add-dim'); + await page.selectOption('.dim-node-select', { value: '10' }); + await page.selectOption('.dim-widget-select', { value: 'seed' }); + + await page.fill('.dim-manual-input', '界'.repeat(5462)); + await page.press('.dim-manual-input', 'Enter'); + + await expect(page.locator('.openclaw-chip')).toHaveCount(0); + await expect(page.locator('.openclaw-banner')).toContainText('scalar_string_too_large'); + }); + + test('does not offer structured widget values as ambiguous object candidates', async ({ page }) => { + await page.click('#lab-add-dim'); + await page.selectOption('.dim-node-select', { value: '10' }); + await page.selectOption('.dim-widget-select', { value: 'video_edit' }); + + const candidates = page.locator('.dim-candidate-select option'); + await expect(candidates).toHaveCount(1); + await expect(candidates).not.toContainText('[object Object]'); + }); + + test('caps dimensions before creating ambiguous experiment state', async ({ page }) => { + for (let index = 0; index < 9; index += 1) { + await page.click('#lab-add-dim'); + } + + await expect(page.locator('.openclaw-lab-dim-row.dynamic')).toHaveCount(8); + await expect(page.locator('.openclaw-banner')).toContainText('too_many_dimensions'); + }); + + test('rejects an oversized serialized workflow before the API request', async ({ page }) => { + await page.evaluate(async () => { + const mod = await import('/web/openclaw_api.js'); + window.__labRequestCount = 0; + const originalFetch = mod.openclawApi.fetch.bind(mod.openclawApi); + mod.openclawApi.fetch = async (url, options = {}) => { + const normalizedPath = String(url || '').replace(/^\/moltbot/, '/openclaw'); + if (normalizedPath.endsWith('/lab/sweep')) { + window.__labRequestCount += 1; + return { ok: false, status: 400, error: 'unexpected_request' }; + } + return originalFetch(url, options); + }; + window.app.graph.serialize = () => ({ payload: 'x'.repeat(4 * 1024 * 1024 + 1) }); + }); + + await page.click('#lab-add-dim'); + await page.selectOption('.dim-node-select', { value: '10' }); + await page.selectOption('.dim-widget-select', { value: 'seed' }); + await page.fill('.dim-manual-input', '1'); + await page.press('.dim-manual-input', 'Enter'); + await page.click('#lab-generate'); + + await expect.poll(() => page.evaluate(() => window.__labRequestCount)).toBe(0); + await expect(page.locator('.openclaw-banner')).toContainText('workflow_too_large'); + }); + + test('redacts workflow serialization failures before the API request', async ({ page }) => { + await page.evaluate(async () => { + const mod = await import('/web/openclaw_api.js'); + window.__labRequestCount = 0; + const originalFetch = mod.openclawApi.fetch.bind(mod.openclawApi); + mod.openclawApi.fetch = async (url, options = {}) => { + const normalizedPath = String(url || '').replace(/^\/moltbot/, '/openclaw'); + if (normalizedPath.endsWith('/lab/sweep')) { + window.__labRequestCount += 1; + return { ok: false, status: 400, error: 'unexpected_request' }; + } + return originalFetch(url, options); + }; + window.app.graph.serialize = () => { + throw new Error('secret=workflow-private-detail'); + }; + }); + + await page.click('#lab-add-dim'); + await page.selectOption('.dim-node-select', { value: '10' }); + await page.selectOption('.dim-widget-select', { value: 'seed' }); + await page.fill('.dim-manual-input', '1'); + await page.press('.dim-manual-input', 'Enter'); + await page.click('#lab-generate'); + + await expect.poll(() => page.evaluate(() => window.__labRequestCount)).toBe(0); + await expect(page.locator('.openclaw-banner')).toContainText('invalid_payload'); + await expect(page.locator('.openclaw-banner')).not.toContainText('workflow-private-detail'); + }); + test('generates correct plan payload', async ({ page }) => { await page.evaluate(async () => { const mod = await import('/web/openclaw_api.js'); diff --git a/tests/static_analysis_policy.json b/tests/static_analysis_policy.json index 3c0eb68..fde0667 100644 --- a/tests/static_analysis_policy.json +++ b/tests/static_analysis_policy.json @@ -1421,27 +1421,6 @@ "message": "Incompatible return value type (got \"object\", expected \"SupportsDunderLT[Any] | SupportsDunderGT[Any]\")", "count": 1 }, - { - "tool": "mypy", - "path": "services/parameter_lab.py", - "code": "arg-type", - "message": "Argument 1 to \"generate\" of \"ComparePlanner\" has incompatible type \"Any | None\"; expected \"str\"", - "count": 1 - }, - { - "tool": "mypy", - "path": "services/parameter_lab.py", - "code": "arg-type", - "message": "Argument 1 to \"generate\" of \"SweepPlanner\" has incompatible type \"Any | None\"; expected \"str\"", - "count": 1 - }, - { - "tool": "mypy", - "path": "services/parameter_lab.py", - "code": "no-any-return", - "message": "Returning Any from function declared to return \"dict[str, Any] | None\"", - "count": 1 - }, { "tool": "mypy", "path": "services/parameter_lab.py", diff --git a/tests/test_architecture_dependency_policy.py b/tests/test_architecture_dependency_policy.py index cd37bcb..36e6b50 100644 --- a/tests/test_architecture_dependency_policy.py +++ b/tests/test_architecture_dependency_policy.py @@ -289,7 +289,7 @@ class RepositoryArchitecturePolicyTests(unittest.TestCase): analysis = dependency_policy.analyze_repository(self.repo_root, policy) self.assertEqual(analysis.findings, ()) - self.assertEqual(len(analysis.owned_paths), 298) + self.assertEqual(len(analysis.owned_paths), 299) self.assertEqual(len(policy["accepted_cycles"]), 2) self.assertEqual(len(policy["dynamic_imports"]), 8) self.assertEqual(len(policy["compatibility_exceptions"]), 9) diff --git a/tests/test_r238_parameter_lab_boundaries.py b/tests/test_r238_parameter_lab_boundaries.py new file mode 100644 index 0000000..cfe122b --- /dev/null +++ b/tests/test_r238_parameter_lab_boundaries.py @@ -0,0 +1,391 @@ +import io +import tempfile +import unittest +from pathlib import Path +from unittest.mock import MagicMock, patch + +from services import parameter_lab as lab + +try: + from aiohttp import web + from aiohttp.test_utils import AioHTTPTestCase, unittest_run_loop +except Exception: # pragma: no cover + web = None # type: ignore + AioHTTPTestCase = unittest.TestCase # type: ignore + + def unittest_run_loop(fn): # type: ignore + return fn + + +MIB = 1024 * 1024 +KIB = 1024 + + +class TestR238PlannerBoundaries(unittest.TestCase): + def setUp(self): + self.planner = lab.SweepPlanner() + + def assert_reason(self, ctx, expected): + self.assertEqual(expected, getattr(ctx.exception, "code", None)) + + def test_policy_constants_are_frozen(self): + expected = { + "PARAMETER_LAB_POLICY_VERSION": "1.0", + "MAX_PARAMETER_LAB_REQUEST_BYTES": 5 * MIB, + "MAX_PARAMETER_LAB_WORKFLOW_UTF8_BYTES": 4 * MIB, + "MAX_SWEEP_DIMENSIONS": 8, + "MAX_VALUES_PER_DIMENSION": 50, + "MAX_NODE_ID_UTF8_BYTES": 128, + "MAX_WIDGET_NAME_UTF8_BYTES": 256, + "MAX_SCALAR_STRING_UTF8_BYTES": 16 * KIB, + "MAX_PARAMETER_LAB_PLAN_UTF8_BYTES": 8 * MIB, + "MAX_SWEEP_COMBINATIONS": 50, + } + for name, value in expected.items(): + self.assertEqual(value, getattr(lab, name, None), name) + + def test_sweep_rejects_structured_and_non_finite_values(self): + invalid_values = [ + None, + [], + {"rich": "widget"}, + float("nan"), + float("inf"), + float("-inf"), + ] + for value in invalid_values: + with self.subTest(value=type(value).__name__): + with self.assertRaises(ValueError) as ctx: + self.planner.generate( + "{}", + [{"node_id": 1, "widget_name": "seed", "values": [value]}], + ) + self.assert_reason(ctx, "invalid_scalar_value") + + def test_sweep_rejects_dimension_and_value_count_limits(self): + with self.assertRaises(ValueError) as ctx: + self.planner.generate( + "{}", + [ + {"node_id": index, "widget_name": "seed", "values": [index]} + for index in range(9) + ], + ) + self.assert_reason(ctx, "too_many_dimensions") + + with self.assertRaises(ValueError) as ctx: + self.planner.generate( + "{}", + [ + { + "node_id": 1, + "widget_name": "seed", + "values": list(range(51)), + } + ], + ) + self.assert_reason(ctx, "too_many_values") + + def test_sweep_rejects_malformed_duplicate_and_ambiguous_dimensions(self): + cases = [ + ([{"node_id": 1, "widget_name": "seed"}], "values_required"), + ( + [{"node_id": "bad.id", "widget_name": "seed", "values": [1]}], + "invalid_node_id", + ), + ( + [{"node_id": True, "widget_name": "seed", "values": [1]}], + "invalid_node_id", + ), + ( + [{"node_id": 1, "widget_name": "bad\u0000name", "values": [1]}], + "invalid_widget_name", + ), + ( + [ + {"node_id": 1, "widget_name": "seed", "values": [1]}, + {"node_id": "1", "widget_name": "seed", "values": [2]}, + ], + "duplicate_dimension", + ), + ( + [{"node_id": 1, "widget_name": "seed", "values": [1, "1"]}], + "duplicate_ambiguous_value", + ), + ( + [ + { + "node_id": 1, + "widget_name": "seed", + "values": [1], + "strategy": "random", + } + ], + "invalid_strategy", + ), + ] + for params, reason in cases: + with self.subTest(reason=reason): + with self.assertRaises(ValueError) as ctx: + self.planner.generate("{}", params) + self.assert_reason(ctx, reason) + + def test_sweep_uses_utf8_identifier_and_scalar_string_limits(self): + cases = [ + ( + [{"node_id": "界" * 43, "widget_name": "seed", "values": [1]}], + "node_id_too_large", + ), + ( + [{"node_id": 1, "widget_name": "界" * 86, "values": [1]}], + "widget_name_too_large", + ), + ( + [{"node_id": 1, "widget_name": "seed", "values": ["界" * 5462]}], + "scalar_string_too_large", + ), + ] + for params, reason in cases: + with self.subTest(reason=reason): + with self.assertRaises(ValueError) as ctx: + self.planner.generate("{}", params) + self.assert_reason(ctx, reason) + + def test_exact_workflow_identifier_and_scalar_limits_are_accepted(self): + plan = self.planner.generate( + "x" * (4 * MIB), + [ + { + "node_id": "n" * 128, + "widget_name": "w" * 256, + "values": ["v" * (16 * KIB)], + } + ], + ) + + self.assertEqual(4 * MIB, len(plan.workflow_json.encode("utf-8"))) + self.assertEqual(128, len(plan.dimensions[0].node_id.encode("utf-8"))) + self.assertEqual(256, len(plan.dimensions[0].widget_name.encode("utf-8"))) + self.assertEqual( + 16 * KIB, + len(plan.dimensions[0].values[0].encode("utf-8")), + ) + + def test_workflow_and_plan_limits_run_before_experiment_id_allocation(self): + with patch.object(lab.uuid, "uuid4", wraps=lab.uuid.uuid4) as mock_uuid: + with self.assertRaises(ValueError) as ctx: + self.planner.generate("x" * (4 * MIB + 1), []) + self.assert_reason(ctx, "workflow_too_large") + mock_uuid.assert_not_called() + + scalar = "x" * (16 * KIB) + first_values = [f"{index:02d}" + scalar[2:] for index in range(50)] + params = [ + {"node_id": 1, "widget_name": "w1", "values": first_values}, + *[ + {"node_id": index, "widget_name": f"w{index}", "values": [scalar]} + for index in range(2, 9) + ], + ] + with patch.object(lab.uuid, "uuid4", wraps=lab.uuid.uuid4) as mock_uuid: + with self.assertRaises(ValueError) as ctx: + self.planner.generate("x" * (4 * MIB - KIB), params) + self.assert_reason(ctx, "plan_too_large") + mock_uuid.assert_not_called() + + def test_compare_uses_the_same_scalar_and_identifier_policy(self): + planner = lab.ComparePlanner() + cases = [ + ([None], 1, "ckpt_name", "invalid_scalar_value"), + ([1, "1"], 1, "ckpt_name", "duplicate_ambiguous_value"), + (["a"], "bad.id", "ckpt_name", "invalid_node_id"), + (["a"], 1, "界" * 86, "widget_name_too_large"), + ] + for items, node_id, widget_name, reason in cases: + with self.subTest(reason=reason): + with self.assertRaises(ValueError) as ctx: + planner.generate("{}", items, node_id, widget_name) + self.assert_reason(ctx, reason) + + +class TestR238StoreBoundaries(unittest.TestCase): + def test_store_revalidates_plan_size_before_file_or_retention_mutation(self): + with tempfile.TemporaryDirectory() as tmp_dir: + store = lab.ExperimentStore(Path(tmp_dir)) + plan = lab.SweepPlan( + experiment_id="exp_too_large", + workflow_json="x" * (8 * MIB + 1), + dimensions=[], + runs=[], + ) + with ( + patch.object(store, "_enforce_retention") as retention, + self.assertRaises(ValueError) as ctx, + ): + store.save_plan(plan) + self.assertEqual("plan_too_large", getattr(ctx.exception, "code", None)) + retention.assert_not_called() + self.assertEqual([], list(store.store_dir.glob("*.json"))) + + def test_atomic_write_failure_leaves_no_plan_and_skips_retention(self): + with tempfile.TemporaryDirectory() as tmp_dir: + store = lab.ExperimentStore(Path(tmp_dir)) + plan = lab.SweepPlanner().generate( + "{}", + [{"node_id": "loader-alpha", "widget_name": "seed", "values": [1]}], + ) + with ( + patch.object( + lab, + "safe_write_text", + create=True, + side_effect=OSError("private failure detail"), + ), + patch.object(store, "_enforce_retention") as retention, + self.assertRaises(OSError), + ): + store.save_plan(plan) + retention.assert_not_called() + self.assertEqual([], list(store.store_dir.glob("*.json"))) + + def test_legacy_read_does_not_rewrite_source_file(self): + with tempfile.TemporaryDirectory() as tmp_dir: + store = lab.ExperimentStore(Path(tmp_dir)) + path = store.store_dir / "exp_legacy.json" + original = b'{"experiment_id":"exp_legacy","runs":[]}' + path.write_bytes(original) + + loaded = store.get_plan("exp_legacy") + + self.assertEqual("0.9", loaded["schema_version"]) + self.assertEqual(original, path.read_bytes()) + + +@unittest.skipIf(web is None, "aiohttp not available") +class TestR238CreationHandlerBoundaries(AioHTTPTestCase): + async def get_application(self): + app = web.Application(client_max_size=6 * MIB) + app.router.add_post("/openclaw/lab/sweep", lab.create_sweep_handler) + app.router.add_post("/openclaw/lab/compare", lab.create_compare_handler) + return app + + async def _post_with_store(self, path, payload, *, store=None): + store = store or MagicMock() + with ( + patch("services.parameter_lab.check_rate_limit", return_value=True), + patch( + "services.parameter_lab.require_admin_token", return_value=(True, None) + ), + patch("services.parameter_lab.get_store", return_value=store), + ): + response = await self.client.post(path, json=payload) + data = await response.json() + return response, data, store + + @unittest_run_loop + async def test_structured_value_is_rejected_before_store_access(self): + response, data, store = await self._post_with_store( + "/openclaw/lab/sweep", + { + "workflow_json": "{}", + "params": [ + { + "node_id": 1, + "widget_name": "video_edit", + "values": [{"trim": [0, 1]}], + } + ], + }, + ) + self.assertEqual(400, response.status) + self.assertEqual("invalid_scalar_value", data["error"]) + store.save_plan.assert_not_called() + + @unittest_run_loop + async def test_oversized_workflow_is_rejected_before_store_access(self): + response, data, store = await self._post_with_store( + "/openclaw/lab/sweep", + { + "workflow_json": "x" * (4 * MIB + 1), + "params": [{"node_id": 1, "widget_name": "seed", "values": [1]}], + }, + ) + self.assertEqual(413, response.status) + self.assertEqual("workflow_too_large", data["error"]) + store.save_plan.assert_not_called() + + @unittest_run_loop + async def test_oversized_request_is_rejected_before_json_or_store_mutation(self): + response, data, store = await self._post_with_store( + "/openclaw/lab/sweep", + { + "workflow_json": "{}", + "params": [ + { + "node_id": 1, + "widget_name": "seed", + "values": ["x" * (5 * MIB)], + } + ], + }, + ) + self.assertEqual(413, response.status) + self.assertEqual("payload_too_large", data["error"]) + store.save_plan.assert_not_called() + + @unittest_run_loop + async def test_request_at_exact_byte_limit_is_accepted(self): + prefix = ( + b'{"workflow_json":"{}",' + b'"params":[{"node_id":1,"widget_name":"seed","values":[1]}],' + b'"padding":"' + ) + suffix = b'"}' + body = prefix + (b"x" * (5 * MIB - len(prefix) - len(suffix))) + suffix + self.assertEqual(5 * MIB, len(body)) + + store = MagicMock() + with ( + patch("services.parameter_lab.check_rate_limit", return_value=True), + patch( + "services.parameter_lab.require_admin_token", return_value=(True, None) + ), + patch("services.parameter_lab.get_store", return_value=store), + ): + response = await self.client.post( + "/openclaw/lab/sweep", + data=io.BytesIO(body), + headers={"Content-Type": "application/json"}, + ) + data = await response.json() + + self.assertEqual(200, response.status) + self.assertTrue(data["ok"]) + store.save_plan.assert_called_once() + + @unittest_run_loop + async def test_store_failure_returns_and_logs_only_content_free_classification( + self, + ): + store = MagicMock() + store.save_plan.side_effect = OSError("secret=private-state-path") + with self.assertLogs(lab.logger.name, level="ERROR") as captured: + response, data, _ = await self._post_with_store( + "/openclaw/lab/sweep", + { + "workflow_json": "{}", + "params": [{"node_id": 1, "widget_name": "seed", "values": [1]}], + }, + store=store, + ) + + self.assertEqual(500, response.status) + self.assertEqual("internal_error", data["error"]) + rendered = "\n".join(captured.output) + self.assertIn("OSError", rendered) + self.assertNotIn("private-state-path", rendered) + self.assertNotIn("secret=", rendered) + + +if __name__ == "__main__": + unittest.main() diff --git a/web/openclaw_graph_host.js b/web/openclaw_graph_host.js index 9418aea..3b8bad9 100644 --- a/web/openclaw_graph_host.js +++ b/web/openclaw_graph_host.js @@ -1,3 +1,5 @@ +import { filterParameterLabCandidates } from "./openclaw_parameter_lab_policy.js"; + const MAX_PROMOTED_WIDGET_DEPTH = 24; const COMPARE_WIDGET_NAMES = new Set(["ckpt_name", "lora_name", "unet_name"]); @@ -289,7 +291,7 @@ export function getGraphWidgetValueCandidates(graph, nodeId, widgetName) { if (!opts.some((candidate) => String(candidate) === String(resolved.widget.value))) { opts.unshift(resolved.widget.value); } - return opts.filter((candidate) => candidate !== undefined); + return filterParameterLabCandidates(opts); } export function findComparableWidget(graph, nodeRefOrId) { diff --git a/web/openclaw_parameter_lab_policy.js b/web/openclaw_parameter_lab_policy.js new file mode 100644 index 0000000..63def37 --- /dev/null +++ b/web/openclaw_parameter_lab_policy.js @@ -0,0 +1,198 @@ +export const PARAMETER_LAB_POLICY = Object.freeze({ + version: "1.0", + maxRequestBytes: 5 * 1024 * 1024, + maxWorkflowUtf8Bytes: 4 * 1024 * 1024, + maxSweepDimensions: 8, + maxValuesPerDimension: 50, + maxNodeIdUtf8Bytes: 128, + maxWidgetNameUtf8Bytes: 256, + maxScalarStringUtf8Bytes: 16 * 1024, + maxPlanUtf8Bytes: 8 * 1024 * 1024, + maxSweepCombinations: 50, + maxCompareItems: 8, +}); + +const encoder = new TextEncoder(); +const valid = () => ({ ok: true, reason: "" }); +const invalid = (reason) => ({ ok: false, reason }); + +function utf8Size(value) { + return encoder.encode(value).byteLength; +} + +function hasControlCharacters(value) { + return Array.from(value).some((character) => { + const code = character.charCodeAt(0); + return code < 32 || code === 127; + }); +} + +function validateNodeId(nodeId) { + if (nodeId === null || nodeId === undefined || nodeId === "") { + return invalid("node_id_required"); + } + if ( + typeof nodeId !== "string" && + !(typeof nodeId === "number" && Number.isInteger(nodeId)) + ) { + return invalid("invalid_node_id"); + } + const normalized = String(nodeId); + if (!normalized.trim()) { + return invalid("node_id_required"); + } + // IMPORTANT: replay keys use the first "." as the node/widget separator. + if (normalized.includes(".") || hasControlCharacters(normalized)) { + return invalid("invalid_node_id"); + } + if (utf8Size(normalized) > PARAMETER_LAB_POLICY.maxNodeIdUtf8Bytes) { + return invalid("node_id_too_large"); + } + return valid(); +} + +function validateWidgetName(widgetName) { + if (typeof widgetName !== "string" || !widgetName.trim()) { + return invalid("widget_name_required"); + } + if (hasControlCharacters(widgetName)) { + return invalid("invalid_widget_name"); + } + if (utf8Size(widgetName) > PARAMETER_LAB_POLICY.maxWidgetNameUtf8Bytes) { + return invalid("widget_name_too_large"); + } + return valid(); +} + +export function isParameterLabScalar(value) { + return ( + typeof value === "string" || + typeof value === "boolean" || + (typeof value === "number" && Number.isFinite(value)) + ); +} + +function validateScalar(value) { + if (!isParameterLabScalar(value)) { + return invalid("invalid_scalar_value"); + } + if ( + typeof value === "string" && + utf8Size(value) > PARAMETER_LAB_POLICY.maxScalarStringUtf8Bytes + ) { + return invalid("scalar_string_too_large"); + } + return valid(); +} + +export function validateParameterLabScalar(value) { + return validateScalar(value); +} + +export function filterParameterLabCandidates(candidates) { + if (!Array.isArray(candidates)) { + return []; + } + const filtered = []; + const presentations = new Set(); + for (const candidate of candidates) { + if (!validateScalar(candidate).ok) { + continue; + } + const presentation = String(candidate); + if (presentations.has(presentation)) { + continue; + } + presentations.add(presentation); + filtered.push(candidate); + } + return filtered; +} + +export function validateParameterLabDimensions(dimensions) { + if (!Array.isArray(dimensions) || dimensions.length === 0) { + return invalid("dimensions_required"); + } + if (dimensions.length > PARAMETER_LAB_POLICY.maxSweepDimensions) { + return invalid("too_many_dimensions"); + } + + const dimensionKeys = new Set(); + let combinations = 1; + for (const dimension of dimensions) { + if (!dimension || typeof dimension !== "object" || Array.isArray(dimension)) { + return invalid("invalid_dimension"); + } + const nodeResult = validateNodeId(dimension.node_id); + if (!nodeResult.ok) { + return nodeResult; + } + const widgetResult = validateWidgetName(dimension.widget_name); + if (!widgetResult.ok) { + return widgetResult; + } + const dimensionKey = `${String(dimension.node_id)}\u0000${dimension.widget_name}`; + if (dimensionKeys.has(dimensionKey)) { + return invalid("duplicate_dimension"); + } + dimensionKeys.add(dimensionKey); + + if (!Array.isArray(dimension.values) || dimension.values.length === 0) { + return invalid("values_required"); + } + if (dimension.values.length > PARAMETER_LAB_POLICY.maxValuesPerDimension) { + return invalid("too_many_values"); + } + const presentations = new Set(); + for (const value of dimension.values) { + const scalarResult = validateScalar(value); + if (!scalarResult.ok) { + return scalarResult; + } + const presentation = String(value); + if (presentations.has(presentation)) { + return invalid("duplicate_ambiguous_value"); + } + presentations.add(presentation); + } + + const strategy = dimension.strategy || "grid"; + if (strategy !== "grid" && strategy !== "compare") { + return invalid("invalid_strategy"); + } + combinations *= dimension.values.length; + if (combinations > PARAMETER_LAB_POLICY.maxSweepCombinations) { + return invalid("sweep_too_large"); + } + } + return valid(); +} + +export function validateParameterLabWorkflow(workflowJson) { + if (typeof workflowJson !== "string" || !workflowJson.trim()) { + return invalid("workflow_required"); + } + if (utf8Size(workflowJson) > PARAMETER_LAB_POLICY.maxWorkflowUtf8Bytes) { + return invalid("workflow_too_large"); + } + return valid(); +} + +export function validateParameterLabRequestBody(payload) { + if (!payload || typeof payload !== "object" || Array.isArray(payload)) { + return invalid("invalid_payload"); + } + let serialized; + try { + serialized = JSON.stringify(payload); + } catch { + return invalid("invalid_payload"); + } + if (typeof serialized !== "string") { + return invalid("invalid_payload"); + } + if (utf8Size(serialized) > PARAMETER_LAB_POLICY.maxRequestBytes) { + return invalid("payload_too_large"); + } + return valid(); +} diff --git a/web/tabs/parameter_lab_tab.js b/web/tabs/parameter_lab_tab.js index 3fe1797..185cb37 100644 --- a/web/tabs/parameter_lab_tab.js +++ b/web/tabs/parameter_lab_tab.js @@ -9,6 +9,14 @@ import { getGraphWidgetValueCandidates, resolveGraphWidget, } from "../openclaw_graph_host.js"; +import { + PARAMETER_LAB_POLICY, + filterParameterLabCandidates, + validateParameterLabDimensions, + validateParameterLabRequestBody, + validateParameterLabScalar, + validateParameterLabWorkflow, +} from "../openclaw_parameter_lab_policy.js"; import { openclawUI } from "../openclaw_ui.js"; /** @@ -235,6 +243,10 @@ export const ParameterLabTab = { }, addDimensionUI(defaults = null) { + if (this.dimensions.length >= PARAMETER_LAB_POLICY.maxSweepDimensions) { + openclawUI.showBanner("error", "Parameter Lab validation failed: too_many_dimensions"); + return false; + } // Add a default blank dimension or use defaults // Allow migration from legacy values_str if needed const newDim = defaults || { @@ -254,6 +266,7 @@ export const ParameterLabTab = { this.dimensions.push(newDim); this.renderDimensions(); + return true; }, removeDimension(index) { @@ -436,6 +449,28 @@ export const ParameterLabTab = { else if (!isNaN(parseFloat(val)) && isFinite(val) && !val.match(/[a-zA-Z]/)) typedVal = parseFloat(val); if (!dim.values) dim.values = []; + const scalarValidation = validateParameterLabScalar(typedVal); + if (!scalarValidation.ok) { + openclawUI.showBanner( + "error", + `Parameter Lab validation failed: ${scalarValidation.reason}` + ); + return; + } + if (dim.values.length >= PARAMETER_LAB_POLICY.maxValuesPerDimension) { + openclawUI.showBanner( + "error", + "Parameter Lab validation failed: too_many_values" + ); + return; + } + if (dim.values.some((existing) => String(existing) === String(typedVal))) { + openclawUI.showBanner( + "error", + "Parameter Lab validation failed: duplicate_ambiguous_value" + ); + return; + } dim.values.push(typedVal); this.renderDimensions(); } @@ -515,7 +550,7 @@ export const ParameterLabTab = { this.dimensions = []; // Add dimension pre-filled - const options = target.widget.options?.values || []; + const options = filterParameterLabCandidates(target.widget.options?.values || []); let initialValues = []; if (options.length > 0) { // Pick top 2 as example @@ -537,17 +572,7 @@ export const ParameterLabTab = { }, async generatePlan() { - // Validate: logic updated to check values array - const validDims = this.dimensions.filter(d => d.node_id && d.widget_name && d.values && d.values.length > 0); - - if (validDims.length === 0) { - openclawUI.showBanner("error", "Please configure at least one valid dimension with values."); - return; - } - - // Prepare Payload - const params = validDims.map(d => { - // Use values directly (already typed from inputs/chips) + const params = this.dimensions.map(d => { return { node_id: d.node_id, widget_name: d.widget_name, @@ -555,6 +580,14 @@ export const ParameterLabTab = { strategy: d.strategy || "grid" }; }); + const dimensionValidation = validateParameterLabDimensions(params); + if (!dimensionValidation.ok) { + openclawUI.showBanner( + "error", + `Parameter Lab validation failed: ${dimensionValidation.reason}` + ); + return; + } const hasCompare = params.some(p => p.strategy === "compare"); if (hasCompare && params.length !== 1) { @@ -564,35 +597,57 @@ export const ParameterLabTab = { ); return; } + if (hasCompare && params[0].values.length > PARAMETER_LAB_POLICY.maxCompareItems) { + openclawUI.showBanner( + "error", + "Parameter Lab validation failed: too_many_values" + ); + return; + } try { - // Serialize current workflow - // Use app.graph.serialize() to get state const graphJson = JSON.stringify(app.graph.serialize()); + const workflowValidation = validateParameterLabWorkflow(graphJson); + if (!workflowValidation.ok) { + openclawUI.showBanner( + "error", + `Parameter Lab validation failed: ${workflowValidation.reason}` + ); + return; + } - let res; + let path; + let payload; if (hasCompare) { const compare = params[0]; openclawUI.showBanner("info", "Generating compare plan..."); - res = await openclawApi.fetch(openclawApi._path("/lab/compare"), { - method: "POST", - body: JSON.stringify({ - workflow_json: graphJson, - items: compare.values, - node_id: compare.node_id, - widget_name: compare.widget_name - }) - }); + path = "/lab/compare"; + payload = { + workflow_json: graphJson, + items: compare.values, + node_id: compare.node_id, + widget_name: compare.widget_name + }; } else { openclawUI.showBanner("info", "Generating sweep plan..."); - res = await openclawApi.fetch(openclawApi._path("/lab/sweep"), { - method: "POST", - body: JSON.stringify({ - workflow_json: graphJson, - params: params - }) - }); + path = "/lab/sweep"; + payload = { + workflow_json: graphJson, + params: params + }; } + const requestValidation = validateParameterLabRequestBody(payload); + if (!requestValidation.ok) { + openclawUI.showBanner( + "error", + `Parameter Lab validation failed: ${requestValidation.reason}` + ); + return; + } + const res = await openclawApi.fetch(openclawApi._path(path), { + method: "POST", + body: JSON.stringify(payload) + }); if (res.ok && res.data) { this.plan = res.data.plan; @@ -602,8 +657,11 @@ export const ParameterLabTab = { } else { openclawUI.showBanner("error", "Failed to generate plan: " + (res.error || "Unknown")); } - } catch (e) { - openclawUI.showBanner("error", "Plan generation error: " + e.message); + } catch { + openclawUI.showBanner( + "error", + "Parameter Lab validation failed: invalid_payload" + ); } }, diff --git a/web/tests/unit/openclaw_graph_host.test.js b/web/tests/unit/openclaw_graph_host.test.js index a0e1bee..dc001bd 100644 --- a/web/tests/unit/openclaw_graph_host.test.js +++ b/web/tests/unit/openclaw_graph_host.test.js @@ -228,4 +228,33 @@ describe("openclaw_graph_host", () => { expect(resolveGraphWidget(graph, "loader-alpha", "palette")?.nodeEntry.id).toBe("loader-alpha"); expect(compareTarget?.nodeId).toBe("loader-alpha"); }); + + it("omits structured and presentation-ambiguous values from Parameter Lab candidates", () => { + const graph = createHostShapedGraphFixture(); + const node = graph.getNodeById("loader-alpha"); + node.widgets.push({ + name: "video_edit", + type: "VIDEO_EDIT", + value: { trim: [0, 1] }, + options: { + values: [ + { trim: [0, 1] }, + ["structured"], + null, + Number.NaN, + 1, + "1", + true, + "true", + "valid", + ], + }, + }); + + expect(getGraphWidgetValueCandidates(graph, "loader-alpha", "video_edit")).toEqual([ + 1, + true, + "valid", + ]); + }); }); diff --git a/web/tests/unit/openclaw_parameter_lab_policy.test.js b/web/tests/unit/openclaw_parameter_lab_policy.test.js new file mode 100644 index 0000000..fec093a --- /dev/null +++ b/web/tests/unit/openclaw_parameter_lab_policy.test.js @@ -0,0 +1,123 @@ +import { describe, expect, it } from "vitest"; +import { + PARAMETER_LAB_POLICY, + filterParameterLabCandidates, + validateParameterLabDimensions, + validateParameterLabRequestBody, + validateParameterLabScalar, + validateParameterLabWorkflow, +} from "../../openclaw_parameter_lab_policy.js"; + +describe("openclaw_parameter_lab_policy", () => { + it("freezes the versioned backend-parity limit contract", () => { + expect(PARAMETER_LAB_POLICY).toEqual({ + version: "1.0", + maxRequestBytes: 5 * 1024 * 1024, + maxWorkflowUtf8Bytes: 4 * 1024 * 1024, + maxSweepDimensions: 8, + maxValuesPerDimension: 50, + maxNodeIdUtf8Bytes: 128, + maxWidgetNameUtf8Bytes: 256, + maxScalarStringUtf8Bytes: 16 * 1024, + maxPlanUtf8Bytes: 8 * 1024 * 1024, + maxSweepCombinations: 50, + maxCompareItems: 8, + }); + expect(Object.isFrozen(PARAMETER_LAB_POLICY)).toBe(true); + }); + + it("filters structured, non-finite, oversized, and presentation-ambiguous candidates", () => { + expect( + filterParameterLabCandidates([ + { rich: true }, + ["structured"], + null, + Number.NaN, + Number.POSITIVE_INFINITY, + 1, + "1", + true, + "true", + "界".repeat(5462), + "valid", + ]) + ).toEqual([1, true, "valid"]); + }); + + it("validates dimensions with stable content-free reasons", () => { + expect( + validateParameterLabDimensions([ + { node_id: "loader-alpha", widget_name: "seed", values: [1, false, "x"] }, + ]) + ).toEqual({ ok: true, reason: "" }); + + const invalidCases = [ + [[], "dimensions_required"], + [ + Array.from({ length: 9 }, (_, index) => ({ + node_id: index, + widget_name: "seed", + values: [index], + })), + "too_many_dimensions", + ], + [ + [{ node_id: "bad.id", widget_name: "seed", values: [1] }], + "invalid_node_id", + ], + [ + [{ node_id: 1, widget_name: "seed", values: [1, "1"] }], + "duplicate_ambiguous_value", + ], + [ + [ + { node_id: 1, widget_name: "seed", values: [1] }, + { node_id: "1", widget_name: "seed", values: [2] }, + ], + "duplicate_dimension", + ], + ]; + for (const [dimensions, reason] of invalidCases) { + expect(validateParameterLabDimensions(dimensions)).toEqual({ ok: false, reason }); + } + }); + + it("validates manual scalar entries before UI state mutation", () => { + expect(validateParameterLabScalar("valid")).toEqual({ ok: true, reason: "" }); + expect(validateParameterLabScalar({ rich: true })).toEqual({ + ok: false, + reason: "invalid_scalar_value", + }); + expect(validateParameterLabScalar("界".repeat(5462))).toEqual({ + ok: false, + reason: "scalar_string_too_large", + }); + }); + + it("measures workflow and request limits in UTF-8 bytes", () => { + expect(validateParameterLabWorkflow("{}")).toEqual({ ok: true, reason: "" }); + expect(validateParameterLabWorkflow("界".repeat(1398102))).toEqual({ + ok: false, + reason: "workflow_too_large", + }); + + expect( + validateParameterLabRequestBody({ + workflow_json: "{}", + params: [{ node_id: 1, widget_name: "seed", values: [1] }], + }) + ).toEqual({ ok: true, reason: "" }); + expect( + validateParameterLabRequestBody({ + workflow_json: "{}", + params: [ + { + node_id: 1, + widget_name: "seed", + values: ["x".repeat(5 * 1024 * 1024)], + }, + ], + }) + ).toEqual({ ok: false, reason: "payload_too_large" }); + }); +});