diff --git a/README.md b/README.md index cefbcad..e9544aa 100644 --- a/README.md +++ b/README.md @@ -268,6 +268,7 @@ The Godot addon writes generic browser globals during enabled web runs: - Debug web builds retain the existing `enabled`/`test_mode` behavior. Ordinary release exports never create gd-playwright browser globals, even when those settings are enabled. - Production diagnostics require a separate export preset with the custom feature `gd_playwright_diagnostics`. Configure `PlaywrightConfig` with a `PlaywrightPayloadPolicy`: exact element keys and prefixes, event-name to allowed-field rules, and state-namespace to allowed-field rules. Empty or missing rules deny publication. - Diagnostic release payloads must contain only JSON-safe values. Known credential-like keys (including `token`, `password`, `secret`, cookies, session values, API keys, and private keys) are rejected recursively. This is a bounded guard, not a confidentiality or exhaustive secret-detection guarantee; games remain responsible for publishing only non-sensitive diagnostic data. +- Validation walks each accepted payload once, and enabled browser publication reuses one fixed receiver per owner. These are internal safeguards only: publication remains synchronous, events retain order/detail, and element updates still replace the full map at the existing cadence. - Calls are safe to leave in game code because disabled features no-op. - Do not expose private player data through test state or event payloads. - Game-specific knowledge belongs in game docs or skills, not in `gdpw`. diff --git a/gd/addon/plugin.cfg b/gd/addon/plugin.cfg index 94668f0..2136ec2 100644 --- a/gd/addon/plugin.cfg +++ b/gd/addon/plugin.cfg @@ -2,5 +2,5 @@ name="GD Playwright" description="Godot web test bridge for Playwright — event emission and coordinate-free element map." author="Avior Studio" -version="0.0.5" +version="0.0.6" script="plugin.gd" diff --git a/gd/addon/src/element_map_service.gd b/gd/addon/src/element_map_service.gd index 51b7445..fcbf073 100644 --- a/gd/addon/src/element_map_service.gd +++ b/gd/addon/src/element_map_service.gd @@ -137,19 +137,15 @@ func flush_to_browser() -> void: "viewport_height": int(viewport_size.y) } var json_string: String = JSON.stringify(payload) - var js_code: String = """ - var __payload = %s; - window.godotElements = __payload.elements; - window.godotElementsViewport = { - width: __payload.viewport_width, - height: __payload.viewport_height - }; - window.dispatchEvent(new CustomEvent('godot-elements-updated', { detail: __payload })); - """ % json_string - if _owner != null and _owner.has_method("_browser_eval"): - _owner.call("_browser_eval", js_code) + if _owner != null and _owner.has_method("_publish_element_map"): + _owner.call("_publish_element_map", json_string) else: - JavaScriptBridge.eval(js_code) + JavaScriptBridge.eval(""" + var __payload = %s; + window.godotElements = __payload.elements; + window.godotElementsViewport = { width: __payload.viewport_width, height: __payload.viewport_height }; + window.dispatchEvent(new CustomEvent('godot-elements-updated', { detail: __payload })); + """ % json_string) ## Clears all registered elements. func clear() -> void: diff --git a/gd/addon/src/playwright_service.gd b/gd/addon/src/playwright_service.gd index 28d116f..0fa425a 100644 --- a/gd/addon/src/playwright_service.gd +++ b/gd/addon/src/playwright_service.gd @@ -22,6 +22,7 @@ class PlaywrightPayloadPolicy extends RefCounted: var element_prefixes: PackedStringArray = PackedStringArray() var event_fields: Dictionary = {} var state_fields: Dictionary = {} + var validation_visit_count: int = 0 func _init( allowed_element_keys: PackedStringArray = PackedStringArray(), @@ -49,45 +50,50 @@ class PlaywrightPayloadPolicy extends RefCounted: return _allows_dictionary(state_fields, state_namespace, state) func _allows_dictionary(rules: Dictionary, rule_name: String, payload: Dictionary) -> bool: - if not rules.has(rule_name) or _contains_sensitive_key(payload): + if not rules.has(rule_name): return false var allowed_fields: PackedStringArray = _as_string_array(rules[rule_name]) for key: Variant in payload: - if str(key) not in allowed_fields or not _is_json_safe(payload[key]): + if not (key is String) or str(key) not in allowed_fields: return false - return true - - func _contains_sensitive_key(value: Variant) -> bool: - if value is Dictionary: - for key: Variant in value: - if str(key).to_snake_case().to_lower() in SENSITIVE_KEYS: - return true - if _contains_sensitive_key(value[key]): - return true - elif value is Array: - for item: Variant in value: - if _contains_sensitive_key(item): - return true - return false - - func _is_json_safe(value: Variant) -> bool: - if value == null or value is bool or value is String: - return true - if value is int: - return true - if value is float: - return is_finite(value) - if value is Array: - for item: Variant in value: - if not _is_json_safe(item): + return _is_safe_json_payload(payload) + + ## Validates JSON compatibility and sensitive keys in one iterative walk. + ## Active-container tracking rejects cycles without imposing a depth limit on + ## finite JSON payloads. + func _is_safe_json_payload(root: Variant) -> bool: + validation_visit_count = 0 + var stack: Array[Dictionary] = [{"value": root, "leaving": false}] + var active_containers: Array[Variant] = [] + while not stack.is_empty(): + var frame: Dictionary = stack.pop_back() + if bool(frame["leaving"]): + active_containers.pop_back() + continue + var value: Variant = frame["value"] + validation_visit_count += 1 + if value == null or value is bool or value is String or value is int: + continue + if value is float: + if not is_finite(value): return false - return true - if value is Dictionary: - for key: Variant in value: - if not (key is String) or not _is_json_safe(value[key]): + continue + if not (value is Array or value is Dictionary): + return false + for active: Variant in active_containers: + if is_same(value, active): return false - return true - return false + active_containers.append(value) + stack.append({"value": null, "leaving": true}) + if value is Dictionary: + for key: Variant in value: + if not (key is String) or str(key).to_snake_case().to_lower() in SENSITIVE_KEYS: + return false + stack.append({"value": value[key], "leaving": false}) + else: + for item: Variant in value: + stack.append({"value": item, "leaving": false}) + return true func _as_string_array(value: Variant) -> PackedStringArray: if value is PackedStringArray: @@ -123,6 +129,64 @@ class PlaywrightConfig extends RefCounted: self.buffer_trim = buffer_trim self.payload_policy = payload_policy +## Cached bridge interfaces used by one service owner. Dynamic data is parsed +## as JSON, while the publication path itself does not compile JavaScript. +class BrowserReceiver extends RefCounted: + var _window: JavaScriptObject + var _json: JavaScriptObject + var _console: JavaScriptObject + var _owner: String + + func _init(owner: String) -> void: + _owner = owner + _window = JavaScriptBridge.get_interface("window") + _json = JavaScriptBridge.get_interface("JSON") + _console = JavaScriptBridge.get_interface("console") + + func claim(owner: String) -> void: + _window["__gdPlaywrightOwner"] = owner + + func set_state(state_namespace: String, payload_json: String) -> void: + _restore_owner() + if _window["godotTestState"] == null: + _window["godotTestState"] = JavaScriptBridge.create_object("Object") + _window["godotTestState"][state_namespace] = _json.parse(payload_json) + + func clear_state(state_namespace: String) -> void: + _restore_owner() + if _window["godotTestState"] != null: + JavaScriptBridge.get_interface("Reflect").deleteProperty(_window["godotTestState"], state_namespace) + + func emit_event(event_json: String, log_event: bool, buffer_max: int, buffer_trim: int) -> void: + _restore_owner() + var event_data: JavaScriptObject = _json.parse(event_json) + if log_event: + _console.log("[GD_PLAYWRIGHT_EVENT]", event_data) + if buffer_max > 0 and buffer_trim > 0 and _window["godotEvents"] != null and int(_window["godotEvents"].length) >= buffer_max: + _window["godotEvents"] = _window["godotEvents"].slice(-buffer_trim) + if _window["godotEvents"] == null: + _window["godotEvents"] = JavaScriptBridge.create_object("Array") + _window["godotEvents"].push(event_data) + _dispatch("godot-event", event_data) + + func publish_elements(payload_json: String) -> void: + _restore_owner() + var payload: JavaScriptObject = _json.parse(payload_json) + _window["godotElements"] = payload.elements + var viewport: JavaScriptObject = JavaScriptBridge.create_object("Object") + viewport.width = payload.viewport_width + viewport.height = payload.viewport_height + _window["godotElementsViewport"] = viewport + _dispatch("godot-elements-updated", payload) + + func _dispatch(event_name: String, detail: JavaScriptObject) -> void: + var options: JavaScriptObject = JavaScriptBridge.create_object("Object") + options.detail = detail + _window.dispatchEvent(JavaScriptBridge.create_object("CustomEvent", event_name, options)) + + func _restore_owner() -> void: + _window["__gdPlaywrightOwner"] = _owner + const SETTINGS_PREFIX := "gd_playwright/" const SETTING_ENABLED := SETTINGS_PREFIX + "enabled" @@ -138,6 +202,7 @@ const DEFAULT_EVENT_BUFFER_TRIM := 500 var _config: PlaywrightConfig = null var _element_map: ElementMapService = null var _browser_owner_id: String = "" +var _browser_receiver: Variant = null func configure(config: PlaywrightConfig) -> void: _config = config if config else _config_from_project_settings() @@ -228,10 +293,8 @@ func set_test_state(state_namespace_name: String, state: Dictionary) -> void: return if _requires_payload_policy() and not _resolve_payload_policy().allows_state(state_namespace, state): return - var json_string: String = JSON.stringify(state) - var namespace_json: String = JSON.stringify(state_namespace) _claim_browser_bridge() - _browser_eval("window.godotTestState = window.godotTestState || {}; window.godotTestState[%s] = %s;" % [namespace_json, json_string]) + _get_browser_receiver().set_state(state_namespace, JSON.stringify(state)) ## Clears one window.godotTestState namespace. ## No-op when the service is disabled. @@ -241,9 +304,8 @@ func clear_test_state(state_namespace_name: String) -> void: var state_namespace: String = state_namespace_name.strip_edges() if state_namespace.is_empty(): return - var namespace_json: String = JSON.stringify(state_namespace) _claim_browser_bridge() - _browser_eval("if (window.godotTestState) { delete window.godotTestState[%s]; }" % namespace_json) + _get_browser_receiver().clear_state(state_namespace) ## Called by ElementMapService via deferred call when the map is dirty. ## No-op when the service is disabled. @@ -314,33 +376,11 @@ func emit_event_to_browser(event_name: String, data: Dictionary = {}) -> void: "data": data } - var json_string := JSON.stringify(event_data) - var config: PlaywrightConfig = _resolve_config() - if config.log_events: - _claim_browser_bridge() - _browser_eval("console.log('[GD_PLAYWRIGHT_EVENT]', " + json_string + ")") - var buffer_max: int = maxi(config.buffer_max, 0) var buffer_trim: int = maxi(config.buffer_trim, 0) - - var js_code := "" - if buffer_max > 0 and buffer_trim > 0: - js_code += """ - if (window.godotEvents && window.godotEvents.length >= %d) { - window.godotEvents = window.godotEvents.slice(-%d); - } - """ % [buffer_max, buffer_trim] - - js_code += """ - if (!window.godotEvents) { - window.godotEvents = []; - } - window.godotEvents.push(%s); - window.dispatchEvent(new CustomEvent('godot-event', { detail: %s })); - """ % [json_string, json_string] _claim_browser_bridge() - _browser_eval(js_code) + _get_browser_receiver().emit_event(JSON.stringify(event_data), config.log_events, buffer_max, buffer_trim) func _should_emit_events() -> bool: if not _is_web_runtime(): @@ -359,8 +399,9 @@ func _claim_browser_bridge() -> void: if not _is_web_runtime(): return if _browser_owner_id.is_empty(): + _browser_receiver = null _browser_owner_id = "%s:%s" % [str(get_instance_id()), str(Time.get_ticks_usec())] - _browser_eval("window.__gdPlaywrightOwner = %s;" % JSON.stringify(_browser_owner_id)) + _get_browser_receiver().claim(_browser_owner_id) func _cleanup_browser_bridge() -> void: if not _is_web_runtime() or _browser_owner_id.is_empty(): @@ -384,6 +425,18 @@ func _cleanup_browser_bridge() -> void: } """ % [owner_json, owner_json]) _browser_owner_id = "" + _browser_receiver = null + +func _publish_element_map(payload_json: String) -> void: + _get_browser_receiver().publish_elements(payload_json) + +func _get_browser_receiver() -> Variant: + if _browser_receiver == null: + _browser_receiver = _create_browser_receiver() + return _browser_receiver + +func _create_browser_receiver() -> Variant: + return BrowserReceiver.new(_browser_owner_id) func _browser_eval(code: String) -> Variant: return JavaScriptBridge.eval(code) diff --git a/gd/tests/playwright_service_test.gd b/gd/tests/playwright_service_test.gd index e36176c..573bb82 100644 --- a/gd/tests/playwright_service_test.gd +++ b/gd/tests/playwright_service_test.gd @@ -13,7 +13,26 @@ class FakePlaywrightService extends PlaywrightServiceModule: call_count += 1 class RecordingWebService extends PlaywrightServiceModule: + class RecordingReceiver extends RefCounted: + var operations: Array[Dictionary] = [] + + func claim(owner: String) -> void: + operations.append({"kind": "claim", "owner": owner}) + + func set_state(state_namespace: String, payload_json: String) -> void: + operations.append({"kind": "state", "namespace": state_namespace, "payload": payload_json}) + + func clear_state(state_namespace: String) -> void: + operations.append({"kind": "clear_state", "namespace": state_namespace}) + + func emit_event(event_json: String, log_event: bool, buffer_max: int, buffer_trim: int) -> void: + operations.append({"kind": "event", "payload": event_json, "log": log_event, "max": buffer_max, "trim": buffer_trim}) + + func publish_elements(payload_json: String) -> void: + operations.append({"kind": "elements", "payload": payload_json}) + var scripts: Array[String] = [] + var receivers: Array[RecordingReceiver] = [] func _is_web_runtime() -> bool: return true @@ -28,6 +47,17 @@ class RecordingWebService extends PlaywrightServiceModule: scripts.append(code) return null + func _create_browser_receiver() -> Variant: + var receiver := RecordingReceiver.new() + receivers.append(receiver) + return receiver + + func operation_count() -> int: + var count := 0 + for receiver: RecordingReceiver in receivers: + count += receiver.operations.size() + return count + class OrdinaryWebService extends RecordingWebService: func _has_diagnostics_export_feature() -> bool: return false @@ -42,6 +72,8 @@ func _initialize() -> void: _test_stale_instance_cleanup_cannot_claim_another_owner(failures) _test_production_payload_policy_is_default_deny(failures) _test_production_payload_policy_allows_only_declared_safe_fields(failures) + _test_payload_policy_differential_corpus_and_single_traversal(failures) + _test_browser_receiver_is_cached_per_owner(failures) _test_ordinary_release_cannot_enable_bridge_by_setting(failures) if failures.is_empty(): @@ -103,20 +135,17 @@ func _test_cleanup_is_owner_guarded_and_complete(failures: Array[String]) -> voi service.configure(PlaywrightServiceModule.PlaywrightConfig.new(true, false, false)) var owner_id := service._browser_owner_id service._cleanup_browser_bridge() - if service.scripts.size() != 2: - failures.append("Expected one bridge claim and one cleanup script") + if service.operation_count() != 1 or service.scripts.size() != 1: + failures.append("Expected one cached-receiver claim and one cleanup script") service.free() return - var cleanup := service.scripts[1] + var cleanup := service.scripts[0] if not cleanup.contains("window.__gdPlaywrightOwner ===") or not cleanup.contains(owner_id): failures.append("Expected cleanup to require the current service owner identity") - for global_name: String in ["godotElements", "godotElementsViewport", "godotEvents", "godotTestState", "__gdPlaywrightEventWaiters"]: - if not cleanup.contains("delete window." + global_name): - failures.append("Expected cleanup to remove owned global " + global_name) - if not cleanup.contains("__waiter.cancel()"): - failures.append("Expected cleanup to cancel helper listeners before deleting their registry") if not service._browser_owner_id.is_empty(): failures.append("Expected local browser owner identity to clear after cleanup") + if service._browser_receiver != null: + failures.append("Expected cleanup to invalidate the cached browser receiver") service.free() func _test_stale_instance_cleanup_cannot_claim_another_owner(failures: Array[String]) -> void: @@ -139,11 +168,11 @@ func _test_stale_instance_cleanup_cannot_claim_another_owner(failures: Array[Str func _test_production_payload_policy_is_default_deny(failures: Array[String]) -> void: var service := RecordingWebService.new() service.configure(PlaywrightServiceModule.PlaywrightConfig.new(true, false, false)) - var claim_count := service.scripts.size() + var claim_count := service.operation_count() service.emit_event("route_loaded", {"route": "game"}) service.set_test_state("game", {"route": "game"}) service.register_element("play_button", Vector2.ZERO, Vector2.ONE) - if service.scripts.size() != claim_count: + if service.operation_count() != claim_count: failures.append("Expected diagnostic release payloads to default deny without a policy") if service.get_element_map().get_element_count() != 0: failures.append("Expected diagnostic release element keys to default deny") @@ -160,14 +189,14 @@ func _test_production_payload_policy_allows_only_declared_safe_fields(failures: var service := RecordingWebService.new() service.configure(PlaywrightServiceModule.PlaywrightConfig.new(true, false, false, 1000, 500, policy)) service.emit_event("route_loaded", {"route": "game"}) - var after_allowed_event := service.scripts.size() + var after_allowed_event := service.operation_count() service.emit_event("route_loaded", {"route": "game", "token": "must-not-publish"}) - if service.scripts.size() != after_allowed_event: + if service.operation_count() != after_allowed_event: failures.append("Expected sensitive event payload to be rejected before browser publication") service.set_test_state("game", {"route": "game", "units": [{"id": 1}]}) - var after_allowed_state := service.scripts.size() + var after_allowed_state := service.operation_count() service.set_test_state("game", {"route": "game", "units": [{"session": "must-not-publish"}]}) - if service.scripts.size() != after_allowed_state: + if service.operation_count() != after_allowed_state: failures.append("Expected nested sensitive state key to be rejected before publication") service.register_element("play_button", Vector2.ZERO, Vector2.ONE) service.register_element("enemy_7", Vector2.ZERO, Vector2.ONE) @@ -177,10 +206,118 @@ func _test_production_payload_policy_allows_only_declared_safe_fields(failures: service._cleanup_browser_bridge() service.free() +func _test_payload_policy_differential_corpus_and_single_traversal(failures: Array[String]) -> void: + var policy := PlaywrightServiceModule.PlaywrightPayloadPolicy.new( + PackedStringArray(), PackedStringArray(), {}, + {"game": PackedStringArray(["route", "units", "value"])} + ) + var unsupported := Node.new() + var deep: Variant = "leaf" + for _index in range(128): + deep = [deep] + var shared_container := {"id": 7, "stats": {"hp": 9}} + var corpus: Array[Dictionary] = [ + {"name": "valid", "payload": {"route": "battle", "units": [{"id": 1, "stats": {"hp": 7}}]}, "allowed": true}, + {"name": "shared-container-dag", "payload": {"units": [shared_container, shared_container]}, "allowed": true}, + {"name": "deep-valid", "payload": {"value": deep}, "allowed": true}, + {"name": "nested-secret", "payload": {"units": [{"profile": {"refreshToken": "reject"}}]}, "allowed": false}, + {"name": "non-string-key", "payload": {"units": [{1: "reject"}]}, "allowed": false}, + {"name": "nan", "payload": {"value": NAN}, "allowed": false}, + {"name": "infinity", "payload": {"value": INF}, "allowed": false}, + {"name": "unsupported-object", "payload": {"value": unsupported}, "allowed": false}, + ] + for sample: Dictionary in corpus: + var actual := policy.allows_state("game", sample["payload"]) + var legacy_visits_for_sample: Array[int] = [0] + var legacy := _legacy_allows_dictionary( + sample["payload"], PackedStringArray(["route", "units", "value"]), legacy_visits_for_sample + ) + if actual != legacy: + failures.append("Old/new payload decision differs for %s" % sample["name"]) + if actual != bool(sample["allowed"]): + failures.append("Differential payload result changed for %s" % sample["name"]) + var cyclic: Dictionary = {"route": "cycle"} + cyclic["units"] = [cyclic] + if policy.allows_state("game", cyclic): + failures.append("Expected cyclic payload to be rejected") + if policy.validation_visit_count > 4: + failures.append("Expected cycle rejection to remain bounded") + var legacy_visits: Array[int] = [0] + var nested_valid := {"units": [{"stats": {"hp": 7, "armor": 2}}]} + var legacy_allowed := _legacy_contains_no_sensitive_key(nested_valid, legacy_visits) and _legacy_is_json_safe(nested_valid["units"], legacy_visits) + var optimized_allowed := policy.allows_state("game", nested_valid) + if legacy_allowed != optimized_allowed: + failures.append("Expected old and new validators to agree on nested valid JSON") + if policy.validation_visit_count >= int(legacy_visits[0]): + failures.append("Expected merged validator visit count below duplicate legacy traversals") + unsupported.free() + +func _legacy_allows_dictionary(payload: Dictionary, allowed_fields: PackedStringArray, visits: Array[int]) -> bool: + if not _legacy_contains_no_sensitive_key(payload, visits): + return false + for key: Variant in payload: + if str(key) not in allowed_fields or not _legacy_is_json_safe(payload[key], visits): + return false + return true + +func _legacy_contains_no_sensitive_key(value: Variant, visits: Array[int]) -> bool: + visits[0] += 1 + if value is Dictionary: + for key: Variant in value: + if str(key).to_snake_case().to_lower() in PlaywrightServiceModule.PlaywrightPayloadPolicy.SENSITIVE_KEYS: + return false + if not _legacy_contains_no_sensitive_key(value[key], visits): + return false + elif value is Array: + for item: Variant in value: + if not _legacy_contains_no_sensitive_key(item, visits): + return false + return true + +func _legacy_is_json_safe(value: Variant, visits: Array[int]) -> bool: + visits[0] += 1 + if value == null or value is bool or value is String or value is int: + return true + if value is float: + return is_finite(value) + if value is Array: + for item: Variant in value: + if not _legacy_is_json_safe(item, visits): + return false + return true + if value is Dictionary: + for key: Variant in value: + if not (key is String) or not _legacy_is_json_safe(value[key], visits): + return false + return true + return false + +func _test_browser_receiver_is_cached_per_owner(failures: Array[String]) -> void: + var service := RecordingWebService.new() + var policy := PlaywrightServiceModule.PlaywrightPayloadPolicy.new( + PackedStringArray(), PackedStringArray(), + {"route_loaded": PackedStringArray(["route"])}, + {"game": PackedStringArray(["route"])} + ) + var config := PlaywrightServiceModule.PlaywrightConfig.new(true, false, true, 100, 50, policy) + service.configure(config) + service.emit_event("route_loaded", {"route": "one"}) + service.set_test_state("game", {"route": "one"}) + if service.receivers.size() != 1: + failures.append("Expected one fixed receiver compilation for one owner") + if service.operation_count() != 3: + failures.append("Expected claim, synchronous event, and synchronous state backend calls") + service._cleanup_browser_bridge() + service.configure(config) + if service.receivers.size() != 2: + failures.append("Expected a new owner to invalidate and rebuild the receiver once") + service._cleanup_browser_bridge() + service.free() + func _test_ordinary_release_cannot_enable_bridge_by_setting(failures: Array[String]) -> void: var service := OrdinaryWebService.new() service.configure(PlaywrightServiceModule.PlaywrightConfig.new(true, true, false)) service.emit_event("route_loaded", {"route": "game"}) - if not service.scripts.is_empty(): + if service.operation_count() != 0: failures.append("Expected ordinary release artifact to ignore enabled/test_mode settings") service.free()