691 commits

Author	SHA1	Message	Date
Lukas Brübach	a94db4171b	[DeckSync] Address tombstone review: remote deletes, revival and purge clock ... - Treat a record whose server row is gone as a remote delete only when the local file still matches the sidecar's base hash. An edit made after the remote delete is newer than the delete, so it uploads and revives the row instead of being removed. - Revive a tombstone with the same user and guid in cmdDeckUpload instead of inserting a second row next to it, so the deck keeps its id and version history and the promise in servatrice.ini.example holds. - Run tombstone purging on its own clock, driven by the new [deck_sync] purge_interval option, so disabling share cleanup no longer disables purging silently. - Write the base snapshot through QSaveFile and log a failed commit, so a crash or a full disk cannot leave a truncated base for DeckDiff::merge().	2026-11-05 12:51:05 +01:00
Lukas Brübach	4e5c4403b2	[Server+Client] Tombstone deck deletes with purge and base snapshots	2026-11-05 12:51:05 +01:00
Lukas Brübach	34e22df5ba	[DeckSync] Match server rows by identity and hash the whole deck ... Sync keyed local files to server rows by path, but the server names a row after the deck, not after the file, so any deck whose file name and deck name differ uploaded again on every run. A second pass then brought the server copy back as an extra local file. - Match by the sidecar's row id, then guid, then content, and only then by the server path, so renaming either side no longer duplicates a deck. - Key the server tree by row id: the server allows several rows with the same name in one folder, and keying by path silently dropped all but one. - Stop discarding rows whose name contains '/' ("U/W Control"). They stay matchable, and only the download file name is sanitised. - Build the download path from the folder plus a sanitised row name, including every folder segment, so a hostile server name cannot escape the deck folder. - Download a colliding server row under a unique name and count it, instead of skipping it without a trace. - Hash the whole deck instead of the card hash: SHA-1 over the native deck string covers name, printings, banner, tokens and sideboard plans. The client and the server hash the exact bytes they exchange, so both agree. Adds deck_content_hash_test for the client/server hashing invariant and for the name-only change the card hash cannot see.	2026-11-05 12:51:01 +01:00
Lukas Brübach	11fdfb4610	[Server] Persist and serve deck sync fields + version rows	2026-11-01 22:10:53 +01:00
Lukas Brübach	67a36b9828	[Server] Deck sync identity: schema v41 + proto guid/hash fields ... Bumps the schema to v41: master is already at v40, so the deck-sync migration is servatrice_0040_to_0041.sql.	2026-11-01 22:10:37 +01:00
BruebachL	00151380e3	[CardDB] Break the CardInfo <-> CardSet cycle on reload (#7645) ... * [CardDB] Drop the CardInfo self pointer The weak self pointer had to be assigned right after every construction, and a missed assignment silently stopped change notifications. CardInfo is always owned by a QSharedPointer, so it now derives from QEnableSharedFromThis<CardInfo> and asks for sharedFromThis() instead: nothing to initialize, and the CardSet cycle stays broken. CardSet::newInstance() also loses the commented-out self-pointer assignment left over from the same design. * [CardDB] Pin the cardInfoChanged contract without the self pointer Dropping CardInfo's self pointer moved change notifications onto QEnableSharedFromThis, so the signal now rests on the card being owned by a QSharedPointer. Two tests pin that down: - A property change on a card owned by a CardDatabase still emits cardInfoChanged() carrying that card, which is what the card database views and models listen to. - clone() starts out unowned, so a change to the clone is reported as the clone's and must not reach the original's observers. * [CardDB] Treat an unowned CardInfo as a bug, not a no-op Every CardInfo is built inside a QSharedPointer (both factory overloads and clone()), and no constructor notifies anyone, so sharedFromThis() is always available by the time addToSet() or a setter runs. Assert that and warn in a released build, instead of silently skipping the notification or the set append. The copy constructor keeps its explicit QEnableSharedFromThis initializer: GCC's -Wextra warns about the uninitialized base in every translation unit that copies a CardInfo otherwise. A comment there records that the clone deliberately starts unowned. * [CardDB] Break the CardInfo <-> CardSet cycle on reload CardSet is a QList<CardInfoPtr> and CardInfo::addToSet() appends the card to its sets, while a card keeps a CardSetPtr per printing in setsToPrintings. The sets therefore hold strong references back to the cards, and the previous database snapshot stayed alive after a reload, so RSS grew by roughly a second database on every reload. Empty the sets before the cards are dropped: CardDatabase::clear() and swapInDatabaseData() clear the sets before replacing the containers, and removeCard() detaches the card from its sets, so nothing keeps a card alive on behalf of a set once the database lets go of it. Two tests pin the behaviour: a reload and a clear() must drop the last strong reference to a card from the previous snapshot. * [CardDB] Keep the set detach in one place clear() and swapInDatabaseData() both emptied every set before dropping the cards, so share that as detachSetMembership() instead of two copies that have to stay in sync. removeCard() now detaches through the first printing of each set directly, the same way CardInfo::newInstance() appends, instead of an inner loop that always breaks after the first item. Because clear() empties the sets up front, its per-card detach was walking every card's printings to remove it from sets that were already empty. Split removeCard() into the per-card detach and unindexCard(), and let clear() call only the latter. * [CardDB] Test that removeCard detaches the card from its set removeCard() is the one teardown path a caller can trigger for a single card (the token editor uses it), and it now detaches the card from its sets as well. Without that detach the set's strong CardInfoPtr keeps the removed card alive forever, which is exactly what the reload and clear tests pin for the whole-snapshot paths. The test loads the fixture, removes Cat, and requires the card to be gone from the CAT set and to be destroyed once the test drops its own reference. * [Servatrice] Drop the settings cache from the logger #7602 removed #include "settingscache.h" from server_logger.cpp but left isLoggingEnabled() reading settingsCache, so master no longer compiles: the servatrice target fails on the undeclared global and server_logger_test, which compiles the same file without main.cpp, fails to link it. The same commit made reloadConfig() the single place that stores writelog and the log filters, precisely so logging threads stop reading the global QSettings. isLoggingEnabled() now mirrors logMessage() and reads those members instead: the console is a sink of its own, and the logfile needs an open file plus writelog. The mutex is mutable so the const query can take it. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-31 19:37:15 +01:00
BruebachL	344dbd2caa	[Servatrice] Fix silently broken socket error connections (#7626) ... * [Servatrice] Fix silently broken socket error connections QAbstractSocket::error() was removed in Qt 6 in favour of errorOccurred(). The ISL interface connected to the old name through SIGNAL(), which compiles but fails at runtime, so catchSocketError never ran. The WebSocket interface used the same name on QWebSocket, where it still exists but is deprecated since 6.5. Use errorOccurred for both. The WebSocket connect needs a lambda because catchSocketError is a protected base-class slot and a base pointer-to-member is not accessible from the derived class. * [Servatrice] Guard the WebSocket error connect against Qt 6.4 QWebSocket gained errorOccurred in 6.5 and dropped the error signal with it, so the typed connect cannot compile against the 6.4 floor that Debian 12 and Ubuntu 24.04 still use. Keep the typed connect on 6.5+ and fall back to the old-style connect below it, which is what the code did before. Verified both branches compile: forcing the guard to the else path and rebuilding serversocketinterface.cpp succeeds, so the 6.4 spelling is intact. * [Servatrice] Initialize ISL serverId before authentication catchSocketError() removes islInterfaces[serverId], but the incoming connection constructor left serverId uninitialized until initServer() set it after the SSL handshake and certificate check. A socket error during waitForEncrypted() therefore read an indeterminate key. * [Servatrice] Correct the QWebSocket error signal note error() was deprecated in 6.5, not removed; errorOccurred() was added in 6.5. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-31 19:02:55 +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	ae43659b91	[Build] Use versionless Qt CMake commands (#7639) ... * [Build] Use versionless Qt CMake commands Replace the versioned qt6_add_executable, qt6_add_resources, qt6_add_shaders,\nqt6_add_translations and qt6_wrap_cpp calls with their unversioned forms.\nAll five are documented as available at the project's 6.4 minimum and accept\nthe same arguments.\n\nVerification: full client, Oracle, server and test build passed. * versionless qt_finalize_target and _project --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de> Co-authored-by: tooomm <tooomm@users.noreply.github.com>	2026-10-31 18:53:00 +01:00
BruebachL	93309e01a2	[Server/Client/Protocol] Let moderators clear banner cards (#7546) ... Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-25 13:19:20 +01:00
BruebachL	b4e24f958a	[Network] Do not format discarded debug messages, parse without memmove (#7606) ... * [Network] Do not format discarded debug messages, parse without memmove Protocol debug logging gets the full protobuf message rendered through TextFormat and reflection. On the server this happened before every session/room/game/moderator/admin/developer command, even though the default Server_ProtocolHandler::logDebugMessage() is a no-op and even when servatrice has no log file configured. On the clients the same string was built as an argument to qCDebug(), which evaluates its arguments regardless of whether the category is enabled. Server_ProtocolHandler now exposes a virtual logsDebugMessages() hook. The base implementation returns false, so handlers without a log destination spend nothing. AbstractServerSocketInterface overrides it with ServerLogger::isLoggingEnabled(), which checks the log file and the server/writelog setting that logMessage() itself consults. All debug message call sites check the hook before formatting. The client send/receive paths check QLoggingCategory::isDebugEnabled() first, the same pattern AbstractClient already uses for its slow-command warning. ISL message decoding does the same. Byte stream decoding called inputBuffer.remove(0, n) for the length header and then again for every message. Each call shifts all remaining bytes, so a read carrying k messages cost O(k * bytes remaining). The three receive loops (remote client, TCP server interface, ISL interface) now parse at a read offset and compact the consumed prefix once, when they stop because a full message is not buffered yet. No buffering or size-limit behavior changes. Also in this batch: - Server_ProtocolHandler::processGameCommandContainer() copied the whole games map under gameListMutex just to look up one game id. A new getGameInfo() performs the single lookup and returns the room/player ids. - The TCP receive loop's `ok` flag is initialized, so a ParseFromArray exception can no longer leave it indeterminate before `if (ok)`. - The exception log prints only the message being parsed instead of hex-dumping the entire pending buffer. Full build and ctest (628 tests) pass, including the server tests. * [Network] Address review comments - Move getGameInfo() out of the header into server_abstractuserinterface.cpp. - Drop the consumed prefix once after the parse loop instead of on every early return: the two "not enough data" exits now break out of the loop. Applied to the remote client, TCP server and ISL receive loops. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-25 12:49:24 +01: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	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	155f7b0143	[Servatrice] Relay room channel messages over ISL (#7480) ... * [Servatrice] Relay room channel messages over ISL * [Servatrice] Skip audit log for unknown ISL channel per review - externalRoomSay drops messages for channels not in the local catalog, matching Server_Room::say, instead of logging what was never delivered --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-16 22:46:40 +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	05dcde69db	[Server] Gate room channels on privilege levels (#7477) ... * [Server] Gate room channels on privilege levels * [Server] Filter channel catalog per user, split read/write gating, add banners - Server_Room::getInfo() now takes the requesting user and only lists channels that user may read. Previously every room member received the full catalog (e.g. the moderators channel) while message delivery was already gated, so restricted channels were visible but silent. - Split channel access into mayReadChannel/mayWriteChannel: history replay, catalog listing, and event fan-out use the read level (access_level); Command_RoomChannelSay uses the write level (write_access_level, falling back to the read level when unset). - Plumb write_access_level and banner_message through both room sources (SQL rooms_channels table and config-file channels via writeaccess and banner keys). - Schema 38 -> 39: add write_access_level (NULL means inherit the read level) and banner_message columns; also widen access_level to the full six-value privilege enum to match the 37 -> 38 migration. * Fixup: evaluate channel access per user flag instead of ladder User flags are independent bits: IsDeveloper does not imply IsModerator/IsAdmin, so developer-only users must not reach moderator/admin channels. Replace the userPrivilegeLevel ladder with hasChannelPrivilege: moderator channels need IsModerator (admins hold it alongside IsAdmin), admin channels need IsAdmin, developer channels need IsDeveloper or IsAdmin, judge channels admit IsJudge/IsModerator. Covers catalog filtering, history replay, write gating and fan-out, and corrects the ini example and proto docs. * [Server] Make channel privilege helpers static per review * [Server] Fail closed on unknown channel access levels per review - roomChannelAccessFromKey returns nullopt for unknown keywords instead of silently falling back to Public - skip channels with unknown read/write levels with qWarning, empty read defaults to Public, empty write means no override - mayWriteChannel now also requires read access so write-below-read configs cannot post into invisible channels * [Server] Remove stray conflict marker in channel replay comment * [Server] Fix servatrice.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:38 +02:00
BruebachL	17d2e8d86e	[Server] Add room chat channels with per-channel fan-out and history (#7476) ... * [Server] Add room chat channels with per-channel fan-out and history * Fixup: redact all channel histories on message removal removeSaidMessages only redacted the main history, so banned/warned content stayed visible to late joiners on every other channel. Drop the unused channelId parameter and redact up to amount messages per channel across the whole channelChatHistory map. * [Server] Validate room channel ids and fix catalog schema per review - servatrice.sql: expand rooms_channels access_level enum to full set to match 0037->0038 migration - servatrice.cpp: skip empty, reserved (__main__) and duplicate channel ids when loading from ini and DB with qWarning - server_protocolhandler.cpp: clarify catalog replay comment (filtering happens in moderator stack) - server_room: drop duplicate channelIndex, linear search preserves catalog order * [Server] Fix server_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:38 +02:00
BruebachL	b11b096828	[Server] Apply card art rules to playmats (#7532) ... * [Server] Apply card art rules to playmats Card art DENY rules were only enforced for user card art (SetCardArtParams) while playmats using the same art bypassed them. Move the rule lookup to Servatrice_DatabaseInterface::isCardArtAllowed with a default-allow base on Server_DatabaseInterface, delegate the session check to it, reject DENY-listed Command_SetPlaymat with RespFunctionNotAllowed, and strip DENY-listed deck playmats on DeckSelect so banned art is never broadcast. * [Server] Address review nits on playmat card art rules --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-16 19:50:43 +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	8968709642	[SMTP] Add reliable email delivery with retry and database-backed queue (#7240) ... * [SMTP] Add reliable email delivery with retry and database-backed queue Rework the SMTP client from fire-and-forget sending into a persistent queue with exponential backoff and per-message retry budget, backed by the existing database rows as the durable source of truth. - Keep pending emails in an in-memory queue with attempts/backoff tracking; enqueue without deleting database rows, and only remove the row once delivery is confirmed via the mailDelivered signal. - Add exponential backoff (doubling, capped at 30 min) between delivery attempts, bounded by new smtp/maxretries and smtp/retrydelay settings. - Track connection-layer failures (connect/auth/encrypt) with their own backoff; route failures to recordFailed/recordConnectionFailure so undeliverable mail eventually drops from the queue instead of retrying forever. - Treat rejected recipients/senders as permanent failures. - Rebuild the SMTP buffer from the authoritative queue before each send via a new QxtSmtp::reset() so reconnects never duplicate deliveries. - Start the idle timeout after authentication, not on TCP connect, so slow SSL/STARTTLS handshakes are not cut short. * [SMTP] Fix retry accounting, backoff reset, and token dedupe --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 20:59:32 +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	bdda3dedfa	[CI] Publish a gcovr line-coverage report from the test run (#7404) ... * [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] 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. * [CI] Publish a gcovr line-coverage report from the test run Sanitizers tell us what a run did wrong; they say nothing about what it never reached. This adds the other half: an instrumented build whose report says which of our own lines no test executes. Add an ENABLE_COVERAGE option that compiles with --coverage -O0 -g, mirroring the ENABLE_SANITIZERS opt-in including the per-source carve-out that keeps vendored trees out of the denominator. -O0 is deliberate: at higher optimisation levels GCC merges and relocates lines, so the report would describe code that no longer exists in that form. Only GCC is supported, since Clang's intermediate format needs llvm-cov; asking for coverage under Clang is a hard error rather than a build that silently produces unreadable .gcno files. Keep the filters in a committed gcovr.cfg rather than in the invocation. The tests themselves are excluded because they are compiled with the same instrumentation and are only ever executed by the run doing the measuring, so leaving them in would report a healthier project than we have. The process entry points and the client's window chrome go too: each needs a display, an interactive terminal or a live database before it does anything a headless test could observe, so they would otherwise sit at 0% forever and bury real movement. Give coverage its own matrix leg rather than a second flag on the sanitizer leg. The two answer different questions, and combining them slows both down without telling us more. The new variant key gives the leg its own Docker image and ccache entry so the two legs cannot race on the same tag. The leg publishes an HTML report with per-file pages, LCOV, Cobertura XML and a JSON summary. gcovr is a report-time dependency rather than a build dependency, so a missing one warns and skips instead of failing an otherwise green build. * [CI] Resolve the coverage source root before the build dir cd The previous commit computed SRC_ROOT after `cd "$BUILD_DIR"`, under a comment claiming it ran before. `$0` is relative to the directory the script was invoked from, so from inside the build dir `dirname "$0"` no longer resolves and the subshell's `cd` fails. Under `set -e` that is fatal rather than an empty variable, which took down every leg in the matrix, coverage or not, including the ones this change does not otherwise touch. Move it above the cd and guard it on MAKE_COVERAGE, since the coverage report is the only consumer. Resolving it unconditionally meant a coverage-only detail was paid for by every build in the fleet, and gave every one of them a way to fail. --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-11 15:05:28 +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	d0fc27e3a8	[Security] Harden network input handling and websocket proxying (#7076) ... - TCP client connections: reject messages whose declared length is negative or exceeds a 1.5MB cap immediately instead of buffering indefinitely, preventing unbounded memory growth and the previously stuck state caused by negative lengths overflowing the int header. - ISL connections: apply the same length validation with a 4MB cap, disconnecting the peer on an implausible length. - Websockets: only trust the forwarded client-address header (e.g. X-Forwarded-For) when the websocket connection originates from an address in security/trusted_sources. Previously any client could spoof its address to bypass bans, user limits and rate limiting. Took 3 minutes Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-10-08 23:18:47 +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	2632f8a040	[Client/Server] Keep non-deck files out of the deck storage (#7352) ... The local deck and replay browsers list every file in the folder, so a stray file (e.g. a PNG screenshot of a deck) can be parsed as a garbage plaintext deck, opened in the editor, and uploaded. The server also accepts oversized deck payloads with only silent truncation. Hide files that are not in a supported deck/replay format in the two QFileSystemModel views (directories stay visible), skip them when opening, reject them when uploading, and make the server reject deck uploads larger than MAX_FILE_LENGTH instead of truncating them. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-27 21:32:49 +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	073ec29c4d	[Server] Add deck share links and public deck visibility (#7241) ... * [Server] Add deck share links and public deck visibility * Address server review comments for deck share links * Document transaction teardown in deck share rollback paths * Address second round of deck share review comments --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-20 20:22:16 +02:00
BruebachL	a5e94d8a4f	[Build] Keep Windows installs free of build-tree artifacts (#7316) ... * [Build] Keep Windows installs free of build-tree artifacts Several Windows packaging gaps could leak Visual Studio CMake build output into the installed application or the NSIS installer: - The per-app DLL sweep used ${CMAKE_BINARY_DIR}/${PROJECT_NAME}/${CMAKE_BUILD_TYPE}, which is empty on multi-config generators, collapsing the recursive DIRECTORY install into the whole build tree (containing *.dir, *_autogen, .qt, .qsb, x64, ...). Point it at the real per-config output with $<TARGET_FILE_DIR:...> and exclude build artifacts. - install(FILES ${OPENSSL_INCLUDE_DIRS} ...) tried to install OpenSSL include directories as files. CMake refuses this ("install FILES given directory"); it only slipped through CI because the vcpkg OpenSSL config leaves the variable empty. Remove it; fixup_bundle already ships the OpenSSL runtime DLLs. - The NSIS uninstaller only deleted *.exe/*.dll and a few known files, so build-tree leftovers survived an uninstall/reinstall cycle. Wipe the whole directory tree instead. - Add a Windows CI gate that lists the packaged installer with 7-Zip and fails the build if any build-tree artifact path is found. * Update .ci/compile.sh Co-authored-by: tooomm <tooomm@users.noreply.github.com> * [Build] Rework Windows installer artifact exclusions per review --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de> Co-authored-by: tooomm <tooomm@users.noreply.github.com>	2026-09-20 20:21:52 +02:00
BruebachL	e11c915a0c	[Servatrice] Detect MySQL strict mode on startup and exit early (#7251) ... * [Servatrice] Detect MySQL strict mode on startup and exit early * Update servatrice/src/servatrice_database_interface.cpp Co-authored-by: tooomm <tooomm@users.noreply.github.com> * [Servatrice] Treat failed strict-mode check as boot error --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de> Co-authored-by: tooomm <tooomm@users.noreply.github.com>	2026-09-18 15:02:45 +02:00
tooomm	5d025ca0bd	Use capitalized app names (#7255) ... * use capitalized app name * update urls * app description * Update main.cpp	2026-09-12 17:30:46 +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	14ecfff700	[Build] Add precompiled headers for Qt-backed executables (#7235) ... * [Build] Add precompiled headers for Qt-backed executables Reparsing QtCore/QtGui/QtWidgets/QtNetwork in ~460 client translation units is the dominant compilation cost. Precompile the two common layers: - qtcore_pch.h (Qt Core only; safe even for headless Servatrice) - qtwidgets_pch.h (adds Gui/Widgets/Network; used by Cockatrice and Oracle) target_precompile_headers() requires CMake 3.16, now the project minimum. Estimated 30-50% faster client rebuilds. * [Build] Format qtwidgets precompiled header clang-format include regrouping and a missing trailing newline. * [Build] Add PCH-aware ccache sloppiness config; format cmake/pch headers --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-05 19:40:36 +02:00
BruebachL	d6fbfb32a1	[Security] Use a CSPRNG for salts, tokens, and RNG seeding (#7192) ... * [Security] Use a CSPRNG for salts, tokens, and RNG seeding Password salts and activation tokens were generated with the global SFMT RNG, which was seeded from a 32-bit timestamp, making registration salts and activation tokens predictable. The game RNG used the same timestamp seed across restarts. Add CryptoUtil backed by OpenSSL RAND_bytes and use it for salt/token generation and to seed RNG_SFMT with a 64-bit CSPRNG value in both the client and server. Link libcockatrice_utility against OpenSSL::Crypto. Took 30 seconds Took 25 minutes * Lint. Took 4 minutes Took 36 seconds --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-09-04 13:49:18 +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	d93f63050c	[Server] Fix unauthenticated crash via replay submit code (#7072) ... cmdReplaySubmitCode dereferenced userInfo without an authentication guard, allowing an unauthenticated connection with a valid replay code to segfault the server. Add the same authState != PasswordRight guard used by all other replay handlers, and gate session command dispatch on a pre-auth whitelist so future handlers cannot be reached before login. Took 2 minutes Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-08-14 08:56:54 +02:00
kongwu	83920a4ae9	[Servatrice] Notify newly logged-in users of pending server shutdown (#6976)	2026-08-11 20:33:41 +02:00
BruebachL	c4316c40e9	[CI] Remove Qt5 (#7071) ... * [CI] Remove Qt5 Took 10 minutes Took 9 minutes * Revert CI failure and fix up comments Took 4 minutes --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-08-08 22:04:50 +02:00
BruebachL	27fb5e51de	[Security] Redact sensitive user data from client-visible responses (#7077) ... The getUserInfo command for a user that is not currently online returned the full database record, including the account id, the email address and the stored client id, to any logged-in requester. Mirror the redaction already applied to online users via copyUserInfo(): the id and email are only ever exposed to the account owner, and the client id only to moderators. The buddy/ignore add-to-list event likewise returned the target user's email address and client id to the requester. The list entry only needs the public profile fields, so strip the email and client id from it as well. Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-08-08 17:32:39 +02:00
DawnFire42	18b23b19a7	Split trice_limits.h into dedicated headers (#7025) ... * Split trice_limits.h into dedicated headers * Updated docstrings	2026-06-29 14:37:52 -07:00
BruebachL	6dc974a05d	[UserListDelegate] Consider providerId (#7018) ... Co-authored-by: Lukas Brübach <lukas.bruebach@bdosecurity.de>	2026-06-27 11:23:55 -04:00
BruebachL	2914874720	[Room][UserList] Introduce style delegate (#6981) ... * [Room] Additionally show a tab for friends and ignored users instead of just all online users. Took 21 minutes Took 12 minutes * [Room][UserList] Introduce style delegate for user list - Allow users to set a card name and parameters as their background banner - Allow mods to white/blacklist cards - Allow toggling back to the old display style Took 7 minutes Took 28 seconds Took 2 minutes Took 2 minutes * Right checkstate. Took 14 minutes Took 2 minutes * Utility for test. Took 9 minutes Took 8 seconds Took 2 seconds * Lint. Took 10 minutes * Algorithm for sql schema migration Took 13 minutes * Use {prefix}, bound card name, return errors. Took 27 seconds * Convert queue to while loop. Took 19 seconds * Hover popup. Took 36 minutes Took 1 minute * More granular signals, popup for user info. Took 25 minutes Took 8 seconds Took 16 minutes --------- Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>	2026-06-26 20:52:24 -04:00
Zach H	80426d77bc	Add Discord account registration monitor (#7013) ... A read-only cron script that posts new Servatrice account registrations to a Discord channel via webhook. Dedups by an auto-increment id high-water-mark (single-integer state, no duplicates, nothing missed across downtime). Reads DB credentials and the webhook from a servatrice-style ini via --config.	2026-06-21 01:09:57 -04:00
DawnFire42	33e0f8699b	Standardize Doxygen documentation (#6885)	2026-05-21 22:58:07 +02:00
DawnFire42	aadee34238	style: Add braces to all control flow statements (#6887) ... * style: Add braces to all control flow statements Standardize code style by adding explicit braces to all single-statement control flow blocks (if, else, for, while) across the entire codebase. Also documents the InsertBraces clang-format option (requires v15+) for future automated enforcement. * InsertBraces-check-enabled	2026-05-16 19:19:53 +02:00
ebbit1q	3ec9ae9772	fix compiling on arch (#6768) ... * fix compiling on arch * redo all the logging in affected files	2026-04-05 21:52:46 +02:00