From b960d84bfc6d7c283b72e61f6f55c6df2e3b2a3a Mon Sep 17 00:00:00 2001 From: Clupai8o0 Date: Sun, 30 Aug 2026 15:35:16 +1000 Subject: [PATCH] COL-BOT-05: parse all config once in AppState, remove per-event env reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finishes the AppState config hoisting (COR-12's groundwork was not present in the repo, so this ticket establishes it): AppState now carries guild_id, honeypot_channel_id, leetcode_channel_id, verified_role_id, logs_channel_id and weather_token, all parsed once in AppState::new(). A missing or unparseable id now produces one boot error naming every offending variable plus a pointer to .env.example, instead of a per-event .expect() panic (a data race under the multi-threaded runtime, since dotenv() calls the now-unsafe set_var). - main.rs: new fields, required_u64() collector, fail-fast with combined message. - events/message.rs: honeypot/create_leetcode_thread/on_message take &Data and read channel/guild ids from AppState; dropped dotenv() and env::var. - events/interaction_create.rs: deleted verified_role_id(); use data.state.verified_role_id. - commands/mods_only.rs: log_embed takes logs_channel_id: ChannelId; caller in message.rs passes data.state.logs_channel_id. - commands/weather.rs: WEATHER_TOKEN read once into AppState (non-fatal), passed to get_weather — required so the new CI grep guard can be clean. - .env.example: add LEETCODE_CHANNEL_ID. - .github/workflows/ci.yml: add "No environment reads outside main.rs" guard. grep -rn 'dotenv()\|env::var' src/ | grep -v '^src/main.rs:' now returns nothing. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_017XrE7F9ZuBWdQnS8CZvYDE --- .env.example | 1 + .github/workflows/ci.yml | 11 +++++++ src/commands/mods_only.rs | 10 ++----- src/commands/weather.rs | 6 ++-- src/events/interaction_create.rs | 13 +------- src/events/message.rs | 44 +++++++++++---------------- src/main.rs | 51 +++++++++++++++++++++++++++++++- 7 files changed, 84 insertions(+), 52 deletions(-) diff --git a/.env.example b/.env.example index 05f3f0a..1fe8a2a 100644 --- a/.env.example +++ b/.env.example @@ -5,6 +5,7 @@ VERIFIED_ROLE_ID="" GUILD_ID="" LOGS_CHANNEL_ID="" HONEYPOT_CHANNEL_ID="" +LEETCODE_CHANNEL_ID="" SUPABASE_URL="" SUPABASE_KEY="" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index abea2e6..68d4881 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -80,3 +80,14 @@ jobs: - name: Test run: cargo test + + # COL-BOT-05: configuration is parsed once in main.rs and stored on + # AppState. A per-event dotenv()/env::var read is a data race under the + # multi-threaded runtime (set_var is unsafe in edition 2024) and turns a + # config typo into a random future handler failure instead of a boot error. + - name: No environment reads outside main.rs + run: | + if grep -rn 'dotenv()\|env::var' src/ | grep -v '^src/main.rs:'; then + echo "::error::Environment variables must be read once in main.rs and stored on AppState." + exit 1 + fi diff --git a/src/commands/mods_only.rs b/src/commands/mods_only.rs index 015bf93..689448a 100644 --- a/src/commands/mods_only.rs +++ b/src/commands/mods_only.rs @@ -1,5 +1,4 @@ use crate::{Context, Error}; -use dotenv::dotenv; use poise::{CreateReply, serenity_prelude as serenity}; /// Send message to logs channel @@ -8,6 +7,7 @@ use poise::{CreateReply, serenity_prelude as serenity}; #[allow(clippy::too_many_arguments)] pub async fn log_embed( ctx: &serenity::Context, + logs_channel_id: serenity::ChannelId, title: Option, title_url: Option, description: Option, @@ -17,7 +17,6 @@ pub async fn log_embed( image_url: Option, timestamp: Option, ) -> Result<(), Error> { - dotenv().ok(); let mut embed = serenity::CreateEmbed::new(); // Set title and title URL @@ -60,12 +59,7 @@ pub async fn log_embed( embed = embed.timestamp(serenity::Timestamp::now()); } - // Send the embed - let logs_channel_id_env = std::env::var("LOGS_CHANNEL_ID") - .expect("missing LOGS_CHANNEL_ID") - .parse::() - .expect("Invalid LOGS_CHANNEL_ID value"); - let logs_channel_id = serenity::ChannelId::new(logs_channel_id_env); + // Send the embed (logs_channel_id is parsed once at boot; see AppState). let builder = serenity::CreateMessage::new().embed(embed); let send_log = logs_channel_id.send_message(ctx, builder).await; diff --git a/src/commands/weather.rs b/src/commands/weather.rs index 8b7dd02..f7a018b 100644 --- a/src/commands/weather.rs +++ b/src/commands/weather.rs @@ -2,9 +2,7 @@ use crate::{Context, Error}; use poise::CreateReply; use serenity::{all::CreateEmbed, json::Value}; -async fn get_weather(location: String) -> Result { - let weather_api_key = std::env::var("WEATHER_TOKEN").expect("missing WEATHER_TOKEN"); - +async fn get_weather(location: String, weather_api_key: &str) -> Result { let request_url = format!( "https://api.weatherapi.com/v1/current.json?key={key}&q={location}", key = weather_api_key, @@ -23,7 +21,7 @@ pub async fn weather( ctx: Context<'_>, #[description = "Location (City or Country)"] location: String, ) -> Result<(), Error> { - let weather_response = get_weather(location).await?; + let weather_response = get_weather(location, &ctx.data().state.weather_token).await?; let value: Value = serde_json::from_str(&weather_response)?; let location_name = value["location"]["name"].as_str().unwrap(); diff --git a/src/events/interaction_create.rs b/src/events/interaction_create.rs index 1c04f06..5577a73 100644 --- a/src/events/interaction_create.rs +++ b/src/events/interaction_create.rs @@ -12,7 +12,6 @@ use ::serenity::{ }, model::guild::Member, }; -use dotenv::dotenv; use poise::Modal as _; use poise::serenity_prelude as serenity; use serde::{Deserialize, Serialize}; @@ -23,16 +22,6 @@ pub struct DiscordMemberRow { pub discord_id: String, } -/// Read the verified role id from the environment. -fn verified_role_id() -> RoleId { - dotenv().ok(); - let role_id_string = std::env::var("VERIFIED_ROLE_ID").expect("missing VERIFIED_ROLE_ID"); - let role_id_u64: u64 = role_id_string - .parse() - .expect("Unable to parse VERIFIED_ROLE_ID into number"); - RoleId::new(role_id_u64) -} - /// Wrap an embed into an ephemeral interaction response. fn ephemeral_embed(embed: CreateEmbed) -> CreateInteractionResponse { CreateInteractionResponse::Message( @@ -204,7 +193,7 @@ async fn handle_verify( return Ok(()); }; - let verified_role_id = verified_role_id(); + let verified_role_id = data.state.verified_role_id; // Fast, no-network "already verified" check using the member data that is // already attached to the button interaction. Anything slower than this diff --git a/src/events/message.rs b/src/events/message.rs index 2f52edb..fedfd45 100644 --- a/src/events/message.rs +++ b/src/events/message.rs @@ -1,21 +1,13 @@ -use crate::{Error, commands::mods_only::log_embed}; -use ::serenity::model::id::{ChannelId, GuildId}; -use dotenv::dotenv; +use crate::{Data, Error, commands::mods_only::log_embed}; use poise::serenity_prelude as serenity; -async fn honeypot(ctx: &serenity::Context, new_message: &serenity::Message) -> Result<(), Error> { - dotenv().ok(); - // check if it's the honeypot channel - let discord_guild_id = std::env::var("GUILD_ID") - .expect("missing GUILD_ID") - .parse::() - .expect("Invalid GUILD_ID value"); - - let honeypot_channel_id_env = std::env::var("HONEYPOT_CHANNEL_ID") - .expect("missing HONEYPOT_CHANNEL_ID") - .parse::() - .expect("Invalid HONEYPOT_CHANNEL_ID value"); - let honeypot_channel_id = ChannelId::new(honeypot_channel_id_env); +async fn honeypot( + ctx: &serenity::Context, + new_message: &serenity::Message, + data: &Data, +) -> Result<(), Error> { + // Config is parsed once at boot and stored on AppState (COL-BOT-05). + let honeypot_channel_id = data.state.honeypot_channel_id; let current_channel_id = &new_message.channel_id; if honeypot_channel_id.eq(current_channel_id) { @@ -23,13 +15,16 @@ async fn honeypot(ctx: &serenity::Context, new_message: &serenity::Message) -> R let username = &user.name; let user_id = &user.id; let avatar_url = user.avatar_url(); - let ban_user = GuildId::new(discord_guild_id) + let ban_user = data + .state + .guild_id .ban_with_reason(ctx, user_id, 2, "Message sent in honeypot channel.") .await; if ban_user.is_ok() { log_embed( ctx, + data.state.logs_channel_id, Some("Honeypot activated!".to_string()), None, Some(format!("User got banned: {}", username)), @@ -49,15 +44,9 @@ async fn honeypot(ctx: &serenity::Context, new_message: &serenity::Message) -> R async fn create_leetcode_thread( ctx: &serenity::Context, new_message: &serenity::Message, + data: &Data, ) -> Result<(), Error> { - dotenv().ok(); - - let leetcode_channel_id_env = std::env::var("LEETCODE_CHANNEL_ID") - .expect("missing LEETCODE_CHANNEL_ID") - .parse::() - .expect("Invalid LEETCODE_CHANNEL_ID value"); - - let leetcode_channel_id = ChannelId::new(leetcode_channel_id_env); + let leetcode_channel_id = data.state.leetcode_channel_id; let current_channel_id = &new_message.channel_id; @@ -93,8 +82,9 @@ async fn create_leetcode_thread( pub async fn on_message( ctx: &serenity::Context, new_message: &serenity::Message, + data: &Data, ) -> Result<(), Error> { - honeypot(ctx, new_message).await?; - create_leetcode_thread(ctx, new_message).await?; + honeypot(ctx, new_message, data).await?; + create_leetcode_thread(ctx, new_message, data).await?; Ok(()) } diff --git a/src/main.rs b/src/main.rs index 8091baa..a9b8b96 100644 --- a/src/main.rs +++ b/src/main.rs @@ -19,6 +19,29 @@ type ApplicationContext<'a> = poise::ApplicationContext<'a, Data, Error>; pub struct AppState { pub supabase: Client, pub student_cache: Mutex>, + // Parsed once at boot. Re-reading these per event means a config typo takes + // down a handler at some random future moment instead of failing the deploy. + pub guild_id: serenity::GuildId, + pub honeypot_channel_id: serenity::ChannelId, + pub leetcode_channel_id: serenity::ChannelId, + pub verified_role_id: serenity::RoleId, + pub logs_channel_id: serenity::ChannelId, + // Optional: only /weather uses it. Read once here so no handler re-reads the + // process environment, but a missing value must not stop the bot from booting. + pub weather_token: String, +} + +/// Read a required `u64` snowflake from the environment, recording the variable +/// name in `missing` (rather than panicking on the first one) so every offending +/// variable can be reported together. +fn required_u64(name: &str, missing: &mut Vec) -> u64 { + match std::env::var(name).ok().and_then(|v| v.parse::().ok()) { + Some(v) => v, + None => { + missing.push(name.to_string()); + 0 + } + } } impl AppState { @@ -27,6 +50,26 @@ impl AppState { #[allow(clippy::result_large_err)] pub async fn new() -> supabase::Result { dotenv().ok(); + + // Parse and validate every required id up front, collecting all failures + // into one actionable startup error instead of dying on the first one. + let mut missing: Vec = Vec::new(); + let guild_id = required_u64("GUILD_ID", &mut missing); + let honeypot_channel_id = required_u64("HONEYPOT_CHANNEL_ID", &mut missing); + let leetcode_channel_id = required_u64("LEETCODE_CHANNEL_ID", &mut missing); + let verified_role_id = required_u64("VERIFIED_ROLE_ID", &mut missing); + let logs_channel_id = required_u64("LOGS_CHANNEL_ID", &mut missing); + if !missing.is_empty() { + eprintln!( + "Missing or unparseable environment variables: {}.\nCopy .env.example to .env and fill them in.", + missing.join(", ") + ); + std::process::exit(1); + } + + // Non-fatal: the bot boots without a weather key, /weather just fails. + let weather_token = std::env::var("WEATHER_TOKEN").unwrap_or_default(); + let supabase_url = std::env::var("SUPABASE_URL").expect("missing SUPABASE_URL"); let supabase_key = std::env::var("SUPABASE_KEY").expect("missing SUPABASE_KEY"); let supabase_user_email = @@ -52,6 +95,12 @@ impl AppState { Ok(Self { supabase: client, student_cache: Mutex::new(HashMap::new()), + guild_id: serenity::GuildId::new(guild_id), + honeypot_channel_id: serenity::ChannelId::new(honeypot_channel_id), + leetcode_channel_id: serenity::ChannelId::new(leetcode_channel_id), + verified_role_id: serenity::RoleId::new(verified_role_id), + logs_channel_id: serenity::ChannelId::new(logs_channel_id), + weather_token, }) } } @@ -70,7 +119,7 @@ async fn event_handler( events::interaction_create::on_interaction_create(ctx, interaction, data).await?; } serenity::FullEvent::Message { new_message } => { - events::message::on_message(ctx, new_message).await?; + events::message::on_message(ctx, new_message, data).await?; } _ => {} }