Repository navigation
fix(encryption): rebuild unusable user key - #3627
Merged
Merged
Conversation
A DPAPI-protected UserKey.bin can become permanently undecryptable, e.g. after
an OS reinstall that keeps user data, a profile migration, a password reset or a
Microsoft account switch. CryptUnprotectData then fails with NTE_BAD_KEY_STATE
("Key not valid for use in specified state"), and since the failure was rethrown
from Lazy<byte[]> it got cached for the whole process: every encrypt/decrypt
failed and profiles could never be saved (PCL-Community#3581).
- Cache the user key only on success, so a failure is retried on the next access
instead of being remembered forever.
- Rebuild an unusable key file: log the HRESULT, keep the broken blob as
UserKey.bin.broken-<timestamp>, then generate and store a fresh v2 key.
- Migrate v1 (CAPI DPAPI) keys to v2 (CNG DPAPI) on successful decrypt, so
existing users converge instead of staying on the legacy path forever.
- Verify the blob round-trips before writing it, so a key that cannot be read
back is never persisted (would otherwise re-generate a key on every launch).
- Leave transient I/O failures (IOException, UnauthorizedAccessException)
untouched: they are retried rather than treated as a broken key.
Data protected by the lost key is unrecoverable, so affected users must log in
again once; the launcher itself now recovers without manually deleting the file.
Adds EncryptHelperKeyTest covering migration, rebuild and the no-rotate-on-I/O
rule.
Contributor
审查者指南EncryptHelper 现在能够安全地从不可用的 UserKey.bin 文件中恢复:将其隔离,并替换为经过验证的 v2 CNG 保护密钥;同时迁移有效的 v1 密钥、重试失败的加载操作,并在暂时性 I/O 错误期间保留密钥;相关测试已覆盖这些场景。 UserKey 恢复与迁移时序图sequenceDiagram
participant Caller
participant EncryptHelper
participant UserKeyFile
participant CNGDPAPI
participant Log
Caller->>EncryptHelper: EncryptionKey
alt existing v2 key
EncryptHelper->>UserKeyFile: Read key
EncryptHelper->>CNGDPAPI: Unprotect
CNGDPAPI-->>EncryptHelper: key
else existing v1 key
EncryptHelper->>UserKeyFile: Read key
EncryptHelper->>CNGDPAPI: ProtectedData.Unprotect
CNGDPAPI-->>EncryptHelper: legacyKey
EncryptHelper->>CNGDPAPI: CngProtectedData.Protect
EncryptHelper->>CNGDPAPI: CngProtectedData.Unprotect
EncryptHelper->>UserKeyFile: _WriteKeyFile version 2
EncryptHelper-->>Caller: legacyKey
else unusable key file
EncryptHelper->>Log: LogWrapper.Error
EncryptHelper->>CNGDPAPI: _TryProtect
CNGDPAPI->>CNGDPAPI: round-trip validation
EncryptHelper->>UserKeyFile: _QuarantineBrokenKeyFile
EncryptHelper->>UserKeyFile: _WriteKeyFile version 2
EncryptHelper->>Log: LogWrapper.Warn
EncryptHelper-->>Caller: new key
else transient I/O failure
EncryptHelper-->>Caller: IOException or UnauthorizedAccessException
Caller->>EncryptHelper: EncryptionKey retry
end
已验证用户密钥创建流程图flowchart TD
A[Create random 32-byte key] --> B[_TryProtect with CNG DPAPI]
B --> C{Round-trip succeeds?}
C -- No --> D[Fail without replacing UserKey.bin]
C -- Yes --> E[_QuarantineBrokenKeyFile]
E --> F[_WriteKeyFile version 2]
F --> G[Return key and preserve it for future startup]
文件级变更
针对相关 issue 的评估
可能相关的 issue
提示和命令与 Sourcery 交互
自定义使用体验访问你的控制面板以:
获取帮助Original review guide in EnglishReviewer's GuideEncryptHelper now safely recovers from unusable UserKey.bin files by quarantining and replacing them with validated v2 CNG-protected keys, migrates valid v1 keys, retries failed loads, and preserves keys across transient I/O errors; focused tests cover these scenarios. Sequence diagram for UserKey recovery and migrationsequenceDiagram
participant Caller
participant EncryptHelper
participant UserKeyFile
participant CNGDPAPI
participant Log
Caller->>EncryptHelper: EncryptionKey
alt existing v2 key
EncryptHelper->>UserKeyFile: Read key
EncryptHelper->>CNGDPAPI: Unprotect
CNGDPAPI-->>EncryptHelper: key
else existing v1 key
EncryptHelper->>UserKeyFile: Read key
EncryptHelper->>CNGDPAPI: ProtectedData.Unprotect
CNGDPAPI-->>EncryptHelper: legacyKey
EncryptHelper->>CNGDPAPI: CngProtectedData.Protect
EncryptHelper->>CNGDPAPI: CngProtectedData.Unprotect
EncryptHelper->>UserKeyFile: _WriteKeyFile version 2
EncryptHelper-->>Caller: legacyKey
else unusable key file
EncryptHelper->>Log: LogWrapper.Error
EncryptHelper->>CNGDPAPI: _TryProtect
CNGDPAPI->>CNGDPAPI: round-trip validation
EncryptHelper->>UserKeyFile: _QuarantineBrokenKeyFile
EncryptHelper->>UserKeyFile: _WriteKeyFile version 2
EncryptHelper->>Log: LogWrapper.Warn
EncryptHelper-->>Caller: new key
else transient I/O failure
EncryptHelper-->>Caller: IOException or UnauthorizedAccessException
Caller->>EncryptHelper: EncryptionKey retry
end
Flow diagram for validated user key creationflowchart TD
A[Create random 32-byte key] --> B[_TryProtect with CNG DPAPI]
B --> C{Round-trip succeeds?}
C -- No --> D[Fail without replacing UserKey.bin]
C -- Yes --> E[_QuarantineBrokenKeyFile]
E --> F[_WriteKeyFile version 2]
F --> G[Return key and preserve it for future startup]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
There was a problem hiding this comment.
您好——我发现了 1 个问题
面向 AI 代理的提示
请处理本次代码审查中的评论:
## 单独评论
### 评论 1
<location path="PCL.Core/Utils/Secret/EncryptHelper.cs" line_range="216-220" />
<code_context>
+ if (!_TryProtect(key, out var storeData))
+ throw new InvalidOperationException("本机 DPAPI 不可用,无法创建用户密钥");
+ _QuarantineBrokenKeyFile(keyFile); // 已确认能重建,才动原文件
+ if (!_WriteKeyFile(keyFile, storeData))
+ LogWrapper.Error("Encryption", $"用户密钥写入失败,本次运行仅使用内存密钥:{keyFile}");
+ else
+ LogWrapper.Warn("Encryption", "用户密钥已重建(version 2)。旧密钥保护的数据无法恢复,需要重新登录。");
+ return key;
+ }
+
</code_context>
<issue_to_address>
**问题(bug_risk):** 当不可用的密钥被隔离,但 `_WriteKeyFile` 失败时,`_CreateKey` 会返回新生成的内存密钥,而 `EncryptionKey` 会将其缓存。在该次运行期间使用此密钥加密的任何机密信息,在重启后都无法解密,因为对应的密钥从未被持久化保存。
**触发条件:** 重建损坏的密钥文件时,遇到临时性或权限相关的写入失败。
**建议修复:** 不要返回并缓存内存密钥,而应向上传播写入失败;或者保留旧文件,并在替换密钥持久化存储成功之前禁止加密。
```suggestion
if (!_WriteKeyFile(keyFile, storeData))
throw new IOException($"用户密钥写入失败:{keyFile}");
LogWrapper.Warn("Encryption", "用户密钥已重建(version 2)。旧密钥保护的数据无法恢复,需要重新登录。");
return key;
```
</issue_to_address>Original comment in English
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="PCL.Core/Utils/Secret/EncryptHelper.cs" line_range="216-220" />
<code_context>
+ if (!_TryProtect(key, out var storeData))
+ throw new InvalidOperationException("本机 DPAPI 不可用,无法创建用户密钥");
+ _QuarantineBrokenKeyFile(keyFile); // 已确认能重建,才动原文件
+ if (!_WriteKeyFile(keyFile, storeData))
+ LogWrapper.Error("Encryption", $"用户密钥写入失败,本次运行仅使用内存密钥:{keyFile}");
+ else
+ LogWrapper.Warn("Encryption", "用户密钥已重建(version 2)。旧密钥保护的数据无法恢复,需要重新登录。");
+ return key;
+ }
+
</code_context>
<issue_to_address>
**issue (bug_risk):** When an unusable key is quarantined but `_WriteKeyFile` fails, `_CreateKey` returns the newly generated in-memory key and `EncryptionKey` caches it. Any secrets encrypted during that run cannot be decrypted after restart because the corresponding key was never persisted.
**Triggers:** When rebuilding a corrupted key file encounters a transient or permission-related write failure.
**Suggested fix:** Propagate the write failure instead of returning and caching an in-memory key, or retain the old file and prevent encryption until the replacement is durably stored.
```suggestion
if (!_WriteKeyFile(keyFile, storeData))
throw new IOException($"用户密钥写入失败:{keyFile}");
LogWrapper.Warn("Encryption", "用户密钥已重建(version 2)。旧密钥保护的数据无法恢复,需要重新登录。");
return key;
```
</issue_to_address>
Lokins577
reviewed
Sep 26, 2026
Co-authored-by: NoClassDefFoundError <rnmddos@163.com>
Chiloven945
approved these changes
Sep 30, 2026
Lokins577
approved these changes
Sep 30, 2026
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
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.
EncryptHelper不再缓存失败的密钥加载。无法使用的UserKey.bin会连同其 HRESULT 一起记录日志,保留为UserKey.bin.broken-<timestamp>,并由新的 v2 密钥替换,因此启动器会自行恢复,而不再依赖“删除 UserKey.bin”的变通方法。已验证:
dotnet test PCL.Core.Test --filter EncryptHelperKeyTest,以及手动复现(损坏%APPDATA%\PCLCE\UserKey.bin。不再出现“写入档案列表失败”,配置文件在重启后仍保留)。注意:使用丢失密钥加密的数据无法恢复,因此这些用户需要重新登录一次。
Sourcery 摘要
在确保数据安全的同时,使用户密钥加载能够从暂时性文件错误中自动恢复。
错误修复:
增强功能:
测试:
Original summary in English
Sourcery 摘要
在防止瞬时存储错误导致数据丢失的同时,使用户密钥加载具备恢复能力。
错误修复:
增强功能:
测试:
Original summary in English
Summary by Sourcery
Make user-key loading recoverable while preventing data loss from transient storage errors.
Bug Fixes:
Enhancements:
Tests: