Skip to content
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.

Sign up for GitHub

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

Make permission check more lenient. #163

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Make permission check more lenient. #163

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 9 additions & 3 deletions autoswitch_virtualenv.plugin.zsh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,15 +179,21 @@ function check_venv()
file_owner="$(/usr/bin/stat -f %u "$venv_path")"
file_permissions="$(/usr/bin/stat -f %OLp "$venv_path")"
fi
if [[ -d "$venv_path" ]]; then
default_permissions=777
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There might be something I'm missing here - so please correct me if I am wrong

But this seems to change the plugin to require the activated file to be executable, writeable and readable by everyone on the system?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah wait - you're specifically checking if its a directory here? I'm a bit lost about what we are covering here then?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code is to figure out what the default permissions of the file would be:

  • If it's a directory, e.g. a .venv folder, the default permissions are 777 & ~umask
  • If it's a regular file, e.g. a Pipfile, the default permissions are 666 & ~umask

So I'm checking for whether it's a directory to figure out what octal value I need to bit-wise AND with umask.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MichaelAquilina : Would you be willing to accept an updated/rebased version of this PR? I'm been getting hit with this quite often recently when working with a poetry based env. A lot of the poetry commands seem to rewrite poetry.lock, causing its permission to be reverted to defaults. Since those 664 on my system (Ubuntu 20.04 Desktop), I get this warning every time after the file has been touched :(

Copy link

@philer-jambit philer-jambit May 9, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any update on this? I've also run into the poetry.lock file permissions issue.

Edit: For anyone else who gets here: It appears that poetry respects the permissions of the parent directory, so if you change your project folder to 644 or stricter it seems to work.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side note: I've been using my patch for quite a while, and it's much better, but I've still been running into the problem occasionally (not sure why and don't care enough to track down what's going on), so I'm inclined to just remove the check in my fork entirely... still think it's a useful check in the case where $venv_path is an actual venv folder but for me, it never is.

else
default_permissions=666
fi
default_permissions="$(printf %o "$((~8#$(umask) & 8#$default_permissions))")"

if [[ "$file_owner" != "$(id -u)" ]]; then
printf "AUTOSWITCH WARNING: Virtualenv will not be activated\n\n"
printf "Reason: Found a $AUTOSWITCH_FILE file but it is not owned by the current user\n"
printf "Change ownership of ${PURPLE}$venv_path${NORMAL} to ${PURPLE}'$USER'${NORMAL} to fix this\n"
elif ! [[ "$file_permissions" =~ ^[64][04][04]$ ]]; then
elif [[ "$file_permissions" != "$default_permissions" ]]; then
printf "AUTOSWITCH WARNING: Virtualenv will not be activated\n\n"
printf "Reason: Found a $AUTOSWITCH_FILE file with weak permission settings ($file_permissions).\n"
printf "Run the following command to fix this: ${PURPLE}\"chmod 600 $venv_path\"${NORMAL}\n"
printf "Reason: Found a $AUTOSWITCH_FILE file with non-default permission settings ($file_permissions).\n"
printf "Run the following command to fix this: ${PURPLE}\"chmod $default_permissions $venv_path\"${NORMAL}\n"
else
if [[ "$venv_path" == *"/Pipfile" ]]; then
if type "pipenv" > /dev/null && _activate_pipenv; then
Expand Down