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

Fix uninstall for Warp and Box #23652

Merged
merged 5 commits into from
Nov 11, 2024
Merged

Fix uninstall for Warp and Box #23652

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
09e62f7
Add fix for warp uninstall failure
dantecatalfamo Nov 6, 2024
5e3fc23
Partial fox for Box uninstaller
dantecatalfamo Nov 7, 2024
83696eb
Add changes/
dantecatalfamo Nov 8, 2024
9019694
Add pre and post custom uninstall scripts
dantecatalfamo Nov 8, 2024
a376029
Update changes/22773-fma-uninstall-fix
dantecatalfamo Nov 11, 2024
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
1 change: 1 addition & 0 deletions changes/22773-fma-uninstall-fix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- Fix some cases where Fleet Managed Apps generated incorrect uninstall scripts
dantecatalfamo marked this conversation as resolved.
Show resolved Hide resolved
10 changes: 9 additions & 1 deletion server/mdm/maintainedapps/apps.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,15 @@
{
"identifier": "box-drive",
"bundle_identifier": "com.box.desktop",
"installer_format": "pkg"
"installer_format": "pkg",
"pre_uninstall_scripts": [
"(cd /Users/$LOGGED_IN_USER; sudo -u $LOGGED_IN_USER fileproviderctl domain remove -A com.box.desktop.boxfileprovider)",
Copy link
Member Author

Choose a reason for hiding this comment

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

We should probably have a function that does this as part of our other toolbox, calling sudo -u $user from /root will make the shell freak out

"(cd /Users/$LOGGED_IN_USER; sudo -u $LOGGED_IN_USER /Applications/Box.app/Contents/MacOS/fpe/streem --remove-fpe-domain-and-archive-unsynced-content Box)",
"(cd /Users/$LOGGED_IN_USER; sudo -u $LOGGED_IN_USER /Applications/Box.app/Contents/MacOS/fpe/streem --remove-fpe-domain-and-preserve-unsynced-content Box)",
"(cd /Users/$LOGGED_IN_USER; defaults delete com.box.desktop)",
"echo \"${LOGGED_IN_USER} ALL = (root) NOPASSWD: /Library/Application\\ Support/Box/uninstall_box_drive_r\" >> /etc/sudoers.d/box_uninstall"
Copy link
Member

Choose a reason for hiding this comment

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

That's so that an explicit sudo is not required to run the uninstall_box_drive_r binary? We can't do the explicit sudo in the uninstall script itself?

Copy link
Member Author

Choose a reason for hiding this comment

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

That's done because there's a sudo in the /Library/Application Support/Box/uninstall_box_drive script that the brew artifact runs. The problem is that without that line sudo fails because the user can't be asked for a password.

Here's an excerpt from the script:

if [ 0 -eq "$user_level_uninstall" ]; then
    # Do the work that requires sudo - this work is placed in a separate script to support
    # developers. On Jenkins passwordless sudo is enabled. But on developer machines we
    # shouldn't require a blanket password-less sudo. This is an issue because developers
    # run Chimp locally and Chimp executes these uninstall scripts and it's often the case
    # that there's no STDIN for sudo to use to get the password. Thus, by placing all the
    # logic that requires sudo in a script called uninstall_box_drive_r... the developers
    # can add an password-exemption for just that script in /etc/sudoers that looks like:
    #     <username> ALL = (root) NOPASSWD: /Library/Application\ Support/Box/uninstall_box_drive_r
    #
    sudo "${0%/*}/"uninstall_box_drive_r $fuse_failure_quits $delete_logs
fi

Copy link
Member Author

Choose a reason for hiding this comment

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

It passes in some arguments from the main uninstall script. In the _r script, it asks that you don't call it independently

Copy link
Member

Choose a reason for hiding this comment

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

Gotcha, thanks!

],
"post_uninstall_scripts": ["rm /etc/sudoers.d/box_uninstall"]
},
{
"identifier": "brave-browser",
Expand Down
31 changes: 19 additions & 12 deletions server/mdm/maintainedapps/ingest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,11 @@ import (
var appsJSON []byte

type maintainedApp struct {
Identifier string `json:"identifier"`
BundleIdentifier string `json:"bundle_identifier"`
InstallerFormat string `json:"installer_format"`
Identifier string `json:"identifier"`
BundleIdentifier string `json:"bundle_identifier"`
InstallerFormat string `json:"installer_format"`
PreUninstallScripts []string `json:"pre_uninstall_scripts"`
PostUninstallScripts []string `json:"post_uninstall_scripts"`
}

const baseBrewAPIURL = "https://formulae.brew.sh/api/"
Expand Down Expand Up @@ -164,6 +166,9 @@ func (i ingester) ingestOne(ctx context.Context, app maintainedApp, client *http
if err != nil {
return ctxerr.Wrapf(ctx, err, "create install script for cask %s", app.Identifier)
}

cask.PreUninstallScripts = app.PreUninstallScripts
cask.PostUninstallScripts = app.PostUninstallScripts
uninstallScript := uninstallScriptForApp(&cask)

_, err = i.ds.UpsertMaintainedApp(ctx, &fleet.MaintainedApp{
Expand All @@ -182,15 +187,17 @@ func (i ingester) ingestOne(ctx context.Context, app maintainedApp, client *http
}

type brewCask struct {
Token string `json:"token"`
FullToken string `json:"full_token"`
Tap string `json:"tap"`
Name []string `json:"name"`
Desc string `json:"desc"`
URL string `json:"url"`
Version string `json:"version"`
SHA256 string `json:"sha256"`
Artifacts []*brewArtifact `json:"artifacts"`
Token string `json:"token"`
FullToken string `json:"full_token"`
Tap string `json:"tap"`
Name []string `json:"name"`
Desc string `json:"desc"`
URL string `json:"url"`
Version string `json:"version"`
SHA256 string `json:"sha256"`
Artifacts []*brewArtifact `json:"artifacts"`
PreUninstallScripts []string `json:"-"`
PostUninstallScripts []string `json:"-"`
}

// brew artifacts are objects that have one and only one of their fields set.
Expand Down
23 changes: 15 additions & 8 deletions server/mdm/maintainedapps/scripts.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,15 @@ func uninstallScriptForApp(cask *brewCask) string {
}
case len(artifact.Uninstall) > 0:
sortUninstall(artifact.Uninstall)
if len(cask.PreUninstallScripts) > 0 {
sb.Write(strings.Join(cask.PreUninstallScripts, "\n"))
}
for _, u := range artifact.Uninstall {
processUninstallArtifact(u, sb)
}
if len(cask.PostUninstallScripts) > 0 {
sb.Write(strings.Join(cask.PostUninstallScripts, "\n"))
}
case len(artifact.Zap) > 0:
sortUninstall(artifact.Zap)
for _, z := range artifact.Zap {
Expand Down Expand Up @@ -179,11 +185,11 @@ func processUninstallArtifact(u *brewUninstall, sb *scriptBuilder) {
if u.Script.IsOther {
addUserVar()
for _, path := range u.Script.Other {
sb.Writef(`sudo -u "$LOGGED_IN_USER" '%s'`, path)
sb.Writef(`(cd /Users/$LOGGED_IN_USER && sudo -u "$LOGGED_IN_USER" '%s')`, path)
}
} else if len(u.Script.String) > 0 {
addUserVar()
sb.Writef(`sudo -u "$LOGGED_IN_USER" '%s'`, u.Script.String)
sb.Writef(`(cd /Users/$LOGGED_IN_USER && sudo -u "$LOGGED_IN_USER" '%s')`, u.Script.String)
}

process(u.PkgUtil, func(pkgID string) {
Expand Down Expand Up @@ -365,15 +371,15 @@ const removeLaunchctlServiceFunc = `remove_launchctl_service() {
local booleans=("true" "false")
local plist_status
local paths
local sudo
local should_sudo

echo "Removing launchctl service ${service}"

for sudo in "${booleans[@]}"; do
for should_sudo in "${booleans[@]}"; do
plist_status=$(launchctl list "${service}" 2>/dev/null)

if [[ $plist_status == \{* ]]; then
if [[ $sudo == "true" ]]; then
if [[ $should_sudo == "true" ]]; then
sudo launchctl remove "${service}"
else
launchctl remove "${service}"
Expand All @@ -387,15 +393,15 @@ const removeLaunchctlServiceFunc = `remove_launchctl_service() {
)

# if not using sudo, prepend the home directory to the paths
if [[ $sudo == "false" ]]; then
if [[ $should_sudo == "false" ]]; then
for i in "${!paths[@]}"; do
paths[i]="${HOME}${paths[i]}"
done
fi

for path in "${paths[@]}"; do
if [[ -e "$path" ]]; then
if [[ $sudo == "true" ]]; then
if [[ $should_sudo == "true" ]]; then
sudo rm -f -- "$path"
else
rm -f -- "$path"
Expand Down Expand Up @@ -450,6 +456,7 @@ const trashFunc = `trash() {
local logged_in_user="$1"
local target_file="$2"
local timestamp="$(date +%Y-%m-%d-%s)"
local rand="$(jot -r 1 0 99999)"

# replace ~ with /Users/$logged_in_user
if [[ "$target_file" == ~* ]]; then
Expand All @@ -461,7 +468,7 @@ const trashFunc = `trash() {

if [[ -e "$target_file" ]]; then
echo "removing $target_file."
mv -f "$target_file" "$trash/${file_name}_${timestamp}"
mv -f "$target_file" "$trash/${file_name}_${timestamp}_${rand}"
else
echo "$target_file doesn't exist."
fi
Expand Down
Loading