Reason/Context
Describe the bug
StringFromCharset(n, charset) in pkg/util/rand/rand.go panics instead of returning an error when called with an empty charset and n > 0. This package also currently has zero unit test coverage, which is how this went unnoticed.
To Reproduce
_, err := rand.StringFromCharset(5, "")
This panics with:
panic: crypto/rand: argument to Int is <= 0
Description
Root cause: maxIdx := big.NewInt(int64(len(charset))) evaluates to 0 when charset is empty, and that value is passed directly into crypto/rand.Int(rand.Reader, maxIdx), which panics whenever max <= 0.
Expected behavior
StringFromCharset should return ("", error) for this input, consistent with how the function already handles other internal failures — errors from crypto/rand.Int elsewhere in the loop are wrapped and returned rather than left to panic.
Additional context
pkg/util/rand has no test file at all today. String() and StringFromCharset() back the OAuth2 state / PKCE value generation used in cmd/login.go, so both the missing coverage and this panic path are worth closing together — the fix without tests would just leave the same class of bug free to reappear.
I'd like to work on this: fixing the panic and adding full unit test coverage for the package in the same change.
Implementation ideas
No response
Reason/Context
Describe the bug
StringFromCharset(n, charset)inpkg/util/rand/rand.gopanics instead of returning an error when called with an empty charset andn > 0. This package also currently has zero unit test coverage, which is how this went unnoticed.To Reproduce
This panics with:
panic: crypto/rand: argument to Int is <= 0Description
Root cause:
maxIdx := big.NewInt(int64(len(charset)))evaluates to0whencharsetis empty, and that value is passed directly intocrypto/rand.Int(rand.Reader, maxIdx), which panics whenevermax <= 0.Expected behavior
StringFromCharsetshould return("", error)for this input, consistent with how the function already handles other internal failures — errors fromcrypto/rand.Intelsewhere in the loop are wrapped and returned rather than left to panic.Additional context
pkg/util/randhas no test file at all today.String()andStringFromCharset()back the OAuth2 state / PKCE value generation used incmd/login.go, so both the missing coverage and this panic path are worth closing together — the fix without tests would just leave the same class of bug free to reappear.I'd like to work on this: fixing the panic and adding full unit test coverage for the package in the same change.
Implementation ideas
No response