feat: dry-run also tests writeability of files

Author: alexeyu

cmd/uploadfun/main.go

@@ -56,8 +56,9 @@ func parseArgs(args []string, stdout, stderr io.Writer) (*cliOptions, int) {
 		"print the full event stream, including byte-level progress")
 	fs.BoolVar(&opts.json, "json", false, "format output as newline-delimited JSON")
 	fs.BoolVar(&opts.dryRun, "dry-run", false,
-		"connect and authenticate to each endpoint and report how many files "+
-			"would upload; no transfer, no delete, no writes")
+		"connect to each endpoint, verify the target is writable via a "+
+			"self-deleting probe file, and report how many files would upload; "+
+			"never touches the actual files being sent")
 	fs.BoolVar(&opts.noVerify, "no-verify", false, "disable post-upload size/hash verification")
 	fs.BoolVar(&opts.version, "version", false, "print version and exit")
 	fs.Usage = func() {

cmd/uploadfun/printer.go

@@ -122,7 +122,11 @@ func (p *printer) handle(ev uploadfun.UploadEvent) {
 			p.write(p.stderr, e, fmt.Sprintf("[%s] dry-run failed: %s", e.Endpoint, e.Err))
 			return
 		}
-		msg := fmt.Sprintf("[%s] dry-run ok: would upload %d files", e.Endpoint, e.Files)
+		msg := fmt.Sprintf(
+			"[%s] dry-run ok: reachable and writable, would upload %d files",
+			e.Endpoint,
+			e.Files,
+		)
 		p.writeUnlessQuiet(e, msg)
 	}
 }

dispatch.go

