Fix double encryption prevention (#346)

* Fix binary file bug double encryption prevention

The `ensureNoMetadata` function was incorrectly implemented and called
LoadEncryptedFile on the InputStore and checked whether the returned error was
MetadataNotFound or not. In the case where loading the input file as an encrypted
file would fail (e.g. due to syntax errors), it would incorrectly report the file as
having a "sops" branch. When using the binary mode, it would try to load the file as
an encrypted binary file (which is expected to be JSON), which would fail, thus
triggering this error.

* Add functional test for binary file roundtrip

cmd/sops/encrypt.go

@@ -43,12 +43,13 @@ func (err *fileAlreadyEncryptedError) UserError() string {
 	return wordwrap.WrapString(message, 75)
 }
 
-func ensureNoMetadata(opts encryptOpts, bytes []byte) error {
-	_, err := opts.InputStore.LoadEncryptedFile(bytes)
-	if err == sops.MetadataNotFound {
-		return nil
+func ensureNoMetadata(opts encryptOpts, branch sops.TreeBranch) error {
+	for _, b := range branch {
+		if b.Key == "sops" {
+			return &fileAlreadyEncryptedError{}
+		}
 	}
-	return &fileAlreadyEncryptedError{}
+	return nil
 }
 
 func encrypt(opts encryptOpts) (encryptedFile []byte, err error) {
@@ -57,13 +58,13 @@ func encrypt(opts encryptOpts) (encryptedFile []byte, err error) {
 	if err != nil {
 		return nil, common.NewExitError(fmt.Sprintf("Error reading file: %s", err), codes.CouldNotReadInputFile)
 	}
-	if err := ensureNoMetadata(opts, fileBytes); err != nil {
-		return nil, common.NewExitError(err, codes.FileAlreadyEncrypted)
-	}
 	branch, err := opts.InputStore.LoadPlainFile(fileBytes)
 	if err != nil {
 		return nil, common.NewExitError(fmt.Sprintf("Error unmarshalling file: %s", err), codes.CouldNotReadInputFile)
 	}
+	if err := ensureNoMetadata(opts, branch); err != nil {
+		return nil, common.NewExitError(err, codes.FileAlreadyEncrypted)
+	}
 	path, err := filepath.Abs(opts.InputPath)
 	if err != nil {
 		return nil, err

functional-tests/src/lib.rs

@@ -385,4 +385,28 @@ b: ba"#
         assert_eq!(output.stdout, b"multi\nline");
     }
 
+
+    #[test]
+    fn roundtrip_binary() {
+        let data = b"\"\"{}this_is_binary_data";
+        let file_path = prepare_temp_file("test.binary", data);
+        let output = Command::new(SOPS_BINARY_PATH)
+            .arg("-i")
+            .arg("-e")
+            .arg(file_path.clone())
+            .output()
+            .expect("Error running sops");
+        assert!(output.status.success(),
+                "SOPS failed to encrypt a binary file");
+        let output = Command::new(SOPS_BINARY_PATH)
+            .arg("-d")
+            .arg(file_path.clone())
+            .output()
+            .expect("Error running sops");
+        assert!(output.status
+                    .success(),
+                "SOPS failed to decrypt a binary file");
+        assert_eq!(output.stdout, data);
+    }
+
 }