diff --git a/server/forge/gitlab/convert.go b/server/forge/gitlab/convert.go index 9a7331fee88..a929565992e 100644 --- a/server/forge/gitlab/convert.go +++ b/server/forge/gitlab/convert.go @@ -30,8 +30,13 @@ import ( ) const ( - mergeRefs = "refs/merge-requests/%d/head" // merge request merged with base - VisibilityLevelInternal = 10 + mergeRefs = "refs/merge-requests/%d/head" // merge request merged with base + + // GitLab project visibility_level values, as sent in webhook payloads. + // See https://docs.gitlab.com/api/projects/#project-visibility-level. + visibilityLevelPrivate = 0 + visibilityLevelInternal = 10 + visibilityLevelPublic = 20 stateOpened = "opened" @@ -251,12 +256,21 @@ func convertPushHook(hook *gitlab.PushEvent) (*model.Repo, *model.Pipeline, erro repo.FullName = hook.Project.PathWithNamespace repo.Branch = hook.Project.DefaultBranch + // GitLab does not send `project.visibility` (string) in push event + // payloads — only `project.visibility_level` (numeric), which the + // go-gitlab library does not expose on PushEventProject. So this switch + // is a no-op for real-world payloads, leaving Visibility/IsSCMPrivate + // at zero values. model.Repo.Update() must therefore guard against + // overwriting the value previously synced via the forge API. switch hook.Project.Visibility { case gitlab.PrivateVisibility: + repo.Visibility = model.VisibilityPrivate repo.IsSCMPrivate = true case gitlab.InternalVisibility: + repo.Visibility = model.VisibilityInternal repo.IsSCMPrivate = true case gitlab.PublicVisibility: + repo.Visibility = model.VisibilityPublic repo.IsSCMPrivate = false } @@ -304,12 +318,17 @@ func convertTagHook(hook *gitlab.TagEvent) (*model.Repo, *model.Pipeline, string repo.FullName = hook.Project.PathWithNamespace repo.Branch = hook.Project.DefaultBranch + // See note in convertPushHook: tag event payloads also omit + // `project.visibility`, so this switch typically does nothing. switch hook.Project.Visibility { case gitlab.PrivateVisibility: + repo.Visibility = model.VisibilityPrivate repo.IsSCMPrivate = true case gitlab.InternalVisibility: + repo.Visibility = model.VisibilityInternal repo.IsSCMPrivate = true case gitlab.PublicVisibility: + repo.Visibility = model.VisibilityPublic repo.IsSCMPrivate = false } @@ -353,7 +372,21 @@ 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 > VisibilityLevelInternal + + // Release events expose visibility as a numeric level (unlike push/tag + // which omit it from the payload entirely). Map it to both Visibility + // and IsSCMPrivate so model.Repo.Update() will propagate the value. + switch hook.Project.VisibilityLevel { + case visibilityLevelPrivate: + repo.Visibility = model.VisibilityPrivate + repo.IsSCMPrivate = true + case visibilityLevelInternal: + repo.Visibility = model.VisibilityInternal + repo.IsSCMPrivate = true + case visibilityLevelPublic: + repo.Visibility = model.VisibilityPublic + repo.IsSCMPrivate = false + } pipeline := &model.Pipeline{ Event: model.EventRelease, diff --git a/server/model/repo.go b/server/model/repo.go index b2eac4f3de4..d79f8452a51 100644 --- a/server/model/repo.go +++ b/server/model/repo.go @@ -138,14 +138,16 @@ func (r *Repo) Update(from *Repo) { r.CloneSSH = from.CloneSSH } r.Branch = from.Branch - if from.IsSCMPrivate != r.IsSCMPrivate { - if from.IsSCMPrivate { - r.Visibility = VisibilityPrivate - } else { - r.Visibility = VisibilityPublic - } + // Only propagate visibility when the source supplies it. Some webhook + // payloads (notably GitLab push/tag/merge events) do not include project + // visibility, leaving from.Visibility empty and from.IsSCMPrivate at the + // zero value. Updating the stored fields from those payloads would + // overwrite the authoritative value previously synced from the forge API + // during activation or repair, breaking netrc-protected clones. + if from.Visibility != "" { + r.Visibility = from.Visibility + r.IsSCMPrivate = from.IsSCMPrivate } - r.IsSCMPrivate = from.IsSCMPrivate } // RepoPatch represents a repository patch object. diff --git a/server/model/repo_test.go b/server/model/repo_test.go new file mode 100644 index 00000000000..99d1bd93c82 --- /dev/null +++ b/server/model/repo_test.go @@ -0,0 +1,76 @@ +// Copyright 2026 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 model + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestRepoUpdate_Visibility(t *testing.T) { + tests := []struct { + name string + stored Repo + from Repo + wantVisibility RepoVisibility + wantPrivate bool + }{ + { + name: "empty source visibility preserves stored value", + stored: Repo{Visibility: VisibilityPrivate, IsSCMPrivate: true}, + from: Repo{Visibility: "", IsSCMPrivate: false}, + wantVisibility: VisibilityPrivate, + wantPrivate: true, + }, + { + name: "empty source visibility preserves stored public value", + stored: Repo{Visibility: VisibilityPublic, IsSCMPrivate: false}, + from: Repo{Visibility: "", IsSCMPrivate: false}, + wantVisibility: VisibilityPublic, + wantPrivate: false, + }, + { + name: "source can change public to private", + stored: Repo{Visibility: VisibilityPublic, IsSCMPrivate: false}, + from: Repo{Visibility: VisibilityPrivate, IsSCMPrivate: true}, + wantVisibility: VisibilityPrivate, + wantPrivate: true, + }, + { + name: "source can change private to public", + stored: Repo{Visibility: VisibilityPrivate, IsSCMPrivate: true}, + from: Repo{Visibility: VisibilityPublic, IsSCMPrivate: false}, + wantVisibility: VisibilityPublic, + wantPrivate: false, + }, + { + name: "internal visibility is preserved (not collapsed to private)", + stored: Repo{Visibility: VisibilityPublic, IsSCMPrivate: false}, + from: Repo{Visibility: VisibilityInternal, IsSCMPrivate: true}, + wantVisibility: VisibilityInternal, + wantPrivate: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := tt.stored + r.Update(&tt.from) + assert.Equal(t, tt.wantVisibility, r.Visibility) + assert.Equal(t, tt.wantPrivate, r.IsSCMPrivate) + }) + } +}