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

"Resynchronize pre-receive, update and post-receive hooks" should explicitly set executable bit #6319

Closed
2 tasks done
SagePtr opened this issue Mar 13, 2019 · 2 comments · Fixed by #10943
Closed
2 tasks done
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/enhancement An improvement of existing functionality

Comments

@SagePtr
Copy link
Contributor

SagePtr commented Mar 13, 2019

  • Gitea version (or commit ref): 1.7.4
  • Git version: 2.17.1
  • Operating system: linux
  • Database (use [x]):
    • Not relevant
  • Can you reproduce the bug at https://try.gitea.io:
    • Not relevant
  • Log gist:

Description

If repositories directory is copied from Windows to Linux machine - hooks will lose executable permissions. It's also easy to forget this aspect when restoring from backup using Windows machine.

Function "Resynchronize pre-receive, update and post-receive hooks" are very handy when dealing with migration from one server to another, but the only problem is - it doesn't set executable bit explicitely if hook files already exist, it set it only for new files via WriteFile third parameter.

Would be nice if this function will also fix hook files permissions, not only overwrite hooks content with new path to gitea executable.

Or separate function "Fix executable permission on all hooks" which should recursively set +x on all files inside each repo hooks directory (hide this function from admin panel if os == 'windows')

@SagePtr SagePtr changed the title "Resynchronize pre-receive, update and post-receive hooks" should explicitly set executable "Resynchronize pre-receive, update and post-receive hooks" should explicitly set executable bit Mar 13, 2019
@lunny lunny added the type/enhancement An improvement of existing functionality label Mar 14, 2019
@stale
Copy link

stale bot commented May 19, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 weeks. Thank you for your contributions.

@stale stale bot added the issue/stale label May 19, 2019
@lunny lunny added the issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented label May 19, 2019
@stale stale bot removed the issue/stale label May 19, 2019
@zeripath
Copy link
Contributor

So looking at:

gitea/models/repo.go

Lines 953 to 990 in 8de76b6

// createDelegateHooks creates all the hooks scripts for the repo
func createDelegateHooks(repoPath string) (err error) {
var (
hookNames = []string{"pre-receive", "update", "post-receive"}
hookTpls = []string{
fmt.Sprintf("#!/usr/bin/env %s\ndata=$(cat)\nexitcodes=\"\"\nhookname=$(basename $0)\nGIT_DIR=${GIT_DIR:-$(dirname $0)}\n\nfor hook in ${GIT_DIR}/hooks/${hookname}.d/*; do\ntest -x \"${hook}\" || continue\necho \"${data}\" | \"${hook}\"\nexitcodes=\"${exitcodes} $?\"\ndone\n\nfor i in ${exitcodes}; do\n[ ${i} -eq 0 ] || exit ${i}\ndone\n", setting.ScriptType),
fmt.Sprintf("#!/usr/bin/env %s\nexitcodes=\"\"\nhookname=$(basename $0)\nGIT_DIR=${GIT_DIR:-$(dirname $0)}\n\nfor hook in ${GIT_DIR}/hooks/${hookname}.d/*; do\ntest -x \"${hook}\" || continue\n\"${hook}\" $1 $2 $3\nexitcodes=\"${exitcodes} $?\"\ndone\n\nfor i in ${exitcodes}; do\n[ ${i} -eq 0 ] || exit ${i}\ndone\n", setting.ScriptType),
fmt.Sprintf("#!/usr/bin/env %s\ndata=$(cat)\nexitcodes=\"\"\nhookname=$(basename $0)\nGIT_DIR=${GIT_DIR:-$(dirname $0)}\n\nfor hook in ${GIT_DIR}/hooks/${hookname}.d/*; do\ntest -x \"${hook}\" || continue\necho \"${data}\" | \"${hook}\"\nexitcodes=\"${exitcodes} $?\"\ndone\n\nfor i in ${exitcodes}; do\n[ ${i} -eq 0 ] || exit ${i}\ndone\n", setting.ScriptType),
}
giteaHookTpls = []string{
fmt.Sprintf("#!/usr/bin/env %s\n\"%s\" hook --config='%s' pre-receive\n", setting.ScriptType, setting.AppPath, setting.CustomConf),
fmt.Sprintf("#!/usr/bin/env %s\n\"%s\" hook --config='%s' update $1 $2 $3\n", setting.ScriptType, setting.AppPath, setting.CustomConf),
fmt.Sprintf("#!/usr/bin/env %s\n\"%s\" hook --config='%s' post-receive\n", setting.ScriptType, setting.AppPath, setting.CustomConf),
}
)
hookDir := filepath.Join(repoPath, "hooks")
for i, hookName := range hookNames {
oldHookPath := filepath.Join(hookDir, hookName)
newHookPath := filepath.Join(hookDir, hookName+".d", "gitea")
if err := os.MkdirAll(filepath.Join(hookDir, hookName+".d"), os.ModePerm); err != nil {
return fmt.Errorf("create hooks dir '%s': %v", filepath.Join(hookDir, hookName+".d"), err)
}
// WARNING: This will override all old server-side hooks
if err = ioutil.WriteFile(oldHookPath, []byte(hookTpls[i]), 0777); err != nil {
return fmt.Errorf("write old hook file '%s': %v", oldHookPath, err)
}
if err = ioutil.WriteFile(newHookPath, []byte(giteaHookTpls[i]), 0777); err != nil {
return fmt.Errorf("write new hook file '%s': %v", newHookPath, err)
}
}
return nil
}

