Audit & rewrite: security, reliability, performance hardening

[dolph]

Summary

Two-pass audit of the codebase: opened 22 issues across security, reliability, performance, and documentation; addressed all of them. Rewrote the walker and the file-rewrite pipeline for safety, bounded resource use, and metadata preservation.

What was wrong with main

• Walked through symlinks (os.Stat follows them), letting a malicious or accidental symlink redirect rewrites to anywhere on disk the running user could write — including /etc if root.
• Predictable temp-file names (math/rand + plain O_CREATE) enabled a co-resident attacker to pre-create the path as a symlink and have the rewrite clobber the link target.
• TOCTOU in rename (os.Stat + os.Rename) silently overwrote concurrently-created destinations.
• Unbounded goroutine fan-out (one per directory entry, recursively) hit EMFILE on large trees and thrashed the disk.
• Whole-file in memory twice — Read to string, Replace allocates another copy. Multi-GB files OOM the host.
• log.Fatal from worker paths — single error abandoned the run with os.Exit, skipping deferred cleanup; tempfiles leaked.
• No exit-code on partial failure, no -race in tests, no validation of empty find, redundant os.Stat per child, network-bound benchmark that didn't even work.

Plus several second-pass issues (preserved on a maintainer re-review): owner not preserved, mtime jumped forward, setuid bits silently stripped, .git worktree files clobbered, orphaned tempfiles picked up on re-run.

What this PR does

Security

• Skip symlinks in the walker (use fs.DirEntry.Type(), never os.Stat).
• os.CreateTemp for tempfiles (O_EXCL + crypto-random name) with defer-ed cleanup.
• Hardlink + remove for race-free no-overwrite renames; falls back to a documented best-effort path on filesystems without link support.
• Refuse to rewrite files with setuid/setgid bits (we cannot reliably preserve them across uid changes).

Reliability

• Single-threaded depth-first walk (matches v1.1.2's design and bounds FD/memory use).
• Errors are recorded and the walk continues; main exits non-zero when anything failed.
• Validate empty find and identical find/replace arguments.
• Skip .git whether it's a directory or a worktree-linkage file.
• Skip orphan .find-replace-* tempfiles from a crashed prior run.
• Tests run under -race.

Performance

• Streaming rewriter with bounded buffer + boundary-spanning match handling. No more whole-file allocations.
• Use fs.DirEntry directly — no extra os.Stat per child.
• Drop strings.Contains pre-check (Replace short-circuits internally).
• Skip filepath.Abs for already-absolute child paths.
• Pre-compute []byte form of find/replace once per Walk.

Metadata preservation

• Original mode, owner/group, and mtime are preserved across rewrites (chown is best-effort under non-root).

Dependencies

• Drop golang.org/x/tools (replaced its 6-line IsText heuristic with an in-tree NUL/UTF-8 check).

Tests

• 34 tests passing under -race. New coverage:
  • security_test.go: symlink traversal, tempfile symlink attack, rename overwrite refusal, mtime/owner preservation, setuid refusal.
  • reliability_test.go: error continuation, exit code, large-file streaming, CLI arg rejection, .git-as-file, stale tempfiles.
  • file_handling_test.go: streaming rewriter (incl. cross-buffer matches), binary detection, mode preservation, no leaked tempfiles, renameNoReplace semantics.
• Replaced the broken network-dependent BenchmarkNova (cloned over SSH inside a removed temp dir) with a reproducible synthetic-tree benchmark.

Issues closed

P0 (critical security): #2 #3
P1 (high): #4 #6 #7 #8
P2 (medium): #5 #9 #10 #11 #12 #13 #17 #20
P3 (low): #14 #15 #16 #18 #19 #21 #22 #23

Test plan

• go test -race -cover ./... — all 34 tests pass (1 skipped under root: permission-bit test)
• go vet ./...
• go test -bench BenchmarkSyntheticTree runs without network and reports stable numbers
• Run the binary in a worktree containing a symlink and confirm the symlink target is untouched (covered by TestSymlinkNotFollowed)
• Run in a tree with a multi-GB file and confirm bounded memory (covered by TestBigFileStreamsThroughBoundedMemory at 5 MB scale)

https://claude.ai/code/session_013qhiGeDHdyL2hSEaZJPSdf

---

Generated by Claude Code