Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gitea post-receive hook not working with docker image 1.17-rc1 #20103

Closed
CleyFaye opened this issue Jun 23, 2022 · 10 comments
Closed

Gitea post-receive hook not working with docker image 1.17-rc1 #20103

CleyFaye opened this issue Jun 23, 2022 · 10 comments
Labels
issue/duplicate The issue has already been reported. issue/not-a-bug The reported issue is the intended behavior or the problem is not inside Gitea
Milestone

Comments

@CleyFaye
Copy link

Description

When pushing new commits to gitea, the hooks are not triggered. This (among other things) does not create references between issues and commits, and we can't close issues with commit messages.

After investigating, it comes down to the test -x command exiting with a status of 1 despite the hook scripts being available and executable.
This issue only happens when using the built-in "test" command from bash. Running /usr/bin/test -x <file> returns the expected result. This seems to only happen in the docker image (current latest is 1.17-rc1)

Here's the things I tested (from a git repository in gitea, using docker exec:

$ ls -l hooks
total 84
-rwxr-xr-x    1 git      git            478 Jun 23 11:38 applypatch-msg.sample
-rwxr-xr-x    1 git      git            896 Jun 23 11:38 commit-msg.sample
-rwxr-xr-x    1 git      git            461 Jun 23 13:18 post-receive
drwxr-xr-x    2 git      git           4096 Jun 23 11:38 post-receive.d
-rwxr-xr-x    1 git      git            189 Jun 23 11:38 post-update.sample
-rwxr-xr-x    1 git      git            424 Jun 23 11:38 pre-applypatch.sample
-rwxr-xr-x    1 git      git           1643 Jun 23 11:38 pre-commit.sample
-rwxr-xr-x    1 git      git            416 Jun 23 11:38 pre-merge-commit.sample
-rwxr-xr-x    1 git      git           1374 Jun 23 11:38 pre-push.sample
-rwxr-xr-x    1 git      git           4898 Jun 23 11:38 pre-rebase.sample
-rwxr-xr-x    1 git      git            376 Jun 23 11:38 pre-receive
drwxr-xr-x    2 git      git           4096 Jun 23 11:38 pre-receive.d
-rwxr-xr-x    1 git      git            544 Jun 23 11:38 pre-receive.sample
-rwxr-xr-x    1 git      git           1492 Jun 23 11:38 prepare-commit-msg.sample
-rwxr-xr-x    1 git      git            134 Jun 23 11:38 proc-receive
drwxr-xr-x    2 git      git           4096 Jun 23 11:38 proc-receive.d
-rwxr-xr-x    1 git      git           2783 Jun 23 11:38 push-to-checkout.sample
-rwxr-xr-x    1 git      git            356 Jun 23 11:38 update
drwxr-xr-x    2 git      git           4096 Jun 23 11:38 update.d
-rwxr-xr-x    1 git      git           3650 Jun 23 11:38 update.sample

$ ls -l hooks/post-receive.d
total 4
-rwxr-xr-x    1 git      git            134 Jun 23 11:38 gitea

$ cat hooks/post-receive
#!/usr/bin/env bash
# AUTO GENERATED BY GITEA, DO NOT MODIFY
data=$(cat)
exitcodes=""
hookname=$(basename $0)
GIT_DIR=${GIT_DIR:-$(dirname $0)/..}

