The check for running WP-CLI as a root user inadvertently is running pretty late and allows for mechanisms like the --exec flag to be executed already before the root user is being checked for.

This PR moves the logic for this check around so that it is included as its own step in the bootstrapping steps and happens at the earliest occasion, before other flags are being processed.

Is there a way to add tests for this that mock the return value of posix_geteuid()?

We could add a helper that wraps the posix_geteuid() function (and does the compatibility check, returning something like -1 if not available).

Then the tests could check the order/timing of the root block.

Does no one have a solution to this problem?

@swissspidy I see this issue has been resolved but no one has closed it yet

@phanduynam I don't know what you mean 🤔 This is a pull request, not an issue. And it's still open, and not merged. So it's not "resolved".

@swissspidy I don't quite understand, all the tests are passed, why not close?

If the only thing this is waiting on is a test, I believe this would cover everything mentioned above:

  Feature: Bootstrap WP-CLI

  Scenario: Test early root detection

    Given an empty directory
    And a include.php file:
    """
    <?php
      namespace WP_CLI\Bootstrap;

      // To override posix_geteuid in our namespace
      function posix_geteuid() {
        return 0;
      }
    ?>
    """

    And I try `WP_CLI_EARLY_REQUIRE=include.php wp cli version --debug`

    Then STDERR should contain:
    """
    WP_CLI\Bootstrap\CheckRoot
    """

    And STDERR should not contain:
    """
    WP_CLI\Bootstrap\IncludeRequestsAutoloader
    """

    And STDERR should contain:
    """
    YIKES!
    """

Happy to add to the PR if that seems OK

@mrsdizzie Yes, that looks like what we need. If you add it here, I'll merge.

I'll put this test in to move forward with the release. Thanks for the prep work on this, @mrsdizzie !

Confirmed that the test fails without and succeeds with the fix.

Ignoring code coverage here to get it merged.

This seems to break passing --allow-root=1. Previously, it was a simple truthy check, now it's a strict true check.