SC-5: SSH transport layer

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,14 +165,75 @@ 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.
 - `internal/cli` - flag/argument parsing (built on cobra).
 - `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).

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)

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
+}

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")
+	}
+}