Codex review: name_matches accepted any non-empty ordered subset, so a single
common token ("John" or "Doe") matched "John Michael Doe" — a student id plus one
name token could claim the verified role for someone else. Now both the first and
last tokens must match, middle tokens the student typed must appear in the roster
in order (omitted middles still fine), and a single token, an arbitrary subset,
or a wrong first/last name is rejected. Add tests: single-token input, wrong
surname and wrong first name do NOT verify; full name and first+last with the
middle omitted do.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017XrE7F9ZuBWdQnS8CZvYDE
Verification compared names with exact lowercased equality and never trimmed, so
a trailing space, a double space, or a missing middle name turned a real paid
member away — and a pasted "s123456789 " was looked up with the trailing space
and missed. Add normalise_name (trim + collapse whitespace), normalise_student_id
(strip whitespace and a leading "s"), and name_matches (equal, or every typed
word present in the roster name in order), and route the database path, the cache
path (cached_name_matches / cache_student) and the student-id lookup through them.
"Doe John" still does not match "John Doe" and an empty name matches nothing.
Update the modal placeholder to "As it appears on your DUSA membership". Four unit
tests added. The two failure embeds are left byte-identical (SEC-19 owns those).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017XrE7F9ZuBWdQnS8CZvYDE
The insert into dsec_discord_members deserialised the response into
Vec<DiscordMemberRow>, but without `.returning(...)` supabase-lib-rs never sends
`Prefer: return=representation`, so PostgREST returns a 201 with an empty body.
Parsing that empty body failed and the `?` aborted before add_role and before
any reply — so every member's first click died with "This interaction failed",
while the row was still written (which is why the second attempt worked). Add
`.returning("student_id,discord_id")` so the row comes back and parses.
Also wrap the post-modal verification work so any failure sends the user an
ephemeral "something went wrong" instead of a dead interaction (poise's on_error
has no handle to the modal submission), and set an explicit on_error on
FrameworkOptions that replies ephemerally on command errors and keeps the
default logging. The database-write-first ordering is left unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017XrE7F9ZuBWdQnS8CZvYDE
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017XrE7F9ZuBWdQnS8CZvYDE
* SEC-01: add pull-request CI and stop the deploy leaving DOT_ENV on the VPS
A merge to main runs the merged commit as root on the club VPS through the
self-hosted runner, and this repo has no PR CI, no required review and no
status check.
- .github/workflows/ci.yml: fmt / clippy / build --locked / test on
ubuntu-latest, contents: read, both actions pinned by commit SHA. Never
the VPS.
- deploy.yml: keep push to main, add workflow_dispatch, permissions
contents: read, environment: production, and an if: always() step that
removes the .env the deploy writes into the workspace.
- .dockerignore: keep that .env, target/ and .git out of the build context.
- Dockerfile: --locked on both cargo build --release lines.
environment: production gates nothing until required reviewers are configured
on the environment itself, and the ruleset still requires zero approving
reviews. Both are owner-only and listed in the pull request.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* SEC-01: make cargo fmt and clippy clean so the CI job can go green
The CI workflow added in #5 fails on code that predates it: three files were
unformatted and `cargo clippy --all-targets -- -D warnings` reported 26 errors.
A red check cannot be made a required status check on the protect-main ruleset,
which is what this unblocks.
`cargo fmt --all` over three files, and 26 clippy errors resolved: 18 via
`cargo clippy --all-targets --fix`, the rest by hand. Two fixes uncovered lints
that had been masked (an `unnecessary_unwrap` in info.rs behind the needless
borrow on the line above, and two `unnecessary_to_owned` at the call sites of a
signature that moved from `&String` to `&str`), so 28 fixes for 26 warnings.
Three `#[allow]`s where the only real fix would change a signature or a public
API: `result_large_err` on `AppState::new` (the large variant is
`supabase::Error`, owned by supabase-lib-rs) and `too_many_arguments` on
`log_embed` and on the `embed` slash command.
No behaviour change. src/ only; .github/, Dockerfile, .dockerignore, Cargo.toml
and Cargo.lock are untouched.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* SEC-01: pin actions/checkout in the deploy workflow to a commit SHA
deploy.yml runs on the self-hosted VPS runner with passwordless sudo, so a
mutable tag on this action is a code-execution path onto that host if the tag
is ever moved. Pinned to the commit v4 currently resolves to
(11d5960a326750d5838078e36cf38b85af677262), verified against upstream — this is
the same code the deploy already runs today, not a version bump. ci.yml is on
v7.0.1; the deploy path is deliberately left on v4 so that pinning does not
smuggle a major-version change into a workflow whose only test is a live deploy.
Raised by Codex review of PR #5 as the one unpinned `uses:` in either workflow.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
