Skip to content

fix(security): RBAC M4 follow-up — role_cache reload + Telegram DM owner-escalation#3241

Merged
houko merged 1 commit into
mainfrom
followup/3202-rbac-m4-fixes
Apr 26, 2026
Merged

fix(security): RBAC M4 follow-up — role_cache reload + Telegram DM owner-escalation#3241
houko merged 1 commit into
mainfrom
followup/3202-rbac-m4-fixes

Conversation

@houko

@houko houko commented Apr 26, 2026

Copy link
Copy Markdown
Contributor

Summary

Two follow-up security fixes for merged PR #3202 (RBAC M4 — channel-native role mapping). Both touch crates/librefang-kernel/src/auth.rs and ship together because they share the channel-role resolution path.

Fix 1 — AuthManager::reload() left role_cache populated

reload() cleared users and channel_index but never role_cache. After an operator edited [[users]] channel bindings or [channel_role_mapping] and triggered a hot reload, any sender whose role was already resolved that session kept the old (potentially elevated) role from the cache until the daemon restarted. The new policy only applied to senders the cache hadn't seen yet.

reload() now calls self.role_cache.clear() alongside the existing clears. DashMap::clear takes per-shard locks internally, so no extra coordination is needed even if a resolve_role_for_sender call races the reload — concurrent calls observe either pre- or post-clear state, never a torn one.

Fix 2 — Telegram DM creator auto-promoted every DMer to Owner

getChatMember(chat_id=user_id, user_id=user_id) — what the resolver issues for a Telegram DM — returns status = "creator" because the user "owns" their own private conversation with the bot. With the documented mapping creator_role = "owner", any user who DMs the bot was silently promoted to Owner of the LibreFang instance.

The fix wraps translate_platform_role with a sender-aware variant that detects the self-DM signature (chat_id == sender.user_id and !is_group) for Telegram and drops the creator token in that case, falling through to default-deny Viewer. Group/supergroup creator (the legitimate chat owner) is unaffected, and administrator / member tokens are not gated since they don't appear for the self-DM query and aren't a privilege risk.

Repro for Fix 2 (pre-fix)

  1. Configure a Telegram channel and add to config.toml:
    [channel_role_mapping.telegram]
    creator_role = "owner"
    admin_role   = "admin"
    member_role  = "user"
  2. Have any user (no [[users]] binding required) open a private chat with the bot and send a message.
  3. The kernel calls getChatMember(chat_id=<user_id>, user_id=<user_id>), gets back creator, maps it to owner via the configured creator_role, and the user now passes every authorization check up to and including UserRole::Owner.

Post-fix the same flow resolves to Viewer (default-deny). Group chats where someone is genuinely the owner still resolve to Owner as designed.

Tests

Added to auth::channel_role_tests in crates/librefang-kernel/src/auth.rs:

  • telegram_dm_creator_does_not_auto_promote_to_ownerchat_id == user_id + creator mapping must yield Viewer, not Owner.
  • telegram_group_creator_still_maps_to_owner — guards that the legitimate group-owner case is unchanged (is_group: true, chat_id != user_id).
  • telegram_dm_administrator_unaffected_by_dm_guard — confirms the DM gate is creator-specific and doesn't accidentally suppress other tokens.
  • reload_clears_role_cache_so_mapping_edits_take_effect — populates the cache via one resolve, calls reload() with a different mapping, asserts the next resolve reflects the new mapping (not the cached old role).

All 46 tests in auth::* pass; cargo test -p librefang-kernel -p librefang-channels --lib is green (1451 tests).

Test plan

  • cargo check -p librefang-kernel -p librefang-channels --lib --tests — clean
  • cargo test -p librefang-kernel -p librefang-channels --lib — 627 + 824 passing
  • cargo clippy -p librefang-kernel -p librefang-channels --lib --tests -- -D warnings — zero warnings
  • cargo fmt --all
  • Manual: live integration test against a real Telegram bot in a fresh DM, verify the user resolves to Viewer not Owner after the fix.

…ner-escalation

Two follow-up fixes for MERGED #3202 (RBAC M4 channel-native role mapping):

1. AuthManager::reload() now clears role_cache alongside users and
   channel_index. Previously, an operator editing channel bindings or
   [channel_role_mapping] and triggering a hot reload would still see
   the OLD resolved role for any sender already cached this session —
   the new policy applied only to fresh senders. Cached entries kept
   stale (often elevated) privileges until daemon restart.

2. Telegram DM `creator` no longer auto-promotes to Owner. The Bot API
   returns status="creator" for getChatMember(chat_id=user_id,
   user_id=user_id) because the user "owns" their own DM with the bot.
   Combined with creator_role = "owner", any user who DMed the bot was
   silently elevated to Owner. New translate_platform_role_for_sender()
   detects the self-DM signature (chat_id == user_id, !is_group) and
   drops the `creator` token so resolution falls through to default-
   deny Viewer. Group `creator` (the legitimate chat owner) and
   `administrator` / `member` tokens are untouched.

Tests added in channel_role_tests:
- telegram_dm_creator_does_not_auto_promote_to_owner
- telegram_group_creator_still_maps_to_owner
- telegram_dm_administrator_unaffected_by_dm_guard
- reload_clears_role_cache_so_mapping_edits_take_effect
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@github-actions github-actions Bot added ready-for-review PR is ready for maintainer review area/kernel Core kernel (scheduling, RBAC, workflows) size/M 50-249 lines changed and removed ready-for-review PR is ready for maintainer review labels Apr 26, 2026
@houko
houko merged commit 6453e8b into main Apr 26, 2026
23 checks passed
@houko
houko deleted the followup/3202-rbac-m4-fixes branch April 26, 2026 14:28
@houko houko mentioned this pull request Apr 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/kernel Core kernel (scheduling, RBAC, workflows) size/M 50-249 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant