Only calculate encrypted values if value has changed

Author: marcqualie

lib/diffcrypt/encryptor.rb

@@ -1,3 +1,4 @@
+# rubocop:disable Layout/LineLength
 # frozen_string_literal: true
 
 require 'pathname'
@@ -41,9 +42,11 @@ def decrypt_hash(data)
     end
 
     # @param [String] contents The raw YAML string to be encrypted
-    def encrypt(contents)
+    # @param [String, nil] original_encrypted_contents The original (encrypted) content to determine which keys have changed
+    def encrypt(contents, original_encrypted_contents = nil)
       yaml = YAML.safe_load contents
-      encrypted = encrypt_values yaml
+      original_yaml = original_encrypted_contents ? YAML.safe_load(original_encrypted_contents) : nil
+      encrypted = encrypt_values yaml, original_yaml
       YAML.dump encrypted
     end
 
@@ -53,18 +56,24 @@ def encrypt_string(value)
       @encryptor.encrypt_and_sign value
     end
 
+    # TODO: Fix the complexity of this method
+    # rubocop:disable Metrics/PerceivedComplexity, Metrics/MethodLength, Metrics/CyclomaticComplexity
     # @param [Hash] keys
     # @return [Hash]
-    def encrypt_values(data)
+    def encrypt_values(data, original_data = nil)
       data.each do |key, value|
+        original_encrypted_value = original_data ? original_data[key] : nil
         data[key] = if value.is_a?(Hash) || value.is_a?(Array)
-                      encrypt_values(value)
+                      encrypt_values(value, original_encrypted_value)
                     else
-                      encrypt_string value
+                      original_decrypted_value = original_data ? decrypt_string(original_encrypted_value) : nil
+                      key_changed = original_decrypted_value.nil? || original_decrypted_value != value
+                      key_changed ? encrypt_string(value) : original_encrypted_value
                     end
       end
       data
     end
+    # rubocop:enable Metrics/PerceivedComplexity, Metrics/MethodLength, Metrics/CyclomaticComplexity
 
     # @param [String] value The encrypted value that needs decrypting
     # @return [String]
@@ -73,3 +82,5 @@ def decrypt_string(value)
     end
   end
 end
+
+# rubocop:enable Layout/LineLength

lib/diffcrypt/rails/encrypted_configuration.rb

@@ -39,10 +39,10 @@ def read
         ''
       end
 
-      def write(contents)
+      def write(contents, original_encrypted_contents = nil)
         deserialize(contents)
 
-        IO.binwrite "#{content_path}.tmp", encrypt(contents)
+        IO.binwrite "#{content_path}.tmp", encrypt(contents, original_encrypted_contents)
         FileUtils.mv "#{content_path}.tmp", content_path
       end
 
@@ -62,6 +62,7 @@ def change(&block)
 
       protected
 
+      # rubocop:disable Metrics/AbcSize
       def writing(contents)
         tmp_file = "#{Process.pid}.#{content_path.basename.to_s.chomp('.enc')}"
         tmp_path = Pathname.new File.join(Dir.tmpdir, tmp_file)
@@ -71,15 +72,17 @@ def writing(contents)
 
         updated_contents = tmp_path.binread
 
-        write(updated_contents) if updated_contents != contents
+        write(updated_contents, content_path.binread)
       ensure
         FileUtils.rm(tmp_path) if tmp_path&.exist?
       end
+      # rubocop:enable Metrics/AbcSize
 
-      # @param [String] contents
-      # @return [String]
-      def encrypt(contents)
-        encryptor.encrypt contents
+      # @param [String] contents The new content to be encrypted
+      # @param [String] diff_against The original (encrypted) content to determine which keys have changed
+      # @return [String] Encrypted content to commit
+      def encrypt(contents, original_encrypted_contents = nil)
+        encryptor.encrypt contents, original_encrypted_contents
       end
 
       # @param [String] contents

test/diffcrypt/encryptor_test.rb

@@ -21,13 +21,13 @@ def test_it_decrypts_root_values
   end
 
   def test_it_encrypts_root_values
-    encrypted_content = <<~CONTENT
+    content = <<~CONTENT
       ---
       secret_key_base: secret_key_base_test
     CONTENT
     expected_pattern = /---\nsecret_key_base: #{ENCRYPTED_VALUE_PATTERN}\n/
 
-    assert_match expected_pattern, Diffcrypt::Encryptor.new(TEST_KEY).encrypt(encrypted_content)
+    assert_match expected_pattern, Diffcrypt::Encryptor.new(TEST_KEY).encrypt(content)
   end
 
   def test_it_decrypts_nested_structures
@@ -47,14 +47,24 @@ def test_it_decrypts_nested_structures
   end
 
   def test_it_encrypts_nested_structures
-    encrypted_content = <<~CONTENT
+    content = <<~CONTENT
       ---
       secret_key_base: secret_key_base_test
       aws:
         access_key_id: AKIAXXX
     CONTENT
     expected_pattern = /---\nsecret_key_base: #{ENCRYPTED_VALUE_PATTERN}\naws:\n  access_key_id: #{ENCRYPTED_VALUE_PATTERN}\n/
 
-    assert_match expected_pattern, Diffcrypt::Encryptor.new(TEST_KEY).encrypt(encrypted_content)
+    assert_match expected_pattern, Diffcrypt::Encryptor.new(TEST_KEY).encrypt(content)
+  end
+
+  # Verifies that a change to one key does not cause the encrypted values for other keys to be recomputed
+  # Mainly used in conjunction with rails credentials editor
+  def test_it_only_updates_changed_values
+    original_encrypted_content = "---\nsecret_key_base_1: 88Ry6HESUoXBr6QUFXmni9zzfCIYt9qGNFvIWFcN--4xoecI5mqbNRBibI--62qPJbkzzh5h8lhFEFOSaQ==\naws:\n  secret_access_key: 88Ry6HESUoXBr6QUFXmni9zzfCIYt9qGNFvIWFcN--4xoecI5mqbNRBibI--62qPJbkzzh5h8lhFEFOSaQ==\n"
+    updated_content = "---\nsecret_key_base_1: secret_key_base_test\naws:\n  secret_access_key: secret_access_key_2"
+    expected_pattern = /---\nsecret_key_base_1: 88Ry6HESUoXBr6QUFXmni9zzfCIYt9qGNFvIWFcN--4xoecI5mqbNRBibI--62qPJbkzzh5h8lhFEFOSaQ==\naws:\n  secret_access_key: #{ENCRYPTED_VALUE_PATTERN}\n/
+
+    assert_match expected_pattern, Diffcrypt::Encryptor.new(TEST_KEY).encrypt(updated_content, original_encrypted_content)
   end
 end