fix(config): surface secret decryption failures#65
Conversation
📝 WalkthroughWalkthroughCentralized decryption error handling across config endpoints by introducing a shared Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client (Frontend)
participant API as Backend API
participant Crypto as Crypto Handler
Client->>API: GET /api/configs/ai?decrypt=true
API->>Crypto: getMaskedSecretResult(encryptedKey)
alt Decryption Succeeds
Crypto-->>API: {decryptedValue: "actual_key", status: "ok"}
API->>API: Include apiKeyStatus: "ok"
API-->>Client: {...config, apiKey: "actual_key", apiKeyStatus: "ok"}
else Decryption Fails
Crypto-->>API: {decryptedValue: "", status: "decrypt_failed"}
API->>API: Log warning, include status
API-->>Client: {...config, apiKey: "****", apiKeyStatus: "decrypt_failed"}
Client->>Client: Display warning to user
else Value is Empty
Crypto-->>API: {decryptedValue: "", status: "empty"}
API-->>Client: {...config, apiKey: "", apiKeyStatus: "empty"}
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@server/src/routes/configs.ts`:
- Around line 402-410: The github_token branch currently only runs
getMaskedSecretResult when value is truthy, so settings.github_token_status is
left undefined for empty tokens; always call getMaskedSecretResult for key ===
'github_token' (passing encryptedValue: value and encryptionKey:
config.encryptionKey), set settings.github_token_status = status
unconditionally, and then set value = status === 'empty' ? '' :
maskApiKey(decryptedValue) so consumers can distinguish an explicit empty token
from a missing status (update logic around getMaskedSecretResult, maskApiKey,
and settings.github_token_status accordingly).
In `@src/components/SettingsPanel.tsx`:
- Around line 946-953: Add the same visible warning flow for GitHub token
decrypt failures by rendering a warning paragraph when
config.github_token_status === 'decrypt_failed' next to the existing API key
block in SettingsPanel (where config.apiKeyStatus is checked); mirror the
styling and translation usage (className "mt-1 text-sm text-amber-600
dark:text-amber-400" and t(...) pattern) and provide a bilingual message
analogous to the API key one (e.g., "存储的 GitHub Token 无法解密,请重新输入并保存该配置。" / "The
stored GitHub token could not be decrypted. Please re-enter and save this
configuration.") so users see the same warning for github_token_status decrypt
failures.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 94475848-56a5-400a-9b24-3dabd10436f3
📒 Files selected for processing (3)
server/src/routes/configs.tssrc/components/SettingsPanel.tsxsrc/types/index.ts
| if (key === 'github_token' && value) { | ||
| try { | ||
| const decrypted = decrypt(value, config.encryptionKey); | ||
| value = maskApiKey(decrypted); | ||
| } catch { | ||
| value = '****'; | ||
| } | ||
| const { decryptedValue, status } = getMaskedSecretResult({ | ||
| encryptedValue: value, | ||
| encryptionKey: config.encryptionKey, | ||
| kind: 'GitHub token', | ||
| }); | ||
| value = status === 'empty' ? '' : maskApiKey(decryptedValue); | ||
| settings.github_token_status = status; | ||
| } |
There was a problem hiding this comment.
Return github_token_status even when token is empty.
Line 402 currently skips status assignment when value is empty/null, so consumers can’t distinguish “missing status” vs explicit empty state. Please run the helper for every github_token row and let it return 'empty'.
💡 Suggested fix
- if (key === 'github_token' && value) {
+ if (key === 'github_token') {
const { decryptedValue, status } = getMaskedSecretResult({
encryptedValue: value,
encryptionKey: config.encryptionKey,
kind: 'GitHub token',
});
value = status === 'empty' ? '' : maskApiKey(decryptedValue);
settings.github_token_status = status;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@server/src/routes/configs.ts` around lines 402 - 410, The github_token branch
currently only runs getMaskedSecretResult when value is truthy, so
settings.github_token_status is left undefined for empty tokens; always call
getMaskedSecretResult for key === 'github_token' (passing encryptedValue: value
and encryptionKey: config.encryptionKey), set settings.github_token_status =
status unconditionally, and then set value = status === 'empty' ? '' :
maskApiKey(decryptedValue) so consumers can distinguish an explicit empty token
from a missing status (update logic around getMaskedSecretResult, maskApiKey,
and settings.github_token_status accordingly).
| {config.apiKeyStatus === 'decrypt_failed' && ( | ||
| <p className="mt-1 text-sm text-amber-600 dark:text-amber-400"> | ||
| {t( | ||
| '存储的 API Key 无法解密,请重新输入并保存该配置。', | ||
| 'The stored API key could not be decrypted. Please re-enter and save this configuration.' | ||
| )} | ||
| </p> | ||
| )} |
There was a problem hiding this comment.
Add equivalent warning flow for github_token_status.
You now surface AI/WebDAV decrypt failures, but GitHub token decrypt failures still have no visible warning in the settings UI. That leaves one credential type silent from the user perspective.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/SettingsPanel.tsx` around lines 946 - 953, Add the same
visible warning flow for GitHub token decrypt failures by rendering a warning
paragraph when config.github_token_status === 'decrypt_failed' next to the
existing API key block in SettingsPanel (where config.apiKeyStatus is checked);
mirror the styling and translation usage (className "mt-1 text-sm text-amber-600
dark:text-amber-400" and t(...) pattern) and provide a bilingual message
analogous to the API key one (e.g., "存储的 GitHub Token 无法解密,请重新输入并保存该配置。" / "The
stored GitHub token could not be decrypted. Please re-enter and save this
configuration.") so users see the same warning for github_token_status decrypt
failures.
Summary
What changed
ok,empty,decrypt_failed) for config reads.Why
Issue #64 correctly pointed out that decryption errors were previously swallowed, which could make secrets appear empty without any explanation.
This PR fixes that without making the whole settings page fail just because one stored secret is broken.
Validation
npm run build✅Closes #64
Summary by CodeRabbit