fix(container): Parsing results in sudo mode #1031
Conversation
📝 WalkthroughWalkthroughAdds non-interactive sudo password handling: new SSH client API returns (exitCode, output) detecting sudo password errors; container provider caches/pipes base64-encoded password into sudo, maps password error codes to new enum cases, and adds localized messages. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ContainerProvider as Container Provider
participant PasswordCache as Password Cache
participant SSHClient as SSH Client
participant ErrorHandler as Error Handler
User->>ContainerProvider: Request run (sudo required)
ContainerProvider->>PasswordCache: Check cached password
alt not cached
ContainerProvider->>User: Prompt for password (_getSudoPassword)
User-->>ContainerProvider: Enter password
ContainerProvider->>PasswordCache: Cache password
end
ContainerProvider->>ContainerProvider: _buildSudoCmd(password)
ContainerProvider->>SSHClient: execWithPwd(command, id, onStdout, onStderr)
SSHClient-->>ContainerProvider: (exitCode, output)
ContainerProvider->>ErrorHandler: Evaluate exitCode/output
alt exitCode == 2
ErrorHandler->>User: Emit sudoPasswordIncorrect
ContainerProvider->>PasswordCache: Clear cache
else
ErrorHandler->>User: Return command result
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 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)
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (1)lib/data/provider/**/*.dart📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (5)📓 Common learnings📚 Learning: 2026-01-29T08:50:01.572ZApplied to files:
📚 Learning: 2026-01-17T04:24:16.501ZApplied to files:
📚 Learning: 2025-10-08T09:36:41.682ZApplied to files:
📚 Learning: 2026-01-22T05:10:54.502ZApplied to files:
🔇 Additional comments (8)
✏️ Tip: You can disable this entire section by setting 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 |
Summary of ChangesHello @GT-610, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the application's ability to interact with container runtimes (Docker, Podman) when elevated privileges via Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@lib/data/provider/container.dart`:
- Around line 139-143: When handling the sudo-password incorrect case in the
error branch that checks for exit code 2 (where state is set with
ContainerErr(type: ContainerErrType.sudoPasswordIncorrect)), also clear the
cached password so subsequent calls to _getSudoPassword() don't return the
invalid value; locate the block around the exit code check in container.dart and
set _cachedPassword to null (or empty) before returning, leaving the
state.copyWith(...) line as-is so the error is still reported.
- Around line 134-137: The execWithPwd helper currently calls exec(...) with
stderr: false so error messages from Docker/Podman end up excluded and the
container-not-found checks (e.g., raw.contains(_dockerNotFound) in the container
state update and the 'podman: not found' check) can miss stderr-only messages;
update the exec(...) call inside execWithPwd to set stderr: true so both stdout
and stderr are captured and the existing ContainerErr detection logic in the
container.dart functions will work reliably. Ensure the returned raw string used
by the checks still concatenates both streams if exec returns separate values.
🧹 Nitpick comments (2)
lib/core/extension/ssh_client.dart (1)
138-144: Remove unnecessaryasynckeyword from callback.The
onStderrcallback is markedasyncbut contains noawaitexpressions. This adds unnecessary overhead and can be simplified.♻️ Suggested fix
- onStderr: (data, session) async { // stderr is in `data` + onStderr: (data, session) { onStderr?.call(data, session); if (data.contains('Sorry, try again.') || data.contains('incorrect password attempt')) { hasPasswordError = true; } },lib/data/provider/container.dart (1)
312-316: Dead code:else if (needSudo)branch is unreachable.If
needSudois true andpasswordis null, the function returns early at lines 307-309. Therefore, theelse if (needSudo)branch at line 314 can never execute.♻️ Proposed fix
- if (needSudo && password != null) { + if (needSudo) { + // password is guaranteed non-null here due to early return above cmd = _buildSudoCmd(cmd, password); - } else if (needSudo) { - cmd = 'sudo -S $cmd'; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
lib/generated/l10n/l10n.dartis excluded by!**/generated/**lib/generated/l10n/l10n_de.dartis excluded by!**/generated/**lib/generated/l10n/l10n_en.dartis excluded by!**/generated/**lib/generated/l10n/l10n_es.dartis excluded by!**/generated/**lib/generated/l10n/l10n_fr.dartis excluded by!**/generated/**lib/generated/l10n/l10n_id.dartis excluded by!**/generated/**lib/generated/l10n/l10n_ja.dartis excluded by!**/generated/**lib/generated/l10n/l10n_nl.dartis excluded by!**/generated/**lib/generated/l10n/l10n_pt.dartis excluded by!**/generated/**lib/generated/l10n/l10n_ru.dartis excluded by!**/generated/**lib/generated/l10n/l10n_tr.dartis excluded by!**/generated/**lib/generated/l10n/l10n_uk.dartis excluded by!**/generated/**lib/generated/l10n/l10n_zh.dartis excluded by!**/generated/**
📒 Files selected for processing (5)
lib/core/extension/ssh_client.dartlib/data/model/app/error.dartlib/data/provider/container.dartlib/l10n/app_en.arblib/l10n/app_zh.arb
🧰 Additional context used
📓 Path-based instructions (3)
lib/data/model/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
After modifying annotated model files, run: dart run build_runner build --delete-conflicting-outputs
Files:
lib/data/model/app/error.dart
lib/l10n/**/*.arb
📄 CodeRabbit inference engine (CLAUDE.md)
lib/l10n/**/*.arb: After modifying ARB files, run: flutter gen-l10n to regenerate localization
Keep localized strings in ARB files under lib/l10n/ (Flutter i18n)
Files:
lib/l10n/app_en.arblib/l10n/app_zh.arb
lib/data/provider/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Organize Riverpod providers by feature under lib/data/provider/
Files:
lib/data/provider/container.dart
🧠 Learnings (4)
📓 Common learnings
Learnt from: GT-610
Repo: lollipopkit/flutter_server_box PR: 0
File: :0-0
Timestamp: 2026-01-17T04:24:16.501Z
Learning: In lib/core/extension/ssh_client.dart and lib/data/provider/container.dart, when executing Docker/Podman commands via SSH, stderr must be included in the output (not set to stderr: false) because the error detection logic relies on stderr messages like "command not found" to properly detect when Docker/Podman is not installed and display appropriate error prompts to users.
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/data/model/container/**/*.dart : Place container/Docker models under lib/data/model/container/
Applied to files:
lib/data/model/app/error.dartlib/data/provider/container.dart
📚 Learning: 2026-01-17T04:24:16.501Z
Learnt from: GT-610
Repo: lollipopkit/flutter_server_box PR: 0
File: :0-0
Timestamp: 2026-01-17T04:24:16.501Z
Learning: In lib/core/extension/ssh_client.dart and lib/data/provider/container.dart, when executing Docker/Podman commands via SSH, stderr must be included in the output (not set to stderr: false) because the error detection logic relies on stderr messages like "command not found" to properly detect when Docker/Podman is not installed and display appropriate error prompts to users.
Applied to files:
lib/data/model/app/error.dartlib/data/provider/container.dartlib/core/extension/ssh_client.dart
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/data/model/ssh/**/*.dart : Place SSH models under lib/data/model/ssh/
Applied to files:
lib/core/extension/ssh_client.dart
🔇 Additional comments (6)
lib/data/model/app/error.dart (1)
30-31: LGTM!The new enum variants
sudoPasswordRequiredandsudoPasswordIncorrectproperly extend the error type system for sudo password handling. They align with the corresponding localization keys and error handling incontainer.dart.lib/l10n/app_en.arb (1)
61-62: LGTM!The new localization strings are clear and provide helpful guidance to users encountering sudo password prompts.
Remember to run
flutter gen-l10nto regenerate localization files after merging. As per coding guidelines: "After modifying ARB files, run: flutter gen-l10n to regenerate localization".lib/l10n/app_zh.arb (1)
61-62: LGTM!Chinese translations are accurate and maintain localization parity with the English strings.
lib/core/extension/ssh_client.dart (1)
111-154: LGTM - Well-documented new method.The
execWithPwdmethod is well-designed with clear documentation explaining the exit codes and stderr handling rationale. The password error detection via stderr inspection is a good approach for non-interactive sudo usage.lib/data/provider/container.dart (2)
351-354: LGTM - Secure password piping approach.Using base64 encoding to pipe the password into
sudo -Sis a reasonable approach for non-interactive sudo usage. While the base64-encoded password is visible in process listings, this is a common and accepted pattern that avoids plain-text passwords in command arguments.
53-63: LGTM - Password caching implementation.The password caching approach with null-check and dialog prompt is well-structured. It correctly handles the empty string case by only caching non-empty passwords.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the logic for executing commands with sudo, which is a great improvement for handling Docker access in various environments. The new approach of piping the password to sudo -S is more robust and less prone to parsing errors.
I've identified a couple of areas for improvement. One is a minor issue regarding an unused parameter in execPowerShell which could cause confusion. The other is a more significant regression where error details from stderr are no longer being captured, which can make debugging failed commands difficult. Please see my detailed comments.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@lib/core/extension/ssh_client.dart`:
- Around line 111-122: The doc comment for the method that calls exec()
incorrectly states "[stderr: false] ensures only stdout...", but the exec()
invocation (exec()) does not pass stderr:false and therefore includes stderr by
default; update the method's documentation to state that stderr is included in
the returned output (exec defaults to stderr:true) and that stderr is used for
error detection, or alternatively change the exec() call to explicitly pass
stderr:false if the intent is to exclude stderr—reference the exec() call in
this method when making the change.
In `@lib/data/provider/container.dart`:
- Around line 350-353: The current _buildSudoCmd returns a shell string that
discards stderr (2>/dev/null), which hides errors like "docker: not found" and
breaks detection in execWithPwd and the checks against _dockerNotFound and
'podman: not found'; update _buildSudoCmd to pipe the base64 password as before
but redirect stderr into stdout (e.g., use 2>&1) instead of discarding so stderr
messages are captured in SSH output for detection while still relying on
execWithPwd's exit-code/password handling to avoid interactive prompts.
🧹 Nitpick comments (3)
lib/core/extension/ssh_client.dart (1)
138-143: Remove unnecessaryasynckeyword from callback.The
onStderrcallback is markedasyncbut contains noawaitexpressions. This is harmless but misleading.♻️ Proposed fix
- onStderr: (data, session) async { // stderr is in `data` + onStderr: (data, session) { // stderr is in `data` onStderr?.call(data, session); if (data.contains('Sorry, try again.') || data.contains('incorrect password attempt')) { hasPasswordError = true; } },lib/data/provider/container.dart (2)
318-331: Consider including actual error output in the error message.When the command fails (code != 0), the error message is generic. The actual output from the command (captured in the discarded
_variable at line 318) could provide useful debugging information for users.♻️ Proposed fix
- final (code, _) = await client!.execWithPwd( + final (code, output) = await client!.execWithPwd( _wrap(cmd), context: context, onStdout: (data, _) { state = state.copyWith(runLog: '${state.runLog}$data'); }, id: hostId, ); state = state.copyWith(runLog: null); if (code != 0) { - return ContainerErr(type: ContainerErrType.unknown, message: 'Command execution failed'); + return ContainerErr(type: ContainerErrType.unknown, message: output.isNotEmpty ? output : 'Command execution failed'); }
382-388: Optional: Improve defensive programming forexec()method.The
sudo -S $baseCmdfallback on line 386 can only be reached ifsudo=trueandpassword=null. While current callers (refresh()on line 115 and the one direct call on line 79) validate that password is non-null when sudo is required, this creates implicit coupling.Consider either:
- Making password non-nullable when sudo is true (e.g.,
required String passwordifsudo=true)- Throwing an error instead of the
sudo -Sfallback, making the API contract explicitThis prevents accidental misuse if the method is called directly in the future.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/core/extension/ssh_client.dartlib/data/provider/container.dart
🧰 Additional context used
📓 Path-based instructions (1)
lib/data/provider/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Organize Riverpod providers by feature under lib/data/provider/
Files:
lib/data/provider/container.dart
🧠 Learnings (4)
📓 Common learnings
Learnt from: GT-610
Repo: lollipopkit/flutter_server_box PR: 0
File: :0-0
Timestamp: 2026-01-17T04:24:16.501Z
Learning: In lib/core/extension/ssh_client.dart and lib/data/provider/container.dart, when executing Docker/Podman commands via SSH, stderr must be included in the output (not set to stderr: false) because the error detection logic relies on stderr messages like "command not found" to properly detect when Docker/Podman is not installed and display appropriate error prompts to users.
📚 Learning: 2026-01-17T04:24:16.501Z
Learnt from: GT-610
Repo: lollipopkit/flutter_server_box PR: 0
File: :0-0
Timestamp: 2026-01-17T04:24:16.501Z
Learning: In lib/core/extension/ssh_client.dart and lib/data/provider/container.dart, when executing Docker/Podman commands via SSH, stderr must be included in the output (not set to stderr: false) because the error detection logic relies on stderr messages like "command not found" to properly detect when Docker/Podman is not installed and display appropriate error prompts to users.
Applied to files:
lib/core/extension/ssh_client.dartlib/data/provider/container.dart
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/data/model/ssh/**/*.dart : Place SSH models under lib/data/model/ssh/
Applied to files:
lib/core/extension/ssh_client.dart
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/data/model/container/**/*.dart : Place container/Docker models under lib/data/model/container/
Applied to files:
lib/data/provider/container.dart
🔇 Additional comments (6)
lib/core/extension/ssh_client.dart (1)
46-47: LGTM!Comment clarifies that stderr is intentionally excluded from the result in PowerShell execution, which aligns with the documented behavior.
lib/data/provider/container.dart (5)
40-40: LGTM!Password caching field introduced to avoid repeated prompts during a session.
53-63: LGTM!The
_getSudoPasswordmethod properly checks for mounted context before showing dialog and caches the password only when non-empty.
100-111: LGTM!Good defensive handling: when sudo is required but password is not provided (user cancels dialog), the method returns early with an appropriate error state instead of proceeding with a null password.
139-144: LGTM!Good fix: the cached password is now cleared when an incorrect password error is detected (exit code 2), allowing the user to retry with a correct password.
391-395: LGTM!The
execAllmethod properly passes through the password parameter to each individualexeccall.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@lib/data/provider/container.dart`:
- Around line 360-363: The _buildSudoCmd currently pipes a base64-decoded
password into sudo -S but doesn't suppress sudo's stderr prompt; update
_buildSudoCmd to append shell-level stderr redirection (e.g., 2>/dev/null) to
the returned command so the sudo prompt won't leak into captured stderr, and
modify the sudo+password sh-wrapper branch in execAll (the code path that builds
a shell wrapper when a password is provided) to call and reuse _buildSudoCmd
instead of duplicating logic so both paths get the same 2>/dev/null suppression;
ensure execWithPwd's stderr capture (raw) no longer receives the sudo prompt.
- Around line 406-424: The debug prints currently leak credentials by logging
the constructed sudo command that includes the base64-encoded password; in the
block that checks needsShWrapper and the branches using sudo && password != null
(referencing variables sudo, password, pwdBase64, cmd, and commands) remove or
redact any print/log lines that would output the full cmd containing the
password (and do the same for the analogous prints further down), or replace
them with a non-sensitive message such as indicating a sudo command was
constructed without including pwdBase64 or the raw command string.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
lib/core/extension/ssh_client.dartlib/data/provider/container.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/core/extension/ssh_client.dart
🧰 Additional context used
📓 Path-based instructions (1)
lib/data/provider/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Organize Riverpod providers by feature under lib/data/provider/
Files:
lib/data/provider/container.dart
🧠 Learnings (4)
📓 Common learnings
Learnt from: GT-610
Repo: lollipopkit/flutter_server_box PR: 1031
File: lib/data/provider/container.dart:350-353
Timestamp: 2026-01-28T16:13:47.416Z
Learning: In lib/data/provider/container.dart's _buildSudoCmd function, using shell-level stderr redirect (2>/dev/null) when piping password to sudo -S does not break Docker/Podman "not installed" detection because the detection primarily relies on exit code 127 (command not found) rather than stderr messages. The 2>/dev/null is necessary to suppress sudo password prompts that would otherwise contaminate stdout and cause parsing failures.
Learnt from: GT-610
Repo: lollipopkit/flutter_server_box PR: 0
File: :0-0
Timestamp: 2026-01-17T04:24:16.501Z
Learning: In lib/core/extension/ssh_client.dart and lib/data/provider/container.dart, when executing Docker/Podman commands via SSH, stderr must be included in the output (not set to stderr: false) because the error detection logic relies on stderr messages like "command not found" to properly detect when Docker/Podman is not installed and display appropriate error prompts to users.
📚 Learning: 2026-01-28T16:13:47.416Z
Learnt from: GT-610
Repo: lollipopkit/flutter_server_box PR: 1031
File: lib/data/provider/container.dart:350-353
Timestamp: 2026-01-28T16:13:47.416Z
Learning: In lib/data/provider/container.dart, for the _buildSudoCmd function, use shell-level stderr redirection (2>/dev/null) when piping a password to sudo -S to suppress password prompts that could pollute stdout and break parsing. This detection relies on the exit code (e.g., 127 for command not found) rather than stderr messages, so redirecting stderr helps maintain reliable stdout parsing. Ensure this change does not alter the actual exit codes and that error handling remains correct.
Applied to files:
lib/data/provider/container.dart
📚 Learning: 2026-01-17T04:24:16.501Z
Learnt from: GT-610
Repo: lollipopkit/flutter_server_box PR: 0
File: :0-0
Timestamp: 2026-01-17T04:24:16.501Z
Learning: In lib/core/extension/ssh_client.dart and lib/data/provider/container.dart, when executing Docker/Podman commands via SSH, stderr must be included in the output (not set to stderr: false) because the error detection logic relies on stderr messages like "command not found" to properly detect when Docker/Podman is not installed and display appropriate error prompts to users.
Applied to files:
lib/data/provider/container.dart
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/data/model/container/**/*.dart : Place container/Docker models under lib/data/model/container/
Applied to files:
lib/data/provider/container.dart
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Add support for sudo password verification for Docker container operations, including: 1. Added ContainerErrType.sudoPasswordRequired error type 2. Add password prompt text in multi-language files 3. Modify the SSH execution logic to correctly handle the input of sudo password 4. Implement password caching and verification mechanism
Add a new error type `sudoPasswordIncorrect` to handle situations where the sudo password is incorrect Modify the password verification logic in the SSH client, and return a specific error code when a password error is detected Update multilingual files to support password error prompt information
…d handling Clean up the no longer needed stderr parameter in the SSH client, which was originally used to handle sudo password prompts Unify the sudo command construction logic, always use the _buildSudoCmd method, and add stderr redirection Clear cached passwords when passwords are incorrect
…detection Fix the sudo command processing logic, remove the masking of stderr to capture password errors Override the detection logic simulated by Podman Refactor the command building logic to support sh wrapping of multi-line commands
Update the sudo password error prompt messages for all languages to more accurately reflect situations of incorrect password or lack of permission Fix the password error detection logic for both the SSH client and container providers simultaneously
GT-610
force-pushed
the
fix/container/sudo-parsing
branch
from
421b4e6 to
0d80cd8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@lib/data/provider/container.dart`:
- Around line 404-406: The branch that returns 'sudo -S $baseCmd' when sudo is
true can hang if password is null; update the code where that branch lives (the
method that constructs the command containing "if (sudo) { return 'sudo -S
$baseCmd'; }") to validate state: if sudo is true but password is null/empty,
throw an ArgumentError or assert (or otherwise fail fast) rather than returning
a sudo command that expects stdin; keep callers exec(), execAll(), refresh(),
and run() assumptions intact but enforce the invariant here so the API cannot
produce a hanging sudo invocation.
🧹 Nitpick comments (2)
lib/core/extension/ssh_client.dart (1)
141-144: Password detection strings may be locale-dependent.The detection relies on English error messages from sudo (
"Sorry, try again.","incorrect password attempt","a password is required"). On systems with non-English locales configured, sudo may output localized error messages, causing detection to fail silently.Consider adding a note in documentation about this limitation, or explore detecting patterns that are more universal (e.g., checking exit codes combined with stderr presence).
lib/data/provider/container.dart (1)
415-439: Unreachable code and duplicated logic.
Dead code (lines 431-439): Since
ContainerCmdType.valuesalways has 4 enum values,commandswill always contain\necho ${ScriptConstants.separator}\n. This meansneedsShWrapperis alwaystrue, making the non-wrapper branches unreachable.Duplicated logic (lines 419-420): The base64 encoding logic is duplicated from
_buildSudoCmd().♻️ Proposed refactor
static String execAll(ContainerType type, {bool sudo = false, bool includeStats = false, String? password}) { final commands = ContainerCmdType.values .map((e) => e.exec(type, sudo: false, includeStats: includeStats)) .join('\necho ${ScriptConstants.separator}\n'); - final needsShWrapper = commands.contains('\n') || commands.contains('echo ${ScriptConstants.separator}'); - - if (needsShWrapper) { - if (sudo && password != null) { - final pwdBase64 = base64Encode(utf8.encode(password)); - final cmd = 'echo "$pwdBase64" | base64 -d | sudo -S sh -c \'${commands.replaceAll("'", "'\\''")}\''; - return cmd; - } - if (sudo) { - final cmd = 'sudo -S sh -c \'${commands.replaceAll("'", "'\\''")}\''; - return cmd; - } - final cmd = 'sh -c \'${commands.replaceAll("'", "'\\''")}\''; - return cmd; - } - - if (sudo && password != null) { - final cmd = _buildSudoCmd(commands, password); - return cmd; - } - if (sudo) { - final cmd = 'sudo -S $commands'; - return cmd; - } - return commands; + final wrappedCommands = 'sh -c \'${commands.replaceAll("'", "'\\''")}\''; + + if (sudo && password != null) { + return _buildSudoCmd(wrappedCommands, password); + } + if (sudo) { + return 'sudo -S $wrappedCommands'; + } + return wrappedCommands; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (13)
lib/generated/l10n/l10n.dartis excluded by!**/generated/**lib/generated/l10n/l10n_de.dartis excluded by!**/generated/**lib/generated/l10n/l10n_en.dartis excluded by!**/generated/**lib/generated/l10n/l10n_es.dartis excluded by!**/generated/**lib/generated/l10n/l10n_fr.dartis excluded by!**/generated/**lib/generated/l10n/l10n_id.dartis excluded by!**/generated/**lib/generated/l10n/l10n_ja.dartis excluded by!**/generated/**lib/generated/l10n/l10n_nl.dartis excluded by!**/generated/**lib/generated/l10n/l10n_pt.dartis excluded by!**/generated/**lib/generated/l10n/l10n_ru.dartis excluded by!**/generated/**lib/generated/l10n/l10n_tr.dartis excluded by!**/generated/**lib/generated/l10n/l10n_uk.dartis excluded by!**/generated/**lib/generated/l10n/l10n_zh.dartis excluded by!**/generated/**
📒 Files selected for processing (16)
lib/core/extension/ssh_client.dartlib/data/model/app/error.dartlib/data/provider/container.dartlib/l10n/app_de.arblib/l10n/app_en.arblib/l10n/app_es.arblib/l10n/app_fr.arblib/l10n/app_id.arblib/l10n/app_ja.arblib/l10n/app_nl.arblib/l10n/app_pt.arblib/l10n/app_ru.arblib/l10n/app_tr.arblib/l10n/app_uk.arblib/l10n/app_zh.arblib/l10n/app_zh_tw.arb
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/l10n/app_zh.arb
- lib/data/model/app/error.dart
🧰 Additional context used
📓 Path-based instructions (2)
lib/l10n/**/*.arb
📄 CodeRabbit inference engine (CLAUDE.md)
lib/l10n/**/*.arb: After modifying ARB files, run: flutter gen-l10n to regenerate localization
Keep localized strings in ARB files under lib/l10n/ (Flutter i18n)
Files:
lib/l10n/app_es.arblib/l10n/app_fr.arblib/l10n/app_ru.arblib/l10n/app_de.arblib/l10n/app_id.arblib/l10n/app_pt.arblib/l10n/app_tr.arblib/l10n/app_ja.arblib/l10n/app_nl.arblib/l10n/app_uk.arblib/l10n/app_zh_tw.arblib/l10n/app_en.arb
lib/data/provider/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Organize Riverpod providers by feature under lib/data/provider/
Files:
lib/data/provider/container.dart
🧠 Learnings (7)
📓 Common learnings
Learnt from: GT-610
Repo: lollipopkit/flutter_server_box PR: 1031
File: lib/data/provider/container.dart:360-363
Timestamp: 2026-01-29T08:50:01.572Z
Learning: In lib/data/provider/container.dart's _buildSudoCmd function, do NOT use shell-level stderr redirect (2>/dev/null) when piping password to sudo -S. Shell-level suppression would discard stderr before the SSH client captures it, breaking the onStderr callback in execWithPwd that detects sudo password errors (e.g., "Sorry, try again.") and Podman emulation messages. Instead, use the layered approach: keep stderr flowing at shell level, capture it via onStderr callback at SSH level, and filter it from the returned result using stderr: false at application level in execWithPwd.
Learnt from: GT-610
Repo: lollipopkit/flutter_server_box PR: 0
File: :0-0
Timestamp: 2026-01-17T04:24:16.501Z
Learning: In lib/core/extension/ssh_client.dart and lib/data/provider/container.dart, when executing Docker/Podman commands via SSH, stderr must be included in the output (not set to stderr: false) because the error detection logic relies on stderr messages like "command not found" to properly detect when Docker/Podman is not installed and display appropriate error prompts to users.
📚 Learning: 2026-01-29T08:50:01.572Z
Learnt from: GT-610
Repo: lollipopkit/flutter_server_box PR: 1031
File: lib/data/provider/container.dart:360-363
Timestamp: 2026-01-29T08:50:01.572Z
Learning: In lib/data/provider/container.dart's _buildSudoCmd function, do NOT use shell-level stderr redirect (2>/dev/null) when piping password to sudo -S. Shell-level suppression would discard stderr before the SSH client captures it, breaking the onStderr callback in execWithPwd that detects sudo password errors (e.g., "Sorry, try again.") and Podman emulation messages. Instead, use the layered approach: keep stderr flowing at shell level, capture it via onStderr callback at SSH level, and filter it from the returned result using stderr: false at application level in execWithPwd.
Applied to files:
lib/l10n/app_es.arblib/l10n/app_de.arblib/core/extension/ssh_client.dart
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/l10n/**/*.arb : Keep localized strings in ARB files under lib/l10n/ (Flutter i18n)
Applied to files:
lib/l10n/app_fr.arblib/l10n/app_id.arblib/l10n/app_en.arb
📚 Learning: 2026-01-17T04:24:16.501Z
Learnt from: GT-610
Repo: lollipopkit/flutter_server_box PR: 0
File: :0-0
Timestamp: 2026-01-17T04:24:16.501Z
Learning: In lib/core/extension/ssh_client.dart and lib/data/provider/container.dart, when executing Docker/Podman commands via SSH, stderr must be included in the output (not set to stderr: false) because the error detection logic relies on stderr messages like "command not found" to properly detect when Docker/Podman is not installed and display appropriate error prompts to users.
Applied to files:
lib/core/extension/ssh_client.dartlib/data/provider/container.dart
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/data/model/ssh/**/*.dart : Place SSH models under lib/data/model/ssh/
Applied to files:
lib/core/extension/ssh_client.dart
📚 Learning: 2026-01-29T08:50:01.572Z
Learnt from: GT-610
Repo: lollipopkit/flutter_server_box PR: 1031
File: lib/data/provider/container.dart:360-363
Timestamp: 2026-01-29T08:50:01.572Z
Learning: In lib/data/provider/container.dart, within the _buildSudoCmd function, do not redirect stderr at the shell level (e.g., 2>/dev/null) when piping the password to sudo -S. Shell-level suppression can swallow stderr before the SSH client receives it, breaking onStderr callbacks in execWithPwd that detect sudo password errors (such as "Sorry, try again.") and Podman emulation messages. Instead, preserve the shell's stderr, receive it via the SSH-level onStderr callback, and filter it from the final result at the application level by using stderr: false in execWithPwd.
Applied to files:
lib/data/provider/container.dart
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/data/model/container/**/*.dart : Place container/Docker models under lib/data/model/container/
Applied to files:
lib/data/provider/container.dart
🔇 Additional comments (15)
lib/l10n/app_id.arb (1)
60-61: Run localization generation after ARB updates.Please run
flutter gen-l10nso generated localization files pick up the new keys.
As per coding guidelines: After modifying ARB files, run: flutter gen-l10n to regenerate localization.lib/core/extension/ssh_client.dart (3)
46-46: LGTM!Clear comment explaining the intentional exclusion of stderr from the combined result in
execPowerShell.
111-123: LGTM - Documentation accurately describes the layered stderr handling.The documentation now correctly explains the layered approach: stderr is captured via
onStderrcallback for password error detection, while being excluded from the returned output viastderr: false. This aligns with the project's established pattern for sudo password error detection.
132-155: LGTM - Password error detection and exit code handling.The implementation correctly:
- Uses
hasPasswordErrorflag set viaonStderrcallback- Passes
stderr: falseto exclude stderr from returned output (while still capturing via callback)- Returns exit code
2for password errors as documentedThis follows the layered approach from project learnings.
lib/l10n/app_ru.arb (1)
61-62: LGTM - Russian translations added correctly.The new localization keys
containerSudoPasswordRequiredandcontainerSudoPasswordIncorrectare properly added with appropriate Russian translations. As per coding guidelines, remember to runflutter gen-l10nto regenerate localization after modifying ARB files.lib/l10n/app_es.arb (1)
60-61: LGTM - Spanish translations added correctly.The localization keys are properly added with appropriate Spanish translations.
lib/l10n/app_ja.arb (1)
60-61: LGTM - Japanese translations added correctly.The localization keys are properly added with appropriate Japanese translations.
lib/l10n/app_de.arb (1)
61-62: LGTM - German translations added correctly.The localization keys are properly added with appropriate German translations.
lib/l10n/app_fr.arb (1)
60-61: LGTM - French translations added correctly.The localization keys are properly added with appropriate French translations.
lib/l10n/app_en.arb (1)
61-62: LGTM!New localization keys
containerSudoPasswordRequiredandcontainerSudoPasswordIncorrectare well-named and provide clear, actionable messages for users.As per coding guidelines: Remember to run
flutter gen-l10nto regenerate localization files after this change.lib/data/provider/container.dart (5)
53-63: LGTM!The password caching implementation correctly:
- Returns cached password if available
- Checks
context.mountedbefore showing dialog- Only caches non-null, non-empty passwords
100-114: LGTM!The password acquisition flow properly handles the case where sudo is required but no password is provided, setting an appropriate error state and returning early.
152-160: LGTM!Password cache is correctly cleared on authentication failure, and the error state is properly set with a localized message.
Note: Exit code 2 for sudo password failures is common but not guaranteed across all systems/configurations. The current approach should work for most environments.
320-344: LGTM!The
run()method consistently handles sudo password acquisition and command construction using the shared_buildSudoCmdhelper.
369-372: LGTM!The
_buildSudoCmdhelper correctly:
- Uses base64 encoding to safely pass passwords containing special characters
- Does not suppress stderr, allowing the
onStderrcallback to capture sudo errors and Podman emulation messagesBased on learnings: This layered approach (shell → SSH → application) ensures error detection works correctly.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…ec method Simplify the exec method signature by removing the sudo and password parameters that are no longer needed, as these functions are no longer in use
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@lib/data/provider/container.dart`:
- Around line 320-350: The run() path must detect execWithPwd returning exit
code 2 as a sudo-password failure: if (code == 2) clear the cached sudo
credential (reset/complete sudoCompleter or call the existing helper that clears
stored sudo password), set state.runLog back to null, and return a ContainerErr
that specifically signals a sudo/password problem (use
ContainerErrType.sudoPasswordRequired and a localized message such as
l10n.containerSudoPasswordIncorrect or similar) so the bad cached password is
discarded and a clear error is surfaced instead of the generic unknown error.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/data/provider/container.dart
🧰 Additional context used
📓 Path-based instructions (1)
lib/data/provider/**/*.dart
📄 CodeRabbit inference engine (CLAUDE.md)
Organize Riverpod providers by feature under lib/data/provider/
Files:
lib/data/provider/container.dart
🧠 Learnings (4)
📓 Common learnings
Learnt from: GT-610
Repo: lollipopkit/flutter_server_box PR: 1031
File: lib/data/provider/container.dart:360-363
Timestamp: 2026-01-29T08:50:01.572Z
Learning: In lib/data/provider/container.dart's _buildSudoCmd function, do NOT use shell-level stderr redirect (2>/dev/null) when piping password to sudo -S. Shell-level suppression would discard stderr before the SSH client captures it, breaking the onStderr callback in execWithPwd that detects sudo password errors (e.g., "Sorry, try again.") and Podman emulation messages. Instead, use the layered approach: keep stderr flowing at shell level, capture it via onStderr callback at SSH level, and filter it from the returned result using stderr: false at application level in execWithPwd.
Learnt from: GT-610
Repo: lollipopkit/flutter_server_box PR: 0
File: :0-0
Timestamp: 2026-01-17T04:24:16.501Z
Learning: In lib/core/extension/ssh_client.dart and lib/data/provider/container.dart, when executing Docker/Podman commands via SSH, stderr must be included in the output (not set to stderr: false) because the error detection logic relies on stderr messages like "command not found" to properly detect when Docker/Podman is not installed and display appropriate error prompts to users.
📚 Learning: 2026-01-29T08:50:01.572Z
Learnt from: GT-610
Repo: lollipopkit/flutter_server_box PR: 1031
File: lib/data/provider/container.dart:360-363
Timestamp: 2026-01-29T08:50:01.572Z
Learning: In lib/data/provider/container.dart, within the _buildSudoCmd function, do not redirect stderr at the shell level (e.g., 2>/dev/null) when piping the password to sudo -S. Shell-level suppression can swallow stderr before the SSH client receives it, breaking onStderr callbacks in execWithPwd that detect sudo password errors (such as "Sorry, try again.") and Podman emulation messages. Instead, preserve the shell's stderr, receive it via the SSH-level onStderr callback, and filter it from the final result at the application level by using stderr: false in execWithPwd.
Applied to files:
lib/data/provider/container.dart
📚 Learning: 2026-01-17T04:24:16.501Z
Learnt from: GT-610
Repo: lollipopkit/flutter_server_box PR: 0
File: :0-0
Timestamp: 2026-01-17T04:24:16.501Z
Learning: In lib/core/extension/ssh_client.dart and lib/data/provider/container.dart, when executing Docker/Podman commands via SSH, stderr must be included in the output (not set to stderr: false) because the error detection logic relies on stderr messages like "command not found" to properly detect when Docker/Podman is not installed and display appropriate error prompts to users.
Applied to files:
lib/data/provider/container.dart
📚 Learning: 2025-10-08T09:36:41.682Z
Learnt from: CR
Repo: lollipopkit/flutter_server_box PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-10-08T09:36:41.682Z
Learning: Applies to lib/data/model/container/**/*.dart : Place container/Docker models under lib/data/model/container/
Applied to files:
lib/data/provider/container.dart
🔇 Additional comments (7)
lib/data/provider/container.dart (7)
40-63: Good password caching + mounted guard.This avoids repeated prompts and safely skips dialog display when the context is no longer active.
100-115: Graceful sudo-password requirement handling.The early return with a clear error state keeps refresh behavior predictable.
118-133: stderr hook for Podman emulation looks solid.Capturing emulation signals via
onStderrwithout mixing it into parsing output is a clean approach.
147-176: Clear error mapping for not-installed / sudo password / podman emulation.The updated error branches are straightforward and reduce ambiguity for the UI.
369-372: LGTM: no shell-level stderr suppression in_buildSudoCmd.This preserves SSH-level stderr handling for sudo-related diagnostics. Based on learnings: In lib/data/provider/container.dart's _buildSudoCmd function, do NOT use shell-level stderr redirect (2>/dev/null) when piping password to sudo -S; keep stderr flowing and filter at the SSH/application level.
383-401: Simplifiedexec()base command construction looks good.The streamlined signature is clear and keeps command generation focused.
404-434: Password-awareexecAll()construction is consistent.The sudo + password handling across wrapper and non-wrapper paths is clear and readable.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Add two new contributors to the GithubIds list and refactor the container command processing logic: 1. Simplify the command wrapping logic and uniformly use `sh -c` for processing 2. Specific error handling when adding a sudo password incorrectly 3. Remove redundant conditional checks and temporary variables
1 participant
Resolve #969.
Rewrite the whole logic of parsing outputs when accessing Docker though
sudo. This should fix all issues about Docker parsing errors when it is installed in Snap, can't directly be accessed by current user, etc.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.