Remove error returns from crypto random helpers and callers (#37240)

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: wxiaoguang <2114189+wxiaoguang@users.noreply.github.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
Co-authored-by: silverwind <115237+silverwind@users.noreply.github.com>

models/actions/runner.go

@@ -171,9 +171,8 @@ func (r *ActionRunner) LoadAttributes(ctx context.Context) error {
 	return nil
 }
 
-func (r *ActionRunner) GenerateToken() (err error) {
-	r.Token, r.TokenSalt, r.TokenHash, _, err = generateSaltedToken()
-	return err
+func (r *ActionRunner) GenerateAndFillToken() {
+	r.Token, r.TokenSalt, r.TokenHash, _ = generateSaltedToken()
 }
 
 // CanMatchLabels checks whether the runner's labels can match a job's "runs-on"

models/actions/runner_token.go

@@ -97,10 +97,7 @@ func NewRunnerTokenWithValue(ctx context.Context, ownerID, repoID int64, token s
 }
 
 func NewRunnerToken(ctx context.Context, ownerID, repoID int64) (*ActionRunnerToken, error) {
-	token, err := util.CryptoRandomString(40)
-	if err != nil {
-		return nil, err
-	}
+	token := util.CryptoRandomString(40)
 	return NewRunnerTokenWithValue(ctx, ownerID, repoID, token)
 }
 

models/actions/task.go

@@ -147,9 +147,8 @@ func (task *ActionTask) LoadAttributes(ctx context.Context) error {
 	return nil
 }
 
-func (task *ActionTask) GenerateToken() (err error) {
-	task.Token, task.TokenSalt, task.TokenHash, task.TokenLastEight, err = generateSaltedToken()
-	return err
+func (task *ActionTask) GenerateAndFillToken() {
+	task.Token, task.TokenSalt, task.TokenHash, task.TokenLastEight = generateSaltedToken()
 }
 
 func GetTaskByID(ctx context.Context, id int64) (*ActionTask, error) {
@@ -288,9 +287,7 @@ func CreateTaskForRunner(ctx context.Context, runner *ActionRunner) (*ActionTask
 		CommitSHA:         job.CommitSHA,
 		IsForkPullRequest: job.IsForkPullRequest,
 	}
-	if err := task.GenerateToken(); err != nil {
-		return nil, false, err
-	}
+	task.GenerateAndFillToken()
 
 	workflowJob, err := job.ParseJob()
 	if err != nil {

models/actions/utils.go

@@ -18,18 +18,12 @@ import (
 	"code.gitea.io/gitea/modules/util"
 )
 
-func generateSaltedToken() (string, string, string, string, error) {
-	salt, err := util.CryptoRandomString(10)
-	if err != nil {
-		return "", "", "", "", err
-	}
-	buf, err := util.CryptoRandomBytes(20)
-	if err != nil {
-		return "", "", "", "", err
-	}
+func generateSaltedToken() (string, string, string, string) {
+	salt := util.CryptoRandomString(10)
+	buf := util.CryptoRandomBytes(20)
 	token := hex.EncodeToString(buf)
 	hash := auth_model.HashToken(token, salt)
-	return token, salt, hash, token[len(token)-8:], nil
+	return token, salt, hash, token[len(token)-8:]
 }
 
 /*

models/auth/access_token.go

@@ -98,19 +98,13 @@ func init() {
 
 // NewAccessToken creates new access token.
 func NewAccessToken(ctx context.Context, t *AccessToken) error {
-	salt, err := util.CryptoRandomString(10)
-	if err != nil {
-		return err
-	}
-	token, err := util.CryptoRandomBytes(20)
-	if err != nil {
-		return err
-	}
+	salt := util.CryptoRandomString(10)
+	token := util.CryptoRandomBytes(20)
 	t.TokenSalt = salt
 	t.Token = hex.EncodeToString(token)
 	t.TokenHash = HashToken(t.Token, t.TokenSalt)
 	t.TokenLastEight = t.Token[len(t.Token)-8:]
-	_, err = db.GetEngine(ctx).Insert(t)
+	_, err := db.GetEngine(ctx).Insert(t)
 	return err
 }
 

models/auth/oauth2.go

@@ -185,10 +185,7 @@ var base32Lower = base32.NewEncoding(lowerBase32Chars).WithPadding(base32.NoPadd
 
 // GenerateClientSecret will generate the client secret and returns the plaintext and saves the hash at the database
 func (app *OAuth2Application) GenerateClientSecret(ctx context.Context) (string, error) {
-	rBytes, err := util.CryptoRandomBytes(32)
-	if err != nil {
-		return "", err
-	}
+	rBytes := util.CryptoRandomBytes(32)
 	// Add a prefix to the base32, this is in order to make it easier
 	// for code scanners to grab sensitive tokens.
 	clientSecret := "gto_" + base32Lower.EncodeToString(rBytes)
@@ -484,10 +481,7 @@ func (grant *OAuth2Grant) TableName() string {
 
 // GenerateNewAuthorizationCode generates a new authorization code for a grant and saves it to the database
 func (grant *OAuth2Grant) GenerateNewAuthorizationCode(ctx context.Context, redirectURI, codeChallenge, codeChallengeMethod string) (code *OAuth2AuthorizationCode, err error) {
-	rBytes, err := util.CryptoRandomBytes(32)
-	if err != nil {
-		return &OAuth2AuthorizationCode{}, err
-	}
+	rBytes := util.CryptoRandomBytes(32)
 	// Add a prefix to the base32, this is in order to make it easier
 	// for code scanners to grab sensitive tokens.
 	codeSecret := "gta_" + base32Lower.EncodeToString(rBytes)

models/auth/twofactor.go

@@ -65,14 +65,11 @@ func init() {
 
 // GenerateScratchToken recreates the scratch token the user is using.
 func (t *TwoFactor) GenerateScratchToken() (string, error) {
-	tokenBytes, err := util.CryptoRandomBytes(6)
-	if err != nil {
-		return "", err
-	}
+	tokenBytes := util.CryptoRandomBytes(6)
 	// these chars are specially chosen, avoid ambiguous chars like `0`, `O`, `1`, `I`.
 	const base32Chars = "ABCDEFGHJKLMNPQRSTUVWXYZ23456789"
 	token := base32.NewEncoding(base32Chars).WithPadding(base32.NoPadding).EncodeToString(tokenBytes)
-	t.ScratchSalt, _ = util.CryptoRandomString(10)
+	t.ScratchSalt = util.CryptoRandomString(10)
 	t.ScratchHash = HashToken(token, t.ScratchSalt)
 	return token, nil
 }

models/migrations/v1_6/v71.go

@@ -51,10 +51,7 @@ func AddScratchHash(x *xorm.Engine) error {
 
 		for _, tfa := range tfas {
 			// generate salt
-			salt, err := util.CryptoRandomString(10)
-			if err != nil {
-				return err
-			}
+			salt := util.CryptoRandomString(10)
 			tfa.ScratchSalt = salt
 			tfa.ScratchHash = base.HashToken(tfa.ScratchToken, salt)
 

models/migrations/v1_9/v85.go

@@ -65,10 +65,7 @@ func HashAppToken(x *xorm.Engine) error {
 
 		for _, token := range tokens {
 			// generate salt
-			salt, err := util.CryptoRandomString(10)
-			if err != nil {
-				return err
-			}
+			salt := util.CryptoRandomString(10)
 			token.TokenSalt = salt
 			token.TokenHash = base.HashToken(token.Sha1, salt)
 			if len(token.Sha1) < 8 {

models/organization/team_invite.go

@@ -116,10 +116,7 @@ func CreateTeamInvite(ctx context.Context, doer *user_model.User, team *Team, em
 		}
 	}
 
-	token, err := util.CryptoRandomString(25)
-	if err != nil {
-		return nil, err
-	}
+	token := util.CryptoRandomString(25)
 
 	invite := &TeamInvite{
 		Token:     token,

models/packages/package_blob_upload.go

@@ -31,16 +31,13 @@ type PackageBlobUpload struct {
 
 // CreateBlobUpload inserts a blob upload
 func CreateBlobUpload(ctx context.Context) (*PackageBlobUpload, error) {
-	id, err := util.CryptoRandomString(25)
-	if err != nil {
-		return nil, err
-	}
+	id := util.CryptoRandomString(25)
 
 	pbu := &PackageBlobUpload{
 		ID: strings.ToLower(id),
 	}
 
-	_, err = db.GetEngine(ctx).Insert(pbu)
+	_, err := db.GetEngine(ctx).Insert(pbu)
 	return pbu, err
 }
 

models/user/user.go

@@ -524,10 +524,7 @@ const SaltByteLength = 16
 
 // GetUserSalt returns a random user salt token.
 func GetUserSalt() (string, error) {
-	rBytes, err := util.CryptoRandomBytes(SaltByteLength)
-	if err != nil {
-		return "", err
-	}
+	rBytes := util.CryptoRandomBytes(SaltByteLength)
 	// Returns a 32-byte long string.
 	return hex.EncodeToString(rBytes), nil
 }

modules/generate/generate.go

@@ -65,10 +65,5 @@ func NewJwtSecretWithBase64() ([]byte, string) {
 
 // NewSecretKey generate a new value intended to be used by SECRET_KEY.
 func NewSecretKey() (string, error) {
-	secretKey, err := util.CryptoRandomString(64)
-	if err != nil {
-		return "", err
-	}
-
-	return secretKey, nil
+	return util.CryptoRandomString(64), nil
 }

modules/util/util.go

@@ -61,48 +61,42 @@ func NormalizeEOL(input []byte) []byte {
 }
 
 // CryptoRandomInt returns a crypto random integer between 0 and limit, inclusive
-func CryptoRandomInt(limit int64) (int64, error) {
+func CryptoRandomInt(limit int64) int64 {
 	rInt, err := rand.Int(rand.Reader, big.NewInt(limit))
 	if err != nil {
-		return 0, err
+		panic(err) // this should never happen
 	}
-	return rInt.Int64(), nil
+	return rInt.Int64()
 }
 
-const alphanumericalChars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
-
 // CryptoRandomString generates a crypto random alphanumerical string, each byte is generated by [0,61] range
-func CryptoRandomString(length int64) (string, error) {
+func CryptoRandomString(length int64) string {
+	const alphanumericalChars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789"
 	buf := make([]byte, length)
 	limit := int64(len(alphanumericalChars))
 	for i := range buf {
-		num, err := CryptoRandomInt(limit)
-		if err != nil {
-			return "", err
-		}
+		num := CryptoRandomInt(limit)
 		buf[i] = alphanumericalChars[num]
 	}
-	return string(buf), nil
+	return string(buf)
 }
 
 // CryptoRandomBytes generates `length` crypto bytes
 // This differs from CryptoRandomString, as each byte in CryptoRandomString is generated by [0,61] range
 // This function generates totally random bytes, each byte is generated by [0,255] range
-// TODO: it never fails, remove the "error" in the future
-func CryptoRandomBytes(length int64) ([]byte, error) {
+func CryptoRandomBytes(length int64) []byte {
 	buf := make([]byte, length)
 	if _, err := rand.Read(buf); err != nil {
 		panic(err) // this should never happen, "rand.Read" never fails
 	}
-	return buf, nil
+	return buf
 }
 
 var chaCha8RandPool = sync.OnceValue(func() *sync.Pool {
 	return &sync.Pool{
 		New: func() any {
-			var buf [32]byte
-			_, _ = rand.Read(buf[:])
-			return rand2.NewChaCha8(buf)
+			seed := CryptoRandomBytes(32)
+			return rand2.NewChaCha8([32]byte(seed))
 		},
 	}
 })

modules/util/util_test.go

@@ -86,35 +86,31 @@ func Test_NormalizeEOL(t *testing.T) {
 }
 
 func Test_RandomInt(t *testing.T) {
-	randInt, err := CryptoRandomInt(255)
+	randInt := CryptoRandomInt(255)
 	assert.GreaterOrEqual(t, randInt, int64(0))
 	assert.LessOrEqual(t, randInt, int64(255))
-	assert.NoError(t, err)
 }
 
 func Test_RandomString(t *testing.T) {
-	str1, err := CryptoRandomString(32)
-	assert.NoError(t, err)
+	str1 := CryptoRandomString(32)
+	var err error
 	matches, err := regexp.MatchString(`^[a-zA-Z0-9]{32}$`, str1)
 	assert.NoError(t, err)
 	assert.True(t, matches)
 
-	str2, err := CryptoRandomString(32)
-	assert.NoError(t, err)
+	str2 := CryptoRandomString(32)
 	matches, err = regexp.MatchString(`^[a-zA-Z0-9]{32}$`, str1)
 	assert.NoError(t, err)
 	assert.True(t, matches)
 
 	assert.NotEqual(t, str1, str2)
 
-	str3, err := CryptoRandomString(256)
-	assert.NoError(t, err)
+	str3 := CryptoRandomString(256)
 	matches, err = regexp.MatchString(`^[a-zA-Z0-9]{256}$`, str3)
 	assert.NoError(t, err)
 	assert.True(t, matches)
 
-	str4, err := CryptoRandomString(256)
-	assert.NoError(t, err)
+	str4 := CryptoRandomString(256)
 	matches, err = regexp.MatchString(`^[a-zA-Z0-9]{256}$`, str4)
 	assert.NoError(t, err)
 	assert.True(t, matches)
@@ -123,19 +119,15 @@ func Test_RandomString(t *testing.T) {
 }
 
 func Test_RandomBytes(t *testing.T) {
-	bytes1, err := CryptoRandomBytes(32)
-	assert.NoError(t, err)
+	bytes1 := CryptoRandomBytes(32)
 
-	bytes2, err := CryptoRandomBytes(32)
-	assert.NoError(t, err)
+	bytes2 := CryptoRandomBytes(32)
 
 	assert.NotEqual(t, bytes1, bytes2)
 
-	bytes3, err := CryptoRandomBytes(256)
-	assert.NoError(t, err)
+	bytes3 := CryptoRandomBytes(256)
 
-	bytes4, err := CryptoRandomBytes(256)
-	assert.NoError(t, err)
+	bytes4 := CryptoRandomBytes(256)
 
 	assert.NotEqual(t, bytes3, bytes4)
 }

routers/api/actions/runner/runner.go

@@ -80,9 +80,7 @@ func (s *Service) Register(
 		AgentLabels: labels,
 		Ephemeral:   req.Msg.Ephemeral,
 	}
-	if err := runner.GenerateToken(); err != nil {
-		return nil, errors.New("can't generate token")
-	}
+	runner.GenerateAndFillToken()
 
 	// create new runner
 	if err := actions_model.CreateRunner(ctx, runner); err != nil {

routers/api/v1/repo/mirror.go

@@ -351,11 +351,7 @@ func CreatePushMirror(ctx *context.APIContext, mirrorOption *api.CreatePushMirro
 		return
 	}
 
-	remoteSuffix, err := util.CryptoRandomString(10)
-	if err != nil {
-		ctx.APIErrorInternal(err)
-		return
-	}
+	remoteSuffix := util.CryptoRandomString(10)
 
 	remoteAddress, err := util.SanitizeURL(mirrorOption.RemoteAddress)
 	if err != nil {

routers/web/auth/openid.go

@@ -272,7 +272,7 @@ func ConnectOpenIDPost(ctx *context.Context) {
 
 	// add OpenID for the user
 	userOID := &user_model.UserOpenID{UID: u.ID, URI: oid}
-	if err = user_model.AddUserOpenID(ctx, userOID); err != nil {
+	if err := user_model.AddUserOpenID(ctx, userOID); err != nil {
 		if user_model.IsErrOpenIDAlreadyUsed(err) {
 			ctx.RenderWithErrDeprecated(ctx.Tr("form.openid_been_used", oid), tplConnectOID, &form)
 			return
@@ -345,11 +345,7 @@ func RegisterOpenIDPost(ctx *context.Context) {
 	}
 
 	length := max(setting.MinPasswordLength, 256)
-	password, err := util.CryptoRandomString(int64(length))
-	if err != nil {
-		ctx.RenderWithErrDeprecated(err.Error(), tplSignUpOID, form)
-		return
-	}
+	password := util.CryptoRandomString(int64(length))
 
 	u := &user_model.User{
 		Name:   form.UserName,
@@ -363,7 +359,7 @@ func RegisterOpenIDPost(ctx *context.Context) {
 
 	// add OpenID for the user
 	userOID := &user_model.UserOpenID{UID: u.ID, URI: oid}
-	if err = user_model.AddUserOpenID(ctx, userOID); err != nil {
+	if err := user_model.AddUserOpenID(ctx, userOID); err != nil {
 		if user_model.IsErrOpenIDAlreadyUsed(err) {
 			ctx.RenderWithErrDeprecated(ctx.Tr("form.openid_been_used", oid), tplSignUpOID, &form)
 			return

routers/web/repo/setting/setting.go

@@ -459,11 +459,7 @@ func handleSettingsPostPushMirrorAdd(ctx *context.Context) {
 		return
 	}
 
-	remoteSuffix, err := util.CryptoRandomString(10)
-	if err != nil {
-		ctx.ServerError("RandomString", err)
-		return
-	}
+	remoteSuffix := util.CryptoRandomString(10)
 
 	remoteAddress, err := util.SanitizeURL(form.PushMirrorAddress)
 	if err != nil {

services/actions/init_test.go

@@ -35,7 +35,7 @@ func TestInitToken(t *testing.T) {
 	})
 
 	t.Run("EnvToken", func(t *testing.T) {
-		tokenValue, _ := util.CryptoRandomString(32)
+		tokenValue := util.CryptoRandomString(32)
 		t.Setenv("GITEA_RUNNER_REGISTRATION_TOKEN", tokenValue)
 		t.Setenv("GITEA_RUNNER_REGISTRATION_TOKEN_FILE", "")
 		err := initGlobalRunnerToken(t.Context())
@@ -52,7 +52,7 @@ func TestInitToken(t *testing.T) {
 	})
 
 	t.Run("EnvFileToken", func(t *testing.T) {
-		tokenValue, _ := util.CryptoRandomString(32)
+		tokenValue := util.CryptoRandomString(32)
 		f := t.TempDir() + "/token"
 		_ = os.WriteFile(f, []byte(tokenValue), 0o644)
 		t.Setenv("GITEA_RUNNER_REGISTRATION_TOKEN", "")

services/auth/auth_token.go

@@ -64,10 +64,7 @@ func CheckAuthToken(ctx context.Context, value string) (*auth_model.AuthToken, e
 }
 
 func RegenerateAuthToken(ctx context.Context, t *auth_model.AuthToken) (*auth_model.AuthToken, string, error) {
-	token, hash, err := generateTokenAndHash()
-	if err != nil {
-		return nil, "", err
-	}
+	token, hash := generateTokenAndHash()
 
 	newToken := &auth_model.AuthToken{
 		ID:          t.ID,
@@ -89,16 +86,9 @@ func CreateAuthTokenForUserID(ctx context.Context, userID int64) (*auth_model.Au
 		ExpiresUnix: timeutil.TimeStampNow().AddDuration(time.Duration(setting.LogInRememberDays*24) * time.Hour),
 	}
 
-	var err error
-	t.ID, err = util.CryptoRandomString(10)
-	if err != nil {
-		return nil, "", err
-	}
+	t.ID = util.CryptoRandomString(10)
 
-	token, hash, err := generateTokenAndHash()
-	if err != nil {
-		return nil, "", err
-	}
+	token, hash := generateTokenAndHash()
 
 	t.TokenHash = hash
 
@@ -109,15 +99,12 @@ func CreateAuthTokenForUserID(ctx context.Context, userID int64) (*auth_model.Au
 	return t, token, nil
 }
 
-func generateTokenAndHash() (string, string, error) {
-	buf, err := util.CryptoRandomBytes(32)
-	if err != nil {
-		return "", "", err
-	}
+func generateTokenAndHash() (string, string) {
+	buf := util.CryptoRandomBytes(32)
 
 	token := hex.EncodeToString(buf)
 
 	hashedToken := sha256.Sum256([]byte(token))
 
-	return token, hex.EncodeToString(hashedToken[:]), nil
+	return token, hex.EncodeToString(hashedToken[:])
 }

tests/integration/actions_job_token_test.go

@@ -137,7 +137,7 @@ func TestActionsJobTokenPermissiveAccess(t *testing.T) {
 				require.NoError(t, repo_model.UpdateRepoUnitConfig(t.Context(), repoActionsUnit))
 
 				// prepare task and its token
-				require.NoError(t, task.GenerateToken())
+				task.GenerateAndFillToken()
 				task.Status = actions_model.StatusRunning
 				task.IsForkPullRequest = tt.isFork
 				err := actions_model.UpdateTask(t.Context(), task, "token_hash", "token_salt", "token_last_eight", "status", "is_fork_pull_request")
@@ -309,7 +309,7 @@ func TestActionsJobTokenPermissionsWriteIssue(t *testing.T) {
 	repoActionsCfg.MaxTokenPermissions = nil
 	require.NoError(t, repo_model.UpdateRepoUnitConfig(t.Context(), repoActionsUnit))
 
-	require.NoError(t, task.GenerateToken())
+	task.GenerateAndFillToken()
 	task.Status = actions_model.StatusRunning
 	require.NoError(t, actions_model.UpdateTask(t.Context(), task, "token_hash", "token_salt", "token_last_eight", "status"))
 
@@ -359,7 +359,7 @@ func createActionTask(t *testing.T, repoID int64, isFork bool) *actions_model.Ac
 		Status:            actions_model.StatusRunning,
 		IsForkPullRequest: isFork,
 	}
-	require.NoError(t, task.GenerateToken())
+	task.GenerateAndFillToken()
 	require.NoError(t, db.Insert(t.Context(), task))
 	return task
 }

tests/integration/api_packages_test.go

@@ -562,7 +562,7 @@ func TestPackageCleanup(t *testing.T) {
 		defer tests.PrintCurrentTest(t)()
 
 		// Upload and delete a generic package and upload a container blob
-		data, _ := util.CryptoRandomBytes(5)
+		data := util.CryptoRandomBytes(5)
 		url := fmt.Sprintf("/api/packages/%s/generic/cleanup-test/1.1.1/file.bin", user.Name)
 		req := NewRequestWithBody(t, "PUT", url, bytes.NewReader(data)).
 			AddBasicAuth(user.Name)
@@ -572,7 +572,7 @@ func TestPackageCleanup(t *testing.T) {
 			AddBasicAuth(user.Name)
 		MakeRequest(t, req, http.StatusNoContent)
 
-		data, _ = util.CryptoRandomBytes(5)
+		data = util.CryptoRandomBytes(5)
 		url = fmt.Sprintf("/v2/%s/cleanup-test/blobs/uploads?digest=sha256:%x", user.Name, sha256.Sum256(data))
 		req = NewRequestWithBody(t, "POST", url, bytes.NewReader(data)).
 			AddBasicAuth(user.Name)