Fix --uninstall/--upgrade crash when saved pre-commit-crypt hook is missing - #212
Merged
jmurty merged 1 commit intoJun 8, 2026
Conversation
uninstall_transcrypt() assumed that if a pre-commit hook exists, the saved pre-commit-crypt copy also exists, and read it unconditionally. When the saved copy is absent the input redirection fails and 'set -euo pipefail' aborts the whole script mid-uninstall. This happens in repos where core.hooksPath points at a directory owned by a hook manager such as husky: the manager re-generates its hooks directory (removing pre-commit-crypt) and installs its own pre-commit shim. It also breaks 'transcrypt --upgrade', which runs uninstall internally. Guard the read and fall through to the existing 'Cannot safely disable' warning, since without the saved copy we cannot tell whether the pre-commit hook is ours.
johnjensenish
force-pushed
the
fix/uninstall-missing-pre-commit-hook-backup
branch
from
June 5, 2026 00:41
88af38e to
4f336db
Compare
johnjensenish
marked this pull request as ready for review
June 5, 2026 00:45
Collaborator
|
Thanks very much for this well explained and tested fix |
jmurty
added a commit
that referenced
this pull request
Jun 8, 2026
jmurty
added a commit
that referenced
this pull request
Jul 13, 2026
# By James Murty (4) and others # Via James Murty (4) and GitHub (1) * main: Add changelog entry for test suite improvements by @mattmc3 Run tests in parallel with isolated per-test tmp locations for speed Mention in changelog support for symlinked gitattributes file for `--install` and `--upgrade` actions Fix symlink following for gitattributes files Set merge.conflictStyle in test helper Use read, rather than awk to address NUL byte handling issues Remove `hexdump` requirement from README, it is not required since version 2.2.3 (2023-03-09) Document fix for crash when pre-commit-crypt hook file is missing #212 Fix --uninstall crash when saved pre-commit-crypt hook is missing # Conflicts: # .github/workflows/run-bats-core-tests.yml # tests/_test_helper.bash # tests/v3/test_cleanup.bats # tests/v3/test_contexts.bats # tests/v3/test_crypt.bats # tests/v3/test_init.bats # tests/v3/test_not_inited.bats # tests/v3/test_pre_commit.bats # transcrypt
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
transcrypt --uninstall(and--upgrade, which calls it) crashes when apre-commithook exists but transcrypt's savedpre-commit-cryptcopy does not:The read on line 905 is unguarded, so
set -euo pipefailaborts the script mid-uninstall.Our use of husky for pre-commit hooks makes this state likely:
core.hooksPathpoints at.husky/_, which husky re-generates on install — removingpre-commit-crypt— while installing its ownpre-commitshim. But any missing saved copy triggers it.The fix guards the read and falls through to the existing "Cannot safely disable Git pre-commit hook" warning — without the saved copy we can't tell whether the
pre-commithook is transcrypt's, so leave it alone and say so.Includes a regression test (fails on
main, passes with the fix). Full bats suite passes;shellcheckandshfmtclean.