diff --git a/openadapt_flow/backends/playwright_backend.py b/openadapt_flow/backends/playwright_backend.py index 478cc1fd..cc1fc0cc 100644 --- a/openadapt_flow/backends/playwright_backend.py +++ b/openadapt_flow/backends/playwright_backend.py @@ -12,6 +12,7 @@ import hmac import math import re +import time import uuid from dataclasses import dataclass, field from datetime import datetime, timezone @@ -33,7 +34,11 @@ ) VIEWPORT: tuple[int, int] = (1280, 800) -_MASKED_SCREENSHOT_ATTEMPTS = 3 +# Secret-masked screenshots must not keep bytes from a changing frame tree. +# Login pages often attach and detach iframes for a few seconds after a click. +# Retry until the tree is quiet. Still refuse if it never settles. +_MASKED_SCREENSHOT_TIMEOUT_S = 8.0 +_MASKED_SCREENSHOT_RETRY_SLEEP_S = 0.05 _MODIFIER_ALIASES = { "meta": "Meta", @@ -2300,10 +2305,14 @@ def screenshot(self) -> bytes: if not self._screenshot_mask_selectors: return self.page.screenshot(type="png", full_page=False, **base_options) - for _attempt in range(_MASKED_SCREENSHOT_ATTEMPTS): + deadline = time.monotonic() + _MASKED_SCREENSHOT_TIMEOUT_S + while True: generation = self._screenshot_frame_generation frames = tuple(self.page.frames) if generation != self._screenshot_frame_generation: + if time.monotonic() >= deadline: + break + time.sleep(_MASKED_SCREENSHOT_RETRY_SLEEP_S) continue options = dict(base_options) options["mask"] = [ @@ -2319,6 +2328,9 @@ def screenshot(self) -> bytes: self.page.evaluate("() => null") except Exception: if generation != self._screenshot_frame_generation: + if time.monotonic() >= deadline: + break + time.sleep(_MASKED_SCREENSHOT_RETRY_SLEEP_S) continue raise current_frames = tuple(self.page.frames) @@ -2328,6 +2340,9 @@ def screenshot(self) -> bytes: return png # ``png`` is intentionally discarded here. It never reaches the # recorder, disk, or a compiled bundle. + if time.monotonic() >= deadline: + break + time.sleep(_MASKED_SCREENSHOT_RETRY_SLEEP_S) raise ScreenshotMaskStabilityError( "the browser frame tree changed during every secret-masked " "screenshot attempt; recording was refused" diff --git a/tests/test_browser_attach.py b/tests/test_browser_attach.py index d23199b5..1c31a551 100644 --- a/tests/test_browser_attach.py +++ b/tests/test_browser_attach.py @@ -268,6 +268,122 @@ def screenshot(self, **kwargs): assert not any(page.listeners.values()) +def test_masked_screenshot_survives_three_iframe_storms_then_settles( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Reproduce the ServiceNow Sign-in refuse with a fake page. + + After Sign in, a login SPA attaches and detaches iframes. The old loop + tried three captures with no wait and then raised + ScreenshotMaskStabilityError. This page storms for those three captures + and then goes quiet. The record must keep a masked frame, not stop. + """ + + class Frame: + def __init__(self, name: str) -> None: + self.name = name + + def locator(self, selector): + return f"locator:{self.name}:{selector}" + + class Page: + def __init__(self) -> None: + self.frames = [Frame("main")] + self.listeners: dict[str, list] = {} + self.capture_count = 0 + + def on(self, event, listener): + self.listeners.setdefault(event, []).append(listener) + + def remove_listener(self, event, listener): + self.listeners[event].remove(listener) + + def evaluate(self, _script): + return None + + def screenshot(self, **kwargs): + self.capture_count += 1 + if self.capture_count <= 3: + frame = Frame(f"storm-{self.capture_count}") + self.frames.append(frame) + for listener in self.listeners.get("frameattached", []): + listener(frame) + self.frames.pop() + for listener in self.listeners.get("framedetached", []): + listener(frame) + return b"png" + + monkeypatch.setattr( + "openadapt_flow.backends.playwright_backend._MASKED_SCREENSHOT_RETRY_SLEEP_S", + 0.0, + ) + page = Page() + backend = PlaywrightBackend( # type: ignore[arg-type] + page, + screenshot_mask_selectors=("input[type='password']",), + ) + assert backend.screenshot() == b"png" + assert page.capture_count >= 4 + backend.stop_screenshot_mask_tracking() + + +def test_masked_screenshot_still_refuses_a_frame_tree_that_never_settles( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A tree that never goes quiet must still refuse. Do not keep those bytes.""" + + class Frame: + def __init__(self, name: str) -> None: + self.name = name + + def locator(self, selector): + return f"locator:{self.name}:{selector}" + + class Page: + def __init__(self) -> None: + self.frames = [Frame("main")] + self.listeners: dict[str, list] = {} + self.capture_count = 0 + + def on(self, event, listener): + self.listeners.setdefault(event, []).append(listener) + + def remove_listener(self, event, listener): + self.listeners[event].remove(listener) + + def evaluate(self, _script): + return None + + def screenshot(self, **kwargs): + self.capture_count += 1 + frame = Frame(f"churn-{self.capture_count}") + self.frames.append(frame) + for listener in self.listeners.get("frameattached", []): + listener(frame) + self.frames.pop() + for listener in self.listeners.get("framedetached", []): + listener(frame) + return b"png" + + monkeypatch.setattr( + "openadapt_flow.backends.playwright_backend._MASKED_SCREENSHOT_TIMEOUT_S", + 0.0, + ) + monkeypatch.setattr( + "openadapt_flow.backends.playwright_backend._MASKED_SCREENSHOT_RETRY_SLEEP_S", + 0.0, + ) + page = Page() + backend = PlaywrightBackend( # type: ignore[arg-type] + page, + screenshot_mask_selectors=("input[type='password']",), + ) + with pytest.raises(ScreenshotMaskStabilityError, match="frame tree"): + backend.screenshot() + assert page.capture_count >= 1 + backend.stop_screenshot_mask_tracking() + + def test_declared_secret_selectors_use_css_string_escaping() -> None: selectors = _secret_screenshot_selectors({"päss", 'quote"\\line\nend'}) @@ -2284,9 +2400,45 @@ def attach_and_detach_during_every_capture(**kwargs): "screenshot", attach_and_detach_during_every_capture, ) + patch_context.setattr( + "openadapt_flow.backends.playwright_backend._MASKED_SCREENSHOT_TIMEOUT_S", + 0.0, + ) + patch_context.setattr( + "openadapt_flow.backends.playwright_backend._MASKED_SCREENSHOT_RETRY_SLEEP_S", + 0.0, + ) with pytest.raises(ScreenshotMaskStabilityError, match="frame tree"): race_backend.screenshot() - assert churn_attempts == 3 + assert churn_attempts >= 1 + + settle_attempts = 0 + + def churn_twice_then_keep(**kwargs): + nonlocal settle_attempts + settle_attempts += 1 + if settle_attempts <= 2: + race_page.evaluate( + """attempt => { + const frame = document.createElement('iframe'); + frame.id = `settle-${attempt}`; + frame.srcdoc = ''; + document.body.appendChild(frame); + frame.remove(); + }""", + settle_attempts, + ) + return original_screenshot(**kwargs) + + with monkeypatch.context() as patch_context: + patch_context.setattr( + race_page, + "screenshot", + churn_twice_then_keep, + ) + png = race_backend.screenshot() + assert png + assert settle_attempts >= 3 finally: if frame_race_session.page is not None: frame_race_session.page.evaluate(