Refactor secrets modification logic (#26873)

- Share code between web and api
- Add some tests

models/secret/secret.go

@@ -33,12 +33,6 @@ type ErrSecretNotFound struct {
 	Name string
 }
 
-// IsErrSecretNotFound checks if an error is a ErrSecretNotFound.
-func IsErrSecretNotFound(err error) bool {
-	_, ok := err.(ErrSecretNotFound)
-	return ok
-}
-
 func (err ErrSecretNotFound) Error() string {
 	return fmt.Sprintf("secret was not found [name: %s]", err.Name)
 }
@@ -47,23 +41,18 @@ func (err ErrSecretNotFound) Unwrap() error {
 	return util.ErrNotExist
 }
 
-// newSecret Creates a new already encrypted secret
-func newSecret(ownerID, repoID int64, name, data string) *Secret {
-	return &Secret{
-		OwnerID: ownerID,
-		RepoID:  repoID,
-		Name:    strings.ToUpper(name),
-		Data:    data,
-	}
-}
-
 // InsertEncryptedSecret Creates, encrypts, and validates a new secret with yet unencrypted data and insert into database
 func InsertEncryptedSecret(ctx context.Context, ownerID, repoID int64, name, data string) (*Secret, error) {
 	encrypted, err := secret_module.EncryptSecret(setting.SecretKey, data)
 	if err != nil {
 		return nil, err
 	}
-	secret := newSecret(ownerID, repoID, name, encrypted)
+	secret := &Secret{
+		OwnerID: ownerID,
+		RepoID:  repoID,
+		Name:    strings.ToUpper(name),
+		Data:    encrypted,
+	}
 	if err := secret.Validate(); err != nil {
 		return secret, err
 	}
@@ -83,8 +72,10 @@ func (s *Secret) Validate() error {
 
 type FindSecretsOptions struct {
 	db.ListOptions
-	OwnerID int64
+	OwnerID  int64
-	RepoID  int64
+	RepoID   int64
+	SecretID int64
+	Name     string
 }
 
 func (opts *FindSecretsOptions) toConds() builder.Cond {
@@ -95,6 +86,12 @@ func (opts *FindSecretsOptions) toConds() builder.Cond {
 	if opts.RepoID > 0 {
 		cond = cond.And(builder.Eq{"repo_id": opts.RepoID})
 	}
+	if opts.SecretID != 0 {
+		cond = cond.And(builder.Eq{"id": opts.SecretID})
+	}
+	if opts.Name != "" {
+		cond = cond.And(builder.Eq{"name": strings.ToUpper(opts.Name)})
+	}
 
 	return cond
 }
@@ -116,75 +113,18 @@ func CountSecrets(ctx context.Context, opts *FindSecretsOptions) (int64, error)
 }
 
 // UpdateSecret changes org or user reop secret.
-func UpdateSecret(ctx context.Context, orgID, repoID int64, name, data string) error {
+func UpdateSecret(ctx context.Context, secretID int64, data string) error {
-	sc := new(Secret)
-	name = strings.ToUpper(name)
-	has, err := db.GetEngine(ctx).
-		Where("owner_id=?", orgID).
-		And("repo_id=?", repoID).
-		And("name=?", name).
-		Get(sc)
-	if err != nil {
-		return err
-	} else if !has {
-		return ErrSecretNotFound{Name: name}
-	}
-
 	encrypted, err := secret_module.EncryptSecret(setting.SecretKey, data)
 	if err != nil {
 		return err
 	}
 
-	sc.Data = encrypted
+	s := &Secret{
-	_, err = db.GetEngine(ctx).ID(sc.ID).Cols("data").Update(sc)
+		Data: encrypted,
-	return err
-}
-
-// DeleteSecret deletes secret from an organization.
-func DeleteSecret(ctx context.Context, orgID, repoID int64, name string) error {
-	sc := new(Secret)
-	has, err := db.GetEngine(ctx).
-		Where("owner_id=?", orgID).
-		And("repo_id=?", repoID).
-		And("name=?", strings.ToUpper(name)).
-		Get(sc)
-	if err != nil {
-		return err
-	} else if !has {
-		return ErrSecretNotFound{Name: name}
-	}
-
-	if _, err := db.GetEngine(ctx).ID(sc.ID).Delete(new(Secret)); err != nil {
-		return fmt.Errorf("Delete: %w", err)
-	}
-
-	return nil
-}
-
-// CreateOrUpdateSecret creates or updates a secret and returns true if it was created
-func CreateOrUpdateSecret(ctx context.Context, orgID, repoID int64, name, data string) (bool, error) {
-	sc := new(Secret)
-	name = strings.ToUpper(name)
-	has, err := db.GetEngine(ctx).
-		Where("owner_id=?", orgID).
-		And("repo_id=?", repoID).
-		And("name=?", name).
-		Get(sc)
-	if err != nil {
-		return false, err
 	}
-
+	affected, err := db.GetEngine(ctx).ID(secretID).Cols("data").Update(s)
-	if !has {
+	if affected != 1 {
-		_, err = InsertEncryptedSecret(ctx, orgID, repoID, name, data)
+		return ErrSecretNotFound{}
-		if err != nil {
-			return false, err
-		}
-		return true, nil
 	}
-
+	return err
-	if err := UpdateSecret(ctx, orgID, repoID, name, data); err != nil {
-		return false, err
-	}
-
-	return false, nil
 }

routers/api/v1/org/action.go

@@ -4,14 +4,16 @@
 package org
 
 import (
+	"errors"
 	"net/http"
 
 	secret_model "code.gitea.io/gitea/models/secret"
 	"code.gitea.io/gitea/modules/context"
 	api "code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/modules/web"
 	"code.gitea.io/gitea/routers/api/v1/utils"
-	"code.gitea.io/gitea/routers/web/shared/actions"
+	secret_service "code.gitea.io/gitea/services/secrets"
 )
 
 // ListActionsSecrets list an organization's actions secrets
@@ -39,11 +41,6 @@ func ListActionsSecrets(ctx *context.APIContext) {
 	//   "200":
 	//     "$ref": "#/responses/SecretList"
 
-	listActionsSecrets(ctx)
-}
-
-// listActionsSecrets list an organization's actions secrets
-func listActionsSecrets(ctx *context.APIContext) {
 	opts := &secret_model.FindSecretsOptions{
 		OwnerID:     ctx.Org.Organization.ID,
 		ListOptions: utils.GetListOptions(ctx),
@@ -104,25 +101,28 @@ func CreateOrUpdateSecret(ctx *context.APIContext) {
 	//     description: response when updating a secret
 	//   "400":
 	//     "$ref": "#/responses/error"
-	//   "403":
+	//   "404":
-	//     "$ref": "#/responses/forbidden"
+	//     "$ref": "#/responses/notFound"
-	secretName := ctx.Params(":secretname")
+
-	if err := actions.NameRegexMatch(secretName); err != nil {
-		ctx.Error(http.StatusBadRequest, "CreateOrUpdateSecret", err)
-		return
-	}
 	opt := web.GetForm(ctx).(*api.CreateOrUpdateSecretOption)
-	isCreated, err := secret_model.CreateOrUpdateSecret(ctx, ctx.Org.Organization.ID, 0, secretName, opt.Data)
+
+	_, created, err := secret_service.CreateOrUpdateSecret(ctx, ctx.Org.Organization.ID, 0, ctx.Params("secretname"), opt.Data)
 	if err != nil {
-		ctx.Error(http.StatusInternalServerError, "CreateOrUpdateSecret", err)
+		if errors.Is(err, util.ErrInvalidArgument) {
+			ctx.Error(http.StatusBadRequest, "CreateOrUpdateSecret", err)
+		} else if errors.Is(err, util.ErrNotExist) {
+			ctx.Error(http.StatusNotFound, "CreateOrUpdateSecret", err)
+		} else {
+			ctx.Error(http.StatusInternalServerError, "CreateOrUpdateSecret", err)
+		}
 		return
 	}
-	if isCreated {
+
+	if created {
 		ctx.Status(http.StatusCreated)
-		return
+	} else {
+		ctx.Status(http.StatusNoContent)
 	}
-
-	ctx.Status(http.StatusNoContent)
 }
 
 // DeleteSecret delete one secret of the organization
@@ -148,22 +148,20 @@ func DeleteSecret(ctx *context.APIContext) {
 	// responses:
 	//   "204":
 	//     description: delete one secret of the organization
-	//   "403":
+	//   "400":
-	//     "$ref": "#/responses/forbidden"
+	//     "$ref": "#/responses/error"
-	secretName := ctx.Params(":secretname")
+	//   "404":
-	if err := actions.NameRegexMatch(secretName); err != nil {
+	//     "$ref": "#/responses/notFound"
-		ctx.Error(http.StatusBadRequest, "DeleteSecret", err)
+
-		return
+	err := secret_service.DeleteSecretByName(ctx, ctx.Org.Organization.ID, 0, ctx.Params("secretname"))
-	}
-	err := secret_model.DeleteSecret(
-		ctx, ctx.Org.Organization.ID, 0, secretName,
-	)
-	if secret_model.IsErrSecretNotFound(err) {
-		ctx.NotFound(err)
-		return
-	}
 	if err != nil {
-		ctx.Error(http.StatusInternalServerError, "DeleteSecret", err)
+		if errors.Is(err, util.ErrInvalidArgument) {
+			ctx.Error(http.StatusBadRequest, "DeleteSecret", err)
+		} else if errors.Is(err, util.ErrNotExist) {
+			ctx.Error(http.StatusNotFound, "DeleteSecret", err)
+		} else {
+			ctx.Error(http.StatusInternalServerError, "DeleteSecret", err)
+		}
 		return
 	}
 

routers/api/v1/repo/action.go

@@ -4,13 +4,14 @@
 package repo
 
 import (
+	"errors"
 	"net/http"
 
-	secret_model "code.gitea.io/gitea/models/secret"
 	"code.gitea.io/gitea/modules/context"
 	api "code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/modules/web"
-	"code.gitea.io/gitea/routers/web/shared/actions"
+	secret_service "code.gitea.io/gitea/services/secrets"
 )
 
 // create or update one secret of the repository
@@ -49,29 +50,31 @@ func CreateOrUpdateSecret(ctx *context.APIContext) {
 	//     description: response when updating a secret
 	//   "400":
 	//     "$ref": "#/responses/error"
-	//   "403":
+	//   "404":
-	//     "$ref": "#/responses/forbidden"
+	//     "$ref": "#/responses/notFound"
 
 	owner := ctx.Repo.Owner
 	repo := ctx.Repo.Repository
 
-	secretName := ctx.Params(":secretname")
-	if err := actions.NameRegexMatch(secretName); err != nil {
-		ctx.Error(http.StatusBadRequest, "CreateOrUpdateSecret", err)
-		return
-	}
 	opt := web.GetForm(ctx).(*api.CreateOrUpdateSecretOption)
-	isCreated, err := secret_model.CreateOrUpdateSecret(ctx, owner.ID, repo.ID, secretName, opt.Data)
+
+	_, created, err := secret_service.CreateOrUpdateSecret(ctx, owner.ID, repo.ID, ctx.Params("secretname"), opt.Data)
 	if err != nil {
-		ctx.Error(http.StatusInternalServerError, "CreateOrUpdateSecret", err)
+		if errors.Is(err, util.ErrInvalidArgument) {
+			ctx.Error(http.StatusBadRequest, "CreateOrUpdateSecret", err)
+		} else if errors.Is(err, util.ErrNotExist) {
+			ctx.Error(http.StatusNotFound, "CreateOrUpdateSecret", err)
+		} else {
+			ctx.Error(http.StatusInternalServerError, "CreateOrUpdateSecret", err)
+		}
 		return
 	}
-	if isCreated {
+
+	if created {
 		ctx.Status(http.StatusCreated)
-		return
+	} else {
+		ctx.Status(http.StatusNoContent)
 	}
-
-	ctx.Status(http.StatusNoContent)
 }
 
 // DeleteSecret delete one secret of the repository
@@ -102,26 +105,23 @@ func DeleteSecret(ctx *context.APIContext) {
 	// responses:
 	//   "204":
 	//     description: delete one secret of the organization
-	//   "403":
+	//   "400":
-	//     "$ref": "#/responses/forbidden"
+	//     "$ref": "#/responses/error"
+	//   "404":
+	//     "$ref": "#/responses/notFound"
 
 	owner := ctx.Repo.Owner
 	repo := ctx.Repo.Repository
 
-	secretName := ctx.Params(":secretname")
+	err := secret_service.DeleteSecretByName(ctx, owner.ID, repo.ID, ctx.Params("secretname"))
-	if err := actions.NameRegexMatch(secretName); err != nil {
-		ctx.Error(http.StatusBadRequest, "DeleteSecret", err)
-		return
-	}
-	err := secret_model.DeleteSecret(
-		ctx, owner.ID, repo.ID, secretName,
-	)
-	if secret_model.IsErrSecretNotFound(err) {
-		ctx.NotFound(err)
-		return
-	}
 	if err != nil {
-		ctx.Error(http.StatusInternalServerError, "DeleteSecret", err)
+		if errors.Is(err, util.ErrInvalidArgument) {
+			ctx.Error(http.StatusBadRequest, "DeleteSecret", err)
+		} else if errors.Is(err, util.ErrNotExist) {
+			ctx.Error(http.StatusNotFound, "DeleteSecret", err)
+		} else {
+			ctx.Error(http.StatusInternalServerError, "DeleteSecret", err)
+		}
 		return
 	}
 

routers/api/v1/user/action.go

@@ -4,13 +4,14 @@
 package user
 
 import (
+	"errors"
 	"net/http"
 
-	secret_model "code.gitea.io/gitea/models/secret"
 	"code.gitea.io/gitea/modules/context"
 	api "code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/modules/util"
 	"code.gitea.io/gitea/modules/web"
-	"code.gitea.io/gitea/routers/web/shared/actions"
+	secret_service "code.gitea.io/gitea/services/secrets"
 )
 
 // create or update one secret of the user scope
@@ -42,23 +43,25 @@ func CreateOrUpdateSecret(ctx *context.APIContext) {
 	//   "404":
 	//     "$ref": "#/responses/notFound"
 
-	secretName := ctx.Params(":secretname")
-	if err := actions.NameRegexMatch(secretName); err != nil {
-		ctx.Error(http.StatusBadRequest, "CreateOrUpdateSecret", err)
-		return
-	}
 	opt := web.GetForm(ctx).(*api.CreateOrUpdateSecretOption)
-	isCreated, err := secret_model.CreateOrUpdateSecret(ctx, ctx.Doer.ID, 0, secretName, opt.Data)
+
+	_, created, err := secret_service.CreateOrUpdateSecret(ctx, ctx.Doer.ID, 0, ctx.Params("secretname"), opt.Data)
 	if err != nil {
-		ctx.Error(http.StatusInternalServerError, "CreateOrUpdateSecret", err)
+		if errors.Is(err, util.ErrInvalidArgument) {
+			ctx.Error(http.StatusBadRequest, "CreateOrUpdateSecret", err)
+		} else if errors.Is(err, util.ErrNotExist) {
+			ctx.Error(http.StatusNotFound, "CreateOrUpdateSecret", err)
+		} else {
+			ctx.Error(http.StatusInternalServerError, "CreateOrUpdateSecret", err)
+		}
 		return
 	}
-	if isCreated {
+
+	if created {
 		ctx.Status(http.StatusCreated)
-		return
+	} else {
+		ctx.Status(http.StatusNoContent)
 	}
-
-	ctx.Status(http.StatusNoContent)
 }
 
 // DeleteSecret delete one secret of the user scope
@@ -84,20 +87,15 @@ func DeleteSecret(ctx *context.APIContext) {
 	//   "404":
 	//     "$ref": "#/responses/notFound"
 
-	secretName := ctx.Params(":secretname")
+	err := secret_service.DeleteSecretByName(ctx, ctx.Doer.ID, 0, ctx.Params("secretname"))
-	if err := actions.NameRegexMatch(secretName); err != nil {
-		ctx.Error(http.StatusBadRequest, "DeleteSecret", err)
-		return
-	}
-	err := secret_model.DeleteSecret(
-		ctx, ctx.Doer.ID, 0, secretName,
-	)
-	if secret_model.IsErrSecretNotFound(err) {
-		ctx.NotFound(err)
-		return
-	}
 	if err != nil {
-		ctx.Error(http.StatusInternalServerError, "DeleteSecret", err)
+		if errors.Is(err, util.ErrInvalidArgument) {
+			ctx.Error(http.StatusBadRequest, "DeleteSecret", err)
+		} else if errors.Is(err, util.ErrNotExist) {
+			ctx.Error(http.StatusNotFound, "DeleteSecret", err)
+		} else {
+			ctx.Error(http.StatusInternalServerError, "DeleteSecret", err)
+		}
 		return
 	}
 

routers/web/shared/actions/variables.go

@@ -14,6 +14,7 @@ import (
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/web"
 	"code.gitea.io/gitea/services/forms"
+	secret_service "code.gitea.io/gitea/services/secrets"
 )
 
 func SetVariablesContext(ctx *context.Context, ownerID, repoID int64) {
@@ -33,20 +34,9 @@ func SetVariablesContext(ctx *context.Context, ownerID, repoID int64) {
 // https://docs.github.com/en/actions/learn-github-actions/variables#naming-conventions-for-configuration-variables
 // https://docs.github.com/en/actions/security-guides/encrypted-secrets#naming-your-secrets
 var (
-	nameRx            = regexp.MustCompile("(?i)^[A-Z_][A-Z0-9_]*$")
-	forbiddenPrefixRx = regexp.MustCompile("(?i)^GIT(EA|HUB)_")
-
 	forbiddenEnvNameCIRx = regexp.MustCompile("(?i)^CI")
 )
 
-func NameRegexMatch(name string) error {
-	if !nameRx.MatchString(name) || forbiddenPrefixRx.MatchString(name) {
-		log.Error("Name %s, regex match error", name)
-		return errors.New("name has invalid character")
-	}
-	return nil
-}
-
 func envNameCIRegexMatch(name string) error {
 	if forbiddenEnvNameCIRx.MatchString(name) {
 		log.Error("Env Name cannot be ci")
@@ -58,7 +48,7 @@ func envNameCIRegexMatch(name string) error {
 func CreateVariable(ctx *context.Context, ownerID, repoID int64, redirectURL string) {
 	form := web.GetForm(ctx).(*forms.EditVariableForm)
 
-	if err := NameRegexMatch(form.Name); err != nil {
+	if err := secret_service.ValidateName(form.Name); err != nil {
 		ctx.JSONError(err.Error())
 		return
 	}
@@ -82,7 +72,7 @@ func UpdateVariable(ctx *context.Context, redirectURL string) {
 	id := ctx.ParamsInt64(":variable_id")
 	form := web.GetForm(ctx).(*forms.EditVariableForm)
 
-	if err := NameRegexMatch(form.Name); err != nil {
+	if err := secret_service.ValidateName(form.Name); err != nil {
 		ctx.JSONError(err.Error())
 		return
 	}

routers/web/shared/secrets/secrets.go

@@ -4,13 +4,13 @@
 package secrets
 
 import (
-	"code.gitea.io/gitea/models/db"
 	secret_model "code.gitea.io/gitea/models/secret"
 	"code.gitea.io/gitea/modules/context"
 	"code.gitea.io/gitea/modules/log"
 	"code.gitea.io/gitea/modules/web"
 	"code.gitea.io/gitea/routers/web/shared/actions"
 	"code.gitea.io/gitea/services/forms"
+	secret_service "code.gitea.io/gitea/services/secrets"
 )
 
 func SetSecretsContext(ctx *context.Context, ownerID, repoID int64) {
@@ -26,14 +26,9 @@ func SetSecretsContext(ctx *context.Context, ownerID, repoID int64) {
 func PerformSecretsPost(ctx *context.Context, ownerID, repoID int64, redirectURL string) {
 	form := web.GetForm(ctx).(*forms.AddSecretForm)
 
-	if err := actions.NameRegexMatch(form.Name); err != nil {
+	s, _, err := secret_service.CreateOrUpdateSecret(ctx, ownerID, repoID, form.Name, actions.ReserveLineBreakForTextarea(form.Data))
-		ctx.JSONError(ctx.Tr("secrets.creation.failed"))
-		return
-	}
-
-	s, err := secret_model.InsertEncryptedSecret(ctx, ownerID, repoID, form.Name, actions.ReserveLineBreakForTextarea(form.Data))
 	if err != nil {
-		log.Error("InsertEncryptedSecret: %v", err)
+		log.Error("CreateOrUpdateSecret failed: %v", err)
 		ctx.JSONError(ctx.Tr("secrets.creation.failed"))
 		return
 	}
@@ -45,11 +40,13 @@ func PerformSecretsPost(ctx *context.Context, ownerID, repoID int64, redirectURL
 func PerformSecretsDelete(ctx *context.Context, ownerID, repoID int64, redirectURL string) {
 	id := ctx.FormInt64("id")
 
-	if _, err := db.DeleteByBean(ctx, &secret_model.Secret{ID: id, OwnerID: ownerID, RepoID: repoID}); err != nil {
+	err := secret_service.DeleteSecretByID(ctx, ownerID, repoID, id)
-		log.Error("Delete secret %d failed: %v", id, err)
+	if err != nil {
+		log.Error("DeleteSecretByID(%d) failed: %v", id, err)
 		ctx.JSONError(ctx.Tr("secrets.deletion.failed"))
 		return
 	}
+
 	ctx.Flash.Success(ctx.Tr("secrets.deletion.success"))
 	ctx.JSONRedirect(redirectURL)
 }

services/secrets/secrets.go

@@ -0,0 +1,83 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package secrets
+
+import (
+	"context"
+
+	"code.gitea.io/gitea/models/db"
+	secret_model "code.gitea.io/gitea/models/secret"
+)
+
+func CreateOrUpdateSecret(ctx context.Context, ownerID, repoID int64, name, data string) (*secret_model.Secret, bool, error) {
+	if err := ValidateName(name); err != nil {
+		return nil, false, err
+	}
+
+	s, err := secret_model.FindSecrets(ctx, secret_model.FindSecretsOptions{
+		OwnerID: ownerID,
+		RepoID:  repoID,
+		Name:    name,
+	})
+	if err != nil {
+		return nil, false, err
+	}
+
+	if len(s) == 0 {
+		s, err := secret_model.InsertEncryptedSecret(ctx, ownerID, repoID, name, data)
+		if err != nil {
+			return nil, false, err
+		}
+		return s, true, nil
+	}
+
+	if err := secret_model.UpdateSecret(ctx, s[0].ID, data); err != nil {
+		return nil, false, err
+	}
+
+	return s[0], false, nil
+}
+
+func DeleteSecretByID(ctx context.Context, ownerID, repoID, secretID int64) error {
+	s, err := secret_model.FindSecrets(ctx, secret_model.FindSecretsOptions{
+		OwnerID:  ownerID,
+		RepoID:   repoID,
+		SecretID: secretID,
+	})
+	if err != nil {
+		return err
+	}
+	if len(s) != 1 {
+		return secret_model.ErrSecretNotFound{}
+	}
+
+	return deleteSecret(ctx, s[0])
+}
+
+func DeleteSecretByName(ctx context.Context, ownerID, repoID int64, name string) error {
+	if err := ValidateName(name); err != nil {
+		return err
+	}
+
+	s, err := secret_model.FindSecrets(ctx, secret_model.FindSecretsOptions{
+		OwnerID: ownerID,
+		RepoID:  repoID,
+		Name:    name,
+	})
+	if err != nil {
+		return err
+	}
+	if len(s) != 1 {
+		return secret_model.ErrSecretNotFound{}
+	}
+
+	return deleteSecret(ctx, s[0])
+}
+
+func deleteSecret(ctx context.Context, s *secret_model.Secret) error {
+	if _, err := db.DeleteByID(ctx, s.ID, s); err != nil {
+		return err
+	}
+	return nil
+}

services/secrets/validation.go

@@ -0,0 +1,25 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package secrets
+
+import (
+	"regexp"
+
+	"code.gitea.io/gitea/modules/util"
+)
+
+// https://docs.github.com/en/actions/security-guides/encrypted-secrets#naming-your-secrets
+var (
+	namePattern            = regexp.MustCompile("(?i)^[A-Z_][A-Z0-9_]*$")
+	forbiddenPrefixPattern = regexp.MustCompile("(?i)^GIT(EA|HUB)_")
+
+	ErrInvalidName = util.NewInvalidArgumentErrorf("invalid secret name")
+)
+
+func ValidateName(name string) error {
+	if !namePattern.MatchString(name) || forbiddenPrefixPattern.MatchString(name) {
+		return ErrInvalidName
+	}
+	return nil
+}

templates/swagger/v1_json.tmpl

@@ -1634,8 +1634,8 @@
           "400": {
             "$ref": "#/responses/error"
           },
-          "403": {
+          "404": {
-            "$ref": "#/responses/forbidden"
+            "$ref": "#/responses/notFound"
           }
         }
       },
@@ -1671,8 +1671,11 @@
           "204": {
             "description": "delete one secret of the organization"
           },
-          "403": {
+          "400": {
-            "$ref": "#/responses/forbidden"
+            "$ref": "#/responses/error"
+          },
+          "404": {
+            "$ref": "#/responses/notFound"
           }
         }
       }
@@ -3283,8 +3286,8 @@
           "400": {
             "$ref": "#/responses/error"
           },
-          "403": {
+          "404": {
-            "$ref": "#/responses/forbidden"
+            "$ref": "#/responses/notFound"
           }
         }
       },
@@ -3327,8 +3330,11 @@
           "204": {
             "description": "delete one secret of the organization"
           },
-          "403": {
+          "400": {
-            "$ref": "#/responses/forbidden"
+            "$ref": "#/responses/error"
+          },
+          "404": {
+            "$ref": "#/responses/notFound"
           }
         }
       }

tests/integration/api_repo_secrets_test.go

@@ -0,0 +1,103 @@
+// Copyright 2023 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package integration
+
+import (
+	"fmt"
+	"net/http"
+	"testing"
+
+	auth_model "code.gitea.io/gitea/models/auth"
+	repo_model "code.gitea.io/gitea/models/repo"
+	"code.gitea.io/gitea/models/unittest"
+	user_model "code.gitea.io/gitea/models/user"
+	api "code.gitea.io/gitea/modules/structs"
+	"code.gitea.io/gitea/tests"
+)
+
+func TestAPIRepoSecrets(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	repo := unittest.AssertExistsAndLoadBean(t, &repo_model.Repository{ID: 1})
+	user := unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: repo.OwnerID})
+	session := loginUser(t, user.Name)
+	token := getTokenForLoggedInUser(t, session, auth_model.AccessTokenScopeWriteRepository)
+
+	t.Run("Create", func(t *testing.T) {
+		cases := []struct {
+			Name           string
+			ExpectedStatus int
+		}{
+			{
+				Name:           "",
+				ExpectedStatus: http.StatusNotFound,
+			},
+			{
+				Name:           "-",
+				ExpectedStatus: http.StatusBadRequest,
+			},
+			{
+				Name:           "_",
+				ExpectedStatus: http.StatusCreated,
+			},
+			{
+				Name:           "secret",
+				ExpectedStatus: http.StatusCreated,
+			},
+			{
+				Name:           "2secret",
+				ExpectedStatus: http.StatusBadRequest,
+			},
+			{
+				Name:           "GITEA_secret",
+				ExpectedStatus: http.StatusBadRequest,
+			},
+			{
+				Name:           "GITHUB_secret",
+				ExpectedStatus: http.StatusBadRequest,
+			},
+		}
+
+		for _, c := range cases {
+			req := NewRequestWithJSON(t, "PUT", fmt.Sprintf("/api/v1/repos/%s/actions/secrets/%s?token=%s", repo.FullName(), c.Name, token), api.CreateOrUpdateSecretOption{
+				Data: "data",
+			})
+			MakeRequest(t, req, c.ExpectedStatus)
+		}
+	})
+
+	t.Run("Update", func(t *testing.T) {
+		name := "update_secret"
+		url := fmt.Sprintf("/api/v1/repos/%s/actions/secrets/%s?token=%s", repo.FullName(), name, token)
+
+		req := NewRequestWithJSON(t, "PUT", url, api.CreateOrUpdateSecretOption{
+			Data: "initial",
+		})
+		MakeRequest(t, req, http.StatusCreated)
+
+		req = NewRequestWithJSON(t, "PUT", url, api.CreateOrUpdateSecretOption{
+			Data: "changed",
+		})
+		MakeRequest(t, req, http.StatusNoContent)
+	})
+
+	t.Run("Delete", func(t *testing.T) {
+		name := "delete_secret"
+		url := fmt.Sprintf("/api/v1/repos/%s/actions/secrets/%s?token=%s", repo.FullName(), name, token)
+
+		req := NewRequestWithJSON(t, "PUT", url, api.CreateOrUpdateSecretOption{
+			Data: "initial",
+		})
+		MakeRequest(t, req, http.StatusCreated)
+
+		req = NewRequest(t, "DELETE", url)
+		MakeRequest(t, req, http.StatusNoContent)
+
+		req = NewRequest(t, "DELETE", url)
+		MakeRequest(t, req, http.StatusNotFound)
+
+		req = NewRequest(t, "DELETE", fmt.Sprintf("/api/v1/repos/%s/actions/secrets/000?token=%s", repo.FullName(), token))
+		MakeRequest(t, req, http.StatusBadRequest)
+	})
+}