-
-
Notifications
You must be signed in to change notification settings - Fork 368
Support Always Run As Administrator #3573
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
base: dev
Are you sure you want to change the base?
Conversation
…hen running admin mode when application is under admin mode
This comment has been minimized.
This comment has been minimized.
🥷 Code experts: onesounds Jack251970, onesounds have most 👩💻 activity in the files. See details
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame:
Activity based on git-commit:
Knowledge based on git-blame: To learn more about /:\ gitStream - Visit our Docs |
Be a legend 🏆 by adding a before and after screenshot of the changes you made, especially if they are around UI/UX. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for running Flow Launcher as administrator by updating startup configurations, user dialogs, and process launching logic. Key changes include:
- Adding a UAC dialog for confirming elevation and updating related UI elements.
- Modifying process launch methods to handle elevated and non-elevated launches based on user settings.
- Enhancing auto-startup and settings logic to support an "Always Run As Administrator" option.
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
Plugins/Flow.Launcher.Plugin.Program/UACDialog.xaml.cs | New UAC dialog implementation for admin confirmation. |
Plugins/Flow.Launcher.Plugin.Program/UACDialog.xaml | UI layout for the UAC dialog. |
Plugins/Flow.Launcher.Plugin.Program/Programs/Win32.cs | Updates to launch processes with optional elevation and minor task configuration. |
Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs | Adjustments to launch UWP packages with elevation support. |
Plugins/Flow.Launcher.Plugin.Program/Main.cs | Added IsAdmin flag to identify current process elevation state. |
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs | Updates to auto-startup handling and admin state checks in settings. |
Other files (Languages, AutoStartup, App.xaml.cs, PublicAPIInstance.cs, Win32Helper.cs, Settings.cs) | Various supporting changes to configuration, localization strings, and helper methods for administrator mode. |
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs
Outdated
Show resolved
Hide resolved
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
📝 Walkthrough""" WalkthroughThe changes implement an option for Flow Launcher to always run as administrator on startup. This includes UI updates, user settings, localization, logic for startup tasks, admin checks, and user prompts for elevation. Related plugins and process launch code are updated to support and respect administrator privileges, including a custom UAC dialog for user confirmation. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SettingsPane
participant Settings
participant AutoStartup
participant Win32Helper
participant OS
User->>SettingsPane: Toggle "Always Run as Administrator"
SettingsPane->>Settings: Update AlwaysRunAsAdministrator
SettingsPane->>AutoStartup: ChangeToViaLogonTask(alwaysRunAsAdministrator)
AutoStartup->>Win32Helper: IsAdministrator()
AutoStartup->>OS: Schedule or update startup task (with/without elevation)
SettingsPane->>SettingsPane: CheckAdminChangeAndAskForRestartAsync()
alt Not running as admin but should
SettingsPane->>User: Prompt to restart as admin
User->>SettingsPane: Confirm restart
SettingsPane->>OS: Restart app elevated
end
Assessment against linked issues
Possibly related PRs
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 8
🔭 Outside diff range comments (1)
Flow.Launcher/Helper/AutoStartup.cs (1)
173-200
: 🛠️ Refactor suggestionAdd error handling for administrator privilege settings.
The method doesn't explicitly check if setting the RunLevel was successful, which could lead to silent failures.
private static bool ScheduleLogonTask(bool alwaysRunAsAdministrator) { using var td = TaskService.Instance.NewTask(); td.RegistrationInfo.Description = LogonTaskDesc; td.Triggers.Add(new LogonTrigger { UserId = WindowsIdentity.GetCurrent().Name, Delay = TimeSpan.FromSeconds(2) }); td.Actions.Add(Constant.ExecutablePath); + bool requestedElevation = false; // Only if the app is running as administrator, we can set the run level to highest if (Win32Helper.IsAdministrator() && alwaysRunAsAdministrator) { td.Principal.RunLevel = TaskRunLevel.Highest; + requestedElevation = true; } td.Settings.StopIfGoingOnBatteries = false; td.Settings.DisallowStartIfOnBatteries = false; td.Settings.ExecutionTimeLimit = TimeSpan.Zero; try { - TaskService.Instance.RootFolder.RegisterTaskDefinition(LogonTaskName, td); + var registeredTask = TaskService.Instance.RootFolder.RegisterTaskDefinition(LogonTaskName, td); + + // Verify that elevation was set if requested + if (requestedElevation && registeredTask.Definition.Principal.RunLevel != TaskRunLevel.Highest) + { + App.API.LogWarning(ClassName, "Failed to set task to run with highest privileges"); + // We don't fail the operation, just log a warning + } + return true; } catch (Exception e) { App.API.LogError(ClassName, $"Failed to schedule logon task: {e}"); return false; } }
🧹 Nitpick comments (10)
Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml (1)
56-66
: Consider adding a warning about security implications.Running applications with administrator privileges can have security implications. Consider adding a more detailed tooltip or warning about the potential risks of always running with elevated privileges.
Example enhancement:
<cc:Card Title="{DynamicResource alwaysRunAsAdministrator}" Icon="" - Sub="{DynamicResource alwaysRunAsAdministratorToolTip}"> + Sub="{DynamicResource alwaysRunAsAdministratorToolTip}" + Style="{StaticResource WarningCardStyle}"> <ui:ToggleSwitch IsOn="{Binding AlwaysRunAsAdministrator}" OffContent="{DynamicResource disable}" OnContent="{DynamicResource enable}" /> </cc:Card>Note: This assumes a WarningCardStyle exists or would need to be created.
Plugins/Flow.Launcher.Plugin.Program/UACDialog.xaml.cs (3)
12-13
: Consider adding XML documentation and using consistent naming conventions for static fields.Both
msgBox
and_result
are static fields, but they use inconsistent naming conventions. Consider prefixing both with underscore for consistency, or use a consistent naming style.- private static UACDialog msgBox; + private static UACDialog _msgBox; private static MessageBoxResult _result = MessageBoxResult.None;
68-78
: Clean up dialog instance consistently in button handlers.The Button_Click method sets the dialog instance to null, but the field is static and might be accessed from other threads. Consider applying the same pattern to the KeyEsc_OnPress method.
private void KeyEsc_OnPress(object sender, ExecutedRoutedEventArgs e) { DialogResult = false; Close(); + _result = MessageBoxResult.None; + _msgBox = null; } private void Button_Click(object sender, RoutedEventArgs e) { if (sender == btnYes) _result = MessageBoxResult.Yes; else if (sender == btnNo) _result = MessageBoxResult.No; else _result = MessageBoxResult.None; - msgBox.Close(); - msgBox = null; + _msgBox.Close(); + _msgBox = null; }
80-85
: Use consistent naming in Button_Cancel method.The Button_Cancel method references
msgBox
directly, but should use the renamed field if you adopt the suggestion above.private void Button_Cancel(object sender, RoutedEventArgs e) { _result = MessageBoxResult.Cancel; - msgBox.Close(); - msgBox = null; + _msgBox.Close(); + _msgBox = null; }Flow.Launcher/Helper/AutoStartup.cs (1)
120-131
: Document startup method changes for administrator mode.The code includes a good comment explaining why registry startup doesn't support administrator mode, but could benefit from additional documentation.
public static void ChangeToViaLogonTask(bool alwaysRunAsAdministrator) { Disable(false); Enable(true, alwaysRunAsAdministrator); } public static void ChangeToViaRegistry() { Disable(true); + // Registry startup doesn't support running as administrator because it doesn't have an option + // to elevate privileges like the Task Scheduler does with RunLevel.Highest // We do not need to use alwaysRunAsAdministrator for registry, so we just set false here Enable(false, false); }Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (5)
4-5
: Namespace collision risk between WPF and WinForms typesAdding
using System.Windows;
to a file that is already importingSystem.Windows.Forms
increases the likelihood of ambiguous type resolutions (MessageBox
,Screen
,DragEventArgs
, …).
Although you currently reference the enumerations by their full namespace (MessageBoxButton
,MessageBoxResult
) the next developer who tries to callMessageBox.Show
(or similar) will hit a compile-time ambiguity.Two low-effort mitigations:
-using System.Windows; +using Wpf = System.Windows;or import only the specific WPF types you need:
using System.Windows.MessageBoxButton; using System.Windows.MessageBoxResult;Either option keeps the intent clear and shields you from hidden compilation errors down the road.
26-27
: Static cache of elevation status can become stale after restart-in-place
_isAdministrator
is captured once when theSettingsPaneGeneralViewModel
type is first touched.
If (in a future enhancement) you decide to elevate the process without a full application restart, this flag will remain false, giving you inconsistent behaviour.Safer alternative: turn it into a computed property or refresh it inside
CheckAdminChangeAndAskForRestart()
right before the comparison.-private static readonly bool _isAdministrator = Win32Helper.IsAdministrator(); +private static bool IsAdministrator => Win32Helper.IsAdministrator();Then replace usages accordingly.
72-78
: Restart prompt executes even when startup-task creation fails
CheckAdminChangeAndAskForRestart()
is invoked regardless of whetherAutoStartup.ChangeToViaLogonTask
threw.
In the failure path we show the user two dialogs: one for the error and then another asking for a restart—yet we know the scheduled task is still out of sync.Consider short-circuiting when the startup change fails or basing the restart prompt on the success flag returned by the helper.
94-113
: Duplicate logic – consider extracting a helperThe
UseLogonTaskForStartup
setter repeats almost the same try/catch & restart-check block found inStartFlowLauncherOnSystemStartup
.
Extracting this into a private method (e.g.UpdateStartupMethod(bool viaLogonTask)
) would keep the two setters terse and DRY.
141-154
: Restart helper could lose original command-line context
App.API.RestartApp(AlwaysRunAsAdministrator ? "runas" : string.Empty);
forwards only the elevation flag.
If the user had started Flow Launcher with extra CLI arguments (portable mode, debug flags, etc.) they will be dropped during restart.Recommend overloading
RestartApp
to accept an argument builder or captureEnvironment.GetCommandLineArgs()
and re-use them when invoking the new process.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
Flow.Launcher.Infrastructure/UserSettings/Settings.cs
(1 hunks)Flow.Launcher.Infrastructure/Win32Helper.cs
(2 hunks)Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs
(1 hunks)Flow.Launcher/App.xaml.cs
(1 hunks)Flow.Launcher/Helper/AutoStartup.cs
(7 hunks)Flow.Launcher/Languages/en.xaml
(2 hunks)Flow.Launcher/MainWindow.xaml.cs
(1 hunks)Flow.Launcher/PublicAPIInstance.cs
(2 hunks)Flow.Launcher/Resources/Pages/WelcomePage5.xaml.cs
(1 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs
(6 hunks)Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml
(1 hunks)Plugins/Flow.Launcher.Plugin.Program/Languages/en.xaml
(1 hunks)Plugins/Flow.Launcher.Plugin.Program/Main.cs
(3 hunks)Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs
(4 hunks)Plugins/Flow.Launcher.Plugin.Program/Programs/Win32.cs
(5 hunks)Plugins/Flow.Launcher.Plugin.Program/UACDialog.xaml
(1 hunks)Plugins/Flow.Launcher.Plugin.Program/UACDialog.xaml.cs
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
Flow.Launcher/Resources/Pages/WelcomePage5.xaml.cs (1)
Flow.Launcher/Helper/AutoStartup.cs (2)
AutoStartup
(12-230)ChangeToViaLogonTask
(120-124)
Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (2)
Flow.Launcher/PublicAPIInstance.cs (1)
RestartApp
(75-75)Flow.Launcher.Core/Plugin/JsonRPCV2Models/JsonRPCPublicAPI.cs (1)
RestartApp
(27-30)
🪛 GitHub Actions: Check Spelling
Flow.Launcher/App.xaml.cs
[warning] 63-63: WMP
is not a recognized word. (unrecognized-spelling)
[warning] 106-119: Ioc
is not a recognized word. (unrecognized-spelling)
[warning] 174-174: Ioc
is not a recognized word. (unrecognized-spelling)
[warning] 199-199: Loadertask
is not a recognized word. (unrecognized-spelling)
[warning] 225-225: VSTHRD
is not a recognized word. (unrecognized-spelling)
Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs
[warning] 216-216: spefic
is not a recognized word. (unrecognized-spelling)
[warning] 357-357: requerying
is not a recognized word. (unrecognized-spelling)
Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs
[warning] 91-94: uap
and rescap
are not recognized words. (unrecognized-spelling)
[warning] 462-462: dlgtitle
is not a recognized word. (unrecognized-spelling)
[warning] 535-535: uap
is not a recognized word. (unrecognized-spelling)
Flow.Launcher/MainWindow.xaml.cs
[warning] 61-61: WMP
is not a recognized word. (unrecognized-spelling)
[warning] 100-100: VSTHRD
is not a recognized word. (unrecognized-spelling)
[warning] 460-475: VSTHRD
is not a recognized word. (unrecognized-spelling)
[warning] 564-566: WMP
is not a recognized word. (unrecognized-spelling)
[warning] 652-678: gamemode
and positionreset
are not recognized words. (unrecognized-spelling)
[warning] 794-795: XRatio
and YRatio
are not recognized words. (unrecognized-spelling)
[warning] 1009-1018: clocksb
and iconsb
are not recognized words. (unrecognized-spelling)
Plugins/Flow.Launcher.Plugin.Program/Main.cs
[warning] 20-20: Reloadable
is not a recognized word. (unrecognized-spelling)
[warning] 46-47: unins
and uninst
are not recognized words. (unrecognized-spelling)
[warning] 50-50: Uninstaller
is not a recognized word. (unrecognized-spelling)
[warning] 66-68: desinstalar
is not a recognized word. (unrecognized-spelling)
[warning] 78-79: Uninstaller
is not a recognized word. (unrecognized-spelling)
[warning] 135-137: Uninstallers
is not a recognized word. (unrecognized-spelling)
[warning] 142-145: uninst
, uninstaller
, and Uninstaller
are not recognized words. (unrecognized-spelling)
[warning] 145-157: Prefixs
and uninstaller
are not recognized words. (unrecognized-spelling)
[warning] 389-449: dlgtitle
is not a recognized word. (unrecognized-spelling)
Plugins/Flow.Launcher.Plugin.Program/UACDialog.xaml
[warning] 2-2: UAC
is not a recognized word. (unrecognized-spelling)
Plugins/Flow.Launcher.Plugin.Program/Programs/Win32.cs
[warning] 223-224: UAC
is not a recognized word. (unrecognized-spelling)
[warning] 231-231: workaround
is not a recognized word. (unrecognized-spelling)
Plugins/Flow.Launcher.Plugin.Program/UACDialog.xaml.cs
[warning] 2-2: UAC
is not a recognized word. (unrecognized-spelling)
Flow.Launcher.Infrastructure/Win32Helper.cs
[warning] 42-42: Dwm
is not a recognized word. (unrecognized-spelling)
[warning] 54-54: DWMSBT
and SYSTEMBACKDROP
are not recognized words. (unrecognized-spelling)
[warning] 55-55: DWMSBT
and SYSTEMBACKDROP
are not recognized words. (unrecognized-spelling)
[warning] 56-56: DWMSBT
and SYSTEMBACKDROP
are not recognized words. (unrecognized-spelling)
[warning] 59-59: Dwm
and PInvoke
are not recognized words. (unrecognized-spelling)
[warning] 61-61: DWMWA
, DWMWINDOWATTRIBUTE
, and SYSTEMBACKDROP
are not recognized words. (unrecognized-spelling)
[warning] 70-70: Dwm
and PInvoke
are not recognized words. (unrecognized-spelling)
[warning] 72-72: DWMWA
and DWMWINDOWATTRIBUTE
are not recognized words. (unrecognized-spelling)
[warning] 88-90: DWMWCP
is not a recognized word. (unrecognized-spelling)
[warning] 94-94: Dwm
and PInvoke
are not recognized words. (unrecognized-spelling)
[warning] 96-96: DWMWA
and DWMWINDOWATTRIBUTE
are not recognized words. (unrecognized-spelling)
[warning] 107-107: PInvoke
is not a recognized word. (unrecognized-spelling)
[warning] 162-189: GWL
is not a recognized word. (unrecognized-spelling)
[warning] 198-210: Wnd
is not a recognized word. (unrecognized-spelling)
[warning] 239-239: Wnd
is not a recognized word. (unrecognized-spelling)
[warning] 263-277: WINTAB
, Progman
, and WORKERW
are not recognized words. (unrecognized-spelling)
[warning] 517-523: hkl
is not a recognized word. (unrecognized-spelling)
[warning] 549-549: nqo
is not a recognized word. (unrecognized-spelling)
[warning] 622-646: tsf
and Tsf
are not recognized words. (unrecognized-spelling)
[warning] 682-685: Noto
is not a recognized word. (unrecognized-spelling)
[warning] 726-751: noto
is not a recognized word. (unrecognized-spelling)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: gitStream workflow automation
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (25)
Flow.Launcher.Infrastructure/Win32Helper.cs (2)
8-8
: New dependency added for administrator check.Good addition of the
System.Security.Principal
namespace, which is required for the newIsAdministrator()
method implementation.
758-767
: Well-implemented administrator check method.The
IsAdministrator()
method follows the standard pattern for detecting administrator privileges in Windows applications usingWindowsPrincipal
andWindowsBuiltInRole
. This centralized implementation will help maintain consistency when checking for elevated permissions throughout the application.Flow.Launcher.Infrastructure/UserSettings/Settings.cs (1)
375-375
: New setting for administrator mode.Good implementation of the
AlwaysRunAsAdministrator
property with a sensible default value offalse
. This ensures that the application doesn't unexpectedly run with elevated permissions unless explicitly configured by the user.Flow.Launcher.Plugin/Interfaces/IPublicAPI.cs (1)
36-41
: New API method for restarting with arguments.The addition of
RestartApp(string arguments)
method to the public API will allow the application to restart with custom command-line arguments, which is essential for implementing the administrator mode toggle functionality.Plugins/Flow.Launcher.Plugin.Program/Languages/en.xaml (1)
100-102
: LGTM! User Account Control strings are well-structured.The new localization strings for the UAC dialog look good. They follow the standard Windows UAC prompt format with a title, confirmation question, and program location display.
Flow.Launcher/App.xaml.cs (1)
239-239
: LGTM! Successfully added administrator parameter to auto-startup check.The change properly passes the new
AlwaysRunAsAdministrator
setting to theCheckIsEnabled
method, ensuring that auto-startup respects the administrator mode preference.Flow.Launcher/MainWindow.xaml.cs (1)
598-605
: LGTM! Notify icon now shows administrator status correctly.Good implementation of conditional text for the notify icon tooltip. This provides clear visual feedback to users when the application is running with elevated privileges.
Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml (1)
47-66
: LGTM! Well-structured UI for administrator mode settings.The new UI elements for administrator mode are well-organized. The CardGroup approach logically groups the related startup settings together, and the toggle for "Always Run as Administrator" is properly bound to the corresponding property in the view model.
I particularly like the choice of icons - the shield icon () is perfect for representing administrator privileges, making the purpose of the setting immediately clear to users.
Plugins/Flow.Launcher.Plugin.Program/Main.cs (3)
6-6
: Appropriate addition of required namespace.Adding the System.Security.Principal namespace is necessary for the WindowsIdentity and WindowsPrincipal classes used in the new IsAdministrator method.
36-36
: Good addition of an administrator status indicator.This static boolean field provides a centralized way to determine the current administrator status within the plugin, properly initialized using the IsAdministrator method.
466-471
: Well-implemented administrator check method.The IsAdministrator method follows the standard pattern for checking Windows administrator privileges:
- Gets the current Windows identity
- Creates a principal from this identity
- Checks if the principal is in the Administrator role
The implementation is clean and correctly disposes of the WindowsIdentity object with a using statement.
Flow.Launcher/Languages/en.xaml (2)
49-49
: Good addition of admin indicator label.This localized string will be used to indicate administrator mode in the UI, supporting the new feature.
135-138
: Complete set of localization strings for administrator mode.The added strings provide comprehensive text resources for the administrator mode functionality:
- Toggle label for the setting
- Tooltip explaining the functionality
- Dialog title for mode change
- Confirmation message for restart
These strings ensure the feature is properly localized and user-friendly.
Flow.Launcher/PublicAPIInstance.cs (3)
75-75
: Good addition of parameter-less overload for backward compatibility.Adding an overload that calls the parameterized version with null maintains compatibility with existing code while extending functionality.
78-78
: Enhancement of RestartApp to support command-line arguments.The method signature change allows passing arguments to the application when restarting, which is essential for the administrator mode feature.
93-93
: Properly passing arguments to the UpdateManager.The UpdateManager.RestartApp call is now correctly passing the arguments parameter, enabling restart with custom command-line options.
Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs (3)
458-458
: Good refactoring to centralize launch logic.Replacing inline launch code with a call to the dedicated Launch method improves code maintainability and consistency.
502-502
: Consistent use of centralized launch method.Using the same Launch method from the context menu handler ensures consistent behavior between different launch scenarios.
514-527
: Well-implemented launch method with proper object initialization.The refactored Launch method:
- Uses object initializer syntax for ProcessStartInfo which is cleaner
- Properly sets UseShellExecute and Verb properties
- Correctly executes the process asynchronously with Task.Run
- Uses ConfigureAwait(false) to avoid context capturing
This implementation properly handles elevation requests through the "runas" verb when needed.
Plugins/Flow.Launcher.Plugin.Program/Programs/Win32.cs (2)
199-202
: Simplified code by using the new Launch method.The code now uses a centralized Launch method for starting processes, which improves code organization and reusability.
271-272
: Simplified "Run as administrator" context menu implementation.The code now uses the centralized Launch method with elevated=true parameter, improving code organization.
Flow.Launcher/Helper/AutoStartup.cs (2)
20-52
: Add administrator privilege support to startup checking.The changes correctly implement the ability to run Flow Launcher with administrator privileges at startup by adding an
alwaysRunAsAdministrator
parameter.
54-84
: Add administrator privilege support to logon task checking.The changes allow checking if the logon task is set to run with administrator privileges.
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (1)
55-56
:✅ Verification successful
Verify all call-sites migrated to new
ChangeToViaLogonTask(bool)
signatureThe extra parameter has been provided here, but older call-sites elsewhere in the codebase will silently compile if they rely on overload resolution (e.g. optional parameters) or might now fail.
Please run a sweep to ensure no stale overloads remain:
🏁 Script executed:
rg -n "ChangeToViaLogonTask(" --type cs
Length of output: 134
Let’s re-run the search using a fixed-string match to avoid regex errors:
🏁 Script executed:
#!/bin/bash rg -n -F "ChangeToViaLogonTask(" --type csLength of output: 755
All
ChangeToViaLogonTask
calls now use the new bool parameter
Verified viarg -F "ChangeToViaLogonTask(" --type cs
—no parameterless calls remain.Call-sites updated:
- Resources/Pages/WelcomePage5.xaml.cs:48
- SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs:55, 94, 127
Definition in Helper/AutoStartup.cs only exposes
ChangeToViaLogonTask(bool)
, so no stale overloads exist.Flow.Launcher/Resources/Pages/WelcomePage5.xaml.cs (1)
48-48
: Code correctly implements administrator mode setting for startupThe change properly passes the
AlwaysRunAsAdministrator
setting to theChangeToViaLogonTask
method, ensuring that Flow Launcher will respect the user's preference for running with elevated privileges on system startup. This aligns with the PR objective of supporting plugins that require administrator mode.
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for launching Flow Launcher as administrator by introducing a new AlwaysRunAsAdministrator setting and updating the process launching, auto-start, and UAC dialog functionalities accordingly. Key changes include:
- Adding a new AlwaysRunAsAdministrator property in settings and updating the settings UI.
- Modifying the logon task and process launch logic throughout the application to account for administrator mode.
- Enhancing the UAC dialog and related helper methods to support elevated launch flows.
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
Plugins/Flow.Launcher.Plugin.Program/UACDialog.xaml.cs | Added a UAC dialog for confirming elevation with asynchronous image loading. |
Plugins/Flow.Launcher.Plugin.Program/Programs/Win32.cs | Updated process launching logic to conditionally show the UAC dialog and to handle elevated launch. |
Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs | Modified UWP package launch flow to support administrator mode with updated auto-start handling. |
Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml | Integrated a new toggle for AlwaysRunAsAdministrator in the settings UI. |
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs | Updated logon task startup logic to include the administrator mode flag. |
Flow.Launcher/Helper/AutoStartup.cs | Adjusted logon task scheduling to incorporate the AlwaysRunAsAdministrator parameter. |
Flow.Launcher/PublicAPIInstance.cs | Provided an overload of RestartApp to accept command-line arguments for admin mode restart. |
Flow.Launcher/Infrastructure/Win32Helper.cs | Added a helper method to check administrator status. |
Flow.Launcher/Infrastructure/UserSettings/Settings.cs | Introduced the AlwaysRunAsAdministrator setting property. |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds support for always running Flow Launcher as administrator by extending startup configuration, command execution, and UI.
- Introduces a UAC confirmation dialog for elevated launches when already running as admin.
- Updates Win32 and UWP launch paths to handle elevated/non-elevated scenarios, including “Always Run As Administrator” toggle.
- Extends settings UI and auto-startup helper to configure logon task run level and prompt for restart on admin-mode changes.
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
Plugins/Flow.Launcher.Plugin.Program/UACDialog.xaml(.cs) | New modal dialog for UAC confirmation when launching elevated |
Plugins/Flow.Launcher.Plugin.Program/Programs/Win32.cs | Refactored launch logic, added elevated flag |
Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs | Updated UWP launch with elevated support |
Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml | Added toggle for “Always run as administrator” |
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs | Handles new setting, prompts restart |
Flow.Launcher/Helper/AutoStartup.cs | Propagates new flag to logon task configuration |
Flow.Launcher/PublicAPIInstance.cs | Added overload to RestartApp with arguments |
Flow.Launcher/MainWindow.xaml.cs | Displays “(Admin)” in tray icon text when elevated |
Flow.Launcher.Infrastructure/Win32Helper.cs | Exposes IsAdministrator helper |
Flow.Launcher.Infrastructure/UserSettings/Settings.cs | New AlwaysRunAsAdministrator setting |
Comments suppressed due to low confidence (2)
Flow.Launcher/MainWindow.xaml.cs:598
- [nitpick] Variable name 'text' is too generic; consider renaming to 'notifyIconText' or similar to clarify its purpose.
var text = Win32Helper.IsAdministrator() ?
Flow.Launcher/PublicAPIInstance.cs:93
- Verify that UpdateManager.RestartApp supports an overload with arguments; if not, this call will fail at runtime or cause a breaking change in the public API.
UpdateManager.RestartApp(Constant.ApplicationFileName, arguments);
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds support for running Flow Launcher (and its Program plugin) as administrator by introducing a new “Always run as administrator” setting, custom UAC confirmation, and updated startup registration.
- Introduces a UACDialog window and refactors program launch paths for Win32 and UWP packages to handle elevated vs. non-elevated scenarios.
- Adds an
AlwaysRunAsAdministrator
toggle in settings, updatesAutoStartup
logic to register a logon task with high run level, and prompts for restart when the administrator setting changes. - Extends
PublicAPI
andMain
to detect admin status, restart with arguments, and display an “(Admin)” badge in the tray icon.
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
Plugins/.../UACDialog.xaml(.cs) | New dialog for custom UAC prompts |
Plugins/.../Win32.cs | Refactored Launch to handle elevation, UAC prompt, and standard-user fallback |
Plugins/.../UWPPackage.cs | Updated Launch and context menus to accept an elevation flag |
Plugins/.../Main.cs & Infrastructure/Win32Helper.cs | Added IsAdministrator detection and Main.IsAdmin flag |
Flow.Launcher/SettingPages/...General.xaml(.cs) | Added AlwaysRunAsAdministrator toggle and restart-on-change logic |
Flow.Launcher/Helper/AutoStartup.cs | Modified logon task and registry startup to respect run-level |
Flow.Launcher/PublicAPIInstance.cs & Interfaces/IPublicAPI.cs | Overloaded RestartApp to accept arguments |
Flow.Launcher/Languages/en.xaml | New localization keys for UAC dialog, settings, and admin badge |
Comments suppressed due to low confidence (1)
Plugins/Flow.Launcher.Plugin.Program/Programs/Win32.cs:209
- Consider adding unit or integration tests for the new launch logic (elevated vs. standard user fallback and UAC prompt) to ensure both paths behave correctly.
private void Launch(bool elevated = false)
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (1)
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (1)
126-151
: Previous review suggestion not implemented: MissingOnPropertyChanged()
and gated restart logicThe same issues flagged in the previous review are still present:
- Missing
OnPropertyChanged()
: UI bindings won't update when the property changes programmatically- Restart check still gated: The restart prompt only appears when both startup flags are true, leaving users in wrong elevation state if they manually start Flow Launcher
The previous review suggested moving
CheckAdminChangeAndAskForRestartAsync()
outside the conditional block and addingOnPropertyChanged()
, which you agreed to but haven't implemented yet.Apply the previously agreed-upon fix:
set { if (AlwaysRunAsAdministrator == value) return; Settings.AlwaysRunAsAdministrator = value; if (StartFlowLauncherOnSystemStartup && UseLogonTaskForStartup) { try { AutoStartup.ChangeToViaLogonTask(value); } catch (Exception e) { App.API.ShowMsg(App.API.GetTranslation("setAutoStartFailed"), e.Message); } - - // If we have enabled logon task startup, we need to check if we need to restart the app - // even if we encounter an error while setting the startup method - _ = CheckAdminChangeAndAskForRestartAsync(); } + + // Ask for restart irrespective of how the app was started + _ = CheckAdminChangeAndAskForRestartAsync(); + + OnPropertyChanged(); }
🧹 Nitpick comments (2)
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (2)
193-209
: Fix method naming conventionThe method name
getUpdateExe()
should follow C# naming conventions and use PascalCase.- private static string getUpdateExe() + private static string GetUpdateExe()And update the call site on line 183:
- FileName = getUpdateExe(), + FileName = GetUpdateExe(),
84-84
: Consider improving async task handlingThe current fire-and-forget pattern (
_ = CheckAdminChangeAndAskForRestartAsync();
) discards the task, which could hide exceptions and make debugging difficult.Consider using a more explicit async handling pattern:
- _ = CheckAdminChangeAndAskForRestartAsync(); + CheckAdminChangeAndAskForRestartAsync().ContinueWith(task => + { + if (task.IsFaulted) + { + // Log or handle the exception appropriately + App.API.ShowMsg("Error", task.Exception?.GetBaseException()?.Message ?? "Unknown error"); + } + }, TaskScheduler.FromCurrentSynchronizationContext());Alternatively, if fire-and-forget is intentional, consider adding a comment explaining why exceptions are being ignored.
Also applies to: 121-121, 148-148
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs
(8 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (1)
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs (1)
153-209
: Excellent implementation of admin elevation functionalityThe new methods for handling administrator mode are well-designed:
CheckAdminChangeAndAskForRestartAsync()
properly prompts users and handles the transition gracefullyRestartAppAsAdministrator()
correctly uses UAC elevation with the "runas" verbGetUpdateExe()
appropriately handles Squirrel-based installations- Good error handling and user feedback throughout
The integration with the existing startup logic is clean and maintains backward compatibility.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces support for running Flow Launcher as administrator by updating UAC dialogs, process launch routines, startup registrations, and associated settings. Key changes include modifications to the UAC dialog implementation, adjustments in process launching (Win32 and UWPPackage), and updates to startup and settings logic for proper handling of administrator mode.
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
Plugins/Flow.Launcher.Plugin.Program/UACDialog.xaml.cs | Adds a UAC dialog to request administrator confirmation and updates the dialog initialization logic. |
Plugins/Flow.Launcher.Plugin.Program/Programs/Win32.cs | Updates process launching to support elevated execution and handles UAC dialog invocation when already elevated. |
Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs | Adjusts process launching routines for UWP packages with consideration for elevated privileges. |
Plugins/Flow.Launcher/Main.cs | Incorporates an administrator check and related startup logic adjustments. |
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs | Integrates new settings for always running as administrator and adds logic to prompt for restart when required. |
Flow.Launcher/Helper/AutoStartup.cs | Updates logon task scheduling to incorporate administrator mode settings. |
Flow.Launcher/App.xaml.cs | Implements a new restart mechanism to relaunch the app as administrator. |
Flow.Launcher/Infrastructure/Win32Helper.cs | Adds a helper method for detecting administrator mode. |
Flow.Launcher.Infrastructure.UserSettings/Settings.cs | Introduces a new setting for always running as administrator. |
Flow.Launcher/Languages/en.xaml | Adds new localization strings for administrator mode features. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Flow.Launcher/Helper/AutoStartup.cs (1)
68-93
: Improve the privilege checking logic.The enhanced task checking logic correctly handles both path and run level validation. However, the logic could be more readable and the error message could be more specific.
Consider refactoring for better readability:
- if (_isAdministrator) - { - // If path or run level is not correct, we need to unschedule and reschedule the task - if (!pathCorrect || !runLevelCorrect) - { - UnscheduleLogonTask(); - ScheduleLogonTask(alwaysRunAsAdministrator); - } - } - else - { - // If run level is not correct, we cannot edit it because we are not administrator - if (!runLevelCorrect) - { - throw new UnauthorizedAccessException("Cannot edit task run level because the app is not running as administrator."); - } - - // If run level is correct and path is not correct, we need to unschedule and reschedule the task - if (!pathCorrect) - { - UnscheduleLogonTask(); - ScheduleLogonTask(alwaysRunAsAdministrator); - } - } + // Check if task needs updating + var needsUpdate = !pathCorrect || !runLevelCorrect; + + if (needsUpdate) + { + if (!_isAdministrator && !runLevelCorrect) + { + throw new UnauthorizedAccessException($"Cannot modify task run level from {task.Definition.Principal.RunLevel} to {(alwaysRunAsAdministrator ? "Highest" : "LUA")} without administrator privileges."); + } + + UnscheduleLogonTask(); + ScheduleLogonTask(alwaysRunAsAdministrator); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
Flow.Launcher/App.xaml.cs
(3 hunks)Flow.Launcher/Helper/AutoStartup.cs
(7 hunks)Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs
(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs
🧰 Additional context used
🪛 GitHub Actions: Check Spelling
Flow.Launcher/App.xaml.cs
[warning] 65: WMP
is not a recognized word. (unrecognized-spelling)
[warning] 108-176: Ioc
is not a recognized word. (unrecognized-spelling)
[warning] 201: Loadertask
is not a recognized word. (unrecognized-spelling)
[warning] 227: VSTHRD
is not a recognized word. (unrecognized-spelling)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (8)
Flow.Launcher/App.xaml.cs (4)
3-4
: LGTM! Required imports for new functionality.The added imports for
System.IO
andSystem.Reflection
are necessary for the newRestartAppAsAdministrator
method implementation.🧰 Tools
🪛 GitHub Actions: Check Spelling
[warning] 65:
WMP
is not a recognized word. (unrecognized-spelling)
[warning] 201:
Loadertask
is not a recognized word. (unrecognized-spelling)
[warning] 227:
VSTHRD
is not a recognized word. (unrecognized-spelling)
243-253
: Good error handling for privilege escalation.The exception handling for
UnauthorizedAccessException
provides a clear user experience by prompting to restart as administrator. The user confirmation dialog prevents unexpected elevation.🧰 Tools
🪛 GitHub Actions: Check Spelling
[warning] 65:
WMP
is not a recognized word. (unrecognized-spelling)
[warning] 201:
Loadertask
is not a recognized word. (unrecognized-spelling)
[warning] 227:
VSTHRD
is not a recognized word. (unrecognized-spelling)
354-369
:❓ Verification inconclusive
Verify the update.exe detection logic.
The local function
getUpdateExe()
contains complex path detection logic that could be fragile:
- String comparisons: Case-insensitive checks for "app-" and "SquirrelTemp"
- Path traversal: Uses "..\" which could be problematic on different systems
- Exception handling: Throws generic exception if update.exe not found
Test the update.exe detection logic in different deployment scenarios:
🏁 Script executed:
#!/bin/bash # Check if there are any references to these specific path patterns in the codebase rg -i "app-|squirreltemp|update\.exe" --type cs -A 2 -B 2Length of output: 4793
Verify and Harden
getUpdateExe()
Path ResolutionThe helper in Flow.Launcher/App.xaml.cs (around lines 354–369) relies on fragile string checks and a hard-coded “..\Update.exe” traversal. Please review and test its behavior across all deployment scenarios:
• Replace ad-hoc
IndexOf("app-")
/IndexOf("SquirrelTemp")
checks with properPath
APIs (e.g.Path.GetFileNameWithoutExtension
,Path.DirectorySeparatorChar
,Path.Combine
).
• Avoid hard-coded “..\” traversal; usePath.GetDirectoryName
+Path.Combine
to locate Update.exe.
• Catch specific exceptions (e.g.FileNotFoundException
) and provide a clear, actionable error message rather than a genericException
.
• Validate in:
– Squirrel auto-update runs (entryAssembly = update.exe in a versioned app-folder)
– Debug launches without Update.exe present
– Portable/stand-alone installs
– Any temp or custom folder layoutsEnsure each scenario returns the correct full path or fails gracefully with diagnostic details.
🧰 Tools
🪛 GitHub Actions: Check Spelling
[warning] 65:
WMP
is not a recognized word. (unrecognized-spelling)
[warning] 201:
Loadertask
is not a recognized word. (unrecognized-spelling)
[warning] 227:
VSTHRD
is not a recognized word. (unrecognized-spelling)
241-241
:✅ Verification successful
Verify parameter consistency across the codebase.
The call to
Helper.AutoStartup.CheckIsEnabled
now includes theAlwaysRunAsAdministrator
setting parameter, which is correctly implemented.Verify that all calls to AutoStartup methods have been updated consistently:
🏁 Script executed:
#!/bin/bash # Search for AutoStartup method calls to ensure consistency rg -A 3 "AutoStartup\.(CheckIsEnabled|ChangeToViaLogonTask|Enable)" --type csLength of output: 2053
#!/usr/bin/env bash # Locate the file declaring the AutoStartup class file=$(rg -l "class AutoStartup") echo "AutoStartup class found in: $file" # Display the signature of CheckIsEnabled echo -e "\n--- CheckIsEnabled signature ---" rg -n "CheckIsEnabled" -C2 "$file" # Display the signature of ChangeToViaLogonTask echo -e "\n--- ChangeToViaLogonTask signature ---" rg -n "ChangeToViaLogonTask" -C2 "$file" # Display the signature of Enable echo -e "\n--- Enable signature ---" rg -n "Enable" -C2 "$file"
#!/usr/bin/env bash set -e # Locate the AutoStartup class file auto_file=$(rg -l "class AutoStartup" --type cs) echo "AutoStartup class defined in: $auto_file" # Show the signatures for relevant methods echo -e "\n--- CheckIsEnabled signature ---" rg -n "static .*CheckIsEnabled" -C1 "$auto_file" echo -e "\n--- ChangeToViaLogonTask signature ---" rg -n "static .*ChangeToViaLogonTask" -C1 "$auto_file" echo -e "\n--- Enable signature ---" rg -n "static .*Enable" -C1 "$auto_file"
#!/usr/bin/env bash set -e # Find the file defining AutoStartup auto_file=$(rg -l "class AutoStartup" --type cs | head -n1) echo "AutoStartup class defined in: $auto_file" # Show the signatures of relevant methods for verification echo -e "\n--- CheckIsEnabled signature ---" rg -n "static .*CheckIsEnabled" -C2 "$auto_file" echo -e "\n--- ChangeToViaLogonTask signature ---" rg -n "static .*ChangeToViaLogonTask" -C2 "$auto_file" echo -e "\n--- Enable signature ---" rg -n "static .*Enable" -C2 "$auto_file"
#!/usr/bin/env bash set -e # Locate the AutoStartup class file=$(rg -l "class AutoStartup" --type cs | head -n1) echo "AutoStartup defined in: $file" # Print method signatures for CheckIsEnabled, ChangeToViaLogonTask, and Enable echo -e "\n--- CheckIsEnabled signature ---" rg -n "public static .*CheckIsEnabled" -C1 "$file" echo -e "\n--- ChangeToViaLogonTask signature ---" rg -n "public static .*ChangeToViaLogonTask" -C1 "$file" echo -e "\n--- Enable signature ---" rg -n "public static .*Enable" -C1 "$file"
#!/usr/bin/env bash set -e # Find the file defining the AutoStartup class auto_file=$(rg -l "public static class AutoStartup" --type cs | head -n1) echo "AutoStartup class file: $auto_file" # Display the signatures of CheckIsEnabled, ChangeToViaLogonTask, and Enable echo -e "\n--- CheckIsEnabled signature ---" rg -n "public static .*CheckIsEnabled" -C1 "$auto_file" echo -e "\n--- ChangeToViaLogonTask signature ---" rg -n "public static .*ChangeToViaLogonTask" -C1 "$auto_file" echo -e "\n--- Enable signature ---" rg -n "public static .*Enable" -C1 "$auto_file"
AutoStartup parameter update verified
TheCheckIsEnabled
invocation now correctly passes both_settings.UseLogonTaskForStartup
and_settings.AlwaysRunAsAdministrator
. OtherAutoStartup
methods (ChangeToViaLogonTask
,Enable
) retain their original single-parameter signatures and have been invoked consistently—no further updates are required.🧰 Tools
🪛 GitHub Actions: Check Spelling
[warning] 65:
WMP
is not a recognized word. (unrecognized-spelling)
[warning] 201:
Loadertask
is not a recognized word. (unrecognized-spelling)
[warning] 227:
VSTHRD
is not a recognized word. (unrecognized-spelling)Flow.Launcher/Helper/AutoStartup.cs (4)
20-20
: Good performance optimization with cached admin status.Caching the administrator status in a static readonly field prevents repeated privilege checks, which is efficient since admin status doesn't change during application runtime.
22-22
: Method signature enhancement looks correct.The addition of
alwaysRunAsAdministrator
parameter toCheckIsEnabled
properly extends the API to support admin mode configuration.
113-116
: Simple and correct run level validation.The
CheckRunLevel
method correctly validates the task run level against the desired admin mode setting.
214-218
:✅ Verification successful
Verify admin privilege requirements for task scheduling.
The logic correctly sets
TaskRunLevel.Highest
only when both the app is running as administrator AND the user wants admin mode. This prevents privilege escalation issues.Verify that task scheduling works correctly in different privilege scenarios:
🏁 Script executed:
#!/bin/bash # Check for any other references to TaskRunLevel in the codebase rg "TaskRunLevel" --type cs -A 2 -B 2Length of output: 814
TaskRunLevel usage verified and code change approved
I searched the entire codebase for
TaskRunLevel
and confirmed only two occurrences—insideCheckRunLevel(...)
and theif (_isAdministrator && alwaysRunAsAdministrator)
block that setstd.Principal.RunLevel = TaskRunLevel.Highest
. The logic correctly restricts highest‐privilege scheduling to when the app is already elevated and the user opts in. No other references or side effects were found.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for always running Flow Launcher (and its launched programs) in administrator mode, including UI controls, startup configuration, and elevation handling.
- Introduces an
AlwaysRunAsAdministrator
setting with a corresponding toggle in general settings - Updates
AutoStartup
and task-scheduler logic to respect the new setting and handle run levels - Adds a
UACDialog
window and refactors program launch logic to prompt when already elevated
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
Plugins/Flow.Launcher.Plugin.Program/UACDialog.xaml(.cs) | New modal dialog for UAC confirmation when launching elevated apps |
Plugins/Flow.Launcher.Plugin.Program/Programs/Win32.cs | Refactored launch method, added elevation handling, fire-and-forget tasks |
Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs | Consistent launch signature, replaced Task.Run patterns |
Plugins/Flow.Launcher.Plugin.Program/Main.cs | Added IsAdmin flag and admin-detection logic |
Flow.Launcher/SettingPages/…/SettingsPaneGeneral.xaml | UI toggle for always-run-as-administrator |
Flow.Launcher/SettingPages/…/SettingsPaneGeneralViewModel.cs | Bound the new setting, prompts for restart on mode change |
Flow.Launcher/MainWindow.xaml.cs | Show “(Admin)” in tray tooltip when elevated |
Flow.Launcher/Helper/AutoStartup.cs | Updated task-scheduler helper to set/run tasks with correct run level |
Flow.Launcher/App.xaml.cs | Added restart-as-administrator logic via Squirrel |
Flow.Launcher.Infrastructure/Win32Helper.cs | Central admin-check helper |
Flow.Launcher.Infrastructure/UserSettings/Settings.cs | New AlwaysRunAsAdministrator property added |
Comments suppressed due to low confidence (3)
Flow.Launcher/Infrastructure/Win32Helper.cs:760
- Admin-check logic is duplicated in both
Win32Helper
andMain
; consider centralizing this into one shared utility to avoid divergence.
public static bool IsAdministrator()
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs:28
- [nitpick] Static fields should follow naming conventions, e.g., rename
_isAdministrator
tos_isAdministrator
orIsAdministratorStatic
for clarity.
private static readonly bool _isAdministrator = Win32Helper.IsAdministrator();
Plugins/Flow.Launcher.Plugin.Program/Programs/Win32.cs:3
- Several using directives (e.g.
System.Diagnostics
,System.Diagnostics.CodeAnalysis
,System.Threading.Channels
) appear unused; removing them will clean up the code.
using System.Diagnostics;
@check-spelling-bot Report🔴 Please reviewSee the 📂 files view, the 📜action log, or 📝 job summary for details.
See ❌ Event descriptions for more information. Forbidden patterns 🙅 (1)In order to address this, you could change the content to not match the forbidden patterns (comments before forbidden patterns may help explain why they're forbidden), add patterns for acceptable instances, or adjust the forbidden patterns themselves. These forbidden patterns matched content: s.b. workaround(s)
Pattern suggestions ✂️ (1)You could add these patterns to
Alternatively, if a pattern suggestion doesn't make sense for this project, add a If the flagged items are 🤯 false positivesIf items relate to a ...
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Flow.Launcher/Helper/AutoStartup.cs (1)
56-94
: Complex but well-structured administrator privilege handling.The logic correctly handles different scenarios based on user privileges:
- Administrators can fix both path and run level mismatches by rescheduling tasks
- Non-administrators can only fix path mismatches and appropriately throw
UnauthorizedAccessException
for run level issuesThe separation of concerns is good, but consider adding inline documentation for the complex conditional logic to improve maintainability.
Consider adding comments to clarify the administrator vs non-administrator logic:
+ // Administrator users can fix both path and run level issues if (_isAdministrator) { // If path or run level is not correct, we need to unschedule and reschedule the task if (!pathCorrect || !runLevelCorrect) { UnscheduleLogonTask(); ScheduleLogonTask(alwaysRunAsAdministrator); } } else { + // Non-administrator users can only fix path issues // If run level is not correct, we cannot edit it because we are not administrator // So we just throw an exception to let the user know if (!runLevelCorrect) { throw new UnauthorizedAccessException("Cannot edit task run level because the app is not running as administrator."); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Flow.Launcher/Helper/AutoStartup.cs
(7 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: gitStream.cm
- GitHub Check: build
🔇 Additional comments (7)
Flow.Launcher/Helper/AutoStartup.cs (7)
20-20
: Good optimization for administrator status caching.Using a static readonly field to cache the administrator status is an excellent performance optimization that avoids repeated expensive calls to
Win32Helper.IsAdministrator()
.
114-117
: Clean and correct run level validation logic.The method clearly encapsulates the run level validation with appropriate logic for both administrator and non-administrator scenarios.
99-103
: Appropriate exception handling for administrator privilege scenarios.The specific handling of
UnauthorizedAccessException
is well-implemented, maintaining consistency with the existing exception handling pattern while providing specific context for privilege-related errors.
208-219
: Security-conscious task scheduling logic.The conditional logic correctly ensures that
TaskRunLevel.Highest
is only set when both the user is an administrator AND has explicitly requested always-run-as-admin functionality. This prevents permission errors and maintains security best practices.
153-166
: Correct handling of registry vs logon task startup methods.The implementation correctly recognizes that registry-based startup cannot support administrator privileges, explicitly setting
alwaysRunAsAdministrator
to false for registry methods. The explanatory comment clarifies this limitation well.
22-22
: Consistent parameter threading maintains clean architecture.The
alwaysRunAsAdministrator
parameter is consistently threaded through all relevant methods, maintaining clean separation of concerns and avoiding global state dependencies.Also applies to: 33-33, 46-46, 153-153, 157-157, 188-188, 194-194
70-78
:✅ Verification successful
Verify task scheduling behavior when admin privileges are lost.
Consider testing the edge case where the application was originally started as administrator (allowing task creation), but is later run as a standard user. The current logic may attempt to reschedule tasks even when the user lacks the necessary privileges.
Run the following script to check for similar edge case handling in the codebase:
🏁 Script executed:
#!/bin/bash # Search for other instances of administrator privilege checking and task scheduling rg -A 5 -B 5 "TaskService.*RegisterTaskDefinition|UnauthorizedAccessException.*task|administrator.*task"Length of output: 3239
Scheduling protected by admin‐only guard
The
if (_isAdministrator)
check aroundUnscheduleLogonTask()
/ScheduleLogonTask()
ensures that no rescheduling is attempted when the app is launched without administrator privileges, and anUnauthorizedAccessException
is already thrown if only the run level is incorrect. The SettingsPaneGeneralViewModel also prompts for elevation when toggling the “run as administrator” setting. No further changes are needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for always running Flow Launcher as administrator by extending startup configuration, plugin behavior, and UI elements.
- Introduces a custom UAC dialog (
UACDialog
) for plugin-managed elevation prompts. - Extends AutoStartup and settings pages to toggle and persist “Always run as administrator,” handling logon task run levels accordingly.
- Implements restart-as-admin logic in the main App flow and updates Program plugin launch methods to respect admin context.
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 5 comments.
Show a summary per file
File | Description |
---|---|
Plugins/Flow.Launcher.Plugin.Program/UACDialog.xaml(.cs) | Adds a custom dialog for user account control prompts. |
Plugins/Flow.Launcher.Plugin.Program/Programs/Win32.cs | Refactors launch logic into Launch(bool elevated) , integrates UAC dialog, fixes verb usage. |
Plugins/Flow.Launcher.Plugin.Program/Programs/UWPPackage.cs | Harmonizes UWP package launch with new elevated parameter. |
Plugins/Flow.Launcher.Plugin.Program/Main.cs | Adds IsAdmin flag and IsAdministrator() helper. |
Flow.Launcher/SettingPages/Views/SettingsPaneGeneral.xaml | Adds “Always run as administrator” toggle to general settings UI. |
Flow.Launcher/SettingPages/ViewModels/SettingsPaneGeneralViewModel.cs | Implements new setting logic and restart prompt on elevation change. |
Flow.Launcher/Helper/AutoStartup.cs | Parameterizes task scheduling run level with admin option. |
Flow.Launcher/App.xaml.cs | Adds RestartAppAsAdministrator() and integrates it in startup failure handling. |
Flow.Launcher.Infrastructure/Win32Helper.cs | Exposes IsAdministrator() helper. |
Flow.Launcher/MainWindow.xaml.cs | Updates tray icon text to indicate admin mode. |
private static UACDialog msgBox; | ||
private static MessageBoxResult _result = MessageBoxResult.None; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using static fields for msgBox
and _result
can lead to race conditions if Show
is invoked concurrently. Consider making these instance members or synchronizing access.
private static UACDialog msgBox; | |
private static MessageBoxResult _result = MessageBoxResult.None; | |
private MessageBoxResult _result = MessageBoxResult.None; |
Copilot uses AI. Check for mistakes.
}; | ||
|
||
// Set icon & app name & program location | ||
_ = msgBox.SetImageAsync(iconPath); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The SetImageAsync
call is not awaited, so the dialog may show before the icon is loaded. Consider await
ing the task or loading the image synchronously before ShowDialog()
.
_ = msgBox.SetImageAsync(iconPath); | |
await msgBox.SetImageAsync(iconPath); |
Copilot uses AI. Check for mistakes.
using System.IO; | ||
using System.Linq; | ||
using System.Security; | ||
using System.Text; | ||
using System.Threading.Channels; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The System.Threading.Channels
namespace is not used in this file. You can remove this using directive to reduce noise.
using System.Threading.Channels; |
Copilot uses AI. Check for mistakes.
} | ||
} | ||
|
||
_ = Task.Run(() => Main.StartProcess(Process.Start, info)).ConfigureAwait(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling ConfigureAwait(false)
on a fire-and-forget Task.Run
has no effect and may confuse readers. Remove ConfigureAwait(false)
.
_ = Task.Run(() => Main.StartProcess(Process.Start, info)).ConfigureAwait(false); | |
_ = Task.Run(() => Main.StartProcess(Process.Start, info)); |
Copilot uses AI. Check for mistakes.
Verb = "runas", | ||
}; | ||
Process.Start(startInfo); | ||
Thread.Sleep(500); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using Thread.Sleep
can block the UI thread during shutdown. Consider waiting on the process handle or using a non-blocking delay.
Thread.Sleep(500); | |
await Task.Delay(500); |
Copilot uses AI. Check for mistakes.
Support Always Run As Administrator
Since some plugins may need administrator mode for handling sth, we need to support admin mode for Flow Launcher.
We can edit RunLevel configuration for logon task and then Flow Launcher can be launched as admin mode.
One more thing
We need to manually config
Program
plugin so that it can handle running application as non-admin/admin correctly.Resolve #2639
Test