STACKITGit/cli
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge pull request #4564 from cli/e-flag

Browse source 
gh cs cp: disable remote shell expansion unless -e flag
This commit is contained in:
Alan Donovan 2021-10-19 11:38:19 -04:00 • committed by GitHub
commit fc1de3aaa0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 35 additions and 8 deletions
Download patch file Download diff file Expand all files Collapse all files

6
internal/codespaces/ssh.go
View file

@ -27,8 +27,10 @@ func Shell(ctx context.Context, log logger, sshArgs []string, port int, destinat
// Copy runs an scp command over the specified port. The arguments may
// include flags and non-flags, optionally separated by "--".
// Remote files are indicated by a "remote:" prefix, and are resolved
// relative to the remote user's home directory.
//
// Remote files indicated by a "remote:" prefix are resolved relative
// to the remote user's home directory, and are subject to shell expansion
// on the remote host; see https://lwn.net/Articles/835962/.
func Copy(ctx context.Context, scpArgs []string, port int, destination string) error {
// Beware: invalid syntax causes scp to exit 1 with
// no error message, so don't let that happen.

37
pkg/cmd/codespace/ssh.go
View file

@ -145,26 +145,39 @@ func (a *App) SSH(ctx context.Context, sshArgs []string, opts sshOptions) (err e
type cpOptions struct {
sshOptions
recursive bool // -r
expand bool // -e
}
func newCpCmd(app *App) *cobra.Command {
var opts cpOptions
cpCmd := &cobra.Command{
Use: "cp [-r] srcs... dest",
Use: "cp [-e] [-r] srcs... dest",
Short: "Copy files between local and remote file systems",
Long: `
The cp command copies files between the local and remote file systems.
A 'remote:' prefix on any file name argument indicates that it refers to
the file system of the remote (Codespace) machine. It is resolved relative
to the home directory of the remote user.
As with the UNIX cp command, the first argument specifies the source and the last
specifies the destination; additional sources may be specified after the first,
if the destination is a directory.
The -r (recursive) flag is required if any source is a directory.
A 'remote:' prefix on any file name argument indicates that it refers to
the file system of the remote (Codespace) machine. It is resolved relative
to the home directory of the remote user.
By default, remote file names are interpreted literally. With the -e flag,
each such argument is treated in the manner of scp, as a Bash expression to
be evaluated on the remote machine, subject to expansion of tildes, braces,
globs, environment variables, and backticks, as in these examples:
$ gh codespace cp -e README.md 'remote:/workspace/$RepositoryName/'
$ gh codespace cp -e 'remote:~/*.go' ./gofiles/
$ gh codespace cp -e 'remote:/workspace/myproj/go.{mod,sum}' ./gofiles/
For security, do not use the -e flag with arguments provided by untrusted
users; see https://lwn.net/Articles/835962/ for discussion.
`,
RunE: func(cmd *cobra.Command, args []string) error {
return app.Copy(cmd.Context(), args, opts)
@ -173,6 +186,7 @@ The -r (recursive) flag is required if any source is a directory.
// We don't expose all sshOptions.
cpCmd.Flags().BoolVarP(&opts.recursive, "recursive", "r", false, "Recursively copy directories")
cpCmd.Flags().BoolVarP(&opts.expand, "expand", "e", false, "Expand remote file names on remote shell")
cpCmd.Flags().StringVarP(&opts.codespace, "codespace", "c", "", "Name of the codespace")
return cpCmd
}
@ -188,7 +202,18 @@ func (a *App) Copy(ctx context.Context, args []string, opts cpOptions) (err erro
}
opts.scpArgs = append(opts.scpArgs, "--")
for _, arg := range args {
if !filepath.IsAbs(arg) && !strings.HasPrefix(arg, "remote:") {
if rest := strings.TrimPrefix(arg, "remote:"); rest != arg {
// scp treats each filename argument as a shell expression,
// subjecting it to expansion of environment variables, braces,
// tilde, backticks, globs and so on. Because these present a
// security risk (see https://lwn.net/Articles/835962/), we
// disable them by shell-escaping the argument unless the user
// provided the -e flag.
if !opts.expand {
arg = `remote:'` + strings.Replace(rest, `'`, `'\''`, -1) + `'`
}
} else if !filepath.IsAbs(arg) {
// scp treats a colon in the first path segment as a host identifier.
// Escape it by prepending "./".
// TODO(adonovan): test on Windows, including with a c:\\foo path.