From f9526638b2a1c742b3e5991012b8b143b441a6a6 Mon Sep 17 00:00:00 2001 From: Kirk Date: Tue, 31 Aug 2021 22:49:11 +0000 Subject: [PATCH] Bug 1726659 - Prevent update UI on a Silent Restart r=agashlin Additionally should prevent elevated updates from running at all on a silent restart, since this will always require UI. Differential Revision: https://phabricator.services.mozilla.com/D123568 --- .../startup/public/nsIAppStartup.idl | 4 ++ toolkit/mozapps/update/updater/updater.cpp | 67 +++++++++++++------ toolkit/xre/nsUpdateDriver.cpp | 34 ++++++++-- 3 files changed, 79 insertions(+), 26 deletions(-) diff --git a/toolkit/components/startup/public/nsIAppStartup.idl b/toolkit/components/startup/public/nsIAppStartup.idl index 59dc3551a11c..f4487ae39b98 100644 --- a/toolkit/components/startup/public/nsIAppStartup.idl +++ b/toolkit/components/startup/public/nsIAppStartup.idl @@ -126,6 +126,10 @@ interface nsIAppStartup : nsISupports * Firefox into this state. But we do occasionally want it to restart this * way. Passing this flag prevents Firefox from opening any windows when it * restarts. + * + * Note that, if there is an application update pending, this also silences + * the update. This means that no UI will be shown including elevation + * dialogs (potentially preventing the update from being installed). */ const uint32_t eSilently = 0x100; diff --git a/toolkit/mozapps/update/updater/updater.cpp b/toolkit/mozapps/update/updater/updater.cpp index 53be1209275d..1db6f092fab4 100644 --- a/toolkit/mozapps/update/updater/updater.cpp +++ b/toolkit/mozapps/update/updater/updater.cpp @@ -285,14 +285,21 @@ static bool sStagedUpdate = false; static bool sReplaceRequest = false; static bool sUsingService = false; -// Whether the callback app is a background task. If true then the updater is -// likely being run as part of a background task and therefore shouldn't: +// Normally, we run updates as a result of user action (the user started Firefox +// or clicked a "Restart to Update" button). But there are some cases when +// we are not: +// a) The callback app is a background task. If true then the updater is +// likely being run as part of a background task. +// The updater could be run with no callback, but this only happens +// when performing a staged update (see calls to ProcessUpdates), and there +// are already checks for sStagedUpdate when showing UI or elevating. +// b) On macOS, the environment variable MOZ_APP_SILENT_RESTART is set and not +// empty. This is set when Firefox had no windows open for a while and +// restarted to apply updates. +// +// In these cases, the update should be installed silently, so we shouldn't: // a) show progress UI // b) prompt for elevation -// -// The updater could be run with no callback, but this only happens -// when performing a staged update (see calls to ProcessUpdates), and there -// are already checks for sStagedUpdate when showing UI or elevating. static bool sUpdateSilently = false; #ifdef XP_WIN @@ -352,13 +359,8 @@ static NS_tchar* mstrtok(const NS_tchar* delims, NS_tchar** str) { return ret; } -#if defined(TEST_UPDATER) -# define HAS_ENV_CHECK 1 -#elif defined(MOZ_MAINTENANCE_SERVICE) -# define HAS_ENV_CHECK 1 -#endif - -#if defined(HAS_ENV_CHECK) +#if defined(TEST_UPDATER) || defined(MOZ_MAINTENANCE_SERVICE) || \ + defined(XP_MACOSX) static bool EnvHasValue(const char* name) { const char* val = getenv(name); return (val && *val); @@ -2704,6 +2706,13 @@ bool ShouldRunSilently(int argc, NS_tchar** argv) { } } #endif // MOZ_BACKGROUNDTASKS + +#ifdef XP_MACOSX + if (EnvHasValue("MOZ_APP_SILENT_RESTART")) { + return true; + } +#endif // XP_MACOSX + return false; } @@ -2976,19 +2985,33 @@ int NS_main(int argc, NS_tchar** argv) { if (!isElevated && !IsRecursivelyWritable(argv[2])) { // If the app directory isn't recursively writeable, an elevated update is // required. - UpdateServerThreadArgs threadArgs; - threadArgs.argc = argc; - threadArgs.argv = const_cast(argv); + if (sUpdateSilently) { + // An elevated update always requires an elevation dialog, so if we are + // updating silently, don't do an elevated update. + // This means that we cannot successfully perform silent updates from + // non-admin accounts on a Mac. + // It also means that we cannot silently perform the first update by an + // admin who was not the installing user. Once the first update has been + // installed, the permissions of the installation directory should be + // changed such that we don't need to elevate in the future. + // Firefox shouldn't actually launch the updater at all in this case. This + // is defense in depth. + WriteStatusFile(SILENT_UPDATE_NEEDED_ELEVATION_ERROR); + fprintf(stderr, + "Skipping update to avoid elevation prompt from silent update."); + } else { + UpdateServerThreadArgs threadArgs; + threadArgs.argc = argc; + threadArgs.argv = const_cast(argv); - Thread t1; - if (t1.Run(ServeElevatedUpdateThreadFunc, &threadArgs) == 0) { - // Show an indeterminate progress bar while an elevated update is in - // progress. - if (!sUpdateSilently) { + Thread t1; + if (t1.Run(ServeElevatedUpdateThreadFunc, &threadArgs) == 0) { + // Show an indeterminate progress bar while an elevated update is in + // progress. ShowProgressUI(true); } + t1.Join(); } - t1.Join(); LaunchCallbackAndPostProcessApps(argc, argv, callbackIndex, false); return gSucceeded ? 0 : 1; diff --git a/toolkit/xre/nsUpdateDriver.cpp b/toolkit/xre/nsUpdateDriver.cpp index e9558783379a..a5b1cb2244e6 100644 --- a/toolkit/xre/nsUpdateDriver.cpp +++ b/toolkit/xre/nsUpdateDriver.cpp @@ -441,6 +441,35 @@ static void ApplyUpdate(nsIFile* greDir, nsIFile* updateDir, nsIFile* appDir, return; } +#if defined(XP_MACOSX) + // We need to detect whether elevation is required for this update. This can + // occur when an admin user installs the application, but another admin + // user attempts to update (see bug 394984). + // We only check if we need elevation if we are restarting. We don't attempt + // to stage if elevation is required. Staging happens without the user knowing + // about it, and we don't want to ask for elevation for seemingly no reason. + bool needElevation = false; + if (restart) { + needElevation = !IsRecursivelyWritable(installDirPath.get()); + if (needElevation) { + // Normally we would check this via nsIAppStartup::wasSilentlyRestarted, + // but nsIAppStartup isn't available yet. + char* mozAppSilentRestart = PR_GetEnv("MOZ_APP_SILENT_RESTART"); + bool wasSilentlyRestarted = + mozAppSilentRestart && (strcmp(mozAppSilentRestart, "") != 0); + if (wasSilentlyRestarted) { + // Elevation always requires prompting for credentials on macOS. If we + // are trying to restart silently, we must not display UI such as this + // prompt. + // We make this check here rather than in the updater, because it is + // actually Firefox that shows the elevation prompt (via + // InstallPrivilegedHelper), not the updater. + return; + } + } + } +#endif + nsAutoCString applyToDirPath; nsCOMPtr updatedDir; if (restart && !isStaged) { @@ -582,10 +611,7 @@ static void ApplyUpdate(nsIFile* greDir, nsIFile* updateDir, nsIFile* appDir, } #elif defined(XP_MACOSX) UpdateDriverSetupMacCommandLine(argc, argv, restart); -// We need to detect whether elevation is required for this update. This can -// occur when an admin user installs the application, but another admin -// user attempts to update (see bug 394984). -if (restart && !IsRecursivelyWritable(installDirPath.get())) { +if (restart && needElevation) { bool hasLaunched = LaunchElevatedUpdate(argc, argv, outpid); free(argv); if (!hasLaunched) {