diff --git a/README.md b/README.md index c0bd269..62b7e17 100644 --- a/README.md +++ b/README.md @@ -319,6 +319,16 @@ Put wp-cli arguments after `--`. `raw` joins them with spaces and the server she `eval` and `eval-file` are the one exception: `raw` checks only their shell syntax, never their PHP, and allows `--exec` and `--require` with them. PHP can write files on the server, so take backups and exports with `wpgo db export`, never with `eval`. +When an argument is `-`, the file argument wp-cli reads from stdin, `raw` forwards piped or redirected stdin to wp. It forwards nothing otherwise: + +```bash +wpgo -s mysite raw -- post update 7 - < content.html +``` + +`raw` refuses a `-` argument, before connecting, when stdin is a terminal, `/dev/null` or empty: wp would read nothing and write empty content. On WP Engine an argument that the gateway's second parse turns into `-`, such as `"'-'"`, counts too. `db export -` and `db dump -` are exempt, since there `-` means stdout. `--dry-run` prints the command without reading stdin. + +`raw` never sends stdin to `wp db`: it refuses `-` on any other `db` subcommand, such as `db query -` or `db import -`, and `db query` with no SQL argument or `--execute`. SQL from stdin would skip the checks above, so pass it as an argument or use `wpgo db import`. + These checks stop agents and operators from writing dumps on the server by accident or in passing. `eval` and `eval-file` are the deliberate escape hatch. The checks are not a sandbox against an operator who sets out to get around them. ### Shortcut Commands diff --git a/internal/cmd/raw.go b/internal/cmd/raw.go index b9559c8..ef17b59 100644 --- a/internal/cmd/raw.go +++ b/internal/cmd/raw.go @@ -1,22 +1,32 @@ package cmd import ( + "bufio" "context" + "errors" "fmt" + "io" + "os" + "slices" "strings" "github.com/builtbyrobben/wpssh/internal/adapter" "github.com/builtbyrobben/wpssh/internal/cache" + internalssh "github.com/builtbyrobben/wpssh/internal/ssh" "github.com/builtbyrobben/wpssh/internal/wpcli" ) // Raw command — pass-through escape hatch to wp-cli. type RawCmd struct { - Args []string `arg:"" optional:"" help:"Arguments to pass to wp-cli. Refuses shell operators outside quotes and commands that write dumps or exports on the server."` + Args []string `arg:"" optional:"" help:"Arguments to pass to wp-cli. Forwards piped or redirected stdin to wp only for a \"-\" argument, and refuses that \"-\" when stdin is a terminal or empty or the command is wp db. Refuses shell operators outside quotes and commands that write dumps or exports on the server."` } func (c *RawCmd) Run(g *Globals) error { + return c.runWithIO(g, os.Stdin) +} + +func (c *RawCmd) runWithIO(g *Globals, stdin io.Reader) error { if err := checkRawArgs(c.Args, false); err != nil { return err } @@ -29,18 +39,35 @@ func (c *RawCmd) Run(g *Globals) error { if err != nil { return err } - if adapter.ForSite(site).Name() == "wpengine" { + nested := adapter.ForSite(site).Name() == "wpengine" + if nested { if err := checkRawArgs(c.Args, true); err != nil { return err } } + readsStdin, err := checkRawStdin(rawWordLayers(c.Args, nested)) + if err != nil { + return err + } // Build the raw command by passing all args through. builder := wpcli.New(c.Args...) cmd := builder.Build(site.WPPath) - result, err := rc.ExecWP(context.Background(), site, cmd) - if err != nil { - return err + var result internalssh.ExecResult + if readsStdin && !g.DryRun { + input, err := rawInput(stdin) + if err != nil { + return err + } + result, err = rc.ExecWPStdin(context.Background(), site, cmd, input) + if err != nil { + return err + } + } else { + result, err = rc.ExecWP(context.Background(), site, cmd) + if err != nil { + return err + } } if result.ExitCode != 0 { return fmt.Errorf("wp %s: %s", strings.Join(c.Args, " "), result.Stderr) @@ -53,3 +80,92 @@ func (c *RawCmd) Run(g *Globals) error { fmt.Fprint(rc.Stdout, result.Stdout) return nil } + +// rawWordLayers returns the argvs wp may receive for args: the words the +// remote shell passes and, with nested, as for WP Engine, the words the +// gateway's second parse makes of them. A word that fails the second parse +// is kept as it is; checkRawArgs has already refused it unless it is eval PHP. +func rawWordLayers(args []string, nested bool) [][]string { + words, _ := shellWords(strings.Join(args, " ")) + layers := [][]string{words} + if !nested { + return layers + } + var inner []string + for _, w := range words { + ws, err := shellWords(w) + if err != nil { + ws = []string{w} + } + inner = append(inner, ws...) + } + return append(layers, inner) +} + +// dbStdoutSubcommands are the wp db subcommands that read "-" as stdout. +var dbStdoutSubcommands = map[string]bool{"export": true, "dump": true} + +// checkRawStdin reports whether any layer of argv holds a "-" word that wp +// reads from stdin; db export and dump write to stdout for "-" instead. It +// refuses every wp db command that would read SQL from stdin, as a "-" file +// or as db query without SQL, because raw checks SQL only in arguments. +func checkRawStdin(layers [][]string) (readsStdin bool, err error) { + for _, words := range layers { + dash := slices.Contains(words, "-") + for _, r := range wpReadings { + pos, flags := parseWPArgs(words, r) + if len(pos) < 2 || pos[0] != "db" { + continue + } + if dbStdoutSubcommands[pos[1]] { + dash = false + continue + } + if pos[1] == "query" && len(pos) == 2 && !slices.ContainsFunc(flags, func(f wpFlag) bool { return f.name == "execute" }) { + return false, errors.New("wpgo raw refuses `wp db query` without SQL: wp would read SQL from stdin, which raw neither checks nor forwards; pass the SQL as an argument") + } + } + if !dash { + continue + } + for _, r := range wpReadings { + if pos, _ := parseWPArgs(words, r); len(pos) > 0 && pos[0] == "db" { + return false, errors.New(`wpgo raw refuses a "-" argument on wp db: SQL read from stdin would skip the checks raw applies to SQL arguments; pass the SQL as an argument or use ` + "`wpgo db import`") + } + } + readsStdin = true + } + return readsStdin, nil +} + +// rawInput returns local stdin for wp to read for a "-" argument. It refuses +// a terminal or empty stdin: wp would read EOF and write empty content, such +// as an empty post body. +func rawInput(stdin io.Reader) (io.Reader, error) { + if !isPiped(stdin) { + return nil, errors.New(`wpgo raw refuses a "-" argument when stdin is a terminal: wp would read no input and write empty content; pipe or redirect the input, e.g. wpgo raw -- post update 7 - < content.html`) + } + buffered := bufio.NewReader(stdin) + if _, err := buffered.Peek(1); err != nil { + if err == io.EOF { + return nil, errors.New(`wpgo raw refuses a "-" argument with empty stdin: wp would write empty content`) + } + return nil, fmt.Errorf("read stdin: %w", err) + } + return buffered, nil +} + +// isPiped reports whether r is input to forward: a pipe, a file or another +// reader, rather than nil, a terminal or another character device such as +// /dev/null. +func isPiped(r io.Reader) bool { + if r == nil { + return false + } + f, ok := r.(*os.File) + if !ok { + return true + } + fi, err := f.Stat() + return err == nil && fi.Mode()&os.ModeCharDevice == 0 +} diff --git a/internal/cmd/raw_stdin_test.go b/internal/cmd/raw_stdin_test.go new file mode 100644 index 0000000..41f0f64 --- /dev/null +++ b/internal/cmd/raw_stdin_test.go @@ -0,0 +1,255 @@ +package cmd + +import ( + "crypto/ed25519" + "crypto/rand" + "encoding/pem" + "io" + "net" + "os" + "path/filepath" + "strconv" + "strings" + "sync" + "testing" + + "golang.org/x/crypto/ssh" +) + +// remoteExec is one exec request the fake SSH server received. +type remoteExec struct { + command, stdin string +} + +// sshSite registers one site of hostType in a temp HOME whose SSH config +// points at an in-process SSH server, and returns a func listing the exec +// requests that server has received. The server reads each command's stdin to +// EOF and answers like wp: "Success" and exit 0. +func sshSite(t *testing.T, hostType string) (execs func() []remoteExec) { + t.Helper() + _, hostKey, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatal(err) + } + hostSigner, err := ssh.NewSignerFromKey(hostKey) + if err != nil { + t.Fatal(err) + } + serverCfg := &ssh.ServerConfig{ + PublicKeyCallback: func(ssh.ConnMetadata, ssh.PublicKey) (*ssh.Permissions, error) { return &ssh.Permissions{}, nil }, + } + serverCfg.AddHostKey(hostSigner) + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { listener.Close() }) + + var mu sync.Mutex + var got []remoteExec + go func() { + for { + nc, err := listener.Accept() + if err != nil { + return + } + go func() { + defer nc.Close() + _, chans, reqs, err := ssh.NewServerConn(nc, serverCfg) + if err != nil { + return + } + go ssh.DiscardRequests(reqs) + for newCh := range chans { + ch, chReqs, err := newCh.Accept() + if err != nil { + continue + } + go func() { + defer ch.Close() + for req := range chReqs { + if req.Type != "exec" { + _ = req.Reply(false, nil) + continue + } + var payload struct{ Command string } + _ = ssh.Unmarshal(req.Payload, &payload) + _ = req.Reply(true, nil) + in, _ := io.ReadAll(ch) + mu.Lock() + got = append(got, remoteExec{payload.Command, string(in)}) + mu.Unlock() + _, _ = ch.Write([]byte("Success: Updated post 7.\n")) + _, _ = ch.SendRequest("exit-status", false, ssh.Marshal(struct{ Status uint32 }{0})) + return + } + }() + } + }() + } + }() + + fakeSite(t, hostType) + home := os.Getenv("HOME") + _, clientKey, err := ed25519.GenerateKey(rand.Reader) + if err != nil { + t.Fatal(err) + } + block, err := ssh.MarshalPrivateKey(clientKey, "") + if err != nil { + t.Fatal(err) + } + keyPath := filepath.Join(home, ".ssh", "id_test") + if err := os.WriteFile(keyPath, pem.EncodeToMemory(block), 0o600); err != nil { + t.Fatal(err) + } + port := strconv.Itoa(listener.Addr().(*net.TCPAddr).Port) + sshConfig := "Host site\n HostName 127.0.0.1\n Port " + port + "\n User u\n IdentityFile " + keyPath + "\n" + if err := os.WriteFile(filepath.Join(home, ".ssh", "config"), []byte(sshConfig), 0o600); err != nil { + t.Fatal(err) + } + t.Setenv("SSH_AUTH_SOCK", "") + + return func() []remoteExec { + mu.Lock() + defer mu.Unlock() + return append([]remoteExec(nil), got...) + } +} + +// pipeWith returns the read end of an OS pipe holding content, closed for +// writing, as a shell gives a command for `printf content | wpgo raw ...`. +func pipeWith(t *testing.T, content string) *os.File { + t.Helper() + r, w, err := os.Pipe() + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { r.Close() }) + if _, err := w.WriteString(content); err != nil { + t.Fatal(err) + } + w.Close() + return r +} + +// TestRawForwardsPipedStdin runs raw with piped stdin against an SSH server +// and checks the remote wp command receives it only when a "-" argument +// makes wp read it. +func TestRawForwardsPipedStdin(t *testing.T) { + const body = "\n

