Remember all previously approved configurations and hooks

CHANGELOG.md

@@ -2,6 +2,8 @@
 
 ## 0.75.0
 
+* Remember all locally approved configuration and hook signatures across rebases
+
 * Gracefully handle hook loading on systems with non-UTF8 locale
 * Re-enable running test suite on Windows environments
 

README.md

@@ -721,22 +721,37 @@ that executes some malicious code. When you fetch and checkout this pull
 request, the `post-checkout` hook will be run on your machine, along with
 the malicious code that you just checked out.
 
-Overcommit attempts to address this problem by storing a signature of your
-configuration and all hook plugin code since the last time it ran. When the
-signature changes, a warning is displayed alerting you to which plugins have
-changed. It is then up to you to manually verify that the changes are not
-malicious, and then continue running the hooks.
-
-The signature is derived from the contents of the plugin's source code itself
-and any configuration for the plugin. Thus a change to the plugin's source
-code or your local repo's `.overcommit.yml` file could result in a signature
-change.
-
-Signatures are stored in your local repo's git configuration. If you use
-multiple [worktrees](https://git-scm.com/docs/git-worktree) whose
-configurations differ, enable `extensions.worktreeConfig` (`git config
-extensions.worktreeConfig true`) so that Overcommit stores signatures per
-worktree and signing in one worktree does not invalidate the others.
+Overcommit stores signatures of configurations and hook plugin code you have
+explicitly approved with `overcommit --sign` and `overcommit --sign <hook-type>`.
+Unrecognized signatures trigger a warning so you can verify the changes before
+running hooks. All previously approved signatures are remembered, so checking
+out an older revision or rebasing through approved configurations does not
+require signing them again.
+
+A plugin's signature covers its source code together with its configuration.
+Approving two versions does not approve other combinations of their code and
+configuration.
+
+Approvals are stored under `overcommit-signatures` in your repository's Git
+directory, outside Git's object database. They cannot be committed, pushed, or
+fetched, and Git garbage collection does not remove them. Each approval is a
+small file at `<SHA256 of signature config key>/<SHA256 signature>`, so lookups
+do not scan the history. The file contains the original config key (such as
+`overcommit.configuration.signature` or `overcommit.PreCommit.MyHook.signature`)
+and the approved signature, one per line. There is no history size limit.
+
+To revoke one approval, delete its file. To revoke all approvals, delete the
+`overcommit-signatures` directory. Run `overcommit --sign` and
+`overcommit --sign <hook-type>` to approve the current versions again. Existing
+signatures in local Git config migrate automatically on first use; their old
+keys are removed so deleting approval files also revokes migrated signatures.
+
+By default, linked [worktrees](https://git-scm.com/docs/git-worktree) share
+history in their common Git directory. Enable `extensions.worktreeConfig`
+(`git config extensions.worktreeConfig true`) to keep approvals and the
+`verify_signatures` setting separate in each worktree's Git directory. Use
+`git rev-parse --git-common-dir` to find shared storage, or
+`git rev-parse --git-path overcommit-signatures` to find per-worktree storage.
 
 ### Disabling Signature Checking
 

lib/overcommit.rb

@@ -24,3 +24,4 @@
 require 'overcommit/installer'
 require 'overcommit/logger'
 require 'overcommit/version'
+require 'overcommit/signature'

lib/overcommit/configuration.rb

@@ -190,20 +190,19 @@ def plugin_hook?(hook_context_or_type, hook_name)
       File.exist?(File.join(plugin_directory, hook_type_name, "#{hook_name}.rb"))
     end
 
-    # Return whether the signature for this configuration has changed since it
-    # was last calculated.
+    # Return whether this configuration has been explicitly trusted before.
     #
     # @return [true,false]
-    def signature_changed?
-      signature != stored_signature
+    def signature_verified?
+      signature_store.include?(signature)
     end
 
     # Return whether a previous signature has been recorded for this
     # configuration.
     #
     # @return [true,false]
     def previous_signature?
-      !stored_signature.empty?
+      signature_store.recorded?
     end
 
     # Returns whether this configuration should verify itself by checking the
@@ -233,13 +232,10 @@ def verify_signatures?
 
     # Update the currently stored signature for this hook.
     def update_signature!
+      signature_store.add(signature)
       scope = Overcommit::GitConfig.signature_scope
-      result = Overcommit::Utils.execute(
-        %W[git config #{scope}] + [signature_config_key, signature]
-      )
-
       verify_signature_value = @hash['verify_signatures'] ? 1 : 0
-      result &&= Overcommit::Utils.execute(
+      result = Overcommit::Utils.execute(
         %W[git config #{scope} #{verify_signature_config_key} #{verify_signature_value}]
       )
 
@@ -320,26 +316,8 @@ def signature
       Digest::SHA256.hexdigest(@hash.to_json)
     end
 
-    # Returns the stored signature of this repo's Overcommit configuration.
-    #
-    # This is intended to be compared against the current signature of this
-    # configuration object.
-    #
-    # @return [String]
-    def stored_signature
-      scope = Overcommit::GitConfig.signature_scope
-      result = Overcommit::Utils.execute(
-        %W[git config #{scope} --get] + [signature_config_key]
-      )
-
-      if result.status == 1 # Key doesn't exist
-        return ''
-      elsif result.status != 0
-        raise Overcommit::Exceptions::GitConfigError,
-              "Unable to read from local repo git config: #{result.stderr}"
-      end
-
-      result.stdout.chomp
+    def signature_store
+      @signature_store ||= Overcommit::Signature.new(signature_config_key)
     end
 
     def signature_config_key

lib/overcommit/configuration_loader.rb

@@ -95,7 +95,7 @@ def verify_signatures(config)
               "No previously recorded signature for configuration file.\n" \
               'Run `overcommit --sign` if you trust the hooks in this repository.'
 
-      elsif config.signature_changed?
+      elsif !config.signature_verified?
         raise Overcommit::Exceptions::ConfigurationSignatureChanged,
               "Signature of configuration file has changed!\n" \
               "Run `overcommit --sign` once you've verified the configuration changes."

lib/overcommit/exceptions.rb

@@ -13,6 +13,9 @@ class ConfigurationSignatureChanged < Error; end
   # Raised when trying to read/write to/from the local repo git config fails.
   class GitConfigError < Error; end
 
+  # Raised when local signature history cannot be read or written.
+  class SignatureError < Error; end
+
   # Raised when there was a problem reading submodule information for a repo.
   class GitSubmoduleError < Error; end
 

lib/overcommit/hook_loader/plugin_hook_loader.rb

@@ -7,7 +7,7 @@ module Overcommit::HookLoader
   # is running in.
   class PluginHookLoader < Base
     def load_hooks
-      check_for_modified_plugins if @config.verify_signatures?
+      check_for_unverified_plugins if @config.verify_signatures?
 
       hooks = plugin_paths.map do |plugin_path|
         require plugin_path
@@ -22,9 +22,9 @@ def load_hooks
     end
 
     def update_signatures
-      log.success('No plugin signatures have changed') if modified_plugins.empty?
+      log.success('All plugin signatures have been verified') if unverified_plugins.empty?
 
-      modified_plugins.each do |plugin|
+      unverified_plugins.each do |plugin|
         plugin.update_signature!
         log.warning "Updated signature of plugin #{plugin.hook_name}"
       end
@@ -47,20 +47,20 @@ def ad_hoc_hook_names
       @config.enabled_ad_hoc_hooks(@context)
     end
 
-    def modified_plugins
+    def unverified_plugins
       (plugin_hook_names + ad_hoc_hook_names).
         map { |hook_name| Overcommit::HookSigner.new(hook_name, @config, @context) }.
-        select(&:signature_changed?)
+        reject(&:signature_verified?)
     end
 
-    def check_for_modified_plugins
-      return if modified_plugins.empty?
+    def check_for_unverified_plugins
+      return if unverified_plugins.empty?
 
       log.bold_warning "The following #{@context.hook_script_name} plugins " \
                        'have been added, changed, or had their configuration modified:'
       log.newline
 
-      modified_plugins.each do |signer|
+      unverified_plugins.each do |signer|
         log.warning " * #{signer.hook_name} in #{signer.hook_path}"
       end
 

lib/overcommit/hook_signer.rb

@@ -58,25 +58,17 @@ def signable_file?(file)
         file.start_with?(Overcommit::Utils.repo_root)
     end
 
-    # Return whether the signature for this hook has changed since it was last
-    # calculated.
+    # Return whether this exact combination of hook code and configuration
+    # has been explicitly trusted before.
     #
     # @return [true,false]
-    def signature_changed?
-      signature != stored_signature
+    def signature_verified?
+      signature_store.include?(signature)
     end
 
-    # Update the current stored signature for this hook.
+    # Remember the current signature without forgetting older approvals.
     def update_signature!
-      scope = Overcommit::GitConfig.signature_scope
-      result = Overcommit::Utils.execute(
-        %W[git config #{scope}] + [signature_config_key, signature]
-      )
-
-      unless result.success?
-        raise Overcommit::Exceptions::GitConfigError,
-              "Unable to write to local repo git config: #{result.stderr}"
-      end
+      signature_store.add(signature)
     end
 
     private
@@ -103,20 +95,8 @@ def hook_contents
       File.read(hook_path)
     end
 
-    def stored_signature
-      scope = Overcommit::GitConfig.signature_scope
-      result = Overcommit::Utils.execute(
-        %W[git config #{scope} --get] + [signature_config_key]
-      )
-
-      if result.status == 1 # Key doesn't exist
-        return ''
-      elsif result.status != 0
-        raise Overcommit::Exceptions::GitConfigError,
-              "Unable to read from local repo git config: #{result.stderr}"
-      end
-
-      result.stdout.chomp
+    def signature_store
+      @signature_store ||= Overcommit::Signature.new(signature_config_key)
     end
 
     def signature_config_key

lib/overcommit/signature.rb

@@ -0,0 +1,71 @@
+# frozen_string_literal: true
+
+require 'digest'
+require 'fileutils'
+
+module Overcommit
+  # Repository-local approvals, indexed by hook identity and SHA256 signature.
+  # These files are outside Git's object database so fetching cannot add trust.
+  class Signature
+    def initialize(config_key)
+      @config_key = config_key
+      scope = Overcommit::GitConfig.signature_scope
+      root = if scope == '--worktree'
+               Overcommit::Utils.git_path('overcommit-signatures')
+             else
+               File.join(Overcommit::Utils.git_dir, 'overcommit-signatures')
+             end
+      @directory = File.join(root, Digest::SHA256.hexdigest(config_key))
+      migrate_legacy_signature(scope)
+    end
+
+    def include?(signature)
+      File.read(path(signature)) == contents(signature)
+    rescue Errno::ENOENT
+      false
+    rescue SystemCallError => e
+      raise Overcommit::Exceptions::SignatureError, "Unable to read signature history: #{e}"
+    end
+
+    def recorded?
+      File.directory?(@directory)
+    end
+
+    def add(signature)
+      FileUtils.mkdir_p(@directory)
+      File.write(path(signature), contents(signature), mode: 'w', perm: 0o600)
+    rescue SystemCallError => e
+      raise Overcommit::Exceptions::SignatureError, "Unable to write signature history: #{e}"
+    end
+
+    private
+
+    def path(signature)
+      File.join(@directory, signature)
+    end
+
+    def contents(signature)
+      "#{@config_key}\n#{signature}\n"
+    end
+
+    # Preserve an existing approval before removing its legacy Git config key.
+    # Removing the key means deleting history files actually revokes trust.
+    def migrate_legacy_signature(scope)
+      result = Overcommit::Utils.execute(%W[git config #{scope} --get] + [@config_key])
+      return if result.status == 1
+
+      check_config_result(result)
+      signature = result.stdout.chomp
+      add(signature) if signature.match?(/\A[0-9a-f]{64}\z/)
+      result = Overcommit::Utils.execute(%W[git config #{scope} --unset-all] + [@config_key])
+      check_config_result(result) unless result.status == 5 # Already removed by another process.
+    end
+
+    def check_config_result(result)
+      return if result.success?
+
+      raise Overcommit::Exceptions::GitConfigError,
+            "Unable to migrate local repo git config signature: #{result.stderr}"
+    end
+  end
+end