mirror of
https://codeberg.org/forgejo/forgejo.git
synced 2026-07-25 02:48:05 +00:00
a5a2cae7ee
**Backport:** https://codeberg.org/forgejo/forgejo/pulls/13143 The details of the refactor are in [a separate branch](https://codeberg.org/limiting-factor/forgejo/commits/branch/wip-api-context-accessors-save) which details how it was done. This pull request contains a squash of those commits rebased on top of the current Forgejo branch, which involved conflict resolution. Given the magnitude of the change, there is a high chance that a large number of future pull requests merged in Forgejo will create conflicts. They will be dealt with in the same way as chore: refactor orgAssignment to two separate middlewares (#13155). ## Review The only risk I can think of is a getter/setter implementation being incorrectly implemented. This is probably where a detailed review is most needed, in the `services/context/api.go` file. To not increase the size of the pull request, the `GetX()` accessors were preserved although they are redundant with `X()`. They will be removed in a followup pull request. ## Backporting strategy. Cherry-picking the commit to v15 has a significant amount of conflicts in 8 files out of 111. ``` unmerged routers/api/v1/activitypub/repository.go unmerged routers/api/v1/admin/user.go unmerged routers/api/v1/api.go unmerged routers/api/v1/org/team.go unmerged routers/api/v1/repo/action.go unmerged routers/api/v1/repo/pull.go unmerged routers/api/v1/user/follower.go unmerged services/context/api.go ``` Instead of resolving them, I think it is easier and safer to repeat the [refactor steps](https://codeberg.org/limiting-factor/forgejo/commits/branch/wip-api-context-accessors-save) in v15. I also think it is worth the effort since it will avoid trivial backport conflicts on over 100 files. ## Context This is a followup pull request discussed originally at https://codeberg.org/forgejo/forgejo/pulls/12512 Co-authored-by: limiting-factor <limiting-factor@posteo.com> Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/13213 Reviewed-by: limiting-factor <limiting-factor@noreply.codeberg.org> Reviewed-by: Mathieu Fenniak <mfenniak@noreply.codeberg.org>
110 lines
3.1 KiB
YAML
110 lines
3.1 KiB
YAML
rules:
|
|
- id: forgejo-api-use-resource-SearchRepoOptions
|
|
patterns:
|
|
- pattern: |
|
|
repo_model.SearchRepoOptions{...}
|
|
- pattern-not: |
|
|
repo_model.SearchRepoOptions{
|
|
...,
|
|
AuthorizationReducer: ctx.Reducer(),
|
|
...
|
|
}
|
|
languages:
|
|
- go
|
|
message: >
|
|
SearchRepoOptions does not take into account fine-grained access token limitations. Include the
|
|
AuthorizationReducer field.
|
|
severity: ERROR
|
|
paths:
|
|
include:
|
|
- "/routers/api/**/*.go"
|
|
|
|
- id: forgejo-api-use-resource-SearchRepoOptions
|
|
patterns:
|
|
- pattern: |
|
|
organization.SearchTeamRepoOptions{...}
|
|
- pattern-not: |
|
|
organization.SearchTeamRepoOptions{
|
|
...,
|
|
AuthorizationReducer: ctx.Reducer(),
|
|
...
|
|
}
|
|
languages:
|
|
- go
|
|
message: >
|
|
SearchTeamRepoOptions does not take into account fine-grained access token limitations. Include the
|
|
AuthorizationReducer field.
|
|
severity: ERROR
|
|
paths:
|
|
include:
|
|
- "/routers/api/**/*.go"
|
|
|
|
- id: forgejo-api-use-resource-GetUserRepoPermission
|
|
patterns:
|
|
- pattern: |
|
|
$X.GetUserRepoPermission($CTX, $REPO, $DOER)
|
|
- metavariable-type:
|
|
metavariable: $CTX
|
|
types:
|
|
- "*context.APIContext"
|
|
languages:
|
|
- go
|
|
message: >
|
|
GetUserRepoPermission does not take into account fine-grained access token limitations. Use
|
|
GetUserRepoPermissionWithReducer.
|
|
fix: |
|
|
$X.GetUserRepoPermissionWithReducer($CTX, $REPO, $DOER, $CTX.Reducer)
|
|
severity: ERROR
|
|
|
|
- id: forgejo-api-suspicious-GetUserRepoPermission
|
|
patterns:
|
|
- pattern: $X.GetUserRepoPermission($CTX, $REPO, $DOER)
|
|
- pattern-not: # don't match if identical to forgejo-api-use-resource-GetUserRepoPermission
|
|
patterns:
|
|
- pattern: |
|
|
$X.GetUserRepoPermission($CTX, $REPO, $DOER)
|
|
- metavariable-type:
|
|
metavariable: $CTX
|
|
types:
|
|
- "*context.APIContext"
|
|
languages:
|
|
- go
|
|
message: >
|
|
API code is accessing GetUserRepoPermission which does not take into account fine-grained access token
|
|
limitations. Should this use GetUserRepoPermissionWithReducer?
|
|
severity: ERROR
|
|
paths:
|
|
include:
|
|
- "/routers/api/**/*.go"
|
|
|
|
- id: forgejo-api-direct-IsAdmin-check
|
|
patterns:
|
|
- pattern: |
|
|
ctx.Doer.IsAdmin
|
|
languages:
|
|
- go
|
|
message: |
|
|
ctx.Doer.IsAdmin does not take into account limited API access tokens. Use ctx.IsUserSiteAdmin() instead.
|
|
fix: |
|
|
ctx.IsUserSiteAdmin()
|
|
severity: ERROR
|
|
paths:
|
|
include:
|
|
- "/routers/api/**/*.go"
|
|
|
|
- id: forgejo-api-direct-repo-Admin-check
|
|
patterns:
|
|
- pattern: |
|
|
ctx.Repo.IsAdmin()
|
|
- pattern: |
|
|
ctx.Repo.IsOwner()
|
|
languages:
|
|
- go
|
|
message: |
|
|
ctx.Repo.IsAdmin/IsOwner() does not take into account limited API access tokens. Use ctx.IsUserRepoAdmin() instead.
|
|
fix: |
|
|
ctx.IsUserRepoAdmin()
|
|
severity: ERROR
|
|
paths:
|
|
include:
|
|
- "/routers/api/**/*.go"
|