fix(gitlab): preserve private flag when webhook payload omits project visibility (#6544)

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

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,

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.

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)
+		})
+	}
+}