History

fix(desktop): defer keyring loading until macOS cookie import (#10667)

Abdel Abdelwaly committed 2 months ago

Commit 20e2e891 parent 7fbc5453 files · +95 −1Browse files

Unified · Split

3 files
  1. apps/desktop/src/preview/BrowserImport/ChromiumKeys.module.test.ts +35 −0
  2. apps/desktop/src/preview/BrowserImport/ChromiumKeys.test.ts +54 −0
  3. apps/desktop/src/preview/BrowserImport/ChromiumKeys.ts +6 −1
apps/desktop/src/preview/BrowserImport/ChromiumKeys.module.test.ts+35−0
@@ -0,0 +1,35 @@
1+import { expect, it } from "@effect/vitest";
2+import * as Effect from "effect/Effect";
3+import { ChildProcessSpawner } from "effect/unstable/process";
4+import { vi } from "vite-plus/test";
5+
6+vi.mock("@napi-rs/keyring", () => {
7+ throw new Error("Cannot find native binding");
8+});
9+
10+it("loads browser import code without a keyring native binding", async () => {
11+ await expect(import("./ChromiumKeys.ts")).resolves.toBeDefined();
12+});
13+
14+it.effect("reports an unavailable keychain when the macOS binding cannot load", () =>
15+ Effect.gen(function* () {
16+ const { ChromiumKeyError, resolveChromiumKeys } = yield* Effect.promise(
17+ () => import("./ChromiumKeys.ts"),
18+ );
19+ const error = yield* resolveChromiumKeys({
20+ platform: "darwin",
21+ keychainService: "Chrome Safe Storage",
22+ keychainAccount: "Chrome",
23+ linuxSecretApplication: undefined,
24+ }).pipe(Effect.flip);
25+
26+ expect(error).toBeInstanceOf(ChromiumKeyError);
27+ expect(error.reason).toBe("keychainUnavailable");
28+ expect(error.cause).toBeInstanceOf(Error);
29+ }).pipe(
30+ Effect.provideService(
31+ ChildProcessSpawner.ChildProcessSpawner,
32+ ChildProcessSpawner.make(() => Effect.die("must not spawn")),
33+ ),
34+ ),
35+);
apps/desktop/src/preview/BrowserImport/ChromiumKeys.test.ts+54−0
@@ -7,6 +7,7 @@ import * as Sink from "effect/Sink"; · 6 unmodified lines
77 import * as Stream from "effect/Stream";
88 import * as PlatformError from "effect/PlatformError";
99 import { ChildProcessSpawner } from "effect/unstable/process";
10+import { beforeEach, vi } from "vite-plus/test";
1011
1112 import {
1213 ChromiumKeyError,
@@ -17,6 +18,18 @@ import { · 4 unmodified lines
1718 } from "./ChromiumKeys.ts";
1819 import { LinuxBrowserSecretPath } from "./LinuxBrowserSecret.ts";
1920
21+const { getPassword } = vi.hoisted(() => ({ getPassword: vi.fn<() => string | null>() }));
22+
23+vi.mock("@napi-rs/keyring", () => ({
24+ Entry: class {
25+ getPassword = getPassword;
26+ },
27+}));
28+
29+beforeEach(() => {
30+ getPassword.mockReset();
31+});
32+
2033 type CapturedCommand = {
2134 readonly command: string;
2235 readonly args: ReadonlyArray<string>;
@@ -63,6 +76,47 @@ const helperLayer = (input: { · 40 unmodified lines
6376 ),
6477 );
6578
79+describe("macOS Chromium secrets", () => {
80+ const request = {
81+ platform: "darwin",
82+ keychainService: "Chrome Safe Storage",
83+ keychainAccount: "Chrome",
84+ linuxSecretApplication: undefined,
85+ } as const;
86+ const noProcesses = Layer.succeed(
87+ ChildProcessSpawner.ChildProcessSpawner,
88+ ChildProcessSpawner.make(() => Effect.die("must not spawn")),
89+ );
90+
91+ it.effect("derives the cookie key from the keychain secret", () =>
92+ Effect.gen(function* () {
93+ getPassword.mockReturnValue("macos-secret");
94+ const keys = yield* resolveChromiumKeys(request);
95+ expect(keys.cbcV10?.toString("hex")).toBe("3df7306fb1eac353289565a2f6b64f74");
96+ }).pipe(Effect.provide(noProcesses)),
97+ );
98+
99+ it.effect("reports a missing keychain entry", () =>
100+ Effect.gen(function* () {
101+ getPassword.mockReturnValue(null);
102+ const error = yield* resolveChromiumKeys(request).pipe(Effect.flip);
103+ expect(error.reason).toBe("keychainItemMissing");
104+ }).pipe(Effect.provide(noProcesses)),
105+ );
106+
107+ it.effect("preserves a denied keychain approval", () =>
108+ Effect.gen(function* () {
109+ const denied = new Error("User denied access");
110+ getPassword.mockImplementation(() => {
111+ throw denied;
112+ });
113+ const error = yield* resolveChromiumKeys(request).pipe(Effect.flip);
114+ expect(error.reason).toBe("needsKeychainApproval");
115+ expect(error.cause).toBe(denied);
116+ }).pipe(Effect.provide(noProcesses)),
117+ );
118+});
119+
66120 describe("Linux Chromium secrets", () => {
67121 it.effect("retains a missing helper failure alongside the keyring-free fallback", () =>
68122 Effect.gen(function* () {
apps/desktop/src/preview/BrowserImport/ChromiumKeys.ts+6−1
@@ -17,7 +17,6 @@ · 16 unmodified lines
1717 *
1818 * @module ChromiumKeys
1919 */
20−import * as Keyring from "@napi-rs/keyring";
2120 import * as NodeCrypto from "node:crypto";
2221
2322 import { HostProcessEnvironment } from "@t3tools/shared/hostProcess";
@@ -102,6 +101,12 @@ const readKeychainSecret = Effect.fn("ChromiumKeys.readKeychainSecret")(function · 78 unmodified lines
102101 service: string,
103102 account: string,
104103 ) {
104+ // Only macOS cookie imports need this binding; loading it at startup can
105+ // prevent the desktop from opening on platforms that never use it.
106+ const Keyring = yield* Effect.tryPromise({
107+ try: () => import("@napi-rs/keyring"),
108+ catch: (cause) => new ChromiumKeyError({ reason: "keychainUnavailable", cause }),
109+ });
105110 const secret = yield* Effect.try({
106111 try: () => new Keyring.Entry(service, account).getPassword(),
107112 catch: (cause) => {