Repository: pre-commit/pre-commit (python)

Issue:
Integrity check
Hello,

First of all, thank you for this project. I really appreciate using it.

This issue is a feature request to add an integrity check for security reasons. In fact, even if there is the possibility to specify a Git tag to download a particular version of a pre-commit hook, it does not guaranty that the tag will point to the same commit between two downloads. Knowing that a Git hook is an executable, and that a Git tag can be modified to point to another commit, it becomes impossible to trust the hooks downloaded by pre-commit without reviewing them each time they are installed, which is a security risk. An attacker getting write access to a Git repository containing pre-commit hooks could add a harmful payload to them and make the latest tag point to this new malicious version for example.

To remediate this situation, I would suggest to add the commit ID as a new `repos` property to the configuration file, at the same imbrication-level as `rev`. This way, pre-commit will be able to verify that the Git tag specified by `rev` points to the expected Git commit.

I look forward to getting your feedback on it.

Cheers.

Pull request:
Implement `pre-commit autoupdate --freeze`
Resolves #1068

Patch:
diff --git a/pre_commit/clientlib.py b/pre_commit/clientlib.py
index c4768ff..74a37a8 100644
--- a/pre_commit/clientlib.py
+++ b/pre_commit/clientlib.py
@@ -133,8 +133,7 @@ class MigrateShaToRev(object):
         if 'sha' in dct:
             dct['rev'] = dct.pop('sha')
 
-    def remove_default(self, dct):
-        pass
+    remove_default = cfgv.Required.remove_default
 
 
 def _entry(modname):
diff --git a/pre_commit/commands/autoupdate.py b/pre_commit/commands/autoupdate.py
index d56a88f..eea5be7 100644
--- a/pre_commit/commands/autoupdate.py
+++ b/pre_commit/commands/autoupdate.py
@@ -1,18 +1,17 @@
 from __future__ import print_function
 from __future__ import unicode_literals
 
+import collections
 import os.path
 import re
 
 import six
 from aspy.yaml import ordered_dump
 from aspy.yaml import ordered_load
-from cfgv import remove_defaults
 
 import pre_commit.constants as C
 from pre_commit import git
 from pre_commit import output
-from pre_commit.clientlib import CONFIG_SCHEMA
 from pre_commit.clientlib import InvalidManifestError
 from pre_commit.clientlib import load_config
 from pre_commit.clientlib import load_manifest
@@ -25,39 +24,44 @@ from pre_commit.util import cmd_output_b
 from pre_commit.util import tmpdir
 
 
-class RepositoryCannotBeUpdatedError(RuntimeError):
-    pass
-
+class RevInfo(collections.namedtuple('RevInfo', ('repo', 'rev', 'frozen'))):
+    __slots__ = ()
 
-def _update_repo(repo_config, store, tags_only):
-    """Updates a repository to the tip of `master`.  If the repository cannot
-    be updated because a hook that is configured does not exist in `master`,
-    this raises a RepositoryCannotBeUpdatedError
+    @classmethod
+    def from_config(cls, config):
+        return cls(config['repo'], config['rev'], None)
 
-    Args:
-        repo_config - A config for a repository
-    """
-    with tmpdir() as repo_path:
-        git.init_repo(repo_path, repo_config['repo'])
-        cmd_output_b('git', 'fetch', 'origin', 'HEAD', '--tags', cwd=repo_path)
-
-        tag_cmd = ('git', 'describe', 'FETCH_HEAD', '--tags')
+    def update(self, tags_only, freeze):
         if tags_only:
-            tag_cmd += ('--abbrev=0',)
+            tag_cmd = ('git', 'describe', 'FETCH_HEAD', '--tags', '--abbrev=0')
         else:
-            tag_cmd += ('--exact',)
-        try:
-            rev = cmd_output(*tag_cmd, cwd=repo_path)[1].strip()
-        except CalledProcessError:
-            tag_cmd = ('git', 'rev-parse', 'FETCH_HEAD')
-            rev = cmd_output(*tag_cmd, cwd=repo_path)[1].strip()
+            tag_cmd = ('git', 'describe', 'FETCH_HEAD', '--tags', '--exact')
+
+        with tmpdir() as tmp:
+            git.init_repo(tmp, self.repo)
+            cmd_output_b('git', 'fetch', 'origin', 'HEAD', '--tags', cwd=tmp)
+
+            try:
+                rev = cmd_output(*tag_cmd, cwd=tmp)[1].strip()
+            except CalledProcessError:
+                cmd = ('git', 'rev-parse', 'FETCH_HEAD')
+                rev = cmd_output(*cmd, cwd=tmp)[1].strip()
+
+            frozen = None
+            if freeze:
+                exact = cmd_output('git', 'rev-parse', rev, cwd=tmp)[1].strip()
+                if exact != rev:
+                    rev, frozen = exact, rev
+        return self._replace(rev=rev, frozen=frozen)
+
+
+class RepositoryCannotBeUpdatedError(RuntimeError):
+    pass
 
