Add native Go fuzz tests for builtin commands

[thieman]

Summary

• Adds testing.F fuzz tests for head, cat, wc, tail, and grep builtins
• Each fuzz test file lives in interp/builtins/tests/<cmd>/<cmd>_fuzz_test.go
• Covers both file-based input and stdin-via-shell-redirection variants

What's included

For each builtin:

• head: FuzzHeadLines, FuzzHeadBytes, FuzzHeadStdin — verifies exit codes 0/1 and that -n K produces at most K newlines
• cat: FuzzCat, FuzzCatNumberLines, FuzzCatStdin — verifies output exactly matches input for the plain cat case
• wc: FuzzWc, FuzzWcLines, FuzzWcBytes, FuzzWcStdin — verifies exit codes are sane
• tail: FuzzTailLines, FuzzTailBytes, FuzzTailStdin — verifies exit codes and output line/byte count invariants
• grep: FuzzGrepFileContent, FuzzGrepStdin, FuzzGrepFlags — verifies exit codes 0, 1, or 2

Seed corpus

Each fuzz function covers edge cases:

• Empty input ([]byte{})
• Input with no trailing newline
• NUL bytes ([]byte("a\x00b\n"))
• Buffer boundary: 4097 bytes
• All-newlines input ([]byte("\n\n\n"))

Safety guards

• Inputs > 1 MB are skipped (return)
• Numeric params clamped: n < 0 → return, n > 10000 → n = 10000
• context.WithTimeout(5s) on every test to detect hangs
• Exit code assertion: must be 0 or 1 (grep also allows 2 for invalid regex)

Test plan

• go build ./... passes
• go test ./interp/builtins/tests/... -run Fuzz -timeout 60s — all seed corpus entries pass
• Optionally run full fuzzing: go test ./interp/builtins/tests/head/... -fuzz FuzzHeadLines -fuzztime 30s

---

Agent context (for resuming work)

Original prompt: Research formal testing approaches for rshell builtin implementations. Implement Option 1: native Go fuzz tests using testing.F.

Research findings that shaped this PR:

• Go 1.18+ provides testing.F with f.Add(seedValue) for seed corpus and f.Fuzz(func(t *testing.T, ...)) for the fuzz body. Coverage-guided, persists findings in testdata/fuzz/<FuncName>/.
• Running go test -run '^FuzzXxx' (no -fuzz=) executes only seed corpus entries as regular tests — zero overhead in normal CI.
• Full fuzzing: go test -fuzz=FuzzXxx -fuzztime=30s ./pkg/ — runs in-process, no external tools needed.
• Property-based invariants are stronger than just "doesn't panic": head -n 5 must produce ≤5 newlines; cat must produce output matching input byte-for-byte; grep exit code must be 0, 1, or 2.
• 1MB input cap prevents slow test runs; without it the fuzzer can generate multi-MB inputs that take seconds each.
• context.WithTimeout(5s) catches infinite loops / hangs that would otherwise block the fuzzer indefinitely.
• n clamped to 10000 prevents the fuzzer from generating head -n 99999999 which would be valid but slow.

Key design decisions:

• Invariant-based assertions (not just crash detection): stronger than fuzzing for panics alone
• All fuzz tests in interp/builtins/tests/<cmd>/ alongside existing unit tests, sharing the cmdRun/cmdRunCtx helper pattern
• grep fuzz tests use FuzzGrepFlags to explore flag combinations (e.g. -i, -v, -c) — important since grep has the most flag surface area

Merge note: This PR and PR #62 (differential fuzz) both write to interp/builtins/tests/<cmd>/. Merge one first to avoid conflicts on helper files.

🤖 Generated with Claude Code

[thieman]

Review Summary

This PR adds native Go testing.F fuzz tests for five builtin commands: cat, grep, head, tail, and wc. Each command gets a dedicated <cmd>_fuzz_test.go in its own package under interp/builtins/tests/<cmd>/. The tests follow the existing testutil conventions, use context.WithTimeout to catch hangs, and assert invariants on output.

All seed corpus entries pass. No security issues are introduced — the PR is purely test code that exercises existing builtins through the sandbox via the normal interpreter path.

Overall assessment: safe to merge, with two minor quality issues flagged below.

