diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index e3e8014..62bf83f 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -14,3 +14,5 @@ jobs: with: python-version: "3.12" - run: python scripts/validate_b3sty_skill.py + - run: sudo apt-get install -y lua5.4 + - run: python scripts/check_lua_examples.py diff --git a/AGENTS.md b/AGENTS.md index adc2442..2e92da5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -12,6 +12,7 @@ The rest of the repo: - `memory/common/` - learned facts shared by RedM and FiveM. - `memory/fivem/` - FiveM-only learned facts. - `memory/redm/` - RedM-only learned facts. +- `commands/` - Claude Code slash commands (`/b3sty-review`, `/b3sty-new-resource`, `/b3sty-perf`) that drive a task using the rules above. - `references/natives/` - generated native lookup files for FiveM/GTA V and RedM/RDR3. ## Working Rules @@ -31,4 +32,4 @@ The rest of the repo: ## Agent Configs -- `agents/openai.yaml` - OpenAI Codex agent configuration; points Codex to `SKILL.md` and `AGENTS.md`. +- `agents/openai.yaml` - OpenAI Codex skill UI metadata (display name, short description, default prompt); Codex loads the skill itself from `SKILL.md`. diff --git a/README.md b/README.md index b51a467..d23036a 100644 --- a/README.md +++ b/README.md @@ -72,6 +72,7 @@ If this repository is private, the installer needs existing GitHub credentials o - `memory/common/` - learned facts shared by RedM and FiveM. - `memory/fivem/` - FiveM-only learned facts. - `memory/redm/` - RedM-only learned facts. +- `commands/` - Claude Code slash commands that drive a task using the rules (`/b3sty-review`, `/b3sty-new-resource`, `/b3sty-perf`). - `references/natives/` - large generated native reference files. ## Common Skills @@ -85,11 +86,21 @@ If this repository is private, the installer needs existing GitHub credentials o - `skills/common/nui.md` - in-game browser UI (NUI): Lua<->browser bridge, focus, JSON contracts, validation, frontend hygiene, performance, and security. - `skills/common/runtime.md` - threads/waits, the `source` variable, exports and stale references, identifiers, convars, resource lifecycle, yield hazards, and game builds. - `skills/common/security-performance.md` - client-hostile/server-authoritative security: event trust boundary, give-value (give-item/give-money) hardening, ACE permissions, built-in client event exploits, SQL injection, secrets/convars, identifier trust, throttles, persistence, cleanup, and the security review checklist. +- `skills/common/network-performance.md` - net event cost, FXServer rate limits and overflow kicks, latent events, payload shape, snapshot-then-deltas caching, and state bag cost. - `skills/common/database.md` - SQL, OxMySQL/mysql-async, migrations, transactions, and persistence rules. - `skills/common/debugging.md` - reproducible debugging flow for resource, native, NUI, DB, and performance failures. +- `skills/common/frameworks.md` - ESX, QBCore, Qbox, VORP, and RSG integration: player lookup, money/items/jobs, lifecycle events, and the bridge pattern. - `skills/common/ox-lib.md` - ox_lib usage rules when the project already depends on ox_lib or explicitly accepts it. - `skills/common/multi-resource.md` - exports, dependencies, state bags, convars, shared scripts, and cross-resource contracts. +## Commands + +- `commands/b3sty-review.md` - `/b3sty-review`: review a resource against the rules and report verified findings by severity. +- `commands/b3sty-perf.md` - `/b3sty-perf`: CPU and network performance pass with a measured/estimated report. +- `commands/b3sty-new-resource.md` - `/b3sty-new-resource`: scaffold a new resource (manifest, controllers, validated events, cleanup, optional bridge/NUI/DB). + +The Claude Code plugin installs these with the main skill. With a git-clone install, copy `commands/*.md` into `~/.claude/commands/` (or `.claude/commands/` for one project) to use them. + ## Game-Specific Skills - `skills/fivem/rules.md` - FiveM-only defaults and compatibility checks. @@ -100,38 +111,41 @@ If this repository is private, the installer needs existing GitHub credentials o - `references/natives/fivem-gta5-natives.md` - GTA V / FiveM native reference. Formerly `NATIVES_GTA5.md`. - `references/natives/redm-rdr3-natives.md` - RDR3 / RedM native reference. Formerly `REDM_NATIVES.md`. - `references/natives/SOURCES.md` - source attribution and publication notes for generated native references. +- `references/server.cfg.example` - copyable `server.cfg` hardening baseline. + +The native files are generated lookup references and are intentionally kept outside `skills/` and `memory/`. + +## Memory + +- `memory/common/common-errors.md` - common shared errors and fixes. +- `memory/common/native-bugs.md` - known shared native issues and workarounds. +- `memory/common/cfx-patterns.md` - reusable shared FXServer/CfxLua patterns. +- `memory/common/security-performance.md` - learned shared security and performance patterns. +- `memory/fivem/README.md` - FiveM-specific memory namespace. +- `memory/fivem/native-bugs.md` - FiveM-only native issues and workarounds. +- `memory/redm/README.md` - RedM-specific memory namespace. +- `memory/redm/native-bugs.md` - RedM-only native issues and workarounds. -These files are generated lookup references and are intentionally kept outside `skills/` and `memory/`. +`skills/` holds stable rules; `memory/` holds learned facts with date and game build; `references/` holds large generated source material. See `SKILL.md` for which file to open for each task. ## Agent Configs - **Claude Code** - uses `SKILL.md` directly as the skill manifest (frontmatter `name` + `description`); no separate config file. - `.claude-plugin/marketplace.json` - Claude Code plugin marketplace manifest for `/plugin` install. -- `agents/openai.yaml` - OpenAI Codex agent configuration; points Codex to `SKILL.md` and `AGENTS.md`. +- `agents/openai.yaml` - OpenAI Codex skill UI metadata (display name, short description, default prompt); Codex loads the skill itself from `SKILL.md`. - `skills.sh.json` - skills.sh grouping/visibility. ## Maintenance Run the package checks before publishing changes: -```powershell +```bash python scripts/validate_b3sty_skill.py -python C:\Users\b3sty191\.codex\skills\.system\skill-creator\scripts\quick_validate.py . +python /scripts/quick_validate.py . ``` +`` is your local skill-creator install (for Codex on Windows: `%USERPROFILE%\.codex\skills\.system\skill-creator`). + ## License Original b3sty skill rules, memory notes, and packaging metadata are MIT licensed. Generated native references keep their upstream terms; see `NOTICE.md` and `references/natives/SOURCES.md`. - -## Memory - -- `memory/common/common-errors.md` - common shared errors and fixes. -- `memory/common/native-bugs.md` - known shared native issues and workarounds. -- `memory/common/cfx-patterns.md` - reusable shared FXServer/CfxLua patterns. -- `memory/common/security-performance.md` - learned shared security and performance patterns. -- `memory/fivem/README.md` - FiveM-specific memory namespace. -- `memory/fivem/native-bugs.md` - FiveM-only native issues and workarounds. -- `memory/redm/README.md` - RedM-specific memory namespace. -- `memory/redm/native-bugs.md` - RedM-only native issues and workarounds. - -`skills/` holds stable rules; `memory/` holds learned facts with date and game build; `references/` holds large generated source material. See `SKILL.md` for which file to open for each task. diff --git a/SKILL.md b/SKILL.md index b2405d6..14c3472 100644 --- a/SKILL.md +++ b/SKILL.md @@ -1,6 +1,6 @@ --- name: b3sty-skill -description: b3sty rules for implementing, reviewing, debugging, refactoring, or optimizing RedM/FiveM Lua resources. Use for FXServer manifests, client/server Lua, natives/entities, native invocation from docs (hashes, InvokeNative, marshalling, RDR3 structs), events/callbacks/exports/NUI, NUI/browser-UI bridge (SendNUIMessage, RegisterNUICallback, SetNuiFocus), NUI XSS/player-string rendering, OneSync/entity networking (net IDs, ownership, routing buckets, broadcasts), built-in client events (weaponDamageEvent, explosionEvent), anti-cheat/event-security hardening (give-item/give-money dupe prevention, RegisterNetEvent trust boundary, ACE permissions, SQL injection, secrets/convars, identifier trust, payload size/depth bounds, economy audit trails), SetHttpHandler endpoint hardening, server convar hardening (sv_pureLevel, sv_scriptHookAllowed, sv_requestParanoia, rcon, endpoint privacy) and third-party resource vetting, server validation, throttles/cooldowns, state bags, CFX runtime gotchas (threads, source, exports, identifiers, convars, game builds), config splitting, ox_lib, SQL/OxMySQL persistence, multi-resource integration, performance, cleanup, and learned memory updates. +description: b3sty rules for implementing, reviewing, debugging, refactoring, or optimizing RedM/FiveM Lua resources. Use for FXServer manifests, client/server Lua, natives/entities, native invocation (hashes, InvokeNative, marshalling, RDR3 structs), events/callbacks/exports, NUI bridge (SendNUIMessage, RegisterNUICallback, SetNuiFocus) and NUI XSS, OneSync networking (net IDs, ownership, routing buckets, broadcasts), built-in client events (weaponDamageEvent, explosionEvent), anti-cheat/event-security hardening (give-item/money dupes, ACE permissions, SQL injection, secrets/convars, identifier trust, payload bounds, audit trails), SetHttpHandler endpoints, server.cfg hardening and third-party resource vetting, throttles/cooldowns, state bags, CFX runtime gotchas (threads, source, exports, identifiers, game builds), config splitting, ox_lib, SQL/OxMySQL persistence, ESX/QBCore/Qbox/VORP/RSG framework bridges, multi-resource integration, performance, cleanup, and learned memory updates. --- # b3sty Skill @@ -25,11 +25,13 @@ Use this skill when working on b3sty RedM/FiveM resources or related Lua code. T - Add `skills/common/networking.md` when the feature creates networked entities, relies on entity ownership, uses routing buckets/instances, broadcasts to clients, reacts to player scope, or handles built-in client events (`weaponDamageEvent`, `startProjectileEvent`, `ptFxEvent`, and the rest). - Add `skills/common/nui.md` when the resource has an in-game browser UI (HTML/CSS/JS, React/Svelte/Vue), `SendNUIMessage`, `RegisterNUICallback`, `SetNuiFocus`, or a `ui_page` in the manifest. - Add `skills/common/runtime.md` for threads/waits, the `source` variable, exports and stale references, identifiers, convars, resource lifecycle, yield hazards, or game-build gating. +- Add `skills/common/network-performance.md` for bandwidth, event spam, `-1` broadcasts, latent events, payload size, repeated full fetches, state bag cost, or network overflow kicks. - Add `skills/common/database.md` when SQL, OxMySQL/mysql-async, schema, transactions, migrations, dirty saves, or persisted state is involved. - Add `skills/common/native-rules.md` and the matching game rules when code calls natives, handles entities, weapons, ammo, vehicles, horses, peds, blips, props, or routing buckets. - Add `skills/common/native-usage.md` when translating a native reference entry into a Lua call, invoking by hash with `Citizen.InvokeNative`, handling out-pointer params, packing RDR3 struct arguments, or gating natives by game build. - Search the matching native reference only when verifying a specific native name, hash, namespace, signature, parameter behavior, or game difference. - Add `skills/common/debugging.md` and the relevant `memory/` files when the task is diagnosis, reproduction, traces, NUI errors, database failures, native bugs, or performance investigation. +- Add `skills/common/frameworks.md` when the resource uses ESX, QBCore, Qbox, VORP, or RSG (player lookup, money, items, jobs, lifecycle events, bridge). - Add `skills/common/ox-lib.md` only when the resource already uses ox_lib or the user explicitly accepts adding it. - Add `skills/common/multi-resource.md` when the feature crosses resources through exports, events, callbacks, dependencies, shared scripts, convars, state bags, or framework integration. - Add `skills/fivem/rules.md` or `skills/redm/rules.md` whenever the resource is game-specific or shared behavior might differ. @@ -66,21 +68,21 @@ Apply these on every b3sty Lua task unless the task says otherwise. ### CfxLua -- In RedM/FiveM code, supported compound operators (`+=`, `-=`, `*=`, `/=`, `<<=`, `>>=`, `&=`, `|=`, `^=`) are fine when clearer. Do **not** use `++`/`--`. +- In RedM/FiveM code, supported compound operators (`+=`, `-=`, `*=`, `/=`, `<<=`, `>>=`, `&=`, `|=`, `^=`) are fine when clearer. `^=` is bitwise XOR (`x = x ~ y`), not power. Do **not** use `++`/`--`. - These operators are CfxLua-only - never use them in standard Lua or standalone Lua tooling. ### Natives - Docs name `GET_ENTITY_HEALTH` -> Lua global `GetEntityHealth`; leading-underscore names drop the underscore; hash-only natives use `Citizen.InvokeNative(hash, ...)` with a `--[[NAME]]` comment. -- `Citizen.InvokeNative` BOOL results are `1`/`0`, and `0` is truthy in Lua - compare `== 1`, never use the raw result in an `if`. -- Float params in hash calls must be float-subtype numbers - write `1.0`, coerce computed values with `+ 0.0`. +- BOOL results from named natives and bare `Citizen.InvokeNative` are `1`/`false`, never `true` - test with `if`/`not`, never `== true`. Append `Citizen.ResultAsInteger()` to int-returning hash calls so a real `0` is not `false`; under it a BOOL is `1`/`0` and `0` is truthy, so compare `~= 0`. +- Float params must be float-subtype numbers in named and hash calls alike (`SetEntityHeading(ped, 90)` is broken) - write `1.0`, coerce computed values with `+ 0.0`. - Prefer hash constants via backtick literals or `joaat`; compare hashes to hashes, never to hex strings. - Full mechanics (out params, marshalling, RDR3 structs, builds, confidence): `skills/common/native-usage.md`. ### Config - Small/shared config in `config.lua`; large datasets split into `configs/*.lua`, each returning a table. -- Require a split config only in the script that uses it; no eager aggregators. +- Load a split config only in the script that uses it; no eager aggregators. Stock CfxLua `require` cannot load resource files - use ox_lib's `require` when the resource already loads ox_lib, otherwise the cached loader in `skills/common/style.md` -> Lua Style (client reads need `files`). ### NUI @@ -117,9 +119,11 @@ Open lazily by task - do not preload all of them. - `skills/common/networking.md` - OneSync, net IDs vs handles, entity ownership, routing buckets, scoped vs broadcast messages, player scope, entity lifecycle events, and built-in client events (`weaponDamageEvent` and friends). - `skills/common/nui.md` - in-game browser UI (NUI): Lua<->browser bridge, `SendNUIMessage`, `RegisterNUICallback`, focus, JSON contracts, validation, frontend hygiene, performance, and security. - `skills/common/runtime.md` - threads/waits, the `source` variable, exports and stale references, identifiers, convars, resource lifecycle, yield hazards, and game builds. +- `skills/common/network-performance.md` - net event cost model, FXServer rate limits and overflow kicks, latent events, payload shape, snapshot-then-deltas caching, audience, and state bag cost. - `skills/common/security-performance.md` - when writing `:server:` events, callbacks, sync, DB writes, or hot loops. - `skills/common/database.md` - when writing SQL, OxMySQL/mysql-async persistence, migrations, transactions, or saved state. - `skills/common/debugging.md` - when diagnosing resource failures, traces, client/server/NUI issues, DB issues, load order, or performance bugs. +- `skills/common/frameworks.md` - when a resource integrates with ESX, QBCore, Qbox, VORP, or RSG: player lookup, money/item/job calls, lifecycle events, and the bridge pattern. - `skills/common/ox-lib.md` - when a resource already uses ox_lib or the task explicitly accepts adding ox_lib. - `skills/common/multi-resource.md` - when resources communicate through exports, events, callbacks, dependencies, state bags, or shared libraries. @@ -136,6 +140,14 @@ These are large generated lookup files. Open only the matching file when verifyi - `references/natives/redm-rdr3-natives.md` - RDR3 / RedM native reference. - `references/server.cfg.example` - copyable server hardening baseline (open when setting up or reviewing `server.cfg`). +## Commands + +Slash commands installed with the Claude Code plugin: + +- `commands/b3sty-review.md` - `/b3sty-review`: review a resource and report verified findings by severity. +- `commands/b3sty-new-resource.md` - `/b3sty-new-resource`: scaffold a new resource that follows these rules. +- `commands/b3sty-perf.md` - `/b3sty-perf`: find and fix CPU and network performance problems with a measured/estimated report. + ## Memory Read only when debugging or reusing a learned pattern. Memory files hold facts learned from real work (each entry carries a date and game build). They cross-link to `skills/` for the stable rules and do not duplicate rule text. diff --git a/commands/b3sty-new-resource.md b/commands/b3sty-new-resource.md new file mode 100644 index 0000000..06c4023 --- /dev/null +++ b/commands/b3sty-new-resource.md @@ -0,0 +1,148 @@ +--- +description: Scaffold a new RedM/FiveM Lua resource that follows the b3sty rules from the first commit - fxmanifest, config, client/server controllers, prefixed events with server validation, cleanup, and optional framework bridge, NUI, and oxmysql pieces. Use when asked to create, start, scaffold, or bootstrap a new FiveM or RedM resource or script. +argument-hint: [resource name and what it does] +--- + +# b3sty New Resource + +Target: $ARGUMENTS + +Create a new RedM/FiveM resource skeleton that already satisfies the b3sty rules. Paths under `skills/` and `memory/` below are relative to the b3sty-skill root (the folder holding the main `SKILL.md`; with the plugin install, the plugin root). + +## 1. Decide The Shape + +Infer from the request and the surrounding server repo; ask only for what cannot be inferred: + +- **Resource name** - lowercase with underscores; it prefixes every event (`name:server:action`). +- **Game** - `gta5`, `rdr3`, or both. +- **Framework** - none (default), or the one the server already runs (ESX, QBCore, Qbox, VORP, RSG). +- **NUI** - only if the feature needs a browser UI. +- **Database** - only if state must survive a restart. +- **ox_lib** - only if the server already runs it (`skills/common/ox-lib.md`). + +Default to the smallest shape: no framework, no NUI, no database. + +## 2. Files + +Follow `skills/common/resource-structure.md` -> Resource Structure and `skills/common/fxserver.md` -> Typical Layout: + +```text +resource_name/ + fxmanifest.lua + config.lua + core/client.lua + core/server.lua + bridge/server.lua -- only with a framework + html/index.html -- only with NUI + sql/install.sql -- only with a database +``` + +### fxmanifest.lua + +```lua +fx_version 'cerulean' +games { 'gta5' } -- { 'rdr3' } or { 'rdr3', 'gta5' } +lua54 'yes' + +-- rdr3_warning 'I acknowledge that this is a prerelease build of RedM, and I am aware my resources *will* become incompatible once RedM ships.' + +shared_scripts { + 'config.lua', +} + +client_scripts { + 'core/client.lua', +} + +server_scripts { + -- '@oxmysql/lib/MySQL.lua', + -- 'bridge/server.lua', + 'core/server.lua', +} +``` + +- Keep `rdr3_warning` only when `rdr3` is in `games`. +- With NUI add `ui_page 'html/index.html'` and list every asset in `files` (`skills/common/nui.md`). +- With oxmysql, uncomment its loader line and add `dependency 'oxmysql'`. + +### core/server.lua + +```lua +local Controller = { + ["ActionThrottle"] = {}, +} + +function Controller:IsThrottled(source, key, limit) + local now = GetGameTimer() + local throttle = self.ActionThrottle[source] + + if not throttle then + throttle = {} + self.ActionThrottle[source] = throttle + end + + if now - (throttle[key] or 0) < limit then + return true + end + + throttle[key] = now + return false +end + +RegisterNetEvent("resource_name:server:action", function(payload) + local source = source + if type(source) ~= "number" or source <= 0 then return end + if type(payload) ~= "table" then return end + if Controller:IsThrottled(source, "action", 500) then return end + + -- validate every payload field, then decide the outcome server-side +end) + +AddEventHandler("playerDropped", function() + local source = source + Controller.ActionThrottle[source] = nil +end) + +AddEventHandler("onResourceStop", function(resourceName) + if resourceName ~= GetCurrentResourceName() then return end + -- delete server-created entities, flush dirty saves without yielding +end) +``` + +### core/client.lua + +```lua +local Controller = {} + +function Controller:Start() + CreateThread(function() + while true do + local sleep = 1000 + -- lower sleep only while the player is near/active; never a constant Wait(0) + Wait(sleep) + end + end) +end + +AddEventHandler("onResourceStop", function(resourceName) + if resourceName ~= GetCurrentResourceName() then return end + -- delete local props, blips, zones; SetNuiFocus(false, false) if NUI is used +end) + +Controller:Start() +``` + +- Replace `resource_name` with the real name everywhere. +- Add a framework bridge only from `skills/common/frameworks.md` -> Bridge Pattern, trimmed to the functions this resource uses. +- Put valuable actions (money, items, rewards) through `skills/common/security-performance.md` -> Give-Value Event Hardening from the start. +- For persisted state, follow `skills/common/database.md` and the persistence shape in `memory/common/cfx-patterns.md`. + +## 3. Check Before Handing Over + +- Every custom event is `resource_name:server:*` or `resource_name:client:*`, and every server handler validates `source` and its payload. +- No client-trusted price, amount, reward, or permission. +- Every entity, blip, zone, timer, NUI focus, and per-player table has a cleanup path on `playerDropped` and `onResourceStop`. +- No constant `Wait(0)` loop. +- The manifest lists only files that exist, and `games` matches the natives used. + +Tell the user the `ensure resource_name` line to add to `server.cfg` (after its framework and oxmysql), and the in-game steps to confirm it starts cleanly. diff --git a/commands/b3sty-perf.md b/commands/b3sty-perf.md new file mode 100644 index 0000000..ac6290c --- /dev/null +++ b/commands/b3sty-perf.md @@ -0,0 +1,65 @@ +--- +description: Find and fix CPU and network performance problems in a RedM/FiveM Lua resource - resmon cost, Wait(0) loops, entity pool scans, event spam, -1 broadcasts, payload size, repeated full fetches, state bag misuse - and report measured versus estimated results. Use when asked to optimize, profile, or fix lag, hitches, high resmon, or network overflow kicks in a FiveM or RedM resource. +argument-hint: [resource path or symptom] +--- + +# b3sty Performance Pass + +Target: $ARGUMENTS + +Lower CPU time and network traffic without changing gameplay or weakening server validation. Paths under `skills/` below are relative to the b3sty-skill root (the folder holding the main `SKILL.md`; with the plugin install, the plugin root). + +Open these before proposing fixes: + +- `skills/common/debugging.md` -> Performance Debugging - tools and how to measure. +- `skills/common/security-performance.md` -> Performance Rules - CPU hot paths. +- `skills/common/network-performance.md` - event cost, rate limits, latent events, snapshot-then-deltas, state bags. + +## 1. Measure Or Mark Estimates + +- Ask for (or use provided) numbers: client `resmon` idle and during the slow action, `netEventLog` if the complaint is network, server profiler or hitch warnings if the complaint is server-side. +- If nothing can be measured, continue, but every figure in the report is an estimate and labeled as one. + +## 2. Find Candidates + +```bash +rg -n "CreateThread|while true do|Wait\((0|1|5|10)\)" --type lua +rg -n "Trigger(Latent)?(Server|Client)Event|TriggerEvent|SendNUIMessage" --type lua +rg -n "TriggerClientEvent\([^,]+,\s*-1" --type lua +rg -n "GetGamePool|GetActivePlayers|GetPlayers\(\)|GetHashKey|json\.(en|de)code" --type lua +rg -n "MySQL\.|\.state\b|\.state\.|GlobalState|AddStateBagChangeHandler|lib\.callback" --type lua +``` + +For each hit record: side, loop or one-shot, how often it runs, payload shape, recipients. + +## 3. Rank + +- **Critical** - events or pool scans per frame, `-1` broadcast of large data, full fetch per UI open, SQL inside a loop. +- **High** - short-wait loops doing real work far from the player, large payloads, repeated `SELECT` on hot paths. +- **Medium** - allocations and repeated natives in loops, unthrottled HUD streams. +- **Low** - micro-optimizations `resmon` will not show. Do not touch Low while Critical remains. + +## 4. Fix Safely + +- Search the whole resource and its dependents before removing or renaming an event, export, or state key. +- Keep behavior of money, items, permissions, ownership, routing buckets, NUI, and framework state unchanged unless asked; if a fix must change behavior, say so. +- Keep every server check (`source`, amount, ownership, distance, bucket, cooldown) after the change. +- Change one class of problem at a time so each can be measured. + +## 5. Report + +```text +Summary: the main bottlenecks in two or three lines. + +Performance + Before: (measured | estimate) + After: (measured | estimate) + +Network (per changed event) + Event / frequency / payload / recipients / bytes per second (measured | estimate) + +Changes: what changed and the reason for each. +Risks: anything that could affect sync, gameplay, or server checks. +Remaining: what to measure next - never claim "fully optimized". +Verify: resmon idle + action, netEventLog, server profiler, and the gameplay paths to retest. +``` diff --git a/commands/b3sty-review.md b/commands/b3sty-review.md new file mode 100644 index 0000000..dd15b94 --- /dev/null +++ b/commands/b3sty-review.md @@ -0,0 +1,86 @@ +--- +description: Review a RedM/FiveM Lua resource against the b3sty rules and report findings ranked by severity. Use when asked to review, audit, check, or security-check a FiveM or RedM resource, a server event, an NUI bridge, or a pull request touching CFX Lua code, or before releasing/selling a resource. Covers server authority, give-item/give-money exploits, event and NUI trust boundaries, SQL injection, XSS, networking, lifecycle cleanup, framework usage, and performance. +argument-hint: [resource path, files, or PR] +--- + +# b3sty Resource Review + +Target: $ARGUMENTS + +Review a RedM/FiveM resource and report verified findings, most severe first. This command drives the process; the rules themselves live in the b3sty-skill package. Paths under `skills/`, `memory/` and `references/` below are relative to the b3sty-skill root (the folder holding the main `SKILL.md`; with the plugin install, the plugin root). + +Do not edit code during the review. Offer fixes at the end; apply them only when asked. + +## 1. Scope + +- Identify the game (`games` in `fxmanifest.lua`), the framework (ESX, QBCore, Qbox, VORP, RSG, or none), ox_lib usage, NUI (`ui_page`), and database use (oxmysql). +- If the request names specific files or a diff, review those plus whatever they call; otherwise review the whole resource. + +## 2. Inventory Entry Points + +List every place untrusted input enters before reading logic in depth: + +```bash +rg -n "RegisterNetEvent|RegisterServerEvent|AddEventHandler\(\"(weaponDamageEvent|explosionEvent|startProjectileEvent|ptFxEvent|removeAllWeaponsEvent|entityCreating)" --type lua +rg -n "RegisterNUICallback|RegisterCommand|exports\(|lib\.callback\.register|CreateCallback|RegisterServerCallback|SetHttpHandler" --type lua +rg -n "MySQL\.|oxmysql|exports\.ghmattimysql|exports\['mysql-async'\]" --type lua +rg -n "innerHTML|v-html|\{@html|dangerouslySetInnerHTML|insertAdjacentHTML" -g "!*.lua" +rg -n "Wait\(0\)|while true do" --type lua +``` + +Record each entry point with file:line and what it can change (money, items, jobs, entities, saved data, UI only). + +## 3. Review In Priority Order + +Follow the order in the main `SKILL.md` -> Review Priorities. Open only the rule files the resource needs: + +| Surface found | Open | +|---|---| +| Any server event, callback, command, export | `skills/common/security-performance.md` (Required Event Pattern, Give-Value Event Hardening, Review Questions) | +| Money, items, rewards, shops | `skills/common/security-performance.md` -> Give-value event checklist | +| Framework calls | `skills/common/frameworks.md` | +| NUI | `skills/common/nui.md` | +| Networked entities, net IDs, buckets, built-in client events | `skills/common/networking.md` | +| SQL | `skills/common/database.md` | +| Threads, `source` across yields, exports, lifecycle | `skills/common/runtime.md` | +| Natives | `skills/common/native-usage.md`, then `references/natives/` to confirm names and hashes | +| Game-specific code | `skills/fivem/rules.md` or `skills/redm/rules.md` | +| Cross-resource calls | `skills/common/multi-resource.md` | + +For each entry point check, in this order: + +1. **Authority** - does the server decide the price, amount, reward, target, and permission, or does it trust the client payload? +2. **Input** - type, range, integer, NaN, string length, table depth; `local source = source` captured before any yield. +3. **Abuse** - throttle, in-flight lock, replay, distance/ownership checks, audit log. +4. **Lifecycle** - cleanup on `playerDropped`, character unload, and `onResourceStop`; NUI focus released; entities guarded with `DoesEntityExist`. +5. **Portability** - RedM/FiveM-specific natives or behavior in shared files. +6. **Performance** - hot loops, `Wait(0)`, repeated lookups, broadcasts to `-1`. +7. **Style** - only after everything above, and only where it hurts readability. + +Check `memory/` for known engine quirks before reporting something as a bug in the resource. + +## 4. Verify Before Reporting + +- Re-read the code around every candidate finding. Drop it if another line already handles it. +- For exploit findings, write the concrete abuse: which event a cheater triggers, with what payload, and what they gain. +- Confirm native names, hashes, and signatures against `references/natives/` instead of memory. +- Do not report taste or formatting preferences as defects. + +## 5. Report + +Group by severity: + +- **Critical** - duplication or free money/items, arbitrary SQL, NUI XSS reachable by other players, admin actions without permission checks, remote crash of the server. +- **High** - exploitable with effort or under a race, missing ownership/distance checks on valuable actions, unbounded payloads. +- **Medium** - leaks (entities, handlers, per-player tables), stuck NUI focus, missing cleanup, noticeable performance cost. +- **Low** - portability risks, minor performance, readability that hides bugs. + +For each finding give: + +```text +[Severity] file.lua:123 - one-line summary +Abuse / impact: how it is triggered and what happens. +Fix: the concrete change (short code when it helps), citing the rule file section. +``` + +End with: entry points reviewed, what was not reviewed and why, and the manual repro steps worth running in-game. If the review surfaced a new engine quirk or a recurring mistake, propose a `memory/` entry (date and game build included). diff --git a/memory/common/README.md b/memory/common/README.md index 93b0bb8..ae14e43 100644 --- a/memory/common/README.md +++ b/memory/common/README.md @@ -2,7 +2,7 @@ Store learned facts that apply to both RedM and FiveM here. -Use `memory/fivem/` or `memory/redm/` for game-specific facts. Add the date and game build to every real entry. +Use `memory/fivem/` or `memory/redm/` for game-specific facts. Add the date and game build to every real entry; build-independent entries (pure Lua, resource structure) may use `Game build: n/a`. ## Files diff --git a/memory/common/cfx-patterns.md b/memory/common/cfx-patterns.md index 598c46a..76f53a1 100644 --- a/memory/common/cfx-patterns.md +++ b/memory/common/cfx-patterns.md @@ -80,18 +80,41 @@ Track reusable FXServer/CfxLua implementation patterns here. These apply to both }, true) -- Client: render from state - local Controller = {} + local Controller = { + ["AttachedProps"] = {}, -- [bagName] = { object, ... } + } + + function Controller:ClearAttachState(bagName) + local props = self.AttachedProps[bagName] + if not props then return end + + for i = 1, #props do + if DoesEntityExist(props[i]) then + DeleteEntity(props[i]) + end + end - function Controller:RenderAttachState(player, attachState) + self.AttachedProps[bagName] = nil + end + + function Controller:RenderAttachState(bagName, player, attachState) + self:ClearAttachState(bagName) -- CreateObject(model, x, y, z, false, false, false) -- AttachEntityToEntity(object, GetPlayerPed(player), boneIndex, ...) + -- Track each object in self.AttachedProps[bagName] so cleanup can find it. end AddStateBagChangeHandler("attach", nil, function(bagName, _, attachState) + -- Cleared key: remove props by bag name, even if the player no longer resolves. + if type(attachState) ~= "table" then + Controller:ClearAttachState(bagName) + return + end + local player = GetPlayerFromStateBagName(bagName) - if player == 0 or type(attachState) ~= "table" then return end + if player == 0 then return end - Controller:RenderAttachState(player, attachState) + Controller:RenderAttachState(bagName, player, attachState) end) ``` - Notes: local attached props need cleanup on state removal, player stream-out/drop, and resource stop. @@ -136,6 +159,7 @@ Track reusable FXServer/CfxLua implementation patterns here. These apply to both local locationsConfig = require("configs.locations") local itemsConfig = require("configs.items") ``` +- Notes: plain CfxLua `require` cannot load resource files (fails with "module 'configs.locations' not found"). This shape needs `@ox_lib/init.lua` in the manifest; without ox_lib use the cached `loadConfig` loader in `skills/common/style.md` -> Lua Style. Client-read configs must be listed in `files` either way. - Notes: full rules in `skills/common/style.md` -> Config Splitting and `skills/common/fxserver.md`. ## Player Persistence Pattern @@ -146,15 +170,20 @@ Track reusable FXServer/CfxLua implementation patterns here. These apply to both - Concrete shape: ```lua -- RAM cache + dirty tracking + spread autosave + force-save on exit. - -- Async so it never blocks the main thread; never lose more than the autosave interval on crash. + -- Non-blocking (`.await` only suspends the calling thread); never lose more than the autosave interval on crash. + local SAVE_QUERY = "UPDATE players SET data = ? WHERE identifier = ?" + local MAX_SAVE_FAILS = 5 + local Controller = { ["PlayerData"] = {}, ["PlayerDirty"] = {}, ["PlayerSaving"] = {}, + ["PlayerSaveFails"] = {}, } - function Controller:SavePlayer(source) - if self.PlayerSaving[source] then return end + -- Call from a thread: it awaits the write. `force` skips the in-flight guard (final save on leave). + function Controller:SavePlayer(source, force) + if self.PlayerSaving[source] and not force then return end local data = self.PlayerData[source] if not data then return end @@ -162,26 +191,44 @@ Track reusable FXServer/CfxLua implementation patterns here. These apply to both self.PlayerSaving[source] = true self.PlayerDirty[source] = nil - MySQL.update("UPDATE players SET data = ? WHERE identifier = ?", { + -- pcall + .await: with default oxmysql settings a callback-style query that errors + -- never runs its callback, so the failure path below would be unreachable. + local ok, affectedRows = pcall(MySQL.update.await, SAVE_QUERY, { json.encode(data), data.identifier, - }, function(affectedRows) - self.PlayerSaving[source] = nil + }) + self.PlayerSaving[source] = nil - if not affectedRows and self.PlayerData[source] then - self.PlayerDirty[source] = true -- failed save stays dirty for the next pass - end - end) + if ok and affectedRows and affectedRows > 0 then + self.PlayerSaveFails[source] = nil + return + end + + -- Query error, or 0 rows (no row for this identifier; `0` is truthy in Lua, so check it explicitly). + print(("[%s] player save failed for %s: %s"):format( + GetCurrentResourceName(), tostring(data.identifier), + ok and "0 rows updated" or tostring(affectedRows):match("([^\n]*)$"):sub(1, 120) + )) + + if not self.PlayerData[source] then return end -- player already cleared + + local fails = (self.PlayerSaveFails[source] or 0) + 1 + self.PlayerSaveFails[source] = fails + + if fails < MAX_SAVE_FAILS then + self.PlayerDirty[source] = true -- capped retry on the next autosave pass + end end function Controller:ClearPlayer(source) self.PlayerData[source] = nil self.PlayerDirty[source] = nil self.PlayerSaving[source] = nil + self.PlayerSaveFails[source] = nil end - -- Spread autosave: snapshot the dirty set, then save with a small yield between - -- players so one pass never stalls the main thread and the backlog cannot grow - -- unbounded. Never mutate/iterate PlayerDirty across a yield without a snapshot. + -- Spread autosave: snapshot the dirty set, then save one player at a time with a small + -- yield between players so one pass never stalls the main thread and the backlog cannot + -- grow unbounded. Never mutate/iterate PlayerDirty across a yield without a snapshot. CreateThread(function() while true do Wait(60 * 1000) @@ -198,39 +245,52 @@ Track reusable FXServer/CfxLua implementation patterns here. These apply to both end end) - -- Force-save on leave (primary save point). + -- Final save on leave (primary save point). Always issued: wait (bounded) for an + -- in-flight autosave so its older snapshot cannot commit last, then force-save and clear. AddEventHandler("playerDropped", function() local source = source + if not Controller.PlayerData[source] then return end - if source then - Controller:SavePlayer(source) + CreateThread(function() + local waited = 0 + while Controller.PlayerSaving[source] and waited < 10000 do + Wait(100) + waited = waited + 100 + end + + Controller:SavePlayer(source, true) Controller:ClearPlayer(source) - end + end) end) -- Force-save all dirty players when this resource stops (covers restart/stop). - -- Await here: fire-and-forget saves can be killed mid-flight on shutdown. + -- Never yield here: the resource is torn down at the first yield, so an awaited loop + -- writes at most one player. Dispatch one non-awaited batch; oxmysql finishes it. AddEventHandler("onResourceStop", function(resourceName) if resourceName ~= GetCurrentResourceName() then return end + local batch = {} for source in pairs(Controller.PlayerDirty) do local data = Controller.PlayerData[source] if data then - MySQL.update.await("UPDATE players SET data = ? WHERE identifier = ?", { - json.encode(data), data.identifier, - }) + batch[#batch + 1] = { json.encode(data), data.identifier } end end + + if #batch > 0 then + MySQL.prepare(SAVE_QUERY, batch) + end end) -- Gameplay code marks a player dirty inline wherever state changes: -- Controller.PlayerDirty[source] = true ``` - Notes: interval is a trade-off - shorter = less data loss but more DB load; 60-120s is a sane default. -- Notes: `Controller:SavePlayer` is kept as a method because it is used in three places (real duplication); dirty marking is inlined because it is one line. -- Notes: async queries are mandatory on gameplay paths - sync DB calls block the single FXServer Lua thread. The one exception is `onResourceStop`, where `.await` is required so the writes finish before the runtime tears the resource down. -- Notes: a failed save must re-mark the player dirty (see the callback above) so data is retried instead of silently lost; on `playerDropped` the final save is best-effort because the RAM cache is cleared right after. +- Notes: `Controller:SavePlayer` is kept as a method because the autosave loop and `playerDropped` share its write, failure, and retry logic (real duplication); `onResourceStop` cannot yield, so it batches its own non-awaited write with the shared `SAVE_QUERY`. Dirty marking is inlined because it is one line. +- Notes: never await in `onResourceStop` (the handler is abandoned at its first yield); dispatch one non-awaited batch there, and for full shutdowns also save from txAdmin's `txAdmin:events:serverShuttingDown` (awaits are fine within its `delay`). Yield rules: `skills/common/runtime.md` -> Yield Hazards. +- Notes: failed or 0-row saves are logged and retried up to `MAX_SAVE_FAILS` (the next gameplay change marks the player dirty again); error-surfacing rules are in `skills/common/database.md` -> OxMySQL API Shape. Use `INSERT ... ON DUPLICATE KEY UPDATE` if this resource owns row creation. +- Notes: on `playerDropped` the final save is always issued, ordered after any in-flight autosave (bounded 10s wait), and the RAM cache is cleared only after it returns. A final save that still fails is logged and that data is lost. If a quick reconnect must never load stale data, have the loader wait while a final save for that identifier is still pending. - Notes: uses the oxmysql API (`MySQL.update` / `MySQL.update.await`), not the legacy `MySQL.async.*` aliases - see `skills/common/database.md` -> OxMySQL API Shape. ## Template diff --git a/memory/common/common-errors.md b/memory/common/common-errors.md index ce18abd..5688f7e 100644 --- a/memory/common/common-errors.md +++ b/memory/common/common-errors.md @@ -16,6 +16,8 @@ Track common errors and fixes here. end ``` - Prevention: use `~= true`, `== false`, or assign the value to a clearly named local before comparing. +- Date: 2026-07-02 +- Game build: n/a (pure Lua / build-independent) ## Missing Entity Guards @@ -24,6 +26,8 @@ Track common errors and fixes here. - Cause: entities can be deleted by the game, stream out, detach, or be removed by another resource. - Fix: check `DoesEntityExist(entity)` before reading, attaching, detaching, or deleting. - Prevention: keep cleanup and reattach code defensive. +- Date: 2026-07-02 +- Game build: n/a (build-independent entity lifetime behavior) ## Template diff --git a/memory/common/native-bugs.md b/memory/common/native-bugs.md index f023090..bd3bb97 100644 --- a/memory/common/native-bugs.md +++ b/memory/common/native-bugs.md @@ -6,13 +6,13 @@ Track recurring native issues that reproduce in both RedM and FiveM here. RedM-o - Native: `GetEntityModel` / `GET_ENTITY_MODEL` - Runtime: RedM/FiveM server, OneSync, `entityCreating` -- Game build: RedM build 1491 and FiveM build 3258/3751 reports; exact affected range unknown. +- Game build: RedM 1491 (#2944, #2241; peds/objects); FiveM artifact 30707 (#4053, client-created vehicles). The ambient-pickup case (#2924, client 10939 / FXServer 10367) has only an unmerged PR (#2940), and even that leaves pickups without a custom model at 0. - Date: 2026-07-02 -- Symptom: `GetEntityModel(entity)` can return `0` for a valid network entity during `entityCreating`, including peds, pickups, or client-created vehicles. +- Symptom: `GetEntityModel(entity)` can return `0` for a valid network entity during `entityCreating`, notably peds/objects (RedM), ambient pickups, or client-created vehicles (FiveM). - Cause: creation/sync data may not be available to the server when `entityCreating` fires. RedM object model parsing is also known to be incomplete in some paths. -- Fix / workaround: do not reject or `CancelEvent()` solely because `GetEntityModel(entity) == 0`. Validate what is available at creation time, then defer model-dependent checks until `entityCreated` or after a short wait when the entity data has synced. +- Fix / workaround: do not reject or `CancelEvent()` solely because `GetEntityModel(entity) == 0`. Validate what is available at creation time, then defer model-dependent checks to `entityCreated`, or to a separate `CreateThread` keyed by the handle that rechecks after a short wait. Never `Wait` inside `entityCreating` (see `skills/common/runtime.md` -> Yield Hazards). Once the entity exists it can no longer be cancelled, so delete it (after a `DoesEntityExist` check) if the recheck fails. - Notes: if model allowlisting is security-critical, combine source/routing bucket/type checks at creation time with a later model recheck and cleanup path instead of assuming `0` means malicious. -- Sources: citizenfx/fivem#2944, citizenfx/fivem#2924, citizenfx/fivem#4053, citizenfx/fivem#2241. +- Sources: citizenfx/fivem#2944, citizenfx/fivem#2924 (pickup case; open PR citizenfx/fivem#2940 is only a partial fix), citizenfx/fivem#4053, citizenfx/fivem#2241. ## Template diff --git a/memory/redm/native-bugs.md b/memory/redm/native-bugs.md index 4eaedfb..8ee30bf 100644 --- a/memory/redm/native-bugs.md +++ b/memory/redm/native-bugs.md @@ -4,19 +4,20 @@ Track recurring RDR3 / RedM-only native issues here. Bugs that reproduce in both ## Server-Side `GetEntityHealth` Returns `0` -- Native: `GetEntityHealth` / `GET_ENTITY_HEALTH` (`0x82368787EA73C0F7`) +- Native: `GetEntityHealth` / `GET_ENTITY_HEALTH` (RDR3 client hash `0x82368787EA73C0F7`; server CFX native `0x8E3222B7`) - Runtime: RedM server (RDR3) CfxLua - Game build: Unknown - Date: 2026-07-02 - Symptom: calling `GetEntityHealth(entity)` on the server can return `0` consistently, even when the entity is alive. - Cause: unknown RedM server native/runtime behavior. Client-side health reads are not affected in the same way. -- Fix / workaround: do not use server-side `GetEntityHealth` as the source of truth in RedM. Use client-side reads only for UI/visual behavior. For server-side decisions, keep server-owned eligibility or health-like state where possible, or validate through server-controlled action state instead of trusting a direct health native read. -- Notes: if client-reported health is unavoidable, treat it as untrusted input: type-check, clamp, rate-limit, compare against expected server-side state, and never use it as the only condition for rewards, damage, permissions, or anti-abuse checks. +- Fix / workaround: do not use server-side `GetEntityHealth` as the source of truth in RedM; use client-side reads only for UI/visuals. +- Related rules: `skills/redm/rules.md` -> Entity Health (server-owned state, handling client-reported health). +- Notes: on the server, call the named global `GetEntityHealth`, not `Citizen.InvokeNative` with the RDR3 client hash; FXServer does not register game hashes. - Notes: confirm the affected game build when the issue is next reproduced. ## Server-Side `GetVehiclePedIsIn(ped, false)` Can Return The Last Vehicle -- Native: `GetVehiclePedIsIn` / `GET_VEHICLE_PED_IS_IN` (`0x9A9112A0FE9A4713`) +- Native: `GetVehiclePedIsIn` / `GET_VEHICLE_PED_IS_IN` (RDR3 client hash `0x9A9112A0FE9A4713`; server CFX native `0xAFE92319`) - Runtime: RedM server (RDR3) CfxLua, OneSync - Game build: RedM/30408, FXServer-master server v1.0.0.29199 win32 report. - Date: 2026-07-02 @@ -24,6 +25,7 @@ Track recurring RDR3 / RedM-only native issues here. Bugs that reproduce in both - Cause: unknown RedM server native/runtime behavior; the `lastVehicle` flag is not respected in the reported server-side path. - Fix / workaround: guard the server-side vehicle lookup with `IsPedInAnyVehicle(ped, false)` and return `0` when the ped is not currently in a vehicle. - Notes: do not use server-side `GetVehiclePedIsIn(ped, false) ~= 0` alone as proof that a RedM player is currently seated in a vehicle. +- Notes: on the server, call the named globals, not `Citizen.InvokeNative` with the RDR3 client hash; FXServer does not register game hashes. - Source: citizenfx/fivem#4006. ```lua @@ -42,15 +44,15 @@ end ## RedM Ammo Set/Remove Natives Can Leave Ammo State Stale -- Native: `SetPedAmmoByType` / `SET_PED_AMMO_BY_TYPE` (`0x5FD1E1F011E76D7E`), `_REMOVE_AMMO_FROM_PED` (`0xF4823C813CB8277D`), `_REMOVE_AMMO_FROM_PED_BY_TYPE` (`0xB6CFEC32E3742779`) +- Native: `SetPedAmmoByType` / `SET_PED_AMMO_BY_TYPE` (`0x5FD1E1F011E76D7E`), `_REMOVE_AMMO_FROM_PED` (`0xF4823C813CB8277D`), `_REMOVE_AMMO_FROM_PED_BY_TYPE` (`0xB6CFEC32E3742779`), workaround uses `RemoveAllPedAmmo` / `_REMOVE_ALL_PED_AMMO` (`0x1B83C0DEEBCBB214`) - Runtime: RedM (RDR3) CfxLua - Game build: Unknown; citizenfx/fivem#3980 reports RedM all versions. - Date: 2026-07-02 - Symptom: direct RedM ammo set/remove natives can fail to decrease ammo, only change it temporarily, or allow the old ammo value to restore after shooting, reload, or weapon switching. - Cause: direct reserve/removal updates can leave native ammo state inconsistent with RedM weapon/inventory state. -- Fix / workaround: own ammo in resource/server state, then apply the full desired ammo map in one wrapper. Clear all ped ammo first with `RemoveAllPedAmmo(ped)`, read and reapply the current ammo map from the resource's local player state/helper, then set the requested ammo type. +- Fix / workaround: own ammo in resource/server state, then apply the full desired ammo map in one wrapper. The wrapper records the requested amount in the owned ammo map first, then clears all ped ammo with `RemoveAllPedAmmo(ped)` and reapplies the whole map, so a later call for another type cannot restore a stale value. - Notes: wrap this behavior in a helper and document the parameters with LuaDoc. -- Notes: RedM-only. Ammo by type is an RDR3 concept; FiveM/GTA V uses different ammo natives. Confirm the affected game build and date when the issue is next reproduced. +- Notes: RedM-only workaround. Both games pool ammo by type (`SetPedAmmoByType` has the same hash `0x5FD1E1F011E76D7E` in GTA V), but this wrapper depends on RDR3's `_REMOVE_ALL_PED_AMMO` and targets a RedM-specific stale-ammo bug, so keep it behind a RedM guard. Confirm the affected game build and date when the issue is next reproduced. - Source: citizenfx/fivem#3980. ```lua @@ -61,26 +63,32 @@ function Controller:GetCurrentAmmoMap() return {} end +---@param ammoMap table Ammo type name -> reserve amount. +function Controller:SetCurrentAmmoMap(ammoMap) + -- Write to this resource's player data/state cache. +end + ---@param ammoType string Native ammo type name. ---@param amount integer Reserve ammo amount to apply. function Controller:SetAmmoByType(ammoType, amount) local ped = PlayerPedId() - RemoveAllPedAmmo(ped) - local ammoMap = self:GetCurrentAmmoMap() ammoMap = type(ammoMap) == "table" and ammoMap or {} + -- Update owned state first so the next call reapplies this amount, not a stale one. + ammoMap[ammoType] = amount + self:SetCurrentAmmoMap(ammoMap) + + RemoveAllPedAmmo(ped) for ammo, reserve in pairs(ammoMap) do SetPedAmmoByType(ped, joaat(ammo), reserve) end - - SetPedAmmoByType(ped, joaat(ammoType), amount) end ``` ## Shared Longarm Cosmetic Components May Not Render On Held Weapons -- Native: RDR3 weapon component apply path (`0x74C9090FDD1BB48E`), `_HAS_PED_GOT_WEAPON_COMPONENT` (`0xBBC67A6F965C688A`), `HAS_WEAPON_GOT_WEAPON_COMPONENT` (`0x76A18844E743BF91`) +- Native: `GiveWeaponComponentToEntity` / `_GIVE_WEAPON_COMPONENT_TO_ENTITY` (`0x74C9090FDD1BB48E`, target can be a ped or a weapon object), `_HAS_PED_GOT_WEAPON_COMPONENT` (`0xBBC67A6F965C688A`), `HAS_WEAPON_GOT_WEAPON_COMPONENT` (`0x76A18844E743BF91`) - Runtime: RedM client (RDR3) CfxLua - Game build: RDR2 build 1491 report; exact affected range unknown. - Date: 2026-07-02 @@ -88,6 +96,7 @@ end - Cause: unknown RedM weapon component/rendering behavior for specific shared longarm cosmetics. - Fix / workaround: verify cosmetics on the actual held weapon entity, not only with the ped-level check. Keep per-component fallback/deny lists for known non-rendering cosmetics and do not promise unsupported visual variants in UI. - Notes: reported failing examples include `COMPONENT_LONGARM_GRIPSTOCK_TINT_PEARL`, `COMPONENT_LONGARM_GRIP_MATERIAL_BURLED`, and `COMPONENT_LONGARM_WRAP_MATERIAL_LEATHER` on rolling block/carcano/bolt-action longarms. +- Notes: per citizenfx/fivem#4026, the standalone weapon object path that does render uses the same `GiveWeaponComponentToEntity` native; the failure is specific to the held weapon. Recheck when reproduced. - Source: citizenfx/fivem#4026. ## Template diff --git a/references/server.cfg.example b/references/server.cfg.example index 1792abf..25b0433 100644 --- a/references/server.cfg.example +++ b/references/server.cfg.example @@ -1,31 +1,44 @@ # b3sty server.cfg hardening baseline. -# Merge into your server.cfg and adjust per server; the rules behind every line -# are in skills/common/security-performance.md -> Server Hardening And Operations. +# Merge into your server.cfg and adjust per server - most rules behind these lines are in +# skills/common/security-performance.md -> Server Hardening And Operations and +# skills/common/networking.md -> Entity Ownership. +# Never put a semicolon in comments - FXServer splits commands on semicolons (outside +# double quotes) before it strips comments, so the rest of the line runs as a command. +# Put 'exec secrets.cfg' (and all set/setr convars) near the top of server.cfg, +# before any ensure/start line - resources read convars when they start. # Convar reference: https://docs.fivem.net/docs/server-manual/server-commands/ +## Secrets - real values live only in secrets.cfg (gitignored), never here. +# secrets.cfg holds: sv_licenseKey, set mysql_connection_string, set steam_webApiKey, webhook URLs (set, never setr/sets) +exec secrets.cfg + ## Core set onesync on # required for entityCreating gates, ownership, routing buckets -# sv_enforceGameBuild 3407 # pick your build (RedM: 1491); startup-only +# sv_enforceGameBuild 3407 # pick your build (RedM: 1491), startup-only ## Anti-cheat / client integrity sv_scriptHookAllowed 0 # keep ScriptHook-style client plugins blocked -sv_pureLevel 1 # reject modified client files; 2 = strictest (FiveM; verify RedM support before relying on it) -sv_requestParanoia 1 # stricter client request handling; raise gradually, watch for false kicks -sv_filterRequestControl 4 # do not route REQUEST_CONTROL_EVENT (entity hijack/explosion-by-proxy vector) +sv_pureLevel 1 # reject modified client files - 2 = strictest (FiveM - verify RedM support before relying on it) +sv_filterRequestControl 4 # FiveM only (no effect on RedM) - blocks ALL client control requests - audit first: +# NetworkRequestControlOfEntity on another client's entity never succeeds (tow/impound/lockpick/delete loops hang) - +# opt entities out server-side with SetEntityIgnoreRequestControlFilter(entity, true), or use +# 2 (player-controlled entities, currently occupied vehicles) / 3 (+ settled non-player entities) as softer modes sv_enableNetworkedPhoneExplosions false sv_enableNetworkedSounds false sv_enableNetworkedScriptEntityStates false setr sv_stateBagStrictMode true # audit resources first: client replicated state bag writes stop working ## Identity / connection -sv_authMinTrust 5 # 1-5, spoof resistance required from identity providers +sv_authMinTrust 5 # effectively a no-op on current artifacts: the ip: identifier always reports trust 5 - +# sv_authMaxVariance 1 below is what actually requires a stable (license/steam) identifier sv_authMaxVariance 1 # 1-5, identifier stability required -sv_endpointPrivacy true # hide player IPs from other clients +# sv_endpointPrivacy - removed on current artifacts (players.json no longer exposes endpoints) - only old artifacts +# need it, where it hid IPs in public HTTP reports, not from other clients ## Exposure -# sv_master1 "" # uncomment to delist a private server -# rcon stays disabled by leaving rcon_password unset; if you must use it, long unique password - -## Secrets - real values live only in secrets.cfg (gitignored), never here. -# secrets.cfg holds: sv_licenseKey, set mysql_connection_string, set steam_webApiKey, webhook URLs (set, never setr/sets) -exec secrets.cfg +sv_requestParanoia 1 # HTTP flood defense on info/dynamic/players.json: block IPs whose requests carry a Via (proxy) header, +# 2 also blocks browser requests to those endpoints, 3 also closes the socket +# sv_master1 "" # marks the server private (browser Connect button disabled), but it is still listed - this does NOT delist. +# Enforce real access with an allowlist in playerConnecting deferrals (optionally 'sets sv_appearAllowlisted true'), +# sv_lan true is LAN/dev only +# rcon stays disabled by leaving rcon_password unset - if you must use it, long unique password diff --git a/scripts/check_lua_examples.py b/scripts/check_lua_examples.py new file mode 100644 index 0000000..db2dd17 --- /dev/null +++ b/scripts/check_lua_examples.py @@ -0,0 +1,118 @@ +#!/usr/bin/env python3 +"""Syntax-check every ```lua block in the skill docs with a Lua 5.4 compiler. + +CfxLua extensions are rewritten to plain Lua 5.4 first (compound assignment, +backtick hashes), so the check proves the example parses, not that it runs. +Put `` on the line before a fence to skip a block that +is intentionally partial. +""" + +from __future__ import annotations + +import re +import shutil +import subprocess +import sys +import tempfile +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[1] +DOC_GLOBS = ["SKILL.md", "skills/**/*.md", "memory/**/*.md", "commands/*.md"] +SKIP_MARKER = "" +FENCE = re.compile(r"^(?P[ \t]*)```lua[ \t]*$") +COMPOUND = re.compile( + r"^(?P[ \t]*[A-Za-z_][\w.:\[\]\"']*)[ \t]*(?:\+|-|\*|/|<<|>>|&|\||\^)=(?!=)(?P.*)$" +) +BACKTICK_HASH = re.compile(r"`[^`\n]*`") + + +def to_plain_lua(code: str) -> str: + lines = [] + for line in code.splitlines(): + line = BACKTICK_HASH.sub("0", line) + match = COMPOUND.match(line) + if match: + line = f"{match.group('lhs')} = ({match.group('rhs').strip()})" + lines.append(line) + return "\n".join(lines) + "\n" + + +def lua_blocks(path: Path): + lines = path.read_text(encoding="utf-8").splitlines() + index = 0 + while index < len(lines): + match = FENCE.match(lines[index]) + if not match: + index += 1 + continue + + skip = index > 0 and lines[index - 1].strip() == SKIP_MARKER + indent = len(match.group("indent")) + start = index + 1 + body = [] + index += 1 + while index < len(lines) and lines[index].strip() != "```": + body.append(lines[index][indent:] if lines[index][:indent].strip() == "" else lines[index]) + index += 1 + index += 1 + if not skip: + yield start, "\n".join(body) + + +def find_compiler(): + for name in ("luac5.4", "luac"): + binary = shutil.which(name) + if binary: + version = subprocess.run([binary, "-v"], capture_output=True, text=True) + if "5.4" in (version.stdout + version.stderr): + return lambda source: compile_with_luac(binary, source) + try: + from lupa import lua54 # type: ignore + + runtime = lua54.LuaRuntime() + loader = runtime.eval("function(s) local f, err = load(s, '=example') return err end") + return lambda source: loader(source) + except ImportError: + return None + + +def compile_with_luac(binary: str, source: str): + with tempfile.NamedTemporaryFile("w", suffix=".lua", delete=False, encoding="utf-8") as handle: + handle.write(source) + name = handle.name + try: + result = subprocess.run([binary, "-p", name], capture_output=True, text=True) + return (result.stderr or result.stdout).replace(name, "example").strip() or None + finally: + Path(name).unlink() + + +def main() -> int: + compile_source = find_compiler() + if compile_source is None: + print("No Lua 5.4 compiler found: install lua5.4 (luac5.4) or `pip install lupa`.") + return 2 + + paths = sorted({path for pattern in DOC_GLOBS for path in ROOT.glob(pattern)}) + checked = 0 + failures = [] + for path in paths: + for line, code in lua_blocks(path): + checked += 1 + error = compile_source(to_plain_lua(code)) + if error: + failures.append(f"{path.relative_to(ROOT).as_posix()}:{line}: {error}") + + if failures: + print(f"Lua example check failed ({len(failures)} of {checked} blocks):") + for failure in failures: + print(f"- {failure}") + return 1 + + print(f"Lua example check passed ({checked} blocks).") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/validate_b3sty_skill.py b/scripts/validate_b3sty_skill.py index 59a6495..a5f9f7e 100644 --- a/scripts/validate_b3sty_skill.py +++ b/scripts/validate_b3sty_skill.py @@ -28,6 +28,11 @@ "skills/common/debugging.md", "skills/common/ox-lib.md", "skills/common/multi-resource.md", + "skills/common/frameworks.md", + "commands/b3sty-review.md", + "commands/b3sty-new-resource.md", + "commands/b3sty-perf.md", + "skills/common/network-performance.md", "skills/fivem/rules.md", "skills/redm/rules.md", "memory/common/README.md", @@ -70,12 +75,60 @@ def check_skill_frontmatter(skill_text: str, failures: list[str]) -> None: frontmatter = match.group("body") if "name: b3sty-skill" not in frontmatter: fail("SKILL.md frontmatter must include name: b3sty-skill", failures) - if "description:" not in frontmatter: + description = re.search(r"^description:\s*(.*)$", frontmatter, re.MULTILINE) + if not description: fail("SKILL.md frontmatter must include description", failures) + elif len(description.group(1).strip()) > 1024: + fail( + f"SKILL.md description is {len(description.group(1).strip())} characters; maximum is 1024", + failures, + ) if "RedM" not in frontmatter or "FiveM" not in frontmatter: fail("SKILL.md description should mention RedM and FiveM", failures) +def check_commands(failures: list[str]) -> None: + # Commands, not nested SKILL.md files: a skill folder may hold only one SKILL.md. + nested = [p for p in ROOT.rglob("SKILL.md") if p != ROOT / "SKILL.md" and ".git" not in p.parts] + for path in nested: + fail(f"Nested SKILL.md breaks skill upload: {path.relative_to(ROOT).as_posix()}", failures) + for command_path in sorted((ROOT / "commands").glob("*.md")): + relative = command_path.relative_to(ROOT).as_posix() + match = re.match(r"^---\n(?P.*?)\n---\n", read_text(command_path), re.DOTALL) + if not match or not re.search(r"^description:\s*\S", match.group("body"), re.MULTILINE): + fail(f"{relative} needs frontmatter with a description", failures) + + +# Hex values and CAPS names in the rules that are not natives (constants, +# placeholders, event/weapon/component names). Everything else must exist in +# references/natives/ so a typo or invented native cannot slip into a rule. +NON_NATIVE_HEX = {"0x7fffffff", "0x1234abcd"} +NON_NATIVE_NAMES = {"REQUEST_CONTROL_EVENT", "NATIVE_NAME", "SAVE_QUERY", "MAX_SAVE_FAILS"} +NON_NATIVE_PREFIXES = ("WEAPON_", "COMPONENT_") + + +def check_native_refs(paths: list[Path], failures: list[str]) -> None: + reference_text = "".join( + read_text(ROOT / "references" / "natives" / name) + for name in ["fivem-gta5-natives.md", "redm-rdr3-natives.md"] + ) + known_hex = {value.lower() for value in re.findall(r"0x[0-9A-Fa-f]+", reference_text)} + known_names = set(re.findall(r"^#+\s+`(_?[A-Z0-9_]+)`", reference_text, re.MULTILINE)) + + for path in paths: + relative = path.relative_to(ROOT).as_posix() + for number, line in enumerate(read_text(path).splitlines(), 1): + for value in re.findall(r"\b0x[0-9A-Fa-f]{8}(?:[0-9A-Fa-f]{8})?\b", line): + lowered = value.lower() + cited = f"docs.fivem.net/natives/?_{lowered}" in line.lower() # CFX native with its doc link + if lowered not in known_hex and lowered not in NON_NATIVE_HEX and not cited: + fail(f"{relative}:{number} hash {value} is not in references/natives/", failures) + for name in re.findall(r"`(_?[A-Z][A-Z0-9]*(?:_[A-Z0-9]+)+)`", line): + if name in known_names or name in NON_NATIVE_NAMES or name.startswith(NON_NATIVE_PREFIXES): + continue + fail(f"{relative}:{number} native {name} is not in references/natives/", failures) + + def check_backticked_paths(markdown_path: Path, failures: list[str]) -> None: text = read_text(markdown_path) for value in re.findall(r"`([^`]+)`", text): @@ -183,6 +236,7 @@ def main() -> int: check_marketplace_json(failures) check_memory_readmes(failures) check_reference_toc(failures) + check_commands(failures) # Validate cross-links (backticked skill/memory/references paths) everywhere # rules live, not only the top-level entry points. Skip the large generated @@ -194,9 +248,14 @@ def main() -> int: cross_link_files = [ROOT / "SKILL.md", ROOT / "README.md", ROOT / "AGENTS.md"] cross_link_files += sorted((ROOT / "skills").rglob("*.md")) cross_link_files += sorted((ROOT / "memory").rglob("*.md")) + cross_link_files += sorted((ROOT / "commands").glob("*.md")) cross_link_files += [ path for path in (ROOT / "references").rglob("*.md") if path not in generated_natives ] + rule_files = [ROOT / "SKILL.md"] + sorted((ROOT / "skills").rglob("*.md")) + rule_files += sorted((ROOT / "memory").rglob("*.md")) + sorted((ROOT / "commands").glob("*.md")) + check_native_refs(rule_files, failures) + for markdown_path in cross_link_files: check_backticked_paths(markdown_path, failures) check_section_references(markdown_path, failures) diff --git a/skills/common/database.md b/skills/common/database.md index 32da0bd..c5acbae 100644 --- a/skills/common/database.md +++ b/skills/common/database.md @@ -32,7 +32,7 @@ Use this file when a RedM/FiveM resource reads or writes SQL, owns persisted pla - Never build SQL by concatenating player input, NUI input, item names, identifiers, job names, or config keys. - Validate and normalize values before passing them as query parameters. - Keep dynamic table/column names out of public input. If dynamic names are unavoidable, choose from a server-side allowlist. -- Prefer `?` placeholders for ordinary OxMySQL queries. Named placeholders (`@name`) are supported too (require the resource's `namedPlaceholders` setting). ([oxmysql](https://overextended.dev/docs/oxmysql)) +- Prefer `?` placeholders for ordinary OxMySQL queries. Named placeholders (`@name`/`:name` with a keyed table) still work and are on unless `mysql_connection_string` sets `namedPlaceholders=false`, but oxmysql marks them deprecated; use `?` in new code. ([oxmysql placeholders](https://overextended.dev/docs/oxmysql/placeholders)) - Backtick fixed table and column names when they may conflict with reserved words. - Check affected row counts for updates that are expected to mutate exactly one row. @@ -49,7 +49,17 @@ MySQL.insert.await("INSERT INTO logs (item) VALUES (?)", { item }) ``` - Bind every value. For a dynamic identifier (table/column name) choose from a server-side allowlist and interpolate only the allowlisted constant - never the raw input. -- Wrap multi-step valuable writes (debit + grant) in one `MySQL.transaction.await({ ... })` so they commit together or roll back together ([oxmysql transaction](https://overextended.dev/docs/oxmysql/Functions/transaction)). See `skills/common/security-performance.md` -> Database And Persistence and Give-Value Event Hardening for the race-safe pattern. +- Wrap multi-step valuable writes in one transaction, but know that `MySQL.transaction` rolls back only when a statement throws ([oxmysql transaction](https://overextended.dev/docs/oxmysql/Functions/transaction)). A conditional `UPDATE ... WHERE money >= ?` that matches 0 rows is not an error: the remaining statements still commit and the call returns `true`. When a conditional statement must gate the grant, use one of these: + - Run the conditional debit alone with `MySQL.update.await`, grant only if it changed exactly 1 row, and refund with a compensating update if the grant fails (wrap the grant in `pcall`, since `.await` raises on an SQL error). + - Use `MySQL.startTransaction`, where returning `false` from the function rolls back. oxmysql marks it experimental: + ```lua + local committed = MySQL.startTransaction(function(query) + local res = query('UPDATE characters SET money = money - ? WHERE owner = ? AND money >= ?', { cost, owner, cost }) + if not res or res.affectedRows ~= 1 then return false end + query('INSERT INTO inventory (owner, item, count) VALUES (?, ?, ?) ON DUPLICATE KEY UPDATE count = count + ?', { owner, item, amount, amount }) + end) + ``` +- See `skills/common/security-performance.md` -> Give-Value Event Hardening (`PurchaseSql`) for the full hardened handler around the conditional debit (validation, throttle, in-flight lock). ## OxMySQL API Shape @@ -63,12 +73,14 @@ MySQL.insert.await("INSERT INTO logs (item) VALUES (?)", { item }) - Prefer `.await` in coroutine-friendly server code when it keeps the flow clearer. - Use callback style when the surrounding project already uses callbacks heavily. - Avoid legacy alias names in new code unless maintaining an existing mysql-async compatibility layer. -- Always handle `nil`, `false`, empty results, and zero affected rows explicitly. +- Handle `nil`/empty results and zero affected rows explicitly, and know how errors surface: + - `MySQL.*.await` (except `MySQL.transaction.await`, which returns `false`) raises a Lua error on SQL failure. Wrap it in `pcall` wherever a lock, in-flight flag, or dirty flag must be restored afterwards. + - Callback-style queries never invoke the callback on error unless the manifest declares `mysql_option 'return_callback_errors'`, in which case the callback receives `(nil, err)` (except `MySQL.transaction`, whose callback always runs and receives `false` on an SQL error). ## Transactions And Atomic Writes - Use a transaction for multi-query valuable state changes such as transfers, purchases, crafting, inventories, ownership changes, and claim rewards. -- Treat a transaction result of `false` as a rejected mutation and do not apply in-memory success state. +- Treat a transaction result of `false` as a rejected mutation and do not apply in-memory success state. A `true` result does not mean every conditional statement matched a row. - Prefer one conditional update over check-then-update when one row can enforce the rule. - For money and inventory, avoid flows that read a balance, yield, and then write a new balance without locking, a transaction, or a conditional update. - Use unique constraints plus `INSERT ... ON DUPLICATE KEY UPDATE` for upsert behavior. @@ -111,7 +123,7 @@ MySQL.insert.await("INSERT INTO logs (item) VALUES (?)", { item }) - Log database failures with resource name, query purpose, source/identifier when relevant, and a bounded error message. - Do not log full payloads containing secrets, tokens, webhooks, passwords, authorization headers, or large JSON blobs. - Use retry windows for transient save failures, but cap retries and keep failure visible. -- Make failed persistence behavior explicit: retry, mark dirty again, reject the action, or roll back in-memory state. +- Make failed persistence behavior explicit: retry, mark dirty again, reject the action, or roll back in-memory state. Put that recovery in a path that actually runs on error: a `pcall` around `.await`, or a callback with `mysql_option 'return_callback_errors'` declared (see OxMySQL API Shape above). - Do not silently create missing critical rows in response to untrusted client input. ## Review Questions diff --git a/skills/common/debugging.md b/skills/common/debugging.md index e524413..66f0283 100644 --- a/skills/common/debugging.md +++ b/skills/common/debugging.md @@ -78,7 +78,7 @@ Use this file when diagnosing RedM/FiveM resource failures, crashes, bad state, - Confirm `ui_page` and every HTML/CSS/JS/font/image file are listed in `fxmanifest.lua`. - Confirm Lua sends the message shape the browser expects. -- Confirm browser callbacks call the Lua callback exactly once. +- Confirm every Lua `RegisterNUICallback` handler calls its `cb` exactly once on every path, including early-return validation failures. A missing `cb` makes the browser `fetch` time out and reject with an error (see `skills/common/nui.md` -> Browser To Lua). - Clear NUI focus on close, player drop, and resource stop. - Treat NUI callback payloads as untrusted input even though they originate from the resource UI. - Use browser devtools for JS errors instead of guessing from Lua logs. @@ -86,10 +86,13 @@ Use this file when diagnosing RedM/FiveM resource failures, crashes, bad state, ## Performance Debugging - Find the hot path before optimizing - measure, do not guess: - - `resmon 1` in the client F8 console shows per-resource frame time (ms) and memory; a resource idling above ~0.10ms or spiking on interaction is the place to look. Recent server artifacts support `resmon` in the server console too. + - `resmon 1` in the client F8 console shows per-resource frame time (ms) and memory; a resource idling above ~0.10ms or spiking on interaction is the place to look (a community rule of thumb, not an engine limit - compare against the same server's baseline, idle and during the action). Server-side `resmon` exists only in the Windows FXServer GUI; on Linux use the server profiler. - `profiler record 500` then `profiler view` (client F8 or server console) captures a tick-level trace of every resource; `profiler saveJSON ` exports it for `chrome://tracing` / Perfetto when the in-game view is not enough. + - `netEventLog 1` (client F8) lists recent net events with direction (`C->S` / `S->C`), name, and payload bytes; state bags are not listed. `netgraph` shows live packets/bytes, `net_statsFile ` logs net stats, and `cl_drawPerf 1` overlays FPS/ping/loss. + - Client `resmon`, `netEventLog`, and `profiler` are developer commands: on the production channel they print "Access denied" until the game is launched with `+set moo 31337`. `cl_drawPerf` works without it. + - Match the tool to the hitch. "server thread hitch warning" means the 50 ms server tick ran late - slow scripts, synchronous loops, heavy handlers; profile the server. A network thread hitch is the network/send loop falling behind - check event and state bag volume with `netEventLog`, and host load; raising an unrelated `Wait` fixes neither. - txAdmin's performance chart shows server tick-time trends over hours - use it to catch slow leaks and load-correlated stalls that a live snapshot misses. -- Measure once before changing anything and once after; an optimization that does not move the number gets reverted. +- Measure once before changing anything and once after, one class of change at a time; an optimization that does not move the number gets reverted. Label every figure as measured or estimated - an estimate must never read like a profiler number. - Look for `Wait(0)` loops, repeated natives, table scans, repeated config requires, N+1 SQL, and spammy NUI messages. - Increase waits based on distance/active state before adding complex caches. - Cache only values that are expensive or hot enough to justify ownership and cleanup. diff --git a/skills/common/frameworks.md b/skills/common/frameworks.md new file mode 100644 index 0000000..498eb51 --- /dev/null +++ b/skills/common/frameworks.md @@ -0,0 +1,143 @@ +# Framework Integration Rules + +Use this file when a RedM/FiveM resource integrates with a player framework: ESX, QBCore, Qbox (FiveM) or VORP, RSG (RedM). b3sty resources stay framework-free by default (`skills/fivem/rules.md` / `skills/redm/rules.md` -> Resource Defaults); add framework code only when the project already runs one. + +Framework APIs change between versions. The shapes below match current mainstream releases; confirm each call against the installed framework's source before relying on it, and record verified differences in `memory/`. + +## Defaults + +- Keep every framework call behind a small bridge (`bridge/server.lua`, `bridge/client.lua`) so gameplay code calls `Bridge.RemoveMoney(...)`, never `xPlayer`/`Player.Functions` directly. This is the "small local adapter" from the game rules files, not a framework of its own. +- Only the server bridge may touch money, items, jobs, or saved character data. The client bridge only reads display data (name, job label) and listens to lifecycle events. +- Fetch the player object inside each server call; do not cache player objects across yields or keep them after `playerDropped`. +- Treat client-side framework data (`ESX.PlayerData`, `QBCore.Functions.GetPlayerData()`, VORP client character cache) as display-only. Every decision about money, items, jobs, or permissions re-reads server-side state. +- Key saved per-character data by the framework's character ID, not by `license`/`license2`: one license can own several characters. + +## Core Objects And Player Lookup + +| Framework | Core object (server) | Player lookup | Character ID | +|---|---|---|---| +| ESX Legacy | `exports.es_extended:getSharedObject()` | `ESX.GetPlayerFromId(source)` | `xPlayer.identifier` | +| QBCore | `exports['qb-core']:GetCoreObject()` | `QBCore.Functions.GetPlayer(source)` | `Player.PlayerData.citizenid` | +| Qbox | none needed | `exports.qbx_core:GetPlayer(source)` | `player.PlayerData.citizenid` | +| VORP | `exports.vorp_core:GetCore()` | `Core.getUser(source).getUsedCharacter` | `character.charIdentifier` | +| RSG | `exports['rsg-core']:GetCoreObject()` | `RSGCore.Functions.GetPlayer(source)` | `Player.PlayerData.citizenid` | + +- Every lookup returns `nil` for a connected player whose character is not loaded yet (character select, spawn screen). Nil-check before indexing. VORP is the exception: `getUser` returns `nil` only for an unknown source, and `getUsedCharacter` is an empty table `{}` until a character is selected, so check `character.charIdentifier` instead. +- Qbox ships a `qb-core` compatibility layer, so `GetResourceState('qb-core')` can look started on a Qbox server. Detect `qbx_core` first. +- RSG is a QBCore fork: same `PlayerData`/`Functions` shape with `RSGCore` names. Qbox player objects also expose `PlayerData` and `Functions`. +- VORP `getUsedCharacter` is a field, not a method: `user.getUsedCharacter`, no call parentheses. + +## Money + +- Check the balance and debit in the same synchronous block with no yield in between (no `Wait`, `.await`, callback, or export that yields). CfxLua runs one handler at a time, so check-then-debit without a yield cannot race another event. +- Do not trust the framework's remove call to refuse an overdraft: + - ESX `xPlayer.removeMoney(amount, reason)` / `removeAccountMoney(account, amount, reason)`: check `getMoney()` / `getAccount(name).money` first. + - QBCore/Qbox/RSG `Functions.RemoveMoney(type, amount, reason)` returns `false` on refusal, but only the types in `DontAllowMinus` are protected from going negative (defaults: QBCore/Qbox `cash`, `crypto`; RSG `cash`, `gold`, `bloodmoney`). `bank` can go below zero (down to `Config.Money.MinusLimit`, default `-5000`, in QBCore/RSG; unlimited in Qbox). Check `PlayerData.money[type]` first and still honor the return value. + - VORP `character.removeCurrency(0, amount)` (`0` money, `1` gold, `2` role token) subtracts without any check and returns nothing: check `character.money` / `character.gold` first. +- Pass a reason string on every add/remove where the framework accepts one; it feeds the framework's own money logs. Keep your resource's audit log as well (`skills/common/security-performance.md` -> Logging And Secrets). +- The full give-value checklist (validation, throttle, in-flight lock, server-side price) still applies: `skills/common/security-performance.md` -> Give-Value Event Hardening. + +## Items + +- Use the inventory the server actually runs; frameworks differ (ESX built-in or ox_inventory, QBCore qb-inventory or ox_inventory, Qbox ox_inventory, VORP vorp_inventory, RSG rsg-inventory). +- Check capacity before taking payment, then grant, and handle a failed grant by refunding in the same code path. With ox_inventory: `exports.ox_inventory:CanCarryItem(source, name, count)` then `exports.ox_inventory:AddItem(source, name, count)`, which returns `false` plus a reason on failure. +- Item names come from server config or the framework item list, never from the client payload directly; look the name up in your own index first. + +## Jobs And Permissions + +- Read job and grade from the server player object: ESX `xPlayer.job.name` / `xPlayer.job.grade`, QBCore/Qbox/RSG `PlayerData.job.name` / `PlayerData.job.grade.level` / `PlayerData.job.onduty`, VORP `character.job` / `character.jobGrade`. +- Check on-duty state where the framework has it; an off-duty officer should not pass a police-only check. +- Staff/admin actions use ACE (`IsPlayerAceAllowed`), not framework groups alone, unless the project already standardizes on framework groups: `skills/common/security-performance.md` -> ACE Permissions And Admin Authority. + +## Lifecycle Events + +| Framework | Character loaded (server) | Character loaded (client) | Unloaded / logout | +|---|---|---|---| +| ESX | `esx:playerLoaded` (`playerId, xPlayer, isNew`) | `esx:playerLoaded` (`playerData, isNew, skin`; plain data table, not an xPlayer object) | server `esx:playerDropped` (`playerId, reason`) | +| QBCore / Qbox | `QBCore:Server:PlayerLoaded` (`Player`) | `QBCore:Client:OnPlayerLoaded` | server `QBCore:Server:OnPlayerUnload` (`source`), client `QBCore:Client:OnPlayerUnload` | +| VORP | `vorp:SelectedCharacter` (`source, character`) | `vorp:SelectedCharacter` (`charId`) | `playerDropped` | +| RSG | `RSGCore:Server:PlayerLoaded` (`Player`) | `RSGCore:Client:OnPlayerLoaded` | `RSGCore:Server:OnPlayerUnload` / `RSGCore:Client:OnPlayerUnload` | + +- Framework server lifecycle events are server-local. Listen with `AddEventHandler`, never `RegisterNetEvent`: registering them as net events lets any client fake a "character loaded" call into your handler. +- Character switch (multicharacter logout) is not `playerDropped`. Clear per-character state on the unload event as well as on `playerDropped`. +- A restart of your resource does not replay "loaded" events. On `onResourceStart` for your own resource, initialize every already-loaded player (ESX `ESX.GetExtendedPlayers()`, QBCore `QBCore.Functions.GetQBPlayers()`, Qbox `exports.qbx_core:GetQBPlayers()`, RSG `RSGCore.Functions.GetRSGPlayers()`; for VORP loop `GetPlayers()` and skip users whose `getUsedCharacter.charIdentifier` is `nil`). + +## Bridge Pattern + +Detect once at load, fail loudly if nothing matches, and expose a few narrow functions that return explicit success values. + +```lua +-- bridge/server.lua (first entry in server_scripts) +Bridge = {} + +if GetResourceState("qbx_core") == "started" then + Bridge.Name = "qb" + Bridge.GetPlayer = function(source) return exports.qbx_core:GetPlayer(source) end +elseif GetResourceState("qb-core") == "started" then + local QBCore = exports["qb-core"]:GetCoreObject() + Bridge.Name = "qb" + Bridge.GetPlayer = function(source) return QBCore.Functions.GetPlayer(source) end +elseif GetResourceState("rsg-core") == "started" then + local RSGCore = exports["rsg-core"]:GetCoreObject() + Bridge.Name = "qb" + Bridge.GetPlayer = function(source) return RSGCore.Functions.GetPlayer(source) end +elseif GetResourceState("es_extended") == "started" then + local ESX = exports.es_extended:getSharedObject() + Bridge.Name = "esx" + Bridge.GetPlayer = function(source) return ESX.GetPlayerFromId(source) end +elseif GetResourceState("vorp_core") == "started" then + local Core = exports.vorp_core:GetCore() + Bridge.Name = "vorp" + Bridge.GetPlayer = function(source) + local user = Core.getUser(source) + local character = user and user.getUsedCharacter + return character and character.charIdentifier and character or nil + end +else + error(("%s: no supported framework started - check server.cfg ensure order"):format(GetCurrentResourceName())) +end + +function Bridge.GetCharId(source) + local player = Bridge.GetPlayer(source) + if not player then return nil end + + if Bridge.Name == "qb" then return player.PlayerData.citizenid end + if Bridge.Name == "esx" then return player.identifier end + return player.charIdentifier +end + +-- Cash only. No yield between the balance check and the debit. +function Bridge.RemoveCash(source, amount, reason) + local player = Bridge.GetPlayer(source) + if not player then return false end + + if Bridge.Name == "qb" then + if player.PlayerData.money.cash < amount then return false end + return player.Functions.RemoveMoney("cash", amount, reason) + end + + if Bridge.Name == "esx" then + if player.getMoney() < amount then return false end + player.removeMoney(amount, reason) + return true + end + + if player.money < amount then return false end + player.removeCurrency(0, amount) + return true +end +``` + +- The core-object lookups run at load time, so the framework must be started before this resource: put it earlier in `server.cfg`, or add a `dependency` line when the resource supports only one framework. A framework restart restarts dependents; see `skills/common/runtime.md` -> Exports And Stale References for resources that outlive a provider restart. +- `Bridge` is a resource-scoped global so other server files in the same resource can call it; it is not visible to other resources. +- Add only the functions this resource needs (`RemoveCash`, `AddItem`, `HasJob`). A bridge that mirrors the whole framework API is overengineering (`skills/common/security-performance.md` -> Anti-Overengineering). + +## Review Questions + +- Does any gameplay file call a framework API directly instead of the bridge? +- Is every framework lookup nil-checked for players who have not loaded a character? +- Is the balance checked before every debit, with no yield between check and debit, and is the remove call's result honored? +- Are framework lifecycle events registered with `AddEventHandler` only? +- Is per-character state cleared on unload/character switch, not only on `playerDropped`? +- Does a restart of this resource re-initialize players who are already loaded? +- Is saved data keyed by character ID rather than license? diff --git a/skills/common/fxserver.md b/skills/common/fxserver.md index f042719..e9a89eb 100644 --- a/skills/common/fxserver.md +++ b/skills/common/fxserver.md @@ -7,7 +7,7 @@ - Avoid loading unused files. - Use `fx_version 'cerulean'`. - Use `lua54 'yes'`. -- Note: `lua54 'yes'` selects the Lua 5.4 runtime, but FXServer's build also adds CfxLua extensions on top of it - the compound assignment operators and bitwise operators. Those operators are CfxLua-only and must not be used in standard Lua or standalone Lua tooling. See `skills/common/style.md` -> CfxLua Syntax for the full list. +- Note: `lua54 'yes'` selects Lua 5.4 (bitwise `&`, `|`, `~`, `<<`, `>>` are standard). CfxLua adds compound assignment operators and backtick hashes on top - CfxLua-only, never in standard Lua or standalone tooling; see `skills/common/style.md` -> CfxLua Syntax. - Use `games { 'rdr3', 'gta5' }` when the resource supports both RedM and FiveM. - For FiveM-only manifests, apply `skills/fivem/rules.md`. - For RedM-only manifests, apply `skills/redm/rules.md`. diff --git a/skills/common/native-usage.md b/skills/common/native-usage.md index 07faf0e..c64b79d 100644 --- a/skills/common/native-usage.md +++ b/skills/common/native-usage.md @@ -108,17 +108,18 @@ Read as four passes over the doc name, applied identically to every native in bo Worked examples: - `GET_ENTITY_HEALTH` → `GetEntityHealth(entity)`. -- `ACTIVATE_TIMECYCLE_EDITOR` → `ActivateTimecycleEditor`. +- `SET_PED_AMMO_BY_TYPE` → `SetPedAmmoByType(ped, ammoType, ammo)`. - `_ACTIVATE_COVER_LAYER` → `ActivateCoverLayer(coverLayer)` (the leading `_` collapses the same as any other). - Hash-only heading (no friendly name, e.g. `0x1234ABCD...`) → `N_0x1234abcd...` (step 2 turns `0x` into `n_0x` before capitalization; the identifier must start with a letter). This global exists, but calling by explicit hash with a `--[[NAME]]` comment reads better in review. Edge case — **an underscore followed by a digit does not collapse**, because the pattern only matches a following letter (`%a`), not a digit. A native with a trailing `_2`-style segment keeps the underscore in the Lua name (for example a hypothetical `..._INDEX_2` becomes `...Index_2`, not `...Index2`). Check the actual global in an F8/server console with `type(FunctionName)` when a name has a digit segment and the direct call is `nil`. -- If the expected global is still `nil` at runtime after accounting for the digit edge case, the running artifact's codegen predates the name (renamed/newer native); fall back to `Citizen.InvokeNative` with the doc hash, which always works. +- If the expected global is still `nil` at runtime after accounting for the digit edge case, the running artifact's codegen predates the name (renamed/newer native); fall back to `Citizen.InvokeNative` with the doc hash. That works on the client when the running game build has the native. It does not work on the server: FXServer registers no game hashes (its server-side natives, including the OneSync entity subset, use CFX hashes), so a hash from these references hits no handler and silently returns `false`/nothing. - A native can have multiple registered names (`aliases` in the codegen); each alias name goes through this same conversion and becomes its own valid global for the same underlying call. The generated reference files here do not enumerate aliases — if a project uses a different name than the doc heading for what looks like the same native, verify the hash matches before assuming it is unrelated. - When calling by hash, keep the doc name beside it so the call stays greppable against the reference: ```lua - local health = Citizen.InvokeNative(0x82368787EA73C0F7 --[[GET_ENTITY_HEALTH]], entity) + -- RDR3 hash (GTA V: 0xEEF059FAD016D209) + local health = Citizen.InvokeNative(0x82368787EA73C0F7 --[[GET_ENTITY_HEALTH]], entity, Citizen.ResultAsInteger()) ``` ## Type Mapping @@ -129,7 +130,7 @@ Edge case — **an underscore followed by a digit does not collapse**, because t | `Hash` | integer hash — see Hashes | | `int` | integer number | | `float` | float number — see InvokeNative Pitfalls | -| `BOOL` | `true`/`false` on named natives; `1`/`0` from `Citizen.InvokeNative` | +| `BOOL` | non-zero integer (usually `1`) or `false` from named natives and from a bare `Citizen.InvokeNative` (0 becomes `false`); `1`/`0` only when `Citizen.ResultAsInteger()` is appended | | `char*` (param) | Lua string | | `char*` (return) | Lua string or `nil` — guard before use | | `float x, float y, float z` | three separate numbers | @@ -154,11 +155,11 @@ Edge case — **an underscore followed by a digit does not collapse**, because t ## Named Native Calls And Out Params -- Named natives are plain global functions; `BOOL` params take `true`/`false` and `BOOL` returns are real booleans. -- Output pointer params (`int*`, `float*`, `Vector3*`) do not appear in the Lua argument list. They come back as extra return values after the primary return, in declaration order: +- Named natives are plain global functions. `BOOL` params take `true`/`false`. `BOOL` returns come back as `1`/`false`, not real booleans: use them directly in `if`/`not` and never compare `== true`. +- Output pointer params (`int*`, `float*`, `Vector3*`) usually do not appear in the Lua argument list (exception: when a native's only pointer is its last param, the generated function keeps a positional slot for it - an optional initial value for `int*`/`float*`, see the caveat under Calling By Hash). They come back as extra return values after the primary return, in declaration order: ```lua -- BOOL GET_GROUND_Z_FOR_3D_COORD(float x, float y, float z, float* groundZ, BOOL includeWater) - local found, groundZ = GetGroundZFor_3dCoord(x, y, z, false) + local found, groundZ = GetGroundZFor_3dCoord(x + 0.0, y + 0.0, z + 0.0, false) ``` - When a native is void with output pointers, the outputs are the only return values. - Some upstream signatures still show an initial value being passed for an out param; when the runtime asks for it, pass a sane default (usually `0` or `0.0`). @@ -167,48 +168,53 @@ Edge case — **an underscore followed by a digit does not collapse**, because t `Citizen.InvokeNative(hash, args...)` invokes any native by its identity hash. Marshal results explicitly: -- `int`/`BOOL` results return by default; `BOOL` arrives as `1`/`0` (see pitfalls). +- With no `ResultAs*` marker, a zero result becomes `false` and a non-zero result an integer (see pitfalls). Append `Citizen.ResultAsInteger()` to any int result that can legitimately be 0. - `float` result: append `Citizen.ResultAsFloat()`. - `char*` result: append `Citizen.ResultAsString()`. - `Vector3` result: append `Citizen.ResultAsVector()`. - 64-bit results (rare): `Citizen.ResultAsLong()`. -- If a result comes back `nil` on an older artifact, append `Citizen.ReturnResultAnyway()`. +- When you pass any `Citizen.PointerValue*` marker and also want the native's own return value, append `Citizen.ReturnResultAnyway()` or the matching `Citizen.ResultAs*`. Without it, only the pointer outputs are returned. ```lua -- int GET_ENTITY_HEALTH(Entity entity) — RDR3 0x82368787EA73C0F7 local health = Citizen.InvokeNative(0x82368787EA73C0F7 --[[GET_ENTITY_HEALTH]], entity, Citizen.ResultAsInteger()) ``` -Output pointers by hash use placeholder markers; outputs return after the primary return value, in order: +Output pointers by hash use placeholder markers. The outputs come back in order after the primary return value only if you also pass `Citizen.ReturnResultAnyway()` or a `Citizen.ResultAs*`. Otherwise the outputs are the only return values: - `Citizen.PointerValueInt()`, `Citizen.PointerValueFloat()`, `Citizen.PointerValueVector()` for plain out params. - `Citizen.PointerValueIntInitialized(v)` / `Citizen.PointerValueFloatInitialized(v)` when the native reads the value before writing it. +- Caveat: in current CfxLua, `Citizen.PointerValueFloatInitialized(v)` ignores `v` and starts the pointer at 0.0 (upstream bug in `Lua_GetPointerField`). This also affects named natives whose only pointer is a trailing `float*` (codegen passes it through `PointerValueFloatInitialized`). For in/out float params, verify in game, or pass a writable buffer (`string.blob(string.pack(' Performance Rules; how to measure is in `skills/common/debugging.md` -> Performance Debugging. Security rules still win: never drop a server check to save bytes. + +## Cost Model + +- Every net event costs roughly `(name + packed args) x sends per second x recipients`, plus handler CPU on every receiver. The event name travels with every send. +- A tiny payload is still Critical when it fires from a per-frame loop: one client at `Wait(0)` sends dozens of events a second, hits the `netEvent` limit, and a hundred clients turn that into thousands for the server to handle. +- `Wait(0)` means "next frame", not a fixed rate. Any events-per-second figure not taken from `netEventLog` or a profiler is an estimate - say so. +- A resource can sit at `0.00ms` in `resmon` and still flood the network, or send nothing and burn CPU. Check both before calling a resource optimized. + +## Rate Limits + +FXServer keeps per-client token buckets (tune with `set rateLimiter__rate` / `_burst`). Fix the sender; raising limits only hides a spammy resource. + +| Limiter (rate / burst) | Over the limit | +|---|---| +| `netEvent` (50 / 200 events) | Event silently dropped | +| `netEventFlood` (75 / 300) | Also empty: kick "Reliable network event overflow." | +| `netEventSize` (128 KiB / 384 KiB of name + payload) | Kick "Reliable network event size overflow: " | +| `stateBag` (75 / 125 updates) | Update dropped, logged on `sbag-update-dropped` | +| `stateBagFlood` (150 / 175) | Also exceeded: kick "Reliable state bag packet overflow." | +| `stateBagSize` (128 KiB / 256 KiB) | Kick | +| `latentEvent` (75 / 125) / `latentEventFlood` (150 / 175) | Fragment dropped / kick "latent event packet overflow." | + +- A silently dropped event looks like a random gameplay bug ("sometimes the item never arrives"). When a handler "sometimes doesn't fire", check the sender's rate before the handler's logic. +- The overflow kick is count-based: too many events, not too many bytes. Find the event in `netEventLog` and stop the flood at the source. + +## Choosing The Channel + +| Need | Use | +|---|---| +| Same machine, two scripts or resources | `TriggerEvent` or an export - never a server round-trip to talk to yourself | +| Small, latency-sensitive action (use item, open door, fire) | `TriggerServerEvent` / `TriggerClientEvent` | +| Visible state other clients render (cuffed, fuel, outfit id) | State bag + `AddStateBagChangeHandler` | +| Large one-off transfer (catalog, outfit pack, bulk sync) | `TriggerLatentClientEvent` / `TriggerLatentServerEvent` | +| Lua to the NUI page | `SendNUIMessage` - no game network, but JSON and CEF cost show in `resmon` | + +- Request/response callbacks (`lib.callback`, framework callbacks) are two net messages per call. Same payload, throttle, and validation rules as events. + +## Latent Events + +- Latent events split a large payload into fragments and send them at a capped rate (`bps`, default 25000 when `0` or negative) so the transfer does not choke the connection. Use them for multi-KB transfers, never for small gameplay actions where the added latency matters. +- `bps` is per recipient: a latent event to `-1` sends about `bps x players` from the server. Very high values (around 10,000,000) defeat the point and cause connection problems. +- Latent events ride on net event reassembly (`sv_enableNetEventReassembly`, on by default); a server that turned it off cannot send them. +- A normal client-to-server event is capped at 384 KiB per packet and by `netEventSize`; server-to-client events have no hard cap, so a huge normal event can stall that client long enough to time it out. That is the case latent events exist for. + +## Payload Shape + +- Net event arguments are packed with msgpack. Calling `json.encode` before sending adds CPU and a larger string on top of that; send the table. +- Send only the fields the receiver reads. Sending a whole player (job, inventory, weapons) to update coords is the classic waste. +- Never send entity handles; send net IDs and resolve them on the other side (`skills/common/networking.md` -> Handles And Net IDs). +- Build the payload when state changes, not every tick. An unchanged table encoded every frame is pure waste. + +## Snapshot Then Deltas + +The expensive mistake is fetching the same full data again and again. + +- Send full data once per cache lifetime: static catalogs at resource start, player data when the character loads, UI data the first time it is needed this session. +- Keep it where it is read: server RAM for authority (`skills/common/database.md` persistence pattern), a client table for display. +- After the snapshot, send only what changed (`itemChanged(name, count)`), not the whole inventory. +- Reopening a UI is not a reason to ask the server again: the client already holds the snapshot and pushes it to NUI locally. +- A server callback that runs `SELECT` on every menu open is a full fetch on a path that should read RAM. +- Invalidate on player drop, character switch (load a fresh snapshot rather than merging into the old one), and resource stop. +- The client copy is display-only; the server still validates every action against its own state. + +## Audience + +- Pick the smallest audience that needs the data: the one player, then server-computed nearby players, then the routing bucket, then a job/role set, and `-1` only when every client truly needs it. Details: `skills/common/networking.md` -> Broadcasting And Scoped Messages. +- `-1` ignores routing buckets: players in other instances receive it too. +- Batch many small updates that happen in the same short window into one event when the extra delay does not change gameplay. Never batch or throttle hits, shots, or anything where arrival order decides an outcome. +- Throttle non-critical streams (HUD position, idle stats) by time (`GetGameTimer()` gate), not by frame. + +## State Bags + +- Server writes replicate; client writes stay local unless set with `state:set(key, value, true)`, and strict mode (`setr sv_stateBagStrictMode true`) blocks client writes entirely. +- Bags are shallow: `Entity(x).state.data.field = v` does not replicate. Reassign the whole value or use a flat key such as `state['data:field']`. +- Each read returns a fresh copy of the value. Read a large value once per tick into a local, and keep large blobs out of bags - use a latent event. +- React with `AddStateBagChangeHandler` instead of polling a bag in a loop. + +## Review Questions + +- Does any `Trigger*Event` run inside a per-frame or short-wait loop? +- Does any event go to `-1` when a smaller audience would do? +- Is the full dataset fetched more than once per cache lifetime? +- Are payloads trimmed to what the receiver reads, with no extra `json.encode`? +- Are multi-KB transfers latent, and small gameplay events not latent? +- Are state bag values small, flat, and written by the server? +- Was the change measured with `netEventLog` before and after, or labeled as an estimate? diff --git a/skills/common/networking.md b/skills/common/networking.md index 3fa4446..f5e5bff 100644 --- a/skills/common/networking.md +++ b/skills/common/networking.md @@ -20,7 +20,7 @@ This file covers the multiplayer/OneSync layer. Native marshalling is in `skills ## Sides And Authority -- The server is authoritative for world decisions: who owns an entity, which routing bucket a player is in, whether an entity may exist, and which clients receive a broadcast. +- The server is authoritative for world decisions: which routing bucket a player is in, whether an entity may exist, which clients receive a broadcast, and ownership-relevant outcomes (who may use, claim, or delete an entity). Under OneSync, clients own and simulate networked entities and FXServer migrates that ownership; server scripts can read the owner (`NetworkGetEntityOwner`) but not assign it. See Entity Ownership. - `IsDuplicityVersion()` returns `true` on the server. Use it in shared scripts to branch by side. - Server-side game natives are a limited OneSync subset (entity getters/setters, player state). Most game natives are client-only; see `skills/common/native-usage.md` -> Client, Server, And Shared Context. - Known server-side native quirks live in `memory/`: RedM `GetEntityHealth` returning `0`, `GetVehiclePedIsIn` returning the last vehicle, `GetEntityModel` returning `0` during `entityCreating`. Check `memory/common/native-bugs.md` and `memory/redm/native-bugs.md` before building server logic on a native read. @@ -39,12 +39,12 @@ This file covers the multiplayer/OneSync layer. Native marshalling is in `skills ## Entity Ownership -- OneSync gives each networked entity an owning client. `NetworkGetEntityOwner(entity)` returns that owner's player source on the server. ([NetworkGetEntityOwner](https://docs.fivem.net/natives/?_0x526FEE31)) +- OneSync gives each networked entity an owning client when one is in scope. `NetworkGetEntityOwner(entity)` returns the owning player's server ID on the server, or `-1` when no client owns the entity (server-owned/orphaned). Handle `-1` before treating the result as a player. ([NetworkGetEntityOwner](https://docs.fivem.net/natives/?_0x526FEE31)) - Ownership migrates: when the owner leaves scope or the game rebalances, ownership can move to another client. Do not assume the creator stays the owner. - Code that writes to a networked entity should usually run on, or be requested by, its owner. The server can also set state directly via its OneSync native subset. - The server decides ownership-relevant outcomes (deletion, locking, damage eligibility), not the client that happens to own the entity at that instant. - For valuable actions tied to an entity, validate ownership/eligibility server-side against a registry, not against a client claim. -- **Ownership migration is an attack surface.** A client requesting control of an entity it does not own (`REQUEST_CONTROL_EVENT`, used to delete/clone/hijack vehicles and objects, or fire explosions by proxy) is the classic entity exploit. Mitigate at the server level with `sv_filterRequestControl` (modes 0-4; mode `4` does not route `REQUEST_CONTROL_EVENT` at all) and re-validate ownership against a server registry before honoring a client request. ([Server Commands](https://docs.fivem.net/docs/server-manual/server-commands/)) +- **Ownership migration is an attack surface.** A client requesting control of an entity it does not own (`REQUEST_CONTROL_EVENT`, used to delete/clone/hijack vehicles and objects, or fire explosions by proxy) is the classic entity exploit. Mitigate on FiveM with `sv_filterRequestControl`: `0` off (default), `1` blocks requests for player-controlled entities (currently occupied vehicles) older than `sv_filterRequestControlSettleTimer` (default `30000` ms), `2` blocks all player-controlled entities, `-1` acts like `2` and warns in console, `3` also blocks settled non-player entities, `4` does not route `REQUEST_CONTROL_EVENT` at all. Any non-zero mode also blocks cross-bucket control requests and senders in strict entity lockdown. Entities flagged server-side with `SetEntityIgnoreRequestControlFilter(entity, true)` bypass every mode and the strict-lockdown check (the cross-bucket check still applies). RedM servers accept the convar, but the filter is compiled only for the GTA V server state and has no effect there, so re-validate ownership against a server registry before honoring a client request on both games. ([Server Commands](https://docs.fivem.net/docs/server-manual/server-commands/)) - Treat client "spawn" or "give me this loot/pickup/vehicle" requests like any give-value request: the server decides what spawns, who owns it, and who may claim it. See `skills/common/security-performance.md` -> Give-Value Event Hardening and Entity And Net ID Validation. ## Routing Buckets @@ -57,7 +57,8 @@ This file covers the multiplayer/OneSync layer. Native marshalling is in `skills ``` - Read with `GetPlayerRoutingBucket(player)` and `GetEntityRoutingBucket(entity)`. Default bucket is `0`. - Players keep their bucket across reconnects only if the server re-applies it; store intended bucket in server/persistence state and reapply on join. -- Spawn instance content server-side and place it in the matching bucket so it is scoped correctly. Entities spawned client-side do not automatically follow a player's bucket. +- Spawn instance content server-side and place it in the matching bucket so it is scoped correctly. +- An entity gets its creating client's routing bucket when it is created, but it does not follow later `SetPlayerRoutingBucket` moves (only the player's own ped moves). Move owned vehicles/props explicitly with `SetEntityRoutingBucket`. Always set the bucket right after server-side creation (once `DoesEntityExist` is true), because RPC creation may run on a nearby client in a different bucket. - Validate that an entity a client references is in the same bucket as that player before acting on it; cross-bucket references are a common exploit/teleport vector. - Clear/reset bucket assignments on instance end, player drop, and resource stop so players are not left in an empty private world. @@ -72,7 +73,7 @@ This file covers the multiplayer/OneSync layer. Native marshalling is in `skills ## Player Scope - OneSync streams entities to clients based on scope. A client only receives data for entities in its scope. -- `playerEnteredScope` and `playerLeftScope` fire with `{ for = playerId, player = playerId }`: another player entered/left the listener's scope. Use them for proximity setup/teardown, not for authority. +- `playerEnteredScope` / `playerLeftScope` receive `data` with string fields: `data["for"]` (the player whose scope changed) and `data.player` (the player who entered/left). `for` is a Lua keyword, so use bracket access, and convert with `tonumber(...)` before comparing with server IDs. These events cost more as player count grows (the docs discourage them). Prefer state bags for scoped replication, and never use them for authority. - Scope events are informational; re-validate any position, distance, or "is near" claim server-side before valuable outcomes. ## Entity Lifecycle Events @@ -87,26 +88,28 @@ This file covers the multiplayer/OneSync layer. Native marshalling is in `skills These are triggered by the game/client toward the server. They are part of FiveM/OneSync (documented at [server-events](https://docs.fivem.net/docs/scripting-reference/events/server-events/)) and are client-callable, so treat every one as untrusted input. The full hardened treatment (validation, rate-limit, cancel nuance, logging) is in `skills/common/security-performance.md` -> Built-in Client Events. -- `weaponDamageEvent(sender, data)` — client-reported weapon damage. `data.weaponType`, `data.weaponDamage`, `data.hitGlobalId(s)`, `data.willKill`, `data.silenced`, etc. `data.weaponType` has been reported to carry incorrect hashes (citizenfx/fivem#3827); validate against server-side weapon/entity state, not the field alone. +- `weaponDamageEvent(sender, data)` — client-reported weapon damage. `data.weaponType`, `data.weaponDamage`, `data.hitGlobalId(s)`, `data.willKill`, `data.silenced`, etc. `data.weaponType` is an unsigned 32-bit hash, while `GetHashKey`/backticks return signed values (citizenfx/fivem#3827: `2982836145` vs `-1312131151` for `weapon_rpg`), so normalize before comparing: `local h = data.weaponType; if h > 0x7FFFFFFF then h = h - 0x100000000 end`. It is still client-reported; validate against server-side weapon/entity state, not the field alone. - `startProjectileEvent(sender, data)` — projectile/thrown creation. Validate `weaponHash`, `projectileHash`, owner, and origin before trusting it. -- `ptFxEvent(sender, data)` — particle effect playback. Cheap to abuse; rate-limit and bound if it can be used to grief or reveal state. -- `removeAllWeaponsEvent(sender, data)` — `data.pedId`; confirm the sender owns/controls that ped before honoring it. +- `ptFxEvent(sender, data)` (FiveM only) — particle effect playback. Cheap to abuse; rate-limit and bound if it can be used to grief or reveal state. +- `removeAllWeaponsEvent(sender, data)` (FiveM only): `data.pedId` is the network ID of a ped owned by *another* player; the event fires because the sender does not own it. Resolve it with `NetworkGetEntityFromNetworkId(data.pedId)`, and call `CancelEvent()` unless a server-authorized flow (for example an ACE-gated police/admin action) expects this sender to disarm that ped. If nothing legitimate uses it, block it with `block_net_game_event "REMOVE_ALL_WEAPONS_EVENT"`. - `explosionEvent(sender, ev)` — interceptable server-side with OneSync: `AddEventHandler('explosionEvent', function(sender, ev) ...)`; `ev` has `explosionType`, `posX/Y/Z`, `damageScale`, `ownerNetId`. `CancelEvent()` here stops the server from routing the explosion to other clients ([cookbook](https://docs.fivem.net/docs/cookbook/2019/08/19/onesync-intercepting-game-events-such-as-explosions/)). Note `CancelEvent()` does not stop other handlers and is not reliable for already-applied damage. +- RedM raises `weaponDamageEvent`, `explosionEvent`, `startProjectileEvent`, `respawnPlayerPedEvent`, `lightningEvent`, `clearPedTasksEvent` (`data.pedId`, `data.immediately`), `endLootEvent` (`data.targetId`), and the carriable events (`pickupCarriableEvent`, `placeCarriableOntoParentEvent`, `sendCarriableUpdateCarryStateEvent`, `carriableVehicleStowStartEvent`, `carriableVehicleStowCompleteEvent`). Loot and carriable events are the RedM dupe surface; validate them against server registry state. -Networked-event abuse is best mitigated with the server convars: `sv_filterRequestControl` (`REQUEST_CONTROL_EVENT`), `sv_enableNetworkedPhoneExplosions` (off by default), `sv_enableNetworkedSounds`, `sv_enableNetworkedScriptEntityStates`, and `block_net_game_event "EVENT_NAME"` ([Server Commands](https://docs.fivem.net/docs/server-manual/server-commands/)). +Networked-event abuse is best mitigated with the server convars: `sv_filterRequestControl` (FiveM only; `REQUEST_CONTROL_EVENT`), `sv_enableNetworkedPhoneExplosions` (off by default), `sv_enableNetworkedSounds`, `sv_enableNetworkedScriptEntityStates`, and `block_net_game_event "EVENT_NAME"` ([Server Commands](https://docs.fivem.net/docs/server-manual/server-commands/)). For all built-in events: - Read `sender` as the reporting player; never let `sender`-controlled fields grant authority. +- `sender` arrives as a string (the player's server ID); convert with `tonumber(sender)` before numeric comparisons or using it as a table key. - Validate, bound, and rate-limit. Reject, cancel, or log; do not auto-punish on a single unverified event. - Log security-relevant ones with bounded fields and rate-limit the logging. ## Creating Networked Entities - Decide networked vs local at creation. Networked = shared gameplay state (pickups, loot, storage, placed world objects, blockers, owned persistent objects). Local = cosmetic/preview/attached/render-only. See `skills/common/resource-structure.md` -> Local Vs Networked Props. -- Create shared gameplay entities server-side (`CreateObject(...)`, etc.) so ownership and routing bucket are server-controlled. Keep a server registry of created entities. -- For client-created networked entities, the creating client becomes the initial owner; re-claim ownership server-side if the server must control it. -- Preload the model before creation and release it after with `SetModelAsNoLongerNeeded`; see `skills/common/native-rules.md` -> Performance. +- Spawn shared gameplay entities from the server (`CreateObject`/`CreateVehicle`/`CreatePed` RPC natives, or `CreateVehicleServerSetter` on FiveM only) so the server decides what spawns and keeps the handle in a registry. This does not make ownership server-controlled: RPC creation is executed by the nearest client, which becomes the initial owner, and ownership migrates with scope. Once `DoesEntityExist(entity)` is true, set the routing bucket explicitly with `SetEntityRoutingBucket(entity, bucket)`. +- For client-created networked entities, the creating client is the initial owner (`NetworkGetFirstEntityOwner`). No server native can take or assign ownership. If the server must control the entity, reject it in `entityCreating` and spawn a server-side replacement, or gate every outcome through the server registry. +- For client-side creation, preload the model (`RequestModel` + `HasModelLoaded`) and release it with `SetModelAsNoLongerNeeded`. Server-side creation takes the model hash directly and needs no model loading. See `skills/common/native-rules.md` -> Performance. - Keep cleanup for every networked entity: delete on state end, player drop, and resource stop; guard deletes with `DoesEntityExist`. ## Debugging @@ -119,7 +122,7 @@ For all built-in events: ## Review Questions -- Is the server deciding ownership, buckets, broadcasts, and creation, or is a client claim being trusted? +- Is the server deciding buckets, broadcasts, creation, and ownership-relevant outcomes (use, claim, delete), or is a client claim (including whoever currently owns the entity) being trusted? - Are client-provided handles and net IDs validated (type, existence, model, owner, bucket, distance) before use? - Are players and their entities moved into the same routing bucket, and reset on instance end? - Is any `TriggerClientEvent(name, -1)` broadcast justified, or should it be scoped/targeted? diff --git a/skills/common/nui.md b/skills/common/nui.md index 3326e28..81d5cff 100644 --- a/skills/common/nui.md +++ b/skills/common/nui.md @@ -54,7 +54,7 @@ NUI is client-local only. It never carries authority. Treat the Lua<->browser br - The browser listens with `window.addEventListener("message", e => ...)`. - Use one stable top-level key that names the message (`action`, `type`, or `event`), not an arbitrary payload the UI must pattern-match blindly. - Do not send updated full data every frame. Send one open/update message and let the UI render its own state between updates. -- Functions, metatables, userdata (vectors, handles), and cyclic tables do not survive JSON. Send plain data: numbers, strings, booleans, arrays, plain tables. +- Functions, userdata, CfxLua vector values (`vector3` etc.), and cyclic tables do not survive JSON. Convert vectors to explicit `{ x = v.x, y = v.y, z = v.z }` tables. Do not send entity handles: they are client-local integers that mean nothing in the browser, so send display data or a server-issued ID instead. Send plain data: numbers, strings, booleans, arrays, plain tables. ## Browser To Lua @@ -75,15 +75,15 @@ NUI is client-local only. It never carries authority. Treat the Lua<->browser br cb({ ok = true }) end) ``` -- The `cb` argument sends the response that resolves the browser `fetch`. Call `cb` exactly once per request. Forgetting it leaves the browser promise pending; calling it twice is undefined. +- The `cb` argument sends the response that resolves the browser `fetch`. Call `cb` exactly once per request. Forgetting it makes the request time out and the browser `fetch` reject with an error (wrap `fetch` in try/catch in the bridge helper); calling it twice is undefined. - Always pass an explicit response object (for example `cb({ ok = true })`) even when there is no useful return value, so the browser promise resolves cleanly. ## Focus And Input -- `SetNuiFocus(hasFocus, hasInput)` controls cursor/game-input: - - `hasFocus = true` shows the cursor and lets it interact with the UI. - - `hasInput = true` lets mouse movement still reach the game while the UI is open. Most menus want `SetNuiFocus(true, false)`. -- `SetNuiFocusKeepInput(true)` keeps keyboard input flowing to the game while NUI has focus (for HUDs or chat-style overlays). Re-enable game input with `SetNuiFocusKeepInput(false)` on close. +- `SetNuiFocus(hasFocus, hasCursor)`: `hasFocus = true` gives the NUI page input focus; `hasCursor = true` shows the mouse cursor over the UI. + - Clickable menus use `SetNuiFocus(true, true)`. `SetNuiFocus(true, false)` gives keyboard focus with no cursor (for example a text prompt). + - To let the game keep receiving input while NUI is focused, use `SetNuiFocusKeepInput(true)` (see below). +- `SetNuiFocusKeepInput(true)` lets game controls (keyboard and mouse) keep reaching the game while NUI has focus (HUDs, chat-style overlays); disable any controls you do not want with `DisableControlAction` each frame. On close, call `SetNuiFocusKeepInput(false)` to restore the default (focused NUI captures input) so the next focused UI does not leak input to the game. `SetNuiFocus(false, false)` is what returns input to the game. - Treat focus as a toggle: set focus on open, clear it on close. Never call `SetNuiFocus(true, true)` every frame. - A stuck cursor after the UI closes is almost always a missing `SetNuiFocus(false, false)` on the close path. @@ -95,7 +95,7 @@ NUI is client-local only. It never carries authority. Treat the Lua<->browser br ## Validation -- Treat every `RegisterNUICallback` payload as untrusted, same as a `:client:` event: the browser can be devtooled or scripted. +- Treat every `RegisterNUICallback` payload as untrusted, exactly as hostile as a client-triggered `:server:` event: the browser can be devtooled or scripted. - Validate `type(...)` and bounds for fields that drive client behavior or that get forwarded to the server. - For actions forwarded to the server, do the authoritative validation server-side and treat the NUI path as a UX hint. - Never grant money, items, permissions, or access from a NUI callback alone. Forward a validated `:server:` event and let the server decide. @@ -114,7 +114,7 @@ RegisterNUICallback("buyItem", function(data, cb) if amount <= 0 or amount > 100 then cb({ ok = false }) return end -- the server re-derives identity, catalog, price, and affordability; cb is NOT the grant - TriggerServerEvent("shop:buy", { Item = itemName, Amount = amount }) + TriggerServerEvent("shop:server:buy", { Item = itemName, Amount = amount }) cb({ ok = true }) end) ``` @@ -137,7 +137,7 @@ end) ## Security - NUI is client-local and client-editable. It is display and UX, not authority. -- **XSS**: any string another player can influence (player names, chat, notes, item labels, shop names) is an XSS payload candidate crossing from an attacker to a victim's screen. Render it as text - framework text interpolation (React/Svelte/Vue `{value}`) escapes safely; `innerHTML`, `insertAdjacentHTML`, `document.write`, `{@html}`, and `v-html` are the sinks that execute it. Never feed player-controlled data to a raw-HTML sink; if rich text is unavoidable, sanitize with a vetted library and a strict allowlist. +- **XSS**: any string another player can influence (player names, chat, notes, item labels, shop names) is an XSS payload candidate crossing from an attacker to a victim's screen. Render it as text - framework text interpolation (React/Svelte `{value}`, Vue `{{ value }}`) escapes safely; `innerHTML`, `insertAdjacentHTML`, `document.write`, `{@html}`, and `v-html` are the sinks that execute it. Never feed player-controlled data to a raw-HTML sink; if rich text is unavoidable, sanitize with a vetted library and a strict allowlist. - XSS inside CEF is not "just UI": injected script can call every `RegisterNUICallback` and trigger anything the UI can - as the victim player. Treat a rendering sink as full compromise of that player's client-side surface. - Validate the scheme of any dynamic `href`/`src` (`https:`/`nui:` only); a `javascript:` URL executes on click. Build `nui://` asset paths from a restricted character set so a crafted name cannot traverse (`../`) out of the intended folder. - Bound the length of player-controlled strings before rendering so one giant value cannot freeze or break the HUD. diff --git a/skills/common/ox-lib.md b/skills/common/ox-lib.md index bbc3de9..d9c6202 100644 --- a/skills/common/ox-lib.md +++ b/skills/common/ox-lib.md @@ -23,6 +23,7 @@ Use this file only when a RedM/FiveM resource already depends on ox_lib or the t 'config.lua', } ``` +- `'@ox_lib/init.lua'` replaces the global `require` with `lib.require`, which is what lets `require('configs.items')` load resource files. Code that relies on it breaks if ox_lib is removed; see `skills/common/style.md` -> Lua Style for the no-ox_lib loader. - Keep ox_lib imports out of standalone Lua tooling and non-Cfx scripts. - If using locale files, include them under `files` and initialize locale once. - Do not mix ox_lib-required code into files that are meant to run without that dependency. diff --git a/skills/common/resource-structure.md b/skills/common/resource-structure.md index 9a2bfaf..4aad035 100644 --- a/skills/common/resource-structure.md +++ b/skills/common/resource-structure.md @@ -44,7 +44,7 @@ Use this file for rules that apply to both RedM and FiveM. Use `skills/fivem/rul ## Entities And Objects - Check entity handles before using them. -- Load models before creating objects, for example with `lib.RequestModel`. +- Load models before creating objects: call `RequestModel(model)` and wait with a timeout until `HasModelLoaded(model)` is true, or use `lib.requestModel(model)` when the resource already depends on ox_lib (see `skills/common/ox-lib.md`). Call `SetModelAsNoLongerNeeded(model)` after spawning. - Delete old entities before replacing them when needed. - Clean up entities on resource stop, player drop, or when synced state disables them. - Reattach loops are acceptable when objects may detach or disappear, but keep the wait time appropriate. diff --git a/skills/common/runtime.md b/skills/common/runtime.md index f813fce..c744456 100644 --- a/skills/common/runtime.md +++ b/skills/common/runtime.md @@ -19,7 +19,7 @@ These are the mechanics every RedM/FiveM Lua author hits. Native call mechanics ## Threads And Waits -- `CreateThread(function() ... end)` starts a coroutine thread. A thread that never `Wait`s blocks the resource's main thread (and on the server, the shared Lua runtime) until it returns. +- `CreateThread(function() ... end)` starts a cooperative coroutine that nothing preempts. Each resource has its own Lua state, but every resource runs on the same main thread (the client game thread or the server main thread). A thread that never `Wait`s therefore freezes the whole client game or server tick, not just its own resource, until it returns. - `Wait(ms)` yields the current thread. `Wait(0)` yields one frame and resumes next frame. A loop without `Wait` is a busy loop that stalls the runtime. - Use staged waits by distance/activeness (idle `Wait(1000)+`, nearby `Wait(500)`, active `Wait(100)`, frame work `Wait(0)`); see `skills/common/security-performance.md` -> Performance Rules. - `Citizen.Wait`, `Citizen.Await`, and `Citizen.CreateThreadNow` exist for specific cases; default to `Wait`/`CreateThread`. `Citizen.Await` is for awaiting a future-like value inside a thread. @@ -36,7 +36,7 @@ These are the mechanics every RedM/FiveM Lua author hits. Native call mechanics -- safe to Wait / await here; `source` is now a stable local end) ``` -- After a yield, the captured `source` can be stale: the player may have dropped, and FXServer recycles server IDs, so the same number can already belong to a **different** player. Re-validate after yielding (`DoesPlayerExist(source)`, or the framework's `GetPlayer(source)`), and for valuable mutations compare an identifier captured before the yield (e.g. `license`) against the current one before applying. +- After a yield, the captured `source` can be stale: the player may have dropped, so natives on that ID return nil, 0 (truthy in Lua), or an empty table (see Players And Identifiers). Server IDs come from a 16-bit counter (1-65534) that wraps back to 1 and skips IDs still in use, so an ID is reused only after ~65k further joins: practically never within one yield, but a `source` kept in a table not cleared on `playerDropped` can map to a different player on a long-running server. During `playerConnecting`, `source` is a temporary ID (65536 and up) that is replaced when the player joins (`playerJoining` receives it as `oldID`), so do not key state by it past the handshake. Re-validate after yielding (`DoesPlayerExist(source)`, or the framework's `GetPlayer(source)`), and for valuable mutations compare an identifier captured before the yield (e.g. `license`) against the current one before applying. - Never accept a `source` value from a client payload as identity. Real `source` comes from the runtime, not the data. See Players And Identifiers. ## Exports And Stale References @@ -47,16 +47,16 @@ These are the mechanics every RedM/FiveM Lua author hits. Native call mechanics local ESX = exports['es_extended']:getSharedObject() -- captured once at load ``` If the providing resource restarts, `ESX` may still reference the old object/code while the rest of the server uses the new one. After a framework restart, prefer re-fetching the shared object over trusting a load-time local. -- For the same reason, do not assume `exports['name']` is non-nil at load. The provider may not have started yet. Check `GetResourceState('name')` or gate the call on `onResourceStart`/`onServerResourceStart`. +- Do not guard with `if exports['name'] then`. The exports proxy is never nil, and indexing a method on a provider that has not started (or has no such export) throws `No such export in resource `. Check `GetResourceState('name') == 'started'` before calling, wrap the call in `pcall(function() return exports['name']:method(...) end)`, or make the call from `onResourceStart`/`onServerResourceStart` for that resource. - See `skills/common/multi-resource.md` -> Dependencies And Load Order for declaring dependencies instead of racing with `Wait`. ## Players And Identifiers - `GetPlayerIdentifiers(source)` returns a table of identifiers. Verified types: `steam` (hex), `discord` (int), `xbl` (int), `live` (Microsoft PUID), `license`/`license2` (ROS hash - `license2` can equal `license` for Steam users), `fivem` (Cfx user id), `ip` (IPv4 string). Availability depends on the server/client link. ([GetPlayerIdentifiers](https://docs.fivem.net/docs/scripting-reference/runtimes/lua/functions/GetPlayerIdentifiers/)) - `GetPlayerIdentifierByType(source, "license")` returns a single identifier when that is all you need. -- `license:` (the player's Rockstar/Cfx license) is the usual stable primary key across sessions. Pick one identifier scheme and keep it server-side. -- Identity is only reliable server-side. Never trust an identifier sent by the client in an event payload; identity comes from `GetPlayerIdentifiers(source)` on the server, keyed by the runtime `source`. Do not trust a single identifier type in isolation - `ip` is weak (shared NAT, rotates), so ban/whitelist on the strongest available identifier (commonly `license`). -- Run ban/whitelist and auth checks during `playerConnecting` using the `deferrals` API (`defer`, `update`, `done` called exactly once) for async DB/license lookups, not after the player is in-game. Tune trust with `sv_authMinTrust` (1-5, spoof-resistance) and `sv_authMaxVariance` (1-5, stability per provider). ([Server Commands](https://docs.fivem.net/docs/server-manual/server-commands/)) +- For a stable primary key across sessions, prefer `license2` when present and fall back to `license` (`GetPlayerIdentifierByType(source, "license2") or GetPlayerIdentifierByType(source, "license")`). Pick one identifier scheme and keep it server-side; if a DB is already keyed by `license`, keep that key stable instead of switching schemes mid-life. +- Identity is only reliable server-side. Never trust an identifier sent by the client in an event payload; identity comes from `GetPlayerIdentifiers(source)` on the server, keyed by the runtime `source`. Do not trust a single identifier type in isolation - `ip` is weak (shared NAT, rotates), so ban/whitelist on the strongest available identifier (`license2`, else `license`). +- Run ban/whitelist and auth checks during `playerConnecting` using the `deferrals` API (`defer`, `update`, `done` called exactly once) for async DB/license lookups, not after the player is in-game. `sv_authMinTrust` is effectively a no-op: FXServer compares it against the highest trust among a player's identifiers, and the always-present `ip:` identifier reports trust 5. `sv_authMaxVariance` (1-5, default 5) is the one that matters: it is compared against the lowest variance, where `license`/`steam` report 1 and `ip` reports 5, so any value below 5 rejects players with no `license`/`steam` identifier. ([Server Commands](https://docs.fivem.net/docs/server-manual/server-commands/), FXServer `InitConnectMethod.cpp`/`EndPointIdentityProvider.cpp`) - `GetPlayerName(source)`, `GetPlayerPed(source)`, `GetPlayerPing(source)` read server-side player state; `DoesPlayerExist(source)` checks existence, and `GetPlayers()` returns connected sources (also see the player state bag API in `skills/common/resource-structure.md`). `GetPlayer(...)` is a framework function (ESX/QBCore/VORP style), not a CFX native - do not use it as an existence check in framework-free code. - Do not use `GetPlayerIdentifiers(source)` as an existence check; it returns a table (truthy even when empty) for invalid IDs. Use `GetPlayerName`/`DoesPlayerExist`. - Validate `source` is a real connected player before using it (`source > 0` and the player exists), especially after a yield. @@ -75,13 +75,13 @@ These are the mechanics every RedM/FiveM Lua author hits. Native call mechanics ## Resource Lifecycle -- `ensure` in `server.cfg` starts the resource and restarts it if it crashes. `start` starts it once. Use `ensure` for resources that must stay up. +- `ensure name` starts the resource if it is stopped, or restarts it if it is already running. That makes it safe to repeat in `server.cfg` and useful as a quick reload in the console. `start name` only starts a stopped resource. Neither command watches for crashes or restarts anything automatically: a script error does not stop a resource, and a stopped resource stays stopped until someone starts it again. - Load order follows declaration order. Declare dependencies with `dependency`/`dependencies` in `fxmanifest.lua` and check `GetResourceState` for optional ones instead of guessing with `Wait`. - Resource events: - `onResourceStart(resource)` — fires on both sides when a resource starts, including this one. - `onResourceStarting(resource)` — fires before start; useful to set up before another resource initializes. - - `onResourceStop(resource)` — fires on both sides; guard with `resourceName == GetCurrentResourceName()`. - - `onServerResourceStart(resource)` / `onServerResourceStop(resource)` — server-side, fired for other resources starting/stopping. + - `onResourceStop(resourceName)` — fires on both sides for every stopping resource. Guard with `if resourceName ~= GetCurrentResourceName() then return end`, using the handler's own parameter name. + - `onServerResourceStart(resource)` / `onServerResourceStop(resource)` — server-only counterparts of `onClientResourceStart`/`onClientResourceStop`. They are queued (they run on a later tick) instead of firing immediately like `onResourceStart`, and they fire for every resource, including this one on start. Guard with `resource == GetCurrentResourceName()` when you only care about your own resource. - When a provider resource restarts, callers holding a cached export result or a pending callback can go stale. Re-fetch references and clear pending state. See Exports And Stale References and `skills/common/security-performance.md` -> Callback Safety. - See `skills/common/multi-resource.md` -> Failure Handling for degrading cleanly when a dependency is absent. @@ -90,12 +90,12 @@ These are the mechanics every RedM/FiveM Lua author hits. Native call mechanics - Do not yield (no `Wait`, no `MySQL.*.await`, no async export) inside `entityCreating`. The handler gates creation; long or yielding work can stall replication or time out the network request. Move heavy work to a thread keyed by the handle. - In `playerConnecting`, do not block. Use the `deferrals` API (`defer`, `update`, `done`, `handover`) to run async checks (DB, license, whitelist) and call `deferrals.done()` exactly once. - Avoid yielding between a validation check and a valuable mutation. Check-and-mutate atomically; otherwise recheck after the yield. See `skills/common/security-performance.md` -> Atomic State Changes And Replay Protection. -- `MySQL.*.await` calls are blocking awaits within their coroutine; on the server they must run in a coroutine context (a thread or event handler), not the synchronous startup path. +- `MySQL.*.await` yields (suspends) only the calling coroutine until the query finishes; it does not block the runtime or other threads. Because it yields, it must run in a coroutine context (a thread, an event handler, or file top level, where it defers the rest of the file), and it counts as a yield for every rule above. ## Game Builds -- The running game build affects which natives are available and how some behave. Compare `GetGameBuildNumber()` against a native's minimum build before relying on newer natives; the server's `sv_enforceGameBuild` sets the enforced build (startup-only). ([Server Commands](https://docs.fivem.net/docs/server-manual/server-commands/)) -- FiveM (GTA V) enforceable builds (as of the current docs): `1604, 2060, 2189, 2372, 2545, 2612, 2699, 2802, 2944, 3095, 3258, 3407, 3570, 3751` - each includes all prior content. +- The running game build affects which natives are available and how some behave. On the client, compare `GetGameBuildNumber()` against a native's minimum build before relying on newer natives. On the server, `GetGameBuildNumber()` returns 0, so read the enforced build with `GetConvarInt("sv_enforceGameBuild", 0)` instead. When the server does not set it, this returns the artifact's default build (for example 3258 on FiveM or 1491 on RedM), not 0. The server's `sv_enforceGameBuild` sets the enforced build (startup-only; DLC names like `mptuner` are stored as their build number). ([Server Commands](https://docs.fivem.net/docs/server-manual/server-commands/)) +- FiveM (GTA V) enforceable builds (fivem-docs `sv_enforceGameBuild` table, checked 2026-09): `1` (base game, no DLCs), `1604, 2060, 2189, 2372, 2545, 2612, 2699, 2802, 2944, 3095, 3258, 3407, 3570, 3751, 3889`. Each includes all prior content. Re-check the table before relying on this list. - RedM (RDR3): `1491` is the current enforceable build (September 2022 update); older defaults like `1311`/`1355`/`1436` existed historically. - Always confirm the live list against the current artifact, since new builds are added over time. - Guard optional newer-build natives with a build check and an explicit fallback rather than letting them fail silently on older servers. @@ -103,7 +103,7 @@ These are the mechanics every RedM/FiveM Lua author hits. Native call mechanics ## Debugging -- A "resource hangs" or "server freezes" symptom is almost always a thread that never yields or a synchronous `MySQL.*.await` on the main path. +- A "resource hangs" or "server freezes" symptom is almost always a loop that never yields (no `Wait`), or heavy synchronous computation or a blocking call on the main path. A top-level `MySQL.*.await` does not error: CfxLua runs each script file inside `CreateThreadNow`, so the await yields and the rest of that file (including handlers registered below it) runs only after the query returns, while later files load first. It errors only in non-yieldable contexts such as a `table.sort` comparator or `__gc` ("attempt to yield across a C-call boundary"). - A "works only sometimes" event bug is usually a stale `source` read after a yield, or a provider resource that restarted under cached references. - Confirm load order and `GetResourceState` for cross-resource calls before assuming an export is broken. - See `skills/common/debugging.md` for the general flow. diff --git a/skills/common/security-performance.md b/skills/common/security-performance.md index 9599813..c4d691f 100644 --- a/skills/common/security-performance.md +++ b/skills/common/security-performance.md @@ -45,9 +45,9 @@ Use these rules for RedM/FiveM resources where player input, server state, entit - `RegisterNetEvent` marks an event as network-callable. Any client can trigger it with `TriggerServerEvent(name, ...)` and arbitrary arguments, whether or not your UI is the intended caller. Treat every registered net event as a public, attacker-callable endpoint. ([RegisterNetEvent](https://docs.fivem.net/docs/scripting-reference/runtimes/lua/functions/RegisterNetEvent/), [Secure your events](https://docs.fivem.net/docs/developers/server-security/)) - `RegisterServerEvent` is the deprecated form and does the same thing; use `RegisterNetEvent` in new code. ([cfx-server-data PR #204](https://github.com/citizenfx/cfx-server-data/pull/204)) - `AddEventHandler` is same-context only (client-to-client or server-to-server) and is not network-reachable from the opposing side. Use it for internal logic that must not be callable by clients. ([Secure your events](https://docs.fivem.net/docs/developers/server-security/)) -- `RegisterNetEvent` does not block same-context execution. Server-originated net events arrive with `source == 65535`; reject that source in handlers that must only run when a real client triggered them. ([Secure your events](https://docs.fivem.net/docs/developers/server-security/)) +- `RegisterNetEvent` does not block same-context execution. On the server, a same-context `TriggerEvent` reaches a `RegisterNetEvent` handler with `source == ""` (empty string) ([scheduler.lua](https://github.com/citizenfx/fivem/blob/master/data/shared/citizen/scripting/lua/scheduler.lua)). Client-triggered events always carry a numeric `source`. Guard with `if type(source) ~= "number" or source <= 0 then return end` in handlers that must only run when a real client triggered them. (`65535` is the `source` that *client-side* handlers see for events sent by the server; use `if source ~= 65535 then return end` there to accept only server-sent events.) ([Secure your events](https://docs.fivem.net/docs/developers/server-security/)) - Always read identity from the implicit `source` (capture `local source = source` before any yield). Never trust a player ID, target, or identifier field from the payload. See `skills/common/runtime.md` -> The `source` Variable and Identifier Trust below. -- Validate `source` (`source > 0` and a real player), then `type(...)` and bounds for every field, before doing work. +- Validate `source` (a number, `> 0`, and a real player), then `type(...)` and bounds for every field, before doing work. ### Naive vs hardened handler @@ -55,7 +55,7 @@ Vulnerable - the client controls the recipient, the item, and the amount: ```lua -- BAD: client controls who gets what, and how much -RegisterNetEvent("shop:buy", function(item, price) +RegisterNetEvent("shop:server:buy", function(item, price) local src = source local player = Framework.GetPlayer(src) player.addMoney(-price) -- client-chosen "price" can be negative @@ -64,7 +64,7 @@ end) ``` - The vulnerable version is a direct dupe: a client passes a negative `price` (money goes up) or an item it should never receive. -- The hardened shape: identity from `source` only, item/price resolved from a server-side catalog, quantity clamped to a positive bounded integer, affordability read from server-held balance at transaction time, and the debit+grant made atomic. The full, race-safe version - the canonical pattern from CFX's own [Secure your events](https://docs.fivem.net/docs/developers/server-security/) guide - is in Give-Value Event Hardening below; use that as the template instead of re-deriving it per resource. +- The hardened shape: identity from `source` only, item/price resolved from a server-side catalog, quantity clamped to a positive bounded integer, affordability read from server-held balance at transaction time, and the debit+grant made atomic. The full, race-safe version is in Give-Value Event Hardening below (it builds on the server-authority principles in CFX's [Secure your events](https://docs.fivem.net/docs/developers/server-security/) guide); use that as the template instead of re-deriving it per resource. @@ -82,7 +82,7 @@ end) - Every custom `:server:` event must validate its payload before doing work. - Validate `source` when the event depends on a real player. - Validate `type(...)` for every field used by the event. -- Normalize numbers with `tonumber`, `math.floor`, and explicit min/max bounds. +- Normalize numbers with `tonumber` and explicit min/max bounds; reject non-integer quantities (`x % 1 ~= 0`) instead of flooring them. - Reject `NaN`, negative values, empty strings, invalid keys, unknown items, unknown jobs, unknown weapons, and unknown config names. - Check requested item/action names against server-side config or indexes. - Never trust client-provided price, reward amount, inventory count, permission group, job, target ownership, or cooldown state. @@ -134,8 +134,15 @@ RegisterCommand("giveitem", function(source, args) local entry = Controller.Catalog[itemName] if not entry then return end - Framework.GetPlayer(target):addItem(itemName, amount) - print(("admin %s gave %sx %s to %s"):format(source, amount, itemName, target)) + local targetPlayer = Framework.GetPlayer(target) + if not targetPlayer then return end -- connected, but character not loaded yet + + targetPlayer.addItem(itemName, amount) + -- audit trail: server IDs are reused, so log identifiers alongside them + print(("admin %s (%s) gave %sx %s to %s (%s)"):format( + source, GetPlayerIdentifierByType(source, "license") or "?", + amount, itemName, + target, GetPlayerIdentifierByType(target, "license") or "?")) end, false) ``` @@ -170,7 +177,7 @@ end RegisterNetEvent("resource_name:server:action", function(payload) local source = source - if not source or source <= 0 then return end + if type(source) ~= "number" or source <= 0 then return end -- "" = server-side TriggerEvent if type(payload) ~= "table" then return end if Controller:IsActionThrottled(source, "action", 500) then return end @@ -181,7 +188,7 @@ RegisterNetEvent("resource_name:server:action", function(payload) if type(itemName) ~= "string" or itemName == "" then return end if not amount or amount ~= amount then return end - amount = math.floor(amount) + if amount % 1 ~= 0 then return end if amount <= 0 or amount > 100 then return end local itemIndex = Controller.ItemIndex[itemName] @@ -202,29 +209,29 @@ end) ## Give-Value Event Hardening -This is the highest-frequency exploit class in CFX servers: shop buys, payouts, crafting, quest rewards, drops, trades, admin grants. A client event may **request** an action; it must never specify the amount, the item, the price, or the recipient of anything of value. The server decides all of those from server-held state. (Canonical pattern: [Secure your events](https://docs.fivem.net/docs/developers/server-security/).) +This is the highest-frequency exploit class in CFX servers: shop buys, payouts, crafting, quest rewards, drops, trades, admin grants. A client event may **request** an action; it must never specify the amount, the item, the price, or the recipient of anything of value. The server decides all of those from server-held state. (Principles: [Secure your events](https://docs.fivem.net/docs/developers/server-security/).) ### Vulnerable ```lua -- BAD: client controls both the item and the price; recipient defaults to source but is never re-checked -RegisterNetEvent("shop:buy", function(item, price) +RegisterNetEvent("shop:server:buy", function(item, price) local player = Framework.GetPlayer(source) player.removeMoney(price) -- client-chosen price can be NEGATIVE -> money goes UP player.addItem(item) -- client-chosen item, no catalog/ownership check, fixed count of 1 end) ``` -A mod menu fires `TriggerServerEvent("shop:buy", "weapon_rpg", -1000000)` and gains money plus the item. Negative price or quantity is a classic dupe. +A mod menu fires `TriggerServerEvent("shop:server:buy", "weapon_rpg", -1000000)` and gains money plus the item. Negative price or quantity is a classic dupe. ### Hardened, in order -1. **Identity = `source`, never the payload.** Reject `source <= 0`, `source == 65535` (server-originated), and any payload field that claims to be the recipient. A normal player must not grant items to an arbitrary ID; only an ACE-gated admin grant may target another player (see ACE Permissions And Admin Authority). +1. **Identity = `source`, never the payload.** Reject a non-numeric/empty `source` (server-originated) or `source <= 0`, and any payload field that claims to be the recipient. A normal player must not grant items to an arbitrary ID; only an ACE-gated admin grant may target another player (see ACE Permissions And Admin Authority). 2. **Server-authoritative catalog.** Items, prices, weights, stack limits, and craft recipes live in server config/DB. The client sends at most an item key/id; the server maps it to real data and rejects unknown keys. Never accept `price` from the client. 3. **Validate every field before touching value.** Type, range, and existence checks on the item key and quantity first. Reject missing/extra fields as hostile. -4. **Quantity is a positive, bounded integer.** Reject `nil`, `NaN`, `0`, negatives, floats, and values above a sane max (`amount = math.floor(tonumber(x)); if not amount or amount ~= amount or amount <= 0 or amount > MAX then return end`). A negative amount in a transfer/remove path is a dupe. +4. **Quantity is a positive, bounded integer.** Reject `nil`, `NaN`, `0`, negatives, floats, and values above a sane max (`local amount = tonumber(x); if not amount or amount ~= amount or amount % 1 ~= 0 or amount <= 0 or amount > MAX then return end`). A negative amount in a transfer/remove path is a dupe. 5. **Affordability from fresh server state.** Re-read balance/inventory at the moment of the transaction, not a value cached from an earlier request. Reject if `balance < price * amount`. -6. **Debit + grant atomically.** The check-then-mutate must not be interruptible by the same player spamming the event. Use a per-source in-flight lock, or one atomic DB transaction; do not `Wait()` between the check and the mutation with shared state exposed. +6. **Debit + grant atomically.** The check-then-mutate must not be interruptible by the same player spamming the event. Use a per-source in-flight lock that is always released (even on error), or a conditional DB debit whose affected row count gates the grant inside one transaction (variant (b) below); do not `Wait()` between the check and the mutation with shared state exposed. Keep one source of truth for money/inventory: the framework's RAM cache or the database, never both. 7. **Rate-limit + idempotency.** Throttle per source. For one-shot grants (quest reward, daily claim), record server-side that it was claimed and reject repeats; never rely on the client not re-firing. 8. **Admin grants are ACE-gated and logged.** No client-side "admin" flag, no spoofable identifier (see ACE Permissions And Admin Authority). @@ -246,9 +253,42 @@ function Controller:IsThrottled(source, key, limit) return false end -RegisterNetEvent("shop:buy", function(payload) +-- (5)+(6) the locked section. Pick ONE source of truth for money/inventory, never both. + +-- (a) Framework/RAM-authoritative (the usual ESX/QBCore/VORP case): mutate through the +-- framework API and let the framework persist. No SQL and no yield between check and mutation. +function Controller:Purchase(player, entry, itemName, amount) + local cost = entry.price * amount + if player.getMoney() < cost then return end -- (5) current server-held balance + if not player.canCarryItem(itemName, amount) then return end -- reject before debiting + player.removeMoney(cost) + player.addItem(itemName, amount) +end + +-- (b) DB-authoritative: only when no framework RAM copy of money/inventory is saved back over +-- these columns (a drop-time or autosave write of stale RAM would undo the committed debit). +function Controller:PurchaseSql(player, entry, itemName, amount) + local cost = entry.price * amount + + -- (5)+(6) one transaction: the conditional debit IS the affordability check; returning false + -- (0 rows = cannot afford) or any SQL error rolls back both statements (oxmysql startTransaction, experimental) + local committed = MySQL.startTransaction(function(query) + local res = query("UPDATE characters SET money = money - ? WHERE owner = ? AND money >= ?", + { cost, player.identifier, cost }) + if not res or res.affectedRows ~= 1 then return false end + query("INSERT INTO inventory (owner, item, count) VALUES (?, ?, ?) ON DUPLICATE KEY UPDATE count = count + ?", + { player.identifier, itemName, amount, amount }) + end) + if not committed then return end + + -- the transaction yielded, so the player may have dropped. If the framework keeps a read-only + -- copy, refresh it by identifier (not `source`); never write back a RAM snapshot taken before the yield. + Framework.RefreshPlayer(player.identifier) -- placeholder: re-read money/inventory from DB +end + +RegisterNetEvent("shop:server:buy", function(payload) local source = source - if not source or source <= 0 or source == 65535 then return end + if type(source) ~= "number" or source <= 0 then return end -- "" = server-side TriggerEvent if type(payload) ~= "table" then return end if Controller:IsThrottled(source, "buy", 250) then return end @@ -256,8 +296,8 @@ RegisterNetEvent("shop:buy", function(payload) local itemName = payload["Item"] local amount = tonumber(payload["Amount"]) if type(itemName) ~= "string" or itemName == "" then return end - if not amount or amount ~= amount then return end -- NaN - amount = math.floor(amount) + if not amount or amount ~= amount then return end -- non-numeric / NaN + if amount % 1 ~= 0 then return end -- reject non-integers if amount <= 0 or amount > MAX_PER_PURCHASE then return end -- (4) positive bounded int -- (2) server catalog; reject unknown / unpurchasable @@ -267,38 +307,16 @@ RegisterNetEvent("shop:buy", function(payload) local player = Framework.GetPlayer(source) -- (1) identity = source if not player then return end - -- (6) atomic: hold an in-flight lock so a flood cannot interleave check/mutate + -- (6) in-flight lock so a flood cannot interleave check/mutate. pcall guarantees the + -- release: one error (nil price, framework throw, rejected .await) would otherwise lock + -- this player out of buying until reconnect. if Controller.InFlight[source] then return end Controller.InFlight[source] = true - - -- (5) affordability against the current server-held balance - local cost = entry.price * amount - if player.getMoney() < cost then - Controller.InFlight[source] = nil - return - end - - -- DB-backed economy: one atomic transaction commits debit + grant, or neither - local ok = MySQL.transaction.await({ - { query = "UPDATE characters SET money = money - ? WHERE owner = ? AND money >= ?", - values = { cost, player.identifier, cost } }, - { query = "INSERT INTO inventory (owner, item, count) VALUES (?, ?, ?) ON DUPLICATE KEY UPDATE count = count + ?", - values = { player.identifier, itemName, amount, amount } }, - }) -- returns false if either fails (oxmysql transaction) - if not ok then - Controller.InFlight[source] = nil - return -- keep state consistent, do not grant in RAM - end - - -- the await yielded: the player may have dropped, and server IDs are recycled, - -- so `source` can already belong to a different player. Re-resolve and compare - -- identity before touching RAM; the DB commit above is correct either way. - local current = Framework.GetPlayer(source) - if current and current.identifier == player.identifier then - current.setMoney(current.getMoney() - cost) - current.addItem(itemName, amount) - end + local okRun, err = pcall(Controller.Purchase, Controller, player, entry, itemName, amount) -- or Controller.PurchaseSql Controller.InFlight[source] = nil + if not okRun then + print(("shop:server:buy failed for %s: %s"):format(source, err)) + end end) AddEventHandler("playerDropped", function() @@ -310,19 +328,20 @@ AddEventHandler("playerDropped", function() end) ``` -- `MySQL.transaction.await` runs the queries in one transaction and commits only if all succeed; a `false` return means rolled back - treat it as a rejected mutation ([oxmysql transaction](https://overextended.dev/docs/oxmysql/Functions/transaction)). `?` placeholders are mandatory; see Database And Persistence and `skills/common/database.md`. -- Every `.await` is a yield. After it resumes, re-validate the player before applying RAM state: FXServer recycles server IDs, so a `source` captured before the yield can point at a *different* player after a drop + reconnect. Compare a pre-yield identifier against the current one, never just the source number. See `skills/common/runtime.md` -> The `source` Variable. -- The `UPDATE ... AND money >= ?` conditional acts as a server-side lock: if two buy requests race, only the one that still satisfies the balance commits. +- An oxmysql `MySQL.transaction` rolls back only on an SQL error ([oxmysql transaction](https://overextended.dev/docs/oxmysql/Functions/transaction)): a conditional `UPDATE ... AND money >= ?` that matches 0 rows still commits the rest and returns `true`. Gate the grant on the debit affecting exactly 1 row inside the same transaction, as variant (b) above does (see `skills/common/database.md` -> Query Safety). `?` placeholders are mandatory; see Database And Persistence. +- Do not debit in SQL and again in a RAM-cached framework: a drop-time or autosave write of stale RAM will overwrite the committed debit (and a framework that saves inventory from RAM can overwrite or double-apply the SQL grant). Use variant (a) whenever the framework caches money/inventory in RAM; see `memory/common/cfx-patterns.md` -> Player Persistence Pattern. +- Every `.await` is a yield. After it resumes, the player may have dropped, so re-validate before touching anything keyed by `source` (`DoesPlayerExist(source)` or the framework's `GetPlayer(source)`). Server IDs are reused only after the 16-bit ID counter wraps, so a stale ID mapping to a *different* player within one yield is practically impossible; the real reuse risk is a per-`source` table not cleared on `playerDropped` on a long-running server. For valuable mutations, key post-yield work by the pre-yield identifier (compare it against the current one), never just the source number. See `skills/common/runtime.md` -> The `source` Variable. +- The locked section runs inside `pcall` so the in-flight lock is always released; `pcall` is yield-safe in CFX Lua 5.4, so `.await` inside it works. ### Give-value event checklist Apply to every handler that grants, removes, transfers, prices, or pays anything of value: -1. Identity = `source` only; reject `0`/`65535` and any payload recipient. +1. Identity = `source` only; reject a non-numeric/empty `source` (server-originated) or `source <= 0`, and any payload recipient. 2. Item/price/limits = server catalog, never the payload. 3. Quantity = positive bounded integer; reject 0/negative/float/NaN. 4. Afford check against a freshly read server-held balance. -5. Debit + grant atomic; per-source in-flight lock or single transaction. +5. Debit + grant atomic; per-source in-flight lock that is always released, or a conditional debit checked for exactly 1 affected row in the same transaction as the grant; one source of truth (framework RAM or DB, never both). 6. Rate-limited; one-shot grants tracked server-side (idempotent). 7. Admin grants ACE-gated (`IsPlayerAceAllowed`) and logged. @@ -394,11 +413,12 @@ Related: Atomic State Changes And Replay Protection below, ACE Permissions And A The game/OneSync routes client->server events that attackers use as free exploit surfaces. Treat every one as attacker-callable with attacker-chosen fields. ([server-events](https://docs.fivem.net/docs/scripting-reference/events/server-events/), [Secure your events](https://docs.fivem.net/docs/developers/server-security/)) -- Documented core events: `weaponDamageEvent(sender, data)`, `startProjectileEvent(sender, data)`, `ptFxEvent(sender, data)`, `removeAllWeaponsEvent(sender, data)` (full fields in the [server-events reference](https://docs.fivem.net/docs/scripting-reference/events/server-events/)). `data.weaponType` on `weaponDamageEvent` has carried incorrect hashes (citizenfx/fivem#3827) - validate against server-side weapon/entity state, not the field alone. +- Documented core events: `weaponDamageEvent(sender, data)`, `startProjectileEvent(sender, data)`, `ptFxEvent(sender, data)`, `removeAllWeaponsEvent(sender, data)` (full fields in the [server-events reference](https://docs.fivem.net/docs/scripting-reference/events/server-events/)). `data.weaponType` on `weaponDamageEvent` is an unsigned 32-bit hash, while `GetHashKey`/backticks return signed values (citizenfx/fivem#3827: 2982836145 vs -1312131151 for `weapon_rpg`). Normalize before comparing: `local h = data.weaponType; if h > 0x7FFFFFFF then h = h - 0x100000000 end`. It is still client-reported, so check it against server-side weapon/entity state. - `explosionEvent` is interceptable server-side with OneSync: register `AddEventHandler('explosionEvent', function(sender, ev) ...)` and inspect `ev` (`explosionType`, `posX/Y/Z`, `damageScale`, `ownerNetId`, ...). `CancelEvent()` here stops the server from routing the explosion to other clients - the documented mitigation. ([cookbook](https://docs.fivem.net/docs/cookbook/2019/08/19/onesync-intercepting-game-events-such-as-explosions/)) - `CancelEvent()` does not stop other handlers for the same event, and for some game events (e.g. `weaponDamageEvent`, see citizenfx/fivem#2395) cancellation does not reliably sync to clients. Do not rely on cancel as your only control for damage that may already have applied; validate the outcome server-side. - `sender` is the reporting player. Never let `sender`-controlled fields (weapon hash, damage amount, target entity, owner net ID) grant money, items, kills, or eligibility. -- Prefer the built-in server controls over hand-rolled filters for networked-event abuse: `sv_filterRequestControl` (blocks `REQUEST_CONTROL_EVENT` routing - the main entity-control/explosion-by-proxy vector), `sv_enableNetworkedPhoneExplosions` (off by default), `sv_enableNetworkedSounds`, `sv_enableNetworkedScriptEntityStates`, and `block_net_game_event "EVENT_NAME"` for a specific net game event. ([Server Commands](https://docs.fivem.net/docs/server-manual/server-commands/)) +- `sender` arrives as a string (server net ID); call `tonumber(sender)` before comparing it or using it as a table key. This applies to `weaponDamageEvent`, `startProjectileEvent`, `ptFxEvent`, `removeAllWeaponsEvent` and `explosionEvent`. +- Prefer the built-in server controls over hand-rolled filters for networked-event abuse: `sv_filterRequestControl` (FiveM only; blocks `REQUEST_CONTROL_EVENT` routing - the main entity-control/explosion-by-proxy vector), `sv_enableNetworkedPhoneExplosions` (off by default), `sv_enableNetworkedSounds`, `sv_enableNetworkedScriptEntityStates`, and `block_net_game_event "EVENT_NAME"` for a specific net game event. ([Server Commands](https://docs.fivem.net/docs/server-manual/server-commands/)) - Bound and rate-limit per `sender` (particles, projectiles, explosions). Reject, cancel, or log; do not auto-punish a player on a single unverified event. ```lua @@ -406,15 +426,16 @@ The game/OneSync routes client->server events that attackers use as free exploit local lastExplosion = {} AddEventHandler("explosionEvent", function(sender, ev) - if not sender or sender <= 0 then return end + local src = tonumber(sender) -- sender is a string; key by number so playerDropped can clean up + if not src or src <= 0 then return end local now = GetGameTimer() - if now - (lastExplosion[sender] or 0) < 250 then -- flood: stop relaying + if now - (lastExplosion[src] or 0) < 250 then -- flood: stop relaying CancelEvent() return end - lastExplosion[sender] = now + lastExplosion[src] = now - if ev.damageScale > 1.0 then -- server rule, not the client's claim + if (tonumber(ev.damageScale) or 0) > 1.0 then -- server rule, not the client's claim CancelEvent() end end) @@ -481,7 +502,7 @@ end) ``` - The same rule applies to dynamic table/column/identifier names: bind values with `?`/`@name`, and for any dynamic identifier choose from a server-side allowlist - never splice client input into the SQL text. -- For multi-step valuable writes, wrap the parameterized statements in one `MySQL.transaction.await({ ... })` so they commit together or not at all ([oxmysql transaction](https://overextended.dev/docs/oxmysql/Functions/transaction)). Full SQL rules and the API shape are in `skills/common/database.md`. +- For multi-step valuable writes, wrap the parameterized statements in one `MySQL.transaction.await({ ... })` so they commit together or not at all ([oxmysql transaction](https://overextended.dev/docs/oxmysql/Functions/transaction)). A transaction only rolls back on an SQL error: a conditional `UPDATE` that matches 0 rows does not abort it, so check the debit's affected row count before granting (see Give-Value Event Hardening). Full SQL rules and the API shape are in `skills/common/database.md`. ## Config, Convars, And Secrets @@ -501,7 +522,8 @@ end) - Player identity is only reliable server-side, from `GetPlayerIdentifiers(source)` ([GetPlayerIdentifiers](https://docs.fivem.net/docs/scripting-reference/runtimes/lua/functions/GetPlayerIdentifiers/)) or `GetPlayerIdentifierByType`. A client-reported identifier is attacker input, not identity. - Identifier types returned: `steam` (hex), `discord` (int), `xbl` (int), `live` (Microsoft PUID), `license`/`license2` (ROS hash - `license2` can equal `license` for Steam users), `fivem` (Cfx user id), `ip` (IPv4 string). -- Do not trust a single identifier type in isolation. Ban/whitelist on the strongest available identifier(s) (commonly `license`), and treat `ip` as the weakest (shared NAT, rotates). Tune trust with `sv_authMinTrust` (1-5, how spoof-resistant) and `sv_authMaxVariance` (1-5, how stable per provider) ([Server Commands](https://docs.fivem.net/docs/server-manual/server-commands/)). +- Do not trust a single identifier type in isolation. Ban/whitelist on the strongest available identifiers (prefer `license2` when present, else `license`; for bans, store every returned identifier plus `GetPlayerToken` hardware tokens rather than a single type), and treat `ip` as the weakest (shared NAT, rotates). Keep an existing DB key scheme stable when migrating to `license2`. +- Of the connect-time trust convars ([Server Commands](https://docs.fivem.net/docs/server-manual/server-commands/)), only `sv_authMaxVariance` (1-5, default 5) has an effect: FXServer compares it against the lowest variance among a player's identifiers (`license`/`steam` = 1, `ip` = 5), so any value below 5 rejects players with no `license`/`steam` identifier. `sv_authMinTrust` is effectively a no-op, because the always-present `ip:` identifier reports trust 5. See `skills/common/runtime.md` -> Players And Identifiers. - Run ban/whitelist lookups during `playerConnecting` using the `deferrals` API (defer, do async checks, call `done()` exactly once), not after the player is in-game. - The recipient/target of a value is always `source` resolved to its identifiers server-side; never a payload field (see Event Trust Boundary and Give-Value Event Hardening). @@ -534,11 +556,11 @@ Resource-level validation cannot compensate for a soft server platform. These ar - `sv_scriptHookAllowed 0` - keep ScriptHook-style client plugins blocked (default off; never enable on a public server). - `sv_pureLevel 1` (or `2`) - reject clients running modified game files; `2` is strictest. Test against your player base before enforcing. -- `sv_requestParanoia 1..3` - stricter handling of malicious client network requests; raise gradually and watch for false kicks. -- `sv_endpointPrivacy true` - hide player IPs from other clients (blocks player deanonymization/DDoS targeting). +- `sv_requestParanoia 0..3` - counters proxy-based HTTP floods against the server's HTTP endpoints (1 blocks IPs that send a `Via` header; 2 also blocks browser-style requests with `Upgrade-Insecure-Requests`, and info/dynamic/players.json then answer "Nope."; 3 also closes the socket). Default 0. Start at 1 and check that server-list and monitoring tools still work before raising it. +- `sv_endpointPrivacy` - removed on current artifacts: FXServer ignores it (and warns if it is set) because player endpoint addresses are no longer exposed on any HTTP endpoint. On old artifacts it only hid player IPs in the public HTTP reports (`players.json`), not from other clients; the real fix there is updating the artifact (see Operator access). - Leave `rcon_password` unset (rcon disabled) unless genuinely needed; if used, a long unique password never reused anywhere. - `set onesync on` - required for the server-side entity gates used above (`entityCreating`, ownership, routing buckets). -- Verify the convars covered elsewhere in this file are actually set: `setr sv_stateBagStrictMode true` (Sync Safety), `sv_filterRequestControl` (Entity And Net ID Validation / `skills/common/networking.md`), `sv_enableNetworkedPhoneExplosions`/`sv_enableNetworkedSounds`/`sv_enableNetworkedScriptEntityStates` (Built-in Client Events), `sv_authMinTrust`/`sv_authMaxVariance` (Identifier Trust). +- Verify the convars covered elsewhere in this file are actually set: `setr sv_stateBagStrictMode true` (Sync Safety), `sv_filterRequestControl` (Built-in Client Events / `skills/common/networking.md` -> Entity Ownership), `sv_enableNetworkedPhoneExplosions`/`sv_enableNetworkedSounds`/`sv_enableNetworkedScriptEntityStates` (Built-in Client Events), `sv_authMaxVariance` (Identifier Trust; `sv_authMinTrust` is effectively a no-op). ### Operator access @@ -584,7 +606,11 @@ Resource-level validation cannot compensate for a soft server platform. These ar - If code repeatedly uses `for` loops to find one entry by name, ID, category, hash, or source, build an index once and read it directly. - Prefer direct indexed lookup like `Items["INDEX"][itemName]` over scanning the whole item list in hot paths. - Require only config modules used by the current script; do not eager-load large config files into scripts that do not need them. -- Keep hot-loop payloads and state-bag data small. +- Keep hot-loop payloads and state-bag data small; network cost, latent events, and rate limits are in `skills/common/network-performance.md`. +- `GetGamePool('CVehicle')` (and `CPed`, `CObject`, `CPickup`) walks the whole pool; never call it every frame. Scan on a slow timer, or track only entities you created or that entered a zone. +- Do not allocate in per-frame loops: no new tables, closures, string concatenation, or `json.encode` per iteration. Reuse a table or build it only when something changed. +- Resolve hashes once: backtick literals or `joaat` at load time, not `GetHashKey` inside a loop. +- Long server loops (all players, big tables) yield with `Wait(0)` every N iterations so one resource cannot stall the server tick. - Clear RAM caches when they are tied to player/resource lifetime so memory does not grow forever. ## Cleanup @@ -606,7 +632,7 @@ Resource-level validation cannot compensate for a soft server platform. These ar - What can a malicious client fake in this feature? - Which server-side checks reject fake values? - Is this network event actually meant to be public, or should it be a local function (`AddEventHandler`)? -- Does the handler reject `source == 65535` / `source <= 0` when only a real client may trigger it? +- Does the handler reject a non-numeric/empty `source` (server-originated) or `source <= 0` when only a real client may trigger it? - Does the recipient of any value come from `source`, never from a payload field? - Do commands, exports, callbacks, and NUI callbacks validate the same things as server events? - Is every value resolved from server-held state (catalog, balance, permissions), not the payload? @@ -623,7 +649,7 @@ Resource-level validation cannot compensate for a soft server platform. These ar - Are client strings and tables bounded (length, key count, depth) before they are decoded, iterated, stored, or logged? - Does any `SetHttpHandler` endpoint exist, and does it authenticate, bound, and rate-limit like a public event? - Do valuable mutations write an audit trail that could reconstruct a dupe after the fact? -- Are the platform hardening convars set (`sv_scriptHookAllowed 0`, `sv_pureLevel`, `sv_requestParanoia`, `sv_endpointPrivacy`, rcon off), and are third-party resources vetted before install? +- Are the platform hardening convars set (`sv_scriptHookAllowed 0`, `sv_pureLevel`, `sv_requestParanoia`, rcon off), is the FXServer artifact recent, and are third-party resources vetted before install? - Are secrets in server-only `set` convars (never `setr`/`sets`/shared/client)? - Does identity come from `GetPlayerIdentifiers(source)` on the server, and are bans/whitelist checked in `playerConnecting` deferrals? - Are prices, rewards, drop rates, and permission gates kept out of shared/client config or revalidated server-side? diff --git a/skills/common/style.md b/skills/common/style.md index 68aace6..ff1a224 100644 --- a/skills/common/style.md +++ b/skills/common/style.md @@ -52,7 +52,20 @@ }, } ``` -- Use `require(...)` for config/data modules when the runtime supports it. +- Plain CfxLua `require` cannot load resource files; it only returns already-loaded or built-in modules. With ox_lib loaded (`'@ox_lib/init.lua'` in the manifest), `require('configs.items')` works through ox_lib's `lib.require`. Without ox_lib, load split configs through a small cached loader. Either way, a config read on the client must be listed in `files`. + ```lua + local loaded = {} + + local function loadConfig(name) + if loaded[name] then return loaded[name] end + local res = GetCurrentResourceName() + local path = ('configs/%s.lua'):format(name) + local src = LoadResourceFile(res, path) + assert(src, ('missing %s'):format(path)) + loaded[name] = assert(load(src, ('@@%s/%s'):format(res, path)))() + return loaded[name] + end + ``` - Prefer bracket key declarations like `["Name"] = value` for config/data tables. - Bracket keys are especially preferred when the data is meant to be edited by humans or may contain spaces. - Runtime access may use bracket or dot style based on the existing code and hot-path readability. @@ -74,7 +87,7 @@ - `>>=` - `&=` - `|=` - - `^=` + - `^=` - bitwise XOR (`x = x ~ y`), NOT exponentiation. Binary `^` is power in Lua, so write `x = x ^ y` for powers; `^=` needs integer-representable operands. - Do not use `++` or `--`; CfxLua does not implement increment/decrement operators. - Do not assume these operators work in standard Lua, standalone Lua tools, or non-Cfx runtimes. - Prefer compound assignment for simple updates such as counters, bit flags, and accumulated values: @@ -91,8 +104,8 @@ - When config/data grows large or has separate concerns, split it into `configs/*.lua` as the standard pattern. - Each split config file should return a table with `return { ... }`. - Do not turn `config.lua` into an eager aggregator that requires every split config file. -- Require split config files directly in the script that actually uses them. -- Store required config modules in local variables near the top of the script. +- Load split config files directly in the script that actually uses them: `require` only when ox_lib is loaded, otherwise the `loadConfig` loader (see Lua Style above). +- Store loaded config modules in local variables near the top of the script. - Split large position lists, object lists, category lists, shop locations, zone data, and similar datasets into separate config files. - Do not over-split small configs; keep simple values in one readable file. diff --git a/skills/redm/rules.md b/skills/redm/rules.md index 8d243fc..a0b4b37 100644 --- a/skills/redm/rules.md +++ b/skills/redm/rules.md @@ -42,9 +42,9 @@ Use this file for RedM-only resources or when a shared resource needs RDR3-speci ## Ammo -- Ammo by type is an RDR3 concept; FiveM/GTA V uses different ammo natives, so do not share this code without a RedM guard. +- GTA V also has `SetPedAmmoByType`/`GetPedAmmoByType` (same hashes in both games: set `0x5FD1E1F011E76D7E`, get `0x39D22031557946C1`), but the stale reserve-ammo bug and the `RemoveAllPedAmmo` reapply workaround below are RedM-specific. Keep that wrapper behind a RedM guard and use the plain native in FiveM. - Do not rely on direct RedM ammo set/remove natives alone when setting or reducing reserve ammo by type. -- Use a wrapper that clears ammo with `RemoveAllPedAmmo(ped)`, reads the current ammo map from the resource's local player state/helper, reapplies that map, then applies the requested ammo type and amount. +- Use a wrapper that first records the requested ammo type and amount in the resource-owned ammo map (local player state/helper), then clears ammo with `RemoveAllPedAmmo(ped)` and reapplies the full map, so a later call for another type cannot restore a stale value. - Keep the public helper signature clean, such as `SetAmmoByType(ammoType, amount)`. - This avoids RedM ammo state bugs where reserve ammo may not decrease correctly or may restore after using direct ammo natives. - See `memory/redm/native-bugs.md` for the full entry and a reference wrapper.