-    # Don't bother trying to update if our rev is the same
-    if rev == repo_config['rev']:
-        return repo_config
 
+def _check_hooks_still_exist_at_rev(repo_config, info, store):
     try:
-        path = store.clone(repo_config['repo'], rev)
+        path = store.clone(repo_config['repo'], info.rev)
         manifest = load_manifest(os.path.join(path, C.MANIFEST_FILE))
     except InvalidManifestError as e:
         raise RepositoryCannotBeUpdatedError(six.text_type(e))
@@ -71,94 +75,91 @@ def _update_repo(repo_config, store, tags_only):
             '{}'.format(', '.join(sorted(hooks_missing))),
         )
 
-    # Construct a new config with the head rev
-    new_config = repo_config.copy()
-    new_config['rev'] = rev
-    return new_config
-
 
-REV_LINE_RE = re.compile(r'^(\s+)rev:(\s*)([^\s#]+)(.*)$', re.DOTALL)
-REV_LINE_FMT = '{}rev:{}{}{}'
+REV_LINE_RE = re.compile(r'^(\s+)rev:(\s*)([^\s#]+)(.*)(\r?\n)$', re.DOTALL)
+REV_LINE_FMT = '{}rev:{}{}{}{}'
 
 
-def _write_new_config_file(path, output):
+def _original_lines(path, rev_infos, retry=False):
+    """detect `rev:` lines or reformat the file"""
     with open(path) as f:
-        original_contents = f.read()
-    output = remove_defaults(output, CONFIG_SCHEMA)
-    new_contents = ordered_dump(output, **C.YAML_DUMP_KWARGS)
-
-    lines = original_contents.splitlines(True)
-    rev_line_indices_reversed = list(
-        reversed([
-            i for i, line in enumerate(lines) if REV_LINE_RE.match(line)
-        ]),
-    )
-
-    for line in new_contents.splitlines(True):
-        if REV_LINE_RE.match(line):
-            # It's possible we didn't identify the rev lines in the original
-            if not rev_line_indices_reversed:
-                break
-            line_index = rev_line_indices_reversed.pop()
-            original_line = lines[line_index]
-            orig_match = REV_LINE_RE.match(original_line)
-            new_match = REV_LINE_RE.match(line)
-            lines[line_index] = REV_LINE_FMT.format(
-                orig_match.group(1), orig_match.group(2),
-                new_match.group(3), orig_match.group(4),
-            )
-
-    # If we failed to intelligently rewrite the rev lines, fall back to the
-    # pretty-formatted yaml output
-    to_write = ''.join(lines)
-    if remove_defaults(ordered_load(to_write), CONFIG_SCHEMA) != output:
-        to_write = new_contents
+        original = f.read()
+
+    lines = original.splitlines(True)
+    idxs = [i for i, line in enumerate(lines) if REV_LINE_RE.match(line)]
+    if len(idxs) == len(rev_infos):
+        return lines, idxs
+    elif retry:
+        raise AssertionError('could not find rev lines')
+    else:
+        with open(path, 'w') as f:
+            f.write(ordered_dump(ordered_load(original), **C.YAML_DUMP_KWARGS))
+        return _original_lines(path, rev_infos, retry=True)
+
+
+def _write_new_config(path, rev_infos):
+    lines, idxs = _original_lines(path, rev_infos)
+
+    for idx, rev_info in zip(idxs, rev_infos):
+        if rev_info is None:
+            continue
+        match = REV_LINE_RE.match(lines[idx])
+        assert match is not None
+        new_rev_s = ordered_dump({'rev': rev_info.rev}, **C.YAML_DUMP_KWARGS)
+        new_rev = new_rev_s.split(':', 1)[1].strip()
+        if rev_info.frozen is not None:
+            comment = '  # {}'.format(rev_info.frozen)
+        else:
+            comment = match.group(4)
+        lines[idx] = REV_LINE_FMT.format(
+            match.group(1), match.group(2), new_rev, comment, match.group(5),
+        )
 
     with open(path, 'w') as f:
