From 40a3b78faabc75f161d3b8640d33d7468add6cb2 Mon Sep 17 00:00:00 2001 From: Oluwatobi Ogundimu Date: Sun, 2 Aug 2026 19:49:18 +0100 Subject: [PATCH] SC-15: test suite, benchmarks, and CI/CD pipeline Real-rsync comparison tests (basic recursive, filtered, attribute preservation) - behavioral equivalence against real rsync's output, not wire-protocol interop (grsync's gob-based wire format was already established as incompatible with real rsync's, per SC-6/SC-9/SC-13/ SC-16). Skip gracefully without rsync on PATH. 6 fuzz targets across sync/daemon/transport, each checking one explicit invariant: checksum roll-vs-scratch equivalence, delta round-trip correctness, filter matcher never panics, greeting parser never panics, line reader respects its length cap, frame decoder never panics or over-allocates. BenchmarkGenerateDelta (3 sizes x 4 change-percentages) and BenchmarkGenerateSignature. CI: new test job (Linux+Windows matrix, -race on Linux only, real rsync installed on Linux only), new fuzz job (6 explicit 15s bursts), new vulncheck job. Real findings: - ci.yaml had build/lint jobs but no actual go test step at all - every test written across every prior ticket was only ever run locally, never enforced as a merge gate. Fixed. - grsync has no trailing-slash sensitivity on source paths (src and src/ behave identically), unlike real rsync's well-known distinction - undocumented anywhere before this ticket's comparison tests surfaced it. Now disclosed in the README. - Lint caught 3 unused-parameter findings in fuzz closures that only check 'never panics' - fixed via _, no suppressions. Local environment (CGO_ENABLED=0, no gcc) can't compile -race here; enforcement happens on CI's Linux leg, same pattern as the already-established missing local rsync/govulncheck binaries. Clean gofmt/build/vet/lint/test on native Windows and cross-compiled Linux. --- .github/workflows/ci.yaml | 97 ++++++++++ README.md | 103 ++++++++++- internal/cli/rsync_compare_test.go | 283 +++++++++++++++++++++++++++++ internal/daemon/fuzz_test.go | 67 +++++++ internal/sync/delta_bench_test.go | 105 +++++++++++ internal/sync/fuzz_test.go | 108 +++++++++++ internal/transport/fuzz_test.go | 38 ++++ 7 files changed, 800 insertions(+), 1 deletion(-) create mode 100644 internal/cli/rsync_compare_test.go create mode 100644 internal/daemon/fuzz_test.go create mode 100644 internal/sync/delta_bench_test.go create mode 100644 internal/sync/fuzz_test.go create mode 100644 internal/transport/fuzz_test.go diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 62b900e..b0c6298 100644 --- a/.github/workflows/ci.yaml +++ b/.github/workflows/ci.yaml @@ -39,3 +39,100 @@ jobs: - uses: golangci/golangci-lint-action@v9 with: version: v2.12 + + test: + name: test (${{ matrix.os }}) + strategy: + fail-fast: false + matrix: + os: [ubuntu-latest, windows-latest] + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v6 + + - uses: actions/setup-go@v6 + with: + go-version-file: go.mod + + # Real-rsync comparison tests (internal/cli/rsync_compare_test.go) + # skip gracefully without a real rsync binary on PATH - the same + # established pattern this project's SSH tests already use for an + # environment-dependent capability - but leaving that skip as the + # *only* thing CI ever exercises would mean those tests never + # actually run for real anywhere. Installing rsync here, on the + # Linux leg only, guarantees they get a genuine execution on at + # least one platform; windows-latest has no equivalent easy + # install, so it continues to exercise (and prove) the graceful + # skip path instead, exactly as a Windows dev machine without + # rsync already does locally. + - name: install rsync (Linux only, for real-rsync comparison tests) + if: runner.os == 'Linux' + run: sudo apt-get update && sudo apt-get install -y rsync + + # -race requires cgo (it links a C-based runtime), which is + # reliably available out of the box on ubuntu-latest but not + # something this project can verify is equally well-supported on + # windows-latest's own default toolchain without access to a real + # GitHub Actions runner to test against - so -race runs where it's + # known-good (Linux), and windows-latest still gets a full, + # real, native (not cross-compiled) test run without it. The + # actual value -race provides - catching a genuine data race in + # this project's own logic (SC-10's progress-reporter goroutine, + # SC-6's per-connection daemon handling) - isn't platform-specific + # in what it would find, so one platform running it is enough to + # get that benefit. + - name: go test -race (Linux) + if: runner.os == 'Linux' + run: go test -race -timeout 10m ./... + + - name: go test (Windows) + if: runner.os != 'Linux' + run: go test -timeout 10m ./... + + fuzz: + name: fuzz + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + + - uses: actions/setup-go@v6 + with: + go-version-file: go.mod + + # `go test ./...` alone only ever replays each Fuzz function's seed + # corpus as ordinary subtests - it does not actually fuzz anything + # (verified locally: a 10s -fuzz run finds thousands of new + # "interesting" corpus entries per target that a seed-only run + # never would). Each target gets its own explicit step, for a + # real, bounded fuzzing burst - not just existing, unexercised, in + # the source tree - and so a failure clearly names which target + # found it rather than a single opaque combined step. + - name: FuzzWeakChecksumRoll + run: go test ./internal/sync/... -run '^$' -fuzz '^FuzzWeakChecksumRoll$' -fuzztime 15s + + - name: FuzzRoundTripDelta + run: go test ./internal/sync/... -run '^$' -fuzz '^FuzzRoundTripDelta$' -fuzztime 15s + + - name: FuzzCompileAndMatch + run: go test ./internal/sync/... -run '^$' -fuzz '^FuzzCompileAndMatch$' -fuzztime 15s + + - name: FuzzReadGreeting + run: go test ./internal/daemon/... -run '^$' -fuzz '^FuzzReadGreeting$' -fuzztime 15s + + - name: FuzzReadLine + run: go test ./internal/daemon/... -run '^$' -fuzz '^FuzzReadLine$' -fuzztime 15s + + - name: FuzzReadFrame + run: go test ./internal/transport/... -run '^$' -fuzz '^FuzzReadFrame$' -fuzztime 15s + + vulncheck: + name: govulncheck + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + + - uses: actions/setup-go@v6 + with: + go-version-file: go.mod + + - uses: golang/govulncheck-action@v1 diff --git a/README.md b/README.md index 9c9dd22..3a31196 100644 --- a/README.md +++ b/README.md @@ -53,7 +53,13 @@ covers and where it hands off to grsync's own (non-rsync-wire-format) transfer protocol. `--ipv4`/`-4`, `--ipv6`/`-6`, and `--address` now genuinely control which address family and local address every transport uses, matching real rsync's own documented scope for each (see -[IPv4/IPv6 Support](#ipv4ipv6-support) below). +[IPv4/IPv6 Support](#ipv4ipv6-support) below). The test suite now also +includes real-rsync comparison integration tests, fuzz targets, and +throughput benchmarks, with `-race` and `govulncheck` wired into CI (see +[Testing](#testing) below) - including a genuine, previously-undisclosed +finding from building those comparison tests: grsync has no +trailing-slash sensitivity on source paths, unlike real rsync's `src` vs +`src/` distinction. ## Build @@ -1497,6 +1503,101 @@ first and skip gracefully (not fail) in an environment without a working IPv6 loopback, the same established pattern `requireLocalSSHServer` already uses for the SSH tests. +## Testing + +Beyond each section's own `### Testing` subsection above (which covers that +feature's specific tests), the project as a whole has four further layers: +real-rsync comparison integration tests, fuzz targets, throughput +benchmarks, and CI hardening (`-race`, `govulncheck`). + +### Real-rsync comparison tests + +`internal/cli/rsync_compare_test.go` runs grsync and a real `rsync` binary +independently against the same source tree and diffs the resulting +destination trees for equivalence. **This checks behavioral equivalence, +not wire-protocol interoperability** - the same distinction already +disclosed for batch mode, compression, and daemon mode above: grsync's +own sync protocol is gob-based, not a reimplementation of rsync's actual +wire format, so the two tools are never talking to each other here, only +being pointed at the same input and compared on output. If no `rsync` +binary is found on `PATH`, these tests skip (not fail), the same +established pattern the SSH tests already use for `requireLocalSSHServer`. + +Three scenarios are covered: a basic recursive sync, a filtered sync +(`--exclude`), and attribute preservation (permissions and mtime, with a +tolerance for filesystem timestamp-precision differences between temp +directories - a real, deliberate design choice to avoid false failures +from test-environment noise rather than a genuine correctness gap). Both +tools are invoked with plain `-r`/`-rpt`, never `-a`, specifically to +avoid owner/group comparisons that would be flaky or require root in CI. + +Building these tests surfaced a genuine, previously-undisclosed scope +boundary: **grsync has no trailing-slash sensitivity on source paths.** +Real rsync famously treats `src` (copy the directory itself into `dest`) +and `src/` (copy `src`'s contents into `dest`) differently; grsync's +`Walk` (`internal/sync/walk.go`) always excludes the root path from its +own output, i.e. it unconditionally behaves as if a trailing slash were +given, regardless of whether the CLI argument actually has one. The +comparison tests account for this by invoking real rsync with an explicit +trailing slash to match; anyone relying on grsync as an rsync CLI +substitute should be aware `src` and `src/` behave identically here, +unlike real rsync. + +### Fuzzing + +Six `testing.F` fuzz targets, each checking one explicit invariant rather +than fuzzing without a clear property in mind: + +- `FuzzWeakChecksumRoll` (`internal/sync`) - the rolling checksum's + incrementally-updated value always matches recomputing from scratch on + the same window, for any data and window size. If this ever broke, the + delta algorithm would silently miss or misidentify block matches. +- `FuzzRoundTripDelta` (`internal/sync`) - `ApplyDelta(old, GenerateDelta(sig, new), sig)` + always reproduces `new` exactly, for arbitrary (old, new) byte pairs. +- `FuzzCompileAndMatch` (`internal/sync`) - the filter pattern compiler + and matcher never panic on any pattern/path string, however malformed. +- `FuzzReadGreeting` and `FuzzReadLine` (`internal/daemon`) - the daemon's + greeting-line and shared line-reading code never panic on untrusted + network input, and `readLine` never returns a line longer than + `maxLineLength` regardless of how much unterminated data a peer sends + (the anti-DoS memory bound the code documents). +- `FuzzReadFrame` (`internal/transport`) - frame decoding never panics and + never attempts an oversized allocation on a corrupted or hostile + stream, the same protection that + `TestE2E_ReadBatchOfMalformedFileFailsClearly` (see + [Batch Mode](#batch-mode) above) already found catching one hand-picked + garbage file cleanly; this target explores far beyond that single case. + +`go test ./...` alone only replays each target's seed corpus as ordinary +subtests - it does not actually fuzz. CI runs each target through a real, +bounded `-fuzz -fuzztime 15s` burst in its own dedicated job step. + +### Benchmarks + +`internal/sync/delta_bench_test.go` benchmarks `GenerateDelta` (block +matching, the most performance-sensitive code in the project, run once +per file on every sync) and `GenerateSignature` across three file sizes +(10KB, 100KB, 1MB) and, for `GenerateDelta`, four change percentages (0%, +10%, 50%, 100%) - unmatched content falls back to an expensive +byte-by-byte incremental scan, so a file's similarity to its old version +matters as much as its size for real throughput. + +### CI + +`.github/workflows/ci.yaml` runs `go build`/`go vet`, `golangci-lint`, +`govulncheck`, the fuzz targets above, and `go test` on both Linux and +Windows runners. `-race` runs on the Linux leg only: it requires cgo and +a C toolchain, reliably available on `ubuntu-latest` but not something +this project can verify for `windows-latest` without a real runner to +test against, and the races it would actually catch (SC-10's +progress-reporter goroutine, SC-6's per-connection daemon handling) +aren't platform-specific in nature, so one reliable platform is enough. +The Linux leg also installs a real `rsync` binary so the comparison tests +above get a genuine execution somewhere in CI, not just a proof they can +skip; the Windows leg has no equivalent easy install and instead +exercises the graceful-skip path, matching a real Windows dev machine +without rsync. + ## Architecture - `cmd/grsync` - CLI entrypoint. diff --git a/internal/cli/rsync_compare_test.go b/internal/cli/rsync_compare_test.go new file mode 100644 index 0000000..8c681a4 --- /dev/null +++ b/internal/cli/rsync_compare_test.go @@ -0,0 +1,283 @@ +// rsync_compare_test.go implements SC-15's "run grsync against a real +// rsync binary and diff the results" requirement. +// +// What this can and can't mean: grsync's actual on-the-wire protocol is +// gob-encoded (see internal/pipeline/messages.go's own "Encoding note"), +// not real rsync's binary wire format - a deliberate, disclosed scope +// boundary every wire-touching ticket (SC-6, SC-9, SC-13, SC-16) has +// established. grsync and a real rsync process cannot talk to each +// other over a live connection. What these tests actually do instead is +// run each tool *independently* against the same source tree and diff +// their *resulting output* - the synced destination's file contents and +// structure, and (separately, tolerant of filesystem precision) each +// tool's own attribute preservation against the source - which is a +// genuine, meaningful behavioral-equivalence check without requiring +// (or claiming) wire-protocol interoperability. +package cli + +import ( + "os" + "os/exec" + "path/filepath" + "sort" + "testing" + "time" +) + +// requireRealRsync skips the calling test if no real rsync binary is on +// PATH - this project's established pattern (see requireLocalSSHServer) +// for a capability the test environment may or may not have, rather than +// failing CI on a runner without one. +func requireRealRsync(t *testing.T) string { + t.Helper() + path, err := exec.LookPath("rsync") + if err != nil { + t.Skipf("no real rsync binary found on PATH, skipping real-rsync comparison test: %v", err) + } + return path +} + +// runRealRsync runs the real rsync binary with args, failing the test +// with its combined output if it exits non-zero - so a genuine rsync-side +// failure (e.g. a flag this rsync build doesn't support) is diagnosable +// from the test output, not just "some exec error". +func runRealRsync(t *testing.T, rsyncPath string, args ...string) { + t.Helper() + cmd := exec.Command(rsyncPath, args...) + out, err := cmd.CombinedOutput() + if err != nil { + t.Fatalf("real rsync %v failed: %v\n%s", args, err, out) + } +} + +// buildComparisonSourceTree creates a small, deterministic tree exercising +// recursion and nested directories, shared by every comparison test below +// so each test's own assertions are about the flags/behavior actually +// under test, not about constructing fixtures. Deliberately no symlinks +// or hard links: real rsync's own handling of those depends on privilege +// and platform in ways that would risk exactly the kind of environment +// noise this ticket's self-review specifically asks to be distinguished +// from a genuine correctness gap - content/structure/basic-attribute +// comparison is where a real, meaningful behavioral check can be made +// without that risk. +func buildComparisonSourceTree(t *testing.T) string { + t.Helper() + src := t.TempDir() + mustWriteFile(t, filepath.Join(src, "top.txt"), "top level content") + mustWriteFile(t, filepath.Join(src, "top.log"), "a log file, to be excluded by the filtered test") + mustMkdirAll(t, filepath.Join(src, "sub")) + mustWriteFile(t, filepath.Join(src, "sub", "nested.txt"), "nested content") + mustMkdirAll(t, filepath.Join(src, "sub", "deeper")) + mustWriteFile(t, filepath.Join(src, "sub", "deeper", "f.txt"), "deeper content") + return src +} + +// treeFile is one regular file's content and (optionally checked) mode, +// keyed by its slash-separated path relative to the tree's root. +type treeFile struct { + relPath string + content string + mode os.FileMode + modTime time.Time +} + +// snapshotTree walks root and returns every regular file found, sorted by +// relPath for a deterministic comparison order. Directories are not +// separately recorded: a directory's presence is already implied by any +// file underneath it, and this project's own attribute-preservation +// tests elsewhere already cover directory-specific attribute handling in +// detail - duplicating that here would test the same thing twice without +// adding confidence. +func snapshotTree(t *testing.T, root string) []treeFile { + t.Helper() + var files []treeFile + err := filepath.Walk(root, func(path string, info os.FileInfo, err error) error { + if err != nil { + return err + } + if info.IsDir() { + return nil + } + rel, err := filepath.Rel(root, path) + if err != nil { + return err + } + content, err := os.ReadFile(path) + if err != nil { + return err + } + files = append(files, treeFile{ + relPath: filepath.ToSlash(rel), + content: string(content), + mode: info.Mode(), + modTime: info.ModTime(), + }) + return nil + }) + if err != nil { + t.Fatalf("walking %q: %v", root, err) + } + sort.Slice(files, func(i, j int) bool { return files[i].relPath < files[j].relPath }) + return files +} + +// assertSameStructureAndContent confirms got and want cover exactly the +// same set of relative paths with exactly the same content - the core +// "did these two tools produce the same result" check, shared by every +// comparison test regardless of which flags were used to produce it. +func assertSameStructureAndContent(t *testing.T, label string, got, want []treeFile) { + t.Helper() + gotPaths := make(map[string]string, len(got)) + for _, f := range got { + gotPaths[f.relPath] = f.content + } + wantPaths := make(map[string]string, len(want)) + for _, f := range want { + wantPaths[f.relPath] = f.content + } + + for path, wantContent := range wantPaths { + gotContent, ok := gotPaths[path] + if !ok { + t.Errorf("%s: %q present in real rsync's output but missing from grsync's", label, path) + continue + } + if gotContent != wantContent { + t.Errorf("%s: %q content differs: grsync=%q real-rsync=%q", label, path, gotContent, wantContent) + } + } + for path := range gotPaths { + if _, ok := wantPaths[path]; !ok { + t.Errorf("%s: %q present in grsync's output but missing from real rsync's", label, path) + } + } +} + +// TestRealRsyncComparison_BasicRecursiveSync is Part 2's first required +// case: a plain recursive sync, comparing the resulting destination +// trees' structure and content. +// +// Deliberately -r, not -a: -a implies owner/group preservation, which +// needs root to succeed cleanly on most systems - a CI runner (or a +// developer's own machine) not running as root is normal, expected +// environment variance, not a correctness signal either tool should be +// judged on here. Structure/content is what this test is actually +// about; TestRealRsyncComparison_AttributePreservation covers perms/ +// times specifically, deliberately excluding owner/group for the same +// reason. +func TestRealRsyncComparison_BasicRecursiveSync(t *testing.T) { + rsyncPath := requireRealRsync(t) + src := buildComparisonSourceTree(t) + + // Real rsync's own trailing-slash convention ("src" copies the + // directory itself into dest; "src/" copies its contents into dest) + // is not implemented by grsync at all - internal/sync.Walk always + // treats its root argument as "copy the contents of this," the + // equivalent of real rsync's trailing-slash form, regardless of + // what the user actually typed (verified by reading Walk's own + // code: the root path itself is always excluded from its own + // output). This is a real, previously-undisclosed CLI convention + // difference this comparison suite surfaced - real rsync is + // therefore deliberately invoked with a trailing slash on src below + // to match grsync's own actual behavior, not to work around a bug in + // this test. + grsyncDest := t.TempDir() + if err := runGrsync(t, "-r", src, grsyncDest); err != nil { + t.Fatalf("grsync sync returned error: %v", err) + } + + realDest := t.TempDir() + runRealRsync(t, rsyncPath, "-r", src+"/", realDest+"/") + + assertSameStructureAndContent(t, "basic recursive sync", snapshotTree(t, grsyncDest), snapshotTree(t, realDest)) +} + +// TestRealRsyncComparison_FilteredSync is Part 2's second required case: +// confirms grsync's --exclude produces the same resulting tree as real +// rsync's own --exclude for the same pattern - this project's filter +// engine was built specifically to match real rsync's own documented +// syntax and semantics (see internal/sync/filter.go and its own tests), +// so this is a real, meaningful cross-check of that claim against an +// actual rsync binary, not just this project's own tests of itself. +func TestRealRsyncComparison_FilteredSync(t *testing.T) { + rsyncPath := requireRealRsync(t) + src := buildComparisonSourceTree(t) + + grsyncDest := t.TempDir() + if err := runGrsync(t, "-r", "--exclude=*.log", src, grsyncDest); err != nil { + t.Fatalf("grsync sync returned error: %v", err) + } + + realDest := t.TempDir() + runRealRsync(t, rsyncPath, "-r", "--exclude=*.log", src+"/", realDest+"/") + + gotFiles := snapshotTree(t, grsyncDest) + wantFiles := snapshotTree(t, realDest) + assertSameStructureAndContent(t, "filtered sync", gotFiles, wantFiles) + + // Belt-and-suspenders: confirm the exclusion genuinely took effect + // for both tools, not just that they happened to agree (which could + // also happen if --exclude were silently a no-op on both sides). + for _, f := range append(gotFiles, wantFiles...) { + if filepath.Ext(f.relPath) == ".log" { + t.Errorf("top.log present in a --exclude=*.log result (relPath=%q) - the exclusion did not take effect", f.relPath) + } + } +} + +// TestRealRsyncComparison_AttributePreservation is Part 2's third +// required case: confirms grsync's -p/-t preserve permissions/mtimes at +// least as well as real rsync's own do, for the same source. +// +// This deliberately does NOT compare grsync's destination attributes +// directly against real rsync's destination attributes bit-for-bit - +// see this ticket's own self-review requirement about distinguishing a +// real correctness gap from environment noise. Instead, each tool's own +// output is compared against the SOURCE independently: this sidesteps +// filesystem timestamp-precision differences between the two temp +// directories entirely (both could legitimately round to different +// sub-second precision without either tool being wrong), while still +// proving both tools achieve the same real property - "the destination's +// attributes match the source's." +func TestRealRsyncComparison_AttributePreservation(t *testing.T) { + rsyncPath := requireRealRsync(t) + src := buildComparisonSourceTree(t) + + // A distinctive, easy-to-misidentify-as-"now" mtime, matching this + // project's own established pattern elsewhere (see + // TestSenderReceiver_DirectoryAttributesSurviveChildCreation) for + // making an mtime-preservation assertion meaningful instead of + // trivially true. + distinctiveTime := time.Date(2019, time.May, 4, 10, 30, 0, 0, time.UTC) + srcFile := filepath.Join(src, "top.txt") + if err := os.Chtimes(srcFile, distinctiveTime, distinctiveTime); err != nil { + t.Fatalf("Chtimes: %v", err) + } + srcInfo, err := os.Stat(srcFile) + if err != nil { + t.Fatalf("Stat source: %v", err) + } + + grsyncDest := t.TempDir() + if err := runGrsync(t, "-rpt", src, grsyncDest); err != nil { + t.Fatalf("grsync sync returned error: %v", err) + } + realDest := t.TempDir() + runRealRsync(t, rsyncPath, "-rpt", src+"/", realDest+"/") + + const mtimeTolerance = 2 * time.Second // filesystem timestamp precision varies; see this test's own doc comment + for label, dest := range map[string]string{"grsync": grsyncDest, "real rsync": realDest} { + destFile := filepath.Join(dest, "top.txt") + destInfo, err := os.Stat(destFile) + if err != nil { + t.Fatalf("%s: Stat destination: %v", label, err) + } + if destInfo.Mode().Perm() != srcInfo.Mode().Perm() { + t.Errorf("%s: destination perms = %v, want %v (matching source, -p was given)", label, destInfo.Mode().Perm(), srcInfo.Mode().Perm()) + } + diff := destInfo.ModTime().Sub(distinctiveTime) + if diff < -mtimeTolerance || diff > mtimeTolerance { + t.Errorf("%s: destination mtime = %v, want within %v of %v (source's, -t was given)", label, destInfo.ModTime(), mtimeTolerance, distinctiveTime) + } + } +} diff --git a/internal/daemon/fuzz_test.go b/internal/daemon/fuzz_test.go new file mode 100644 index 0000000..d419568 --- /dev/null +++ b/internal/daemon/fuzz_test.go @@ -0,0 +1,67 @@ +package daemon + +import ( + "bufio" + "strings" + "testing" +) + +// FuzzReadGreeting is SC-15's fuzz target for the daemon protocol's own +// greeting-line parsing (the "@RSYNCD: VERSION.SUB ..." line every +// connection starts with, before any authentication happens - genuinely +// untrusted network input, arriving before the peer has proven anything +// about itself). The property checked is that readGreeting never panics +// for any line content, however malformed - its own prefix check, +// strings.Fields split, and two strconv.Atoi calls all look +// defensively coded already, but fuzzing confirms that holds for inputs +// nobody thought to hand-write, not just the cases +// TestReadGreeting-style unit tests already cover. +func FuzzReadGreeting(f *testing.F) { + f.Add("@RSYNCD: 31.0\n") + f.Add("@RSYNCD: 30\n") + f.Add("not a greeting at all\n") + f.Add("@RSYNCD: \n") + f.Add("@RSYNCD: abc.def\n") + f.Add("@RSYNCD: 31.0.0.0.0\n") + f.Add("\n") + f.Add("") + + f.Fuzz(func(_ *testing.T, line string) { + r := bufio.NewReader(strings.NewReader(line)) + // readGreeting itself calls readLine, so a line with no trailing + // "\n" is a valid, expected input here too (readLine returns an + // error for it) - not appended manually, so this fuzzes the real + // end-to-end parsing path exactly as a live connection would + // present it. + _, _, _ = readGreeting(r) + }) +} + +// FuzzReadLine is SC-15's fuzz target for the single shared line-reading +// primitive every text-based phase of the daemon protocol (greeting, +// module selection, authentication) reads through - the one chokepoint +// genuinely untrusted network bytes always pass. The property checked is +// exactly what readLine's own doc comment promises: it never panics, and +// it never returns a line longer than maxLineLength, regardless of how +// much unterminated data a hostile or corrupted peer sends - the +// protection that keeps an attacker from forcing unbounded memory growth +// just by never sending a newline. +func FuzzReadLine(f *testing.F) { + f.Add("hello\n") + f.Add("\n") + f.Add("no trailing newline at all") + f.Add("line with \r\n crlf ending") + f.Add(strings.Repeat("x", maxLineLength*2) + "\n") + f.Add("") + + f.Fuzz(func(t *testing.T, data string) { + r := bufio.NewReader(strings.NewReader(data)) + line, err := readLine(r) + if err != nil { + return // a rejected/incomplete line is a valid, expected outcome + } + if len(line) > maxLineLength { + t.Fatalf("readLine returned a %d-byte line, want it bounded by maxLineLength (%d)", len(line), maxLineLength) + } + }) +} diff --git a/internal/sync/delta_bench_test.go b/internal/sync/delta_bench_test.go new file mode 100644 index 0000000..731ce07 --- /dev/null +++ b/internal/sync/delta_bench_test.go @@ -0,0 +1,105 @@ +package sync + +import ( + "fmt" + "math/rand" + "testing" +) + +// generateBenchData returns (oldData, newData), both n bytes long: oldData +// is pseudo-random (a fixed seed, so every run of a given size/percent +// combination benchmarks the identical input, not fresh randomness each +// time), and newData is derived from it by changing roughly +// changePercent% of its bytes, scattered at random positions rather than +// as one contiguous run - closer to how a real file's edits tend to be +// spread through it than a single block of difference would be. +// changePercent >= 100 instead generates a completely independent +// newData, guaranteeing "genuinely nothing matches" rather than relying +// on random flips to (probabilistically, imperfectly) cover every byte. +func generateBenchData(n, changePercent int, seed int64) (oldData, newData []byte) { + rng := rand.New(rand.NewSource(seed)) + oldData = make([]byte, n) + _, _ = rng.Read(oldData) + + newData = make([]byte, n) + if changePercent >= 100 { + _, _ = rng.Read(newData) + return oldData, newData + } + + copy(newData, oldData) + numChanged := n * changePercent / 100 + for i := 0; i < numChanged; i++ { + pos := rng.Intn(n) + newData[pos] = byte(rng.Intn(256)) + } + return oldData, newData +} + +// BenchmarkGenerateDelta measures block-matching throughput - almost +// certainly the most performance-sensitive code in this project, since +// it runs once per regular file on every sync, over the file's entire +// content. Varied across two independent dimensions, not just one +// arbitrary case: file size (does throughput hold up as files grow, or +// does something scale worse than linearly), and change percentage (a +// file that's mostly identical to its old version, real delta transfer's +// whole reason to exist, versus one that's mostly or entirely different, +// where nearly every window has to fall through to a literal DataOp +// instead of matching a block). +func BenchmarkGenerateDelta(b *testing.B) { + sizes := []int{10 * 1024, 100 * 1024, 1024 * 1024} + changePercents := []int{0, 10, 50, 100} + + for _, size := range sizes { + for _, pct := range changePercents { + name := fmt.Sprintf("size=%s/changed=%d%%", humanByteSize(size), pct) + b.Run(name, func(b *testing.B) { + oldData, newData := generateBenchData(size, pct, 42) + sig := GenerateSignature(oldData) + + b.ReportAllocs() + b.SetBytes(int64(size)) + b.ResetTimer() + for i := 0; i < b.N; i++ { + GenerateDelta(sig, newData) + } + }) + } + } +} + +// BenchmarkGenerateSignature measures the other half of a real sync's +// per-file cost on the receiving side: splitting the existing +// destination file into blocks and checksumming each one. Unlike +// GenerateDelta, signature generation doesn't depend on how similar the +// two files are - only on the existing file's own size - so this only +// varies that one dimension. +func BenchmarkGenerateSignature(b *testing.B) { + sizes := []int{10 * 1024, 100 * 1024, 1024 * 1024} + + for _, size := range sizes { + b.Run(humanByteSize(size), func(b *testing.B) { + data := make([]byte, size) + rng := rand.New(rand.NewSource(42)) + _, _ = rng.Read(data) + + b.ReportAllocs() + b.SetBytes(int64(size)) + b.ResetTimer() + for i := 0; i < b.N; i++ { + GenerateSignature(data) + } + }) + } +} + +func humanByteSize(n int) string { + switch { + case n >= 1024*1024: + return fmt.Sprintf("%dMB", n/(1024*1024)) + case n >= 1024: + return fmt.Sprintf("%dKB", n/1024) + default: + return fmt.Sprintf("%dB", n) + } +} diff --git a/internal/sync/fuzz_test.go b/internal/sync/fuzz_test.go new file mode 100644 index 0000000..bfec1bc --- /dev/null +++ b/internal/sync/fuzz_test.go @@ -0,0 +1,108 @@ +package sync + +import ( + "bytes" + "testing" +) + +// FuzzWeakChecksumRoll is SC-15's fuzz target for "the rolling/strong +// checksum code (SC-3)". The property it checks is the exact one +// TestWeakChecksum_RollMatchesFromScratch already proves for one hand- +// picked string and window size: roll() must always produce the same +// result as recomputing the checksum from scratch on the shifted +// window, for ANY data and ANY valid window size - not just the cases a +// human thought to write down. If this property ever broke, the delta +// algorithm would silently miss real block matches (or produce false +// ones only caught by the strong checksum, masking the bug), so this is +// worth exploring far beyond one fixed fixture. +func FuzzWeakChecksumRoll(f *testing.F) { + f.Add([]byte("the quick brown fox jumps over the lazy dog"), 8) + f.Add([]byte("aaaaaaaaaaaaaaaa"), 1) + f.Add([]byte{0, 0, 0, 0, 0, 0, 0, 0}, 4) + f.Add([]byte{}, 1) + + f.Fuzz(func(t *testing.T, data []byte, windowSize int) { + if windowSize <= 0 || windowSize > len(data) { + t.Skip() // not a valid window for this data; nothing to check + } + + current := newWeakChecksum(data[:windowSize]) + for offset := 1; offset+windowSize <= len(data); offset++ { + out := data[offset-1] + in := data[offset+windowSize-1] + current = current.roll(out, in) + + want := newWeakChecksum(data[offset : offset+windowSize]) + if current.sum() != want.sum() { + t.Fatalf("offset %d, window %d: rolled sum = %d, want %d (from scratch)", + offset, windowSize, current.sum(), want.sum()) + } + } + }) +} + +// FuzzRoundTripDelta is a bonus target beyond the ticket's own three +// named ones, extending TestRoundTrip's hand-picked (old, new) pairs to +// arbitrary fuzzer-generated ones. The property checked is the whole +// delta algorithm's own reason for existing: ApplyDelta(oldData, +// GenerateDelta(sig, newData), sig) must equal newData exactly, for any +// pair of byte slices, not just the identical/prepend/append/middle-edit +// cases TestRoundTrip already covers by hand. +// +// blockSize is fixed at 4 (much smaller than DefaultBlockSize's 700) +// specifically so small fuzzer-generated inputs still produce multiple +// blocks and exercise real CopyOp matching, not just a single +// all-literal DataOp - a large default block size would make most +// fuzz-generated inputs too short to ever test the matching path at +// all. +func FuzzRoundTripDelta(f *testing.F) { + f.Add([]byte("hello world"), []byte("hello there world")) + f.Add([]byte(""), []byte("brand new file")) + f.Add([]byte("identical content"), []byte("identical content")) + f.Add([]byte("this file shrinks a lot"), []byte("short")) + + f.Fuzz(func(t *testing.T, oldData, newData []byte) { + const maxFuzzInput = 1 << 16 // keep each iteration fast so many can run in a bounded CI time budget + if len(oldData) > maxFuzzInput || len(newData) > maxFuzzInput { + t.Skip() + } + + sig := GenerateSignatureWithBlockSize(oldData, 4) + ops := GenerateDelta(sig, newData) + got, err := ApplyDelta(oldData, ops, sig) + if err != nil { + t.Fatalf("ApplyDelta returned error: %v", err) + } + if !bytes.Equal(got, newData) { + t.Fatalf("round trip mismatch: got %d bytes, want %d bytes (oldData=%d bytes)", len(got), len(newData), len(oldData)) + } + }) +} + +// FuzzCompileAndMatch is SC-15's fuzz target for "the filter pattern +// matcher (SC-7)". The property checked is simply that neither +// compilePattern nor Rule.matches ever panics for any pattern/path +// string, however malformed - path.Match (which matchSegments calls per +// segment) can itself return a syntax error for a bad glob, which +// matchSegments already treats as "no match" rather than propagating, +// but that defensive handling is exactly the kind of thing worth +// fuzzing to confirm holds for inputs nobody thought to hand-write, +// including empty strings, strings that are entirely "/" or "**", and +// patterns with unbalanced "[" character classes. +func FuzzCompileAndMatch(f *testing.F) { + f.Add("*.txt", "dir/file.txt") + f.Add("/anchored/**/pattern", "anchored/deep/nested/pattern") + f.Add("**", "") + f.Add("[unclosed", "path") + f.Add("/", "/") + f.Add("a//b", "a/b") + + f.Fuzz(func(_ *testing.T, pattern, path string) { + rule, err := compilePattern(Include, pattern) + if err != nil { + return // a rejected pattern is a valid, expected outcome - nothing more to check + } + rule.matches(path, false) + rule.matches(path, true) + }) +} diff --git a/internal/transport/fuzz_test.go b/internal/transport/fuzz_test.go new file mode 100644 index 0000000..eba12ab --- /dev/null +++ b/internal/transport/fuzz_test.go @@ -0,0 +1,38 @@ +package transport + +import ( + "bytes" + "testing" +) + +// FuzzReadFrame is a bonus SC-15 target beyond the ticket's own three +// named ones: the literal "binary parsing of untrusted input" its own +// rationale describes, one layer below internal/daemon's text-based +// greeting/auth lines - every transport (local pipe, SSH session, and a +// daemon connection once its own text handshake ends) reads its actual +// payload frames through this exact function. The property checked is +// ReadFrame's own documented safety guarantee: it never panics, and it +// never attempts to allocate a payload larger than maxFramePayload, +// regardless of what bytes a corrupted stream or hostile peer sends - +// precisely the protection SC-13's own TestE2E_ReadBatchOfMalformedFileFailsClearly +// found catching a garbage --read-batch file cleanly rather than +// crashing; this fuzz target explores far beyond that one hand-picked +// case. +func FuzzReadFrame(f *testing.F) { + var valid bytes.Buffer + _ = WriteFrame(&valid, Frame{Type: FrameFileList, Payload: []byte("hello")}) + f.Add(valid.Bytes()) + + var empty bytes.Buffer + _ = WriteFrame(&empty, Frame{Type: FrameSignature, Payload: nil}) + f.Add(empty.Bytes()) + + f.Add([]byte{0xFF, 0xFF, 0xFF, 0xFF, 0x00}) // length prefix claiming ~4 GiB + f.Add([]byte{}) + f.Add([]byte{0, 0, 0, 0, 0}) + f.Add([]byte{1, 2, 3}) + + f.Fuzz(func(_ *testing.T, data []byte) { + _, _ = ReadFrame(bytes.NewReader(data)) + }) +}