From 0c2e75d888a4a8f66ee75f7e394ccd9c24981f11 Mon Sep 17 00:00:00 2001 From: Oluwatobi Ogundimu Date: Fri, 31 Jul 2026 20:04:40 +0100 Subject: [PATCH] SC-5: SSH transport layer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ParseRemotePath (user@host:path, IPv6, Windows drive-letter disambiguation), BuildRSHCommand with quote-aware tokenizing (bare strings.Fields would mis-split real -e usage with quoted args), length-prefixed+type-tagged frame protocol, Session (subprocess/pipe lifecycle), and Handshake/ServeHandshake round trip. --server wired into CLI, hidden from --help like upstream. Option A (spawn ssh subprocess) confirmed correct: -e/--rsh's real semantics only work against an arbitrary shelled-out command. Verified against upstream that --port is daemon-mode-only and -i already means --itemize-changes, so -e is the sole customization mechanism, matching real rsync rather than inventing new flags. Self-review found a real pipe leak: StdoutPipe() failing after StdinPipe() succeeded meant Start() was never reached, leaking the stdin pipe. Fixed. Scope note: this builds transport primitives + a minimal handshake proof, not full pipeline wiring (file-list/signature/delta exchange over this transport) — that's separate follow-up scope, not silently included or silently skipped. --- README.md | 83 +++++++++++++++++-- internal/cli/root.go | 34 +++++++- internal/transport/frame.go | 86 +++++++++++++++++++ internal/transport/frame_test.go | 110 +++++++++++++++++++++++++ internal/transport/handshake.go | 59 +++++++++++++ internal/transport/handshake_test.go | 76 +++++++++++++++++ internal/transport/integration_test.go | 82 ++++++++++++++++++ internal/transport/remotepath.go | 87 +++++++++++++++++++ internal/transport/remotepath_test.go | 45 ++++++++++ internal/transport/rsh.go | 85 +++++++++++++++++++ internal/transport/rsh_test.go | 70 ++++++++++++++++ internal/transport/session.go | 101 +++++++++++++++++++++++ 12 files changed, 907 insertions(+), 11 deletions(-) create mode 100644 internal/transport/frame.go create mode 100644 internal/transport/frame_test.go create mode 100644 internal/transport/handshake.go create mode 100644 internal/transport/handshake_test.go create mode 100644 internal/transport/integration_test.go create mode 100644 internal/transport/remotepath.go create mode 100644 internal/transport/remotepath_test.go create mode 100644 internal/transport/rsh.go create mode 100644 internal/transport/rsh_test.go create mode 100644 internal/transport/session.go diff --git a/README.md b/README.md index d179c28..ab079d0 100644 --- a/README.md +++ b/README.md @@ -5,14 +5,17 @@ An rsync-inspired file synchronization tool written in Go. ## Status CLI parsing, file enumeration, filter-rule matching, the delta-transfer -algorithm, and file attribute preservation are implemented; nothing is -wired together into an actual sync yet. `internal/sync` can list a source -tree (`sync.Walk`), filter it (`sync.FilterEntries`), compute/apply binary -deltas between two versions of a file -(`sync.GenerateDelta`/`sync.ApplyDelta`), and apply permissions/times/ -ownership/symlinks/hard links (`sync.ApplyAttributes` and friends) - but -the CLI only echoes parsed flags, and `internal/transport` is still empty, -so none of this runs end to end yet. +algorithm, file attribute preservation, and the SSH transport primitives +are implemented; nothing is wired together into an actual sync yet. +`internal/sync` can list a source tree (`sync.Walk`), filter it +(`sync.FilterEntries`), compute/apply binary deltas between two versions +of a file (`sync.GenerateDelta`/`sync.ApplyDelta`), and apply +permissions/times/ownership/symlinks/hard links (`sync.ApplyAttributes` +and friends). `internal/transport` can parse remote endpoints, spawn and +frame a connection to a remote `grsync --server`, and complete a minimal +handshake over it. But the CLI's normal sync path still only echoes parsed +flags - none of this is wired into an actual end-to-end sync yet (see +[SSH Transport](#ssh-transport) below for exactly what that gap is). ## Build @@ -50,6 +53,7 @@ argument is always the destination. | `--filter RULE` | | add a filter rule (repeatable) | | `--exclude-from FILE` | | read exclude patterns from FILE, one per line (repeatable) | | `--include-from FILE` | | read include patterns from FILE, one per line (repeatable) | +| `--rsh COMMAND` | `-e` | remote shell to use for SSH transport, e.g. `"ssh -p 2222 -i key.pem"` (default: `ssh`) | All five filter-related flags share one ordered rule list - their relative order on the command line is preserved, matching rsync's first-match-wins @@ -161,6 +165,64 @@ capture. rather than attempting a syscall that fails for most callers and calling that "support." +## SSH Transport + +`internal/transport` reaches a remote grsync the same way upstream rsync +does: by spawning a remote-shell subprocess (`ssh` by default) and +speaking a protocol over its stdin/stdout, rather than a native Go SSH +client. This was a deliberate choice, not just the default option: the +`--rsh`/`-e` flag's real meaning in rsync only makes sense if grsync is +actually invoking an arbitrary shell command, and shelling out means +`~/.ssh/config`, `ssh-agent`, and `known_hosts` all keep working exactly +as already configured, instead of being reimplemented. + +- **Endpoint syntax**: `transport.ParseRemotePath` recognizes + `[user@]host:path`, including IPv6 literals (`user@[::1]:path`), while + correctly treating a Windows drive letter (`C:\...`) as local rather + than a remote host. +- **`--rsh`/`-e`** is the *only* customization mechanism for the remote + shell - there's no separate `--port` or `--identity` flag. This matches + real rsync: upstream's `--port` only applies to daemon-mode (`rsync://`) + connections, and its `-i` flag already means `--itemize-changes`, not + "identity file." Port/identity/`ProxyJump`/etc. go through `-e` (e.g. + `-e "ssh -p 2222 -i key.pem"`) or `~/.ssh/config`, exactly as with real + rsync. +- **Host-key verification** is not reimplemented at all, in either + direction: no flag here ever weakens it (no `StrictHostKeyChecking=no`, + no null `UserKnownHostsFile`), and none of its logic is duplicated + either. Whatever the invoked shell command does by default is exactly + what happens - this is genuinely real, not a stub, precisely because + nothing here touches it. +- **Framing**: `transport.WriteFrame`/`transport.ReadFrame` multiplex the single + stdin/stdout stream into typed, length-prefixed messages (4-byte + length + 1-byte type + payload, capped at 64 MiB per frame against a + corrupt or hostile length prefix). +- **`--server` mode**: hidden from `--help` (like rsync's own `--server`), + this is how a remotely-invoked grsync switches into speaking the + protocol instead of doing a normal sync. Right now it implements only a + minimal handshake (`transport.ServeHandshake`/`transport.Handshake`) - enough to + prove the subprocess, pipes, and framing work correctly end to end + through a real `ssh` connection, not a full remote sync. + +**What's genuinely missing, not just untested:** nothing in the normal +(non-`--server`) CLI path detects a remote `user@host:path` argument or +calls `transport.Dial`/`transport.Handshake` - +`transport.ParseRemotePath`, `transport.BuildRSHCommand`, +`transport.Dial`, and `transport.Handshake` are all implemented and +independently tested, but not yet invoked from a real sync. Wiring an +actual file-list/signature/delta exchange on top of this frame/session +foundation - so a remote sync really happens - is separately-scoped +follow-up work, not part of this ticket. + +**Testing note**: `TestSSHLocalhost_HandshakeRoundTrip` builds the real +`grsync` binary and drives the full `transport.Dial`/`transport.Session`/ +`transport.Handshake` path through actual `ssh` against `127.0.0.1`, +skipping gracefully if no SSH server is reachable there +non-interactively. No such server was available in this development +environment (an `ssh` client is present, but nothing +was listening), so while the test is believed correct by code review, it +has not been observed to pass against a live server. + ## Architecture - `cmd/grsync` - CLI entrypoint. @@ -168,7 +230,10 @@ capture. - `internal/sync` - file-list generation, filter matching, the delta-transfer algorithm, and attribute preservation today; wiring these together into an actual sync comes later. -- `internal/transport` - (placeholder) data movement, local and remote. +- `internal/transport` - remote endpoint parsing, RSH command + construction, frame protocol, subprocess session management, and a + minimal `--server` handshake today; the full remote sync pipeline + (file list/signature/delta exchange) is not wired up yet. Goal: full feature parity with upstream rsync, including protocol/format interoperability where specified (e.g. batch mode's file format). diff --git a/internal/cli/root.go b/internal/cli/root.go index 64726f7..e31e28a 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -10,6 +10,8 @@ import ( "strings" "github.com/spf13/cobra" + + "github.com/syntaxroot-cc/grsync/internal/transport" ) // FilterRuleType identifies which kind of rule a FilterRule represents. @@ -51,6 +53,8 @@ type options struct { delete bool progress bool filterRules []FilterRule + rsh string + server bool } // filterRuleFlag implements pflag.Value. Each of --exclude/--include/ @@ -93,8 +97,21 @@ func NewRootCmd() *cobra.Command { Short: "grsync synchronizes files between one or more sources and a destination", Long: "grsync is an rsync-inspired file synchronization tool.\n" + "At this stage it only parses arguments and flags; no files are copied yet.", - Args: cobra.MinimumNArgs(2), + // --server takes no positional source/destination args at all: it + // is how a remote-invoked grsync (e.g. `ssh host grsync --server`) + // switches into speaking internal/transport's protocol over its + // own stdin/stdout, rather than a normal source/destination sync. + // A plain MinimumNArgs(2) would reject that invocation outright. + Args: func(cmd *cobra.Command, args []string) error { + if opts.server { + return nil + } + return cobra.MinimumNArgs(2)(cmd, args) + }, RunE: func(cmd *cobra.Command, args []string) error { + if opts.server { + return transport.ServeHandshake(cmd.InOrStdin(), cmd.OutOrStdout()) + } sources, destination := args[:len(args)-1], args[len(args)-1] return run(cmd, sources, destination, opts) }, @@ -119,6 +136,18 @@ func NewRootCmd() *cobra.Command { "exclude-from", "read exclude patterns from FILE, one per line (repeatable, order preserved)") flags.Var(&filterRuleFlag{ruleType: FilterRuleIncludeFrom, rules: &opts.filterRules}, "include-from", "read include patterns from FILE, one per line (repeatable, order preserved)") + flags.StringVarP(&opts.rsh, "rsh", "e", "", + "specify the remote shell to use, e.g. \"ssh -p 2222 -i key.pem\" (default: ssh); "+ + "the sole way to customize port/identity/proxy for remote transport, matching rsync") + flags.BoolVar(&opts.server, "server", false, + "run in server mode, speaking the transport protocol over stdin/stdout "+ + "(internal use only - invoked remotely via --rsh, never typed directly, matching rsync's own --server)") + // Hidden, not just undocumented: real rsync's --server is likewise + // absent from its own --help output, since it's a protocol + // implementation detail, not a user-facing feature to advertise. + if err := flags.MarkHidden("server"); err != nil { + panic(err) // only fails if "server" isn't a registered flag name, which would be a programming error caught immediately by any test run + } return cmd } @@ -150,10 +179,11 @@ func run(cmd *cobra.Command, sources []string, destination string, opts *options "dry-run: %t\n"+ "delete: %t\n"+ "progress: %t\n"+ + "rsh: %q\n"+ "filters: %s\n", sources, destination, opts.archive, opts.verbose, opts.compress, opts.recursive, opts.dirs, opts.dryRun, - opts.delete, opts.progress, rules.String(), + opts.delete, opts.progress, opts.rsh, rules.String(), ) _, err := fmt.Fprint(cmd.OutOrStdout(), summary) diff --git a/internal/transport/frame.go b/internal/transport/frame.go new file mode 100644 index 0000000..847229f --- /dev/null +++ b/internal/transport/frame.go @@ -0,0 +1,86 @@ +package transport + +import ( + "encoding/binary" + "fmt" + "io" +) + +// FrameType tags what kind of message a Frame carries, so a single +// stdin/stdout byte stream can multiplex different message kinds (file +// list entries, signatures, delta ops, control messages) without a +// separate channel for each. +type FrameType byte + +const ( + // FrameHello and FrameHelloAck are this ticket's minimal --server + // handshake: the client sends FrameHello, the server replies with + // FrameHelloAck. Later tickets add the frame types an actual sync + // needs (file list, signature, delta ops); this ticket only proves + // the pipe, subprocess, and framing work end to end. + FrameHello FrameType = iota + // FrameHelloAck is the server's reply to FrameHello. + FrameHelloAck + // FrameError carries a human-readable error message from one side to + // the other, rather than the connection just dying silently. + FrameError +) + +// maxFramePayload bounds how large a single frame's payload may be. A +// length prefix isn't validated against anything else in this protocol +// (there's no higher-level "expected size" to check it against), so +// without a cap, a corrupted stream or a hostile peer could send a +// length like 0xFFFFFFFF and force ReadFrame to attempt a multi-gigabyte +// allocation before ever reading a single payload byte. 64 MiB is +// comfortably larger than any frame type this ticket defines needs. +const maxFramePayload = 64 * 1024 * 1024 + +// Frame is a single multiplexed protocol message. +type Frame struct { + Type FrameType + Payload []byte +} + +// WriteFrame writes f to w as: 4-byte big-endian length of Payload, +// 1-byte Type, then Payload itself. +func WriteFrame(w io.Writer, f Frame) error { + if len(f.Payload) > maxFramePayload { + return fmt.Errorf("frame payload of %d bytes exceeds max %d", len(f.Payload), maxFramePayload) + } + + header := make([]byte, 5) + binary.BigEndian.PutUint32(header[:4], uint32(len(f.Payload))) + header[4] = byte(f.Type) + + if _, err := w.Write(header); err != nil { + return fmt.Errorf("writing frame header: %w", err) + } + if len(f.Payload) > 0 { + if _, err := w.Write(f.Payload); err != nil { + return fmt.Errorf("writing frame payload: %w", err) + } + } + return nil +} + +// ReadFrame reads one Frame from r, written by WriteFrame. +func ReadFrame(r io.Reader) (Frame, error) { + header := make([]byte, 5) + if _, err := io.ReadFull(r, header); err != nil { + return Frame{}, fmt.Errorf("reading frame header: %w", err) + } + + length := binary.BigEndian.Uint32(header[:4]) + if length > maxFramePayload { + return Frame{}, fmt.Errorf("frame payload of %d bytes exceeds max %d", length, maxFramePayload) + } + + payload := make([]byte, length) + if length > 0 { + if _, err := io.ReadFull(r, payload); err != nil { + return Frame{}, fmt.Errorf("reading frame payload: %w", err) + } + } + + return Frame{Type: FrameType(header[4]), Payload: payload}, nil +} diff --git a/internal/transport/frame_test.go b/internal/transport/frame_test.go new file mode 100644 index 0000000..eca3df8 --- /dev/null +++ b/internal/transport/frame_test.go @@ -0,0 +1,110 @@ +package transport + +import ( + "bytes" + "encoding/binary" + "io" + "testing" +) + +func TestWriteReadFrame_RoundTrip(t *testing.T) { + var buf bytes.Buffer + want := Frame{Type: FrameHello, Payload: []byte("hello world")} + + if err := WriteFrame(&buf, want); err != nil { + t.Fatalf("WriteFrame returned error: %v", err) + } + got, err := ReadFrame(&buf) + if err != nil { + t.Fatalf("ReadFrame returned error: %v", err) + } + if got.Type != want.Type || string(got.Payload) != string(want.Payload) { + t.Errorf("ReadFrame = %+v, want %+v", got, want) + } +} + +func TestWriteReadFrame_EmptyPayload(t *testing.T) { + var buf bytes.Buffer + want := Frame{Type: FrameHelloAck, Payload: nil} + + if err := WriteFrame(&buf, want); err != nil { + t.Fatalf("WriteFrame returned error: %v", err) + } + got, err := ReadFrame(&buf) + if err != nil { + t.Fatalf("ReadFrame returned error: %v", err) + } + if got.Type != want.Type || len(got.Payload) != 0 { + t.Errorf("ReadFrame = %+v, want Type=%v with empty payload", got, want.Type) + } +} + +func TestWriteReadFrame_MultipleFramesInSequence(t *testing.T) { + var buf bytes.Buffer + frames := []Frame{ + {Type: FrameHello, Payload: []byte("first")}, + {Type: FrameError, Payload: []byte("second, a bit longer")}, + {Type: FrameHelloAck, Payload: nil}, + } + for _, f := range frames { + if err := WriteFrame(&buf, f); err != nil { + t.Fatalf("WriteFrame returned error: %v", err) + } + } + + for i, want := range frames { + got, err := ReadFrame(&buf) + if err != nil { + t.Fatalf("ReadFrame #%d returned error: %v", i, err) + } + if got.Type != want.Type || string(got.Payload) != string(want.Payload) { + t.Errorf("frame #%d = %+v, want %+v", i, got, want) + } + } + + // The stream must be fully consumed - proves frame boundaries were + // tracked correctly rather than one frame's read accidentally + // consuming into the next frame's bytes (or leaving some behind). + if buf.Len() != 0 { + t.Errorf("%d bytes left unread after consuming all frames", buf.Len()) + } +} + +func TestReadFrame_TruncatedHeaderErrors(t *testing.T) { + buf := bytes.NewBuffer([]byte{0x00, 0x00}) // only 2 of 5 header bytes + if _, err := ReadFrame(buf); err == nil { + t.Fatalf("ReadFrame with a truncated header returned nil error, want an error") + } +} + +func TestReadFrame_TruncatedPayloadErrors(t *testing.T) { + var buf bytes.Buffer + header := make([]byte, 5) + binary.BigEndian.PutUint32(header[:4], 100) // claims 100 payload bytes + header[4] = byte(FrameHello) + buf.Write(header) + buf.WriteString("only a few bytes") // far fewer than the claimed 100 + + if _, err := ReadFrame(&buf); err == nil { + t.Fatalf("ReadFrame with a truncated payload returned nil error, want an error") + } +} + +func TestReadFrame_OversizedLengthRejected(t *testing.T) { + var buf bytes.Buffer + header := make([]byte, 5) + binary.BigEndian.PutUint32(header[:4], maxFramePayload+1) + header[4] = byte(FrameHello) + buf.Write(header) + + if _, err := ReadFrame(&buf); err == nil { + t.Fatalf("ReadFrame with an over-max length prefix returned nil error, want an error") + } +} + +func TestWriteFrame_OversizedPayloadRejected(t *testing.T) { + f := Frame{Type: FrameHello, Payload: make([]byte, maxFramePayload+1)} + if err := WriteFrame(io.Discard, f); err == nil { + t.Fatalf("WriteFrame with an over-max payload returned nil error, want an error") + } +} diff --git a/internal/transport/handshake.go b/internal/transport/handshake.go new file mode 100644 index 0000000..45162a4 --- /dev/null +++ b/internal/transport/handshake.go @@ -0,0 +1,59 @@ +package transport + +import ( + "fmt" + "io" +) + +// ProtocolVersion identifies this ticket's minimal handshake protocol. +// Bump it if the frame types or handshake sequence ever change +// incompatibly, so an old client/server pair fails the version check +// below instead of misinterpreting each other's frames. +const ProtocolVersion = 1 + +// ServeHandshake implements grsync's --server-mode entry point for this +// ticket's scope: read one FrameHello from r, verify its protocol +// version, and reply with one FrameHelloAck on w. +// +// This proves the subprocess/pipe/framing machinery works end to end +// through a real remote-shell connection - it is deliberately not a full +// sync server. Wiring an actual file-list/signature/delta exchange on top +// of this frame/session foundation is later, separately-scoped work (see +// README's note on this). +func ServeHandshake(r io.Reader, w io.Writer) error { + f, err := ReadFrame(r) + if err != nil { + return fmt.Errorf("reading hello: %w", err) + } + if f.Type != FrameHello { + return fmt.Errorf("expected FrameHello, got frame type %d", f.Type) + } + if len(f.Payload) != 1 || f.Payload[0] != ProtocolVersion { + return fmt.Errorf("unsupported protocol version (got payload %v, want [%d])", f.Payload, ProtocolVersion) + } + + return WriteFrame(w, Frame{Type: FrameHelloAck, Payload: []byte{ProtocolVersion}}) +} + +// Handshake performs the client side of ServeHandshake over rw: sends +// FrameHello, then reads back FrameHelloAck, confirming the round trip +// actually completed rather than just assuming the connection is good +// because Dial succeeded. +func Handshake(rw io.ReadWriter) error { + if err := WriteFrame(rw, Frame{Type: FrameHello, Payload: []byte{ProtocolVersion}}); err != nil { + return fmt.Errorf("sending hello: %w", err) + } + + f, err := ReadFrame(rw) + if err != nil { + return fmt.Errorf("reading hello-ack: %w", err) + } + if f.Type != FrameHelloAck { + return fmt.Errorf("expected FrameHelloAck, got frame type %d", f.Type) + } + if len(f.Payload) != 1 || f.Payload[0] != ProtocolVersion { + return fmt.Errorf("server reported incompatible protocol version (got payload %v, want [%d])", f.Payload, ProtocolVersion) + } + + return nil +} diff --git a/internal/transport/handshake_test.go b/internal/transport/handshake_test.go new file mode 100644 index 0000000..bfd85c8 --- /dev/null +++ b/internal/transport/handshake_test.go @@ -0,0 +1,76 @@ +package transport + +import ( + "io" + "testing" +) + +// pipeReadWriter joins two io.Pipe halves into a single io.ReadWriter, so +// Handshake (which needs one bidirectional stream, like a real Session) +// can be driven against an in-memory pipe instead of a real subprocess. +type pipeReadWriter struct { + io.Reader + io.Writer +} + +func TestHandshake_PureLogic(t *testing.T) { + // Two independent pipes, one per direction, wired crosswise so each + // side's writes become the other side's reads - simulating a real + // bidirectional Session without spawning a subprocess at all. + clientReadsFromServer, serverWritesToClient := io.Pipe() + serverReadsFromClient, clientWritesToServer := io.Pipe() + + client := pipeReadWriter{Reader: clientReadsFromServer, Writer: clientWritesToServer} + + serverErrCh := make(chan error, 1) + go func() { + serverErrCh <- ServeHandshake(serverReadsFromClient, serverWritesToClient) + }() + + if err := Handshake(client); err != nil { + t.Fatalf("Handshake returned error: %v", err) + } + if err := <-serverErrCh; err != nil { + t.Fatalf("ServeHandshake returned error: %v", err) + } +} + +func TestServeHandshake_RejectsWrongFrameType(t *testing.T) { + r, w := io.Pipe() + go func() { + _ = WriteFrame(w, Frame{Type: FrameError, Payload: []byte{ProtocolVersion}}) + _ = w.Close() + }() + + if err := ServeHandshake(r, io.Discard); err == nil { + t.Fatalf("ServeHandshake with a non-Hello frame returned nil error, want an error") + } +} + +func TestServeHandshake_RejectsWrongVersion(t *testing.T) { + r, w := io.Pipe() + go func() { + _ = WriteFrame(w, Frame{Type: FrameHello, Payload: []byte{ProtocolVersion + 1}}) + _ = w.Close() + }() + + if err := ServeHandshake(r, io.Discard); err == nil { + t.Fatalf("ServeHandshake with a mismatched protocol version returned nil error, want an error") + } +} + +func TestHandshake_RejectsWrongFrameType(t *testing.T) { + r, w := io.Pipe() + go func() { + _ = WriteFrame(w, Frame{Type: FrameError, Payload: []byte{ProtocolVersion}}) + _ = w.Close() + }() + + // Handshake's own outgoing FrameHello is simply discarded here - this + // test only cares what happens when the response it reads back isn't + // a FrameHelloAck. + client := pipeReadWriter{Reader: r, Writer: io.Discard} + if err := Handshake(client); err == nil { + t.Fatalf("Handshake with a non-HelloAck frame returned nil error, want an error") + } +} diff --git a/internal/transport/integration_test.go b/internal/transport/integration_test.go new file mode 100644 index 0000000..514b312 --- /dev/null +++ b/internal/transport/integration_test.go @@ -0,0 +1,82 @@ +package transport + +import ( + "os" + "os/exec" + "path/filepath" + "runtime" + "testing" + "time" +) + +// requireLocalSSHServer skips the calling test unless a real SSH server +// is actually reachable at 127.0.0.1 for the current user, non- +// interactively. This is a capability probe, not an assumption: most +// dev machines and CI runners have an ssh *client* installed but no +// sshd listening, so skipping here (rather than failing) is the normal, +// expected outcome almost everywhere this runs - the same pattern +// TestWalk_Symlink (internal/sync) and the Lchown/Mkfifo tests +// (internal/sync/attributes_test.go, specialfiles_test.go) already use +// for privilege/platform-gated behavior. +// +// 127.0.0.1 is used rather than "localhost": in at least this project's +// own Windows dev environment, the bundled ssh client failed to resolve +// "localhost" at all (a local DNS/hosts quirk, not an SSH problem) before +// ever getting to the real "is anything listening" question - the IP +// literal sidesteps that unrelated failure mode entirely. +func requireLocalSSHServer(t *testing.T) { + t.Helper() + cmd := exec.Command("ssh", + "-o", "BatchMode=yes", // fail immediately rather than prompt (password, unknown host key, ...) + "-o", "ConnectTimeout=5", + "127.0.0.1", "true") + if err := cmd.Run(); err != nil { + t.Skipf("no SSH server reachable at 127.0.0.1 for a non-interactive connection: %v", err) + } +} + +// buildGrsyncBinary compiles cmd/grsync fresh into a temp file and +// returns its path, so this test exercises the real --server flag wiring +// in internal/cli/root.go exactly as a real invocation would, rather than +// some test-only stand-in for it. +func buildGrsyncBinary(t *testing.T) string { + t.Helper() + + out := filepath.Join(t.TempDir(), "grsync") + if runtime.GOOS == "windows" { + out += ".exe" + } + + cmd := exec.Command("go", "build", "-o", out, "github.com/syntaxroot-cc/grsync/cmd/grsync") + cmd.Env = os.Environ() + if output, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("building grsync binary: %v\n%s", err, output) + } + return out +} + +func TestSSHLocalhost_HandshakeRoundTrip(t *testing.T) { + requireLocalSSHServer(t) + grsyncPath := buildGrsyncBinary(t) + + session, err := Dial("", "", "127.0.0.1", []string{grsyncPath, "--server"}) + if err != nil { + t.Fatalf("Dial returned error: %v", err) + } + + done := make(chan error, 1) + go func() { done <- Handshake(session) }() + + select { + case err := <-done: + if err != nil { + t.Errorf("Handshake returned error: %v", err) + } + case <-time.After(15 * time.Second): + t.Fatal("Handshake did not complete within 15s") + } + + if err := session.Close(); err != nil { + t.Errorf("Session.Close returned error: %v", err) + } +} diff --git a/internal/transport/remotepath.go b/internal/transport/remotepath.go new file mode 100644 index 0000000..fd7556d --- /dev/null +++ b/internal/transport/remotepath.go @@ -0,0 +1,87 @@ +// Package transport implements grsync's remote-shell (SSH) transport: +// spawning a remote-shell subprocess to reach a remote grsync process +// running in --server mode, and the framed protocol multiplexed over its +// stdin/stdout. +package transport + +import "strings" + +// RemotePath is a parsed [user@]host:path or [user@][host]:path (IPv6) +// remote endpoint, as used for the source/destination arguments of a +// remote sync. +type RemotePath struct { + User string // empty if no "user@" prefix was present + Host string // hostname or IP literal; brackets are stripped for IPv6 + Path string +} + +// ParseRemotePath reports whether s looks like a remote +// [user@]host:path (or IPv6 [user@][host]:path) endpoint rather than a +// local filesystem path, returning the parsed form when it does. +// +// Disambiguation rule, in order: +// +// 1. A single ASCII letter immediately followed by ":" (e.g. "C:", +// "C:\Users\...") is always a Windows drive letter, never a remote +// host - real single-letter hostnames in this position are +// vanishingly rare in practice, while grsync runs natively on +// Windows (unlike upstream rsync), so this ambiguity has to be +// resolved in favor of the overwhelmingly common case. +// 2. A "/" appearing before the separating ":" means this can't be +// [user@]host:path at all - no real hostname contains a "/", so +// finding one first proves whatever precedes the colon is a path +// segment, not a host (this also naturally handles a "user@" prefix +// that isn't really one, e.g. a local path that happens to contain +// "@"). +// 3. Otherwise, an "[...]" immediately after any "user@" prefix is an +// IPv6 literal - the separating ":" is the one right after the +// closing "]", not the first ":" in the string (an IPv6 address is +// full of colons itself). +// 4. Otherwise, the first ":" is the separator. +func ParseRemotePath(s string) (RemotePath, bool) { + if s == "" || isWindowsDriveLetterPath(s) { + return RemotePath{}, false + } + + rest := s + user := "" + if at := strings.IndexByte(rest, '@'); at != -1 { + if strings.IndexByte(rest[:at], '/') == -1 { + user = rest[:at] + rest = rest[at+1:] + } + } + + var host, path string + switch { + case strings.HasPrefix(rest, "["): + end := strings.IndexByte(rest, ']') + if end == -1 || end+1 >= len(rest) || rest[end+1] != ':' { + return RemotePath{}, false + } + host, path = rest[1:end], rest[end+2:] + default: + colon := strings.IndexByte(rest, ':') + if colon == -1 { + return RemotePath{}, false + } + if strings.IndexByte(rest[:colon], '/') != -1 { + return RemotePath{}, false + } + host, path = rest[:colon], rest[colon+1:] + } + + if host == "" { + return RemotePath{}, false + } + return RemotePath{User: user, Host: host, Path: path}, true +} + +func isWindowsDriveLetterPath(s string) bool { + if len(s) < 2 { + return false + } + c := s[0] + isLetter := (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z') + return isLetter && s[1] == ':' +} diff --git a/internal/transport/remotepath_test.go b/internal/transport/remotepath_test.go new file mode 100644 index 0000000..cd1dd1a --- /dev/null +++ b/internal/transport/remotepath_test.go @@ -0,0 +1,45 @@ +package transport + +import "testing" + +func TestParseRemotePath(t *testing.T) { + tests := []struct { + name string + input string + want RemotePath + wantOK bool + }{ + {"user host path", "alice@example.com:/home/alice/data", RemotePath{User: "alice", Host: "example.com", Path: "/home/alice/data"}, true}, + {"host path no user", "example.com:/home/alice/data", RemotePath{Host: "example.com", Path: "/home/alice/data"}, true}, + {"relative path after colon", "example.com:data", RemotePath{Host: "example.com", Path: "data"}, true}, + {"ipv6 with user", "alice@[::1]:/data", RemotePath{User: "alice", Host: "::1", Path: "/data"}, true}, + {"ipv6 without user", "[2001:db8::1]:/data", RemotePath{Host: "2001:db8::1", Path: "/data"}, true}, + + {"plain relative local path", "src/file.txt", RemotePath{}, false}, + {"plain absolute posix path", "/home/alice/data", RemotePath{}, false}, + {"windows absolute path", `C:\Users\PC\file.txt`, RemotePath{}, false}, + {"windows forward-slash path", "C:/Users/PC/file.txt", RemotePath{}, false}, + {"bare drive letter", "C:", RemotePath{}, false}, + {"lowercase drive letter", `d:\data`, RemotePath{}, false}, + {"slash before colon disqualifies host", "some/dir:with:colons", RemotePath{}, false}, + {"at-sign in local path with slash before it disqualifies user", "some/dir@literal:path", RemotePath{}, false}, + {"empty string", "", RemotePath{}, false}, + {"malformed ipv6 missing close bracket", "[::1:path", RemotePath{}, false}, + {"malformed ipv6 no colon after bracket", "[::1]path", RemotePath{}, false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, ok := ParseRemotePath(tt.input) + if ok != tt.wantOK { + t.Fatalf("ParseRemotePath(%q) ok = %v, want %v (got %+v)", tt.input, ok, tt.wantOK, got) + } + if !ok { + return + } + if got != tt.want { + t.Errorf("ParseRemotePath(%q) = %+v, want %+v", tt.input, got, tt.want) + } + }) + } +} diff --git a/internal/transport/rsh.go b/internal/transport/rsh.go new file mode 100644 index 0000000..e42d7cc --- /dev/null +++ b/internal/transport/rsh.go @@ -0,0 +1,85 @@ +package transport + +import "strings" + +// DefaultRSH is the remote-shell command used when no --rsh/-e override is +// given, matching real rsync's own default. +const DefaultRSH = "ssh" + +// BuildRSHCommand builds the argv for invoking the remote shell to reach +// host (optionally as user@host) and run remoteArgs there, e.g. +// ["ssh", "user@host", "grsync", "--server"]. +// +// rsh is the raw --rsh/-e override string (e.g. "ssh -p 2222 -i key.pem"), +// or empty to use DefaultRSH. This is deliberately the *only* +// customization mechanism here - there's no separate --port or +// --identity flag, matching real rsync: upstream's --port only applies to +// daemon-mode (rsync://) connections, not the remote-shell transport, and +// its -i flag already means --itemize-changes, not "identity file" - so +// port/identity/ProxyJump/etc. are customized via -e (e.g. +// -e "ssh -p 2222 -i key.pem") or the user's own ~/.ssh/config, exactly as +// with real rsync. Inventing grsync-only flags for these would be a +// *worse* match for SC-1's parity goal, not a better one. +func BuildRSHCommand(rsh, user, host string, remoteArgs []string) []string { + fields := splitRSHCommand(rsh) + if len(fields) == 0 { + fields = []string{DefaultRSH} + } + + target := host + if user != "" { + target = user + "@" + host + } + + cmd := make([]string, 0, len(fields)+1+len(remoteArgs)) + cmd = append(cmd, fields...) + cmd = append(cmd, target) + cmd = append(cmd, remoteArgs...) + return cmd +} + +// splitRSHCommand splits an --rsh/-e command string into argv-style +// fields, honoring single- and double-quoted substrings so an argument +// containing spaces (e.g. -e `ssh -o "ProxyCommand=nc %h %p"`) survives as +// one field instead of being split apart - strings.Fields alone would +// silently mis-split exactly that case, which is common enough in real +// -e usage to be worth handling. +// +// This is not a full POSIX shell parser: no backslash escaping, no +// nested/mixed quotes, no variable expansion - just enough for the common +// case of one quoted argument. An unterminated quote is not treated as an +// error; whatever follows the opening quote to the end of the string +// becomes that field's content, which is a reasonable low-stakes fallback +// for malformed CLI input rather than something worth failing on. +func splitRSHCommand(s string) []string { + var fields []string + var current strings.Builder + var inQuote byte + + flush := func() { + if current.Len() > 0 { + fields = append(fields, current.String()) + current.Reset() + } + } + + for i := 0; i < len(s); i++ { + c := s[i] + switch { + case inQuote != 0: + if c == inQuote { + inQuote = 0 + } else { + current.WriteByte(c) + } + case c == '\'' || c == '"': + inQuote = c + case c == ' ' || c == '\t': + flush() + default: + current.WriteByte(c) + } + } + flush() + return fields +} diff --git a/internal/transport/rsh_test.go b/internal/transport/rsh_test.go new file mode 100644 index 0000000..9804fac --- /dev/null +++ b/internal/transport/rsh_test.go @@ -0,0 +1,70 @@ +package transport + +import "testing" + +func stringSlicesEqual(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + +func TestBuildRSHCommand_Default(t *testing.T) { + got := BuildRSHCommand("", "alice", "example.com", []string{"grsync", "--server"}) + want := []string{"ssh", "alice@example.com", "grsync", "--server"} + if !stringSlicesEqual(got, want) { + t.Errorf("BuildRSHCommand = %v, want %v", got, want) + } +} + +func TestBuildRSHCommand_NoUser(t *testing.T) { + got := BuildRSHCommand("", "", "example.com", []string{"grsync", "--server"}) + want := []string{"ssh", "example.com", "grsync", "--server"} + if !stringSlicesEqual(got, want) { + t.Errorf("BuildRSHCommand = %v, want %v", got, want) + } +} + +func TestBuildRSHCommand_CustomRSH(t *testing.T) { + got := BuildRSHCommand("ssh -p 2222 -i key.pem", "alice", "example.com", []string{"grsync", "--server"}) + want := []string{"ssh", "-p", "2222", "-i", "key.pem", "alice@example.com", "grsync", "--server"} + if !stringSlicesEqual(got, want) { + t.Errorf("BuildRSHCommand = %v, want %v", got, want) + } +} + +func TestBuildRSHCommand_QuotedArgumentSurvivesAsOneField(t *testing.T) { + got := BuildRSHCommand(`ssh -o "ProxyCommand=nc %h %p"`, "", "example.com", nil) + want := []string{"ssh", "-o", "ProxyCommand=nc %h %p", "example.com"} + if !stringSlicesEqual(got, want) { + t.Errorf("BuildRSHCommand = %v, want %v", got, want) + } +} + +func TestSplitRSHCommand(t *testing.T) { + tests := []struct { + name string + input string + want []string + }{ + {"empty", "", nil}, + {"single word", "ssh", []string{"ssh"}}, + {"multiple words", "ssh -p 2222", []string{"ssh", "-p", "2222"}}, + {"double quoted argument", `ssh -o "a b c"`, []string{"ssh", "-o", "a b c"}}, + {"single quoted argument", `ssh -o 'a b c'`, []string{"ssh", "-o", "a b c"}}, + {"extra whitespace collapses", "ssh -p 2222", []string{"ssh", "-p", "2222"}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := splitRSHCommand(tt.input) + if !stringSlicesEqual(got, tt.want) { + t.Errorf("splitRSHCommand(%q) = %v, want %v", tt.input, got, tt.want) + } + }) + } +} diff --git a/internal/transport/session.go b/internal/transport/session.go new file mode 100644 index 0000000..8fc4d97 --- /dev/null +++ b/internal/transport/session.go @@ -0,0 +1,101 @@ +package transport + +import ( + "bytes" + "fmt" + "io" + "os/exec" + "strings" +) + +// Session wraps a running remote-shell subprocess (e.g. ssh), exposing +// its stdin/stdout as a single Read/Write pair so the framed protocol +// (frame.go) can be layered directly on top without the caller needing +// to know this is a subprocess at all. +type Session struct { + cmd *exec.Cmd + stdin io.WriteCloser + stdout io.ReadCloser + stderr *bytes.Buffer +} + +// Dial spawns the remote-shell command built by BuildRSHCommand (ssh, or +// whatever --rsh/-e overrides it to) and returns a Session wrapping its +// stdin/stdout. The subprocess's stderr is captured (not connected to +// this process's own stderr) so it can be surfaced as part of a +// meaningful error from Close if the process exits non-zero. +// +// Host-key verification is never touched here: this deliberately never +// adds flags like "-o StrictHostKeyChecking=no" or a null +// UserKnownHostsFile. Whatever the invoked command (ssh by default) does +// by default - checking known_hosts, prompting or failing on an unknown +// or changed host key - is exactly what happens, unmodified. There is no +// stubbed-out or weakened host-key behavior to document here because none +// of that logic is reimplemented at all; it's entirely the system ssh +// client's own, unchanged behavior. +func Dial(rsh, user, host string, remoteArgs []string) (*Session, error) { + argv := BuildRSHCommand(rsh, user, host, remoteArgs) + cmd := exec.Command(argv[0], argv[1:]...) + + stdin, err := cmd.StdinPipe() + if err != nil { + return nil, fmt.Errorf("creating stdin pipe: %w", err) + } + stdout, err := cmd.StdoutPipe() + if err != nil { + // StdinPipe already succeeded above, and Start is never reached + // on this path - so nothing will ever close that pipe's file + // descriptor automatically (that's normally Cmd.Wait's job, and + // Wait only does it once Start has actually run). Verified + // against golang/go#58369, which documents this exact gap: Start + // itself failing *does* clean up already-created pipes, but never + // reaching Start at all does not. Close stdin explicitly here + // rather than leak it. + _ = stdin.Close() + return nil, fmt.Errorf("creating stdout pipe: %w", err) + } + + var stderr bytes.Buffer + cmd.Stderr = &stderr + + if err := cmd.Start(); err != nil { + // Unlike the StdoutPipe case above, Start failing here *does* + // close both already-created pipes as part of its own documented + // cleanup (golang/go#58369) - nothing further to release. + return nil, fmt.Errorf("starting %q: %w", argv[0], err) + } + + return &Session{cmd: cmd, stdin: stdin, stdout: stdout, stderr: &stderr}, nil +} + +// Read reads from the subprocess's stdout. +func (s *Session) Read(p []byte) (int, error) { return s.stdout.Read(p) } + +// Write writes to the subprocess's stdin. +func (s *Session) Write(p []byte) (int, error) { return s.stdin.Write(p) } + +// Close closes the subprocess's stdin (signaling EOF to the remote side) +// and waits for it to exit. +// +// stdout is deliberately not closed here: cmd.Wait's own documentation +// states it closes any pipe created via StdoutPipe automatically once the +// command exits, and that closing it earlier is incorrect if reads from +// it haven't all completed yet - so an explicit s.stdout.Close() here +// would either be redundant or actively wrong, depending on timing. +// +// If the process exited with an error, that error is enriched with +// whatever the subprocess wrote to stderr: a bare "exit status 255" is +// nearly useless without the diagnostic message ssh itself printed +// explaining why (unknown host, auth failure, connection refused, ...). +func (s *Session) Close() error { + stdinErr := s.stdin.Close() + waitErr := s.cmd.Wait() + + if waitErr != nil { + if msg := strings.TrimSpace(s.stderr.String()); msg != "" { + return fmt.Errorf("%w: %s", waitErr, msg) + } + return waitErr + } + return stdinErr +}