Raw RCON replies (including player IPs) are exposed to every viewer #61

New Issue

2026-10-08T16:33:28-04:00

goddard commented

2026-10-08 16:33:28 -04:00

Raw RCON status replies, which include each player's ip:port, reach every role, including viewers.

rcon.ts:373,378 keeps up to 4,000 characters of the raw status reply, whether the check passes or fails.
worker.ts:230 and status-updates.ts:11 save it as last_reply_snippet.
status.ts:57 and worker.ts:264 put it in every fleet snapshot and in the /api/events/servers stream. Viewers can read both (access.ts:44), and the Diagnostics tab shows it to every role.

The parsed roster deliberately leaves out player addresses, and the agent comments flag them as sensitive. This also contradicts the docs, which say raw diagnostics are operator-only.

Fix

Remove replySnippet from fleet snapshots and the /api/events/servers stream
Serve it from an operator-only endpoint
Strip ip:port before saving
Don't save the snippet when the check succeeds

Raw RCON `status` replies, which include each player's `ip:port`, reach every role, including viewers. - `rcon.ts:373,378` keeps up to 4,000 characters of the raw `status` reply, whether the check passes or fails. - `worker.ts:230` and `status-updates.ts:11` save it as `last_reply_snippet`. - `status.ts:57` and `worker.ts:264` put it in every fleet snapshot and in the `/api/events/servers` stream. Viewers can read both (`access.ts:44`), and the Diagnostics tab shows it to every role. The parsed roster deliberately leaves out player addresses, and the agent comments flag them as sensitive. This also contradicts the docs, which say raw diagnostics are operator-only. **Fix** - [x] Remove `replySnippet` from fleet snapshots and the `/api/events/servers` stream - [x] Serve it from an operator-only endpoint - [x] Strip `ip:port` before saving - [x] Don't save the snippet when the check succeeds

goddard added the

priority

high

type/security labels 2026-10-08 16:39:08 -04:00

josh referenced this issue from a commit

2026-10-08 16:47:57 -04:00

Protect RCON diagnostics and redact player addresses (Refs #61)

goddard commented

2026-10-08 16:50:37 -04:00

Reviewed 0cc4498b53. All four fix items are done, so I'm closing this.

Checklist

Removed from snapshots and stream. replySnippet is gone from ServerStatusEntry, statusEntry and the UI stores. serverObservation now strips last_reply_snippet before spreading the row, so neither the fleet snapshot nor /api/events/servers carries it.
Operator-only endpoint. The new GET /api/servers/[name]/rcon-diagnostics is added to the operator list in routePermission (covered in auth-access/auth-guard tests). It returns the snippet only while last_error is set. The Diagnostics tab fetches it only when canOperate, and the "Test connection" button is hidden for viewers. probe-rcon is a POST, so it was already operator-only.
Addresses stripped before saving. redactRconAddresses runs on the Latin-1 string before the hexdump is built, so an address can't be split across the hex/ASCII columns. It's applied again by failureReplySnippet and at read time. A one-time migration (rcon_private_diagnostics_v1) clears old snippets and the old print reply not recognized: "…" errors, which could have contained reply text.
Not saved on success. The success branch of RconStatusResult no longer has rawReply (rawReply?: never). successfulStatusFields, the worker and probe-rcon all write NULL on success.

Review notes

A good extra catch: the viewer-visible last_error detail no longer echoes the first line of the reply body. That would have been a second leak path.
The 7 affected test files pass locally (265 tests).
Minor and non-blocking: the IPv6 alternative in the redaction regex also matches colon-separated times (e.g. uptime 12:34:56 → uptime [redacted]), and 1.2.3.4.5 becomes [redacted].5. That only over-redacts operator diagnostics, so it's safe. Tighten it only if it starts hiding useful output.
Docs (authentication.md) are updated to list RCON diagnostics as operator-only, which matches the behaviour.

Reviewed 0cc4498b53091c9739e32a2b538ffd66ed61936f. All four fix items are done, so I'm closing this. **Checklist** - [x] **Removed from snapshots and stream.** `replySnippet` is gone from `ServerStatusEntry`, `statusEntry` and the UI stores. `serverObservation` now strips `last_reply_snippet` before spreading the row, so neither the fleet snapshot nor `/api/events/servers` carries it. - [x] **Operator-only endpoint.** The new `GET /api/servers/[name]/rcon-diagnostics` is added to the operator list in `routePermission` (covered in `auth-access`/`auth-guard` tests). It returns the snippet only while `last_error` is set. The Diagnostics tab fetches it only when `canOperate`, and the "Test connection" button is hidden for viewers. `probe-rcon` is a POST, so it was already operator-only. - [x] **Addresses stripped before saving.** `redactRconAddresses` runs on the Latin-1 string *before* the hexdump is built, so an address can't be split across the hex/ASCII columns. It's applied again by `failureReplySnippet` and at read time. A one-time migration (`rcon_private_diagnostics_v1`) clears old snippets and the old `print reply not recognized: "…"` errors, which could have contained reply text. - [x] **Not saved on success.** The success branch of `RconStatusResult` no longer has `rawReply` (`rawReply?: never`). `successfulStatusFields`, the worker and `probe-rcon` all write `NULL` on success. **Review notes** - A good extra catch: the viewer-visible `last_error` detail no longer echoes the first line of the reply body. That would have been a second leak path. - The 7 affected test files pass locally (265 tests). - Minor and non-blocking: the IPv6 alternative in the redaction regex also matches colon-separated times (e.g. `uptime 12:34:56` → `uptime [redacted]`), and `1.2.3.4.5` becomes `[redacted].5`. That only over-redacts operator diagnostics, so it's safe. Tighten it only if it starts hiding useful output. - Docs (`authentication.md`) are updated to list RCON diagnostics as operator-only, which matches the behaviour.

goddard closed this issue

2026-10-08 16:50:37 -04:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: josh/electric-cherry#61