fix(encryption): deprecate encryption toggle; fix mobile chat failure on desktop-encrypted keys (#2446)

* fix(mobile): handle desktop-encrypted API keys on mobile (follow-up to #2443)

PR #2443 added the Buffer polyfill to base64.ts, utils.ts, and
BedrockChatModel.ts but missed src/encryptionService.ts:119. My
original write-up incorrectly claimed mobile never enters the
DESKTOP_PREFIX branch of getDecryptedKey — it does, whenever a user
encrypts their API keys on desktop and then syncs the vault to mobile
via Obsidian Sync. Mobile reads the same encrypted blob, enters the
DESKTOP_PREFIX branch, and hits `Buffer.from(...)`, which throws
"Can't find variable: Buffer" on iOS WebKit.

User reproducer: "Model request failed: Can't find variable: Buffer"
during normal chat send on mobile after PR #2443 merged.

Two-part fix:

1. Add `import { Buffer } from "buffer"` (the npm polyfill, already in
   dependencies and used in PR #2443) so the literal ReferenceError is
   gone. The legacy ENCRYPTION_PREFIX branch at line 137 also benefits.

2. Even with Buffer available, the next line
   `getSafeStorage()!.decryptString(buffer)` cannot succeed on mobile
   because there is no Electron safeStorage there. The encrypted blob
   uses OS-level encryption (Keychain on macOS, DPAPI on Windows,
   libsecret on Linux) that cannot be replayed from mobile. Add a
   Platform.isMobile guard that throws a clear, actionable error
   asking the user to re-enter their API key on this device instead
   of crashing on the unreachable decryption call.

Out of scope:

- The DESKTOP_PREFIX-on-mobile situation requires the user to re-save
  their key in mobile settings (which will re-encrypt with
  WEBCRYPTO_PREFIX, the cross-platform path that Web Crypto handles).
  Surfacing a clear error is the right user experience until the
  plugin auto-migrates on first mobile launch — that's a larger
  follow-up not appropriate for a same-day hotfix.

- The legacy ENCRYPTION_PREFIX branch at line 137 still references
  Buffer, but it's already gated by
  `getSafeStorage()?.isEncryptionAvailable()` which returns false on
  mobile, so mobile never enters it. The new import covers it
  defensively if that gate ever changes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(mobile): list desktop-encrypted keys on mobile load + block desktop encryption

Two additions on top of the existing fix in this PR:

1. **Guard the encryption side with Platform.isDesktop.** Mobile should
   NEVER produce DESKTOP_PREFIX keys, even if `require("electron")`
   happens to return a truthy shim. Before this change, the only gate
   was `getSafeStorage()?.isEncryptionAvailable()` — fragile, since
   any mobile environment that exposes a partial Electron-like shim
   could slip through and save a key as DESKTOP_PREFIX that the same
   device then can't decrypt.

2. **Surface a Notice on mobile load** listing every key field that's
   still DESKTOP_PREFIX-encrypted. Without this, the user only finds
   out which key needs re-entry by hitting a chat error and having to
   guess. With it, they see the full list at startup:

     "Copilot: 3 API key(s) encrypted on desktop and unusable on
      mobile. Re-enter in Copilot settings: plusLicenseKey,
      openAIApiKey, activeModels[gpt-5.5]"

   Notice has timeout 0 so it stays until dismissed.

The new exported `findDesktopEncryptedKeyFields(settings)` walks:
- All top-level string settings (catches plusLicenseKey,
  openAIApiKey, anthropicApiKey, githubCopilotToken, etc.)
- settings.activeModels[*].apiKey (per-model custom keys)
- settings.activeEmbeddingModels[*].apiKey

The exact field names returned match the keys in data.json so the
user can grep / inspect / re-enter them confidently.

This addresses the user report after the first commit in this PR:
"despite reentering api key on mobile" — which happened because they
re-entered one key but a *different* key (likely plusLicenseKey or
a model-specific apiKey) was still DESKTOP_PREFIX, causing the chat
to fail on that other key.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(encryption): deprecate encryption-at-rest; make encrypt/save a no-op

The encryption toggle has created repeated incidents:

1. Mobile cannot decrypt DESKTOP_PREFIX (Electron safeStorage) keys
   that synced over from desktop, because mobile has no safeStorage.
2. The WebCrypto path is security theater — the AES-GCM key is a
   hardcoded "obsidian-copilot-v1" string, so anyone with data.json
   can decrypt every "encrypted" key in seconds.
3. PRs #2401 and #2403 (3.3.0) introduced bare `Buffer.from` in the
   encryption + base64 paths, which crashed mobile WebView with
   "Can't find variable: Buffer" — see #2443 and earlier commits in
   this PR.
4. Even with that fixed, the eager dispatch-map builders in
   chatModelManager / embeddingManager await getDecryptedKey for
   every provider; a throw on the one bad desktop-encrypted key
   killed chats that used a different provider with a valid key.

Rather than continue patching, deprecate the toggle and stop adding
encryption to new keys. Existing encrypted blobs in data.json remain
readable via getDecryptedKey (for all three legacy formats:
DESKTOP_PREFIX, WEBCRYPTO_PREFIX, and the original `enc_`); they
migrate organically to plaintext the next time the user re-saves any
given key.

Changes:

- `encryptAllKeys` → no-op (returns settings unchanged). 6+ lines of
  per-field encryption gone.
- `getEncryptedKey` → no-op (returns plaintext, strips `dec_` marker
  if present). 25 lines of safeStorage / WebCrypto / DESKTOP_PREFIX
  branching gone.
- `main.ts` settings-save → unconditionally `saveData(next)` instead
  of branching on `enableEncryption`. The setting field stays in the
  CopilotSettings interface for backwards-compat with existing
  data.json files but is no longer read or written.
- `AdvancedSettings.tsx` → "Enable Encryption" toggle removed from
  the UI. Users who had it on get no further encryption; users who
  had it off see no change.

Mobile-specific path inside `getDecryptedKey`:

- DESKTOP_PREFIX keys on mobile return `DESKTOP_KEY_ON_MOBILE_SENTINEL`
  (no longer throw). chatModelManager / embeddingManager's eager
  dispatch-map construction completes; the sentinel sits harmlessly
  in unused config slots; providers actually being used with valid
  plaintext keys work. The startup Notice (already in this PR)
  surfaces which fields the user must re-enter.

Tests:

- `getEncryptedKey` test suite rewritten to verify the new no-op
  behavior (plaintext passthrough, dec_ stripping, empty-string).
- `encryptAllKeys` test suite shrunk to one identity-return assertion.
- Cross-platform encrypt/decrypt round-trip tests removed (encryption
  is gone). Added a legacy WebCrypto decrypt test to confirm we can
  still read existing enc_web_* blobs.
- 1974 tests pass (was 1975; -1 for the dropped round-trip test).

Net change: encryptionService.ts down from 207 to 130 lines.

Risk:

- `data.json` now contains plaintext API keys after first re-save.
  Worth saying explicitly in release notes. The previous "encryption"
  was indistinguishable from plaintext for anyone with filesystem
  access (hardcoded WebCrypto key), so this is a documentation
  change more than a security change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

* fix(mobile): simplify to a throw; drop sentinel + Notice + helper

User feedback: the sentinel + startup-Notice approach added 70+ lines of
transitional code that we'd have to delete later. Replace with the
lightest viable shape.

- Mobile DESKTOP_PREFIX branch in getDecryptedKey now throws a generic
  but actionable error ("re-enter your API keys on this device") and
  nothing else.
- Removed DESKTOP_KEY_ON_MOBILE_SENTINEL constant.
- Removed findDesktopEncryptedKeyFields function.
- Removed the startup Notice block in main.ts and its import.

Trade-off accepted: with multiple desktop-encrypted keys present, the
chat fails on the first one the eager dispatch-map encounters; user
re-enters that key, retries, hits the next bad key, repeats. The
generic message tells them what to do; they iterate rather than seeing
the full list up front.

Net: -71 lines vs. previous approach. Future cleanup (when no users
have desktop-encrypted keys anymore) is one small if-branch in
getDecryptedKey instead of a sentinel + assert + helper + Notice
spread across four files.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Logan Yang 2026-05-14 00:47:59 -07:00 committed by GitHub
parent dd74cf9654
commit 249f9e2472
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 70 additions and 157 deletions

View file

@ -69,21 +69,21 @@ describe("EncryptionService", () => {
jest.resetModules();
});
describe("getEncryptedKey", () => {
it("should encrypt an API key", async () => {
describe("getEncryptedKey (deprecated no-op)", () => {
it("should return plaintext key unchanged", async () => {
const apiKey = "testApiKey";
const encryptedKey = await getEncryptedKey(apiKey);
// The key is base64 encoded, so we should expect that format
expect(encryptedKey).toMatch(/^enc_(desk|web)_[A-Za-z0-9+/=]+$/);
// Verify we can decrypt it back
const decryptedKey = await getDecryptedKey(encryptedKey);
expect(decryptedKey).toBe(apiKey);
const result = await getEncryptedKey(apiKey);
expect(result).toBe(apiKey);
});
it("should return the original key if already encrypted", async () => {
const apiKey = "enc_testApiKey";
const encryptedKey = await getEncryptedKey(apiKey);
expect(encryptedKey).toBe(apiKey);
it("should strip the dec_ prefix when present", async () => {
const result = await getEncryptedKey("dec_testApiKey");
expect(result).toBe("testApiKey");
});
it("should return empty string for empty input", async () => {
const result = await getEncryptedKey("");
expect(result).toBe("");
});
});
@ -102,8 +102,8 @@ describe("EncryptionService", () => {
});
});
describe("encryptAllKeys", () => {
it("should encrypt all keys containing 'apikey'", async () => {
describe("encryptAllKeys (deprecated no-op)", () => {
it("should return settings unchanged regardless of enableEncryption", async () => {
const settings = {
enableEncryption: true,
openAIApiKey: "testApiKey",
@ -111,28 +111,8 @@ describe("EncryptionService", () => {
userSystemPrompt: "shouldBeIgnored",
} as unknown as CopilotSettings;
const newSettings = await encryptAllKeys(settings);
expect(newSettings.openAIApiKey).toMatch(/^enc_(desk|web)_[A-Za-z0-9+/=]+$/);
expect(newSettings.cohereApiKey).toMatch(/^enc_(desk|web)_[A-Za-z0-9+/=]+$/);
expect(newSettings.userSystemPrompt).toBe("shouldBeIgnored");
// Verify we can decrypt the keys back
const decryptedOpenAI = await getDecryptedKey(newSettings.openAIApiKey);
const decryptedCohere = await getDecryptedKey(newSettings.cohereApiKey);
expect(decryptedOpenAI).toBe("testApiKey");
expect(decryptedCohere).toBe("anotherTestApiKey");
});
it("should not encrypt keys when encryption is not enabled", async () => {
const newSettings = await encryptAllKeys({
enableEncryption: false,
openAIApiKey: "testApiKey",
cohereApiKey: "anotherTestApiKey",
userSystemPrompt: "shouldBeIgnored",
} as unknown as CopilotSettings);
expect(newSettings.openAIApiKey).toBe("testApiKey");
expect(newSettings.cohereApiKey).toBe("anotherTestApiKey");
expect(newSettings.userSystemPrompt).toBe("shouldBeIgnored");
const result = await encryptAllKeys(settings);
expect(result).toBe(settings);
});
});
});
@ -153,41 +133,23 @@ describe("Cross-platform compatibility", () => {
console.error = originalConsoleError;
});
it("should encrypt and decrypt consistently on mobile", async () => {
// Mock as mobile by making safeStorage unavailable
mockElectron.remote.safeStorage.isEncryptionAvailable.mockReturnValue(false);
it("should decrypt legacy WebCrypto-encrypted keys on mobile", async () => {
// Simulate a key encrypted with the legacy WebCrypto path (the only path
// mobile ever used, and the path desktop fell back to when safeStorage
// wasn't available). The mocked subtle.decrypt strips the `_encrypted`
// suffix the encrypt mock appends.
const enc = (await mockSubtle.encrypt(
null,
null,
new TextEncoder().encode("testApiKey").buffer
)) as ArrayBuffer;
const encBytes = new Uint8Array(enc);
let binary = "";
for (let i = 0; i < encBytes.length; i++) binary += String.fromCharCode(encBytes[i]);
const webEncryptedKey = "enc_web_" + window.btoa(binary);
const originalKey = "testApiKey";
const encryptedKey = await getEncryptedKey(originalKey);
expect(encryptedKey).toMatch(/^enc_(desk|web)_[A-Za-z0-9+/=]+$/);
// Reset the mock counts before decryption
mockSubtle.encrypt.mockClear();
mockSubtle.decrypt.mockClear();
const decryptedKey = await getDecryptedKey(encryptedKey);
expect(decryptedKey).toBe(originalKey);
// On mobile, we should use Web Crypto API for decryption
const decryptedKey = await getDecryptedKey(webEncryptedKey);
expect(decryptedKey).toBe("testApiKey");
expect(mockSubtle.decrypt).toHaveBeenCalled();
});
it("should be able to decrypt mobile-encrypted keys on desktop", async () => {
// First encrypt on mobile
mockElectron.remote.safeStorage.isEncryptionAvailable.mockReturnValue(false);
const originalKey = "testApiKey";
const mobileEncryptedKey = await getEncryptedKey(originalKey);
expect(mobileEncryptedKey).toMatch(/^enc_(desk|web)_[A-Za-z0-9+/=]+$/);
expect(mockSubtle.encrypt).toHaveBeenCalled();
// Reset the mock counts before desktop decryption
mockSubtle.encrypt.mockClear();
mockSubtle.decrypt.mockClear();
// Then decrypt on desktop
mockElectron.remote.safeStorage.isEncryptionAvailable.mockReturnValue(true);
const decryptedKey = await getDecryptedKey(mobileEncryptedKey);
expect(decryptedKey).toBe(originalKey);
});
});

View file

@ -1,3 +1,10 @@
// Reason: `buffer` is the npm polyfill (browser-compatible), bundled by
// esbuild so the same Buffer code path works on desktop (Electron) and
// mobile (WebView). Without this import, bare `Buffer` is undefined on
// mobile and throws "Can't find variable: Buffer" — see PR #2443.
// eslint-disable-next-line import/no-nodejs-modules
import { Buffer } from "buffer";
import { type CopilotSettings } from "@/settings/model";
import { Platform } from "obsidian";
@ -19,7 +26,11 @@ function getSafeStorage(): SafeStorage | null {
return safeStorageInternal;
}
// Add new prefixes to distinguish encryption methods
// Encryption-at-rest is deprecated as of this commit. `getEncryptedKey`
// and `encryptAllKeys` are now no-ops, so new keys are saved plaintext.
// We still decrypt legacy `enc_*` blobs at read time via `getDecryptedKey`
// for backwards compatibility; existing keys migrate organically the next
// time the user re-saves them.
const DESKTOP_PREFIX = "enc_desk_";
const WEBCRYPTO_PREFIX = "enc_web_";
// Keep old prefix for backward compatibility
@ -37,72 +48,25 @@ async function getEncryptionKey(): Promise<CryptoKey> {
]);
}
/**
* No-op: encryption-at-rest is deprecated. Returns settings unchanged.
* Existing encrypted keys (`enc_*`) stay in data.json and are decrypted at
* read time via getDecryptedKey for backwards compatibility. New keys are
* saved plaintext.
*/
export async function encryptAllKeys(
settings: Readonly<CopilotSettings>
): Promise<Readonly<CopilotSettings>> {
if (!settings.enableEncryption) {
return settings;
}
const newSettings = { ...settings };
const keysToEncrypt = Object.keys(settings).filter(
(key) =>
key.toLowerCase().includes("apikey") ||
key === "plusLicenseKey" ||
key === "githubCopilotAccessToken" ||
key === "githubCopilotToken"
);
for (const key of keysToEncrypt) {
const apiKey = settings[key as keyof CopilotSettings] as string;
(newSettings[key as keyof CopilotSettings] as any) = await getEncryptedKey(apiKey);
}
if (Array.isArray(settings.activeModels)) {
newSettings.activeModels = await Promise.all(
settings.activeModels.map(async (model) => ({
...model,
apiKey: await getEncryptedKey(model.apiKey || ""),
}))
);
}
if (Array.isArray(settings.activeEmbeddingModels)) {
newSettings.activeEmbeddingModels = await Promise.all(
settings.activeEmbeddingModels.map(async (model) => ({
...model,
apiKey: await getEncryptedKey(model.apiKey || ""),
}))
);
}
return newSettings;
return settings;
}
/**
* No-op: encryption-at-rest is deprecated. Strips the `dec_` marker if
* present and returns the raw plaintext key.
*/
export async function getEncryptedKey(apiKey: string): Promise<string> {
if (!apiKey || apiKey.startsWith(ENCRYPTION_PREFIX)) {
return apiKey;
}
if (isDecrypted(apiKey)) {
apiKey = apiKey.replace(DECRYPTION_PREFIX, "");
}
try {
// Try desktop encryption first
if (getSafeStorage()?.isEncryptionAvailable()) {
const encryptedBuffer = getSafeStorage()!.encryptString(apiKey);
return DESKTOP_PREFIX + encryptedBuffer.toString("base64");
}
// Fallback to Web Crypto API
const key = await getEncryptionKey();
const encodedData = new TextEncoder().encode(apiKey);
const encryptedData = await crypto.subtle.encrypt(ALGORITHM, key, encodedData);
return WEBCRYPTO_PREFIX + arrayBufferToBase64(encryptedData);
} catch (error) {
console.error("Encryption failed:", error);
return apiKey;
}
if (!apiKey) return apiKey;
return isDecrypted(apiKey) ? apiKey.replace(DECRYPTION_PREFIX, "") : apiKey;
}
export async function getDecryptedKey(apiKey: string): Promise<string> {
@ -113,8 +77,16 @@ export async function getDecryptedKey(apiKey: string): Promise<string> {
return apiKey.replace(DECRYPTION_PREFIX, "");
}
// Handle different encryption methods
// Handle different encryption methods (legacy data only — new keys are
// saved plaintext per the deprecation above).
if (apiKey.startsWith(DESKTOP_PREFIX)) {
if (Platform.isMobile) {
throw new Error(
"An API key was encrypted on desktop with OS-level encryption that's " +
"unavailable on mobile. Please re-enter your API keys in Copilot " +
"settings on this device."
);
}
const base64Data = apiKey.replace(DESKTOP_PREFIX, "");
const buffer = Buffer.from(base64Data, "base64");
return getSafeStorage()!.decryptString(buffer);
@ -162,15 +134,6 @@ function isDecrypted(keyBuffer: string): boolean {
return keyBuffer.startsWith(DECRYPTION_PREFIX);
}
function arrayBufferToBase64(buffer: ArrayBuffer): string {
const bytes = new Uint8Array(buffer);
let binary = "";
for (let i = 0; i < bytes.byteLength; i++) {
binary += String.fromCharCode(bytes[i]);
}
return window.btoa(binary);
}
function base64ToArrayBuffer(base64: string): ArrayBuffer {
const binaryString = window.atob(base64);
const bytes = new Uint8Array(binaryString.length);

View file

@ -21,7 +21,6 @@ import { ProjectRegister } from "@/projects/projectRegister";
import { ABORT_REASON, CHAT_VIEWTYPE, DEFAULT_OPEN_AREA, EVENT_NAMES } from "@/constants";
import { ChatManager } from "@/core/ChatManager";
import { MessageRepository } from "@/core/MessageRepository";
import { encryptAllKeys } from "@/encryptionService";
import { logError, logInfo, logWarn } from "@/logger";
import { logFileManager } from "@/logFileManager";
import { UserMemoryManager } from "@/memory/UserMemoryManager";
@ -106,11 +105,10 @@ export default class CopilotPlugin extends Plugin {
await this.loadSettings();
this.settingsUnsubscriber = subscribeToSettingsChange((prev, next) => {
void (async () => {
if (next.enableEncryption) {
await this.saveData(await encryptAllKeys(next));
} else {
await this.saveData(next);
}
// Reason: encryption-at-rest is deprecated. encryptAllKeys is a no-op
// now; new keys are saved plaintext. Existing encrypted keys remain
// in data.json and are decrypted lazily by getDecryptedKey when read.
await this.saveData(next);
registerCommands(this, prev, next);
})();
});

View file

@ -95,16 +95,6 @@ export const AdvancedSettings: React.FC = () => {
<section className="tw-space-y-4 tw-rounded-lg tw-border tw-p-4">
<h3 className="tw-text-lg tw-font-semibold">Others</h3>
<SettingItem
type="switch"
title="Enable Encryption"
description="Enable encryption for the API keys."
checked={settings.enableEncryption}
onCheckedChange={(checked) => {
updateSetting("enableEncryption", checked);
}}
/>
<SettingItem
type="switch"
title="Debug Mode"