@@ -2,6 +2,7 @@ package uploadfun
 
 import (
 	"context"
+	"crypto/rand"
 	"errors"
 	"fmt"
 	"os"
@@ -295,8 +296,9 @@ func (w *endpointWorker) sleepBeforeRetry(attempt int) {
 }
 
 // runDryRun performs the --dry-run preflight for one endpoint: connect and
-// authenticate to prove the endpoint is reachable, disconnect, and report
-// how many files a real run would upload - never touching any file.
+// authenticate, prove the target directory is writable by round-tripping a
+// throwaway probe file, then report how many files a real run would upload -
+// never touching the actual files being sent.
 func runDryRun(
 	ctx context.Context,
 	up uploader,
@@ -311,10 +313,60 @@ func runDryRun(
 		events <- DryRunEvent{Endpoint: ep.Name, Err: err}
 		return
 	}
-	_ = up.Disconnect(ctx)
+	defer func() { _ = up.Disconnect(ctx) }()
+
+	if err := probeWritable(ctx, up); err != nil {
+		events <- DryRunEvent{Endpoint: ep.Name, Err: err}
+		return
+	}
 	events <- DryRunEvent{Endpoint: ep.Name, Files: len(files)}
 }
 
+// probeWritable proves the endpoint's target directory accepts writes by
+// uploading a tiny uniquely-named file and deleting it. This is the one
+// deliberate remote mutation a dry run makes; the probe exists on the
+// server only between its upload and its delete.
+func probeWritable(ctx context.Context, up uploader) error {
+	local, err := newProbeFile()
+	if err != nil {
+		return err
+	}
+	defer func() { _ = os.Remove(local) }()
+
+	remote := probeRemoteName()
+	if err := up.Upload(ctx, local, remote, func(sent, total int64) {}); err != nil {
+		return fmt.Errorf("write probe: %w", err)
+	}
+	if err := up.Delete(ctx, remote); err != nil {
+		return fmt.Errorf("write probe cleanup: left %q on server: %w", remote, err)
+	}
+	return nil
+}
+
+// newProbeFile writes a throwaway local file for the write probe and
+// returns its path; the caller removes it.
+func newProbeFile() (string, error) {
+	f, err := os.CreateTemp("", "uploadfun-probe-*")
+	if err != nil {
+		return "", err
+	}
+	defer func() { _ = f.Close() }()
+	if _, err := f.WriteString("uploadfun dry-run write probe\n"); err != nil {
+		_ = os.Remove(f.Name())
+		return "", err
+	}
+	return f.Name(), nil
+}
+
+// probeRemoteName returns a collision-resistant name so the probe never
+// clashes with a real upload or a concurrent endpoint's probe. It avoids a
+// leading dot on purpose - servers like pure-ftpd reject dotfile writes.
+func probeRemoteName() string {
+	var b [8]byte
+	_, _ = rand.Read(b[:])
+	return fmt.Sprintf("uploadfun-probe-%x.tmp", b)
+}
+
 func failAllFiles(ep Endpoint, files []string, err error, events chan<- UploadEvent) {
 	for _, f := range files {
 		events <- FileErrorEvent{Endpoint: ep.Name, File: f, Attempt: 1, Reason: err.Error(), Err: err}

dispatch_test.go

@@ -663,15 +663,47 @@ func TestDispatchDryRunSuccess(t *testing.T) {
 		t.Errorf("expected Files to report %d planned uploads, got %d", len(files), dr.Files)
 	}
 
-	if f.uploadCalls != 0 || f.deleteCalls != 0 {
-		t.Errorf("expected no upload/delete calls during a dry run, got uploadCalls=%d deleteCalls=%d",
-			f.uploadCalls, f.deleteCalls)
+	if f.uploadCalls != 1 || f.deleteCalls != 1 {
+		t.Errorf(
+			"expected exactly 1 upload/delete for the write probe, got uploadCalls=%d deleteCalls=%d",
+			f.uploadCalls,
+			f.deleteCalls,
+		)
 	}
 	if f.disconnectCalls != 1 {
 		t.Errorf("expected exactly 1 disconnect, got %d", f.disconnectCalls)
 	}
 }
 
+func TestDispatchDryRunWriteProbeFailure(t *testing.T) {
+	f := &fakeUploader{failUploadN: 999}
+	withFakeUploader(t, f)
+
+	events := collectEvents(Upload(
+		context.Background(), []string{"a.jpg"}, []Endpoint{testEndpoint("ep1")}, Options{DryRun: true},
+	))
+
+	if len(events) != 1 {
+		t.Fatalf("expected exactly 1 event, got %d: %+v", len(events), events)
+	}
+	dr, ok := events[0].(DryRunEvent)
+	if !ok {
+		t.Fatalf("expected a DryRunEvent, got %T", events[0])
+	}
+	if dr.Err == nil {
+		t.Error("expected a write-probe error")
+	}
+	if dr.Files != 0 {
+		t.Errorf("expected Files=0 when the write probe fails, got %d", dr.Files)
+	}
+	if f.deleteCalls != 0 {
+		t.Errorf("expected no delete after a failed probe upload, got %d", f.deleteCalls)
+	}
+	if f.disconnectCalls != 1 {
+		t.Errorf("expected exactly 1 disconnect after a probe failure, got %d", f.disconnectCalls)
+	}
+}
+
 func TestDispatchDryRunConnectFailure(t *testing.T) {
 	f := &fakeUploader{failConnectN: 999}
 	withFakeUploader(t, f)

integration_test.go

@@ -234,3 +234,53 @@ func TestIntegrationSFTPPermissionDeniedGivesUpEndToEnd(t *testing.T) {
 		t.Errorf("expected the second file to be reported skipped, got %+v", givenUp.SkippedFiles)
 	}
 }
+
+// TestIntegrationDryRunWriteProbeEndToEnd proves the --dry-run write probe
+// against real servers: it succeeds on the writable FTPS target and, on the
+// SFTP endpoint whose chroot root is unwritable, catches the exact
+// permission problem TestIntegrationSFTPPermissionDeniedGivesUpEndToEnd
+// shows a real run only discovers mid-transfer - which is the whole point
+// of preflighting writability.
+func TestIntegrationDryRunWriteProbeEndToEnd(t *testing.T) {
+	t.Run("passes on a writable endpoint", func(t *testing.T) {
+		endpoints, err := LoadConfig(rootFTPSConfig(t, "true"))
+		if err != nil {
+			t.Fatalf("LoadConfig: %v", err)
+		}
+
+		events := collectEvents(Upload(
+			context.Background(), []string{"a.jpg", "b.jpg"}, endpoints, Options{DryRun: true},
+		))
+
+		dr := singleDryRunEvent(t, events)
+		if dr.Err != nil {
+			t.Errorf("expected the write probe to pass on a writable endpoint, got %v", dr.Err)
+		}
+		if dr.Files != 2 {
+			t.Errorf("expected Files=2, got %d", dr.Files)
+		}
+	})
+
+	t.Run("fails on an unwritable target", func(t *testing.T) {
+		events := collectEvents(Upload(
+			context.Background(), []string{"a.jpg"}, []Endpoint{rootSFTPEndpoint()}, Options{DryRun: true},
+		))
+
+		dr := singleDryRunEvent(t, events)
+		if dr.Err == nil {
+			t.Error("expected the write probe to fail on an unwritable target")
+		}
+	})
+}
+
+func singleDryRunEvent(t *testing.T, events []UploadEvent) DryRunEvent {
+	t.Helper()
+	if len(events) != 1 {
+		t.Fatalf("expected exactly 1 event for a dry run, got %d: %+v", len(events), events)
+	}
+	dr, ok := events[0].(DryRunEvent)
+	if !ok {
+		t.Fatalf("expected a DryRunEvent, got %T", events[0])
+	}
+	return dr
+}

uploadfun.go

@@ -78,9 +78,10 @@ type Options struct {
 	// NoVerify disables the post-upload size/hash verification that's on
 	// by default.
 	NoVerify bool
-	// DryRun connects and authenticates per endpoint and reports how many
-	// files a real run would upload, without transferring, deleting, or
-	// writing anything.
+	// DryRun connects and authenticates per endpoint, verifies the target
+	// directory is writable by round-tripping a throwaway probe file, and
+	// reports how many files a real run would upload - without touching any
+	// of the actual files being sent.
 	DryRun bool
 }
 
@@ -194,17 +195,18 @@ type EndpointDoneEvent struct {
 func (EndpointDoneEvent) uploadEvent() {}
 
 // DryRunEvent reports the outcome of a --dry-run preflight for one
-// endpoint: connect and authenticate to prove it's reachable, then report
-// how many files a real run would upload, without transferring anything.
-// Exactly one is sent per endpoint when Options.DryRun is set, replacing
-// the per-file events.
+// endpoint: connect and authenticate to prove it's reachable, probe the
+// target directory for writability, then report how many files a real run
+// would upload. Exactly one is sent per endpoint when Options.DryRun is
+// set, replacing the per-file events.
 type DryRunEvent struct {
 	Endpoint string `json:"endpoint"`
 	// Files is how many files a real run would upload to this endpoint;
 	// meaningful only when Err is nil.
 	Files int `json:"files"`
-	// Err is set if connecting or authenticating failed; nil means the
-	// endpoint is reachable and Files reflects the planned upload.
+	// Err is set if connecting, authenticating, or the write probe failed;
+	// nil means the endpoint is reachable and writable and Files reflects
+	// the planned upload.
 	Err error `json:"-"`
 }