Discord: check bot permissions before handing a token to a node (server-26#225 S3)
start_stream now asks Discord (cached REST snapshots, S2) whether each bot
it could pick may View, Connect and Speak in the voice channel, before the
lease. Covers all four entry points: bot POST /discord/streams, web picker,
nodes discord_join, and the reconciler's re-place.
- pick_token: denied bots skipped; confirmed-ok sorts before unknown;
unknown is allowed (fail open); all denied -> missing_permissions naming
each bot, missing permission, trace reason and fix. The reuse branch
falls through when its token is denied.
- Bounded: discord_perm_check_concurrency at once, within
discord_perm_check_budget_seconds; unanswered = unknown.
- Non-voice channel -> 400 bad_channel (not missing_permissions).
- systems/{id}.preferred_token_id is now read (plan finding 1), as a hint.
- Pool bots at discord_pool_bot_guild_cap are never offered as invites;
pool_full when nothing else is left (owner Q3).
- handle_actual drops cached access on missing_permissions, channel_full,
not_in_guild, bad_channel.
- New GET /discord/nodes/{node_id}/guilds/{guild_id}/voice-check (org
member, names only) and GET /discord/guilds/{guild_id}/voice-check
(service key or admin).
- PUT /tokens/{id}/prefer/{system_id}: org owners for their own systems
and own BYO bots; shared systems and pool bots stay admin-only (owner Q2).
Keeps both pointers single-valued; audited.
- Kill switch: DISCORD_PERM_CHECK_ENABLED=false.
- tests/conftest.py: autouse fixture so no test ever reaches discord.com.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
a75149b2eb
commit
2c2cf9d9be
@@ -49,6 +49,7 @@ from dataclasses import dataclass, field
|
||||
from datetime import datetime, timezone
|
||||
from typing import Optional
|
||||
|
||||
from app.config import settings
|
||||
from app.internal import firestore as fstore
|
||||
|
||||
TOKENS = "bot_tokens"
|
||||
@@ -92,6 +93,25 @@ def guild_of(stream_id: str) -> str:
|
||||
return stream_id.split(":", 1)[0]
|
||||
|
||||
|
||||
def invitable(tok: dict) -> bool:
|
||||
"""May DRB suggest inviting this bot to one more server? A pool bot
|
||||
(``org_id`` None) at ``discord_pool_bot_guild_cap`` servers is full: an
|
||||
unverified Discord bot can't join more than 100, and the cap leaves room
|
||||
(owner decision Q3, 2026-10-05: cap 90, add bots when full). An org's own
|
||||
BYO bot is the org's business, never capped here."""
|
||||
if tok.get("org_id") is not None:
|
||||
return True
|
||||
members = tok.get("guild_ids")
|
||||
return members is None or len(members) < settings.discord_pool_bot_guild_cap
|
||||
|
||||
|
||||
# Permission-check outcomes the lease code acts on (server-26#225 S3). The
|
||||
# full per-bot result is built by discord_streams.check_bots; pick_token reads
|
||||
# only ``status`` (plus name/missing/reason/fix for the refusal).
|
||||
PERM_OK = "ok"
|
||||
PERM_DENIED = "denied"
|
||||
|
||||
|
||||
def _now() -> datetime:
|
||||
return datetime.now(timezone.utc)
|
||||
|
||||
@@ -105,7 +125,11 @@ class LeaseResult:
|
||||
# error codes: pool_empty | no_bot_in_guild | pool_exhausted |
|
||||
# preferred_busy (the chosen bot already has a stream in this guild) |
|
||||
# preferred_other_node (the chosen bot is logged in on another node) |
|
||||
# preferred_unavailable (unknown, invalid, or another org's bot)
|
||||
# preferred_unavailable (unknown, invalid, or another org's bot) |
|
||||
# missing_permissions (every bot that could take it lacks a Discord
|
||||
# permission in the channel — ``detail["denied"]`` names each) |
|
||||
# pool_full (no bot is in this server and every pool bot that could be
|
||||
# invited is at discord_pool_bot_guild_cap)
|
||||
error: Optional[str] = None
|
||||
# bots that would work if invited to this guild: [{name, invite_url}]
|
||||
invite: list = field(default_factory=list)
|
||||
@@ -128,12 +152,23 @@ def _legacy_mirror(lease: Optional[dict]) -> dict:
|
||||
}
|
||||
|
||||
|
||||
def _perm_status(perms: Optional[dict], tok_id: str) -> Optional[str]:
|
||||
return ((perms or {}).get(tok_id) or {}).get("status")
|
||||
|
||||
|
||||
def _denied_row(tok_id: str, tok: dict, perms: dict) -> dict:
|
||||
p = perms.get(tok_id) or {}
|
||||
return {"name": tok.get("name") or tok_id, "missing": list(p.get("missing") or []),
|
||||
"reason": p.get("reason"), "fix": p.get("fix")}
|
||||
|
||||
|
||||
def _candidate_order(tok_id: str, tok: dict, node_id: str, org_id: Optional[str],
|
||||
preferred: Optional[str]) -> tuple:
|
||||
preferred: Optional[str], perms: Optional[dict] = None) -> tuple:
|
||||
lease = tok.get("lease") or {}
|
||||
return (
|
||||
0 if tok_id == preferred else 1,
|
||||
0 if tok_id == preferred else 1, # the system's preferred bot
|
||||
0 if org_id and tok.get("org_id") == org_id else 1, # BYO before pool
|
||||
0 if _perm_status(perms, tok_id) == PERM_OK else 1, # confirmed-ok before unknown
|
||||
0 if lease.get("node_id") == node_id else 1, # reuse a session
|
||||
0 if tok.get("guild_ids") is not None else 1, # known member first
|
||||
str(tok.get("name") or ""),
|
||||
@@ -149,18 +184,32 @@ def pick_token(
|
||||
org_id: Optional[str],
|
||||
preferred_token_id: Optional[str] = None,
|
||||
exclude_stream_id: Optional[str] = None,
|
||||
soft_preferred_token_id: Optional[str] = None,
|
||||
perms: Optional[dict] = None,
|
||||
) -> LeaseResult:
|
||||
"""Pure choice over a snapshot of bot_tokens. No I/O — unit tested directly.
|
||||
|
||||
A ``preferred_token_id`` is a demand, not a hint: if that bot cannot take
|
||||
the stream the result says why instead of quietly using another bot."""
|
||||
the stream the result says why instead of quietly using another bot.
|
||||
``soft_preferred_token_id`` (the system's preferred bot,
|
||||
``systems/{id}.preferred_token_id``) is a hint: tried first, but any other
|
||||
bot that fits is used when it can't.
|
||||
|
||||
``perms`` ({token_id: {status, missing, reason, fix}}, server-26#225) is
|
||||
the Discord permission check for this voice channel. A ``denied`` bot is
|
||||
skipped; a confirmed ``ok`` bot sorts before one whose check was
|
||||
``unknown`` (or absent), and unknown is allowed (fail open: a Discord
|
||||
outage must never block a start). None = no check (kill switch).
|
||||
"""
|
||||
if not tokens:
|
||||
return LeaseResult(ok=False, error="pool_empty")
|
||||
if preferred_token_id:
|
||||
return _pick_preferred(tokens, preferred_token_id, node_id=node_id, guild_id=guild_id,
|
||||
org_id=org_id, exclude_stream_id=exclude_stream_id)
|
||||
org_id=org_id, exclude_stream_id=exclude_stream_id, perms=perms)
|
||||
usable: list[tuple] = []
|
||||
not_member: list[dict] = []
|
||||
denied: list[dict] = []
|
||||
pool_full = False
|
||||
for tok_id, tok in tokens.items():
|
||||
if tok.get("valid") is False:
|
||||
continue
|
||||
@@ -174,24 +223,41 @@ def pick_token(
|
||||
if guild_id in busy_guilds:
|
||||
continue # this bot already has a voice connection in this guild
|
||||
members = tok.get("guild_ids")
|
||||
if members is not None and guild_id not in members:
|
||||
not_member.append({"name": tok.get("name") or tok_id,
|
||||
"invite_url": invite_url(tok.get("application_id"), guild_id)})
|
||||
if (members is not None and guild_id not in members) or _perm_status(perms, tok_id) == "not_member":
|
||||
if invitable(tok):
|
||||
not_member.append({"name": tok.get("name") or tok_id,
|
||||
"invite_url": invite_url(tok.get("application_id"), guild_id)})
|
||||
else:
|
||||
pool_full = True
|
||||
continue
|
||||
usable.append((_candidate_order(tok_id, tok, node_id, org_id, preferred_token_id), tok_id))
|
||||
if _perm_status(perms, tok_id) == "invalid":
|
||||
continue # Discord answered 401 during the check
|
||||
if _perm_status(perms, tok_id) == PERM_DENIED:
|
||||
denied.append(_denied_row(tok_id, tok, perms or {}))
|
||||
continue
|
||||
usable.append((_candidate_order(tok_id, tok, node_id, org_id, soft_preferred_token_id, perms), tok_id))
|
||||
if usable:
|
||||
usable.sort()
|
||||
return LeaseResult(ok=True, token_id=usable[0][1])
|
||||
if denied:
|
||||
# Fixing a permission is the shortest way to radio; inviting another
|
||||
# bot is the other, so the invite links ride along.
|
||||
return LeaseResult(ok=False, error="missing_permissions", invite=not_member,
|
||||
detail={"denied": denied})
|
||||
if not_member:
|
||||
return LeaseResult(ok=False, error="no_bot_in_guild", invite=not_member)
|
||||
if pool_full:
|
||||
return LeaseResult(ok=False, error="pool_full")
|
||||
return LeaseResult(ok=False, error="pool_exhausted")
|
||||
|
||||
|
||||
def _pick_preferred(tokens: dict[str, dict], tok_id: str, *, node_id: str, guild_id: str,
|
||||
org_id: Optional[str], exclude_stream_id: Optional[str]) -> LeaseResult:
|
||||
org_id: Optional[str], exclude_stream_id: Optional[str],
|
||||
perms: Optional[dict] = None) -> LeaseResult:
|
||||
tok = tokens.get(tok_id)
|
||||
owner = (tok or {}).get("org_id")
|
||||
if not tok or tok.get("valid") is False or (owner is not None and owner != org_id):
|
||||
if (not tok or tok.get("valid") is False or (owner is not None and owner != org_id)
|
||||
or _perm_status(perms, tok_id) == "invalid"):
|
||||
return LeaseResult(ok=False, error="preferred_unavailable",
|
||||
detail={"token_name": (tok or {}).get("name") or tok_id})
|
||||
name = tok.get("name") or tok_id
|
||||
@@ -206,9 +272,14 @@ def _pick_preferred(tokens: dict[str, dict], tok_id: str, *, node_id: str, guild
|
||||
return LeaseResult(ok=False, error="preferred_other_node",
|
||||
detail={"token_name": name, "node_id": lease["node_id"]})
|
||||
members = tok.get("guild_ids")
|
||||
if members is not None and guild_id not in members:
|
||||
if (members is not None and guild_id not in members) or _perm_status(perms, tok_id) == "not_member":
|
||||
if not invitable(tok):
|
||||
return LeaseResult(ok=False, error="pool_full", detail={"token_name": name})
|
||||
return LeaseResult(ok=False, error="no_bot_in_guild",
|
||||
invite=[{"name": name, "invite_url": invite_url(tok.get("application_id"), guild_id)}])
|
||||
if _perm_status(perms, tok_id) == PERM_DENIED:
|
||||
return LeaseResult(ok=False, error="missing_permissions",
|
||||
detail={"token_name": name, "denied": [_denied_row(tok_id, tok, perms or {})]})
|
||||
return LeaseResult(ok=True, token_id=tok_id)
|
||||
|
||||
|
||||
@@ -219,12 +290,18 @@ async def lease_for_stream(
|
||||
org_id: Optional[str],
|
||||
preferred_token_id: Optional[str] = None,
|
||||
stream_fields: Optional[dict] = None,
|
||||
soft_preferred_token_id: Optional[str] = None,
|
||||
perms: Optional[dict] = None,
|
||||
) -> LeaseResult:
|
||||
"""Bind ``stream_id`` to a token leased to ``node_id``, atomically.
|
||||
|
||||
Also writes ``stream_fields`` (desired=on, node_id, token_id, lease_id...)
|
||||
onto the stream doc in the same transaction, so a stream doc can never name
|
||||
a token whose lease does not list it.
|
||||
|
||||
``perms`` / ``soft_preferred_token_id``: see ``pick_token``. A stream's
|
||||
current token is reused only while its permission check is not
|
||||
``denied`` — otherwise the stream moves to a bot that can speak there.
|
||||
"""
|
||||
guild_id = guild_of(stream_id)
|
||||
db = fstore.db
|
||||
@@ -243,6 +320,7 @@ async def lease_for_stream(
|
||||
and (prev_tok.get("lease") or {}).get("node_id") == node_id
|
||||
and stream_id in _lease_streams(prev_tok)
|
||||
and prev_tok.get("valid") is not False
|
||||
and _perm_status(perms, prev_tok_id) not in (PERM_DENIED, "invalid", "not_member")
|
||||
# An explicit edit to a different bot moves the stream to it.
|
||||
and (not preferred_token_id or preferred_token_id == prev_tok_id)
|
||||
):
|
||||
@@ -255,7 +333,8 @@ async def lease_for_stream(
|
||||
return LeaseResult(ok=True, token_id=prev_tok_id, lease_id=lease["lease_id"], reused=True)
|
||||
|
||||
result = pick_token(tokens, node_id=node_id, guild_id=guild_id, org_id=org_id,
|
||||
preferred_token_id=preferred_token_id, exclude_stream_id=stream_id)
|
||||
preferred_token_id=preferred_token_id, exclude_stream_id=stream_id,
|
||||
soft_preferred_token_id=soft_preferred_token_id, perms=perms)
|
||||
if not result.ok:
|
||||
return result
|
||||
|
||||
|
||||
Reference in New Issue
Block a user