[Installer] Graceful close and a way out of a stuck update (#7687)
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] Stop calling the payload free while an executable is locked

The guard answered "nothing is holding the payload" from two lookups that can
both miss a running instance: Restart Manager does not reliably report a
program's own image or the modules it mapped, and the image path of a process
running under another account cannot be read from the probe at all. A user log
showed exactly that: the first guard passed while cockatrice.exe was running
from the directory, the old uninstaller silently skipped the file it could not
delete, and the update aborted half an uninstall later with "cancelled by user"
after a close attempt the running window ignored.

Ask the file itself. A running image is opened without write sharing, so the
write open replacing it needs is refused with ERROR_SHARING_VIOLATION for as
long as the program runs, while a file that is merely being read still accepts
it. When neither lookup named anybody and a payload executable refuses that
open, the probe answers that the file is in use and that the program holding it
could not be identified - the guard's "could not confirm" dialog, whose Retry
works once the user has closed it - instead of passing the payload as free.

Only the payload executables are tested, not every file the purge removes: a
locked executable proves a program is running and a mapped DLL belongs to a
running executable, while a locked DLL or qt.conf can be the asking process
itself, which is the loop the guard already had. The open is retried once after
300 ms, and only for the files that refused, so a scanner holding a file for a
moment cannot stop an install for nothing.

The Windows probe check now covers that answer: a payload executable held by
the asking process, so that it cannot be named, has to come back as in use
rather than as free.

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>

* [Installer] Close running clients over a local socket

The installer could only ask a running client to close with WM_CLOSE, which an instance hidden in the notification area or sitting on its own confirmation cannot answer usefully. Give every client a per-process local socket and have the holder probe ask it there first, falling back to WM_CLOSE for clients from before the socket.

* [Installer] Offer to check again or update by hand when files stay locked

Replace the two-answer guard prompt with force close, close by hand and check again, or cancel. Retry the automatic close while the set of holders changes, and point a cancelled run at the latest release so a user who does not want anything killed has a way forward that is not cancelling the update again.

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
This commit is contained in:
sentry[bot] 2026-10-31 15:32:59 +01:00 • committed by GitHub
parent 720adf5410
commit bbc7e462ff
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
25 changed files with 897 additions and 130 deletions

View file

@ -0,0 +1,101 @@
#include "installer_close_name.h"
#include "installer_close_server.h"
#include <QByteArray>
#include <QCoreApplication>
#include <QEventLoop>
#include <QLocalSocket>
#include <QString>
#include <QTimer>
#include <QtGlobal>
#include <gtest/gtest.h>
#include <libcockatrice/utility/installer_close_protocol.h>
#include <string>
namespace
{
constexpr int HANDSHAKE_TIMEOUT_MS = 5000;
/// The bytes both ends exchange, from the header they both include. The separate
/// WireCommandIsThePinnedBytes test is what catches an accidental change to the literal.
const QByteArray CLOSE_COMMAND = QByteArray::fromRawData(INSTALLER_CLOSE_COMMAND, sizeof(INSTALLER_CLOSE_COMMAND) - 1);
/// Scopes the socket to this test process, so a Cockatrice the developer happens to be
/// running cannot claim the name and the test cannot close it. Kept short on purpose:
/// QLocalServer on Unix puts the socket file under QDir::tempPath(), and macOS caps that
/// path at 104 bytes (sun_path). A longer name passes on Linux (/tmp) and fails to listen
/// on the macOS CI runners, whose temp directory is already ~48 characters.
QByteArray testUserName()
{
return QByteArrayLiteral("t") + QByteArray::number(QCoreApplication::applicationPid());
}
} // namespace
// The probe builds the name from the process id and the account name it reads from the
// environment; the client builds it with scopedLocalServerName(). They have to produce the
// same string or the installer can never reach the instance it is trying to close.
TEST(InstallerCloseServerTest, SocketNameMatchesTheProbeForTheSameProcessAndUser)
{
constexpr unsigned long PROCESS_ID = 4242;
const QString userName = QStringLiteral("installer-test");
const QString expected = QStringLiteral("CockatriceInstallerClose-4242-installer-test");
EXPECT_EQ(expected, InstallerCloseServer::socketNameForUser(PROCESS_ID, userName));
EXPECT_EQ(expected.toStdWString(), installerCloseSocketName(PROCESS_ID, userName.toStdWString()));
}
TEST(InstallerCloseServerTest, SocketNameScopesTheProcessIdByTheCurrentUser)
{
qputenv("USER", testUserName());
EXPECT_EQ(QStringLiteral("CockatriceInstallerClose-4242-") + QString::fromLatin1(testUserName()),
InstallerCloseServer::socketName(4242));
}
// The line is a wire format shared with the Windows-only probe, which cannot be compiled
// into this test. Pinning the exact bytes here makes a change to the shared header a
// deliberate protocol change rather than a typo that only shows up in the installer.
TEST(InstallerCloseServerTest, WireCommandIsThePinnedBytes)
{
EXPECT_EQ(QByteArrayLiteral("COCKATRICE_INSTALLER_CLOSE\n"), CLOSE_COMMAND);
EXPECT_EQ(27, CLOSE_COMMAND.size());
}
// The installer only writes the command; the listening instance is what turns it into a
// close. Anything but that exact line must not be treated as a request.
TEST(InstallerCloseServerTest, CloseRequestIsEmittedForTheProbeCommand)
{
qputenv("USER", testUserName());
InstallerCloseServer server;
ASSERT_TRUE(server.listen()) << "the installer close socket could not be started";
int closeRequests = 0;
QObject::connect(&server, &InstallerCloseServer::closeRequested, &server, [&closeRequests]() { ++closeRequests; });
QLocalSocket socket;
socket.connectToServer(InstallerCloseServer::socketName(QCoreApplication::applicationPid()));
ASSERT_TRUE(socket.waitForConnected(HANDSHAKE_TIMEOUT_MS)) << "could not reach the installer close socket";
ASSERT_EQ(CLOSE_COMMAND.size(), socket.write(CLOSE_COMMAND)) << "the close request was not accepted";
// No waitForBytesWritten(): on Windows QLocalSocket completes the write through the
// event dispatcher, and this test runs the server in the same thread, so a blocking
// wait would hold the loop the server needs to accept and read. The loop below is the
// real handshake: it flushes the write and delivers the server's readyRead.
QEventLoop loop;
QTimer timeout;
timeout.setSingleShot(true);
QObject::connect(&timeout, &QTimer::timeout, &loop, &QEventLoop::quit);
QObject::connect(&server, &InstallerCloseServer::closeRequested, &loop, &QEventLoop::quit);
timeout.start(HANDSHAKE_TIMEOUT_MS);
loop.exec();
EXPECT_EQ(1, closeRequests) << "the probe's line must reach the close handler exactly once";
}
int main(int argc, char **argv)
{
QCoreApplication app(argc, argv);
::testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
}