grain.social/grain

Switch repository

Loading…

What changes

Reports. grain-serve answers com.atproto.moderation.createReport, from a service token or the web's cookie, for an account (repoRef) or a record (strongRef). A report names atproto's reasonType and is filed under the grain label it asks for (grain::labels::label_for): atproto's and Ozone's reasons each map to one, Ozone's reasonChildSafety* to csae, and two new tokens, social.grain.moderation.defs#reasonCopyright and #reasonNudity, cover the labels neither has a reason for. An account's subject is its bare DID.

Admin methods. The accounts GRAIN_ADMINS names (DIDs, commas or spaces), each with a service token for the method from their own PDS, and only so (no cookie):

• social.grain.admin.listReports (status, label, cursor, total) and resolveReport (resolving gives out the report's label on its subject; dismissing does not);
• com.atproto.admin.updateSubjectStatus / getSubjectStatus: an account is taken down in takedown (which every read already leaves out) and by a !takedown label on its DID, negated when restored, in one transaction; a record by the !takedown label alone, which reads already leave out of lists;
• social.grain.admin.applyLabel / negateLabel, given out as the server's DID (refusing !takedown, which only updateSubjectStatus gives); listLabelDefinitions (the nine labels from server/labels/, now in grain::labels);
• social.grain.admin.getReviewSubject: an account's status, the labels standing on it and on its records, every report against them, the newest six records of each collection with their values and counts, and childSafety, true if a child-safety report (any state) or csae label (negated or not) ever touched it. The later MCP pull keys off that flag.

The classifiers. They file through Index::file_report_once (beside file_report, filing unless a report on the subject asking for the label is open), in place of their own report_once, and may_classify asks Index::child_safety: a report by its label or its child-safety reason type, in any state, a csae label negated or not, or an author's own csae self-label, on the account or any of its records. tests/classify.rs gains the reason-type-only case; tests/moderation.rs the self-label one.

Reporters are DIDs. A classifier's report is filed by the server's DID (its labeler, as its labels), the classifier named in its reason (<name>: <signal> <score>); with no labeler none is filed. grain-import gives the old server's non-DID reporters (system:<name>) and admin closings (admin) the server's DID, the old reporter kept at the head of the reason. listReports with a classifier's report goes through the answers check in tests/moderation.rs.

Labels out. com.atproto.label.queryLabels (public): standing labels by exact subject or * prefix, filtered by sources, paged by label id.

Schema and import. V11 adds report.reason_type and an index on report (subject_did, id). grain-import now gives hatk's own labels (src of admin or self, which are not DIDs and fail com.atproto.label.defs#label) out as grain-serve's DID: the did:web of GRAIN_PUBLIC_URL, or did:web:grain.social. It already carried _reports, _labels, _mutes and takedowns.

Lexicons. Copies of atproto's createReport, admin.updateSubjectStatus, admin.getSubjectStatus, label.queryLabels, the upstream moderation/defs.json, and the part of admin/defs.json they name. reportView gains reasonType; getReviewSubject gains reports and childSafety. hatk.generated*.ts regenerated (the old CreateReport/RepoRef types are now HatkCreateReport/HatkCreateReportRepoRef; nothing imports them).

Why

Step one of moderation off the old server: the admin web UI is dropped, and the CLI and an MCP server (the next two pulls, stacked on this) call these methods instead. The shapes follow atproto and Ozone where they have one, and the names lexicons/README.md already set for the old /admin routes.

Differences from the old admin:

• createReport takes reasonType, not one of grain's labels; a report keeps both.
• Admins authenticate with a service token for the method from their own account, not an OAuth session on the appview or the shared admin bearer token; there is no whoami.
• A record can be taken down (by label); the old server only took accounts down. An account's takedown is also said as a !takedown label, so it reaches queryLabels.
• Labels are given out as the server's DID, not admin.
• listReports pages by cursor, not offset.
• Not ported here: the review queue, scans and their stats (the classifier step's), actOnReview, repo add/remove/resync, getInfo, listRepos, searchRecords, labels/reset, labels/rescan.

How it was checked

crates/grain-serve/tests/moderation.rs drives every method through common against the reference PDS, with real service tokens for an admin and a non-admin: filing and refusing reports, paging and filtering them, resolving and dismissing, account and record takedowns and restores, labels given, negated and queried by subject, prefix, source and page, the whole-account view and its child-safety flag, refusals for non-admins, missing tokens and wrong-method tokens, and a server with no index. Unit tests cover the reason map and subjects; grain-import's tests cover the label source rewrite and GRAIN_PUBLIC_URL. Locally only cargo check, clippy and fmt; the tests, coverage and the answers check are left to the runner.

Shortcomings

• No com.atproto.label.subscribeLabels yet, and labels go out unsigned.
• getSubjectStatus for a record the index does not hold answers RecordNotFound, since a strongRef needs a CID.
• A record takedown leaves the record answering by its URI with its !takedown label, as other hiding labels do; clients must honour it.
• The migration is V11, after carryover's V8, push's V9 and the classifiers' V10 on feature/rust; a pull landing another V11 first means renumbering here.

Review

Start at crates/grain-serve/src/moderation.rs, then index/moderation.rs. The design worth a look is record takedown by label and the child-safety flag's breadth.

🤖 Generated with Claude Code

This adds createReport, queryLabels and the service-token admin methods (reports, takedown, labels, getReviewSubject with the childSafety flag). It also adds the V8 schema change and the grain-import label source rewrite. I read the whole of crates/grain-serve/src/moderation.rs and index/moderation.rs. I did not read the rest of the roughly 3,000-line diff: the lexicons, generated types, tests and grain-import changes. I did not check the CI results either.

The parts I read hold up:

• The admin gate requires an Authorization header and a service token for the specific method, and checks the caller against app.admins. It does not accept the web cookie.
• Account takedown writes the takedown row and the !takedown label in one transaction.
• close_report re-checks that the report is still open inside its transaction, so a double resolve cannot give the label twice.
• The SQL is parameterised, and the account-prefix range (at://<did>/ to at://<did>0) is correct.
• strongRef and repoRef inputs are validated, and the length caps are enforced.

Two points to know about, neither blocking:

• applyLabel with !takedown on a bare DID adds the label but does not set the takedown row. Only updateSubjectStatus does both. Consider rejecting !takedown in applyLabel and negateLabel, or routing it through set_takedown.
• Record takedown depends on every list read honouring the !takedown label. That is the design question you raised, and I did not trace the read paths.

The only change since my last review is a merge of feature/rust into the branch. The migration clash the description predicted is resolved: the carry-over table keeps V8, and this pull's report.reason_type column and index are now V9, listed in MIGRATIONS. I read only the V8 and V9 definitions and the MIGRATIONS list. I did not re-read the rest of the merge.

My earlier review asked for nothing blocking. Its two non-blocking notes are still open: applyLabel and negateLabel accept !takedown on a bare DID without touching the takedown row, and I did not trace the read paths that record takedown relies on. The CI checks were still queued when I looked, so none had run on this commit. Please confirm they pass before you merge.

This round adds two commits and a merge of feature/rust. One commit gives imported reports no reason_type. The other makes applyLabel and negateLabel refuse !takedown. The refusal I saw at crates/grain-serve/src/moderation.rs:468 rejects !takedown and points to updateSubjectStatus. That answers my earlier note: an account's takedown row and its label can no longer drift apart. I did not read the negateLabel handler or re-read the rest of the merge or the import change.

I still haven't traced the read paths that record takedown relies on. That is the design question you raised, and it isn't blocking. The checks on this commit (check, harness, test, rust) were queued or running when I looked, so please confirm they pass before you merge.

The only change since my last review is a merge of feature/rust that conflicted in crates/grain-serve/src/index/schema.rs. I didn't re-read the merge or the resolved conflict. Nothing I asked for earlier is outstanding: the !takedown refusal in applyLabel and negateLabel landed last round. I still haven't traced the read paths that record takedown relies on. That isn't blocking, but it is the design risk the description names. The four checks on this commit (check, harness, test, rust) were all queued when I looked. Please confirm they pass before you merge, and look at the schema conflict resolution in particular: the V8 and V9 migrations should both still be in MIGRATIONS.

Both notes are answered at a5130fe. applyLabel and negateLabel refuse !takedown with InvalidRequest naming updateSubjectStatus, so an account's takedown row and its label are only written together; moderation.rs tests it. For record takedown, every list read filters on HIDDEN_LABELS in index/schema.rs (through read/moderation.rs's shown and unhidden), and that list includes !takedown. At 75f73e6 feature/rust is merged again: push added a V9, so this pull's migration is now V10.

The only change since my last review is one commit: "A test runs holds_json, which FIELDS otherwise only runs at compile time". It touches tests only, and I didn't read it. Nothing I asked for earlier is outstanding, because the !takedown refusal in applyLabel and negateLabel landed in round 3. I still haven't traced the read paths that record takedown relies on. That isn't blocking, but it is the design risk the description names. The four checks on this commit (check, harness, test, rust) were all still queued when I looked, so none has run. Please confirm they pass before you merge.