Skip to content

Commit

Permalink
Prevent reauthentication for tilde prefix expansion errors
Browse files Browse the repository at this point in the history
In #24254 expansion
of tilde prefixes(i.e. `tsh scp CHANGELOG.md root@host:~foo/`) was
explicitly denied. However, because the error returned to the user
is a trace.BadParameter error the reauthentication logic kicks in
and attempts to resolve the issue with fresh credentials. This
error is now caught and wrapped in a NonRetryableError to prevent
the authentication logic from providing a weird UX.

Updates #22886.
  • Loading branch information
rosstimothy committed Jan 24, 2025
1 parent c8d8c21 commit 735e897
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 4 deletions.
10 changes: 9 additions & 1 deletion lib/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -722,7 +722,15 @@ func (c *NodeClient) TransferFiles(ctx context.Context, cfg *sftp.Config) error
)
defer span.End()

return trace.Wrap(cfg.TransferFiles(ctx, c.Client.Client))
if err := cfg.TransferFiles(ctx, c.Client.Client); err != nil {
if trace.IsBadParameter(err) && strings.Contains(err.Error(), "expanding remote ~user paths is not supported") {
return trace.Wrap(&NonRetryableError{Err: err})
}

return trace.Wrap(err)
}

return nil
}

type netDialer interface {
Expand Down
6 changes: 3 additions & 3 deletions lib/sshutils/sftp/sftp.go
Original file line number Diff line number Diff line change
Expand Up @@ -329,15 +329,15 @@ func (c *Config) expandPaths(srcIsRemote, dstIsRemote bool) (err error) {
for i, srcPath := range c.srcPaths {
c.srcPaths[i], err = expandPath(srcPath)
if err != nil {
return trace.Wrap(err, "error expanding %q", srcPath)
return trace.Wrap(err)
}
}
}

if dstIsRemote {
c.dstPath, err = expandPath(c.dstPath)
if err != nil {
return trace.Wrap(err, "error expanding %q", c.dstPath)
return trace.Wrap(err)
}
}

Expand All @@ -358,7 +358,7 @@ func expandPath(pathStr string) (string, error) {
return ".", nil
}
if pfxLen == 1 && len(pathStr) > 1 {
return "", trace.BadParameter("expanding remote ~user paths is not supported, specify an absolute path instead")
return "", trace.BadParameter("expanding remote ~user paths is not supported, specify an absolute path instead of %q", pathStr)
}

// if an SFTP path is not absolute, it is assumed to start at the user's
Expand Down

0 comments on commit 735e897

Please sign in to comment.