Commit graph

44 commits

Author SHA1 Message Date
Lukas Brübach
1385b69bac [Fonts] Add AppFont roles and FontConfig fonts.toml round-trip 2026-10-16 09:00:57 +02:00
BruebachL
456d9d9e70
[Server] Inform client of login/registration rate limit timeout (#7468)
Some checks are pending
CodeQL / Analyze (cpp) (push) Waiting to run
CodeQL / Analyze (actions) (push) Waiting to run
Build Desktop / Configure (push) Waiting to run
Build Desktop / Debian 13 (push) Blocked by required conditions
Build Desktop / Debian 12 (push) Blocked by required conditions
Build Desktop / Fedora 44 (push) Blocked by required conditions
Build Desktop / Fedora 43 (push) Blocked by required conditions
Build Desktop / Servatrice_Debian 12 (push) Blocked by required conditions
Build Desktop / Ubuntu 24.04 (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 (push) Blocked by required conditions
Build Desktop / Arch (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 cover (push) Blocked by required conditions
Build Desktop / macOS 13 Intel (push) Blocked by required conditions
Build Desktop / macOS 14 (push) Blocked by required conditions
Build Desktop / macOS 15 (push) Blocked by required conditions
Build Desktop / macOS 26 Debug (push) Blocked by required conditions
Build Desktop / Windows 10 (push) Blocked by required conditions
Build Desktop / Prune caches (push) Waiting to run
Build Docker / Servatrice (arm) (push) Waiting to run
Build Docker / Servatrice (x86) (push) Waiting to run
Build Docker / Publish multi-platform Servatrice image (push) Blocked by required conditions
* [Security] Add per-address rate limiting for auth endpoints

Introduce a thread-safe RateLimiter that tracks attempts per key
(IP address) within a sliding time window, and wire it into the
authentication endpoints:

- Login: failed login attempts from an address are counted; once the
  configured maximum is exceeded within the window, further logins
  from that address are rejected with RespTooManyRequests. A
  successful login clears the failed attempts for that address.
- Registration: implement the previously stubbed
  tooManyRegistrationAttempts, limiting how many accounts can be
  created per address per window.
- Forgot-password: throttle both the email-request and the
  email-challenge paths per address.

New [security] settings with defaults:
  max_login_attempts_per_ip=5 / login_attempt_window_seconds=900
  max_registrations_per_ip=2 / registration_window_seconds=3600
  max_forgot_password_requests_per_ip=3 / forgot_password_window_seconds=3600

Adds unit tests for the RateLimiter (window limit, over-limit
blocking, clearing, per-key independence).

Took 3 minutes

* [Server] Fix rate limiter: disable semantics, draining window, bounded keys, pre-work login gate

- recordAttempt()/isBlocked() return false when maxAttempts <= 0 or windowSeconds <= 0, so the documented 'set to 0 to disable' actually disables instead of blocking every address permanently
- Keys at their limit are rejected without recording, so repeated attempts at the attacker's pace cannot keep sliding the window and holding the lockout open; a block now clears once the recorded attempts age out of windowSeconds
- Attempts map is bounded: an opportunistic prune (every 60s) erases keys whose attempts have fully aged out
- Login is throttled before the work: isLoginRateLimited() (a new virtual on Server, backed by isBlocked()) gates loginUser(), so a locked-out address no longer burns a database round trip and password verification per attempt
- recordAttemptAt()/isBlockedAt() time seams make the sliding-window behaviour deterministically testable; tests rewritten for the real semantics (no reliance on the maxAttempts=0 bug, no sleeps)

* [Server] Inform client of login/registration rate limit timeout

- Add retryAfterSeconds/retryAfterSecondsAt to RateLimiter to compute
  seconds until oldest attempt exits the sliding window
- Expose loginRateLimitRemainingSeconds via Server/Servatrice
- Include denied_end_time (absolute epoch) in RespTooManyRequests for
  login pre-auth and when transitioning to lockout after failed attempt
- Include denied_end_time in registration rate limit response
- Client: show retry time for login RespTooManyRequests
- Client: simplify registration RespTooManyRequests message to show
  retry time when provided
- Add test for retryAfterSecondsAt

* [Client] Restore registration guidance and drop duplicated rate limit reason

The registration RespTooManyRequests case now only shows the retry-after
copy when the server actually sent a denied_end_time; the per-email account
cap path (no end time) falls back to the previous guidance about already
having an account and contacting the server operator.

Drop the canned denied_reason_str from the new login rate limit branches so
the client no longer prints the same sentence twice.

* [Server] Exempt trusted_sources from auth rate limits

Addresses in security/trusted_sources bypass per-address limits, matching
the existing max_users_per_address bypass in initSession. Without this,
Webatrice behind local nginx (all clients as 127.0.0.1) would lock out
every web user after 5 mistyped passwords, and cap the whole server at
2 registrations / 3 forgot-password requests per window.

Adds Servatrice::isRateLimitExempt helper; recordFailedLogin,
isLoginRateLimited, recordRegistrationAttempt and
recordForgotPasswordAttempt all check it.

# Conflicts:
#	servatrice/src/servatrice.cpp

* [Server] Key login rate limit by (address, username)

A successful login cleared the whole per-IP failure history, so 4 bad
guesses plus 1 self-login (or guest UnknownUser login) from the same
address bypassed the limiter. Failures are now bucketed per
(address, lowercase username) and a success clears only its own pair.

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
2026-10-16 05:47:43 +02:00
BruebachL
cce606764c
[Server] Resolve a zone dump request to an in-range index span (#7401)
Some checks are pending
CodeQL / Analyze (cpp) (push) Waiting to run
CodeQL / Analyze (actions) (push) Waiting to run
Build Desktop / Configure (push) Waiting to run
Build Desktop / Debian 13 (push) Blocked by required conditions
Build Desktop / Debian 12 (push) Blocked by required conditions
Build Desktop / Fedora 44 (push) Blocked by required conditions
Build Desktop / Fedora 43 (push) Blocked by required conditions
Build Desktop / Servatrice_Debian 12 (push) Blocked by required conditions
Build Desktop / Ubuntu 24.04 (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 (push) Blocked by required conditions
Build Desktop / Arch (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 cover (push) Blocked by required conditions
Build Desktop / macOS 13 Intel (push) Blocked by required conditions
Build Desktop / macOS 14 (push) Blocked by required conditions
Build Desktop / macOS 15 (push) Blocked by required conditions
Build Desktop / macOS 26 Debug (push) Blocked by required conditions
Build Desktop / Windows 10 (push) Blocked by required conditions
Build Docker / Servatrice (arm) (push) Waiting to run
Build Docker / Servatrice (x86) (push) Waiting to run
Build Docker / Publish multi-platform Servatrice image (push) Blocked by required conditions
Command_DumpZone.number_cards is an unbounded sint32 supplied by the client, and
cmdDumpZone used it as an index without ever putting it in range.

    for (int i = 0; (i < cards.size()) && (i < numberCards || numberCards == -1); ++i) {
        const auto &findId = cmd.is_reversed() ? cards.size() - numberCards + i : i;
        Server_Card *card = cards[findId];

A forward request happened to be safe, because the loop stopped at the zone's own size.
A reversed one was not. number_cards is documented by the client as -1 for "the whole
zone", and cards.size() - numberCards is therefore cardCount + 1, one past the last card.
A request larger than the zone puts the offset at a negative index instead. Both read
out of bounds, then dereference the result as a Server_Card, so either adjacent heap
memory or a crash.

Any player can reach it: the zone only has to be a public one, or the requester's own,
and both number_cards and is_reversed come straight off the wire. A reversed dump of -1
is enough on any public zone with cards in it.

Resolve the request to a half-open span up front. -1 and any positive count are clamped
to what the zone actually holds, any other negative count resolves to nothing, and a
reversed request takes the span from the end rather than computing an offset from a value
that may not be in range. cards.at() then carries the bound as a second line of defence,
and qsizetype keeps the loop and the list's own indexing type aligned.

Also report card_count as the zone's real size. It was
numberCards < cards.size() ? cards.size() : numberCards, which agrees with the zone size
for every request that was in range and reported the client's own oversized number
otherwise. The client uses card_count to draw placeholders when a zone lists no cards, so
overstating it invents cards that were never there.

Put the resolution in its own translation unit. server_abstract_player.cpp refers to the
RNG global that only servatrice's main.cpp defines, so anything linking that object needs
the whole server binary behind it, which is why no test could reach this code before.

server_dump_zone_test covers each request shape and then asserts the span is contained in
[0, cardCount) across the edges of the request domain, INT_MIN and INT_MAX included.
Reverting the arithmetic makes three of them fail, the two out-of-bounds shapes and the
containment sweep.

The client's own copy of this convention at view_zone.cpp:93 already clamps the way this
now does. Leaving that duplication for a follow-up, since the client reaching the same
answer by a different route is not a defect.

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
2026-10-13 11:23:13 +02:00
BruebachL
4a010bd17f
[Report] Add report reopen action for closed reports (#7442)
- New Command_ReportReopen moderator command (extension 1206) with server
  handler that reopens resolved/dismissed reports, resets resolution state
  and notified, and records the reopening note in the audit log
- Reopen... button in the report queue action bar, gated to resolved and
  dismissed reports, with optional note prompt and tooltip
- Success confirmations for Assign, Resolve/Dismiss and Reopen now survive
  the list refresh via a pending status message instead of being clobbered
- Dispatch/auth coverage in server_moderator_role_test

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
2026-10-11 15:28:45 +02:00
BruebachL
8822ade49d
[Tests] Register each test case with CTest instead of each test binary (#7399)
* [Tests] Register each test case with CTest instead of each test binary

Registering a test cost four edits across a dozen CMakeLists files: add_test,
add_executable, a conditional add_dependencies on gtest, and target_link_libraries.
The conditional branch was the trap -- it is only needed when GTest was fetched and
add_subdirectory'd, and omitting it yields undefined gtest symbols at link time --
so it was easy to leave out and impossible to notice until the link failed.

Fold all four steps into cockatrice_add_test() and migrate every test directory
onto it, with a cockatrice_add_manual_test() wrapper for the three benchmarks that
are built but deliberately kept out of ctest. The calls now read as a declaration
of what a test is rather than a transcript of how to register one.

While rewriting the registrations, replace add_test(NAME x COMMAND x) with
gtest_discover_tests(). Naming the binary as a single CTest entry meant a CI failure
reported only which binary failed, and ctest -R Suite.Case could not select an
individual case. Discovery raises the suite from 35 opaque entries to 469 real ones:
ctest -R ThemeMigrationTest.IsIdempotent now runs exactly that case, and
ctest -L server / -LE bench work off labels applied at registration.

Give every test a 60s TIMEOUT. CTest's implicit default is 1500s, so one hung
socket wait would stall CI for 25 minutes, and single_instance_manager_test already
does real QLocalSocket waits.

Dropping per-case granularity costs wall-clock time, since the suite becomes
process-spawn bound rather than CPU bound: serial ctest went from 3.8s to 30.8s. Run
it with -j in CI to recover it -- 7.6s at -j4, and 3.5s at -j8, both at or below
the old serial time. The binaries are parallel-safe: the only test claiming a fixed
resource scopes its local socket name to its own pid, so concurrent copies cannot
collide.

Also delete dummy_test, which asserted 1 == 1 and only proved the harness was
wired up -- something the other 468 cases already establish.

No test is added, removed, or changed by this commit; the 469 cases are the same
ones the 35 binaries contained.

* [Tests] Make QtTest available to test targets

QtTest was not among the modules test targets could link, so the Qt model and
signal contracts had no way to be verified from a test: QAbstractItemModelTester
and QSignalSpy both live there.

Add Test to _TEST_NEEDED in cmake/FindQtRuntime.cmake. That is the single place the
union of test-only Qt modules is declared, so this is the only file that needs to
change to make the module reachable from every test target.

Nothing uses it yet. This lands the capability ahead of the model-layer tests that
need it, so those can be added as tests alone rather than as test plus build-system
plumbing.

* [Tests] Correct the note on the oracle test compression defines

add_definitions() populates the directory's COMPILE_DEFINITIONS property, which is
consumed at generate time, so it reaches every target in the directory no matter
where it is called relative to add_executable(). The previous note claimed the
flags stayed confined to the benchmark because the find_package block came after
it; oracle_importer_test, created above that block, carries both defines too.

Verified harmless rather than a live bug: HAS_ZLIB and HAS_LZMA gate code only in
oracle/src/pages.cpp and oracle/src/pagetemplates.cpp, and no test target compiles
either. The ordering and the leaked defines are both pre-existing, so this only
corrects the explanation.

* [Tests] Carry one CTest label per test instead of several

gtest_discover_tests() can hand CTest exactly one label. GoogleTestAddTests.cmake
accumulates its generated script with `foreach(arg ${ARGN})`, so a label list reaches
the CTestTestfile as bare `LABELS ci server`, and CTest reads only the first value of a
property and drops the rest without a word.

The `ci` label that is appended to every registered test was therefore absent from every
test that also asked for `server` or `bench`: `ctest -L ci` reported 400 of 469 cases,
and `ctest --print-labels` never listed a second label anywhere in the build.

Reduce each test to exactly one of `ci`, `server`, or `bench`, which is all the
selections need. The three now partition the suite exactly, 400 + 64 + 5 = 469, and
`ctest -LE bench` still means everything but benchmarks. A second label is now a
configure-time FATAL_ERROR rather than a line that quietly means something else.

Anything orthogonal to a test's kind belongs in a helper flag beside the label, so it
composes with the group instead of competing for the single slot.

Also raise DISCOVERY_TIMEOUT off GoogleTestAddTests' 5s default. A plain debug binary
answers --gtest_list_tests in 0.04s, but that budget is thin once several binaries
discover concurrently behind a -j build, and being over it fails at the *link* step with
a bare "Process terminated due to timeout" that points nowhere near the cause.

* [Tests] Link gtest_main on the fetched-gtest path

The downloaded-gtest branch set GTEST_BOTH_LIBRARIES to `gtest` alone, while
FindGTest sets it to both halves. Every test target links that list rather than
picking an entry point, so on this path nothing provides main() and any test
source that does not define its own fails at link with `undefined reference to
main` pointing at crt1.o, naming neither gtest nor the line responsible.

That stayed latent while every test carried its own four-line main(). A test that
omits it now fails, and the error points at the C runtime rather than at gtest,
which is a long way from the cause. Restoring both halves makes the two paths
agree and drops the boilerplate requirement on new test sources.

The 18 sources that still define main() keep linking: gtest_main.cc defines
nothing but main, so the linker leaves its object in the archive unused rather
than reporting a duplicate.

* [Build] Run the test suite under ASan and UBSan (#7400)

* [Build] Add an ENABLE_SANITIZERS option for ASan and UBSan

Adds -DENABLE_SANITIZERS, off by default, building with AddressSanitizer and
UndefinedBehaviorSanitizer. The bulk of the work is in the three ways the obvious
implementation silently does the wrong thing.

Probe the flags on the link step, not just the compile step. check_cxx_compiler_flag()
only compiles, so every sanitizer flag comes back unsupported, because linking an ASan
object without -fsanitize=address leaves __asan_init undefined. The result is not a
build failure but a build that appears instrumented and is not, which is worse. Use
check_cxx_source_compiles() with CMAKE_REQUIRED_LINK_OPTIONS instead, and treat an
unsupported sanitizer pair as fatal since the cosmetic flags are allowed to degrade.

Opt vendored sources out per source, through a cockatrice_disable_sanitizers_for()
helper rather than a list of set_source_files_properties() calls at the root. The
helper has to exist because CMake resolves a source-file property against the directory
scope its target was declared in: setting the property from the root with absolute paths
is a no-op, silently, which is why the call sites sit next to the add_library and
add_executable that pull the vendored code in. SFMT, minizip, the LZMA SDK and Qxt SMTP
are excluded. oracle/src/qt-json is in the history but not in the tree, removed in
#7214.

Also mark those sources SKIP_PRECOMPILE_HEADERS. The PCH is built with the sanitizer
flags, so it defines __SANITIZE_ADDRESS__; a translation unit compiled with
-fno-sanitize=all does not and GCC then refuses the PCH, which is a fatal
-Werror=invalid-pch in the servatrice and oracle builds.

-fno-sanitize=vptr is not optional. Qt constructs QObject subclasses into raw storage
and writes the vptr afterwards, so the vptr check reports on essentially every QObject
instantiation, and it has no value here because nothing outside moc-generated code
inherits QObject.

Verified on the resulting build: our own translation units carry ASan and UBSan calls,
SFMT.o carries neither, and the full suite of 464 cases passes clean under
ASAN_OPTIONS=detect_leaks=0:abort_on_error=1 with UBSAN_OPTIONS=halt_on_error=0.

The test suite needs two adjustments to make this option usable at all, both of which
land here because they are consequences of COCKATRICE_SANITIZERS existing rather than
independent changes.

DISCOVERY_TIMEOUT rises from 60s to 300s under sanitizers. An instrumented binary takes
2.8s to answer --gtest_list_tests against 0.04s uninstrumented, and discovery runs
several binaries at once behind a -j build.

One target is excluded via a NO_SANITIZE flag, server_developer_role_test. Reaching
google::protobuf::TextFormat::Printer's destructor aborts on an absl assertion inside
system protobuf 33.4 whenever the consumer translation unit is built without NDEBUG,
which a Debug sanitizer build is. This reproduces with no Cockatrice code at all, just
"TextFormat::Printer p; p.PrintToString(msg, &out);" built with -fsanitize=address, so it
is upstream rather than ours. It is a flag and not a fourth label because "skip this in
the sanitizer leg" has to compose with a test's group rather than compete for the single
label slot, and because a flag excludes by not registering the cases at all, which
cannot rot the way a label silently can. The target still builds and still runs on the
legs without sanitizers.

* [CI] Run the suite once under ASan and UBSan

Adds --sanitize to .ci/compile.sh, mirroring --test, and turns it on for one entry of
the existing Linux Docker matrix: Ubuntu 26.04, which already had a test-enabled leg.
That reuses the image, the ccache layer and the existing step shape, so the CI diff is a
matrix key plus the argument plumbing. Debian 13 and Fedora 44 keep covering the
uninstrumented suite, so nothing is lost by instrumenting one leg.

Sanitizers stay off the release and package legs, which build with --release for
packaging: instrumenting those would change the shipped binaries' optimization profile
and build time for no benefit. macOS and Windows stay uninstrumented too, since ASan is
unreliable against the Qt build there and MSVC has no equivalent without /fsanitize=address.

--sanitize does not imply --test, so an instrumented build can be produced locally
without paying for ctest. When both are given, ctest runs with detect_leaks=0 because Qt
and the sanitizer runtimes both leak at exit by design and LeakSanitizer's default buries
real findings under hundreds of process-teardown reports, and with UBSan halt_on_error=0 so
one run collects many findings instead of stopping at the first. ASan keeps
abort_on_error=1, since a bad access leaves the process unusable and continuing only
produces a second, more confusing report from the wreckage.

* [Build] Fix sanitizer flag leak into release package and make UBSan fail CI

- compile.sh always passes -DENABLE_SANITIZERS explicitly so the
  release-package reconfigure in the same build dir cannot inherit ON
  from the sanitized debug step (which shipped libasan/libubsan DEB deps).
- Sanitizers.cmake resets COCKATRICE_SANITIZERS to OFF when the option is
  off (CACHE INTERNAL otherwise sticks forever, keeping NO_SANITIZE skips
  and the 300s discovery timeout).
- UBSan now fails the job: -fno-sanitize-recover=undefined at compile time
  plus UBSAN_OPTIONS halt_on_error=1 (recover mode exits 0, so the old
  halt_on_error=0 could never fail).

---------

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

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
2026-10-11 14:50:09 +02:00
BruebachL
a44757bd11
[Client] Improve card database update visibility (#7458)
* [Client] Improve card database update visibility

Addresses Cockatrice/Cockatrice#7431.

Cockatrice previously offered no in-client feedback while the card
database updater ran: the only progress UI lived in the first-run
wizard, stderr/stdout were not parsed in interactive runs, and starting
a second update with Oracle already open or with a hidden background run
active just showed a confusing 'already running' guard - even though
Oracle is launched as a separate process with no lifecycle tracking.

- Add a status-bar widget (CardDatabaseUpdateStatusBar) that appears
  while an update runs and shows the Downloading/Parsing/Importing
  stages with percent progress; it is also used in the quit-confirm
  prompt so users see an unfinished update before cancelling it.
- Parse the updater's PROGRESS lines (new CardUpdateProgressParser,
  unit-tested) in both foreground and background runs and log stage
  changes to the new card_database.update category, throttled to at
  most one line per second; forward Oracle stderr to the logs.
- Show a tray balloon when a background update starts and on finish or
  failure instead of staying silent.
- Request a running Oracle instance to raise itself (QLocalServer
  CockatriceOracleRaise-<user>) when the user starts another update, and
  demote the 'already running' guard to an informative message only when
  no running instance can be found.
- Emit download/scan/import progress unconditionally from Oracle so
  interactive and background runs behave the same.
- Document the new log category in the troubleshooting guide.

* [Client] Address card database update review feedback

* [Client] Add setting to control card database update tray notifications

* [Client] Address remaining card database update review nits

- Rename card_update_progress_parser.{h,cpp} to card_update_progress.{h,cpp}
- Store the CardUpdateProgress struct in the status bar widget

* Lint.

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
2026-10-09 11:07:48 +02:00
BruebachL
0934380a5c
[Translations] Load Qt's own translations and build ours with the binary (#7384)
* [Translations] Load Qt's own translations and build ours with the binary

Three separate bugs kept translations from working.

installNewTranslator() loaded the application translation into
qtTranslator. QTranslator discards its previous contents on every load, so
that wiped the qt_<lang>.qm loaded one line earlier and installed an empty
translator instead. Qt's own strings were never translated on any platform.
Give each file its own translator.

QLibraryInfo::path(TranslationsPath) was the only directory searched for
Qt's translations. A relocated Qt can report a path that does not exist: the
KDE Flatpak runtime reports /usr/translations while its files live in
/usr/share/qt6/translations. Probe the usual install layouts and only hand
back the ones that exist.

The .qm files were attached to the default build target only, so building
the executable on its own left it without any translations and the language
selector empty. Attach them to the target as well.

Move the shared lookup into TranslationLoader so client and Oracle cannot
drift again, and have it report a missing translation at debug level. An
unset language is the default for English, which has no file to begin with,
and warning about it only added noise to every fresh debug log.

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

* [Tests] Define main() in translation_loader_test

Without a GTest installation the fallback in tests/CMakeLists.txt links
only gtest, not gtest_main, so the test has to bring its own main(). Every
other test already does; only this one relied on the distribution package
and failed to link on CI.

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

* [Tests] Put the translation loader fixture next to the test executable

A multi-config generator (MSBuild, Xcode) writes the executable to a
per-config subdirectory while the .qm stays in the current binary
directory, so applicationDirPath() never saw the fixture and
ABuildTreeFindsTheTranslationsNextToTheExecutable failed on Windows.

* [Translations] Depend on Qt's lrelease target and skip the loader test without lrelease

Attaching the .qm custom commands to a second custom target races with
Qt's own <Project>_lrelease target on MSBuild/Makefiles and is rejected
by Xcode, so depend on Qt's target instead. Only build the translation
loader test when Qt6::lrelease is available.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
Co-authored-by: ZeldaZach <zahalpern+github@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-03 15:04:30 +02:00
BruebachL
014983b5bc
[Theme] Carry Default theme directory customizations over to System (#7385)
* [Theme] Carry Default theme directory customizations over to System

The Default -> System rename rewrote the stored theme name but left the
user profile's themes/Default directory behind, so anyone who had
customized zone graphics, stylesheets or palettes there silently lost them.

Migrate the directory alongside the name. Rename it when System does not
exist yet, otherwise merge file by file without overwriting what System
already has, then drop the legacy directory. Users who already ran the
rename get their customizations back on the next start, and a hand-made
Default folder of a user on another theme is left untouched.

* [Tests] Give theme_migration_test an explicit main and gate it on WITH_CLIENT

The build machines that cannot find GTest fall back to the downloaded
googletest, which links the bare gtest library without gtest_main. Every
other test defines its own main() for that reason; this one did not, so
Debian 13, Fedora 44 and Ubuntu 26.04 failed to link it.

The test compiles client sources, so -DTEST=1 -DWITH_CLIENT=0 has no way
to build it. Gate the subdirectory on WITH_CLIENT like the other
client-source test.

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
2026-10-03 13:43:25 +02:00
BruebachL
add65caae5
[Client] Hand off single-instance payloads outside the readyRead signal (#7373)
Some checks are pending
CodeQL / Analyze (cpp) (push) Waiting to run
CodeQL / Analyze (actions) (push) Waiting to run
Build Desktop / Configure (push) Waiting to run
Build Desktop / Debian 13 (push) Blocked by required conditions
Build Desktop / Debian 12 (push) Blocked by required conditions
Build Desktop / Fedora 44 (push) Blocked by required conditions
Build Desktop / Fedora 43 (push) Blocked by required conditions
Build Desktop / Servatrice_Debian 12 (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 (push) Blocked by required conditions
Build Desktop / Ubuntu 24.04 (push) Blocked by required conditions
Build Desktop / Arch (push) Blocked by required conditions
Build Desktop / macOS 13 Intel (push) Blocked by required conditions
Build Desktop / macOS 14 (push) Blocked by required conditions
Build Desktop / macOS 15 (push) Blocked by required conditions
Build Desktop / macOS 26 Debug (push) Blocked by required conditions
Build Desktop / Windows 10 (push) Blocked by required conditions
Build Docker / Servatrice (arm) (push) Waiting to run
Build Docker / Servatrice (x86) (push) Waiting to run
Build Docker / Publish multi-platform Servatrice image (push) Blocked by required conditions
* [Client] Hand off single-instance payloads outside the readyRead signal

Opening a cockatrice:// link from outside a running client showed the
"Join game ...?" confirmation and then crashed on both answers. The
join reached the server first, so the other players saw the user join
and immediately disconnect.

SingleInstanceManager::handleNewConnection() emitted filesReceived from
inside the forwarding socket's readyRead lambda and only disconnected
afterwards. A joingame link makes the handler show a modal QMessageBox,
which runs a nested event loop. The sending instance exits as soon as
it has the acknowledgment, so the socket was disconnected and
deleteLater'd inside that loop: it died in the middle of its own
readyRead signal, and both this lambda and Qt's signal dispatch kept
using the freed memory.

Disconnect right after the acknowledgment and emit filesReceived
through a queued call, so no handler and no nested event loop ever runs
inside the socket's signal. Guarding the disconnect call alone is not
enough, since Qt still touches the socket after the lambda returns.

Add a regression test that reproduces the nested-event-loop hand-off.

* [Tests] Apply cmake-format to the single-instance test wiring

* [Client] Defer the single-instance socket teardown past the readyRead frame

The forwarded payload is emitted synchronously from inside the forwarding
socket's readyRead, which is what lets the modal "Join game?" confirmation
appear. A handler can spin a nested event loop, and the sending instance
exits as soon as it has the ACK, so the socket is disconnected and
deleteLater'd while that loop runs. Control then returns to this frame with
the socket already destroyed, and the inline disconnectFromServer() touched
freed memory.

Queue the teardown instead and guard it with a QPointer taken before the
handlers run -- building the guard afterwards is itself a use-after-free,
since QPointer dereferences the pointer it is constructed from. The payload
still reaches the handlers synchronously, so the confirmation is unchanged.

The regression test covers both halves: the handlers must run while the
socket is still connected, and the socket must not be torn down until the
readyRead frame has returned.

* Lint.

* [Client] Only delete the single-instance socket outside the readyRead frame

The auto-delete on `disconnected` was the remaining use-after-free. A
handler may spin a nested event loop (the modal "Join game?" box), the
sending instance exits as soon as it has the ACK, and so the peer closes
while that loop runs. deleteLater() issued from inside such a loop is not
deferred to the outer loop: Qt frees the object as soon as the nested loop
unwinds, which is still inside this readyRead emission, so Qt's own signal
dispatch and the handler lambda both keep using a freed socket. When the
notification happens to arrive late the bug hides, which is why this only
reproduced on some platforms.

Drop the auto-delete and let the queued teardown own the socket's lifetime:
it disconnects and then deletes, once the frame has unwound. The QPointer
guard was necessary but not sufficient -- it stopped us dereferencing a
dead pointer, not the socket being freed underneath the frame.

The test waits on the socket's destroyed signal instead of polling, so the
teardown assertion does not depend on platform event timing.

* [Client] Do not queue the single-instance socket teardown

The previous commit moved the deleteLater() off the disconnected signal
and into a queued invocation, believing that "queued" meant "after the
readyRead frame returns". It does not. A queued call is delivered by
whichever event loop is running, and the modal "Join game?" box runs a
nested one -- so the teardown was still executed inside the readyRead
emission, which is exactly the use-after-free it claimed to prevent.

Queueing was never necessary. Because nothing connects deleted/close to
the socket's disconnected signal any more, no nested loop can free it
while the handlers run, and emit filesReceived() is synchronous: once it
returns, the frame has unwound and the socket can be closed and deleted
inline. The QPointer stays, covering the one case that remains -- the
primary shutting down while the confirmation box is open, since
QLocalServer destroys its pending connections.

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
2026-10-02 21:19:02 +02:00
BruebachL
eae9591a17
[Server/Client] Unify report categories (#7345)
* [Server/Client] Unify report categories into one shared source of truth

* Lont.

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
2026-09-24 18:37:54 +02:00
BruebachL
e01912d3c8
[Tests] Add manual picture loader benchmark against the real card hosts (#7288)
* tests: add manual picture loader benchmark against the real card hosts

(cherry picked from commit 588d71d3b9a8278ab98b847802908f322c1035b7)

* tests: address review on picture loader benchmark

- Sandbox via unique app/org names plus Linux-only XDG redirection, derive
  the warm-cache probe and data path from SettingsCache, and bail out when the
  temporary sandbox cannot be created.
- Run each pass with a fresh worker and shut workers down before reading the
  429 counters, so the redirect cache is persisted and no worker thread can
  outlive the stack-local counters or the installed message handler.
- Make s_activeCounters atomic and always forward log output when the previous
  handler is the built-in (nullptr) one.
- Validate --timeout-min, scale the cached-pass budget with --count, honour the
  first --url as the stress template, and add the missing trailing newlines.
- Zero-initialise SettingsCache members so the benchmark mock cannot
  dereference an indeterminate pointer.

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
2026-09-21 18:30:04 +02:00
BruebachL
db2e159dca
[Game] Allow judges to enter any game regardless of restrictions (#7315)
Some checks are pending
CodeQL / Analyze (cpp) (push) Waiting to run
CodeQL / Analyze (actions) (push) Waiting to run
Build Desktop / Configure (push) Waiting to run
Build Desktop / Debian 13 (push) Blocked by required conditions
Build Desktop / Debian 12 (push) Blocked by required conditions
Build Desktop / Fedora 44 (push) Blocked by required conditions
Build Desktop / Fedora 43 (push) Blocked by required conditions
Build Desktop / Servatrice_Debian 12 (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 (push) Blocked by required conditions
Build Desktop / Ubuntu 24.04 (push) Blocked by required conditions
Build Desktop / Arch (push) Blocked by required conditions
Build Desktop / macOS 13 Intel (push) Blocked by required conditions
Build Desktop / macOS 14 (push) Blocked by required conditions
Build Desktop / macOS 15 (push) Blocked by required conditions
Build Desktop / macOS 26 Debug (push) Blocked by required conditions
Build Desktop / Windows 10 (push) Blocked by required conditions
Build Docker / Servatrice (arm) (push) Waiting to run
Build Docker / Servatrice (x86) (push) Waiting to run
Build Docker / Publish multi-platform Servatrice image (push) Blocked by required conditions
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
2026-09-19 10:39:52 +02:00
BruebachL
cca4af0ec7
[PictureLoader] Add local override storage and resolution with matcher tests (#7311)
* [PictureLoader] Add local override storage and resolution with matcher tests

* [PictureLoader] Address review comments

- Make deleteAllLocalOverrides static; it does not touch instance state
- Drop the now-unused hasCustomArt dead code
- Rename the override install methods to installPrintingOverride /
  installPrintingOverrideOnLoad

* [Tests] Give loader matcher tests a writable HOME in CI

Under GitHub's docker runner the process uid has no passwd entry, so HOME
resolves to '/' and the test-mode qttest data dir cannot be created.
SettingsCache's QSettings then drops every write, getPicsPath() comes back
empty, and the loader searches a blank path. Point HOME at a QTemporaryDir
for the duration of the run.

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
2026-09-18 15:03:08 +02:00
BruebachL
202a5ac958
[Client/Server/Protocol] Surface live metrics in the Developer tab (#7212)
* [Server] Instrument command processing, game starts, and event loops

Add a lock-free MetricsRegistry that accumulates per-command processing
times in preallocated histogram slots (one per protobuf command type,
bucketed at 1/5/10/25/50/100/250/500/1000/2500/5000 ms +Inf). The
hot-path observeCommand() uses only relaxed atomic adds — no locks,
no allocations, no cache-line ping-pong beyond the unavoidable counter
updates.

Wire the registry into AbstractServerSocketInterface::processCommandContainer()
so every processed command is attributed with its container's wall-clock
time. When a container exceeds metrics/slow_command_ms (default 500),
a warning is logged including the connected username.

Add an EventLoopWatchdog heartbeat that runs on every socket pool thread.
If a heartbeat overshoots metrics/stall_warn_ms (default 2000 ms), the
overshoot is recorded in atomic counters and a warning is logged. Both
thresholds are configurable in servatrice.ini; setting stall_warn_ms to 0
disables the watchdogs entirely.

Track game-start durations via a separate histogram in MetricsRegistry.
Server_Game::startGameNow() measures the time from zone creation through
player materialization and reports it via Server::observeGameStartDurationMs().

Add a live card-count gauge: Server_Game exposes getCardsInGame() and
Servatrice::getCardsInGamesTotal() sums across all running games under
the appropriate read locks.

Include a standalone metrics_registry_test (Google Test) that validates
empty registries, single/multi-sample histograms, kind encoding,
overflow-slot collapse, negative-duration clamping, gauge rendering,
and the game-start histogram separation.

Took 10 minutes

* [Client/Server/Protocol] Surface live metrics in the Developer tab

Extend Response_GetServerStats with live counters from the in-process
MetricsRegistry: cards in games, event loop stall totals/worst,
total commands processed, average command time, active command types,
and game-start count/duration. Add a repeated CommandStats message
carrying per-command breakdowns (kind, extension number, resolved
protobuf name, count, total ms) for every type that has seen at
least one sample.

Server-side cmdGetServerStats() populates all new fields after the
existing DB uptime snapshot query, resolving protobuf extension names
via the descriptor pool for human-readable labels like
session/Command_Ping.

Expand TabDeveloper with two tables: an overview section (existing
DB stats plus the new live metrics) and a per-command breakdown table
(Command / Count / Total ms / Avg ms) sorted by total_ms descending
so the hottest commands surface first.

Took 55 minutes

Took 47 seconds

* [Server] Drop dead Prometheus histogram, add developer command metrics, fix watchdog init order

- metrics_registry: remove toPrometheusText/appendCumulativeBuckets and the time-bucket histogram that nothing in production ever emitted (the future /metrics exporter can bring it back); keep counts/totals read by the Developer tab
- Fix +Inf bucket routing that never incremented, and its test that locked the bug in
- Instrument developer_command container (kind 6) in processCommandContainer and stats label resolution
- Read metrics/{slow_command_ms,stall_warn_ms} at the top of initServer() so stall_warn_ms=0 disables the watchdogs before pool threads start
- Shrink KindStride to 1280 (largest extension in use is 1206) with a static_assert; document scrape cost of getCardsInGamesTotal; note slow_command logging has no rate limit in servatrice.ini.example

* [Tests] Give metrics_registry_test an explicit main

* [Server] Record only the dispatched command family; drop unused totals

processCommandContainer recorded every family in a container even though
the base if/else-if dispatch processes at most one. An unauthenticated
client could batch a session command (login) with fabricated developer,
moderator, and admin entries and forge genuine-looking samples that were
never executed or authorized. Mirror the base's selection, skip when the
handler was already deleted, and skip entries whose extension number is
-1 (which would otherwise wrap into the previous kind's id range).

[Server] Drop dead process-lifetime byte/uptime counters

txBytesTotal/rxBytesTotal added an atomic RMW to every socket write and
read for counters nothing consumes (cmdGetServerStats fills tx_bytes,
rx_bytes, and uptime_secs from the DB snapshot). Remove the two atomics
and the getTxBytesTotal/getRxBytesTotal/getUptimeSeconds getters; the
incTxBytes/incRxBytes slots and mutexes remain for the ISL legacy
counters.

[Protocol] Document kind 5 as developer in CommandStats

NumKinds is 6 and the server emits kind_index = 5 for developer
commands; the comment stopped at 4.

* [Client] Togglable auto-refresh for Developer stats tab

* [Oracle] Fix clang-format alignment of card type priority list

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
2026-09-11 17:18:56 +02:00
BruebachL
d5d99e4dfb
[Server/Client/Protocol] Add developer staff role (#7211)
* [Server/Client/Protocol] Add developer staff role

Introduce a Developer staff level (proto flag 32, DB admin bit 8) that
sits between admin and moderator: no kick/ban/warn/report/admin powers,
but gets server log access via a new developer command container family
(GET_SERVER_STATS, VIEWLOG_HISTORY) and an idle-timeout exemption.

- Protocol: IsDeveloper flag, developer_commands.proto envelope,
  Command_GetServerStats/Command_GetLogHistory, Response_GetServerStats,
  Command_AdjustMod.should_be_developer
- Servatrice: fail-closed developer dispatcher, uptime snapshot handler,
  shared log history handler reuse, bit-8 DB mapping
- Client: burgundy pawn/badge/labels/sort order, prepareDeveloperCommand,
  minimal Developer stats tab, log tab access, promote/demote actions

Took 24 minutes

Took 18 seconds

* [Server/Client/Protocol] Address developer role review feedback

Address ZeizaZach's review of the developer staff role:

- Nudge the developer log query to exclude private chat and sender IPs
  (the ModeratorCommand path still sees everything).
- Deduplicate Command_GetLogHistory into Command_ViewLogHistory, which now
  extends both ModeratorCommand (ext) and DeveloperCommand (dev_ext); the
  client picks the DeveloperCommand-scoped extension by extendee, and the
  server reads it via the extension number.
- Pull the uptime snapshot SQL into Servatrice_DatabaseInterface as
  getLatestUptimeSnapshot() and widen the reported counters to 64-bit.
- Document the admin bitfield (1 admin, 2 moderator, 4 judge, 8 developer)
  and add a server-side test for the developer command path.

* Add missing trailing newline to user_context_menu.cpp

* Remove stale includes of deleted command_get_log_history proto

The Command_GetLogHistory message was folded into Command_ViewLogHistory,
which deleted command_get_log_history.proto, but serversocketinterface
still #included its generated header. Fresh CI builds fail on the missing
file; local builds masked it by reusing a previously generated header.

* [Server] Exclude chat rows when private-chat filter is bypassable

A developer who omits log_location entirely — or sends only "chat" —
leaves chatType, gameType, roomType all false, so getMessageLogHistory
skips the target_type clause and returns every row, private messages
included. When !allowPrivateChat the server now forces game+room when
no surviving location was requested, guaranteeing the query always
carries a target_type restriction.

[Client] Demote mod+dev to moderator path in log-tab dispatch

The developer command family is strictly weaker than the moderator one
(no private chat, no sender_ip, ip filter ignored), so granting the
developer bit to an existing moderator must not silently strip their
capabilities. useDeveloperCommands is now true only when the user holds
the developer bit and not the moderator bit.

[Client] Hide the IP-address filter for developer log tab users

The developer path ignores the ip_address query field server-side.
Showing the field lets a developer type an IP and get results that are
silently unfiltered by it rather than an empty result set — reads as a
broken filter. Hide labelFindIPAddress/findIPAddress alongside the
privateChat checkbox.

* Developer pawn is silver.

* [Client] Fix indentation of merged Card Art Rules / Developer tabs

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
2026-09-11 17:18:56 +02:00
BruebachL
e8ec28572f
[Models] Mirror custom deck zones in the deck list model (#7203)
* [Models] Mirror custom deck zones in the deck list model

DeckListModel now surfaces the custom zones from the deck tree so
views can render and edit them alongside criteria groups.

The custom-zone bookkeeping that made the model unwieldy is extracted
into DeckListModelCustomZones (deck_list_model_custom_zones.h/.cpp), a
single self-contained unit owning every "what is / where is a custom
zone" decision for the model's shadow tree:

- rebuildTree mirrors each custom zone as a DecklistModelSubZoneNode
  under its board zone, cards flat inside (no further grouping).
- The freshly built shadow tree is sorted while the model reset is
  still open, so views never observe unsorted intermediate order and
  proxies cannot desync.
- Custom zones always sort after criteria groups within a board,
  regardless of their names. One shared sortWithCustomZonesLast backs
  both the live sortHelper (which remaps persistent indexes from the
  movement mapping) and the silent reset-time sortShadowTree.
- addCard inserts flat into a custom zone by name and keeps grouping
  by active criteria for board zones. findCardNode resolves cards in
  both layouts, legacy top-level zones unchanged.
- New IsCustomZoneRole lets views tell zones apart from groups.
- Empty custom zones survive row removal. Zone rows themselves are
  only mutable through the deck tree API.

A new deck_list_model_custom_zones_test suite locks the extracted
shadow-tree logic (type testing, mirroring, name lookup, and the
sort-with-custom-zones-last mapping).

No behavior change.

* [Models] Route group lookups around mirrored custom zones

Group lookups (createNodeIfNeeded, findCardNode) must not resolve a
mirrored custom zone that shares the group name. Introduce
findGroupChild to search only non-custom children, and make addCard
consult the deck tree before falling back to creating a top-level zone
so cards added to an un-mirrored custom zone land inside it.

mirrorCustomZones now flattens cards nested at any depth into the
mirrored zone so no card is left without a model row.

Add model behaviour tests (addCard routing, same-name group/zone
collision, removeRows guard, empty-zone survival, findCard inside a
custom zone) and fix the missing main() in the unit test binaries.

* [Models] Fix addCard routing for card-named zones and nested custom zones

- hasDeckZone no longer matches board cards that merely share the zone
  name, which previously caused infinite addCard/rebuildTree recursion
- Adding to a custom zone whose deck side holds nested sub-zones appends
  to the deck tree instead of writing past its direct children

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
2026-09-05 22:00:20 +02:00
ebbit1q
cf8e5858ab
increase the timeout for the hashing performance test (#7166)
it seems like there is too much variance across platforms for how long this test takes
probably fixes #7152
2026-08-29 15:20:49 +02:00
BruebachL
0a09884c78
[DeckList] Add custom deck zones to the deck tree (#7176)
Some checks failed
CodeQL / Analyze (cpp) (push) Has been cancelled
CodeQL / Analyze (actions) (push) Has been cancelled
Build Desktop / Configure (push) Has been cancelled
Build Docker / Servatrice (arm) (push) Has been cancelled
Build Docker / Servatrice (x86) (push) Has been cancelled
Build Desktop / Debian 13 (push) Has been cancelled
Build Desktop / Debian 12 (push) Has been cancelled
Build Desktop / Fedora 44 (push) Has been cancelled
Build Desktop / Fedora 43 (push) Has been cancelled
Build Desktop / Servatrice_Debian 12 (push) Has been cancelled
Build Desktop / Ubuntu 26.04 (push) Has been cancelled
Build Desktop / Ubuntu 24.04 (push) Has been cancelled
Build Desktop / Arch (push) Has been cancelled
Build Desktop / macOS 13 Intel (push) Has been cancelled
Build Desktop / macOS 14 (push) Has been cancelled
Build Desktop / macOS 15 (push) Has been cancelled
Build Desktop / macOS 26 Debug (push) Has been cancelled
Build Desktop / Windows 10 (push) Has been cancelled
Build Docker / Publish multi-platform Servatrice image (push) Has been cancelled
* [DeckList] Add custom deck zones to the deck tree

Introduce user-definable zones nested under a board zone (main, side
or maybeboard) so players can organize cards inside a board without
changing board semantics.

- addCustomZone, renameCustomZone, moveCustomZone and removeCustomZone
  manage zones. Names are unique across the whole deck and the standard
  zone names (main/side/maybeboard/tokens) stay reserved.
- Board zones are created lazily on first use.
- getZoneObjFromName resolves custom names to their nested node so
  addCard and XML loading route cards into them. Unknown names keep
  creating legacy top-level zones.
- deleteNode keeps empty custom zones alive and only prunes empty
  board zones.
- New deck_list_zones test suite locks hash parity with flat decks,
  sideboard size accounting, maybeboard exclusion from plain export
  and native-format round-trips.

Took 17 minutes


Took 11 minutes

* Extract to function

Took 4 minutes

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
2026-08-26 23:15:16 +02:00
BruebachL
88aa036f7e
[Client] Detect main-thread event loop stalls (#7155)
* [Client] Detect main-thread event loop stalls

LagMonitor ticks the GUI event loop every 500 ms and records gaps
beyond 2 s as stalls, warning with their duration and keeping a
bounded ring of recent records for diagnostics. Measurement uses a
monotonic QElapsedTimer so wall-clock steps and suspend do not
fabricate stalls. Recorded timestamps stay in wall time for
correlating with user reports.

Took 1 minute

Took 13 minutes


Took 2 minutes

* [Client] Rename LagMonitor constants to SCREAMING_SNAKE_CASE

Took 15 minutes

* [Client] Discard suspend-spanning gaps in LagMonitor

Windows counts sleep time in its monotonic clock, so a suspend would
fabricate one bogus stall per resume. Reset the clock on application
state changes and drop implausibly huge gaps; extract recordGap() for
testability.

Took 3 minutes

* [Client] Unit test LagMonitor stall recording

Drives recordGap() directly to cover the threshold, plausibility cap,
trim, and clear behavior without timing-dependent waits.

Took 36 seconds

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
2026-08-23 00:53:40 +02:00
BruebachL
b91e872f5f
[Network] Measure real server round-trip times (#7153)
* [Network] Measure real server round-trip times

Time each command container from send to response with QElapsedTimer,
aggregate samples in a fixed-size ring buffer (last/median/p95/max),
and emit aggregated pingStatsUpdated at most once per second so the
hot path stays free of signal traffic. Stats are cleared on
disconnect. Forward the signal through ConnectionController for UI
consumers. Unit-tested in latency_tracker_test.

Took 37 minutes

Took 4 minutes

# Commit time for manual adjustment:
# Took 8 minutes

* Move params to struct, more informative debug

Took 56 seconds

Took 53 seconds

Took 2 minutes

Took 33 seconds

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
2026-08-23 00:03:52 +02:00
BruebachL
ed4eb1cb31
[Server/Client/Protocol] Reporting users + moderation queue functionality (#7091)
Some checks are pending
Build Desktop / Configure (push) Waiting to run
Build Desktop / Debian 13 (push) Blocked by required conditions
Build Desktop / Debian 12 (push) Blocked by required conditions
Build Desktop / Fedora 44 (push) Blocked by required conditions
Build Desktop / Fedora 43 (push) Blocked by required conditions
Build Desktop / Servatrice_Debian 12 (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 (push) Blocked by required conditions
Build Desktop / Ubuntu 24.04 (push) Blocked by required conditions
Build Desktop / Arch (push) Blocked by required conditions
Build Desktop / macOS 15 (push) Blocked by required conditions
Build Desktop / macOS 13 Intel (push) Blocked by required conditions
Build Desktop / macOS 14 (push) Blocked by required conditions
Build Desktop / macOS 15 Debug (push) Blocked by required conditions
Build Desktop / Windows 10 (push) Blocked by required conditions
Build Docker Image / amd64 & arm64 (push) Waiting to run
* [Server/Client/Protocol] Reporting users + moderation queue functionality

Took 6 minutes

Took 3 minutes

Took 8 seconds

Took 11 minutes

Took 12 minutes

Took 7 minutes

Took 15 seconds

Took 2 minutes

Took 1 minute


Took 30 seconds

Took 16 seconds

* CI Fix

Took 6 minutes

* CI Fix

Took 6 minutes

* [Protocol] Add moderation investigation commands

Adds the protocol layer for the moderation investigation suite:
- Command_GetUserSessions/GetUserAlts/GetModeratorLastLogins/ResetUserPassword/RemoveUserAvatar (1013-1017)
- Response extensions 1215-1219 with ServerInfo messages for sessions, alts, and staff logins
- last_login on Response_ReportUserInfo and warning_il on Response_WarnList

Took 2 minutes

* [Utility] Add warning categories parser with infraction levels

Parses the server's 'officialwarnings' setting (comma-separated, optional
'|IL' suffix) into WarningCategory structs so the client can display the
infraction level of each warning category. Includes GTest coverage.

* [Server] Add moderation investigation tools

Implements the server side of the moderation suite:
- getUserSessions/getUserAlts/getModeratorLastLogins/removeUserAvatar DB methods
- Handlers for all five new commands with audit records (PASSWORD_RESET,
  REMOVE_USER_AVATAR); password resets return a generated temporary password
- cmdGetWarnList now reports per-category infraction levels from the
  officialwarnings setting; cmdReportUserInfo reports last_login
- Update servatrice.ini.example with the warning taxonomy
- Password/avatar mutations report RespNameNotFound when the user does not exist

* [Client] Add moderation tab with investigate, password reset, and avatar removal

- New Moderation tab: search a user to show account info, alternate
  accounts, login sessions, and staff last logins; actions to reset the
  user's password (shows the generated temporary password) and remove the
  user's avatar
- 'Investigate user' entry in the user context menu opens the tab pre-loaded
  for that user
- Warning dialog shows the infraction level of each warning category
- Tab wired into TabSupervisor with a moderator-gated menu action, shortcut,
  and tabs.ini persistence (default closed)

* [Server/Client/Protocol] Address PR #7091 review: security, bug, and perf fixes

Security:
- Promote RESET_USER_PASSWORD to admin-only dispatch (was moderator-accessible)
- Reject password reset on users with equal/higher privilege than caller
- Notify affected user via Event_NotifyUser::CUSTOM when password is reset
- Add server-side category whitelist for reports
- Drop reporter name fallback in comment/details authorization (ID-only)
- Force password change: new DB column + login enforcement + client disconnect

Bugs:
- XSS via QTextEdit::append() → insertPlainText() in report tab and utils
- allNotified initialized to true even with empty recipients list
- Warning combo box: use currentData() instead of baked-in display text
- Report resolution now records who resolved (resolved_by column + audit)

Performance:
- IP-correlation subquery: add 6-month window + LIMIT 200
- getUserSessions: clamp limit to 500

Non-blocking:
- Palette-aware colors in report_utils.cpp (dark/light mode)
- Report list pagination: offset/limit fields + total_count in response
- SessionCommand enum gap comment for reserved values 1201-1203

Schema: 36→37 (force_password_change), 37→38 (resolved_by)

Took 12 minutes


Took 16 seconds

* Fix macOs pedantry

Took 5 minutes

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
2026-08-21 15:00:13 +02:00
BruebachL
9eafd90a91
[Game] Playmats (#7101)
* [Game] Playmats

Took 19 seconds

Took 1 minute

* [Playmats] Add fixed override and configurable fallbacks to settings.

Took 29 minutes

Took 43 seconds

* Add main to test.

Took 1 minute

Took 29 seconds

* Move settings to own group

Took 11 minutes

* Some attempts to refresh macOS compositor

Took 2 minutes

* Try something else

Took 17 minutes

* Don't manipulate live list

Took 11 minutes

* Change things about resolution, address comments.

Took 45 minutes

Took 12 minutes

* Comments.

Took 14 minutes

Took 8 seconds

* Re-order settings menu location

Took 2 minutes

* Rename PlaymatResolution to Info and add enums

Took 8 minutes

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
2026-08-21 10:40:49 +02:00
BruebachL
e9eab328a4
[PictureLoader] Schedule exponential backoff on 429 - Too many request handler and call failed on fail (#7053)
Took 16 minutes

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
2026-08-08 22:55:27 +02:00
BruebachL
12f0f59453
[Settings] Split cache_settings monolith into multiple SettingsManager sub-classes (#7050)
* [Settings] Split cache_settings into multiple files

Took 9 minutes

Took 4 minutes

* [Settings] Fwd declare settings classes in cache_settings

Took 15 minutes

* Fix oracle includes.

Took 8 minutes

* Address comments, fix windows CI

Took 8 minutes

* fix copy constructor visibility

Took 3 minutes

* lint

Took 2 minutes

* Fix native format tests.

Took 5 minutes

* Remove test header guard

Took 4 seconds

* Remove tests invalid in CI environ

Took 24 seconds

* Adjust to rebase.

Took 11 minutes

* Change settings file name.

Took 8 minutes

---------

Co-authored-by: Lukas Brübach <lukas.bruebach@bdosecurity.de>
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
2026-07-27 11:25:39 +02:00
DawnFire42
05ae6f47a6
Unify counter clamp arithmetic into shared addClamped() helper (#7009)
Some checks are pending
Build Desktop / Configure (push) Waiting to run
Build Desktop / Debian 13 (push) Blocked by required conditions
Build Desktop / Debian 12 (push) Blocked by required conditions
Build Desktop / Fedora 44 (push) Blocked by required conditions
Build Desktop / Fedora 43 (push) Blocked by required conditions
Build Desktop / Servatrice_Debian 12 (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 (push) Blocked by required conditions
Build Desktop / Ubuntu 24.04 (push) Blocked by required conditions
Build Desktop / Arch (push) Blocked by required conditions
Build Desktop / macOS 14 (push) Blocked by required conditions
Build Desktop / macOS 15 (push) Blocked by required conditions
Build Desktop / macOS 13 Intel (push) Blocked by required conditions
Build Desktop / macOS 15 Debug (push) Blocked by required conditions
Build Desktop / Windows 10 (push) Blocked by required conditions
Build Docker Image / amd64 & arm64 (push) Waiting to run
* Unify counter clamp arithmetic into shared addClamped() helper

- Add addClamped() in new header clamped_arithmetic.h; uses a 64-bit
  intermediate so the addition cannot overflow int.
- Use it in Server_Card::incrementCounter() (clamps [0, MAX_COUNTERS_ON_CARD])
  and Server_Counter::incrementCount() (clamps [INT_MIN, INT_MAX]), removing
  the duplicated overflow-safe logic and its keep-in-sync TODO.
- Inline incrementCount() into server_counter.h; server_counter.cpp now holds
  only the constructor and getInfo().
- Clarify the card-counter bounds comment in trice_limits.h.

* Rename MAX_COUNTERS_ON_CARD to MAX_COUNTER_VALUE

The constant caps the counter's value, not how many counters can be on the card

* Add direct unit tests for addClamped() helper

* Harden offsetCardCounter() against signed-int overflow

Replace the raw oldValue + offset sum with addClamped(), clamping to [0, MAX_COUNTER_VALUE] without overflow.

* Comment update

* Remove class names from addClamped() docstring
2026-06-28 16:10:57 -07:00
ebbit1q
6be9cec6e2
do not save a const reference to the user data in the info dialog (#6974)
Some checks are pending
Build Desktop / Configure (push) Waiting to run
Build Desktop / Debian 13 (push) Blocked by required conditions
Build Desktop / Debian 12 (push) Blocked by required conditions
Build Desktop / Fedora 44 (push) Blocked by required conditions
Build Desktop / Fedora 43 (push) Blocked by required conditions
Build Desktop / Servatrice_Debian 12 (push) Blocked by required conditions
Build Desktop / Ubuntu 26.04 (push) Blocked by required conditions
Build Desktop / Ubuntu 24.04 (push) Blocked by required conditions
Build Desktop / Arch (push) Blocked by required conditions
Build Desktop / macOS 14 (push) Blocked by required conditions
Build Desktop / macOS 15 (push) Blocked by required conditions
Build Desktop / macOS 13 Intel (push) Blocked by required conditions
Build Desktop / macOS 15 Debug (push) Blocked by required conditions
Build Desktop / Windows 10 (push) Blocked by required conditions
Build Docker Image / amd64 & arm64 (push) Waiting to run
* do not save a const reference to the user data in the info dialog

* cmake format
2026-06-09 23:35:00 -07:00
DawnFire42
8dca14933c
Centralize counter API with server-side bounds and no-op filtering (#6879)
* Refactor server counter API to own overflow protection and filter no-op events

  Counter modifications now clamp to int bounds server-side and return change
  status, allowing command handlers to skip network broadcasts when values
  don't actually change.

* Centralize MAX_COUNTERS_ON_CARD and enforce [0, 999] bounds on server

  - Move MAX_COUNTERS_ON_CARD to trice_limits.h
  - Server clamps values in setCounter() and incrementCounter()
  - Client uses clamped comparison to allow recovery from invalid states
  - Add tests for clamping behavior

* move incrementCount() implementation from header to cpp
2026-05-21 20:39:35 -07:00
Vorliz
a20f3c0fb4
Fix #6659: Correct logging for bottom-of-library card moves (#6764)
Some checks failed
Build Desktop / Configure (push) Has been cancelled
Build Docker Image / amd64 & arm64 (push) Has been cancelled
Build Desktop / Debian 11 (push) Has been cancelled
Build Desktop / Debian 13 (push) Has been cancelled
Build Desktop / Debian 12 (push) Has been cancelled
Build Desktop / Fedora 43 (push) Has been cancelled
Build Desktop / Fedora 42 (push) Has been cancelled
Build Desktop / Servatrice_Debian 11 (push) Has been cancelled
Build Desktop / Ubuntu 24.04 (push) Has been cancelled
Build Desktop / Ubuntu 26.04 (push) Has been cancelled
Build Desktop / Ubuntu 22.04 (push) Has been cancelled
Build Desktop / Arch (push) Has been cancelled
Build Desktop / macOS 14 (push) Has been cancelled
Build Desktop / macOS 15 (push) Has been cancelled
Build Desktop / macOS 13 Intel (push) Has been cancelled
Build Desktop / macOS 15 Debug (push) Has been cancelled
Build Desktop / Windows 10 (push) Has been cancelled
* Fix #6659:  Correct logging for bottom-of-library card moves

Cause:
- This issue happens due to logic of moving the card from the top of the
  deck being reused when moving from the bottom of the deck, in a way
  that makes it impossible to check if the card came from the bottom.

Resolution:
- Updated the logging logic in the client for card moves.
- Added a gRPC parameter ('is_from_bottom') for card moves.
- Updates the server logic to reverse the order of the card move if the
'is_from_bottom' parameter is true.
- Added a test to show the expected behaviour of the fix.

NOTE: While the changes in this patch seem big, this is due to changing
the loop in the moveCard function to a helper function, in order to make
the bug fix change. The only change to the loop was to pass a
variable attribution to the moveCard function because it was redundant
to be in the loop.

* chore: run format on test

* refactor: new way to check if a move is from the bottom of the deck

* refactor: change isFromBottom check to static function

* update comments

Co-authored-by: ebbit1q <ebbit1q@gmail.com>

---------

Co-authored-by: ebbit1q <ebbit1q@gmail.com>
2026-04-21 19:05:31 +02:00
DawnFire42
ce652de272
Fix/cmake test only build (#6709)
* fix(cmake): guard filter_string_test behind WITH_ORACLE or WITH_CLIENT. filter_string_test links against libcockatrice_filters, which is only built when WITH_ORACLE or WITH_CLIENT is enabled. Without this guard, test-only builds fail at configure time because the target doesn't exist. The guard condition mirrors the one in the root CMakeLists.txt that controls whether libcockatrice_filters is built.

* fix(cmake): centralize TEST_QT_MODULES in FindQtRuntime.cmake  Each test CMakeLists.txt was independently defining TEST_QT_MODULES with its own subset of Qt modules. This duplicated knowledge that already lives in FindQtRuntime.cmake (which handles module discovery for all other targets: SERVATRICE, COCKATRICE, ORACLE). Consolidate into a single definition using the union of all test requirements (Concurrent Network Svg Widgets), matching the existing pattern for application-target modules. This ensures test-only builds (-DTEST=ON without application targets) discover all necessary Qt components.

* fix(cmake): guard libcockatrice_network behind application targets. libcockatrice_network is only needed by the client, server, and oracle targets. Other application-specific libraries (settings, models, filters) already have similar guards. This was an oversight that caused test-only builds to fail when network dependencies weren't available.
2026-03-15 17:24:50 +01:00
DawnFire42
9bb399606c
refactor: extract shared card insertion algorithm from hand/stack zones (#6701)
Hand and stack zones had near-identical addCardImpl() implementations, differing only in whether resetState() preserves annotations.
Extract the shared pattern into a template function (CardZoneAlgorithms::addCardToList) to eliminate duplication and enable isolated testing without Qt dependencies.
Pile, table, and zone-view logic are intentionally excluded — their post-add behavior (signals, coordinate placement, hidden cards) is materially different.
2026-03-15 00:39:44 -07:00
ebbit1q
521046fb09
Hashing tests (#5026)
* add deck hashing tests

* format

* fix header

* fix cmakelists

* fix test

* add 5 second timeout to test

let the optimising begin

* expand tests

* remove debug message

* manually format

* I installed cmake format from the aur

* use decklist library

* format
2025-12-23 17:48:10 +01:00
BruebachL
1ef07309d6
Turn Card, Deck_List, Protocol, RNG, Network (Client, Server), Settings and Utility into libraries and remove cockatrice_common. (#6212)
---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
Co-authored-by: ebbit1q <ebbit1q@gmail.com>
2025-10-09 07:36:12 +02:00
RickyRister
3a11ccb854
Update cipt parsing (#5712)
* refactor

* move thing out

* write unit tests

* get thing to work

* optimization?

* fix build failure
2025-03-14 21:44:13 -04:00
ebbit1q
28aa473362
rework formatting with cmake-format (#4627)
* merge clangify and cmakify into format.sh

update desktop lint workflow to 22.04

print cmake-format version as well

um, rename things?

add extra examples to format.sh --help
add option to not run clang-format
fix version display in .ci/lint_cpp.sh
fix relative paths in format.sh

fix formatting dirs

* run ./format.sh --cmake --branch ""

* revert formatting of cmake comments
2022-06-08 21:14:26 -04:00
Zach H
a95b338c80
Add cmake format (#4618)
* Support CMakeify operation

* Run Cmakeify

* Update cmakeify.sh

Co-authored-by: ebbit1q <ebbit1q@gmail.com>
2022-05-08 21:22:43 +02:00
ZeldaZach
b02adccf87 Support Qt6, Min Qt5.8, Fix Win32, Fix Servatrice
Add lock around deleting arrows for commanding cards

Add support for Qt6 w/ Backwards Qt5

Handle Qt5/6 cross compilation better

Last cleanups

caps matter

Fix serv

Prevent crash on 6.3.0 Linux & bump to 5.8 min

Prevent out of bounds indexing

Delete shutdown timer if it exists

Fixup ticket comments, remove unneeded guards

Try to add support for missing OSes

Update .ci/release_template.md

Update PR based on comments

Update XML name after done and remove Hirsute

Address local game crash

Address comments from PR (again)
Tests don't work on mac, will see if a problem on other OSes

make soundengine more consistent across qt versions

disable tests on distros that are covered by others

Fix Oracle Crash due to bad memory access

Update Oracle to use new Qt6 way of adding translations

Add support for Qt5/Qt6 compiling of Cockatrice

Remove unneeded calls to QtMath/cmath/math.h

Update how we handle bitwise comparisons for enums with Tray Icon

Change header guards to not duplicate function

Leave comment & Fix Path for GHA Qt

Update common/server.h

Update cockatrice/src/window_main.cpp

Rollback change on cmake module path for NSIS

check docker image requirements

add size limit to ccache

put variables in quotes

properly set build type on mac

avoid names used in cmake

fix up cmake module path

cmake 3.10 does not recognize prepend

Support Tests in FindQtRuntime

set ccache size on non debug builds as well

immediately return when removing non existing client

handle incTxBytes with a signal instead

don't set common link libraries in cockatrice/CMakeLists.txt

add comments

set macos qt version to 6

Try upgrading XCode versions to latest they can be supported on

Ensure Qt gets linked

add tmate so i can see what's going on

Qt6 points two directories further down than Qt5 with regard to the top lib path, so we need to account for this

Establish Plugins directory for Qt6

Establish TLS plugins for Qt6 services

Minor change for release channel network manager

Let windows build in parallel cores

Wrong symbols

Qt6 patch up for signal

add missing qt6 package on deb builds

boolean expressions are hard

negative indexes should go to the end

Intentionally fail cache

move size checks to individual zone types

Hardcode libs needed for building on Windows, as the regex was annoying

Update wording

use the --parallel option in all builds

clean up the .ci scripts some more

tweak fedora build

add os parameter to compile.sh

I don't really like this but it seems the easiest way
I'd prefer if these types of quirks would live in the main configuration
file, the yml

fixup yml

readd appended cache key to vcpkg step

fix windows 32 quirk

the json hash is already added to the key as well

remove os parameter and clean up ci files

set name_build.sh to output relative paths

set backwards compatible version of xcode and qt on mac

set QTDIR for mac builds on qt5

has no effect for qt6

export BUILD_DIR to name_build.sh

merge mac build steps

merge homebrew steps, set package suffix

link qt5

remove brew link

set qtdir to qt5 only

compile.sh vars need to be empty not 0

fix sets manager search bar on qt 5.12/15

fix oracle subprocess errors being ignored on qt 5

clean up translation loading

move en@source translation file so it will not get included in packages
NOTE: this needs to be done at transifex as well!

Use generator platform over osname

Short circuit if not Win defined
2022-05-06 17:31:08 -04:00
Zach H
d61c604bf4
Address macOS issue where right-clicking a username in the main chat (#4523)
* Address macOS issue where right-clicking a username in the main chat (or game chat) areas would pop up a seemingly empty user profile. This is because the resize event is overridden and doesn't actually attempt to resize based on the size hint of the dialog. Now that we're explicit with the call, this resize should be forced and have comparable results to popping up user profile from the user list.

* use datetime for calculating account age (#4526)

* use datetime for calculating account age

make translating easier by using tr multiples
automatically account for leap days

Co-authored-by: ebbit1q <ebbit1q@gmail.com>
2022-01-16 16:51:13 -05:00
ebbit1q
1e70989f38
add password hash test (#4528)
* clangify tests

* add password hash test

* properly use googletest semantics
2022-01-16 16:32:11 -05:00
ebbit1q
7712862036
add ubuntu 21.10 to ci (#4446)
* add ubuntu 21.10 to ci

remove ubuntu 20.10

* forgot to update dockerfile

* add googletest from repos

* update downloaded gtest

ideally this should just grab the master version

* fix hash

* fix cmake issue
2021-10-30 22:15:50 -04:00
olegshtch
446f9be24d
Fix unresolved symbols when link tests to system libgtest-dev (#4055) 2020-07-30 14:52:44 -04:00
Rob Blanckaert
c9c0fb28ee Counter expressions (#3534)
* Add peglib

* - Add expression engine
- Take an expression when setting a counter

* Shift + Click = Middleclick for counters

* minor cleanup for clangify

Signed-off-by: Zach Halpern <ZaHalpern+github@gmail.com>

* Added tip entry
2019-02-02 13:56:49 -05:00
Mark McDonald
9cbae8c707 Decklist testing (#2537) 2017-04-01 01:24:16 -04:00
Fabio Bas
18993b397b First test for card database: loading and clear 2016-03-21 11:08:14 +01:00
Fabio Bas
7fe60279de Fix PR #1600 2015-10-09 14:59:42 +02:00