diff --git a/README.rst b/README.rst index 9a8e55e598dfbb49e1e46bfc2282ed848d435803_UkVBRE1FLnJzdA==..b46639f34ff66857fc4cf29eb4dddbb5e696b779_UkVBRE1FLnJzdA== 100644 --- a/README.rst +++ b/README.rst @@ -216,22 +216,6 @@ author file is mostly useful for translating legacy changesets. -``git.blockdotgit`` -------------------- - -Blocks exporting revisions to Git that contain a directory named .git or -any letter-case variation thereof. This prevents creating repositories -that newer versions of Git and many Git hosting services block due to -security concerns. Defaults to True. - - -``git.blockdothg`` ------------------- - -Blocks importing revisions from Git that contain a directory named .hg. -Defaults to True. - - ``git.branch_bookmark_suffix`` ------------------------------ @@ -366,3 +350,23 @@ repositories, this means the remote ``master`` branch will be converted as public. Publishing commits prevents their modification, and speeds up many local Mercurial operations, such as ``hg shelve``. + +``hggit.invalidpaths`` +------------------- + +Both Mercurial and Git consider paths as just bytestrings internally, +and allow almost anything. The difference, however, is in the _almost_ +part. For example, many Git servers will reject a push for security +reasons if it contains a nested Git repository. Similarly, Mercurial +cannot checkout commits with a nested repository, and it cannot even +store paths containing an embedded newline or carrage return +character. + +The default is to issue a warning and skip these paths. You can +change this by setting ``hggit.invalidpaths`` in ``.hgrc``:: + + [hggit] + invalidpaths = keep + +Possible values are ``keep``, ``skip`` or ``abort``. Prior to 0.11, +the default was ``abort``. diff --git a/hggit/__init__.py b/hggit/__init__.py index 9a8e55e598dfbb49e1e46bfc2282ed848d435803_aGdnaXQvX19pbml0X18ucHk=..b46639f34ff66857fc4cf29eb4dddbb5e696b779_aGdnaXQvX19pbml0X18ucHk= 100644 --- a/hggit/__init__.py +++ b/hggit/__init__.py @@ -98,6 +98,25 @@ Revsets are accepted by several Mercurial commands for specifying revisions. See :hg:`help revsets` for details. +Invalid and dangerous paths +--------------------------- + +Both Mercurial and Git consider paths as just bytestrings internally, +and allow almost anything. The difference, however, is in the _almost_ +part. For example, many Git servers will reject a push for security +reasons if it contains a nested Git repository. Similarly, Mercurial +cannot checkout commits with a nested repository, and it cannot even +store paths containing an embedded newline or carrage return +character. + +The default is to issue a warning and skip these paths. You can +change this by setting ``hggit.invalidpaths`` in :hg:`config`:: + + [hggit] + invalidpaths = keep + +Possible values are ``keep``, ``skip`` or ``abort``. + ''' from __future__ import generator_stop diff --git a/hggit/compat.py b/hggit/compat.py index 9a8e55e598dfbb49e1e46bfc2282ed848d435803_aGdnaXQvY29tcGF0LnB5..b46639f34ff66857fc4cf29eb4dddbb5e696b779_aGdnaXQvY29tcGF0LnB5 100644 --- a/hggit/compat.py +++ b/hggit/compat.py @@ -47,8 +47,6 @@ CONFIG_DEFAULTS = { b'git': { b'authors': None, - b'blockdotgit': True, - b'blockdothg': True, b'branch_bookmark_suffix': None, b'debugextrainmessage': False, # test only -- do not document this! b'findcopiesharder': False, @@ -61,6 +59,7 @@ b'hggit': { b'mapsavefrequency': 0, b'usephases': False, + b'invalidpaths': b'skip', } } diff --git a/hggit/git_handler.py b/hggit/git_handler.py index 9a8e55e598dfbb49e1e46bfc2282ed848d435803_aGdnaXQvZ2l0X2hhbmRsZXIucHk=..b46639f34ff66857fc4cf29eb4dddbb5e696b779_aGdnaXQvZ2l0X2hhbmRsZXIucHk= 100644 --- a/hggit/git_handler.py +++ b/hggit/git_handler.py @@ -1614,6 +1614,9 @@ # stored in 'old = file' case, then membership check fails in 'new # = file' case so is overwritten. if newmode == 0o160000: + if not self.audit_hg_path(newfile): + # disregard illegal or inconvenient paths + continue # new = gitlink gitlinks[newfile] = newsha if change.type == diff_tree.CHANGE_RENAME: @@ -1629,7 +1632,8 @@ gitlinks[oldfile] = None continue if newfile is not None: - self.audit_hg_path(newfile) + if not self.audit_hg_path(newfile): + continue # new = file files[newfile] = False, newmode, newsha if renames is not None and newfile != oldfile: @@ -1714,6 +1718,12 @@ return path.name def audit_hg_path(self, path): - if b'.hg' in path.split(b'/'): - if self.ui.configbool(b'git', b'blockdothg'): + if b'.hg' in path.split(b'/') or b'\r' in path or b'\n' in path: + ui = self.ui + + # escape the path when printing it out + prettypath = path.decode('latin1').encode('unicode-escape') + + opt = ui.config(b'hggit', b'invalidpaths') + if opt == b'abort': raise error.Abort( @@ -1719,11 +1729,19 @@ raise error.Abort( - (b"Refusing to import problematic path '%s'" % path), - hint=(b"Mercurial cannot check out paths inside nested " + - b"repositories; if you need to continue, then set " + - b"'[git] blockdothg = false' in your hgrc.")) - self.ui.warn((b"warning: path '%s' is within a nested " + - b'repository, which Mercurial cannot check out.\n') - % path) + b"invalid path '%s' rejected by configuration" % prettypath, + hint=b"see 'hg help hggit' for details", + ) + elif opt == b'keep' and b'\r' not in path and b'\n' not in path: + ui.warn( + b"warning: path '%s' contains an invalid path component\n" + % prettypath, + ) + return True + else: + # undocumented: just let anything else mean "skip" + ui.warn(b"warning: skipping invalid path '%s'\n" % prettypath) + return False + + return True # Stolen from hgsubversion def swap_out_encoding(self, new_encoding=b'UTF-8'): diff --git a/hggit/hg2git.py b/hggit/hg2git.py index 9a8e55e598dfbb49e1e46bfc2282ed848d435803_aGdnaXQvaGcyZ2l0LnB5..b46639f34ff66857fc4cf29eb4dddbb5e696b779_aGdnaXQvaGcyZ2l0LnB5 100644 --- a/hggit/hg2git.py +++ b/hggit/hg2git.py @@ -115,7 +115,7 @@ """ return self._dirs[b''].id - def _audit_one_path(self, path): + def audit_path(self, path): r"""Check for path components that case-fold to .git. Returns ``True`` for normal paths. The results for insecure @@ -119,5 +119,9 @@ r"""Check for path components that case-fold to .git. Returns ``True`` for normal paths. The results for insecure - paths depend on the configuration in ``ui``: + paths depend on the ``hggit.invalidpaths`` configuration in + ``ui``: + + ``abort`` means insecure paths abort the conversion — + this was the default prior to 0.11. @@ -123,4 +127,4 @@ - If ``git.blockdotgit`` is set, insecure paths raise an - exception. + ``keep`` means issue a warning and keep the path, + returning ``True``. @@ -126,7 +130,6 @@ - Otherwise, a warning may be printed, but the return value is - ``True``, indicating that all paths should be retained - regardless of any problems they may cause. + Anything else, but documented as ``skip``, means issue a + warning and disregard the path, returning ``False``. """ ui = self._hg.ui @@ -142,5 +145,10 @@ dangerous = True break if dangerous: - if ui.configbool(b'git', b'blockdotgit'): + opt = ui.config(b'hggit', b'invalidpaths') + + # escape the path when printing it out + prettypath = path.decode('latin1').encode('unicode-escape') + + if opt == b'abort': raise error.Abort( @@ -146,12 +154,17 @@ raise error.Abort( - b"Refusing to export likely-dangerous path '%s'" % path, - hint=(b'If you need to continue, read about ' - b'CVE-2014-9390 and then set ' - b"'[git] blockdotgit = false' in your hgrc.")) - ui.warn(b"warning: path '%s' contains a potentially dangerous " - b'path component.\n' - b'It may not be legal to check out in Git.\n' - b'It may also be rejected by some git server ' - b'configurations.\n' - % path) + b"invalid path '%s' rejected by configuration" % prettypath, + hint=b"see 'hg help hggit' for details", + ) + elif opt == b'keep': + ui.warn( + b"warning: path '%s' contains an invalid path component\n" + % prettypath, + ) + return True + else: + # undocumented: just let anything else mean "skip" + ui.warn(b"warning: skipping invalid path '%s'\n" % prettypath) + return False + else: + return True @@ -157,9 +170,12 @@ - def audit_paths(self, paths): - """Handle insecure paths""" - for path in paths: - self._audit_one_path(path) - return paths + def filter_unsafe_paths(self, paths): + """Return a copy of the given list, with dangerous paths removed. + + Please note that the configuration can suppress the removal; + see above. + + """ + return [path for path in paths if self.audit_path(path)] def update_changeset(self, newctx): """Set the tree to track a new Mercurial changeset. @@ -197,7 +213,7 @@ # localrepo.status(). status = self._hg.status(self._ctx, newctx) modified, added, removed = map( - self.audit_paths, (status.modified, status.added, status.removed), + self.filter_unsafe_paths, (status.modified, status.added, status.removed), ) # We track which directories/trees have modified in this update and we diff --git a/tests/test-illegal-contents.t b/tests/test-illegal-contents.t index 9a8e55e598dfbb49e1e46bfc2282ed848d435803_dGVzdHMvdGVzdC1pbGxlZ2FsLWNvbnRlbnRzLnQ=..b46639f34ff66857fc4cf29eb4dddbb5e696b779_dGVzdHMvdGVzdC1pbGxlZ2FsLWNvbnRlbnRzLnQ= 100644 --- a/tests/test-illegal-contents.t +++ b/tests/test-illegal-contents.t @@ -11,13 +11,7 @@ > #!/bin/sh > echo pwned > EOF - $ python - foo/git~100/wat bar/.gi\\u200ct/wut this/is/safe <<EOF - > import os, sys - > for p in sys.argv[1:]: - > p = p.encode('ascii').decode('unicode_escape').encode('utf-8') - > os.makedirs(os.path.dirname(p)) - > open(p, 'w') - > EOF + $ fn_touch_escaped foo/git~100/wat bar/.gi\\u200ct/wut this/is/safe $ hg addremove adding .git/hooks/post-update adding bar/.gi\xe2\x80\x8ct/wut (esc) @@ -26,19 +20,17 @@ $ hg ci -m "we should refuse to export this" $ hg book master $ hg gexport - abort: Refusing to export likely-dangerous path '.git/hooks/post-update' - (If you need to continue, read about CVE-2014-9390 and then set '[git] blockdotgit = false' in your hgrc.) - [255] - $ hg gexport --config git.blockdotgit=no - warning: path '.git/hooks/post-update' contains a potentially dangerous path component. - It may not be legal to check out in Git. - It may also be rejected by some git server configurations. - warning: path 'bar/.gi\xe2\x80\x8ct/wut' contains a potentially dangerous path component. (esc) - It may not be legal to check out in Git. - It may also be rejected by some git server configurations. - warning: path 'foo/git~100/wat' contains a potentially dangerous path component. - It may not be legal to check out in Git. - It may also be rejected by some git server configurations. + warning: skipping invalid path '.git/hooks/post-update' + warning: skipping invalid path 'bar/.gi\xe2\x80\x8ct/wut' + warning: skipping invalid path 'foo/git~100/wat' + $ GIT_DIR=.hg/git git ls-tree -r --name-only master + this/is/safe + $ hg gclear + clearing out the git cache data + $ hg gexport --config hggit.invalidpaths=keep + warning: path '.git/hooks/post-update' contains an invalid path component + warning: path 'bar/.gi\xe2\x80\x8ct/wut' contains an invalid path component + warning: path 'foo/git~100/wat' contains an invalid path component $ GIT_DIR=.hg/git git ls-tree -r --name-only master .git/hooks/post-update "bar/.gi\342\200\214t/wut" @@ -60,6 +52,16 @@ $ hg ci -m "also refuse to export this" $ hg book master $ hg gexport - abort: Refusing to export likely-dangerous path 'nested/.git/hooks/post-update' - (If you need to continue, read about CVE-2014-9390 and then set '[git] blockdotgit = false' in your hgrc.) + warning: skipping invalid path 'nested/.git/hooks/post-update' + $ git clone .hg/git git + Cloning into 'git'... + done. + $ rm -rf git + +We can trigger an error: + + $ hg -q gclear + $ hg --config hggit.invalidpaths=abort gexport + abort: invalid path 'nested/.git/hooks/post-update' rejected by configuration + (see 'hg help hggit' for details) [255] @@ -65,2 +67,3 @@ [255] + We can override if needed: @@ -66,8 +69,7 @@ We can override if needed: - $ hg --config git.blockdotgit=false gexport - warning: path 'nested/.git/hooks/post-update' contains a potentially dangerous path component. - It may not be legal to check out in Git. - It may also be rejected by some git server configurations. + + $ hg --config hggit.invalidpaths=keep gexport + warning: path 'nested/.git/hooks/post-update' contains an invalid path component $ cd .. $ # different git versions give different return codes $ git clone hg/.hg/git git || true @@ -116,9 +118,7 @@ $ hg ci -m "also refuse to export this" $ hg book master $ hg gexport - abort: Refusing to export likely-dangerous path 'GIT~1/hooks/post-checkout' - (If you need to continue, read about CVE-2014-9390 and then set '[git] blockdotgit = false' in your hgrc.) - [255] + warning: skipping invalid path 'GIT~1/hooks/post-checkout' $ cd .. Now check a Git repository containing a Mercurial repository, which @@ -132,5 +132,5 @@ $ git add nested $ fn_git_commit -m 'add a Mercurial repository' $ cd .. - $ hg clone git hg + $ hg clone --config hggit.invalidpaths=abort git hg importing git objects into hg @@ -136,5 +136,5 @@ importing git objects into hg - abort: Refusing to import problematic path 'nested/.hg/00changelog.i' - (Mercurial cannot check out paths inside nested repositories; if you need to continue, then set '[git] blockdothg = false' in your hgrc.) + abort: invalid path 'nested/.hg/00changelog.i' rejected by configuration + (see 'hg help hggit' for details) [255] $ rm -rf hg @@ -139,4 +139,4 @@ [255] $ rm -rf hg - $ hg clone --config git.blockdothg=false git hg + $ hg clone --config hggit.invalidpaths=keep git hg importing git objects into hg @@ -142,7 +142,7 @@ importing git objects into hg - warning: path 'nested/.hg/00changelog.i' is within a nested repository, which Mercurial cannot check out. - warning: path 'nested/.hg/requires' is within a nested repository, which Mercurial cannot check out. + warning: path 'nested/.hg/00changelog.i' contains an invalid path component + warning: path 'nested/.hg/requires' contains an invalid path component updating to branch default (no-hg57 !) updating to bookmark master (hg57 !) abort: path 'nested/.hg/00changelog.i' is inside nested repo 'nested' [255] @@ -145,5 +145,13 @@ updating to branch default (no-hg57 !) updating to bookmark master (hg57 !) abort: path 'nested/.hg/00changelog.i' is inside nested repo 'nested' [255] + $ rm -rf hg + $ hg clone git hg + importing git objects into hg + warning: skipping invalid path 'nested/.hg/00changelog.i' + warning: skipping invalid path 'nested/.hg/requires' + updating to branch default (no-hg57 !) + updating to bookmark master (hg57 !) + 0 files updated, 0 files merged, 0 files removed, 0 files unresolved $ cd .. @@ -149,1 +157,34 @@ $ cd .. + +Now check a Git repository containing paths with carriage return and +newline, which Mercurial expressly forbids +(see https://bz.mercurial-scm.org/show_bug.cgi?id=352) + + $ rm -rf hg git + $ git init -q git + $ cd git + $ fn_touch_escaped Icon\\r the\\nfile + $ git add . + $ fn_git_commit -m 'add files disallowed by mercurial' + $ cd .. + $ hg clone --config hggit.invalidpaths=abort git hg + importing git objects into hg + abort: invalid path 'Icon\r' rejected by configuration + (see 'hg help hggit' for details) + [255] + $ hg clone --config hggit.invalidpaths=keep git hg + importing git objects into hg + warning: skipping invalid path 'Icon\r' + warning: skipping invalid path 'the\nfile' + updating to branch default (no-hg57 !) + updating to bookmark master (hg57 !) + 0 files updated, 0 files merged, 0 files removed, 0 files unresolved + $ rm -rf hg + $ hg clone git hg + importing git objects into hg + warning: skipping invalid path 'Icon\r' + warning: skipping invalid path 'the\nfile' + updating to branch default (no-hg57 !) + updating to bookmark master (hg57 !) + 0 files updated, 0 files merged, 0 files removed, 0 files unresolved + diff --git a/tests/testutil b/tests/testutil index 9a8e55e598dfbb49e1e46bfc2282ed848d435803_dGVzdHMvdGVzdHV0aWw=..b46639f34ff66857fc4cf29eb4dddbb5e696b779_dGVzdHMvdGVzdHV0aWw= 100755 --- a/tests/testutil +++ b/tests/testutil @@ -72,3 +72,14 @@ hg tag -d "$HGDATE" "$@" >/dev/null || echo "hg tag error" count=`expr $count + 1` } + +fn_touch_escaped() { + python - "$@" <<EOF +import os, sys +for p in sys.argv[1:]: + p = p.encode('ascii').decode('unicode_escape').encode('utf-8') + if b'/' in p and not os.path.exists(os.path.dirname(p)): + os.makedirs(os.path.dirname(p)) + open(p, 'w') +EOF +}