Per https://bugzilla.mozilla.org/show_bug.cgi?id=1448137#c5, I think the parser should be modified to not barf on unexpected input lines. Otherwise this will continue to break when Mercurial adds new labels to hg log output. Using HGPLAIN does offer you some protection here. But it is no guarantee against future additions.

The reason the upstream projects aren't using -- is that it isn't portable. For example, Putty's ssh doesn't support it.

That same code I pointed to for Mercurial also seems to perform Git working copy checkouts. Although I can't recall Git's semantics for automatically updating submodules (because I don't use them). It is worth auditing.

Note that Phabricator can manifest Mercurial working directories. See executeMercurialUpdate() in src/applications/repository/engine/PhabricatorRepositoryPullEngine.php. It does this when pulling non-hosted repos. I know this occurs when observing repos. Not sure where else this code is used.

https://www.mercurial-scm.org/repo/hg/file/default/contrib/phabricator.py is a Mercurial extension that allows you to send a series of changesets to Phabricator by calling Conduit APIs directly. The extension is currently tailored for the use cases of the Mercurial project itself, isn't distributed with Mercurial, and may require Mercurial 4.3. And it obviously bypasses Phabricator's built-in Mercurial server. But if you are willing to live with the caveats, you may find it a suitable workaround.

Heads up: Mercurial 4.1 (to be released ~February 1) introduces media type negotiation in the HTTP protocol. https://www.mercurial-scm.org/repo/hg/rev/753b9d43ca81 captures the wire protocol changes and the commit message explains why things are done the way they are.

Running vanilla Mercurial 3.4 should not be recommended as all vanilla versions older than 3.7.3 are vulnerable to a trio of CVEs, the most likely exploitable being CVE-2016-3630. Instead, Mercurial <4.0 can be configured to disable bundle2 via the experimental.bundle2-exp=false config option and Mercurial >=4.0 can do the same via devel.legacy.exchange=bundle1. Both of these options are undocumented. I discourage their use. But as a workaround for Phabricator, they are preferred to exposing clients to known security bugs.

It should be possible to determine whether an operation is read or write by the command name: a command either performs writes or it doesn't. The only write commands you should see are unbundle and pushkey. While it might be possible for batch to contain pushkey operations, I don't think that's done today (but I'd have to audit the code to be sure). One thing you should be able to rely on is the HTTP method: GET requests should never perform any significant writes (although some cache files may be written to as a result of resolving data, but that shouldn't be important). It should be perfectly valid to treat GET and POST differently. Although if you route requests to different endpoints within the same TCP/HTTP stream, inconsistent state of a read-only mirror from a master could lead to race conditions (e.g. push fails because a mirror is out of sync). If you route by HTTP request method, you should not need to know anything about the Mercurial wire protocol.

While I'm hesitant to throw stones here and don't really have any context on bundle2 or the wire protocol in general and there may be plenty of reasons that it works like it does, my general impression is that the Mercurial wire protocol is significantly under-designed, and is now on at least the 3rd and possibly 4th iteration of hacks to try to address the lack of actual protocol design.

This applies to the Persona provider (T3958) as well.