diff --git a/CLAUDE.md b/CLAUDE.md index ba12e2d..2300ac2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -28,10 +28,12 @@ npx tsx --test --test-name-pattern "two adults" test/rules.test.ts ``` CLI subcommands: `import-roster`, `import-consents`, `set-role`, `sweep`, -`backfill`, `findings [status]`, `export-conversation [out.json]`. -`set-role` is the bootstrap that matters — the user-group sync only ever -assigns `student` or `adult`, so the first `lead_coach` must be set from the -CLI or nobody can run `/hawkmod` at all. +`backfill`, `findings [status]`, `export-conversation [out.json]`. None of +them is a bootstrap step: administrative access is Slack's Workspace +Owner/Admin flags, read live in `src/slack/authz.ts`, so a fresh install is +usable by whoever installed it without anyone touching the host. Don't +reintroduce a roster role that grants access — see the "Lead Coach" note in +`docs/policy-mapping.md`. ## What this is diff --git a/README.md b/README.md index bc05e8f..5c5be5e 100644 --- a/README.md +++ b/README.md @@ -49,18 +49,19 @@ appear: - **adults who have not authorized hawk-mod, or who revoked it** — see below - channels containing students with fewer than two screened adults (§4.2) - fewer than two workspace owners, or a student holding Owner/Admin (§6) +- fewer than two of those Owners/Admins being screened adults on the roster (§3) Findings are posted once to a private channel, deduplicated, auto-closed when -the underlying problem goes away, and closable by a Lead Coach from **Resolve** -and **Acknowledge** buttons on the alert itself. +the underlying problem goes away, and closable by a workspace admin from +**Resolve** and **Acknowledge** buttons on the alert itself. -**The adults involved get a private nudge, not just the Lead Coaches.** When a +**The adults involved get a private nudge, not just the coaches.** When a DM raises a finding, hawk-mod sends each adult in it a direct message naming the rule and the concrete fix — add a second screened adult, or move it to a channel. It is deliberately a colleague's heads-up rather than a warning: most violations are people who did not know the rule, and an adult who feels accused moves the conversation somewhere nobody can see it. The nudge goes once per -occurrence, it says plainly that a Lead Coach has been notified, and it never +occurrence, it says plainly that a coach has been notified, and it never replaces the finding. **Students never receive one.** Youth protection governs adult conduct toward @@ -169,7 +170,9 @@ and gates on it. 4. `docker compose up -d` (or `npm install && npm run dev`). For the server — DNS, TLS, backups, and the public-reachability requirement — see [docs/deploy.md](docs/deploy.md). -5. A Lead Coach installs the app: visit `$PUBLIC_URL/slack/install`. +5. A Slack workspace Owner or Admin installs the app: visit + `$PUBLIC_URL/slack/install`. Whoever can install it can administer it — + there is nothing to grant afterwards. 6. Import the roster and the consents you have already collected: ```bash @@ -235,8 +238,9 @@ projection of Slack — not a second roster to keep in sync. email,full_name,role,ypp_completed_on,mentor_ready_on,cori_completed_on,active,notes ``` -`role` is one of `student`, `adult`, `lead_coach`, `admin`, -`district_observer` — the last being the MPS administrator seat from §8. Dates +`role` is one of `student`, `adult`, `district_observer` — the last being the +MPS administrator seat from §8. Nothing in this file grants access to +`/hawkmod`; that is Slack's Owner/Admin, and only Slack's. Dates are `YYYY-MM-DD`. Email is the join key; Slack IDs are matched automatically once people sign up. **If a Slack account's email doesn't match a roster row it resolves to an unknown account, not a student** — which produces silence rather @@ -258,7 +262,9 @@ the signed copy is actually filed. ## Commands -In Slack, restricted to Lead Coaches and admins: +In Slack, restricted to the workspace's Owners and Admins — read live from +Slack on every command, so granting or revoking access is something you do in +Slack's own admin settings and nowhere else: ``` /hawkmod status | enroll | findings [kind] | whois @user @@ -266,6 +272,9 @@ In Slack, restricted to Lead Coaches and admins: /hawkmod ack | resolve | sweep | backfill ``` +There is deliberately no roster role that confers this. A student who somehow +holds Owner or Admin is refused anyway, and reported as a §6 violation. + `screening` and `consent` open a form in Slack. They are how screening dates and consent records get in day to day — the CSV importers below are a season-start bulk load, not a workflow. Both record who entered what and when, diff --git a/docs/deploy.md b/docs/deploy.md index aaedeff..8e8f53a 100644 --- a/docs/deploy.md +++ b/docs/deploy.md @@ -90,7 +90,7 @@ before setup — running and waiting. ## 4. Set up the workspace -1. A Lead Coach visits `https://hawk-mod.example.org/slack/install`. +1. A workspace Owner or Admin visits `https://hawk-mod.example.org/slack/install`. 2. Invite the bot to the private findings channel. 3. Import the roster and consents (see the README). 4. Send every adult the same install URL, and watch `/hawkmod status` until diff --git a/docs/examples/roster.example.csv b/docs/examples/roster.example.csv index a662292..42fd3ed 100644 --- a/docs/examples/roster.example.csv +++ b/docs/examples/roster.example.csv @@ -1,9 +1,9 @@ email,full_name,role,ypp_completed_on,ypt_completed_on,mentor_ready_on,cori_completed_on,active,notes -lead1@example.org,Alex Lead,lead_coach,2026-09-01,2026-09-01,2026-09-01,2024-08-15,1, -lead2@example.org,Sam Lead,lead_coach,2026-09-01,2026-09-01,2026-09-01,2025-01-20,1, +lead1@example.org,Alex Lead,adult,2026-09-01,2026-09-01,2026-09-01,2024-08-15,1,Workspace Owner — §3 +lead2@example.org,Sam Lead,adult,2026-09-01,2026-09-01,2026-09-01,2025-01-20,1,Workspace Owner — §3 adult1@example.org,Jordan Adult,adult,2026-09-03,2026-09-03,2026-09-03,2024-09-10,1, adult2@example.org,Casey Adult,adult,2026-09-05,2026-09-05,2026-09-05,2026-01-05,1,MPS employee — see policy IJNDD observer@melroseschools.com,MPS Administrator,district_observer,2026-08-20,2026-08-20,2026-08-20,2025-06-01,1,District visibility seat (§8) -treasurer@example.org,Pat Treasurer,admin,2026-09-01,2026-09-01,2026-09-01,2024-08-15,1,Workspace owner #2 +treasurer@example.org,Pat Treasurer,adult,2026-09-01,2026-09-01,2026-09-01,2024-08-15,1,Workspace Owner #3 student1@example.org,Robin Student,student,,,,,1, student2@example.org,Avery Student,student,,,,,1, diff --git a/docs/policy-mapping.md b/docs/policy-mapping.md index b40bfd8..23cbd88 100644 --- a/docs/policy-mapping.md +++ b/docs/policy-mapping.md @@ -8,7 +8,7 @@ Each control from _Moving Team Communication to Slack_, and what carries it. | 2 | Parental consent on file before a student account exists | `consent.ts`; `team_join` event raises `unconsented_account` the moment an account appears; nightly sweep re-checks | | 2 | Consent re-collected annually | `CONSENT_VALID_YEARS = 1`; consents expire rather than linger | | 2 | Consents kept on file and producible | `consents` table records `document_ref`; the signed copies themselves live wherever the team files them — **manual** | -| 3 | Two YPP-screened Lead Coaches | roster `role = lead_coach` + `screening_lapsed` findings | +| 3 | Two YPP-screened Lead Coaches | `workspace_config` `screened_admins`: two of the workspace's Owners/Admins must be screened adults on the roster, plus `screening_lapsed` findings | | 3 | Written communications copied to a second adult | `dmPolicy` — two screened adults required in any student conversation | | 4.1 | No 1:1 adult–student DMs, ever | `dmPolicy` `one_to_one_adult_student`, raised on each new message and on backfill; a message after a finding is closed raises it again (`recurrence.ts`) | | 4.2 | Two screened adults in every channel students are in | `twoAdults.ts`; re-evaluated on every join/leave, plus nightly | @@ -36,6 +36,17 @@ training inside it is required. hawk-mod tracks it and reports it as outstanding, but it never blocks screened-adult status. Requiring it would have excluded adults who had done everything actually asked of them. +**"Lead Coach" is not a role hawk-mod stores.** It used to be: a roster role +that granted every administrative action in the app. That put a +youth-protection permission behind a label anyone with CLI access could type, +checked against nothing, and it meant a freshly installed app had no +administrator at all until someone opened a shell on the host. Authority is now +read live from Slack's Workspace Owner/Admin flags, which the workspace already +manages and audits. §3 is still checked — see the `screened_admins` row — but +it asks the question of the people who demonstrably hold the authority rather +than of a self-assigned label. A student holding Owner or Admin is refused +regardless (§6), and reported. + **The screening and the training run on different clocks.** The background screening is valid for longer than a year; the training is annual. Treating both as annual flags people who are current, and an alert channel that cries diff --git a/migrations/0005_authority_from_slack.sql b/migrations/0005_authority_from_slack.sql new file mode 100644 index 0000000..4a8f7fb --- /dev/null +++ b/migrations/0005_authority_from_slack.sql @@ -0,0 +1,30 @@ +-- Administrative authority moved out of the roster and onto Slack's own +-- Workspace Owner/Admin flags (see src/slack/authz.ts). The `lead_coach` and +-- `admin` roles existed only to answer "may this person run /hawkmod", which +-- Slack already answers, and answering it here meant a fresh install had no +-- administrator at all until someone ran the CLI on the host. +-- +-- Both collapse to `adult`. No rule loses anything: enrollment, screening, and +-- the two-adult rule already treated all three identically. + +INSERT INTO role_changes + (person_id, slack_user_id, from_role, to_role, source, detail, changed_at) +SELECT id, slack_user_id, role, 'adult', 'migration', + json_object('migration', '0005_authority_from_slack', + 'reason', 'authority now read from Slack admin/owner'), + strftime('%Y-%m-%dT%H:%M:%fZ', 'now') +FROM people +WHERE role IN ('lead_coach', 'admin'); + +UPDATE people +SET role = 'adult', + updated_at = strftime('%Y-%m-%dT%H:%M:%fZ', 'now') +WHERE role IN ('lead_coach', 'admin'); + +-- The CHECK constraint on people.role still lists the two retired names, and +-- stays that way on purpose. Dropping a CHECK in SQLite means rebuilding the +-- table, and `people` is the parent of four ON DELETE CASCADE foreign keys — +-- consents among them. A migration that can destroy consent records to tidy a +-- constraint is a bad trade. Nothing can write the old values now: ROLES no +-- longer contains them, the CLI validates against ROLES, and the user-group +-- sync only ever writes 'student' or 'adult'. diff --git a/scripts/setup-local.sh b/scripts/setup-local.sh index 880f423..ea2e3a7 100755 --- a/scripts/setup-local.sh +++ b/scripts/setup-local.sh @@ -630,14 +630,8 @@ const {listPeople}=require("./dist/src/db/repo.js"); for (const p of listPeople()) console.log(` ${p.role.padEnd(18)} ${p.full_name}`); ' 2>/dev/null || note " (could not list — check the logs)" say "" -say "The group sync only ever assigns student or adult, so make yourself a" -say "Lead Coach — /hawkmod commands need one to exist." -ask LEAD_COACH_EMAIL "Your Slack email:" -"${COMPOSE[@]}" exec -T hawk-mod node dist/src/cli/index.js \ - set-role "$LEAD_COACH_EMAIL" lead_coach | sed 's/^/ /' \ - || warn "couldn't set the role — is that the email on your Slack account?" -note "Being a lead_coach also survives the next sync: someone already recorded" -note "as a lead coach is not demoted for being in the adults group." +note "Nothing to grant yourself here: /hawkmod answers to Slack's Workspace" +note "Owners and Admins, read live. You installed the app, so you can run it." say "" step "Now record consent for the test student, from Slack:" note " /hawkmod consent @their-account" diff --git a/src/cli/index.ts b/src/cli/index.ts index 71f3cfd..1638581 100644 --- a/src/cli/index.ts +++ b/src/cli/index.ts @@ -22,9 +22,9 @@ const USAGE = `hawk-mod cli cori_completed_on,active,notes import-consents email,signed_on,form_version,guardian_name, guardian_email,document_ref,recorded_by[,expires_on] - set-role role: student|adult|lead_coach|admin| - district_observer. Bootstraps the first Lead - Coach, since /hawkmod needs one to exist. + set-role role: student|adult|district_observer. + Nothing here grants access to /hawkmod — that + is Slack's Owner/Admin, read live. sweep run the compliance sweep backfill walk enrolled adults' DM history findings [status] list findings (default: open) @@ -124,9 +124,9 @@ function exportConversation(id: string, out?: string) { } /** - * The user group sync only ever assigns `student` or `adult`, so the first - * Lead Coach has to be set from outside Slack — otherwise nobody can run - * /hawkmod at all. Recorded in role_changes like any other role change. + * The user group sync only ever assigns `student` or `adult`, so + * `district_observer` (§8) has to be set from outside Slack. Recorded in + * role_changes like any other role change. */ function setRole(who: string, role: string) { if (!ROLES.includes(role as Role)) { diff --git a/src/domain/guidance.ts b/src/domain/guidance.ts index c89b720..a54fa53 100644 --- a/src/domain/guidance.ts +++ b/src/domain/guidance.ts @@ -6,7 +6,7 @@ import type { DmVerdict } from "./rules/dmPolicy.js"; * * The audience is someone who almost certainly did not know the rule, so the * tone is a colleague's, not a compliance system's: name the rule, give the - * concrete fix, say plainly that a Lead Coach can see it, and do not imply + * concrete fix, say plainly that a coach can see it, and do not imply * anyone is in trouble. An adult who feels accused moves the conversation * somewhere nobody can see it, which is the opposite of what this is for. * @@ -23,8 +23,8 @@ const OPENING = ":wave: Hi — a quick heads-up, not a telling-off."; const CLOSING = "Conversations that include a student are recorded for youth-protection " + - "audit, and a Lead Coach has been notified as usual. Nothing here is an " + - "accusation — if you are not sure what to do, just ask a Lead Coach."; + "audit, and a coach has been notified as usual. Nothing here is an " + + "accusation — if you are not sure what to do, just ask a coach."; function mentions(ids: string[]): string { return ids.map((id) => `<@${id}>`).join(", "); @@ -67,7 +67,7 @@ export function guidanceFor(verdict: DmVerdict): string | null { `This conversation includes a student and ` + `${verdict.unknownIds.length} account(s) that are not on the team ` + `roster, so ${APP_NAME} cannot tell whether they are a screened adult.`, - "*To put it right:* ask a Lead Coach to add them to the roster, or " + + "*To put it right:* ask a coach to add them to the roster, or " + "to the students or adults user group. Until then the " + "conversation counts as an exception.", ]; diff --git a/src/domain/people.ts b/src/domain/people.ts index bb6b576..66b37ad 100644 --- a/src/domain/people.ts +++ b/src/domain/people.ts @@ -1,12 +1,12 @@ import type { IsoDate } from "./dates.js"; -export const ROLES = [ - "student", - "adult", - "lead_coach", - "admin", - "district_observer", -] as const; +/** + * What the roster says about a person, and nothing more. Deliberately no role + * for "runs hawk-mod": administrative authority is Slack's Owner/Admin flags, + * read live in `slack/authz.ts`, so there is nothing here to keep in sync with + * Slack and nothing to bootstrap by hand on a fresh install. + */ +export const ROLES = ["student", "adult", "district_observer"] as const; export type Role = (typeof ROLES)[number]; @@ -43,29 +43,19 @@ export function isStudent(m: Member): boolean { } /** - * Anyone on the roster who is not a student — `lead_coach`, `admin`, and - * `district_observer` included, not just the `adult` role. Do not rewrite this - * as `role === "adult"`: a Lead Coach alone with a student is exactly the - * situation the rules exist for. Unknown members are not adults either, so an - * unidentified account can never satisfy the two-adult rule. + * Anyone on the roster who is not a student — `district_observer` included, + * not just the `adult` role, and a workspace Owner is no exception. Seniority + * has never been an argument for being alone with a student; that is exactly + * the situation the rules exist for. Unknown members are not adults either, so + * an unidentified account can never satisfy the two-adult rule. */ export function isAdult(m: Member): boolean { return isKnown(m) && m.role !== "student"; } -/** Roles that may hold Workspace Owner/Admin. Never a student (§6). */ -export function mayAdministerWorkspace(p: Person): boolean { - return p.role === "lead_coach" || p.role === "admin"; -} - /** Roles whose DMs hawk-mod expects to monitor via an enrolled user token. */ export function requiresEnrollment(p: Person): boolean { - return ( - p.active === 1 && - (p.role === "adult" || - p.role === "lead_coach" || - p.role === "district_observer") - ); + return p.active === 1 && p.role !== "student"; } export function label(m: Member): string { diff --git a/src/domain/rules/rosterSync.ts b/src/domain/rules/rosterSync.ts index d0ad50a..05644b8 100644 --- a/src/domain/rules/rosterSync.ts +++ b/src/domain/rules/rosterSync.ts @@ -23,12 +23,7 @@ export type RoleDecision = /** In both groups at once; too ambiguous to act on. */ | { kind: "conflict"; slackId: string; personId: number | null }; -const ADULT_ROLES: Role[] = [ - "adult", - "lead_coach", - "admin", - "district_observer", -]; +const ADULT_ROLES: Role[] = ["adult", "district_observer"]; function isAdultRole(role: Role): boolean { return ADULT_ROLES.includes(role); @@ -74,8 +69,8 @@ export function reconcileRoles( continue; } - // Someone already recorded as a lead coach, admin, or district observer is - // in the adults group too; that is agreement, not a demotion to `adult`. + // A district observer in the adults group is agreement, not a demotion to + // plain `adult`. if (target === "adult" && isAdultRole(person.role)) { decisions.push({ kind: "unchanged", slackId }); continue; diff --git a/src/health.ts b/src/health.ts index dc63d72..6165895 100644 --- a/src/health.ts +++ b/src/health.ts @@ -15,7 +15,7 @@ type Health = { * later, when the failing thing would be a DM nobody recorded. * * `installed: false` is not unhealthy — a freshly deployed container is - * expected to be running and waiting for a Lead Coach to install the app. + * expected to be running and waiting for an admin to install the app. */ export function healthHandler(_req: unknown, res: ServerResponse): void { let body: Health; diff --git a/src/jobs/sweep.ts b/src/jobs/sweep.ts index 0991098..dfbafe2 100644 --- a/src/jobs/sweep.ts +++ b/src/jobs/sweep.ts @@ -150,7 +150,9 @@ export async function runSweep(): Promise { } } - if (p.role === "adult" || p.role === "lead_coach") { + // Every adult on the roster, `district_observer` included (§8: they count + // as an adult only once screening dates are recorded). + if (p.role !== "student") { const screening = screeningStatus(p, asOf); if (!screening.current) { stats.screeningLapsed += 1; @@ -286,6 +288,33 @@ export async function runSweep(): Promise { } } + /* ---- responsible adults (§3) ------------------------------------------ */ + + // FIRST requires two screened Lead Coaches. hawk-mod used to answer this + // from a `lead_coach` roster role — a label anyone could type, that granted + // administrative access, and that nothing checked against reality. The + // people who actually hold the authority are the workspace's Owners and + // Admins, which Slack knows and nobody can quietly self-assign here, so the + // question becomes: are at least two of them screened adults on the roster? + const responsible = users.filter((u) => { + if (u.isDeleted || u.isBot || !(u.isOwner || u.isAdmin)) return false; + const person = rosterBySlackId.get(u.id); + if (!person || person.role === "student" || person.active !== 1) + return false; + return screeningStatus(person, asOf).current; + }); + if (responsible.length < 2) { + await emit({ + kind: "workspace_config", + dedupeKey: dedupeKey("workspace_config", "screened_admins"), + severity: "violation", + summary: + `${responsible.length} of the workspace's Owners/Admins are screened ` + + `adults on the roster; FIRST requires two (§3).`, + detail: { screened: responsible.map((u) => u.id) }, + }); + } + const closed = autoResolveMissing(SWEEP_OWNED, seen); stats.findingsClosed = closed.length; // Redraw each one: a finding closed in the database but still showing diff --git a/src/monitor/conversations.ts b/src/monitor/conversations.ts index 0e8f663..fdc54af 100644 --- a/src/monitor/conversations.ts +++ b/src/monitor/conversations.ts @@ -176,7 +176,7 @@ export async function raiseDmViolation( { at: tsToIso(ts) } ); - // Only when the Lead Coaches were told. Guidance rides on the alert rather + // Only when the alert channel was told. Guidance rides on the alert rather // than on the message, so an adult gets one nudge per occurrence instead of // one per line they type — and it never quietly replaces the finding. if (alerted) await nudgeAdults(verdict); diff --git a/src/slack/actions.ts b/src/slack/actions.ts index 8a1b6bc..f5cb353 100644 --- a/src/slack/actions.ts +++ b/src/slack/actions.ts @@ -1,9 +1,9 @@ import type { App } from "@slack/bolt"; import { closeFinding } from "../close.js"; -import { getFinding, personBySlackId } from "../db/repo.js"; -import { mayAdministerWorkspace } from "../domain/people.js"; +import { getFinding } from "../db/repo.js"; import { log } from "../logger.js"; import { ACK_ACTION, RESOLVE_ACTION, refreshFinding } from "./alerts.js"; +import { administrator } from "./authz.js"; const NOTE_MODAL = "hawkmod_finding_note"; const NOTE_BLOCK = "note"; @@ -79,13 +79,13 @@ export function registerActions(app: App): void { const findingId = Number(payload.actions?.[0]?.value); if (!Number.isInteger(findingId) || !payload.trigger_id) return; - const caller = personBySlackId(payload.user.id); + const caller = await administrator(client, payload.user.id); const finding = getFinding(findingId); if (!finding) return; // Anyone who can see the channel can click; only the people responsible // for youth protection may close. - if (!caller || !mayAdministerWorkspace(caller)) { + if (!caller) { await client.views.open({ trigger_id: payload.trigger_id, view: { @@ -97,7 +97,7 @@ export function registerActions(app: App): void { type: "section", text: { type: "mrkdwn", - text: "Only Lead Coaches and workspace admins can close findings.", + text: "Only Slack workspace Owners and Admins can close findings.", }, }, ], @@ -118,13 +118,14 @@ export function registerActions(app: App): void { }); } - app.view(NOTE_MODAL, async ({ ack, body, view }) => { - const caller = personBySlackId(body.user.id); - if (!caller || !mayAdministerWorkspace(caller)) { + app.view(NOTE_MODAL, async ({ ack, body, view, client }) => { + const caller = await administrator(client, body.user.id); + if (!caller) { await ack({ response_action: "errors", errors: { - [NOTE_BLOCK]: "Only Lead Coaches and admins can close findings.", + [NOTE_BLOCK]: + "Only Slack workspace Owners and Admins can close findings.", }, }); return; @@ -145,11 +146,11 @@ export function registerActions(app: App): void { try { await ack(); - await closeFinding(findingId, caller.full_name, note, status); + await closeFinding(findingId, caller.name, note, status); log.info("finding closed from Slack", { findingId, status, - by: caller.full_name, + by: caller.name, }); } catch (err) { log.error("could not close finding", { findingId, error: String(err) }); diff --git a/src/slack/app.ts b/src/slack/app.ts index f82769d..c92369f 100644 --- a/src/slack/app.ts +++ b/src/slack/app.ts @@ -75,7 +75,7 @@ export function createApp(): App { include a student, and records them for youth-protection audit. Conversations with no student in them are not recorded.

