Page MenuHomePhabricator
Create Task
Maniphest T12416

Repository mirroring fails due to new `git ls-remote` usage
Closed, ResolvedPublic
Actions

Description

Since the introduction of rP2b0ad243d179: Use "git ls-remote" to guess if "git fetch" is a no-op mirroring git repositories fails here:

[18-Mar-2017 22:27:03 UTC] [2017-03-18 22:27:03] EXCEPTION: (PhutilProxyException) Error while updating the "R10" repository. {>} (CommandException) Command failed with error #255!
COMMAND
'/var/www/phabricator.company.tld/phabricator/builds/69261d34/bin/repository' update  -- 'R10'

STDOUT
(empty)

STDERR
[2017-03-18 22:27:03] EXCEPTION: (CommandException) Command failed with error #129!
COMMAND
git ls-remote -- '********'

STDOUT
(empty)

STDERR
usage: git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>]
                     [-q|--quiet] [--exit-code] [--get-url] [<repository> [<refs>...]]
 at [<phutil>/src/future/exec/ExecFuture.php:369]
arcanist(head=master, ref.master=3b6b523c2b23), phabricator(head=master, ref.master=688c120f9f33), phutil(head=master, ref.master=91ab940c3979)
  #0 ExecFuture::resolvex() called at [<phabricator>/src/applications/repository/storage/PhabricatorRepository.php:491]
  #1 PhabricatorRepository::execxRemoteCommand(string, PhutilOpaqueEnvelope) called at [<phabricator>/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php:397]
  #2 PhabricatorRepositoryPullEngine::loadGitRemoteRefs(PhabricatorRepository) called at [<phabricator>/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php:343]
  #3 PhabricatorRepos... (869 more bytes) ... at [<phutil>/src/future/exec/ExecFuture.php:369]
[18-Mar-2017 22:27:03 UTC] arcanist(head=master, ref.master=3b6b523c2b23), phabricator(head=master, ref.master=688c120f9f33), phutil(head=master, ref.master=91ab940c3979)
[18-Mar-2017 22:27:03 UTC]   #0 <#3> ExecFuture::resolvex() called at [<phabricator>/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php:442]
[18-Mar-2017 22:27:03 UTC]   #1 phlog(PhutilProxyException) called at [<phabricator>/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php:449]
[18-Mar-2017 22:27:03 UTC]   #2 PhabricatorRepositoryPullLocalDaemon::resolveUpdateFuture(PhabricatorRepository, ExecFuture, integer) called at [<phabricator>/src/applications/repository/daemon/PhabricatorRepositoryPullLocalDaemon.php:221]
[18-Mar-2017 22:27:03 UTC]   #3 PhabricatorRepositoryPullLocalDaemon::run() called at [<phutil>/src/daemon/PhutilDaemon.php:219]
[18-Mar-2017 22:27:03 UTC]   #4 PhutilDaemon::execute() called at [<phutil>/scripts/daemon/exec/exec_daemon.php:131]

Version Information

Revisions and Commits

rP Phabricator
D17508rP56dd1b297c3e (stable) Don't use "--" to separate flags and arguments in "git ls-remote"
D17508rPd19fc2335e62 Don't use "--" to separate flags and arguments in "git ls-remote"

Related Objects

Event Timeline

eliaspro created this task.Mar 18 2017, 10:34 PM
eliaspro mentioned this in T12417: Show failing status of Diffusion repositories in top main menu bar..Mar 18 2017, 11:09 PM
eliaspro added a project: Diffusion.Mar 18 2017, 11:12 PM
epriestley added a subscriber: epriestley.Mar 19 2017, 12:14 AM

I can't immediately reproduce this, although the systems I have immediate access to are all running Git 2.11.0.

Does this command work on your system?

$ git ls-remote -- https://github.com/phacility/libphutil.git
eliaspro added a comment.Mar 19 2017, 12:20 AM
In T12416#215813, @epriestley wrote:

I can't immediately reproduce this, although the systems I have immediate access to are all running Git 2.11.0.

Does this command work on your system?

$ git ls-remote -- https://github.com/phacility/libphutil.git

No, the result is:

phabricator@prod-phab-infra-4608e666:~$ git ls-remote -- https://github.com/phacility/libphutil.git
usage: git ls-remote [--heads] [--tags]  [-u <exec> | --upload-pack <exec>]
                     [-q|--quiet] [--exit-code] [--get-url] [<repository> [<refs>...]]
epriestley added a comment.Mar 19 2017, 12:21 AM

Does it work without the --?

$ git ls-remote https://github.com/phacility/libphutil.git
eliaspro added a comment.Mar 19 2017, 12:24 AM

Just tried that too, and strangely: it does.

I wonder why that is as I also often use this approach to clearly separate args from values for shell commands.

epriestley added a comment.Mar 19 2017, 12:28 AM

I think this commit in the Git upstream is probably the culprit (January 2016):

https://github.com/git/git/commit/ba5f28bf79ea652fbe2be12f5ca6b351bb6ad591

I'm not terribly familiar with the git internals but it looks like that replaced hand-rolled flag parsing with standard flag parsing, and presumably added support for --.

I'll remove the -- from our command. The risk is that users could possibly configure a remote URI like --destroy-everything-everywere and cause us to run git ls-remote --destory-everything-everywere, but none of the flags available to git ls-remote appear to be dangerous and there are other checks on the URI values.

epriestley added a revision: D17508: Don't use "--" to separate flags and arguments in "git ls-remote".Mar 19 2017, 12:32 AM
epriestley closed this task as Resolved by committing rPd19fc2335e62: Don't use "--" to separate flags and arguments in "git ls-remote".Mar 19 2017, 12:54 AM
epriestley added a commit: rPd19fc2335e62: Don't use "--" to separate flags and arguments in "git ls-remote".
epriestley added a commit: rP56dd1b297c3e: (stable) Don't use "--" to separate flags and arguments in "git ls-remote".
epriestley added a comment.Mar 19 2017, 12:55 AM

This should now be fixed in HEAD of master. I've also cherry-picked it to stable. Thanks for the report!

apierritba awarded a token.Mar 21 2017, 2:37 PM
epriestley mentioned this in T12961: [CVE-2017-1000117, et al] Git, Mercurial and Subversion could all execute arbitrary commands when interacting with malicious SSH URIs (`ssh://-...`).Aug 11 2017, 12:50 PM
Phabricator · Built by Phacility