fix(browser): honor live Developer Mode for privileged capability selection
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).
This commit is contained in:
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user