fix(crypt): symmetric crypt compatibility#546
Conversation
Signed-off-by: fufesou <linlong1266@gmail.com>
|
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)
📝 WalkthroughWalkthroughBoth ChangesEncrypted Password Preservation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
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 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.
🧹 Nitpick comments (1)
src/config.rs (1)
724-732: 💤 Low valueRedundant assignment when passwords are equal.
When
stored.password == config.passwordevaluates to true, the subsequent assignmentconfig.password = stored.passwordis a no-op since both strings are already equal. Consider simplifying to only call the helper when values differ:♻️ Suggested simplification
let stored = Config::load_::<Config>(""); - if stored.password == config.password { - config.password = stored.password; - } else { + if stored.password != config.password { config.password = keep_encrypted_storage_if_plaintext_unchanged( &config.password, &stored.password, ); }Note: The equality check correctly handles the legacy plaintext case where neither value is encrypted, so the overall logic is sound.
🤖 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/config.rs` around lines 724 - 732, The branch assigns stored.password to config.password even when they're equal, which is redundant; update the logic in the block that loads Config::load_::<Config>("") so that you only call keep_encrypted_storage_if_plaintext_unchanged(&config.password, &stored.password) and assign its result to config.password when stored.password != config.password, otherwise leave config.password untouched (i.e., remove the no-op assignment of stored.password when values are equal).
🤖 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.
Nitpick comments:
In `@src/config.rs`:
- Around line 724-732: The branch assigns stored.password to config.password
even when they're equal, which is redundant; update the logic in the block that
loads Config::load_::<Config>("") so that you only call
keep_encrypted_storage_if_plaintext_unchanged(&config.password,
&stored.password) and assign its result to config.password when stored.password
!= config.password, otherwise leave config.password untouched (i.e., remove the
no-op assignment of stored.password when values are equal).
Signed-off-by: fufesou <linlong1266@gmail.com>
Retain the local permanent and SOCKS passwords if unchanged.
Testing
Switching between 1.4.6 and the current version does not alter the local permanent or SOCKS passwords unless manually changed.
Notes
Changes to the local permanent password, RDP password, OS username, or OS password only affect the control/local side.
Compatibility for this does not need to be over-engineered.
Summary by CodeRabbit