diff --git a/src/commands/mods_only.rs b/src/commands/mods_only.rs index e42c1df..2405596 100644 --- a/src/commands/mods_only.rs +++ b/src/commands/mods_only.rs @@ -1,5 +1,6 @@ use crate::{ - AppState, ApplicationContext, Context, Error, events::interaction_create::DiscordMemberRow, + AppState, ApplicationContext, Context, Error, + events::interaction_create::{DiscordMemberRow, user_attempt_lock}, redact_digits, }; use poise::{CreateReply, serenity_prelude as serenity}; @@ -219,6 +220,15 @@ pub async fn unlink( return Ok(()); } + // Serialize against the TARGET user's own verification (SEC-19): this holds the + // same per-user async lock verification uses, keyed on the user being unlinked, so + // an in-flight verify for them cannot interleave with our delete + role removal and + // leave a role-with-no-row (or the inverse). Acquired AFTER defer so waiting on a + // running verify never eats the ack window; the tokio guard is held across the DB + // and Discord calls below. + let target_lock = user_attempt_lock(ctx.data, user.id); + let _target_guard = target_lock.lock().await; + let user_id = user.id.to_string(); let moderator = &interaction.user; diff --git a/src/events/interaction_create.rs b/src/events/interaction_create.rs index 7c24054..afe01be 100644 --- a/src/events/interaction_create.rs +++ b/src/events/interaction_create.rs @@ -95,7 +95,11 @@ fn record_failure(attempts: &AttemptMap, user_id: UserId) { /// released before the caller awaits the returned tokio lock, so no std guard is /// ever held across an await. Entries no attempt is using any more (only the map /// still references them) are pruned so the map cannot grow without bound. -fn user_attempt_lock(data: &Data, user_id: UserId) -> Arc> { +/// +/// Shared with `/unlink`, which acquires the lock keyed on its TARGET user so a +/// concurrent verify for that user cannot interleave with the delete + role removal +/// (SEC-19). Callers hold the returned tokio guard across their whole operation. +pub(crate) fn user_attempt_lock(data: &Data, user_id: UserId) -> Arc> { let mut locks = data .state .verify_locks diff --git a/src/main.rs b/src/main.rs index 784d9d3..95de5d6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -235,15 +235,23 @@ async fn on_error(error: poise::FrameworkError<'_, Data, Error>) { #[tokio::main] async fn main() { - // Initialise logging first, before anything can log. Default to `info`: - // supabase-lib-rs logs generated query URLs (containing student IDs) and the - // service-account email at `debug`, so RUST_LOG must never be set to debug or - // trace on the VPS. See OPS-04 and the README. - tracing_subscriber::fmt() - .with_env_filter( - tracing_subscriber::EnvFilter::try_from_default_env().unwrap_or_else(|_| "info".into()), - ) - .init(); + // Initialise logging first, before anything can log. Default to `info`, then floor + // the supabase crate at `info` *regardless of RUST_LOG*. supabase-lib-rs logs the + // generated query URL — `…student_id=eq.` — and the service-account email via + // `tracing::debug!` (its target is `supabase`, its [lib] name). That bypasses our + // own `redact_digits`, so an operator setting `RUST_LOG=debug` to debug something + // else would leak student ids. `add_directive` replaces any same-target directive + // parsed from RUST_LOG, so this wins over `RUST_LOG=debug` and even an explicit + // `RUST_LOG=supabase=debug` (only a hyper-specific `supabase::database=debug` could + // override it). Our own modules keep their normal level. See OPS-04 and the README. + let env_filter = tracing_subscriber::EnvFilter::try_from_default_env() + .unwrap_or_else(|_| tracing_subscriber::EnvFilter::new("info")) + .add_directive( + "supabase=info" + .parse() + .expect("static tracing directive `supabase=info` is valid"), + ); + tracing_subscriber::fmt().with_env_filter(env_filter).init(); dotenv().ok(); // load env