Merge pull request #76 from elasticdog/69-fix-merge-of-encrypted-files-with-conflicts

Fix `transcrypt`'s handling of merges where encrypted files have conflicting changes, a situation which would lead to Git producing "merged" files with conflict markers around partially- or fully-encrypted content that cannot be sensibly merged by a person. See issue #69 and a bunch of related issues.

The root problem is that git does not run the `smudge`/`textconv` filter on all BASE, LOCAL, REMOTE conflicting version files before attempting a three-way merge.

This change adds:
- a merge driver script to pre-decrypt conflicting BASE, LOCAL, and REMOTE file  versions then run git's internal `merge-file`  command to merge the decrypted versions
- git repo settings to configure the merge driver
- recommendation to add the extra `merge=crypt`  setting to *.gitattribute* definitions
- tests of merge functionality to prove that non-conflicting and conflicting merges work.

Also included are minor listing and formatting fixes from applying the recommended tools to do this clean-up, and documentation for how to run these tools in *README.md*

The bulk of this work is originally from https://github.com/ixc/transcrypt/commits/fix-merge-with-conflicts

Author: jmurty

.editorconfig

@@ -7,7 +7,7 @@ insert_final_newline = true
 trim_trailing_whitespace = true
 
 [*.md]
-indent_size = 4
+indent_size = 2
 indent_style = space
 trim_trailing_whitespace = false
 

README.md

@@ -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 &copy; 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

tests/_test_helper.bash

@@ -43,7 +43,7 @@ function encrypt_named_file {
   if [ "$content" ]; then
     echo "$content" > $filename
   fi
-  echo "$filename filter=crypt diff=crypt" >> .gitattributes
+  echo "$filename filter=crypt diff=crypt merge=crypt" >> .gitattributes
   git add .gitattributes $filename
   git commit -m "Encrypt file $filename"
 }

tests/test_init.bats

@@ -17,7 +17,7 @@ SETUP_SKIP_INIT_TRANSCRYPT=1
   init_transcrypt
   [ -f .gitattributes ]
   run cat .gitattributes
-  [ "${lines[0]}" = "#pattern  filter=crypt diff=crypt" ]
+  [ "${lines[0]}" = "#pattern  filter=crypt diff=crypt merge=crypt" ]
 }
 
 @test "init: creates scripts in .git/crypt/" {

tests/test_merge.bats

@@ -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" ]
+}

transcrypt

@@ -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
+
+		# 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'