-
Notifications
You must be signed in to change notification settings - Fork 104
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
Fix merge of encrypted files with conflicts (#69) #76
Changes from all commits
577c005
3fabd75
546cec9
3710f95
44f1c0e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -92,14 +92,14 @@ using the command line options. Run `transcrypt --help` for more details. | |
### Designate a File to be Encrypted | ||
|
||
Once a repository has been configured with transcrypt, you can designate for | ||
files to be encrypted by applying the "crypt" filter and diff to a | ||
files to be encrypted by applying the "crypt" filter, diff, and merge to a | ||
[pattern](https://www.kernel.org/pub/software/scm/git/docs/gitignore.html#_pattern_format) | ||
in the top-level _[.gitattributes](http://git-scm.com/docs/gitattributes)_ | ||
config. If that pattern matches a file in your repository, the file will be | ||
transparently encrypted once you stage and commit it: | ||
|
||
$ cd <path-to-your-repo>/ | ||
$ echo 'sensitive_file filter=crypt diff=crypt' >> .gitattributes | ||
$ echo 'sensitive_file filter=crypt diff=crypt merge=crypt' >> .gitattributes | ||
$ git add .gitattributes sensitive_file | ||
$ git commit -m 'Add encrypted version of a sensitive file' | ||
|
||
|
@@ -297,11 +297,22 @@ Copyright © 2014-2020, [Aaron Bull Schaefer](mailto:[email protected]). | |
|
||
## Contributing | ||
|
||
### Linting and formatting | ||
|
||
Please use: | ||
|
||
- the [shellcheck](https://www.shellcheck.net) tool to check for subtle bash | ||
scripting errors in the _transcrypt_ file, and apply the recommendations when | ||
possible. E.g: `shellcheck transcrypt` | ||
- the [shfmt](https://github.com/mvdan/sh) tool to apply consistent formatting | ||
to the _transcrypt_ file, e.g: `shfmt -w transcrypt` | ||
- the [Prettier](https://prettier.io) tool to apply consistent formatting to the | ||
_README.md_ file, e.g: `prettier --write README.md` | ||
|
||
### Tests | ||
|
||
Tests are written using [bats-core](https://github.com/bats-core/bats-core) | ||
version of "Bash Automated Testing System" and stored in the *tests/* | ||
directory. | ||
version of "Bash Automated Testing System" and stored in the _tests/_ directory. | ||
|
||
To run the tests: | ||
|
||
|
@@ -311,6 +322,15 @@ To run the tests: | |
|
||
## Changes | ||
|
||
Fixes: | ||
|
||
- Fix handling of branch merges with conflicts in encrypted files, which would | ||
previously leave the user to manually merge files with a mix of encrypted and | ||
unencrypted content. | ||
|
||
To apply this fix in projects that already use transcrypt: uninstall and | ||
re-init transcrypt, then add `merge=crypt` to the patterns in _.gitattributes_ | ||
|
||
Improvements: | ||
|
||
- Add Git pre-commit hook to reject commit of file that should be encrypted but | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,69 @@ | ||
#!/usr/bin/env bats | ||
|
||
load $BATS_TEST_DIRNAME/_test_helper.bash | ||
|
||
@test "merge: branches with encrypted file - addition, no conflict" { | ||
echo "1. First step" > sensitive_file | ||
encrypt_named_file sensitive_file | ||
|
||
git checkout -b branch-2 | ||
echo "2. Second step" >> sensitive_file | ||
git add sensitive_file | ||
git commit -m "Add line 2" | ||
|
||
git checkout - | ||
git merge branch-2 | ||
|
||
run cat sensitive_file | ||
[ "$status" -eq 0 ] | ||
[ "${lines[0]}" = "1. First step" ] | ||
[ "${lines[1]}" = "2. Second step" ] | ||
} | ||
|
||
@test "merge: branches with encrypted file - line change, no conflict" { | ||
echo "1. First step" > sensitive_file | ||
encrypt_named_file sensitive_file | ||
|
||
git checkout -b branch-2 | ||
echo "1. Step the first" > sensitive_file # Cause line conflict | ||
echo "2. Second step" >> sensitive_file | ||
git add sensitive_file | ||
git commit -m "Add line 2, change line 1" | ||
|
||
git checkout - | ||
git merge branch-2 | ||
|
||
run cat sensitive_file | ||
[ "$status" -eq 0 ] | ||
[ "${lines[0]}" = "1. Step the first" ] | ||
[ "${lines[1]}" = "2. Second step" ] | ||
} | ||
|
||
@test "merge: branches with encrypted file - with conflicts" { | ||
echo "1. First step" > sensitive_file | ||
encrypt_named_file sensitive_file | ||
|
||
git checkout -b branch-2 | ||
echo "1. Step the first" > sensitive_file # Cause line conflict | ||
echo "2. Second step" >> sensitive_file | ||
git add sensitive_file | ||
git commit -m "Add line 2, change line 1" | ||
|
||
git checkout - | ||
echo "a. First step" > sensitive_file | ||
git add sensitive_file | ||
git commit -m "Change line 1 in original branch to set up conflict" | ||
|
||
run git merge branch-2 | ||
[ "$status" -ne 0 ] | ||
[ "${lines[1]}" = "CONFLICT (content): Merge conflict in sensitive_file" ] | ||
|
||
run cat sensitive_file | ||
[ "$status" -eq 0 ] | ||
[ "${lines[0]}" = "<<<<<<< master" ] | ||
[ "${lines[1]}" = "a. First step" ] | ||
[ "${lines[2]}" = "=======" ] | ||
[ "${lines[3]}" = "1. Step the first" ] | ||
[ "${lines[4]}" = "2. Second step" ] | ||
[ "${lines[5]}" = ">>>>>>> branch-2" ] | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -322,8 +322,52 @@ save_helper_scripts() { | |
fi | ||
EOF | ||
|
||
cat <<-'EOF' >"${GIT_DIR}/crypt/merge" | ||
#!/usr/bin/env bash | ||
|
||
# Look up name of local branch/ref to which changes are being merged | ||
OURS_LABEL=$(git rev-parse --abbrev-ref HEAD) | ||
|
||
# Look up name of the incoming "theirs" branch/ref being merged in. | ||
# TODO There must be a better way of doing this than relying on this reflog | ||
# action environment variable, but I don't know what it is | ||
if [[ "$GIT_REFLOG_ACTION" = "merge "* ]]; then | ||
THEIRS_LABEL=$(echo $GIT_REFLOG_ACTION | awk '{print $2}') | ||
fi | ||
if [[ ! "$THEIRS_LABEL" ]]; then | ||
THEIRS_LABEL="theirs" | ||
fi | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I haven't pulled your changes yet to test things out locally, but would this be:
??? There's also There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I tried that but it doesn't work for the tests which have no upstream, and isn't quite what is needed. Ideally we would use something like this: So far the awful |
||
|
||
# Decrypt BASE, LOCAL, and REMOTE versions of file being merged | ||
echo "$(cat $1 | ./.git/crypt/smudge)" > $1 | ||
echo "$(cat $2 | ./.git/crypt/smudge)" > $2 | ||
echo "$(cat $3 | ./.git/crypt/smudge)" > $3 | ||
|
||
# Merge the decrypted files to the working copy named by $5 | ||
# We must redirect stdout to $5 here instead of letting merge-file write to | ||
# $2 as it would by default, because we need $5 to contain the final result | ||
# content so a later crypt `clean` generates the correct hash salt value | ||
git merge-file --stdout --marker-size=$4 -L $OURS_LABEL -L base -L $THEIRS_LABEL $2 $1 $3 > $5 | ||
|
||
if [[ "$?" == "0" ]]; then | ||
# If the merge was successful (no conflicts) re-encrypt the merged working | ||
# copy file to the incoming "local" temp file $2 which git will then | ||
# update in the index during the "Auto-merging" step. | ||
# Git needs the cleaned copy to avoid triggering the error: | ||
# error: add_cacheinfo failed to refresh for path 'FILE'; merge aborting. | ||
echo "$(cat $5 | ./.git/crypt/clean $5)" > $2 | ||
exit 0 | ||
else | ||
# If the merge was not successful, copy the merged working copy file to the | ||
# "local" temp file $2 which git will then re-copy back to the working copy | ||
# so the user can fix it manually | ||
cp $5 $2 | ||
exit 1 | ||
fi | ||
EOF | ||
|
||
# make scripts executable | ||
for script in {clean,smudge,textconv}; do | ||
for script in {clean,smudge,textconv,merge}; do | ||
chmod 0755 "${GIT_DIR}/crypt/${script}" | ||
done | ||
} | ||
|
@@ -392,18 +436,23 @@ save_configuration() { | |
git config filter.crypt.smudge '"$(git rev-parse --git-common-dir)"/crypt/smudge' | ||
# shellcheck disable=SC2016 | ||
git config diff.crypt.textconv '"$(git rev-parse --git-common-dir)"/crypt/textconv' | ||
# shellcheck disable=SC2016 | ||
git config merge.crypt.driver '"$(git rev-parse --git-common-dir)"/crypt/merge %O %A %B %L %P' | ||
else | ||
# shellcheck disable=SC2016 | ||
git config filter.crypt.clean '"$(git rev-parse --git-dir)"/crypt/clean %f' | ||
# shellcheck disable=SC2016 | ||
git config filter.crypt.smudge '"$(git rev-parse --git-dir)"/crypt/smudge' | ||
# shellcheck disable=SC2016 | ||
git config diff.crypt.textconv '"$(git rev-parse --git-dir)"/crypt/textconv' | ||
# shellcheck disable=SC2016 | ||
git config merge.crypt.driver '"$(git rev-parse --git-dir)"/crypt/merge %O %A %B %L %P' | ||
fi | ||
git config filter.crypt.required 'true' | ||
git config diff.crypt.cachetextconv 'true' | ||
git config diff.crypt.binary 'true' | ||
git config merge.renormalize 'true' | ||
git config merge.crypt.name 'Merge transcrypt secret files' | ||
|
||
# add a git alias for listing encrypted files | ||
git config alias.ls-crypt "!git ls-files | git check-attr --stdin filter | awk 'BEGIN { FS = \":\" }; /crypt$/{ print \$1 }'" | ||
|
@@ -433,6 +482,7 @@ clean_gitconfig() { | |
git config --remove-section transcrypt 2>/dev/null || true | ||
git config --remove-section filter.crypt 2>/dev/null || true | ||
git config --remove-section diff.crypt 2>/dev/null || true | ||
git config --remove-section merge.crypt 2>/dev/null || true | ||
git config --unset merge.renormalize | ||
|
||
# remove the merge section if it's now empty | ||
|
@@ -512,7 +562,7 @@ uninstall_transcrypt() { | |
clean_gitconfig | ||
|
||
# remove helper scripts | ||
for script in {clean,smudge,textconv}; do | ||
for script in {clean,smudge,textconv,merge}; do | ||
[[ ! -f "${GIT_DIR}/crypt/${script}" ]] || rm "${GIT_DIR}/crypt/${script}" | ||
done | ||
[[ ! -d "${GIT_DIR}/crypt" ]] || rmdir "${GIT_DIR}/crypt" | ||
|
@@ -554,9 +604,11 @@ uninstall_transcrypt() { | |
case $OSTYPE in | ||
darwin*) | ||
/usr/bin/sed -i '' '/filter=crypt diff=crypt[ \t]*$/d' "$GIT_ATTRIBUTES" | ||
/usr/bin/sed -i '' '/filter=crypt diff=crypt merge=crypt[ \t]*$/d' "$GIT_ATTRIBUTES" | ||
;; | ||
linux*) | ||
sed -i '/filter=crypt diff=crypt[ \t]*$/d' "$GIT_ATTRIBUTES" | ||
sed -i '/filter=crypt diff=crypt merge=crypt[ \t]*$/d' "$GIT_ATTRIBUTES" | ||
;; | ||
esac | ||
|
||
|
@@ -745,7 +797,7 @@ help() { | |
a file in your repository, the file will be transparently encrypted | ||
once you stage and commit it: | ||
|
||
$ echo 'sensitive_file filter=crypt diff=crypt' >> .gitattributes | ||
$ echo 'sensitive_file filter=crypt diff=crypt merge=crypt' >> .gitattributes | ||
$ git add .gitattributes sensitive_file | ||
$ git commit -m 'Add encrypted version of a sensitive file' | ||
|
||
|
@@ -942,7 +994,7 @@ fi | |
# ensure the git attributes file exists | ||
if [[ ! -f $GIT_ATTRIBUTES ]]; then | ||
mkdir -p "${GIT_ATTRIBUTES%/*}" | ||
printf '#pattern filter=crypt diff=crypt\n' >"$GIT_ATTRIBUTES" | ||
printf '#pattern filter=crypt diff=crypt merge=crypt\n' >"$GIT_ATTRIBUTES" | ||
fi | ||
|
||
printf 'The repository has been successfully configured by transcrypt.\n' | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for picking up on those and adding the details to the README!