feat: add toggle to control key inclusion in backup/export#209
Merged
Conversation
- Add includeKeysInBackup state (default: false, security first) - Create IncludeKeysToggle shared component - Update BackupPanel with conditional key masking/restoration - Update DataManagementPanel with conditional key masking/restoration - Add warning for masked secrets in import preview - Cover all secret types: AI API keys, WebDAV passwords, proxy passwords, RPC secrets, backend API secret - Smart fallback: masked secrets preserve existing values on import - Version migration: v6 -> v7 Closes #207 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a persisted includeKeysInBackup preference and toggle; export now records the flag and masks secret fields as '***' when disabled; import/restore applies secret values only when backups indicate keys were included and values are unmasked, preserving on-device secrets otherwise. ChangesBackup and export secret masking
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/settings/BackupPanel.tsx (1)
190-299:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore should trust the backup metadata, not the current UI toggle.
All secret restore branches here are gated by the local
includeKeysInBackupstate. On a fresh machine that value defaults tofalse, so a backup that actually contains real secrets will restore with preserved/blank local values instead of the backed-up credentials. This also breaks legacy backups that predate the flag. Read the decision frombackupData.includeKeysInBackuponce and reuse it for the AI/WebDAV/proxy/RPC/backend branches, with a legacy fallback oftrue.Suggested fix
const backupData = JSON.parse(backupContent); + const backupIncludedKeys = backupData.includeKeysInBackup ?? true; ... - apiKey: includeKeysInBackup && cfg.apiKey && cfg.apiKey !== '***' ? cfg.apiKey : existing.apiKey, + apiKey: backupIncludedKeys && cfg.apiKey && cfg.apiKey !== '***' ? cfg.apiKey : existing.apiKey, ... - password: includeKeysInBackup && cfg.password && cfg.password !== '***' ? cfg.password : existing.password, + password: backupIncludedKeys && cfg.password && cfg.password !== '***' ? cfg.password : existing.password, ... - password: includeKeysInBackup && backupProxyConfig.password && backupProxyConfig.password !== '***' + password: backupIncludedKeys && backupProxyConfig.password && backupProxyConfig.password !== '***' ? backupProxyConfig.password : latestProxyConfig.password, ... - secret: includeKeysInBackup && backupRpcDownloadConfig.secret && backupRpcDownloadConfig.secret !== '***' + secret: backupIncludedKeys && backupRpcDownloadConfig.secret && backupRpcDownloadConfig.secret !== '***' ? backupRpcDownloadConfig.secret : latestRpcDownloadConfig.secret, ... - if (includeKeysInBackup && backupData.backendApiSecret !== undefined && backupData.backendApiSecret !== '***') { + if (backupIncludedKeys && backupData.backendApiSecret !== undefined && backupData.backendApiSecret !== '***') {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/settings/BackupPanel.tsx` around lines 190 - 299, The restore logic incorrectly relies on the local includeKeysInBackup UI state instead of the backup's metadata; change the code to read a single boolean from backupData.includeKeysInBackup (use true as the legacy fallback when the field is missing) once at the start of the restore and then use that local variable in all secret decisions in restoreAIConfigs (the AI config block), restoreWebDAVConfigs (WebDAV block), proxy restore (setProxyConfig block), rpcDownload restore (setRpcDownloadConfig block), and backendApiSecret assignment, preserving the existing '***' masking checks when deciding whether to apply the backed-up secret.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/components/settings/DataManagementPanel.tsx`:
- Around line 713-715: Replace the direct state mutation that writes
backendApiSecret via useAppStore.setState with the proper store action
setBackendApiSecret so the value is persisted to sessionStorage and survives
reloads; specifically, inside the block that checks wasIncluded,
importedData.backendApiSecret !== undefined and
isRealSecret(importedData.backendApiSecret), call
setBackendApiSecret(importedData.backendApiSecret) (instead of
useAppStore.setState) to mirror the existing persistence behavior.
- Line 592: The import logic in DataManagementPanel uses const wasIncluded =
importedData.includeKeysInBackup ?? false which treats missing
includeKeysInBackup as false and strips legacy secrets; update the fallback to
treat undefined as a legacy export by defaulting to true (change the fallback
for importedData.includeKeysInBackup to true) so older exports retain keys on
import; update the declaration of wasIncluded and add a short comment noting the
backward-compatibility behavior.
- Around line 528-539: The export currently always writes proxyConfig,
rpcDownloadConfig and backendApiSecret into exportDataObj.data regardless of the
user's selected export checklist; change the logic so those fields are only
serialized when the user explicitly selects the appropriate category/flag (e.g.,
a new "secrets"/"sensitive configs" checkbox or an existing "settings/configs"
category) — update the code around exportDataObj.data.proxyConfig,
rpcDownloadConfig and backendApiSecret to guard on that selected category (not
just includeKeys), and ensure MASKED_SECRET behavior remains when the
secret-category is selected but includeKeys is false; also update the export
preview/export checklist code paths so these fields appear only when that
category is checked.
---
Outside diff comments:
In `@src/components/settings/BackupPanel.tsx`:
- Around line 190-299: The restore logic incorrectly relies on the local
includeKeysInBackup UI state instead of the backup's metadata; change the code
to read a single boolean from backupData.includeKeysInBackup (use true as the
legacy fallback when the field is missing) once at the start of the restore and
then use that local variable in all secret decisions in restoreAIConfigs (the AI
config block), restoreWebDAVConfigs (WebDAV block), proxy restore
(setProxyConfig block), rpcDownload restore (setRpcDownloadConfig block), and
backendApiSecret assignment, preserving the existing '***' masking checks when
deciding whether to apply the backed-up secret.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 552402ee-ae01-4920-8c71-2dd5d3e15818
📒 Files selected for processing (5)
src/components/settings/BackupPanel.tsxsrc/components/settings/DataManagementPanel.tsxsrc/components/settings/IncludeKeysToggle.tsxsrc/store/useAppStore.tssrc/types/index.ts
- Use backup metadata (includeKeysInBackup) instead of current UI state for restore - Default legacy backups to true (treat missing flag as keys included) - Use setBackendApiSecret action instead of direct setState - Only export special configs (proxy/RPC/backend) when uiSettings is selected - Add comment explaining legacy compatibility fallback Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Move toggle from inside Export panel to standalone container - Place between section title and export/import panels - Prevent toggle from being compressed in layout Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概述
添加备份/导出时是否包含敏感密钥的可选开关，用户可以选择导出完整配置（包含密钥）或安全配置（密钥屏蔽）。
功能特性
🔒 安全优先设计
***屏蔽真实密钥🎯 覆盖范围
开关控制以下所有密钥的备份/导出行为：
apiKeypasswordpasswordsecretbackendApiSecret📍 使用场景
技术实现
新增组件
IncludeKeysToggle.tsx- 可复用的开关组件修改文件
useAppStore.ts- 新增includeKeysInBackup状态（v6→v7 迁移）BackupPanel.tsx- 集成开关，处理备份/恢复密钥逻辑DataManagementPanel.tsx- 集成开关，处理导出/导入密钥逻辑，添加警告提示types/index.ts- 新增类型定义数据结构变更
导出/备份数据新增字段：
includeKeysInBackup：标记该备份是否包含密钥proxyConfig：网络代理配置rpcDownloadConfig：远程下载配置backendApiSecret：后端 API 密钥审计问题修复
🔴 关键安全问题
false（安全优先）undefined→ 空字符串）🟡 改进项
测试建议
测试场景
***验证点
Breaking Change
includeKeysInBackup默认为false，现有用户需要手动开启才能导出完整密钥。截图
（建议添加开关界面和警告提示的截图）
Closes #207
🤖 Generated with Claude Code
Summary by CodeRabbit