Fix bug when push to prs with allow edits from maintainers and lfs usage #36376
+109
−28
Conversation
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
GiteaBot
added
the
lgtm/need 2
github-actions
bot
added
modifies/go
silverwind
approved these changes
Jan 16, 2026
GiteaBot
added
lgtm/need 1
wxiaoguang
reviewed
Jan 16, 2026
services/lfs/server.go
Outdated
Comment on lines
56
to
110
| func normalizeLFSRefName(raw string) string { | ||
| ref := strings.TrimSpace(raw) | ||
| if ref == "" { | ||
| return "" | ||
| } | ||
| prefixes := []string{"refs/heads/", "refs/remotes/", "refs/"} | ||
| for _, prefix := range prefixes { | ||
| if trimmed, ok := strings.CutPrefix(ref, prefix); ok { | ||
| ref = trimmed | ||
| break | ||
| } | ||
| } | ||
| return ref | ||
| } | ||
|
|
||
| func refNameFromBatchRequest(br *lfs_module.BatchRequest) string { | ||
| if br == nil || br.Ref == nil { | ||
| return "" | ||
| } | ||
| return normalizeLFSRefName(br.Ref.Name) | ||
| } | ||
|
|
||
| func setLFSRefInContext(ctx *context.Context, ref string) { | ||
| ref = normalizeLFSRefName(ref) | ||
| if ref == "" { | ||
| return | ||
| } | ||
| if ctx.Data == nil { | ||
| ctx.Data = make(map[string]any) | ||
| } | ||
| ctx.Data[lfsRefContextKey] = ref | ||
| } | ||
|
|
||
| func getLFSRefFromContext(ctx *context.Context) string { | ||
| if ctx.Data == nil { | ||
| return "" | ||
| } | ||
| if ref, ok := ctx.Data[lfsRefContextKey].(string); ok { | ||
| return ref | ||
| } | ||
| return "" | ||
| } | ||
|
|
||
| func setLFSRefFromQuery(ctx *context.Context) { | ||
| ref := ctx.Req.URL.Query().Get(lfsRefQueryKey) | ||
| setLFSRefInContext(ctx, ref) | ||
| } | ||
|
|
||
| func appendRefQuery(baseURL, ref string) string { | ||
| if ref == "" { | ||
| return baseURL | ||
| } | ||
| return baseURL + "?" + lfsRefQueryKey + "=" + url.QueryEscape(ref) | ||
| } | ||
|
|
Contributor
There was a problem hiding this comment.
I really don't understand why these newly added function can be right or necessary.
Contributor
|
I really don't understand what you are doing. You always have the |
wxiaoguang
reviewed
Jan 28, 2026
| if ref == "" { | ||
| return baseURL | ||
| } | ||
| return baseURL + "?" + lfsRefQueryKey + "=" + url.QueryEscape(ref) |
Contributor
There was a problem hiding this comment.
What if baseURL contains ?
wxiaoguang
reviewed
Jan 28, 2026
|
|
||
| branchName := refName.BranchName() | ||
| if branchName == "" { | ||
| if strings.HasPrefix(ref, "refs/") { |
wxiaoguang
reviewed
Jan 28, 2026
| } | ||
| } | ||
|
|
||
| func canMaintainerWriteLFS(ctx *context.Context, perm access_model.Permission, user *user_model.User, ref string) bool { |
Contributor
There was a problem hiding this comment.
It needs to clearly document and test this behavior:
refcan be anything passed by the user, it is not fully trusted- if a PR branch is set "allow maintainer edit", a maintainer can pass that branch name and fully write all the LFS objects in the head repo, there is no check / no relation between the branch and LFS objects.
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.
Fix #35226