Skip to content

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

Merged
6543 merged 2 commits into
woodpecker-ci:mainfrom
EdwardSalter:fix/gitlab-webhook-visibility-detection
May 5, 2026
Merged

fix(gitlab): preserve private flag when webhook payload omits project visibility#6544
6543 merged 2 commits into
woodpecker-ci:mainfrom
EdwardSalter:fix/gitlab-webhook-visibility-detection

Conversation

@EdwardSalter
Copy link
Copy Markdown
Contributor

Symptom

On every push (or tag/MR) webhook from GitLab to a Woodpecker server hosting a private GitLab repo, the clone step starts failing with:

+ git fetch --no-tags --depth=1 --filter=tree:0 origin +<sha>:
fatal: could not read Username for 'https://gitlab.com': No such device or address
exit status 128

Clicking Repair Repository in the UI fixes it temporarily — until the next push, when the cycle starts again. WOODPECKER_AUTHENTICATE_PUBLIC_REPOS=true is an effective workaround.

Root cause

GitLab's push, tag, and merge-request webhook payloads include project.visibility_level (numeric: 0 / 10 / 20) but not project.visibility (string). The fixtures in this repo confirm this — server/forge/gitlab/fixtures/HookPush.json, HookTag.json, and HookPullRequestOpened.json all carry visibility_level and no visibility.

The go-gitlab library (gitlab.com/gitlab-org/api/client-go/v2) maps PushEventProject.Visibility to JSON key visibility only and does not expose visibility_level for these event types. So hook.Project.Visibility is "" for real GitLab payloads, the switch in convertPushHook / convertTagHook matches no case, and the returned Repo.IsSCMPrivate is left at its Go zero value false.

(*Repo).Update then unconditionally copied that false over the stored value previously set by convertGitLabRepo during repo activation or repair — flipping the stored repo to private=0, visibility=public. Downstream, server/pipeline/items.go skips attaching the netrc to the clone step when repo.IsSCMPrivate is false (and AuthenticatePublicRepos is unset), so the next clone runs without credentials and fails with the error above.

The Repair button hits the GitLab API (where visibility is populated), routes through convertGitLabRepo, and re-syncs the correct value — until the next webhook clobbers it again.

I traced this end-to-end on a 3.14.0 deployment by:

  1. Snapshotting the SQLite store and confirming the affected repo had private=0, visibility=public while the GitLab API returned visibility=private for the same project.
  2. Verifying the stored access token clones the repo fine via direct git clone https://oauth2:$TOKEN@gitlab.com/... — it is not a token / scope problem.
  3. Tracing parsePipeline → compiler.WithNetrc(..., repo.IsSCMPrivate || AuthenticatePublicRepos) and identifying the gate.
  4. Confirming WOODPECKER_AUTHENTICATE_PUBLIC_REPOS=true made clones succeed without any other change.
  5. Confirming the patch keeps the stored row at private=1, visibility=private after a fresh push event.

Fix

  • model.Repo.Update: only propagate Visibility / IsSCMPrivate when the source actually supplies visibility info. An empty from.Visibility now signals "no information" and leaves the stored fields untouched. The API path (convertGitLabRepo) always populates Visibility, so repo activation/repair is unchanged. Side benefit: "internal" is now preserved instead of being collapsed to "private" by the old IsSCMPrivate-based recomputation.

  • gitlab.convertReleaseHook: the previous expression

    repo.IsSCMPrivate = hook.Project.VisibilityLevel > VisibilityLevelInternal

    was inverted: with VisibilityLevelInternal == 10, only Public (20) satisfied > 10, so it marked public projects as private and private/internal projects as public. Replaced with an explicit switch over the three numeric levels that sets both Visibility and IsSCMPrivate, so the new Update gate propagates the value.

  • gitlab.convertPushHook / convertTagHook: also set Visibility alongside IsSCMPrivate, mirroring convertGitLabRepo. Today this is effectively dead code (because the Visibility string is empty in real payloads), but it is symmetric and forward-compatible if GitLab ever populates the field.

Tests

server/model/repo_test.go (new) covers:

  • empty from.Visibility preserves the stored private value (the regression);
  • empty from.Visibility preserves the stored public value;
  • API path can change public→private and private→public;
  • internal is preserved instead of being collapsed to private.

Existing server/forge/gitlab and server/model tests continue to pass.

Verification

Built from this branch on top of a 3.14.0 base and deployed against a private GitLab project that previously failed on every push. After the patch:

  • DB row stays at private=1, visibility=private across multiple webhook-triggered pipelines.
  • Clone step passes without WOODPECKER_AUTHENTICATE_PUBLIC_REPOS.
  • Repair button no longer needs to be clicked between pushes.

EdwardSalter and others added 2 commits May 5, 2026 17:26
GitLab webhook payloads for push, tag, and merge-request events do not
include `project.visibility` (string); they only carry
`project.visibility_level` (numeric), which the go-gitlab library does
not expose on the corresponding event types. As a result,
`hook.Project.Visibility` is always empty for these events, the switch
in convertPushHook/convertTagHook never matches, and IsSCMPrivate is
left at its zero value (false).

`(*Repo).Update` then unconditionally copied that false value into the
stored repo, overwriting the correct value previously synced via the
forge API during repo activation or repair. Once stored, downstream
code (server/pipeline/items.go) skipped attaching the netrc to the
clone step on the assumption the repo was public. For genuinely private
GitLab repos this manifested as `fatal: could not read Username for
'https://gitlab.com'` on every clone after the next push, requiring a
manual repair to re-sync the API value, which then survived only until
the next push webhook clobbered it again.

Fix:

- model.Repo.Update: only propagate Visibility/IsSCMPrivate when the
  source actually supplies visibility info. Empty `from.Visibility` now
  signals "no information" and leaves the stored fields untouched. The
  API path (convertGitLabRepo) always populates Visibility, so repo
  activation/repair behaviour is unchanged.

- gitlab convertReleaseHook: the previous logic
  (`VisibilityLevel > VisibilityLevelInternal`) was inverted — it
  marked private/internal repos as public and public repos as private.
  Replace with a switch over the three numeric levels and set both
  Visibility and IsSCMPrivate so the new gate in Update propagates the
  value.

- gitlab convertPushHook/convertTagHook: also set Visibility alongside
  IsSCMPrivate so any future GitLab payload (or self-hosted variant)
  that does include `project.visibility` is propagated correctly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Lock in the regression fix from the previous commit: webhook payloads
that omit project visibility (empty from.Visibility) must not overwrite
the stored value, while API payloads that supply visibility — including
the "internal" GitLab variant — must propagate.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@6543 6543 added bug Something isn't working forge/gitlab gitlab forge related labels May 5, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 5, 2026

Codecov Report

❌ Patch coverage is 67.56757% with 12 lines in your changes missing coverage. Please review.
✅ Project coverage is 41.60%. Comparing base (db63579) to head (3edb2fb).

Files with missing lines Patch % Lines
server/forge/gitlab/convert.go 57.14% 12 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6544      +/-   ##
==========================================
+ Coverage   41.45%   41.60%   +0.14%     
==========================================
  Files         431      431              
  Lines       28696    28725      +29     
==========================================
+ Hits        11896    11950      +54     
+ Misses      15729    15703      -26     
- Partials     1071     1072       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@6543 6543 merged commit 97eae44 into woodpecker-ci:main May 5, 2026
9 checks passed
@woodpecker-bot woodpecker-bot mentioned this pull request May 5, 2026
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working forge/gitlab gitlab forge related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants