feat(desktop): import from Chrome, Edge, Brave, Vivaldi, Opera, Arc and Firefox - #7260
feat(desktop): import from Chrome, Edge, Brave, Vivaldi, Opera, Arc and Firefox#7260juliusmarminge wants to merge 6 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
Thread transfer impact✅ Thread transfer remains within every enforced ceiling.
Baseline: unavailable · PR result: Scenario and decoded snapshot size10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.
Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed. |
bc1215b to
973b17e
Compare
There was a problem hiding this comment.
One convention finding: the new Firefox cookie read path models its failure as an anonymous object literal instead of a Schema.TaggedErrorClass. Details inline.
Posted via Macroscope — Effect Service Conventions
ApprovabilityVerdict: Needs human review 1 blocking correctness issue found. Substantial new feature adding cookie import from 7 browsers with significant new logic and security-sensitive file operations. An unresolved HIGH severity finding identifies a path traversal vulnerability in Firefox profile parsing that could allow importing attacker-controlled SQLite files. You can customize Macroscope's approvability policy. Learn more. |
973b17e to
1b132e4
Compare
3343565 to
56f1705
Compare
There was a problem hiding this comment.
Effect service conventions: one finding on the new FirefoxCookieReadError. The tagged-error union fix from the previous round looks good; what remains is the structural context the error captures.
Posted via Macroscope — Effect Service Conventions
56f1705 to
4d48bd9
Compare
4d48bd9 to
0d85caa
Compare
7a8cf25 to
27037c9
Compare
27037c9 to
9328514
Compare
9328514 to
de17611
Compare
de17611 to
333c68e
Compare
There was a problem hiding this comment.
One retained finding in the Firefox error model; the rest of the Effect service conventions look satisfied (namespace subpath imports, tagged errors at the failure boundary, Context.Service/make/layer order preserved, no new runtimes or injected service instances).
Posted via Macroscope — Effect Service Conventions
333c68e to
c2e27ca
Compare
c2e27ca to
7487b3c
Compare
There was a problem hiding this comment.
One finding: the mapped error union is discriminated by hand instead of with Effect.catchTags.
Posted via Macroscope — Effect Service Conventions
7487b3c to
8538b6c
Compare
8538b6c to
b9dd903
Compare
b9dd903 to
5f17f5b
Compare
5f17f5b to
533438c
Compare
| const key = line.slice(0, separator).trim().toLowerCase(); | ||
| const value = line.slice(separator + 1).trim(); | ||
| if (key === "name") current.name = value; | ||
| if (key === "path") current.path = value; |
There was a problem hiding this comment.
🟠 High BrowserImport/Sources.ts:232
listSourceProfiles returns Path=../../... and arbitrary absolute paths from profiles.ini as trusted profile directories, so cookieDatabasePath can open <that path>/cookies.sqlite outside Firefox’s user-data directory and import an attacker-selected SQLite file. Validate and normalize each parsed path before returning it, enforcing the intended Firefox profile-root boundary (while explicitly handling any supported legitimate absolute-profile case).
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @apps/desktop/src/preview/BrowserImport/Sources.ts around line 232:
`listSourceProfiles` returns `Path=../../...` and arbitrary absolute paths from `profiles.ini` as trusted profile directories, so `cookieDatabasePath` can open `<that path>/cookies.sqlite` outside Firefox’s user-data directory and import an attacker-selected SQLite file. Validate and normalize each parsed path before returning it, enforcing the intended Firefox profile-root boundary (while explicitly handling any supported legitimate absolute-profile case).
5f17f5b to
18520a8
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Bugbot Autofix is ON, but a cloud agent failed to start.
Reviewed by Cursor Bugbot for commit 18520a8. Configure here.
…Firefox Generalises the importer from one hardcoded browser to a source registry with two engines. Chromium forks are table entries: Chrome, Edge, Brave, Vivaldi, Opera, Arc and Helium all share the existing extractor and differ only in their paths and keychain coordinates. Those coordinates are pinned per fork rather than derived, because the forks disagree — Helium uses "Helium Storage Key" / "Helium" where the others use "<Name> Safe Storage" / "<Name>". Firefox is a second engine. It stores cookies unencrypted in `cookies.sqlite`, so there is no key to fetch and no consent prompt — that is Mozilla's design choice, not a control being circumvented, and it is why Firefox works identically on all three platforms while Chromium still needs a per-platform credential store. Paths resolve for macOS, Windows and Linux from an injected context rather than from `process`, so a platform's layout can be checked without running on it. Chromium off macOS still reports `unsupportedPlatform` until those key stores land; Firefox does not. The snapshot-before-read step moves to a shared module, since both engines keep the database open with WAL and must never have the browser's own file opened for writing. Firefox has tests against a real `moz_cookies` fixture, including that the source file is left untouched, and `profiles.ini` parsing covers the `Install*` sections that name a default profile without describing one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ontainers apart Firefox keeps its lock files inside each profile, not at the user-data root, so the running check never found them and offered imports from a live, mid-write database. It now walks the source's profiles and looks for all three names the platforms use. Opera does not follow the local-AppData `User Data` convention its Chromium relatives use — it lives under roaming `%APPDATA%\Opera Software\Opera Stable` — so it was never detected on Windows. Firefox isolates cookies per container and per private window through `originAttributes`. Electron has no equivalent, so importing them all collapsed several identities onto one host/name/path and handed the profile whichever container was written last. Only the default container is imported. The sidecar copy no longer ignores every error alongside the missing-file case; the new snapshot test caught that the earlier fix had landed on the pre-extraction copy of this code rather than the shared module. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Firefox branch failed with an anonymous `{ reason, cause }` literal, and
typing the shared `read` value as that shape erased `ChromiumCookieReadError`'s
tag from the error channel — neither branch could then be handled with
`Effect.catchTags`.
`FirefoxCookieReadError` mirrors its Chromium counterpart, so the union stays
structurally identifiable.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Firefox keeps one cookie database per profile, so a failure carrying only a reason cannot be traced back to the profile that produced it. The path is now a structural attribute, matching `ChromiumCookieReadError`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Firefox has one failure mode — its plaintext database would not open — so a single-value `reason` literal encoded the same thing as the tag, and `cause` was optional though every construction site wraps a real failure. The error now carries the database path and a required cause, and `BrowserImport` supplies the user-facing reason where it maps the union. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The union was discriminated with a `cause._tag` ternary inside `Effect.mapError` even though both members are statically known tagged errors; `Effect.catchTags` says the same thing without the manual check. The comment above `read` claiming both carry a `reason` was stale — Firefox's no longer does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
18520a8 to
f17e123
Compare

