Improve permissions adjustment logic #27
Conversation
- First, attempts permissions adjustment without elevated privileges - Fallback to privilege elevation tools (`sudo` or `doas`) if available - If applicable, print permissions adjustment error and guide user
miclaus
force-pushed
the
feature/permissions-adjustment-logic
branch
from
c25ed54 to
e23ef22
Compare
|
I'm scared to keep tinkering with this user stuff much more unless there is a serious problem with for most developers using Sail. 😬 |
|
@taylorotwell I totally get the concern around "tinkering with this user stuff". That said, I believe sudo isn't always necessary here, and we can set permissions without it in most cases. My change first tries to set permissions without sudo. If that fails, it gracefully falls back to using sudo, preserving the script's original behavior when permissions can't be set without sudo (or doas). The current script isn't ideal because it always prompts developers for their password, even when it's unnecessary. And even if they provide it, it might not work in certain cases (as mentioned above). My version avoids the need for a password when permissions can be set without sudo, making it more developer-friendly. Think about how many security-conscious developers have probably wondered, "Why does this script need my password? What am I giving it access to?" Some, myself included, likely had to kill the script, inspect what it was doing with elevated permissions, and then rerun it, which interrupts the workflow. I'm confident this change will improve the developer experience, by avoiding asking developers for their password, when it's not needed. It should work smoothly across environments (macOS, Linux, WSL2), but feedback from other developers could help confirm that. |
|
@taylorotwell Just had the same issue with yet another project:
|
Previously, permissions were exclusively adjusted using privilege elevation tools (
sudoordoas). This causes an issue on systems where the current user is not in the sudoers file (e.g., on macOS, where the current user is a standard user, not an administrator), resulting in an error like: "User is not in the sudoers file. This incident has been reported to the administrator."This PR fixes and improves this by first attempting to adjust permissions without elevated privileges. If that fails, it falls back to using
sudoordoas(if available). If neither method succeeds, the user is informed and encouraged to try running the application, as it might still work without adjusted permissions. (In the example mentioned, the application worked correctly even though permissions adjustment failed for the standard user.)