Qt's SVG renderer has no CSS cascade, so a `currentColor` fill has to be
substituted before the bytes reach QSvgRenderer.
- Match and replace on bytes: both the sentinel and the replacement are
ASCII, so the document never goes through a UTF-16 round trip, and files
that are not valid UTF-8 survive unchanged.
- Document that the match is a plain case-insensitive substring, so an
identifier that merely starts with the sentinel gets rewritten too.
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
deriveKey passes maxmem = 128nr + 128rp + 4096 to EVP_PBE_scrypt, but scrypt
also allocates XY at 256r bytes. costParamsAreSane accepts r up to 32, so any
server asking for r >= 17 made derivation fail and the client report a login
abort it could never recover from. Cover the XY array and add a regression test
at the accepted-range boundary.
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
The root already set CMAKE_AUTOMOC; add CMAKE_AUTOUIC and CMAKE_AUTORCC
next to it and drop the per-directory copies from 18 CMakeLists files.
These variables are inherited by the subdirectories, so the duplicates
never changed anything.
qt_standard_project_setup is not used: besides the AUTO* variables it
sets CMAKE_RUNTIME_OUTPUT_DIRECTORY on Windows, which breaks the
install(DIRECTORY "$<TARGET_FILE_DIR:...>") copies and the per-app
symbol paths, and appends to CMAKE_INSTALL_RPATH on Linux.
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [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>
* [Translation] Name languages without reading the translation files
The language selector derived every label from the language's own .qm file:
it loaded the file and translated the literal "English" in the "i18n"
context. lupdate could never extract that literal from the callers, which
pass the DEFAULT_LANG_NAME macro, so a dummy QT_TRANSLATE_NOOP in
cache_settings.cpp existed to give it something to extract. #7050 deleted
that dummy, the next lupdate push dropped the string from Transifex, and
the following pull rewrote the .ts files without it. lrelease only emits
translated messages, so 22 of 25 .qm files returned an empty name.
The names are data, not translations: a selector has to be readable before
any translation is loaded, and a label must not depend on the translation
of the language it names. TranslationLoader::languageDisplayName now holds
them next to availableLanguages, falling back to Qt's native language name
and then to the code, so a language added later still gets a label.
Also select the configured language by item data instead of by matching the
translated "English" string, which pointed at the wrong row whenever the
lookup came back empty, and restore that English entry's own name: en_US is
"English (United States)" so it can no longer be confused with the
untranslated default.
Oracle shared the code path, so its wizard language page is fixed too.
* [Translation] Name the default language selection once
Review follow-up: log the unresolvable language through the page's own
category, and move the en_US entry both selectors fall back to for an unset
setting into TranslationLoader::defaultLanguageCode.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [QtUtils] Add layout creation and relayout helpers
* Swap parameter order (spacing before margin) and remove static from relayout()
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Search] Stop allocating per card in card-name completion
CardSearchModel::updateSearchResults() runs on every textEdited()
signal, walks the whole card database and runs Levenshtein against every
matching name. Three costs dominated that path:
1. Every candidate built a QStringList of its searchable names, then
lowercased each name into another QString, only to break out of the
loop after the first hit.
2. The sort comparator called sortableName() for each comparison, which
re-fetched the name (QMap lookup plus QString construction) O(n log n)
times.
3. levenshteinDistance() allocated a full len1 x len2 vector of vectors.
The search model now lowercases the one name it tests directly, stores
the tie-breaking name length once per result, and the comparator only
compares integers. levenshteinDistance() uses the standard two-row
formulation: same result, O(len2) memory instead of O(len1*len2), with
early exits for empty and equal strings.
FilterString query closures get the same treatment:
- Set queries iterate the set map directly instead of materializing
keys()/values() lists for every card.
- Rarity queries iterate printings in place instead of copying them into
a QList<PrintingInfo> first.
- Power/toughness split the string with indexOf and compare QStringViews
instead of allocating a QStringList.
- Field queries do one hash lookup instead of contains() followed by
value().
- Color queries bind the colors string by reference instead of copying.
Verified with filter_string_test (42 tests) and a full client build.
* [Search] Address review comments
- Use std::any_of/std::none_of over the set map's key/value iterators
instead of hand-rolled loops.
- Extract the name matching into matchingLowercaseName() returning
std::optional<QString>.
- Reuse the matched name's length for the distance tie-break instead of
looking the name up again, removing one getLocalizedName() call per card.
- Take the color strings by value; the getters already return by value.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Security] Add challenge-response auth with scrypt verifiers and stop storing plaintext passwords
Challenge-response authentication: the client derives a scrypt verifier
(RFC 7914, EVP_PBE_scrypt, N=32768, r=8, p=1) and authenticates with
HMAC-SHA256(key, nonce), so neither the password nor its hash is
transmitted. The stored format becomes
"$scrypt$<n>$<r>$<p>$<salt>$<verifier>" and Response_PasswordSalt
now carries the cost parameters. Strict servers only accept scrypt
verifiers; legacy accounts are migrated after a successful login.
Fix#344 for challenge-response servers: a saved profile stores the
derived verifier under the password key instead of the plaintext password.
The connect dialog loads it without revealing it, autoconnect passes it
through, the change-password dialog no longer prefills the old password
field with it, and the client only persists the verifier when
"Save password" is checked.
Took 3 minutes
Took 1 minute
Took 10 seconds
Took 7 minutes
* [Security] Harden credential format acceptance and challenge nonce validation
* [Security] Refuse KDF downgrade; trim profile name and warn on silent credential drop
* [Security] Add SupportsAll ServerOptions enumerator to fix enum validation assert
The server advertised both SupportsPasswordHash (1) and
SupportsChallengeResponseAuth (2) via a bitwise-OR that produced the
value 3, which no enumerator declared. Protobuf's proto2 enum
validation rejects the value in _internal_set_server_options, crashing
servatrice on the debug assertion. Add a SupportsAll = 3 enumerator with
an identical wire value so the bitmask reads unchanged while the value
is now a documented, valid member of the enum.
* [Security] Normalize username when issuing challenge nonce
cmdRequestPasswordSalt bound the challenge nonce to the raw, un-simplified
username while cmdLogin verifies it against the simplified name. A user
who typed leading or trailing whitespace in the username box would be
silently rejected during challenge-response login even with the correct
password. Simplify at the nonce-issuing site so both commands agree on
the normalized name.
Also drive-by formatting: collapse the now single-expression
serverOptions initializer onto one line.
* [Security] Keep an encrypted legacy password backup for old-client logins
Migrating an account to scrypt stranded pre-challenge clients, which can
only present the legacy salt+hash: after migration the server served them
a scrypt salt they cannot use, so the only recovery was a password reset.
When security/legacy_backup_key is configured (64 hex chars,
openssl rand -hex 32), submitPasswordVerifier now seals the legacy
credential with AES-256-GCM under that key before overwriting the row,
storing it in password_legacy_backup with the time it was last used. Old
clients keep working: cmdRequestPasswordSalt serves the backup's 16-char
salt in password_salt so they can derive the legacy hash they were
always able to compute, while new clients derive the verifier from the
new scrypt_salt field, which always carries the verifier salt. The
non-challenge login path accepts the plaintext or legacy hash against
the decrypted backup and refreshes its window; a migrated row with no
usable backup fails closed.
Backups age out so a database dump taken after the window closes never
carries a crackable legacy hash: getUsableLegacyBackup purges on first
use past security/legacy_backup_ttl_days (default 90, clamped 1..3650),
and a six-hourly sweep drops backups idle accounts never touch.
changeUserPassword clears the backup since a new credential invalidates
it. Without a configured key nothing is sealed or purged, so a
temporarily missing key cannot destroy existing backups.
The AAD is the simplified user name on both sides, sealing the blob to
its row so it cannot be moved between accounts. Sealing failures
degrade to the previous lockout behaviour rather than blocking
migration. The columns extend the unmerged 0036->0037 migration, so the
schema version stays 37.
* [Security] Address review: proto compat, plaintext verify, AAD case, strict scope
Fix the two blockers and follow-ups from the 2026-10-16 review:
- Advertise challenge-response in a new supports_challenge_response
field instead of the ServerOptions enum and drop SupportsAll:
proto2 enums are closed, so already-released clients parsed
server_options = 3 as NoOptions and fell back to plaintext
passwords. server_options stays exactly SupportsPasswordHash
on the wire; old clients ignore the new field.
- Verify a plaintext password directly against a migrated scrypt
row with verifyPassword() so email changes (cmdAccountEdit)
and old clients sending password keep working without a
legacy backup key. Pre-hashed values are still checked
against the backup, and its TTL window is refreshed only
when the backup is what actually matched.
- Use the stored row name as the AES-GCM AAD on both seal
and open so a login whose case differs from the stored
name (utf8mb4_unicode_ci) still opens its backup.
- Require challenge-response in strict mode only for
registered SQL accounts (userExists gate), leaving
guest/unregistered logins and none/password auth
methods unaffected.
- Document in servatrice.ini.example that a captured
legacy hash can take over a not-yet-migrated account
and that strict only constrains registered SQL
accounts. Schema version is 38.
* [Security] Raise legacy backup TTL default to 180 days
Gives operators and slow-to-upgrade users a longer downgrade
window for pre-challenge clients before unused encrypted
legacy password backups age out.
* [Security] Fix servatrice.cpp formatting per clang-format-18
* [Security] Add missing forward declaration for Command_SubmitPasswordVerifier
The cmdSubmitPasswordVerifier declaration references the proto-generated
type, which otherwise does not name a type in this header and breaks
every C++ build.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
Co-authored-by: BruebachL <bruebachl@cockatrice.example>
* [Installer][Client] Back up user data before old uninstallers can delete it
* [Installer][Client] Fix stale backup resurrection, batch session backups, extend coverage
NSIS: clear stale %TEMP% backup and only arm restore when something was copied, directory-existence checks, back up loose .dec decks, drop downloaded pics but keep pics\CUSTOM.
Helper: single session dir per run (ms timestamps), same-basename disambiguation, partial copies still report their location.
* [Installer][Client] Fix review findings: throttle backups, NSIS old dir, portable messaging
* [Installer][Client] Fix review findings: backup markers, NSIS relocate, size cap
* [Installer][Client] Fix review findings: sounds, session coverage, NSIS wording
Client: back up data/sounds in portable mode and mention it in the
warning; reuse a recent session only when complete and covering the
current sources (source manifest), estimate size only for fresh copies,
normalize dedup via isInsideDir.
Helper: source manifest per session with sessionCoversSources(), clear
out-params on all failure paths.
NSIS: document defensive portable branch, merge backup over surviving
data/ to repair partial deletes, copy (not move) wording in relocate,
note .txt exclusion in deck notice.
* [Installer][Client] Fix review findings: stale backup, empty reuse, relocate, portable warning
NSIS: wipe TEMP backup once per run to avoid resurrecting deleted files,
check data dir existence in relocate so dir-only trees are moved.
Client: never reuse a recent session for an empty source set, only fall
back to a covering session on copy failure, warn fresh portable installs
instead of staying silent.
* [Installer][Client] Fix review findings: usable session reuse, loose decks, NSIS pics, symlinks
* [Installer][Client] Fix portable backup exclusions in offender merge
* [Client] Split at-risk data warning into focused helpers
Ricky's nit on the 200+ line warnAboutDataPathsInProgramDir: extract
collection (portable/installed), backup, message building and dialog
into named helpers. No behavior change, all tr() strings untouched.
* [Client] Read path settings inside isPortableBackupExcluded
* [Client] Make stateless at-risk helpers static
* [Client] Extract at-risk data warning out of MainWindow
* [Installer][Client] Fix review findings: subdir backup, portable warning, backup API
NSIS:
- Back up unknown top-level install subdirs (e.g. <installdir>\decks),
not just loose deck files, so the 3.1.1 -> new upgrade no longer loses
data folders the old RMDir /r deletes before the client ever runs.
Payload dirs (plugins/translations/sounds/themes) stay in place to avoid
restoring stale DLLs; restore + relocate handle the new backup set and
the one-time notice covers both decks and folders.
- Restart the %TEMP% backup when the 32/64-bit passes point at different
dirs instead of merging (which restored one dir's files into the other);
document that only the last pass relocates and the %TEMP% copy is not
a durable second copy.
Client:
- Drop the portable at-risk warning: portable installs never shipped an
uninstaller and portable updates preserve data/, so every portable user
got a spurious dialog plus daily host-machine copies.
- AtRiskDataWarning is now a plain class with Q_DECLARE_TR_FUNCTIONS
instead of QObject+Q_OBJECT; logging category renamed to
at_risk_data_warning; MainWindow wrapper inlined into startupConfigCheck.
- Lower the synchronous startup backup cap 500 -> 200 MB.
Utility/tests:
- Remove test-only backupAtRiskPath()/findRecentBackupSession() and the
no-marker/no-manifest legacy paths; isBackupSessionComplete() now
requires the complete marker; tests updated to the plural API.
* [Client] Restore collectInstalledAtRiskPaths per review
a23e02b5 removed the definition along with the portable helpers,
breaking the build (undefined reference at link time, and
isInsideAnyDir unused under -Werror on Arch/Debian 13). Restores
the 202c0d65 version (pathsInsideDir plus loose top-level deck
files) as suggested.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
The executables precompile Qt since #7235, but the libcockatrice_*
static libraries still reparse the full Qt Core stack in every TU.
Measured over the tree after the IWYU pass: 380 MB of header parse in
the libraries, 92% of it reachable from qtcore_pch.h.
- Apply qtcore_pch.h to the 14 Qt Core static library targets
(skips libcockatrice_rng: its SFMT.c is C and cannot use a C++
precompiled header).
- Grow qtcore_pch.h with 25 QtCore headers that IWYU shows are
re-parsed across many TUs and were not already reachable from the
existing list (JSON/QUuid cluster, QSettings, item models,
threading, XmlStream, QTemporaryDir, ...).
- Grow qtwidgets_pch.h with 30 QtGui/QtWidgets headers (dialog
cluster, item views, QImageReader, ...).
Only stable Qt headers go into the PCH, so it rarely invalidates.
Test executables stay PCH-less: 1-3 TUs per target does not amortize
the extra precompile.
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Client] Improve card database update visibility
Addresses Cockatrice/Cockatrice#7431.
Cockatrice previously offered no in-client feedback while the card
database updater ran: the only progress UI lived in the first-run
wizard, stderr/stdout were not parsed in interactive runs, and starting
a second update with Oracle already open or with a hidden background run
active just showed a confusing 'already running' guard - even though
Oracle is launched as a separate process with no lifecycle tracking.
- Add a status-bar widget (CardDatabaseUpdateStatusBar) that appears
while an update runs and shows the Downloading/Parsing/Importing
stages with percent progress; it is also used in the quit-confirm
prompt so users see an unfinished update before cancelling it.
- Parse the updater's PROGRESS lines (new CardUpdateProgressParser,
unit-tested) in both foreground and background runs and log stage
changes to the new card_database.update category, throttled to at
most one line per second; forward Oracle stderr to the logs.
- Show a tray balloon when a background update starts and on finish or
failure instead of staying silent.
- Request a running Oracle instance to raise itself (QLocalServer
CockatriceOracleRaise-<user>) when the user starts another update, and
demote the 'already running' guard to an informative message only when
no running instance can be found.
- Emit download/scan/import progress unconditionally from Oracle so
interactive and background runs behave the same.
- Document the new log category in the troubleshooting guide.
* [Client] Address card database update review feedback
* [Client] Add setting to control card database update tray notifications
* [Client] Address remaining card database update review nits
- Rename card_update_progress_parser.{h,cpp} to card_update_progress.{h,cpp}
- Store the CardUpdateProgress struct in the status bar widget
* Lint.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [DeckList] Apply include-what-you-use to libcockatrice_deck_list
* [IWYU] Apply include-what-you-use across the tree
Run one whole-tree IWYU pass over the libcockatrice_* libraries, the
Cockatrice client, Oracle, Servatrice and the test suite, so each
translation unit includes what it uses directly and forward-declares
what it does not need.
IWYU only analyses real translation units, never the moc code CMake
generates from Q_OBJECT headers, so it strips includes that moc output
silently relies on. Where a type is passed by value (or nested in a
QList/QMap) in a signal or slot signature, the generated moc code needs
the complete type; those includes are kept with an explanatory
`// IWYU pragma: keep`. The same holds for Q_DECLARE_METATYPE targets
such as QSqlDatabase.
A handful of includes are added by hand where callers brace-initialize a
const-reference parameter, which also requires the complete type.
Applied as a single commit on purpose: stripping includes from the
libcockatrice_* headers is precisely what breaks the client, test and
server translation units, and the fixes for those live in the same
translation units. Splitting by component would therefore leave
intermediate commits that do not compile.
All 426 modified Q_OBJECT/Q_GADGET headers were verified to be
moc-self-contained by compiling each generated moc_*.cpp standalone;
several built only because of moc include ordering in
mocs_compilation.cpp before the keep-includes were added.
* [Tooling] Emit public Qt headers and fix IWYU fallout CI found
Three problems the local build could not see, all because a green local
build only proves the configuration it was run under.
Private Qt headers. IWYU emitted private, lowercase Qt headers, and
qtypes.h does not exist at all before Qt 6.9, so the Debian 12 and
Ubuntu 24.04 jobs on Qt 6.4.2 failed to compile. The generator only
emitted a mapping when the capital header name mirrored the private one
(QAbstractAnimation for qabstractanimation.h), so qtypes.h, which is
wrapped by QtTypes instead, got no mapping and kept the private
spelling. Map it explicitly to <QtTypes>, which is public since Qt 5 and
provides the same typedefs either way, and rewrite the 100 affected
includes.
Do not generalize this to "prefer any Qt* wrapper". The table is
generated from whichever Qt is installed, and the wrappers are not
equally old: QtLogging only appeared in Qt 6.8 and QtMinMax in 6.5, so
that heuristic trades one breakage for another. Every other lowercase
header still in the tree has shipped publicly since Qt 5, so those are
left alone.
Platform-guarded includes. IWYU only sees the preprocessor configuration
it is handed, so it dropped an include that only the macOS branch of
main.cpp needs, and the macOS jobs failed on an undeclared
UrlSchemeEventFilter. Restore the include. Force-compiling every file
that carries a platform guard under Q_OS_MAC, Q_OS_MACOS and __APPLE__
reports nothing else, so the macOS branches are covered rather than
assumed.
shellcheck. Drop a leftover variable and stop assigning the
include-what-you-use lookup, which was only ever an existence check.
* [Tooling] Map Qt 6.5+ split headers to QtGlobal for Qt 6.4 compatibility
IWYU emitted QtTypes, QtPreprocessorSupport, QtVersionChecks, QtAssert,
QtMinMax, QtClassHelperMacros, QtEnvironmentVariables and QtTranslation.
Qt 6.5 split those declarations out of qglobal.h into separate headers, so
neither the Qt* wrapper nor the q*.h private name exists on the oldest Qt we
support (6.4.2), and Debian 12 / Ubuntu 24.04 fail with "No such file or
directory". qtypes.h is worse still: it only appeared in Qt 6.9.
QtGlobal is the one spelling that works across the whole supported range. It
has been public since Qt 5, defines these declarations itself up to Qt 6.4, and
includes the split headers from Qt 6.5 on, so redirect all eight private
headers to it and collapse the affected includes in the tree.
Curated overrides now take precedence over the name-mirroring rule, which
cannot know about Qt version floors because the mapping is generated from
whatever Qt is installed. That rule was mapping qtversionchecks.h to
<QtVersionChecks> even with an override present, because the override was only
consulted as a fallback. A curated key that matches nothing is now a hard
error rather than a silent fallback, and the redirected headers are no longer
advertised as the home of a symbol.
Verified every Qt include in the tree resolves against the real
qt6-base-dev 6.4.2 package: 15 distinct includes, 0 missing.
* [Tooling] Map QOverload to QtGlobal and drop the QMetaMethodArgument include
Two more headers IWYU emitted do not exist on Qt 6.4.2:
QOverload was split out of qglobal.h in Qt 6.5, so on 6.4.2 it is declared in
qglobal.h directly and the standalone header is a hard error. QtGlobal covers
both, so map qoverload.h to it like the other 6.5+ split headers.
QMetaMethodArgument is never referenced anywhere in the tree. It is a bare
forwarder for qobjectdefs.h that our generator advertised as a class symbol,
which is how IWYU came to add the include at all. The symbol mappings are
generated from header names, so they cannot tell a forwarder that happens to be
capitalised from a real class; removing the unused include is the fix, and the
generic case stays a known limitation of the generator.
Extending the audit to every Q* header (an earlier pass only matched headers
starting with "Qt" or lowercase q*.h, so it missed all of them) against the real
qt6-base-dev, qt6-multimedia-dev, qt6-declarative-dev, qt6-svg-dev and
qt6-websockets-dev 6.4.2 packages: 341 distinct Qt includes, 0 missing.
* [Game] Give moc the complete CardItem type in player_actions.h
player_actions.h forward declares CardItem but passes QList<CardItem *> by
value in actPlay/actPlayFacedown/actHide. moc instantiates the metatype for
those parameters, and on Qt 6.4 that expands to sizeof(CardItem), so Ubuntu
24.04 fails with "invalid application of 'sizeof' to incomplete type
'CardItem'". Newer Qt tolerates the incomplete type, which is why only the
6.4.2 jobs failed.
Include the defining header with an IWYU pragma keep, matching how the other
MOC-required includes in this area are handled.
* [Game] Give moc the complete ServerInfo_Card type
player_logic.h, game_scene.h, view_zone.h and view_zone_widget.h forward
declare ServerInfo_Card but pass QList<const ServerInfo_Card *> through signal
and slot signatures. moc instantiates the metatype for those parameters, which
Qt 6.4 expands to sizeof(ServerInfo_Card), so the connect in game_scene.cpp
fails on Debian 12 and Ubuntu 24.04 with "invalid application of 'sizeof' to
incomplete type 'const ServerInfo_Card'". Newer Qt tolerates it, so only the
6.4.2 jobs failed.
Include the generated protobuf header with an IWYU pragma keep, following the
pattern already used for ServerInfo_Player and ServerInfo_User in
game_event_handler.h.
* [Client] Complete the types Qt 6.4 needs for connect and moc
Qt 6.4 expands QMetaType::fromType on the normalised argument list of a
PMF connect and on moc-generated metatypes, which requires sizeof on every
signal and slot parameter. Newer Qt tolerates forward declarations there,
so only the Debian 12 and Ubuntu 24.04 jobs failed, with
"invalid application of 'sizeof' to incomplete type" for QMouseEvent,
QPrinter and GameReplay.
Replace the forward declarations with the real includes:
- QMouseEvent in the nine widget headers that pass it through signals
- QPrinter in abstract_tab_deck_editor.cpp, for the actPrintDeck lambda
- game_replay.pb.h in tab_replays.h. GameReplay is a protobuf message, so
the project's game/replay.h only forward declares it and cannot supply
the complete type.
Found by recompiling all 1918 project TUs against the Debian Qt 6.4.2
headers, which reproduces those two CI jobs locally in one pass.
* [Client] Complete Release and QMenu for Qt 6.4 moc and connect
Continuing the Qt 6.4 complete-type sweep:
- window_main.cpp connects ClientUpdateChecker::finishedCheck to
MainWindow::checkClientUpdatesFinished, whose Release * parameter made
moc 6.4 instantiate the metatype. GameReplay turned out to be a protobuf
message, so Release likewise needed release_channel.h in the three
headers declaring those signals: window_main.h, dlg_update.h and
client_update_checker.h.
- moc 6.4 also needs sizeof(QMenu) for the QList<QMenu *> carried by
TabSupervisor::setMenu and MainWindow::updateTabMenu.
Verified by regenerating moc output with the Debian moc 6.4.2 for all 434
Q_OBJECT/Q_GADGET classes and compiling it, plus recompiling all 1612
source TUs, against the Qt 6.4.2 headers: no remaining failures.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Translations] Load Qt's own translations and build ours with the binary
Three separate bugs kept translations from working.
installNewTranslator() loaded the application translation into
qtTranslator. QTranslator discards its previous contents on every load, so
that wiped the qt_<lang>.qm loaded one line earlier and installed an empty
translator instead. Qt's own strings were never translated on any platform.
Give each file its own translator.
QLibraryInfo::path(TranslationsPath) was the only directory searched for
Qt's translations. A relocated Qt can report a path that does not exist: the
KDE Flatpak runtime reports /usr/translations while its files live in
/usr/share/qt6/translations. Probe the usual install layouts and only hand
back the ones that exist.
The .qm files were attached to the default build target only, so building
the executable on its own left it without any translations and the language
selector empty. Attach them to the target as well.
Move the shared lookup into TranslationLoader so client and Oracle cannot
drift again, and have it report a missing translation at debug level. An
unset language is the default for English, which has no file to begin with,
and warning about it only added noise to every fresh debug log.
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Tests] Define main() in translation_loader_test
Without a GTest installation the fallback in tests/CMakeLists.txt links
only gtest, not gtest_main, so the test has to bring its own main(). Every
other test already does; only this one relied on the distribution package
and failed to link on CI.
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Tests] Put the translation loader fixture next to the test executable
A multi-config generator (MSBuild, Xcode) writes the executable to a
per-config subdirectory while the .qm stays in the current binary
directory, so applicationDirPath() never saw the fixture and
ABuildTreeFindsTheTranslationsNextToTheExecutable failed on Windows.
* [Translations] Depend on Qt's lrelease target and skip the loader test without lrelease
Attaching the .qm custom commands to a second custom target races with
Qt's own <Project>_lrelease target on MSBuild/Makefiles and is rejected
by Xcode, so depend on Qt's target instead. Only build the translation
loader test when Qt6::lrelease is available.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
Co-authored-by: ZeldaZach <zahalpern+github@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* [Server/Client] Unify report categories into one shared source of truth
* Lont.
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Security] Use a CSPRNG for salts, tokens, and RNG seeding
Password salts and activation tokens were generated with the global SFMT
RNG, which was seeded from a 32-bit timestamp, making registration
salts and activation tokens predictable. The game RNG used the same
timestamp seed across restarts.
Add CryptoUtil backed by OpenSSL RAND_bytes and use it for salt/token
generation and to seed RNG_SFMT with a 64-bit CSPRNG value in both the
client and server. Link libcockatrice_utility against OpenSSL::Crypto.
Took 30 seconds
Took 25 minutes
* Lint.
Took 4 minutes
Took 36 seconds
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Server/Client/Protocol] Reporting users + moderation queue functionality
Took 6 minutes
Took 3 minutes
Took 8 seconds
Took 11 minutes
Took 12 minutes
Took 7 minutes
Took 15 seconds
Took 2 minutes
Took 1 minute
Took 30 seconds
Took 16 seconds
* CI Fix
Took 6 minutes
* CI Fix
Took 6 minutes
* [Protocol] Add moderation investigation commands
Adds the protocol layer for the moderation investigation suite:
- Command_GetUserSessions/GetUserAlts/GetModeratorLastLogins/ResetUserPassword/RemoveUserAvatar (1013-1017)
- Response extensions 1215-1219 with ServerInfo messages for sessions, alts, and staff logins
- last_login on Response_ReportUserInfo and warning_il on Response_WarnList
Took 2 minutes
* [Utility] Add warning categories parser with infraction levels
Parses the server's 'officialwarnings' setting (comma-separated, optional
'|IL' suffix) into WarningCategory structs so the client can display the
infraction level of each warning category. Includes GTest coverage.
* [Server] Add moderation investigation tools
Implements the server side of the moderation suite:
- getUserSessions/getUserAlts/getModeratorLastLogins/removeUserAvatar DB methods
- Handlers for all five new commands with audit records (PASSWORD_RESET,
REMOVE_USER_AVATAR); password resets return a generated temporary password
- cmdGetWarnList now reports per-category infraction levels from the
officialwarnings setting; cmdReportUserInfo reports last_login
- Update servatrice.ini.example with the warning taxonomy
- Password/avatar mutations report RespNameNotFound when the user does not exist
* [Client] Add moderation tab with investigate, password reset, and avatar removal
- New Moderation tab: search a user to show account info, alternate
accounts, login sessions, and staff last logins; actions to reset the
user's password (shows the generated temporary password) and remove the
user's avatar
- 'Investigate user' entry in the user context menu opens the tab pre-loaded
for that user
- Warning dialog shows the infraction level of each warning category
- Tab wired into TabSupervisor with a moderator-gated menu action, shortcut,
and tabs.ini persistence (default closed)
* [Server/Client/Protocol] Address PR #7091 review: security, bug, and perf fixes
Security:
- Promote RESET_USER_PASSWORD to admin-only dispatch (was moderator-accessible)
- Reject password reset on users with equal/higher privilege than caller
- Notify affected user via Event_NotifyUser::CUSTOM when password is reset
- Add server-side category whitelist for reports
- Drop reporter name fallback in comment/details authorization (ID-only)
- Force password change: new DB column + login enforcement + client disconnect
Bugs:
- XSS via QTextEdit::append() → insertPlainText() in report tab and utils
- allNotified initialized to true even with empty recipients list
- Warning combo box: use currentData() instead of baked-in display text
- Report resolution now records who resolved (resolved_by column + audit)
Performance:
- IP-correlation subquery: add 6-month window + LIMIT 200
- getUserSessions: clamp limit to 500
Non-blocking:
- Palette-aware colors in report_utils.cpp (dark/light mode)
- Report list pagination: offset/limit fields + total_count in response
- SessionCommand enum gap comment for reserved values 1201-1203
Schema: 36→37 (force_password_change), 37→38 (resolved_by)
Took 12 minutes
Took 16 seconds
* Fix macOs pedantry
Took 5 minutes
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [Game] Playmats
Took 19 seconds
Took 1 minute
* [Playmats] Add fixed override and configurable fallbacks to settings.
Took 29 minutes
Took 43 seconds
* Add main to test.
Took 1 minute
Took 29 seconds
* Move settings to own group
Took 11 minutes
* Some attempts to refresh macOS compositor
Took 2 minutes
* Try something else
Took 17 minutes
* Don't manipulate live list
Took 11 minutes
* Change things about resolution, address comments.
Took 45 minutes
Took 12 minutes
* Comments.
Took 14 minutes
Took 8 seconds
* Re-order settings menu location
Took 2 minutes
* Rename PlaymatResolution to Info and add enums
Took 8 minutes
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* Unify counter clamp arithmetic into shared addClamped() helper
- Add addClamped() in new header clamped_arithmetic.h; uses a 64-bit
intermediate so the addition cannot overflow int.
- Use it in Server_Card::incrementCounter() (clamps [0, MAX_COUNTERS_ON_CARD])
and Server_Counter::incrementCount() (clamps [INT_MIN, INT_MAX]), removing
the duplicated overflow-safe logic and its keep-in-sync TODO.
- Inline incrementCount() into server_counter.h; server_counter.cpp now holds
only the constructor and getInfo().
- Clarify the card-counter bounds comment in trice_limits.h.
* Rename MAX_COUNTERS_ON_CARD to MAX_COUNTER_VALUE
The constant caps the counter's value, not how many counters can be on the card
* Add direct unit tests for addClamped() helper
* Harden offsetCardCounter() against signed-int overflow
Replace the raw oldValue + offset sum with addClamped(), clamping to [0, MAX_COUNTER_VALUE] without overflow.
* Comment update
* Remove class names from addClamped() docstring
* Add subtype breakdown counter for card selection
Display a categorized count of creature subtypes (and other card type
subtypes) when multiple cards are selected. The breakdown appears above
the total selection counter in the bottom-right corner.
Subtypes are grouped by main card type and sorted by frequency, with
the most common subtypes positioned adjacent to the total count for
quick reference. The feature can be toggled via a new checkbox in
Settings > User Interface.
* Alignment fix
* Computation logic moved to helper funtction in separate file
* Rename SubtypeCounter to SubtypeTally
* Fix subtype tally alignment by using grid layout instead of character padding
* Rename count to tally in the subtype breakdown feature
* partial rename
* list position fixed
* Clean up code and documentation
* Rename subtypeCountLabelStyle to subtypeTallyLabelStyle and fix include ordering
* Fix include path for selection_subtype_tally.h after file relocation
* fixed count to tally rename inconsistencies
* [Room] Additionally show a tab for friends and ignored users instead of just all online users.
Took 21 minutes
Took 12 minutes
* [Room][UserList] Introduce style delegate for user list
- Allow users to set a card name and parameters as their background banner
- Allow mods to white/blacklist cards
- Allow toggling back to the old display style
Took 7 minutes
Took 28 seconds
Took 2 minutes
Took 2 minutes
* Right checkstate.
Took 14 minutes
Took 2 minutes
* Utility for test.
Took 9 minutes
Took 8 seconds
Took 2 seconds
* Lint.
Took 10 minutes
* Algorithm for sql schema migration
Took 13 minutes
* Use {prefix}, bound card name, return errors.
Took 27 seconds
* Convert queue to while loop.
Took 19 seconds
* Hover popup.
Took 36 minutes
Took 1 minute
* More granular signals, popup for user info.
Took 25 minutes
Took 8 seconds
Took 16 minutes
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* Refactor server counter API to own overflow protection and filter no-op events
Counter modifications now clamp to int bounds server-side and return change
status, allowing command handlers to skip network broadcasts when values
don't actually change.
* Centralize MAX_COUNTERS_ON_CARD and enforce [0, 999] bounds on server
- Move MAX_COUNTERS_ON_CARD to trice_limits.h
- Server clamps values in setCounter() and incrementCounter()
- Client uses clamped comparison to allow recovery from invalid states
- Add tests for clamping behavior
* move incrementCount() implementation from header to cpp
* [Counters] Split counters into graphics and logic states
Took 22 minutes
* Don't have widget hold pointer to state -> Copy what we need and subscribe to changes.
Took 12 minutes
Took 5 seconds
* Sync value too.
Took 3 minutes
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* style: Add braces to all control flow statements
Standardize code style by adding explicit braces to all single-statement
control flow blocks (if, else, for, while) across the entire codebase.
Also documents the InsertBraces clang-format option (requires v15+) for
future automated enforcement.
* InsertBraces-check-enabled
* Add ZoneNames constants for protocol zone identifiers. Introduce a centralized ZoneNames namespace providing constexpr constants for zone identifiers used in the client-server protocol. This establishes a single source of truth for zone names like TABLE, GRAVE, EXILE, HAND, DECK, SIDEBOARD, and STACK. The protocol values remain unchanged (e.g., EXILE maps to rfg for backwards compatibility) while providing meaningful constant names.
* refactor(server): use ZoneNames constants in server game logic
Replace hardcoded zone name strings with ZoneNames:: constants in:
- server_player.cpp: zone setup, draw, shuffle, mulligan operations
- server_abstract_player.cpp: card movement and token destruction
- server_game.cpp: returning cards when players leave
No functional changes - purely mechanical string literal replacement.
* refactor(client): use ZoneNames constants in core player/zone logic
Update the foundational player and zone classes to use ZoneNames::
constants instead of string literals. Changes include:
- player.h/cpp: zone initialization and builtinZones set
- card_zone_logic.cpp: zone name translation for UI display
- table_zone.cpp: table zone operations
No functional changes - purely mechanical string literal replacement.
* refactor(client): use ZoneNames constants in player actions and events
Replace zone name strings with ZoneNames:: constants in the player
action and event handling code. player_actions.cpp contains the most
extensive changes (~90+ replacements) covering all card movement
commands.
No functional changes - purely mechanical string literal replacement.
* refactor(client): use ZoneNames constants in zone menu handlers
Update all zone-specific menu files to use ZoneNames:: constants
for QAction data values and zone targeting. This covers context menus
for cards, graveyard, hand, and exile (RFG) zones.
No functional changes - purely mechanical string literal replacement.
* refactor(client): use ZoneNames constants in game scene components
Update remaining game scene components to use ZoneNames:: constants:
- arrow_item.cpp: arrow drawing between cards
- game_scene.cpp: zone view positioning
- message_log_widget.cpp: removes duplicate local static constants
that were previously defining zone names redundantly
- phases_toolbar.cpp: phase actions (untap all)
Notable: message_log_widget.cpp previously had its own local constants
(TABLE_ZONE_NAME, GRAVE_ZONE_NAME, etc.) which are now removed in favor
of the centralized ZoneNames:: constants.
* formatting fix
* [DeckAnalytics] Enforce WUBRGC ordering for analytics.
Took 6 minutes
Took 7 seconds
* Include QSet
Took 51 seconds
* Move include out of namespace.
Took 6 minutes
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
* [VDE] Deck Analytics Widgets overhaul
Took 2 minutes
Took 3 minutes
Took 3 minutes
* Qt5 version guards.
Took 33 minutes
Took 3 seconds
* Include QtMath
Took 3 minutes
Took 8 seconds
* Use getCards()
Took 4 minutes
* Non pointer stuff
Took 52 seconds
* Add a newline to the tooltip
Took 2 minutes
Took 27 seconds
* Fix build failure on macOS 15
* Rename some things.
Took 17 minutes
Took 11 seconds
Took 18 seconds
* Address overloads, fix default configuration.
Took 1 hour 9 minutes
Took 8 seconds
* Fix mana curve default config.
Took 4 minutes
* Namespace to Qt libs
Took 5 minutes
* Selection overlay is transparent for mouse events.
Took 2 minutes
* Brace initialize.
Took 8 minutes
* Debian 11.
Took 5 minutes
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
Co-authored-by: RickyRister <ricky.rister.wang@gmail.com>
* Move logger and key signals from libcockatrice_utility to Cockatrice.
Took 9 minutes
* Only link Qt::Core instead of COCKATRICE_QT_MODULES to libraries, if possible.
Took 2 minutes
---------
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
