feat(inputs): opt-in authorize_controlled_targets helper (spoofed-target defense)

[mmannerm]

Summary

Reframes this PR as an opt-in helper for the spoofed-target defense, built on the validation seam in #1535 — rather than a compulsory, inline authorization check.

Per the discussion on #1531, Controlled / ControlledBy are optional, non-authoritative helpers (some games legitimately let several clients drive one entity), so lightyear should not enforce target authorization by default. Instead it offers a ready-made check a game can switch on:

app.add_input_validator(authorize_controlled_targets::<MySequence>);

The defense

receive_input_message reads InputTarget::Entity(entity) off the wire and applies it to that entity's InputBuffer with no ownership check — so a modified client can send InputTarget::Entity(other_player) to drive an entity it doesn't control.

authorize_controlled_targets::<S> is a public InputSystems::ValidateInputs system that strips every InputTarget::Entity the sender isn't authorized to control (not in its ControlledByRemote). The message itself is kept even if filtering removes all its targets — an empty InputMessage is a legitimate keepalive that still carries end_tick and advances confirmation; dropping it would stall the confirmed tick and risk a large rollback. Only the unauthorized targets are removed (before any rebroadcast). Host-client inputs (is_local) are exempt; InputTarget::PreSpawned (hash-identified) is passed through. It logs (trace!) when it strips targets, so a missing ControlledBy is diagnosable.

To run your own validation after this helper, order it with .after(authorize_controlled_targets::<S>).

Tests

• test_authorize_controlled_targets_helper — client 0 controls A and forges a target on uncontrolled B; A's authorized input lands (non-overblocking), B's spoofed input is dropped.
• test_authorize_controlled_targets_keeps_emptied_message — a fully-unauthorized message is stripped but kept (the keepalive isn't dropped); input doesn't land and the pipeline stays healthy.
• test_authorize_controlled_targets_exempts_host_client (host-server) — a host-client's inputs are not stripped (is_local bypass); verified discriminating.
• test_authorize_controlled_targets_passes_through_prespawned — a forged PreSpawned target survives the filter.
• test_custom_validator_ordered_after_authorize — a custom validator ordered .after the helper sees only authorized targets.

Notes

• Stacked on feat(inputs): server-side input-validation seam (ValidateInputs) #1535 (the ValidateInputs seam + MessageReceiver::retain_messages this builds on). Once feat(inputs): server-side input-validation seam (ValidateInputs) #1535 merges this rebases down to just the helper + test.
• Default-on enforcement was deliberately not chosen (it would make ControlledBy mandatory and break games/tests that don't use it — see the audit on feat(inputs): server-side input-validation seam (ValidateInputs) #1535). This is the opt-in middle ground: lightyear ships the check and documents the security tradeoff, the game decides.
• Design discussion: RFC: server-side input-validation seam (ValidateInputs) #1531. Supersedes this PR's earlier inline implementation.

🤖 Generated with Claude Code
Co-authored with Claude Opus 4.8

[mmannerm]

Audit: opt-in authorize_controlled_targets helper (#1526)

Verdict: [APPROVE] — the BLOCK below (empty-keepalive drop) was fixed; the test follow-ups are closed. Re-run at c3a9cdcf (rebased on #1535 08ee8e04, + validator-ordering docs/test). Merge after #1535.

Build / test / lint (re-run at c3a9cdcf)

• build: ✓ · fmt: ✓ · clippy (-D warnings, --all-targets): ✓
• test (cargo test -p lightyear_tests --lib client_server::input host_server::input, serial): ✓ 30 passed, 0 failed (+keepalive, +host-client exemption, +PreSpawned, +ordering).

Resolution

[was BLOCK] helper dropped legitimately-empty keepalive messages → confirmed-tick stall / massive rollback

Verified: clients deliberately send empty InputMessages ("0 inputs is itself information", client.rs:721); the server deliberately does not drop them (guard commented out, server.rs:331-333: dropping → "LastConfirmedTick really old → massive rollback"). The helper's !message.inputs.is_empty() dropped pre-filter-empty messages, reintroducing the stall.

Fixed: the helper now strips unauthorized targets but always keeps the message — spoofed entries are already removed before any rebroadcast, and an emptied message is a valid keepalive the receive path handles. Removes the empty-vs-emptied ambiguity entirely. Added a regression test (test_authorize_controlled_targets_keeps_emptied_message) and a trace! on drops (Codex P2 — a missing ControlledBy is now diagnosable).

Findings (non-blocking)

Security (invariants): This is the spoofed-target defense; logic sound — authorized InputTarget::Entity kept, unauthorized stripped before rebroadcast, host-client (is_local) exempt, PreSpawned passed through. No new vulnerability; opt-in, no default behavior change.

Simplicity: SIMPLIFY — With<Connected> on the query is arguably dead weight (a disconnected receiver has no messages), but it mirrors receive_input_message for consistency; low value either way. Kept.

Test quality: all documented branches covered: authorized-lands, spoofed-stripped, empty-after-filter keepalive, host-client is_local exemption (host-server test, verified discriminating), PreSpawned pass-through, and custom-validator ordering via .after(authorize_controlled_targets::<S>) (also verified discriminating — fails when ordered .before).

---

Run by /audit-task. Re-runs edit this comment in place.

[mmannerm]

Ready for review/merge (stacked on #1535). This is the opt-in authorize_controlled_targets helper — not default-on (ControlledBy stays optional) — keeping emptied messages as keepalives, stripping only unauthorized targets before rebroadcast, with a trace! on drops. Coverage: authorized-lands, spoofed-stripped, empty-keepalive, host-client is_local exemption, PreSpawned pass-through, and custom-validator ordering via .after(...). Codex review + /audit-task both APPROVE, no open findings.

Please merge #1535 first; I'll rebase this down to just the helper + tests afterward.

CI: Doc + Lint green; Test job running (suite passes locally — 30/30 across client_server::input + host_server::input).

🤖 Co-authored with Claude Code (Claude Opus 4.8)

[mmannerm]

Rebased onto main now that #1535 is merged — this PR is down to just the authorize_controlled_targets helper + tests (the seam commits dropped out, as they're now in main). No conflicts; ready for review. Local suite green (30/30 across client_server::input + host_server::input); CI re-running.

🤖 Co-authored with Claude Code (Claude Opus 4.8)