|
1 | 1 | """BrowserActChannel — the generic browser-act pack manifest interpreter |
2 | | -(GOAL-7 PR-C; see GOAL-7.md architecture decisions #1, #4, #5, #6, #8, #10). |
| 2 | +(GOAL-7 PR-C/PR-E; see GOAL-7.md architecture decisions #1, #4, #5, #6, #7, |
| 3 | +#8, #10). |
3 | 4 |
|
4 | 5 | Drives a vendored pack (backend/browser_act_packs/<category>/<pack>/, |
5 | 6 | PR-A) through its channel.manifest.json (PR-A schema, backend/ |
|
29 | 30 | (``"result_count < N"`` / ``"<="``). Anything else falls back to a fixed |
30 | 31 | ``max_pages`` cap and an "a page returned 0 items" stop signal -- documented |
31 | 32 | limitation, not silently pretended to be complete (see ``_stop_when_triggered``). |
| 33 | +
|
| 34 | +Credentials (PR-E, decision #7): ``mode == "stealth"`` requires a BrowserAct |
| 35 | +API key, stored encrypted as a ``SourceCredential`` (key_name |
| 36 | +``CREDENTIAL_KEY_NAME``) and resolved via the existing ``AuthManager`` |
| 37 | +(same encrypted store every other channel's credentials go through -- |
| 38 | +``_resolve_session_env``). It is injected into the browser-act CLI |
| 39 | +subprocess env (never argv, never logged) under ``BROWSER_ACT_API_KEY_ENV``. |
| 40 | +``collect()`` takes an additive optional ``source_id`` param for this; only |
| 41 | +``fetch()`` (the thick-contract entry point ``run_channel`` calls) ever |
| 42 | +passes a real one in production -- see both methods' docstrings. |
32 | 43 | """ |
33 | 44 |
|
34 | 45 | import json |
|
43 | 54 | from backend.browser_act.scripts import ScriptError, run_pack_script |
44 | 55 | from backend.browser_act_packs.catalog import PackCatalog, PackInfo |
45 | 56 | from backend.browser_act_packs.manifest import PackManifest, load_manifest |
46 | | -from backend.channels.base import AbstractChannel, Capabilities, ChannelResult |
| 57 | +from backend.channels.base import ( |
| 58 | + AbstractChannel, |
| 59 | + Capabilities, |
| 60 | + ChannelFetchError, |
| 61 | + ChannelResult, |
| 62 | + FetchContext, |
| 63 | + FetchResult, |
| 64 | +) |
47 | 65 | from backend.channels.registry import register_channel |
48 | 66 |
|
49 | 67 | logger = logging.getLogger(__name__) |
|
80 | 98 | #: taobao-keyword-search's "result count < 10") is understood. |
81 | 99 | _STOP_WHEN_RE = re.compile(r"result_count\s*(<=|<)\s*(\d+)") |
82 | 100 |
|
| 101 | +#: SourceCredential key_name the BrowserAct API key is stored under (decision |
| 102 | +#: #7). Resolved via the existing AuthManager.resolve(source_id) pattern -- |
| 103 | +#: same encrypted store api_channel.py / crawl4ai_channel.py already use, no |
| 104 | +#: new credential path. |
| 105 | +CREDENTIAL_KEY_NAME = "browser_act_api_key" |
| 106 | + |
| 107 | +#: Env var the resolved key is injected under for the browser-act CLI |
| 108 | +#: subprocess (BrowserActSession's ``env``, see backend.browser_act.cli). |
| 109 | +#: The upstream CLI's own docs (browser-act-skills/docs/installation.md) |
| 110 | +#: only document a persistent local auth store -- ``browser-act auth set |
| 111 | +#: <key>`` / ``auth login`` / ``auth poll`` -- and never mention an env var |
| 112 | +#: for feeding a key non-interactively; there is no live browser-act binary |
| 113 | +#: in this environment to confirm one against. This name is this PR's |
| 114 | +#: documented assumption (most likely candidate), chosen to match the |
| 115 | +#: existing BROWSER_ACT_BIN / browser_act_timeout naming convention already |
| 116 | +#: in this codebase -- and PR-C's own test (test_secret_not_leaked_in_error) |
| 117 | +#: and BrowserActError's docstring ("PR-C injects secrets ... via env") |
| 118 | +#: already anticipated exactly this name. |
| 119 | +BROWSER_ACT_API_KEY_ENV = "BROWSER_ACT_API_KEY" |
| 120 | + |
83 | 121 |
|
84 | 122 | def _classify_error(message: str) -> str: |
85 | 123 | """Classify a pack script's reported error message. |
@@ -192,6 +230,49 @@ def _load_pack_manifest( |
192 | 230 | ) |
193 | 231 | return pack_dir, manifest, None |
194 | 232 |
|
| 233 | + @staticmethod |
| 234 | + async def _resolve_session_env( |
| 235 | + source_id: str | None, mode: str |
| 236 | + ) -> tuple[dict[str, str] | None, str | None]: |
| 237 | + """Resolve the stored BrowserAct API key (decision #7, |
| 238 | + ``CREDENTIAL_KEY_NAME``) into subprocess env for the browser-act CLI |
| 239 | + hop, via ``AuthManager.resolve(source_id)`` -- the same encrypted- |
| 240 | + store pattern ``ApiChannel._resolve_auth_headers`` / |
| 241 | + ``Crawl4AIChannel._resolve_cookies`` already use; no new credential |
| 242 | + path. |
| 243 | +
|
| 244 | + Returns ``(session_env, error)``: |
| 245 | +
|
| 246 | + - ``error`` is set only when ``mode == "stealth"`` and no key could |
| 247 | + be resolved -- stealth REQUIRES a key and must never silently fall |
| 248 | + back to chrome-direct or run keyless (decision #7 DoD). |
| 249 | + - ``chrome-direct`` never errors here: a resolved key, if any, is |
| 250 | + still injected (harmless -- browser-act simply doesn't need it), |
| 251 | + but its absence is fine. |
| 252 | +
|
| 253 | + ``source_id`` is ``None`` when ``collect()`` is called directly |
| 254 | + (bypassing the runner). That is treated identically to "no key |
| 255 | + stored" -- best-effort resolution, not a fabricated source id (see |
| 256 | + ``collect()``'s docstring and ``fetch()`` below, the only path that |
| 257 | + ever has a real one in production). |
| 258 | + """ |
| 259 | + key: str | None = None |
| 260 | + if source_id: |
| 261 | + from backend.auth.manager import AuthManager |
| 262 | + |
| 263 | + creds = await AuthManager().resolve(source_id) |
| 264 | + key = creds.get(CREDENTIAL_KEY_NAME) |
| 265 | + |
| 266 | + if mode == "stealth" and not key: |
| 267 | + return None, ( |
| 268 | + "stealth mode requires a BrowserAct API key " |
| 269 | + f"(store it as {CREDENTIAL_KEY_NAME})" |
| 270 | + ) |
| 271 | + |
| 272 | + if key: |
| 273 | + return {BROWSER_ACT_API_KEY_ENV: key}, None |
| 274 | + return None, None |
| 275 | + |
195 | 276 | async def validate_config(self, config: dict[str, Any]) -> list[str]: |
196 | 277 | """Validate channel_config; only checks what config alone can prove: |
197 | 278 | pack resolvability, mode legality, and required params NOT |
@@ -226,8 +307,20 @@ async def validate_config(self, config: dict[str, Any]) -> list[str]: |
226 | 307 | # ── collect() ──────────────────────────────────────────────────────── |
227 | 308 |
|
228 | 309 | async def collect( |
229 | | - self, config: dict[str, Any], parameters: dict[str, Any] |
| 310 | + self, |
| 311 | + config: dict[str, Any], |
| 312 | + parameters: dict[str, Any], |
| 313 | + source_id: str | None = None, |
230 | 314 | ) -> ChannelResult: |
| 315 | + """``source_id`` is an additive optional param beyond the |
| 316 | + ``AbstractChannel.collect(config, parameters)`` contract (legal -- |
| 317 | + ABC only requires the method exist, not an exact signature): a |
| 318 | + direct call (tests, or any future non-source-scoped caller) omits it |
| 319 | + and gets best-effort credential resolution (chrome-direct works |
| 320 | + keyless; stealth still requires a key, see ``_resolve_session_env``). |
| 321 | + ``fetch()`` below is this channel's only production path that ever |
| 322 | + has a real value to pass -- ``run_channel`` populates |
| 323 | + ``FetchContext.source_id`` from the real ``DataSource.id``.""" |
231 | 324 | pack_info, pack_error = self._resolve_pack_info(config) |
232 | 325 | if pack_error: |
233 | 326 | return ChannelResult(success=False, error_type="error", error=pack_error) |
@@ -264,10 +357,14 @@ async def collect( |
264 | 357 |
|
265 | 358 | max_pages = config.get("max_pages") or _DEFAULT_MAX_PAGES |
266 | 359 |
|
267 | | - # Credential injection is PR-E's job (decision #7: SourceCredential / |
268 | | - # AuthManager, key_name="browser_act_api_key"). This hook exists so |
269 | | - # PR-E only has to fill session_env -- the loop below doesn't change. |
270 | | - session_env: dict[str, str] | None = None |
| 360 | + # Credential injection (decision #7): resolve the stored BrowserAct |
| 361 | + # API key via the existing AuthManager.resolve(source_id) pattern and |
| 362 | + # inject it into the browser-act subprocess env. stealth mode without |
| 363 | + # a resolvable key is a hard, loud error here -- it never silently |
| 364 | + # falls back to chrome-direct and never runs keyless. |
| 365 | + session_env, cred_error = await self._resolve_session_env(source_id, mode) |
| 366 | + if cred_error: |
| 367 | + return ChannelResult(success=False, error_type="error", error=cred_error) |
271 | 368 | session_name = f"browser-act-{pack_info.domain}-{pack_info.capability}" |
272 | 369 |
|
273 | 370 | items: list[dict[str, Any]] = [] |
@@ -354,6 +451,37 @@ async def collect( |
354 | 451 | metadata={"pack": pack_info.path, "pages_fetched": pages_fetched, "mode": mode}, |
355 | 452 | ) |
356 | 453 |
|
| 454 | + async def fetch(self, ctx: FetchContext) -> FetchResult: |
| 455 | + """Thick-contract entry point, overridden narrowly (decision #7): |
| 456 | + ``AbstractChannel``'s default ``fetch()`` bridges straight to |
| 457 | + ``collect(ctx.config, ctx.params)`` and drops ``ctx.source_id`` on |
| 458 | + the floor -- every other collect()-only channel (including this |
| 459 | + one before PR-E) is fine with that. Credential resolution needs the |
| 460 | + real ``DataSource.id`` though, and ``run_channel`` (backend.pipeline. |
| 461 | + channel_runner) is the only place that ever has one -- so this |
| 462 | + override exists solely to thread ``ctx.source_id`` through to |
| 463 | + ``collect()``. A direct ``collect()`` call (tests, or any future |
| 464 | + caller outside the runner) still works, just chrome-direct-only / |
| 465 | + best-effort on stealth (see ``_resolve_session_env``). |
| 466 | +
|
| 467 | + Documented trade-off: overriding ``fetch()`` flips |
| 468 | + ``channel_runner.run_channel``'s ``channel_migrated`` check, so the |
| 469 | + runner builds a ``RateLimitedClient``/``httpx.AsyncClient`` for this |
| 470 | + channel's run and tears it down afterward even though this channel |
| 471 | + never reads ``ctx.http`` (browser-act drives its own subprocess, not |
| 472 | + HTTP). Accepted deliberately: ``httpx.AsyncClient()`` never opens a |
| 473 | + real connection until first used, so the unused cost here is one |
| 474 | + Python object for the run's duration, not an open socket -- real |
| 475 | + credential resolution matters more than avoiding that. |
| 476 | + """ |
| 477 | + result = await self.collect(ctx.config, ctx.params, source_id=ctx.source_id) |
| 478 | + if not result.success: |
| 479 | + raise ChannelFetchError( |
| 480 | + result.error or f"{self.channel_type} collect failed", |
| 481 | + error_type=result.error_type, |
| 482 | + ) |
| 483 | + return FetchResult(items=result.items, metadata=result.metadata) |
| 484 | + |
357 | 485 | async def _run_page( |
358 | 486 | self, |
359 | 487 | sess: Any, |
|
0 commit comments