The gitea hooks should be being recreated with their executable bit set.

zeripath added a commit that referenced this issue Apr 6, 2020
* Add `gitea doctor --list` flag to list the checks that will be run, including those by default
* Add `gitea doctor --run` to run specific checks
* Add `gitea doctor --all` to run all checks
* Add db version checker
* Add non-default recalculate merge bases check/fixer to doctor
* Add hook checker (Fix #9878) and ensure hooks are executable (Fix #6319)
* Fix authorized_keys checker - slight change of functionality here because parsing the command is fragile and we should just check if the authorized_keys file is essentially the same as what gitea would produce. (This is still not perfect as order matters - we should probably just md5sum the two files.)
* Add SCRIPT_TYPE check (Fix #10977)
* Add `gitea doctor --fix` to attempt to fix what is possible to easily fix
* Add `gitea doctor --log-file` to set the log-file, be it a file, stdout or to switch off completely. (Fixes previously undetected bug with certain xorm logging configurations - see @6543 comment.)

Signed-off-by: Andrew Thornton <art27@cantab.net>
zeripath added a commit to zeripath/gitea that referenced this issue Apr 6, 2020
Backport go-gitea#10943

* Add `gitea doctor --list` flag to list the checks that will be run, including those by default
* Add `gitea doctor --run` to run specific checks
* Add `gitea doctor --all` to run all checks
* Add db version checker
* Add non-default recalculate merge bases check/fixer to doctor
* Add hook checker (Fix go-gitea#9878) and ensure hooks are executable (Fix go-gitea#6319)
* Fix authorized_keys checker - slight change of functionality here because parsing the command is fragile and we should just check if the authorized_keys file is essentially the same as what gitea would produce. (This is still not perfect as order matters - we should probably just md5sum the two files.)
* Add SCRIPT_TYPE check (Fix go-gitea#10977)
* Add `gitea doctor --fix` to attempt to fix what is possible to easily fix
* Add `gitea doctor --log-file` to set the log-file, be it a file, stdout or to switch off completely. (Fixes previously undetected bug with certain xorm logging configurations - see @6543 comment.)

Signed-off-by: Andrew Thornton <art27@cantab.net>
techknowlogick pushed a commit that referenced this issue Apr 6, 2020
…#10991)

* Mulitple Gitea Doctor improvements (#10943)

Backport #10943

* Add `gitea doctor --list` flag to list the checks that will be run, including those by default
* Add `gitea doctor --run` to run specific checks
* Add `gitea doctor --all` to run all checks
* Add db version checker
* Add non-default recalculate merge bases check/fixer to doctor
* Add hook checker (Fix #9878) and ensure hooks are executable (Fix #6319)
* Fix authorized_keys checker - slight change of functionality here because parsing the command is fragile and we should just check if the authorized_keys file is essentially the same as what gitea would produce. (This is still not perfect as order matters - we should probably just md5sum the two files.)
* Add SCRIPT_TYPE check (Fix #10977)
* Add `gitea doctor --fix` to attempt to fix what is possible to easily fix
* Add `gitea doctor --log-file` to set the log-file, be it a file, stdout or to switch off completely. (Fixes previously undetected bug with certain xorm logging configurations - see @6543 comment.)

Signed-off-by: Andrew Thornton <art27@cantab.net>

* Switch to io.Writer instead of io.StringWriter

Signed-off-by: Andrew Thornton <art27@cantab.net>
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/confirmed Issue has been reviewed and confirmed to be present or accepted to be implemented type/enhancement An improvement of existing functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants