diff --git a/.dockerignore b/.dockerignore new file mode 100644 index 0000000..b220708 --- /dev/null +++ b/.dockerignore @@ -0,0 +1,9 @@ +# Keep the docker build context to what the Dockerfile actually COPYs +# (Cargo.toml, Cargo.lock, src). deploy.yml writes the DOT_ENV secret to .env in +# the workspace before running `docker compose up --build`; without this file +# that secret is tarred into the build context and handed to the docker daemon +# on every deploy. +.env +.env.* +target/ +.git diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..abea2e6 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,82 @@ +# Format, lint, build and test on every pull request. +# +# This must run on a GitHub-hosted runner and never on the club VPS. The runner +# deploy.yml uses lives on the club's own server and executes whatever a workflow +# tells it to, as root, so nothing triggered by a pull request may be pointed at +# it. Read that as a rule for this file, not a property of the repository: this +# repo is public, forking is on, and fork pull requests only need approval from a +# first-time contributor, so a fork that brings its own workflow can still reach +# that runner. See SEC-01. +# +# SEC-01: point the protect-main ruleset's required status check at this job's +# name ("fmt / clippy / build / test") so a red build blocks the merge that +# deploys. + +name: CI + +on: + pull_request: + +# A newer push to the same pull request makes an in-flight run irrelevant. +concurrency: + group: ${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +permissions: + contents: read + +jobs: + check: + name: fmt / clippy / build / test + runs-on: ubuntu-latest + # A cold build of this dependency tree (serenity, reqwest, image, exr) in + # release, then clippy, then test, is slow on a 4-vCPU hosted runner. This is + # a guard against a hung job, not a target. + timeout-minutes: 45 + + steps: + # Third-party and first-party actions alike are pinned to a full commit + # SHA: a tag is mutable and can be repointed at new code by whoever owns + # the action. + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + + # Caches written by a pull_request run are scoped to that pull request, so + # this pays off across pushes to the same branch rather than across branches. + - name: Cache cargo registry and build artifacts + uses: actions/cache@55cc8345863c7cc4c66a329aec7e433d2d1c52a9 # v6.1.0 + with: + path: | + ~/.cargo/registry + ~/.cargo/git + target + key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} + restore-keys: | + ${{ runner.os }}-cargo- + + # actions/runner-images documents rustfmt for ubuntu-24.04 but not clippy (it + # lists clippy only under ubuntu-22.04), while the image actually in use does + # ship it. Adding both components is a no-op when they are present and keeps + # this job working if that undocumented extra ever goes away. Print the + # versions too: clippy's lint set moves between releases, so knowing which one + # ran is what explains a lint that appeared from nowhere. + - name: Toolchain + run: | + rustup component add clippy rustfmt + cargo --version + cargo fmt --version + cargo clippy --version + + - name: Format + run: cargo fmt --all -- --check + + # Kept ahead of clippy and test: those resolve dependencies and would + # refresh Cargo.lock in place, so a stale lockfile would slip past + # --locked if they ran first. + - name: Build + run: cargo build --locked --release + + - name: Clippy + run: cargo clippy --all-targets -- -D warnings + + - name: Test + run: cargo test diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 7f4759b..ae6e01f 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -4,14 +4,23 @@ on: push: branches: - main + # Manual re-run, for redeploying after a rollback or re-creating the + # workspace .env that the cleanup step below now removes. + workflow_dispatch: + +permissions: + contents: read jobs: build_and_deploy: runs-on: [self-hosted, linux] + # SEC-01: this gates nothing until required reviewers are configured on the + # "production" environment itself. See the pull request description. + environment: production steps: - name: Checkout code - uses: actions/checkout@v4 + uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4 with: clean: true fetch-depth: 1 @@ -35,5 +44,10 @@ jobs: sudo docker compose up -d --build --force-recreate echo "Deployment complete." + - name: Remove environment file + if: always() + working-directory: ${{ github.workspace }} + run: rm -f .env + - name: Clean up old Docker images run: sudo docker image prune -f diff --git a/Dockerfile b/Dockerfile index 92180b1..8a5dff4 100644 --- a/Dockerfile +++ b/Dockerfile @@ -17,14 +17,14 @@ RUN mkdir src && \ echo 'fn main() { println!("Dummy main for dependency caching"); }' > src/main.rs # Build dependencies (this layer will be cached unless Cargo.toml/Cargo.lock changes) -RUN cargo build --release && \ +RUN cargo build --locked --release && \ rm -rf src target/release/deps/dsec_bot* # Copy the actual source code COPY src ./src # Build the actual application -RUN cargo build --release +RUN cargo build --locked --release # Runtime stage - use a minimal image FROM debian:bookworm-slim diff --git a/src/commands/info.rs b/src/commands/info.rs index 287c18a..bfec8f3 100644 --- a/src/commands/info.rs +++ b/src/commands/info.rs @@ -151,10 +151,10 @@ pub async fn serverinfo(ctx: Context<'_>) -> Result<(), Error> { let server_description = server_description_option.as_deref().unwrap_or("N/A"); // rules channel, if empty N/A - let rules_channel = if (&partial_guild.rules_channel_id).is_none() { - "N/A" + let rules_channel = if let Some(rules_channel_id) = partial_guild.rules_channel_id { + &format!("<#{}>", rules_channel_id) } else { - &format!("<#{}>", &partial_guild.rules_channel_id.unwrap()) + "N/A" }; let embed_footer = CreateEmbedFooter::new(format!("ID: {}", server_id)); diff --git a/src/commands/member_info.rs b/src/commands/member_info.rs index 57895c2..6d9ccc2 100644 --- a/src/commands/member_info.rs +++ b/src/commands/member_info.rs @@ -18,7 +18,7 @@ struct DiscordLinkRow { student_id: String, } -async fn member_data(database: &Database, user_id: &String) -> Result, Error> { +async fn member_data(database: &Database, user_id: &str) -> Result, Error> { let rows: Vec = database .from("active_members") .select("student_id, full_name, campus, membership_status, end_date") @@ -30,7 +30,7 @@ async fn member_data(database: &Database, user_id: &String) -> Result Result, Error> { +async fn recorded_member(database: &Database, user_id: &str) -> Result, Error> { let rows: Vec = database .from("dsec_discord_members") .select("student_id") @@ -66,15 +66,7 @@ pub async fn member_info(ctx: ApplicationContext<'_>) -> Result<(), Error> { let user_data_option = member_data(&database, &student_id).await?; - if user_data_option.is_none() { - ctx.send(CreateReply::default().embed( - CreateEmbed::new().title("Couldn't find info").description( - "Your membership may have expired. Contact a club executive to be sure.", - ), - )) - .await?; - } else { - let user_data = user_data_option.expect("Member not found"); + if let Some(user_data) = user_data_option { let member_name = user_data.full_name; let member_campus = user_data.campus; let membership_status = user_data.membership_status; @@ -97,6 +89,13 @@ pub async fn member_info(ctx: ApplicationContext<'_>) -> Result<(), Error> { .ephemeral(true), ) .await?; + } else { + ctx.send(CreateReply::default().embed( + CreateEmbed::new().title("Couldn't find info").description( + "Your membership may have expired. Contact a club executive to be sure.", + ), + )) + .await?; } // let student_data = state diff --git a/src/commands/mods_only.rs b/src/commands/mods_only.rs index 289d56e..015bf93 100644 --- a/src/commands/mods_only.rs +++ b/src/commands/mods_only.rs @@ -1,8 +1,11 @@ use crate::{Context, Error}; use dotenv::dotenv; -use poise::{serenity_prelude as serenity, CreateReply}; +use poise::{CreateReply, serenity_prelude as serenity}; /// Send message to logs channel +// Each argument is one optional embed field; collapsing them into a struct would +// change this function's signature and every call site, which this PR does not do. +#[allow(clippy::too_many_arguments)] pub async fn log_embed( ctx: &serenity::Context, title: Option, @@ -36,10 +39,10 @@ pub async fn log_embed( } // Set color (parse hex color) - if let Some(color_str) = colour { - if let Ok(color_value) = u32::from_str_radix(color_str.trim_start_matches('#'), 16) { - embed = embed.color(color_value); - } + if let Some(color_str) = colour + && let Ok(color_value) = u32::from_str_radix(color_str.trim_start_matches('#'), 16) + { + embed = embed.color(color_value); } // Set thumbnail @@ -77,6 +80,9 @@ pub async fn log_embed( slash_command, required_permissions = "MANAGE_MESSAGES | MANAGE_THREADS" )] +// These arguments are the slash command's options as Discord presents them; +// bundling them into a struct would change the command's public interface. +#[allow(clippy::too_many_arguments)] pub async fn embed( ctx: Context<'_>, #[description = "Title of embed"] title: Option, @@ -109,10 +115,10 @@ pub async fn embed( } // Set color (parse hex color) - if let Some(color_str) = colour { - if let Ok(color_value) = u32::from_str_radix(color_str.trim_start_matches('#'), 16) { - embed = embed.color(color_value); - } + if let Some(color_str) = colour + && let Ok(color_value) = u32::from_str_radix(color_str.trim_start_matches('#'), 16) + { + embed = embed.color(color_value); } // Set thumbnail diff --git a/src/commands/weather.rs b/src/commands/weather.rs index 43622bf..8b7dd02 100644 --- a/src/commands/weather.rs +++ b/src/commands/weather.rs @@ -26,24 +26,24 @@ pub async fn weather( let weather_response = get_weather(location).await?; let value: Value = serde_json::from_str(&weather_response)?; - let location_name = (&value["location"]["name"]).as_str().unwrap(); - let location_region = (&value["location"]["region"]).as_str().unwrap(); - let location_country = (&value["location"]["country"]).as_str().unwrap(); + let location_name = value["location"]["name"].as_str().unwrap(); + let location_region = value["location"]["region"].as_str().unwrap(); + let location_country = value["location"]["country"].as_str().unwrap(); - let weather_condition = (&value["current"]["condition"]["text"]).as_str().unwrap(); + let weather_condition = value["current"]["condition"]["text"].as_str().unwrap(); let weather_temp = &value["current"]["temp_c"]; let weather_feels_like = &value["current"]["feelslike_c"]; let weather_wind_kph = &value["current"]["wind_kph"]; let weather_humidity = &value["current"]["humidity"]; let weather_cloud = &value["current"]["cloud"]; - let weather_icon = (&value["current"]["condition"]["icon"]).as_str().unwrap(); + let weather_icon = value["current"]["condition"]["icon"].as_str().unwrap(); let embed = CreateEmbed::new() - .field("Name", format!("{}", location_name), true) - .field("Region", format!("{}", location_region), true) - .field("Country", format!("{}", location_country), true) - .field("Condition", format!("{}", weather_condition), true) + .field("Name", location_name.to_string(), true) + .field("Region", location_region.to_string(), true) + .field("Country", location_country.to_string(), true) + .field("Condition", weather_condition.to_string(), true) .field("Temperature", format!("{} °C", weather_temp), true) .field("Feels like", format!("{} °C", weather_feels_like), true) .field("Wind", format!("{} kph", weather_wind_kph), true) diff --git a/src/events.rs b/src/events.rs index 49002e6..720a303 100644 --- a/src/events.rs +++ b/src/events.rs @@ -1,3 +1,3 @@ -pub mod ready; pub mod interaction_create; -pub mod message; \ No newline at end of file +pub mod message; +pub mod ready; diff --git a/src/events/interaction_create.rs b/src/events/interaction_create.rs index 20f7ca9..1c04f06 100644 --- a/src/events/interaction_create.rs +++ b/src/events/interaction_create.rs @@ -115,7 +115,7 @@ async fn grant_verified_role( data: &Data, modal_submit: &ModalInteraction, discord_member: &Member, - student_id: &String, + student_id: &str, verified_role_id: RoleId, via_cache: bool, ) -> Result<(), Error> { @@ -170,7 +170,7 @@ async fn fetch_student(data: &Data, student_id: &str) -> Result Result { +async fn member_recorded(data: &Data, user_id: &str) -> Result { let rows: Vec = data .state .supabase @@ -210,18 +210,18 @@ async fn handle_verify( // already attached to the button interaction. Anything slower than this // (a DB query, a member fetch) must NOT run before the modal is shown, or // Discord's ~3s acknowledgement window elapses and the click fails. - if let Some(member) = &component_interaction.member { - if member.roles.contains(&verified_role_id) { - component_interaction - .create_response( - ctx, - ephemeral_embed(CreateEmbed::new().title("Already Verified ✅").description( - format!("You already have the <@&{}> role!", verified_role_id), - )), - ) - .await?; - return Ok(()); - } + if let Some(member) = &component_interaction.member + && member.roles.contains(&verified_role_id) + { + component_interaction + .create_response( + ctx, + ephemeral_embed(CreateEmbed::new().title("Already Verified ✅").description( + format!("You already have the <@&{}> role!", verified_role_id), + )), + ) + .await?; + return Ok(()); } // Respond to the click with the modal immediately. @@ -247,7 +247,7 @@ async fn handle_verify( data, &modal_submit, &discord_member, - &student_id.to_string(), + student_id, verified_role_id, true, ) @@ -277,7 +277,7 @@ async fn handle_verify( data, &modal_submit, &discord_member, - &student_id.to_string(), + student_id, verified_role_id, false, ) diff --git a/src/events/message.rs b/src/events/message.rs index 47a68a9..2f52edb 100644 --- a/src/events/message.rs +++ b/src/events/message.rs @@ -65,7 +65,7 @@ async fn create_leetcode_thread( fn create_title_from_message(message: impl Into) -> String { let message_string: String = message.into(); - let title = message_string + message_string .lines() .next() .map(|line| { @@ -77,8 +77,7 @@ async fn create_leetcode_thread( &line[start..] }) .unwrap_or("") - .to_string(); - title + .to_string() } let new_thread = @@ -95,7 +94,7 @@ pub async fn on_message( ctx: &serenity::Context, new_message: &serenity::Message, ) -> Result<(), Error> { - let _ = honeypot(ctx, new_message).await?; - let _ = create_leetcode_thread(ctx, new_message).await?; + honeypot(ctx, new_message).await?; + create_leetcode_thread(ctx, new_message).await?; Ok(()) } diff --git a/src/main.rs b/src/main.rs index a5b7e67..8091baa 100644 --- a/src/main.rs +++ b/src/main.rs @@ -22,6 +22,9 @@ pub struct AppState { } impl AppState { + // The large `Err` variant is `supabase::Error` from the `supabase-lib-rs` crate; + // boxing it would change this public signature rather than shrink their type. + #[allow(clippy::result_large_err)] pub async fn new() -> supabase::Result { dotenv().ok(); let supabase_url = std::env::var("SUPABASE_URL").expect("missing SUPABASE_URL"); @@ -42,9 +45,7 @@ impl AppState { None => println!("User not found"), }, Err(err) => { - eprintln!( - "Failed to connect/sign in to Supabase, continuing setup anyways: {err}" - ); + eprintln!("Failed to connect/sign in to Supabase, continuing setup anyways: {err}"); } } @@ -66,7 +67,7 @@ async fn event_handler( events::ready::on_ready(ctx, data_about_bot).await?; } serenity::FullEvent::InteractionCreate { interaction } => { - events::interaction_create::on_interaction_create(ctx, interaction, &data).await?; + events::interaction_create::on_interaction_create(ctx, interaction, data).await?; } serenity::FullEvent::Message { new_message } => { events::message::on_message(ctx, new_message).await?;