diff --git a/.env.example b/.env.example index f031836..d459423 100644 --- a/.env.example +++ b/.env.example @@ -27,8 +27,11 @@ LOG_MODE=full # Slack user groups that declare roles (Business+ only; user groups do not # exist on the free plan). Leave unset to maintain the roster purely by CSV. # Restrict "Create and edit user groups" to Owners/Admins before using these. +# Seeds only. Once hawk-mod is running these four are set from Slack with +# `/hawkmod config`, which stores them in the database and shows you where each +# value came from. A value set there wins over anything here. STUDENT_USERGROUP=students -ADULT_USERGROUP=adults +ADULT_USERGROUP=mentors # Schedules (cron, in TZ). Defaults shown. TZ=America/New_York diff --git a/CLAUDE.md b/CLAUDE.md index fa8b0c6..9217bc7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -80,6 +80,22 @@ that has shipped. All SQL lives in `src/db/repo.ts` and nowhere else, and better-sqlite3 is synchronous — repo functions are not `async`, so anything `await`ed in this codebase is Slack, not the database. +**Settings a Slack admin owns live in the database, not the environment.** +`src/settings.ts` resolves each one **database → environment → unset**, so the +env var is a _seed_ rather than the source of truth and an existing host keeps +working with no flag day. `/hawkmod config` shows every value **and where it +came from**, which is the question actually asked when the roster looks wrong. +`SETTINGS` is an allowlist and must stay one: Slack credentials cannot be set +from Slack, and `TOKEN_ENCRYPTION_KEY` must never be reachable — changing it +makes every stored token undecryptable and every enrolled adult invisible while +coverage still reads 100%. A user group handle is validated against Slack before +it is stored, because a stored typo reads exactly like an empty group. Changing +a role group re-syncs immediately; leaving it until 3am would mean the setting +looked applied and was not. Every change lands in `setting_changes`. + +Note `settings.ts` reads `process.env` directly rather than through `config()` — +it is reachable from the CLI, and `config()` there would throw at import time. + **`config()` is all or nothing.** One zod parse of the whole environment, on first use; a missing `SLACK_*` var throws for every caller. That is why `dataDir()` and `logMode()` exist as separate readers — the CLI runs @@ -104,7 +120,7 @@ is idempotent, which is what makes the duplicate events harmless). The reconciliation in `domain/rules/rosterSync.ts` is pure and **may only ever add monitoring, never subtract it**: a person dropped from the students group stays a student, since the alternative is silently ending someone's monitoring. Only -an explicit move into the adults group leaves `student`, and that raises +an explicit move into the mentors group leaves `student`, and that raises `roster_drift`. Every change lands in `role_changes` — Slack's audit log API is Grid-only, so that table is the only trail. Do not make this bidirectional. diff --git a/CONTEXT.md b/CONTEXT.md index e23b703..e1bb3e1 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -38,7 +38,7 @@ _Avoid_: Lead coach, admin role, superuser These two are the distinction most easily lost, and the one that matters most. **Declared**: -Present in the Slack user group that names a role — `@students` or `@adults`. +Present in the Slack user group that names a role — `@students` or `@mentors`. Cheap, reversible, and edited by hand in Slack or through hawk-mod. A declaration is a statement of intent, not a fact about monitoring. _Avoid_: Enrolled, rostered, assigned diff --git a/README.md b/README.md index fed266b..7571ffc 100644 --- a/README.md +++ b/README.md @@ -192,8 +192,9 @@ npm run cli -- import-consents consents.csv ### Roles from Slack user groups -Set `STUDENT_USERGROUP` and `ADULT_USERGROUP` to user group handles (e.g. -`students`, `adults`) and each sweep reconciles roles from them, so membership +Set the student and mentor groups with `/hawkmod config` — this team uses +`students` and `mentors` — and each sweep reconciles roles from them, so +membership is managed in Slack rather than by editing a CSV. Group membership is by Slack user ID, which removes the email-matching failure below entirely: a group member with no roster row gets one created from their Slack profile instead of @@ -203,8 +204,11 @@ Two properties make this safe to rely on: - **Membership is only ever added, never subtracted.** Dropping someone from the students group does _not_ un-student them — that would silently end their - monitoring. The only way out of `student` is being put in the adults group, - which is deliberate and raises a `roster_drift` finding. + monitoring. The only way out of `student` is being put in the mentors + group, which is deliberate and raises a `roster_drift` finding. +- **The handle is checked before it is stored.** A user group that does not + resolve is refused outright, because a stored typo reads exactly like an + empty group: nobody rostered, nobody monitored, no complaint. - **Every role change is recorded** in `role_changes` with who, when, and from what. Slack's audit log API is Enterprise Grid only, so on Business+ this table is the sole durable trail of who was monitored when. diff --git a/migrations/0007_settings.sql b/migrations/0007_settings.sql new file mode 100644 index 0000000..9f12ffb --- /dev/null +++ b/migrations/0007_settings.sql @@ -0,0 +1,35 @@ +-- Settings a Slack admin can change without a shell on the host. +-- +-- `authz.ts` already made this argument once, about who may administer +-- hawk-mod: "an app whose first-run instruction is 'SSH into the server' is +-- broken." The same was still true of which user group declares students — a +-- decision a Slack admin makes, that only a Linode login could change, and that +-- reads identically to "the group is empty" when it is wrong. +-- +-- Credentials stay in the environment: you cannot configure from Slack the +-- things that let hawk-mod reach Slack, and TOKEN_ENCRYPTION_KEY changing would +-- make every stored token undecryptable. What moves here is policy, not +-- identity. +-- +-- Lives in the same SQLite file as everything else, so it survives a redeploy +-- and is already covered by the backup step in docs/deploy.md. +CREATE TABLE settings ( + key TEXT PRIMARY KEY, + value TEXT NOT NULL, + updated_by TEXT NOT NULL, + updated_at TEXT NOT NULL +); + +-- Which user group declares who is monitored is a youth-protection fact, so +-- changing it leaves the same kind of trail as changing somebody's role does. +-- Slack's audit log API is Enterprise Grid only; this is the trail. +CREATE TABLE setting_changes ( + id INTEGER PRIMARY KEY, + key TEXT NOT NULL, + from_value TEXT, -- NULL when the setting was previously unset + to_value TEXT NOT NULL, + actor TEXT NOT NULL, + actor_name TEXT NOT NULL, + changed_at TEXT NOT NULL +); +CREATE INDEX setting_changes_key_idx ON setting_changes (key, changed_at); diff --git a/scripts/setup-local.sh b/scripts/setup-local.sh index ea2e3a7..99ebca1 100755 --- a/scripts/setup-local.sh +++ b/scripts/setup-local.sh @@ -554,11 +554,11 @@ stage "Slack — user groups that declare roles" say "hawk-mod reads two user groups to decide who is a student and who is an" say "adult. Give it the TEST groups." step "In Slack: create the groups if they don't exist, and put ONLY accounts" -note " you control in them — you in the adults one, your second account" +note " you control in them — you in the mentors one, your second account" note " in the students one." say "" ask STUDENT_USERGROUP "Students group handle (without @):" -ask ADULT_USERGROUP "Adults group handle (without @):" +ask ADULT_USERGROUP "Mentors group handle (without @):" write_env STUDENT_USERGROUP "$STUDENT_USERGROUP" write_env ADULT_USERGROUP "$ADULT_USERGROUP" note "Membership is only ever added to the roster, never subtracted — dropping" diff --git a/src/config.ts b/src/config.ts index 3526f18..45bdd2e 100644 --- a/src/config.ts +++ b/src/config.ts @@ -7,23 +7,20 @@ const schema = z.object({ SLACK_STATE_SECRET: z.string().min(16), PUBLIC_URL: z.string().url(), PORT: z.coerce.number().int().positive().default(3000), - ALERT_CHANNEL_ID: z.string().min(1), + // Seeds only, all four of them. A value set from Slack wins — see + // src/settings.ts. Kept optional so a fresh install can be configured + // entirely from Slack, and so an existing host keeps working unchanged. + ALERT_CHANNEL_ID: z.string().optional(), DATA_DIR: z.string().default("./data"), TOKEN_ENCRYPTION_KEY: z.string().min(1), LOG_MODE: z.enum(["full", "metadata"]).default("full"), - // Slack user groups that declare roles. Both optional: leave them unset and - // the roster is maintained purely by CSV import. + // Slack user groups that declare roles. Seeds for the settings of the same + // name; leave both unset and unconfigured and the roster is maintained purely + // by CSV import. STUDENT_USERGROUP: z.string().optional(), ADULT_USERGROUP: z.string().optional(), - // Further user group handles hawk-mod may edit, comma separated. The two - // role groups above are always editable; this widens the allowlist to - // subteams (@programming, @drive-team) without widening it to everything. - // - // The allowlist is not authorization — every caller is already a Workspace - // Owner or Admin who could edit any group in Slack's own UI. It is blast - // radius. `usergroups.users.update` replaces a group's entire membership, so - // a bad plan does not corrupt a group, it empties one, and this bounds how - // many groups a single bug can reach. + // Seed for the `managed-groups` setting; see src/settings.ts for what the + // allowlist is actually for. MANAGED_USERGROUPS: z.string().optional(), TZ: z.string().default("America/New_York"), SWEEP_CRON: z.string().default("0 3 * * *"), @@ -62,17 +59,3 @@ export function dataDir(): string { export function logMode(): "full" | "metadata" { return process.env.LOG_MODE === "metadata" ? "metadata" : "full"; } - -/** Handles hawk-mod is permitted to edit, lowercased and without the `@`. */ -export function managedGroupHandles(): Set { - const cfg = config(); - const extra = (cfg.MANAGED_USERGROUPS ?? "") - .split(",") - .map((h) => h.trim()) - .filter(Boolean); - return new Set( - [cfg.STUDENT_USERGROUP, cfg.ADULT_USERGROUP, ...extra] - .filter((h): h is string => Boolean(h)) - .map((h) => h.replace(/^@/, "").toLowerCase()) - ); -} diff --git a/src/db/repo.ts b/src/db/repo.ts index 519cc06..f62fa37 100644 --- a/src/db/repo.ts +++ b/src/db/repo.ts @@ -373,6 +373,82 @@ export function revokeConsent(consentId: number, on: string): void { .run(on, consentId); } +/* -------------------------------------------------------------- settings */ + +/** A setting a Slack admin has changed, or undefined if none has. */ +export function getSetting(key: string): string | undefined { + const row = db() + .prepare("SELECT value FROM settings WHERE key = ?") + .get(key) as { value: string } | undefined; + return row?.value; +} + +export function listSettingRows(): Array<{ + key: string; + value: string; + updated_by: string; + updated_at: string; +}> { + return db().prepare("SELECT * FROM settings ORDER BY key").all() as Array<{ + key: string; + value: string; + updated_by: string; + updated_at: string; + }>; +} + +/** + * Changes a setting and records who changed it, in one transaction. + * + * Which user group declares who is monitored is a youth-protection fact, so it + * leaves the same kind of trail as changing somebody's role does. Slack's audit + * log API is Enterprise Grid only; `setting_changes` is the trail. + */ +export function setSetting(args: { + key: string; + value: string; + actor: string; + actorName: string; +}): void { + const now = nowIso(); + const previous = getSetting(args.key) ?? null; + db().transaction(() => { + db() + .prepare( + `INSERT INTO settings (key, value, updated_by, updated_at) + VALUES (?, ?, ?, ?) + ON CONFLICT (key) DO UPDATE SET + value = excluded.value, + updated_by = excluded.updated_by, + updated_at = excluded.updated_at` + ) + .run(args.key, args.value, args.actorName, now); + db() + .prepare( + `INSERT INTO setting_changes (key, from_value, to_value, actor, + actor_name, changed_at) + VALUES (?, ?, ?, ?, ?, ?)` + ) + .run(args.key, previous, args.value, args.actor, args.actorName, now); + })(); +} + +export function listSettingChanges(limit = 100) { + return db() + .prepare( + `SELECT * FROM setting_changes ORDER BY changed_at DESC, id DESC LIMIT ?` + ) + .all(limit) as Array<{ + id: number; + key: string; + from_value: string | null; + to_value: string; + actor: string; + actor_name: string; + changed_at: string; + }>; +} + /* --------------------------------------------------------- group changes */ export type GroupChangeInput = { diff --git a/src/domain/rules/rosterSync.ts b/src/domain/rules/rosterSync.ts index c31e31a..8ad2fd1 100644 --- a/src/domain/rules/rosterSync.ts +++ b/src/domain/rules/rosterSync.ts @@ -3,7 +3,7 @@ import { type Person, type Role } from "../people.js"; export type GroupMembership = { /** Slack ids in the group designating students. */ students: ReadonlySet; - /** Slack ids in the group designating adults/adults. */ + /** Slack ids in the group designating adults — @mentors, in this team. */ adults: ReadonlySet; }; diff --git a/src/index.ts b/src/index.ts index 5e79a44..6ebe4b5 100644 --- a/src/index.ts +++ b/src/index.ts @@ -2,6 +2,7 @@ import { config } from "./config.js"; import { db } from "./db/client.js"; import { startSchedules } from "./jobs/schedule.js"; import { log } from "./logger.js"; +import { setting } from "./settings.js"; import { createApp } from "./slack/app.js"; async function main() { @@ -10,16 +11,33 @@ async function main() { const app = createApp(); await app.start(cfg.PORT); startSchedules(); + const students = setting("student-group"); + const mentors = setting("mentor-group"); + const alerts = setting("alert-channel"); + log.info("hawk-mod started", { port: cfg.PORT, logMode: cfg.LOG_MODE, installUrl: `${cfg.PUBLIC_URL}/slack/install`, - // Silence here used to look identical to "the groups are empty". + // Silence here used to look identical to "the groups are empty". The + // source matters as much as the value: "which group?" and "who set it?" + // are the two questions asked when the roster looks wrong. roleSource: - cfg.STUDENT_USERGROUP || cfg.ADULT_USERGROUP - ? `user groups (@${cfg.STUDENT_USERGROUP ?? "-"} / @${cfg.ADULT_USERGROUP ?? "-"})` + students.value || mentors.value + ? `user groups (@${students.value ?? "-"} [${students.source}] / ` + + `@${mentors.value ?? "-"} [${mentors.source}])` : "CSV import only — no user groups configured", + alertChannel: alerts.value ? `${alerts.value} [${alerts.source}]` : "unset", }); + + // A finding nobody is told about is the failure this project is built to + // avoid, so an unset alert channel is an error at boot rather than a surprise + // the first time something goes wrong. + if (!alerts.value) { + log.error("no alert channel configured; findings will not be announced", { + fix: "/hawkmod config set alert-channel #channel", + }); + } } main().catch((err) => { diff --git a/src/jobs/syncRoles.ts b/src/jobs/syncRoles.ts index bf198f5..524913d 100644 --- a/src/jobs/syncRoles.ts +++ b/src/jobs/syncRoles.ts @@ -1,5 +1,4 @@ import type { WebClient } from "@slack/web-api"; -import { config } from "../config.js"; import { createPersonFromSlack, peopleBySlackId, @@ -10,6 +9,7 @@ import { import { dedupeKey } from "../domain/findings.js"; import { reconcileRoles } from "../domain/rules/rosterSync.js"; import { log } from "../logger.js"; +import { settingValue } from "../settings.js"; import { raise } from "../raise.js"; import { fetchProfiles, resolveGroup } from "../slack/userGroups.js"; @@ -49,7 +49,9 @@ const SOURCE = "usergroup_sync"; export async function syncRolesFromUserGroups( client: WebClient ): Promise { - const cfg = config(); + // From Slack if an admin has set it, from the environment otherwise. + const studentHandle = settingValue("student-group"); + const adultHandle = settingValue("mentor-group"); const stats: RoleSyncStats = { enabled: false, studentsInGroup: 0, @@ -62,22 +64,22 @@ export async function syncRolesFromUserGroups( missingGroups: 0, }; - if (!cfg.STUDENT_USERGROUP && !cfg.ADULT_USERGROUP) return stats; + if (!studentHandle && !adultHandle) return stats; stats.enabled = true; - const studentGroup = cfg.STUDENT_USERGROUP - ? await resolveGroup(client, cfg.STUDENT_USERGROUP) + const studentGroup = studentHandle + ? await resolveGroup(client, studentHandle) : null; - const adultGroup = cfg.ADULT_USERGROUP - ? await resolveGroup(client, cfg.ADULT_USERGROUP) + const adultGroup = adultHandle + ? await resolveGroup(client, adultHandle) : null; // A configured group that does not exist is a typo, and it reads exactly // like an empty group: nobody rostered, nothing monitored, no complaint. // Being quieter by being blinder is the failure this project must not have. for (const [handle, group] of [ - [cfg.STUDENT_USERGROUP, studentGroup], - [cfg.ADULT_USERGROUP, adultGroup], + [studentHandle, studentGroup], + [adultHandle, adultGroup], ] as const) { if (!handle || group) continue; stats.missingGroups += 1; @@ -88,7 +90,7 @@ export async function syncRolesFromUserGroups( summary: `Configured user group @${handle} does not exist in this workspace. ` + `Nobody is being rostered from it, so nobody is being monitored ` + - `through it either.`, + `through it either. Fix it with \`/hawkmod config\`.`, subjectRef: handle, }); } diff --git a/src/monitor/remediation.ts b/src/monitor/remediation.ts index f3f69cf..1692710 100644 --- a/src/monitor/remediation.ts +++ b/src/monitor/remediation.ts @@ -14,6 +14,7 @@ import { } from "../domain/rules/remediation.js"; import type { DmVerdict } from "../domain/rules/dmPolicy.js"; import { log } from "../logger.js"; +import { settingValue } from "../settings.js"; import { botClient } from "../slack/tokens.js"; type FindingDetail = { @@ -81,8 +82,10 @@ export async function remediateOneOnOnes( // to the alarm, rather than leaving a violation that looks unanswered. if (finding.alert_ts) { try { + const channel = settingValue("alert-channel"); + if (!channel) throw new Error("no alert channel configured"); await botClient().chat.postMessage({ - channel: config().ALERT_CHANNEL_ID, + channel, thread_ts: finding.alert_ts, text: `:white_check_mark: Put right — ${note} Acknowledged automatically; the 1:1 messages remain on record.`, }); diff --git a/src/settings.ts b/src/settings.ts new file mode 100644 index 0000000..49e72f4 --- /dev/null +++ b/src/settings.ts @@ -0,0 +1,121 @@ +import { getSetting } from "./db/repo.js"; + +/** + * Settings a Slack admin can change from Slack. + * + * Deliberately reads `process.env` directly rather than going through + * `config()`. `config()` is an all-or-nothing parse of the whole Slack + * environment, and this module is reachable from the CLI, which runs + * `import-roster` and `findings` with no Slack credentials present. Touching + * `config()` from here would break those commands at import time. + * + * The environment is no longer the source of truth for these — it is the seed. + * A value set in Slack wins; the env var is what a fresh install starts from, + * so nothing breaks for a host that already has one and there is no flag day. + */ + +export type SettingKind = "usergroup" | "usergroup_list" | "channel"; + +export type SettingSpec = { + /** The environment variable this used to live in, and still falls back to. */ + env: string; + label: string; + kind: SettingKind; + /** Shown by `/hawkmod config` when nothing is set anywhere. */ + hint: string; +}; + +/** + * The allowlist, and it is an allowlist on purpose. Slack credentials cannot be + * here — you cannot configure from Slack the thing that lets hawk-mod reach + * Slack — and `TOKEN_ENCRYPTION_KEY` must never be, because changing it makes + * every stored token undecryptable and every enrolled adult silently invisible. + */ +export const SETTINGS = { + "student-group": { + env: "STUDENT_USERGROUP", + label: "Student user group", + kind: "usergroup", + hint: "the group whose members are monitored as students", + }, + "mentor-group": { + env: "ADULT_USERGROUP", + label: "Mentor user group", + kind: "usergroup", + hint: "the group whose members are rostered as adults", + }, + "managed-groups": { + env: "MANAGED_USERGROUPS", + label: "Other editable groups", + kind: "usergroup_list", + hint: "comma separated; the two role groups are always editable", + }, + "alert-channel": { + env: "ALERT_CHANNEL_ID", + label: "Alert channel", + kind: "channel", + hint: "where findings are posted", + }, +} as const satisfies Record; + +export type SettingKey = keyof typeof SETTINGS; + +export const SETTING_KEYS = Object.keys(SETTINGS) as SettingKey[]; + +export function isSettingKey(key: string): key is SettingKey { + return Object.hasOwn(SETTINGS, key); +} + +export type Resolved = { + value: string | undefined; + /** Where the value came from — the thing that makes a wrong one debuggable. */ + source: "slack" | "env" | "unset"; +}; + +/** + * Pure so it can be tested without a database: the precedence rule is the whole + * point of this module and is worth pinning down on its own. + */ +export function resolveSetting( + fromDb: string | undefined, + fromEnv: string | undefined +): Resolved { + const db = fromDb?.trim(); + if (db) return { value: db, source: "slack" }; + const env = fromEnv?.trim(); + if (env) return { value: env, source: "env" }; + return { value: undefined, source: "unset" }; +} + +/** Splits a comma-separated setting into handles, without `@` and lowercased. */ +export function parseHandles(raw: string | undefined): string[] { + return (raw ?? "") + .split(",") + .map((h) => h.trim().replace(/^@/, "").toLowerCase()) + .filter(Boolean); +} + +export function setting(key: SettingKey): Resolved { + return resolveSetting(getSetting(key), process.env[SETTINGS[key].env]); +} + +/** The value alone, for the many callers that do not care where it came from. */ +export function settingValue(key: SettingKey): string | undefined { + return setting(key).value; +} + +/** + * Handles `/hawkmod group` is permitted to edit. + * + * The two role groups are always editable — they are the ones hawk-mod is for. + * Everything else has to be named, because `usergroups.users.update` replaces a + * group's whole membership, so a bad plan does not corrupt a group, it empties + * one. This bounds how many groups a single bug can reach. + */ +export function managedGroupHandles(): Set { + return new Set([ + ...parseHandles(settingValue("student-group")), + ...parseHandles(settingValue("mentor-group")), + ...parseHandles(settingValue("managed-groups")), + ]); +} diff --git a/src/slack/alerts.ts b/src/slack/alerts.ts index 9dd52ba..e3eae55 100644 --- a/src/slack/alerts.ts +++ b/src/slack/alerts.ts @@ -1,10 +1,31 @@ -import { config } from "../config.js"; import { getFinding, setFindingAlertTs } from "../db/repo.js"; import type { Finding } from "../domain/findings.js"; import { severityEmoji } from "../domain/findings.js"; import { log } from "../logger.js"; +import { settingValue } from "../settings.js"; import { botClient } from "./tokens.js"; +/** + * Where findings go, or `null` if nobody has said. + * + * Unset is not a quiet state. A finding with nowhere to be announced is + * recorded and invisible, which is the exact failure this project defines + * itself against — so every attempt to use it logs at error level, naming the + * finding that is going unreported and the command that fixes it. `/hawkmod + * config` shows it too, and so does startup. + */ +function alertChannel(context: string): string | null { + const channel = settingValue("alert-channel"); + if (!channel) { + log.error("no alert channel configured; nobody is being told", { + context, + fix: "/hawkmod config set alert-channel #channel", + }); + return null; + } + return channel; +} + export const ACK_ACTION = "hawkmod_finding_ack"; export const RESOLVE_ACTION = "hawkmod_finding_resolve"; @@ -84,8 +105,10 @@ export function findingBlocks(f: Finding): { */ async function supersede(previousTs: string, finding: Finding): Promise { try { + const channel = alertChannel("supersede"); + if (!channel) return; await botClient().chat.update({ - channel: config().ALERT_CHANNEL_ID, + channel, ts: previousTs, text: `${severityEmoji(finding.severity)} ${finding.kind} — happened again`, blocks: [ @@ -123,8 +146,10 @@ export async function postFinding(findingId: number): Promise { const previousTs = finding.alert_ts; const { text, blocks } = findingBlocks(finding); try { + const channel = alertChannel(`finding ${findingId}`); + if (!channel) return; const res = await botClient().chat.postMessage({ - channel: config().ALERT_CHANNEL_ID, + channel, text, blocks: blocks as never, }); @@ -146,8 +171,10 @@ export async function refreshFinding(findingId: number): Promise { if (!finding?.alert_ts) return; const { text, blocks } = findingBlocks(finding); try { + const channel = alertChannel(`finding ${finding.id}`); + if (!channel) return; await botClient().chat.update({ - channel: config().ALERT_CHANNEL_ID, + channel, ts: finding.alert_ts, text, blocks: blocks as never, @@ -162,8 +189,10 @@ export async function refreshFinding(findingId: number): Promise { export async function postToAlertChannel(text: string): Promise { try { + const channel = alertChannel("digest"); + if (!channel) return; await botClient().chat.postMessage({ - channel: config().ALERT_CHANNEL_ID, + channel, text, }); } catch (err) { diff --git a/src/slack/app.ts b/src/slack/app.ts index ae743a3..3e732e2 100644 --- a/src/slack/app.ts +++ b/src/slack/app.ts @@ -23,7 +23,7 @@ export const BOT_SCOPES = [ "team:read", "users:read", "users:read.email", - // Reads the @students / @adults groups that declare roles. + // Reads the @students / @mentors groups that declare roles. "usergroups:read", ]; diff --git a/src/slack/commands.ts b/src/slack/commands.ts index d301a21..26a6974 100644 --- a/src/slack/commands.ts +++ b/src/slack/commands.ts @@ -5,6 +5,7 @@ import { closeFinding } from "../close.js"; import { config } from "../config.js"; import { countOpenByKind, + setSetting, getFinding, getInstallation, listConsents, @@ -21,9 +22,18 @@ import { requiresEnrollment, type Person } from "../domain/people.js"; import { consentStatus } from "../domain/rules/consent.js"; import { screeningStatus } from "../domain/rules/screening.js"; import { log } from "../logger.js"; +import { + isSettingKey, + SETTING_KEYS, + SETTINGS, + setting, + settingValue, + type SettingKey, +} from "../settings.js"; import { backfillAll } from "../monitor/backfill.js"; import { administrator, type Actor, NOT_PERMITTED } from "./authz.js"; import { applyGroupEdit } from "./groupAdmin.js"; +import { resolveGroup } from "./userGroups.js"; import { openConsent, openScreening } from "./modals.js"; import { runSweep } from "../jobs/sweep.js"; import { syncRolesFromUserGroups } from "../jobs/syncRoles.js"; @@ -37,6 +47,7 @@ const HELP = [ "`/hawkmod group add @user @group` — put someone in a user group", "`/hawkmod group remove @user @group` — take someone out of a user group", "`/hawkmod deactivate @user ` — stop monitoring someone", + "`/hawkmod config` — show settings; `config set ` to change one", "`/hawkmod screening @user` — record YPP / Mentor Ready / CORI dates", "`/hawkmod consent @user` — record a signed parental consent", "`/hawkmod ack ` — acknowledge without closing", @@ -46,7 +57,7 @@ const HELP = [ "`/hawkmod backfill` — walk enrolled adults' DM history now", "", "_Roles come from Slack user groups. To add someone to the roster, add them", - "to the students or adults group — it applies straight away._", + "to the @students or @mentors group — it applies straight away._", ].join("\n"); export function registerCommands(app: App): void { @@ -100,8 +111,8 @@ export function registerCommands(app: App): void { text: `Couldn't find \`${rest.join(" ") || "(nobody)"}\` on the roster.\n` + `Usage: \`/hawkmod ${sub} @user\`. Roster membership comes from ` + - `the user groups, so add them to @${config().STUDENT_USERGROUP ?? "students"} ` + - `or @${config().ADULT_USERGROUP ?? "adults"} first.`, + `the user groups, so add them to @${settingValue("student-group") ?? "students"} ` + + `or @${settingValue("mentor-group") ?? "mentors"} first.`, }); return; } @@ -151,6 +162,14 @@ export function registerCommands(app: App): void { return; } + case "config": { + await respond({ + response_type: "ephemeral", + text: await configText(client, caller, rest), + }); + return; + } + case "group": { await respond({ response_type: "ephemeral", @@ -294,6 +313,43 @@ async function resolvePerson( return undefined; } +/** + * Resolves a Slack account, roster row or not. + * + * `resolvePerson` answers "who is this on the roster", which is the right + * question almost everywhere and the wrong one for group edits: joining a role + * user group is *how* somebody gets a roster row, so demanding one first is a + * deadlock. This answers the smaller question — which Slack account did they + * mean — so the edit can proceed and the sync can create the row from it. + */ +async function resolveSlackId( + client: WebClient, + mention: string +): Promise { + const raw = mention.trim(); + if (!raw) return undefined; + + const escaped = raw.match(/^<@([A-Z0-9]+)/i)?.[1]; + if (escaped) return escaped.toUpperCase(); + if (/^U[A-Z0-9]{4,}$/i.test(raw)) return raw.toUpperCase(); + + const wanted = raw.replace(/^@/, "").toLowerCase(); + try { + const list = await client.users.list({ limit: 500 }); + const match = (list.members ?? []).find( + (m) => + !m.deleted && + !m.is_bot && + [m.name, m.profile?.display_name, m.profile?.real_name] + .filter(Boolean) + .some((n) => (n as string).toLowerCase() === wanted) + ); + return match?.id; + } catch { + return undefined; + } +} + async function whoisText( client: WebClient, teamId: string, @@ -358,7 +414,7 @@ function groupRef(raw: string): string | null { /** * `/hawkmod group add|remove @user @group`. * - * Moving a student into the adults group requires a written reason. Refusing it + * Moving a student into the mentors group requires a written reason. Refusing it * outright would be worse than allowing it: the action would simply happen in * Slack's own UI instead, where hawk-mod learns of it from an event carrying no * reason and no author. Requiring a sentence keeps the most consequential edit @@ -380,14 +436,21 @@ async function groupText( return `Usage: \`/hawkmod group ${action} @user @group\`.`; } + // A roster row is not a precondition here, and requiring one was a deadlock: + // the sync only creates rows for people already in a role group, so the + // command meant to put somebody in their first group refused everybody who + // needed it. Slack's account is enough — `subteam_members_changed` fires on + // the write and the sync creates the roster row moments later. const person = await resolvePerson(client, mention); - if (!person) { + const slackId = + person?.slack_user_id ?? (await resolveSlackId(client, mention)); + if (!slackId) { return ( - `Couldn't find \`${mention}\` on the roster. hawk-mod only edits groups ` + - `for people it already knows, so the edit is never the first thing it ` + - `learns about someone.` + `Couldn't work out who \`${mention}\` is. Mention them with @ so Slack ` + + `sends their account, or paste their Slack member ID.` ); } + const who = person?.full_name ?? `<@${slackId}>`; const reason = reasonWords.join(" ").trim(); @@ -396,7 +459,7 @@ async function groupText( actor: caller, groupRef: ref, action, - subject: person, + subject: person ?? { slackUserId: slackId }, reason: reason || null, source: "command", }); @@ -407,27 +470,36 @@ async function groupText( // where the caller's own words are still to hand. if ("needsReason" in outcome) { return ( - `*${person.full_name}* is a student. ${outcome.reason}\n` + + `*${who}* is a student. ${outcome.reason}\n` + `\`/hawkmod group add ${mention} ${group} \`` ); } return outcome.reason; } if (outcome.noop) { - return `*${person.full_name}* was already ${ + return `*${who}* was already ${ action === "add" ? "in" : "out of" } @${outcome.handle}. Nothing changed.`; } const lines = [ - `${action === "add" ? "Added" : "Removed"} *${person.full_name}* ` + + `${action === "add" ? "Added" : "Removed"} *${who}* ` + `${action === "add" ? "to" : "from"} @${outcome.handle}.`, ]; + // Says out loud that the roster is about to catch up, so a caller who runs + // `whois` a second later and sees nothing knows to wait rather than to worry. + if (!person && action === "add") { + lines.push( + `_hawk-mod had no roster entry for them. The user group sync creates one ` + + `within a few seconds; \`/hawkmod whois\` will show it._` + ); + } + // The honest half. Group membership declares a role; it does not end // monitoring, and saying otherwise here would be the quiet failure this // project exists to avoid. - if (action === "remove") { + if (action === "remove" && person) { lines.push( `_${person.full_name} is still a ${person.role} on the roster and still ` + `monitored. Leaving a group never ends monitoring — use ` + @@ -436,7 +508,7 @@ async function groupText( } if (outcome.reducedMonitoring) { lines.push( - `_${person.full_name} is no longer monitored as a student. Recorded ` + + `_${who} is no longer monitored as a student. Recorded ` + `against your name: ${reason}_` ); } @@ -489,3 +561,161 @@ async function deactivateText( } return lines.join("\n"); } + +/** + * `/hawkmod config` and `/hawkmod config set `. + * + * Everything here used to live in a `.env` file on the host, which meant a + * Slack admin could not change which user group declares students without an + * SSH session. `authz.ts` already rejected that shape of problem once, for + * administrative authority; this is the same argument applied to the settings + * that decide who is monitored. + * + * Credentials are deliberately absent, and cannot be added: `SETTINGS` is an + * allowlist. You cannot configure from Slack the things that let hawk-mod reach + * Slack, and changing the encryption key would make every stored token + * undecryptable. + */ +async function configText( + client: WebClient, + caller: Actor, + rest: string[] +): Promise { + const [verb, key, ...valueWords] = rest; + + if (!verb) return configListing(); + + if (verb !== "set") { + return ( + "Usage: `/hawkmod config` to show, " + + "`/hawkmod config set ` to change one." + ); + } + + if (!key || !isSettingKey(key)) { + return ( + `Unknown setting \`${key ?? "(none)"}\`. Settable: ` + + SETTING_KEYS.map((k) => `\`${k}\``).join(", ") + + ".\nSlack credentials and the encryption key are deliberately not " + + "settable from here." + ); + } + + const raw = valueWords.join(" ").trim(); + if (!raw) return `Usage: \`/hawkmod config set ${key} \`.`; + + const cleaned = await validateSetting(client, key, raw); + if ("error" in cleaned) return cleaned.error; + + const before = setting(key); + setSetting({ + key, + value: cleaned.value, + actor: caller.slackUserId, + actorName: caller.name, + }); + + const lines = [ + `*${SETTINGS[key].label}* is now \`${cleaned.value}\`` + + (before.value + ? ` (was \`${before.value}\`, from ${before.source})` + : "") + + ".", + ]; + + // Roles are read from these groups by everything downstream, so leaving the + // roster stale until 3am would mean the setting looked applied and was not. + if (key === "student-group" || key === "mentor-group") { + const stats = await syncRolesFromUserGroups(client); + lines.push( + `_Re-synced: ${stats.created} rostered, ${stats.changed} changed, ` + + `${stats.reactivated} resumed._` + ); + } + + return lines.join("\n"); +} + +function configListing(): string { + const rows = SETTING_KEYS.map((key) => { + const { value, source } = setting(key); + const where = + source === "slack" + ? "set here" + : source === "env" + ? `from ${SETTINGS[key].env}` + : "*not set*"; + return `• \`${key}\` — ${value ?? "—"} _(${where})_`; + }); + + const unset = SETTING_KEYS.filter((k) => setting(k).source === "unset"); + + return [ + "*Settings*", + ...rows, + "", + "`/hawkmod config set `", + ...(unset.length + ? ["", ...unset.map((k) => `_\`${k}\` is unset — ${SETTINGS[k].hint}._`)] + : []), + "_Slack credentials and the token encryption key stay in the environment " + + "and cannot be changed from here._", + ].join("\n"); +} + +/** + * Checks a value against Slack before storing it. + * + * A user group handle that does not resolve is a typo, and a stored typo reads + * exactly like an empty group: nobody rostered, nobody monitored, no complaint. + * The sweep would raise that eventually; refusing it here turns tomorrow's + * finding into an error message the person who caused it is still reading. + */ +async function validateSetting( + client: WebClient, + key: SettingKey, + raw: string +): Promise<{ value: string } | { error: string }> { + const kind = SETTINGS[key].kind; + + if (kind === "channel") { + // `<#C123|name>` when escaping is on, a bare id or #name when it is not. + const id = raw.match(/^<#([A-Z0-9]+)/i)?.[1] ?? raw.replace(/^#/, ""); + try { + const info = await client.conversations.info({ channel: id }); + if (!info.channel?.id) return { error: `No channel \`${raw}\`.` }; + return { value: info.channel.id }; + } catch (err) { + return { + error: + `Couldn't read \`${raw}\`: ${String(err)}\n` + + `hawk-mod must be a member of the channel it posts findings to.`, + }; + } + } + + const handles = raw + .split(",") + .map((h) => h.trim()) + .filter(Boolean) + .map((h) => h.match(/^]+)>$/i)?.[1] ?? h) + .map((h) => h.replace(/^@/, "")); + + if (kind === "usergroup" && handles.length !== 1) { + return { error: `\`${key}\` takes exactly one user group.` }; + } + + for (const handle of handles) { + const group = await resolveGroup(client, handle); + if (!group) { + return { + error: + `No user group @${handle} in this workspace. Nothing was changed — ` + + `a stored typo looks exactly like an empty group, which is why this ` + + `is checked before saving.`, + }; + } + } + + return { value: handles.join(",") }; +} diff --git a/src/slack/groupAdmin.ts b/src/slack/groupAdmin.ts index 60d5a99..f888896 100644 --- a/src/slack/groupAdmin.ts +++ b/src/slack/groupAdmin.ts @@ -1,5 +1,5 @@ import { WebClient } from "@slack/web-api"; -import { config, managedGroupHandles } from "../config.js"; +import { config } from "../config.js"; import { getInstallation, insertGroupChange } from "../db/repo.js"; import type { Person } from "../domain/people.js"; import { @@ -9,6 +9,7 @@ import { type GroupPlan, } from "../domain/rules/groupMembership.js"; import { log } from "../logger.js"; +import { managedGroupHandles, settingValue } from "../settings.js"; import type { Actor } from "./authz.js"; import { resolveGroup, setGroupMembership } from "./userGroups.js"; @@ -142,7 +143,7 @@ export async function applyGroupEdit( }; } - // Moving a student into the adults group ends their monitoring as a + // Moving a student into the mentors group ends their monitoring as a // student. Allowed — refusing would only push the same act into Slack's own // UI, where hawk-mod learns of it from an event with no author and no // reason — but never silently, and never by typo. @@ -152,7 +153,7 @@ export async function applyGroupEdit( action: req.action, subjectRole: req.subject.role, handle: group.handle, - adultHandle: config().ADULT_USERGROUP ?? "adults", + adultHandle: settingValue("mentor-group") ?? "mentors", }); if (reduces && !req.reason) { diff --git a/test/groupMembership.test.ts b/test/groupMembership.test.ts index fe6161e..df5d13b 100644 --- a/test/groupMembership.test.ts +++ b/test/groupMembership.test.ts @@ -98,27 +98,27 @@ describe("group membership plans", () => { * It is keyed on the resolved handle for a reason — see the regression below. */ describe("edits that end a student's monitoring", () => { - const adults = "adults"; + const mentors = "mentors"; - it("flags a student being added to the adults group", () => { + it("flags a student being added to the mentors group", () => { assert.equal( reducesMonitoring({ action: "add", subjectRole: "student", - handle: "adults", - adultHandle: adults, + handle: "mentors", + adultHandle: mentors, }), true ); }); - it("ignores an adult being added to the adults group", () => { + it("ignores an adult being added to the mentors group", () => { assert.equal( reducesMonitoring({ action: "add", subjectRole: "adult", - handle: "adults", - adultHandle: adults, + handle: "mentors", + adultHandle: mentors, }), false ); @@ -130,7 +130,7 @@ describe("edits that end a student's monitoring", () => { action: "add", subjectRole: "student", handle: "programming", - adultHandle: adults, + adultHandle: mentors, }), false ); @@ -141,8 +141,8 @@ describe("edits that end a student's monitoring", () => { reducesMonitoring({ action: "remove", subjectRole: "student", - handle: "adults", - adultHandle: adults, + handle: "mentors", + adultHandle: mentors, }), false ); @@ -153,8 +153,8 @@ describe("edits that end a student's monitoring", () => { reducesMonitoring({ action: "add", subjectRole: "student", - handle: "Adults", - adultHandle: "@adults", + handle: "mentors", + adultHandle: "@Mentors", }), true ); @@ -163,9 +163,9 @@ describe("edits that end a student's monitoring", () => { /** * Regression. The first version of this compared the *raw slash-command * argument* to the configured handle. The app sets `should_escape: true`, so - * Slack sends `` and the raw argument is an opaque - * id — which never equals "adults", so the gate never fired and a student - * could be moved into the adults group by typo, with no reason recorded. + * Slack sends `` and the raw argument is an opaque + * id — which never equals "mentors", so the gate never fired and a student + * could be moved into the mentors group by typo, with no reason recorded. * Only the resolved handle is a safe input here. */ it("is not fooled by a group id, because it never sees one", () => { @@ -174,7 +174,7 @@ describe("edits that end a student's monitoring", () => { action: "add", subjectRole: "student", handle: "S0614TY5A", - adultHandle: adults, + adultHandle: mentors, }), false ); diff --git a/test/settings.test.ts b/test/settings.test.ts new file mode 100644 index 0000000..41495d0 --- /dev/null +++ b/test/settings.test.ts @@ -0,0 +1,113 @@ +import assert from "node:assert/strict"; +import { describe, it } from "node:test"; +import { + isSettingKey, + parseHandles, + resolveSetting, + SETTINGS, + SETTING_KEYS, +} from "../src/settings.js"; + +describe("setting resolution", () => { + it("prefers a value set from Slack", () => { + assert.deepEqual(resolveSetting("mentors", "adults"), { + value: "mentors", + source: "slack", + }); + }); + + it("falls back to the environment", () => { + // The point of the fallback: an existing host keeps working unchanged, so + // moving these settings needs no flag day and no coordinated deploy. + assert.deepEqual(resolveSetting(undefined, "adults"), { + value: "adults", + source: "env", + }); + }); + + it("reports unset rather than guessing", () => { + assert.deepEqual(resolveSetting(undefined, undefined), { + value: undefined, + source: "unset", + }); + }); + + it("treats blank as unset on both sides", () => { + // A variable set to "" in a compose file is the common shape of this, and + // it must not beat a real value stored in Slack. + assert.deepEqual(resolveSetting(" ", "adults"), { + value: "adults", + source: "env", + }); + assert.deepEqual(resolveSetting("", ""), { + value: undefined, + source: "unset", + }); + }); + + it("trims, because a trailing space in a handle finds no group", () => { + assert.deepEqual(resolveSetting(" mentors ", undefined), { + value: "mentors", + source: "slack", + }); + }); +}); + +describe("handle parsing", () => { + it("splits a comma separated list", () => { + assert.deepEqual(parseHandles("programming, drive-team"), [ + "programming", + "drive-team", + ]); + }); + + it("strips @ and lowercases, so stored and typed forms match", () => { + assert.deepEqual(parseHandles("@Mentors"), ["mentors"]); + }); + + it("survives empty entries and trailing commas", () => { + assert.deepEqual(parseHandles("a,,b,"), ["a", "b"]); + assert.deepEqual(parseHandles(undefined), []); + assert.deepEqual(parseHandles(""), []); + }); +}); + +describe("the settable allowlist", () => { + it("recognises only known keys", () => { + assert.equal(isSettingKey("student-group"), true); + assert.equal(isSettingKey("nonsense"), false); + }); + + /** + * The load-bearing property. Slack credentials cannot be configured from + * Slack, and TOKEN_ENCRYPTION_KEY changing would make every stored token + * undecryptable — every enrolled adult silently invisible, with coverage + * still reading 100%. + */ + it("cannot reach credentials or the encryption key", () => { + const forbidden = [ + "SLACK_SIGNING_SECRET", + "SLACK_CLIENT_ID", + "SLACK_CLIENT_SECRET", + "SLACK_STATE_SECRET", + "TOKEN_ENCRYPTION_KEY", + "DATA_DIR", + "PUBLIC_URL", + "PORT", + ]; + const reachable = SETTING_KEYS.map((k) => SETTINGS[k].env); + for (const env of forbidden) { + assert.ok( + !reachable.includes(env as never), + `${env} must not be settable from Slack` + ); + } + }); + + it("every key names an env var to fall back to", () => { + for (const key of SETTING_KEYS) { + assert.ok(SETTINGS[key].env, `${key} has no env fallback`); + assert.ok(SETTINGS[key].label, `${key} has no label`); + } + }); +});