feat(enclave): per-enclave message send rate-limit override (LC-217) #259
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/lc-217-enclave-msg-rate-limit"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
Per-enclave message send rate-limit override. Enclave admins set a max-messages-per-minute burst at
/enclave/{id}/settings;0= use the site default, non-zero layers in addition to the globalrate_limit_messagescap (never relaxes it). Surfaced during LC-216 review as the operator-controllable anti-spam knob the user explicitly asked for.What ships
chat/0057_enclave_msg_rate_limit.sqladdsenclaves.msg_rate_limit_burst INTEGER NOT NULL DEFAULT 0.Enclave.msg_rate_limit_burst: u32, threaded through all fiveSELECTsites indb/enclave.rs. Newset_msg_rate_limit_burstmutator andget_msg_rate_limit_burst_for_roomcheap JOIN helper.routes/room.rs::post_messageadds a second rate-limit check after the existing global one. Composite keyenc:{eid}:{user.id}so the per-enclave counter is independent of the global counter. DMs (no enclave) and zero-override rooms fall through. 429 withRetry-After.templates/enclave/settings.htmlgets a new section above the emoji card. One number input (0..10000) and a save button; status renders next to the form.POST /enclave/{id}/rate-limitwithMsgRateLimitForm { burst }. Gated through the existingrequire_managepredicate.en/enclave.ftlandes/enclave.ftl.tests/enclave_msg_rate_limit.rs: zero falls through, non-zero blocks at threshold, per-enclave isolation.Test plan
cargo check --workspace --all-targetsclean under standalone and saas.cargo clippy --workspace --all-targets --features standalone -- -D warningsclean.cargo test --test enclave_msg_rate_limitall three tests pass.cargo fmt --allno changes.Out of scope
rate_limit_messagessettings KV is the only knob today; LC-217 layers ON TOP).Enclave admins can now cap message sends per-minute for any member posting in any room inside their enclave, in addition to (never relaxing) the global `rate_limit_messages` setting. Surfaced during LC-216 review as the operator-controllable anti-spam knob the user explicitly asked for. Schema (`server/migrations/chat/0057_enclave_msg_rate_limit.sql`): one new column `enclaves.msg_rate_limit_burst INTEGER NOT NULL DEFAULT 0`. `0` means "use the global cap"; non-zero is a per-minute burst applied to in-enclave posts. Model + DB: - `Enclave.msg_rate_limit_burst: u32` added to `server/src/models/enclave.rs`; all five `SELECT` statements in `server/src/db/enclave.rs` and the matching struct literals carry the new column. - `set_msg_rate_limit_burst(pool, enclave_id, burst)` mutator mirrors the `set_share_emojis_globally` shape. - `get_msg_rate_limit_burst_for_room(pool, room_id)` is the cheap pre-gate helper. JOINs `rooms` -> `enclaves` so the caller does not need a separate "what enclave is this room in" query. Returns `(None, 0)` for DMs (no enclave) and for rooms whose enclave has the override at 0. Enforcement (`server/src/routes/room.rs::post_message`): a new check runs AFTER the existing global `RateLimitKind::Message` gate. It calls `get_msg_rate_limit_burst_for_room`; when both an enclave id and a non-zero burst come back, it consults `state.rate_limits` with a distinct composite key `enc:{eid}:{user.id}` and the enclave's burst. A separate counter key ensures in-enclave activity is tracked independently of the global counter (which the prior call already incremented). DMs and zero-override rooms fall through. 429 with `Retry-After` on deny, mirroring the global path's error shape. Settings UI (`server/templates/enclave/settings.html`): new section above the emoji card. One number input (0..10000) and a save button; current value renders next to the form as "Using site default" or "Limit: N per minute". Gated through the existing `require_manage` predicate that the sibling forms already use. Eight new Fluent keys in `server/locales/{en,es}/enclave.ftl`. Route (`server/src/routes/enclave.rs`): `POST /enclave/{id}/rate-limit` with the `MsgRateLimitForm { burst }` shape. Parses to `u32`, rejects > 10_000 as `BadRequest` defense in depth (UI clamps at 10_000 already). Tests (`server/tests/enclave_msg_rate_limit.rs`): three integration cases covering the three states a reviewer would ask about. Zero override falls through to the global cap (10 fast posts all succeed when global is 0). Non-zero override blocks at its threshold (`burst=3` -> 3 succeed, 4th 429s). Per-enclave isolation (enclave A at `burst=2` blocks at A3, enclave B at `burst=5` independently allows 5 more posts in B then blocks at B6). Verified: `cargo check --workspace --all-targets` clean under both standalone and saas. `cargo clippy --workspace --all-targets --features standalone -- -D warnings` clean. `cargo test --test enclave_msg_rate_limit` all three tests pass. `cargo fmt --all` no changes. Documented in CLAUDE.md is deferred to the LC-217 follow-up reviewer's note (the user's tickets stay terse; this commit body is the durable record). Out of scope for this ticket: per-room override (room-granularity is too fine), site-wide global tunable via admin settings (existing `rate_limit_messages` setting KV remains the only knob; this ticket layers ON TOP). #LC-217Second-pass review of the LC-217 follow-up commit caught one missed `replace_all` site. The inner B-enclave loop in `limits_are_per_enclave` still used the loose `assert!(s == SEE_OTHER || s == OK)` shape because the message text ("B post {i}") differed from the others ("post {i}") so the replace did not match. Tightened to `assert_eq!(s, StatusCode::OK)` like the other five sites in the file. Test still green: `cargo test --test enclave_msg_rate_limit` 3/3 passes. #LC-217Two LOW-severity findings from the second-pass review: 1. "IN ADDITION" shouty all-caps -> lowercase "in addition" (the explanatory paragraph below already calls out the never-relax posture; the all-caps emphasis is redundant noise). 2. POST endpoint added alongside the GET. Was: "Enclave admins set it at `/enclave/{id}/settings`". Now also names `POST /enclave/{id}/rate-limit` with the `require_manage` gate and `burst` form field. Grep-friendly for the next operator hunting the surface. #LC-217