feat(teams): reserved-name screening, auto-hide, and the admin-approval gate
The one place untrusted game data becomes a public page (docs/website/TEAMS.md
§2.8), and the gate on releasing it (§2.9).
A Team's name is written by a player, in the game, with no review, and this
platform turns it into a public page, a URL and eventually a Discord channel
name. Someone naming their guild "Admin" or "<Brand> Staff" gets an
official-looking page on the operator's own site for free.
Hide, never reject. Core cannot refuse a name -- the guild already exists in the
game and core is a mirror of it, not an authority over it. A match hides the Team
from public surfaces and files it in a review queue, and it keeps working
completely for its own members: their forum, their grants, their notifications.
The people in it are not being punished for a name their leader chose.
That asymmetry -- a false positive costs a human glance, a false negative costs
an impersonated staff page -- is what lets the matcher be conservative. It is not
licence to be sloppy the other way: a check that fires on "Badminton" gets
switched off, and then the real cost is paid in full. So matching is whole WORDS
after normalisation, never substrings, following the precedent
scripts/checkModuleIdentifiers.js set for exactly this reason.
Three matcher gaps found by writing the tests, all real impersonation vectors:
- "Guild of Moderators" did not match `moderator`. Only a trailing s off the
WHOLE term is stripped, so "Nomads" still does not match `mod`.
- "G.M." normalises to two single-letter words and matched nothing. A run of
two or more single-letter words is now also offered joined. Deliberately not
a whole-name condensation, which would re-admit substring matching.
- The multi-word condensed form was already handled and is what makes
"RunicGateway" match the two-word term -- the form an impersonator would
reach for, since it is what the Gitea org and every URL use.
Terms resolve at CHECK time, never baked in, so renaming a deployment protects
the new name without a redeploy. A failed settings read falls back to the static
role and project terms rather than to an empty list: screening fewer terms is
bad, screening none is the whole hole.
Re-screening runs on every reconcile, over names no human has ruled on. Names are
immutable per row, so it only ever changes an outcome when the TERM LIST changed
-- an operator adding one, or a rename -- which is exactly what a create-time-only
check would miss forever. `name_reviewed_at` is what makes a staff decision
sticky; without it an override would be undone every fifteen minutes.
The gate is scoped to three actions because they publish untrusted game-sourced
strings, and to nothing else. Ordinary forum grants, leadership overrides,
archives and forum moderation still apply immediately and are audited. A
moderator initiating one files a pending request; an admin applies at once.
Never four-eyes on admins: users.role defaults to admin and `npm run seed`
creates exactly one, so most deployments have precisely one and a second-approver
rule would wedge them with no way out.
Hiding is deliberately NOT gated. Publishing untrusted data needs a second pair
of eyes; withdrawing it needs to be possible at once, by whoever is on duty.
Two concurrency details worth the review: a decision moves the row out of
`pending` under a guard and applies its effect only if the row actually moved,
so two admins clicking approve cannot double-apply or overwrite each other's
record; and a JSON payload is parsed defensively, because the driver returns
JSON columns already parsed on some versions and as a string on others.
Screening is stubbed in the reconciler's own tests -- it is a separate unit, and
the real call reads settings, which this suite must never do against a live
database. That was caught the hard way: the suite went from 11s to hanging, and
the cause was the reconciler reaching a dead pool through the new call.
44 tests in the reconciler file (up from 39), 19 for the matcher, 25 for the
gate. Full suite 877 passed, 0 failed.
Refs docs/website/TEAMS.md §2.8, §2.9, Part 12 phase 2
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -11,6 +11,7 @@ const assert = require('node:assert/strict')
|
||||
|
||||
const registries = require('../src/modules/registries')
|
||||
const teamsDb = require('../src/model/teams/teams.db')
|
||||
const moderation = require('../src/model/teams/teamModeration.model')
|
||||
const settings = require('../src/model/settings/settings.model')
|
||||
const teamSync = require('../src/model/teams/teamSync.model')
|
||||
|
||||
@@ -169,6 +170,18 @@ function stubDb() {
|
||||
s.pending_empty_since = since
|
||||
store.sync.set(moduleId, s)
|
||||
})
|
||||
|
||||
// Reserved-name screening is its own unit (teamModeration.test.js). Stubbed
|
||||
// here so these tests stay about the reconciler — and because the real calls
|
||||
// read settings, which means a live database connection this suite must never
|
||||
// make. `screened` records that the reconciler asked, which is the integration
|
||||
// point worth asserting from this side.
|
||||
store.screened = []
|
||||
patch(moderation, 'screenForCreate', async (name) => {
|
||||
store.screened.push(name)
|
||||
return { hidden: false }
|
||||
})
|
||||
patch(moderation, 'rescreen', async () => 0)
|
||||
}
|
||||
|
||||
// A provider whose answers the test controls. Defaults are authoritative and
|
||||
@@ -612,6 +625,58 @@ test('a name with nothing URL-safe in it still gets an address', async () => {
|
||||
assert.equal(store.teams[0].name, '★☆★', 'the identity keeps what the player typed')
|
||||
})
|
||||
|
||||
// ── Screening is on the create path, and on every run ──────────────────────
|
||||
|
||||
test('every newly created team has its name screened', async () => {
|
||||
provide({ getTeams: async () => ({ ok: true, teams: [team('g1', 'Admin'), team('g2', 'The Silver Hand')] }) })
|
||||
await teamSync.reconcileNow('test')
|
||||
assert.deepEqual(store.screened, ['Admin', 'The Silver Hand'])
|
||||
})
|
||||
|
||||
test('a renamed team is screened again under its new name', async () => {
|
||||
provide({ getTeams: async () => ({ ok: true, teams: [team('g1', 'Ordinary')] }) })
|
||||
await teamSync.reconcileNow('setup')
|
||||
|
||||
registries._reset()
|
||||
provide({ getTeams: async () => ({ ok: true, teams: [team('g1', 'Admin')] }) })
|
||||
await teamSync.reconcileNow('rename')
|
||||
assert.deepEqual(store.screened, ['Ordinary', 'Admin'], 'a rename is a create, so it screens')
|
||||
})
|
||||
|
||||
test('a hidden team is still created and still syncs its roster', async () => {
|
||||
// Hide, never reject: the Team works completely for its own members. The people
|
||||
// in it are not being punished for a name their leader chose.
|
||||
patch(moderation, 'screenForCreate', async () => ({
|
||||
hidden: true, hiddenReason: 'reserved_name', hiddenTerm: 'admin',
|
||||
}))
|
||||
provide({
|
||||
getTeams: async () => ({ ok: true, teams: [team('g1', 'Admin')] }),
|
||||
getTeamMembers: async () => ({ ok: true, members: [member('0x1'), member('0x2')] }),
|
||||
})
|
||||
await teamSync.reconcileNow('test')
|
||||
|
||||
assert.equal(store.teams[0].hidden, 1)
|
||||
assert.equal(store.teams[0].hidden_term, 'admin')
|
||||
assert.equal(activeMembers(1).length, 2, 'suppression is a public-surface rule, not a shutdown')
|
||||
assert.equal(store.teams[0].member_count, 2)
|
||||
})
|
||||
|
||||
test('a successful run re-screens the names no human has ruled on', async () => {
|
||||
let called = 0
|
||||
patch(moderation, 'rescreen', async () => { called += 1; return 0 })
|
||||
provide()
|
||||
await teamSync.reconcileNow('test')
|
||||
assert.equal(called, 1)
|
||||
})
|
||||
|
||||
test('a refused run does not re-screen — it does nothing at all', async () => {
|
||||
let called = 0
|
||||
patch(moderation, 'rescreen', async () => { called += 1; return 0 })
|
||||
provide({ getTeams: async () => ({ ok: false, reason: 'down' }) })
|
||||
await teamSync.reconcileNow('test')
|
||||
assert.equal(called, 0)
|
||||
})
|
||||
|
||||
// ── Events (§2.3) ──────────────────────────────────────────────────────────
|
||||
|
||||
test('an unknown event kind is rejected', async () => {
|
||||
|
||||
Reference in New Issue
Block a user