This repository was archived by the owner on Jan 24, 2026. It is now read-only.
chore: sync with upstream goharbor/harbor#109
Open
github-actions[bot] wants to merge 3 commits intonextfrom
Open
chore: sync with upstream goharbor/harbor#109github-actions[bot] wants to merge 3 commits intonextfrom
github-actions[bot] wants to merge 3 commits intonextfrom
Conversation
Remove the unused function MostMatchSorter, it should not be implemented in golang, should be implement in the db query. Remove the unused function onBoardCommonUserGroup() fixes goharbor#22573 Signed-off-by: stonezdj <stonezdj@gmail.com>
|
Important Review skippedBot user detected. To trigger a single review, invoke the You can disable this status message by setting the Comment |
This commit fixes the style issues introduced in f529003 according to the output from Gofumpt and Prettier. Details: https://github.com/container-registry/harbor-next/pull/109
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 9 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/pkg/user/manager.go">
<violation number="1" location="src/pkg/user/manager.go:32">
P3: Comment incorrectly describes this as a "project manager" when it's actually a user manager. This is in the `user` package and manages user operations.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| // Mgr is the global project manager | ||
| Mgr = New() | ||
| ) | ||
| // Mgr is the global project manager |
There was a problem hiding this comment.
P3: Comment incorrectly describes this as a "project manager" when it's actually a user manager. This is in the user package and manages user operations.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/pkg/user/manager.go, line 32:
<comment>Comment incorrectly describes this as a "project manager" when it's actually a user manager. This is in the `user` package and manages user operations.</comment>
<file context>
@@ -29,10 +29,8 @@ import (
- // Mgr is the global project manager
- Mgr = New()
-)
+// Mgr is the global project manager
+var Mgr = New()
</file context>
Suggested change
| // Mgr is the global project manager | |
| // Mgr is the global user manager |
There was a problem hiding this comment.
5 issues found across 16 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/server/v2.0/handler/user.go">
<violation number="1" location="src/server/v2.0/handler/user.go:281">
P1: Pagination is broken: `params.Page` is used for counting and generating pagination links but ignored when fetching results. `SearchByName` only accepts `limitSize`, not a page offset, so requests for page > 1 will incorrectly return page 1 data.</violation>
</file>
<file name="src/pkg/user/dao/dao.go">
<violation number="1" location="src/pkg/user/dao/dao.go:136">
P2: LIKE wildcard characters (`%` and `_`) in the `name` parameter are not escaped. A user could search for `%` or `_` to match all usernames, potentially enumerating users in the system. Consider escaping these characters before constructing the LIKE pattern:
```go
import "strings"
// ...
escapedName := strings.ReplaceAll(strings.ReplaceAll(name, "%", "\\%"), "_", "\\_")
likePattern := "%" + escapedName + "%"
```</violation>
</file>
<file name="src/pkg/usergroup/dao/dao.go">
<violation number="1" location="src/pkg/usergroup/dao/dao.go:176">
P2: Consider escaping SQL LIKE wildcards (`%`, `_`) in the search term to prevent unexpected matching behavior. Users who include these characters in their search may get unexpected results since they are interpreted as wildcards rather than literal characters.</violation>
</file>
<file name="src/pkg/user/manager.go">
<violation number="1" location="src/pkg/user/manager.go:250">
P2: The `SearchByName` method doesn't exclude the default admin user (user_id=1), unlike `List` and `Count` methods which both call `excludeDefaultAdmin()` by default. This inconsistency could expose the default admin in user search results when other listing operations would hide it. Consider whether the default admin should be excluded from search results for consistency.</violation>
</file>
<file name="src/server/v2.0/handler/usergroup.go">
<violation number="1" location="src/server/v2.0/handler/usergroup.go:205">
P1: Pagination is broken: `SearchByName` ignores `params.Page`. The method only accepts `limitSize` with no offset/page parameter, so requesting page 2 returns the same results as page 1. Either update `SearchByName` to accept pagination parameters, or document this endpoint as unpaginated and remove pagination links.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| return operation.NewSearchUsersOK().WithXTotalCount(0).WithPayload([]*models.UserSearchRespItem{}) | ||
| } | ||
| l, err := u.ctl.List(ctx, query) | ||
| l, err := u.ctl.SearchByName(ctx, params.Username, int(*params.PageSize)) |
There was a problem hiding this comment.
P1: Pagination is broken: params.Page is used for counting and generating pagination links but ignored when fetching results. SearchByName only accepts limitSize, not a page offset, so requests for page > 1 will incorrectly return page 1 data.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/server/v2.0/handler/user.go, line 281:
<comment>Pagination is broken: `params.Page` is used for counting and generating pagination links but ignored when fetching results. `SearchByName` only accepts `limitSize`, not a page offset, so requests for page > 1 will incorrectly return page 1 data.</comment>
<file context>
@@ -279,7 +278,7 @@ func (u *usersAPI) SearchUsers(ctx context.Context, params operation.SearchUsers
return operation.NewSearchUsersOK().WithXTotalCount(0).WithPayload([]*models.UserSearchRespItem{})
}
- l, err := u.ctl.List(ctx, query)
+ l, err := u.ctl.SearchByName(ctx, params.Username, int(*params.PageSize))
if err != nil {
return u.SendError(ctx, err)
</file context>
| return operation.NewSearchUserGroupsOK().WithXTotalCount(0).WithPayload([]*models.UserGroupSearchItem{}) | ||
| } | ||
| ug, err := u.ctl.List(ctx, query) | ||
| ug, err := u.ctl.SearchByName(ctx, params.Groupname, int(*params.PageSize)) |
There was a problem hiding this comment.
P1: Pagination is broken: SearchByName ignores params.Page. The method only accepts limitSize with no offset/page parameter, so requesting page 2 returns the same results as page 1. Either update SearchByName to accept pagination parameters, or document this endpoint as unpaginated and remove pagination links.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/server/v2.0/handler/usergroup.go, line 205:
<comment>Pagination is broken: `SearchByName` ignores `params.Page`. The method only accepts `limitSize` with no offset/page parameter, so requesting page 2 returns the same results as page 1. Either update `SearchByName` to accept pagination parameters, or document this endpoint as unpaginated and remove pagination links.</comment>
<file context>
@@ -204,14 +202,11 @@ func (u *userGroupAPI) SearchUserGroups(ctx context.Context, params operation.Se
return operation.NewSearchUserGroupsOK().WithXTotalCount(0).WithPayload([]*models.UserGroupSearchItem{})
}
- ug, err := u.ctl.List(ctx, query)
+ ug, err := u.ctl.SearchByName(ctx, params.Groupname, int(*params.PageSize))
if err != nil {
return u.SendError(ctx, err)
</file context>
| var users []*User | ||
| // use raw sql to return the most matched user first, then by alphabetic order | ||
| sql := "select * from harbor_user where username like ? and deleted = false order by length(username), username asc limit ?" | ||
| likePattern := "%" + name + "%" |
There was a problem hiding this comment.
P2: LIKE wildcard characters (% and _) in the name parameter are not escaped. A user could search for % or _ to match all usernames, potentially enumerating users in the system. Consider escaping these characters before constructing the LIKE pattern:
import "strings"
// ...
escapedName := strings.ReplaceAll(strings.ReplaceAll(name, "%", "\\%"), "_", "\\_")
likePattern := "%" + escapedName + "%"Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/pkg/user/dao/dao.go, line 136:
<comment>LIKE wildcard characters (`%` and `_`) in the `name` parameter are not escaped. A user could search for `%` or `_` to match all usernames, potentially enumerating users in the system. Consider escaping these characters before constructing the LIKE pattern:
```go
import "strings"
// ...
escapedName := strings.ReplaceAll(strings.ReplaceAll(name, "%", "\\%"), "_", "\\_")
likePattern := "%" + escapedName + "%"
```</comment>
<file context>
@@ -122,3 +124,24 @@ func (d *dao) List(ctx context.Context, query *q.Query) ([]*commonmodels.User, e
+ var users []*User
+ // use raw sql to return the most matched user first, then by alphabetic order
+ sql := "select * from harbor_user where username like ? and deleted = false order by length(username), username asc limit ?"
+ likePattern := "%" + name + "%"
+ _, err = o.Raw(sql, likePattern, limitSize).QueryRows(&users)
+ if err != nil {
</file context>
| var usergroups []*model.UserGroup | ||
| // use raw sql to return the most matched user first, then by alphabetic order | ||
| sql := "select id, group_name, group_type, ldap_group_dn, creation_time, update_time from user_group where group_name like ? order by length(group_name), group_name asc limit ?" | ||
| likePattern := "%" + name + "%" |
There was a problem hiding this comment.
P2: Consider escaping SQL LIKE wildcards (%, _) in the search term to prevent unexpected matching behavior. Users who include these characters in their search may get unexpected results since they are interpreted as wildcards rather than literal characters.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/pkg/usergroup/dao/dao.go, line 176:
<comment>Consider escaping SQL LIKE wildcards (`%`, `_`) in the search term to prevent unexpected matching behavior. Users who include these characters in their search may get unexpected results since they are interpreted as wildcards rather than literal characters.</comment>
<file context>
@@ -185,3 +164,19 @@ func (d *dao) Count(ctx context.Context, query *q.Query) (int64, error) {
+ var usergroups []*model.UserGroup
+ // use raw sql to return the most matched user first, then by alphabetic order
+ sql := "select id, group_name, group_type, ldap_group_dn, creation_time, update_time from user_group where group_name like ? order by length(group_name), group_name asc limit ?"
+ likePattern := "%" + name + "%"
+ _, err = o.Raw(sql, likePattern, limitSize).QueryRows(&usergroups)
+ if err != nil {
</file context>
| u.PasswordVersion = utils.SHA256 | ||
| } | ||
|
|
||
| func (m *manager) SearchByName(ctx context.Context, name string, limitSize int) (commonmodels.Users, error) { |
There was a problem hiding this comment.
P2: The SearchByName method doesn't exclude the default admin user (user_id=1), unlike List and Count methods which both call excludeDefaultAdmin() by default. This inconsistency could expose the default admin in user search results when other listing operations would hide it. Consider whether the default admin should be excluded from search results for consistency.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/pkg/user/manager.go, line 250:
<comment>The `SearchByName` method doesn't exclude the default admin user (user_id=1), unlike `List` and `Count` methods which both call `excludeDefaultAdmin()` by default. This inconsistency could expose the default admin in user search results when other listing operations would hide it. Consider whether the default admin should be excluded from search results for consistency.</comment>
<file context>
@@ -244,3 +246,7 @@ func injectPasswd(u *commonmodels.User, password string) {
u.PasswordVersion = utils.SHA256
}
+
+func (m *manager) SearchByName(ctx context.Context, name string, limitSize int) (commonmodels.Users, error) {
+ return m.dao.SearchByName(ctx, name, limitSize)
+}
</file context>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
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.
Automated PR to sync 1 new commit(s) from upstream goharbor/harbor main branch.
Merge strategy: Our changes in
nextare preserved on conflicts (upstream changes are additive only).Note: The
.githubfolder is preserved and not synced from upstream.Summary by cubic
Sync with upstream Harbor to fix user and user group search. Moves fuzzy matching and sorting into SQL and adds SearchByName APIs; removes the unused Golang sorter and an unused onboarding function.
Bug Fixes
Refactors
Written for commit 93e84e4. Summary will update on new commits.