fix: four HIGH correctness/security fixes from the UI audit (LC-349..352) #367

Merged

longjacksonle merged 4 commits from fix/lc-349-352-audit-high-correctness into main

2026-06-19 05:10:28 +02:00

longjacksonle commented

2026-06-19 05:09:55 +02:00

Owner

Batch of the four HIGH correctness/security findings from the whole-app UI audit. One commit per ticket; each ships a regression test. just test and just test-saas both green.

LC-349 - scheduled-message body not length-capped

post_scheduled / patch_scheduled validated only emptiness. The body renders through synchronous markdown::render on every /scheduled view and at delivery, so an uncapped body was the LC-153 hazard the live path already guards. Now enforces check_message_length. Test: over-cap body -> 400, no row.

LC-350 - poll closes_in overflow panic

Unbounded closes_in made Utc::now() + Duration::minutes(m) overflow-panic (500). Clamped to one year server-side + max on the input. Test: closes_in=9999999999999999 -> 200 (clamped), poll created.

LC-351 - enclave Owner could be demoted

post_member_role had no owner guard; a forged POST could demote the Owner -> ownerless, un-transferable, un-deletable enclave. Now rejects targeting the Owner (mirrors post_kick). Test: demote owner -> 400.

LC-352 - no admin-count floor

post_role / post_ban / post_delete_user could remove the last admin and lock everyone out of /admin. Added db::auth::other_active_admin_exists + guard_not_last_admin refusing demote/ban/delete of the last active admin (role=admin, not banned). Tests: demote/ban sole admin -> 400; demote allowed once a second admin exists. (Admin routes are standalone-gated, so the test file is #![cfg(feature = "standalone")].)

Closes LC-349, LC-350, LC-351, LC-352.

Batch of the four HIGH correctness/security findings from the whole-app UI audit. One commit per ticket; each ships a regression test. `just test` and `just test-saas` both green. ## LC-349 - scheduled-message body not length-capped `post_scheduled` / `patch_scheduled` validated only emptiness. The body renders through synchronous `markdown::render` on every `/scheduled` view and at delivery, so an uncapped body was the LC-153 hazard the live path already guards. Now enforces `check_message_length`. Test: over-cap body -> 400, no row. ## LC-350 - poll closes_in overflow panic Unbounded `closes_in` made `Utc::now() + Duration::minutes(m)` overflow-panic (500). Clamped to one year server-side + `max` on the input. Test: `closes_in=9999999999999999` -> 200 (clamped), poll created. ## LC-351 - enclave Owner could be demoted `post_member_role` had no owner guard; a forged POST could demote the Owner -> ownerless, un-transferable, un-deletable enclave. Now rejects targeting the Owner (mirrors `post_kick`). Test: demote owner -> 400. ## LC-352 - no admin-count floor `post_role` / `post_ban` / `post_delete_user` could remove the last admin and lock everyone out of `/admin`. Added `db::auth::other_active_admin_exists` + `guard_not_last_admin` refusing demote/ban/delete of the last active admin (role=admin, not banned). Tests: demote/ban sole admin -> 400; demote allowed once a second admin exists. (Admin routes are standalone-gated, so the test file is `#![cfg(feature = "standalone")]`.) Closes LC-349, LC-350, LC-351, LC-352.

longjacksonle added 4 commits

2026-06-19 05:09:55 +02:00

fix(scheduled): cap scheduled-message body length on the write path (LC-349) ede91bc216

post_scheduled and patch_scheduled validated only emptiness, never the message-length cap. The stored body is rendered through the synchronous markdown::render on every /scheduled view and again for every viewer at delivery, so an uncapped body (bounded only by Axum's ~2 MiB DefaultBodyLimit) is the LC-153 hazard the live send path already guards against. Enforce crate::routes::room::check_message_length after the empty check in both handlers. Test: an over-MAX_MESSAGE_CHARS body returns 400 and inserts no row.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

fix(polls): clamp poll closes_in to avoid Duration overflow panic (LC-350) 3701e9e68a

closes_in (i64 minutes) was unbounded; the modal input had min=0 but no max. A large value made Utc::now() + Duration::minutes(m) overflow and panic the handler (500). Clamp to MAX_POLL_CLOSE_MINUTES (one year) before the Duration math and add a matching max= on the number input. Test: closes_in=9999999999999999 returns 200 (clamped) and creates the poll instead of 500-ing.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

fix(enclave): refuse role changes that would demote the Owner (LC-351) 4d26c85587

post_member_role ran an unconditioned update_role on the target; the template hides the owner's role control but the handler did not enforce it, so a forged POST could demote the owner and leave the enclave with zero owners (then un-transferable and un-deletable, since leave/delete gate on Owner). Reject when the target's role is Owner, mirroring the existing post_kick owner guard. Test: demoting the owner returns 400.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

fix(admin): block removing the last active admin (LC-352)

check-secrets / Kingfisher (push) Successful in 5s

Details

check-secrets / TruffleHog (push) Successful in 5s

Details

check-secrets / Kingfisher (pull_request) Successful in 5s

Details

check-secrets / Nosey parker (push) Successful in 3s

Details

check-secrets / Nosey parker (pull_request) Successful in 3s

Details

check-secrets / TruffleHog (pull_request) Successful in 4s

Details

Create release / Create release from merged PR (pull_request) Has been skipped

Details

Check / clippy + fmt + tests (pull_request) Successful in 3m27s

Details

7c4bddd127

post_role / post_ban / post_delete_user were gated only by AdminUser, with no admin-count floor, so one admin could demote/ban/delete every other admin (or the last one) and lock everyone out of /admin. Add db::auth::other_active_admin_exists and a guard_not_last_admin helper that refuses demote/ban/delete when the target is the last active admin (role=admin, not banned). Tests: demoting and banning the sole admin return 400; demotion succeeds once a second admin exists.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>