Fork Bomb Fixes

[directionless]

In #590 we expanded checkExecutable to invoke the executable to verify correctness. When taken with launcher's inclination to exec newer versions, this created a fork bomb. There was a fix added in #595, but that's not adequate.

This PR attempts to address this in three ways. While any one of them should be adequate, we may as one run all three.

First, we add an environmental variable that skips the initial re-exec functionality. This isn't documented, and is meant for internal use only. Frustratingly, we cannot just add it to flags, as we can't parse flags prior to exec.

Second, this risk primarily comes from the FindNewestSelf code path. This introduces withRunningExectuable as a mechanism to indicate that we should not check a given path.

Third, if we can determine os.Executable, don't exec it. This is duplicative with the second option. But, it comes at it from a different direction. Seems better to have both

[blaedj]

Is there a good reason not to parse options before running the findNewest check?

It would be nice if the --version option/subcommand (and also --usage, really) skipped checking for updates, since if I'm running the binary with that command I really just want to know the version of the binary that I called

[blaedj]

If we don't want to parse the options first, could we put the isSubCommand before the update check? If we did that, we could call launcher version instead of launcher --version when we see if the binary is executable, the version subcommand does the same thing as the --version flag without requiring us to parse options.

Note that I may be missing something fundamental here too..

[directionless]

I struggled a bunch with these questions...

The standard library's flag parsing has, AFAIK, no way to ignore unknown options. They will trigger an os.Exit(2) This means that we cannot nicely parse flags or subcommands before invoking the newest. Else new options trigger fatal errors. (I would love to parse it and ignore the errors)

I thought about whether to move the --version prior to FindNewest. I deliberately have them in this order. I think the most common use case, even for us, is trying to understand what command will be operating. Thus, it pulls the newest first. I see this as a small step towards some deeper refactoring. Launcher's top level should be seen as a launcher and an updater. Which components for osquery and tables. (Even if it's the same binary, thinking as these as split helps)

And while I understand the appeal of making version different from --version I think that confusion is a bad idea. Consistent is probably better.

Maybe another thing to remember, is the whole FindNewest implementation replaces one that would have copied the binary over the current running on.

[blaedj]

Ok that all makes sense! We could reason about a dedicated 'test executable' subcommand (the subcommand check doesn't require flag parsing) that might be too trivial an execution path to be useful..

[directionless]

about a dedicated 'test executable' subcommand

I thought about that too! As is, the code path is the same for testing osqueryd and launcher. Making a dedicated launcher test command (even if --version-no-updates) requires splitting those code paths. The ENV was a workaround.

[blaedj]

lgtm once tests are passing