Answer to Slack's admins, not to a roster role - #1
Merged
Conversation
/hawkmod was gated on a `lead_coach` roster role. A fresh install has an empty roster, so a newly installed app had no administrator at all: the only way to get one was a shell on the host running `set-role`. An app whose first-run instruction is "SSH into the server" is broken, and the role was a hand-maintained second copy of something Slack already knows. Authority now comes from Slack's Workspace Owner/Admin flags, read live per command in `slack/authz.ts`. Whoever can install the app can run it. - Fails closed: a Slack API error denies rather than allows. Findings name students, so a retry costs less than a wrong read. - A student holding Owner/Admin is refused anyway. The sweep raises that as a §6 violation, and that finding names other students. - 60s cache: a burst of button clicks is one API call, and revoking someone's Slack admin revokes their access within the minute. - `users:read` was already a bot scope, so no manifest change. `lead_coach` and `admin` are removed from ROLES — both existed only to answer "may this person administer hawk-mod". Migration 0005 collapses them into `adult` and records each in role_changes. It leaves the retired names in the CHECK constraint on purpose: dropping a CHECK in SQLite means rebuilding `people`, parent of four ON DELETE CASCADE keys including `consents`, and destroying consent records to tidy a constraint is a bad trade. §3 keeps its check rather than losing it with the role. The sweep now asks whether two of the workspace's Owners/Admins are screened adults on the roster — the same question, put to the people who demonstrably hold the authority instead of to a self-assigned label. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
/hawkmodwas gated on alead_coachroster role. A fresh install has an empty roster, so a newly installed app had no administrator at all — the only way to get one was a shell on the host runningset-role. That is what this fixes: an app whose first-run instruction is "SSH into the server" is broken, and the role was a hand-maintained second copy of a fact Slack already knows.What changed
New
src/slack/authz.tsresolves the caller viausers.infoand admits onis_admin || is_owner || is_primary_owner. Every gate uses it:/hawkmod, the screening and consent modals, and the Resolve/Acknowledge buttons with their note modal.mayAdministerWorkspaceis deleted.users:readwas already a bot scope, so no manifest change and no reinstall.Roles
ROLESbecomesstudent | adult | district_observer.admingoes along withlead_coach— it existed for the same reason and would otherwise be a vestigial label granting nothing.district_observerstays; §8 gives it a distinct meaning.migrations/0005_authority_from_slack.sqlcollapses both retired roles intoadultand records each one inrole_changes. It deliberately leaves the old names in theCHECKconstraint: dropping a CHECK in SQLite means rebuildingpeople, which is the parent of fourON DELETE CASCADEkeys includingconsents, and a migration that can destroy consent records to tidy a constraint is a bad trade. Nothing can write the old values now —ROLESno longer contains them, the CLI validates againstROLES, and the user-group sync only ever writesstudentoradult.§3 keeps its check
FIRST requires two YPP-screened Lead Coaches, and that mapped onto the removed role. Rather than drop the control, the sweep now asks whether at least two of the workspace's Owners/Admins are screened adults on the roster, emitting a
workspace_config/screened_adminsfinding otherwise. Same question, put to the people who demonstrably hold the authority instead of to a label anyone could type.docs/policy-mapping.mdrecords the reasoning.Verification
npm run typecheck,npm test(61 pass; 7 new intest/authz.test.tscovering the non-rostered admin, owners and primary owners, ordinary members, the student-admin refusal, failing closed, audit naming, and cache expiry), andprettier --checkall clean.🤖 Generated with Claude Code