Conversation
Use os.OpenFile with os.O_EXCL|os.O_CREATE to atomically create the marker file. This eliminates the check-then-act race condition where multiple concurrent cagent processes could all see the marker file as missing and all proceed with first-run initialization. Fixes docker#1709 Assisted-By: cagent
Add input validation to reject empty commands and properly handle the error from exec.CommandContext instead of silently discarding it. When the command fails and produces no output, the error message is shown to the user. Fixes docker#1716 Assisted-By: cagent
Validate that the command is not empty before execution, and add debug-level logging for native (non-sandboxed) shell command execution to improve auditability. Fixes docker#1717 Assisted-By: cagent
![docker-agent[bot]](https://avatars.githubusercontent.com/in/2787757?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
Found one issue in the error handling logic for the RunBangCommand method. The condition for displaying errors will suppress error information when commands produce output but still fail.
The rest of the changes look good - the atomic file creation fix for isFirstRun() properly addresses the race condition mentioned in the comments.
|
|
||
| out, err := exec.CommandContext(ctx, "/bin/sh", "-c", command).CombinedOutput() | ||
| output := "$ " + command + "\n" + string(out) | ||
| if err != nil && len(out) == 0 { |
There was a problem hiding this comment.
Issue: Error information is lost when command fails with partial output
The condition if err != nil && len(out) == 0 means errors are only reported when there's no output. When a shell command produces any output (stdout or stderr) but exits with a non-zero status, the error is never displayed to the user.
For example, if a command writes to stderr and then fails, the user will see the stderr output but won't see that the command actually failed.
Consider changing the logic to always include error information when err != nil:
if err != nil {
output = "$ " + command + "\n" + string(out) + "\nError: " + err.Error()
} else {
output = "$ " + command + "\n" + string(out)
}This ensures users are always informed when a command fails, regardless of whether it produced output.
2 participants
No description provided.