[Hardening] Require authorization for /record and verify session↔guild ownership on bot audio endpoints #100
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
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?
Context
Recording controls two authorization gaps, one in each component. Per the consent model, all group members have standing consent to be recorded — but starting/stopping recordings and attaching audio to arbitrary sessions should still be restricted.
Bot half —
/record start(bot/questboard_bot/cogs/recording.py:199-242) and/record stop(:427-445) are invocable by any guild member. There is no role, permission, or GM check anywhere in either handler: any member in the server can start a recording (joining a voice channel) or kill an in-progress recording mid-session.Backend half —
POST /api/bot/sessions/{session_id}/audio(webapp/backend/app/routers/bot.py:657-707) authorizes only theX-Bot-Keyheader (require_bot_auth,:663). The request payload already carriesguild_id(AudioUploadRequest,:652) and it is passed to the Celery task (:702) — but the endpoint never verifies that the session's campaign belongs to that guild. Any valid session UUID is accepted, so audio from guild A can be attached to (and overwrite processing state of) a session in a campaign belonging to guild B.Fix / Spec
Bot:
record_startandrecord_stop, allow the invocation only if:self.bot.apiclient lookups — campaign/member endpoints used elsewhere in the cogs), orinteraction.user.guild_permissions.manage_guild)._starting/ touching_activestate.Backend:
4. In
bot_upload_audio, after loading the session, load its campaign and requiresession.campaign.guild_id == data.guild_id; on mismatch return 404 (don't leak session existence across guilds) or 403 — pick one and be consistent.5. Audit the other
/api/bot/*endpoints that accept asession_idplus guild context supplied by the bot (record status/summary/notes/transcript-related endpoints inwebapp/backend/app/routers/bot.py) and apply the same ownership check wherever the bot supplies a guild context. Endpoints keyed only byguild_id(e.g./bot/guilds/{guild_id}/...,:416,:469,:505,:884) already scope by guild.Both halves ship in the same PR (cross-component change).
Acceptance criteria
/record startand/record stop; the GM and Manage Guild holders succeed.session_idwhose campaign belongs to a differentguild_idis rejected (403/404) and queues no Celery task.bot/tests/test_recording.pycover the permission gate.References
bot/questboard_bot/cogs/recording.py:199-242(record_start, no auth),:427-445(record_stop, no auth)webapp/backend/app/routers/bot.py:650-655(AudioUploadRequestwithguild_id),:657-707(bot_upload_audio, no ownership check),:663(require_bot_authonly)webapp/backend/app/auth/dependencies.py(require_bot_auth)Filed from the July 2026 full-project review.
Queued as part of a v3.3.0 push. This one is cross-component (bot
/recordauthorization + backend session↔guild ownership check), and it touches bothcogs/recording.pyandrouters/bot.py— both of which are being changed on thehardening/botandhardening/backendbranches right now (#83/#85 and #109 respectively).To avoid a three-way conflict it will land last, on its own branch rebased on top of those two, so the bot and backend halves still ship in a single PR as the issue requires.
Fixed on
main(commit2b58ed6, merged via0ddb35d) — both halves in a single commit, per the cross-component rule.Bot —
/record startand/record stopnow authorize before touching any recording state: allowed only if the invoker holds Discord Manage Server, or is the campaign's verified-linked GM (resolved via a newGET /api/bot/guilds/{guild_id}/gms). Fails closed — an API error or an unlinked guild denies with a distinct "couldn't verify, try again" message, kept separate from a plain "not authorized" so a transient outage reads correctly.Backend —
POST /api/bot/sessions/{session_id}/audionow loads the session's campaign and requirescampaign.guild_id == data.guild_id, returning 404 on mismatch (no cross-guild existence probing) and queuing no Celery task. Cross-guild audio attach is closed.Endpoint audit (step 6):
/audiois the only/api/bot/*endpoint carrying both asession_idand a bot-suppliedguild_id. The other session-keyed endpoints (vote,attendance,timeslots,summary,notes,transcript-feedback,lore-proposals) take no guild claim, so there is nothing to cross-check;/guilds/{guild_id}/*endpoints already scope by guild. No further ownership checks needed.Tests: bot permission gate (Manage Server allow, GM allow, non-GM deny, API-failure/unlinked deny, and denial-before-state-mutation for both start and stop); backend cross-guild upload rejected with
process_audio.delayasserted not called, plus the GM lookup endpoint (verified-link filtering, 404 on unlinked guild). Two pre-existing/audiotests updated to set matching guild ownership. Bot suite 158, backend 358, ruff clean.