-        f.write(to_write)
+        f.write(''.join(lines))
 
 
-def autoupdate(config_file, store, tags_only, repos=()):
+def autoupdate(config_file, store, tags_only, freeze, repos=()):
     """Auto-update the pre-commit config to the latest versions of repos."""
     migrate_config(config_file, quiet=True)
     retv = 0
-    output_repos = []
+    rev_infos = []
     changed = False
 
-    input_config = load_config(config_file)
+    config = load_config(config_file)
+    for repo_config in config['repos']:
+        if repo_config['repo'] in {LOCAL, META}:
+            continue
 
-    for repo_config in input_config['repos']:
-        if (
-            repo_config['repo'] in {LOCAL, META} or
-            # Skip updating any repo_configs that aren't for the specified repo
-            repos and repo_config['repo'] not in repos
-        ):
-            output_repos.append(repo_config)
+        info = RevInfo.from_config(repo_config)
+        if repos and info.repo not in repos:
+            rev_infos.append(None)
             continue
-        output.write('Updating {}...'.format(repo_config['repo']))
+
+        output.write('Updating {}...'.format(info.repo))
+        new_info = info.update(tags_only=tags_only, freeze=freeze)
         try:
-            new_repo_config = _update_repo(repo_config, store, tags_only)
+            _check_hooks_still_exist_at_rev(repo_config, new_info, store)
         except RepositoryCannotBeUpdatedError as error:
             output.write_line(error.args[0])
-            output_repos.append(repo_config)
+            rev_infos.append(None)
             retv = 1
             continue
 
-        if new_repo_config['rev'] != repo_config['rev']:
+        if new_info.rev != info.rev:
             changed = True
-            output.write_line(
-                'updating {} -> {}.'.format(
-                    repo_config['rev'], new_repo_config['rev'],
-                ),
-            )
-            output_repos.append(new_repo_config)
+            if new_info.frozen:
+                updated_to = '{} (frozen)'.format(new_info.frozen)
+            else:
+                updated_to = new_info.rev
+            msg = 'updating {} -> {}.'.format(info.rev, updated_to)
+            output.write_line(msg)
+            rev_infos.append(new_info)
         else:
             output.write_line('already up to date.')
-            output_repos.append(repo_config)
+            rev_infos.append(None)
 
     if changed:
-        output_config = input_config.copy()
-        output_config['repos'] = output_repos
-        _write_new_config_file(config_file, output_config)
+        _write_new_config(config_file, rev_infos)
 
     return retv
diff --git a/pre_commit/main.py b/pre_commit/main.py
index fe1beaf..8fd130f 100644
--- a/pre_commit/main.py
+++ b/pre_commit/main.py
@@ -175,6 +175,10 @@ def main(argv=None):
             'tagged version (the default behavior).'
         ),
     )
+    autoupdate_parser.add_argument(
+        '--freeze', action='store_true',
+        help='Store "frozen" hashes in `rev` instead of tag names',
+    )
     autoupdate_parser.add_argument(
         '--repo', dest='repos', action='append', metavar='REPO',
         help='Only update this repository -- may be specified multiple times.',
@@ -313,6 +317,7 @@ def main(argv=None):
             return autoupdate(
                 args.config, store,
                 tags_only=not args.bleeding_edge,
+                freeze=args.freeze,
                 repos=args.repos,
             )
         elif args.command == 'clean':


Test patch:
diff --git a/tests/commands/autoupdate_test.py b/tests/commands/autoupdate_test.py
index ead0efe..9a72558 100644
--- a/tests/commands/autoupdate_test.py
+++ b/tests/commands/autoupdate_test.py
@@ -6,9 +6,10 @@ import pytest
 
 import pre_commit.constants as C
 from pre_commit import git
-from pre_commit.commands.autoupdate import _update_repo
+from pre_commit.commands.autoupdate import _check_hooks_still_exist_at_rev
 from pre_commit.commands.autoupdate import autoupdate
 from pre_commit.commands.autoupdate import RepositoryCannotBeUpdatedError
+from pre_commit.commands.autoupdate import RevInfo
 from pre_commit.util import cmd_output
 from testing.auto_namedtuple import auto_namedtuple
 from testing.fixtures import add_config_to_repo
@@ -22,30 +23,114 @@ from testing.util import git_commit
 
 
 @pytest.fixture
-def up_to_date_repo(tempdir_factory):
+def up_to_date(tempdir_factory):
     yield make_repo(tempdir_factory, 'python_hooks_repo')
 
 
-def test_up_to_date_repo(up_to_date_repo, store):
-    config = make_config_from_repo(up_to_date_repo)
-    input_rev = config['rev']
-    ret = _update_repo(config, store, tags_only=False)
-    assert ret['rev'] == input_rev
+@pytest.fixture
+def out_of_date(tempdir_factory):
+    path = make_repo(tempdir_factory, 'python_hooks_repo')
+    original_rev = git.head_rev(path)
 
+    git_commit(cwd=path)
+    head_rev = git.head_rev(path)
 
-def test_autoupdate_up_to_date_repo(up_to_date_repo, in_tmpdir, store):
-    # Write out the config
-    config = make_config_from_repo(up_to_date_repo, check=False)
-    write_config('.', config)
+    yield auto_namedtuple(
+        path=path, original_rev=original_rev, head_rev=head_rev,
+    )
 
-    with open(C.CONFIG_FILE) as f:
-        before = f.read()
-    assert '^$' not in before
-    ret = autoupdate(C.CONFIG_FILE, store, tags_only=False)
-    with open(C.CONFIG_FILE) as f:
-        after = f.read()
-    assert ret == 0
-    assert before == after
+
+@pytest.fixture
+def tagged(out_of_date):
+    cmd_output('git', 'tag', 'v1.2.3', cwd=out_of_date.path)
+    yield out_of_date
+
+
+@pytest.fixture
+def hook_disappearing(tempdir_factory):
+    path = make_repo(tempdir_factory, 'python_hooks_repo')
+    original_rev = git.head_rev(path)
+
+    with modify_manifest(path) as manifest:
+        manifest[0]['id'] = 'bar'
+
+    yield auto_namedtuple(path=path, original_rev=original_rev)
+
+
+def test_rev_info_from_config():
+    info = RevInfo.from_config({'repo': 'repo/path', 'rev': 'v1.2.3'})
+    assert info == RevInfo('repo/path', 'v1.2.3', None)
+
+
+def test_rev_info_update_up_to_date_repo(up_to_date):
+    config = make_config_from_repo(up_to_date)
+    info = RevInfo.from_config(config)
+    new_info = info.update(tags_only=False, freeze=False)
+    assert info == new_info
+
+
+def test_rev_info_update_out_of_date_repo(out_of_date):
+    config = make_config_from_repo(
+        out_of_date.path, rev=out_of_date.original_rev,
+    )
+    info = RevInfo.from_config(config)
+    new_info = info.update(tags_only=False, freeze=False)
+    assert new_info.rev == out_of_date.head_rev
+
+
+def test_rev_info_update_non_master_default_branch(out_of_date):
+    # change the default branch to be not-master
+    cmd_output('git', '-C', out_of_date.path, 'branch', '-m', 'dev')
+    test_rev_info_update_out_of_date_repo(out_of_date)
+
+
+def test_rev_info_update_tags_even_if_not_tags_only(tagged):
+    config = make_config_from_repo(tagged.path, rev=tagged.original_rev)
+    info = RevInfo.from_config(config)
+    new_info = info.update(tags_only=False, freeze=False)
+    assert new_info.rev == 'v1.2.3'
+
+
+def test_rev_info_update_tags_only_does_not_pick_tip(tagged):
+    git_commit(cwd=tagged.path)
+    config = make_config_from_repo(tagged.path, rev=tagged.original_rev)
+    info = RevInfo.from_config(config)
+    new_info = info.update(tags_only=True, freeze=False)
+    assert new_info.rev == 'v1.2.3'
+
+
+def test_rev_info_update_freeze_tag(tagged):
+    git_commit(cwd=tagged.path)
+    config = make_config_from_repo(tagged.path, rev=tagged.original_rev)
+    info = RevInfo.from_config(config)
+    new_info = info.update(tags_only=True, freeze=True)
+    assert new_info.rev == tagged.head_rev
+    assert new_info.frozen == 'v1.2.3'
+
+
+def test_rev_info_update_does_not_freeze_if_already_sha(out_of_date):
+    config = make_config_from_repo(
+        out_of_date.path, rev=out_of_date.original_rev,
+    )
+    info = RevInfo.from_config(config)
+    new_info = info.update(tags_only=True, freeze=True)
+    assert new_info.rev == out_of_date.head_rev
+    assert new_info.frozen is None
+
+
+def test_autoupdate_up_to_date_repo(up_to_date, tmpdir, store):
+    contents = (
+        'repos:\n'
+        '-   repo: {}\n'
+        '    rev: {}\n'
+        '    hooks:\n'
+        '    -   id: foo\n'
+    ).format(up_to_date, git.head_rev(up_to_date))
+    cfg = tmpdir.join(C.CONFIG_FILE)
+    cfg.write(contents)
+
+    assert autoupdate(str(cfg), store, freeze=False, tags_only=False) == 0
+    assert cfg.read() == contents
 
 
 def test_autoupdate_old_revision_broken(tempdir_factory, in_tmpdir, store):
@@ -68,98 +153,101 @@ def test_autoupdate_old_revision_broken(tempdir_factory, in_tmpdir, store):
     write_config('.', config)
     with open(C.CONFIG_FILE) as f:
         before = f.read()
-    ret = autoupdate(C.CONFIG_FILE, store, tags_only=False)
+    assert autoupdate(C.CONFIG_FILE, store, freeze=False, tags_only=False) == 0
     with open(C.CONFIG_FILE) as f:
         after = f.read()
-    assert ret == 0
     assert before != after
     assert update_rev in after
 
 
-@pytest.fixture
-def out_of_date_repo(tempdir_factory):
-    path = make_repo(tempdir_factory, 'python_hooks_repo')
-    original_rev = git.head_rev(path)
-
-    git_commit(cwd=path)
-    head_rev = git.head_rev(path)
-
-    yield auto_namedtuple(
-        path=path, original_rev=original_rev, head_rev=head_rev,
+def test_autoupdate_out_of_date_repo(out_of_date, tmpdir, store):
+    fmt = (
+        'repos:\n'
+        '-   repo: {}\n'
+        '    rev: {}\n'
+        '    hooks:\n'
+        '    -   id: foo\n'
     )
+    cfg = tmpdir.join(C.CONFIG_FILE)
+    cfg.write(fmt.format(out_of_date.path, out_of_date.original_rev))
 
-
-def test_out_of_date_repo(out_of_date_repo, store):
-    config = make_config_from_repo(
-        out_of_date_repo.path, rev=out_of_date_repo.original_rev,
-    )
-    ret = _update_repo(config, store, tags_only=False)
-    assert ret['rev'] != out_of_date_repo.original_rev
-    assert ret['rev'] == out_of_date_repo.head_rev
+    assert autoupdate(str(cfg), store, freeze=False, tags_only=False) == 0
+    assert cfg.read() == fmt.format(out_of_date.path, out_of_date.head_rev)
 
 
-def test_autoupdate_out_of_date_repo(out_of_date_repo, in_tmpdir, store):
-    # Write out the config
-    config = make_config_from_repo(
-        out_of_date_repo.path, rev=out_of_date_repo.original_rev, check=False,
+def test_autoupdate_only_one_to_update(up_to_date, out_of_date, tmpdir, store):
+    fmt = (
+        'repos:\n'
+        '-   repo: {}\n'
+        '    rev: {}\n'
+        '    hooks:\n'
+        '    -   id: foo\n'
+        '-   repo: {}\n'
+        '    rev: {}\n'
+        '    hooks:\n'
+        '    -   id: foo\n'
     )
-    write_config('.', config)
+    cfg = tmpdir.join(C.CONFIG_FILE)
+    before = fmt.format(
+        up_to_date, git.head_rev(up_to_date),
+        out_of_date.path, out_of_date.original_rev,
+    )
+    cfg.write(before)
 
-    with open(C.CONFIG_FILE) as f:
-        before = f.read()
-    ret = autoupdate(C.CONFIG_FILE, store, tags_only=False)
-    with open(C.CONFIG_FILE) as f:
-        after = f.read()
-    assert ret == 0
-    assert before != after
-    # Make sure we don't add defaults
-    assert 'exclude' not in after
-    assert out_of_date_repo.head_rev in after
+    assert autoupdate(str(cfg), store, freeze=False, tags_only=False) == 0
+    assert cfg.read() == fmt.format(
+        up_to_date, git.head_rev(up_to_date),
+        out_of_date.path, out_of_date.head_rev,
+    )
 
 
 def test_autoupdate_out_of_date_repo_with_correct_repo_name(
-        out_of_date_repo, in_tmpdir, store,
+        out_of_date, in_tmpdir, store,
 ):
     stale_config = make_config_from_repo(
-        out_of_date_repo.path, rev=out_of_date_repo.original_rev, check=False,
+        out_of_date.path, rev=out_of_date.original_rev, check=False,
     )
     local_config = sample_local_config()
     config = {'repos': [stale_config, local_config]}
-    # Write out the config
     write_config('.', config)
 
     with open(C.CONFIG_FILE) as f:
         before = f.read()
-    repo_name = 'file://{}'.format(out_of_date_repo.path)
-    ret = autoupdate(C.CONFIG_FILE, store, tags_only=False, repos=(repo_name,))
+    repo_name = 'file://{}'.format(out_of_date.path)
+    ret = autoupdate(
+        C.CONFIG_FILE, store, freeze=False, tags_only=False,
+        repos=(repo_name,),
+    )
     with open(C.CONFIG_FILE) as f:
         after = f.read()
     assert ret == 0
     assert before != after
-    assert out_of_date_repo.head_rev in after
+    assert out_of_date.head_rev in after
     assert 'local' in after
 
 
 def test_autoupdate_out_of_date_repo_with_wrong_repo_name(
-        out_of_date_repo, in_tmpdir, store,
+        out_of_date, in_tmpdir, store,
 ):
-    # Write out the config
     config = make_config_from_repo(
-        out_of_date_repo.path, rev=out_of_date_repo.original_rev, check=False,
+        out_of_date.path, rev=out_of_date.original_rev, check=False,
     )
     write_config('.', config)
 
     with open(C.CONFIG_FILE) as f:
         before = f.read()
     # It will not update it, because the name doesn't match
-    ret = autoupdate(C.CONFIG_FILE, store, tags_only=False, repos=('dne',))
+    ret = autoupdate(
+        C.CONFIG_FILE, store, freeze=False, tags_only=False,
+        repos=('dne',),
+    )
     with open(C.CONFIG_FILE) as f:
         after = f.read()
     assert ret == 0
     assert before == after
 
 
-def test_does_not_reformat(in_tmpdir, out_of_date_repo, store):
+def test_does_not_reformat(tmpdir, out_of_date, store):
     fmt = (
         'repos:\n'
         '-   repo: {}\n'
@@ -169,20 +257,15 @@ def test_does_not_reformat(in_tmpdir, out_of_date_repo, store):
         '        # These args are because reasons!\n'
         '        args: [foo, bar, baz]\n'
     )
-    config = fmt.format(out_of_date_repo.path, out_of_date_repo.original_rev)
-    with open(C.CONFIG_FILE, 'w') as f:
-        f.write(config)
+    cfg = tmpdir.join(C.CONFIG_FILE)
+    cfg.write(fmt.format(out_of_date.path, out_of_date.original_rev))
 
-    autoupdate(C.CONFIG_FILE, store, tags_only=False)
-    with open(C.CONFIG_FILE) as f:
-        after = f.read()
-    expected = fmt.format(out_of_date_repo.path, out_of_date_repo.head_rev)
-    assert after == expected
+    assert autoupdate(str(cfg), store, freeze=False, tags_only=False) == 0
+    expected = fmt.format(out_of_date.path, out_of_date.head_rev)
+    assert cfg.read() == expected
 
 
-def test_loses_formatting_when_not_detectable(
-        out_of_date_repo, store, in_tmpdir,
-):
+def test_loses_formatting_when_not_detectable(out_of_date, store, tmpdir):
     """A best-effort attempt is made at updating rev without rewriting
     formatting.  When the original formatting cannot be detected, this
     is abandoned.
@@ -197,149 +280,119 @@ def test_loses_formatting_when_not_detectable(
         '        ],\n'
         '    }}\n'
         ']\n'.format(
-            pipes.quote(out_of_date_repo.path), out_of_date_repo.original_rev,
+            pipes.quote(out_of_date.path), out_of_date.original_rev,
         )
     )
-    with open(C.CONFIG_FILE, 'w') as f:
-        f.write(config)
+    cfg = tmpdir.join(C.CONFIG_FILE)
+    cfg.write(config)
 
-    autoupdate(C.CONFIG_FILE, store, tags_only=False)
-    with open(C.CONFIG_FILE) as f:
-        after = f.read()
+    assert autoupdate(str(cfg), store, freeze=False, tags_only=False) == 0
     expected = (
         'repos:\n'
         '-   repo: {}\n'
         '    rev: {}\n'
         '    hooks:\n'
         '    -   id: foo\n'
-    ).format(out_of_date_repo.path, out_of_date_repo.head_rev)
-    assert after == expected
-
-
-@pytest.fixture
-def tagged_repo(out_of_date_repo):
-    cmd_output('git', 'tag', 'v1.2.3', cwd=out_of_date_repo.path)
-    yield out_of_date_repo
+    ).format(out_of_date.path, out_of_date.head_rev)
+    assert cfg.read() == expected
 
 
-def test_autoupdate_tagged_repo(tagged_repo, in_tmpdir, store):
-    config = make_config_from_repo(
-        tagged_repo.path, rev=tagged_repo.original_rev,
-    )
+def test_autoupdate_tagged_repo(tagged, in_tmpdir, store):
+    config = make_config_from_repo(tagged.path, rev=tagged.original_rev)
     write_config('.', config)
 
-    ret = autoupdate(C.CONFIG_FILE, store, tags_only=False)
-    assert ret == 0
+    assert autoupdate(C.CONFIG_FILE, store, freeze=False, tags_only=False) == 0
     with open(C.CONFIG_FILE) as f:
         assert 'v1.2.3' in f.read()
 
 
-@pytest.fixture
-def tagged_repo_with_more_commits(tagged_repo):
-    git_commit(cwd=tagged_repo.path)
-    yield tagged_repo
+def test_autoupdate_freeze(tagged, in_tmpdir, store):
+    config = make_config_from_repo(tagged.path, rev=tagged.original_rev)
+    write_config('.', config)
 
+    assert autoupdate(C.CONFIG_FILE, store, freeze=True, tags_only=False) == 0
+    with open(C.CONFIG_FILE) as f:
+        expected = 'rev: {}  # v1.2.3'.format(tagged.head_rev)
+        assert expected in f.read()
 
-def test_autoupdate_tags_only(tagged_repo_with_more_commits, in_tmpdir, store):
-    config = make_config_from_repo(
-        tagged_repo_with_more_commits.path,
-        rev=tagged_repo_with_more_commits.original_rev,
-    )
+
+def test_autoupdate_tags_only(tagged, in_tmpdir, store):
+    # add some commits after the tag
+    git_commit(cwd=tagged.path)
+
+    config = make_config_from_repo(tagged.path, rev=tagged.original_rev)
     write_config('.', config)
 
-    ret = autoupdate(C.CONFIG_FILE, store, tags_only=True)
-    assert ret == 0
+    assert autoupdate(C.CONFIG_FILE, store, freeze=False, tags_only=True) == 0
     with open(C.CONFIG_FILE) as f:
         assert 'v1.2.3' in f.read()
 
 
-def test_autoupdate_latest_no_config(out_of_date_repo, in_tmpdir, store):
+def test_autoupdate_latest_no_config(out_of_date, in_tmpdir, store):
     config = make_config_from_repo(
-        out_of_date_repo.path, rev=out_of_date_repo.original_rev,
+        out_of_date.path, rev=out_of_date.original_rev,
     )
     write_config('.', config)
 
-    cmd_output('git', 'rm', '-r', ':/', cwd=out_of_date_repo.path)
-    git_commit(cwd=out_of_date_repo.path)
+    cmd_output('git', 'rm', '-r', ':/', cwd=out_of_date.path)
+    git_commit(cwd=out_of_date.path)
 
-    ret = autoupdate(C.CONFIG_FILE, store, tags_only=False)
-    assert ret == 1
+    assert autoupdate(C.CONFIG_FILE, store, freeze=False, tags_only=False) == 1
     with open(C.CONFIG_FILE) as f:
-        assert out_of_date_repo.original_rev in f.read()
+        assert out_of_date.original_rev in f.read()
 
 
-@pytest.fixture
-def hook_disappearing_repo(tempdir_factory):
-    path = make_repo(tempdir_factory, 'python_hooks_repo')
-    original_rev = git.head_rev(path)
-
-    with modify_manifest(path) as manifest:
-        manifest[0]['id'] = 'bar'
-
-    yield auto_namedtuple(path=path, original_rev=original_rev)
-
-
-def test_hook_disppearing_repo_raises(hook_disappearing_repo, store):
+def test_hook_disppearing_repo_raises(hook_disappearing, store):
     config = make_config_from_repo(
-        hook_disappearing_repo.path,
-        rev=hook_disappearing_repo.original_rev,
+        hook_disappearing.path,
+        rev=hook_disappearing.original_rev,
         hooks=[{'id': 'foo'}],
     )
+    info = RevInfo.from_config(config).update(tags_only=False, freeze=False)
     with pytest.raises(RepositoryCannotBeUpdatedError):
-        _update_repo(config, store, tags_only=False)
-
-
-def test_autoupdate_hook_disappearing_repo(
-        hook_disappearing_repo, in_tmpdir, store,
-):
-    config = make_config_from_repo(
-        hook_disappearing_repo.path,
-        rev=hook_disappearing_repo.original_rev,
-        hooks=[{'id': 'foo'}],
-        check=False,
-    )
-    write_config('.', config)
+        _check_hooks_still_exist_at_rev(config, info, store)
 
-    with open(C.CONFIG_FILE) as f:
-        before = f.read()
-    ret = autoupdate(C.CONFIG_FILE, store, tags_only=False)
-    with open(C.CONFIG_FILE) as f:
-        after = f.read()
-    assert ret == 1
-    assert before == after
 
+def test_autoupdate_hook_disappearing_repo(hook_disappearing, tmpdir, store):
+    contents = (
+        'repos:\n'
+        '-   repo: {}\n'
+        '    rev: {}\n'
+        '    hooks:\n'
+        '    -   id: foo\n'
+    ).format(hook_disappearing.path, hook_disappearing.original_rev)
+    cfg = tmpdir.join(C.CONFIG_FILE)
+    cfg.write(contents)
 
-def test_autoupdate_non_master_default_branch(up_to_date_repo, store):
-    # change the default branch to be not-master
-    cmd_output('git', '-C', up_to_date_repo, 'branch', '-m', 'dev')
-    test_up_to_date_repo(up_to_date_repo, store)
+    assert autoupdate(str(cfg), store, freeze=False, tags_only=False) == 1
+    assert cfg.read() == contents
 
 
 def test_autoupdate_local_hooks(in_git_dir, store):
     config = sample_local_config()
     add_config_to_repo('.', config)
-    assert autoupdate(C.CONFIG_FILE, store, tags_only=False) == 0
+    assert autoupdate(C.CONFIG_FILE, store, freeze=False, tags_only=False) == 0
     new_config_writen = read_config('.')
     assert len(new_config_writen['repos']) == 1
     assert new_config_writen['repos'][0] == config
 
 
 def test_autoupdate_local_hooks_with_out_of_date_repo(
-        out_of_date_repo, in_tmpdir, store,
+        out_of_date, in_tmpdir, store,
 ):
     stale_config = make_config_from_repo(
-        out_of_date_repo.path, rev=out_of_date_repo.original_rev, check=False,
+        out_of_date.path, rev=out_of_date.original_rev, check=False,
     )
     local_config = sample_local_config()
     config = {'repos': [local_config, stale_config]}
     write_config('.', config)
-    assert autoupdate(C.CONFIG_FILE, store, tags_only=False) == 0
+    assert autoupdate(C.CONFIG_FILE, store, freeze=False, tags_only=False) == 0
     new_config_writen = read_config('.')
     assert len(new_config_writen['repos']) == 2
     assert new_config_writen['repos'][0] == local_config
 
 
-def test_autoupdate_meta_hooks(tmpdir, capsys, store):
+def test_autoupdate_meta_hooks(tmpdir, store):
     cfg = tmpdir.join(C.CONFIG_FILE)
     cfg.write(
         'repos:\n'
@@ -347,9 +400,7 @@ def test_autoupdate_meta_hooks(tmpdir, capsys, store):
         '    hooks:\n'
         '    -   id: check-useless-excludes\n',
     )
-    with tmpdir.as_cwd():
-        ret = autoupdate(C.CONFIG_FILE, store, tags_only=True)
-    assert ret == 0
+    assert autoupdate(str(cfg), store, freeze=False, tags_only=True) == 0
     assert cfg.read() == (
         'repos:\n'
         '-   repo: meta\n'
@@ -368,9 +419,7 @@ def test_updates_old_format_to_new_format(tmpdir, capsys, store):
         '        entry: ./bin/foo.sh\n'
         '        language: script\n',
     )
-    with tmpdir.as_cwd():
-        ret = autoupdate(C.CONFIG_FILE, store, tags_only=True)
-    assert ret == 0
+    assert autoupdate(str(cfg), store, freeze=False, tags_only=True) == 0
     contents = cfg.read()
     assert contents == (
         'repos:\n'
diff --git a/tests/commands/gc_test.py b/tests/commands/gc_test.py
index 5be86b1..02b3694 100644
--- a/tests/commands/gc_test.py
+++ b/tests/commands/gc_test.py
@@ -42,7 +42,7 @@ def test_gc(tempdir_factory, store, in_git_dir, cap_out):
 
     # update will clone both the old and new repo, making the old one gc-able
     install_hooks(C.CONFIG_FILE, store)
-    assert not autoupdate(C.CONFIG_FILE, store, tags_only=False)
+    assert not autoupdate(C.CONFIG_FILE, store, freeze=False, tags_only=False)
 
     assert _config_count(store) == 1
     assert _repo_count(store) == 2