Skip to content

Commit 67680a5

Browse files
committed
fix(desktop): remove safeStorage, store API keys as plaintext 0600
Every decrypt call on unsigned / ad-hoc-signed macOS builds triggered the system keychain password prompt, making the app practically unusable in dev and noisy for users who didn't click "Always Allow". Even "Always Allow" gets invalidated whenever the code signature changes (every pnpm dev rebuild, every release upgrade). Fixes the prompt at the root: stop calling safeStorage. Follow the same convention as Claude Code, Codex, gh CLI, aws CLI, and gcloud — plaintext credentials in ~/.config/open-codesign/config.toml with file mode 0600. No OS prompt, ever. - apps/desktop/src/main/keychain.ts now stores secrets as `plain:<key>` and falls back to safeStorage only to migrate legacy ciphertexts once at boot (the last keychain prompt ever, if any). - Boot-time migration runs inside loadConfigOnBoot; rewritten secrets land back on disk immediately so the prompt can't repeat. - Removed apps/desktop/src/main/keychain-ux.ts entirely (explainer + unavailable dialogs are no longer needed). - Removed withKeychain / prepareKeychain wrappers from all IPC handlers — they were only there to gate safeStorage calls. - Updated onboarding i18n copy and CLAUDE.md / README.zh-CN.md to reflect the new storage model. Threat model is unchanged: an attacker with filesystem access to the user's home dir could already read a decrypted secret via process memory or by exploiting safeStorage. Plaintext just removes the UX tax that bought zero real security.
1 parent fb3ef85 commit 67680a5

8 files changed

Lines changed: 139 additions & 258 deletions

File tree

CLAUDE.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ These are project-level commitments, not preferences:
1616

1717
1. **Install size budget: ≤ 80 MB.** Adding a dependency that pushes us over requires PR justification with size diff and alternatives considered. CI enforces this.
1818
2. **No bundled model runtimes.** No Ollama, llama.cpp, Python, or browser binaries shipped in the installer. Use system installs or lazy-download on demand.
19-
3. **BYOK only.** No proxied API calls, no cloud account, no telemetry by default. User credentials stay in `~/.config/open-codesign/config.toml` (encrypted via Electron `safeStorage`).
19+
3. **BYOK only.** No proxied API calls, no cloud account, no telemetry by default. User credentials stay in `~/.config/open-codesign/config.toml` (plaintext, file mode 0600 — matching Claude Code / Codex / gh CLI conventions).
2020
4. **Local-first storage.** Designs, history, and codebase scans live on disk (SQLite via `better-sqlite3`). No mandatory cloud sync.
2121
5. **MIT-compatible permissive licenses only.** Reject GPL/AGPL/SSPL/proprietary deps. Check license before adding anything.
2222
6. **Lazy-load heavy features.** PPTX export, web capture, codebase scan, etc. must dynamic-import on first use, not on app start.

README.zh-CN.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ Open CoDesign 可以把一句自然语言提示词,直接变成一个完成度
159159
- Google Gemini
160160
- 任意 OpenAI 兼容中继,比如 OpenRouter、SiliconFlow、本地 Ollama
161161
162-
凭证会保存在 `~/.config/open-codesign/config.toml`,并通过 Electron `safeStorage` 加密。除非你选择的模型提供商本身需要联网,请求内容不会额外离开你的机器。
162+
凭证会保存在 `~/.config/open-codesign/config.toml`(文件权限 0600,与 Claude Code、Codex、`gh` CLI 的做法一致)。除非你选择的模型提供商本身需要联网,请求内容不会额外离开你的机器。
163163
164164
### 3. 输入第一条提示词
165165

apps/desktop/src/main/keychain-ux.ts

Lines changed: 0 additions & 138 deletions
This file was deleted.

apps/desktop/src/main/keychain.ts

Lines changed: 93 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -1,52 +1,75 @@
11
import { CodesignError, type Config, ERROR_CODES, type SecretRef } from '@open-codesign/shared';
22
import { safeStorage } from './electron-runtime';
3+
import { getLogger } from './logger';
34

4-
export function ensureKeychainAvailable(): void {
5-
if (!safeStorage.isEncryptionAvailable()) {
6-
throw new CodesignError(
7-
'OS keychain (safeStorage) is not available. Cannot persist API keys securely.',
8-
ERROR_CODES.KEYCHAIN_UNAVAILABLE,
9-
);
10-
}
11-
}
5+
/**
6+
* Secret storage is now plaintext-in-config.toml (mode 0600), matching
7+
* Claude Code / Codex / gh / aws / gcloud. safeStorage was removed because
8+
* unsigned macOS builds triggered a system keychain password prompt on
9+
* every decrypt — real UX tax for no real security gain (an attacker with
10+
* filesystem access could read the plaintext ciphertext either way).
11+
*
12+
* The file name stays `keychain.ts` to minimise churn across importers;
13+
* the module is now really a plaintext passthrough with one-shot legacy
14+
* migration (safeStorage ciphertext → plaintext on first boot after
15+
* upgrade).
16+
*/
17+
18+
const log = getLogger('secret-store');
19+
20+
/** Prefix that marks a new-format (plaintext) stored secret. */
21+
const PLAIN_PREFIX = 'plain:';
1222

