magic-folder CLI parsing error

[warner]

In a demo today, Zooko tried to join a magic-folder instance with the following (incorrect) command:

% tahoe magic-folder join "URI:DIR2-RO:geydj2tsjteoxiezxwxmff46jq:nfczfkluafttupukia2umqthm2mqh3lu5olouby4nxcdcc36s3fa+URI:DIR2:yy5l52jcuj7k5khumae7stbtry:feo2jnoqcnhktoyggewibqjbr26unjajnudogaa2sbxhafnlvidq" --basedir=c 

This is wrong for two reasons:

• the --basedir option needs to come between magic-folder and join, rather than at the end of the command
• it lacks the LOCALDIR positional argument, which is supposed to come after the INVITATION-CODE pair-of-dircaps

What happened was even worse:

• the command modified tahoe.cfg in the default basedir (~/.tahoe/), rather than the intended ./c
• the command recorded --basedir=c as the name of the directory to sync. As in ./--basedir=c/files.

I'm somewhat surprised that twisted.python.usage (the argument parser) was willing to accept a -- -prefixed string as a positional argument.. it's a legal (if ill-advised) pathname, so it makes sense that it "worked", but leads to bad failure modes like this one. Some argument parsers (maybe argparse?) remove all ---prefixed strings first, attempt to parser them as options, then use only the remaining arguments to fill the positional slots. Such a parser would reject that command twice: once for having no --basedir option (at that level of the subcommands), and again for failing to have a positional argument at the expected location (since --basedir=c would have been removed by that point).

I'm not sure what tahoe can do about this, but it's tempting to make a rule that filenames must start with one of [a-zA-Z0-9\.\/], or at least reject filenames that start with - or -- (and remind users to say ./-foo if they really want that).

[dawuud]

in this dev branch i've made a simple modification to JoinOptions class to reject filepaths starting with -.

​https://github.com/david415/tahoe-lafs/tree/2533.fix-cli-parsing.0

however the testing API we use for the CLI is not sufficient to test this CLI failure case. it makes me think that in general we need to rewrite all the CLI tests so that they actually stand a chance of triggering usage errors if arguments are malformed.

[daira]

I'm highly skeptical of this approach to a fix, because the same problem can happen with any command; it is purely a coincidence that it happened with tahoe magic-folder join.

A solution should therefore apply consistently to all places that accept local paths, at least. A possible approach is to change encodingutil.argv_to_abspath (which tahoe magic-folder join should be using in order to fix #2513).

[daira]

Note however that changing argv_to_abspath would not catch cases where an argument is not known in advance to be a local path (e.g. where it is a Tahoe path, or either a Tahoe or local path).

[dawuud]

in my dev branch some progress... although the test_create_invite_join fails

​https://github.com/david415/tahoe-lafs/tree/2533.fix-cli-parsing.1

[dawuud]

all tests passing my in latest commit. please review.

[daira]

This branch was reviewed and is included on ​https://github.com/tahoe-lafs/tahoe-lafs/commits/2438.magic-folder-stable.4.

[daira]

Also I just smoke-tested it and it appears to work as intended.