-
Notifications
You must be signed in to change notification settings - Fork 994
Check for root earlier #5987
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Check for root earlier #5987
Conversation
/cc @johnbillion |
dafa9b4
to
8083d35
Compare
Is there a way to add tests for this that mock the return value of |
We could add a helper that wraps the 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? |
Codecov ReportAttention: Patch coverage is
📢 Thoughts on this report? Let us know! |
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 |
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.