FrankChen021 commented on code in PR #20477:
URL: https://github.com/apache/druid/pull/20477#discussion_r4175793798
##########
hooks/uninstall-hooks.sh:
##########
@@ -19,19 +19,14 @@
# under the License.
#
-if [ $# != 1 ]
- then
- echo 'usage: program {$DRUID_ROOT}'
- exit 1
-fi
-
-DRUID_ROOT=$1
+DRUID_ROOT="${1:-$(dirname "$0")/..}"
+GIT_HOOKS_DIR="$(git -C "${DRUID_ROOT}" rev-parse --path-format=absolute
--git-path hooks)"
# This script does not remove .git/hooks/pre-commit, .git/hooks/pre-push, or
any other git hook scripts
# because those files may have user-custom hooks.
# Instead, we remove only the files and directories that we are sure it is
safe to remove.
-rm -f ${DRUID_ROOT}/.git/hooks/run-all-in-dir.py
-rm -rf ${DRUID_ROOT}/.git/hooks/pre-commits
-rm -rf ${DRUID_ROOT}/.git/hooks/pre-pushes
+rm -f "${GIT_HOOKS_DIR}/run-all-in-dir.py"
+rm -rf "${GIT_HOOKS_DIR}/pre-commits"
Review Comment:
[P1] Avoid deleting unowned shared hook directories
**Finding:** Because GIT_HOOKS_DIR now follows core.hooksPath, it can be a
global/shared hooks directory—or the repository's tracked hooks/ directory. The
unconditional recursive removal of pre-commits (and the analogous pre-pushes
removal) can delete hook directories owned by another repository or tracked
Druid sources when uninstall-hooks.sh is run, causing data loss and leaving
hooks broken.
**Suggestion:** Track ownership of installed files and directories and only
remove entries created by this installer; at minimum, refuse to uninstall when
the resolved hooks path is shared or overlaps the source hooks tree.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
