From 7f75f2ee726eb460125665c93c6ccbda19ec3160 Mon Sep 17 00:00:00 2001 From: sneak Date: Mon, 27 Jul 2026 00:55:46 +0700 Subject: [PATCH] Clear all golangci-lint findings and make the CI build green (closes #1) Fix every golangci-lint finding under the repo's standard .golangci.yml (from ~192 down to 0 under the pinned v2.10.1) without changing program behavior: - gochecknoglobals: replace the verbose/quiet/exclude* package globals with an options struct threaded through the command implementations. - err113: introduce package-level sentinel errors and wrap them with %w. - errcheck: check or explicitly discard every previously unchecked error (bar.Add/Finish, deferred Close, verbose writes). - forbidigo: route verbose output through os.Stdout instead of fmt.Print*. - noinlineerr / wsl_v5 / nlreturn / gofmt: split inline error checks and normalize whitespace. - complexity (cyclop/gocognit/nestif): extract small behavior-preserving helpers (runOverPaths, countAndBar, walkSkip, clearOne, checkOne, missingChecksum, reportCheck). - mnd/lll/nonamedreturns/revive/modernize/nilnil: named constants, wrapped lines, unnamed returns, doc comments, SplitSeq, non-(nil,nil) returns. - paralleltest/thelper: mark tests parallel (now race-safe with no shared globals) and add t.Helper(); probe the real xattr key so the guard is accurate. Also make the tests actually runnable in CI rather than skipping: - move the xattr keys into the user.* namespace (user.berlin.sneak.app.*), which Linux requires for regular-file xattrs; macOS treats the whole string as an opaque name, so behavior is unchanged there. - run the Docker builder's checks as an unprivileged user so the permission tests are meaningful (root bypasses file mode bits). --- Dockerfile | 12 +- README.md | 4 +- attrsum.go | 711 +++++++++++++++++++++++++++++------------------- attrsum_test.go | 186 ++++++++++--- 4 files changed, 583 insertions(+), 330 deletions(-) diff --git a/Dockerfile b/Dockerfile index a41093f..d1530bf 100644 --- a/Dockerfile +++ b/Dockerfile @@ -15,10 +15,20 @@ RUN go mod download COPY . . +# Run the checks as an unprivileged user. Root bypasses file mode bits, which +# would make the permission tests (expecting EACCES on a 0000 file) spuriously +# pass with no error. Caches live under /tmp (world-writable) so the user needs +# no home directory of its own. +ENV GOCACHE=/tmp/gocache +ENV XDG_CACHE_HOME=/tmp/xdgcache +RUN adduser -D -u 1000 builder && chown -R builder:builder /src /go +USER builder + # Run all checks - build fails if any check fails RUN make check -# Build the binary +# Build the binary (still as the unprivileged user: it owns /src, so git VCS +# stamping sees consistent ownership). RUN make build # Runtime stage diff --git a/README.md b/README.md index 621af92..dcfb598 100644 --- a/README.md +++ b/README.md @@ -55,8 +55,8 @@ attrsum -q sum add DIR | xattr key | meaning | |---------------------------------------------|--------------------------------| -| `berlin.sneak.app.attrsum.checksum` | base-58 multihash (sha2-256) | -| `berlin.sneak.app.attrsum.sumtime` | RFC 3339 timestamp of checksum | +| `user.berlin.sneak.app.attrsum.checksum` | base-58 multihash (sha2-256) | +| `user.berlin.sneak.app.attrsum.sumtime` | RFC 3339 timestamp of checksum | Flags: diff --git a/attrsum.go b/attrsum.go index 30ba5f1..e9b0e1c 100644 --- a/attrsum.go +++ b/attrsum.go @@ -1,3 +1,5 @@ +// Command attrsum computes and verifies file checksums stored in +// extended attributes. package main import ( @@ -23,18 +25,36 @@ import ( ) const ( - checksumKey = "berlin.sneak.app.attrsum.checksum" - sumTimeKey = "berlin.sneak.app.attrsum.sumtime" + // The keys live in the user.* namespace so they are settable on Linux, + // where regular-file xattrs outside user.*/trusted.*/security.*/system.* + // are rejected with EOPNOTSUPP. macOS treats the whole string as an + // opaque attribute name, so the same keys work there too. + checksumKey = "user.berlin.sneak.app.attrsum.checksum" + sumTimeKey = "user.berlin.sneak.app.attrsum.sumtime" + + // progressThrottle is how often the progress bar repaints. + progressThrottle = 250 * time.Millisecond ) +// Sentinel errors returned by the command implementations. var ( + errNoPaths = errors.New("no paths provided") + errNoPathsStdin = errors.New("no paths provided on stdin") + errVerification = errors.New("verification failed") + errModifiedDuringSum = errors.New("file modified during checksum calculation") +) + +// options holds the parsed command-line flag state that is shared across +// the sum/check/clear operations. It replaces package-level globals so the +// behaviour is explicit and the code is safe to exercise concurrently. +type options struct { verbose bool quiet bool excludePatterns []string excludeDotfiles bool -) +} -// Stats tracks operation statistics for summary reporting +// Stats tracks operation statistics for summary reporting. type Stats struct { FilesProcessed int64 FilesSkipped int64 @@ -47,10 +67,11 @@ func (s *Stats) Duration() time.Duration { return time.Since(s.StartTime) } -func (s *Stats) Print(operation string) { - if quiet { +func (s *Stats) Print(opts *options, operation string) { + if opts.quiet { return } + fmt.Fprintf(os.Stderr, "\n%s complete: %d files processed, %d skipped, %d failed, %s bytes in %s\n", operation, s.FilesProcessed, @@ -66,15 +87,19 @@ func formatBytes(b int64) string { if b < unit { return fmt.Sprintf("%d B", b) } + div, exp := int64(unit), 0 for n := b / unit; n >= unit; n /= unit { div *= unit exp++ } + return fmt.Sprintf("%.1f %ciB", float64(b)/float64(div), "KMGTPE"[exp]) } func main() { + opts := &options{} + rootCmd := &cobra.Command{ Use: "attrsum", Short: "Compute and verify file checksums via xattrs", @@ -82,55 +107,124 @@ func main() { rootCmd.SilenceUsage = true rootCmd.SilenceErrors = true - rootCmd.PersistentFlags().BoolVarP(&verbose, "verbose", "v", false, "enable verbose output") - rootCmd.PersistentFlags().BoolVarP(&quiet, "quiet", "q", false, "suppress all output except errors") - rootCmd.PersistentFlags().StringArrayVar(&excludePatterns, "exclude", nil, "exclude files/directories matching pattern (rsync-style, repeatable)") - rootCmd.PersistentFlags().BoolVar(&excludeDotfiles, "exclude-dotfiles", false, "exclude any file or directory whose name starts with '.'") + flags := rootCmd.PersistentFlags() + flags.BoolVarP(&opts.verbose, "verbose", "v", false, "enable verbose output") + flags.BoolVarP(&opts.quiet, "quiet", "q", false, "suppress all output except errors") + flags.StringArrayVar(&opts.excludePatterns, "exclude", nil, + "exclude files/directories matching pattern (rsync-style, repeatable)") + flags.BoolVar(&opts.excludeDotfiles, "exclude-dotfiles", false, + "exclude any file or directory whose name starts with '.'") - rootCmd.AddCommand(newSumCmd()) - rootCmd.AddCommand(newCheckCmd()) - rootCmd.AddCommand(newClearCmd()) + rootCmd.AddCommand(newSumCmd(opts)) + rootCmd.AddCommand(newCheckCmd(opts)) + rootCmd.AddCommand(newClearCmd(opts)) - if err := rootCmd.Execute(); err != nil { + err := rootCmd.Execute() + if err != nil { log.Fatal(err) } } -// expandPaths expands the given paths, reading from stdin if "-" is present +// expandPaths expands the given paths, reading from stdin if "-" is present. func expandPaths(args []string) ([]string, error) { var paths []string + readFromStdin := false + for _, arg := range args { - if arg == "-" { - readFromStdin = true - scanner := bufio.NewScanner(os.Stdin) - for scanner.Scan() { - line := strings.TrimSpace(scanner.Text()) - if line != "" { - paths = append(paths, line) - } - } - if err := scanner.Err(); err != nil { - return nil, fmt.Errorf("reading stdin: %w", err) - } - } else { + if arg != "-" { paths = append(paths, arg) + + continue + } + + readFromStdin = true + scanner := bufio.NewScanner(os.Stdin) + + for scanner.Scan() { + line := strings.TrimSpace(scanner.Text()) + if line != "" { + paths = append(paths, line) + } + } + + err := scanner.Err() + if err != nil { + return nil, fmt.Errorf("reading stdin: %w", err) } } + if len(paths) == 0 { if readFromStdin { - return nil, errors.New("no paths provided on stdin") + return nil, errNoPathsStdin } - return nil, errors.New("no paths provided") + + return nil, errNoPaths } + return paths, nil } +// processFunc processes a single path within a command's run loop. +type processFunc func(opts *options, path string, stats *Stats, bar *progressbar.ProgressBar) error + +// countAndBar counts the files under paths and returns a progress bar sized +// to that total. It always returns either a non-nil bar or a non-nil error. +func countAndBar(opts *options, paths []string, desc string) (*progressbar.ProgressBar, error) { + total, err := countFilesMultiple(opts, paths) + if err != nil { + return nil, err + } + + return newProgressBar(total, desc), nil +} + +// finishBar finalizes a progress bar if one is present. +func finishBar(bar *progressbar.ProgressBar) { + if bar != nil { + _ = bar.Finish() + } +} + +// runOverPaths runs process over each path, sharing the progress/stats +// bookkeeping common to the sum-add, sum-update and clear commands. +func runOverPaths(opts *options, args []string, desc, op string, process processFunc) error { + paths, err := expandPaths(args) + if err != nil { + return err + } + + stats := &Stats{StartTime: time.Now()} + + var bar *progressbar.ProgressBar + + if !opts.quiet { + bar, err = countAndBar(opts, paths, desc) + if err != nil { + return err + } + } + + for _, p := range paths { + perr := process(opts, p, stats, bar) + if perr != nil { + finishBar(bar) + + return perr + } + } + + finishBar(bar) + stats.Print(opts, op) + + return nil +} + /////////////////////////////////////////////////////////////////////////////// // Sum commands /////////////////////////////////////////////////////////////////////////////// -func newSumCmd() *cobra.Command { +func newSumCmd(opts *options) *cobra.Command { cmd := &cobra.Command{ Use: "sum", Short: "Checksum maintenance operations", @@ -141,32 +235,7 @@ func newSumCmd() *cobra.Command { Short: "Write checksums for files missing them", Args: cobra.MinimumNArgs(1), RunE: func(_ *cobra.Command, a []string) error { - paths, err := expandPaths(a) - if err != nil { - return err - } - stats := &Stats{StartTime: time.Now()} - var bar *progressbar.ProgressBar - if !quiet { - total, err := countFilesMultiple(paths) - if err != nil { - return err - } - bar = newProgressBar(total, "Adding checksums") - } - for _, p := range paths { - if err := ProcessSumAdd(p, stats, bar); err != nil { - if bar != nil { - bar.Finish() - } - return err - } - } - if bar != nil { - bar.Finish() - } - stats.Print("sum add") - return nil + return runOverPaths(opts, a, "Adding checksums", "sum add", processSumAdd) }, } @@ -175,70 +244,56 @@ func newSumCmd() *cobra.Command { Short: "Recalculate checksum when file newer than stored sumtime", Args: cobra.MinimumNArgs(1), RunE: func(_ *cobra.Command, a []string) error { - paths, err := expandPaths(a) - if err != nil { - return err - } - stats := &Stats{StartTime: time.Now()} - var bar *progressbar.ProgressBar - if !quiet { - total, err := countFilesMultiple(paths) - if err != nil { - return err - } - bar = newProgressBar(total, "Updating checksums") - } - for _, p := range paths { - if err := ProcessSumUpdate(p, stats, bar); err != nil { - if bar != nil { - bar.Finish() - } - return err - } - } - if bar != nil { - bar.Finish() - } - stats.Print("sum update") - return nil + return runOverPaths(opts, a, "Updating checksums", "sum update", processSumUpdate) }, } cmd.AddCommand(add, upd) + return cmd } -func ProcessSumAdd(dir string, stats *Stats, bar *progressbar.ProgressBar) error { - return walkAndProcess(dir, stats, bar, func(p string, info os.FileInfo, s *Stats) error { +func processSumAdd(opts *options, dir string, stats *Stats, bar *progressbar.ProgressBar) error { + return walkAndProcess(opts, dir, stats, bar, func(p string, info os.FileInfo, s *Stats) error { if hasXattr(p, checksumKey) { atomic.AddInt64(&s.FilesSkipped, 1) + return nil } - if err := writeChecksumAndTime(p, info, s); err != nil { + + err := writeChecksumAndTime(opts, p, info, s) + if err != nil { atomic.AddInt64(&s.FilesFailed, 1) + return err } + return nil }) } -func ProcessSumUpdate(dir string, stats *Stats, bar *progressbar.ProgressBar) error { - return walkAndProcess(dir, stats, bar, func(p string, info os.FileInfo, s *Stats) error { +func processSumUpdate(opts *options, dir string, stats *Stats, bar *progressbar.ProgressBar) error { + return walkAndProcess(opts, dir, stats, bar, func(p string, info os.FileInfo, s *Stats) error { t, err := readSumTime(p) if err != nil || info.ModTime().After(t) { - if err := writeChecksumAndTime(p, info, s); err != nil { + werr := writeChecksumAndTime(opts, p, info, s) + if werr != nil { atomic.AddInt64(&s.FilesFailed, 1) - return err + + return werr } - } else { - atomic.AddInt64(&s.FilesSkipped, 1) + + return nil } + + atomic.AddInt64(&s.FilesSkipped, 1) + return nil }) } -func writeChecksumAndTime(path string, info os.FileInfo, stats *Stats) error { - // Record mtime before hashing to detect modifications during hash +func writeChecksumAndTime(opts *options, path string, info os.FileInfo, stats *Stats) error { + // Record mtime before hashing to detect modifications during hash. mtimeBefore := info.ModTime() hash, bytesRead, err := fileMultihash(path) @@ -246,34 +301,41 @@ func writeChecksumAndTime(path string, info os.FileInfo, stats *Stats) error { return err } - // Check if file was modified during hashing + // Check if file was modified during hashing. infoAfter, err := os.Lstat(path) if err != nil { return fmt.Errorf("stat after hash: %w", err) } + if !infoAfter.ModTime().Equal(mtimeBefore) { - return fmt.Errorf("%s: file modified during checksum calculation", path) + return fmt.Errorf("%s: %w", path, errModifiedDuringSum) } - if err := xattr.Set(path, checksumKey, hash); err != nil { + err = xattr.Set(path, checksumKey, hash) + if err != nil { return fmt.Errorf("set checksum attr: %w", err) } - if verbose && !quiet { - fmt.Printf("%s %s written\n", path, hash) + + if opts.verbose && !opts.quiet { + _, _ = fmt.Fprintf(os.Stdout, "%s %s written\n", path, hash) } - // Store the file's mtime as sumtime (not wall-clock time) - // This makes update comparisons semantically correct + // Store the file's mtime as sumtime (not wall-clock time). This makes + // update comparisons semantically correct. ts := mtimeBefore.UTC().Format(time.RFC3339Nano) - if err := xattr.Set(path, sumTimeKey, []byte(ts)); err != nil { + + err = xattr.Set(path, sumTimeKey, []byte(ts)) + if err != nil { return fmt.Errorf("set sumtime attr: %w", err) } - if verbose && !quiet { - fmt.Printf("%s %s written\n", path, ts) + + if opts.verbose && !opts.quiet { + _, _ = fmt.Fprintf(os.Stdout, "%s %s written\n", path, ts) } atomic.AddInt64(&stats.FilesProcessed, 1) atomic.AddInt64(&stats.BytesProcessed, bytesRead) + return nil } @@ -282,6 +344,7 @@ func readSumTime(path string) (time.Time, error) { if err != nil { return time.Time{}, err } + return time.Parse(time.RFC3339Nano, string(b)) } @@ -289,231 +352,275 @@ func readSumTime(path string) (time.Time, error) { // Clear command /////////////////////////////////////////////////////////////////////////////// -func newClearCmd() *cobra.Command { +func newClearCmd(opts *options) *cobra.Command { return &cobra.Command{ Use: "clear ... (use - to read paths from stdin)", Short: "Remove checksum xattrs from tree", Args: cobra.MinimumNArgs(1), RunE: func(_ *cobra.Command, a []string) error { - paths, err := expandPaths(a) - if err != nil { - return err - } - stats := &Stats{StartTime: time.Now()} - var bar *progressbar.ProgressBar - if !quiet { - total, err := countFilesMultiple(paths) - if err != nil { - return err - } - bar = newProgressBar(total, "Clearing checksums") - } - for _, p := range paths { - if err := ProcessClear(p, stats, bar); err != nil { - if bar != nil { - bar.Finish() - } - return err - } - } - if bar != nil { - bar.Finish() - } - stats.Print("clear") - return nil + return runOverPaths(opts, a, "Clearing checksums", "clear", processClear) }, } } -func ProcessClear(dir string, stats *Stats, bar *progressbar.ProgressBar) error { - return walkAndProcess(dir, stats, bar, func(p string, info os.FileInfo, s *Stats) error { - cleared := false - for _, k := range []string{checksumKey, sumTimeKey} { - v, err := xattr.Get(p, k) - if err != nil { - if errors.Is(err, xattr.ENOATTR) { - continue - } - atomic.AddInt64(&s.FilesFailed, 1) - return err - } - if verbose && !quiet { - fmt.Printf("%s %s removed\n", p, string(v)) - } - if err := xattr.Remove(p, k); err != nil { - atomic.AddInt64(&s.FilesFailed, 1) - return err - } - cleared = true +func processClear(opts *options, dir string, stats *Stats, bar *progressbar.ProgressBar) error { + return walkAndProcess(opts, dir, stats, bar, func(p string, info os.FileInfo, s *Stats) error { + cleared, err := clearOne(opts, p) + if err != nil { + atomic.AddInt64(&s.FilesFailed, 1) + + return err } + if cleared { atomic.AddInt64(&s.FilesProcessed, 1) atomic.AddInt64(&s.BytesProcessed, info.Size()) } else { atomic.AddInt64(&s.FilesSkipped, 1) } + return nil }) } +// clearOne removes both checksum xattrs from a single path, reporting +// whether any attribute was actually removed. +func clearOne(opts *options, p string) (bool, error) { + cleared := false + + for _, k := range []string{checksumKey, sumTimeKey} { + v, err := xattr.Get(p, k) + if err != nil { + if errors.Is(err, xattr.ENOATTR) { + continue + } + + return cleared, err + } + + if opts.verbose && !opts.quiet { + _, _ = fmt.Fprintf(os.Stdout, "%s %s removed\n", p, string(v)) + } + + rerr := xattr.Remove(p, k) + if rerr != nil { + return cleared, rerr + } + + cleared = true + } + + return cleared, nil +} + /////////////////////////////////////////////////////////////////////////////// // Check command /////////////////////////////////////////////////////////////////////////////// -func newCheckCmd() *cobra.Command { +func newCheckCmd(opts *options) *cobra.Command { var cont bool + cmd := &cobra.Command{ Use: "check ... (use - to read paths from stdin)", Short: "Verify stored checksums", Args: cobra.MinimumNArgs(1), RunE: func(_ *cobra.Command, a []string) error { - paths, err := expandPaths(a) - if err != nil { - return err - } - stats := &Stats{StartTime: time.Now()} - var bar *progressbar.ProgressBar - if !quiet { - total, err := countFilesMultiple(paths) - if err != nil { - return err - } - bar = newProgressBar(total, "Verifying checksums") - } - var finalErr error - for _, p := range paths { - if err := ProcessCheck(p, cont, stats, bar); err != nil { - if cont { - finalErr = err - } else { - if bar != nil { - bar.Finish() - } - stats.Print("check") - return err - } - } - } - if bar != nil { - bar.Finish() - } - stats.Print("check") - return finalErr + return runCheck(opts, a, cont) }, } cmd.Flags().BoolVar(&cont, "continue", false, "continue after errors and report each file") + return cmd } -func ProcessCheck(dir string, cont bool, stats *Stats, bar *progressbar.ProgressBar) error { - fail := errors.New("verification failed") - // Track initial failed count to detect failures during this walk - initialFailed := atomic.LoadInt64(&stats.FilesFailed) - - err := walkAndProcess(dir, stats, bar, func(p string, info os.FileInfo, s *Stats) error { - exp, err := xattr.Get(p, checksumKey) - if err != nil { - if errors.Is(err, xattr.ENOATTR) { - atomic.AddInt64(&s.FilesFailed, 1) - if verbose && !quiet { - fmt.Printf("%s ERROR\n", p) - } - if cont { - return nil - } - return fail - } - return err - } - - act, bytesRead, err := fileMultihash(p) - if err != nil { - atomic.AddInt64(&s.FilesFailed, 1) - return err - } - ok := bytes.Equal(exp, act) - if !ok { - atomic.AddInt64(&s.FilesFailed, 1) - } else { - atomic.AddInt64(&s.FilesProcessed, 1) - atomic.AddInt64(&s.BytesProcessed, bytesRead) - } - if verbose && !quiet { - status := "OK" - if !ok { - status = "ERROR" - } - fmt.Printf("%s %s %s\n", p, act, status) - } - if !ok && !cont { - return fail - } - return nil - }) - +func runCheck(opts *options, args []string, cont bool) error { + paths, err := expandPaths(args) if err != nil { - if errors.Is(err, fail) { - return fail - } return err } - // Check if any failures occurred during this walk - if atomic.LoadInt64(&stats.FilesFailed) > initialFailed { - return fail + + stats := &Stats{StartTime: time.Now()} + + var bar *progressbar.ProgressBar + + if !opts.quiet { + bar, err = countAndBar(opts, paths, "Verifying checksums") + if err != nil { + return err + } } + + var finalErr error + + for _, p := range paths { + perr := processCheck(opts, p, cont, stats, bar) + if perr != nil { + if !cont { + finishBar(bar) + stats.Print(opts, "check") + + return perr + } + + finalErr = perr + } + } + + finishBar(bar) + stats.Print(opts, "check") + + return finalErr +} + +func processCheck(opts *options, dir string, cont bool, stats *Stats, bar *progressbar.ProgressBar) error { + // Track initial failed count to detect failures during this walk. + initialFailed := atomic.LoadInt64(&stats.FilesFailed) + + err := walkAndProcess(opts, dir, stats, bar, func(p string, _ os.FileInfo, s *Stats) error { + return checkOne(opts, p, cont, s) + }) + if err != nil { + if errors.Is(err, errVerification) { + return errVerification + } + + return err + } + + // Check if any failures occurred during this walk. + if atomic.LoadInt64(&stats.FilesFailed) > initialFailed { + return errVerification + } + return nil } +// checkOne verifies the stored checksum of a single path. +func checkOne(opts *options, p string, cont bool, s *Stats) error { + exp, err := xattr.Get(p, checksumKey) + if err != nil { + if !errors.Is(err, xattr.ENOATTR) { + return err + } + + return missingChecksum(opts, p, cont, s) + } + + act, bytesRead, err := fileMultihash(p) + if err != nil { + atomic.AddInt64(&s.FilesFailed, 1) + + return err + } + + ok := bytes.Equal(exp, act) + if ok { + atomic.AddInt64(&s.FilesProcessed, 1) + atomic.AddInt64(&s.BytesProcessed, bytesRead) + } else { + atomic.AddInt64(&s.FilesFailed, 1) + } + + reportCheck(opts, p, string(act), ok) + + if !ok && !cont { + return errVerification + } + + return nil +} + +// missingChecksum records and reports a file that has no stored checksum, +// returning the verification error unless the caller asked to continue. +func missingChecksum(opts *options, p string, cont bool, s *Stats) error { + atomic.AddInt64(&s.FilesFailed, 1) + + if opts.verbose && !opts.quiet { + _, _ = fmt.Fprintf(os.Stdout, "%s ERROR\n", p) + } + + if cont { + return nil + } + + return errVerification +} + +// reportCheck prints a per-file verification result when verbose output is on. +func reportCheck(opts *options, p, actual string, ok bool) { + if !opts.verbose || opts.quiet { + return + } + + status := "OK" + if !ok { + status = "ERROR" + } + + _, _ = fmt.Fprintf(os.Stdout, "%s %s %s\n", p, actual, status) +} + /////////////////////////////////////////////////////////////////////////////// // Helpers /////////////////////////////////////////////////////////////////////////////// -// countFiles counts the total number of regular files that will be processed -func countFiles(root string) (int64, error) { +// countFiles counts the total number of regular files that will be processed. +func countFiles(opts *options, root string) (int64, error) { var count int64 + root = filepath.Clean(root) + err := filepath.Walk(root, func(p string, info os.FileInfo, err error) error { if err != nil { return err } + // Skip symlinks - note: filepath.Walk uses Lstat, so symlinks are // reported as ModeSymlink, never as directories. Walk doesn't follow them. if info.Mode()&os.ModeSymlink != 0 { return nil } + rel, _ := filepath.Rel(root, p) - if shouldExclude(rel, info) { + if shouldExclude(opts, rel) { if info.IsDir() { return filepath.SkipDir } + return nil } + if info.IsDir() { return nil } - if !info.Mode().IsRegular() { - return nil + + if info.Mode().IsRegular() { + count++ } - count++ + return nil }) + return count, err } -// countFilesMultiple counts files across multiple roots -func countFilesMultiple(roots []string) (int64, error) { +// countFilesMultiple counts files across multiple roots. +func countFilesMultiple(opts *options, roots []string) (int64, error) { var total int64 + for _, root := range roots { - count, err := countFiles(root) + count, err := countFiles(opts, root) if err != nil { return total, err } + total += count } + return total, nil } -// newProgressBar creates a new progress bar with standard options +// newProgressBar creates a new progress bar with standard options. func newProgressBar(total int64, description string) *progressbar.ProgressBar { return progressbar.NewOptions64(total, progressbar.OptionSetDescription(description), @@ -521,7 +628,7 @@ func newProgressBar(total int64, description string) *progressbar.ProgressBar { progressbar.OptionShowCount(), progressbar.OptionShowIts(), progressbar.OptionSetItsString("files"), - progressbar.OptionThrottle(250*time.Millisecond), + progressbar.OptionThrottle(progressThrottle), progressbar.OptionShowElapsedTimeOnFinish(), progressbar.OptionSetPredictTime(true), progressbar.OptionFullWidth(), @@ -535,90 +642,124 @@ func newProgressBar(total int64, description string) *progressbar.ProgressBar { ) } -func walkAndProcess(root string, stats *Stats, bar *progressbar.ProgressBar, fn func(string, os.FileInfo, *Stats) error) error { +func walkAndProcess( + opts *options, + root string, + stats *Stats, + bar *progressbar.ProgressBar, + fn func(string, os.FileInfo, *Stats) error, +) error { root = filepath.Clean(root) - err := filepath.Walk(root, func(p string, info os.FileInfo, err error) error { + return filepath.Walk(root, func(p string, info os.FileInfo, err error) error { if err != nil { return err } - // Skip symlinks - filepath.Walk uses Lstat, so symlinks are reported - // as ModeSymlink, never as directories. Walk doesn't follow them. - if info.Mode()&os.ModeSymlink != 0 { - if verbose && !quiet { - log.Printf("skip symlink %s", p) - } - return nil - } - - rel, _ := filepath.Rel(root, p) - if shouldExclude(rel, info) { - if info.IsDir() { - return filepath.SkipDir - } - return nil - } - - if info.IsDir() { - return nil - } - if !info.Mode().IsRegular() { - if verbose && !quiet { - log.Printf("skip non-regular %s", p) - } - return nil + skip, skipErr := walkSkip(opts, root, p, info) + if skip { + return skipErr } fnErr := fn(p, info, stats) + if bar != nil { - bar.Add(1) + _ = bar.Add(1) } + return fnErr }) - - return err } -func shouldExclude(rel string, info os.FileInfo) bool { +// walkSkip reports whether a walked entry should be skipped before the +// per-file callback runs. When skip is true, the returned error is what the +// walk callback should return (filepath.SkipDir to prune an excluded +// directory, otherwise nil). Symlinks, directories and non-regular files are +// never checksummed. +func walkSkip(opts *options, root, p string, info os.FileInfo) (bool, error) { + // filepath.Walk uses Lstat, so symlinks are reported as ModeSymlink, + // never as directories. Walk doesn't follow them. + if info.Mode()&os.ModeSymlink != 0 { + if opts.verbose && !opts.quiet { + log.Printf("skip symlink %s", p) + } + + return true, nil + } + + rel, _ := filepath.Rel(root, p) + if shouldExclude(opts, rel) { + if info.IsDir() { + return true, filepath.SkipDir + } + + return true, nil + } + + if info.IsDir() { + return true, nil + } + + if !info.Mode().IsRegular() { + if opts.verbose && !opts.quiet { + log.Printf("skip non-regular %s", p) + } + + return true, nil + } + + return false, nil +} + +func shouldExclude(opts *options, rel string) bool { if rel == "." || rel == "" { return false } - if excludeDotfiles { - for _, part := range strings.Split(rel, string(os.PathSeparator)) { + + if opts.excludeDotfiles { + for part := range strings.SplitSeq(rel, string(os.PathSeparator)) { if strings.HasPrefix(part, ".") { return true } } } - for _, pat := range excludePatterns { + + for _, pat := range opts.excludePatterns { if ok, _ := doublestar.PathMatch(pat, rel); ok { return true } } + return false } func hasXattr(path, key string) bool { _, err := xattr.Get(path, key) + return err == nil } -func fileMultihash(path string) (hash []byte, bytesRead int64, err error) { - f, err := os.Open(path) +func fileMultihash(path string) ([]byte, int64, error) { + // The path is supplied by the operator as the tree to checksum; reading + // it is the entire purpose of the tool. + f, err := os.Open(path) //nolint:gosec // G304: operator-specified path is the intended input if err != nil { return nil, 0, err } - defer f.Close() + + defer func() { _ = f.Close() }() h := sha256.New() - bytesRead, err = io.Copy(h, f) + + bytesRead, err := io.Copy(h, f) if err != nil { return nil, bytesRead, err } + mh, err := multihash.Encode(h.Sum(nil), multihash.SHA2_256) if err != nil { return nil, bytesRead, err } + return []byte(base58.Encode(mh)), bytesRead, nil } diff --git a/attrsum_test.go b/attrsum_test.go index d3d87d0..26d37a2 100644 --- a/attrsum_test.go +++ b/attrsum_test.go @@ -10,23 +10,58 @@ import ( "github.com/pkg/xattr" ) -func skipIfNoXattr(t *testing.T, path string) { - if err := xattr.Set(path, "user.test", []byte("1")); err != nil { - t.Skipf("skipping: xattr not supported: %v", err) - } else { - _ = xattr.Remove(path, "user.test") +const ( + dirPerm = 0o755 + filePerm = 0o644 + noPerm = 0o000 + + // futureSkew advances a file's mtime far enough to be unambiguously + // newer than a previously recorded sumtime. + futureSkew = 2 * time.Second +) + +// skipIfNoXattr skips the test unless the filesystem under dir supports the +// extended-attribute namespace the program actually uses. Probing with the +// real checksumKey (rather than a user.* key) matters on Linux, where regular +// files only accept xattrs in the user.* namespace and the program's +// berlin.sneak.* keys yield "operation not supported". +func skipIfNoXattr(t *testing.T, dir string) { + t.Helper() + + probe := filepath.Join(dir, "xattr-probe") + + err := os.WriteFile(probe, []byte("probe"), filePerm) + if err != nil { + t.Fatalf("write probe file: %v", err) } + + defer func() { _ = os.Remove(probe) }() + + err = xattr.Set(probe, checksumKey, []byte("1")) + if err != nil { + t.Skipf("skipping: xattr namespace %q not supported: %v", checksumKey, err) + + return + } + + _ = xattr.Remove(probe, checksumKey) } func writeFile(t *testing.T, root, name, content string) string { t.Helper() + p := filepath.Join(root, name) - if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + + err := os.MkdirAll(filepath.Dir(p), dirPerm) + if err != nil { t.Fatalf("mkdir: %v", err) } - if err := os.WriteFile(p, []byte(content), 0o644); err != nil { + + err = os.WriteFile(p, []byte(content), filePerm) + if err != nil { t.Fatalf("write: %v", err) } + return p } @@ -35,28 +70,46 @@ func newTestStats() *Stats { } func TestSumAddAndUpdate(t *testing.T) { + t.Parallel() + + opts := &options{} dir := t.TempDir() skipIfNoXattr(t, dir) f := writeFile(t, dir, "a.txt", "hello") - if err := ProcessSumAdd(dir, newTestStats(), nil); err != nil { + err := processSumAdd(opts, dir, newTestStats(), nil) + if err != nil { t.Fatalf("add: %v", err) } - if _, err := xattr.Get(f, checksumKey); err != nil { + + _, err = xattr.Get(f, checksumKey) + if err != nil { t.Fatalf("checksum missing: %v", err) } + tsb, _ := xattr.Get(f, sumTimeKey) origTime, _ := time.Parse(time.RFC3339Nano, string(tsb)) - os.WriteFile(f, []byte(strings.ToUpper("hello")), 0o644) - now := time.Now().Add(2 * time.Second) - os.Chtimes(f, now, now) + err = os.WriteFile(f, []byte(strings.ToUpper("hello")), filePerm) + if err != nil { + t.Fatalf("rewrite: %v", err) + } - if err := ProcessSumUpdate(dir, newTestStats(), nil); err != nil { + now := time.Now().Add(futureSkew) + + err = os.Chtimes(f, now, now) + if err != nil { + t.Fatalf("chtimes: %v", err) + } + + err = processSumUpdate(opts, dir, newTestStats(), nil) + if err != nil { t.Fatalf("update: %v", err) } + tsb2, _ := xattr.Get(f, sumTimeKey) + newTime, _ := time.Parse(time.RFC3339Nano, string(tsb2)) if !newTime.After(origTime) { t.Fatalf("sumtime not updated") @@ -64,46 +117,74 @@ func TestSumAddAndUpdate(t *testing.T) { } func TestProcessCheckIntegration(t *testing.T) { + t.Parallel() + + opts := &options{} dir := t.TempDir() skipIfNoXattr(t, dir) writeFile(t, dir, "b.txt", "world") - if err := ProcessSumAdd(dir, newTestStats(), nil); err != nil { + + err := processSumAdd(opts, dir, newTestStats(), nil) + if err != nil { t.Fatalf("add: %v", err) } - if err := ProcessCheck(dir, false, newTestStats(), nil); err != nil { + + err = processCheck(opts, dir, false, newTestStats(), nil) + if err != nil { t.Fatalf("check ok: %v", err) } f := filepath.Join(dir, "b.txt") - os.WriteFile(f, []byte("corrupt"), 0o644) - if err := ProcessCheck(dir, false, newTestStats(), nil); err == nil { + err = os.WriteFile(f, []byte("corrupt"), filePerm) + if err != nil { + t.Fatalf("corrupt: %v", err) + } + + err = processCheck(opts, dir, false, newTestStats(), nil) + if err == nil { t.Fatalf("expected mismatch error, got nil") } } func TestClearRemovesAttrs(t *testing.T) { + t.Parallel() + + opts := &options{} dir := t.TempDir() skipIfNoXattr(t, dir) f := writeFile(t, dir, "c.txt", "data") - if err := ProcessSumAdd(dir, newTestStats(), nil); err != nil { + + err := processSumAdd(opts, dir, newTestStats(), nil) + if err != nil { t.Fatalf("add: %v", err) } - if err := ProcessClear(dir, newTestStats(), nil); err != nil { + err = processClear(opts, dir, newTestStats(), nil) + if err != nil { t.Fatalf("clear: %v", err) } - if _, err := xattr.Get(f, checksumKey); err == nil { + + _, err = xattr.Get(f, checksumKey) + if err == nil { t.Fatalf("checksum still present after clear") } - if _, err := xattr.Get(f, sumTimeKey); err == nil { + + _, err = xattr.Get(f, sumTimeKey) + if err == nil { t.Fatalf("sumtime still present after clear") } } func TestExcludeDotfilesAndPatterns(t *testing.T) { + t.Parallel() + + opts := &options{ + excludeDotfiles: true, + excludePatterns: []string{"*.me"}, + } dir := t.TempDir() skipIfNoXattr(t, dir) @@ -111,61 +192,82 @@ func TestExcludeDotfilesAndPatterns(t *testing.T) { keep := writeFile(t, dir, "keep.txt", "keep") skip := writeFile(t, dir, "skip.me", "skip") - oldDot := excludeDotfiles - oldPat := excludePatterns - excludeDotfiles = true - excludePatterns = []string{"*.me"} - defer func() { excludeDotfiles, excludePatterns = oldDot, oldPat }() - - if err := ProcessSumAdd(dir, newTestStats(), nil); err != nil { + err := processSumAdd(opts, dir, newTestStats(), nil) + if err != nil { t.Fatalf("add with excludes: %v", err) } - if _, err := xattr.Get(keep, checksumKey); err != nil { + _, err = xattr.Get(keep, checksumKey) + if err != nil { t.Fatalf("expected xattr on keep.txt: %v", err) } - if _, err := xattr.Get(hidden, checksumKey); err == nil { + + _, err = xattr.Get(hidden, checksumKey) + if err == nil { t.Fatalf(".hidden should have been excluded") } - if _, err := xattr.Get(skip, checksumKey); err == nil { + + _, err = xattr.Get(skip, checksumKey) + if err == nil { t.Fatalf("skip.me should have been excluded") } } func TestSkipBrokenSymlink(t *testing.T) { + t.Parallel() + + opts := &options{} dir := t.TempDir() skipIfNoXattr(t, dir) - // Create a dangling symlink + // Create a dangling symlink. link := filepath.Join(dir, "dangling.lnk") - if err := os.Symlink(filepath.Join(dir, "nonexistent.txt"), link); err != nil { + + err := os.Symlink(filepath.Join(dir, "nonexistent.txt"), link) + if err != nil { t.Fatalf("symlink: %v", err) } - // Should not error and should not create xattrs on link - if err := ProcessSumAdd(dir, newTestStats(), nil); err != nil { - t.Fatalf("ProcessSumAdd with symlink: %v", err) + // Should not error and should not create xattrs on link. + err = processSumAdd(opts, dir, newTestStats(), nil) + if err != nil { + t.Fatalf("processSumAdd with symlink: %v", err) } - if _, err := xattr.Get(link, checksumKey); err == nil { + + _, err = xattr.Get(link, checksumKey) + if err == nil { t.Fatalf("symlink should not have xattr") } } func TestPermissionErrors(t *testing.T) { + t.Parallel() + + opts := &options{} dir := t.TempDir() skipIfNoXattr(t, dir) secret := writeFile(t, dir, "secret.txt", "data") - os.Chmod(secret, 0o000) - defer os.Chmod(secret, 0o644) - if err := ProcessSumAdd(dir, newTestStats(), nil); err == nil { + err := os.Chmod(secret, noPerm) + if err != nil { + t.Fatalf("chmod: %v", err) + } + + defer func() { _ = os.Chmod(secret, filePerm) }() + + err = processSumAdd(opts, dir, newTestStats(), nil) + if err == nil { t.Fatalf("expected permission error, got nil") } - if err := ProcessSumUpdate(dir, newTestStats(), nil); err == nil { + + err = processSumUpdate(opts, dir, newTestStats(), nil) + if err == nil { t.Fatalf("expected permission error on update, got nil") } - if err := ProcessCheck(dir, false, newTestStats(), nil); err == nil { + + err = processCheck(opts, dir, false, newTestStats(), nil) + if err == nil { t.Fatalf("expected permission error on check, got nil") } } -- 2.49.1