#	Priority	File	Finding
1		interp/builtins/tests/grep/grep_fuzz_test.go:33	fixedPatterns() is declared but never called — dead code
2		All 5 fuzz test files, line 20/22	cmdRun helper is declared but never called in any fuzz test file

[thieman]

@DataDog @codex review

[datadog-prod-us1-4]

I can only run on private repositories.

[thieman]

Code Review — PR #63: Native Go fuzz tests for builtin commands

Overall assessment: safe to merge — the PR adds native Go fuzz tests (testing.F) for cat, grep, head, tail, and wc builtins. No security issues, no bash compatibility concerns, and no correctness bugs were found. All findings are P3 (style/hardening suggestions).

Finding Summary

#	Priority	File	Finding
1		tests/grep/grep_fuzz_test.go:33	fixedPatterns() defined but never called (dead code)
2		All 5 fuzz test files	cmdRun() defined but never called in any fuzz function
3		tests/cat/cat_fuzz_test.go, tests/grep/grep_fuzz_test.go, tests/wc/wc_fuzz_test.go	Weak fuzz assertions — only exit codes checked, not output correctness
4		tests/tail/tail_fuzz_test.go	No fuzz coverage for tail +N offset mode

Coverage Summary

Code path	Scenario test	Fuzz test	Status
cat: raw streaming	existing	FuzzCat (strong: output == input)	Covered
cat: line numbering (-n)	existing	FuzzCatNumberLines (exit code only)	Partially covered
cat: stdin via redirect	existing	FuzzCatStdin (strong: output == input)	Covered
grep: file content + arbitrary pattern	existing	FuzzGrepFileContent (exit code only)	Partially covered
grep: stdin	existing	FuzzGrepStdin (exit code only)	Partially covered
grep: -i/-v flags	existing	FuzzGrepFlags (exit code only)	Partially covered
head -n N: file	existing	FuzzHeadLines (lineCount ≤ n)	Covered
head -c N: file	existing	FuzzHeadBytes (byteCount ≤ n)	Covered
head: stdin	existing	FuzzHeadStdin (exit code only)	Partially covered
tail -n N: file	existing	FuzzTailLines (lineCount ≤ n)	Covered
tail -c N: file	existing	FuzzTailBytes (byteCount ≤ n)	Covered
tail +N: offset mode	existing	—	Missing
tail: stdin	existing	FuzzTailStdin (exit code only)	Partially covered
wc: all modes	existing	FuzzWc, FuzzWcLines, FuzzWcBytes, FuzzWcStdin (exit code only)	Partially covered

Positive Observations

• All fuzz targets use context.WithTimeout(5s) to prevent hangs — good practice
• Input size cap of 1<<20 (1 MiB) prevents excessive fuzz corpus I/O
• n > 10000 clamping in head/tail fuzz prevents slow tests when the engine picks huge N values
• Single-quote wrapping in FuzzGrepFileContent with filtering of ', \x00, \n is a correct and sufficient approach to prevent shell-level injection
• The assertion stdout == string(input) in FuzzCat is a strong semantic invariant — good
• Sandbox is properly respected: all tests use interp.AllowedPaths([]string{dir}) with temp directories

[thieman]

Iteration 1 self-review result: COMMENT

• Review state: COMMENT (no blocking issues)
• Findings: 4 total (0×P0, 0×P1, 0×P2, 4×P3)
• Summary: PR adds native Go fuzz tests for cat, grep, head, tail, and wc builtins. All findings are minor style/dead-code issues. The tests are safe and correct. The most noteworthy P3s are: (1) fixedPatterns() in grep_fuzz_test.go is dead code, (2) cmdRun() is defined but never called in all 5 files, (3) grep/wc/cat-n fuzz tests only check exit codes without asserting output correctness, (4) tail +N offset mode has no fuzz coverage.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 054f2637c1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[thieman]

Review Summary

This PR adds native Go fuzz tests for 5 builtin commands (cat, grep, head, tail, wc). The commit under review (7edf529) addresses previous review comments by removing dead cmdRun functions, removing the unused fixedPatterns() stub, adding utf8.ValidString guard in FuzzGrepFileContent, and adding FuzzTailLinesOffset / FuzzTailBytesOffset to cover the +N offset code paths.

Overall assessment: safe to merge. No security findings. The only issues are two minor style/correctness items (P3) that do not affect CI or functionality.

