Conversation
📝 WalkthroughWalkthroughAdds per-server secure sudo-password overrides via a new SudoPassword utility (read/write/clear, terminal resolution, and biometric-gated authentication). UI to view, edit, clear, and persist sudo-passwords is added to the server edit page and integrated into the save flow. Adds a virtual key and app-bar action to inject stored sudo passwords into SSH terminals with prompt detection. Adds a desktop setting to optionally auto-copy SSH passwords, updates Hive adapters/enums/localizations, and clears stored sudo overrides when servers are deleted. Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/data/provider/server/all.dart (1)
243-243: Isolate secure-storage cleanup failures to avoid partial delete flows.A thrown error from
SudoPassword.clearOverride(...)can short-circuit later cleanup in these methods. Consider making override cleanup best-effort and logging failures.♻️ Suggested hardening
@@ - await SudoPassword.clearOverride(id); + try { + await SudoPassword.clearOverride(id); + } catch (e) { + Loggers.app.warning('Failed to clear sudo override for $id: $e'); + } @@ - await Future.wait(serverIds.map(SudoPassword.clearOverride)); + await Future.wait( + serverIds.map((id) async { + try { + await SudoPassword.clearOverride(id); + } catch (e) { + Loggers.app.warning('Failed to clear sudo override for $id: $e'); + } + }), + );Also applies to: 267-267
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/data/provider/server/all.dart` at line 243, Make the secure-storage cleanup a best-effort operation by wrapping calls to SudoPassword.clearOverride(id) in a try/catch so any thrown error is caught and logged (using the existing logger in the file) and does not short-circuit the remaining cleanup or delete flow; update every place where SudoPassword.clearOverride is invoked (including the other occurrence mentioned) to catch exceptions, log a descriptive message with the error and id/context, and then continue without rethrowing so later cleanup and deletion proceed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lib/data/provider/server/all.dart`:
- Line 243: Make the secure-storage cleanup a best-effort operation by wrapping
calls to SudoPassword.clearOverride(id) in a try/catch so any thrown error is
caught and logged (using the existing logger in the file) and does not
short-circuit the remaining cleanup or delete flow; update every place where
SudoPassword.clearOverride is invoked (including the other occurrence mentioned)
to catch exceptions, log a descriptive message with the error and id/context,
and then continue without rethrowing so later cleanup and deletion proceed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 24f80193-71e1-4f5a-9293-665ef7019ec8
📒 Files selected for processing (1)
lib/data/provider/server/all.dart
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/data/provider/server/all.dart`:
- Around line 84-90: The cleanup helper _clearSudoPasswordOverrideBestEffort
currently clears sudo_pwd for the current id but updateServer's ID-rename path
leaves sudo_pwd_<oldId> behind; update the updateServer flow (the branch that
renames a server) to call _clearSudoPasswordOverrideBestEffort(oldId) (or
directly SudoPassword.clearOverride(oldId)) after a successful rename so the old
secret key sudo_pwd_<oldId> is removed; ensure this call runs even if the rename
is asynchronous/fails gracefully and keep the existing try/catch pattern used in
_clearSudoPasswordOverrideBestEffort to avoid throwing from updateServer.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 38a874b9-43a2-4496-9e19-6bd53e896149
📒 Files selected for processing (1)
lib/data/provider/server/all.dart
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/data/provider/server/all.dart (1)
263-275:⚠️ Potential issue | 🟠 MajorPreserve or cancel the auto-refresh timer before state reset.
At Line 271,
state = const ServersState();dropsautoRefreshTimerwithout canceling it. The periodic timer can keep running in the background whileisAutoRefreshOnbecomes false, and later toggles may create duplicate timers.🔧 Suggested fix
Future<void> deleteAll() async { final serverIds = state.servers.keys.toList(); // Remove all SSH sessions before clearing servers for (final id in serverIds) { final sessionId = 'ssh_$id'; TermSessionManager.remove(sessionId); } - state = const ServersState(); + state = state.copyWith( + servers: const <String, Spi>{}, + serverOrder: const <String>[], + tags: const <String>{}, + manualDisconnectedIds: const <String>{}, + ); Stores.setting.serverOrder.put([]); Stores.server.clear(); await Future.wait(serverIds.map(_clearSudoPasswordOverrideBestEffort)); await Stores.connectionStats.clearAll(); bakSync.sync(milliDelay: 1000); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/data/provider/server/all.dart` around lines 263 - 275, Before resetting state to ServersState, ensure any running auto-refresh timer is stopped: check state.autoRefreshTimer (or call an existing stop/disable helper such as stopAutoRefresh()/toggleAutoRefreshOff) and cancel it and clear the reference so the periodic Timer won't continue after state = const ServersState(); then proceed to remove SSH sessions and reset Stores; update the block around state = const ServersState() (referencing state, autoRefreshTimer, isAutoRefreshOn, and any stopAutoRefresh/toggleAutoRefresh methods) to cancel the timer and set isAutoRefreshOn to false (or null) before assigning the new ServersState to avoid duplicate timers later.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@lib/data/provider/server/all.dart`:
- Around line 263-275: Before resetting state to ServersState, ensure any
running auto-refresh timer is stopped: check state.autoRefreshTimer (or call an
existing stop/disable helper such as stopAutoRefresh()/toggleAutoRefreshOff) and
cancel it and clear the reference so the periodic Timer won't continue after
state = const ServersState(); then proceed to remove SSH sessions and reset
Stores; update the block around state = const ServersState() (referencing state,
autoRefreshTimer, isAutoRefreshOn, and any stopAutoRefresh/toggleAutoRefresh
methods) to cancel the timer and set isAutoRefreshOn to false (or null) before
assigning the new ServersState to avoid duplicate timers later.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 42d18ddd-29dd-4681-8656-bc1c83f1f405
📒 Files selected for processing (1)
lib/data/provider/server/all.dart
1 participant
Resolve #1070.
Resolve #471.
Summary by CodeRabbit
New Features
Localization
Chores