fix: refuse to keep a sign-in Linux would store in plain text

README.md

@@ -126,5 +126,16 @@ nobody notices, but it is still the UI thread doing it, and the file only grows.
 the officer is the one who decides a session was the guild's raid. An "always send
 finished nights for this guild" setting is reasonable once the matching has earned trust.
 
-**Linux `basic_text` detection.** `safeStorage` can silently fall back to plaintext on
-Linux; `canPersist()` reports availability but not which backend answered.
+## Where the sign-in is kept
+
+`safeStorage` — DPAPI on Windows, Keychain on macOS, libsecret or KWallet on Linux. When
+no real store is available the token is not written at all; the officer signs in each
+launch instead.
+
+The Linux case is the one worth stating. With no keyring daemon running, Electron falls
+back to a backend named `basic_text` that scrambles with a key hardcoded in Chromium —
+recoverable by anyone who can read the file — and `isEncryptionAvailable()` still answers
+**true** for it. So availability alone is not the check: on Linux the selected backend is
+read as well, `basic_text` counts as no store, and a credentials file left by an earlier
+build that trusted the flag is deleted on the next launch rather than left lying around.
+The setup panel says which daemon to start.

src/main/index.ts

@@ -3,7 +3,7 @@ import { app, BrowserWindow, dialog, ipcMain, shell } from 'electron';
 import { ApiClient } from './api';
 import { buildAuthorizeUrl, createPkcePair, LoopbackReceiver } from './auth';
 import { autoDetect, findSavedVariables } from './discovery';
-import { canPersist, clearToken, loadToken, saveToken } from './secrets';
+import { canPersist, clearToken, loadToken, persistenceBlocker, saveToken } from './secrets';
 import { readNights } from './savedVariables';
 import { loadSettings, rememberUpload, saveSettings } from './settings';
 import { nightKey } from '../shared/nightKey';
