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.

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

[BUG] trash moves files to wrong user when executing with elevated privileges #11

Open
JayBrown opened this issue Mar 5, 2021 · 11 comments

Comments

@JayBrown
Copy link

JayBrown commented Mar 5, 2021

Let's say we have a file /Users/Shared/foo.txt owned by root:wheel.

Moving this to the currently logged-in user's trash works fine in Finder: you just have to enter your admin password.

Problems arise when using trash instead:

  • sudo trash /Users/Shared/foo doesn't work, and that's probably OK, but a question nonetheless: is there a reason why sudo trash doesn't work?
  • what works fine is osascript -e 'do shell script "trash /Users/Shared/foo" with administrator privileges' … but the file actually doesn't wind up in the the user's trash directory ~/.Trash, as it does in Finder, but in /private/var/root/.Trash.

So it seems that the trash CLI is only checking which user is executing the command (0), but not if that's also the same as the logged-in user, e.g. 501.

If that's true, the trash CLI should determine the actual home folder of the executing user by running the equivalent of eval echo "~$(scutil <<< \"show State:/Users/ConsoleUser\" | awk '/Name :/ && ! /loginwindow/ { print $3 }')", and then use the trash directory of that user.

This bug should be fixed, because users should be able to move non-writable files to their own trash, in the same way it's possible in the Finder.

@sindresorhus
Copy link
Owner

Currently, it's just reverting back to non-sudo user:

/// Revert back to original user if sudo.
static func revertSudo() {
guard
let sudoUIDstring = ProcessInfo.processInfo.environment["SUDO_UID"],
let sudoUID = uid_t(sudoUIDstring)
else {
return
}
setuid(sudoUID)
}
}

I would be happy to see this improved, but it's not something I plan to work on.


Related issue: sindresorhus/trash-cli#28

@sindresorhus
Copy link
Owner

sindresorhus commented Mar 6, 2021

This is how to get the console username:

import SystemConfiguration

func getConsoleUser() -> String? {
	SCDynamicStoreCopyConsoleUser(nil, nil, nil) as String?
}

getConsoleUser()

And: https://github.com/Nikhil0487/privileges-CLI/blob/32686cf01f26d697479bca978d129eff4d7d538e/privilegehelper/privilegehelper/com_privilege_helper.swift#L128-L140

@JayBrown
Copy link
Author

JayBrown commented Mar 6, 2021

I have read the issue you pointed to (#28), and your reasoning there makes sense, also what @russelldavis wrote. Personally I think it wouldn't be a good idea to change the ownership or permissions of the parent directory of the file specified for removal, when the user runs sudo trash or the above osascript command with admin privileges.

Maybe, for these cases, you could rather add an -For --finder argument, and when trash -F foo fails because of permissions when using the system API, trash will just call Finder to activate & perform the deletion, and Finder would then ask the user for the password in the GUI. That would probably be the safest way.

Or you concoct some weird workaround that relies on neither the system API nor the Finder. But the users can probably do that themselves with a shellscript using sudo somehow. That's what I'm testing now, but I also kinda tend to just calling Finder instead.

@sindresorhus
Copy link
Owner

I think falling back to Finder automatically if it fails and the terminal is interactive is an ok workaround.

@JayBrown
Copy link
Author

JayBrown commented Mar 6, 2021

Ah, yes, the interactive issue. Maybe you could use the trash -F argument for users who run trash in shell scripts in the background (detached), but still need user interaction.

Another thing would be the following: when executing trash foo bar, and both foo & bar are protected, the program will fail already with the first one. Maybe trash could cycle through the list of files and try to move them to the trash, but continue if the system API fails, and come back to the ones that failed at the end, to then remove the remaining files by calling Finder.

@JayBrown
Copy link
Author

JayBrown commented Mar 6, 2021

If anyone's interested… this should probably do it in a shell script context:

rmlist=""
for filepath in "$@"
do
	! trash "$filepath" &>/dev/null && rmlist="$rmlist\n$filepath"
done
rmlist=$(echo "$rmlist | grep "^$")
if [[ $rmlist ]] ; then
	osascript 2>/dev/null << EOR
set theFiles to the paragraphs of "$rmlist"
repeat with aFile in theFiles
	set aFile's contents to (POSIX file aFile) as Unicode text
end repeat
theFiles
tell application "Finder"
   move theFiles to the trash
end tell
EOR
fi

@sindresorhus
Copy link
Owner

Maybe trash could cycle through the list of files and try to move them to the trash, but continue if the system API fails

It's much faster to pass all paths to the system API than to iterate through them.

@JayBrown
Copy link
Author

JayBrown commented Mar 6, 2021

Definitely. I use it in a shell script anyway and cycle through the filepaths, similar to the snippet above… so I have my own workaround for that. ;) The only implementation I could think of is another argument, e.g. trash -c or trash --cycle, but not as the default functionality.

@eugenesvk
Copy link

Have you by any chance found a non-Finder solution?
I don't have a running Finder process and would prefer not to launch one for moving files to the proper trash folder

@JayBrown
Copy link
Author

JayBrown commented Nov 1, 2022

I have disabled Finder too, and I'm using the following workaround.

When I need the Finder, I'm toggling it with the following shell script that's mapped to my TouchBar using BetterTouchTool. (You can also map it differently, of course.)

#!/bin/zsh
# toggleFinder.sh
tempdir="$HOME/.cache/Finder"
! [[ -d $tempdir ]] && mkdir -p "$tempdir" &>/dev/null
if pgrep -x Finder &>/dev/null ; then
	osascript -e 'tell application "Finder" to quit' &>/dev/null
	rm -f "$tempdir/manual" 2>/dev/null
else
	touch "$tempdir/manual"
	osascript -e 'tell application "Finder" to activate' &>/dev/null
fi
exit

Additionally, I have a enabled a user LaunchAgent which runs every 30 seconds and auto-quits Finder whenever Finder has launched without the above toggleFinder script:

#!/bin/zsh
# checkFinder.sh
! pgrep -x Finder &>/dev/null && exit
[[ -e "$HOME/.cache/Finder/manual" ]] && exit
osascript -e 'tell application "Finder" to quit' &>/dev/null
localdate=$(date)
echo "Finder automatic quit: $localdate"
exit

The .plist for the agent:

<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
	<key>Label</key>
	<string>local.checkFinder</string>
	<key>LowPriorityBackgroundIO</key>
	<true/>
	<key>LowPriorityIO</key>
	<true/>
	<key>Nice</key>
	<integer>20</integer>
	<key>ProcessType</key>
	<string>Background</string>
	<key>Program</key>
	<string>/usr/local/bin/checkFinder.sh</string>
	<key>RunAtLoad</key>
	<true/>
	<key>StandardErrorPath</key>
	<string>/tmp/local.checkFinder.stderr</string>
	<key>StandardOutPath</key>
	<string>/tmp/local.checkFinder.stdout</string>
	<key>StartInterval</key>
	<integer>30</integer>
</dict>
</plist>

In addition, I'm using macos-trash as part of a workflow to delete my files, and in that workflow I've added a simple check: if Finder is already running before trash commences, leave Finder running at the end; if Finder is not running, quit Finder at the end using osascript.

@eugenesvk
Copy link

Thanks for your detailed guide on how to get rid of the pesky Finder, but am I correct to assume from this that there is no known fix, i.e., it's impossible to correctly identify the user's Trash and move files there instead of using the system Trash?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants