CityRP Documentation Repository

Phase one — the library and the two migrations

Last updated

The buildable spec for the first shippable slice of the server-sessions effort. It is the exploit fix, and it must stay shippable alone: phases two and three are severable and nothing here waits on them.

Nothing in this document is a new decision. The reasoning lives in ADR 0001 and in the tickets under #3431; this is the same content arranged for someone about to write the code. Where the tickets left a choice to the spec, the choice is marked Spec's call and carries its reasoning inline.

What ships

  1. gamemode/core/libraries/sv_session.lua — the library.
  2. A server_sessions ledger, with server_stats imported into it and then dropped (currentstats with it).
  3. crash_container moved onto the internal session key, its two inverted column names fixed, and the /c crashed cross-server leak closed.
  4. track rebuilt around session_id, with destruction and refund both deleting the row.
  5. Four cityrpserver table-name keys deleted.

What does not ship, and is not blocked by this: the sv_logs heartbeat and announced flag, giveprops scoping, the three boot-versus-hot-reload sites, phase two's player_sessions, and phase three's lost and found. Each is a consumer of a library that will already exist.

Naming: server_sessions

Spec's call. #3436 left the ledger's table name as <sessions>.

The prose in this effort avoids "server session" — GLOSSARY.md defines the term as session and lists "server session" under Avoid, because it invites reading the process as the unit. The table is nonetheless server_sessions, for two reasons that only apply inside the database: it sits in a schema shared with the website and the forums, where a bare sessions reads as web sessions and is the kind of name someone else's tooling will collide with; and phase two adds player_sessions, against which server_sessions is the pair that tells a reader which is which. The vocabulary rule governs how we write and speak about the concept; the table name is a disambiguator inside a shared namespace.

Schema