1323
export function encryptSecret(plaintext: string): string {
14-
ensureKeychainAvailable();
1524
if (plaintext.length === 0) {
16-
throw new CodesignError('Cannot encrypt empty secret', ERROR_CODES.KEYCHAIN_EMPTY_INPUT);
25+
throw new CodesignError('Cannot store empty secret', ERROR_CODES.KEYCHAIN_EMPTY_INPUT);
1726
}
18-
const buf = safeStorage.encryptString(plaintext);
19-
return buf.toString('base64');
27+
return `${PLAIN_PREFIX}${plaintext}`;
2028
}
2129

22-
export function decryptSecret(ciphertextBase64: string): string {
23-
ensureKeychainAvailable();
24-
if (ciphertextBase64.length === 0) {
25-
throw new CodesignError('Cannot decrypt empty ciphertext', ERROR_CODES.KEYCHAIN_EMPTY_INPUT);
30+
export function decryptSecret(stored: string): string {
31+
if (stored.length === 0) {
32+
throw new CodesignError('Cannot read empty secret', ERROR_CODES.KEYCHAIN_EMPTY_INPUT);
2633
}
27-
const buf = Buffer.from(ciphertextBase64, 'base64');
28-
return safeStorage.decryptString(buf);
34+
if (stored.startsWith(PLAIN_PREFIX)) {
35+
return stored.slice(PLAIN_PREFIX.length);
36+
}
37+
return decryptLegacy(stored);
2938
}
3039

3140
/**
32-
* Derive a display-safe mask for a secret — e.g. "sk-ant-***xyz9". Stored
33-
* alongside the ciphertext so Settings can render the provider row without
34-
* invoking `safeStorage.decryptString` (which prompts for the keychain
35-
* password on unsigned macOS builds).
41+
* Legacy safeStorage fallback. Invoked only for secrets written by older
42+
* app versions that encrypted via Electron's `safeStorage`. On macOS this
43+
* will prompt for the keychain password the FIRST time it's called (and
44+
* only then — subsequent calls in the same process are served from
45+
* safeStorage's in-process key cache). After `migrateSecrets` rewrites the
46+
* config, this path is never hit again.
3647
*/
48+
function decryptLegacy(base64: string): string {
49+
if (!safeStorage.isEncryptionAvailable()) {
50+
throw new CodesignError(
51+
'A legacy encrypted API key was found but the OS keychain is unavailable. Please re-enter your API key in Settings.',
52+
ERROR_CODES.KEYCHAIN_UNAVAILABLE,
53+
);
54+
}
55+
try {
56+
return safeStorage.decryptString(Buffer.from(base64, 'base64'));
57+
} catch (err) {
58+
throw new CodesignError(
59+
'Failed to decrypt a legacy API key. Please re-enter your API key in Settings.',
60+
ERROR_CODES.KEYCHAIN_UNAVAILABLE,
61+
{ cause: err },
62+
);
63+
}
64+
}
65+
3766
export function maskSecret(plaintext: string): string {
3867
if (plaintext.length <= 8) return '***';
3968
const prefix = plaintext.startsWith('sk-') ? 'sk-' : plaintext.slice(0, 4);
4069
const suffix = plaintext.slice(-4);
4170
return `${prefix}***${suffix}`;
4271
}
4372

44-
/**
45-
* Convenience wrapper: encrypt a plaintext API key and return the full
46-
* `SecretRef` (ciphertext + mask) in one shot. Use this at every save site
47-
* so mask metadata is always written. Older configs missing `mask` are
48-
* migrated by `migrateSecretMasks` on first read.
49-
*/
5073
export function buildSecretRef(plaintext: string): SecretRef {
5174
return {
5275
ciphertext: encryptSecret(plaintext),
@@ -55,58 +78,64 @@ export function buildSecretRef(plaintext: string): SecretRef {
5578
}
5679

5780
/**
58-
* Non-throwing variant of `buildSecretRef` — returns `null` when safeStorage
59-
* is unavailable (unsigned macOS without keychain entitlements, Linux
60-
* without a secret-service daemon, etc.). Callers should skip persisting
61-
* the secret and surface a warning so the REST of the imported config
62-
* still lands; the user can re-add the key by hand once keychain is fixed.
63-
*
64-
* Only `KEYCHAIN_UNAVAILABLE` is caught — real unexpected errors still
65-
* propagate so they're not silently swallowed.
81+
* Kept for call-site compat. The new plaintext store can't fail the way
82+
* safeStorage used to (no keychain dependency), so this is really just an
83+
* alias for `buildSecretRef`. Callers that previously got `null` should
84+
* now always get a valid ref.
6685
*/
6786
export function tryBuildSecretRef(plaintext: string): SecretRef | null {
6887
try {
6988
return buildSecretRef(plaintext);
70-
} catch (err) {
71-
if (err instanceof CodesignError && err.code === ERROR_CODES.KEYCHAIN_UNAVAILABLE) {
72-
return null;
73-
}
74-
throw err;
89+
} catch {
90+
return null;
7591
}
7692
}
7793

7894
/**
79-
* One-shot migration: for any secret missing the `mask` field, decrypt it
80-
* once and populate the mask. Designed to run on config load — after this
81-
* pass, `toProviderRows` never touches safeStorage again. Returns the
82-
* migrated config plus a flag indicating whether anything changed (so the
83-
* caller can decide whether to persist).
95+
* One-shot migration run on boot:
96+
* 1. Any secret stored in legacy safeStorage base64 format → decrypt
97+
* once (last keychain prompt ever) → rewrite as `plain:<apikey>`.
98+
* 2. Any plaintext secret missing its display `mask` → fill it.
8499
*
85-
* Each decrypt-for-migration triggers exactly one keychain prompt per
86-
* provider on unsigned macOS builds — unavoidable since we have to read
87-
* the plaintext once to derive the mask. This is a one-time cost; all
88-
* future launches read masks directly from disk.
89-
*
90-
* Robust to partial failures: if a single provider fails to decrypt (e.g.
91-
* keychain revoked access mid-migration), we leave that row's mask
92-
* unset and carry on with the rest.
100+
* Idempotent. Partial failures (a single row that can't be decrypted)
101+
* leave that row untouched so the rest of the migration can land; the
102+
* user can re-enter that key from Settings.
93103
*/
94-
export function migrateSecretMasks(cfg: Config): { config: Config; changed: boolean } {
104+
export function migrateSecrets(cfg: Config): { config: Config; changed: boolean } {
95105
const secrets = cfg.secrets ?? {};
96106
const entries = Object.entries(secrets);
97-
const needs = entries.filter(([, ref]) => ref.mask === undefined || ref.mask.length === 0);
98-
if (needs.length === 0) return { config: cfg, changed: false };
107+
if (entries.length === 0) return { config: cfg, changed: false };
99108

100109
const nextSecrets: Record<string, SecretRef> = { ...secrets };
101110
let changed = false;
102-
for (const [provider, ref] of needs) {
103-
try {
104-
const plain = decryptSecret(ref.ciphertext);
105-
nextSecrets[provider] = { ...ref, mask: maskSecret(plain) };
106-
changed = true;
107-
} catch {
108-
/* skip — row will retry on next boot or when user edits it */
111+
for (const [provider, ref] of entries) {
112+
const isLegacy = !ref.ciphertext.startsWith(PLAIN_PREFIX);
113+
const needsMask = ref.mask === undefined || ref.mask.length === 0;
114+
if (!isLegacy && !needsMask) continue;
115+
116+
let plaintext: string;
117+
if (isLegacy) {
118+
try {
119+
plaintext = decryptLegacy(ref.ciphertext);
120+
} catch (err) {
121+
log.warn('secret.migration.decrypt_failed', {
122+
provider,
123+
err: err instanceof Error ? err.message : String(err),
124+
});
125+
continue;
126+
}
127+
} else {
128+
plaintext = ref.ciphertext.slice(PLAIN_PREFIX.length);
109129
}
130+
131+
nextSecrets[provider] = {
132+
ciphertext: `${PLAIN_PREFIX}${plaintext}`,
133+
mask: maskSecret(plaintext),
134+
};
135+
changed = true;
110136
}
111137
return { config: { ...cfg, secrets: nextSecrets }, changed };
112138
}
139+
140+
/** @deprecated Use `migrateSecrets`. Kept as alias for a few callers. */
141+
export const migrateSecretMasks = migrateSecrets;

apps/desktop/src/main/onboarding-ipc.test.ts

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -68,17 +68,14 @@ vi.mock('./keychain', () => ({
6868
ciphertext: `enc:${s}`,
6969
mask: s.length > 8 ? `${s.slice(0, 4)}***${s.slice(-4)}` : '***',
7070
})),
71+
migrateSecrets: vi.fn((cfg: { secrets?: Record<string, unknown> }) => ({
72+
config: cfg,
73+
changed: false,
74+
})),
7175
migrateSecretMasks: vi.fn((cfg: { secrets?: Record<string, unknown> }) => ({
7276
config: cfg,
7377
changed: false,
7478
})),
75-
ensureKeychainAvailable: vi.fn(() => {}),
76-
}));
77-
78-
vi.mock('./keychain-ux', () => ({
79-
prepareKeychain: vi.fn(async () => true),
80-
maybeShowKeychainExplainer: vi.fn(async () => {}),
81-
maybeShowKeychainUnavailableDialog: vi.fn(async () => {}),
8279
}));
8380

8481
vi.mock('./storage-settings', () => ({

0 commit comments

Comments
 (0)