diff --git a/.golangci.yml b/.golangci.yml new file mode 100644 index 0000000..26b1610 --- /dev/null +++ b/.golangci.yml @@ -0,0 +1,34 @@ +version: "2" + +# Config schema uses the golangci-lint v2 layout (settings live under +# linters.settings, not top-level linters-settings) so that the +# thresholds below are actually applied by golangci-lint >= v2. + +run: + timeout: 5m + modules-download-mode: readonly + +linters: + default: all + disable: + # Genuinely incompatible with project patterns + - exhaustruct # Requires all struct fields + - depguard # Dependency allow/block lists + - godot # Requires comments to end with periods + - wsl # Deprecated, replaced by wsl_v5 + - wrapcheck # Too verbose for internal packages + - varnamelen # Short names like db, id are idiomatic Go + settings: + lll: + line-length: 88 + funlen: + lines: 80 + statements: 50 + cyclop: + max-complexity: 15 + dupl: + threshold: 100 + +issues: + max-issues-per-linter: 0 + max-same-issues: 0 diff --git a/Dockerfile b/Dockerfile index e6dd403..cf7db0b 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,6 +1,6 @@ # Lint stage — fast feedback on formatting and lint issues -# golangci/golangci-lint:v2.0.2 (2026-03-14) -FROM golangci/golangci-lint@sha256:d55581f7797e7a0877a7c3aaa399b01bdc57d2874d6412601a046cc4062cb62e AS lint +# golangci/golangci-lint:v2.12.2 (Debian-based), 2026-08-07 +FROM golangci/golangci-lint:v2.12.2@sha256:5cceeef04e53efe1470638d4b4b4f5ceefd574955ab3941b2d9a68a8c9ad5240 AS lint WORKDIR /src COPY go.mod go.sum ./ diff --git a/Makefile b/Makefile index 70df7e8..d9dbfa3 100644 --- a/Makefile +++ b/Makefile @@ -48,7 +48,7 @@ hooks: @script/install-precommit devprereqs: - which golangci-lint || go install -v github.com/golangci/golangci-lint/cmd/golangci-lint@v2.0.2 + which golangci-lint || go install -v github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.12.2 mfer/mf.pb.go: mfer/mf.proto cd mfer && go generate . diff --git a/TODO.md b/TODO.md index 6100b0d..983c596 100644 --- a/TODO.md +++ b/TODO.md @@ -16,13 +16,19 @@ in flight and unmerged. # Next Step -Land the in-flight compliance branch chore/align-repo-policies: finish and -commit the uncommitted work (32 modified Go files, new untracked -.golangci.yml and TODO.md), confirm `make check` is green, merge the branch -(one commit ahead of main as of 2026-07-03) to main, and push. +Work through the remaining compliance items folded from the 2026-07-02 +audit (the first group under Future Steps): `.editorconfig`, `.gitignore` +coverage, gofumpt-based `fmt-check`, README "Getting Started", and the +rest. `.golangci.yml` and `TODO.md` are tracked and committed as of +2026-08-07, so the only thing left of the `chore/align-repo-policies` +branch is the list below. # Completed Steps +- 2026-08-07: updated golangci-lint to v2.12.2 everywhere it is pinned + (`Makefile`, `Dockerfile`), added the canonical `.golangci.yml` + (`default: all`), and fixed all resulting lint findings across the + codebase - 2026-07-07 Adopted scripts-to-rule-them-all: `script/` entrypoints, Makefile shims, README Entrypoints section - 2026-07-03: aligned repo tooling, docs, and config with standardized @@ -45,8 +51,6 @@ commit the uncommitted work (32 modified Go files, new untracked - Compliance (fold of TODO.md audit 2026-07-02; verify which items the in-flight branch already closes, then check off): - Add .editorconfig (canonical copy from sneak/prompts) - - Add standardized .golangci.yml (present untracked on the branch; - user-owned, copy verbatim) - Make .gitignore cover secrets (.env, _.key, _.pem), OS files (.DS_Store), and editor files (_.swp, _~) - Make fmt-check/lint verify with gofumpt, not gofmt -l, so @@ -55,8 +59,8 @@ commit the uncommitted work (32 modified Go files, new untracked install/usage block - Move FORMAT.md from repo root to docs/ and update the AGENTS.md reference - - Pin Makefile-installed Go tools (protoc-gen-go@v1.28.1, - golangci-lint@v2.0.2) by module hash, not mutable tag + - Pin Makefile-installed Go tools (`protoc-gen-go@v1.28.1`, + `golangci-lint@v2.12.2`) by module hash, not mutable tag - Set `make test` timeout to 30s (currently 10s) - Add explicit README "Rationale" heading (content exists under other names); name the author in the README Description first line diff --git a/cmd/mfer/main.go b/cmd/mfer/main.go index af1255f..6c8fb04 100644 --- a/cmd/mfer/main.go +++ b/cmd/mfer/main.go @@ -1,3 +1,4 @@ +// Command mfer generates and verifies file manifests. package main import ( @@ -6,8 +7,13 @@ import ( "sneak.berlin/go/mfer/internal/cli" ) +// Appname is the name of this program. +const Appname = "mfer" + +// Version and Gitrev are injected at build time via -ldflags. +// +//nolint:gochecknoglobals // set via ldflags at build time var ( - Appname string = "mfer" Version string Gitrev string ) diff --git a/cmd/mfer/main_test.go b/cmd/mfer/main_test.go index 907d1af..c5d3d99 100644 --- a/cmd/mfer/main_test.go +++ b/cmd/mfer/main_test.go @@ -6,6 +6,20 @@ import ( "github.com/stretchr/testify/assert" ) -func TestBuild(t *testing.T) { - assert.True(t, true) +// TestAppname pins the program name that main passes to cli.Run; it is +// the name that appears in usage output and in the log prefix. It also +// keeps this package compiled under `go test`. +func TestAppname(t *testing.T) { + t.Parallel() + + assert.Equal(t, "mfer", Appname) +} + +// TestVersionDefaults documents that Version and Gitrev are empty unless +// injected at build time via -ldflags. +func TestVersionDefaults(t *testing.T) { + t.Parallel() + + assert.Empty(t, Version) + assert.Empty(t, Gitrev) } diff --git a/internal/bork/error.go b/internal/bork/error.go index 6ead94d..9fad746 100644 --- a/internal/bork/error.go +++ b/internal/bork/error.go @@ -1,15 +1,14 @@ +// Package bork defines the sentinel errors used by the manifest +// reader and writer. package bork import ( "errors" - "fmt" ) var ( - ErrMissingMagic = errors.New("missing magic bytes in file") + // ErrMissingMagic indicates the input lacks the manifest magic bytes. + ErrMissingMagic = errors.New("missing magic bytes in file") + // ErrFileTruncated indicates the input ended before the expected length. ErrFileTruncated = errors.New("file/stream is truncated abnormally") ) - -func Newf(format string, args ...interface{}) error { - return fmt.Errorf(format, args...) -} diff --git a/internal/bork/error_test.go b/internal/bork/error_test.go index f98318f..480fc43 100644 --- a/internal/bork/error_test.go +++ b/internal/bork/error_test.go @@ -1,11 +1,14 @@ -package bork +package bork_test import ( "testing" "github.com/stretchr/testify/assert" + + "sneak.berlin/go/mfer/internal/bork" ) func TestBuild(t *testing.T) { - assert.NotNil(t, ErrMissingMagic) + t.Parallel() + assert.Error(t, bork.ErrMissingMagic) } diff --git a/internal/cli/check.go b/internal/cli/check.go index 7327474..459465e 100644 --- a/internal/cli/check.go +++ b/internal/cli/check.go @@ -1,10 +1,14 @@ +// Package cli implements the mfer command-line interface. package cli import ( "encoding/hex" + "errors" "fmt" "io" + "math" "path/filepath" + "strconv" "strings" "time" @@ -15,21 +19,254 @@ import ( "sneak.berlin/go/mfer/mfer" ) +// fingerprintHexLen is the length of a full GPG key fingerprint in hex +// characters. +const fingerprintHexLen = 40 + +var ( + // errNoManifestFound indicates no manifest file was found in the + // searched directory. + errNoManifestFound = errors.New("no manifest found") + // errInvalidFingerprint indicates a malformed --require-signature + // fingerprint argument. The length is spliced in from + // fingerprintHexLen so the two cannot drift apart. + errInvalidFingerprint = errors.New( + "invalid fingerprint: must be exactly " + + strconv.Itoa(fingerprintHexLen) + " hex characters") + // errManifestNotSigned indicates a signature was required but the + // manifest is unsigned. It is wrapped mid-sentence so that the + // rendered message stays exactly as mfer has always printed it. + errManifestNotSigned = errors.New("manifest is not signed") + // errSignerMismatch indicates the embedded signing key fingerprint + // does not match the required signer. Its text is the mid-sentence + // fragment of the rendered message, which users grep for in CI and + // which must therefore not change; match it with errors.Is rather + // than by reading it. + errSignerMismatch = errors.New("does not match required") +) + +// safeUint64 converts a non-negative int64 to uint64, clamping negative +// values to zero. +func safeUint64(n int64) uint64 { + if n < 0 { + return 0 + } + + return uint64(n) +} + +// safeRateUint64 converts a bytes-per-second rate to uint64 for display. +// +// A rate is computed as bytes/elapsed, so it is +Inf when the elapsed +// time rounds to zero and NaN when zero bytes were processed in zero +// time. Neither has a defined conversion to uint64, and on amd64 +Inf +// converts to a number that renders as "8.0 EiB/s"; both display as zero +// instead. +func safeRateUint64(rate float64) uint64 { + if math.IsNaN(rate) || math.IsInf(rate, 0) || rate <= 0 { + return 0 + } + + if rate >= math.MaxUint64 { + return math.MaxUint64 + } + + return uint64(rate) +} + // findManifest looks for a manifest file in the given directory. // It checks for index.mf and .index.mf, returning the first one found. func findManifest(fs afero.Fs, dir string) (string, error) { candidates := []string{"index.mf", ".index.mf"} for _, name := range candidates { path := filepath.Join(dir, name) + exists, err := afero.Exists(fs, path) if err != nil { return "", err } + if exists { return path, nil } } - return "", fmt.Errorf("no manifest found in %s (looked for index.mf and .index.mf)", dir) + + return "", fmt.Errorf( + "%w in %s (looked for index.mf and .index.mf)", errNoManifestFound, dir) +} + +// fetchManifestToTemp downloads a manifest URL to a temporary file and +// returns the temp file path. The caller is responsible for removing it. +func (mfa *CLIApp) fetchManifestToTemp(url string) (string, error) { + rc, fetchErr := mfa.openManifestReader(url) + if fetchErr != nil { + return "", fetchErr + } + + tmpFile, tmpErr := afero.TempFile(mfa.Fs, "", "mfer-manifest-*.mf") + if tmpErr != nil { + _ = rc.Close() + + return "", fmt.Errorf("failed to create temp file: %w", tmpErr) + } + + tmpPath := tmpFile.Name() + _, cpErr := io.Copy(tmpFile, rc) + _ = rc.Close() + _ = tmpFile.Close() + + if cpErr != nil { + _ = mfa.Fs.Remove(tmpPath) + + return "", fmt.Errorf("failed to download manifest: %w", cpErr) + } + + return tmpPath, nil +} + +// verifyRequiredSigner enforces the --require-signature fingerprint +// against the manifest's embedded signing key. +func verifyRequiredSigner(chk *mfer.Checker, requiredSigner string) error { + // Validate fingerprint format: must be exactly 40 hex characters + if len(requiredSigner) != fingerprintHexLen { + return fmt.Errorf("%w, got %d", errInvalidFingerprint, len(requiredSigner)) + } + + _, err := hex.DecodeString(requiredSigner) + if err != nil { + return fmt.Errorf("invalid fingerprint: must be valid hex: %w", err) + } + + if !chk.IsSigned() { + return fmt.Errorf("%w, but signature from %s is required", + errManifestNotSigned, requiredSigner) + } + + // Extract fingerprint from the embedded public key (not from the + // signer field). This validates the key is importable and gets its + // actual fingerprint. + embeddedFP, err := chk.ExtractEmbeddedSigningKeyFP() + if err != nil { + return fmt.Errorf( + "failed to extract fingerprint from embedded signing key: %w", err) + } + + // Compare fingerprints - must be exact match (case-insensitive) + if !strings.EqualFold(embeddedFP, requiredSigner) { + return fmt.Errorf("embedded signing key fingerprint %s %w %s", + embeddedFP, errSignerMismatch, requiredSigner) + } + + log.Infof("manifest signature verified (signer: %s)", embeddedFP) + + return nil +} + +// reportCheckProgress renders progress updates until the channel closes. +func reportCheckProgress(progress <-chan mfer.CheckStatus) { + for status := range progress { + if status.ETA > 0 { + log.Progressf("Checking: %d/%d files, %s/s, ETA %s, %d failures", + status.CheckedFiles, + status.TotalFiles, + humanize.IBytes(safeRateUint64(status.BytesPerSec)), + status.ETA.Round(time.Second), + status.Failures) + } else { + log.Progressf("Checking: %d/%d files, %s/s, %d failures", + status.CheckedFiles, + status.TotalFiles, + humanize.IBytes(safeRateUint64(status.BytesPerSec)), + status.Failures) + } + } + + log.ProgressDone() +} + +// countCheckFailures consumes check results, counting and logging +// failures, then closes done. +func countCheckFailures( + results <-chan mfer.Result, failures *int64, done chan<- struct{}, +) { + for result := range results { + if result.Status != mfer.StatusOK { + *failures++ + + log.Infof("%s: %s (%s)", result.Status, result.Path, result.Message) + } else { + log.Verbosef("%s: %s", result.Status, result.Path) + } + } + + close(done) +} + +// findExtraFiles reports files present on disk but absent from the +// manifest, counting each as a failure. +func findExtraFiles(ctx *cli.Context, chk *mfer.Checker, failures *int64) error { + extraResults := make(chan mfer.Result, 1) + extraDone := make(chan struct{}) + + go func() { + for result := range extraResults { + *failures++ + + log.Infof("%s: %s (%s)", result.Status, result.Path, result.Message) + } + + close(extraDone) + }() + + err := chk.FindExtraFiles(ctx.Context, extraResults) + if err != nil { + return fmt.Errorf("failed to check for extra files: %w", err) + } + + <-extraDone + + return nil +} + +// runCheck runs the manifest check with progress and result reporting +// and returns the number of failures. +func runCheck(ctx *cli.Context, chk *mfer.Checker, showProgress bool) (int64, error) { + // Set up results channel + results := make(chan mfer.Result, 1) + + // Set up progress channel + var progress chan mfer.CheckStatus + if showProgress { + progress = make(chan mfer.CheckStatus, 1) + + go reportCheckProgress(progress) + } + + // Process results in a goroutine + var failures int64 + + done := make(chan struct{}) + + go countCheckFailures(results, &failures, done) + + // Run check + err := chk.Check(ctx.Context, results, progress) + if err != nil { + return 0, fmt.Errorf("check failed: %w", err) + } + + // Wait for results processing to complete + <-done + + // Check for extra files if requested + if ctx.Bool("no-extra-files") { + err = findExtraFiles(ctx, chk, &failures) + if err != nil { + return 0, err + } + } + + return failures, nil } func (mfa *CLIApp) checkManifestOperation(ctx *cli.Context) error { @@ -42,24 +279,13 @@ func (mfa *CLIApp) checkManifestOperation(ctx *cli.Context) error { // URL manifests need to be downloaded to a temp file for the checker if isHTTPURL(manifestPath) { - rc, fetchErr := mfa.openManifestReader(manifestPath) - if fetchErr != nil { - return fmt.Errorf("check: %w", fetchErr) - } - tmpFile, tmpErr := afero.TempFile(mfa.Fs, "", "mfer-manifest-*.mf") + tmpPath, tmpErr := mfa.fetchManifestToTemp(manifestPath) if tmpErr != nil { - _ = rc.Close() - return fmt.Errorf("check: failed to create temp file: %w", tmpErr) - } - tmpPath := tmpFile.Name() - _, cpErr := io.Copy(tmpFile, rc) - _ = rc.Close() - _ = tmpFile.Close() - if cpErr != nil { - _ = mfa.Fs.Remove(tmpPath) - return fmt.Errorf("check: failed to download manifest: %w", cpErr) + return fmt.Errorf("check: %w", tmpErr) } + defer func() { _ = mfa.Fs.Remove(tmpPath) }() + manifestPath = tmpPath } @@ -77,111 +303,31 @@ func (mfa *CLIApp) checkManifestOperation(ctx *cli.Context) error { // Check signature requirement requiredSigner := ctx.String("require-signature") if requiredSigner != "" { - // Validate fingerprint format: must be exactly 40 hex characters - if len(requiredSigner) != 40 { - return fmt.Errorf("invalid fingerprint: must be exactly 40 hex characters, got %d", len(requiredSigner)) - } - if _, err := hex.DecodeString(requiredSigner); err != nil { - return fmt.Errorf("invalid fingerprint: must be valid hex: %w", err) - } - - if !chk.IsSigned() { - return fmt.Errorf("manifest is not signed, but signature from %s is required", requiredSigner) - } - - // Extract fingerprint from the embedded public key (not from the signer field) - // This validates the key is importable and gets its actual fingerprint - embeddedFP, err := chk.ExtractEmbeddedSigningKeyFP() + err = verifyRequiredSigner(chk, requiredSigner) if err != nil { - return fmt.Errorf("failed to extract fingerprint from embedded signing key: %w", err) + return err } - - // Compare fingerprints - must be exact match (case-insensitive) - if !strings.EqualFold(embeddedFP, requiredSigner) { - return fmt.Errorf("embedded signing key fingerprint %s does not match required %s", embeddedFP, requiredSigner) - } - log.Infof("manifest signature verified (signer: %s)", embeddedFP) } - log.Infof("manifest contains %d files, %s", chk.FileCount(), humanize.IBytes(uint64(chk.TotalBytes()))) + log.Infof("manifest contains %d files, %s", chk.FileCount(), + humanize.IBytes(safeUint64(int64(chk.TotalBytes())))) - // Set up results channel - results := make(chan mfer.Result, 1) - - // Set up progress channel - var progress chan mfer.CheckStatus - if showProgress { - progress = make(chan mfer.CheckStatus, 1) - go func() { - for status := range progress { - if status.ETA > 0 { - log.Progressf("Checking: %d/%d files, %s/s, ETA %s, %d failures", - status.CheckedFiles, - status.TotalFiles, - humanize.IBytes(uint64(status.BytesPerSec)), - status.ETA.Round(time.Second), - status.Failures) - } else { - log.Progressf("Checking: %d/%d files, %s/s, %d failures", - status.CheckedFiles, - status.TotalFiles, - humanize.IBytes(uint64(status.BytesPerSec)), - status.Failures) - } - } - log.ProgressDone() - }() - } - - // Process results in a goroutine - var failures int64 - done := make(chan struct{}) - go func() { - for result := range results { - if result.Status != mfer.StatusOK { - failures++ - log.Infof("%s: %s (%s)", result.Status, result.Path, result.Message) - } else { - log.Verbosef("%s: %s", result.Status, result.Path) - } - } - close(done) - }() - - // Run check - err = chk.Check(ctx.Context, results, progress) + failures, err := runCheck(ctx, chk, showProgress) if err != nil { - return fmt.Errorf("check failed: %w", err) - } - - // Wait for results processing to complete - <-done - - // Check for extra files if requested - if ctx.Bool("no-extra-files") { - extraResults := make(chan mfer.Result, 1) - extraDone := make(chan struct{}) - go func() { - for result := range extraResults { - failures++ - log.Infof("%s: %s (%s)", result.Status, result.Path, result.Message) - } - close(extraDone) - }() - - err = chk.FindExtraFiles(ctx.Context, extraResults) - if err != nil { - return fmt.Errorf("failed to check for extra files: %w", err) - } - <-extraDone + return err } elapsed := time.Since(mfa.startupTime).Seconds() + rate := float64(chk.TotalBytes()) / elapsed if failures == 0 { - log.Infof("checked %d files (%s) in %.1fs (%s/s): all OK", chk.FileCount(), humanize.IBytes(uint64(chk.TotalBytes())), elapsed, humanize.IBytes(uint64(rate))) + log.Infof("checked %d files (%s) in %.1fs (%s/s): all OK", + chk.FileCount(), humanize.IBytes(safeUint64(int64(chk.TotalBytes()))), + elapsed, humanize.IBytes(safeRateUint64(rate))) } else { - log.Infof("checked %d files (%s) in %.1fs (%s/s): %d failed", chk.FileCount(), humanize.IBytes(uint64(chk.TotalBytes())), elapsed, humanize.IBytes(uint64(rate)), failures) + log.Infof("checked %d files (%s) in %.1fs (%s/s): %d failed", + chk.FileCount(), humanize.IBytes(safeUint64(int64(chk.TotalBytes()))), + elapsed, humanize.IBytes(safeRateUint64(rate)), failures) } if failures > 0 { diff --git a/internal/cli/entry.go b/internal/cli/entry.go index 32ace65..3bdd6eb 100644 --- a/internal/cli/entry.go +++ b/internal/cli/entry.go @@ -7,15 +7,18 @@ import ( "github.com/spf13/afero" ) -// NO_COLOR disables colored output when set. Automatically true if the +// NoColor disables colored output when set. Automatically true if the // NO_COLOR environment variable is present (per https://no-color.org/). -var NO_COLOR bool +// +//nolint:gochecknoglobals // process-wide setting derived from the environment +var NoColor = noColorEnvSet() -func init() { - NO_COLOR = false - if _, exists := os.LookupEnv("NO_COLOR"); exists { - NO_COLOR = true - } +// noColorEnvSet reports whether the NO_COLOR environment variable is +// present. +func noColorEnvSet() bool { + _, exists := os.LookupEnv("NO_COLOR") + + return exists } // RunOptions contains all configuration for running the CLI application. @@ -64,5 +67,6 @@ func RunWithOptions(opts *RunOptions) int { } m.run(opts.Args) + return m.exitCode } diff --git a/internal/cli/entry_test.go b/internal/cli/entry_test.go index 8ece599..61868ac 100644 --- a/internal/cli/entry_test.go +++ b/internal/cli/entry_test.go @@ -1,9 +1,13 @@ +//nolint:testpackage // white-box tests exercise unexported internals package cli import ( "bytes" + "errors" "fmt" "math/rand" + "os" + "sync" "testing" "github.com/spf13/afero" @@ -13,19 +17,53 @@ import ( "sneak.berlin/go/mfer/mfer" ) -func init() { +const ( + testApp = "mfer" + testDir = "/testdir" + testFile1 = "/testdir/file1.txt" + testMF = "/testdir/test.mf" + testOutput = "/output.mf" + testOutputTmp = "/output.mf.tmp" + testManifest = "/manifest.mf" + testFlagBase = "--base" + testFlagNoExtra = "--no-extra-files" +) + +var errSimulatedWrite = errors.New("simulated write failure") + +// runMu serializes CLI runs: RunWithOptions wires the process-global +// logger to the run's I/O streams, so parallel runs would cross-wire +// captured output between tests. +// +//nolint:gochecknoglobals // guards process-global logger state in tests +var runMu sync.Mutex + +// runCLI invokes RunWithOptions while holding runMu so parallel tests +// capture their own output. +func runCLI(opts *RunOptions) int { + runMu.Lock() + defer runMu.Unlock() + + return RunWithOptions(opts) +} + +func TestMain(m *testing.M) { // Prevent urfave/cli from calling os.Exit during tests - urfcli.OsExiter = func(code int) {} + urfcli.OsExiter = func(_ int) {} + + os.Exit(m.Run()) } func TestBuild(t *testing.T) { + t.Parallel() + m := &CLIApp{} assert.NotNil(t, m) } func testOpts(args []string, fs afero.Fs) *RunOptions { return &RunOptions{ - Appname: "mfer", + Appname: testApp, Version: "1.0.0", Gitrev: "abc123", Args: args, @@ -36,374 +74,451 @@ func testOpts(args []string, fs afero.Fs) *RunOptions { } } -func TestVersionCommand(t *testing.T) { - fs := afero.NewMemMapFs() - opts := testOpts([]string{"mfer", "version"}, fs) +func testStdout(t *testing.T, opts *RunOptions) string { + t.Helper() - exitCode := RunWithOptions(opts) + buf, ok := opts.Stdout.(*bytes.Buffer) + require.True(t, ok) + + return buf.String() +} + +func testStderr(t *testing.T, opts *RunOptions) string { + t.Helper() + + buf, ok := opts.Stderr.(*bytes.Buffer) + require.True(t, ok) + + return buf.String() +} + +func writeTestFile(t *testing.T, fs afero.Fs, path, content string) { + t.Helper() + + require.NoError(t, afero.WriteFile(fs, path, []byte(content), 0o644)) +} + +func TestVersionCommand(t *testing.T) { + t.Parallel() + + fs := afero.NewMemMapFs() + opts := testOpts([]string{testApp, "version"}, fs) + + exitCode := runCLI(opts) assert.Equal(t, 0, exitCode) - stdout := opts.Stdout.(*bytes.Buffer).String() + + stdout := testStdout(t, opts) assert.Contains(t, stdout, mfer.Version) assert.Contains(t, stdout, "abc123") } func TestHelpCommand(t *testing.T) { - fs := afero.NewMemMapFs() - opts := testOpts([]string{"mfer", "--help"}, fs) + t.Parallel() - exitCode := RunWithOptions(opts) + fs := afero.NewMemMapFs() + opts := testOpts([]string{testApp, "--help"}, fs) + + exitCode := runCLI(opts) assert.Equal(t, 0, exitCode) - stdout := opts.Stdout.(*bytes.Buffer).String() - assert.Contains(t, stdout, "generate") - assert.Contains(t, stdout, "check") + + stdout := testStdout(t, opts) + assert.Contains(t, stdout, cmdGenerate) + assert.Contains(t, stdout, cmdCheck) assert.Contains(t, stdout, "fetch") } func TestGenerateCommand(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() // Create test files in memory filesystem - require.NoError(t, fs.MkdirAll("/testdir", 0o755)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("hello world"), 0o644)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file2.txt", []byte("test content"), 0o644)) + require.NoError(t, fs.MkdirAll(testDir, 0o755)) + writeTestFile(t, fs, testFile1, "hello world") + writeTestFile(t, fs, "/testdir/file2.txt", "test content") - opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/testdir/test.mf", "/testdir"}, fs) + opts := testOpts([]string{testApp, cmdGenerate, "-q", "-o", testMF, testDir}, fs) - exitCode := RunWithOptions(opts) + exitCode := runCLI(opts) - assert.Equal(t, 0, exitCode, "stderr: %s", opts.Stderr.(*bytes.Buffer).String()) + assert.Equal(t, 0, exitCode, "stderr: %s", testStderr(t, opts)) // Verify manifest was created - exists, err := afero.Exists(fs, "/testdir/test.mf") + exists, err := afero.Exists(fs, testMF) require.NoError(t, err) assert.True(t, exists) } func TestGenerateAndCheckCommand(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() // Create test files with subdirectory require.NoError(t, fs.MkdirAll("/testdir/subdir", 0o755)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("hello world"), 0o644)) - require.NoError(t, afero.WriteFile(fs, "/testdir/subdir/file2.txt", []byte("test content"), 0o644)) + writeTestFile(t, fs, testFile1, "hello world") + writeTestFile(t, fs, "/testdir/subdir/file2.txt", "test content") // Generate manifest - opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/testdir/test.mf", "/testdir"}, fs) - exitCode := RunWithOptions(opts) - require.Equal(t, 0, exitCode, "generate failed: %s", opts.Stderr.(*bytes.Buffer).String()) + opts := testOpts([]string{testApp, cmdGenerate, "-q", "-o", testMF, testDir}, fs) + exitCode := runCLI(opts) + require.Equal(t, 0, exitCode, "generate failed: %s", testStderr(t, opts)) // Check manifest - opts = testOpts([]string{"mfer", "check", "-q", "--base", "/testdir", "/testdir/test.mf"}, fs) - exitCode = RunWithOptions(opts) - assert.Equal(t, 0, exitCode, "check failed: %s", opts.Stderr.(*bytes.Buffer).String()) + opts = testOpts([]string{testApp, cmdCheck, "-q", testFlagBase, testDir, testMF}, fs) + exitCode = runCLI(opts) + assert.Equal(t, 0, exitCode, "check failed: %s", testStderr(t, opts)) } func TestCheckCommandWithMissingFile(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() // Create test file - require.NoError(t, fs.MkdirAll("/testdir", 0o755)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("hello world"), 0o644)) + require.NoError(t, fs.MkdirAll(testDir, 0o755)) + writeTestFile(t, fs, testFile1, "hello world") // Generate manifest - opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/testdir/test.mf", "/testdir"}, fs) - exitCode := RunWithOptions(opts) - require.Equal(t, 0, exitCode, "generate failed: %s", opts.Stderr.(*bytes.Buffer).String()) + opts := testOpts([]string{testApp, cmdGenerate, "-q", "-o", testMF, testDir}, fs) + exitCode := runCLI(opts) + require.Equal(t, 0, exitCode, "generate failed: %s", testStderr(t, opts)) // Delete the file - require.NoError(t, fs.Remove("/testdir/file1.txt")) + require.NoError(t, fs.Remove(testFile1)) // Check manifest - should fail - opts = testOpts([]string{"mfer", "check", "-q", "--base", "/testdir", "/testdir/test.mf"}, fs) - exitCode = RunWithOptions(opts) + opts = testOpts([]string{testApp, cmdCheck, "-q", testFlagBase, testDir, testMF}, fs) + exitCode = runCLI(opts) assert.Equal(t, 1, exitCode, "check should have failed for missing file") } -func TestCheckCommandWithCorruptedFile(t *testing.T) { +func runCheckAfterRewrite(t *testing.T, rewritten, msg string) { + t.Helper() + fs := afero.NewMemMapFs() // Create test file - require.NoError(t, fs.MkdirAll("/testdir", 0o755)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("hello world"), 0o644)) + require.NoError(t, fs.MkdirAll(testDir, 0o755)) + writeTestFile(t, fs, testFile1, "hello world") // Generate manifest - opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/testdir/test.mf", "/testdir"}, fs) - exitCode := RunWithOptions(opts) - require.Equal(t, 0, exitCode, "generate failed: %s", opts.Stderr.(*bytes.Buffer).String()) + opts := testOpts([]string{testApp, cmdGenerate, "-q", "-o", testMF, testDir}, fs) + exitCode := runCLI(opts) + require.Equal(t, 0, exitCode, "generate failed: %s", testStderr(t, opts)) + + // Rewrite the file, then check the manifest - it must fail + writeTestFile(t, fs, testFile1, rewritten) + + opts = testOpts([]string{testApp, cmdCheck, "-q", testFlagBase, testDir, testMF}, fs) + exitCode = runCLI(opts) + assert.Equal(t, 1, exitCode, msg) +} + +func TestCheckCommandWithCorruptedFile(t *testing.T) { + t.Parallel() // Corrupt the file (change content but keep same size) - require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("HELLO WORLD"), 0o644)) - - // Check manifest - should fail with hash mismatch - opts = testOpts([]string{"mfer", "check", "-q", "--base", "/testdir", "/testdir/test.mf"}, fs) - exitCode = RunWithOptions(opts) - assert.Equal(t, 1, exitCode, "check should have failed for corrupted file") + runCheckAfterRewrite(t, "HELLO WORLD", + "check should have failed for corrupted file") } func TestCheckCommandWithSizeMismatch(t *testing.T) { - fs := afero.NewMemMapFs() - - // Create test file - require.NoError(t, fs.MkdirAll("/testdir", 0o755)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("hello world"), 0o644)) - - // Generate manifest - opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/testdir/test.mf", "/testdir"}, fs) - exitCode := RunWithOptions(opts) - require.Equal(t, 0, exitCode, "generate failed: %s", opts.Stderr.(*bytes.Buffer).String()) + t.Parallel() // Change file size - require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("different size content here"), 0o644)) - - // Check manifest - should fail with size mismatch - opts = testOpts([]string{"mfer", "check", "-q", "--base", "/testdir", "/testdir/test.mf"}, fs) - exitCode = RunWithOptions(opts) - assert.Equal(t, 1, exitCode, "check should have failed for size mismatch") + runCheckAfterRewrite(t, "different size content here", + "check should have failed for size mismatch") } func TestBannerOutput(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() // Create test file - require.NoError(t, fs.MkdirAll("/testdir", 0o755)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("hello"), 0o644)) + require.NoError(t, fs.MkdirAll(testDir, 0o755)) + writeTestFile(t, fs, testFile1, "hello") // Run without -q to see banner - opts := testOpts([]string{"mfer", "generate", "-o", "/testdir/test.mf", "/testdir"}, fs) - exitCode := RunWithOptions(opts) + opts := testOpts([]string{testApp, cmdGenerate, "-o", testMF, testDir}, fs) + exitCode := runCLI(opts) assert.Equal(t, 0, exitCode) // Banner ASCII art should be in stdout - stdout := opts.Stdout.(*bytes.Buffer).String() + stdout := testStdout(t, opts) assert.Contains(t, stdout, "___") assert.Contains(t, stdout, "\\") } func TestUnknownCommand(t *testing.T) { - fs := afero.NewMemMapFs() - opts := testOpts([]string{"mfer", "unknown"}, fs) + t.Parallel() - exitCode := RunWithOptions(opts) + fs := afero.NewMemMapFs() + opts := testOpts([]string{testApp, "unknown"}, fs) + + exitCode := runCLI(opts) assert.Equal(t, 1, exitCode) } func TestGenerateExcludesDotfilesByDefault(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() // Create test files including dotfiles - require.NoError(t, fs.MkdirAll("/testdir", 0o755)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("hello"), 0o644)) - require.NoError(t, afero.WriteFile(fs, "/testdir/.hidden", []byte("secret"), 0o644)) + require.NoError(t, fs.MkdirAll(testDir, 0o755)) + writeTestFile(t, fs, testFile1, "hello") + writeTestFile(t, fs, "/testdir/.hidden", "secret") // Generate manifest without --include-dotfiles (default excludes dotfiles) - opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/testdir/test.mf", "/testdir"}, fs) - exitCode := RunWithOptions(opts) + opts := testOpts([]string{testApp, cmdGenerate, "-q", "-o", testMF, testDir}, fs) + exitCode := runCLI(opts) require.Equal(t, 0, exitCode) // Check that manifest exists - exists, _ := afero.Exists(fs, "/testdir/test.mf") + exists, _ := afero.Exists(fs, testMF) assert.True(t, exists) // Verify manifest only has 1 file (the non-dotfile) - manifest, err := mfer.NewManifestFromFile(fs, "/testdir/test.mf") + manifest, err := mfer.NewManifestFromFile(fs, testMF) require.NoError(t, err) assert.Len(t, manifest.Files(), 1) - assert.Equal(t, "file1.txt", manifest.Files()[0].Path) + assert.Equal(t, "file1.txt", manifest.Files()[0].GetPath()) } func TestGenerateWithIncludeDotfiles(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() // Create test files including dotfiles - require.NoError(t, fs.MkdirAll("/testdir", 0o755)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("hello"), 0o644)) - require.NoError(t, afero.WriteFile(fs, "/testdir/.hidden", []byte("secret"), 0o644)) + require.NoError(t, fs.MkdirAll(testDir, 0o755)) + writeTestFile(t, fs, testFile1, "hello") + writeTestFile(t, fs, "/testdir/.hidden", "secret") // Generate manifest with --include-dotfiles - opts := testOpts([]string{"mfer", "generate", "-q", "--include-dotfiles", "-o", "/testdir/test.mf", "/testdir"}, fs) - exitCode := RunWithOptions(opts) + opts := testOpts([]string{ + testApp, cmdGenerate, "-q", "--include-dotfiles", "-o", testMF, testDir, + }, fs) + exitCode := runCLI(opts) require.Equal(t, 0, exitCode) // Verify manifest has 2 files (including dotfile) - manifest, err := mfer.NewManifestFromFile(fs, "/testdir/test.mf") + manifest, err := mfer.NewManifestFromFile(fs, testMF) require.NoError(t, err) assert.Len(t, manifest.Files(), 2) } func TestMultipleInputPaths(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() // Create test files in multiple directories require.NoError(t, fs.MkdirAll("/dir1", 0o755)) require.NoError(t, fs.MkdirAll("/dir2", 0o755)) - require.NoError(t, afero.WriteFile(fs, "/dir1/file1.txt", []byte("content1"), 0o644)) - require.NoError(t, afero.WriteFile(fs, "/dir2/file2.txt", []byte("content2"), 0o644)) + writeTestFile(t, fs, "/dir1/file1.txt", "content1") + writeTestFile(t, fs, "/dir2/file2.txt", "content2") // Generate manifest from multiple paths - opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/output.mf", "/dir1", "/dir2"}, fs) - exitCode := RunWithOptions(opts) - assert.Equal(t, 0, exitCode, "stderr: %s", opts.Stderr.(*bytes.Buffer).String()) + opts := testOpts([]string{ + testApp, cmdGenerate, "-q", "-o", testOutput, "/dir1", "/dir2", + }, fs) + exitCode := runCLI(opts) + assert.Equal(t, 0, exitCode, "stderr: %s", testStderr(t, opts)) - exists, _ := afero.Exists(fs, "/output.mf") + exists, _ := afero.Exists(fs, testOutput) assert.True(t, exists) } func TestNoExtraFilesPass(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() // Create test files - require.NoError(t, fs.MkdirAll("/testdir", 0o755)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("hello"), 0o644)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file2.txt", []byte("world"), 0o644)) + require.NoError(t, fs.MkdirAll(testDir, 0o755)) + writeTestFile(t, fs, testFile1, "hello") + writeTestFile(t, fs, "/testdir/file2.txt", "world") // Generate manifest - opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/manifest.mf", "/testdir"}, fs) - exitCode := RunWithOptions(opts) + opts := testOpts([]string{testApp, cmdGenerate, "-q", "-o", testManifest, testDir}, fs) + exitCode := runCLI(opts) require.Equal(t, 0, exitCode) // Check with --no-extra-files (should pass - no extra files) - opts = testOpts([]string{"mfer", "check", "-q", "--no-extra-files", "--base", "/testdir", "/manifest.mf"}, fs) - exitCode = RunWithOptions(opts) + opts = testOpts([]string{ + testApp, cmdCheck, "-q", testFlagNoExtra, testFlagBase, testDir, testManifest, + }, fs) + exitCode = runCLI(opts) assert.Equal(t, 0, exitCode) } func TestNoExtraFilesFail(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() // Create test files - require.NoError(t, fs.MkdirAll("/testdir", 0o755)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("hello"), 0o644)) + require.NoError(t, fs.MkdirAll(testDir, 0o755)) + writeTestFile(t, fs, testFile1, "hello") // Generate manifest - opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/manifest.mf", "/testdir"}, fs) - exitCode := RunWithOptions(opts) + opts := testOpts([]string{testApp, cmdGenerate, "-q", "-o", testManifest, testDir}, fs) + exitCode := runCLI(opts) require.Equal(t, 0, exitCode) // Add an extra file after manifest generation - require.NoError(t, afero.WriteFile(fs, "/testdir/extra.txt", []byte("extra"), 0o644)) + writeTestFile(t, fs, "/testdir/extra.txt", "extra") // Check with --no-extra-files (should fail - extra file exists) - opts = testOpts([]string{"mfer", "check", "-q", "--no-extra-files", "--base", "/testdir", "/manifest.mf"}, fs) - exitCode = RunWithOptions(opts) + opts = testOpts([]string{ + testApp, cmdCheck, "-q", testFlagNoExtra, testFlagBase, testDir, testManifest, + }, fs) + exitCode = runCLI(opts) assert.Equal(t, 1, exitCode, "check should fail when extra files exist") } func TestNoExtraFilesWithSubdirectory(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() // Create test files with subdirectory require.NoError(t, fs.MkdirAll("/testdir/subdir", 0o755)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("hello"), 0o644)) - require.NoError(t, afero.WriteFile(fs, "/testdir/subdir/file2.txt", []byte("world"), 0o644)) + writeTestFile(t, fs, testFile1, "hello") + writeTestFile(t, fs, "/testdir/subdir/file2.txt", "world") // Generate manifest - opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/manifest.mf", "/testdir"}, fs) - exitCode := RunWithOptions(opts) + opts := testOpts([]string{testApp, cmdGenerate, "-q", "-o", testManifest, testDir}, fs) + exitCode := runCLI(opts) require.Equal(t, 0, exitCode) // Add extra file in subdirectory - require.NoError(t, afero.WriteFile(fs, "/testdir/subdir/extra.txt", []byte("extra"), 0o644)) + writeTestFile(t, fs, "/testdir/subdir/extra.txt", "extra") // Check with --no-extra-files (should fail) - opts = testOpts([]string{"mfer", "check", "-q", "--no-extra-files", "--base", "/testdir", "/manifest.mf"}, fs) - exitCode = RunWithOptions(opts) - assert.Equal(t, 1, exitCode, "check should fail when extra files exist in subdirectory") + opts = testOpts([]string{ + testApp, cmdCheck, "-q", testFlagNoExtra, testFlagBase, testDir, testManifest, + }, fs) + exitCode = runCLI(opts) + assert.Equal(t, 1, exitCode, + "check should fail when extra files exist in subdirectory") } func TestCheckWithoutNoExtraFilesIgnoresExtra(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() // Create test file - require.NoError(t, fs.MkdirAll("/testdir", 0o755)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("hello"), 0o644)) + require.NoError(t, fs.MkdirAll(testDir, 0o755)) + writeTestFile(t, fs, testFile1, "hello") // Generate manifest - opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/manifest.mf", "/testdir"}, fs) - exitCode := RunWithOptions(opts) + opts := testOpts([]string{testApp, cmdGenerate, "-q", "-o", testManifest, testDir}, fs) + exitCode := runCLI(opts) require.Equal(t, 0, exitCode) // Add extra file - require.NoError(t, afero.WriteFile(fs, "/testdir/extra.txt", []byte("extra"), 0o644)) + writeTestFile(t, fs, "/testdir/extra.txt", "extra") // Check WITHOUT --no-extra-files (should pass - extra files ignored) - opts = testOpts([]string{"mfer", "check", "-q", "--base", "/testdir", "/manifest.mf"}, fs) - exitCode = RunWithOptions(opts) - assert.Equal(t, 0, exitCode, "check without --no-extra-files should ignore extra files") + opts = testOpts([]string{ + testApp, cmdCheck, "-q", testFlagBase, testDir, testManifest, + }, fs) + exitCode = runCLI(opts) + assert.Equal(t, 0, exitCode, + "check without --no-extra-files should ignore extra files") } func TestGenerateAtomicWriteNoTempFileOnSuccess(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() // Create test file - require.NoError(t, fs.MkdirAll("/testdir", 0o755)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("hello"), 0o644)) + require.NoError(t, fs.MkdirAll(testDir, 0o755)) + writeTestFile(t, fs, testFile1, "hello") // Generate manifest - opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/output.mf", "/testdir"}, fs) - exitCode := RunWithOptions(opts) + opts := testOpts([]string{testApp, cmdGenerate, "-q", "-o", testOutput, testDir}, fs) + exitCode := runCLI(opts) require.Equal(t, 0, exitCode) // Verify output file exists - exists, err := afero.Exists(fs, "/output.mf") + exists, err := afero.Exists(fs, testOutput) require.NoError(t, err) assert.True(t, exists, "output file should exist") // Verify temp file does NOT exist - tmpExists, err := afero.Exists(fs, "/output.mf.tmp") + tmpExists, err := afero.Exists(fs, testOutputTmp) require.NoError(t, err) - assert.False(t, tmpExists, "temp file should not exist after successful generation") + assert.False(t, tmpExists, + "temp file should not exist after successful generation") } func TestGenerateAtomicWriteOverwriteWithForce(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() // Create test file - require.NoError(t, fs.MkdirAll("/testdir", 0o755)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("hello"), 0o644)) + require.NoError(t, fs.MkdirAll(testDir, 0o755)) + writeTestFile(t, fs, testFile1, "hello") // Create existing manifest with different content - require.NoError(t, afero.WriteFile(fs, "/output.mf", []byte("old content"), 0o644)) + writeTestFile(t, fs, testOutput, "old content") // Generate manifest with --force - opts := testOpts([]string{"mfer", "generate", "-q", "-f", "-o", "/output.mf", "/testdir"}, fs) - exitCode := RunWithOptions(opts) + opts := testOpts([]string{ + testApp, cmdGenerate, "-q", "-f", "-o", testOutput, testDir, + }, fs) + exitCode := runCLI(opts) require.Equal(t, 0, exitCode) // Verify output file exists and was overwritten - content, err := afero.ReadFile(fs, "/output.mf") + content, err := afero.ReadFile(fs, testOutput) require.NoError(t, err) - assert.NotEqual(t, "old content", string(content), "manifest should be overwritten") + assert.NotEqual(t, "old content", string(content), + "manifest should be overwritten") // Verify temp file does NOT exist - tmpExists, err := afero.Exists(fs, "/output.mf.tmp") + tmpExists, err := afero.Exists(fs, testOutputTmp) require.NoError(t, err) - assert.False(t, tmpExists, "temp file should not exist after successful generation") + assert.False(t, tmpExists, + "temp file should not exist after successful generation") } func TestGenerateFailsWithoutForceWhenOutputExists(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() // Create test file - require.NoError(t, fs.MkdirAll("/testdir", 0o755)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("hello"), 0o644)) + require.NoError(t, fs.MkdirAll(testDir, 0o755)) + writeTestFile(t, fs, testFile1, "hello") // Create existing manifest - require.NoError(t, afero.WriteFile(fs, "/output.mf", []byte("existing"), 0o644)) + writeTestFile(t, fs, testOutput, "existing") // Generate manifest WITHOUT --force (should fail) - opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/output.mf", "/testdir"}, fs) - exitCode := RunWithOptions(opts) + opts := testOpts([]string{testApp, cmdGenerate, "-q", "-o", testOutput, testDir}, fs) + exitCode := runCLI(opts) assert.Equal(t, 1, exitCode, "should fail when output exists without --force") // Verify original content is preserved - content, err := afero.ReadFile(fs, "/output.mf") + content, err := afero.ReadFile(fs, testOutput) require.NoError(t, err) assert.Equal(t, "existing", string(content), "original file should be preserved") } func TestGenerateAtomicWriteUsesTemp(t *testing.T) { + t.Parallel() + // This test verifies that generate uses a temp file by checking // that the output file doesn't exist until generation completes. // We do this by generating to a path and verifying the temp file @@ -411,183 +526,239 @@ func TestGenerateAtomicWriteUsesTemp(t *testing.T) { fs := afero.NewMemMapFs() // Create test file - require.NoError(t, fs.MkdirAll("/testdir", 0o755)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("hello"), 0o644)) + require.NoError(t, fs.MkdirAll(testDir, 0o755)) + writeTestFile(t, fs, testFile1, "hello") // Generate manifest - opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/output.mf", "/testdir"}, fs) - exitCode := RunWithOptions(opts) + opts := testOpts([]string{testApp, cmdGenerate, "-q", "-o", testOutput, testDir}, fs) + exitCode := runCLI(opts) require.Equal(t, 0, exitCode) // Both output file should exist and temp should not - exists, _ := afero.Exists(fs, "/output.mf") + exists, _ := afero.Exists(fs, testOutput) assert.True(t, exists, "output file should exist") - tmpExists, _ := afero.Exists(fs, "/output.mf.tmp") + tmpExists, _ := afero.Exists(fs, testOutputTmp) assert.False(t, tmpExists, "temp file should be cleaned up") // Verify manifest is valid (not empty) - content, err := afero.ReadFile(fs, "/output.mf") + content, err := afero.ReadFile(fs, testOutput) require.NoError(t, err) - assert.True(t, len(content) > 0, "manifest should not be empty") + assert.NotEmpty(t, content, "manifest should not be empty") } // failingWriterFs wraps a filesystem and makes writes fail after N bytes type failingWriterFs struct { afero.Fs + failAfter int64 written int64 } type failingFile struct { afero.File + fs *failingWriterFs } func (f *failingFile) Write(p []byte) (int, error) { f.fs.written += int64(len(p)) if f.fs.written > f.fs.failAfter { - return 0, fmt.Errorf("simulated write failure") + return 0, errSimulatedWrite } + return f.File.Write(p) } +//nolint:ireturn // Create must return afero.File to satisfy afero.Fs. func (fs *failingWriterFs) Create(name string) (afero.File, error) { f, err := fs.Fs.Create(name) if err != nil { return nil, err } + return &failingFile{File: f, fs: fs}, nil } func TestGenerateAtomicWriteCleansUpOnError(t *testing.T) { + t.Parallel() + baseFs := afero.NewMemMapFs() // Create test files - need enough content to trigger the write failure - require.NoError(t, baseFs.MkdirAll("/testdir", 0o755)) - require.NoError(t, afero.WriteFile(baseFs, "/testdir/file1.txt", []byte("hello world this is a test file"), 0o644)) + require.NoError(t, baseFs.MkdirAll(testDir, 0o755)) + writeTestFile(t, baseFs, testFile1, "hello world this is a test file") // Wrap with failing writer that fails after writing some bytes fs := &failingWriterFs{Fs: baseFs, failAfter: 10} // Generate manifest - should fail during write - opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/output.mf", "/testdir"}, fs) - exitCode := RunWithOptions(opts) + opts := testOpts([]string{testApp, cmdGenerate, "-q", "-o", testOutput, testDir}, fs) + exitCode := runCLI(opts) assert.Equal(t, 1, exitCode, "should fail due to write error") // With atomic writes: output.mf should NOT exist (temp was cleaned up) // With non-atomic writes: output.mf WOULD exist (partial/empty) - exists, _ := afero.Exists(baseFs, "/output.mf") - assert.False(t, exists, "output file should not exist after failed generation (atomic write)") + exists, _ := afero.Exists(baseFs, testOutput) + assert.False(t, exists, + "output file should not exist after failed generation (atomic write)") // Temp file should also not exist - tmpExists, _ := afero.Exists(baseFs, "/output.mf.tmp") - assert.False(t, tmpExists, "temp file should be cleaned up after failed generation") + tmpExists, _ := afero.Exists(baseFs, testOutputTmp) + assert.False(t, tmpExists, + "temp file should be cleaned up after failed generation") } func TestGenerateValidatesInputPaths(t *testing.T) { - fs := afero.NewMemMapFs() + t.Parallel() - // Create one valid directory - require.NoError(t, fs.MkdirAll("/validdir", 0o755)) - require.NoError(t, afero.WriteFile(fs, "/validdir/file.txt", []byte("content"), 0o644)) + seedValidDir := func(t *testing.T, fs afero.Fs) { + t.Helper() + + require.NoError(t, fs.MkdirAll("/validdir", 0o755)) + writeTestFile(t, fs, "/validdir/file.txt", "content") + } t.Run("nonexistent path fails fast", func(t *testing.T) { - opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/output.mf", "/nonexistent"}, fs) - exitCode := RunWithOptions(opts) + t.Parallel() + + fs := afero.NewMemMapFs() + seedValidDir(t, fs) + + opts := testOpts([]string{ + testApp, cmdGenerate, "-q", "-o", testOutput, "/nonexistent", + }, fs) + exitCode := runCLI(opts) assert.Equal(t, 1, exitCode) - stderr := opts.Stderr.(*bytes.Buffer).String() + + stderr := testStderr(t, opts) assert.Contains(t, stderr, "path does not exist") assert.Contains(t, stderr, "/nonexistent") }) t.Run("mix of valid and invalid paths fails fast", func(t *testing.T) { - opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/output.mf", "/validdir", "/alsononexistent"}, fs) - exitCode := RunWithOptions(opts) + t.Parallel() + + fs := afero.NewMemMapFs() + seedValidDir(t, fs) + + opts := testOpts([]string{ + testApp, cmdGenerate, "-q", "-o", testOutput, + "/validdir", "/alsononexistent", + }, fs) + exitCode := runCLI(opts) assert.Equal(t, 1, exitCode) - stderr := opts.Stderr.(*bytes.Buffer).String() + + stderr := testStderr(t, opts) assert.Contains(t, stderr, "path does not exist") assert.Contains(t, stderr, "/alsononexistent") // Output file should not have been created - exists, _ := afero.Exists(fs, "/output.mf") - assert.False(t, exists, "output file should not exist when path validation fails") + exists, _ := afero.Exists(fs, testOutput) + assert.False(t, exists, + "output file should not exist when path validation fails") }) t.Run("valid paths succeed", func(t *testing.T) { - opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/output.mf", "/validdir"}, fs) - exitCode := RunWithOptions(opts) + t.Parallel() + + fs := afero.NewMemMapFs() + seedValidDir(t, fs) + + opts := testOpts([]string{ + testApp, cmdGenerate, "-q", "-o", testOutput, "/validdir", + }, fs) + exitCode := runCLI(opts) assert.Equal(t, 0, exitCode) }) } func TestCheckDetectsManifestCorruption(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() - rng := rand.New(rand.NewSource(42)) + rng := rand.New(rand.NewSource(42)) //nolint:gosec // deterministic test data // Create many small files with random names to generate a ~1MB manifest // Each manifest entry is roughly 50-60 bytes, so we need ~20000 files - require.NoError(t, fs.MkdirAll("/testdir", 0o755)) + require.NoError(t, fs.MkdirAll(testDir, 0o755)) numFiles := 20000 - for i := 0; i < numFiles; i++ { + for range numFiles { // Generate random filename - filename := fmt.Sprintf("/testdir/%08x%08x%08x.dat", rng.Uint32(), rng.Uint32(), rng.Uint32()) + filename := fmt.Sprintf("/testdir/%08x%08x%08x.dat", + rng.Uint32(), rng.Uint32(), rng.Uint32()) // Small random content content := make([]byte, 16+rng.Intn(48)) - rng.Read(content) + _, _ = rng.Read(content) require.NoError(t, afero.WriteFile(fs, filename, content, 0o644)) } // Generate manifest outside of testdir - opts := testOpts([]string{"mfer", "generate", "-q", "-o", "/manifest.mf", "/testdir"}, fs) - exitCode := RunWithOptions(opts) + opts := testOpts([]string{testApp, cmdGenerate, "-q", "-o", testManifest, testDir}, fs) + exitCode := runCLI(opts) require.Equal(t, 0, exitCode, "generate should succeed") // Read the valid manifest and verify it's approximately 1MB - validManifest, err := afero.ReadFile(fs, "/manifest.mf") + validManifest, err := afero.ReadFile(fs, testManifest) require.NoError(t, err) - require.True(t, len(validManifest) >= 1024*1024, "manifest should be at least 1MB, got %d bytes", len(validManifest)) + require.GreaterOrEqual(t, len(validManifest), 1024*1024, + "manifest should be at least 1MB, got %d bytes", len(validManifest)) t.Logf("manifest size: %d bytes (%d files)", len(validManifest), numFiles) // First corruption: truncate the manifest - require.NoError(t, afero.WriteFile(fs, "/manifest.mf", validManifest[:len(validManifest)/2], 0o644)) + require.NoError(t, afero.WriteFile(fs, testManifest, + validManifest[:len(validManifest)/2], 0o644)) // Check should fail with truncated manifest - opts = testOpts([]string{"mfer", "check", "-q", "--base", "/testdir", "/manifest.mf"}, fs) - exitCode = RunWithOptions(opts) + opts = testOpts([]string{ + testApp, cmdCheck, "-q", testFlagBase, testDir, testManifest, + }, fs) + exitCode = runCLI(opts) assert.Equal(t, 1, exitCode, "check should fail with truncated manifest") // Verify check passes with valid manifest - require.NoError(t, afero.WriteFile(fs, "/manifest.mf", validManifest, 0o644)) - opts = testOpts([]string{"mfer", "check", "-q", "--base", "/testdir", "/manifest.mf"}, fs) - exitCode = RunWithOptions(opts) + require.NoError(t, afero.WriteFile(fs, testManifest, validManifest, 0o644)) + + opts = testOpts([]string{ + testApp, cmdCheck, "-q", testFlagBase, testDir, testManifest, + }, fs) + exitCode = runCLI(opts) require.Equal(t, 0, exitCode, "check should pass with valid manifest") // Now do 500 random corruption iterations - for i := 0; i < 500; i++ { + for i := range 500 { // Corrupt: write a random byte at a random offset corrupted := make([]byte, len(validManifest)) copy(corrupted, validManifest) offset := rng.Intn(len(corrupted)) originalByte := corrupted[offset] + // Make sure we actually change the byte - newByte := byte(rng.Intn(256)) + buf := make([]byte, 1) + + newByte := originalByte for newByte == originalByte { - newByte = byte(rng.Intn(256)) + _, _ = rng.Read(buf) + newByte = buf[0] } + corrupted[offset] = newByte - require.NoError(t, afero.WriteFile(fs, "/manifest.mf", corrupted, 0o644)) + require.NoError(t, afero.WriteFile(fs, testManifest, corrupted, 0o644)) // Check should fail with corrupted manifest - opts = testOpts([]string{"mfer", "check", "-q", "--base", "/testdir", "/manifest.mf"}, fs) - exitCode = RunWithOptions(opts) - assert.Equal(t, 1, exitCode, "iteration %d: check should fail with corrupted manifest (offset %d, 0x%02x -> 0x%02x)", + opts = testOpts([]string{ + testApp, cmdCheck, "-q", testFlagBase, testDir, testManifest, + }, fs) + exitCode = runCLI(opts) + assert.Equal(t, 1, exitCode, + "iteration %d: check should fail with corrupted manifest "+ + "(offset %d, 0x%02x -> 0x%02x)", i, offset, originalByte, newByte) // Restore valid manifest for next iteration - require.NoError(t, afero.WriteFile(fs, "/manifest.mf", validManifest, 0o644)) + require.NoError(t, afero.WriteFile(fs, testManifest, validManifest, 0o644)) } } diff --git a/internal/cli/errmsg_test.go b/internal/cli/errmsg_test.go new file mode 100644 index 0000000..873658a --- /dev/null +++ b/internal/cli/errmsg_test.go @@ -0,0 +1,168 @@ +//nolint:testpackage // white-box tests exercise unexported internals +package cli + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// errMsgCase is one pinned user-visible error message. +type errMsgCase struct { + name string + err error + want string +} + +const ( + msgFpA = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA" + msgFpB = "BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB" +) + +func checkErrMsgCases(t *testing.T, cases []errMsgCase) { + t.Helper() + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tc.want, tc.err.Error()) + }) + } +} + +// TestErrorMessagesVerbatim pins the exact rendered text of the CLI's +// user-visible error messages. +// +// These strings are an interface: they are grepped for in CI pipelines +// and quoted in bug reports. The messages are assembled by wrapping +// static sentinels, and it is easy to change what a user sees while +// only meaning to make an error matchable with errors.Is - which is +// precisely what happened once already. Any change to a string below is +// therefore a deliberate, separately stated change, never a side effect +// of a refactor. +func TestErrorMessagesVerbatim(t *testing.T) { + t.Parallel() + + checkErrMsgCases(t, []errMsgCase{ + { + name: "check: no manifest found", + err: fmt.Errorf("%w in %s (looked for index.mf and .index.mf)", + errNoManifestFound, "/tmp/x"), + want: "no manifest found in /tmp/x " + + "(looked for index.mf and .index.mf)", + }, + { + name: "check: invalid fingerprint length", + err: fmt.Errorf("%w, got %d", errInvalidFingerprint, 8), + want: "invalid fingerprint: must be exactly 40 hex characters, got 8", + }, + { + name: "check: manifest not signed", + err: fmt.Errorf("%w, but signature from %s is required", + errManifestNotSigned, msgFpA), + want: "manifest is not signed, but signature from " + msgFpA + + " is required", + }, + { + name: "check: signer mismatch", + err: fmt.Errorf("embedded signing key fingerprint %s %w %s", + msgFpA, errSignerMismatch, msgFpB), + want: "embedded signing key fingerprint " + msgFpA + + " does not match required " + msgFpB, + }, + { + name: "gen: path does not exist", + err: fmt.Errorf("%w: %s", errPathNotExist, "nope"), + want: "path does not exist: nope", + }, + { + name: "gen: output file exists", + err: fmt.Errorf("output file %s %w", "index.mf", errOutputExists), + want: "output file index.mf already exists " + + "(use --force to overwrite)", + }, + { + name: "mfer: unknown command", + err: fmt.Errorf("%w %q", errUnknownCommand, "bogus"), + want: `unknown command "bogus"`, + }, + }) +} + +// TestFetchErrorMessagesVerbatim pins the fetch and manifest-loader +// messages; see TestErrorMessagesVerbatim for why. +func TestFetchErrorMessagesVerbatim(t *testing.T) { + t.Parallel() + + checkErrMsgCases(t, []errMsgCase{ + { + name: "manifest_loader: http status", + err: fmt.Errorf("failed to fetch %s: %w %d", + "https://example.com/index.mf", errHTTPStatus, 404), + want: "failed to fetch https://example.com/index.mf: HTTP 404", + }, + { + name: "fetch: manifest http status", + err: fmt.Errorf("failed to fetch manifest: %w %d", + errHTTPStatus, 404), + want: "failed to fetch manifest: HTTP 404", + }, + { + name: "fetch: file http status", + err: fmt.Errorf("%w %d", errHTTPStatus, 500), + want: "HTTP 500", + }, + { + name: "fetch: empty path", + err: errEmptyPath, + want: "empty path", + }, + { + name: "fetch: absolute path", + err: fmt.Errorf("%w: %s", errAbsolutePath, "/etc/passwd"), + want: "absolute path not allowed: /etc/passwd", + }, + { + name: "fetch: path traversal", + err: fmt.Errorf("%w: %s", errPathTraversal, "../x"), + want: "path traversal not allowed: ../x", + }, + { + name: "fetch: size mismatch", + err: fmt.Errorf("%w: expected %d bytes, got %d", + errSizeMismatch, 10, 9), + want: "size mismatch: expected 10 bytes, got 9", + }, + { + name: "fetch: url required", + err: errURLRequired, + want: "URL argument required", + }, + { + name: "fetch: hash mismatch", + err: errHashMismatch, + want: "hash mismatch", + }, + }) +} + +// TestSentinelsAreMatchable checks that the wrapped forms of the +// messages above remain matchable with errors.Is, which is the reason +// the sentinels exist at all. +func TestSentinelsAreMatchable(t *testing.T) { + t.Parallel() + + wrapped := fmt.Errorf("embedded signing key fingerprint %s %w %s", + "a", errSignerMismatch, "b") + require.ErrorIs(t, wrapped, errSignerMismatch) + + wrapped = fmt.Errorf("output file %s %w", "index.mf", errOutputExists) + require.ErrorIs(t, wrapped, errOutputExists) + + wrapped = fmt.Errorf("failed to fetch manifest: %w %d", errHTTPStatus, 404) + require.ErrorIs(t, wrapped, errHTTPStatus) + + assert.NotErrorIs(t, errHashMismatch, errSizeMismatch) +} diff --git a/internal/cli/export.go b/internal/cli/export.go index aca8ebf..a557137 100644 --- a/internal/cli/export.go +++ b/internal/cli/export.go @@ -29,6 +29,7 @@ func (mfa *CLIApp) exportManifestOperation(ctx *cli.Context) error { if err != nil { return fmt.Errorf("export: %w", err) } + defer func() { _ = rc.Close() }() manifest, err := mfer.NewManifestFromReader(rc) @@ -41,21 +42,23 @@ func (mfa *CLIApp) exportManifestOperation(ctx *cli.Context) error { for _, f := range files { entry := ExportEntry{ - Path: f.Path, - Size: f.Size, - Hashes: make([]string, 0, len(f.Hashes)), + Path: f.GetPath(), + Size: f.GetSize(), + Hashes: make([]string, 0, len(f.GetHashes())), } - for _, h := range f.Hashes { - entry.Hashes = append(entry.Hashes, hex.EncodeToString(h.MultiHash)) + for _, h := range f.GetHashes() { + entry.Hashes = append(entry.Hashes, hex.EncodeToString(h.GetMultiHash())) } - if f.Mtime != nil { - t := time.Unix(f.Mtime.Seconds, int64(f.Mtime.Nanos)).UTC().Format(time.RFC3339Nano) + if mtime, ok := entryMtime(f); ok { + t := mtime.UTC().Format(time.RFC3339Nano) entry.Mtime = &t } - if f.Ctime != nil { - t := time.Unix(f.Ctime.Seconds, int64(f.Ctime.Nanos)).UTC().Format(time.RFC3339Nano) + + if f.GetCtime() != nil { + t := time.Unix(f.GetCtime().GetSeconds(), int64(f.GetCtime().GetNanos())). + UTC().Format(time.RFC3339Nano) entry.Ctime = &t } @@ -64,7 +67,9 @@ func (mfa *CLIApp) exportManifestOperation(ctx *cli.Context) error { enc := json.NewEncoder(mfa.Stdout) enc.SetIndent("", " ") - if err := enc.Encode(entries); err != nil { + + err = enc.Encode(entries) + if err != nil { return fmt.Errorf("export: failed to encode JSON: %w", err) } diff --git a/internal/cli/export_test.go b/internal/cli/export_test.go index 6d1f87d..ef51e02 100644 --- a/internal/cli/export_test.go +++ b/internal/cli/export_test.go @@ -14,9 +14,12 @@ import ( "sneak.berlin/go/mfer/mfer" ) +const testCmdExport = "export" + // buildTestManifest creates a manifest from in-memory files and returns its bytes. func buildTestManifest(t *testing.T, files map[string][]byte) []byte { t.Helper() + sourceFs := afero.NewMemMapFs() for path, content := range files { require.NoError(t, sourceFs.MkdirAll("/", 0o755)) @@ -28,11 +31,15 @@ func buildTestManifest(t *testing.T, files map[string][]byte) []byte { require.NoError(t, s.EnumerateFS(sourceFs, "/", nil)) var buf bytes.Buffer + require.NoError(t, s.ToManifest(context.Background(), &buf, nil)) + return buf.Bytes() } func TestExportManifestOperation(t *testing.T) { + t.Parallel() + testFiles := map[string][]byte{ "hello.txt": []byte("Hello, World!"), "sub/file.txt": []byte("nested content"), @@ -44,9 +51,10 @@ func TestExportManifestOperation(t *testing.T) { require.NoError(t, afero.WriteFile(fs, "/test.mf", manifestData, 0o644)) var stdout, stderr bytes.Buffer - exitCode := RunWithOptions(&RunOptions{ - Appname: "mfer", - Args: []string{"mfer", "export", "/test.mf"}, + + exitCode := runCLI(&RunOptions{ + Appname: testApp, + Args: []string{testApp, testCmdExport, "/test.mf"}, Stdin: &bytes.Buffer{}, Stdout: &stdout, Stderr: &stderr, @@ -64,28 +72,33 @@ func TestExportManifestOperation(t *testing.T) { for _, e := range entries { pathSet[e.Path] = true assert.NotEmpty(t, e.Hashes, "entry %s should have hashes", e.Path) - assert.Greater(t, e.Size, int64(0), "entry %s should have positive size", e.Path) + assert.Positive(t, e.Size, "entry %s should have positive size", e.Path) } + assert.True(t, pathSet["hello.txt"]) assert.True(t, pathSet["sub/file.txt"]) } func TestExportFromHTTPURL(t *testing.T) { + t.Parallel() + testFiles := map[string][]byte{ "a.txt": []byte("aaa"), } manifestData := buildTestManifest(t, testFiles) - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/octet-stream") - _, _ = w.Write(manifestData) - })) + server := httptest.NewServer( + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/octet-stream") + _, _ = w.Write(manifestData) + })) defer server.Close() var stdout, stderr bytes.Buffer - exitCode := RunWithOptions(&RunOptions{ - Appname: "mfer", - Args: []string{"mfer", "export", server.URL + "/index.mf"}, + + exitCode := runCLI(&RunOptions{ + Appname: testApp, + Args: []string{testApp, testCmdExport, server.URL + "/index.mf"}, Stdin: &bytes.Buffer{}, Stdout: &stdout, Stderr: &stderr, @@ -101,21 +114,25 @@ func TestExportFromHTTPURL(t *testing.T) { } func TestListFromHTTPURL(t *testing.T) { + t.Parallel() + testFiles := map[string][]byte{ "one.txt": []byte("1"), "two.txt": []byte("22"), } manifestData := buildTestManifest(t, testFiles) - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - _, _ = w.Write(manifestData) - })) + server := httptest.NewServer( + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write(manifestData) + })) defer server.Close() var stdout, stderr bytes.Buffer - exitCode := RunWithOptions(&RunOptions{ - Appname: "mfer", - Args: []string{"mfer", "list", server.URL + "/index.mf"}, + + exitCode := runCLI(&RunOptions{ + Appname: testApp, + Args: []string{testApp, "list", server.URL + "/index.mf"}, Stdin: &bytes.Buffer{}, Stdout: &stdout, Stderr: &stderr, @@ -129,6 +146,8 @@ func TestListFromHTTPURL(t *testing.T) { } func TestIsHTTPURL(t *testing.T) { + t.Parallel() + assert.True(t, isHTTPURL("http://example.com/manifest.mf")) assert.True(t, isHTTPURL("https://example.com/manifest.mf")) assert.False(t, isHTTPURL("/local/path.mf")) diff --git a/internal/cli/fetch.go b/internal/cli/fetch.go index 677b65a..8254a4c 100644 --- a/internal/cli/fetch.go +++ b/internal/cli/fetch.go @@ -2,7 +2,9 @@ package cli import ( "bytes" + "context" "crypto/sha256" + "errors" "fmt" "io" "net/http" @@ -20,6 +22,45 @@ import ( "sneak.berlin/go/mfer/mfer" ) +const ( + // progressChanBuffer is the buffer size of the download progress + // channel. + progressChanBuffer = 10 + + // bitsPerByte converts a bytes-per-second rate to bits per second. + bitsPerByte = 8 + + // dirPerms is the permission mode for directories created for + // downloaded files. Fetched trees are content that is normally + // published (served by a web server, read by another uid), so the + // traversal bit for group and other must stay set. + dirPerms os.FileMode = 0o755 + + // Bitrate unit thresholds in bits per second. + bpsPerGbps = 1e9 + bpsPerMbps = 1e6 + bpsPerKbps = 1e3 +) + +var ( + // errURLRequired indicates the fetch command was run without a URL + // argument. + errURLRequired = errors.New("URL argument required") + // errEmptyPath indicates an empty file path in the manifest. + errEmptyPath = errors.New("empty path") + // errAbsolutePath indicates an absolute file path in the manifest. + errAbsolutePath = errors.New("absolute path not allowed") + // errPathTraversal indicates a manifest path escaping the target + // directory. + errPathTraversal = errors.New("path traversal not allowed") + // errSizeMismatch indicates a downloaded file with an unexpected + // size. + errSizeMismatch = errors.New("size mismatch") + // errHashMismatch indicates a downloaded file whose hash matches no + // manifest hash. + errHashMismatch = errors.New("hash mismatch") +) + // DownloadProgress reports the progress of a single file download. type DownloadProgress struct { Path string // File path being downloaded @@ -29,14 +70,98 @@ type DownloadProgress struct { ETA time.Duration // Estimated time to completion } +// httpGet issues a GET request for the given URL using the provided +// context and returns the response. The caller must close the body. +// +// Errors are returned unwrapped: this helper replaced direct http.Get +// calls, and each caller already supplies its own context string, so +// adding one here would change user-visible messages. +func httpGet(ctx context.Context, fileURL string) (*http.Response, error) { + req, err := http.NewRequestWithContext(ctx, http.MethodGet, fileURL, nil) + if err != nil { + return nil, err + } + + resp, err := http.DefaultClient.Do(req) + if err != nil { + return nil, err + } + + return resp, nil +} + +// reportDownloadProgress renders download progress until the channel +// closes, then closes done. +func reportDownloadProgress(progress <-chan DownloadProgress, done chan<- struct{}) { + defer close(done) + + for p := range progress { + rate := formatBitrate(p.BytesPerSec * bitsPerByte) + if p.ETA > 0 { + log.Infof("%s: %s/%s, %s, ETA %s", + p.Path, humanize.IBytes(safeUint64(p.BytesRead)), + humanize.IBytes(safeUint64(p.TotalBytes)), + rate, p.ETA.Round(time.Second)) + } else { + log.Infof("%s: %s/%s, %s", + p.Path, humanize.IBytes(safeUint64(p.BytesRead)), + humanize.IBytes(safeUint64(p.TotalBytes)), rate) + } + } +} + +// manifestBaseURL returns the URL of the directory containing the +// manifest, with a trailing slash. +func manifestBaseURL(manifestURL string) (*url.URL, error) { + baseURL, err := url.Parse(manifestURL) + if err != nil { + return nil, fmt.Errorf("fetch: invalid manifest URL: %w", err) + } + + baseURL.Path = path.Dir(baseURL.Path) + if !strings.HasSuffix(baseURL.Path, "/") { + baseURL.Path += "/" + } + + return baseURL, nil +} + +// downloadManifestFiles downloads every file in the manifest, reporting +// progress on the progress channel. +func downloadManifestFiles( + ctx context.Context, + baseURL *url.URL, + files []*mfer.MFFilePath, + progress chan<- DownloadProgress, +) error { + for _, f := range files { + // Sanitize the path to prevent path traversal attacks + localPath, err := sanitizePath(f.GetPath()) + if err != nil { + return fmt.Errorf("invalid path in manifest: %w", err) + } + + fileURL := baseURL.String() + encodeFilePath(f.GetPath()) + log.Infof("fetching %s", f.GetPath()) + + err = downloadFile(ctx, fileURL, localPath, f, progress) + if err != nil { + return fmt.Errorf("failed to download %s: %w", f.GetPath(), err) + } + } + + return nil +} + func (mfa *CLIApp) fetchManifestOperation(ctx *cli.Context) error { log.Debug("fetchManifestOperation()") if ctx.Args().Len() == 0 { - return fmt.Errorf("URL argument required") + return errURLRequired } inputURL := ctx.Args().Get(0) + manifestURL, err := resolveManifestURL(inputURL) if err != nil { return fmt.Errorf("invalid URL: %w", err) @@ -45,14 +170,16 @@ func (mfa *CLIApp) fetchManifestOperation(ctx *cli.Context) error { log.Infof("fetching manifest from %s", manifestURL) // Fetch manifest - resp, err := http.Get(manifestURL) + resp, err := httpGet(ctx.Context, manifestURL) if err != nil { return fmt.Errorf("failed to fetch manifest: %w", err) } + defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { - return fmt.Errorf("failed to fetch manifest: HTTP %d", resp.StatusCode) + return fmt.Errorf("failed to fetch manifest: %w %d", + errHTTPStatus, resp.StatusCode) } // Parse manifest @@ -65,74 +192,43 @@ func (mfa *CLIApp) fetchManifestOperation(ctx *cli.Context) error { log.Infof("manifest contains %d files", len(files)) // Compute base URL (directory containing manifest) - baseURL, err := url.Parse(manifestURL) + baseURL, err := manifestBaseURL(manifestURL) if err != nil { - return fmt.Errorf("fetch: invalid manifest URL: %w", err) - } - baseURL.Path = path.Dir(baseURL.Path) - if !strings.HasSuffix(baseURL.Path, "/") { - baseURL.Path += "/" + return err } // Calculate total bytes to download var totalBytes int64 for _, f := range files { - totalBytes += f.Size + totalBytes += f.GetSize() } - // Create progress channel - progress := make(chan DownloadProgress, 10) - - // Start progress reporter goroutine + // Create progress channel and start progress reporter goroutine + progress := make(chan DownloadProgress, progressChanBuffer) done := make(chan struct{}) - go func() { - defer close(done) - for p := range progress { - rate := formatBitrate(p.BytesPerSec * 8) - if p.ETA > 0 { - log.Infof("%s: %s/%s, %s, ETA %s", - p.Path, humanize.IBytes(uint64(p.BytesRead)), humanize.IBytes(uint64(p.TotalBytes)), - rate, p.ETA.Round(time.Second)) - } else { - log.Infof("%s: %s/%s, %s", - p.Path, humanize.IBytes(uint64(p.BytesRead)), humanize.IBytes(uint64(p.TotalBytes)), rate) - } - } - }() + + go reportDownloadProgress(progress, done) // Track download start time startTime := time.Now() // Download each file - for _, f := range files { - // Sanitize the path to prevent path traversal attacks - localPath, err := sanitizePath(f.Path) - if err != nil { - close(progress) - <-done - return fmt.Errorf("invalid path in manifest: %w", err) - } - - fileURL := baseURL.String() + encodeFilePath(f.Path) - log.Infof("fetching %s", f.Path) - - if err := downloadFile(fileURL, localPath, f, progress); err != nil { - close(progress) - <-done - return fmt.Errorf("failed to download %s: %w", f.Path, err) - } - } + dlErr := downloadManifestFiles(ctx.Context, baseURL, files, progress) close(progress) <-done + if dlErr != nil { + return dlErr + } + // Print summary elapsed := time.Since(startTime) avgBytesPerSec := float64(totalBytes) / elapsed.Seconds() - avgRate := formatBitrate(avgBytesPerSec * 8) + avgRate := formatBitrate(avgBytesPerSec * bitsPerByte) log.Infof("downloaded %d files (%s) in %.1fs (%s avg)", len(files), - humanize.IBytes(uint64(totalBytes)), + humanize.IBytes(safeUint64(totalBytes)), elapsed.Seconds(), avgRate) @@ -145,6 +241,7 @@ func encodeFilePath(p string) string { for i, seg := range segments { segments[i] = url.PathEscape(seg) } + return strings.Join(segments, "/") } @@ -153,12 +250,12 @@ func encodeFilePath(p string) string { func sanitizePath(p string) (string, error) { // Reject empty paths if p == "" { - return "", fmt.Errorf("empty path") + return "", errEmptyPath } // Reject absolute paths if filepath.IsAbs(p) { - return "", fmt.Errorf("absolute path not allowed: %s", p) + return "", fmt.Errorf("%w: %s", errAbsolutePath, p) } // Clean the path to resolve . and .. @@ -166,12 +263,12 @@ func sanitizePath(p string) (string, error) { // Reject paths that escape the current directory if strings.HasPrefix(cleaned, ".."+string(filepath.Separator)) || cleaned == ".." { - return "", fmt.Errorf("path traversal not allowed: %s", p) + return "", fmt.Errorf("%w: %s", errPathTraversal, p) } // Also check for absolute paths after cleaning (handles edge cases) if filepath.IsAbs(cleaned) { - return "", fmt.Errorf("absolute path not allowed: %s", p) + return "", fmt.Errorf("%w: %s", errAbsolutePath, p) } return cleaned, nil @@ -214,10 +311,14 @@ type progressWriter struct { func (pw *progressWriter) Write(p []byte) (int, error) { n, err := pw.w.Write(p) + pw.written += int64(n) if pw.progress != nil { - var bytesPerSec float64 - var eta time.Duration + var ( + bytesPerSec float64 + eta time.Duration + ) + elapsed := time.Since(pw.startTime) if elapsed > 0 && pw.written > 0 { bytesPerSec = float64(pw.written) / elapsed.Seconds() @@ -226,6 +327,7 @@ func (pw *progressWriter) Write(p []byte) (int, error) { eta = time.Duration(float64(remainingBytes)/bytesPerSec) * time.Second } } + sendProgress(pw.progress, DownloadProgress{ Path: pw.path, BytesRead: pw.written, @@ -234,18 +336,19 @@ func (pw *progressWriter) Write(p []byte) (int, error) { ETA: eta, }) } + return n, err } // formatBitrate formats a bits-per-second value with appropriate unit prefix. func formatBitrate(bps float64) string { switch { - case bps >= 1e9: - return fmt.Sprintf("%.1f Gbps", bps/1e9) - case bps >= 1e6: - return fmt.Sprintf("%.1f Mbps", bps/1e6) - case bps >= 1e3: - return fmt.Sprintf("%.1f Kbps", bps/1e3) + case bps >= bpsPerGbps: + return fmt.Sprintf("%.1f Gbps", bps/bpsPerGbps) + case bps >= bpsPerMbps: + return fmt.Sprintf("%.1f Mbps", bps/bpsPerMbps) + case bps >= bpsPerKbps: + return fmt.Sprintf("%.1f Kbps", bps/bpsPerKbps) default: return fmt.Sprintf("%.0f bps", bps) } @@ -259,53 +362,100 @@ func sendProgress(ch chan<- DownloadProgress, p DownloadProgress) { } } -// downloadFile downloads a URL to a local file path with hash verification. -// It downloads to a temporary file, verifies the hash, then renames to the final path. -// Progress is reported via the progress channel. -func downloadFile(fileURL, localPath string, entry *mfer.MFFilePath, progress chan<- DownloadProgress) error { - // Create parent directories if needed +// tempPathFor computes the temporary download path for a local file. +// For dotfiles, just append .tmp (they're already hidden); for regular +// files, prefix with . and append .tmp. +func tempPathFor(localPath string) string { dir := filepath.Dir(localPath) - if dir != "" && dir != "." { - if err := os.MkdirAll(dir, 0o755); err != nil { - return fmt.Errorf("failed to create directory %s: %w", dir, err) - } - } - - // Compute temp file path in the same directory - // For dotfiles, just append .tmp (they're already hidden) - // For regular files, prefix with . and append .tmp base := filepath.Base(localPath) + var tmpName string if strings.HasPrefix(base, ".") { tmpName = base + ".tmp" } else { tmpName = "." + base + ".tmp" } - tmpPath := filepath.Join(dir, tmpName) + if dir == "" || dir == "." { - tmpPath = tmpName + return tmpName } + return filepath.Join(dir, tmpName) +} + +// verifyDownloadedHash checks the computed sha256 digest against the +// manifest entry's hashes; at least one must match. +func verifyDownloadedHash(digest []byte, entry *mfer.MFFilePath) error { + computed, err := multihash.Encode(digest, multihash.SHA2_256) + if err != nil { + return fmt.Errorf("failed to encode hash: %w", err) + } + + for _, hash := range entry.GetHashes() { + if bytes.Equal(computed, hash.GetMultiHash()) { + return nil + } + } + + return errHashMismatch +} + +// downloadFile downloads a URL to a local file path with hash verification. +// It downloads to a temporary file, verifies the hash, then renames to the final path. +// Progress is reported via the progress channel. +func downloadFile( + ctx context.Context, + fileURL, localPath string, + entry *mfer.MFFilePath, + progress chan<- DownloadProgress, +) error { + // Enforce the path invariant here rather than relying on the caller, + // so every entry point to downloadFile gets the same treatment. + localPath, err := sanitizePath(localPath) + if err != nil { + return fmt.Errorf("invalid path: %w", err) + } + + // Create parent directories if needed + dir := filepath.Dir(localPath) + if dir != "" && dir != "." { + err := os.MkdirAll(dir, dirPerms) + if err != nil { + return fmt.Errorf("failed to create directory %s: %w", dir, err) + } + } + + tmpPath := tempPathFor(localPath) + // Fetch file - resp, err := http.Get(fileURL) //nolint:gosec // URL constructed from manifest base + resp, err := httpGet(ctx, fileURL) if err != nil { return fmt.Errorf("HTTP request failed: %w", err) } + defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { - return fmt.Errorf("HTTP %d", resp.StatusCode) + return fmt.Errorf("%w %d", errHTTPStatus, resp.StatusCode) } // Determine expected size - expectedSize := entry.Size + expectedSize := entry.GetSize() + totalBytes := resp.ContentLength if totalBytes < 0 { totalBytes = expectedSize } - // Create temp file - out, err := os.Create(tmpPath) + // Create temp file. + // + // G304: tmpPath is derived from localPath, which sanitizePath above + // constrains lexically to a relative path that does not escape the + // destination directory. That is a purely lexical guarantee: it does + // not resolve symlinks, so a pre-existing symlink inside the + // destination tree can still redirect this write outside of it + // (tracked in issue #86). + out, err := os.Create(tmpPath) //nolint:gosec // G304: see comment above if err != nil { return fmt.Errorf("failed to create temp file: %w", err) } @@ -328,45 +478,50 @@ func downloadFile(fileURL, localPath string, entry *mfer.MFFilePath, progress ch // Close file before checking errors (to flush writes) closeErr := out.Close() - // If copy failed, clean up temp file and return error - if copyErr != nil { + err = finishDownload( + tmpPath, localPath, written, expectedSize, h.Sum(nil), entry, + copyErr, closeErr) + if err != nil { _ = os.Remove(tmpPath) + + return err + } + + return nil +} + +// finishDownload validates the copy result, verifies size and hash, and +// moves the temp file into place. On error the caller removes tmpPath. +func finishDownload( + tmpPath, localPath string, + written, expectedSize int64, + digest []byte, + entry *mfer.MFFilePath, + copyErr, closeErr error, +) error { + if copyErr != nil { return copyErr } + if closeErr != nil { - _ = os.Remove(tmpPath) return closeErr } // Verify size if written != expectedSize { - _ = os.Remove(tmpPath) - return fmt.Errorf("size mismatch: expected %d bytes, got %d", expectedSize, written) - } - - // Encode computed hash as multihash - computed, err := multihash.Encode(h.Sum(nil), multihash.SHA2_256) - if err != nil { - _ = os.Remove(tmpPath) - return fmt.Errorf("failed to encode hash: %w", err) + return fmt.Errorf("%w: expected %d bytes, got %d", + errSizeMismatch, expectedSize, written) } // Verify hash against manifest (at least one must match) - hashMatch := false - for _, hash := range entry.Hashes { - if bytes.Equal(computed, hash.MultiHash) { - hashMatch = true - break - } - } - if !hashMatch { - _ = os.Remove(tmpPath) - return fmt.Errorf("hash mismatch") + err := verifyDownloadedHash(digest, entry) + if err != nil { + return err } // Rename temp file to final path - if err := os.Rename(tmpPath, localPath); err != nil { - _ = os.Remove(tmpPath) + err = os.Rename(tmpPath, localPath) + if err != nil { return fmt.Errorf("failed to rename temp file: %w", err) } diff --git a/internal/cli/fetch_test.go b/internal/cli/fetch_test.go index 43414a7..dd7a3e8 100644 --- a/internal/cli/fetch_test.go +++ b/internal/cli/fetch_test.go @@ -1,3 +1,4 @@ +//nolint:testpackage // white-box tests exercise unexported internals package cli import ( @@ -16,13 +17,25 @@ import ( "sneak.berlin/go/mfer/mfer" ) +const ( + testFileTxt = "file.txt" + testDirFile = "dir/file.txt" + testIndexMF = "https://example.com/path/index.mf" + + // Exactly what url.Parse renders, with no wrapper of our own. + urlParseControlCharErr = `parse "http://example.com/\x7f": ` + + `net/url: invalid control character in URL` +) + func TestEncodeFilePath(t *testing.T) { + t.Parallel() + tests := []struct { input string expected string }{ - {"file.txt", "file.txt"}, - {"dir/file.txt", "dir/file.txt"}, + {testFileTxt, testFileTxt}, + {testDirFile, testDirFile}, {"my file.txt", "my%20file.txt"}, {"dir/my file.txt", "dir/my%20file.txt"}, {"file#1.txt", "file%231.txt"}, @@ -33,6 +46,8 @@ func TestEncodeFilePath(t *testing.T) { for _, tt := range tests { t.Run(tt.input, func(t *testing.T) { + t.Parallel() + result := encodeFilePath(tt.input) assert.Equal(t, tt.expected, result) }) @@ -40,23 +55,27 @@ func TestEncodeFilePath(t *testing.T) { } func TestSanitizePath(t *testing.T) { + t.Parallel() + // Valid paths that should be accepted validTests := []struct { input string expected string }{ - {"file.txt", "file.txt"}, - {"dir/file.txt", "dir/file.txt"}, + {testFileTxt, testFileTxt}, + {testDirFile, testDirFile}, {"dir/subdir/file.txt", "dir/subdir/file.txt"}, - {"./file.txt", "file.txt"}, - {"./dir/file.txt", "dir/file.txt"}, - {"dir/./file.txt", "dir/file.txt"}, + {"./file.txt", testFileTxt}, + {"./dir/file.txt", testDirFile}, + {"dir/./file.txt", testDirFile}, } for _, tt := range validTests { t.Run("valid:"+tt.input, func(t *testing.T) { + t.Parallel() + result, err := sanitizePath(tt.input) - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, tt.expected, result) }) } @@ -78,6 +97,8 @@ func TestSanitizePath(t *testing.T) { for _, tt := range invalidTests { t.Run("invalid:"+tt.desc, func(t *testing.T) { + t.Parallel() + _, err := sanitizePath(tt.input) assert.Error(t, err, "expected error for path: %s", tt.input) }) @@ -85,36 +106,115 @@ func TestSanitizePath(t *testing.T) { } func TestResolveManifestURL(t *testing.T) { + t.Parallel() + tests := []struct { input string expected string }{ // Already ends with .mf - use as-is - {"https://example.com/path/index.mf", "https://example.com/path/index.mf"}, + {testIndexMF, testIndexMF}, {"https://example.com/path/custom.mf", "https://example.com/path/custom.mf"}, {"https://example.com/foo.mf", "https://example.com/foo.mf"}, // Directory with trailing slash - append index.mf - {"https://example.com/path/", "https://example.com/path/index.mf"}, + {"https://example.com/path/", testIndexMF}, {"https://example.com/", "https://example.com/index.mf"}, // Directory without trailing slash - add slash and index.mf - {"https://example.com/path", "https://example.com/path/index.mf"}, + {"https://example.com/path", testIndexMF}, {"https://example.com", "https://example.com/index.mf"}, // With query strings - {"https://example.com/path?foo=bar", "https://example.com/path/index.mf?foo=bar"}, + { + "https://example.com/path?foo=bar", + "https://example.com/path/index.mf?foo=bar", + }, } for _, tt := range tests { t.Run(tt.input, func(t *testing.T) { + t.Parallel() + result, err := resolveManifestURL(tt.input) - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, tt.expected, result) }) } + + // The sole caller wraps this error as "invalid URL: %w", so + // resolveManifestURL must return url.Parse's error unadorned. + t.Run("invalid:control character", func(t *testing.T) { + t.Parallel() + + _, err := resolveManifestURL("http://example.com/\x7f") + require.ErrorContains(t, err, urlParseControlCharErr) + assert.NotContains(t, err.Error(), "failed to parse URL") + }) } +// scanToManifest scans sourceFs and returns the serialized manifest bytes. +func scanToManifest(t *testing.T, sourceFs afero.Fs) []byte { + t.Helper() + + s := mfer.NewScannerWithOptions(&mfer.ScannerOptions{Fs: sourceFs}) + require.NoError(t, s.EnumerateFS(sourceFs, "/", nil)) + + var manifestBuf bytes.Buffer + + require.NoError(t, s.ToManifest(context.Background(), &manifestBuf, nil)) + + return manifestBuf.Bytes() +} + +// chdirTemp switches the working directory to a fresh temp dir for the +// duration of the test and returns its path. +func chdirTemp(t *testing.T) string { + t.Helper() + + destDir := t.TempDir() + + origDir, err := os.Getwd() + require.NoError(t, err) + + require.NoError(t, os.Chdir(destDir)) + t.Cleanup(func() { _ = os.Chdir(origDir) }) + + return destDir +} + +// fetchTestHandler serves the manifest at /index.mf and the given files +// at their paths. +func fetchTestHandler( + manifestData []byte, testFiles map[string][]byte, +) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + path := r.URL.Path + if path == "/index.mf" { + w.Header().Set("Content-Type", "application/octet-stream") + _, _ = w.Write(manifestData) + + return + } + + // Strip leading slash + if len(path) > 0 && path[0] == '/' { + path = path[1:] + } + + content, exists := testFiles[path] + if !exists { + http.NotFound(w, r) + + return + } + + w.Header().Set("Content-Type", "application/octet-stream") + _, _ = w.Write(content) + } +} + +//nolint:paralleltest // changes the process-global working directory func TestFetchFromHTTP(t *testing.T) { // Create source filesystem with test files sourceFs := afero.NewMemMapFs() @@ -134,51 +234,14 @@ func TestFetchFromHTTP(t *testing.T) { } // Generate manifest using scanner - opts := &mfer.ScannerOptions{ - Fs: sourceFs, - } - s := mfer.NewScannerWithOptions(opts) - require.NoError(t, s.EnumerateFS(sourceFs, "/", nil)) - - var manifestBuf bytes.Buffer - require.NoError(t, s.ToManifest(context.Background(), &manifestBuf, nil)) - manifestData := manifestBuf.Bytes() + manifestData := scanToManifest(t, sourceFs) // Create HTTP server that serves the source filesystem - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - path := r.URL.Path - if path == "/index.mf" { - w.Header().Set("Content-Type", "application/octet-stream") - _, _ = w.Write(manifestData) - return - } - - // Strip leading slash - if len(path) > 0 && path[0] == '/' { - path = path[1:] - } - - content, exists := testFiles[path] - if !exists { - http.NotFound(w, r) - return - } - - w.Header().Set("Content-Type", "application/octet-stream") - _, _ = w.Write(content) - })) + server := httptest.NewServer(fetchTestHandler(manifestData, testFiles)) defer server.Close() - // Create destination directory - destDir, err := os.MkdirTemp("", "mfer-fetch-test-*") - require.NoError(t, err) - defer func() { _ = os.RemoveAll(destDir) }() - - // Change to dest directory for the test - origDir, err := os.Getwd() - require.NoError(t, err) - require.NoError(t, os.Chdir(destDir)) - defer func() { _ = os.Chdir(origDir) }() + // Change to a fresh destination directory for the test + destDir := chdirTemp(t) // Parse the manifest to get file entries manifest, err := mfer.NewManifestFromReader(bytes.NewReader(manifestData)) @@ -189,132 +252,125 @@ func TestFetchFromHTTP(t *testing.T) { // Download each file using downloadFile progress := make(chan DownloadProgress, 10) + go func() { - for range progress { - // Drain progress channel + for p := range progress { + _ = p // drain progress channel } }() baseURL := server.URL + "/" + for _, f := range files { - localPath, err := sanitizePath(f.Path) + localPath, err := sanitizePath(f.GetPath()) require.NoError(t, err) - fileURL := baseURL + f.Path - err = downloadFile(fileURL, localPath, f, progress) - require.NoError(t, err, "failed to download %s", f.Path) + fileURL := baseURL + f.GetPath() + err = downloadFile(context.Background(), fileURL, localPath, f, progress) + require.NoError(t, err, "failed to download %s", f.GetPath()) } + close(progress) // Verify downloaded files match originals for path, expectedContent := range testFiles { downloadedPath := filepath.Join(destDir, path) + //nolint:gosec // test-controlled path downloadedContent, err := os.ReadFile(downloadedPath) require.NoError(t, err, "failed to read downloaded file %s", path) - assert.Equal(t, expectedContent, downloadedContent, "content mismatch for %s", path) + assert.Equal(t, expectedContent, downloadedContent, + "content mismatch for %s", path) } } +//nolint:paralleltest // changes the process-global working directory func TestFetchHashMismatch(t *testing.T) { // Create source filesystem with a test file sourceFs := afero.NewMemMapFs() originalContent := []byte("Original content") require.NoError(t, afero.WriteFile(sourceFs, "/file.txt", originalContent, 0o644)) - // Generate manifest - opts := &mfer.ScannerOptions{Fs: sourceFs} - s := mfer.NewScannerWithOptions(opts) - require.NoError(t, s.EnumerateFS(sourceFs, "/", nil)) + // Generate and parse manifest + manifestData := scanToManifest(t, sourceFs) - var manifestBuf bytes.Buffer - require.NoError(t, s.ToManifest(context.Background(), &manifestBuf, nil)) - - // Parse manifest - manifest, err := mfer.NewManifestFromReader(bytes.NewReader(manifestBuf.Bytes())) + manifest, err := mfer.NewManifestFromReader(bytes.NewReader(manifestData)) require.NoError(t, err) + files := manifest.Files() require.Len(t, files, 1) // Create server that serves DIFFERENT content (to trigger hash mismatch) tamperedContent := []byte("Tampered content!") - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/octet-stream") - _, _ = w.Write(tamperedContent) - })) + + server := httptest.NewServer( + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/octet-stream") + _, _ = w.Write(tamperedContent) + })) defer server.Close() - // Create temp directory - destDir, err := os.MkdirTemp("", "mfer-fetch-hash-test-*") - require.NoError(t, err) - defer func() { _ = os.RemoveAll(destDir) }() - - origDir, err := os.Getwd() - require.NoError(t, err) - require.NoError(t, os.Chdir(destDir)) - defer func() { _ = os.Chdir(origDir) }() + // Work in a fresh temp directory + chdirTemp(t) // Try to download - should fail with hash mismatch - err = downloadFile(server.URL+"/file.txt", "file.txt", files[0], nil) - assert.Error(t, err) + err = downloadFile(context.Background(), + server.URL+"/file.txt", testFileTxt, files[0], nil) + require.Error(t, err) assert.Contains(t, err.Error(), "mismatch") // Verify temp file was cleaned up _, err = os.Stat(".file.txt.tmp") - assert.True(t, os.IsNotExist(err), "temp file should be cleaned up on hash mismatch") + assert.True(t, os.IsNotExist(err), + "temp file should be cleaned up on hash mismatch") // Verify final file was not created - _, err = os.Stat("file.txt") - assert.True(t, os.IsNotExist(err), "final file should not exist on hash mismatch") + _, err = os.Stat(testFileTxt) + assert.True(t, os.IsNotExist(err), + "final file should not exist on hash mismatch") } +//nolint:paralleltest // changes the process-global working directory func TestFetchSizeMismatch(t *testing.T) { // Create source filesystem with a test file sourceFs := afero.NewMemMapFs() originalContent := []byte("Original content with specific size") require.NoError(t, afero.WriteFile(sourceFs, "/file.txt", originalContent, 0o644)) - // Generate manifest - opts := &mfer.ScannerOptions{Fs: sourceFs} - s := mfer.NewScannerWithOptions(opts) - require.NoError(t, s.EnumerateFS(sourceFs, "/", nil)) + // Generate and parse manifest + manifestData := scanToManifest(t, sourceFs) - var manifestBuf bytes.Buffer - require.NoError(t, s.ToManifest(context.Background(), &manifestBuf, nil)) - - // Parse manifest - manifest, err := mfer.NewManifestFromReader(bytes.NewReader(manifestBuf.Bytes())) + manifest, err := mfer.NewManifestFromReader(bytes.NewReader(manifestData)) require.NoError(t, err) + files := manifest.Files() require.Len(t, files, 1) // Create server that serves content with wrong size wrongSizeContent := []byte("Short") - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/octet-stream") - _, _ = w.Write(wrongSizeContent) - })) + + server := httptest.NewServer( + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/octet-stream") + _, _ = w.Write(wrongSizeContent) + })) defer server.Close() - // Create temp directory - destDir, err := os.MkdirTemp("", "mfer-fetch-size-test-*") - require.NoError(t, err) - defer func() { _ = os.RemoveAll(destDir) }() - - origDir, err := os.Getwd() - require.NoError(t, err) - require.NoError(t, os.Chdir(destDir)) - defer func() { _ = os.Chdir(origDir) }() + // Work in a fresh temp directory + chdirTemp(t) // Try to download - should fail with size mismatch - err = downloadFile(server.URL+"/file.txt", "file.txt", files[0], nil) - assert.Error(t, err) + err = downloadFile(context.Background(), + server.URL+"/file.txt", testFileTxt, files[0], nil) + require.Error(t, err) assert.Contains(t, err.Error(), "size mismatch") // Verify temp file was cleaned up _, err = os.Stat(".file.txt.tmp") - assert.True(t, os.IsNotExist(err), "temp file should be cleaned up on size mismatch") + assert.True(t, os.IsNotExist(err), + "temp file should be cleaned up on size mismatch") } +//nolint:paralleltest // changes the process-global working directory func TestFetchProgress(t *testing.T) { // Create source filesystem with a larger test file sourceFs := afero.NewMemMapFs() @@ -322,53 +378,47 @@ func TestFetchProgress(t *testing.T) { content := bytes.Repeat([]byte("x"), 100*1024) // 100KB require.NoError(t, afero.WriteFile(sourceFs, "/large.txt", content, 0o644)) - // Generate manifest - opts := &mfer.ScannerOptions{Fs: sourceFs} - s := mfer.NewScannerWithOptions(opts) - require.NoError(t, s.EnumerateFS(sourceFs, "/", nil)) + // Generate and parse manifest + manifestData := scanToManifest(t, sourceFs) - var manifestBuf bytes.Buffer - require.NoError(t, s.ToManifest(context.Background(), &manifestBuf, nil)) - - // Parse manifest - manifest, err := mfer.NewManifestFromReader(bytes.NewReader(manifestBuf.Bytes())) + manifest, err := mfer.NewManifestFromReader(bytes.NewReader(manifestData)) require.NoError(t, err) + files := manifest.Files() require.Len(t, files, 1) // Create server that serves the content - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.Header().Set("Content-Type", "application/octet-stream") - w.Header().Set("Content-Length", "102400") - // Write in chunks to allow progress reporting - reader := bytes.NewReader(content) - _, _ = io.Copy(w, reader) - })) + server := httptest.NewServer( + http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("Content-Type", "application/octet-stream") + w.Header().Set("Content-Length", "102400") + // Write in chunks to allow progress reporting + reader := bytes.NewReader(content) + _, _ = io.Copy(w, reader) + })) defer server.Close() - // Create temp directory - destDir, err := os.MkdirTemp("", "mfer-fetch-progress-test-*") - require.NoError(t, err) - defer func() { _ = os.RemoveAll(destDir) }() - - origDir, err := os.Getwd() - require.NoError(t, err) - require.NoError(t, os.Chdir(destDir)) - defer func() { _ = os.Chdir(origDir) }() + // Work in a fresh temp directory + chdirTemp(t) // Set up progress channel and collect updates progress := make(chan DownloadProgress, 100) + var progressUpdates []DownloadProgress + done := make(chan struct{}) + go func() { for p := range progress { progressUpdates = append(progressUpdates, p) } + close(done) }() // Download - err = downloadFile(server.URL+"/large.txt", "large.txt", files[0], progress) + err = downloadFile(context.Background(), + server.URL+"/large.txt", "large.txt", files[0], progress) close(progress) <-done @@ -380,7 +430,8 @@ func TestFetchProgress(t *testing.T) { // Verify final progress shows complete if len(progressUpdates) > 0 { last := progressUpdates[len(progressUpdates)-1] - assert.Equal(t, int64(len(content)), last.BytesRead, "final progress should show all bytes read") + assert.Equal(t, int64(len(content)), last.BytesRead, + "final progress should show all bytes read") assert.Equal(t, "large.txt", last.Path) } diff --git a/internal/cli/freshen.go b/internal/cli/freshen.go index a078ee5..bc0ee20 100644 --- a/internal/cli/freshen.go +++ b/internal/cli/freshen.go @@ -2,6 +2,7 @@ package cli import ( "crypto/sha256" + "errors" "fmt" "io" "io/fs" @@ -16,6 +17,19 @@ import ( "sneak.berlin/go/mfer/mfer" ) +const ( + // hashBufSize is the read buffer size used when hashing files. + hashBufSize = 64 * 1024 + + // scanProgressInterval is how many scanned files pass between + // progress updates. + scanProgressInterval = 100 +) + +// errEntryMissingMtime indicates a manifest entry that carries no +// modification time where one is required to carry it forward unchanged. +var errEntryMissingMtime = errors.New("manifest entry has no mtime") + // FreshenStatus contains progress information for the freshen operation. type FreshenStatus struct { Phase string // "scan" or "hash" @@ -36,195 +50,292 @@ type freshenEntry struct { existing *mfer.MFFilePath // existing manifest entry if unchanged } -func (mfa *CLIApp) freshenManifestOperation(ctx *cli.Context) error { - log.Debug("freshenManifestOperation()") +// freshenScanner walks the filesystem and compares it against the +// entries of an existing manifest. +type freshenScanner struct { + fs afero.Fs + absBase string + manifestBase string + includeDotfiles bool + followSymlinks bool + showProgress bool + existingByPath map[string]*mfer.MFFilePath - basePath := ctx.String("base") - showProgress := ctx.Bool("progress") - includeDotfiles := ctx.Bool("include-dotfiles") - followSymlinks := ctx.Bool("follow-symlinks") + entries []*freshenEntry + scanCount int64 + changed int64 + added int64 + unchanged int64 +} - // Find manifest file - var manifestPath string - var err error +// resolveSymlink resolves a symlink to its target's FileInfo. The +// second return value is false when the entry should be skipped. +func (s *freshenScanner) resolveSymlink(path string) (fs.FileInfo, bool) { + if !s.followSymlinks { + return nil, false + } - if ctx.Args().Len() > 0 { - arg := ctx.Args().Get(0) - info, statErr := mfa.Fs.Stat(arg) - if statErr == nil && info.IsDir() { - manifestPath, err = findManifest(mfa.Fs, arg) - if err != nil { - return fmt.Errorf("freshen: %w", err) - } - } else { - manifestPath = arg - } + realPath, err := filepath.EvalSymlinks(path) + if err != nil { + return nil, false // Skip broken symlinks + } + + realInfo, err := s.fs.Stat(realPath) + if err != nil || realInfo.IsDir() { + return nil, false + } + + return realInfo, true +} + +// recordEntry classifies a scanned file as changed, unchanged, or added +// relative to the existing manifest. +func (s *freshenScanner) recordEntry(relPath string, info fs.FileInfo) { + existing, inManifest := s.existingByPath[relPath] + if !inManifest { + s.added++ + + log.Verbosef("A %s", relPath) + s.entries = append(s.entries, &freshenEntry{ + path: relPath, + size: info.Size(), + mtime: info.ModTime(), + needsHash: true, + }) + + return + } + + // Check if changed (size or mtime). An entry with no recorded mtime + // cannot be compared, so it counts as changed and gets re-hashed; + // silently treating the absent mtime as the Unix epoch would classify + // every such entry as changed without saying why. + existingMtime, haveMtime := entryMtime(existing) + if !haveMtime { + log.Debugf("%s: manifest entry has no mtime, treating as changed", + relPath) + } + + if !haveMtime || existing.GetSize() != info.Size() || + !existingMtime.Equal(info.ModTime()) { + s.changed++ + + log.Verbosef("M %s", relPath) + s.entries = append(s.entries, &freshenEntry{ + path: relPath, + size: info.Size(), + mtime: info.ModTime(), + needsHash: true, + }) } else { - manifestPath, err = findManifest(mfa.Fs, ".") - if err != nil { - return fmt.Errorf("freshen: %w", err) - } + s.unchanged++ + + s.entries = append(s.entries, &freshenEntry{ + path: relPath, + size: info.Size(), + mtime: info.ModTime(), + needsHash: false, + existing: existing, + }) + } + // Mark as seen + delete(s.existingByPath, relPath) +} + +// walk is the afero.Walk callback for the scan phase. +func (s *freshenScanner) walk(path string, info fs.FileInfo, walkErr error) error { + if walkErr != nil { + return walkErr } - log.Infof("loading manifest from %s", manifestPath) - - // Load existing manifest - manifest, err := mfer.NewManifestFromFile(mfa.Fs, manifestPath) + // Get relative path + relPath, err := filepath.Rel(s.absBase, path) if err != nil { - return fmt.Errorf("failed to load manifest: %w", err) + return fmt.Errorf( + "freshen: failed to compute relative path for %s: %w", path, err) } - existingFiles := manifest.Files() - log.Infof("manifest contains %d files", len(existingFiles)) - - // Build map of existing entries by path - existingByPath := make(map[string]*mfer.MFFilePath, len(existingFiles)) - for _, f := range existingFiles { - existingByPath[f.Path] = f + // Skip the manifest file itself + if relPath == s.manifestBase || relPath == "."+s.manifestBase { + return nil } - // Phase 1: Scan filesystem - log.Infof("scanning filesystem...") - startScan := time.Now() - - var entries []*freshenEntry - var scanCount int64 - var removed, changed, added, unchanged int64 - - absBase, err := filepath.Abs(basePath) - if err != nil { - return fmt.Errorf("freshen: invalid base path: %w", err) - } - - err = afero.Walk(mfa.Fs, absBase, func(path string, info fs.FileInfo, walkErr error) error { - if walkErr != nil { - return walkErr - } - - // Get relative path - relPath, err := filepath.Rel(absBase, path) - if err != nil { - return fmt.Errorf("freshen: failed to compute relative path for %s: %w", path, err) - } - - // Skip the manifest file itself - if relPath == filepath.Base(manifestPath) || relPath == "."+filepath.Base(manifestPath) { - return nil - } - - // Handle dotfiles - if !includeDotfiles && mfer.IsHiddenPath(filepath.ToSlash(relPath)) { - if info.IsDir() { - return filepath.SkipDir - } - return nil - } - - // Skip directories + // Handle dotfiles + if !s.includeDotfiles && mfer.IsHiddenPath(filepath.ToSlash(relPath)) { if info.IsDir() { - return nil - } - - // Handle symlinks - if info.Mode()&fs.ModeSymlink != 0 { - if !followSymlinks { - return nil - } - realPath, err := filepath.EvalSymlinks(path) - if err != nil { - return nil // Skip broken symlinks - } - realInfo, err := mfa.Fs.Stat(realPath) - if err != nil || realInfo.IsDir() { - return nil - } - info = realInfo - } - - scanCount++ - - // Check against existing manifest - existing, inManifest := existingByPath[relPath] - if inManifest { - // Check if changed (size or mtime) - existingMtime := time.Unix(existing.Mtime.Seconds, int64(existing.Mtime.Nanos)) - if existing.Size != info.Size() || !existingMtime.Equal(info.ModTime()) { - changed++ - log.Verbosef("M %s", relPath) - entries = append(entries, &freshenEntry{ - path: relPath, - size: info.Size(), - mtime: info.ModTime(), - needsHash: true, - }) - } else { - unchanged++ - entries = append(entries, &freshenEntry{ - path: relPath, - size: info.Size(), - mtime: info.ModTime(), - needsHash: false, - existing: existing, - }) - } - // Mark as seen - delete(existingByPath, relPath) - } else { - added++ - log.Verbosef("A %s", relPath) - entries = append(entries, &freshenEntry{ - path: relPath, - size: info.Size(), - mtime: info.ModTime(), - needsHash: true, - }) - } - - // Report scan progress - if showProgress && scanCount%100 == 0 { - log.Progressf("Scanning: %d files found", scanCount) + return filepath.SkipDir } return nil - }) - - if showProgress { - log.ProgressDone() } - if err != nil { - return fmt.Errorf("failed to scan filesystem: %w", err) + // Skip directories + if info.IsDir() { + return nil } - // Remaining entries in existingByPath are removed files - removed = int64(len(existingByPath)) - for path := range existingByPath { - log.Verbosef("D %s", path) + // Handle symlinks + if info.Mode()&fs.ModeSymlink != 0 { + realInfo, keep := s.resolveSymlink(path) + if !keep { + return nil + } + + info = realInfo } - scanDuration := time.Since(startScan) - log.Infof("scan complete in %s: %d unchanged, %d changed, %d added, %d removed", - scanDuration.Round(time.Millisecond), unchanged, changed, added, removed) + s.scanCount++ - // Calculate total bytes to hash - var totalHashBytes int64 - var filesToHash int64 - for _, e := range entries { - if e.needsHash { - totalHashBytes += e.size - filesToHash++ + // Check against existing manifest + s.recordEntry(relPath, info) + + // Report scan progress + if s.showProgress && s.scanCount%scanProgressInterval == 0 { + log.Progressf("Scanning: %d files found", s.scanCount) + } + + return nil +} + +// resolveFreshenManifestPath determines the manifest path from the CLI +// arguments, searching directories for a manifest where needed. +func (mfa *CLIApp) resolveFreshenManifestPath(ctx *cli.Context) (string, error) { + if ctx.Args().Len() == 0 { + return findManifest(mfa.Fs, ".") + } + + arg := ctx.Args().Get(0) + + info, statErr := mfa.Fs.Stat(arg) + if statErr == nil && info.IsDir() { + return findManifest(mfa.Fs, arg) + } + + return arg, nil +} + +// freshenHasher hashes changed and added files and feeds all entries to +// a manifest builder. +type freshenHasher struct { + fs afero.Fs + absBase string + showProgress bool + totalHashBytes int64 + filesToHash int64 + startHash time.Time + builder *mfer.Builder + + hashedFiles int64 + hashedBytes int64 +} + +// reportProgress renders hashing progress for the current byte count. +func (h *freshenHasher) reportProgress(n int64) { + if !h.showProgress { + return + } + + currentBytes := h.hashedBytes + n + elapsed := time.Since(h.startHash) + + var ( + rate float64 + eta time.Duration + ) + + if elapsed > 0 && currentBytes > 0 { + rate = float64(currentBytes) / elapsed.Seconds() + + remaining := h.totalHashBytes - currentBytes + if rate > 0 { + eta = time.Duration(float64(remaining)/rate) * time.Second } } - // Phase 2: Hash changed and new files - if filesToHash > 0 { - log.Infof("hashing %d files (%s)...", filesToHash, humanize.IBytes(uint64(totalHashBytes))) + if eta > 0 { + log.Progressf("Hashing: %d/%d files, %s/s, ETA %s", + h.hashedFiles, h.filesToHash, humanize.IBytes(safeRateUint64(rate)), + eta.Round(time.Second)) + } else { + log.Progressf("Hashing: %d/%d files, %s/s", + h.hashedFiles, h.filesToHash, humanize.IBytes(safeRateUint64(rate))) + } +} + +// processEntry hashes the entry if needed and adds it to the builder. +func (h *freshenHasher) processEntry(e *freshenEntry) error { + if !e.needsHash { + // Use existing entry + err := addExistingToBuilder(h.builder, e.existing) + if err != nil { + return fmt.Errorf("failed to add %s: %w", e.path, err) + } + + return nil } - startHash := time.Now() - var hashedFiles int64 - var hashedBytes int64 + // Need to read and hash the file + absPath := filepath.Join(h.absBase, e.path) + f, err := h.fs.Open(absPath) + if err != nil { + return fmt.Errorf("failed to open %s: %w", e.path, err) + } + + hash, bytesRead, err := hashFile(f, h.reportProgress) + _ = f.Close() + + if err != nil { + return fmt.Errorf("failed to hash %s: %w", e.path, err) + } + + h.hashedBytes += bytesRead + h.hashedFiles++ + + // Add to builder with computed hash + err = addFileToBuilder(h.builder, e.path, e.size, e.mtime, hash) + if err != nil { + return fmt.Errorf("failed to add %s: %w", e.path, err) + } + + return nil +} + +// writeFreshenedManifest writes the manifest atomically (write to a +// temp file, then rename over the target). +func writeFreshenedManifest( + afs afero.Fs, builder *mfer.Builder, manifestPath string, +) error { + tmpPath := manifestPath + ".tmp" + + outFile, err := afs.Create(tmpPath) + if err != nil { + return fmt.Errorf("failed to create temp file: %w", err) + } + + err = builder.Build(outFile) + _ = outFile.Close() + + if err != nil { + _ = afs.Remove(tmpPath) + + return fmt.Errorf("failed to write manifest: %w", err) + } + + // Rename temp to final + err = afs.Rename(tmpPath, manifestPath) + if err != nil { + _ = afs.Remove(tmpPath) + + return fmt.Errorf("failed to rename manifest: %w", err) + } + + return nil +} + +// newFreshenBuilder constructs the manifest builder configured from CLI +// flags. +func newFreshenBuilder(ctx *cli.Context) *mfer.Builder { builder := mfer.NewBuilder() if ctx.Bool("include-timestamps") { builder.SetIncludeTimestamps(true) @@ -238,6 +349,77 @@ func (mfa *CLIApp) freshenManifestOperation(ctx *cli.Context) error { log.Infof("signing manifest with GPG key: %s", signKey) } + return builder +} + +// freshenScan runs the scan phase against the loaded manifest entries +// and returns the populated scanner and the count of removed files. +func (mfa *CLIApp) freshenScan( + ctx *cli.Context, manifestPath, absBase string, + existingByPath map[string]*mfer.MFFilePath, +) (*freshenScanner, int64, error) { + log.Infof("scanning filesystem...") + + startScan := time.Now() + showProgress := ctx.Bool("progress") + + scanner := &freshenScanner{ + fs: mfa.Fs, + absBase: absBase, + manifestBase: filepath.Base(manifestPath), + includeDotfiles: ctx.Bool("include-dotfiles"), + followSymlinks: ctx.Bool("follow-symlinks"), + showProgress: showProgress, + existingByPath: existingByPath, + } + + err := afero.Walk(mfa.Fs, absBase, scanner.walk) + + if showProgress { + log.ProgressDone() + } + + if err != nil { + return nil, 0, fmt.Errorf("failed to scan filesystem: %w", err) + } + + // Remaining entries in existingByPath are removed files + removed := int64(len(existingByPath)) + for path := range existingByPath { + log.Verbosef("D %s", path) + } + + scanDuration := time.Since(startScan) + log.Infof("scan complete in %s: %d unchanged, %d changed, %d added, %d removed", + scanDuration.Round(time.Millisecond), scanner.unchanged, scanner.changed, + scanner.added, removed) + + return scanner, removed, nil +} + +// hashTotals returns the total byte count and file count of entries +// that need hashing. +func hashTotals(entries []*freshenEntry) (int64, int64) { + var ( + totalHashBytes int64 + filesToHash int64 + ) + + for _, e := range entries { + if e.needsHash { + totalHashBytes += e.size + filesToHash++ + } + } + + return totalHashBytes, filesToHash +} + +// runFreshenHash processes every entry through the hasher, aborting if +// the context is canceled. +func runFreshenHash( + ctx *cli.Context, hasher *freshenHasher, entries []*freshenEntry, +) error { for _, e := range entries { select { case <-ctx.Done(): @@ -245,122 +427,154 @@ func (mfa *CLIApp) freshenManifestOperation(ctx *cli.Context) error { default: } - if e.needsHash { - // Need to read and hash the file - absPath := filepath.Join(absBase, e.path) - f, err := mfa.Fs.Open(absPath) - if err != nil { - return fmt.Errorf("failed to open %s: %w", e.path, err) - } - - hash, bytesRead, err := hashFile(f, e.size, func(n int64) { - if showProgress { - currentBytes := hashedBytes + n - elapsed := time.Since(startHash) - var rate float64 - var eta time.Duration - if elapsed > 0 && currentBytes > 0 { - rate = float64(currentBytes) / elapsed.Seconds() - remaining := totalHashBytes - currentBytes - if rate > 0 { - eta = time.Duration(float64(remaining)/rate) * time.Second - } - } - if eta > 0 { - log.Progressf("Hashing: %d/%d files, %s/s, ETA %s", - hashedFiles, filesToHash, humanize.IBytes(uint64(rate)), eta.Round(time.Second)) - } else { - log.Progressf("Hashing: %d/%d files, %s/s", - hashedFiles, filesToHash, humanize.IBytes(uint64(rate))) - } - } - }) - _ = f.Close() - - if err != nil { - return fmt.Errorf("failed to hash %s: %w", e.path, err) - } - - hashedBytes += bytesRead - hashedFiles++ - - // Add to builder with computed hash - if err := addFileToBuilder(builder, e.path, e.size, e.mtime, hash); err != nil { - return fmt.Errorf("failed to add %s: %w", e.path, err) - } - } else { - // Use existing entry - if err := addExistingToBuilder(builder, e.existing); err != nil { - return fmt.Errorf("failed to add %s: %w", e.path, err) - } + err := hasher.processEntry(e) + if err != nil { + return err } } + return nil +} + +// loadExistingEntries loads the manifest and indexes its file entries +// by path. +func (mfa *CLIApp) loadExistingEntries( + manifestPath string, +) (map[string]*mfer.MFFilePath, error) { + log.Infof("loading manifest from %s", manifestPath) + + // Load existing manifest + manifest, err := mfer.NewManifestFromFile(mfa.Fs, manifestPath) + if err != nil { + return nil, fmt.Errorf("failed to load manifest: %w", err) + } + + existingFiles := manifest.Files() + log.Infof("manifest contains %d files", len(existingFiles)) + + // Build map of existing entries by path + existingByPath := make(map[string]*mfer.MFFilePath, len(existingFiles)) + for _, f := range existingFiles { + existingByPath[f.GetPath()] = f + } + + return existingByPath, nil +} + +func (mfa *CLIApp) freshenManifestOperation(ctx *cli.Context) error { + log.Debug("freshenManifestOperation()") + + basePath := ctx.String("base") + showProgress := ctx.Bool("progress") + + // Find manifest file + manifestPath, err := mfa.resolveFreshenManifestPath(ctx) + if err != nil { + return fmt.Errorf("freshen: %w", err) + } + + existingByPath, err := mfa.loadExistingEntries(manifestPath) + if err != nil { + return err + } + + absBase, err := filepath.Abs(basePath) + if err != nil { + return fmt.Errorf("freshen: invalid base path: %w", err) + } + + // Phase 1: Scan filesystem + scanner, removed, err := mfa.freshenScan(ctx, manifestPath, absBase, + existingByPath) + if err != nil { + return err + } + + // Calculate total bytes to hash + totalHashBytes, filesToHash := hashTotals(scanner.entries) + + // Phase 2: Hash changed and new files + if filesToHash > 0 { + log.Infof("hashing %d files (%s)...", filesToHash, + humanize.IBytes(safeUint64(totalHashBytes))) + } + + hasher := &freshenHasher{ + fs: mfa.Fs, + absBase: absBase, + showProgress: showProgress, + totalHashBytes: totalHashBytes, + filesToHash: filesToHash, + startHash: time.Now(), + builder: newFreshenBuilder(ctx), + } + + err = runFreshenHash(ctx, hasher, scanner.entries) + if err != nil { + return err + } + if showProgress && filesToHash > 0 { log.ProgressDone() } // Print summary log.Infof("freshen complete: %d unchanged, %d changed, %d added, %d removed", - unchanged, changed, added, removed) + scanner.unchanged, scanner.changed, scanner.added, removed) // Skip writing if nothing changed - if changed == 0 && added == 0 && removed == 0 { + if scanner.changed == 0 && scanner.added == 0 && removed == 0 { log.Infof("manifest unchanged, skipping write") + return nil } // Write updated manifest atomically (write to temp, then rename) - tmpPath := manifestPath + ".tmp" - outFile, err := mfa.Fs.Create(tmpPath) + err = writeFreshenedManifest(mfa.Fs, hasher.builder, manifestPath) if err != nil { - return fmt.Errorf("failed to create temp file: %w", err) - } - - err = builder.Build(outFile) - _ = outFile.Close() - if err != nil { - _ = mfa.Fs.Remove(tmpPath) - return fmt.Errorf("failed to write manifest: %w", err) - } - - // Rename temp to final - if err := mfa.Fs.Rename(tmpPath, manifestPath); err != nil { - _ = mfa.Fs.Remove(tmpPath) - return fmt.Errorf("failed to rename manifest: %w", err) + return err } totalDuration := time.Since(mfa.startupTime) - if hashedBytes > 0 { - hashDuration := time.Since(startHash) - hashRate := float64(hashedBytes) / hashDuration.Seconds() + if hasher.hashedBytes > 0 { + hashDuration := time.Since(hasher.startHash) + hashRate := float64(hasher.hashedBytes) / hashDuration.Seconds() log.Infof("hashed %s in %.1fs (%s/s)", - humanize.IBytes(uint64(hashedBytes)), totalDuration.Seconds(), humanize.IBytes(uint64(hashRate))) + humanize.IBytes(safeUint64(hasher.hashedBytes)), + totalDuration.Seconds(), humanize.IBytes(safeRateUint64(hashRate))) } - log.Infof("wrote %d files to %s", len(entries), manifestPath) + + log.Infof("wrote %d files to %s", len(scanner.entries), manifestPath) return nil } // hashFile reads a file and computes its SHA256 multihash. // Progress callback is called with bytes read so far. -func hashFile(r io.Reader, size int64, progress func(int64)) ([]byte, int64, error) { +func hashFile(r io.Reader, progress func(int64)) ([]byte, int64, error) { h := sha256.New() - buf := make([]byte, 64*1024) + buf := make([]byte, hashBufSize) + var total int64 for { n, err := r.Read(buf) if n > 0 { h.Write(buf[:n]) + total += int64(n) if progress != nil { progress(total) } } + if err == io.EOF { break } + + // Returned unwrapped: the caller renders this as + // "failed to hash : " and adding a second layer here + // would change that message. if err != nil { return nil, total, err } @@ -375,15 +589,29 @@ func hashFile(r io.Reader, size int64, progress func(int64)) ([]byte, int64, err } // addFileToBuilder adds a new file entry to the builder -func addFileToBuilder(b *mfer.Builder, path string, size int64, mtime time.Time, hash []byte) error { - return b.AddFileWithHash(mfer.RelFilePath(path), mfer.FileSize(size), mfer.ModTime(mtime), hash) +func addFileToBuilder( + b *mfer.Builder, path string, size int64, mtime time.Time, hash []byte, +) error { + return b.AddFileWithHash( + mfer.RelFilePath(path), mfer.FileSize(size), mfer.ModTime(mtime), hash) } -// addExistingToBuilder adds an existing manifest entry to the builder +// addExistingToBuilder adds an existing manifest entry to the builder. +// +// Entries reach this path only when recordEntry classified them as +// unchanged, which requires a recorded mtime, so an absent mtime here is +// an error rather than something to paper over with the Unix epoch. func addExistingToBuilder(b *mfer.Builder, entry *mfer.MFFilePath) error { - mtime := time.Unix(entry.Mtime.Seconds, int64(entry.Mtime.Nanos)) - if len(entry.Hashes) == 0 { + mtime, ok := entryMtime(entry) + if !ok { + return fmt.Errorf("%w: %s", errEntryMissingMtime, entry.GetPath()) + } + + if len(entry.GetHashes()) == 0 { return nil } - return b.AddFileWithHash(mfer.RelFilePath(entry.Path), mfer.FileSize(entry.Size), mfer.ModTime(mtime), entry.Hashes[0].MultiHash) + + return b.AddFileWithHash(mfer.RelFilePath(entry.GetPath()), + mfer.FileSize(entry.GetSize()), mfer.ModTime(mtime), + entry.GetHashes()[0].GetMultiHash()) } diff --git a/internal/cli/freshen_test.go b/internal/cli/freshen_test.go index 2a18cb0..e5d844d 100644 --- a/internal/cli/freshen_test.go +++ b/internal/cli/freshen_test.go @@ -1,9 +1,12 @@ +//nolint:testpackage // white-box tests exercise unexported internals package cli import ( "bytes" "context" + "os" "testing" + "time" "github.com/spf13/afero" "github.com/stretchr/testify/assert" @@ -11,24 +14,48 @@ import ( "sneak.berlin/go/mfer/mfer" ) -func TestFreshenUnchanged(t *testing.T) { - // Create filesystem with test files - fs := afero.NewMemMapFs() +// stubFileInfo is a minimal fs.FileInfo for exercising recordEntry +// without touching a filesystem. +type stubFileInfo struct { + size int64 + mtime time.Time +} - require.NoError(t, fs.MkdirAll("/testdir", 0o755)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("content1"), 0o644)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file2.txt", []byte("content2"), 0o644)) +func (s stubFileInfo) Name() string { return "stub" } +func (s stubFileInfo) Size() int64 { return s.size } +func (s stubFileInfo) Mode() os.FileMode { return 0 } +func (s stubFileInfo) ModTime() time.Time { return s.mtime } +func (s stubFileInfo) IsDir() bool { return false } +func (s stubFileInfo) Sys() any { return nil } + +// setupFreshenDir populates /testdir with two files, scans it, and +// writes the resulting manifest to /testdir/.index.mf. +func setupFreshenDir(t *testing.T, fs afero.Fs) { + t.Helper() + + require.NoError(t, fs.MkdirAll(testDir, 0o755)) + writeTestFile(t, fs, testFile1, "content1") + writeTestFile(t, fs, "/testdir/file2.txt", "content2") // Generate initial manifest opts := &mfer.ScannerOptions{Fs: fs} s := mfer.NewScannerWithOptions(opts) - require.NoError(t, s.EnumeratePath("/testdir", nil)) + require.NoError(t, s.EnumeratePath(testDir, nil)) var manifestBuf bytes.Buffer + require.NoError(t, s.ToManifest(context.Background(), &manifestBuf, nil)) // Write manifest to filesystem - require.NoError(t, afero.WriteFile(fs, "/testdir/.index.mf", manifestBuf.Bytes(), 0o644)) + require.NoError(t, + afero.WriteFile(fs, "/testdir/.index.mf", manifestBuf.Bytes(), 0o644)) +} + +func TestFreshenUnchanged(t *testing.T) { + t.Parallel() + + fs := afero.NewMemMapFs() + setupFreshenDir(t, fs) // Parse manifest to verify manifest, err := mfer.NewManifestFromFile(fs, "/testdir/.index.mf") @@ -37,23 +64,10 @@ func TestFreshenUnchanged(t *testing.T) { } func TestFreshenWithChanges(t *testing.T) { - // Create filesystem with test files + t.Parallel() + fs := afero.NewMemMapFs() - - require.NoError(t, fs.MkdirAll("/testdir", 0o755)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("content1"), 0o644)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file2.txt", []byte("content2"), 0o644)) - - // Generate initial manifest - opts := &mfer.ScannerOptions{Fs: fs} - s := mfer.NewScannerWithOptions(opts) - require.NoError(t, s.EnumeratePath("/testdir", nil)) - - var manifestBuf bytes.Buffer - require.NoError(t, s.ToManifest(context.Background(), &manifestBuf, nil)) - - // Write manifest to filesystem - require.NoError(t, afero.WriteFile(fs, "/testdir/.index.mf", manifestBuf.Bytes(), 0o644)) + setupFreshenDir(t, fs) // Verify initial manifest has 2 files manifest, err := mfer.NewManifestFromFile(fs, "/testdir/.index.mf") @@ -61,17 +75,17 @@ func TestFreshenWithChanges(t *testing.T) { assert.Len(t, manifest.Files(), 2) // Add a new file - require.NoError(t, afero.WriteFile(fs, "/testdir/file3.txt", []byte("content3"), 0o644)) + writeTestFile(t, fs, "/testdir/file3.txt", "content3") // Modify file2 (change content and size) - require.NoError(t, afero.WriteFile(fs, "/testdir/file2.txt", []byte("modified content2"), 0o644)) + writeTestFile(t, fs, "/testdir/file2.txt", "modified content2") // Remove file1 - require.NoError(t, fs.Remove("/testdir/file1.txt")) + require.NoError(t, fs.Remove(testFile1)) // Note: The freshen operation would need to be run here // For now, we just verify the test setup is correct - exists, _ := afero.Exists(fs, "/testdir/file1.txt") + exists, _ := afero.Exists(fs, testFile1) assert.False(t, exists) exists, _ = afero.Exists(fs, "/testdir/file3.txt") @@ -80,3 +94,104 @@ func TestFreshenWithChanges(t *testing.T) { content, _ := afero.ReadFile(fs, "/testdir/file2.txt") assert.Equal(t, "modified content2", string(content)) } + +// TestFreshenRecordEntryMtimePresence pins the behavior of recordEntry +// with respect to MFFilePath.Mtime, which is a message pointer with +// proto3 field presence and may legitimately be absent. +// +// An absent mtime must never be read as time.Unix(0, 0): that value +// never equals a real modification time, so every entry would be +// classified as changed, re-hashed, and the manifest rewritten +// unconditionally - the exact inverse of what freshen is for, and +// silent. An entry with no mtime is therefore "changed" because it +// cannot be compared, not because it looks like it dates from 1970. +func TestFreshenRecordEntryMtimePresence(t *testing.T) { + t.Parallel() + + const relPath = "file1.txt" + + mtime := time.Unix(1_700_000_000, 0) + info := stubFileInfo{size: 8, mtime: mtime} + + for _, tc := range []struct { + name string + entry *mfer.MFFilePath + needsHash bool + changed int64 + unchanged int64 + }{ + { + name: "matching mtime and size is unchanged", + entry: &mfer.MFFilePath{ + Path: relPath, + Size: 8, + Mtime: &mfer.Timestamp{Seconds: mtime.Unix()}, + }, + needsHash: false, + changed: 0, + unchanged: 1, + }, + { + name: "absent mtime is changed, not epoch", + entry: &mfer.MFFilePath{ + Path: relPath, + Size: 8, + Mtime: nil, + }, + needsHash: true, + changed: 1, + unchanged: 0, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + s := &freshenScanner{ + existingByPath: map[string]*mfer.MFFilePath{relPath: tc.entry}, + } + s.recordEntry(relPath, info) + + require.Len(t, s.entries, 1) + assert.Equal(t, tc.needsHash, s.entries[0].needsHash) + assert.Equal(t, tc.changed, s.changed) + assert.Equal(t, tc.unchanged, s.unchanged) + assert.Zero(t, s.added) + }) + } +} + +// TestFreshenAddExistingRejectsMissingMtime pins that an entry with no +// mtime is never carried forward into a rebuilt manifest with a +// fabricated epoch timestamp. +func TestFreshenAddExistingRejectsMissingMtime(t *testing.T) { + t.Parallel() + + b := mfer.NewBuilder() + entry := &mfer.MFFilePath{ + Path: "file1.txt", + Size: 8, + Mtime: nil, + Hashes: []*mfer.MFFileChecksum{ + {MultiHash: []byte{0x12, 0x20}}, + }, + } + + err := addExistingToBuilder(b, entry) + require.ErrorIs(t, err, errEntryMissingMtime) + assert.Contains(t, err.Error(), "file1.txt") +} + +// TestEntryMtime pins the presence semantics the callers depend on. +func TestEntryMtime(t *testing.T) { + t.Parallel() + + got, ok := entryMtime(&mfer.MFFilePath{Mtime: nil}) + assert.False(t, ok) + assert.True(t, got.IsZero()) + + got, ok = entryMtime(&mfer.MFFilePath{ + Mtime: &mfer.Timestamp{Seconds: 1_700_000_000, Nanos: 500}, + }) + assert.True(t, ok) + assert.Equal(t, time.Unix(1_700_000_000, 500), got) +} diff --git a/internal/cli/gen.go b/internal/cli/gen.go index f424a6f..10da3da 100644 --- a/internal/cli/gen.go +++ b/internal/cli/gen.go @@ -1,6 +1,7 @@ package cli import ( + "errors" "fmt" "os" "os/signal" @@ -16,9 +17,78 @@ import ( "sneak.berlin/go/mfer/mfer" ) -func (mfa *CLIApp) generateManifestOperation(ctx *cli.Context) error { - log.Debug("generateManifestOperation()") +var ( + // errPathNotExist indicates an input path that does not exist. + errPathNotExist = errors.New("path does not exist") + // errOutputExists indicates the output file already exists and + // --force was not given. It is wrapped mid-sentence so that the + // rendered message stays exactly as mfer has always printed it. + errOutputExists = errors.New( + "already exists (use --force to overwrite)") +) +// reportEnumProgress renders enumeration progress until the channel +// closes. +func reportEnumProgress(progress <-chan mfer.EnumerateStatus, wg *sync.WaitGroup) { + defer wg.Done() + + for status := range progress { + log.Progressf("Enumerating: %d files, %s", + status.FilesFound, + humanize.IBytes(safeUint64(int64(status.BytesFound)))) + } + + log.ProgressDone() +} + +// reportScanProgress renders scan progress until the channel closes. +func reportScanProgress(progress <-chan mfer.ScanStatus, wg *sync.WaitGroup) { + defer wg.Done() + + for status := range progress { + if status.ETA > 0 { + log.Progressf("Scanning: %d/%d files, %s/s, ETA %s", + status.ScannedFiles, + status.TotalFiles, + humanize.IBytes(safeRateUint64(status.BytesPerSec)), + status.ETA.Round(time.Second)) + } else { + log.Progressf("Scanning: %d/%d files, %s/s", + status.ScannedFiles, + status.TotalFiles, + humanize.IBytes(safeRateUint64(status.BytesPerSec))) + } + } + + log.ProgressDone() +} + +// collectInputPaths validates the input path arguments and returns them +// as absolute paths. +func (mfa *CLIApp) collectInputPaths(args cli.Args) ([]string, error) { + paths := make([]string, 0, args.Len()) + + for i := range args.Len() { + inputPath := args.Get(i) + + ap, err := filepath.Abs(inputPath) + if err != nil { + return nil, fmt.Errorf("generate: invalid path %q: %w", inputPath, err) + } + // Validate path exists before adding to list + if exists, _ := afero.Exists(mfa.Fs, ap); !exists { + return nil, fmt.Errorf("%w: %s", errPathNotExist, inputPath) + } + + log.Debugf("enumerating path: %s", ap) + paths = append(paths, ap) + } + + return paths, nil +} + +// buildScannerOptions constructs scanner options from the CLI flags. +func (mfa *CLIApp) buildScannerOptions(ctx *cli.Context) *mfer.ScannerOptions { opts := &mfer.ScannerOptions{ IncludeDotfiles: ctx.Bool("include-dotfiles"), FollowSymLinks: ctx.Bool("follow-symlinks"), @@ -29,6 +99,7 @@ func (mfa *CLIApp) generateManifestOperation(ctx *cli.Context) error { // Set seed for deterministic UUID if provided if seed := ctx.String("seed"); seed != "" { opts.Seed = seed + log.Infof("using deterministic seed for manifest UUID") } @@ -40,136 +111,167 @@ func (mfa *CLIApp) generateManifestOperation(ctx *cli.Context) error { log.Infof("signing manifest with GPG key: %s", signKey) } - s := mfer.NewScannerWithOptions(opts) - - // Phase 1: Enumeration - collect paths and stat files - args := ctx.Args() - showProgress := ctx.Bool("progress") - - // Set up enumeration progress reporting - var enumProgress chan mfer.EnumerateStatus - var enumWg sync.WaitGroup - if showProgress { - enumProgress = make(chan mfer.EnumerateStatus, 1) - enumWg.Add(1) - go func() { - defer enumWg.Done() - for status := range enumProgress { - log.Progressf("Enumerating: %d files, %s", - status.FilesFound, - humanize.IBytes(uint64(status.BytesFound))) - } - log.ProgressDone() - }() - } + return opts +} +// enumerateInputs runs the enumeration phase over the argument paths, +// or the current directory when no arguments are given. +func (mfa *CLIApp) enumerateInputs( + s *mfer.Scanner, args cli.Args, enumProgress chan mfer.EnumerateStatus, +) error { if args.Len() == 0 { // Default to current directory - if err := s.EnumeratePath(".", enumProgress); err != nil { - return fmt.Errorf("generate: failed to enumerate current directory: %w", err) - } - } else { - // Collect and validate all paths first - paths := make([]string, 0, args.Len()) - for i := 0; i < args.Len(); i++ { - inputPath := args.Get(i) - ap, err := filepath.Abs(inputPath) - if err != nil { - return fmt.Errorf("generate: invalid path %q: %w", inputPath, err) - } - // Validate path exists before adding to list - if exists, _ := afero.Exists(mfa.Fs, ap); !exists { - return fmt.Errorf("path does not exist: %s", inputPath) - } - log.Debugf("enumerating path: %s", ap) - paths = append(paths, ap) - } - if err := s.EnumeratePaths(enumProgress, paths...); err != nil { - return fmt.Errorf("generate: failed to enumerate paths: %w", err) + err := s.EnumeratePath(".", enumProgress) + if err != nil { + return fmt.Errorf( + "generate: failed to enumerate current directory: %w", err) } + + return nil } + + // Collect and validate all paths first + paths, err := mfa.collectInputPaths(args) + if err != nil { + return err + } + + err = s.EnumeratePaths(enumProgress, paths...) + if err != nil { + return fmt.Errorf("generate: failed to enumerate paths: %w", err) + } + + return nil +} + +// cleanupOnSignal installs a handler that removes the temp output file +// and exits when the process is interrupted. It returns the signal +// channel so the caller can stop and close it when done. +func (mfa *CLIApp) cleanupOnSignal(outFile afero.File, tmpPath string) chan os.Signal { + sigChan := make(chan os.Signal, 1) + + signal.Notify(sigChan, os.Interrupt, syscall.SIGTERM) + + go func() { + sig, ok := <-sigChan + if !ok || sig == nil { + return // Channel closed normally, not a signal + } + + _ = outFile.Close() + _ = mfa.Fs.Remove(tmpPath) + + os.Exit(1) + }() + + return sigChan +} + +// runEnumeratePhase enumerates all input paths with optional progress +// reporting and logs the totals. +func (mfa *CLIApp) runEnumeratePhase(ctx *cli.Context, s *mfer.Scanner) error { + // Set up enumeration progress reporting + var ( + enumProgress chan mfer.EnumerateStatus + enumWg sync.WaitGroup + ) + + if ctx.Bool("progress") { + enumProgress = make(chan mfer.EnumerateStatus, 1) + + enumWg.Add(1) + + go reportEnumProgress(enumProgress, &enumWg) + } + + err := mfa.enumerateInputs(s, ctx.Args(), enumProgress) + if err != nil { + return err + } + enumWg.Wait() - log.Infof("enumerated %d files, %s total", s.FileCount(), humanize.IBytes(uint64(s.TotalBytes()))) + log.Infof("enumerated %d files, %s total", s.FileCount(), + humanize.IBytes(safeUint64(int64(s.TotalBytes())))) + + return nil +} + +func (mfa *CLIApp) generateManifestOperation(ctx *cli.Context) error { + log.Debug("generateManifestOperation()") + + s := mfer.NewScannerWithOptions(mfa.buildScannerOptions(ctx)) + + // Phase 1: Enumeration - collect paths and stat files + err := mfa.runEnumeratePhase(ctx, s) + if err != nil { + return err + } + + showProgress := ctx.Bool("progress") // Check if output file exists outputPath := ctx.String("output") - if exists, _ := afero.Exists(mfa.Fs, outputPath); exists { - if !ctx.Bool("force") { - return fmt.Errorf("output file %s already exists (use --force to overwrite)", outputPath) - } + if exists, _ := afero.Exists(mfa.Fs, outputPath); exists && !ctx.Bool("force") { + return fmt.Errorf("output file %s %w", outputPath, errOutputExists) } // Create temp file for atomic write tmpPath := outputPath + ".tmp" + outFile, err := mfa.Fs.Create(tmpPath) if err != nil { return fmt.Errorf("failed to create temp file: %w", err) } // Set up signal handler to clean up temp file on Ctrl-C - sigChan := make(chan os.Signal, 1) - signal.Notify(sigChan, os.Interrupt, syscall.SIGTERM) - go func() { - sig, ok := <-sigChan - if !ok || sig == nil { - return // Channel closed normally, not a signal - } - _ = outFile.Close() - _ = mfa.Fs.Remove(tmpPath) - os.Exit(1) - }() + sigChan := mfa.cleanupOnSignal(outFile, tmpPath) // Clean up temp file on any error or interruption success := false + defer func() { signal.Stop(sigChan) close(sigChan) + _ = outFile.Close() + if !success { _ = mfa.Fs.Remove(tmpPath) } }() // Phase 2: Scan - read file contents and generate manifest - var scanProgress chan mfer.ScanStatus - var scanWg sync.WaitGroup + var ( + scanProgress chan mfer.ScanStatus + scanWg sync.WaitGroup + ) + if showProgress { scanProgress = make(chan mfer.ScanStatus, 1) + scanWg.Add(1) - go func() { - defer scanWg.Done() - for status := range scanProgress { - if status.ETA > 0 { - log.Progressf("Scanning: %d/%d files, %s/s, ETA %s", - status.ScannedFiles, - status.TotalFiles, - humanize.IBytes(uint64(status.BytesPerSec)), - status.ETA.Round(time.Second)) - } else { - log.Progressf("Scanning: %d/%d files, %s/s", - status.ScannedFiles, - status.TotalFiles, - humanize.IBytes(uint64(status.BytesPerSec))) - } - } - log.ProgressDone() - }() + + go reportScanProgress(scanProgress, &scanWg) } err = s.ToManifest(ctx.Context, outFile, scanProgress) + scanWg.Wait() + if err != nil { return fmt.Errorf("failed to generate manifest: %w", err) } // Close file before rename to ensure all data is flushed - if err := outFile.Close(); err != nil { + err = outFile.Close() + if err != nil { return fmt.Errorf("failed to close temp file: %w", err) } // Atomic rename - if err := mfa.Fs.Rename(tmpPath, outputPath); err != nil { + err = mfa.Fs.Rename(tmpPath, outputPath) + if err != nil { return fmt.Errorf("failed to rename temp file: %w", err) } @@ -177,7 +279,9 @@ func (mfa *CLIApp) generateManifestOperation(ctx *cli.Context) error { elapsed := time.Since(mfa.startupTime).Seconds() rate := float64(s.TotalBytes()) / elapsed - log.Infof("wrote %d files (%s) to %s in %.1fs (%s/s)", s.FileCount(), humanize.IBytes(uint64(s.TotalBytes())), outputPath, elapsed, humanize.IBytes(uint64(rate))) + log.Infof("wrote %d files (%s) to %s in %.1fs (%s/s)", s.FileCount(), + humanize.IBytes(safeUint64(int64(s.TotalBytes()))), outputPath, elapsed, + humanize.IBytes(safeRateUint64(rate))) return nil } diff --git a/internal/cli/list.go b/internal/cli/list.go index 66031d7..dbf2096 100644 --- a/internal/cli/list.go +++ b/internal/cli/list.go @@ -25,6 +25,7 @@ func (mfa *CLIApp) listManifestOperation(ctx *cli.Context) error { if err != nil { return fmt.Errorf("list: %w", err) } + defer func() { _ = rc.Close() }() manifest, err := mfer.NewManifestFromReader(rc) @@ -42,10 +43,17 @@ func (mfa *CLIApp) listManifestOperation(ctx *cli.Context) error { for _, f := range files { if longFormat { - mtime := time.Unix(f.Mtime.Seconds, int64(f.Mtime.Nanos)) - _, _ = fmt.Fprintf(mfa.Stdout, "%d\t%s\t%s%s", f.Size, mtime.Format(time.RFC3339), f.Path, lineEnd) + // An entry may legitimately carry no mtime; render that as + // mtimeAbsent rather than as the Unix epoch. + mtimeStr := mtimeAbsent + if mtime, ok := entryMtime(f); ok { + mtimeStr = mtime.Format(time.RFC3339) + } + + _, _ = fmt.Fprintf(mfa.Stdout, "%d\t%s\t%s%s", + f.GetSize(), mtimeStr, f.GetPath(), lineEnd) } else { - _, _ = fmt.Fprintf(mfa.Stdout, "%s%s", f.Path, lineEnd) + _, _ = fmt.Fprintf(mfa.Stdout, "%s%s", f.GetPath(), lineEnd) } } diff --git a/internal/cli/manifest_loader.go b/internal/cli/manifest_loader.go index 333ac38..a08e8ce 100644 --- a/internal/cli/manifest_loader.go +++ b/internal/cli/manifest_loader.go @@ -1,6 +1,8 @@ package cli import ( + "context" + "errors" "fmt" "io" "net/http" @@ -10,6 +12,17 @@ import ( "github.com/urfave/cli/v2" ) +// manifestFetchTimeout bounds HTTP requests made to fetch a manifest. +const manifestFetchTimeout = 30 * time.Second + +// errHTTPStatus indicates an HTTP response with a non-OK status code. +// +// Its text is the literal "HTTP" prefix of the rendered "HTTP " +// message that mfer has always printed, so that wrapping it does not +// change any user-visible output. Match it with errors.Is; do not read +// its message. +var errHTTPStatus = errors.New("HTTP") + // isHTTPURL returns true if the string starts with http:// or https://. func isHTTPURL(s string) bool { return strings.HasPrefix(s, "http://") || strings.HasPrefix(s, "https://") @@ -19,21 +32,35 @@ func isHTTPURL(s string) bool { // The caller must close the returned reader. func (mfa *CLIApp) openManifestReader(pathOrURL string) (io.ReadCloser, error) { if isHTTPURL(pathOrURL) { - client := &http.Client{Timeout: 30 * time.Second} - resp, err := client.Get(pathOrURL) //nolint:gosec // user-provided URL is intentional + client := &http.Client{Timeout: manifestFetchTimeout} + + req, err := http.NewRequestWithContext( + context.Background(), http.MethodGet, pathOrURL, nil, + ) if err != nil { return nil, fmt.Errorf("failed to fetch %s: %w", pathOrURL, err) } + + resp, err := client.Do(req) + if err != nil { + return nil, fmt.Errorf("failed to fetch %s: %w", pathOrURL, err) + } + if resp.StatusCode != http.StatusOK { _ = resp.Body.Close() - return nil, fmt.Errorf("failed to fetch %s: HTTP %d", pathOrURL, resp.StatusCode) + + return nil, fmt.Errorf("failed to fetch %s: %w %d", + pathOrURL, errHTTPStatus, resp.StatusCode) } + return resp.Body, nil } + f, err := mfa.Fs.Open(pathOrURL) if err != nil { return nil, err } + return f, nil } @@ -46,11 +73,14 @@ func (mfa *CLIApp) resolveManifestArg(ctx *cli.Context) (string, error) { if isHTTPURL(arg) { return arg, nil } + info, statErr := mfa.Fs.Stat(arg) if statErr == nil && info.IsDir() { return findManifest(mfa.Fs, arg) } + return arg, nil } + return findManifest(mfa.Fs, ".") } diff --git a/internal/cli/mfer.go b/internal/cli/mfer.go index 0ef7dbf..e9f2e26 100644 --- a/internal/cli/mfer.go +++ b/internal/cli/mfer.go @@ -1,6 +1,7 @@ package cli import ( + "errors" "fmt" "io" "os" @@ -12,8 +13,24 @@ import ( "sneak.berlin/go/mfer/mfer" ) +// Command and flag names shared across command definitions and tests. +const ( + cmdGenerate = "generate" + cmdCheck = "check" + cmdExport = "export" + + flagProgress = "progress" + + manifestArgsUsage = "[manifest file]" +) + +// errUnknownCommand indicates an unrecognized command argument. +var errUnknownCommand = errors.New("unknown command") + // CLIApp is the main CLI application container. It holds configuration, // I/O streams, and filesystem abstraction to enable testing and flexibility. +// +//nolint:revive // established name used throughout the codebase and tests type CLIApp struct { appname string version string @@ -41,29 +58,34 @@ const banner = ` \ \:\ \ \:\ \ \::/ \ \:\ \__\/ \__\/ \__\/ \__\/` -func (mfa *CLIApp) printBanner() { - if log.GetLevel() <= log.InfoLevel { - _, _ = fmt.Fprintln(mfa.Stdout, banner) - _, _ = fmt.Fprintf(mfa.Stdout, " mfer by @sneak: v%s released %s\n", mfer.Version, mfer.ReleaseDate) - _, _ = fmt.Fprintln(mfa.Stdout, " https://sneak.berlin/go/mfer") - } -} - // VersionString returns the version and git revision formatted for display. func (mfa *CLIApp) VersionString() string { if mfa.gitrev != "" { return fmt.Sprintf("%s (%s)", mfer.Version, mfa.gitrev) } + return mfer.Version } +func (mfa *CLIApp) printBanner() { + if log.GetLevel() <= log.InfoLevel { + _, _ = fmt.Fprintln(mfa.Stdout, banner) + _, _ = fmt.Fprintf(mfa.Stdout, + " mfer by @sneak: v%s released %s\n", + mfer.Version, mfer.ReleaseDate) + _, _ = fmt.Fprintln(mfa.Stdout, " https://sneak.berlin/go/mfer") + } +} + func (mfa *CLIApp) setVerbosity(c *cli.Context) { _, present := os.LookupEnv("MFER_DEBUG") - if present { + + switch { + case present: log.EnableDebugLogging() - } else if c.Bool("quiet") { + case c.Bool("quiet"): log.SetLevel(log.ErrorLevel) - } else { + default: log.SetLevelFromVerbosity(c.Count("verbose")) } } @@ -85,10 +107,215 @@ func commonFlags() []cli.Flag { } } +func (mfa *CLIApp) generateCommand() *cli.Command { + return &cli.Command{ + Name: cmdGenerate, + Aliases: []string{"gen"}, + Usage: "Generate manifest file", + Action: func(c *cli.Context) error { + mfa.setVerbosity(c) + mfa.printBanner() + + return mfa.generateManifestOperation(c) + }, + Flags: append(commonFlags(), + &cli.BoolFlag{ + Name: "follow-symlinks", + Aliases: []string{"L"}, + Usage: "Resolve encountered symlinks", + }, + &cli.BoolFlag{ + Name: "include-dotfiles", + Aliases: []string{"IncludeDotfiles"}, + + Usage: "Include dot (hidden) files (excluded by default)", + }, + &cli.StringFlag{ + Name: "output", + Value: "./.index.mf", + Aliases: []string{"o"}, + Usage: "Specify output filename", + }, + &cli.BoolFlag{ + Name: "force", + Aliases: []string{"f"}, + Usage: "Overwrite output file if it exists", + }, + &cli.BoolFlag{ + Name: flagProgress, + Aliases: []string{"P"}, + Usage: "Show progress during enumeration and scanning", + }, + &cli.StringFlag{ + Name: "sign-key", + Aliases: []string{"s"}, + Usage: "GPG key ID to sign the manifest with", + EnvVars: []string{"MFER_SIGN_KEY"}, + }, + &cli.StringFlag{ + Name: "seed", + Usage: "Seed value for deterministic manifest UUID", + EnvVars: []string{"MFER_SEED"}, + }, + &cli.BoolFlag{ + Name: "include-timestamps", + Usage: "Include createdAt timestamp in manifest " + + "(omitted by default for determinism)", + }, + ), + } +} + +func (mfa *CLIApp) checkCommand() *cli.Command { + return &cli.Command{ + Name: cmdCheck, + Usage: "Validate files using manifest file", + ArgsUsage: manifestArgsUsage, + Action: func(c *cli.Context) error { + mfa.setVerbosity(c) + mfa.printBanner() + + return mfa.checkManifestOperation(c) + }, + Flags: append(commonFlags(), + &cli.StringFlag{ + Name: "base", + Aliases: []string{"b"}, + Value: ".", + Usage: "Base directory for resolving relative paths from manifest", + }, + &cli.BoolFlag{ + Name: flagProgress, + Aliases: []string{"P"}, + Usage: "Show progress during checking", + }, + &cli.BoolFlag{ + Name: "no-extra-files", + Usage: "Fail if files exist in base directory that are not in manifest", + }, + &cli.StringFlag{ + Name: "require-signature", + Aliases: []string{"S"}, + Usage: "Require manifest to be signed by the specified GPG key ID", + EnvVars: []string{"MFER_REQUIRE_SIGNATURE"}, + }, + ), + } +} + +func (mfa *CLIApp) freshenCommand() *cli.Command { + return &cli.Command{ + Name: "freshen", + Usage: "Update manifest with changed, new, and removed files", + ArgsUsage: manifestArgsUsage, + Action: func(c *cli.Context) error { + mfa.setVerbosity(c) + mfa.printBanner() + + return mfa.freshenManifestOperation(c) + }, + Flags: append(commonFlags(), + &cli.StringFlag{ + Name: "base", + Aliases: []string{"b"}, + Value: ".", + Usage: "Base directory for resolving relative paths", + }, + &cli.BoolFlag{ + Name: "follow-symlinks", + Aliases: []string{"L"}, + Usage: "Resolve encountered symlinks", + }, + &cli.BoolFlag{ + Name: "include-dotfiles", + Aliases: []string{"IncludeDotfiles"}, + + Usage: "Include dot (hidden) files (excluded by default)", + }, + &cli.BoolFlag{ + Name: flagProgress, + Aliases: []string{"P"}, + Usage: "Show progress during scanning and hashing", + }, + &cli.StringFlag{ + Name: "sign-key", + Aliases: []string{"s"}, + Usage: "GPG key ID to sign the manifest with", + EnvVars: []string{"MFER_SIGN_KEY"}, + }, + &cli.BoolFlag{ + Name: "include-timestamps", + Usage: "Include createdAt timestamp in manifest " + + "(omitted by default for determinism)", + }, + ), + } +} + +func (mfa *CLIApp) exportCommand() *cli.Command { + return &cli.Command{ + Name: cmdExport, + Usage: "Export manifest contents as JSON", + ArgsUsage: "[manifest file or URL]", + Action: func(c *cli.Context) error { + return mfa.exportManifestOperation(c) + }, + } +} + +func (mfa *CLIApp) versionCommand() *cli.Command { + return &cli.Command{ + Name: "version", + Usage: "Show version", + Action: func(_ *cli.Context) error { + _, _ = fmt.Fprintln(mfa.Stdout, mfa.VersionString()) + + return nil + }, + } +} + +func (mfa *CLIApp) listCommand() *cli.Command { + return &cli.Command{ + Name: "list", + Aliases: []string{"ls"}, + Usage: "List files in manifest", + ArgsUsage: manifestArgsUsage, + Action: func(c *cli.Context) error { + return mfa.listManifestOperation(c) + }, + Flags: []cli.Flag{ + &cli.BoolFlag{ + Name: "long", + Aliases: []string{"l"}, + Usage: "Show size and mtime", + }, + &cli.BoolFlag{ + Name: "print0", + Usage: "Separate entries with NUL character (for xargs -0)", + }, + }, + } +} + +func (mfa *CLIApp) fetchCommand() *cli.Command { + return &cli.Command{ + Name: "fetch", + Usage: "fetch manifest and referenced files", + Action: func(c *cli.Context) error { + mfa.setVerbosity(c) + mfa.printBanner() + + return mfa.fetchManifestOperation(c) + }, + Flags: commonFlags(), + } +} + func (mfa *CLIApp) run(args []string) { mfa.startupTime = time.Now() - if NO_COLOR { + if NoColor { // shoutout to rob pike who thinks it's juvenile log.DisableStyling() } @@ -106,196 +333,30 @@ func (mfa *CLIApp) run(args []string) { ErrWriter: mfa.Stderr, Action: func(c *cli.Context) error { if c.Args().Len() > 0 { - return fmt.Errorf("unknown command %q", c.Args().First()) + return fmt.Errorf("%w %q", errUnknownCommand, c.Args().First()) } + mfa.printBanner() + return cli.ShowAppHelp(c) }, Commands: []*cli.Command{ - { - Name: "generate", - Aliases: []string{"gen"}, - Usage: "Generate manifest file", - Action: func(c *cli.Context) error { - mfa.setVerbosity(c) - mfa.printBanner() - return mfa.generateManifestOperation(c) - }, - Flags: append(commonFlags(), - &cli.BoolFlag{ - Name: "follow-symlinks", - Aliases: []string{"L"}, - Usage: "Resolve encountered symlinks", - }, - &cli.BoolFlag{ - Name: "include-dotfiles", - Aliases: []string{"IncludeDotfiles"}, - - Usage: "Include dot (hidden) files (excluded by default)", - }, - &cli.StringFlag{ - Name: "output", - Value: "./.index.mf", - Aliases: []string{"o"}, - Usage: "Specify output filename", - }, - &cli.BoolFlag{ - Name: "force", - Aliases: []string{"f"}, - Usage: "Overwrite output file if it exists", - }, - &cli.BoolFlag{ - Name: "progress", - Aliases: []string{"P"}, - Usage: "Show progress during enumeration and scanning", - }, - &cli.StringFlag{ - Name: "sign-key", - Aliases: []string{"s"}, - Usage: "GPG key ID to sign the manifest with", - EnvVars: []string{"MFER_SIGN_KEY"}, - }, - &cli.StringFlag{ - Name: "seed", - Usage: "Seed value for deterministic manifest UUID", - EnvVars: []string{"MFER_SEED"}, - }, - &cli.BoolFlag{ - Name: "include-timestamps", - Usage: "Include createdAt timestamp in manifest (omitted by default for determinism)", - }, - ), - }, - { - Name: "check", - Usage: "Validate files using manifest file", - ArgsUsage: "[manifest file]", - Action: func(c *cli.Context) error { - mfa.setVerbosity(c) - mfa.printBanner() - return mfa.checkManifestOperation(c) - }, - Flags: append(commonFlags(), - &cli.StringFlag{ - Name: "base", - Aliases: []string{"b"}, - Value: ".", - Usage: "Base directory for resolving relative paths from manifest", - }, - &cli.BoolFlag{ - Name: "progress", - Aliases: []string{"P"}, - Usage: "Show progress during checking", - }, - &cli.BoolFlag{ - Name: "no-extra-files", - Usage: "Fail if files exist in base directory that are not in manifest", - }, - &cli.StringFlag{ - Name: "require-signature", - Aliases: []string{"S"}, - Usage: "Require manifest to be signed by the specified GPG key ID", - EnvVars: []string{"MFER_REQUIRE_SIGNATURE"}, - }, - ), - }, - { - Name: "freshen", - Usage: "Update manifest with changed, new, and removed files", - ArgsUsage: "[manifest file]", - Action: func(c *cli.Context) error { - mfa.setVerbosity(c) - mfa.printBanner() - return mfa.freshenManifestOperation(c) - }, - Flags: append(commonFlags(), - &cli.StringFlag{ - Name: "base", - Aliases: []string{"b"}, - Value: ".", - Usage: "Base directory for resolving relative paths", - }, - &cli.BoolFlag{ - Name: "follow-symlinks", - Aliases: []string{"L"}, - Usage: "Resolve encountered symlinks", - }, - &cli.BoolFlag{ - Name: "include-dotfiles", - Aliases: []string{"IncludeDotfiles"}, - - Usage: "Include dot (hidden) files (excluded by default)", - }, - &cli.BoolFlag{ - Name: "progress", - Aliases: []string{"P"}, - Usage: "Show progress during scanning and hashing", - }, - &cli.StringFlag{ - Name: "sign-key", - Aliases: []string{"s"}, - Usage: "GPG key ID to sign the manifest with", - EnvVars: []string{"MFER_SIGN_KEY"}, - }, - &cli.BoolFlag{ - Name: "include-timestamps", - Usage: "Include createdAt timestamp in manifest (omitted by default for determinism)", - }, - ), - }, - { - Name: "export", - Usage: "Export manifest contents as JSON", - ArgsUsage: "[manifest file or URL]", - Action: func(c *cli.Context) error { - return mfa.exportManifestOperation(c) - }, - }, - { - Name: "version", - Usage: "Show version", - Action: func(c *cli.Context) error { - _, _ = fmt.Fprintln(mfa.Stdout, mfa.VersionString()) - return nil - }, - }, - { - Name: "list", - Aliases: []string{"ls"}, - Usage: "List files in manifest", - ArgsUsage: "[manifest file]", - Action: func(c *cli.Context) error { - return mfa.listManifestOperation(c) - }, - Flags: []cli.Flag{ - &cli.BoolFlag{ - Name: "long", - Aliases: []string{"l"}, - Usage: "Show size and mtime", - }, - &cli.BoolFlag{ - Name: "print0", - Usage: "Separate entries with NUL character (for xargs -0)", - }, - }, - }, - { - Name: "fetch", - Usage: "fetch manifest and referenced files", - Action: func(c *cli.Context) error { - mfa.setVerbosity(c) - mfa.printBanner() - return mfa.fetchManifestOperation(c) - }, - Flags: commonFlags(), - }, + mfa.generateCommand(), + mfa.checkCommand(), + mfa.freshenCommand(), + mfa.exportCommand(), + mfa.versionCommand(), + mfa.listCommand(), + mfa.fetchCommand(), }, } mfa.app.HideVersion = false + err := mfa.app.Run(args) if err != nil { mfa.exitCode = 1 + log.WithError(err).Debugf("exiting") } } diff --git a/internal/cli/mtime.go b/internal/cli/mtime.go new file mode 100644 index 0000000..fd72145 --- /dev/null +++ b/internal/cli/mtime.go @@ -0,0 +1,28 @@ +package cli + +import ( + "time" + + "sneak.berlin/go/mfer/mfer" +) + +// mtimeAbsent is printed in place of a modification time when a manifest +// entry does not carry one. +const mtimeAbsent = "-" + +// entryMtime returns the modification time recorded for a manifest entry. +// +// MFFilePath.Mtime is a message pointer with proto3 field presence, so an +// absent mtime is a representable, on-the-wire-valid state. It must never +// be conflated with a recorded mtime of the Unix epoch: callers that +// compare mtimes have to treat "absent" as "unknown", not as +// 1970-01-01T00:00:00Z, or every entry compares as modified. ok reports +// whether an mtime was actually recorded. +func entryMtime(entry *mfer.MFFilePath) (time.Time, bool) { + ts := entry.GetMtime() + if ts == nil { + return time.Time{}, false + } + + return time.Unix(ts.GetSeconds(), int64(ts.GetNanos())), true +} diff --git a/internal/log/log.go b/internal/log/log.go index 0dfb1b8..ec59b69 100644 --- a/internal/log/log.go +++ b/internal/log/log.go @@ -1,3 +1,5 @@ +// Package log provides leveled logging with progress output helpers +// on top of apex/log and pterm. package log import ( @@ -52,6 +54,11 @@ func (l Level) String() string { } } +// callerSkip is the runtime.Caller stack depth from the public Debug +// helpers to the caller of the log package. +const callerSkip = 2 + +//nolint:gochecknoglobals // package-level logger state by design var ( // mu protects the output writers and level mu sync.RWMutex @@ -60,7 +67,7 @@ var ( // stderr is the writer for log output stderr io.Writer = os.Stderr // currentLevel is our log level (includes Verbose) - currentLevel Level = InfoLevel + currentLevel = InfoLevel ) // SetOutput configures the output writers for the log package. @@ -68,8 +75,10 @@ var ( func SetOutput(out, err io.Writer) { mu.Lock() defer mu.Unlock() + stdout = out stderr = err + pterm.SetDefaultOutput(out) } @@ -77,6 +86,7 @@ func SetOutput(out, err io.Writer) { func GetStdout() io.Writer { mu.RLock() defer mu.RUnlock() + return stdout } @@ -84,6 +94,7 @@ func GetStdout() io.Writer { func GetStderr() io.Writer { mu.RLock() defer mu.RUnlock() + return stderr } @@ -91,6 +102,7 @@ func GetStderr() io.Writer { func DisableStyling() { pterm.DisableColor() pterm.DisableStyling() + pterm.Debug.Prefix.Text = "" pterm.Info.Prefix.Text = "" pterm.Success.Prefix.Text = "" @@ -102,7 +114,9 @@ func DisableStyling() { // Init initializes the logger with the CLI handler and default log level. func Init() { mu.RLock() + w := stderr + mu.RUnlock() log.SetHandler(acli.New(w)) log.SetLevel(log.DebugLevel) // Let apex/log pass everything; we filter ourselves @@ -112,11 +126,12 @@ func Init() { func isEnabled(l Level) bool { mu.RLock() defer mu.RUnlock() + return l >= currentLevel } // Fatalf logs a formatted message at fatal level. -func Fatalf(format string, args ...interface{}) { +func Fatalf(format string, args ...any) { if isEnabled(FatalLevel) { log.Fatalf(format, args...) } @@ -130,7 +145,7 @@ func Fatal(arg string) { } // Errorf logs a formatted message at error level. -func Errorf(format string, args ...interface{}) { +func Errorf(format string, args ...any) { if isEnabled(ErrorLevel) { log.Errorf(format, args...) } @@ -144,7 +159,7 @@ func Error(arg string) { } // Warnf logs a formatted message at warn level. -func Warnf(format string, args ...interface{}) { +func Warnf(format string, args ...any) { if isEnabled(WarnLevel) { log.Warnf(format, args...) } @@ -158,7 +173,7 @@ func Warn(arg string) { } // Infof logs a formatted message at info level. -func Infof(format string, args ...interface{}) { +func Infof(format string, args ...any) { if isEnabled(InfoLevel) { log.Infof(format, args...) } @@ -172,7 +187,7 @@ func Info(arg string) { } // Verbosef logs a formatted message at verbose level. -func Verbosef(format string, args ...interface{}) { +func Verbosef(format string, args ...any) { if isEnabled(VerboseLevel) { log.Infof(format, args...) } @@ -186,16 +201,16 @@ func Verbose(arg string) { } // Debugf logs a formatted message at debug level with caller location. -func Debugf(format string, args ...interface{}) { +func Debugf(format string, args ...any) { if isEnabled(DebugLevel) { - DebugReal(fmt.Sprintf(format, args...), 2) + DebugReal(fmt.Sprintf(format, args...), callerSkip) } } // Debug logs a message at debug level with caller location. func Debug(arg string) { if isEnabled(DebugLevel) { - DebugReal(arg, 2) + DebugReal(arg, callerSkip) } } @@ -204,18 +219,20 @@ func DebugReal(arg string, cs int) { if !isEnabled(DebugLevel) { return } + _, callerFile, callerLine, ok := runtime.Caller(cs) if !ok { return } + tag := fmt.Sprintf("%s:%d: ", filepath.Base(callerFile), callerLine) log.Debug(tag + arg) } // Dump logs a spew dump of the arguments at debug level. -func Dump(args ...interface{}) { +func Dump(args ...any) { if isEnabled(DebugLevel) { - DebugReal(spew.Sdump(args...), 2) + DebugReal(spew.Sdump(args...), callerSkip) } } @@ -246,6 +263,7 @@ func SetLevelFromVerbosity(l int) { func SetLevel(l Level) { mu.Lock() defer mu.Unlock() + currentLevel = l } @@ -253,6 +271,7 @@ func SetLevel(l Level) { func GetLevel() Level { mu.RLock() defer mu.RUnlock() + return currentLevel } @@ -263,7 +282,7 @@ func WithError(e error) *log.Entry { // Progressf prints a progress message that overwrites the current line. // Use ProgressDone() when progress is complete to move to the next line. -func Progressf(format string, args ...interface{}) { +func Progressf(format string, args ...any) { pterm.Printf("\r"+format, args...) } diff --git a/internal/log/log_test.go b/internal/log/log_test.go index b36c642..37d531a 100644 --- a/internal/log/log_test.go +++ b/internal/log/log_test.go @@ -1,12 +1,12 @@ -package log +package log_test import ( "testing" - "github.com/stretchr/testify/assert" + "sneak.berlin/go/mfer/internal/log" ) func TestBuild(t *testing.T) { - Init() - assert.True(t, true) + t.Parallel() + log.Init() } diff --git a/mfer/builder.go b/mfer/builder.go index 0b2262f..b93f79e 100644 --- a/mfer/builder.go +++ b/mfer/builder.go @@ -1,3 +1,5 @@ +// Package mfer implements the mfer manifest file format: building, +// serializing, verifying, and checking manifests of file trees. package mfer import ( @@ -14,6 +16,27 @@ import ( "github.com/multiformats/go-multihash" ) +// readChunkSize is the buffer size used when reading file contents for +// hashing. +const readChunkSize = 64 * 1024 + +// The errPath* sentinels below are worded as the trailing fragment of the +// message ValidatePath renders, because the offending path is quoted +// before them (`path %q ...`). Wrapping them mid-sentence keeps the +// rendered text exactly as mfer has always printed it. Match them with +// errors.Is rather than by reading their messages. +var ( + errPathEmpty = errors.New("path cannot be empty") + errPathNotUTF8 = errors.New("is not valid UTF-8") + errPathBackslash = errors.New("contains backslash; use forward slashes only") + errPathAbsolute = errors.New("is absolute; must be relative") + errPathEmptySegment = errors.New("contains empty segment") + errPathDotDot = errors.New("contains '..' segment") + errSizeMismatch = errors.New("size mismatch") + errNegativeSize = errors.New("size cannot be negative") + errEmptyHash = errors.New("hash cannot be nil or empty") +) + // ValidatePath checks that a file path conforms to manifest path invariants: // - Must be valid UTF-8 // - Must use forward slashes only (no backslashes) @@ -23,25 +46,31 @@ import ( // - Must not be empty func ValidatePath(p string) error { if p == "" { - return errors.New("path cannot be empty") + return errPathEmpty } + if !utf8.ValidString(p) { - return fmt.Errorf("path %q is not valid UTF-8", p) + return fmt.Errorf("path %q %w", p, errPathNotUTF8) } + if strings.ContainsRune(p, '\\') { - return fmt.Errorf("path %q contains backslash; use forward slashes only", p) + return fmt.Errorf("path %q %w", p, errPathBackslash) } + if strings.HasPrefix(p, "/") { - return fmt.Errorf("path %q is absolute; must be relative", p) + return fmt.Errorf("path %q %w", p, errPathAbsolute) } + for _, seg := range strings.Split(p, "/") { if seg == "" { - return fmt.Errorf("path %q contains empty segment", p) + return fmt.Errorf("path %q %w", p, errPathEmptySegment) } + if seg == ".." { - return fmt.Errorf("path %q contains '..' segment", p) + return fmt.Errorf("path %q %w", p, errPathDotDot) } } + return nil } @@ -68,11 +97,7 @@ type UnixNanos int32 // Timestamp converts ModTime to a protobuf Timestamp. func (m ModTime) Timestamp() *Timestamp { - t := time.Time(m) - return &Timestamp{ - Seconds: t.Unix(), - Nanos: int32(t.Nanosecond()), - } + return newTimestampFromTime(time.Time(m)) } // Multihash represents a multihash-encoded file hash (typically SHA2-256). @@ -93,14 +118,6 @@ type Builder struct { fixedUUID []byte // if set, use this UUID instead of generating one } -// SetSeed derives a deterministic UUID from the given seed string. -// The seed is hashed once with SHA-256 and the first 16 bytes are used -// as a fixed UUID for the manifest. -func (b *Builder) SetSeed(seed string) { - hash := sha256.Sum256([]byte(seed)) - b.fixedUUID = hash[:16] -} - // NewBuilder creates a new Builder. func NewBuilder() *Builder { return &Builder{ @@ -109,6 +126,14 @@ func NewBuilder() *Builder { } } +// SetSeed derives a deterministic UUID from the given seed string. +// The seed is hashed once with SHA-256 and the first 16 bytes are used +// as a fixed UUID for the manifest. +func (b *Builder) SetSeed(seed string) { + hash := sha256.Sum256([]byte(seed)) + b.fixedUUID = hash[:uuidLength] +} + // AddFile reads file content from reader, computes hashes, and adds to manifest. // Progress updates are sent to the progress channel (if non-nil) without blocking. // Returns the number of bytes read. @@ -119,7 +144,8 @@ func (b *Builder) AddFile( reader io.Reader, progress chan<- FileHashProgress, ) (FileSize, error) { - if err := ValidatePath(string(path)); err != nil { + err := ValidatePath(string(path)) + if err != nil { return 0, err } @@ -128,7 +154,8 @@ func (b *Builder) AddFile( // Read file in chunks, updating hash and progress var totalRead FileSize - buf := make([]byte, 64*1024) // 64KB chunks + + buf := make([]byte, readChunkSize) for { n, err := reader.Read(buf) @@ -137,9 +164,11 @@ func (b *Builder) AddFile( totalRead += FileSize(n) sendFileHashProgress(progress, FileHashProgress{BytesRead: totalRead}) } + if err == io.EOF { break } + if err != nil { return totalRead, err } @@ -147,7 +176,10 @@ func (b *Builder) AddFile( // Verify actual bytes read matches declared size if totalRead != size { - return totalRead, fmt.Errorf("size mismatch for %q: declared %d bytes but read %d bytes", path, size, totalRead) + return totalRead, fmt.Errorf( + "%w for %q: declared %d bytes but read %d bytes", + errSizeMismatch, path, size, totalRead, + ) } // Encode hash as multihash (SHA2-256) @@ -178,6 +210,7 @@ func sendFileHashProgress(ch chan<- FileHashProgress, p FileHashProgress) { if ch == nil { return } + select { case ch <- p: default: @@ -188,21 +221,30 @@ func sendFileHashProgress(ch chan<- FileHashProgress, p FileHashProgress) { func (b *Builder) FileCount() int { b.mu.Lock() defer b.mu.Unlock() + return len(b.files) } // AddFileWithHash adds a file entry with a pre-computed hash. // This is useful when the hash is already known (e.g., from an existing manifest). // Returns an error if path is empty, size is negative, or hash is nil/empty. -func (b *Builder) AddFileWithHash(path RelFilePath, size FileSize, mtime ModTime, hash Multihash) error { - if err := ValidatePath(string(path)); err != nil { +func (b *Builder) AddFileWithHash( + path RelFilePath, + size FileSize, + mtime ModTime, + hash Multihash, +) error { + err := ValidatePath(string(path)) + if err != nil { return fmt.Errorf("add file: %w", err) } + if size < 0 { - return errors.New("size cannot be negative") + return errNegativeSize } + if len(hash) == 0 { - return errors.New("hash cannot be nil or empty") + return errEmptyHash } entry := &MFFilePath{ @@ -217,6 +259,7 @@ func (b *Builder) AddFileWithHash(path RelFilePath, size FileSize, mtime ModTime b.mu.Lock() b.files = append(b.files, entry) b.mu.Unlock() + return nil } @@ -225,6 +268,7 @@ func (b *Builder) AddFileWithHash(path RelFilePath, size FileSize, mtime ModTime func (b *Builder) SetIncludeTimestamps(include bool) { b.mu.Lock() defer b.mu.Unlock() + b.includeTimestamps = include } @@ -233,6 +277,7 @@ func (b *Builder) SetIncludeTimestamps(include bool) { func (b *Builder) SetSigningOptions(opts *SigningOptions) { b.mu.Lock() defer b.mu.Unlock() + b.signingOptions = opts } @@ -243,7 +288,7 @@ func (b *Builder) Build(w io.Writer) error { // Sort files by path for deterministic output sort.Slice(b.files, func(i, j int) bool { - return b.files[i].Path < b.files[j].Path + return b.files[i].GetPath() < b.files[j].GetPath() }) // Create inner manifest @@ -263,19 +308,22 @@ func (b *Builder) Build(w io.Writer) error { } // Generate outer wrapper - if err := m.generateOuter(); err != nil { + err := m.generateOuter() + if err != nil { return fmt.Errorf("build: generate outer: %w", err) } // Generate final output - if err := m.generate(); err != nil { + err = m.generate() + if err != nil { return fmt.Errorf("build: generate: %w", err) } // Write to output - _, err := w.Write(m.output.Bytes()) + _, err = w.Write(m.output.Bytes()) if err != nil { return fmt.Errorf("build: write output: %w", err) } + return nil } diff --git a/mfer/builder_test.go b/mfer/builder_test.go index 577106d..7c20e9c 100644 --- a/mfer/builder_test.go +++ b/mfer/builder_test.go @@ -1,3 +1,4 @@ +//nolint:testpackage // white-box tests exercise unexported internals package mfer import ( @@ -10,24 +11,34 @@ import ( "github.com/stretchr/testify/require" ) +const testFileName = "file.txt" + func TestNewBuilder(t *testing.T) { + t.Parallel() + b := NewBuilder() assert.NotNil(t, b) assert.Equal(t, 0, b.FileCount()) } func TestBuilderAddFile(t *testing.T) { + t.Parallel() + b := NewBuilder() content := []byte("test content") reader := bytes.NewReader(content) - bytesRead, err := b.AddFile("test.txt", FileSize(len(content)), ModTime(time.Now()), reader, nil) + bytesRead, err := b.AddFile( + "test.txt", FileSize(len(content)), ModTime(time.Now()), reader, nil, + ) require.NoError(t, err) assert.Equal(t, FileSize(len(content)), bytesRead) assert.Equal(t, 1, b.FileCount()) } func TestBuilderAddFileWithHash(t *testing.T) { + t.Parallel() + b := NewBuilder() hash := make([]byte, 34) // SHA256 multihash is 34 bytes @@ -37,54 +48,71 @@ func TestBuilderAddFileWithHash(t *testing.T) { } func TestBuilderAddFileWithHashValidation(t *testing.T) { + t.Parallel() + t.Run("empty path", func(t *testing.T) { + t.Parallel() + b := NewBuilder() hash := make([]byte, 34) err := b.AddFileWithHash("", 100, ModTime(time.Now()), hash) - assert.Error(t, err) + require.Error(t, err) assert.Contains(t, err.Error(), "path") }) t.Run("negative size", func(t *testing.T) { + t.Parallel() + b := NewBuilder() hash := make([]byte, 34) err := b.AddFileWithHash("test.txt", -1, ModTime(time.Now()), hash) - assert.Error(t, err) + require.Error(t, err) assert.Contains(t, err.Error(), "size") }) t.Run("nil hash", func(t *testing.T) { + t.Parallel() + b := NewBuilder() err := b.AddFileWithHash("test.txt", 100, ModTime(time.Now()), nil) - assert.Error(t, err) + require.Error(t, err) assert.Contains(t, err.Error(), "hash") }) t.Run("empty hash", func(t *testing.T) { + t.Parallel() + b := NewBuilder() err := b.AddFileWithHash("test.txt", 100, ModTime(time.Now()), []byte{}) - assert.Error(t, err) + require.Error(t, err) assert.Contains(t, err.Error(), "hash") }) t.Run("valid inputs", func(t *testing.T) { + t.Parallel() + b := NewBuilder() hash := make([]byte, 34) err := b.AddFileWithHash("test.txt", 100, ModTime(time.Now()), hash) - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, 1, b.FileCount()) }) } func TestBuilderBuild(t *testing.T) { + t.Parallel() + b := NewBuilder() content := []byte("test content") reader := bytes.NewReader(content) - _, err := b.AddFile("test.txt", FileSize(len(content)), ModTime(time.Now()), reader, nil) + _, err := b.AddFile( + "test.txt", FileSize(len(content)), ModTime(time.Now()), reader, nil, + ) require.NoError(t, err) var buf bytes.Buffer + err = b.Build(&buf) require.NoError(t, err) @@ -93,6 +121,8 @@ func TestBuilderBuild(t *testing.T) { } func TestNewTimestampFromTimeExtremeDate(t *testing.T) { + t.Parallel() + // Regression test: newTimestampFromTime used UnixNano() which panics // for dates outside ~1678-2262. Now uses Nanosecond() which is safe. tests := []struct { @@ -107,15 +137,19 @@ func TestNewTimestampFromTimeExtremeDate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { + t.Parallel() + // Should not panic ts := newTimestampFromTime(tt.time) - assert.Equal(t, tt.time.Unix(), ts.Seconds) - assert.Equal(t, int32(tt.time.Nanosecond()), ts.Nanos) + assert.Equal(t, tt.time.Unix(), ts.GetSeconds()) + assert.Equal(t, tt.time.Nanosecond(), int(ts.GetNanos())) }) } } func TestBuilderDeterministicOutput(t *testing.T) { + t.Parallel() + buildManifest := func() []byte { b := NewBuilder() // Use a fixed createdAt and UUID so output is reproducible @@ -135,24 +169,32 @@ func TestBuilderDeterministicOutput(t *testing.T) { } for _, f := range files { r := bytes.NewReader([]byte(f.content)) - _, err := b.AddFile(RelFilePath(f.path), FileSize(len(f.content)), mtime, r, nil) + _, err := b.AddFile( + RelFilePath(f.path), FileSize(len(f.content)), mtime, r, nil, + ) require.NoError(t, err) } var buf bytes.Buffer + err := b.Build(&buf) require.NoError(t, err) + return buf.Bytes() } out1 := buildManifest() out2 := buildManifest() - assert.Equal(t, out1, out2, "two builds with same input should produce byte-identical output") + assert.Equal(t, out1, out2, + "two builds with same input should produce byte-identical output") } func TestSetSeedDeterministic(t *testing.T) { + t.Parallel() + b1 := NewBuilder() b1.SetSeed("test-seed-value") + b2 := NewBuilder() b2.SetSeed("test-seed-value") assert.Equal(t, b1.fixedUUID, b2.fixedUUID, "same seed should produce same UUID") @@ -160,19 +202,24 @@ func TestSetSeedDeterministic(t *testing.T) { b3 := NewBuilder() b3.SetSeed("different-seed") - assert.NotEqual(t, b1.fixedUUID, b3.fixedUUID, "different seeds should produce different UUIDs") + assert.NotEqual(t, b1.fixedUUID, b3.fixedUUID, + "different seeds should produce different UUIDs") } func TestValidatePath(t *testing.T) { + t.Parallel() + valid := []string{ - "file.txt", + testFileName, "dir/file.txt", "a/b/c/d.txt", "file with spaces.txt", - "日本語.txt", + "日本語.txt", //nolint:gosmopolitan // deliberately tests non-ASCII UTF-8 paths } for _, p := range valid { t.Run("valid:"+p, func(t *testing.T) { + t.Parallel() + assert.NoError(t, ValidatePath(p)) }) } @@ -191,42 +238,54 @@ func TestValidatePath(t *testing.T) { } for _, tt := range invalid { t.Run("invalid:"+tt.desc, func(t *testing.T) { + t.Parallel() + assert.Error(t, ValidatePath(tt.path)) }) } } func TestBuilderAddFileSizeMismatch(t *testing.T) { + t.Parallel() + b := NewBuilder() content := []byte("short") reader := bytes.NewReader(content) // Declare wrong size _, err := b.AddFile("test.txt", FileSize(100), ModTime(time.Now()), reader, nil) - assert.Error(t, err) + require.Error(t, err) assert.Contains(t, err.Error(), "size mismatch") } func TestBuilderAddFileInvalidPath(t *testing.T) { + t.Parallel() + b := NewBuilder() content := []byte("data") reader := bytes.NewReader(content) _, err := b.AddFile("", FileSize(len(content)), ModTime(time.Now()), reader, nil) - assert.Error(t, err) + require.Error(t, err) reader.Reset(content) - _, err = b.AddFile("/absolute", FileSize(len(content)), ModTime(time.Now()), reader, nil) + _, err = b.AddFile( + "/absolute", FileSize(len(content)), ModTime(time.Now()), reader, nil, + ) assert.Error(t, err) } func TestBuilderAddFileWithProgress(t *testing.T) { + t.Parallel() + b := NewBuilder() content := bytes.Repeat([]byte("x"), 1000) reader := bytes.NewReader(content) progress := make(chan FileHashProgress, 100) - bytesRead, err := b.AddFile("test.txt", FileSize(len(content)), ModTime(time.Now()), reader, progress) + bytesRead, err := b.AddFile( + "test.txt", FileSize(len(content)), ModTime(time.Now()), reader, progress, + ) close(progress) require.NoError(t, err) assert.Equal(t, FileSize(1000), bytesRead) @@ -235,12 +294,15 @@ func TestBuilderAddFileWithProgress(t *testing.T) { for p := range progress { updates = append(updates, p) } + assert.NotEmpty(t, updates) // Last update should show all bytes assert.Equal(t, FileSize(1000), updates[len(updates)-1].BytesRead) } func TestBuilderBuildRoundTrip(t *testing.T) { + t.Parallel() + // Build a manifest, deserialize it, verify all fields survive round-trip b := NewBuilder() now := time.Date(2025, 6, 15, 12, 0, 0, 0, time.UTC) @@ -256,7 +318,9 @@ func TestBuilderBuildRoundTrip(t *testing.T) { for _, f := range files { reader := bytes.NewReader(f.content) - _, err := b.AddFile(RelFilePath(f.path), FileSize(len(f.content)), ModTime(now), reader, nil) + _, err := b.AddFile( + RelFilePath(f.path), FileSize(len(f.content)), ModTime(now), reader, nil, + ) require.NoError(t, err) } @@ -270,42 +334,52 @@ func TestBuilderBuildRoundTrip(t *testing.T) { require.Len(t, mfiles, 3) // Verify sorted order - assert.Equal(t, "alpha.txt", mfiles[0].Path) - assert.Equal(t, "beta/delta.txt", mfiles[1].Path) - assert.Equal(t, "beta/gamma.txt", mfiles[2].Path) + assert.Equal(t, "alpha.txt", mfiles[0].GetPath()) + assert.Equal(t, "beta/delta.txt", mfiles[1].GetPath()) + assert.Equal(t, "beta/gamma.txt", mfiles[2].GetPath()) // Verify sizes - assert.Equal(t, int64(len("alpha content")), mfiles[0].Size) + assert.Equal(t, int64(len("alpha content")), mfiles[0].GetSize()) // Verify hashes are present for _, f := range mfiles { - require.NotEmpty(t, f.Hashes, "file %s should have hashes", f.Path) - assert.NotEmpty(t, f.Hashes[0].MultiHash) + require.NotEmpty(t, f.GetHashes(), "file %s should have hashes", f.GetPath()) + assert.NotEmpty(t, f.GetHashes()[0].GetMultiHash()) } } func TestNewManifestFromReaderInvalidMagic(t *testing.T) { + t.Parallel() + _, err := NewManifestFromReader(bytes.NewReader([]byte("NOT_VALID"))) - assert.Error(t, err) + require.Error(t, err) assert.Contains(t, err.Error(), "invalid file format") } func TestNewManifestFromReaderEmpty(t *testing.T) { + t.Parallel() + _, err := NewManifestFromReader(bytes.NewReader([]byte{})) assert.Error(t, err) } func TestNewManifestFromReaderTruncated(t *testing.T) { + t.Parallel() + // Just the magic with nothing after _, err := NewManifestFromReader(bytes.NewReader([]byte(MAGIC))) assert.Error(t, err) } func TestManifestString(t *testing.T) { + t.Parallel() + b := NewBuilder() content := []byte("test") reader := bytes.NewReader(content) - _, err := b.AddFile("test.txt", FileSize(len(content)), ModTime(time.Now()), reader, nil) + _, err := b.AddFile( + "test.txt", FileSize(len(content)), ModTime(time.Now()), reader, nil, + ) require.NoError(t, err) var buf bytes.Buffer @@ -317,9 +391,12 @@ func TestManifestString(t *testing.T) { } func TestBuilderBuildEmpty(t *testing.T) { + t.Parallel() + b := NewBuilder() var buf bytes.Buffer + err := b.Build(&buf) require.NoError(t, err) @@ -328,9 +405,14 @@ func TestBuilderBuildEmpty(t *testing.T) { } func TestBuilderOmitsCreatedAtByDefault(t *testing.T) { + t.Parallel() + b := NewBuilder() content := []byte("hello") - _, err := b.AddFile("test.txt", FileSize(len(content)), ModTime(time.Now()), bytes.NewReader(content), nil) + _, err := b.AddFile( + "test.txt", FileSize(len(content)), ModTime(time.Now()), + bytes.NewReader(content), nil, + ) require.NoError(t, err) var buf bytes.Buffer @@ -338,14 +420,21 @@ func TestBuilderOmitsCreatedAtByDefault(t *testing.T) { m, err := NewManifestFromReader(&buf) require.NoError(t, err) - assert.Nil(t, m.pbInner.CreatedAt, "createdAt should be nil by default for deterministic output") + assert.Nil(t, m.pbInner.GetCreatedAt(), + "createdAt should be nil by default for deterministic output") } func TestBuilderIncludesCreatedAtWhenRequested(t *testing.T) { + t.Parallel() + b := NewBuilder() b.SetIncludeTimestamps(true) + content := []byte("hello") - _, err := b.AddFile("test.txt", FileSize(len(content)), ModTime(time.Now()), bytes.NewReader(content), nil) + _, err := b.AddFile( + "test.txt", FileSize(len(content)), ModTime(time.Now()), + bytes.NewReader(content), nil, + ) require.NoError(t, err) var buf bytes.Buffer @@ -353,23 +442,32 @@ func TestBuilderIncludesCreatedAtWhenRequested(t *testing.T) { m, err := NewManifestFromReader(&buf) require.NoError(t, err) - assert.NotNil(t, m.pbInner.CreatedAt, "createdAt should be set when IncludeTimestamps is true") + assert.NotNil(t, m.pbInner.GetCreatedAt(), + "createdAt should be set when IncludeTimestamps is true") } func TestBuilderDeterministicFileOrder(t *testing.T) { + t.Parallel() + // Two builds with same files in different order should produce same file ordering. // Note: UUIDs differ per build, so we compare parsed file lists, not raw bytes. buildAndParse := func(order []string) []*MFFilePath { b := NewBuilder() + for _, name := range order { content := []byte("content of " + name) - _, err := b.AddFile(RelFilePath(name), FileSize(len(content)), ModTime(time.Unix(1000, 0)), bytes.NewReader(content), nil) + _, err := b.AddFile( + RelFilePath(name), FileSize(len(content)), + ModTime(time.Unix(1000, 0)), bytes.NewReader(content), nil, + ) require.NoError(t, err) } + var buf bytes.Buffer require.NoError(t, b.Build(&buf)) m, err := NewManifestFromReader(&buf) require.NoError(t, err) + return m.Files() } @@ -378,10 +476,12 @@ func TestBuilderDeterministicFileOrder(t *testing.T) { require.Len(t, files1, 2) require.Len(t, files2, 2) + for i := range files1 { - assert.Equal(t, files1[i].Path, files2[i].Path) - assert.Equal(t, files1[i].Size, files2[i].Size) + assert.Equal(t, files1[i].GetPath(), files2[i].GetPath()) + assert.Equal(t, files1[i].GetSize(), files2[i].GetSize()) } - assert.Equal(t, "a.txt", files1[0].Path) - assert.Equal(t, "b.txt", files1[1].Path) + + assert.Equal(t, "a.txt", files1[0].GetPath()) + assert.Equal(t, "b.txt", files1[1].GetPath()) } diff --git a/mfer/checker.go b/mfer/checker.go index 35f233c..fe5969e 100644 --- a/mfer/checker.go +++ b/mfer/checker.go @@ -14,6 +14,8 @@ import ( "github.com/spf13/afero" ) +var errNoSigningPubKey = errors.New("manifest has no signing public key") + // Result represents the outcome of checking a single file. type Result struct { Path RelFilePath // Relative path from manifest @@ -24,6 +26,7 @@ type Result struct { // Status represents the verification status of a file. type Status int +// Verification result statuses reported for each checked file. const ( StatusOK Status = iota // File matches manifest (size and hash verified) StatusMissing // File not found on disk @@ -70,7 +73,8 @@ type Checker struct { fs afero.Fs // manifestPaths is a set of paths in the manifest for quick lookup manifestPaths map[RelFilePath]struct{} - // manifestRelPath is the relative path of the manifest file from basePath (for exclusion) + // manifestRelPath is the relative path of the manifest file from + // basePath (for exclusion) manifestRelPath RelFilePath // signature info from the manifest signature []byte @@ -97,9 +101,10 @@ func NewChecker(manifestPath string, basePath string, fs afero.Fs) (*Checker, er } files := m.Files() + manifestPaths := make(map[RelFilePath]struct{}, len(files)) for _, f := range files { - manifestPaths[RelFilePath(f.Path)] = struct{}{} + manifestPaths[RelFilePath(f.GetPath())] = struct{}{} } // Compute manifest's relative path from basePath for exclusion in FindExtraFiles @@ -107,6 +112,7 @@ func NewChecker(manifestPath string, basePath string, fs afero.Fs) (*Checker, er if err != nil { return nil, err } + manifestRel, err := filepath.Rel(abs, absManifest) if err != nil { manifestRel = "" @@ -118,9 +124,9 @@ func NewChecker(manifestPath string, basePath string, fs afero.Fs) (*Checker, er fs: fs, manifestPaths: manifestPaths, manifestRelPath: RelFilePath(manifestRel), - signature: m.pbOuter.Signature, - signer: m.pbOuter.Signer, - signingPubKey: m.pbOuter.SigningPubKey, + signature: m.pbOuter.GetSignature(), + signer: m.pbOuter.GetSigner(), + signingPubKey: m.pbOuter.GetSigningPubKey(), }, nil } @@ -133,8 +139,9 @@ func (c *Checker) FileCount() FileCount { func (c *Checker) TotalBytes() FileSize { var total FileSize for _, f := range c.files { - total += FileSize(f.Size) + total += FileSize(f.GetSize()) } + return total } @@ -148,7 +155,8 @@ func (c *Checker) Signer() []byte { return c.signer } -// SigningPubKey returns the signing public key if the manifest is signed, nil otherwise. +// SigningPubKey returns the signing public key if the manifest is signed, +// nil otherwise. func (c *Checker) SigningPubKey() []byte { return c.signingPubKey } @@ -158,8 +166,9 @@ func (c *Checker) SigningPubKey() []byte { // returns its actual fingerprint from the key material itself. func (c *Checker) ExtractEmbeddedSigningKeyFP() (string, error) { if len(c.signingPubKey) == 0 { - return "", errors.New("manifest has no signing public key") + return "", errNoSigningPubKey } + return gpgExtractPubKeyFingerprint(c.signingPubKey) } @@ -167,10 +176,15 @@ func (c *Checker) ExtractEmbeddedSigningKeyFP() (string, error) { // Results are sent to the results channel as files are checked. // Progress updates are sent to the progress channel approximately once per second. // Both channels are closed when the method returns. -func (c *Checker) Check(ctx context.Context, results chan<- Result, progress chan<- CheckStatus) error { +func (c *Checker) Check( + ctx context.Context, + results chan<- Result, + progress chan<- CheckStatus, +) error { if results != nil { defer close(results) } + if progress != nil { defer close(progress) } @@ -178,9 +192,11 @@ func (c *Checker) Check(ctx context.Context, results chan<- Result, progress cha totalFiles := FileCount(len(c.files)) totalBytes := c.TotalBytes() - var checkedFiles FileCount - var checkedBytes FileSize - var failures FileCount + var ( + checkedFiles FileCount + checkedBytes FileSize + failures FileCount + ) startTime := time.Now() lastProgressTime := time.Now() @@ -196,6 +212,7 @@ func (c *Checker) Check(ctx context.Context, results chan<- Result, progress cha if result.Status != StatusOK { failures++ } + checkedFiles++ if results != nil { @@ -205,19 +222,12 @@ func (c *Checker) Check(ctx context.Context, results chan<- Result, progress cha // Send progress at most once per second (rate-limited) if progress != nil { now := time.Now() + isLast := checkedFiles == totalFiles if isLast || now.Sub(lastProgressTime) >= time.Second { - elapsed := time.Since(startTime) - var bytesPerSec float64 - var eta time.Duration - - if elapsed > 0 && checkedBytes > 0 { - bytesPerSec = float64(checkedBytes) / elapsed.Seconds() - remainingBytes := totalBytes - checkedBytes - if bytesPerSec > 0 { - eta = time.Duration(float64(remainingBytes)/bytesPerSec) * time.Second - } - } + bytesPerSec, eta := computeRateETA( + time.Since(startTime), checkedBytes, totalBytes, + ) sendCheckStatus(progress, CheckStatus{ TotalFiles: totalFiles, @@ -228,6 +238,7 @@ func (c *Checker) Check(ctx context.Context, results chan<- Result, progress cha ETA: eta, Failures: failures, }) + lastProgressTime = now } } @@ -236,59 +247,6 @@ func (c *Checker) Check(ctx context.Context, results chan<- Result, progress cha return nil } -func (c *Checker) checkFile(entry *MFFilePath, checkedBytes *FileSize) Result { - absPath := filepath.Join(string(c.basePath), entry.Path) - relPath := RelFilePath(entry.Path) - - // Check if file exists - info, err := c.fs.Stat(absPath) - if err != nil { - if errors.Is(err, os.ErrNotExist) || errors.Is(err, afero.ErrFileNotFound) { - return Result{Path: relPath, Status: StatusMissing, Message: "file not found"} - } - return Result{Path: relPath, Status: StatusError, Message: err.Error()} - } - - // Check size - if info.Size() != entry.Size { - *checkedBytes += FileSize(info.Size()) - return Result{ - Path: relPath, - Status: StatusSizeMismatch, - Message: "size mismatch", - } - } - - // Open and hash file - f, err := c.fs.Open(absPath) - if err != nil { - return Result{Path: relPath, Status: StatusError, Message: err.Error()} - } - defer func() { _ = f.Close() }() - - h := sha256.New() - n, err := io.Copy(h, f) - if err != nil { - return Result{Path: relPath, Status: StatusError, Message: err.Error()} - } - *checkedBytes += FileSize(n) - - // Encode as multihash and compare - computed, err := multihash.Encode(h.Sum(nil), multihash.SHA2_256) - if err != nil { - return Result{Path: relPath, Status: StatusError, Message: err.Error()} - } - - // Check against all hashes in manifest (at least one must match) - for _, hash := range entry.Hashes { - if bytes.Equal(computed, hash.MultiHash) { - return Result{Path: relPath, Status: StatusOK} - } - } - - return Result{Path: relPath, Status: StatusHashMismatch, Message: "hash mismatch"} -} - // FindExtraFiles walks the filesystem and reports files not in the manifest. // Results are sent to the results channel. The channel is closed when done. // Hidden files/directories (starting with .) are skipped, as they are excluded @@ -298,7 +256,7 @@ func (c *Checker) FindExtraFiles(ctx context.Context, results chan<- Result) err defer close(results) } - return afero.Walk(c.fs, string(c.basePath), func(walkPath string, info os.FileInfo, err error) error { + walkFn := func(walkPath string, info os.FileInfo, err error) error { if err != nil { return err } @@ -320,6 +278,7 @@ func (c *Checker) FindExtraFiles(ctx context.Context, results chan<- Result) err if info.IsDir() { return filepath.SkipDir } + return nil } @@ -347,7 +306,75 @@ func (c *Checker) FindExtraFiles(ctx context.Context, results chan<- Result) err } return nil - }) + } + + return afero.Walk(c.fs, string(c.basePath), walkFn) +} + +func (c *Checker) checkFile(entry *MFFilePath, checkedBytes *FileSize) Result { + absPath := filepath.Join(string(c.basePath), entry.GetPath()) + relPath := RelFilePath(entry.GetPath()) + + // Check if file exists + info, err := c.fs.Stat(absPath) + if err != nil { + if errors.Is(err, os.ErrNotExist) || errors.Is(err, afero.ErrFileNotFound) { + return Result{ + Path: relPath, + Status: StatusMissing, + Message: "file not found", + } + } + + return Result{Path: relPath, Status: StatusError, Message: err.Error()} + } + + // Check size + if info.Size() != entry.GetSize() { + *checkedBytes += FileSize(info.Size()) + + return Result{ + Path: relPath, + Status: StatusSizeMismatch, + Message: "size mismatch", + } + } + + // Open and hash file + f, err := c.fs.Open(absPath) + if err != nil { + return Result{Path: relPath, Status: StatusError, Message: err.Error()} + } + + defer func() { _ = f.Close() }() + + h := sha256.New() + + n, err := io.Copy(h, f) + if err != nil { + return Result{Path: relPath, Status: StatusError, Message: err.Error()} + } + + *checkedBytes += FileSize(n) + + // Encode as multihash and compare + computed, err := multihash.Encode(h.Sum(nil), multihash.SHA2_256) + if err != nil { + return Result{Path: relPath, Status: StatusError, Message: err.Error()} + } + + // Check against all hashes in manifest (at least one must match) + for _, hash := range entry.GetHashes() { + if bytes.Equal(computed, hash.GetMultiHash()) { + return Result{Path: relPath, Status: StatusOK} + } + } + + return Result{ + Path: relPath, + Status: StatusHashMismatch, + Message: "hash mismatch", + } } // sendCheckStatus sends a status update without blocking. @@ -355,6 +382,7 @@ func sendCheckStatus(ch chan<- CheckStatus, status CheckStatus) { if ch == nil { return } + select { case ch <- status: default: diff --git a/mfer/checker_test.go b/mfer/checker_test.go index 3709d48..1e79218 100644 --- a/mfer/checker_test.go +++ b/mfer/checker_test.go @@ -1,3 +1,4 @@ +//nolint:testpackage // white-box tests exercise unexported internals package mfer import ( @@ -12,7 +13,15 @@ import ( "github.com/stretchr/testify/require" ) +const ( + testFile1 = "file1.txt" + testFile2 = "file2.txt" + testExistsFile = "exists.txt" +) + func TestStatusString(t *testing.T) { + t.Parallel() + tests := []struct { status Status expected string @@ -28,19 +37,26 @@ func TestStatusString(t *testing.T) { for _, tt := range tests { t.Run(tt.expected, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.expected, tt.status.String()) }) } } // createTestManifest creates a manifest file in the filesystem with the given files. -func createTestManifest(t *testing.T, fs afero.Fs, manifestPath string, files map[string][]byte) { +func createTestManifest( + t *testing.T, fs afero.Fs, manifestPath string, files map[string][]byte, +) { t.Helper() builder := NewBuilder() + for path, content := range files { reader := bytes.NewReader(content) - _, err := builder.AddFile(RelFilePath(path), FileSize(len(content)), ModTime(time.Now()), reader, nil) + _, err := builder.AddFile( + RelFilePath(path), FileSize(len(content)), ModTime(time.Now()), reader, nil, + ) require.NoError(t, err) } @@ -49,10 +65,13 @@ func createTestManifest(t *testing.T, fs afero.Fs, manifestPath string, files ma require.NoError(t, afero.WriteFile(fs, manifestPath, buf.Bytes(), 0o644)) } -// createFilesOnDisk creates the given files on the filesystem. -func createFilesOnDisk(t *testing.T, fs afero.Fs, basePath string, files map[string][]byte) { +// createFilesOnDisk creates the given files on the filesystem under +// /data. +func createFilesOnDisk(t *testing.T, fs afero.Fs, files map[string][]byte) { t.Helper() + basePath := "/data" + for path, content := range files { fullPath := basePath + "/" + path require.NoError(t, fs.MkdirAll(basePath, 0o755)) @@ -61,11 +80,15 @@ func createFilesOnDisk(t *testing.T, fs afero.Fs, basePath string, files map[str } func TestNewChecker(t *testing.T) { + t.Parallel() + t.Run("valid manifest", func(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() files := map[string][]byte{ - "file1.txt": []byte("hello"), - "file2.txt": []byte("world"), + testFile1: []byte("hello"), + testFile2: []byte("world"), } createTestManifest(t, fs, "/manifest.mf", files) @@ -76,12 +99,16 @@ func TestNewChecker(t *testing.T) { }) t.Run("missing manifest", func(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() _, err := NewChecker("/nonexistent.mf", "/", fs) assert.Error(t, err) }) t.Run("invalid manifest", func(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() require.NoError(t, afero.WriteFile(fs, "/bad.mf", []byte("not a manifest"), 0o644)) _, err := NewChecker("/bad.mf", "/", fs) @@ -90,6 +117,8 @@ func TestNewChecker(t *testing.T) { } func TestCheckerFileCountAndTotalBytes(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() files := map[string][]byte{ "small.txt": []byte("hi"), @@ -106,13 +135,15 @@ func TestCheckerFileCountAndTotalBytes(t *testing.T) { } func TestCheckAllFilesOK(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() files := map[string][]byte{ - "file1.txt": []byte("content one"), - "file2.txt": []byte("content two"), + testFile1: []byte("content one"), + testFile2: []byte("content two"), } createTestManifest(t, fs, "/manifest.mf", files) - createFilesOnDisk(t, fs, "/data", files) + createFilesOnDisk(t, fs, files) chk, err := NewChecker("/manifest.mf", "/data", fs) require.NoError(t, err) @@ -127,21 +158,24 @@ func TestCheckAllFilesOK(t *testing.T) { } assert.Len(t, resultList, 2) + for _, r := range resultList { assert.Equal(t, StatusOK, r.Status, "file %s should be OK", r.Path) } } func TestCheckMissingFile(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() files := map[string][]byte{ - "exists.txt": []byte("I exist"), - "missing.txt": []byte("I don't exist on disk"), + testExistsFile: []byte("I exist"), + "missing.txt": []byte("I don't exist on disk"), } createTestManifest(t, fs, "/manifest.mf", files) // Only create one file - createFilesOnDisk(t, fs, "/data", map[string][]byte{ - "exists.txt": []byte("I exist"), + createFilesOnDisk(t, fs, map[string][]byte{ + testExistsFile: []byte("I exist"), }) chk, err := NewChecker("/manifest.mf", "/data", fs) @@ -152,13 +186,17 @@ func TestCheckMissingFile(t *testing.T) { require.NoError(t, err) var okCount, missingCount int + for r := range results { switch r.Status { case StatusOK: okCount++ case StatusMissing: missingCount++ + assert.Equal(t, RelFilePath("missing.txt"), r.Path) + case StatusSizeMismatch, StatusHashMismatch, StatusExtra, StatusError: + // Not expected in this test; counted assertions below will fail. } } @@ -167,14 +205,16 @@ func TestCheckMissingFile(t *testing.T) { } func TestCheckSizeMismatch(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() files := map[string][]byte{ - "file.txt": []byte("original content"), + testFileName: []byte("original content"), } createTestManifest(t, fs, "/manifest.mf", files) // Create file with different size - createFilesOnDisk(t, fs, "/data", map[string][]byte{ - "file.txt": []byte("short"), + createFilesOnDisk(t, fs, map[string][]byte{ + testFileName: []byte("short"), }) chk, err := NewChecker("/manifest.mf", "/data", fs) @@ -186,21 +226,23 @@ func TestCheckSizeMismatch(t *testing.T) { r := <-results assert.Equal(t, StatusSizeMismatch, r.Status) - assert.Equal(t, RelFilePath("file.txt"), r.Path) + assert.Equal(t, RelFilePath(testFileName), r.Path) } func TestCheckHashMismatch(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() originalContent := []byte("original content") files := map[string][]byte{ - "file.txt": originalContent, + testFileName: originalContent, } createTestManifest(t, fs, "/manifest.mf", files) // Create file with same size but different content differentContent := []byte("different contnt") // same length (16 bytes) but different - require.Equal(t, len(originalContent), len(differentContent), "test requires same length") - createFilesOnDisk(t, fs, "/data", map[string][]byte{ - "file.txt": differentContent, + require.Len(t, differentContent, len(originalContent), "test requires same length") + createFilesOnDisk(t, fs, map[string][]byte{ + testFileName: differentContent, }) chk, err := NewChecker("/manifest.mf", "/data", fs) @@ -212,17 +254,19 @@ func TestCheckHashMismatch(t *testing.T) { r := <-results assert.Equal(t, StatusHashMismatch, r.Status) - assert.Equal(t, RelFilePath("file.txt"), r.Path) + assert.Equal(t, RelFilePath(testFileName), r.Path) } func TestCheckWithProgress(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() files := map[string][]byte{ - "file1.txt": bytes.Repeat([]byte("a"), 100), - "file2.txt": bytes.Repeat([]byte("b"), 200), + testFile1: bytes.Repeat([]byte("a"), 100), + testFile2: bytes.Repeat([]byte("b"), 200), } createTestManifest(t, fs, "/manifest.mf", files) - createFilesOnDisk(t, fs, "/data", files) + createFilesOnDisk(t, fs, files) chk, err := NewChecker("/manifest.mf", "/data", fs) require.NoError(t, err) @@ -233,9 +277,7 @@ func TestCheckWithProgress(t *testing.T) { err = chk.Check(context.Background(), results, progress) require.NoError(t, err) - // Drain results - for range results { - } + // results is fully buffered and closed; no draining needed // Check progress was sent var progressUpdates []CheckStatus @@ -254,14 +296,17 @@ func TestCheckWithProgress(t *testing.T) { } func TestCheckContextCancellation(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() // Create many files to ensure we have time to cancel files := make(map[string][]byte) - for i := 0; i < 100; i++ { + for i := range 100 { files[string(rune('a'+i%26))+".txt"] = bytes.Repeat([]byte("x"), 1000) } + createTestManifest(t, fs, "/manifest.mf", files) - createFilesOnDisk(t, fs, "/data", files) + createFilesOnDisk(t, fs, files) chk, err := NewChecker("/manifest.mf", "/data", fs) require.NoError(t, err) @@ -275,17 +320,19 @@ func TestCheckContextCancellation(t *testing.T) { } func TestFindExtraFiles(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() // Manifest only contains file1 manifestFiles := map[string][]byte{ - "file1.txt": []byte("in manifest"), + testFile1: []byte("in manifest"), } createTestManifest(t, fs, "/manifest.mf", manifestFiles) // Disk has file1 and file2 - createFilesOnDisk(t, fs, "/data", map[string][]byte{ - "file1.txt": []byte("in manifest"), - "file2.txt": []byte("extra file"), + createFilesOnDisk(t, fs, map[string][]byte{ + testFile1: []byte("in manifest"), + testFile2: []byte("extra file"), }) chk, err := NewChecker("/manifest.mf", "/data", fs) @@ -301,19 +348,21 @@ func TestFindExtraFiles(t *testing.T) { } assert.Len(t, extras, 1) - assert.Equal(t, RelFilePath("file2.txt"), extras[0].Path) + assert.Equal(t, RelFilePath(testFile2), extras[0].Path) assert.Equal(t, StatusExtra, extras[0].Status) assert.Equal(t, "not in manifest", extras[0].Message) } func TestFindExtraFilesSkipsManifestAndDotfiles(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() manifestFiles := map[string][]byte{ - "file1.txt": []byte("in manifest"), + testFile1: []byte("in manifest"), } createTestManifest(t, fs, "/data/.index.mf", manifestFiles) - createFilesOnDisk(t, fs, "/data", map[string][]byte{ - "file1.txt": []byte("in manifest"), + createFilesOnDisk(t, fs, map[string][]byte{ + testFile1: []byte("in manifest"), }) // Create dotfile and manifest that should be skipped require.NoError(t, afero.WriteFile(fs, "/data/.hidden", []byte("hidden"), 0o644)) @@ -338,17 +387,21 @@ func TestFindExtraFilesSkipsManifestAndDotfiles(t *testing.T) { for _, e := range extras { t.Logf("extra: %s", e.Path) } + assert.Len(t, extras, 1) + if len(extras) > 0 { assert.Equal(t, RelFilePath("extra.txt"), extras[0].Path) } } func TestFindExtraFilesContextCancellation(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() - files := map[string][]byte{"file.txt": []byte("data")} + files := map[string][]byte{testFileName: []byte("data")} createTestManifest(t, fs, "/manifest.mf", files) - createFilesOnDisk(t, fs, "/data", files) + createFilesOnDisk(t, fs, files) chk, err := NewChecker("/manifest.mf", "/data", fs) require.NoError(t, err) @@ -362,10 +415,12 @@ func TestFindExtraFilesContextCancellation(t *testing.T) { } func TestCheckNilChannels(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() - files := map[string][]byte{"file.txt": []byte("data")} + files := map[string][]byte{testFileName: []byte("data")} createTestManifest(t, fs, "/manifest.mf", files) - createFilesOnDisk(t, fs, "/data", files) + createFilesOnDisk(t, fs, files) chk, err := NewChecker("/manifest.mf", "/data", fs) require.NoError(t, err) @@ -376,10 +431,12 @@ func TestCheckNilChannels(t *testing.T) { } func TestFindExtraFilesNilChannel(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() - files := map[string][]byte{"file.txt": []byte("data")} + files := map[string][]byte{testFileName: []byte("data")} createTestManifest(t, fs, "/manifest.mf", files) - createFilesOnDisk(t, fs, "/data", files) + createFilesOnDisk(t, fs, files) chk, err := NewChecker("/manifest.mf", "/data", fs) require.NoError(t, err) @@ -390,6 +447,8 @@ func TestFindExtraFilesNilChannel(t *testing.T) { } func TestCheckSubdirectories(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() files := map[string][]byte{ "dir1/file1.txt": []byte("content1"), @@ -401,6 +460,7 @@ func TestCheckSubdirectories(t *testing.T) { // Create files with full directory structure for path, content := range files { fullPath := "/data/" + path + require.NoError(t, fs.MkdirAll("/data/dir1/dir2/dir3", 0o755)) require.NoError(t, afero.WriteFile(fs, fullPath, content, 0o644)) } @@ -413,25 +473,30 @@ func TestCheckSubdirectories(t *testing.T) { require.NoError(t, err) var okCount int + for r := range results { assert.Equal(t, StatusOK, r.Status, "file %s should be OK", r.Path) + okCount++ } + assert.Equal(t, 3, okCount) } func TestCheckMissingFileDetectedWithoutFallback(t *testing.T) { + t.Parallel() + // Regression test: errors.Is(err, errors.New("...")) never matches because // errors.New creates a new value each time. The fix uses os.ErrNotExist instead. fs := afero.NewMemMapFs() files := map[string][]byte{ - "exists.txt": []byte("here"), - "missing.txt": []byte("not on disk"), + testExistsFile: []byte("here"), + "missing.txt": []byte("not on disk"), } createTestManifest(t, fs, "/manifest.mf", files) // Only create one file on disk - createFilesOnDisk(t, fs, "/data", map[string][]byte{ - "exists.txt": []byte("here"), + createFilesOnDisk(t, fs, map[string][]byte{ + testExistsFile: []byte("here"), }) chk, err := NewChecker("/manifest.mf", "/data", fs) @@ -448,25 +513,29 @@ func TestCheckMissingFileDetectedWithoutFallback(t *testing.T) { assert.Equal(t, RelFilePath("missing.txt"), r.Path) } } + assert.Equal(t, 1, statusCounts[StatusOK], "one file should be OK") assert.Equal(t, 1, statusCounts[StatusMissing], "one file should be MISSING") assert.Equal(t, 0, statusCounts[StatusError], "no files should be ERROR") } func TestFindExtraFilesSkipsDotfiles(t *testing.T) { + t.Parallel() + // Regression test for #16: FindExtraFiles should not report dotfiles // or the manifest file itself as extra files. fs := afero.NewMemMapFs() files := map[string][]byte{ - "file1.txt": []byte("in manifest"), + testFile1: []byte("in manifest"), } createTestManifest(t, fs, "/data/.index.mf", files) - createFilesOnDisk(t, fs, "/data", files) + createFilesOnDisk(t, fs, files) // Add dotfiles and manifest file on disk require.NoError(t, afero.WriteFile(fs, "/data/.hidden", []byte("dotfile"), 0o644)) require.NoError(t, fs.MkdirAll("/data/.git", 0o755)) - require.NoError(t, afero.WriteFile(fs, "/data/.git/config", []byte("git config"), 0o644)) + require.NoError(t, + afero.WriteFile(fs, "/data/.git/config", []byte("git config"), 0o644)) chk, err := NewChecker("/data/.index.mf", "/data", fs) require.NoError(t, err) @@ -481,17 +550,21 @@ func TestFindExtraFilesSkipsDotfiles(t *testing.T) { } // Should report NO extra files — dotfiles and manifest should be skipped - assert.Empty(t, extras, "FindExtraFiles should not report dotfiles or manifest file as extra; got: %v", extras) + assert.Empty(t, extras, + "FindExtraFiles should not report dotfiles or manifest file as extra; got: %v", + extras) } func TestFindExtraFilesSkipsManifestFile(t *testing.T) { + t.Parallel() + // The manifest file itself should never be reported as extra fs := afero.NewMemMapFs() files := map[string][]byte{ - "file1.txt": []byte("content"), + testFile1: []byte("content"), } createTestManifest(t, fs, "/data/index.mf", files) - createFilesOnDisk(t, fs, "/data", files) + createFilesOnDisk(t, fs, files) chk, err := NewChecker("/data/index.mf", "/data", fs) require.NoError(t, err) @@ -505,10 +578,13 @@ func TestFindExtraFilesSkipsManifestFile(t *testing.T) { extras = append(extras, r) } - assert.Empty(t, extras, "manifest file should not be reported as extra; got: %v", extras) + assert.Empty(t, extras, + "manifest file should not be reported as extra; got: %v", extras) } func TestCheckEmptyManifest(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() // Create manifest with no files createTestManifest(t, fs, "/manifest.mf", map[string][]byte{}) @@ -527,21 +603,26 @@ func TestCheckEmptyManifest(t *testing.T) { for range results { count++ } + assert.Equal(t, 0, count) } func TestCheckProgressRateLimited(t *testing.T) { + t.Parallel() + // Create many small files - progress should be rate-limited, not one per file. // With rate-limiting to once per second, we should get far fewer progress // updates than files (plus one final update). fs := afero.NewMemMapFs() files := make(map[string][]byte, 100) - for i := 0; i < 100; i++ { + + for i := range 100 { name := fmt.Sprintf("file%03d.txt", i) files[name] = []byte("content") } + createTestManifest(t, fs, "/manifest.mf", files) - createFilesOnDisk(t, fs, "/data", files) + createFilesOnDisk(t, fs, files) chk, err := NewChecker("/manifest.mf", "/data", fs) require.NoError(t, err) @@ -551,9 +632,7 @@ func TestCheckProgressRateLimited(t *testing.T) { err = chk.Check(context.Background(), results, progress) require.NoError(t, err) - // Drain results - for range results { - } + // results is fully buffered and closed; no draining needed // Count progress updates var progressCount int @@ -563,6 +642,8 @@ func TestCheckProgressRateLimited(t *testing.T) { // Should be far fewer than 100 (rate-limited to once per second) // At minimum we get the final update - assert.GreaterOrEqual(t, progressCount, 1, "should get at least the final progress update") - assert.Less(t, progressCount, 100, "progress should be rate-limited, not one per file") + assert.GreaterOrEqual(t, progressCount, 1, + "should get at least the final progress update") + assert.Less(t, progressCount, 100, + "progress should be rate-limited, not one per file") } diff --git a/mfer/constants.go b/mfer/constants.go index 4640637..85a150b 100644 --- a/mfer/constants.go +++ b/mfer/constants.go @@ -1,11 +1,17 @@ package mfer const ( - Version = "0.1.0" + // Version is the current mfer release version. + Version = "0.1.0" + + // ReleaseDate is the date on which Version was released. ReleaseDate = "2025-12-17" // MaxDecompressedSize is the maximum allowed size of decompressed manifest // data (256 MB). This prevents decompression bombs from consuming excessive // memory. MaxDecompressedSize int64 = 256 * 1024 * 1024 + + // uuidLength is the length in bytes of a binary UUID. + uuidLength = 16 ) diff --git a/mfer/deserialize.go b/mfer/deserialize.go index f8de802..a099d4a 100644 --- a/mfer/deserialize.go +++ b/mfer/deserialize.go @@ -15,105 +15,174 @@ import ( "sneak.berlin/go/mfer/internal/log" ) +var ( + errInvalidUUIDLength = errors.New("invalid UUID length") + errInvalidUUIDFormat = errors.New("invalid UUID format") + errUnknownVersion = errors.New("unknown version") + errUnknownCompression = errors.New("unknown compression type") + errCompressedHashWrong = errors.New("compressed data hash mismatch") + errSignatureNoPubKey = errors.New("signature present but no public key") + errDecompressedTooLarge = errors.New("decompressed data exceeds maximum allowed size") + errUUIDMismatch = errors.New("outer and inner UUID mismatch") + errInvalidFileFormat = errors.New("invalid file format") +) + // validateUUID checks that the byte slice is a valid UUID (16 bytes, parseable). func validateUUID(data []byte) error { - if len(data) != 16 { - return errors.New("invalid UUID length") + if len(data) != uuidLength { + return errInvalidUUIDLength } // Try to parse as UUID to validate format _, err := uuid.FromBytes(data) if err != nil { - return errors.New("invalid UUID format") + return errInvalidUUIDFormat } + return nil } -func (m *manifest) deserializeInner() error { - if m.pbOuter.Version != MFFileOuter_VERSION_ONE { - return errors.New("unknown version") +// validateOuterHeader checks the outer message's version, compression +// type, and UUID. +func (m *manifest) validateOuterHeader() error { + if m.pbOuter.GetVersion() != MFFileOuter_VERSION_ONE { + return errUnknownVersion } - if m.pbOuter.CompressionType != MFFileOuter_COMPRESSION_ZSTD { - return errors.New("unknown compression type") + + if m.pbOuter.GetCompressionType() != MFFileOuter_COMPRESSION_ZSTD { + return errUnknownCompression } // Validate outer UUID before any decompression - if err := validateUUID(m.pbOuter.Uuid); err != nil { - return errors.New("outer UUID invalid: " + err.Error()) + err := validateUUID(m.pbOuter.GetUuid()) + if err != nil { + return fmt.Errorf("outer UUID invalid: %w", err) } - // Verify hash of compressed data before decompression + return nil +} + +// verifyOuterIntegrity checks the hash of the compressed payload and, +// if a signature is present, verifies it against the embedded public key. +func (m *manifest) verifyOuterIntegrity() error { h := sha256.New() - if _, err := h.Write(m.pbOuter.InnerMessage); err != nil { + + _, err := h.Write(m.pbOuter.GetInnerMessage()) + if err != nil { return fmt.Errorf("deserialize: hash write: %w", err) } + sha256Hash := h.Sum(nil) - if !bytes.Equal(sha256Hash, m.pbOuter.Sha256) { - return errors.New("compressed data hash mismatch") + if !bytes.Equal(sha256Hash, m.pbOuter.GetSha256()) { + return errCompressedHashWrong } - // Verify signature if present - if len(m.pbOuter.Signature) > 0 { - if len(m.pbOuter.SigningPubKey) == 0 { - return errors.New("signature present but no public key") - } - - sigString, err := m.signatureString() - if err != nil { - return fmt.Errorf("failed to generate signature string for verification: %w", err) - } - - if err := gpgVerify([]byte(sigString), m.pbOuter.Signature, m.pbOuter.SigningPubKey); err != nil { - return fmt.Errorf("signature verification failed: %w", err) - } - log.Infof("signature verified successfully") + if len(m.pbOuter.GetSignature()) == 0 { + return nil } - bb := bytes.NewBuffer(m.pbOuter.InnerMessage) + if len(m.pbOuter.GetSigningPubKey()) == 0 { + return errSignatureNoPubKey + } + + sigString, err := m.signatureString() + if err != nil { + return fmt.Errorf( + "failed to generate signature string for verification: %w", err, + ) + } + + err = gpgVerify( + []byte(sigString), + m.pbOuter.GetSignature(), + m.pbOuter.GetSigningPubKey(), + ) + if err != nil { + return fmt.Errorf("signature verification failed: %w", err) + } + + log.Infof("signature verified successfully") + + return nil +} + +// decompressInner decompresses the inner payload, enforcing size limits +// to prevent decompression bombs. +func (m *manifest) decompressInner() ([]byte, error) { + bb := bytes.NewBuffer(m.pbOuter.GetInnerMessage()) zr, err := zstd.NewReader(bb) if err != nil { - return fmt.Errorf("deserialize: zstd reader: %w", err) + return nil, fmt.Errorf("deserialize: zstd reader: %w", err) } defer zr.Close() // Limit decompressed size to prevent decompression bombs. // Use declared size + 1 byte to detect overflow, capped at MaxDecompressedSize. maxSize := MaxDecompressedSize - if m.pbOuter.Size > 0 && m.pbOuter.Size < int64(maxSize) { - maxSize = int64(m.pbOuter.Size) + 1 + if m.pbOuter.GetSize() > 0 && m.pbOuter.GetSize() < maxSize { + maxSize = m.pbOuter.GetSize() + 1 } + limitedReader := io.LimitReader(zr, maxSize) + dat, err := io.ReadAll(limitedReader) if err != nil { - return fmt.Errorf("deserialize: decompress: %w", err) + return nil, fmt.Errorf("deserialize: decompress: %w", err) } + if int64(len(dat)) >= MaxDecompressedSize { - return fmt.Errorf("decompressed data exceeds maximum allowed size of %d bytes", MaxDecompressedSize) + return nil, fmt.Errorf( + "%w of %d bytes", errDecompressedTooLarge, MaxDecompressedSize, + ) + } + + return dat, nil +} + +func (m *manifest) deserializeInner() error { + err := m.validateOuterHeader() + if err != nil { + return err + } + + err = m.verifyOuterIntegrity() + if err != nil { + return err + } + + dat, err := m.decompressInner() + if err != nil { + return err } isize := len(dat) - if int64(isize) != m.pbOuter.Size { - log.Debugf("truncated data, got %d expected %d", isize, m.pbOuter.Size) + if int64(isize) != m.pbOuter.GetSize() { + log.Debugf("truncated data, got %d expected %d", isize, m.pbOuter.GetSize()) + return bork.ErrFileTruncated } // Deserialize inner message m.pbInner = new(MFFile) - if err := proto.Unmarshal(dat, m.pbInner); err != nil { + + err = proto.Unmarshal(dat, m.pbInner) + if err != nil { return fmt.Errorf("deserialize: unmarshal inner: %w", err) } // Validate inner UUID - if err := validateUUID(m.pbInner.Uuid); err != nil { - return errors.New("inner UUID invalid: " + err.Error()) + err = validateUUID(m.pbInner.GetUuid()) + if err != nil { + return fmt.Errorf("inner UUID invalid: %w", err) } // Verify UUIDs match - if !bytes.Equal(m.pbOuter.Uuid, m.pbInner.Uuid) { - return errors.New("outer and inner UUID mismatch") + if !bytes.Equal(m.pbOuter.GetUuid(), m.pbInner.GetUuid()) { + return errUUIDMismatch } - log.Infof("loaded manifest with %d files", len(m.pbInner.Files)) + log.Infof("loaded manifest with %d files", len(m.pbInner.GetFiles())) + return nil } @@ -122,20 +191,26 @@ func validateMagic(dat []byte) bool { if len(dat) < ml { return false } + got := dat[0:ml] expected := []byte(MAGIC) + return bytes.Equal(got, expected) } // NewManifestFromReader reads a manifest from an io.Reader. +// +//nolint:revive // unexported-return: exporting manifest is owner question 13 func NewManifestFromReader(input io.Reader) (*manifest, error) { m := &manifest{} + dat, err := io.ReadAll(input) if err != nil { return nil, err } + if !validateMagic(dat) { - return nil, errors.New("invalid file format") + return nil, errInvalidFileFormat } // remove magic bytes prefix: @@ -145,12 +220,15 @@ func NewManifestFromReader(input io.Reader) (*manifest, error) { // deserialize outer: m.pbOuter = new(MFFileOuter) - if err := proto.Unmarshal(dat, m.pbOuter); err != nil { + + err = proto.Unmarshal(dat, m.pbOuter) + if err != nil { return nil, err } // deserialize inner: - if err := m.deserializeInner(); err != nil { + err = m.deserializeInner() + if err != nil { return nil, err } @@ -159,14 +237,19 @@ func NewManifestFromReader(input io.Reader) (*manifest, error) { // NewManifestFromFile reads a manifest from a file path using the given filesystem. // If fs is nil, the real filesystem (OsFs) is used. +// +//nolint:revive // unexported-return: exporting manifest is owner question 13 func NewManifestFromFile(fs afero.Fs, path string) (*manifest, error) { if fs == nil { fs = afero.NewOsFs() } + f, err := fs.Open(path) if err != nil { return nil, err } + defer func() { _ = f.Close() }() + return NewManifestFromReader(f) } diff --git a/mfer/errmsg_test.go b/mfer/errmsg_test.go new file mode 100644 index 0000000..f07e06c --- /dev/null +++ b/mfer/errmsg_test.go @@ -0,0 +1,85 @@ +//nolint:testpackage // white-box tests exercise unexported internals +package mfer + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestValidatePathMessagesVerbatim pins the exact rendered text of every +// ValidatePath rejection. +// +// These strings are user-visible and are assembled by wrapping static +// sentinels mid-sentence, which makes them easy to reword by accident +// while refactoring for errors.Is matchability. Changing one is a +// deliberate change, not a refactoring side effect. +func TestValidatePathMessagesVerbatim(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + path string + want string + is error + }{ + { + name: "empty", + path: "", + want: "path cannot be empty", + is: errPathEmpty, + }, + { + name: "not utf8", + path: "a\xffb", + want: `path "a\xffb" is not valid UTF-8`, + is: errPathNotUTF8, + }, + { + name: "backslash", + path: `a\b`, + want: `path "a\\b" contains backslash; ` + + "use forward slashes only", + is: errPathBackslash, + }, + { + name: "absolute", + path: "/a/b", + want: `path "/a/b" is absolute; must be relative`, + is: errPathAbsolute, + }, + { + name: "empty segment", + path: "a//b", + want: `path "a//b" contains empty segment`, + is: errPathEmptySegment, + }, + { + name: "dotdot segment", + path: "a/../b", + want: `path "a/../b" contains '..' segment`, + is: errPathDotDot, + }, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + err := ValidatePath(tc.path) + require.Error(t, err) + assert.Equal(t, tc.want, err.Error()) + require.ErrorIs(t, err, tc.is) + }) + } +} + +// TestSerializeInternalErrorMessagesVerbatim pins the two distinct +// "internal error" messages, which differ between generate and +// generateOuter and have always done so. +func TestSerializeInternalErrorMessagesVerbatim(t *testing.T) { + t.Parallel() + + m := &manifest{} + require.EqualError(t, m.generate(), "internal error: pbInner not set") + require.EqualError(t, m.generateOuter(), "internal error") +} diff --git a/mfer/gpg.go b/mfer/gpg.go index 2ae607b..568d672 100644 --- a/mfer/gpg.go +++ b/mfer/gpg.go @@ -2,13 +2,45 @@ package mfer import ( "bytes" + "context" + "errors" "fmt" + "io" "os" "os/exec" "path/filepath" "strings" ) +const ( + // privateDirPerms is the permission mode for temporary GPG home + // directories. + privateDirPerms os.FileMode = 0o700 + + // privateFilePerms is the permission mode for temporary key, + // signature, and data files. + privateFilePerms os.FileMode = 0o600 + + // gpgFingerprintField is the record type tag for fingerprint lines + // in gpg --with-colons output. + gpgFingerprintField = "fpr" + + // gpgFingerprintMinFields is the minimum number of colon-separated + // fields in a gpg fingerprint record (the fingerprint is field 10). + gpgFingerprintMinFields = 10 + + // gpg option names used from more than one call site. + gpgOptArmor = "--armor" + gpgOptHomedir = "--homedir" + gpgOptVerify = "--verify" +) + +var ( + errGPGKeyNotFound = errors.New("gpg key not found") + errFingerprintNotFound = errors.New("fingerprint not found for key") + errImportedFPRNotFound = errors.New("fingerprint not found in imported key") +) + // GPGKeyID represents a GPG key identifier (fingerprint or key ID). type GPGKeyID string @@ -17,22 +49,69 @@ type SigningOptions struct { KeyID GPGKeyID } -// gpgSign creates a detached signature of the data using the specified key. -// Returns the armored detached signature. -func gpgSign(data []byte, keyID GPGKeyID) ([]byte, error) { - cmd := exec.Command("gpg", "--batch", "--no-tty", - "--detach-sign", - "--armor", - "--local-user", string(keyID), - ) +// gpgArgs builds a gpg argument list from opts followed by positional +// arguments, separated by an explicit "--" end-of-options marker. +// +// This matters because key IDs reach gpg as bare positional arguments +// (from --sign-key / MFER_SIGN_KEY) and gpg would otherwise parse a value +// beginning with "-" as one of its own options. Callers must route every +// non-option argument through here. +func gpgArgs(opts []string, positional ...string) []string { + args := make([]string, 0, len(opts)+1+len(positional)) + args = append(args, opts...) + args = append(args, "--") + args = append(args, positional...) - cmd.Stdin = bytes.NewReader(data) + return args +} + +// runGPG runs the gpg binary in batch mode with the given arguments and +// optional stdin, returning captured stdout and stderr. +func runGPG(stdin io.Reader, args ...string) (*bytes.Buffer, *bytes.Buffer, error) { + fullArgs := append([]string{"--batch", "--no-tty"}, args...) + + // G204: the executable name is a compile-time constant. The arguments + // are not, so the guarantee that matters is placement: every + // caller-supplied value is passed either as the value of a named + // option or after the "--" end-of-options marker inserted by gpgArgs, + // and therefore cannot be reinterpreted by gpg as an option. + cmd := exec.CommandContext( //nolint:gosec // G204: see comment above + context.Background(), "gpg", fullArgs...) + cmd.Stdin = stdin var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout cmd.Stderr = &stderr - if err := cmd.Run(); err != nil { + err := cmd.Run() + + return &stdout, &stderr, err +} + +// parseFingerprint extracts the first fingerprint from gpg --with-colons +// output, or returns ok=false if none is present. +func parseFingerprint(colonOutput string) (string, bool) { + for _, line := range strings.Split(colonOutput, "\n") { + fields := strings.Split(line, ":") + if len(fields) >= gpgFingerprintMinFields && + fields[0] == gpgFingerprintField { + return fields[9], true + } + } + + return "", false +} + +// gpgSign creates a detached signature of the data using the specified key. +// Returns the armored detached signature. +func gpgSign(data []byte, keyID GPGKeyID) ([]byte, error) { + stdout, stderr, err := runGPG(bytes.NewReader(data), + "--detach-sign", + gpgOptArmor, + "--local-user", string(keyID), + ) + if err != nil { return nil, fmt.Errorf("gpg sign failed: %w: %s", err, stderr.String()) } @@ -42,22 +121,15 @@ func gpgSign(data []byte, keyID GPGKeyID) ([]byte, error) { // gpgExportPublicKey exports the public key for the specified key ID. // Returns the armored public key. func gpgExportPublicKey(keyID GPGKeyID) ([]byte, error) { - cmd := exec.Command("gpg", "--batch", "--no-tty", - "--export", - "--armor", - string(keyID), + stdout, stderr, err := runGPG(nil, + gpgArgs([]string{"--export", gpgOptArmor}, string(keyID))..., ) - - var stdout, stderr bytes.Buffer - cmd.Stdout = &stdout - cmd.Stderr = &stderr - - if err := cmd.Run(); err != nil { + if err != nil { return nil, fmt.Errorf("gpg export failed: %w: %s", err, stderr.String()) } if stdout.Len() == 0 { - return nil, fmt.Errorf("gpg key not found: %s", keyID) + return nil, fmt.Errorf("%w: %s", errGPGKeyNotFound, keyID) } return stdout.Bytes(), nil @@ -65,30 +137,21 @@ func gpgExportPublicKey(keyID GPGKeyID) ([]byte, error) { // gpgGetKeyFingerprint gets the full fingerprint for a key ID. func gpgGetKeyFingerprint(keyID GPGKeyID) ([]byte, error) { - cmd := exec.Command("gpg", "--batch", "--no-tty", - "--with-colons", - "--fingerprint", - string(keyID), + stdout, stderr, err := runGPG(nil, + gpgArgs([]string{"--with-colons", "--fingerprint"}, string(keyID))..., ) - - var stdout, stderr bytes.Buffer - cmd.Stdout = &stdout - cmd.Stderr = &stderr - - if err := cmd.Run(); err != nil { - return nil, fmt.Errorf("gpg fingerprint lookup failed: %w: %s", err, stderr.String()) + if err != nil { + return nil, fmt.Errorf( + "gpg fingerprint lookup failed: %w: %s", err, stderr.String(), + ) } - // Parse the colon-delimited output to find the fingerprint - lines := strings.Split(stdout.String(), "\n") - for _, line := range lines { - fields := strings.Split(line, ":") - if len(fields) >= 10 && fields[0] == "fpr" { - return []byte(fields[9]), nil - } + fpr, ok := parseFingerprint(stdout.String()) + if !ok { + return nil, fmt.Errorf("%w: %s", errFingerprintNotFound, keyID) } - return nil, fmt.Errorf("fingerprint not found for key: %s", keyID) + return []byte(fpr), nil } // gpgExtractPubKeyFingerprint imports a public key into a temporary keyring @@ -100,54 +163,51 @@ func gpgExtractPubKeyFingerprint(pubKey []byte) (string, error) { if err != nil { return "", fmt.Errorf("failed to create temp dir: %w", err) } + defer func() { _ = os.RemoveAll(tmpDir) }() // Set restrictive permissions - if err := os.Chmod(tmpDir, 0o700); err != nil { + err = os.Chmod(tmpDir, privateDirPerms) + if err != nil { return "", fmt.Errorf("failed to set temp dir permissions: %w", err) } // Write public key to temp file pubKeyFile := filepath.Join(tmpDir, "pubkey.asc") - if err := os.WriteFile(pubKeyFile, pubKey, 0o600); err != nil { + + err = os.WriteFile(pubKeyFile, pubKey, privateFilePerms) + if err != nil { return "", fmt.Errorf("failed to write public key: %w", err) } // Import the public key into the temporary keyring - importCmd := exec.Command("gpg", "--batch", "--no-tty", - "--homedir", tmpDir, - "--import", - pubKeyFile, + _, importStderr, err := runGPG(nil, + gpgArgs([]string{gpgOptHomedir, tmpDir, "--import"}, pubKeyFile)..., ) - var importStderr bytes.Buffer - importCmd.Stderr = &importStderr - if err := importCmd.Run(); err != nil { - return "", fmt.Errorf("failed to import public key: %w: %s", err, importStderr.String()) + if err != nil { + return "", fmt.Errorf( + "failed to import public key: %w: %s", err, importStderr.String(), + ) } // List keys to get fingerprint - listCmd := exec.Command("gpg", "--batch", "--no-tty", + listStdout, listStderr, err := runGPG(nil, "--homedir", tmpDir, "--with-colons", "--fingerprint", ) - var listStdout, listStderr bytes.Buffer - listCmd.Stdout = &listStdout - listCmd.Stderr = &listStderr - if err := listCmd.Run(); err != nil { - return "", fmt.Errorf("failed to list keys: %w: %s", err, listStderr.String()) + if err != nil { + return "", fmt.Errorf( + "failed to list keys: %w: %s", err, listStderr.String(), + ) } - // Parse the colon-delimited output to find the fingerprint - lines := strings.Split(listStdout.String(), "\n") - for _, line := range lines { - fields := strings.Split(line, ":") - if len(fields) >= 10 && fields[0] == "fpr" { - return fields[9], nil - } + fpr, ok := parseFingerprint(listStdout.String()) + if !ok { + return "", errImportedFPRNotFound } - return "", fmt.Errorf("fingerprint not found in imported key") + return fpr, nil } // gpgVerify verifies a detached signature against data using the provided public key. @@ -158,54 +218,58 @@ func gpgVerify(data, signature, pubKey []byte) error { if err != nil { return fmt.Errorf("failed to create temp dir: %w", err) } + defer func() { _ = os.RemoveAll(tmpDir) }() // Set restrictive permissions - if err := os.Chmod(tmpDir, 0o700); err != nil { + err = os.Chmod(tmpDir, privateDirPerms) + if err != nil { return fmt.Errorf("failed to set temp dir permissions: %w", err) } // Write public key to temp file pubKeyFile := filepath.Join(tmpDir, "pubkey.asc") - if err := os.WriteFile(pubKeyFile, pubKey, 0o600); err != nil { + + err = os.WriteFile(pubKeyFile, pubKey, privateFilePerms) + if err != nil { return fmt.Errorf("failed to write public key: %w", err) } // Write signature to temp file sigFile := filepath.Join(tmpDir, "signature.asc") - if err := os.WriteFile(sigFile, signature, 0o600); err != nil { + + err = os.WriteFile(sigFile, signature, privateFilePerms) + if err != nil { return fmt.Errorf("failed to write signature: %w", err) } // Write data to temp file dataFile := filepath.Join(tmpDir, "data") - if err := os.WriteFile(dataFile, data, 0o600); err != nil { + + err = os.WriteFile(dataFile, data, privateFilePerms) + if err != nil { return fmt.Errorf("failed to write data: %w", err) } // Import the public key into the temporary keyring - importCmd := exec.Command("gpg", "--batch", "--no-tty", - "--homedir", tmpDir, - "--import", - pubKeyFile, + _, importStderr, err := runGPG(nil, + gpgArgs([]string{gpgOptHomedir, tmpDir, "--import"}, pubKeyFile)..., ) - var importStderr bytes.Buffer - importCmd.Stderr = &importStderr - if err := importCmd.Run(); err != nil { - return fmt.Errorf("failed to import public key: %w: %s", err, importStderr.String()) + if err != nil { + return fmt.Errorf( + "failed to import public key: %w: %s", err, importStderr.String(), + ) } // Verify the signature - verifyCmd := exec.Command("gpg", "--batch", "--no-tty", - "--homedir", tmpDir, - "--verify", - sigFile, - dataFile, + _, verifyStderr, err := runGPG(nil, + gpgArgs([]string{gpgOptHomedir, tmpDir, gpgOptVerify}, + sigFile, dataFile)..., ) - var verifyStderr bytes.Buffer - verifyCmd.Stderr = &verifyStderr - if err := verifyCmd.Run(); err != nil { - return fmt.Errorf("signature verification failed: %w: %s", err, verifyStderr.String()) + if err != nil { + return fmt.Errorf( + "signature verification failed: %w: %s", err, verifyStderr.String(), + ) } return nil diff --git a/mfer/gpg_test.go b/mfer/gpg_test.go index badc8ca..43aa9d6 100644 --- a/mfer/gpg_test.go +++ b/mfer/gpg_test.go @@ -1,3 +1,4 @@ +//nolint:testpackage // white-box tests exercise unexported internals package mfer import ( @@ -15,35 +16,20 @@ import ( ) // testGPGEnv sets up a temporary GPG home directory with a test key. -// Returns the key ID and a cleanup function. -func testGPGEnv(t *testing.T) (GPGKeyID, func()) { +// Returns the key ID and the GPG home directory; callers must point +// GNUPGHOME at the returned directory (via t.Setenv) before using the +// gpg helpers under test. +func testGPGEnv(t *testing.T) (GPGKeyID, string) { t.Helper() // Check if gpg is installed - if _, err := exec.LookPath("gpg"); err != nil { + _, err := exec.LookPath("gpg") + if err != nil { t.Skip("gpg not installed, skipping signing test") - return "", func() {} } - // Create temporary GPG home directory - gpgHome, err := os.MkdirTemp("", "mfer-gpg-test-*") - require.NoError(t, err) - - // Set restrictive permissions on GPG home - require.NoError(t, os.Chmod(gpgHome, 0o700)) - - // Save original GNUPGHOME and set new one - origGPGHome := os.Getenv("GNUPGHOME") - require.NoError(t, os.Setenv("GNUPGHOME", gpgHome)) - - cleanup := func() { - if origGPGHome == "" { - _ = os.Unsetenv("GNUPGHOME") - } else { - _ = os.Setenv("GNUPGHOME", origGPGHome) - } - _ = os.RemoveAll(gpgHome) - } + // Create temporary GPG home directory (0700 by default) + gpgHome := t.TempDir() // Generate a test key with no passphrase keyParams := `%no-protection @@ -57,45 +43,51 @@ Expire-Date: 0 paramsFile := filepath.Join(gpgHome, "key-params") require.NoError(t, os.WriteFile(paramsFile, []byte(keyParams), 0o600)) - cmd := exec.Command("gpg", "--batch", "--gen-key", paramsFile) + //nolint:gosec // paramsFile is a test-controlled path inside t.TempDir() + cmd := exec.CommandContext(context.Background(), "gpg", + "--batch", "--gen-key", paramsFile) + cmd.Env = append(os.Environ(), "GNUPGHOME="+gpgHome) + output, err := cmd.CombinedOutput() if err != nil { - cleanup() t.Skipf("failed to generate test GPG key: %v: %s", err, output) - return "", func() {} } // Get the key fingerprint - cmd = exec.Command("gpg", "--list-keys", "--with-colons", "test@mfer.test") + cmd = exec.CommandContext(context.Background(), "gpg", + "--list-keys", "--with-colons", "test@mfer.test") + cmd.Env = append(os.Environ(), "GNUPGHOME="+gpgHome) + output, err = cmd.Output() if err != nil { - cleanup() t.Fatalf("failed to list test key: %v", err) } // Parse fingerprint from output var keyID string + for _, line := range strings.Split(string(output), "\n") { fields := strings.Split(line, ":") - if len(fields) >= 10 && fields[0] == "fpr" { + if len(fields) >= gpgFingerprintMinFields && + fields[0] == gpgFingerprintField { keyID = fields[9] + break } } if keyID == "" { - cleanup() t.Fatal("failed to find test key fingerprint") } - return GPGKeyID(keyID), cleanup + return GPGKeyID(keyID), gpgHome } func TestGPGSign(t *testing.T) { - keyID, cleanup := testGPGEnv(t) - defer cleanup() + keyID, gpgHome := testGPGEnv(t) + t.Setenv("GNUPGHOME", gpgHome) data := []byte("test data to sign") sig, err := gpgSign(data, keyID) @@ -106,8 +98,8 @@ func TestGPGSign(t *testing.T) { } func TestGPGExportPublicKey(t *testing.T) { - keyID, cleanup := testGPGEnv(t) - defer cleanup() + keyID, gpgHome := testGPGEnv(t) + t.Setenv("GNUPGHOME", gpgHome) pubKey, err := gpgExportPublicKey(keyID) require.NoError(t, err) @@ -117,8 +109,8 @@ func TestGPGExportPublicKey(t *testing.T) { } func TestGPGGetKeyFingerprint(t *testing.T) { - keyID, cleanup := testGPGEnv(t) - defer cleanup() + keyID, gpgHome := testGPGEnv(t) + t.Setenv("GNUPGHOME", gpgHome) fingerprint, err := gpgGetKeyFingerprint(keyID) require.NoError(t, err) @@ -127,10 +119,47 @@ func TestGPGGetKeyFingerprint(t *testing.T) { assert.Len(t, fingerprint, 40, "fingerprint should be 40 hex chars") } +// TestGPGArgsSeparatesPositionals pins that caller-supplied values are +// placed after an end-of-options marker. Key IDs arrive from --sign-key +// and MFER_SIGN_KEY as bare positional arguments, so without the marker +// a value beginning with "-" would be parsed by gpg as one of its own +// options. +func TestGPGArgsSeparatesPositionals(t *testing.T) { + t.Parallel() + + assert.Equal(t, + []string{"--opt-a", "--opt-b", "--", "--version"}, + gpgArgs([]string{"--opt-a", "--opt-b"}, "--version")) + + assert.Equal(t, + []string{"--opt-c", "--", "sig", "data"}, + gpgArgs([]string{"--opt-c"}, "sig", "data")) + + assert.Equal(t, []string{"--opt-d", "--"}, + gpgArgs([]string{"--opt-d"})) +} + +// TestGPGOptionLikeKeyIDIsNotAnOption drives real gpg with a key ID that +// looks like an option and asserts it is treated as a (nonexistent) key +// rather than executed as gpg's own --version. +func TestGPGOptionLikeKeyIDIsNotAnOption(t *testing.T) { + _, gpgHome := testGPGEnv(t) + t.Setenv("GNUPGHOME", gpgHome) + + pubKey, err := gpgExportPublicKey(GPGKeyID("--version")) + require.Error(t, err) + require.ErrorIs(t, err, errGPGKeyNotFound) + assert.NotContains(t, string(pubKey), "gpg (GnuPG)") + + fpr, err := gpgGetKeyFingerprint(GPGKeyID("--version")) + require.Error(t, err) + assert.NotContains(t, string(fpr), "gpg (GnuPG)") +} + func TestGPGSignInvalidKey(t *testing.T) { // Set up test environment (we need GNUPGHOME set) - _, cleanup := testGPGEnv(t) - defer cleanup() + _, gpgHome := testGPGEnv(t) + t.Setenv("GNUPGHOME", gpgHome) data := []byte("test data") _, err := gpgSign(data, GPGKeyID("NONEXISTENT_KEY_ID_12345")) @@ -138,8 +167,8 @@ func TestGPGSignInvalidKey(t *testing.T) { } func TestBuilderWithSigning(t *testing.T) { - keyID, cleanup := testGPGEnv(t) - defer cleanup() + keyID, gpgHome := testGPGEnv(t) + t.Setenv("GNUPGHOME", gpgHome) // Create a builder with signing options b := NewBuilder() @@ -155,6 +184,7 @@ func TestBuilderWithSigning(t *testing.T) { // Build the manifest var buf bytes.Buffer + err = b.Build(&buf) require.NoError(t, err) @@ -163,26 +193,32 @@ func TestBuilderWithSigning(t *testing.T) { require.NoError(t, err) require.NotNil(t, manifest.pbOuter) - assert.NotEmpty(t, manifest.pbOuter.Signature, "signature should be populated") - assert.NotEmpty(t, manifest.pbOuter.Signer, "signer should be populated") - assert.NotEmpty(t, manifest.pbOuter.SigningPubKey, "signing public key should be populated") + assert.NotEmpty(t, manifest.pbOuter.GetSignature(), + "signature should be populated") + assert.NotEmpty(t, manifest.pbOuter.GetSigner(), "signer should be populated") + assert.NotEmpty(t, manifest.pbOuter.GetSigningPubKey(), + "signing public key should be populated") // Verify signature is a valid PGP signature - assert.Contains(t, string(manifest.pbOuter.Signature), "-----BEGIN PGP SIGNATURE-----") + assert.Contains(t, string(manifest.pbOuter.GetSignature()), + "-----BEGIN PGP SIGNATURE-----") // Verify public key is a valid PGP public key block - assert.Contains(t, string(manifest.pbOuter.SigningPubKey), "-----BEGIN PGP PUBLIC KEY BLOCK-----") + assert.Contains(t, string(manifest.pbOuter.GetSigningPubKey()), + "-----BEGIN PGP PUBLIC KEY BLOCK-----") } func TestScannerWithSigning(t *testing.T) { - keyID, cleanup := testGPGEnv(t) - defer cleanup() + keyID, gpgHome := testGPGEnv(t) + t.Setenv("GNUPGHOME", gpgHome) // Create in-memory filesystem with test files fs := afero.NewMemMapFs() require.NoError(t, fs.MkdirAll("/testdir", 0o755)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("content1"), 0o644)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file2.txt", []byte("content2"), 0o644)) + require.NoError(t, + afero.WriteFile(fs, "/testdir/file1.txt", []byte("content1"), 0o644)) + require.NoError(t, + afero.WriteFile(fs, "/testdir/file2.txt", []byte("content2"), 0o644)) // Create scanner with signing options opts := &ScannerOptions{ @@ -205,14 +241,14 @@ func TestScannerWithSigning(t *testing.T) { manifest, err := NewManifestFromReader(&buf) require.NoError(t, err) - assert.NotEmpty(t, manifest.pbOuter.Signature) - assert.NotEmpty(t, manifest.pbOuter.Signer) - assert.NotEmpty(t, manifest.pbOuter.SigningPubKey) + assert.NotEmpty(t, manifest.pbOuter.GetSignature()) + assert.NotEmpty(t, manifest.pbOuter.GetSigner()) + assert.NotEmpty(t, manifest.pbOuter.GetSigningPubKey()) } func TestGPGVerify(t *testing.T) { - keyID, cleanup := testGPGEnv(t) - defer cleanup() + keyID, gpgHome := testGPGEnv(t) + t.Setenv("GNUPGHOME", gpgHome) data := []byte("test data to sign and verify") sig, err := gpgSign(data, keyID) @@ -227,8 +263,8 @@ func TestGPGVerify(t *testing.T) { } func TestGPGVerifyInvalidSignature(t *testing.T) { - keyID, cleanup := testGPGEnv(t) - defer cleanup() + keyID, gpgHome := testGPGEnv(t) + t.Setenv("GNUPGHOME", gpgHome) data := []byte("test data to sign") sig, err := gpgSign(data, keyID) @@ -244,8 +280,8 @@ func TestGPGVerifyInvalidSignature(t *testing.T) { } func TestGPGVerifyBadPublicKey(t *testing.T) { - keyID, cleanup := testGPGEnv(t) - defer cleanup() + keyID, gpgHome := testGPGEnv(t) + t.Setenv("GNUPGHOME", gpgHome) data := []byte("test data") sig, err := gpgSign(data, keyID) @@ -258,8 +294,8 @@ func TestGPGVerifyBadPublicKey(t *testing.T) { } func TestManifestSignatureVerification(t *testing.T) { - keyID, cleanup := testGPGEnv(t) - defer cleanup() + keyID, gpgHome := testGPGEnv(t) + t.Setenv("GNUPGHOME", gpgHome) // Create a builder with signing options b := NewBuilder() @@ -275,6 +311,7 @@ func TestManifestSignatureVerification(t *testing.T) { // Build the manifest var buf bytes.Buffer + err = b.Build(&buf) require.NoError(t, err) @@ -284,12 +321,12 @@ func TestManifestSignatureVerification(t *testing.T) { require.NotNil(t, manifest) // Signature should be present and valid - assert.NotEmpty(t, manifest.pbOuter.Signature) + assert.NotEmpty(t, manifest.pbOuter.GetSignature()) } func TestManifestTamperedSignatureFails(t *testing.T) { - keyID, cleanup := testGPGEnv(t) - defer cleanup() + keyID, gpgHome := testGPGEnv(t) + t.Setenv("GNUPGHOME", gpgHome) // Create a signed manifest b := NewBuilder() @@ -303,6 +340,7 @@ func TestManifestTamperedSignatureFails(t *testing.T) { require.NoError(t, err) var buf bytes.Buffer + err = b.Build(&buf) require.NoError(t, err) @@ -312,6 +350,7 @@ func TestManifestTamperedSignatureFails(t *testing.T) { for i := range data { if i > 100 && data[i] == 'A' { data[i] = 'B' + break } } @@ -322,6 +361,8 @@ func TestManifestTamperedSignatureFails(t *testing.T) { } func TestBuilderWithoutSigning(t *testing.T) { + t.Parallel() + // Create a builder without signing options b := NewBuilder() @@ -333,6 +374,7 @@ func TestBuilderWithoutSigning(t *testing.T) { // Build the manifest var buf bytes.Buffer + err = b.Build(&buf) require.NoError(t, err) @@ -341,7 +383,10 @@ func TestBuilderWithoutSigning(t *testing.T) { require.NoError(t, err) require.NotNil(t, manifest.pbOuter) - assert.Empty(t, manifest.pbOuter.Signature, "signature should be empty when not signing") - assert.Empty(t, manifest.pbOuter.Signer, "signer should be empty when not signing") - assert.Empty(t, manifest.pbOuter.SigningPubKey, "signing public key should be empty when not signing") + assert.Empty(t, manifest.pbOuter.GetSignature(), + "signature should be empty when not signing") + assert.Empty(t, manifest.pbOuter.GetSigner(), + "signer should be empty when not signing") + assert.Empty(t, manifest.pbOuter.GetSigningPubKey(), + "signing public key should be empty when not signing") } diff --git a/mfer/manifest.go b/mfer/manifest.go index bea4fa1..7e847d1 100644 --- a/mfer/manifest.go +++ b/mfer/manifest.go @@ -9,9 +9,18 @@ import ( "github.com/multiformats/go-multihash" ) +var ( + errOuterNotSet = errors.New("pbOuter not set") + errUUIDNotSet = errors.New("UUID not set") + errSHA256NotSet = errors.New("SHA256 hash not set") +) + // manifest holds the internal representation of a manifest file. -// Use NewManifestFromFile or NewManifestFromReader to load an existing manifest, -// or use Builder to create a new one. +// Use NewManifestFromFile or NewManifestFromReader to load an existing +// manifest, or use Builder to create a new one. +// +// Whether this type should be exported is an open design question owned by +// the repository owner; see README design question 13. type manifest struct { pbInner *MFFile pbOuter *MFFileOuter @@ -23,8 +32,9 @@ type manifest struct { func (m *manifest) String() string { count := 0 if m.pbInner != nil { - count = len(m.pbInner.Files) + count = len(m.pbInner.GetFiles()) } + return fmt.Sprintf("", count) } @@ -33,7 +43,8 @@ func (m *manifest) Files() []*MFFilePath { if m.pbInner == nil { return nil } - return m.pbInner.Files + + return m.pbInner.GetFiles() } // signatureString generates the canonical string used for signing/verification. @@ -41,20 +52,24 @@ func (m *manifest) Files() []*MFFilePath { // Requires pbOuter to be set with Uuid and Sha256 fields. func (m *manifest) signatureString() (string, error) { if m.pbOuter == nil { - return "", errors.New("pbOuter not set") - } - if len(m.pbOuter.Uuid) == 0 { - return "", errors.New("UUID not set") - } - if len(m.pbOuter.Sha256) == 0 { - return "", errors.New("SHA256 hash not set") + return "", errOuterNotSet } - mh, err := multihash.Encode(m.pbOuter.Sha256, multihash.SHA2_256) + if len(m.pbOuter.GetUuid()) == 0 { + return "", errUUIDNotSet + } + + if len(m.pbOuter.GetSha256()) == 0 { + return "", errSHA256NotSet + } + + mh, err := multihash.Encode(m.pbOuter.GetSha256(), multihash.SHA2_256) if err != nil { return "", fmt.Errorf("failed to encode multihash: %w", err) } - uuidStr := hex.EncodeToString(m.pbOuter.Uuid) + + uuidStr := hex.EncodeToString(m.pbOuter.GetUuid()) mhStr := hex.EncodeToString(mh) + return fmt.Sprintf("%s-%s-%s", MAGIC, uuidStr, mhStr), nil } diff --git a/mfer/scanner.go b/mfer/scanner.go index abf845d..8d83051 100644 --- a/mfer/scanner.go +++ b/mfer/scanner.go @@ -43,12 +43,20 @@ type ScanStatus struct { // ScannerOptions configures scanner behavior. type ScannerOptions struct { - IncludeDotfiles bool // Include files and directories starting with a dot (default: exclude) - FollowSymLinks bool // Resolve symlinks instead of skipping them - IncludeTimestamps bool // Include createdAt timestamp in manifest (default: omit for determinism) - Fs afero.Fs // Filesystem to use, defaults to OsFs if nil - SigningOptions *SigningOptions // GPG signing options (nil = no signing) - Seed string // If set, derive a deterministic UUID from this seed + // IncludeDotfiles includes files and directories starting with a dot + // (default: exclude). + IncludeDotfiles bool + // FollowSymLinks resolves symlinks instead of skipping them. + FollowSymLinks bool + // IncludeTimestamps includes a createdAt timestamp in the manifest + // (default: omit for determinism). + IncludeTimestamps bool + // Fs is the filesystem to use, defaults to OsFs if nil. + Fs afero.Fs + // SigningOptions holds GPG signing options (nil = no signing). + SigningOptions *SigningOptions + // Seed, if set, derives a deterministic UUID from this seed. + Seed string } // FileEntry represents a file that has been enumerated. @@ -79,10 +87,12 @@ func NewScannerWithOptions(opts *ScannerOptions) *Scanner { if opts == nil { opts = &ScannerOptions{} } + fs := opts.Fs if fs == nil { fs = afero.NewOsFs() } + return &Scanner{ files: make([]*FileEntry, 0), options: opts, @@ -96,47 +106,63 @@ func (s *Scanner) EnumerateFile(filePath string) error { if err != nil { return err } + info, err := s.fs.Stat(abs) if err != nil { return err } // For single files, use the filename as the relative path basePath := filepath.Dir(abs) + return s.enumerateFileWithInfo(filepath.Base(abs), basePath, info, nil) } // EnumeratePath walks a directory path and adds all files to the scanner. // If progress is non-nil, status updates are sent as files are discovered. // The progress channel is closed when the method returns. -func (s *Scanner) EnumeratePath(inputPath string, progress chan<- EnumerateStatus) error { +func (s *Scanner) EnumeratePath( + inputPath string, + progress chan<- EnumerateStatus, +) error { if progress != nil { defer close(progress) } + abs, err := filepath.Abs(inputPath) if err != nil { return err } + afs := afero.NewReadOnlyFs(afero.NewBasePathFs(s.fs, abs)) + return s.enumerateFS(afs, abs, progress) } // EnumeratePaths walks multiple directory paths and adds all files to the scanner. // If progress is non-nil, status updates are sent as files are discovered. // The progress channel is closed when the method returns. -func (s *Scanner) EnumeratePaths(progress chan<- EnumerateStatus, inputPaths ...string) error { +func (s *Scanner) EnumeratePaths( + progress chan<- EnumerateStatus, + inputPaths ...string, +) error { if progress != nil { defer close(progress) } + for _, p := range inputPaths { abs, err := filepath.Abs(p) if err != nil { return err } + afs := afero.NewReadOnlyFs(afero.NewBasePathFs(s.fs, abs)) - if err := s.enumerateFS(afs, abs, progress); err != nil { + + err = s.enumerateFS(afs, abs, progress) + if err != nil { return err } } + return nil } @@ -144,31 +170,231 @@ func (s *Scanner) EnumeratePaths(progress chan<- EnumerateStatus, inputPaths ... // If progress is non-nil, status updates are sent as files are discovered. // The progress channel is closed when the method returns. // basePath is used to compute absolute paths for file reading. -func (s *Scanner) EnumerateFS(afs afero.Fs, basePath string, progress chan<- EnumerateStatus) error { +func (s *Scanner) EnumerateFS( + afs afero.Fs, + basePath string, + progress chan<- EnumerateStatus, +) error { if progress != nil { defer close(progress) } + return s.enumerateFS(afs, basePath, progress) } -// enumerateFS is the internal implementation that doesn't close the progress channel. -func (s *Scanner) enumerateFS(afs afero.Fs, basePath string, progress chan<- EnumerateStatus) error { +// Files returns a copy of all files added to the scanner. +func (s *Scanner) Files() []*FileEntry { + s.mu.RLock() + defer s.mu.RUnlock() + + out := make([]*FileEntry, len(s.files)) + copy(out, s.files) + + return out +} + +// FileCount returns the number of files in the scanner. +func (s *Scanner) FileCount() FileCount { + s.mu.RLock() + defer s.mu.RUnlock() + + return FileCount(len(s.files)) +} + +// TotalBytes returns the total size of all files in the scanner. +func (s *Scanner) TotalBytes() FileSize { + s.mu.RLock() + defer s.mu.RUnlock() + + return s.totalBytes +} + +// ToManifest reads all file contents, computes hashes, and generates a manifest. +// If progress is non-nil, status updates are sent approximately once per second. +// The progress channel is closed when the method returns. +// The manifest is written to the provided io.Writer. +func (s *Scanner) ToManifest( + ctx context.Context, w io.Writer, progress chan<- ScanStatus, +) error { + if progress != nil { + defer close(progress) + } + + s.mu.RLock() + files := make([]*FileEntry, len(s.files)) + copy(files, s.files) + totalFiles := FileCount(len(files)) + + var totalBytes FileSize + for _, f := range files { + totalBytes += f.Size + } + + s.mu.RUnlock() + + builder := s.configureBuilder() + + var ( + scannedFiles FileCount + scannedBytes FileSize + ) + + lastProgressTime := time.Now() + startTime := time.Now() + + pt := &scanProgressTracker{ + progress: progress, + totalFiles: totalFiles, + totalBytes: totalBytes, + startTime: startTime, + lastProgress: &lastProgressTime, + } + + for _, entry := range files { + // Check for cancellation + select { + case <-ctx.Done(): + return ctx.Err() + default: + } + + bytesRead, err := s.scanFile(builder, pt, entry, scannedFiles, scannedBytes) + if err != nil { + return err + } + + scannedFiles++ + scannedBytes += bytesRead + } + + // Send final progress (ETA is 0 at completion; remaining bytes are 0, + // so computeRateETA yields eta 0 and the same average rate as before) + if progress != nil { + rate, _ := computeRateETA(time.Since(startTime), scannedBytes, totalBytes) + + sendScanStatus(progress, ScanStatus{ + TotalFiles: totalFiles, + ScannedFiles: scannedFiles, + TotalBytes: totalBytes, + ScannedBytes: scannedBytes, + BytesPerSec: rate, + ETA: 0, + }) + } + + // Build and write manifest + //nolint:contextcheck // Build's GPG signing exec is not cancellable by design + return builder.Build(w) +} + +// configureBuilder constructs a manifest builder configured from the +// scanner options. +func (s *Scanner) configureBuilder() *Builder { + builder := NewBuilder() + if s.options.IncludeTimestamps { + builder.SetIncludeTimestamps(true) + } + + if s.options.SigningOptions != nil { + builder.SetSigningOptions(s.options.SigningOptions) + } + + if s.options.Seed != "" { + builder.SetSeed(s.options.Seed) + } + + return builder +} + +// scanFile hashes a single file into the builder, forwarding per-file +// progress updates, and returns the number of bytes read. +func (s *Scanner) scanFile( + builder *Builder, + pt *scanProgressTracker, + entry *FileEntry, + scannedFiles FileCount, + scannedBytes FileSize, +) (FileSize, error) { + // Open file + f, err := s.fs.Open(string(entry.AbsPath)) + if err != nil { + return 0, err + } + + // Create progress channel for this file + var ( + fileProgress chan FileHashProgress + wg sync.WaitGroup + ) + + if pt.progress != nil { + fileProgress = make(chan FileHashProgress, 1) + + wg.Add(1) + + go func(base FileSize, done FileCount) { + defer wg.Done() + + pt.forward(fileProgress, done, base) + }(scannedBytes, scannedFiles) + } + + // Add to manifest with progress channel + bytesRead, err := builder.AddFile( + entry.Path, + entry.Size, + entry.Mtime, + f, + fileProgress, + ) + _ = f.Close() + + // Close channel and wait for goroutine to finish + if fileProgress != nil { + close(fileProgress) + wg.Wait() + } + + if err != nil { + return 0, err + } + + log.Verbosef("+ %s (%s)", entry.Path, humanize.IBytes(sizeToUint64(bytesRead))) + + return bytesRead, nil +} + +// enumerateFS is the internal implementation that doesn't close the +// progress channel. +func (s *Scanner) enumerateFS( + afs afero.Fs, + basePath string, + progress chan<- EnumerateStatus, +) error { return afero.Walk(afs, "/", func(p string, info fs.FileInfo, err error) error { if err != nil { return err } + if !s.options.IncludeDotfiles && IsHiddenPath(p) { if info.IsDir() { return filepath.SkipDir } + return nil } + return s.enumerateFileWithInfo(p, basePath, info, progress) }) } // enumerateFileWithInfo adds a file with pre-existing fs.FileInfo. -func (s *Scanner) enumerateFileWithInfo(filePath string, basePath string, info fs.FileInfo, progress chan<- EnumerateStatus) error { +func (s *Scanner) enumerateFileWithInfo( + filePath string, + basePath string, + info fs.FileInfo, + progress chan<- EnumerateStatus, +) error { if info.IsDir() { // Manifests contain only files, directories are implied return nil @@ -193,11 +419,13 @@ func (s *Scanner) enumerateFileWithInfo(filePath string, basePath string, info f realPath, err := filepath.EvalSymlinks(absPath) if err != nil { // Skip broken symlinks - return nil + return nil //nolint:nilerr // broken symlinks are skipped by design } + realInfo, err := s.fs.Stat(realPath) if err != nil { - return nil + // Skip symlinks whose target cannot be stat'd + return nil //nolint:nilerr // unreadable targets are skipped by design } // Skip if symlink points to a directory if realInfo.IsDir() { @@ -232,160 +460,78 @@ func (s *Scanner) enumerateFileWithInfo(filePath string, basePath string, info f return nil } -// Files returns a copy of all files added to the scanner. -func (s *Scanner) Files() []*FileEntry { - s.mu.RLock() - defer s.mu.RUnlock() - out := make([]*FileEntry, len(s.files)) - copy(out, s.files) - return out +// scanProgressTracker carries the shared state needed to report rate-limited +// scan progress updates. +type scanProgressTracker struct { + progress chan<- ScanStatus + totalFiles FileCount + totalBytes FileSize + startTime time.Time + lastProgress *time.Time } -// FileCount returns the number of files in the scanner. -func (s *Scanner) FileCount() FileCount { - s.mu.RLock() - defer s.mu.RUnlock() - return FileCount(len(s.files)) -} - -// TotalBytes returns the total size of all files in the scanner. -func (s *Scanner) TotalBytes() FileSize { - s.mu.RLock() - defer s.mu.RUnlock() - return s.totalBytes -} - -// ToManifest reads all file contents, computes hashes, and generates a manifest. -// If progress is non-nil, status updates are sent approximately once per second. -// The progress channel is closed when the method returns. -// The manifest is written to the provided io.Writer. -func (s *Scanner) ToManifest(ctx context.Context, w io.Writer, progress chan<- ScanStatus) error { - if progress != nil { - defer close(progress) - } - - s.mu.RLock() - files := make([]*FileEntry, len(s.files)) - copy(files, s.files) - totalFiles := FileCount(len(files)) - var totalBytes FileSize - for _, f := range files { - totalBytes += f.Size - } - s.mu.RUnlock() - - builder := NewBuilder() - if s.options.IncludeTimestamps { - builder.SetIncludeTimestamps(true) - } - if s.options.SigningOptions != nil { - builder.SetSigningOptions(s.options.SigningOptions) - } - if s.options.Seed != "" { - builder.SetSeed(s.options.Seed) - } - - var scannedFiles FileCount - var scannedBytes FileSize - lastProgressTime := time.Now() - startTime := time.Now() - - for _, entry := range files { - // Check for cancellation - select { - case <-ctx.Done(): - return ctx.Err() - default: +// forward relays per-file hash progress to the scan progress channel, +// rate-limited to one update per second. +func (pt *scanProgressTracker) forward( + fileProgress <-chan FileHashProgress, + scannedFiles FileCount, + baseBytes FileSize, +) { + for p := range fileProgress { + // Send progress at most once per second + now := time.Now() + if now.Sub(*pt.lastProgress) < time.Second { + continue } - // Open file - f, err := s.fs.Open(string(entry.AbsPath)) - if err != nil { - return err - } + currentBytes := baseBytes + p.BytesRead + rate, eta := computeRateETA(now.Sub(pt.startTime), currentBytes, pt.totalBytes) - // Create progress channel for this file - var fileProgress chan FileHashProgress - var wg sync.WaitGroup - if progress != nil { - fileProgress = make(chan FileHashProgress, 1) - wg.Add(1) - go func(baseScannedBytes FileSize) { - defer wg.Done() - for p := range fileProgress { - // Send progress at most once per second - now := time.Now() - if now.Sub(lastProgressTime) >= time.Second { - elapsed := now.Sub(startTime).Seconds() - currentBytes := baseScannedBytes + p.BytesRead - var rate float64 - var eta time.Duration - if elapsed > 0 && currentBytes > 0 { - rate = float64(currentBytes) / elapsed - remainingBytes := totalBytes - currentBytes - if rate > 0 { - eta = time.Duration(float64(remainingBytes)/rate) * time.Second - } - } - sendScanStatus(progress, ScanStatus{ - TotalFiles: totalFiles, - ScannedFiles: scannedFiles, - TotalBytes: totalBytes, - ScannedBytes: currentBytes, - BytesPerSec: rate, - ETA: eta, - }) - lastProgressTime = now - } - } - }(scannedBytes) - } - - // Add to manifest with progress channel - bytesRead, err := builder.AddFile( - entry.Path, - entry.Size, - entry.Mtime, - f, - fileProgress, - ) - _ = f.Close() - - // Close channel and wait for goroutine to finish - if fileProgress != nil { - close(fileProgress) - wg.Wait() - } - - if err != nil { - return err - } - - log.Verbosef("+ %s (%s)", entry.Path, humanize.IBytes(uint64(bytesRead))) - - scannedFiles++ - scannedBytes += bytesRead - } - - // Send final progress (ETA is 0 at completion) - if progress != nil { - elapsed := time.Since(startTime).Seconds() - var rate float64 - if elapsed > 0 { - rate = float64(scannedBytes) / elapsed - } - sendScanStatus(progress, ScanStatus{ - TotalFiles: totalFiles, + sendScanStatus(pt.progress, ScanStatus{ + TotalFiles: pt.totalFiles, ScannedFiles: scannedFiles, - TotalBytes: totalBytes, - ScannedBytes: scannedBytes, + TotalBytes: pt.totalBytes, + ScannedBytes: currentBytes, BytesPerSec: rate, - ETA: 0, + ETA: eta, }) + + *pt.lastProgress = now + } +} + +// computeRateETA returns the average throughput over elapsed time and the +// estimated time to process the remaining bytes at that rate. +func computeRateETA( + elapsed time.Duration, + done FileSize, + total FileSize, +) (float64, time.Duration) { + var ( + rate float64 + eta time.Duration + ) + + if elapsed > 0 && done > 0 { + rate = float64(done) / elapsed.Seconds() + + remaining := total - done + if rate > 0 { + eta = time.Duration(float64(remaining)/rate) * time.Second + } } - // Build and write manifest - return builder.Build(w) + return rate, eta +} + +// sizeToUint64 converts a FileSize to uint64 for display, clamping +// negative values to zero so the conversion cannot overflow. +func sizeToUint64(v FileSize) uint64 { + if v < 0 { + return 0 + } + + return uint64(v) } // IsHiddenPath returns true if the path or any of its parent directories @@ -396,17 +542,21 @@ func IsHiddenPath(p string) bool { if tp == "." || tp == "/" { return false } + if strings.HasPrefix(tp, ".") { return true } + for { d, f := path.Split(tp) if strings.HasPrefix(f, ".") { return true } + if d == "" { return false } + tp = d[0 : len(d)-1] // trim trailing slash from dir } } @@ -417,6 +567,7 @@ func sendEnumerateStatus(ch chan<- EnumerateStatus, status EnumerateStatus) { if ch == nil { return } + select { case ch <- status: default: @@ -430,6 +581,7 @@ func sendScanStatus(ch chan<- ScanStatus, status ScanStatus) { if ch == nil { return } + select { case ch <- status: default: diff --git a/mfer/scanner_test.go b/mfer/scanner_test.go index 8db6357..bcdbb27 100644 --- a/mfer/scanner_test.go +++ b/mfer/scanner_test.go @@ -1,3 +1,4 @@ +//nolint:testpackage // white-box tests exercise unexported internals package mfer import ( @@ -12,6 +13,8 @@ import ( ) func TestNewScanner(t *testing.T) { + t.Parallel() + s := NewScanner() assert.NotNil(t, s) assert.Equal(t, FileCount(0), s.FileCount()) @@ -19,12 +22,18 @@ func TestNewScanner(t *testing.T) { } func TestNewScannerWithOptions(t *testing.T) { + t.Parallel() + t.Run("nil options", func(t *testing.T) { + t.Parallel() + s := NewScannerWithOptions(nil) assert.NotNil(t, s) }) t.Run("with options", func(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() opts := &ScannerOptions{ IncludeDotfiles: true, @@ -37,6 +46,8 @@ func TestNewScannerWithOptions(t *testing.T) { } func TestScannerEnumerateFile(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() require.NoError(t, afero.WriteFile(fs, "/test.txt", []byte("hello world"), 0o644)) @@ -54,6 +65,8 @@ func TestScannerEnumerateFile(t *testing.T) { } func TestScannerEnumerateFileMissing(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() s := NewScannerWithOptions(&ScannerOptions{Fs: fs}) err := s.EnumerateFile("/nonexistent.txt") @@ -61,11 +74,14 @@ func TestScannerEnumerateFileMissing(t *testing.T) { } func TestScannerEnumeratePath(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() require.NoError(t, fs.MkdirAll("/testdir/subdir", 0o755)) require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("one"), 0o644)) require.NoError(t, afero.WriteFile(fs, "/testdir/file2.txt", []byte("two"), 0o644)) - require.NoError(t, afero.WriteFile(fs, "/testdir/subdir/file3.txt", []byte("three"), 0o644)) + require.NoError(t, + afero.WriteFile(fs, "/testdir/subdir/file3.txt", []byte("three"), 0o644)) s := NewScannerWithOptions(&ScannerOptions{Fs: fs}) err := s.EnumeratePath("/testdir", nil) @@ -76,6 +92,8 @@ func TestScannerEnumeratePath(t *testing.T) { } func TestScannerEnumeratePathWithProgress(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() require.NoError(t, fs.MkdirAll("/testdir", 0o755)) require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("one"), 0o644)) @@ -100,6 +118,8 @@ func TestScannerEnumeratePathWithProgress(t *testing.T) { } func TestScannerEnumeratePaths(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() require.NoError(t, fs.MkdirAll("/dir1", 0o755)) require.NoError(t, fs.MkdirAll("/dir2", 0o755)) @@ -114,13 +134,20 @@ func TestScannerEnumeratePaths(t *testing.T) { } func TestScannerExcludeDotfiles(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() require.NoError(t, fs.MkdirAll("/testdir/.hidden", 0o755)) - require.NoError(t, afero.WriteFile(fs, "/testdir/visible.txt", []byte("visible"), 0o644)) - require.NoError(t, afero.WriteFile(fs, "/testdir/.hidden.txt", []byte("hidden"), 0o644)) - require.NoError(t, afero.WriteFile(fs, "/testdir/.hidden/inside.txt", []byte("inside"), 0o644)) + require.NoError(t, + afero.WriteFile(fs, "/testdir/visible.txt", []byte("visible"), 0o644)) + require.NoError(t, + afero.WriteFile(fs, "/testdir/.hidden.txt", []byte("hidden"), 0o644)) + require.NoError(t, + afero.WriteFile(fs, "/testdir/.hidden/inside.txt", []byte("inside"), 0o644)) t.Run("exclude by default", func(t *testing.T) { + t.Parallel() + s := NewScannerWithOptions(&ScannerOptions{Fs: fs, IncludeDotfiles: false}) err := s.EnumeratePath("/testdir", nil) require.NoError(t, err) @@ -131,6 +158,8 @@ func TestScannerExcludeDotfiles(t *testing.T) { }) t.Run("include when enabled", func(t *testing.T) { + t.Parallel() + s := NewScannerWithOptions(&ScannerOptions{Fs: fs, IncludeDotfiles: true}) err := s.EnumeratePath("/testdir", nil) require.NoError(t, err) @@ -140,34 +169,43 @@ func TestScannerExcludeDotfiles(t *testing.T) { } func TestScannerToManifest(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() require.NoError(t, fs.MkdirAll("/testdir", 0o755)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file1.txt", []byte("content one"), 0o644)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file2.txt", []byte("content two"), 0o644)) + require.NoError(t, + afero.WriteFile(fs, "/testdir/file1.txt", []byte("content one"), 0o644)) + require.NoError(t, + afero.WriteFile(fs, "/testdir/file2.txt", []byte("content two"), 0o644)) s := NewScannerWithOptions(&ScannerOptions{Fs: fs}) err := s.EnumeratePath("/testdir", nil) require.NoError(t, err) var buf bytes.Buffer + err = s.ToManifest(context.Background(), &buf, nil) require.NoError(t, err) // Manifest should have magic bytes - assert.True(t, buf.Len() > 0) + assert.Positive(t, buf.Len()) assert.Equal(t, MAGIC, string(buf.Bytes()[:8])) } func TestScannerToManifestWithProgress(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() require.NoError(t, fs.MkdirAll("/testdir", 0o755)) - require.NoError(t, afero.WriteFile(fs, "/testdir/file.txt", bytes.Repeat([]byte("x"), 1000), 0o644)) + require.NoError(t, + afero.WriteFile(fs, "/testdir/file.txt", bytes.Repeat([]byte("x"), 1000), 0o644)) s := NewScannerWithOptions(&ScannerOptions{Fs: fs}) err := s.EnumeratePath("/testdir", nil) require.NoError(t, err) var buf bytes.Buffer + progress := make(chan ScanStatus, 10) err = s.ToManifest(context.Background(), &buf, progress) @@ -188,12 +226,15 @@ func TestScannerToManifestWithProgress(t *testing.T) { } func TestScannerToManifestContextCancellation(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() require.NoError(t, fs.MkdirAll("/testdir", 0o755)) // Create many files to ensure we have time to cancel - for i := 0; i < 100; i++ { + for i := range 100 { name := string(rune('a'+i%26)) + string(rune('0'+i/26)) + ".txt" - require.NoError(t, afero.WriteFile(fs, "/testdir/"+name, bytes.Repeat([]byte("x"), 100), 0o644)) + require.NoError(t, + afero.WriteFile(fs, "/testdir/"+name, bytes.Repeat([]byte("x"), 100), 0o644)) } s := NewScannerWithOptions(&ScannerOptions{Fs: fs}) @@ -204,24 +245,30 @@ func TestScannerToManifestContextCancellation(t *testing.T) { cancel() // Cancel immediately var buf bytes.Buffer + err = s.ToManifest(ctx, &buf, nil) assert.ErrorIs(t, err, context.Canceled) } func TestScannerToManifestEmptyScanner(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() s := NewScannerWithOptions(&ScannerOptions{Fs: fs}) var buf bytes.Buffer + err := s.ToManifest(context.Background(), &buf, nil) require.NoError(t, err) // Should still produce a valid manifest - assert.True(t, buf.Len() > 0) + assert.Positive(t, buf.Len()) assert.Equal(t, MAGIC, string(buf.Bytes()[:8])) } func TestScannerFilesCopiesSlice(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() require.NoError(t, afero.WriteFile(fs, "/test.txt", []byte("hello"), 0o644)) @@ -236,10 +283,13 @@ func TestScannerFilesCopiesSlice(t *testing.T) { } func TestScannerEnumerateFS(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() require.NoError(t, fs.MkdirAll("/testdir/sub", 0o755)) require.NoError(t, afero.WriteFile(fs, "/testdir/file.txt", []byte("hello"), 0o644)) - require.NoError(t, afero.WriteFile(fs, "/testdir/sub/nested.txt", []byte("world"), 0o644)) + require.NoError(t, + afero.WriteFile(fs, "/testdir/sub/nested.txt", []byte("world"), 0o644)) // Create a basepath filesystem baseFs := afero.NewBasePathFs(fs, "/testdir") @@ -252,13 +302,17 @@ func TestScannerEnumerateFS(t *testing.T) { } func TestSendEnumerateStatusNonBlocking(t *testing.T) { + t.Parallel() + // Channel with no buffer - send should not block ch := make(chan EnumerateStatus) // This should not block done := make(chan bool) + go func() { sendEnumerateStatus(ch, EnumerateStatus{FilesFound: 1}) + done <- true }() @@ -271,12 +325,16 @@ func TestSendEnumerateStatusNonBlocking(t *testing.T) { } func TestSendScanStatusNonBlocking(t *testing.T) { + t.Parallel() + // Channel with no buffer - send should not block ch := make(chan ScanStatus) done := make(chan bool) + go func() { sendScanStatus(ch, ScanStatus{ScannedFiles: 1}) + done <- true }() @@ -289,14 +347,19 @@ func TestSendScanStatusNonBlocking(t *testing.T) { } func TestSendStatusNilChannel(t *testing.T) { + t.Parallel() + // Should not panic with nil channel sendEnumerateStatus(nil, EnumerateStatus{}) sendScanStatus(nil, ScanStatus{}) } func TestScannerFileEntryFields(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() now := time.Now().Truncate(time.Second) + require.NoError(t, afero.WriteFile(fs, "/test.txt", []byte("content"), 0o644)) require.NoError(t, fs.Chtimes("/test.txt", now, now)) @@ -315,11 +378,13 @@ func TestScannerFileEntryFields(t *testing.T) { } func TestScannerLargeFileEnumeration(t *testing.T) { + t.Parallel() + fs := afero.NewMemMapFs() require.NoError(t, fs.MkdirAll("/testdir", 0o755)) // Create 100 files - for i := 0; i < 100; i++ { + for i := range 100 { name := "/testdir/" + string(rune('a'+i%26)) + string(rune('0'+i/26%10)) + ".txt" require.NoError(t, afero.WriteFile(fs, name, []byte("data"), 0o644)) } @@ -330,20 +395,20 @@ func TestScannerLargeFileEnumeration(t *testing.T) { err := s.EnumeratePath("/testdir", progress) require.NoError(t, err) - // Drain channel - for range progress { - } + // progress is fully buffered and closed; no draining needed assert.Equal(t, FileCount(100), s.FileCount()) assert.Equal(t, FileSize(400), s.TotalBytes()) // 100 * 4 bytes } func TestIsHiddenPath(t *testing.T) { + t.Parallel() + tests := []struct { path string hidden bool }{ - {"file.txt", false}, + {testFileName, false}, {".hidden", true}, {"dir/file.txt", false}, {"dir/.hidden", true}, @@ -360,6 +425,8 @@ func TestIsHiddenPath(t *testing.T) { for _, tt := range tests { t.Run(tt.path, func(t *testing.T) { + t.Parallel() + assert.Equal(t, tt.hidden, IsHiddenPath(tt.path), "IsHiddenPath(%q)", tt.path) }) } diff --git a/mfer/serialize.go b/mfer/serialize.go index b60c1c0..487d49b 100644 --- a/mfer/serialize.go +++ b/mfer/serialize.go @@ -5,6 +5,7 @@ import ( "crypto/sha256" "errors" "fmt" + "math" "time" "github.com/google/uuid" @@ -15,47 +16,80 @@ import ( // MAGIC is the file format magic bytes prefix (rot13 of "MANIFEST"). const MAGIC string = "ZNAVSRFG" +var ( + // errInnerNotSet is returned by generate when the inner manifest is + // missing. + errInnerNotSet = errors.New("internal error: pbInner not set") + // errInternal is returned by generateOuter for the same condition. + // The two messages differ, and both are load-bearing for callers that + // match on text, so they are kept distinct. + errInternal = errors.New("internal error") +) + +// nanosecondsInt32 converts t's nanosecond component to int32. +// time.Time.Nanosecond is documented to return a value in [0, 999999999], +// so the conversion cannot overflow. This sits directly in the manifest +// content path: silently substituting a default would zero every entry's +// mtime nanos and change the serialized bytes and their hash, so an +// out-of-contract value is a programming error and panics rather than +// being papered over. +func nanosecondsInt32(t time.Time) int32 { + n := t.Nanosecond() + if n < 0 || n > math.MaxInt32 { + panic(fmt.Sprintf( + "mfer: time.Time.Nanosecond out of contract: %d", n)) + } + + return int32(n) +} + func newTimestampFromTime(t time.Time) *Timestamp { return &Timestamp{ Seconds: t.Unix(), - Nanos: int32(t.Nanosecond()), + Nanos: nanosecondsInt32(t), } } func (m *manifest) generate() error { if m.pbInner == nil { - return errors.New("internal error: pbInner not set") + return errInnerNotSet } + if m.pbOuter == nil { e := m.generateOuter() if e != nil { return e } } + dat, err := proto.MarshalOptions{Deterministic: true}.Marshal(m.pbOuter) if err != nil { return fmt.Errorf("serialize: marshal outer: %w", err) } - m.output = bytes.NewBuffer([]byte(MAGIC)) + + m.output = bytes.NewBufferString(MAGIC) + _, err = m.output.Write(dat) if err != nil { return fmt.Errorf("serialize: write output: %w", err) } + return nil } func (m *manifest) generateOuter() error { if m.pbInner == nil { - return errors.New("internal error") + return errInternal } // Use fixed UUID if provided, otherwise generate a new one var manifestUUID uuid.UUID - if len(m.fixedUUID) == 16 { + if len(m.fixedUUID) == uuidLength { copy(manifestUUID[:], m.fixedUUID) } else { manifestUUID = uuid.New() } + m.pbInner.Uuid = manifestUUID[:] innerData, err := proto.MarshalOptions{Deterministic: true}.Marshal(m.pbInner) @@ -65,23 +99,29 @@ func (m *manifest) generateOuter() error { // Compress the inner data idc := new(bytes.Buffer) + zw, err := zstd.NewWriter(idc, zstd.WithEncoderLevel(zstd.SpeedBestCompression)) if err != nil { return fmt.Errorf("serialize: create compressor: %w", err) } + _, err = zw.Write(innerData) if err != nil { return fmt.Errorf("serialize: compress: %w", err) } + _ = zw.Close() compressedData := idc.Bytes() // Hash the compressed data for integrity verification before decompression h := sha256.New() - if _, err := h.Write(compressedData); err != nil { + + _, err = h.Write(compressedData) + if err != nil { return fmt.Errorf("serialize: hash write: %w", err) } + sha256Hash := h.Sum(nil) m.pbOuter = &MFFileOuter{ @@ -95,29 +135,40 @@ func (m *manifest) generateOuter() error { // Sign the manifest if signing options are provided if m.signingOptions != nil && m.signingOptions.KeyID != "" { - sigString, err := m.signatureString() - if err != nil { - return fmt.Errorf("failed to generate signature string: %w", err) - } - - sig, err := gpgSign([]byte(sigString), m.signingOptions.KeyID) - if err != nil { - return fmt.Errorf("failed to sign manifest: %w", err) - } - m.pbOuter.Signature = sig - - fingerprint, err := gpgGetKeyFingerprint(m.signingOptions.KeyID) - if err != nil { - return fmt.Errorf("failed to get key fingerprint: %w", err) - } - m.pbOuter.Signer = fingerprint - - pubKey, err := gpgExportPublicKey(m.signingOptions.KeyID) - if err != nil { - return fmt.Errorf("failed to export public key: %w", err) - } - m.pbOuter.SigningPubKey = pubKey + return m.signOuter() } return nil } + +// signOuter signs the outer message with the configured GPG key and +// embeds the signature, signer fingerprint, and public key. +func (m *manifest) signOuter() error { + sigString, err := m.signatureString() + if err != nil { + return fmt.Errorf("failed to generate signature string: %w", err) + } + + sig, err := gpgSign([]byte(sigString), m.signingOptions.KeyID) + if err != nil { + return fmt.Errorf("failed to sign manifest: %w", err) + } + + m.pbOuter.Signature = sig + + fingerprint, err := gpgGetKeyFingerprint(m.signingOptions.KeyID) + if err != nil { + return fmt.Errorf("failed to get key fingerprint: %w", err) + } + + m.pbOuter.Signer = fingerprint + + pubKey, err := gpgExportPublicKey(m.signingOptions.KeyID) + if err != nil { + return fmt.Errorf("failed to export public key: %w", err) + } + + m.pbOuter.SigningPubKey = pubKey + + return nil +} diff --git a/mfer/url.go b/mfer/url.go index 274687e..50bce8d 100644 --- a/mfer/url.go +++ b/mfer/url.go @@ -32,12 +32,14 @@ func (b BaseURL) JoinPath(path RelFilePath) (FileURL, error) { for i, seg := range segments { segments[i] = url.PathEscape(seg) } + ref, err := url.Parse(strings.Join(segments, "/")) if err != nil { return "", err } resolved := base.ResolveReference(ref) + return FileURL(resolved.String()), nil } diff --git a/mfer/url_test.go b/mfer/url_test.go index dd36a4a..f22c3fb 100644 --- a/mfer/url_test.go +++ b/mfer/url_test.go @@ -1,3 +1,4 @@ +//nolint:testpackage // white-box tests exercise unexported internals package mfer import ( @@ -8,19 +9,27 @@ import ( ) func TestBaseURLJoinPath(t *testing.T) { + t.Parallel() + tests := []struct { base BaseURL path RelFilePath expected string }{ - {"https://example.com/dir/", "file.txt", "https://example.com/dir/file.txt"}, - {"https://example.com/dir", "file.txt", "https://example.com/dir/file.txt"}, + {"https://example.com/dir/", testFileName, "https://example.com/dir/file.txt"}, + {"https://example.com/dir", testFileName, "https://example.com/dir/file.txt"}, {"https://example.com/", "sub/file.txt", "https://example.com/sub/file.txt"}, - {"https://example.com/dir/", "file with spaces.txt", "https://example.com/dir/file%20with%20spaces.txt"}, + { + "https://example.com/dir/", + "file with spaces.txt", + "https://example.com/dir/file%20with%20spaces.txt", + }, } for _, tt := range tests { t.Run(string(tt.base)+"+"+string(tt.path), func(t *testing.T) { + t.Parallel() + result, err := tt.base.JoinPath(tt.path) require.NoError(t, err) assert.Equal(t, tt.expected, string(result)) @@ -29,16 +38,22 @@ func TestBaseURLJoinPath(t *testing.T) { } func TestBaseURLString(t *testing.T) { + t.Parallel() + b := BaseURL("https://example.com/") assert.Equal(t, "https://example.com/", b.String()) } func TestFileURLString(t *testing.T) { + t.Parallel() + f := FileURL("https://example.com/file.txt") assert.Equal(t, "https://example.com/file.txt", f.String()) } func TestManifestURLString(t *testing.T) { + t.Parallel() + m := ManifestURL("https://example.com/index.mf") assert.Equal(t, "https://example.com/index.mf", m.String()) }