Commit graph

670 commits

Author SHA1 Message Date
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)
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
- 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)
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
* 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)
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
* [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)
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
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)
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
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)
Some checks failed
Build Desktop / Configure (push) Has been cancelled
Build Docker Image / amd64 & arm64 (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 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
* 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)
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
* [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)
Some checks failed
Build Desktop / Configure (push) Has been cancelled
Build Docker Image / amd64 & arm64 (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 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
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)
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 compiling on arch

* redo all the logging in affected files
2026-04-05 21:52:46 +02:00
ebbit1q
8a126263a9
do not ignore return value of opening log in servatrice when rotating (#6564) 2026-01-24 15:05:26 -05:00
RickyRister
70f9982c29
Bump minimum Qt version from 5.8 to 5.15 (#6442)
* Bump minimum Qt version from 5.8 to 5.15

* remove version check

* remove version checks
2025-12-23 09:58:23 -08:00
tooomm
8485bbe575
Docs: Use Doxygen \todo comments (#6421)
* format todo comments for doxygen

* Mention Todo List & link search
2025-12-15 00:13:16 +01:00
BruebachL
c5fde071e7
[Cleanup] Unused #includes (#6367)
* [Cleanup] Unused #includes

Took 44 minutes

* [Cleanup] More unused #includes

Took 55 minutes

* [Cleanup] Include QSet

Took 4 minutes

* [Cleanup] Include QDebug in deck_list.cpp

Took 3 minutes

* [Cleanup] Include protocol stuff in servatrice_database_interface.h

Took 3 minutes

* [Cleanup] Include QDialogButtonBox

Took 8 minutes

* [Cleanup] Include QUrl

Took 8 minutes

* [Cleanup] Include QTextOption in header.

Took 3 minutes

* [Cleanup] Include QMap in user_list_manager.h

Took 8 minutes

* [Cleanup] Adjust qjson

Took 8 minutes

* [Cleanup] include button box.

Took 3 minutes

* [Cleanup] Redo fwd declarations.

* [Cleanup] Redo last removed fwd declarations.

---------

Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>
2025-11-29 18:53:11 +01:00
BruebachL
8abd04dab1
[Fix-Warnings] Remove more redundant empty declarations. (extra semicolons) (#6374) 2025-11-29 14:19:11 +01:00
Zach H
8f80996515
Strip dashes in Yahoo addresses (#6284) 2025-11-07 13:35:44 -05:00
ebbit1q
d206a70b8a
update format.sh (#6240)
* update format.sh

add shellcheck to format.sh
add statement macros to .clang-format
add no clang format to format.sh
add changed file list to format.sh diff
rename --cf-version to --print-version in format.sh
lint files

* enable --shell on ci runs

* remove useless semicolons

removes the semicolons after empty function definitions
these semicolons are optional, they don't do anything
this will have functions be consistently formatted
if we want to keep the option to have these on the same line like they
were before we should use the option AllowShortFunctionsOnASingleLine: None

* fix script

* update echo line in lint_cpp.sh which doesn't lint cpp only at all
2025-11-07 15:00:39 +01:00
tooomm
aa61032cdf
Delete servatrice/scripts/mk_pypb.sh (#6267)
See https://github.com/Cockatrice/Cockatrice/pull/6212#issuecomment-3447840864
2025-11-03 18:32:35 +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
ebbit1q
f0c3860032
squash #6156 (#6161)
* move common server files

* update includes with move

* create participant, move code

* fix linker errors

* fix regressions

* mark function as override to make clang happy

* split out spectator to new file

* forgot to add to cmakelists

* autocompleter picking wrong casing for var name

* clean up forwards declarations in player

* fix includes in game
2025-09-20 14:37:12 +02:00
BruebachL
2eba126ed7
Modularize and Doxygen decklist.cpp (#6099) 2025-09-04 22:52:46 -04:00
RickyRister
ab4373d025
Implement replay sharing (#6066)
* new protos

* implement commands on server

* add buttons

* icons

* run formatter

* Message on get replay code failure

* Add new commands to switch statement

* Better failure messages

* Fix permission check query

* Change hash method

* Prevent adding duplicate replays

* Clean up TabReplay ui

* Copy over replay name

* base64 encode the hash

* Shorten hash

* Better failure messages

* change icon back to search icon

* check hash before checking if user already has access

* update share icon

* Update label text
2025-08-24 22:40:44 -04:00
HypersonicWalrus
b6e6328e6a
Modified setup_addfirstadmin script to bring it up to date to current cockatrice_users table (#5784)
Co-authored-by: Ben Boelens <bboelens@pop-os.tailc0350.ts.net>
2025-04-03 00:18:31 -04:00
tooomm
1f0846297f
websocket is our default port/connection (#5679) 2025-03-14 21:43:11 -04:00
Zach H
cb060f43b5
Prevent server crash if DB is down and game is attempted to be created (#5600) 2025-02-09 22:11:00 +00:00
danbopes
8cbfe85ed4
Fix db reconnecting issues (#5590)
See:

c1b0d50237 (diff-02a32f437187bd4cbfab74877100fee0cfc669dab2c05418681a3557c2cf73f2R109)

We should be checking to see if the query is notActive. In this case, we're literally closing and reopening the connection to the database every time `checkSql()` is called, which is called in numerous places.
2025-02-09 12:12:16 +00:00
Zach H
f0adafb275
Reconnect Servatrice if DB Connection Dies (#5548) 2025-02-02 18:25:01 +00:00
RickyRister
95cea0f191
Add custom server-side pawn colors (#5543) 2025-02-02 03:25:25 +00:00
Zach H
ec6a23de56
Support more indices (#5505)
* Support more indices

* Support more indices
2025-01-25 04:16:41 +00:00
Zach H
090cc8c144
Support more indices (#5503) 2025-01-20 01:42:24 -05:00
RickyRister
c148c8df7f
replace foreach macro with standard for each loop (#5485) 2025-01-17 05:18:15 +00:00