Conversation
…browsing + local_browser arg
૮ >ﻌ< ა ci reviewran on cf7d308 — feat(browser): consent-gated real default-Chromium profile f ❌ Job failuresPython lints / Windows footguns (blocking) · View jobJob Python lints / Windows footguns (blocking) failed. Python tests / Run tests · View jobJob Python tests / Run tests failed.
|
alt-glitch
added
type/feature
Review notes — real Chromium profile browsing
Really like the direction here — default-off, honest risk description, reusing the existing sidecar routing. A few things we noticed that might save a follow-up fix cycle, roughly in order of impact: Worth verifying before relying on it
Bugs we can reproduce
Security thoughtsThe agreed framing — consent-gated convenience, not a hardened isolation boundary — is stated clearly in the PR, and the inherent real-profile risks are squarely inside that accepted tradeoff. Nothing in the change adds injection, an external bypass, or an isolation escape; argv handling is clean, CDP/Camofox precedence holds, and no-consent Three things touch the consent mechanism itself, which is the one property the PR does claim:
And two friendly hardening suggestions:
Small stuff
SummaryToggle off, this is essentially inert and low-risk. Toggle on, the highest-value improvements are items 4–6 plus a real-launch check for item 1 — those cover the cases most likely to come back as bug reports. Items 10 and 11 are the best bang-for-buck hardening. Happy to help with any of these. |
Review: the resolver/consent plumbing is clean, but the launch it feeds doesn't deliver a usable real profile on the main platformsRead the PR head ( Blockers (the feature as described does not work end to end)
Should fix before merge
Minor
Related open work
Suggested pathEither adopt agent-browser's copy semantics (profile name → |
|
Following up on my review above with empirical testing results — I ran the launch path end to end on Linux (agent-browser 0.26.0, isolated Item 1 confirmed: real Google Chrome ≥136 blocks the launch path — silent hangTested with branded Google Chrome 152.0.7977.64 stable (extracted current
So the Chrome ≥136 restriction this PR defers for the CDP/ The good news: the mechanism works where Chrome doesn't block itFull unmocked end-to-end on Chromium 151 (Chrome for Testing build, which doesn't enforce the block): config read → default-browser detection → resolver → agent-browser launch → Other reproductions
Happy to share exact commands/fixtures for any of these. |
|
Two gaps none of the reviews have covered yet, plus one caveat on the Chromium 151 result and two small corrections. 1. The bundled-Chromium gate blocks or bloats the real-profile path on Windows and macOS
Two outcomes, both wrong for a consented user whose real browser exists:
This is the desktop flip side of the headless-Linux note above. On headless Linux the resolver fails closed because xdg has no answer. On a Windows or macOS desktop the detection succeeds and this separate gate is what misfires. Fix: when consent is on and 2. Non-stable browser channels resolve to the wrong profileThe detection maps treat every channel of a browser as the same browser. They are not. I did not test this on real Beta/Canary installs, so the registry details are worth a quick live check, but the mapping logic is right there in the diff.
The consent story is "we drive your default browser's real profile". Driving a different profile is the kind of silent wrong turn the fail-closed design exists to prevent. Fix direction: on Windows, resolve the real exe from 3. Caveat on the Chromium 151 "good news" resultThe cookie that decrypted in that test proves less than it seems. Per the agent-browser source, path-mode profiles always get Small
Two corrections
|
|
Follow-up PR with fixes for the points above, stacked on this branch: #95549. Twelve commits, one topic each with its own tests, so they can be cherry-picked individually: the three CI failures; fail-closed on a missing binary; launch-flag resolution once per process with Thanks @unsupportedpastels — the Chrome 152 and lock reproductions went straight into the commit messages and test fixtures. Not touched, since they're design calls: in-place drive vs. agent-browser's profile copy, the mock-keychain flags (agent-browser side), a shared session for concurrent tasks, |
|
@Adolanium — both gaps are real and both are now in #95549 (launch-path and detection commits):
The consent-off note now names |
E2E test: Chrome 136+ blocks the --profile path — fix insideThe problemTested your PR end-to-end on Chrome 152 / macOS. The --profile injection works in code, but Chrome 136+ blocks --remote-debugging-port on the default user-data-dir. agent-browser launches Chrome with --remote-debugging-port=0 --user-data-dir=, so Chrome starts but never opens a CDP port -> 120s timeout. Chrome stderr: Your PR body already documents this as a browser_exec-only limitation, but it also breaks the built-in browser tools path — agent-browser uses CDP internally (--remote-debugging-port=0) even for --session/--profile launches. The fix (3 changes)1. hermes_cli/browser_connect.py — add ensure_remote_debugging_policy() after detect_default_chromium(): The enterprise policy RemoteDebuggingAllowed overrides Chrome 136+ default-profile block. Per-OS:
Full function with all 4 browsers per-OS: 2. tools/browser_tool.py — rewrite _real_profile_launch_args() to return --cdp instead of --profile when the policy succeeds: def _real_profile_launch_args() -> tuple:
if not _use_real_profile():
return [], None
from hermes_cli.browser_connect import (
chromium_executable, detect_default_chromium,
ensure_remote_debugging_policy, real_profile_data_dir,
)
browser = detect_default_chromium()
if browser is None:
return [], ("...not a supported Chromium...")
data_dir = real_profile_data_dir(browser)
if not data_dir or not os.path.isdir(data_dir):
return [], ("...profile directory was not found...")
exe = chromium_executable(browser)
policy_ok = ensure_remote_debugging_policy(browser)
if policy_ok and exe:
_launch_real_profile_chrome(exe, data_dir, _REAL_PROFILE_CDP_PORT)
return ["--cdp", f"http://127.0.0.1:{_REAL_PROFILE_CDP_PORT}"], None
# Fallback: old --profile path (Chrome <136)
args = ["--profile", data_dir]
if exe:
args += ["--executable-path", exe]
return args, NonePlus a _launch_real_profile_chrome() helper that spawns Chrome with --remote-debugging-port= --remote-allow-origins=* (detached, skips if SingletonLock exists): 3. tools/browser_tool.py caller (_run_browser_command) — when _real_profile_launch_args() returns --cdp, it must REPLACE --session, not append: if _profile_args:
if _profile_args[0] == "--cdp":
backend_args = _profile_args # replace --session with --cdp
else:
backend_args += _profile_args # old --profile pathThis is critical — agent-browser silently ignores --cdp when --session is present (you already noted this in a comment for the cloud path). Port noteChrome chrome://inspect toggle (devtools.remote_debugging.user-enabled) forces port 9222 and overrides --remote-debugging-port=. Use 9222 if the toggle is on, or any port if it is off (the policy allows it either way). I hardcoded 9222 in the test; a DevToolsActivePort read would be more robust. E2E verification
Full diff: https://github.com/kshitijk4poor/hermes-agent/compare/browser-real-profile-fix |
|
Closed in favor of newer: #95620 |
|
Superseded by #95620 (merged, Thanks @teknium1 — the real-profile consent + resolvers from this PR are the foundation of the merged change. The redo re-based it onto the current lane and fixed the issues surfaced in review:
Your authorship is preserved in the merged history. Closing this in favor of #95620. |
6 participants
Adds a backend-independent, consent-gated way to drive the user's real default-Chromium profile (their logins/cookies) for local browser use.
What the user asked for
local_browser: booltool arg so the agent can force a local real-profile session even under a cloud backend — same spirit as the existing localhost/LAN→local fallback.How it works
The local backend (
agent-browser) natively supports--profile <user-data-dir>(persists real cookies/logins) and--executable-path. So this needs no CDP chrome://inspect toggle and no cua-driver dependency — a local launch just points at the real profile dir + browser binary.browser.use_real_profile(new config, default false). Consent gate. When on, every local Chromium launch — built-in tools, or a local sidecar spawned under a cloud backend — appends--profile <real-default-profile>+--executable-path <binary>.hermes_cli/browser_connect.py):detect_default_chromium()(Windows UserChoice ProgId, macOS LaunchServices, Linuxxdg-settings),real_profile_data_dir(),chromium_executable()for all four browsers, per-OS. Non-Chromium default → resolves to None → fail closed with a message naming the browser and the toggle.local_browserarg onbrowser_navigate: routes through the existing::localsidecar mechanism (same one used for private-URL→local today), but only when consent is granted. Requested without consent → ignored, with anotein the result (never silently routes the real profile). A CDP override or Camofox mode still owns the session.FIELD_LABELS/FIELD_DESCRIPTIONS.Scope / honest boundaries
browser_exec) attaches only via CDP, and real-profile-over-CDP hits Chrome ≥136's default-user-data-dir remote-debugging block — which requires the chrome://inspect-toggle route. That's deliberately deferred (documented follow-up); this PR does not claim to coverbrowser_exec.--profileon the real dir may fail; agent-browser surfaces that. We do not silently fork the profile (avoids the token-fork class from feat(gateway): share_auth on profiles.create + MCP servers in profiles.describe/configure #85963).Files
hermes_cli/config_defaults.py—browser.use_real_profile(default false).hermes_cli/browser_connect.py— resolvers + default-browser detection (per-OS, POSIX/NT path-correct via posixpath/ntpath so an explicitsystemarg resolves right on any host).tools/browser_tool.py—_use_real_profile(),_real_profile_launch_args(), local-launch injection (fail-closed),local_browseron_navigation_session_key+browser_navigate+ schema + dispatch, consent-miss note.apps/desktop/src/app/settings/constants.ts— Browser section + labels/description.tests/tools/test_browser_real_profile.py— 12 tests.Verification
tests/tools/test_browser_real_profile.py: 12 passed (resolvers per-OS, consent on/off, non-Chromium fail-closed, missing-dir fail-closed,local_browserrouting with/without consent, CDP-override precedence).tests/tools/test_browser_hybrid_routing.py: 10 passed — no regression from the_navigation_session_keysignature change (new param defaulted).browser.use_real_profile→browser.useRealProfile, matches the added label/description entries). Full desktop vitest not run here (node_modules not installed in the worktree); CI runs it.Not run: broad suite (per maintainer preference — targeted checks only).