mirror of
https://github.com/microsoft/PowerToys.git
synced 2025-12-16 11:48:06 +01:00
[Awake] Fix lack of process validation for --pid and --use-parent-pid options (#41803)
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request This adds validation for the "PID binding" modes of Awake. Previously, Awake did not validate that a user-supplied process ID actually corresponded to a running process (leading to an infinite keep-awake duration); nor did it validate that the parent process could be found and bound to when using the `--use-parent-pid` option (which left Awake in an unresponsive state without setting a keep-awake mode). This PR fixes those issues by validating that the process exists when using `--pid` (or when the PID comes from PowerToys Runner itself), and also early-exits if the parent process cannot be bound to when using `--use-parent-pid`. This supersedes a prior PR which just fixes the `--use-parent-pid`-related flaw, #41744. <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #41709, #41722 - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [ ] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [ ] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx <!-- Provide a more detailed description of the PR, other things fixed, or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments For the `--pid` fix, this is validated both when the command line is parsed (by extending the existing `pidOption` validator) and just before the process ID is bound to (in `HandleCommandLineArguments` and a new `HandleProcessScopedKeepAwake` method). These use a new `ProcessExists` method which checks that the process exists (funnily enough) and isn't exiting. I also added a very paranoid check that the process ID isn't Awake's own. This couldn't be done deliberately, but if a user mis-typed their desired PID and it happened to match Awake's, this would lead to an indefinite keep-awake. It's a very remote possibility, but one-in-ten-thousand odds still happen. The fix for the `--use-parent-pid` checks the return value of the `Manager.GetParentProcess` call, which was previously lacking, exiting early if it sees a `0` failure value. Added validation for PID value not being zero or negative. There are new string resources for the general PID-binding failure and the specific parent process binding issue. I don't actually know why these are resources, but I followed the existing convention from the project. <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed Tested that: - When `null` (`0` when marshalled) is returned from the `GetParentProcess` path that Awake exits early and does not enter its failed state. - When a non-existent PID is input via the `--pid` command line that Awake exits early and does not attempt to bind to a non-existent process. - PID-binding still works without issue when a correct process ID is provided on the command line. - `--use-parent-pid` still works when the parent process can be located and bound to. - New PID-binding parameter checks are caught (0 or negative numbers are rejected). - Other modes still work as expected.
This commit is contained in:
@@ -157,9 +157,33 @@ namespace Awake
|
||||
|
||||
pidOption.AddValidator(result =>
|
||||
{
|
||||
if (result.Tokens.Count != 0 && !int.TryParse(result.Tokens[0].Value, out _))
|
||||
if (result.Tokens.Count == 0)
|
||||
{
|
||||
string errorMessage = $"PID value in --pid could not be parsed correctly. Check that the value is valid and falls within the boundaries of Windows PID process limits. Value used: {result.Tokens[0].Value}.";
|
||||
return;
|
||||
}
|
||||
|
||||
string tokenValue = result.Tokens[0].Value;
|
||||
|
||||
if (!int.TryParse(tokenValue, out int parsed))
|
||||
{
|
||||
string errorMessage = $"PID value in --pid could not be parsed correctly. Check that the value is valid and falls within the boundaries of Windows PID process limits. Value used: {tokenValue}.";
|
||||
Logger.LogError(errorMessage);
|
||||
result.ErrorMessage = errorMessage;
|
||||
return;
|
||||
}
|
||||
|
||||
if (parsed <= 0)
|
||||
{
|
||||
string errorMessage = $"PID value in --pid must be a positive integer. Value used: {parsed}.";
|
||||
Logger.LogError(errorMessage);
|
||||
result.ErrorMessage = errorMessage;
|
||||
return;
|
||||
}
|
||||
|
||||
// Process existence check. (We also re-validate just before binding.)
|
||||
if (!ProcessExists(parsed))
|
||||
{
|
||||
string errorMessage = $"No running process found with an ID of {parsed}.";
|
||||
Logger.LogError(errorMessage);
|
||||
result.ErrorMessage = errorMessage;
|
||||
}
|
||||
@@ -216,6 +240,25 @@ namespace Awake
|
||||
Manager.CompleteExit(exitCode);
|
||||
}
|
||||
|
||||
private static bool ProcessExists(int processId)
|
||||
{
|
||||
if (processId <= 0)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
// Throws if the Process ID is not found.
|
||||
using var p = Process.GetProcessById(processId);
|
||||
return !p.HasExited;
|
||||
}
|
||||
catch
|
||||
{
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
private static void HandleCommandLineArguments(bool usePtConfig, bool displayOn, uint timeLimit, int pid, string expireAt, bool useParentPid)
|
||||
{
|
||||
if (pid == 0 && !useParentPid)
|
||||
@@ -271,6 +314,12 @@ namespace Awake
|
||||
|
||||
if (pid != 0)
|
||||
{
|
||||
if (!ProcessExists(pid))
|
||||
{
|
||||
Logger.LogError($"PID {pid} does not exist or is not accessible. Exiting.");
|
||||
Exit(Resources.AWAKE_EXIT_PROCESS_BINDING_FAILURE_MESSAGE, 1);
|
||||
}
|
||||
|
||||
Logger.LogInfo($"Bound to target process while also using PowerToys settings: {pid}");
|
||||
|
||||
RunnerHelper.WaitForPowerToysRunner(pid, () =>
|
||||
@@ -287,28 +336,7 @@ namespace Awake
|
||||
}
|
||||
else if (pid != 0 || useParentPid)
|
||||
{
|
||||
// Second, we snap to process-based execution. Because this is something that
|
||||
// is snapped to a running entity, we only want to enable the ability to set
|
||||
// indefinite keep-awake with the display settings that the user wants to set.
|
||||
// In this context, manual (explicit) PID takes precedence over parent PID.
|
||||
int targetPid = pid != 0 ? pid : useParentPid ? Manager.GetParentProcess()?.Id ?? 0 : 0;
|
||||
|
||||
if (targetPid != 0)
|
||||
{
|
||||
Logger.LogInfo($"Bound to target process: {targetPid}");
|
||||
|
||||
Manager.SetIndefiniteKeepAwake(displayOn, targetPid);
|
||||
|
||||
RunnerHelper.WaitForPowerToysRunner(targetPid, () =>
|
||||
{
|
||||
Logger.LogInfo($"Triggered PID-based exit handler for PID {targetPid}.");
|
||||
Exit(Resources.AWAKE_EXIT_BINDING_HOOK_MESSAGE, 0);
|
||||
});
|
||||
}
|
||||
else
|
||||
{
|
||||
Logger.LogError("Not binding to any process.");
|
||||
}
|
||||
HandleProcessScopedKeepAwake(pid, useParentPid, displayOn);
|
||||
}
|
||||
else
|
||||
{
|
||||
@@ -344,6 +372,62 @@ namespace Awake
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Start a process-scoped keep-awake session. The application will keep the system awake
|
||||
/// indefinitely until the target process terminates.
|
||||
/// </summary>
|
||||
/// <param name="pid">The explicit process ID to monitor.</param>
|
||||
/// <param name="useParentPid">A flag indicating whether the application should monitor its
|
||||
/// parent process.</param>
|
||||
/// <param name="displayOn">Whether to keep the display on during the session.</param>
|
||||
private static void HandleProcessScopedKeepAwake(int pid, bool useParentPid, bool displayOn)
|
||||
{
|
||||
int targetPid = 0;
|
||||
|
||||
// We prioritize a user-provided PID over the parent PID. If both are given on the
|
||||
// command line, the --pid value will be used.
|
||||
if (pid != 0)
|
||||
{
|
||||
if (pid == Environment.ProcessId)
|
||||
{
|
||||
Logger.LogError("Awake cannot bind to itself, as this would lead to an indefinite keep-awake state.");
|
||||
Exit(Resources.AWAKE_EXIT_BIND_TO_SELF_FAILURE_MESSAGE, 1);
|
||||
}
|
||||
|
||||
if (!ProcessExists(pid))
|
||||
{
|
||||
Logger.LogError($"PID {pid} does not exist or is not accessible. Exiting.");
|
||||
Exit(Resources.AWAKE_EXIT_PROCESS_BINDING_FAILURE_MESSAGE, 1);
|
||||
}
|
||||
|
||||
targetPid = pid;
|
||||
}
|
||||
else if (useParentPid)
|
||||
{
|
||||
targetPid = Manager.GetParentProcess()?.Id ?? 0;
|
||||
|
||||
if (targetPid == 0)
|
||||
{
|
||||
// The parent process could not be identified.
|
||||
Logger.LogError("Failed to identify a parent process for binding.");
|
||||
Exit(Resources.AWAKE_EXIT_PARENT_BINDING_FAILURE_MESSAGE, 1);
|
||||
}
|
||||
}
|
||||
|
||||
// We have a valid non-zero PID to monitor.
|
||||
Logger.LogInfo($"Bound to target process: {targetPid}");
|
||||
|
||||
// Sets the keep-awake plan and updates the tray icon.
|
||||
Manager.SetIndefiniteKeepAwake(displayOn, targetPid);
|
||||
|
||||
// Synchronize with the target process, and trigger Exit() when it finishes.
|
||||
RunnerHelper.WaitForPowerToysRunner(targetPid, () =>
|
||||
{
|
||||
Logger.LogInfo($"Triggered PID-based exit handler for PID {targetPid}.");
|
||||
Exit(Resources.AWAKE_EXIT_BINDING_HOOK_MESSAGE, 0);
|
||||
});
|
||||
}
|
||||
|
||||
private static void AllocateLocalConsole()
|
||||
{
|
||||
Manager.AllocateConsole();
|
||||
|
||||
Reference in New Issue
Block a user