3506 commits

Author	SHA1	Message	Date
BruebachL	9709fe4a1a	[FlowWidget] Restore arrow-key navigation for items in flow widgets (#7591) ... Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-20 16:53:04 +02:00
BruebachL	9f59f5c495	[CrashReporting] Breakpad capture, session detection and symbol upload (#7523) ... * [CrashReporting] Breakpad everywhere, session detection, symbol CI (inc 3) Breakpad on all platforms (no crashpad packaging); remove the Windows MiniDumpWithFullMemory filter and the installer WER LocalDumps full-dump registration. QLockFile per-process session tracking replaces the Windows-only .dmp scan, placed after parser.process and the instance hand-off so help/version exits and handed-off secondaries never leak a lock. CI: Release-only DSN bake from SENTRY_DSN secret, tag-only sentry-cli symbol upload (org cockatrice, project native). Tests: 533/533 ctest green, format clean, code gate PASS. * [CrashReporting] Enable http_retry, harden symbol upload, fix NSIS context Sets sentry_options_set_http_retry so the cache_keep-held crash envelope in Ask mode is written with a retry filename and actually flushes on giveConsent() instead of sitting in <db>/cache until pruning. Adds continue-on-error to both sentry-cli install steps so a Sentry registry outage or broken Git Bash install cannot block release assets. Cleans the legacy WER CrashDumps folder under SetShellVarContext current (under all it resolves to ProgramData, not the user's AppData) in installer and uninstaller; registry delete works in either context. * [CrashReporting] Extract the startup crash-report flow from main() per review * [CrashReporting] Break up the CrashReporter class doc per review * [CrashReporting] Drop the invalid --release from the symbol upload `debug-files upload` never accepted `--release` (checked against the pinned sentry-cli 3.8.0: the subcommand declares --org/--project and a set of type/id/scan options, and execute() reads no release; the only global flags are --url/--header/--auth-token/--log-level/--quiet/--allow-failure). Clap rejects the unknown flag before any upload happens, so every tag build has been shipping without symbols, with continue-on-error keeping the release pipeline green. Release association is also the wrong mechanism: debug information files are matched to events by debug ID via the event's debug_meta. The Sentry release of a native event comes from the event itself (COCKATRICE_SENTRY_RELEASE), not from the DIF upload. Remove the flag and the now-unused SENTRY_RELEASE env from both invocations (Linux, and Windows/macOS). The symbol upload stays continue-on-error (a Sentry outage must not block the release assets); revisit once it has proven itself on a tag. * [CrashReporting] Split debug information out of packaged release binaries -g is required so the CI symbol upload has DWARF to send, but it was left in the shipped binaries too: the Linux Release client is 563 MiB, 552 MiB of it debug info, and every distribution channel pays for that. Keep the build tree unchanged (CI uploads symbols from build/) and strip only the packaged copy. A CPack pre-build script extracts each ELF's debug info to COCKATRICE_DEBUG_SYMBOL_DIR and then runs `strip --strip-debug --strip-unneeded`. .eh_frame is allocatable and survives, so unwinding is unaffected, and the build ID is identical in the stripped binary and the sidecar. Measured on a local Release build (GCC -O2 -g, WITH_CRASH_REPORTING=ON), bytes: binary build tree packaged before packaged after .debug sidecar cockatrice 590,722,560 590,722,560 28,086,480 579,623,576 oracle 53,432,608 53,432,608 1,649,888 52,075,344 tar.gz of both trees: 289,964,521 -> 19,018,058 (~15x smaller). Only ELF (Linux/BSD) is handled: Windows already links /DEBUG with separate PDBs, and macOS extracts a dSYM from the bundle. Opt-out via COCKATRICE_SPLIT_DEBUG_SYMBOLS; skipped with a warning when objcopy/strip are missing. CPACK_PRE_BUILD_SCRIPTS (not CPACK_INSTALL_SCRIPTS, which runs before staging) invokes it, and the script refuses to run unless CPACK_TEMPORARY_DIRECTORY is a real CPack staging tree so a missing variable can never turn its glob into a filesystem-wide walk. * [CrashReporting] Upload Windows PDBs from where Ninja actually puts them The Windows branch of the symbol upload looked in build/<target>/Release, which is the Visual Studio generator's layout. The Windows leg uses Ninja, which is single-config: linker PDBs land in build/<target>/, next to the binaries — the same paths the "[Windows] Upload PDBs" artifact step below globs with if-no-files-found: error. So paths was always empty on Windows, the step printed "No symbol paths found, skipping symbol upload" and exited 0: a silent no-op behind continue-on-error. Every Windows build would then report "A required debug information file was missing" for cockatrice.exe with a PDB-style debug id (GUID-age), exactly as if no upload step existed at all. Use the artifact step's globs. Also turn the empty-path case into a failure in both symbol steps: a silent skip is indistinguishable from a successful upload (which is how the wrong path went unnoticed), and continue-on-error already keeps a broken upload from failing the release. Verified by extracting both steps' run blocks and executing the Windows branch against a Ninja-style tree: it now passes the three linker PDBs to sentry-cli (and not the CMakeFiles/*.pdb intermediates), and exits non-zero with an ::error:: annotation when there is nothing to upload. * [CrashReporting] Archive PDBs on tag builds so a failed upload is recoverable The "[Windows] Upload PDBs" artifact step skipped tag builds (github.ref_type != 'tag'), so for a release the symbols existed only inside the Sentry upload — which is continue-on-error. A failed upload therefore lost the ability to symbolicate that release permanently: no rebuild produces the same PDB, since the debug id is a per-build GUID+age. That is how every Windows crash reported so far became unreadable. Archive the PDBs for tag builds as well, and order the step after "Upload to release" so a missing PDB can never block a release from being published. Retention stays at the repository default; nothing consumes the artifact automatically, it is the manual recovery path (gh run download <run> -n <name>-PDBs, then sentry-cli debug-files upload). Verified by re-parsing the workflow and asserting the step order (release -> PDBs -> attest) and the broadened condition. * [CrashReporting] Upload the Windows executables with their PDBs Sentry needs two things per image on Windows: symbolication and stack unwinding. The PDB covers symbolication, but for a 64-bit PE the Call Frame Information (.pdata/.xdata) lives in the executable and is not copied into the PDB, so uploading PDBs alone leaves the image at "Stack Unwinding: Missing". That is exactly what the event's Images panel showed for cockatrice.exe, while Sentry's own Microsoft symbol server resolved KERNELBASE.dll and ucrtbase.dll from the same dump. sentry-cli also needs the pair in the same invocation: only then does it associate the PE's code identifier with the PDB's debug identifier, otherwise "the identifiers diverge and Sentry might not be able to resolve the correct file for symbolication" (docs: Debug Identifiers). Do the same for macOS, where the dSYM carries the debug information but the Mach-O carries the unwind information, and include the executables in the symbol artifact so a re-upload still has the pair to offer. Verified by extracting the step and running both branches against stub harnesses: Windows passes the three exe/pdb pairs in a single sentry-cli invocation and still fails loudly on an empty tree; macOS passes the dSYM and the Mach-O together. * [CrashReporting] Upload the shipped Qt runtime DLLs too Qt ships no PDBs with its MSVC SDK, so every Qt frame in a crash report resolved to nothing: an image with neither PDB nor PE has no unwind information (on 64-bit Windows the Call Frame Information lives in the PE and is not copied into the PDB) and no symbol source to fall back on. The list is required-runtime.txt, the manifest .ci/compile.sh already asserts the installer against, so what gets uploaded is exactly what an install carries. Qt frames now get unwinding plus export-table names; line numbers still need Qt's debug information files, which the SDK omits. The guards are not cosmetic: the step runs under `bash -eo pipefail`, where a sed over a missing file or an unmatched ls glob aborts the whole step, and continue-on-error makes that look identical to a successful upload. * [CrashReporting] Upload only the binaries, archive symbols only on tags The Linux leg handed sentry-cli whole build directories: 471 candidate files, of which 7 were object files that can never match a stack frame. Only the three linked binaries carry the build-ids Sentry looks up; the packaged copy is split and stripped but keeps that same build-id. The Windows symbol archive ran on every packaging build even though only tag builds upload to Sentry, so a PR build paid for an artifact nothing could ever consume. Tags keep it as the durable copy for a failed upload. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-19 14:57:04 +02:00
BruebachL	ad0fd2d4b0	[CrashReporting] Sentry send path with consent gating (#7522) ... * [CrashReporting] Real send path with consent gating (DSN still placeholder) require_user_consent + give/revoke around the Ask dialog, message event with in-memory crash-log.txt bytes attachment, before_send scrubs message/logentry/breadcrumbs/data of cockatrice:// secrets. Failure box offers Save (shows path) / Discard (revokes + rejects) / Cancel (back to dialog); reject() override so X/Escape cannot bypass revocation; silent AlwaysSend path falls back to a local file. Empty DSN keeps the previous mock behavior byte-identical. Tests: 528/528 ctest green, format clean, code + UI gates PASS. * [CrashReporting] Attach crash log and user note to the pending crash event The redacted log used to ride a separate message event from the native crash (minidump + stack), forcing manual correlation in Sentry. Fold it into the pending crash envelope(s) instead by appending well-formed attachment items before consent releases them: sentry-native replays cached envelopes byte-for-byte (raw read, raw serialize in 0.17.3), so the appended crash-log.txt and user-note.txt items upload with the very event that already carries the dump and stack. Covers both storage locations: Ask mode's consent-held files in <database>/cache and AlwaysSend's transport-dumped files in <database>/<run>.run, which must be attached before sentry_init replays them. When no pending crash event exists (unclean exit without a native crash) the standalone message event remains as fallback, now also carrying the optional note. The dialog gains an optional free-text field ('Here's what I was doing...') that travels as user-note.txt; the mock JSON records it too. * [CrashReporting] Send PROJECT_VERSION as the Sentry release main() reported VERSION_STRING (PROJECT_VERSION_FRIENDLY) as the Sentry release — that carries a build date, so every build of the same version landed in its own release, and it never matched the installer, which reports CPACK/PROJECT_VERSION. Bake COCKATRICE_SENTRY_RELEASE= PROJECT_VERSION instead: date-free, identical between client and installer, and derived from the git tag on tag builds, so events, symbol uploads and the installer log all group under one release. * [CrashReporting] Friendlier crash dialog copy Reword the report dialog along a single friendly arc: headline ('Oops, we crashed — and now we need your help to make Cockatrice better!'), why helping is easy, exactly what gets collected (crash dump with stack trace, app version, OS info, redacted log excerpt), the value of each report ('better and less crashy for everyone'), and the consent choice ('Send and remember makes this automatic without interruption in the future'). The checkbox itself stays direction-neutral ('Remember my choice') because it also persists a 'Don't send' decision. * [CrashReporting] Log where crash reports go and warn when they cannot The client resolved COCKATRICE_SENTRY_DSN (environment over the baked-in value) without ever saying which one won, and sentry-native refuses to build a request for a DSN it considers invalid without reporting anything, so a tester pointing the client at a scratch project had nothing to go on when a crash never appeared. Say which DSN source won, and log the destination (host and project id, never the key) once the SDK is armed. Warn instead of staying quiet when no DSN is configured at all, when a DSN is malformed - disabling reporting rather than silently falling back to the project baked into the build, which is the rule the installer already follows in cmake/CrashReporting.cmake - and when the build was compiled without Sentry support. A deliberate Never-send stays at info level: that is a choice, not a misconfiguration. Also warn when a pending crash envelope was captured for a different destination than the session that will upload it. sentry-native posts every envelope with the DSN held in the transport state of the session doing the sending, and envelopes restored from disk are raw, so the "dsn" header of a cached envelope is never read: a crash captured for one project can be uploaded to another with nothing in the log to show it. Tests cover the DSN shape check (same unanchored pattern the build matches, so a DSN this build accepted is never rejected here), destination description without the key, and the mismatch detection, which stays quiet for a different key on the same project, unreadable files, and headers without a dsn. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-19 14:57:03 +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
BruebachL	4343544588	[Client] Settings: isolate the selected section when drilling down (#7488) ... * [Client] Settings: isolate the selected section when drilling down Show only the selected section's group box. Restore the overview on page rows. Filter search results to the control's containing section. Reset the scroll position on every change. Reworked to stay in the enlarged modal instead of the settings tab. * [Client] Settings: re-clicking an isolated section restores the overview * [Settings] Keep section row selected when drilling down per review * [Settings] Stop stealing nav selection mid-gesture per review * [Settings] Sample nav selection pre-press so re-click detection works per review * Don't flash the section on drill-down * [Client] Settings: clear the section filter when selecting the page row per review onTabClicked() returned early for the already-current page before switchPageContent() could call applySectionFilter(index, nullptr), so clicking the page row while a section was isolated moved the selection but left the page filtered. Only skip the restore when no section is isolated. * [Settings] Select the search result's section in the sidebar per review Reaching a setting through the search isolated its section and flashed the widget, but left the navigation selection on the page row, so the sidebar looked like it had only jumped to the section's page. Extract the section-row selection shared with drill-down into selectSectionRow() and use it for search results too, keeping the gesture/re-click behavior untouched. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-18 22:48:52 +02:00
BruebachL	4750e67b7f	[Client] Settings: sub-page rows deep-linking into page sections (#7487) ... Add AbstractSettingsPage::getSections built from each page's group boxes. Show each multi-group page's sections as child rows in the navigation tree. Section rows show the parent page and scroll to and highlight the group box. Start pages collapsed. Keep single-group pages flat. Reworked to stay in the enlarged modal instead of the settings tab. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-18 22:48:51 +02:00
BruebachL	4974cac59a	[Client] Settings: add top-right cogwheel shortcut (#7485) ... * [Client] Settings: add top-right cogwheel shortcut Add a subtle flat settings button to the tab bar corner that opens the settings dialog via the existing menu action slot. Icon and tooltip follow the menu action in retranslateUi. * [Client] Settings: flat black-cogwheel corner entry per review --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-18 22:48:51 +02:00
BruebachL	860625be4d	[Client] Settings: grouped left navigation for settings pages (#7484) ... * [Client] Settings: grouped left navigation for settings pages Replace the top icon-button bar in SettingsView with a grouped left QTreeWidget: Application, Cards and Data, Chat and Sound, Advanced. The active page is the selected row (palette highlight); search mode hides the tree; group headers expand but never select. Page tr() contexts stay DlgSettings; new group names are SettingsView strings. Reworked to stay in the enlarged modal instead of the settings tab. * [Settings] Make groupedPages static const per review --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-18 22:48:50 +02:00
BruebachL	665f7267e4	[Client] Settings: enlarge the settings dialog (#7483) ... * [Client] Settings: enlarge the settings dialog Keep settings modal per review: size to ~90% of the main window (5% margins each side) with screen fallback, size grip and 750x700 minimum bounded to the screen. Drops the tab approach while giving dense pages room without the tab-switch friction. * [Client] Settings: clamp dialog width to 1280 per review * [Client] Settings: extract dialog size calculation per review Move the host/screen target math out of the DlgSettings constructor into a calculateDialogSize helper. Drop the leftover 'per review' wording from the clamp comment. No behavior change. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-18 22:48:50 +02:00
BruebachL	679762c624	[Client] Settings: extract SettingsView from DlgSettings (#7482) ... * [Client] Settings: extract SettingsView from DlgSettings Move the search bar, tab bar, stacked pages and search results out of DlgSettings into a reusable SettingsView widget. The dialog becomes a thin frame (title, OK button, close validation) delegating to the view. No visual or behavior change; tr() contexts preserved as DlgSettings. * [Client] Settings: match CI clang-format-18 line breaking --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-18 22:48:50 +02:00
RickyRister	b849a57dab	[TabRoom] Fix compile failure due to unused lambda capture (#7577)	2026-10-18 11:10:26 +02:00
BruebachL	a139084ad6	[Security] Add challenge-response auth with scrypt verifiers and stop storing plaintext passwords (#7193) ... * [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>	2026-10-16 22:52:26 +02: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	c99113ab0e	[Client] Tabbed room channels with unread states and collapsible chat (#7478) ... * [Client] Add room channel selector with per-channel chat views * [Client] Tabbed room channels with unread states, collapse, read-only, banners Reworks the room chat channel display: channels move out of the combobox into a tab bar below the message input showing every channel available to the user. - Each tab shows its unread message count and is tinted when it holds unreads or a mention; both colors are user-configurable chat settings (channelUnreadColor, channelMentionColor) with a settings-page section. - Clicking the active tab collapses the chat: message view and input hide and the splitter section minimizes to the tab strip; expanding restores the previous splitter sizes. Viewing a channel clears its unread state. - Channels the user may read but not write (per write_access_level) show with the message input disabled and a read-only placeholder; sends are guarded locally, the server still enforces. - Channel banner_message strings render at the top of their channel view on join, like the room join message on Main. - Replayed channel history no longer inflates unread counts; moderation redaction now applies across all channel views. * Fixup: keep chat tab bar visible when chat is collapsed QSplitter children are collapsible by default, so setSizes({1, 0}) hid the whole chat group box including the channel tab bar. Mark the chat widget non-collapsible so collapsing clamps to its minimum size (title + tab bar) instead. * [Room] Explicitly initialize MAIN_CHANNEL per review * [Room] Address channel tab review follow-ups - gate @all mentions on moderator/admin sender to match ChatView, whole-word matching for own name and highlight words - hide single-channel tab bar so old servers do not show a lone Main tab that collapses chat on misclick - capture target ChatView in sendMessage so errors land in the channel the message was sent from - share ignore/unregistered filtering between RoomSay and RoomChannelSay via isRoomMessageIgnored * [Client] Fix tab_room.cpp formatting per clang-format-18 --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de> Co-authored-by: BruebachL <bruebachl@cockatrice.example>	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	711386574d	[Client] Tell the user when their playmat art is blocked (#7533) ... * [Client] Tell the user when their playmat art is blocked A server DENY rule silently drops banned playmat art: SetPlaymat fails with no handler and DeckSelect strips the mat while answering RespOk. Surface both cases with the same critical message box the banner card art flow uses. DeckSelect strips are detected by comparing the requested mat, passed via PendingCommand extraData, against the server-echoed deck. Notices dedupe per card and reset on success. * [Client] Gate playmat blocked notice on RespFunctionNotAllowed per review Only SetPlaymat with RespFunctionNotAllowed signals a card art DENY rule. Older servers answer RespInvalidCommand and strip the mat from DeckSelect, so drop the DeckSelect comparison to avoid false blacklisted popups on pre-playmat servers. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de> Co-authored-by: BruebachL <bruebachl@users.noreply.github.com>	2026-10-16 19:50:57 +02:00
BruebachL	bbe438b09a	[Completer] Highlight popup rows without rewriting the line edit (#7502) ... * [Completer] Highlight popup rows without rewriting the line edit Selecting a popup row with ClearAndSelect emits selectionChanged, which QCompleter turns into highlighted() and QLineEdit turns into setText(). Since be2c16eca5 (#7466) that rewrites the typed text on every completion reset / arrow-key press (e.g. typing "A" becomes "Axe" and cannot be edited), regressing the 514014dc00 (#7447) fix that used NoUpdate. Add selectWithoutRewrite(), which blocks the selection-model signals while applying ClearAndSelect so the highlight stays visible without touching the line edit, then syncs the preview manually. Route both the above-popup auto-select and Up/Down navigation through it, and handle Up/Down for below-popups too instead of falling through to Qt's rewriting default. Enter/click acceptance via activated() is unchanged. * [Completer] Repaint popup highlight after silent selection change Blocking the selection model's signals also suppresses the view's own selectionChanged/currentChanged repaints, so force a viewport update after restoring signals. Use EnsureVisible to match Qt's normal scroll behaviour. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-16 19:01:38 +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	213ce691dd	[Completer] Minimum size popup, fall back to compact design if not possible. (#7111) ... * [Completer] Minimum size popup, fall back to compact design if not possible. Took 15 minutes Took 6 seconds Took 5 minutes * [Completer] Fix popup going offscreen by clamping in screen coordinates The position clamping in applyPopupWidth() was comparing popup->pos() (parent coordinates) against screen->availableGeometry() (screen coordinates). When the parent widget is not at the screen origin the clamping computed a position in the wrong coordinate space, placing the popup offscreen. Convert to global coordinates first, clamp, then map back to parent coordinates. * [Completer] Clamp popup from geometry() in screen coordinates setMinimumWidth() already widens the popup synchronously, so the old width() < minWidth guard never ran and the popup grew past the right screen edge. The popup is also a top-level Qt::Popup window, so move() takes global coordinates and the mapFromGlobal round-trip moved it to the top-left of the screen. Clamp x from geometry() after setMinimumWidth() and move(QPoint(clampedX, popup->y())) directly. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-15 18:10:07 +02:00
BruebachL	96b0888a73	[Oracle] Don't report a cancelled card database update as successful (#7082) ... * [Oracle] Don't report a cancelled card database update as successful (#3470) Took 3 minutes * [Oracle] Only report cancel when nothing was saved yet Rejecting the wizard after SaveSetsPage or a download page already wrote the database to disk still replaced cards.xml on disk, but wasCancelled() forced exit code 1 so Cockatrice skipped the reload. Track successful saves with OracleWizard::markDatabaseSaved() and only treat a cancel as a failure when nothing was saved. * [Oracle] Restore updater stderr in card update error dialog --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-15 15:21:07 +02:00
BruebachL	bbf0425655	[Style] Confirm broken Windows styles with a dialog offering Fusion (#7507) ... * [Style] Confirm broken Windows styles with a dialog offering Fusion * [Style] Address review nits on broken style dialog Extract Fusion index lookup into findFusionIndex helper and add all message box buttons before choosing the default button. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-15 12:41:01 +02:00
BruebachL	677692500b	[Game] Raise rubber band above count label and collapse multi-selection on click (#7451) ... * [Game] Raise rubber band above count label and collapse multi-selection on click Raise rubber band on start and collapse multi-selection to clicked card when left-clicking an already-selected card with no modifiers. * [Game] Collapse multi-selection on release instead of press Collapsing at press time dropped the selection before the drag started, which broke dragging multiple cards. Keep the selection on press and collapse it on release when the pointer stayed within the drag distance. * [Game] Defer selection collapse until the double-click window passes A release without drag collapsed the multi-selection immediately, so a double-click that followed (playSelected acts on scene()->selectedItems()) only ever saw the single clicked card. When the card handles double-clicks on the selection, arm the collapse on a single-shot timer for QApplication::doubleClickInterval() instead and cancel it from the double-click handler; a new press disarms the other cards and a selection-count guard covers rubber-band/ctrl changes meanwhile. * [Game] Keep sideboard multi-selection on double-click DeckViewCard overrides doubleClickActsOnSelection to preserve the selection, restoring master behavior where double-clicking moves all selected cards between main and side. DeckView cancels any pending collapse before reading the selection. * [Game] Remove duplicated inline selection-collapse declarations * [Game] Fix DeckView collapse call visibility and multi-selection collapse Make cancelPendingSelectionCollapse() public so DeckView can call it on selected DeckViewCards. Always arm the deferred collapse on release instead of skipping cards whose double-click acts on the selection; the double-click-interval timer plus the cancel calls in CardItem::mouseDoubleClickEvent and DeckView::mouseDoubleClickEvent already preserve the selection for double-clicks. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-15 11:35:03 +02:00
BruebachL	24874b10cf	[GameSelector] Persist quick-filter toolbar edits and repair filter settings migration (#7500) ... * [GameSelector] Persist quick-filter toolbar edits and repair filter settings migration - Save game-filter parameters on toolbar changes, debounced with flush on close - Repair pre-camelCase gamefilters.ini keys in place on startup - Map the legacy creator-name filter to hostNameFilter - Balance begin/endFilterChange in GamesProxyModel::refresh - Align the hideIgnoredUserGames default between model and settings - Cover the migration with regression tests * [Review] Address RickyRister comments on filter persistence - Rename game-type prefix constants to SCREAMING_SNAKE to match MIGRATION_SENTINEL_KEY and project style - Document why GameSelector destructor flushes a pending debounced filter save (toolbar path has no other writer) --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-13 07:45:08 +02:00
BruebachL	a712553c81	[CardDB] Reload after the first run enables all sets without freezing startup (#7470) ... * [CardDB] Reload when the first run enables all sets checkUnknownSets() enabled every set when none was enabled, but the database had already been parsed under that empty enablement, so the parsers had stripped every printing. It only notified the models and never reloaded, leaving the deck editor's card database display empty until the next launch. Route the all-sets-enabled case through the same reload the new-sets path uses, and skip caching a snapshot whose sets are all disabled. * [CardDB] Own checkUnknownSets after startup and fix the cache skip The loader's initialLoadComplete guard skipped checkUnknownSets() on the first successful load of the session, so a download that landed after startupConfigCheck() and the onboarding wizard had already run (e.g. skipping the wizard download and fetching later from the menu) left every set disabled and the deck editor blank until the next launch. The guard also let the check run on the worker before the queued snapshot swap, and alongside reloadCardDatabasesAndNotify()'s own check, double-emitting cardDatabaseNewSetsFound on manual reloads. Move the check to the callers that load after startup: the post-update, custom-import, and spoiler paths run it via a QFutureWatcher once the finished snapshot has been swapped in, and the loader's synchronous path-change reload runs it directly. Judge the cache skip by the parsed sets' own enablement, what the parsers actually saw, instead of getEnabledSetNames(), which reports empty for the noop controller even though it enables every set. Wait for the first-run reparse before opening Manage Sets so the dialog cannot bind to the pre-reload set instances. * [CardDB] Queue post-reload check and unblock first-run Manage Sets reloadCardDatabasesAndNotify() no longer emits cardDatabaseEnabledSetsChanged after the swap's Reset already rebuilt the models (second 36k rebuild froze startup), and its follow-up checkUnknownSets() is queued to the GUI thread so it sees swapped-in data. cardDatabaseAllNewSetsEnabled() chains Welcome/Manage Sets off a QFutureWatcher instead of waitForFinished() blocking the event loop during the reparse. * [CardDB] Batch the first-run enable-all into a single settings flush checkUnknownSets() enabled every set with ~3 QSettings rewrite cycles per set (sort key, known, enabled), and each setValue re-opens and re-syncs the whole cardDatabase.ini. With a full database (874 sets) that froze the GUI thread for ~9s after the database download (LagMonitor: event loop stalled for 9129 ms, measured offscreen with a real 59MB cards.xml), reading as a total application hang that recovers on its own. Persist the enable-all through the controller's batched saveSets() in a single flush instead, mirroring what SetsModel::save() already does: CardSetList::enableAllBatched() assigns fresh sort keys and marks every set known+enabled in memory, then writes all records at once. SetSaveData gains the isKnown flag so the batch covers all three options, and SetsModel::save() passes the sets' known state through (unchanged behavior there). Also drop the cardDatabaseEnabledSetsChanged emission from the all-sets-enabled branch: the reload chained off cardDatabaseAllNewSetsEnabled rebuilds the models from the re-parsed snapshot via cardDatabaseReset, so emitting both rebuilt the 36k-card models twice on the GUI thread. Adds a cache-invalidation test driving the real checkUnknownSets() first-run path. * [CardDB] Batch the new-sets-found enable-all the same way The first-run batching fixed the 874-set case, but the new-sets-found dialog path (enableAllUnknownSets/markAllSetsAsKnown) still wrote per set and per option, each setValue re-opening and re-syncing the whole cardDatabase.ini. Users with hundreds of custom sets hit the same multi-second GUI stall when answering the dialog. Persist both through the controller's batched saveSets() in a single flush, writing only changed sets (so the common no-op check skips the flush entirely). Removes the now-unused write-through CardSetList methods (enableAll, enableAllUnknown, markAllAsKnown, guessSortKeys) so the batched flush is the only way to persist bulk set changes. Extends the test controller mock with per-write/batched-flush counters and adds tests asserting both dialog paths persist with exactly one flush and zero individual writes. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-13 06:29:16 +02:00
BruebachL	1a4d89ff8f	[Game] Don't alert game tab for spectator activity (#1848) (#7263) ... Spectator join/leave events were triggering the game tab's attention indicator (exclamation mark and taskbar alert) for players who often leave the tab open while waiting for an opponent. This made spectator activity a false alarm. Only emit the user event when an actual player joins or leaves, keeping spectator joins/leaves quiet. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 16:54:03 +02:00
BruebachL	4c5a5e3e55	[Game] Invalidate view when hiding replaced card menu (#7453) ... * [Game] Invalidate view when hiding replaced card menu Hide any visible card menu before replacing and invalidate viewport. * [Game] Fix viewport update compile error in setCardMenu QWidget::update has no QRectF overload, and sceneRect is in scene coordinates rather than viewport coordinates anyway, so invalidate the whole viewport instead. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 15:36:11 +02:00
BruebachL	5d06bb903d	[Chat] Fix card completion after an @ mention (#7469) ... An open [[ card link now takes precedence over a space-preceded @ mention, so typing an @ before a card no longer shadows the card completer. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 15:35:10 +02:00
BruebachL	57a6a0d9cb	[Client] Warn when data paths point inside the program directory (#7473) ... Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 15:34:49 +02:00
BruebachL	07eb9f0103	[Report] Prompt for a note on Resolve and drop the redundant option (#7445) ... - Remove the third close option (Resolve with note...) so closing a report is one of two clear actions - Make Resolve always prompt for the optional resolution note, matching Dismiss, and rename the button to Resolve... - Halve the resolveReport signature now that every caller prompts Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 15:33:48 +02:00
BruebachL	38acddeede	[Report] Add context menu to report queue rows (#7444) ... * [Report] Add report reopen action for closed reports - 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 * [Report] Surface resolution audit info in report details - Add resolved_by_name to ServerInfo_Report and join resolved_by users in the report list, my-list and details queries - Render resolution note and resolved-at/by in the report detail pane instead of duplicating the description into the chat log context * [Report] Add context menu to report queue rows - Open the standard UserContextMenu on right-click/Menu key for the reported user, selecting the row first so the details panel matches the menu target - Carry the reported user's level in ServerInfo_Report so the menu shows the same actions as the user list, with an IsUser fallback for deleted or unregistered users - Wire the Private chat action through TabReport to TabSupervisor and retranslate the menu on live language switches * [Server] Fix reported_user_level column indices in report queries The SELECT lists resolved_by_name at index 10, shifting room_id to 11, replay_id to 12, reported_user_id to 13 and reported_user_admin to 14. The code read 12/13, so 'user exists' meant 'a replay exists' and the admin flags came from the user id (e.g. id 5 -> admin+judge; users without a replay reported as unregistered, hiding registered-only actions). Fixed in cmdReportList and cmdReportMyList, plus the column comment. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 15:33:47 +02:00
BruebachL	74a6acde91	[Report] Surface resolution audit info in report details (#7443) ... - Add resolved_by_name to ServerInfo_Report and join resolved_by users in the report list, my-list and details queries - Render resolution note and resolved-at/by in the report detail pane instead of duplicating the description into the chat log context Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 15:29:09 +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	be2c16eca5	[UI] Fix auto-completion acceptance on Space/Tab/Enter (#7466) ... * [UI] Fix auto-completion acceptance on Space/Tab/Enter * [UI] Accept current completion when no item selected in popup * [UI] Also accept completion on focus out when none selected * [UI] Keep focus in chat on Tab completion acceptance * [UI] Prevent Tab from moving focus when no completion popup * [UI] Refocus on Tab when completing * [UI] Let focusOut handle Tab completion acceptance * [UI] Restore Tab behavior to match original (handle in focusOut) * [UI] Defer refocus on Tab to avoid focus chain issues * [UI] Auto-select first completion match in popup * [UI] Auto-select first match when completer popup resets * Revert "[UI] Auto-select first match when completer popup resets" This reverts commit c7a3de3595. * Revert "[UI] Auto-select first completion match in popup" This reverts commit ae0dfb8622. * [UI] Ensure completion selection updates on model changes * Revert "[UI] Ensure completion selection updates on model changes" This reverts commit 686bbef5b2. * [UI] Highlight the selected completion in chat popups The card completer's above-variant moved the current row with NoUpdate, so the delegate never painted the selection border, and it used Select which accumulated highlights while navigating. Use ClearAndSelect so only the current row is highlighted and the old one is cleared. The mention popup opened with no current row at all, so nothing was highlighted and there was no visual hint of what Enter would insert. Select the row currentCompletion() uses, and install a delegate that mirrors the active highlight into the inactive group so the selection stays visible when the transient popup is not the active window. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-10 09:45:42 +02:00
BruebachL	b8e8f2c738	[PrintingSelector] Fix crash setting all printings to preferred (#7467) ... getPreferredPrinting returns a null PrintingInfo when a deck card has no database entry or printings. SetProviderIdToPreferred dereferenced getSet() unconditionally, crashing on custom or unresolved cards. Guard against a null preferred set and leave the card's cleared printing data as is. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-10 09:36:26 +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	6bed5d6f67	[Style] Warn about the broken Windows native styles and stop resolving System to them (#7461) ... * [Style] Fall back to the built-in windows style instead of windowsvista The Windows Vista native style is broken the same way as the Windows 11 one, so the sanitizer now catches both and falls back to the built-in QWindowsStyle instead of the Vista plugin. The style picker no longer offers either. * [Style] Keep Windows native styles selectable but warn about them --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-09 11:07:22 +02:00
BruebachL	aaafa72c0e	[Game] Use native title bar height for ZoneViewWidget close button (#7450) ... * [Game] Use native title bar height for ZoneViewWidget close button Compute title bar height from style (PM_TitleBarHeight) instead of hardcoded 24.0 to align hit box with native close button. * [Game] Fix ZoneViewWidget title bar height compile errors Drop the redundant static on the anonymous namespace helper, stop redeclaring titleBarHeight's parameter and reuse tbh in the fallback path, and restore the position clamp that still referenced the removed kTitleBarHeight constant. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-09 06:08:15 +02:00
BruebachL	56c456414b	[Game] Use non-blocking popup for zone context menus (#7452) ... Replace QMenu::exec with popup() for zone context menus to avoid blocking the scene. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-09 06:07:39 +02:00
BruebachL	95f02ac0a1	[CardDB] Drop the rebalanced-cards setting entirely (#7457) ... * [CardDB] Drop rebalanced-card filtering from the v4 parser Rebalanced cards no longer exist, so the includeRebalancedCards preference has no data to filter. Always keep printings marked isRebalanced and remove the now-unused ICardPreferenceProvider plumbing from CockatriceXml4Parser and CardDatabaseLoader. The setting itself (CardsDisplaySettings, Manage Sets checkbox, Oracle attribute mapping) is removed in a follow-up. * [CardDB] Drop the rebalanced-cards setting entirely Rebalanced Alchemy cards no longer exist, so remove the includeRebalancedCards preference surface: the CardsDisplaySettings getter/setter/signal, the ICardPreferenceProvider and ICardsDisplaySettingsProvider methods, the Manage Sets checkbox, the legacy settings migration mapping, the isRebalanced attribute mapping in the Oracle importer, and the isRebalanced attribute in the v4 schema. Builds on #7448, which already stopped the v4 parser from filtering on the preference. * [CardDB] Drop the unused isOnlineOnly printing property The Arena-card detection switched from isOnlineOnly to isRebalanced in #5778, so nothing has read this printing property since; it only round-trips from the Oracle importer into cards.xml. Remove the importer mapping, the benchmark fixture field and the v4 schema attribute so regenerated databases stop carrying it. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-09 06:02:47 +02:00
BruebachL	514014dc00	[Completer] Stop line edit rewrite on popup highlight (#7447) ... * [Completer] Stop line edit rewrite on popup highlight * [Completer] Extract the completion index into a variable Make the popup selection update readable at a glance. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-08 14:38:11 +02:00
BruebachL	d603a2bb6a	[Tooling] Apply include-what-you-use across the tree (#7396) ... * [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>	2026-10-08 11:13:50 +02:00
BruebachL	d0991b68f6	[EDHRec] Update tab title when a card is searched (#7408) ... TabEdhRecMain never emitted tabTextChanged, so TabSupervisor::updateTabText was never called and the tab kept the title it was created with. addEdhrecMainTab() creates the tab without a card, so every search left the title reading "EDHRec: " with no card name for the rest of the tab's life. addEdhrecTab() sets the card before myAddTab(), which is why the older TabEdhRec tab was never affected. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-07 14:55:25 +02:00
tooomm	5dc5390880	Fix and rework client updater logic (#7038) ... * first version * combined logic + better regex * Simplify, we only support 64bit, installer prevents wrong installs * lint + fix Linux versions (missing declaration) * update regex * improve/add comments * comments * ReleaseChannel: parse non-numeric OS versions on Windows Server QSysInfo::productVersion() returns "Server 2022" on Windows Server, so splitting on '.' and calling toInt() made the version unparseable and disabled the in-client updater there. Extract the first numeric token instead; Server 2022 becomes 2022 and matches the Win10 assets. * ReleaseChannel: don't offer 64-bit assets on 32-bit Windows The old downloadMatchesCurrentOS() had an explicit 32-bit branch that only accepted "32bit" assets. Restore that guard: 32-bit Windows builds return no match instead of being offered the 64-bit Win10 installer that won't run. * ReleaseChannel: only query the OS version on updater platforms The in-client updater only ships assets for macOS and Windows, but the host system version was still being resolved on every platform, including Linux where it is never used. Compute it only under the Q_OS_MACOS / Q_OS_WIN guards. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-03 17:57:03 +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	3aba1ef929	[Settings] Back up the settings folder before migrating (#7386) ... * [Settings] Back up the settings folder before migrating The per-file settings migration only preserved global.ini, as global.ini.old. Everything else the migrations write was overwritten in place with no way back: the legacy migration and the 17 per-domain migrateXxxSettings functions all write straight into the per-domain .ini files, so any of them that got a key mapping wrong silently replaced a working file. The re-migration path is the sharpest case, because migrateSettingsFromGlobalIni() runs again whenever an older build wrote non-sentinel keys back into global.ini, and those values then land on top of per-domain files the user had already edited. Snapshot the whole settings folder before either migration runs, into a timestamped sibling settings-backup-<yyyyMMdd-HHmmss>. A sibling rather than a child, so a backup is never copied into the next backup and the "Open Settings Folder" action keeps showing only live files. Keep every snapshot; settings files are kilobytes, so growth is not a concern. SettingsMigration::isMigrationPending() gates the copy so a normal start creates nothing. It reuses the same condition migrateSettingsFromGlobalIni() uses to decide whether it has work, factored into a shared helper so the two cannot drift, and it ignores the legacy migration on an empty profile where that migration only writes its sentinel. The backup itself skips a folder whose files are all empty, which covers a fresh profile whose global.ini was created but never written. A failed copy logs a warning and returns an empty path rather than aborting the migration, since a full disk or an unwritable data folder must not leave the client unable to start. global.ini.old still works as the single-file escape hatch. Document the folder layout, the automatic backups, and the fact that migrations only run forwards, so users know to copy the settings folder before downgrading to a version that cannot read the per-group files. * [Client] Match the desktop file name case for the portal app ID QGuiApplication::setDesktopFileName() was given "Cockatrice"/"Oracle" while the installed desktop entries are cockatrice.desktop and oracle.desktop. Linux paths are case-sensitive, so xdg-desktop-portal failed the app-info lookup for "Cockatrice" and Qt warned on every QDesktopServices call that it could not register with the host portal. The same string becomes the xdg-shell appId, so the window icon lookup failed on Wayland for the same reason. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-03 14:59:58 +02:00
BruebachL	b8b81170bf	[Client] Show card names in the game log and [card] tags in the chosen card language (#7387) ... The game log translates its sentences, zone names and counter names, but every card name was rendered as the raw canonical name straight from the server. The card board and the card info text already went through CardLocalization::displayName(), so the log was the odd one out. cardLink() used one string for both the card:// href and the visible label, which left no room for a localized label. Split the two: the href keeps the canonical name because ChatView::mousePressEvent feeds it straight to showCardInfoPopup for lookup, while the label goes through the display language. Same split for [card] chat tags in ChatView::appendCardTag. Sanitize the name while building the link. logUndoDraw already did this by hand and had drifted into its own non-italic copy of cardLink(); route it back through the helper. Add CardLocalization::displayNameFor() for callers that only hold a canonical name and need no CardInfoPtr of their own. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-03 13:47:07 +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	4ce2e8c3d3	[UserList] Fix join/spectate from the profile hover card doing nothing (#7383) ... The games list in the profile hover card offered "Join game" and "Spectate", but UserInfoPopup::joinGameRequested was only forwarded to an identical UserListWidget::joinGameRequested signal that nothing connected to. Clicking either entry sent no command, opened no tab and showed no error. Route the request to TabSupervisor, which owns the join flow. The popup's games list spans every room the user is in, so the target room may not be open yet; IntentJoinServerGame already joins the room first and retries, which is why this goes through the intent rather than straight to GameSelector. Extract the intent construction that joinReportGame already had into a shared joinGameByIntent helper instead of duplicating it. Also stop disabling "Join game" for games that are full or already running. GameSelector::joinGame() offers to join as a spectator in that case, so a greyed-out entry read as a dead button; GameSelector's own context menu never disables it either. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-02 18:57:07 +02:00
BruebachL	750d5b3c21	[Client] Add option to use game time in game log timestamps (#7265) ... * [Client] Add option to use game time in game log timestamps (#2202) Game log timestamps showed the local clock time, which is meaningless in a replay or in a log read after the fact — nobody can tell when a play happened relative to the game. Add a "Use game time instead of local time in game logs" setting. When enabled, the message log stamps entries with the elapsed game time (HH:MM:SS since the game started); regular chat views keep the local clock. * [Game] Move formatElapsedTime helper out of public slots (#2202) The message log needs the HH:MM:SS formatter used by the game timer, but it lives in GameState's public slots, tying a pure formatting helper to the signal-slot machinery. Move formatElapsedTime into a public section of GameState so the log can call it directly, and add the missing trailing newline to game_state.cpp. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-02 17:30:48 +02:00