feat: add security fuzz tests and harden input sinks#20
Merged
Conversation
Add native Go fuzz tests covering the untrusted-input boundaries (clone method flag, repository/organization names and URLs, path resolution, clone-URL construction, and API JSON decoding) to guard against path traversal and command/argument injection. The type-safety validators (IsValid) existed but were never called, so untrusted GitHub API data flowed straight into filepath.Join and exec.Command. Harden those sinks first so the fuzz tests assert real security invariants: - Tighten name validators to strict allowlists (reject "."/"..", path separators, leading/trailing '-', control/whitespace chars). - Strengthen URL validators to reject leading '-' and embedded whitespace/control chars. - Add pure choke-point helpers resolveRepoPath, buildCloneURL, and parseRepos, wired into CloneRepo/FetchAllRepos. - Use `git clone --` separator to prevent argument injection. Fuzzing is split into two tiers: - Lightweight (CI): `task fuzz-ci` runs a short burst per target and is wired into a new fuzz job in ci.yml; seed corpora also run under `task test`. - Heavyweight (ad hoc): `task fuzz` runs each target for a long, configurable duration (FUZZTIME, default 5m). Both tiers auto-discover every Fuzz* target since `go test -fuzz` fuzzes one target per invocation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds native Go fuzz tests that harden
gitgrabagainst path traversal and command/argument injection at its untrusted-input boundaries, split into a lightweight CI tier and a heavyweight ad-hoc tier.While building the tests I found the type-safety validators (
IsValid()) were defined but never called — untrusted GitHub API data and CLI flags flowed straight intofilepath.Joinandexec.Command("git", "clone", ...). Fuzz tests that only asserted current behavior wouldn't harden anything, so this PR closes the holes first, then fuzzes the invariants.Hardening (
gitgrab.go)./.., path separators, leading/trailing-, and control/whitespace characters.-(git option injection) and embedded whitespace/control chars (command splitting).CloneRepo/FetchAllRepos:resolveRepoPath— validates the repo name and confirms the joined path stays directly within the target dir.buildCloneURL— validates every untrusted component before constructing/returning a clone URL.parseRepos— strict JSON decode of API responses (rejects trailing garbage).git clone -- URL PATHseparator so a--prefixed value can't be interpreted as a flag.Fuzz targets (
fuzz_test.go)Six targets asserting security invariants (not fixed outputs), each with a malicious seed corpus (traversal, command substitution,
ext::sh,--upload-pack=, NUL bytes, oversized inputs):FuzzParseCloneMethod,FuzzRepositoryNameValidation,FuzzOrganizationNameValidation,FuzzResolveRepoPath,FuzzBuildCloneURL,FuzzParseRepos.Two tiers
task fuzz-ci(short burst per target, defaultFUZZTIME=15s), wired into a newfuzzjob in.github/workflows/ci.yml. Seed corpora also run undertask teston every push/PR.task fuzz(default 5m/target, e.g.task fuzz FUZZTIME=30m).Both tiers auto-discover every
Fuzz*target (grep in the internalfuzz-runtask), sincego test -fuzzfuzzes one target per invocation.Testing
go build,go vet,task testall pass.🤖 Generated with Claude Code