Skip to content

Commit 39dc654

Browse files
fix(ssh): remove unreachable code in connectOrDie, wrap last error (#655)
After the retry loop, connectionFailed is always true, making the final return statement unreachable. Simplify to unconditionally return the error after the loop. Also wrap the last dial error with %w so callers see the root cause (e.g. 'connection refused'). Audit findings #5 (HIGH), #6 (HIGH), #27 (LOW). Signed-off-by: Carlos Eduardo Arango Gutierrez <eduardoa@nvidia.com>
1 parent 6e63f12 commit 39dc654

File tree

2 files changed

+7
-20
lines changed

2 files changed

+7
-20
lines changed

cmd/cli/dryrun/dryrun.go

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -147,22 +147,16 @@ func connectOrDie(keyPath, userName, hostUrl string) error {
147147
HostKeyCallback: sshutil.TOFUHostKeyCallback(),
148148
}
149149

150-
connectionFailed := false
150+
var lastErr error
151151
for range 20 {
152152
client, err := ssh.Dial("tcp", hostUrl+":22", sshConfig)
153153
if err == nil {
154-
client.Close() // nolint:errcheck, gosec
155-
return nil // Connection succeeded,
154+
_ = client.Close()
155+
return nil
156156
}
157-
connectionFailed = true
158-
// Sleep for a brief moment before retrying.
159-
// You can adjust the duration based on your requirements.
157+
lastErr = err
160158
time.Sleep(1 * time.Second)
161159
}
162160

163-
if connectionFailed {
164-
return fmt.Errorf("failed to connect to %s", hostUrl)
165-
}
166-
167-
return nil
161+
return fmt.Errorf("failed to connect to %s after 20 retries: %w", hostUrl, lastErr)
168162
}

pkg/provisioner/provisioner.go

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -464,20 +464,13 @@ func connectOrDie(keyPath, userName, hostUrl string) (*ssh.Client, error) {
464464
HostKeyCallback: sshutil.TOFUHostKeyCallback(),
465465
}
466466

467-
connectionFailed := false
468467
for range sshMaxRetries {
469468
client, err = ssh.Dial("tcp", hostUrl+":22", sshConfig)
470469
if err == nil {
471-
return client, nil // Connection succeeded, return the client.
470+
return client, nil
472471
}
473-
connectionFailed = true
474-
// Brief delay before retrying.
475472
time.Sleep(sshRetryDelay)
476473
}
477474

478-
if connectionFailed {
479-
return nil, fmt.Errorf("failed to connect to %s after %d retries, giving up", hostUrl, sshMaxRetries)
480-
}
481-
482-
return client, nil
475+
return nil, fmt.Errorf("failed to connect to %s after %d retries: %w", hostUrl, sshMaxRetries, err)
483476
}

0 commit comments

Comments
 (0)