Revise credentials diffing flow to use a separate diff command

Didn't like the complicated stuff that happened on credentials:edit. It
would append to .gitattributes multiple times. Though I see why it was
written that way.

I'm cutting off for now, but since this new flow would require each developer
to run --enable perhaps this should really be:

1. Developer enrolls Rails app by running `credentials:diff --enable`
2. credentials:edit checks .gitattributes for `diff=rails_credentials` and
   if the current file is covered by that.
3. If so, set up the "rails_credentials" driver automatically.

railties/lib/rails/commands/credentials/USAGE

@@ -30,6 +30,20 @@ You could prepend that to your server's start command like this:
 
    RAILS_MASTER_KEY="very-secret-and-secure" server.start
 
+=== Set up Git to Diff Credentials
+
+Rails provides `rails credentials:diff --enable` to instruct Git to call `rails credentials:diff`
+when `git diff` is run on a credentials file.
+
+Any credentials files are set to use the "rails_credentials" diff driver in .gitattributes.
+Since Git requires the diff driver to be set up in a config file, the command uses
+the project local .git/config. Since that config isn't stored in Git each team member
+must enable separately.
+
+Or set up the "rails_credentials" diff driver globally with:
+
+   git config --global diff.rails_credentials.textconv "bin/rails credentials:diff"
+
 === Editing Credentials
 
 This will open a temporary file in `$EDITOR` with the decrypted contents to edit

railties/lib/rails/commands/credentials/credentials_command.rb

@@ -38,7 +38,6 @@ def edit
         end
 
         say "File encrypted and saved."
-        enable_credentials_diffing
       rescue ActiveSupport::MessageEncryptor::InvalidMessage
         say "Couldn't decrypt #{content_path}. Perhaps you passed the wrong key?"
       end
@@ -50,14 +49,20 @@ def show
         say credentials.read.presence || missing_credentials_message
       end
 
-      def diff(content_path)
-        @content_path = content_path
+      option :enable, type: :boolean, default: false,
+        desc: "Pass `--enable` to make credential files diffable with `git diff`"
 
-        extract_environment_option_from_argument(default_environment: extract_environment_from_path(content_path))
-        require_application!
+      def diff(content_path = nil)
+        if @content_path = content_path
+          extract_environment_option_from_argument(default_environment: extract_environment_from_path(content_path))
+          require_application!
 
-        say credentials.read.presence || credentials.content_path.read
-      rescue
+          say credentials.read.presence || credentials.content_path.read
+        else
+          require_application!
+          enable_diffing if options[:enable]
+        end
+      rescue ActiveSupport::MessageEncryptor::InvalidMessage
         say credentials.content_path.read
       end
 

railties/lib/rails/commands/credentials/credentials_command/diffing.rb

@@ -3,45 +3,28 @@
 module Rails::Command::CredentialsCommand::Diffing # :nodoc:
   class Error < StandardError; end
 
-  def enable_credentials_diffing
-    unless already_answered? || enabled?
-      answer = yes?("Would you like to make the credentials diff from git more readable in the future? [Y/n]")
+  def enable_diffing
+    if enabled?
+      say "Already enabled!"
+    else
+      enable
+      say "Diffing enabled! Editing a credentials file will display a diff of what actually changed."
     end
-
-    enable if answer
-    FileUtils.touch(tracker) unless answer.nil?
   rescue Error
-    say "Couldn't setup git to enable credentials diffing"
+    say "Couldn't setup Git to enable credentials diffing."
   end
 
   private
-    def already_answered?
-      tracker.exist?
-    end
-
     def enabled?
-      system_call("git config --get 'diff.rails_credentials.textconv'", accepted_codes: [0, 1])
+      system "git config --get diff.rails_credentials.textconv", out: File::NULL
     end
 
     def enable
