Add fuzz tests and fix parseRetryAfter overflow - #94
Conversation
Large Retry-After values (e.g. "9227000000") overflow time.Duration when multiplied by time.Second, producing negative durations. Cap at 300 seconds — any server asking for more than 5 minutes is bogus.
Cover parseLinkNext, parseRetryAfter, ParsePage (client) and parsePseudoColumnID, normalizeSkillPath, expandPath (commands). Includes regression corpus entry for the parseRetryAfter overflow found during initial fuzzing.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eddb89a17b
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
This PR hardens the HTTP client’s retry/backoff handling by preventing Retry-After-derived time.Duration overflow, and adds Go native fuzz tests for several parsing helpers to support ongoing robustness checks.
Changes:
- Cap numeric
Retry-Aftervalues inparseRetryAfterto avoidtime.Durationoverflow (e.g., very large seconds values wrapping negative). - Add 6 Go fuzzers across
internal/clientandinternal/commandsto ensure parser helpers don’t panic. - Add a fuzz regression corpus entry for the previously-overflowing
Retry-Aftervalue.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| internal/client/client.go | Caps integer Retry-After seconds to prevent time.Duration overflow. |
| internal/client/fuzz_test.go | Adds fuzzers for parseLinkNext, parseRetryAfter, and ParsePage. |
| internal/client/testdata/fuzz/FuzzParseRetryAfter/2f261fc7602f5d07 | Adds corpus input reproducing the overflow case. |
| internal/commands/fuzz_test.go | Adds fuzzers for parsePseudoColumnID, normalizeSkillPath, and expandPath. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Hoist parseLinkNext regexp to package-level var (Copilot) - Cap HTTP-date Retry-After branch at 300s too (Copilot) - Add unit test cases for cap/overflow behavior (Copilot)
Summary
time.Durationoverflow inparseRetryAfter— large Retry-After header values (e.g."9227000000") cause negative durations. Capped at 300 seconds.internal/clientandinternal/commands, satisfying the OpenSSF Scorecard Fuzzing check.Test plan
FuzzParseRetryAfterregression corpus passes (previously overflowed)make buildsucceedsmake test-unitpassesSummary by cubic
Fixes
parseRetryAfteroverflow by capping numeric and HTTP-dateRetry-Afterat 300s to prevent negative durations. Adds fuzz tests for six parser helpers with a regression seed, adds unit tests for the cap (meets OpenSSF Scorecard Fuzzing), and hoists theparseLinkNextregexp to a package var.Written for commit 265537b. Summary will update on new commits.