Updated body.

\n\n" + tests := []struct { + name string + args []string + wantStdin string + }{ + {"post update dash", []string{"post", "update", "7", "-"}, body}, + {"quoted dash", []string{"post", "update", "7", "'-'"}, body}, + {"no dash", []string{"option", "get", "home"}, ""}, + } + for _, hostType := range []string{"standard", "wpengine"} { + for _, tt := range tests { + t.Run(hostType+"/"+tt.name, func(t *testing.T) { + execs := sshSite(t, hostType) + err := (&RawCmd{Args: tt.args}).runWithIO(&Globals{Site: "site", NoCache: true}, pipeWith(t, body)) + if err != nil { + t.Fatalf("raw %q: %v", tt.args, err) + } + got := execs() + if len(got) != 1 || got[0].stdin != tt.wantStdin || !strings.HasSuffix(got[0].command, " && wp "+strings.Join(tt.args, " ")) { + t.Fatalf("remote got %+v, want one `wp %s` with stdin %q", got, strings.Join(tt.args, " "), tt.wantStdin) + } + }) + } + } +} + +// TestRawRefusesStdinReads checks that raw refuses, before anything runs +// remotely, a "-" argument with no input on stdin, including one that only +// WP Engine's second parse turns into "-", and any wp db command that would +// read SQL from stdin. db export's "-", which means stdout, still runs. +func TestRawRefusesStdinReads(t *testing.T) { + devNull := func(t *testing.T) io.Reader { + t.Helper() + f, err := os.Open(os.DevNull) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { f.Close() }) + return f + } + piped := func(content string) func(*testing.T) io.Reader { + return func(t *testing.T) io.Reader { + t.Helper() + return pipeWith(t, content) + } + } + const outfile = "SELECT 1 INTO OUTFILE '/tmp/dump.sql';" + tests := []struct { + name string + hostType string + args []string + stdin func(*testing.T) io.Reader + wantErr string // "" means the command must reach the server + }{ + {"terminal-like stdin", "standard", []string{"post", "update", "7", "-"}, devNull, "stdin is a terminal"}, + {"empty pipe", "standard", []string{"post", "update", "7", "-"}, piped(""), "empty stdin"}, + {"empty pipe quoted dash", "standard", []string{"post", "update", "7", "'-'"}, piped(""), "empty stdin"}, + {"no stdin", "standard", []string{"post", "update", "7", "-"}, func(*testing.T) io.Reader { return nil }, "stdin is a terminal"}, + {"empty pipe dash after gateway parse", "wpengine", []string{"post", "update", "7", `"'-'"`}, piped(""), "empty stdin"}, + {"piped SQL to db query", "standard", []string{"db", "query"}, piped(outfile), "without SQL"}, + {"piped SQL to db query dash", "standard", []string{"db", "query", "-"}, piped(outfile), "on wp db"}, + {"piped SQL to db import dash", "standard", []string{"sql", "import", "-"}, piped(outfile), "refuses `wp db import`"}, + {"piped SQL to db import after gateway parse", "wpengine", []string{"db", "'import -'"}, piped(outfile), "refuses `wp db import`"}, + {"db export to stdout", "standard", []string{"db", "export", "-"}, devNull, ""}, + {"no dash, empty pipe", "standard", []string{"option", "get", "home"}, piped(""), ""}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + execs := sshSite(t, tt.hostType) + err := (&RawCmd{Args: tt.args}).runWithIO(&Globals{Site: "site", NoCache: true}, tt.stdin(t)) + got := execs() + if tt.wantErr == "" { + if err != nil || len(got) != 1 { + t.Fatalf("raw %q: err = %v, remote got %+v; want it run once", tt.args, err, got) + } + return + } + if err == nil || !strings.Contains(err.Error(), tt.wantErr) || len(got) != 0 { + t.Fatalf("raw %q: err = %v, remote got %+v; want refusal %q before anything runs", tt.args, err, got, tt.wantErr) + } + }) + } +} + +// TestRawDryRunLeavesStdin checks that dry-run prints a "-" command without +// reading stdin or refusing for missing input. +func TestRawDryRunLeavesStdin(t *testing.T) { + args := []string{"post", "update", "7", "-"} + for _, tt := range []struct { + name string + stdin string + piped bool + }{{"piped body", "body", true}, {"no stdin", "", false}} { + t.Run(tt.name, func(t *testing.T) { + execs := sshSite(t, "standard") + var stdin io.Reader + if tt.piped { + stdin = pipeWith(t, tt.stdin) + } + if err := (&RawCmd{Args: args}).runWithIO(&Globals{Site: "site", NoCache: true, DryRun: true}, stdin); err != nil { + t.Fatalf("dry-run raw %q: %v", args, err) + } + if got := execs(); len(got) != 0 { + t.Fatalf("dry-run sent %+v", got) + } + if stdin == nil { + return + } + left, err := io.ReadAll(stdin) + if err != nil || string(left) != tt.stdin { + t.Fatalf("stdin left = %q, %v; want %q unread", left, err, tt.stdin) + } + }) + } +}