64 commits

Author	SHA1	Message	Date
Bryce!	0de92c06f8	[Spoilers] Preserve spoiler files through update and reload failures (#7707) ... * Preserve spoiler files and serialize update reloads * Surface spoiler removal reload failures and allow disabled-state retry * Address review feedback on spoiler update lifecycle Only remove the spoiler file when the user unchecks the setting; opening Settings no longer queues an unrequested reload for a leftover file. Remove the spoilersUpdatedSuccessfully signal, which had no receiver. * Fix spoiler settings state after reopen and while disabled Reopen Settings while an application-owned update is running: the new page now observes that updater, so the button reads Updating... and re-enables on completion instead of offering a second update that cancels the first. Observation does not take ownership: only updaters the page creates are retired with deleteLater, because the launch check is stack-allocated. When spoilers are off, the hint now describes what Reload Card Database actually does, stays readable, and wraps instead of clipping. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-11-04 18:24:19 +01:00
Bryce!	e558c0dffa	[Banner] Keep horizontal user banner crops inside the artwork (#7742)	2026-11-04 17:08:54 +01:00
BruebachL	d8700b2947	[Fonts] Add AppFont roles and FontConfig fonts.toml round-trip (#7534) ... Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-11-04 07:57:52 +01:00
Bryce!	94f7de42f2	Fix user card games for offline users and stale requests (#7738)	2026-11-04 05:29:51 +01:00
Bryce!	dc4aa19d23	Import Moxfield signature spells into the deck [skip ci] (#7751) ... Co-authored-by: Codex <codex@openai.com>	2026-11-03 10:40:28 +01:00
Jurriaan	bc09ec6afe	[Client] Make DeckStateManager::decrementCard remove tokens from the token zone if the token is not in the given zone (#7588) ... * Fixed decrementCard for tokens not using the token zone * Removed dead function actDecrementCard Updated documentation * Fixed no-op, added documentation * Removed stray blank line * Allow finding card nodes even if no database entry exists for the given card * Added DeckStateManager tests * Added tokens on mainboard test * Added if checks and sorted sources Added if checks * Library include fix * Renamed zone to targetZone * Revert "Allow finding card nodes even if no database entry exists for the given card" This reverts commit 88578cd0d9. * Combined loader_local_matching_test and deck_state_manager_test settings sources * Added settings sandbox, refactor * Format * Renamed return variable * Flattened if statement * Style fix	2026-11-03 10:37:17 +01:00
sentry[bot]	9f30892dff	[Client] Fix crash when local game commands are sent after shutdown (#7714) ... * [Client] Fix crash when local game commands are sent after shutdown * [Network] Fix formatting in local client debug log * [CI] Keep the vcpkg submodule pointer on the manifest baseline vcpkg.json asks for builtin-baseline 9eec002 (vcpkg 2026.10.16), but the submodule gitlink was still at 9e593bb. The runners check submodules out shallowly, so vcpkg-tool cannot read versions/baseline.json out of the baseline commit and every vcpkg leg (Windows and all macOS) fails in CMake configure with: failed to `git show` versions/baseline.json git phrases that as "path ... exists on disk, but not in <sha>" even when the commit is simply absent from the shallow clone, so it reads like a tooling or network failure rather than a mismatched pointer. Point the submodule back at 9eec002, and add a cheap job that fails when the gitlink and builtin-baseline disagree, before the five build legs pay for it. --------- Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com> Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-11-02 13:28:23 +01:00
BruebachL	ce543896f5	[Tests] Use QtTest for model contracts and signal spies (#7630) ... Qt6::Test was linked into every test but never used. Add a small RAII QAbstractItemModelTester helper and apply it to the deck list model, the card database model and display proxy, and the sets model. Add QSignalSpy coverage for the card-added and download-settings signals, and assert explicitly that a card row is a leaf with no child columns (the tester does not check that). The contract checker immediately found four model bugs, fixed here: - DeckListModel allowed children below non-zero columns. - CardDatabaseModel reported every card as a child of every card row. - CardDatabaseModel reported six columns for a valid (leaf) card index. - CardDatabaseDisplayModel claimed more rows were fetchable while rowCount already exposed them all, so fetchMore() inserted duplicates. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-11-02 07:27:43 +01:00
BruebachL	50a2ae9cc8	[Server/Client] Enforce and complete the forced password change at login (#7680) ... checkUserPassword ignored force_password_change on the challenge-response path, so a reset password stayed valid. Returning PasswordChangeRequired was not enough on its own: the session was dropped and the client told the user to ask the administrator for the temporary password they had just used. The pending session now keeps its identity without registering a user session or announcing a join, may only ping and change its password, and the client completes the change in-session before reconnecting. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-11-02 07:01:29 +01:00
BruebachL	825aba953a	[Servatrice] Fail loudly when startup initialization fails (#7602) ... * [Servatrice] Exit non-zero when server initialization fails initServer() reported every startup failure through a zero exit code: the database could not be opened, the server printed nothing of its own, and the process looked like a clean shutdown. Log the two refusal reasons at critical level and return 1 so supervisors and the Docker exit status can tell the difference. * [Servatrice] Keep console logging independent of the logfile logMessage() returned before the console echo unless a logfile was open and server/writelog was true, so --log-to-console produced nothing when server/logfile was empty or could not be opened. That is how a refused database connection could end in a silent exit. Print to the console before the logfile gates, drop the duplicate echo from flushBuffer(), and read writelog/logfilters once in main instead of calling the non-thread-safe global QSettings from every logging thread. * [Servatrice] Uninstall the log handler before tearing down globals The handler reaches into the global logger and settingsCache. Leaving it installed while main deletes those objects turns any Qt warning emitted during QCoreApplication/QFactoryLoader teardown into a use-after-free; with QT_DEBUG_PLUGINS=1 that warning exists, and Servatrice died with SIGSEGV in QSettings::value() on the way out. * [Servatrice] Buffer console logs on the logger thread Address review feedback on the startup failure change: - Append console lines to a buffer drained by the logger thread, so a stalled stdout reader cannot block game threads. - Report islServer->errorString() instead of the literal call text. - Let the constructor reuse reloadConfig() instead of parsing the filter list twice. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-31 19:02:49 +01:00
BruebachL	345b0e8ba9	[Refactor] Move game logic into libcockatrice_game (#7437) ... * [Refactor] Move game logic into libcockatrice_game * [Refactor] Include card_state.h in player_actions.h * [Game] Inject settings providers into libcockatrice_game, drop Widgets * [Game] Follow base rename to createCardRequested after rebase * [Game] Drop unused CardItem forward declaration * [Game] Fix includes after moving game logic into libcockatrice_game --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-31 18:44:15 +01:00
sentry[bot]	bbc7e462ff	[Installer] Graceful close and a way out of a stuck update (#7687) ... * [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>	2026-10-31 15:32:59 +01:00
sentry[bot]	0369aaffa9	[Installer] Name only the payload programs installed in the directory (#7684) ... The running-instance lookup named every process whose image path began with the install directory, subfolders included. Installing into a folder that holds another application in a subfolder therefore reported that application as holding the previous version, and nothing the guard offers could clear it: the close asks cockatrice.exe only, and the end answer refuses anything that is not a payload program. Both answers were no-ops, so the update could only abort and tell the user to close a program that was never touching the files. Name only the programs the payload installs, and only when the image lies in the install directory itself. A program below the directory that really does hold a payload file is still named by the Restart Manager lookup, which is the lookup for that question. The Windows probe check now covers a payload-named program in a subfolder and a program the payload does not install; both have to come back as nothing. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-30 13:16:50 +01:00
BruebachL	675e1f7447	[Translation] Name languages without reading the translation files (#7662) ... * [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>	2026-10-26 02:39:26 +01:00
BruebachL	518c7e6125	[Network] Add a configured QNetworkAccessManager factory (#7628) ... * [Network] Add a configured QNetworkAccessManager factory Every HTTP caller built its own QNetworkAccessManager, so the system proxy and a transfer timeout were easy to omit. Add NetworkManagerFactory::create() as the single place that applies the process-wide proxy configuration and a transfer timeout, and use it at all 14 construction sites. Per-site settings such as disk caches and manual redirect policies stay with their callers. * [Tests] Link libcockatrice_network into the tests that compile its headers release_channel.cpp and card_picture_loader_worker.cpp now include libcockatrice/network/network_manager.h, but the test targets that compile them only linked libcockatrice_settings, so the include directory never reached them and every platform failed with "file not found": loader_local_matching_test and picture_loader_benchmark_test. libcockatrice_network is an INTERFACE library, so linking it costs these targets nothing but hands over the include path. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-25 00:33:43 +02:00
BruebachL	bbc6003f0a	[CrashReporting] Name the files and processes behind a failed installer purge (#7590) ... * [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
BruebachL	6385f3fab3	[CrashReporting] Mock crash-report dialog and reporting preference (#7521) ... * [CrashReporting] Mock dialog, sentry skeleton and CI prerequisites Mock: DlgCrashReport preview dialog (no account needed, redacted log preview), CrashReportingMode Ask/Always/Never setting in DebugSettings, --simulate-crash flag and Help-menu preview entry. Send writes a local mock JSON only. Vendor sentry-native 0.17.3 via FetchContent behind WITH_CRASH_REPORTING (breakpad everywhere, backend none under sanitizers) plus a CrashReporter facade with early init and post-loop shutdown. DSN is a placeholder: empty CMake default with COCKATRICE_SENTRY_DSN env override, so SDK init is skipped and mock behavior is unchanged until a real DSN is configured. CI prerequisites riding here so every layer below builds green: libcurl dev plus CA bundles in the Linux images and CodeQL env, sentry Qt integration off (its own find_package cannot see the macOS thin Qt layout), and the StrFunc Using construction makensis requires. Tests: ctest green, format clean, code plus UI gates PASS. * [CrashReporting] Forward mode changes to SDK and fix Linux CI opt-in Connects PersonalSettings::crashReportingModeChanged to giveConsent()/revokeConsent() so switching from AlwaysSend to NeverSend (or Ask) mid-session reaches the SDK instead of leaving consent given until restart. Forwards WITH_CRASH_REPORTING into the docker container the way CCACHE_DIR is, so the Linux package leg actually builds with crash reporting ON instead of silently building OFF. * [CrashReporting] Give crash-report logging its own category per review * [CrashReporting] Drop unused DebugSettings constructor visibility per review * [CrashReporting] Document reporting modes on the enum per review * [CrashReporting] Match the dump scan to the writer's temp fallback Review nit: when the handler's SHGetKnownFolderPath lookup fails it writes to <temp>/../.., but the startup scan looked in QDir::tempPath(), so a dump produced through the fallback would never be found and its crash would never offer to be reported. Walk up the same two levels in the scan. The path is Windows-only (#ifdef Q_OS_WIN); the statements were compiled and run standalone against Qt 6 to confirm the API use. * [CrashReporting] Complete the dump-scan fallback path The previous commit walked the scan's fallback up two levels but dropped the CrashDumps/Cockatrice suffix that the writer appends unconditionally, so a dump written through the fallback was still invisible to the scan. Evaluate the writer's expression verbatim rather than mirroring it with QDir: cdUp() is existence-checked and silently leaves the path alone when the directory is missing, parent_path() is not, and QDir::absoluteFilePath() joins the shared suffix without risking a doubled separator when the walk lands on a root. Verified by compiling the block verbatim against Qt 6 with -Wextra -Werror and comparing it with the writer's join for both the normal and the fallback case. Also record that %LOCALAPPDATA% is the scan's stand-in for FOLDERID_LocalAppData and why the two normally agree. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-19 14:57:03 +02:00
Zach H	ea6af0048c	[Server] Parse the rightmost address from forwarded header lists (#7586) ... X-Forwarded-For carries a comma-separated list, and constructing a QHostAddress from the raw header value returns a null address for it, so a configured X-Forwarded-For or a multi-hop header never resolved and websocket clients stayed on the proxy's address. - Add parseForwardedAddress, returning the rightmost usable entry of a forwarded header list - Use it for the websocket handshake instead of parsing the raw value - Exempt loopback from per-address auth rate limits so a same-host proxy with a customized trusted_sources list cannot lock every user out - Add forwarded_address_test Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-18 16:53:55 -04:00
Zach H	46cdbac830	[Servatrice] Match trusted_sources by address, not substring (#7582) ... trusted_sources was checked with QString::contains. A dual-stack listener reports a local proxy as ::ffff:127.0.0.1, which never matches the default "127.0.0.1,::1". Behind nginx the forwarded client IP header was then ignored, every websocket user shared the proxy's address, and logins failed with "too many concurrent connections" once max_users_per_address was reached. Add isTrustedSource(), which parses each entry as a QHostAddress and compares exact addresses with IPv4-mapped addresses normalized, and use it at all four call sites. The ban command also read server/trusted_sources instead of the documented security/trusted_sources; it now uses the same getter. Entries that are not IP addresses are logged at startup since they no longer match anything. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>	2026-10-18 16:50:04 -04:00
BruebachL	ce51e7147d	[Servatrice] Log room channel messages in audit log (#7479) ... * [Servatrice] Log room channel messages and bump schema to 38 * Fixup: show room_channel audit rows and keep migration nullable The log history query returns room_channel rows, but the log tab dropped them. Render them in the room table with an extra Channel column. Also drop the NOT NULL on target_type from the 37->38 migration so migrated schemas match the nullable servatrice.sql definition (and strict-mode ALTERs cannot fail on legacy NULL rows). Update the channel tests to the flag-based privilege model: the admin fixture now carries IsModerator alongside IsAdmin, as the database layer assigns. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-16 22:46:39 +02:00
BruebachL	6c660d6b21	[Installer][Client] Back up user data before old uninstallers can delete it (#7503) ... * [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>	2026-10-16 22:05:42 +02:00
BruebachL	456d9d9e70	[Server] Inform client of login/registration rate limit timeout (#7468) ... * [Security] Add per-address rate limiting for auth endpoints Introduce a thread-safe RateLimiter that tracks attempts per key (IP address) within a sliding time window, and wire it into the authentication endpoints: - Login: failed login attempts from an address are counted; once the configured maximum is exceeded within the window, further logins from that address are rejected with RespTooManyRequests. A successful login clears the failed attempts for that address. - Registration: implement the previously stubbed tooManyRegistrationAttempts, limiting how many accounts can be created per address per window. - Forgot-password: throttle both the email-request and the email-challenge paths per address. New [security] settings with defaults: max_login_attempts_per_ip=5 / login_attempt_window_seconds=900 max_registrations_per_ip=2 / registration_window_seconds=3600 max_forgot_password_requests_per_ip=3 / forgot_password_window_seconds=3600 Adds unit tests for the RateLimiter (window limit, over-limit blocking, clearing, per-key independence). Took 3 minutes * [Server] Fix rate limiter: disable semantics, draining window, bounded keys, pre-work login gate - recordAttempt()/isBlocked() return false when maxAttempts <= 0 or windowSeconds <= 0, so the documented 'set to 0 to disable' actually disables instead of blocking every address permanently - Keys at their limit are rejected without recording, so repeated attempts at the attacker's pace cannot keep sliding the window and holding the lockout open; a block now clears once the recorded attempts age out of windowSeconds - Attempts map is bounded: an opportunistic prune (every 60s) erases keys whose attempts have fully aged out - Login is throttled before the work: isLoginRateLimited() (a new virtual on Server, backed by isBlocked()) gates loginUser(), so a locked-out address no longer burns a database round trip and password verification per attempt - recordAttemptAt()/isBlockedAt() time seams make the sliding-window behaviour deterministically testable; tests rewritten for the real semantics (no reliance on the maxAttempts=0 bug, no sleeps) * [Server] Inform client of login/registration rate limit timeout - Add retryAfterSeconds/retryAfterSecondsAt to RateLimiter to compute seconds until oldest attempt exits the sliding window - Expose loginRateLimitRemainingSeconds via Server/Servatrice - Include denied_end_time (absolute epoch) in RespTooManyRequests for login pre-auth and when transitioning to lockout after failed attempt - Include denied_end_time in registration rate limit response - Client: show retry time for login RespTooManyRequests - Client: simplify registration RespTooManyRequests message to show retry time when provided - Add test for retryAfterSecondsAt * [Client] Restore registration guidance and drop duplicated rate limit reason The registration RespTooManyRequests case now only shows the retry-after copy when the server actually sent a denied_end_time; the per-email account cap path (no end time) falls back to the previous guidance about already having an account and contacting the server operator. Drop the canned denied_reason_str from the new login rate limit branches so the client no longer prints the same sentence twice. * [Server] Exempt trusted_sources from auth rate limits Addresses in security/trusted_sources bypass per-address limits, matching the existing max_users_per_address bypass in initSession. Without this, Webatrice behind local nginx (all clients as 127.0.0.1) would lock out every web user after 5 mistyped passwords, and cap the whole server at 2 registrations / 3 forgot-password requests per window. Adds Servatrice::isRateLimitExempt helper; recordFailedLogin, isLoginRateLimited, recordRegistrationAttempt and recordForgotPasswordAttempt all check it. # Conflicts: # servatrice/src/servatrice.cpp * [Server] Key login rate limit by (address, username) A successful login cleared the whole per-IP failure history, so 4 bad guesses plus 1 self-login (or guest UnknownUser login) from the same address bypassed the limiter. Failures are now bucketed per (address, lowercase username) and a success clears only its own pair. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-16 05:47:43 +02:00
BruebachL	cce606764c	[Server] Resolve a zone dump request to an in-range index span (#7401) ... Command_DumpZone.number_cards is an unbounded sint32 supplied by the client, and cmdDumpZone used it as an index without ever putting it in range. for (int i = 0; (i < cards.size()) && (i < numberCards || numberCards == -1); ++i) { const auto &findId = cmd.is_reversed() ? cards.size() - numberCards + i : i; Server_Card *card = cards[findId]; A forward request happened to be safe, because the loop stopped at the zone's own size. A reversed one was not. number_cards is documented by the client as -1 for "the whole zone", and cards.size() - numberCards is therefore cardCount + 1, one past the last card. A request larger than the zone puts the offset at a negative index instead. Both read out of bounds, then dereference the result as a Server_Card, so either adjacent heap memory or a crash. Any player can reach it: the zone only has to be a public one, or the requester's own, and both number_cards and is_reversed come straight off the wire. A reversed dump of -1 is enough on any public zone with cards in it. Resolve the request to a half-open span up front. -1 and any positive count are clamped to what the zone actually holds, any other negative count resolves to nothing, and a reversed request takes the span from the end rather than computing an offset from a value that may not be in range. cards.at() then carries the bound as a second line of defence, and qsizetype keeps the loop and the list's own indexing type aligned. Also report card_count as the zone's real size. It was numberCards < cards.size() ? cards.size() : numberCards, which agrees with the zone size for every request that was in range and reported the client's own oversized number otherwise. The client uses card_count to draw placeholders when a zone lists no cards, so overstating it invents cards that were never there. Put the resolution in its own translation unit. server_abstract_player.cpp refers to the RNG global that only servatrice's main.cpp defines, so anything linking that object needs the whole server binary behind it, which is why no test could reach this code before. server_dump_zone_test covers each request shape and then asserts the span is contained in [0, cardCount) across the edges of the request domain, INT_MIN and INT_MAX included. Reverting the arithmetic makes three of them fail, the two out-of-bounds shapes and the containment sweep. The client's own copy of this convention at view_zone.cpp:93 already clamps the way this now does. Leaving that duplication for a follow-up, since the client reaching the same answer by a different route is not a defect. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-13 11:23:13 +02:00
BruebachL	4a010bd17f	[Report] Add report reopen action for closed reports (#7442) ... - New Command_ReportReopen moderator command (extension 1206) with server handler that reopens resolved/dismissed reports, resets resolution state and notified, and records the reopening note in the audit log - Reopen... button in the report queue action bar, gated to resolved and dismissed reports, with optional note prompt and tooltip - Success confirmations for Assign, Resolve/Dismiss and Reopen now survive the list refresh via a pending status message instead of being clobbered - Dispatch/auth coverage in server_moderator_role_test Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 15:28:45 +02:00
BruebachL	8822ade49d	[Tests] Register each test case with CTest instead of each test binary (#7399) ... * [Tests] Register each test case with CTest instead of each test binary Registering a test cost four edits across a dozen CMakeLists files: add_test, add_executable, a conditional add_dependencies on gtest, and target_link_libraries. The conditional branch was the trap -- it is only needed when GTest was fetched and add_subdirectory'd, and omitting it yields undefined gtest symbols at link time -- so it was easy to leave out and impossible to notice until the link failed. Fold all four steps into cockatrice_add_test() and migrate every test directory onto it, with a cockatrice_add_manual_test() wrapper for the three benchmarks that are built but deliberately kept out of ctest. The calls now read as a declaration of what a test is rather than a transcript of how to register one. While rewriting the registrations, replace add_test(NAME x COMMAND x) with gtest_discover_tests(). Naming the binary as a single CTest entry meant a CI failure reported only which binary failed, and ctest -R Suite.Case could not select an individual case. Discovery raises the suite from 35 opaque entries to 469 real ones: ctest -R ThemeMigrationTest.IsIdempotent now runs exactly that case, and ctest -L server / -LE bench work off labels applied at registration. Give every test a 60s TIMEOUT. CTest's implicit default is 1500s, so one hung socket wait would stall CI for 25 minutes, and single_instance_manager_test already does real QLocalSocket waits. Dropping per-case granularity costs wall-clock time, since the suite becomes process-spawn bound rather than CPU bound: serial ctest went from 3.8s to 30.8s. Run it with -j in CI to recover it -- 7.6s at -j4, and 3.5s at -j8, both at or below the old serial time. The binaries are parallel-safe: the only test claiming a fixed resource scopes its local socket name to its own pid, so concurrent copies cannot collide. Also delete dummy_test, which asserted 1 == 1 and only proved the harness was wired up -- something the other 468 cases already establish. No test is added, removed, or changed by this commit; the 469 cases are the same ones the 35 binaries contained. * [Tests] Make QtTest available to test targets QtTest was not among the modules test targets could link, so the Qt model and signal contracts had no way to be verified from a test: QAbstractItemModelTester and QSignalSpy both live there. Add Test to _TEST_NEEDED in cmake/FindQtRuntime.cmake. That is the single place the union of test-only Qt modules is declared, so this is the only file that needs to change to make the module reachable from every test target. Nothing uses it yet. This lands the capability ahead of the model-layer tests that need it, so those can be added as tests alone rather than as test plus build-system plumbing. * [Tests] Correct the note on the oracle test compression defines add_definitions() populates the directory's COMPILE_DEFINITIONS property, which is consumed at generate time, so it reaches every target in the directory no matter where it is called relative to add_executable(). The previous note claimed the flags stayed confined to the benchmark because the find_package block came after it; oracle_importer_test, created above that block, carries both defines too. Verified harmless rather than a live bug: HAS_ZLIB and HAS_LZMA gate code only in oracle/src/pages.cpp and oracle/src/pagetemplates.cpp, and no test target compiles either. The ordering and the leaked defines are both pre-existing, so this only corrects the explanation. * [Tests] Carry one CTest label per test instead of several gtest_discover_tests() can hand CTest exactly one label. GoogleTestAddTests.cmake accumulates its generated script with `foreach(arg ${ARGN})`, so a label list reaches the CTestTestfile as bare `LABELS ci server`, and CTest reads only the first value of a property and drops the rest without a word. The `ci` label that is appended to every registered test was therefore absent from every test that also asked for `server` or `bench`: `ctest -L ci` reported 400 of 469 cases, and `ctest --print-labels` never listed a second label anywhere in the build. Reduce each test to exactly one of `ci`, `server`, or `bench`, which is all the selections need. The three now partition the suite exactly, 400 + 64 + 5 = 469, and `ctest -LE bench` still means everything but benchmarks. A second label is now a configure-time FATAL_ERROR rather than a line that quietly means something else. Anything orthogonal to a test's kind belongs in a helper flag beside the label, so it composes with the group instead of competing for the single slot. Also raise DISCOVERY_TIMEOUT off GoogleTestAddTests' 5s default. A plain debug binary answers --gtest_list_tests in 0.04s, but that budget is thin once several binaries discover concurrently behind a -j build, and being over it fails at the *link* step with a bare "Process terminated due to timeout" that points nowhere near the cause. * [Tests] Link gtest_main on the fetched-gtest path The downloaded-gtest branch set GTEST_BOTH_LIBRARIES to `gtest` alone, while FindGTest sets it to both halves. Every test target links that list rather than picking an entry point, so on this path nothing provides main() and any test source that does not define its own fails at link with `undefined reference to main` pointing at crt1.o, naming neither gtest nor the line responsible. That stayed latent while every test carried its own four-line main(). A test that omits it now fails, and the error points at the C runtime rather than at gtest, which is a long way from the cause. Restoring both halves makes the two paths agree and drops the boilerplate requirement on new test sources. The 18 sources that still define main() keep linking: gtest_main.cc defines nothing but main, so the linker leaves its object in the archive unused rather than reporting a duplicate. * [Build] Run the test suite under ASan and UBSan (#7400) * [Build] Add an ENABLE_SANITIZERS option for ASan and UBSan Adds -DENABLE_SANITIZERS, off by default, building with AddressSanitizer and UndefinedBehaviorSanitizer. The bulk of the work is in the three ways the obvious implementation silently does the wrong thing. Probe the flags on the link step, not just the compile step. check_cxx_compiler_flag() only compiles, so every sanitizer flag comes back unsupported, because linking an ASan object without -fsanitize=address leaves __asan_init undefined. The result is not a build failure but a build that appears instrumented and is not, which is worse. Use check_cxx_source_compiles() with CMAKE_REQUIRED_LINK_OPTIONS instead, and treat an unsupported sanitizer pair as fatal since the cosmetic flags are allowed to degrade. Opt vendored sources out per source, through a cockatrice_disable_sanitizers_for() helper rather than a list of set_source_files_properties() calls at the root. The helper has to exist because CMake resolves a source-file property against the directory scope its target was declared in: setting the property from the root with absolute paths is a no-op, silently, which is why the call sites sit next to the add_library and add_executable that pull the vendored code in. SFMT, minizip, the LZMA SDK and Qxt SMTP are excluded. oracle/src/qt-json is in the history but not in the tree, removed in #7214. Also mark those sources SKIP_PRECOMPILE_HEADERS. The PCH is built with the sanitizer flags, so it defines __SANITIZE_ADDRESS__; a translation unit compiled with -fno-sanitize=all does not and GCC then refuses the PCH, which is a fatal -Werror=invalid-pch in the servatrice and oracle builds. -fno-sanitize=vptr is not optional. Qt constructs QObject subclasses into raw storage and writes the vptr afterwards, so the vptr check reports on essentially every QObject instantiation, and it has no value here because nothing outside moc-generated code inherits QObject. Verified on the resulting build: our own translation units carry ASan and UBSan calls, SFMT.o carries neither, and the full suite of 464 cases passes clean under ASAN_OPTIONS=detect_leaks=0:abort_on_error=1 with UBSAN_OPTIONS=halt_on_error=0. The test suite needs two adjustments to make this option usable at all, both of which land here because they are consequences of COCKATRICE_SANITIZERS existing rather than independent changes. DISCOVERY_TIMEOUT rises from 60s to 300s under sanitizers. An instrumented binary takes 2.8s to answer --gtest_list_tests against 0.04s uninstrumented, and discovery runs several binaries at once behind a -j build. One target is excluded via a NO_SANITIZE flag, server_developer_role_test. Reaching google::protobuf::TextFormat::Printer's destructor aborts on an absl assertion inside system protobuf 33.4 whenever the consumer translation unit is built without NDEBUG, which a Debug sanitizer build is. This reproduces with no Cockatrice code at all, just "TextFormat::Printer p; p.PrintToString(msg, &out);" built with -fsanitize=address, so it is upstream rather than ours. It is a flag and not a fourth label because "skip this in the sanitizer leg" has to compose with a test's group rather than compete for the single label slot, and because a flag excludes by not registering the cases at all, which cannot rot the way a label silently can. The target still builds and still runs on the legs without sanitizers. * [CI] Run the suite once under ASan and UBSan Adds --sanitize to .ci/compile.sh, mirroring --test, and turns it on for one entry of the existing Linux Docker matrix: Ubuntu 26.04, which already had a test-enabled leg. That reuses the image, the ccache layer and the existing step shape, so the CI diff is a matrix key plus the argument plumbing. Debian 13 and Fedora 44 keep covering the uninstrumented suite, so nothing is lost by instrumenting one leg. Sanitizers stay off the release and package legs, which build with --release for packaging: instrumenting those would change the shipped binaries' optimization profile and build time for no benefit. macOS and Windows stay uninstrumented too, since ASan is unreliable against the Qt build there and MSVC has no equivalent without /fsanitize=address. --sanitize does not imply --test, so an instrumented build can be produced locally without paying for ctest. When both are given, ctest runs with detect_leaks=0 because Qt and the sanitizer runtimes both leak at exit by design and LeakSanitizer's default buries real findings under hundreds of process-teardown reports, and with UBSan halt_on_error=0 so one run collects many findings instead of stopping at the first. ASan keeps abort_on_error=1, since a bad access leaves the process unusable and continuing only produces a second, more confusing report from the wreckage. * [Build] Fix sanitizer flag leak into release package and make UBSan fail CI - compile.sh always passes -DENABLE_SANITIZERS explicitly so the release-package reconfigure in the same build dir cannot inherit ON from the sanitized debug step (which shipped libasan/libubsan DEB deps). - Sanitizers.cmake resets COCKATRICE_SANITIZERS to OFF when the option is off (CACHE INTERNAL otherwise sticks forever, keeping NO_SANITIZE skips and the 300s discovery timeout). - UBSan now fails the job: -fno-sanitize-recover=undefined at compile time plus UBSAN_OPTIONS halt_on_error=1 (recover mode exits 0, so the old halt_on_error=0 could never fail). --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de> --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 14:50:09 +02:00
BruebachL	a44757bd11	[Client] Improve card database update visibility (#7458) ... * [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>	2026-10-09 11:07:48 +02:00
BruebachL	0934380a5c	[Translations] Load Qt's own translations and build ours with the binary (#7384) ... * [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>	2026-10-03 15:04:30 +02:00
BruebachL	014983b5bc	[Theme] Carry Default theme directory customizations over to System (#7385) ... * [Theme] Carry Default theme directory customizations over to System The Default -> System rename rewrote the stored theme name but left the user profile's themes/Default directory behind, so anyone who had customized zone graphics, stylesheets or palettes there silently lost them. Migrate the directory alongside the name. Rename it when System does not exist yet, otherwise merge file by file without overwriting what System already has, then drop the legacy directory. Users who already ran the rename get their customizations back on the next start, and a hand-made Default folder of a user on another theme is left untouched. * [Tests] Give theme_migration_test an explicit main and gate it on WITH_CLIENT The build machines that cannot find GTest fall back to the downloaded googletest, which links the bare gtest library without gtest_main. Every other test defines its own main() for that reason; this one did not, so Debian 13, Fedora 44 and Ubuntu 26.04 failed to link it. The test compiles client sources, so -DTEST=1 -DWITH_CLIENT=0 has no way to build it. Gate the subdirectory on WITH_CLIENT like the other client-source test. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-03 13:43:25 +02:00
BruebachL	add65caae5	[Client] Hand off single-instance payloads outside the readyRead signal (#7373) ... * [Client] Hand off single-instance payloads outside the readyRead signal Opening a cockatrice:// link from outside a running client showed the "Join game ...?" confirmation and then crashed on both answers. The join reached the server first, so the other players saw the user join and immediately disconnect. SingleInstanceManager::handleNewConnection() emitted filesReceived from inside the forwarding socket's readyRead lambda and only disconnected afterwards. A joingame link makes the handler show a modal QMessageBox, which runs a nested event loop. The sending instance exits as soon as it has the acknowledgment, so the socket was disconnected and deleteLater'd inside that loop: it died in the middle of its own readyRead signal, and both this lambda and Qt's signal dispatch kept using the freed memory. Disconnect right after the acknowledgment and emit filesReceived through a queued call, so no handler and no nested event loop ever runs inside the socket's signal. Guarding the disconnect call alone is not enough, since Qt still touches the socket after the lambda returns. Add a regression test that reproduces the nested-event-loop hand-off. * [Tests] Apply cmake-format to the single-instance test wiring * [Client] Defer the single-instance socket teardown past the readyRead frame The forwarded payload is emitted synchronously from inside the forwarding socket's readyRead, which is what lets the modal "Join game?" confirmation appear. A handler can spin a nested event loop, and the sending instance exits as soon as it has the ACK, so the socket is disconnected and deleteLater'd while that loop runs. Control then returns to this frame with the socket already destroyed, and the inline disconnectFromServer() touched freed memory. Queue the teardown instead and guard it with a QPointer taken before the handlers run -- building the guard afterwards is itself a use-after-free, since QPointer dereferences the pointer it is constructed from. The payload still reaches the handlers synchronously, so the confirmation is unchanged. The regression test covers both halves: the handlers must run while the socket is still connected, and the socket must not be torn down until the readyRead frame has returned. * Lint. * [Client] Only delete the single-instance socket outside the readyRead frame The auto-delete on `disconnected` was the remaining use-after-free. A handler may spin a nested event loop (the modal "Join game?" box), the sending instance exits as soon as it has the ACK, and so the peer closes while that loop runs. deleteLater() issued from inside such a loop is not deferred to the outer loop: Qt frees the object as soon as the nested loop unwinds, which is still inside this readyRead emission, so Qt's own signal dispatch and the handler lambda both keep using a freed socket. When the notification happens to arrive late the bug hides, which is why this only reproduced on some platforms. Drop the auto-delete and let the queued teardown own the socket's lifetime: it disconnects and then deletes, once the frame has unwound. The QPointer guard was necessary but not sufficient -- it stopped us dereferencing a dead pointer, not the socket being freed underneath the frame. The test waits on the socket's destroyed signal instead of polling, so the teardown assertion does not depend on platform event timing. * [Client] Do not queue the single-instance socket teardown The previous commit moved the deleteLater() off the disconnected signal and into a queued invocation, believing that "queued" meant "after the readyRead frame returns". It does not. A queued call is delivered by whichever event loop is running, and the modal "Join game?" box runs a nested one -- so the teardown was still executed inside the readyRead emission, which is exactly the use-after-free it claimed to prevent. Queueing was never necessary. Because nothing connects deleted/close to the socket's disconnected signal any more, no nested loop can free it while the handlers run, and emit filesReceived() is synchronous: once it returns, the frame has unwound and the socket can be closed and deleted inline. The QPointer stays, covering the one case that remains -- the primary shutting down while the confirmation box is open, since QLocalServer destroys its pending connections. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-02 21:19:02 +02:00
BruebachL	eae9591a17	[Server/Client] Unify report categories (#7345) ... * [Server/Client] Unify report categories into one shared source of truth * Lont. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-24 18:37:54 +02:00
BruebachL	e01912d3c8	[Tests] Add manual picture loader benchmark against the real card hosts (#7288) ... * tests: add manual picture loader benchmark against the real card hosts (cherry picked from commit 588d71d3b9a8278ab98b847802908f322c1035b7) * tests: address review on picture loader benchmark - Sandbox via unique app/org names plus Linux-only XDG redirection, derive the warm-cache probe and data path from SettingsCache, and bail out when the temporary sandbox cannot be created. - Run each pass with a fresh worker and shut workers down before reading the 429 counters, so the redirect cache is persisted and no worker thread can outlive the stack-local counters or the installed message handler. - Make s_activeCounters atomic and always forward log output when the previous handler is the built-in (nullptr) one. - Validate --timeout-min, scale the cached-pass budget with --count, honour the first --url as the stress template, and add the missing trailing newlines. - Zero-initialise SettingsCache members so the benchmark mock cannot dereference an indeterminate pointer. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-21 18:30:04 +02:00
BruebachL	db2e159dca	[Game] Allow judges to enter any game regardless of restrictions (#7315) ... Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-19 10:39:52 +02:00
BruebachL	cca4af0ec7	[PictureLoader] Add local override storage and resolution with matcher tests (#7311) ... * [PictureLoader] Add local override storage and resolution with matcher tests * [PictureLoader] Address review comments - Make deleteAllLocalOverrides static; it does not touch instance state - Drop the now-unused hasCustomArt dead code - Rename the override install methods to installPrintingOverride / installPrintingOverrideOnLoad * [Tests] Give loader matcher tests a writable HOME in CI Under GitHub's docker runner the process uid has no passwd entry, so HOME resolves to '/' and the test-mode qttest data dir cannot be created. SettingsCache's QSettings then drops every write, getPicsPath() comes back empty, and the loader searches a blank path. Point HOME at a QTemporaryDir for the duration of the run. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-18 15:03:08 +02:00
BruebachL	202a5ac958	[Client/Server/Protocol] Surface live metrics in the Developer tab (#7212) ... * [Server] Instrument command processing, game starts, and event loops Add a lock-free MetricsRegistry that accumulates per-command processing times in preallocated histogram slots (one per protobuf command type, bucketed at 1/5/10/25/50/100/250/500/1000/2500/5000 ms +Inf). The hot-path observeCommand() uses only relaxed atomic adds — no locks, no allocations, no cache-line ping-pong beyond the unavoidable counter updates. Wire the registry into AbstractServerSocketInterface::processCommandContainer() so every processed command is attributed with its container's wall-clock time. When a container exceeds metrics/slow_command_ms (default 500), a warning is logged including the connected username. Add an EventLoopWatchdog heartbeat that runs on every socket pool thread. If a heartbeat overshoots metrics/stall_warn_ms (default 2000 ms), the overshoot is recorded in atomic counters and a warning is logged. Both thresholds are configurable in servatrice.ini; setting stall_warn_ms to 0 disables the watchdogs entirely. Track game-start durations via a separate histogram in MetricsRegistry. Server_Game::startGameNow() measures the time from zone creation through player materialization and reports it via Server::observeGameStartDurationMs(). Add a live card-count gauge: Server_Game exposes getCardsInGame() and Servatrice::getCardsInGamesTotal() sums across all running games under the appropriate read locks. Include a standalone metrics_registry_test (Google Test) that validates empty registries, single/multi-sample histograms, kind encoding, overflow-slot collapse, negative-duration clamping, gauge rendering, and the game-start histogram separation. Took 10 minutes * [Client/Server/Protocol] Surface live metrics in the Developer tab Extend Response_GetServerStats with live counters from the in-process MetricsRegistry: cards in games, event loop stall totals/worst, total commands processed, average command time, active command types, and game-start count/duration. Add a repeated CommandStats message carrying per-command breakdowns (kind, extension number, resolved protobuf name, count, total ms) for every type that has seen at least one sample. Server-side cmdGetServerStats() populates all new fields after the existing DB uptime snapshot query, resolving protobuf extension names via the descriptor pool for human-readable labels like session/Command_Ping. Expand TabDeveloper with two tables: an overview section (existing DB stats plus the new live metrics) and a per-command breakdown table (Command / Count / Total ms / Avg ms) sorted by total_ms descending so the hottest commands surface first. Took 55 minutes Took 47 seconds * [Server] Drop dead Prometheus histogram, add developer command metrics, fix watchdog init order - metrics_registry: remove toPrometheusText/appendCumulativeBuckets and the time-bucket histogram that nothing in production ever emitted (the future /metrics exporter can bring it back); keep counts/totals read by the Developer tab - Fix +Inf bucket routing that never incremented, and its test that locked the bug in - Instrument developer_command container (kind 6) in processCommandContainer and stats label resolution - Read metrics/{slow_command_ms,stall_warn_ms} at the top of initServer() so stall_warn_ms=0 disables the watchdogs before pool threads start - Shrink KindStride to 1280 (largest extension in use is 1206) with a static_assert; document scrape cost of getCardsInGamesTotal; note slow_command logging has no rate limit in servatrice.ini.example * [Tests] Give metrics_registry_test an explicit main * [Server] Record only the dispatched command family; drop unused totals processCommandContainer recorded every family in a container even though the base if/else-if dispatch processes at most one. An unauthenticated client could batch a session command (login) with fabricated developer, moderator, and admin entries and forge genuine-looking samples that were never executed or authorized. Mirror the base's selection, skip when the handler was already deleted, and skip entries whose extension number is -1 (which would otherwise wrap into the previous kind's id range). [Server] Drop dead process-lifetime byte/uptime counters txBytesTotal/rxBytesTotal added an atomic RMW to every socket write and read for counters nothing consumes (cmdGetServerStats fills tx_bytes, rx_bytes, and uptime_secs from the DB snapshot). Remove the two atomics and the getTxBytesTotal/getRxBytesTotal/getUptimeSeconds getters; the incTxBytes/incRxBytes slots and mutexes remain for the ISL legacy counters. [Protocol] Document kind 5 as developer in CommandStats NumKinds is 6 and the server emits kind_index = 5 for developer commands; the comment stopped at 4. * [Client] Togglable auto-refresh for Developer stats tab * [Oracle] Fix clang-format alignment of card type priority list --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-11 17:18:56 +02:00
BruebachL	d5d99e4dfb	[Server/Client/Protocol] Add developer staff role (#7211) ... * [Server/Client/Protocol] Add developer staff role Introduce a Developer staff level (proto flag 32, DB admin bit 8) that sits between admin and moderator: no kick/ban/warn/report/admin powers, but gets server log access via a new developer command container family (GET_SERVER_STATS, VIEWLOG_HISTORY) and an idle-timeout exemption. - Protocol: IsDeveloper flag, developer_commands.proto envelope, Command_GetServerStats/Command_GetLogHistory, Response_GetServerStats, Command_AdjustMod.should_be_developer - Servatrice: fail-closed developer dispatcher, uptime snapshot handler, shared log history handler reuse, bit-8 DB mapping - Client: burgundy pawn/badge/labels/sort order, prepareDeveloperCommand, minimal Developer stats tab, log tab access, promote/demote actions Took 24 minutes Took 18 seconds * [Server/Client/Protocol] Address developer role review feedback Address ZeizaZach's review of the developer staff role: - Nudge the developer log query to exclude private chat and sender IPs (the ModeratorCommand path still sees everything). - Deduplicate Command_GetLogHistory into Command_ViewLogHistory, which now extends both ModeratorCommand (ext) and DeveloperCommand (dev_ext); the client picks the DeveloperCommand-scoped extension by extendee, and the server reads it via the extension number. - Pull the uptime snapshot SQL into Servatrice_DatabaseInterface as getLatestUptimeSnapshot() and widen the reported counters to 64-bit. - Document the admin bitfield (1 admin, 2 moderator, 4 judge, 8 developer) and add a server-side test for the developer command path. * Add missing trailing newline to user_context_menu.cpp * Remove stale includes of deleted command_get_log_history proto The Command_GetLogHistory message was folded into Command_ViewLogHistory, which deleted command_get_log_history.proto, but serversocketinterface still #included its generated header. Fresh CI builds fail on the missing file; local builds masked it by reusing a previously generated header. * [Server] Exclude chat rows when private-chat filter is bypassable A developer who omits log_location entirely — or sends only "chat" — leaves chatType, gameType, roomType all false, so getMessageLogHistory skips the target_type clause and returns every row, private messages included. When !allowPrivateChat the server now forces game+room when no surviving location was requested, guaranteeing the query always carries a target_type restriction. [Client] Demote mod+dev to moderator path in log-tab dispatch The developer command family is strictly weaker than the moderator one (no private chat, no sender_ip, ip filter ignored), so granting the developer bit to an existing moderator must not silently strip their capabilities. useDeveloperCommands is now true only when the user holds the developer bit and not the moderator bit. [Client] Hide the IP-address filter for developer log tab users The developer path ignores the ip_address query field server-side. Showing the field lets a developer type an IP and get results that are silently unfiltered by it rather than an empty result set — reads as a broken filter. Hide labelFindIPAddress/findIPAddress alongside the privateChat checkbox. * Developer pawn is silver. * [Client] Fix indentation of merged Card Art Rules / Developer tabs --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-11 17:18:56 +02:00
BruebachL	e8ec28572f	[Models] Mirror custom deck zones in the deck list model (#7203) ... * [Models] Mirror custom deck zones in the deck list model DeckListModel now surfaces the custom zones from the deck tree so views can render and edit them alongside criteria groups. The custom-zone bookkeeping that made the model unwieldy is extracted into DeckListModelCustomZones (deck_list_model_custom_zones.h/.cpp), a single self-contained unit owning every "what is / where is a custom zone" decision for the model's shadow tree: - rebuildTree mirrors each custom zone as a DecklistModelSubZoneNode under its board zone, cards flat inside (no further grouping). - The freshly built shadow tree is sorted while the model reset is still open, so views never observe unsorted intermediate order and proxies cannot desync. - Custom zones always sort after criteria groups within a board, regardless of their names. One shared sortWithCustomZonesLast backs both the live sortHelper (which remaps persistent indexes from the movement mapping) and the silent reset-time sortShadowTree. - addCard inserts flat into a custom zone by name and keeps grouping by active criteria for board zones. findCardNode resolves cards in both layouts, legacy top-level zones unchanged. - New IsCustomZoneRole lets views tell zones apart from groups. - Empty custom zones survive row removal. Zone rows themselves are only mutable through the deck tree API. A new deck_list_model_custom_zones_test suite locks the extracted shadow-tree logic (type testing, mirroring, name lookup, and the sort-with-custom-zones-last mapping). No behavior change. * [Models] Route group lookups around mirrored custom zones Group lookups (createNodeIfNeeded, findCardNode) must not resolve a mirrored custom zone that shares the group name. Introduce findGroupChild to search only non-custom children, and make addCard consult the deck tree before falling back to creating a top-level zone so cards added to an un-mirrored custom zone land inside it. mirrorCustomZones now flattens cards nested at any depth into the mirrored zone so no card is left without a model row. Add model behaviour tests (addCard routing, same-name group/zone collision, removeRows guard, empty-zone survival, findCard inside a custom zone) and fix the missing main() in the unit test binaries. * [Models] Fix addCard routing for card-named zones and nested custom zones - hasDeckZone no longer matches board cards that merely share the zone name, which previously caused infinite addCard/rebuildTree recursion - Adding to a custom zone whose deck side holds nested sub-zones appends to the deck tree instead of writing past its direct children --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-05 22:00:20 +02:00
ebbit1q	cf8e5858ab	increase the timeout for the hashing performance test (#7166) ... it seems like there is too much variance across platforms for how long this test takes probably fixes #7152	2026-08-29 15:20:49 +02:00
BruebachL	0a09884c78	[DeckList] Add custom deck zones to the deck tree (#7176) ... * [DeckList] Add custom deck zones to the deck tree Introduce user-definable zones nested under a board zone (main, side or maybeboard) so players can organize cards inside a board without changing board semantics. - addCustomZone, renameCustomZone, moveCustomZone and removeCustomZone manage zones. Names are unique across the whole deck and the standard zone names (main/side/maybeboard/tokens) stay reserved. - Board zones are created lazily on first use. - getZoneObjFromName resolves custom names to their nested node so addCard and XML loading route cards into them. Unknown names keep creating legacy top-level zones. - deleteNode keeps empty custom zones alive and only prunes empty board zones. - New deck_list_zones test suite locks hash parity with flat decks, sideboard size accounting, maybeboard exclusion from plain export and native-format round-trips. Took 17 minutes Took 11 minutes * Extract to function Took 4 minutes --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-08-26 23:15:16 +02:00
BruebachL	88aa036f7e	[Client] Detect main-thread event loop stalls (#7155) ... * [Client] Detect main-thread event loop stalls LagMonitor ticks the GUI event loop every 500 ms and records gaps beyond 2 s as stalls, warning with their duration and keeping a bounded ring of recent records for diagnostics. Measurement uses a monotonic QElapsedTimer so wall-clock steps and suspend do not fabricate stalls. Recorded timestamps stay in wall time for correlating with user reports. Took 1 minute Took 13 minutes Took 2 minutes * [Client] Rename LagMonitor constants to SCREAMING_SNAKE_CASE Took 15 minutes * [Client] Discard suspend-spanning gaps in LagMonitor Windows counts sleep time in its monotonic clock, so a suspend would fabricate one bogus stall per resume. Reset the clock on application state changes and drop implausibly huge gaps; extract recordGap() for testability. Took 3 minutes * [Client] Unit test LagMonitor stall recording Drives recordGap() directly to cover the threshold, plausibility cap, trim, and clear behavior without timing-dependent waits. Took 36 seconds --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-08-23 00:53:40 +02:00
BruebachL	b91e872f5f	[Network] Measure real server round-trip times (#7153) ... * [Network] Measure real server round-trip times Time each command container from send to response with QElapsedTimer, aggregate samples in a fixed-size ring buffer (last/median/p95/max), and emit aggregated pingStatsUpdated at most once per second so the hot path stays free of signal traffic. Stats are cleared on disconnect. Forward the signal through ConnectionController for UI consumers. Unit-tested in latency_tracker_test. Took 37 minutes Took 4 minutes # Commit time for manual adjustment: # Took 8 minutes * Move params to struct, more informative debug Took 56 seconds Took 53 seconds Took 2 minutes Took 33 seconds --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-08-23 00:03:52 +02:00
BruebachL	ed4eb1cb31	[Server/Client/Protocol] Reporting users + moderation queue functionality (#7091) ... * [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>	2026-08-21 15:00:13 +02:00
BruebachL	9eafd90a91	[Game] Playmats (#7101) ... * [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>	2026-08-21 10:40:49 +02:00
BruebachL	e9eab328a4	[PictureLoader] Schedule exponential backoff on 429 - Too many request handler and call failed on fail (#7053) ... Took 16 minutes Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-08-08 22:55:27 +02:00
BruebachL	12f0f59453	[Settings] Split cache_settings monolith into multiple SettingsManager sub-classes (#7050) ... * [Settings] Split cache_settings into multiple files Took 9 minutes Took 4 minutes * [Settings] Fwd declare settings classes in cache_settings Took 15 minutes * Fix oracle includes. Took 8 minutes * Address comments, fix windows CI Took 8 minutes * fix copy constructor visibility Took 3 minutes * lint Took 2 minutes * Fix native format tests. Took 5 minutes * Remove test header guard Took 4 seconds * Remove tests invalid in CI environ Took 24 seconds * Adjust to rebase. Took 11 minutes * Change settings file name. Took 8 minutes --------- Co-authored-by: Lukas Brübach <lukas.bruebach@bdosecurity.de> Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-07-27 11:25:39 +02:00
DawnFire42	05ae6f47a6	Unify counter clamp arithmetic into shared addClamped() helper (#7009) ... * 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	2026-06-28 16:10:57 -07:00
ebbit1q	6be9cec6e2	do not save a const reference to the user data in the info dialog (#6974) ... * do not save a const reference to the user data in the info dialog * cmake format	2026-06-09 23:35:00 -07:00
DawnFire42	8dca14933c	Centralize counter API with server-side bounds and no-op filtering (#6879) ... * 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	2026-05-21 20:39:35 -07:00
Vorliz	a20f3c0fb4	Fix #6659: Correct logging for bottom-of-library card moves (#6764) ... * Fix #6659: Correct logging for bottom-of-library card moves Cause: - This issue happens due to logic of moving the card from the top of the deck being reused when moving from the bottom of the deck, in a way that makes it impossible to check if the card came from the bottom. Resolution: - Updated the logging logic in the client for card moves. - Added a gRPC parameter ('is_from_bottom') for card moves. - Updates the server logic to reverse the order of the card move if the 'is_from_bottom' parameter is true. - Added a test to show the expected behaviour of the fix. NOTE: While the changes in this patch seem big, this is due to changing the loop in the moveCard function to a helper function, in order to make the bug fix change. The only change to the loop was to pass a variable attribution to the moveCard function because it was redundant to be in the loop. * chore: run format on test * refactor: new way to check if a move is from the bottom of the deck * refactor: change isFromBottom check to static function * update comments Co-authored-by: ebbit1q <ebbit1q@gmail.com> --------- Co-authored-by: ebbit1q <ebbit1q@gmail.com>	2026-04-21 19:05:31 +02:00
DawnFire42	ce652de272	Fix/cmake test only build (#6709) ... * fix(cmake): guard filter_string_test behind WITH_ORACLE or WITH_CLIENT. filter_string_test links against libcockatrice_filters, which is only built when WITH_ORACLE or WITH_CLIENT is enabled. Without this guard, test-only builds fail at configure time because the target doesn't exist. The guard condition mirrors the one in the root CMakeLists.txt that controls whether libcockatrice_filters is built. * fix(cmake): centralize TEST_QT_MODULES in FindQtRuntime.cmake Each test CMakeLists.txt was independently defining TEST_QT_MODULES with its own subset of Qt modules. This duplicated knowledge that already lives in FindQtRuntime.cmake (which handles module discovery for all other targets: SERVATRICE, COCKATRICE, ORACLE). Consolidate into a single definition using the union of all test requirements (Concurrent Network Svg Widgets), matching the existing pattern for application-target modules. This ensures test-only builds (-DTEST=ON without application targets) discover all necessary Qt components. * fix(cmake): guard libcockatrice_network behind application targets. libcockatrice_network is only needed by the client, server, and oracle targets. Other application-specific libraries (settings, models, filters) already have similar guards. This was an oversight that caused test-only builds to fail when network dependencies weren't available.	2026-03-15 17:24:50 +01:00
DawnFire42	9bb399606c	refactor: extract shared card insertion algorithm from hand/stack zones (#6701) ... Hand and stack zones had near-identical addCardImpl() implementations, differing only in whether resetState() preserves annotations. Extract the shared pattern into a template function (CardZoneAlgorithms::addCardToList) to eliminate duplication and enable isolated testing without Qt dependencies. Pile, table, and zone-view logic are intentionally excluded — their post-add behavior (signals, coordinate placement, hidden cards) is materially different.	2026-03-15 00:39:44 -07:00