Add ADE browser CLI proof flow#512
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Warning Review limit reached
More reviews will be available in 10 minutes and 48 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (35)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@copilot review but do not make fixes |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
|
@copilot review but do not make fixes |
arul28
force-pushed
the
ade/ade-browser-1d208e91
branch
from
6d1388f to
5647e78
Compare
|
@copilot review but do not make fixes |
1 participant
Summary
Validation
npm --prefix apps/ade-cli run testnpm --prefix apps/ade-cli run typechecknpm --prefix apps/ade-cli run buildnpm --prefix apps/desktop run typechecknpm --prefix apps/desktop run lint(passes with existing warnings)npm --prefix apps/desktop run buildnpx vitest run src/main/services/builtInBrowser/builtInBrowserService.test.ts src/main/services/builtInBrowser/builtInBrowserWebAuthn.test.ts src/main/services/computerUse/computerUseArtifactBrokerService.test.tsnpx vitest run src/renderer/components/chat/AgentChatComposer.test.tsx src/renderer/components/chat/AgentChatPane.test.tsx src/renderer/components/terminals/WorkSidebar.test.tsx src/renderer/components/terminals/WorkViewArea.test.tsxnpm --prefix apps/ade-cli run typecheck,npm --prefix apps/desktop run typecheck, and targeted desktop browser/chat testsRisks
Greptile Summary
This PR introduces the ADE Browser — a project-scoped built-in browser with full agent automation capabilities, replacing the previous global single-partition model with per-project Electron sessions and per-tab ownership/leasing.
builtInBrowserServicemanages multiple window×profile service instances and routes API calls by project root or source window.observe,click,fill,typeText,dispatchKey,scroll,clear, andwaitmethods write screenshots and DOM snapshots to disk (.ade/cache/browser-observations/), capture per-tab network/console diagnostics, and record action traces. Corresponding CLI subcommands (browser session,browser observe,browser click, etc.) are added with rich flag support.AgentChatPane: Draft kind keyschatandcliare unified underwork-start, with a multi-key read that picks the most recently updated entry for backward compatibility.Confidence Score: 3/5
The core browser automation machinery is well-structured, but there is a path traversal in the element-handle file lookup that must be fixed before this ships to agents.
The new
readObservationElementHandlefunction builds a file path from a user-supplied element handle whose observation ID is validated by a regex that permits/characters, allowingpath.joinnormalization to read files outside the intended per-tab cache directory. Legitimate observation IDs never contain slashes, so a one-linesanitizePathSegmentcall onparsed.observationIdwould close the issue. The rest of the change — project-scoped partitions, per-tab leasing, action tracing, observation pruning — is carefully implemented with appropriate bounds on all user-controlled numeric inputs.builtInBrowserService.tsaroundreadObservationElementHandleand theparseElementHandleregex;main.tsfor the window-activation side effect ingetWindowForProjectRoot.Security Review
readObservationElementHandleinbuiltInBrowserService.tsuses theobservationIdfrom a user-supplied element handle directly inpath.joinwithout sanitization. The parsing regex (obs-[^:]+) permits/in the observation ID, allowing a crafted handle likeobs-x/../../etc/secret:e:1to traverse outside the intended per-tab cache directory and read arbitrary.jsonfiles from the host filesystem. Impact in this desktop context is limited by thetabIdfield check, but the unauthorized file read and potential in-memory load of large files are present vulnerabilities.Important Files Changed
readObservationElementHandlewhere unsanitized user-suppliedobservationIdcan escape the expected cache directory viapath.join. Also has inconsistentdeletedCountunits in the cleanup result.fillvalue vs. element-targettextfield.autoNewOwnedTabheuristic that opens a fresh tab for agent callers without an explicit tab target is a good safety guard.getWindowForProjectRootcallback may activate (foreground) windows as a side effect of passive API routing calls, which could disrupt user focus unexpectedly.parseBuiltInBrowserTabTargetArgshelper. Changes look correct and consistent with the new per-tab targeting model.work-startkind (shared betweenchatandcli), reads across multiple legacy keys by picking the most-recently-updated entry, and adds a guard that rejects cursor model configs for incompatible draft kinds. Migration logic is careful and uses Sets to deduplicate keys.configureBuiltInBrowserSessionWebAuthnguarded by a module-levelWeakSet, enabling each new browser profile partition to get its own WebAuthn handler. TheshouldConfigureTouchIdWebAuthndefault now enables Touch ID for packaged builds without an explicit env var.BUILT_IN_BROWSER_PROFILE_PREFIXfor project-scoped partition names. Simple and correct.Sequence Diagram
sequenceDiagram participant Agent as Agent CLI participant Bridge as Desktop Bridge participant Service as BuiltInBrowserService participant WinService as WindowBrowserService participant FS as File System (project/.ade/cache) participant WC as WebContents (CDP) Agent->>Bridge: browser open url --lane id Bridge->>Service: navigate(input, sourceWindow) Service->>WinService: navigate(input) WinService->>WC: loadURL(url) WinService-->>Agent: BuiltInBrowserStatus Agent->>Bridge: browser session start --tab id Bridge->>Service: startSession(input) Service->>WinService: startSession(input) WinService-->>Agent: session + status Agent->>Bridge: browser observe --browser-session sid --map Bridge->>Service: observe(input) Service->>WinService: observe(input) WinService->>WC: capturePage CDP WC-->>WinService: screenshot PNG WinService->>WC: Runtime.evaluate BROWSER_DOM_FUNCTION WC-->>WinService: DOM snapshot + elements WinService->>WC: Runtime.evaluate ELEMENT_MAP_OVERLAY WC-->>WinService: element map screenshot WinService->>FS: write obs-id.png obs-id.map.png obs-id.json WinService->>FS: pruneObservationDirectory keepCount WinService-->>Agent: BuiltInBrowserObservation filePath dom elementMap Agent->>Bridge: browser click --handle obs-id:e:3 Bridge->>Service: click(input) Service->>WinService: click(input) WinService->>FS: readFile obs-id.json element handle lookup FS-->>WinService: element snapshot selector center WinService->>WC: Input.dispatchMouseEvent CDP WinService->>WinService: observe post-action screenshot WinService-->>Agent: BuiltInBrowserAgentActionResult ok observation tracePrompt To Fix All With AI
Reviews (1): Last reviewed commit: "Add ADE browser CLI proof flow" | Re-trigger Greptile