You can revoke this at any time from Slack → Settings - → Manage apps. Revoking is reported to the Lead Coaches, + → Manage apps. Revoking is reported to the coaches, because unmonitored is not the same as compliant.

` ); @@ -83,7 +83,7 @@ export function createApp(): App { failure: (error, _options, _req, res) => { log.error("oauth failure", { error: String(error) }); res.writeHead(500, { "content-type": "text/plain; charset=utf-8" }); - res.end("Authorization failed. Tell a Lead Coach."); + res.end("Authorization failed. Tell a coach."); }, }, }, diff --git a/src/slack/authz.ts b/src/slack/authz.ts new file mode 100644 index 0000000..391a0c2 --- /dev/null +++ b/src/slack/authz.ts @@ -0,0 +1,120 @@ +import type { WebClient } from "@slack/web-api"; +import { APP_NAME } from "../brand.js"; +import { personBySlackId } from "../db/repo.js"; +import type { Person } from "../domain/people.js"; +import { log } from "../logger.js"; + +/** + * Who may administer hawk-mod: Slack's own Workspace Owners and Admins, read + * live from `users.info`. + * + * This used to be a roster role. That was a second, hand-maintained copy of a + * fact Slack already knows, and it had a worse failure mode than being merely + * redundant: a fresh install had an empty roster, so *nobody* could run + * `/hawkmod`, and the only way out was a shell on the host running the CLI. + * An app whose first-run instruction is "SSH into the server" is broken. + * Whoever can install the app can now use it. + * + * The roster still says who is a student, who is an adult, and who has been + * screened. It no longer says who is in charge. + */ +export const NOT_PERMITTED = `${APP_NAME} is limited to Slack workspace Owners and Admins.`; + +export type Actor = { + slackUserId: string; + /** Roster name where there is one, Slack's profile name otherwise. */ + name: string; + /** The roster row, when this person has one. Admins need not be on it. */ + person: Person | undefined; +}; + +/** + * Long enough that a burst of button clicks is one API call, short enough that + * revoking someone's Slack admin revokes their access to findings within a + * minute. Promotions are picked up just as fast, which matters on first run. + */ +const TTL_MS = 60_000; + +const cache = new Map(); + +/** Only for tests, and for the rare caller that must not see a stale verdict. */ +export function forgetAuthorization(slackUserId?: string): void { + if (slackUserId) cache.delete(slackUserId); + else cache.clear(); +} + +type Deps = { + now?: number; + /** Injected by the tests so this module can be exercised without a database. */ + lookup?: (slackUserId: string) => Person | undefined; +}; + +/** + * Resolves the caller if they may administer hawk-mod, `null` if they may not. + * + * Fails closed: a Slack API error denies the action rather than allowing it. + * Findings name students and describe conduct concerns, so the cost of a + * wrongly-allowed read is much higher than the cost of a retry. + */ +export async function administrator( + client: WebClient, + slackUserId: string, + deps: Deps = {} +): Promise { + const now = deps.now ?? Date.now(); + const hit = cache.get(slackUserId); + if (hit && now - hit.at < TTL_MS) return hit.actor; + + const actor = await resolve( + client, + slackUserId, + deps.lookup ?? personBySlackId + ); + cache.set(slackUserId, { at: now, actor }); + return actor; +} + +async function resolve( + client: WebClient, + slackUserId: string, + lookup: (slackUserId: string) => Person | undefined +): Promise { + let user; + try { + user = (await client.users.info({ user: slackUserId })).user; + } catch (err) { + log.error("could not read caller from Slack; denying", { + slackUserId, + error: String(err), + }); + return null; + } + if (!user || user.deleted || user.is_bot) return null; + + const person = lookup(slackUserId); + + // §6: a student must never hold Owner or Admin. The sweep raises that as a + // violation, but the finding is written to a channel this student could then + // read. Refusing here means the workspace being misconfigured for an hour + // does not also hand a minor the youth-protection case file. + if (person?.role === "student") { + log.warn("student holds workspace admin; refused", { + slackUserId, + person: person.email, + }); + return null; + } + + if (!(user.is_admin || user.is_owner || user.is_primary_owner)) return null; + + return { + slackUserId, + name: + person?.full_name ?? + user.profile?.real_name ?? + user.real_name ?? + user.name ?? + slackUserId, + person, + }; +} diff --git a/src/slack/commands.ts b/src/slack/commands.ts index 071503a..d29c4eb 100644 --- a/src/slack/commands.ts +++ b/src/slack/commands.ts @@ -15,15 +15,12 @@ import { } from "../db/repo.js"; import { today } from "../domain/dates.js"; import { severityEmoji } from "../domain/findings.js"; -import { - mayAdministerWorkspace, - requiresEnrollment, - type Person, -} from "../domain/people.js"; +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 { backfillAll } from "../monitor/backfill.js"; +import { administrator, NOT_PERMITTED } from "./authz.js"; import { openConsent, openScreening } from "./modals.js"; import { runSweep } from "../jobs/sweep.js"; import { syncRolesFromUserGroups } from "../jobs/syncRoles.js"; @@ -50,14 +47,11 @@ export function registerCommands(app: App): void { app.command("/hawkmod", async ({ command, ack, respond, client }) => { await ack(); - // Findings name students and describe conduct concerns. Only the adults - // responsible for youth protection get to read them. - const caller = personBySlackId(command.user_id); - if (!caller || !mayAdministerWorkspace(caller)) { - await respond({ - response_type: "ephemeral", - text: `${APP_NAME} is limited to Lead Coaches and workspace admins.`, - }); + // Findings name students and describe conduct concerns. Only the people + // Slack already trusts to run the workspace get to read them. + const caller = await administrator(client, command.user_id); + if (!caller) { + await respond({ response_type: "ephemeral", text: NOT_PERMITTED }); return; } @@ -140,7 +134,7 @@ export function registerCommands(app: App): void { } await closeFinding( id, - caller.full_name, + caller.name, note, sub === "ack" ? "acknowledged" : "resolved" ); diff --git a/src/slack/installStore.ts b/src/slack/installStore.ts index 5a141ed..786fe1c 100644 --- a/src/slack/installStore.ts +++ b/src/slack/installStore.ts @@ -31,7 +31,7 @@ function teamKey( /** * Two kinds of row live here: * - * 'bot' — one per workspace, installed once by a Lead Coach. Posts alerts. + * 'bot' — one per workspace, installed once by a workspace admin. Posts alerts. * 'user' — one per enrolled adult, holding that adult's user token. This is * what lets hawk-mod see DMs at all; Slack exposes no other way to * read them below Enterprise Grid. diff --git a/src/slack/modals.ts b/src/slack/modals.ts index f42e404..e489f58 100644 --- a/src/slack/modals.ts +++ b/src/slack/modals.ts @@ -6,14 +6,14 @@ import { insertConsent, listConsents, personById, - personBySlackId, setScreeningDates, type ScreeningField, } from "../db/repo.js"; import { closeFinding } from "../close.js"; import { today } from "../domain/dates.js"; import { dedupeKey } from "../domain/findings.js"; -import { mayAdministerWorkspace, type Person } from "../domain/people.js"; +import { type Person } from "../domain/people.js"; +import { administrator } from "./authz.js"; import { consentStatus, defaultExpiry, @@ -234,12 +234,14 @@ async function settleConsent(personId: number): Promise { } export function registerViews(app: App): void { - app.view(SCREENING_MODAL, async ({ ack, body, view }) => { - const caller = personBySlackId(body.user.id); - if (!caller || !mayAdministerWorkspace(caller)) { + app.view(SCREENING_MODAL, async ({ ack, body, view, client }) => { + const caller = await administrator(client, body.user.id); + if (!caller) { await ack({ response_action: "errors", - errors: { ypp: "Only Lead Coaches and admins can record screening." }, + errors: { + ypp: "Only Slack workspace Owners and Admins can record screening.", + }, }); return; } @@ -273,14 +275,14 @@ export function registerViews(app: App): void { const changed = setScreeningDates({ personId, values, - recordedBy: caller.full_name, + recordedBy: caller.name, source: "slack_modal", }); await ack(); await settleScreening(personId); log.info("screening recorded", { personId, - by: caller.full_name, + by: caller.name, changed, }); } catch (err) { @@ -297,13 +299,14 @@ export function registerViews(app: App): void { } }); - app.view(CONSENT_MODAL, async ({ ack, body, view }) => { - const caller = personBySlackId(body.user.id); - if (!caller || !mayAdministerWorkspace(caller)) { + app.view(CONSENT_MODAL, async ({ ack, body, view, client }) => { + const caller = await administrator(client, body.user.id); + if (!caller) { await ack({ response_action: "errors", errors: { - signed_on: "Only Lead Coaches and admins can record consent.", + signed_on: + "Only Slack workspace Owners and Admins can record consent.", }, }); return; @@ -339,11 +342,11 @@ export function registerViews(app: App): void { guardianName: textOf(state, "guardian_name"), guardianEmail: textOf(state, "guardian_email") || null, documentRef: textOf(state, "document_ref") || null, - recordedBy: caller.full_name, + recordedBy: caller.name, }); await ack(); await settleConsent(personId); - log.info("consent recorded", { personId, by: caller.full_name }); + log.info("consent recorded", { personId, by: caller.name }); } catch (err) { log.error("consent submission failed", { personId, error: String(err) }); await ack({ diff --git a/test/authz.test.ts b/test/authz.test.ts new file mode 100644 index 0000000..0b30473 --- /dev/null +++ b/test/authz.test.ts @@ -0,0 +1,117 @@ +import assert from "node:assert/strict"; +import { beforeEach, describe, it } from "node:test"; +import type { WebClient } from "@slack/web-api"; +import type { Person, Role } from "../src/domain/people.js"; +import { administrator, forgetAuthorization } from "../src/slack/authz.js"; + +type SlackFlags = { + is_admin?: boolean; + is_owner?: boolean; + is_primary_owner?: boolean; + is_bot?: boolean; + deleted?: boolean; + real_name?: string; +}; + +/** A WebClient with just enough of `users.info` to exercise the gate. */ +function slack(flags: SlackFlags | Error): { + client: WebClient; + calls: () => number; +} { + let calls = 0; + const client = { + users: { + info: async () => { + calls += 1; + if (flags instanceof Error) throw flags; + return { user: { id: "U1", name: "someone", ...flags } }; + }, + }, + } as unknown as WebClient; + return { client, calls: () => calls }; +} + +function person(role: Role): Person { + return { + id: 1, + slack_user_id: "U1", + email: "p1@example.org", + full_name: "Roster Name", + role, + active: 1, + ypp_completed_on: null, + ypt_completed_on: null, + mentor_ready_on: null, + cori_completed_on: null, + notes: null, + created_at: "2026-01-01T00:00:00.000Z", + updated_at: "2026-01-01T00:00:00.000Z", + }; +} + +const nobody = () => undefined; + +describe("who may administer hawk-mod", () => { + beforeEach(() => forgetAuthorization()); + + it("admits a workspace admin who is not on the roster at all", async () => { + // The whole point: a fresh install has an empty roster, and whoever + // installed the app must still be able to use it. + const { client } = slack({ is_admin: true, real_name: "Ty" }); + const actor = await administrator(client, "U1", { lookup: nobody }); + assert.equal(actor?.name, "Ty"); + assert.equal(actor?.person, undefined); + }); + + it("admits owners and primary owners", async () => { + for (const flags of [{ is_owner: true }, { is_primary_owner: true }]) { + forgetAuthorization(); + const { client } = slack(flags); + assert.ok(await administrator(client, "U1", { lookup: nobody })); + } + }); + + it("refuses an ordinary member, roster row or not", async () => { + const { client } = slack({ real_name: "Someone" }); + assert.equal( + await administrator(client, "U1", { lookup: () => person("adult") }), + null + ); + }); + + it("refuses a student who holds workspace admin (§6)", async () => { + // The sweep raises this as a violation; the finding names other students, + // so this one must not be readable by the student it is about. + const { client } = slack({ is_owner: true }); + assert.equal( + await administrator(client, "U1", { lookup: () => person("student") }), + null + ); + }); + + it("fails closed when Slack cannot be reached", async () => { + const { client } = slack(new Error("ratelimited")); + assert.equal(await administrator(client, "U1", { lookup: nobody }), null); + }); + + it("prefers the roster name, for a stable audit trail", async () => { + const { client } = slack({ is_admin: true, real_name: "tyt" }); + const actor = await administrator(client, "U1", { + lookup: () => person("adult"), + }); + assert.equal(actor?.name, "Roster Name"); + }); + + it("caches a verdict briefly, then re-reads it", async () => { + const { client, calls } = slack({ is_admin: true }); + await administrator(client, "U1", { lookup: nobody, now: 0 }); + await administrator(client, "U1", { lookup: nobody, now: 59_000 }); + assert.equal(calls(), 1); + await administrator(client, "U1", { lookup: nobody, now: 61_000 }); + assert.equal( + calls(), + 2, + "a revoked admin must lose access within a minute" + ); + }); +}); diff --git a/test/guidance.test.ts b/test/guidance.test.ts index f5020d3..42c9e02 100644 --- a/test/guidance.test.ts +++ b/test/guidance.test.ts @@ -71,7 +71,7 @@ describe("guidance to an adult", () => { assert.match(text, /add another screened adult/); }); - it("points an unknown participant at a Lead Coach, not at the adult", () => { + it("points an unknown participant at a coach, not at the adult", () => { const verdict = classifyConversation( "mpim", [ @@ -85,7 +85,7 @@ describe("guidance to an adult", () => { assert.equal(verdict.violation, "unknown_participant_with_student"); const text = guidanceFor(verdict); assert.ok(text); - assert.match(text, /ask a Lead Coach/); + assert.match(text, /ask a coach/); }); it("never accuses, and always says the finding still stands", () => { @@ -101,6 +101,6 @@ describe("guidance to an adult", () => { assert.match(text, /not a telling-off/); assert.match(text, /Nothing here is an accusation/); // Guidance must never read as a substitute for the alert. - assert.match(text, /Lead Coach has been notified/); + assert.match(text, /coach has been notified/); }); }); diff --git a/test/rosterSync.test.ts b/test/rosterSync.test.ts index 0ebcbfb..7e6ec5f 100644 --- a/test/rosterSync.test.ts +++ b/test/rosterSync.test.ts @@ -52,15 +52,7 @@ describe("user group reconciliation", () => { ); }); - it("does not demote a lead coach who is also in the adults group", () => { - const decisions = reconcileRoles( - roster(person("U1", "lead_coach")), - groups([], ["U1"]) - ); - assert.equal(decisions[0]!.kind, "unchanged"); - }); - - it("does not demote the district observer either", () => { + it("does not demote the district observer", () => { const decisions = reconcileRoles( roster(person("U1", "district_observer")), groups([], ["U1"]) diff --git a/test/rules.test.ts b/test/rules.test.ts index 61aa5bc..bccdf1d 100644 --- a/test/rules.test.ts +++ b/test/rules.test.ts @@ -209,10 +209,10 @@ describe("DM policy", () => { assert.equal(verdict.severity, "violation"); }); - it("flags a 1:1 even when the adult is fully screened and a Lead Coach", () => { + it("flags a 1:1 even when the adult is fully screened and senior", () => { const verdict = classifyConversation( "im", - [screened("lead_coach"), person("student")], + [screened("district_observer"), person("student")], asOf ); assert.equal(verdict.violation, "one_to_one_adult_student"); @@ -272,7 +272,7 @@ describe("DM policy", () => { [ screened(), screened(), - screened("lead_coach"), + screened("district_observer"), person("student"), person("student"), person("student"), @@ -323,7 +323,7 @@ describe("two screened adults", () => { channelId: "C3", channelName: "build", isPrivate: false, - members: [screened(), screened("lead_coach"), person("student")], + members: [screened(), screened("district_observer"), person("student")], }, asOf );