Generalises the importer from one hardcoded browser to a source registry with two engines, taking it from 1 source to 8.
¹ Chromium off macOS reports
unsupportedPlatformuntil its key stores land — that's the next PR.Notes for review
Chromium forks are table entries. They share the existing extractor and differ only in paths and keychain coordinates. Those are pinned per fork rather than derived, because the forks disagree — Helium uses
Helium Storage Key/Heliumwhere the others use<Name> Safe Storage/<Name>.Firefox is a second engine with no crypto at all. It stores cookies in plaintext
cookies.sqlite, so there's no key and no consent prompt. That's Mozilla's design choice, not a control being circumvented, and it's why Firefox works identically on all three platforms while Chromium needs a per-platform credential store.Paths resolve from an injected context, not
process, so Windows and Linux layouts can be checked without running on them.The snapshot-before-read step moved to a shared module. Both engines keep the database open with WAL, and neither may have the browser's own file opened for writing.
Testing
Verified against the machine this was written on — all seven Chromium sources detected, Chrome reporting its real profile name:
Firefox isn't installed here, so it's covered by tests against a real
moz_cookiesfixture rather than a claim — including one asserting the source file's mtime and size are unchanged after a read.profiles.iniparsing has its own tests because of a genuine trap:Install*sections name a default profile without describing one, so counting them invents a directory that doesn't exist.Note
Add browser import support for Chrome, Edge, Brave, Vivaldi, Opera, Arc, and Firefox
BROWSER_IMPORT_SOURCESin Sources.ts to include multiple Chromium-based browsers and Firefox, with per-platform user-data directory resolution.cookies.sqliteusingNodeSqliteClient, restricted to the default container, with sameSite and session cookie mapping.cookieScope,bareHost,snapshotCookieDatabase,ImportedCookie) into CookieDatabase.ts for reuse across Chromium and Firefox paths.BrowserImportPathContextto carry platform, home, appData, and localAppData for cross-platform path resolution; Chromium sources are now gated to macOS.unsupportedPlatformfor all Chromium-engine sources.Macroscope summarized f17e123.
Note
Medium Risk
Touches cookie decryption, keychain access, and filesystem reads of other browsers’ profiles; path traversal hardening and lock checks are intentional, but wrong paths or lock logic could block imports or read stale DBs.
Overview
Generalises desktop browser cookie import from one hardcoded Chromium source to a registry of eight browsers (Chrome, Edge, Brave, Vivaldi, Opera, Arc, Helium, Firefox), modelled as
chromiumvsfirefoxengines with per-fork paths and keychain coordinates inSources.ts.BrowserImportnow resolves paths viasourcePathContext/BrowserImportPathContext(platform,%APPDATA%,%LOCALAPPDATA%), lists profiles per engine (ChromiumLocal Statevs Firefoxprofiles.ini), and dispatches reads toreadChromiumCookiesorreadFirefoxCookies. Chromium import is rejected off macOS withunsupportedPlatformuntil non-macOS key stores exist; Firefox works on all platforms (plaintext DB, no keychain).New
FirefoxCookiesmapsmoz_cookiesto Electron’s cookie shape, snapshots before read, and imports only the default container (originAttributes = ''). SharedCookieDatabaseholdsImportedCookie,cookieScope, andsnapshotCookieDatabase(WAL/SHM copy semantics) used by both engines.Source detection fixes: Firefox running checks profile-level locks (
.parentlock, etc.) instead of Chromium’s rootSingletonLock; Opera on Windows uses roaming AppData without aUser Datasegment. ContractsBROWSER_IMPORT_SOURCE_IDSgrows to match the registry.Reviewed by Cursor Bugbot for commit f17e123. Bugbot is set up for automated code reviews on this repo. Configure here.