diff --git a/.golangci.yaml b/.golangci.yaml index f3e00a706..703304a52 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -166,6 +166,13 @@ linters: - contextcheck - forcetypeassert - gci + - gomnd + +issues: + exclude-rules: + - path: 'fixtures|cmd/agent/flags.go|cmd/server/flags.go|pipeline/backend/kubernetes/flags.go|_test.go' + linters: + - gomnd run: timeout: 15m diff --git a/agent/rpc/auth_client_grpc.go b/agent/rpc/auth_client_grpc.go index 1ef645d43..94a01151e 100644 --- a/agent/rpc/auth_client_grpc.go +++ b/agent/rpc/auth_client_grpc.go @@ -40,7 +40,7 @@ func NewAuthGrpcClient(conn *grpc.ClientConn, agentToken string, agentID int64) } func (c *AuthClient) Auth() (string, int64, error) { - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) //nolint: gomnd defer cancel() req := &proto.AuthRequest{ diff --git a/agent/rpc/client_grpc.go b/agent/rpc/client_grpc.go index 39e7e2365..5648d31cf 100644 --- a/agent/rpc/client_grpc.go +++ b/agent/rpc/client_grpc.go @@ -53,8 +53,8 @@ func (c *client) Close() error { func (c *client) newBackOff() backoff.BackOff { b := backoff.NewExponentialBackOff() - b.MaxInterval = 10 * time.Second - b.InitialInterval = 10 * time.Millisecond + b.MaxInterval = 10 * time.Second //nolint: gomnd + b.InitialInterval = 10 * time.Millisecond //nolint: gomnd return b } diff --git a/agent/runner.go b/agent/runner.go index 21413f9ef..22a8729ef 100644 --- a/agent/runner.go +++ b/agent/runner.go @@ -158,7 +158,7 @@ func (r *Runner) Run(runnerCtx context.Context) error { if canceled.IsSet() { state.Error = "" - state.ExitCode = 137 + state.ExitCode = pipeline.ExitCodeKilled } else if err != nil { pExitError := &pipeline.ExitError{} switch { @@ -166,7 +166,7 @@ func (r *Runner) Run(runnerCtx context.Context) error { state.ExitCode = pExitError.Code case errors.Is(err, pipeline.ErrCancel): state.Error = "" - state.ExitCode = 137 + state.ExitCode = pipeline.ExitCodeKilled canceled.SetTo(true) default: state.ExitCode = 1 diff --git a/cli/deploy/deploy.go b/cli/deploy/deploy.go index bcc2019a8..9a9ffd32c 100644 --- a/cli/deploy/deploy.go +++ b/cli/deploy/deploy.go @@ -106,7 +106,8 @@ func deploy(c *cli.Context) error { } } - env := c.Args().Get(2) + envArgIndex := 2 + env := c.Args().Get(envArgIndex) if env == "" { return fmt.Errorf("please specify the target environment (i.e. production)") } diff --git a/cli/exec/exec.go b/cli/exec/exec.go index aa13b37bf..44dabe116 100644 --- a/cli/exec/exec.go +++ b/cli/exec/exec.go @@ -123,13 +123,13 @@ func execWithAxis(c *cli.Context, file, repoPath string, axis matrix.Axis) error pipelineEnv := make(map[string]string) for _, env := range c.StringSlice("env") { - envs := strings.SplitN(env, "=", 2) - pipelineEnv[envs[0]] = envs[1] - if oldVar, exists := environ[envs[0]]; exists { + before, after, _ := strings.Cut(env, "=") + pipelineEnv[before] = after + if oldVar, exists := environ[before]; exists { // override existing values, but print a warning - log.Warn().Msgf("environment variable '%s' had value '%s', but got overwritten", envs[0], oldVar) + log.Warn().Msgf("environment variable '%s' had value '%s', but got overwritten", before, oldVar) } - environ[envs[0]] = envs[1] + environ[before] = after } tmpl, err := envsubst.ParseFile(file) @@ -244,8 +244,16 @@ func execWithAxis(c *cli.Context, file, repoPath string, axis matrix.Axis) error ).Run(c.Context) } +// convertPathForWindows converts a path to use slash separators +// for Windows. If the path is a Windows volume name like C:, it +// converts it to an absolute root path starting with slash (e.g. +// C: -> /c). Otherwise it just converts backslash separators to +// slashes. func convertPathForWindows(path string) string { base := filepath.VolumeName(path) + + // Check if path is volume name like C: + //nolint: gomnd if len(base) == 2 { path = strings.TrimPrefix(path, base) base = strings.ToLower(strings.TrimSuffix(base, ":")) diff --git a/cli/internal/util.go b/cli/internal/util.go index 47eff719a..35a1d8da0 100644 --- a/cli/internal/util.go +++ b/cli/internal/util.go @@ -107,11 +107,11 @@ func ParseRepo(client woodpecker.Client, str string) (repoID int64, err error) { func ParseKeyPair(p []string) map[string]string { params := map[string]string{} for _, i := range p { - parts := strings.SplitN(i, "=", 2) - if len(parts) != 2 { + before, after, ok := strings.Cut(i, "=") + if !ok || before == "" { continue } - params[parts[0]] = parts[1] + params[before] = after } return params } diff --git a/cli/pipeline/create.go b/cli/pipeline/create.go index c64362bf8..3db896a9f 100644 --- a/cli/pipeline/create.go +++ b/cli/pipeline/create.go @@ -60,9 +60,9 @@ func pipelineCreate(c *cli.Context) error { variables := make(map[string]string) for _, vaz := range c.StringSlice("var") { - sp := strings.SplitN(vaz, "=", 2) - if len(sp) == 2 { - variables[sp[0]] = sp[1] + before, after, _ := strings.Cut(vaz, "=") + if before != "" && after != "" { + variables[before] = after } } diff --git a/cli/pipeline/list.go b/cli/pipeline/list.go index 5a2fbaf92..5cac3aa4d 100644 --- a/cli/pipeline/list.go +++ b/cli/pipeline/list.go @@ -24,6 +24,7 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/cli/internal" ) +//nolint:gomnd var pipelineListCmd = &cli.Command{ Name: "ls", Usage: "show pipeline history", diff --git a/cli/pipeline/logs.go b/cli/pipeline/logs.go index 8951b2052..116d68a4d 100644 --- a/cli/pipeline/logs.go +++ b/cli/pipeline/logs.go @@ -41,12 +41,14 @@ func pipelineLogs(c *cli.Context) error { return err } - number, err := strconv.ParseInt(c.Args().Get(1), 10, 64) + numberArgIndex := 1 + number, err := strconv.ParseInt(c.Args().Get(numberArgIndex), 10, 64) if err != nil { return err } - step, err := strconv.ParseInt(c.Args().Get(2), 10, 64) + stepArgIndex := 2 + step, err := strconv.ParseInt(c.Args().Get(stepArgIndex), 10, 64) if err != nil { return err } diff --git a/cli/setup/token_fetcher.go b/cli/setup/token_fetcher.go index 54dd18eee..e96772cac 100644 --- a/cli/setup/token_fetcher.go +++ b/cli/setup/token_fetcher.go @@ -61,7 +61,7 @@ func setupRouter(tokenReceived chan string) *gin.Engine { c.Writer.Header().Set("Access-Control-Allow-Methods", "POST, OPTIONS, GET, PUT") if c.Request.Method == "OPTIONS" { - c.AbortWithStatus(204) + c.AbortWithStatus(http.StatusNoContent) return } @@ -76,7 +76,7 @@ func setupRouter(tokenReceived chan string) *gin.Engine { err := c.BindJSON(&data) if err != nil { log.Debug().Err(err).Msg("Failed to bind JSON") - c.JSON(400, gin.H{ + c.JSON(http.StatusBadRequest, gin.H{ "error": "invalid request", }) return @@ -84,7 +84,7 @@ func setupRouter(tokenReceived chan string) *gin.Engine { tokenReceived <- data.Token - c.JSON(200, gin.H{ + c.JSON(http.StatusOK, gin.H{ "ok": "true", }) }) @@ -111,7 +111,10 @@ func openBrowser(url string) error { } func randomPort() int { - s1 := rand.NewSource(time.Now().UnixNano()) - r1 := rand.New(s1) - return r1.Intn(10000) + 20000 + const minPort = 10000 + const maxPort = 65535 + + source := rand.NewSource(time.Now().UnixNano()) + rand := rand.New(source) + return rand.Intn(maxPort-minPort+1) + minPort } diff --git a/cmd/agent/core/agent.go b/cmd/agent/core/agent.go index 8a1c972cd..c845222dc 100644 --- a/cmd/agent/core/agent.go +++ b/cmd/agent/core/agent.go @@ -89,7 +89,7 @@ func run(c *cli.Context, backends []types.Backend) error { agentToken := c.String("grpc-token") authClient := agentRpc.NewAuthGrpcClient(authConn, agentToken, agentConfig.AgentID) - authInterceptor, err := agentRpc.NewAuthInterceptor(authClient, 30*time.Minute) + authInterceptor, err := agentRpc.NewAuthInterceptor(authClient, 30*time.Minute) //nolint: gomnd if err != nil { return err } @@ -276,12 +276,12 @@ func stringSliceAddToMap(sl []string, m map[string]string) error { m = make(map[string]string) } for _, v := range utils.StringSliceDeleteEmpty(sl) { - parts := strings.SplitN(v, "=", 2) - switch len(parts) { - case 2: - m[parts[0]] = parts[1] - case 1: - return fmt.Errorf("key '%s' does not have a value assigned", parts[0]) + before, after, _ := strings.Cut(v, "=") + switch { + case before != "" && after != "": + m[before] = after + case before != "": + return fmt.Errorf("key '%s' does not have a value assigned", before) default: return fmt.Errorf("empty string in slice") } diff --git a/cmd/agent/core/flags.go b/cmd/agent/core/flags.go index 001757d0e..1034438f2 100644 --- a/cmd/agent/core/flags.go +++ b/cmd/agent/core/flags.go @@ -22,6 +22,7 @@ import ( "github.com/urfave/cli/v2" ) +//nolint:gomnd var flags = []cli.Flag{ &cli.StringFlag{ EnvVars: []string{"WOODPECKER_SERVER"}, diff --git a/cmd/agent/core/health.go b/cmd/agent/core/health.go index 80d5135ea..c05765bcc 100644 --- a/cmd/agent/core/health.go +++ b/cmd/agent/core/health.go @@ -39,14 +39,14 @@ func initHealth() { func handleHeartbeat(w http.ResponseWriter, _ *http.Request) { if counter.Healthy() { - w.WriteHeader(200) + w.WriteHeader(http.StatusOK) } else { - w.WriteHeader(500) + w.WriteHeader(http.StatusInternalServerError) } } func handleVersion(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(200) + w.WriteHeader(http.StatusOK) w.Header().Add("Content-Type", "text/json") err := json.NewEncoder(w).Encode(versionResp{ Source: "https://github.com/woodpecker-ci/woodpecker", @@ -59,9 +59,9 @@ func handleVersion(w http.ResponseWriter, _ *http.Request) { func handleStats(w http.ResponseWriter, _ *http.Request) { if counter.Healthy() { - w.WriteHeader(200) + w.WriteHeader(http.StatusOK) } else { - w.WriteHeader(500) + w.WriteHeader(http.StatusInternalServerError) } w.Header().Add("Content-Type", "text/json") if _, err := counter.WriteTo(w); err != nil { @@ -92,8 +92,8 @@ func pinger(c *cli.Context) error { return err } defer resp.Body.Close() - if resp.StatusCode != 200 { - return fmt.Errorf("agent returned non-200 status code") + if resp.StatusCode != http.StatusOK { + return fmt.Errorf("agent returned non-http.StatusOK status code") } return nil } diff --git a/pipeline/backend/docker/convert.go b/pipeline/backend/docker/convert.go index 86afd070f..1e4ad589b 100644 --- a/pipeline/backend/docker/convert.go +++ b/pipeline/backend/docker/convert.go @@ -27,6 +27,9 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/pipeline/backend/types" ) +// Valid container volumes must have at least two components, source and destination. +const minVolumeComponents = 2 + // returns a container configuration. func (e *docker) toConfig(step *types.Step) *container.Config { config := &container.Config{ @@ -131,7 +134,7 @@ func toVol(paths []string) map[string]struct{} { if err != nil { continue } - if len(parts) < 2 { + if len(parts) < minVolumeComponents { continue } set[parts[1]] = struct{}{} @@ -149,16 +152,18 @@ func toEnv(env map[string]string) []string { return envs } -// helper function that converts a slice of device paths to a slice of -// container.DeviceMapping. +// toDev converts a slice of volume paths to a set of device mappings for +// use in a Docker container config. It handles splitting the volume paths +// into host and container paths, and setting default permissions. func toDev(paths []string) []container.DeviceMapping { var devices []container.DeviceMapping + for _, path := range paths { parts, err := splitVolumeParts(path) if err != nil { continue } - if len(parts) < 2 { + if len(parts) < minVolumeComponents { continue } if strings.HasSuffix(parts[1], ":ro") || strings.HasSuffix(parts[1], ":rw") { @@ -183,7 +188,15 @@ func encodeAuthToBase64(authConfig types.Auth) (string, error) { return base64.URLEncoding.EncodeToString(buf), nil } -// helper function that split volume path +// splitVolumeParts splits a volume string into its constituent parts. +// +// The parts are: +// +// 1. The path on the host machine +// 2. The path inside the container +// 3. The read/write mode +// +// It handles Windows and Linux style volume paths. func splitVolumeParts(volumeParts string) ([]string, error) { pattern := `^((?:[\w]\:)?[^\:]*)\:((?:[\w]\:)?[^\:]*)(?:\:([rwom]*))?` r, err := regexp.Compile(pattern) diff --git a/pipeline/backend/kubernetes/kubernetes.go b/pipeline/backend/kubernetes/kubernetes.go index 69b2a7dde..3567100a6 100644 --- a/pipeline/backend/kubernetes/kubernetes.go +++ b/pipeline/backend/kubernetes/kubernetes.go @@ -42,6 +42,8 @@ import ( const ( EngineName = "kubernetes" + // TODO 5 seconds is against best practice, k3s didn't work otherwise + defaultResyncDuration = 5 * time.Second ) var defaultDeleteOptions = newDefaultDeleteOptions() @@ -249,8 +251,7 @@ func (e *kube) WaitStep(ctx context.Context, step *types.Step, taskUUID string) } } - // TODO 5 seconds is against best practice, k3s didn't work otherwise - si := informers.NewSharedInformerFactoryWithOptions(e.client, 5*time.Second, informers.WithNamespace(e.config.Namespace)) + si := informers.NewSharedInformerFactoryWithOptions(e.client, defaultResyncDuration, informers.WithNamespace(e.config.Namespace)) if _, err := si.Core().V1().Pods().Informer().AddEventHandler( cache.ResourceEventHandlerFuncs{ UpdateFunc: podUpdated, @@ -322,8 +323,7 @@ func (e *kube) TailStep(ctx context.Context, step *types.Step, taskUUID string) } } - // TODO 5 seconds is against best practice, k3s didn't work otherwise - si := informers.NewSharedInformerFactoryWithOptions(e.client, 5*time.Second, informers.WithNamespace(e.config.Namespace)) + si := informers.NewSharedInformerFactoryWithOptions(e.client, defaultResyncDuration, informers.WithNamespace(e.config.Namespace)) if _, err := si.Core().V1().Pods().Informer().AddEventHandler( cache.ResourceEventHandlerFuncs{ UpdateFunc: podUpdated, diff --git a/pipeline/const.go b/pipeline/const.go new file mode 100644 index 000000000..a4bec9789 --- /dev/null +++ b/pipeline/const.go @@ -0,0 +1,17 @@ +// Copyright 2023 Woodpecker Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package pipeline + +const ExitCodeKilled int = 137 diff --git a/pipeline/frontend/metadata/drone_compatibility_test.go b/pipeline/frontend/metadata/drone_compatibility_test.go index 292d45e67..c2334585c 100644 --- a/pipeline/frontend/metadata/drone_compatibility_test.go +++ b/pipeline/frontend/metadata/drone_compatibility_test.go @@ -113,11 +113,11 @@ DRONE_TARGET_BRANCH=main` func convertListToEnvMap(t *testing.T, list string) map[string]string { result := make(map[string]string) for _, s := range strings.Split(list, "\n") { - ss := strings.SplitN(strings.TrimSpace(s), "=", 2) - if len(ss) != 2 { + before, after, _ := strings.Cut(strings.TrimSpace(s), "=") + if before == "" || after == "" { t.Fatal("helper function got invalid test data") } - result[ss[0]] = ss[1] + result[before] = after } return result } diff --git a/pipeline/frontend/metadata/environment.go b/pipeline/frontend/metadata/environment.go index d63eae018..d32378d02 100644 --- a/pipeline/frontend/metadata/environment.go +++ b/pipeline/frontend/metadata/environment.go @@ -38,7 +38,7 @@ func (m *Metadata) Environ() map[string]string { ) branchParts := strings.Split(m.Curr.Commit.Refspec, ":") - if len(branchParts) == 2 { + if len(branchParts) == 2 { //nolint: gomnd sourceBranch = branchParts[0] targetBranch = branchParts[1] } diff --git a/pipeline/frontend/yaml/types/base/map.go b/pipeline/frontend/yaml/types/base/map.go index 849adc96d..daa83a44f 100644 --- a/pipeline/frontend/yaml/types/base/map.go +++ b/pipeline/frontend/yaml/types/base/map.go @@ -31,13 +31,7 @@ func (s *SliceOrMap) UnmarshalYAML(unmarshal func(any) error) error { for _, s := range sliceType { if str, ok := s.(string); ok { str := strings.TrimSpace(str) - keyValueSlice := strings.SplitN(str, "=", 2) - - key := keyValueSlice[0] - val := "" - if len(keyValueSlice) == 2 { - val = keyValueSlice[1] - } + key, val, _ := strings.Cut(str, "=") parts[key] = val } else { return fmt.Errorf("cannot unmarshal '%v' of type %T into a string value", s, s) diff --git a/pipeline/frontend/yaml/types/volume.go b/pipeline/frontend/yaml/types/volume.go index d07a3eec6..3d925e25b 100644 --- a/pipeline/frontend/yaml/types/volume.go +++ b/pipeline/frontend/yaml/types/volume.go @@ -68,6 +68,7 @@ func (v *Volumes) UnmarshalYAML(unmarshal func(any) error) error { } elts := strings.SplitN(name, ":", 3) var vol *Volume + //nolint: gomnd switch { case len(elts) == 1: vol = &Volume{ diff --git a/pipeline/shared/replace_secrets.go b/pipeline/shared/replace_secrets.go index 668874b9a..223253800 100644 --- a/pipeline/shared/replace_secrets.go +++ b/pipeline/shared/replace_secrets.go @@ -16,11 +16,21 @@ package shared import "strings" +// NewSecretsReplacer creates a new strings.Replacer to replace sensitive +// strings with asterisks. It takes a slice of secrets strings as input +// and returns a populated strings.Replacer that will replace those +// secrets with asterisks. Each secret string is split on newlines to +// handle multi-line secrets. func NewSecretsReplacer(secrets []string) *strings.Replacer { var oldnew []string + + // Strings shorter than minStringLength are not considered secrets. + // Do not sanitize them. + const minStringLength = 3 + for _, old := range secrets { old = strings.TrimSpace(old) - if len(old) <= 3 { + if len(old) <= minStringLength { continue } // since replacer is executed on each line we have to split multi-line-secrets diff --git a/server/api/login.go b/server/api/login.go index 8e72285dc..4daea4a57 100644 --- a/server/api/login.go +++ b/server/api/login.go @@ -86,8 +86,8 @@ func HandleAuth(c *gin.Context) { if server.Config.Permissions.Orgs.IsConfigured { teams, terr := _forge.Teams(c, tmpuser) if terr != nil || !server.Config.Permissions.Orgs.IsMember(teams) { - log.Error().Err(terr).Msgf("cannot verify team membership for %s", u.Login) - c.Redirect(303, server.Config.Server.RootPath+"/login?error=access_denied") + log.Error().Err(terr).Msgf("cannot verify team membership for %s.", u.Login) + c.Redirect(http.StatusSeeOther, server.Config.Server.RootPath+"/login?error=access_denied") return } } diff --git a/server/api/registry.go b/server/api/registry.go index 60e6648ff..be2cc6b76 100644 --- a/server/api/registry.go +++ b/server/api/registry.go @@ -44,7 +44,7 @@ func GetRegistry(c *gin.Context) { handleDBError(c, err) return } - c.JSON(200, registry.Copy()) + c.JSON(http.StatusOK, registry.Copy()) } // PostRegistry diff --git a/server/cache/membership.go b/server/cache/membership.go index 3c73c12e0..45b6af5d1 100644 --- a/server/cache/membership.go +++ b/server/cache/membership.go @@ -39,6 +39,7 @@ type membershipCache struct { // NewMembershipService creates a new membership service. func NewMembershipService(f forge.Forge) MembershipService { + //nolint:gomnd return &membershipCache{ ttl: 10 * time.Minute, forge: f, diff --git a/server/forge/bitbucket/bitbucket.go b/server/forge/bitbucket/bitbucket.go index 095836be9..c1590fde7 100644 --- a/server/forge/bitbucket/bitbucket.go +++ b/server/forge/bitbucket/bitbucket.go @@ -38,6 +38,7 @@ import ( const ( DefaultAPI = "https://api.bitbucket.org" DefaultURL = "https://bitbucket.org" + pageSize = 100 ) // Opts are forge options for bitbucket @@ -141,7 +142,7 @@ func (c *config) Refresh(ctx context.Context, user *model.User) (bool, error) { func (c *config) Teams(ctx context.Context, u *model.User) ([]*model.Team, error) { return shared_utils.Paginate(func(page int) ([]*model.Team, error) { opts := &internal.ListWorkspacesOpts{ - PageLen: 100, + PageLen: pageSize, Page: page, Role: "member", } @@ -190,7 +191,7 @@ func (c *config) Repos(ctx context.Context, u *model.User) ([]*model.Repo, error workspaces, err := shared_utils.Paginate(func(page int) ([]*internal.Workspace, error) { resp, err := client.ListWorkspaces(&internal.ListWorkspacesOpts{ Page: page, - PageLen: 100, + PageLen: pageSize, Role: "member", }) if err != nil { diff --git a/server/forge/bitbucket/fixtures/handler.go b/server/forge/bitbucket/fixtures/handler.go index a2a42d8aa..ab2104f7b 100644 --- a/server/forge/bitbucket/fixtures/handler.go +++ b/server/forge/bitbucket/fixtures/handler.go @@ -45,26 +45,26 @@ func Handler() http.Handler { func getOauth(c *gin.Context) { if c.PostForm("error") == "invalid_scope" { - c.String(500, "") + c.String(http.StatusInternalServerError, "") } switch c.PostForm("code") { case "code_bad_request": - c.String(500, "") + c.String(http.StatusInternalServerError, "") return case "code_user_not_found": - c.String(200, tokenNotFoundPayload) + c.String(http.StatusOK, tokenNotFoundPayload) return } switch c.PostForm("refresh_token") { case "refresh_token_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") case "refresh_token_is_empty": c.Header("Content-Type", "application/json") - c.String(200, "{}") + c.String(http.StatusOK, "{}") default: c.Header("Content-Type", "application/json") - c.String(200, tokenPayload) + c.String(http.StatusOK, tokenPayload) } } @@ -74,12 +74,12 @@ func getWorkspaces(c *gin.Context) { switch c.Request.Header.Get("Authorization") { case "Bearer teams_not_found", "Bearer c81e728d": - c.String(404, "") + c.String(http.StatusNotFound, "") default: if c.Query("page") == "" || c.Query("page") == "1" { - c.String(200, workspacesPayload) + c.String(http.StatusOK, workspacesPayload) } else { - c.String(200, "{\"values\":[]}") + c.String(http.StatusOK, "{\"values\":[]}") } } } @@ -87,25 +87,25 @@ func getWorkspaces(c *gin.Context) { func getRepo(c *gin.Context) { switch c.Param("name") { case "not_found", "repo_unknown", "repo_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") case "permission_read", "permission_write", "permission_admin": - c.String(200, fmt.Sprintf(permissionRepoPayload, c.Param("name"))) + c.String(http.StatusOK, fmt.Sprintf(permissionRepoPayload, c.Param("name"))) default: - c.String(200, repoPayload) + c.String(http.StatusOK, repoPayload) } } func getRepoHooks(c *gin.Context) { switch c.Param("name") { case "hooks_not_found", "repo_no_hooks": - c.String(404, "") + c.String(http.StatusNotFound, "") case "hook_empty": - c.String(200, "{}") + c.String(http.StatusOK, "{}") default: if c.Query("page") == "" || c.Query("page") == "1" { - c.String(200, repoHookPayload) + c.String(http.StatusOK, repoHookPayload) } else { - c.String(200, "{\"values\":[]}") + c.String(http.StatusOK, "{\"values\":[]}") } } } @@ -113,83 +113,83 @@ func getRepoHooks(c *gin.Context) { func getRepoFile(c *gin.Context) { switch c.Param("file") { case "dir": - c.String(200, repoDirPayload) + c.String(http.StatusOK, repoDirPayload) case "dir_not_found/": - c.String(404, "") + c.String(http.StatusNotFound, "") case "file_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") default: - c.String(200, repoFilePayload) + c.String(http.StatusOK, repoFilePayload) } } func getBranchHead(c *gin.Context) { switch c.Param("commit") { case "branch_name": - c.String(200, branchCommitsPayload) + c.String(http.StatusOK, branchCommitsPayload) default: - c.String(404, "") + c.String(http.StatusNotFound, "") } } func getPullRequests(c *gin.Context) { switch c.Param("name") { case "repo_name": - c.String(200, pullRequestsPayload) + c.String(http.StatusOK, pullRequestsPayload) default: - c.String(404, "") + c.String(http.StatusNotFound, "") } } func createRepoStatus(c *gin.Context) { switch c.Param("name") { case "repo_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") default: - c.String(200, "") + c.String(http.StatusOK, "") } } func createRepoHook(c *gin.Context) { - c.String(200, "") + c.String(http.StatusOK, "") } func deleteRepoHook(c *gin.Context) { switch c.Param("name") { case "hook_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") default: - c.String(200, "") + c.String(http.StatusOK, "") } } func getUser(c *gin.Context) { switch c.Request.Header.Get("Authorization") { case "Bearer user_not_found", "Bearer a87ff679": - c.String(404, "") + c.String(http.StatusNotFound, "") default: - c.String(200, userPayload) + c.String(http.StatusOK, userPayload) } } func getUserRepos(c *gin.Context) { switch c.Request.Header.Get("Authorization") { case "Bearer repos_not_found", "Bearer 70efdf2e": - c.String(404, "") + c.String(http.StatusNotFound, "") default: if c.Query("page") == "" || c.Query("page") == "1" { - c.String(200, userRepoPayload) + c.String(http.StatusOK, userRepoPayload) } else { - c.String(200, "{\"values\":[]}") + c.String(http.StatusOK, "{\"values\":[]}") } } } func getPermissions(c *gin.Context) { if c.Query("page") == "" || c.Query("page") == "1" { - c.String(200, permissionsPayLoad) + c.String(http.StatusOK, permissionsPayLoad) } else { - c.String(200, "{\"values\":[]}") + c.String(http.StatusOK, "{\"values\":[]}") } } diff --git a/server/forge/bitbucket/internal/client.go b/server/forge/bitbucket/internal/client.go index 779029ca9..16e4380bc 100644 --- a/server/forge/bitbucket/internal/client.go +++ b/server/forge/bitbucket/internal/client.go @@ -53,6 +53,7 @@ const ( pathPullRequests = "%s/2.0/repositories/%s/%s/pullrequests?%s" pathBranchCommits = "%s/2.0/repositories/%s/%s/commits/%s" pathDir = "%s/2.0/repositories/%s/%s/src/%s%s" + pageSize = 100 ) type Client struct { @@ -115,7 +116,7 @@ func (c *Client) ListRepos(workspace string, opts *ListOpts) (*RepoResp, error) func (c *Client) ListReposAll(workspace string) ([]*Repo, error) { return shared_utils.Paginate(func(page int) ([]*Repo, error) { - resp, err := c.ListRepos(workspace, &ListOpts{Page: page, PageLen: 100}) + resp, err := c.ListRepos(workspace, &ListOpts{Page: page, PageLen: pageSize}) if err != nil { return nil, err } @@ -183,7 +184,7 @@ func (c *Client) ListPermissions(opts *ListOpts) (*RepoPermResp, error) { func (c *Client) ListPermissionsAll() ([]*RepoPerm, error) { return shared_utils.Paginate(func(page int) ([]*RepoPerm, error) { - resp, err := c.ListPermissions(&ListOpts{Page: page, PageLen: 100}) + resp, err := c.ListPermissions(&ListOpts{Page: page, PageLen: pageSize}) if err != nil { return nil, err } @@ -213,7 +214,7 @@ func (c *Client) GetBranchHead(owner, name, branch string) (*Commit, error) { func (c *Client) GetUserWorkspaceMembership(workspace, user string) (string, error) { out := new(WorkspaceMembershipResp) - opts := &ListOpts{Page: 1, PageLen: 100} + opts := &ListOpts{Page: 1, PageLen: pageSize} for { uri := fmt.Sprintf(pathOrgPerms, c.base, workspace, opts.Encode()) _, err := c.do(uri, get, nil, out) diff --git a/server/forge/bitbucketdatacenter/bitbucketdatacenter.go b/server/forge/bitbucketdatacenter/bitbucketdatacenter.go index 386be1a80..b881a701d 100644 --- a/server/forge/bitbucketdatacenter/bitbucketdatacenter.go +++ b/server/forge/bitbucketdatacenter/bitbucketdatacenter.go @@ -34,6 +34,8 @@ import ( "go.woodpecker-ci.org/woodpecker/v2/server/store" ) +const listLimit = 250 + // Opts defines configuration options. type Opts struct { URL string // Bitbucket server url for API access. @@ -166,7 +168,7 @@ func (c *client) Repo(ctx context.Context, u *model.User, rID model.ForgeRemoteI var repo *bb.Repository if rID.IsValid() { - opts := &bb.RepositorySearchOptions{Permission: bb.PermissionRepoWrite, ListOptions: bb.ListOptions{Limit: 250}} + opts := &bb.RepositorySearchOptions{Permission: bb.PermissionRepoWrite, ListOptions: bb.ListOptions{Limit: listLimit}} for { repos, resp, err := bc.Projects.SearchRepositories(ctx, opts) if err != nil { @@ -213,7 +215,7 @@ func (c *client) Repos(ctx context.Context, u *model.User) ([]*model.Repo, error return nil, fmt.Errorf("unable to create bitbucket client: %w", err) } - opts := &bb.RepositorySearchOptions{Permission: bb.PermissionRepoWrite, ListOptions: bb.ListOptions{Limit: 250}} + opts := &bb.RepositorySearchOptions{Permission: bb.PermissionRepoWrite, ListOptions: bb.ListOptions{Limit: listLimit}} var all []*model.Repo for { repos, resp, err := bc.Projects.SearchRepositories(ctx, opts) @@ -231,7 +233,7 @@ func (c *client) Repos(ctx context.Context, u *model.User) ([]*model.Repo, error } // Add admin permissions to relevant repositories - opts = &bb.RepositorySearchOptions{Permission: bb.PermissionRepoAdmin, ListOptions: bb.ListOptions{Limit: 250}} + opts = &bb.RepositorySearchOptions{Permission: bb.PermissionRepoAdmin, ListOptions: bb.ListOptions{Limit: listLimit}} for { repos, resp, err := bc.Projects.SearchRepositories(ctx, opts) if err != nil { diff --git a/server/forge/bitbucketdatacenter/convert.go b/server/forge/bitbucketdatacenter/convert.go index 0e9f68510..bef0c30d5 100644 --- a/server/forge/bitbucketdatacenter/convert.go +++ b/server/forge/bitbucketdatacenter/convert.go @@ -136,7 +136,10 @@ func convertPullRequestEvent(ev *bb.PullRequestEvent, baseURL string) *model.Pip func authorLabel(name string) string { var result string - if len(name) > 40 { + + const maxNameLength = 40 + + if len(name) > maxNameLength { result = name[0:37] + "..." } else { result = name diff --git a/server/forge/gitea/fixtures/handler.go b/server/forge/gitea/fixtures/handler.go index d6ff48fa1..7bc2bbb42 100644 --- a/server/forge/gitea/fixtures/handler.go +++ b/server/forge/gitea/fixtures/handler.go @@ -43,35 +43,35 @@ func Handler() http.Handler { func listRepoHooks(c *gin.Context) { page := c.Query("page") if page != "" && page != "1" { - c.String(200, "[]") + c.String(http.StatusOK, "[]") } else { - c.String(200, listRepoHookPayloads) + c.String(http.StatusOK, listRepoHookPayloads) } } func getRepo(c *gin.Context) { switch c.Param("name") { case "repo_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") default: - c.String(200, repoPayload) + c.String(http.StatusOK, repoPayload) } } func getRepoByID(c *gin.Context) { switch c.Param("id") { case "repo_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") default: - c.String(200, repoPayload) + c.String(http.StatusOK, repoPayload) } } func createRepoCommitStatus(c *gin.Context) { if c.Param("commit") == "v1.0.0" || c.Param("commit") == "9ecad50" { - c.String(200, repoPayload) + c.String(http.StatusOK, repoPayload) } - c.String(404, "") + c.String(http.StatusNotFound, "") } func getRepoFile(c *gin.Context) { @@ -79,12 +79,12 @@ func getRepoFile(c *gin.Context) { ref := c.Query("ref") if file == "file_not_found" { - c.String(404, "") + c.String(http.StatusNotFound, "") } if ref == "v1.0.0" || ref == "9ecad50" { - c.String(200, repoFilePayload) + c.String(http.StatusOK, repoFilePayload) } - c.String(404, "") + c.String(http.StatusNotFound, "") } func createRepoHook(c *gin.Context) { @@ -99,41 +99,41 @@ func createRepoHook(c *gin.Context) { if in.Type != "gitea" || in.Conf.Type != "json" || in.Conf.URL != "http://localhost" { - c.String(500, "") + c.String(http.StatusInternalServerError, "") return } - c.String(200, "{}") + c.String(http.StatusOK, "{}") } func deleteRepoHook(c *gin.Context) { - c.String(200, "{}") + c.String(http.StatusOK, "{}") } func getUserRepos(c *gin.Context) { switch c.Request.Header.Get("Authorization") { case "token repos_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") default: page := c.Query("page") if page != "" && page != "1" { - c.String(200, "[]") + c.String(http.StatusOK, "[]") } else { - c.String(200, userRepoPayload) + c.String(http.StatusOK, userRepoPayload) } } } func getVersion(c *gin.Context) { - c.JSON(200, map[string]any{"version": "1.18.0"}) + c.JSON(http.StatusOK, map[string]any{"version": "1.18.0"}) } func getPRFiles(c *gin.Context) { page := c.Query("page") if page == "1" { - c.String(200, prFilesPayload) + c.String(http.StatusOK, prFilesPayload) } else { - c.String(200, "[]") + c.String(http.StatusOK, "[]") } } diff --git a/server/forge/gitea/gitea.go b/server/forge/gitea/gitea.go index 59907e948..eba7baabf 100644 --- a/server/forge/gitea/gitea.go +++ b/server/forge/gitea/gitea.go @@ -399,10 +399,10 @@ func (c *Gitea) Activate(ctx context.Context, u *model.User, r *model.Repo, link _, response, err := client.CreateRepoHook(r.Owner, r.Name, hook) if err != nil { if response != nil { - if response.StatusCode == 404 { + if response.StatusCode == http.StatusNotFound { return fmt.Errorf("could not find repository") } - if response.StatusCode == 200 { + if response.StatusCode == http.StatusOK { return fmt.Errorf("could not find repository, repository was probably renamed") } } diff --git a/server/forge/github/fixtures/handler.go b/server/forge/github/fixtures/handler.go index 320805a51..0f0255705 100644 --- a/server/forge/github/fixtures/handler.go +++ b/server/forge/github/fixtures/handler.go @@ -37,29 +37,29 @@ func Handler() http.Handler { func getRepo(c *gin.Context) { switch c.Param("name") { case "repo_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") default: - c.String(200, repoPayload) + c.String(http.StatusOK, repoPayload) } } func getRepoByID(c *gin.Context) { switch c.Param("id") { case "repo_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") default: - c.String(200, repoPayload) + c.String(http.StatusOK, repoPayload) } } func getMembership(c *gin.Context) { switch c.Param("org") { case "org_not_found": - c.String(404, "") + c.String(http.StatusNotFound, "") case "github": - c.String(200, membershipIsMemberPayload) + c.String(http.StatusOK, membershipIsMemberPayload) default: - c.String(200, membershipIsOwnerPayload) + c.String(http.StatusOK, membershipIsOwnerPayload) } } diff --git a/server/forge/github/github.go b/server/forge/github/github.go index b94401f2b..e272b0f34 100644 --- a/server/forge/github/github.go +++ b/server/forge/github/github.go @@ -489,7 +489,9 @@ func (c *client) Status(ctx context.Context, user *model.User, repo *model.Repo, client := c.newClientToken(ctx, user.Token) if pipeline.Event == model.EventDeploy { + // Get id from url. If not found, skip. matches := reDeploy.FindStringSubmatch(pipeline.ForgeURL) + //nolint:gomnd if len(matches) != 2 { return nil } diff --git a/server/forge/gitlab/convert.go b/server/forge/gitlab/convert.go index 7c8f4995b..9e9adc7cd 100644 --- a/server/forge/gitlab/convert.go +++ b/server/forge/gitlab/convert.go @@ -28,7 +28,8 @@ import ( ) const ( - mergeRefs = "refs/merge-requests/%d/head" // merge request merged with base + mergeRefs = "refs/merge-requests/%d/head" // merge request merged with base + VisibilityLevelInternal = 10 ) func (g *GitLab) convertGitLabRepo(_repo *gitlab.Project, projectMember *gitlab.ProjectMember) (*model.Repo, error) { @@ -258,7 +259,7 @@ func convertReleaseHook(hook *gitlab.ReleaseEvent) (*model.Repo, *model.Pipeline repo.CloneSSH = hook.Project.GitSSHURL repo.FullName = hook.Project.PathWithNamespace repo.Branch = hook.Project.DefaultBranch - repo.IsSCMPrivate = hook.Project.VisibilityLevel > 10 + repo.IsSCMPrivate = hook.Project.VisibilityLevel > VisibilityLevelInternal pipeline := &model.Pipeline{ Event: model.EventRelease, @@ -292,9 +293,13 @@ func getUserAvatar(email string) string { ) } +// extractFromPath splits a repository path string into owner and name components. +// It requires at least two path components, otherwise an error is returned. func extractFromPath(str string) (string, string, error) { + const minPathComponents = 2 + s := strings.Split(str, "/") - if len(s) < 2 { + if len(s) < minPathComponents { return "", "", fmt.Errorf("minimum match not found") } return s[0], s[1], nil diff --git a/server/forge/gitlab/gitlab.go b/server/forge/gitlab/gitlab.go index 008d037a1..79c51462b 100644 --- a/server/forge/gitlab/gitlab.go +++ b/server/forge/gitlab/gitlab.go @@ -544,7 +544,7 @@ func (g *GitLab) Deactivate(ctx context.Context, user *model.User, repo *model.R hookID := -1 listProjectHooksOptions := &gitlab.ListProjectHooksOptions{ - PerPage: 10, + PerPage: perPage, Page: 1, } for { @@ -685,7 +685,7 @@ func (g *GitLab) OrgMembership(ctx context.Context, u *model.User, owner string) groups, _, err := client.Groups.ListGroups(&gitlab.ListGroupsOptions{ ListOptions: gitlab.ListOptions{ Page: 1, - PerPage: 100, + PerPage: perPage, }, Search: gitlab.Ptr(owner), }, gitlab.WithContext(ctx)) @@ -706,7 +706,7 @@ func (g *GitLab) OrgMembership(ctx context.Context, u *model.User, owner string) opts := &gitlab.ListGroupMembersOptions{ ListOptions: gitlab.ListOptions{ Page: 1, - PerPage: 100, + PerPage: perPage, }, } diff --git a/server/forge/refresh.go b/server/forge/refresh.go index dd904c91f..931b67d6e 100644 --- a/server/forge/refresh.go +++ b/server/forge/refresh.go @@ -32,11 +32,14 @@ type Refresher interface { } func Refresh(c context.Context, forge Forge, _store store.Store, user *model.User) { + // Remaining ttl of 30 minutes (1800 seconds) until a token is refreshed. + const tokenMinTTL = 1800 + if refresher, ok := forge.(Refresher); ok { // Check to see if the user token is expired or // will expire within the next 30 minutes (1800 seconds). // If not, there is nothing we really need to do here. - if time.Now().UTC().Unix() < (user.Expiry - 1800) { + if time.Now().UTC().Unix() < (user.Expiry - tokenMinTTL) { return } diff --git a/server/model/repo.go b/server/model/repo.go index 34d50bb50..b99c66b15 100644 --- a/server/model/repo.go +++ b/server/model/repo.go @@ -65,13 +65,13 @@ func (r *Repo) ResetVisibility() { // ParseRepo parses the repository owner and name from a string. func ParseRepo(str string) (user, repo string, err error) { - parts := strings.Split(str, "/") - if len(parts) != 2 { - err = fmt.Errorf("error: Invalid or missing repository. eg octocat/hello-world") + before, after, _ := strings.Cut(str, "/") + if before == "" || after == "" { + err = fmt.Errorf("invalid or missing repository (e.g. octocat/hello-world)") return } - user = parts[0] - repo = parts[1] + user = before + repo = after return } diff --git a/server/model/user.go b/server/model/user.go index 73eca7263..152df68ac 100644 --- a/server/model/user.go +++ b/server/model/user.go @@ -25,6 +25,8 @@ var reUsername = regexp.MustCompile("^[a-zA-Z0-9-_.]+$") var errUserLoginInvalid = errors.New("invalid user login") +const maxLoginLen = 250 + // User represents a registered user. type User struct { // the id for this user. @@ -79,7 +81,7 @@ func (u *User) Validate() error { switch { case len(u.Login) == 0: return errUserLoginInvalid - case len(u.Login) > 250: + case len(u.Login) > maxLoginLen: return errUserLoginInvalid case !reUsername.MatchString(u.Login): return errUserLoginInvalid diff --git a/server/pipeline/step_status.go b/server/pipeline/step_status.go index 8d0c1d538..4fb869fcc 100644 --- a/server/pipeline/step_status.go +++ b/server/pipeline/step_status.go @@ -18,6 +18,7 @@ package pipeline import ( "time" + "go.woodpecker-ci.org/woodpecker/v2/pipeline" "go.woodpecker-ci.org/woodpecker/v2/pipeline/rpc" "go.woodpecker-ci.org/woodpecker/v2/server/model" "go.woodpecker-ci.org/woodpecker/v2/server/store" @@ -32,7 +33,7 @@ func UpdateStepStatus(store store.Store, step *model.Step, state rpc.State) erro if state.ExitCode != 0 || state.Error != "" { step.State = model.StatusFailure } - if state.ExitCode == 137 { + if state.ExitCode == pipeline.ExitCodeKilled { step.State = model.StatusKilled } } else if step.Stopped == 0 { @@ -78,6 +79,6 @@ func UpdateStepToStatusKilled(store store.Store, step model.Step) (*model.Step, if step.Started == 0 { step.Started = step.Stopped } - step.ExitCode = 137 + step.ExitCode = pipeline.ExitCodeKilled return &step, store.StepUpdate(&step) } diff --git a/server/pipeline/step_status_test.go b/server/pipeline/step_status_test.go index b0ecd85f4..9ac124d99 100644 --- a/server/pipeline/step_status_test.go +++ b/server/pipeline/step_status_test.go @@ -22,6 +22,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" + "go.woodpecker-ci.org/woodpecker/v2/pipeline" "go.woodpecker-ci.org/woodpecker/v2/pipeline/rpc" "go.woodpecker-ci.org/woodpecker/v2/server/model" "go.woodpecker-ci.org/woodpecker/v2/server/store" @@ -45,7 +46,7 @@ func TestUpdateStepStatusNotExited(t *testing.T) { Exited: false, // Dummy data Finished: int64(1), - ExitCode: 137, + ExitCode: pipeline.ExitCodeKilled, Error: "not an error", } @@ -69,7 +70,7 @@ func TestUpdateStepStatusNotExitedButStopped(t *testing.T) { Exited: false, // Dummy data Finished: int64(1), - ExitCode: 137, + ExitCode: pipeline.ExitCodeKilled, Error: "not an error", } @@ -93,7 +94,7 @@ func TestUpdateStepStatusExited(t *testing.T) { Started: int64(42), Exited: true, Finished: int64(34), - ExitCode: 137, + ExitCode: pipeline.ExitCodeKilled, Error: "an error", } @@ -102,7 +103,7 @@ func TestUpdateStepStatusExited(t *testing.T) { assert.EqualValues(t, model.StatusKilled, step.State) assert.EqualValues(t, 42, step.Started) assert.EqualValues(t, 34, step.Stopped) - assert.EqualValues(t, 137, step.ExitCode) + assert.EqualValues(t, pipeline.ExitCodeKilled, step.ExitCode) assert.EqualValues(t, "an error", step.Error) } diff --git a/server/queue/fifo.go b/server/queue/fifo.go index 29e24d103..476df8eba 100644 --- a/server/queue/fifo.go +++ b/server/queue/fifo.go @@ -52,6 +52,8 @@ type fifo struct { } // New returns a new fifo queue. +// +//nolint:gomnd func New(_ context.Context) Queue { return &fifo{ workers: map[*worker]struct{}{}, diff --git a/server/router/middleware/header/header.go b/server/router/middleware/header/header.go index 571354604..663bf451d 100644 --- a/server/router/middleware/header/header.go +++ b/server/router/middleware/header/header.go @@ -42,7 +42,7 @@ func Options(c *gin.Context) { c.Header("Access-Control-Allow-Headers", "authorization, origin, content-type, accept") c.Header("Allow", "HEAD,GET,POST,PUT,PATCH,DELETE,OPTIONS") c.Header("Content-Type", "application/json") - c.AbortWithStatus(200) + c.AbortWithStatus(http.StatusOK) } } diff --git a/server/router/middleware/session/agent.go b/server/router/middleware/session/agent.go index 3f96a57c0..cacaf0d44 100644 --- a/server/router/middleware/session/agent.go +++ b/server/router/middleware/session/agent.go @@ -16,6 +16,8 @@ package session import ( + "net/http" + "github.com/gin-gonic/gin" "go.woodpecker-ci.org/woodpecker/v2/shared/token" @@ -25,7 +27,7 @@ import ( func AuthorizeAgent(c *gin.Context) { secret, _ := c.MustGet("agent").(string) if secret == "" { - c.String(401, "invalid or empty token.") + c.String(http.StatusUnauthorized, "invalid or empty token.") return } @@ -34,10 +36,10 @@ func AuthorizeAgent(c *gin.Context) { }) switch { case err != nil: - c.String(500, "invalid or empty token. %s", err) + c.String(http.StatusInternalServerError, "invalid or empty token. %s", err) c.Abort() case parsed.Kind != token.AgentToken: - c.String(403, "invalid token. please use an agent token") + c.String(http.StatusForbidden, "invalid token. please use an agent token") c.Abort() default: c.Next() diff --git a/server/router/middleware/session/user.go b/server/router/middleware/session/user.go index a622f9c92..b0c1b7176 100644 --- a/server/router/middleware/session/user.go +++ b/server/router/middleware/session/user.go @@ -75,10 +75,10 @@ func MustAdmin() gin.HandlerFunc { user := User(c) switch { case user == nil: - c.String(401, "User not authorized") + c.String(http.StatusUnauthorized, "User not authorized") c.Abort() case !user.Admin: - c.String(403, "User not authorized") + c.String(http.StatusForbidden, "User not authorized") c.Abort() default: c.Next() @@ -92,10 +92,10 @@ func MustRepoAdmin() gin.HandlerFunc { perm := Perm(c) switch { case user == nil: - c.String(401, "User not authorized") + c.String(http.StatusUnauthorized, "User not authorized") c.Abort() case !perm.Admin: - c.String(403, "User not authorized") + c.String(http.StatusForbidden, "User not authorized") c.Abort() default: c.Next() @@ -108,7 +108,7 @@ func MustUser() gin.HandlerFunc { user := User(c) switch { case user == nil: - c.String(401, "User not authorized") + c.String(http.StatusUnauthorized, "User not authorized") c.Abort() default: c.Next() diff --git a/server/services/config/http.go b/server/services/config/http.go index dc51a4734..3912fdbeb 100644 --- a/server/services/config/http.go +++ b/server/services/config/http.go @@ -18,6 +18,7 @@ import ( "context" "crypto" "fmt" + nethttp "net/http" "go.woodpecker-ci.org/woodpecker/v2/server/forge" "go.woodpecker-ci.org/woodpecker/v2/server/forge/types" @@ -75,7 +76,7 @@ func (h *http) Fetch(ctx context.Context, forge forge.Forge, user *model.User, r return nil, fmt.Errorf("failed to fetch config via http (%d) %w", status, err) } - if status != 200 { + if status != nethttp.StatusOK { return oldConfigData, nil } diff --git a/server/services/environment/parse.go b/server/services/environment/parse.go index c1a6b8c87..0a1ccb0c9 100644 --- a/server/services/environment/parse.go +++ b/server/services/environment/parse.go @@ -31,13 +31,13 @@ func Parse(params []string) Service { var globals []*model.Environ for _, item := range params { - kvPair := strings.SplitN(item, ":", 2) - if len(kvPair) != 2 { + before, after, _ := strings.Cut(item, ":") + if after == "" { // ignore items only containing a key and no value - log.Warn().Msgf("key '%s' has no value, will be ignored", kvPair[0]) + log.Warn().Msgf("key '%s' has no value, will be ignored", before) continue } - globals = append(globals, &model.Environ{Name: kvPair[0], Value: kvPair[1]}) + globals = append(globals, &model.Environ{Name: before, Value: after}) } return &builtin{globals} } diff --git a/server/services/registry/filesystem.go b/server/services/registry/filesystem.go index 4d98818cc..6d7006274 100644 --- a/server/services/registry/filesystem.go +++ b/server/services/registry/filesystem.go @@ -113,10 +113,10 @@ func decodeAuth(authStr string) (string, string, error) { if n > decLen { return "", "", fmt.Errorf("something went wrong decoding auth config") } - arr := strings.SplitN(string(decoded), ":", 2) - if len(arr) != 2 { + before, after, _ := strings.Cut(string(decoded), ":") + if before == "" || after == "" { return "", "", fmt.Errorf("invalid auth configuration file") } - password := strings.Trim(arr[1], "\x00") - return arr[0], password, nil + password := strings.Trim(after, "\x00") + return before, password, nil } diff --git a/server/services/utils/http.go b/server/services/utils/http.go index 728234012..c11bfe27f 100644 --- a/server/services/utils/http.go +++ b/server/services/utils/http.go @@ -65,7 +65,7 @@ func Send(ctx context.Context, method, path string, privateKey crypto.PrivateKey } defer resp.Body.Close() - if resp.StatusCode != 200 { + if resp.StatusCode != http.StatusOK { body, err := io.ReadAll(resp.Body) if err != nil { return resp.StatusCode, err diff --git a/shared/httputil/httputil.go b/shared/httputil/httputil.go index 11c9a3fe9..ce44152d5 100644 --- a/shared/httputil/httputil.go +++ b/shared/httputil/httputil.go @@ -15,6 +15,7 @@ package httputil import ( + "math" "net/http" "strings" ) @@ -47,7 +48,7 @@ func SetCookie(w http.ResponseWriter, r *http.Request, name, value string) { Domain: r.URL.Host, HttpOnly: true, Secure: IsHTTPS(r), - MaxAge: 2147483647, // the cooke value (token) is responsible for expiration + MaxAge: math.MaxInt32, // the cookie value (token) is responsible for expiration } http.SetCookie(w, &cookie)