Commit graph

2 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
BruebachL
c6f682f9cf
[CrashReporting] Offer to send the installer log on abort (#7524)
* [CrashReporting] Installer log send offer on abort (inc 4)

DSN parsed to CPACK_SENTRY_* (scheme-aware for http self-hosts) before
include(CPack); new SendInstallerLog.ps1.in builds a byte-accurate
Sentry envelope (128 KiB tail cap, X-Sentry-Auth, exit 0/1) with the
abort reason read from the log file itself, so the NSIS side needs no
string functions and quoting stays trivial. AbortInstall asks consent
(attended installs only, silent path untouched, abort never
swallowed). Empty/malformed DSN disables the offer; no secrets in repo.

* [CrashReporting] Embed installer-log sender via a relative NSIS path

The AbortInstall hook embedded SendInstallerLog.ps1 through the absolute
CPACK_SENTRY_PS1_PATH, which carries CMake's forward slashes (D:/a/...).
makensis does not treat a forward-slash drive path as absolute, so it
resolved it relative to project.nsi, found nothing and aborted packaging
on the Windows jobs of every branch in the stack.

Embed it with the same '..\..\..\' relative path that already resolves
NSIS.definitions.nsh: both are configure_file'd into the build root,
three levels up from where CPack places project.nsi. Drop the now-unused
CPACK_SENTRY_PS1_PATH cache variable.

* [CrashReporting] Drop the installer /CRASHTEST hook, tidy the log sender

Remove the /CRASHTEST switch and the matching COCKATRICE_SENTRY_DSN
environment override on the installer abort path. With no deliberate
in-installer trigger left, a package built without a baked DSN must never
offer to post a user's log to an endpoint chosen by whatever happens to be
in the environment, so the prompt is now gated on CPACK_SENTRY_ENABLED
alone. This also retires the comment that claimed /S /R /CRASHTEST reaches
the prompt: the switch was evaluated before /R set ReinstallMode, so that
form never showed it.

Alongside: correct the CPACK_SENTRY_RELEASE comment (PROJECT_VERSION keeps
its "-<label>" on Development tags where CPACK_PACKAGE_VERSION does not),
delete the unused CPACK_SENTRY_HOST/CPACK_SENTRY_PROJECT_ID cache vars,
document the DSN regex's path-prefix fallback, log a declined send, keep
the uploaded installer log free of helper-injected newlines, and bound
the post with -TimeoutSec/-NonInteractive.

* [CrashReporting] Derive the installer DSN vars as plain variables

Review nit: every CPACK_SENTRY_* value is recomputed from
COCKATRICE_SENTRY_DSN on each configure, so CACHE ... FORCE bought
nothing. Set empty defaults once, override them in the match branch, and
drop the duplicated else() copies. Still CPACK_-prefixed, so include(CPack)
still snapshots them into CPackConfig.cmake.

* [CrashReporting] Gate the installer log send at build time

Review nit: test CPACK_SENTRY_ENABLED with the NSIS preprocessor
instead of copying it into $SentryEnabled and branching at runtime, so a
package built without a DSN compiles the prompt out rather than carrying
dead code. The configure_file that produces the helper is gated the same
way, so a build without reporting neither references nor embeds it.

* [CrashReporting] Disclose the Windows user name in the send prompt

Review nit: say that the log can include the Windows user name (a real
part of what gets uploaded) and drop the "No account needed." clause,
which answered a question the prompt never raised.

* [CrashReporting] Show progress while the installer log is sent

Review nit: DetailPrint a line before nsExec runs, so the details pane
says something while the send is in flight instead of staying blank for
up to the 60 s /TIMEOUT.

* [CrashReporting] Rename the Sentry event local in the installer helper

Review nit: $Event is a PowerShell automatic variable, so name the local
$sentryEvent instead. Also drop $firstLine: Select-String returns the
matching line alone, so the reason is already one physical line and the
extra split added nothing.

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
2026-10-19 14:57:04 +02:00