for hook in ${GIT_DIR}/hooks/${hookname}.d/*; do
  echo "TEST BEFORE:${hook}" # Added for testing
  test -x "${hook}" && test -f "${hook}" || continue
  echo "TEST AFTER" # Added for testing
  echo "${data}" | "${hook}"
  exitcodes="${exitcodes} $?"
done

for i in ${exitcodes}; do
  [ ${i} -eq 0 ] || exit ${i}
done

$ test -x ./hooks/post-receive.d/gitea || echo "Failed"

$ bash

$ test -x ./hooks/post-receive.d/gitea || echo "Failed"
Failed

I added the "echo" to check, and indeed when pushing I do get "TEST BEFORE", but not "TEST AFTER".

I have no idea what causes this behavior, exclusively in bash, exclusively in docker, but replacing "test" with "/usr/bin/test" do fix the issue (hooks run and tickets are referenced correctly, among other things).

Unfortunately I do not know where I could live-edit the container to update all repositories hook at once, which could be a temporary fix too.

Gitea Version

1.17-rc1

Can you reproduce the bug on the Gitea demo site?

No

Log Gist

No response

Screenshots

No response

Git Version

No response

Operating System

No response

How are you running Gitea?

We run gitea through docker, using docker-compose as suggested in the doc and using the "official" gitea images on docker hub.

Database

PostgreSQL

@CleyFaye
Copy link
Author

Among the things affected, authorization control (for users that have read access but no write access) is also broken; a user did push into a repository he should not have access to (the pre-receive hook didn't stop him).

@noerw
Copy link
Member

noerw commented Jun 23, 2022

What docker / runc / kernel version are you running?
This may be caused by this bug/breaking change in alpine 3.14+, you need to update your docker install or host kernel: alpinelinux/docker-alpine#156 (comment)

Interestingly, #18050 added a safeguard against this which doesn't seem to work in your case.

Either way, the pre-receive hook probably should have a safeguard that denies access if the script failed earlier..

@6543 6543 added this to the 1.17.0 milestone Jun 23, 2022
@CleyFaye
Copy link
Author

CleyFaye commented Jun 23, 2022

Here they are:

  • Docker v18.09.1, build 4c52b90
  • runc version 1.0.0rc6+dfsg1 (comit: 1.0.0rc6+dfsg1-3 spec: 1.0.1, I have no idea which one of these is useful)
  • Kernel 4.19.0-20-amd64

Since I'm based on Debian 10, I can't really update them individually; however, also since I'm based on Debian 10 which is due for an OS update, I'll just do that and see if it fixes the issue (it should, as the issue you've linked is most likely the culprit).

Still, pre-receive really should err on the side of caution. It was quite a shock seeing an unexpected new branch pushed into one of our (allegedly) read-only repository. Not much trouble in our case, but still worrying.

Regarding the safeguard, it won't work in this case because running if [ -x /bin/sh ] in sh works as intended, even in this case. It's the built-in test from bash that behaves differently.

Also, thanks, this stumbled us for quite a bit; having an "easy" way out of this trouble is great :)

@techknowlogick
Copy link
Member

Docker v18.09.1, build 4c52b90

Please see notes for alpine which conflict with the version of docker you are using https://wiki.alpinelinux.org/wiki/Release_Notes_for_Alpine_3.14.0 specifically the faccessat2 section due to seccomp changes.

@CleyFaye
Copy link
Author

Yes, got it. Updating the environment outside docker fixed it.

I think all that remains here is to modify the safeguard so that other won't fall into this pitfall.

Thanks again.

@zeripath zeripath added issue/duplicate The issue has already been reported. issue/not-a-bug The reported issue is the intended behavior or the problem is not inside Gitea and removed type/bug labels Jun 23, 2022
@zeripath
Copy link
Contributor

If only modifying the safeguard were so simple. We've been aware of the problem for a while, Gitlab has also been affected and unfortunately it seems the only solution is to tell people to upgrade their Docker. (https://about.gitlab.com/blog/2021/08/26/its-time-to-upgrade-docker-engine/)

See #18050 where we upgraded alpine. In the blog post for 1.17 we will add a specific BREAKING notice.

If you are willing to find and write a PR that will detect the correct version we'll be happy to consider a PR.

Otherwise I'm going to close this as a duplicate of #16428, #16451, #16170, #16464, & #16484 amongst others

@CleyFaye
Copy link
Author

Thinking about it, I fear that "improving" the safeguard might not be enough to avoid other potential issues. Best case scenario it would detect if bash specifically fails (something like /bin/bash -c 'test -x /bin/sh) and might hide other less obvious issues.

I could write something that provides the basis of testing docker (if applicable) and kernel versions so that minimum could be enforced, but I'm not sure I'd have time to thoroughly check this through. Maybe depending on sysadmin doing their due diligences with updates (which I clearly did not, sorry about that) would be a better bet after all.

@jpraet
Copy link
Member

jpraet commented Jun 24, 2022

I also got bitten by this one. And the safeguard indeed does not catch it unfortunately.

I have no idea what causes this behavior, exclusively in bash, exclusively in docker, but replacing "test" with "/usr/bin/test" do fix the issue (hooks run and tickets are referenced correctly, among other things).

Would it make sense to change these hooks to use "/usr/bin/test" instead of "test"?
I think that way older versions of docker can remain supported, by executing gitea admin regenerate hooks.

@jpraet
Copy link
Member

jpraet commented Jun 25, 2022

Or to avoid depending on the /usr/bin/test location:
$(which test) -x "${hook}" && $(which test) -f "${hook}" || continue

@zeripath
Copy link
Contributor

Failures will almost certainly occur elsewhere as anything that calls faccessat2 will fail.

Alpine 3.14+ is fundamentally broken on Docker < 20.10.6 because of this.

The Alpine 3.14 release notes state:


faccessat2

Use of the faccessat2 syscall has been enabled in musl. Due to runc issue 2151, new system calls incorrectly returned EPERM instead of ENOSYS when invoked under a Docker with libseccomp predating their release. Therefore, Alpine Linux 3.14 requires at least one of the following:

  1. runc v1.0.0-rc93
    • if using Docker's Debian repositories, this is part of containerd.io 1.4.3-2
    • if using Docker Desktop for Windows or Mac, this is part of Docker Desktop 3.3.0
  2. Docker 20.10.0 (which contains moby commit a181391) or greater, AND libseccomp 2.4.4 (which contains backported libseccomp commit 5696c89) or greater. In this case, to check if your host libseccomp is faccessat2-compatible, invoke scmp_sys_resolver faccessat2. If 439 is returned, faccessat2 is supported. If -1 is returned, faccessat2 is not supported. Note that if runc is older than v1.0.0-rc93, Docker must still be at least version 20.10.0, regardless of the result of this command.
  3. As a workaround, in order to run under old Docker or libseccomp versions, the moby default seccomp profile should be downloaded and on line 2, defaultAction changed to SCMP_ACT_TRACE, then --seccomp-profile=default.json can be passed to dockerd, or --security-opt=seccomp=default.json passed to docker create or docker run. This will cause the system calls to return ENOSYS instead of EPERM, allowing the container to fall back to faccessat.

Note also that when using nested Docker, every layer must meet one of the above requirements, since if any layer improperly denies the use of faccessat2, Alpine Linux 3.14 will not function correctly.


We could use /usr/bin/test which will probably work until that is updated to use faccessat2, we could write some code in go that would detect that faccessat2 is reporting incorrectly but fundamentally people just need to upgrade.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/duplicate The issue has already been reported. issue/not-a-bug The reported issue is the intended behavior or the problem is not inside Gitea
Projects
None yet
Development

No branches or pull requests

6 participants