Commit graph

3 commits

Author SHA1 Message Date
sentry[bot]
ded83aaab5
[Installer] Ask a hidden instance to close, then offer to end it once (#7683)
Some checks are pending
CodeQL / Analyze (cpp) (push) Waiting to run
CodeQL / Analyze (actions) (push) Waiting to run
Build Desktop / Configure (push) Waiting to run
Build Desktop / Debian 13 (push) Blocked by required conditions
Build Desktop / Debian 12 (push) Blocked by required conditions
Build Desktop / Fedora 44 (push) Blocked by required conditions
Build Desktop / Fedora 43 (push) Blocked by required conditions
Build Desktop / Servatrice_Debian 12 (push) Blocked by required conditions
Build Desktop / Ubuntu 24.04 (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 (push) Blocked by required conditions
Build Desktop / Arch (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 cover (push) Blocked by required conditions
Build Desktop / macOS 13 Intel (push) Blocked by required conditions
Build Desktop / macOS 14 (push) Blocked by required conditions
Build Desktop / macOS 15 (push) Blocked by required conditions
Build Desktop / macOS 26 Debug (push) Blocked by required conditions
Build Desktop / Windows 10 (push) Blocked by required conditions
Build Desktop / Prune caches (push) Waiting to run
Build Docker / Servatrice (arm) (push) Waiting to run
Build Docker / Servatrice (x86) (push) Waiting to run
Build Docker / Publish multi-platform Servatrice image (push) Blocked by required conditions
* [Installer] Request quit for hidden Cockatrice instances and record holders in abort telemetry

* [Installer] Fix compiler warnings in holder probe and installer quit handler

* [VisualDeckStorage] Order the includes the way clang-format wants them

* [Installer] Ask a hidden instance to close, then offer to end it once

The previous attempt had the client answer a registered window message, which
cannot work for the update it was reported on: the instance being replaced is a
client from before that message exists, so it never answers, and the user is
left with "Cockatrice is still running" exactly as reported. Every step that can
free the files has to be the installer's, so the client side is reverted.

The probe now posts WM_CLOSE to the instance's own window whether that window is
on screen or only in the notification area: the request does not need a visible
window, an instance in the tray is the one this update has no other way to
reach, and the confirmation Cockatrice answers with is a window Qt shows itself,
so it is painted - unlike one restored from outside. A visible window is brought
forward first, and Qt's bookkeeping windows stay excluded, because a close to the
notification-area one terminates the instance without its confirmation.

A question of the instance's own no longer suppresses the close. Withholding it
protected nobody: the installer asked the user to close a program it had never
asked, and a current client refuses the close while its confirmation is pending
anyway, leaving the question where it is. The launcher mode keeps the
visible-only rule, because a launcher without a window has already answered its
own close and the wait ends it after a grace instead of asking a user who is not
there.

The guard asked the same question until the user gave up: after the automatic
close failed it offered to end the programs, then re-ran the probe and showed
the same dialog word for word when the answer was No, which is how an abort was
reported three times over. It now asks once - Yes ends the Cockatrice, Oracle
and Servatrice programs the payload installs and says what that costs, No
cancels - and what survives an end is named in the abort instead of being put to
the user as the same question again. The uninstaller's guard and the recovery
path after a failed purge follow the same shape.

Kept: the abort telemetry that records the holders, the close result and the end
result as event data.

---------

Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com>
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
2026-10-29 21:01:16 +01:00
sentry[bot]
bf3562a85a
[Installer] Offer to end running Cockatrice immediately when auto-close fails (#7675)
Some checks are pending
CodeQL / Analyze (cpp) (push) Waiting to run
CodeQL / Analyze (actions) (push) Waiting to run
Build Desktop / Configure (push) Waiting to run
Build Desktop / Debian 13 (push) Blocked by required conditions
Build Desktop / Debian 12 (push) Blocked by required conditions
Build Desktop / Fedora 44 (push) Blocked by required conditions
Build Desktop / Fedora 43 (push) Blocked by required conditions
Build Desktop / Servatrice_Debian 12 (push) Blocked by required conditions
Build Desktop / Ubuntu 24.04 (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 (push) Blocked by required conditions
Build Desktop / Arch (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 cover (push) Blocked by required conditions
Build Desktop / macOS 13 Intel (push) Blocked by required conditions
Build Desktop / macOS 14 (push) Blocked by required conditions
Build Desktop / macOS 15 (push) Blocked by required conditions
Build Desktop / macOS 26 Debug (push) Blocked by required conditions
Build Desktop / Windows 10 (push) Blocked by required conditions
Build Desktop / Prune caches (push) Waiting to run
Build Docker / Servatrice (arm) (push) Waiting to run
Build Docker / Servatrice (x86) (push) Waiting to run
Build Docker / Publish multi-platform Servatrice image (push) Blocked by required conditions
* [Installer] Offer to end running Cockatrice immediately when auto-close fails

* [Installer] Keep the way out that costs nothing in the first prompt

The prompt now offers to end Cockatrice, so what the deleted close-again text
said has to survive in it: an instance the probe reports as waiting for an
answer to a question of its own has been brought forward, and answering that
window is the way out that ends nothing. Say so, and say where to go when Yes
leaves it listed - Task Manager - so an end that cannot take effect does not
loop on the same dialog.

* [Installer] Bring the comments back in line with the code

GuardCloseTried no longer counts closes, and the file header, CloseHolders and
GuardAppsNotRunning still described an escalation after a second close, as did
the probe's --end-holders note.

* [Installer] End Oracle and Servatrice too when the guard's end answer is confirmed

The guard's end answer only ever stopped cockatrice.exe, so an Oracle holding
the card database or a Servatrice service kept the payload locked and the
prompt came back with nothing left that could free it.

The probe now ends every program the payload installs, services included:
isPayloadProgram() - the list restart_manager_holders already used for its
parent walk - decides the set, so a scanner holding a DLL or a program the
user keeps in the install directory is still named and left to the user. A
service is terminated rather than sent a stop through the SCM, which notices
the process exit and marks it stopped afterwards.

The guard prompt now says what ending costs: an unsaved deck, an unwritten
setting, players on a running Servatrice, and an Oracle mid-way through the
card database. It also keeps the notification-area hint, which the probe's
"has no window to ask" answer depends on, and drops the claim that Oracle
and Servatrice had been brought forward.

* [Installer] Give every running-program prompt the same end-or-retry answers

Four prompts meet a running program and each offered a different set: the
guard ended it, the launcher wait only kept waiting, and the recovery prompt
after a failed purge could only ask Cockatrice to close. With Oracle or
Servatrice holding the files, that last one had no answer that could free
anything - Yes and No both retried the purge and Cancel aborted an install
whose old version was already gone.

All four now follow the guard's shape: one automatic attempt, then a prompt
whose Yes ends the programs it names and whose No retries, with Cancel to
give up.

- RecoverFromHolders closes once and purges before it asks anything, then
  offers EndHolders - the same end answer as the guard - instead of a close
  the user had to trigger himself.
- WaitForLauncherToExit can end the launcher by pid, the one process it
  holds a pid for, rather than looping 60 s waits with Cancel as the only
  way out.
- END_COST and AFTER_CLOSING_GUIDANCE are !defines, so the cost of ending
  and the guidance for the answers that end nothing read the same in every
  prompt.

* [Installer] End the launcher through the handle it was waited on

The "end it" answer used to look the launcher's pid up again after a dialog that
can stay open as long as the user takes, so a pid that had been reused in the
meantime - or a /PID= that was never the launcher - would be terminated instead.
The handle the wait already holds names that process and cannot be handed to
another, so the terminate answer now acts on it.

PROCESS_TERMINATE is requested along with PROCESS_SYNCHRONIZE; when only the
synchronize right is granted the launcher is still waited on and the answer falls
back to asking it to close, instead of reporting a live launcher as gone and
writing the payload over it.

* [Installer] Say what the end answer ends, and report a failed end

The prompt offered "end every program listed above", but EndHolders only ends the
programs the payload installs: a scanner holding a payload DLL, or a service the
Restart Manager names by its long name, was listed and then silently left alone, so
Yes looked like it did nothing and the same dialog came back unchanged.

The wording now says which programs Yes ends and that anything else has to be closed
by hand, and the round after an end that did not free the files says so, so the same
dialog no longer reappears word for word. The three prompts are one macro so they
cannot drift apart again.

The added sentences eat into the 1024 characters one NSIS string holds, so the
holder line's budget drops from 400 to 320 characters to keep the longest message
(cancel line included) inside it: 977 characters, measured against the worst case
the probe can produce.

---------

Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com>
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
2026-10-27 19:09:21 +01:00
BruebachL
bbc6003f0a
[CrashReporting] Name the files and processes behind a failed installer purge (#7590)
Some checks are pending
CodeQL / Analyze (cpp) (push) Waiting to run
CodeQL / Analyze (actions) (push) Waiting to run
Build Desktop / Configure (push) Waiting to run
Build Desktop / Debian 13 (push) Blocked by required conditions
Build Desktop / Debian 12 (push) Blocked by required conditions
Build Desktop / Fedora 44 (push) Blocked by required conditions
Build Desktop / Fedora 43 (push) Blocked by required conditions
Build Desktop / Servatrice_Debian 12 (push) Blocked by required conditions
Build Desktop / Ubuntu 24.04 (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 (push) Blocked by required conditions
Build Desktop / Arch (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 cover (push) Blocked by required conditions
Build Desktop / macOS 13 Intel (push) Blocked by required conditions
Build Desktop / macOS 14 (push) Blocked by required conditions
Build Desktop / macOS 15 (push) Blocked by required conditions
Build Desktop / macOS 26 Debug (push) Blocked by required conditions
Build Desktop / Windows 10 (push) Blocked by required conditions
Build Desktop / Prune caches (push) Waiting to run
Build Docker / Servatrice (arm) (push) Waiting to run
Build Docker / Servatrice (x86) (push) Waiting to run
Build Docker / Publish multi-platform Servatrice image (push) Blocked by required conditions
* [CrashReporting] Log which files and processes blocked the installer purge

A purge that fails aborts the upgrade with a message that can only guess
at the cause, and the log only listed which DLLs survived. Record what
Windows reports for each survivor and which processes still have the
install directory mapped, then repeat the holder list in the abort
message.

The diagnostics are additive and run only on the abort path: they open
each survivor for DELETE access and close it again without deleting, so a
failed upgrade can never end up half-removed. The purge itself is
unchanged.

* [CrashReporting] Name the holders of a failed purge with Restart Manager

The process-and-module scan could only name a holder whose image lives in the
install directory or that loaded one of its DLLs, so the case this diagnostic
exists for - an antivirus scanner holding a plain handle - was reported as "no
holder named, only a code". Reaching the Restart Manager from the NSIS script
would have meant marshalling RM_PROCESS_INFO through System::Call as a table of
hand-computed offsets, which cannot be checked without a Windows toolchain, and
that is why it was dropped when this diagnostics was written.

Move the call into tools/installer-holder-probe: a small executable the same
Windows build compiles, linking nothing from this project so it still starts
when the installed runtime is what the purge could not replace. It rides inside
the installer and is extracted to $PLUGINSDIR only when the purge has already
failed, so the successful path is unchanged and nothing new is installed.

.ci/compile.sh runs it against a file held with FileShare.None on the Windows
runners, so the lookup is exercised on real Windows instead of only compiled.

* [CrashReporting] Fix the holder probe check and cap the holder line

The probe block ran after the package steps, where the working directory is
already the build dir, so its $BUILD_DIR-prefixed paths named build/build and
the PowerShell file could not be written. Move it ahead of install/package,
where the build dir is still the cwd, and name both files relative to it.
Guard it on MAKE_NO_CLIENT, since the probe is only built with the client.

Cap the rendered holder line at HOLDER_LINE_CHAR_MAX: the count limit alone
does not keep long service names inside the NSIS string that also carries the
abort text and the log path. Whole entries are kept and the rest counted, with
the first holder always shown.

Also run cmake-format on the probe's CMakeLists so the lint job accepts it.

* [CrashReporting] Report a read-only purge survivor and bound the probe check

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
2026-10-24 00:46:54 +02:00