-      system_call("git config diff.rails_credentials.textconv 'bin/rails credentials:diff'", accepted_codes: [0])
-
-      git_attributes = Rails.root.join(".gitattributes")
-      File.open(git_attributes, "a+") do |file|
-        file.write(<<~EOM)
-          config/credentials/*.yml.enc diff=rails_credentials
-          config/credentials.yml.enc diff=rails_credentials
-        EOM
-      end
-    end
-
-    def tracker
-      Rails.root.join("tmp", "rails_pretty_credentials")
-    end
+      raise Error unless system("git config diff.rails_credentials.textconv 'bin/rails credentials:diff'")
 
-    def system_call(command_line, accepted_codes:)
-      result = system(command_line)
-      raise(Error) if accepted_codes.exclude?($?.exitstatus)
-      result
+      Rails.root.join(".gitattributes").write(<<~end_of_template, mode: "a")
+        config/credentials/*.yml.enc diff=rails_credentials
+        config/credentials.yml.enc diff=rails_credentials
+      end_of_template
     end
 end

railties/test/commands/credentials_test.rb

@@ -89,62 +89,60 @@ class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase
     assert_match(/secret_key_base/, output)
   end
 
-  test "edit ask the user to opt in to pretty credentials" do
-    assert_match(/Would you like to make the credentials diff from git/, run_edit_command)
+
+  test "show credentials" do
+    assert_match(/access_key_id: 123/, run_show_command)
   end
 
-  test "edit doesn't ask the user to opt in to pretty credentials when alreasy asked" do
-    app_file("tmp/rails_pretty_credentials", "")
+  test "show command raises error when require_master_key is specified and key does not exist" do
+    remove_file "config/master.key"
+    add_to_config "config.require_master_key = true"
 
-    assert_no_match(/Would you like to make the credentials diff from git/, run_edit_command)
+    assert_match(/Missing encryption key to decrypt file with/, run_show_command(allow_failure: true))
   end
 
-  test "edit doesn't ask the user to opt in when user already opted in" do
-    content = <<~EOM
-      [diff "rails_credentials"]
-          textconv = bin/rails credentials:show
-    EOM
-    app_file(".git/config", content)
+  test "show command does not raise error when require_master_key is false and master key does not exist" do
+    remove_file "config/master.key"
+    add_to_config "config.require_master_key = false"
 
-    assert_no_match(/Would you like to make the credentials diff from git/, run_edit_command)
+    assert_match(/Missing 'config\/master\.key' to decrypt credentials/, run_show_command)
   end
 
-  test "edit ask the user to opt in to pretty credentials, user accepts" do
-    file = Tempfile.open("credentials_test")
-    file.write("y")
-    file.rewind
+  test "show command displays content specified by environment option" do
+    run_edit_command(environment: "production")
 
-    run_edit_command(stdin: file.path)
+    assert_match(/access_key_id: 123/, run_show_command(environment: "production"))
+  end
+
+  test "show command properly expands environment option" do
+    run_edit_command(environment: "production")
 
-    git_attributes = app_path(".gitattributes")
-    expected = <<~EOM
+    output = run_show_command(environment: "prod")
+    assert_match(/access_key_id: 123/, output)
+    assert_no_match(/secret_key_base/, output)
+  end
+
+
+  test "diff enable diffing" do
+    run_diff_command(enable: true)
+
+    assert_equal <<~EOM, File.read(app_path(".gitattributes"))
       config/credentials/*.yml.enc diff=rails_credentials
       config/credentials.yml.enc diff=rails_credentials
     EOM
-    assert(File.exist?(git_attributes))
-    assert_equal(expected, File.read(git_attributes))
+
     Dir.chdir(app_path) do
       assert_equal "bin/rails credentials:diff", `git config --get 'diff.rails_credentials.textconv'`.strip
     end
-  ensure
-    file.close!
   end
 
-  test "edit ask the user to opt in to pretty credentials, user refuses" do
-    file = Tempfile.open("credentials_test")
-    file.write("n")
-    file.rewind
-
-    run_edit_command(stdin: file.path)
-
-    git_attributes = app_path(".gitattributes")
-    assert_not(File.exist?(git_attributes))
-  ensure
-    file.close!
-  end
+  test "diff won't enable again if already enabled" do
+    app_file(".git/config", <<~EOM)
+      [diff "rails_credentials"]
+          textconv = bin/rails credentials:diff
+    EOM
 
-  test "show credentials" do
-    assert_match(/access_key_id: 123/, run_show_command)
+    assert_no_match(/Diffing enabled/, run_diff_command(enable: true))
   end
 
   test "diff from git diff left file" do
@@ -184,33 +182,6 @@ class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase
     assert_match(encrypted_content, run_diff_command(content_path))
   end
 
-  test "show command raises error when require_master_key is specified and key does not exist" do
-    remove_file "config/master.key"
-    add_to_config "config.require_master_key = true"
-
-    assert_match(/Missing encryption key to decrypt file with/, run_show_command(allow_failure: true))
-  end
-
-  test "show command does not raise error when require_master_key is false and master key does not exist" do
-    remove_file "config/master.key"
-    add_to_config "config.require_master_key = false"
-
-    assert_match(/Missing 'config\/master\.key' to decrypt credentials/, run_show_command)
-  end
-
-  test "show command displays content specified by environment option" do
-    run_edit_command(environment: "production")
-
-    assert_match(/access_key_id: 123/, run_show_command(environment: "production"))
-  end
-
-  test "show command properly expands environment option" do
-    run_edit_command(environment: "production")
-
-    output = run_show_command(environment: "prod")
-    assert_match(/access_key_id: 123/, output)
-    assert_no_match(/secret_key_base/, output)
-  end
 
   private
     def run_edit_command(editor: "cat", environment: nil, **options)
@@ -225,7 +196,8 @@ def run_show_command(environment: nil, **options)
       rails "credentials:show", args, **options
     end
 
-    def run_diff_command(path, **options)
-      rails "credentials:diff", path, **options
+    def run_diff_command(path = nil, enable: nil, **options)
+      args = enable ? ["--enable"] : [path]
+      rails "credentials:diff", args, **options
     end
 end

railties/test/isolation/abstract_unit.rb

@@ -301,7 +301,7 @@ def engine(name)
     # stderr:: true to pass STDERR output straight to the "real" STDERR.
     #   By default, the STDERR and STDOUT of the process will be
     #   combined in the returned string.
-    def rails(*args, allow_failure: false, stderr: false, stdin: File::NULL)
+    def rails(*args, allow_failure: false, stderr: false)
       args = args.flatten
       fork = true
 
@@ -328,7 +328,7 @@ def rails(*args, allow_failure: false, stderr: false, stdin: File::NULL)
           out_read.close
           err_read.close if err_read
 
-          $stdin.reopen(stdin, "r")
+          $stdin.reopen(File::NULL, "r")
           $stdout.reopen(out_write)
           $stderr.reopen(err_write)