Correct forge_id and org handling on user/repo#6820
Merged
Conversation
…rmissions
ForgeFromUser dereferences user.ForgeID, but the nil-user guard ran only
after that call. GET /orgs/{org_id}/permissions has no MustUser middleware,
so an unauthenticated request crashed the handler with a nil pointer
dereference instead of returning empty permissions.
Org names and user logins are only unique per forge. MustOrgMember and GetOrgPermissions compared names without checking the forge, so with multiple forges configured a user could gain member or even admin access (including org secrets and registries) to a foreign forge's org in two ways: a login matching a foreign user-org's name, or a membership in a same-named org on the user's own forge being credited to the foreign org. Deny access when the org belongs to a different forge than the user; only instance admins keep cross-forge access.
…mote id The cache key was ForgeRemoteID-org, but forge remote ids are only unique per forge. With multiple forges, two different users sharing a numeric remote id and an org name would hit the same cache entry, so one user's membership (including admin) leaked to a user of another forge for up to the cache TTL. Key by the globally unique user ID instead.
SetPerm resolved the forge from the repo but synced stale permissions with whatever user is in the session. With multiple forges a user of forge A visiting a repo of forge B caused their forge-A OAuth token to be sent to forge B's API, and the fetched repo was mislabeled with the user's forge id. Skip the sync when user and repo belong to different forges; such users keep the zero permission set.
PostUser stored whatever forge id the request body carried, including the zero value when the field was omitted. Users and their orgs then ended up with forge_id 0, which no forge resolves, breaking every forge lookup for them; migration 025 exists solely to clean up such rows. Clamp to the default forge like PatchUser already does.
MoveRepo updated owner and full name but kept the old OrgID, so org-level secrets and registries of the previous owner kept applying to the moved repo while the new owner's never did. Resolve (or create) the org of the new owner on the repo's forge and relink, reusing the org resolution already done during repo activation.
Contributor
|
Surge PR preview deployment was removed |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6820 +/- ##
==========================================
+ Coverage 47.30% 47.74% +0.43%
==========================================
Files 440 440
Lines 29726 29757 +31
==========================================
+ Hits 14062 14207 +145
+ Misses 14496 14340 -156
- Partials 1168 1210 +42 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
lafriks
approved these changes
Jul 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes a set of multi-forge bugs where Woodpecker treats org names, user
logins and forge-local IDs as globally unique. With more than one forge
configured these collide, leaking permissions across forges and creating
unresolvable records.
Fixes
Anonymous permission crash —
GetOrgPermissionsresolved the forgefrom the user before the nil-user guard, so an unauthenticated call to
/orgs/{id}/permissions(noMustUseron the route) paniced.Cross-forge org access —
MustOrgMemberandGetOrgPermissionscompared org names and user logins without checking the forge. A user
could gain member or admin access (incl. org secrets and registries) to
a same-named org on a foreign forge. Now scoped to the org's forge;
instance admins keep cross-forge access.
Membership cache leak — the cache key used
ForgeRemoteID, which isonly unique per forge, so two users sharing a remote id leaked
membership across forges for the TTL. Keyed by the global user id.
Foreign-forge permission sync —
SetPermresolved the forge from therepo but synced with the session user, sending that user's OAuth token to
a different forge's API and mislabeling the fetched repo. Skipped when
user and repo differ in forge.
Zero forge id on user creation —
PostUserstoredforge_id: 0whenomitted, producing users and orgs no forge resolves. Clamped to the
default forge, matching
PatchUser.Stale org link on repo move —
MoveRepochanged the owner but keptOrgID, so the old owner's org secrets/registries kept applying. Relinksto the new owner's org (reusing the activation-time resolution).
in peer work with claude fable5