fix: harden keychain consent startup and modal feedback

2026-07-27 20:40:12 +00:00 · 2026-07-27 09:58:32 +03:30
parent 61dfcf5d8a
commit 5db9388d07
3 changed files with 178 additions and 119 deletions
@@ -417,6 +417,101 @@ function App() {
        unlistenDownload = null;
      };

+      // Establish the consent barrier before registering listeners or loading
+      // persisted downloads. Those later startup steps can cause user-facing
+      // work to begin; the explanation must already be committed before any
+      // path is allowed to reach the credential store.
+      await waitForSettingsHydration();
+      if (!active) return;
+      const [currentAppVersion, currentPlatform] = await Promise.all([
+        getVersion().catch(() => ''),
+        getPlatformInfo().catch(() => null)
+      ]);
+      if (!active) return;
+      const currentKeychainConsentVersion = getKeychainConsentVersion(currentAppVersion);
+      setKeychainConsentVersion(currentKeychainConsentVersion);
+
+      try {
+        const settings = useSettingsStore.getState();
+        const isStartupActive = () => active;
+        const { deferKeychainHydration, showKeychainPrompt } = getKeychainStartupDecision({
+          portable: currentPlatform?.portable === true,
+          appVersion: currentKeychainConsentVersion,
+          approvedVersion: settings.keychainAccessVersion,
+          accessGranted: settings.keychainAccessGranted,
+          promptDismissed: settings.keychainPromptDismissed
+        });
+        let changed = false;
+        if (deferKeychainHydration) {
+          settings.setKeychainAccessReady(false);
+          if (showKeychainPrompt) settings.setShowKeychainModal(true);
+          await settings.hydrateSessionPairingToken(isStartupActive);
+          if (!active) return;
+        } else {
+          await invoke('authorize_keychain_access');
+          if (!active) return;
+          changed = await settings.hydratePairingToken(isStartupActive);
+          if (!active) return;
+          const currentSettings = useSettingsStore.getState();
+          settings.setKeychainAccessReady(getKeychainAccessReady({
+            portable: currentPlatform?.portable === true,
+            accessGranted: currentSettings.keychainAccessGranted,
+            persistent: currentSettings.isPairingTokenPersistent
+          }));
+        }
+        if (changed) {
+          addToast({
+            variant: 'warning',
+            isActionable: true,
+            message: (
+              <div className="flex flex-col gap-2">
+                <p>{t($ => $.app.extensionDisconnected)}</p>
+                <div className="flex gap-2 justify-end">
+                  <button
+                    type="button"
+                    className="app-button px-2 py-1 bg-surface-raised border border-border-color rounded"
+                    onClick={async () => {
+                      const token = useSettingsStore.getState().extensionPairingToken;
+                      try {
+                        if (token) await navigator.clipboard.writeText(token);
+                        acknowledgePairingTokenChange();
+                      } catch (error) {
+                        addToast({
+                          message: t($ => $.app.copyTokenFailed, { detail: String(error) }),
+                          variant: 'error',
+                          isActionable: true
+                        });
+                      }
+                    }}
+                  >
+                    {t($ => $.app.copyToken)}
+                  </button>
+                  <button
+                    type="button"
+                    className="app-button px-2 py-1 bg-surface-raised border border-border-color rounded"
+                    onClick={() => {
+                      const settings = useSettingsStore.getState();
+                      settings.setActiveSettingsTab('integrations');
+                      settings.setActiveView('settings');
+                      acknowledgePairingTokenChange();
+                    }}
+                  >
+                    {t($ => $.app.integrations)}
+                  </button>
+                </div>
+              </div>
+            )
+          });
+        }
+      } catch (error) {
+        console.error('Failed to hydrate extension pairing token:', error);
+        addToast({
+          message: t($ => $.app.credentialPersistenceFailed, { detail: String(error) }),
+          variant: 'error',
+          isActionable: true
+        });
+      }
+
      try {
        unlistenDownload = await initDownloadListener();
        unlistenTerminalState = await listen('download-state', (event) => {
@@ -479,7 +574,20 @@ function App() {
          cleanupListeners = null;
          return;
        }
+      } catch (error) {
+        disposeListeners();
+        cleanupListeners = null;
+        if (!active) return;
+        console.error('Failed to initialize Firelink listeners:', error);
+        addToast({
+          message: t($ => $.app.initializeDownloadsFailed, { detail: String(error) }),
+          variant: 'error',
+          isActionable: true
+        });
+        return;
+      }

+      try {
        await initializeDownloadState();
        if (!active) return;
      } catch (error) {
@@ -495,110 +603,6 @@ function App() {
        return;
      }

-      const [currentAppVersion, currentPlatform] = await Promise.all([
-        getVersion().catch(() => ''),
-        getPlatformInfo().catch(() => null)
-      ]);
-      if (!active) return;
-      const currentKeychainConsentVersion = getKeychainConsentVersion(currentAppVersion);
-      setKeychainConsentVersion(currentKeychainConsentVersion);
-
-      try {
-        const settings = useSettingsStore.getState();
-        const isStartupActive = () => active;
-        const { deferKeychainHydration, showKeychainPrompt } = getKeychainStartupDecision({
-          portable: currentPlatform?.portable === true,
-          appVersion: currentKeychainConsentVersion,
-          approvedVersion: settings.keychainAccessVersion,
-          accessGranted: settings.keychainAccessGranted,
-          promptDismissed: settings.keychainPromptDismissed
-        });
-
-        let changed = false;
-        if (deferKeychainHydration) {
-          settings.setKeychainAccessReady(false);
-          if (showKeychainPrompt) {
-            // Commit the explanation before the harmless session-token IPC so
-            // a slow startup cannot leave the user facing an unexplained
-            // credential-store request.
-            settings.setShowKeychainModal(true);
-          }
-          // This token is already owned by the backend and does not access
-          // the OS credential store. Render our explanation before any native
-          // Keychain/Credential Manager prompt can be user-triggered.
-          await settings.hydrateSessionPairingToken(isStartupActive);
-          if (!active) return;
-        } else {
-          // The backend keeps credential-store access disabled for every new
-          // process. Arm it only after the persisted startup decision has
-          // confirmed that this build was already approved; the hydrate call
-          // below is then the first operation allowed to touch the OS store.
-          await invoke('authorize_keychain_access');
-          if (!active) return;
-          changed = await settings.hydratePairingToken(isStartupActive);
-          if (!active) return;
-          const currentSettings = useSettingsStore.getState();
-          settings.setKeychainAccessReady(getKeychainAccessReady({
-            portable: currentPlatform?.portable === true,
-            accessGranted: currentSettings.keychainAccessGranted,
-            persistent: currentSettings.isPairingTokenPersistent
-          }));
-        }
-        if (changed) {
-          addToast({
-            variant: 'warning',
-            isActionable: true,
-            message: (
-              <div className="flex flex-col gap-2">
-                <p>{t($ => $.app.extensionDisconnected)}</p>
-                <div className="flex gap-2 justify-end">
-                  <button
-                    type="button"
-                    className="app-button px-2 py-1 bg-surface-raised border border-border-color rounded"
-                    onClick={async () => {
-                      const token = useSettingsStore.getState().extensionPairingToken;
-                      try {
-                        if (token) {
-                          await navigator.clipboard.writeText(token);
-                        }
-                        acknowledgePairingTokenChange();
-                      } catch (error) {
-                        addToast({
-                          message: t($ => $.app.copyTokenFailed, { detail: String(error) }),
-                          variant: 'error',
-                          isActionable: true
-                        });
-                      }
-                    }}
-                  >
-                    {t($ => $.app.copyToken)}
-                  </button>
-                  <button
-                    type="button"
-                    className="app-button px-2 py-1 bg-surface-raised border border-border-color rounded"
-                    onClick={() => {
-                      const settings = useSettingsStore.getState();
-                      settings.setActiveSettingsTab('integrations');
-                      settings.setActiveView('settings');
-                      acknowledgePairingTokenChange();
-                    }}
-                  >
-                    {t($ => $.app.integrations)}
-                  </button>
-                </div>
-              </div>
-            )
-          });
-        }
-      } catch (error) {
-        console.error('Failed to hydrate extension pairing token:', error);
-        addToast({
-          message: t($ => $.app.credentialPersistenceFailed, { detail: String(error) }),
-          variant: 'error',
-          isActionable: true
-        });
-      }
-
      if (!active) return;
      setCoreReady(true);
    };
@@ -21,9 +21,15 @@ export const KeychainPermissionModal: React.FC<KeychainPermissionModalProps> = (
  const platform = usePlatformInfo();
  const [isGranting, setIsGranting] = useState(false);
  const [grantRequestPending, setGrantRequestPending] = useState(false);
+  const [grantRequestTimedOut, setGrantRequestTimedOut] = useState(false);
  const [error, setError] = useState<string | null>(null);
+  const isMountedRef = useRef(true);
  const grantRequestRef = useRef<Promise<PairingTokenHydration> | null>(null);

+  useEffect(() => () => {
+    isMountedRef.current = false;
+  }, []);
+
  useEffect(() => {
    if (!showKeychainModal || isGranting || grantRequestPending) return;
    const handleEscape = (event: KeyboardEvent) => {
@@ -68,6 +74,7 @@ export const KeychainPermissionModal: React.FC<KeychainPermissionModalProps> = (
    // launch a second OS prompt while the first one is still outstanding.
    if (grantRequestRef.current) return;
    setIsGranting(true);
+    setGrantRequestTimedOut(false);
    setError(null);

    let timeoutId: number | undefined;
@@ -95,11 +102,17 @@ export const KeychainPermissionModal: React.FC<KeychainPermissionModalProps> = (
    void grantRequest.then(
      () => {
        if (grantRequestRef.current === grantRequest) grantRequestRef.current = null;
-        setGrantRequestPending(false);
+        if (isMountedRef.current) {
+          setGrantRequestPending(false);
+          setGrantRequestTimedOut(false);
+        }
      },
      () => {
        if (grantRequestRef.current === grantRequest) grantRequestRef.current = null;
-        setGrantRequestPending(false);
+        if (isMountedRef.current) {
+          setGrantRequestPending(false);
+          setGrantRequestTimedOut(false);
+        }
      }
    );
    // A native credential-store call cannot be cancelled by the webview. Keep
@@ -112,19 +125,24 @@ export const KeychainPermissionModal: React.FC<KeychainPermissionModalProps> = (
        grantRequest,
        new Promise<never>((_, reject) => {
          timeoutId = window.setTimeout(
-            () => reject(new Error(t($ => $.keychain.timeout))),
+            () => {
+              if (isMountedRef.current) setGrantRequestTimedOut(true);
+              reject(new Error(t($ => $.keychain.timeout)));
+            },
            KEYCHAIN_GRANT_TIMEOUT_MS
          );
        })
      ]);
      if (!(await applyPersistentGrant(result))) {
-        setError(result.error || t($ => $.keychain.unavailable, { store: siteCredentialStoreName }));
+        if (isMountedRef.current) {
+          setError(result.error || t($ => $.keychain.unavailable, { store: siteCredentialStoreName }));
+        }
      }
    } catch (e: any) {
-      setError(e.toString());
+      if (isMountedRef.current) setError(e.toString());
    } finally {
      if (timeoutId !== undefined) window.clearTimeout(timeoutId);
-      setIsGranting(false);
+      if (isMountedRef.current) setIsGranting(false);
    }
  };

@@ -148,6 +166,7 @@ export const KeychainPermissionModal: React.FC<KeychainPermissionModalProps> = (
      }}
      role="dialog"
      aria-modal="true"
+      aria-labelledby="keychain-permission-title"
    >
      <div
        className="window-safe-modal bg-bg-modal rounded-xl w-full max-w-md overflow-hidden flex flex-col shadow-2xl border border-border-modal scale-in"
@@ -157,7 +176,7 @@ export const KeychainPermissionModal: React.FC<KeychainPermissionModalProps> = (
          <div className="p-2 bg-blue-500/10 rounded-full items-center justify-center">
            <KeyRound size={20} className="text-blue-500" />
          </div>
-          <h2 className="text-lg font-semibold text-text-primary m-0">{t($ => $.keychain.title)}</h2>
+          <h2 id="keychain-permission-title" className="text-lg font-semibold text-text-primary m-0">{t($ => $.keychain.title)}</h2>
        </div>

        <div className="px-5 py-6 flex-1 min-h-0 overflow-y-auto text-sm text-text-secondary leading-relaxed space-y-4">
@@ -198,16 +217,18 @@ export const KeychainPermissionModal: React.FC<KeychainPermissionModalProps> = (

        <div className="px-5 py-4 border-t border-border-modal flex justify-end gap-3 bg-bg-modal-accent">
          <button
+            type="button"
            onClick={handleLater}
-            disabled={isGranting || grantRequestPending}
-            className="px-4 py-2 rounded-lg text-sm font-medium transition-colors text-text-secondary hover:bg-item-hover hover:text-text-primary disabled:opacity-50"
+            disabled={isGranting || (grantRequestPending && !grantRequestTimedOut)}
+            className="keychain-modal-action px-4 py-2 rounded-lg text-sm font-medium text-text-secondary hover:bg-item-hover hover:text-text-primary disabled:opacity-50"
          >
            {t($ => $.keychain.later)}
          </button>
          <button
+            type="button"
            onClick={handleGrant}
            disabled={isGranting || grantRequestPending}
-            className="px-4 py-2 rounded-lg text-sm font-medium transition-colors bg-accent text-accent-foreground hover:bg-accent/90 disabled:opacity-50"
+            className="keychain-modal-action px-4 py-2 rounded-lg text-sm font-medium bg-accent text-accent-foreground hover:bg-accent/90 disabled:opacity-50"
          >
            {isGranting || grantRequestPending ? t($ => $.keychain.enabling) : grantLabel}
          </button>
@@ -25,7 +25,7 @@
  --surface-overlay: 0 0% 100% / 0.95;
  --shadow-color: 220 10% 20% / 0.1;
  --sidebar-shell-bg: 0 0% 92%;
-  --sidebar-panel-bg: 0 0% 93%;
+  --sidebar-panel-bg: 0 0% 96%;
  --workspace-bg: 0 0% 98%;
  --statusbar-bg: 0 0% 96%;
  --status-completed: 136 62% 48%;
@@ -69,7 +69,7 @@
  --surface-overlay: 0 0% 100% / 0.95;
  --shadow-color: 220 10% 20% / 0.1;
  --sidebar-shell-bg: 0 0% 92%;
-  --sidebar-panel-bg: 0 0% 93%;
+  --sidebar-panel-bg: 0 0% 96%;
  --workspace-bg: 0 0% 98%;
  --statusbar-bg: 0 0% 96%;
  --sidebar-border: 0 0% 0% / 0.18;
@@ -103,7 +103,7 @@
  --status-queued: 38 95% 55%;
  --status-retrying: 50 100% 60%;
  --sidebar-shell-bg: 0 0% 11%;
-  --sidebar-panel-bg: 0 0% 13%;
+  --sidebar-panel-bg: 0 0% 11%;
  --workspace-bg: 0 0% 11%;
  --statusbar-bg: 0 0% 11%;
  --sidebar-border: 0 0% 100% / 0.11;
@@ -148,7 +148,7 @@
  --status-queued: 38 95% 60%;
  --status-retrying: 50 100% 65%;
  --sidebar-shell-bg: 231 15% 15%;
-  --sidebar-panel-bg: 232 14% 23%;
+  --sidebar-panel-bg: 231 15% 16%;
  --workspace-bg: 231 15% 18%;
  --statusbar-bg: 231 15% 16%;
  --sidebar-border: 326 100% 74% / 0.16;
@@ -193,7 +193,7 @@
  --status-queued: 40 71% 60%;
  --status-retrying: 50 85% 55%;
  --sidebar-shell-bg: 220 18% 18%;
-  --sidebar-panel-bg: 220 17% 27%;
+  --sidebar-panel-bg: 220 18% 19%;
  --workspace-bg: 220 16% 22%;
  --statusbar-bg: 220 18% 19%;
  --sidebar-border: 193 43% 67% / 0.16;
@@ -531,6 +531,35 @@ html[data-list-density="relaxed"] {
    transform: scale(0.96);
  }

+  .keychain-modal-action {
+    transition:
+      background-color 100ms ease,
+      border-color 100ms ease,
+      box-shadow 100ms ease,
+      color 100ms ease,
+      transform 70ms ease,
+      opacity 100ms ease;
+  }
+
+  .keychain-modal-action:active:not(:disabled) {
+    transform: scale(0.96);
+  }
+
+  .keychain-modal-action:focus-visible {
+    outline: 2px solid hsl(var(--accent-color) / 0.7);
+    outline-offset: 2px;
+  }
+
+  @media (prefers-reduced-motion: reduce) {
+    .keychain-modal-action {
+      transition: none;
+    }
+
+    .keychain-modal-action:active:not(:disabled) {
+      transform: none;
+    }
+  }
+
  .app-button-primary {
    border-color: transparent;
    background: var(--color-accent);
@@ -1488,6 +1517,11 @@ html[data-list-density="relaxed"] {
    margin-bottom: 0;
  }

+  .sidebar-settings-button:not([data-active="true"]):hover {
+    background: hsl(var(--sidebar-hover));
+    color: hsl(var(--text-primary));
+  }
+
  .sidebar-section-label {
    padding: 0 12px;
    margin-bottom: 7px;