@@ -208,6 +208,9 @@ async function runSmokeCheck(target: BrowserWindow): Promise<void> {
 ipcMain.handle('app:info', () => ({
   version: CLIENT_VERSION,
   canRememberSignIn: canPersist(),
+  // Why not, when not. "No secure credential store" is accurate and unhelpful on a Linux
+  // desktop whose only missing piece is a keyring daemon nobody has had to think about.
+  signInMemoryBlocker: persistenceBlocker(),
   platform: process.platform,
 }));
 

src/main/secrets.test.ts

@@ -0,0 +1,164 @@
+import { existsSync, mkdtempSync, readFileSync, writeFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
+
+/**
+ * Token storage, and specifically the Linux case that cannot be checked by hand from a
+ * Windows or macOS machine.
+ *
+ * `safeStorage.isEncryptionAvailable()` answers **true** on a Linux desktop with no
+ * keyring, where Electron falls back to a backend called `basic_text` that scrambles with
+ * a key hardcoded in Chromium. Trusting that answer alone meant writing a working API
+ * token to disk in effectively plain text while telling the officer it was stored
+ * securely — the exact downgrade the module's own comment says it refuses to perform.
+ *
+ * Nothing here talks to a real credential store; the point is the decision made from what
+ * one reports.
+ */
+
+const userData = mkdtempSync(join(tmpdir(), 'raidify-secrets-'));
+const FILE = join(userData, 'credentials.bin');
+
+const state = {
+  available: true,
+  backend: 'gnome_libsecret' as string | undefined,
+  platform: 'linux' as string,
+};
+
+vi.mock('electron', () => ({
+  app: { getPath: () => userData },
+  safeStorage: {
+    isEncryptionAvailable: () => state.available,
+    getSelectedStorageBackend: () => {
+      if (state.backend === undefined) throw new Error('not supported on this platform');
+      return state.backend;
+    },
+    // Stand-ins. Reversing the string is enough to prove which bytes were written and
+    // that a round trip happened; the real implementation is the OS's problem.
+    encryptString: (s: string) => Buffer.from(`enc:${s}`),
+    decryptString: (b: Buffer) => b.toString().replace(/^enc:/, ''),
+  },
+}));
+
+const { canPersist, persistenceBlocker, saveToken, loadToken, clearToken } = await import(
+  './secrets'
+);
+
+const realPlatform = process.platform;
+
+function setPlatform(value: string) {
+  Object.defineProperty(process, 'platform', { value, configurable: true });
+}
+
+beforeEach(() => {
+  state.available = true;
+  state.backend = 'gnome_libsecret';
+  setPlatform('linux');
+  clearToken();
+});
+
+afterEach(() => {
+  Object.defineProperty(process, 'platform', { value: realPlatform, configurable: true });
+});
+
+describe('a machine with a real credential store', () => {
+  it('keeps the sign-in and gives it back', () => {
+    saveToken('tok-abc');
+
+    expect(loadToken()).toBe('tok-abc');
+    expect(canPersist()).toBe(true);
+    expect(persistenceBlocker()).toBeNull();
+  });
+
+  it('writes something other than the bare token', () => {
+    saveToken('tok-abc');
+
+    expect(readFileSync(FILE).toString()).not.toBe('tok-abc');
+  });
+});
+
+describe('a Linux desktop with no keyring running', () => {
+  beforeEach(() => {
+    // What Electron actually reports there: encryption "available", backend that isn't.
+    state.available = true;
+    state.backend = 'basic_text';
+  });
+
+  it('refuses to keep the sign-in', () => {
+    expect(canPersist()).toBe(false);
+  });
+
+  /**
+   * The refusal has to be the write refusing, not just the flag reading false. A caller
+   * that skipped the flag would otherwise put the token on disk anyway.
+   */
+  it('refuses to write the token at all', () => {
+    expect(() => saveToken('tok-abc')).toThrow();
+    expect(existsSync(FILE)).toBe(false);
+  });
+
+  /**
+   * A file left by a build that trusted isEncryptionAvailable() alone. Declining to read
+   * it while leaving it there is the worst outcome: the officer signs in again and the
+   * recoverable copy stays on disk indefinitely.
+   */
+  it('deletes a token an earlier build stored unprotected', () => {
+    writeFileSync(FILE, 'enc:tok-from-old-build');
+
+    expect(loadToken()).toBeNull();
+    expect(existsSync(FILE)).toBe(false);
+  });
+
+  /**
+   * "No secure credential store" is true and useless here — the desktop looks perfectly
+   * normal and the missing piece is a daemon. The message has to name it.
+   */
+  it('says what to start', () => {
+    const blocker = persistenceBlocker();
+
+    expect(blocker).toBeTruthy();
+    expect(blocker).toMatch(/keyring|KWallet/i);
+  });
+});
+
+describe('a machine with no encryption at all', () => {
+  beforeEach(() => {
+    state.available = false;
+    state.backend = 'basic_text';
+  });
+
+  it('refuses, and says so without inventing a Linux fix', () => {
+    expect(canPersist()).toBe(false);
+    expect(persistenceBlocker()).toMatch(/no secure credential store/i);
+  });
+});
+
+describe('Windows and macOS', () => {
+  /**
+   * Neither has a backend to choose, and `getSelectedStorageBackend` is Linux-only —
+   * older Electron does not define it at all. Calling it anywhere else must not be able
+   * to break sign-in.
+   */
+  it('never consults the Linux backend, even when asking would throw', () => {
+    setPlatform('win32');
+    state.backend = undefined;
+
+    expect(canPersist()).toBe(true);
+    expect(persistenceBlocker()).toBeNull();
+
+    saveToken('tok-win');
+    expect(loadToken()).toBe('tok-win');
+  });
+
+  /**
+   * `basic_text` is a Linux backend name. If the platform check were dropped, a stray
+   * answer on Windows would start refusing DPAPI — which works fine.
+   */
+  it('is not refused by a Linux backend name', () => {
+    setPlatform('darwin');
+    state.backend = 'basic_text';
+
+    expect(canPersist()).toBe(true);
+  });
+});

src/main/secrets.ts

@@ -16,25 +16,84 @@ import { app, safeStorage } from 'electron';
 
 const FILE = () => join(app.getPath('userData'), 'credentials.bin');
 
+/**
+ * Linux desktops without a running keyring.
+ *
+ * Electron falls back to a backend it calls `basic_text`, which is not encryption: it
+ * scrambles with a key hardcoded in Chromium's source, so anything written with it is
+ * recoverable by anyone who can read the file. `isEncryptionAvailable()` still answers
+ * **true** for it.
+ *
+ * That answer was the whole check, so on a machine with no gnome-keyring or KWallet this
+ * module did exactly what the comment above says it refuses to do — write a working API
+ * token to disk in effectively plain text, while telling the officer their sign-in was
+ * stored securely. The token uploads attendance for their guild.
+ */
+const INSECURE_BACKEND = 'basic_text';
+
+/**
+ * Which OS credential store is actually behind `safeStorage`, on the one platform where
+ * the answer varies. Windows (DPAPI) and macOS (Keychain) have nothing to choose.
+ */
+function selectedBackend(): string | null {
+  if (process.platform !== 'linux') return null;
+  // Guarded: the method is Linux-only and absent on older Electron, and a crash here
+  // would take out sign-in entirely.
+  if (typeof safeStorage.getSelectedStorageBackend !== 'function') return null;
+
+  try {
+    return safeStorage.getSelectedStorageBackend();
+  } catch {
+    return null;
+  }
+}
+
 export function canPersist(): boolean {
-  return safeStorage.isEncryptionAvailable();
+  if (!safeStorage.isEncryptionAvailable()) return false;
+  return selectedBackend() !== INSECURE_BACKEND;
 }
 
-export function saveToken(token: string): void {
-  if (!canPersist()) {
-    throw new Error(
-      'This system has no secure credential store available, so the sign-in cannot be remembered.',
-    );
+/**
+ * Why the sign-in cannot be kept, in words that name the fix.
+ *
+ * "No secure credential store" is true and useless on Linux: the officer has a desktop
+ * that looks perfectly normal, and the missing piece is a keyring daemon they have never
+ * had to think about. Null when persistence works.
+ */
+export function persistenceBlocker(): string | null {
+  if (canPersist()) return null;
+
+  // Availability first. When there is no encryption at all, the backend can still report
+  // `basic_text` — and telling someone to start a keyring daemon when the real problem is
+  // that nothing is available sends them off fixing the wrong thing.
+  if (safeStorage.isEncryptionAvailable() && selectedBackend() === INSECURE_BACKEND) {
+    return 'No keyring is running, so the only store available would keep your sign-in in plain text. Start gnome-keyring or KWallet and sign in again to stay signed in.';
   }
 
+  return 'This system has no secure credential store, so the sign-in cannot be remembered between launches.';
+}
+
+export function saveToken(token: string): void {
+  const blocker = persistenceBlocker();
+  if (blocker) throw new Error(blocker);
+
   const path = FILE();
   mkdirSync(dirname(path), { recursive: true });
   writeFileSync(path, safeStorage.encryptString(token), { mode: 0o600 });
 }
 
 export function loadToken(): string | null {
   const path = FILE();
-  if (!existsSync(path) || !canPersist()) return null;
+  if (!existsSync(path)) return null;
+
+  if (!canPersist()) {
+    // A file written by a build that trusted `isEncryptionAvailable()` alone, on a machine
+    // where that answer was wrong. Refusing to read it while leaving it on disk would be
+    // the worst of both: the officer signs in again, and the recoverable copy stays there
+    // forever. Delete it and make them sign in.
+    clearToken();
+    return null;
+  }
 
   try {
     return safeStorage.decryptString(readFileSync(path));

src/preload/index.ts

@@ -14,7 +14,12 @@ import type {
  * describing something the officer asked for.
  */
 const bridge = {
-  appInfo: (): Promise<{ version: string; canRememberSignIn: boolean; platform: string }> =>
+  appInfo: (): Promise<{
+    version: string;
+    canRememberSignIn: boolean;
+    signInMemoryBlocker: string | null;
+    platform: string;
+  }> =>
     ipcRenderer.invoke('app:info'),
 
   checkCompat: (): Promise<CompatVerdict> => ipcRenderer.invoke('compat:check'),

src/renderer/App.tsx

@@ -9,6 +9,7 @@ import { StatusBanner } from './components/StatusBanner';
 interface AppInfo {
   version: string;
   canRememberSignIn: boolean;
+  signInMemoryBlocker: string | null;
   platform: string;
 }
 
@@ -116,6 +117,7 @@ export function App() {
           settings={settings}
           signedIn={signedIn}
           canRememberSignIn={info?.canRememberSignIn ?? true}
+          signInMemoryBlocker={info?.signInMemoryBlocker ?? null}
           guilds={guilds}
           installs={installs}
           watchingPath={watchingPath}

src/renderer/components/SetupPanel.tsx

@@ -16,6 +16,7 @@ export function SetupPanel({
   settings,
   signedIn,
   canRememberSignIn,
+  signInMemoryBlocker,
   guilds,
   installs,
   watchingPath,
@@ -29,6 +30,7 @@ export function SetupPanel({
   settings: Settings | null;
   signedIn: boolean;
   canRememberSignIn: boolean;
+  signInMemoryBlocker: string | null;
   guilds: CompanionGuild[] | null;
   installs: SavedVariablesCandidate[] | null;
   watchingPath: string | null;
@@ -120,8 +122,8 @@ export function SetupPanel({
               </p>
               {!canRememberSignIn && (
                 <p className="mt-2 text-xs text-[var(--warning)]">
-                  This system has no secure credential store, so the sign-in cannot be
-                  remembered between launches.
+                  {signInMemoryBlocker ??
+                    'This system has no secure credential store, so the sign-in cannot be remembered between launches.'}
                 </p>
               )}
             </>