checkUserPushPermission is checking the wrong user's permission to push
- Dominant language
- TypeScript
- Stars
- 249
- Forks
- 176
- Avg merge
- 3d 8h
- Merged PRs (30d)
- 20
Description
**Describe the bug**
`checkUserPushPermission` is intended to check whether the user performing the push is allowed to do so. However, it is currently checking the permission of the committer of the last commit in the push in its current location, rather than the user pushing it into git proxy.
Commits in git are tagged with two users' details: the _author_ (who created the commit and originally pushed it to a repo) and the _committer_ (initially the same as the author, ~but changes whenever the commit is pushed to a different repository or branch~ but changes whenever a commit is created or modified (e.g., via `git commit`, `git commit --amend`, `git rebase`, `git merge`) - `git push` does NOT change the _committer_. When we process a push in git proxy, we do so using the original committer of the last commit rather than the user performing the push - which is not the intent of the check.
`parsePush` is extracting the `committer` and `committerEmail` from the last commit in the push and setting them as the `user` and `userEmail` in the action object:
https://github.com/finos/git-proxy/blob/6fb63d0597212e9e0056330d3a90df9997f2d505/src/proxy/processors/push-action/parsePush.ts#L93-L96
This is not the correct user information and needs to be corrected in `parsePush`, without using `action.commitData`.
**To Reproduce**
1. Setup a project in Git proxy where you are allowed to contribute but no other user is
2. Have a different user push a commit to a branch of the respository.
3. Clone that branch yourself.
4. While that different user's commit is the last one on that branch, push the branch to git proxy.
5. `checkUserPushPermission` will reject the push as it will use the other user's information as the committer,
**Expected behavior**
`parsePush` should use the details of the user trying to push through the proxy as the `user` and `userEmail` values of the action (rather than the user that original pushed the last commit), ensuring that `checkUserPushPermission` checks the correct user's details.
Contributor guide
Research direction
Read src/proxy/processors/push-action/parsePush.ts around lines 93-96 and trace how the action's user and userEmail are populated. Confirm the values represent the user performing the proxy push rather than the last commit's committer, without using action.commitData. Reproduce the described push scenario and verify checkUserPushPermission evaluates the pushing user's details.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- git, typescript
- Domain
- backend, security
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100