Dead permission checking code? #8447

Closed
opened 2025-11-02 08:06:21 -06:00 by GiteaMirror · 2 comments
Owner

Originally created by @kousu on GitHub (Feb 1, 2022).

Hi there! I'm trying to understand this line

80048c091a/routers/private/serv.go (L319-L322)

This is in the "permissions checking" section:

80048c091a/routers/private/serv.go (L303)

but it seems that in the common case, by clamping the requested permission to AccessModeRead, this check never triggers:

80048c091a/routers/private/serv.go (L336-L342)

Instead, permissions are enforced by a pre-receive hook, implemented at:

80048c091a/routers/private/hook_pre_receive.go (L61-L67)

Is this dead code or is there something bigger I'm misunderstanding? It seems like these are redundant code paths.

Where is that "delay[ed] write permission check" it mentions? Did it used to be in this file but now it's long gone?

Thanks for you time.

Originally created by @kousu on GitHub (Feb 1, 2022). Hi there! I'm trying to understand this line https://github.com/go-gitea/gitea/blob/80048c091a13d2bae9b72497d306e1acf4046be4/routers/private/serv.go#L319-L322 This is in the "permissions checking" section: https://github.com/go-gitea/gitea/blob/80048c091a13d2bae9b72497d306e1acf4046be4/routers/private/serv.go#L303 but it seems that in the common case, by clamping the requested permission to AccessModeRead, this check never triggers: https://github.com/go-gitea/gitea/blob/80048c091a13d2bae9b72497d306e1acf4046be4/routers/private/serv.go#L336-L342 Instead, permissions are enforced by a pre-receive hook, implemented at: https://github.com/go-gitea/gitea/blob/80048c091a13d2bae9b72497d306e1acf4046be4/routers/private/hook_pre_receive.go#L61-L67 Is this dead code or is there something bigger I'm misunderstanding? It seems like these are redundant code paths. Where is that "delay[ed] write permission check" it mentions? Did it used to be in this file but now it's long gone? Thanks for you time.
Author
Owner

@kousu commented on GitHub (Feb 1, 2022):

If I comment out that first line, then pulling from a read-only repo still works:

[user2@hostname beep]$ git pull
remote: Enumerating objects: 26, done.
remote: Counting objects: 100% (26/26), done.
remote: Compressing objects: 100% (17/17), done.
remote: Total 21 (delta 7), reused 0 (delta 0), pack-reused 0
Unpacking objects: 100% (21/21), 1.59 KiB | 24.00 KiB/s, done.
From localhost:root/beep
   50c2256..fb82c07  git-annex  -> origin/git-annex
Already up to date.

And pushing fails; but it fails with a modified error message. That is, instead of the usual:

[user2@hostname beep]$ git push
Enumerating objects: 3, done.
Counting objects: 100% (3/3), done.
Delta compression using up to 4 threads
Compressing objects: 100% (2/2), done.
Writing objects: 100% (2/2), 216 bytes | 216.00 KiB/s, done.
Total 2 (delta 1), reused 0 (delta 0), pack-reused 0
remote: 
remote: Gitea: User permission denied.
To localhost:root/beep
 ! [remote rejected] master -> master (pre-receive hook declined)
error: failed to push some refs to 'localhost:root/beep'

with this in the log:

2022/02/01 01:51:56 router: completed POST /api/internal/hook/pre-receive/user1/beep for [::1]:46612, 403 Forbidden in 4.7ms @ private/hook_pre_receive.go:107(private.HookPreReceive)

I get this

[user2@hostname beep]$ git push
Gitea: Unauthorized
fatal: Could not read from remote repository.

Please make sure you have the correct access rights
and the repository exists.

With this in the log:

2022/02/01 01:45:31 ...ters/private/serv.go:343:ServCommand() [W] Failed authentication attempt for user2 with key user2@hostname (not authorized to write to user1/beep) from [::1]:46602

so either path seems to do the same thing, but with that line commented out the error is returned a bit quicker to the user -- though with a misleading error message.

@kousu commented on GitHub (Feb 1, 2022): If I comment out that first line, then pulling from a read-only repo still works: ``` [user2@hostname beep]$ git pull remote: Enumerating objects: 26, done. remote: Counting objects: 100% (26/26), done. remote: Compressing objects: 100% (17/17), done. remote: Total 21 (delta 7), reused 0 (delta 0), pack-reused 0 Unpacking objects: 100% (21/21), 1.59 KiB | 24.00 KiB/s, done. From localhost:root/beep 50c2256..fb82c07 git-annex -> origin/git-annex Already up to date. ``` And pushing fails; but it fails with a modified error message. That is, instead of the usual: ``` [user2@hostname beep]$ git push Enumerating objects: 3, done. Counting objects: 100% (3/3), done. Delta compression using up to 4 threads Compressing objects: 100% (2/2), done. Writing objects: 100% (2/2), 216 bytes | 216.00 KiB/s, done. Total 2 (delta 1), reused 0 (delta 0), pack-reused 0 remote: remote: Gitea: User permission denied. To localhost:root/beep ! [remote rejected] master -> master (pre-receive hook declined) error: failed to push some refs to 'localhost:root/beep' ``` with this in the log: ``` 2022/02/01 01:51:56 router: completed POST /api/internal/hook/pre-receive/user1/beep for [::1]:46612, 403 Forbidden in 4.7ms @ private/hook_pre_receive.go:107(private.HookPreReceive) ``` I get this ``` [user2@hostname beep]$ git push Gitea: Unauthorized fatal: Could not read from remote repository. Please make sure you have the correct access rights and the repository exists. ``` With this in the log: ``` 2022/02/01 01:45:31 ...ters/private/serv.go:343:ServCommand() [W] Failed authentication attempt for user2 with key user2@hostname (not authorized to write to user1/beep) from [::1]:46602 ``` so either path seems to do the same thing, but with that line commented out the error is returned a bit quicker to the user -- though with a misleading error message.
Author
Owner

@kousu commented on GitHub (Feb 1, 2022):

I traced this out from

80048c091a/cmd/serv.go (L217-L227)

It seems like what that is trying to say is: "we know that git-lfs-authenticate $repo upload needs write permission, and download needs read permission. Ask the API to confirm this user is allowed to use git-lfs this way." But over in the API (routers/private/serv.go) mode is used in very few places, and lfsVerb not at all.

@kousu commented on GitHub (Feb 1, 2022): I traced this out from https://github.com/go-gitea/gitea/blob/80048c091a13d2bae9b72497d306e1acf4046be4/cmd/serv.go#L217-L227 It seems like what that is trying to say is: "we know that `git-lfs-authenticate $repo upload` needs write permission, and `download` needs read permission. Ask the API to confirm this user is allowed to use git-lfs this way." But over in the API (routers/private/serv.go) `mode` is used in very few places, and `lfsVerb` not at all.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github-starred/gitea#8447