mirror of https://github.com/ospab/ostp.git
3 Commits
| Author | SHA1 | Message | Date |
|---|---|---|---|
|
|
732d0bf5ae |
fix(installer): grant users permission to start the helper task
The task was registered correctly and pointed at the right binary — the app's own log confirmed the match — but starting it failed: run: schtasks /Run failed (Some(1)): ERROR: Access is denied. falling back to a direct elevated launch — this is the consent prompt Registering a task and being allowed to start one are separate things, and I had conflated them. The principal (BUILTIN\Users by SID, HighestAvailable) decides who the task runs AS. Who may START it comes from the task's security descriptor, and a task created by an elevated installer defaults to granting execution to Administrators only. So the unprivileged GUI was refused and fell back to prompting on every connect, exactly as before the installer existed. This also explains why manual testing said the opposite: running the task by hand happened from an elevated console, where it works, which pointed suspicion at the app for several rounds. Register-ScheduledTask cannot set a descriptor, so the hook now follows the registration with a SetSecurityDescriptor call through the Task Scheduler COM object: GA for Administrators and SYSTEM, GR+GX for BUILTIN\Users. A failure there is reported on its own rather than being folded into the success message, since the task would otherwise look registered while remaining unusable. |
|
|
|
234497759b |
fix(gui): config went to the working directory; installer wrote unusable XML
Three defects the first installer build exposed. Settings could not be read or saved, "os error 5". With no config beside the executable — which is the case for every fresh install — get_config_path fell back to a bare relative "config.json", resolved against the process working directory. Launched from a Start Menu shortcut that is whatever Windows chose, frequently C:\Windows\System32. On a writable working directory the silent outcome would have been worse than the error: settings persisting somewhere unrelated and appearing to vanish. The config now lives beside the executable only where that directory actually accepts writes, and otherwise under the user's own profile, carrying an existing read-only copy across once. Writability is measured, not inferred from the path: an install onto a data drive may well be writable where Program Files is not. The installer could not register the task: "The task XML is malformed. (1,2)::ERROR: incorrect document syntax". Writing it from NSIS emitted a UTF-16 byte-order mark ahead of content whose encoding depends on whether makensis was built in Unicode mode. Replaced with the ScheduledTasks cmdlets, which take the same settings as arguments — no file, so no encoding to get wrong. Verified the invocation reaches Register-ScheduledTask and fails only on "Access is denied" when unelevated, which is exactly what the elevated installer supplies. That command is delimited with backticks, NSIS's third quote character. As a single-quoted string it would have ended at PowerShell's first quote. "Copy failed" on wintun.dll: CopyFiles takes a destination directory, and it was given a file path. It is also guarded now, so a missing resource says so instead of failing mutely. Finally, per request, the app no longer registers the task itself — that is the installer's job alone. Without a task it goes straight to the direct elevated launch, which prompts per connect as it always did, rather than spending a prompt on a registration attempt and then another on the launch. |
|
|
|
8b5c0a3a8c |
feat(gui): register the helper task from an installer, not from the app
Elevation belongs to install time. Registering a task that runs elevated is itself privileged, so an unprivileged GUI can only obtain one by raising the very prompt we are trying to remove. There was nowhere to put it: the Windows GUI ships as a portable zip built with --no-bundle, so the project had no installer at all. Adds an NSIS one, whose POSTINSTALL hook registers the task while already elevated. Connecting then prompts zero times. NSIS over WiX because installerHooks is an NSIS feature; the MSI equivalent needs a custom action, which is more bespoke machinery, not less. installMode is perMachine — the default, currentUser, does not run elevated, and the hook would fail exactly as the in-app attempt did. The task's principal is the SID S-1-5-32-545 (BUILTIN\Users) with InteractiveToken rather than the installing user, so a machine-wide install serves every account instead of only whoever ran the installer; the name is localized and would not resolve. %LOCALAPPDATA% in the arguments is left unexpanded for the same reason — Task Scheduler expands it per running user. Also fixes the in-app fallback, which the portable zip still needs and which had never once worked. It trusted the exit code of an elevated schtasks, but -Verb RunAs launches through ShellExecute and a non-elevated parent generally cannot read the child's exit code: $p.ExitCode yields $null, and `exit $null` leaves PowerShell reporting 0 (measured, not assumed). Failure was arriving disguised as success. -Wait does not reliably block either, so deleting the task XML afterwards raced schtasks reading it. It now waits for the task to actually appear before deleting anything, and treats the exit code as advisory except for 1223, a declined prompt, which is worth failing fast on. Corrects one comment that asserted the opposite of the truth: schtasks writes UTF-16 to a console but UTF-8 with no BOM into a redirected pipe, which is the case that matters here. Only the fallback made the path check work at all. wintun.dll rides along as a bundled resource and the hook copies it beside the executables, since the helper loads it with a plain LoadLibrary. The uninstall hook removes both it and the task, so no stale registration is left pointing at a deleted binary. |