CREATE TABLE `server_sessions` (
  `session_id`         int unsigned NOT NULL AUTO_INCREMENT,
  `llserver`           varchar(32) NOT NULL,
  `map`                varchar(64) NULL,
  `token`              char(64) NULL,
  `opened_at`          datetime NOT NULL,
  `closed_at`          datetime NULL,
  `last_heartbeat_at`  datetime NULL,
  PRIMARY KEY (`session_id`),
  UNIQUE KEY `server_sessions_token` (`token`),
  KEY `server_sessions_llserver` (`llserver`, `session_id`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci
  • session_id is int unsigned, never BIGINT and never varchar. tmysql4 marshals INT as a Lua number and BIGINT as a Lua string while lastid is always a number, so BIGINT would be a number on the write path and a string on the read path (#3442). 4.29bn world-loads is not a range worth that asymmetry.
  • llserver holds the server code and map holds the raw map string, as separate columns. server_stats matched map = <LLServer> — a server code compared against a column named map — and that ambiguity does not survive.
  • token is UNIQUE but not NOT NULL: imported and sentinel rows carry NULL, and MySQL permits multiple NULLs in a unique index. token IS NOT NULL is therefore the fence that separates a real post-migration session from a pre-ledger one, and #3436's D1 depends on it.
  • closed_at and last_heartbeat_at are written by the library. Nothing reads last_heartbeat_at back in phase one; it is shipped so that phase two's playtime integrity and any later restart tooling are follow-ups rather than schema changes. Nothing reads closed_at back either — a session's own IsClosing() is in-memory state, not a query.
  • Every foreign key against this table is written ON DELETE RESTRICT ON UPDATE RESTRICT explicitly. The ledger is append-only; a cascade would silently destroy unclaimable player property.

Token generation

The token is minted by MySQL at INSERT, never in Lua: math.randomseed is never called anywhere in this repo, so a Lua draw is both weak and correlated across a fleet booting together, and nothing can authenticate against a session that does not exist yet.

INSERT INTO `server_sessions` (`llserver`, `map`, `token`, `opened_at`)
VALUES (?, ?, SHA2(RANDOM_BYTES(32), 256), NOW())

RANDOM_BYTES() requires MySQL 8.0. Open item, must be settled before implementation: one SELECT VERSION() against Live. On a pre-8.0 server the fallback is SHA2(CONCAT(UUID(), UUID(), UUID()), 256), which is weaker (UUIDv1 is time-and-MAC derived) and is accepted only because phase one asserts nothing about what the token authorises — if the fallback is what ships, the eventual server-to-server auth design must re-mint tokens rather than trust them.

The library

gamemode/core/libraries/sv_session.lua, server-only, loaded by sh_init.lua's includeCore("libraries") pass. Not to be confused with gamemode/core/plugins/referrals/sv_session.lua, which is unrelated.

Lifecycle

  1. File load — cityrp.session = cityrp.session or {}. This guard is the boot predicate: a hot reload finds the table already there, so IsFirstLoad() is false and no state is re-derived (#3432). Follow sh_tophints.lua's x = x or default idiom.
  2. DBConnected — the library's own hook.Add issues the INSERT through DB_MAIN:RunChecked. Not DB_MAIN:Run: Run dispatches its callback even when MySQL errored (db.lua:139-156 has no return between the error branch and the dispatch), which would invert the whole fail-closed premise.
  3. Callback — lastid becomes the internal session key, readiness flips to true, and the queued OnReady callbacks fire in registration order.
  4. ShutDown — a hook.Add (a library cannot own GM:ShutDown) sets the closing flag and writes closed_at. sv_logs.lua:188's existing ShutDown handler returns nothing, so footgun #3 cannot have it suppress ours.

Note that a boot which applies migrations never reaches step 2: Migrations.RunPending issues a changelevel and returns without firing DBConnected (init.lua:114-124). That boot has no ledger row, which is correct — it wiped its own world on the way out.

Surface

Member Unready behaviour Notes
OnReady(name, fn) queues The only sanctioned wait. Fires immediately if already ready. Passes the internal key to fn. name makes it idempotent across hot reloads, exactly as hook.Add's name does (footgun #7).
IsReady() false Mid-flight guards only (Track), never for waiting.
Get() errors The internal session key. Correct code receives it from OnReady.
Is(value) false True when value denotes this session.
Matches(row, field) false field defaults to "session_id". Handles a nil row.
GetStartTime() always Local wall clock, logging only — opened_at is authoritative.
IsClosing() always Teardown flag, set from the library's ShutDown handler.
IsFirstLoad() always Boot versus hot reload. Documented, zero callers in phase one.

Not on the surface: previous(), isCurrent(), a token accessor, and processId. Each was cut under one rule — expose derived state the library already holds; do not derive new state for a hypothetical consumer — and the ADR carries the reasoning.

DBConnected is not a session-readiness signal. It dispatches synchronously and the INSERT's callback lands a frame later, so no DBConnected handler can ever observe a ready session. Say this in the library's own doc comment, because every current consumer is on that hook today.

Is / Matches

Five rules, each of them a way to get this wrong:

  • Is(nil) is false. The tempting tonumber(a) == tonumber(b) gives nil == nil → true, which matches every row with a NULL session — the worst possible answer on a fail-closed path.
  • Is(anything) is false while the library is not ready.
  • Normalise the candidate, not the stored id. The stored key is a number by construction; only the incoming value has unknown provenance.
  • tonumber, not tostring. String comparison makes 1 unequal to "01", and would fail to reject a 64-character token where tonumber correctly returns nil.
  • Matches takes the field name, and is where the nil-row case is handled once. Is(row.session_id) on a nil row hard-errors inside an async callback; Matches(row) is false.

The docs page must state outright that result-set values are compared with cityrp.session.Is, never with a bare ==.

Retry and escalation

  • Five attempts, 2s linear backoff, then cityrp.logging.GetLogger("session"):Critical() (footgun #8) plus a line on the Discord relay channel sv_auto_restart.lua:88 already uses.
  • Readiness is monotonic. Once true it never returns to false: the key is a fact about this world load, not a claim about connectivity, and a flapping predicate makes every consumer's guard racy.
  • No self-bounce. A changelevel does not fix a down database and would loop; the graver condition — a failed migration — already halts rather than restarting; and bouncing would make cityrp.session the arbiter of server availability for a failure it merely noticed first.
  • The server keeps serving while permanently unready. This is a deliberate accepted degradation: contraband spawned in that window is untracked and is lost on the next wipe.
  • Note that DB:Connect never calls its callback on a failed connection, so DBConnected never fires and the retry never runs at all. "Permanently unready" has two shapes and only one of them is retryable; the Critical line should distinguish them.

On build

The library runs normally on build. Excluding it there would reuse "not ready" to mean "deliberately off", and would leave the not-ready path exercised only where its consequences do not matter. refund_ents keeps its LLServer == "build" decision inside Track (sv_init.lua:72) and does not acquire a .meta.lua — it publishes cityrp.contrack to eight call sites outside the plugin, none nil-guarded, so not_servers would make all eight throw.

Migration 1 — the ledger

One numbered migration under gamemode/core/migrations/, taking the next free number.

  1. CREATE TABLE server_sessions (above).
  2. Import server_stats, preserving session_id: INSERT INTO server_sessions (session_id, llserver, opened_at) SELECT session_id, map, startup_time FROM server_stats. Token, map, closed_at and last_heartbeat_at stay NULL — a pre-ledger boot cannot supply them, and NULL reads as "this boot predates the ledger", which is true. A synthesised token would read as a real credential and would be a lie.
  3. DROP TABLE server_stats, DROP TABLE currentstats.

The import is what lets crash_container's existing values keep resolving with no rewrite of that table at all, and it is what stops the ledger's AUTO_INCREMENT eventually colliding with a legacy container_id — a collision that would silently hide one player's container from /c crashed and return it on the next restart.

Ordering hazard: the import must read server_stats before the DROP, in this order, in this migration. Getting that backwards is unrecoverable.

In the same change, delete gamemode/init.lua:2977-2980 — the file-scope timer.Simple(10, ...) that is server_stats' sole writer, and a hot-reload boot-marker advance in its own right.

Migration 2 — crash_container and container_code

crash_container is a live cross-service contract: the GDPR subject-access export (limelight-intern/acp/gdpr/access.php:1769) and the ACP backup manifest both name it. The table name and the owner / json / done columns must not move. Only the two columns nothing outside the repo reads are touched.

Schema:

  • u_key → creation_id, and container_id → session_id. The names are inverted today: the INSERT writes VALUES(GetCreationID(), <session subquery>, ...), so u_key holds the entity creation id and container_id holds the session id.
  • session_id is redeclared int unsigned NOT NULL with a real foreign key to server_sessions(session_id), ON DELETE RESTRICT ON UPDATE RESTRICT. It does not inherit varchar(40).
  • The foreign key lands non-null immediately, because migration 1 preserved every legacy id.

Lua, all in container_code/sv_dbcode.lua:

  • :14 (select previous boots' containers) and :61 (insert tagged with this boot) both want the current boot. The (SELECT session_id FROM server_stats WHERE map = "<LLServer>" ORDER BY session_id DESC LIMIT 1) subquery, its ORDER BY ... LIMIT 1 and its map = LLServer filter all leave; the value is a bound literal from cityrp.session.
  • The /c crashed cross-server leak is closed here. crash_container has no server column and the selection was never scoped — only the exclusion was, via that subquery — so Live rows are visible to /c crashed on beta today. Spec's call: scope by joining the ledger (JOIN server_sessions s ON s.session_id = c.session_id WHERE s.llserver = <LLServer>), not by denormalising a server column onto crash_container. The join is one row per player on a command invocation, server_sessions_llserver covers it, and a denormalised column is a second copy of a fact the ledger already owns — which is the failure mode this whole effort is about.
  • crashproof_containers (the 30s timer at :50) gains a fail-closed early return on not cityrp.session.IsReady(). This is load-bearing, not hygiene: the internal key does not exist until the row lands, and today the 30s timer beats the old 10s insert by coincidence rather than design.

Not fixed here, and not blocked on this spec: #3447 (the commented-out inventory write that still marks the row done, and the unguarded empty result set).

Migration 3 — track and refund_ents

track has no external reader, and flagged has exactly three readers repo-wide, all inside refund_ents. Live holds 12,898 rows, of which 18 are unflagged, across 2 of 356 sessions — the refund path has been effectively dead since 2021, because the CallOnRemove at sv_init.lua:121 almost certainly fires during shutdown teardown and pre-flags everything.

Target schema:

CREATE TABLE `track` (
  `session_id`    int unsigned NOT NULL,
  `ent_index`     int unsigned NOT NULL,
  `owner_steamid` varchar(50) NULL,
  `entity`        varchar(45) NULL,
  `data`          text NULL,
  PRIMARY KEY (`session_id`, `ent_index`),
  KEY `track_owner` (`owner_steamid`, `session_id`),
  CONSTRAINT `track_session_fk` FOREIGN KEY (`session_id`)
    REFERENCES `server_sessions` (`session_id`) ON DELETE RESTRICT ON UPDATE RESTRICT
) ENGINE=InnoDB CHARSET=latin1

server, start_time and flagged are all dropped. The primary key stays a real key rather than gaining a surrogate id: if the delete-before-insert ordering ever breaks, a natural key fails loudly with a duplicate-key error, where a surrogate would let two rows coexist and the delete take both. The integer foreign key never touches latin1, so there is no charset conversion and no reason to rewrite the table's charset.

Migration steps, in order:

  1. DELETE FROM track WHERE flagged = 1 — dead history with no reader, unclaimable under the fence below. This leaves ~18 rows, so the rewrite that follows is instant and needs no online-DDL tooling.
  2. ALTER TABLE track ADD COLUMN session_id int unsigned NULL.
  3. One sentinel session per distinct server value in track: insert a server_sessions row with llserver = that value and token, map, closed_at, last_heartbeat_at all NULL; capture LAST_INSERT_ID(); UPDATE track SET session_id = @sid WHERE server = '<x>'. Capturing the id explicitly rather than joining is what keeps token IS NULL unambiguous between imported and sentinel rows. This step must run after migration 1's import, or the ids collide.
  4. MODIFY session_id ... NOT NULL, drop and rebuild the primary key, drop server / start_time / flagged, add track_owner, add the foreign key.

Spec's call on step 3's data dependence. #3436 flagged the tension with #3450's principle that a migration's value is being a literal, replayable export. Enumerate at run time (SELECT DISTINCT server FROM track), and log each server → session_id pair the migration creates. The principle governs table names, which are static facts about the schema and are in the repo; it does not govern data values, which by definition are not — and hardcoding four server strings makes the migration wrong on any database whose track holds a fifth. up is a callback, not a literal list, so Migrations.RunSequence is simply not the tool for this step.

refund_ents/sv_init.lua changes:

  • Track early-returns while not ready, logging one Error per session on first refusal — not per entity, or a busy server spams the log for hours. No queue: during the transient window no player is connected and Track requires a valid player owner, so zero calls can occur; in the permanent case a queue's only non-empty state is the one where it never flushes.
  • OnDBConnected becomes cityrp.session.OnReady, and its SELECT MAX(start_time) previous-session lookup (:233-246) is deleted outright. There is no previous() and no second notion of "session" left in the file.
  • Destruction deletes the row. The CallOnRemove at :121 deletes rather than flagging, which is also what closes a second live bug: GMod reuses entity index slots, so under the old (server, start_time, ent_index) key the second contraband entity to occupy an index in one session hits a duplicate key, and that entity is silently untracked for the rest of its life.
  • The CallOnRemove asks cityrp.session.IsClosing() rather than keeping a private flag, so any world wipe — crash, clean shutdown, changelevel, map reload — leaves the row claimable and only in-play destruction does not. Per footgun #21 this is library state, not a file-scope local.
  • The refund path deletes too. The batched UPDATE ... SET flagged = 1 WHERE <key list> at :219 becomes a batched DELETE of the same shape. Refunds stay audited by the existing cityrp.logs.addPly(..., "auto-refund", ...) at :225, so nothing is lost.
  • The refund predicate is fenced on the session having a token, which grants zero backlog: 
    SELECT ... FROM track t
  JOIN server_sessions s ON s.session_id = t.session_id
 WHERE t.owner_steamid = ? AND t.session_id != ? AND s.token IS NOT NULL
    Legacy and sentinel rows carry NULL and are excluded by construction — no magic id, no date cutoff. Without the fence, session_id != current alone would dump years of contraband in as a lump grant through Load (sv_player.lua:1630), the very auto-refund path phase three exists to replace. Phase one is still a strict improvement on today: any post-migration session's losses are refunded, not just the last one.

Config keys

Delete four keys from gamemode/core/sv_configuration.lua. The keys go; the tables stay — rppoints and commendations may well be read by the website or the forums, and nothing here touches them.

Key Value Call sites
MySQL Entity Tracker Table track 6, all refund_ents/sv_init.lua (:94, :124, :154, :167, :219, :236)
MySQL Online Log Table onlinelog 6 — onlinelog/sv_init.lua (:43, :55, :62, :67), referrals/sv_init.lua (:160, :189)
MySQL RPP Table rppoints 0
MySQL Commendations Table commendations 0

The %s positional trap: four of the refund_ents sites pass the table name as a positional %s argument to DB_MAIN:FRun. Removing the %s without removing its argument (or the reverse) silently shifts every subsequent argument. They go together.

The two dead keys ship as their own commit inside the phase-one PR, so the record shows them as a ride-along rather than as part of the session decision. The remaining MySQL * Table keys are #3466 and #3467, and neither blocks this.

Verification

Three checks, and the first is a hard gate rather than a nice-to-have.

  1. The teardown-ordering acceptance test — mandatory. Spawn a tracked contraband entity, changelevel, assert the track row survives. GM:ShutDown's ordering against entity teardown is documented nowhere; if it runs after, IsClosing() is a silent no-op and one map change deletes every outstanding row on the server. A hard crash runs no Lua, so this one test covers the entire surface of the assumption. If it fails, the fallback is dropping CallOnRemove for explicit Untrack(ent) calls on the in-play destruction paths — the worse design, which is why it is the fallback. This test also settles the 12,898-versus-18 reading: if the row survives with the guard removed, the shutdown-flags-everything explanation was wrong.
  2. tmysql4 serialises queries per connection. Destruction-deletes-the-row is correct only if the DELETE is issued before the reusing INSERT. DB_MAIN wraps a single connection (db.lua:2) and the outage path replays self.cache in order, so this is expected to hold — but it is unproven, and it is load-bearing.
  3. SELECT VERSION() against Live, before writing migration 1, to settle RANDOM_BYTES() against the UUID() fallback.

Then both diff-scoped gates from AGENTS.md.

Rollout

Migrations are numbered, so the order is enforced by the numbers: ledger, then crash_container, then track. Each is independently replayable and carries its own down.

Applying a migration changelevels the server (init.lua:114-124), so a deploy that lands all three restarts once and comes up on the new schema with a fresh session row. The Lua changes and the migrations ship together; a server running the new Lua against the old schema has no ledger to insert into and would sit permanently unready, which is loud but is a needless outage.

Rollback is per-migration down, in reverse. Note that down on migration 1 cannot restore server_stats' rows once they have been imported and the table dropped — take a backup of server_stats, crash_container and track before the deploy. This is the one irreversible step in the set.

Open items

  • SELECT VERSION() against Live (token generation, above). Blocks migration 1.
  • The teardown-ordering test's result. Blocks marking phase one done, not writing it.

Deliberately not here

The map's Out of scope section is the authority; the ones most likely to be re-proposed while reading this document are the remote restart kill-switch, a query builder, migrating the internal tools off onlinelog, and game.CleanUpMap (an admin-initiated wipe that ends no session, so its rows are deleted as ordinary destructions — a status-quo hold, since today they are equally unrecoverable at flagged = 1).