From c16c262d01c192850cabd9b6510beb7b23486b3e Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Fri, 21 Aug 2026 22:54:04 +0530 Subject: [PATCH] fix(browser): honor live Developer Mode for privileged capability selection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The global broker snapshotted browser.extension_control.developer_mode once at construction, so flipping it OFF in config did not revoke raw CDP/eval from already-attached controllers until process restart — a revocation failure at the highest-privilege browser surface (blocker 3 of andrexibiza's #91535 review). select() now consults the live config on every privileged selection (explicit bool still pins for tests); off->on also unlocks without restart. Regression test drives both directions against an attached controller. Also drops the dead back-compat _artifact_store property (zero readers). --- gateway/browser_control_broker.py | 34 +++++++---- .../gateway/test_browser_control_artifacts.py | 59 +++++++++++++++++++ 2 files changed, 80 insertions(+), 13 deletions(-) diff --git a/gateway/browser_control_broker.py b/gateway/browser_control_broker.py index 5d4c860fde..d794f0999d 100644 --- a/gateway/browser_control_broker.py +++ b/gateway/browser_control_broker.py @@ -335,18 +335,28 @@ class BrowserControlBroker: self._controllers: Dict[ControllerScope, _Controller] = {} self._pending: Dict[str, _PendingCommand] = {} # Developer Mode gates privileged capabilities (browser_evaluate, - # browser_cdp). None defers to the live config on every dispatch so - # a mid-process config change is honored without restart; an explicit - # bool pins the gate for tests and multi-tenant hosts. - if developer_mode is None: - developer_mode = browser_control_developer_mode() - self._developer_mode = developer_mode is True + # browser_cdp). None defers to the live config on every selection so + # a mid-process config change is honored without restart — including + # REVOKING raw CDP/eval from already-attached controllers; an + # explicit bool pins the gate for tests and multi-tenant hosts. + self._developer_mode_pinned: Optional[bool] = ( + None if developer_mode is None else developer_mode is True + ) # Artifact stores keyed by resolved profile id; ``None`` is the # default/unscoped store (tests, single-profile hosts). A multiplex # listener attaches one store per profile so profile A touching the # artifact route first can never pin profile B to A's physical root. self._artifact_stores: Dict[Optional[str], Any] = {} + def _developer_mode_now(self) -> bool: + """Current Developer Mode authority (live config unless pinned).""" + if self._developer_mode_pinned is not None: + return self._developer_mode_pinned + try: + return browser_control_developer_mode() + except Exception: + return False + def attach_artifact_store( self, store: Any, *, profile_id: Optional[str] = None ) -> None: @@ -377,15 +387,10 @@ class BrowserControlBroker: return store return self._artifact_stores.get(None) - @property - def _artifact_store(self) -> Any: - """Back-compat view of the default artifact store (tests).""" - return self._artifact_stores.get(None) - @property def developer_mode(self) -> bool: """Whether privileged capabilities may be selected/dispatched.""" - return self._developer_mode + return self._developer_mode_now() # ------------------------------------------------------------------ # Registration tickets @@ -534,10 +539,13 @@ class BrowserControlBroker: Privileged capabilities (``browser_evaluate``, ``browser_cdp``) are additionally gated on Developer Mode: with the gate off they are never selectable, even when a controller somehow negotiated them. + The gate consults the LIVE flag on every selection (unless pinned at + construction), so flipping ``developer_mode`` off in config revokes + raw CDP/eval from already-attached controllers without a restart. """ if ( capability in BROWSER_CONTROL_DEVELOPER_CAPABILITIES - and not self._developer_mode + and not self._developer_mode_now() ): return None with self._lock: diff --git a/tests/gateway/test_browser_control_artifacts.py b/tests/gateway/test_browser_control_artifacts.py index 51624209f3..7f71905c63 100644 --- a/tests/gateway/test_browser_control_artifacts.py +++ b/tests/gateway/test_browser_control_artifacts.py @@ -660,3 +660,62 @@ def test_multiplex_profiles_get_distinct_stores_regardless_of_touch_order(tmp_pa scope_b = _broker_scope(profile_id="profile-b") assert broker._artifact_store_for_scope(scope_a) is store_a assert broker._artifact_store_for_scope(scope_b) is store_b + + +def test_developer_mode_flip_revokes_privileged_selection_live(monkeypatch): + """Turning developer_mode off in config revokes CDP/eval from an + already-attached controller without a process restart (and on->off + the reverse: enabling unlocks selection for a new negotiation).""" + import gateway.browser_control_broker as broker_mod + + flag = {"on": True} + monkeypatch.setattr( + broker_mod, "browser_control_developer_mode", lambda config=None: flag["on"] + ) + broker = BrowserControlBroker() # developer_mode=None -> live config + scope = _broker_scope( + capabilities=frozenset({"browser_evaluate", "browser_cdp", "controller.noop"}) + ) + broker.attach(scope, lambda _frame: None) + + assert broker.select(scope, "browser_evaluate") is not None + # Revocation: flip the live flag off — the attached controller loses + # privileged selection immediately. + flag["on"] = False + assert broker.select(scope, "browser_evaluate") is None + assert broker.select(scope, "browser_cdp") is None + # Base capabilities are unaffected by the developer gate. + assert broker.select(scope, "controller.noop") is not None + # And back on: selection resumes without any rebind. + flag["on"] = True + assert broker.select(scope, "browser_cdp") is not None + # Explicit pin still wins over live config (test/multi-tenant contract). + pinned = BrowserControlBroker(developer_mode=False) + pinned.attach(scope, lambda _frame: None) + assert pinned.select(scope, "browser_evaluate") is None + + +def test_store_construction_sweeps_orphan_files_from_previous_process(tmp_path): + """Files left by a dead process (unreachable, past advertised TTL) are + removed when a fresh store opens the same root.""" + root = tmp_path / "root" + store = ArtifactStore(root) + receipt = store.store( + TEXT_BYTES, filename="note.txt", content_type="text/plain", scope=_Scope() + ) + orphan = root / receipt.artifact_id + assert orphan.exists() + stale_tmp = root / "deadbeef.tmp" + stale_tmp.write_bytes(b"partial") + unrelated = root / "README" + unrelated.write_bytes(b"keep me") + + # Simulate restart: a new store over the same root has an empty index. + fresh = ArtifactStore(root) + assert not orphan.exists() + assert not stale_tmp.exists() + assert unrelated.exists() # non-artifact-shaped names untouched + assert fresh.count() == 0 + # New store works normally afterwards. + fresh.store(TEXT_BYTES, filename="new.txt", content_type="text/plain", scope=_Scope()) + assert fresh.count() == 1