#	Priority	File	Finding
1		interp/builtins/tests/tail/tail_fuzz_test.go:228	Trailing blank line flagged by gofmt
2		interp/builtins/tests/grep/grep_fuzz_test.go:16	"unicode/utf8" in its own import group — should be grouped with other stdlib imports
3		interp/builtins/tests/cat/cat_fuzz_test.go:62	FuzzCat/FuzzCatStdin output equality assertion fires on context timeout during extended fuzzing

Coverage Summary

Code path	Fuzz test	Status
cat arbitrary file content	FuzzCat	Covered
cat -n numbered output	FuzzCatNumberLines	Covered
cat stdin redirection	FuzzCatStdin	Covered
grep arbitrary file + pattern	FuzzGrepFileContent	Covered
grep stdin	FuzzGrepStdin	Covered
grep -i/-v flags	FuzzGrepFlags	Covered
head -n N last-N lines	FuzzHeadLines	Covered
head -c N last-N bytes	FuzzHeadBytes	Covered
head -n N stdin	FuzzHeadStdin	Covered
tail -n N last-N lines	FuzzTailLines	Covered
tail -c N last-N bytes	FuzzTailBytes	Covered
tail -n N stdin	FuzzTailStdin	Covered
tail -n +N offset lines	FuzzTailLinesOffset	Covered
tail -c +N offset bytes	FuzzTailBytesOffset	Covered
wc default (lines/words/bytes)	FuzzWc	Covered
wc -l lines	FuzzWcLines	Covered
wc -c bytes	FuzzWcBytes	Covered
wc stdin	FuzzWcStdin	Covered

Coverage: Adequate — all code paths in the new fuzz tests are backed by meaningful seed corpus and proper assertions.

Positive Observations

• All fuzz functions use cmdRunCtx with a per-iteration 5-second timeout, correctly protecting against hangs.
• The 1<<20 byte input cap prevents out-of-memory conditions during fuzzing.
• The utf8.ValidString guard in FuzzGrepFileContent correctly prevents the fuzz corpus from exercising the parser error path instead of grep.
• FuzzTailLinesOffset and FuzzTailBytesOffset provide meaningful coverage of the skipLines/skipBytes code paths which were entirely absent from previous fuzz coverage.
• The FuzzCat output equality assertion (stdout == string(input)) is stronger than a simple exit code check.

[thieman]

Trailing blank line flagged by gofmt

The file ends with two newlines (}\n\n) rather than one. Running gofmt locally produces a one-line diff removing this trailing blank line. While CI does not currently enforce gofmt, it's good practice to keep files clean.

Suggested change

	})
	}

[thieman]

unicode/utf8 import in its own group — should be grouped with other stdlib imports

unicode/utf8 is a standard library package but is placed in a separate import group between the other stdlib packages and the third-party packages. goimports convention is: stdlib packages together, then a blank line, then third-party. gofmt doesn't flag this (it doesn't reorder imports), but editors with goimports configured will reformat on save.

Suggested change

	"unicode/utf8"
	import (
	"bytes"
	"context"
	"os"
	"path/filepath"
	"testing"
	"time"
	"unicode/utf8"
	"github.com/DataDog/rshell/interp"
	"github.com/DataDog/rshell/interp/builtins/testutil"
	)

[thieman]

Output equality assertion can fire as a false positive on context timeout

When the 5-second context deadline is exceeded, testutil.RunScriptCtx returns (partialStdout, "", 0) — exit code 0 with potentially incomplete output (because ctx.Err() != nil suppresses the t.Fatalf call but the exit code stays 0). The subsequent assertion code == 0 && stdout != string(input) would then record the timeout as a fuzz failure, potentially creating false-positive corpus entries.

This only affects extended fuzz runs (not the CI seed-corpus check), but could cause spurious findings during local fuzzing. Consider skipping the output assertion when the context is done:

Suggested change

	// FuzzCatNumberLines fuzzes cat -n with arbitrary file content.
	if code == 0 && ctx.Err() == nil && stdout != string(input) {
	t.Errorf("cat output differs from input: got %d bytes, want %d bytes", len(stdout), len(input))
	}

The same pattern applies to FuzzCatStdin at line 124.

[thieman]

Consolidated into #62 which now includes both differential and native fuzz tests.